[PATCH v2] gpio: Get rid of gpio_hog_probe_all()

[Marek Vasut]

On 9/22/22 17:33, Quentin Schulz wrote:
> Hi Marek,

Hi,

[...]

>>> +                /*
>>> +                 * Since gpio-hog is a U_BOOT_DRIVER and not
>>> +                 * a U_BOOT_CLASS, the DM core does not bind
>>> +                 * it and therefore it's up to this driver to
>>> +                 * set the DM_FLAG_PRE_RELOC appropriately.
>>> +                 */
>>> +                if (ofnode_pre_reloc(node))
>>> +                    dev_or_flags(child, DM_FLAG_PRE_RELOC);
>>
>>
>> This second part should be handled by the DM, or you need dm-pre-reloc 
>> in your GPIO controller in DT. This would fail e.g. in case your GPIO 
>> controller has higher depth of hog subnodes, like:
>>
>> gpio-controller {
> 
> You need u-boot,dm-pre-reloc here otherwise the gpio controller device 
> tree node content won't be part of the SPL Device Tree. e.g. I get:
> 
>                  gpio at ff260000 {
> 
>                          bios_disable_override {
>                                  gpios = <0x0d 0x01>;
>                                  output-high;
>                                  line-name = "bios_disable_override";
>                                  gpio-hog;
>                          };
>                  };
> 
> in the SPL if I don't have u-boot,dm-pre-reloc for gpio2 (gpio at ff260000).
> 
> Obviously no driver will be bound to gpio at ff260000.
> 
> If I have the property:
> 
>                  gpio at ff260000 {
>                          compatible = "rockchip,gpio-bank";
>                          reg = <0x00 0xff260000 0x00 0x100>;
>                          interrupts = <0x00 0x05 0x04>;
>                          clocks = <0x02 0x15d>;
>                          gpio-controller;
>                          #gpio-cells = <0x02>;
>                          interrupt-controller;
>                          #interrupt-cells = <0x02>;
>                          phandle = <0x6c>;
> 
>                          bios_disable_override {
>                                  gpios = <0x0d 0x01>;
>                                  output-high;
>                                  line-name = "bios_disable_override";
>                                  gpio-hog;
>                          };
>                  };
> 
> But I'm not sure this is what you wanted to highlight? Can you rephrase 
> please?

Nevermind, I think the DM core patch solves the problem I thought you 
had in DT already.

[...]

>>
>> At some point, I had the idea to instead of littering the DT with 
>> u-boot,dm-pre-reloc , we could use phandles instead and do something 
>> like:
>>
>> / {
>>   config {
>>    u-boot,dm-pre-reloc = <&node1 &node2 ... &gpio_hog ...>;
> 
> I don't think that's a good idea for inheritance.
> 
> E.g. today we have px30-u-boot.dtsi with u-boot,dm-pre-reloc for many 
> nodes. If I want to add more nodes to it, I'd have to override it and 
> then I need to specify all that are in px30-u-boot.dtsi already. This is 
> not good because then a change made to  u-boot,dm-pre-reloc in 
> px30-u-boot.dtsi wouldn't be propagated to my board unless I update the 
> property in my "end" device-tree.

That part could be solved by some sort of syntax sugar in DTC I think, 
some /append-node/ or so.

> Also, the Device Tree is supposed to represent the hardware and I don't 
> feel like specifying the devices to have available in TPL/SPL in Device 
> Tree is correct. It is somewhat user-friendly though compared to a 
> defconfig or constant in an include file. I don't have anything to 
> suggest at the moment though :/

The /config {} node is a bit special in that aspect, although I am not 
entirely sure whether it is even part of the DT spec. /chosen I think is.

[...]