| Message ID | 20181205124945.22813-1-stefan@olimex.com |
|---|---|
| State | Accepted |
| Commit | 46a3f276549f3e5720b6e80278cda354c7fa859f |
| Delegated to: | Marek Vasut |
| Headers | show |
| Series | [U-Boot,1/1] usb: musb-new: sunxi: Fix null pointer access | expand |
On 12/05/2018 01:49 PM, Stefan Mavrodiev wrote: > When the device is in peripheral mode Can you have two devices, one in peripheral mode and one in host mode, on the same system ? > there is no > struct usb_bus_priv allocated pointer, as the uclass driver > ("usb_dev_generic") doesn't call per_device_auto_alloc_size. > > This results in writing to the internal SDRAM at > priv->desc_before_addr = true; > > Signed-off-by: Stefan Mavrodiev <stefan@olimex.com> > --- > drivers/usb/musb-new/sunxi.c | 8 ++++++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > > diff --git a/drivers/usb/musb-new/sunxi.c b/drivers/usb/musb-new/sunxi.c > index 6cf9826cda..f3deb9bc66 100644 > --- a/drivers/usb/musb-new/sunxi.c > +++ b/drivers/usb/musb-new/sunxi.c > @@ -435,11 +435,14 @@ static int musb_usb_probe(struct udevice *dev) > { > struct sunxi_glue *glue = dev_get_priv(dev); > struct musb_host_data *host = &glue->mdata; > - struct usb_bus_priv *priv = dev_get_uclass_priv(dev); > struct musb_hdrc_platform_data pdata; > void *base = dev_read_addr_ptr(dev); > int ret; > > +#ifdef CONFIG_USB_MUSB_HOST > + struct usb_bus_priv *priv = dev_get_uclass_priv(dev); > +#endif > + > if (!base) > return -EINVAL; > > @@ -459,7 +462,6 @@ static int musb_usb_probe(struct udevice *dev) > return ret; > } > > - priv->desc_before_addr = true; See my question at the beginning, and if that can be the case, the fix is to check if priv is not null here, eg. if (priv) priv->... Still, why is the priv data not allocated for device ? > memset(&pdata, 0, sizeof(pdata)); > pdata.power = 250; > @@ -467,6 +469,8 @@ static int musb_usb_probe(struct udevice *dev) > pdata.config = glue->cfg->config; > > #ifdef CONFIG_USB_MUSB_HOST > + priv->desc_before_addr = true; > + > pdata.mode = MUSB_HOST; > host->host = musb_init_controller(&pdata, &glue->dev, base); > if (!host->host) >
On 12/5/18 2:57 PM, Marek Vasut wrote: > On 12/05/2018 01:49 PM, Stefan Mavrodiev wrote: >> When the device is in peripheral mode > Can you have two devices, one in peripheral mode and one in host mode, > on the same system ? Not 100% sure, but I'm thinking there is only one OTG port for all sunxi boards. The operation is decided in the Kconfig. > >> there is no >> struct usb_bus_priv allocated pointer, as the uclass driver >> ("usb_dev_generic") doesn't call per_device_auto_alloc_size. >> >> This results in writing to the internal SDRAM at >> priv->desc_before_addr = true; >> >> Signed-off-by: Stefan Mavrodiev <stefan@olimex.com> >> --- >> drivers/usb/musb-new/sunxi.c | 8 ++++++-- >> 1 file changed, 6 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/usb/musb-new/sunxi.c b/drivers/usb/musb-new/sunxi.c >> index 6cf9826cda..f3deb9bc66 100644 >> --- a/drivers/usb/musb-new/sunxi.c >> +++ b/drivers/usb/musb-new/sunxi.c >> @@ -435,11 +435,14 @@ static int musb_usb_probe(struct udevice *dev) >> { >> struct sunxi_glue *glue = dev_get_priv(dev); >> struct musb_host_data *host = &glue->mdata; >> - struct usb_bus_priv *priv = dev_get_uclass_priv(dev); >> struct musb_hdrc_platform_data pdata; >> void *base = dev_read_addr_ptr(dev); >> int ret; >> >> +#ifdef CONFIG_USB_MUSB_HOST >> + struct usb_bus_priv *priv = dev_get_uclass_priv(dev); >> +#endif >> + >> if (!base) >> return -EINVAL; >> >> @@ -459,7 +462,6 @@ static int musb_usb_probe(struct udevice *dev) >> return ret; >> } >> >> - priv->desc_before_addr = true; > See my question at the beginning, and if that can be the case, the fix > is to check if priv is not null here, eg. > if (priv) > priv->... > > Still, why is the priv data not allocated for device ? Depending on configuration, the device is registered ether as UCLASS_USB_DEV_GENERIC or UCLASS_USB. There is no .per_device_auto_alloc_size = sizeof(struct usb_bus_priv), for the second. (As seen in drivers/usb/host/usb-uclass.c) > >> memset(&pdata, 0, sizeof(pdata)); >> pdata.power = 250; >> @@ -467,6 +469,8 @@ static int musb_usb_probe(struct udevice *dev) >> pdata.config = glue->cfg->config; >> >> #ifdef CONFIG_USB_MUSB_HOST >> + priv->desc_before_addr = true; >> + >> pdata.mode = MUSB_HOST; >> host->host = musb_init_controller(&pdata, &glue->dev, base); >> if (!host->host) >> >
On Wed, Dec 05, 2018 at 01:57:14PM +0100, Marek Vasut wrote: > On 12/05/2018 01:49 PM, Stefan Mavrodiev wrote: > > When the device is in peripheral mode > > Can you have two devices, one in peripheral mode and one in host mode, > on the same system ? No, or at least, on all of the SoCs that Allwinner ever produced, there's only a single musb controller. Maxime
On 12/05/2018 02:06 PM, Stefan Mavrodiev wrote: > > On 12/5/18 2:57 PM, Marek Vasut wrote: >> On 12/05/2018 01:49 PM, Stefan Mavrodiev wrote: >>> When the device is in peripheral mode >> Can you have two devices, one in peripheral mode and one in host mode, >> on the same system ? > > Not 100% sure, but I'm thinking there is only one OTG port for > all sunxi boards. The operation is decided in the Kconfig. I'm rather sure I saw sunxi boards with more than one USB port. >>> there is no >>> struct usb_bus_priv allocated pointer, as the uclass driver >>> ("usb_dev_generic") doesn't call per_device_auto_alloc_size. >>> >>> This results in writing to the internal SDRAM at >>> priv->desc_before_addr = true; >>> >>> Signed-off-by: Stefan Mavrodiev <stefan@olimex.com> >>> --- >>> drivers/usb/musb-new/sunxi.c | 8 ++++++-- >>> 1 file changed, 6 insertions(+), 2 deletions(-) >>> >>> diff --git a/drivers/usb/musb-new/sunxi.c b/drivers/usb/musb-new/sunxi.c >>> index 6cf9826cda..f3deb9bc66 100644 >>> --- a/drivers/usb/musb-new/sunxi.c >>> +++ b/drivers/usb/musb-new/sunxi.c >>> @@ -435,11 +435,14 @@ static int musb_usb_probe(struct udevice *dev) >>> { >>> struct sunxi_glue *glue = dev_get_priv(dev); >>> struct musb_host_data *host = &glue->mdata; >>> - struct usb_bus_priv *priv = dev_get_uclass_priv(dev); >>> struct musb_hdrc_platform_data pdata; >>> void *base = dev_read_addr_ptr(dev); >>> int ret; >>> +#ifdef CONFIG_USB_MUSB_HOST >>> + struct usb_bus_priv *priv = dev_get_uclass_priv(dev); >>> +#endif >>> + >>> if (!base) >>> return -EINVAL; >>> @@ -459,7 +462,6 @@ static int musb_usb_probe(struct udevice *dev) >>> return ret; >>> } >>> - priv->desc_before_addr = true; >> See my question at the beginning, and if that can be the case, the fix >> is to check if priv is not null here, eg. >> if (priv) >> priv->... >> >> Still, why is the priv data not allocated for device ? > > Depending on configuration, the device is registered ether as > UCLASS_USB_DEV_GENERIC or UCLASS_USB. There is no > > .per_device_auto_alloc_size = sizeof(struct usb_bus_priv), > > for the second. (As seen in drivers/usb/host/usb-uclass.c) I see the code is rather horrible. I'd expect all that configuration to come from DT otg-mode property instead of being hard-wired into the code. Sigh. Jagan, A-B ? I'd like to pick this . >> >>> memset(&pdata, 0, sizeof(pdata)); >>> pdata.power = 250; >>> @@ -467,6 +469,8 @@ static int musb_usb_probe(struct udevice *dev) >>> pdata.config = glue->cfg->config; >>> #ifdef CONFIG_USB_MUSB_HOST >>> + priv->desc_before_addr = true; >>> + >>> pdata.mode = MUSB_HOST; >>> host->host = musb_init_controller(&pdata, &glue->dev, base); >>> if (!host->host) >>> >>
On 12/5/18 3:16 PM, Marek Vasut wrote: > On 12/05/2018 02:06 PM, Stefan Mavrodiev wrote: >> On 12/5/18 2:57 PM, Marek Vasut wrote: >>> On 12/05/2018 01:49 PM, Stefan Mavrodiev wrote: >>>> When the device is in peripheral mode >>> Can you have two devices, one in peripheral mode and one in host mode, >>> on the same system ? >> Not 100% sure, but I'm thinking there is only one OTG port for >> all sunxi boards. The operation is decided in the Kconfig. > I'm rather sure I saw sunxi boards with more than one USB port. > >>>> there is no >>>> struct usb_bus_priv allocated pointer, as the uclass driver >>>> ("usb_dev_generic") doesn't call per_device_auto_alloc_size. >>>> >>>> This results in writing to the internal SDRAM at >>>> priv->desc_before_addr = true; >>>> >>>> Signed-off-by: Stefan Mavrodiev <stefan@olimex.com> >>>> --- >>>> drivers/usb/musb-new/sunxi.c | 8 ++++++-- >>>> 1 file changed, 6 insertions(+), 2 deletions(-) >>>> >>>> diff --git a/drivers/usb/musb-new/sunxi.c b/drivers/usb/musb-new/sunxi.c >>>> index 6cf9826cda..f3deb9bc66 100644 >>>> --- a/drivers/usb/musb-new/sunxi.c >>>> +++ b/drivers/usb/musb-new/sunxi.c >>>> @@ -435,11 +435,14 @@ static int musb_usb_probe(struct udevice *dev) >>>> { >>>> struct sunxi_glue *glue = dev_get_priv(dev); >>>> struct musb_host_data *host = &glue->mdata; >>>> - struct usb_bus_priv *priv = dev_get_uclass_priv(dev); >>>> struct musb_hdrc_platform_data pdata; >>>> void *base = dev_read_addr_ptr(dev); >>>> int ret; >>>> +#ifdef CONFIG_USB_MUSB_HOST >>>> + struct usb_bus_priv *priv = dev_get_uclass_priv(dev); >>>> +#endif >>>> + >>>> if (!base) >>>> return -EINVAL; >>>> @@ -459,7 +462,6 @@ static int musb_usb_probe(struct udevice *dev) >>>> return ret; >>>> } >>>> - priv->desc_before_addr = true; >>> See my question at the beginning, and if that can be the case, the fix >>> is to check if priv is not null here, eg. >>> if (priv) >>> priv->... >>> >>> Still, why is the priv data not allocated for device ? >> Depending on configuration, the device is registered ether as >> UCLASS_USB_DEV_GENERIC or UCLASS_USB. There is no >> >> .per_device_auto_alloc_size = sizeof(struct usb_bus_priv), >> >> for the second. (As seen in drivers/usb/host/usb-uclass.c) > I see the code is rather horrible. I'd expect all that configuration to > come from DT otg-mode property instead of being hard-wired into the > code. Sigh. > > Jagan, A-B ? I'd like to pick this . > >>>> memset(&pdata, 0, sizeof(pdata)); >>>> pdata.power = 250; >>>> @@ -467,6 +469,8 @@ static int musb_usb_probe(struct udevice *dev) >>>> pdata.config = glue->cfg->config; >>>> #ifdef CONFIG_USB_MUSB_HOST >>>> + priv->desc_before_addr = true; >>>> + >>>> pdata.mode = MUSB_HOST; >>>> host->host = musb_init_controller(&pdata, &glue->dev, base); >>>> if (!host->host) >>>> > Any further comments?
On 12/13/2018 08:14 AM, Stefan Mavrodiev wrote: > > On 12/5/18 3:16 PM, Marek Vasut wrote: >> On 12/05/2018 02:06 PM, Stefan Mavrodiev wrote: >>> On 12/5/18 2:57 PM, Marek Vasut wrote: >>>> On 12/05/2018 01:49 PM, Stefan Mavrodiev wrote: >>>>> When the device is in peripheral mode >>>> Can you have two devices, one in peripheral mode and one in host mode, >>>> on the same system ? >>> Not 100% sure, but I'm thinking there is only one OTG port for >>> all sunxi boards. The operation is decided in the Kconfig. >> I'm rather sure I saw sunxi boards with more than one USB port. >> >>>>> there is no >>>>> struct usb_bus_priv allocated pointer, as the uclass driver >>>>> ("usb_dev_generic") doesn't call per_device_auto_alloc_size. >>>>> >>>>> This results in writing to the internal SDRAM at >>>>> priv->desc_before_addr = true; >>>>> >>>>> Signed-off-by: Stefan Mavrodiev <stefan@olimex.com> >>>>> --- >>>>> drivers/usb/musb-new/sunxi.c | 8 ++++++-- >>>>> 1 file changed, 6 insertions(+), 2 deletions(-) >>>>> >>>>> diff --git a/drivers/usb/musb-new/sunxi.c >>>>> b/drivers/usb/musb-new/sunxi.c >>>>> index 6cf9826cda..f3deb9bc66 100644 >>>>> --- a/drivers/usb/musb-new/sunxi.c >>>>> +++ b/drivers/usb/musb-new/sunxi.c >>>>> @@ -435,11 +435,14 @@ static int musb_usb_probe(struct udevice *dev) >>>>> { >>>>> struct sunxi_glue *glue = dev_get_priv(dev); >>>>> struct musb_host_data *host = &glue->mdata; >>>>> - struct usb_bus_priv *priv = dev_get_uclass_priv(dev); >>>>> struct musb_hdrc_platform_data pdata; >>>>> void *base = dev_read_addr_ptr(dev); >>>>> int ret; >>>>> +#ifdef CONFIG_USB_MUSB_HOST >>>>> + struct usb_bus_priv *priv = dev_get_uclass_priv(dev); >>>>> +#endif >>>>> + >>>>> if (!base) >>>>> return -EINVAL; >>>>> @@ -459,7 +462,6 @@ static int musb_usb_probe(struct udevice *dev) >>>>> return ret; >>>>> } >>>>> - priv->desc_before_addr = true; >>>> See my question at the beginning, and if that can be the case, the fix >>>> is to check if priv is not null here, eg. >>>> if (priv) >>>> priv->... >>>> >>>> Still, why is the priv data not allocated for device ? >>> Depending on configuration, the device is registered ether as >>> UCLASS_USB_DEV_GENERIC or UCLASS_USB. There is no >>> >>> .per_device_auto_alloc_size = sizeof(struct usb_bus_priv), >>> >>> for the second. (As seen in drivers/usb/host/usb-uclass.c) >> I see the code is rather horrible. I'd expect all that configuration to >> come from DT otg-mode property instead of being hard-wired into the >> code. Sigh. >> >> Jagan, A-B ? I'd like to pick this . >> >>>>> memset(&pdata, 0, sizeof(pdata)); >>>>> pdata.power = 250; >>>>> @@ -467,6 +469,8 @@ static int musb_usb_probe(struct udevice *dev) >>>>> pdata.config = glue->cfg->config; >>>>> #ifdef CONFIG_USB_MUSB_HOST >>>>> + priv->desc_before_addr = true; >>>>> + >>>>> pdata.mode = MUSB_HOST; >>>>> host->host = musb_init_controller(&pdata, &glue->dev, base); >>>>> if (!host->host) >>>>> >> > Any further comments? As Jagan is inactive, applied.
On 05/12/2018 13:57, Marek Vasut wrote: > On 12/05/2018 01:49 PM, Stefan Mavrodiev wrote: >> When the device is in peripheral mode > Can you have two devices, one in peripheral mode and one in host mode, > on the same system ? It is possible with the musb-new. Using DM_USB and DM_USB_GADGET, I did it with on a beaglebone black. JJ >> there is no >> struct usb_bus_priv allocated pointer, as the uclass driver >> ("usb_dev_generic") doesn't call per_device_auto_alloc_size. >> >> This results in writing to the internal SDRAM at >> priv->desc_before_addr = true; >> >> Signed-off-by: Stefan Mavrodiev <stefan@olimex.com> >> --- >> drivers/usb/musb-new/sunxi.c | 8 ++++++-- >> 1 file changed, 6 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/usb/musb-new/sunxi.c b/drivers/usb/musb-new/sunxi.c >> index 6cf9826cda..f3deb9bc66 100644 >> --- a/drivers/usb/musb-new/sunxi.c >> +++ b/drivers/usb/musb-new/sunxi.c >> @@ -435,11 +435,14 @@ static int musb_usb_probe(struct udevice *dev) >> { >> struct sunxi_glue *glue = dev_get_priv(dev); >> struct musb_host_data *host = &glue->mdata; >> - struct usb_bus_priv *priv = dev_get_uclass_priv(dev); >> struct musb_hdrc_platform_data pdata; >> void *base = dev_read_addr_ptr(dev); >> int ret; >> >> +#ifdef CONFIG_USB_MUSB_HOST >> + struct usb_bus_priv *priv = dev_get_uclass_priv(dev); >> +#endif >> + >> if (!base) >> return -EINVAL; >> >> @@ -459,7 +462,6 @@ static int musb_usb_probe(struct udevice *dev) >> return ret; >> } >> >> - priv->desc_before_addr = true; > See my question at the beginning, and if that can be the case, the fix > is to check if priv is not null here, eg. > if (priv) > priv->... > > Still, why is the priv data not allocated for device ? > >> memset(&pdata, 0, sizeof(pdata)); >> pdata.power = 250; >> @@ -467,6 +469,8 @@ static int musb_usb_probe(struct udevice *dev) >> pdata.config = glue->cfg->config; >> >> #ifdef CONFIG_USB_MUSB_HOST >> + priv->desc_before_addr = true; >> + >> pdata.mode = MUSB_HOST; >> host->host = musb_init_controller(&pdata, &glue->dev, base); >> if (!host->host) >> >
On 12/13/2018 03:03 PM, Jean-Jacques Hiblot wrote: > > On 05/12/2018 13:57, Marek Vasut wrote: >> On 12/05/2018 01:49 PM, Stefan Mavrodiev wrote: >>> When the device is in peripheral mode >> Can you have two devices, one in peripheral mode and one in host mode, >> on the same system ? > > It is possible with the musb-new. Using DM_USB and DM_USB_GADGET, I did > it with on a beaglebone black. Does this patch break it ? And if so, can you send a proper fix ? > JJ > >>> there is no >>> struct usb_bus_priv allocated pointer, as the uclass driver >>> ("usb_dev_generic") doesn't call per_device_auto_alloc_size. >>> >>> This results in writing to the internal SDRAM at >>> priv->desc_before_addr = true; >>> >>> Signed-off-by: Stefan Mavrodiev <stefan@olimex.com> >>> --- >>> drivers/usb/musb-new/sunxi.c | 8 ++++++-- >>> 1 file changed, 6 insertions(+), 2 deletions(-) >>> >>> diff --git a/drivers/usb/musb-new/sunxi.c b/drivers/usb/musb-new/sunxi.c >>> index 6cf9826cda..f3deb9bc66 100644 >>> --- a/drivers/usb/musb-new/sunxi.c >>> +++ b/drivers/usb/musb-new/sunxi.c >>> @@ -435,11 +435,14 @@ static int musb_usb_probe(struct udevice *dev) >>> { >>> struct sunxi_glue *glue = dev_get_priv(dev); >>> struct musb_host_data *host = &glue->mdata; >>> - struct usb_bus_priv *priv = dev_get_uclass_priv(dev); >>> struct musb_hdrc_platform_data pdata; >>> void *base = dev_read_addr_ptr(dev); >>> int ret; >>> +#ifdef CONFIG_USB_MUSB_HOST >>> + struct usb_bus_priv *priv = dev_get_uclass_priv(dev); >>> +#endif >>> + >>> if (!base) >>> return -EINVAL; >>> @@ -459,7 +462,6 @@ static int musb_usb_probe(struct udevice *dev) >>> return ret; >>> } >>> - priv->desc_before_addr = true; >> See my question at the beginning, and if that can be the case, the fix >> is to check if priv is not null here, eg. >> if (priv) >> priv->... >> >> Still, why is the priv data not allocated for device ? >> >>> memset(&pdata, 0, sizeof(pdata)); >>> pdata.power = 250; >>> @@ -467,6 +469,8 @@ static int musb_usb_probe(struct udevice *dev) >>> pdata.config = glue->cfg->config; >>> #ifdef CONFIG_USB_MUSB_HOST >>> + priv->desc_before_addr = true; >>> + >>> pdata.mode = MUSB_HOST; >>> host->host = musb_init_controller(&pdata, &glue->dev, base); >>> if (!host->host) >>> >>
On 13/12/2018 15:05, Marek Vasut wrote: > On 12/13/2018 03:03 PM, Jean-Jacques Hiblot wrote: >> On 05/12/2018 13:57, Marek Vasut wrote: >>> On 12/05/2018 01:49 PM, Stefan Mavrodiev wrote: >>>> When the device is in peripheral mode >>> Can you have two devices, one in peripheral mode and one in host mode, >>> on the same system ? >> It is possible with the musb-new. Using DM_USB and DM_USB_GADGET, I did >> it with on a beaglebone black. > Does this patch break it ? And if so, can you send a proper fix ? This file is not compiled when building for beaglebone, so it won't break anything. BTW this driver could be adapted, with little work, to support a dynamic selection of the role based on "dr-mode" in the DT. Not knowing the boards, I don't know if this work has any added value though. JJ > >> JJ >> >>>> there is no >>>> struct usb_bus_priv allocated pointer, as the uclass driver >>>> ("usb_dev_generic") doesn't call per_device_auto_alloc_size. >>>> >>>> This results in writing to the internal SDRAM at >>>> priv->desc_before_addr = true; >>>> >>>> Signed-off-by: Stefan Mavrodiev <stefan@olimex.com> >>>> --- >>>> drivers/usb/musb-new/sunxi.c | 8 ++++++-- >>>> 1 file changed, 6 insertions(+), 2 deletions(-) >>>> >>>> diff --git a/drivers/usb/musb-new/sunxi.c b/drivers/usb/musb-new/sunxi.c >>>> index 6cf9826cda..f3deb9bc66 100644 >>>> --- a/drivers/usb/musb-new/sunxi.c >>>> +++ b/drivers/usb/musb-new/sunxi.c >>>> @@ -435,11 +435,14 @@ static int musb_usb_probe(struct udevice *dev) >>>> { >>>> struct sunxi_glue *glue = dev_get_priv(dev); >>>> struct musb_host_data *host = &glue->mdata; >>>> - struct usb_bus_priv *priv = dev_get_uclass_priv(dev); >>>> struct musb_hdrc_platform_data pdata; >>>> void *base = dev_read_addr_ptr(dev); >>>> int ret; >>>> +#ifdef CONFIG_USB_MUSB_HOST >>>> + struct usb_bus_priv *priv = dev_get_uclass_priv(dev); >>>> +#endif >>>> + >>>> if (!base) >>>> return -EINVAL; >>>> @@ -459,7 +462,6 @@ static int musb_usb_probe(struct udevice *dev) >>>> return ret; >>>> } >>>> - priv->desc_before_addr = true; >>> See my question at the beginning, and if that can be the case, the fix >>> is to check if priv is not null here, eg. >>> if (priv) >>> priv->... >>> >>> Still, why is the priv data not allocated for device ? >>> >>>> memset(&pdata, 0, sizeof(pdata)); >>>> pdata.power = 250; >>>> @@ -467,6 +469,8 @@ static int musb_usb_probe(struct udevice *dev) >>>> pdata.config = glue->cfg->config; >>>> #ifdef CONFIG_USB_MUSB_HOST >>>> + priv->desc_before_addr = true; >>>> + >>>> pdata.mode = MUSB_HOST; >>>> host->host = musb_init_controller(&pdata, &glue->dev, base); >>>> if (!host->host) >>>> >
On 12/13/2018 04:40 PM, Jean-Jacques Hiblot wrote: > > On 13/12/2018 15:05, Marek Vasut wrote: >> On 12/13/2018 03:03 PM, Jean-Jacques Hiblot wrote: >>> On 05/12/2018 13:57, Marek Vasut wrote: >>>> On 12/05/2018 01:49 PM, Stefan Mavrodiev wrote: >>>>> When the device is in peripheral mode >>>> Can you have two devices, one in peripheral mode and one in host mode, >>>> on the same system ? >>> It is possible with the musb-new. Using DM_USB and DM_USB_GADGET, I did >>> it with on a beaglebone black. >> Does this patch break it ? And if so, can you send a proper fix ? > > This file is not compiled when building for beaglebone, so it won't > break anything. Ah, sunxi, right. > BTW this driver could be adapted, with little work, to support a dynamic > selection of the role based on "dr-mode" in the DT. > > Not knowing the boards, I don't know if this work has any added value > though. It would, as it would remove ifdeffery.
On Thu, Dec 13, 2018 at 7:00 PM Marek Vasut <marex@denx.de> wrote: > > On 12/13/2018 08:14 AM, Stefan Mavrodiev wrote: > > > > On 12/5/18 3:16 PM, Marek Vasut wrote: > >> On 12/05/2018 02:06 PM, Stefan Mavrodiev wrote: > >>> On 12/5/18 2:57 PM, Marek Vasut wrote: > >>>> On 12/05/2018 01:49 PM, Stefan Mavrodiev wrote: > >>>>> When the device is in peripheral mode > >>>> Can you have two devices, one in peripheral mode and one in host mode, > >>>> on the same system ? > >>> Not 100% sure, but I'm thinking there is only one OTG port for > >>> all sunxi boards. The operation is decided in the Kconfig. > >> I'm rather sure I saw sunxi boards with more than one USB port. > >> > >>>>> there is no > >>>>> struct usb_bus_priv allocated pointer, as the uclass driver > >>>>> ("usb_dev_generic") doesn't call per_device_auto_alloc_size. > >>>>> > >>>>> This results in writing to the internal SDRAM at > >>>>> priv->desc_before_addr = true; > >>>>> > >>>>> Signed-off-by: Stefan Mavrodiev <stefan@olimex.com> > >>>>> --- > >>>>> drivers/usb/musb-new/sunxi.c | 8 ++++++-- > >>>>> 1 file changed, 6 insertions(+), 2 deletions(-) > >>>>> > >>>>> diff --git a/drivers/usb/musb-new/sunxi.c > >>>>> b/drivers/usb/musb-new/sunxi.c > >>>>> index 6cf9826cda..f3deb9bc66 100644 > >>>>> --- a/drivers/usb/musb-new/sunxi.c > >>>>> +++ b/drivers/usb/musb-new/sunxi.c > >>>>> @@ -435,11 +435,14 @@ static int musb_usb_probe(struct udevice *dev) > >>>>> { > >>>>> struct sunxi_glue *glue = dev_get_priv(dev); > >>>>> struct musb_host_data *host = &glue->mdata; > >>>>> - struct usb_bus_priv *priv = dev_get_uclass_priv(dev); > >>>>> struct musb_hdrc_platform_data pdata; > >>>>> void *base = dev_read_addr_ptr(dev); > >>>>> int ret; > >>>>> +#ifdef CONFIG_USB_MUSB_HOST > >>>>> + struct usb_bus_priv *priv = dev_get_uclass_priv(dev); > >>>>> +#endif > >>>>> + > >>>>> if (!base) > >>>>> return -EINVAL; > >>>>> @@ -459,7 +462,6 @@ static int musb_usb_probe(struct udevice *dev) > >>>>> return ret; > >>>>> } > >>>>> - priv->desc_before_addr = true; > >>>> See my question at the beginning, and if that can be the case, the fix > >>>> is to check if priv is not null here, eg. > >>>> if (priv) > >>>> priv->... > >>>> > >>>> Still, why is the priv data not allocated for device ? > >>> Depending on configuration, the device is registered ether as > >>> UCLASS_USB_DEV_GENERIC or UCLASS_USB. There is no > >>> > >>> .per_device_auto_alloc_size = sizeof(struct usb_bus_priv), > >>> > >>> for the second. (As seen in drivers/usb/host/usb-uclass.c) > >> I see the code is rather horrible. I'd expect all that configuration to > >> come from DT otg-mode property instead of being hard-wired into the > >> code. Sigh. > >> > >> Jagan, A-B ? I'd like to pick this . > >> > >>>>> memset(&pdata, 0, sizeof(pdata)); > >>>>> pdata.power = 250; > >>>>> @@ -467,6 +469,8 @@ static int musb_usb_probe(struct udevice *dev) > >>>>> pdata.config = glue->cfg->config; > >>>>> #ifdef CONFIG_USB_MUSB_HOST > >>>>> + priv->desc_before_addr = true; > >>>>> + > >>>>> pdata.mode = MUSB_HOST; > >>>>> host->host = musb_init_controller(&pdata, &glue->dev, base); > >>>>> if (!host->host) > >>>>> > >> > > Any further comments? > > As Jagan is inactive, applied. Sorry, I was in travel in multiple locations couldn't find this patch. Thanks for applying on your side.
diff --git a/drivers/usb/musb-new/sunxi.c b/drivers/usb/musb-new/sunxi.c index 6cf9826cda..f3deb9bc66 100644 --- a/drivers/usb/musb-new/sunxi.c +++ b/drivers/usb/musb-new/sunxi.c @@ -435,11 +435,14 @@ static int musb_usb_probe(struct udevice *dev) { struct sunxi_glue *glue = dev_get_priv(dev); struct musb_host_data *host = &glue->mdata; - struct usb_bus_priv *priv = dev_get_uclass_priv(dev); struct musb_hdrc_platform_data pdata; void *base = dev_read_addr_ptr(dev); int ret; +#ifdef CONFIG_USB_MUSB_HOST + struct usb_bus_priv *priv = dev_get_uclass_priv(dev); +#endif + if (!base) return -EINVAL; @@ -459,7 +462,6 @@ static int musb_usb_probe(struct udevice *dev) return ret; } - priv->desc_before_addr = true; memset(&pdata, 0, sizeof(pdata)); pdata.power = 250; @@ -467,6 +469,8 @@ static int musb_usb_probe(struct udevice *dev) pdata.config = glue->cfg->config; #ifdef CONFIG_USB_MUSB_HOST + priv->desc_before_addr = true; + pdata.mode = MUSB_HOST; host->host = musb_init_controller(&pdata, &glue->dev, base); if (!host->host)
When the device is in peripheral mode there is no struct usb_bus_priv allocated pointer, as the uclass driver ("usb_dev_generic") doesn't call per_device_auto_alloc_size. This results in writing to the internal SDRAM at priv->desc_before_addr = true; Signed-off-by: Stefan Mavrodiev <stefan@olimex.com> --- drivers/usb/musb-new/sunxi.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)