Re: [PATCH v2 2/3] mmc: sdhci-dwcmshc: Add Canaan K230 DWCMSHC controller support

[Jiayu Du <jiayu.riscv@isrc.iscas.ac.cn>]

On Thu, Mar 05, 2026 at 12:48:29PM +0100, Ulf Hansson wrote:
> On Thu, 26 Feb 2026 at 12:59, Jiayu Du <jiayu.riscv@isrc.iscas.ac.cn> wrote:
> >
> > Add SDHCI controller driver for Canaan k230 SoC. Implement custom
> > sdhci_ops for set_clock, phy init, init and reset.
> >
> > Signed-off-by: Jiayu Du <jiayu.riscv@isrc.iscas.ac.cn>
> > ---
> >  drivers/mmc/host/sdhci-of-dwcmshc.c | 288 ++++++++++++++++++++++++++++
> >  1 file changed, 288 insertions(+)
> >
> > diff --git a/drivers/mmc/host/sdhci-of-dwcmshc.c b/drivers/mmc/host/sdhci-of-dwcmshc.c
> > index 2b75a36c096b..21c77e908d77 100644
> > --- a/drivers/mmc/host/sdhci-of-dwcmshc.c
> > +++ b/drivers/mmc/host/sdhci-of-dwcmshc.c
...
> >  struct dwcmshc_pltfm_data {
> > +       const void *match_data;
> 
> This makes sense to me!
> 
> Although, I realized that dwcmshc_rk35xx_init() could also move its
> assignment of "devtype" into this match_data.
> 
> Can you please create a follow-up patch to fixup this and to avoid
> storing this type of data in two different ways?
> 
...
> > +static int dwcmshc_k230_init(struct device *dev, struct sdhci_host *host,
> > +                            struct dwcmshc_priv *dwc_priv)
> > +{
> > +       static const char * const clk_ids[] = {"block", "timer", "axi"};
> > +       const struct dwcmshc_k230_match_data *match_data;
> > +       const struct dwcmshc_pltfm_data *pltfm_data;
> > +       struct device_node *usb_phy_node;
> > +       struct k230_priv *k230_priv;
> > +       u32 data;
> > +       int ret;
> > +
> > +       pltfm_data = device_get_match_data(dev);
> > +
> > +       if (!pltfm_data || !pltfm_data->match_data) {
> > +               dev_err(dev, "No vendor data found for K230\n");
> > +               return -EINVAL;
> > +       }
> > +       match_data = pltfm_data->match_data;
> 
> I don't think this should be specific to dwcmshc_k230_init().
> 
> Instead I suggest adding a "const void *match_data" to the "struct
> dwcmshc_priv" - and copy the pointer in the common dwcmshc_probe()
> instead. In this way, all variants will be able to use it.

Your suggestions make perfect sense. I'll prepare a new version to add
the "const void *match_data" to "struct dwcmshc_priv", assign it in
"dwcmshc_probe()", and adjust the K230 init accordingly.

I'll also migrate the rk35xx devtype assignment to match_data for
consistency. Once ready, I'll post the link as a reply to this k230 mmc
series.

Best regards,
Jiayu Du

> 
> Kind regards
> Uffe