linux-kernel - Re: [PATCH v3 09/27] dt-bindings: clock: mediatek: Describe MT8196 clock controllers

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: <4330785c-6564-4c66-b298-75655106e99f@collabora.com>
Date: Mon, 4 Aug 2025 16:35:57 +0200
From: AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>
To: Krzysztof Kozlowski <krzk@...nel.org>, Laura Nao
 <laura.nao@...labora.com>, wenst@...omium.org
Cc: conor+dt@...nel.org, devicetree@...r.kernel.org,
 guangjie.song@...iatek.com, kernel@...labora.com, krzk+dt@...nel.org,
 linux-arm-kernel@...ts.infradead.org, linux-clk@...r.kernel.org,
 linux-kernel@...r.kernel.org, linux-mediatek@...ts.infradead.org,
 matthias.bgg@...il.com, mturquette@...libre.com, netdev@...r.kernel.org,
 nfraprado@...labora.com, p.zabel@...gutronix.de, richardcochran@...il.com,
 robh@...nel.org, sboyd@...nel.org
Subject: Re: [PATCH v3 09/27] dt-bindings: clock: mediatek: Describe MT8196
 clock controllers

Il 04/08/25 16:33, Krzysztof Kozlowski ha scritto:
> On 04/08/2025 16:31, AngeloGioacchino Del Regno wrote:
>> Il 04/08/25 16:19, Krzysztof Kozlowski ha scritto:
>>> On 04/08/2025 15:58, Krzysztof Kozlowski wrote:
>>>>>
>>>>> So, what should we do then?
>>>>>
>>>>> Change it to "mediatek,clock-hw-refcounter", and adding a comment to the binding
>>>>> saying that this is called "Hardware Voter (HWV)" in the datasheets?
>>>>>
>>>>> Or is using the "interconnect" property without any driver in the interconnect API
>>>>> actually legit? - Because to me it doesn't look like being legit (and if it is, it
>>>>> shouldn't be, as I'm sure that everyone would expect an interconnect API driver
>>>>> when encountering an "interconnect" property in DT), and if so, we should just add
>>>>
>>>> Why you would not add any interconnect driver for interconnect API?
>>>> Look, the current phandle allows you to poke in some other MMIO space
>>>> for the purpose of enabling the clock FOO? So interconnect or power
>>>> domains or whatever allows you to have existing or new driver to receive
>>>> xlate() and, when requested resources associated with clock FOO.
>>>
>>> Something got here cut. Last sentence is supposed to be:
>>>
>>> "So interconnect or power
>>> domains or whatever allows you to have existing or new driver to receive
>>> xlate() and, when requested, toggle the resources associated with clock
>>> FOO."
>>>
>>>>
>>>> Instead of the FOO clock driver poking resources, you do
>>>> clk_prepare_enable() or pm_domain or icc_enable().
>>>
>>> I looked now at the driver and see your clock drivers poking via regmap
>>> to other MMIO. That's exactly usecase of syscon and exactly the pattern
>>> *we are usually discouraging*. It's limited, non-scalable and vendor-driven.
>>>
>>
>> If the HWV wasn't BROKEN, I'd be the first one to go for generic stuff, but
>> since it is what it is, adding bloat to generic, non vendor-driven APIs would
>> be bad.
>>
>>> If this was a power domain provider then:
>>> 1. Your clock drivers would only do runtime PM.
>>
>> The clock drivers would have to get a list of power domain that is *equal to*
>> (in their amount) the list of clocks.
>> But then those are not power domains, as those registers in the MCU are ONLY
>> ungating a clock and nothing else in the current state of the hardware.
>>
>>> 2. Your MCU would be the power domain controller doing whatever is
>>> necessary - toggling these set/clr bits - when given clock is enabled.
>>
>> That MCU does support power domain voting (for two power domains in the main
>> PD Controller, and for all power domains in the multimedia PD controller), and
>> this is something completely separated from the *clock* controllers.
>>
>> Just to make the picture complete for you: the power domains that this MCU can
>> manage are not in any way related to the clocks that it can manage. At all.
> 
> 
> OK, thanks for explanations. Please rephrase commit msg and property
> description in v4. I am fine in using "hardware voter" terminology in
> some places, so it will match datasheet, but I want to make it clear
> that it is not voting for resources how we usually understand it. It's
> just syscon stuff, poking in other system-like device registers because
> hardware is like that.
> 

I'm happy that we finally reached a conclusion that works for both of us, and
I am sorry that all this went on for weeks with (very) long discussions.

Thanks for that.

Regards,
Angelo

> 
> Best regards,
> Krzysztof