[RFC PATCH v2 00/19] Enable drm/imagination BXM-4-64 Support for LicheePi 4A

[RFC PATCH v2 00/19] Enable drm/imagination BXM-4-64 Support for LicheePi 4A

[Michal Wilczynski]

The LicheePi 4A board, featuring the T-HEAD TH1520 SoC, includes an Imagination
Technologies BXM-4-64 GPU. Initial support for this GPU was provided through a
downstream driver [1]. Recently, efforts have been made to upstream support for
the Rogue family GPUs, which the BXM-4-64 is part of [2].

While the initial upstream driver focused on the AXE-1-16 GPU, newer patches
have introduced support for the BXS-4-64 GPU [3]. The modern upstream
drm/imagination driver is expected to support the BXM-4-64 as well [4][5]. As
this support is being developed, it's crucial to upstream the necessary glue
code including clock and power-domain drivers so they're ready for integration
with the drm/imagination driver.

Recent Progress:

Firmware Improvements:
Since August, the vendor has provided updated firmware
[6][7] that correctly initiates the firmware for the BXM-4-64.

Mesa Driver Testing:
The vendor-supplied Mesa driver [8] partially works with Vulkan examples, such
as rendering a triangle using Sascha Willems' Vulkan samples [9]. Although the
triangle isn't rendered correctly (only the blue background appears), shader
job submissions function properly, and IOCTL calls are correctly invoked.  For
testing, we used the following resources:

Kernel Source: Custom kernel with necessary modifications [10].
Mesa Driver: Vendor-provided Mesa implementation [11].

Dependencies:
Testing required a functional Display Processing Unit (DPU) and HDMI driver,
which are currently not upstreamed. Efforts are underway to upstream the DPU
DC8200 driver used in StarFive boards [12], which is the same DPU used on the
LicheePi 4A. Once the DPU and HDMI drivers are upstreamed, GPU support can be
fully upstream.

Testing Status:
This series has been tested by performing a probe-only operation, confirming
that the firmware begins execution. The probe function initiates firmware
execution and waits for the firmware to flip a specific status bit.

[   12.637880] powervr ffef400000.gpu: [drm] loaded firmware powervr/rogue_36.52.104.182_v1.fw
[   12.648979] powervr ffef400000.gpu: [drm] FW version v1.0 (build 6645434 OS)
[   12.678906] [drm] Initialized powervr 1.0.0 for ffef400000.gpu on minor 0

Power Management:
Full power management capabilities require implementing the T-HEAD SoC AON
protocol messaging via the hardware mailbox. Support for the mailbox was merged
in kernel 6.13 [13], and the AON protocol implementation is part of this
series, since v2. Therefore this series support full power management
capabilities for the GPU driver.

Thanks Krzysztof and Stephen for taking the time to review the last revision !
Your guidance and the direction was very helpful.

v2:

Removed AP_SUBSYS clock refactoring commits (1-6):
 - instead of refactoring, I opted to extend the current driver and its
   associated device tree node to include support for a second address space.

Expanded patchset scope to fully support power management capabilities:
 - introduced a new firmware driver to manage power-related operations.
 - rewrote the power-domain driver to function alongside the firmware driver.
   These nodes in the device tree lack direct address spaces, despite
   representing HW blocks. Control is achieved via firmware protocol messages
   transmitted through a mailbox to the E902 core.

Implemented a reset controller for the TH1520 SoC:
 - developed a reset controller driver for the TH1520 to manage reset
   sequences.
 - updated the drm/imagination driver to act as a reset controller consumer.
   While this patchset is focused on the LPI4A board, the reset controller is
   designed to be useful for other boards, such as the BPI-3F, which also require
   a reset sequence after power-up.

Updated dt-bindings:
 - added new dt-bindings for power, reset, and firmware nodes.
 - updated the powervr dt-binding to include reset support and new compatibles.
 - ran dtbs_check and dt_binding_check to ensure compliance.

Addressed code quality:
 - resolved all checkpatch issues using --strict, except for the call to
   devm_clk_hw_register_gate_parent_data().  The current implementation remains
   preferable in this context, and clang-format aligns with this choice.

Also included the mailbox device tree commit, already queued for 6.14 [14] for
completeness.

References:

[1] Downstream Driver Source:
    https://gitlab.freedesktop.org/frankbinns/powervr/-/blob/cb1929932095649a24f051b9cfdd2cd2ceab35cb/drivers/gpu/drm/img-rogue/Kconfig

[2] Initial Upstream Driver Series:
    https://lore.kernel.org/all/cover.1700668843.git.donald.robson@imgtec.com/

[3] BXS-4-64 GPU Support Patches:
    https://lore.kernel.org/all/20241105-sets-bxs-4-64-patch-v1-v1-0-4ed30e865892@imgtec.com/

[4] Firmware Issue Discussion 1:
    https://gitlab.freedesktop.org/imagination/linux-firmware/-/issues/1

[5] Firmware Issue Discussion 2:
    https://gitlab.freedesktop.org/imagination/linux-firmware/-/issues/2

[6] Firmware Update Commit 1:
    https://gitlab.freedesktop.org/imagination/linux-firmware/-/commit/6ac2247e9a1d1837af495fb6d0fbd6f35547c2d1

[7] Firmware Update Commit 2:
    https://gitlab.freedesktop.org/imagination/linux-firmware/-/commit/efbebc90f25adb2b2e1499e3cc24ea3f3c3e4f4c

[8] Vendor-Provided Mesa Driver:
    https://gitlab.freedesktop.org/imagination/mesa/-/tree/dev/devinfo

[9] Sascha Willems' Vulkan Samples:     https://github.com/SaschaWillems/Vulkan

[10] Test Kernel Source:
    https://github.com/mwilczy/linux/tree/2_December_reference_linux_kernel_imagination

[11] Test Mesa Driver:
    https://github.com/mwilczy/mesa-reference

[12] DPU DC8200 Driver Upstream Attempt:
    https://lore.kernel.org/all/20241120061848.196754-1-keith.zhao@starfivetech.com/

[13] Pull request kernel 6.13 for mailbox
    https://lore.kernel.org/all/CABb+yY33qnivK-PzqpSMgmtbFid4nS8wcNvP7wED9DXrYAyLKg@mail.gmail.com/

[14] Mailbox commit queued for 6.14
    https://lore.kernel.org/all/20241104100734.1276116-4-m.wilczynski@samsung.com/

Michal Wilczynski (19):
  dt-bindings: clock: Add VO subsystem clocks and update address
    requirements
  clk: thead: Add clock support for VO subsystem in T-Head TH1520 SoC
  dt-bindings: power: thead,th1520: Add support for power domains
  dt-bindings: firmware: thead,th1520: Add support for firmware node
  firmware: thead: Add AON firmware protocol driver
  soc: thead: power-domain: Add power-domain driver for TH1520
  riscv: Enable PM_GENERIC_DOMAINS for T-Head SoCs
  dt-bindings: reset: Add T-HEAD TH1520 SoC Reset Controller
  reset: thead: Add TH1520 reset controller driver
  drm/imagination: Add reset controller support for GPU initialization
  dt-bindings: gpu: Add 'resets' property for GPU initialization
  dt-bindings: gpu: Add compatibles for T-HEAD TH1520 GPU
  drm/imagination: Add support for IMG BXM-4-64 GPU
  drm/imagination: Enable PowerVR driver for RISC-V
  riscv: dts: thead: Extend device tree clk with VO reg
  riscv: dts: thead: Add mailbox node
  riscv: dts: thead: Introduce power domain nodes with aon firmware
  riscv: dts: thead: Introduce reset controller node
  riscv: dts: thead: Add GPU node to TH1520 device tree

 .../bindings/clock/thead,th1520-clk-ap.yaml   |  15 +-
 .../bindings/firmware/thead,th1520-aon.yaml   |  59 +++++
 .../bindings/gpu/img,powervr-rogue.yaml       |  38 +++-
 .../bindings/power/thead,th1520-power.yaml    |  42 ++++
 .../bindings/reset/thead,th1520-reset.yaml    |  45 ++++
 MAINTAINERS                                   |   8 +
 arch/riscv/Kconfig.socs                       |   1 +
 arch/riscv/boot/dts/thead/th1520.dtsi         |  54 ++++-
 drivers/clk/thead/clk-th1520-ap.c             | 160 ++++++++++++--
 drivers/firmware/Kconfig                      |   9 +
 drivers/firmware/Makefile                     |   1 +
 drivers/firmware/thead,th1520-aon.c           | 203 ++++++++++++++++++
 drivers/gpu/drm/imagination/Kconfig           |   2 +-
 drivers/gpu/drm/imagination/pvr_device.c      |  21 ++
 drivers/gpu/drm/imagination/pvr_device.h      |   9 +
 drivers/gpu/drm/imagination/pvr_drv.c         |   1 +
 drivers/gpu/drm/imagination/pvr_power.c       |  15 +-
 drivers/pmdomain/Kconfig                      |   1 +
 drivers/pmdomain/Makefile                     |   1 +
 drivers/pmdomain/thead/Kconfig                |  12 ++
 drivers/pmdomain/thead/Makefile               |   2 +
 drivers/pmdomain/thead/th1520-pm-domains.c    | 181 ++++++++++++++++
 drivers/reset/Kconfig                         |  10 +
 drivers/reset/Makefile                        |   1 +
 drivers/reset/reset-th1520.c                  | 151 +++++++++++++
 .../dt-bindings/clock/thead,th1520-clk-ap.h   |  33 +++
 .../dt-bindings/power/thead,th1520-power.h    |  18 ++
 .../dt-bindings/reset/thead,th1520-reset.h    |  13 ++
 .../linux/firmware/thead/thead,th1520-aon.h   | 186 ++++++++++++++++
 29 files changed, 1266 insertions(+), 26 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/firmware/thead,th1520-aon.yaml
 create mode 100644 Documentation/devicetree/bindings/power/thead,th1520-power.yaml
 create mode 100644 Documentation/devicetree/bindings/reset/thead,th1520-reset.yaml
 create mode 100644 drivers/firmware/thead,th1520-aon.c
 create mode 100644 drivers/pmdomain/thead/Kconfig
 create mode 100644 drivers/pmdomain/thead/Makefile
 create mode 100644 drivers/pmdomain/thead/th1520-pm-domains.c
 create mode 100644 drivers/reset/reset-th1520.c
 create mode 100644 include/dt-bindings/power/thead,th1520-power.h
 create mode 100644 include/dt-bindings/reset/thead,th1520-reset.h
 create mode 100644 include/linux/firmware/thead/thead,th1520-aon.h

-- 
2.34.1

[RFC PATCH v2 01/19] dt-bindings: clock: Add VO subsystem clocks and update address requirements

[Michal Wilczynski]

The T-Head TH1520 SoC’s AP clock controller now needs two address ranges
to manage both the Application Processor (AP) and Video Output (VO)
subsystem clocks. Update the device tree bindings to require two `reg`
entries, one for the AP clocks and one for the VO clocks.

Additionally, introduce new VO subsystem clock constants in the header
file. These constants will be used by the driver to control VO-related
components such as display and graphics units.

Signed-off-by: Michal Wilczynski <m.wilczynski@samsung.com>
---
 .../bindings/clock/thead,th1520-clk-ap.yaml   | 15 +++++++--
 .../dt-bindings/clock/thead,th1520-clk-ap.h   | 33 +++++++++++++++++++
 2 files changed, 45 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/clock/thead,th1520-clk-ap.yaml b/Documentation/devicetree/bindings/clock/thead,th1520-clk-ap.yaml
index 0129bd0ba4b3..f0df97a450ef 100644
--- a/Documentation/devicetree/bindings/clock/thead,th1520-clk-ap.yaml
+++ b/Documentation/devicetree/bindings/clock/thead,th1520-clk-ap.yaml
@@ -8,7 +8,8 @@ title: T-HEAD TH1520 AP sub-system clock controller
 
 description: |
   The T-HEAD TH1520 AP sub-system clock controller configures the
-  CPU, DPU, GMAC and TEE PLLs.
+  CPU, DPU, GMAC and TEE PLLs. Additionally the VO subsystem configures
+  the clock gates for the HDMI, MIPI and the GPU.
 
   SoC reference manual
   https://openbeagle.org/beaglev-ahead/beaglev-ahead/-/blob/main/docs/TH1520%20System%20User%20Manual.pdf
@@ -23,7 +24,13 @@ properties:
     const: thead,th1520-clk-ap
 
   reg:
-    maxItems: 1
+    minItems: 2
+    maxItems: 2
+
+  reg-names:
+    items:
+      - const: ap-clks
+      - const: vo-clks
 
   clocks:
     items:
@@ -47,7 +54,9 @@ examples:
     #include <dt-bindings/clock/thead,th1520-clk-ap.h>
     clock-controller@ef010000 {
         compatible = "thead,th1520-clk-ap";
-        reg = <0xef010000 0x1000>;
+        reg = <0xef010000 0x1000>,
+              <0xff010000 0x1000>;
+        reg-names = "ap-clks", "vo-clks";
         clocks = <&osc>;
         #clock-cells = <1>;
     };
diff --git a/include/dt-bindings/clock/thead,th1520-clk-ap.h b/include/dt-bindings/clock/thead,th1520-clk-ap.h
index a199784b3512..8b9a98f878a6 100644
--- a/include/dt-bindings/clock/thead,th1520-clk-ap.h
+++ b/include/dt-bindings/clock/thead,th1520-clk-ap.h
@@ -93,4 +93,37 @@
 #define CLK_SRAM3		83
 #define CLK_PLL_GMAC_100M	84
 #define CLK_UART_SCLK		85
+
+/* VO clocks */
+#define CLK_AXI4_VO_ACLK		86
+#define CLK_GPU_CORE			87
+#define CLK_GPU_CFG_ACLK		88
+#define CLK_DPU_PIXELCLK0		89
+#define CLK_DPU_PIXELCLK1		90
+#define CLK_DPU_HCLK			91
+#define CLK_DPU_ACLK			92
+#define CLK_DPU_CCLK			93
+#define CLK_HDMI_SFR			94
+#define CLK_HDMI_PCLK			95
+#define CLK_HDMI_CEC			96
+#define CLK_MIPI_DSI0_PCLK		97
+#define CLK_MIPI_DSI1_PCLK		98
+#define CLK_MIPI_DSI0_CFG		99
+#define CLK_MIPI_DSI1_CFG		100
+#define CLK_MIPI_DSI0_REFCLK		101
+#define CLK_MIPI_DSI1_REFCLK		102
+#define CLK_HDMI_I2S			103
+#define CLK_X2H_DPU1_ACLK		104
+#define CLK_X2H_DPU_ACLK		105
+#define CLK_AXI4_VO_PCLK		106
+#define CLK_IOPMP_VOSYS_DPU_PCLK	107
+#define CLK_IOPMP_VOSYS_DPU1_PCLK	108
+#define CLK_IOPMP_VOSYS_GPU_PCLK	109
+#define CLK_IOPMP_DPU1_ACLK		110
+#define CLK_IOPMP_DPU_ACLK		111
+#define CLK_IOPMP_GPU_ACLK		112
+#define CLK_MIPIDSI0_PIXCLK		113
+#define CLK_MIPIDSI1_PIXCLK		114
+#define CLK_HDMI_PIXCLK			115
+
 #endif
-- 
2.34.1

Re: [RFC PATCH v2 01/19] dt-bindings: clock: Add VO subsystem clocks and update address requirements

[Krzysztof Kozlowski]

On 23/12/2024 13:55, Michal Wilczynski wrote:

>  description: |
>    The T-HEAD TH1520 AP sub-system clock controller configures the
> -  CPU, DPU, GMAC and TEE PLLs.
> +  CPU, DPU, GMAC and TEE PLLs. Additionally the VO subsystem configures
> +  the clock gates for the HDMI, MIPI and the GPU.
>  
>    SoC reference manual
>    https://openbeagle.org/beaglev-ahead/beaglev-ahead/-/blob/main/docs/TH1520%20System%20User%20Manual.pdf
> @@ -23,7 +24,13 @@ properties:
>      const: thead,th1520-clk-ap
>  
>    reg:
> -    maxItems: 1
> +    minItems: 2

You can drop minItems in this case.

> +    maxItems: 2
> +
> +  reg-names:
> +    items:
> +      - const: ap-clks
> +      - const: vo-clks


Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>


Best regards,
Krzysztof

Re: [RFC PATCH v2 01/19] dt-bindings: clock: Add VO subsystem clocks and update address requirements

[Stephen Boyd]

Quoting Michal Wilczynski (2024-12-23 04:55:35)
> The T-Head TH1520 SoC’s AP clock controller now needs two address ranges
> to manage both the Application Processor (AP) and Video Output (VO)
> subsystem clocks. Update the device tree bindings to require two `reg`
> entries, one for the AP clocks and one for the VO clocks.
> 
> Additionally, introduce new VO subsystem clock constants in the header
> file. These constants will be used by the driver to control VO-related
> components such as display and graphics units.
> 
> Signed-off-by: Michal Wilczynski <m.wilczynski@samsung.com>
> ---
[...]
> diff --git a/Documentation/devicetree/bindings/clock/thead,th1520-clk-ap.yaml b/Documentation/devicetree/bindings/clock/thead,th1520-clk-ap.yaml
> index 0129bd0ba4b3..f0df97a450ef 100644
> --- a/Documentation/devicetree/bindings/clock/thead,th1520-clk-ap.yaml
> +++ b/Documentation/devicetree/bindings/clock/thead,th1520-clk-ap.yaml
> @@ -47,7 +54,9 @@ examples:
>      #include <dt-bindings/clock/thead,th1520-clk-ap.h>
>      clock-controller@ef010000 {
>          compatible = "thead,th1520-clk-ap";
> -        reg = <0xef010000 0x1000>;
> +        reg = <0xef010000 0x1000>,
> +              <0xff010000 0x1000>;

I don't get it. Why not have two nodes and two devices? They have
different register regions so likely they're different devices on the
internal SoC bus. They may have the same input clks, but otherwise I
don't see how they're the same node.

> +        reg-names = "ap-clks", "vo-clks";
>          clocks = <&osc>;
>          #clock-cells = <1>;
>      };

Re: [RFC PATCH v2 01/19] dt-bindings: clock: Add VO subsystem clocks and update address requirements

[Krzysztof Kozlowski]

