[PATCH RFC 0/6] Add Renesas PMIC RAA215300 and built-in RTC support

[PATCH RFC 0/6] Add Renesas PMIC RAA215300 and built-in RTC support

[Biju Das]

This patch series aims to add support for Renesas PMIC RAA215300 and
built-in RTC found on this PMIC device.

The details of PMIC can be found here[1].

The built-in RTC is the same as ISL-1208. Enabling of the
RTC is done by the PMIC module. 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.

Also, the polarity of the external oscillator is different between PMIC
versions. So the PMIC version is shared between the PMIC driver and the
RTC driver.

Please share your thoughts on this patch series.

[1]
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

Biju Das (6):
  dt-bindings: mfd: Add Renesas RAA215300 PMIC bindings
  mfd: Add Renesas PMIC RAA215300 driver
  dt-bindings: rtc: isl1208: Convert to json-schema
  dt-bindings: rtc: isl1208: Document built-in RTC device on PMIC
    RAA215300
  rtc: isl1208: Add support for the built-in RTC on the PMIC RAA215300
  arm64: dts: renesas: rzg2l-smarc-som: Enable PMIC and built-in RTC

 .../bindings/mfd/renesas,raa215300.yaml       | 57 ++++++++++++
 .../devicetree/bindings/rtc/isil,isl1208.txt  | 38 --------
 .../devicetree/bindings/rtc/isil,isl1208.yaml | 87 ++++++++++++++++++
 .../boot/dts/renesas/rzg2l-smarc-som.dtsi     | 16 ++++
 drivers/mfd/Kconfig                           |  8 ++
 drivers/mfd/Makefile                          |  2 +
 drivers/mfd/raa215300.c                       | 91 +++++++++++++++++++
 drivers/rtc/rtc-isl1208.c                     | 50 ++++++++++
 8 files changed, 311 insertions(+), 38 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/mfd/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/mfd/raa215300.c

-- 
2.25.1

[PATCH RFC 1/6] dt-bindings: mfd: Add Renesas RAA215300 PMIC bindings

[Biju Das]

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>
---
 .../bindings/mfd/renesas,raa215300.yaml       | 57 +++++++++++++++++++
 1 file changed, 57 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mfd/renesas,raa215300.yaml

diff --git a/Documentation/devicetree/bindings/mfd/renesas,raa215300.yaml b/Documentation/devicetree/bindings/mfd/renesas,raa215300.yaml
new file mode 100644
index 000000000000..1e65f7618333
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/renesas,raa215300.yaml
@@ -0,0 +1,57 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mfd/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: 1
+
+  renesas,raa215300-rtc:
+    $ref: /schemas/types.yaml#/definitions/phandle
+    description: phandle to the built-in RTC device.
+
+required:
+  - compatible
+  - reg
+
+additionalProperties: false
+
+examples:
+  - |
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        pmic: raa215300@12 {
+            compatible = "renesas,raa215300";
+            reg = <0x12>;
+
+            renesas,raa215300-rtc = <&rtc_raa215300>;
+        };
+    };
-- 
2.25.1

[PATCH RFC 2/6] mfd: Add Renesas PMIC RAA215300 driver

[Biju Das]

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.

Add PMIC RAA215300 driver for enabling RTC and sharing PMIC version
to RTC driver.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
 drivers/mfd/Kconfig     |  8 ++++
 drivers/mfd/Makefile    |  2 +
 drivers/mfd/raa215300.c | 91 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 101 insertions(+)
 create mode 100644 drivers/mfd/raa215300.c

diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index e90463c4441c..3e2a7f033b26 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -311,6 +311,14 @@ config MFD_CS47L92
 	help
 	  Support for Cirrus Logic CS42L92, CS47L92 and CS47L93 Smart Codecs
 
+config PMIC_RAA215300
+	tristate "Renesas RAA215300 driver"
+	select REGMAP_I2C
+	depends on I2C
+	help
+	  Select this to get support for the Renesas RAA215300 PMIC and to
+	  enable the built-in RTC.
+
 config PMIC_DA903X
 	bool "Dialog Semiconductor DA9030/DA9034 PMIC Support"
 	depends on I2C=y
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index 1d2392f06f78..d9c601120bfd 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -126,6 +126,8 @@ ifeq ($(CONFIG_SA1100_ASSABET),y)
 obj-$(CONFIG_MCP_UCB1200)	+= ucb1x00-assabet.o
 endif
 
+obj-$(CONFIG_PMIC_RAA215300)	+= raa215300.o
+
 obj-$(CONFIG_PMIC_DA903X)	+= da903x.o
 
 obj-$(CONFIG_PMIC_DA9052)	+= da9052-irq.o
diff --git a/drivers/mfd/raa215300.c b/drivers/mfd/raa215300.c
new file mode 100644
index 000000000000..33f93bd9ef17
--- /dev/null
+++ b/drivers/mfd/raa215300.c
@@ -0,0 +1,91 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Renesas RAA215300 PMIC driver
+ *
+ * Copyright (C) 2023 Renesas Electronics Corporation
+ */
+
+#include <linux/err.h>
+#include <linux/i2c.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/regmap.h>
+
+#define RAA215300_REG_BLOCK_EN	0x6c
+#define RAA215300_HW_REV	0xf8
+
+#define RAA215300_REG_BLOCK_EN_RTC_EN	BIT(6)
+
+static bool raa215300_is_volatile_reg(struct device *dev, unsigned int reg)
+{
+	return true;
+}
+
+static const struct regmap_config raa215300_regmap_config = {
+	.reg_bits = 8,
+	.val_bits = 8,
+	.max_register = 0xff,
+	.volatile_reg = raa215300_is_volatile_reg,
+	.cache_type = REGCACHE_FLAT,
+};
+
+static int raa215300_i2c_probe(struct i2c_client *client)
+{
+	struct device *dev = &client->dev;
+	unsigned int *pmic_version;
+	struct device_node *np;
+	struct regmap *regmap;
+	int ret;
+
+	pmic_version = devm_kzalloc(dev, sizeof(*pmic_version), GFP_KERNEL);
+	if (!pmic_version)
+		return -ENOMEM;
+
+	if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C))
+		return -EOPNOTSUPP;
+
+	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_set_drvdata(dev, pmic_version);
+	dev_dbg(dev, "RAA215300 PMIC version 0x%04x\n", *pmic_version);
+
+	/* Enable RTC block */
+	np = of_parse_phandle(dev->of_node, "renesas,raa215300-rtc", 0);
+	if (np)
+		regmap_update_bits(regmap, RAA215300_REG_BLOCK_EN,
+				   RAA215300_REG_BLOCK_EN_RTC_EN,
+				   RAA215300_REG_BLOCK_EN_RTC_EN);
+
+	of_node_put(np);
+
+	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");
+MODULE_SOFTDEP("post: rtc_isl1208");
-- 
2.25.1

[PATCH RFC 3/6] dt-bindings: rtc: isl1208: Convert to json-schema

[Biju Das]

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>
---
 .../devicetree/bindings/rtc/isil,isl1208.txt  | 38 ----------
 .../devicetree/bindings/rtc/isil,isl1208.yaml | 74 +++++++++++++++++++
 2 files changed, 74 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..04d51887855f
--- /dev/null
+++ b/Documentation/devicetree/bindings/rtc/isil,isl1208.yaml
@@ -0,0 +1,74 @@
+# 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 ISL12{08,09,18,19} I2C RTC/Alarm chip
+
+maintainers:
+  - Biju Das <biju.das.jz@bp.renesas.com>
+  - Trent Piepho <tpiepho@impinj.com>
+
+properties:
+  compatible:
+    oneOf:
+      - enum:
+          - isil,isl1208
+          - isil,isl1209
+          - isil,isl1218
+          - isil,isl1219
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    minItems: 1
+    maxItems: 2
+
+  interrupt-names:
+    items:
+      - const: irq
+      - const: evdet
+
+  isil,ev-evienb:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    enum: [ 0, 1 ]
+    default: 0
+    description: |
+      Enable or disable internal pull on EVIN pin:
+        <0> : Enable internal pull-up
+        <1> : Disable internal pull-up
+
+required:
+  - compatible
+  - reg
+
+allOf:
+  - $ref: rtc.yaml#
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - isil,isl1209
+              - isil,isl1219
+    then:
+      required:
+        - interrupts-extended
+        - interrupt-names
+        - isil,ev-evienb
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        rtc_twi: rtc@6f {
+            compatible = "isil,isl1208";
+            reg = <0x6f>;
+        };
+    };
-- 
2.25.1

[PATCH RFC 4/6] dt-bindings: rtc: isl1208: Document built-in RTC device on PMIC RAA215300

[Biju Das]

The Built-in RTC device found on PMIC RAA215300 is similar to the isl1208
IP. However, RTC is enabled by PMIC RAA215300 and the polarity of the
external oscillator is determined by the PMIC revision.

Document renesas,raa215300-isl1208 compatible and renesas,raa215300-pmic
property to handle these differences.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
 .../devicetree/bindings/rtc/isil,isl1208.yaml       | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/Documentation/devicetree/bindings/rtc/isil,isl1208.yaml b/Documentation/devicetree/bindings/rtc/isil,isl1208.yaml
index 04d51887855f..888a832ed1db 100644
--- a/Documentation/devicetree/bindings/rtc/isil,isl1208.yaml
+++ b/Documentation/devicetree/bindings/rtc/isil,isl1208.yaml
@@ -18,6 +18,7 @@ properties:
           - isil,isl1209
           - isil,isl1218
           - isil,isl1219
+          - renesas,raa215300-isl1208
 
   reg:
     maxItems: 1
@@ -40,6 +41,10 @@ properties:
         <0> : Enable internal pull-up
         <1> : Disable internal pull-up
 
+  renesas,raa215300-pmic:
+    $ref: /schemas/types.yaml#/definitions/phandle
+    description: phandle to the pmic device with isl1208 built-in RTC.
+
 required:
   - compatible
   - reg
@@ -58,6 +63,14 @@ allOf:
         - interrupts-extended
         - interrupt-names
         - isil,ev-evienb
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: renesas,raa215300-isl1208
+    then:
+      required:
+        - renesas,raa215300-pmic
 
 unevaluatedProperties: false
 
-- 
2.25.1

[PATCH RFC 5/6] rtc: isl1208: Add support for the built-in RTC on the PMIC RAA215300

[Biju Das]

The built-in RTC found on PMIC RAA215300 is the same as ISL1208.
However, the external oscillator polarity is determined by the
PMIC version. For eg: the PMIC version has inverted polarity for
the external oscillator and the corresponding bit in RTC need to
be inverted(XTOSCB). This info needs to be shared from PMIC driver
to RTC driver, so that it can support all versions without any code
changes.

Add a new compatible renesas,raa215300-isl1208 to support RTC found
on PMIC RAA215300 and renesas,raa215300-pmic property to support
different versions.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
 drivers/rtc/rtc-isl1208.c | 50 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 50 insertions(+)

