Re: [PATCH 2/2] mmc: sdhci-of-k1: spacemit: Add support for K3 SoC

From: Yixun Lan <dlan@kernel.org>
To: Yao Zi <me@ziyao.cc>
Cc: Ulf Hansson <ulf.hansson@linaro.org>,
	Rob Herring <robh@kernel.org>,
	Krzysztof Kozlowski <krzk+dt@kernel.org>,
	Conor Dooley <conor+dt@kernel.org>,
	Adrian Hunter <adrian.hunter@intel.com>,
	linux-mmc@vger.kernel.org, devicetree@vger.kernel.org,
	linux-riscv@lists.infradead.org, spacemit@lists.linux.dev,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 2/2] mmc: sdhci-of-k1: spacemit: Add support for K3 SoC
Date: Wed, 21 Jan 2026 18:44:53 +0800	[thread overview]
Message-ID: <20260121104453-GKD61591@kernel.org> (raw)
In-Reply-To: <aXCg43ZepMm1DNHq@pie>

On 09:48 Wed 21 Jan     , Yao Zi wrote:
> On Wed, Jan 21, 2026 at 03:27:36PM +0800, Yixun Lan wrote:
> > The SDHCI controller found on SpacemiT K3 SoC share the same IP with K1
> > generation and introduce a compatible data to denote the change that broken
> > 64BIT DMA issue has been fixed.
> > 
> > Signed-off-by: Yixun Lan <dlan@kernel.org>
> > ---
> >  drivers/mmc/host/sdhci-of-k1.c | 22 ++++++++++++++++++++--
> >  1 file changed, 20 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/mmc/host/sdhci-of-k1.c b/drivers/mmc/host/sdhci-of-k1.c
> > index a160e1d5d9bd..a56cb8a36982 100644
> > --- a/drivers/mmc/host/sdhci-of-k1.c
> > +++ b/drivers/mmc/host/sdhci-of-k1.c
> > @@ -259,8 +259,20 @@ static const struct sdhci_pltfm_data spacemit_sdhci_k1_pdata = {
> >  		   SDHCI_QUIRK2_PRESET_VALUE_BROKEN,
> >  };
> >  
> > +static const struct sdhci_pltfm_data spacemit_sdhci_k3_pdata = {
> > +	.ops = &spacemit_sdhci_ops,
> > +	.quirks = SDHCI_QUIRK_DATA_TIMEOUT_USES_SDCLK |
> > +		  SDHCI_QUIRK_NO_ENDATTR_IN_NOPDESC |
> > +		  SDHCI_QUIRK_32BIT_ADMA_SIZE |
> > +		  SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN |
> > +		  SDHCI_QUIRK_BROKEN_CARD_DETECTION |
> > +		  SDHCI_QUIRK_BROKEN_TIMEOUT_VAL,
> > +	.quirks2 = SDHCI_QUIRK2_PRESET_VALUE_BROKEN,
> > +};
> > +
> >  static const struct of_device_id spacemit_sdhci_of_match[] = {
> > -	{ .compatible = "spacemit,k1-sdhci" },
> > +	{ .compatible = "spacemit,k1-sdhci", .data = &spacemit_sdhci_k1_pdata },
> > +	{ .compatible = "spacemit,k3-sdhci", .data = &spacemit_sdhci_k3_pdata },
> >  	{ /* sentinel */ }
> >  };
> >  MODULE_DEVICE_TABLE(of, spacemit_sdhci_of_match);
> > @@ -271,10 +283,16 @@ static int spacemit_sdhci_probe(struct platform_device *pdev)
> >  	struct spacemit_sdhci_host *sdhst;
> >  	struct sdhci_pltfm_host *pltfm_host;
> >  	struct sdhci_host *host;
> > +	const struct sdhci_pltfm_data *data;
> >  	struct mmc_host_ops *mops;
> >  	int ret;
> >  
> > -	host = sdhci_pltfm_init(pdev, &spacemit_sdhci_k1_pdata, sizeof(*sdhst));
> > +
got one extra blank line here, will drop in next version

> > +	data = of_device_get_match_data(&pdev->dev);
> > +	if (!data)
> > +		return dev_err_probe(dev, -EINVAL, "failed to get platform data\n");
> 
> Is there a need to check against return value of
> of_device_get_match_data()? It isn't a user input, instead missing the
> data seems a kernel bug to me, and shouldn't happen in production.
> 
I agree, it should be safe to drop this check

> Regards,
> Yao Zi

-- 
Yixun Lan (dlan)