| Message ID | 20220912175513.4178793-1-nate.d@variscite.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | gpio: uclass: Introduce gpio-hog-optional property | expand |
On Mon, 12 Sept 2022 at 11:55, Nate Drude <nate.d@variscite.com> wrote: > > gpio_hog_probe_all is invoked by init_sequence_r in board_r.c. > If device_probe fails for any gpio-hog, boot hangs with the following error: > > > initcall sequence 00000000fffc8e18 failed at call 000000004023b320 (err=-121) > > ### ERROR ### Please RESET the board ### > > gpio-hog-optional allows the boot sequence to continue if device_probe > fails for optional gpio-hog(s). > > Signed-off-by: Nate Drude <nate.d@variscite.com> > --- > doc/device-tree-bindings/gpio/gpio.txt | 1 + > drivers/gpio/gpio-uclass.c | 4 +++- > 2 files changed, 4 insertions(+), 1 deletion(-) Reviewed-by: Simon Glass <sjg@chromium.org> nit below > > diff --git a/doc/device-tree-bindings/gpio/gpio.txt b/doc/device-tree-bindings/gpio/gpio.txt > index 1481ed607d..02d296316b 100644 > --- a/doc/device-tree-bindings/gpio/gpio.txt > +++ b/doc/device-tree-bindings/gpio/gpio.txt > @@ -189,6 +189,7 @@ gpio-controller's driver probe function. > Each GPIO hog definition is represented as a child node of the GPIO controller. > Required properties: > - gpio-hog: A property specifying that this child node represents a GPIO hog. > +- gpio-hog-optional: A property specifying to continue boot when device_probe fails in gpio_hog_probe_all > - gpios: Store the GPIO information (id, flags, ...) for each GPIO to > affect. Shall contain an integer multiple of the number of cells > specified in its parent node (GPIO controller node). > diff --git a/drivers/gpio/gpio-uclass.c b/drivers/gpio/gpio-uclass.c > index 0ed32b7217..7ef9f4abc8 100644 > --- a/drivers/gpio/gpio-uclass.c > +++ b/drivers/gpio/gpio-uclass.c > @@ -329,7 +329,9 @@ int gpio_hog_probe_all(void) > if (ret) { > printf("Failed to probe device %s err: %d\n", > dev->name, ret); > - retval = ret; > + if (!dev_read_bool(dev, "gpio-hog-optional")) { > + retval = ret; > + } Should not have {} for single-line statements > } > } > } > -- > 2.37.3 > Regards, Simon
Hi Nate and Simon, On Mon, Sep 12, 2022 at 2:55 PM Nate Drude <nate.d@variscite.com> wrote: > > gpio_hog_probe_all is invoked by init_sequence_r in board_r.c. > If device_probe fails for any gpio-hog, boot hangs with the following error: > > > initcall sequence 00000000fffc8e18 failed at call 000000004023b320 (err=-121) > > ### ERROR ### Please RESET the board ### > > gpio-hog-optional allows the boot sequence to continue if device_probe > fails for optional gpio-hog(s). > > Signed-off-by: Nate Drude <nate.d@variscite.com> > --- > doc/device-tree-bindings/gpio/gpio.txt | 1 + > drivers/gpio/gpio-uclass.c | 4 +++- > 2 files changed, 4 insertions(+), 1 deletion(-) > > diff --git a/doc/device-tree-bindings/gpio/gpio.txt b/doc/device-tree-bindings/gpio/gpio.txt > index 1481ed607d..02d296316b 100644 > --- a/doc/device-tree-bindings/gpio/gpio.txt > +++ b/doc/device-tree-bindings/gpio/gpio.txt > @@ -189,6 +189,7 @@ gpio-controller's driver probe function. > Each GPIO hog definition is represented as a child node of the GPIO controller. > Required properties: > - gpio-hog: A property specifying that this child node represents a GPIO hog. > +- gpio-hog-optional: A property specifying to continue boot when device_probe fails in gpio_hog_probe_all gpio-hog-optional property does not exist in Linux. If this property is introduced then U-Boot and Linux devicetrees will not be in sync. Can this be fixed differently?
Hi, On Mon, 12 Sept 2022 at 12:48, Fabio Estevam <festevam@gmail.com> wrote: > > Hi Nate and Simon, > > On Mon, Sep 12, 2022 at 2:55 PM Nate Drude <nate.d@variscite.com> wrote: > > > > gpio_hog_probe_all is invoked by init_sequence_r in board_r.c. > > If device_probe fails for any gpio-hog, boot hangs with the following error: > > > > > initcall sequence 00000000fffc8e18 failed at call 000000004023b320 (err=-121) > > > ### ERROR ### Please RESET the board ### > > > > gpio-hog-optional allows the boot sequence to continue if device_probe > > fails for optional gpio-hog(s). > > > > Signed-off-by: Nate Drude <nate.d@variscite.com> > > --- > > doc/device-tree-bindings/gpio/gpio.txt | 1 + > > drivers/gpio/gpio-uclass.c | 4 +++- > > 2 files changed, 4 insertions(+), 1 deletion(-) > > > > diff --git a/doc/device-tree-bindings/gpio/gpio.txt b/doc/device-tree-bindings/gpio/gpio.txt > > index 1481ed607d..02d296316b 100644 > > --- a/doc/device-tree-bindings/gpio/gpio.txt > > +++ b/doc/device-tree-bindings/gpio/gpio.txt > > @@ -189,6 +189,7 @@ gpio-controller's driver probe function. > > Each GPIO hog definition is represented as a child node of the GPIO controller. > > Required properties: > > - gpio-hog: A property specifying that this child node represents a GPIO hog. > > +- gpio-hog-optional: A property specifying to continue boot when device_probe fails in gpio_hog_probe_all > > gpio-hog-optional property does not exist in Linux. > > If this property is introduced then U-Boot and Linux devicetrees will > not be in sync. > > Can this be fixed differently? Nate, can you send a patch to Linux with the binding update? Regards, Simon
Hi Simon and Fabio, On 9/12/22 3:16 PM, Simon Glass wrote: > Hi, > > On Mon, 12 Sept 2022 at 12:48, Fabio Estevam <festevam@gmail.com> wrote: >> >> Hi Nate and Simon, >> >> On Mon, Sep 12, 2022 at 2:55 PM Nate Drude <nate.d@variscite.com> wrote: >>> >>> gpio_hog_probe_all is invoked by init_sequence_r in board_r.c. >>> If device_probe fails for any gpio-hog, boot hangs with the following error: >>> >>>> initcall sequence 00000000fffc8e18 failed at call 000000004023b320 (err=-121) >>>> ### ERROR ### Please RESET the board ### >>> >>> gpio-hog-optional allows the boot sequence to continue if device_probe >>> fails for optional gpio-hog(s). >>> >>> Signed-off-by: Nate Drude <nate.d@variscite.com> >>> --- >>> doc/device-tree-bindings/gpio/gpio.txt | 1 + >>> drivers/gpio/gpio-uclass.c | 4 +++- >>> 2 files changed, 4 insertions(+), 1 deletion(-) >>> >>> diff --git a/doc/device-tree-bindings/gpio/gpio.txt b/doc/device-tree-bindings/gpio/gpio.txt >>> index 1481ed607d..02d296316b 100644 >>> --- a/doc/device-tree-bindings/gpio/gpio.txt >>> +++ b/doc/device-tree-bindings/gpio/gpio.txt >>> @@ -189,6 +189,7 @@ gpio-controller's driver probe function. >>> Each GPIO hog definition is represented as a child node of the GPIO controller. >>> Required properties: >>> - gpio-hog: A property specifying that this child node represents a GPIO hog. >>> +- gpio-hog-optional: A property specifying to continue boot when device_probe fails in gpio_hog_probe_all >> >> gpio-hog-optional property does not exist in Linux. >> >> If this property is introduced then U-Boot and Linux devicetrees will >> not be in sync. >> >> Can this be fixed differently? > > Nate, can you send a patch to Linux with the binding update? > > Regards, > Simon Thanks for your responses and feedback. I don't think gpio-hog-optional is relevant to Linux. The problem is if gpio_hog_probe_all returns an error, board_init_r will hang() See: - https://github.com/u-boot/u-boot/blob/v2022.07/common/board_r.c#L824-L825 - https://github.com/u-boot/u-boot/blob/v2022.07/common/board_r.c#L763 - https://github.com/u-boot/u-boot/blob/v2022.07/drivers/gpio/gpio-uclass.c#L330-L332 A practical example of how this may occur is when an i2c gpio expander (e.g. nxp,pca9534) uses gpio-hog, but the gpio expander is depopulated. Arguably, the best solution is to use a different device tree when the gpio expander is not populated. This patch allows the gpio-hog to fail gracefully and continue booting if gpio-hog-optional is used. Do you have any suggestions for a better approach? Does it make sense for gpio_hog_probe_all to cause a fatal error when the gpio hog probe fails (most devices, including the gpio-expander, will not cause a hang if they fail to probe)? Regards, Nate
Hi Nate, On Mon, 12 Sept 2022 at 14:57, Nate Drude <nate.d@variscite.com> wrote: > > Hi Simon and Fabio, > > On 9/12/22 3:16 PM, Simon Glass wrote: > > Hi, > > > > On Mon, 12 Sept 2022 at 12:48, Fabio Estevam <festevam@gmail.com> wrote: > >> > >> Hi Nate and Simon, > >> > >> On Mon, Sep 12, 2022 at 2:55 PM Nate Drude <nate.d@variscite.com> wrote: > >>> > >>> gpio_hog_probe_all is invoked by init_sequence_r in board_r.c. > >>> If device_probe fails for any gpio-hog, boot hangs with the following error: > >>> > >>>> initcall sequence 00000000fffc8e18 failed at call 000000004023b320 (err=-121) > >>>> ### ERROR ### Please RESET the board ### > >>> > >>> gpio-hog-optional allows the boot sequence to continue if device_probe > >>> fails for optional gpio-hog(s). > >>> > >>> Signed-off-by: Nate Drude <nate.d@variscite.com> > >>> --- > >>> doc/device-tree-bindings/gpio/gpio.txt | 1 + > >>> drivers/gpio/gpio-uclass.c | 4 +++- > >>> 2 files changed, 4 insertions(+), 1 deletion(-) > >>> > >>> diff --git a/doc/device-tree-bindings/gpio/gpio.txt b/doc/device-tree-bindings/gpio/gpio.txt > >>> index 1481ed607d..02d296316b 100644 > >>> --- a/doc/device-tree-bindings/gpio/gpio.txt > >>> +++ b/doc/device-tree-bindings/gpio/gpio.txt > >>> @@ -189,6 +189,7 @@ gpio-controller's driver probe function. > >>> Each GPIO hog definition is represented as a child node of the GPIO controller. > >>> Required properties: > >>> - gpio-hog: A property specifying that this child node represents a GPIO hog. > >>> +- gpio-hog-optional: A property specifying to continue boot when device_probe fails in gpio_hog_probe_all > >> > >> gpio-hog-optional property does not exist in Linux. > >> > >> If this property is introduced then U-Boot and Linux devicetrees will > >> not be in sync. > >> > >> Can this be fixed differently? > > > > Nate, can you send a patch to Linux with the binding update? > > > > Regards, > > Simon > > Thanks for your responses and feedback. > > I don't think gpio-hog-optional is relevant to Linux. Sure, but Linux is (for better or worse) the main repo for the device tree bindings. > > The problem is if gpio_hog_probe_all returns an error, board_init_r will > hang() > > See: > - https://github.com/u-boot/u-boot/blob/v2022.07/common/board_r.c#L824-L825 > - https://github.com/u-boot/u-boot/blob/v2022.07/common/board_r.c#L763 > - > https://github.com/u-boot/u-boot/blob/v2022.07/drivers/gpio/gpio-uclass.c#L330-L332 > > A practical example of how this may occur is when an i2c gpio expander > (e.g. nxp,pca9534) uses gpio-hog, but the gpio expander is depopulated. > > Arguably, the best solution is to use a different device tree when the > gpio expander is not populated. This patch allows the gpio-hog to fail > gracefully and continue booting if gpio-hog-optional is used. > > Do you have any suggestions for a better approach? Does it make sense > for gpio_hog_probe_all to cause a fatal error when the gpio hog probe > fails (most devices, including the gpio-expander, will not cause a hang > if they fail to probe)? I think your approach is fine as is. Regards, Simon
Hi Simon, On 9/14/22 7:49 AM, Simon Glass wrote: > Hi Nate, > > On Mon, 12 Sept 2022 at 14:57, Nate Drude <nate.d@variscite.com> wrote: >> >> Hi Simon and Fabio, >> >> On 9/12/22 3:16 PM, Simon Glass wrote: >>> Hi, >>> >>> On Mon, 12 Sept 2022 at 12:48, Fabio Estevam <festevam@gmail.com> wrote: >>>> >>>> Hi Nate and Simon, >>>> >>>> On Mon, Sep 12, 2022 at 2:55 PM Nate Drude <nate.d@variscite.com> wrote: >>>>> >>>>> gpio_hog_probe_all is invoked by init_sequence_r in board_r.c. >>>>> If device_probe fails for any gpio-hog, boot hangs with the following error: >>>>> >>>>>> initcall sequence 00000000fffc8e18 failed at call 000000004023b320 (err=-121) >>>>>> ### ERROR ### Please RESET the board ### >>>>> >>>>> gpio-hog-optional allows the boot sequence to continue if device_probe >>>>> fails for optional gpio-hog(s). >>>>> >>>>> Signed-off-by: Nate Drude <nate.d@variscite.com> >>>>> --- >>>>> doc/device-tree-bindings/gpio/gpio.txt | 1 + >>>>> drivers/gpio/gpio-uclass.c | 4 +++- >>>>> 2 files changed, 4 insertions(+), 1 deletion(-) >>>>> >>>>> diff --git a/doc/device-tree-bindings/gpio/gpio.txt b/doc/device-tree-bindings/gpio/gpio.txt >>>>> index 1481ed607d..02d296316b 100644 >>>>> --- a/doc/device-tree-bindings/gpio/gpio.txt >>>>> +++ b/doc/device-tree-bindings/gpio/gpio.txt >>>>> @@ -189,6 +189,7 @@ gpio-controller's driver probe function. >>>>> Each GPIO hog definition is represented as a child node of the GPIO controller. >>>>> Required properties: >>>>> - gpio-hog: A property specifying that this child node represents a GPIO hog. >>>>> +- gpio-hog-optional: A property specifying to continue boot when device_probe fails in gpio_hog_probe_all >>>> >>>> gpio-hog-optional property does not exist in Linux. >>>> >>>> If this property is introduced then U-Boot and Linux devicetrees will >>>> not be in sync. >>>> >>>> Can this be fixed differently? >>> >>> Nate, can you send a patch to Linux with the binding update? >>> >>> Regards, >>> Simon >> >> Thanks for your responses and feedback. >> >> I don't think gpio-hog-optional is relevant to Linux. > > Sure, but Linux is (for better or worse) the main repo for the device > tree bindings. I am not understanding the action. I think you're suggesting I update the Linux device tree bindings so they stay aligned with U-Boot, adding a property gpio-hog-optional after this line: https://github.com/torvalds/linux/blob/v6.0-rc5/Documentation/devicetree/bindings/gpio/gpio.txt#L191 However, since it's not relevant to Linux, I think it will be confusing since it will have no effect and won't be be used in any Linux code. Can you please advise what description I should use for the gpio-hog-optional property so that the Linux maintainers would accept such a patch? > >> >> The problem is if gpio_hog_probe_all returns an error, board_init_r will >> hang() >> >> See: >> - https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fu-boot%2Fu-boot%2Fblob%2Fv2022.07%2Fcommon%2Fboard_r.c%23L824-L825&data=05%7C01%7Cnate.d%40variscite.com%7C105f793941264d50bdad08da964f8a23%7C399ae6ac38f44ef094a8440b0ad581de%7C1%7C0%7C637987565614703613%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=wU%2Fz3uWE%2Bqj6jKzLeCx61Gfvyln7z60jRmONTN5Q46o%3D&reserved=0 >> - https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fu-boot%2Fu-boot%2Fblob%2Fv2022.07%2Fcommon%2Fboard_r.c%23L763&data=05%7C01%7Cnate.d%40variscite.com%7C105f793941264d50bdad08da964f8a23%7C399ae6ac38f44ef094a8440b0ad581de%7C1%7C0%7C637987565614703613%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=b%2BtvcG21zH4WZqP39DFs%2FylUD9UOZp6tRZ%2BmeUwL2lc%3D&reserved=0 >> - >> https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fu-boot%2Fu-boot%2Fblob%2Fv2022.07%2Fdrivers%2Fgpio%2Fgpio-uclass.c%23L330-L332&data=05%7C01%7Cnate.d%40variscite.com%7C105f793941264d50bdad08da964f8a23%7C399ae6ac38f44ef094a8440b0ad581de%7C1%7C0%7C637987565614859942%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=FwqsboWzGmSdUHrFUL48AwfTDbAPFpUmKbaQJEnprq0%3D&reserved=0 >> >> A practical example of how this may occur is when an i2c gpio expander >> (e.g. nxp,pca9534) uses gpio-hog, but the gpio expander is depopulated. >> >> Arguably, the best solution is to use a different device tree when the >> gpio expander is not populated. This patch allows the gpio-hog to fail >> gracefully and continue booting if gpio-hog-optional is used. >> >> Do you have any suggestions for a better approach? Does it make sense >> for gpio_hog_probe_all to cause a fatal error when the gpio hog probe >> fails (most devices, including the gpio-expander, will not cause a hang >> if they fail to probe)? > > I think your approach is fine as is. An alternate approach is to modify the default behavior so that gpio_hog_probe_all will not trigger a fatal error. Do you think this is better? > > Regards, > Simon Thanks, Nate
Hi Nate, On Wed, Sep 14, 2022 at 11:00 AM Nate Drude <nate.d@variscite.com> wrote: > An alternate approach is to modify the default behavior so that > gpio_hog_probe_all will not trigger a fatal error. Do you think this is > better? IMHO that would be better, as we could keep using the same devicetree files from Linux. Thanks
On Wed, Sep 14, 2022 at 08:59:52AM -0500, Nate Drude wrote: > Hi Simon, > > On 9/14/22 7:49 AM, Simon Glass wrote: > > Hi Nate, > > > > On Mon, 12 Sept 2022 at 14:57, Nate Drude <nate.d@variscite.com> wrote: > > > > > > Hi Simon and Fabio, > > > > > > On 9/12/22 3:16 PM, Simon Glass wrote: > > > > Hi, > > > > > > > > On Mon, 12 Sept 2022 at 12:48, Fabio Estevam <festevam@gmail.com> wrote: > > > > > > > > > > Hi Nate and Simon, > > > > > > > > > > On Mon, Sep 12, 2022 at 2:55 PM Nate Drude <nate.d@variscite.com> wrote: > > > > > > > > > > > > gpio_hog_probe_all is invoked by init_sequence_r in board_r.c. > > > > > > If device_probe fails for any gpio-hog, boot hangs with the following error: > > > > > > > > > > > > > initcall sequence 00000000fffc8e18 failed at call 000000004023b320 (err=-121) > > > > > > > ### ERROR ### Please RESET the board ### > > > > > > > > > > > > gpio-hog-optional allows the boot sequence to continue if device_probe > > > > > > fails for optional gpio-hog(s). > > > > > > > > > > > > Signed-off-by: Nate Drude <nate.d@variscite.com> > > > > > > --- > > > > > > doc/device-tree-bindings/gpio/gpio.txt | 1 + > > > > > > drivers/gpio/gpio-uclass.c | 4 +++- > > > > > > 2 files changed, 4 insertions(+), 1 deletion(-) > > > > > > > > > > > > diff --git a/doc/device-tree-bindings/gpio/gpio.txt b/doc/device-tree-bindings/gpio/gpio.txt > > > > > > index 1481ed607d..02d296316b 100644 > > > > > > --- a/doc/device-tree-bindings/gpio/gpio.txt > > > > > > +++ b/doc/device-tree-bindings/gpio/gpio.txt > > > > > > @@ -189,6 +189,7 @@ gpio-controller's driver probe function. > > > > > > Each GPIO hog definition is represented as a child node of the GPIO controller. > > > > > > Required properties: > > > > > > - gpio-hog: A property specifying that this child node represents a GPIO hog. > > > > > > +- gpio-hog-optional: A property specifying to continue boot when device_probe fails in gpio_hog_probe_all > > > > > > > > > > gpio-hog-optional property does not exist in Linux. > > > > > > > > > > If this property is introduced then U-Boot and Linux devicetrees will > > > > > not be in sync. > > > > > > > > > > Can this be fixed differently? > > > > > > > > Nate, can you send a patch to Linux with the binding update? > > > > > > > > Regards, > > > > Simon > > > > > > Thanks for your responses and feedback. > > > > > > I don't think gpio-hog-optional is relevant to Linux. > > > > Sure, but Linux is (for better or worse) the main repo for the device > > tree bindings. > > I am not understanding the action. I think you're suggesting I update the > Linux device tree bindings so they stay aligned with U-Boot, adding a > property gpio-hog-optional after this line: https://github.com/torvalds/linux/blob/v6.0-rc5/Documentation/devicetree/bindings/gpio/gpio.txt#L191 > > However, since it's not relevant to Linux, I think it will be confusing > since it will have no effect and won't be be used in any Linux code. > > Can you please advise what description I should use for the > gpio-hog-optional property so that the Linux maintainers would accept such a > patch? Yes, that would be one way to go about this, and there are other non-Linux bindings in the Linux kernel tree, but this might be the first property of an existing binding, so that might also be a bit challenging to get accepted, or find out what the preferred solution is. [snip] > > > Do you have any suggestions for a better approach? Does it make sense > > > for gpio_hog_probe_all to cause a fatal error when the gpio hog probe > > > fails (most devices, including the gpio-expander, will not cause a hang > > > if they fail to probe)? > > > > I think your approach is fine as is. > > An alternate approach is to modify the default behavior so that > gpio_hog_probe_all will not trigger a fatal error. Do you think this is > better? This too would be acceptable.
Hi All, On 9/14/22 9:16 AM, Tom Rini wrote: > On Wed, Sep 14, 2022 at 08:59:52AM -0500, Nate Drude wrote: >> Hi Simon, >> >> On 9/14/22 7:49 AM, Simon Glass wrote: >>> Hi Nate, >>> >>> On Mon, 12 Sept 2022 at 14:57, Nate Drude <nate.d@variscite.com> wrote: >>>> >>>> Hi Simon and Fabio, >>>> >>>> On 9/12/22 3:16 PM, Simon Glass wrote: >>>>> Hi, >>>>> >>>>> On Mon, 12 Sept 2022 at 12:48, Fabio Estevam <festevam@gmail.com> wrote: >>>>>> >>>>>> Hi Nate and Simon, >>>>>> >>>>>> On Mon, Sep 12, 2022 at 2:55 PM Nate Drude <nate.d@variscite.com> wrote: >>>>>>> >>>>>>> gpio_hog_probe_all is invoked by init_sequence_r in board_r.c. >>>>>>> If device_probe fails for any gpio-hog, boot hangs with the following error: >>>>>>> >>>>>>>> initcall sequence 00000000fffc8e18 failed at call 000000004023b320 (err=-121) >>>>>>>> ### ERROR ### Please RESET the board ### >>>>>>> >>>>>>> gpio-hog-optional allows the boot sequence to continue if device_probe >>>>>>> fails for optional gpio-hog(s). >>>>>>> >>>>>>> Signed-off-by: Nate Drude <nate.d@variscite.com> >>>>>>> --- >>>>>>> doc/device-tree-bindings/gpio/gpio.txt | 1 + >>>>>>> drivers/gpio/gpio-uclass.c | 4 +++- >>>>>>> 2 files changed, 4 insertions(+), 1 deletion(-) >>>>>>> >>>>>>> diff --git a/doc/device-tree-bindings/gpio/gpio.txt b/doc/device-tree-bindings/gpio/gpio.txt >>>>>>> index 1481ed607d..02d296316b 100644 >>>>>>> --- a/doc/device-tree-bindings/gpio/gpio.txt >>>>>>> +++ b/doc/device-tree-bindings/gpio/gpio.txt >>>>>>> @@ -189,6 +189,7 @@ gpio-controller's driver probe function. >>>>>>> Each GPIO hog definition is represented as a child node of the GPIO controller. >>>>>>> Required properties: >>>>>>> - gpio-hog: A property specifying that this child node represents a GPIO hog. >>>>>>> +- gpio-hog-optional: A property specifying to continue boot when device_probe fails in gpio_hog_probe_all >>>>>> >>>>>> gpio-hog-optional property does not exist in Linux. >>>>>> >>>>>> If this property is introduced then U-Boot and Linux devicetrees will >>>>>> not be in sync. >>>>>> >>>>>> Can this be fixed differently? >>>>> >>>>> Nate, can you send a patch to Linux with the binding update? >>>>> >>>>> Regards, >>>>> Simon >>>> >>>> Thanks for your responses and feedback. >>>> >>>> I don't think gpio-hog-optional is relevant to Linux. >>> >>> Sure, but Linux is (for better or worse) the main repo for the device >>> tree bindings. >> >> I am not understanding the action. I think you're suggesting I update the >> Linux device tree bindings so they stay aligned with U-Boot, adding a >> property gpio-hog-optional after this line: https://github.com/torvalds/linux/blob/v6.0-rc5/Documentation/devicetree/bindings/gpio/gpio.txt#L191 >> >> However, since it's not relevant to Linux, I think it will be confusing >> since it will have no effect and won't be be used in any Linux code. >> >> Can you please advise what description I should use for the >> gpio-hog-optional property so that the Linux maintainers would accept such a >> patch? > > Yes, that would be one way to go about this, and there are other > non-Linux bindings in the Linux kernel tree, but this might be the first > property of an existing binding, so that might also be a bit challenging > to get accepted, or find out what the preferred solution is. > > [snip] >>>> Do you have any suggestions for a better approach? Does it make sense >>>> for gpio_hog_probe_all to cause a fatal error when the gpio hog probe >>>> fails (most devices, including the gpio-expander, will not cause a hang >>>> if they fail to probe)? >>> >>> I think your approach is fine as is. >> >> An alternate approach is to modify the default behavior so that >> gpio_hog_probe_all will not trigger a fatal error. Do you think this is >> better? > > This too would be acceptable. > Thanks for the discussion and feedback. I prefer to avoid changing the bindings in Linux if possible. Would it be acceptable if I rework gpio_hog_probe_all so that it prints an error "Failed to probe device..." if any device_probe fails, but always returns 0 to avoid a fatal error in board_init_r? I can submit a v2 patch, but I want to screen any problems with the approach first. Thanks, Nate
Hi, On Wed, 14 Sept 2022 at 08:31, Nate Drude <nate.d@variscite.com> wrote: > > Hi All, > > On 9/14/22 9:16 AM, Tom Rini wrote: > > On Wed, Sep 14, 2022 at 08:59:52AM -0500, Nate Drude wrote: > >> Hi Simon, > >> > >> On 9/14/22 7:49 AM, Simon Glass wrote: > >>> Hi Nate, > >>> > >>> On Mon, 12 Sept 2022 at 14:57, Nate Drude <nate.d@variscite.com> wrote: > >>>> > >>>> Hi Simon and Fabio, > >>>> > >>>> On 9/12/22 3:16 PM, Simon Glass wrote: > >>>>> Hi, > >>>>> > >>>>> On Mon, 12 Sept 2022 at 12:48, Fabio Estevam <festevam@gmail.com> wrote: > >>>>>> > >>>>>> Hi Nate and Simon, > >>>>>> > >>>>>> On Mon, Sep 12, 2022 at 2:55 PM Nate Drude <nate.d@variscite.com> wrote: > >>>>>>> > >>>>>>> gpio_hog_probe_all is invoked by init_sequence_r in board_r.c. > >>>>>>> If device_probe fails for any gpio-hog, boot hangs with the following error: > >>>>>>> > >>>>>>>> initcall sequence 00000000fffc8e18 failed at call 000000004023b320 (err=-121) > >>>>>>>> ### ERROR ### Please RESET the board ### > >>>>>>> > >>>>>>> gpio-hog-optional allows the boot sequence to continue if device_probe > >>>>>>> fails for optional gpio-hog(s). > >>>>>>> > >>>>>>> Signed-off-by: Nate Drude <nate.d@variscite.com> > >>>>>>> --- > >>>>>>> doc/device-tree-bindings/gpio/gpio.txt | 1 + > >>>>>>> drivers/gpio/gpio-uclass.c | 4 +++- > >>>>>>> 2 files changed, 4 insertions(+), 1 deletion(-) > >>>>>>> > >>>>>>> diff --git a/doc/device-tree-bindings/gpio/gpio.txt b/doc/device-tree-bindings/gpio/gpio.txt > >>>>>>> index 1481ed607d..02d296316b 100644 > >>>>>>> --- a/doc/device-tree-bindings/gpio/gpio.txt > >>>>>>> +++ b/doc/device-tree-bindings/gpio/gpio.txt > >>>>>>> @@ -189,6 +189,7 @@ gpio-controller's driver probe function. > >>>>>>> Each GPIO hog definition is represented as a child node of the GPIO controller. > >>>>>>> Required properties: > >>>>>>> - gpio-hog: A property specifying that this child node represents a GPIO hog. > >>>>>>> +- gpio-hog-optional: A property specifying to continue boot when device_probe fails in gpio_hog_probe_all > >>>>>> > >>>>>> gpio-hog-optional property does not exist in Linux. > >>>>>> > >>>>>> If this property is introduced then U-Boot and Linux devicetrees will > >>>>>> not be in sync. > >>>>>> > >>>>>> Can this be fixed differently? > >>>>> > >>>>> Nate, can you send a patch to Linux with the binding update? > >>>>> > >>>>> Regards, > >>>>> Simon > >>>> > >>>> Thanks for your responses and feedback. > >>>> > >>>> I don't think gpio-hog-optional is relevant to Linux. > >>> > >>> Sure, but Linux is (for better or worse) the main repo for the device > >>> tree bindings. > >> > >> I am not understanding the action. I think you're suggesting I update the > >> Linux device tree bindings so they stay aligned with U-Boot, adding a > >> property gpio-hog-optional after this line: https://github.com/torvalds/linux/blob/v6.0-rc5/Documentation/devicetree/bindings/gpio/gpio.txt#L191 > >> > >> However, since it's not relevant to Linux, I think it will be confusing > >> since it will have no effect and won't be be used in any Linux code. > >> > >> Can you please advise what description I should use for the > >> gpio-hog-optional property so that the Linux maintainers would accept such a > >> patch? You can mention why it is needed and use a u-boot, property-name prefix perhaps. > > > > Yes, that would be one way to go about this, and there are other > > non-Linux bindings in the Linux kernel tree, but this might be the first > > property of an existing binding, so that might also be a bit challenging > > to get accepted, or find out what the preferred solution is. > > > > [snip] > >>>> Do you have any suggestions for a better approach? Does it make sense > >>>> for gpio_hog_probe_all to cause a fatal error when the gpio hog probe > >>>> fails (most devices, including the gpio-expander, will not cause a hang > >>>> if they fail to probe)? > >>> > >>> I think your approach is fine as is. > >> > >> An alternate approach is to modify the default behavior so that > >> gpio_hog_probe_all will not trigger a fatal error. Do you think this is > >> better? > > > > This too would be acceptable. I don't like this as it introduces undefined behaviour and may cause the boot to fail with no indication of what has happened, nor any way for higher-level code to resolve the issue. Anything we do here should be deterministic. Failures should be ignored only when the board vendor wants that. > > > > Thanks for the discussion and feedback. I prefer to avoid changing the > bindings in Linux if possible. > > Would it be acceptable if I rework gpio_hog_probe_all so that it prints > an error "Failed to probe device..." if any device_probe fails, but > always returns 0 to avoid a fatal error in board_init_r? I can submit a > v2 patch, but I want to screen any problems with the approach first. How would this be controlled? One option might be to add a binding to the /options node. I don't like it, but it could be a fallback if you get push-back on this binding. Regards, Simon
Hi Nate, On Wed, Sep 14, 2022 at 11:31 AM Nate Drude <nate.d@variscite.com> wrote: > Thanks for the discussion and feedback. I prefer to avoid changing the > bindings in Linux if possible. > > Would it be acceptable if I rework gpio_hog_probe_all so that it prints > an error "Failed to probe device..." if any device_probe fails, but > always returns 0 to avoid a fatal error in board_init_r? I can submit a > v2 patch, but I want to screen any problems with the approach first. Does the patch from Marek help? https://patchwork.ozlabs.org/project/uboot/patch/20220919194502.106252-1-marex@denx.de/
diff --git a/doc/device-tree-bindings/gpio/gpio.txt b/doc/device-tree-bindings/gpio/gpio.txt index 1481ed607d..02d296316b 100644 --- a/doc/device-tree-bindings/gpio/gpio.txt +++ b/doc/device-tree-bindings/gpio/gpio.txt @@ -189,6 +189,7 @@ gpio-controller's driver probe function. Each GPIO hog definition is represented as a child node of the GPIO controller. Required properties: - gpio-hog: A property specifying that this child node represents a GPIO hog. +- gpio-hog-optional: A property specifying to continue boot when device_probe fails in gpio_hog_probe_all - gpios: Store the GPIO information (id, flags, ...) for each GPIO to affect. Shall contain an integer multiple of the number of cells specified in its parent node (GPIO controller node). diff --git a/drivers/gpio/gpio-uclass.c b/drivers/gpio/gpio-uclass.c index 0ed32b7217..7ef9f4abc8 100644 --- a/drivers/gpio/gpio-uclass.c +++ b/drivers/gpio/gpio-uclass.c @@ -329,7 +329,9 @@ int gpio_hog_probe_all(void) if (ret) { printf("Failed to probe device %s err: %d\n", dev->name, ret); - retval = ret; + if (!dev_read_bool(dev, "gpio-hog-optional")) { + retval = ret; + } } } }