[U-Boot] [PATCH 3/3] usb: musb-new: omap2430: Fix compilation warning with USB_MUSB_GADGET
Derald D. Woods
woods.technical at gmail.com
Wed May 29 12:33:45 UTC 2019
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.
> > You are speaking of a more
> > extensive refactoring/changing of the driver.
>
> I am speaking of that diff I posted a few hours ago, nothing else.
> This should fix this problem _and_ not fail in the remove path.
> There is no refactoring necessary for that to work.
>
> > 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.
> OK
>
> --
> Best regards,
> Marek Vasut
More information about the U-Boot
mailing list