[PATCH v6 1/5] dt-bindings: mfd: Initial commit of silergy,sy7636a.yaml

[PATCH v6 1/5] dt-bindings: mfd: Initial commit of silergy,sy7636a.yaml

[Alistair Francis]

Initial support for the Silergy SY7636A Power Management chip
and regulator.

Signed-off-by: Alistair Francis <alistair@alistair23.me>
---
 .../bindings/mfd/silergy,sy7636a.yaml         | 73 +++++++++++++++++++
 1 file changed, 73 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mfd/silergy,sy7636a.yaml

diff --git a/Documentation/devicetree/bindings/mfd/silergy,sy7636a.yaml b/Documentation/devicetree/bindings/mfd/silergy,sy7636a.yaml
new file mode 100644
index 000000000000..9e50f57d5e8d
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/silergy,sy7636a.yaml
@@ -0,0 +1,73 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mfd/silergy,sy7636a.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: silergy sy7636a PMIC
+
+maintainers:
+  - Alistair Francis <alistair@alistair23.me>
+
+properties:
+  compatible:
+    const: silergy,sy7636a
+
+  reg:
+    maxItems: 1
+
+  '#thermal-sensor-cells':
+    const: 0
+
+  epd-pwr-good-gpios:
+    description:
+      Specifying the power good GPIOs. As defined in bindings/gpio.txt.
+    maxItems: 1
+
+  regulators:
+    type: object
+
+    properties:
+      compatible:
+        const: silergy,sy7636a-regulator
+
+      "vcom":
+        type: object
+        $ref: /schemas/regulator/regulator.yaml#
+
+      regulator-name:
+        const: "vcom"
+
+    additionalProperties: false
+
+required:
+  - compatible
+  - reg
+  - '#thermal-sensor-cells'
+
+additionalProperties: false
+
+examples:
+  - |
+    i2c {
+      #address-cells = <1>;
+      #size-cells = <0>;
+
+      pmic@62 {
+        compatible = "silergy,sy7636a";
+        reg = <0x62>;
+        status = "okay";
+        pinctrl-names = "default";
+        pinctrl-0 = <&pinctrl_epdpmic>;
+        #thermal-sensor-cells = <0>;
+
+        regulators {
+          compatible = "silergy,sy7636a-regulator";
+          reg_epdpmic: vcom {
+            regulator-name = "vcom";
+            regulator-boot-on;
+          };
+        };
+      };
+    };
+...
-- 
2.31.1

[PATCH v6 2/5] mfd: sy7636a: Initial commit

[Alistair Francis]

Initial support for the Silergy SY7636A Power Management chip.

Signed-off-by: Alistair Francis <alistair@alistair23.me>
---
 drivers/mfd/Kconfig         |  9 ++++
 drivers/mfd/Makefile        |  1 +
 drivers/mfd/sy7636a.c       | 82 +++++++++++++++++++++++++++++++++++++
 include/linux/mfd/sy7636a.h | 47 +++++++++++++++++++++
 4 files changed, 139 insertions(+)
 create mode 100644 drivers/mfd/sy7636a.c
 create mode 100644 include/linux/mfd/sy7636a.h

diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 5c7f2b100191..7d6cf32b1549 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -1339,6 +1339,15 @@ config MFD_SYSCON
 	  Select this option to enable accessing system control registers
 	  via regmap.
 
+config MFD_SY7636A
+	tristate "Silergy SY7636A Power Management chip"
+	select MFD_CORE
+	select REGMAP_I2C
+	depends on I2C
+	help
+	  Select this option to enable support for the Silergy SY7636A
+	  Power Management chip.
+
 config MFD_DAVINCI_VOICECODEC
 	tristate
 	select MFD_CORE
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index 4f6d2b8a5f76..f95e1e725a95 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -265,6 +265,7 @@ obj-$(CONFIG_MFD_STMFX) 	+= stmfx.o
 obj-$(CONFIG_MFD_KHADAS_MCU) 	+= khadas-mcu.o
 obj-$(CONFIG_MFD_ACER_A500_EC)	+= acer-ec-a500.o
 
+obj-$(CONFIG_MFD_SY7636A)	+= sy7636a.o
 obj-$(CONFIG_SGI_MFD_IOC3)	+= ioc3.o
 obj-$(CONFIG_MFD_SIMPLE_MFD_I2C)	+= simple-mfd-i2c.o
 obj-$(CONFIG_MFD_INTEL_M10_BMC)   += intel-m10-bmc.o
diff --git a/drivers/mfd/sy7636a.c b/drivers/mfd/sy7636a.c
new file mode 100644
index 000000000000..e08f29ea63f8
--- /dev/null
+++ b/drivers/mfd/sy7636a.c
@@ -0,0 +1,82 @@
+// SPDX-License-Identifier: GPL-2.0+
+//
+// MFD parent driver for SY7636A chip
+//
+// Copyright (C) 2021 reMarkable AS - http://www.remarkable.com/
+//
+// Authors: Lars Ivar Miljeteig <lars.ivar.miljeteig@remarkable.com>
+//          Alistair Francis <alistair@alistair23.me>
+//
+// Based on the lp87565 driver by Keerthy <j-keerthy@ti.com>
+
+#include <linux/interrupt.h>
+#include <linux/mfd/core.h>
+#include <linux/module.h>
+#include <linux/of_device.h>
+
+#include <linux/mfd/sy7636a.h>
+
+static const struct regmap_config sy7636a_regmap_config = {
+	.reg_bits = 8,
+	.val_bits = 8,
+};
+
+static const struct mfd_cell sy7636a_cells[] = {
+	{ .name = "sy7636a-regulator", },
+	{ .name = "sy7636a-temperature", },
+	{ .name = "sy7636a-thermal", },
+};
+
+static const struct of_device_id of_sy7636a_match_table[] = {
+	{ .compatible = "silergy,sy7636a", },
+	{}
+};
+MODULE_DEVICE_TABLE(of, of_sy7636a_match_table);
+
+static int sy7636a_probe(struct i2c_client *client,
+			 const struct i2c_device_id *ids)
+{
+	struct sy7636a *sy7636a;
+	int ret;
+
+	sy7636a = devm_kzalloc(&client->dev, sizeof(*sy7636a), GFP_KERNEL);
+	if (!sy7636a)
+		return -ENOMEM;
+
+	sy7636a->dev = &client->dev;
+
+	sy7636a->regmap = devm_regmap_init_i2c(client, &sy7636a_regmap_config);
+	if (IS_ERR(sy7636a->regmap)) {
+		ret = PTR_ERR(sy7636a->regmap);
+		dev_err(sy7636a->dev,
+			"Failed to initialize register map: %d\n", ret);
+		return ret;
+	}
+
+	i2c_set_clientdata(client, sy7636a);
+
+	ret = devm_mfd_add_devices(sy7636a->dev, PLATFORM_DEVID_AUTO,
+					sy7636a_cells, ARRAY_SIZE(sy7636a_cells),
+					NULL, 0, NULL);
+	return 0;
+}
+
+static const struct i2c_device_id sy7636a_id_table[] = {
+	{ "sy7636a", 0 },
+	{ },
+};
+MODULE_DEVICE_TABLE(i2c, sy7636a_id_table);
+
+static struct i2c_driver sy7636a_driver = {
+	.driver	= {
+		.name	= "sy7636a",
+		.of_match_table = of_sy7636a_match_table,
+	},
+	.probe = sy7636a_probe,
+	.id_table = sy7636a_id_table,
+};
+module_i2c_driver(sy7636a_driver);
+
+MODULE_AUTHOR("Lars Ivar Miljeteig <lars.ivar.miljeteig@remarkable.com>");
+MODULE_DESCRIPTION("Silergy SY7636A Multi-Function Device Driver");
+MODULE_LICENSE("GPL v2");
diff --git a/include/linux/mfd/sy7636a.h b/include/linux/mfd/sy7636a.h
new file mode 100644
index 000000000000..8e71a6e21791
--- /dev/null
+++ b/include/linux/mfd/sy7636a.h
@@ -0,0 +1,47 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Functions to access SY3686A power management chip.
+ *
+ * Copyright (C) 2021 reMarkable AS - http://www.remarkable.com/
+ */
+
+#ifndef __MFD_SY7636A_H
+#define __MFD_SY7636A_H
+
+#include <linux/i2c.h>
+#include <linux/regmap.h>
+#include <linux/regulator/driver.h>
+#include <linux/regulator/machine.h>
+
+#define SY7636A_REG_OPERATION_MODE_CRL		0x00
+#define SY7636A_OPERATION_MODE_CRL_VCOMCTL	BIT(6)
+#define SY7636A_OPERATION_MODE_CRL_ONOFF	BIT(7)
+#define SY7636A_REG_VCOM_ADJUST_CTRL_L		0x01
+#define SY7636A_REG_VCOM_ADJUST_CTRL_H		0x02
+#define SY7636A_REG_VCOM_ADJUST_CTRL_MASK	0x01ff
+#define SY7636A_REG_VLDO_VOLTAGE_ADJULST_CTRL	0x03
+#define SY7636A_REG_POWER_ON_DELAY_TIME		0x06
+#define SY7636A_REG_FAULT_FLAG			0x07
+#define SY7636A_FAULT_FLAG_PG			BIT(0)
+#define SY7636A_REG_TERMISTOR_READOUT		0x08
+
+#define SY7636A_REG_MAX				0x08
+
+#define VCOM_MIN		0
+#define VCOM_MAX		5000
+
+#define VCOM_ADJUST_CTRL_MASK	0x1ff
+// Used to shift the high byte
+#define VCOM_ADJUST_CTRL_SHIFT	8
+// Used to scale from VCOM_ADJUST_CTRL to mv
+#define VCOM_ADJUST_CTRL_SCAL	10
+
+#define FAULT_FLAG_SHIFT	1
+
+struct sy7636a {
+	struct device *dev;
+	struct regmap *regmap;
+	struct gpio_desc *pgood_gpio;
+};
+
+#endif /* __LINUX_MFD_SY7636A_H */
-- 
2.31.1

