[PATCH 6.1.y-cip 00/13] Add Renesas PMIC RAA215300 driver and builtin RTC support

[PATCH 6.1.y-cip 00/13] Add Renesas PMIC RAA215300 driver and builtin RTC support

[Biju Das]

This patch series aims to add PMIC RAA215300 driver and builtin RTC support
 
All the patches are cherry-picked from the mainline except the last
3 patches.

The last 3 patches are from next and just added here for testing.

Note:
Some improvement patches to mainline after the internal review[1]
[1] https://lore.kernel.org/linux-renesas-soc/20230816135550.146657-1-biju.das.jz@bp.renesas.com/T/#t

Biju Das (13):
  regulator: dt-bindings: Add Renesas RAA215300 PMIC bindings
  regulator: Add Renesas PMIC RAA215300 driver
  regulator: raa215300: Add build dependency with COMMON_CLK
  dt-bindings: rtc: isl1208: Convert to json-schema
  dt-bindings: rtc: isil,isl1208: Document clock and clock-names
    properties
  rtc: isl1208: Drop name variable
  rtc: isl1208: Make similar I2C and DT-based matching table
  rtc: isl1208: Drop enum isl1208_id and split isl1208_configs[]
  rtc: isl1208: Add isl1208_set_xtoscb()
  rtc: isl1208: Add support for the built-in RTC on the PMIC RAA215300
  arm64: defconfig: Enable PMIC RAA215300 and RTC ISL 1208 configs
  arm64: dts: renesas: rzg2l-smarc-som: Enable PMIC and built-in RTC
  arm64: dts: renesas: rzg2lc-smarc-som: Enable PMIC and built-in RTC

 .../bindings/regulator/renesas,raa215300.yaml |  85 ++++++++
 .../devicetree/bindings/rtc/isil,isl1208.txt  |  38 ----
 .../devicetree/bindings/rtc/isil,isl1208.yaml | 100 +++++++++
 .../boot/dts/renesas/rzg2l-smarc-som.dtsi     |  18 ++
 .../boot/dts/renesas/rzg2lc-smarc-som.dtsi    |  18 ++
 arch/arm64/configs/defconfig                  |   2 +
 drivers/regulator/Kconfig                     |   8 +
 drivers/regulator/Makefile                    |   1 +
 drivers/regulator/raa215300.c                 | 190 ++++++++++++++++++
 drivers/rtc/rtc-isl1208.c                     | 130 +++++++++---
 10 files changed, 520 insertions(+), 70 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/regulator/renesas,raa215300.yaml
 delete mode 100644 Documentation/devicetree/bindings/rtc/isil,isl1208.txt
 create mode 100644 Documentation/devicetree/bindings/rtc/isil,isl1208.yaml
 create mode 100644 drivers/regulator/raa215300.c

-- 
2.25.1

[PATCH 6.1.y-cip 01/13] regulator: dt-bindings: Add Renesas RAA215300 PMIC bindings

[Biju Das]

commit fff8f6b0723159f09eb2c067e626fb96402c0e53 upstream.

Document Renesas RAA215300 PMIC bindings.

The RAA215300 is a high Performance 9-Channel PMIC supporting DDR
Memory, with Built-In Charger and RTC.

It supports DDR3, DDR3L, DDR4, and LPDDR4 memory power requirements.
The internally compensated regulators, built-in Real-Time Clock (RTC),
32kHz crystal oscillator, and coin cell battery charger provide a
highly integrated, small footprint power solution ideal for
System-On-Module (SOM) applications. A spread spectrum feature
provides an ease-of-use solution for noise-sensitive audio or RF
applications.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
Link: https://lore.kernel.org/r/Message-Id: <20230623140948.384762-2-biju.das.jz@bp.renesas.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
 .../bindings/regulator/renesas,raa215300.yaml | 85 +++++++++++++++++++
 1 file changed, 85 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/regulator/renesas,raa215300.yaml

diff --git a/Documentation/devicetree/bindings/regulator/renesas,raa215300.yaml b/Documentation/devicetree/bindings/regulator/renesas,raa215300.yaml
new file mode 100644
index 000000000000..97cff71d2967
--- /dev/null
+++ b/Documentation/devicetree/bindings/regulator/renesas,raa215300.yaml
@@ -0,0 +1,85 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/regulator/renesas,raa215300.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Renesas RAA215300 Power Management Integrated Circuit (PMIC)
+
+maintainers:
+  - Biju Das <biju.das.jz@bp.renesas.com>
+
+description: |
+  The RAA215300 is a high-performance, low-cost 9-channel PMIC designed for
+  32-bit and 64-bit MCU and MPU applications. It supports DDR3, DDR3L, DDR4,
+  and LPDDR4 memory power requirements. The internally compensated regulators,
+  built-in Real-Time Clock (RTC), 32kHz crystal oscillator, and coin cell
+  battery charger provide a highly integrated, small footprint power solution
+  ideal for System-On-Module (SOM) applications. A spread spectrum feature
+  provides an ease-of-use solution for noise-sensitive audio or RF applications.
+
+  This device exposes two devices via I2C. One for the integrated RTC IP, and
+  one for everything else.
+
+  Link to datasheet:
+  https://www.renesas.com/in/en/products/power-power-management/multi-channel-power-management-ics-pmics/ssdsoc-power-management-ics-pmic-and-pmus/raa215300-high-performance-9-channel-pmic-supporting-ddr-memory-built-charger-and-rtc
+
+properties:
+  compatible:
+    enum:
+      - renesas,raa215300
+
+  reg:
+    maxItems: 2
+
+  reg-names:
+    items:
+      - const: main
+      - const: rtc
+
+  interrupts:
+    maxItems: 1
+
+  clocks:
+    description: |
+      The clocks are optional. The RTC is disabled, if no clocks are
+      provided(either xin or clkin).
+    maxItems: 1
+
+  clock-names:
+    description: |
+      Use xin, if connected to an external crystal.
+      Use clkin, if connected to an external clock signal.
+    enum:
+      - xin
+      - clkin
+
+required:
+  - compatible
+  - reg
+  - reg-names
+
+additionalProperties: false
+
+examples:
+  - |
+    /* 32.768kHz crystal */
+    x2: x2-clock {
+        compatible = "fixed-clock";
+        #clock-cells = <0>;
+        clock-frequency = <32768>;
+    };
+
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        raa215300: pmic@12 {
+            compatible = "renesas,raa215300";
+            reg = <0x12>, <0x6f>;
+            reg-names = "main", "rtc";
+
+            clocks = <&x2>;
+            clock-names = "xin";
+        };
+    };
-- 
2.25.1

[PATCH 6.1.y-cip 02/13] regulator: Add Renesas PMIC RAA215300 driver

[Biju Das]

commit 7bce16630837c705f72e8fd53a11ae8c236236f4 upstream.

The RAA215300 is a 9-channel PMIC that consists of
 * Internally compensated regulators
 * built-in Real Time Clock (RTC)
 * 32kHz crystal oscillator
 * coin cell battery charger

The RTC on RAA215300 is similar to the IP found in the ISL1208.
The existing driver for the ISL1208 works for this PMIC too,
however the RAA215300 exposes two devices via I2C, one for the RTC
IP, and one for everything else. The RTC IP has to be enabled
by the other I2C device, therefore this driver is necessary to get
the RTC to work.

The external oscillator bit is inverted on PMIC version 0x11.

Add PMIC RAA215300 driver for enabling RTC block and instantiating
RTC device based on PMIC version.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Link: https://lore.kernel.org/r/Message-Id: <20230623140948.384762-3-biju.das.jz@bp.renesas.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
 drivers/regulator/Kconfig     |   7 ++
 drivers/regulator/Makefile    |   1 +
 drivers/regulator/raa215300.c | 190 ++++++++++++++++++++++++++++++++++
 3 files changed, 198 insertions(+)
 create mode 100644 drivers/regulator/raa215300.c

diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
index 070e4403c6c2..524abe99e406 100644
--- a/drivers/regulator/Kconfig
+++ b/drivers/regulator/Kconfig
@@ -1016,6 +1016,13 @@ config REGULATOR_QCOM_USB_VBUS
 	  Say M here if you want to include support for enabling the VBUS output
 	  as a module. The module will be named "qcom_usb_vbus_regulator".
 
+config REGULATOR_RAA215300
+	tristate "Renesas RAA215300 driver"
+	select REGMAP_I2C
+	depends on I2C
+	help
+	  Support for the Renesas RAA215300 PMIC.
+
 config REGULATOR_RASPBERRYPI_TOUCHSCREEN_ATTINY
 	tristate "Raspberry Pi 7-inch touchscreen panel ATTINY regulator"
 	depends on BACKLIGHT_CLASS_DEVICE
diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile
index 5962307e1130..58ea83a3f800 100644
--- a/drivers/regulator/Makefile
+++ b/drivers/regulator/Makefile
@@ -122,6 +122,7 @@ obj-$(CONFIG_REGULATOR_TPS51632) += tps51632-regulator.o
 obj-$(CONFIG_REGULATOR_PBIAS) += pbias-regulator.o
 obj-$(CONFIG_REGULATOR_PCAP) += pcap-regulator.o
 obj-$(CONFIG_REGULATOR_PCF50633) += pcf50633-regulator.o
+obj-$(CONFIG_REGULATOR_RAA215300) += raa215300.o
 obj-$(CONFIG_REGULATOR_RASPBERRYPI_TOUCHSCREEN_ATTINY)  += rpi-panel-attiny-regulator.o
 obj-$(CONFIG_REGULATOR_RC5T583)  += rc5t583-regulator.o
 obj-$(CONFIG_REGULATOR_RK808)   += rk808-regulator.o
diff --git a/drivers/regulator/raa215300.c b/drivers/regulator/raa215300.c
new file mode 100644
index 000000000000..24a1c89f5dbc
--- /dev/null
+++ b/drivers/regulator/raa215300.c
@@ -0,0 +1,190 @@
+// SPDX-License-Identifier: GPL-2.0
+//
+// Renesas RAA215300 PMIC driver
+//
+// Copyright (C) 2023 Renesas Electronics Corporation
+//
+
+#include <linux/clk.h>
+#include <linux/clkdev.h>
+#include <linux/clk-provider.h>
+#include <linux/err.h>
+#include <linux/i2c.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/regmap.h>
+
+#define RAA215300_FAULT_LATCHED_STATUS_1	0x59
+#define RAA215300_FAULT_LATCHED_STATUS_2	0x5a
+#define RAA215300_FAULT_LATCHED_STATUS_3	0x5b
+#define RAA215300_FAULT_LATCHED_STATUS_4	0x5c
+#define RAA215300_FAULT_LATCHED_STATUS_6	0x5e
+
+#define RAA215300_INT_MASK_1	0x64
+#define RAA215300_INT_MASK_2	0x65
+#define RAA215300_INT_MASK_3	0x66
+#define RAA215300_INT_MASK_4	0x67
+#define RAA215300_INT_MASK_6	0x68
+
+#define RAA215300_REG_BLOCK_EN	0x6c
+#define RAA215300_HW_REV	0xf8
+
+#define RAA215300_INT_MASK_1_ALL	GENMASK(5, 0)
+#define RAA215300_INT_MASK_2_ALL	GENMASK(3, 0)
+#define RAA215300_INT_MASK_3_ALL	GENMASK(5, 0)
+#define RAA215300_INT_MASK_4_ALL	BIT(0)
+#define RAA215300_INT_MASK_6_ALL	GENMASK(7, 0)
+
+#define RAA215300_REG_BLOCK_EN_RTC_EN	BIT(6)
+#define RAA215300_RTC_DEFAULT_ADDR	0x6f
+
+const char *clkin_name = "clkin";
+const char *xin_name = "xin";
+static struct clk *clk;
+
+static const struct regmap_config raa215300_regmap_config = {
+	.reg_bits = 8,
+	.val_bits = 8,
+	.max_register = 0xff,
+};
+
+static void raa215300_rtc_unregister_device(void *data)
+{
+	i2c_unregister_device(data);
+	if (!clk) {
+		clk_unregister_fixed_rate(clk);
+		clk = NULL;
+	}
+}
+
+static int raa215300_clk_present(struct i2c_client *client, const char *name)
+{
+	struct clk *clk;
+
+	clk = devm_clk_get_optional(&client->dev, name);
+	if (IS_ERR(clk))
+		return PTR_ERR(clk);
+
+	return !!clk;
+}
+
+static int raa215300_i2c_probe(struct i2c_client *client)
+{
+	struct device *dev = &client->dev;
+	const char *clk_name = xin_name;
+	unsigned int pmic_version, val;
+	struct regmap *regmap;
+	int ret;
+
+	regmap = devm_regmap_init_i2c(client, &raa215300_regmap_config);
+	if (IS_ERR(regmap))
+		return dev_err_probe(dev, PTR_ERR(regmap),
+				     "regmap i2c init failed\n");
+
+	ret = regmap_read(regmap, RAA215300_HW_REV, &pmic_version);
+	if (ret < 0)
+		return dev_err_probe(dev, ret, "HW rev read failed\n");
+
+	dev_dbg(dev, "RAA215300 PMIC version 0x%04x\n", pmic_version);
+
+	/* Clear all blocks except RTC, if enabled */
+	regmap_read(regmap, RAA215300_REG_BLOCK_EN, &val);
+	val &= RAA215300_REG_BLOCK_EN_RTC_EN;
+	regmap_write(regmap, RAA215300_REG_BLOCK_EN, val);
+
+	/*Clear the latched registers */
+	regmap_read(regmap, RAA215300_FAULT_LATCHED_STATUS_1, &val);
+	regmap_write(regmap, RAA215300_FAULT_LATCHED_STATUS_1, val);
+	regmap_read(regmap, RAA215300_FAULT_LATCHED_STATUS_2, &val);
+	regmap_write(regmap, RAA215300_FAULT_LATCHED_STATUS_2, val);
+	regmap_read(regmap, RAA215300_FAULT_LATCHED_STATUS_3, &val);
+	regmap_write(regmap, RAA215300_FAULT_LATCHED_STATUS_3, val);
+	regmap_read(regmap, RAA215300_FAULT_LATCHED_STATUS_4, &val);
+	regmap_write(regmap, RAA215300_FAULT_LATCHED_STATUS_4, val);
+	regmap_read(regmap, RAA215300_FAULT_LATCHED_STATUS_6, &val);
+	regmap_write(regmap, RAA215300_FAULT_LATCHED_STATUS_6, val);
+
+	/* Mask all the PMIC interrupts */
+	regmap_write(regmap, RAA215300_INT_MASK_1, RAA215300_INT_MASK_1_ALL);
+	regmap_write(regmap, RAA215300_INT_MASK_2, RAA215300_INT_MASK_2_ALL);
+	regmap_write(regmap, RAA215300_INT_MASK_3, RAA215300_INT_MASK_3_ALL);
+	regmap_write(regmap, RAA215300_INT_MASK_4, RAA215300_INT_MASK_4_ALL);
+	regmap_write(regmap, RAA215300_INT_MASK_6, RAA215300_INT_MASK_6_ALL);
+
+	ret = raa215300_clk_present(client, xin_name);
+	if (ret < 0) {
+		return ret;
+	} else if (!ret) {
+		ret = raa215300_clk_present(client, clkin_name);
+		if (ret < 0)
+			return ret;
+
+		clk_name = clkin_name;
+	}
+
+	if (ret) {
+		char *name = pmic_version >= 0x12 ? "isl1208" : "raa215300_a0";
+		struct device_node *np = client->dev.of_node;
+		u32 addr = RAA215300_RTC_DEFAULT_ADDR;
+		struct i2c_board_info info = {};
+		struct i2c_client *rtc_client;
+		ssize_t size;
+
+		clk = clk_register_fixed_rate(NULL, clk_name, NULL, 0, 32000);
+		clk_register_clkdev(clk, clk_name, NULL);
+
+		if (np) {
+			int i;
+
+			i = of_property_match_string(np, "reg-names", "rtc");
+			if (i >= 0)
+				of_property_read_u32_index(np, "reg", i, &addr);
+		}
+
+		info.addr = addr;
+		if (client->irq > 0)
+			info.irq = client->irq;
+
+		size = strscpy(info.type, name, sizeof(info.type));
+		if (size < 0)
+			return dev_err_probe(dev, size,
+					     "Invalid device name: %s\n", name);
+
+		/* Enable RTC block */
+		regmap_update_bits(regmap, RAA215300_REG_BLOCK_EN,
+				   RAA215300_REG_BLOCK_EN_RTC_EN,
+				   RAA215300_REG_BLOCK_EN_RTC_EN);
+
+		rtc_client = i2c_new_client_device(client->adapter, &info);
+		if (IS_ERR(rtc_client))
+			return PTR_ERR(rtc_client);
+
+		ret = devm_add_action_or_reset(dev,
+					       raa215300_rtc_unregister_device,
+					       rtc_client);
+		if (ret < 0)
+			return ret;
+	}
+
+	return 0;
+}
+
+static const struct of_device_id raa215300_dt_match[] = {
+	{ .compatible = "renesas,raa215300" },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, raa215300_dt_match);
+
+static struct i2c_driver raa215300_i2c_driver = {
+	.driver = {
+		.name = "raa215300",
+		.of_match_table = raa215300_dt_match,
+	},
+	.probe_new = raa215300_i2c_probe,
+};
+module_i2c_driver(raa215300_i2c_driver);
+
+MODULE_DESCRIPTION("Renesas RAA215300 PMIC driver");
+MODULE_AUTHOR("Fabrizio Castro <fabrizio.castro.jz@renesas.com>");
+MODULE_AUTHOR("Biju Das <biju.das.jz@bp.renesas.com>");
+MODULE_LICENSE("GPL");
-- 
2.25.1

[PATCH 6.1.y-cip 03/13] regulator: raa215300: Add build dependency with COMMON_CLK

[Biju Das]

commit e9bd04e52d649c3cfd713b594c5db35cab03c42b upstream.

The COMMON_CLK config is not enabled in some of the architectures.
This causes build issues. Fix these issues by adding build dependency.

