Re: [RFC] early init and DT platform devices allocation/registration
On Fri, Jun 28, 2013 at 10:49:15AM +0200, Hiroshi Doyu wrote: Grant Likely grant.lik...@secretlab.ca wrote @ Wed, 26 Jun 2013 12:03:20 +0200: On Wed, Jun 26, 2013 at 7:00 AM, Hiroshi Doyu hd...@nvidia.com wrote: Grant Likely grant.lik...@secretlab.ca wrote @ Tue, 25 Jun 2013 19:52:33 +0200: Here's my workaround. I need to call of_detach_node() with OF_DYNAMIC to avoid duplicated device registration. Gah! my eyes! Don't do that. It is incredibly problematic. Look at inhibiting duplicate device creation instead. I may not follow this thread correctly, but could anyone point out the above inhibiting duplicate device creation if there's already such solution? No, the solution doesn't exist yet, but it wouldn't be hard to implement. What you need to do is to add a struct device pointer to struct device_node, and set the pointer to the struct device when of_platform_device_create creates a device. (it would also need to be set for early_platform_device creation, but that's not something that should affect you). You would also add a check to of_platform_device_create to check if the device pointer is already set. If it is, then skip creation of the device. Implemented as Grant suggested. At least this works for our case, where IOMMU needs to be instanciated earlier than other device[1]. early_platform_device case still need to be covered. I think we arrived at a different conclusion in another branch of this thread. With the patch below every driver needs to explicitly allocate the platform device and set the struct device_node's .dev field, which has other side-effects such as the device hierarchy getting messed up. A better alternative would be to have of_platform_populate() run early such that the .dev field in the struct device_node can be set by core code, which would not require every driver to be changed. I'm not sure exactly what needs to be done to make this work, however. Grant, can you provide some guidance here as to how this may be fixed? Where would we have to call of_platform_populate() from and what makes this break with the current implementation? Thierry pgpRFQaDjxGkx.pgp Description: PGP signature ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [RFC] early init and DT platform devices allocation/registration
On Fri, Jun 28, 2013 at 11:38 AM, Thierry Reding thierry.red...@gmail.com wrote: On Fri, Jun 28, 2013 at 10:49:15AM +0200, Hiroshi Doyu wrote: Grant Likely grant.lik...@secretlab.ca wrote @ Wed, 26 Jun 2013 12:03:20 +0200: On Wed, Jun 26, 2013 at 7:00 AM, Hiroshi Doyu hd...@nvidia.com wrote: Grant Likely grant.lik...@secretlab.ca wrote @ Tue, 25 Jun 2013 19:52:33 +0200: Here's my workaround. I need to call of_detach_node() with OF_DYNAMIC to avoid duplicated device registration. Gah! my eyes! Don't do that. It is incredibly problematic. Look at inhibiting duplicate device creation instead. I may not follow this thread correctly, but could anyone point out the above inhibiting duplicate device creation if there's already such solution? No, the solution doesn't exist yet, but it wouldn't be hard to implement. What you need to do is to add a struct device pointer to struct device_node, and set the pointer to the struct device when of_platform_device_create creates a device. (it would also need to be set for early_platform_device creation, but that's not something that should affect you). You would also add a check to of_platform_device_create to check if the device pointer is already set. If it is, then skip creation of the device. Implemented as Grant suggested. At least this works for our case, where IOMMU needs to be instanciated earlier than other device[1]. early_platform_device case still need to be covered. I think we arrived at a different conclusion in another branch of this thread. With the patch below every driver needs to explicitly allocate the platform device and set the struct device_node's .dev field, which has other side-effects such as the device hierarchy getting messed up. A better alternative would be to have of_platform_populate() run early such that the .dev field in the struct device_node can be set by core code, which would not require every driver to be changed. I'm not sure exactly what needs to be done to make this work, however. Grant, can you provide some guidance here as to how this may be fixed? Where would we have to call of_platform_populate() from and what makes this break with the current implementation? Try it and see! :-) I cannot give a definite answer, but I suspect that it will fall down when registering the devices on to the platform_bus because it isn't initialized yet. If called early, the code would need to hold the platform_device in some kind of deferred list until the platform bus was initialize, and then have a cleanup function at initcall time to finish the registration of all early devices. g. ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [RFC] early init and DT platform devices allocation/registration
Grant Likely grant.lik...@secretlab.ca wrote @ Tue, 25 Jun 2013 19:52:33 +0200: Here's my workaround. I need to call of_detach_node() with OF_DYNAMIC to avoid duplicated device registration. Gah! my eyes! Don't do that. It is incredibly problematic. Look at inhibiting duplicate device creation instead. I may not follow this thread correctly, but could anyone point out the above inhibiting duplicate device creation if there's already such solution? ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [RFC] early init and DT platform devices allocation/registration
Hi Lorenzo, On Wed, Jun 26, 2013 at 2:07 AM, Lorenzo Pieralisi lorenzo.pieral...@arm.com wrote: On Tue, Jun 25, 2013 at 03:56:22PM +0100, Stephen Warren wrote: On 06/25/2013 05:45 AM, Grant Likely wrote: On Mon, Jun 24, 2013 at 5:33 PM, Lorenzo Pieralisi lorenzo.pieral...@arm.com wrote: Hi all, I am dealing with a lingering problem related to init and probing of platform devices early (before initcalls) in the kernel boot process. The problem, which is nothing new, is related to how platform devices are created in the kernel from DT and when they become available. Platform devices are created through (assuming any legacy static initialization is removed from the kernel) of_platform_populate() at arch_initcall time on ARM. This is a problem for drivers that need to be probed and initialized before arch_initcall (ie early_initcall) because the corresponding platform_device has not been created yet. What's the use-case here; why does the driver /have/ to be allocated/probed so early? Can't drivers all be allocated from DT in the usual fashion, and any dependencies resolved using deferred probe? In almost all cases, that should work. The driver must be initialized in order to boot secondary CPUs, so very very early, its initialization cannot be deferred. Otherwise we would end up with MCPM back-end having to add ad-hoc kludges to boot secondary CPUs, and then replace the early function calls to the power controller drivers when the power controller has been properly initialized. Feasible, but really horrible. How about moving part of the SMP initialization to later during boot? In particular I mean the bring up of the secondary cores. Just booting with a single CPU for a bit longer is certainly doable because it is today possible to bring secondary CPU cores up from user space via the hotplug interface. Below is some old prototype code that I hacked up a while ago - note that I copied the original comment about that this should be done in user space. =) I'd like to have secondary CPU cores initialized later so I can use the regular device model (DT or platform device) to describe one or more power domain hardware devices. These power domains control the CPU cores, so there is a dependency on the SMP bring up code... Cheers, / magnus --- 0001/include/linux/smp.h +++ work/include/linux/smp.h2013-05-21 20:08:42.0 +0900 @@ -124,6 +124,7 @@ void smp_prepare_boot_cpu(void); extern unsigned int setup_max_cpus; extern void __init setup_nr_cpu_ids(void); extern void __init smp_init(void); +extern void __init smp_late_init(void); #else /* !SMP */ --- 0001/init/main.c +++ work/init/main.c2013-05-21 20:09:34.0 +0900 @@ -334,6 +334,7 @@ static void __init smp_init(void) static inline void setup_nr_cpu_ids(void) { } static inline void smp_prepare_cpus(unsigned int maxcpus) { } +static inline void smp_late_init(void) { } #endif /* @@ -879,6 +880,8 @@ static noinline void __init kernel_init_ do_basic_setup(); + smp_late_init(); + /* Open the /dev/console on the rootfs, this should never fail */ if (sys_open((const char __user *) /dev/console, O_RDWR, 0) 0) pr_err(Warning: unable to open an initial console.\n); --- 0001/kernel/smp.c +++ work/kernel/smp.c 2013-05-21 20:07:06.0 +0900 @@ -549,6 +549,20 @@ void __init smp_init(void) if (!cpu_online(cpu)) cpu_up(cpu); } +} + +/* Called by boot processor late during boot to activate the rest. */ +void __init smp_late_init(void) +{ + unsigned int cpu; + + /* FIXME: This should be done in userspace --RR */ + for_each_present_cpu(cpu) { + if (num_online_cpus() = setup_max_cpus) + break; + if (!cpu_online(cpu)) + cpu_up(cpu); + } /* Any cleanup work */ printk(KERN_INFO Brought up %ld CPUs\n, (long)num_online_cpus()); Cheers, / magnus ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [RFC] early init and DT platform devices allocation/registration
Hi Magnus, thank you. On Wed, Jun 26, 2013 at 07:20:32AM +0100, Magnus Damm wrote: Hi Lorenzo, On Wed, Jun 26, 2013 at 2:07 AM, Lorenzo Pieralisi lorenzo.pieral...@arm.com wrote: On Tue, Jun 25, 2013 at 03:56:22PM +0100, Stephen Warren wrote: On 06/25/2013 05:45 AM, Grant Likely wrote: On Mon, Jun 24, 2013 at 5:33 PM, Lorenzo Pieralisi lorenzo.pieral...@arm.com wrote: Hi all, I am dealing with a lingering problem related to init and probing of platform devices early (before initcalls) in the kernel boot process. The problem, which is nothing new, is related to how platform devices are created in the kernel from DT and when they become available. Platform devices are created through (assuming any legacy static initialization is removed from the kernel) of_platform_populate() at arch_initcall time on ARM. This is a problem for drivers that need to be probed and initialized before arch_initcall (ie early_initcall) because the corresponding platform_device has not been created yet. What's the use-case here; why does the driver /have/ to be allocated/probed so early? Can't drivers all be allocated from DT in the usual fashion, and any dependencies resolved using deferred probe? In almost all cases, that should work. The driver must be initialized in order to boot secondary CPUs, so very very early, its initialization cannot be deferred. Otherwise we would end up with MCPM back-end having to add ad-hoc kludges to boot secondary CPUs, and then replace the early function calls to the power controller drivers when the power controller has been properly initialized. Feasible, but really horrible. How about moving part of the SMP initialization to later during boot? In particular I mean the bring up of the secondary cores. Just booting with a single CPU for a bit longer is certainly doable because it is today possible to bring secondary CPU cores up from user space via the hotplug interface. Below is some old prototype code that I hacked up a while ago - note that I copied the original comment about that this should be done in user space. =) Well, the question is probably why smp_init() is called before do_basic_setup() as the kernel does now. We need to check if there is something we are missing or not, and I guess the only way to do it is to post your patch to LKML. I'd like to have secondary CPU cores initialized later so I can use the regular device model (DT or platform device) to describe one or more power domain hardware devices. These power domains control the CPU cores, so there is a dependency on the SMP bring up code... Looks like we are on the same boat then :-). This way we will solve this particular issue(s), but still can not rely on DT to initialize early platform devices. This stuff requires some serious thought, that's for certain, I will try to allocate some time to have a thorough look into this. --- 0001/include/linux/smp.h +++ work/include/linux/smp.h2013-05-21 20:08:42.0 +0900 @@ -124,6 +124,7 @@ void smp_prepare_boot_cpu(void); extern unsigned int setup_max_cpus; extern void __init setup_nr_cpu_ids(void); extern void __init smp_init(void); +extern void __init smp_late_init(void); #else /* !SMP */ --- 0001/init/main.c +++ work/init/main.c2013-05-21 20:09:34.0 +0900 @@ -334,6 +334,7 @@ static void __init smp_init(void) static inline void setup_nr_cpu_ids(void) { } static inline void smp_prepare_cpus(unsigned int maxcpus) { } +static inline void smp_late_init(void) { } #endif /* @@ -879,6 +880,8 @@ static noinline void __init kernel_init_ do_basic_setup(); + smp_late_init(); + /* Open the /dev/console on the rootfs, this should never fail */ if (sys_open((const char __user *) /dev/console, O_RDWR, 0) 0) pr_err(Warning: unable to open an initial console.\n); --- 0001/kernel/smp.c +++ work/kernel/smp.c 2013-05-21 20:07:06.0 +0900 @@ -549,6 +549,20 @@ void __init smp_init(void) if (!cpu_online(cpu)) cpu_up(cpu); } +} + +/* Called by boot processor late during boot to activate the rest. */ +void __init smp_late_init(void) +{ + unsigned int cpu; + + /* FIXME: This should be done in userspace --RR */ + for_each_present_cpu(cpu) { + if (num_online_cpus() = setup_max_cpus) + break; + if (!cpu_online(cpu)) + cpu_up(cpu); + } /* Any cleanup work */ printk(KERN_INFO Brought up %ld CPUs\n, (long)num_online_cpus()); You probably need to remove the same code from smp_init() then ? If you feel like posting this patch as an RFC (even if it just were to draw attention to the problem) please go ahead. I still think this is a work around to make things work, and we are still not solving
Re: [RFC] early init and DT platform devices allocation/registration
On Wed, Jun 26, 2013 at 7:00 AM, Hiroshi Doyu hd...@nvidia.com wrote: Grant Likely grant.lik...@secretlab.ca wrote @ Tue, 25 Jun 2013 19:52:33 +0200: Here's my workaround. I need to call of_detach_node() with OF_DYNAMIC to avoid duplicated device registration. Gah! my eyes! Don't do that. It is incredibly problematic. Look at inhibiting duplicate device creation instead. I may not follow this thread correctly, but could anyone point out the above inhibiting duplicate device creation if there's already such solution? No, the solution doesn't exist yet, but it wouldn't be hard to implement. What you need to do is to add a struct device pointer to struct device_node, and set the pointer to the struct device when of_platform_device_create creates a device. (it would also need to be set for early_platform_device creation, but that's not something that should affect you). You would also add a check to of_platform_device_create to check if the device pointer is already set. If it is, then skip creation of the device. g. ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [RFC] early init and DT platform devices allocation/registration
On Wed, Jun 26, 2013 at 11:03:20AM +0100, Grant Likely wrote: On Wed, Jun 26, 2013 at 7:00 AM, Hiroshi Doyu hd...@nvidia.com wrote: Grant Likely grant.lik...@secretlab.ca wrote @ Tue, 25 Jun 2013 19:52:33 +0200: Here's my workaround. I need to call of_detach_node() with OF_DYNAMIC to avoid duplicated device registration. Gah! my eyes! Don't do that. It is incredibly problematic. Look at inhibiting duplicate device creation instead. I may not follow this thread correctly, but could anyone point out the above inhibiting duplicate device creation if there's already such solution? No, the solution doesn't exist yet, but it wouldn't be hard to implement. What you need to do is to add a struct device pointer to struct device_node, and set the pointer to the struct device when of_platform_device_create creates a device. (it would also need to be set for early_platform_device creation, but that's not something that should affect you). You would also add a check to of_platform_device_create to check if the device pointer is already set. If it is, then skip creation of the device. One problem with this method is that every driver that needs or wants the device early has to do it explicitly, but I guess we can't have it all. I find this solution rather appealing because it allows for instance the Marvell Armada 370/XP IRQ chip driver to do this as well so we no longer have to keep around the extra struct device_node * in msi_chip but we can actually reuse the one from struct device instead. Thierry pgpMlqnnQY3WG.pgp Description: PGP signature ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [RFC] early init and DT platform devices allocation/registration
On Wed, Jun 26, 2013 at 11:31 AM, Thierry Reding thierry.red...@gmail.com wrote: On Wed, Jun 26, 2013 at 11:03:20AM +0100, Grant Likely wrote: On Wed, Jun 26, 2013 at 7:00 AM, Hiroshi Doyu hd...@nvidia.com wrote: Grant Likely grant.lik...@secretlab.ca wrote @ Tue, 25 Jun 2013 19:52:33 +0200: Here's my workaround. I need to call of_detach_node() with OF_DYNAMIC to avoid duplicated device registration. Gah! my eyes! Don't do that. It is incredibly problematic. Look at inhibiting duplicate device creation instead. I may not follow this thread correctly, but could anyone point out the above inhibiting duplicate device creation if there's already such solution? No, the solution doesn't exist yet, but it wouldn't be hard to implement. What you need to do is to add a struct device pointer to struct device_node, and set the pointer to the struct device when of_platform_device_create creates a device. (it would also need to be set for early_platform_device creation, but that's not something that should affect you). You would also add a check to of_platform_device_create to check if the device pointer is already set. If it is, then skip creation of the device. One problem with this method is that every driver that needs or wants the device early has to do it explicitly, but I guess we can't have it all. The other option is to modify of_device_add() to allow it to be called before the platform bus is set up, and then to 'fixup' the registrations at initcall time. That would mean of_platform_populate can be called really early and it would solve the problem of preserving the device hierarchy. g. ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [RFC] early init and DT platform devices allocation/registration
On Wed, Jun 26, 2013 at 1:44 PM, Sebastian Hesselbarth sebastian.hesselba...@gmail.com wrote: On 06/26/13 12:03, Grant Likely wrote: On Wed, Jun 26, 2013 at 7:00 AM, Hiroshi Doyu hd...@nvidia.com wrote: Grant Likely grant.lik...@secretlab.ca wrote @ Tue, 25 Jun 2013 19:52:33 +0200: Here's my workaround. I need to call of_detach_node() with OF_DYNAMIC to avoid duplicated device registration. Gah! my eyes! Don't do that. It is incredibly problematic. Look at inhibiting duplicate device creation instead. I may not follow this thread correctly, but could anyone point out the above inhibiting duplicate device creation if there's already such solution? No, the solution doesn't exist yet, but it wouldn't be hard to implement. What you need to do is to add a struct device pointer to struct device_node, and set the pointer to the struct device when of_platform_device_create creates a device. (it would also need to be set for early_platform_device creation, but that's not something that should affect you). You would also add a check to of_platform_device_create to check if the device pointer is already set. If it is, then skip creation of the device. Grant, What about the other way round, i.e. check if there is a device with .of_node pointed to the struct device_node currently at of_platform_device_create? That will avoid adding struct device to struct device_node which you fought against for good reasons. The main thing is that it means searching through the entire list of platform devices every time a new platform device is created. That seems unnecessarily expensive to me. Also, I guess of_platform_device_create could be exported and used by anyone who wants to create platform_devices early. Yes, but in that case it is probably better for them to call of_platform_populate early if of_platform_device_create is fixed to support early calling. Then you'd just set up all the devices earlier in init, allow drivers that support early probing to do so, and then everything else uses the normal initcall path. g. ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [RFC] early init and DT platform devices allocation/registration
On Mon, Jun 24, 2013 at 5:33 PM, Lorenzo Pieralisi lorenzo.pieral...@arm.com wrote: Hi all, I am dealing with a lingering problem related to init and probing of platform devices early (before initcalls) in the kernel boot process. The problem, which is nothing new, is related to how platform devices are created in the kernel from DT and when they become available. Platform devices are created through (assuming any legacy static initialization is removed from the kernel) of_platform_populate() at arch_initcall time on ARM. This is a problem for drivers that need to be probed and initialized before arch_initcall (ie early_initcall) because the corresponding platform_device has not been created yet. To work around this awkward situation, a driver, instead of relying on platform driver probing mechanism, can get initialized using early_initcall mechanism and rely on DT helpers (ie of_iomap() and company) to initialize resources. The problem comes when the driver functions must rely on an API (eg regmap) that requires a struct device to function properly; since the platform_device has not been created yet at early_initcall time, the driver cannot rely on it. The only solution consists in allocating a platform_device on the fly at driver init through of_platform_device_create() and use that device to initialize the (eg regmap) API. There is an issue with this solution, basically a platform device is allocated twice for a given device node - compatible property (because of_platform_populate() allocates a platform device for a node containing a compatible string even if a platform device has already been allocated for that device node). The second solution relies on early platform devices. Early platform devices are added by mach code and driver probing is carried out using the early platform device probing mechanism early_platform_driver_probe() The drawback with this solution is, AFAIK, it does not play well with DT, since the platform device duplication issue is still there (ie of_platform_populate() will allocate a platform device which is a duplicate of the one allocated at early boot to make the early platform device initialization possible). Please correct me if I am wrong, just trying to untangle a problem that does not look easy to solve. How this problem is supposed to be solved in the kernel ? 1- drivers that are to be up and running at early_initcall time must not rely on the device/driver model (but then they cannot use any API that requires a struct device to function (eg regmap)) 2- the driver should allocate a platform device at early initcall from a DT compatible node. Do not know how to deal with platform device duplication though, since of_platform_populate() will create another platform device when the node is parsed While I've resisted it in the past, I would be okay with adding struct device pointer in the device_node structure. I've resisted because I don't want drivers following the device_node pointer and making an assumption about what /kind/ of device is pointed to by it. However, this is an important use case and it makes it feasible to use an early platform device with of_platform_populate. Alternately, if others chime in and think it is too risky to have the device pointer in the device node, we could simply add a flag to the device_node which indicates the node has been parsed by of_platform_populate. There would need to be some careful coding to make sure that any call to early platform device creation sets the pointer in the device node correctly. I would also make it a requirement that any early platform device *must* be converted into a 'real' platform device during initcall time. That includes being possible to be freed correctly. Static early platform device definitions should not be allowed, otherwise there needs to be a special case for the platform device release hook. I really don't want that. Something else that needs to be investigated is how the device hierarchy will be affected by using early_platform_device. We still want the platform_device to appear in the same place in the hierarchy regardless of whether or not it was created 'early'. the question is how to make sure that actually happens. 3- driver should rely on early platform device/driver, but this does not solve (?) the platform device duplication problem either, it will happen when of_platform_populate() parses the DT and creates devices on the fly In theory there are other solutions such as: (a) declaring the platform device statically in arm/mach-* code and do not define the device node in dts, basically going back in time to ARM legacy kernel mechanism for this kind of devices As stated above, no. Static device definitions are not a good idea, and it doesn't scale in a multiplatform kernel world. (b) add a way to of_platform_populate() to exclude some compatible strings from being matched This method
Re: [RFC] early init and DT platform devices allocation/registration
On 06/25/2013 05:45 AM, Grant Likely wrote: On Mon, Jun 24, 2013 at 5:33 PM, Lorenzo Pieralisi lorenzo.pieral...@arm.com wrote: Hi all, I am dealing with a lingering problem related to init and probing of platform devices early (before initcalls) in the kernel boot process. The problem, which is nothing new, is related to how platform devices are created in the kernel from DT and when they become available. Platform devices are created through (assuming any legacy static initialization is removed from the kernel) of_platform_populate() at arch_initcall time on ARM. This is a problem for drivers that need to be probed and initialized before arch_initcall (ie early_initcall) because the corresponding platform_device has not been created yet. What's the use-case here; why does the driver /have/ to be allocated/probed so early? Can't drivers all be allocated from DT in the usual fashion, and any dependencies resolved using deferred probe? In almost all cases, that should work. ... How this problem is supposed to be solved in the kernel ? 1- drivers that are to be up and running at early_initcall time must not rely on the device/driver model (but then they cannot use any API that requires a struct device to function (eg regmap)) 2- the driver should allocate a platform device at early initcall from a DT compatible node. Do not know how to deal with platform device duplication though, since of_platform_populate() will create another platform device when the node is parsed While I've resisted it in the past, I would be okay with adding struct device pointer in the device_node structure. I've resisted because I don't want drivers following the device_node pointer and making an assumption about what /kind/ of device is pointed to by it. However, this is an important use case and it makes it feasible to use an early platform device with of_platform_populate. Hiroshi (who I have CC'd here) has also been asking about this same issue downstream. The issue for us is that we need to initialize an SMMU driver before any devices that are translated by the SMMU. One option is to force the register/probe of the SMMU driver explicitly, early in the machine's .init_machine() callback, before of_platform_populate(), to ensure the ordering. Then, we run into the same duplicate device issue, and the change Grant mentions above would help solve this. ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [RFC] early init and DT platform devices allocation/registration
Stephen Warren swar...@wwwdotorg.org wrote @ Tue, 25 Jun 2013 16:56:22 +0200: How this problem is supposed to be solved in the kernel ? 1- drivers that are to be up and running at early_initcall time must not rely on the device/driver model (but then they cannot use any API that requires a struct device to function (eg regmap)) 2- the driver should allocate a platform device at early initcall from a DT compatible node. Do not know how to deal with platform device duplication though, since of_platform_populate() will create another platform device when the node is parsed While I've resisted it in the past, I would be okay with adding struct device pointer in the device_node structure. I've resisted because I don't want drivers following the device_node pointer and making an assumption about what /kind/ of device is pointed to by it. However, this is an important use case and it makes it feasible to use an early platform device with of_platform_populate. Hiroshi (who I have CC'd here) has also been asking about this same issue downstream. The issue for us is that we need to initialize an SMMU driver before any devices that are translated by the SMMU. One option is to force the register/probe of the SMMU driver explicitly, early in the machine's .init_machine() callback, before of_platform_populate(), to ensure the ordering. Then, we run into the same duplicate device issue, and the change Grant mentions above would help solve this. Here's my workaround. I need to call of_detach_node() with OF_DYNAMIC to avoid duplicated device registration. From f4d88b8521c278b41b72028d326c03cfd2e90af8 Mon Sep 17 00:00:00 2001 From: Hiroshi Doyu hd...@nvidia.com Date: Fri, 14 Jun 2013 15:22:02 +0300 Subject: [PATCH 1/1] ARM: tegra: Populate AHB/IOMMU earlier than others Populate AHB/IOMMU earlier than others. IOMMU depends on AHB. IOMMU needs to be instanciated earlier than others so that IOMMU can register other platform devices as IOMMU'able. Once IOMMU is populated, IOMMU/AHB nodes are detached to prevent another registeration. Signed-off-by: Hiroshi Doyu hd...@nvidia.com --- arch/arm/mach-tegra/Kconfig | 1 + arch/arm/mach-tegra/tegra.c | 24 drivers/iommu/tegra-smmu.c | 3 +++ 3 files changed, 28 insertions(+) diff --git a/arch/arm/mach-tegra/Kconfig b/arch/arm/mach-tegra/Kconfig index ef3a8da..79905fe 100644 --- a/arch/arm/mach-tegra/Kconfig +++ b/arch/arm/mach-tegra/Kconfig @@ -15,6 +15,7 @@ config ARCH_TEGRA select SOC_BUS select SPARSE_IRQ select USE_OF + select OF_DYNAMIC help This enables support for NVIDIA Tegra based systems. diff --git a/arch/arm/mach-tegra/tegra.c b/arch/arm/mach-tegra/tegra.c index 0d1e412..0c494b5 100644 --- a/arch/arm/mach-tegra/tegra.c +++ b/arch/arm/mach-tegra/tegra.c @@ -80,6 +80,28 @@ static struct of_dev_auxdata tegra20_auxdata_lookup[] __initdata = { {} }; +static void tegra_of_platform_populate_iommu(void) +{ + int i; + struct platform_device *pdev; + const char * const dname[] = {ahb, iommu, }; + + for (i = 0; i ARRAY_SIZE(dname); i++) { + struct device_node *np; + char path[NAME_MAX]; + + snprintf(path, sizeof(path), /%s, dname[i]); + np = of_find_node_by_path(path); + if (!np) + break; + + pdev = of_platform_device_create(np, NULL, NULL); + of_node_put(np); + if (!pdev) + break; + } +} + static void __init tegra_dt_init(void) { struct soc_device_attribute *soc_dev_attr; @@ -107,6 +129,8 @@ static void __init tegra_dt_init(void) parent = soc_device_to_device(soc_dev); + tegra_of_platform_populate_iommu(); + /* * Finished with the static registrations now; fill in the missing * devices diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c index f6f120e..6c9de3f 100644 --- a/drivers/iommu/tegra-smmu.c +++ b/drivers/iommu/tegra-smmu.c @@ -1238,6 +1238,9 @@ static int tegra_smmu_probe(struct platform_device *pdev) smmu_debugfs_create(smmu); smmu_handle = smmu; bus_set_iommu(platform_bus_type, smmu_iommu_ops); + + of_detach_node(dev-of_node); + of_detach_node(smmu-ahb); return 0; } -- 1.8.1.5 ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [RFC] early init and DT platform devices allocation/registration
Hi Grant, thanks for commenting. On Tue, Jun 25, 2013 at 12:45:25PM +0100, Grant Likely wrote: On Mon, Jun 24, 2013 at 5:33 PM, Lorenzo Pieralisi lorenzo.pieral...@arm.com wrote: Hi all, I am dealing with a lingering problem related to init and probing of platform devices early (before initcalls) in the kernel boot process. The problem, which is nothing new, is related to how platform devices are created in the kernel from DT and when they become available. Platform devices are created through (assuming any legacy static initialization is removed from the kernel) of_platform_populate() at arch_initcall time on ARM. This is a problem for drivers that need to be probed and initialized before arch_initcall (ie early_initcall) because the corresponding platform_device has not been created yet. To work around this awkward situation, a driver, instead of relying on platform driver probing mechanism, can get initialized using early_initcall mechanism and rely on DT helpers (ie of_iomap() and company) to initialize resources. The problem comes when the driver functions must rely on an API (eg regmap) that requires a struct device to function properly; since the platform_device has not been created yet at early_initcall time, the driver cannot rely on it. The only solution consists in allocating a platform_device on the fly at driver init through of_platform_device_create() and use that device to initialize the (eg regmap) API. There is an issue with this solution, basically a platform device is allocated twice for a given device node - compatible property (because of_platform_populate() allocates a platform device for a node containing a compatible string even if a platform device has already been allocated for that device node). The second solution relies on early platform devices. Early platform devices are added by mach code and driver probing is carried out using the early platform device probing mechanism early_platform_driver_probe() The drawback with this solution is, AFAIK, it does not play well with DT, since the platform device duplication issue is still there (ie of_platform_populate() will allocate a platform device which is a duplicate of the one allocated at early boot to make the early platform device initialization possible). Please correct me if I am wrong, just trying to untangle a problem that does not look easy to solve. How this problem is supposed to be solved in the kernel ? 1- drivers that are to be up and running at early_initcall time must not rely on the device/driver model (but then they cannot use any API that requires a struct device to function (eg regmap)) 2- the driver should allocate a platform device at early initcall from a DT compatible node. Do not know how to deal with platform device duplication though, since of_platform_populate() will create another platform device when the node is parsed While I've resisted it in the past, I would be okay with adding struct device pointer in the device_node structure. I've resisted because I don't want drivers following the device_node pointer and making an assumption about what /kind/ of device is pointed to by it. However, this is an important use case and it makes it feasible to use an early platform device with of_platform_populate. Alternately, if others chime in and think it is too risky to have the device pointer in the device node, we could simply add a flag to the device_node which indicates the node has been parsed by of_platform_populate. There would need to be some careful coding to make sure that any call to early platform device creation sets the pointer in the device node correctly. I would also make it a requirement that any early platform device *must* be converted into a 'real' platform device during initcall time. That includes being possible to be freed correctly. Static early platform device definitions should not be allowed, otherwise there needs to be a special case for the platform device release hook. I really don't want that. Something else that needs to be investigated is how the device hierarchy will be affected by using early_platform_device. We still want the platform_device to appear in the same place in the hierarchy regardless of whether or not it was created 'early'. the question is how to make sure that actually happens. While acknowledging the complexity of adding this code, I tend to prefer a solution that allows allocation of early platform devices from DT instead of adding a flag to avoid duplicates, it is cleaner (but more complex as you mentioned) that's the point I wanted to make, otherwise we end up adding all kinds of kludges to the kernel to make early init work. The question is: should we live with workarounds that allow early allocation of devices or should we fix the kernel to cope with these use cases ? I do not know
Re: [RFC] early init and DT platform devices allocation/registration
On Tue, Jun 25, 2013 at 03:56:22PM +0100, Stephen Warren wrote: On 06/25/2013 05:45 AM, Grant Likely wrote: On Mon, Jun 24, 2013 at 5:33 PM, Lorenzo Pieralisi lorenzo.pieral...@arm.com wrote: Hi all, I am dealing with a lingering problem related to init and probing of platform devices early (before initcalls) in the kernel boot process. The problem, which is nothing new, is related to how platform devices are created in the kernel from DT and when they become available. Platform devices are created through (assuming any legacy static initialization is removed from the kernel) of_platform_populate() at arch_initcall time on ARM. This is a problem for drivers that need to be probed and initialized before arch_initcall (ie early_initcall) because the corresponding platform_device has not been created yet. What's the use-case here; why does the driver /have/ to be allocated/probed so early? Can't drivers all be allocated from DT in the usual fashion, and any dependencies resolved using deferred probe? In almost all cases, that should work. The driver must be initialized in order to boot secondary CPUs, so very very early, its initialization cannot be deferred. Otherwise we would end up with MCPM back-end having to add ad-hoc kludges to boot secondary CPUs, and then replace the early function calls to the power controller drivers when the power controller has been properly initialized. Feasible, but really horrible. On top of that, the HW component is an IP that provides functionality beyond power management, so we really want it to be initialized and running after boot, we cannot just hide a couple of function calls in the MCPM back-end to boot the CPUs and forget about that IP afterwards. ... How this problem is supposed to be solved in the kernel ? 1- drivers that are to be up and running at early_initcall time must not rely on the device/driver model (but then they cannot use any API that requires a struct device to function (eg regmap)) 2- the driver should allocate a platform device at early initcall from a DT compatible node. Do not know how to deal with platform device duplication though, since of_platform_populate() will create another platform device when the node is parsed While I've resisted it in the past, I would be okay with adding struct device pointer in the device_node structure. I've resisted because I don't want drivers following the device_node pointer and making an assumption about what /kind/ of device is pointed to by it. However, this is an important use case and it makes it feasible to use an early platform device with of_platform_populate. Hiroshi (who I have CC'd here) has also been asking about this same issue downstream. The issue for us is that we need to initialize an SMMU driver before any devices that are translated by the SMMU. One option is to force the register/probe of the SMMU driver explicitly, early in the machine's .init_machine() callback, before of_platform_populate(), to ensure the ordering. Then, we run into the same duplicate device issue, and the change Grant mentions above would help solve this. Good to hear we are not alone. I still do not know if we are dealing with specific use cases or this is a problem that should be solved generically at kernel level. I think it is best to solve it once for all, otherwise we will all come up with workarounds to make things work, possibly violating the driver/device model usage guidelines. Lorenzo ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [RFC] early init and DT platform devices allocation/registration
On Tue, Jun 25, 2013 at 5:36 PM, Hiroshi Doyu hd...@nvidia.com wrote: Stephen Warren swar...@wwwdotorg.org wrote @ Tue, 25 Jun 2013 16:56:22 +0200: How this problem is supposed to be solved in the kernel ? 1- drivers that are to be up and running at early_initcall time must not rely on the device/driver model (but then they cannot use any API that requires a struct device to function (eg regmap)) 2- the driver should allocate a platform device at early initcall from a DT compatible node. Do not know how to deal with platform device duplication though, since of_platform_populate() will create another platform device when the node is parsed While I've resisted it in the past, I would be okay with adding struct device pointer in the device_node structure. I've resisted because I don't want drivers following the device_node pointer and making an assumption about what /kind/ of device is pointed to by it. However, this is an important use case and it makes it feasible to use an early platform device with of_platform_populate. Hiroshi (who I have CC'd here) has also been asking about this same issue downstream. The issue for us is that we need to initialize an SMMU driver before any devices that are translated by the SMMU. One option is to force the register/probe of the SMMU driver explicitly, early in the machine's .init_machine() callback, before of_platform_populate(), to ensure the ordering. Then, we run into the same duplicate device issue, and the change Grant mentions above would help solve this. Here's my workaround. I need to call of_detach_node() with OF_DYNAMIC to avoid duplicated device registration. Gah! my eyes! Don't do that. It is incredibly problematic. Look at inhibiting duplicate device creation instead. g. From f4d88b8521c278b41b72028d326c03cfd2e90af8 Mon Sep 17 00:00:00 2001 From: Hiroshi Doyu hd...@nvidia.com Date: Fri, 14 Jun 2013 15:22:02 +0300 Subject: [PATCH 1/1] ARM: tegra: Populate AHB/IOMMU earlier than others Populate AHB/IOMMU earlier than others. IOMMU depends on AHB. IOMMU needs to be instanciated earlier than others so that IOMMU can register other platform devices as IOMMU'able. Once IOMMU is populated, IOMMU/AHB nodes are detached to prevent another registeration. Signed-off-by: Hiroshi Doyu hd...@nvidia.com --- arch/arm/mach-tegra/Kconfig | 1 + arch/arm/mach-tegra/tegra.c | 24 drivers/iommu/tegra-smmu.c | 3 +++ 3 files changed, 28 insertions(+) diff --git a/arch/arm/mach-tegra/Kconfig b/arch/arm/mach-tegra/Kconfig index ef3a8da..79905fe 100644 --- a/arch/arm/mach-tegra/Kconfig +++ b/arch/arm/mach-tegra/Kconfig @@ -15,6 +15,7 @@ config ARCH_TEGRA select SOC_BUS select SPARSE_IRQ select USE_OF + select OF_DYNAMIC help This enables support for NVIDIA Tegra based systems. diff --git a/arch/arm/mach-tegra/tegra.c b/arch/arm/mach-tegra/tegra.c index 0d1e412..0c494b5 100644 --- a/arch/arm/mach-tegra/tegra.c +++ b/arch/arm/mach-tegra/tegra.c @@ -80,6 +80,28 @@ static struct of_dev_auxdata tegra20_auxdata_lookup[] __initdata = { {} }; +static void tegra_of_platform_populate_iommu(void) +{ + int i; + struct platform_device *pdev; + const char * const dname[] = {ahb, iommu, }; + + for (i = 0; i ARRAY_SIZE(dname); i++) { + struct device_node *np; + char path[NAME_MAX]; + + snprintf(path, sizeof(path), /%s, dname[i]); + np = of_find_node_by_path(path); + if (!np) + break; + + pdev = of_platform_device_create(np, NULL, NULL); + of_node_put(np); + if (!pdev) + break; + } +} + static void __init tegra_dt_init(void) { struct soc_device_attribute *soc_dev_attr; @@ -107,6 +129,8 @@ static void __init tegra_dt_init(void) parent = soc_device_to_device(soc_dev); + tegra_of_platform_populate_iommu(); + /* * Finished with the static registrations now; fill in the missing * devices diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c index f6f120e..6c9de3f 100644 --- a/drivers/iommu/tegra-smmu.c +++ b/drivers/iommu/tegra-smmu.c @@ -1238,6 +1238,9 @@ static int tegra_smmu_probe(struct platform_device *pdev) smmu_debugfs_create(smmu); smmu_handle = smmu; bus_set_iommu(platform_bus_type, smmu_iommu_ops); + + of_detach_node(dev-of_node); + of_detach_node(smmu-ahb); return 0; } -- 1.8.1.5 ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
[RFC] early init and DT platform devices allocation/registration
Hi all, I am dealing with a lingering problem related to init and probing of platform devices early (before initcalls) in the kernel boot process. The problem, which is nothing new, is related to how platform devices are created in the kernel from DT and when they become available. Platform devices are created through (assuming any legacy static initialization is removed from the kernel) of_platform_populate() at arch_initcall time on ARM. This is a problem for drivers that need to be probed and initialized before arch_initcall (ie early_initcall) because the corresponding platform_device has not been created yet. To work around this awkward situation, a driver, instead of relying on platform driver probing mechanism, can get initialized using early_initcall mechanism and rely on DT helpers (ie of_iomap() and company) to initialize resources. The problem comes when the driver functions must rely on an API (eg regmap) that requires a struct device to function properly; since the platform_device has not been created yet at early_initcall time, the driver cannot rely on it. The only solution consists in allocating a platform_device on the fly at driver init through of_platform_device_create() and use that device to initialize the (eg regmap) API. There is an issue with this solution, basically a platform device is allocated twice for a given device node - compatible property (because of_platform_populate() allocates a platform device for a node containing a compatible string even if a platform device has already been allocated for that device node). The second solution relies on early platform devices. Early platform devices are added by mach code and driver probing is carried out using the early platform device probing mechanism early_platform_driver_probe() The drawback with this solution is, AFAIK, it does not play well with DT, since the platform device duplication issue is still there (ie of_platform_populate() will allocate a platform device which is a duplicate of the one allocated at early boot to make the early platform device initialization possible). Please correct me if I am wrong, just trying to untangle a problem that does not look easy to solve. How this problem is supposed to be solved in the kernel ? 1- drivers that are to be up and running at early_initcall time must not rely on the device/driver model (but then they cannot use any API that requires a struct device to function (eg regmap)) 2- the driver should allocate a platform device at early initcall from a DT compatible node. Do not know how to deal with platform device duplication though, since of_platform_populate() will create another platform device when the node is parsed 3- driver should rely on early platform device/driver, but this does not solve (?) the platform device duplication problem either, it will happen when of_platform_populate() parses the DT and creates devices on the fly In theory there are other solutions such as: (a) declaring the platform device statically in arm/mach-* code and do not define the device node in dts, basically going back in time to ARM legacy kernel mechanism for this kind of devices (b) add a way to of_platform_populate() to exclude some compatible strings from being matched Honestly there is not a solution I can say I like and maybe I am trying to solve a problem that has already been solved, apologies if so, that's why I am asking on the list to people with more knowledge than me on the subject. Comments are really really welcome, thank you ! Lorenzo ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss