Re: Aw: Re: [PATCH v2 2/2] mmc: mtk-sd: add support for mt7988

[AngeloGioacchino Del Regno]

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@xxxxxxxxxxxxx>
> > 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@xxxxxxxxxxxxxxx>
> > >
> > > Add support for mmc on MT7988 SoC.
> > >
> > > Signed-off-by: Frank Wunderlich <frank-w@xxxxxxxxxxxxxxx>
> >
> > 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},