diff mbox

[U-Boot] Please pull u-boot-dm.git [take 2]

Message ID 55316186.8090204@redhat.com
State RFC
Delegated to: Simon Glass
Headers show

Commit Message

Hans de Goede April 17, 2015, 7:39 p.m. UTC
Hi,

On 17-04-15 21:28, Hans de Goede wrote:
> Hi,
>
> On 17-04-15 19:53, Tom Rini wrote:
>> On Fri, Apr 17, 2015 at 10:54:21AM -0600, Simon Glass wrote:
>>> Hi Tom,
>>>
>>> On 17 April 2015 at 10:46, Tom Rini <trini@konsulko.com> wrote:
>>>> On Fri, Apr 17, 2015 at 10:30:36AM -0600, Simon Glass wrote:
>>>>> Hi Tom,
>>>>>
>>>>> On 17 April 2015 at 10:27, Tom Rini <trini@konsulko.com> wrote:
>>>>>>
>>>>>> On Thu, Apr 16, 2015 at 09:15:26PM -0600, Simon Glass wrote:
>>>>>>
>>>>>>> Hi Tom,
>>>>>>>
>>>>>>> As mentioned I reverted this patch as it conflicted with the dm tree
>>>>>>> and I suspect it might be buggy:
>>>>>>>
>>>>>>> cd749658 usb_storage : scan all interfaces to find a storage device
>>>>>>>
>>>>>>> Assuming this is OK and applies successfully I will rebase and resend
>>>>>>> this patch, then reply with some comments I have on the patch.
>>>>>>>
>>>>>>>
>>>>>>> The following changes since commit 4564faeafbf11feb839e2e3f927be2f1a919ba96:
>>>>>>>
>>>>>>>    ti: dwc3: Enable clocks in enable_basic_clocks() in hw_data.c
>>>>>>> (2015-04-16 15:08:36 -0400)
>>>>>>>
>>>>>>> are available in the git repository at:
>>>>>>>
>>>>>>>    http://git.denx.de/u-boot-dm.git
>>>>>>>
>>>>>>> for you to fetch changes up to 2e6263093b3a5c2d2c586afaedfd346d6628f784:
>>>>>>>
>>>>>>>    sandbox: exynos: Move CONFIG_SOUND_SANDBOX to Kconfig (2015-04-16
>>>>>>> 20:47:57 -0600)
>>>>>>
>>>>>> With this PR, am335x_boneblack (which has DM enabled) no longer
>>>>>> functions.  I'm running a bisect now, but heads up.
>>>>>
>>>>> OK, I'll wait for your bisect - also what is the console output when it breaks?
>>>>
>>>> OK, disregard Beaglebone Black error, that was me (need to remember that
>>>> eMMC boot only works off of am335x_boneblack_config).  But,
>>>> A20-OLinuXino-Lime2_defconfig is broken by this.  The error log:
>>>> Command(A20 OLinuXino Lime2 console)> on
>>>> (user:trini) Power turned on
>>>>
>>>> U-Boot SPL 2015.04-00342-g2e62630 (Apr 17 2015 - 12:40:45)
>>>> DRAM: 1024 MiB
>>>> CPU: 912000000Hz, AXI/AHB/APB: 3/2/2
>>>>
>>>>
>>>> U-Boot 2015.04-00342-g2e62630 (Apr 17 2015 - 12:40:45) Allwinner Technology
>>>>
>>>> CPU:   Allwinner A20 (SUN7I)
>>>> I2C:   ready
>>>> DRAM:  1 GiB
>>>> MMC:   SUNXI SD/MMC: 0
>>>> *** Warning - bad CRC, using default environment
>>>>
>>>> In:    serial
>>>> Out:   serial
>>>> Err:   serial
>>>> SCSI:  SUNXI SCSI INIT
>>>> SATA link 0 timeout.
>>>> AHCI 0001.0100 32 slots 1 ports 3 Gbps 0x1 impl SATA mode
>>>> flags: ncq stag pm led clo only pmp pio slum part ccc apst
>>>> Net:   dwmac.1c50000
>>>> starting USB...
>>>> USB0:   USB EHCI 1.00
>>>> scanning bus 0 for devices...
>>>> U-Boot SPL 2015.04-00342-g2e62630 (Apr 17 2015 - 12:40:45)
>>>> DRAM: 1024 MiB
>>>> CPU: 912000000Hz, AXI/AHB/APB: 3/2/2
>>>> MMC Device 0 not found
>>>> spl: mmc device not found!!
>>>> ### ERROR ### Please RESET the board ###
>>>>
>>>> And good:
>>>> Command(A20 OLinuXino Lime2 console)> on
>>>> (user:trini) Power turned on
>>>>
>>>> U-Boot SPL 2015.04-00121-g4564fae (Apr 17 2015 - 12:42:40)
>>>> DRAM: 1024 MiB
>>>> CPU: 912000000Hz, AXI/AHB/APB: 3/2/2
>>>>
>>>>
>>>> U-Boot 2015.04-00121-g4564fae (Apr 17 2015 - 12:42:40) Allwinner Technology
>>>>
>>>> CPU:   Allwinner A20 (SUN7I)
>>>> I2C:   ready
>>>> DRAM:  1 GiB
>>>> MMC:   SUNXI SD/MMC: 0
>>>> *** Warning - bad CRC, using default environment
>>>>
>>>> In:    serial
>>>> Out:   serial
>>>> Err:   serial
>>>> SCSI:  SUNXI SCSI INIT
>>>> SATA link 0 timeout.
>>>> AHCI 0001.0100 32 slots 1 ports 3 Gbps 0x1 impl SATA mode
>>>> flags: ncq stag pm led clo only pmp pio slum part ccc apst
>>>> Net:   dwmac.1c50000
>>>> starting USB...
>>>> USB0:   USB EHCI 1.00
>>>> scanning bus 0 for devices... 1 USB Device(s) found
>>>> USB1:   USB EHCI 1.00
>>>> scanning bus 1 for devices... 1 USB Device(s) found
>>>>         scanning usb for storage devices... 0 Storage Device(s) found
>>>> Hit any key to stop autoboot:  0
>>>> sunxi#
>>>>
>>>> So... we cause a reset during USB scan and then fail to boot a second
>>>> time?
>>>
>>> Thanks for testing this.
>>>
>>> I'm not sure if I have a Lime (I might do, will need to look). I was
>>> testing sunxi on a pcduino3 but USB does not work on that for reasons
>>> I was looking into (it just says lowlevel init failure due to moving
>>> to driver model for GPIO).
>>>
>>> I will take a look. In the meantime, which commit breaks this? I could
>>> perhaps issue a pull for commits up to that point to reduce the load.
>>
>> I haven't had a chance to bisect yet but I will in a few hours.
>
> No need to, I've just completed a bisect, it points to:
>
> 5bca5a6303f3526ab2cf9c0a62cd26c16e0d5c2f is the first bad commit
> commit 5bca5a6303f3526ab2cf9c0a62cd26c16e0d5c2f
> Author: Simon Glass <sjg@chromium.org>
> Date:   Wed Mar 25 12:22:27 2015 -0600
>
>      dm: usb: Drop the EHCI weak functions
>
>      These are a pain with driver model because we might have different EHCI
>      drivers which want to implement them differently. Now that they use
>      consistent function signatures, we can in good conscience move them to
>      a struct.
>
>      Signed-off-by: Simon Glass <sjg@chromium.org>
>      Reviewed-by: Marek Vasut <marex@denx.de>
>
> I'm going to first spend some time with my family now, I may look into
> this later tonight, or otherwise this weekend. I'll be sure to check mail
> first to avoid double work, so feel free to fix the problem while I'm
> relaxing :)

