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

[Krzysztof Kozlowski <krzk@kernel.org>]

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