RE: [PATCH v16 1/3] dt-bindings: i2c: aspeed: support for AST2600-i2cv2

[Ryan Chen <ryan_chen@aspeedtech.com>]

> Subject: Re: [PATCH v16 1/3] dt-bindings: i2c: aspeed: support for
> AST2600-i2cv2
> 
> Hi Krzysztof,
> 
> > You trimmed response and brought some very old thread which does not
> > exist in my inbox.
> >
> > I have absolutely no clue what this refers to.
> 
> OK, reconstructing the relevant parts of that thread - Ryan providing
> background on the old/new register interfaces (trimmed a little for brevity;
> original context at [1] if you need):
> 
> >On 24/03/2025 09:30, Ryan Chen wrote:
> >>> Subject: Re: [PATCH v16 1/3] dt-bindings: i2c: aspeed: support for
> >>> AST2600-i2cv2
> >>>
> >>> On 19/03/2025 12:12, Ryan Chen wrote:
> >>>>> Subject: Re: [PATCH v16 1/3] dt-bindings: i2c: aspeed: support for
> >>>>> AST2600-i2cv2
> >>>>>
> >>>>> On 17/03/2025 10:21, Ryan Chen wrote:
> >>>>>>> Neither this.
> >>>>>>>
> >>>>>>> So it seems you describe already existing and documented I2C,
> >>>>>>> but for some reason you want second compatible. The problem is
> >>>>>>> that you do not provide reason from the point of view of bindings.
> >>>>>>>
> >>>>>>> To summarize: what your users want - don't care. Start properly
> >>>>>>> describing hardware and your SoC.
> >>>>>>
> >>>>>> OK, for ast2600 i2c controller have two register mode setting.
> >>>>>> One, I call it is old register setting, that is right now
> >>>>>> i2c-aspeed.c .compatible = "aspeed,ast2600-i2c-bus", And there
> >>>>>> have a global register that can set i2c controller as new mode
> >>>>>> register set. That I am going to drive. That I post is all
> >>>>>> register in new an old register list.
> >>>>>>
> >>>>>> For example,
> >>>>>> Global register [2] = 0 => i2c present as old register set Global
> >>>>>> register [2] = 1 => i2c present as new register set
> >>>>> It's the same device though, so the same compatible.
> >>>>
> >>>> Sorry, it is different design, and it share the same register
> >>>> space. So that the reason add new compatible "aspeed,ast2600-i2cv2"
> >>>> for this driver. It is different register layout.
> >>>
> >>> Which device is described by the existing "aspeed,ast2600-i2c-bus"
> >>> compatible? And which device is described by new compatible?
> >>>
> >> On the AST2600 SoC, there are up to 16 I2C controller instances (I2C1 ~
> I2C16).
> >
> > So you have 16 same devices.
> >
> >> Each of these controllers is hardwired at the SoC level to use either
> >> the legacy register layout or the new v2 register layout. The mode is
> >> selected by a bit in the global register, these represent two
> >> different hardware blocks: "aspeed,ast2600-i2c-bus" describes
> >> controllers using the legacy register layout. "aspeed,ast2600-i2cv2"
> >> describes controllers using the new register layout
> >
> > Which part of "same device" is not clear? You have one device, one
> > compatible. Whatever you do with register layout, is already defined
> > by that compatible. It does not matter that you forgot to implement it
> > in the Linux kernel.
> 
> So, I'm trying to pick up (from Ryan) on whether we're actually dealing with
> separate devices here; that was ambiguous in his responses.

Hello Jeremy, Krzysztof

Sorry, for ambiguous.
The global register like a mux selection for new/old register layout.
Like following example.

						=======================
						Driver : compatible = "aspeed,ast2600-i2c-bus"
						Old register layout : i2c0 ~ 15
						=======================
					0:/
==================
aspeed,global-regs
Global MUX (new/old)
==================
					1:\
						=======================
						Driver: compatible = "aspeed,ast2600-i2cv2-bus "
						New register layout : i2c0 ~ 15
						=======================

> 
> To me, it seems like we do have separate IP cores, just multiplexed to the same
> MMIO space. And if so, what the preference on binding implementation is,
> particularly with different SoCs having either only the "old", only the "new", or
> a switchable set of both.
> 
> Hence my query:
> 
> > Given there are actual behavioural differences between the two
> > peripherals - beyond just the register set - that would seem to
> > indicate separate binding types (+ a syscon mux control) to me, but
> > I'm keen to hear any other options.
> >
> > Krzysztof, if that is the case, any thoughts on the representation of
> > separate bindings?
> 
> - given we may not be dealing with "the same device" in actual hardware, in
> reference to Ryan's proposed compatible split between the two.
> 
> Cheers,
> 
> 
> Jeremy
> 
> [1]:
> https://lore.kernel.org/all/20250224055936.1804279-2-ryan_chen@aspeedtec
> h.com/t/#u