[PATCH] usb: onboard-hub: Update the bind function based on "peer_hub" property

[Marek Vasut]

On 12/3/24 6:02 PM, Michal Simek wrote:
> 
> 
> On 12/3/24 17:24, Marek Vasut wrote:
>> On 12/3/24 2:44 PM, Michal Simek wrote:
>>>
>>>
>>> On 12/3/24 05:49, Venkatesh Yadav Abbarapu wrote:
>>>> Add the bool variable "peer_hub" and set this only for the
>>>> hubs which have the "peer-hub" property in their DT nodes.
>>>> Skip the bind function for usb hubs which don't have "peer-hub"
>>>> property.
>>>>
>>>> Fixes: 57e30b09fc ("usb: onboard-hub: Bail out if peer hub is 
>>>> already probed")
>>>> Signed-off-by: Venkatesh Yadav Abbarapu <venkatesh.abbarapu at amd.com>
>>>> ---
>>>>   common/usb_onboard_hub.c | 8 ++++++++
>>>>   1 file changed, 8 insertions(+)
>>>>
>>>> diff --git a/common/usb_onboard_hub.c b/common/usb_onboard_hub.c
>>>> index 6f28036e09..0a8fac38ee 100644
>>>> --- a/common/usb_onboard_hub.c
>>>> +++ b/common/usb_onboard_hub.c
>>>> @@ -26,6 +26,7 @@ struct onboard_hub {
>>>>   struct onboard_hub_data {
>>>>       unsigned long reset_us;
>>>> +    bool peer_hub;
>>>>       unsigned long power_on_delay_us;
>>>>       int (*init)(struct udevice *dev);
>>>>   };
>>>> @@ -178,10 +179,15 @@ err:
>>>>   static int usb_onboard_hub_bind(struct udevice *dev)
>>>>   {
>>>> +    struct onboard_hub_data *data =
>>>> +        (struct onboard_hub_data *)dev_get_driver_data(dev);
>>>>       struct ofnode_phandle_args phandle;
>>>>       const void *fdt = gd->fdt_blob;
>>>>       int ret, off;
>>>> +    if (!data->peer_hub)
>>>> +        return 0;
>>>> +
>>>>       ret = dev_read_phandle_with_args(dev, "peer-hub", NULL, 0, 0, 
>>>> &phandle);
>>>>       if (ret)  {
>>>>           dev_err(dev, "peer-hub not specified\n");
>>>> @@ -212,12 +218,14 @@ static int usb_onboard_hub_remove(struct 
>>>> udevice *dev)
>>>>   }
>>>>   static const struct onboard_hub_data usb2514_data = {
>>>> +    .peer_hub = false,
>>>>       .power_on_delay_us = 500,
>>>>       .reset_us = 1,
>>>>   };
>>>>   static const struct onboard_hub_data usb5744_data = {
>>>>       .init = usb5744_i2c_init,
>>>> +    .peer_hub = true,
>>>>       .power_on_delay_us = 1000,
>>>>       .reset_us = 5,
>>>>   };
>>>
>>> I don't think this is correct solution. I think if optional property 
>>> peer-hub is not present in DT bind should return 0 and that should 
>>> fix issue on stm board where only one hub is present.
>> Shouldn't the 'peer-hub' property be mandatory for this hub ?
> 
> You can use this hub with usb2.0 only or with usb3.0 only. It means 
> peer-hub is correctly optional property. It is just saying when it is 
> there hub should be configured only once not twice.
> 
> It means right fix is when peer-hub property is missing bind should just 
> pass because it is just hub which don't use another ID.
OK