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

[Marek Vasut]

On 5/29/19 2:33 PM, Derald D. Woods wrote:
> On Wed, May 29, 2019 at 02:17:03PM +0200, Marek Vasut wrote:
>> On 5/29/19 1:44 PM, Derald Woods wrote:
>>>
>>>
>>> On Wed, 29 May 2019, 05:07 Marek Vasut, <marek.vasut at gmail.com
>>> <mailto: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
>>>     <mailto: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 <mailto: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 <mailto: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
>>>     <mailto: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.
>>
>> Can you point out the code you're talking about ? Which CPP directive do
>> you have in mind exactly?
>>
> 
> 
> omap2430.c lines 216 and 229. 'host' is only used in that context.

Remove the ifdef on line 216 , better ?

Also see the remove callback .

-- 
Best regards,
Marek Vasut