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

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

[Derald D. Woods]

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;
 #endif
 	return ret;
 }
-- 
2.21.0

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

[Marek Vasut]

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;
>  }
> 


-- 
Best regards,
Marek Vasut

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

[Derald Woods]

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

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

[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() .

-- 
Best regards,
Marek Vasut

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

[Derald Woods]

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

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

[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;
 }

-- 
Best regards,
Marek Vasut

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

[Derald Woods]

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

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

[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.

-- 
Best regards,
Marek Vasut

[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@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
>

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

[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 at 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 at 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

-- 
Best regards,
Marek Vasut

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

[Derald D. Woods]

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 at 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 at 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

[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@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 at 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 at 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 .

-- 
Best regards,
Marek Vasut

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

[Tom Rini]

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!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20190714/55d59fe1/attachment.sig>