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

Neil Armstrong
2019-08-20T14:55:51Z

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

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@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.

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
>