lkml.org 
[lkml]   [2020]   [Oct]   [5]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH v4 2/3] iommu/tegra-smmu: Rework tegra_smmu_probe_device()
    On Mon, Oct 05, 2020 at 11:57:54AM +0200, Thierry Reding wrote:
    > On Fri, Oct 02, 2020 at 11:58:29AM -0700, Nicolin Chen wrote:
    > > On Fri, Oct 02, 2020 at 06:02:18PM +0300, Dmitry Osipenko wrote:
    > > > 02.10.2020 09:08, Nicolin Chen пишет:
    > > > > static int tegra_smmu_of_xlate(struct device *dev,
    > > > > struct of_phandle_args *args)
    > > > > {
    > > > > + struct platform_device *iommu_pdev = of_find_device_by_node(args->np);
    > > > > + struct tegra_mc *mc = platform_get_drvdata(iommu_pdev);
    > > > > u32 id = args->args[0];
    > > > >
    > > > > + of_node_put(args->np);
    > > >
    > > > of_find_device_by_node() takes device reference and not the np
    > > > reference. This is a bug, please remove of_node_put().
    > >
    > > Looks like so. Replacing it with put_device(&iommu_pdev->dev);
    >
    > Putting the put_device() here is wrong, though. You need to make sure
    > you keep a reference to it as long as you keep accessing the data that
    > is owned by it.

    I am confused. You said in the other reply (to Dmitry) that we do
    need to put_device(mc->dev), where mc->dev should be the same as
    iommu_pdev->dev. But here your comments sounds that we should not
    put_device at all since ->probe_device/group_device/attach_dev()
    will use it later.

    > Like I said earlier, this is a bit weird in this case because we're
    > self-referencing, so iommu_pdev->dev is going to stay around as long as
    > the SMMU is. However, it might be worth to properly track the lifetime
    > anyway just so that the code can serve as a good example of how to do
    > things.

    What's this "track-the-lifetime"?

    > If you decide to go for the shortcut and not track this reference
    > properly, then at least you need to add a comment as to why it is safe
    > to do in this case. This ensures that readers are away of the
    > circumstances and don't copy this bad code into a context where the
    > circumstances are different.

    I don't quite get this "shortcut" here either...mind elaborating?

    \
     
     \ /
      Last update: 2020-10-06 03:13    [W:4.750 / U:0.016 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site