Re: [EXT] Re: [PATCH 4/4] imx8mq_evk: Enable the USB3.0 host port
Am Mon, Jul 12, 2021 at 10:28:25AM -0300 schrieb Fabio Estevam: > Hi Patrick, > > On Sat, Jul 10, 2021 at 8:35 PM Patrick Wildt wrote: > > > is this patchset still being reviewed? I think the discussion has moved > > to some SD card problem, which is fixed now? Would be nice if USB 3.0 > > I think you are referring to > https://source.denx.de/u-boot/u-boot/-/commit/63756575b42b8b4fb3f59cbbf0cedf03331bc2d2 > > If so, I had to revert it as it caused boot time regression (10s in > SPL + 10 s in U-Boot proper) in several > i.MX boards: > > https://source.denx.de/u-boot/u-boot/-/commit/f132aab403271ff00c0cfdd3af6504e87c7d0aaf > > I haven't tested USB 3.0 in mainline U-Boot on imx8mq-evk. Maybe Peng > or Ye Li can comment. Hi Fabio, I cherry-picked this USB 3.0 series from Ye Ling on to the MNT Reform2 patchset I sent out, and I'm happy with the results. Best regards, Patrick
Re: [EXT] Re: [PATCH 4/4] imx8mq_evk: Enable the USB3.0 host port
Hi Patrick, On Sat, Jul 10, 2021 at 8:35 PM Patrick Wildt wrote: > is this patchset still being reviewed? I think the discussion has moved > to some SD card problem, which is fixed now? Would be nice if USB 3.0 I think you are referring to https://source.denx.de/u-boot/u-boot/-/commit/63756575b42b8b4fb3f59cbbf0cedf03331bc2d2 If so, I had to revert it as it caused boot time regression (10s in SPL + 10 s in U-Boot proper) in several i.MX boards: https://source.denx.de/u-boot/u-boot/-/commit/f132aab403271ff00c0cfdd3af6504e87c7d0aaf I haven't tested USB 3.0 in mainline U-Boot on imx8mq-evk. Maybe Peng or Ye Li can comment. Regards, Fabio Estevam > worked on i.MX8MQ platforms. > > I can also have a look at reviewing the functionality, but I don't think > I can spot U-Boot coding style issues. > > Best regards, > Patrick
Re: [EXT] Re: [PATCH 4/4] imx8mq_evk: Enable the USB3.0 host port
Am Wed, Mar 03, 2021 at 08:53:52AM + schrieb Bough Chen:
> > -Original Message-
> > From: Ye Li
> > Sent: 2021年2月27日 14:05
> > To: feste...@gmail.com; Bough Chen
> > Cc: Peng Fan ; u-boot@lists.denx.de; dl-uboot-imx
> > ; sba...@denx.de
> > Subject: Re: [EXT] Re: [PATCH 4/4] imx8mq_evk: Enable the USB3.0 host port
> >
> > Hi Fabio,
> >
> > On Thu, 2021-02-25 at 10:49 -0300, Fabio Estevam wrote:
> > > Caution: EXT Email
> > >
> > > Hi Ye Li,
> > >
> > > On Thu, Feb 25, 2021 at 10:34 AM Ye Li wrote:
> > >
> > > >
> > > > Sure, I have tested it on 8mq evk. I can reproduce the two issues
> > > > you met.
> > > > The first issue is caused by the ALIGN. The implementation of
> > > > standard ALIGN requires the aligned size to be power of 2. But the
> > > > ALIGN in imx8mimage does not have this requirement. So below result
> > > > is wrong by using the standard ALIGN. Your fix should be OK for this
> > > > issue.
> > > Good, could you please reply to my ALIGN macro patch with your
> > > Tested-by tag then?
> > >
> > Replied it.
> >
> > > >
> > > > For the second issue, I did not debug into it. But our vendor tree
> > > > also uses off-on-delay-us in both u-boot and kernel. So it is likely
> > > > caused by other change.
> > > Considering we are already at 2021.04-rc2, I think it would be safer
> > > to go with my patch that removes off-on-delay-us.
> > >
> > > What do you think?
> > >
> > > Thanks
> > My debug shows the issue is triggered by below commit:
> >
> > commit 9098682200e6cca4b776638a51200dafa16f50fb
> > Author: Haibo Chen
> > Date: Tue Sep 22 18:11:43 2020 +0800
> >
> > mmc: fsl_esdhc_imx: remove the 1ms delay before sending command
> >
> > This 1ms delay before sending command already exist from the beginning
> > of the fsl_esdhc driver added in year 2008. Now this driver has been
> > split for two files: fsl_esdhc.c and fsl_esdhc_imx.c.
> > fsl_esdhc_imx.c
> > only for i.MX series. i.MX series esdhc/usdhc do not need this 1ms delay
> > before sending any command. So remove this 1ms, this will save a lot
> > time if handling a large mmc data.
> >
> > Signed-off-by: Haibo Chen
> >
> >
> > The first "go idle" command in mmc_get_op_cond seems not put SD card to
> > idle status, but if adding a delay before it (like 1ms delay), then
> > everything
> > works. This commit removed 1ms delay in sending command, so the issue is
> > triggered. The root cause might be "startup-delay-us"
> > needed for this regulator to reach a threshold voltage for SD working.
> > Below change also can fix the issue.
> >
> > --- a/arch/arm/dts/imx8mq-evk-u-boot.dtsi
> > +++ b/arch/arm/dts/imx8mq-evk-u-boot.dtsi
> > @@ -1,6 +1,7 @@
> > // SPDX-License-Identifier: (GPL-2.0 OR MIT)
> >
> > ®_usdhc2_vmmc {
> > + startup-delay-us = <1000>;
> > u-boot,off-on-delay-us = <2>;
> > };
> >
> >
> > @Haibo, Could you help looking into the issue. What's your opinion to add
> > the
> > startup-delay-us or revert your commit?
> >
>
> Hi Fabio,
>
> I co-debug with Ye, and find the issue is also related with clock
> enable/disable. For current logic on imx usdhc, hardware automatically gate
> off the card clock when idle.
> So before the first command "go idle", there is no clock on the clock line,
> which not align with the sd spec.
> Refer to SD3.0 spec 6.4.1 Power UP
> The host shall supply power to the card so that the voltage is reached to
> Vdd_min within 250ms and
> start to supply at least 74 SD clocks to the SD card with keeping CMD line to
> high. In case of SPI
> mode, CS shall be held to high during 74 clock cycles
>
> if we give the card the correct clock rate before the first "go idle"
> command, this issue gone.
> Please try to apply the patch I send on 2021/1/27 [PATCH] mmc:
> fsl_esdhc_imx: use VENDORSPEC_FRC_SDCLK_ON to control card clock output
>
> > Best regards,
> > Ye Li
Hi,
is this patchset still being reviewed? I think the discussion has moved
to some SD card problem, which is fixed now? Would be nice if USB 3.0
worked on i.MX8MQ platforms.
I can also have a look at reviewing the functionality, but I don't think
I can spot U-Boot coding style issues.
Best regards,
Patrick
RE: [EXT] Re: [PATCH 4/4] imx8mq_evk: Enable the USB3.0 host port
> -Original Message-
> From: Ye Li
> Sent: 2021年2月27日 14:05
> To: feste...@gmail.com; Bough Chen
> Cc: Peng Fan ; u-boot@lists.denx.de; dl-uboot-imx
> ; sba...@denx.de
> Subject: Re: [EXT] Re: [PATCH 4/4] imx8mq_evk: Enable the USB3.0 host port
>
> Hi Fabio,
>
> On Thu, 2021-02-25 at 10:49 -0300, Fabio Estevam wrote:
> > Caution: EXT Email
> >
> > Hi Ye Li,
> >
> > On Thu, Feb 25, 2021 at 10:34 AM Ye Li wrote:
> >
> > >
> > > Sure, I have tested it on 8mq evk. I can reproduce the two issues
> > > you met.
> > > The first issue is caused by the ALIGN. The implementation of
> > > standard ALIGN requires the aligned size to be power of 2. But the
> > > ALIGN in imx8mimage does not have this requirement. So below result
> > > is wrong by using the standard ALIGN. Your fix should be OK for this
> > > issue.
> > Good, could you please reply to my ALIGN macro patch with your
> > Tested-by tag then?
> >
> Replied it.
>
> > >
> > > For the second issue, I did not debug into it. But our vendor tree
> > > also uses off-on-delay-us in both u-boot and kernel. So it is likely
> > > caused by other change.
> > Considering we are already at 2021.04-rc2, I think it would be safer
> > to go with my patch that removes off-on-delay-us.
> >
> > What do you think?
> >
> > Thanks
> My debug shows the issue is triggered by below commit:
>
> commit 9098682200e6cca4b776638a51200dafa16f50fb
> Author: Haibo Chen
> Date: Tue Sep 22 18:11:43 2020 +0800
>
> mmc: fsl_esdhc_imx: remove the 1ms delay before sending command
>
> This 1ms delay before sending command already exist from the beginning
> of the fsl_esdhc driver added in year 2008. Now this driver has been
> split for two files: fsl_esdhc.c and fsl_esdhc_imx.c.
> fsl_esdhc_imx.c
> only for i.MX series. i.MX series esdhc/usdhc do not need this 1ms delay
> before sending any command. So remove this 1ms, this will save a lot
> time if handling a large mmc data.
>
> Signed-off-by: Haibo Chen
>
>
> The first "go idle" command in mmc_get_op_cond seems not put SD card to
> idle status, but if adding a delay before it (like 1ms delay), then everything
> works. This commit removed 1ms delay in sending command, so the issue is
> triggered. The root cause might be "startup-delay-us"
> needed for this regulator to reach a threshold voltage for SD working.
> Below change also can fix the issue.
>
> --- a/arch/arm/dts/imx8mq-evk-u-boot.dtsi
> +++ b/arch/arm/dts/imx8mq-evk-u-boot.dtsi
> @@ -1,6 +1,7 @@
> // SPDX-License-Identifier: (GPL-2.0 OR MIT)
>
> ®_usdhc2_vmmc {
> + startup-delay-us = <1000>;
> u-boot,off-on-delay-us = <2>;
> };
>
>
> @Haibo, Could you help looking into the issue. What's your opinion to add the
> startup-delay-us or revert your commit?
>
Hi Fabio,
I co-debug with Ye, and find the issue is also related with clock
enable/disable. For current logic on imx usdhc, hardware automatically gate off
the card clock when idle.
So before the first command "go idle", there is no clock on the clock line,
which not align with the sd spec.
Refer to SD3.0 spec 6.4.1 Power UP
The host shall supply power to the card so that the voltage is reached to
Vdd_min within 250ms and
start to supply at least 74 SD clocks to the SD card with keeping CMD line to
high. In case of SPI
mode, CS shall be held to high during 74 clock cycles
if we give the card the correct clock rate before the first "go idle" command,
this issue gone.
Please try to apply the patch I send on 2021/1/27 [PATCH] mmc: fsl_esdhc_imx:
use VENDORSPEC_FRC_SDCLK_ON to control card clock output
> Best regards,
> Ye Li
smime.p7s
Description: S/MIME cryptographic signature
Re: [EXT] Re: [PATCH 4/4] imx8mq_evk: Enable the USB3.0 host port
Hi Ye Li,
On Sat, Feb 27, 2021 at 3:04 AM Ye Li wrote:
> My debug shows the issue is triggered by below commit:
Thanks for investigating this issue. Appreciate it.
> commit 9098682200e6cca4b776638a51200dafa16f50fb
> Author: Haibo Chen
> Date: Tue Sep 22 18:11:43 2020 +0800
>
> mmc: fsl_esdhc_imx: remove the 1ms delay before sending command
>
> This 1ms delay before sending command already exist from the
> beginning
> of the fsl_esdhc driver added in year 2008. Now this driver has
> been
> split for two files: fsl_esdhc.c and fsl_esdhc_imx.c.
> fsl_esdhc_imx.c
> only for i.MX series. i.MX series esdhc/usdhc do not need this 1ms
> delay
> before sending any command. So remove this 1ms, this will save a
> lot
> time if handling a large mmc data.
>
> Signed-off-by: Haibo Chen
>
>
> The first "go idle" command in mmc_get_op_cond seems not put SD card to
> idle status, but if adding a delay before it (like 1ms delay), then
> everything works. This commit removed 1ms delay in sending command, so
> the issue is triggered. The root cause might be "startup-delay-us"
> needed for this regulator to reach a threshold voltage for SD working.
> Below change also can fix the issue.
>
> --- a/arch/arm/dts/imx8mq-evk-u-boot.dtsi
> +++ b/arch/arm/dts/imx8mq-evk-u-boot.dtsi
> @@ -1,6 +1,7 @@
> // SPDX-License-Identifier: (GPL-2.0 OR MIT)
>
> ®_usdhc2_vmmc {
> + startup-delay-us = <1000>;
> u-boot,off-on-delay-us = <2>;
> };
>
>
> @Haibo, Could you help looking into the issue. What's your opinion to
> add the startup-delay-us or revert your commit?
As the 1ms delay in the driver has been present since 2008, I prefer
to go with the revert for the following reasons:
1) We would need to fix all the esdhc users in devicetree.
2) By adding startup-delay-us only to the U-Boot dts we are deviating
from the Linux devicetree once again, which we should avoid.
Thanks,
Fabio Estevam
Re: [EXT] Re: [PATCH 4/4] imx8mq_evk: Enable the USB3.0 host port
Hi Fabio,
On Thu, 2021-02-25 at 10:49 -0300, Fabio Estevam wrote:
> Caution: EXT Email
>
> Hi Ye Li,
>
> On Thu, Feb 25, 2021 at 10:34 AM Ye Li wrote:
>
> >
> > Sure, I have tested it on 8mq evk. I can reproduce the two issues
> > you
> > met.
> > The first issue is caused by the ALIGN. The implementation of
> > standard
> > ALIGN requires the aligned size to be power of 2. But the ALIGN in
> > imx8mimage does not have this requirement. So below result is wrong
> > by
> > using the standard ALIGN. Your fix should be OK for this issue.
> Good, could you please reply to my ALIGN macro patch with your
> Tested-by tag then?
>
Replied it.
> >
> > For the second issue, I did not debug into it. But our vendor tree
> > also
> > uses off-on-delay-us in both u-boot and kernel. So it is likely
> > caused
> > by other change.
> Considering we are already at 2021.04-rc2, I think it would be safer
> to go with my patch that removes off-on-delay-us.
>
> What do you think?
>
> Thanks
My debug shows the issue is triggered by below commit:
commit 9098682200e6cca4b776638a51200dafa16f50fb
Author: Haibo Chen
Date: Tue Sep 22 18:11:43 2020 +0800
mmc: fsl_esdhc_imx: remove the 1ms delay before sending command
This 1ms delay before sending command already exist from the
beginning
of the fsl_esdhc driver added in year 2008. Now this driver has
been
split for two files: fsl_esdhc.c and fsl_esdhc_imx.c.
fsl_esdhc_imx.c
only for i.MX series. i.MX series esdhc/usdhc do not need this 1ms
delay
before sending any command. So remove this 1ms, this will save a
lot
time if handling a large mmc data.
Signed-off-by: Haibo Chen
The first "go idle" command in mmc_get_op_cond seems not put SD card to
idle status, but if adding a delay before it (like 1ms delay), then
everything works. This commit removed 1ms delay in sending command, so
the issue is triggered. The root cause might be "startup-delay-us"
needed for this regulator to reach a threshold voltage for SD working.
Below change also can fix the issue.
--- a/arch/arm/dts/imx8mq-evk-u-boot.dtsi
+++ b/arch/arm/dts/imx8mq-evk-u-boot.dtsi
@@ -1,6 +1,7 @@
// SPDX-License-Identifier: (GPL-2.0 OR MIT)
®_usdhc2_vmmc {
+ startup-delay-us = <1000>;
u-boot,off-on-delay-us = <2>;
};
@Haibo, Could you help looking into the issue. What's your opinion to
add the startup-delay-us or revert your commit?
Best regards,
Ye Li
Re: [EXT] Re: [PATCH 4/4] imx8mq_evk: Enable the USB3.0 host port
Hi Ye Li, On Thu, Feb 25, 2021 at 10:34 AM Ye Li wrote: > Sure, I have tested it on 8mq evk. I can reproduce the two issues you > met. > The first issue is caused by the ALIGN. The implementation of standard > ALIGN requires the aligned size to be power of 2. But the ALIGN in > imx8mimage does not have this requirement. So below result is wrong by > using the standard ALIGN. Your fix should be OK for this issue. Good, could you please reply to my ALIGN macro patch with your Tested-by tag then? > For the second issue, I did not debug into it. But our vendor tree also > uses off-on-delay-us in both u-boot and kernel. So it is likely caused > by other change. Considering we are already at 2021.04-rc2, I think it would be safer to go with my patch that removes off-on-delay-us. What do you think? Thanks
Re: [EXT] Re: [PATCH 4/4] imx8mq_evk: Enable the USB3.0 host port
Hi Fabio, On Thu, 2021-02-25 at 08:01 -0300, Fabio Estevam wrote: > Caution: EXT Email > > Hi Ye Li, > > On Thu, Feb 25, 2021 at 3:36 AM Ye Li wrote: > > > > > > Setup USB clock in board codes, and enable the DWC3 XHCI and > > PHY drivers to make USB3.0 host port working on i.MX8MQ EVK. > > > > Signed-off-by: Ye Li > Thanks for the patch. > > Have you tested it in the imx8mq-evk using the latest U-Boot master > branch? > > The reason I am asking is that imx8mq-evk does not even boot for me > unless I apply the following two patches: > > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flis > ts.denx.de%2Fpipermail%2Fu-boot%2F2021- > February%2F441971.html&data=04%7C01%7Cye.li%40nxp.com%7C16ed6adce > 72548e2c57e08d8d97cbf71%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C > 637498477099446700%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIj > oiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=10PTxobzvXV > UaVhn1Rv1yM5xe0uZk3aluha81cPAK%2Fc%3D&reserved=0 > > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flis > ts.denx.de%2Fpipermail%2Fu-boot%2F2021- > February%2F441988.html&data=04%7C01%7Cye.li%40nxp.com%7C16ed6adce > 72548e2c57e08d8d97cbf71%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C > 637498477099446700%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIj > oiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=lyG6qFbrKVf > vMjii3vS5fZ3DQTbxinqms%2FO4D1yX4l4%3D&reserved=0 > > Please advise. Sure, I have tested it on 8mq evk. I can reproduce the two issues you met. The first issue is caused by the ALIGN. The implementation of standard ALIGN requires the aligned size to be power of 2. But the ALIGN in imx8mimage does not have this requirement. So below result is wrong by using the standard ALIGN. Your fix should be OK for this issue. file_off += ALIGN(sbuf.st_size, HDMI_FW_SIZE + 0x2000 + 0x1000); For the second issue, I did not debug into it. But our vendor tree also uses off-on-delay-us in both u-boot and kernel. So it is likely caused by other change. Attach the log of usb host test. U-Boot SPL 2021.04-rc2-00059-g1784e9b (Feb 21 2021 - 14:35:00 -0800) PMIC: PFUZE100 ID=0x10 Normal Boot Trying to boot from MMC2 E/TC:0 0 caam_mp_init:364 * E/TC:0 0 caam_mp_init:365 * Warning: Manufacturing protection * E/TC:0 0 caam_mp_init:366 * is not supported * E/TC:0 0 caam_mp_init:367 * U-Boot 2021.04-rc2-00059-g1784e9b (Feb 21 2021 - 14:35:00 -0800) CPU: Freescale i.MX8MQ rev2.0 at 1000 MHz Reset cause: POR Model: NXP i.MX8MQ EVK DRAM: 3 GiB MMC: FSL_SDHC: 0, FSL_SDHC: 1 Loading Environment from MMC... *** Warning - No block device, using default environment In:serial Out: serial Err: serial Net: eth0: ethernet@30be Hit any key to stop autoboot: 0 u-boot=> usb start starting USB... Bus usb@3820: Register 2000140 NbrPorts 2 Starting the controller USB XHCI 1.10 scanning bus usb@3820 for devices... cannot reset port 1!? 2 USB Device(s) found scanning usb for storage devices... 1 Storage Device(s) found u-boot=> usb dev IDE device 0: Vendor: Kingston Rev: Prod: DataTraveler 3.0 Type: Removable Hard Disk Capacity: 14755.2 MB = 14.4 GB (30218842 x 512) u-boot=> usb read 0x4048 0x0 0x1000 usb read: device 0 block # 0, count 4096 ... 4096 blocks read: OK Best regards, Ye Li
