[PATCH v15 0/8] Add support for the silergy,sy7636a

[PATCH v15 0/8] Add support for the silergy,sy7636a

[Alistair Francis]

v15:
 - Address comments on the patches
v14:
 - Merge the thermal driver and hwmon
v13:
 - Address comments on thermal driver
 - Rebase on master (without other patches)
v12:
 - Rebase
v11:
 - Address comments on hwmon
 - Improve "mfd: simple-mfd-i2c: Add a Kconfig name" commit message
v10:
 - Use dev_get_regmap() instead of dev_get_drvdata()
v9:
 - Convert to use the simple-mfd-i2c instead

Alistair Francis (8):
  dt-bindings: mfd: Initial commit of silergy,sy7636a.yaml
  mfd: simple-mfd-i2c: Add a Kconfig name
  mfd: simple-mfd-i2c: Enable support for the silergy,sy7636a
  regulator: sy7636a: Remove requirement on sy7636a mfd
  hwmon: sy7636a: Add temperature driver for sy7636a
  ARM: imx_v6_v7_defconfig: Enable silergy,sy7636a
  ARM: dts: imx7d: remarkable2: Enable silergy,sy7636a
  ARM: dts: imx7d: remarkable2: Enable lcdif

 .../bindings/mfd/silergy,sy7636a.yaml         |  79 ++++++++++
 Documentation/hwmon/index.rst                 |   1 +
 Documentation/hwmon/sy7636a-hwmon.rst         |  24 ++++
 arch/arm/boot/dts/imx7d-remarkable2.dts       | 136 ++++++++++++++++++
 arch/arm/configs/imx_v6_v7_defconfig          |   3 +
 drivers/hwmon/Kconfig                         |   9 ++
 drivers/hwmon/Makefile                        |   1 +
 drivers/hwmon/sy7636a-hwmon.c                 | 108 ++++++++++++++
 drivers/mfd/Kconfig                           |   2 +-
 drivers/mfd/simple-mfd-i2c.c                  |  11 ++
 drivers/regulator/Kconfig                     |   1 -
 drivers/regulator/sy7636a-regulator.c         |   7 +-
 include/linux/mfd/sy7636a.h                   |  36 +++++
 13 files changed, 414 insertions(+), 4 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/mfd/silergy,sy7636a.yaml
 create mode 100644 Documentation/hwmon/sy7636a-hwmon.rst
 create mode 100644 drivers/hwmon/sy7636a-hwmon.c
 create mode 100644 include/linux/mfd/sy7636a.h

-- 
2.31.1

[PATCH v15 1/8] 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>
Reviewed-by: Rob Herring <robh@kernel.org>
Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org>
---
 .../bindings/mfd/silergy,sy7636a.yaml         | 79 +++++++++++++++++++
 1 file changed, 79 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..0566f9498e2f
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/silergy,sy7636a.yaml
@@ -0,0 +1,79 @@
+# 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:
+    description:
+      I2C device address.
+    maxItems: 1
+
+  "#address-cells":
+    const: 1
+
+  "#size-cells":
+    const: 0
+
+  '#thermal-sensor-cells':
+    const: 0
+
+  epd-pwr-good-gpios:
+    description:
+      Specifying the power good GPIOs.
+    maxItems: 1
+
+  regulators:
+    type: object
+
+    properties:
+      compatible:
+        const: silergy,sy7636a-regulator
+
+      vcom:
+        type: object
+        $ref: /schemas/regulator/regulator.yaml#
+        properties:
+          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>;
+        pinctrl-names = "default";
+        pinctrl-0 = <&pinctrl_epdpmic>;
+        #thermal-sensor-cells = <0>;
+
+        regulators {
+          reg_epdpmic: vcom {
+            regulator-name = "vcom";
+            regulator-boot-on;
+          };
+        };
+      };
+    };
+...
-- 
2.31.1

[PATCH v15 2/8] mfd: simple-mfd-i2c: Add a Kconfig name

[Alistair Francis]

Add a Kconfig name to the "Simple Multi-Functional Device support (I2C)"
device so that it can be enabled via menuconfig.

Signed-off-by: Alistair Francis <alistair@alistair23.me>
Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/mfd/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 3fb480818599..97976ea83fdf 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -1194,7 +1194,7 @@ config MFD_SI476X_CORE
 	  module will be called si476x-core.
 
 config MFD_SIMPLE_MFD_I2C
-	tristate
+	tristate "Simple Multi-Functional Device support (I2C)"
 	depends on I2C
 	select MFD_CORE
 	select REGMAP_I2C
-- 
2.31.1

[PATCH v15 3/8] mfd: simple-mfd-i2c: Enable support for the silergy,sy7636a

[Alistair Francis]

Signed-off-by: Alistair Francis <alistair@alistair23.me>
Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/mfd/simple-mfd-i2c.c | 11 +++++++++++
 include/linux/mfd/sy7636a.h  | 36 ++++++++++++++++++++++++++++++++++++
 2 files changed, 47 insertions(+)
 create mode 100644 include/linux/mfd/sy7636a.h

diff --git a/drivers/mfd/simple-mfd-i2c.c b/drivers/mfd/simple-mfd-i2c.c
index 51536691ad9d..f4c8fc3ee463 100644
--- a/drivers/mfd/simple-mfd-i2c.c
+++ b/drivers/mfd/simple-mfd-i2c.c
@@ -62,8 +62,19 @@ static int simple_mfd_i2c_probe(struct i2c_client *i2c)
 	return ret;
 }
 
+static const struct mfd_cell sy7636a_cells[] = {
+	{ .name = "sy7636a-regulator", },
+	{ .name = "sy7636a-temperature", },
+};
+
+static const struct simple_mfd_data silergy_sy7636a = {
+	.mfd_cell = sy7636a_cells,
+	.mfd_cell_size = ARRAY_SIZE(sy7636a_cells),
+};
+
 static const struct of_device_id simple_mfd_i2c_of_match[] = {
 	{ .compatible = "kontron,sl28cpld" },
+	{ .compatible = "silergy,sy7636a", .data = &silergy_sy7636a},
 	{}
 };
 MODULE_DEVICE_TABLE(of, simple_mfd_i2c_of_match);
diff --git a/include/linux/mfd/sy7636a.h b/include/linux/mfd/sy7636a.h
new file mode 100644
index 000000000000..2797c22dabc2
--- /dev/null
+++ b/include/linux/mfd/sy7636a.h
@@ -0,0 +1,36 @@
+/* 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
+#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	10000
+
+#define FAULT_FLAG_SHIFT	1
+
+#endif /* __LINUX_MFD_SY7636A_H */
-- 
2.31.1

[PATCH v15 4/8] regulator: sy7636a: Remove requirement on sy7636a mfd

[Alistair Francis]

Signed-off-by: Alistair Francis <alistair@alistair23.me>
---
 drivers/regulator/Kconfig             | 1 -
 drivers/regulator/sy7636a-regulator.c | 7 +++++--
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
index 6be9b1c8a615..3e515a3fae73 100644
--- a/drivers/regulator/Kconfig
+++ b/drivers/regulator/Kconfig
@@ -1199,7 +1199,6 @@ config REGULATOR_STW481X_VMMC
 
 config REGULATOR_SY7636A
 	tristate "Silergy SY7636A voltage regulator"
-	depends on MFD_SY7636A
 	help
 	  This driver supports Silergy SY3686A voltage regulator.
 
diff --git a/drivers/regulator/sy7636a-regulator.c b/drivers/regulator/sy7636a-regulator.c
index 22fddf868e4c..29fc27c2cda0 100644
--- a/drivers/regulator/sy7636a-regulator.c
+++ b/drivers/regulator/sy7636a-regulator.c
@@ -7,11 +7,14 @@
 // Authors: Lars Ivar Miljeteig <lars.ivar.miljeteig@remarkable.com>
 //          Alistair Francis <alistair@alistair23.me>
 
+#include <linux/gpio/consumer.h>
+#include <linux/i2c.h>
+#include <linux/mfd/sy7636a.h>
 #include <linux/module.h>
 #include <linux/platform_device.h>
+#include <linux/regulator/driver.h>
+#include <linux/regulator/machine.h>
 #include <linux/regmap.h>
-#include <linux/gpio/consumer.h>
-#include <linux/mfd/sy7636a.h>
 
 struct sy7636a_data {
 	struct regmap *regmap;
-- 
2.31.1

[PATCH v15 5/8] hwmon: sy7636a: Add temperature driver for sy7636a

[Alistair Francis]

This is a multi-function device to interface with the sy7636a
EPD PMIC chip from Silergy.

Signed-off-by: Alistair Francis <alistair@alistair23.me>
---
 Documentation/hwmon/index.rst         |   1 +
 Documentation/hwmon/sy7636a-hwmon.rst |  24 ++++++
 drivers/hwmon/Kconfig                 |   9 +++
 drivers/hwmon/Makefile                |   1 +
 drivers/hwmon/sy7636a-hwmon.c         | 108 ++++++++++++++++++++++++++
 5 files changed, 143 insertions(+)
 create mode 100644 Documentation/hwmon/sy7636a-hwmon.rst
 create mode 100644 drivers/hwmon/sy7636a-hwmon.c

diff --git a/Documentation/hwmon/index.rst b/Documentation/hwmon/index.rst
index 7046bf1870d9..a887308850cd 100644
--- a/Documentation/hwmon/index.rst
+++ b/Documentation/hwmon/index.rst
@@ -180,6 +180,7 @@ Hardware Monitoring Kernel Drivers
    smsc47m1
    sparx5-temp
    stpddc60
+   sy7636a-hwmon
    tc654
    tc74
    thmc50
diff --git a/Documentation/hwmon/sy7636a-hwmon.rst b/Documentation/hwmon/sy7636a-hwmon.rst
new file mode 100644
index 000000000000..6b3e36d028dd
--- /dev/null
+++ b/Documentation/hwmon/sy7636a-hwmon.rst
@@ -0,0 +1,24 @@
+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
+      - SoC on-die temperature in milli-degree C
+
+sysfs-Interface
+---------------
+
+temp0_input
+	- SoC on-die temperature (milli-degree C)
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index 64bd3dfba2c4..3139a286c35a 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -1662,6 +1662,15 @@ 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"
+	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 I2C && !PPC
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index baee6a8d4dd1..8f8da52098d1 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -182,6 +182,7 @@ obj-$(CONFIG_SENSORS_SMSC47M1)	+= smsc47m1.o
 obj-$(CONFIG_SENSORS_SMSC47M192)+= smsc47m192.o
 obj-$(CONFIG_SENSORS_SPARX5)	+= sparx5-temp.o
 obj-$(CONFIG_SENSORS_STTS751)	+= stts751.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
new file mode 100644
index 000000000000..84ceaae3a404
--- /dev/null
+++ b/drivers/hwmon/sy7636a-hwmon.c
@@ -0,0 +1,108 @@
+// 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 *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 regulator *regulator;
+	struct device *hwmon_dev;
+	int err;
+
+	if (!regmap)
+		return -EPROBE_DEFER;
+
+	regulator = devm_regulator_get(&pdev->dev, "vcom");
+	if (IS_ERR(regulator)) {
+		return PTR_ERR(regulator);
+	}
+
+	err = regulator_enable(regulator);
+	if (err) {
+		regulator_put(regulator);
+		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");
-- 
2.31.1

[PATCH v15 6/8] 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 | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/arm/configs/imx_v6_v7_defconfig b/arch/arm/configs/imx_v6_v7_defconfig
index 1fbb8e45e604..6add186e189e 100644
--- a/arch/arm/configs/imx_v6_v7_defconfig
+++ b/arch/arm/configs/imx_v6_v7_defconfig
@@ -223,6 +223,7 @@ CONFIG_RN5T618_POWER=m
 CONFIG_SENSORS_MC13783_ADC=y
 CONFIG_SENSORS_GPIO_FAN=y
 CONFIG_SENSORS_IIO_HWMON=y
+CONFIG_SENSORS_SY7636A=y
 CONFIG_THERMAL_STATISTICS=y
 CONFIG_THERMAL_WRITABLE_TRIPS=y
 CONFIG_CPU_THERMAL=y
@@ -239,6 +240,7 @@ CONFIG_MFD_DA9063=y
 CONFIG_MFD_MC13XXX_SPI=y
 CONFIG_MFD_MC13XXX_I2C=y
 CONFIG_MFD_RN5T618=y
+CONFIG_MFD_SIMPLE_MFD_I2C=y
 CONFIG_MFD_STMPE=y
 CONFIG_REGULATOR_FIXED_VOLTAGE=y
 CONFIG_REGULATOR_ANATOP=y
@@ -251,6 +253,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 v15 7/8] 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 | 62 +++++++++++++++++++++++++
 1 file changed, 62 insertions(+)

diff --git a/arch/arm/boot/dts/imx7d-remarkable2.dts b/arch/arm/boot/dts/imx7d-remarkable2.dts
index 89cbf13097a4..b66d28b30d75 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";
@@ -51,6 +72,33 @@ &clks {
 	assigned-clock-rates = <0>, <32768>;
 };
 
+&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>;
+		#address-cells = <1>;
+		#size-cells = <0>;
+		#thermal-sensor-cells = <0>;
+
+		epd-pwr-good-gpios = <&gpio6 21 GPIO_ACTIVE_HIGH>;
+		regulators {
+			reg_epdpmic: vcom {
+				regulator-name = "vcom";
+				regulator-boot-on;
+			};
+		};
+	};
+};
+
 &snvs_pwrkey {
 	status = "okay";
 };
@@ -125,6 +173,20 @@ 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_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

[PATCH v15 8/8] ARM: dts: imx7d: remarkable2: Enable lcdif

[Alistair Francis]

Connect the dispaly on the reMarkable2.

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

diff --git a/arch/arm/boot/dts/imx7d-remarkable2.dts b/arch/arm/boot/dts/imx7d-remarkable2.dts
index b66d28b30d75..fe68f6eaa2ec 100644
--- a/arch/arm/boot/dts/imx7d-remarkable2.dts
+++ b/arch/arm/boot/dts/imx7d-remarkable2.dts
@@ -55,6 +55,16 @@ reg_brcm: regulator-brcm {
 		startup-delay-us = <150>;
 	};
 
+	reg_sdoe: regulator-sdoe {
+		compatible = "regulator-fixed";
+		regulator-name = "SDOE";
+		pinctrl-names = "default", "sleep";
+		pinctrl-0 = <&pinctrl_sdoe_reg>;
+		pinctrl-1 = <&pinctrl_sdoe_reg>;
+		gpio = <&gpio3 27 GPIO_ACTIVE_HIGH>;
+		enable-active-high;
+	};
+
 	wifi_pwrseq: wifi_pwrseq {
 		compatible = "mmc-pwrseq-simple";
 		pinctrl-names = "default";
@@ -63,6 +73,16 @@ wifi_pwrseq: wifi_pwrseq {
 		clocks = <&clks IMX7D_CLKO2_ROOT_DIV>;
 		clock-names = "ext_clock";
 	};
+
+	panel {
+		compatible = "eink,vb3300-kca";
+
+		port {
+			panel_in: endpoint {
+				remote-endpoint = <&display_out>;
+			};
+		};
+	};
 };
 
 &clks {
@@ -99,6 +119,20 @@ reg_epdpmic: vcom {
 	};
 };
 
