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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <e633232e-f0ad-43f7-9897-39836e6b98b4@kernel.org>
Date: Tue, 19 Nov 2024 12:45:54 +0200
From: Roger Quadros <rogerq@...nel.org>
To: Vignesh Raghavendra <vigneshr@...com>,
 MD Danish Anwar <danishanwar@...com>, conor+dt@...nel.org,
 krzk+dt@...nel.org, robh@...nel.org, ssantosh@...nel.org, nm@...com
Cc: devicetree@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
 linux-kernel@...r.kernel.org, s-anna@...com, kristo@...nel.org, srk@...com
Subject: Re: [PATCH v3 1/2] dt-bindings: soc: ti: pruss: Add clocks for ICSSG



On 19/11/2024 08:12, Vignesh Raghavendra wrote:
> 
> 
> 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@...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.

Fair point. But there are a total 11 clocks instead of 7 in [1]

> 
>>> 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]+$:
>>>
>>
> 

-- 
cheers,
-roger