Ok, so I could not help myself and took a quick look at the patch causing the
issue, this fixes the reset on usb scan problem:

And should probably be squashed into the original patch to avoid bisect
problems.

But with this in place, all is still not well wrt non devicetree usb,
usb keyboard support does not work, "usb tree" says:

USB device tree:
   1  Hub (480 Mb/s, 0mA)
   |  u-boot EHCI Host Controller
   |
   +-2  Hub (480 Mb/s, 100mA)
     |
     +-3  Hub (12 Mb/s, 100mA)
       |
       | -1  See Interface (12 Mb/s, 0mA)
       |

Note the -1 as device number for the "See Interface" device.

This particular usb setup used to work fine.

I guess this is another issue to git bisect, no idea when I'll get around
to that.

Regards,

Hans





>
> Regards,
>
> Hans
>
>

Comments

Tom Rini April 17, 2015, 8:41 p.m. UTC | #1
On Fri, Apr 17, 2015 at 09:39:50PM +0200, Hans de Goede wrote:
> Hi,
> 
> On 17-04-15 21:28, Hans de Goede wrote:
> >Hi,
> >
> >On 17-04-15 19:53, Tom Rini wrote:
> >>On Fri, Apr 17, 2015 at 10:54:21AM -0600, Simon Glass wrote:
> >>>Hi Tom,
> >>>
> >>>On 17 April 2015 at 10:46, Tom Rini <trini@konsulko.com> wrote:
> >>>>On Fri, Apr 17, 2015 at 10:30:36AM -0600, Simon Glass wrote:
> >>>>>Hi Tom,
> >>>>>
> >>>>>On 17 April 2015 at 10:27, Tom Rini <trini@konsulko.com> wrote:
> >>>>>>
> >>>>>>On Thu, Apr 16, 2015 at 09:15:26PM -0600, Simon Glass wrote:
> >>>>>>
> >>>>>>>Hi Tom,
> >>>>>>>
> >>>>>>>As mentioned I reverted this patch as it conflicted with the dm tree
> >>>>>>>and I suspect it might be buggy:
> >>>>>>>
> >>>>>>>cd749658 usb_storage : scan all interfaces to find a storage device
> >>>>>>>
> >>>>>>>Assuming this is OK and applies successfully I will rebase and resend
> >>>>>>>this patch, then reply with some comments I have on the patch.
> >>>>>>>
> >>>>>>>
> >>>>>>>The following changes since commit 4564faeafbf11feb839e2e3f927be2f1a919ba96:
> >>>>>>>
> >>>>>>>   ti: dwc3: Enable clocks in enable_basic_clocks() in hw_data.c
> >>>>>>>(2015-04-16 15:08:36 -0400)
> >>>>>>>
> >>>>>>>are available in the git repository at:
> >>>>>>>
> >>>>>>>   http://git.denx.de/u-boot-dm.git
> >>>>>>>
> >>>>>>>for you to fetch changes up to 2e6263093b3a5c2d2c586afaedfd346d6628f784:
> >>>>>>>
> >>>>>>>   sandbox: exynos: Move CONFIG_SOUND_SANDBOX to Kconfig (2015-04-16
> >>>>>>>20:47:57 -0600)
> >>>>>>
> >>>>>>With this PR, am335x_boneblack (which has DM enabled) no longer
> >>>>>>functions.  I'm running a bisect now, but heads up.
> >>>>>
> >>>>>OK, I'll wait for your bisect - also what is the console output when it breaks?
> >>>>
> >>>>OK, disregard Beaglebone Black error, that was me (need to remember that
> >>>>eMMC boot only works off of am335x_boneblack_config).  But,
> >>>>A20-OLinuXino-Lime2_defconfig is broken by this.  The error log:
> >>>>Command(A20 OLinuXino Lime2 console)> on
> >>>>(user:trini) Power turned on
> >>>>
> >>>>U-Boot SPL 2015.04-00342-g2e62630 (Apr 17 2015 - 12:40:45)
> >>>>DRAM: 1024 MiB
> >>>>CPU: 912000000Hz, AXI/AHB/APB: 3/2/2
> >>>>
> >>>>
> >>>>U-Boot 2015.04-00342-g2e62630 (Apr 17 2015 - 12:40:45) Allwinner Technology
> >>>>
> >>>>CPU:   Allwinner A20 (SUN7I)
> >>>>I2C:   ready
> >>>>DRAM:  1 GiB
> >>>>MMC:   SUNXI SD/MMC: 0
> >>>>*** Warning - bad CRC, using default environment
> >>>>
> >>>>In:    serial
> >>>>Out:   serial
> >>>>Err:   serial
> >>>>SCSI:  SUNXI SCSI INIT
> >>>>SATA link 0 timeout.
> >>>>AHCI 0001.0100 32 slots 1 ports 3 Gbps 0x1 impl SATA mode
> >>>>flags: ncq stag pm led clo only pmp pio slum part ccc apst
> >>>>Net:   dwmac.1c50000
> >>>>starting USB...
> >>>>USB0:   USB EHCI 1.00
> >>>>scanning bus 0 for devices...
> >>>>U-Boot SPL 2015.04-00342-g2e62630 (Apr 17 2015 - 12:40:45)
> >>>>DRAM: 1024 MiB
> >>>>CPU: 912000000Hz, AXI/AHB/APB: 3/2/2
> >>>>MMC Device 0 not found
> >>>>spl: mmc device not found!!
> >>>>### ERROR ### Please RESET the board ###
> >>>>
> >>>>And good:
> >>>>Command(A20 OLinuXino Lime2 console)> on
> >>>>(user:trini) Power turned on
> >>>>
> >>>>U-Boot SPL 2015.04-00121-g4564fae (Apr 17 2015 - 12:42:40)
> >>>>DRAM: 1024 MiB
> >>>>CPU: 912000000Hz, AXI/AHB/APB: 3/2/2
> >>>>
> >>>>
> >>>>U-Boot 2015.04-00121-g4564fae (Apr 17 2015 - 12:42:40) Allwinner Technology
> >>>>
> >>>>CPU:   Allwinner A20 (SUN7I)
> >>>>I2C:   ready
> >>>>DRAM:  1 GiB
> >>>>MMC:   SUNXI SD/MMC: 0
> >>>>*** Warning - bad CRC, using default environment
> >>>>
> >>>>In:    serial
> >>>>Out:   serial
> >>>>Err:   serial
> >>>>SCSI:  SUNXI SCSI INIT
> >>>>SATA link 0 timeout.
> >>>>AHCI 0001.0100 32 slots 1 ports 3 Gbps 0x1 impl SATA mode
> >>>>flags: ncq stag pm led clo only pmp pio slum part ccc apst
> >>>>Net:   dwmac.1c50000
> >>>>starting USB...
> >>>>USB0:   USB EHCI 1.00
> >>>>scanning bus 0 for devices... 1 USB Device(s) found
> >>>>USB1:   USB EHCI 1.00
> >>>>scanning bus 1 for devices... 1 USB Device(s) found
> >>>>        scanning usb for storage devices... 0 Storage Device(s) found
> >>>>Hit any key to stop autoboot:  0
> >>>>sunxi#
> >>>>
> >>>>So... we cause a reset during USB scan and then fail to boot a second
> >>>>time?
> >>>
> >>>Thanks for testing this.
> >>>
> >>>I'm not sure if I have a Lime (I might do, will need to look). I was
> >>>testing sunxi on a pcduino3 but USB does not work on that for reasons
> >>>I was looking into (it just says lowlevel init failure due to moving
> >>>to driver model for GPIO).
> >>>
> >>>I will take a look. In the meantime, which commit breaks this? I could
> >>>perhaps issue a pull for commits up to that point to reduce the load.
> >>
> >>I haven't had a chance to bisect yet but I will in a few hours.
> >
> >No need to, I've just completed a bisect, it points to:
> >
> >5bca5a6303f3526ab2cf9c0a62cd26c16e0d5c2f is the first bad commit
> >commit 5bca5a6303f3526ab2cf9c0a62cd26c16e0d5c2f
> >Author: Simon Glass <sjg@chromium.org>
> >Date:   Wed Mar 25 12:22:27 2015 -0600
> >
> >     dm: usb: Drop the EHCI weak functions
> >
> >     These are a pain with driver model because we might have different EHCI
> >     drivers which want to implement them differently. Now that they use
> >     consistent function signatures, we can in good conscience move them to
> >     a struct.
> >
> >     Signed-off-by: Simon Glass <sjg@chromium.org>
> >     Reviewed-by: Marek Vasut <marex@denx.de>
> >
> >I'm going to first spend some time with my family now, I may look into
> >this later tonight, or otherwise this weekend. I'll be sure to check mail
> >first to avoid double work, so feel free to fix the problem while I'm
> >relaxing :)
> 
> Ok, so I could not help myself and took a quick look at the patch causing the
> issue, this fixes the reset on usb scan problem:
> 
> diff --git a/drivers/usb/host/ehci-sunxi.c b/drivers/usb/host/ehci-sunxi.c
> index eda9f69..a847ac5 100644
> --- a/drivers/usb/host/ehci-sunxi.c
> +++ b/drivers/usb/host/ehci-sunxi.c
> @@ -34,6 +34,8 @@ int ehci_hcd_init(int index, enum usb_init_type init, struct ehci_hccr **hccr,
>               (uint32_t)*hccr, (uint32_t)*hcor,
>               (uint32_t)HC_LENGTH(ehci_readl(&(*hccr)->cr_capbase)));
> 
> + ehci_set_controller_priv(index, NULL, NULL);
> +
>         return 0;
>  }
> 
> And should probably be squashed into the original patch to avoid bisect
> problems.
> 
> But with this in place, all is still not well wrt non devicetree usb,
> usb keyboard support does not work, "usb tree" says:
> 
> USB device tree:
>   1  Hub (480 Mb/s, 0mA)
>   |  u-boot EHCI Host Controller
>   |
>   +-2  Hub (480 Mb/s, 100mA)
>     |
>     +-3  Hub (12 Mb/s, 100mA)
>       |
>       | -1  See Interface (12 Mb/s, 0mA)
>       |
> 
> Note the -1 as device number for the "See Interface" device.
> 
> This particular usb setup used to work fine.
> 
> I guess this is another issue to git bisect, no idea when I'll get around
> to that.

