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

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 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 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 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 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 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 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