[U-Boot] [PATCH 1/1] usb: musb-new: sunxi: Fix null pointer access

[Jean-Jacques Hiblot]

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