[PATCH 0/2] Add support for the P1 PMIC from SpacemiT

[PATCH 0/2] Add support for the P1 PMIC from SpacemiT

[Troy Mitchell]

P1 is a multi-channel power management IC from SpacemiT.[1]

It has 6 constant-on-time (COT) buck converters, 12 low-dropout
regulators (LDOs), as well as pinctrl, RTC, and pwrkey functions.

The datasheet of P1 can be found here. [2]

This series is based on K1 initial series [3] and I2C of K1 series [4].

Link:
https://developer.spacemit.com/documentation?token=JtOgwFZzGiExmHkoLFDcE1aSnHe&type=pdf [1]
https://developer.spacemit.com/documentation?token=WsLAwb7OqiMbcMkRZw4cVJWWnlg [2]
https://lore.kernel.org/all/20240730-k1-01-basic-dt-v5-0-98263aae83be@gentoo.org [3]
https://lore.kernel.org/all/20241125-k1-i2c-master-v4-0-0f3d5886336b@gmail.com [4]

Signed-off-by: Troy Mitchell <TroyMitchell988@gmail.com>
---
Troy Mitchell (2):
      dt-bindings: mfd: add support for P1 from SpacemiT
      mfd: add new driver for P1 PMIC from SpacemiT

 .../devicetree/bindings/mfd/spacemit,p1.yaml       | 153 +++++++
 drivers/mfd/Kconfig                                |  14 +
 drivers/mfd/Makefile                               |   1 +
 drivers/mfd/spacemit-pmic.c                        | 159 +++++++
 include/linux/mfd/spacemit/spacemit-p1.h           | 491 +++++++++++++++++++++
 include/linux/mfd/spacemit/spacemit-pmic.h         |  39 ++
 6 files changed, 857 insertions(+)
---
base-commit: 8e929cb546ee42c9a61d24fae60605e9e3192354
change-id: 20241204-k1-p1-d41ad7c95d72
prerequisite-change-id: 20241031-k1-i2c-master-fe7f7b0dce93:v4
prerequisite-patch-id: 9526a79ce73cba25daaf9d748aa0073b5f0ab283
prerequisite-patch-id: d75871ad3cdf179f429be441cb9e69874fb83d1e
prerequisite-change-id: 20240626-k1-01-basic-dt-1aa31eeebcd2:v5
prerequisite-patch-id: 47dcf6861f7d434d25855b379e6d7ef4ce369c9c
prerequisite-patch-id: 77787fe82911923aff15ccf565e8fa451538c3a6
prerequisite-patch-id: b0bdb1742d96c5738f05262c3b0059102761390b
prerequisite-patch-id: 3927d39d8d77e35d5bfe53d9950da574ff8f2054
prerequisite-patch-id: a98039136a4796252a6029e474f03906f2541643
prerequisite-patch-id: c95f6dc0547a2a63a76e3cba0cf5c623b212b4e6
prerequisite-patch-id: 66e750e438ee959ddc2a6f0650814a2d8c989139
prerequisite-patch-id: 29a0fd8c36c1a4340f0d0b68a4c34d2b8abfb1ab
prerequisite-patch-id: 0bdfff661c33c380d1cf00a6c68688e05f88c0b3
prerequisite-patch-id: 99f15718e0bfbb7ed1a96dfa19f35841b004dae9

Best regards,
-- 
Troy Mitchell <TroyMitchell988@gmail.com>

[PATCH 1/2] dt-bindings: mfd: add support for P1 from SpacemiT

[Troy Mitchell]

P1 PMIC contains of regulator, pinctrl, pwrkey and rtc.

P1 contains a load switch, which allows you to control
whether a device is powered on (such as a MIPI screen)

Signed-off-by: Troy Mitchell <TroyMitchell988@gmail.com>
---
 .../devicetree/bindings/mfd/spacemit,p1.yaml       | 153 +++++++++++++++++++++
 1 file changed, 153 insertions(+)

diff --git a/Documentation/devicetree/bindings/mfd/spacemit,p1.yaml b/Documentation/devicetree/bindings/mfd/spacemit,p1.yaml
new file mode 100644
index 0000000000000000000000000000000000000000..7475d403aeb3d0e72ffa9b6fbcbb6545d5bc429f
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/spacemit,p1.yaml
@@ -0,0 +1,153 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mfd/spacemit,p1.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: P1 Power Management Integrated Circuit
+
+maintainers:
+  - Troy Mitchell <troymitchell988@gmail.com>
+
+description:
+  PMIC chip P1 produced by SpacemiT. The device consists of I2C controlled MFD,
+  which contains regulator, pinctrl, pwrkey and rtc.
+
+properties:
+  compatible:
+    const: spacemit,p1-pmic
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  regulators:
+    type: object
+
+    properties:
+      compatible:
+        const: pmic,p1-regulator
+
+    required:
+      - compatible
+
+    patternProperties:
+      "^(dcdc-reg[1-6]|aldo-reg[1-4]|dldo-reg[1-7]|switch)$":
+        type: object
+        $ref: /schemas/regulator/regulator.yaml#
+        unevaluatedProperties: false
+
+    unevaluatedProperties: false
+
+  pinctrl:
+    type: object
+
+    properties:
+      compatible:
+        const: pmic,p1-pinctrl
+
+      "#gpio-cells":
+        const: 2
+
+      gpio-controller: true
+
+    required:
+      - compatible
+      - "#gpio-cells"
+      - gpio-controller
+
+    unevaluatedProperties: false
+
+  pwrkey:
+    type: object
+    $ref: /schemas/input/input.yaml#
+
+    properties:
+      compatible:
+        const: pmic,p1-pwrkey
+
+    required:
+      - compatible
+
+    unevaluatedProperties: false
+
+  rtc:
+    type: object
+    $ref: /schemas/rtc/rtc.yaml#
+
+    properties:
+      compatible:
+        const: pmic,p1-rtc
+
+    required:
+      - compatible
+
+    unevaluatedProperties: false
+
+
+required:
+  - compatible
+  - reg
+  - interrupts
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        pmic@41 {
+            compatible = "spacemit,p1";
+            reg = <0x41>;
+            interrupt-parent = <&intc>;
+            interrupts = <64>;
+
+            regulators {
+                compatible = "pmic,p1-regulator";
+
+                dcdc-reg1 {
+                    regulator-name = "dcdc1";
+                    regulator-min-microvolt = <500000>;
+                    regulator-max-microvolt = <3450000>;
+                    regulator-ramp-delay = <5000>;
+                    regulator-always-on;
+                };
+
+                aldo-reg1 {
+                    regulator-name = "aldo1";
+                    regulator-min-microvolt = <500000>;
+                    regulator-max-microvolt = <3400000>;
+                    regulator-boot-on;
+                };
+
+                dldo-reg1 {
+                    regulator-name = "dldo1";
+                    regulator-min-microvolt = <500000>;
+                    regulator-max-microvolt = <3400000>;
+                    regulator-boot-on;
+                };
+
+                switch {
+                    regulator-name = "switch";
+                };
+            };
+
+            pinctrl {
+                    compatible = "pmic,p1-pinctrl";
+                    gpio-controller;
+                    #gpio-cells = <2>;
+            };
+
+            pwrkey {
+                compatible = "pmic,p1-pwrkey";
+            };
+
+            rtc {
+                compatible = "pmic,p1-rtc";
+            };
+        };
+    };

-- 
2.34.1

[PATCH 2/2] mfd: add new driver for P1 PMIC from SpacemiT

[Troy Mitchell]

Add the core MFD driver for P1 PMIC. I define four sub-devices
for which the drivers will be added in subsequent patches.

For this patch, It supports `reboot` and `shutdown`.

Signed-off-by: Troy Mitchell <TroyMitchell988@gmail.com>
---
 drivers/mfd/Kconfig                        |  14 +
 drivers/mfd/Makefile                       |   1 +
 drivers/mfd/spacemit-pmic.c                | 159 ++++++++++
 include/linux/mfd/spacemit/spacemit-p1.h   | 491 +++++++++++++++++++++++++++++
 include/linux/mfd/spacemit/spacemit-pmic.h |  39 +++
 5 files changed, 704 insertions(+)

diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index ae23b317a64e49f0cb529ae6bd1becbb90b7c282..c062bf6b11fd23d420a6d5f6ee51b3ec97f9fcbb 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -1173,6 +1173,20 @@ config MFD_QCOM_RPM
 	  Say M here if you want to include support for the Qualcomm RPM as a
 	  module. This will build a module called "qcom_rpm".
 
+config MFD_SPACEMIT_PMIC
+	tristate "SpacemiT PMIC"
+	depends on ARCH_SPACEMIT || COMPILE_TEST
+	depends on I2C && OF
+	select MFD_CORE
+	select REGMAP_I2C
+	select REGMAP_IRQ
+	help
+	  If this option is turned on, the P1 chip produced by SpacemiT will
+	  be supported.
+
+	  This driver can also be compiled as a module. If you choose to build
+	  it as a module, the resulting kernel module will be named `spacemit-pmic`.
+
 config MFD_SPMI_PMIC
 	tristate "Qualcomm SPMI PMICs"
 	depends on ARCH_QCOM || COMPILE_TEST
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index e057d6d6faef5c1d639789e2560f336fa26cd872..284dbb8fe2ef83bdd994a598504fe315f2eabbdf 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -266,6 +266,7 @@ obj-$(CONFIG_MFD_SUN4I_GPADC)	+= sun4i-gpadc.o
 obj-$(CONFIG_MFD_STM32_LPTIMER)	+= stm32-lptimer.o
 obj-$(CONFIG_MFD_STM32_TIMERS) 	+= stm32-timers.o
 obj-$(CONFIG_MFD_MXS_LRADC)     += mxs-lradc.o
+obj-$(CONFIG_MFD_SPACEMIT_PMIC)	+= spacemit-pmic.o
 obj-$(CONFIG_MFD_SC27XX_PMIC)	+= sprd-sc27xx-spi.o
 obj-$(CONFIG_RAVE_SP_CORE)	+= rave-sp.o
 obj-$(CONFIG_MFD_ROHM_BD71828)	+= rohm-bd71828.o
