Re: [EXT] Re: [PATCH 4/4] imx8mq_evk: Enable the USB3.0 host port

[Patrick Wildt <patrick@blueri.se>]

Am Wed, Mar 03, 2021 at 08:53:52AM +0000 schrieb Bough Chen:
> > -----Original Message-----
> > From: Ye Li
> > Sent: 2021年2月27日 14:05
> > To: festevam@gmail.com; Bough Chen <haibo.chen@nxp.com>
> > Cc: Peng Fan <peng.fan@nxp.com>; u-boot@lists.denx.de; dl-uboot-imx
> > <uboot-imx@nxp.com>; sbabic@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 <ye.li@nxp.com> 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 <haibo.chen@nxp.com>
> > 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 <haibo.chen@nxp.com>
> > 
> > 
> > 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)
> > 
> >  &reg_usdhc2_vmmc {
> > +       startup-delay-us = <1000>;
> >         u-boot,off-on-delay-us = <20000>;
> >  };
> > 
> > 
> > @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