diff --git a/drivers/rtc/rtc-isl1208.c b/drivers/rtc/rtc-isl1208.c
index 73cc6aaf9b8b..f4ea19691ac1 100644
--- a/drivers/rtc/rtc-isl1208.c
+++ b/drivers/rtc/rtc-isl1208.c
@@ -74,6 +74,7 @@ enum isl1208_id {
 	TYPE_ISL1209,
 	TYPE_ISL1218,
 	TYPE_ISL1219,
+	TYPE_RAA215300_ISL1208,
 	ISL_LAST_ID
 };
 
@@ -83,11 +84,13 @@ static const struct isl1208_config {
 	unsigned int	nvmem_length;
 	unsigned	has_tamper:1;
 	unsigned	has_timestamp:1;
+	unsigned	has_pmic_parent: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_RAA215300_ISL1208] = { "isl1208", 2, false, false, true },
 };
 
 static const struct i2c_device_id isl1208_id[] = {
@@ -104,6 +107,10 @@ static const __maybe_unused struct of_device_id isl1208_of_match[] = {
 	{ .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 = "renesas,raa215300-isl1208",
+		.data = &isl1208_configs[TYPE_RAA215300_ISL1208]
+	},
 	{ }
 };
 MODULE_DEVICE_TABLE(of, isl1208_of_match);
@@ -166,6 +173,43 @@ isl1208_i2c_validate_client(struct i2c_client *client)
 	return 0;
 }
 
+static bool isl1208_is_xtosc_polarity_inverted(struct i2c_client *client)
+{
+	struct device *dev = &client->dev;
+	struct i2c_client *pmic_dev;
+	unsigned int *pmic_version;
+	struct device_node *np;
+	bool ret = false;
+
+	np = of_parse_phandle(dev->of_node, "renesas,raa215300-pmic", 0);
+	if (np)
+		pmic_dev = of_find_i2c_device_by_node(np);
+
+	of_node_put(np);
+	if (!pmic_dev)
+		return ret;
+
+	pmic_version = dev_get_drvdata(&pmic_dev->dev);
+	/* External Oscillator polarity is inverted on revision 0x12 onwards */
+	if (*pmic_version >= 0x12)
+		ret = true;
+
+	put_device(&pmic_dev->dev);
+
+	return ret;
+}
+
+static int
+isl1208_set_ext_osc_based_on_pmic_version(struct i2c_client *client, int rc)
+{
+	if (isl1208_is_xtosc_polarity_inverted(client))
+		rc &= ~ISL1208_REG_SR_XTOSCB;
+	else
+		rc |= ISL1208_REG_SR_XTOSCB;
+
+	return i2c_smbus_write_byte_data(client, ISL1208_REG_SR, rc);
+}
+
 static int
 isl1208_i2c_get_sr(struct i2c_client *client)
 {
@@ -845,6 +889,12 @@ isl1208_probe(struct i2c_client *client)
 		return rc;
 	}
 
+	if (isl1208->config->has_pmic_parent) {
+		rc = isl1208_set_ext_osc_based_on_pmic_version(client, rc);
+		if (rc)
+			return rc;
+	}
+
 	if (rc & ISL1208_REG_SR_RTCF)
 		dev_warn(&client->dev, "rtc power failure detected, "
 			 "please set clock.\n");
-- 
2.25.1

[PATCH RFC 6/6] arm64: dts: renesas: rzg2l-smarc-som: Enable PMIC and built-in RTC

[Biju Das]

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>
---
 arch/arm64/boot/dts/renesas/rzg2l-smarc-som.dtsi | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/rzg2l-smarc-som.dtsi b/arch/arm64/boot/dts/renesas/rzg2l-smarc-som.dtsi
index fbbb4f03440b..c9805d76ebb0 100644
--- a/arch/arm64/boot/dts/renesas/rzg2l-smarc-som.dtsi
+++ b/arch/arm64/boot/dts/renesas/rzg2l-smarc-som.dtsi
@@ -351,3 +351,19 @@ &wdt1 {
 	status = "okay";
 	timeout-sec = <60>;
 };
+
+&i2c3 {
+	pmic: raa215300@12 {
+		compatible = "renesas,raa215300";
+		reg = <0x12>;
+
+		renesas,raa215300-rtc = <&rtc_raa215300>;
+	};
+
+	rtc_raa215300: rtc@6f {
+		compatible = "renesas,raa215300-isl1208";
+		reg = <0x6f>;
+
+		renesas,raa215300-pmic = <&pmic>;
+	};
+};
-- 
2.25.1

RE: [PATCH RFC 3/6] dt-bindings: rtc: isl1208: Convert to json-schema

[Biju Das]

Hi all,


> -----Original Message-----
> From: Biju Das <biju.das.jz@bp.renesas.com>
> Sent: Wednesday, May 3, 2023 9:46 AM
> To: Alessandro Zummo <a.zummo@towertech.it>; Alexandre Belloni
> <alexandre.belloni@bootlin.com>; Rob Herring <robh+dt@kernel.org>; Krzysztof
> Kozlowski <krzysztof.kozlowski+dt@linaro.org>
> Cc: Biju Das <biju.das.jz@bp.renesas.com>; Trent Piepho
> <tpiepho@impinj.com>; linux-rtc@vger.kernel.org; devicetree@vger.kernel.org;
> Geert Uytterhoeven <geert+renesas@glider.be>; Fabrizio Castro
> <fabrizio.castro.jz@renesas.com>; linux-renesas-soc@vger.kernel.org
> Subject: [PATCH RFC 3/6] dt-bindings: rtc: isl1208: Convert to json-schema
> 
> 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>
> ---
>  .../devicetree/bindings/rtc/isil,isl1208.txt  | 38 ----------
> .../devicetree/bindings/rtc/isil,isl1208.yaml | 74 +++++++++++++++++++
>  2 files changed, 74 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..04d51887855f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/rtc/isil,isl1208.yaml
> @@ -0,0 +1,74 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) %YAML 1.2
> +---
> +$id:
> +https://jpn01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevice
> +tree.org%2Fschemas%2Frtc%2Fisil%2Cisl1208.yaml%23&data=05%7C01%7Cbiju.d
> +as.jz%40bp.renesas.com%7C369929b1ce554af8b10008db4bb2e184%7C53d82571da1
> +947e49cb4625a166a4a2a%7C0%7C0%7C638187003880337413%7CUnknown%7CTWFpbGZs
> +b3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%
> +7C3000%7C%7C%7C&sdata=hhWJWxRzJEgudMmnw%2Fl9DgpL0%2BaPRWfWDK2mGqztl3c%3
> +D&reserved=0
> +$schema:
> +https://jpn01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevice
> +tree.org%2Fmeta-schemas%2Fcore.yaml%23&data=05%7C01%7Cbiju.das.jz%40bp.
> +renesas.com%7C369929b1ce554af8b10008db4bb2e184%7C53d82571da1947e49cb462
> +5a166a4a2a%7C0%7C0%7C638187003880337413%7CUnknown%7CTWFpbGZsb3d8eyJWIjo
> +iMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7
> +C%7C&sdata=figbanniGklELxfV4t10SVsg4i0X%2Bozm68rxXy4pupg%3D&reserved=0
> +
> +title: Intersil ISL12{08,09,18,19} I2C RTC/Alarm chip
> +
> +maintainers:
> +  - Biju Das <biju.das.jz@bp.renesas.com>
> +  - Trent Piepho <tpiepho@impinj.com>
> +
> +properties:
> +  compatible:
> +    oneOf:
> +      - enum:
> +          - isil,isl1208
> +          - isil,isl1209
> +          - isil,isl1218
> +          - isil,isl1219
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    minItems: 1
> +    maxItems: 2
> +
> +  interrupt-names:
> +    items:
> +      - const: irq
> +      - const: evdet
> +
> +  isil,ev-evienb:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    enum: [ 0, 1 ]
> +    default: 0

Not sure, default should be removes as per [1]?

[1] https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/Documentation/devicetree/bindings/rtc/isil,isl1208.txt?h=next-20230428#n20


Cheers,
Biju

> +    description: |
> +      Enable or disable internal pull on EVIN pin:
> +        <0> : Enable internal pull-up
> +        <1> : Disable internal pull-up
> +
> +required:
> +  - compatible
> +  - reg
> +
> +allOf:
> +  - $ref: rtc.yaml#
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - isil,isl1209
> +              - isil,isl1219
> +    then:
> +      required:
> +        - interrupts-extended
> +        - interrupt-names
> +        - isil,ev-evienb
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    i2c {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        rtc_twi: rtc@6f {
> +            compatible = "isil,isl1208";
> +            reg = <0x6f>;
> +        };
> +    };
> --
> 2.25.1

Re: [PATCH RFC 3/6] dt-bindings: rtc: isl1208: Convert to json-schema

[Geert Uytterhoeven]

Hi Biju,

On Wed, May 3, 2023 at 10:46 AM Biju Das <biju.das.jz@bp.renesas.com> wrote:
> 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>

Thanks for your patch!

> --- 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..04d51887855f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/rtc/isil,isl1208.yaml
> @@ -0,0 +1,74 @@
> +# 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 ISL12{08,09,18,19} I2C RTC/Alarm chip
> +
> +maintainers:
> +  - Biju Das <biju.das.jz@bp.renesas.com>
> +  - Trent Piepho <tpiepho@impinj.com>
> +
> +properties:
> +  compatible:
> +    oneOf:
> +      - enum:
> +          - isil,isl1208
> +          - isil,isl1209
> +          - isil,isl1218
> +          - isil,isl1219
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    minItems: 1
> +    maxItems: 2
> +
> +  interrupt-names:
> +    items:
> +      - const: irq
> +      - const: evdet
> +
> +  isil,ev-evienb:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    enum: [ 0, 1 ]
> +    default: 0
> +    description: |
> +      Enable or disable internal pull on EVIN pin:
> +        <0> : Enable internal pull-up
> +        <1> : Disable internal pull-up
> +
> +required:
> +  - compatible
> +  - reg
> +
> +allOf:
> +  - $ref: rtc.yaml#
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - isil,isl1209
> +              - isil,isl1219
> +    then:
> +      required:
> +        - interrupts-extended

interrupts (-extended is handled by the tooling)

> +        - interrupt-names
> +        - isil,ev-evienb

else interrupts maxitems 1?

> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    i2c {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        rtc_twi: rtc@6f {
> +            compatible = "isil,isl1208";
> +            reg = <0x6f>;
> +        };

Is there any specific reason you changed the example from the most
complex to the most simplest case?

> +    };
> --
> 2.25.1

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH RFC 3/6] dt-bindings: rtc: isl1208: Convert to json-schema

[Geert Uytterhoeven]

CC Trent's latest address

On Wed, May 3, 2023 at 11:25 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> Hi Biju,
>
> On Wed, May 3, 2023 at 10:46 AM Biju Das <biju.das.jz@bp.renesas.com> wrote:
> > 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>
>
> Thanks for your patch!
>
> > --- 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..04d51887855f
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/rtc/isil,isl1208.yaml
> > @@ -0,0 +1,74 @@
> > +# 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 ISL12{08,09,18,19} I2C RTC/Alarm chip
> > +
> > +maintainers:
> > +  - Biju Das <biju.das.jz@bp.renesas.com>
> > +  - Trent Piepho <tpiepho@impinj.com>
> > +
> > +properties:
> > +  compatible:
> > +    oneOf:
> > +      - enum:
> > +          - isil,isl1208
> > +          - isil,isl1209
> > +          - isil,isl1218
> > +          - isil,isl1219
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  interrupts:
> > +    minItems: 1
> > +    maxItems: 2
> > +
> > +  interrupt-names:
> > +    items:
> > +      - const: irq
> > +      - const: evdet
> > +
> > +  isil,ev-evienb:
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +    enum: [ 0, 1 ]
> > +    default: 0
> > +    description: |
> > +      Enable or disable internal pull on EVIN pin:
> > +        <0> : Enable internal pull-up
> > +        <1> : Disable internal pull-up
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +
> > +allOf:
> > +  - $ref: rtc.yaml#
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          contains:
> > +            enum:
> > +              - isil,isl1209
> > +              - isil,isl1219
> > +    then:
> > +      required:
> > +        - interrupts-extended
>
> interrupts (-extended is handled by the tooling)
>
> > +        - interrupt-names
> > +        - isil,ev-evienb
>
> else interrupts maxitems 1?
>
> > +
> > +unevaluatedProperties: false
> > +
> > +examples:
> > +  - |
> > +    i2c {
> > +        #address-cells = <1>;
> > +        #size-cells = <0>;
> > +
> > +        rtc_twi: rtc@6f {
> > +            compatible = "isil,isl1208";
> > +            reg = <0x6f>;
> > +        };
>
> Is there any specific reason you changed the example from the most
> complex to the most simplest case?
>
> > +    };
> > --
> > 2.25.1
>
> Gr{oetje,eeting}s,
>
>                         Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds

Re: [PATCH RFC 4/6] dt-bindings: rtc: isl1208: Document built-in RTC device on PMIC RAA215300

[Geert Uytterhoeven]

Hi Biju,

CC Trent's latest address

On Wed, May 3, 2023 at 10:46 AM Biju Das <biju.das.jz@bp.renesas.com> wrote:
> The Built-in RTC device found on PMIC RAA215300 is similar to the isl1208
> IP. However, RTC is enabled by PMIC RAA215300 and the polarity of the
> external oscillator is determined by the PMIC revision.
>
> Document renesas,raa215300-isl1208 compatible and renesas,raa215300-pmic
> property to handle these differences.
>
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> ---
>  .../devicetree/bindings/rtc/isil,isl1208.yaml       | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/rtc/isil,isl1208.yaml b/Documentation/devicetree/bindings/rtc/isil,isl1208.yaml
> index 04d51887855f..888a832ed1db 100644
> --- a/Documentation/devicetree/bindings/rtc/isil,isl1208.yaml
> +++ b/Documentation/devicetree/bindings/rtc/isil,isl1208.yaml
> @@ -18,6 +18,7 @@ properties:
>            - isil,isl1209
>            - isil,isl1218
>            - isil,isl1219
> +          - renesas,raa215300-isl1208

That sounds a bit over-complicated.
What about just "renesas,raa215300-rtc"?
If you consider them sufficiently compatible, you could add
"isil,isl1208" as a fallback.

>
>    reg:
>      maxItems: 1
> @@ -40,6 +41,10 @@ properties:
>          <0> : Enable internal pull-up
>          <1> : Disable internal pull-up
>
> +  renesas,raa215300-pmic:

"renesas,pmic"?

> +    $ref: /schemas/types.yaml#/definitions/phandle
> +    description: phandle to the pmic device with isl1208 built-in RTC.
> +
>  required:
>    - compatible
>    - reg
> @@ -58,6 +63,14 @@ allOf:
>          - interrupts-extended
>          - interrupt-names
>          - isil,ev-evienb
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: renesas,raa215300-isl1208
> +    then:
> +      required:
> +        - renesas,raa215300-pmic
>
>  unevaluatedProperties: false

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH RFC 1/6] dt-bindings: mfd: Add Renesas RAA215300 PMIC bindings

[Geert Uytterhoeven]

Hi Biju,

On Wed, May 3, 2023 at 10:46 AM Biju Das <biju.das.jz@bp.renesas.com> wrote:
> 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>

Thanks for your patch!

> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/renesas,raa215300.yaml
> @@ -0,0 +1,57 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/mfd/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

renesas,raa215300-pmic?

> +
> +  reg:
> +    maxItems: 1
> +
> +  renesas,raa215300-rtc:

renesas,rtc?


> +    $ref: /schemas/types.yaml#/definitions/phandle
> +    description: phandle to the built-in RTC device.
> +
> +required:
> +  - compatible
> +  - reg
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    i2c {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        pmic: raa215300@12 {
> +            compatible = "renesas,raa215300";
> +            reg = <0x12>;
> +
> +            renesas,raa215300-rtc = <&rtc_raa215300>;
> +        };
> +    };
> --
> 2.25.1
>


--
Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

RE: [PATCH RFC 3/6] dt-bindings: rtc: isl1208: Convert to json-schema

[Biju Das]

Hi Geert,

Thanks for the feedback.

> Subject: Re: [PATCH RFC 3/6] dt-bindings: rtc: isl1208: Convert to json-
> schema
> 
> Hi Biju,
> 
> On Wed, May 3, 2023 at 10:46 AM Biju Das <biju.das.jz@bp.renesas.com> wrote:
> > 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>
> 
> Thanks for your patch!
> 
> > --- 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..04d51887855f
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/rtc/isil,isl1208.yaml
> > @@ -0,0 +1,74 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) %YAML 1.2
> > +---
> > +$id:
> > +https://jpn01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevi
> > +cetree.org%2Fschemas%2Frtc%2Fisil%2Cisl1208.yaml%23&data=05%7C01%7Cbi
> > +ju.das.jz%40bp.renesas.com%7C321f62730bc24f076af608db4bb861fa%7C53d82
> > +571da1947e49cb4625a166a4a2a%7C0%7C0%7C638187027511491836%7CUnknown%7C
> > +TWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXV
> > +CI6Mn0%3D%7C3000%7C%7C%7C&sdata=wzxyh4VtGR8ZHpirUIX04Zo7aaj6jaScAvD0U
> > +8sfoXg%3D&reserved=0
> > +$schema:
> > +https://jpn01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevi
> > +cetree.org%2Fmeta-schemas%2Fcore.yaml%23&data=05%7C01%7Cbiju.das.jz%4
> > +0bp.renesas.com%7C321f62730bc24f076af608db4bb861fa%7C53d82571da1947e4
> > +9cb4625a166a4a2a%7C0%7C0%7C638187027511491836%7CUnknown%7CTWFpbGZsb3d
> > +8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7
> > +C3000%7C%7C%7C&sdata=4wWCv23cLyO%2BSDa4Ng4ptuciHZ%2BNo%2FozoRLKIbUtnM
> > +A%3D&reserved=0
> > +
> > +title: Intersil ISL12{08,09,18,19} I2C RTC/Alarm chip
> > +
> > +maintainers:
> > +  - Biju Das <biju.das.jz@bp.renesas.com>
> > +  - Trent Piepho <tpiepho@impinj.com>
> > +
> > +properties:
> > +  compatible:
> > +    oneOf:
> > +      - enum:
> > +          - isil,isl1208
> > +          - isil,isl1209
> > +          - isil,isl1218
> > +          - isil,isl1219
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  interrupts:
> > +    minItems: 1
> > +    maxItems: 2
> > +
> > +  interrupt-names:
> > +    items:
> > +      - const: irq
> > +      - const: evdet
> > +
> > +  isil,ev-evienb:
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +    enum: [ 0, 1 ]
> > +    default: 0
> > +    description: |
> > +      Enable or disable internal pull on EVIN pin:
> > +        <0> : Enable internal pull-up
> > +        <1> : Disable internal pull-up
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +
> > +allOf:
> > +  - $ref: rtc.yaml#
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          contains:
> > +            enum:
> > +              - isil,isl1209
> > +              - isil,isl1219
> > +    then:
> > +      required:
> > +        - interrupts-extended
> 
> interrupts (-extended is handled by the tooling)

I got binding check warning for the example, where I used example with complex case
with interrupts-extended property.

> 
> > +        - interrupt-names
> > +        - isil,ev-evienb
> 
> else interrupts maxitems 1?

OK I will add it in next version. Most of the dts doesn't define interrupts. Since it is optional
It is Ok.

> 
> > +
> > +unevaluatedProperties: false
> > +
> > +examples:
> > +  - |
> > +    i2c {
> > +        #address-cells = <1>;
> > +        #size-cells = <0>;
> > +
> > +        rtc_twi: rtc@6f {
> > +            compatible = "isil,isl1208";
> > +            reg = <0x6f>;
> > +        };
> 
> Is there any specific reason you changed the example from the most complex
> to the most simplest case?

I did not find actual dts with complex use case in the kernel tree.

Cheers,
Biju

> 
> > +    };
> > --
> > 2.25.1
> 
> Gr{oetje,eeting}s,
> 
>                         Geert
> 
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-
> m68k.org
> 
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like
> that.
>                                 -- Linus Torvalds

RE: [PATCH RFC 4/6] dt-bindings: rtc: isl1208: Document built-in RTC device on PMIC RAA215300

[Biju Das]

Hi Geert,

Thanks for the feedback.

> Subject: Re: [PATCH RFC 4/6] dt-bindings: rtc: isl1208: Document built-in
> RTC device on PMIC RAA215300
> 
> Hi Biju,
> 
> CC Trent's latest address

Thanks, I will fix this in bindings.

> 
> On Wed, May 3, 2023 at 10:46 AM Biju Das <biju.das.jz@bp.renesas.com> wrote:
> > The Built-in RTC device found on PMIC RAA215300 is similar to the
> > isl1208 IP. However, RTC is enabled by PMIC RAA215300 and the polarity
> > of the external oscillator is determined by the PMIC revision.
> >
> > Document renesas,raa215300-isl1208 compatible and
> > renesas,raa215300-pmic property to handle these differences.
> >
> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > ---
> >  .../devicetree/bindings/rtc/isil,isl1208.yaml       | 13 +++++++++++++
> >  1 file changed, 13 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/rtc/isil,isl1208.yaml
> > b/Documentation/devicetree/bindings/rtc/isil,isl1208.yaml
> > index 04d51887855f..888a832ed1db 100644
> > --- a/Documentation/devicetree/bindings/rtc/isil,isl1208.yaml
> > +++ b/Documentation/devicetree/bindings/rtc/isil,isl1208.yaml
> > @@ -18,6 +18,7 @@ properties:
> >            - isil,isl1209
> >            - isil,isl1218
> >            - isil,isl1219
> > +          - renesas,raa215300-isl1208
> 
> That sounds a bit over-complicated.
> What about just "renesas,raa215300-rtc"?

