linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v5 0/4] Add NVIDIA VRS RTC support
@ 2025-07-23 13:03 Shubhi Garg
  2025-07-23 13:03 ` [PATCH v5 1/4] dt-bindings: rtc: Document NVIDIA VRS RTC Shubhi Garg
                   ` (3 more replies)
  0 siblings, 4 replies; 11+ messages in thread
From: Shubhi Garg @ 2025-07-23 13:03 UTC (permalink / raw)
  To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Catalin Marinas,
	Will Deacon, Alexandre Belloni, Jonathan Hunter
  Cc: devicetree, linux-arm-kernel, linux-rtc, linux-tegra, Shubhi Garg

This patch series adds support for NVIDIA's Voltage Regulator Specification
(VRS) RTC device. It provides following features:
- read/set system time
- 32kHz clock support with backup battery input to retain system time
  across boot
- alarm functionality to wake system from suspend and shutdown state

The series includes:
- Device tree bindings for the VRS RTC
- VRS device tree nodes for NVIDIA platforms
- VRS RTC device driver
- Configuration updates to enable the driver

Changes in v5:
- moved device tree bindings from mfd to rtc
- changed dtb node name to rtc@3c
- removed VRS MFD driver
- moved VRS common functions to RTC driver
- removed unused register definitions from header
- changed driver compatible to "nvidia,vrs10-rtc"

Changes in v4:
- fixed device tree node name to "pmic@3c" in dtb aliases

Changes in v3:
- fixed device tree node name to generic "pmic@3c"
- fixed indentation in dt-bindings
- added rate limiting to interrupt clearing debug logs
- removed unnecessary braces in if blocks
- changed dependency from I2C=y to I2C in mfd Kconfig
- fixed return value in RTC driver function calls
- fixed sizeof(*variable) inside rtc driver devm_kzalloc
- switch to devm_device_init_wakeup() for automatic cleanup

Changes in v2:
- fixed, copyrights, definitions and dtb node in dt-bindings
- removed unnecessary logs from MFD and RTC driver
- fixed RTC allocation and registration APIs
- removed unnecessary functions in RTC driver
- used rtc_lock/unlock in RTC irq handler
- added alias to assign VRS RTC as RTC0
- added driver entry in MAINTAINERS
- few other miinor changes done in drivers

Shubhi Garg (4):
  dt-bindings: rtc: Document NVIDIA VRS RTC
  arm64: tegra: Add device-tree node for NVVRS RTC
  rtc: nvvrs: add NVIDIA VRS RTC device driver
  arm64: defconfig: enable NVIDIA VRS PSEQ RTC

 .../bindings/rtc/nvidia,vrs10-rtc.yaml        |  57 ++
 MAINTAINERS                                   |   8 +
 .../arm64/boot/dts/nvidia/tegra234-p3701.dtsi |  11 +
 .../arm64/boot/dts/nvidia/tegra234-p3767.dtsi |  15 +
 arch/arm64/configs/defconfig                  |   1 +
 drivers/rtc/Kconfig                           |   9 +
 drivers/rtc/Makefile                          |   1 +
 drivers/rtc/rtc-nvidia-vrs10.c                | 508 ++++++++++++++++++
 include/linux/rtc/rtc-nvidia-vrs10.h          |  78 +++
 9 files changed, 688 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/rtc/nvidia,vrs10-rtc.yaml
 create mode 100644 drivers/rtc/rtc-nvidia-vrs10.c
 create mode 100644 include/linux/rtc/rtc-nvidia-vrs10.h

-- 
2.43.0



^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v5 1/4] dt-bindings: rtc: Document NVIDIA VRS RTC
  2025-07-23 13:03 [PATCH v5 0/4] Add NVIDIA VRS RTC support Shubhi Garg
@ 2025-07-23 13:03 ` Shubhi Garg
  2025-07-24  7:59   ` Krzysztof Kozlowski
  2025-07-23 13:03 ` [PATCH v5 2/4] arm64: tegra: Add device-tree node for NVVRS RTC Shubhi Garg
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 11+ messages in thread
From: Shubhi Garg @ 2025-07-23 13:03 UTC (permalink / raw)
  To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Catalin Marinas,
	Will Deacon, Alexandre Belloni, Jonathan Hunter
  Cc: devicetree, linux-arm-kernel, linux-rtc, linux-tegra, Shubhi Garg

Add device tree bindings for NVIDIA VRS (Voltage Regulator Specification)
RTC device. NVIDIA VRS RTC provides functionality to get/set system time,
retain system time across boot, wake system from suspend and shutdown
state.

Supported platforms:
- NVIDIA Jetson AGX Orin Developer Kit
- NVIDIA IGX Orin Development Kit
- NVIDIA Jetson Orin NX Developer Kit
- NVIDIA Jetson Orin Nano Developer Kit

Signed-off-by: Shubhi Garg <shgarg@nvidia.com>
---

v5:
- moved device tree bindings from mfd to rtc
- changed dtb node name to rtc@3c
- changed compatible string to "nvidia,vrs10-rtc" 

v4:
- no changes

v3:
- fixed device tree node name to generic "pmic@3c"
- fixed indentation

v2:
- fixed copyrights
- updated description with RTC information
- added status node in dtb node example

 .../bindings/rtc/nvidia,vrs10-rtc.yaml        | 57 +++++++++++++++++++
 1 file changed, 57 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/rtc/nvidia,vrs10-rtc.yaml

diff --git a/Documentation/devicetree/bindings/rtc/nvidia,vrs10-rtc.yaml b/Documentation/devicetree/bindings/rtc/nvidia,vrs10-rtc.yaml
new file mode 100644
index 000000000000..b7eea5f03e5a
--- /dev/null
+++ b/Documentation/devicetree/bindings/rtc/nvidia,vrs10-rtc.yaml
@@ -0,0 +1,57 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/rtc/nvidia,vrs10-rtc.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: NVIDIA Voltage Regulator Specification Real Time Clock
+
+maintainers:
+  - Shubhi Garg <shgarg@nvidia.com>
+
+description:
+  NVIDIA VRS (Voltage Regulator Specification) RTC provides 32kHz RTC clock
+  support with backup battery for system timing. It provides alarm functionality
+  to wake system from suspend and shutdown state. The device also acts as an
+  interrupt controller for managing interrupts from the VRS.
+
+properties:
+  compatible:
+    const: nvidia,vrs10-rtc
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  interrupt-controller: true
+
+  '#interrupt-cells':
+    const: 2
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - interrupt-controller
+  - '#interrupt-cells'
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        rtc@3c {
+            compatible = "nvidia,vrs10-rtc";
+            reg = <0x3c>;
+            interrupt-parent = <&pmc>;
+            interrupts = <24 IRQ_TYPE_LEVEL_LOW>;
+            interrupt-controller;
+            #interrupt-cells = <2>;
+        };
+    };
-- 
2.43.0



^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v5 2/4] arm64: tegra: Add device-tree node for NVVRS RTC
  2025-07-23 13:03 [PATCH v5 0/4] Add NVIDIA VRS RTC support Shubhi Garg
  2025-07-23 13:03 ` [PATCH v5 1/4] dt-bindings: rtc: Document NVIDIA VRS RTC Shubhi Garg
@ 2025-07-23 13:03 ` Shubhi Garg
  2025-07-23 13:03 ` [PATCH v5 3/4] rtc: nvvrs: add NVIDIA VRS device driver Shubhi Garg
  2025-07-23 13:03 ` [PATCH v5 4/4] arm64: defconfig: enable NVIDIA VRS RTC Shubhi Garg
  3 siblings, 0 replies; 11+ messages in thread
From: Shubhi Garg @ 2025-07-23 13:03 UTC (permalink / raw)
  To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Catalin Marinas,
	Will Deacon, Alexandre Belloni, Jonathan Hunter
  Cc: devicetree, linux-arm-kernel, linux-rtc, linux-tegra, Shubhi Garg

Add NVIDIA VRS (Voltage Regulator Specification) RTC device tree node for
Tegra234 P3701 and P3767 platforms. Assign VRS RTC as primary RTC (rtc0).

Signed-off-by: Shubhi Garg <shgarg@nvidia.com>
---

v5:
- changed dtb node name to rtc@3c

v4:
- fixed device tree node name to "pmic@3c" in aliases

v3:
- fixed device tree node name to generic "pmic@3c"

v2:
- added alias to assign VRS RTC to rtc0
- removed status node from VRS DTB node

 arch/arm64/boot/dts/nvidia/tegra234-p3701.dtsi | 11 +++++++++++
 arch/arm64/boot/dts/nvidia/tegra234-p3767.dtsi | 15 +++++++++++++++
 2 files changed, 26 insertions(+)

diff --git a/arch/arm64/boot/dts/nvidia/tegra234-p3701.dtsi b/arch/arm64/boot/dts/nvidia/tegra234-p3701.dtsi
index 9086a0d010e5..caa8d42438a4 100644
--- a/arch/arm64/boot/dts/nvidia/tegra234-p3701.dtsi
+++ b/arch/arm64/boot/dts/nvidia/tegra234-p3701.dtsi
@@ -8,6 +8,7 @@ / {
 	aliases {
 		mmc0 = "/bus@0/mmc@3460000";
 		mmc1 = "/bus@0/mmc@3400000";
+		rtc0 = "/bpmp/i2c/rtc@3c";
 	};
 
 	bus@0 {
@@ -170,6 +171,16 @@ bpmp {
 		i2c {
 			status = "okay";
 
+			rtc@3c {
+				compatible = "nvidia,vrs10-rtc";
+				reg = <0x3c>;
+				interrupt-parent = <&pmc>;
+				/* VRS Wake ID is 24 */
+				interrupts = <24 IRQ_TYPE_LEVEL_LOW>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
+			};
+
 			thermal-sensor@4c {
 				compatible = "ti,tmp451";
 				status = "okay";
diff --git a/arch/arm64/boot/dts/nvidia/tegra234-p3767.dtsi b/arch/arm64/boot/dts/nvidia/tegra234-p3767.dtsi
index 84db7132e8fc..565b613e0684 100644
--- a/arch/arm64/boot/dts/nvidia/tegra234-p3767.dtsi
+++ b/arch/arm64/boot/dts/nvidia/tegra234-p3767.dtsi
@@ -7,6 +7,7 @@ / {
 
 	aliases {
 		mmc0 = "/bus@0/mmc@3400000";
+		rtc0 = "/bpmp/i2c/rtc@3c";
 	};
 
 	bus@0 {
@@ -121,6 +122,20 @@ pmc@c360000 {
 		};
 	};
 
+	bpmp {
+		i2c {
+			rtc@3c {
+				compatible = "nvidia,vrs10-rtc";
+				reg = <0x3c>;
+				interrupt-parent = <&pmc>;
+				/* VRS Wake ID is 24 */
+				interrupts = <24 IRQ_TYPE_LEVEL_LOW>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
+			};
+		};
+	};
+
 	vdd_5v0_sys: regulator-vdd-5v0-sys {
 		compatible = "regulator-fixed";
 		regulator-name = "VDD_5V0_SYS";
-- 
2.43.0



^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v5 3/4] rtc: nvvrs: add NVIDIA VRS device driver
  2025-07-23 13:03 [PATCH v5 0/4] Add NVIDIA VRS RTC support Shubhi Garg
  2025-07-23 13:03 ` [PATCH v5 1/4] dt-bindings: rtc: Document NVIDIA VRS RTC Shubhi Garg
  2025-07-23 13:03 ` [PATCH v5 2/4] arm64: tegra: Add device-tree node for NVVRS RTC Shubhi Garg
@ 2025-07-23 13:03 ` Shubhi Garg
  2025-07-23 13:03 ` [PATCH v5 4/4] arm64: defconfig: enable NVIDIA VRS RTC Shubhi Garg
  3 siblings, 0 replies; 11+ messages in thread
From: Shubhi Garg @ 2025-07-23 13:03 UTC (permalink / raw)
  To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Catalin Marinas,
	Will Deacon, Alexandre Belloni, Jonathan Hunter
  Cc: devicetree, linux-arm-kernel, linux-rtc, linux-tegra, Shubhi Garg

Add support for NVIDIA VRS (Voltage Regulator Specification) RTC device
driver. The driver provides functionality to get/set system time, retain
system time across boot, wake system from suspend and shutdown state.

Supported platforms:
- NVIDIA Jetson AGX Orin Developer Kit
- NVIDIA IGX Orin Development Kit
- NVIDIA Jetson Orin NX Developer Kit
- NVIDIA Jetson Orin Nano Developer Kit

Signed-off-by: Shubhi Garg <shgarg@nvidia.com>
---

v5:
- removed unused register definitions from header
- added VRS to maintainers list
- removed MFD dependency from Kconfig
- improved driver filename and CONFIG name
- handle all VRS interrupts in RTC driver
- validate chip vendor info during RTC probe
- clear any pending IRQs during RTC probe

v4:
- no changes

v3:
- fixed return value in RTC read_time and read_alarm functions
- fixed sizeof(*variable) inside rtc driver devm_kzalloc
- switch to devm_device_init_wakeup() for automatic cleanup

v2:
- removed regmap struct since it is not required
- removed rtc_map definition to directly use register definition
- removed unnecessary dev_err logs
- fixed dev_err logs to dev_dbg
- used rtc_lock/unlock in irq handler
- changed RTC allocation and register APIs as per latest kernel
- removed nvvrs_rtc_remove function since it's not required

 MAINTAINERS                          |   8 +
 drivers/rtc/Kconfig                  |   9 +
 drivers/rtc/Makefile                 |   1 +
 drivers/rtc/rtc-nvidia-vrs10.c       | 508 +++++++++++++++++++++++++++
 include/linux/rtc/rtc-nvidia-vrs10.h |  78 ++++
 5 files changed, 604 insertions(+)
 create mode 100644 drivers/rtc/rtc-nvidia-vrs10.c
 create mode 100644 include/linux/rtc/rtc-nvidia-vrs10.h

diff --git a/MAINTAINERS b/MAINTAINERS
index ffb35359f1e2..944ef074352d 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -18008,6 +18008,14 @@ S:	Maintained
 F:	drivers/video/fbdev/nvidia/
 F:	drivers/video/fbdev/riva/
 
+NVIDIA VRS RTC DRIVER
+M:	Shubhi Garg <shgarg@nvidia.com>
+L:	linux-tegra@vger.kernel.org
+S:	Maintained
+F:	Documentation/devicetree/bindings/rtc/nvidia,vrs10-rtc.yaml
+F:	drivers/rtc/rtc-nvidia-vrs10.c
+F:	include/linux/rtc/rtc-nvidia-vrs10.h
+
 NVIDIA WMI EC BACKLIGHT DRIVER
 M:	Daniel Dadap <ddadap@nvidia.com>
 L:	platform-driver-x86@vger.kernel.org
diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
index 9aec922613ce..4a573919d4fa 100644
--- a/drivers/rtc/Kconfig
+++ b/drivers/rtc/Kconfig
@@ -406,6 +406,15 @@ config RTC_DRV_MAX77686
 	  This driver can also be built as a module. If so, the module
 	  will be called rtc-max77686.
 
+config RTC_DRV_NVIDIA_VRS10
+	tristate "NVIDIA VRS10 RTC device"
+	help
+	  If you say yes here you will get support for the battery backed RTC device
+	  of NVIDIA VRS (Voltage Regulator Specification). The RTC is connected via
+	  I2C interface and supports alarm functionality.
+	  This driver can also be built as a module. If so, the module will be called
+	  rtc-nvidia-vrs10.
+
 config RTC_DRV_NCT3018Y
 	tristate "Nuvoton NCT3018Y"
 	depends on OF
diff --git a/drivers/rtc/Makefile b/drivers/rtc/Makefile
index 4619aa2ac469..4311b42438ea 100644
--- a/drivers/rtc/Makefile
+++ b/drivers/rtc/Makefile
@@ -120,6 +120,7 @@ obj-$(CONFIG_RTC_DRV_MXC_V2)	+= rtc-mxc_v2.o
 obj-$(CONFIG_RTC_DRV_GAMECUBE)	+= rtc-gamecube.o
 obj-$(CONFIG_RTC_DRV_NCT3018Y)	+= rtc-nct3018y.o
 obj-$(CONFIG_RTC_DRV_NTXEC)	+= rtc-ntxec.o
+obj-$(CONFIG_RTC_DRV_NVIDIA_VRS10)+= rtc-nvidia-vrs10.o
 obj-$(CONFIG_RTC_DRV_OMAP)	+= rtc-omap.o
 obj-$(CONFIG_RTC_DRV_OPAL)	+= rtc-opal.o
 obj-$(CONFIG_RTC_DRV_OPTEE)	+= rtc-optee.o
diff --git a/drivers/rtc/rtc-nvidia-vrs10.c b/drivers/rtc/rtc-nvidia-vrs10.c
new file mode 100644
index 000000000000..54b4afe89afb
--- /dev/null
+++ b/drivers/rtc/rtc-nvidia-vrs10.c
@@ -0,0 +1,508 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * NVIDIA Voltage Regulator Specification RTC
+ *
+ * SPDX-FileCopyrightText: Copyright (c) 2025 NVIDIA CORPORATION & AFFILIATES.
+ * All rights reserved.
+ */
+
+#include <linux/err.h>
+#include <linux/i2c.h>
+#include <linux/interrupt.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/rtc.h>
+#include <linux/rtc/rtc-nvidia-vrs10.h>
+
+#define ALARM_RESET_VAL		0xffffffff
+#define NVVRS_MIN_MODEL_REV	0x40
+
+struct nvvrs_rtc_info {
+	struct device          *dev;
+	struct i2c_client      *client;
+	struct rtc_device      *rtc;
+	unsigned int           irq;
+	/* Mutex to protect RTC operations */
+	struct mutex           lock;
+};
+
+static int nvvrs_update_bits(struct nvvrs_rtc_info *info, u8 reg,
+			     u8 mask, u8 value)
+{
+	int ret;
+	u8 val;
+
+	ret = i2c_smbus_read_byte_data(info->client, reg);
+	if (ret < 0)
+		return ret;
+
+	val = (u8)ret;
+	val &= ~mask;
+	val |= (value & mask);
+
+	return i2c_smbus_write_byte_data(info->client, reg, val);
+}
+
+static int nvvrs_rtc_write_alarm(struct i2c_client *client, u8 *time)
+{
+	int ret;
+
+	ret = i2c_smbus_write_byte_data(client, NVVRS_REG_RTC_A3, time[3]);
+	if (ret < 0)
+		return ret;
+
+	ret = i2c_smbus_write_byte_data(client, NVVRS_REG_RTC_A2, time[2]);
+	if (ret < 0)
+		return ret;
+
+	ret = i2c_smbus_write_byte_data(client, NVVRS_REG_RTC_A1, time[1]);
+	if (ret < 0)
+		return ret;
+
+	return i2c_smbus_write_byte_data(client, NVVRS_REG_RTC_A0, time[0]);
+}
+
+static int nvvrs_rtc_enable_alarm(struct nvvrs_rtc_info *info)
+{
+	int ret;
+
+	/* Set RTC_WAKE bit for autonomous wake from sleep */
+	ret = nvvrs_update_bits(info, NVVRS_REG_CTL_2, NVVRS_REG_CTL_2_RTC_WAKE,
+				NVVRS_REG_CTL_2_RTC_WAKE);
+	if (ret < 0) {
+		dev_err(info->dev, "Failed to set RTC_WAKE bit (%d)\n", ret);
+		return ret;
+	}
+
+	/* Set RTC_PU bit for autonomous wake from shutdown */
+	ret = nvvrs_update_bits(info, NVVRS_REG_CTL_2, NVVRS_REG_CTL_2_RTC_PU,
+				NVVRS_REG_CTL_2_RTC_PU);
+	if (ret < 0) {
+		dev_err(info->dev, "Failed to set RTC_PU bit (%d)\n", ret);
+		return ret;
+	}
+
+	return 0;
+}
+
+static int nvvrs_rtc_disable_alarm(struct nvvrs_rtc_info *info)
+{
+	struct i2c_client *client = info->client;
+	u8 val[4];
+	int ret;
+
+	/* Clear RTC_WAKE bit */
+	ret = nvvrs_update_bits(info, NVVRS_REG_CTL_2, NVVRS_REG_CTL_2_RTC_WAKE,
+				0);
+	if (ret < 0) {
+		dev_err(info->dev, "Failed to clear RTC_WAKE bit (%d)\n", ret);
+		return ret;
+	}
+
+	/* Clear RTC_PU bit */
+	ret = nvvrs_update_bits(info, NVVRS_REG_CTL_2, NVVRS_REG_CTL_2_RTC_PU,
+				0);
+	if (ret < 0) {
+		dev_err(info->dev, "Failed to clear RTC_PU bit (%d)\n", ret);
+		return ret;
+	}
+
+	/* Write ALARM_RESET_VAL in RTC Alarm register to disable alarm */
+	val[0] = 0xff;
+	val[1] = 0xff;
+	val[2] = 0xff;
+	val[3] = 0xff;
+
+	ret = nvvrs_rtc_write_alarm(client, val);
+	if (ret < 0)
+		dev_err(info->dev, "Failed to disable Alarm (%d)\n", ret);
+
+	return 0;
+}
+
+static int nvvrs_rtc_read_time(struct device *dev, struct rtc_time *tm)
+{
+	struct nvvrs_rtc_info *info = dev_get_drvdata(dev);
+	time64_t secs = 0;
+	int ret;
+	u8 val;
+
+	mutex_lock(&info->lock);
+
+	/*
+	 * Multi-byte transfers are not supported with PEC enabled
+	 * Read MSB first to avoid coherency issues
+	 */
+	ret = i2c_smbus_read_byte_data(info->client, NVVRS_REG_RTC_T3);
+	if (ret < 0)
+		goto out;
+
+	val = (u8)ret;
+	secs |= (time64_t)val << 24;
+
+	ret = i2c_smbus_read_byte_data(info->client, NVVRS_REG_RTC_T2);
+	if (ret < 0)
+		goto out;
+
+	val = (u8)ret;
+	secs |= (time64_t)val << 16;
+
+	ret = i2c_smbus_read_byte_data(info->client, NVVRS_REG_RTC_T1);
+	if (ret < 0)
+		goto out;
+
+	val = (u8)ret;
+	secs |= (time64_t)val << 8;
+
+	ret = i2c_smbus_read_byte_data(info->client, NVVRS_REG_RTC_T0);
+	if (ret < 0)
+		goto out;
+
+	val = (u8)ret;
+	secs |= val;
+
+	rtc_time64_to_tm(secs, tm);
+	ret = 0;
+out:
+	mutex_unlock(&info->lock);
+	return ret;
+}
+
+static int nvvrs_rtc_set_time(struct device *dev, struct rtc_time *tm)
+{
+	struct nvvrs_rtc_info *info = dev_get_drvdata(dev);
+	time64_t secs;
+	u8 time[4];
+	int ret;
+
+	mutex_lock(&info->lock);
+
+	secs = rtc_tm_to_time64(tm);
+	time[0] = secs & 0xff;
+	time[1] = (secs >> 8) & 0xff;
+	time[2] = (secs >> 16) & 0xff;
+	time[3] = (secs >> 24) & 0xff;
+
+	ret = i2c_smbus_write_byte_data(info->client, NVVRS_REG_RTC_T3, time[3]);
+	if (ret < 0)
+		goto out;
+
+	ret = i2c_smbus_write_byte_data(info->client, NVVRS_REG_RTC_T2, time[2]);
+	if (ret < 0)
+		goto out;
+
+	ret = i2c_smbus_write_byte_data(info->client, NVVRS_REG_RTC_T1, time[1]);
+	if (ret < 0)
+		goto out;
+
+	ret = i2c_smbus_write_byte_data(info->client, NVVRS_REG_RTC_T0, time[0]);
+out:
+	mutex_unlock(&info->lock);
+	return ret;
+}
+
+static int nvvrs_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alrm)
+{
+	struct nvvrs_rtc_info *info = dev_get_drvdata(dev);
+	time64_t alarm_val = 0;
+	int ret;
+	u8 val;
+
+	mutex_lock(&info->lock);
+
+	/* Multi-byte transfers are not supported with PEC enabled */
+	ret = i2c_smbus_read_byte_data(info->client, NVVRS_REG_RTC_A3);
+	if (ret < 0)
+		goto out;
+
+	val = (u8)ret;
+	alarm_val |= (time64_t)val << 24;
+
+	ret = i2c_smbus_read_byte_data(info->client, NVVRS_REG_RTC_A2);
+	if (ret < 0)
+		goto out;
+
+	val = (u8)ret;
+	alarm_val |= (time64_t)val << 16;
+
+	ret = i2c_smbus_read_byte_data(info->client, NVVRS_REG_RTC_A1);
+	if (ret < 0)
+		goto out;
+
+	val = (u8)ret;
+	alarm_val |= (time64_t)val << 8;
+
+	ret = i2c_smbus_read_byte_data(info->client, NVVRS_REG_RTC_A0);
+	if (ret < 0)
+		goto out;
+
+	val = (u8)ret;
+	alarm_val |= val;
+
+	if (alarm_val == ALARM_RESET_VAL)
+		alrm->enabled = 0;
+	else
+		alrm->enabled = 1;
+
+	rtc_time64_to_tm(alarm_val, &alrm->time);
+	ret = 0;
+out:
+	mutex_unlock(&info->lock);
+	return ret;
+}
+
+static int nvvrs_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alrm)
+{
+	struct nvvrs_rtc_info *info = dev_get_drvdata(dev);
+	time64_t secs;
+	u8 time[4];
+	int ret;
+
+	mutex_lock(&info->lock);
+
+	if (!alrm->enabled) {
+		ret = nvvrs_rtc_disable_alarm(info);
+		if (ret < 0)
+			goto out;
+	}
+
+	ret = nvvrs_rtc_enable_alarm(info);
+	if (ret < 0)
+		goto out;
+
+	secs = rtc_tm_to_time64(&alrm->time);
+	time[0] = secs & 0xff;
+	time[1] = (secs >> 8) & 0xff;
+	time[2] = (secs >> 16) & 0xff;
+	time[3] = (secs >> 24) & 0xff;
+
+	ret = nvvrs_rtc_write_alarm(info->client, time);
+
+out:
+	mutex_unlock(&info->lock);
+	return ret;
+}
+
+static int nvvrs_pseq_irq_clear(struct nvvrs_rtc_info *info)
+{
+	unsigned int i;
+	int ret;
+
+	for (i = 0; i < NVVRS_IRQ_REG_COUNT; i++) {
+		ret = i2c_smbus_read_byte_data(info->client,
+					       NVVRS_REG_INT_SRC1 + i);
+		if (ret < 0) {
+			dev_err(info->dev, "Failed to read INT_SRC%d : %d\n",
+				i + 1, ret);
+			return ret;
+		}
+
+		ret = i2c_smbus_write_byte_data(info->client,
+						NVVRS_REG_INT_SRC1 + i,
+						(u8)ret);
+		if (ret < 0) {
+			dev_err(info->dev, "Failed to clear INT_SRC%d : %d\n",
+				i + 1, ret);
+			return ret;
+		}
+	}
+
+	return 0;
+}
+
+static irqreturn_t nvvrs_rtc_irq_handler(int irq, void *data)
+{
+	struct nvvrs_rtc_info *info = data;
+	int ret;
+
+	/* Check for RTC alarm interrupt */
+	ret = i2c_smbus_read_byte_data(info->client, NVVRS_REG_INT_SRC1);
+	if (ret < 0) {
+		dev_err(info->dev, "Failed to read INT_SRC1: %d\n", ret);
+		return IRQ_NONE;
+	}
+
+	if (ret & NVVRS_INT_SRC1_RTC_MASK) {
+		rtc_lock(info->rtc);
+		rtc_update_irq(info->rtc, 1, RTC_IRQF | RTC_AF);
+		rtc_unlock(info->rtc);
+	}
+
+	/* Clear all interrupts */
+	if (nvvrs_pseq_irq_clear(info) < 0)
+		return IRQ_NONE;
+
+	return IRQ_HANDLED;
+}
+
+static int nvvrs_rtc_alarm_irq_enable(struct device *dev, unsigned int enabled)
+{
+	/*
+	 * This hardware does not support enabling/disabling the alarm IRQ
+	 * independently. The alarm is disabled by clearing the alarm time
+	 * via set_alarm().
+	 */
+	return 0;
+}
+
+static const struct rtc_class_ops nvvrs_rtc_ops = {
+	.read_time = nvvrs_rtc_read_time,
+	.set_time = nvvrs_rtc_set_time,
+	.read_alarm = nvvrs_rtc_read_alarm,
+	.set_alarm = nvvrs_rtc_set_alarm,
+	.alarm_irq_enable = nvvrs_rtc_alarm_irq_enable,
+};
+
+static int nvvrs_pseq_vendor_info(struct nvvrs_rtc_info *info)
+{
+	struct i2c_client *client = info->client;
+	u8 vendor_id, model_rev;
+	int ret;
+
+	ret = i2c_smbus_read_byte_data(client, NVVRS_REG_VENDOR_ID);
+	if (ret < 0)
+		return dev_err_probe(&client->dev, ret,
+				     "Failed to read Vendor ID\n");
+
+	vendor_id = (u8)ret;
+
+	ret = i2c_smbus_read_byte_data(client, NVVRS_REG_MODEL_REV);
+	if (ret < 0)
+		return dev_err_probe(&client->dev, ret,
+				     "Failed to read Model Revision\n");
+
+	model_rev = (u8)ret;
+
+	if (model_rev < NVVRS_MIN_MODEL_REV) {
+		return dev_err_probe(&client->dev, -ENODEV,
+				     "Chip revision 0x%02x is not supported!\n",
+				     model_rev);
+	}
+
+	dev_dbg(&client->dev, "NVVRS Vendor ID: 0x%02x, Model Rev: 0x%02x\n",
+		vendor_id, model_rev);
+
+	return 0;
+}
+
+static int nvvrs_rtc_probe(struct i2c_client *client)
+{
+	struct nvvrs_rtc_info *info;
+	int ret;
+
+	info = devm_kzalloc(&client->dev, sizeof(*info), GFP_KERNEL);
+	if (!info)
+		return -ENOMEM;
+
+	mutex_init(&info->lock);
+
+	if (client->irq <= 0)
+		return dev_err_probe(&client->dev, -EINVAL, "No IRQ specified\n");
+
+	info->irq = client->irq;
+	info->dev = &client->dev;
+	client->flags |= I2C_CLIENT_PEC;
+	i2c_set_clientdata(client, info);
+	info->client = client;
+
+	/* Check vendor info */
+	if (nvvrs_pseq_vendor_info(info) < 0)
+		return dev_err_probe(&client->dev, -EINVAL,
+				     "Failed to get vendor info\n");
+
+	/* Clear any pending IRQs before requesting IRQ handler */
+	if (nvvrs_pseq_irq_clear(info) < 0)
+		return dev_err_probe(&client->dev, -EINVAL,
+				     "Failed to clear interrupts\n");
+
+	/* Allocate RTC device */
+	info->rtc = devm_rtc_allocate_device(info->dev);
+	if (IS_ERR(info->rtc))
+		return PTR_ERR(info->rtc);
+
+	info->rtc->ops = &nvvrs_rtc_ops;
+	info->rtc->range_min = RTC_TIMESTAMP_BEGIN_2000;
+	info->rtc->range_max = RTC_TIMESTAMP_END_2099;
+
+	/* Request RTC IRQ */
+	ret = devm_request_threaded_irq(info->dev, info->irq, NULL,
+					nvvrs_rtc_irq_handler, IRQF_ONESHOT,
+					"nvvrs-rtc", info);
+	if (ret < 0) {
+		dev_err_probe(info->dev, ret, "Failed to request RTC IRQ\n");
+		return ret;
+	}
+
+	/* RTC as a wakeup source */
+	devm_device_init_wakeup(info->dev);
+
+	return devm_rtc_register_device(info->rtc);
+}
+
+#ifdef CONFIG_PM_SLEEP
+static int nvvrs_rtc_suspend(struct device *dev)
+{
+	struct nvvrs_rtc_info *info = dev_get_drvdata(dev);
+	int ret;
+
+	if (device_may_wakeup(dev)) {
+		/* Set RTC_WAKE bit for auto wake system from suspend state */
+		ret = nvvrs_update_bits(info, NVVRS_REG_CTL_2,
+					NVVRS_REG_CTL_2_RTC_WAKE,
+					NVVRS_REG_CTL_2_RTC_WAKE);
+		if (ret < 0) {
+			dev_err(info->dev, "Failed to set RTC_WAKE bit (%d)\n",
+				ret);
+			return ret;
+		}
+
+		return enable_irq_wake(info->irq);
+	}
+
+	return 0;
+}
+
+static int nvvrs_rtc_resume(struct device *dev)
+{
+	struct nvvrs_rtc_info *info = dev_get_drvdata(dev);
+	int ret;
+
+	if (device_may_wakeup(dev)) {
+		/* Clear FORCE_ACT bit */
+		ret = nvvrs_update_bits(info, NVVRS_REG_CTL_1,
+					NVVRS_REG_CTL_1_FORCE_ACT, 0);
+		if (ret < 0) {
+			dev_err(info->dev, "Failed to clear FORCE_ACT bit (%d)\n",
+				ret);
+			return ret;
+		}
+
+		return disable_irq_wake(info->irq);
+	}
+
+	return 0;
+}
+
+#endif
+static SIMPLE_DEV_PM_OPS(nvvrs_rtc_pm_ops, nvvrs_rtc_suspend, nvvrs_rtc_resume);
+
+static const struct of_device_id nvvrs_rtc_of_match[] = {
+	{ .compatible = "nvidia,vrs10-rtc" },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, nvvrs_rtc_of_match);
+
+static struct i2c_driver nvvrs_rtc_driver = {
+	.driver		= {
+		.name   = "rtc-nvidia-vrs10",
+		.pm     = &nvvrs_rtc_pm_ops,
+		.of_match_table = nvvrs_rtc_of_match,
+	},
+	.probe		= nvvrs_rtc_probe,
+};
+
+module_i2c_driver(nvvrs_rtc_driver);
+
+MODULE_AUTHOR("Shubhi Garg <shgarg@nvidia.com>");
+MODULE_DESCRIPTION("NVIDIA Voltage Regulator Specification RTC driver");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/rtc/rtc-nvidia-vrs10.h b/include/linux/rtc/rtc-nvidia-vrs10.h
new file mode 100644
index 000000000000..3c9c46abf555
--- /dev/null
+++ b/include/linux/rtc/rtc-nvidia-vrs10.h
@@ -0,0 +1,78 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * SPDX-FileCopyrightText: Copyright (c) 2025 NVIDIA CORPORATION & AFFILIATES.
+ * All rights reserved
+ */
+
+#ifndef _RTC_NVIDIA_VRS_H_
+#define _RTC_NVIDIA_VRS_H_
+
+#include <linux/types.h>
+
+/* Vendor Info */
+#define NVVRS_REG_VENDOR_ID			0x00
+#define NVVRS_REG_MODEL_REV			0x01
+
+/*  Interrupts registers */
+#define NVVRS_REG_INT_SRC1			0x10
+#define NVVRS_REG_INT_SRC2			0x11
+#define NVVRS_REG_INT_VENDOR			0x12
+
+/* Control Registers */
+#define NVVRS_REG_CTL_1				0x28
+#define NVVRS_REG_CTL_2				0x29
+
+/* RTC Registers */
+#define NVVRS_REG_RTC_T3			0x70
+#define NVVRS_REG_RTC_T2			0x71
+#define NVVRS_REG_RTC_T1			0x72
+#define NVVRS_REG_RTC_T0			0x73
+#define NVVRS_REG_RTC_A3			0x74
+#define NVVRS_REG_RTC_A2			0x75
+#define NVVRS_REG_RTC_A1			0x76
+#define NVVRS_REG_RTC_A0			0x77
+
+/* Interrupt Mask */
+#define NVVRS_INT_SRC1_RSTIRQ_MASK		BIT(0)
+#define NVVRS_INT_SRC1_OSC_MASK			BIT(1)
+#define NVVRS_INT_SRC1_EN_MASK			BIT(2)
+#define NVVRS_INT_SRC1_RTC_MASK			BIT(3)
+#define NVVRS_INT_SRC1_PEC_MASK			BIT(4)
+#define NVVRS_INT_SRC1_WDT_MASK			BIT(5)
+#define NVVRS_INT_SRC1_EM_PD_MASK		BIT(6)
+#define NVVRS_INT_SRC1_INTERNAL_MASK		BIT(7)
+#define NVVRS_INT_SRC2_PBSP_MASK		BIT(0)
+#define NVVRS_INT_SRC2_ECC_DED_MASK		BIT(1)
+#define NVVRS_INT_SRC2_TSD_MASK			BIT(2)
+#define NVVRS_INT_SRC2_LDO_MASK			BIT(3)
+#define NVVRS_INT_SRC2_BIST_MASK		BIT(4)
+#define NVVRS_INT_SRC2_RT_CRC_MASK		BIT(5)
+#define NVVRS_INT_SRC2_VENDOR_MASK		BIT(7)
+#define NVVRS_INT_VENDOR0_MASK			BIT(0)
+#define NVVRS_INT_VENDOR1_MASK			BIT(1)
+#define NVVRS_INT_VENDOR2_MASK			BIT(2)
+#define NVVRS_INT_VENDOR3_MASK			BIT(3)
+#define NVVRS_INT_VENDOR4_MASK			BIT(4)
+#define NVVRS_INT_VENDOR5_MASK			BIT(5)
+#define NVVRS_INT_VENDOR6_MASK			BIT(6)
+#define NVVRS_INT_VENDOR7_MASK			BIT(7)
+
+/* Controller Register Mask */
+#define NVVRS_REG_CTL_1_FORCE_SHDN		(BIT(0) | BIT(1))
+#define NVVRS_REG_CTL_1_FORCE_ACT		BIT(2)
+#define NVVRS_REG_CTL_1_FORCE_INT		BIT(3)
+#define NVVRS_REG_CTL_2_EN_PEC			BIT(0)
+#define NVVRS_REG_CTL_2_REQ_PEC			BIT(1)
+#define NVVRS_REG_CTL_2_RTC_PU			BIT(2)
+#define NVVRS_REG_CTL_2_RTC_WAKE		BIT(3)
+#define NVVRS_REG_CTL_2_RST_DLY			0xF0
+
+enum nvvrs_irq_regs {
+	NVVRS_IRQ_REG_INT_SRC1 = 0,
+	NVVRS_IRQ_REG_INT_SRC2 = 1,
+	NVVRS_IRQ_REG_INT_VENDOR = 2,
+	NVVRS_IRQ_REG_COUNT = 3,
+};
+
+#endif /* _RTC_NVIDIA_VRS_H_ */
+
-- 
2.43.0



^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v5 4/4] arm64: defconfig: enable NVIDIA VRS RTC
  2025-07-23 13:03 [PATCH v5 0/4] Add NVIDIA VRS RTC support Shubhi Garg
                   ` (2 preceding siblings ...)
  2025-07-23 13:03 ` [PATCH v5 3/4] rtc: nvvrs: add NVIDIA VRS device driver Shubhi Garg
@ 2025-07-23 13:03 ` Shubhi Garg
  3 siblings, 0 replies; 11+ messages in thread
