Message ID | 20190528022200.GA14816@ethiopia |
---|---|
State | Accepted |
Commit | 9adaa039b67a7f43e5618fcd5d4c9fc0e8f9dd68 |
Delegated to: | Tom Rini |
Headers | show |
Series | [U-Boot,1/3] ARM: dts: omap3-evm: Sync dts(i) files from Linux 5.1.5 | expand |
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 > [...] > > Signed-off-by: Derald D. Woods <woods.technical@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; > #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, > (struct device *)otg_board_data, > platdata->base); > + if (IS_ERR_OR_NULL(musbp)) > + 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? > #endif > return ret; > } >
On Tue, May 28, 2019 at 4:16 PM Marek Vasut <marek.vasut@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 > > [...] > > > > Signed-off-by: Derald D. Woods <woods.technical@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; > > #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, > > (struct device *)otg_board_data, > > platdata->base); > > + if (IS_ERR_OR_NULL(musbp)) > > + 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. Derald > > #endif > > return ret; > > } > > > > > -- > Best regards, > Marek Vasut
On 5/29/19 1:16 AM, Derald Woods wrote: > On Tue, May 28, 2019 at 4:16 PM Marek Vasut <marek.vasut@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@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() .
On Tue, May 28, 2019 at 6:27 PM Marek Vasut <marek.vasut@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@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@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. Thanks for the feedback. Derald > -- > Best regards, > Marek Vasut
On 5/29/19 2:00 AM, Derald Woods wrote: > On Tue, May 28, 2019 at 6:27 PM Marek Vasut <marek.vasut@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@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@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 ? diff --git a/drivers/usb/musb-new/omap2430.c b/drivers/usb/musb-new/omap2430.c index 32743aa72c..bffcb61eaf 100644 --- a/drivers/usb/musb-new/omap2430.c +++ b/drivers/usb/musb-new/omap2430.c @@ -236,9 +236,12 @@ static int omap2430_musb_probe(struct udevice *dev) ret = musb_lowlevel_init(host); #else - ret = musb_register(&platdata->plat, + host->host = musb_register(&platdata->plat, (struct device *)otg_board_data, platdata->base); + + if (!host->host) + return -EIO; #endif return ret; }
On Tue, May 28, 2019 at 7:49 PM Marek Vasut <marek.vasut@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@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@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@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. Derald > diff --git a/drivers/usb/musb-new/omap2430.c > b/drivers/usb/musb-new/omap2430.c > index 32743aa72c..bffcb61eaf 100644 > --- a/drivers/usb/musb-new/omap2430.c > +++ b/drivers/usb/musb-new/omap2430.c > @@ -236,9 +236,12 @@ static int omap2430_musb_probe(struct udevice *dev) > > ret = musb_lowlevel_init(host); > #else > - ret = musb_register(&platdata->plat, > + host->host = musb_register(&platdata->plat, > (struct device *)otg_board_data, > platdata->base); > + > + if (!host->host) > + return -EIO; > #endif > return ret; > } > > -- > Best regards, > Marek Vasut
On 5/29/19 3:34 AM, Derald Woods wrote: > On Tue, May 28, 2019 at 7:49 PM Marek Vasut <marek.vasut@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@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@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@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 patch compiles fine for omap3_beagle_defconfig too, which doesn't enable the MUSB_HOST.
On Wed, 29 May 2019, 05:07 Marek Vasut, <marek.vasut@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@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@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@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@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 >
On 5/29/19 1:44 PM, Derald Woods wrote: > > > On Wed, 29 May 2019, 05:07 Marek Vasut, <marek.vasut@gmail.com > <mailto:marek.vasut@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@gmail.com > <mailto:marek.vasut@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@gmail.com <mailto:marek.vasut@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@gmail.com <mailto:marek.vasut@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@gmail.com > <mailto:woods.technical@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? > 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
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@gmail.com > > <mailto:marek.vasut@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@gmail.com > > <mailto:marek.vasut@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@gmail.com <mailto:marek.vasut@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@gmail.com <mailto:marek.vasut@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@gmail.com > > <mailto:woods.technical@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
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@gmail.com >>> <mailto:marek.vasut@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@gmail.com >>> <mailto:marek.vasut@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@gmail.com <mailto:marek.vasut@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@gmail.com <mailto:marek.vasut@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@gmail.com >>> <mailto:woods.technical@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 .
On Mon, May 27, 2019 at 09:22:00PM -0500, 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 > [...] > > Signed-off-by: Derald D. Woods <woods.technical@gmail.com> Applied to u-boot/master, thanks!
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; #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, (struct device *)otg_board_data, platdata->base); + if (IS_ERR_OR_NULL(musbp)) + return -EINVAL; #endif return ret; }
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 [...] Signed-off-by: Derald D. Woods <woods.technical@gmail.com> --- drivers/usb/musb-new/omap2430.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)