OK, good to me.

> If you consider them sufficiently compatible, you could add "isil,isl1208"
> as a fallback.

The pmic has to enable RTC block to make it functional.
The registers and functionality are compatible.
But the configuration of Oscillator polarity is different on PMIC version.
So we need to handle it here. 

You mean like below?

+      - items:
+          - enum:
+              - renesas, raa215300-rtc
+          - const: isil,isl1208

> 
> >
> >    reg:
> >      maxItems: 1
> > @@ -40,6 +41,10 @@ properties:
> >          <0> : Enable internal pull-up
> >          <1> : Disable internal pull-up
> >
> > +  renesas,raa215300-pmic:
> 
> "renesas,pmic"?

OK. Agreed.

Cheers,
Biju

> 
> > +    $ref: /schemas/types.yaml#/definitions/phandle
> > +    description: phandle to the pmic device with isl1208 built-in RTC.
> > +
> >  required:
> >    - compatible
> >    - reg
> > @@ -58,6 +63,14 @@ allOf:
> >          - interrupts-extended
> >          - interrupt-names
> >          - isil,ev-evienb
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          contains:
> > +            const: renesas,raa215300-isl1208
> > +    then:
> > +      required:
> > +        - renesas,raa215300-pmic
> >
> >  unevaluatedProperties: false
> 
> Gr{oetje,eeting}s,
> 
>                         Geert
> 
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-
> m68k.org
> 
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like
> that.
>                                 -- Linus Torvalds

RE: [PATCH RFC 1/6] dt-bindings: mfd: Add Renesas RAA215300 PMIC bindings

[Biju Das]

Hi Geert,

Thanks for the feedback.

> Subject: Re: [PATCH RFC 1/6] dt-bindings: mfd: Add Renesas RAA215300 PMIC
> bindings
> 
> Hi Biju,
> 
> On Wed, May 3, 2023 at 10:46 AM Biju Das <biju.das.jz@bp.renesas.com> wrote:
> > 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>
> 
> Thanks for your patch!
> 
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/mfd/renesas,raa215300.yaml
> > @@ -0,0 +1,57 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) %YAML 1.2
> > +---
> > +$id:
> > +https://jpn01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevi
> > +cetree.org%2Fschemas%2Fmfd%2Frenesas%2Craa215300.yaml%23&data=05%7C01
> > +%7Cbiju.das.jz%40bp.renesas.com%7C37302c6f37b048ae260f08db4bba1f2f%7C
> > +53d82571da1947e49cb4625a166a4a2a%7C0%7C0%7C638187034978703511%7CUnkno
> > +wn%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwi
> > +LCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=qnhJGzPolFSsY1fN2p8BJTw%2FCZunFI
> > +%2BWgZne6CXS0T4%3D&reserved=0
> > +$schema:
> > +https://jpn01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevi
> > +cetree.org%2Fmeta-schemas%2Fcore.yaml%23&data=05%7C01%7Cbiju.das.jz%4
> > +0bp.renesas.com%7C37302c6f37b048ae260f08db4bba1f2f%7C53d82571da1947e4
> > +9cb4625a166a4a2a%7C0%7C0%7C638187034978703511%7CUnknown%7CTWFpbGZsb3d
> > +8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7
> > +C3000%7C%7C%7C&sdata=86%2FKxrWS6oJZVpmTyYmKqJmRuBTYWqmqSlwMvtS16pc%3D
> > +&reserved=0
> > +
> > +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-me
> > + mory-built-charger-and-rtc
> > +
> > +properties:
> > +  compatible:
> > +    enum:
> > +      - renesas,raa215300
> 
> renesas,raa215300-pmic?

OK.

> 
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  renesas,raa215300-rtc:
> 
> renesas,rtc?

OK. Will wait for others comments.

Cheers,
Biju

> 
> 
> > +    $ref: /schemas/types.yaml#/definitions/phandle
> > +    description: phandle to the built-in RTC device.
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    i2c {
> > +        #address-cells = <1>;
> > +        #size-cells = <0>;
> > +
> > +        pmic: raa215300@12 {
> > +            compatible = "renesas,raa215300";
> > +            reg = <0x12>;
> > +
> > +            renesas,raa215300-rtc = <&rtc_raa215300>;
> > +        };
> > +    };
> > --
> > 2.25.1
> >

RE: [PATCH RFC 3/6] dt-bindings: rtc: isl1208: Convert to json-schema

[Biju Das]

Hi All,

> Subject: RE: [PATCH RFC 3/6] dt-bindings: rtc: isl1208: Convert to json-
> schema
> 
> 
> 
> 
> > -----Original Message-----
> > From: Biju Das <biju.das.jz@bp.renesas.com>
> > Sent: Wednesday, May 3, 2023 9:46 AM
> > To: Alessandro Zummo <a.zummo@towertech.it>; Alexandre Belloni
> > <alexandre.belloni@bootlin.com>; Rob Herring <robh+dt@kernel.org>;
> > Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
> > Cc: Biju Das <biju.das.jz@bp.renesas.com>; Trent Piepho
> > <tpiepho@impinj.com>; linux-rtc@vger.kernel.org;
> > devicetree@vger.kernel.org; Geert Uytterhoeven
> > <geert+renesas@glider.be>; Fabrizio Castro
> > <fabrizio.castro.jz@renesas.com>; linux-renesas-soc@vger.kernel.org
> > Subject: [PATCH RFC 3/6] dt-bindings: rtc: isl1208: Convert to
> > json-schema
> >
> > 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>
> > ---
> >  .../devicetree/bindings/rtc/isil,isl1208.txt  | 38 ----------
> > .../devicetree/bindings/rtc/isil,isl1208.yaml | 74 +++++++++++++++++++
> >  2 files changed, 74 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..04d51887855f
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/rtc/isil,isl1208.yaml
> > @@ -0,0 +1,74 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) %YAML 1.2
> > +---
> > +$id:
> > +https://jpn01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevi
> > +ce
> > +tree.org%2Fschemas%2Frtc%2Fisil%2Cisl1208.yaml%23&data=05%7C01%7Cbiju
> > +.d
> > +as.jz%40bp.renesas.com%7C369929b1ce554af8b10008db4bb2e184%7C53d82571d
> > +a1
> > +947e49cb4625a166a4a2a%7C0%7C0%7C638187003880337413%7CUnknown%7CTWFpbG
> > +Zs
> > +b3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3
> > +D%
> > +7C3000%7C%7C%7C&sdata=hhWJWxRzJEgudMmnw%2Fl9DgpL0%2BaPRWfWDK2mGqztl3c
> > +%3
> > +D&reserved=0
> > +$schema:
> > +https://jpn01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevi
> > +ce
> > +tree.org%2Fmeta-schemas%2Fcore.yaml%23&data=05%7C01%7Cbiju.das.jz%40bp.
> > +renesas.com%7C369929b1ce554af8b10008db4bb2e184%7C53d82571da1947e49cb4
> > +62
> > +5a166a4a2a%7C0%7C0%7C638187003880337413%7CUnknown%7CTWFpbGZsb3d8eyJWI
> > +jo
> > +iMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C
> > +%7
> > +C%7C&sdata=figbanniGklELxfV4t10SVsg4i0X%2Bozm68rxXy4pupg%3D&reserved=
> > +0
> > +
> > +title: Intersil ISL12{08,09,18,19} I2C RTC/Alarm chip
> > +
> > +maintainers:
> > +  - Biju Das <biju.das.jz@bp.renesas.com>
> > +  - Trent Piepho <tpiepho@impinj.com>
> > +
> > +properties:
> > +  compatible:
> > +    oneOf:
> > +      - enum:
> > +          - isil,isl1208
> > +          - isil,isl1209
> > +          - isil,isl1218
> > +          - isil,isl1219
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  interrupts:
> > +    minItems: 1
> > +    maxItems: 2
> > +
> > +  interrupt-names:
> > +    items:
> > +      - const: irq
> > +      - const: evdet
> > +
> > +  isil,ev-evienb:
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +    enum: [ 0, 1 ]
> > +    default: 0
> 
> Not sure, default should be removed as per [1]?
> 
> [1] https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-
> next.git/tree/Documentation/devicetree/bindings/rtc/isil,isl1208.txt?h=next-
> 20230428#n20

Or

as per HW data sheet[1], the reset value is 0, so we should keep the default value.

[2] https://www.renesas.com/us/en/document/dst/isl1219-datasheet


Cheers,
Biju

> 
> > +    description: |
> > +      Enable or disable internal pull on EVIN pin:
> > +        <0> : Enable internal pull-up
> > +        <1> : Disable internal pull-up
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +
> > +allOf:
> > +  - $ref: rtc.yaml#
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          contains:
> > +            enum:
> > +              - isil,isl1209
> > +              - isil,isl1219
> > +    then:
> > +      required:
> > +        - interrupts-extended
> > +        - interrupt-names
> > +        - isil,ev-evienb
> > +
> > +unevaluatedProperties: false
> > +
> > +examples:
> > +  - |
> > +    i2c {
> > +        #address-cells = <1>;
> > +        #size-cells = <0>;
> > +
> > +        rtc_twi: rtc@6f {
> > +            compatible = "isil,isl1208";
> > +            reg = <0x6f>;
> > +        };
> > +    };
> > --
> > 2.25.1

Re: [PATCH RFC 5/6] rtc: isl1208: Add support for the built-in RTC on the PMIC RAA215300

[Alexandre Belloni]

