From mboxrd@z Thu Jan 1 00:00:00 1970 From: Marek Vasut Date: Wed, 29 May 2019 14:17:03 +0200 Subject: [U-Boot] [PATCH 3/3] usb: musb-new: omap2430: Fix compilation warning with USB_MUSB_GADGET In-Reply-To: References: <20190528022200.GA14816@ethiopia> <9cf07647-2861-7741-e8f6-03e022b5b075@gmail.com> Message-ID: <4fdb6502-0dbe-646a-43af-3c0ff1308f7a@gmail.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 8bit To: u-boot@lists.denx.de On 5/29/19 1:44 PM, Derald Woods wrote: > > > On Wed, 29 May 2019, 05:07 Marek Vasut, > wrote: > > On 5/29/19 3:34 AM, Derald Woods wrote: > > On Tue, May 28, 2019 at 7:49 PM Marek Vasut > wrote: > >> > >> On 5/29/19 2:00 AM, Derald Woods wrote: > >>> On Tue, May 28, 2019 at 6:27 PM Marek Vasut > > wrote: > >>>> > >>>> On 5/29/19 1:16 AM, Derald Woods wrote: > >>>>> On Tue, May 28, 2019 at 4:16 PM Marek Vasut > > 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 > > >>>>>>> --- > >>>>>>>  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? > 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