RE: [PATCH v3 2/3] dt-bindings: i2c-ast2600: Add bindings for AST2600 i2C driver

[Ryan Chen]

Hello,

> -----Original Message-----
> From: Andrew Jeffery <andrew@xxxxxxxx>
> Sent: Tuesday, August 9, 2022 8:35 AM
> To: Ryan Chen <ryan_chen@xxxxxxxxxxxxxx>; Joel Stanley <joel@xxxxxxxxx>;
> Philipp Zabel <p.zabel@xxxxxxxxxxxxxx>; linux-arm-kernel@xxxxxxxxxxxxxxxxxxx;
> linux-aspeed@xxxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx;
> openbmc@xxxxxxxxxxxxxxxx
> Cc: BMC-SW <BMC-SW@xxxxxxxxxxxxxx>
> Subject: Re: [PATCH v3 2/3] dt-bindings: i2c-ast2600: Add bindings for AST2600
> i2C driver
> 
> 
> 
> On Tue, 2 Aug 2022, at 18:34, Ryan Chen wrote:
> > Hello,
> >
> >> -----Original Message-----
> >> From: Andrew Jeffery <andrew@xxxxxxxx>
> >> Sent: Friday, July 29, 2022 11:13 AM
> >> To: Ryan Chen <ryan_chen@xxxxxxxxxxxxxx>; Joel Stanley
> >> <joel@xxxxxxxxx>; Philipp Zabel <p.zabel@xxxxxxxxxxxxxx>;
> >> linux-arm-kernel@xxxxxxxxxxxxxxxxxxx;
> >> linux-aspeed@xxxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx;
> >> openbmc@xxxxxxxxxxxxxxxx
> >> Cc: BMC-SW <BMC-SW@xxxxxxxxxxxxxx>
> >> Subject: Re: [PATCH v3 2/3] dt-bindings: i2c-ast2600: Add bindings
> >> for AST2600 i2C driver
> >>
> >>
> >>
> >> On Fri, 29 Jul 2022, at 12:33, Ryan Chen wrote:
> >> > Hello Andrew,
> >> >
> >> >> -----Original Message-----
> >> >> From: Andrew Jeffery <andrew@xxxxxxxx>
> >> >> Sent: Friday, July 29, 2022 10:29 AM
> >> >> To: Ryan Chen <ryan_chen@xxxxxxxxxxxxxx>; Joel Stanley
> >> >> <joel@xxxxxxxxx>; Philipp Zabel <p.zabel@xxxxxxxxxxxxxx>;
> >> >> linux-arm-kernel@xxxxxxxxxxxxxxxxxxx;
> >> >> linux-aspeed@xxxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx;
> >> >> openbmc@xxxxxxxxxxxxxxxx
> >> >> Cc: BMC-SW <BMC-SW@xxxxxxxxxxxxxx>
> >> >> Subject: Re: [PATCH v3 2/3] dt-bindings: i2c-ast2600: Add bindings
> >> >> for AST2600 i2C driver
> >> >>
> >> >> Hi Ryan,
> >> >>
> >> >> On Mon, 16 May 2022, at 16:18, ryan_chen wrote:
> >> >> > +    i2c0: i2c-bus@80 {
> >> >> > +      #address-cells = <1>;
> >> >> > +      #size-cells = <0>;
> >> >> > +      #interrupt-cells = <1>;
> >> >> > +      compatible = "aspeed,ast2600-i2c-bus";
> >> >>
> >> >> This isn't quite right with respect to your binding description
> >> >> above
> >> >> :)
> >> > Yes, the compatible need to be " aspeed,ast2600-i2c" is that your point ?
> >>
> >> Yes, but only if we agree that we should have different compatibles
> >> for the different drivers. I'm not convinced about that yet.
> >>
> >> I think it's enough to have different Kconfig symbols, and select the
> >> old driver in aspeed_g4_defconfig, and the new driver in
> >> aspeed_g5_defconfig. Won't that gives us the right outcome without
> requiring a new set of compatibles?
> >>
> > The new driver in aspeed_g5_defconfig.
> 
> Right, behind a new Kconfig option.
> 
> > And different compatible string
> > claim will
> > Load the new or legacy driver,
> 
> I don't think we need this. It's enough to enable the new driver in the defconfig
> (and possibly disable the config option for the old driver).
> 
> > it should ok like the different
> > generation SOC. Have
> > different design.
> > Am I right?
> 
> We have SoC-specific compatibles already, so the new driver can just bind on
> the compatibles for the SoC revisions that have the new register interface. The
> old driver just binds to the device in the SoCs that have the old register
> interface.
> 
> There's an overlap in support between the two drivers, but for people who care
> about which implementation they use they can choose to exclude that driver
> from their kernel config.
> 
> None of this requires more compatibles be added.
> 
> Does that help?

The kernel device tree driver use compatibles string to separate the driver, 
like currently aspeed_g5_defconfig include the ast2500/ast2600.
So use dtsi/dts to separate which driver to loaded. Use can use dts to choice which 
driver to loaded.

So keep the dtsi/dts for user implement to loaded should be good, right?

> 
> Andrew