On 03/05/2023 09:46:07+0100, Biju Das wrote:
> The built-in RTC found on PMIC RAA215300 is the same as ISL1208.
> However, the external oscillator polarity is determined by the
> PMIC version. For eg: the PMIC version has inverted polarity for
> the external oscillator and the corresponding bit in RTC need to
> be inverted(XTOSCB). This info needs to be shared from PMIC driver
> to RTC driver, so that it can support all versions without any code
> changes.
> 
> Add a new compatible renesas,raa215300-isl1208 to support RTC found
> on PMIC RAA215300 and renesas,raa215300-pmic property to support
> different versions.
> 
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> ---
>  drivers/rtc/rtc-isl1208.c | 50 +++++++++++++++++++++++++++++++++++++++
>  1 file changed, 50 insertions(+)
> 
> diff --git a/drivers/rtc/rtc-isl1208.c b/drivers/rtc/rtc-isl1208.c
> index 73cc6aaf9b8b..f4ea19691ac1 100644
> --- a/drivers/rtc/rtc-isl1208.c
> +++ b/drivers/rtc/rtc-isl1208.c
> @@ -74,6 +74,7 @@ enum isl1208_id {
>  	TYPE_ISL1209,
>  	TYPE_ISL1218,
>  	TYPE_ISL1219,
> +	TYPE_RAA215300_ISL1208,
>  	ISL_LAST_ID
>  };
>  
> @@ -83,11 +84,13 @@ static const struct isl1208_config {
>  	unsigned int	nvmem_length;
>  	unsigned	has_tamper:1;
>  	unsigned	has_timestamp:1;
> +	unsigned	has_pmic_parent: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_RAA215300_ISL1208] = { "isl1208", 2, false, false, true },
>  };
>  
>  static const struct i2c_device_id isl1208_id[] = {
> @@ -104,6 +107,10 @@ static const __maybe_unused struct of_device_id isl1208_of_match[] = {
>  	{ .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 = "renesas,raa215300-isl1208",
> +		.data = &isl1208_configs[TYPE_RAA215300_ISL1208]
> +	},
>  	{ }
>  };
>  MODULE_DEVICE_TABLE(of, isl1208_of_match);
> @@ -166,6 +173,43 @@ isl1208_i2c_validate_client(struct i2c_client *client)
>  	return 0;
>  }
>  
> +static bool isl1208_is_xtosc_polarity_inverted(struct i2c_client *client)

I'd remove polarity from the name of this function

> +{
> +	struct device *dev = &client->dev;
> +	struct i2c_client *pmic_dev;
> +	unsigned int *pmic_version;
> +	struct device_node *np;
> +	bool ret = false;
> +
> +	np = of_parse_phandle(dev->of_node, "renesas,raa215300-pmic", 0);
> +	if (np)
> +		pmic_dev = of_find_i2c_device_by_node(np);
> +
> +	of_node_put(np);
> +	if (!pmic_dev)
> +		return ret;
> +
> +	pmic_version = dev_get_drvdata(&pmic_dev->dev);
> +	/* External Oscillator polarity is inverted on revision 0x12 onwards */

s/polarity/bit/

My understanding is that the bit meaning is inverted. It is still a
on/off bit.

> +	if (*pmic_version >= 0x12)
> +		ret = true;
> +
> +	put_device(&pmic_dev->dev);
> +
> +	return ret;
> +}
> +
> +static int
> +isl1208_set_ext_osc_based_on_pmic_version(struct i2c_client *client, int rc)
> +{
> +	if (isl1208_is_xtosc_polarity_inverted(client))
> +		rc &= ~ISL1208_REG_SR_XTOSCB;
> +	else
> +		rc |= ISL1208_REG_SR_XTOSCB;
> +
> +	return i2c_smbus_write_byte_data(client, ISL1208_REG_SR, rc);
> +}
> +
>  static int
>  isl1208_i2c_get_sr(struct i2c_client *client)
>  {
> @@ -845,6 +889,12 @@ isl1208_probe(struct i2c_client *client)
>  		return rc;
>  	}
>  
> +	if (isl1208->config->has_pmic_parent) {
> +		rc = isl1208_set_ext_osc_based_on_pmic_version(client, rc);
> +		if (rc)
> +			return rc;
> +	}
> +
>  	if (rc & ISL1208_REG_SR_RTCF)
>  		dev_warn(&client->dev, "rtc power failure detected, "
>  			 "please set clock.\n");
> -- 
> 2.25.1
> 

-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

RE: [PATCH RFC 5/6] rtc: isl1208: Add support for the built-in RTC on the PMIC RAA215300

[Biju Das]

Hi Alexandre Belloni,

Thanks for the feedback.

> Subject: Re: [PATCH RFC 5/6] rtc: isl1208: Add support for the built-in RTC
> on the PMIC RAA215300
>
> On 03/05/2023 09:46:07+0100, Biju Das wrote:
> > The built-in RTC found on PMIC RAA215300 is the same as ISL1208.
> > However, the external oscillator polarity is determined by the PMIC
> > version. For eg: the PMIC version has inverted polarity for the
> > external oscillator and the corresponding bit in RTC need to be
> > inverted(XTOSCB). This info needs to be shared from PMIC driver to RTC
> > driver, so that it can support all versions without any code changes.
> >
> > Add a new compatible renesas,raa215300-isl1208 to support RTC found on
> > PMIC RAA215300 and renesas,raa215300-pmic property to support
> > different versions.
> >
> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > ---
> >  drivers/rtc/rtc-isl1208.c | 50
> > +++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 50 insertions(+)
> >
> > diff --git a/drivers/rtc/rtc-isl1208.c b/drivers/rtc/rtc-isl1208.c
> > index 73cc6aaf9b8b..f4ea19691ac1 100644
> > --- a/drivers/rtc/rtc-isl1208.c
> > +++ b/drivers/rtc/rtc-isl1208.c
> > @@ -74,6 +74,7 @@ enum isl1208_id {
> >     TYPE_ISL1209,
> >     TYPE_ISL1218,
> >     TYPE_ISL1219,
> > +   TYPE_RAA215300_ISL1208,
> >     ISL_LAST_ID
> >  };
> >
> > @@ -83,11 +84,13 @@ static const struct isl1208_config {
> >     unsigned int    nvmem_length;
> >     unsigned        has_tamper:1;
> >     unsigned        has_timestamp:1;
> > +   unsigned        has_pmic_parent: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_RAA215300_ISL1208] = { "isl1208", 2, false, false, true },
> >  };
> >
> >  static const struct i2c_device_id isl1208_id[] = { @@ -104,6 +107,10
> > @@ static const __maybe_unused struct of_device_id isl1208_of_match[] = {
> >     { .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 = "renesas,raa215300-isl1208",
> > +           .data = &isl1208_configs[TYPE_RAA215300_ISL1208]
> > +   },
> >     { }
> >  };
> >  MODULE_DEVICE_TABLE(of, isl1208_of_match); @@ -166,6 +173,43 @@
> > isl1208_i2c_validate_client(struct i2c_client *client)
> >     return 0;
> >  }
> >
> > +static bool isl1208_is_xtosc_polarity_inverted(struct i2c_client
> > +*client)
>
> I'd remove polarity from the name of this function

Agreed.

>
> > +{
> > +   struct device *dev = &client->dev;
> > +   struct i2c_client *pmic_dev;
> > +   unsigned int *pmic_version;
> > +   struct device_node *np;
> > +   bool ret = false;
> > +
> > +   np = of_parse_phandle(dev->of_node, "renesas,raa215300-pmic", 0);
> > +   if (np)
> > +           pmic_dev = of_find_i2c_device_by_node(np);
> > +
> > +   of_node_put(np);
> > +   if (!pmic_dev)
> > +           return ret;
> > +
> > +   pmic_version = dev_get_drvdata(&pmic_dev->dev);
> > +   /* External Oscillator polarity is inverted on revision 0x12 onwards
> > +*/
>
> s/polarity/bit/
>
> My understanding is that the bit meaning is inverted. It is still a on/off
> bit.

Yes, that is correct bit is inverted.


PMIC version 0x11
-----------------
bit 0: Disable internal crystal oscillator
    1: Enable internal crystal oscillator

PMIC version 0x12
----------------
bit 0: Enable internal crystal oscillator
    1: Disable internal crystal oscillator

Cheers,
Biju

>
> > +   if (*pmic_version >= 0x12)
> > +           ret = true;
> > +
> > +   put_device(&pmic_dev->dev);
> > +
> > +   return ret;
> > +}
> > +
> > +static int
> > +isl1208_set_ext_osc_based_on_pmic_version(struct i2c_client *client,
> > +int rc) {
> > +   if (isl1208_is_xtosc_polarity_inverted(client))
> > +           rc &= ~ISL1208_REG_SR_XTOSCB;
> > +   else
> > +           rc |= ISL1208_REG_SR_XTOSCB;
> > +
> > +   return i2c_smbus_write_byte_data(client, ISL1208_REG_SR, rc); }
> > +
> >  static int
> >  isl1208_i2c_get_sr(struct i2c_client *client)  { @@ -845,6 +889,12 @@
> > isl1208_probe(struct i2c_client *client)
> >             return rc;
> >     }
> >
> > +   if (isl1208->config->has_pmic_parent) {
> > +           rc = isl1208_set_ext_osc_based_on_pmic_version(client, rc);
> > +           if (rc)
> > +                   return rc;
> > +   }
> > +
> >     if (rc & ISL1208_REG_SR_RTCF)
> >             dev_warn(&client->dev, "rtc power failure detected, "
> >                      "please set clock.\n");
> > --
> > 2.25.1
> >
>
> --
> Alexandre Belloni, co-owner and COO, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.co/
> m%2F&data=05%7C01%7Cbiju.das.jz%40bp.renesas.com%7C68856b4bc4f14d42055608db4
> bc51ccd%7C53d82571da1947e49cb4625a166a4a2a%7C0%7C0%7C638187082188797517%7CUn
> known%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJX
> VCI6Mn0%3D%7C3000%7C%7C%7C&sdata=mzGMSc1wg7XjQ97oMyCzQZ6UEHbuViyvOErFefp8pF0
> %3D&reserved=0

Re: [PATCH RFC 4/6] dt-bindings: rtc: isl1208: Document built-in RTC device on PMIC RAA215300

[Geert Uytterhoeven]

Hi Biju,

On Wed, May 3, 2023 at 12:08 PM Biju Das <biju.das.jz@bp.renesas.com> wrote:
> > Subject: Re: [PATCH RFC 4/6] dt-bindings: rtc: isl1208: Document built-in
> > RTC device on PMIC RAA215300
> > On Wed, May 3, 2023 at 10:46 AM Biju Das <biju.das.jz@bp.renesas.com> wrote:
> > > The Built-in RTC device found on PMIC RAA215300 is similar to the
> > > isl1208 IP. However, RTC is enabled by PMIC RAA215300 and the polarity
> > > of the external oscillator is determined by the PMIC revision.
> > >
> > > Document renesas,raa215300-isl1208 compatible and
> > > renesas,raa215300-pmic property to handle these differences.
> > >
> > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > > ---
> > >  .../devicetree/bindings/rtc/isil,isl1208.yaml       | 13 +++++++++++++
> > >  1 file changed, 13 insertions(+)
> > >
> > > diff --git a/Documentation/devicetree/bindings/rtc/isil,isl1208.yaml
> > > b/Documentation/devicetree/bindings/rtc/isil,isl1208.yaml
> > > index 04d51887855f..888a832ed1db 100644
> > > --- a/Documentation/devicetree/bindings/rtc/isil,isl1208.yaml
> > > +++ b/Documentation/devicetree/bindings/rtc/isil,isl1208.yaml
> > > @@ -18,6 +18,7 @@ properties:
> > >            - isil,isl1209
> > >            - isil,isl1218
> > >            - isil,isl1219
> > > +          - renesas,raa215300-isl1208
> >
> > That sounds a bit over-complicated.
> > What about just "renesas,raa215300-rtc"?
>
> OK, good to me.
>
> > If you consider them sufficiently compatible, you could add "isil,isl1208"
> > as a fallback.
>
> The pmic has to enable RTC block to make it functional.
> The registers and functionality are compatible.
> But the configuration of Oscillator polarity is different on PMIC version.
> So we need to handle it here.
>
> You mean like below?
>
> +      - items:
> +          - enum:
> +              - renesas, raa215300-rtc
> +          - const: isil,isl1208

