Re: [PATCH v2 1/2] dt-bindings: watchdog: Add NXP Software Watchdog Timer

[Daniel Lezcano <daniel.lezcano@linaro.org>]

On 04/04/2025 08:35, Ghennadi Procopciuc wrote:
> On 4/3/2025 6:10 PM, Daniel Lezcano wrote:
>> On 03/04/2025 08:19, Ghennadi Procopciuc wrote:
>>> On 4/2/2025 6:49 PM, Daniel Lezcano wrote:
>>> [ ... ]
>>>> +examples:
>>>> +  - |
>>>> +    watchdog@0x40100000 {
>>>> +        compatible = "nxp,s32g2-swt";
>>>> +        reg = <0x40100000 0x1000>;
>>>> +        clocks = <&clks 0x3a>;
>>>> +        timeout-sec = <10>;
>>>> +    };
>>>
>>> The S32G reference manual specifies two clocks for the SWT module: one
>>> for the registers and another for the counter itself. Shouldn't both
>>> clocks be represented in the bindings?
>>
>> AFAICS, there are two clocks as described in the documentation for the
>> s32g2 page 846, section 23.7.3.3 SWT clocking.
> 
> This diagram illustrates the module clocks and their connections to the
> S32GS system clocks. From the module's perspective, there are three
> clocks: MODULE_CLOCK, REG_INTF, and COUNTER_CLOCK. Specifically, on
> S32G2 SoCs, the first two are connected to XBAR_DIV3_CLK, while the
> counter clock is linked to FIRC_CLK. Based on my understanding of the
> device tree, this configuration should be listed as follows:
> 
> clocks = <&clks XBAR_DIV3_CLK>, <&clks XBAR_DIV3_CLK>, <&clks FIRC_CLK>;
> clock-names = "module", "reg", "counter";
> 
> Configuring it this way allows flexibility to reuse the same clocking
> scheme for other SoCs where the integration is performed differently. It
> is possible that the 'module' and 'reg' clocks could be linked to two
> distinct system clocks.

That is something we can handle when the other SoC will be in the 
process of being upstream, no ?

I don't see how that can help with the current hardware we are 
describing. What is the benefit ?

I would prefer to stick to what is needed today

>> The module and the register clock are fed by the XBAR_DIV3_CLK which is
>> an system clock always-on.
> 
> XBAR_DIV3_CLK is not an always-on clock, meaning it is not available
> during suspend, if that is what you mean by always-on. The SIRC can be
> considered the only always-on clock on this device.
> 
>>
>> The counter is fed by the FIRC_CLK which described as "FIRC_CLK is the
>> default clock for the entire system at power-up."
>>
>>  From my understanding, we should not describe the XBAR_DIV3_CLK as it is
>> a system clock.
>>
>> And the FIRC_CLK is only there to get the clock rate in the driver.
>>
> 


-- 
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog