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

[Patrick DELAUNAY]

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

> >> btw is this fix for current release or next ?
> >
> > I hope it for the v2019.07 (as it is only impact the stm32mp1 arch/board).
> > But it is not blocking.
> 
> OK
> 
> --
> Best regards,
> Marek Vasut