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

Kevin Hilman
2019-08-20T19:19:15Z

Neil Armstrong <narmstrong@baylibre.com> writes:

> 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 ?

You currently create all these SoC-specific `mem_domain` tables. We'll
need more of those for the other SoCs, so my suggestion was that, in
order to use this across multiple SoCs, you select the set of mem_domain
tables based on compatible string.

That was just my first idea. If you have a better idea, I'm open to
that too.

> Let me try to fit the VPU stuff in it.

Great, thanks!

Kevin