[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20260205-choice-require-1794796b10e5@spud>
Date: Thu, 5 Feb 2026 19:19:36 +0000
From: Conor Dooley <conor@...nel.org>
To: Jiayu Du <jiayu.riscv@...c.iscas.ac.cn>
Cc: ulf.hansson@...aro.org, adrian.hunter@...el.com, robh@...nel.org,
krzk+dt@...nel.org, conor+dt@...nel.org, pjw@...nel.org,
palmer@...belt.com, aou@...s.berkeley.edu,
linux-mmc@...r.kernel.org, devicetree@...r.kernel.org,
linux-riscv@...ts.infradead.org, linux-kernel@...r.kernel.org,
gaohan@...as.ac.cn, me@...ao.cc
Subject: Re: [PATCH 1/3] dt-bindings: mmc: Add sdhci support for Canaan k230
On Thu, Feb 05, 2026 at 03:13:00PM +0800, Jiayu Du wrote:
> On Wed, Feb 04, 2026 at 06:10:25PM +0000, Conor Dooley wrote:
> > On Wed, Feb 04, 2026 at 04:29:06PM +0800, Jiayu Du wrote:
> > > The Canaan k230 uses the SDHCI from Synopsys. Add compatible strings
> > > to the k230. The k230 has two controllers. MMC0 supports eMMC, while
> > > MMC1 supports SDIO.
> > >
> > > Signed-off-by: Jiayu Du <jiayu.riscv@...c.iscas.ac.cn>
> > > ---
> > > .../bindings/mmc/snps,dwcmshc-sdhci.yaml | 22 +++++++++++++++++++
> > > 1 file changed, 22 insertions(+)
> > >
> > > diff --git a/Documentation/devicetree/bindings/mmc/snps,dwcmshc-sdhci.yaml b/Documentation/devicetree/bindings/mmc/snps,dwcmshc-sdhci.yaml
> > > index 7e7c55dc2440..cab33da3af7d 100644
> > > --- a/Documentation/devicetree/bindings/mmc/snps,dwcmshc-sdhci.yaml
> > > +++ b/Documentation/devicetree/bindings/mmc/snps,dwcmshc-sdhci.yaml
> > > @@ -23,6 +23,8 @@ properties:
> > > - const: sophgo,sg2044-dwcmshc
> > > - const: sophgo,sg2042-dwcmshc
> > > - enum:
> > > + - canaan,k230-emmc
> > > + - canaan,k230-sdio
> >
> > I don't understand why there are two compatibles here, when the driver
> > is able to handle them both identically, using the common properties for
> > mmc controllers to differentiate. Is which is emmc and which sdio
> > actually just determined by how the k230 boards use them?
> >
> > Or, if there are pinout differences, would pinctrl allow each to be used
> > for either?
>
> Hi, Thanks for the review!
>
> Actually, the two compatibles aren't just board choices. They come from
> real SoC-level differences between the two MMC controllers.
>
> From the K230 Hardware Guide [1]:
> - MMC0 supports eMMC5.0 and SDIO3.0, usually for eMMC chips.
> - MMC1 only does SDIO3.0 in 4/1-bit mode up to SDR104, and the manual
> clearly says it can't handle eMMC because of pin count and limits.
>
> And the driver treats them a bit differently, such as whether there
> is a phy and the initialization method. So I think keeping separate
> compatibles makes sense to show these hardware differences clearly.
Ah, I didn't notice that there was some logic, I just checked the match
data and I missed that. What you have is fine then I think.
>
> About pinctrl, MMC1 pins can mux to other things like GPIO, but MMC0
> pins are fixed in hardware. In next version, I will add pinctrl node
> for MMC1 in the board dts.
>
> Link: https://github.com/kendryte/k230_docs/blob/main/en/00_hardware/K230_Hardware_Design_Guide.md#mmc-circuit [1]
> >
> > > - rockchip,rk3568-dwcmshc
> > > - rockchip,rk3588-dwcmshc
> > > - snps,dwcmshc-sdhci
> > > @@ -87,6 +89,26 @@ required:
> > > allOf:
> > > - $ref: mmc-controller.yaml#
> > >
> > > + - if:
> > > + properties:
> > > + compatible:
> > > + contains:
> > > + enum:
> > > + - canaan,k230-emmc
> > > + - canaan,k230-sdio
> > > + then:
> > > + properties:
> > > + clocks:
> > > + minItems: 2
> > > + maxItems: 5
Additionally, why does this have a range? Why is it not minItems: 5?
You've got only one instance per compatible, so the number of clocks for
each compatible is not variable.
> > > + clock-names:
> > > + items:
> > > + - const: core
> > > + - const: bus
> > > + - const: axi
> > > + - const: block
> > > + - const: timer
> > > +
> > > - if:
> > > properties:
> > > compatible:
> > > --
> > > 2.52.0
> > >
>
>
Download attachment "signature.asc" of type "application/pgp-signature" (229 bytes)
Powered by blists - more mailing lists