[PATCH v6 3/5] regulator: sy7636a: Initial commit

[Alistair Francis]

Initial support for the Silergy SY7636A-regulator Power Management chip.

Signed-off-by: Alistair Francis <alistair@alistair23.me>
---
 drivers/regulator/Kconfig             |   6 ++
 drivers/regulator/Makefile            |   1 +
 drivers/regulator/sy7636a-regulator.c | 127 ++++++++++++++++++++++++++
 3 files changed, 134 insertions(+)
 create mode 100644 drivers/regulator/sy7636a-regulator.c

diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
index 3e7a38525cb3..ebb968e8b2b0 100644
--- a/drivers/regulator/Kconfig
+++ b/drivers/regulator/Kconfig
@@ -1150,6 +1150,12 @@ config REGULATOR_STW481X_VMMC
 	  This driver supports the internal VMMC regulator in the STw481x
 	  PMIC chips.
 
+config REGULATOR_SY7636A
+	tristate "Silergy SY7636A voltage regulator"
+	depends on MFD_SY7636A
+	help
+	  This driver supports Silergy SY3686A voltage regulator.
+
 config REGULATOR_SY8106A
 	tristate "Silergy SY8106A regulator"
 	depends on I2C && (OF || COMPILE_TEST)
diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile
index 580b015296ea..b17c24d93f3b 100644
--- a/drivers/regulator/Makefile
+++ b/drivers/regulator/Makefile
@@ -136,6 +136,7 @@ obj-$(CONFIG_REGULATOR_STM32_VREFBUF) += stm32-vrefbuf.o
 obj-$(CONFIG_REGULATOR_STM32_PWR) += stm32-pwr.o
 obj-$(CONFIG_REGULATOR_STPMIC1) += stpmic1_regulator.o
 obj-$(CONFIG_REGULATOR_STW481X_VMMC) += stw481x-vmmc.o
+obj-$(CONFIG_REGULATOR_SY7636A) += sy7636a-regulator.o
 obj-$(CONFIG_REGULATOR_SY8106A) += sy8106a-regulator.o
 obj-$(CONFIG_REGULATOR_SY8824X) += sy8824x.o
 obj-$(CONFIG_REGULATOR_SY8827N) += sy8827n.o
diff --git a/drivers/regulator/sy7636a-regulator.c b/drivers/regulator/sy7636a-regulator.c
new file mode 100644
index 000000000000..c384c2b6ac46
--- /dev/null
+++ b/drivers/regulator/sy7636a-regulator.c
@@ -0,0 +1,127 @@
+// SPDX-License-Identifier: GPL-2.0+
+//
+// Functions to access SY3686A power management chip voltages
+//
+// Copyright (C) 2019 reMarkable AS - http://www.remarkable.com/
+//
+// Authors: Lars Ivar Miljeteig <lars.ivar.miljeteig@remarkable.com>
+//          Alistair Francis <alistair@alistair23.me>
+
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/gpio/consumer.h>
+#include <linux/mfd/sy7636a.h>
+
+#define SY7636A_POLL_ENABLED_TIME 500
+
+static int sy7636a_get_vcom_voltage_op(struct regulator_dev *rdev)
+{
+	int ret;
+	unsigned int val, val_h;
+
+	ret = regmap_read(rdev->regmap, SY7636A_REG_VCOM_ADJUST_CTRL_L, &val);
+	if (ret)
+		return ret;
+
+	ret = regmap_read(rdev->regmap, SY7636A_REG_VCOM_ADJUST_CTRL_H, &val_h);
+	if (ret)
+		return ret;
+
+	val |= (val_h << VCOM_ADJUST_CTRL_SHIFT);
+
+	return (val & VCOM_ADJUST_CTRL_MASK) * VCOM_ADJUST_CTRL_SCAL;
+}
+
+static int sy7636a_get_status(struct regulator_dev *rdev)
+{
+	struct sy7636a *sy7636a = dev_get_drvdata(rdev->dev.parent);
+	int ret = 0;
+
+	ret = gpiod_get_value_cansleep(sy7636a->pgood_gpio);
+	if (ret < 0)
+		dev_err(&rdev->dev, "Failed to read pgood gpio: %d\n", ret);
+
+	return ret;
+}
+
+static const struct regulator_ops sy7636a_vcom_volt_ops = {
+	.get_voltage = sy7636a_get_vcom_voltage_op,
+	.enable = regulator_enable_regmap,
+	.disable = regulator_disable_regmap,
+	.is_enabled = regulator_is_enabled_regmap,
+	.get_status = sy7636a_get_status,
+};
+
+struct regulator_desc desc = {
+	.name = "vcom",
+	.id = 0,
+	.ops = &sy7636a_vcom_volt_ops,
+	.type = REGULATOR_VOLTAGE,
+	.owner = THIS_MODULE,
+	.enable_reg = SY7636A_REG_OPERATION_MODE_CRL,
+	.enable_mask = SY7636A_OPERATION_MODE_CRL_ONOFF,
+	.poll_enabled_time	= SY7636A_POLL_ENABLED_TIME,
+	.regulators_node = of_match_ptr("regulators"),
+	.of_match = of_match_ptr("vcom"),
+};
+
+static int sy7636a_regulator_probe(struct platform_device *pdev)
+{
+	struct sy7636a *sy7636a = dev_get_drvdata(pdev->dev.parent);
+	struct regulator_config config = { };
+	struct regulator_dev *rdev;
+	struct gpio_desc *gdp;
+	int ret;
+
+	if (!sy7636a)
+		return -EPROBE_DEFER;
+
+	platform_set_drvdata(pdev, sy7636a);
+
+	gdp = devm_gpiod_get(sy7636a->dev, "epd-pwr-good", GPIOD_IN);
+	if (IS_ERR(gdp)) {
+		dev_err(sy7636a->dev, "Power good GPIO fault %ld\n", PTR_ERR(gdp));
+		return PTR_ERR(gdp);
+	}
+
+	sy7636a->pgood_gpio = gdp;
+
+	ret = regmap_write(sy7636a->regmap, SY7636A_REG_POWER_ON_DELAY_TIME, 0x0);
+	if (ret) {
+		dev_err(sy7636a->dev, "Failed to initialize regulator: %d\n", ret);
+		return ret;
+	}
+
+	config.dev = &pdev->dev;
+	config.dev->of_node = sy7636a->dev->of_node;
+	config.driver_data = sy7636a;
+	config.regmap = sy7636a->regmap;
+
+	rdev = devm_regulator_register(&pdev->dev, &desc, &config);
+	if (IS_ERR(rdev)) {
+		dev_err(sy7636a->dev, "Failed to register %s regulator\n",
+			pdev->name);
+		return PTR_ERR(rdev);
+	}
+
+	return 0;
+}
+
+static const struct platform_device_id sy7636a_regulator_id_table[] = {
+	{ "sy7636a-regulator", },
+};
+MODULE_DEVICE_TABLE(platform, sy7636a_regulator_id_table);
+
+static struct platform_driver sy7636a_regulator_driver = {
+	.driver = {
+		.name = "sy7636a-regulator",
+	},
+	.probe = sy7636a_regulator_probe,
+	.id_table = sy7636a_regulator_id_table,
+};
+module_platform_driver(sy7636a_regulator_driver);
+
+MODULE_AUTHOR("Lars Ivar Miljeteig <lars.ivar.miljeteig@remarkable.com>");
+MODULE_DESCRIPTION("SY7636A voltage regulator driver");
+MODULE_LICENSE("GPL v2");
-- 
2.31.1

[PATCH v6 4/5] ARM: imx_v6_v7_defconfig: Enable silergy,sy7636a

[Alistair Francis]

Enable the silergy,sy7636a and silergy,sy7636a-regulator for the
reMarkable2.

Signed-off-by: Alistair Francis <alistair@alistair23.me>
---
 arch/arm/configs/imx_v6_v7_defconfig | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm/configs/imx_v6_v7_defconfig b/arch/arm/configs/imx_v6_v7_defconfig
index cd80e85d37cf..bafd1d7b4ad5 100644
--- a/arch/arm/configs/imx_v6_v7_defconfig
+++ b/arch/arm/configs/imx_v6_v7_defconfig
@@ -245,6 +245,7 @@ CONFIG_MFD_MC13XXX_I2C=y
 CONFIG_MFD_RN5T618=y
 CONFIG_MFD_STMPE=y
 CONFIG_REGULATOR=y
+CONFIG_MFD_SY7636A=y
 CONFIG_REGULATOR_FIXED_VOLTAGE=y
 CONFIG_REGULATOR_ANATOP=y
 CONFIG_REGULATOR_DA9052=y
@@ -255,6 +256,7 @@ CONFIG_REGULATOR_MC13783=y
 CONFIG_REGULATOR_MC13892=y
 CONFIG_REGULATOR_PFUZE100=y
 CONFIG_REGULATOR_RN5T618=y
+CONFIG_REGULATOR_SY7636A=y
 CONFIG_RC_CORE=y
 CONFIG_RC_DEVICES=y
 CONFIG_IR_GPIO_CIR=y
-- 
2.31.1

[PATCH v6 5/5] ARM: dts: imx7d: remarkable2: Enable silergy,sy7636a

[Alistair Francis]

Enable the silergy,sy7636a and silergy,sy7636a-regulator on the
reMarkable2.