From: Shubhi Garg @ 2025-07-23 13:03 UTC (permalink / raw)
  To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Catalin Marinas,
	Will Deacon, Alexandre Belloni, Jonathan Hunter
  Cc: devicetree, linux-arm-kernel, linux-rtc, linux-tegra, Shubhi Garg

Enable NVIDIA VRS (Voltage Regulator Specification) RTC device module.
It provides functionality to get/set system time, retain system time
across boot, wake system from suspend and shutdown state.

Supported platforms:
- NVIDIA Jetson AGX Orin Developer Kit
- NVIDIA IGX Orin Development Kit
- NVIDIA Jetson Orin NX Developer Kit
- NVIDIA Jetson Orin Nano Developer Kit

Signed-off-by: Shubhi Garg <shgarg@nvidia.com>
---

v5:
- removed VRS MFD CONFIG
- changed VRS RTC CONFIG name

v4:
- no changes

v3:
- no changes

v2:
- moved VRS RTC config to correct place

 arch/arm64/configs/defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index 4dcc43c62c4d..49a0e2372ab7 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -1275,6 +1275,7 @@ CONFIG_RTC_DRV_MT6397=m
 CONFIG_RTC_DRV_XGENE=y
 CONFIG_RTC_DRV_TI_K3=m
 CONFIG_RTC_DRV_RENESAS_RTCA3=m
