On Fri, Oct 02, 2020 at 04:55:34AM +0300, Dmitry Osipenko wrote: > 02.10.2020 04:07, Nicolin Chen пишет: > > On Thu, Oct 01, 2020 at 11:33:38PM +0300, Dmitry Osipenko wrote: > >>>>> If we can't come to an agreement on globalizing mc pointer, would > >>>>> it be possible to pass tegra_mc_driver through tegra_smmu_probe() > >>>>> so we can continue to use driver_find_device_by_fwnode() as v1? > >>>>> > >>>>> v1: https://lkml.org/lkml/2020/9/26/68 > >>>> > >>>> tegra_smmu_probe() already takes a struct tegra_mc *. Did you mean > >>>> tegra_smmu_probe_device()? I don't think we can do that because it isn't > >>> > >>> I was saying to have a global parent_driver pointer: similar to > >>> my v1, yet rather than "extern" the tegra_mc_driver, we pass it > >>> through egra_smmu_probe() and store it in a static global value > >>> so as to call tegra_smmu_get_by_fwnode() in ->probe_device(). > >>> > >>> Though I agree that creating a global device pointer (mc) might > >>> be controversial, yet having a global parent_driver pointer may > >>> not be against the rule, considering that it is common in iommu > >>> drivers to call driver_find_device_by_fwnode in probe_device(). > >> > >> You don't need the global pointer if you have SMMU OF node. > >> > >> You could also get driver pointer from mc->dev->driver. > >> > >> But I don't think you need to do this at all. The probe_device() could > >> be invoked only for the tegra_smmu_ops and then seems you could use > >> dev_iommu_priv_set() in tegra_smmu_of_xlate(), like sun50i-iommu driver > >> does. > > > > Getting iommu device pointer using driver_find_device_by_fwnode() > > is a common practice in ->probe_device() of other iommu drivers. > > Please give me a full list of the IOMMU drivers which use this method. > > > But this requires a device_driver pointer that tegra-smmu doesn't > > have. So passing tegra_mc_driver through tegra_smmu_probe() will > > address it. > > > > If you're borrowing code and ideas from other drivers, then at least > please borrow them from a modern good-looking drivers. And I already > pointed out that following cargo cult is not always a good idea. > > ARM-SMMU isn't a modern driver and it has legacy code. You shouldn't > copy it blindly. The sun50i-iommu driver was added half year ago, you > may use it as a reference. I took a closer look at sun50i-iommu driver. It's a good idea. I think I can come up with a cleaner one. Will send v4. Thanks for the advice.