That's indeed what I meant.  But given the inverted osc bit, I think the
fallback is not appropriate.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH RFC 1/6] dt-bindings: mfd: Add Renesas RAA215300 PMIC bindings

[Krzysztof Kozlowski]

On 03/05/2023 12:20, Biju Das wrote:
> Hi Geert,
> 
> Thanks for the feedback.
> 
>> Subject: Re: [PATCH RFC 1/6] dt-bindings: mfd: Add Renesas RAA215300 PMIC
>> bindings
>>
>> Hi Biju,
>>
>> On Wed, May 3, 2023 at 10:46 AM Biju Das <biju.das.jz@bp.renesas.com> wrote:
>>> 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>
>>
>> Thanks for your patch!
>>
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/mfd/renesas,raa215300.yaml
>>> @@ -0,0 +1,57 @@
>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) %YAML 1.2
>>> +---
>>> +$id:
>>> +https://jpn01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevi
>>> +cetree.org%2Fschemas%2Fmfd%2Frenesas%2Craa215300.yaml%23&data=05%7C01
>>> +%7Cbiju.das.jz%40bp.renesas.com%7C37302c6f37b048ae260f08db4bba1f2f%7C
>>> +53d82571da1947e49cb4625a166a4a2a%7C0%7C0%7C638187034978703511%7CUnkno
>>> +wn%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwi
>>> +LCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=qnhJGzPolFSsY1fN2p8BJTw%2FCZunFI
>>> +%2BWgZne6CXS0T4%3D&reserved=0
>>> +$schema:
>>> +https://jpn01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevi
>>> +cetree.org%2Fmeta-schemas%2Fcore.yaml%23&data=05%7C01%7Cbiju.das.jz%4
>>> +0bp.renesas.com%7C37302c6f37b048ae260f08db4bba1f2f%7C53d82571da1947e4
>>> +9cb4625a166a4a2a%7C0%7C0%7C638187034978703511%7CUnknown%7CTWFpbGZsb3d
>>> +8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7
>>> +C3000%7C%7C%7C&sdata=86%2FKxrWS6oJZVpmTyYmKqJmRuBTYWqmqSlwMvtS16pc%3D
>>> +&reserved=0
>>> +
>>> +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-me
>>> + mory-built-charger-and-rtc
>>> +
>>> +properties:
>>> +  compatible:
>>> +    enum:
>>> +      - renesas,raa215300
>>
>> renesas,raa215300-pmic?

Depends if raa215300 is piece of SoC or just PMIC. If the latter,
usually compatibles do not have types added.

Best regards,
Krzysztof

Re: [PATCH RFC 1/6] dt-bindings: mfd: Add Renesas RAA215300 PMIC bindings

[Krzysztof Kozlowski]

On 03/05/2023 10:46, Biju Das wrote:
> 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>
> ---
>  .../bindings/mfd/renesas,raa215300.yaml       | 57 +++++++++++++++++++
>  1 file changed, 57 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mfd/renesas,raa215300.yaml
> 
> diff --git a/Documentation/devicetree/bindings/mfd/renesas,raa215300.yaml b/Documentation/devicetree/bindings/mfd/renesas,raa215300.yaml
> new file mode 100644
> index 000000000000..1e65f7618333
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/renesas,raa215300.yaml
> @@ -0,0 +1,57 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/mfd/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: 1
> +
> +  renesas,raa215300-rtc:
> +    $ref: /schemas/types.yaml#/definitions/phandle
> +    description: phandle to the built-in RTC device.

Why do you need phandle to anything else? This looks like wrong
relationship described. If these are siblings, why do you need
cross-linking via phandles?

Most of PMICs are described with one node, even though RTC is on
separate address.


> +
> +required:
> +  - compatible
> +  - reg
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    i2c {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        pmic: raa215300@12 {

Node names should be generic.
https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation

> +            compatible = "renesas,raa215300";
> +            reg = <0x12>;
> +
> +            renesas,raa215300-rtc = <&rtc_raa215300>;
> +        };
> +    };

Best regards,
Krzysztof

Re: [PATCH RFC 4/6] dt-bindings: rtc: isl1208: Document built-in RTC device on PMIC RAA215300

[Krzysztof Kozlowski]

On 03/05/2023 10:46, Biju Das wrote:
> The Built-in RTC device found on PMIC RAA215300 is similar to the isl1208
> IP. However, RTC is enabled by PMIC RAA215300 and the polarity of the
> external oscillator is determined by the PMIC revision.
> 
> Document renesas,raa215300-isl1208 compatible and renesas,raa215300-pmic
> property to handle these differences.
> 
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> ---
>  .../devicetree/bindings/rtc/isil,isl1208.yaml       | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/rtc/isil,isl1208.yaml b/Documentation/devicetree/bindings/rtc/isil,isl1208.yaml
> index 04d51887855f..888a832ed1db 100644
> --- a/Documentation/devicetree/bindings/rtc/isil,isl1208.yaml
> +++ b/Documentation/devicetree/bindings/rtc/isil,isl1208.yaml
> @@ -18,6 +18,7 @@ properties:
>            - isil,isl1209
>            - isil,isl1218
>            - isil,isl1219
> +          - renesas,raa215300-isl1208
>  
>    reg:
>      maxItems: 1
> @@ -40,6 +41,10 @@ properties:
>          <0> : Enable internal pull-up
>          <1> : Disable internal pull-up
>  
> +  renesas,raa215300-pmic:
> +    $ref: /schemas/types.yaml#/definitions/phandle
> +    description: phandle to the pmic device with isl1208 built-in RTC.

No. You don't need cross-linking. We do not represent one device as two
and then fix this by cross-linking them. The existing binding does not
have it, so it should be a hint for you.

Best regards,
Krzysztof

Re: [PATCH RFC 4/6] dt-bindings: rtc: isl1208: Document built-in RTC device on PMIC RAA215300

[Geert Uytterhoeven]

Hi Krzysztof,

On Thu, May 4, 2023 at 9:11 AM Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
> On 03/05/2023 10:46, Biju Das wrote:
> > The Built-in RTC device found on PMIC RAA215300 is similar to the isl1208
> > IP. However, RTC is enabled by PMIC RAA215300 and the polarity of the
> > external oscillator is determined by the PMIC revision.
> >
> > Document renesas,raa215300-isl1208 compatible and renesas,raa215300-pmic
> > property to handle these differences.
> >
> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > ---
> >  .../devicetree/bindings/rtc/isil,isl1208.yaml       | 13 +++++++++++++
> >  1 file changed, 13 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/rtc/isil,isl1208.yaml b/Documentation/devicetree/bindings/rtc/isil,isl1208.yaml
> > index 04d51887855f..888a832ed1db 100644
> > --- a/Documentation/devicetree/bindings/rtc/isil,isl1208.yaml
> > +++ b/Documentation/devicetree/bindings/rtc/isil,isl1208.yaml
> > @@ -18,6 +18,7 @@ properties:
> >            - isil,isl1209
> >            - isil,isl1218
> >            - isil,isl1219
> > +          - renesas,raa215300-isl1208
> >
> >    reg:
> >      maxItems: 1
> > @@ -40,6 +41,10 @@ properties:
> >          <0> : Enable internal pull-up
> >          <1> : Disable internal pull-up
> >
> > +  renesas,raa215300-pmic:
> > +    $ref: /schemas/types.yaml#/definitions/phandle
> > +    description: phandle to the pmic device with isl1208 built-in RTC.
>
> No. You don't need cross-linking. We do not represent one device as two
> and then fix this by cross-linking them. The existing binding does not
> have it, so it should be a hint for you.

Makes sense.
So there should be a single device node with 2 reg cells, and
a "renesas,raa215300" compatible value.

On the Linux side, the "renesas,raa215300" MFD driver can instantiate
a PMIC and an RTC cell, the latter served by the (enhanced) existing
rtc-isl1208 driver.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH RFC 4/6] dt-bindings: rtc: isl1208: Document built-in RTC device on PMIC RAA215300

[Krzysztof Kozlowski]

On 04/05/2023 09:47, Geert Uytterhoeven wrote:
> Hi Krzysztof,
> 
> On Thu, May 4, 2023 at 9:11 AM Krzysztof Kozlowski
> <krzysztof.kozlowski@linaro.org> wrote:
>> On 03/05/2023 10:46, Biju Das wrote:
>>> The Built-in RTC device found on PMIC RAA215300 is similar to the isl1208
>>> IP. However, RTC is enabled by PMIC RAA215300 and the polarity of the
>>> external oscillator is determined by the PMIC revision.
>>>
>>> Document renesas,raa215300-isl1208 compatible and renesas,raa215300-pmic
>>> property to handle these differences.
>>>
>>> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
>>> ---
>>>  .../devicetree/bindings/rtc/isil,isl1208.yaml       | 13 +++++++++++++
>>>  1 file changed, 13 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/rtc/isil,isl1208.yaml b/Documentation/devicetree/bindings/rtc/isil,isl1208.yaml
>>> index 04d51887855f..888a832ed1db 100644
>>> --- a/Documentation/devicetree/bindings/rtc/isil,isl1208.yaml
>>> +++ b/Documentation/devicetree/bindings/rtc/isil,isl1208.yaml
>>> @@ -18,6 +18,7 @@ properties:
>>>            - isil,isl1209
>>>            - isil,isl1218
>>>            - isil,isl1219
>>> +          - renesas,raa215300-isl1208
>>>
>>>    reg:
>>>      maxItems: 1
>>> @@ -40,6 +41,10 @@ properties:
>>>          <0> : Enable internal pull-up
>>>          <1> : Disable internal pull-up
>>>
>>> +  renesas,raa215300-pmic:
>>> +    $ref: /schemas/types.yaml#/definitions/phandle
>>> +    description: phandle to the pmic device with isl1208 built-in RTC.
>>
>> No. You don't need cross-linking. We do not represent one device as two
>> and then fix this by cross-linking them. The existing binding does not
>> have it, so it should be a hint for you.
> 
> Makes sense.
> So there should be a single device node with 2 reg cells, and
> a "renesas,raa215300" compatible value.

Yes.

> 
> On the Linux side, the "renesas,raa215300" MFD driver can instantiate
> a PMIC and an RTC cell, the latter served by the (enhanced) existing
> rtc-isl1208 driver.

Right.

Best regards,
Krzysztof

RE: [PATCH RFC 4/6] dt-bindings: rtc: isl1208: Document built-in RTC device on PMIC RAA215300

[Biju Das]

Hi Krystoff,

> Subject: Re: [PATCH RFC 4/6] dt-bindings: rtc: isl1208: Document built-in
> RTC device on PMIC RAA215300
> 
> On 04/05/2023 09:47, Geert Uytterhoeven wrote:
> > Hi Krzysztof,
> >
> > On Thu, May 4, 2023 at 9:11 AM Krzysztof Kozlowski
> > <krzysztof.kozlowski@linaro.org> wrote:
> >> On 03/05/2023 10:46, Biju Das wrote:
> >>> The Built-in RTC device found on PMIC RAA215300 is similar to the
> >>> isl1208 IP. However, RTC is enabled by PMIC RAA215300 and the
> >>> polarity of the external oscillator is determined by the PMIC revision.
> >>>
> >>> Document renesas,raa215300-isl1208 compatible and
> >>> renesas,raa215300-pmic property to handle these differences.
> >>>
> >>> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> >>> ---
> >>>  .../devicetree/bindings/rtc/isil,isl1208.yaml       | 13 +++++++++++++
> >>>  1 file changed, 13 insertions(+)
> >>>
> >>> diff --git a/Documentation/devicetree/bindings/rtc/isil,isl1208.yaml
> >>> b/Documentation/devicetree/bindings/rtc/isil,isl1208.yaml
> >>> index 04d51887855f..888a832ed1db 100644
> >>> --- a/Documentation/devicetree/bindings/rtc/isil,isl1208.yaml
> >>> +++ b/Documentation/devicetree/bindings/rtc/isil,isl1208.yaml
> >>> @@ -18,6 +18,7 @@ properties:
> >>>            - isil,isl1209
> >>>            - isil,isl1218
> >>>            - isil,isl1219
> >>> +          - renesas,raa215300-isl1208
> >>>
> >>>    reg:
> >>>      maxItems: 1
> >>> @@ -40,6 +41,10 @@ properties:
> >>>          <0> : Enable internal pull-up
> >>>          <1> : Disable internal pull-up
> >>>
> >>> +  renesas,raa215300-pmic:
> >>> +    $ref: /schemas/types.yaml#/definitions/phandle
> >>> +    description: phandle to the pmic device with isl1208 built-in RTC.
> >>
> >> No. You don't need cross-linking. We do not represent one device as
> >> two and then fix this by cross-linking them. The existing binding
> >> does not have it, so it should be a hint for you.
> >
> > Makes sense.
> > So there should be a single device node with 2 reg cells, and a
> > "renesas,raa215300" compatible value.
> 
> Yes.
> 
> >
> > On the Linux side, the "renesas,raa215300" MFD driver can instantiate
> > a PMIC and an RTC cell, the latter served by the (enhanced) existing
> > rtc-isl1208 driver.
> 
> Right.

We cannot use MFD driver to instantiate RTC as it is not platform device.

Thanks to Geert for pointing out "i2c_new_ancillary_device".

With this, we can register rtc device from pmic driver
Using the api "raa215300_rtc_probe(struct i2c_client *client, unsigned int pmic_version)".

Cheers,
Biju

RE: [PATCH RFC 1/6] dt-bindings: mfd: Add Renesas RAA215300 PMIC bindings

[Biju Das]

Hi Krzysztof Kozlowski,

Thanks for the feedback.

> Subject: Re: [PATCH RFC 1/6] dt-bindings: mfd: Add Renesas RAA215300 PMIC
> bindings
> 
> On 03/05/2023 10:46, Biju Das wrote:
> > 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>
> > ---
> >  .../bindings/mfd/renesas,raa215300.yaml       | 57 +++++++++++++++++++
> >  1 file changed, 57 insertions(+)
> >  create mode 100644
> > Documentation/devicetree/bindings/mfd/renesas,raa215300.yaml
> >
> > diff --git
> > a/Documentation/devicetree/bindings/mfd/renesas,raa215300.yaml
> > b/Documentation/devicetree/bindings/mfd/renesas,raa215300.yaml
> > new file mode 100644
> > index 000000000000..1e65f7618333
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/mfd/renesas,raa215300.yaml
> > @@ -0,0 +1,57 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) %YAML 1.2
> > +---
> > +$id:
> > +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-me
> > + mory-built-charger-and-rtc
> > +
> > +properties:
> > +  compatible:
> > +    enum:
> > +      - renesas,raa215300
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  renesas,raa215300-rtc:
> > +    $ref: /schemas/types.yaml#/definitions/phandle
> > +    description: phandle to the built-in RTC device.
> 
> Why do you need phandle to anything else? This looks like wrong relationship
> described. If these are siblings, why do you need cross-linking via
> phandles?
> 
> Most of PMICs are described with one node, even though RTC is on separate
> address.

OK, will model like below

	raa215300: pmic@12 {
		compatible = "renesas,raa215300";
		reg = <0x12 0x6f>;
		reg-names = "main", "rtc";
	};

Cheers,
Biju

> 
> 
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    i2c {
> > +        #address-cells = <1>;
> > +        #size-cells = <0>;
> > +
> > +        pmic: raa215300@12 {
> 
> Node names should be generic.

OK, will fix.
> 
> > +            compatible = "renesas,raa215300";
> > +            reg = <0x12>;
> > +
> > +            renesas,raa215300-rtc = <&rtc_raa215300>;
> > +        };
> > +    };
> 
> Best regards,
> Krzysztof

RE: [PATCH RFC 4/6] dt-bindings: rtc: isl1208: Document built-in RTC device on PMIC RAA215300

[Biju Das]

Hi Krzysztof Kozlowski,

Thanks for the feedback.	

> -----Original Message-----
> From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> Sent: Thursday, May 4, 2023 8:11 AM
> To: Biju Das <biju.das.jz@bp.renesas.com>; Alessandro Zummo
> <a.zummo@towertech.it>; Alexandre Belloni <alexandre.belloni@bootlin.com>;
> Rob Herring <robh+dt@kernel.org>; Krzysztof Kozlowski
> <krzysztof.kozlowski+dt@linaro.org>
> Cc: Geert Uytterhoeven <geert+renesas@glider.be>; Magnus Damm
> <magnus.damm@gmail.com>; Trent Piepho <tpiepho@impinj.com>; linux-
> rtc@vger.kernel.org; devicetree@vger.kernel.org; linux-renesas-
> soc@vger.kernel.org; Fabrizio Castro <fabrizio.castro.jz@renesas.com>
> Subject: Re: [PATCH RFC 4/6] dt-bindings: rtc: isl1208: Document built-in
> RTC device on PMIC RAA215300
> 
> On 03/05/2023 10:46, Biju Das wrote:
> > The Built-in RTC device found on PMIC RAA215300 is similar to the
> > isl1208 IP. However, RTC is enabled by PMIC RAA215300 and the polarity
> > of the external oscillator is determined by the PMIC revision.
> >
> > Document renesas,raa215300-isl1208 compatible and
> > renesas,raa215300-pmic property to handle these differences.
> >
> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > ---
> >  .../devicetree/bindings/rtc/isil,isl1208.yaml       | 13 +++++++++++++
> >  1 file changed, 13 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/rtc/isil,isl1208.yaml
> > b/Documentation/devicetree/bindings/rtc/isil,isl1208.yaml
> > index 04d51887855f..888a832ed1db 100644
> > --- a/Documentation/devicetree/bindings/rtc/isil,isl1208.yaml
> > +++ b/Documentation/devicetree/bindings/rtc/isil,isl1208.yaml
> > @@ -18,6 +18,7 @@ properties:
> >            - isil,isl1209
> >            - isil,isl1218
> >            - isil,isl1219
> > +          - renesas,raa215300-isl1208

As with the below model, above compatible is not required.

	raa215300: pmic@12 {
		compatible = "renesas,raa215300";
		reg = <0x12 0x6f>;
		reg-names = "main", "rtc";
	};

> >
> >    reg:
> >      maxItems: 1
> > @@ -40,6 +41,10 @@ properties:
> >          <0> : Enable internal pull-up
> >          <1> : Disable internal pull-up
> >
> > +  renesas,raa215300-pmic:
> > +    $ref: /schemas/types.yaml#/definitions/phandle
> > +    description: phandle to the pmic device with isl1208 built-in RTC.
> 
> No. You don't need cross-linking. We do not represent one device as two and
> then fix this by cross-linking them. The existing binding does not have it,
> so it should be a hint for you.

OK will remove as discussed above.

Cheers,
Biju

RE: [PATCH RFC 3/6] dt-bindings: rtc: isl1208: Convert to json-schema

[Biju Das]

Hi Krzysztof Kozlowski and  Rob,

> > > <fabrizio.castro.jz@renesas.com>; linux-renesas-soc@vger.kernel.org
> > > Subject: [PATCH RFC 3/6] dt-bindings: rtc: isl1208: Convert to
> > > json-schema
> > >
> > > 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>
> > > ---
> > >  .../devicetree/bindings/rtc/isil,isl1208.txt  | 38 ----------
> > > .../devicetree/bindings/rtc/isil,isl1208.yaml | 74
> > > +++++++++++++++++++
> > >  2 files changed, 74 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..04d51887855f
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/rtc/isil,isl1208.yaml
> > > @@ -0,0 +1,74 @@
> > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) %YAML 1.2
> > > +---
> > > +$id:
> > > +
> > > +title: Intersil ISL12{08,09,18,19} I2C RTC/Alarm chip
> > > +
> > > +maintainers:
> > > +  - Biju Das <biju.das.jz@bp.renesas.com>
> > > +  - Trent Piepho <tpiepho@impinj.com>
> > > +
> > > +properties:
> > > +  compatible:
> > > +    oneOf:
> > > +      - enum:
> > > +          - isil,isl1208
> > > +          - isil,isl1209
> > > +          - isil,isl1218
> > > +          - isil,isl1219
> > > +
> > > +  reg:
> > > +    maxItems: 1
> > > +
> > > +  interrupts:
> > > +    minItems: 1
> > > +    maxItems: 2
> > > +
> > > +  interrupt-names:
> > > +    items:
> > > +      - const: irq
> > > +      - const: evdet
> > > +
> > > +  isil,ev-evienb:
> > > +    $ref: /schemas/types.yaml#/definitions/uint32
> > > +    enum: [ 0, 1 ]
> > > +    default: 0


What is your thoughts on this? we should keep default or we should remove?

As per HW data sheet[1], the reset value is 0,  
[1] https://www.renesas.com/us/en/document/dst/isl1219-datasheet

But as per text version of bindings [2], Looks like default is not needed.

[2] https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/Documentation/devicetree/bindings/rtc/isil,isl1208.txt?h=next-20230428#n20