+CONFIG_RTC_DRV_NVIDIA_VRS10=m
 CONFIG_DMADEVICES=y
 CONFIG_DMA_BCM2835=y
 CONFIG_DMA_SUN6I=m
-- 
2.43.0



^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH v5 1/4] dt-bindings: rtc: Document NVIDIA VRS RTC
  2025-07-23 13:03 ` [PATCH v5 1/4] dt-bindings: rtc: Document NVIDIA VRS RTC Shubhi Garg
@ 2025-07-24  7:59   ` Krzysztof Kozlowski
  2025-07-24  9:41     ` Jon Hunter
  0 siblings, 1 reply; 11+ messages in thread
From: Krzysztof Kozlowski @ 2025-07-24  7:59 UTC (permalink / raw)
  To: Shubhi Garg
  Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Catalin Marinas,
	Will Deacon, Alexandre Belloni, Jonathan Hunter, devicetree,
	linux-arm-kernel, linux-rtc, linux-tegra

On Wed, Jul 23, 2025 at 01:03:40PM +0000, Shubhi Garg wrote:
> +description:
> +  NVIDIA VRS (Voltage Regulator Specification) RTC provides 32kHz RTC clock
> +  support with backup battery for system timing. It provides alarm functionality
> +  to wake system from suspend and shutdown state. The device also acts as an
> +  interrupt controller for managing interrupts from the VRS.
> +
> +properties:
> +  compatible:
> +    const: nvidia,vrs10-rtc

Nothing improved. You never replied to comments and then replaced one
redundant word into other redundant word.

Respond to review or implement it fully, not partially.

Or add COMPLETE bindings, not partial ones. See writing bindings doc.

Best regards,
Krzysztof



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v5 1/4] dt-bindings: rtc: Document NVIDIA VRS RTC
  2025-07-24  7:59   ` Krzysztof Kozlowski
@ 2025-07-24  9:41     ` Jon Hunter
  2025-07-24 10:06       ` Krzysztof Kozlowski
  0 siblings, 1 reply; 11+ messages in thread
From: Jon Hunter @ 2025-07-24  9:41 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Shubhi Garg
  Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Catalin Marinas,
	Will Deacon, Alexandre Belloni, devicetree, linux-arm-kernel,
	linux-rtc, linux-tegra


On 24/07/2025 08:59, Krzysztof Kozlowski wrote:
> On Wed, Jul 23, 2025 at 01:03:40PM +0000, Shubhi Garg wrote:
>> +description:
>> +  NVIDIA VRS (Voltage Regulator Specification) RTC provides 32kHz RTC clock
>> +  support with backup battery for system timing. It provides alarm functionality
>> +  to wake system from suspend and shutdown state. The device also acts as an
>> +  interrupt controller for managing interrupts from the VRS.
>> +
>> +properties:
>> +  compatible:
>> +    const: nvidia,vrs10-rtc
> 
> Nothing improved. You never replied to comments and then replaced one
> redundant word into other redundant word.
> 
> Respond to review or implement it fully, not partially.
> 
> Or add COMPLETE bindings, not partial ones. See writing bindings doc.

OK, right so the DT binding should describe the overall PMIC device, 
even though the driver needs to support the RTC.

