[U-Boot] [UBOOT PATCH] usb: ehci-hcd: Fix crash when rootdev not initialized

[Simon Glass]

Hi,

On 27 June 2016 at 23:06, Siva Durga Prasad Paladugu
<siva.durga.paladugu at xilinx.com> wrote:
>
> Hi,
>
>> -----Original Message-----
>> From: Hans de Goede [mailto:hdegoede at redhat.com]
>> Sent: Monday, June 27, 2016 4:35 PM
>> To: Siva Durga Prasad Paladugu <sivadur at xilinx.com>; u-boot at lists.denx.de
>> Cc: marex at denx.de; Siva Durga Prasad Paladugu <sivadur at xilinx.com>
>> Subject: Re: [U-Boot] [UBOOT PATCH] usb: ehci-hcd: Fix crash when rootdev not
>> initialized
>>
>> Hi,
>>
>> On 27-06-16 10:59, Siva Durga Prasad Paladugu wrote:
>> > This patch fixes the issue on zynq USB failure with DM when rootdev is
>> > not initialized. This variable is initalized to zero in non driver
>> > model case but not in DM.
>> >
>> > Signed-off-by: Siva Durga Prasad Paladugu <sivadur at xilinx.com>
>> > ---
>> > - Tested on Zynq ZC702 board with USB DM driver model patches
>> >   sent by Simon.
>>
>> This patch does not seem like the right fix, normally something like the ehci_ctrl
>> struct would be allocated by dm by setting priv_auto_alloc_size in the driver
>> description, and then the data will get calloc-ed. If you're allocating the ehci_ctrl
>> struct differently and not memsetting it to 0 you may have other bugs lurking, or
>> may get new bugs when new members get added in the future.
> This is also fine for me, no Problem.
> I will reply to Simon on his patch of Zynq USB DM and will ask him to do so.
>
> Thanks,
> Siva
>
>>
>> My advice would be to use priv_auto_alloc_size, see .e.g :
>> drivers/usb/host/ehci-sunxi.c
>>
>> If you cannot use that for some reason, make sure to memset struct ehci_Ctrl to
>> 0 before calling ehci_register()

Agreed, but this is how EHCI works at present. Since EHCI is not a
uclass it cannot have its own private data. So the driver must
incorporate EHCI's private data in its own private data.

Yes this is error-prone. I'm not sure of a better solution at present,
but welcome ideas!

Reviewed-by: Simon Glass <sjg at chromium.org>

>>
>> Regards,
>>
>> Hans
>>
>>
>>
>> > ---
>> >  drivers/usb/host/ehci-hcd.c | 2 ++
>> >  1 file changed, 2 insertions(+)
>> >
>> > diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c
>> > index 13aa70d..8adffa6 100644
>> > --- a/drivers/usb/host/ehci-hcd.c
>> > +++ b/drivers/usb/host/ehci-hcd.c
>> > @@ -1624,6 +1624,8 @@ int ehci_register(struct udevice *dev, struct
>> ehci_hccr *hccr,
>> >                     goto err;
>> >     }
>> >
>> > +   ctrl->rootdev = 0;
>> > +
>> >     ret = ehci_common_init(ctrl, tweaks);
>> >     if (ret)
>> >             goto err;
>> >