[U-Boot] [PATCH 1/2] usb: dwc2: correctly handle binding for g-tx-fifo-size

[Marek Vasut]

On 6/18/19 3:33 PM, Patrick DELAUNAY wrote:
> Hi Marek
> 
>> From: Marek Vasut <marex at denx.de>
>> Sent: mardi 18 juin 2019 12:49
>>
>> On 6/18/19 9:32 AM, Patrick DELAUNAY wrote:
>>> Hi Marek,
>>>
>>>> From: Marek Vasut <marex at denx.de>
>>>> Sent: lundi 17 juin 2019 17:54
>>>>
>>>> On 6/14/19 1:08 PM, Patrick Delaunay wrote:
>>>>> Manage g-tx-fifo-size as a array as specify in the binding.
>>>>>
>>>>> Signed-off-by: Patrick Delaunay <patrick.delaunay at st.com>
>>>>> ---
>>>>>
>>>>>  arch/arm/dts/stm32mp157c-ev1-u-boot.dtsi |  4 ----
>>>>>  drivers/usb/gadget/dwc2_udc_otg.c        | 17 ++++++++++++++++-
>>>>>  2 files changed, 16 insertions(+), 5 deletions(-)
>>>>>
>>>>> diff --git a/arch/arm/dts/stm32mp157c-ev1-u-boot.dtsi
>>>>> b/arch/arm/dts/stm32mp157c-ev1-u-boot.dtsi
>>>>> index 5b19e44..994092a 100644
>>>>> --- a/arch/arm/dts/stm32mp157c-ev1-u-boot.dtsi
>>>>> +++ b/arch/arm/dts/stm32mp157c-ev1-u-boot.dtsi
>>>>> @@ -56,10 +56,6 @@
>>>>>  	};
>>>>>  };
>>>>>
>>>>> -&usbotg_hs {
>>>>> -	g-tx-fifo-size = <576>;
>>>>
>>>> Should this really be in this patch ?
>>>
>>> As I change the binding parsing, the stm32mp1 will don't work without this
>> patch.
>>> I make a commun patch only to allow bisec, but I can split the serie with 2
>> patches.
>>>
>>>>
>>>>> -};
>>>>> -
>>>>>  &v3v3 {
>>>>>  	regulator-always-on;
>>>>>  };
>>>>> diff --git a/drivers/usb/gadget/dwc2_udc_otg.c
>>>>> b/drivers/usb/gadget/dwc2_udc_otg.c
>>>>> index 494ab53..7e6b5fc 100644
>>>>> --- a/drivers/usb/gadget/dwc2_udc_otg.c
>>>>> +++ b/drivers/usb/gadget/dwc2_udc_otg.c
>>>>> @@ -1039,6 +1039,8 @@ static int
>>>>> dwc2_udc_otg_ofdata_to_platdata(struct
>>>> udevice *dev)
>>>>>  	int node = dev_of_offset(dev);
>>>>>  	ulong drvdata;
>>>>>  	void (*set_params)(struct dwc2_plat_otg_data *data);
>>>>> +	u32 tx_fifo_sz_array[DWC2_MAX_HW_ENDPOINTS];
>>>>
>>>> Can't you just read directly into platdata->tx_fifo_sz_array[] below,
>>>> and thus drop this temporary variable ?
>>>
>>> It was the case in in my first internal version.
>>>
>>> if (platdata->tx_fifo_sz_nb) {
>>> 		ret = dev_read_u32_array(dev, "g-tx-fifo-size",
>>> 					 &platdata->tx_fifo_sz_array,
>>> 					 platdata->tx_fifo_sz_nb);
>>> 		if (ret)
>>> 			return ret;
>>> 	}
>>>
>>> And I add it to avoid the warning / potential issue:
>>>
>>> /local/home/frq07632/views/u-boot/u-
>> boot/drivers/usb/gadget/dwc2_udc_otg.c:1062:7:
>>> 	warning: passing argument 3 of ‘dev_read_u32_array’ from incompatible
>> pointer type [-Wincompatible-pointer-types]
>>>        &platdata->tx_fifo_sz_array,
>>>        ^~~~~~~~~~~~~~~~~~~~~~~~~~~
>>> In file included from /local/home/frq07632/views/u-boot/u-boot/include/dm.h:12,
>>>              from /local/home/frq07632/views/u-boot/u-
>> boot/drivers/usb/gadget/dwc2_udc_otg.c:22:
>>> 	/local/home/frq07632/views/u-boot/u-boot/include/dm/read.h:710:15: note:
>> expected ‘u32 *’ {aka ‘unsigned int *’} but argument is of type ‘unsigned int (*)[16]’
>>>           u32 *out_values, size_t sz)
>>>           ~~~~~^~~~~~~~~~
>>
>> Looks like GCC is complaining because you're passing an array of pointers to
>> dev_read_u32_array() , instead of plain pointer .
>>
>> Try
>>
>>  		ret = dev_read_u32_array(dev, "g-tx-fifo-size",
>> - 					 &platdata->tx_fifo_sz_array,
>> + 					 platdata->tx_fifo_sz_array,
>>  					 platdata->tx_fifo_sz_nb);
> 
> You are right, it is working
> I don't double check the type of the buffer :-<
> => I will push update in V2

Thanks

-- 
Best regards,
Marek Vasut