Shubhi, is vrs10 the version of the VRS spec for the PMIC device or just 
the RTC portion? If it is, the maybe 'nvidia,vrs10' is sufficient here.

Jon

-- 
nvpublic



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v5 1/4] dt-bindings: rtc: Document NVIDIA VRS RTC
  2025-07-24  9:41     ` Jon Hunter
@ 2025-07-24 10:06       ` Krzysztof Kozlowski
  2025-07-24 10:50         ` Jon Hunter
  0 siblings, 1 reply; 11+ messages in thread
From: Krzysztof Kozlowski @ 2025-07-24 10:06 UTC (permalink / raw)
  To: Jon Hunter, Shubhi Garg
  Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Catalin Marinas,
	Will Deacon, Alexandre Belloni, devicetree, linux-arm-kernel,
	linux-rtc, linux-tegra

On 24/07/2025 11:41, Jon Hunter wrote:
> 
> On 24/07/2025 08:59, Krzysztof Kozlowski wrote:
>> On Wed, Jul 23, 2025 at 01:03:40PM +0000, Shubhi Garg wrote:
>>> +description:
>>> +  NVIDIA VRS (Voltage Regulator Specification) RTC provides 32kHz RTC clock
>>> +  support with backup battery for system timing. It provides alarm functionality
>>> +  to wake system from suspend and shutdown state. The device also acts as an
>>> +  interrupt controller for managing interrupts from the VRS.
>>> +
>>> +properties:
>>> +  compatible:
>>> +    const: nvidia,vrs10-rtc
>>
>> Nothing improved. You never replied to comments and then replaced one
>> redundant word into other redundant word.
>>
>> Respond to review or implement it fully, not partially.
>>
>> Or add COMPLETE bindings, not partial ones. See writing bindings doc.
> 
> OK, right so the DT binding should describe the overall PMIC device, 
> even though the driver needs to support the RTC.


This is not a driver patch. This is patch for hardware. Sending
incomplete pieces of a device, without complete picture is really not
the right way. Knowing this is part of PMIC this should be rejected, but
how can we decide on that if contributor never tells us this is a part
of PMIC?

> 
> Shubhi, is vrs10 the version of the VRS spec for the PMIC device or just 
> the RTC portion? If it is, the maybe 'nvidia,vrs10' is sufficient here.
> 
> Jon
> 


Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v5 1/4] dt-bindings: rtc: Document NVIDIA VRS RTC
  2025-07-24 10:06       ` Krzysztof Kozlowski
@ 2025-07-24 10:50         ` Jon Hunter
  2025-07-25 15:58           ` Shubhi Garg
       [not found]           ` <424cd602-412f-4981-9b7f-9d04d769b3c7@nvidia.com>
  0 siblings, 2 replies; 11+ messages in thread
From: Jon Hunter @ 2025-07-24 10:50 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Shubhi Garg
  Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Catalin Marinas,
	Will Deacon, Alexandre Belloni, devicetree, linux-arm-kernel,
	linux-rtc, linux-tegra


On 24/07/2025 11:06, Krzysztof Kozlowski wrote:
> On 24/07/2025 11:41, Jon Hunter wrote:
>>
>> On 24/07/2025 08:59, Krzysztof Kozlowski wrote:
>>> On Wed, Jul 23, 2025 at 01:03:40PM +0000, Shubhi Garg wrote:
>>>> +description:
>>>> +  NVIDIA VRS (Voltage Regulator Specification) RTC provides 32kHz RTC clock
>>>> +  support with backup battery for system timing. It provides alarm functionality
>>>> +  to wake system from suspend and shutdown state. The device also acts as an
>>>> +  interrupt controller for managing interrupts from the VRS.
>>>> +
>>>> +properties:
>>>> +  compatible:
>>>> +    const: nvidia,vrs10-rtc
>>>
>>> Nothing improved. You never replied to comments and then replaced one
>>> redundant word into other redundant word.
>>>
>>> Respond to review or implement it fully, not partially.
>>>
>>> Or add COMPLETE bindings, not partial ones. See writing bindings doc.
>>
>> OK, right so the DT binding should describe the overall PMIC device,
>> even though the driver needs to support the RTC.
> 
> 
> This is not a driver patch. This is patch for hardware. Sending
> incomplete pieces of a device, without complete picture is really not
> the right way. Knowing this is part of PMIC this should be rejected, but
> how can we decide on that if contributor never tells us this is a part
> of PMIC?

Yes I understand that this is not a driver patch and must describe the 
hardware. It is a simple misunderstanding because it was rejected as an 
MFD, but we should not have then made the DT look like only a RTC 
device. This is a mistake on our side and we will fix.

Jon

-- 
nvpublic



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v5 1/4] dt-bindings: rtc: Document NVIDIA VRS RTC
  2025-07-24 10:50         ` Jon Hunter
