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

[Andrew Davis]

On 8/9/23 4:31 PM, Conor Dooley wrote:
> 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?

I was not exactly aware of the extent of the incompatibilities.
If they are not compatible, then there is really no decision needed,
they must have different strings.

> > 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@xxxxxx>
> > > > > ---
> > > > >    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?

Sounds like it, I'll go and make this soc-specific.

Thanks,
Andrew

> Apologies if I have misunderstood.
>
> Thanks,
> Conor.
>
> > > > >                - const: syscon
> > > > >