Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
From:   Kevin Hilman <khilman@baylibre.com>
To:     Neil Armstrong <narmstrong@baylibre.com>, jbrunet@baylibre.com
Cc:     linux-arm-kernel@lists.infradead.org,
        linux-amlogic@lists.infradead.org, linux-kernel@vger.kernel.org,
        Neil Armstrong <narmstrong@baylibre.com>
Subject: Re: [RFC 04/11] soc: amlogic: Add support for SM1 power controller
In-Reply-To: <20190701104705.18271-5-narmstrong@baylibre.com>
References: <20190701104705.18271-1-narmstrong@baylibre.com> <20190701104705.18271-5-narmstrong@baylibre.com>
Date:   Mon, 19 Aug 2019 16:56:55 -0700
Message-ID: <7hftlwvhdk.fsf@baylibre.com>
MIME-Version: 1.0
Content-Type: text/plain
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Neil Armstrong <narmstrong@baylibre.com> writes:

> Add support for the General Purpose Amlogic SM1 Power controller,
> dedicated to the PCIe, USB, NNA and GE2D Power Domains.
>
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>

I like this driver in general, but as I look at all the EE power domains
for GX, G12 and SM1 they are really very similar.  I had started to
generalize the gx-pwrc-vpu driver and it ends up looking just like this.

I think this driver could be generalized just a little bit more and then
replace the the GX-specific VPU one, and AFAICT, then be used across all
the 64-bit SoCs, and be called "meson-pwrc-ee" or something like that...

> ---
>  drivers/soc/amlogic/Kconfig          |  11 ++
>  drivers/soc/amlogic/Makefile         |   1 +
>  drivers/soc/amlogic/meson-sm1-pwrc.c | 245 +++++++++++++++++++++++++++
>  3 files changed, 257 insertions(+)
>  create mode 100644 drivers/soc/amlogic/meson-sm1-pwrc.c
>
> diff --git a/drivers/soc/amlogic/Kconfig b/drivers/soc/amlogic/Kconfig
> index 5501ad5650b2..596f1afef1a7 100644
> --- a/drivers/soc/amlogic/Kconfig
> +++ b/drivers/soc/amlogic/Kconfig
> @@ -36,6 +36,17 @@ config MESON_GX_PM_DOMAINS
>  	  Say yes to expose Amlogic Meson GX Power Domains as
>  	  Generic Power Domains.
>  
> +config MESON_SM1_PM_DOMAINS
> +	bool "Amlogic Meson SM1 Power Domains driver"
> +	depends on ARCH_MESON || COMPILE_TEST
> +	depends on PM && OF
> +	default ARCH_MESON
> +	select PM_GENERIC_DOMAINS
> +	select PM_GENERIC_DOMAINS_OF
> +	help
> +	  Say yes to expose Amlogic Meson SM1 Power Domains as
> +	  Generic Power Domains.
> +
>  config MESON_MX_SOCINFO
>  	bool "Amlogic Meson MX SoC Information driver"
>  	depends on ARCH_MESON || COMPILE_TEST
> diff --git a/drivers/soc/amlogic/Makefile b/drivers/soc/amlogic/Makefile
> index bf2d109f61e9..f99935499ee6 100644
> --- a/drivers/soc/amlogic/Makefile
> +++ b/drivers/soc/amlogic/Makefile
> @@ -3,3 +3,4 @@ obj-$(CONFIG_MESON_CLK_MEASURE) += meson-clk-measure.o
>  obj-$(CONFIG_MESON_GX_SOCINFO) += meson-gx-socinfo.o
>  obj-$(CONFIG_MESON_GX_PM_DOMAINS) += meson-gx-pwrc-vpu.o
>  obj-$(CONFIG_MESON_MX_SOCINFO) += meson-mx-socinfo.o
> +obj-$(CONFIG_MESON_SM1_PM_DOMAINS) += meson-sm1-pwrc.o
> diff --git a/drivers/soc/amlogic/meson-sm1-pwrc.c b/drivers/soc/amlogic/meson-sm1-pwrc.c
> new file mode 100644
> index 000000000000..9ece1d06f417
> --- /dev/null
> +++ b/drivers/soc/amlogic/meson-sm1-pwrc.c
> @@ -0,0 +1,245 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (c) 2017 BayLibre, SAS
> + * Author: Neil Armstrong <narmstrong@baylibre.com>
> + */
> +
> +#include <linux/of_address.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm_domain.h>
> +#include <linux/bitfield.h>
> +#include <linux/regmap.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/of_device.h>
> +#include <dt-bindings/power/meson-sm1-power.h>
> +
> +/* AO Offsets */
> +
> +#define AO_RTI_GEN_PWR_SLEEP0		(0x3a << 2)
> +#define AO_RTI_GEN_PWR_ISO0		(0x3b << 2)
> +
> +/* HHI Offsets */
> +
> +#define HHI_MEM_PD_REG0			(0x40 << 2)
> +#define HHI_NANOQ_MEM_PD_REG0		(0x46 << 2)
> +#define HHI_NANOQ_MEM_PD_REG1		(0x47 << 2)
> +
> +struct meson_sm1_pwrc;
> +
> +struct meson_sm1_pwrc_mem_domain {
> +	unsigned int reg;
> +	unsigned int mask;
> +};
> +
> +struct meson_sm1_pwrc_domain_desc {
> +	char *name;
> +	unsigned int sleep_reg;
> +	unsigned int sleep_bit;
> +	unsigned int iso_reg;
> +	unsigned int iso_bit;
> +	unsigned int mem_pd_count;
> +	struct meson_sm1_pwrc_mem_domain *mem_pd;
> +};

If you add resets and clocks (using clk bulk like my other proposed
patch to gx-pwrc-vpu) then this could be used for VPU also.  We could
ignore my clk bulk patch and then just deprecate the old driver and use
this one for everything.

We would just need SoC-specific tables selected by compatible-string to
select the memory pds, and the clocks and resets could (optionaly) come
from the DT.

Kevin