@ 2025-07-25 15:58           ` Shubhi Garg
       [not found]           ` <424cd602-412f-4981-9b7f-9d04d769b3c7@nvidia.com>
  1 sibling, 0 replies; 11+ messages in thread
From: Shubhi Garg @ 2025-07-25 15:58 UTC (permalink / raw)
  To: Jon Hunter, Krzysztof Kozlowski
  Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Catalin Marinas,
	Will Deacon, Alexandre Belloni, devicetree, linux-arm-kernel,
	linux-rtc, linux-tegra, Shubhi Garg



On 24/07/25 4:20 pm, Jon Hunter wrote:
> 
> On 24/07/2025 11:06, Krzysztof Kozlowski wrote:
>> On 24/07/2025 11:41, Jon Hunter wrote:
>>>
>>> On 24/07/2025 08:59, Krzysztof Kozlowski wrote:
>>>> On Wed, Jul 23, 2025 at 01:03:40PM +0000, Shubhi Garg wrote:
>>>>> +description:
>>>>> +  NVIDIA VRS (Voltage Regulator Specification) RTC provides 32kHz 
>>>>> RTC clock
>>>>> +  support with backup battery for system timing. It provides alarm 
>>>>> functionality
>>>>> +  to wake system from suspend and shutdown state. The device also 
>>>>> acts as an
>>>>> +  interrupt controller for managing interrupts from the VRS.
>>>>> +
>>>>> +properties:
>>>>> +  compatible:
>>>>> +    const: nvidia,vrs10-rtc
>>>>
>>>> Nothing improved. You never replied to comments and then replaced one
>>>> redundant word into other redundant word.
>>>>
>>>> Respond to review or implement it fully, not partially.
>>>>
>>>> Or add COMPLETE bindings, not partial ones. See writing bindings doc.
>>>
>>> OK, right so the DT binding should describe the overall PMIC device,
>>> even though the driver needs to support the RTC.

VRS-10 is the device name. VRS-10 has an I2C interface and implements a 
power sequencer, RTC , 32kHZ clock output. From software perspective, we 
are handling VRS-10 interrupts and providing RTC driver support. I will 
add complete VRS-10 information in bindings.

>>
>> This is not a driver patch. This is patch for hardware. Sending
>> incomplete pieces of a device, without complete picture is really not
>> the right way. Knowing this is part of PMIC this should be rejected, but
>> how can we decide on that if contributor never tells us this is a part
>> of PMIC?
> 
> Yes I understand that this is not a driver patch and must describe the 
> hardware. It is a simple misunderstanding because it was rejected as an 
> MFD, but we should not have then made the DT look like only a RTC 
> device. This is a mistake on our side and we will fix.
> 

I acknowledge, will improve bindings by describing VRS-10 hardware for 
clear understanding.

-- 
Regards,
Shubhi



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v5 1/4] dt-bindings: rtc: Document NVIDIA VRS RTC
       [not found]           ` <424cd602-412f-4981-9b7f-9d04d769b3c7@nvidia.com>
@ 2025-08-05  6:18             ` Shubhi Garg
  0 siblings, 0 replies; 11+ messages in thread
From: Shubhi Garg @ 2025-08-05  6:18 UTC (permalink / raw)
  To: Jon Hunter, Krzysztof Kozlowski
  Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Catalin Marinas,
	Will Deacon, Alexandre Belloni, devicetree, linux-arm-kernel,
	linux-rtc, linux-tegra



On 25/07/25 7:09 pm, Shubhi Garg wrote:
> 
> On 24/07/25 4:20 pm, Jon Hunter wrote:
>>
>> On 24/07/2025 11:06, Krzysztof Kozlowski wrote:
>>> On 24/07/2025 11:41, Jon Hunter wrote:
>>>>
>>>> On 24/07/2025 08:59, Krzysztof Kozlowski wrote:
>>>>> On Wed, Jul 23, 2025 at 01:03:40PM +0000, Shubhi Garg wrote:
>>>>>> +description:
>>>>>> +  NVIDIA VRS (Voltage Regulator Specification) RTC provides 32kHz 
>>>>>> RTC clock
>>>>>> +  support with backup battery for system timing. It provides 
>>>>>> alarm functionality
>>>>>> +  to wake system from suspend and shutdown state. The device also 
>>>>>> acts as an
>>>>>> +  interrupt controller for managing interrupts from the VRS.
>>>>>> +
>>>>>> +properties:
>>>>>> +  compatible:
>>>>>> +    const: nvidia,vrs10-rtc
>>>>>
>>>>> Nothing improved. You never replied to comments and then replaced one
>>>>> redundant word into other redundant word.
>>>>>
>>>>> Respond to review or implement it fully, not partially.
>>>>>
>>>>> Or add COMPLETE bindings, not partial ones. See writing bindings doc.
>>>>
>>>> OK, right so the DT binding should describe the overall PMIC device,
>>>> even though the driver needs to support the RTC.
> VRS-10 is the device name. VRS-10 has an I2C interface and implements a 
> Power Sequencer,
> RTC , 32kHZ clock output. From software perspective, we are handling 
> VRS-10 interrupts and
> providing RTC driver support. I will add complete VRS-10 information in 
> bindings.

VRS-10 hardware specifications are completely designed by NVIDIA.
NVIDIA is getting chip manufactured by multiple vendors.
Therefore, part number and vendor ID varies on each NVIDIA platform 
revisions.
But, our software is independent of vendor details.
So, is it okay to keep compatible string as "nvidia, vrs-10" in VRS-10 
RTC driver ?


-- 
Regards,
Shubhi



^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2025-08-05  6:24 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-07-23 13:03 [PATCH v5 0/4] Add NVIDIA VRS RTC support Shubhi Garg
2025-07-23 13:03 ` [PATCH v5 1/4] dt-bindings: rtc: Document NVIDIA VRS RTC Shubhi Garg
2025-07-24  7:59   ` Krzysztof Kozlowski
2025-07-24  9:41     ` Jon Hunter
2025-07-24 10:06       ` Krzysztof Kozlowski
2025-07-24 10:50         ` Jon Hunter
2025-07-25 15:58           ` Shubhi Garg
     [not found]           ` <424cd602-412f-4981-9b7f-9d04d769b3c7@nvidia.com>
2025-08-05  6:18             ` Shubhi Garg
2025-07-23 13:03 ` [PATCH v5 2/4] arm64: tegra: Add device-tree node for NVVRS RTC Shubhi Garg
2025-07-23 13:03 ` [PATCH v5 3/4] rtc: nvvrs: add NVIDIA VRS device driver Shubhi Garg
2025-07-23 13:03 ` [PATCH v5 4/4] arm64: defconfig: enable NVIDIA VRS RTC Shubhi Garg

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).