Message-ID: <2555e9fe-3bc0-4f89-9d0b-2f7f946632e7@kernel.org>
Date: Mon, 4 Aug 2025 15:58:50 +0200
From: Krzysztof Kozlowski <krzk@...nel.org>
To: AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>,
 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

On 04/08/2025 15:27, AngeloGioacchino Del Regno wrote:
> 
> We discussed about aggregating votes, yes, in software - this instead is a
> *broken* hardware that does the aggregation internally and does not require
> nor want external drivers to do the aggregation.
> 
>> Maybe it is just the name, so avoid all the confusing "votes" if this is
>> not voting system. If this is a voting system, then don't use custom
>> phandles.
> 
> Being it fundamentally *broken*, this being a voting system is what the hardware
> initially wanted to be - but effectively, since it requires YOU to:
>   - Make sure that power supplies are turned on, if not, turn them on by "touching"
>     HW registers (so, without any assistance from the voter MCU), if any;
>   - Turn on parent clocks manually, if any, before using the "voter mcu" to try
>     to ungate that clock; and
>     - Enable the "FENC" manually, after the mcu says that the clock was ungated.


I understand that "YOU" as Linux driver, when you want to do something
(e.g. toggle) a clock?
If so this looks a lot like power domain, although with some differences.

> 
> in the current state, it is just an hardware managed refcounting system and
> nothing else, because the MCU seems to be unfinished, hence, again, b r o k e n.
> 
> Note that by "manually" I always mean "with direct writes to a clock controller's
> registerS, and without any automation/assistance from the HWV MCU".
> 
> We're using the "hardware-voter" name because this is how MediaTek calls it in the
> datasheets, and no it doesn't really *deserve* that name for what it is exactly in
> MT8196 and MT6991.

Please capture most/all of this in the property description, so it will
be clear that we treat it as some sort of exception and other users of
that property would need similar rationale.

I am asking for this because I do not want this to be re-used for any
other work which would represent something like real voting for
resources. I want it to be clear for whoever looks at it later during
new SoC bringup.

If you send the same code as v4, the same commit msg, just like Laura
did twice in v2 and v3, I will just keep NAKing via mutt macro because
it's a waste of my time.

> 
> And mind you - if using the "interconnect" property for this means that we have to
> add an interconnect driver for it, no, we will not do that, as placing a software

Existing driver(s) can be as well interconnect providers. Same with
power domains.

I do not talk here how you should implement this in the drivers.

> vote that votes clocks in a a voter MCU that does exactly what the interconnect

What is a "software vote"? How did you encode it in DT? Via that phandle?

> driver would do - then requiring virtual/fake clocks - is not a good solution.

We do not add "software votes" in DT as separate properties, because
they are "software". So maybe that's another problem here...

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

Instead of the FOO clock driver poking resources, you do
clk_prepare_enable() or pm_domain or icc_enable().



Best regards,
Krzysztof

Powered by blists - more mailing lists