ERROR: modpost: "clk_unregister_fixed_rate" [drivers/regulator/raa215300.ko] undefined!
ERROR: modpost: "clk_register_fixed_rate" [drivers/regulator/raa215300.ko] undefined!

Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202306282012.sPQAuAN7-lkp@intel.com/
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Link: https://lore.kernel.org/r/20230628174004.63984-1-biju.das.jz@bp.renesas.com
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
Signed-off-by: Mark Brown <broonie@kernel.org>
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
 drivers/regulator/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
index 524abe99e406..905c43525ee6 100644
--- a/drivers/regulator/Kconfig
+++ b/drivers/regulator/Kconfig
@@ -1019,6 +1019,7 @@ config REGULATOR_QCOM_USB_VBUS
 config REGULATOR_RAA215300
 	tristate "Renesas RAA215300 driver"
 	select REGMAP_I2C
+	depends on COMMON_CLK
 	depends on I2C
 	help
 	  Support for the Renesas RAA215300 PMIC.
-- 
2.25.1

[PATCH 6.1.y-cip 04/13] dt-bindings: rtc: isl1208: Convert to json-schema

[Biju Das]

commit ac739bac5201d4308cba2525dacb5da654b3ff31 upstream.

Convert the isl1208 RTC device tree binding documentation to json-schema.

Update the example to match reality.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Link: https://lore.kernel.org/r/20230623140948.384762-5-biju.das.jz@bp.renesas.com
Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
 .../devicetree/bindings/rtc/isil,isl1208.txt  | 38 --------
 .../devicetree/bindings/rtc/isil,isl1208.yaml | 89 +++++++++++++++++++
 2 files changed, 89 insertions(+), 38 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/rtc/isil,isl1208.txt
 create mode 100644 Documentation/devicetree/bindings/rtc/isil,isl1208.yaml

diff --git a/Documentation/devicetree/bindings/rtc/isil,isl1208.txt b/Documentation/devicetree/bindings/rtc/isil,isl1208.txt
deleted file mode 100644
index 51f003006f04..000000000000
--- a/Documentation/devicetree/bindings/rtc/isil,isl1208.txt
+++ /dev/null
@@ -1,38 +0,0 @@
-Intersil ISL1209/19 I2C RTC/Alarm chip with event in
-
-ISL12X9 have additional pins EVIN and #EVDET for tamper detection, while the
-ISL1208 and ISL1218 do not.  They are all use the same driver with the bindings
-described here, with chip specific properties as noted.
-
-Required properties supported by the device:
- - "compatible": Should be one of the following:
-		- "isil,isl1208"
-		- "isil,isl1209"
-		- "isil,isl1218"
-		- "isil,isl1219"
- - "reg": I2C bus address of the device
-
-Optional properties:
- - "interrupt-names": list which may contains "irq" and "evdet"
-	evdet applies to isl1209 and isl1219 only
- - "interrupts": list of interrupts for "irq" and "evdet"
-	evdet applies to isl1209 and isl1219 only
- - "isil,ev-evienb": Enable or disable internal pull on EVIN pin
-	Applies to isl1209 and isl1219 only
-	Possible values are 0 and 1
-	Value 0 enables internal pull-up on evin pin, 1 disables it.
-	Default will leave the non-volatile configuration of the pullup
-	as is.
-
-Example isl1219 node with #IRQ pin connected to SoC gpio1 pin12 and #EVDET pin
-connected to SoC gpio2 pin 24 and internal pull-up enabled in EVIN pin.
-
-	isl1219: rtc@68 {
-		compatible = "isil,isl1219";
-		reg = <0x68>;
-		interrupt-names = "irq", "evdet";
-		interrupts-extended = <&gpio1 12 IRQ_TYPE_EDGE_FALLING>,
-			<&gpio2 24 IRQ_TYPE_EDGE_FALLING>;
-		isil,ev-evienb = <1>;
-	};
-
diff --git a/Documentation/devicetree/bindings/rtc/isil,isl1208.yaml b/Documentation/devicetree/bindings/rtc/isil,isl1208.yaml
new file mode 100644
index 000000000000..565965147ce6
--- /dev/null
+++ b/Documentation/devicetree/bindings/rtc/isil,isl1208.yaml
@@ -0,0 +1,89 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/rtc/isil,isl1208.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Intersil ISL1209/19 I2C RTC/Alarm chip with event in
+
+maintainers:
+  - Biju Das <biju.das.jz@bp.renesas.com>
+  - Trent Piepho <tpiepho@gmail.com>
+
+description:
+  ISL12X9 have additional pins EVIN and EVDET for tamper detection, while the
+  ISL1208 and ISL1218 do not.
+
+properties:
+  compatible:
+    enum:
+      - isil,isl1208
+      - isil,isl1209
+      - isil,isl1218
+      - isil,isl1219
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    minItems: 1
+    maxItems: 2
+
+  interrupt-names:
+    minItems: 1
+    items:
+      - const: irq
+      - const: evdet
+
+  isil,ev-evienb:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    enum: [ 0, 1 ]
+    description: |
+      Enable or disable internal pull on EVIN pin
+      Default will leave the non-volatile configuration of the pullup
+      as is.
+        <0> : Enables internal pull-up on evin pin
+        <1> : Disables internal pull-up on evin pin
+
+required:
+  - compatible
+  - reg
+
+allOf:
+  - $ref: rtc.yaml#
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - isil,isl1209
+              - isil,isl1219
+    then:
+      properties:
+        interrupts:
+          maxItems: 2
+        interrupt-names:
+          items:
+            - const: irq
+            - const: evdet
+    else:
+      properties:
+        interrupts:
+          maxItems: 1
+        interrupt-names:
+          items:
+            - const: irq
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        rtc_twi: rtc@6f {
+            compatible = "isil,isl1208";
+            reg = <0x6f>;
+        };
+    };
-- 
2.25.1

[PATCH 6.1.y-cip 05/13] dt-bindings: rtc: isil,isl1208: Document clock and clock-names properties

[Biju Das]

commit 138f352556d791d7e0ca3ac9a4f4815123af8c82 upstream.

As per the HW manual, XTOSCB bit setting is as follows

If using an external clock signal, set the XTOSCB bit as 1 to
disable the crystal oscillator.

If using an external crystal, the XTOSCB bit needs to be set at 0
to enable the crystal oscillator.

Document clock and clock-names properties.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
Link: https://lore.kernel.org/r/20230623140948.384762-6-biju.das.jz@bp.renesas.com
Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
 .../devicetree/bindings/rtc/isil,isl1208.yaml         | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/Documentation/devicetree/bindings/rtc/isil,isl1208.yaml b/Documentation/devicetree/bindings/rtc/isil,isl1208.yaml
index 565965147ce6..11f7378d4997 100644
--- a/Documentation/devicetree/bindings/rtc/isil,isl1208.yaml
+++ b/Documentation/devicetree/bindings/rtc/isil,isl1208.yaml
@@ -25,6 +25,17 @@ properties:
   reg:
     maxItems: 1
 
+  clocks:
+    maxItems: 1
+
+  clock-names:
+    description: |
+      Use xin, if connected to an external crystal.
+      Use clkin, if connected to an external clock signal.
+    enum:
+      - xin
+      - clkin
+
   interrupts:
     minItems: 1
     maxItems: 2
-- 
2.25.1

[PATCH 6.1.y-cip 06/13] rtc: isl1208: Drop name variable

[Biju Das]

commit 380960c40a1d106bba3476c9a010eaf28195115d upstream.

Drop unused name variable from struct isl1208_config.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
Link: https://lore.kernel.org/r/20230623140948.384762-7-biju.das.jz@bp.renesas.com
Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
 drivers/rtc/rtc-isl1208.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/rtc/rtc-isl1208.c b/drivers/rtc/rtc-isl1208.c
