[U-Boot] [PATCH 3/3] usb: musb-new: omap2430: Fix compilation warning with USB_MUSB_GADGET

[Derald Woods]

On Wed, 29 May 2019, 05:07 Marek Vasut, <marek.vasut at gmail.com> wrote:

> On 5/29/19 3:34 AM, Derald Woods wrote:
> > On Tue, May 28, 2019 at 7:49 PM Marek Vasut <marek.vasut at gmail.com>
> wrote:
> >>
> >> On 5/29/19 2:00 AM, Derald Woods wrote:
> >>> On Tue, May 28, 2019 at 6:27 PM Marek Vasut <marek.vasut at gmail.com>
> wrote:
> >>>>
> >>>> On 5/29/19 1:16 AM, Derald Woods wrote:
> >>>>> On Tue, May 28, 2019 at 4:16 PM Marek Vasut <marek.vasut at gmail.com>
> wrote:
> >>>>>>
> >>>>>> On 5/28/19 4:22 AM, Derald D. Woods wrote:
> >>>>>>> This commit addresses the following warning, when _NOT_
> USB_MUSB_HOST:
> >>>>>>>
> >>>>>>> [...]
> >>>>>>>   CC      drivers/usb/gadget/f_mass_storage.o
> >>>>>>>   CC      drivers/usb/musb-new/omap2430.o
> >>>>>>>   CC      drivers/usb/gadget/f_fastboot.o
> >>>>>>>   CC      env/common.o
> >>>>>>>   CC      env/env.o
> >>>>>>>
> /src/etinker/software/u-boot-master/drivers/usb/musb-new/omap2430.c: In
> function ‘omap2430_musb_probe’:
> >>>>>>>
> /src/etinker/software/u-boot-master/drivers/usb/musb-new/omap2430.c:239:6:
> warning: assignment to ‘int’ from ‘struct musb *’ makes integer from
> pointer without a cast [-Wint-conversion]
> >>>>>>>   ret = musb_register(&platdata->plat,
> >>>>>>>       ^
> >>>>>>>   LD      drivers/usb/host/built-in.o
> >>>>>>>   CC      drivers/usb/gadget/f_sdp.o
> >>>>>>>   CC      fs/ext4/ext4fs.o
> >>>>>>> [...]
> >>>>
> >>>> Skip to the end first
> >>>>
> >>>>>>> Signed-off-by: Derald D. Woods <woods.technical at gmail.com>
> >>>>>>> ---
> >>>>>>>  drivers/usb/musb-new/omap2430.c | 8 ++++++--
> >>>>>>>  1 file changed, 6 insertions(+), 2 deletions(-)
> >>>>>>>
> >>>>>>> diff --git a/drivers/usb/musb-new/omap2430.c
> b/drivers/usb/musb-new/omap2430.c
> >>>>>>> index 32743aa72c..cca1653f1e 100644
> >>>>>>> --- a/drivers/usb/musb-new/omap2430.c
> >>>>>>> +++ b/drivers/usb/musb-new/omap2430.c
> >>>>>>> @@ -215,11 +215,13 @@ static int omap2430_musb_probe(struct
> udevice *dev)
> >>>>>>>  {
> >>>>>>>  #ifdef CONFIG_USB_MUSB_HOST
> >>>>>>>       struct musb_host_data *host = dev_get_priv(dev);
> >>>>>>> +#else
> >>>>>>> +     struct musb *musbp;
> >>>>
> >>>> Drop this hunk
> >>>>
> >>>>>>>  #endif
> >>>>>>>       struct omap2430_musb_platdata *platdata =
> dev_get_platdata(dev);
> >>>>>>>       struct usb_bus_priv *priv = dev_get_uclass_priv(dev);
> >>>>>>>       struct omap_musb_board_data *otg_board_data;
> >>>>>>> -     int ret;
> >>>>>>> +     int ret = 0;
> >>>>>>>       void *base = dev_read_addr_ptr(dev);
> >>>>>>>
> >>>>>>>       priv->desc_before_addr = true;
> >>>>>>> @@ -236,9 +238,11 @@ static int omap2430_musb_probe(struct udevice
> *dev)
> >>>>>>>
> >>>>>>>       ret = musb_lowlevel_init(host);
> >>>>>>>  #else
> >>>>>>> -     ret = musb_register(&platdata->plat,
> >>>>>>> +     musbp = musb_register(&platdata->plat,
> >>>>
> >>>> Replace musbp with host->host
> >>>>
> >>>>>>>                         (struct device *)otg_board_data,
> >>>>>>>                         platdata->base);
> >>>>>>> +     if (IS_ERR_OR_NULL(musbp))
> >>>>
> >>>> here too
> >>>>
> >>>>>>> +             return -EINVAL;
> >>>>>>
> >>>>>> For example the pic32 glue code holds the musb_host_data in private
> data
> >>>>>> , so it can call musb_stop() in .remove callback . Can you do the
> same?
> >>>>>>
> >>>>>
> >>>>> This was just a non-structural change to eliminate a compiler
> warning.
> >>>>> I agree that other things can/should be done with this driver. That
> >>>>> work would be outside of what I was trying to accomplsh here, at the
> >>>>> moment.
> >>>>
> >>>> See above, I believe that should fix the problem _and_ not crash in
> the
> >>>> .remove() .
> >>>>
> >>>
> >>> As with 'ti-musb', there needs to be a clear distinction between
> >>> 'host' and 'peripheral'/'gadget'. The issues here are much more broad
> >>> than the compile warning that I was trying to address. The required
> >>> refactoring, for USB Gadgets, deserves a proper series for older OMAP3
> >>> devices.
> >>>
> >>> I would rather _DROP_ this patch than apply something that could
> >>> result in a series of kludges.
> >>
> >> Does the following patch work for you ?
> >>
> >
> > My use case does _not_ enable 'CONFIG_USB_MUSB_HOST'. The C
> > preprocessor directives prevent 'host->host' from being included in
> > compilation in that case.
>
> drivers/usb/musb-new/musb_uboot.h doesn't have any special stuff to
> remove the struct musb_host_data { struct musb *musb ... } field .
> What am I missing ?
>

The code is guarded by CPP directives. You are speaking of a more extensive
refactoring/changing of the driver. There is a set of patches on the
mailing list, by Adam Ford, that does just that. Or at least makes the
proper start. I will look at those as a start. I am dropping my effort on
this single compilation fix patch. So I will be looking at a different path.

Thanks again.

Derald



> The patch compiles fine for omap3_beagle_defconfig too, which doesn't
> enable the MUSB_HOST.
>
> --
> Best regards,
> Marek Vasut
>