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

Message ID 20230809165752.46133-1-afd@ti.com
State New
Headers
Series [1/2] dt-bindings: mfd: syscon: Add ti,dss-oldi-io-ctrl compatible |

Commit Message

Andrew Davis Aug. 9, 2023, 4:57 p.m. UTC
  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.

[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(+)
  

Comments

Aradhya Bhatia Aug. 9, 2023, 5:39 p.m. UTC | #1
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?

Regards
Aradhya

> 
> [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
>  
>            - const: syscon
>
  
Andrew Davis Aug. 9, 2023, 5:59 p.m. UTC | #2
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.

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..

Andrew

> Regards
> Aradhya
> 
>>
>> [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
>>   
>>             - const: syscon
>>
  
Aradhya Bhatia Aug. 9, 2023, 7 p.m. UTC | #3
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.

It is certainly not compatible. More on this below.

> 
> 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..

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.

Regards
Aradhya


> 
> Andrew
> 
>> Regards
>> Aradhya
>>
>>>
>>> [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
>>>               - const: syscon
>>>
  
Conor Dooley Aug. 9, 2023, 9:31 p.m. UTC | #4
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
> >>>   
>
  
Andrew Davis Aug. 9, 2023, 9:38 p.m. UTC | #5
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@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?
> 

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

Thanks,
Andrew

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

Patch

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
 
           - const: syscon