* [PATCH] gpio: uclass: Introduce gpio-hog-optional property @ 2022-09-12 17:55 Nate Drude 2022-09-12 18:31 ` Simon Glass 2022-09-12 18:48 ` Fabio Estevam 0 siblings, 2 replies; 12+ messages in thread From: Nate Drude @ 2022-09-12 17:55 UTC (permalink / raw) To: Simon Glass, Samuel Holland, Heinrich Schuchardt, Andrew Jeffery, Eddie James, u-boot Cc: eran.m, Nate Drude 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 - 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; + } } } } -- 2.37.3 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH] gpio: uclass: Introduce gpio-hog-optional property 2022-09-12 17:55 [PATCH] gpio: uclass: Introduce gpio-hog-optional property Nate Drude @ 2022-09-12 18:31 ` Simon Glass 2022-09-12 18:48 ` Fabio Estevam 1 sibling, 0 replies; 12+ messages in thread From: Simon Glass @ 2022-09-12 18:31 UTC (permalink / raw) To: Nate Drude Cc: Samuel Holland, Heinrich Schuchardt, Andrew Jeffery, Eddie James, U-Boot Mailing List, eran.m 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] gpio: uclass: Introduce gpio-hog-optional property 2022-09-12 17:55 [PATCH] gpio: uclass: Introduce gpio-hog-optional property Nate Drude 2022-09-12 18:31 ` Simon Glass @ 2022-09-12 18:48 ` Fabio Estevam 2022-09-12 20:16 ` Simon Glass 1 sibling, 1 reply; 12+ messages in thread From: Fabio Estevam @ 2022-09-12 18:48 UTC (permalink / raw) To: Nate Drude Cc: Simon Glass, Samuel Holland, Heinrich Schuchardt, Andrew Jeffery, Eddie James, u-boot, eran.m, Tom Rini 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? ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] gpio: uclass: Introduce gpio-hog-optional property 2022-09-12 18:48 ` Fabio Estevam @ 2022-09-12 20:16 ` Simon Glass 2022-09-12 20:56 ` Nate Drude 0 siblings, 1 reply; 12+ messages in thread From: Simon Glass @ 2022-09-12 20:16 UTC (permalink / raw) To: Fabio Estevam Cc: Nate Drude, Samuel Holland, Heinrich Schuchardt, Andrew Jeffery, Eddie James, U-Boot Mailing List, eran.m, Tom Rini 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] gpio: uclass: Introduce gpio-hog-optional property 2022-09-12 20:16 ` Simon Glass @ 2022-09-12 20:56 ` Nate Drude 2022-09-14 12:49 ` Simon Glass 0 siblings, 1 reply; 12+ messages in thread From: Nate Drude @ 2022-09-12 20:56 UTC (permalink / raw) To: Simon Glass, Fabio Estevam Cc: Samuel Holland, Heinrich Schuchardt, Andrew Jeffery, Eddie James, U-Boot Mailing List, eran.m, Tom Rini 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] gpio: uclass: Introduce gpio-hog-optional property 2022-09-12 20:56 ` Nate Drude @ 2022-09-14 12:49 ` Simon Glass 2022-09-14 13:59 ` Nate Drude 0 siblings, 1 reply; 12+ messages in thread From: Simon Glass @ 2022-09-14 12:49 UTC (permalink / raw) To: Nate Drude Cc: Fabio Estevam, Samuel Holland, Heinrich Schuchardt, Andrew Jeffery, Eddie James, U-Boot Mailing List, eran.m, Tom Rini 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] gpio: uclass: Introduce gpio-hog-optional property 2022-09-14 12:49 ` Simon Glass @ 2022-09-14 13:59 ` Nate Drude 2022-09-14 14:09 ` Fabio Estevam 2022-09-14 14:16 ` Tom Rini 0 siblings, 2 replies; 12+ messages in thread From: Nate Drude @ 2022-09-14 13:59 UTC (permalink / raw) To: Simon Glass Cc: Fabio Estevam, Samuel Holland, Heinrich Schuchardt, Andrew Jeffery, Eddie James, U-Boot Mailing List, eran.m, Tom Rini 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] gpio: uclass: Introduce gpio-hog-optional property 2022-09-14 13:59 ` Nate Drude @ 2022-09-14 14:09 ` Fabio Estevam 2022-09-14 14:16 ` Tom Rini 1 sibling, 0 replies; 12+ messages in thread From: Fabio Estevam @ 2022-09-14 14:09 UTC (permalink / raw) To: Nate Drude Cc: Simon Glass, Samuel Holland, Heinrich Schuchardt, Andrew Jeffery, Eddie James, U-Boot Mailing List, eran.m, Tom Rini 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] gpio: uclass: Introduce gpio-hog-optional property 2022-09-14 13:59 ` Nate Drude 2022-09-14 14:09 ` Fabio Estevam @ 2022-09-14 14:16 ` Tom Rini 2022-09-14 14:31 ` Nate Drude 1 sibling, 1 reply; 12+ messages in thread From: Tom Rini @ 2022-09-14 14:16 UTC (permalink / raw) To: Nate Drude Cc: Simon Glass, Fabio Estevam, Samuel Holland, Heinrich Schuchardt, Andrew Jeffery, Eddie James, U-Boot Mailing List, eran.m [-- Attachment #1: Type: text/plain, Size: 4034 bytes --] 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. -- Tom [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 659 bytes --] ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] gpio: uclass: Introduce gpio-hog-optional property 2022-09-14 14:16 ` Tom Rini @ 2022-09-14 14:31 ` Nate Drude 2022-09-14 17:09 ` Simon Glass 2022-09-20 11:43 ` Fabio Estevam 0 siblings, 2 replies; 12+ messages in thread From: Nate Drude @ 2022-09-14 14:31 UTC (permalink / raw) To: Tom Rini Cc: Simon Glass, Fabio Estevam, Samuel Holland, Heinrich Schuchardt, Andrew Jeffery, Eddie James, U-Boot Mailing List, eran.m 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] gpio: uclass: Introduce gpio-hog-optional property 2022-09-14 14:31 ` Nate Drude @ 2022-09-14 17:09 ` Simon Glass 2022-09-20 11:43 ` Fabio Estevam 1 sibling, 0 replies; 12+ messages in thread From: Simon Glass @ 2022-09-14 17:09 UTC (permalink / raw) To: Nate Drude Cc: Tom Rini, Fabio Estevam, Samuel Holland, Heinrich Schuchardt, Andrew Jeffery, Eddie James, U-Boot Mailing List, eran.m 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] gpio: uclass: Introduce gpio-hog-optional property 2022-09-14 14:31 ` Nate Drude 2022-09-14 17:09 ` Simon Glass @ 2022-09-20 11:43 ` Fabio Estevam 1 sibling, 0 replies; 12+ messages in thread From: Fabio Estevam @ 2022-09-20 11:43 UTC (permalink / raw) To: Nate Drude Cc: Tom Rini, Simon Glass, Samuel Holland, Heinrich Schuchardt, Andrew Jeffery, Eddie James, U-Boot Mailing List, eran.m 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/ ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2022-09-20 11:43 UTC | newest] Thread overview: 12+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2022-09-12 17:55 [PATCH] gpio: uclass: Introduce gpio-hog-optional property Nate Drude 2022-09-12 18:31 ` Simon Glass 2022-09-12 18:48 ` Fabio Estevam 2022-09-12 20:16 ` Simon Glass 2022-09-12 20:56 ` Nate Drude 2022-09-14 12:49 ` Simon Glass 2022-09-14 13:59 ` Nate Drude 2022-09-14 14:09 ` Fabio Estevam 2022-09-14 14:16 ` Tom Rini 2022-09-14 14:31 ` Nate Drude 2022-09-14 17:09 ` Simon Glass 2022-09-20 11:43 ` Fabio Estevam
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.