+&lcdif {
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_lcdif>;
+	lcd-supply = <&reg_epdpmic>;
+	lcd2-supply = <&reg_sdoe>;
+	status = "okay";
+
+	port {
+		display_out: endpoint {
+			remote-endpoint = <&panel_in>;
+		};
+	};
+};
+
 &snvs_pwrkey {
 	status = "okay";
 };
@@ -187,6 +221,46 @@ MX7D_PAD_I2C4_SCL__I2C4_SCL		0x4000007f
 		>;
 	};
 
+	pinctrl_lcdif: lcdifgrp {
+		fsl,pins = <
+			MX7D_PAD_LCD_DATA00__LCD_DATA0		0x79
+			MX7D_PAD_LCD_DATA01__LCD_DATA1		0x79
+			MX7D_PAD_LCD_DATA02__LCD_DATA2		0x79
+			MX7D_PAD_LCD_DATA03__LCD_DATA3		0x79
+			MX7D_PAD_LCD_DATA04__LCD_DATA4		0x79
+			MX7D_PAD_LCD_DATA05__LCD_DATA5		0x79
+			MX7D_PAD_LCD_DATA06__LCD_DATA6		0x79
+			MX7D_PAD_LCD_DATA07__LCD_DATA7		0x79
+			MX7D_PAD_LCD_DATA08__LCD_DATA8		0x79
+			MX7D_PAD_LCD_DATA09__LCD_DATA9		0x79
+			MX7D_PAD_LCD_DATA10__LCD_DATA10		0x79
+			MX7D_PAD_LCD_DATA11__LCD_DATA11		0x79
+			MX7D_PAD_LCD_DATA12__LCD_DATA12		0x79
+			MX7D_PAD_LCD_DATA13__LCD_DATA13		0x79
+			MX7D_PAD_LCD_DATA14__LCD_DATA14		0x79
+			MX7D_PAD_LCD_DATA15__LCD_DATA15		0x79
+
+			MX7D_PAD_LCD_DATA17__LCD_DATA17		0x79
+			MX7D_PAD_LCD_DATA18__LCD_DATA18		0x79
+			MX7D_PAD_LCD_DATA19__LCD_DATA19		0x79
+			MX7D_PAD_LCD_DATA20__LCD_DATA20		0x79
+			MX7D_PAD_LCD_DATA21__LCD_DATA21		0x79
+
+			MX7D_PAD_LCD_DATA23__LCD_DATA23		0x79
+			MX7D_PAD_LCD_CLK__LCD_CLK		0x79
+			MX7D_PAD_LCD_ENABLE__LCD_ENABLE		0x79
+			MX7D_PAD_LCD_VSYNC__LCD_VSYNC		0x79
+			MX7D_PAD_LCD_HSYNC__LCD_HSYNC		0x79
+			MX7D_PAD_LCD_RESET__LCD_RESET		0x79
+		>;
+	};
+
+	pinctrl_sdoe_reg: sdoereggrp {
+		fsl,pins = <
+			MX7D_PAD_LCD_DATA22__GPIO3_IO27		0x74
+		>;
+	};
+
 	pinctrl_uart1: uart1grp {
 		fsl,pins = <
 			MX7D_PAD_UART1_TX_DATA__UART1_DCE_TX	0x79
-- 
2.31.1

Re: [PATCH v15 4/8] regulator: sy7636a: Remove requirement on sy7636a mfd

[Mark Brown]

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

On Wed, Nov 10, 2021 at 10:29:44PM +1000, Alistair Francis wrote:
> Signed-off-by: Alistair Francis <alistair@alistair23.me>

Acked-by: Mark Brown <broonie@kernel.org>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v15 5/8] hwmon: sy7636a: Add temperature driver for sy7636a

[Guenter Roeck]