On Mon, Dec 23, 2024 at 12:50:59PM -0800, Stephen Boyd wrote:
> Quoting Michal Wilczynski (2024-12-23 04:55:35)
> > The T-Head TH1520 SoC’s AP clock controller now needs two address ranges
> > to manage both the Application Processor (AP) and Video Output (VO)
> > subsystem clocks. Update the device tree bindings to require two `reg`
> > entries, one for the AP clocks and one for the VO clocks.
> > 
> > Additionally, introduce new VO subsystem clock constants in the header
> > file. These constants will be used by the driver to control VO-related
> > components such as display and graphics units.
> > 
> > Signed-off-by: Michal Wilczynski <m.wilczynski@samsung.com>
> > ---
> [...]
> > diff --git a/Documentation/devicetree/bindings/clock/thead,th1520-clk-ap.yaml b/Documentation/devicetree/bindings/clock/thead,th1520-clk-ap.yaml
> > index 0129bd0ba4b3..f0df97a450ef 100644
> > --- a/Documentation/devicetree/bindings/clock/thead,th1520-clk-ap.yaml
> > +++ b/Documentation/devicetree/bindings/clock/thead,th1520-clk-ap.yaml
> > @@ -47,7 +54,9 @@ examples:
> >      #include <dt-bindings/clock/thead,th1520-clk-ap.h>
> >      clock-controller@ef010000 {
> >          compatible = "thead,th1520-clk-ap";
> > -        reg = <0xef010000 0x1000>;
> > +        reg = <0xef010000 0x1000>,
> > +              <0xff010000 0x1000>;
> 
> I don't get it. Why not have two nodes and two devices? They have
> different register regions so likely they're different devices on the
> internal SoC bus. They may have the same input clks, but otherwise I
> don't see how they're the same node.

That's a good point. Aren't here simply two different clock controllers?

Best regards,
Krzysztof

Re: [RFC PATCH v2 01/19] dt-bindings: clock: Add VO subsystem clocks and update address requirements

[Michal Wilczynski]

On 12/24/24 09:53, Krzysztof Kozlowski wrote:
> On Mon, Dec 23, 2024 at 12:50:59PM -0800, Stephen Boyd wrote:
>> Quoting Michal Wilczynski (2024-12-23 04:55:35)
>>> The T-Head TH1520 SoC’s AP clock controller now needs two address ranges
>>> to manage both the Application Processor (AP) and Video Output (VO)
>>> subsystem clocks. Update the device tree bindings to require two `reg`
>>> entries, one for the AP clocks and one for the VO clocks.
>>>
>>> Additionally, introduce new VO subsystem clock constants in the header
>>> file. These constants will be used by the driver to control VO-related
>>> components such as display and graphics units.
>>>
>>> Signed-off-by: Michal Wilczynski <m.wilczynski@samsung.com>
>>> ---
>> [...]
>>> diff --git a/Documentation/devicetree/bindings/clock/thead,th1520-clk-ap.yaml b/Documentation/devicetree/bindings/clock/thead,th1520-clk-ap.yaml
>>> index 0129bd0ba4b3..f0df97a450ef 100644
>>> --- a/Documentation/devicetree/bindings/clock/thead,th1520-clk-ap.yaml
>>> +++ b/Documentation/devicetree/bindings/clock/thead,th1520-clk-ap.yaml
>>> @@ -47,7 +54,9 @@ examples:
>>>      #include <dt-bindings/clock/thead,th1520-clk-ap.h>
>>>      clock-controller@ef010000 {
>>>          compatible = "thead,th1520-clk-ap";
>>> -        reg = <0xef010000 0x1000>;
>>> +        reg = <0xef010000 0x1000>,
>>> +              <0xff010000 0x1000>;
>>
>> I don't get it. Why not have two nodes and two devices? They have
>> different register regions so likely they're different devices on the
>> internal SoC bus. They may have the same input clks, but otherwise I
>> don't see how they're the same node.
> 
> That's a good point. Aren't here simply two different clock controllers?

Yeah there are two clock controllers, based on the review comments I was
trying to re-use the driver, but the driver can also be re-used to serve
multiple nodes and multiple compatible and .data properties, if that's
fine with you that's how it will look like in v3.

Thanks,
Michał
> 
> Best regards,
> Krzysztof
> 
> 

Re: [RFC PATCH v2 01/19] dt-bindings: clock: Add VO subsystem clocks and update address requirements

[Krzysztof Kozlowski]

On 24/12/2024 10:23, Michal Wilczynski wrote:
> 
> 
> On 12/24/24 09:53, Krzysztof Kozlowski wrote:
>> On Mon, Dec 23, 2024 at 12:50:59PM -0800, Stephen Boyd wrote:
>>> Quoting Michal Wilczynski (2024-12-23 04:55:35)
>>>> The T-Head TH1520 SoC’s AP clock controller now needs two address ranges
>>>> to manage both the Application Processor (AP) and Video Output (VO)
>>>> subsystem clocks. Update the device tree bindings to require two `reg`
>>>> entries, one for the AP clocks and one for the VO clocks.
>>>>
>>>> Additionally, introduce new VO subsystem clock constants in the header
>>>> file. These constants will be used by the driver to control VO-related
>>>> components such as display and graphics units.
>>>>
>>>> Signed-off-by: Michal Wilczynski <m.wilczynski@samsung.com>
>>>> ---
>>> [...]
>>>> diff --git a/Documentation/devicetree/bindings/clock/thead,th1520-clk-ap.yaml b/Documentation/devicetree/bindings/clock/thead,th1520-clk-ap.yaml
>>>> index 0129bd0ba4b3..f0df97a450ef 100644
>>>> --- a/Documentation/devicetree/bindings/clock/thead,th1520-clk-ap.yaml
>>>> +++ b/Documentation/devicetree/bindings/clock/thead,th1520-clk-ap.yaml
>>>> @@ -47,7 +54,9 @@ examples:
>>>>      #include <dt-bindings/clock/thead,th1520-clk-ap.h>
>>>>      clock-controller@ef010000 {
>>>>          compatible = "thead,th1520-clk-ap";
>>>> -        reg = <0xef010000 0x1000>;
>>>> +        reg = <0xef010000 0x1000>,
>>>> +              <0xff010000 0x1000>;
>>>
>>> I don't get it. Why not have two nodes and two devices? They have
>>> different register regions so likely they're different devices on the
>>> internal SoC bus. They may have the same input clks, but otherwise I
>>> don't see how they're the same node.
>>
>> That's a good point. Aren't here simply two different clock controllers?
> 
> Yeah there are two clock controllers, based on the review comments I was
> trying to re-use the driver, but the driver can also be re-used to serve
> multiple nodes and multiple compatible and .data properties, if that's
> fine with you that's how it will look like in v3.
Yeah, please drop my review tag and rework it to have two different
devices. Driver design should not influence DTS.

Best regards,
Krzysztof

[RFC PATCH v2 02/19] clk: thead: Add clock support for VO subsystem in T-Head TH1520 SoC

[Michal Wilczynski]

The T-Head TH1520 SoC integrates a variety of clocks for its subsystems,
including the Application Processor (AP) and the Video Output (VO) [1].
Up until now, the T-Head clock driver only supported AP clocks.

This commit extends the driver to provide clock functionality for the VO
subsystem. At this stage, the focus is on implementing the VO clock
gates, as these are currently the most relevant and required components
for enabling and disabling the VO subsystem functionality. Future
enhancements may introduce additional VO-related clocks as necessary.

Link: https://openbeagle.org/beaglev-ahead/beaglev-ahead/-/blob/main/docs/TH1520%20System%20User%20Manual.pdf [1]

Signed-off-by: Michal Wilczynski <m.wilczynski@samsung.com>
---
 drivers/clk/thead/clk-th1520-ap.c | 160 ++++++++++++++++++++++++++----
 1 file changed, 142 insertions(+), 18 deletions(-)

diff --git a/drivers/clk/thead/clk-th1520-ap.c b/drivers/clk/thead/clk-th1520-ap.c
index 17e32ae08720..b3c719d4f4ac 100644
--- a/drivers/clk/thead/clk-th1520-ap.c
+++ b/drivers/clk/thead/clk-th1520-ap.c
@@ -840,6 +840,67 @@ static CCU_GATE(CLK_SRAM1, sram1_clk, "sram1", axi_aclk_pd, 0x20c, BIT(3), 0);
 static CCU_GATE(CLK_SRAM2, sram2_clk, "sram2", axi_aclk_pd, 0x20c, BIT(2), 0);
 static CCU_GATE(CLK_SRAM3, sram3_clk, "sram3", axi_aclk_pd, 0x20c, BIT(1), 0);
 
+static CCU_GATE(CLK_AXI4_VO_ACLK, axi4_vo_aclk, "axi4-vo-aclk",
+		video_pll_clk_pd, 0x0, BIT(0), 0);
+static CCU_GATE(CLK_GPU_CORE, gpu_core_clk, "gpu-core-clk", video_pll_clk_pd,
+		0x0, BIT(3), 0);
+static CCU_GATE(CLK_GPU_CFG_ACLK, gpu_cfg_aclk, "gpu-cfg-aclk",
+		video_pll_clk_pd, 0x0, BIT(4), 0);
+static CCU_GATE(CLK_DPU_PIXELCLK0, dpu0_pixelclk, "dpu0-pixelclk",
+		video_pll_clk_pd, 0x0, BIT(5), 0);
+static CCU_GATE(CLK_DPU_PIXELCLK1, dpu1_pixelclk, "dpu1-pixelclk",
+		video_pll_clk_pd, 0x0, BIT(6), 0);
+static CCU_GATE(CLK_DPU_HCLK, dpu_hclk, "dpu-hclk", video_pll_clk_pd, 0x0,
+		BIT(7), 0);
+static CCU_GATE(CLK_DPU_ACLK, dpu_aclk, "dpu-aclk", video_pll_clk_pd, 0x0,
+		BIT(8), 0);
+static CCU_GATE(CLK_DPU_CCLK, dpu_cclk, "dpu-cclk", video_pll_clk_pd, 0x0,
+		BIT(9), 0);
+static CCU_GATE(CLK_HDMI_SFR, hdmi_sfr_clk, "hdmi-sfr-clk", video_pll_clk_pd,
+		0x0, BIT(10), 0);
+static CCU_GATE(CLK_HDMI_PCLK, hdmi_pclk, "hdmi-pclk", video_pll_clk_pd, 0x0,
+		BIT(11), 0);
+static CCU_GATE(CLK_HDMI_CEC, hdmi_cec_clk, "hdmi-cec-clk", video_pll_clk_pd,
+		0x0, BIT(12), 0);
+static CCU_GATE(CLK_MIPI_DSI0_PCLK, mipi_dsi0_pclk, "mipi-dsi0-pclk",
+		video_pll_clk_pd, 0x0, BIT(13), 0);
+static CCU_GATE(CLK_MIPI_DSI1_PCLK, mipi_dsi1_pclk, "mipi-dsi1-pclk",
+		video_pll_clk_pd, 0x0, BIT(14), 0);
+static CCU_GATE(CLK_MIPI_DSI0_CFG, mipi_dsi0_cfg_clk, "mipi-dsi0-cfg-clk",
+		video_pll_clk_pd, 0x0, BIT(15), 0);
+static CCU_GATE(CLK_MIPI_DSI1_CFG, mipi_dsi1_cfg_clk, "mipi-dsi1-cfg-clk",
+		video_pll_clk_pd, 0x0, BIT(16), 0);
+static CCU_GATE(CLK_MIPI_DSI0_REFCLK, mipi_dsi0_refclk, "mipi-dsi0-refclk",
+		video_pll_clk_pd, 0x0, BIT(17), 0);
+static CCU_GATE(CLK_MIPI_DSI1_REFCLK, mipi_dsi1_refclk, "mipi-dsi1-refclk",
+		video_pll_clk_pd, 0x0, BIT(18), 0);
+static CCU_GATE(CLK_HDMI_I2S, hdmi_i2c_clk, "hdmi-i2c-clk", video_pll_clk_pd,
+		0x0, BIT(19), 0);
+static CCU_GATE(CLK_X2H_DPU1_ACLK, x2h_dpu1_aclk, "x2h-dpu1-aclk",
+		video_pll_clk_pd, 0x0, BIT(20), 0);
+static CCU_GATE(CLK_X2H_DPU_ACLK, x2h_dpu_aclk, "x2h-dpu-aclk",
+		video_pll_clk_pd, 0x0, BIT(21), 0);
+static CCU_GATE(CLK_AXI4_VO_PCLK, axi4_vo_pclk, "axi4-vo-pclk",
+		video_pll_clk_pd, 0x0, BIT(22), 0);
+static CCU_GATE(CLK_IOPMP_VOSYS_DPU_PCLK, iopmp_vosys_dpu_pclk,
+		"iopmp-vosys-dpu-pclk", video_pll_clk_pd, 0x0, BIT(23), 0);
+static CCU_GATE(CLK_IOPMP_VOSYS_DPU1_PCLK, iopmp_vosys_dpu1_pclk,
+		"iopmp-vosys-dpu1-pclk", video_pll_clk_pd, 0x0, BIT(24), 0);
+static CCU_GATE(CLK_IOPMP_VOSYS_GPU_PCLK, iopmp_vosys_gpu_pclk,
+		"iopmp-vosys-gpu-pclk", video_pll_clk_pd, 0x0, BIT(25), 0);
+static CCU_GATE(CLK_IOPMP_DPU1_ACLK, iopmp_dpu1_aclk, "iopmp-dpu1-aclk",
+		video_pll_clk_pd, 0x0, BIT(27), 0);
+static CCU_GATE(CLK_IOPMP_DPU_ACLK, iopmp_dpu_aclk, "iopmp-dpu-aclk",
+		video_pll_clk_pd, 0x0, BIT(28), 0);
+static CCU_GATE(CLK_IOPMP_GPU_ACLK, iopmp_gpu_aclk, "iopmp-gpu-aclk",
+		video_pll_clk_pd, 0x0, BIT(29), 0);
+static CCU_GATE(CLK_MIPIDSI0_PIXCLK, mipi_dsi0_pixclk, "mipi-dsi0-pixclk",
+		video_pll_clk_pd, 0x0, BIT(30), 0);
+static CCU_GATE(CLK_MIPIDSI1_PIXCLK, mipi_dsi1_pixclk, "mipi-dsi1-pixclk",
+		video_pll_clk_pd, 0x0, BIT(31), 0);
+static CCU_GATE(CLK_HDMI_PIXCLK, hdmi_pixclk, "hdmi-pixclk", video_pll_clk_pd,
+		0x4, BIT(0), 0);
+
 static CLK_FIXED_FACTOR_HW(gmac_pll_clk_100m, "gmac-pll-clk-100m",
 			   &gmac_pll_clk.common.hw, 10, 1, 0);
 
@@ -956,7 +1017,40 @@ static struct ccu_common *th1520_gate_clks[] = {
 	&sram3_clk.common,
 };
 
-#define NR_CLKS	(CLK_UART_SCLK + 1)
+static struct ccu_common *th1520_vo_gate_clks[] = {
+	&axi4_vo_aclk.common,
+	&gpu_core_clk.common,
+	&gpu_cfg_aclk.common,
+	&dpu0_pixelclk.common,
+	&dpu1_pixelclk.common,
+	&dpu_hclk.common,
+	&dpu_aclk.common,
+	&dpu_cclk.common,
+	&hdmi_sfr_clk.common,
+	&hdmi_pclk.common,
+	&hdmi_cec_clk.common,
+	&mipi_dsi0_pclk.common,
+	&mipi_dsi1_pclk.common,
+	&mipi_dsi0_cfg_clk.common,
+	&mipi_dsi1_cfg_clk.common,
+	&mipi_dsi0_refclk.common,
+	&mipi_dsi1_refclk.common,
+	&hdmi_i2c_clk.common,
+	&x2h_dpu1_aclk.common,
+	&x2h_dpu_aclk.common,
+	&axi4_vo_pclk.common,
+	&iopmp_vosys_dpu_pclk.common,
+	&iopmp_vosys_dpu1_pclk.common,
+	&iopmp_vosys_gpu_pclk.common,
+	&iopmp_dpu1_aclk.common,
+	&iopmp_dpu_aclk.common,
+	&iopmp_gpu_aclk.common,
+	&mipi_dsi0_pixclk.common,
+	&mipi_dsi1_pixclk.common,
+	&hdmi_pixclk.common
+};
+
+#define NR_CLKS	(CLK_HDMI_PIXCLK + 1)
 
 static const struct regmap_config th1520_clk_regmap_config = {
 	.reg_bits = 32,
@@ -965,13 +1059,40 @@ static const struct regmap_config th1520_clk_regmap_config = {
 	.fast_io = true,
 };
 
+static int th1520_add_gate_clocks(struct device *dev,
+				  struct clk_hw_onecell_data *priv,
+				  struct regmap *map, void __iomem *base,
+				  struct ccu_common **gate_clks,
+				  size_t gate_clks_size)
+{
+	struct clk_hw *hw;
+	int i;
+
+	for (i = 0; i < gate_clks_size; i++) {
+		struct ccu_gate *cg = hw_to_ccu_gate(&gate_clks[i]->hw);
+
+		gate_clks[i]->map = map;
+
+		hw = devm_clk_hw_register_gate_parent_data(
+			dev, cg->common.hw.init->name,
+			cg->common.hw.init->parent_data, 0,
+			base + cg->common.cfg0, ffs(cg->enable) - 1, 0, NULL);
+		if (IS_ERR(hw))
+			return PTR_ERR(hw);
+
+		priv->hws[cg->common.clkid] = hw;
+	}
+
+	return 0;
+}
+
 static int th1520_clk_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
 	struct clk_hw_onecell_data *priv;
 
-	struct regmap *map;
-	void __iomem *base;
+	struct regmap *map, *map_vosys;
+	void __iomem *base, *base_vosys;
 	struct clk_hw *hw;
 	int ret, i;
 
@@ -981,14 +1102,22 @@ static int th1520_clk_probe(struct platform_device *pdev)
 
 	priv->num = NR_CLKS;
 
-	base = devm_platform_ioremap_resource(pdev, 0);
+	base = devm_platform_ioremap_resource_byname(pdev, "ap-clks");
 	if (IS_ERR(base))
 		return PTR_ERR(base);
 
+	base_vosys = devm_platform_ioremap_resource_byname(pdev, "vo-clks");
+	if (IS_ERR(base_vosys))
+		return PTR_ERR(base_vosys);
+
 	map = devm_regmap_init_mmio(dev, base, &th1520_clk_regmap_config);
 	if (IS_ERR(map))
 		return PTR_ERR(map);
 
+	map_vosys = devm_regmap_init_mmio(dev, base_vosys, &th1520_clk_regmap_config);
+	if (IS_ERR(map_vosys))
+		return PTR_ERR(map_vosys);
+
 	for (i = 0; i < ARRAY_SIZE(th1520_pll_clks); i++) {
 		struct ccu_pll *cp = hw_to_ccu_pll(&th1520_pll_clks[i]->hw);
 
@@ -1033,21 +1162,16 @@ static int th1520_clk_probe(struct platform_device *pdev)
 		priv->hws[cm->common.clkid] = hw;
 	}
 
-	for (i = 0; i < ARRAY_SIZE(th1520_gate_clks); i++) {
-		struct ccu_gate *cg = hw_to_ccu_gate(&th1520_gate_clks[i]->hw);
-
-		th1520_gate_clks[i]->map = map;
-
-		hw = devm_clk_hw_register_gate_parent_data(dev,
-							   cg->common.hw.init->name,
-							   cg->common.hw.init->parent_data,
-							   0, base + cg->common.cfg0,
-							   ffs(cg->enable) - 1, 0, NULL);
-		if (IS_ERR(hw))
-			return PTR_ERR(hw);
+	ret = th1520_add_gate_clocks(dev, priv, map, base, th1520_gate_clks,
+				     ARRAY_SIZE(th1520_gate_clks));
+	if (ret)
+		return ret;
 
-		priv->hws[cg->common.clkid] = hw;
-	}
+	ret = th1520_add_gate_clocks(dev, priv, map_vosys, base_vosys,
+				     th1520_vo_gate_clks,
+				     ARRAY_SIZE(th1520_vo_gate_clks));
+	if (ret)
+		return ret;
 
 	ret = devm_clk_hw_register(dev, &osc12m_clk.hw);
 	if (ret)
-- 
2.34.1

[RFC PATCH v2 03/19] dt-bindings: power: thead,th1520: Add support for power domains

[Michal Wilczynski]

Introduce a device tree binding for the T-HEAD TH1520 power domain
controller. This allows devices to specify their power domain
dependencies, thereby improving power management for subsystems such as
the GPU. The new YAML schema describes the power domain node for the
T-HEAD TH1520 SoC.

Signed-off-by: Michal Wilczynski <m.wilczynski@samsung.com>
---
 .../bindings/power/thead,th1520-power.yaml    | 42 +++++++++++++++++++
 MAINTAINERS                                   |  1 +
 .../dt-bindings/power/thead,th1520-power.h    | 18 ++++++++
 3 files changed, 61 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/power/thead,th1520-power.yaml
 create mode 100644 include/dt-bindings/power/thead,th1520-power.h

diff --git a/Documentation/devicetree/bindings/power/thead,th1520-power.yaml b/Documentation/devicetree/bindings/power/thead,th1520-power.yaml
new file mode 100644
index 000000000000..28ff86f2cb05
--- /dev/null
+++ b/Documentation/devicetree/bindings/power/thead,th1520-power.yaml
@@ -0,0 +1,42 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/power/thead,th1520-power.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: T-HEAD TH1520 Power Domain
+
+description: |
+  The T-HEAD TH1520 SoC includes a power domain controller responsible for
+  managing the power states of various hardware domains such as the GPU.
+
+maintainers:
+  - Michal Wilczynski <m.wilczynski@samsung.com>
+
+properties:
+  compatible:
+    const: thead,th1520-pd
+
+  "#power-domain-cells":
+    const: 1
+
+additionalProperties: false
+
+required:
+  - compatible
+  - "#power-domain-cells"
+
+examples:
+  - |
+    firmware {
+        aon: aon {
+            compatible = "thead,th1520-aon";
+            mboxes = <&mbox_910t 1>;
+            mbox-names = "aon";
+
+            pd: power-domain {
+                compatible = "thead,th1520-pd";
+                #power-domain-cells = <1>;
+            };
+        };
+    };
diff --git a/MAINTAINERS b/MAINTAINERS
index 910305c11e8a..c0e9639ac210 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -20192,6 +20192,7 @@ F:	Documentation/devicetree/bindings/clock/thead,th1520-clk-ap.yaml
 F:	Documentation/devicetree/bindings/mailbox/thead,th1520-mbox.yaml
 F:	Documentation/devicetree/bindings/net/thead,th1520-gmac.yaml
 F:	Documentation/devicetree/bindings/pinctrl/thead,th1520-pinctrl.yaml
+F:	Documentation/devicetree/bindings/power/thead,th1520-power.yaml
 F:	arch/riscv/boot/dts/thead/
 F:	drivers/clk/thead/clk-th1520-ap.c
 F:	drivers/mailbox/mailbox-th1520.c
diff --git a/include/dt-bindings/power/thead,th1520-power.h b/include/dt-bindings/power/thead,th1520-power.h
new file mode 100644
index 000000000000..45835f25010d
--- /dev/null
+++ b/include/dt-bindings/power/thead,th1520-power.h
@@ -0,0 +1,18 @@
+/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
+/*
+ * Copyright (C) 2022 Alibaba Group Holding Limited.
+ * Copyright (c) 2024 Samsung Electronics Co., Ltd.
+ * Author: Michal Wilczynski <m.wilczynski@samsung.com>
+ */
+
+#ifndef __DT_BINDINGS_POWER_TH1520_H
+#define __DT_BINDINGS_POWER_TH1520_H
+
+#define TH1520_AON_VDEC_PD	1
+#define TH1520_AON_NPU_PD	2
+#define TH1520_AON_VENC_PD	3
+#define TH1520_AON_GPU_PD	4
+#define TH1520_AON_DSP0_PD	5
+#define TH1520_AON_DSP1_PD	6
+
+#endif
-- 
2.34.1

Re: [RFC PATCH v2 03/19] dt-bindings: power: thead,th1520: Add support for power domains

[Rob Herring (Arm)]

On Mon, 23 Dec 2024 13:55:37 +0100, Michal Wilczynski wrote:
> Introduce a device tree binding for the T-HEAD TH1520 power domain
> controller. This allows devices to specify their power domain
> dependencies, thereby improving power management for subsystems such as
> the GPU. The new YAML schema describes the power domain node for the
> T-HEAD TH1520 SoC.
> 
> Signed-off-by: Michal Wilczynski <m.wilczynski@samsung.com>
> ---
>  .../bindings/power/thead,th1520-power.yaml    | 42 +++++++++++++++++++
>  MAINTAINERS                                   |  1 +
>  .../dt-bindings/power/thead,th1520-power.h    | 18 ++++++++
>  3 files changed, 61 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/power/thead,th1520-power.yaml
>  create mode 100644 include/dt-bindings/power/thead,th1520-power.h
> 

My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:

dtschema/dtc warnings/errors:
Documentation/devicetree/bindings/power/thead,th1520-power.example.dtb: /example-0/firmware/aon: failed to match any schema with compatible: ['thead,th1520-aon']

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20241223125553.3527812-4-m.wilczynski@samsung.com

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.

Re: [RFC PATCH v2 03/19] dt-bindings: power: thead,th1520: Add support for power domains

[Michal Wilczynski]

On 12/23/24 16:16, Rob Herring (Arm) wrote:
> 
> On Mon, 23 Dec 2024 13:55:37 +0100, Michal Wilczynski wrote:
>> Introduce a device tree binding for the T-HEAD TH1520 power domain
>> controller. This allows devices to specify their power domain
>> dependencies, thereby improving power management for subsystems such as
>> the GPU. The new YAML schema describes the power domain node for the
>> T-HEAD TH1520 SoC.
>>
>> Signed-off-by: Michal Wilczynski <m.wilczynski@samsung.com>
>> ---
>>  .../bindings/power/thead,th1520-power.yaml    | 42 +++++++++++++++++++
>>  MAINTAINERS                                   |  1 +
>>  .../dt-bindings/power/thead,th1520-power.h    | 18 ++++++++
>>  3 files changed, 61 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/power/thead,th1520-power.yaml
>>  create mode 100644 include/dt-bindings/power/thead,th1520-power.h
>>
> 
> My bot found errors running 'make dt_binding_check' on your patch:
> 
> yamllint warnings/errors:
> 
> dtschema/dtc warnings/errors:
> Documentation/devicetree/bindings/power/thead,th1520-power.example.dtb: /example-0/firmware/aon: failed to match any schema with compatible: ['thead,th1520-aon']

It is introduced in the next commit in this patch series.

Re: [RFC PATCH v2 03/19] dt-bindings: power: thead,th1520: Add support for power domains

[Krzysztof Kozlowski]

On 23/12/2024 17:02, Michal Wilczynski wrote:
>>>  .../bindings/power/thead,th1520-power.yaml    | 42 +++++++++++++++++++
>>>  MAINTAINERS                                   |  1 +
>>>  .../dt-bindings/power/thead,th1520-power.h    | 18 ++++++++
>>>  3 files changed, 61 insertions(+)
>>>  create mode 100644 Documentation/devicetree/bindings/power/thead,th1520-power.yaml
>>>  create mode 100644 include/dt-bindings/power/thead,th1520-power.h
>>>
>>
>> My bot found errors running 'make dt_binding_check' on your patch:
>>
>> yamllint warnings/errors:
>>
>> dtschema/dtc warnings/errors:
>> Documentation/devicetree/bindings/power/thead,th1520-power.example.dtb: /example-0/firmware/aon: failed to match any schema with compatible: ['thead,th1520-aon']
> 
> It is introduced in the next commit in this patch series.

Then your patchset is not bisectable. Reorder patches.

Best regards,
Krzysztof

Re: [RFC PATCH v2 03/19] dt-bindings: power: thead,th1520: Add support for power domains

[Krzysztof Kozlowski]

On 23/12/2024 13:55, Michal Wilczynski wrote:
> +  compatible:
> +    const: thead,th1520-pd
> +
> +  "#power-domain-cells":
> +    const: 1
> +
> +additionalProperties: false
> +
> +required:
> +  - compatible
> +  - "#power-domain-cells"
> +
> +examples:
> +  - |
> +    firmware {

Drop

> +        aon: aon {
> +            compatible = "thead,th1520-aon";
> +            mboxes = <&mbox_910t 1>;
> +            mbox-names = "aon";

Drop aon node... but the main problem is you do not have any resources
in your power-domain device node, assuming your binding is complete.
This suggests that this is part of aon, not separate device. Fold the
device node into its parent (so everything goes to AON).

> +
> +            pd: power-domain {
> +                compatible = "thead,th1520-pd";
> +                #power-domain-cells = <1>;
> +            };
> +        };
> +    };

Best regards,
Krzysztof

Re: [RFC PATCH v2 03/19] dt-bindings: power: thead,th1520: Add support for power domains

[Michal Wilczynski]

On 12/23/24 17:09, Krzysztof Kozlowski wrote:
> On 23/12/2024 13:55, Michal Wilczynski wrote:
>> +  compatible:
>> +    const: thead,th1520-pd
>> +
>> +  "#power-domain-cells":
>> +    const: 1
>> +
>> +additionalProperties: false
>> +
>> +required:
>> +  - compatible
>> +  - "#power-domain-cells"
>> +
>> +examples:
>> +  - |
>> +    firmware {
> 
> Drop
> 
>> +        aon: aon {
>> +            compatible = "thead,th1520-aon";
>> +            mboxes = <&mbox_910t 1>;
>> +            mbox-names = "aon";
> 
> Drop aon node... but the main problem is you do not have any resources
> in your power-domain device node, assuming your binding is complete.
> This suggests that this is part of aon, not separate device. Fold the
> device node into its parent (so everything goes to AON).

Merging everything to AON node would definitely work. I was looking at
the other implementations of firmware protocols for example, and that's
how I figured the current implementation:

arch/arm/boot/dts/broadcom/bcm2835-rpi.dtsi
soc {
	firmware: firmware {
		compatible = "raspberrypi,bcm2835-firmware", "simple-mfd";
		mboxes = <&mailbox>;

		firmware_clocks: clocks {
			compatible = "raspberrypi,firmware-clocks";
			#clock-cells = <1>;
		};
	};

	power: power {
		compatible = "raspberrypi,bcm2835-power";
		firmware = <&firmware>;
		#power-domain-cells = <1>;
	};
};

This is fairly similar, as the firmware is passed as property, instead
as in a parent-child relationship. Would you consider it more canonical
?

I would be happy to merge everything to AON node, and merge the
power-domain driver and AON driver together, but it seemed to me like
those could use some separation, and since power-domain and the AON
represent actual HW it's fine to represent them in the device tree.

Thanks,
Michał
> 
>> +
>> +            pd: power-domain {
>> +                compatible = "thead,th1520-pd";
>> +                #power-domain-cells = <1>;
>> +            };
>> +        };
>> +    };
> 
> Best regards,
> Krzysztof
> 

Re: [RFC PATCH v2 03/19] dt-bindings: power: thead,th1520: Add support for power domains

[Krzysztof Kozlowski]

On 24/12/2024 10:31, Michal Wilczynski wrote:
> 
> 
> On 12/23/24 17:09, Krzysztof Kozlowski wrote:
>> On 23/12/2024 13:55, Michal Wilczynski wrote:
>>> +  compatible:
>>> +    const: thead,th1520-pd
>>> +
>>> +  "#power-domain-cells":
>>> +    const: 1
>>> +
>>> +additionalProperties: false
>>> +
>>> +required:
>>> +  - compatible
>>> +  - "#power-domain-cells"
>>> +
>>> +examples:
>>> +  - |
>>> +    firmware {
>>
>> Drop
>>
>>> +        aon: aon {
>>> +            compatible = "thead,th1520-aon";
>>> +            mboxes = <&mbox_910t 1>;
>>> +            mbox-names = "aon";
>>
>> Drop aon node... but the main problem is you do not have any resources
>> in your power-domain device node, assuming your binding is complete.
>> This suggests that this is part of aon, not separate device. Fold the
>> device node into its parent (so everything goes to AON).
> 
> Merging everything to AON node would definitely work. I was looking at
> the other implementations of firmware protocols for example, and that's
> how I figured the current implementation:
> 
> arch/arm/boot/dts/broadcom/bcm2835-rpi.dtsi
> soc {
> 	firmware: firmware {
> 		compatible = "raspberrypi,bcm2835-firmware", "simple-mfd";
> 		mboxes = <&mailbox>;

That's a 10 year old binding. Not even reviewed initially by DT maintainers.

> 
> 		firmware_clocks: clocks {
> 			compatible = "raspberrypi,firmware-clocks";
> 			#clock-cells = <1>;
> 		};
> 	};
> 
> 	power: power {
> 		compatible = "raspberrypi,bcm2835-power";
> 		firmware = <&firmware>;
> 		#power-domain-cells = <1>;
> 	};
> };
> 
> This is fairly similar, as the firmware is passed as property, instead
> as in a parent-child relationship. Would you consider it more canonical
> ?

I consider early Raspberry bindings as antipattern. Same as many other
early approaches and that's not an accusation to their authors. Just
rules changed and capacity of DT maintainers increased.

> 
> I would be happy to merge everything to AON node, and merge the
> power-domain driver and AON driver together, but it seemed to me like
> those could use some separation, and since power-domain and the AON
> represent actual HW it's fine to represent them in the device tree.
> 

I said nothing about drivers and I did not suggest merging drivers.
Device nodes should not be split because they have different functions.
This is one single device with multiple functions. Just like hundreds of
others, e.g. clock, power domain and reset controller - all in one. Lack
of separate address space is the main aspect here indicating this is not
a separate device.

So merge the device nodes.



Best regards,
Krzysztof

[RFC PATCH v2 04/19] dt-bindings: firmware: thead,th1520: Add support for firmware node

[Michal Wilczynski]

The kernel communicates with the E902 core through the mailbox
transport using AON firmware protocol. Add dt-bindings to document it
the dt node.

Signed-off-by: Michal Wilczynski <m.wilczynski@samsung.com>
---
 .../bindings/firmware/thead,th1520-aon.yaml   | 59 +++++++++++++++++++
 MAINTAINERS                                   |  1 +
 2 files changed, 60 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/firmware/thead,th1520-aon.yaml

diff --git a/Documentation/devicetree/bindings/firmware/thead,th1520-aon.yaml b/Documentation/devicetree/bindings/firmware/thead,th1520-aon.yaml
new file mode 100644
index 000000000000..ca4c276766a3
--- /dev/null
+++ b/Documentation/devicetree/bindings/firmware/thead,th1520-aon.yaml
@@ -0,0 +1,59 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/firmware/thead,th1520-aon.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: T-HEAD TH1520 AON (Always-On) Firmware Node
+
+description: |
+  The Always-On (AON) subsystem in the TH1520 SoC is responsible for managing
+  low-power states, system wakeup events, and power management tasks. It is
+  designed to operate independently in a dedicated power domain, allowing it to
+  remain functional even during the SoC's deep sleep states.
+
+  At the heart of the AON subsystem is the E902, a low-power core that executes
+  firmware responsible for coordinating tasks such as power domain control,
+  clock management, and system wakeup signaling. Communication between the main
+  SoC and the AON subsystem is handled through a mailbox interface, which
+  enables message-based interactions with the AON firmware.
+
+maintainers:
+  - Michal Wilczynski <m.wilczynski@samsung.com>
+
+properties:
+  compatible:
+    const: thead,th1520-aon
+
+  mboxes:
+    maxItems: 1
+
+  mbox-names:
+    items:
+      - const: aon
+
+  power-domain:
+    $ref: /schemas/power/thead,th1520-power.yaml#
+    description: Subnode representing the hardware power domain of the AON subsystem.
+
+additionalProperties: false
+
+required:
+  - compatible
+  - mboxes
+  - mbox-names
+
+examples:
+  - |
+    firmware {
+        aon: aon {
+            compatible = "thead,th1520-aon";
+            mboxes = <&mbox_910t 1>;
+            mbox-names = "aon";
+
+            pd: power-domain {
+                compatible = "thead,th1520-pd";
+                #power-domain-cells = <1>;
+            };
+        };
+    };
diff --git a/MAINTAINERS b/MAINTAINERS
index c0e9639ac210..7705d1b6dd7a 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -20188,6 +20188,7 @@ M:	Fu Wei <wefu@redhat.com>
 L:	linux-riscv@lists.infradead.org
 S:	Maintained
 T:	git https://github.com/pdp7/linux.git
+F:	Documentation/devicetree/bindings/firmware/thead,th1520-aon.yaml
 F:	Documentation/devicetree/bindings/clock/thead,th1520-clk-ap.yaml
 F:	Documentation/devicetree/bindings/mailbox/thead,th1520-mbox.yaml
 F:	Documentation/devicetree/bindings/net/thead,th1520-gmac.yaml
-- 
2.34.1

Re: [RFC PATCH v2 04/19] dt-bindings: firmware: thead,th1520: Add support for firmware node

[Krzysztof Kozlowski]

On 23/12/2024 13:55, Michal Wilczynski wrote:
> The kernel communicates with the E902 core through the mailbox
> transport using AON firmware protocol. Add dt-bindings to document it
> the dt node.
> 
> Signed-off-by: Michal Wilczynski <m.wilczynski@samsung.com>
> ---
>  .../bindings/firmware/thead,th1520-aon.yaml   | 59 +++++++++++++++++++
>  MAINTAINERS                                   |  1 +
>  2 files changed, 60 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/firmware/thead,th1520-aon.yaml
> 
> diff --git a/Documentation/devicetree/bindings/firmware/thead,th1520-aon.yaml b/Documentation/devicetree/bindings/firmware/thead,th1520-aon.yaml
> new file mode 100644
> index 000000000000..ca4c276766a3
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/firmware/thead,th1520-aon.yaml
> @@ -0,0 +1,59 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/firmware/thead,th1520-aon.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: T-HEAD TH1520 AON (Always-On) Firmware Node

Drop "Node", unless this is somehow name of device (not a DT node).

> +
> +description: |
> +  The Always-On (AON) subsystem in the TH1520 SoC is responsible for managing
> +  low-power states, system wakeup events, and power management tasks. It is
> +  designed to operate independently in a dedicated power domain, allowing it to
> +  remain functional even during the SoC's deep sleep states.
> +
> +  At the heart of the AON subsystem is the E902, a low-power core that executes
> +  firmware responsible for coordinating tasks such as power domain control,
> +  clock management, and system wakeup signaling. Communication between the main
> +  SoC and the AON subsystem is handled through a mailbox interface, which
> +  enables message-based interactions with the AON firmware.
> +
> +maintainers:
> +  - Michal Wilczynski <m.wilczynski@samsung.com>
> +
> +properties:
> +  compatible:
> +    const: thead,th1520-aon
> +
> +  mboxes:
> +    maxItems: 1
> +
> +  mbox-names:
> +    items:
> +      - const: aon
> +
> +  power-domain:
> +    $ref: /schemas/power/thead,th1520-power.yaml#
> +    description: Subnode representing the hardware power domain of the AON subsystem.
> +
> +additionalProperties: false
> +
> +required:
> +  - compatible
> +  - mboxes
> +  - mbox-names

In all your bindings patches: "required" block goes before
additional/unevaluatedProperties. See also example-schema.

> +
> +examples:
> +  - |
> +    firmware {
> +        aon: aon {

Best regards,
Krzysztof

[RFC PATCH v2 05/19] firmware: thead: Add AON firmware protocol driver

[Michal Wilczynski]

The T-Head TH1520 SoC uses an E902 co-processor running Always-On (AON)
firmware to manage power, clock, and other system resources [1]. This
patch introduces a driver implementing the AON firmware protocol,
allowing the Linux kernel to communicate with the firmware via mailbox
channels.  Through an RPC-based interface, the kernel can initiate power
state transitions, update resource configurations, and perform other
AON-related tasks.

Link: https://openbeagle.org/beaglev-ahead/beaglev-ahead/-/blob/main/docs/TH1520%20System%20User%20Manual.pdf [1]

Signed-off-by: Michal Wilczynski <m.wilczynski@samsung.com>
---
 MAINTAINERS                                   |   2 +
 drivers/firmware/Kconfig                      |   9 +
 drivers/firmware/Makefile                     |   1 +
 drivers/firmware/thead,th1520-aon.c           | 203 ++++++++++++++++++
 .../linux/firmware/thead/thead,th1520-aon.h   | 186 ++++++++++++++++
 5 files changed, 401 insertions(+)
 create mode 100644 drivers/firmware/thead,th1520-aon.c
 create mode 100644 include/linux/firmware/thead/thead,th1520-aon.h

diff --git a/MAINTAINERS b/MAINTAINERS
index 7705d1b6dd7a..42aef66bd257 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -20196,10 +20196,12 @@ F:	Documentation/devicetree/bindings/pinctrl/thead,th1520-pinctrl.yaml
 F:	Documentation/devicetree/bindings/power/thead,th1520-power.yaml
 F:	arch/riscv/boot/dts/thead/
 F:	drivers/clk/thead/clk-th1520-ap.c
+F:	drivers/firmware/thead,th1520-aon.c
 F:	drivers/mailbox/mailbox-th1520.c
 F:	drivers/net/ethernet/stmicro/stmmac/dwmac-thead.c
 F:	drivers/pinctrl/pinctrl-th1520.c
 F:	include/dt-bindings/clock/thead,th1520-clk-ap.h
+F:	include/linux/firmware/thead/thead,th1520-aon.h
 
 RNBD BLOCK DRIVERS
 M:	Md. Haris Iqbal <haris.iqbal@ionos.com>
diff --git a/drivers/firmware/Kconfig b/drivers/firmware/Kconfig
index 71d8b26c4103..e08e01de3ee3 100644
--- a/drivers/firmware/Kconfig
+++ b/drivers/firmware/Kconfig
@@ -212,6 +212,15 @@ config SYSFB_SIMPLEFB
 
 	  If unsure, say Y.
 
+config TH1520_AON_PROTOCOL
+	tristate "Always-On firmware protocol"
+	depends on THEAD_TH1520_MBOX
+	help
+	  Power, clock, and resource management capabilities on the TH1520 SoC are
+	  managed by the E902 core. Firmware running on this core communicates with
+	  the kernel through the Always-On protocol, using hardware mailbox as a medium.
+	  Say yes if you need such capabilities.
+
 config TI_SCI_PROTOCOL
 	tristate "TI System Control Interface (TISCI) Message Protocol"
 	depends on TI_MESSAGE_MANAGER
diff --git a/drivers/firmware/Makefile b/drivers/firmware/Makefile
index 7a8d486e718f..5db9c042430c 100644
--- a/drivers/firmware/Makefile
+++ b/drivers/firmware/Makefile
@@ -18,6 +18,7 @@ obj-$(CONFIG_RASPBERRYPI_FIRMWARE) += raspberrypi.o
 obj-$(CONFIG_FW_CFG_SYSFS)	+= qemu_fw_cfg.o
 obj-$(CONFIG_SYSFB)		+= sysfb.o
 obj-$(CONFIG_SYSFB_SIMPLEFB)	+= sysfb_simplefb.o
+obj-$(CONFIG_TH1520_AON_PROTOCOL) += thead,th1520-aon.o
 obj-$(CONFIG_TI_SCI_PROTOCOL)	+= ti_sci.o
 obj-$(CONFIG_TRUSTED_FOUNDATIONS) += trusted_foundations.o
 obj-$(CONFIG_TURRIS_MOX_RWTM)	+= turris-mox-rwtm.o
diff --git a/drivers/firmware/thead,th1520-aon.c b/drivers/firmware/thead,th1520-aon.c
new file mode 100644
index 000000000000..82198bcfc3a8
--- /dev/null
+++ b/drivers/firmware/thead,th1520-aon.c
@@ -0,0 +1,203 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2021 Alibaba Group Holding Limited.
+ * Copyright (c) 2024 Samsung Electronics Co., Ltd.
+ * Author: Michal Wilczynski <m.wilczynski@samsung.com>
+ */
+
+#include <linux/mailbox_client.h>
+#include <linux/mailbox_controller.h>
+#include <linux/of.h>
+#include <linux/of_platform.h>
+#include <linux/platform_device.h>
+#include <linux/firmware/thead/thead,th1520-aon.h>
+
+#define MAX_RX_TIMEOUT (msecs_to_jiffies(3000))
+#define MAX_TX_TIMEOUT 500
+
+struct th1520_aon_chan {
+	struct mbox_chan *ch;
+	struct th1520_aon_rpc_ack_common ack_msg;
+	struct mbox_client cl;
+	struct completion done;
+
+	/* make sure only one RPC is perfomed at a time */
+	struct mutex transaction_lock;
+};
+
+struct th1520_aon_msg_req_set_resource_power_mode {
+	struct th1520_aon_rpc_msg_hdr hdr;
+	u16 resource;
+	u16 mode;
+	u16 reserved[10];
+} __packed __aligned(1);
+
+/*
+ * This type is used to indicate error response for most functions.
+ */
+enum th1520_aon_error_codes {
+	LIGHT_AON_ERR_NONE = 0, /* Success */
+	LIGHT_AON_ERR_VERSION = 1, /* Incompatible API version */
+	LIGHT_AON_ERR_CONFIG = 2, /* Configuration error */
+	LIGHT_AON_ERR_PARM = 3, /* Bad parameter */
+	LIGHT_AON_ERR_NOACCESS = 4, /* Permission error (no access) */
+	LIGHT_AON_ERR_LOCKED = 5, /* Permission error (locked) */
+	LIGHT_AON_ERR_UNAVAILABLE = 6, /* Unavailable (out of resources) */
+	LIGHT_AON_ERR_NOTFOUND = 7, /* Not found */
+	LIGHT_AON_ERR_NOPOWER = 8, /* No power */
+	LIGHT_AON_ERR_IPC = 9, /* Generic IPC error */
+	LIGHT_AON_ERR_BUSY = 10, /* Resource is currently busy/active */
+	LIGHT_AON_ERR_FAIL = 11, /* General I/O failure */
+	LIGHT_AON_ERR_LAST
+};
+
+static int th1520_aon_linux_errmap[LIGHT_AON_ERR_LAST] = {
+	0, /* LIGHT_AON_ERR_NONE */
+	-EINVAL, /* LIGHT_AON_ERR_VERSION */
+	-EINVAL, /* LIGHT_AON_ERR_CONFIG */
+	-EINVAL, /* LIGHT_AON_ERR_PARM */
+	-EACCES, /* LIGHT_AON_ERR_NOACCESS */
+	-EACCES, /* LIGHT_AON_ERR_LOCKED */
+	-ERANGE, /* LIGHT_AON_ERR_UNAVAILABLE */
+	-EEXIST, /* LIGHT_AON_ERR_NOTFOUND */
+	-EPERM, /* LIGHT_AON_ERR_NOPOWER */
+	-EPIPE, /* LIGHT_AON_ERR_IPC */
+	-EBUSY, /* LIGHT_AON_ERR_BUSY */
+	-EIO, /* LIGHT_AON_ERR_FAIL */
+};
+
+static inline int th1520_aon_to_linux_errno(int errno)
+{
+	if (errno >= LIGHT_AON_ERR_NONE && errno < LIGHT_AON_ERR_LAST)
+		return th1520_aon_linux_errmap[errno];
+
+	return -EIO;
+}
+
+static void th1520_aon_rx_callback(struct mbox_client *c, void *rx_msg)
+{
+	struct th1520_aon_chan *aon_chan =
+		container_of(c, struct th1520_aon_chan, cl);
+	struct th1520_aon_rpc_msg_hdr *hdr =
+		(struct th1520_aon_rpc_msg_hdr *)rx_msg;
+	u8 recv_size = sizeof(struct th1520_aon_rpc_msg_hdr) + hdr->size;
+
+	if (recv_size != sizeof(struct th1520_aon_rpc_ack_common)) {
+		dev_err(c->dev, "Invalid ack size, not completing\n");
+		return;
+	}
+
+	memcpy(&aon_chan->ack_msg, rx_msg, recv_size);
+	complete(&aon_chan->done);
+}
+
+int th1520_aon_call_rpc(struct th1520_aon_chan *aon_chan, void *msg)
+{
+	struct th1520_aon_rpc_msg_hdr *hdr = msg;
+	int ret;
+
+	mutex_lock(&aon_chan->transaction_lock);
+	reinit_completion(&aon_chan->done);
+
+	RPC_SET_VER(hdr, TH1520_AON_RPC_VERSION);
+	RPC_SET_SVC_ID(hdr, hdr->svc);
+	RPC_SET_SVC_FLAG_MSG_TYPE(hdr, RPC_SVC_MSG_TYPE_DATA);
+	RPC_SET_SVC_FLAG_ACK_TYPE(hdr, RPC_SVC_MSG_NEED_ACK);
+
+	ret = mbox_send_message(aon_chan->ch, msg);
+	if (ret < 0) {
+		dev_err(aon_chan->cl.dev, "RPC send msg failed: %d\n", ret);
+		goto out;
+	}
+
+	if (!wait_for_completion_timeout(&aon_chan->done, MAX_RX_TIMEOUT)) {
+		dev_err(aon_chan->cl.dev, "RPC send msg timeout\n");
+		mutex_unlock(&aon_chan->transaction_lock);
+		return -ETIMEDOUT;
+	}
+
+	ret = aon_chan->ack_msg.err_code;
+
+out:
+	mutex_unlock(&aon_chan->transaction_lock);
+
+	return th1520_aon_to_linux_errno(ret);
+}
+EXPORT_SYMBOL_GPL(th1520_aon_call_rpc);
+
+int th1520_aon_power_update(struct th1520_aon_chan *aon_chan, u16 rsrc,
+			    bool power_on)
+{
+	struct th1520_aon_msg_req_set_resource_power_mode msg = {};
+	struct th1520_aon_rpc_msg_hdr *hdr = &msg.hdr;
+	int ret;
+
+	hdr->svc = TH1520_AON_RPC_SVC_PM;
+	hdr->func = TH1520_AON_PM_FUNC_SET_RESOURCE_POWER_MODE;
+	hdr->size = TH1520_AON_RPC_MSG_NUM;
+
+	RPC_SET_BE16(&msg.resource, 0, rsrc);
+	RPC_SET_BE16(&msg.resource, 2,
+		     (power_on ? TH1520_AON_PM_PW_MODE_ON :
+				 TH1520_AON_PM_PW_MODE_OFF));
+
+	ret = th1520_aon_call_rpc(aon_chan, &msg);
+	if (ret)
+		dev_err(aon_chan->cl.dev, "failed to power %s resource %d ret %d\n",
+			power_on ? "up" : "off", rsrc, ret);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(th1520_aon_power_update);
+
+static int th1520_aon_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct th1520_aon_chan *aon_chan;
+	struct mbox_client *cl;
+	int ret;
+
+	aon_chan = devm_kzalloc(dev, sizeof(*aon_chan), GFP_KERNEL);
+	if (!aon_chan)
+		return -ENOMEM;
+
+	cl = &aon_chan->cl;
+	cl->dev = dev;
+	cl->tx_block = true;
+	cl->tx_tout = MAX_TX_TIMEOUT;
+	cl->rx_callback = th1520_aon_rx_callback;
+
+	aon_chan->ch = mbox_request_channel_byname(cl, "aon");
+	if (IS_ERR(aon_chan->ch)) {
+		ret = PTR_ERR(aon_chan->ch);
+		if (ret != -EPROBE_DEFER)
+			dev_err(dev, "Failed to request aon mbox chan ret %d\n",
+				ret);
+		return ret;
+	}
+
+	mutex_init(&aon_chan->transaction_lock);
+	init_completion(&aon_chan->done);
+
+	platform_set_drvdata(pdev, aon_chan);
+
+	return devm_of_platform_populate(dev);
+}
+
+static const struct of_device_id th1520_aon_match[] = {
+	{ .compatible = "thead,th1520-aon" },
+	{ /* Sentinel */ }
+};
+
+static struct platform_driver th1520_aon_driver = {
+	.driver = {
+		.name = "th1520-aon",
+		.of_match_table = th1520_aon_match,
+	},
+	.probe = th1520_aon_probe,
+};
+module_platform_driver(th1520_aon_driver);
+
+MODULE_AUTHOR("Michal Wilczynski <m.wilczynski@samsung.com>");
+MODULE_DESCRIPTION("T-HEAD TH1520 Always-On firmware driver");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/firmware/thead/thead,th1520-aon.h b/include/linux/firmware/thead/thead,th1520-aon.h
new file mode 100644
index 000000000000..3daa17c01d17
--- /dev/null
+++ b/include/linux/firmware/thead/thead,th1520-aon.h
@@ -0,0 +1,186 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) 2021 Alibaba Group Holding Limited.
+ */
+
+#ifndef _THEAD_AON_H
+#define _THEAD_AON_H
+
+#include <linux/device.h>
+#include <linux/types.h>
+
+#define AON_RPC_MSG_MAGIC (0xef)
+#define TH1520_AON_RPC_VERSION 2
+#define TH1520_AON_RPC_MSG_NUM 7
+
+extern struct th1520_aon_chan *aon_chan;
+
+enum th1520_aon_rpc_svc {
+	TH1520_AON_RPC_SVC_UNKNOWN = 0,
+	TH1520_AON_RPC_SVC_PM = 1,
+	TH1520_AON_RPC_SVC_MISC = 2,
+	TH1520_AON_RPC_SVC_AVFS = 3,
+	TH1520_AON_RPC_SVC_SYS = 4,
+	TH1520_AON_RPC_SVC_WDG = 5,
+	TH1520_AON_RPC_SVC_LPM = 6,
+	TH1520_AON_RPC_SVC_MAX = 0x3F,
+};
+
+enum th1520_aon_misc_func {
+	TH1520_AON_MISC_FUNC_UNKNOWN = 0,
+	TH1520_AON_MISC_FUNC_SET_CONTROL = 1,
+	TH1520_AON_MISC_FUNC_GET_CONTROL = 2,
+	TH1520_AON_MISC_FUNC_REGDUMP_CFG = 3,
+};
+
+enum th1520_aon_wdg_func {
+	TH1520_AON_WDG_FUNC_UNKNOWN = 0,
+	TH1520_AON_WDG_FUNC_START = 1,
+	TH1520_AON_WDG_FUNC_STOP = 2,
+	TH1520_AON_WDG_FUNC_PING = 3,
+	TH1520_AON_WDG_FUNC_TIMEOUTSET = 4,
+	TH1520_AON_WDG_FUNC_RESTART = 5,
+	TH1520_AON_WDG_FUNC_GET_STATE = 6,
+	TH1520_AON_WDG_FUNC_POWER_OFF = 7,
+	TH1520_AON_WDG_FUNC_AON_WDT_ON = 8,
+	TH1520_AON_WDG_FUNC_AON_WDT_OFF = 9,
+};
+
+enum th1520_aon_sys_func {
+	TH1520_AON_SYS_FUNC_UNKNOWN = 0,
+	TH1520_AON_SYS_FUNC_AON_RESERVE_MEM = 1,
+};
+
+enum th1520_aon_lpm_func {
+	TH1520_AON_LPM_FUNC_UNKNOWN = 0,
+	TH1520_AON_LPM_FUNC_REQUIRE_STR = 1,
+	TH1520_AON_LPM_FUNC_RESUME_STR = 2,
+	TH1520_AON_LPM_FUNC_REQUIRE_STD = 3,
+	TH1520_AON_LPM_FUNC_CPUHP = 4,
+	TH1520_AON_LPM_FUNC_REGDUMP_CFG = 5,
+};
+
+enum th1520_aon_pm_func {
+	TH1520_AON_PM_FUNC_UNKNOWN = 0,
+	TH1520_AON_PM_FUNC_SET_RESOURCE_REGULATOR = 1,
+	TH1520_AON_PM_FUNC_GET_RESOURCE_REGULATOR = 2,
+	TH1520_AON_PM_FUNC_SET_RESOURCE_POWER_MODE = 3,
+	TH1520_AON_PM_FUNC_PWR_SET = 4,
+	TH1520_AON_PM_FUNC_PWR_GET = 5,
+	TH1520_AON_PM_FUNC_CHECK_FAULT = 6,
+	TH1520_AON_PM_FUNC_GET_TEMPERATURE = 7,
+};
+
+struct th1520_aon_rpc_msg_hdr {
+	u8 ver; /* version of msg hdr */
+	u8 size; /*  msg size ,uinit in bytes,the size includes rpc msg header self */
+	u8 svc; /* rpc main service id */
+	u8 func; /* rpc sub func id of specific service, sent by caller */
+} __packed __aligned(1);
+
+struct th1520_aon_rpc_ack_common {
+	struct th1520_aon_rpc_msg_hdr hdr;
+	u8 err_code;
+} __packed __aligned(1);
+
+#define RPC_SVC_MSG_TYPE_DATA 0
+#define RPC_SVC_MSG_TYPE_ACK 1
+#define RPC_SVC_MSG_NEED_ACK 0
+#define RPC_SVC_MSG_NO_NEED_ACK 1
+
+#define RPC_GET_VER(MESG) ((MESG)->ver)
+#define RPC_SET_VER(MESG, VER) ((MESG)->ver = (VER))
+#define RPC_GET_SVC_ID(MESG) ((MESG)->svc & 0x3F)
+#define RPC_SET_SVC_ID(MESG, ID) ((MESG)->svc |= 0x3F & (ID))
+#define RPC_GET_SVC_FLAG_MSG_TYPE(MESG) (((MESG)->svc & 0x80) >> 7)
+#define RPC_SET_SVC_FLAG_MSG_TYPE(MESG, TYPE) ((MESG)->svc |= (TYPE) << 7)
+#define RPC_GET_SVC_FLAG_ACK_TYPE(MESG) (((MESG)->svc & 0x40) >> 6)
+#define RPC_SET_SVC_FLAG_ACK_TYPE(MESG, ACK) ((MESG)->svc |= (ACK) << 6)
+
+#define RPC_SET_BE64(MESG, OFFSET, SET_DATA)                                \
+	do {                                                                \
+		u8 *data = (u8 *)(MESG);                                    \
+		u64 _offset = (OFFSET);                                     \
+		u64 _set_data = (SET_DATA);                                 \
+		data[_offset + 7] = _set_data & 0xFF;                       \
+		data[_offset + 6] = (_set_data & 0xFF00) >> 8;              \
+		data[_offset + 5] = (_set_data & 0xFF0000) >> 16;           \
+		data[_offset + 4] = (_set_data & 0xFF000000) >> 24;         \
+		data[_offset + 3] = (_set_data & 0xFF00000000) >> 32;       \
+		data[_offset + 2] = (_set_data & 0xFF0000000000) >> 40;     \
+		data[_offset + 1] = (_set_data & 0xFF000000000000) >> 48;   \
+		data[_offset + 0] = (_set_data & 0xFF00000000000000) >> 56; \
+	} while (0)
+
+#define RPC_SET_BE32(MESG, OFFSET, SET_DATA)			    \
+	do {							    \
+		u8 *data = (u8 *)(MESG);			    \
+		u64 _offset = (OFFSET);				    \
+		u64 _set_data = (SET_DATA);			    \
+		data[_offset + 3] = (_set_data) & 0xFF;		    \
+		data[_offset + 2] = (_set_data & 0xFF00) >> 8;	    \
+		data[_offset + 1] = (_set_data & 0xFF0000) >> 16;   \
+		data[_offset + 0] = (_set_data & 0xFF000000) >> 24; \
+	} while (0)
+
+#define RPC_SET_BE16(MESG, OFFSET, SET_DATA)		       \
+	do {						       \
+		u8 *data = (u8 *)(MESG);		       \
+		u64 _offset = (OFFSET);			       \
+		u64 _set_data = (SET_DATA);		       \
+		data[_offset + 1] = (_set_data) & 0xFF;	       \
+		data[_offset + 0] = (_set_data & 0xFF00) >> 8; \
+	} while (0)
+
+#define RPC_SET_U8(MESG, OFFSET, SET_DATA)	  \
+	do {					  \
+		u8 *data = (u8 *)(MESG);	  \
+		data[OFFSET] = (SET_DATA) & 0xFF; \
+	} while (0)
+
+#define RPC_GET_BE64(MESG, OFFSET, PTR)                                      \
+	do {                                                                 \
+		u8 *data = (u8 *)(MESG);                                     \
+		u64 _offset = (OFFSET);                                      \
+		*(u32 *)(PTR) =                                              \
+			(data[_offset + 7] | data[_offset + 6] << 8 |        \
+			 data[_offset + 5] << 16 | data[_offset + 4] << 24 | \
+			 data[_offset + 3] << 32 | data[_offset + 2] << 40 | \
+			 data[_offset + 1] << 48 | data[_offset + 0] << 56); \
+	} while (0)
+
+#define RPC_GET_BE32(MESG, OFFSET, PTR)                                      \
+	do {                                                                 \
+		u8 *data = (u8 *)(MESG);                                     \
+		u64 _offset = (OFFSET);                                      \
+		*(u32 *)(PTR) =                                              \
+			(data[_offset + 3] | data[_offset + 2] << 8 |        \
+			 data[_offset + 1] << 16 | data[_offset + 0] << 24); \
+	} while (0)
+
+#define RPC_GET_BE16(MESG, OFFSET, PTR)                                       \
+	do {                                                                  \
+		u8 *data = (u8 *)(MESG);                                      \
+		u64 _offset = (OFFSET);                                       \
+		*(u16 *)(PTR) = (data[_offset + 1] | data[_offset + 0] << 8); \
+	} while (0)
+
+#define RPC_GET_U8(MESG, OFFSET, PTR)          \
+	do {                                   \
+		u8 *data = (u8 *)(MESG);       \
+		*(u8 *)(PTR) = (data[OFFSET]); \
+	} while (0)
+
+/*
+ * Defines for SC PM Power Mode
+ */
+#define TH1520_AON_PM_PW_MODE_OFF 0 /* Power off */
+#define TH1520_AON_PM_PW_MODE_STBY 1 /* Power in standby */
+#define TH1520_AON_PM_PW_MODE_LP 2 /* Power in low-power */
+#define TH1520_AON_PM_PW_MODE_ON 3 /* Power on */
+
+int th1520_aon_call_rpc(struct th1520_aon_chan *aon_chan, void *msg);
+int th1520_aon_power_update(struct th1520_aon_chan *aon_chan, u16 rsrc,
+			    bool power_on);
+
+#endif /* _THEAD_AON_H */
-- 
2.34.1

Re: [RFC PATCH v2 05/19] firmware: thead: Add AON firmware protocol driver

[Krzysztof Kozlowski]

On 23/12/2024 13:55, Michal Wilczynski wrote:
> The T-Head TH1520 SoC uses an E902 co-processor running Always-On (AON)
> firmware to manage power, clock, and other system resources [1]. This
> patch introduces a driver implementing the AON firmware protocol,
> allowing the Linux kernel to communicate with the firmware via mailbox
> channels.  Through an RPC-based interface, the kernel can initiate power
> state transitions, update resource configurations, and perform other
> AON-related tasks.
> 
> Link: https://openbeagle.org/beaglev-ahead/beaglev-ahead/-/blob/main/docs/TH1520%20System%20User%20Manual.pdf [1]
> 
> Signed-off-by: Michal Wilczynski <m.wilczynski@samsung.com>
> ---
>  MAINTAINERS                                   |   2 +
>  drivers/firmware/Kconfig                      |   9 +
>  drivers/firmware/Makefile                     |   1 +
>  drivers/firmware/thead,th1520-aon.c           | 203 ++++++++++++++++++
>  .../linux/firmware/thead/thead,th1520-aon.h   | 186 ++++++++++++++++
>  5 files changed, 401 insertions(+)
>  create mode 100644 drivers/firmware/thead,th1520-aon.c
>  create mode 100644 include/linux/firmware/thead/thead,th1520-aon.h
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 7705d1b6dd7a..42aef66bd257 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -20196,10 +20196,12 @@ F:	Documentation/devicetree/bindings/pinctrl/thead,th1520-pinctrl.yaml
>  F:	Documentation/devicetree/bindings/power/thead,th1520-power.yaml
>  F:	arch/riscv/boot/dts/thead/
>  F:	drivers/clk/thead/clk-th1520-ap.c
> +F:	drivers/firmware/thead,th1520-aon.c
>  F:	drivers/mailbox/mailbox-th1520.c
>  F:	drivers/net/ethernet/stmicro/stmmac/dwmac-thead.c
>  F:	drivers/pinctrl/pinctrl-th1520.c
>  F:	include/dt-bindings/clock/thead,th1520-clk-ap.h
> +F:	include/linux/firmware/thead/thead,th1520-aon.h
>  
>  RNBD BLOCK DRIVERS
>  M:	Md. Haris Iqbal <haris.iqbal@ionos.com>
> diff --git a/drivers/firmware/Kconfig b/drivers/firmware/Kconfig
> index 71d8b26c4103..e08e01de3ee3 100644
> --- a/drivers/firmware/Kconfig
> +++ b/drivers/firmware/Kconfig
> @@ -212,6 +212,15 @@ config SYSFB_SIMPLEFB
>  
>  	  If unsure, say Y.
>  
> +config TH1520_AON_PROTOCOL
> +	tristate "Always-On firmware protocol"
> +	depends on THEAD_TH1520_MBOX

Would:
	|| COMPILE_TEST
work?

What sort of dependency is this? Build time? Runtime? If runtime, this
should be just dependency on ARCH_THEAD (or whatever is there)

> +	help
> +	  Power, clock, and resource management capabilities on the TH1520 SoC are
> +	  managed by the E902 core. Firmware running on this core communicates with
> +	  the kernel through the Always-On protocol, using hardware mailbox as a medium.
> +	  Say yes if you need such capabilities.
> +


...

> +static void th1520_aon_rx_callback(struct mbox_client *c, void *rx_msg)
> +{
> +	struct th1520_aon_chan *aon_chan =
> +		container_of(c, struct th1520_aon_chan, cl);
> +	struct th1520_aon_rpc_msg_hdr *hdr =
> +		(struct th1520_aon_rpc_msg_hdr *)rx_msg;
> +	u8 recv_size = sizeof(struct th1520_aon_rpc_msg_hdr) + hdr->size;
> +
> +	if (recv_size != sizeof(struct th1520_aon_rpc_ack_common)) {
> +		dev_err(c->dev, "Invalid ack size, not completing\n");
> +		return;
> +	}
> +
> +	memcpy(&aon_chan->ack_msg, rx_msg, recv_size);
> +	complete(&aon_chan->done);
> +}
> +

You need proper (and useful) kerneldoc for all exported functions.

> +int th1520_aon_call_rpc(struct th1520_aon_chan *aon_chan, void *msg)
> +{
> +	struct th1520_aon_rpc_msg_hdr *hdr = msg;
> +	int ret;
> +
> +	mutex_lock(&aon_chan->transaction_lock);
> +	reinit_completion(&aon_chan->done);
> +
> +	RPC_SET_VER(hdr, TH1520_AON_RPC_VERSION);
> +	RPC_SET_SVC_ID(hdr, hdr->svc);
> +	RPC_SET_SVC_FLAG_MSG_TYPE(hdr, RPC_SVC_MSG_TYPE_DATA);
> +	RPC_SET_SVC_FLAG_ACK_TYPE(hdr, RPC_SVC_MSG_NEED_ACK);
> +
> +	ret = mbox_send_message(aon_chan->ch, msg);
> +	if (ret < 0) {
> +		dev_err(aon_chan->cl.dev, "RPC send msg failed: %d\n", ret);
> +		goto out;
> +	}
> +
> +	if (!wait_for_completion_timeout(&aon_chan->done, MAX_RX_TIMEOUT)) {
> +		dev_err(aon_chan->cl.dev, "RPC send msg timeout\n");
> +		mutex_unlock(&aon_chan->transaction_lock);
> +		return -ETIMEDOUT;
> +	}
> +
> +	ret = aon_chan->ack_msg.err_code;
> +
> +out:
> +	mutex_unlock(&aon_chan->transaction_lock);
> +
> +	return th1520_aon_to_linux_errno(ret);
> +}
> +EXPORT_SYMBOL_GPL(th1520_aon_call_rpc);
> +

Here as well.

> +int th1520_aon_power_update(struct th1520_aon_chan *aon_chan, u16 rsrc,
> +			    bool power_on)
> +{
> +	struct th1520_aon_msg_req_set_resource_power_mode msg = {};
> +	struct th1520_aon_rpc_msg_hdr *hdr = &msg.hdr;
> +	int ret;
> +
> +	hdr->svc = TH1520_AON_RPC_SVC_PM;
> +	hdr->func = TH1520_AON_PM_FUNC_SET_RESOURCE_POWER_MODE;
> +	hdr->size = TH1520_AON_RPC_MSG_NUM;
> +
> +	RPC_SET_BE16(&msg.resource, 0, rsrc);
> +	RPC_SET_BE16(&msg.resource, 2,
> +		     (power_on ? TH1520_AON_PM_PW_MODE_ON :
> +				 TH1520_AON_PM_PW_MODE_OFF));
> +
> +	ret = th1520_aon_call_rpc(aon_chan, &msg);
> +	if (ret)
> +		dev_err(aon_chan->cl.dev, "failed to power %s resource %d ret %d\n",
> +			power_on ? "up" : "off", rsrc, ret);
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(th1520_aon_power_update);
> +
> +static int th1520_aon_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct th1520_aon_chan *aon_chan;
> +	struct mbox_client *cl;
> +	int ret;
> +
> +	aon_chan = devm_kzalloc(dev, sizeof(*aon_chan), GFP_KERNEL);
> +	if (!aon_chan)
> +		return -ENOMEM;
> +
> +	cl = &aon_chan->cl;
> +	cl->dev = dev;
> +	cl->tx_block = true;
> +	cl->tx_tout = MAX_TX_TIMEOUT;
> +	cl->rx_callback = th1520_aon_rx_callback;
> +
> +	aon_chan->ch = mbox_request_channel_byname(cl, "aon");
> +	if (IS_ERR(aon_chan->ch)) {
> +		ret = PTR_ERR(aon_chan->ch);
> +		if (ret != -EPROBE_DEFER)
> +			dev_err(dev, "Failed to request aon mbox chan ret %d\n",
> +				ret);

You just open-coded dev_err_probe. Syntax is:

return dev_err_probe()

> +		return ret;
> +	}
> +
> +	mutex_init(&aon_chan->transaction_lock);
> +	init_completion(&aon_chan->done);
> +
> +	platform_set_drvdata(pdev, aon_chan);
> +
> +	return devm_of_platform_populate(dev);
> +}
> +

No remove() callback to free mbox channel? Looks like a leak.



Best regards,
Krzysztof

[RFC PATCH v2 06/19] soc: thead: power-domain: Add power-domain driver for TH1520

[Michal Wilczynski]

The T-Head TH1520 SoC contains multiple power islands that can be
programmatically turned on and off using the AON (Always-On) protocol
and a hardware mailbox [1]. The relevant mailbox driver has already been
merged into the mainline kernel in commit 5d4d263e1c6b ("mailbox:
Introduce support for T-head TH1520 Mailbox driver");

This commit introduces a power-domain driver for the TH1520 SoC, which
is using AON firmware protocol to communicate with E902 core through the
hardware mailbox. This way it can send power on/off commands to the E902
core.

Link: https://openbeagle.org/beaglev-ahead/beaglev-ahead/-/blob/main/docs/TH1520%20System%20User%20Manual.pdf [1]

Signed-off-by: Michal Wilczynski <m.wilczynski@samsung.com>
---
 MAINTAINERS                                |   1 +
 drivers/pmdomain/Kconfig                   |   1 +
 drivers/pmdomain/Makefile                  |   1 +
 drivers/pmdomain/thead/Kconfig             |  12 ++
 drivers/pmdomain/thead/Makefile            |   2 +
 drivers/pmdomain/thead/th1520-pm-domains.c | 181 +++++++++++++++++++++
 6 files changed, 198 insertions(+)
 create mode 100644 drivers/pmdomain/thead/Kconfig
 create mode 100644 drivers/pmdomain/thead/Makefile
 create mode 100644 drivers/pmdomain/thead/th1520-pm-domains.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 42aef66bd257..173eeaf3bbe8 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -20200,6 +20200,7 @@ F:	drivers/firmware/thead,th1520-aon.c
 F:	drivers/mailbox/mailbox-th1520.c
 F:	drivers/net/ethernet/stmicro/stmmac/dwmac-thead.c
 F:	drivers/pinctrl/pinctrl-th1520.c
+F:	drivers/pmdomain/thead/
 F:	include/dt-bindings/clock/thead,th1520-clk-ap.h
 F:	include/linux/firmware/thead/thead,th1520-aon.h
 
diff --git a/drivers/pmdomain/Kconfig b/drivers/pmdomain/Kconfig
index 23c64851a5b0..91f04ace35d4 100644
--- a/drivers/pmdomain/Kconfig
+++ b/drivers/pmdomain/Kconfig
@@ -16,6 +16,7 @@ source "drivers/pmdomain/st/Kconfig"
 source "drivers/pmdomain/starfive/Kconfig"
 source "drivers/pmdomain/sunxi/Kconfig"
 source "drivers/pmdomain/tegra/Kconfig"
+source "drivers/pmdomain/thead/Kconfig"
 source "drivers/pmdomain/ti/Kconfig"
 source "drivers/pmdomain/xilinx/Kconfig"
 
diff --git a/drivers/pmdomain/Makefile b/drivers/pmdomain/Makefile
index a68ece2f4c68..7030f44a49df 100644
--- a/drivers/pmdomain/Makefile
+++ b/drivers/pmdomain/Makefile
@@ -14,6 +14,7 @@ obj-y					+= st/
 obj-y					+= starfive/
 obj-y					+= sunxi/
 obj-y					+= tegra/
+obj-y					+= thead/
 obj-y					+= ti/
 obj-y					+= xilinx/
 obj-y					+= core.o governor.o
diff --git a/drivers/pmdomain/thead/Kconfig b/drivers/pmdomain/thead/Kconfig
new file mode 100644
index 000000000000..3b61e92dde43
--- /dev/null
+++ b/drivers/pmdomain/thead/Kconfig
@@ -0,0 +1,12 @@
+# SPDX-License-Identifier: GPL-2.0-only
+
+config TH1520_PM_DOMAINS
+	tristate "Support TH1520 Power Domains"
+	depends on TH1520_AON_PROTOCOL || COMPILE_TEST
+	select REGMAP_MMIO
+	help
+	  This driver enables power domain management for the T-HEAD
+	  TH-1520 SoC. On this SoC there are number of power domains,
+	  which can be managed independently. For example GPU, NPU,
+	  and DPU reside in their own power domains which can be
+	  turned on/off.
diff --git a/drivers/pmdomain/thead/Makefile b/drivers/pmdomain/thead/Makefile
new file mode 100644
index 000000000000..adfdf5479c68
--- /dev/null
+++ b/drivers/pmdomain/thead/Makefile
@@ -0,0 +1,2 @@
+# SPDX-License-Identifier: GPL-2.0-only
+obj-$(CONFIG_TH1520_PM_DOMAINS)		+= th1520-pm-domains.o
diff --git a/drivers/pmdomain/thead/th1520-pm-domains.c b/drivers/pmdomain/thead/th1520-pm-domains.c
new file mode 100644
index 000000000000..a56ba164a58d
--- /dev/null
+++ b/drivers/pmdomain/thead/th1520-pm-domains.c
@@ -0,0 +1,181 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2021 Alibaba Group Holding Limited.
+ * Copyright (c) 2024 Samsung Electronics Co., Ltd.
+ * Author: Michal Wilczynski <m.wilczynski@samsung.com>
+ */
+
+#include <linux/firmware/thead/thead,th1520-aon.h>
+#include <linux/slab.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/pm_domain.h>
+
+#include <dt-bindings/power/thead,th1520-power.h>
+
+struct th1520_power_domain {
+	struct th1520_aon_chan *aon_chan;
+	struct generic_pm_domain genpd;
+	u32 rsrc;
+};
+
+struct th1520_power_info {
+	const char *name;
+	u32 rsrc;
+};
+
+static const struct th1520_power_info th1520_pd_ranges[] = {
+	{ "vdec", TH1520_AON_VDEC_PD },
+	{ "npu", TH1520_AON_NPU_PD },
+	{ "venc", TH1520_AON_VENC_PD },
+	{ "gpu", TH1520_AON_GPU_PD },
+	{ "dsp0", TH1520_AON_DSP0_PD },
+	{ "dsp1", TH1520_AON_DSP1_PD }
+};
+
+static inline struct th1520_power_domain *
+to_th1520_power_domain(struct generic_pm_domain *genpd)
+{
+	return container_of(genpd, struct th1520_power_domain, genpd);
+}
+
+static int th1520_pd_power_on(struct generic_pm_domain *domain)
+{
+	struct th1520_power_domain *pd = to_th1520_power_domain(domain);
+
+	return th1520_aon_power_update(pd->aon_chan, pd->rsrc, true);
+}
+
+static int th1520_pd_power_off(struct generic_pm_domain *domain)
+{
+	struct th1520_power_domain *pd = to_th1520_power_domain(domain);
+
+	return th1520_aon_power_update(pd->aon_chan, pd->rsrc, false);
+}
+
+static struct generic_pm_domain *th1520_pd_xlate(const struct of_phandle_args *spec,
+						 void *data)
+{
+	struct generic_pm_domain *domain = ERR_PTR(-ENOENT);
+	struct genpd_onecell_data *pd_data = data;
+	unsigned int i;
+
+	for (i = 0; i < ARRAY_SIZE(th1520_pd_ranges); i++) {
+		struct th1520_power_domain *pd;
+
+		pd = to_th1520_power_domain(pd_data->domains[i]);
+		if (pd->rsrc == spec->args[0]) {
+			domain = &pd->genpd;
+			break;
+		}
+	}
+
+	return domain;
+}
+
+static struct th1520_power_domain *
+th1520_add_pm_domain(struct device *dev, const struct th1520_power_info *pi)
+{
+	struct th1520_power_domain *pd;
+	int ret;
+
+	pd = devm_kzalloc(dev, sizeof(*pd), GFP_KERNEL);
+	if (!pd)
+		return ERR_PTR(-ENOMEM);
+
+	pd->rsrc = pi->rsrc;
+	pd->genpd.power_on = th1520_pd_power_on;
+	pd->genpd.power_off = th1520_pd_power_off;
+	pd->genpd.name = pi->name;
+
+	ret = pm_genpd_init(&pd->genpd, NULL, true);
+	if (ret)
+		return ERR_PTR(ret);
+
+	return pd;
+}
+
+static void th1520_pd_init_all_off(struct generic_pm_domain **domains,
+				   struct device *dev)
+{
+	int ret;
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(th1520_pd_ranges); i++) {
+		struct th1520_power_domain *pd =
+			to_th1520_power_domain(domains[i]);
+
+		ret = th1520_aon_power_update(pd->aon_chan, pd->rsrc, false);
+		if (ret)
+			dev_err(dev,
+				"Failed to initially power down power domain %s\n",
+				pd->genpd.name);
+	}
+}
+
+static int th1520_pd_probe(struct platform_device *pdev)
+{
+	struct generic_pm_domain **domains;
+	struct genpd_onecell_data *pd_data;
+	struct th1520_aon_chan *aon_chan;
+	struct device *dev = &pdev->dev;
+	int i;
+
+	aon_chan = dev_get_drvdata(dev->parent);
+	if (!aon_chan) {
+		dev_err(dev, "Failed to get AON channel from parent\n");
+		return -EINVAL;
+	}
+
+	domains = devm_kcalloc(dev, ARRAY_SIZE(th1520_pd_ranges),
+			       sizeof(*domains), GFP_KERNEL);
+	if (!domains)
+		return -ENOMEM;
+
+	pd_data = devm_kzalloc(dev, sizeof(*pd_data), GFP_KERNEL);
+	if (!pd_data)
+		return -ENOMEM;
+
+	for (i = 0; i < ARRAY_SIZE(th1520_pd_ranges); i++) {
+		struct th1520_power_domain *pd;
+
+		pd = th1520_add_pm_domain(dev, &th1520_pd_ranges[i]);
+		if (IS_ERR(pd))
+			return PTR_ERR(pd);
+
+		pd->aon_chan = aon_chan;
+		domains[i] = &pd->genpd;
+		dev_dbg(dev, "added power domain %s\n", pd->genpd.name);
+	}
+
+	pd_data->domains = domains;
+	pd_data->num_domains = ARRAY_SIZE(th1520_pd_ranges);
+	pd_data->xlate = th1520_pd_xlate;
+
+	/*
+	 * Initialize all power domains to off to ensure they start in a
+	 * low-power state. This allows device drivers to manage power
+	 * domains by turning them on or off as needed.
+	 */
+	th1520_pd_init_all_off(domains, dev);
+
+	return of_genpd_add_provider_onecell(dev->of_node, pd_data);
+}
+
+static const struct of_device_id th1520_pd_match[] = {
+	{ .compatible = "thead,th1520-pd" },
+	{ /* sentinel */ }
+};
+
+static struct platform_driver th1520_pd_driver = {
+	.driver = {
+		.name = "th1520-pd",
+		.of_match_table = th1520_pd_match,
+	},
+	.probe = th1520_pd_probe,
+};
+module_platform_driver(th1520_pd_driver);
+
+MODULE_AUTHOR("Michal Wilczynski <m.wilczynski@samsung.com>");
+MODULE_DESCRIPTION("T-HEAD TH1520 SoC power domain controller");
+MODULE_LICENSE("GPL");
-- 
2.34.1

Re: [RFC PATCH v2 06/19] soc: thead: power-domain: Add power-domain driver for TH1520

[Krzysztof Kozlowski]

On 23/12/2024 13:55, Michal Wilczynski wrote:
> The T-Head TH1520 SoC contains multiple power islands that can be
> programmatically turned on and off using the AON (Always-On) protocol
> and a hardware mailbox [1]. The relevant mailbox driver has already been
> merged into the mainline kernel in commit 5d4d263e1c6b ("mailbox:
> Introduce support for T-head TH1520 Mailbox driver");
> 
> This commit introduces a power-domain driver for the TH1520 SoC, which
> is using AON firmware protocol to communicate with E902 core through the
> hardware mailbox. This way it can send power on/off commands to the E902
> core.
> 
> Link: https://openbeagle.org/beaglev-ahead/beaglev-ahead/-/blob/main/docs/TH1520%20System%20User%20Manual.pdf [1]
> 
> Signed-off-by: Michal Wilczynski <m.wilczynski@samsung.com>

Please use subject prefixes matching the subsystem. You can get them for
example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory
your patch is touching. For bindings, the preferred subjects are
explained here:
https://www.kernel.org/doc/html/latest/devicetree/bindings/submitting-patches.html#i-for-patch-submitters

E.g.
pmdomain: thead: Add driver for TH1520 AON


> ---
>  MAINTAINERS                                |   1 +
>  drivers/pmdomain/Kconfig                   |   1 +
>  drivers/pmdomain/Makefile                  |   1 +
>  drivers/pmdomain/thead/Kconfig             |  12 ++
>  drivers/pmdomain/thead/Makefile            |   2 +
>  drivers/pmdomain/thead/th1520-pm-domains.c | 181 +++++++++++++++++++++
>  6 files changed, 198 insertions(+)
>  create mode 100644 drivers/pmdomain/thead/Kconfig
>  create mode 100644 drivers/pmdomain/thead/Makefile
>  create mode 100644 drivers/pmdomain/thead/th1520-pm-domains.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 42aef66bd257..173eeaf3bbe8 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -20200,6 +20200,7 @@ F:	drivers/firmware/thead,th1520-aon.c
>  F:	drivers/mailbox/mailbox-th1520.c
>  F:	drivers/net/ethernet/stmicro/stmmac/dwmac-thead.c
>  F:	drivers/pinctrl/pinctrl-th1520.c
> +F:	drivers/pmdomain/thead/
>  F:	include/dt-bindings/clock/thead,th1520-clk-ap.h
>  F:	include/linux/firmware/thead/thead,th1520-aon.h
>  
> diff --git a/drivers/pmdomain/Kconfig b/drivers/pmdomain/Kconfig
> index 23c64851a5b0..91f04ace35d4 100644
> --- a/drivers/pmdomain/Kconfig
> +++ b/drivers/pmdomain/Kconfig
> @@ -16,6 +16,7 @@ source "drivers/pmdomain/st/Kconfig"
>  source "drivers/pmdomain/starfive/Kconfig"
>  source "drivers/pmdomain/sunxi/Kconfig"
>  source "drivers/pmdomain/tegra/Kconfig"
> +source "drivers/pmdomain/thead/Kconfig"
>  source "drivers/pmdomain/ti/Kconfig"
>  source "drivers/pmdomain/xilinx/Kconfig"
>  
> diff --git a/drivers/pmdomain/Makefile b/drivers/pmdomain/Makefile
> index a68ece2f4c68..7030f44a49df 100644
> --- a/drivers/pmdomain/Makefile
> +++ b/drivers/pmdomain/Makefile
> @@ -14,6 +14,7 @@ obj-y					+= st/
>  obj-y					+= starfive/
>  obj-y					+= sunxi/
>  obj-y					+= tegra/
> +obj-y					+= thead/
>  obj-y					+= ti/
>  obj-y					+= xilinx/
>  obj-y					+= core.o governor.o
> diff --git a/drivers/pmdomain/thead/Kconfig b/drivers/pmdomain/thead/Kconfig
> new file mode 100644
> index 000000000000..3b61e92dde43
> --- /dev/null
> +++ b/drivers/pmdomain/thead/Kconfig
> @@ -0,0 +1,12 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +
> +config TH1520_PM_DOMAINS
> +	tristate "Support TH1520 Power Domains"
> +	depends on TH1520_AON_PROTOCOL || COMPILE_TEST


What sort of dependency is this? Looks build time, but you miss handling
modules. You might need: https://lwn.net/Articles/944368/

Test if it really builds if TH1520_AON_PROTOCOL is a module.

> +	select REGMAP_MMIO
> +	help
> +	  This driver enables power domain management for the T-HEAD
> +	  TH-1520 SoC. On this SoC there are number of power domains,
> +	  which can be managed independently. For example GPU, NPU,
> +	  and DPU reside in their own power domains which can be
> +	  turned on/off.
> diff --git a/drivers/pmdomain/thead/Makefile b/drivers/pmdomain/thead/Makefile
> new file mode 100644
> index 000000000000..adfdf5479c68

Best regards,
Krzysztof

Re: [RFC PATCH v2 06/19] soc: thead: power-domain: Add power-domain driver for TH1520

[Krzysztof Kozlowski]

On 23/12/2024 13:55, Michal Wilczynski wrote:
> +	pd_data->domains = domains;
> +	pd_data->num_domains = ARRAY_SIZE(th1520_pd_ranges);
> +	pd_data->xlate = th1520_pd_xlate;
> +
> +	/*
> +	 * Initialize all power domains to off to ensure they start in a
> +	 * low-power state. This allows device drivers to manage power
> +	 * domains by turning them on or off as needed.
> +	 */
> +	th1520_pd_init_all_off(domains, dev);
> +
> +	return of_genpd_add_provider_onecell(dev->of_node, pd_data);
> +}
> +
> +static const struct of_device_id th1520_pd_match[] = {
> +	{ .compatible = "thead,th1520-pd" },
> +	{ /* sentinel */ }
> +};
MODULE_DEVICE_TABLE, this applies to all your patches.

Best regards,
Krzysztof

[RFC PATCH v2 07/19] riscv: Enable PM_GENERIC_DOMAINS for T-Head SoCs

[Michal Wilczynski]

T-Head SoCs feature separate power domains (power islands) for major
components like the GPU, Audio, and NPU. To manage the power states of
these components effectively, the kernel requires generic power domain
support.

This commit enables `CONFIG_PM_GENERIC_DOMAINS` for T-Head SoCs,
allowing the power domain driver for these components to be compiled and
integrated. This ensures proper power management and energy efficiency
on T-Head platforms.

By selecting `PM_GENERIC_DOMAINS`, we provide the necessary framework
for the power domain drivers to function correctly on RISC-V
architecture with T-Head SoCs.

Signed-off-by: Michal Wilczynski <m.wilczynski@samsung.com>
---
 arch/riscv/Kconfig.socs | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/riscv/Kconfig.socs b/arch/riscv/Kconfig.socs
index f51bb24bc84c..c414dc618b66 100644
--- a/arch/riscv/Kconfig.socs
+++ b/arch/riscv/Kconfig.socs
@@ -48,6 +48,7 @@ config ARCH_THEAD
 	bool "T-HEAD RISC-V SoCs"
 	depends on MMU && !XIP_KERNEL
 	select ERRATA_THEAD
+	select PM_GENERIC_DOMAINS if PM
 	help
 	  This enables support for the RISC-V based T-HEAD SoCs.
 
-- 
2.34.1

[RFC PATCH v2 08/19] dt-bindings: reset: Add T-HEAD TH1520 SoC Reset Controller

[Michal Wilczynski]

Add a YAML schema for the T-HEAD TH1520 SoC reset controller. This
controller manages resets for subsystems such as the GPU within the
TH1520 SoC.

Signed-off-by: Michal Wilczynski <m.wilczynski@samsung.com>
---
 .../bindings/reset/thead,th1520-reset.yaml    | 45 +++++++++++++++++++
 MAINTAINERS                                   |  2 +
 .../dt-bindings/reset/thead,th1520-reset.h    | 13 ++++++
 3 files changed, 60 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/reset/thead,th1520-reset.yaml
 create mode 100644 include/dt-bindings/reset/thead,th1520-reset.h

diff --git a/Documentation/devicetree/bindings/reset/thead,th1520-reset.yaml b/Documentation/devicetree/bindings/reset/thead,th1520-reset.yaml
new file mode 100644
index 000000000000..46d0e6b8c712
--- /dev/null
+++ b/Documentation/devicetree/bindings/reset/thead,th1520-reset.yaml
@@ -0,0 +1,45 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/reset/thead,th1520-reset.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: T-HEAD TH1520 SoC Reset Controller
+
+description: |
+  The T-HEAD TH1520 reset controller is a hardware block that asserts/deasserts
+  resets for SoC subsystems.
+
+maintainers:
+  - Michal Wilczynski <m.wilczynski@samsung.com>
+
+properties:
+  compatible:
+    enum:
+      - thead,th1520-reset
+
+  reg:
+    maxItems: 1
+
+  "#reset-cells":
+    const: 1
+
+required:
+  - compatible
+  - reg
+  - "#reset-cells"
+
+additionalProperties: false
+
+examples:
+  - |
+    soc {
+      #address-cells = <2>;
+      #size-cells = <2>;
+      rst: reset-controller@ffef528000 {
+        compatible = "thead,th1520-reset";
+        reg = <0xff 0xef528000 0x0 0x1000>;
+        #reset-cells = <1>;
+      };
+    };
diff --git a/MAINTAINERS b/MAINTAINERS
index 173eeaf3bbe8..db8c55058c29 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -20194,6 +20194,7 @@ F:	Documentation/devicetree/bindings/mailbox/thead,th1520-mbox.yaml
 F:	Documentation/devicetree/bindings/net/thead,th1520-gmac.yaml
 F:	Documentation/devicetree/bindings/pinctrl/thead,th1520-pinctrl.yaml
 F:	Documentation/devicetree/bindings/power/thead,th1520-power.yaml
+F:	Documentation/devicetree/bindings/reset/thead,th1520-reset.yaml
 F:	arch/riscv/boot/dts/thead/
 F:	drivers/clk/thead/clk-th1520-ap.c
 F:	drivers/firmware/thead,th1520-aon.c
@@ -20202,6 +20203,7 @@ F:	drivers/net/ethernet/stmicro/stmmac/dwmac-thead.c
 F:	drivers/pinctrl/pinctrl-th1520.c
 F:	drivers/pmdomain/thead/
 F:	include/dt-bindings/clock/thead,th1520-clk-ap.h
+F:	include/dt-bindings/reset/thead,th1520-reset.h
 F:	include/linux/firmware/thead/thead,th1520-aon.h
 
 RNBD BLOCK DRIVERS
diff --git a/include/dt-bindings/reset/thead,th1520-reset.h b/include/dt-bindings/reset/thead,th1520-reset.h
new file mode 100644
index 000000000000..a4958b2ed710
--- /dev/null
+++ b/include/dt-bindings/reset/thead,th1520-reset.h
@@ -0,0 +1,13 @@
+/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
+/*
+ * Copyright (c) 2024 Samsung Electronics Co., Ltd.
+ * Author: Michal Wilczynski <m.wilczynski@samsung.com>
+ */
+
+#ifndef _DT_BINDINGS_TH1520_RESET_H
+#define _DT_BINDINGS_TH1520_RESET_H
+
+#define TH1520_RESET_ID_GPU	0
+#define TH1520_RESET_NUM_IDS	1
+
+#endif /* _DT_BINDINGS_TH1520_RESET_H */
-- 
2.34.1

Re: [RFC PATCH v2 08/19] dt-bindings: reset: Add T-HEAD TH1520 SoC Reset Controller

[Krzysztof Kozlowski]

On 23/12/2024 13:55, Michal Wilczynski wrote:
> Add a YAML schema for the T-HEAD TH1520 SoC reset controller. This
> controller manages resets for subsystems such as the GPU within the
> TH1520 SoC.
> 
> Signed-off-by: Michal Wilczynski <m.wilczynski@samsung.com>
> ---
>  .../bindings/reset/thead,th1520-reset.yaml    | 45 +++++++++++++++++++
>  MAINTAINERS                                   |  2 +
>  .../dt-bindings/reset/thead,th1520-reset.h    | 13 ++++++
>  3 files changed, 60 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/reset/thead,th1520-reset.yaml
>  create mode 100644 include/dt-bindings/reset/thead,th1520-reset.h
> 
> diff --git a/Documentation/devicetree/bindings/reset/thead,th1520-reset.yaml b/Documentation/devicetree/bindings/reset/thead,th1520-reset.yaml
> new file mode 100644
> index 000000000000..46d0e6b8c712
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/reset/thead,th1520-reset.yaml
> @@ -0,0 +1,45 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +

Drop blank line

> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/reset/thead,th1520-reset.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: T-HEAD TH1520 SoC Reset Controller
> +
> +description: |

Do not need '|' unless you need to preserve formatting.

> +  The T-HEAD TH1520 reset controller is a hardware block that asserts/deasserts
> +  resets for SoC subsystems.
> +
> +maintainers:
> +  - Michal Wilczynski <m.wilczynski@samsung.com>
> +
> +properties:
> +  compatible:
> +    enum:
> +      - thead,th1520-reset
> +

...

>  RNBD BLOCK DRIVERS
> diff --git a/include/dt-bindings/reset/thead,th1520-reset.h b/include/dt-bindings/reset/thead,th1520-reset.h
> new file mode 100644
> index 000000000000..a4958b2ed710
> --- /dev/null
> +++ b/include/dt-bindings/reset/thead,th1520-reset.h
> @@ -0,0 +1,13 @@
> +/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
> +/*
> + * Copyright (c) 2024 Samsung Electronics Co., Ltd.
> + * Author: Michal Wilczynski <m.wilczynski@samsung.com>
> + */
> +
> +#ifndef _DT_BINDINGS_TH1520_RESET_H
> +#define _DT_BINDINGS_TH1520_RESET_H
> +
> +#define TH1520_RESET_ID_GPU	0
> +#define TH1520_RESET_NUM_IDS	1

Drop the NUM_IDS define. Number is not a binding.

But this leads to another question: only one reset? Then reset-cells
should be 0.

> +
> +#endif /* _DT_BINDINGS_TH1520_RESET_H */


Best regards,
Krzysztof

[RFC PATCH v2 09/19] reset: thead: Add TH1520 reset controller driver

[Michal Wilczynski]

This patch introduces the reset controller driver for the T-HEAD
TH1520 SoC. The controller manages hardware reset lines for various SoC
subsystems, such as the GPU. By exposing these resets via the Linux
reset subsystem, drivers can request and control hardware resets to
reliably initialize or recover key components.

Signed-off-by: Michal Wilczynski <m.wilczynski@samsung.com>
---
 MAINTAINERS                  |   1 +
 drivers/reset/Kconfig        |  10 +++
 drivers/reset/Makefile       |   1 +
 drivers/reset/reset-th1520.c | 151 +++++++++++++++++++++++++++++++++++
 4 files changed, 163 insertions(+)
 create mode 100644 drivers/reset/reset-th1520.c

diff --git a/MAINTAINERS b/MAINTAINERS
index db8c55058c29..38130ae25c6f 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -20202,6 +20202,7 @@ F:	drivers/mailbox/mailbox-th1520.c
 F:	drivers/net/ethernet/stmicro/stmmac/dwmac-thead.c
 F:	drivers/pinctrl/pinctrl-th1520.c
 F:	drivers/pmdomain/thead/
+F:	drivers/reset/reset-th1520.c
 F:	include/dt-bindings/clock/thead,th1520-clk-ap.h
 F:	include/dt-bindings/reset/thead,th1520-reset.h
 F:	include/linux/firmware/thead/thead,th1520-aon.h
diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
index 5b3abb6db248..fa0943c3d1de 100644
--- a/drivers/reset/Kconfig
+++ b/drivers/reset/Kconfig
@@ -272,6 +272,16 @@ config RESET_SUNXI
 	help
 	  This enables the reset driver for Allwinner SoCs.
 
+config RESET_TH1520
+	tristate "T-HEAD 1520 reset controller"
+	depends on ARCH_THEAD || COMPILE_TEST
+	select REGMAP_MMIO
+	help
+	  This driver provides support for the T-HEAD TH1520 SoC reset controller,
+	  which manages hardware reset lines for SoC components such as the GPU.
+	  Enable this option if you need to control hardware resets on TH1520-based
+	  systems.
+
 config RESET_TI_SCI
 	tristate "TI System Control Interface (TI-SCI) reset driver"
 	depends on TI_SCI_PROTOCOL || (COMPILE_TEST && TI_SCI_PROTOCOL=n)
diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile
index 677c4d1e2632..d6c2774407ae 100644
--- a/drivers/reset/Makefile
+++ b/drivers/reset/Makefile
@@ -35,6 +35,7 @@ obj-$(CONFIG_RESET_SIMPLE) += reset-simple.o
 obj-$(CONFIG_RESET_SOCFPGA) += reset-socfpga.o
 obj-$(CONFIG_RESET_SUNPLUS) += reset-sunplus.o
 obj-$(CONFIG_RESET_SUNXI) += reset-sunxi.o
+obj-$(CONFIG_RESET_TH1520) += reset-th1520.o
 obj-$(CONFIG_RESET_TI_SCI) += reset-ti-sci.o
 obj-$(CONFIG_RESET_TI_SYSCON) += reset-ti-syscon.o
 obj-$(CONFIG_RESET_TI_TPS380X) += reset-tps380x.o
diff --git a/drivers/reset/reset-th1520.c b/drivers/reset/reset-th1520.c
new file mode 100644
index 000000000000..10ca200690d5
--- /dev/null
+++ b/drivers/reset/reset-th1520.c
@@ -0,0 +1,151 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2024 Samsung Electronics Co., Ltd.
+ * Author: Michal Wilczynski <m.wilczynski@samsung.com>
+ */
+
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/reset-controller.h>
+#include <linux/regmap.h>
+
+#include <dt-bindings/reset/thead,th1520-reset.h>
+
+ /* register offset in VOSYS_REGMAP */
+#define TH1520_GPU_RST_CFG		0x0
+#define TH1520_GPU_RST_CFG_MASK		GENMASK(2, 0)
+
+/* register values */
+#define TH1520_GPU_SW_GPU_RST		BIT(0)
+#define TH1520_GPU_SW_CLKGEN_RST	BIT(1)
+
+struct th1520_reset_priv {
+	struct reset_controller_dev rcdev;
+	struct regmap *map;
+};
+
+static inline struct th1520_reset_priv *
+to_th1520_reset(struct reset_controller_dev *rcdev)
+{
+	return container_of(rcdev, struct th1520_reset_priv, rcdev);
+}
+
+static void th1520_rst_gpu_enable(struct regmap *reg)
+{
+	int val;
+
+	/* if the GPU is not in a reset state it, put it into one */
+	regmap_read(reg, TH1520_GPU_RST_CFG, &val);
+	if (val) {
+		regmap_update_bits(reg, TH1520_GPU_RST_CFG,
+				   TH1520_GPU_RST_CFG_MASK, 0x0);
+	}
+
+	/* rst gpu clkgen */
+	regmap_set_bits(reg, TH1520_GPU_RST_CFG, TH1520_GPU_SW_CLKGEN_RST);
+
+	/*
+	 * According to the hardware manual, a delay of at least 32 clock
+	 * cycles is required between de-asserting the clkgen reset and
+	 * de-asserting the GPU reset. Assuming a worst-case scenario with
+	 * a very high GPU clock frequency, a delay of 1 microsecond is
+	 * sufficient to ensure this requirement is met across all
+	 * feasible GPU clock speeds.
+	 */
+	udelay(1);
+
+	/* rst gpu */
+	regmap_set_bits(reg, TH1520_GPU_RST_CFG, TH1520_GPU_SW_GPU_RST);
+}
+
+static void th1520_rst_gpu_disable(struct regmap *reg)
+{
+	regmap_update_bits(reg, TH1520_GPU_RST_CFG, TH1520_GPU_RST_CFG_MASK, 0x0);
+}
+
+static int th1520_reset_assert(struct reset_controller_dev *rcdev, unsigned long id)
+{
+	struct th1520_reset_priv *priv = to_th1520_reset(rcdev);
+
+	switch (id) {
+	case TH1520_RESET_ID_GPU:
+		th1520_rst_gpu_disable(priv->map);
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int th1520_reset_deassert(struct reset_controller_dev *rcdev, unsigned long id)
+{
+	struct th1520_reset_priv *priv = to_th1520_reset(rcdev);
+
+	switch (id) {
+	case TH1520_RESET_ID_GPU:
+		th1520_rst_gpu_enable(priv->map);
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static const struct reset_control_ops th1520_reset_ops = {
+	.assert	= th1520_reset_assert,
+	.deassert = th1520_reset_deassert,
+};
+
+const struct regmap_config th1520_reset_regmap_config = {
+	.reg_bits = 32,
+	.val_bits = 32,
+	.reg_stride = 4,
+	.fast_io = true,
+};
+
+static int th1520_reset_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct th1520_reset_priv *priv;
+	void __iomem *base;
+
+	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(base))
+		return PTR_ERR(base);
+
+	priv->map =
+		devm_regmap_init_mmio(dev, base, &th1520_reset_regmap_config);
+	if (IS_ERR(priv->map))
+		return PTR_ERR(priv->map);
+
+	priv->rcdev.owner = THIS_MODULE;
+	priv->rcdev.nr_resets = TH1520_RESET_NUM_IDS;
+	priv->rcdev.ops = &th1520_reset_ops;
+	priv->rcdev.of_node = dev->of_node;
+
+	return devm_reset_controller_register(dev, &priv->rcdev);
+}
+
+static const struct of_device_id th1520_reset_match[] = {
+	{ .compatible = "thead,th1520-reset" },
+	{ /* sentinel */ }
+};
+
+static struct platform_driver th1520_reset_driver = {
+	.driver = {
+		.name = "th1520-reset",
+		.of_match_table = th1520_reset_match,
+	},
+	.probe = th1520_reset_probe,
+};
+module_platform_driver(th1520_reset_driver);
+
+MODULE_AUTHOR("Michal Wilczynski <m.wilczynski@samsung.com>");
+MODULE_DESCRIPTION("T-HEAD TH1520 SoC reset controller");
+MODULE_LICENSE("GPL");
-- 
2.34.1

Re: [RFC PATCH v2 09/19] reset: thead: Add TH1520 reset controller driver

[Krzysztof Kozlowski]

On 23/12/2024 13:55, Michal Wilczynski wrote:
> This patch introduces the reset controller driver for the T-HEAD

Please do not use "This commit/patch/change", but imperative mood. See
longer explanation here:
https://elixir.bootlin.com/linux/v5.17.1/source/Documentation/process/submitting-patches.rst#L95

> TH1520 SoC. The controller manages hardware reset lines for various SoC
> subsystems, such as the GPU. By exposing these resets via the Linux
> reset subsystem, drivers can request and control hardware resets to
> reliably initialize or recover key components.
> 



>  config RESET_TI_SCI
>  	tristate "TI System Control Interface (TI-SCI) reset driver"
>  	depends on TI_SCI_PROTOCOL || (COMPILE_TEST && TI_SCI_PROTOCOL=n)
> diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile
> index 677c4d1e2632..d6c2774407ae 100644
> --- a/drivers/reset/Makefile
> +++ b/drivers/reset/Makefile
> @@ -35,6 +35,7 @@ obj-$(CONFIG_RESET_SIMPLE) += reset-simple.o
>  obj-$(CONFIG_RESET_SOCFPGA) += reset-socfpga.o
>  obj-$(CONFIG_RESET_SUNPLUS) += reset-sunplus.o
>  obj-$(CONFIG_RESET_SUNXI) += reset-sunxi.o
> +obj-$(CONFIG_RESET_TH1520) += reset-th1520.o
>  obj-$(CONFIG_RESET_TI_SCI) += reset-ti-sci.o
>  obj-$(CONFIG_RESET_TI_SYSCON) += reset-ti-syscon.o
>  obj-$(CONFIG_RESET_TI_TPS380X) += reset-tps380x.o
> diff --git a/drivers/reset/reset-th1520.c b/drivers/reset/reset-th1520.c
> new file mode 100644
> index 000000000000..10ca200690d5
> --- /dev/null
> +++ b/drivers/reset/reset-th1520.c
> @@ -0,0 +1,151 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2024 Samsung Electronics Co., Ltd.
> + * Author: Michal Wilczynski <m.wilczynski@samsung.com>
> + */
> +
> +#include <linux/of.h>

This looks unused. What you need is mod_devicetable.h and
MODULE_DEVICE_TABLE(th1520_reset_match) after th1520_reset_match.

> +#include <linux/platform_device.h>
> +#include <linux/reset-controller.h>
> +#include <linux/regmap.h>
> +
> +#include <dt-bindings/reset/thead,th1520-reset.h>
> +
> + /* register offset in VOSYS_REGMAP */
> +#define TH1520_GPU_RST_CFG		0x0
> +#define TH1520_GPU_RST_CFG_MASK		GENMASK(2, 0)
> +
> +/* register values */
> +#define TH1520_GPU_SW_GPU_RST		BIT(0)
> +#define TH1520_GPU_SW_CLKGEN_RST	BIT(1)
> +
> +struct th1520_reset_priv {
> +	struct reset_controller_dev rcdev;
> +	struct regmap *map;
> +};
> +
> +static inline struct th1520_reset_priv *
> +to_th1520_reset(struct reset_controller_dev *rcdev)
> +{
> +	return container_of(rcdev, struct th1520_reset_priv, rcdev);
> +}
> +
> +static void th1520_rst_gpu_enable(struct regmap *reg)
> +{
> +	int val;
> +
> +	/* if the GPU is not in a reset state it, put it into one */
> +	regmap_read(reg, TH1520_GPU_RST_CFG, &val);
> +	if (val) {

Drop {}

> +		regmap_update_bits(reg, TH1520_GPU_RST_CFG,
> +				   TH1520_GPU_RST_CFG_MASK, 0x0);
> +	}
> +
> +	/* rst gpu clkgen */
> +	regmap_set_bits(reg, TH1520_GPU_RST_CFG, TH1520_GPU_SW_CLKGEN_RST);
> +
> +	/*
> +	 * According to the hardware manual, a delay of at least 32 clock
> +	 * cycles is required between de-asserting the clkgen reset and
> +	 * de-asserting the GPU reset. Assuming a worst-case scenario with
> +	 * a very high GPU clock frequency, a delay of 1 microsecond is
> +	 * sufficient to ensure this requirement is met across all
> +	 * feasible GPU clock speeds.
> +	 */
> +	udelay(1);
> +
> +	/* rst gpu */
> +	regmap_set_bits(reg, TH1520_GPU_RST_CFG, TH1520_GPU_SW_GPU_RST);
> +}
> +
> +static void th1520_rst_gpu_disable(struct regmap *reg)
> +{
> +	regmap_update_bits(reg, TH1520_GPU_RST_CFG, TH1520_GPU_RST_CFG_MASK, 0x0);
> +}
> +
> +static int th1520_reset_assert(struct reset_controller_dev *rcdev, unsigned long id)
> +{
> +	struct th1520_reset_priv *priv = to_th1520_reset(rcdev);
> +
> +	switch (id) {
> +	case TH1520_RESET_ID_GPU:
> +		th1520_rst_gpu_disable(priv->map);
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
> +static int th1520_reset_deassert(struct reset_controller_dev *rcdev, unsigned long id)
> +{
> +	struct th1520_reset_priv *priv = to_th1520_reset(rcdev);
> +
> +	switch (id) {
> +	case TH1520_RESET_ID_GPU:
> +		th1520_rst_gpu_enable(priv->map);
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
> +static const struct reset_control_ops th1520_reset_ops = {
> +	.assert	= th1520_reset_assert,
> +	.deassert = th1520_reset_deassert,
> +};
> +
> +const struct regmap_config th1520_reset_regmap_config = {

Should be static

> +	.reg_bits = 32,
> +	.val_bits = 32,
> +	.reg_stride = 4,
> +	.fast_io = true,
> +};
> +
> +static int th1520_reset_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct th1520_reset_priv *priv;
> +	void __iomem *base;
> +
> +	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> +	if (!priv)
> +		return -ENOMEM;
> +
> +	base = devm_platform_ioremap_resource(pdev, 0);
> +	if (IS_ERR(base))
> +		return PTR_ERR(base);
> +
> +	priv->map =
> +		devm_regmap_init_mmio(dev, base, &th1520_reset_regmap_config);

Join lines. I feel you used some incorrect clang or other editor
settings leading to such code format. Code can exceed 80 if improves
readability, but if you wanted to wrap, then the wrapping should be
after 'base' and next line aligned with opening (.

> +	if (IS_ERR(priv->map))
> +		return PTR_ERR(priv->map);
> +
> +	priv->rcdev.owner = THIS_MODULE;
> +	priv->rcdev.nr_resets = TH1520_RESET_NUM_IDS;
> +	priv->rcdev.ops = &th1520_reset_ops;



Best regards,
Krzysztof

[RFC PATCH v2 10/19] drm/imagination: Add reset controller support for GPU initialization

[Michal Wilczynski]

Certain platforms, such as the T-Head TH1520 and Banana Pi BPI-F3,
require a controlled GPU reset sequence during the power-up procedure
to ensure proper initialization. Without this reset, the GPU may remain
in an undefined state, potentially leading to stability or performance
issues.

This commit integrates a dedicated reset controller within the
drm/imagination driver. By doing so, the driver can coordinate the
necessary reset operations as part of the normal GPU bring-up process,
improving reliability and ensuring that the hardware is ready for
operation.

Signed-off-by: Michal Wilczynski <m.wilczynski@samsung.com>
---
 drivers/gpu/drm/imagination/pvr_device.c | 21 +++++++++++++++++++++
 drivers/gpu/drm/imagination/pvr_device.h |  9 +++++++++
 drivers/gpu/drm/imagination/pvr_power.c  | 15 ++++++++++++++-
 3 files changed, 44 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/imagination/pvr_device.c b/drivers/gpu/drm/imagination/pvr_device.c
index 1704c0268589..83a8ba70a30e 100644
--- a/drivers/gpu/drm/imagination/pvr_device.c
+++ b/drivers/gpu/drm/imagination/pvr_device.c
@@ -25,6 +25,7 @@
 #include <linux/interrupt.h>
 #include <linux/platform_device.h>
 #include <linux/pm_runtime.h>
+#include <linux/reset.h>
 #include <linux/slab.h>
 #include <linux/stddef.h>
 #include <linux/types.h>
@@ -120,6 +121,21 @@ static int pvr_device_clk_init(struct pvr_device *pvr_dev)
 	return 0;
 }
 
+static int pvr_device_reset_init(struct pvr_device *pvr_dev)
+{
+	struct drm_device *drm_dev = from_pvr_device(pvr_dev);
+	struct reset_control *reset;
+
+	reset = devm_reset_control_get_optional_exclusive(drm_dev->dev, "gpu");
+	if (IS_ERR(reset))
+		return dev_err_probe(drm_dev->dev, PTR_ERR(reset),
+				     "failed to get gpu reset line\n");
+
+	pvr_dev->reset = reset;
+
+	return 0;
+}
+
 /**
  * pvr_device_process_active_queues() - Process all queue related events.
  * @pvr_dev: PowerVR device to check
@@ -509,6 +525,11 @@ pvr_device_init(struct pvr_device *pvr_dev)
 	if (err)
 		return err;
 
+	/* Get the reset line for the GPU */
+	err = pvr_device_reset_init(pvr_dev);
+	if (err)
+		return err;
+
 	/* Explicitly power the GPU so we can access control registers before the FW is booted. */
 	err = pm_runtime_resume_and_get(dev);
 	if (err)
diff --git a/drivers/gpu/drm/imagination/pvr_device.h b/drivers/gpu/drm/imagination/pvr_device.h
index 6d0dfacb677b..f6576c08111c 100644
--- a/drivers/gpu/drm/imagination/pvr_device.h
+++ b/drivers/gpu/drm/imagination/pvr_device.h
@@ -131,6 +131,15 @@ struct pvr_device {
 	 */
 	struct clk *mem_clk;
 
+	/**
+	 * @reset: Optional reset line.
+	 *
+	 * This may be used on some platforms to provide a reset line that needs to be de-asserted
+	 * after power-up procedure. It would also need to be asserted after the power-down
+	 * procedure.
+	 */
+	struct reset_control *reset;
+
 	/** @irq: IRQ number. */
 	int irq;
 
diff --git a/drivers/gpu/drm/imagination/pvr_power.c b/drivers/gpu/drm/imagination/pvr_power.c
index ba7816fd28ec..87a955600d8b 100644
--- a/drivers/gpu/drm/imagination/pvr_power.c
+++ b/drivers/gpu/drm/imagination/pvr_power.c
@@ -15,6 +15,7 @@
 #include <linux/mutex.h>
 #include <linux/platform_device.h>
 #include <linux/pm_runtime.h>
+#include <linux/reset.h>
 #include <linux/timer.h>
 #include <linux/types.h>
 #include <linux/workqueue.h>
@@ -252,6 +253,9 @@ pvr_power_device_suspend(struct device *dev)
 	clk_disable_unprepare(pvr_dev->sys_clk);
 	clk_disable_unprepare(pvr_dev->core_clk);
 
+	if (pvr_dev->reset)
+		err = reset_control_assert(pvr_dev->reset);
+
 err_drm_dev_exit:
 	drm_dev_exit(idx);
 
@@ -282,16 +286,25 @@ pvr_power_device_resume(struct device *dev)
 	if (err)
 		goto err_sys_clk_disable;
 
+	if (pvr_dev->reset) {
+		err = reset_control_deassert(pvr_dev->reset);
+		if (err)
+			goto err_mem_clk_disable;
+	}
+
 	if (pvr_dev->fw_dev.booted) {
 		err = pvr_power_fw_enable(pvr_dev);
 		if (err)
-			goto err_mem_clk_disable;
+			goto err_reset_assert;
 	}
 
 	drm_dev_exit(idx);
 
 	return 0;
 
+err_reset_assert:
+	reset_control_assert(pvr_dev->reset);
+
 err_mem_clk_disable:
 	clk_disable_unprepare(pvr_dev->mem_clk);
 
-- 
2.34.1

[RFC PATCH v2 11/19] dt-bindings: gpu: Add 'resets' property for GPU initialization

[Michal Wilczynski]

Many RISC-V boards featuring Imagination Technologies GPUs require a
reset line to be de-asserted as part of the GPU power-up sequence. To
support this, add a 'resets' property (and corresponding 'reset-names')
to the GPU device tree bindings. This ensures the GPU can be properly
initialized on these platforms.

Signed-off-by: Michal Wilczynski <m.wilczynski@samsung.com>
---
 .../devicetree/bindings/gpu/img,powervr-rogue.yaml          | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml b/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml
index 256e252f8087..4078cc816ea1 100644
--- a/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml
+++ b/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml
@@ -37,6 +37,12 @@ properties:
   power-domains:
     maxItems: 1
 
+  resets:
+    maxItems: 1
+
+  reset-names:
+    const: gpu
+
 required:
   - compatible
   - reg
-- 
2.34.1

Re: [RFC PATCH v2 11/19] dt-bindings: gpu: Add 'resets' property for GPU initialization

[Krzysztof Kozlowski]

On 23/12/2024 13:55, Michal Wilczynski wrote:
> diff --git a/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml b/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml
> index 256e252f8087..4078cc816ea1 100644
> --- a/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml
> +++ b/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml
> @@ -37,6 +37,12 @@ properties:
>    power-domains:
>      maxItems: 1
>  
> +  resets:
> +    maxItems: 1
> +
> +  reset-names:
> +    const: gpu
Name being equal to the name of the block is not useful. Drop
reset-names. Reset framework allows to get the reset instance without name.

Best regards,
Krzysztof

[RFC PATCH v2 12/19] dt-bindings: gpu: Add compatibles for T-HEAD TH1520 GPU

[Michal Wilczynski]

Add a new SoC-specific compatible ("thead,th1520-gpu") for the T-HEAD
TH1520 GPU, alongside the Imagination BXM family compatible
("img,img-bxm").  This documents the GPU integration on the T-HEAD
platform.

Also adjust clock name constraints to accommodate a second clock named
"sys" instead of "mem" for T-HEAD. This is achieved by changing the
order, and making the 'sys' appear before 'mem'.

Provide example of the new GPU node.

Signed-off-by: Michal Wilczynski <m.wilczynski@samsung.com>
---
 .../bindings/gpu/img,powervr-rogue.yaml       | 32 +++++++++++++++++--
 1 file changed, 30 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml b/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml
index 4078cc816ea1..1eb1c15baec4 100644
--- a/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml
+++ b/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml
@@ -15,7 +15,10 @@ properties:
     items:
       - enum:
           - ti,am62-gpu
-      - const: img,img-axe # IMG AXE GPU model/revision is fully discoverable
+          - thead,th1520-gpu
+      - enum:
+          - img,img-axe # IMG AXE GPU model/revision is fully discoverable
+          - img,img-bxm
 
   reg:
     maxItems: 1
@@ -27,8 +30,8 @@ properties:
   clock-names:
     items:
       - const: core
-      - const: mem
       - const: sys
+      - const: mem
     minItems: 1
 
   interrupts:
@@ -62,6 +65,15 @@ allOf:
       properties:
         clocks:
           maxItems: 1
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: thead,th1520-gpu
+    then:
+      properties:
+        clocks:
+          maxItems: 2
 
 examples:
   - |
@@ -77,3 +89,19 @@ examples:
         interrupts = <GIC_SPI 86 IRQ_TYPE_LEVEL_HIGH>;
         power-domains = <&k3_pds 187 TI_SCI_PD_EXCLUSIVE>;
     };
+
+    #include <dt-bindings/reset/thead,th1520-reset.h>
+    #include <dt-bindings/clock/thead,th1520-clk-ap.h>
+    #include <dt-bindings/power/thead,th1520-power.h>
+
+    gpu: gpu@fff0000 {
+        compatible = "thead,th1520-gpu", "img,img-bxm";
+        reg = <0xfff0000 0x1000>;
+        interrupt-parent = <&plic>;
+        interrupts = <102 IRQ_TYPE_LEVEL_HIGH>;
+        clocks = <&clk CLK_GPU_CORE>, <&clk CLK_GPU_CFG_ACLK>;
+        clock-names = "core", "sys";
+        power-domains = <&pd TH1520_AON_GPU_PD>;
+        resets = <&rst TH1520_RESET_ID_GPU>;
+        reset-names = "gpu";
+    };
-- 
2.34.1

Re: [RFC PATCH v2 12/19] dt-bindings: gpu: Add compatibles for T-HEAD TH1520 GPU

[Krzysztof Kozlowski]

On 23/12/2024 13:55, Michal Wilczynski wrote:
> Add a new SoC-specific compatible ("thead,th1520-gpu") for the T-HEAD
> TH1520 GPU, alongside the Imagination BXM family compatible
> ("img,img-bxm").  This documents the GPU integration on the T-HEAD
> platform.
> 
> Also adjust clock name constraints to accommodate a second clock named
> "sys" instead of "mem" for T-HEAD. This is achieved by changing the
> order, and making the 'sys' appear before 'mem'.
> 
> Provide example of the new GPU node.
> 
> Signed-off-by: Michal Wilczynski <m.wilczynski@samsung.com>
> ---
>  .../bindings/gpu/img,powervr-rogue.yaml       | 32 +++++++++++++++++--
>  1 file changed, 30 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml b/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml
> index 4078cc816ea1..1eb1c15baec4 100644
> --- a/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml
> +++ b/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml
> @@ -15,7 +15,10 @@ properties:
>      items:
>        - enum:
>            - ti,am62-gpu
> -      - const: img,img-axe # IMG AXE GPU model/revision is fully discoverable
> +          - thead,th1520-gpu
> +      - enum:
> +          - img,img-axe # IMG AXE GPU model/revision is fully discoverable
> +          - img,img-bxm

This cannot be flexible. You now changed am62-gpu. Look how other
bindings do it: oneOf with two lists.
>  
>    reg:
>      maxItems: 1
> @@ -27,8 +30,8 @@ properties:
>    clock-names:
>      items:
>        - const: core
> -      - const: mem
>        - const: sys
> +      - const: mem

That's ABI break, so no.

Widest constraints - min/maxItems - stay here. In allOf:if:then you
specify the list of items. See:
https://elixir.bootlin.com/linux/v6.11-rc6/source/Documentation/devicetree/bindings/ufs/qcom,ufs.yaml#L127

You will need to adjust existing variants - list the items there.

>      minItems: 1
>  
Best regards,
Krzysztof

[RFC PATCH v2 13/19] drm/imagination: Add support for IMG BXM-4-64 GPU

[Michal Wilczynski]

The IMG BXM-4-64 GPU is integrated into the T-Head TH1520 SoC. This
commit adds the compatible string "img,img-bxm" to the device tree match
table in the drm/imagination driver, enabling support for this GPU.

By including this GPU in the compatible devices list, the driver can
initialize and manage the BXM-4-64 GPU on the TH1520 SoC, providing
graphics acceleration capabilities upstream.

Signed-off-by: Michal Wilczynski <m.wilczynski@samsung.com>
---
 drivers/gpu/drm/imagination/pvr_drv.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/imagination/pvr_drv.c b/drivers/gpu/drm/imagination/pvr_drv.c
index 85ee9abd1811..91af060bb3e0 100644
--- a/drivers/gpu/drm/imagination/pvr_drv.c
+++ b/drivers/gpu/drm/imagination/pvr_drv.c
@@ -1475,6 +1475,7 @@ static void pvr_remove(struct platform_device *plat_dev)
 
 static const struct of_device_id dt_match[] = {
 	{ .compatible = "img,img-axe", .data = NULL },
+	{ .compatible = "img,img-bxm", .data = NULL },
 	{}
 };
 MODULE_DEVICE_TABLE(of, dt_match);
-- 
2.34.1

[RFC PATCH v2 14/19] drm/imagination: Enable PowerVR driver for RISC-V

[Michal Wilczynski]

Several RISC-V boards feature Imagination GPUs that are compatible with
the PowerVR driver. An example is the IMG BXM-4-64 GPU on the Lichee Pi
4A board. This commit adjusts the driver's Kconfig dependencies to allow
the PowerVR driver to be compiled on the RISC-V architecture.

By enabling compilation on RISC-V, we expand support for these GPUs,
providing graphics acceleration capabilities and enhancing hardware
compatibility on RISC-V platforms.

Signed-off-by: Michal Wilczynski <m.wilczynski@samsung.com>
---
 drivers/gpu/drm/imagination/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/imagination/Kconfig b/drivers/gpu/drm/imagination/Kconfig
index 3bfa2ac212dc..5f218896114c 100644
--- a/drivers/gpu/drm/imagination/Kconfig
+++ b/drivers/gpu/drm/imagination/Kconfig
@@ -3,7 +3,7 @@
 
 config DRM_POWERVR
 	tristate "Imagination Technologies PowerVR (Series 6 and later) & IMG Graphics"
-	depends on ARM64
+	depends on (ARM64 || RISCV)
 	depends on DRM
 	depends on PM
 	select DRM_EXEC
-- 
2.34.1

[RFC PATCH v2 15/19] riscv: dts: thead: Extend device tree clk with VO reg

[Michal Wilczynski]

VO clocks reside in a different address space from the AP clocks on the
T-HEAD SoC. Extend the device tree node of a clock-controller to handle
VO address space as well.

Signed-off-by: Michal Wilczynski <m.wilczynski@samsung.com>
---
 arch/riscv/boot/dts/thead/th1520.dtsi | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/riscv/boot/dts/thead/th1520.dtsi b/arch/riscv/boot/dts/thead/th1520.dtsi
index acfe030e803a..d823b6da75df 100644
--- a/arch/riscv/boot/dts/thead/th1520.dtsi
+++ b/arch/riscv/boot/dts/thead/th1520.dtsi
@@ -484,7 +484,9 @@ uart2: serial@ffec010000 {
 
 		clk: clock-controller@ffef010000 {
 			compatible = "thead,th1520-clk-ap";
-			reg = <0xff 0xef010000 0x0 0x1000>;
+			reg = <0xff 0xef010000 0x0 0x1000>,
+			      <0xff 0xef528050 0x0 0xfb0>;
+			reg-names = "ap-clks", "vo-clks";
 			clocks = <&osc>;
 			#clock-cells = <1>;
 		};
-- 
2.34.1

[RFC PATCH v2 16/19] riscv: dts: thead: Add mailbox node

[Michal Wilczynski]

Add mailbox device tree node. This work is based on the vendor kernel [1].

Link: https://github.com/revyos/thead-kernel.git [1]

Reviewed-by: Drew Fustini <dfustini@tenstorrent.com>
Signed-off-by: Michal Wilczynski <m.wilczynski@samsung.com>
---
 arch/riscv/boot/dts/thead/th1520.dtsi | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/arch/riscv/boot/dts/thead/th1520.dtsi b/arch/riscv/boot/dts/thead/th1520.dtsi
index d823b6da75df..0993de0cdd44 100644
--- a/arch/riscv/boot/dts/thead/th1520.dtsi
+++ b/arch/riscv/boot/dts/thead/th1520.dtsi
@@ -601,6 +601,22 @@ timer7: timer@ffffc3303c {
 			status = "disabled";
 		};
 
+		mbox_910t: mailbox@ffffc38000 {
+			compatible = "thead,th1520-mbox";
+			reg = <0xff 0xffc38000 0x0 0x6000>,
+			      <0xff 0xffc40000 0x0 0x6000>,
+			      <0xff 0xffc4c000 0x0 0x2000>,
+			      <0xff 0xffc54000 0x0 0x2000>;
+			reg-names = "local", "remote-icu0", "remote-icu1", "remote-icu2";
+			clocks = <&clk CLK_MBOX0>, <&clk CLK_MBOX1>, <&clk CLK_MBOX2>,
+				 <&clk CLK_MBOX3>;
+			clock-names = "clk-local", "clk-remote-icu0", "clk-remote-icu1",
+				      "clk-remote-icu2";
+			interrupt-parent = <&plic>;
+			interrupts = <28 IRQ_TYPE_LEVEL_HIGH>;
+			#mbox-cells = <1>;
+		};
+
 		gpio@fffff41000 {
 			compatible = "snps,dw-apb-gpio";
 			reg = <0xff 0xfff41000 0x0 0x1000>;
-- 
2.34.1

[RFC PATCH v2 17/19] riscv: dts: thead: Introduce power domain nodes with aon firmware

[Michal Wilczynski]

The DRM Imagination GPU requires a power-domain driver. In the T-HEAD
TH1520 SoC implements power management capabilities through the E902
core, which can be communicated with through the mailbox, using firmware
protocol.

Add AON node, as a parent for the power-domain controller.

Signed-off-by: Michal Wilczynski <m.wilczynski@samsung.com>
---
 arch/riscv/boot/dts/thead/th1520.dtsi | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/arch/riscv/boot/dts/thead/th1520.dtsi b/arch/riscv/boot/dts/thead/th1520.dtsi
index 0993de0cdd44..d4090074fd27 100644
--- a/arch/riscv/boot/dts/thead/th1520.dtsi
+++ b/arch/riscv/boot/dts/thead/th1520.dtsi
@@ -6,6 +6,7 @@
 
 #include <dt-bindings/interrupt-controller/irq.h>
 #include <dt-bindings/clock/thead,th1520-clk-ap.h>
+#include <dt-bindings/power/thead,th1520-power.h>
 
 / {
 	compatible = "thead,th1520";
@@ -229,6 +230,19 @@ stmmac_axi_config: stmmac-axi-config {
 		snps,blen = <0 0 64 32 0 0 0>;
 	};
 
+	firmware {
+		aon: aon {
+			compatible = "thead,th1520-aon";
+			mboxes = <&mbox_910t 1>;
+			mbox-names = "aon";
+
+			pd: power-domain {
+				compatible = "thead,th1520-pd";
+				#power-domain-cells = <1>;
+			};
+		};
+	};
+
 	soc {
 		compatible = "simple-bus";
 		interrupt-parent = <&plic>;
-- 
2.34.1

[RFC PATCH v2 18/19] riscv: dts: thead: Introduce reset controller node

[Michal Wilczynski]

T-HEAD TH1520 SoC requires to put the GPU out of the reset state as part
of the power-up sequence.

Signed-off-by: Michal Wilczynski <m.wilczynski@samsung.com>
---
 arch/riscv/boot/dts/thead/th1520.dtsi | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/arch/riscv/boot/dts/thead/th1520.dtsi b/arch/riscv/boot/dts/thead/th1520.dtsi
index d4090074fd27..c6fe2ea3197d 100644
--- a/arch/riscv/boot/dts/thead/th1520.dtsi
+++ b/arch/riscv/boot/dts/thead/th1520.dtsi
@@ -7,6 +7,7 @@
 #include <dt-bindings/interrupt-controller/irq.h>
 #include <dt-bindings/clock/thead,th1520-clk-ap.h>
 #include <dt-bindings/power/thead,th1520-power.h>
+#include <dt-bindings/reset/thead,th1520-reset.h>
 
 / {
 	compatible = "thead,th1520";
@@ -505,6 +506,12 @@ clk: clock-controller@ffef010000 {
 			#clock-cells = <1>;
 		};
 
+		rst: reset-controller@ffef528000 {
+			compatible = "thead,th1520-reset";
+			reg = <0xff 0xef528000 0x0 0x4f>;
+			#reset-cells = <1>;
+		};
+
 		dmac0: dma-controller@ffefc00000 {
 			compatible = "snps,axi-dma-1.01a";
 			reg = <0xff 0xefc00000 0x0 0x1000>;
-- 
2.34.1

[RFC PATCH v2 19/19] riscv: dts: thead: Add GPU node to TH1520 device tree

[Michal Wilczynski]

Add a device tree node for the IMG BXM-4-64 GPU present in the T-HEAD
TH1520 SoC used by the Lichee Pi 4A board. This node enables support for
the GPU using the drm/imagination driver.

By adding this node, the kernel can recognize and initialize the GPU,
providing graphics acceleration capabilities on the Lichee Pi 4A and
other boards based on the TH1520 SoC.

Signed-off-by: Michal Wilczynski <m.wilczynski@samsung.com>
---
 arch/riscv/boot/dts/thead/th1520.dtsi | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/arch/riscv/boot/dts/thead/th1520.dtsi b/arch/riscv/boot/dts/thead/th1520.dtsi
index c6fe2ea3197d..7e4b97f9e1b5 100644
--- a/arch/riscv/boot/dts/thead/th1520.dtsi
+++ b/arch/riscv/boot/dts/thead/th1520.dtsi
@@ -506,6 +506,19 @@ clk: clock-controller@ffef010000 {
 			#clock-cells = <1>;
 		};
 
+		gpu: gpu@ffef400000 {
+			compatible = "thead,th1520-gpu", "img,img-bxm";
+			reg = <0xff 0xef400000 0x0 0x100000>;
+			interrupt-parent = <&plic>;
+			interrupts = <102 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&clk CLK_GPU_CORE>,
+				 <&clk CLK_GPU_CFG_ACLK>;
+			clock-names = "core", "sys";
+			power-domains = <&pd TH1520_AON_GPU_PD>;
+			resets = <&rst TH1520_RESET_ID_GPU>;
+			reset-names = "gpu";
+		};
+
 		rst: reset-controller@ffef528000 {
 			compatible = "thead,th1520-reset";
 			reg = <0xff 0xef528000 0x0 0x4f>;
-- 
2.34.1