Re: [PATCH v3 1/2] dt-bindings: soc: ti: pruss: Add clocks for ICSSG

[Vignesh Raghavendra <vigneshr@ti.com>]

On 18/11/24 19:22, Roger Quadros wrote:
> 
> 
> On 18/11/2024 15:33, Roger Quadros wrote:
>> Hi,
>>
>> On 13/11/2024 13:09, MD Danish Anwar wrote:
>>> The ICSSG module has 7 clocks for each instance.
>>>
>>> These clocks are ICSSG0_CORE_CLK, ICSSG0_IEP_CLK, ICSSG0_ICLK,
>>> ICSSG0_UART_CLK, RGMII_MHZ_250_CLK, RGMII_MHZ_50_CLK and RGMII_MHZ_5_CLK
>>> These clocks are described in AM64x TRM Section 6.4.3 Table 6-398.
>>>
>>> Add these clocks to the dt binding of ICSSG.
>>>
>>> Link: https://www.ti.com/lit/pdf/spruim2 (AM64x TRM)
>>> Signed-off-by: MD Danish Anwar <danishanwar@ti.com>
>>> ---
>>>  Documentation/devicetree/bindings/soc/ti/ti,pruss.yaml | 10 ++++++++++
>>>  1 file changed, 10 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/soc/ti/ti,pruss.yaml b/Documentation/devicetree/bindings/soc/ti/ti,pruss.yaml
>>> index 3cb1471cc6b6..927b3200e29e 100644
>>> --- a/Documentation/devicetree/bindings/soc/ti/ti,pruss.yaml
>>> +++ b/Documentation/devicetree/bindings/soc/ti/ti,pruss.yaml
>>> @@ -92,6 +92,16 @@ properties:
>>>      description: |
>>>        This property is as per sci-pm-domain.txt.
>>>  
>>> +  clocks:
>>> +    items:
>>> +      - description: ICSSG_CORE Clock
>>> +      - description: ICSSG_IEP Clock
>>> +      - description: ICSSG_RGMII_MHZ_250 Clock
>>> +      - description: ICSSG_RGMII_MHZ_50 Clock
>>> +      - description: ICSSG_RGMII_MHZ_5 Clock
>>> +      - description: ICSSG_UART Clock
>>> +      - description: ICSSG_ICLK Clock
>>> +
>>
>> There are actually many more clocks [1]
>> What is the purpose of adding all these clocks in the DT if driver doesn't
>> use them?
>>

DT should completely describe the HW and not based on what Linux driver
needs. So its valid to describe all clock inputs to a module
irrespective of what driver does with it.

>> Only CORE and IEP clocks parent can be configured via clock muxes.
>> Those are already defined in the icssg?_cfg nodes.
> 
> Actually those clock muxes are internal to ICSSG.
> We still need to be able to set clock parents of CORE and IEP clock.
> 
> So pruss block needs at most 2 clocks like you had in v2 of this patch?
> 
>>
>> [1] - https://software-dl.ti.com/tisci/esd/22_01_02/5_soc_doc/am64x/clocks.html
>>
>>>  patternProperties:
>>>  
>>>    memories@[a-f0-9]+$:
>>
> 

-- 
Regards
Vignesh
https://ti.com/opensource