Cheers,
Biju

Re: [PATCH RFC 1/6] dt-bindings: mfd: Add Renesas RAA215300 PMIC bindings

[Krzysztof Kozlowski]

On 04/05/2023 18:13, Biju Das wrote:
> Hi Krzysztof Kozlowski,
> 
> Thanks for the feedback.
> 
>> Subject: Re: [PATCH RFC 1/6] dt-bindings: mfd: Add Renesas RAA215300 PMIC
>> bindings
>>
>> On 03/05/2023 10:46, Biju Das wrote:
>>> 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>
>>> ---
>>>  .../bindings/mfd/renesas,raa215300.yaml       | 57 +++++++++++++++++++
>>>  1 file changed, 57 insertions(+)
>>>  create mode 100644
>>> Documentation/devicetree/bindings/mfd/renesas,raa215300.yaml
>>>
>>> diff --git
>>> a/Documentation/devicetree/bindings/mfd/renesas,raa215300.yaml
>>> b/Documentation/devicetree/bindings/mfd/renesas,raa215300.yaml
>>> new file mode 100644
>>> index 000000000000..1e65f7618333
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/mfd/renesas,raa215300.yaml
>>> @@ -0,0 +1,57 @@
>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) %YAML 1.2
>>> +---
>>> +$id:
>>> +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-me
>>> + mory-built-charger-and-rtc
>>> +
>>> +properties:
>>> +  compatible:
>>> +    enum:
>>> +      - renesas,raa215300
>>> +
>>> +  reg:
>>> +    maxItems: 1
>>> +
>>> +  renesas,raa215300-rtc:
>>> +    $ref: /schemas/types.yaml#/definitions/phandle
>>> +    description: phandle to the built-in RTC device.
>>
>> Why do you need phandle to anything else? This looks like wrong relationship
>> described. If these are siblings, why do you need cross-linking via
>> phandles?
>>
>> Most of PMICs are described with one node, even though RTC is on separate
>> address.
> 
> OK, will model like below
> 
> 	raa215300: pmic@12 {
> 		compatible = "renesas,raa215300";
> 		reg = <0x12 0x6f>;
> 		reg-names = "main", "rtc";

Just two separate regs. I think this should work for I2C bus. The DT
schema allows multiple addresses for children.

Best regards,
Krzysztof

Re: [PATCH RFC 3/6] dt-bindings: rtc: isl1208: Convert to json-schema

[Krzysztof Kozlowski]

On 04/05/2023 18:22, Biju Das wrote:
> Hi Krzysztof Kozlowski and  Rob,
> 
>>>> <fabrizio.castro.jz@renesas.com>; linux-renesas-soc@vger.kernel.org
>>>> Subject: [PATCH RFC 3/6] dt-bindings: rtc: isl1208: Convert to
>>>> json-schema
>>>>
>>>> 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>
>>>> ---
>>>>  .../devicetree/bindings/rtc/isil,isl1208.txt  | 38 ----------
>>>> .../devicetree/bindings/rtc/isil,isl1208.yaml | 74
>>>> +++++++++++++++++++
>>>>  2 files changed, 74 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..04d51887855f
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/rtc/isil,isl1208.yaml
>>>> @@ -0,0 +1,74 @@
>>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) %YAML 1.2
>>>> +---
>>>> +$id:
>>>> +
>>>> +title: Intersil ISL12{08,09,18,19} I2C RTC/Alarm chip
>>>> +
>>>> +maintainers:
>>>> +  - Biju Das <biju.das.jz@bp.renesas.com>
>>>> +  - Trent Piepho <tpiepho@impinj.com>
>>>> +
>>>> +properties:
>>>> +  compatible:
>>>> +    oneOf:
>>>> +      - enum:
>>>> +          - isil,isl1208
>>>> +          - isil,isl1209
>>>> +          - isil,isl1218
>>>> +          - isil,isl1219
>>>> +
>>>> +  reg:
>>>> +    maxItems: 1
>>>> +
>>>> +  interrupts:
>>>> +    minItems: 1
>>>> +    maxItems: 2
>>>> +
>>>> +  interrupt-names:
>>>> +    items:
>>>> +      - const: irq
>>>> +      - const: evdet
>>>> +
>>>> +  isil,ev-evienb:
>>>> +    $ref: /schemas/types.yaml#/definitions/uint32
>>>> +    enum: [ 0, 1 ]
>>>> +    default: 0
> 
> 
> What is your thoughts on this? we should keep default or we should remove?
> 
> As per HW data sheet[1], the reset value is 0,  
> [1] https://www.renesas.com/us/en/document/dst/isl1219-datasheet
> 
> But as per text version of bindings [2], Looks like default is not needed.

Missing value has different meaning in original binding, so default is
wrong here and you should explain that meaning in description.

Best regards,
Krzysztof

RE: [PATCH RFC 1/6] dt-bindings: mfd: Add Renesas RAA215300 PMIC bindings

[Biju Das]

Hi Krzysztof Kozlowski,

> Subject: Re: [PATCH RFC 1/6] dt-bindings: mfd: Add Renesas RAA215300 PMIC
> bindings
> 
> On 04/05/2023 18:13, Biju Das wrote:
> > Hi Krzysztof Kozlowski,
> >
> > Thanks for the feedback.
> >
> >> Subject: Re: [PATCH RFC 1/6] dt-bindings: mfd: Add Renesas RAA215300
> >> PMIC bindings
> >>
> >> On 03/05/2023 10:46, Biju Das wrote:
> >>> 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>
> >>> ---
> >>>  .../bindings/mfd/renesas,raa215300.yaml       | 57 +++++++++++++++++++
> >>>  1 file changed, 57 insertions(+)
> >>>  create mode 100644
> >>> Documentation/devicetree/bindings/mfd/renesas,raa215300.yaml
> >>>
> >>> diff --git
> >>> a/Documentation/devicetree/bindings/mfd/renesas,raa215300.yaml
> >>> b/Documentation/devicetree/bindings/mfd/renesas,raa215300.yaml
> >>> new file mode 100644
> >>> index 000000000000..1e65f7618333
> >>> --- /dev/null
> >>> +++ b/Documentation/devicetree/bindings/mfd/renesas,raa215300.yaml
> >>> @@ -0,0 +1,57 @@
> >>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) %YAML 1.2
> >>> +---
> >>> +$id:
> >>> +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/mult
> >>> + i-
> >>> + channel-power-management-ics-pmics/ssdsoc-power-management-ics-pmi
> >>> + c-
> >>> + and-pmus/raa215300-high-performance-9-channel-pmic-supporting-ddr-
> >>> + me
> >>> + mory-built-charger-and-rtc
> >>> +
> >>> +properties:
> >>> +  compatible:
> >>> +    enum:
> >>> +      - renesas,raa215300
> >>> +
> >>> +  reg:
> >>> +    maxItems: 1
> >>> +
> >>> +  renesas,raa215300-rtc:
> >>> +    $ref: /schemas/types.yaml#/definitions/phandle
> >>> +    description: phandle to the built-in RTC device.
> >>
> >> Why do you need phandle to anything else? This looks like wrong
> >> relationship described. If these are siblings, why do you need
> >> cross-linking via phandles?
> >>
> >> Most of PMICs are described with one node, even though RTC is on
> >> separate address.
> >
> > OK, will model like below
> >
> > 	raa215300: pmic@12 {
> > 		compatible = "renesas,raa215300";
> > 		reg = <0x12 0x6f>;
> > 		reg-names = "main", "rtc";
> 
> Just two separate regs. I think this should work for I2C bus. The DT schema
> allows multiple addresses for children.

OK, I will add like below
reg = <0x12>, <0x6f>;

Apart from this, I would like to add below optional properties as the enabling is based
On board design. 

renesas,rtc-enable: To indicate RTC IP is enabled (eg:Cases like both XIN and XOUT grounded)
interrupts: (eg: Cases like interrupt line is not connected to SoC)

Cheers,
Biju

RE: [PATCH RFC 3/6] dt-bindings: rtc: isl1208: Convert to json-schema

[Biju Das]

Hi Krzysztof Kozlowski,

> Subject: Re: [PATCH RFC 3/6] dt-bindings: rtc: isl1208: Convert to json-
> schema
> 
> On 04/05/2023 18:22, Biju Das wrote:
> > Hi Krzysztof Kozlowski and  Rob,
> >
> >>>> <fabrizio.castro.jz@renesas.com>; linux-renesas-soc@vger.kernel.org
> >>>> Subject: [PATCH RFC 3/6] dt-bindings: rtc: isl1208: Convert to
> >>>> json-schema
> >>>>
> >>>> 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>
> >>>> ---
> >>>>  .../devicetree/bindings/rtc/isil,isl1208.txt  | 38 ----------
> >>>> .../devicetree/bindings/rtc/isil,isl1208.yaml | 74
> >>>> +++++++++++++++++++
> >>>>  2 files changed, 74 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..04d51887855f
> >>>> --- /dev/null
> >>>> +++ b/Documentation/devicetree/bindings/rtc/isil,isl1208.yaml
> >>>> @@ -0,0 +1,74 @@
> >>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) %YAML
> >>>> +1.2
> >>>> +---
> >>>> +$id:
> >>>> +
> >>>> +title: Intersil ISL12{08,09,18,19} I2C RTC/Alarm chip
> >>>> +
> >>>> +maintainers:
> >>>> +  - Biju Das <biju.das.jz@bp.renesas.com>
> >>>> +  - Trent Piepho <tpiepho@impinj.com>
> >>>> +
> >>>> +properties:
> >>>> +  compatible:
> >>>> +    oneOf:
> >>>> +      - enum:
> >>>> +          - isil,isl1208
> >>>> +          - isil,isl1209
> >>>> +          - isil,isl1218
> >>>> +          - isil,isl1219
> >>>> +
> >>>> +  reg:
> >>>> +    maxItems: 1
> >>>> +
> >>>> +  interrupts:
> >>>> +    minItems: 1
> >>>> +    maxItems: 2
> >>>> +
> >>>> +  interrupt-names:
> >>>> +    items:
> >>>> +      - const: irq
> >>>> +      - const: evdet
> >>>> +
> >>>> +  isil,ev-evienb:
> >>>> +    $ref: /schemas/types.yaml#/definitions/uint32
> >>>> +    enum: [ 0, 1 ]
> >>>> +    default: 0
> >
> >
> > What is your thoughts on this? we should keep default or we should remove?
> >
> > As per HW data sheet[1], the reset value is 0, [1]
> > https://www.renesas.com/us/en/document/dst/isl1219-datasheet
> >
> > But as per text version of bindings [2], Looks like default is not needed.
> 
> Missing value has different meaning in original binding, so default is wrong
> here and you should explain that meaning in description.

OK, I will remove default and use the same description from the original bindings
and send it as separate patch.

Cheers,
Biju