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

[Krzysztof Kozlowski <krzk@kernel.org>]

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.

Best regards,
Krzysztof