Re: [PATCH 1/2] dt-bindings: mfd: syscon: Add ti,dss-oldi-io-ctrl compatible

[Conor Dooley <conor@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 3381 bytes --]

On Thu, Aug 10, 2023 at 12:30:03AM +0530, Aradhya Bhatia wrote:
> 
> 
> On 09-Aug-23 23:29, Andrew Davis wrote:
> > On 8/9/23 12:39 PM, Aradhya Bhatia wrote:
> >> Hi Andrew,
> >>
> >> Thank you for quickly whipping up these patches! =)
> >>
> >> On 09-Aug-23 22:27, Andrew Davis wrote:
> >>> Add TI DSS OLDI-IO control registers compatible. This is a region of 5
> >>> 32bit registers found in the TI AM65 CTRL_MMR0 register space[0]. They
> >>> are used to control the characteristics of the OLDI DATA/CLK IO as
> >>> needed
> >>> by the DSS display controller node.
> >>
> >> As long as the driver takes care of it, we can reuse the same compatible
> >> even when OLDI IO Ctrl registers change from SoC to SoC, (in this case,
> >> AM65 to AM62), right?
> >>
> > 
> > That depends, is the register space still "compatible" with the AM65
> > version of this space? If not then we would want to qualify these
> > with their SoC versions.

Even if they are compatible, having soc-specific compatibles with a
fallback to the common oldi compatible string would be ideal.

> It is certainly not compatible. More on this below.

But as they're not compatible, that's kinda moot anyway.

> > A quick check of the documentation shows the register space is still
> > 5 registers, 4 DATA and 1 CLK. The contents are different though, but
> > since this compatible string is not used to match with a driver that
> > would care (that is handled by the DSS node which does have different
> > compatibles for each device), I'm actually not sure.

> > Guess we can leave
> > that decision to the DT binding maintainers..

I'm not 100% sure what this decision actually is. Could you elaborate?


> Exactly. The DSS driver in our, as you like to call, evil-vendor-tree
> uses the compatible information to decide which register offsets to
> write to, and what to write.
> 
> 
> On the register compatibility situation...
> 
> AM62 OLDI IO Ctrl has, in total, 12 registers (as opposed to 5 in AM65).
> 
> There are 4 Data and 1 Clk registers (per OLDI). And there are 2 OLDI
> transmitters in AM62. However, their contents are different as you
> noted.
> 
> There are 2 more registers in AM62 (unlike AM65), that control
> power-down and loop-back. And it is the power-down register, that will
> be primarily used by the driver.

> >>> [0] https://www.ti.com/lit/pdf/spruid7
> >>>
> >>> Signed-off-by: Andrew Davis <afd@ti.com>
> >>> ---
> >>>   Documentation/devicetree/bindings/mfd/syscon.yaml | 1 +
> >>>   1 file changed, 1 insertion(+)
> >>>
> >>> diff --git a/Documentation/devicetree/bindings/mfd/syscon.yaml
> >>> b/Documentation/devicetree/bindings/mfd/syscon.yaml
> >>> index 8103154bbb529..5029abd6d6411 100644
> >>> --- a/Documentation/devicetree/bindings/mfd/syscon.yaml
> >>> +++ b/Documentation/devicetree/bindings/mfd/syscon.yaml
> >>> @@ -69,6 +69,7 @@ properties:
> >>>                 - rockchip,rk3588-qos
> >>>                 - rockchip,rv1126-qos
> >>>                 - starfive,jh7100-sysmain
> >>> +              - ti,dss-oldi-io-ctrl

So it sounds like this compatible, that appears to be generic, should
instead be soc-specific as the register layout is different between
SoCs?

Apologies if I have misunderstood.

Thanks,
Conor.

> >>>               - const: syscon
> >>>   
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]