Il 04/08/25 15:58, Krzysztof Kozlowski ha scritto:

On 04/08/2025 15:27, AngeloGioacchino Del Regno wrote:

quoted

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.

quoted

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?

"you" == Linux driver, yes.

If so this looks a lot like power domain, although with some differences.

A power domain ungates power to something.

These are clocks, giving a (x) (M)Hz signal to something.

quoted

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.

Okay, now that sounds reasonable, and that sounds like a clear suggestion with
a clear action to take.

Perfect.

Laura, please do exactly that.

P.S.: I understand what you're trying to do here, and I agree; preventing stuff
like this for things that aren't as broken as this is completely right.

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.

My time isn't infinite, either :-)

quoted

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.

quoted

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?

quoted

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

Indeed - the point is, the only way to make this *broken* thing to work with an
interconnect provider would be to place a software vote to place a vote in the HW
voter, which would be ugly and wrong.

But anyway, a solution was reached. Let's just stop and avoid useless discussions
about what X could be if hardware Y wasn't broken; that'd be just a waste of time.

Regards,
Angelo

quoted

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