Neil Armstrong <[email protected]> writes: > On 20/08/2019 01:56, Kevin Hilman wrote: >> Neil Armstrong <[email protected]> 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 <[email protected]> >> >> 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 <[email protected]> >>> + */ >>> + >>> +#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
