Re: [RFC 04/11] soc: amlogic: Add support for SM1 power controller

[Neil Armstrong]

On 20/08/2019 01:56, Kevin Hilman wrote:
> Neil Armstrong <narmstrong@xxxxxxxxxxxx> 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@xxxxxxxxxxxx>
> 
> 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.

Yes I developed it to be generic, but when starting to fill up the GXBB/GXL/G12A
domains, except the VPU, they only need the PD parts.

> 
> 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@xxxxxxxxxxxx>
>> + */
>> +
>> +#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.

Could you elaborate ?

Do you mean I should slit out the memory PDs as different compatible ?

Let me try to fit the VPU stuff in it.

Neil

> 
> Kevin
>