diff --git a/drivers/mfd/spacemit-pmic.c b/drivers/mfd/spacemit-pmic.c
new file mode 100644
index 0000000000000000000000000000000000000000..d9f6785cecbd405821dead13cdf8d1f9fd64e508
--- /dev/null
+++ b/drivers/mfd/spacemit-pmic.c
@@ -0,0 +1,159 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (C) 2024 Troy Mitchell <troymitchell988@gmail.com>
+ */
+
+#include <linux/i2c.h>
+#include <linux/mfd/core.h>
+#include <linux/mfd/spacemit/spacemit-pmic.h>
+#include <linux/module.h>
+#include <linux/of_device.h>
+#include <linux/pm_wakeirq.h>
+#include <linux/reboot.h>
+
+P1_REGMAP_CONFIG;
+P1_IRQS_DESC;
+P1_IRQ_CHIP_DESC;
+P1_POWER_KEY_RESOURCES_DESC;
+P1_RTC_RESOURCES_DESC;
+P1_MFD_CELL;
+P1_MFD_MATCH_DATA;
+
+static int spacemit_pmic_shutdown(struct sys_off_data *data)
+{
+	int ret;
+	struct spacemit_pmic *pmic = data->cb_data;
+
+	ret = regmap_update_bits(pmic->regmap,
+				 pmic->match_data->shutdown.reg,
+				 pmic->match_data->shutdown.bit,
+				 pmic->match_data->shutdown.bit);
+	if (ret)
+		dev_err(data->dev, "failed to reboot device!");
+
+	return NOTIFY_DONE;
+}
+
+static int spacemit_pmic_restart(struct sys_off_data *data)
+{
+	int ret;
+	struct spacemit_pmic *pmic = data->cb_data;
+
+	ret = regmap_update_bits(pmic->regmap,
+				 pmic->match_data->reboot.reg,
+				 pmic->match_data->reboot.bit,
+				 pmic->match_data->reboot.bit);
+	if (ret)
+		dev_err(data->dev, "failed to reboot device!");
+
+	return NOTIFY_DONE;
+}
+
+static int spacemit_pmic_probe(struct i2c_client *client)
+{
+	const struct spacemit_pmic_match_data *match_data;
+	const struct mfd_cell *cells;
+	struct spacemit_pmic *pmic;
+	int nr_cells, ret;
+
+	if (!client->irq)
+		return dev_err_probe(&client->dev, -ENXIO, "no interrupt supported");
+
+	match_data = of_device_get_match_data(&client->dev);
+	if (WARN_ON(!match_data))
+		return -EINVAL;
+
+	pmic = devm_kzalloc(&client->dev, sizeof(*pmic), GFP_KERNEL);
+	if (!pmic)
+		return -ENOMEM;
+
+	cells = match_data->mfd_cells;
+	nr_cells = match_data->nr_cells;
+
+	pmic->regmap_cfg = match_data->regmap_cfg;
+	pmic->regmap_irq_chip = match_data->regmap_irq_chip;
+	pmic->i2c = client;
+	pmic->match_data = match_data;
+	pmic->regmap = devm_regmap_init_i2c(client, pmic->regmap_cfg);
+	if (IS_ERR(pmic->regmap))
+		return dev_err_probe(&client->dev,
+				     PTR_ERR(pmic->regmap),
+				     "regmap initialization failed");
+
+	regcache_cache_bypass(pmic->regmap, true);
+
+	i2c_set_clientdata(client, pmic);
+
+	if (pmic->regmap_irq_chip) {
+		ret = regmap_add_irq_chip(pmic->regmap, client->irq, IRQF_ONESHOT, -1,
+						pmic->regmap_irq_chip, &pmic->irq_data);
+		if (ret)
+			return dev_err_probe(&client->dev, ret, "failed to add irqchip");
+	}
+
+	dev_pm_set_wake_irq(&client->dev, client->irq);
+	device_init_wakeup(&client->dev, true);
+
+	ret = devm_mfd_add_devices(&client->dev, PLATFORM_DEVID_NONE,
+				   cells, nr_cells, NULL, 0,
+				   regmap_irq_get_domain(pmic->irq_data));
+	if (ret)
+		return dev_err_probe(&client->dev, ret, "failed to add MFD devices");
+
+	if (match_data->shutdown.reg) {
+		ret = devm_register_sys_off_handler(&client->dev,
+						    SYS_OFF_MODE_POWER_OFF_PREPARE,
+						    SYS_OFF_PRIO_HIGH,
+						    &spacemit_pmic_shutdown,
+						    pmic);
+		if (ret)
+			return dev_err_probe(&client->dev,
+					     ret,
+					     "failed to register restart handler");
+
+	}
+
+	if (match_data->reboot.reg) {
+		ret = devm_register_sys_off_handler(&client->dev,
+						    SYS_OFF_MODE_RESTART,
+						    SYS_OFF_PRIO_HIGH,
+						    &spacemit_pmic_restart,
+						    pmic);
+		if (ret)
+			return dev_err_probe(&client->dev,
+					     ret,
+					     "failed to register restart handler");
+	}
+
+	return 0;
+}
+
+static const struct of_device_id spacemit_pmic_of_match[] = {
+	{ .compatible = "spacemit,p1", .data = &pmic_p1_match_data },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, spacemit_pmic_of_match);
+
+static struct i2c_driver spacemit_pmic_i2c_driver = {
+	.driver = {
+		.name = "spacemit-pmic",
+		.of_match_table = spacemit_pmic_of_match,
+	},
+	.probe    = spacemit_pmic_probe,
+};
+
+static int __init spacemit_pmic_init(void)
+{
+	return platform_driver_register(&spacemit_pmic_i2c_driver);
+}
+
+static void __exit spacemit_pmic_exit(void)
+{
+	platform_driver_unregister(&spacemit_pmic_i2c_driver);
+}
+
+module_init(spacemit_pmic_init);
+module_exit(spacemit_pmic_exit);
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("mfd core driver for the SpacemiT PMIC");
diff --git a/include/linux/mfd/spacemit/spacemit-p1.h b/include/linux/mfd/spacemit/spacemit-p1.h
new file mode 100644
index 0000000000000000000000000000000000000000..6d4d5126b79d6b887d23dea097ec585ecca5e758
--- /dev/null
+++ b/include/linux/mfd/spacemit/spacemit-p1.h
@@ -0,0 +1,491 @@
+/* SPDX-License-Identifier: GPL-2.0-only
+ *
+ * Copyright (C) 2024 Troy Mitchell <troymitchell988@gmail.com>
+ */
+#ifndef __SPACEMIT_P1_H__
+#define __SPACEMIT_P1_H__
+
+enum p1_reg {
+	P1_ID_DCDC1,
+	P1_ID_DCDC2,
+	P1_ID_DCDC3,
+	P1_ID_DCDC4,
+	P1_ID_DCDC5,
+	P1_ID_DCDC6,
+	P1_ID_LDO1,
+	P1_ID_LDO2,
+	P1_ID_LDO3,
+	P1_ID_LDO4,
+	P1_ID_LDO5,
+	P1_ID_LDO6,
+	P1_ID_LDO7,
+	P1_ID_LDO8,
+	P1_ID_LDO9,
+	P1_ID_LDO10,
+	P1_ID_LDO11,
+	P1_ID_SWITCH1,
+};
+
+enum p1_irq_line {
+	/* reg: 0x91 */
+	P1_E_GPI0,
+	P1_E_GPI1,
+	P1_E_GPI2,
+	P1_E_GPI3,
+	P1_E_GPI4,
+	P1_E_GPI5,
+
+	/* reg: 0x92 */
+	P1_E_ADC_TEMP,
+	P1_E_ADC_EOC,
+	P1_E_ADC_EOS,
+	P1_E_WDT_TO,
+	P1_E_ALARM,
+	P1_E_TICK,
+
+	/* reg: 0x93 */
+	P1_E_LDO_OV,
+	P1_E_LDO_UV,
+	P1_E_LDO_SC,
+	P1_E_SW_SC,
+	P1_E_TEMP_WARN,
+	P1_E_TEMP_SEVERE,
+	P1_E_TEMP_CRIT,
+
+	/* reg: 0x94 */
+	P1_E_BUCK1_OV,
+	P1_E_BUCK2_OV,
+	P1_E_BUCK3_OV,
+	P1_E_BUCK4_OV,
+	P1_E_BUCK5_OV,
+	P1_E_BUCK6_OV,
+
+	/* reg: 0x95 */
+	P1_E_BUCK1_UV,
+	P1_E_BUCK2_UV,
+	P1_E_BUCK3_UV,
+	P1_E_BUCK4_UV,
+	P1_E_BUCK5_UV,
+	P1_E_BUCK6_UV,
+
+	/* reg: 0x96 */
+	P1_E_BUCK1_SC,
+	P1_E_BUCK2_SC,
+	P1_E_BUCK3_SC,
+	P1_E_BUCK4_SC,
+	P1_E_BUCK5_SC,
+	P1_E_BUCK6_SC,
+
+	/* reg: 0x97 */
+	P1_E_PWRON_RINTR,
+	P1_E_PWRON_FINTR,
+	P1_E_PWRON_SINTR,
+	P1_E_PWRON_LINTR,
+	P1_E_PWRON_SDINTR,
+	P1_E_VSYS_OV,
+};
+
+#define P1_MAX_REG			0xA8
+
+#define P1_BUCK_VSEL_MASK		0xff
+#define P1_BUCK_EN_MASK			0x1
+
+#define P1_BUCK1_CTRL_REG		0x47
+#define P1_BUCK2_CTRL_REG		0x4a
+#define P1_BUCK3_CTRL_REG		0x4d
+#define P1_BUCK4_CTRL_REG		0x50
+#define P1_BUCK5_CTRL_REG		0x53
+#define P1_BUCK6_CTRL_REG		0x56
+
+#define P1_BUCK1_VSEL_REG		0x48
+#define P1_BUCK2_VSEL_REG		0x4b
+#define P1_BUCK3_VSEL_REG		0x4e
+#define P1_BUCK4_VSEL_REG		0x51
+#define P1_BUCK5_VSEL_REG		0x54
+#define P1_BUCK6_VSEL_REG		0x57
+
+#define P1_ALDO1_CTRL_REG		0x5b
+#define P1_ALDO2_CTRL_REG		0x5e
+#define P1_ALDO3_CTRL_REG		0x61
+#define P1_ALDO4_CTRL_REG		0x64
+
+#define P1_ALDO1_VOLT_REG		0x5c
+#define P1_ALDO2_VOLT_REG		0x5f
+#define P1_ALDO3_VOLT_REG		0x62
+#define P1_ALDO4_VOLT_REG		0x65
+
+#define P1_ALDO_EN_MASK			0x1
+#define P1_ALDO_VSEL_MASK		0x7f
+
+#define P1_DLDO1_CTRL_REG		0x67
+#define P1_DLDO2_CTRL_REG		0x6a
+#define P1_DLDO3_CTRL_REG		0x6d
+#define P1_DLDO4_CTRL_REG		0x70
+#define P1_DLDO5_CTRL_REG		0x73
+#define P1_DLDO6_CTRL_REG		0x76
+#define P1_DLDO7_CTRL_REG		0x79
+
+#define P1_DLDO1_VOLT_REG		0x68
+#define P1_DLDO2_VOLT_REG		0x6b
+#define P1_DLDO3_VOLT_REG		0x6e
+#define P1_DLDO4_VOLT_REG		0x71
+#define P1_DLDO5_VOLT_REG		0x74
+#define P1_DLDO6_VOLT_REG		0x77
+#define P1_DLDO7_VOLT_REG		0x7a
+
+#define P1_DLDO_EN_MASK			0x1
+#define P1_DLDO_VSEL_MASK		0x7f
+
+#define P1_SWITCH_CTRL_REG		0x59
+#define P1_SWTICH_EN_MASK		0x1
+
+#define P1_PWR_CTRL2			0x7e
+#define P1_SW_SHUTDOWN_BIT_MSK		0x4
+#define P1_SW_RESET_BIT_MSK		0x2
+
+#define P1_E_GPI0_MSK			BIT(0)
+#define P1_E_GPI1_MSK			BIT(1)
+#define P1_E_GPI2_MSK			BIT(2)
+#define P1_E_GPI3_MSK			BIT(3)
+#define P1_E_GPI4_MSK			BIT(4)
+#define P1_E_GPI5_MSK			BIT(5)
+
+#define P1_E_ADC_TEMP_MSK		BIT(0)
+#define P1_E_ADC_EOC_MSK		BIT(1)
+#define P1_E_ADC_EOS_MSK		BIT(2)
+#define P1_E_WDT_TO_MSK			BIT(3)
+#define P1_E_ALARM_MSK			BIT(4)
+#define P1_E_TICK_MSK			BIT(5)
+
+#define P1_E_LDO_OV_MSK			BIT(0)
+#define P1_E_LDO_UV_MSK			BIT(1)
+#define P1_E_LDO_SC_MSK			BIT(2)
+#define P1_E_SW_SC_MSK			BIT(3)
+#define P1_E_TEMP_WARN_MSK		BIT(4)
+#define P1_E_TEMP_SEVERE_MSK		BIT(5)
+#define P1_E_TEMP_CRIT_MSK		BIT(6)
+
+#define P1_E_BUCK1_OV_MSK		BIT(0)
+#define P1_E_BUCK2_OV_MSK		BIT(1)
+#define P1_E_BUCK3_OV_MSK		BIT(2)
+#define P1_E_BUCK4_OV_MSK		BIT(3)
+#define P1_E_BUCK5_OV_MSK		BIT(4)
+#define P1_E_BUCK6_OV_MSK		BIT(5)
+
+#define P1_E_BUCK1_UV_MSK		BIT(0)
+#define P1_E_BUCK2_UV_MSK		BIT(1)
+#define P1_E_BUCK3_UV_MSK		BIT(2)
+#define P1_E_BUCK4_UV_MSK		BIT(3)
+#define P1_E_BUCK5_UV_MSK		BIT(4)
+#define P1_E_BUCK6_UV_MSK		BIT(5)
+
+#define P1_E_BUCK1_SC_MSK		BIT(0)
+#define P1_E_BUCK2_SC_MSK		BIT(1)
+#define P1_E_BUCK3_SC_MSK		BIT(2)
+#define P1_E_BUCK4_SC_MSK		BIT(3)
+#define P1_E_BUCK5_SC_MSK		BIT(4)
+#define P1_E_BUCK6_SC_MSK		BIT(5)
+
+#define P1_E_PWRON_RINTR_MSK		BIT(0)
+#define P1_E_PWRON_FINTR_MSK		BIT(1)
+#define P1_E_PWRON_SINTR_MSK		BIT(2)
+#define P1_E_PWRON_LINTR_MSK		BIT(3)
+#define P1_E_PWRON_SDINTR_MSK		BIT(4)
+#define P1_E_VSYS_OV_MSK		BIT(5)
+
+#define P1_E_STATUS_REG_BASE		0x91
+#define P1_E_EN_REG_BASE		0x98
+
+#define P1_REGMAP_CONFIG	\
+	static const struct regmap_config p1_regmap_config = {	\
+		.reg_bits = 8,					\
+		.val_bits = 8,					\
+		.max_register = P1_MAX_REG,			\
+		.cache_type = REGCACHE_RBTREE,			\
+	}
+
+#define P1_IRQS_DESC					\
+static const struct regmap_irq p1_irqs[] = {		\
+	[P1_E_GPI0] = {					\
+		.mask = P1_E_GPI0_MSK,			\
+		.reg_offset = 0,			\
+	},						\
+							\
+	[P1_E_GPI1] = {					\
+		.mask = P1_E_GPI1_MSK,			\
+		.reg_offset = 0,			\
+	},						\
+							\
+	[P1_E_GPI2] = {					\
+		.mask = P1_E_GPI2_MSK,			\
+		.reg_offset = 0,			\
+	},						\
+							\
+	[P1_E_GPI3] = {					\
+		.mask = P1_E_GPI3_MSK,			\
+		.reg_offset = 0,			\
+	},						\
+							\
+	[P1_E_GPI4] = {					\
+		.mask = P1_E_GPI4_MSK,			\
+		.reg_offset = 0,			\
+	},						\
+							\
+	[P1_E_GPI5] = {					\
+		.mask = P1_E_GPI5_MSK,			\
+		.reg_offset = 0,			\
+	},						\
+							\
+	[P1_E_ADC_TEMP] = {				\
+		.mask = P1_E_ADC_TEMP_MSK,		\
+		.reg_offset = 1,			\
+	},						\
+							\
+	[P1_E_ADC_EOC] = {				\
+		.mask = P1_E_ADC_EOC_MSK,		\
+		.reg_offset = 1,			\
+	},						\
+							\
+	[P1_E_ADC_EOS] = {				\
+		.mask = P1_E_ADC_EOS_MSK,		\
+		.reg_offset = 1,			\
+	},						\
+							\
+	[P1_E_WDT_TO] = {				\
+		.mask = P1_E_WDT_TO_MSK,		\
+		.reg_offset = 1,			\
+	},						\
+							\
+	[P1_E_ALARM] = {				\
+		.mask = P1_E_ALARM_MSK,			\
+		.reg_offset = 1,			\
+	},						\
+							\
+	[P1_E_TICK] = {					\
+		.mask = P1_E_TICK_MSK,			\
+		.reg_offset = 1,			\
+	},						\
+							\
+	[P1_E_LDO_OV] = {				\
+		.mask = P1_E_LDO_OV_MSK,		\
+		.reg_offset = 2,			\
+	},						\
+							\
+	[P1_E_LDO_UV] = {				\
+		.mask = P1_E_LDO_UV_MSK,		\
+		.reg_offset = 2,			\
+	},						\
+							\
+	[P1_E_LDO_SC] = {				\
+		.mask = P1_E_LDO_SC_MSK,		\
+		.reg_offset = 2,			\
+	},						\
+							\
+	[P1_E_SW_SC] = {				\
+		.mask = P1_E_SW_SC_MSK,			\
+		.reg_offset = 2,			\
+	},						\
+							\
+	[P1_E_TEMP_WARN] = {				\
+		.mask = P1_E_TEMP_WARN_MSK,		\
+		.reg_offset = 2,			\
+	},						\
+							\
+	[P1_E_TEMP_SEVERE] = {				\
+		.mask = P1_E_TEMP_SEVERE_MSK,		\
+		.reg_offset = 2,			\
+	},						\
+							\
+	[P1_E_TEMP_CRIT] = {				\
+		.mask = P1_E_TEMP_CRIT_MSK,		\
+		.reg_offset = 2,			\
+	},						\
+							\
+	[P1_E_BUCK1_OV] = {				\
+		.mask = P1_E_BUCK1_OV_MSK,		\
+		.reg_offset = 3,			\
+	},						\
+							\
+	[P1_E_BUCK2_OV] = {				\
+		.mask = P1_E_BUCK2_OV_MSK,		\
+		.reg_offset = 3,			\
+	},						\
+							\
+	[P1_E_BUCK3_OV] = {				\
+		.mask = P1_E_BUCK3_OV_MSK,		\
+		.reg_offset = 3,			\
+	},						\
+							\
+	[P1_E_BUCK4_OV] = {				\
+		.mask = P1_E_BUCK4_OV_MSK,		\
+		.reg_offset = 3,			\
+	},						\
+							\
+	[P1_E_BUCK5_OV] = {				\
+		.mask = P1_E_BUCK5_OV_MSK,		\
+		.reg_offset = 3,			\
+	},						\
+							\
+	[P1_E_BUCK6_OV] = {				\
+		.mask = P1_E_BUCK6_OV_MSK,		\
+		.reg_offset = 3,			\
+	},						\
+							\
+	[P1_E_BUCK1_UV] = {				\
+		.mask = P1_E_BUCK1_UV_MSK,		\
+		.reg_offset = 4,			\
+	},						\
+							\
+	[P1_E_BUCK2_UV] = {				\
+		.mask = P1_E_BUCK2_UV_MSK,		\
+		.reg_offset = 4,			\
+	},						\
+							\
+	[P1_E_BUCK3_UV] = {				\
+		.mask = P1_E_BUCK3_UV_MSK,		\
+		.reg_offset = 4,			\
+	},						\
+							\
+	[P1_E_BUCK4_UV] = {				\
+		.mask = P1_E_BUCK4_UV_MSK,		\
+		.reg_offset = 4,			\
+	},						\
+							\
+	[P1_E_BUCK5_UV] = {				\
+		.mask = P1_E_BUCK5_UV_MSK,		\
+		.reg_offset = 4,			\
+	},						\
+							\
+	[P1_E_BUCK6_UV] = {				\
+		.mask = P1_E_BUCK6_UV_MSK,		\
+		.reg_offset = 4,			\
+	},						\
+							\
+	[P1_E_BUCK1_SC] = {				\
+		.mask = P1_E_BUCK1_SC_MSK,		\
+		.reg_offset = 5,			\
+	},						\
+							\
+	[P1_E_BUCK2_SC] = {				\
+		.mask = P1_E_BUCK2_SC_MSK,		\
+		.reg_offset = 5,			\
+	},						\
+							\
+	[P1_E_BUCK3_SC] = {				\
+		.mask = P1_E_BUCK3_SC_MSK,		\
+		.reg_offset = 5,			\
+	},						\
+							\
+	[P1_E_BUCK4_SC] = {				\
+		.mask = P1_E_BUCK4_SC_MSK,		\
+		.reg_offset = 5,			\
+	},						\
+							\
+	[P1_E_BUCK5_SC] = {				\
+		.mask = P1_E_BUCK5_SC_MSK,		\
+		.reg_offset = 5,			\
+	},						\
+							\
+	[P1_E_BUCK6_SC] = {				\
+		.mask = P1_E_BUCK6_SC_MSK,		\
+		.reg_offset = 5,			\
+	},						\
+							\
+	[P1_E_PWRON_RINTR] = {				\
+		.mask = P1_E_PWRON_RINTR_MSK,		\
+		.reg_offset = 6,			\
+	},						\
+							\
+	[P1_E_PWRON_FINTR] = {				\
+		.mask = P1_E_PWRON_FINTR_MSK,		\
+		.reg_offset = 6,			\
+	},						\
+							\
+	[P1_E_PWRON_SINTR] = {				\
+		.mask = P1_E_PWRON_SINTR_MSK,		\
+		.reg_offset = 6,			\
+	},						\
+							\
+	[P1_E_PWRON_LINTR] = {				\
+		.mask = P1_E_PWRON_LINTR_MSK,		\
+		.reg_offset = 6,			\
+	},						\
+							\
+	[P1_E_PWRON_SDINTR] = {				\
+		.mask = P1_E_PWRON_SDINTR_MSK,		\
+		.reg_offset = 6,			\
+	},						\
+							\
+	[P1_E_VSYS_OV] = {				\
+		.mask = P1_E_VSYS_OV_MSK,		\
+		.reg_offset = 6,			\
+	},						\
+}
+
+#define P1_IRQ_CHIP_DESC				\
+static const struct regmap_irq_chip p1_irq_chip = {	\
+	.name			= "p1",			\
+	.irqs			= p1_irqs,		\
+	.num_irqs		= ARRAY_SIZE(p1_irqs),	\
+	.num_regs		= 7,			\
+	.status_base		= P1_E_STATUS_REG_BASE,	\
+	.mask_base		= P1_E_EN_REG_BASE,	\
+	.ack_base		= P1_E_STATUS_REG_BASE,	\
+	.init_ack_masked	= true,			\
+}
+
+#define P1_POWER_KEY_RESOURCES_DESC			\
+static const struct resource p1_pwrkey_resources[] = {	\
+	DEFINE_RES_IRQ(P1_E_PWRON_RINTR),		\
+	DEFINE_RES_IRQ(P1_E_PWRON_FINTR),		\
+	DEFINE_RES_IRQ(P1_E_PWRON_SINTR),		\
+	DEFINE_RES_IRQ(P1_E_PWRON_LINTR),		\
+}
+
+#define P1_RTC_RESOURCES_DESC				\
+static const struct resource p1_rtc_resources[] = {	\
+	DEFINE_RES_IRQ(P1_E_ALARM),			\
+}
+
+#define P1_MFD_CELL	\
+	static const struct mfd_cell p1[] = {					\
+		{								\
+			.name = "spacemit-regulator@p1",			\
+			.of_compatible = "pmic,p1-regulator",			\
+		},								\
+		{								\
+			.name = "spacemit-pinctrl@p1",				\
+			.of_compatible = "pmic,p1-pinctrl",			\
+		},								\
+		{								\
+			.name = "spacemit-pwrkey@p1",				\
+			.of_compatible = "pmic,p1-pwrkey",			\
+			.num_resources = ARRAY_SIZE(p1_pwrkey_resources),	\
+			.resources = &p1_pwrkey_resources[0],			\
+		},								\
+		{								\
+			.name = "spacemit-rtc@p1",				\
+			.of_compatible = "pmic,p1-rtc",				\
+			.num_resources = ARRAY_SIZE(p1_rtc_resources),		\
+			.resources = &p1_rtc_resources[0],			\
+		},								\
+	}
+
+#define P1_MFD_MATCH_DATA							\
+static struct spacemit_pmic_match_data pmic_p1_match_data = {			\
+	.regmap_cfg = &p1_regmap_config,					\
+	.regmap_irq_chip = &p1_irq_chip,					\
+	.mfd_cells = p1,							\
+	.nr_cells = ARRAY_SIZE(p1),						\
+	.name = "p1",								\
+	.shutdown = {								\
+		.reg = P1_PWR_CTRL2,						\
+		.bit = P1_SW_SHUTDOWN_BIT_MSK,					\
+	},									\
+	.reboot = {								\
+		.reg = P1_PWR_CTRL2,						\
+		.bit = P1_SW_RESET_BIT_MSK,					\
+	},									\
+}
+
+#endif // __SPACEMIT_P1_H__
diff --git a/include/linux/mfd/spacemit/spacemit-pmic.h b/include/linux/mfd/spacemit/spacemit-pmic.h
new file mode 100644
index 0000000000000000000000000000000000000000..808bbf1883c4ba019e552ffca94a10e46036ad3a
--- /dev/null
+++ b/include/linux/mfd/spacemit/spacemit-pmic.h
@@ -0,0 +1,39 @@
+/* SPDX-License-Identifier: GPL-2.0-only
+ *
+ * Copyright (C) 2024 Troy Mitchell <troymitchell988@gmail.com>
+ */
+#ifndef __SPACEMIT_PMIC_H__
+#define __SPACEMIT_PMIC_H__
+
+#include <linux/regulator/machine.h>
+#include <linux/regmap.h>
+#include <linux/mfd/spacemit/spacemit-p1.h>
+
+struct spacemit_pmic_match_data {
+	const struct regmap_config *regmap_cfg;
+	const struct regmap_irq_chip *regmap_irq_chip;
+	const struct mfd_cell *mfd_cells;
+	int nr_cells;
+	const char *name;
+
+	struct {
+		unsigned char reg;
+		unsigned char bit;
+	} shutdown;
+
+	struct {
+		unsigned int reg;
+		unsigned char bit;
+	} reboot;
+};
+
+struct spacemit_pmic {
+	struct i2c_client		*i2c;
+	struct regmap_irq_chip_data	*irq_data;
+	struct regmap			*regmap;
+	const struct regmap_config	*regmap_cfg;
+	const struct regmap_irq_chip	*regmap_irq_chip;
+	const struct spacemit_pmic_match_data *match_data;
+};
+
+#endif // __SPACEMIT_PMIC_H__