On Wed, Nov 10, 2021 at 10:29:45PM +1000, Alistair Francis wrote:
> This is a multi-function device to interface with the sy7636a
> EPD PMIC chip from Silergy.
> 
> Signed-off-by: Alistair Francis <alistair@alistair23.me>
> ---
>  Documentation/hwmon/index.rst         |   1 +
>  Documentation/hwmon/sy7636a-hwmon.rst |  24 ++++++
>  drivers/hwmon/Kconfig                 |   9 +++
>  drivers/hwmon/Makefile                |   1 +
>  drivers/hwmon/sy7636a-hwmon.c         | 108 ++++++++++++++++++++++++++
>  5 files changed, 143 insertions(+)
>  create mode 100644 Documentation/hwmon/sy7636a-hwmon.rst
>  create mode 100644 drivers/hwmon/sy7636a-hwmon.c
> 
> diff --git a/Documentation/hwmon/index.rst b/Documentation/hwmon/index.rst
> index 7046bf1870d9..a887308850cd 100644
> --- a/Documentation/hwmon/index.rst
> +++ b/Documentation/hwmon/index.rst
> @@ -180,6 +180,7 @@ Hardware Monitoring Kernel Drivers
>     smsc47m1
>     sparx5-temp
>     stpddc60
> +   sy7636a-hwmon
>     tc654
>     tc74
>     thmc50
> diff --git a/Documentation/hwmon/sy7636a-hwmon.rst b/Documentation/hwmon/sy7636a-hwmon.rst
> new file mode 100644
> index 000000000000..6b3e36d028dd
> --- /dev/null
> +++ b/Documentation/hwmon/sy7636a-hwmon.rst
> @@ -0,0 +1,24 @@
> +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
> +      - SoC on-die temperature in milli-degree C
> +
> +sysfs-Interface
> +---------------
> +
> +temp0_input
> +	- SoC on-die temperature (milli-degree C)
> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> index 64bd3dfba2c4..3139a286c35a 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -1662,6 +1662,15 @@ 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"
> +	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 I2C && !PPC
> diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
> index baee6a8d4dd1..8f8da52098d1 100644
> --- a/drivers/hwmon/Makefile
> +++ b/drivers/hwmon/Makefile
> @@ -182,6 +182,7 @@ obj-$(CONFIG_SENSORS_SMSC47M1)	+= smsc47m1.o
>  obj-$(CONFIG_SENSORS_SMSC47M192)+= smsc47m192.o
>  obj-$(CONFIG_SENSORS_SPARX5)	+= sparx5-temp.o
>  obj-$(CONFIG_SENSORS_STTS751)	+= stts751.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
> new file mode 100644
> index 000000000000..84ceaae3a404
> --- /dev/null
> +++ b/drivers/hwmon/sy7636a-hwmon.c
> @@ -0,0 +1,108 @@
> +// 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 *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 regulator *regulator;
> +	struct device *hwmon_dev;
> +	int err;
> +
> +	if (!regmap)
> +		return -EPROBE_DEFER;
> +
> +	regulator = devm_regulator_get(&pdev->dev, "vcom");
> +	if (IS_ERR(regulator)) {
> +		return PTR_ERR(regulator);
> +	}
> +
> +	err = regulator_enable(regulator);
> +	if (err) {
> +		regulator_put(regulator);

Is this needed ? I would have assumed that the devm_ function
above would ensure that the put is handled automatically.

Guenter

> +		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");
> -- 
> 2.31.1
> 

Re: [PATCH v15 3/8] mfd: simple-mfd-i2c: Enable support for the silergy,sy7636a

[Andreas Kemnade]

Hi,

this all creates a lot of question marks...
One of my main question is whether sy7636a = sy7636 (at least the
driver in the kobo vendor kernels does not have the "A" at the end,
whic does not necessarily mean a difference).

https://www.silergy.com/products/panel_pmic
lists only a SY7636ARMC, so chances are good that the letters were just
stripped away by the driver developers. Printing on chip package is
cryptic so it is not that helpful. It is just "BWNBDA"

 On Wed, 10 Nov 2021 22:29:43 +1000
Alistair Francis <alistair@alistair23.me> wrote:

[...]
> diff --git a/include/linux/mfd/sy7636a.h b/include/linux/mfd/sy7636a.h
> new file mode 100644
> index 000000000000..2797c22dabc2
> --- /dev/null
> +++ b/include/linux/mfd/sy7636a.h
> @@ -0,0 +1,36 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Functions to access SY3686A power management chip.

Typo? or is it really a SY3686A? So what we are talking about?

> + *
> + * Copyright (C) 2021 reMarkable AS - http://www.remarkable.com/
> + */
> +
> +#ifndef __MFD_SY7636A_H
> +#define __MFD_SY7636A_H
> +
> +#define SY7636A_REG_OPERATION_MODE_CRL		0x00
> +#define SY7636A_OPERATION_MODE_CRL_VCOMCTL	BIT(6)

hmm, this thing is called VCOM_MANUAL in the 4.1.15-based driver for the
Kobos and in the 3.0.35 kernel for the Tolinos it is:

// 1:controll the vcom by external VCOM_EN pin 
#define SY7636_REG_OPM_VCOM_EXT_mask    0x1 //  
#define SY7636_REG_OPM_VCOM_EXT_lsb             6 //  

In both kernels, it is set if a gpio is used to control the regulator.
That does not necessarily conflict with your usage. The gpio might just
be hardwired to something in your device. Maybe just a comment about
that issue.
 
> +#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

hmm, what does that maximum mean? What you can set without something
freaking out just by setting it? Or the limit where the driver works
reliably?
> +
> +#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 */

Hmm, are that all defines you know about? I am fine with not including
unused things now, but I am curious.
For comparison, here is my "scratchpad" of all the information I could
squeeze out of the sy7636 driver until now:

OPMODE 0
  RAILS_ON 7
  VCOM_MANUAL 6
  LIGHTNESS 5

  VDDH_DISABLE 4
  VEE_DISABLE 3
  VPOS_DISABLE 2
  VNEG_DISABLE 1
  VCOM_DISABLE 0

  -> combined as RAILS_DISABLE in code

  VCOM: 10000 uV per step, accepts up to 2.75V (that is a bit contradictory)
VCOM_ADJ1 1

VCOM_ADJ2 2
  VCOM2_B8 7
  VDDH_EXT 0..4

VLDO_ADJ 3
  VLDO_ADJ = 5..7
  VPDD_ADJ = 0..4 

VPDD_LEN 4 
  VPPD_LEN 0..4

VEE_VP_EXT 5
  VP_EXT 5..6
  VEE_EXT 0..4

PWRON_DLY = 6
  TDLY4 = 6..7
  TDLY3 = 4..5
  TDLY2 = 2..3
  TDLY1 = 0..1

FAULTFLAGS 7
  FAULS 1..4: to be read out after interrupt and cleared
      0  no faults
      1  UVP at VB rail
      2  UVP at VN rail
      3  UVP at VPOS rail
      4  UVP at VNEG rail
      5  UVP at VDDH rail
      6  UVP at VEE rail
      7  SCP at VB rail
      8  SCP at VN rail
      9  SCP at VPOS rail
      A  SCP at VNEG rail
      B  SCP at VDDH rail
      C  SCP at VEE rail
      D  SCP at VCOM rail
      E  UVLO
      F  Thermal shutdown

  PG 0

THERM 8

Regards,
Andreas

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

[Andreas Kemnade]

On Wed, 10 Nov 2021 22:29:41 +1000
Alistair Francis <alistair@alistair23.me> wrote:

> Initial support for the Silergy SY7636A Power Management chip
> and regulator.
> 
> Signed-off-by: Alistair Francis <alistair@alistair23.me>
> Reviewed-by: Rob Herring <robh@kernel.org>
> Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org>
> ---
>  .../bindings/mfd/silergy,sy7636a.yaml         | 79 +++++++++++++++++++
>  1 file changed, 79 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..0566f9498e2f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/silergy,sy7636a.yaml
[...]
> +  regulators:
> +    type: object
> +
> +    properties:
> +      compatible:
> +        const: silergy,sy7636a-regulator
> +
> +      vcom:
> +        type: object
> +        $ref: /schemas/regulator/regulator.yaml#
> +        properties:
> +          regulator-name:
> +            const: vcom
> +
hmm, this is what? If I understand it correctly, vcom means some
voltage for compensation. On other comparable pmics (e.g. TPS65185
which has also a sane public datasheet, MAX17135) I have seen some
methods to measure a voltage while the display is doing something
defined and then program this voltage non-volatile for compensation
during manufacturing.

If I understand the code correctly all the bunch of voltages are
powered up if this one is enabled.
So at least a description should be suitable.

The other comparable PMICs have at least regulators named VCOM, DISPLAY
(controls several regulators, started with delays configured via
registers) and V3P3. MAX17135 source can be found in NXP kernels, 
TPS65185 in Kobo vendor kernels.

So I would expect to see something similar here and a description or at
least not such a misleading name as vcom if it is for some reason not
feasible to separate the regulators.

Regards,
Andreas

Re: [PATCH v15 3/8] mfd: simple-mfd-i2c: Enable support for the silergy,sy7636a

[Alistair Francis]

On Tue, Nov 16, 2021 at 9:10 AM Andreas Kemnade <andreas@kemnade.info> wrote:
>
> Hi,
>
> this all creates a lot of question marks...
> One of my main question is whether sy7636a = sy7636 (at least the
> driver in the kobo vendor kernels does not have the "A" at the end,
> whic does not necessarily mean a difference).
>
> https://www.silergy.com/products/panel_pmic
> lists only a SY7636ARMC, so chances are good that the letters were just
> stripped away by the driver developers. Printing on chip package is
> cryptic so it is not that helpful. It is just "BWNBDA"

I don't have a definite answer for you. But I think it's sy7636a

The page you linked to above lists SY7636ARMC as well as SY7627RMC,
SY7570RMC. That makes me think that the RMC is a generic suffix and
this actual IC is the SY7636A.

>
>  On Wed, 10 Nov 2021 22:29:43 +1000
> Alistair Francis <alistair@alistair23.me> wrote:
>
> [...]
> > diff --git a/include/linux/mfd/sy7636a.h b/include/linux/mfd/sy7636a.h
> > new file mode 100644
> > index 000000000000..2797c22dabc2
> > --- /dev/null
> > +++ b/include/linux/mfd/sy7636a.h
> > @@ -0,0 +1,36 @@
> > +/* SPDX-License-Identifier: GPL-2.0-only */
> > +/*
> > + * Functions to access SY3686A power management chip.
>
> Typo? or is it really a SY3686A? So what we are talking about?

I think it's SY7636A

>
> > + *
> > + * Copyright (C) 2021 reMarkable AS - http://www.remarkable.com/
> > + */
> > +
> > +#ifndef __MFD_SY7636A_H
> > +#define __MFD_SY7636A_H
> > +
> > +#define SY7636A_REG_OPERATION_MODE_CRL               0x00
> > +#define SY7636A_OPERATION_MODE_CRL_VCOMCTL   BIT(6)
>
> hmm, this thing is called VCOM_MANUAL in the 4.1.15-based driver for the
> Kobos and in the 3.0.35 kernel for the Tolinos it is:
>
> // 1:controll the vcom by external VCOM_EN pin
> #define SY7636_REG_OPM_VCOM_EXT_mask    0x1 //
> #define SY7636_REG_OPM_VCOM_EXT_lsb             6 //
>
> In both kernels, it is set if a gpio is used to control the regulator.
> That does not necessarily conflict with your usage. The gpio might just
> be hardwired to something in your device. Maybe just a comment about
> that issue.

Ok, I'll add a comment.

>
> > +#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
>
> hmm, what does that maximum mean? What you can set without something
> freaking out just by setting it? Or the limit where the driver works
> reliably?

Good question. This is unused so I have just removed it.

> > +
> > +#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 */
>
> Hmm, are that all defines you know about? I am fine with not including
> unused things now, but I am curious.

Yep, this is all that I currently have information on.

> For comparison, here is my "scratchpad" of all the information I could
> squeeze out of the sy7636 driver until now:
>
> OPMODE 0
>   RAILS_ON 7
>   VCOM_MANUAL 6
>   LIGHTNESS 5
>
>   VDDH_DISABLE 4
>   VEE_DISABLE 3
>   VPOS_DISABLE 2
>   VNEG_DISABLE 1
>   VCOM_DISABLE 0
>
>   -> combined as RAILS_DISABLE in code
>
>   VCOM: 10000 uV per step, accepts up to 2.75V (that is a bit contradictory)
> VCOM_ADJ1 1
>
> VCOM_ADJ2 2
>   VCOM2_B8 7
>   VDDH_EXT 0..4
>
> VLDO_ADJ 3
>   VLDO_ADJ = 5..7
>   VPDD_ADJ = 0..4
>
> VPDD_LEN 4
>   VPPD_LEN 0..4
>
> VEE_VP_EXT 5
>   VP_EXT 5..6
>   VEE_EXT 0..4
>
> PWRON_DLY = 6
>   TDLY4 = 6..7
>   TDLY3 = 4..5
>   TDLY2 = 2..3
>   TDLY1 = 0..1
>
> FAULTFLAGS 7
>   FAULS 1..4: to be read out after interrupt and cleared
>       0  no faults
>       1  UVP at VB rail
>       2  UVP at VN rail
>       3  UVP at VPOS rail
>       4  UVP at VNEG rail
>       5  UVP at VDDH rail
>       6  UVP at VEE rail
>       7  SCP at VB rail
>       8  SCP at VN rail
>       9  SCP at VPOS rail
>       A  SCP at VNEG rail
>       B  SCP at VDDH rail
>       C  SCP at VEE rail
>       D  SCP at VCOM rail
>       E  UVLO
>       F  Thermal shutdown
>
>   PG 0
>
> THERM 8

Cool!

Alistair

>
> Regards,
> Andreas

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

[Alistair Francis]

On Thu, Nov 18, 2021 at 7:40 AM Andreas Kemnade <andreas@kemnade.info> wrote:
>
> On Wed, 10 Nov 2021 22:29:41 +1000
> Alistair Francis <alistair@alistair23.me> wrote:
>
> > Initial support for the Silergy SY7636A Power Management chip
> > and regulator.
> >
> > Signed-off-by: Alistair Francis <alistair@alistair23.me>
> > Reviewed-by: Rob Herring <robh@kernel.org>
> > Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org>
> > ---
> >  .../bindings/mfd/silergy,sy7636a.yaml         | 79 +++++++++++++++++++
> >  1 file changed, 79 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..0566f9498e2f
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/mfd/silergy,sy7636a.yaml
> [...]
> > +  regulators:
> > +    type: object
> > +
> > +    properties:
> > +      compatible:
> > +        const: silergy,sy7636a-regulator
> > +
> > +      vcom:
> > +        type: object
> > +        $ref: /schemas/regulator/regulator.yaml#
> > +        properties:
> > +          regulator-name:
> > +            const: vcom
> > +
> hmm, this is what? If I understand it correctly, vcom means some
> voltage for compensation. On other comparable pmics (e.g. TPS65185
> which has also a sane public datasheet, MAX17135) I have seen some
> methods to measure a voltage while the display is doing something
> defined and then program this voltage non-volatile for compensation
> during manufacturing.
>
> If I understand the code correctly all the bunch of voltages are
> powered up if this one is enabled.
> So at least a description should be suitable.
>
> The other comparable PMICs have at least regulators named VCOM, DISPLAY
> (controls several regulators, started with delays configured via
> registers) and V3P3. MAX17135 source can be found in NXP kernels,
> TPS65185 in Kobo vendor kernels.
>
> So I would expect to see something similar here and a description or at
> least not such a misleading name as vcom if it is for some reason not
> feasible to separate the regulators.

This is a vcom in the sense of voltage for compensation. We just
currently don't support setting the vcom.

I had a look at the Kobo code and this is similar to
https://github.com/akemnade/linux/blob/kobo/epdc-pmic-5.15/drivers/regulator/sy7636-regulator.c#L614

So I think that vcom is still the appropriate name for this.

Alistair

>
> Regards,
> Andreas

Re: [PATCH v15 3/8] mfd: simple-mfd-i2c: Enable support for the silergy,sy7636a

[Guenter Roeck]

On 11/23/21 4:14 AM, Alistair Francis wrote:
> On Tue, Nov 16, 2021 at 9:10 AM Andreas Kemnade <andreas@kemnade.info> wrote:
>>
>> Hi,
>>
>> this all creates a lot of question marks...
>> One of my main question is whether sy7636a = sy7636 (at least the
>> driver in the kobo vendor kernels does not have the "A" at the end,
>> whic does not necessarily mean a difference).
>>
>> https://www.silergy.com/products/panel_pmic
>> lists only a SY7636ARMC, so chances are good that the letters were just
>> stripped away by the driver developers. Printing on chip package is
>> cryptic so it is not that helpful. It is just "BWNBDA"
> 
> I don't have a definite answer for you. But I think it's sy7636a
> 
> The page you linked to above lists SY7636ARMC as well as SY7627RMC,
> SY7570RMC. That makes me think that the RMC is a generic suffix and
> this actual IC is the SY7636A.
> 

Almost all chips have an ordering suffix, indicating things like
temperature range or packaging. The datasheet says:

Ordering Information
SY7636 □(□□)□
             | Temperature Code (C)
          | Package Code (RM)
        | Optional Spec Code (A)

The datasheet otherwise refers to the chip as SY7636A.

Guenter

Re: [PATCH v15 3/8] mfd: simple-mfd-i2c: Enable support for the silergy,sy7636a

[Alistair Francis]

On Wed, Nov 24, 2021 at 1:39 AM Guenter Roeck <linux@roeck-us.net> wrote:
>
> On 11/23/21 4:14 AM, Alistair Francis wrote:
> > On Tue, Nov 16, 2021 at 9:10 AM Andreas Kemnade <andreas@kemnade.info> wrote:
> >>
> >> Hi,
> >>
> >> this all creates a lot of question marks...
> >> One of my main question is whether sy7636a = sy7636 (at least the
> >> driver in the kobo vendor kernels does not have the "A" at the end,
> >> whic does not necessarily mean a difference).
> >>
> >> https://www.silergy.com/products/panel_pmic
> >> lists only a SY7636ARMC, so chances are good that the letters were just
> >> stripped away by the driver developers. Printing on chip package is
> >> cryptic so it is not that helpful. It is just "BWNBDA"
> >
> > I don't have a definite answer for you. But I think it's sy7636a
> >
> > The page you linked to above lists SY7636ARMC as well as SY7627RMC,
> > SY7570RMC. That makes me think that the RMC is a generic suffix and
> > this actual IC is the SY7636A.
> >
>
> Almost all chips have an ordering suffix, indicating things like
> temperature range or packaging. The datasheet says:
>
> Ordering Information
> SY7636 □(□□)□
>              | Temperature Code (C)
>           | Package Code (RM)
>         | Optional Spec Code (A)
>
> The datasheet otherwise refers to the chip as SY7636A.

To me this seems like SY7636A is the correct name then.

Alistair

>
> Guenter

Re: [PATCH v15 3/8] mfd: simple-mfd-i2c: Enable support for the silergy,sy7636a

[Andreas Kemnade]

Hi,

On Tue, 23 Nov 2021 07:39:05 -0800
Guenter Roeck <linux@roeck-us.net> wrote:

> On 11/23/21 4:14 AM, Alistair Francis wrote:
> > On Tue, Nov 16, 2021 at 9:10 AM Andreas Kemnade <andreas@kemnade.info> wrote:  
> >>
> >> Hi,
> >>
> >> this all creates a lot of question marks...
> >> One of my main question is whether sy7636a = sy7636 (at least the
> >> driver in the kobo vendor kernels does not have the "A" at the end,
> >> whic does not necessarily mean a difference).
> >>
> >> https://www.silergy.com/products/panel_pmic
> >> lists only a SY7636ARMC, so chances are good that the letters were just
> >> stripped away by the driver developers. Printing on chip package is
> >> cryptic so it is not that helpful. It is just "BWNBDA"  
> > 
> > I don't have a definite answer for you. But I think it's sy7636a
> > 
> > The page you linked to above lists SY7636ARMC as well as SY7627RMC,
> > SY7570RMC. That makes me think that the RMC is a generic suffix and
> > this actual IC is the SY7636A.
> >   
> 
> Almost all chips have an ordering suffix, indicating things like
> temperature range or packaging. The datasheet says:
> 
yes, they have. The only question is where it starts. So did you find a
public datasheet which you can chere

> Ordering Information
> SY7636 □(□□)□
>              | Temperature Code (C)
>           | Package Code (RM)
>         | Optional Spec Code (A)
> 
> The datasheet otherwise refers to the chip as SY7636A.
> 
so there is no indication of something like this where the A really
makes a difference:

commit 28e64a68a2ef1c48f30e8b6803725199929069fc
Author: Daniel Jeong <gshark.jeong@gmail.com>
Date:   Tue Nov 12 15:08:58 2013 -0800

    backlight: lm3630: apply chip revision
    
    The LM3630 chip was revised by TI and chip name was also changed to
    LM3630A.  And register map, default values and initial sequences are
    changed.  The files, lm3630_bl.{c,h} are replaced by lm3630a_bl.{c,h} You
    can find more information about LM3630A(datasheet, evm etc) at
    http://www.ti.com/product/lm3630a

That is good to know. Thanks for your investigation. 

Regards,
Andreas

Re: [PATCH v15 3/8] mfd: simple-mfd-i2c: Enable support for the silergy,sy7636a

[Guenter Roeck]

On 11/24/21 11:35 AM, Andreas Kemnade wrote:
> Hi,
> 
> On Tue, 23 Nov 2021 07:39:05 -0800
> Guenter Roeck <linux@roeck-us.net> wrote:
> 
>> On 11/23/21 4:14 AM, Alistair Francis wrote:
>>> On Tue, Nov 16, 2021 at 9:10 AM Andreas Kemnade <andreas@kemnade.info> wrote:
>>>>
>>>> Hi,
>>>>
>>>> this all creates a lot of question marks...
>>>> One of my main question is whether sy7636a = sy7636 (at least the
>>>> driver in the kobo vendor kernels does not have the "A" at the end,
>>>> whic does not necessarily mean a difference).
>>>>
>>>> https://www.silergy.com/products/panel_pmic
>>>> lists only a SY7636ARMC, so chances are good that the letters were just
>>>> stripped away by the driver developers. Printing on chip package is
>>>> cryptic so it is not that helpful. It is just "BWNBDA"
>>>
>>> I don't have a definite answer for you. But I think it's sy7636a
>>>
>>> The page you linked to above lists SY7636ARMC as well as SY7627RMC,
>>> SY7570RMC. That makes me think that the RMC is a generic suffix and
>>> this actual IC is the SY7636A.
>>>    
>>
>> Almost all chips have an ordering suffix, indicating things like
>> temperature range or packaging. The datasheet says:
>>
> yes, they have. The only question is where it starts. So did you find a
> public datasheet which you can chere
> 

I registered an account on the Silergy web site, and I was subsequently
able to download the datasheet. The document has a "confidential"
watermark, so I can not share it. You should be able to register an
account and download it yourself, though.

>> Ordering Information
>> SY7636 □(□□)□
>>               | Temperature Code (C)
>>            | Package Code (RM)
>>          | Optional Spec Code (A)
>>
>> The datasheet otherwise refers to the chip as SY7636A.
>>
> so there is no indication of something like this where the A really
> makes a difference:
> 

I may be missing it, but I see nothing in the datasheet that would indicate
that or if the "A" has any relevance other than "Optional Spec Code",
and I do not see an explanation for that term either.

Guenter

Re: [PATCH v15 3/8] mfd: simple-mfd-i2c: Enable support for the silergy,sy7636a

[Andreas Kemnade]

On Wed, 24 Nov 2021 12:09:44 -0800
Guenter Roeck <linux@roeck-us.net> wrote:

> On 11/24/21 11:35 AM, Andreas Kemnade wrote:
> > Hi,
> > 
> > On Tue, 23 Nov 2021 07:39:05 -0800
> > Guenter Roeck <linux@roeck-us.net> wrote:
> >   
> >> On 11/23/21 4:14 AM, Alistair Francis wrote:  
> >>> On Tue, Nov 16, 2021 at 9:10 AM Andreas Kemnade <andreas@kemnade.info> wrote:  
> >>>>
> >>>> Hi,
> >>>>
> >>>> this all creates a lot of question marks...
> >>>> One of my main question is whether sy7636a = sy7636 (at least the
> >>>> driver in the kobo vendor kernels does not have the "A" at the end,
> >>>> whic does not necessarily mean a difference).
> >>>>
> >>>> https://www.silergy.com/products/panel_pmic
> >>>> lists only a SY7636ARMC, so chances are good that the letters were just
> >>>> stripped away by the driver developers. Printing on chip package is
> >>>> cryptic so it is not that helpful. It is just "BWNBDA"  
> >>>
> >>> I don't have a definite answer for you. But I think it's sy7636a
> >>>
> >>> The page you linked to above lists SY7636ARMC as well as SY7627RMC,
> >>> SY7570RMC. That makes me think that the RMC is a generic suffix and
> >>> this actual IC is the SY7636A.
> >>>      
> >>
> >> Almost all chips have an ordering suffix, indicating things like
> >> temperature range or packaging. The datasheet says:
> >>  
> > yes, they have. The only question is where it starts. So did you find a
> > public datasheet which you can chere
> >   
> 
> I registered an account on the Silergy web site, and I was subsequently
> able to download the datasheet. The document has a "confidential"
> watermark, so I can not share it. You should be able to register an
> account and download it yourself, though.
> 
ok, did so. 

> >> Ordering Information
> >> SY7636 □(□□)□
> >>               | Temperature Code (C)
> >>            | Package Code (RM)
> >>          | Optional Spec Code (A)
> >>
> >> The datasheet otherwise refers to the chip as SY7636A.
> >>  
> > so there is no indication of something like this where the A really
> > makes a difference:
> >   
> 
> I may be missing it, but I see nothing in the datasheet that would indicate
> that or if the "A" has any relevance other than "Optional Spec Code",
> and I do not see an explanation for that term either.

well things seems to match with things I got from analysing the kobo
sources. So at least the thing in the Kobo Libra H2O seems to be that
one described in the datasheet, so we can have one sy7636a driver for
it.

BTW: If I search for a sy7636 on aliexpress I get some SO-8 lithium
charger ICs.

Regards,
Andreas

Re: [PATCH v15 3/8] mfd: simple-mfd-i2c: Enable support for the silergy,sy7636a

[Guenter Roeck]

On 11/24/21 2:50 PM, Andreas Kemnade wrote:
> On Wed, 24 Nov 2021 12:09:44 -0800
> Guenter Roeck <linux@roeck-us.net> wrote:
> 
>> On 11/24/21 11:35 AM, Andreas Kemnade wrote:
>>> Hi,
>>>
>>> On Tue, 23 Nov 2021 07:39:05 -0800
>>> Guenter Roeck <linux@roeck-us.net> wrote:
>>>    
>>>> On 11/23/21 4:14 AM, Alistair Francis wrote:
>>>>> On Tue, Nov 16, 2021 at 9:10 AM Andreas Kemnade <andreas@kemnade.info> wrote:
>>>>>>
>>>>>> Hi,
>>>>>>
>>>>>> this all creates a lot of question marks...
>>>>>> One of my main question is whether sy7636a = sy7636 (at least the
>>>>>> driver in the kobo vendor kernels does not have the "A" at the end,
>>>>>> whic does not necessarily mean a difference).
>>>>>>
>>>>>> https://www.silergy.com/products/panel_pmic
>>>>>> lists only a SY7636ARMC, so chances are good that the letters were just
>>>>>> stripped away by the driver developers. Printing on chip package is
>>>>>> cryptic so it is not that helpful. It is just "BWNBDA"
>>>>>
>>>>> I don't have a definite answer for you. But I think it's sy7636a
>>>>>
>>>>> The page you linked to above lists SY7636ARMC as well as SY7627RMC,
>>>>> SY7570RMC. That makes me think that the RMC is a generic suffix and
>>>>> this actual IC is the SY7636A.
>>>>>       
>>>>
>>>> Almost all chips have an ordering suffix, indicating things like
>>>> temperature range or packaging. The datasheet says:
>>>>   
>>> yes, they have. The only question is where it starts. So did you find a
>>> public datasheet which you can chere
>>>    
>>
>> I registered an account on the Silergy web site, and I was subsequently
>> able to download the datasheet. The document has a "confidential"
>> watermark, so I can not share it. You should be able to register an
>> account and download it yourself, though.
>>
> ok, did so.
> 
>>>> Ordering Information
>>>> SY7636 □(□□)□
>>>>                | Temperature Code (C)
>>>>             | Package Code (RM)
>>>>           | Optional Spec Code (A)
>>>>
>>>> The datasheet otherwise refers to the chip as SY7636A.
>>>>   
>>> so there is no indication of something like this where the A really
>>> makes a difference:
>>>    
>>
>> I may be missing it, but I see nothing in the datasheet that would indicate
>> that or if the "A" has any relevance other than "Optional Spec Code",
>> and I do not see an explanation for that term either.
> 
> well things seems to match with things I got from analysing the kobo
> sources. So at least the thing in the Kobo Libra H2O seems to be that
> one described in the datasheet, so we can have one sy7636a driver for
> it.
> 
> BTW: If I search for a sy7636 on aliexpress I get some SO-8 lithium
> charger ICs.
> 

The datasheet says "PMIC for Electronic Paper Display".

Guenter

Re: [PATCH v15 3/8] mfd: simple-mfd-i2c: Enable support for the silergy,sy7636a

[Andreas Kemnade]

Hi,

On Wed, 24 Nov 2021 14:56:46 -0800
Guenter Roeck <linux@roeck-us.net> wrote:

[...]
> >>>> Ordering Information
> >>>> SY7636 □(□□)□
> >>>>                | Temperature Code (C)
> >>>>             | Package Code (RM)
> >>>>           | Optional Spec Code (A)
> >>>>
> >>>> The datasheet otherwise refers to the chip as SY7636A.
> >>>>     
> >>> so there is no indication of something like this where the A really
> >>> makes a difference:
> >>>      
> >>
> >> I may be missing it, but I see nothing in the datasheet that would indicate
> >> that or if the "A" has any relevance other than "Optional Spec Code",
> >> and I do not see an explanation for that term either.  
> > 
> > well things seems to match with things I got from analysing the kobo
> > sources. So at least the thing in the Kobo Libra H2O seems to be that
> > one described in the datasheet, so we can have one sy7636a driver for
> > it.
> > 
> > BTW: If I search for a sy7636 on aliexpress I get some SO-8 lithium
> > charger ICs.
> >   
> 
> The datasheet says "PMIC for Electronic Paper Display".
> 
Correct. And we have the silergy vendor prefix in the dt compatible, so
if some other company decides to call its chip sy7636, we can
distinguish.

Regards,
Andreas

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

[Andreas Kemnade]

On Tue, 23 Nov 2021 23:29:26 +1000
Alistair Francis <alistair23@gmail.com> wrote:

> On Thu, Nov 18, 2021 at 7:40 AM Andreas Kemnade <andreas@kemnade.info> wrote:
> >
> > On Wed, 10 Nov 2021 22:29:41 +1000
> > Alistair Francis <alistair@alistair23.me> wrote:
> >  
> > > Initial support for the Silergy SY7636A Power Management chip
> > > and regulator.
> > >
> > > Signed-off-by: Alistair Francis <alistair@alistair23.me>
> > > Reviewed-by: Rob Herring <robh@kernel.org>
> > > Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org>
> > > ---
> > >  .../bindings/mfd/silergy,sy7636a.yaml         | 79 +++++++++++++++++++
> > >  1 file changed, 79 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..0566f9498e2f
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/mfd/silergy,sy7636a.yaml  
> > [...]  
> > > +  regulators:
> > > +    type: object
> > > +
> > > +    properties:
> > > +      compatible:
> > > +        const: silergy,sy7636a-regulator
> > > +
> > > +      vcom:
> > > +        type: object
> > > +        $ref: /schemas/regulator/regulator.yaml#
> > > +        properties:
> > > +          regulator-name:
> > > +            const: vcom
> > > +  
> > hmm, this is what? If I understand it correctly, vcom means some
> > voltage for compensation. On other comparable pmics (e.g. TPS65185
> > which has also a sane public datasheet, MAX17135) I have seen some
> > methods to measure a voltage while the display is doing something
> > defined and then program this voltage non-volatile for compensation
> > during manufacturing.
> >
> > If I understand the code correctly all the bunch of voltages are
> > powered up if this one is enabled.
> > So at least a description should be suitable.
> >
> > The other comparable PMICs have at least regulators named VCOM, DISPLAY
> > (controls several regulators, started with delays configured via
> > registers) and V3P3. MAX17135 source can be found in NXP kernels,
> > TPS65185 in Kobo vendor kernels.
> >
> > So I would expect to see something similar here and a description or at
> > least not such a misleading name as vcom if it is for some reason not
> > feasible to separate the regulators.  
> 
> This is a vcom in the sense of voltage for compensation. We just
> currently don't support setting the vcom.
> 
> I had a look at the Kobo code and this is similar to
> https://github.com/akemnade/linux/blob/kobo/epdc-pmic-5.15/drivers/regulator/sy7636-regulator.c#L614
> 
> So I think that vcom is still the appropriate name for this.
> 
seems that you did not get me. If I understand the code behind it
correctly, it turns on all power rails (the +-15V stuff, VEE and so on)
with the defined delays, not just vcom because it sets
SY7636A_OPERATION_MODE_CRL_ONOFF. Controlling VCOM separately is possible
by using SY7636A_OPERATION_MODE_CRL_VCOMCTL in combintion with a
vcom_en gpio.

I do not see a reason to turn on vcom only without the other higher
voltage rails, so the behaviour is not necessarily wrong but if I read
the binding documentation I would expect that just vcom is turned on.
That is the mismatch I am talking about.

If we agree on this idea that one regulator is enabling everything, I
would adapt my EPDC drm driver and tps65185 driver (which are both in
my clean up to be upstreamable-queue). 

Just another thing to compare with:
https://github.com/Freescale/linux-fslc/blob/4.1-2.0.x-imx/drivers/regulator/max17135-regulator.c
that seems to be the starting point for kobo vendor kernel epd pmics.
They seem to have taken the source and modified things. There we have
regulators with empty ops for that step-up converted stuff, a separate
vcom and a display regulator which really controls all of these step-up
things.

Regards,
Andreas

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

[Alistair Francis]

On Fri, Nov 26, 2021 at 8:59 AM Andreas Kemnade <andreas@kemnade.info> wrote:
>
> On Tue, 23 Nov 2021 23:29:26 +1000
> Alistair Francis <alistair23@gmail.com> wrote:
>
> > On Thu, Nov 18, 2021 at 7:40 AM Andreas Kemnade <andreas@kemnade.info> wrote:
> > >
> > > On Wed, 10 Nov 2021 22:29:41 +1000
> > > Alistair Francis <alistair@alistair23.me> wrote:
> > >
> > > > Initial support for the Silergy SY7636A Power Management chip
> > > > and regulator.
> > > >
> > > > Signed-off-by: Alistair Francis <alistair@alistair23.me>
> > > > Reviewed-by: Rob Herring <robh@kernel.org>
> > > > Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org>
> > > > ---
> > > >  .../bindings/mfd/silergy,sy7636a.yaml         | 79 +++++++++++++++++++
> > > >  1 file changed, 79 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..0566f9498e2f
> > > > --- /dev/null
> > > > +++ b/Documentation/devicetree/bindings/mfd/silergy,sy7636a.yaml
> > > [...]
> > > > +  regulators:
> > > > +    type: object
> > > > +
> > > > +    properties:
> > > > +      compatible:
> > > > +        const: silergy,sy7636a-regulator
> > > > +
> > > > +      vcom:
> > > > +        type: object
> > > > +        $ref: /schemas/regulator/regulator.yaml#
> > > > +        properties:
> > > > +          regulator-name:
> > > > +            const: vcom
> > > > +
> > > hmm, this is what? If I understand it correctly, vcom means some
> > > voltage for compensation. On other comparable pmics (e.g. TPS65185
> > > which has also a sane public datasheet, MAX17135) I have seen some
> > > methods to measure a voltage while the display is doing something
> > > defined and then program this voltage non-volatile for compensation
> > > during manufacturing.
> > >
> > > If I understand the code correctly all the bunch of voltages are
> > > powered up if this one is enabled.
> > > So at least a description should be suitable.
> > >
> > > The other comparable PMICs have at least regulators named VCOM, DISPLAY
> > > (controls several regulators, started with delays configured via
> > > registers) and V3P3. MAX17135 source can be found in NXP kernels,
> > > TPS65185 in Kobo vendor kernels.
> > >
> > > So I would expect to see something similar here and a description or at
> > > least not such a misleading name as vcom if it is for some reason not
> > > feasible to separate the regulators.
> >
> > This is a vcom in the sense of voltage for compensation. We just
> > currently don't support setting the vcom.
> >
> > I had a look at the Kobo code and this is similar to
> > https://github.com/akemnade/linux/blob/kobo/epdc-pmic-5.15/drivers/regulator/sy7636-regulator.c#L614
> >
> > So I think that vcom is still the appropriate name for this.
> >
> seems that you did not get me. If I understand the code behind it
> correctly, it turns on all power rails (the +-15V stuff, VEE and so on)
> with the defined delays, not just vcom because it sets
> SY7636A_OPERATION_MODE_CRL_ONOFF. Controlling VCOM separately is possible
> by using SY7636A_OPERATION_MODE_CRL_VCOMCTL in combintion with a
> vcom_en gpio.
>
> I do not see a reason to turn on vcom only without the other higher
> voltage rails, so the behaviour is not necessarily wrong but if I read
> the binding documentation I would expect that just vcom is turned on.
> That is the mismatch I am talking about.

Ah! Ok I understand. I'll rename it to vdd then.

Alistair

>
> If we agree on this idea that one regulator is enabling everything, I
> would adapt my EPDC drm driver and tps65185 driver (which are both in
> my clean up to be upstreamable-queue).
>
> Just another thing to compare with:
> https://github.com/Freescale/linux-fslc/blob/4.1-2.0.x-imx/drivers/regulator/max17135-regulator.c
> that seems to be the starting point for kobo vendor kernel epd pmics.
> They seem to have taken the source and modified things. There we have
> regulators with empty ops for that step-up converted stuff, a separate
> vcom and a display regulator which really controls all of these step-up
> things.
>
> Regards,
> Andreas

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

[Andreas Kemnade]

Hi,

[...]
> > > This is a vcom in the sense of voltage for compensation. We just
> > > currently don't support setting the vcom.
> > >
> > > I had a look at the Kobo code and this is similar to
> > > https://github.com/akemnade/linux/blob/kobo/epdc-pmic-5.15/drivers/regulator/sy7636-regulator.c#L614
> > >
> > > So I think that vcom is still the appropriate name for this.
> > >  
> > seems that you did not get me. If I understand the code behind it
> > correctly, it turns on all power rails (the +-15V stuff, VEE and so on)
> > with the defined delays, not just vcom because it sets
> > SY7636A_OPERATION_MODE_CRL_ONOFF. Controlling VCOM separately is possible
> > by using SY7636A_OPERATION_MODE_CRL_VCOMCTL in combintion with a
> > vcom_en gpio.
> >
> > I do not see a reason to turn on vcom only without the other higher
> > voltage rails, so the behaviour is not necessarily wrong but if I read
> > the binding documentation I would expect that just vcom is turned on.
> > That is the mismatch I am talking about.  
> 
> Ah! Ok I understand. I'll rename it to vdd then.
> 
Most important is IMHO some human-readable description in the bindings
document.

I am also just wondering whether this kind of logical
regulator which turns on several other regulators is actually accepted
or just slipped through review. I have no strong opinion here. I just
want to be able to clean up the tps65185 driver in the same way and not
having two similar pmics with different bindings and then a mess at the
consumer side. 

Regards,
Andreas

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

[Alistair Francis]

On Thu, Dec 2, 2021 at 8:36 AM Andreas Kemnade <andreas@kemnade.info> wrote:
>
> Hi,
>
> [...]
> > > > This is a vcom in the sense of voltage for compensation. We just
> > > > currently don't support setting the vcom.
> > > >
> > > > I had a look at the Kobo code and this is similar to
> > > > https://github.com/akemnade/linux/blob/kobo/epdc-pmic-5.15/drivers/regulator/sy7636-regulator.c#L614
> > > >
> > > > So I think that vcom is still the appropriate name for this.
> > > >
> > > seems that you did not get me. If I understand the code behind it
> > > correctly, it turns on all power rails (the +-15V stuff, VEE and so on)
> > > with the defined delays, not just vcom because it sets
> > > SY7636A_OPERATION_MODE_CRL_ONOFF. Controlling VCOM separately is possible
> > > by using SY7636A_OPERATION_MODE_CRL_VCOMCTL in combintion with a
> > > vcom_en gpio.
> > >
> > > I do not see a reason to turn on vcom only without the other higher
> > > voltage rails, so the behaviour is not necessarily wrong but if I read
> > > the binding documentation I would expect that just vcom is turned on.
> > > That is the mismatch I am talking about.
> >
> > Ah! Ok I understand. I'll rename it to vdd then.
> >
> Most important is IMHO some human-readable description in the bindings
> document.

That is what I ended up going with instead.

Alistair

>
> I am also just wondering whether this kind of logical
> regulator which turns on several other regulators is actually accepted
> or just slipped through review. I have no strong opinion here. I just
> want to be able to clean up the tps65185 driver in the same way and not
> having two similar pmics with different bindings and then a mess at the
> consumer side.
>
> Regards,
> Andreas