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

Message-ID: <a6440cbf-f7b5-4bce-8e2b-8aa3ec4d2342@kernel.org>
Date: Wed, 25 Jun 2025 10:48:37 +0200
From: Krzysztof Kozlowski <krzk@...nel.org>
To: Clement LE GOFFIC <clement.legoffic@...s.st.com>,
 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 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.

Best regards,
Krzysztof