diff mbox series

[v2,2/2] mmc: mtk-sd: add support for mt7988

Message ID 20241006153447.41377-3-linux@fw-web.de (mailing list archive)
State New, archived
Headers show
Series Add mmc support for mt7988 | expand

Commit Message

Frank Wunderlich Oct. 6, 2024, 3:34 p.m. UTC 
From: Frank Wunderlich <frank-w@public-files.de>

Add support for mmc on MT7988 SoC.

Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
---
 drivers/mmc/host/mtk-sd.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

Comments

AngeloGioacchino Del Regno Oct. 7, 2024, 7:58 a.m. UTC | #1 
Il 06/10/24 17:34, Frank Wunderlich ha scritto:
> From: Frank Wunderlich <frank-w@public-files.de>
> 
> Add support for mmc on MT7988 SoC.
> 
> Signed-off-by: Frank Wunderlich <frank-w@public-files.de>

There's no need to add yet one more duplicate mtk_mmc_compatible platform
data, nor one more compatible string to this driver, as this is exactly
the same as mt7986.

Please reuse the MT7986 compatible; in DT you'll have:

compatible = "mediatek,mt7988-mmc", "mediatek,mt7986-mmc";

Cheers,
Angelo

> ---
>   drivers/mmc/host/mtk-sd.c | 14 ++++++++++++++
>   1 file changed, 14 insertions(+)
> 
> diff --git a/drivers/mmc/host/mtk-sd.c b/drivers/mmc/host/mtk-sd.c
> index 89018b6c97b9..6d5afe51a61d 100644
> --- a/drivers/mmc/host/mtk-sd.c
> +++ b/drivers/mmc/host/mtk-sd.c
> @@ -571,6 +571,19 @@ static const struct mtk_mmc_compatible mt7986_compat = {
>   	.support_64g = true,
>   };
>   
> +static const struct mtk_mmc_compatible mt7988_compat = {
> +	.clk_div_bits = 12,
> +	.recheck_sdio_irq = true,
> +	.hs400_tune = false,
> +	.pad_tune_reg = MSDC_PAD_TUNE0,
> +	.async_fifo = true,
> +	.data_tune = true,
> +	.busy_check = true,
> +	.stop_clk_fix = true,
> +	.enhance_rx = true,
> +	.support_64g = true,
> +};
> +
>   static const struct mtk_mmc_compatible mt8135_compat = {
>   	.clk_div_bits = 8,
>   	.recheck_sdio_irq = true,
> @@ -629,6 +642,7 @@ static const struct of_device_id msdc_of_ids[] = {
>   	{ .compatible = "mediatek,mt7620-mmc", .data = &mt7620_compat},
>   	{ .compatible = "mediatek,mt7622-mmc", .data = &mt7622_compat},
>   	{ .compatible = "mediatek,mt7986-mmc", .data = &mt7986_compat},
> +	{ .compatible = "mediatek,mt7988-mmc", .data = &mt7988_compat},
>   	{ .compatible = "mediatek,mt8135-mmc", .data = &mt8135_compat},
>   	{ .compatible = "mediatek,mt8173-mmc", .data = &mt8173_compat},
>   	{ .compatible = "mediatek,mt8183-mmc", .data = &mt8183_compat},
Frank Wunderlich Oct. 7, 2024, 8:37 a.m. UTC | #2 
Hi

> Gesendet: Montag, 07. Oktober 2024 um 09:58 Uhr
> Von: "AngeloGioacchino Del Regno" <angelogioacchino.delregno@collabora.com>
> Betreff: Re: [PATCH v2 2/2] mmc: mtk-sd: add support for mt7988
>
> Il 06/10/24 17:34, Frank Wunderlich ha scritto:
> > From: Frank Wunderlich <frank-w@public-files.de>
> >
> > Add support for mmc on MT7988 SoC.
> >
> > Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
>
> There's no need to add yet one more duplicate mtk_mmc_compatible platform
> data, nor one more compatible string to this driver, as this is exactly
> the same as mt7986.
>
> Please reuse the MT7986 compatible; in DT you'll have:
>
> compatible = "mediatek,mt7988-mmc", "mediatek,mt7986-mmc";

as explained in binding, the clock config is completely different (except first 2 also required by driver - 3-7 are optional there). mt7988 uses axi and ahb clocks.

but i could of course use the mt7988 compatible with mt7986 compat data...but looked dirty to me so just copied the block (to allow later changes if needed).

> Cheers,
> Angelo
>
> > ---
> >   drivers/mmc/host/mtk-sd.c | 14 ++++++++++++++
> >   1 file changed, 14 insertions(+)
> >
> > diff --git a/drivers/mmc/host/mtk-sd.c b/drivers/mmc/host/mtk-sd.c
> > index 89018b6c97b9..6d5afe51a61d 100644
> > --- a/drivers/mmc/host/mtk-sd.c
> > +++ b/drivers/mmc/host/mtk-sd.c
> > @@ -571,6 +571,19 @@ static const struct mtk_mmc_compatible mt7986_compat = {
> >   	.support_64g = true,
> >   };
> >
> > +static const struct mtk_mmc_compatible mt7988_compat = {
> > +	.clk_div_bits = 12,
> > +	.recheck_sdio_irq = true,
> > +	.hs400_tune = false,
> > +	.pad_tune_reg = MSDC_PAD_TUNE0,
> > +	.async_fifo = true,
> > +	.data_tune = true,
> > +	.busy_check = true,
> > +	.stop_clk_fix = true,
> > +	.enhance_rx = true,
> > +	.support_64g = true,
> > +};
> > +
> >   static const struct mtk_mmc_compatible mt8135_compat = {
> >   	.clk_div_bits = 8,
> >   	.recheck_sdio_irq = true,
> > @@ -629,6 +642,7 @@ static const struct of_device_id msdc_of_ids[] = {
> >   	{ .compatible = "mediatek,mt7620-mmc", .data = &mt7620_compat},
> >   	{ .compatible = "mediatek,mt7622-mmc", .data = &mt7622_compat},
> >   	{ .compatible = "mediatek,mt7986-mmc", .data = &mt7986_compat},
> > +	{ .compatible = "mediatek,mt7988-mmc", .data = &mt7988_compat},
> >   	{ .compatible = "mediatek,mt8135-mmc", .data = &mt8135_compat},
> >   	{ .compatible = "mediatek,mt8173-mmc", .data = &mt8173_compat},
> >   	{ .compatible = "mediatek,mt8183-mmc", .data = &mt8183_compat},
>
>
AngeloGioacchino Del Regno Oct. 7, 2024, 10:14 a.m. UTC | #3 
Il 07/10/24 10:37, Frank Wunderlich ha scritto:
> Hi
> 
>> Gesendet: Montag, 07. Oktober 2024 um 09:58 Uhr
>> Von: "AngeloGioacchino Del Regno" <angelogioacchino.delregno@collabora.com>
>> Betreff: Re: [PATCH v2 2/2] mmc: mtk-sd: add support for mt7988
>>
>> Il 06/10/24 17:34, Frank Wunderlich ha scritto:
>>> From: Frank Wunderlich <frank-w@public-files.de>
>>>
>>> Add support for mmc on MT7988 SoC.
>>>
>>> Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
>>
>> There's no need to add yet one more duplicate mtk_mmc_compatible platform
>> data, nor one more compatible string to this driver, as this is exactly
>> the same as mt7986.
>>
>> Please reuse the MT7986 compatible; in DT you'll have:
>>
>> compatible = "mediatek,mt7988-mmc", "mediatek,mt7986-mmc";
> 
> as explained in binding, the clock config is completely different (except first 2 also required by driver - 3-7 are optional there). mt7988 uses axi and ahb clocks.
> 
> but i could of course use the mt7988 compatible with mt7986 compat data...but looked dirty to me so just copied the block (to allow later changes if needed).
> 

In case there will be any changes required *later*, you can always add new platform
data for the MT7988 compatible, as it's just only a code change and nothing else.

For now, since they're the same, just reuse mt7986_compat.

Reusing is way better than duplicating - here and everywhere else - especially when
this implies a 100% duplication.

>> Cheers,
>> Angelo
>>
>>> ---
>>>    drivers/mmc/host/mtk-sd.c | 14 ++++++++++++++
>>>    1 file changed, 14 insertions(+)
>>>
>>> diff --git a/drivers/mmc/host/mtk-sd.c b/drivers/mmc/host/mtk-sd.c
>>> index 89018b6c97b9..6d5afe51a61d 100644
>>> --- a/drivers/mmc/host/mtk-sd.c
>>> +++ b/drivers/mmc/host/mtk-sd.c
>>> @@ -571,6 +571,19 @@ static const struct mtk_mmc_compatible mt7986_compat = {
>>>    	.support_64g = true,
>>>    };
>>>
>>> +static const struct mtk_mmc_compatible mt7988_compat = {
>>> +	.clk_div_bits = 12,
>>> +	.recheck_sdio_irq = true,
>>> +	.hs400_tune = false,
>>> +	.pad_tune_reg = MSDC_PAD_TUNE0,
>>> +	.async_fifo = true,
>>> +	.data_tune = true,
>>> +	.busy_check = true,
>>> +	.stop_clk_fix = true,
>>> +	.enhance_rx = true,
>>> +	.support_64g = true,
>>> +};
>>> +
>>>    static const struct mtk_mmc_compatible mt8135_compat = {
>>>    	.clk_div_bits = 8,
>>>    	.recheck_sdio_irq = true,
>>> @@ -629,6 +642,7 @@ static const struct of_device_id msdc_of_ids[] = {
>>>    	{ .compatible = "mediatek,mt7620-mmc", .data = &mt7620_compat},
>>>    	{ .compatible = "mediatek,mt7622-mmc", .data = &mt7622_compat},
>>>    	{ .compatible = "mediatek,mt7986-mmc", .data = &mt7986_compat},
>>> +	{ .compatible = "mediatek,mt7988-mmc", .data = &mt7988_compat},
>>>    	{ .compatible = "mediatek,mt8135-mmc", .data = &mt8135_compat},
>>>    	{ .compatible = "mediatek,mt8173-mmc", .data = &mt8173_compat},
>>>    	{ .compatible = "mediatek,mt8183-mmc", .data = &mt8183_compat},
>>
>>
Krzysztof Kozlowski Oct. 7, 2024, 11:30 a.m. UTC | #4 
On 07/10/2024 12:14, AngeloGioacchino Del Regno wrote:
> Il 07/10/24 10:37, Frank Wunderlich ha scritto:
>> Hi
>>
>>> Gesendet: Montag, 07. Oktober 2024 um 09:58 Uhr
>>> Von: "AngeloGioacchino Del Regno" <angelogioacchino.delregno@collabora.com>
>>> Betreff: Re: [PATCH v2 2/2] mmc: mtk-sd: add support for mt7988
>>>
>>> Il 06/10/24 17:34, Frank Wunderlich ha scritto:
>>>> From: Frank Wunderlich <frank-w@public-files.de>
>>>>
>>>> Add support for mmc on MT7988 SoC.
>>>>
>>>> Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
>>>
>>> There's no need to add yet one more duplicate mtk_mmc_compatible platform
>>> data, nor one more compatible string to this driver, as this is exactly
>>> the same as mt7986.
>>>
>>> Please reuse the MT7986 compatible; in DT you'll have:
>>>
>>> compatible = "mediatek,mt7988-mmc", "mediatek,mt7986-mmc";
>>
>> as explained in binding, the clock config is completely different (except first 2 also required by driver - 3-7 are optional there). mt7988 uses axi and ahb clocks.
>>
>> but i could of course use the mt7988 compatible with mt7986 compat data...but looked dirty to me so just copied the block (to allow later changes if needed).
>>
> 
> In case there will be any changes required *later*, you can always add new platform
> data for the MT7988 compatible, as it's just only a code change and nothing else.
> 
> For now, since they're the same, just reuse mt7986_compat.
> 
> Reusing is way better than duplicating - here and everywhere else - especially when
> this implies a 100% duplication.

If you use same match data, then entire change should be dropped because
it is redundant. Instead express compatibility in the bindings and use
fallback.

Best regards,
Krzysztof
diff mbox series

Patch

diff --git a/drivers/mmc/host/mtk-sd.c b/drivers/mmc/host/mtk-sd.c
index 89018b6c97b9..6d5afe51a61d 100644
--- a/drivers/mmc/host/mtk-sd.c
+++ b/drivers/mmc/host/mtk-sd.c
@@ -571,6 +571,19 @@  static const struct mtk_mmc_compatible mt7986_compat = {
 	.support_64g = true,
 };
 
+static const struct mtk_mmc_compatible mt7988_compat = {
+	.clk_div_bits = 12,
+	.recheck_sdio_irq = true,
+	.hs400_tune = false,
+	.pad_tune_reg = MSDC_PAD_TUNE0,
+	.async_fifo = true,
+	.data_tune = true,
+	.busy_check = true,
+	.stop_clk_fix = true,
+	.enhance_rx = true,
+	.support_64g = true,
+};
+
 static const struct mtk_mmc_compatible mt8135_compat = {
 	.clk_div_bits = 8,
 	.recheck_sdio_irq = true,
@@ -629,6 +642,7 @@  static const struct of_device_id msdc_of_ids[] = {
 	{ .compatible = "mediatek,mt7620-mmc", .data = &mt7620_compat},
 	{ .compatible = "mediatek,mt7622-mmc", .data = &mt7622_compat},
 	{ .compatible = "mediatek,mt7986-mmc", .data = &mt7986_compat},
+	{ .compatible = "mediatek,mt7988-mmc", .data = &mt7988_compat},
 	{ .compatible = "mediatek,mt8135-mmc", .data = &mt8135_compat},
 	{ .compatible = "mediatek,mt8173-mmc", .data = &mt8173_compat},
 	{ .compatible = "mediatek,mt8183-mmc", .data = &mt8183_compat},