[PATCH] usb: dwc2: handle return code of dev_read_size() in of to plat function

[Wolfgang Grandegger]

Am 12.03.22 um 22:27 schrieb Marek Vasut:
> On 3/12/22 18:22, Wolfgang Grandegger wrote:
>> dev_read_size() returns -EINVAL (-22) if the property does not exist.
> 
> Which property ?

g-tx-fifo-size

> 
>> Signed-off-by: Wolfgang Grandegger <wg at aries-embedded.de>
>> ---
>>   drivers/usb/gadget/dwc2_udc_otg.c | 6 +++---
>>   1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/usb/gadget/dwc2_udc_otg.c
>> b/drivers/usb/gadget/dwc2_udc_otg.c
>> index 2748270..00f7e8e 100644
>> --- a/drivers/usb/gadget/dwc2_udc_otg.c
>> +++ b/drivers/usb/gadget/dwc2_udc_otg.c
>> @@ -996,8 +996,9 @@ static int dwc2_udc_otg_of_to_plat(struct udevice
>> *dev)
>>       plat->rx_fifo_sz = dev_read_u32_default(dev, "g-rx-fifo-size", 0);
>>       plat->np_tx_fifo_sz = dev_read_u32_default(dev,
>> "g-np-tx-fifo-size", 0);
>>   -    plat->tx_fifo_sz_nb =
>> -        dev_read_size(dev, "g-tx-fifo-size") / sizeof(u32);
>> +    ret = dev_read_size(dev, "g-tx-fifo-size");
>> +    if (ret > 0)
>> +        platdata->tx_fifo_sz_nb = ret / sizeof(u32);
> 
> If ret <= 0, then platdata->tx_fifo_sz_nb is now uninitialized ?

It's initialized to zero like the properties above. Later on default
values will be used. platdata->tx_fifo_sz_nb is an unsigned char and
therefore -22 is then a positive value resulting in an error in the
following lines.

So far, CONFIG_DM_USB_GADGET=y has only be used by boards, which do
define these dts properties, obviously. This is not the case for the
socfpga boards. They use the internal default values.

As you know, I'm trying to fix the reset issue with USB for "ums 0 mmc
0" for the mcvevk. There are other issues with CONFIG_DM_USB_GADGET=y.
More an that beginning of next week.

>>       if (plat->tx_fifo_sz_nb > DWC2_MAX_HW_ENDPOINTS)
>>           plat->tx_fifo_sz_nb = DWC2_MAX_HW_ENDPOINTS;
>>       if (plat->tx_fifo_sz_nb) {
>> @@ -1052,7 +1053,6 @@ static int dwc2_udc_otg_reset_init(struct
>> udevice *dev,
>>           return ret;
>>         ret = reset_assert_bulk(resets);
>> -
>>       if (!ret) {
>>           udelay(2);
>>           ret = reset_deassert_bulk(resets);
> 
> This shouldn't be part of the patch ?

OK, I have just sent out v2.

Wolfgang