Re: [PATCH v2 1/6] riscv: dts: sophgo: Put sdhci compatible in dt of specific SoC

[Jisheng Zhang <jszhang@kernel.org>]

On Mon, Jun 17, 2024 at 11:16:32AM +0200, Thomas Bonnefille wrote:
> 
> 
> On 6/17/24 1:58 AM, Yixun Lan wrote:
> > Hi
> > 
> > On 18:47 Wed 12 Jun     , Inochi Amaoto wrote:
> > > On Wed, Jun 12, 2024 at 10:02:31AM GMT, Thomas Bonnefille wrote:
> > > > Remove SDHCI compatible for CV1800b from common dtsi file to put it in
> > > > the specific dtsi file of the CV1800b.
> > > > This commits aims at following the same guidelines as in the other nodes
> > > > of the CV18XX family.
> > is there any URL of guideline? or did I miss anything
> > couldn't find any discussion about this in v1
> > 
> 
> Not explicitly, the fact is that I had to use a specific compatible on
> SG2002 for the sdhci (it is already defined mainline), I had to choose
> between :
> 
> 1. cv18xx.dtsi : compatible cv1800b-dwcmshc
>    cv1800b.dtsi : no redefined compatible
>    sg2002.dtsi : overwrite the previous compatible to use sg2002-dwcmshc
> 
> 2. cv18xx.dtsi : no compatible
>    cv1800b.dtsi : compatible for cv1800b-dwcmshc
>    sg2002.dtsi : compatible for sg2002-dwcmshc
> 
> As in the plic and clint controllers, the second option was chosen I
> consider this as a "guideline" and reformat the dtsis accordingly.
> 
> > > > 
> > > > Signed-off-by: Thomas Bonnefille <thomas.bonnefille@bootlin.com>
> > > > ---
> > > >   arch/riscv/boot/dts/sophgo/cv1800b.dtsi | 4 ++++
> > > >   arch/riscv/boot/dts/sophgo/cv18xx.dtsi  | 1 -
> > > >   2 files changed, 4 insertions(+), 1 deletion(-)
> > > > 
> > > > diff --git a/arch/riscv/boot/dts/sophgo/cv1800b.dtsi b/arch/riscv/boot/dts/sophgo/cv1800b.dtsi
> > > > index ec9530972ae2..b9cd51457b4c 100644
> > > > --- a/arch/riscv/boot/dts/sophgo/cv1800b.dtsi
> > > > +++ b/arch/riscv/boot/dts/sophgo/cv1800b.dtsi
> > > > @@ -25,3 +25,7 @@ &clint {
> > > >   &clk {
> > > >   	compatible = "sophgo,cv1800-clk";
> > > >   };
> > > > +
> > > > +&sdhci0 {
> > > > +	compatible = "sophgo,cv1800b-dwcmshc";
> > > > +};
> > > > diff --git a/arch/riscv/boot/dts/sophgo/cv18xx.dtsi b/arch/riscv/boot/dts/sophgo/cv18xx.dtsi
> > > > index 891932ae470f..7247c7c3013c 100644
> > > > --- a/arch/riscv/boot/dts/sophgo/cv18xx.dtsi
> > > > +++ b/arch/riscv/boot/dts/sophgo/cv18xx.dtsi
> > > > @@ -288,7 +288,6 @@ uart4: serial@41c0000 {
> > > >   		};
> > > >   		sdhci0: mmc@4310000 {
> > > > -			compatible = "sophgo,cv1800b-dwcmshc";
> > > >   			reg = <0x4310000 0x1000>;
> > > >   			interrupts = <36 IRQ_TYPE_LEVEL_HIGH>;
> > > >   			clocks = <&clk CLK_AXI4_SD0>,
> > > > 
> > > > -- 
> > > > 2.45.2
> > > > 
> > > 
> > > Hi, Jisheng,
> > > 
> > > Is this change necessary? IIRC, the sdhci is the same across
> > > the whole series.

Hi,

sorry for being late, I was busy in the past 2.5 month. Per my
understanding, the sdhci in cv1800b is the same as the one in
sg200x. Maybe I'm wrong, but this was my impression when I cooked
the sdhci driver patch for these SoCs.

> > I tend to agree with Inochi here, if it's same across all SoC, then no bother to
> > split, it will cause more trouble to maintain..
> > 
> 
> To be honest, I agree with this to, but as a specific compatible for the
> SG2002 was created in commit 849e81817b9b, I thought that the best practice
> was to use it.

I'd like to take this chance to query DT maintainers: FWICT, in the past
even if the PLIC is the same between SoCs, adding a new compatible for
them seems a must. So when time goes on, the compatbile list would be
longer and longer, is it really necessary? Can we just use the existing
compatible string?
DT maintainers may answered the query in the past, if so, sorry for
querying again.

> 
> > > 
> > > Regards,
> > > Inochi
> > 
> 
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv