[PATCH 0/7] sy7636a: power the panel only during updates

[PATCH 0/7] sy7636a: power the panel only during updates

[Pablo Mazzini]

The SY7636A hwmon driver keeps VCOM on from probe, so a display driver
can not power the panel only during updates.

Read the temperature in the regulator driver instead, as the TPS65185
and FP9931 drivers do, and remove the hwmon driver. Boards that relied
on VCOM staying on mark it always-on. The regulator also switches VCOM
with VCOM_EN, gains VCOM voltage control and binds to the I2C device
directly.

Tested on a PocketBook Verse (B288).

Pablo Mazzini (7):
  ARM: dts: imx: keep the SY7636A VCOM on
  regulator: sy7636a: fix the chip name
  regulator: sy7636a: add VCOM voltage control
  regulator: sy7636a: wait for power good, then switch VCOM_EN
  regulator: sy7636a: report the temperature
  regulator: sy7636a: bind to the I2C device directly
  ARM: imx_v6_v7_defconfig: drop the SY7636A MFD and hwmon options

 Documentation/hwmon/index.rst                 |   1 -
 Documentation/hwmon/sy7636a-hwmon.rst         |  26 --
 arch/arm/boot/dts/nxp/imx/e70k02.dtsi         |   1 +
 .../boot/dts/nxp/imx/imx7d-remarkable2.dts    |   1 +
 arch/arm/configs/imx_v6_v7_defconfig          |   2 -
 drivers/hwmon/Kconfig                         |  10 -
 drivers/hwmon/Makefile                        |   1 -
 drivers/hwmon/sy7636a-hwmon.c                 | 102 --------
 drivers/mfd/Kconfig                           |  10 -
 drivers/mfd/simple-mfd-i2c.c                  |  11 -
 drivers/regulator/Kconfig                     |   6 +-
 drivers/regulator/sy7636a-regulator.c         | 240 +++++++++++++++---
 include/linux/mfd/sy7636a.h                   |  34 ---
 13 files changed, 205 insertions(+), 240 deletions(-)
 delete mode 100644 Documentation/hwmon/sy7636a-hwmon.rst
 delete mode 100644 drivers/hwmon/sy7636a-hwmon.c
 delete mode 100644 include/linux/mfd/sy7636a.h


base-commit: aac26bee2287c88af5be5a5ff96d783b19a28790
-- 
2.52.0

[PATCH 1/7] ARM: dts: imx: keep the SY7636A VCOM on

[Pablo Mazzini]

The SY7636A hwmon driver turns VCOM on at probe. The reMarkable 2, Kobo
Libra H2O and Tolino Vision 5 have had the rails on so far. Mark VCOM
always-on before the driver stops doing it.

Signed-off-by: Pablo Mazzini <pmazzini@gmail.com>
---
 arch/arm/boot/dts/nxp/imx/e70k02.dtsi           | 1 +
 arch/arm/boot/dts/nxp/imx/imx7d-remarkable2.dts | 1 +
 2 files changed, 2 insertions(+)

diff --git a/arch/arm/boot/dts/nxp/imx/e70k02.dtsi b/arch/arm/boot/dts/nxp/imx/e70k02.dtsi
index dc7ff108aa85..bd9c6c6fa117 100644
--- a/arch/arm/boot/dts/nxp/imx/e70k02.dtsi
+++ b/arch/arm/boot/dts/nxp/imx/e70k02.dtsi
@@ -154,6 +154,7 @@ sy7636: pmic@62 {
 		regulators {
 			reg_epdpmic: vcom {
 				regulator-name = "vcom";
+				regulator-always-on;
 			};
 		};
 	};
diff --git a/arch/arm/boot/dts/nxp/imx/imx7d-remarkable2.dts b/arch/arm/boot/dts/nxp/imx/imx7d-remarkable2.dts
index ff9d50942884..6cf5603c0752 100644
--- a/arch/arm/boot/dts/nxp/imx/imx7d-remarkable2.dts
+++ b/arch/arm/boot/dts/nxp/imx/imx7d-remarkable2.dts
@@ -327,6 +327,7 @@ sy7636a: pmic@62 {
 		regulators {
 			reg_epdpmic: vcom {
 				regulator-name = "vcom";
+				regulator-always-on;
 				regulator-boot-on;
 			};
 		};
-- 
2.52.0

[PATCH 2/7] regulator: sy7636a: fix the chip name

[Pablo Mazzini]

The driver header and the Kconfig help call the chip SY3686A.

Signed-off-by: Pablo Mazzini <pmazzini@gmail.com>
---
 drivers/regulator/Kconfig             | 2 +-
 drivers/regulator/sy7636a-regulator.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
index be6b72daa76f..88ba2770e378 100644
--- a/drivers/regulator/Kconfig
+++ b/drivers/regulator/Kconfig
@@ -1623,7 +1623,7 @@ config REGULATOR_SY7636A
 	tristate "Silergy SY7636A voltage regulator"
 	depends on MFD_SY7636A
 	help
-	  This driver supports Silergy SY3686A voltage regulator.
+	  This driver supports Silergy SY7636A voltage regulator.
 
 config REGULATOR_SY8106A
 	tristate "Silergy SY8106A regulator"
diff --git a/drivers/regulator/sy7636a-regulator.c b/drivers/regulator/sy7636a-regulator.c
index c44c445ea139..015227cb113e 100644
--- a/drivers/regulator/sy7636a-regulator.c
+++ b/drivers/regulator/sy7636a-regulator.c
@@ -1,6 +1,6 @@
 // SPDX-License-Identifier: GPL-2.0+
 //
-// Functions to access SY3686A power management chip voltages
+// Functions to access SY7636A power management chip voltages
 //
 // Copyright (C) 2019 reMarkable AS - http://www.remarkable.com/
 //
-- 
2.52.0

[PATCH 3/7] regulator: sy7636a: add VCOM voltage control

[Pablo Mazzini]

VCOM is panel specific, so let boards set it in the devicetree. Switch
to the selector ops and add the write side.

Per the datasheet, VCOM bit 8 is bit 7 of register 0x02, not bit 0.

Signed-off-by: Pablo Mazzini <pmazzini@gmail.com>
---
 drivers/regulator/sy7636a-regulator.c | 31 +++++++++++++++++++++++----
 1 file changed, 27 insertions(+), 4 deletions(-)

diff --git a/drivers/regulator/sy7636a-regulator.c b/drivers/regulator/sy7636a-regulator.c
index 015227cb113e..382bab017784 100644
--- a/drivers/regulator/sy7636a-regulator.c
+++ b/drivers/regulator/sy7636a-regulator.c
@@ -24,7 +24,7 @@ struct sy7636a_data {
 	struct gpio_desc *vcom_en_gpio;
 };
 
-static int sy7636a_get_vcom_voltage_op(struct regulator_dev *rdev)
+static int sy7636a_get_vcom_voltage_sel(struct regulator_dev *rdev)
 {
 	int ret;
 	unsigned int val, val_h;
@@ -37,9 +37,20 @@ static int sy7636a_get_vcom_voltage_op(struct regulator_dev *rdev)
 	if (ret)
 		return ret;
 
-	val |= (val_h << VCOM_ADJUST_CTRL_SHIFT);
+	/* VCOM[8] is bit 7 of register 0x02 */
+	return val | (val_h & BIT(7)) << 1;
+}
 
-	return (val & VCOM_ADJUST_CTRL_MASK) * VCOM_ADJUST_CTRL_SCAL;
+static int sy7636a_set_vcom_voltage_sel(struct regulator_dev *rdev, unsigned int sel)
+{
+	int ret;
+
+	ret = regmap_write(rdev->regmap, SY7636A_REG_VCOM_ADJUST_CTRL_L, sel & 0xff);
+	if (ret)
+		return ret;
+
+	return regmap_update_bits(rdev->regmap, SY7636A_REG_VCOM_ADJUST_CTRL_H, BIT(7),
+				  (sel & BIT(8)) >> 1);
 }
 
 static int sy7636a_get_status(struct regulator_dev *rdev)
@@ -55,19 +66,31 @@ static int sy7636a_get_status(struct regulator_dev *rdev)
 }
 
 static const struct regulator_ops sy7636a_vcom_volt_ops = {
-	.get_voltage = sy7636a_get_vcom_voltage_op,
+	.list_voltage = regulator_list_voltage_linear_range,
+	.map_voltage = regulator_map_voltage_linear_range,
+	.get_voltage_sel = sy7636a_get_vcom_voltage_sel,
+	.set_voltage_sel = sy7636a_set_vcom_voltage_sel,
 	.enable = regulator_enable_regmap,
 	.disable = regulator_disable_regmap,
 	.is_enabled = regulator_is_enabled_regmap,
 	.get_status = sy7636a_get_status,
 };
 
+/* 10 mV steps up to 5 V, higher codes also give 5 V */
+static const struct linear_range sy7636a_vcom_ranges[] = {
+	REGULATOR_LINEAR_RANGE(0, 0x000, 0x1f4, VCOM_ADJUST_CTRL_SCAL),
+	REGULATOR_LINEAR_RANGE(5000000, 0x1f5, 0x1ff, 0),
+};
+
 static const struct regulator_desc desc = {
 	.name = "vcom",
 	.id = 0,
 	.ops = &sy7636a_vcom_volt_ops,
 	.type = REGULATOR_VOLTAGE,
 	.owner = THIS_MODULE,
+	.n_voltages = VCOM_ADJUST_CTRL_MASK + 1,
+	.linear_ranges = sy7636a_vcom_ranges,
+	.n_linear_ranges = ARRAY_SIZE(sy7636a_vcom_ranges),
 	.enable_reg = SY7636A_REG_OPERATION_MODE_CRL,
 	.enable_mask = SY7636A_OPERATION_MODE_CRL_ONOFF,
 	.regulators_node = of_match_ptr("regulators"),
-- 
2.52.0

[PATCH 4/7] regulator: sy7636a: wait for power good, then switch VCOM_EN

[Pablo Mazzini]

Wait for power good on enable, as the TPS65185 and FP9931 drivers do.

The VCOM_EN line was requested but held low. When it is wired, let it
switch VCOM after power good.

Signed-off-by: Pablo Mazzini <pmazzini@gmail.com>
---
 drivers/regulator/sy7636a-regulator.c | 77 +++++++++++++++++++++++++--
 1 file changed, 73 insertions(+), 4 deletions(-)

diff --git a/drivers/regulator/sy7636a-regulator.c b/drivers/regulator/sy7636a-regulator.c
index 382bab017784..5705599d1b55 100644
--- a/drivers/regulator/sy7636a-regulator.c
+++ b/drivers/regulator/sy7636a-regulator.c
@@ -7,8 +7,10 @@
 // Authors: Lars Ivar Miljeteig <lars.ivar.miljeteig@remarkable.com>
 //          Alistair Francis <alistair@alistair23.me>
 
+#include <linux/completion.h>
 #include <linux/gpio/consumer.h>
 #include <linux/i2c.h>
+#include <linux/interrupt.h>
 #include <linux/mfd/sy7636a.h>
 #include <linux/module.h>
 #include <linux/platform_device.h>
@@ -17,11 +19,14 @@
 #include <linux/regulator/machine.h>
 #include <linux/regmap.h>
 
+#define PGOOD_TIMEOUT_MSECS 200
+
 struct sy7636a_data {
 	struct regmap *regmap;
 	struct gpio_desc *pgood_gpio;
 	struct gpio_desc *en_gpio;
 	struct gpio_desc *vcom_en_gpio;
+	struct completion pgood_completion;
 };
 
 static int sy7636a_get_vcom_voltage_sel(struct regulator_dev *rdev)
@@ -65,13 +70,58 @@ static int sy7636a_get_status(struct regulator_dev *rdev)
 	return ret;
 }
 
+static irqreturn_t sy7636a_pgood_handler(int irq, void *dev_id)
+{
+	struct sy7636a_data *data = dev_id;
+
+	complete(&data->pgood_completion);
+
+	return IRQ_HANDLED;
+}
+
+/* wait for the rails to be good, then switch VCOM on with VCOM_EN if wired */
+static int sy7636a_enable(struct regulator_dev *rdev)
+{
+	struct sy7636a_data *data = dev_get_drvdata(rdev->dev.parent);
+	int ret;
+
+	reinit_completion(&data->pgood_completion);
+	ret = regulator_enable_regmap(rdev);
+	if (ret)
+		return ret;
+
+	wait_for_completion_timeout(&data->pgood_completion,
+				    msecs_to_jiffies(PGOOD_TIMEOUT_MSECS));
+	ret = gpiod_get_value_cansleep(data->pgood_gpio);
+	if (ret <= 0) {
+		regulator_disable_regmap(rdev);
+		return ret ? ret : -ETIMEDOUT;
+	}
+
+	if (data->vcom_en_gpio) {
+		gpiod_set_value_cansleep(data->vcom_en_gpio, 1);
+		usleep_range(500, 700);
+	}
+
+	return 0;
+}
+
+static int sy7636a_disable(struct regulator_dev *rdev)
+{
+	struct sy7636a_data *data = dev_get_drvdata(rdev->dev.parent);
+
+	gpiod_set_value_cansleep(data->vcom_en_gpio, 0);
+
+	return regulator_disable_regmap(rdev);
+}
+
 static const struct regulator_ops sy7636a_vcom_volt_ops = {
 	.list_voltage = regulator_list_voltage_linear_range,
 	.map_voltage = regulator_map_voltage_linear_range,
 	.get_voltage_sel = sy7636a_get_vcom_voltage_sel,
 	.set_voltage_sel = sy7636a_set_vcom_voltage_sel,
-	.enable = regulator_enable_regmap,
-	.disable = regulator_disable_regmap,
+	.enable = sy7636a_enable,
+	.disable = sy7636a_disable,
 	.is_enabled = regulator_is_enabled_regmap,
 	.get_status = sy7636a_get_status,
 };
@@ -104,7 +154,7 @@ static int sy7636a_regulator_probe(struct platform_device *pdev)
 	struct regulator_dev *rdev;
 	struct gpio_desc *gdp;
 	struct sy7636a_data *data;
-	int ret;
+	int irq, ret;
 
 	if (!regmap)
 		return -EPROBE_DEFER;
@@ -136,7 +186,6 @@ static int sy7636a_regulator_probe(struct platform_device *pdev)
 				     PTR_ERR(data->en_gpio),
 				     "failed to get en gpio\n");
 
-	/* Let VCOM just follow the default power on sequence */
 	data->vcom_en_gpio = devm_gpiod_get_optional(&pdev->dev,
 						     "vcom-en", GPIOD_OUT_LOW);
 	if (IS_ERR(data->vcom_en_gpio))
@@ -156,6 +205,26 @@ static int sy7636a_regulator_probe(struct platform_device *pdev)
 		return ret;
 	}
 
+	irq = gpiod_to_irq(data->pgood_gpio);
+	if (irq < 0)
+		return dev_err_probe(&pdev->dev, irq, "no power good interrupt\n");
+
+	init_completion(&data->pgood_completion);
+	ret = devm_request_threaded_irq(&pdev->dev, irq, NULL, sy7636a_pgood_handler,
+					IRQF_TRIGGER_RISING | IRQF_ONESHOT,
+					"sy7636a-pgood", data);
+	if (ret)
+		return dev_err_probe(&pdev->dev, ret,
+				     "failed to request power good interrupt\n");
+
+	/* VCOM switched by the VCOM_EN line; otherwise it follows the power on sequence */
+	if (data->vcom_en_gpio) {
+		ret = regmap_set_bits(regmap, SY7636A_REG_OPERATION_MODE_CRL,
+				      SY7636A_OPERATION_MODE_CRL_VCOMCTL);
+		if (ret)
+			return ret;
+	}
+
 	config.dev = &pdev->dev;
 	config.regmap = regmap;
 
-- 
2.52.0

[PATCH 5/7] regulator: sy7636a: report the temperature

[Pablo Mazzini]

The separate hwmon cell enables VCOM at probe. Report the temperature
from here, as the TPS65185 and FP9931 drivers do, and under the chip
name as they do: read it while the rails are on and return -ENODATA
otherwise. Boards that need the rails on mark VCOM always-on. Drop the
hwmon driver.

Signed-off-by: Pablo Mazzini <pmazzini@gmail.com>
---
 Documentation/hwmon/index.rst         |   1 -
 Documentation/hwmon/sy7636a-hwmon.rst |  26 -------
 drivers/hwmon/Kconfig                 |  10 ---
 drivers/hwmon/Makefile                |   1 -
 drivers/hwmon/sy7636a-hwmon.c         | 102 --------------------------
 drivers/mfd/simple-mfd-i2c.c          |   1 -
 drivers/regulator/Kconfig             |   1 +
 drivers/regulator/sy7636a-regulator.c |  56 ++++++++++++++
 8 files changed, 57 insertions(+), 141 deletions(-)
 delete mode 100644 Documentation/hwmon/sy7636a-hwmon.rst
 delete mode 100644 drivers/hwmon/sy7636a-hwmon.c

diff --git a/Documentation/hwmon/index.rst b/Documentation/hwmon/index.rst
index 199ca79621e9..43aecc4e49cd 100644
--- a/Documentation/hwmon/index.rst
+++ b/Documentation/hwmon/index.rst
@@ -265,7 +265,6 @@ Hardware Monitoring Kernel Drivers
    sq24860
    stpddc60
    surface_fan
-   sy7636a-hwmon
    tc654
    tc74
    tda38740
diff --git a/Documentation/hwmon/sy7636a-hwmon.rst b/Documentation/hwmon/sy7636a-hwmon.rst
deleted file mode 100644
index 03d866aba6e8..000000000000
--- a/Documentation/hwmon/sy7636a-hwmon.rst
+++ /dev/null
@@ -1,26 +0,0 @@
-.. SPDX-License-Identifier: GPL-2.0-or-later
-
-Kernel driver sy7636a-hwmon
-===========================
-
-Supported chips:
-
- * Silergy SY7636A PMIC
-
-
-Description
------------
-
-This driver adds hardware temperature reading support for
-the Silergy SY7636A PMIC.
-
-The following sensors are supported
-
-  * Temperature
-      - Temperature of external NTC in milli-degree C
-
-sysfs-Interface
----------------
-
-temp1_input
-	- Temperature of external NTC (milli-degree C)
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index 18c682e1a475..36e08c333e01 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -2104,16 +2104,6 @@ config SENSORS_SIS5595
 	  This driver can also be built as a module. If so, the module
 	  will be called sis5595.
 
-config SENSORS_SY7636A
-	tristate "Silergy SY7636A"
-	depends on MFD_SY7636A
-	help
-	  If you say yes here you get support for the thermistor readout of
-	  the Silergy SY7636A PMIC.
-
-	  This driver can also be built as a module.  If so, the module
-	  will be called sy7636a-hwmon.
-
 config SENSORS_DME1737
 	tristate "SMSC DME1737, SCH311x and compatibles"
 	depends on HAS_IOPORT
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index d4c345ed7261..537fb0bc1778 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -234,7 +234,6 @@ obj-$(CONFIG_SENSORS_SPD5118)	+= spd5118.o
 obj-$(CONFIG_SENSORS_STTS751)	+= stts751.o
 obj-$(CONFIG_SENSORS_SURFACE_FAN)+= surface_fan.o
 obj-$(CONFIG_SENSORS_SURFACE_TEMP)+= surface_temp.o
-obj-$(CONFIG_SENSORS_SY7636A)	+= sy7636a-hwmon.o
 obj-$(CONFIG_SENSORS_AMC6821)	+= amc6821.o
 obj-$(CONFIG_SENSORS_TC74)	+= tc74.o
 obj-$(CONFIG_SENSORS_THMC50)	+= thmc50.o
diff --git a/drivers/hwmon/sy7636a-hwmon.c b/drivers/hwmon/sy7636a-hwmon.c
deleted file mode 100644
index d51daaf63d63..000000000000
--- a/drivers/hwmon/sy7636a-hwmon.c
+++ /dev/null
@@ -1,102 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0+
-/*
- * Functions to access SY3686A power management chip temperature
- *
- * Copyright (C) 2021 reMarkable AS - http://www.remarkable.com/
- *
- * Authors: Lars Ivar Miljeteig <lars.ivar.miljeteig@remarkable.com>
- *          Alistair Francis <alistair@alistair23.me>
- */
-
-#include <linux/err.h>
-#include <linux/hwmon.h>
-#include <linux/init.h>
-#include <linux/module.h>
-#include <linux/platform_device.h>
-#include <linux/regmap.h>
-#include <linux/regulator/machine.h>
-
-#include <linux/mfd/sy7636a.h>
-
-static int sy7636a_read(struct device *dev, enum hwmon_sensor_types type,
-			u32 attr, int channel, long *temp)
-{
-	struct regmap *regmap = dev_get_drvdata(dev);
-	int ret, reg_val;
-
-	ret = regmap_read(regmap,
-			  SY7636A_REG_TERMISTOR_READOUT, &reg_val);
-	if (ret)
-		return ret;
-
-	*temp = reg_val * 1000;
-
-	return 0;
-}
-
-static umode_t sy7636a_is_visible(const void *data,
-				  enum hwmon_sensor_types type,
-				  u32 attr, int channel)
-{
-	if (type != hwmon_temp)
-		return 0;
-
-	if (attr != hwmon_temp_input)
-		return 0;
-
-	return 0444;
-}
-
-static const struct hwmon_ops sy7636a_hwmon_ops = {
-	.is_visible = sy7636a_is_visible,
-	.read = sy7636a_read,
-};
-
-static const struct hwmon_channel_info * const sy7636a_info[] = {
-	HWMON_CHANNEL_INFO(chip, HWMON_C_REGISTER_TZ),
-	HWMON_CHANNEL_INFO(temp, HWMON_T_INPUT),
-	NULL
-};
-
-static const struct hwmon_chip_info sy7636a_chip_info = {
-	.ops = &sy7636a_hwmon_ops,
-	.info = sy7636a_info,
-};
-
-static int sy7636a_sensor_probe(struct platform_device *pdev)
-{
-	struct regmap *regmap = dev_get_regmap(pdev->dev.parent, NULL);
-	struct device *hwmon_dev;
-	int err;
-
-	if (!regmap)
-		return -EPROBE_DEFER;
-
-	err = devm_regulator_get_enable(&pdev->dev, "vcom");
-	if (err)
-		return err;
-
-	hwmon_dev = devm_hwmon_device_register_with_info(&pdev->dev,
-							 "sy7636a_temperature", regmap,
-							 &sy7636a_chip_info, NULL);
-
-	if (IS_ERR(hwmon_dev)) {
-		err = PTR_ERR(hwmon_dev);
-		dev_err(&pdev->dev, "Unable to register hwmon device, returned %d\n", err);
-		return err;
-	}
-
-	return 0;
-}
-
-static struct platform_driver sy7636a_sensor_driver = {
-	.probe = sy7636a_sensor_probe,
-	.driver = {
-		.name = "sy7636a-temperature",
-	},
-};
-module_platform_driver(sy7636a_sensor_driver);
-
-MODULE_DESCRIPTION("SY7636A sensor driver");
-MODULE_LICENSE("GPL");
-MODULE_ALIAS("platform:sy7636a-temperature");
diff --git a/drivers/mfd/simple-mfd-i2c.c b/drivers/mfd/simple-mfd-i2c.c
index ef3ce4bdf98b..168b4cf49b8c 100644
--- a/drivers/mfd/simple-mfd-i2c.c
+++ b/drivers/mfd/simple-mfd-i2c.c
@@ -69,7 +69,6 @@ static int simple_mfd_i2c_probe(struct i2c_client *i2c)
 
 static const struct mfd_cell sy7636a_cells[] = {
 	{ .name = "sy7636a-regulator", },
-	{ .name = "sy7636a-temperature", },
 };
 
 static const struct simple_mfd_data silergy_sy7636a = {
diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
index 88ba2770e378..f0d8d1319a1b 100644
--- a/drivers/regulator/Kconfig
+++ b/drivers/regulator/Kconfig
@@ -1624,6 +1624,7 @@ config REGULATOR_SY7636A
 	depends on MFD_SY7636A
 	help
 	  This driver supports Silergy SY7636A voltage regulator.
+	  If HWMON is enabled, it also provides temperature measurement.
 
 config REGULATOR_SY8106A
 	tristate "Silergy SY8106A regulator"
diff --git a/drivers/regulator/sy7636a-regulator.c b/drivers/regulator/sy7636a-regulator.c
index 5705599d1b55..816df7716c0e 100644
--- a/drivers/regulator/sy7636a-regulator.c
+++ b/drivers/regulator/sy7636a-regulator.c
@@ -9,6 +9,7 @@
 
 #include <linux/completion.h>
 #include <linux/gpio/consumer.h>
+#include <linux/hwmon.h>
 #include <linux/i2c.h>
 #include <linux/interrupt.h>
 #include <linux/mfd/sy7636a.h>
@@ -147,6 +148,52 @@ static const struct regulator_desc desc = {
 	.of_match = of_match_ptr("vcom"),
 };
 
+static int sy7636a_hwmon_read(struct device *dev, enum hwmon_sensor_types type,
+			      u32 attr, int channel, long *temp)
+{
+	struct sy7636a_data *data = dev_get_drvdata(dev);
+	unsigned int mode, val;
+	int ret;
+
+	ret = regmap_read(data->regmap, SY7636A_REG_OPERATION_MODE_CRL, &mode);
+	if (ret)
+		return ret;
+
+	/* the thermistor is only sampled while the rails are on */
+	if (!(mode & SY7636A_OPERATION_MODE_CRL_ONOFF))
+		return -ENODATA;
+
+	ret = regmap_read(data->regmap, SY7636A_REG_TERMISTOR_READOUT, &val);
+	if (ret)
+		return ret;
+
+	*temp = (s8)val * 1000;
+
+	return 0;
+}
+
+static umode_t sy7636a_hwmon_is_visible(const void *data, enum hwmon_sensor_types type,
+					u32 attr, int channel)
+{
+	return 0444;
+}
+
+static const struct hwmon_channel_info * const sy7636a_hwmon_info[] = {
+	HWMON_CHANNEL_INFO(chip, HWMON_C_REGISTER_TZ),
+	HWMON_CHANNEL_INFO(temp, HWMON_T_INPUT),
+	NULL
+};
+
+static const struct hwmon_ops sy7636a_hwmon_ops = {
+	.is_visible = sy7636a_hwmon_is_visible,
+	.read = sy7636a_hwmon_read,
+};
+
+static const struct hwmon_chip_info sy7636a_hwmon_chip_info = {
+	.ops = &sy7636a_hwmon_ops,
+	.info = sy7636a_hwmon_info,
+};
+
 static int sy7636a_regulator_probe(struct platform_device *pdev)
 {
 	struct regmap *regmap = dev_get_regmap(pdev->dev.parent, NULL);
@@ -235,6 +282,15 @@ static int sy7636a_regulator_probe(struct platform_device *pdev)
 		return PTR_ERR(rdev);
 	}
 
+	if (IS_REACHABLE(CONFIG_HWMON)) {
+		struct device *hwmon_dev;
+
+		hwmon_dev = devm_hwmon_device_register_with_info(&pdev->dev, "sy7636a", data,
+								 &sy7636a_hwmon_chip_info, NULL);
+		if (IS_ERR(hwmon_dev))
+			dev_notice(&pdev->dev, "failed to register hwmon\n");
+	}
+
 	return 0;
 }
 
-- 
2.52.0

[PATCH 6/7] regulator: sy7636a: bind to the I2C device directly

[Pablo Mazzini]

With the temperature reported here, the regulator is all that is left
behind simple-mfd-i2c. Bind to the chip directly, as the TPS65185 and
FP9931 drivers do, and drop the MFD glue and header. It now probes before
the GPIOs may be ready, so report the power good GPIO with
dev_err_probe().

Signed-off-by: Pablo Mazzini <pmazzini@gmail.com>
---
 drivers/mfd/Kconfig                   | 10 ----
 drivers/mfd/simple-mfd-i2c.c          | 10 ----
 drivers/regulator/Kconfig             |  3 +-
 drivers/regulator/sy7636a-regulator.c | 86 +++++++++++++++------------
 include/linux/mfd/sy7636a.h           | 34 -----------
 5 files changed, 50 insertions(+), 93 deletions(-)
 delete mode 100644 include/linux/mfd/sy7636a.h

diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 0a2778052519..9130dac7a7ff 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -1305,16 +1305,6 @@ config MFD_SPMI_PMIC
 	  Say M here if you want to include support for the SPMI PMIC
 	  series as a module.  The module will be called "qcom-spmi-pmic".
 
-config MFD_SY7636A
-	tristate "Silergy SY7636A voltage regulator"
-	depends on I2C
-	select MFD_SIMPLE_MFD_I2C
-	help
-	  Enable support for Silergy SY7636A voltage regulator.
-
-	  To enable support for building sub-devices as modules,
-	  choose M here.
-
 config MFD_RDC321X
 	tristate "RDC R-321x southbridge"
 	select MFD_CORE
diff --git a/drivers/mfd/simple-mfd-i2c.c b/drivers/mfd/simple-mfd-i2c.c
index 168b4cf49b8c..0c9162034899 100644
--- a/drivers/mfd/simple-mfd-i2c.c
+++ b/drivers/mfd/simple-mfd-i2c.c
@@ -67,15 +67,6 @@ static int simple_mfd_i2c_probe(struct i2c_client *i2c)
 	return ret;
 }
 
-static const struct mfd_cell sy7636a_cells[] = {
-	{ .name = "sy7636a-regulator", },
-};
-
-static const struct simple_mfd_data silergy_sy7636a = {
-	.mfd_cell = sy7636a_cells,
-	.mfd_cell_size = ARRAY_SIZE(sy7636a_cells),
-};
-
 static const struct mfd_cell max5970_cells[] = {
 	{ .name = "max5970-regulator", },
 	{ .name = "max5970-iio", },
@@ -123,7 +114,6 @@ static const struct of_device_id simple_mfd_i2c_of_match[] = {
 	{ .compatible = "maxim,max5970", .data = &maxim_max5970 },
 	{ .compatible = "maxim,max5978", .data = &maxim_max5970 },
 	{ .compatible = "maxim,max77705-battery", .data = &maxim_mon_max77705 },
-	{ .compatible = "silergy,sy7636a", .data = &silergy_sy7636a },
 	{ .compatible = "spacemit,p1", .data = &spacemit_p1 },
 	{}
 };
diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
index f0d8d1319a1b..49b7c6dc541f 100644
--- a/drivers/regulator/Kconfig
+++ b/drivers/regulator/Kconfig
@@ -1621,7 +1621,8 @@ config REGULATOR_SUN20I
 
 config REGULATOR_SY7636A
 	tristate "Silergy SY7636A voltage regulator"
-	depends on MFD_SY7636A
+	depends on I2C
+	select REGMAP_I2C
 	help
 	  This driver supports Silergy SY7636A voltage regulator.
 	  If HWMON is enabled, it also provides temperature measurement.
diff --git a/drivers/regulator/sy7636a-regulator.c b/drivers/regulator/sy7636a-regulator.c
index 816df7716c0e..e170461a15ad 100644
--- a/drivers/regulator/sy7636a-regulator.c
+++ b/drivers/regulator/sy7636a-regulator.c
@@ -12,14 +12,23 @@
 #include <linux/hwmon.h>
 #include <linux/i2c.h>
 #include <linux/interrupt.h>
-#include <linux/mfd/sy7636a.h>
 #include <linux/module.h>
-#include <linux/platform_device.h>
 #include <linux/regulator/consumer.h>
 #include <linux/regulator/driver.h>
 #include <linux/regulator/machine.h>
 #include <linux/regmap.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_POWER_ON_DELAY_TIME		0x06
+#define SY7636A_REG_TERMISTOR_READOUT		0x08
+
+#define VCOM_ADJUST_CTRL_MASK	0x1ff
+#define VCOM_ADJUST_CTRL_SCAL	10000
+
 #define PGOOD_TIMEOUT_MSECS 200
 
 struct sy7636a_data {
@@ -194,49 +203,52 @@ static const struct hwmon_chip_info sy7636a_hwmon_chip_info = {
 	.info = sy7636a_hwmon_info,
 };
 
-static int sy7636a_regulator_probe(struct platform_device *pdev)
+static const struct regmap_config sy7636a_regmap_config = {
+	.reg_bits = 8,
+	.val_bits = 8,
+};
+
+static int sy7636a_regulator_probe(struct i2c_client *client)
 {
-	struct regmap *regmap = dev_get_regmap(pdev->dev.parent, NULL);
+	struct device *dev = &client->dev;
 	struct regulator_config config = { };
 	struct regulator_dev *rdev;
 	struct gpio_desc *gdp;
 	struct sy7636a_data *data;
+	struct regmap *regmap;
 	int irq, ret;
 
-	if (!regmap)
-		return -EPROBE_DEFER;
-
-	device_set_of_node_from_dev(&pdev->dev, pdev->dev.parent);
+	regmap = devm_regmap_init_i2c(client, &sy7636a_regmap_config);
+	if (IS_ERR(regmap))
+		return dev_err_probe(dev, PTR_ERR(regmap), "failed to allocate regmap\n");
 
-	gdp = devm_gpiod_get(&pdev->dev, "epd-pwr-good", GPIOD_IN);
-	if (IS_ERR(gdp)) {
-		dev_err(&pdev->dev, "Power good GPIO fault %ld\n", PTR_ERR(gdp));
-		return PTR_ERR(gdp);
-	}
+	gdp = devm_gpiod_get(dev, "epd-pwr-good", GPIOD_IN);
+	if (IS_ERR(gdp))
+		return dev_err_probe(dev, PTR_ERR(gdp), "Power good GPIO fault\n");
 
-	data = devm_kzalloc(&pdev->dev, sizeof(struct sy7636a_data), GFP_KERNEL);
+	data = devm_kzalloc(dev, sizeof(struct sy7636a_data), GFP_KERNEL);
 	if (!data)
 		return -ENOMEM;
 
 	data->regmap = regmap;
 	data->pgood_gpio = gdp;
 
-	ret = devm_regulator_get_enable_optional(&pdev->dev, "vin");
+	ret = devm_regulator_get_enable_optional(dev, "vin");
 	if (ret)
-		return dev_err_probe(&pdev->dev, ret,
+		return dev_err_probe(dev, ret,
 				     "failed to get vin regulator\n");
 
-	data->en_gpio = devm_gpiod_get_optional(&pdev->dev, "enable",
+	data->en_gpio = devm_gpiod_get_optional(dev, "enable",
 						GPIOD_OUT_HIGH);
 	if (IS_ERR(data->en_gpio))
-		return dev_err_probe(&pdev->dev,
+		return dev_err_probe(dev,
 				     PTR_ERR(data->en_gpio),
 				     "failed to get en gpio\n");
 
-	data->vcom_en_gpio = devm_gpiod_get_optional(&pdev->dev,
+	data->vcom_en_gpio = devm_gpiod_get_optional(dev,
 						     "vcom-en", GPIOD_OUT_LOW);
 	if (IS_ERR(data->vcom_en_gpio))
-		return dev_err_probe(&pdev->dev,
+		return dev_err_probe(dev,
 				     PTR_ERR(data->vcom_en_gpio),
 				     "failed to get vcom-en gpio\n");
 
@@ -244,25 +256,24 @@ static int sy7636a_regulator_probe(struct platform_device *pdev)
 	if (data->en_gpio)
 		usleep_range(2500, 4000);
 
-	platform_set_drvdata(pdev, data);
+	i2c_set_clientdata(client, data);
 
 	ret = regmap_write(regmap, SY7636A_REG_POWER_ON_DELAY_TIME, 0x0);
 	if (ret) {
-		dev_err(pdev->dev.parent, "Failed to initialize regulator: %d\n", ret);
+		dev_err(dev, "Failed to initialize regulator: %d\n", ret);
 		return ret;
 	}
 
 	irq = gpiod_to_irq(data->pgood_gpio);
 	if (irq < 0)
-		return dev_err_probe(&pdev->dev, irq, "no power good interrupt\n");
+		return dev_err_probe(dev, irq, "no power good interrupt\n");
 
 	init_completion(&data->pgood_completion);
-	ret = devm_request_threaded_irq(&pdev->dev, irq, NULL, sy7636a_pgood_handler,
+	ret = devm_request_threaded_irq(dev, irq, NULL, sy7636a_pgood_handler,
 					IRQF_TRIGGER_RISING | IRQF_ONESHOT,
 					"sy7636a-pgood", data);
 	if (ret)
-		return dev_err_probe(&pdev->dev, ret,
-				     "failed to request power good interrupt\n");
+		return dev_err_probe(dev, ret, "failed to request power good interrupt\n");
 
 	/* VCOM switched by the VCOM_EN line; otherwise it follows the power on sequence */
 	if (data->vcom_en_gpio) {
@@ -272,43 +283,42 @@ static int sy7636a_regulator_probe(struct platform_device *pdev)
 			return ret;
 	}
 
-	config.dev = &pdev->dev;
+	config.dev = dev;
 	config.regmap = regmap;
 
-	rdev = devm_regulator_register(&pdev->dev, &desc, &config);
+	rdev = devm_regulator_register(dev, &desc, &config);
 	if (IS_ERR(rdev)) {
-		dev_err(pdev->dev.parent, "Failed to register %s regulator\n",
-			pdev->name);
+		dev_err(dev, "Failed to register %s regulator\n", desc.name);
 		return PTR_ERR(rdev);
 	}
 
 	if (IS_REACHABLE(CONFIG_HWMON)) {
 		struct device *hwmon_dev;
 
-		hwmon_dev = devm_hwmon_device_register_with_info(&pdev->dev, "sy7636a", data,
+		hwmon_dev = devm_hwmon_device_register_with_info(dev, "sy7636a", data,
 								 &sy7636a_hwmon_chip_info, NULL);
 		if (IS_ERR(hwmon_dev))
-			dev_notice(&pdev->dev, "failed to register hwmon\n");
+			dev_notice(dev, "failed to register hwmon\n");
 	}
 
 	return 0;
 }
 
-static const struct platform_device_id sy7636a_regulator_id_table[] = {
-	{ .name = "sy7636a-regulator" },
+static const struct of_device_id sy7636a_of_match[] = {
+	{ .compatible = "silergy,sy7636a" },
 	{ }
 };
-MODULE_DEVICE_TABLE(platform, sy7636a_regulator_id_table);
+MODULE_DEVICE_TABLE(of, sy7636a_of_match);
 
-static struct platform_driver sy7636a_regulator_driver = {
+static struct i2c_driver sy7636a_regulator_driver = {
 	.driver = {
 		.name = "sy7636a-regulator",
 		.probe_type = PROBE_PREFER_ASYNCHRONOUS,
+		.of_match_table = sy7636a_of_match,
 	},
 	.probe = sy7636a_regulator_probe,
-	.id_table = sy7636a_regulator_id_table,
 };
-module_platform_driver(sy7636a_regulator_driver);
+module_i2c_driver(sy7636a_regulator_driver);
 
 MODULE_AUTHOR("Lars Ivar Miljeteig <lars.ivar.miljeteig@remarkable.com>");
 MODULE_DESCRIPTION("SY7636A voltage regulator driver");
diff --git a/include/linux/mfd/sy7636a.h b/include/linux/mfd/sy7636a.h
deleted file mode 100644
index 22f03b2f851e..000000000000
--- a/include/linux/mfd/sy7636a.h
+++ /dev/null
@@ -1,34 +0,0 @@
-/* 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
-
-#define SY7636A_REG_OPERATION_MODE_CRL		0x00
-/* It is set if a gpio is used to control the regulator */
-#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_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	10000
-
-#define FAULT_FLAG_SHIFT	1
-
-#endif /* __LINUX_MFD_SY7636A_H */
-- 
2.52.0

[PATCH 7/7] ARM: imx_v6_v7_defconfig: drop the SY7636A MFD and hwmon options

[Pablo Mazzini]

The SY7636A regulator driver now covers the whole chip.

Signed-off-by: Pablo Mazzini <pmazzini@gmail.com>
---
 arch/arm/configs/imx_v6_v7_defconfig | 2 --
 1 file changed, 2 deletions(-)

diff --git a/arch/arm/configs/imx_v6_v7_defconfig b/arch/arm/configs/imx_v6_v7_defconfig
index 6ac42c06c1e9..bd1f692dd7b2 100644
--- a/arch/arm/configs/imx_v6_v7_defconfig
+++ b/arch/arm/configs/imx_v6_v7_defconfig
@@ -223,7 +223,6 @@ CONFIG_SENSORS_IIO_HWMON=y
 CONFIG_SENSORS_JC42=m
 CONFIG_SENSORS_LM75=m
 CONFIG_SENSORS_PWM_FAN=y
-CONFIG_SENSORS_SY7636A=y
 CONFIG_THERMAL_STATISTICS=y
 CONFIG_CPU_THERMAL=y
 CONFIG_IMX_THERMAL=y
@@ -238,7 +237,6 @@ CONFIG_MFD_DA9062=y
 CONFIG_MFD_DA9063=y
 CONFIG_MFD_MC13XXX_SPI=y
 CONFIG_MFD_MC13XXX_I2C=y
-CONFIG_MFD_SY7636A=y
 CONFIG_MFD_RN5T618=y
 CONFIG_MFD_STMPE=y
 CONFIG_MFD_ROHM_BD71828=y
-- 
2.52.0

Re: [PATCH 1/7] ARM: dts: imx: keep the SY7636A VCOM on

[Andreas Kemnade]

On Mon, 26 Oct 2026 00:39:12 +0000
Pablo Mazzini <pmazzini@gmail.com> wrote:

> The SY7636A hwmon driver turns VCOM on at probe. The reMarkable 2, Kobo
> Libra H2O and Tolino Vision 5 have had the rails on so far. Mark VCOM
> always-on before the driver stops doing it.
> 
> Signed-off-by: Pablo Mazzini <pmazzini@gmail.com>
> ---
>  arch/arm/boot/dts/nxp/imx/e70k02.dtsi           | 1 +
>  arch/arm/boot/dts/nxp/imx/imx7d-remarkable2.dts | 1 +
>  2 files changed, 2 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/nxp/imx/e70k02.dtsi b/arch/arm/boot/dts/nxp/imx/e70k02.dtsi
> index dc7ff108aa85..bd9c6c6fa117 100644
> --- a/arch/arm/boot/dts/nxp/imx/e70k02.dtsi
> +++ b/arch/arm/boot/dts/nxp/imx/e70k02.dtsi
> @@ -154,6 +154,7 @@ sy7636: pmic@62 {
>  		regulators {
>  			reg_epdpmic: vcom {
>  				regulator-name = "vcom";
> +				regulator-always-on;
>  			};
>  		};
>  	};
No, please not. That regulator is in practice used with a not yet upstreamed
driver for the EPDC on that platform used to deal with the current kernels
fp9931 / tps65185 drivers. So it is very welcome to have it not always turned
on.


> diff --git a/arch/arm/boot/dts/nxp/imx/imx7d-remarkable2.dts b/arch/arm/boot/dts/nxp/imx/imx7d-remarkable2.dts
> index ff9d50942884..6cf5603c0752 100644
> --- a/arch/arm/boot/dts/nxp/imx/imx7d-remarkable2.dts
> +++ b/arch/arm/boot/dts/nxp/imx/imx7d-remarkable2.dts
> @@ -327,6 +327,7 @@ sy7636a: pmic@62 {
>  		regulators {
>  			reg_epdpmic: vcom {
>  				regulator-name = "vcom";
> +				regulator-always-on;
>  				regulator-boot-on;
>  			};
>  		};


Here the situation is a bit different. If I remember correctly, there
is some special EPD logic in userspace on top of some ordinary kernel
display driver. But thesse things are missing is devicetree?! But
that might need to learn to deal with regulator enabling/disabling.

Regulator always on is probably not desired here, either.

Regards,
Andreas

Re: [PATCH 2/7] regulator: sy7636a: fix the chip name

[Andreas Kemnade]

On Mon, 26 Oct 2026 00:39:13 +0000
Pablo Mazzini <pmazzini@gmail.com> wrote:

> The driver header and the Kconfig help call the chip SY3686A.
> 
> Signed-off-by: Pablo Mazzini <pmazzini@gmail.com>

Reviewed-by: Andreas Kemnade <andreas@kemnade.info>

> ---
>  drivers/regulator/Kconfig             | 2 +-
>  drivers/regulator/sy7636a-regulator.c | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
> index be6b72daa76f..88ba2770e378 100644
> --- a/drivers/regulator/Kconfig
> +++ b/drivers/regulator/Kconfig
> @@ -1623,7 +1623,7 @@ config REGULATOR_SY7636A
>  	tristate "Silergy SY7636A voltage regulator"
>  	depends on MFD_SY7636A
>  	help
> -	  This driver supports Silergy SY3686A voltage regulator.
> +	  This driver supports Silergy SY7636A voltage regulator.
>  
>  config REGULATOR_SY8106A
>  	tristate "Silergy SY8106A regulator"
> diff --git a/drivers/regulator/sy7636a-regulator.c b/drivers/regulator/sy7636a-regulator.c
> index c44c445ea139..015227cb113e 100644
> --- a/drivers/regulator/sy7636a-regulator.c
> +++ b/drivers/regulator/sy7636a-regulator.c
> @@ -1,6 +1,6 @@
>  // SPDX-License-Identifier: GPL-2.0+
>  //
> -// Functions to access SY3686A power management chip voltages
> +// Functions to access SY7636A power management chip voltages
>  //
>  // Copyright (C) 2019 reMarkable AS - http://www.remarkable.com/
>  //

Re: [PATCH 3/7] regulator: sy7636a: add VCOM voltage control

[Andreas Kemnade]

On Mon, 26 Oct 2026 00:39:14 +0000
Pablo Mazzini <pmazzini@gmail.com> wrote:

> VCOM is panel specific, so let boards set it in the devicetree. Switch
> to the selector ops and add the write side.
> 
> Per the datasheet, VCOM bit 8 is bit 7 of register 0x02, not bit 0.
> 
> Signed-off-by: Pablo Mazzini <pmazzini@gmail.com>

Reviewed-by: Andreas Kemnade <andreas@kemnade.info>
> ---
>  drivers/regulator/sy7636a-regulator.c | 31 +++++++++++++++++++++++----
>  1 file changed, 27 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/regulator/sy7636a-regulator.c b/drivers/regulator/sy7636a-regulator.c
> index 015227cb113e..382bab017784 100644
> --- a/drivers/regulator/sy7636a-regulator.c
> +++ b/drivers/regulator/sy7636a-regulator.c
> @@ -24,7 +24,7 @@ struct sy7636a_data {
>  	struct gpio_desc *vcom_en_gpio;
>  };
>  
> -static int sy7636a_get_vcom_voltage_op(struct regulator_dev *rdev)
> +static int sy7636a_get_vcom_voltage_sel(struct regulator_dev *rdev)
>  {
>  	int ret;
>  	unsigned int val, val_h;
> @@ -37,9 +37,20 @@ static int sy7636a_get_vcom_voltage_op(struct regulator_dev *rdev)
>  	if (ret)
>  		return ret;
>  
> -	val |= (val_h << VCOM_ADJUST_CTRL_SHIFT);
> +	/* VCOM[8] is bit 7 of register 0x02 */
> +	return val | (val_h & BIT(7)) << 1;
> +}
>  
> -	return (val & VCOM_ADJUST_CTRL_MASK) * VCOM_ADJUST_CTRL_SCAL;
> +static int sy7636a_set_vcom_voltage_sel(struct regulator_dev *rdev, unsigned int sel)
> +{
> +	int ret;
> +
> +	ret = regmap_write(rdev->regmap, SY7636A_REG_VCOM_ADJUST_CTRL_L, sel & 0xff);
> +	if (ret)
> +		return ret;
> +
> +	return regmap_update_bits(rdev->regmap, SY7636A_REG_VCOM_ADJUST_CTRL_H, BIT(7),
> +				  (sel & BIT(8)) >> 1);
>  }
>  
>  static int sy7636a_get_status(struct regulator_dev *rdev)
> @@ -55,19 +66,31 @@ static int sy7636a_get_status(struct regulator_dev *rdev)
>  }
>  
>  static const struct regulator_ops sy7636a_vcom_volt_ops = {
> -	.get_voltage = sy7636a_get_vcom_voltage_op,
> +	.list_voltage = regulator_list_voltage_linear_range,
> +	.map_voltage = regulator_map_voltage_linear_range,
> +	.get_voltage_sel = sy7636a_get_vcom_voltage_sel,
> +	.set_voltage_sel = sy7636a_set_vcom_voltage_sel,
>  	.enable = regulator_enable_regmap,
>  	.disable = regulator_disable_regmap,
>  	.is_enabled = regulator_is_enabled_regmap,
>  	.get_status = sy7636a_get_status,
>  };
>  
> +/* 10 mV steps up to 5 V, higher codes also give 5 V */
> +static const struct linear_range sy7636a_vcom_ranges[] = {
> +	REGULATOR_LINEAR_RANGE(0, 0x000, 0x1f4, VCOM_ADJUST_CTRL_SCAL),
> +	REGULATOR_LINEAR_RANGE(5000000, 0x1f5, 0x1ff, 0),
> +};
> +
>  static const struct regulator_desc desc = {
>  	.name = "vcom",
>  	.id = 0,
>  	.ops = &sy7636a_vcom_volt_ops,
>  	.type = REGULATOR_VOLTAGE,
>  	.owner = THIS_MODULE,
> +	.n_voltages = VCOM_ADJUST_CTRL_MASK + 1,
> +	.linear_ranges = sy7636a_vcom_ranges,
> +	.n_linear_ranges = ARRAY_SIZE(sy7636a_vcom_ranges),
>  	.enable_reg = SY7636A_REG_OPERATION_MODE_CRL,
>  	.enable_mask = SY7636A_OPERATION_MODE_CRL_ONOFF,
>  	.regulators_node = of_match_ptr("regulators"),

Re: [PATCH 4/7] regulator: sy7636a: wait for power good, then switch VCOM_EN

[Andreas Kemnade]

On Mon, 26 Oct 2026 00:39:15 +0000
Pablo Mazzini <pmazzini@gmail.com> wrote:

> Wait for power good on enable, as the TPS65185 and FP9931 drivers do.
> 
which is a good idea that the consumers can relay on the regulators being
useable when enabled.

> The VCOM_EN line was requested but held low. When it is wired, let it
> switch VCOM after power good.
> 
What is the reason for this change? VCOM gets enabled this way, too.
I added this line just for having the hardware completely documented.

A possible scenario where I could imagine this line being useful:

Calibration of required VCOM voltage, You would turn on everything else,
do a mode 0 refresh on the EPD and measure some kickback voltage on the
otherwise inactive VCOM line and have your calculated value for that
regulator.

To be useful, several things are needed:
- SY7636a driver needs to model vcom as a separate regulator turning
  on a vposneg regulator. The TPS65185 driver defines a vposneg regulator.
- ADC input is needed. The TPS65185 provides (not implemented in the
  driver yet) such measurement. Of course that can come from another
  chip.
- Some (probably userspace logic) to control all of this.

Regards,
Andreas

Re: [PATCH 1/7] ARM: dts: imx: keep the SY7636A VCOM on

[Pablo Mazzini]

Thanks, I will drop this patch in v2, so VCOM stays off until a consumer
enables it.

On Mon, Oct 26, 2026 at 7:31 AM Andreas Kemnade <andreas@kemnade.info> wrote:
>
> On Mon, 26 Oct 2026 00:39:12 +0000
> Pablo Mazzini <pmazzini@gmail.com> wrote:
>
> > The SY7636A hwmon driver turns VCOM on at probe. The reMarkable 2, Kobo
> > Libra H2O and Tolino Vision 5 have had the rails on so far. Mark VCOM
> > always-on before the driver stops doing it.
> >
> > Signed-off-by: Pablo Mazzini <pmazzini@gmail.com>
> > ---
> >  arch/arm/boot/dts/nxp/imx/e70k02.dtsi           | 1 +
> >  arch/arm/boot/dts/nxp/imx/imx7d-remarkable2.dts | 1 +
> >  2 files changed, 2 insertions(+)
> >
> > diff --git a/arch/arm/boot/dts/nxp/imx/e70k02.dtsi b/arch/arm/boot/dts/nxp/imx/e70k02.dtsi
> > index dc7ff108aa85..bd9c6c6fa117 100644
> > --- a/arch/arm/boot/dts/nxp/imx/e70k02.dtsi
> > +++ b/arch/arm/boot/dts/nxp/imx/e70k02.dtsi
> > @@ -154,6 +154,7 @@ sy7636: pmic@62 {
> >               regulators {
> >                       reg_epdpmic: vcom {
> >                               regulator-name = "vcom";
> > +                             regulator-always-on;
> >                       };
> >               };
> >       };
> No, please not. That regulator is in practice used with a not yet upstreamed
> driver for the EPDC on that platform used to deal with the current kernels
> fp9931 / tps65185 drivers. So it is very welcome to have it not always turned
> on.
>
>
> > diff --git a/arch/arm/boot/dts/nxp/imx/imx7d-remarkable2.dts b/arch/arm/boot/dts/nxp/imx/imx7d-remarkable2.dts
> > index ff9d50942884..6cf5603c0752 100644
> > --- a/arch/arm/boot/dts/nxp/imx/imx7d-remarkable2.dts
> > +++ b/arch/arm/boot/dts/nxp/imx/imx7d-remarkable2.dts
> > @@ -327,6 +327,7 @@ sy7636a: pmic@62 {
> >               regulators {
> >                       reg_epdpmic: vcom {
> >                               regulator-name = "vcom";
> > +                             regulator-always-on;
> >                               regulator-boot-on;
> >                       };
> >               };
>
>
> Here the situation is a bit different. If I remember correctly, there
> is some special EPD logic in userspace on top of some ordinary kernel
> display driver. But thesse things are missing is devicetree?! But
> that might need to learn to deal with regulator enabling/disabling.
>
> Regulator always on is probably not desired here, either.
>
> Regards,
> Andreas

Re: [PATCH 5/7] regulator: sy7636a: report the temperature

[Andreas Kemnade]

On Mon, 26 Oct 2026 00:39:16 +0000
Pablo Mazzini <pmazzini@gmail.com> wrote:

> The separate hwmon cell enables VCOM at probe. Report the temperature
> from here, as the TPS65185 and FP9931 drivers do, and under the chip
> name as they do: read it while the rails are on and return -ENODATA
> otherwise. Boards that need the rails on mark VCOM always-on. Drop the
> hwmon driver.

This might deserve a Fixes: tag as it fixes a race condition in probe.
But then it depends on the 4/7 patch, so it would be quite massive.

[...]
> -
> -static int sy7636a_sensor_probe(struct platform_device *pdev)
> -{
> -	struct regmap *regmap = dev_get_regmap(pdev->dev.parent, NULL);
> -	struct device *hwmon_dev;
> -	int err;
> -
> -	if (!regmap)
> -		return -EPROBE_DEFER;
> -
> -	err = devm_regulator_get_enable(&pdev->dev, "vcom");
> -	if (err)
> -		return err;
> -
and has depended on the regulator being registered first, otherwise
a dummy regulator would be provided.

Regards,
Andreas

Re: [PATCH 4/7] regulator: sy7636a: wait for power good, then switch VCOM_EN

[Pablo Mazzini]

It only changes the power-off order (VCOM before the other rails instead
of all together), which made no visible difference. I will drop it in v2
and keep only the power good wait. Splitting VCOM from vposneg can come
later if someone needs it.

On Mon, Oct 26, 2026 at 8:10 AM Andreas Kemnade <andreas@kemnade.info> wrote:
>
> On Mon, 26 Oct 2026 00:39:15 +0000
> Pablo Mazzini <pmazzini@gmail.com> wrote:
>
> > Wait for power good on enable, as the TPS65185 and FP9931 drivers do.
> >
> which is a good idea that the consumers can relay on the regulators being
> useable when enabled.
>
> > The VCOM_EN line was requested but held low. When it is wired, let it
> > switch VCOM after power good.
> >
> What is the reason for this change? VCOM gets enabled this way, too.
> I added this line just for having the hardware completely documented.
>
> A possible scenario where I could imagine this line being useful:
>
> Calibration of required VCOM voltage, You would turn on everything else,
> do a mode 0 refresh on the EPD and measure some kickback voltage on the
> otherwise inactive VCOM line and have your calculated value for that
> regulator.
>
> To be useful, several things are needed:
> - SY7636a driver needs to model vcom as a separate regulator turning
>   on a vposneg regulator. The TPS65185 driver defines a vposneg regulator.
> - ADC input is needed. The TPS65185 provides (not implemented in the
>   driver yet) such measurement. Of course that can come from another
>   chip.
> - Some (probably userspace logic) to control all of this.
>
> Regards,
> Andreas

Re: [PATCH 0/7] sy7636a: power the panel only during updates

[Andreas Kemnade]

On Mon, 26 Oct 2026 00:39:11 +0000
Pablo Mazzini <pmazzini@gmail.com> wrote:

> The SY7636A hwmon driver keeps VCOM on from probe, so a display driver
> can not power the panel only during updates.
> 
Yes, that is the common use case, power things on, tune update process
to the temperature, update, power off. So it makes completely sense.

> Read the temperature in the regulator driver instead, as the TPS65185
> and FP9931 drivers do, and remove the hwmon driver. Boards that relied
> on VCOM staying on mark it always-on. The regulator also switches VCOM
> with VCOM_EN, gains VCOM voltage control and binds to the I2C device
> directly.
> 
> Tested on a PocketBook Verse (B288).
> 
Which kernel do you use? Just curious, maybe some further upstreaming
actions might be coordinated or cross-reviewed.
There was discussion about in-kernel hwmon readout interface like it
exists for iio.

Regards,
Andreas

Re: [PATCH 0/7] sy7636a: power the panel only during updates

[Pablo Mazzini]

Mainline, on top of linux-next. I started looking at the hwmon IIO
bridge. Coordinating and cross-reviewing further work sounds good.

On Mon, Oct 26, 2026 at 9:37 AM Andreas Kemnade <andreas@kemnade.info> wrote:
>
> On Mon, 26 Oct 2026 00:39:11 +0000
> Pablo Mazzini <pmazzini@gmail.com> wrote:
>
> > The SY7636A hwmon driver keeps VCOM on from probe, so a display driver
> > can not power the panel only during updates.
> >
> Yes, that is the common use case, power things on, tune update process
> to the temperature, update, power off. So it makes completely sense.
>
> > Read the temperature in the regulator driver instead, as the TPS65185
> > and FP9931 drivers do, and remove the hwmon driver. Boards that relied
> > on VCOM staying on mark it always-on. The regulator also switches VCOM
> > with VCOM_EN, gains VCOM voltage control and binds to the I2C device
> > directly.
> >
> > Tested on a PocketBook Verse (B288).
> >
> Which kernel do you use? Just curious, maybe some further upstreaming
> actions might be coordinated or cross-reviewed.
> There was discussion about in-kernel hwmon readout interface like it
> exists for iio.
>
> Regards,
> Andreas

Re: [PATCH 5/7] regulator: sy7636a: report the temperature

[Guenter Roeck]

On Mon, Oct 26, 2026 at 12:39:16AM +0000, Pablo Mazzini wrote:
> The separate hwmon cell enables VCOM at probe. Report the temperature
> from here, as the TPS65185 and FP9931 drivers do, and under the chip
> name as they do: read it while the rails are on and return -ENODATA
> otherwise. Boards that need the rails on mark VCOM always-on. Drop the
> hwmon driver.

This is going the wrong way. The correct solution would be to convert
the hwmon driver to an auxiliary driver. I am not going to approve
this patch. I am not giving an official NACK (yet).

"Other drivers do the same wrong thing" is not an argument.

Guenter