I see that the mailing lists are hiccuping right now.  I also found the
same problem and a quick solution, but whacking ehci-sunxi.c means all
of the others need it too.  I reworked usb_lowlevel_init so that
everyone gets the defaults and then can override as needed.

The USB keyboard I plugged in shows a problem with the DM series and a
different problem on master but I think it's probably the same problem
so I'll bisect it down shortly.
Tom Rini April 17, 2015, 9:13 p.m. UTC | #2
On Fri, Apr 17, 2015 at 09:39:50PM +0200, Hans de Goede wrote:
> Hi,
> 
> On 17-04-15 21:28, Hans de Goede wrote:
> >Hi,
> >
> >On 17-04-15 19:53, Tom Rini wrote:
> >>On Fri, Apr 17, 2015 at 10:54:21AM -0600, Simon Glass wrote:
> >>>Hi Tom,
> >>>
> >>>On 17 April 2015 at 10:46, Tom Rini <trini@konsulko.com> wrote:
> >>>>On Fri, Apr 17, 2015 at 10:30:36AM -0600, Simon Glass wrote:
> >>>>>Hi Tom,
> >>>>>
> >>>>>On 17 April 2015 at 10:27, Tom Rini <trini@konsulko.com> wrote:
> >>>>>>
> >>>>>>On Thu, Apr 16, 2015 at 09:15:26PM -0600, Simon Glass wrote:
> >>>>>>
> >>>>>>>Hi Tom,
> >>>>>>>
> >>>>>>>As mentioned I reverted this patch as it conflicted with the dm tree
> >>>>>>>and I suspect it might be buggy:
> >>>>>>>
> >>>>>>>cd749658 usb_storage : scan all interfaces to find a storage device
> >>>>>>>
> >>>>>>>Assuming this is OK and applies successfully I will rebase and resend
> >>>>>>>this patch, then reply with some comments I have on the patch.
> >>>>>>>
> >>>>>>>
> >>>>>>>The following changes since commit 4564faeafbf11feb839e2e3f927be2f1a919ba96:
> >>>>>>>
> >>>>>>>   ti: dwc3: Enable clocks in enable_basic_clocks() in hw_data.c
> >>>>>>>(2015-04-16 15:08:36 -0400)
> >>>>>>>
> >>>>>>>are available in the git repository at:
> >>>>>>>
> >>>>>>>   http://git.denx.de/u-boot-dm.git
> >>>>>>>
> >>>>>>>for you to fetch changes up to 2e6263093b3a5c2d2c586afaedfd346d6628f784:
> >>>>>>>
> >>>>>>>   sandbox: exynos: Move CONFIG_SOUND_SANDBOX to Kconfig (2015-04-16
> >>>>>>>20:47:57 -0600)
> >>>>>>
> >>>>>>With this PR, am335x_boneblack (which has DM enabled) no longer
> >>>>>>functions.  I'm running a bisect now, but heads up.
> >>>>>
> >>>>>OK, I'll wait for your bisect - also what is the console output when it breaks?
> >>>>
> >>>>OK, disregard Beaglebone Black error, that was me (need to remember that
> >>>>eMMC boot only works off of am335x_boneblack_config).  But,
> >>>>A20-OLinuXino-Lime2_defconfig is broken by this.  The error log:
> >>>>Command(A20 OLinuXino Lime2 console)> on
> >>>>(user:trini) Power turned on
> >>>>
> >>>>U-Boot SPL 2015.04-00342-g2e62630 (Apr 17 2015 - 12:40:45)
> >>>>DRAM: 1024 MiB
> >>>>CPU: 912000000Hz, AXI/AHB/APB: 3/2/2
> >>>>
> >>>>
> >>>>U-Boot 2015.04-00342-g2e62630 (Apr 17 2015 - 12:40:45) Allwinner Technology
> >>>>
> >>>>CPU:   Allwinner A20 (SUN7I)
> >>>>I2C:   ready
> >>>>DRAM:  1 GiB
> >>>>MMC:   SUNXI SD/MMC: 0
> >>>>*** Warning - bad CRC, using default environment
> >>>>
> >>>>In:    serial
> >>>>Out:   serial
> >>>>Err:   serial
> >>>>SCSI:  SUNXI SCSI INIT
> >>>>SATA link 0 timeout.
> >>>>AHCI 0001.0100 32 slots 1 ports 3 Gbps 0x1 impl SATA mode
> >>>>flags: ncq stag pm led clo only pmp pio slum part ccc apst
> >>>>Net:   dwmac.1c50000
> >>>>starting USB...
> >>>>USB0:   USB EHCI 1.00
> >>>>scanning bus 0 for devices...
> >>>>U-Boot SPL 2015.04-00342-g2e62630 (Apr 17 2015 - 12:40:45)
> >>>>DRAM: 1024 MiB
> >>>>CPU: 912000000Hz, AXI/AHB/APB: 3/2/2
> >>>>MMC Device 0 not found
> >>>>spl: mmc device not found!!
> >>>>### ERROR ### Please RESET the board ###
> >>>>
> >>>>And good:
> >>>>Command(A20 OLinuXino Lime2 console)> on
> >>>>(user:trini) Power turned on
> >>>>
> >>>>U-Boot SPL 2015.04-00121-g4564fae (Apr 17 2015 - 12:42:40)
> >>>>DRAM: 1024 MiB
> >>>>CPU: 912000000Hz, AXI/AHB/APB: 3/2/2
> >>>>
> >>>>
> >>>>U-Boot 2015.04-00121-g4564fae (Apr 17 2015 - 12:42:40) Allwinner Technology
> >>>>
> >>>>CPU:   Allwinner A20 (SUN7I)
> >>>>I2C:   ready
> >>>>DRAM:  1 GiB
> >>>>MMC:   SUNXI SD/MMC: 0
> >>>>*** Warning - bad CRC, using default environment
> >>>>
> >>>>In:    serial
> >>>>Out:   serial
> >>>>Err:   serial
> >>>>SCSI:  SUNXI SCSI INIT
> >>>>SATA link 0 timeout.
> >>>>AHCI 0001.0100 32 slots 1 ports 3 Gbps 0x1 impl SATA mode
> >>>>flags: ncq stag pm led clo only pmp pio slum part ccc apst
> >>>>Net:   dwmac.1c50000
> >>>>starting USB...
> >>>>USB0:   USB EHCI 1.00
> >>>>scanning bus 0 for devices... 1 USB Device(s) found
> >>>>USB1:   USB EHCI 1.00
> >>>>scanning bus 1 for devices... 1 USB Device(s) found
> >>>>        scanning usb for storage devices... 0 Storage Device(s) found
> >>>>Hit any key to stop autoboot:  0
> >>>>sunxi#
> >>>>
> >>>>So... we cause a reset during USB scan and then fail to boot a second
> >>>>time?
> >>>
> >>>Thanks for testing this.
> >>>
> >>>I'm not sure if I have a Lime (I might do, will need to look). I was
> >>>testing sunxi on a pcduino3 but USB does not work on that for reasons
> >>>I was looking into (it just says lowlevel init failure due to moving
> >>>to driver model for GPIO).
> >>>
> >>>I will take a look. In the meantime, which commit breaks this? I could
> >>>perhaps issue a pull for commits up to that point to reduce the load.
> >>
> >>I haven't had a chance to bisect yet but I will in a few hours.
> >
> >No need to, I've just completed a bisect, it points to:
> >
> >5bca5a6303f3526ab2cf9c0a62cd26c16e0d5c2f is the first bad commit
> >commit 5bca5a6303f3526ab2cf9c0a62cd26c16e0d5c2f
> >Author: Simon Glass <sjg@chromium.org>
> >Date:   Wed Mar 25 12:22:27 2015 -0600
> >
> >     dm: usb: Drop the EHCI weak functions
> >
> >     These are a pain with driver model because we might have different EHCI
> >     drivers which want to implement them differently. Now that they use
> >     consistent function signatures, we can in good conscience move them to
> >     a struct.
> >
> >     Signed-off-by: Simon Glass <sjg@chromium.org>
> >     Reviewed-by: Marek Vasut <marex@denx.de>
> >
> >I'm going to first spend some time with my family now, I may look into
> >this later tonight, or otherwise this weekend. I'll be sure to check mail
> >first to avoid double work, so feel free to fix the problem while I'm
> >relaxing :)
> 
> Ok, so I could not help myself and took a quick look at the patch causing the
> issue, this fixes the reset on usb scan problem:
> 
> diff --git a/drivers/usb/host/ehci-sunxi.c b/drivers/usb/host/ehci-sunxi.c
> index eda9f69..a847ac5 100644
> --- a/drivers/usb/host/ehci-sunxi.c
> +++ b/drivers/usb/host/ehci-sunxi.c
> @@ -34,6 +34,8 @@ int ehci_hcd_init(int index, enum usb_init_type init, struct ehci_hccr **hccr,
>               (uint32_t)*hccr, (uint32_t)*hcor,
>               (uint32_t)HC_LENGTH(ehci_readl(&(*hccr)->cr_capbase)));
> 
> + ehci_set_controller_priv(index, NULL, NULL);
> +
>         return 0;
>  }
> 
> And should probably be squashed into the original patch to avoid bisect
> problems.
> 
> But with this in place, all is still not well wrt non devicetree usb,
> usb keyboard support does not work, "usb tree" says:
> 
> USB device tree:
>   1  Hub (480 Mb/s, 0mA)
>   |  u-boot EHCI Host Controller
>   |
>   +-2  Hub (480 Mb/s, 100mA)
>     |
>     +-3  Hub (12 Mb/s, 100mA)
>       |
>       | -1  See Interface (12 Mb/s, 0mA)
>       |
> 
> Note the -1 as device number for the "See Interface" device.
> 
> This particular usb setup used to work fine.
> 
> I guess this is another issue to git bisect, no idea when I'll get around
> to that.