Signed-off-by: Alistair Francis <alistair@alistair23.me>
---
 arch/arm/boot/dts/imx7d-remarkable2.dts | 61 +++++++++++++++++++++++++
 1 file changed, 61 insertions(+)

diff --git a/arch/arm/boot/dts/imx7d-remarkable2.dts b/arch/arm/boot/dts/imx7d-remarkable2.dts
index 35262c60f015..5876d16c4494 100644
--- a/arch/arm/boot/dts/imx7d-remarkable2.dts
+++ b/arch/arm/boot/dts/imx7d-remarkable2.dts
@@ -22,6 +22,27 @@ memory@80000000 {
 		reg = <0x80000000 0x40000000>;
 	};
 
+	thermal-zones {
+		epd-thermal {
+			thermal-sensors = <&epd_pmic>;
+			polling-delay-passive = <30000>;
+			polling-delay = <30000>;
+			trips {
+				trip0 {
+					temperature = <49000>;
+					hysteresis = <2000>;
+					type = "passive";
+				};
+
+				trip1 {
+					temperature = <50000>;
+					hysteresis = <2000>;
+					type = "critical";
+				};
+			};
+		};
+	};
+
 	reg_brcm: regulator-brcm {
 		compatible = "regulator-fixed";
 		regulator-name = "brcm_reg";
@@ -86,6 +107,32 @@ wacom_digitizer: digitizer@9 {
 	};
 };
 
+&i2c4 {
+	clock-frequency = <100000>;
+	pinctrl-names = "default", "sleep";
+	pinctrl-0 = <&pinctrl_i2c4>;
+	pinctrl-1 = <&pinctrl_i2c4>;
+	status = "okay";
+
+	epd_pmic: sy7636a@62 {
+		compatible = "silergy,sy7636a";
+		reg = <0x62>;
+		status = "okay";
+		pinctrl-names = "default";
+		pinctrl-0 = <&pinctrl_epdpmic>;
+		#thermal-sensor-cells = <0>;
+
+		epd-pwr-good-gpios = <&gpio6 21 GPIO_ACTIVE_HIGH>;
+		regulators {
+			compatible = "silergy,sy7636a-regulator";
+			reg_epdpmic: vcom {
+				regulator-name = "vcom";
+				regulator-boot-on;
+			};
+		};
+	};
+};
+
 &snvs_pwrkey {
 	status = "okay";
 };
@@ -180,6 +227,13 @@ MX7D_PAD_SAI1_TX_BCLK__GPIO6_IO13	0x14
 		>;
 	};
 
+	pinctrl_epdpmic: epdpmicgrp {
+		fsl,pins = <
+			MX7D_PAD_SAI2_RX_DATA__GPIO6_IO21 0x00000074
+			MX7D_PAD_ENET1_RGMII_TXC__GPIO7_IO11 0x00000014
+		>;
+	};
+
 	pinctrl_i2c1: i2c1grp {
 		fsl,pins = <
 			MX7D_PAD_I2C1_SDA__I2C1_SDA		0x4000007f
@@ -187,6 +241,13 @@ MX7D_PAD_I2C1_SCL__I2C1_SCL		0x4000007f
 		>;
 	};
 
+	pinctrl_i2c4: i2c4grp {
+		fsl,pins = <
+			MX7D_PAD_I2C4_SDA__I2C4_SDA		0x4000007f
+			MX7D_PAD_I2C4_SCL__I2C4_SCL		0x4000007f
+		>;
+	};
+
 	pinctrl_uart1: uart1grp {
 		fsl,pins = <
 			MX7D_PAD_UART1_TX_DATA__UART1_DCE_TX	0x79
-- 
2.31.1

Re: (subset) [PATCH v6 1/5] dt-bindings: mfd: Initial commit of silergy,sy7636a.yaml

[Mark Brown]

On Tue, 15 Jun 2021 20:33:56 +1000, Alistair Francis wrote:
> Initial support for the Silergy SY7636A Power Management chip
> and regulator.

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git for-next

Thanks!

[3/5] regulator: sy7636a: Initial commit
      commit: 8c485bedfb7852fa4de2a34aac2a6fd911f539f4

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

Re: [PATCH v6 2/5] mfd: sy7636a: Initial commit

[Lee Jones]

On Tue, 15 Jun 2021, Alistair Francis wrote:

> Initial support for the Silergy SY7636A Power Management chip.
> 
> Signed-off-by: Alistair Francis <alistair@alistair23.me>
> ---
>  drivers/mfd/Kconfig         |  9 ++++
>  drivers/mfd/Makefile        |  1 +
>  drivers/mfd/sy7636a.c       | 82 +++++++++++++++++++++++++++++++++++++
>  include/linux/mfd/sy7636a.h | 47 +++++++++++++++++++++
>  4 files changed, 139 insertions(+)
>  create mode 100644 drivers/mfd/sy7636a.c
>  create mode 100644 include/linux/mfd/sy7636a.h
> 
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index 5c7f2b100191..7d6cf32b1549 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -1339,6 +1339,15 @@ config MFD_SYSCON
>  	  Select this option to enable accessing system control registers
>  	  via regmap.
>  
> +config MFD_SY7636A
> +	tristate "Silergy SY7636A Power Management chip"
> +	select MFD_CORE
> +	select REGMAP_I2C
> +	depends on I2C
> +	help
> +	  Select this option to enable support for the Silergy SY7636A
> +	  Power Management chip.
> +
>  config MFD_DAVINCI_VOICECODEC
>  	tristate
>  	select MFD_CORE
> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> index 4f6d2b8a5f76..f95e1e725a95 100644
> --- a/drivers/mfd/Makefile
> +++ b/drivers/mfd/Makefile
> @@ -265,6 +265,7 @@ obj-$(CONFIG_MFD_STMFX) 	+= stmfx.o
>  obj-$(CONFIG_MFD_KHADAS_MCU) 	+= khadas-mcu.o
>  obj-$(CONFIG_MFD_ACER_A500_EC)	+= acer-ec-a500.o
>  
> +obj-$(CONFIG_MFD_SY7636A)	+= sy7636a.o
>  obj-$(CONFIG_SGI_MFD_IOC3)	+= ioc3.o
>  obj-$(CONFIG_MFD_SIMPLE_MFD_I2C)	+= simple-mfd-i2c.o
>  obj-$(CONFIG_MFD_INTEL_M10_BMC)   += intel-m10-bmc.o
> diff --git a/drivers/mfd/sy7636a.c b/drivers/mfd/sy7636a.c
> new file mode 100644
> index 000000000000..e08f29ea63f8
> --- /dev/null
> +++ b/drivers/mfd/sy7636a.c
> @@ -0,0 +1,82 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +//

Only the SPDX with C++ style comments please.

> +// MFD parent driver for SY7636A chip

Drop the MFD part.  It's a Linuxisum that doesn't really exist.

> +// Copyright (C) 2021 reMarkable AS - http://www.remarkable.com/
> +//
> +// Authors: Lars Ivar Miljeteig <lars.ivar.miljeteig@remarkable.com>
> +//          Alistair Francis <alistair@alistair23.me>
> +//
> +// Based on the lp87565 driver by Keerthy <j-keerthy@ti.com>
> +
> +#include <linux/interrupt.h>
> +#include <linux/mfd/core.h>
> +#include <linux/module.h>
> +#include <linux/of_device.h>
> +
> +#include <linux/mfd/sy7636a.h>
> +
> +static const struct regmap_config sy7636a_regmap_config = {
> +	.reg_bits = 8,
> +	.val_bits = 8,
> +};
> +
> +static const struct mfd_cell sy7636a_cells[] = {
> +	{ .name = "sy7636a-regulator", },
> +	{ .name = "sy7636a-temperature", },
> +	{ .name = "sy7636a-thermal", },
> +};
> +
> +static const struct of_device_id of_sy7636a_match_table[] = {
> +	{ .compatible = "silergy,sy7636a", },
> +	{}
> +};
> +MODULE_DEVICE_TABLE(of, of_sy7636a_match_table);

Hold on.  This driver doesn't really do anything.  If you create OF
nodes for all the sub-devices, you can use simple-mfd-i2c.

Any reason you can't do that?

-- 
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH v6 2/5] mfd: sy7636a: Initial commit

[Alistair Francis]

