[PATCH] mmc: sdhci-of-esdhc: limit the SDHC clock frequency

[PATCH] mmc: sdhci-of-esdhc: limit the SDHC clock frequency

[andy.tang]

From: Yuantian Tang <andy.tang@nxp.com>

The highest clock frequency for eMMC HS200 mode on ls1043a
is 116.7Mhz according to its specification.
So add the limit to gate the frequency.

Signed-off-by: Andy Tang <andy.tang@nxp.com>
---
 drivers/mmc/host/sdhci-of-esdhc.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/mmc/host/sdhci-of-esdhc.c b/drivers/mmc/host/sdhci-of-esdhc.c
index 00542dd74c07..1fd4ae10862d 100644
--- a/drivers/mmc/host/sdhci-of-esdhc.c
+++ b/drivers/mmc/host/sdhci-of-esdhc.c
@@ -43,6 +43,12 @@ static const struct esdhc_clk_fixup ls1021a_esdhc_clk = {
 	.max_clk[MMC_TIMING_SD_HS] = 46500000,
 };
 
+static const struct esdhc_clk_fixup ls1043a_esdhc_clk = {
+	.sd_dflt_max_clk = 25000000,
+	.max_clk[MMC_TIMING_UHS_SDR104] = 116700000,
+	.max_clk[MMC_TIMING_MMC_HS200] = 116700000,
+};
+
 static const struct esdhc_clk_fixup ls1046a_esdhc_clk = {
 	.sd_dflt_max_clk = 25000000,
 	.max_clk[MMC_TIMING_UHS_SDR104] = 167000000,
@@ -64,6 +70,7 @@ static const struct esdhc_clk_fixup p1010_esdhc_clk = {
 
 static const struct of_device_id sdhci_esdhc_of_match[] = {
 	{ .compatible = "fsl,ls1021a-esdhc", .data = &ls1021a_esdhc_clk},
+	{ .compatible = "fsl,ls1043a-esdhc", .data = &ls1043a_esdhc_clk},
 	{ .compatible = "fsl,ls1046a-esdhc", .data = &ls1046a_esdhc_clk},
 	{ .compatible = "fsl,ls1012a-esdhc", .data = &ls1012a_esdhc_clk},
 	{ .compatible = "fsl,p1010-esdhc",   .data = &p1010_esdhc_clk},
-- 
2.25.1

Re: [PATCH] mmc: sdhci-of-esdhc: limit the SDHC clock frequency

[Adrian Hunter]

On 24/11/22 08:46, andy.tang@nxp.com wrote:
> From: Yuantian Tang <andy.tang@nxp.com>
> 
> The highest clock frequency for eMMC HS200 mode on ls1043a
> is 116.7Mhz according to its specification.
> So add the limit to gate the frequency.
> 
> Signed-off-by: Andy Tang <andy.tang@nxp.com>

It is probably nicer to use a consistent email name.  checkpatch gives this warning:

WARNING: From:/Signed-off-by: email name mismatch: 'From: Yuantian Tang <andy.tang@nxp.com>' != 'Signed-off-by: Andy Tang <andy.tang@nxp.com>'

> ---
>  drivers/mmc/host/sdhci-of-esdhc.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/mmc/host/sdhci-of-esdhc.c b/drivers/mmc/host/sdhci-of-esdhc.c
> index 00542dd74c07..1fd4ae10862d 100644
> --- a/drivers/mmc/host/sdhci-of-esdhc.c
> +++ b/drivers/mmc/host/sdhci-of-esdhc.c
> @@ -43,6 +43,12 @@ static const struct esdhc_clk_fixup ls1021a_esdhc_clk = {
>  	.max_clk[MMC_TIMING_SD_HS] = 46500000,
>  };
>  
> +static const struct esdhc_clk_fixup ls1043a_esdhc_clk = {
> +	.sd_dflt_max_clk = 25000000,
> +	.max_clk[MMC_TIMING_UHS_SDR104] = 116700000,
> +	.max_clk[MMC_TIMING_MMC_HS200] = 116700000,
> +};

ls1043a_esdhc_clk seems the same as ls1046a_esdhc_clk
Should they be shared? e.g. called ls104xx_esdhc_clk

> +
>  static const struct esdhc_clk_fixup ls1046a_esdhc_clk = {
>  	.sd_dflt_max_clk = 25000000,
>  	.max_clk[MMC_TIMING_UHS_SDR104] = 167000000,
> @@ -64,6 +70,7 @@ static const struct esdhc_clk_fixup p1010_esdhc_clk = {
>  
>  static const struct of_device_id sdhci_esdhc_of_match[] = {
>  	{ .compatible = "fsl,ls1021a-esdhc", .data = &ls1021a_esdhc_clk},
> +	{ .compatible = "fsl,ls1043a-esdhc", .data = &ls1043a_esdhc_clk},
>  	{ .compatible = "fsl,ls1046a-esdhc", .data = &ls1046a_esdhc_clk},
>  	{ .compatible = "fsl,ls1012a-esdhc", .data = &ls1012a_esdhc_clk},
>  	{ .compatible = "fsl,p1010-esdhc",   .data = &p1010_esdhc_clk},

RE: [EXT] Re: [PATCH] mmc: sdhci-of-esdhc: limit the SDHC clock frequency

[Andy Tang]

Hi Adrian,

Thanks for your review. Please see my reply inline.

> -----Original Message-----
> From: Adrian Hunter <adrian.hunter@intel.com>
> Sent: 2022年11月29日 16:05
> To: Andy Tang <andy.tang@nxp.com>; ulf.hansson@linaro.org
> Cc: linux-mmc@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: [EXT] Re: [PATCH] mmc: sdhci-of-esdhc: limit the SDHC clock
> frequency
> 
> Caution: EXT Email
> 
> On 24/11/22 08:46, andy.tang@nxp.com wrote:
> > From: Yuantian Tang <andy.tang@nxp.com>
> >
> > The highest clock frequency for eMMC HS200 mode on ls1043a is 116.7Mhz
> > according to its specification.
> > So add the limit to gate the frequency.
> >
> > Signed-off-by: Andy Tang <andy.tang@nxp.com>
> 
> It is probably nicer to use a consistent email name.  checkpatch gives this
> warning:
> 
> WARNING: From:/Signed-off-by: email name mismatch: 'From: Yuantian Tang
> <andy.tang@nxp.com>' != 'Signed-off-by: Andy Tang <andy.tang@nxp.com>'
OK, will keep it consistent.

> 
> > ---
> >  drivers/mmc/host/sdhci-of-esdhc.c | 7 +++++++
> >  1 file changed, 7 insertions(+)
> >
> > diff --git a/drivers/mmc/host/sdhci-of-esdhc.c
> > b/drivers/mmc/host/sdhci-of-esdhc.c
> > index 00542dd74c07..1fd4ae10862d 100644
> > --- a/drivers/mmc/host/sdhci-of-esdhc.c
> > +++ b/drivers/mmc/host/sdhci-of-esdhc.c
> > @@ -43,6 +43,12 @@ static const struct esdhc_clk_fixup
> ls1021a_esdhc_clk = {
> >       .max_clk[MMC_TIMING_SD_HS] = 46500000,  };
> >
> > +static const struct esdhc_clk_fixup ls1043a_esdhc_clk = {
> > +     .sd_dflt_max_clk = 25000000,
> > +     .max_clk[MMC_TIMING_UHS_SDR104] = 116700000,
> > +     .max_clk[MMC_TIMING_MMC_HS200] = 116700000, };
> 
> ls1043a_esdhc_clk seems the same as ls1046a_esdhc_clk Should they be
> shared? e.g. called ls104xx_esdhc_clk
The values are different. One is the 116700Khz, the other is 167000Khz.
Even they are the same, it is better to have its own clk struct for future compatibility.

Thanks,
Andy
> 
> > +
> >  static const struct esdhc_clk_fixup ls1046a_esdhc_clk = {
> >       .sd_dflt_max_clk = 25000000,
> >       .max_clk[MMC_TIMING_UHS_SDR104] = 167000000, @@ -64,6
> +70,7 @@
> > static const struct esdhc_clk_fixup p1010_esdhc_clk = {
> >
> >  static const struct of_device_id sdhci_esdhc_of_match[] = {
> >       { .compatible = "fsl,ls1021a-esdhc", .data =
> > &ls1021a_esdhc_clk},
> > +     { .compatible = "fsl,ls1043a-esdhc", .data =
> > + &ls1043a_esdhc_clk},
> >       { .compatible = "fsl,ls1046a-esdhc", .data = &ls1046a_esdhc_clk},
> >       { .compatible = "fsl,ls1012a-esdhc", .data = &ls1012a_esdhc_clk},
> >       { .compatible = "fsl,p1010-esdhc",   .data = &p1010_esdhc_clk},