[PATCH 0/2] mmc: mtk-sd: Limit second register range to SoCs requiring it

[PATCH 0/2] mmc: mtk-sd: Limit second register range to SoCs requiring it

[Chen-Yu Tsai]

Currently the usage of a second register range for MediaTek's MMC
controller is seen only on a few platforms. However the driver tries
to grab the register base regardless, and then ignores any errors.
This makes all the other platforms that don't need it produce confusing
error messages.

Instead, make the driver only grab the register range for platforms that
actually require it, and correctly handle the error. Also fix up the
binding so that validation of the "reg" property is as strict as
possible.

Please take a look.


Thanks
ChenYu

Chen-Yu Tsai (2):
  dt-bindings: mmc: mtk-sd: Document compatibles that need two register
    ranges
  mmc: mtk-sd: Limit getting top_base to SoCs that require it

 Documentation/devicetree/bindings/mmc/mtk-sd.yaml |  9 ++++++++-
 drivers/mmc/host/mtk-sd.c                         | 15 +++++++++++----
 2 files changed, 19 insertions(+), 5 deletions(-)

-- 
2.47.0.338.g60cca15819-goog

[PATCH 1/2] dt-bindings: mmc: mtk-sd: Document compatibles that need two register ranges

[Chen-Yu Tsai]

Besides the MT8183's MMC controller and all its compatible derivatives,
the recently added MT7986 and MT8196 also require two register ranges.
This is based on the actual device trees.

Properly enforce this in the binding.

Fixes: 4a8bd2b07d88 ("dt-bindings: mmc: mtk-sd: Add mt7988 SoC")
Fixes: 58927c9dc4ab ("dt-bindings: mmc: mtk-sd: Add support for MT8196")
Cc: Frank Wunderlich <frank-w@public-files.de>
Cc: Andy-ld Lu <andy-ld.lu@mediatek.com>
Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
---
 Documentation/devicetree/bindings/mmc/mtk-sd.yaml | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/mmc/mtk-sd.yaml b/Documentation/devicetree/bindings/mmc/mtk-sd.yaml
index f86ebd81f5a5..9ea035928563 100644
--- a/Documentation/devicetree/bindings/mmc/mtk-sd.yaml
+++ b/Documentation/devicetree/bindings/mmc/mtk-sd.yaml
@@ -235,11 +235,18 @@ allOf:
       properties:
         compatible:
           contains:
-            const: mediatek,mt8183-mmc
+            enum:
+              - mediatek,mt7986-mmc
+              - mediatek,mt8183-mmc
+              - mediatek,mt8196-mmc
     then:
       properties:
         reg:
           minItems: 2
+    else:
+      properties:
+        reg:
+          maxItems: 1
 
   - if:
       properties:
-- 
2.47.0.338.g60cca15819-goog

[PATCH 2/2] mmc: mtk-sd: Limit getting top_base to SoCs that require it

[Chen-Yu Tsai]

Currently the mtk-sd driver tries to get and map the second register
base, named top_base in the code, regardless of whether the SoC model
actually has it or not. This produces confusing big error messages on
the platforms that don't need it:

    mtk-msdc 11260000.mmc: error -EINVAL: invalid resource (null)

Limit it to the platforms that actually require it, based on their
device tree entries, and properly fail if it is missing. There is
no MMC node in the MT6779 dts, so it's currently unknown if that
platform needs it or not.

Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
---
 drivers/mmc/host/mtk-sd.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/drivers/mmc/host/mtk-sd.c b/drivers/mmc/host/mtk-sd.c
