Re: [PATCH] gpio: uclass: Introduce gpio-hog-optional property

[Nate Drude <nate.d@variscite.com>]

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&amp;data=05%7C01%7Cnate.d%40variscite.com%7C105f793941264d50bdad08da964f8a23%7C399ae6ac38f44ef094a8440b0ad581de%7C1%7C0%7C637987565614703613%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=wU%2Fz3uWE%2Bqj6jKzLeCx61Gfvyln7z60jRmONTN5Q46o%3D&amp;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&amp;data=05%7C01%7Cnate.d%40variscite.com%7C105f793941264d50bdad08da964f8a23%7C399ae6ac38f44ef094a8440b0ad581de%7C1%7C0%7C637987565614703613%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=b%2BtvcG21zH4WZqP39DFs%2FylUD9UOZp6tRZ%2BmeUwL2lc%3D&amp;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&amp;data=05%7C01%7Cnate.d%40variscite.com%7C105f793941264d50bdad08da964f8a23%7C399ae6ac38f44ef094a8440b0ad581de%7C1%7C0%7C637987565614859942%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=FwqsboWzGmSdUHrFUL48AwfTDbAPFpUmKbaQJEnprq0%3D&amp;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