On Mon, May 14, 2018 at 6:38 AM, Bartosz Golaszewski <brgl@xxxxxxxx> wrote: > 2018-05-11 22:13 GMT+02:00 Rob Herring <robh+dt@xxxxxxxxxx>: >> On Fri, May 11, 2018 at 11:20 AM, Bartosz Golaszewski <brgl@xxxxxxxx> wrote: >>> This series is a follow-up to the RFC[1] posted a couple days ago. >>> >>> NOTE: this series applies on top of my recent patches[2] that move the previous >>> implementation of early platform devices to arch/sh. >>> >>> Problem: >>> >>> Certain class of devices, such as timers, certain clock drivers and irq chip >>> drivers need to be probed early in the boot sequence. The currently preferred >>> approach is using one of the OF_DECLARE() macros. This however does not create >>> a platform device which has many drawbacks - such as not being able to use >>> devres routines, dev_ log functions or no way of deferring the init OF function >>> if some other resources are missing. >> >> I skimmed though this and it doesn't look horrible (how's that for >> positive feedback? ;) ). But before going into the details, I think >> first there needs to be agreement this is the right direction. >> >> The question does remain though as to whether this class of devices >> should be platform drivers. They can't be modules. They can't be >> hotplugged. Can they be runtime-pm enabled? So the advantage is ... >> > > The main (but not the only) advantage for drivers that can both be > platform drivers and OF_DECLARE drivers is that we get a single entry > point and can reuse code without resorting to checking if (!dev). It > results in more consistent code base. Another big advantage is > consolidation of device tree and machine code for SoC drivers used in > different boards of which some are still using board files and others > are defined in DT (see: DaVinci). > >> I assume that the clock maintainers had some reason to move clocks to >> be platform drivers. It's just not clear to me what that was. >> >>> For drivers that use both platform drivers and OF_DECLARE the situation is even >>> more complicated as the code needs to take into account that there can possibly >>> be no struct device present. For a specific use case that we're having problems >>> with, please refer to the recent DaVinci common-clock conversion patches and >>> the nasty workaround that this problem implies[3]. >> >> So devm_kzalloc will work with this solution? Why did we need >> devm_kzalloc in the first place? The clocks can never be removed and >> cleaning up on error paths is kind of pointless. The system would be >> hosed, right? >> > > It depends - not all clocks are necessary for system to boot. That doesn't matter. You have a single driver for all/most of the clocks, so the driver can't be removed. >>> We also used to have an early platform drivers implementation but they were not >>> integrated with the linux device model at all - they merely used the same data >>> structures. The users could not use devres, defer probe and the early devices >>> never became actual platform devices later on. >>> >>> Proposed solution: >>> >>> This series aims at solving this problem by (re-)introducing the concept of >>> early platform drivers and devices - this time however in a way that seamlessly >>> integrates with the existing platform drivers and also offers device-tree >>> support. >>> >>> The idea is to provide a way for users to probe devices early, while already >>> being able to use devres, devices resources and properties and also deferred >>> probing. >>> >>> New structures are introduced: the early platform driver contains the >>> early_probe callback which has the same signature as regular platform_device >>> probe. This callback is called early on. The user can have both the early and >>> regular probe speficied or only one of them and they both receive the same >>> platform device object as argument. Any device data allocated early will be >>> carried over to the normal probe. >>> >>> The architecture code is responsible for calling early_platform_start() in >>> which the early drivers will be registered and devices populated from DT. >> >> Can we really do this in one spot for different devices (clk, timers, >> irq). The sequence is all very carefully crafted. Platform specific >> hooks is another thing to consider. >> > > This is why I added support for early probe deferral - so that we can > stop caring for the order as long as the drivers are aware of other > resources they need and we call early_platform_start() the moment the > earliest of the early devices is needed. Deferred probe helps for inter-device dependencies, but I am more concerned about timing of trying to register clocksources, irqchips, etc. What happens if we probe drivers before the infrastructure is initialized? Rob