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] [thread-next>] [day] [month] [year] [list]
Message-ID: <d41a6c60-5368-4bcd-b028-2477e42e29bb@collabora.com>
Date: Thu, 3 Jul 2025 10:56:11 +0200
From: AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>
To: Krzysztof Kozlowski <krzk@...nel.org>
Cc: robh@...nel.org, conor+dt@...nel.org, matthias.bgg@...il.com,
 ulf.hansson@...aro.org, arnd@...db.de, m.wilczynski@...sung.com, nm@...com,
 khilman@...libre.com, kabel@...nel.org, quic_hyiwei@...cinc.com,
 pjp@...oraproject.org, tudor.ambarus@...aro.org, drew@...7.com,
 u.kleine-koenig@...libre.com, gregkh@...uxfoundation.org,
 devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
 linux-arm-kernel@...ts.infradead.org, linux-mediatek@...ts.infradead.org,
 laura.nao@...labora.com, kernel@...labora.com
Subject: Re: [RFC PATCH 2/3] dt-bindings: firmware: Document the MediaTek
 Hardware Voter (HWV)

Il 02/07/25 08:50, Krzysztof Kozlowski ha scritto:
> On Tue, Jul 01, 2025 at 05:11:48PM +0200, AngeloGioacchino Del Regno wrote:
>> Add documentation for the new MediaTek Hardware Voter, found in
>> MediaTek SoCs like the MT8196 Kompanio Ultra for Chromebooks and
>> the MT6991 Dimensity 9400 for Smartphones.
>>
>> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>
>> ---
>>   .../mediatek,mt6991-hardware-voter.yaml       | 70 +++++++++++++++++++
>>   1 file changed, 70 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/firmware/mediatek,mt6991-hardware-voter.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/firmware/mediatek,mt6991-hardware-voter.yaml b/Documentation/devicetree/bindings/firmware/mediatek,mt6991-hardware-voter.yaml
>> new file mode 100644
>> index 000000000000..173b74c23a91
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/firmware/mediatek,mt6991-hardware-voter.yaml
>> @@ -0,0 +1,70 @@
>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>> +# Copyright 2025 Collabora Ltd
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/firmware/mediatek,mt6991-hardware-voter.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: MediaTek Hardware Voter (HWV)
>> +
>> +maintainers:
>> +  - AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>
>> +
>> +description:
>> +  The MediaTek Hardware Voter (HWV) is a SoC-internal fixed-function MCU
>> +  used to collect votes from both the Application Processor and from the
>> +  various other remote processors present in the SoC, and transparently
>> +  turn on or off various hardware resources (for example, power domains
>> +  or system clocks) based on aggregation of votes done in the HWV MCU's
>> +  internal state machine, therefore guaranteeing synchronization of the
>> +  hardware resource requests between all components of the SoC and hence
>> +  avoiding, for example, unclocked or unpowered access to the hardware.
>> +
>> +properties:
>> +  $nodename:
>> +    pattern: "^system-controller@[0-9a-f]+$"
>> +
>> +  compatible:
>> +    const: mediatek,mt6991-hardware-voter
>> +
>> +  reg:
>> +    items:
>> +      - description: Address and size of the Hardware Voter MMIO
>> +
> 
> No resources here, so this should go to power controller
> 
>> +  power-controller:
>> +    $ref: /schemas/power/mediatek,power-controller.yaml
>> +
>> +required:
>> +  - compatible
>> +  - reg
>> +
>> +additionalProperties: true
>> +
>> +examples:
>> + - |
>> +   scp_hwv: system-controller@...00000 {
>> +     compatible = "mediatek,mt6991-hardware-voter";
>> +     reg = <0 0x14500000 0 0x3000>;
>> +
>> +     power-controller {
>> +       compatible = "mediatek,mt8196-hwv-scp-power-controller";
> 
> mt8196 in mt6991 is very confusing.
> 

Yeah that wasn't intentional; fyi, it's almost the same soc, that's why I mixed
them up... :-)

> Anyway, this does not address my comment at all. You again create some
> sort of syscon for voting, so no. You are supposed to use generic API
> for voting: clocks, power domains, interconnects - whatever is there
> applicable or necessary.
> 

Making that loud and clear: Interconnect is not applicable.

The only way to do what you're proposing would be to add a bunch of `reg`
to each devicetree node for each clock controller and each power controller;
I can do that, but looks a bit dirty - and still yet another syscon-like
alternative, but without having a real syscon declared in there.

Mind you - both clock and power controllers are writing both to their own
register space (and enabling external regulators, etc, for power domains)
and to the hardware voter MMIO (which means that the HWV, in hardware, is
fundamentally broken).

After this reply, the only option that is left to me is the following:

		topckgen: clock-controller@...00000 {
			compatible = "mediatek,mt8196-topckgen", "syscon";
			reg = <0 0x10000000 0 0x800>, <0 0x14500010 0 0x48>,
			      <0 0x14502c08 0 0x24>;
			reg-names = "base", "hwvoter-base", "hwvoter-status";
			#clock-cells = <1>;
		};

		imp_iic_wrap_north: clock-controller@...30000 {
			compatible = "mediatek,mt8196-imp-iic-wrap-n", "syscon";
			reg = <0 0x13c30000 0 0x1000>, <0 0x14500000 0 0xc>,
			      <0 0x14502c00 0 0xc>;
			reg-names = "base", "hwvoter-base", "hwvoter-status";
			#clock-cells = <1>;
		};

		/* Power Manager with Hardware Voter */
		spm_hwv: power-controller@...00218 {
			compatible = "mediatek,mt8196-hwv-scp-power-controller";
			reg = <0 0x14500218 0 0x20>, <0 0x14501410 0 0x20>,
			      <0 0x14505514 0 0xc>;
			reg-names = "hwvoter-base", "hwvoter-status", "hwvoter-ack";
			#address-cells = <1>;
			#size-cells = <0>;
			#power-domain-cells = <1>;

			/* SCPSYS hardware voter power domains */
			mm_proc_dormant: power-domain@...196_POWER_DOMAIN_MM_PROC_DORMANT {
				..... etc, all power domains

At this point, I'm really not sure that this would be better than just passing
the mediatek,hardware-voter syscon to the clock controllers - as what I've done
previously was effectively representing the hardware in the devicetree as it is,
matching the real HW layout 1:1 (because again, each of the whole HWV MCU(s) are
embedded into each of the two power controllers, one for System power, and one
for Multimedia power).

(btw, hardware speaking, the power controller is child of a system controller:
there are two system controllers - "scpsystem" is for "compute part", and the
"hfrpsystem" is for the "multimedia part" of the soc).

  _______________________________________
|                                       |
| SYSTEM CONTROLLER (SCPSYS or HFRPSYS) |
|   _____________________               |
|  |                     |              | <===> Clock Controllers (more than one)
|  | Power Controller    |     SOME     |       (provide subsystem clocks for iso
|  |                     |    OTHER     |        during power domain enablement
|  |     ______________  |   BLOCKS     |        even if a PD is voted)
|  |    |              | |              |       non-subsystem clocks are voted,
|  |    | HW Voter MCU | |              |       but subsystem ones are not voted
|  |    |______________| |              |
|  |_____________________|              | ===> Rest of the SoC
|_______________________________________|


Hence I'm asking you - does your idea still stand?

Because after this, sorry for that - this doesn't want to be an attack - but
I'm starting to have doubts about an approach that doesn't involve syscons.

Cheers,
Angelo

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