RE: [PATCH 2/3] mmc: sdhci-esdhc-imx: add SD clock limitation for imx6ull

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> -----Original Message-----
> From: Russell King - ARM Linux [mailto:linux@xxxxxxxxxxxxxxx]
> Sent: 2018年12月27日 20:23
> To: Adrian Hunter <adrian.hunter@xxxxxxxxx>
> Cc: BOUGH CHEN <haibo.chen@xxxxxxx>; ulf.hansson@xxxxxxxxxx;
> robh+dt@xxxxxxxxxx; mark.rutland@xxxxxxx; shawnguo@xxxxxxxxxx;
> s.hauer@xxxxxxxxxxxxxx; kernel@xxxxxxxxxxxxxx; Fabio Estevam
> <fabio.estevam@xxxxxxx>; devicetree@xxxxxxxxxxxxxxx;
> linux-mmc@xxxxxxxxxxxxxxx; dl-linux-imx <linux-imx@xxxxxxx>;
> linux-arm-kernel@xxxxxxxxxxxxxxxxxxx
> Subject: Re: [PATCH 2/3] mmc: sdhci-esdhc-imx: add SD clock limitation for
> imx6ull
> 
> On Thu, Dec 27, 2018 at 10:01:24AM +0200, Adrian Hunter wrote:
> > On 20/12/18 9:49 AM, BOUGH CHEN wrote:
> > > i.MX6ULL has errata ERR010450, point out that due to SOC I/O timing
> > > limitation, for eMMC HS200 and SD/SDIO 3.0 SDR104, the clock rate
> > > can't exceed 150MHz. And for eMMC DDR52 and SD/SDIO
> > > DDR50 mode, the clock rate can't exceed 45MHz.
> > >
> > > This patch add this limit for imx6ull.
> > >
> > > Signed-off-by: Haibo Chen <haibo.chen@xxxxxxx>
> >
> > Apart from the kbuild test robot complaints (do they need to be fixed?):
> >
> > Acked-by: Adrian Hunter <adrian.hunter@xxxxxxxxx>
> 
> Nacked-by: Russell King <rmk+kernel@xxxxxxxxxxxxxxx>
> 
> The kbuild test robot complaints do need to be fixed first.  They're confusing
> because GCC produces quite a lot of garbage in its error messages now.  If
> you look at the patch, there's a hunk that has:
> 
> +       if (imx_data->socdata->flags & ESDHC_FLAG_ERR010450) {
> +               if (imx_data->is_ddr)
> +                       clock = clock > 45000000 ? 45000000 : clock;
> +               else
> +                       clock = clock > 150000000 ? 150000000 : clock;
> +
> 
> which is missing a closing brace.  This patch could not have been build tested
> before it was mailed to the list, and the test robot is highlighting that fact.
> 

I'm really sorry about that.  Thanks for point out that, I will double check my patch when I mail to the list next time.

> It may also be a good idea to encourage a different approach to the above
> anyway:
> 
> 	if (imx_data->socdata->flags & ESDHC_FLAG_ERR010450) {
> 		unsigned long max_clock;
> 
> 		max_clock = imx_data->is_ddr ? 45000000 : 150000000;
> 
> 		clock = max(clock, max_clock);
> 	}
> 
> rather than open-coding the max() stuff in the driver.

Thanks for giving such a good suggestion, I will take care of it, with a little change:
		clock = min(clock, max_clock);

Best Regard
Bough Chen
> 
> Thanks.
> 
> --
> RMK's Patch system:
> https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.
> armlinux.org.uk%2Fdeveloper%2Fpatches%2F&amp;data=02%7C01%7Chaibo.
> chen%40nxp.com%7C13ae59760de24de29a7c08d66bf61174%7C686ea1d3bc2
> b4c6fa92cd99c5c301635%7C0%7C0%7C636815101937146642&amp;sdata=UP
> S7z2UTLpfgqgyWFPM%2FyXP%2BQu1qULoavFOBLFWkQjw%3D&amp;reserved
> =0
> FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps
> up According to speedtest.net: 11.9Mbps down 500kbps up




[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux