public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
From: Hans de Goede <hdegoede@redhat.com>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH v3] dm: usb: Copy over usb_device values from usb_scan_device() to final usb_device
Date: Tue, 05 May 2015 11:49:06 +0200	[thread overview]
Message-ID: <55489212.3080602@redhat.com> (raw)
In-Reply-To: <CAPnjgZ3G6+Y=xCwZ0D-89_To==rhYX1x8ukbVVvx8z1k50rtoQ@mail.gmail.com>

Hi,

On 04-05-15 23:35, Simon Glass wrote:
> On 4 May 2015 at 15:35, Simon Glass <sjg@chromium.org> wrote:
>> Hi Hans,
>>
>> On 4 May 2015 at 14:28, Simon Glass <sjg@chromium.org> wrote:
>>> On 4 May 2015 at 11:19, Hans de Goede <hdegoede@redhat.com> wrote:
>>>> Currently we copy over a number of usb_device values stored in the on stack
>>>> struct usb_device probed in usb_scan_device() to the final driver-model managed
>>>> struct usb_device in usb_child_pre_probe() through usb_device_platdata, and
>>>> then call usb_select_config() to fill in the rest.
>>>>
>>>> There are 3 problems with this approach:
>>>>
>>>> 1) It does not fill in enough fields before calling usb_select_config(),
>>>> specifically it does not fill in ep0's maxpacketsize causing a div by zero
>>>> exception in the ehci driver.
>>>>
>>>> 2) It unnecessarily redoes a number of usb requests making usb probing slower
>>>>
>>>> 3) Calling usb_select_config() a second time fails on some usb-1 devices
>>>> plugged into usb-2 hubs, causing u-boot to not recognize these devices.
>>>>
>>>> This commit fixes these issues by removing the usb_select_config() call from
>>>> usb_child_pre_probe(), and instead of copying over things field by field
>>>> through usb_device_platdata, store a pointer to the in stack usb_device
>>>> (which is still valid when usb_child_pre_probe() gets called) and copy
>>>> over the entire struct.
>>>>
>>>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>>>
>>> Acked-by: Simon Glass <sjg@chromium.org>
>>
>> But sadly this fails with sandbox. Please do run the driver model tests.
>
> Output:
>
> 02: dm: usb: Copy over usb_device values from usb_scan_device() to
> final usb_device
>     sandbox:  +   sandbox
> +../drivers/usb/emul/usb-emul-uclass.c: In function ?usb_emul_find?:
> +../drivers/usb/emul/usb-emul-uclass.c:125:11: error: ?struct
> usb_dev_platdata? has no member named ?devnum?
> +   if (udev->devnum == devnum) {
> +           ^
> +In file included from ../drivers/usb/emul/usb-emul-uclass.c:8:0:
> +../drivers/usb/emul/usb-emul-uclass.c:127:25: error: ?struct
> usb_dev_platdata? has no member named ?devnum?
> +          dev->name, udev->devnum);
> +                         ^
> +../include/common.h:109:26: note: in definition of macro ?debug_cond?
> +    printf(pr_fmt(fmt), ##args); \
> +                          ^
> +../drivers/usb/emul/usb-emul-uclass.c:126:4: note: in expansion of
> macro ?debug?
> +    debug("%s: Found emulator '%s', addr %d\n", __func__,
> +    ^
> +../drivers/usb/emul/usb-emul-uclass.c: In function ?usb_emul_control?:
> +../drivers/usb/emul/usb-emul-uclass.c:168:8: error: ?struct
> usb_dev_platdata? has no member named ?devnum?
> +    plat->devnum = setup->value;
> +        ^
> +../drivers/usb/emul/usb-emul-uclass.c: In function ?usb_emul_reset?:
> +../drivers/usb/emul/usb-emul-uclass.c:253:6: error: ?struct
> usb_dev_platdata? has no member named ?devnum?
> +  plat->devnum = 0;
> +      ^


Oops sorry about that, I'll try to learn myself to always run dm-test.sh
after making dm related changes.

Fixing this was a bit trickier then I would have liked, but I've it fixed
now. I also had to reshuffle the order of some patches for this, so I'm
going to post a v4 of the entire set. I've also added some extra comments
(but no functional changes) to the sunxi ehci dm patch as request by Ian,
so please take all patches from the v4 posting when merging.

Regards,

Hans

  reply	other threads:[~2015-05-05  9:49 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-05-04 17:19 [U-Boot] [PATCH v3 0/1] dm: usb: Copy over usb_device values from usb_scan_device() to final usb_device Hans de Goede
2015-05-04 17:19 ` [U-Boot] [PATCH v3] " Hans de Goede
2015-05-04 20:28   ` Simon Glass
2015-05-04 21:35     ` Simon Glass
2015-05-04 21:35       ` Simon Glass
2015-05-05  9:49         ` Hans de Goede [this message]
2015-05-04 17:22 ` [U-Boot] [PATCH v3 0/1] " Simon Glass
2015-05-04 19:12   ` Hans de Goede
2015-05-04 19:24     ` Simon Glass

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=55489212.3080602@redhat.com \
    --to=hdegoede@redhat.com \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox