Hello,
This patchset adds PMU (Power Management Unit) controller driver for the
StarFive JH7110 SoC. In order to meet low power requirements, PMU is
designed for including multiple PM domains that can be used for power
gating of selected IP blocks for power saving by reduced leakage
current. The first patch adds device tree binding for PM domain provider
and consumer. The second patch adds pmu driver and support JH7110 SoC.
The last patch adds device node about pmu to JH7110 dts.
The series has been tested on the VisionFive 2 boards which equip with
JH7110 SoC and works normally.
The last patch should be applied after the following patchset:
https://lore.kernel.org/all/[email protected]/
Changes in v3:
- Rebased on tag v6.1.
- Renamed the dt-bindings 'starfive,jh71xx-power.yaml' to
'starfive,jh7110-pmu.yaml' which is matching compatible.
- Fixed wrong indentation and error when running 'make dt_binding_check'
in dt-bindings.
- Changed the license of the dt-bindings header to be same with
dt-bindings.
- Changed a little bit on dependency conditions in Kconfig of driver.
- Dropped some macros that are temporarily useless.
- Simplified the definition of macro 'JH71XX_PMU_INT_ALL_MASK'.
- Changed the sorting of structure members, such as 'struct
jh71xx_domain_info', 'struct jh71xx_pmu', etc.
- Modified detailed comment about controlling power domain.
- Dropped useless comment when running 'platform_get_irq'.
v2 - https://lore.kernel.org/all/[email protected]/
Changes in v2:
- Squashed 1st patch (dt-bindings header) into 2nd which is related to
dt-bindings stuff.
- Renamed the dt-bindings header 'jh7110-power.h' to
'starfive,jh7110-pmu.h' and used dual license for it.
- Renamed the dt-bindings 'starfive,jh71xx-power.yaml' to
'starfive,jh71xx-pmu.yaml', dropped items from properties.
- Change of MAINTAINERS: added the entry of 'starfive soc drivers';
changed status to 'Supported' for the entry of
'STARFIVE JH71XX PMU CONTROLLER DRIVER' and sorted the lines alphabetically.
- Dropped the header file 'include/soc/starfive/pm_domains.h'.
- Dropped starfive_pmu_hw_event_turn_on() and starfive_pmu_hw_event_turn_off().
- Added 'default SOC_STARFIVE' and expanded help text in the Kconfig.
- Added a JH71XX_PMU_ prefix to those macro definitions in driver.
- Replaced the data type 'uint8_t / uint32_t' with 'u8 / u32'.
- Fixed some complains by using checkpatch.pl
- Added spinlock to jh71xx_pmu_int_enable().
- Dropped spinlock from jh71xx_pmu_interrupt().
- Used jh71xx_pmu_ as prefix to all functions.
- Replaced io accessors '__raw_readl / __raw_writel' with 'readl / writel'.
- Added jh71xx_pmu_get_state() to the beginning of jh71xx_pmu_set_state().
- Added more detailed comment about controlling power domain.
- Simplified the usage of loop when performing pm_genpd_init() to register
power domain.
- Added more detailed description about the features of power domain
hardware to commit message in 2nd patch.
- Replaced dev_info() with dev_dbg() in jh71xx_pmu_set_state().
- Decreased the timeout numbers of polling power status when switching
power mode.
v1 - https://lore.kernel.org/all/[email protected]/
Best regards,
Walker
Walker Chen (3):
dt-bindings: power: Add starfive,jh7110-pmu
soc: starfive: Add StarFive JH71XX pmu driver
riscv: dts: starfive: add pmu controller node
.../bindings/power/starfive,jh7110-pmu.yaml | 45 ++
MAINTAINERS | 14 +
arch/riscv/boot/dts/starfive/jh7110.dtsi | 7 +
drivers/soc/Kconfig | 1 +
drivers/soc/Makefile | 1 +
drivers/soc/starfive/Kconfig | 12 +
drivers/soc/starfive/Makefile | 3 +
drivers/soc/starfive/jh71xx_pmu.c | 384 ++++++++++++++++++
.../dt-bindings/power/starfive,jh7110-pmu.h | 17 +
9 files changed, 484 insertions(+)
create mode 100644 Documentation/devicetree/bindings/power/starfive,jh7110-pmu.yaml
create mode 100644 drivers/soc/starfive/Kconfig
create mode 100644 drivers/soc/starfive/Makefile
create mode 100644 drivers/soc/starfive/jh71xx_pmu.c
create mode 100644 include/dt-bindings/power/starfive,jh7110-pmu.h
base-commit: 830b3c68c1fb1e9176028d02ef86f3cf76aa2476
prerequisite-patch-id: 54ce870d6ea747466474b5d4105cfbc05e1b01ab
prerequisite-patch-id: e8dd8258a4c4062eee2cf07c4607d52baea71f3a
prerequisite-patch-id: 057fa35870d8d7d22a57c13362588ffb9e9df316
prerequisite-patch-id: 102368a6ff799c4cb639aed513deff09c1839161
prerequisite-patch-id: 7c1a50a37919fedbbd336ca5dec295ac63c2a89d
prerequisite-patch-id: a5d9e0f7d4f8163f566678894cf693015119f2d9
prerequisite-patch-id: 87cb528acd9a7f1ffe7475d7261553f6a4de5753
prerequisite-patch-id: 417736eb958e1158c60a5ed74bc2350394321a80
prerequisite-patch-id: a137312ca162b5712e28719f77d0da78e9fdd778
prerequisite-patch-id: f7c548b4619f491ce27f319242c4e3685c76173b
prerequisite-patch-id: 4d90febab2fb7928f50a73104e7454312b9ce6c8
prerequisite-patch-id: 645a807d50e0e56593ffdc6c3b50ea54a230827a
prerequisite-patch-id: 165f8cd740ae60585d22c95b99a0689084d468e3
prerequisite-patch-id: 480d910deccadc2947b3318c3c13dfa0882c8e0d
prerequisite-patch-id: 1d1cb90ec12dfc9312e448759c7cab89f2bc6394
prerequisite-patch-id: 5f539ac7c96023b36489c6da7c70c31eaf64a25b
prerequisite-patch-id: 6bb9a780c62af3bcc2368dfd20303c7b1bc91e23
prerequisite-patch-id: 258ea5f9b8bf41b6981345dcc81795f25865d38f
prerequisite-patch-id: 8b6f2c9660c0ac0ee4e73e4c21aca8e6b75e81b9
prerequisite-patch-id: e3b986b9c60b2b93b7812ec174c9e1b4cfb14c97
prerequisite-patch-id: 2e03eeb766aefd5d38f132d091618e9fa19a37b6
prerequisite-patch-id: e0ba7af0f8d3d41844da9fbcba14b548cbc18f55
prerequisite-patch-id: c1f8603e58c64828d0f36deac9b93c24289d8e05
--
2.17.1
Add pmu driver for the StarFive JH71XX SoC.
As the power domains provider, the Power Management Unit (PMU) is
designed for including multiple PM domains that can be used for power
gating of selected IP blocks for power saving by reduced leakage
current. It accepts software encourage command to switch the power mode
of SoC.
Signed-off-by: Walker Chen <[email protected]>
---
MAINTAINERS | 14 ++
drivers/soc/Kconfig | 1 +
drivers/soc/Makefile | 1 +
drivers/soc/starfive/Kconfig | 12 +
drivers/soc/starfive/Makefile | 3 +
drivers/soc/starfive/jh71xx_pmu.c | 384 ++++++++++++++++++++++++++++++
6 files changed, 415 insertions(+)
create mode 100644 drivers/soc/starfive/Kconfig
create mode 100644 drivers/soc/starfive/Makefile
create mode 100644 drivers/soc/starfive/jh71xx_pmu.c
diff --git a/MAINTAINERS b/MAINTAINERS
index 85e8f83161d7..84fd7054cb6e 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -19659,6 +19659,20 @@ F: Documentation/devicetree/bindings/reset/starfive,jh7100-reset.yaml
F: drivers/reset/starfive/reset-starfive-jh71*
F: include/dt-bindings/reset/starfive?jh71*.h
+STARFIVE SOC DRIVER
+M: Conor Dooley <[email protected]>
+S: Maintained
+T: git https://git.kernel.org/pub/scm/linux/kernel/git/conor/linux.git/
+F: drivers/soc/starfive/
+F: include/soc/starfive/
+
+STARFIVE JH71XX PMU CONTROLLER DRIVER
+M: Walker Chen <[email protected]>
+S: Supported
+F: Documentation/devicetree/bindings/power/starfive*
+F: drivers/soc/starfive/jh71xx_pmu.c
+F: include/dt-bindings/power/starfive,jh7110-pmu.h
+
STATIC BRANCH/CALL
M: Peter Zijlstra <[email protected]>
M: Josh Poimboeuf <[email protected]>
diff --git a/drivers/soc/Kconfig b/drivers/soc/Kconfig
index e461c071189b..628fda4d5ed9 100644
--- a/drivers/soc/Kconfig
+++ b/drivers/soc/Kconfig
@@ -21,6 +21,7 @@ source "drivers/soc/renesas/Kconfig"
source "drivers/soc/rockchip/Kconfig"
source "drivers/soc/samsung/Kconfig"
source "drivers/soc/sifive/Kconfig"
+source "drivers/soc/starfive/Kconfig"
source "drivers/soc/sunxi/Kconfig"
source "drivers/soc/tegra/Kconfig"
source "drivers/soc/ti/Kconfig"
diff --git a/drivers/soc/Makefile b/drivers/soc/Makefile
index 534669840858..cbe076f42068 100644
--- a/drivers/soc/Makefile
+++ b/drivers/soc/Makefile
@@ -27,6 +27,7 @@ obj-y += renesas/
obj-y += rockchip/
obj-$(CONFIG_SOC_SAMSUNG) += samsung/
obj-y += sifive/
+obj-y += starfive/
obj-y += sunxi/
obj-$(CONFIG_ARCH_TEGRA) += tegra/
obj-y += ti/
diff --git a/drivers/soc/starfive/Kconfig b/drivers/soc/starfive/Kconfig
new file mode 100644
index 000000000000..bdb96dc4c989
--- /dev/null
+++ b/drivers/soc/starfive/Kconfig
@@ -0,0 +1,12 @@
+# SPDX-License-Identifier: GPL-2.0
+
+config JH71XX_PMU
+ bool "Support PMU for StarFive JH71XX Soc"
+ depends on PM
+ depends on SOC_STARFIVE || COMPILE_TEST
+ default SOC_STARFIVE
+ select PM_GENERIC_DOMAINS
+ help
+ Say 'y' here to enable support power domain support.
+ In order to meet low power requirements, a Power Management Unit (PMU)
+ is designed for controlling power resources in StarFive JH71XX SoCs.
diff --git a/drivers/soc/starfive/Makefile b/drivers/soc/starfive/Makefile
new file mode 100644
index 000000000000..13b589d6b5f3
--- /dev/null
+++ b/drivers/soc/starfive/Makefile
@@ -0,0 +1,3 @@
+# SPDX-License-Identifier: GPL-2.0
+
+obj-$(CONFIG_JH71XX_PMU) += jh71xx_pmu.o
diff --git a/drivers/soc/starfive/jh71xx_pmu.c b/drivers/soc/starfive/jh71xx_pmu.c
new file mode 100644
index 000000000000..c1869969d9ba
--- /dev/null
+++ b/drivers/soc/starfive/jh71xx_pmu.c
@@ -0,0 +1,384 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * StarFive JH71XX PMU (Power Management Unit) Controller Driver
+ *
+ * Copyright (C) 2022 StarFive Technology Co., Ltd.
+ */
+
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/iopoll.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/pm_domain.h>
+#include <dt-bindings/power/starfive,jh7110-pmu.h>
+
+/* register offset */
+#define JH71XX_PMU_SW_TURN_ON_POWER 0x0C
+#define JH71XX_PMU_SW_TURN_OFF_POWER 0x10
+#define JH71XX_PMU_SW_ENCOURAGE 0x44
+#define JH71XX_PMU_TIMER_INT_MASK 0x48
+#define JH71XX_PMU_CURR_POWER_MODE 0x80
+#define JH71XX_PMU_EVENT_STATUS 0x88
+#define JH71XX_PMU_INT_STATUS 0x8C
+
+/* sw encourage cfg */
+#define JH71XX_PMU_SW_ENCOURAGE_EN_LO 0x05
+#define JH71XX_PMU_SW_ENCOURAGE_EN_HI 0x50
+#define JH71XX_PMU_SW_ENCOURAGE_DIS_LO 0x0A
+#define JH71XX_PMU_SW_ENCOURAGE_DIS_HI 0xA0
+#define JH71XX_PMU_SW_ENCOURAGE_ON 0xFF
+
+/* pmu int status */
+#define JH71XX_PMU_INT_SEQ_DONE BIT(0)
+#define JH71XX_PMU_INT_HW_REQ BIT(1)
+#define JH71XX_PMU_INT_SW_FAIL GENMASK(3, 2)
+#define JH71XX_PMU_INT_HW_FAIL GENMASK(5, 4)
+#define JH71XX_PMU_INT_PCH_FAIL GENMASK(8, 6)
+#define JH71XX_PMU_INT_ALL_MASK GENMASK(8, 0)
+
+/*
+ * The time required for switching power status is based on the time
+ * to turn on the largest domain's power, which is at microsecond level
+ */
+#define JH71XX_PMU_TIMEOUT_US 100
+
+struct jh71xx_domain_info {
+ const char * const name;
+ unsigned int flags;
+ u8 bit;
+};
+
+struct jh71xx_pmu_match_data {
+ const struct jh71xx_domain_info *domain_info;
+ int num_domains;
+};
+
+struct jh71xx_pmu {
+ struct device *dev;
+ const struct jh71xx_pmu_match_data *match_data;
+ void __iomem *base;
+ struct generic_pm_domain **genpd;
+ struct genpd_onecell_data genpd_data;
+ int irq;
+ spinlock_t lock; /* protects pmu reg */
+};
+
+struct jh71xx_pmu_dev {
+ const struct jh71xx_domain_info *domain_info;
+ struct jh71xx_pmu *pmu;
+ struct generic_pm_domain genpd;
+};
+
+static int jh71xx_pmu_get_state(struct jh71xx_pmu_dev *pmd, u32 mask, bool *is_on)
+{
+ struct jh71xx_pmu *pmu = pmd->pmu;
+
+ if (!mask) {
+ *is_on = false;
+ return -EINVAL;
+ }
+
+ *is_on = readl(pmu->base + JH71XX_PMU_CURR_POWER_MODE) & mask;
+
+ return 0;
+}
+
+static int jh71xx_pmu_set_state(struct jh71xx_pmu_dev *pmd, u32 mask, bool on)
+{
+ struct jh71xx_pmu *pmu = pmd->pmu;
+ unsigned long flags;
+ u32 val;
+ u32 mode;
+ u32 encourage_lo;
+ u32 encourage_hi;
+ bool is_on;
+ int ret;
+
+ ret = jh71xx_pmu_get_state(pmd, mask, &is_on);
+ if (ret) {
+ dev_dbg(pmu->dev, "unable to get current state for %s\n",
+ pmd->genpd.name);
+ return ret;
+ }
+
+ if (is_on == on) {
+ dev_dbg(pmu->dev, "pm domain [%s] is already %sable status.\n",
+ pmd->genpd.name, on ? "en" : "dis");
+ return 0;
+ }
+
+ spin_lock_irqsave(&pmu->lock, flags);
+
+ /*
+ * The PMU accepts software encourage to switch power mode in the following 2 steps:
+ *
+ * 1.Configure the register SW_TURN_ON_POWER (offset 0x0c) by writing 1 to
+ * the bit corresponding to the power domain that will be turned on
+ * and writing 0 to the others.
+ * Likewise, configure the register SW_TURN_OFF_POWER (offset 0x10) by
+ * writing 1 to the bit corresponding to the power domain that will be
+ * turned off and writing 0 to the others.
+ */
+ if (on) {
+ mode = JH71XX_PMU_SW_TURN_ON_POWER;
+ encourage_lo = JH71XX_PMU_SW_ENCOURAGE_EN_LO;
+ encourage_hi = JH71XX_PMU_SW_ENCOURAGE_EN_HI;
+ } else {
+ mode = JH71XX_PMU_SW_TURN_OFF_POWER;
+ encourage_lo = JH71XX_PMU_SW_ENCOURAGE_DIS_LO;
+ encourage_hi = JH71XX_PMU_SW_ENCOURAGE_DIS_HI;
+ }
+
+ writel(mask, pmu->base + mode);
+
+ /*
+ * 2.Write SW encourage command sequence to the Software Encourage Reg (offset 0x44)
+ * First write SW_MODE_ENCOURAGE_ON to JH71XX_PMU_SW_ENCOURAGE. This will reset
+ * the state machine which parses the command sequence. This register must be
+ * written every time software wants to power on/off a domain.
+ * Then write the lower bits of the command sequence, followed by the upper
+ * bits. The sequence differs between powering on & off a domain.
+ */
+ writel(JH71XX_PMU_SW_ENCOURAGE_ON, pmu->base + JH71XX_PMU_SW_ENCOURAGE);
+ writel(encourage_lo, pmu->base + JH71XX_PMU_SW_ENCOURAGE);
+ writel(encourage_hi, pmu->base + JH71XX_PMU_SW_ENCOURAGE);
+
+ spin_unlock_irqrestore(&pmu->lock, flags);
+
+ /* Wait for the power domain bit to be enabled / disabled */
+ if (on) {
+ ret = readl_poll_timeout_atomic(pmu->base + JH71XX_PMU_CURR_POWER_MODE,
+ val, val & mask,
+ 1, JH71XX_PMU_TIMEOUT_US);
+ } else {
+ ret = readl_poll_timeout_atomic(pmu->base + JH71XX_PMU_CURR_POWER_MODE,
+ val, !(val & mask),
+ 1, JH71XX_PMU_TIMEOUT_US);
+ }
+
+ if (ret) {
+ dev_err(pmu->dev, "%s: failed to power %s\n",
+ pmd->genpd.name, on ? "on" : "off");
+ return -ETIMEDOUT;
+ }
+
+ return 0;
+}
+
+static int jh71xx_pmu_on(struct generic_pm_domain *genpd)
+{
+ struct jh71xx_pmu_dev *pmd = container_of(genpd,
+ struct jh71xx_pmu_dev, genpd);
+ u32 pwr_mask = BIT(pmd->domain_info->bit);
+
+ return jh71xx_pmu_set_state(pmd, pwr_mask, true);
+}
+
+static int jh71xx_pmu_off(struct generic_pm_domain *genpd)
+{
+ struct jh71xx_pmu_dev *pmd = container_of(genpd,
+ struct jh71xx_pmu_dev, genpd);
+ u32 pwr_mask = BIT(pmd->domain_info->bit);
+
+ return jh71xx_pmu_set_state(pmd, pwr_mask, false);
+}
+
+static void jh71xx_pmu_int_enable(struct jh71xx_pmu *pmu, u32 mask, bool enable)
+{
+ u32 val;
+ unsigned long flags;
+
+ spin_lock_irqsave(&pmu->lock, flags);
+ val = readl(pmu->base + JH71XX_PMU_TIMER_INT_MASK);
+
+ if (enable)
+ val &= ~mask;
+ else
+ val |= mask;
+
+ writel(val, pmu->base + JH71XX_PMU_TIMER_INT_MASK);
+ spin_unlock_irqrestore(&pmu->lock, flags);
+}
+
+static irqreturn_t jh71xx_pmu_interrupt(int irq, void *data)
+{
+ struct jh71xx_pmu *pmu = data;
+ u32 val;
+
+ val = readl(pmu->base + JH71XX_PMU_INT_STATUS);
+
+ if (val & JH71XX_PMU_INT_SEQ_DONE)
+ dev_dbg(pmu->dev, "sequence done.\n");
+ if (val & JH71XX_PMU_INT_HW_REQ)
+ dev_dbg(pmu->dev, "hardware encourage requestion.\n");
+ if (val & JH71XX_PMU_INT_SW_FAIL)
+ dev_err(pmu->dev, "software encourage fail.\n");
+ if (val & JH71XX_PMU_INT_HW_FAIL)
+ dev_err(pmu->dev, "hardware encourage fail.\n");
+ if (val & JH71XX_PMU_INT_PCH_FAIL)
+ dev_err(pmu->dev, "p-channel fail event.\n");
+
+ /* clear interrupts */
+ writel(val, pmu->base + JH71XX_PMU_INT_STATUS);
+ writel(val, pmu->base + JH71XX_PMU_EVENT_STATUS);
+
+ return IRQ_HANDLED;
+}
+
+static int jh71xx_pmu_init_domain(struct jh71xx_pmu *pmu, int index)
+{
+ struct jh71xx_pmu_dev *pmd;
+ bool is_on;
+ u32 pwr_mask;
+ int ret;
+
+ pmd = devm_kzalloc(pmu->dev, sizeof(*pmd), GFP_KERNEL);
+ if (!pmd)
+ return -ENOMEM;
+
+ pmd->domain_info = &pmu->match_data->domain_info[index];
+ pmd->pmu = pmu;
+ pwr_mask = BIT(pmd->domain_info->bit);
+
+ pmd->genpd.name = pmd->domain_info->name;
+ pmd->genpd.flags = pmd->domain_info->flags;
+
+ ret = jh71xx_pmu_get_state(pmd, pwr_mask, &is_on);
+ if (ret)
+ dev_warn(pmu->dev, "unable to get current state for %s\n",
+ pmd->genpd.name);
+
+ pmd->genpd.power_on = jh71xx_pmu_on;
+ pmd->genpd.power_off = jh71xx_pmu_off;
+ pm_genpd_init(&pmd->genpd, NULL, !is_on);
+
+ pmu->genpd_data.domains[index] = &pmd->genpd;
+
+ return 0;
+}
+
+static int jh71xx_pmu_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct device_node *np = dev->of_node;
+ const struct jh71xx_pmu_match_data *match_data;
+ struct jh71xx_pmu *pmu;
+ unsigned int i;
+ int ret;
+
+ pmu = devm_kzalloc(dev, sizeof(*pmu), GFP_KERNEL);
+ if (!pmu)
+ return -ENOMEM;
+
+ pmu->base = devm_platform_ioremap_resource(pdev, 0);
+ if (IS_ERR(pmu->base))
+ return PTR_ERR(pmu->base);
+
+ pmu->irq = platform_get_irq(pdev, 0);
+ if (pmu->irq < 0)
+ return pmu->irq;
+
+ ret = devm_request_irq(dev, pmu->irq, jh71xx_pmu_interrupt,
+ 0, pdev->name, pmu);
+ if (ret)
+ dev_err(dev, "failed to request irq\n");
+
+ match_data = of_device_get_match_data(dev);
+ if (!match_data)
+ return -EINVAL;
+
+ pmu->genpd = devm_kcalloc(dev, match_data->num_domains,
+ sizeof(struct generic_pm_domain *),
+ GFP_KERNEL);
+ if (!pmu->genpd)
+ return -ENOMEM;
+
+ pmu->dev = dev;
+ pmu->match_data = match_data;
+ pmu->genpd_data.domains = pmu->genpd;
+ pmu->genpd_data.num_domains = match_data->num_domains;
+
+ for (i = 0; i < match_data->num_domains; i++) {
+ ret = jh71xx_pmu_init_domain(pmu, i);
+ if (ret) {
+ dev_err(dev, "failed to initialize power domain\n");
+ return ret;
+ }
+ }
+
+ spin_lock_init(&pmu->lock);
+ jh71xx_pmu_int_enable(pmu, JH71XX_PMU_INT_ALL_MASK & ~JH71XX_PMU_INT_PCH_FAIL, true);
+
+ ret = of_genpd_add_provider_onecell(np, &pmu->genpd_data);
+ if (ret) {
+ dev_err(dev, "failed to register genpd driver: %d\n", ret);
+ return ret;
+ }
+
+ dev_info(dev, "registered %u power domains\n", i);
+
+ return 0;
+}
+
+static const struct jh71xx_domain_info jh7110_power_domains[] = {
+ [JH7110_PD_SYSTOP] = {
+ .name = "SYSTOP",
+ .bit = 0,
+ .flags = GENPD_FLAG_ALWAYS_ON,
+ },
+ [JH7110_PD_CPU] = {
+ .name = "CPU",
+ .bit = 1,
+ .flags = GENPD_FLAG_ALWAYS_ON,
+ },
+ [JH7110_PD_GPUA] = {
+ .name = "GPUA",
+ .bit = 2,
+ },
+ [JH7110_PD_VDEC] = {
+ .name = "VDEC",
+ .bit = 3,
+ },
+ [JH7110_PD_VOUT] = {
+ .name = "VOUT",
+ .bit = 4,
+ },
+ [JH7110_PD_ISP] = {
+ .name = "ISP",
+ .bit = 5,
+ },
+ [JH7110_PD_VENC] = {
+ .name = "VENC",
+ .bit = 6,
+ },
+};
+
+static const struct jh71xx_pmu_match_data jh7110_pmu = {
+ .num_domains = ARRAY_SIZE(jh7110_power_domains),
+ .domain_info = jh7110_power_domains,
+};
+
+static const struct of_device_id jh71xx_pmu_of_match[] = {
+ {
+ .compatible = "starfive,jh7110-pmu",
+ .data = (void *)&jh7110_pmu,
+ }, {
+ /* sentinel */
+ }
+};
+
+static struct platform_driver jh71xx_pmu_driver = {
+ .driver = {
+ .name = "jh71xx-pmu",
+ .of_match_table = jh71xx_pmu_of_match,
+ },
+ .probe = jh71xx_pmu_probe,
+};
+builtin_platform_driver(jh71xx_pmu_driver);
+
+MODULE_AUTHOR("Walker Chen <[email protected]>");
+MODULE_DESCRIPTION("StarFive JH71XX PMU Driver");
+MODULE_LICENSE("GPL");
--
2.17.1
Hey Walker,
On Mon, Jan 16, 2023 at 03:42:56PM +0800, Walker Chen wrote:
> Hello,
>
> This patchset adds PMU (Power Management Unit) controller driver for the
> StarFive JH7110 SoC. In order to meet low power requirements, PMU is
> designed for including multiple PM domains that can be used for power
> gating of selected IP blocks for power saving by reduced leakage
> current. The first patch adds device tree binding for PM domain provider
> and consumer. The second patch adds pmu driver and support JH7110 SoC.
> The last patch adds device node about pmu to JH7110 dts.
>
> The series has been tested on the VisionFive 2 boards which equip with
> JH7110 SoC and works normally.
For the series:
Reviewed-by: Conor Dooley <[email protected]>
I'm hoping that someone with knowledge of the power APIs will take a
look now that the driver looks to be in a pretty good state (to my naive
eyes at least).
> Changes in v3:
> - Rebased on tag v6.1.
FYI, please pick something more recent than that.
Ideally, the last -rc1, which in this case is v6.2-rc1.
It's helpful to do this, as when I went to apply your patch, there were
some conflicts that needed to be sorted out. Because of your prerequisite
patches, the usual `b4` commands would not usable. E.g.
b4 am -3 [email protected]
Analyzing 4 messages in the thread
Checking attestation on all messages, may take a moment...
---
[PATCH v3 1/3] dt-bindings: power: Add starfive,jh7110-pmu
[PATCH v3 2/3] soc: starfive: Add StarFive JH71XX pmu driver
[PATCH v3 3/3] riscv: dts: starfive: add pmu controller node
---
Total patches: 3
Preparing fake-am for v3: JH7110 PMU Support
ERROR: Could not find matching blob for MAINTAINERS (85e8f83161d7)
If you know on which tree this patchset is based,
add it as a remote and perform "git remote update"
in order to fetch the missing objects.
Fortunately, this is just a driver addition so despite `b4` not
helping it was easy to resolve but for other patches in the future,
this may not be the case.
Assuming the dt maintainers are happy with the binding, ping me in 2
weeks if no-one else has commented and I'll apply patches 1 & 2.
Thanks,
Conor.
> base-commit: 830b3c68c1fb1e9176028d02ef86f3cf76aa2476
> prerequisite-patch-id: 54ce870d6ea747466474b5d4105cfbc05e1b01ab
> prerequisite-patch-id: e8dd8258a4c4062eee2cf07c4607d52baea71f3a
> prerequisite-patch-id: 057fa35870d8d7d22a57c13362588ffb9e9df316
> prerequisite-patch-id: 102368a6ff799c4cb639aed513deff09c1839161
> prerequisite-patch-id: 7c1a50a37919fedbbd336ca5dec295ac63c2a89d
> prerequisite-patch-id: a5d9e0f7d4f8163f566678894cf693015119f2d9
> prerequisite-patch-id: 87cb528acd9a7f1ffe7475d7261553f6a4de5753
> prerequisite-patch-id: 417736eb958e1158c60a5ed74bc2350394321a80
> prerequisite-patch-id: a137312ca162b5712e28719f77d0da78e9fdd778
> prerequisite-patch-id: f7c548b4619f491ce27f319242c4e3685c76173b
> prerequisite-patch-id: 4d90febab2fb7928f50a73104e7454312b9ce6c8
> prerequisite-patch-id: 645a807d50e0e56593ffdc6c3b50ea54a230827a
> prerequisite-patch-id: 165f8cd740ae60585d22c95b99a0689084d468e3
> prerequisite-patch-id: 480d910deccadc2947b3318c3c13dfa0882c8e0d
> prerequisite-patch-id: 1d1cb90ec12dfc9312e448759c7cab89f2bc6394
> prerequisite-patch-id: 5f539ac7c96023b36489c6da7c70c31eaf64a25b
> prerequisite-patch-id: 6bb9a780c62af3bcc2368dfd20303c7b1bc91e23
> prerequisite-patch-id: 258ea5f9b8bf41b6981345dcc81795f25865d38f
> prerequisite-patch-id: 8b6f2c9660c0ac0ee4e73e4c21aca8e6b75e81b9
> prerequisite-patch-id: e3b986b9c60b2b93b7812ec174c9e1b4cfb14c97
> prerequisite-patch-id: 2e03eeb766aefd5d38f132d091618e9fa19a37b6
> prerequisite-patch-id: e0ba7af0f8d3d41844da9fbcba14b548cbc18f55
> prerequisite-patch-id: c1f8603e58c64828d0f36deac9b93c24289d8e05
On 2023/1/17 3:19, Conor Dooley wrote:
> Hey Walker,
>
> On Mon, Jan 16, 2023 at 03:42:56PM +0800, Walker Chen wrote:
>> Hello,
>>
>> This patchset adds PMU (Power Management Unit) controller driver for the
>> StarFive JH7110 SoC. In order to meet low power requirements, PMU is
>> designed for including multiple PM domains that can be used for power
>> gating of selected IP blocks for power saving by reduced leakage
>> current. The first patch adds device tree binding for PM domain provider
>> and consumer. The second patch adds pmu driver and support JH7110 SoC.
>> The last patch adds device node about pmu to JH7110 dts.
>>
>> The series has been tested on the VisionFive 2 boards which equip with
>> JH7110 SoC and works normally.
>
> For the series:
> Reviewed-by: Conor Dooley <[email protected]>
>
> I'm hoping that someone with knowledge of the power APIs will take a
> look now that the driver looks to be in a pretty good state (to my naive
> eyes at least).
>
>> Changes in v3:
>> - Rebased on tag v6.1.
>
> FYI, please pick something more recent than that.
> Ideally, the last -rc1, which in this case is v6.2-rc1.
OK, the next version will be rebased to the latest kernel.
> It's helpful to do this, as when I went to apply your patch, there were
> some conflicts that needed to be sorted out. Because of your prerequisite
> patches, the usual `b4` commands would not usable. E.g.
>
> b4 am -3 [email protected]
> Analyzing 4 messages in the thread
> Checking attestation on all messages, may take a moment...
> ---
> [PATCH v3 1/3] dt-bindings: power: Add starfive,jh7110-pmu
> [PATCH v3 2/3] soc: starfive: Add StarFive JH71XX pmu driver
> [PATCH v3 3/3] riscv: dts: starfive: add pmu controller node
> ---
> Total patches: 3
> Preparing fake-am for v3: JH7110 PMU Support
> ERROR: Could not find matching blob for MAINTAINERS (85e8f83161d7)
> If you know on which tree this patchset is based,
> add it as a remote and perform "git remote update"
> in order to fetch the missing objects.
>
> Fortunately, this is just a driver addition so despite `b4` not
> helping it was easy to resolve but for other patches in the future,
> this may not be the case.
>
> Assuming the dt maintainers are happy with the binding, ping me in 2
> weeks if no-one else has commented and I'll apply patches 1 & 2.
Could I drop patch 3 and rebase patch 1 & 2 on the latest mainline then submit as v4 ?
Best regards,
Walker
Hey Walker,
On Tue, Jan 17, 2023 at 03:45:22PM +0800, Walker Chen wrote:
> On 2023/1/17 3:19, Conor Dooley wrote:
> > It's helpful to do this, as when I went to apply your patch, there were
> > some conflicts that needed to be sorted out. Because of your prerequisite
> > patches, the usual `b4` commands would not usable. E.g.
> >
> > b4 am -3 [email protected]
> > Analyzing 4 messages in the thread
> > Checking attestation on all messages, may take a moment...
> > ---
> > [PATCH v3 1/3] dt-bindings: power: Add starfive,jh7110-pmu
> > [PATCH v3 2/3] soc: starfive: Add StarFive JH71XX pmu driver
> > [PATCH v3 3/3] riscv: dts: starfive: add pmu controller node
> > ---
> > Total patches: 3
> > Preparing fake-am for v3: JH7110 PMU Support
> > ERROR: Could not find matching blob for MAINTAINERS (85e8f83161d7)
> > If you know on which tree this patchset is based,
> > add it as a remote and perform "git remote update"
> > in order to fetch the missing objects.
> >
> > Fortunately, this is just a driver addition so despite `b4` not
> > helping it was easy to resolve but for other patches in the future,
> > this may not be the case.
> >
> > Assuming the dt maintainers are happy with the binding, ping me in 2
> > weeks if no-one else has commented and I'll apply patches 1 & 2.
>
> Could I drop patch 3 and rebase patch 1 & 2 on the latest mainline then submit as v4 ?
I dunno, if someone has the pre-requisites then they should be okay, but
I need to apply 1 & 2 without the pre-requisites.
I dropped patch 3 & did the rebase myself so that the build bots would
be able to build the patches:
https://git.kernel.org/pub/scm/linux/kernel/git/conor/linux.git/log/?h=riscv-jh7110_pmu
I got the build success email this morning, so they appear happy with
it.
I would wait for some comments from the dt maintainers on patch 1 before
considering re-sending the series. If they have comments, then please do
rebase on v6.2
Thanks,
Conor.
Hi Walker,
Am Montag, 16. Januar 2023, 08:42:58 CET schrieb Walker Chen:
> +static int jh71xx_pmu_get_state(struct jh71xx_pmu_dev *pmd, u32 mask, bool *is_on)
> +{
> + struct jh71xx_pmu *pmu = pmd->pmu;
> +
> + if (!mask) {
> + *is_on = false;
nit: While it's not necessarily bad, I don't think callers of
jh71xx_pmu_get_state() should expect this to be set in error case.
> + return -EINVAL;
> + }
> +
> + *is_on = readl(pmu->base + JH71XX_PMU_CURR_POWER_MODE) & mask;
> +
> + return 0;
> +}
[...]
> +static int jh71xx_pmu_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct device_node *np = dev->of_node;
> + const struct jh71xx_pmu_match_data *match_data;
> + struct jh71xx_pmu *pmu;
> + unsigned int i;
> + int ret;
> +
> + pmu = devm_kzalloc(dev, sizeof(*pmu), GFP_KERNEL);
> + if (!pmu)
> + return -ENOMEM;
> +
> + pmu->base = devm_platform_ioremap_resource(pdev, 0);
> + if (IS_ERR(pmu->base))
> + return PTR_ERR(pmu->base);
> +
> + pmu->irq = platform_get_irq(pdev, 0);
> + if (pmu->irq < 0)
> + return pmu->irq;
> +
> + ret = devm_request_irq(dev, pmu->irq, jh71xx_pmu_interrupt,
> + 0, pdev->name, pmu);
> + if (ret)
> + dev_err(dev, "failed to request irq\n");
> +
> + match_data = of_device_get_match_data(dev);
> + if (!match_data)
> + return -EINVAL;
> +
> + pmu->genpd = devm_kcalloc(dev, match_data->num_domains,
> + sizeof(struct generic_pm_domain *),
> + GFP_KERNEL);
> + if (!pmu->genpd)
> + return -ENOMEM;
> +
> + pmu->dev = dev;
> + pmu->match_data = match_data;
> + pmu->genpd_data.domains = pmu->genpd;
> + pmu->genpd_data.num_domains = match_data->num_domains;
> +
> + for (i = 0; i < match_data->num_domains; i++) {
> + ret = jh71xx_pmu_init_domain(pmu, i);
> + if (ret) {
> + dev_err(dev, "failed to initialize power domain\n");
> + return ret;
> + }
> + }
> +
> + spin_lock_init(&pmu->lock);
> + jh71xx_pmu_int_enable(pmu, JH71XX_PMU_INT_ALL_MASK & ~JH71XX_PMU_INT_PCH_FAIL, true);
> +
> + ret = of_genpd_add_provider_onecell(np, &pmu->genpd_data);
> + if (ret) {
> + dev_err(dev, "failed to register genpd driver: %d\n", ret);
> + return ret;
> + }
> +
> + dev_info(dev, "registered %u power domains\n", i);
nit: I guess that could be a dev_dbg to not spam the kernel log too much?
> + return 0;
> +}
> +
> +static const struct jh71xx_domain_info jh7110_power_domains[] = {
> + [JH7110_PD_SYSTOP] = {
> + .name = "SYSTOP",
> + .bit = 0,
> + .flags = GENPD_FLAG_ALWAYS_ON,
> + },
> + [JH7110_PD_CPU] = {
> + .name = "CPU",
> + .bit = 1,
> + .flags = GENPD_FLAG_ALWAYS_ON,
> + },
> + [JH7110_PD_GPUA] = {
> + .name = "GPUA",
> + .bit = 2,
> + },
> + [JH7110_PD_VDEC] = {
> + .name = "VDEC",
> + .bit = 3,
> + },
> + [JH7110_PD_VOUT] = {
> + .name = "VOUT",
> + .bit = 4,
> + },
> + [JH7110_PD_ISP] = {
> + .name = "ISP",
> + .bit = 5,
> + },
> + [JH7110_PD_VENC] = {
> + .name = "VENC",
> + .bit = 6,
> + },
> +};
> +
> +static const struct jh71xx_pmu_match_data jh7110_pmu = {
> + .num_domains = ARRAY_SIZE(jh7110_power_domains),
> + .domain_info = jh7110_power_domains,
> +};
> +
> +static const struct of_device_id jh71xx_pmu_of_match[] = {
> + {
> + .compatible = "starfive,jh7110-pmu",
> + .data = (void *)&jh7110_pmu,
> + }, {
> + /* sentinel */
> + }
> +};
> +
> +static struct platform_driver jh71xx_pmu_driver = {
> + .driver = {
> + .name = "jh71xx-pmu",
> + .of_match_table = jh71xx_pmu_of_match,
In the rockchip pm-domains driver we have
/*
* We can't forcibly eject devices from the power
* domain, so we can't really remove power domains
* once they were added.
*/
.suppress_bind_attrs = true,
here, which might be valid for your pmu driver as well.
Other than that
Reviewed-by: Heiko Stuebner <[email protected]>
On 2023/1/17 18:09, Heiko Stübner wrote:
> Hi Walker,
>
> Am Montag, 16. Januar 2023, 08:42:58 CET schrieb Walker Chen:
>> +static int jh71xx_pmu_get_state(struct jh71xx_pmu_dev *pmd, u32 mask, bool *is_on)
>> +{
>> + struct jh71xx_pmu *pmu = pmd->pmu;
>> +
>> + if (!mask) {
>> + *is_on = false;
>
> nit: While it's not necessarily bad, I don't think callers of
> jh71xx_pmu_get_state() should expect this to be set in error case.
Maybe setting 'is_on' in this case is redundant operation. Will be dropped.
>
>> + return -EINVAL;
>> + }
>> +
>> + *is_on = readl(pmu->base + JH71XX_PMU_CURR_POWER_MODE) & mask;
>> +
>> + return 0;
>> +}
>
> [...]
>
>> +static int jh71xx_pmu_probe(struct platform_device *pdev)
>> +{
>> + struct device *dev = &pdev->dev;
>> + struct device_node *np = dev->of_node;
>> + const struct jh71xx_pmu_match_data *match_data;
>> + struct jh71xx_pmu *pmu;
>> + unsigned int i;
>> + int ret;
>> +
>> + pmu = devm_kzalloc(dev, sizeof(*pmu), GFP_KERNEL);
>> + if (!pmu)
>> + return -ENOMEM;
>> +
>> + pmu->base = devm_platform_ioremap_resource(pdev, 0);
>> + if (IS_ERR(pmu->base))
>> + return PTR_ERR(pmu->base);
>> +
>> + pmu->irq = platform_get_irq(pdev, 0);
>> + if (pmu->irq < 0)
>> + return pmu->irq;
>> +
>> + ret = devm_request_irq(dev, pmu->irq, jh71xx_pmu_interrupt,
>> + 0, pdev->name, pmu);
>> + if (ret)
>> + dev_err(dev, "failed to request irq\n");
>> +
>> + match_data = of_device_get_match_data(dev);
>> + if (!match_data)
>> + return -EINVAL;
>> +
>> + pmu->genpd = devm_kcalloc(dev, match_data->num_domains,
>> + sizeof(struct generic_pm_domain *),
>> + GFP_KERNEL);
>> + if (!pmu->genpd)
>> + return -ENOMEM;
>> +
>> + pmu->dev = dev;
>> + pmu->match_data = match_data;
>> + pmu->genpd_data.domains = pmu->genpd;
>> + pmu->genpd_data.num_domains = match_data->num_domains;
>> +
>> + for (i = 0; i < match_data->num_domains; i++) {
>> + ret = jh71xx_pmu_init_domain(pmu, i);
>> + if (ret) {
>> + dev_err(dev, "failed to initialize power domain\n");
>> + return ret;
>> + }
>> + }
>> +
>> + spin_lock_init(&pmu->lock);
>> + jh71xx_pmu_int_enable(pmu, JH71XX_PMU_INT_ALL_MASK & ~JH71XX_PMU_INT_PCH_FAIL, true);
>> +
>> + ret = of_genpd_add_provider_onecell(np, &pmu->genpd_data);
>> + if (ret) {
>> + dev_err(dev, "failed to register genpd driver: %d\n", ret);
>> + return ret;
>> + }
>> +
>> + dev_info(dev, "registered %u power domains\n", i);
>
> nit: I guess that could be a dev_dbg to not spam the kernel log too much?
OK, will be used with dev_dbg().
>
>
>> + return 0;
>> +}
>> +
>> +static const struct jh71xx_domain_info jh7110_power_domains[] = {
>> + [JH7110_PD_SYSTOP] = {
>> + .name = "SYSTOP",
>> + .bit = 0,
>> + .flags = GENPD_FLAG_ALWAYS_ON,
>> + },
>> + [JH7110_PD_CPU] = {
>> + .name = "CPU",
>> + .bit = 1,
>> + .flags = GENPD_FLAG_ALWAYS_ON,
>> + },
>> + [JH7110_PD_GPUA] = {
>> + .name = "GPUA",
>> + .bit = 2,
>> + },
>> + [JH7110_PD_VDEC] = {
>> + .name = "VDEC",
>> + .bit = 3,
>> + },
>> + [JH7110_PD_VOUT] = {
>> + .name = "VOUT",
>> + .bit = 4,
>> + },
>> + [JH7110_PD_ISP] = {
>> + .name = "ISP",
>> + .bit = 5,
>> + },
>> + [JH7110_PD_VENC] = {
>> + .name = "VENC",
>> + .bit = 6,
>> + },
>> +};
>> +
>> +static const struct jh71xx_pmu_match_data jh7110_pmu = {
>> + .num_domains = ARRAY_SIZE(jh7110_power_domains),
>> + .domain_info = jh7110_power_domains,
>> +};
>> +
>> +static const struct of_device_id jh71xx_pmu_of_match[] = {
>> + {
>> + .compatible = "starfive,jh7110-pmu",
>> + .data = (void *)&jh7110_pmu,
>> + }, {
>> + /* sentinel */
>> + }
>> +};
>> +
>> +static struct platform_driver jh71xx_pmu_driver = {
>> + .driver = {
>> + .name = "jh71xx-pmu",
>> + .of_match_table = jh71xx_pmu_of_match,
>
> In the rockchip pm-domains driver we have
>
> /*
> * We can't forcibly eject devices from the power
> * domain, so we can't really remove power domains
> * once they were added.
> */
> .suppress_bind_attrs = true,
>
> here, which might be valid for your pmu driver as well.
Okay, this attribute will be added in next version.
>
>
> Other than that
> Reviewed-by: Heiko Stuebner <[email protected]>
>
Thanks for your review.
Best regards,
Walker Chen