* [U-Boot] [PATCH 1/1] usb: musb-new: sunxi: Fix null pointer access
@ 2018-12-05 12:49 Stefan Mavrodiev
2018-12-05 12:57 ` Marek Vasut
0 siblings, 1 reply; 12+ messages in thread
From: Stefan Mavrodiev @ 2018-12-05 12:49 UTC (permalink / raw)
To: u-boot
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(-)
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)
--
2.17.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [U-Boot] [PATCH 1/1] usb: musb-new: sunxi: Fix null pointer access
2018-12-05 12:49 [U-Boot] [PATCH 1/1] usb: musb-new: sunxi: Fix null pointer access Stefan Mavrodiev
@ 2018-12-05 12:57 ` Marek Vasut
2018-12-05 13:06 ` Stefan Mavrodiev
` (2 more replies)
0 siblings, 3 replies; 12+ messages in thread
From: Marek Vasut @ 2018-12-05 12:57 UTC (permalink / raw)
To: u-boot
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)
>
--
Best regards,
Marek Vasut
^ permalink raw reply [flat|nested] 12+ messages in thread
* [U-Boot] [PATCH 1/1] usb: musb-new: sunxi: Fix null pointer access
2018-12-05 12:57 ` Marek Vasut
@ 2018-12-05 13:06 ` Stefan Mavrodiev
2018-12-05 13:16 ` Marek Vasut
2018-12-05 13:11 ` Maxime Ripard
2018-12-13 14:03 ` Jean-Jacques Hiblot
2 siblings, 1 reply; 12+ messages in thread
From: Stefan Mavrodiev @ 2018-12-05 13:06 UTC (permalink / raw)
To: u-boot
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)
>>
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* [U-Boot] [PATCH 1/1] usb: musb-new: sunxi: Fix null pointer access
2018-12-05 12:57 ` Marek Vasut
2018-12-05 13:06 ` Stefan Mavrodiev
@ 2018-12-05 13:11 ` Maxime Ripard
2018-12-13 14:03 ` Jean-Jacques Hiblot
2 siblings, 0 replies; 12+ messages in thread
From: Maxime Ripard @ 2018-12-05 13:11 UTC (permalink / raw)
To: u-boot
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
--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 228 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20181205/29062adb/attachment.sig>
^ permalink raw reply [flat|nested] 12+ messages in thread
* [U-Boot] [PATCH 1/1] usb: musb-new: sunxi: Fix null pointer access
2018-12-05 13:06 ` Stefan Mavrodiev
@ 2018-12-05 13:16 ` Marek Vasut
2018-12-13 7:14 ` Stefan Mavrodiev
0 siblings, 1 reply; 12+ messages in thread
From: Marek Vasut @ 2018-12-05 13:16 UTC (permalink / raw)
To: u-boot
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)
>>>
>>
--
Best regards,
Marek Vasut
^ permalink raw reply [flat|nested] 12+ messages in thread
* [U-Boot] [PATCH 1/1] usb: musb-new: sunxi: Fix null pointer access
2018-12-05 13:16 ` Marek Vasut
@ 2018-12-13 7:14 ` Stefan Mavrodiev
2018-12-13 13:28 ` Marek Vasut
0 siblings, 1 reply; 12+ messages in thread
From: Stefan Mavrodiev @ 2018-12-13 7:14 UTC (permalink / raw)
To: u-boot
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?
^ permalink raw reply [flat|nested] 12+ messages in thread
* [U-Boot] [PATCH 1/1] usb: musb-new: sunxi: Fix null pointer access
2018-12-13 7:14 ` Stefan Mavrodiev
@ 2018-12-13 13:28 ` Marek Vasut
2018-12-14 10:14 ` Jagan Teki
0 siblings, 1 reply; 12+ messages in thread
From: Marek Vasut @ 2018-12-13 13:28 UTC (permalink / raw)
To: u-boot
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.
--
Best regards,
Marek Vasut
^ permalink raw reply [flat|nested] 12+ messages in thread
* [U-Boot] [PATCH 1/1] usb: musb-new: sunxi: Fix null pointer access
2018-12-05 12:57 ` Marek Vasut
2018-12-05 13:06 ` Stefan Mavrodiev
2018-12-05 13:11 ` Maxime Ripard
@ 2018-12-13 14:03 ` Jean-Jacques Hiblot
2018-12-13 14:05 ` Marek Vasut
2 siblings, 1 reply; 12+ messages in thread
From: Jean-Jacques Hiblot @ 2018-12-13 14:03 UTC (permalink / raw)
To: u-boot
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)
>>
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* [U-Boot] [PATCH 1/1] usb: musb-new: sunxi: Fix null pointer access
2018-12-13 14:03 ` Jean-Jacques Hiblot
@ 2018-12-13 14:05 ` Marek Vasut
2018-12-13 15:40 ` Jean-Jacques Hiblot
0 siblings, 1 reply; 12+ messages in thread
From: Marek Vasut @ 2018-12-13 14:05 UTC (permalink / raw)
To: u-boot
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)
>>>
>>
--
Best regards,
Marek Vasut
^ permalink raw reply [flat|nested] 12+ messages in thread
* [U-Boot] [PATCH 1/1] usb: musb-new: sunxi: Fix null pointer access
2018-12-13 14:05 ` Marek Vasut
@ 2018-12-13 15:40 ` Jean-Jacques Hiblot
2018-12-13 15:49 ` Marek Vasut
0 siblings, 1 reply; 12+ messages in thread
From: Jean-Jacques Hiblot @ 2018-12-13 15:40 UTC (permalink / raw)
To: u-boot
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)
>>>>
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* [U-Boot] [PATCH 1/1] usb: musb-new: sunxi: Fix null pointer access
2018-12-13 15:40 ` Jean-Jacques Hiblot
@ 2018-12-13 15:49 ` Marek Vasut
0 siblings, 0 replies; 12+ messages in thread
From: Marek Vasut @ 2018-12-13 15:49 UTC (permalink / raw)
To: u-boot
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.
--
Best regards,
Marek Vasut
^ permalink raw reply [flat|nested] 12+ messages in thread
* [U-Boot] [PATCH 1/1] usb: musb-new: sunxi: Fix null pointer access
2018-12-13 13:28 ` Marek Vasut
@ 2018-12-14 10:14 ` Jagan Teki
0 siblings, 0 replies; 12+ messages in thread
From: Jagan Teki @ 2018-12-14 10:14 UTC (permalink / raw)
To: u-boot
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.
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2018-12-14 10:14 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-12-05 12:49 [U-Boot] [PATCH 1/1] usb: musb-new: sunxi: Fix null pointer access Stefan Mavrodiev
2018-12-05 12:57 ` Marek Vasut
2018-12-05 13:06 ` Stefan Mavrodiev
2018-12-05 13:16 ` Marek Vasut
2018-12-13 7:14 ` Stefan Mavrodiev
2018-12-13 13:28 ` Marek Vasut
2018-12-14 10:14 ` Jagan Teki
2018-12-05 13:11 ` Maxime Ripard
2018-12-13 14:03 ` Jean-Jacques Hiblot
2018-12-13 14:05 ` Marek Vasut
2018-12-13 15:40 ` Jean-Jacques Hiblot
2018-12-13 15:49 ` Marek Vasut
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.