[PATCH] misc: Port USB251xB/xBi Hi-Speed Hub Controller Driver from Linux

[Marek Vasut]

On 6/13/22 19:23, Stefan Herbrechtsmeier wrote:
> Hi Marek,

Hi,

[...]

>>>> +
>>>> +    if (IS_ENABLED(CONFIG_DM_REGULATOR)) {
>>>> +        err = device_get_supply_regulator(dev, "vdd-supply",
>>>> +                          &hub->vdd);
>>>> +        if (err && err != -ENOENT) {
>>>> +            dev_err(dev, "Warning: cannot get power supply\n");
>>>> +            return err;
>>>> +        }
>>>> +    }
>>>
>>> Shouldn't the gpio and regulator request be part of the probe?
>>
>> What makes you think that ?
> 
> It's more a question regarding the device model design:
> 
> of_to_plat - convert device tree data to plat

As far as I can tell, you should be able to obtain reference to a 
regulator here, but not probe the regulator.

>>>> +    if (dev_read_u32(dev, "vendor-id", &hub->vendor_id))
>>>> +        hub->vendor_id = USB251XB_DEF_VENDOR_ID;
>>>> +
>>>> +    if (dev_read_u32(dev, "product-id", &hub->product_id))
>>>> +        hub->product_id = data->product_id;
>>>> +
>>>> +    if (dev_read_u32(dev, "device-id", &hub->device_id))
>>>> +        hub->device_id = USB251XB_DEF_DEVICE_ID;
>>>> +
>>>
>>> [snip]
>>>
>>>> +    if (dev_read_u32(dev, "language-id", &hub->lang_id))
>>>> +        hub->lang_id = USB251XB_DEF_LANGUAGE_ID;
>>>> +
>>>
>>> This doesn't work because the ids are 16 bit [1,2] and the 
>>> dev_read_u32 function checks the size.
>>>
>>>> +    if (!dev_read_u32(dev, "boost-up", &hub->boost_up))
>>>> +        hub->boost_up = USB251XB_DEF_BOOST_UP;
>>>
>>> This looks like a 8 bit value [1].
>>
>> The dev_read_u32() is also capable of reading 0xNNNN 16bit value from DT.
>>
>> What kind of problem are you running into exactly ?
> 
> Have you test the values from device tree binding documentation:
> 
> vendor-id = /bits/ 16 <0xNNNN>;
> product-id = /bits/ 16 <0xNNNN>;
> 
> I get an EOVERFLOW error.

No, I only tested foo = <0xNNNN>; .

Can you send a fix ?