On Wed, Jun 16, 2021 at 8:56 PM Lee Jones <lee.jones@linaro.org> wrote:
>
> On Tue, 15 Jun 2021, Alistair Francis wrote:
>
> > Initial support for the Silergy SY7636A Power Management chip.
> >
> > Signed-off-by: Alistair Francis <alistair@alistair23.me>
> > ---
> >  drivers/mfd/Kconfig         |  9 ++++
> >  drivers/mfd/Makefile        |  1 +
> >  drivers/mfd/sy7636a.c       | 82 +++++++++++++++++++++++++++++++++++++
> >  include/linux/mfd/sy7636a.h | 47 +++++++++++++++++++++
> >  4 files changed, 139 insertions(+)
> >  create mode 100644 drivers/mfd/sy7636a.c
> >  create mode 100644 include/linux/mfd/sy7636a.h
> >
> > diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> > index 5c7f2b100191..7d6cf32b1549 100644
> > --- a/drivers/mfd/Kconfig
> > +++ b/drivers/mfd/Kconfig
> > @@ -1339,6 +1339,15 @@ config MFD_SYSCON
> >         Select this option to enable accessing system control registers
> >         via regmap.
> >
> > +config MFD_SY7636A
> > +     tristate "Silergy SY7636A Power Management chip"
> > +     select MFD_CORE
> > +     select REGMAP_I2C
> > +     depends on I2C
> > +     help
> > +       Select this option to enable support for the Silergy SY7636A
> > +       Power Management chip.
> > +
> >  config MFD_DAVINCI_VOICECODEC
> >       tristate
> >       select MFD_CORE
> > diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> > index 4f6d2b8a5f76..f95e1e725a95 100644
> > --- a/drivers/mfd/Makefile
> > +++ b/drivers/mfd/Makefile
> > @@ -265,6 +265,7 @@ obj-$(CONFIG_MFD_STMFX)   += stmfx.o
> >  obj-$(CONFIG_MFD_KHADAS_MCU)         += khadas-mcu.o
> >  obj-$(CONFIG_MFD_ACER_A500_EC)       += acer-ec-a500.o
> >
> > +obj-$(CONFIG_MFD_SY7636A)    += sy7636a.o
> >  obj-$(CONFIG_SGI_MFD_IOC3)   += ioc3.o
> >  obj-$(CONFIG_MFD_SIMPLE_MFD_I2C)     += simple-mfd-i2c.o
> >  obj-$(CONFIG_MFD_INTEL_M10_BMC)   += intel-m10-bmc.o
> > diff --git a/drivers/mfd/sy7636a.c b/drivers/mfd/sy7636a.c
> > new file mode 100644
> > index 000000000000..e08f29ea63f8
> > --- /dev/null
> > +++ b/drivers/mfd/sy7636a.c
> > @@ -0,0 +1,82 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +//
>
> Only the SPDX with C++ style comments please.
>
> > +// MFD parent driver for SY7636A chip
>
> Drop the MFD part.  It's a Linuxisum that doesn't really exist.
>
> > +// Copyright (C) 2021 reMarkable AS - http://www.remarkable.com/
> > +//
> > +// Authors: Lars Ivar Miljeteig <lars.ivar.miljeteig@remarkable.com>
> > +//          Alistair Francis <alistair@alistair23.me>
> > +//
> > +// Based on the lp87565 driver by Keerthy <j-keerthy@ti.com>
> > +
> > +#include <linux/interrupt.h>
> > +#include <linux/mfd/core.h>
> > +#include <linux/module.h>
> > +#include <linux/of_device.h>
> > +
> > +#include <linux/mfd/sy7636a.h>
> > +
> > +static const struct regmap_config sy7636a_regmap_config = {
> > +     .reg_bits = 8,
> > +     .val_bits = 8,
> > +};
> > +
> > +static const struct mfd_cell sy7636a_cells[] = {
> > +     { .name = "sy7636a-regulator", },
> > +     { .name = "sy7636a-temperature", },
> > +     { .name = "sy7636a-thermal", },
> > +};
> > +
> > +static const struct of_device_id of_sy7636a_match_table[] = {
> > +     { .compatible = "silergy,sy7636a", },
> > +     {}
> > +};
> > +MODULE_DEVICE_TABLE(of, of_sy7636a_match_table);
>
> Hold on.  This driver doesn't really do anything.  If you create OF
> nodes for all the sub-devices, you can use simple-mfd-i2c.
>
> Any reason you can't do that?

Just to confirm, you mean something like this?

