2022-11-24 06:45:09

by Andy Tang

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

From: Yuantian Tang <[email protected]>

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 <[email protected]>
---
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


2022-11-29 09:09:06

by Adrian Hunter

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

On 24/11/22 08:46, [email protected] wrote:
> From: Yuantian Tang <[email protected]>
>
> 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 <[email protected]>

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 <[email protected]>' != 'Signed-off-by: Andy Tang <[email protected]>'

> ---
> 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},

2022-11-29 09:45:37

by Andy Tang

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

Hi Adrian,

Thanks for your review. Please see my reply inline.

> -----Original Message-----
> From: Adrian Hunter <[email protected]>
> Sent: 2022年11月29日 16:05
> To: Andy Tang <[email protected]>; [email protected]
> Cc: [email protected]; [email protected]
> Subject: [EXT] Re: [PATCH] mmc: sdhci-of-esdhc: limit the SDHC clock
> frequency
>
> Caution: EXT Email
>
> On 24/11/22 08:46, [email protected] wrote:
> > From: Yuantian Tang <[email protected]>
> >
> > 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 <[email protected]>
>
> 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
> <[email protected]>' != 'Signed-off-by: Andy Tang <[email protected]>'
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},