* [RFC PATCH] driver core: make deferring probe forever optional @ 2018-05-01 21:31 Rob Herring 2018-05-01 22:08 ` Greg Kroah-Hartman ` (3 more replies) 0 siblings, 4 replies; 20+ messages in thread From: Rob Herring @ 2018-05-01 21:31 UTC (permalink / raw) To: linux-kernel-u79uwXL29TY76Z2rM5mHXA, devicetree-u79uwXL29TY76Z2rM5mHXA Cc: boot-architecture-cunTk1MwBs8s++Sfvej+rw, Stephen Boyd, Greg Kroah-Hartman, Linus Walleij, Alexander Graf, Grant Likely, Mark Brown, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r Deferred probe will currently wait forever on dependent devices to probe, but sometimes a driver will never exist. It's also not always critical for a driver to exist. Platforms can rely on default configuration from the bootloader or reset defaults for things such as pinctrl and power domains. This is often the case with initial platform support until various drivers get enabled. There's at least 2 scenarios where deferred probe can render a platform broken. Both involve using a DT which has more devices and dependencies than the kernel supports. The 1st case is a driver may be disabled in the kernel config. The 2nd case is the kernel version may simply not have the dependent driver. This can happen if using a newer DT (provided by firmware perhaps) with a stable kernel version. Unfortunately, this change breaks with modules as we have no way of knowing when modules are done loading. One possibility is to make this opt in or out based on compatible strings rather than at a subsystem level. Ideally this information could be extracted automatically somehow. OTOH, maybe the lists are pretty small. There's only a handful of subsystems that can be optional, and then only so many drivers in those that can be modules (at least for pinctrl, many drivers are built-in only). Cc: Alexander Graf <agraf@suse.de> Signed-off-by: Rob Herring <robh@kernel.org> --- This patch came out of a discussion on the ARM boot-architecture list[1] about DT forwards and backwards compatibility issues. There are issues with newer DTs breaking on older, stable kernels. Some of these are difficult to solve, but cases of optional devices not having kernel support should be solvable. I tested this on a RPi3 B with the pinctrl driver forced off. With this change, the MMC/SD and UART drivers can function without the pinctrl driver. Rob [1] https://lists.linaro.org/pipermail/boot-architecture/2018-April/000466.html drivers/base/dd.c | 16 ++++++++++++++++ drivers/pinctrl/devicetree.c | 2 +- include/linux/device.h | 2 ++ 3 files changed, 19 insertions(+), 1 deletion(-) diff --git a/drivers/base/dd.c b/drivers/base/dd.c index c9f54089429b..5848808b9d7a 100644 --- a/drivers/base/dd.c +++ b/drivers/base/dd.c @@ -226,6 +226,15 @@ void device_unblock_probing(void) driver_deferred_probe_trigger(); } + +int driver_deferred_probe_optional(void) +{ + if (initcalls_done) + return -ENODEV; + + return -EPROBE_DEFER; +} + /** * deferred_probe_initcall() - Enable probing of deferred devices * @@ -240,6 +249,13 @@ static int deferred_probe_initcall(void) /* Sort as many dependencies as possible before exiting initcalls */ flush_work(&deferred_probe_work); initcalls_done = true; + + /* + * Trigger deferred probe again, this time we won't defer anything + * that is optional + */ + driver_deferred_probe_trigger(); + flush_work(&deferred_probe_work); return 0; } late_initcall(deferred_probe_initcall); diff --git a/drivers/pinctrl/devicetree.c b/drivers/pinctrl/devicetree.c index b601039d6c69..096e52a5c506 100644 --- a/drivers/pinctrl/devicetree.c +++ b/drivers/pinctrl/devicetree.c @@ -120,7 +120,7 @@ static int dt_to_map_one_config(struct pinctrl *p, np_config); of_node_put(np_pctldev); /* OK let's just assume this will appear later then */ - return -EPROBE_DEFER; + return driver_deferred_probe_optional(); } /* If we're creating a hog we can use the passed pctldev */ if (pctldev && (np_pctldev == p->dev->of_node)) diff --git a/include/linux/device.h b/include/linux/device.h index 0059b99e1f25..8de920442bc1 100644 --- a/include/linux/device.h +++ b/include/linux/device.h @@ -332,6 +332,8 @@ struct device *driver_find_device(struct device_driver *drv, struct device *start, void *data, int (*match)(struct device *dev, void *data)); +int driver_deferred_probe_optional(void); + /** * struct subsys_interface - interfaces to device functions * @name: name of the device function -- 2.17.0 _______________________________________________ boot-architecture mailing list boot-architecture@lists.linaro.org https://lists.linaro.org/mailman/listinfo/boot-architecture ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [RFC PATCH] driver core: make deferring probe forever optional 2018-05-01 21:31 [RFC PATCH] driver core: make deferring probe forever optional Rob Herring @ 2018-05-01 22:08 ` Greg Kroah-Hartman 2018-05-02 11:40 ` Robin Murphy ` (2 subsequent siblings) 3 siblings, 0 replies; 20+ messages in thread From: Greg Kroah-Hartman @ 2018-05-01 22:08 UTC (permalink / raw) To: Rob Herring Cc: linux-kernel, devicetree, Grant Likely, Linus Walleij, Mark Brown, Stephen Boyd, boot-architecture, linux-arm-kernel, Alexander Graf On Tue, May 01, 2018 at 04:31:14PM -0500, Rob Herring wrote: > Deferred probe will currently wait forever on dependent devices to probe, > but sometimes a driver will never exist. It's also not always critical for > a driver to exist. Platforms can rely on default configuration from the > bootloader or reset defaults for things such as pinctrl and power domains. > This is often the case with initial platform support until various drivers > get enabled. There's at least 2 scenarios where deferred probe can render > a platform broken. Both involve using a DT which has more devices and > dependencies than the kernel supports. The 1st case is a driver may be > disabled in the kernel config. The 2nd case is the kernel version may > simply not have the dependent driver. This can happen if using a newer DT > (provided by firmware perhaps) with a stable kernel version. > > Unfortunately, this change breaks with modules as we have no way of > knowing when modules are done loading. One possibility is to make this > opt in or out based on compatible strings rather than at a subsystem level. > Ideally this information could be extracted automatically somehow. OTOH, > maybe the lists are pretty small. There's only a handful of subsystems > that can be optional, and then only so many drivers in those that can be > modules (at least for pinctrl, many drivers are built-in only). > > Cc: Alexander Graf <agraf@suse.de> > Signed-off-by: Rob Herring <robh@kernel.org> > --- > This patch came out of a discussion on the ARM boot-architecture > list[1] about DT forwards and backwards compatibility issues. There are > issues with newer DTs breaking on older, stable kernels. Some of these > are difficult to solve, but cases of optional devices not having > kernel support should be solvable. > > I tested this on a RPi3 B with the pinctrl driver forced off. With this > change, the MMC/SD and UART drivers can function without the pinctrl > driver. > > Rob > > [1] https://lists.linaro.org/pipermail/boot-architecture/2018-April/000466.html > > drivers/base/dd.c | 16 ++++++++++++++++ > drivers/pinctrl/devicetree.c | 2 +- > include/linux/device.h | 2 ++ > 3 files changed, 19 insertions(+), 1 deletion(-) > > diff --git a/drivers/base/dd.c b/drivers/base/dd.c > index c9f54089429b..5848808b9d7a 100644 > --- a/drivers/base/dd.c > +++ b/drivers/base/dd.c > @@ -226,6 +226,15 @@ void device_unblock_probing(void) > driver_deferred_probe_trigger(); > } > > + > +int driver_deferred_probe_optional(void) > +{ > + if (initcalls_done) > + return -ENODEV; > + > + return -EPROBE_DEFER; > +} The name is ackward for this function, but I can't think of anything better at the moment, sorry. However, the overall idea for this is sane, no objection from me at all for this change. thanks, greg k-h ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [RFC PATCH] driver core: make deferring probe forever optional 2018-05-01 21:31 [RFC PATCH] driver core: make deferring probe forever optional Rob Herring 2018-05-01 22:08 ` Greg Kroah-Hartman @ 2018-05-02 11:40 ` Robin Murphy 2018-05-02 14:48 ` Rob Herring 2018-05-02 13:16 ` Alexander Graf 2018-05-07 18:31 ` Bjorn Andersson 3 siblings, 1 reply; 20+ messages in thread From: Robin Murphy @ 2018-05-02 11:40 UTC (permalink / raw) To: Rob Herring, linux-kernel, devicetree Cc: boot-architecture, Stephen Boyd, Greg Kroah-Hartman, Linus Walleij, Alexander Graf, Grant Likely, Mark Brown, linux-arm-kernel On 01/05/18 22:31, Rob Herring wrote: > Deferred probe will currently wait forever on dependent devices to probe, > but sometimes a driver will never exist. It's also not always critical for > a driver to exist. Platforms can rely on default configuration from the > bootloader or reset defaults for things such as pinctrl and power domains. > This is often the case with initial platform support until various drivers > get enabled. There's at least 2 scenarios where deferred probe can render > a platform broken. Both involve using a DT which has more devices and > dependencies than the kernel supports. The 1st case is a driver may be > disabled in the kernel config. The 2nd case is the kernel version may > simply not have the dependent driver. This can happen if using a newer DT > (provided by firmware perhaps) with a stable kernel version. > > Unfortunately, this change breaks with modules as we have no way of > knowing when modules are done loading. One possibility is to make this > opt in or out based on compatible strings rather than at a subsystem level. > Ideally this information could be extracted automatically somehow. OTOH, > maybe the lists are pretty small. There's only a handful of subsystems > that can be optional, and then only so many drivers in those that can be > modules (at least for pinctrl, many drivers are built-in only). Ooh, this is exactly what I wanted for of_iommu_xlate(), and would be much nicer than the current bodge using system_state that I ended up with. The context there is very similar - if the device has a parent IOMMU then we want to wait for that to probe first if possible, but with a deadline such that if it doesn't show up then we can go ahead and make progress without it (on the assumption that DMA ops can fall back to CMA). The modules problem doesn't currently apply to IOMMU drivers either, although we do use a special of_device_id table for detecting built-in drivers via OF_IOMMU_DECLARE() to avoid deferring at all when we know it would be pointless - a more generic solution for that could certainly be useful too. I agree with Greg that the naming here is a bit tricky - FWIW my initial thought would be something like driver_defer_until_init(), which is still clunky but at least imperative. Robin. > Cc: Alexander Graf <agraf@suse.de> > Signed-off-by: Rob Herring <robh@kernel.org> > --- > This patch came out of a discussion on the ARM boot-architecture > list[1] about DT forwards and backwards compatibility issues. There are > issues with newer DTs breaking on older, stable kernels. Some of these > are difficult to solve, but cases of optional devices not having > kernel support should be solvable. > > I tested this on a RPi3 B with the pinctrl driver forced off. With this > change, the MMC/SD and UART drivers can function without the pinctrl > driver. > > Rob > > [1] https://lists.linaro.org/pipermail/boot-architecture/2018-April/000466.html > > drivers/base/dd.c | 16 ++++++++++++++++ > drivers/pinctrl/devicetree.c | 2 +- > include/linux/device.h | 2 ++ > 3 files changed, 19 insertions(+), 1 deletion(-) > > diff --git a/drivers/base/dd.c b/drivers/base/dd.c > index c9f54089429b..5848808b9d7a 100644 > --- a/drivers/base/dd.c > +++ b/drivers/base/dd.c > @@ -226,6 +226,15 @@ void device_unblock_probing(void) > driver_deferred_probe_trigger(); > } > > + > +int driver_deferred_probe_optional(void) > +{ > + if (initcalls_done) > + return -ENODEV; > + > + return -EPROBE_DEFER; > +} > + > /** > * deferred_probe_initcall() - Enable probing of deferred devices > * > @@ -240,6 +249,13 @@ static int deferred_probe_initcall(void) > /* Sort as many dependencies as possible before exiting initcalls */ > flush_work(&deferred_probe_work); > initcalls_done = true; > + > + /* > + * Trigger deferred probe again, this time we won't defer anything > + * that is optional > + */ > + driver_deferred_probe_trigger(); > + flush_work(&deferred_probe_work); > return 0; > } > late_initcall(deferred_probe_initcall); > diff --git a/drivers/pinctrl/devicetree.c b/drivers/pinctrl/devicetree.c > index b601039d6c69..096e52a5c506 100644 > --- a/drivers/pinctrl/devicetree.c > +++ b/drivers/pinctrl/devicetree.c > @@ -120,7 +120,7 @@ static int dt_to_map_one_config(struct pinctrl *p, > np_config); > of_node_put(np_pctldev); > /* OK let's just assume this will appear later then */ > - return -EPROBE_DEFER; > + return driver_deferred_probe_optional(); > } > /* If we're creating a hog we can use the passed pctldev */ > if (pctldev && (np_pctldev == p->dev->of_node)) > diff --git a/include/linux/device.h b/include/linux/device.h > index 0059b99e1f25..8de920442bc1 100644 > --- a/include/linux/device.h > +++ b/include/linux/device.h > @@ -332,6 +332,8 @@ struct device *driver_find_device(struct device_driver *drv, > struct device *start, void *data, > int (*match)(struct device *dev, void *data)); > > +int driver_deferred_probe_optional(void); > + > /** > * struct subsys_interface - interfaces to device functions > * @name: name of the device function > ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [RFC PATCH] driver core: make deferring probe forever optional 2018-05-02 11:40 ` Robin Murphy @ 2018-05-02 14:48 ` Rob Herring 2018-05-02 18:49 ` Robin Murphy 0 siblings, 1 reply; 20+ messages in thread From: Rob Herring @ 2018-05-02 14:48 UTC (permalink / raw) To: Robin Murphy Cc: linux-kernel, devicetree, boot-architecture, Stephen Boyd, Greg Kroah-Hartman, Linus Walleij, Alexander Graf, Grant Likely, Mark Brown, moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE On Wed, May 2, 2018 at 6:40 AM, Robin Murphy <robin.murphy@arm.com> wrote: > On 01/05/18 22:31, Rob Herring wrote: >> >> Deferred probe will currently wait forever on dependent devices to probe, >> but sometimes a driver will never exist. It's also not always critical for >> a driver to exist. Platforms can rely on default configuration from the >> bootloader or reset defaults for things such as pinctrl and power domains. >> This is often the case with initial platform support until various drivers >> get enabled. There's at least 2 scenarios where deferred probe can render >> a platform broken. Both involve using a DT which has more devices and >> dependencies than the kernel supports. The 1st case is a driver may be >> disabled in the kernel config. The 2nd case is the kernel version may >> simply not have the dependent driver. This can happen if using a newer DT >> (provided by firmware perhaps) with a stable kernel version. >> >> Unfortunately, this change breaks with modules as we have no way of >> knowing when modules are done loading. One possibility is to make this >> opt in or out based on compatible strings rather than at a subsystem >> level. >> Ideally this information could be extracted automatically somehow. OTOH, >> maybe the lists are pretty small. There's only a handful of subsystems >> that can be optional, and then only so many drivers in those that can be >> modules (at least for pinctrl, many drivers are built-in only). > > > Ooh, this is exactly what I wanted for of_iommu_xlate(), and would be much > nicer than the current bodge using system_state that I ended up with. The > context there is very similar - if the device has a parent IOMMU then we > want to wait for that to probe first if possible, but with a deadline such > that if it doesn't show up then we can go ahead and make progress without it > (on the assumption that DMA ops can fall back to CMA). The modules problem > doesn't currently apply to IOMMU drivers either, although we do use a > special of_device_id table for detecting built-in drivers via > OF_IOMMU_DECLARE() to avoid deferring at all when we know it would be > pointless - a more generic solution for that could certainly be useful too. Ah, so you kept the IOMMU_OF_DECLARE() but it does nothing but define a table. We already have the driver match table which should pretty much be the same data, so it would be better if we could use that if possible. If we used MODULE_DEVICE_TABLE somehow, we could avoid modifying lots of drivers. Though many built-in only drivers omit that. The other problem is it would become a large set of tables to search thru because it is global. That would probably end up slower than just deferring. So we need something like <subsystem>_DEVICE_TABLE() to have per subsystem tables. Then this function in this patch would need to be told which table to use. However, this is all really just an optimization to avoid deferring at all and could be addressed later. Is there any data on how much time you save avoiding deferring? This has come up in the past and I don't think it is much. I've also been thinking about if we could use MODULE_DEVICE_TABLE to provide a list compatible strings from modules as a whitelist of devices to keep deferring probe on. That would require building modules to build the kernel which I don't think would work. I think my conclusion is that the cases we care about may be short enough to just manually maintain such a list. > > I agree with Greg that the naming here is a bit tricky - FWIW my initial > thought would be something like driver_defer_until_init(), which is still > clunky but at least imperative. Yeah, I didn't give it much thought as I expected more comments on the general idea... Maybe my poor naming is a good diversion. :) Rob ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [RFC PATCH] driver core: make deferring probe forever optional 2018-05-02 14:48 ` Rob Herring @ 2018-05-02 18:49 ` Robin Murphy 2018-05-05 1:25 ` Mark Brown 0 siblings, 1 reply; 20+ messages in thread From: Robin Murphy @ 2018-05-02 18:49 UTC (permalink / raw) To: Rob Herring Cc: linux-kernel, devicetree, boot-architecture, Stephen Boyd, Greg Kroah-Hartman, Linus Walleij, Alexander Graf, Grant Likely, Mark Brown, moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE On 02/05/18 15:48, Rob Herring wrote: > On Wed, May 2, 2018 at 6:40 AM, Robin Murphy <robin.murphy@arm.com> wrote: >> On 01/05/18 22:31, Rob Herring wrote: >>> >>> Deferred probe will currently wait forever on dependent devices to probe, >>> but sometimes a driver will never exist. It's also not always critical for >>> a driver to exist. Platforms can rely on default configuration from the >>> bootloader or reset defaults for things such as pinctrl and power domains. >>> This is often the case with initial platform support until various drivers >>> get enabled. There's at least 2 scenarios where deferred probe can render >>> a platform broken. Both involve using a DT which has more devices and >>> dependencies than the kernel supports. The 1st case is a driver may be >>> disabled in the kernel config. The 2nd case is the kernel version may >>> simply not have the dependent driver. This can happen if using a newer DT >>> (provided by firmware perhaps) with a stable kernel version. >>> >>> Unfortunately, this change breaks with modules as we have no way of >>> knowing when modules are done loading. One possibility is to make this >>> opt in or out based on compatible strings rather than at a subsystem >>> level. >>> Ideally this information could be extracted automatically somehow. OTOH, >>> maybe the lists are pretty small. There's only a handful of subsystems >>> that can be optional, and then only so many drivers in those that can be >>> modules (at least for pinctrl, many drivers are built-in only). >> >> >> Ooh, this is exactly what I wanted for of_iommu_xlate(), and would be much >> nicer than the current bodge using system_state that I ended up with. The >> context there is very similar - if the device has a parent IOMMU then we >> want to wait for that to probe first if possible, but with a deadline such >> that if it doesn't show up then we can go ahead and make progress without it >> (on the assumption that DMA ops can fall back to CMA). The modules problem >> doesn't currently apply to IOMMU drivers either, although we do use a >> special of_device_id table for detecting built-in drivers via >> OF_IOMMU_DECLARE() to avoid deferring at all when we know it would be >> pointless - a more generic solution for that could certainly be useful too. > > Ah, so you kept the IOMMU_OF_DECLARE() but it does nothing but define > a table. We already have the driver match table which should pretty > much be the same data, so it would be better if we could use that if > possible. If we used MODULE_DEVICE_TABLE somehow, we could avoid > modifying lots of drivers. Though many built-in only drivers omit > that. The other problem is it would become a large set of tables to > search thru because it is global. That would probably end up slower > than just deferring. So we need something like > <subsystem>_DEVICE_TABLE() to have per subsystem tables. Then this > function in this patch would need to be told which table to use. > However, this is all really just an optimization to avoid deferring at > all and could be addressed later. Is there any data on how much time > you save avoiding deferring? This has come up in the past and I don't > think it is much. In the of_iommu case it's not actually an optimisation, but dodges a big problem with the self-contained implementation - if we blindly defer on a not-yet-present IOMMU until all built-in drivers have had a chance to register themselves naturally, then by the time we could safely assume the relevant IOMMU driver *isn't* built-in, all the clients (which may include the boot device) can already be stuck on the deferred probe list with nothing left to kick it and make progress again. It's quite possible I could have done better, I just wasn't very familiar with the driver core at the time, and repurposing the magic table instead of entirely removing it was by far the easiest way forward. With this patch we would have a guarantee that the deferred list gets at least one kick after the deadline for waiting has passed, so in theory we could then just use the regular driver matching mechanisms to see whether a given IOMMU node can possibly probe or not. I'm not sure we could get rid of the driver introspection aspect entirely, since it might be the case that the IOMMU itself has dependencies and winds up on the deferred list behind one or more of its clients, in which case we'd still want them to keep waiting even after the deadline is nominally up. > I've also been thinking about if we could use MODULE_DEVICE_TABLE to > provide a list compatible strings from modules as a whitelist of > devices to keep deferring probe on. That would require building > modules to build the kernel which I don't think would work. I think my > conclusion is that the cases we care about may be short enough to just > manually maintain such a list. FWIW we did get rid of the equivalent table completely for the arm64 ACPI code, but that only has to support 2 types of IOMMU so just evaluates the respective driver config symbols directly with IS_BUILTIN(). Sadly that method really can't scale to DT with multiple compatible strings per driver... I guess there's also the possibility that a single driver may want multiple behaviours, if e.g. if SoC variants A and B have some identical peripherals but slightly different pinctrl/IOMMU/etc. hardware such that A has workable default behaviour and can be treated as optional, whereas B absolutely must be controlled by the kernel for the consumers to function properly, and they *should* defer forever otherwise. I think that would pretty much demand some sort of explicitly-curated white/blacklist setup at the subsystem or driver level. Robin. ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [RFC PATCH] driver core: make deferring probe forever optional 2018-05-02 18:49 ` Robin Murphy @ 2018-05-05 1:25 ` Mark Brown 2018-05-07 13:37 ` Rob Herring 0 siblings, 1 reply; 20+ messages in thread From: Mark Brown @ 2018-05-05 1:25 UTC (permalink / raw) To: Robin Murphy Cc: Rob Herring, linux-kernel, devicetree, boot-architecture, Stephen Boyd, Greg Kroah-Hartman, Linus Walleij, Alexander Graf, Grant Likely, moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE [-- Attachment #1: Type: text/plain, Size: 940 bytes --] On Wed, May 02, 2018 at 07:49:57PM +0100, Robin Murphy wrote: > I guess there's also the possibility that a single driver may want multiple > behaviours, if e.g. if SoC variants A and B have some identical peripherals > but slightly different pinctrl/IOMMU/etc. hardware such that A has workable > default behaviour and can be treated as optional, whereas B absolutely must > be controlled by the kernel for the consumers to function properly, and they > *should* defer forever otherwise. I think that would pretty much demand some > sort of explicitly-curated white/blacklist setup at the subsystem or driver > level. Different board variants, and possibly even different bootloaders might also be an issue here - a vendor bootloader might do pinmuxing that an upstream bootloader doesn't for example. In some cases the pinmuxing even depends on the boot method with things only getting configured if the bootloader wanted to use them. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [RFC PATCH] driver core: make deferring probe forever optional 2018-05-05 1:25 ` Mark Brown @ 2018-05-07 13:37 ` Rob Herring 0 siblings, 0 replies; 20+ messages in thread From: Rob Herring @ 2018-05-07 13:37 UTC (permalink / raw) To: Mark Brown Cc: Robin Murphy, linux-kernel, devicetree, boot-architecture, Stephen Boyd, Greg Kroah-Hartman, Linus Walleij, Alexander Graf, Grant Likely, moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE On Fri, May 4, 2018 at 8:25 PM, Mark Brown <broonie@kernel.org> wrote: > On Wed, May 02, 2018 at 07:49:57PM +0100, Robin Murphy wrote: > >> I guess there's also the possibility that a single driver may want multiple >> behaviours, if e.g. if SoC variants A and B have some identical peripherals >> but slightly different pinctrl/IOMMU/etc. hardware such that A has workable >> default behaviour and can be treated as optional, whereas B absolutely must >> be controlled by the kernel for the consumers to function properly, and they >> *should* defer forever otherwise. I think that would pretty much demand some >> sort of explicitly-curated white/blacklist setup at the subsystem or driver >> level. > > Different board variants, and possibly even different bootloaders might > also be an issue here - a vendor bootloader might do pinmuxing that an > upstream bootloader doesn't for example. In some cases the pinmuxing > even depends on the boot method with things only getting configured if > the bootloader wanted to use them. I think this is going to be too big of a hammer for pinctrl at least. My current thought is to define a pinctrl DT property to indicate pins are configured already which the OS can use to decide if pinctrl is optional or not. I'd prefer to keep it simple and be a per pin controller flag even though this is quite possibly a per client or pin group state (as you say, the bootloader may only configure what it uses). Making this per pin group could be a lot of nodes and difficult to really get right without testing. Making it per pin controller could make drivers fail in less predictable ways if their pins are not configured. Rob ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [RFC PATCH] driver core: make deferring probe forever optional 2018-05-01 21:31 [RFC PATCH] driver core: make deferring probe forever optional Rob Herring 2018-05-01 22:08 ` Greg Kroah-Hartman 2018-05-02 11:40 ` Robin Murphy @ 2018-05-02 13:16 ` Alexander Graf 2018-05-07 18:31 ` Bjorn Andersson 3 siblings, 0 replies; 20+ messages in thread From: Alexander Graf @ 2018-05-02 13:16 UTC (permalink / raw) To: Rob Herring, linux-kernel, devicetree Cc: Greg Kroah-Hartman, Grant Likely, Linus Walleij, Mark Brown, Stephen Boyd, boot-architecture, linux-arm-kernel On 05/01/2018 11:31 PM, Rob Herring wrote: > Deferred probe will currently wait forever on dependent devices to probe, > but sometimes a driver will never exist. It's also not always critical for > a driver to exist. Platforms can rely on default configuration from the > bootloader or reset defaults for things such as pinctrl and power domains. > This is often the case with initial platform support until various drivers > get enabled. There's at least 2 scenarios where deferred probe can render > a platform broken. Both involve using a DT which has more devices and > dependencies than the kernel supports. The 1st case is a driver may be > disabled in the kernel config. The 2nd case is the kernel version may > simply not have the dependent driver. This can happen if using a newer DT > (provided by firmware perhaps) with a stable kernel version. > > Unfortunately, this change breaks with modules as we have no way of > knowing when modules are done loading. One possibility is to make this > opt in or out based on compatible strings rather than at a subsystem level. > Ideally this information could be extracted automatically somehow. OTOH, > maybe the lists are pretty small. There's only a handful of subsystems > that can be optional, and then only so many drivers in those that can be > modules (at least for pinctrl, many drivers are built-in only). > > Cc: Alexander Graf <agraf@suse.de> > Signed-off-by: Rob Herring <robh@kernel.org> > --- > This patch came out of a discussion on the ARM boot-architecture > list[1] about DT forwards and backwards compatibility issues. There are > issues with newer DTs breaking on older, stable kernels. Some of these > are difficult to solve, but cases of optional devices not having > kernel support should be solvable. I think this is a reasonable approach. Maybe this should be a CONFIG option that disallows pinctrl drivers (and power domain later) to be =m? Then by default we could force those drivers to be compiled in, but if you really wanted to do kernel modules for pinctrl/pd you'd consciously potentially lose forward compatibility. Alex ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [RFC PATCH] driver core: make deferring probe forever optional 2018-05-01 21:31 [RFC PATCH] driver core: make deferring probe forever optional Rob Herring ` (2 preceding siblings ...) 2018-05-02 13:16 ` Alexander Graf @ 2018-05-07 18:31 ` Bjorn Andersson 2018-05-07 19:55 ` Rob Herring 2018-05-09 9:44 ` Alexander Graf 3 siblings, 2 replies; 20+ messages in thread From: Bjorn Andersson @ 2018-05-07 18:31 UTC (permalink / raw) To: Rob Herring Cc: linux-kernel, devicetree, Greg Kroah-Hartman, Grant Likely, Linus Walleij, Mark Brown, Stephen Boyd, boot-architecture, linux-arm-kernel, Alexander Graf On Tue 01 May 14:31 PDT 2018, Rob Herring wrote: > Deferred probe will currently wait forever on dependent devices to probe, > but sometimes a driver will never exist. It's also not always critical for > a driver to exist. Platforms can rely on default configuration from the > bootloader or reset defaults for things such as pinctrl and power domains. But how do you know if this is the case? > This is often the case with initial platform support until various drivers > get enabled. Can you please name platform that has enough support for Alexander to care about backwards and forwards compatibility but lacks a pinctrl driver. > There's at least 2 scenarios where deferred probe can render > a platform broken. Both involve using a DT which has more devices and > dependencies than the kernel supports. The 1st case is a driver may be > disabled in the kernel config. I agree that there is a chance that you _might_ get some parts of the system working by relying on the boot loader configuration, but misconfiguration issues applies to any other fundamental providers, such as clocks, regulators, power domains and gpios as well. > The 2nd case is the kernel version may > simply not have the dependent driver. This can happen if using a newer DT > (provided by firmware perhaps) with a stable kernel version. > As above, this is in no way limited to pinctrl drivers. > Unfortunately, this change breaks with modules as we have no way of > knowing when modules are done loading. One possibility is to make this > opt in or out based on compatible strings rather than at a subsystem level. > Ideally this information could be extracted automatically somehow. OTOH, > maybe the lists are pretty small. There's only a handful of subsystems > that can be optional, and then only so many drivers in those that can be > modules (at least for pinctrl, many drivers are built-in only). > On the Qualcomm platform most drivers are tristate and on most platforms there are size restrictions in the proprietary boot loader preventing us from boot the kernel after switching all these frameworks from tristate to bool (which feels like a more appropriate solution than your hack). > Cc: Alexander Graf <agraf@suse.de> > Signed-off-by: Rob Herring <robh@kernel.org> > --- > This patch came out of a discussion on the ARM boot-architecture > list[1] about DT forwards and backwards compatibility issues. There are > issues with newer DTs breaking on older, stable kernels. Some of these > are difficult to solve, but cases of optional devices not having > kernel support should be solvable. > There are two cases here: 1) DT contains compatibles that isn't supported by the kernel. In this case the associated device will remain in the probe deferral list and user space won't know about the device. 2) DT contains compatibles known to the kernel but has new optional properties that makes things functional or works around hardware bugs. > I tested this on a RPi3 B with the pinctrl driver forced off. With this > change, the MMC/SD and UART drivers can function without the pinctrl > driver. > Cool, so what about graphics, audio, networking, usb and all the other things that people actually expect when they _use_ a distro? > Rob > > [1] https://lists.linaro.org/pipermail/boot-architecture/2018-April/000466.html > > drivers/base/dd.c | 16 ++++++++++++++++ > drivers/pinctrl/devicetree.c | 2 +- > include/linux/device.h | 2 ++ > 3 files changed, 19 insertions(+), 1 deletion(-) > > diff --git a/drivers/base/dd.c b/drivers/base/dd.c > index c9f54089429b..5848808b9d7a 100644 > --- a/drivers/base/dd.c > +++ b/drivers/base/dd.c > @@ -226,6 +226,15 @@ void device_unblock_probing(void) > driver_deferred_probe_trigger(); > } > > + > +int driver_deferred_probe_optional(void) > +{ > + if (initcalls_done) > + return -ENODEV; You forgot the humongous printout here that tells the users that we do not want any bug reports related hardware not working as expected after this point. > + > + return -EPROBE_DEFER; > +} > + I do not agree with the partial benefits of this at the cost of not supporting kernel modules. Regards, Bjorn ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [RFC PATCH] driver core: make deferring probe forever optional 2018-05-07 18:31 ` Bjorn Andersson @ 2018-05-07 19:55 ` Rob Herring 2018-05-07 22:34 ` Bjorn Andersson 2018-05-09 9:44 ` Alexander Graf 1 sibling, 1 reply; 20+ messages in thread From: Rob Herring @ 2018-05-07 19:55 UTC (permalink / raw) To: Bjorn Andersson Cc: linux-kernel, devicetree, Greg Kroah-Hartman, Grant Likely, Linus Walleij, Mark Brown, Stephen Boyd, Architecture Mailman List, moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE, Alexander Graf On Mon, May 7, 2018 at 1:31 PM, Bjorn Andersson <bjorn.andersson@linaro.org> wrote: > On Tue 01 May 14:31 PDT 2018, Rob Herring wrote: > >> Deferred probe will currently wait forever on dependent devices to probe, >> but sometimes a driver will never exist. It's also not always critical for >> a driver to exist. Platforms can rely on default configuration from the >> bootloader or reset defaults for things such as pinctrl and power domains. > > But how do you know if this is the case? Because the platform worked before adding the dependency in the dts. >> This is often the case with initial platform support until various drivers >> get enabled. > > Can you please name platform that has enough support for Alexander to > care about backwards and forwards compatibility but lacks a pinctrl > driver. Alex will have to answer that. I do agree pinctrl drivers shouldn't be that hard because it is all just translating a bunch of pin data into one-time (mostly) register writes, so it shouldn't take that long to implement support. OTOH, maybe a pinctrl driver is low priority because nothing needs it yet. Either a given board works with the defaults and only some new board needs to change things or you don't need pinctrl until low power modes are implemented. However, power domains have the same problem and it can take years for those to get proper support. Alex proposed declaring dts files stable and then enforcing compatibility after that point. If anyone believes that will work, then please send a patch marking all the platforms in the kernel tree that are stable. >> There's at least 2 scenarios where deferred probe can render >> a platform broken. Both involve using a DT which has more devices and >> dependencies than the kernel supports. The 1st case is a driver may be >> disabled in the kernel config. > > I agree that there is a chance that you _might_ get some parts of the > system working by relying on the boot loader configuration, but > misconfiguration issues applies to any other fundamental providers, such > as clocks, regulators, power domains and gpios as well. If it is only a chance, then perhaps we shouldn't allow things upstream without proper drivers for all these things. That will only give users the wrong impression. >> The 2nd case is the kernel version may >> simply not have the dependent driver. This can happen if using a newer DT >> (provided by firmware perhaps) with a stable kernel version. >> > > As above, this is in no way limited to pinctrl drivers. Yes, I wasn't trying to imply that with this patch. I was just starting with 1 example. IOMMUs (which essentially is already doing what this patch does) and power domains are the main other 2. Clocks is an obvious one too, but from the discussion I referenced that problem is a bit different as platforms change from dummy fixed-clocks to a real clock controller driver. That will need a different solution. >> Unfortunately, this change breaks with modules as we have no way of >> knowing when modules are done loading. One possibility is to make this >> opt in or out based on compatible strings rather than at a subsystem level. >> Ideally this information could be extracted automatically somehow. OTOH, >> maybe the lists are pretty small. There's only a handful of subsystems >> that can be optional, and then only so many drivers in those that can be >> modules (at least for pinctrl, many drivers are built-in only). >> > > On the Qualcomm platform most drivers are tristate and on most platforms > there are size restrictions in the proprietary boot loader preventing us > from boot the kernel after switching all these frameworks from tristate > to bool (which feels like a more appropriate solution than your hack). BTW, QCom platforms are almost the only ones with pinctrl drivers as modules. They are also happen to be PIA to configure correctly for a board. However, I would like a solution that works with modules. It would be nice to know when userspace finished processing all the coldplug uevents. That would be sufficient to support modules. I researched that a bit and it doesn't seem the kernel can tell when that has happened. > >> Cc: Alexander Graf <agraf@suse.de> >> Signed-off-by: Rob Herring <robh@kernel.org> >> --- >> This patch came out of a discussion on the ARM boot-architecture >> list[1] about DT forwards and backwards compatibility issues. There are >> issues with newer DTs breaking on older, stable kernels. Some of these >> are difficult to solve, but cases of optional devices not having >> kernel support should be solvable. >> > > There are two cases here: > 1) DT contains compatibles that isn't supported by the kernel. In this > case the associated device will remain in the probe deferral list and > user space won't know about the device. > > 2) DT contains compatibles known to the kernel but has new optional > properties that makes things functional or works around hardware bugs. > >> I tested this on a RPi3 B with the pinctrl driver forced off. With this >> change, the MMC/SD and UART drivers can function without the pinctrl >> driver. >> > > Cool, so what about graphics, audio, networking, usb and all the other > things that people actually expect when they _use_ a distro? I often care about none of those things. When I do, I'd rather boot to a working console with those broken than have to debug why the kernel panicked. >> [1] https://lists.linaro.org/pipermail/boot-architecture/2018-April/000466.html >> >> drivers/base/dd.c | 16 ++++++++++++++++ >> drivers/pinctrl/devicetree.c | 2 +- >> include/linux/device.h | 2 ++ >> 3 files changed, 19 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/base/dd.c b/drivers/base/dd.c >> index c9f54089429b..5848808b9d7a 100644 >> --- a/drivers/base/dd.c >> +++ b/drivers/base/dd.c >> @@ -226,6 +226,15 @@ void device_unblock_probing(void) >> driver_deferred_probe_trigger(); >> } >> >> + >> +int driver_deferred_probe_optional(void) >> +{ >> + if (initcalls_done) >> + return -ENODEV; > > You forgot the humongous printout here that tells the users that we do > not want any bug reports related hardware not working as expected after > this point. I assume you were joking, but I would happily add a WARN here. Spewing new warnings while still booting is a better UX than just panicking. Ideally, it would be once per missing dependency. Rob ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [RFC PATCH] driver core: make deferring probe forever optional 2018-05-07 19:55 ` Rob Herring @ 2018-05-07 22:34 ` Bjorn Andersson 2018-05-09 9:18 ` Mark Brown ` (2 more replies) 0 siblings, 3 replies; 20+ messages in thread From: Bjorn Andersson @ 2018-05-07 22:34 UTC (permalink / raw) To: Rob Herring Cc: linux-kernel, devicetree, Greg Kroah-Hartman, Grant Likely, Linus Walleij, Mark Brown, Stephen Boyd, Architecture Mailman List, moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE, Alexander Graf On Mon 07 May 12:55 PDT 2018, Rob Herring wrote: > On Mon, May 7, 2018 at 1:31 PM, Bjorn Andersson > <bjorn.andersson@linaro.org> wrote: > > On Tue 01 May 14:31 PDT 2018, Rob Herring wrote: > > > >> Deferred probe will currently wait forever on dependent devices to probe, > >> but sometimes a driver will never exist. It's also not always critical for > >> a driver to exist. Platforms can rely on default configuration from the > >> bootloader or reset defaults for things such as pinctrl and power domains. > > > > But how do you know if this is the case? > > Because the platform worked before adding the dependency in the dts. > I'm worried about how to write dts files and drivers to support all permutation of forward and backward dependencies. And you most definitely have the same case with bootloader-enabled clocks, regulators and interconnects. > >> This is often the case with initial platform support until various drivers > >> get enabled. > > > > Can you please name platform that has enough support for Alexander to > > care about backwards and forwards compatibility but lacks a pinctrl > > driver. > > Alex will have to answer that. I do agree pinctrl drivers shouldn't be > that hard because it is all just translating a bunch of pin data into > one-time (mostly) register writes, so it shouldn't take that long to > implement support. OTOH, maybe a pinctrl driver is low priority > because nothing needs it yet. Either a given board works with the > defaults and only some new board needs to change things or you don't > need pinctrl until low power modes are implemented. However, power > domains have the same problem and it can take years for those to get > proper support. > > Alex proposed declaring dts files stable and then enforcing > compatibility after that point. If anyone believes that will work, > then please send a patch marking all the platforms in the kernel tree > that are stable. > That might be a reasonable idea, but at least in our corner the current decision that devicetree should be backwards compatible does make it quite cumbersome to break this assumption - and in the cases we have had to do it it's really been necessary. > >> There's at least 2 scenarios where deferred probe can render > >> a platform broken. Both involve using a DT which has more devices and > >> dependencies than the kernel supports. The 1st case is a driver may be > >> disabled in the kernel config. > > > > I agree that there is a chance that you _might_ get some parts of the > > system working by relying on the boot loader configuration, but > > misconfiguration issues applies to any other fundamental providers, such > > as clocks, regulators, power domains and gpios as well. > > If it is only a chance, then perhaps we shouldn't allow things > upstream without proper drivers for all these things. That will only > give users the wrong impression. > It's not as much the drivers that's the problem here as it is the composition of the drivers. For this particular case it would be convenient not to ship the partial dtb, or at least not ship it with the promise that it's stable. > >> The 2nd case is the kernel version may > >> simply not have the dependent driver. This can happen if using a newer DT > >> (provided by firmware perhaps) with a stable kernel version. > >> > > > > As above, this is in no way limited to pinctrl drivers. > > Yes, I wasn't trying to imply that with this patch. I was just > starting with 1 example. IOMMUs (which essentially is already doing > what this patch does) and power domains are the main other 2. qcom,iommu-v1 is bool, but depends on e.g. CONFIG_MSM_GCC_8916 which is tristate. So you would need to s/tristate/bool/ everything in drivers/clk/qcom/Kconfig as well. Not to mention that there are interconnects and power domains actually involved here as well... > Clocks is an obvious one too, but from the discussion I referenced > that problem is a bit different as platforms change from dummy > fixed-clocks to a real clock controller driver. That will need a > different solution. So how are you going to deal with the case when a vendor decides to ship their firmware package with all clocks enabled and only fixed clocks described in DT and as they upstream a clock driver and patch their firmware to do the right thing? (Or the much less extreme case where this happens for a single clock, regulator, pinctrl, interconnect, etc to fix some bug/power management behavior) And is this really a problem that does not exists in the ACPI world? > > >> Unfortunately, this change breaks with modules as we have no way of > >> knowing when modules are done loading. One possibility is to make this > >> opt in or out based on compatible strings rather than at a subsystem level. > >> Ideally this information could be extracted automatically somehow. OTOH, > >> maybe the lists are pretty small. There's only a handful of subsystems > >> that can be optional, and then only so many drivers in those that can be > >> modules (at least for pinctrl, many drivers are built-in only). > >> > > > > On the Qualcomm platform most drivers are tristate and on most platforms > > there are size restrictions in the proprietary boot loader preventing us > > from boot the kernel after switching all these frameworks from tristate > > to bool (which feels like a more appropriate solution than your hack). > > BTW, QCom platforms are almost the only ones with pinctrl drivers as > modules. They are also happen to be PIA to configure correctly for a > board. > There are a few pinctrl drivers for chips sitting on i2c busses, as such changing this requirement would trickle down to a number of possible i2c masters as well. Sorry to hear that you find it so difficult to configure the pinctrl, it's (almost) entirely using the common pinctrl bindings. Perhaps we need to add some documentation of the hardware in the binding? > However, I would like a solution that works with modules. It would be > nice to know when userspace finished processing all the coldplug > uevents. That would be sufficient to support modules. I researched > that a bit and it doesn't seem the kernel can tell when that has > happened. > It's not that far from the issue I have in remoteproc, where I would like to boot a DSP as soon as the firmware is available - which might be probed at any time after boot. [..] > >> I tested this on a RPi3 B with the pinctrl driver forced off. With this > >> change, the MMC/SD and UART drivers can function without the pinctrl > >> driver. > >> > > > > Cool, so what about graphics, audio, networking, usb and all the other > > things that people actually expect when they _use_ a distro? > > I often care about none of those things. When I do, I'd rather boot to > a working console with those broken than have to debug why the kernel > panicked. > But that's developer-you speaking, developer-me totally agree. But when I take the role of being a user of a distro I most definitely do expect functionality beyond the basics used by the boot loader (UART and dependencies of the primary storage device). My argument is simply that in neither of these cases this patch is helpful. [..] > >> +int driver_deferred_probe_optional(void) > >> +{ > >> + if (initcalls_done) > >> + return -ENODEV; > > > > You forgot the humongous printout here that tells the users that we do > > not want any bug reports related hardware not working as expected after > > this point. > > I assume you were joking, but I would happily add a WARN here. About the print yes, but I most definitely do not want to debug issues related to this! The crazy issues you get from having electrical properties slightly off (e.g. drive-strength of the SD-card pins) or the fact that any driver using pinmuxing will depend on the modprobe ordering. > Spewing new warnings while still booting is a better UX than just > panicking. Ideally, it would be once per missing dependency. > Having a convenient way of listing all unmatched devices or devices sitting in probe deferral would be quite convenient, as a development tool. I know this hassle was the starting point of some of Frank's tools. Regards, Bjorn ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [RFC PATCH] driver core: make deferring probe forever optional 2018-05-07 22:34 ` Bjorn Andersson @ 2018-05-09 9:18 ` Mark Brown 2018-05-09 9:57 ` Alexander Graf 2018-05-09 22:30 ` Rob Herring 2 siblings, 0 replies; 20+ messages in thread From: Mark Brown @ 2018-05-09 9:18 UTC (permalink / raw) To: Bjorn Andersson Cc: Rob Herring, linux-kernel, devicetree, Greg Kroah-Hartman, Grant Likely, Linus Walleij, Stephen Boyd, Architecture Mailman List, moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE, Alexander Graf [-- Attachment #1: Type: text/plain, Size: 825 bytes --] On Mon, May 07, 2018 at 03:34:38PM -0700, Bjorn Andersson wrote: > And is this really a problem that does not exists in the ACPI world? Sort of, in that on ACPI systems all devices are expected to live in glorious isolation and anything they need transparently configured by the firmware with any information about clock speeds or whatever coming from proprietary/device specific properties (though that's severely frowned upon) or the apparently idiomatic technique of hardcoding based on DMI data which has some scalability issues. This works great for systems intended to work in the ACPI model but is not entirely successful once you get outside of that. Some of the embedded ACPI people have been importing bits of DT wholesale into ACPI, for those bits obviously all the DT issues get imported too. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [RFC PATCH] driver core: make deferring probe forever optional 2018-05-07 22:34 ` Bjorn Andersson 2018-05-09 9:18 ` Mark Brown @ 2018-05-09 9:57 ` Alexander Graf 2018-05-09 22:34 ` Rob Herring 2018-05-09 22:30 ` Rob Herring 2 siblings, 1 reply; 20+ messages in thread From: Alexander Graf @ 2018-05-09 9:57 UTC (permalink / raw) To: Bjorn Andersson, Rob Herring Cc: linux-kernel, devicetree, Greg Kroah-Hartman, Grant Likely, Linus Walleij, Mark Brown, Stephen Boyd, Architecture Mailman List, moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE On 05/08/2018 12:34 AM, Bjorn Andersson wrote: > On Mon 07 May 12:55 PDT 2018, Rob Herring wrote: > >> On Mon, May 7, 2018 at 1:31 PM, Bjorn Andersson >> <bjorn.andersson@linaro.org> wrote: >>> On Tue 01 May 14:31 PDT 2018, Rob Herring wrote: >>> >>>> Deferred probe will currently wait forever on dependent devices to probe, >>>> but sometimes a driver will never exist. It's also not always critical for >>>> a driver to exist. Platforms can rely on default configuration from the >>>> bootloader or reset defaults for things such as pinctrl and power domains. >>> But how do you know if this is the case? >> Because the platform worked before adding the dependency in the dts. >> > I'm worried about how to write dts files and drivers to support all > permutation of forward and backward dependencies. And you most > definitely have the same case with bootloader-enabled clocks, > regulators and interconnects. > >>>> This is often the case with initial platform support until various drivers >>>> get enabled. >>> Can you please name platform that has enough support for Alexander to >>> care about backwards and forwards compatibility but lacks a pinctrl >>> driver. >> Alex will have to answer that. I do agree pinctrl drivers shouldn't be >> that hard because it is all just translating a bunch of pin data into >> one-time (mostly) register writes, so it shouldn't take that long to >> implement support. OTOH, maybe a pinctrl driver is low priority >> because nothing needs it yet. Either a given board works with the >> defaults and only some new board needs to change things or you don't >> need pinctrl until low power modes are implemented. However, power >> domains have the same problem and it can take years for those to get >> proper support. >> >> Alex proposed declaring dts files stable and then enforcing >> compatibility after that point. If anyone believes that will work, >> then please send a patch marking all the platforms in the kernel tree >> that are stable. >> > That might be a reasonable idea, but at least in our corner the current > decision that devicetree should be backwards compatible does make it > quite cumbersome to break this assumption - and in the cases we have had > to do it it's really been necessary. I'm sure Rob would be happy to get a list of every one of those instances so we can see how to solve them going forward. To give you some background: The whole discussion started with a proposal from me to support embedded (maybe dtc aided) overlays. Some way to have a single dtb that only enables new features such as pinctrl when the kernel indicates support for them. I think eventually we will have to have a mechanism like that for platforms that want to maintain compatibility. But the less we have to solve using it the better off everyone is, because it just increases complexity. > >>>> There's at least 2 scenarios where deferred probe can render >>>> a platform broken. Both involve using a DT which has more devices and >>>> dependencies than the kernel supports. The 1st case is a driver may be >>>> disabled in the kernel config. >>> I agree that there is a chance that you _might_ get some parts of the >>> system working by relying on the boot loader configuration, but >>> misconfiguration issues applies to any other fundamental providers, such >>> as clocks, regulators, power domains and gpios as well. >> If it is only a chance, then perhaps we shouldn't allow things >> upstream without proper drivers for all these things. That will only >> give users the wrong impression. >> > It's not as much the drivers that's the problem here as it is the > composition of the drivers. For this particular case it would be > convenient not to ship the partial dtb, or at least not ship it with the > promise that it's stable. Yes, we of course need a gatekeeping event. Not every DT is in a state where you can promise compatibility. However, if you want to have a stable OS interface so that slow moving Linux distribtions work well with the platform and non-Linux OSs jump on the platform, you will have to provide some guarantees. And people just need to be aware that they either give the guarantees or they don't get their benefits :). > >>>> The 2nd case is the kernel version may >>>> simply not have the dependent driver. This can happen if using a newer DT >>>> (provided by firmware perhaps) with a stable kernel version. >>>> >>> As above, this is in no way limited to pinctrl drivers. >> Yes, I wasn't trying to imply that with this patch. I was just >> starting with 1 example. IOMMUs (which essentially is already doing >> what this patch does) and power domains are the main other 2. > qcom,iommu-v1 is bool, but depends on e.g. CONFIG_MSM_GCC_8916 which is > tristate. So you would need to s/tristate/bool/ everything in > drivers/clk/qcom/Kconfig as well. Not to mention that there are > interconnects and power domains actually involved here as well... > >> Clocks is an obvious one too, but from the discussion I referenced >> that problem is a bit different as platforms change from dummy >> fixed-clocks to a real clock controller driver. That will need a >> different solution. > So how are you going to deal with the case when a vendor decides to ship > their firmware package with all clocks enabled and only fixed clocks > described in DT and as they upstream a clock driver and patch their > firmware to do the right thing? That is the ZynqMP case. I think this really needs to be solved using embedded overlays, but Rob might have additional ideas :). > (Or the much less extreme case where this happens for a single clock, > regulator, pinctrl, interconnect, etc to fix some bug/power management > behavior) > > And is this really a problem that does not exists in the ACPI world? > >>>> Unfortunately, this change breaks with modules as we have no way of >>>> knowing when modules are done loading. One possibility is to make this >>>> opt in or out based on compatible strings rather than at a subsystem level. >>>> Ideally this information could be extracted automatically somehow. OTOH, >>>> maybe the lists are pretty small. There's only a handful of subsystems >>>> that can be optional, and then only so many drivers in those that can be >>>> modules (at least for pinctrl, many drivers are built-in only). >>>> >>> On the Qualcomm platform most drivers are tristate and on most platforms >>> there are size restrictions in the proprietary boot loader preventing us >>> from boot the kernel after switching all these frameworks from tristate >>> to bool (which feels like a more appropriate solution than your hack). >> BTW, QCom platforms are almost the only ones with pinctrl drivers as >> modules. They are also happen to be PIA to configure correctly for a >> board. >> > There are a few pinctrl drivers for chips sitting on i2c busses, as such > changing this requirement would trickle down to a number of possible i2c > masters as well. > > Sorry to hear that you find it so difficult to configure the pinctrl, > it's (almost) entirely using the common pinctrl bindings. Perhaps we > need to add some documentation of the hardware in the binding? > >> However, I would like a solution that works with modules. It would be >> nice to know when userspace finished processing all the coldplug >> uevents. That would be sufficient to support modules. I researched >> that a bit and it doesn't seem the kernel can tell when that has >> happened. >> > It's not that far from the issue I have in remoteproc, where I would > like to boot a DSP as soon as the firmware is available - which might be > probed at any time after boot. > > [..] >>>> I tested this on a RPi3 B with the pinctrl driver forced off. With this >>>> change, the MMC/SD and UART drivers can function without the pinctrl >>>> driver. >>>> >>> Cool, so what about graphics, audio, networking, usb and all the other >>> things that people actually expect when they _use_ a distro? >> I often care about none of those things. When I do, I'd rather boot to >> a working console with those broken than have to debug why the kernel >> panicked. >> > But that's developer-you speaking, developer-me totally agree. > > But when I take the role of being a user of a distro I most definitely > do expect functionality beyond the basics used by the boot loader (UART > and dependencies of the primary storage device). > > My argument is simply that in neither of these cases this patch is > helpful. The patch allows firmware to provide pinctrl information but maintain backwards compatibility with kernels that don't implement pinctrl setting. It's useful to solve that part of the transition of the DT to enable new functionality. If you now add a device that explicitly needs pinctrl configuration to work, that would probably need to get added using the overlay mechanism I described above. Alex ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [RFC PATCH] driver core: make deferring probe forever optional 2018-05-09 9:57 ` Alexander Graf @ 2018-05-09 22:34 ` Rob Herring 0 siblings, 0 replies; 20+ messages in thread From: Rob Herring @ 2018-05-09 22:34 UTC (permalink / raw) To: Alexander Graf Cc: Bjorn Andersson, linux-kernel, devicetree, Greg Kroah-Hartman, Grant Likely, Linus Walleij, Mark Brown, Stephen Boyd, Architecture Mailman List, moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE On Wed, May 9, 2018 at 4:57 AM, Alexander Graf <agraf@suse.de> wrote: > On 05/08/2018 12:34 AM, Bjorn Andersson wrote: >> >> On Mon 07 May 12:55 PDT 2018, Rob Herring wrote: >> >>> On Mon, May 7, 2018 at 1:31 PM, Bjorn Andersson >>> <bjorn.andersson@linaro.org> wrote: >>>> >>>> On Tue 01 May 14:31 PDT 2018, Rob Herring wrote: >>>> >>>>> Deferred probe will currently wait forever on dependent devices to >>>>> probe, >>>>> but sometimes a driver will never exist. It's also not always critical >>>>> for >>>>> a driver to exist. Platforms can rely on default configuration from the >>>>> bootloader or reset defaults for things such as pinctrl and power >>>>> domains. >>>> >>>> But how do you know if this is the case? >>> >>> Because the platform worked before adding the dependency in the dts. >>> >> I'm worried about how to write dts files and drivers to support all >> permutation of forward and backward dependencies. And you most >> definitely have the same case with bootloader-enabled clocks, >> regulators and interconnects. >> >>>>> This is often the case with initial platform support until various >>>>> drivers >>>>> get enabled. >>>> >>>> Can you please name platform that has enough support for Alexander to >>>> care about backwards and forwards compatibility but lacks a pinctrl >>>> driver. >>> >>> Alex will have to answer that. I do agree pinctrl drivers shouldn't be >>> that hard because it is all just translating a bunch of pin data into >>> one-time (mostly) register writes, so it shouldn't take that long to >>> implement support. OTOH, maybe a pinctrl driver is low priority >>> because nothing needs it yet. Either a given board works with the >>> defaults and only some new board needs to change things or you don't >>> need pinctrl until low power modes are implemented. However, power >>> domains have the same problem and it can take years for those to get >>> proper support. >>> >>> Alex proposed declaring dts files stable and then enforcing >>> compatibility after that point. If anyone believes that will work, >>> then please send a patch marking all the platforms in the kernel tree >>> that are stable. >>> >> That might be a reasonable idea, but at least in our corner the current >> decision that devicetree should be backwards compatible does make it >> quite cumbersome to break this assumption - and in the cases we have had >> to do it it's really been necessary. > > > I'm sure Rob would be happy to get a list of every one of those instances so > we can see how to solve them going forward. > > To give you some background: The whole discussion started with a proposal > from me to support embedded (maybe dtc aided) overlays. Some way to have a > single dtb that only enables new features such as pinctrl when the kernel > indicates support for them. > > I think eventually we will have to have a mechanism like that for platforms > that want to maintain compatibility. But the less we have to solve using it > the better off everyone is, because it just increases complexity. > >> >>>>> There's at least 2 scenarios where deferred probe can render >>>>> a platform broken. Both involve using a DT which has more devices and >>>>> dependencies than the kernel supports. The 1st case is a driver may be >>>>> disabled in the kernel config. >>>> >>>> I agree that there is a chance that you _might_ get some parts of the >>>> system working by relying on the boot loader configuration, but >>>> misconfiguration issues applies to any other fundamental providers, such >>>> as clocks, regulators, power domains and gpios as well. >>> >>> If it is only a chance, then perhaps we shouldn't allow things >>> upstream without proper drivers for all these things. That will only >>> give users the wrong impression. >>> >> It's not as much the drivers that's the problem here as it is the >> composition of the drivers. For this particular case it would be >> convenient not to ship the partial dtb, or at least not ship it with the >> promise that it's stable. > > > Yes, we of course need a gatekeeping event. Not every DT is in a state where > you can promise compatibility. > > However, if you want to have a stable OS interface so that slow moving Linux > distribtions work well with the platform and non-Linux OSs jump on the > platform, you will have to provide some guarantees. And people just need to > be aware that they either give the guarantees or they don't get their > benefits :). > >> >>>>> The 2nd case is the kernel version may >>>>> simply not have the dependent driver. This can happen if using a newer >>>>> DT >>>>> (provided by firmware perhaps) with a stable kernel version. >>>>> >>>> As above, this is in no way limited to pinctrl drivers. >>> >>> Yes, I wasn't trying to imply that with this patch. I was just >>> starting with 1 example. IOMMUs (which essentially is already doing >>> what this patch does) and power domains are the main other 2. >> >> qcom,iommu-v1 is bool, but depends on e.g. CONFIG_MSM_GCC_8916 which is >> tristate. So you would need to s/tristate/bool/ everything in >> drivers/clk/qcom/Kconfig as well. Not to mention that there are >> interconnects and power domains actually involved here as well... >> >>> Clocks is an obvious one too, but from the discussion I referenced >>> that problem is a bit different as platforms change from dummy >>> fixed-clocks to a real clock controller driver. That will need a >>> different solution. >> >> So how are you going to deal with the case when a vendor decides to ship >> their firmware package with all clocks enabled and only fixed clocks >> described in DT and as they upstream a clock driver and patch their >> firmware to do the right thing? > > > That is the ZynqMP case. I think this really needs to be solved using > embedded overlays, but Rob might have additional ideas :). > > >> (Or the much less extreme case where this happens for a single clock, >> regulator, pinctrl, interconnect, etc to fix some bug/power management >> behavior) >> >> And is this really a problem that does not exists in the ACPI world? >> >>>>> Unfortunately, this change breaks with modules as we have no way of >>>>> knowing when modules are done loading. One possibility is to make this >>>>> opt in or out based on compatible strings rather than at a subsystem >>>>> level. >>>>> Ideally this information could be extracted automatically somehow. >>>>> OTOH, >>>>> maybe the lists are pretty small. There's only a handful of subsystems >>>>> that can be optional, and then only so many drivers in those that can >>>>> be >>>>> modules (at least for pinctrl, many drivers are built-in only). >>>>> >>>> On the Qualcomm platform most drivers are tristate and on most platforms >>>> there are size restrictions in the proprietary boot loader preventing us >>>> from boot the kernel after switching all these frameworks from tristate >>>> to bool (which feels like a more appropriate solution than your hack). >>> >>> BTW, QCom platforms are almost the only ones with pinctrl drivers as >>> modules. They are also happen to be PIA to configure correctly for a >>> board. >>> >> There are a few pinctrl drivers for chips sitting on i2c busses, as such >> changing this requirement would trickle down to a number of possible i2c >> masters as well. >> >> Sorry to hear that you find it so difficult to configure the pinctrl, >> it's (almost) entirely using the common pinctrl bindings. Perhaps we >> need to add some documentation of the hardware in the binding? >> >>> However, I would like a solution that works with modules. It would be >>> nice to know when userspace finished processing all the coldplug >>> uevents. That would be sufficient to support modules. I researched >>> that a bit and it doesn't seem the kernel can tell when that has >>> happened. >>> >> It's not that far from the issue I have in remoteproc, where I would >> like to boot a DSP as soon as the firmware is available - which might be >> probed at any time after boot. >> >> [..] >>>>> >>>>> I tested this on a RPi3 B with the pinctrl driver forced off. With this >>>>> change, the MMC/SD and UART drivers can function without the pinctrl >>>>> driver. >>>>> >>>> Cool, so what about graphics, audio, networking, usb and all the other >>>> things that people actually expect when they _use_ a distro? >>> >>> I often care about none of those things. When I do, I'd rather boot to >>> a working console with those broken than have to debug why the kernel >>> panicked. >>> >> But that's developer-you speaking, developer-me totally agree. >> >> But when I take the role of being a user of a distro I most definitely >> do expect functionality beyond the basics used by the boot loader (UART >> and dependencies of the primary storage device). >> >> My argument is simply that in neither of these cases this patch is >> helpful. > > > The patch allows firmware to provide pinctrl information but maintain > backwards compatibility with kernels that don't implement pinctrl setting. > It's useful to solve that part of the transition of the DT to enable new > functionality. If you now add a device that explicitly needs pinctrl > configuration to work, that would probably need to get added using the > overlay mechanism I described above. I started looking at making this more of a debug option with a kernel command line option for deferred probe timeout. That only kind of works for serial ports due to the console. I can at least get console output after the timeout expires. However, userspace expects the console to g > > > Alex > ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [RFC PATCH] driver core: make deferring probe forever optional 2018-05-07 22:34 ` Bjorn Andersson 2018-05-09 9:18 ` Mark Brown 2018-05-09 9:57 ` Alexander Graf @ 2018-05-09 22:30 ` Rob Herring 2 siblings, 0 replies; 20+ messages in thread From: Rob Herring @ 2018-05-09 22:30 UTC (permalink / raw) To: Bjorn Andersson Cc: linux-kernel, devicetree, Greg Kroah-Hartman, Grant Likely, Linus Walleij, Mark Brown, Stephen Boyd, Architecture Mailman List, moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE, Alexander Graf On Mon, May 7, 2018 at 5:34 PM, Bjorn Andersson <bjorn.andersson@linaro.org> wrote: > On Mon 07 May 12:55 PDT 2018, Rob Herring wrote: > >> On Mon, May 7, 2018 at 1:31 PM, Bjorn Andersson >> <bjorn.andersson@linaro.org> wrote: >> > On Tue 01 May 14:31 PDT 2018, Rob Herring wrote: >> > >> >> Deferred probe will currently wait forever on dependent devices to probe, >> >> but sometimes a driver will never exist. It's also not always critical for >> >> a driver to exist. Platforms can rely on default configuration from the >> >> bootloader or reset defaults for things such as pinctrl and power domains. >> > >> > But how do you know if this is the case? >> >> Because the platform worked before adding the dependency in the dts. >> > > I'm worried about how to write dts files and drivers to support all > permutation of forward and backward dependencies. And you most > definitely have the same case with bootloader-enabled clocks, > regulators and interconnects. > >> >> This is often the case with initial platform support until various drivers >> >> get enabled. >> > >> > Can you please name platform that has enough support for Alexander to >> > care about backwards and forwards compatibility but lacks a pinctrl >> > driver. >> >> Alex will have to answer that. I do agree pinctrl drivers shouldn't be >> that hard because it is all just translating a bunch of pin data into >> one-time (mostly) register writes, so it shouldn't take that long to >> implement support. OTOH, maybe a pinctrl driver is low priority >> because nothing needs it yet. Either a given board works with the >> defaults and only some new board needs to change things or you don't >> need pinctrl until low power modes are implemented. However, power >> domains have the same problem and it can take years for those to get >> proper support. >> >> Alex proposed declaring dts files stable and then enforcing >> compatibility after that point. If anyone believes that will work, >> then please send a patch marking all the platforms in the kernel tree >> that are stable. >> > > That might be a reasonable idea, but at least in our corner the current > decision that devicetree should be backwards compatible does make it > quite cumbersome to break this assumption - and in the cases we have had > to do it it's really been necessary. > >> >> There's at least 2 scenarios where deferred probe can render >> >> a platform broken. Both involve using a DT which has more devices and >> >> dependencies than the kernel supports. The 1st case is a driver may be >> >> disabled in the kernel config. >> > >> > I agree that there is a chance that you _might_ get some parts of the >> > system working by relying on the boot loader configuration, but >> > misconfiguration issues applies to any other fundamental providers, such >> > as clocks, regulators, power domains and gpios as well. >> >> If it is only a chance, then perhaps we shouldn't allow things >> upstream without proper drivers for all these things. That will only >> give users the wrong impression. >> > > It's not as much the drivers that's the problem here as it is the > composition of the drivers. For this particular case it would be > convenient not to ship the partial dtb, or at least not ship it with the > promise that it's stable. > >> >> The 2nd case is the kernel version may >> >> simply not have the dependent driver. This can happen if using a newer DT >> >> (provided by firmware perhaps) with a stable kernel version. >> >> >> > >> > As above, this is in no way limited to pinctrl drivers. >> >> Yes, I wasn't trying to imply that with this patch. I was just >> starting with 1 example. IOMMUs (which essentially is already doing >> what this patch does) and power domains are the main other 2. > > qcom,iommu-v1 is bool, but depends on e.g. CONFIG_MSM_GCC_8916 which is > tristate. So you would need to s/tristate/bool/ everything in > drivers/clk/qcom/Kconfig as well. Not to mention that there are > interconnects and power domains actually involved here as well... This would be part of the fun in configuring QCom platforms. >> Clocks is an obvious one too, but from the discussion I referenced >> that problem is a bit different as platforms change from dummy >> fixed-clocks to a real clock controller driver. That will need a >> different solution. > > So how are you going to deal with the case when a vendor decides to ship > their firmware package with all clocks enabled and only fixed clocks > described in DT and as they upstream a clock driver and patch their > firmware to do the right thing? That's exactly the problem for clocks. For that, I think we have to say no and require a driver from the start. Now perhaps we just need to define clock ids and every clock id just maps to some fixed clock. We could have a common driver with tables of id and freq for devices. There's probably some cases like platforms that are being reverse engineered where even coming up with the blocks and clock ids is a problem, but I think that is the exception. > (Or the much less extreme case where this happens for a single clock, > regulator, pinctrl, interconnect, etc to fix some bug/power management > behavior) > > And is this really a problem that does not exists in the ACPI world? It's a problem that doesn't exist in some of the DT world. The part that doesn't doesn't expose all these low-level details to the kernel. ACPI only exposes a higher level interface and the kernel doesn't know the details >> >> >> Unfortunately, this change breaks with modules as we have no way of >> >> knowing when modules are done loading. One possibility is to make this >> >> opt in or out based on compatible strings rather than at a subsystem level. >> >> Ideally this information could be extracted automatically somehow. OTOH, >> >> maybe the lists are pretty small. There's only a handful of subsystems >> >> that can be optional, and then only so many drivers in those that can be >> >> modules (at least for pinctrl, many drivers are built-in only). >> >> >> > >> > On the Qualcomm platform most drivers are tristate and on most platforms >> > there are size restrictions in the proprietary boot loader preventing us >> > from boot the kernel after switching all these frameworks from tristate >> > to bool (which feels like a more appropriate solution than your hack). >> >> BTW, QCom platforms are almost the only ones with pinctrl drivers as >> modules. They are also happen to be PIA to configure correctly for a >> board. >> > > There are a few pinctrl drivers for chips sitting on i2c busses, as such > changing this requirement would trickle down to a number of possible i2c > masters as well. > > Sorry to hear that you find it so difficult to configure the pinctrl, > it's (almost) entirely using the common pinctrl bindings. Perhaps we > need to add some documentation of the hardware in the binding? I didn't mean pinctrl specifically. That one is a bit easier because it is very much 1:1 with part numbers. It's just the combination of all the different QCom buses, which options apply to which SoC and options getting hidden behind depends that make it hard. Perhaps I should just give up using menuconfig. However, it's also a problem when drivers get added but folks forget to update the defconfig. I recall hitting that on DB410c some. Basically, config options for each SoC with a bunch of selects would solve the problem. > >> However, I would like a solution that works with modules. It would be >> nice to know when userspace finished processing all the coldplug >> uevents. That would be sufficient to support modules. I researched >> that a bit and it doesn't seem the kernel can tell when that has >> happened. >> > > It's not that far from the issue I have in remoteproc, where I would > like to boot a DSP as soon as the firmware is available - which might be > probed at any time after boot. > > [..] >> >> I tested this on a RPi3 B with the pinctrl driver forced off. With this >> >> change, the MMC/SD and UART drivers can function without the pinctrl >> >> driver. >> >> >> > >> > Cool, so what about graphics, audio, networking, usb and all the other >> > things that people actually expect when they _use_ a distro? >> >> I often care about none of those things. When I do, I'd rather boot to >> a working console with those broken than have to debug why the kernel >> panicked. >> > > But that's developer-you speaking, developer-me totally agree. > > But when I take the role of being a user of a distro I most definitely > do expect functionality beyond the basics used by the boot loader (UART > and dependencies of the primary storage device). > > My argument is simply that in neither of these cases this patch is > helpful. > > [..] >> >> +int driver_deferred_probe_optional(void) >> >> +{ >> >> + if (initcalls_done) >> >> + return -ENODEV; >> > >> > You forgot the humongous printout here that tells the users that we do >> > not want any bug reports related hardware not working as expected after >> > this point. >> >> I assume you were joking, but I would happily add a WARN here. > > About the print yes, but I most definitely do not want to debug issues > related to this! > > The crazy issues you get from having electrical properties slightly off > (e.g. drive-strength of the SD-card pins) or the fact that any driver > using pinmuxing will depend on the modprobe ordering. > >> Spewing new warnings while still booting is a better UX than just >> panicking. Ideally, it would be once per missing dependency. >> > > Having a convenient way of listing all unmatched devices or devices > sitting in probe deferral would be quite convenient, as a development > tool. I know this hassle was the starting point of some of Frank's > tools. > > Regards, > Bjorn ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [RFC PATCH] driver core: make deferring probe forever optional 2018-05-07 18:31 ` Bjorn Andersson 2018-05-07 19:55 ` Rob Herring @ 2018-05-09 9:44 ` Alexander Graf 2018-05-13 22:01 ` Linus Walleij 1 sibling, 1 reply; 20+ messages in thread From: Alexander Graf @ 2018-05-09 9:44 UTC (permalink / raw) To: Bjorn Andersson, Rob Herring Cc: linux-kernel, devicetree, Greg Kroah-Hartman, Grant Likely, Linus Walleij, Mark Brown, Stephen Boyd, boot-architecture, linux-arm-kernel On 05/07/2018 08:31 PM, Bjorn Andersson wrote: > On Tue 01 May 14:31 PDT 2018, Rob Herring wrote: > >> Deferred probe will currently wait forever on dependent devices to probe, >> but sometimes a driver will never exist. It's also not always critical for >> a driver to exist. Platforms can rely on default configuration from the >> bootloader or reset defaults for things such as pinctrl and power domains. > But how do you know if this is the case? > >> This is often the case with initial platform support until various drivers >> get enabled. > Can you please name platform that has enough support for Alexander to > care about backwards and forwards compatibility but lacks a pinctrl > driver. ZynqMP is one example that immediately comes to my mind. I'm sure there are others too. In general it's very frustrating to debug what goes wrong when you can't even get serial to output anything at all just because there are now pinctrl bindings that your kernel may not know about yet. I've run into that too many times. > >> There's at least 2 scenarios where deferred probe can render >> a platform broken. Both involve using a DT which has more devices and >> dependencies than the kernel supports. The 1st case is a driver may be >> disabled in the kernel config. > I agree that there is a chance that you _might_ get some parts of the > system working by relying on the boot loader configuration, but > misconfiguration issues applies to any other fundamental providers, such > as clocks, regulators, power domains and gpios as well. > >> The 2nd case is the kernel version may >> simply not have the dependent driver. This can happen if using a newer DT >> (provided by firmware perhaps) with a stable kernel version. >> > As above, this is in no way limited to pinctrl drivers. > >> Unfortunately, this change breaks with modules as we have no way of >> knowing when modules are done loading. One possibility is to make this >> opt in or out based on compatible strings rather than at a subsystem level. >> Ideally this information could be extracted automatically somehow. OTOH, >> maybe the lists are pretty small. There's only a handful of subsystems >> that can be optional, and then only so many drivers in those that can be >> modules (at least for pinctrl, many drivers are built-in only). >> > On the Qualcomm platform most drivers are tristate and on most platforms > there are size restrictions in the proprietary boot loader preventing us > from boot the kernel after switching all these frameworks from tristate > to bool (which feels like a more appropriate solution than your hack). I don't see how setting them to bool contradicts with the hack? The goal of this patch is to allow systems to load drivers on firmware provided pinctrl setups if there is no matching pinctrl driver in the kernel. > >> Cc: Alexander Graf <agraf@suse.de> >> Signed-off-by: Rob Herring <robh@kernel.org> >> --- >> This patch came out of a discussion on the ARM boot-architecture >> list[1] about DT forwards and backwards compatibility issues. There are >> issues with newer DTs breaking on older, stable kernels. Some of these >> are difficult to solve, but cases of optional devices not having >> kernel support should be solvable. >> > There are two cases here: > 1) DT contains compatibles that isn't supported by the kernel. In this > case the associated device will remain in the probe deferral list and > user space won't know about the device. > > 2) DT contains compatibles known to the kernel but has new optional > properties that makes things functional or works around hardware bugs. The key point is not to regress. Imagine you have firmware 1.0 which works with OS 1.0. Firmware provides the device tree. When you update to firmware to 1.1 you want to make sure OS 1.0 still works. The bug you're referring to that existed before of course still exists. But we're not worse off. > >> I tested this on a RPi3 B with the pinctrl driver forced off. With this >> change, the MMC/SD and UART drivers can function without the pinctrl >> driver. >> > Cool, so what about graphics, audio, networking, usb and all the other > things that people actually expect when they _use_ a distro? Again, it's about regressions. If audio didn't work before, a firmware update may not get you working audio with OS 1.0. But it may enable OS 1.1 to provide audio. Alex ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [RFC PATCH] driver core: make deferring probe forever optional 2018-05-09 9:44 ` Alexander Graf @ 2018-05-13 22:01 ` Linus Walleij 2018-05-14 7:37 ` Alexander Graf 0 siblings, 1 reply; 20+ messages in thread From: Linus Walleij @ 2018-05-13 22:01 UTC (permalink / raw) To: Alexander Graf, Sören Brinkmann Cc: Bjorn Andersson, Rob Herring, linux-kernel, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, Greg Kroah-Hartman, Grant Likely, Mark Brown, Stephen Boyd, boot-architecture, Linux ARM On Wed, May 9, 2018 at 11:44 AM, Alexander Graf <agraf@suse.de> wrote: > On 05/07/2018 08:31 PM, Bjorn Andersson wrote: >> Can you please name platform that has enough support for Alexander to >> care about backwards and forwards compatibility but lacks a pinctrl >> driver. > > ZynqMP is one example that immediately comes to my mind. I'm sure there are > others too. Why isn't that using drivers/pinctrl/pinctrl-zynq.c? How is it so very different from (old) Zynq as it is already using the same GPIO driver? Yours, Linus Walleij ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [RFC PATCH] driver core: make deferring probe forever optional 2018-05-13 22:01 ` Linus Walleij @ 2018-05-14 7:37 ` Alexander Graf 2018-05-14 12:44 ` Michal Simek 0 siblings, 1 reply; 20+ messages in thread From: Alexander Graf @ 2018-05-14 7:37 UTC (permalink / raw) To: Linus Walleij, Sören Brinkmann Cc: Bjorn Andersson, Rob Herring, linux-kernel, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, Greg Kroah-Hartman, Grant Likely, Mark Brown, Stephen Boyd, boot-architecture, Linux ARM, Michal Simek, Jan Kiszka On 05/14/2018 12:01 AM, Linus Walleij wrote: > On Wed, May 9, 2018 at 11:44 AM, Alexander Graf <agraf@suse.de> wrote: >> On 05/07/2018 08:31 PM, Bjorn Andersson wrote: >>> Can you please name platform that has enough support for Alexander to >>> care about backwards and forwards compatibility but lacks a pinctrl >>> driver. >> ZynqMP is one example that immediately comes to my mind. I'm sure there are >> others too. > Why isn't that using drivers/pinctrl/pinctrl-zynq.c? > > How is it so very different from (old) Zynq as it is already using > the same GPIO driver? That one is very simple: ZynqMP is usually an AMP system where Linux doesn't have full knowledge of the overall system. IIUC they have a tiny microblaze (PMU) that does the actual full system configuration for peripherals that may interfere with each other. This architecture also allows for safety critical code to run alongside a (less safe) Linux system. I think we'll see architectures like this pop up more over time. TI Sitara has similar issues. I know that Jailhouse ran into exactly that problem before. I also know that during Linaro Connect Budapest even the OP-TEE people realized the current model is bad, because Linux may control pins/clocks/etc of devices that the secure world wants to use. So I actually believe we will see more SoCs in the future that may even start with Linux controllable pinctrl or no pinctrl driver but then will move to firmware controlled drivers once it starts being necessary. Alex ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [RFC PATCH] driver core: make deferring probe forever optional 2018-05-14 7:37 ` Alexander Graf @ 2018-05-14 12:44 ` Michal Simek 2018-05-16 14:38 ` Linus Walleij 0 siblings, 1 reply; 20+ messages in thread From: Michal Simek @ 2018-05-14 12:44 UTC (permalink / raw) To: Alexander Graf, Linus Walleij Cc: Bjorn Andersson, Rob Herring, linux-kernel, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, Greg Kroah-Hartman, Grant Likely, Mark Brown, Stephen Boyd, boot-architecture, Linux ARM, Michal Simek, Jan Kiszka On 14.5.2018 09:37, Alexander Graf wrote: > > On 05/14/2018 12:01 AM, Linus Walleij wrote: >> On Wed, May 9, 2018 at 11:44 AM, Alexander Graf <agraf@suse.de> wrote: >>> On 05/07/2018 08:31 PM, Bjorn Andersson wrote: >>>> Can you please name platform that has enough support for Alexander to >>>> care about backwards and forwards compatibility but lacks a pinctrl >>>> driver. >>> ZynqMP is one example that immediately comes to my mind. I'm sure >>> there are >>> others too. >> Why isn't that using drivers/pinctrl/pinctrl-zynq.c? >> >> How is it so very different from (old) Zynq as it is already using >> the same GPIO driver? > > That one is very simple: ZynqMP is usually an AMP system where Linux > doesn't have full knowledge of the overall system. IIUC they have a tiny > microblaze (PMU) that does the actual full system configuration for > peripherals that may interfere with each other. This architecture also > allows for safety critical code to run alongside a (less safe) Linux > system. Linux is running in non secure world that's why Linux can't have full system visibility and Linux should ask firmware. It doesn't matter if firmware is running on specific unit or just secure SW in EL3/EL1-S, EL0-S. You can also configure ZynqMP to protect these address ranges not to be accessible from NS sw. If you don't care about security you can use normal read/write accesses at least for gpio case. Pinctrl/clk will be driven via firmware interface. On Zynq we have never finished running Linux in NS and having firmware to handle it that's why only read/write is used there. Thanks, Michal ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [RFC PATCH] driver core: make deferring probe forever optional 2018-05-14 12:44 ` Michal Simek @ 2018-05-16 14:38 ` Linus Walleij 0 siblings, 0 replies; 20+ messages in thread From: Linus Walleij @ 2018-05-16 14:38 UTC (permalink / raw) To: Michal Simek Cc: Alexander Graf, Bjorn Andersson, Rob Herring, linux-kernel, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, Greg Kroah-Hartman, Grant Likely, Mark Brown, Stephen Boyd, boot-architecture, Linux ARM, Jan Kiszka On Mon, May 14, 2018 at 2:44 PM, Michal Simek <michal.simek@xilinx.com> wrote: > On 14.5.2018 09:37, Alexander Graf wrote: >> On 05/14/2018 12:01 AM, Linus Walleij wrote: >>> On Wed, May 9, 2018 at 11:44 AM, Alexander Graf <agraf@suse.de> wrote: >>>> On 05/07/2018 08:31 PM, Bjorn Andersson wrote: >>>>> Can you please name platform that has enough support for Alexander to >>>>> care about backwards and forwards compatibility but lacks a pinctrl >>>>> driver. >>>> ZynqMP is one example that immediately comes to my mind. I'm sure >>>> there are >>>> others too. >>> Why isn't that using drivers/pinctrl/pinctrl-zynq.c? >>> >>> How is it so very different from (old) Zynq as it is already using >>> the same GPIO driver? >> >> That one is very simple: ZynqMP is usually an AMP system where Linux >> doesn't have full knowledge of the overall system. IIUC they have a tiny >> microblaze (PMU) that does the actual full system configuration for >> peripherals that may interfere with each other. This architecture also >> allows for safety critical code to run alongside a (less safe) Linux >> system. > > Linux is running in non secure world that's why Linux can't have full > system visibility and Linux should ask firmware. It doesn't matter if > firmware is running on specific unit or just secure SW in EL3/EL1-S, EL0-S. > You can also configure ZynqMP to protect these address ranges not to be > accessible from NS sw. > If you don't care about security you can use normal read/write accesses > at least for gpio case. Pinctrl/clk will be driven via firmware interface. OK I get it, the situation is similar to some ACPI-based BIOSes on PC where one needs to ask the firmware for misc services. What would be nice is to standardize these APIs (like ACPI or device tree does) so we don't end up with one per-SoC-specific driver per system. But I guess it is not my pick. Yours, Linus Walleij ^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2018-05-16 14:38 UTC | newest] Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-05-01 21:31 [RFC PATCH] driver core: make deferring probe forever optional Rob Herring 2018-05-01 22:08 ` Greg Kroah-Hartman 2018-05-02 11:40 ` Robin Murphy 2018-05-02 14:48 ` Rob Herring 2018-05-02 18:49 ` Robin Murphy 2018-05-05 1:25 ` Mark Brown 2018-05-07 13:37 ` Rob Herring 2018-05-02 13:16 ` Alexander Graf 2018-05-07 18:31 ` Bjorn Andersson 2018-05-07 19:55 ` Rob Herring 2018-05-07 22:34 ` Bjorn Andersson 2018-05-09 9:18 ` Mark Brown 2018-05-09 9:57 ` Alexander Graf 2018-05-09 22:34 ` Rob Herring 2018-05-09 22:30 ` Rob Herring 2018-05-09 9:44 ` Alexander Graf 2018-05-13 22:01 ` Linus Walleij 2018-05-14 7:37 ` Alexander Graf 2018-05-14 12:44 ` Michal Simek 2018-05-16 14:38 ` Linus Walleij
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).