So, another bisect.  The problem commit is:
commit 5bbc627e33a9b3666438e03d560d06ab3de28f6f
Author: Simon Glass <sjg@chromium.org>
Date:   Wed Mar 25 12:22:06 2015 -0600

    dm: usb: Split out more code from usb_new_device()
    
    Move the code that sets up the device with a new address into its own
    function, usb_prepare_device().
    
    Signed-off-by: Simon Glass <sjg@chromium.org>
    Reviewed-by: Marek Vasut <marex@denx.de>

Prior to this commit on a non-DM board (my A20 Lime2) a USB keyboard
being inserted causes:
scanning bus 1 for devices... cannot reset port 1!?
1 USB Device(s) found
And then at the end, USB is "running" and I can usb tree.  Afterwards,
same message about reset but then No USB Device found.
Simon Glass April 17, 2015, 9:18 p.m. UTC | #3
Hi Tom,

On 17 April 2015 at 15:13, Tom Rini <trini@konsulko.com> wrote:
>
> On Fri, Apr 17, 2015 at 09:39:50PM +0200, Hans de Goede wrote:
> > Hi,
> >
> > On 17-04-15 21:28, Hans de Goede wrote:
> > >Hi,
> > >
> > >On 17-04-15 19:53, Tom Rini wrote:
> > >>On Fri, Apr 17, 2015 at 10:54:21AM -0600, Simon Glass wrote:
> > >>>Hi Tom,
> > >>>
> > >>>On 17 April 2015 at 10:46, Tom Rini <trini@konsulko.com> wrote:
> > >>>>On Fri, Apr 17, 2015 at 10:30:36AM -0600, Simon Glass wrote:
> > >>>>>Hi Tom,
> > >>>>>
> > >>>>>On 17 April 2015 at 10:27, Tom Rini <trini@konsulko.com> wrote:
> > >>>>>>
> > >>>>>>On Thu, Apr 16, 2015 at 09:15:26PM -0600, Simon Glass wrote:
> > >>>>>>
> > >>>>>>>Hi Tom,
> > >>>>>>>
> > >>>>>>>As mentioned I reverted this patch as it conflicted with the dm tree
> > >>>>>>>and I suspect it might be buggy:
> > >>>>>>>
> > >>>>>>>cd749658 usb_storage : scan all interfaces to find a storage device
> > >>>>>>>
> > >>>>>>>Assuming this is OK and applies successfully I will rebase and resend
> > >>>>>>>this patch, then reply with some comments I have on the patch.
> > >>>>>>>
> > >>>>>>>
> > >>>>>>>The following changes since commit 4564faeafbf11feb839e2e3f927be2f1a919ba96:
> > >>>>>>>
> > >>>>>>>   ti: dwc3: Enable clocks in enable_basic_clocks() in hw_data.c
> > >>>>>>>(2015-04-16 15:08:36 -0400)
> > >>>>>>>
> > >>>>>>>are available in the git repository at:
> > >>>>>>>
> > >>>>>>>   http://git.denx.de/u-boot-dm.git
> > >>>>>>>
> > >>>>>>>for you to fetch changes up to 2e6263093b3a5c2d2c586afaedfd346d6628f784:
> > >>>>>>>
> > >>>>>>>   sandbox: exynos: Move CONFIG_SOUND_SANDBOX to Kconfig (2015-04-16
> > >>>>>>>20:47:57 -0600)
> > >>>>>>
> > >>>>>>With this PR, am335x_boneblack (which has DM enabled) no longer
> > >>>>>>functions.  I'm running a bisect now, but heads up.
> > >>>>>
> > >>>>>OK, I'll wait for your bisect - also what is the console output when it breaks?
> > >>>>
> > >>>>OK, disregard Beaglebone Black error, that was me (need to remember that
> > >>>>eMMC boot only works off of am335x_boneblack_config).  But,
> > >>>>A20-OLinuXino-Lime2_defconfig is broken by this.  The error log:
> > >>>>Command(A20 OLinuXino Lime2 console)> on
> > >>>>(user:trini) Power turned on
> > >>>>
> > >>>>U-Boot SPL 2015.04-00342-g2e62630 (Apr 17 2015 - 12:40:45)
> > >>>>DRAM: 1024 MiB
> > >>>>CPU: 912000000Hz, AXI/AHB/APB: 3/2/2
> > >>>>
> > >>>>
> > >>>>U-Boot 2015.04-00342-g2e62630 (Apr 17 2015 - 12:40:45) Allwinner Technology
> > >>>>
> > >>>>CPU:   Allwinner A20 (SUN7I)
> > >>>>I2C:   ready
> > >>>>DRAM:  1 GiB
> > >>>>MMC:   SUNXI SD/MMC: 0
> > >>>>*** Warning - bad CRC, using default environment
> > >>>>
> > >>>>In:    serial
> > >>>>Out:   serial
> > >>>>Err:   serial
> > >>>>SCSI:  SUNXI SCSI INIT
> > >>>>SATA link 0 timeout.
> > >>>>AHCI 0001.0100 32 slots 1 ports 3 Gbps 0x1 impl SATA mode
> > >>>>flags: ncq stag pm led clo only pmp pio slum part ccc apst
> > >>>>Net:   dwmac.1c50000
> > >>>>starting USB...
> > >>>>USB0:   USB EHCI 1.00
> > >>>>scanning bus 0 for devices...
> > >>>>U-Boot SPL 2015.04-00342-g2e62630 (Apr 17 2015 - 12:40:45)
> > >>>>DRAM: 1024 MiB
> > >>>>CPU: 912000000Hz, AXI/AHB/APB: 3/2/2
> > >>>>MMC Device 0 not found
> > >>>>spl: mmc device not found!!
> > >>>>### ERROR ### Please RESET the board ###
> > >>>>
> > >>>>And good:
> > >>>>Command(A20 OLinuXino Lime2 console)> on
> > >>>>(user:trini) Power turned on
> > >>>>
> > >>>>U-Boot SPL 2015.04-00121-g4564fae (Apr 17 2015 - 12:42:40)
> > >>>>DRAM: 1024 MiB
> > >>>>CPU: 912000000Hz, AXI/AHB/APB: 3/2/2
> > >>>>
> > >>>>
> > >>>>U-Boot 2015.04-00121-g4564fae (Apr 17 2015 - 12:42:40) Allwinner Technology
> > >>>>
> > >>>>CPU:   Allwinner A20 (SUN7I)
> > >>>>I2C:   ready
> > >>>>DRAM:  1 GiB
> > >>>>MMC:   SUNXI SD/MMC: 0
> > >>>>*** Warning - bad CRC, using default environment
> > >>>>
> > >>>>In:    serial
> > >>>>Out:   serial
> > >>>>Err:   serial
> > >>>>SCSI:  SUNXI SCSI INIT
> > >>>>SATA link 0 timeout.
> > >>>>AHCI 0001.0100 32 slots 1 ports 3 Gbps 0x1 impl SATA mode
> > >>>>flags: ncq stag pm led clo only pmp pio slum part ccc apst
> > >>>>Net:   dwmac.1c50000
> > >>>>starting USB...
> > >>>>USB0:   USB EHCI 1.00
> > >>>>scanning bus 0 for devices... 1 USB Device(s) found
> > >>>>USB1:   USB EHCI 1.00
> > >>>>scanning bus 1 for devices... 1 USB Device(s) found
> > >>>>        scanning usb for storage devices... 0 Storage Device(s) found
> > >>>>Hit any key to stop autoboot:  0
> > >>>>sunxi#
> > >>>>
> > >>>>So... we cause a reset during USB scan and then fail to boot a second
> > >>>>time?
> > >>>
> > >>>Thanks for testing this.
> > >>>
> > >>>I'm not sure if I have a Lime (I might do, will need to look). I was
> > >>>testing sunxi on a pcduino3 but USB does not work on that for reasons
> > >>>I was looking into (it just says lowlevel init failure due to moving
> > >>>to driver model for GPIO).
> > >>>
> > >>>I will take a look. In the meantime, which commit breaks this? I could
> > >>>perhaps issue a pull for commits up to that point to reduce the load.
> > >>
> > >>I haven't had a chance to bisect yet but I will in a few hours.
> > >
> > >No need to, I've just completed a bisect, it points to:
> > >
> > >5bca5a6303f3526ab2cf9c0a62cd26c16e0d5c2f is the first bad commit
> > >commit 5bca5a6303f3526ab2cf9c0a62cd26c16e0d5c2f
> > >Author: Simon Glass <sjg@chromium.org>
> > >Date:   Wed Mar 25 12:22:27 2015 -0600
> > >
> > >     dm: usb: Drop the EHCI weak functions
> > >
> > >     These are a pain with driver model because we might have different EHCI
> > >     drivers which want to implement them differently. Now that they use
> > >     consistent function signatures, we can in good conscience move them to
> > >     a struct.
> > >
> > >     Signed-off-by: Simon Glass <sjg@chromium.org>
> > >     Reviewed-by: Marek Vasut <marex@denx.de>
> > >
> > >I'm going to first spend some time with my family now, I may look into
> > >this later tonight, or otherwise this weekend. I'll be sure to check mail
> > >first to avoid double work, so feel free to fix the problem while I'm
> > >relaxing :)
> >
> > Ok, so I could not help myself and took a quick look at the patch causing the
> > issue, this fixes the reset on usb scan problem:
> >
> > diff --git a/drivers/usb/host/ehci-sunxi.c b/drivers/usb/host/ehci-sunxi.c
> > index eda9f69..a847ac5 100644
> > --- a/drivers/usb/host/ehci-sunxi.c
> > +++ b/drivers/usb/host/ehci-sunxi.c
> > @@ -34,6 +34,8 @@ int ehci_hcd_init(int index, enum usb_init_type init, struct ehci_hccr **hccr,
> >               (uint32_t)*hccr, (uint32_t)*hcor,
> >               (uint32_t)HC_LENGTH(ehci_readl(&(*hccr)->cr_capbase)));
> >
> > + ehci_set_controller_priv(index, NULL, NULL);
> > +
> >         return 0;
> >  }
> >
> > And should probably be squashed into the original patch to avoid bisect
> > problems.
> >
> > But with this in place, all is still not well wrt non devicetree usb,
> > usb keyboard support does not work, "usb tree" says:
> >
> > USB device tree:
> >   1  Hub (480 Mb/s, 0mA)
> >   |  u-boot EHCI Host Controller
> >   |
> >   +-2  Hub (480 Mb/s, 100mA)
> >     |
> >     +-3  Hub (12 Mb/s, 100mA)
> >       |
> >       | -1  See Interface (12 Mb/s, 0mA)
> >       |
> >
> > Note the -1 as device number for the "See Interface" device.
> >
> > This particular usb setup used to work fine.
> >
> > I guess this is another issue to git bisect, no idea when I'll get around
> > to that.
>
> So, another bisect.  The problem commit is:
> commit 5bbc627e33a9b3666438e03d560d06ab3de28f6f
> Author: Simon Glass <sjg@chromium.org>
> Date:   Wed Mar 25 12:22:06 2015 -0600
>
>     dm: usb: Split out more code from usb_new_device()
>
>     Move the code that sets up the device with a new address into its own
>     function, usb_prepare_device().
>
>     Signed-off-by: Simon Glass <sjg@chromium.org>
>     Reviewed-by: Marek Vasut <marex@denx.de>
>
> Prior to this commit on a non-DM board (my A20 Lime2) a USB keyboard
> being inserted causes:
> scanning bus 1 for devices... cannot reset port 1!?
> 1 USB Device(s) found
> And then at the end, USB is "running" and I can usb tree.  Afterwards,
> same message about reset but then No USB Device found.