diff --git a/arch/arm/boot/dts/imx7d-remarkable2.dts
b/arch/arm/boot/dts/imx7d-remarkable2.dts
index 9327d1c06c96..3577104b3853 100644
--- a/arch/arm/boot/dts/imx7d-remarkable2.dts
+++ b/arch/arm/boot/dts/imx7d-remarkable2.dts
@@ -382,6 +382,21 @@ epd_pmic: sy7636a@62 {
                pinctrl-0 = <&pinctrl_epdpmic>;
                #thermal-sensor-cells = <0>;

+               regulator@0 {
+                       compatible = "sy7636a-regulator";
+                       reg = <0>;
+               };
+
+               temperature@0 {
+                       compatible = "sy7636a-temperature";
+                       reg = <0>;
+               };
+
+               thermal@0 {
+                       compatible = "sy7636a-thermal";
+                       reg = <0>;
+               };
+
                regulators {
                        compatible = "silergy,sy7636a-regulator";
diff --git a/drivers/mfd/simple-mfd-i2c.c b/drivers/mfd/simple-mfd-i2c.c
index 87f684cff9a1..622a05318cff 100644
--- a/drivers/mfd/simple-mfd-i2c.c
+++ b/drivers/mfd/simple-mfd-i2c.c
@@ -39,6 +39,7 @@ static int simple_mfd_i2c_probe(struct i2c_client *i2c)

 static const struct of_device_id simple_mfd_i2c_of_match[] = {
        { .compatible = "kontron,sl28cpld" },
+       { .compatible = "silergy,sy7636a" },
        {}
 };
 MODULE_DEVICE_TABLE(of, simple_mfd_i2c_of_match);

Alistair

>
> --
> Lee Jones [李琼斯]
> Senior Technical Lead - Developer Services
> Linaro.org │ Open source software for Arm SoCs
> Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH v6 2/5] mfd: sy7636a: Initial commit

[Lee Jones]

On Thu, 17 Jun 2021, Alistair Francis wrote:

> On Wed, Jun 16, 2021 at 8:56 PM Lee Jones <lee.jones@linaro.org> wrote:
> >
> > On Tue, 15 Jun 2021, Alistair Francis wrote:
> >
> > > Initial support for the Silergy SY7636A Power Management chip.
> > >
> > > Signed-off-by: Alistair Francis <alistair@alistair23.me>
> > > ---
> > >  drivers/mfd/Kconfig         |  9 ++++
> > >  drivers/mfd/Makefile        |  1 +
> > >  drivers/mfd/sy7636a.c       | 82 +++++++++++++++++++++++++++++++++++++
> > >  include/linux/mfd/sy7636a.h | 47 +++++++++++++++++++++
> > >  4 files changed, 139 insertions(+)
> > >  create mode 100644 drivers/mfd/sy7636a.c
> > >  create mode 100644 include/linux/mfd/sy7636a.h
> > >
> > > diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> > > index 5c7f2b100191..7d6cf32b1549 100644
> > > --- a/drivers/mfd/Kconfig
> > > +++ b/drivers/mfd/Kconfig
> > > @@ -1339,6 +1339,15 @@ config MFD_SYSCON
> > >         Select this option to enable accessing system control registers
> > >         via regmap.
> > >
> > > +config MFD_SY7636A
> > > +     tristate "Silergy SY7636A Power Management chip"
> > > +     select MFD_CORE
> > > +     select REGMAP_I2C
> > > +     depends on I2C
> > > +     help
> > > +       Select this option to enable support for the Silergy SY7636A
> > > +       Power Management chip.
> > > +
> > >  config MFD_DAVINCI_VOICECODEC
> > >       tristate
> > >       select MFD_CORE
> > > diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> > > index 4f6d2b8a5f76..f95e1e725a95 100644
> > > --- a/drivers/mfd/Makefile
> > > +++ b/drivers/mfd/Makefile
> > > @@ -265,6 +265,7 @@ obj-$(CONFIG_MFD_STMFX)   += stmfx.o
> > >  obj-$(CONFIG_MFD_KHADAS_MCU)         += khadas-mcu.o
> > >  obj-$(CONFIG_MFD_ACER_A500_EC)       += acer-ec-a500.o
> > >
> > > +obj-$(CONFIG_MFD_SY7636A)    += sy7636a.o
> > >  obj-$(CONFIG_SGI_MFD_IOC3)   += ioc3.o
> > >  obj-$(CONFIG_MFD_SIMPLE_MFD_I2C)     += simple-mfd-i2c.o
> > >  obj-$(CONFIG_MFD_INTEL_M10_BMC)   += intel-m10-bmc.o
> > > diff --git a/drivers/mfd/sy7636a.c b/drivers/mfd/sy7636a.c
> > > new file mode 100644
> > > index 000000000000..e08f29ea63f8
> > > --- /dev/null
> > > +++ b/drivers/mfd/sy7636a.c
> > > @@ -0,0 +1,82 @@
> > > +// SPDX-License-Identifier: GPL-2.0+
> > > +//
> >
> > Only the SPDX with C++ style comments please.
> >
> > > +// MFD parent driver for SY7636A chip
> >
> > Drop the MFD part.  It's a Linuxisum that doesn't really exist.
> >
> > > +// Copyright (C) 2021 reMarkable AS - http://www.remarkable.com/
> > > +//
> > > +// Authors: Lars Ivar Miljeteig <lars.ivar.miljeteig@remarkable.com>
> > > +//          Alistair Francis <alistair@alistair23.me>
> > > +//
> > > +// Based on the lp87565 driver by Keerthy <j-keerthy@ti.com>
> > > +
> > > +#include <linux/interrupt.h>
> > > +#include <linux/mfd/core.h>
> > > +#include <linux/module.h>
> > > +#include <linux/of_device.h>
> > > +
> > > +#include <linux/mfd/sy7636a.h>
> > > +
> > > +static const struct regmap_config sy7636a_regmap_config = {
> > > +     .reg_bits = 8,
> > > +     .val_bits = 8,
> > > +};
> > > +
> > > +static const struct mfd_cell sy7636a_cells[] = {
> > > +     { .name = "sy7636a-regulator", },
> > > +     { .name = "sy7636a-temperature", },
> > > +     { .name = "sy7636a-thermal", },
> > > +};
> > > +
> > > +static const struct of_device_id of_sy7636a_match_table[] = {
> > > +     { .compatible = "silergy,sy7636a", },
> > > +     {}
> > > +};
> > > +MODULE_DEVICE_TABLE(of, of_sy7636a_match_table);
> >
> > Hold on.  This driver doesn't really do anything.  If you create OF
> > nodes for all the sub-devices, you can use simple-mfd-i2c.
> >
> > Any reason you can't do that?
> 
> Just to confirm, you mean something like this?
> 
> diff --git a/arch/arm/boot/dts/imx7d-remarkable2.dts
> b/arch/arm/boot/dts/imx7d-remarkable2.dts
> index 9327d1c06c96..3577104b3853 100644
> --- a/arch/arm/boot/dts/imx7d-remarkable2.dts
> +++ b/arch/arm/boot/dts/imx7d-remarkable2.dts
> @@ -382,6 +382,21 @@ epd_pmic: sy7636a@62 {
>                 pinctrl-0 = <&pinctrl_epdpmic>;
>                 #thermal-sensor-cells = <0>;
> 
> +               regulator@0 {
> +                       compatible = "sy7636a-regulator";
> +                       reg = <0>;
> +               };
> +
> +               temperature@0 {
> +                       compatible = "sy7636a-temperature";
> +                       reg = <0>;
> +               };
> +
> +               thermal@0 {
> +                       compatible = "sy7636a-thermal";
> +                       reg = <0>;
> +               };
> +
>                 regulators {
>                         compatible = "silergy,sy7636a-regulator";
> diff --git a/drivers/mfd/simple-mfd-i2c.c b/drivers/mfd/simple-mfd-i2c.c
> index 87f684cff9a1..622a05318cff 100644
> --- a/drivers/mfd/simple-mfd-i2c.c
> +++ b/drivers/mfd/simple-mfd-i2c.c
> @@ -39,6 +39,7 @@ static int simple_mfd_i2c_probe(struct i2c_client *i2c)
> 
>  static const struct of_device_id simple_mfd_i2c_of_match[] = {
>         { .compatible = "kontron,sl28cpld" },
> +       { .compatible = "silergy,sy7636a" },
>         {}
>  };
>  MODULE_DEVICE_TABLE(of, simple_mfd_i2c_of_match);

Essentially.  Take a look at how the other users are implementing.

The reg entries look bogus to me though.  Maybe just leave them out?

-- 
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH v6 1/5] dt-bindings: mfd: Initial commit of silergy,sy7636a.yaml

[Rob Herring]

On Tue, Jun 15, 2021 at 08:33:56PM +1000, Alistair Francis wrote:
> Initial support for the Silergy SY7636A Power Management chip
> and regulator.
> 
> Signed-off-by: Alistair Francis <alistair@alistair23.me>
> ---
>  .../bindings/mfd/silergy,sy7636a.yaml         | 73 +++++++++++++++++++
>  1 file changed, 73 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mfd/silergy,sy7636a.yaml
> 
> diff --git a/Documentation/devicetree/bindings/mfd/silergy,sy7636a.yaml b/Documentation/devicetree/bindings/mfd/silergy,sy7636a.yaml
> new file mode 100644
> index 000000000000..9e50f57d5e8d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/silergy,sy7636a.yaml
> @@ -0,0 +1,73 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/mfd/silergy,sy7636a.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: silergy sy7636a PMIC
> +
> +maintainers:
> +  - Alistair Francis <alistair@alistair23.me>
> +
> +properties:
> +  compatible:
> +    const: silergy,sy7636a
> +
> +  reg:
> +    maxItems: 1
> +
> +  '#thermal-sensor-cells':
> +    const: 0
> +
> +  epd-pwr-good-gpios:
> +    description:
> +      Specifying the power good GPIOs. As defined in bindings/gpio.txt.

Drop the 2nd sentence.

> +    maxItems: 1
> +
> +  regulators:
> +    type: object
> +
> +    properties:
> +      compatible:
> +        const: silergy,sy7636a-regulator
> +
> +      "vcom":

Don't need quotes.

> +        type: object
> +        $ref: /schemas/regulator/regulator.yaml#
> +
> +      regulator-name:
> +        const: "vcom"

Don't need quotes.

Doesn't this belong in the 'vcom' node? You need another 'properties' 
and this under it.


> +
> +    additionalProperties: false
> +
> +required:
> +  - compatible
> +  - reg
> +  - '#thermal-sensor-cells'
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    i2c {
> +      #address-cells = <1>;
> +      #size-cells = <0>;
> +
> +      pmic@62 {
> +        compatible = "silergy,sy7636a";
> +        reg = <0x62>;
> +        status = "okay";

Don't show status in examples.

> +        pinctrl-names = "default";
> +        pinctrl-0 = <&pinctrl_epdpmic>;
> +        #thermal-sensor-cells = <0>;
> +
> +        regulators {
> +          compatible = "silergy,sy7636a-regulator";
> +          reg_epdpmic: vcom {
> +            regulator-name = "vcom";
> +            regulator-boot-on;
> +          };
> +        };
> +      };
> +    };
> +...
> -- 
> 2.31.1
> 
> 

Re: [PATCH v6 2/5] mfd: sy7636a: Initial commit

[Alistair Francis]

On Thu, Jun 17, 2021 at 7:20 PM Lee Jones <lee.jones@linaro.org> wrote:
>
> On Thu, 17 Jun 2021, Alistair Francis wrote:
>
> > On Wed, Jun 16, 2021 at 8:56 PM Lee Jones <lee.jones@linaro.org> wrote:
> > >
> > > On Tue, 15 Jun 2021, Alistair Francis wrote:
> > >
> > > > Initial support for the Silergy SY7636A Power Management chip.
> > > >
> > > > Signed-off-by: Alistair Francis <alistair@alistair23.me>
> > > > ---
> > > >  drivers/mfd/Kconfig         |  9 ++++
> > > >  drivers/mfd/Makefile        |  1 +
> > > >  drivers/mfd/sy7636a.c       | 82 +++++++++++++++++++++++++++++++++++++
> > > >  include/linux/mfd/sy7636a.h | 47 +++++++++++++++++++++
> > > >  4 files changed, 139 insertions(+)
> > > >  create mode 100644 drivers/mfd/sy7636a.c
> > > >  create mode 100644 include/linux/mfd/sy7636a.h
> > > >
> > > > diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> > > > index 5c7f2b100191..7d6cf32b1549 100644
> > > > --- a/drivers/mfd/Kconfig
> > > > +++ b/drivers/mfd/Kconfig
> > > > @@ -1339,6 +1339,15 @@ config MFD_SYSCON
> > > >         Select this option to enable accessing system control registers
> > > >         via regmap.
> > > >
> > > > +config MFD_SY7636A
> > > > +     tristate "Silergy SY7636A Power Management chip"
> > > > +     select MFD_CORE
> > > > +     select REGMAP_I2C
> > > > +     depends on I2C
> > > > +     help
> > > > +       Select this option to enable support for the Silergy SY7636A
> > > > +       Power Management chip.
> > > > +
> > > >  config MFD_DAVINCI_VOICECODEC
> > > >       tristate
> > > >       select MFD_CORE
> > > > diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> > > > index 4f6d2b8a5f76..f95e1e725a95 100644
> > > > --- a/drivers/mfd/Makefile
> > > > +++ b/drivers/mfd/Makefile
> > > > @@ -265,6 +265,7 @@ obj-$(CONFIG_MFD_STMFX)   += stmfx.o
> > > >  obj-$(CONFIG_MFD_KHADAS_MCU)         += khadas-mcu.o
> > > >  obj-$(CONFIG_MFD_ACER_A500_EC)       += acer-ec-a500.o
> > > >
> > > > +obj-$(CONFIG_MFD_SY7636A)    += sy7636a.o
> > > >  obj-$(CONFIG_SGI_MFD_IOC3)   += ioc3.o
> > > >  obj-$(CONFIG_MFD_SIMPLE_MFD_I2C)     += simple-mfd-i2c.o
> > > >  obj-$(CONFIG_MFD_INTEL_M10_BMC)   += intel-m10-bmc.o
> > > > diff --git a/drivers/mfd/sy7636a.c b/drivers/mfd/sy7636a.c
> > > > new file mode 100644
> > > > index 000000000000..e08f29ea63f8
> > > > --- /dev/null
> > > > +++ b/drivers/mfd/sy7636a.c
> > > > @@ -0,0 +1,82 @@
> > > > +// SPDX-License-Identifier: GPL-2.0+
> > > > +//
> > >
> > > Only the SPDX with C++ style comments please.
> > >
> > > > +// MFD parent driver for SY7636A chip
> > >
> > > Drop the MFD part.  It's a Linuxisum that doesn't really exist.
> > >
> > > > +// Copyright (C) 2021 reMarkable AS - http://www.remarkable.com/
> > > > +//
> > > > +// Authors: Lars Ivar Miljeteig <lars.ivar.miljeteig@remarkable.com>
> > > > +//          Alistair Francis <alistair@alistair23.me>
> > > > +//
> > > > +// Based on the lp87565 driver by Keerthy <j-keerthy@ti.com>
> > > > +
> > > > +#include <linux/interrupt.h>
> > > > +#include <linux/mfd/core.h>
> > > > +#include <linux/module.h>
> > > > +#include <linux/of_device.h>
> > > > +
> > > > +#include <linux/mfd/sy7636a.h>
> > > > +
> > > > +static const struct regmap_config sy7636a_regmap_config = {
> > > > +     .reg_bits = 8,
> > > > +     .val_bits = 8,
> > > > +};
> > > > +
> > > > +static const struct mfd_cell sy7636a_cells[] = {
> > > > +     { .name = "sy7636a-regulator", },
> > > > +     { .name = "sy7636a-temperature", },
> > > > +     { .name = "sy7636a-thermal", },
> > > > +};
> > > > +
> > > > +static const struct of_device_id of_sy7636a_match_table[] = {
> > > > +     { .compatible = "silergy,sy7636a", },
> > > > +     {}
> > > > +};
> > > > +MODULE_DEVICE_TABLE(of, of_sy7636a_match_table);
> > >
> > > Hold on.  This driver doesn't really do anything.  If you create OF
> > > nodes for all the sub-devices, you can use simple-mfd-i2c.
> > >
> > > Any reason you can't do that?
> >
> > Just to confirm, you mean something like this?
> >
> > diff --git a/arch/arm/boot/dts/imx7d-remarkable2.dts
> > b/arch/arm/boot/dts/imx7d-remarkable2.dts
> > index 9327d1c06c96..3577104b3853 100644
> > --- a/arch/arm/boot/dts/imx7d-remarkable2.dts
> > +++ b/arch/arm/boot/dts/imx7d-remarkable2.dts
> > @@ -382,6 +382,21 @@ epd_pmic: sy7636a@62 {
> >                 pinctrl-0 = <&pinctrl_epdpmic>;
> >                 #thermal-sensor-cells = <0>;
> >
> > +               regulator@0 {
> > +                       compatible = "sy7636a-regulator";
> > +                       reg = <0>;
> > +               };
> > +
> > +               temperature@0 {
> > +                       compatible = "sy7636a-temperature";
> > +                       reg = <0>;
> > +               };
> > +
> > +               thermal@0 {
> > +                       compatible = "sy7636a-thermal";
> > +                       reg = <0>;
> > +               };
> > +
> >                 regulators {
> >                         compatible = "silergy,sy7636a-regulator";
> > diff --git a/drivers/mfd/simple-mfd-i2c.c b/drivers/mfd/simple-mfd-i2c.c
> > index 87f684cff9a1..622a05318cff 100644
> > --- a/drivers/mfd/simple-mfd-i2c.c
> > +++ b/drivers/mfd/simple-mfd-i2c.c
> > @@ -39,6 +39,7 @@ static int simple_mfd_i2c_probe(struct i2c_client *i2c)
> >
> >  static const struct of_device_id simple_mfd_i2c_of_match[] = {
> >         { .compatible = "kontron,sl28cpld" },
> > +       { .compatible = "silergy,sy7636a" },
> >         {}
> >  };
> >  MODULE_DEVICE_TABLE(of, simple_mfd_i2c_of_match);
>
> Essentially.  Take a look at how the other users are implementing.
>
> The reg entries look bogus to me though.  Maybe just leave them out?

So I tried this and didn't have any luck.

After some Kconfig changes to allow it to build, I managed to get it
probing, but I never got it to power up. It doesn't seem to be the
same.

Alistair

>
> --
> Lee Jones [李琼斯]
> Senior Technical Lead - Developer Services
> Linaro.org │ Open source software for Arm SoCs
> Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH v6 2/5] mfd: sy7636a: Initial commit

[Lee Jones]

On Thu, 08 Jul 2021, Alistair Francis wrote:

> On Thu, Jun 17, 2021 at 7:20 PM Lee Jones <lee.jones@linaro.org> wrote:
> >
> > On Thu, 17 Jun 2021, Alistair Francis wrote:
> >
> > > On Wed, Jun 16, 2021 at 8:56 PM Lee Jones <lee.jones@linaro.org> wrote:
> > > >
> > > > On Tue, 15 Jun 2021, Alistair Francis wrote:
> > > >
> > > > > Initial support for the Silergy SY7636A Power Management chip.
> > > > >
> > > > > Signed-off-by: Alistair Francis <alistair@alistair23.me>
> > > > > ---
> > > > >  drivers/mfd/Kconfig         |  9 ++++
> > > > >  drivers/mfd/Makefile        |  1 +
> > > > >  drivers/mfd/sy7636a.c       | 82 +++++++++++++++++++++++++++++++++++++
> > > > >  include/linux/mfd/sy7636a.h | 47 +++++++++++++++++++++
> > > > >  4 files changed, 139 insertions(+)
> > > > >  create mode 100644 drivers/mfd/sy7636a.c
> > > > >  create mode 100644 include/linux/mfd/sy7636a.h
> > > > >
> > > > > diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> > > > > index 5c7f2b100191..7d6cf32b1549 100644
> > > > > --- a/drivers/mfd/Kconfig
> > > > > +++ b/drivers/mfd/Kconfig
> > > > > @@ -1339,6 +1339,15 @@ config MFD_SYSCON
> > > > >         Select this option to enable accessing system control registers
> > > > >         via regmap.
> > > > >
> > > > > +config MFD_SY7636A
> > > > > +     tristate "Silergy SY7636A Power Management chip"
> > > > > +     select MFD_CORE
> > > > > +     select REGMAP_I2C
> > > > > +     depends on I2C
> > > > > +     help
> > > > > +       Select this option to enable support for the Silergy SY7636A
> > > > > +       Power Management chip.
> > > > > +
> > > > >  config MFD_DAVINCI_VOICECODEC
> > > > >       tristate
> > > > >       select MFD_CORE
> > > > > diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> > > > > index 4f6d2b8a5f76..f95e1e725a95 100644
> > > > > --- a/drivers/mfd/Makefile
> > > > > +++ b/drivers/mfd/Makefile
> > > > > @@ -265,6 +265,7 @@ obj-$(CONFIG_MFD_STMFX)   += stmfx.o
> > > > >  obj-$(CONFIG_MFD_KHADAS_MCU)         += khadas-mcu.o
> > > > >  obj-$(CONFIG_MFD_ACER_A500_EC)       += acer-ec-a500.o
> > > > >
> > > > > +obj-$(CONFIG_MFD_SY7636A)    += sy7636a.o
> > > > >  obj-$(CONFIG_SGI_MFD_IOC3)   += ioc3.o
> > > > >  obj-$(CONFIG_MFD_SIMPLE_MFD_I2C)     += simple-mfd-i2c.o
> > > > >  obj-$(CONFIG_MFD_INTEL_M10_BMC)   += intel-m10-bmc.o
> > > > > diff --git a/drivers/mfd/sy7636a.c b/drivers/mfd/sy7636a.c
> > > > > new file mode 100644
> > > > > index 000000000000..e08f29ea63f8
> > > > > --- /dev/null
> > > > > +++ b/drivers/mfd/sy7636a.c
> > > > > @@ -0,0 +1,82 @@
> > > > > +// SPDX-License-Identifier: GPL-2.0+
> > > > > +//
> > > >
> > > > Only the SPDX with C++ style comments please.
> > > >
> > > > > +// MFD parent driver for SY7636A chip
> > > >
> > > > Drop the MFD part.  It's a Linuxisum that doesn't really exist.
> > > >
> > > > > +// Copyright (C) 2021 reMarkable AS - http://www.remarkable.com/
> > > > > +//
> > > > > +// Authors: Lars Ivar Miljeteig <lars.ivar.miljeteig@remarkable.com>
> > > > > +//          Alistair Francis <alistair@alistair23.me>
> > > > > +//
> > > > > +// Based on the lp87565 driver by Keerthy <j-keerthy@ti.com>
> > > > > +
> > > > > +#include <linux/interrupt.h>
> > > > > +#include <linux/mfd/core.h>
> > > > > +#include <linux/module.h>
> > > > > +#include <linux/of_device.h>
> > > > > +
> > > > > +#include <linux/mfd/sy7636a.h>
> > > > > +
> > > > > +static const struct regmap_config sy7636a_regmap_config = {
> > > > > +     .reg_bits = 8,
> > > > > +     .val_bits = 8,
> > > > > +};
> > > > > +
> > > > > +static const struct mfd_cell sy7636a_cells[] = {
> > > > > +     { .name = "sy7636a-regulator", },
> > > > > +     { .name = "sy7636a-temperature", },
> > > > > +     { .name = "sy7636a-thermal", },
> > > > > +};
> > > > > +
> > > > > +static const struct of_device_id of_sy7636a_match_table[] = {
> > > > > +     { .compatible = "silergy,sy7636a", },
> > > > > +     {}
> > > > > +};
> > > > > +MODULE_DEVICE_TABLE(of, of_sy7636a_match_table);
> > > >
> > > > Hold on.  This driver doesn't really do anything.  If you create OF
> > > > nodes for all the sub-devices, you can use simple-mfd-i2c.
> > > >
> > > > Any reason you can't do that?
> > >
> > > Just to confirm, you mean something like this?
> > >
> > > diff --git a/arch/arm/boot/dts/imx7d-remarkable2.dts
> > > b/arch/arm/boot/dts/imx7d-remarkable2.dts
> > > index 9327d1c06c96..3577104b3853 100644
> > > --- a/arch/arm/boot/dts/imx7d-remarkable2.dts
> > > +++ b/arch/arm/boot/dts/imx7d-remarkable2.dts
> > > @@ -382,6 +382,21 @@ epd_pmic: sy7636a@62 {
> > >                 pinctrl-0 = <&pinctrl_epdpmic>;
> > >                 #thermal-sensor-cells = <0>;
> > >
> > > +               regulator@0 {
> > > +                       compatible = "sy7636a-regulator";
> > > +                       reg = <0>;
> > > +               };
> > > +
> > > +               temperature@0 {
> > > +                       compatible = "sy7636a-temperature";
> > > +                       reg = <0>;
> > > +               };
> > > +
> > > +               thermal@0 {
> > > +                       compatible = "sy7636a-thermal";
> > > +                       reg = <0>;
> > > +               };
> > > +
> > >                 regulators {
> > >                         compatible = "silergy,sy7636a-regulator";
> > > diff --git a/drivers/mfd/simple-mfd-i2c.c b/drivers/mfd/simple-mfd-i2c.c
> > > index 87f684cff9a1..622a05318cff 100644
> > > --- a/drivers/mfd/simple-mfd-i2c.c
> > > +++ b/drivers/mfd/simple-mfd-i2c.c
> > > @@ -39,6 +39,7 @@ static int simple_mfd_i2c_probe(struct i2c_client *i2c)
> > >
> > >  static const struct of_device_id simple_mfd_i2c_of_match[] = {
> > >         { .compatible = "kontron,sl28cpld" },
> > > +       { .compatible = "silergy,sy7636a" },
> > >         {}
> > >  };
> > >  MODULE_DEVICE_TABLE(of, simple_mfd_i2c_of_match);
> >
> > Essentially.  Take a look at how the other users are implementing.
> >
> > The reg entries look bogus to me though.  Maybe just leave them out?
> 
> So I tried this and didn't have any luck.
> 
> After some Kconfig changes to allow it to build, I managed to get it
> probing, but I never got it to power up. It doesn't seem to be the
> same.

I need a more technical reason why this is not the correct approach
for you.  "I can't get it to work" doesn't quite reach the quality
line I'm afraid.

Did you try enabling the debug prints in of_platform_bus_create() and
friends to see if your devices are probing correctly?

-- 
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH v6 2/5] mfd: sy7636a: Initial commit

[Alistair Francis]

On Thu, Jul 8, 2021 at 10:47 PM Lee Jones <lee.jones@linaro.org> wrote:
>
> On Thu, 08 Jul 2021, Alistair Francis wrote:
>
> > On Thu, Jun 17, 2021 at 7:20 PM Lee Jones <lee.jones@linaro.org> wrote:
> > >
> > > On Thu, 17 Jun 2021, Alistair Francis wrote:
> > >
> > > > On Wed, Jun 16, 2021 at 8:56 PM Lee Jones <lee.jones@linaro.org> wrote:
> > > > >
> > > > > On Tue, 15 Jun 2021, Alistair Francis wrote:
> > > > >
> > > > > > Initial support for the Silergy SY7636A Power Management chip.
> > > > > >
> > > > > > Signed-off-by: Alistair Francis <alistair@alistair23.me>
> > > > > > ---
> > > > > >  drivers/mfd/Kconfig         |  9 ++++
> > > > > >  drivers/mfd/Makefile        |  1 +
> > > > > >  drivers/mfd/sy7636a.c       | 82 +++++++++++++++++++++++++++++++++++++
> > > > > >  include/linux/mfd/sy7636a.h | 47 +++++++++++++++++++++
> > > > > >  4 files changed, 139 insertions(+)
> > > > > >  create mode 100644 drivers/mfd/sy7636a.c
> > > > > >  create mode 100644 include/linux/mfd/sy7636a.h
> > > > > >
> > > > > > diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> > > > > > index 5c7f2b100191..7d6cf32b1549 100644
> > > > > > --- a/drivers/mfd/Kconfig
> > > > > > +++ b/drivers/mfd/Kconfig
> > > > > > @@ -1339,6 +1339,15 @@ config MFD_SYSCON
> > > > > >         Select this option to enable accessing system control registers
> > > > > >         via regmap.
> > > > > >
> > > > > > +config MFD_SY7636A
> > > > > > +     tristate "Silergy SY7636A Power Management chip"
> > > > > > +     select MFD_CORE
> > > > > > +     select REGMAP_I2C
> > > > > > +     depends on I2C
> > > > > > +     help
> > > > > > +       Select this option to enable support for the Silergy SY7636A
> > > > > > +       Power Management chip.
> > > > > > +
> > > > > >  config MFD_DAVINCI_VOICECODEC
> > > > > >       tristate
> > > > > >       select MFD_CORE
> > > > > > diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> > > > > > index 4f6d2b8a5f76..f95e1e725a95 100644
> > > > > > --- a/drivers/mfd/Makefile
> > > > > > +++ b/drivers/mfd/Makefile
> > > > > > @@ -265,6 +265,7 @@ obj-$(CONFIG_MFD_STMFX)   += stmfx.o
> > > > > >  obj-$(CONFIG_MFD_KHADAS_MCU)         += khadas-mcu.o
> > > > > >  obj-$(CONFIG_MFD_ACER_A500_EC)       += acer-ec-a500.o
> > > > > >
> > > > > > +obj-$(CONFIG_MFD_SY7636A)    += sy7636a.o
> > > > > >  obj-$(CONFIG_SGI_MFD_IOC3)   += ioc3.o
> > > > > >  obj-$(CONFIG_MFD_SIMPLE_MFD_I2C)     += simple-mfd-i2c.o
> > > > > >  obj-$(CONFIG_MFD_INTEL_M10_BMC)   += intel-m10-bmc.o
> > > > > > diff --git a/drivers/mfd/sy7636a.c b/drivers/mfd/sy7636a.c
> > > > > > new file mode 100644
> > > > > > index 000000000000..e08f29ea63f8
> > > > > > --- /dev/null
> > > > > > +++ b/drivers/mfd/sy7636a.c
> > > > > > @@ -0,0 +1,82 @@
> > > > > > +// SPDX-License-Identifier: GPL-2.0+
> > > > > > +//
> > > > >
> > > > > Only the SPDX with C++ style comments please.
> > > > >
> > > > > > +// MFD parent driver for SY7636A chip
> > > > >
> > > > > Drop the MFD part.  It's a Linuxisum that doesn't really exist.
> > > > >
> > > > > > +// Copyright (C) 2021 reMarkable AS - http://www.remarkable.com/
> > > > > > +//
> > > > > > +// Authors: Lars Ivar Miljeteig <lars.ivar.miljeteig@remarkable.com>
> > > > > > +//          Alistair Francis <alistair@alistair23.me>
> > > > > > +//
> > > > > > +// Based on the lp87565 driver by Keerthy <j-keerthy@ti.com>
> > > > > > +
> > > > > > +#include <linux/interrupt.h>
> > > > > > +#include <linux/mfd/core.h>
> > > > > > +#include <linux/module.h>
> > > > > > +#include <linux/of_device.h>
> > > > > > +
> > > > > > +#include <linux/mfd/sy7636a.h>
> > > > > > +
> > > > > > +static const struct regmap_config sy7636a_regmap_config = {
> > > > > > +     .reg_bits = 8,
> > > > > > +     .val_bits = 8,
> > > > > > +};
> > > > > > +
> > > > > > +static const struct mfd_cell sy7636a_cells[] = {
> > > > > > +     { .name = "sy7636a-regulator", },
> > > > > > +     { .name = "sy7636a-temperature", },
> > > > > > +     { .name = "sy7636a-thermal", },
> > > > > > +};
> > > > > > +
> > > > > > +static const struct of_device_id of_sy7636a_match_table[] = {
> > > > > > +     { .compatible = "silergy,sy7636a", },
> > > > > > +     {}
> > > > > > +};
> > > > > > +MODULE_DEVICE_TABLE(of, of_sy7636a_match_table);
> > > > >
> > > > > Hold on.  This driver doesn't really do anything.  If you create OF
> > > > > nodes for all the sub-devices, you can use simple-mfd-i2c.
> > > > >
> > > > > Any reason you can't do that?
> > > >
> > > > Just to confirm, you mean something like this?
> > > >
> > > > diff --git a/arch/arm/boot/dts/imx7d-remarkable2.dts
> > > > b/arch/arm/boot/dts/imx7d-remarkable2.dts
> > > > index 9327d1c06c96..3577104b3853 100644
> > > > --- a/arch/arm/boot/dts/imx7d-remarkable2.dts
> > > > +++ b/arch/arm/boot/dts/imx7d-remarkable2.dts
> > > > @@ -382,6 +382,21 @@ epd_pmic: sy7636a@62 {
> > > >                 pinctrl-0 = <&pinctrl_epdpmic>;
> > > >                 #thermal-sensor-cells = <0>;
> > > >
> > > > +               regulator@0 {
> > > > +                       compatible = "sy7636a-regulator";
> > > > +                       reg = <0>;
> > > > +               };
> > > > +
> > > > +               temperature@0 {
> > > > +                       compatible = "sy7636a-temperature";
> > > > +                       reg = <0>;
> > > > +               };
> > > > +
> > > > +               thermal@0 {
> > > > +                       compatible = "sy7636a-thermal";
> > > > +                       reg = <0>;
> > > > +               };
> > > > +
> > > >                 regulators {
> > > >                         compatible = "silergy,sy7636a-regulator";
> > > > diff --git a/drivers/mfd/simple-mfd-i2c.c b/drivers/mfd/simple-mfd-i2c.c
> > > > index 87f684cff9a1..622a05318cff 100644
> > > > --- a/drivers/mfd/simple-mfd-i2c.c
> > > > +++ b/drivers/mfd/simple-mfd-i2c.c
> > > > @@ -39,6 +39,7 @@ static int simple_mfd_i2c_probe(struct i2c_client *i2c)
> > > >
> > > >  static const struct of_device_id simple_mfd_i2c_of_match[] = {
> > > >         { .compatible = "kontron,sl28cpld" },
> > > > +       { .compatible = "silergy,sy7636a" },
> > > >         {}
> > > >  };
> > > >  MODULE_DEVICE_TABLE(of, simple_mfd_i2c_of_match);
> > >
> > > Essentially.  Take a look at how the other users are implementing.
> > >
> > > The reg entries look bogus to me though.  Maybe just leave them out?
> >
> > So I tried this and didn't have any luck.
> >
> > After some Kconfig changes to allow it to build, I managed to get it
> > probing, but I never got it to power up. It doesn't seem to be the
> > same.
>
> I need a more technical reason why this is not the correct approach
> for you.  "I can't get it to work" doesn't quite reach the quality
> line I'm afraid.

Yep, so one thing that we need that the simple-mfd-i2c doesn't do is
the epd-pwr-good-gpios GPIO. This is a GPIO line from the mfd that the
regulator uses.

>
> Did you try enabling the debug prints in of_platform_bus_create() and
> friends to see if your devices are probing correctly?

I think I did, but I can have another look and see if I can get it working.

Alistair

>
> --
> Lee Jones [李琼斯]
> Senior Technical Lead - Developer Services
> Linaro.org │ Open source software for Arm SoCs
> Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH v6 2/5] mfd: sy7636a: Initial commit

[Lee Jones]

On Wed, 28 Jul 2021, Alistair Francis wrote:

> On Thu, Jul 8, 2021 at 10:47 PM Lee Jones <lee.jones@linaro.org> wrote:
> >
> > On Thu, 08 Jul 2021, Alistair Francis wrote:
> >
> > > On Thu, Jun 17, 2021 at 7:20 PM Lee Jones <lee.jones@linaro.org> wrote:
> > > >
> > > > On Thu, 17 Jun 2021, Alistair Francis wrote:
> > > >
> > > > > On Wed, Jun 16, 2021 at 8:56 PM Lee Jones <lee.jones@linaro.org> wrote:
> > > > > >
> > > > > > On Tue, 15 Jun 2021, Alistair Francis wrote:
> > > > > >
> > > > > > > Initial support for the Silergy SY7636A Power Management chip.
> > > > > > >
> > > > > > > Signed-off-by: Alistair Francis <alistair@alistair23.me>
> > > > > > > ---
> > > > > > >  drivers/mfd/Kconfig         |  9 ++++
> > > > > > >  drivers/mfd/Makefile        |  1 +
> > > > > > >  drivers/mfd/sy7636a.c       | 82 +++++++++++++++++++++++++++++++++++++
> > > > > > >  include/linux/mfd/sy7636a.h | 47 +++++++++++++++++++++
> > > > > > >  4 files changed, 139 insertions(+)
> > > > > > >  create mode 100644 drivers/mfd/sy7636a.c
> > > > > > >  create mode 100644 include/linux/mfd/sy7636a.h
> > > > > > >
> > > > > > > diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> > > > > > > index 5c7f2b100191..7d6cf32b1549 100644
> > > > > > > --- a/drivers/mfd/Kconfig
> > > > > > > +++ b/drivers/mfd/Kconfig
> > > > > > > @@ -1339,6 +1339,15 @@ config MFD_SYSCON
> > > > > > >         Select this option to enable accessing system control registers
> > > > > > >         via regmap.
> > > > > > >
> > > > > > > +config MFD_SY7636A
> > > > > > > +     tristate "Silergy SY7636A Power Management chip"
> > > > > > > +     select MFD_CORE
> > > > > > > +     select REGMAP_I2C
> > > > > > > +     depends on I2C
> > > > > > > +     help
> > > > > > > +       Select this option to enable support for the Silergy SY7636A
> > > > > > > +       Power Management chip.
> > > > > > > +
> > > > > > >  config MFD_DAVINCI_VOICECODEC
> > > > > > >       tristate
> > > > > > >       select MFD_CORE
> > > > > > > diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> > > > > > > index 4f6d2b8a5f76..f95e1e725a95 100644
> > > > > > > --- a/drivers/mfd/Makefile
> > > > > > > +++ b/drivers/mfd/Makefile
> > > > > > > @@ -265,6 +265,7 @@ obj-$(CONFIG_MFD_STMFX)   += stmfx.o
> > > > > > >  obj-$(CONFIG_MFD_KHADAS_MCU)         += khadas-mcu.o
> > > > > > >  obj-$(CONFIG_MFD_ACER_A500_EC)       += acer-ec-a500.o
> > > > > > >
> > > > > > > +obj-$(CONFIG_MFD_SY7636A)    += sy7636a.o
> > > > > > >  obj-$(CONFIG_SGI_MFD_IOC3)   += ioc3.o
> > > > > > >  obj-$(CONFIG_MFD_SIMPLE_MFD_I2C)     += simple-mfd-i2c.o
> > > > > > >  obj-$(CONFIG_MFD_INTEL_M10_BMC)   += intel-m10-bmc.o
> > > > > > > diff --git a/drivers/mfd/sy7636a.c b/drivers/mfd/sy7636a.c
> > > > > > > new file mode 100644
> > > > > > > index 000000000000..e08f29ea63f8
> > > > > > > --- /dev/null
> > > > > > > +++ b/drivers/mfd/sy7636a.c
> > > > > > > @@ -0,0 +1,82 @@
> > > > > > > +// SPDX-License-Identifier: GPL-2.0+
> > > > > > > +//
> > > > > >
> > > > > > Only the SPDX with C++ style comments please.
> > > > > >
> > > > > > > +// MFD parent driver for SY7636A chip
> > > > > >
> > > > > > Drop the MFD part.  It's a Linuxisum that doesn't really exist.
> > > > > >
> > > > > > > +// Copyright (C) 2021 reMarkable AS - http://www.remarkable.com/
> > > > > > > +//
> > > > > > > +// Authors: Lars Ivar Miljeteig <lars.ivar.miljeteig@remarkable.com>
> > > > > > > +//          Alistair Francis <alistair@alistair23.me>
> > > > > > > +//
> > > > > > > +// Based on the lp87565 driver by Keerthy <j-keerthy@ti.com>
> > > > > > > +
> > > > > > > +#include <linux/interrupt.h>
> > > > > > > +#include <linux/mfd/core.h>
> > > > > > > +#include <linux/module.h>
> > > > > > > +#include <linux/of_device.h>
> > > > > > > +
> > > > > > > +#include <linux/mfd/sy7636a.h>
> > > > > > > +
> > > > > > > +static const struct regmap_config sy7636a_regmap_config = {
> > > > > > > +     .reg_bits = 8,
> > > > > > > +     .val_bits = 8,
> > > > > > > +};
> > > > > > > +
> > > > > > > +static const struct mfd_cell sy7636a_cells[] = {
> > > > > > > +     { .name = "sy7636a-regulator", },
> > > > > > > +     { .name = "sy7636a-temperature", },
> > > > > > > +     { .name = "sy7636a-thermal", },
> > > > > > > +};
> > > > > > > +
> > > > > > > +static const struct of_device_id of_sy7636a_match_table[] = {
> > > > > > > +     { .compatible = "silergy,sy7636a", },
> > > > > > > +     {}
> > > > > > > +};
> > > > > > > +MODULE_DEVICE_TABLE(of, of_sy7636a_match_table);
> > > > > >
> > > > > > Hold on.  This driver doesn't really do anything.  If you create OF
> > > > > > nodes for all the sub-devices, you can use simple-mfd-i2c.
> > > > > >
> > > > > > Any reason you can't do that?
> > > > >
> > > > > Just to confirm, you mean something like this?
> > > > >
> > > > > diff --git a/arch/arm/boot/dts/imx7d-remarkable2.dts
> > > > > b/arch/arm/boot/dts/imx7d-remarkable2.dts
> > > > > index 9327d1c06c96..3577104b3853 100644
> > > > > --- a/arch/arm/boot/dts/imx7d-remarkable2.dts
> > > > > +++ b/arch/arm/boot/dts/imx7d-remarkable2.dts
> > > > > @@ -382,6 +382,21 @@ epd_pmic: sy7636a@62 {
> > > > >                 pinctrl-0 = <&pinctrl_epdpmic>;
> > > > >                 #thermal-sensor-cells = <0>;
> > > > >
> > > > > +               regulator@0 {
> > > > > +                       compatible = "sy7636a-regulator";
> > > > > +                       reg = <0>;
> > > > > +               };
> > > > > +
> > > > > +               temperature@0 {
> > > > > +                       compatible = "sy7636a-temperature";
> > > > > +                       reg = <0>;
> > > > > +               };
> > > > > +
> > > > > +               thermal@0 {
> > > > > +                       compatible = "sy7636a-thermal";
> > > > > +                       reg = <0>;
> > > > > +               };
> > > > > +
> > > > >                 regulators {
> > > > >                         compatible = "silergy,sy7636a-regulator";
> > > > > diff --git a/drivers/mfd/simple-mfd-i2c.c b/drivers/mfd/simple-mfd-i2c.c
> > > > > index 87f684cff9a1..622a05318cff 100644
> > > > > --- a/drivers/mfd/simple-mfd-i2c.c
> > > > > +++ b/drivers/mfd/simple-mfd-i2c.c
> > > > > @@ -39,6 +39,7 @@ static int simple_mfd_i2c_probe(struct i2c_client *i2c)
> > > > >
> > > > >  static const struct of_device_id simple_mfd_i2c_of_match[] = {
> > > > >         { .compatible = "kontron,sl28cpld" },
> > > > > +       { .compatible = "silergy,sy7636a" },
> > > > >         {}
> > > > >  };
> > > > >  MODULE_DEVICE_TABLE(of, simple_mfd_i2c_of_match);
> > > >
> > > > Essentially.  Take a look at how the other users are implementing.
> > > >
> > > > The reg entries look bogus to me though.  Maybe just leave them out?
> > >
> > > So I tried this and didn't have any luck.
> > >
> > > After some Kconfig changes to allow it to build, I managed to get it
> > > probing, but I never got it to power up. It doesn't seem to be the
> > > same.
> >
> > I need a more technical reason why this is not the correct approach
> > for you.  "I can't get it to work" doesn't quite reach the quality
> > line I'm afraid.
> 
> Yep, so one thing that we need that the simple-mfd-i2c doesn't do is
> the epd-pwr-good-gpios GPIO. This is a GPIO line from the mfd that the
> regulator uses.

Can this be obtained from anywhere else?

Like in the regulator driver for instance.

> > Did you try enabling the debug prints in of_platform_bus_create() and
> > friends to see if your devices are probing correctly?
> 
> I think I did, but I can have another look and see if I can get it working.
> 
> Alistair
> 
> >

-- 
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog