[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <62edb8e3-aff6-4225-b520-f4b73aef145d@collabora.com>
Date: Mon, 4 Aug 2025 16:15:49 +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 15:58, Krzysztof Kozlowski ha scritto:
> 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?
"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.
>>
>> 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 :-)
>>
>> 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...
>
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
>>
>> 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