-- 
2.34.1

Re: [PATCH 2/2] mfd: add new driver for P1 PMIC from SpacemiT

[Troy Mitchell]

On 2024/12/30 18:02, Troy Mitchell wrote:
> Add the core MFD driver for P1 PMIC. I define four sub-devices
> for which the drivers will be added in subsequent patches.
> 
> For this patch, It supports `reboot` and `shutdown`.
> 
> Signed-off-by: Troy Mitchell <TroyMitchell988@gmail.com>
> ---
>  drivers/mfd/Kconfig                        |  14 +
>  drivers/mfd/Makefile                       |   1 +
>  drivers/mfd/spacemit-pmic.c                | 159 ++++++++++
>  include/linux/mfd/spacemit/spacemit-p1.h   | 491 +++++++++++++++++++++++++++++
>  include/linux/mfd/spacemit/spacemit-pmic.h |  39 +++
>  5 files changed, 704 insertions(+)
> 
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index ae23b317a64e49f0cb529ae6bd1becbb90b7c282..c062bf6b11fd23d420a6d5f6ee51b3ec97f9fcbb 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -1173,6 +1173,20 @@ config MFD_QCOM_RPM
>  	  Say M here if you want to include support for the Qualcomm RPM as a
>  	  module. This will build a module called "qcom_rpm".
>  
> +config MFD_SPACEMIT_PMIC
> +	tristate "SpacemiT PMIC"
> +	depends on ARCH_SPACEMIT || COMPILE_TEST
> +	depends on I2C && OF
> +	select MFD_CORE
> +	select REGMAP_I2C
> +	select REGMAP_IRQ
> +	help
> +	  If this option is turned on, the P1 chip produced by SpacemiT will
> +	  be supported.
> +
> +	  This driver can also be compiled as a module. If you choose to build
> +	  it as a module, the resulting kernel module will be named `spacemit-pmic`.
> +
>  config MFD_SPMI_PMIC
>  	tristate "Qualcomm SPMI PMICs"
>  	depends on ARCH_QCOM || COMPILE_TEST
> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> index e057d6d6faef5c1d639789e2560f336fa26cd872..284dbb8fe2ef83bdd994a598504fe315f2eabbdf 100644
> --- a/drivers/mfd/Makefile
> +++ b/drivers/mfd/Makefile
> @@ -266,6 +266,7 @@ obj-$(CONFIG_MFD_SUN4I_GPADC)	+= sun4i-gpadc.o
>  obj-$(CONFIG_MFD_STM32_LPTIMER)	+= stm32-lptimer.o
>  obj-$(CONFIG_MFD_STM32_TIMERS) 	+= stm32-timers.o
>  obj-$(CONFIG_MFD_MXS_LRADC)     += mxs-lradc.o
> +obj-$(CONFIG_MFD_SPACEMIT_PMIC)	+= spacemit-pmic.o
>  obj-$(CONFIG_MFD_SC27XX_PMIC)	+= sprd-sc27xx-spi.o
>  obj-$(CONFIG_RAVE_SP_CORE)	+= rave-sp.o
>  obj-$(CONFIG_MFD_ROHM_BD71828)	+= rohm-bd71828.o
> diff --git a/drivers/mfd/spacemit-pmic.c b/drivers/mfd/spacemit-pmic.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..d9f6785cecbd405821dead13cdf8d1f9fd64e508
> --- /dev/null
> +++ b/drivers/mfd/spacemit-pmic.c
> @@ -0,0 +1,159 @@
> +static const struct of_device_id spacemit_pmic_of_match[] = {
> +	{ .compatible = "spacemit,p1", .data = &pmic_p1_match_data },
> +	{ /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, spacemit_pmic_of_match);
> +
> +static struct i2c_driver spacemit_pmic_i2c_driver = {
> +	.driver = {
> +		.name = "spacemit-pmic",
> +		.of_match_table = spacemit_pmic_of_match,
> +	},
> +	.probe    = spacemit_pmic_probe,
> +};
> +
> +static int __init spacemit_pmic_init(void)
> +{
> +	return platform_driver_register(&spacemit_pmic_i2c_driver);
> +}
> +
> +static void __exit spacemit_pmic_exit(void)
> +{
> +	platform_driver_unregister(&spacemit_pmic_i2c_driver);
> +}
I should use i2c_add_driver/i2c_del_driver here.
I forgot to add my modified c file via stg :(
> +
> +module_init(spacemit_pmic_init);
> +module_exit(spacemit_pmic_exit);
> 

-- 
Troy Mitchell

Re: [PATCH 1/2] dt-bindings: mfd: add support for P1 from SpacemiT

[Rob Herring (Arm)]

On Mon, 30 Dec 2024 18:02:05 +0800, Troy Mitchell wrote:
> P1 PMIC contains of regulator, pinctrl, pwrkey and rtc.
> 
> P1 contains a load switch, which allows you to control
> whether a device is powered on (such as a MIPI screen)
> 
> Signed-off-by: Troy Mitchell <TroyMitchell988@gmail.com>
> ---
>  .../devicetree/bindings/mfd/spacemit,p1.yaml       | 153 +++++++++++++++++++++
>  1 file changed, 153 insertions(+)
> 

My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:

dtschema/dtc warnings/errors:
Documentation/devicetree/bindings/mfd/spacemit,p1.example.dtb: /example-0/i2c/pmic@41: failed to match any schema with compatible: ['spacemit,p1']

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20241230-k1-p1-v1-1-aa4e02b9f993@gmail.com

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.

Re: [PATCH 1/2] dt-bindings: mfd: add support for P1 from SpacemiT

[Krzysztof Kozlowski]

On 30/12/2024 11:02, Troy Mitchell wrote:
> +
> +properties:
> +  compatible:
> +    const: spacemit,p1-pmic


spacemit,p1

Unless this can be something else?

> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  regulators:
> +    type: object
> +
> +    properties:
> +      compatible:
> +        const: pmic,p1-regulator

Drop compatible, regulators are not re-usable blocks.

> +
> +    required:
> +      - compatible
> +
> +    patternProperties:
> +      "^(dcdc-reg[1-6]|aldo-reg[1-4]|dldo-reg[1-7]|switch)$":

You can drop all "reg" suffixes.

> +        type: object
> +        $ref: /schemas/regulator/regulator.yaml#
> +        unevaluatedProperties: false
> +
> +    unevaluatedProperties: false
> +
> +  pinctrl:

pinctrl or gpio?

> +    type: object
> +
> +    properties:
> +      compatible:
> +        const: pmic,p1-pinctrl
> +
> +      "#gpio-cells":
> +        const: 2
> +
> +      gpio-controller: true
> +
> +    required:
> +      - compatible
> +      - "#gpio-cells"
> +      - gpio-controller
> +
> +    unevaluatedProperties: false

No $ref? Then additionalProperties and put it after type: ... but if you
do not have here pinctrl, then just fold the node into parent.


> +
> +  pwrkey:
> +    type: object
> +    $ref: /schemas/input/input.yaml#
> +
> +    properties:
> +      compatible:
> +        const: pmic,p1-pwrkey
> +
> +    required:
> +      - compatible


No resources here, fold the device into parent.


> +
> +    unevaluatedProperties: false
> +
> +  rtc:
> +    type: object
> +    $ref: /schemas/rtc/rtc.yaml#


No resources here, fold the device into parent.

> +
> +    properties:
> +      compatible:
> +        const: pmic,p1-rtc
> +
> +    required:
> +      - compatible
> +
> +    unevaluatedProperties: false
> +
> +

Only one blank line

> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +
> +unevaluatedProperties: false


example-schema explains when to use unevaluated/additionalProps.


Best regards,
Krzysztof

Re: [PATCH 2/2] mfd: add new driver for P1 PMIC from SpacemiT

[Krzysztof Kozlowski]

On 30/12/2024 11:02, Troy Mitchell wrote:
> Add the core MFD driver for P1 PMIC. I define four sub-devices


I do not see any definition of MFD subdevices.

> for which the drivers will be added in subsequent patches.
> 
> For this patch, It supports `reboot` and `shutdown`.
> 
> Signed-off-by: Troy Mitchell <TroyMitchell988@gmail.com>
> ---
>  drivers/mfd/Kconfig                        |  14 +
>  drivers/mfd/Makefile                       |   1 +
>  drivers/mfd/spacemit-pmic.c                | 159 ++++++++++
>  include/linux/mfd/spacemit/spacemit-p1.h   | 491 +++++++++++++++++++++++++++++
>  include/linux/mfd/spacemit/spacemit-pmic.h |  39 +++
>  5 files changed, 704 insertions(+)
> 
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index ae23b317a64e49f0cb529ae6bd1becbb90b7c282..c062bf6b11fd23d420a6d5f6ee51b3ec97f9fcbb 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -1173,6 +1173,20 @@ config MFD_QCOM_RPM
>  	  Say M here if you want to include support for the Qualcomm RPM as a
>  	  module. This will build a module called "qcom_rpm".
>  
> +config MFD_SPACEMIT_PMIC
> +	tristate "SpacemiT PMIC"
> +	depends on ARCH_SPACEMIT || COMPILE_TEST
> +	depends on I2C && OF
> +	select MFD_CORE
> +	select REGMAP_I2C
> +	select REGMAP_IRQ
> +	help
> +	  If this option is turned on, the P1 chip produced by SpacemiT will
> +	  be supported.
> +
> +	  This driver can also be compiled as a module. If you choose to build
> +	  it as a module, the resulting kernel module will be named `spacemit-pmic`.
> +
>  config MFD_SPMI_PMIC
>  	tristate "Qualcomm SPMI PMICs"
>  	depends on ARCH_QCOM || COMPILE_TEST
> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> index e057d6d6faef5c1d639789e2560f336fa26cd872..284dbb8fe2ef83bdd994a598504fe315f2eabbdf 100644
> --- a/drivers/mfd/Makefile
> +++ b/drivers/mfd/Makefile
> @@ -266,6 +266,7 @@ obj-$(CONFIG_MFD_SUN4I_GPADC)	+= sun4i-gpadc.o
>  obj-$(CONFIG_MFD_STM32_LPTIMER)	+= stm32-lptimer.o
>  obj-$(CONFIG_MFD_STM32_TIMERS) 	+= stm32-timers.o
>  obj-$(CONFIG_MFD_MXS_LRADC)     += mxs-lradc.o
> +obj-$(CONFIG_MFD_SPACEMIT_PMIC)	+= spacemit-pmic.o
>  obj-$(CONFIG_MFD_SC27XX_PMIC)	+= sprd-sc27xx-spi.o
>  obj-$(CONFIG_RAVE_SP_CORE)	+= rave-sp.o
>  obj-$(CONFIG_MFD_ROHM_BD71828)	+= rohm-bd71828.o
> diff --git a/drivers/mfd/spacemit-pmic.c b/drivers/mfd/spacemit-pmic.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..d9f6785cecbd405821dead13cdf8d1f9fd64e508
> --- /dev/null
> +++ b/drivers/mfd/spacemit-pmic.c
> @@ -0,0 +1,159 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (C) 2024 Troy Mitchell <troymitchell988@gmail.com>
> + */
> +
> +#include <linux/i2c.h>
> +#include <linux/mfd/core.h>
> +#include <linux/mfd/spacemit/spacemit-pmic.h>
> +#include <linux/module.h>
> +#include <linux/of_device.h>

I don't see usage of this header. Include what is used directly.

> +#include <linux/pm_wakeirq.h>
> +#include <linux/reboot.h>
> +
> +P1_REGMAP_CONFIG;
> +P1_IRQS_DESC;
> +P1_IRQ_CHIP_DESC;
> +P1_POWER_KEY_RESOURCES_DESC;
> +P1_RTC_RESOURCES_DESC;
> +P1_MFD_CELL;
> +P1_MFD_MATCH_DATA;

Hm? Declarations and definitions go here, not to somewhere else.



> +
> +static int spacemit_pmic_probe(struct i2c_client *client)
> +{
> +	const struct spacemit_pmic_match_data *match_data;
> +	const struct mfd_cell *cells;
> +	struct spacemit_pmic *pmic;
> +	int nr_cells, ret;
> +
> +	if (!client->irq)
> +		return dev_err_probe(&client->dev, -ENXIO, "no interrupt supported");

And why is this fatal error? If interrupt is not supported by hardware,
why would you add "unsupported" interrupt?

> +
> +	match_data = of_device_get_match_data(&client->dev);
> +	if (WARN_ON(!match_data))
> +		return -EINVAL;
> +
> +	pmic = devm_kzalloc(&client->dev, sizeof(*pmic), GFP_KERNEL);
> +	if (!pmic)
> +		return -ENOMEM;
> +
> +	cells = match_data->mfd_cells;
> +	nr_cells = match_data->nr_cells;
> +
> +	pmic->regmap_cfg = match_data->regmap_cfg;
> +	pmic->regmap_irq_chip = match_data->regmap_irq_chip;
> +	pmic->i2c = client;
> +	pmic->match_data = match_data;
> +	pmic->regmap = devm_regmap_init_i2c(client, pmic->regmap_cfg);
> +	if (IS_ERR(pmic->regmap))
> +		return dev_err_probe(&client->dev,
> +				     PTR_ERR(pmic->regmap),
> +				     "regmap initialization failed");
> +
> +	regcache_cache_bypass(pmic->regmap, true);
> +
> +	i2c_set_clientdata(client, pmic);
> +
> +	if (pmic->regmap_irq_chip) {


It's impossible to have it false. Test your driver.

> +		ret = regmap_add_irq_chip(pmic->regmap, client->irq, IRQF_ONESHOT, -1,
> +						pmic->regmap_irq_chip, &pmic->irq_data);
> +		if (ret)
> +			return dev_err_probe(&client->dev, ret, "failed to add irqchip");
> +	}
> +
> +	dev_pm_set_wake_irq(&client->dev, client->irq);
> +	device_init_wakeup(&client->dev, true);
> +
> +	ret = devm_mfd_add_devices(&client->dev, PLATFORM_DEVID_NONE,
> +				   cells, nr_cells, NULL, 0,
> +				   regmap_irq_get_domain(pmic->irq_data));
> +	if (ret)
> +		return dev_err_probe(&client->dev, ret, "failed to add MFD devices");
> +
> +	if (match_data->shutdown.reg) {

Also not possible, useless if.

> +		ret = devm_register_sys_off_handler(&client->dev,
> +						    SYS_OFF_MODE_POWER_OFF_PREPARE,
> +						    SYS_OFF_PRIO_HIGH,
> +						    &spacemit_pmic_shutdown,
> +						    pmic);
> +		if (ret)
> +			return dev_err_probe(&client->dev,
> +					     ret,
> +					     "failed to register restart handler");
> +
> +	}
> +
> +	if (match_data->reboot.reg) {

Also not possible.

> +		ret = devm_register_sys_off_handler(&client->dev,
> +						    SYS_OFF_MODE_RESTART,
> +						    SYS_OFF_PRIO_HIGH,
> +						    &spacemit_pmic_restart,
> +						    pmic);
> +		if (ret)
> +			return dev_err_probe(&client->dev,
> +					     ret,
> +					     "failed to register restart handler");
> +	}
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id spacemit_pmic_of_match[] = {
> +	{ .compatible = "spacemit,p1", .data = &pmic_p1_match_data },
> +	{ /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, spacemit_pmic_of_match);
> +
> +static struct i2c_driver spacemit_pmic_i2c_driver = {
> +	.driver = {
> +		.name = "spacemit-pmic",
> +		.of_match_table = spacemit_pmic_of_match,
> +	},
> +	.probe    = spacemit_pmic_probe,
> +};
> +
> +static int __init spacemit_pmic_init(void)
> +{
> +	return platform_driver_register(&spacemit_pmic_i2c_driver);
> +}
> +
> +static void __exit spacemit_pmic_exit(void)
> +{
> +	platform_driver_unregister(&spacemit_pmic_i2c_driver);
> +}
> +
> +module_init(spacemit_pmic_init);
> +module_exit(spacemit_pmic_exit);

Use proper wrapper for these above.

> +
> +MODULE_LICENSE("GPL");
> +MODULE_DESCRIPTION("mfd core driver for the SpacemiT PMIC");

...

> +
> +#define P1_MAX_REG			0xA8
> +
> +#define P1_BUCK_VSEL_MASK		0xff
> +#define P1_BUCK_EN_MASK			0x1
> +
> +#define P1_BUCK1_CTRL_REG		0x47
> +#define P1_BUCK2_CTRL_REG		0x4a


Either lowercase or uppercase hex, not both.

> +#define P1_BUCK3_CTRL_REG		0x4d
> +#define P1_BUCK4_CTRL_REG		0x50
> +#define P1_BUCK5_CTRL_REG		0x53
> +#define P1_BUCK6_CTRL_REG		0x56
> +
> +#define P1_BUCK1_VSEL_REG		0x48
> +#define P1_BUCK2_VSEL_REG		0x4b
> +#define P1_BUCK3_VSEL_REG		0x4e
> +#define P1_BUCK4_VSEL_REG		0x51
> +#define P1_BUCK5_VSEL_REG		0x54
> +#define P1_BUCK6_VSEL_REG		0x57
> +
> +#define P1_ALDO1_CTRL_REG		0x5b
> +#define P1_ALDO2_CTRL_REG		0x5e
> +#define P1_ALDO3_CTRL_REG		0x61
> +#define P1_ALDO4_CTRL_REG		0x64
> +
> +#define P1_ALDO1_VOLT_REG		0x5c
> +#define P1_ALDO2_VOLT_REG		0x5f
> +#define P1_ALDO3_VOLT_REG		0x62
> +#define P1_ALDO4_VOLT_REG		0x65
> +
> +#define P1_ALDO_EN_MASK			0x1
> +#define P1_ALDO_VSEL_MASK		0x7f
> +
> +#define P1_DLDO1_CTRL_REG		0x67
> +#define P1_DLDO2_CTRL_REG		0x6a
> +#define P1_DLDO3_CTRL_REG		0x6d
> +#define P1_DLDO4_CTRL_REG		0x70
> +#define P1_DLDO5_CTRL_REG		0x73
> +#define P1_DLDO6_CTRL_REG		0x76
> +#define P1_DLDO7_CTRL_REG		0x79
> +
> +#define P1_DLDO1_VOLT_REG		0x68
> +#define P1_DLDO2_VOLT_REG		0x6b
> +#define P1_DLDO3_VOLT_REG		0x6e
> +#define P1_DLDO4_VOLT_REG		0x71
> +#define P1_DLDO5_VOLT_REG		0x74
> +#define P1_DLDO6_VOLT_REG		0x77
> +#define P1_DLDO7_VOLT_REG		0x7a
> +
> +#define P1_DLDO_EN_MASK			0x1
> +#define P1_DLDO_VSEL_MASK		0x7f
> +
> +#define P1_SWITCH_CTRL_REG		0x59
> +#define P1_SWTICH_EN_MASK		0x1
> +
> +#define P1_PWR_CTRL2			0x7e
> +#define P1_SW_SHUTDOWN_BIT_MSK		0x4
> +#define P1_SW_RESET_BIT_MSK		0x2
> +
> +#define P1_E_GPI0_MSK			BIT(0)
> +#define P1_E_GPI1_MSK			BIT(1)
> +#define P1_E_GPI2_MSK			BIT(2)
> +#define P1_E_GPI3_MSK			BIT(3)
> +#define P1_E_GPI4_MSK			BIT(4)
> +#define P1_E_GPI5_MSK			BIT(5)
> +
> +#define P1_E_ADC_TEMP_MSK		BIT(0)
> +#define P1_E_ADC_EOC_MSK		BIT(1)
> +#define P1_E_ADC_EOS_MSK		BIT(2)
> +#define P1_E_WDT_TO_MSK			BIT(3)
> +#define P1_E_ALARM_MSK			BIT(4)
> +#define P1_E_TICK_MSK			BIT(5)
> +
> +#define P1_E_LDO_OV_MSK			BIT(0)
> +#define P1_E_LDO_UV_MSK			BIT(1)
> +#define P1_E_LDO_SC_MSK			BIT(2)
> +#define P1_E_SW_SC_MSK			BIT(3)
> +#define P1_E_TEMP_WARN_MSK		BIT(4)
> +#define P1_E_TEMP_SEVERE_MSK		BIT(5)
> +#define P1_E_TEMP_CRIT_MSK		BIT(6)
> +
> +#define P1_E_BUCK1_OV_MSK		BIT(0)
> +#define P1_E_BUCK2_OV_MSK		BIT(1)
> +#define P1_E_BUCK3_OV_MSK		BIT(2)
> +#define P1_E_BUCK4_OV_MSK		BIT(3)
> +#define P1_E_BUCK5_OV_MSK		BIT(4)
> +#define P1_E_BUCK6_OV_MSK		BIT(5)
> +
> +#define P1_E_BUCK1_UV_MSK		BIT(0)
> +#define P1_E_BUCK2_UV_MSK		BIT(1)
> +#define P1_E_BUCK3_UV_MSK		BIT(2)
> +#define P1_E_BUCK4_UV_MSK		BIT(3)
> +#define P1_E_BUCK5_UV_MSK		BIT(4)
> +#define P1_E_BUCK6_UV_MSK		BIT(5)
> +
> +#define P1_E_BUCK1_SC_MSK		BIT(0)
> +#define P1_E_BUCK2_SC_MSK		BIT(1)
> +#define P1_E_BUCK3_SC_MSK		BIT(2)
> +#define P1_E_BUCK4_SC_MSK		BIT(3)
> +#define P1_E_BUCK5_SC_MSK		BIT(4)
> +#define P1_E_BUCK6_SC_MSK		BIT(5)
> +
> +#define P1_E_PWRON_RINTR_MSK		BIT(0)
> +#define P1_E_PWRON_FINTR_MSK		BIT(1)
> +#define P1_E_PWRON_SINTR_MSK		BIT(2)
> +#define P1_E_PWRON_LINTR_MSK		BIT(3)
> +#define P1_E_PWRON_SDINTR_MSK		BIT(4)
> +#define P1_E_VSYS_OV_MSK		BIT(5)
> +
> +#define P1_E_STATUS_REG_BASE		0x91
> +#define P1_E_EN_REG_BASE		0x98
> +
> +#define P1_REGMAP_CONFIG	\
> +	static const struct regmap_config p1_regmap_config = {	\
> +		.reg_bits = 8,					\
> +		.val_bits = 8,					\
> +		.max_register = P1_MAX_REG,			\
> +		.cache_type = REGCACHE_RBTREE,			\
> +	}
> +
> +#define P1_IRQS_DESC					\
> +static const struct regmap_irq p1_irqs[] = {		\


No, all these defines are just not needed, not readable. Please follow
existing kernel style - just look at recent drivers in drivers/mfd/ to
see how they are designed and developed.

> +	[P1_E_GPI0] = {					\
> +		.mask = P1_E_GPI0_MSK,			\
> +		.reg_offset = 0,			\
> +	},						\
> +							\
> +	[P1_E_GPI1] = {					\
> +		.mask = P1_E_GPI1_MSK,			\

Best regards,
Krzysztof

Re: [PATCH 2/2] mfd: add new driver for P1 PMIC from SpacemiT

[Troy Mitchell]

Hi, Krzysztof.
Thanks for ur review!

On 2024/12/30 19:33, Krzysztof Kozlowski wrote:
> On 30/12/2024 11:02, Troy Mitchell wrote:
>> Add the core MFD driver for P1 PMIC. I define four sub-devices
> 
> 
> I do not see any definition of MFD subdevices.
I define them in spacemit-p1.h.
the macro is named `P1_MFD_CELL`
> 
>> for which the drivers will be added in subsequent patches.
>>
>> For this patch, It supports `reboot` and `shutdown`.
>>
>> Signed-off-by: Troy Mitchell <TroyMitchell988@gmail.com>
>> ---
>>  drivers/mfd/Kconfig                        |  14 +
>>  drivers/mfd/Makefile                       |   1 +
>>  drivers/mfd/spacemit-pmic.c                | 159 ++++++++++
>>  include/linux/mfd/spacemit/spacemit-p1.h   | 491 +++++++++++++++++++++++++++++
>>  include/linux/mfd/spacemit/spacemit-pmic.h |  39 +++
>>  5 files changed, 704 insertions(+)
>>
>> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
>> index ae23b317a64e49f0cb529ae6bd1becbb90b7c282..c062bf6b11fd23d420a6d5f6ee51b3ec97f9fcbb 100644
>> --- a/drivers/mfd/Kconfig
>> +++ b/drivers/mfd/Kconfig
>> @@ -1173,6 +1173,20 @@ config MFD_QCOM_RPM
>>  	  Say M here if you want to include support for the Qualcomm RPM as a
>>  	  module. This will build a module called "qcom_rpm".
>>  
>> +config MFD_SPACEMIT_PMIC
>> +	tristate "SpacemiT PMIC"
>> +	depends on ARCH_SPACEMIT || COMPILE_TEST
>> +	depends on I2C && OF
>> +	select MFD_CORE
>> +	select REGMAP_I2C
>> +	select REGMAP_IRQ
>> +	help
>> +	  If this option is turned on, the P1 chip produced by SpacemiT will
>> +	  be supported.
>> +
>> +	  This driver can also be compiled as a module. If you choose to build
>> +	  it as a module, the resulting kernel module will be named `spacemit-pmic`.
>> +
>>  config MFD_SPMI_PMIC
>>  	tristate "Qualcomm SPMI PMICs"
>>  	depends on ARCH_QCOM || COMPILE_TEST
>> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
>> index e057d6d6faef5c1d639789e2560f336fa26cd872..284dbb8fe2ef83bdd994a598504fe315f2eabbdf 100644
>> --- a/drivers/mfd/Makefile
>> +++ b/drivers/mfd/Makefile
>> @@ -266,6 +266,7 @@ obj-$(CONFIG_MFD_SUN4I_GPADC)	+= sun4i-gpadc.o
>>  obj-$(CONFIG_MFD_STM32_LPTIMER)	+= stm32-lptimer.o
>>  obj-$(CONFIG_MFD_STM32_TIMERS) 	+= stm32-timers.o
>>  obj-$(CONFIG_MFD_MXS_LRADC)     += mxs-lradc.o
>> +obj-$(CONFIG_MFD_SPACEMIT_PMIC)	+= spacemit-pmic.o
>>  obj-$(CONFIG_MFD_SC27XX_PMIC)	+= sprd-sc27xx-spi.o
>>  obj-$(CONFIG_RAVE_SP_CORE)	+= rave-sp.o
>>  obj-$(CONFIG_MFD_ROHM_BD71828)	+= rohm-bd71828.o
>> diff --git a/drivers/mfd/spacemit-pmic.c b/drivers/mfd/spacemit-pmic.c
>> new file mode 100644
>> index 0000000000000000000000000000000000000000..d9f6785cecbd405821dead13cdf8d1f9fd64e508
>> --- /dev/null
>> +++ b/drivers/mfd/spacemit-pmic.c
>> @@ -0,0 +1,159 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +/*
>> + * Copyright (C) 2024 Troy Mitchell <troymitchell988@gmail.com>
>> + */
>> +
>> +#include <linux/i2c.h>
>> +#include <linux/mfd/core.h>
>> +#include <linux/mfd/spacemit/spacemit-pmic.h>
>> +#include <linux/module.h>
>> +#include <linux/of_device.h>
> 
> I don't see usage of this header. Include what is used directly.
> 
>> +#include <linux/pm_wakeirq.h>
>> +#include <linux/reboot.h>
>> +
>> +P1_REGMAP_CONFIG;
>> +P1_IRQS_DESC;
>> +P1_IRQ_CHIP_DESC;
>> +P1_POWER_KEY_RESOURCES_DESC;
>> +P1_RTC_RESOURCES_DESC;
>> +P1_MFD_CELL;
>> +P1_MFD_MATCH_DATA;
> 
> Hm? Declarations and definitions go here, not to somewhere else.
I will write them here directly in next version
> 
> 
> 
>> +
>> +static int spacemit_pmic_probe(struct i2c_client *client)
>> +{
>> +	const struct spacemit_pmic_match_data *match_data;
>> +	const struct mfd_cell *cells;
>> +	struct spacemit_pmic *pmic;
>> +	int nr_cells, ret;
>> +
>> +	if (!client->irq)
>> +		return dev_err_probe(&client->dev, -ENXIO, "no interrupt supported");
> 
> And why is this fatal error? If interrupt is not supported by hardware,
> why would you add "unsupported" interrupt?
bcs the I2C driver is based on interrupt whatever I2C if use FIFO or dma.
So I judge here whether the client successfully obtains the irq. If not, the I2C
driver is unavailable.

I think I need to delete this `if`? Because if the I2C driver fails to load, the
I2C device driver will not be loaded
> 
>> +
>> +	match_data = of_device_get_match_data(&client->dev);
>> +	if (WARN_ON(!match_data))
>> +		return -EINVAL;
>> +
>> +	pmic = devm_kzalloc(&client->dev, sizeof(*pmic), GFP_KERNEL);
>> +	if (!pmic)
>> +		return -ENOMEM;
>> +
>> +	cells = match_data->mfd_cells;
>> +	nr_cells = match_data->nr_cells;
>> +
>> +	pmic->regmap_cfg = match_data->regmap_cfg;
>> +	pmic->regmap_irq_chip = match_data->regmap_irq_chip;
>> +	pmic->i2c = client;
>> +	pmic->match_data = match_data;
>> +	pmic->regmap = devm_regmap_init_i2c(client, pmic->regmap_cfg);
>> +	if (IS_ERR(pmic->regmap))
>> +		return dev_err_probe(&client->dev,
>> +				     PTR_ERR(pmic->regmap),
>> +				     "regmap initialization failed");
>> +
>> +	regcache_cache_bypass(pmic->regmap, true);
>> +
>> +	i2c_set_clientdata(client, pmic);
>> +
>> +	if (pmic->regmap_irq_chip) {
> 
> 
> It's impossible to have it false. Test your driver.
SpacemiT has another PMIC named P1S. I'm not sure if P1S has these features, and
I don't have a P1S chip to test and verify.
Therefore, I added a judgement here. But I will drop them in next version. I
should add the check only after confirming that P1S really doesn't have these
features
> 
>> +		ret = regmap_add_irq_chip(pmic->regmap, client->irq, IRQF_ONESHOT, -1,
>> +						pmic->regmap_irq_chip, &pmic->irq_data);
>> +		if (ret)
>> +			return dev_err_probe(&client->dev, ret, "failed to add irqchip");
>> +	}
>> +
>> +	dev_pm_set_wake_irq(&client->dev, client->irq);
>> +	device_init_wakeup(&client->dev, true);
>> +
>> +	ret = devm_mfd_add_devices(&client->dev, PLATFORM_DEVID_NONE,
>> +				   cells, nr_cells, NULL, 0,
>> +				   regmap_irq_get_domain(pmic->irq_data));
>> +	if (ret)
>> +		return dev_err_probe(&client->dev, ret, "failed to add MFD devices");
>> +
>> +	if (match_data->shutdown.reg) {
> 
> Also not possible, useless if.
same
> 
>> +		ret = devm_register_sys_off_handler(&client->dev,
>> +						    SYS_OFF_MODE_POWER_OFF_PREPARE,
>> +						    SYS_OFF_PRIO_HIGH,
>> +						    &spacemit_pmic_shutdown,
>> +						    pmic);
>> +		if (ret)
>> +			return dev_err_probe(&client->dev,
>> +					     ret,
>> +					     "failed to register restart handler");
>> +
>> +	}
>> +
>> +	if (match_data->reboot.reg) {
> 
> Also not possible.
same
> 
>> +		ret = devm_register_sys_off_handler(&client->dev,
>> +						    SYS_OFF_MODE_RESTART,
>> +						    SYS_OFF_PRIO_HIGH,
>> +						    &spacemit_pmic_restart,
>> +						    pmic);
>> +		if (ret)
>> +			return dev_err_probe(&client->dev,
>> +					     ret,
>> +					     "failed to register restart handler");
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +static const struct of_device_id spacemit_pmic_of_match[] = {
>> +	{ .compatible = "spacemit,p1", .data = &pmic_p1_match_data },
>> +	{ /* sentinel */ }
>> +};
>> +MODULE_DEVICE_TABLE(of, spacemit_pmic_of_match);
>> +
>> +static struct i2c_driver spacemit_pmic_i2c_driver = {
>> +	.driver = {
>> +		.name = "spacemit-pmic",
>> +		.of_match_table = spacemit_pmic_of_match,
>> +	},
>> +	.probe    = spacemit_pmic_probe,
>> +};
>> +
>> +static int __init spacemit_pmic_init(void)
>> +{
>> +	return platform_driver_register(&spacemit_pmic_i2c_driver);
>> +}
>> +
>> +static void __exit spacemit_pmic_exit(void)
>> +{
>> +	platform_driver_unregister(&spacemit_pmic_i2c_driver);
>> +}
>> +
>> +module_init(spacemit_pmic_init);
>> +module_exit(spacemit_pmic_exit);
> 
> Use proper wrapper for these above.
ok
> 
>> +
>> +MODULE_LICENSE("GPL");
>> +MODULE_DESCRIPTION("mfd core driver for the SpacemiT PMIC");
> 
> ...
> 
>> +
>> +#define P1_MAX_REG			0xA8
>> +
>> +#define P1_BUCK_VSEL_MASK		0xff
>> +#define P1_BUCK_EN_MASK			0x1
>> +
>> +#define P1_BUCK1_CTRL_REG		0x47
>> +#define P1_BUCK2_CTRL_REG		0x4a
> 
> 
> Either lowercase or uppercase hex, not both.
ok
> 
>> +#define P1_BUCK3_CTRL_REG		0x4d
>> +#define P1_BUCK4_CTRL_REG		0x50
>> +#define P1_BUCK5_CTRL_REG		0x53
>> +#define P1_BUCK6_CTRL_REG		0x56
>> +
>> +#define P1_BUCK1_VSEL_REG		0x48
>> +#define P1_BUCK2_VSEL_REG		0x4b
>> +#define P1_BUCK3_VSEL_REG		0x4e
>> +#define P1_BUCK4_VSEL_REG		0x51
>> +#define P1_BUCK5_VSEL_REG		0x54
>> +#define P1_BUCK6_VSEL_REG		0x57
>> +
>> +#define P1_ALDO1_CTRL_REG		0x5b
>> +#define P1_ALDO2_CTRL_REG		0x5e
>> +#define P1_ALDO3_CTRL_REG		0x61
>> +#define P1_ALDO4_CTRL_REG		0x64
>> +
>> +#define P1_ALDO1_VOLT_REG		0x5c
>> +#define P1_ALDO2_VOLT_REG		0x5f
>> +#define P1_ALDO3_VOLT_REG		0x62
>> +#define P1_ALDO4_VOLT_REG		0x65
>> +
>> +#define P1_ALDO_EN_MASK			0x1
>> +#define P1_ALDO_VSEL_MASK		0x7f
>> +
>> +#define P1_DLDO1_CTRL_REG		0x67
>> +#define P1_DLDO2_CTRL_REG		0x6a
>> +#define P1_DLDO3_CTRL_REG		0x6d
>> +#define P1_DLDO4_CTRL_REG		0x70
>> +#define P1_DLDO5_CTRL_REG		0x73
>> +#define P1_DLDO6_CTRL_REG		0x76
>> +#define P1_DLDO7_CTRL_REG		0x79
>> +
>> +#define P1_DLDO1_VOLT_REG		0x68
>> +#define P1_DLDO2_VOLT_REG		0x6b
>> +#define P1_DLDO3_VOLT_REG		0x6e
>> +#define P1_DLDO4_VOLT_REG		0x71
>> +#define P1_DLDO5_VOLT_REG		0x74
>> +#define P1_DLDO6_VOLT_REG		0x77
>> +#define P1_DLDO7_VOLT_REG		0x7a
>> +
>> +#define P1_DLDO_EN_MASK			0x1
>> +#define P1_DLDO_VSEL_MASK		0x7f
>> +
>> +#define P1_SWITCH_CTRL_REG		0x59
>> +#define P1_SWTICH_EN_MASK		0x1
>> +
>> +#define P1_PWR_CTRL2			0x7e
>> +#define P1_SW_SHUTDOWN_BIT_MSK		0x4
>> +#define P1_SW_RESET_BIT_MSK		0x2
>> +
>> +#define P1_E_GPI0_MSK			BIT(0)
>> +#define P1_E_GPI1_MSK			BIT(1)
>> +#define P1_E_GPI2_MSK			BIT(2)
>> +#define P1_E_GPI3_MSK			BIT(3)
>> +#define P1_E_GPI4_MSK			BIT(4)
>> +#define P1_E_GPI5_MSK			BIT(5)
>> +
>> +#define P1_E_ADC_TEMP_MSK		BIT(0)
>> +#define P1_E_ADC_EOC_MSK		BIT(1)
>> +#define P1_E_ADC_EOS_MSK		BIT(2)
>> +#define P1_E_WDT_TO_MSK			BIT(3)
>> +#define P1_E_ALARM_MSK			BIT(4)
>> +#define P1_E_TICK_MSK			BIT(5)
>> +
>> +#define P1_E_LDO_OV_MSK			BIT(0)
>> +#define P1_E_LDO_UV_MSK			BIT(1)
>> +#define P1_E_LDO_SC_MSK			BIT(2)
>> +#define P1_E_SW_SC_MSK			BIT(3)
>> +#define P1_E_TEMP_WARN_MSK		BIT(4)
>> +#define P1_E_TEMP_SEVERE_MSK		BIT(5)
>> +#define P1_E_TEMP_CRIT_MSK		BIT(6)
>> +
>> +#define P1_E_BUCK1_OV_MSK		BIT(0)
>> +#define P1_E_BUCK2_OV_MSK		BIT(1)
>> +#define P1_E_BUCK3_OV_MSK		BIT(2)
>> +#define P1_E_BUCK4_OV_MSK		BIT(3)
>> +#define P1_E_BUCK5_OV_MSK		BIT(4)
>> +#define P1_E_BUCK6_OV_MSK		BIT(5)
>> +
>> +#define P1_E_BUCK1_UV_MSK		BIT(0)
>> +#define P1_E_BUCK2_UV_MSK		BIT(1)
>> +#define P1_E_BUCK3_UV_MSK		BIT(2)
>> +#define P1_E_BUCK4_UV_MSK		BIT(3)
>> +#define P1_E_BUCK5_UV_MSK		BIT(4)
>> +#define P1_E_BUCK6_UV_MSK		BIT(5)
>> +
>> +#define P1_E_BUCK1_SC_MSK		BIT(0)
>> +#define P1_E_BUCK2_SC_MSK		BIT(1)
>> +#define P1_E_BUCK3_SC_MSK		BIT(2)
>> +#define P1_E_BUCK4_SC_MSK		BIT(3)
>> +#define P1_E_BUCK5_SC_MSK		BIT(4)
>> +#define P1_E_BUCK6_SC_MSK		BIT(5)
>> +
>> +#define P1_E_PWRON_RINTR_MSK		BIT(0)
>> +#define P1_E_PWRON_FINTR_MSK		BIT(1)
>> +#define P1_E_PWRON_SINTR_MSK		BIT(2)
>> +#define P1_E_PWRON_LINTR_MSK		BIT(3)
>> +#define P1_E_PWRON_SDINTR_MSK		BIT(4)
>> +#define P1_E_VSYS_OV_MSK		BIT(5)
>> +
>> +#define P1_E_STATUS_REG_BASE		0x91
>> +#define P1_E_EN_REG_BASE		0x98
>> +
>> +#define P1_REGMAP_CONFIG	\
>> +	static const struct regmap_config p1_regmap_config = {	\
>> +		.reg_bits = 8,					\
>> +		.val_bits = 8,					\
>> +		.max_register = P1_MAX_REG,			\
>> +		.cache_type = REGCACHE_RBTREE,			\
>> +	}
>> +
>> +#define P1_IRQS_DESC					\
>> +static const struct regmap_irq p1_irqs[] = {		\
> 
> 
> No, all these defines are just not needed, not readable. Please follow
> existing kernel style - just look at recent drivers in drivers/mfd/ to
> see how they are designed and developed.
ok thanks!
> 
>> +	[P1_E_GPI0] = {					\
>> +		.mask = P1_E_GPI0_MSK,			\
>> +		.reg_offset = 0,			\
>> +	},						\
>> +							\
>> +	[P1_E_GPI1] = {					\
>> +		.mask = P1_E_GPI1_MSK,			\
> 
> Best regards,
> Krzysztof

-- 
Troy Mitchell

Re: [PATCH 2/2] mfd: add new driver for P1 PMIC from SpacemiT

[Jure Repinc]

[-- Attachment #1: Type: text/plain, Size: 956 bytes --]

On 30/12/2024 11:02, Troy Mitchell wrote:

> +static int spacemit_pmic_shutdown(struct sys_off_data *data)
> +{
> +	int ret;
> +	struct spacemit_pmic *pmic = data->cb_data;
> +
> +	ret = regmap_update_bits(pmic->regmap,
> +				 pmic->match_data->shutdown.reg,
> +				 pmic->match_data->shutdown.bit,
> +				 pmic->match_data->shutdown.bit);
> +	if (ret)
> +		dev_err(data->dev, "failed to reboot device!");

reboot -> shutdown

> +	if (match_data->shutdown.reg) {
> +		ret = devm_register_sys_off_handler(&client->dev,
> +						    
SYS_OFF_MODE_POWER_OFF_PREPARE,
> +						    SYS_OFF_PRIO_HIGH,
> +						    
&spacemit_pmic_shutdown,
> +						    pmic);
> +		if (ret)
> +			return dev_err_probe(&client->dev,
> +					     ret,
> +					     "failed to register restart 
handler");

restart -> shutdown



Have a great time,
Jure Repinc

-- 
         Jabber/XMPP: JLP@jabber.org
              Matrix: @jlp:matrix.org
Mastodon/ActivityPub: @JRepin@mstdn.io

[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 4066 bytes --]

Re: [PATCH 2/2] mfd: add new driver for P1 PMIC from SpacemiT

[Lee Jones]

Deleted pointless signed mail without looking at it, since it slows down
mail traversal too much.

-- 
Lee Jones [李琼斯]