On Mon, Jun 17, 2024 at 10:57:54AM GMT, Samuel Holland wrote: > Hi Jisheng, Thomas, > > On 2024-06-17 10:40 AM, Conor Dooley wrote: > > On Mon, Jun 17, 2024 at 09:16:43PM +0800, Jisheng Zhang wrote: > >> On Mon, Jun 17, 2024 at 11:16:32AM +0200, Thomas Bonnefille wrote: > >>> On 6/17/24 1:58 AM, Yixun Lan wrote: > >>>> On 18:47 Wed 12 Jun , Inochi Amaoto wrote: > > > >>>>> Is this change necessary? IIRC, the sdhci is the same across > >>>>> the whole series. > > > >> 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. > > > > For new integrations of an IP, yes, new specific compatibles please. New > > integrations may have different bugs etc, even if the IP itself is the > > same. If there's different SoCs that are the same die, but with elements > > fused off, then sure, use the same compatible. > > > > I expect the list of compatibles in the binding to grow rather large, but > > that is fine. No one SoC is going to do anything other than something like > > compatible = "renesas,$soc-plic", "andestech,corecomplex-plic", "riscv,plic"; > > which I think is perfectly fine. > > And you can do the same thing here for the SDHCI controller: if you think sg200x > has the same controller (and integration! e.g. number of clocks/resets) as > cv1800b, then you should keep sophgo,cv1800b-dwcmshc as a fallback compatible > string. Then the driver doesn't need any changes until/unless you eventually > find some reason they are not compatible. > > It's better to have a SoC-specific compatible string in the DT and not need it, > than find out later you need one and not have it. :) > > Regards, > Samuel > This is excellect and reasonable. I will take your advice for the DT change. With your suggetion, I think it may be acceptable to mark the low-profile SoC as the default value and let other override it. Let take the clk as the example: // in the base file cv18xx.dtsi clk: clock-controller@3002000 { // set the "sophgo,cv1800-clk" as the fallback. compatible = "sophgo,cv1800-clk"; [...] }; // in the cv1800b.dtsi // now no need for the clk override since we have the compatible // default. // in the cv1812h.dtsi // we still need this override as it is not compatible &clk { compatible = "sophgo,cv1810-clk"; }; // in the sg2002.dtsi of the patch // we need this override as it is also not compatible &clk { compatible = "sophgo,sg2000-clk"; }; Do I understand correctly? Or we still need to duplicate these node for SoCs with different profiles. Regards, Inochi