index f448a525333e..1716c0c3b550 100644
--- a/drivers/rtc/rtc-isl1208.c
+++ b/drivers/rtc/rtc-isl1208.c
@@ -79,15 +79,14 @@ enum isl1208_id {
 
 /* Chip capabilities table */
 static const struct isl1208_config {
-	const char	name[8];
 	unsigned int	nvmem_length;
 	unsigned	has_tamper:1;
 	unsigned	has_timestamp:1;
 } isl1208_configs[] = {
-	[TYPE_ISL1208] = { "isl1208", 2, false, false },
-	[TYPE_ISL1209] = { "isl1209", 2, true,  false },
-	[TYPE_ISL1218] = { "isl1218", 8, false, false },
-	[TYPE_ISL1219] = { "isl1219", 2, true,  true },
+	[TYPE_ISL1208] = { 2, false, false },
+	[TYPE_ISL1209] = { 2, true,  false },
+	[TYPE_ISL1218] = { 8, false, false },
+	[TYPE_ISL1219] = { 2, true,  true },
 };
 
 static const struct i2c_device_id isl1208_id[] = {
-- 
2.25.1

[PATCH 6.1.y-cip 07/13] rtc: isl1208: Make similar I2C and DT-based matching table

[Biju Das]

commit fbc06a53561c64ec6d7f9a1b3bc04597de4cbb2d upstream.

The isl1208_id[].driver_data could store a pointer to the config,
like for DT-based matching, making I2C and DT-based matching
more similar.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
Link: https://lore.kernel.org/r/20230623140948.384762-8-biju.das.jz@bp.renesas.com
Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
 drivers/rtc/rtc-isl1208.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/rtc/rtc-isl1208.c b/drivers/rtc/rtc-isl1208.c
index 1716c0c3b550..67487a784e58 100644
--- a/drivers/rtc/rtc-isl1208.c
+++ b/drivers/rtc/rtc-isl1208.c
@@ -90,10 +90,10 @@ static const struct isl1208_config {
 };
 
 static const struct i2c_device_id isl1208_id[] = {
-	{ "isl1208", TYPE_ISL1208 },
-	{ "isl1209", TYPE_ISL1209 },
-	{ "isl1218", TYPE_ISL1218 },
-	{ "isl1219", TYPE_ISL1219 },
+	{ "isl1208", .driver_data = (kernel_ulong_t)&isl1208_configs[TYPE_ISL1208] },
+	{ "isl1209", .driver_data = (kernel_ulong_t)&isl1208_configs[TYPE_ISL1209] },
+	{ "isl1218", .driver_data = (kernel_ulong_t)&isl1208_configs[TYPE_ISL1218] },
+	{ "isl1219", .driver_data = (kernel_ulong_t)&isl1208_configs[TYPE_ISL1219] },
 	{ }
 };
 MODULE_DEVICE_TABLE(i2c, isl1208_id);
@@ -820,9 +820,9 @@ isl1208_probe(struct i2c_client *client, const struct i2c_device_id *id)
 		if (!isl1208->config)
 			return -ENODEV;
 	} else {
-		if (id->driver_data >= ISL_LAST_ID)
+		if (!id)
 			return -ENODEV;
-		isl1208->config = &isl1208_configs[id->driver_data];
+		isl1208->config = (struct isl1208_config *)id->driver_data;
 	}
 
 	isl1208->rtc = devm_rtc_allocate_device(&client->dev);
-- 
2.25.1

[PATCH 6.1.y-cip 08/13] rtc: isl1208: Drop enum isl1208_id and split isl1208_configs[]

[Biju Das]

commit 5923fc75d0dfcebce53894ddada7e2440d756f8b upstream.

Drop enum isl1208_id and split the array isl1208_configs[] as individual
variables, and make lines shorter by referring to e.g. &config_isl1219
instead of &isl1208_configs[TYPE_ISL1219].

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
Link: https://lore.kernel.org/r/20230623140948.384762-9-biju.das.jz@bp.renesas.com
Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
 drivers/rtc/rtc-isl1208.c | 56 +++++++++++++++++++++++----------------
 1 file changed, 33 insertions(+), 23 deletions(-)

diff --git a/drivers/rtc/rtc-isl1208.c b/drivers/rtc/rtc-isl1208.c
index 67487a784e58..80ecb724ae6b 100644
--- a/drivers/rtc/rtc-isl1208.c
+++ b/drivers/rtc/rtc-isl1208.c
@@ -68,41 +68,51 @@
 
 static struct i2c_driver isl1208_driver;
 
-/* ISL1208 various variants */
-enum isl1208_id {
-	TYPE_ISL1208 = 0,
-	TYPE_ISL1209,
-	TYPE_ISL1218,
-	TYPE_ISL1219,
-	ISL_LAST_ID
-};
-
 /* Chip capabilities table */
-static const struct isl1208_config {
+struct isl1208_config {
 	unsigned int	nvmem_length;
 	unsigned	has_tamper:1;
 	unsigned	has_timestamp:1;
-} isl1208_configs[] = {
-	[TYPE_ISL1208] = { 2, false, false },
-	[TYPE_ISL1209] = { 2, true,  false },
-	[TYPE_ISL1218] = { 8, false, false },
-	[TYPE_ISL1219] = { 2, true,  true },
+};
+
+static const struct isl1208_config config_isl1208 = {
+	.nvmem_length = 2,
+	.has_tamper = false,
+	.has_timestamp = false
+};
+
+static const struct isl1208_config config_isl1209 = {
+	.nvmem_length = 2,
+	.has_tamper = true,
+	.has_timestamp = false
+};
+
+static const struct isl1208_config config_isl1218 = {
+	.nvmem_length = 8,
+	.has_tamper = false,
+	.has_timestamp = false
+};
+
+static const struct isl1208_config config_isl1219 = {
+	.nvmem_length = 2,
+	.has_tamper = true,
+	.has_timestamp = true
 };
 
 static const struct i2c_device_id isl1208_id[] = {
-	{ "isl1208", .driver_data = (kernel_ulong_t)&isl1208_configs[TYPE_ISL1208] },
-	{ "isl1209", .driver_data = (kernel_ulong_t)&isl1208_configs[TYPE_ISL1209] },
-	{ "isl1218", .driver_data = (kernel_ulong_t)&isl1208_configs[TYPE_ISL1218] },
-	{ "isl1219", .driver_data = (kernel_ulong_t)&isl1208_configs[TYPE_ISL1219] },
+	{ "isl1208", .driver_data = (kernel_ulong_t)&config_isl1208 },
+	{ "isl1209", .driver_data = (kernel_ulong_t)&config_isl1209 },
+	{ "isl1218", .driver_data = (kernel_ulong_t)&config_isl1218 },
+	{ "isl1219", .driver_data = (kernel_ulong_t)&config_isl1219 },
 	{ }
 };
 MODULE_DEVICE_TABLE(i2c, isl1208_id);
 
 static const __maybe_unused struct of_device_id isl1208_of_match[] = {
-	{ .compatible = "isil,isl1208", .data = &isl1208_configs[TYPE_ISL1208] },
-	{ .compatible = "isil,isl1209", .data = &isl1208_configs[TYPE_ISL1209] },
-	{ .compatible = "isil,isl1218", .data = &isl1208_configs[TYPE_ISL1218] },
-	{ .compatible = "isil,isl1219", .data = &isl1208_configs[TYPE_ISL1219] },
+	{ .compatible = "isil,isl1208", .data = &config_isl1208 },
+	{ .compatible = "isil,isl1209", .data = &config_isl1209 },
+	{ .compatible = "isil,isl1218", .data = &config_isl1218 },
+	{ .compatible = "isil,isl1219", .data = &config_isl1219 },
 	{ }
 };
 MODULE_DEVICE_TABLE(of, isl1208_of_match);
-- 
2.25.1

[PATCH 6.1.y-cip 09/13] rtc: isl1208: Add isl1208_set_xtoscb()

[Biju Das]

commit 262f72b4656e182eefaab91ab24a7575dda5524f upstream.

As per the HW manual, set the XTOSCB bit as follows:

If using an external clock signal, set the XTOSCB bit as 1 to
disable the crystal oscillator.

If using an external crystal, the XTOSCB bit needs to be set at 0
to enable the crystal oscillator.

Add isl1208_set_xtoscb() to set XTOSCB bit based on the clock-names
property. Fallback is enabling the internal crystal oscillator.

While at it, introduce a variable "sr" for reading the status register
in probe() as it is reused for writing and also remove the unnecessary
blank line.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Link: https://lore.kernel.org/r/20230623140948.384762-10-biju.das.jz@bp.renesas.com
Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
 drivers/rtc/rtc-isl1208.c | 57 ++++++++++++++++++++++++++++++++++-----
 1 file changed, 51 insertions(+), 6 deletions(-)

diff --git a/drivers/rtc/rtc-isl1208.c b/drivers/rtc/rtc-isl1208.c
index 80ecb724ae6b..915f23668228 100644
--- a/drivers/rtc/rtc-isl1208.c
+++ b/drivers/rtc/rtc-isl1208.c
@@ -6,6 +6,7 @@
  */
 
 #include <linux/bcd.h>
+#include <linux/clk.h>
 #include <linux/i2c.h>
 #include <linux/module.h>
 #include <linux/of_device.h>
@@ -175,6 +176,20 @@ isl1208_i2c_validate_client(struct i2c_client *client)
 	return 0;
 }
 
+static int isl1208_set_xtoscb(struct i2c_client *client, int sr, int xtosb_val)
+{
+	/* Do nothing if bit is already set to desired value */
+	if ((sr & ISL1208_REG_SR_XTOSCB) == xtosb_val)
+		return 0;
+
+	if (xtosb_val)
+		sr |= ISL1208_REG_SR_XTOSCB;
+	else
+		sr &= ~ISL1208_REG_SR_XTOSCB;
+
+	return i2c_smbus_write_byte_data(client, ISL1208_REG_SR, sr);
+}
+
 static int
 isl1208_i2c_get_sr(struct i2c_client *client)
 {
@@ -511,7 +526,6 @@ isl1208_i2c_set_time(struct i2c_client *client, struct rtc_time const *tm)
 	return 0;
 }
 
-
 static int
 isl1208_rtc_set_time(struct device *dev, struct rtc_time *tm)
 {
@@ -805,12 +819,26 @@ static int isl1208_setup_irq(struct i2c_client *client, int irq)
 	return rc;
 }
 
+static int
+isl1208_clk_present(struct i2c_client *client, const char *name)
+{
+	struct clk *clk;
+
+	clk = devm_clk_get_optional(&client->dev, name);
+	if (IS_ERR(clk))
+		return PTR_ERR(clk);
+
+	return !!clk;
+}
+
 static int
 isl1208_probe(struct i2c_client *client, const struct i2c_device_id *id)
 {
-	int rc = 0;
 	struct isl1208_state *isl1208;
 	int evdet_irq = -1;
+	int xtosb_val = 0;
+	int rc = 0;
+	int sr;
 
 	if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C))
 		return -ENODEV;
@@ -835,6 +863,19 @@ isl1208_probe(struct i2c_client *client, const struct i2c_device_id *id)
 		isl1208->config = (struct isl1208_config *)id->driver_data;
 	}
 
+	rc = isl1208_clk_present(client, "xin");
+	if (rc < 0)
+		return rc;
+
+	if (!rc) {
+		rc = isl1208_clk_present(client, "clkin");
+		if (rc < 0)
+			return rc;
+
+		if (rc)
+			xtosb_val = 1;
+	}
+
 	isl1208->rtc = devm_rtc_allocate_device(&client->dev);
 	if (IS_ERR(isl1208->rtc))
 		return PTR_ERR(isl1208->rtc);
@@ -846,13 +887,17 @@ isl1208_probe(struct i2c_client *client, const struct i2c_device_id *id)
 	isl1208->nvmem_config.size = isl1208->config->nvmem_length;
 	isl1208->nvmem_config.priv = isl1208;
 
-	rc = isl1208_i2c_get_sr(client);
-	if (rc < 0) {
+	sr = isl1208_i2c_get_sr(client);
+	if (sr < 0) {
 		dev_err(&client->dev, "reading status failed\n");
-		return rc;
+		return sr;
 	}
 
-	if (rc & ISL1208_REG_SR_RTCF)
+	rc = isl1208_set_xtoscb(client, sr, xtosb_val);
+	if (rc)
+		return rc;
+
+	if (sr & ISL1208_REG_SR_RTCF)
 		dev_warn(&client->dev, "rtc power failure detected, "
 			 "please set clock.\n");
 
-- 
2.25.1

[PATCH 6.1.y-cip 10/13] rtc: isl1208: Add support for the built-in RTC on the PMIC RAA215300

[Biju Das]

commit fdd63f65ac25d0851dade4c7ba94a7a882b8d9c2 upstream.

The built-in RTC found on PMIC RAA215300 is the same as ISL1208.
However, the external oscillator bit is inverted on PMIC version
0x11. The PMIC driver detects PMIC version and instantiates the
RTC device based on i2c_device_id.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Link: https://lore.kernel.org/r/20230623140948.384762-11-biju.das.jz@bp.renesas.com
Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
 drivers/rtc/rtc-isl1208.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/rtc/rtc-isl1208.c b/drivers/rtc/rtc-isl1208.c
index 915f23668228..b8cb99810ce4 100644
--- a/drivers/rtc/rtc-isl1208.c
+++ b/drivers/rtc/rtc-isl1208.c
@@ -74,6 +74,7 @@ struct isl1208_config {
 	unsigned int	nvmem_length;
 	unsigned	has_tamper:1;
 	unsigned	has_timestamp:1;
+	unsigned	has_inverted_osc_bit:1;
 };
 
 static const struct isl1208_config config_isl1208 = {
@@ -100,11 +101,19 @@ static const struct isl1208_config config_isl1219 = {
 	.has_timestamp = true
 };
 
+static const struct isl1208_config config_raa215300_a0 = {
+	.nvmem_length = 2,
+	.has_tamper = false,
+	.has_timestamp = false,
+	.has_inverted_osc_bit = true
+};
+
 static const struct i2c_device_id isl1208_id[] = {
 	{ "isl1208", .driver_data = (kernel_ulong_t)&config_isl1208 },
 	{ "isl1209", .driver_data = (kernel_ulong_t)&config_isl1209 },
 	{ "isl1218", .driver_data = (kernel_ulong_t)&config_isl1218 },
 	{ "isl1219", .driver_data = (kernel_ulong_t)&config_isl1219 },
+	{ "raa215300_a0", .driver_data = (kernel_ulong_t)&config_raa215300_a0 },
 	{ }
 };
 MODULE_DEVICE_TABLE(i2c, isl1208_id);
@@ -893,6 +902,9 @@ isl1208_probe(struct i2c_client *client, const struct i2c_device_id *id)
 		return sr;
 	}
 
+	if (isl1208->config->has_inverted_osc_bit)
+		xtosb_val = !xtosb_val;
+
 	rc = isl1208_set_xtoscb(client, sr, xtosb_val);
 	if (rc)
 		return rc;
-- 
2.25.1

[PATCH 6.1.y-cip 11/13] arm64: defconfig: Enable PMIC RAA215300 and RTC ISL 1208 configs

[Biju Das]

commit 0bfe5475f6b9fc766aa1fabb0a90c88b882c2f70 next.

Enable PMIC RAA215300 and ISL 1208 configs, as it is populated
on Renesas RZ/{G2L,V2L} SMARC EVKs.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
Link: https://lore.kernel.org/r/20230703144108.413938-1-biju.das.jz@bp.renesas.com
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
 arch/arm64/configs/defconfig | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index 3164657aa297..3e6ae07852fc 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -678,6 +678,7 @@ CONFIG_REGULATOR_PWM=y
 CONFIG_REGULATOR_QCOM_RPMH=y
 CONFIG_REGULATOR_QCOM_SMD_RPM=y
 CONFIG_REGULATOR_QCOM_SPMI=y
+CONFIG_REGULATOR_RAA215300=y
 CONFIG_REGULATOR_RK808=y
 CONFIG_REGULATOR_S2MPS11=y
 CONFIG_REGULATOR_TPS65132=m
@@ -984,6 +985,7 @@ CONFIG_RTC_DRV_DS1307=m
 CONFIG_RTC_DRV_HYM8563=m
 CONFIG_RTC_DRV_MAX77686=y
 CONFIG_RTC_DRV_RK808=m
+CONFIG_RTC_DRV_ISL1208=m
 CONFIG_RTC_DRV_PCF85063=m
 CONFIG_RTC_DRV_PCF85363=m
 CONFIG_RTC_DRV_M41T80=m
-- 
2.25.1

[PATCH 6.1.y-cip 12/13] arm64: dts: renesas: rzg2l-smarc-som: Enable PMIC and built-in RTC

[Biju Das]

commit fdf19e44e0ef07b721c5cc1b413941ddbdc4bd78 next.

Enable PMIC RAA215300 and the built-in RTC on the RZ/{G2L,V2L} SMARC
EVK.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
Link: https://lore.kernel.org/r/20230623140948.384762-4-biju.das.jz@bp.renesas.com
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 .../boot/dts/renesas/rzg2l-smarc-som.dtsi      | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/rzg2l-smarc-som.dtsi b/arch/arm64/boot/dts/renesas/rzg2l-smarc-som.dtsi
index c4faff092380..a2b6237b10ac 100644
--- a/arch/arm64/boot/dts/renesas/rzg2l-smarc-som.dtsi
+++ b/arch/arm64/boot/dts/renesas/rzg2l-smarc-som.dtsi
@@ -73,6 +73,13 @@ vccq_sdhi0: regulator-vccq-sdhi0 {
 		gpios = <&pinctrl RZG2L_GPIO(39, 0) GPIO_ACTIVE_HIGH>;
 		regulator-always-on;
 	};
+
+	/* 32.768kHz crystal */
+	x2: x2-clock {
+		compatible = "fixed-clock";
+		#clock-cells = <0>;
+		clock-frequency = <32768>;
+	};
 };
 
 &adc {
@@ -148,6 +155,17 @@ &gpu {
 	mali-supply = <&reg_1p1v>;
 };
 
+&i2c3 {
+	raa215300: pmic@12 {
+		compatible = "renesas,raa215300";
+		reg = <0x12>, <0x6f>;
+		reg-names = "main", "rtc";
+
+		clocks = <&x2>;
+		clock-names = "xin";
+	};
+};
+
 &ostm1 {
 	status = "okay";
 };
-- 
2.25.1

[PATCH 6.1.y-cip 13/13] arm64: dts: renesas: rzg2lc-smarc-som: Enable PMIC and built-in RTC

[Biju Das]

commit bf8abcd7e7a804b825b11828d13c057e8678899a next.

Enable PMIC RAA215300 and the built-in RTC on the RZ/G2LC SMARC
EVK.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
Link: https://lore.kernel.org/r/20230712151342.82690-1-biju.das.jz@bp.renesas.com
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 .../boot/dts/renesas/rzg2lc-smarc-som.dtsi     | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/rzg2lc-smarc-som.dtsi b/arch/arm64/boot/dts/renesas/rzg2lc-smarc-som.dtsi
index 78e6e2376b01..8e90cb05cac7 100644
--- a/arch/arm64/boot/dts/renesas/rzg2lc-smarc-som.dtsi
+++ b/arch/arm64/boot/dts/renesas/rzg2lc-smarc-som.dtsi
@@ -61,6 +61,13 @@ vccq_sdhi0: regulator-vccq-sdhi0 {
 		gpios = <&pinctrl RZG2L_GPIO(39, 0) GPIO_ACTIVE_HIGH>;
 		regulator-always-on;
 	};
+
+	/* 32.768kHz crystal */
+	x2: x2-clock {
+		compatible = "fixed-clock";
+		#clock-cells = <0>;
+		clock-frequency = <32768>;
+	};
 };
 
 &eth0 {
@@ -97,6 +104,17 @@ &gpu {
 	mali-supply = <&reg_1p1v>;
 };
 
+&i2c2 {
+	raa215300: pmic@12 {
+		compatible = "renesas,raa215300";
+		reg = <0x12>, <0x6f>;
+		reg-names = "main", "rtc";
+
+		clocks = <&x2>;
+		clock-names = "xin";
+	};
+};
+
 &ostm1 {
 	status = "okay";
 };
-- 
2.25.1

Re: [PATCH 6.1.y-cip 00/13] Add Renesas PMIC RAA215300 driver and builtin RTC support

[Pavel Machek]

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

Hi!

> This patch series aims to add PMIC RAA215300 driver and builtin RTC support
>  
> All the patches are cherry-picked from the mainline except the last
> 3 patches.

I reviewed the patches, and am currently running tests. I'll have some
comments on patches, but nothing that should stop the merge; I can
merge the first 10 patches if there are no other comments and if
testing passes.

Best regards,
								Pavel
-- 
DENX Software Engineering GmbH,        Managing Director: Erika Unter
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

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

Re: [PATCH 6.1.y-cip 01/13] regulator: dt-bindings: Add Renesas RAA215300 PMIC bindings

[Pavel Machek]

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

Hi!

> +++ b/Documentation/devicetree/bindings/regulator/renesas,raa215300.yaml
> @@ -0,0 +1,85 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/regulator/renesas,raa215300.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Renesas RAA215300 Power Management Integrated Circuit (PMIC)
> +
> +maintainers:
> +  - Biju Das <biju.das.jz@bp.renesas.com>
> +
> +description: |
> +  The RAA215300 is a high-performance, low-cost 9-channel PMIC designed for
> +  32-bit and 64-bit MCU and MPU applications. It supports DDR3, DDR3L, DDR4,
> +  and LPDDR4 memory power requirements. The internally compensated regulators,
> +  built-in Real-Time Clock (RTC), 32kHz crystal oscillator, and coin cell
> +  battery charger provide a highly integrated, small footprint power solution
> +  ideal for System-On-Module (SOM) applications. A spread spectrum feature
> +  provides an ease-of-use solution for noise-sensitive audio or RF applications.

"an easy-to-use"?

> +    description: |
> +      The clocks are optional. The RTC is disabled, if no clocks are
> +      provided(either xin or clkin).

"If no clocks are provided (neither xin nor clkin), the RTC is
disabled"?

Best regards,
								Pavel
-- 
DENX Software Engineering GmbH,        Managing Director: Erika Unter
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

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

Re: [PATCH 6.1.y-cip 02/13] regulator: Add Renesas PMIC RAA215300 driver

[Pavel Machek]

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

Hi!

> +++ b/drivers/regulator/Kconfig
> @@ -1016,6 +1016,13 @@ config REGULATOR_QCOM_USB_VBUS
>  	  Say M here if you want to include support for enabling the VBUS output
>  	  as a module. The module will be named "qcom_usb_vbus_regulator".
>  
> +config REGULATOR_RAA215300
> +	tristate "Renesas RAA215300 driver"
> +	select REGMAP_I2C
> +	depends on I2C
> +	help
> +	  Support for the Renesas RAA215300 PMIC.
> +


"Say M here ... the module will be named..."?

> --- /dev/null
> +++ b/drivers/regulator/raa215300.c
> @@ -0,0 +1,190 @@

> +static void raa215300_rtc_unregister_device(void *data)
> +{
> +	i2c_unregister_device(data);
> +	if (!clk) {
> +		clk_unregister_fixed_rate(clk);
> +		clk = NULL;
> +	}
> +}

This one looks suspect. Should be (clk)?

> +static int raa215300_i2c_probe(struct i2c_client *client)
> +{
> +	struct device *dev = &client->dev;
> +	const char *clk_name = xin_name;
> +	unsigned int pmic_version, val;
> +	struct regmap *regmap;
> +	int ret;
> +
> +	regmap = devm_regmap_init_i2c(client, &raa215300_regmap_config);
> +	if (IS_ERR(regmap))
> +		return dev_err_probe(dev, PTR_ERR(regmap),
> +				     "regmap i2c init failed\n");
> +
> +	ret = regmap_read(regmap, RAA215300_HW_REV, &pmic_version);
> +	if (ret < 0)
> +		return dev_err_probe(dev, ret, "HW rev read failed\n");
> +
> +	dev_dbg(dev, "RAA215300 PMIC version 0x%04x\n", pmic_version);
> +
> +	/* Clear all blocks except RTC, if enabled */
> +	regmap_read(regmap, RAA215300_REG_BLOCK_EN, &val);
> +	val &= RAA215300_REG_BLOCK_EN_RTC_EN;
> +	regmap_write(regmap, RAA215300_REG_BLOCK_EN, val);
> +
> +	/*Clear the latched registers */

"/* Clear". I would not mind checking errors from regmap here.

> +	ret = raa215300_clk_present(client, xin_name);
> +	if (ret < 0) {
> +		return ret;
> +	} else if (!ret) {
> +		ret = raa215300_clk_present(client, clkin_name);
> +		if (ret < 0)
> +			return ret;
> +

dev_err_probe would be consistent/useful for the other returns?

> +		clk = clk_register_fixed_rate(NULL, clk_name, NULL, 0, 32000);

Elsewhere, you use 32767 for a rate.

Best regards,
								Pavel
-- 
DENX Software Engineering GmbH,        Managing Director: Erika Unter
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

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

Re: [PATCH 6.1.y-cip 04/13] dt-bindings: rtc: isl1208: Convert to json-schema

[Pavel Machek]

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

Hi!

> +++ b/Documentation/devicetree/bindings/rtc/isil,isl1208.yaml
> @@ -0,0 +1,89 @@
> +  isil,ev-evienb:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    enum: [ 0, 1 ]
> +    description: |
> +      Enable or disable internal pull on EVIN pin

Add "." at end of line.

> +      Default will leave the non-volatile configuration of the pullup

"By default driver will not touch the configuration of the pullup"?

> +      as is.
> +        <0> : Enables internal pull-up on evin pin
> +        <1> : Disables internal pull-up on evin pin

Too late to change this but logic seems inverted from what
"isil,ev-evienb" suggests. Other dts-es spell xxx-pull-up.

Best regards,
								Pavel
-- 
DENX Software Engineering GmbH,        Managing Director: Erika Unter
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

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

Re: [PATCH 6.1.y-cip 07/13] rtc: isl1208: Make similar I2C and DT-based matching table

[Pavel Machek]

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

Hi!

> commit fbc06a53561c64ec6d7f9a1b3bc04597de4cbb2d upstream.
> 

> +++ b/drivers/rtc/rtc-isl1208.c
> @@ -90,10 +90,10 @@ static const struct isl1208_config {
>  };
>  
>  static const struct i2c_device_id isl1208_id[] = {
> -	{ "isl1208", TYPE_ISL1208 },
> -	{ "isl1209", TYPE_ISL1209 },
> -	{ "isl1218", TYPE_ISL1218 },
> -	{ "isl1219", TYPE_ISL1219 },
> +	{ "isl1208", .driver_data = (kernel_ulong_t)&isl1208_configs[TYPE_ISL1208] },
> +	{ "isl1209", .driver_data = (kernel_ulong_t)&isl1208_configs[TYPE_ISL1209] },
> +	{ "isl1218", .driver_data = (kernel_ulong_t)&isl1208_configs[TYPE_ISL1218] },
> +	{ "isl1219", .driver_data = (kernel_ulong_t)&isl1208_configs[TYPE_ISL1219] },

I'd expect to see "(unsigned long)" here.

Best regards,
								Pavel
-- 
DENX Software Engineering GmbH,        Managing Director: Erika Unter
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

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

Re: [PATCH 6.1.y-cip 09/13] rtc: isl1208: Add isl1208_set_xtoscb()

[Pavel Machek]

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

Hi!

> As per the HW manual, set the XTOSCB bit as follows:
> 
> If using an external clock signal, set the XTOSCB bit as 1 to
> disable the crystal oscillator.
> 
> If using an external crystal, the XTOSCB bit needs to be set at 0
> to enable the crystal oscillator.

> +++ b/drivers/rtc/rtc-isl1208.c
> @@ -6,6 +6,7 @@
> @@ -175,6 +176,20 @@ isl1208_i2c_validate_client(struct i2c_client *client)
>  	return 0;
>  }
>  
> +static int isl1208_set_xtoscb(struct i2c_client *client, int sr, int xtosb_val)
> +{
> +	/* Do nothing if bit is already set to desired value */
> +	if ((sr & ISL1208_REG_SR_XTOSCB) == xtosb_val)
> +		return 0;

XTOSCB bit is not bit 0, but xtosb_val is either 0 or 1. If it is 1,
test will never succeed. I don't think that's intended.

> +	if (xtosb_val)
> +		sr |= ISL1208_REG_SR_XTOSCB;
> +	else
> +		sr &= ~ISL1208_REG_SR_XTOSCB;
> +
> +	return i2c_smbus_write_byte_data(client, ISL1208_REG_SR, sr);

Best regards,
								Pavel
-- 
DENX Software Engineering GmbH,        Managing Director: Erika Unter
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

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

RE: [PATCH 6.1.y-cip 01/13] regulator: dt-bindings: Add Renesas RAA215300 PMIC bindings

[Biju Das]

Hi Pavel,

Thanks for the feedback.

> Subject: Re: [PATCH 6.1.y-cip 01/13] regulator: dt-bindings: Add Renesas
> RAA215300 PMIC bindings
> 
> Hi!
> 
> > +++ b/Documentation/devicetree/bindings/regulator/renesas,raa215300.ya
> > +++ ml
> > @@ -0,0 +1,85 @@
> > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause %YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/regulator/renesas,raa215300.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Renesas RAA215300 Power Management Integrated Circuit (PMIC)
> > +
> > +maintainers:
> > +  - Biju Das <biju.das.jz@bp.renesas.com>
> > +
> > +description: |
> > +  The RAA215300 is a high-performance, low-cost 9-channel PMIC
> > +designed for
> > +  32-bit and 64-bit MCU and MPU applications. It supports DDR3,
> > +DDR3L, DDR4,
> > +  and LPDDR4 memory power requirements. The internally compensated
> > +regulators,
> > +  built-in Real-Time Clock (RTC), 32kHz crystal oscillator, and coin
> > +cell
> > +  battery charger provide a highly integrated, small footprint power
> > +solution
> > +  ideal for System-On-Module (SOM) applications. A spread spectrum
> > +feature
> > +  provides an ease-of-use solution for noise-sensitive audio or RF
> applications.
> 
> "an easy-to-use"?

It is from hardware manual.

"A spread spectrum feature provides an ease-of-use solution for
noise-sensitive audio or RF applications."

I will discuss this internally, if required will ask HW manual to fix this and then will update bindings. They are going to update the HW manual soon.

> 
> > +    description: |
> > +      The clocks are optional. The RTC is disabled, if no clocks are
> > +      provided(either xin or clkin).
> 
> "If no clocks are provided (neither xin nor clkin), the RTC is disabled"?

Ok. If required, along with above change will update this as well.

Cheers,
Biju

RE: [PATCH 6.1.y-cip 04/13] dt-bindings: rtc: isl1208: Convert to json-schema

[Biju Das]

Hi Pavel Machek,

Thanks for the feedback.

> Subject: Re: [PATCH 6.1.y-cip 04/13] dt-bindings: rtc: isl1208: Convert to
> json-schema
> 
> Hi!
> 
> > +++ b/Documentation/devicetree/bindings/rtc/isil,isl1208.yaml
> > @@ -0,0 +1,89 @@
> > +  isil,ev-evienb:
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +    enum: [ 0, 1 ]
> > +    description: |
> > +      Enable or disable internal pull on EVIN pin
> 
> Add "." at end of line.

It is just cut + paste from .txt file [1]
[1] https://elixir.bootlin.com/linux/v6.1.46/source/Documentation/devicetree/bindings/rtc/isil,isl1208.txt

> 
> > +      Default will leave the non-volatile configuration of the pullup
> 
> "By default driver will not touch the configuration of the pullup"?
> 
> > +      as is.
> > +        <0> : Enables internal pull-up on evin pin
> > +        <1> : Disables internal pull-up on evin pin
> 
> Too late to change this but logic seems inverted from what "isil,ev-evienb"
> suggests. Other dts-es spell xxx-pull-up.

I agree.

Cheers,
Biju

RE: [PATCH 6.1.y-cip 07/13] rtc: isl1208: Make similar I2C and DT-based matching table

[Biju Das]

Hi Pavel Machek,

Thanks for the feedback.

> Subject: Re: [PATCH 6.1.y-cip 07/13] rtc: isl1208: Make similar I2C and DT-
> based matching table
> 
> Hi!
> 
> > commit fbc06a53561c64ec6d7f9a1b3bc04597de4cbb2d upstream.
> >
> 
> > +++ b/drivers/rtc/rtc-isl1208.c
> > @@ -90,10 +90,10 @@ static const struct isl1208_config {  };
> >
> >  static const struct i2c_device_id isl1208_id[] = {
> > -	{ "isl1208", TYPE_ISL1208 },
> > -	{ "isl1209", TYPE_ISL1209 },
> > -	{ "isl1218", TYPE_ISL1218 },
> > -	{ "isl1219", TYPE_ISL1219 },
> > +	{ "isl1208", .driver_data =
> (kernel_ulong_t)&isl1208_configs[TYPE_ISL1208] },
> > +	{ "isl1209", .driver_data =
> (kernel_ulong_t)&isl1208_configs[TYPE_ISL1209] },
> > +	{ "isl1218", .driver_data =
> (kernel_ulong_t)&isl1208_configs[TYPE_ISL1218] },
> > +	{ "isl1219", .driver_data =
> > +(kernel_ulong_t)&isl1208_configs[TYPE_ISL1219] },
> 
> I'd expect to see "(unsigned long)" here.

Because driver_data is kernel_ulong_t. See below

struct i2c_device_id {
	char name[I2C_NAME_SIZE];
	kernel_ulong_t driver_data;	/* Data private to the driver */
};

Cheers,
Biju

RE: [PATCH 6.1.y-cip 09/13] rtc: isl1208: Add isl1208_set_xtoscb()

[Biju Das]

Hi Pavel Machek,

Thanks for the feedback.

> Subject: Re: [PATCH 6.1.y-cip 09/13] rtc: isl1208: Add isl1208_set_xtoscb()
> 
> Hi!
> 
> > As per the HW manual, set the XTOSCB bit as follows:
> >
> > If using an external clock signal, set the XTOSCB bit as 1 to disable
> > the crystal oscillator.
> >
> > If using an external crystal, the XTOSCB bit needs to be set at 0 to
> > enable the crystal oscillator.
> 
> > +++ b/drivers/rtc/rtc-isl1208.c
> > @@ -6,6 +6,7 @@
> > @@ -175,6 +176,20 @@ isl1208_i2c_validate_client(struct i2c_client
> *client)
> >  	return 0;
> >  }
> >
> > +static int isl1208_set_xtoscb(struct i2c_client *client, int sr, int
> > +xtosb_val) {
> > +	/* Do nothing if bit is already set to desired value */
> > +	if ((sr & ISL1208_REG_SR_XTOSCB) == xtosb_val)
> > +		return 0;
> 
> XTOSCB bit is not bit 0, but xtosb_val is either 0 or 1. If it is 1, test
> will never succeed. I don't think that's intended.

As mentioned in the comment, check is to avoid unnecessary
writing to I2C register if it is same value.

0 & 1 == 0 -->skip
0 & 1 == 1 -->write
1 & 1 == 0 --> write
1 & 1 == 1 -->skip

Is it expected result right?

Cheers,
Biju

> 
> > +	if (xtosb_val)
> > +		sr |= ISL1208_REG_SR_XTOSCB;
> > +	else
> > +		sr &= ~ISL1208_REG_SR_XTOSCB;
> > +
> > +	return i2c_smbus_write_byte_data(client, ISL1208_REG_SR, sr);
> 
> Best regards,
> 								Pavel
> --
> DENX Software Engineering GmbH,        Managing Director: Erika Unter
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

RE: [PATCH 6.1.y-cip 02/13] regulator: Add Renesas PMIC RAA215300 driver

[Biju Das]

Hi Pavel Machek,

Thanks for the feedback.

> Subject: Re: [PATCH 6.1.y-cip 02/13] regulator: Add Renesas PMIC RAA215300
> driver
> 
> Hi!
> 
> > +++ b/drivers/regulator/Kconfig
> > @@ -1016,6 +1016,13 @@ config REGULATOR_QCOM_USB_VBUS
> >  	  Say M here if you want to include support for enabling the VBUS
> output
> >  	  as a module. The module will be named "qcom_usb_vbus_regulator".
> >
> > +config REGULATOR_RAA215300
> > +	tristate "Renesas RAA215300 driver"
> > +	select REGMAP_I2C
> > +	depends on I2C
> > +	help
> > +	  Support for the Renesas RAA215300 PMIC.
> > +
> 
> 
> "Say M here ... the module will be named..."?

Ok. Will fix this.

> 
> > --- /dev/null
> > +++ b/drivers/regulator/raa215300.c
> > @@ -0,0 +1,190 @@
> 
> > +static void raa215300_rtc_unregister_device(void *data) {
> > +	i2c_unregister_device(data);
> > +	if (!clk) {
> > +		clk_unregister_fixed_rate(clk);
> > +		clk = NULL;
> > +	}
> > +}
> 
> This one looks suspect. Should be (clk)?

This is fixed in [1]
[1] https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?h=next-20230817&id=e21ac64e669e960688e79bf5babeed63132dac8a

> 
> > +static int raa215300_i2c_probe(struct i2c_client *client) {
> > +	struct device *dev = &client->dev;
> > +	const char *clk_name = xin_name;
> > +	unsigned int pmic_version, val;
> > +	struct regmap *regmap;
> > +	int ret;
> > +
> > +	regmap = devm_regmap_init_i2c(client, &raa215300_regmap_config);
> > +	if (IS_ERR(regmap))
> > +		return dev_err_probe(dev, PTR_ERR(regmap),
> > +				     "regmap i2c init failed\n");
> > +
> > +	ret = regmap_read(regmap, RAA215300_HW_REV, &pmic_version);
> > +	if (ret < 0)
> > +		return dev_err_probe(dev, ret, "HW rev read failed\n");
> > +
> > +	dev_dbg(dev, "RAA215300 PMIC version 0x%04x\n", pmic_version);
> > +
> > +	/* Clear all blocks except RTC, if enabled */
> > +	regmap_read(regmap, RAA215300_REG_BLOCK_EN, &val);
> > +	val &= RAA215300_REG_BLOCK_EN_RTC_EN;
> > +	regmap_write(regmap, RAA215300_REG_BLOCK_EN, val);
> > +
> > +	/*Clear the latched registers */
> 
> "/* Clear". I would not mind checking errors from regmap here.

OK, will fix it. I checked most of the drivers are not doing
Checks for regmap in kernel. As you suggested will do it here.


> 
> > +	ret = raa215300_clk_present(client, xin_name);
> > +	if (ret < 0) {
> > +		return ret;
> > +	} else if (!ret) {
> > +		ret = raa215300_clk_present(client, clkin_name);
> > +		if (ret < 0)
> > +			return ret;
> > +
> 
> dev_err_probe would be consistent/useful for the other returns?

OK.

> 
> > +		clk = clk_register_fixed_rate(NULL, clk_name, NULL, 0, 32000);
> 
> Elsewhere, you use 32767 for a rate.

I need to check with hardware people, whether 32K means 32000 or 32 *1024= 32768? and then update.

Cheers,
Biju

Re: [PATCH 6.1.y-cip 07/13] rtc: isl1208: Make similar I2C and DT-based matching table

[Pavel Machek]

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

Hi!

> > > +++ b/drivers/rtc/rtc-isl1208.c
> > > @@ -90,10 +90,10 @@ static const struct isl1208_config {  };
> > >
> > >  static const struct i2c_device_id isl1208_id[] = {
> > > -	{ "isl1208", TYPE_ISL1208 },
> > > -	{ "isl1209", TYPE_ISL1209 },
> > > -	{ "isl1218", TYPE_ISL1218 },
> > > -	{ "isl1219", TYPE_ISL1219 },
> > > +	{ "isl1208", .driver_data =
> > (kernel_ulong_t)&isl1208_configs[TYPE_ISL1208] },
> > > +	{ "isl1209", .driver_data =
> > (kernel_ulong_t)&isl1208_configs[TYPE_ISL1209] },
> > > +	{ "isl1218", .driver_data =
> > (kernel_ulong_t)&isl1208_configs[TYPE_ISL1218] },
> > > +	{ "isl1219", .driver_data =
> > > +(kernel_ulong_t)&isl1208_configs[TYPE_ISL1219] },
> > 
> > I'd expect to see "(unsigned long)" here.
> 
> Because driver_data is kernel_ulong_t. See below
> 
> struct i2c_device_id {
> 	char name[I2C_NAME_SIZE];
> 	kernel_ulong_t driver_data;	/* Data private to the driver */
> };

Aha, ok, that's quite unusual. Sorry for the noise.

Best regards,
								Pavel
-- 
DENX Software Engineering GmbH,        Managing Director: Erika Unter
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

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

Re: [PATCH 6.1.y-cip 09/13] rtc: isl1208: Add isl1208_set_xtoscb()

[Pavel Machek]

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

Hi!

> > > If using an external crystal, the XTOSCB bit needs to be set at 0 to
> > > enable the crystal oscillator.
> > 
> > > +++ b/drivers/rtc/rtc-isl1208.c
> > > @@ -6,6 +6,7 @@
> > > @@ -175,6 +176,20 @@ isl1208_i2c_validate_client(struct i2c_client
> > *client)
> > >  	return 0;
> > >  }
> > >
> > > +static int isl1208_set_xtoscb(struct i2c_client *client, int sr, int
> > > +xtosb_val) {
> > > +	/* Do nothing if bit is already set to desired value */
> > > +	if ((sr & ISL1208_REG_SR_XTOSCB) == xtosb_val)
> > > +		return 0;
> > 
> > XTOSCB bit is not bit 0, but xtosb_val is either 0 or 1. If it is 1, test
> > will never succeed. I don't think that's intended.
> 
> As mentioned in the comment, check is to avoid unnecessary
> writing to I2C register if it is same value.
> 
> 0 & 1 == 0 -->skip
> 0 & 1 == 1 -->write
> 1 & 1 == 0 --> write
> 1 & 1 == 1 -->skip
> 
> Is it expected result right?

As far as I can see, you'll get

#define ISL1208_REG_SR_XTOSCB  (1<<6)

xtosb_val = 1;

  64 & 64 == 1
  64 == 1
  false --> write

when you should be skipping.

Best regards,
								Pavel
-- 
DENX Software Engineering GmbH,        Managing Director: Erika Unter
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

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

RE: [PATCH 6.1.y-cip 09/13] rtc: isl1208: Add isl1208_set_xtoscb()

[Biju Das]

Hi Pavel,

Thanks for the feedback.

> Subject: Re: [PATCH 6.1.y-cip 09/13] rtc: isl1208: Add isl1208_set_xtoscb()
> 
> Hi!
> 
> > > > If using an external crystal, the XTOSCB bit needs to be set at 0
> > > > to enable the crystal oscillator.
> > >
> > > > +++ b/drivers/rtc/rtc-isl1208.c
> > > > @@ -6,6 +6,7 @@
> > > > @@ -175,6 +176,20 @@ isl1208_i2c_validate_client(struct i2c_client
> > > *client)
> > > >  	return 0;
> > > >  }
> > > >
> > > > +static int isl1208_set_xtoscb(struct i2c_client *client, int sr,
> > > > +int
> > > > +xtosb_val) {
> > > > +	/* Do nothing if bit is already set to desired value */
> > > > +	if ((sr & ISL1208_REG_SR_XTOSCB) == xtosb_val)
> > > > +		return 0;
> > >
> > > XTOSCB bit is not bit 0, but xtosb_val is either 0 or 1. If it is 1,
> > > test will never succeed. I don't think that's intended.
> >
> > As mentioned in the comment, check is to avoid unnecessary writing to
> > I2C register if it is same value.
> >
> > 0 & 1 == 0 -->skip
> > 0 & 1 == 1 -->write
> > 1 & 1 == 0 --> write
> > 1 & 1 == 1 -->skip
> >
> > Is it expected result right?
> 
> As far as I can see, you'll get
> 
> #define ISL1208_REG_SR_XTOSCB  (1<<6)
> 
> xtosb_val = 1;
> 
>   64 & 64 == 1
>   64 == 1
>   false --> write
> 
> when you should be skipping.

You are correct.

I need to do double negation to fix this.

	/* Do nothing if bit is already set to desired value */
	if (!!(sr & ISL1208_REG_SR_XTOSCB) == xtosb_val)
		return 0;

Cheers,
Biju

Re: [PATCH 6.1.y-cip 02/13] regulator: Add Renesas PMIC RAA215300 driver

[Pavel Machek]

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

Hi!

> > > +	/*Clear the latched registers */
> > 
> > "/* Clear". I would not mind checking errors from regmap here.
> 
> OK, will fix it. I checked most of the drivers are not doing
> Checks for regmap in kernel. As you suggested will do it here.

Up to you really. I doubt regmap will fail in any real situation. I
just noticed because you do check errors on the first one.

Best regards,
								Pavel
-- 
DENX Software Engineering GmbH,        Managing Director: Erika Unter
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

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

RE: [PATCH 6.1.y-cip 02/13] regulator: Add Renesas PMIC RAA215300 driver

[Biju Das]

Hi Pavel,

Thanks for the feedback

> Subject: Re: [PATCH 6.1.y-cip 02/13] regulator: Add Renesas PMIC RAA215300
> driver
> 
> Hi!
> 
> > > > +	/*Clear the latched registers */
> > >
> > > "/* Clear". I would not mind checking errors from regmap here.
> >
> > OK, will fix it. I checked most of the drivers are not doing Checks
> > for regmap in kernel. As you suggested will do it here.
> 
> Up to you really. I doubt regmap will fail in any real situation. I just
> noticed because you do check errors on the first one.

Then I will leave like this as to get some benefit for boot time.

Cheers,
Biju

Re: [PATCH 6.1.y-cip 00/13] Add Renesas PMIC RAA215300 driver and builtin RTC support

[Pavel Machek]

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

Hi!

> This patch series aims to add PMIC RAA215300 driver and builtin RTC support
>  
> All the patches are cherry-picked from the mainline except the last
> 3 patches.
> 
> The last 3 patches are from next and just added here for testing.

Thanks for the patches, I applied the first 10 to the 6.1-cip.

Best regards,
								Pavel
-- 
DENX Software Engineering GmbH,        Managing Director: Erika Unter
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

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