index e2c385853eef..1bb7044f4ca1 100644
--- a/drivers/mmc/host/mtk-sd.c
+++ b/drivers/mmc/host/mtk-sd.c
@@ -414,6 +414,7 @@ struct mtk_mmc_compatible {
 	u8 clk_div_bits;
 	bool recheck_sdio_irq;
 	bool hs400_tune; /* only used for MT8173 */
+	bool needs_top_base;
 	u32 pad_tune_reg;
 	bool async_fifo;
 	bool data_tune;
@@ -587,6 +588,7 @@ static const struct mtk_mmc_compatible mt7986_compat = {
 	.clk_div_bits = 12,
 	.recheck_sdio_irq = true,
 	.hs400_tune = false,
+	.needs_top_base = true,
 	.pad_tune_reg = MSDC_PAD_TUNE0,
 	.async_fifo = true,
 	.data_tune = true,
@@ -627,6 +629,7 @@ static const struct mtk_mmc_compatible mt8183_compat = {
 	.clk_div_bits = 12,
 	.recheck_sdio_irq = false,
 	.hs400_tune = false,
+	.needs_top_base = true,
 	.pad_tune_reg = MSDC_PAD_TUNE0,
 	.async_fifo = true,
 	.data_tune = true,
@@ -653,6 +656,7 @@ static const struct mtk_mmc_compatible mt8196_compat = {
 	.clk_div_bits = 12,
 	.recheck_sdio_irq = false,
 	.hs400_tune = false,
+	.needs_top_base = true,
 	.pad_tune_reg = MSDC_PAD_TUNE0,
 	.async_fifo = true,
 	.data_tune = true,
@@ -2887,9 +2891,13 @@ static int msdc_drv_probe(struct platform_device *pdev)
 	if (IS_ERR(host->base))
 		return PTR_ERR(host->base);
 
-	host->top_base = devm_platform_ioremap_resource(pdev, 1);
-	if (IS_ERR(host->top_base))
-		host->top_base = NULL;
+	host->dev_comp = of_device_get_match_data(&pdev->dev);
+
+	if (host->dev_comp->needs_top_base) {
+		host->top_base = devm_platform_ioremap_resource(pdev, 1);
+		if (IS_ERR(host->top_base))
+			return PTR_ERR(host->top_base);
+	}
 
 	ret = mmc_regulator_get_supply(mmc);
 	if (ret)
@@ -2951,7 +2959,6 @@ static int msdc_drv_probe(struct platform_device *pdev)
 	msdc_of_property_parse(pdev, host);
 
 	host->dev = &pdev->dev;
-	host->dev_comp = of_device_get_match_data(&pdev->dev);
 	host->src_clk_freq = clk_get_rate(host->src_clk);
 	/* Set host parameters to mmc */
 	mmc->ops = &mt_msdc_ops;
-- 
2.47.0.338.g60cca15819-goog

Re: [PATCH 1/2] dt-bindings: mmc: mtk-sd: Document compatibles that need two register ranges

[Frank Wunderlich]

Am 4. Dezember 2024 10:28:52 MEZ schrieb Chen-Yu Tsai <wenst@chromium.org>:
>Besides the MT8183's MMC controller and all its compatible derivatives,
>the recently added MT7986 and MT8196 also require two register ranges.
>This is based on the actual device trees.

Hi

Mt7988 is similar to mt7986 (only using different clock settings) and using 2 reg too.

Currently there is no mmc node in mt7988 dts (i currently upstreaming it) so i guess this is why you have not yet noticed it.

See this for more:

https://patchwork.kernel.org/project/linux-mediatek/patch/20241202122602.30734-3-linux@fw-web.de/

Maybe you can add mt7988 compatible here too in v2.

Regards Frank

>Properly enforce this in the binding.
>
>Fixes: 4a8bd2b07d88 ("dt-bindings: mmc: mtk-sd: Add mt7988 SoC")
>Fixes: 58927c9dc4ab ("dt-bindings: mmc: mtk-sd: Add support for MT8196")
>Cc: Frank Wunderlich <frank-w@public-files.de>
>Cc: Andy-ld Lu <andy-ld.lu@mediatek.com>
>Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
>---
> Documentation/devicetree/bindings/mmc/mtk-sd.yaml | 9 ++++++++-
> 1 file changed, 8 insertions(+), 1 deletion(-)
>
>diff --git a/Documentation/devicetree/bindings/mmc/mtk-sd.yaml b/Documentation/devicetree/bindings/mmc/mtk-sd.yaml
>index f86ebd81f5a5..9ea035928563 100644
>--- a/Documentation/devicetree/bindings/mmc/mtk-sd.yaml
>+++ b/Documentation/devicetree/bindings/mmc/mtk-sd.yaml
>@@ -235,11 +235,18 @@ allOf:
>       properties:
>         compatible:
>           contains:
>-            const: mediatek,mt8183-mmc
>+            enum:
>+              - mediatek,mt7986-mmc
>+              - mediatek,mt8183-mmc
>+              - mediatek,mt8196-mmc
>     then:
>       properties:
>         reg:
>           minItems: 2
>+    else:
>+      properties:
>+        reg:
>+          maxItems: 1
> 
>   - if:
>       properties:


regards Frank

Re: [PATCH 1/2] dt-bindings: mmc: mtk-sd: Document compatibles that need two register ranges

[Chen-Yu Tsai]

On Wed, Dec 4, 2024 at 6:38 PM Frank Wunderlich <frank-w@public-files.de> wrote:
>
> Am 4. Dezember 2024 10:28:52 MEZ schrieb Chen-Yu Tsai <wenst@chromium.org>:
> >Besides the MT8183's MMC controller and all its compatible derivatives,
> >the recently added MT7986 and MT8196 also require two register ranges.
> >This is based on the actual device trees.
>
> Hi
>
> Mt7988 is similar to mt7986 (only using different clock settings) and using 2 reg too.
>
> Currently there is no mmc node in mt7988 dts (i currently upstreaming it) so i guess this is why you have not yet noticed it.
>
> See this for more:
>
> https://patchwork.kernel.org/project/linux-mediatek/patch/20241202122602.30734-3-linux@fw-web.de/

I think I noticed it at the beginning, but it then slipped my mind. I
might have gotten it mixed up with mt7986.

> Maybe you can add mt7988 compatible here too in v2.

Will do.

Thanks
ChenYu

> Regards Frank
>
> >Properly enforce this in the binding.
> >
> >Fixes: 4a8bd2b07d88 ("dt-bindings: mmc: mtk-sd: Add mt7988 SoC")
> >Fixes: 58927c9dc4ab ("dt-bindings: mmc: mtk-sd: Add support for MT8196")
> >Cc: Frank Wunderlich <frank-w@public-files.de>
> >Cc: Andy-ld Lu <andy-ld.lu@mediatek.com>
> >Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
> >---
> > Documentation/devicetree/bindings/mmc/mtk-sd.yaml | 9 ++++++++-
> > 1 file changed, 8 insertions(+), 1 deletion(-)
> >
> >diff --git a/Documentation/devicetree/bindings/mmc/mtk-sd.yaml b/Documentation/devicetree/bindings/mmc/mtk-sd.yaml
> >index f86ebd81f5a5..9ea035928563 100644
> >--- a/Documentation/devicetree/bindings/mmc/mtk-sd.yaml
> >+++ b/Documentation/devicetree/bindings/mmc/mtk-sd.yaml
> >@@ -235,11 +235,18 @@ allOf:
> >       properties:
> >         compatible:
> >           contains:
> >-            const: mediatek,mt8183-mmc
> >+            enum:
> >+              - mediatek,mt7986-mmc
> >+              - mediatek,mt8183-mmc
> >+              - mediatek,mt8196-mmc
> >     then:
> >       properties:
> >         reg:
> >           minItems: 2
> >+    else:
> >+      properties:
> >+        reg:
> >+          maxItems: 1
> >
> >   - if:
> >       properties:
>
>
> regards Frank

Re: [PATCH 1/2] dt-bindings: mmc: mtk-sd: Document compatibles that need two register ranges

[AngeloGioacchino Del Regno]

Il 04/12/24 10:28, Chen-Yu Tsai ha scritto:
> Besides the MT8183's MMC controller and all its compatible derivatives,
> the recently added MT7986 and MT8196 also require two register ranges.
> This is based on the actual device trees.
> 
> Properly enforce this in the binding.
> 
> Fixes: 4a8bd2b07d88 ("dt-bindings: mmc: mtk-sd: Add mt7988 SoC")
> Fixes: 58927c9dc4ab ("dt-bindings: mmc: mtk-sd: Add support for MT8196")
> Cc: Frank Wunderlich <frank-w@public-files.de>
> Cc: Andy-ld Lu <andy-ld.lu@mediatek.com>
> Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

Re: [PATCH 2/2] mmc: mtk-sd: Limit getting top_base to SoCs that require it

[AngeloGioacchino Del Regno]

Il 04/12/24 10:28, Chen-Yu Tsai ha scritto:
> Currently the mtk-sd driver tries to get and map the second register
> base, named top_base in the code, regardless of whether the SoC model
> actually has it or not. This produces confusing big error messages on
> the platforms that don't need it:
> 
>      mtk-msdc 11260000.mmc: error -EINVAL: invalid resource (null)
> 
> Limit it to the platforms that actually require it, based on their
> device tree entries, and properly fail if it is missing. There is
> no MMC node in the MT6779 dts, so it's currently unknown if that
> platform needs it or not.
> 

No, it doesn't require it. The controller is (very) similar to the one
found on MT6795 :-)

> Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>

Can you please remove that "I don't know about mt6779" mention, now that
you do know?

Btw,

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

> ---
>   drivers/mmc/host/mtk-sd.c | 15 +++++++++++----
>   1 file changed, 11 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/mmc/host/mtk-sd.c b/drivers/mmc/host/mtk-sd.c
> index e2c385853eef..1bb7044f4ca1 100644
> --- a/drivers/mmc/host/mtk-sd.c
> +++ b/drivers/mmc/host/mtk-sd.c
> @@ -414,6 +414,7 @@ struct mtk_mmc_compatible {
>   	u8 clk_div_bits;
>   	bool recheck_sdio_irq;
>   	bool hs400_tune; /* only used for MT8173 */
> +	bool needs_top_base;
>   	u32 pad_tune_reg;
>   	bool async_fifo;
>   	bool data_tune;
> @@ -587,6 +588,7 @@ static const struct mtk_mmc_compatible mt7986_compat = {
>   	.clk_div_bits = 12,
>   	.recheck_sdio_irq = true,
>   	.hs400_tune = false,
> +	.needs_top_base = true,
>   	.pad_tune_reg = MSDC_PAD_TUNE0,
>   	.async_fifo = true,
>   	.data_tune = true,
> @@ -627,6 +629,7 @@ static const struct mtk_mmc_compatible mt8183_compat = {
>   	.clk_div_bits = 12,
>   	.recheck_sdio_irq = false,
>   	.hs400_tune = false,
> +	.needs_top_base = true,
>   	.pad_tune_reg = MSDC_PAD_TUNE0,
>   	.async_fifo = true,
>   	.data_tune = true,
> @@ -653,6 +656,7 @@ static const struct mtk_mmc_compatible mt8196_compat = {
>   	.clk_div_bits = 12,
>   	.recheck_sdio_irq = false,
>   	.hs400_tune = false,
> +	.needs_top_base = true,
>   	.pad_tune_reg = MSDC_PAD_TUNE0,
>   	.async_fifo = true,
>   	.data_tune = true,
> @@ -2887,9 +2891,13 @@ static int msdc_drv_probe(struct platform_device *pdev)
>   	if (IS_ERR(host->base))
>   		return PTR_ERR(host->base);
>   
> -	host->top_base = devm_platform_ioremap_resource(pdev, 1);
> -	if (IS_ERR(host->top_base))
> -		host->top_base = NULL;
> +	host->dev_comp = of_device_get_match_data(&pdev->dev);
> +
> +	if (host->dev_comp->needs_top_base) {
> +		host->top_base = devm_platform_ioremap_resource(pdev, 1);
> +		if (IS_ERR(host->top_base))
> +			return PTR_ERR(host->top_base);
> +	}
>   
>   	ret = mmc_regulator_get_supply(mmc);
>   	if (ret)
> @@ -2951,7 +2959,6 @@ static int msdc_drv_probe(struct platform_device *pdev)
>   	msdc_of_property_parse(pdev, host);
>   
>   	host->dev = &pdev->dev;
> -	host->dev_comp = of_device_get_match_data(&pdev->dev);
>   	host->src_clk_freq = clk_get_rate(host->src_clk);
>   	/* Set host parameters to mmc */
>   	mmc->ops = &mt_msdc_ops;