Message-ID: <3a499702-ba75-4d8a-b38d-222a62bffb34@collabora.com>
Date: Mon, 4 Aug 2025 16:31:05 +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: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.

> And it really looks like what you described...
> 
> 
> Best regards,
> Krzysztof

Powered by blists - more mailing lists