linux-kernel - Re: [PATCH 06/13] perf: stm32: introduce DDRPERFM driver

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] [day] [month] [year] [list]

Message-ID: <dabecbc6-967e-4912-8297-4d53fd2d9cc1@foss.st.com>
Date: Wed, 25 Jun 2025 11:09:05 +0200
From: Clement LE GOFFIC <clement.legoffic@...s.st.com>
To: Krzysztof Kozlowski <krzk@...nel.org>, Will Deacon <will@...nel.org>,
        Mark
 Rutland <mark.rutland@....com>, Rob Herring <robh@...nel.org>,
        Krzysztof
 Kozlowski <krzk+dt@...nel.org>,
        Conor Dooley <conor+dt@...nel.org>,
        Maxime
 Coquelin <mcoquelin.stm32@...il.com>,
        Alexandre Torgue
	<alexandre.torgue@...s.st.com>,
        Philipp Zabel <p.zabel@...gutronix.de>,
        Jonathan Corbet <corbet@....net>,
        Gatien Chevallier
	<gatien.chevallier@...s.st.com>,
        Michael Turquette <mturquette@...libre.com>,
        Stephen Boyd <sboyd@...nel.org>,
        Gabriel Fernandez
	<gabriel.fernandez@...s.st.com>
CC: <linux-arm-kernel@...ts.infradead.org>, <linux-perf-users@...r.kernel.org>,
        <devicetree@...r.kernel.org>,
        <linux-stm32@...md-mailman.stormreply.com>,
        <linux-kernel@...r.kernel.org>, <linux-doc@...r.kernel.org>,
        <linux-clk@...r.kernel.org>
Subject: Re: [PATCH 06/13] perf: stm32: introduce DDRPERFM driver

On 6/25/25 10:48, Krzysztof Kozlowski wrote:
> On 25/06/2025 10:33, Clement LE GOFFIC wrote:
>> On 6/25/25 08:35, Krzysztof Kozlowski wrote:
>>> On 24/06/2025 12:43, Clement LE GOFFIC wrote:
>>>> On 6/23/25 11:45, Krzysztof Kozlowski wrote:
>>>> [...]
>>>>
>>>> Hi Krzysztof,
>>>>
>>>> Sorry I forgot to address comments below.
>>>>
>>>>>> +
>>>>>> +static const struct stm32_ddr_pmu_cfg stm32_ddr_pmu_cfg_mp1 = {
>>>>>> +	.regs = &stm32_ddr_pmu_regspec_mp1,
>>>>>> +	.attribute = stm32_ddr_pmu_attr_groups_mp1,
>>>>>> +	.counters_nb = MP1_CNT_NB,
>>>>>> +	.evt_counters_nb = MP1_CNT_NB - 1, /* Time counter is not an event counter */
>>>>>> +	.time_cnt_idx = MP1_TIME_CNT_IDX,
>>>>>> +	.get_counter = stm32_ddr_pmu_get_event_counter_mp1,
>>>>>> +};
>>>>>> +
>>>>>> +static const struct stm32_ddr_pmu_cfg stm32_ddr_pmu_cfg_mp2 = {
>>>>>> +	.regs = &stm32_ddr_pmu_regspec_mp2,
>>>>>> +	.attribute = stm32_ddr_pmu_attr_groups_mp2,
>>>>>> +	.counters_nb = MP2_CNT_NB,
>>>>>> +	.evt_counters_nb = MP2_CNT_NB - 1, /* Time counter is an event counter */
>>>>>> +	.time_cnt_idx = MP2_TIME_CNT_IDX,
>>>>>> +	.get_counter = stm32_ddr_pmu_get_event_counter_mp2,
>>>>>> +};
>>>>>> +
>>>>>> +static const struct dev_pm_ops stm32_ddr_pmu_pm_ops = {
>>>>>> +	SET_SYSTEM_SLEEP_PM_OPS(NULL, stm32_ddr_pmu_device_resume)
>>>>>> +};
>>>>>> +
>>>>>> +static const struct of_device_id stm32_ddr_pmu_of_match[] = {
>>>>>> +	{
>>>>>> +		.compatible = "st,stm32mp131-ddr-pmu",
>>>>>> +		.data = &stm32_ddr_pmu_cfg_mp1
>>>>>> +	},
>>>>>> +	{
>>>>>> +		.compatible = "st,stm32mp151-ddr-pmu",
>>>>>> +		.data = &stm32_ddr_pmu_cfg_mp1
>>>>>
>>>>> So devices are compatible, thus express it correctly and drop this.
>>>>
>>>> Ok so I assume this comes with your comment in the bindings and
>>>> basically don't get you point here.
>>>> Can you please be more precise ?
>>>
>>> Express compatibility in the bindings, like 90% of SoCs are doing, so
>>> with proper fallback and drop this entry in the table. My comment was
>>> pretty precise, because this is completely standard pattern, also used
>>> already in stm32.
>>>
>>
>> Ok I remember your discussion with Alex in my V1 of pinctrl-hdp :
>> https://lore.kernel.org/all/1de58672-5355-4b75-99f4-c48687017d2f@kernel.org/
>>
>> Does it suits you :
>> In the SoC DT:
>> MP13: compatible = "st,stm32mp131-ddr-pmu", "st,stm32mp1-ddr-pmu";
>> MP15: compatible = "st,stm32mp151-ddr-pmu", "st,stm32mp1-ddr-pmu";
> 
> No, because I did not say to change other entry in the table. Please
> read again what I asked: drop this. "This" means ONLY this entry. "Drop
> this" does not mean "change something else". Do not change other entries
> by introducing some generic compatible. That's not the pattern ever
> endorsed by DT maintainers. Add front compatible and you are done,
> smallest amount of changes, most obvious code.
>

Ok so in the SoC DT I'll keep:
MP13: compatible = "st,stm32mp131-ddr-pmu";
MP15: compatible = "st,stm32mp151-ddr-pmu", "st,stm32mp131-ddr-pmu";

Thanks for clarifying.

Best regards,
Clément

> Best regards,
> Krzysztof