This seems related to the other problem I fixed (reported by Stephen),
and I wonder if with the conflicts I have messed something up. I did
have to adjust the code. Anyway I should be able to dig into this one
later today.

Gosh it will be good to get this all resolved...

Regards,
Simon
Hans de Goede April 18, 2015, 9:19 a.m. UTC | #4
Hi,

On 17-04-15 21:39, Hans de Goede wrote:
> Hi,
>
> On 17-04-15 21:28, Hans de Goede wrote:
>> Hi,
>>
>> On 17-04-15 19:53, Tom Rini wrote:

<snip>

>>> I haven't had a chance to bisect yet but I will in a few hours.
>>
>> No need to, I've just completed a bisect, it points to:
>>
>> 5bca5a6303f3526ab2cf9c0a62cd26c16e0d5c2f is the first bad commit
>> commit 5bca5a6303f3526ab2cf9c0a62cd26c16e0d5c2f
>> Author: Simon Glass <sjg@chromium.org>
>> Date:   Wed Mar 25 12:22:27 2015 -0600
>>
>>      dm: usb: Drop the EHCI weak functions
>>
>>      These are a pain with driver model because we might have different EHCI
>>      drivers which want to implement them differently. Now that they use
>>      consistent function signatures, we can in good conscience move them to
>>      a struct.
>>
>>      Signed-off-by: Simon Glass <sjg@chromium.org>
>>      Reviewed-by: Marek Vasut <marex@denx.de>
>>
>> I'm going to first spend some time with my family now, I may look into
>> this later tonight, or otherwise this weekend. I'll be sure to check mail
>> first to avoid double work, so feel free to fix the problem while I'm
>> relaxing :)
>
> Ok, so I could not help myself and took a quick look at the patch causing the
> issue, this fixes the reset on usb scan problem:
>
> diff --git a/drivers/usb/host/ehci-sunxi.c b/drivers/usb/host/ehci-sunxi.c
> index eda9f69..a847ac5 100644
> --- a/drivers/usb/host/ehci-sunxi.c
> +++ b/drivers/usb/host/ehci-sunxi.c
> @@ -34,6 +34,8 @@ int ehci_hcd_init(int index, enum usb_init_type init, struct ehci_hccr **hccr,
>                (uint32_t)*hccr, (uint32_t)*hcor,
>                (uint32_t)HC_LENGTH(ehci_readl(&(*hccr)->cr_capbase)));
>
> + ehci_set_controller_priv(index, NULL, NULL);
> +
>          return 0;
>   }
>
> And should probably be squashed into the original patch to avoid bisect
> problems.
>
> But with this in place, all is still not well wrt non devicetree usb,
> usb keyboard support does not work, "usb tree" says:
>
> USB device tree:
>    1  Hub (480 Mb/s, 0mA)
>    |  u-boot EHCI Host Controller
>    |
>    +-2  Hub (480 Mb/s, 100mA)
>      |
>      +-3  Hub (12 Mb/s, 100mA)
>        |
>        | -1  See Interface (12 Mb/s, 0mA)
>        |
>
> Note the -1 as device number for the "See Interface" device.
>
> This particular usb setup used to work fine.
>
> I guess this is another issue to git bisect, no idea when I'll get around
> to that.

