linux-kernel - Re: [PATCH 2/2] clocksource/drivers/nxp-timer: Add the System Timer Module for the s32g platform

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: <e8054f85-2a23-4ceb-8e1c-7cdfe7c87628@linaro.org>
Date: Tue, 25 Mar 2025 14:54:26 +0100
From: Daniel Lezcano <daniel.lezcano@...aro.org>
To: Ghennadi Procopciuc <ghennadi.procopciuc@....nxp.com>, tglx@...utronix.de
Cc: linux-kernel@...r.kernel.org, Thomas Fossati <thomas.fossati@...aro.org>,
 Larisa Grigore <Larisa.Grigore@....com>,
 Ghennadi Procopciuc <ghennadi.procopciuc@....com>,
 Maxime Coquelin <mcoquelin.stm32@...il.com>,
 Alexandre Torgue <alexandre.torgue@...s.st.com>,
 "moderated list:ARM/STM32 ARCHITECTURE"
 <linux-stm32@...md-mailman.stormreply.com>,
 "moderated list:ARM/STM32 ARCHITECTURE"
 <linux-arm-kernel@...ts.infradead.org>, NXP S32 Linux Team <s32@....com>
Subject: Re: [PATCH 2/2] clocksource/drivers/nxp-timer: Add the System Timer
 Module for the s32g platform

On 25/03/2025 14:21, Ghennadi Procopciuc wrote:
> On 3/25/2025 2:51 PM, Daniel Lezcano wrote:
> [ ... ]
>>>>>>>> +static int __init nxp_stm_timer_probe(struct platform_device *pdev)
>>>>>>>> +{
>>>>>>>> +    struct device *dev = &pdev->dev;
>>>>>>>> +    struct device_node *np = dev->of_node;
>>>>>>>> +    struct stm_instances *stm_instances;
>>>>>>>> +    const char *name = of_node_full_name(np);
>>>>>>>> +    void __iomem *base;
>>>>>>>> +    int irq, ret;
>>>>>>>> +    struct clk *clk;
>>>>>>>> +
>>>>>>>> +    stm_instances =
>>>>>>>> (typeof(stm_instances))of_device_get_match_data(dev);
>>>>>>>> +    if (!stm_instances) {
>>>>>>>> +        dev_err(dev, "No STM instances associated with a cpu");
>>>>>>>> +        return -EINVAL;
>>>>>>>> +    }
>>>>>>>> +
>>>>>>>> +    base = devm_of_iomap(dev, np, 0, NULL);
>>>>>>>> +    if (IS_ERR(base)) {
>>>>>>>> +        dev_err(dev, "Failed to iomap %pOFn\n", np);
>>>>>>>> +        return PTR_ERR(base);
>>>>>>>> +    }
>>>>>>>> +
>>>>>>>> +    irq = irq_of_parse_and_map(np, 0);
>>>>>>>> +    if (irq <= 0) {
>>>>>>>> +        dev_err(dev, "Failed to parse and map IRQ: %d\n", irq);
>>>>>>>> +        return -EINVAL;
>>>>>>>> +    }
>>>>>>>
>>>>>>>     From commit description:
>>>>>>>
>>>>>>>> The first probed STM is used as a clocksource, the second will be
>>>>>>>> the
>>>>>>>> broadcast timer and the rest are used as a clockevent with the
>>>>>>>> affinity set to a CPU.
>>>>>>>
>>>>>>> Why is the interrupt mandatory when the node is probed as a
>>>>>>> clocksource?
>>>>>>
>>>>>> It relies on the DT description and it does not hurt to have a
>>>>>> consistent code path for clockevent / clocksource even if the
>>>>>> interrupt
>>>>>> is not requested for the clocksource later.
>>>>>>
>>>>>
>>>>> If so, in my opinion, it would make sense to use the same STM instance
>>>>> for both the clocksource and broadcast clockevent, as both functions
>>>>> can
>>>>> be accommodated within a single STM instance, which will help reduce
>>>>> the
>>>>> number of STM instances used.
>>>>
>>>> The broadcast timer is stopped when it is unused which is the case for
>>>> the s32 because there are no idle state powering down the local timers.
>>>> They have to have be separated.
>>>>
>>>
>>> This wouldn't be the case if the STM is kept running/counting during the
>>> clock event setup, with only the clock event interrupt being disabled
>>> (CCR.CEN).
>>
>> Are you asking to use two different channels for the same STM instance,
>> one for the clocksource and one for the clockevent ?
>>
> 
> I suggested using the CNT register to obtain the count for the clock
> source, while using one of the STM channels for the clock event.

Ah, ok.

I think it is preferable to keep them separated to keep the code 
modular. Given the number of STM on the platform, it does not hurt

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