Ok, so I've done a git bisect of this (using a branch with my patch for
the usb-reset issue squashed into the original commit to keep things
bisectable), and it points to:

commit 3f7af70db23fc1c6b8f9e1bd966cadf2eb139f93
Author: Simon Glass <sjg@chromium.org>
Date:   Wed Mar 25 12:22:07 2015 -0600

     dm: usb: Complete the splitting up of usb_new_device()

     This function now calls usb_setup_device() to set up the device and
     usb_hub_probe() to check if it is a hub. The XHCI special case is now a
     parameter to usb_setup_device(). The latter will be used by the USB uclass
     when it is added, since it does not rely on any CONFIGs or legacy data
     structures.

     Signed-off-by: Simon Glass <sjg@chromium.org>
     Reviewed-by: Marek Vasut <marex@denx.de>

(the commit id may be of because as said I'm using a custom branch for this).

I'll see if I can figure out why that commit breaks things, but I thought
I would share the bisect result ASAP to avoid double work.

Regards,

Hans
diff mbox

Patch

diff --git a/drivers/usb/host/ehci-sunxi.c b/drivers/usb/host/ehci-sunxi.c
index eda9f69..a847ac5 100644
--- a/drivers/usb/host/ehci-sunxi.c
+++ b/drivers/usb/host/ehci-sunxi.c
@@ -34,6 +34,8 @@  int ehci_hcd_init(int index, enum usb_init_type init, struct ehci_hccr **hccr,
               (uint32_t)*hccr, (uint32_t)*hcor,
               (uint32_t)HC_LENGTH(ehci_readl(&(*hccr)->cr_capbase)));

+ ehci_set_controller_priv(index, NULL, NULL);
+
         return 0;
  }