[PATCH 0/2] rtc: Add Qualcomm PMIC RTC-GLINK support for Glymur

[PATCH 0/2] rtc: Add Qualcomm PMIC RTC-GLINK support for Glymur

[Kamal Wadhwa]

On some targets like Glymur, the permission for receving the RTC
alarms is routed to charger firmware running on remote subsystem.

This is because Glymur boards have to  support MSFT's `TAD feature`
on the windows OS devices.

So rtc-pm8xxx.c driver cannot recieve RTC alarm IRQs via SPMI on
Glymur. So in order to make the IRQ available on the Linux kernel,
the PMIC RTC Alarm IRQ must be routed to APPS via remote subsystem.

To implement this, add support for a New Qualcomm PMIC GLINK RTC
client, which will request for RTC set/get calls for time/alarm IRQ
 via the existing pmic_glink interface to the charger firmware.

Also, add a DT property to explicily support enabling the RTC
writes, keep RTC writes disabled by default to align behaviour
with the existing rtc-pm8xxx.c driver.

Signed-off-by: Kamal Wadhwa <kamal.wadhwa@oss.qualcomm.com>
---
Kamal Wadhwa (2):
      dt-bindings: soc: qcom: qcom,pmic-glink: Document RTC client properties
      rtc: qcom: rtc-glink: Add PMIC GLINK RTC driver and extend pmic_glink

 .../bindings/soc/qcom/qcom,pmic-glink.yaml         |   5 +
 MAINTAINERS                                        |   7 +
 drivers/rtc/Kconfig                                |  10 +
 drivers/rtc/Makefile                               |   1 +
 drivers/rtc/rtc-qcom-glink.c                       | 690 +++++++++++++++++++++
 drivers/soc/qcom/pmic_glink.c                      |  22 +-
 include/linux/soc/qcom/pmic_glink.h                |   1 +
 7 files changed, 735 insertions(+), 1 deletion(-)
---
base-commit: 1701fda2f58e345c050f4309971bdc07cd6146ba
change-id: 20260806-rtc-glink-cleanup-038163045afd

Best regards,
--  
Kamal Wadhwa <kamal.wadhwa@oss.qualcomm.com>

[PATCH 1/2] dt-bindings: soc: qcom: qcom,pmic-glink: Document RTC client properties

[Kamal Wadhwa]

The pmic_glink driver can now register an RTC auxiliary device on
glymur, whose PMIC exposes an RTC over GLINK (see rtc-glink driver).
Document the allow-set-time flag it consumes from the parent
pmic-glink node.

Assisted-by: Claude:claude-sonnet-5
Signed-off-by: Raj Aryan <raryan@qti.qualcomm.com>
Signed-off-by: Kamal Wadhwa <kamal.wadhwa@oss.qualcomm.com>
---
 Documentation/devicetree/bindings/soc/qcom/qcom,pmic-glink.yaml | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,pmic-glink.yaml b/Documentation/devicetree/bindings/soc/qcom/qcom,pmic-glink.yaml
index 16ad4f5698fc..38db1ff473c0 100644
--- a/Documentation/devicetree/bindings/soc/qcom/qcom,pmic-glink.yaml
+++ b/Documentation/devicetree/bindings/soc/qcom/qcom,pmic-glink.yaml
@@ -79,6 +79,11 @@ properties:
       - const: charge_limit_end
       - const: charge_limit_delta
 
+  allow-set-time:
+    $ref: /schemas/types.yaml#/definitions/flag
+    description:
+      Indicates that the setting of RTC time is allowed by the host CPU.
+
 patternProperties:
   '^connector@\d$':
     $ref: /schemas/connector/usb-connector.yaml#

-- 
2.43.0

[PATCH 2/2] rtc: qcom: rtc-glink: Add PMIC GLINK RTC driver and extend pmic_glink

[Kamal Wadhwa]

Add a new RTC driver that communicates with the PMIC firmware over
the GLINK transport layer. The driver supports reading time and
alarm functionality via GET_PROPERTY and SET_PROPERTY opcodes.

Time is read via GET_PROPERTY (opcode 0x62) using QCOM_RTC_GLINK_TIME
property (0x00). Alarm set and enable are handled via SET_PROPERTY
(opcode 0x63) using QCOM_RTC_GLINK_ALARM and QCOM_RTC_GLINK_ALARM_ENABLE
properties respectively.

GET_PROPERTY and SET_PROPERTY responses share the same firmware message
layout, with the firmware echoing back the property value. Both opcodes
are handled by a common callback path accordingly.

Also extend pmic_glink to register the RTC as a PMIC GLINK client and
add PMIC GLINK RTC client support for glymur.

Assisted-by: Claude:claude-sonnet-5
Signed-off-by: Raj Aryan <raryan@qti.qualcomm.com>
Signed-off-by: Kamal Wadhwa <kamal.wadhwa@oss.qualcomm.com>
---
 MAINTAINERS                         |   7 +
 drivers/rtc/Kconfig                 |  10 +
 drivers/rtc/Makefile                |   1 +
 drivers/rtc/rtc-qcom-glink.c        | 690 ++++++++++++++++++++++++++++++++++++
 drivers/soc/qcom/pmic_glink.c       |  22 +-
 include/linux/soc/qcom/pmic_glink.h |   1 +
 6 files changed, 730 insertions(+), 1 deletion(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 5f178a5ff026..752cf667b8ad 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -22505,6 +22505,13 @@ S:	Maintained
 F:	Documentation/devicetree/bindings/power/supply/qcom,pmi8998-charger.yaml
 F:	drivers/power/supply/qcom_smbx.c
 
+QUALCOMM PMIC GLINK RTC DRIVER
+M:	Kamal Wadhwa <kamal.wadhwa@oss.qualcomm.com>
+L:	linux-rtc@vger.kernel.org
+L:	linux-arm-msm@vger.kernel.org
+S:	Maintained
+F:	drivers/rtc/rtc-qcom-glink.c
+
 QUALCOMM PPE DRIVER
 M:	Luo Jie <jie.luo@oss.qualcomm.com>
 L:	netdev@vger.kernel.org
diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
index 01def8231873..3ac39704b2e1 100644
--- a/drivers/rtc/Kconfig
+++ b/drivers/rtc/Kconfig
@@ -1836,6 +1836,16 @@ config RTC_DRV_PM8XXX
 	  To compile this driver as a module, choose M here: the
 	  module will be called rtc-pm8xxx.
 
+config RTC_DRV_QCOM_GLINK
+	tristate "Qualcomm PMIC GLINK RTC"
+	depends on QCOM_PMIC_GLINK
+	help
+	  Say Y here to enable support for the RTC exposed by the PMIC
+	  over the GLINK transport on Qualcomm platforms.
+
+	  This driver can also be built as a module. If so, the module
+	  will be called rtc-qcom-glink.
+
 config RTC_DRV_TEGRA
 	tristate "NVIDIA Tegra Internal RTC driver"
 	depends on ARCH_TEGRA || COMPILE_TEST
diff --git a/drivers/rtc/Makefile b/drivers/rtc/Makefile
index 0347645b021f..8215a1fc29fb 100644
--- a/drivers/rtc/Makefile
+++ b/drivers/rtc/Makefile
@@ -73,6 +73,7 @@ obj-$(CONFIG_RTC_DRV_FM3130)	+= rtc-fm3130.o
 obj-$(CONFIG_RTC_DRV_FSL_FTM_ALARM)	+= rtc-fsl-ftm-alarm.o
 obj-$(CONFIG_RTC_DRV_FTRTC010)	+= rtc-ftrtc010.o
 obj-$(CONFIG_RTC_DRV_GENERIC)	+= rtc-generic.o
+obj-$(CONFIG_RTC_DRV_QCOM_GLINK) += rtc-qcom-glink.o
 obj-$(CONFIG_RTC_DRV_GOLDFISH)	+= rtc-goldfish.o
 obj-$(CONFIG_RTC_DRV_HID_SENSOR_TIME) += rtc-hid-sensor-time.o
 obj-$(CONFIG_RTC_DRV_HYM8563)	+= rtc-hym8563.o
diff --git a/drivers/rtc/rtc-qcom-glink.c b/drivers/rtc/rtc-qcom-glink.c
new file mode 100644
index 000000000000..611f06a3cc5c
--- /dev/null
+++ b/drivers/rtc/rtc-qcom-glink.c
@@ -0,0 +1,690 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
+ */
+#include <linux/auxiliary_bus.h>
+#include <linux/completion.h>
+#include <linux/device.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/property.h>
+#include <linux/rtc.h>
+#include <linux/slab.h>
+#include <linux/soc/qcom/pdr.h>
+#include <linux/soc/qcom/pmic_glink.h>
+#include <linux/timekeeping.h>
+#include <linux/workqueue.h>
+
+#define RTC_GLINK_SET_PROPERTY		0x63
+#define RTC_GLINK_GET_PROPERTY		0x65
+#define RTC_GLINK_GET_RTC_TICKS		0x67
+#define RTC_GLINK_GET_REAL_TIME		0x68
+#define RTC_GLINK_SET_REAL_TIME		0x69
+#define RTC_GLINK_ALARM_EXPIRED		0x6A
+
+enum qcom_rtc_glink_properties {
+	QCOM_RTC_GLINK_TIME = 0,
+	QCOM_RTC_GLINK_ALARM_TIME,
+	QCOM_RTC_GLINK_ALARM_ENABLE,
+};
+
+struct qcom_rtc_glink_msg {
+	struct pmic_glink_hdr	hdr;
+	__le32			property;
+	__le32			value;
+};
+
+struct qcom_rtc_glink_generic_req {
+	struct pmic_glink_hdr	hdr;
+};
+
+struct qcom_rtc_glink_status_resp {
+	struct pmic_glink_hdr	hdr;
+	__le32			return_status;
+};
+
+struct qcom_rtc_glink_ticks_resp {
+	struct pmic_glink_hdr	hdr;
+	__le32			return_status;
+	__le32			rtc_ticks;
+};
+
+struct qcom_rtc_glink_real_time_resp {
+	struct pmic_glink_hdr	hdr;
+	__le32			return_status;
+	__le32			real_time_data[4];
+};
+
+struct qcom_rtc_glink_set_real_time_req {
+	struct pmic_glink_hdr	hdr;
+	__le32			real_time_data[4];
+};
+
+struct qcom_rtc_glink {
+	struct device		*dev;
+	struct pmic_glink_client *client;
+	struct rtc_device	*rtc;
+
+	/* Serializes requests: only one may be in flight at a time */
+	struct mutex		lock;
+	struct completion	ack;
+	struct work_struct	alarm_work;
+
+	/* Protects service_up and request_pending across callback contexts */
+	spinlock_t		state_lock;
+	bool			service_up;
+	bool			request_pending;
+
+	int			error;
+	u32			resp_value;
+	u32			offset;
+	struct rtc_time		resp_tm;
+	bool			resp_valid;
+	bool			allow_set_time;
+};
+
+static int qcom_rtc_glink_request(struct qcom_rtc_glink *rtc_glink,
+				  void *data, size_t len)
+{
+	unsigned long flags;
+	unsigned long left;
+	int ret;
+
+	/*
+	 * Reinit the completion before request_pending becomes visible to
+	 * qcom_rtc_glink_pdr_tify(), so a concurrent SSR down-transition
+	 * can't complete() a stale completion that gets wiped out by
+	 * reinit_completion() right after.
+	 */
+	reinit_completion(&rtc_glink->ack);
+	rtc_glink->error = 0;
+	rtc_glink->resp_valid = false;
+
+	spin_lock_irqsave(&rtc_glink->state_lock, flags);
+	if (!rtc_glink->service_up) {
+		spin_unlock_irqrestore(&rtc_glink->state_lock, flags);
+		return -ECONNRESET;
+	}
+	rtc_glink->request_pending = true;
+	spin_unlock_irqrestore(&rtc_glink->state_lock, flags);
+
+	ret = pmic_glink_send(rtc_glink->client, data, len);
+	if (ret < 0) {
+		spin_lock_irqsave(&rtc_glink->state_lock, flags);
+		rtc_glink->request_pending = false;
+		spin_unlock_irqrestore(&rtc_glink->state_lock, flags);
+		return ret;
+	}
+
+	left = wait_for_completion_timeout(&rtc_glink->ack, HZ);
+	spin_lock_irqsave(&rtc_glink->state_lock, flags);
+	rtc_glink->request_pending = false;
+	spin_unlock_irqrestore(&rtc_glink->state_lock, flags);
+	if (!left)
+		return -ETIMEDOUT;
+
+	return rtc_glink->error;
+}
+
+static int qcom_rtc_glink_set_property(struct qcom_rtc_glink *rtc_glink,
+				       u32 property, u32 value)
+{
+	struct qcom_rtc_glink_msg msg = {
+		.hdr.owner = cpu_to_le32(PMIC_GLINK_OWNER_RTC),
+		.hdr.type = cpu_to_le32(PMIC_GLINK_REQ_RESP),
+		.hdr.opcode = cpu_to_le32(RTC_GLINK_SET_PROPERTY),
+		.property = cpu_to_le32(property),
+		.value = cpu_to_le32(value),
+	};
+	int ret;
+
+	dev_dbg(rtc_glink->dev,
+		"TX opcode=0x%x property=%u value=%u\n",
+		RTC_GLINK_SET_PROPERTY, property, value);
+	mutex_lock(&rtc_glink->lock);
+	ret = qcom_rtc_glink_request(rtc_glink, &msg, sizeof(msg));
+	mutex_unlock(&rtc_glink->lock);
+	return ret;
+}
+
+static int qcom_rtc_glink_get_property(struct qcom_rtc_glink *rtc_glink,
+				       u32 property, u32 *value)
+{
+	struct qcom_rtc_glink_msg msg = {
+		.hdr.owner = cpu_to_le32(PMIC_GLINK_OWNER_RTC),
+		.hdr.type = cpu_to_le32(PMIC_GLINK_REQ_RESP),
+		.hdr.opcode = cpu_to_le32(RTC_GLINK_GET_PROPERTY),
+		.property = cpu_to_le32(property),
+	};
+	int ret;
+
+	dev_dbg(rtc_glink->dev,
+		"TX opcode=0x%x property=%u\n",
+		RTC_GLINK_GET_PROPERTY, property);
+	mutex_lock(&rtc_glink->lock);
+	ret = qcom_rtc_glink_request(rtc_glink, &msg, sizeof(msg));
+	if (!ret) {
+		if (!rtc_glink->resp_valid)
+			ret = -EIO;
+		else
+			*value = rtc_glink->resp_value;
+	}
+	mutex_unlock(&rtc_glink->lock);
+	return ret;
+}
+
+static int qcom_rtc_glink_get_ticks(struct qcom_rtc_glink *rtc_glink,
+				    u32 *ticks)
+{
+	struct qcom_rtc_glink_generic_req msg = {
+		.hdr.owner = cpu_to_le32(PMIC_GLINK_OWNER_RTC),
+		.hdr.type = cpu_to_le32(PMIC_GLINK_REQ_RESP),
+		.hdr.opcode = cpu_to_le32(RTC_GLINK_GET_RTC_TICKS),
+	};
+	int ret;
+
+	dev_dbg(rtc_glink->dev, "TX opcode=0x%x\n", RTC_GLINK_GET_RTC_TICKS);
+	mutex_lock(&rtc_glink->lock);
+	ret = qcom_rtc_glink_request(rtc_glink, &msg, sizeof(msg));
+	if (!ret) {
+		if (!rtc_glink->resp_valid)
+			ret = -EIO;
+		else
+			*ticks = rtc_glink->resp_value;
+	}
+	mutex_unlock(&rtc_glink->lock);
+	return ret;
+}
+
+static int qcom_rtc_glink_get_real_time(struct qcom_rtc_glink *rtc_glink,
+					struct rtc_time *tm)
+{
+	struct qcom_rtc_glink_generic_req msg = {
+		.hdr.owner = cpu_to_le32(PMIC_GLINK_OWNER_RTC),
+		.hdr.type = cpu_to_le32(PMIC_GLINK_REQ_RESP),
+		.hdr.opcode = cpu_to_le32(RTC_GLINK_GET_REAL_TIME),
+	};
+	int ret;
+
+	dev_dbg(rtc_glink->dev, "TX opcode=0x%x\n", RTC_GLINK_GET_REAL_TIME);
+	mutex_lock(&rtc_glink->lock);
+	ret = qcom_rtc_glink_request(rtc_glink, &msg, sizeof(msg));
+	if (!ret) {
+		if (!rtc_glink->resp_valid)
+			ret = -EIO;
+		else
+			*tm = rtc_glink->resp_tm;
+	}
+	mutex_unlock(&rtc_glink->lock);
+	return ret;
+}
+
+static int qcom_rtc_glink_set_real_time(struct qcom_rtc_glink *rtc_glink,
+					struct rtc_time *tm)
+{
+	struct qcom_rtc_glink_set_real_time_req msg = {};
+	u32 w0, w1;
+	int ret;
+
+	w0 = ((tm->tm_year + 1900) & 0xffff) |
+	     (((tm->tm_mon + 1) & 0xff) << 16) |
+	     ((tm->tm_mday & 0xff) << 24);
+	w1 = (tm->tm_hour & 0xff) |
+	     ((tm->tm_min & 0xff) << 8) |
+	     ((tm->tm_sec & 0xff) << 16) |
+	     (1U << 24);
+	msg.hdr.owner = cpu_to_le32(PMIC_GLINK_OWNER_RTC);
+	msg.hdr.type = cpu_to_le32(PMIC_GLINK_REQ_RESP);
+	msg.hdr.opcode = cpu_to_le32(RTC_GLINK_SET_REAL_TIME);
+	msg.real_time_data[0] = cpu_to_le32(w0);
+	msg.real_time_data[1] = cpu_to_le32(w1);
+	msg.real_time_data[2] = 0;
+	msg.real_time_data[3] = 0;
+	dev_dbg(rtc_glink->dev,
+		"TX opcode=0x%x data=%08x %08x\n",
+		RTC_GLINK_SET_REAL_TIME, w0, w1);
+	mutex_lock(&rtc_glink->lock);
+	ret = qcom_rtc_glink_request(rtc_glink, &msg, sizeof(msg));
+	mutex_unlock(&rtc_glink->lock);
+	return ret;
+}
+
+static int qcom_rtc_glink_get_time(struct qcom_rtc_glink *rtc_glink,
+				   struct rtc_time *time)
+{
+	u32 ticks;
+	int ret;
+
+	ret = qcom_rtc_glink_get_real_time(rtc_glink, time);
+	if (!ret)
+		return 0;
+	dev_warn(rtc_glink->dev, "0x68 failed (%d), falling back\n", ret);
+
+	ret = qcom_rtc_glink_get_ticks(rtc_glink, &ticks);
+	if (!ret) {
+		rtc_time64_to_tm((time64_t)ticks + rtc_glink->offset, time);
+		return 0;
+	}
+
+	ret = qcom_rtc_glink_get_property(rtc_glink, QCOM_RTC_GLINK_TIME,
+					  &ticks);
+	if (!ret && ticks != U32_MAX) {
+		rtc_time64_to_tm((time64_t)ticks + rtc_glink->offset, time);
+		return 0;
+	}
+
+	dev_err(rtc_glink->dev, "all time sources failed\n");
+	return -EIO;
+}
+
+static int qcom_rtc_glink_set_time(struct qcom_rtc_glink *rtc_glink,
+				   struct rtc_time *time)
+{
+	time64_t t = rtc_tm_to_time64(time);
+	u32 ticks;
+	int ret;
+
+	if (!rtc_glink->allow_set_time)
+		return -EOPNOTSUPP;
+
+	ret = qcom_rtc_glink_set_real_time(rtc_glink, time);
+	if (ret)
+		dev_warn(rtc_glink->dev, "0x69 failed (%d)\n", ret);
+
+	ret = qcom_rtc_glink_set_property(rtc_glink, QCOM_RTC_GLINK_TIME, (u32)t);
+	if (ret)
+		return ret;
+
+	ret = qcom_rtc_glink_get_ticks(rtc_glink, &ticks);
+	if (!ret && ticks != U32_MAX) {
+		rtc_glink->offset = (u32)t - ticks;
+	} else {
+		dev_warn(rtc_glink->dev, "no tick source for offset\n");
+		return -EIO;
+	}
+
+	return 0;
+}
+
+static int qcom_rtc_glink_set_alarm_en(struct qcom_rtc_glink *rtc_glink,
+				       int enabled)
+{
+	return qcom_rtc_glink_set_property(rtc_glink,
+					   QCOM_RTC_GLINK_ALARM_ENABLE,
+					   enabled);
+}
+
+static int qcom_rtc_glink_set_alarm(struct qcom_rtc_glink *rtc_glink,
+				    struct rtc_wkalrm *alarm)
+{
+	time64_t alarm_t = rtc_tm_to_time64(&alarm->time);
+	struct rtc_time now_tm;
+	time64_t now_real, secs_until_alarm;
+	u32 fw_current, fw_alarm;
+	int ret;
+
+	ret = qcom_rtc_glink_get_time(rtc_glink, &now_tm);
+	if (ret)
+		return ret;
+	now_real = rtc_tm_to_time64(&now_tm);
+	secs_until_alarm = alarm_t - now_real;
+	dev_dbg(rtc_glink->dev,
+		"set_alarm: now_real=%lld alarm_t=%lld delta=%lld\n",
+		now_real, alarm_t, secs_until_alarm);
+	if (secs_until_alarm < 0)
+		return -EINVAL;
+	if (secs_until_alarm > U32_MAX)
+		return -ERANGE;
+
+	ret = qcom_rtc_glink_get_property(rtc_glink, QCOM_RTC_GLINK_TIME,
+					  &fw_current);
+	if (ret)
+		return ret;
+	if (fw_current == U32_MAX)
+		fw_current = 0;
+	fw_alarm = fw_current + (u32)secs_until_alarm;
+	dev_dbg(rtc_glink->dev,
+		"set_alarm: fw_current=%u fw_alarm=%u\n",
+		fw_current, fw_alarm);
+
+	ret = qcom_rtc_glink_set_alarm_en(rtc_glink, 0);
+	if (ret)
+		return ret;
+
+	ret = qcom_rtc_glink_set_property(rtc_glink,
+					  QCOM_RTC_GLINK_ALARM_TIME, fw_alarm);
+	if (ret)
+		return ret;
+
+	return qcom_rtc_glink_set_alarm_en(rtc_glink, alarm->enabled);
+}
+
+static int qcom_rtc_glink_get_alarm(struct qcom_rtc_glink *rtc_glink,
+				    struct rtc_wkalrm *alarm)
+{
+	u32 fw_alarm_time, fw_current, alarm_en;
+	struct rtc_time now_tm;
+	time64_t now_real, secs_until_alarm, alarm_real;
+	int ret;
+
+	ret = qcom_rtc_glink_get_property(rtc_glink,
+					  QCOM_RTC_GLINK_ALARM_TIME,
+					  &fw_alarm_time);
+	if (ret)
+		return ret;
+
+	ret = qcom_rtc_glink_get_property(rtc_glink, QCOM_RTC_GLINK_TIME,
+					  &fw_current);
+	if (ret)
+		return ret;
+
+	ret = qcom_rtc_glink_get_property(rtc_glink,
+					  QCOM_RTC_GLINK_ALARM_ENABLE,
+					  &alarm_en);
+	if (ret)
+		return ret;
+	if (fw_current == U32_MAX)
+		fw_current = 0;
+
+	ret = qcom_rtc_glink_get_time(rtc_glink, &now_tm);
+	if (ret)
+		return ret;
+	now_real = rtc_tm_to_time64(&now_tm);
+	secs_until_alarm = (time64_t)fw_alarm_time - (time64_t)fw_current;
+	alarm_real = now_real + secs_until_alarm;
+	dev_dbg(rtc_glink->dev,
+		"get_alarm: now_real=%lld fw_current=%u fw_alarm=%u delta=%lld\n",
+		now_real, fw_current, fw_alarm_time, secs_until_alarm);
+	rtc_time64_to_tm(alarm_real, &alarm->time);
+	alarm->enabled = !!alarm_en;
+	return 0;
+}
+
+static void qcom_rtc_glink_alarm_work(struct work_struct *work)
+{
+	struct qcom_rtc_glink *rtc_glink =
+		container_of(work, struct qcom_rtc_glink, alarm_work);
+	int ret;
+
+	ret = qcom_rtc_glink_set_alarm_en(rtc_glink, 0);
+	if (ret)
+		dev_err(rtc_glink->dev,
+			"failed to disable alarm after expiry (%d)\n", ret);
+	rtc_update_irq(rtc_glink->rtc, 1, RTC_IRQF | RTC_AF);
+}
+
+static void qcom_rtc_glink_callback(const void *data, size_t len, void *priv)
+{
+	struct qcom_rtc_glink *rtc_glink = priv;
+	const struct pmic_glink_hdr *hdr = data;
+	unsigned long flags;
+	bool pending;
+
+	if (len < sizeof(*hdr))
+		return;
+
+	dev_dbg(rtc_glink->dev,
+		"RX opcode=0x%x type=0x%x owner=0x%x len=%zu\n",
+		le32_to_cpu(hdr->opcode), le32_to_cpu(hdr->type),
+		le32_to_cpu(hdr->owner), len);
+
+	if (le32_to_cpu(hdr->opcode) == RTC_GLINK_ALARM_EXPIRED) {
+		dev_info(rtc_glink->dev, "alarm expired\n");
+		schedule_work(&rtc_glink->alarm_work);
+		return;
+	}
+
+	/*
+	 * Hold state_lock across the pending check and the response fields
+	 * below: qcom_rtc_glink_pdr_notify() writes the same error/resp_*
+	 * fields under this lock when it force-completes a request on an
+	 * SSR down-transition, and the two must not race. Dropping the
+	 * response here if !pending also covers requests that already
+	 * timed out or were aborted; qcom_rtc_glink_request() will call
+	 * reinit_completion() again before this response could be mistaken
+	 * for a later request's.
+	 */
+	spin_lock_irqsave(&rtc_glink->state_lock, flags);
+	pending = rtc_glink->request_pending;
+
+	if (!pending) {
+		spin_unlock_irqrestore(&rtc_glink->state_lock, flags);
+		dev_dbg(rtc_glink->dev,
+			"dropping stale response opcode=0x%x\n",
+			le32_to_cpu(hdr->opcode));
+		return;
+	}
+
+	switch (le32_to_cpu(hdr->opcode)) {
+	case RTC_GLINK_GET_PROPERTY:
+		fallthrough;
+	case RTC_GLINK_SET_PROPERTY: {
+		const struct qcom_rtc_glink_msg *msg = data;
+
+		if (len < sizeof(struct pmic_glink_hdr) + sizeof(__le32)) {
+			rtc_glink->error = -EINVAL;
+			break;
+		}
+		rtc_glink->resp_value = le32_to_cpu(msg->property);
+		rtc_glink->resp_valid = true;
+		rtc_glink->error = 0;
+		dev_dbg(rtc_glink->dev, "ACK opcode=0x%x resp_value=%u\n",
+			le32_to_cpu(hdr->opcode), rtc_glink->resp_value);
+		break;
+	}
+
+	case RTC_GLINK_GET_RTC_TICKS: {
+		const struct qcom_rtc_glink_ticks_resp *resp = data;
+
+		if (len < sizeof(*resp)) {
+			rtc_glink->error = -EINVAL;
+			break;
+		}
+		if (le32_to_cpu(resp->return_status)) {
+			rtc_glink->error = -EIO;
+			break;
+		}
+		rtc_glink->resp_value = le32_to_cpu(resp->rtc_ticks);
+		rtc_glink->resp_valid = true;
+		rtc_glink->error = 0;
+		dev_dbg(rtc_glink->dev, "ACK opcode=0x%x ticks=%u\n",
+			RTC_GLINK_GET_RTC_TICKS, rtc_glink->resp_value);
+		break;
+	}
+
+	case RTC_GLINK_GET_REAL_TIME: {
+		const struct qcom_rtc_glink_real_time_resp *resp = data;
+		u32 w0, w1;
+
+		if (len < sizeof(*resp)) {
+			rtc_glink->error = -EINVAL;
+			break;
+		}
+		if (le32_to_cpu(resp->return_status)) {
+			rtc_glink->error = -EIO;
+			break;
+		}
+		w0 = le32_to_cpu(resp->real_time_data[0]);
+		w1 = le32_to_cpu(resp->real_time_data[1]);
+		rtc_glink->resp_tm.tm_year = (w0 & 0xffff) - 1900;
+		rtc_glink->resp_tm.tm_mon = ((w0 >> 16) & 0xff) - 1;
+		rtc_glink->resp_tm.tm_mday = (w0 >> 24) & 0xff;
+		rtc_glink->resp_tm.tm_hour = w1 & 0xff;
+		rtc_glink->resp_tm.tm_min = (w1 >> 8) & 0xff;
+		rtc_glink->resp_tm.tm_sec = (w1 >> 16) & 0xff;
+		rtc_glink->resp_valid = true;
+		rtc_glink->error = 0;
+		dev_dbg(rtc_glink->dev,
+			"ACK opcode=0x%x %04d-%02d-%02d %02d:%02d:%02d\n",
+			RTC_GLINK_GET_REAL_TIME,
+			rtc_glink->resp_tm.tm_year + 1900,
+			rtc_glink->resp_tm.tm_mon + 1,
+			rtc_glink->resp_tm.tm_mday,
+			rtc_glink->resp_tm.tm_hour,
+			rtc_glink->resp_tm.tm_min,
+			rtc_glink->resp_tm.tm_sec);
+		break;
+	}
+
+	case RTC_GLINK_SET_REAL_TIME: {
+		const struct qcom_rtc_glink_status_resp *resp = data;
+
+		if (len < sizeof(*resp)) {
+			rtc_glink->error = -EINVAL;
+			break;
+		}
+		rtc_glink->error = le32_to_cpu(resp->return_status) ? -EIO : 0;
+		dev_dbg(rtc_glink->dev, "ACK opcode=0x%x status=%u\n",
+			RTC_GLINK_SET_REAL_TIME,
+			le32_to_cpu(resp->return_status));
+		break;
+	}
+
+	default:
+		spin_unlock_irqrestore(&rtc_glink->state_lock, flags);
+		dev_warn(rtc_glink->dev,
+			 "unhandled RX opcode=0x%x len=%zu\n",
+			 le32_to_cpu(hdr->opcode), len);
+		return;
+	}
+
+	spin_unlock_irqrestore(&rtc_glink->state_lock, flags);
+	complete(&rtc_glink->ack);
+}
+
+static void qcom_rtc_glink_pdr_notify(void *priv, int state)
+{
+	struct qcom_rtc_glink *rtc_glink = priv;
+	unsigned long flags;
+	bool up = (state == SERVREG_SERVICE_STATE_UP);
+
+	spin_lock_irqsave(&rtc_glink->state_lock, flags);
+	rtc_glink->service_up = up;
+	if (!up && rtc_glink->request_pending) {
+		rtc_glink->error = -ECONNRESET;
+		rtc_glink->resp_valid = false;
+		complete(&rtc_glink->ack);
+	}
+	spin_unlock_irqrestore(&rtc_glink->state_lock, flags);
+}
+
+static int qcom_rtc_glink_read_time(struct device *dev, struct rtc_time *tm)
+{
+	return qcom_rtc_glink_get_time(dev_get_drvdata(dev), tm);
+}
+
+static int qcom_rtc_glink_set_time_dev(struct device *dev, struct rtc_time *tm)
+{
+	return qcom_rtc_glink_set_time(dev_get_drvdata(dev), tm);
+}
+
+static int qcom_rtc_glink_read_alarm(struct device *dev,
+				     struct rtc_wkalrm *alrm)
+{
+	return qcom_rtc_glink_get_alarm(dev_get_drvdata(dev), alrm);
+}
+
+static int qcom_rtc_glink_set_alarm_dev(struct device *dev,
+					struct rtc_wkalrm *alrm)
+{
+	return qcom_rtc_glink_set_alarm(dev_get_drvdata(dev), alrm);
+}
+
+static int qcom_rtc_glink_alarm_irq_enable(struct device *dev,
+					   unsigned int enabled)
+{
+	return qcom_rtc_glink_set_alarm_en(dev_get_drvdata(dev), enabled);
+}
+
+static const struct rtc_class_ops qcom_rtc_glink_rtc_ops = {
+	.read_time = qcom_rtc_glink_read_time,
+	.set_time = qcom_rtc_glink_set_time_dev,
+	.read_alarm = qcom_rtc_glink_read_alarm,
+	.set_alarm = qcom_rtc_glink_set_alarm_dev,
+	.alarm_irq_enable = qcom_rtc_glink_alarm_irq_enable,
+};
+
+static const struct of_device_id qcom_rtc_glink_of_variants[] = {
+	{ .compatible = "qcom,glymur-pmic-glink" },
+	{}
+};
+
+static int qcom_rtc_glink_probe(struct auxiliary_device *adev,
+				const struct auxiliary_device_id *id)
+{
+	struct device *dev = &adev->dev;
+	struct qcom_rtc_glink *rtc_glink;
+
+	if (!of_match_device(qcom_rtc_glink_of_variants, dev->parent))
+		return -ENXIO;
+
+	rtc_glink = devm_kzalloc(dev, sizeof(*rtc_glink), GFP_KERNEL);
+	if (!rtc_glink)
+		return -ENOMEM;
+
+	rtc_glink->dev            = dev;
+	rtc_glink->allow_set_time = device_property_read_bool(dev->parent,
+							      "allow-set-time");
+
+	mutex_init(&rtc_glink->lock);
+	spin_lock_init(&rtc_glink->state_lock);
+	init_completion(&rtc_glink->ack);
+	INIT_WORK(&rtc_glink->alarm_work, qcom_rtc_glink_alarm_work);
+	dev_set_drvdata(dev, rtc_glink);
+
+	rtc_glink->client = devm_pmic_glink_client_alloc(dev,
+							 PMIC_GLINK_OWNER_RTC,
+							 qcom_rtc_glink_callback,
+							 qcom_rtc_glink_pdr_notify,
+							 rtc_glink);
+	if (IS_ERR(rtc_glink->client))
+		return dev_err_probe(dev, PTR_ERR(rtc_glink->client),
+				     "failed to allocate glink client\n");
+
+	pmic_glink_client_register(rtc_glink->client);
+
+	device_init_wakeup(dev, true);
+
+	rtc_glink->rtc = devm_rtc_allocate_device(dev);
+	if (IS_ERR(rtc_glink->rtc))
+		return dev_err_probe(dev, PTR_ERR(rtc_glink->rtc),
+				     "failed to allocate RTC device\n");
+
+	rtc_glink->rtc->ops      = &qcom_rtc_glink_rtc_ops;
+	rtc_glink->rtc->range_min = 0;
+	rtc_glink->rtc->range_max = U32_MAX;
+
+	return devm_rtc_register_device(rtc_glink->rtc);
+}
+
+static void qcom_rtc_glink_remove(struct auxiliary_device *adev)
+{
+	struct qcom_rtc_glink *rtc_glink = dev_get_drvdata(&adev->dev);
+	/*
+	 * Nothing left to arm alarm_work after this returns: the glink
+	 * client (and its callback/pdr_notify) is torn down by devm after
+	 * .remove() returns, and rtc_update_irq() below still targets a
+	 * live devm-managed rtc device.
+	 */
+	cancel_work_sync(&rtc_glink->alarm_work);
+}
+
+static const struct auxiliary_device_id qcom_rtc_glink_id_table[] = {
+	{ .name = "pmic_glink.rtc-glink" },
+	{}
+};
+
+MODULE_DEVICE_TABLE(auxiliary, qcom_rtc_glink_id_table);
+
+static struct auxiliary_driver qcom_rtc_glink_driver = {
+	.name = "qcom_pmic_rtc_glink",
+	.id_table = qcom_rtc_glink_id_table,
+	.probe = qcom_rtc_glink_probe,
+	.remove = qcom_rtc_glink_remove,
+};
+
+module_auxiliary_driver(qcom_rtc_glink_driver);
+MODULE_DESCRIPTION("Qualcomm PMIC GLINK RTC driver");
+MODULE_LICENSE("GPL");
diff --git a/drivers/soc/qcom/pmic_glink.c b/drivers/soc/qcom/pmic_glink.c
index 3042261578aa..7ee70a31ba15 100644
--- a/drivers/soc/qcom/pmic_glink.c
+++ b/drivers/soc/qcom/pmic_glink.c
@@ -21,6 +21,7 @@ enum {
 	PMIC_GLINK_CLIENT_BATT = 0,
 	PMIC_GLINK_CLIENT_ALTMODE,
 	PMIC_GLINK_CLIENT_UCSI,
+	PMIC_GLINK_CLIENT_RTC,
 };
 
 struct pmic_glink_data {
@@ -40,6 +41,7 @@ struct pmic_glink {
 	struct auxiliary_device altmode_aux;
 	struct auxiliary_device ps_aux;
 	struct auxiliary_device ucsi_aux;
+	struct auxiliary_device rtc_aux;
 
 	/* serializing client_state and pdr_state updates */
 	struct mutex state_lock;
@@ -341,6 +343,12 @@ static int pmic_glink_probe(struct platform_device *pdev)
 			goto out_release_altmode_aux;
 	}
 
+	if (pg->data->client_mask & BIT(PMIC_GLINK_CLIENT_RTC)) {
+		ret = pmic_glink_add_aux_device(pg, &pg->rtc_aux, "rtc-glink");
+		if (ret)
+			goto out_release_ps_aux;
+	}
+
 	if (pg->data->charger_pdr_service_name && pg->data->charger_pdr_service_path) {
 		service = pdr_add_lookup(pg->pdr, pg->data->charger_pdr_service_name,
 					 pg->data->charger_pdr_service_path);
@@ -358,6 +366,9 @@ static int pmic_glink_probe(struct platform_device *pdev)
 	return 0;
 
 out_release_aux_devices:
+	if (pg->data->client_mask & BIT(PMIC_GLINK_CLIENT_RTC))
+		pmic_glink_del_aux_device(pg, &pg->rtc_aux);
+out_release_ps_aux:
 	if (pg->data->client_mask & BIT(PMIC_GLINK_CLIENT_BATT))
 		pmic_glink_del_aux_device(pg, &pg->ps_aux);
 out_release_altmode_aux:
@@ -384,11 +395,20 @@ static void pmic_glink_remove(struct platform_device *pdev)
 		pmic_glink_del_aux_device(pg, &pg->altmode_aux);
 	if (pg->data->client_mask & BIT(PMIC_GLINK_CLIENT_UCSI))
 		pmic_glink_del_aux_device(pg, &pg->ucsi_aux);
+	if (pg->data->client_mask & BIT(PMIC_GLINK_CLIENT_RTC))
+		pmic_glink_del_aux_device(pg, &pg->rtc_aux);
 
 	guard(mutex)(&__pmic_glink_lock);
 	__pmic_glink = NULL;
 }
 
+static const struct pmic_glink_data pmic_glink_glymur_data = {
+	.client_mask = BIT(PMIC_GLINK_CLIENT_BATT) |
+		       BIT(PMIC_GLINK_CLIENT_ALTMODE) |
+		       BIT(PMIC_GLINK_CLIENT_UCSI) |
+		       BIT(PMIC_GLINK_CLIENT_RTC),
+};
+
 static const struct pmic_glink_data pmic_glink_adsp_data = {
 	.client_mask = BIT(PMIC_GLINK_CLIENT_BATT) |
 		       BIT(PMIC_GLINK_CLIENT_ALTMODE) |
@@ -404,7 +424,7 @@ static const struct pmic_glink_data pmic_glink_soccp_data = {
 };
 
 static const struct of_device_id pmic_glink_of_match[] = {
-	{ .compatible = "qcom,glymur-pmic-glink", .data = &pmic_glink_soccp_data },
+	{ .compatible = "qcom,glymur-pmic-glink", .data = &pmic_glink_glymur_data },
 	{ .compatible = "qcom,kaanapali-pmic-glink", .data = &pmic_glink_soccp_data },
 	{ .compatible = "qcom,pmic-glink", .data = &pmic_glink_adsp_data },
 	{}
diff --git a/include/linux/soc/qcom/pmic_glink.h b/include/linux/soc/qcom/pmic_glink.h
index 7cddf1027752..11031b06b810 100644
--- a/include/linux/soc/qcom/pmic_glink.h
+++ b/include/linux/soc/qcom/pmic_glink.h
@@ -11,6 +11,7 @@ struct pmic_glink_client;
 #define PMIC_GLINK_OWNER_BATTMGR	32778
 #define PMIC_GLINK_OWNER_USBC		32779
 #define PMIC_GLINK_OWNER_USBC_PAN	32780
+#define PMIC_GLINK_OWNER_RTC		32784
 
 #define PMIC_GLINK_REQ_RESP		1
 #define PMIC_GLINK_NOTIFY		2

-- 
2.43.0

Re: [PATCH 2/2] rtc: qcom: rtc-glink: Add PMIC GLINK RTC driver and extend pmic_glink

[Bjorn Andersson]

On Fri, Aug 07, 2026 at 02:26:31AM +0530, Kamal Wadhwa wrote:
> Add a new RTC driver that communicates with the PMIC firmware over
> the GLINK transport layer. The driver supports reading time and
> alarm functionality via GET_PROPERTY and SET_PROPERTY opcodes.

Please review the second sentence here, it doesn't match the
implementation.

In fact,
https://docs.kernel.org/process/submitting-patches.html#describe-your-changes
says that your commit message should "describe your problem" and nowhere
in this commit message is it described why this patch exists. This is
just regurgitating the code.

N.B. There are no valid problem descriptions that starts with the word
"Add"...

> 
> Time is read via GET_PROPERTY (opcode 0x62) using QCOM_RTC_GLINK_TIME
> property (0x00). Alarm set and enable are handled via SET_PROPERTY
> (opcode 0x63) using QCOM_RTC_GLINK_ALARM and QCOM_RTC_GLINK_ALARM_ENABLE
> properties respectively.
> 

In both of these paragraphs you talk about reading time (and presumably
set/get alarm) - but you do implement rtc_class_ops::set_time, what's up
with that?

> GET_PROPERTY and SET_PROPERTY responses share the same firmware message
> layout, with the firmware echoing back the property value. Both opcodes
> are handled by a common callback path accordingly.
> 

That's true, but it's capturing one detail of the implementation. I
don't think it's relevant in the commit message.

> Also extend pmic_glink to register the RTC as a PMIC GLINK client and
> add PMIC GLINK RTC client support for glymur.
> 

The fact that those 22 lines of code is for a complete separate
subsystem/maintainer and that there isn't a build-time dependency means
that it would be much nicer if it came as separate commits.

The PMIC_GLINK_OWNER_RTC define can be carried in the rtc driver.

> Assisted-by: Claude:claude-sonnet-5
> Signed-off-by: Raj Aryan <raryan@qti.qualcomm.com>
> Signed-off-by: Kamal Wadhwa <kamal.wadhwa@oss.qualcomm.com>

Who wrote this driver? The author seems to be you Kamal, but the first
one to sign off on the work was Raj?

> ---
>  MAINTAINERS                         |   7 +
>  drivers/rtc/Kconfig                 |  10 +
>  drivers/rtc/Makefile                |   1 +
>  drivers/rtc/rtc-qcom-glink.c        | 690 ++++++++++++++++++++++++++++++++++++
>  drivers/soc/qcom/pmic_glink.c       |  22 +-
>  include/linux/soc/qcom/pmic_glink.h |   1 +
>  6 files changed, 730 insertions(+), 1 deletion(-)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 5f178a5ff026..752cf667b8ad 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -22505,6 +22505,13 @@ S:	Maintained
>  F:	Documentation/devicetree/bindings/power/supply/qcom,pmi8998-charger.yaml
>  F:	drivers/power/supply/qcom_smbx.c
>  
> +QUALCOMM PMIC GLINK RTC DRIVER
> +M:	Kamal Wadhwa <kamal.wadhwa@oss.qualcomm.com>
> +L:	linux-rtc@vger.kernel.org
> +L:	linux-arm-msm@vger.kernel.org
> +S:	Maintained
> +F:	drivers/rtc/rtc-qcom-glink.c
> +
>  QUALCOMM PPE DRIVER
>  M:	Luo Jie <jie.luo@oss.qualcomm.com>
>  L:	netdev@vger.kernel.org
> diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
> index 01def8231873..3ac39704b2e1 100644
> --- a/drivers/rtc/Kconfig
> +++ b/drivers/rtc/Kconfig
> @@ -1836,6 +1836,16 @@ config RTC_DRV_PM8XXX
>  	  To compile this driver as a module, choose M here: the
>  	  module will be called rtc-pm8xxx.
>  
> +config RTC_DRV_QCOM_GLINK
> +	tristate "Qualcomm PMIC GLINK RTC"
> +	depends on QCOM_PMIC_GLINK
> +	help
> +	  Say Y here to enable support for the RTC exposed by the PMIC
> +	  over the GLINK transport on Qualcomm platforms.
> +
> +	  This driver can also be built as a module. If so, the module
> +	  will be called rtc-qcom-glink.
> +
>  config RTC_DRV_TEGRA
>  	tristate "NVIDIA Tegra Internal RTC driver"
>  	depends on ARCH_TEGRA || COMPILE_TEST
> diff --git a/drivers/rtc/Makefile b/drivers/rtc/Makefile
> index 0347645b021f..8215a1fc29fb 100644
> --- a/drivers/rtc/Makefile
> +++ b/drivers/rtc/Makefile
> @@ -73,6 +73,7 @@ obj-$(CONFIG_RTC_DRV_FM3130)	+= rtc-fm3130.o
>  obj-$(CONFIG_RTC_DRV_FSL_FTM_ALARM)	+= rtc-fsl-ftm-alarm.o
>  obj-$(CONFIG_RTC_DRV_FTRTC010)	+= rtc-ftrtc010.o
>  obj-$(CONFIG_RTC_DRV_GENERIC)	+= rtc-generic.o
> +obj-$(CONFIG_RTC_DRV_QCOM_GLINK) += rtc-qcom-glink.o
>  obj-$(CONFIG_RTC_DRV_GOLDFISH)	+= rtc-goldfish.o
>  obj-$(CONFIG_RTC_DRV_HID_SENSOR_TIME) += rtc-hid-sensor-time.o
>  obj-$(CONFIG_RTC_DRV_HYM8563)	+= rtc-hym8563.o
> diff --git a/drivers/rtc/rtc-qcom-glink.c b/drivers/rtc/rtc-qcom-glink.c
> new file mode 100644
> index 000000000000..611f06a3cc5c
> --- /dev/null
> +++ b/drivers/rtc/rtc-qcom-glink.c
> @@ -0,0 +1,690 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
> + */
> +#include <linux/auxiliary_bus.h>
> +#include <linux/completion.h>
> +#include <linux/device.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
> +#include <linux/property.h>
> +#include <linux/rtc.h>
> +#include <linux/slab.h>
> +#include <linux/soc/qcom/pdr.h>
> +#include <linux/soc/qcom/pmic_glink.h>
> +#include <linux/timekeeping.h>
> +#include <linux/workqueue.h>
> +
> +#define RTC_GLINK_SET_PROPERTY		0x63
> +#define RTC_GLINK_GET_PROPERTY		0x65
> +#define RTC_GLINK_GET_RTC_TICKS		0x67
> +#define RTC_GLINK_GET_REAL_TIME		0x68
> +#define RTC_GLINK_SET_REAL_TIME		0x69
> +#define RTC_GLINK_ALARM_EXPIRED		0x6A
> +
> +enum qcom_rtc_glink_properties {
> +	QCOM_RTC_GLINK_TIME = 0,
> +	QCOM_RTC_GLINK_ALARM_TIME,
> +	QCOM_RTC_GLINK_ALARM_ENABLE,
> +};

This isn't an enumeration, these are protocol-level constants. Please
use #define to make that clear.

> +
> +struct qcom_rtc_glink_msg {
> +	struct pmic_glink_hdr	hdr;
> +	__le32			property;
> +	__le32			value;
> +};
> +
> +struct qcom_rtc_glink_generic_req {
> +	struct pmic_glink_hdr	hdr;
> +};
> +
> +struct qcom_rtc_glink_status_resp {
> +	struct pmic_glink_hdr	hdr;
> +	__le32			return_status;
> +};
> +
> +struct qcom_rtc_glink_ticks_resp {
> +	struct pmic_glink_hdr	hdr;
> +	__le32			return_status;
> +	__le32			rtc_ticks;
> +};
> +
> +struct qcom_rtc_glink_real_time_resp {
> +	struct pmic_glink_hdr	hdr;
> +	__le32			return_status;
> +	__le32			real_time_data[4];
> +};
> +
> +struct qcom_rtc_glink_set_real_time_req {
> +	struct pmic_glink_hdr	hdr;
> +	__le32			real_time_data[4];
> +};
> +
> +struct qcom_rtc_glink {
> +	struct device		*dev;
> +	struct pmic_glink_client *client;
> +	struct rtc_device	*rtc;
> +
> +	/* Serializes requests: only one may be in flight at a time */
> +	struct mutex		lock;
> +	struct completion	ack;
> +	struct work_struct	alarm_work;
> +
> +	/* Protects service_up and request_pending across callback contexts */
> +	spinlock_t		state_lock;
> +	bool			service_up;
> +	bool			request_pending;
> +
> +	int			error;
> +	u32			resp_value;
> +	u32			offset;
> +	struct rtc_time		resp_tm;
> +	bool			resp_valid;
> +	bool			allow_set_time;
> +};
> +
> +static int qcom_rtc_glink_request(struct qcom_rtc_glink *rtc_glink,
> +				  void *data, size_t len)
> +{
> +	unsigned long flags;
> +	unsigned long left;
> +	int ret;
> +
> +	/*
> +	 * Reinit the completion before request_pending becomes visible to
> +	 * qcom_rtc_glink_pdr_tify(), so a concurrent SSR down-transition
> +	 * can't complete() a stale completion that gets wiped out by
> +	 * reinit_completion() right after.
> +	 */
> +	reinit_completion(&rtc_glink->ack);
> +	rtc_glink->error = 0;
> +	rtc_glink->resp_valid = false;
> +
> +	spin_lock_irqsave(&rtc_glink->state_lock, flags);
> +	if (!rtc_glink->service_up) {
> +		spin_unlock_irqrestore(&rtc_glink->state_lock, flags);
> +		return -ECONNRESET;
> +	}
> +	rtc_glink->request_pending = true;
> +	spin_unlock_irqrestore(&rtc_glink->state_lock, flags);
> +
> +	ret = pmic_glink_send(rtc_glink->client, data, len);
> +	if (ret < 0) {
> +		spin_lock_irqsave(&rtc_glink->state_lock, flags);
> +		rtc_glink->request_pending = false;
> +		spin_unlock_irqrestore(&rtc_glink->state_lock, flags);
> +		return ret;

These 4 lines are effectively duplicated after the
wait_for_completion_timeout(), please refactor this with a goto.

> +	}
> +
> +	left = wait_for_completion_timeout(&rtc_glink->ack, HZ);
> +	spin_lock_irqsave(&rtc_glink->state_lock, flags);
> +	rtc_glink->request_pending = false;
> +	spin_unlock_irqrestore(&rtc_glink->state_lock, flags);
> +	if (!left)
> +		return -ETIMEDOUT;
> +
> +	return rtc_glink->error;
> +}
> +
> +static int qcom_rtc_glink_set_property(struct qcom_rtc_glink *rtc_glink,
> +				       u32 property, u32 value)
> +{
> +	struct qcom_rtc_glink_msg msg = {
> +		.hdr.owner = cpu_to_le32(PMIC_GLINK_OWNER_RTC),
> +		.hdr.type = cpu_to_le32(PMIC_GLINK_REQ_RESP),
> +		.hdr.opcode = cpu_to_le32(RTC_GLINK_SET_PROPERTY),
> +		.property = cpu_to_le32(property),
> +		.value = cpu_to_le32(value),
> +	};
> +	int ret;
> +
> +	dev_dbg(rtc_glink->dev,
> +		"TX opcode=0x%x property=%u value=%u\n",
> +		RTC_GLINK_SET_PROPERTY, property, value);
> +	mutex_lock(&rtc_glink->lock);
> +	ret = qcom_rtc_glink_request(rtc_glink, &msg, sizeof(msg));
> +	mutex_unlock(&rtc_glink->lock);
> +	return ret;
> +}
> +
> +static int qcom_rtc_glink_get_property(struct qcom_rtc_glink *rtc_glink,
> +				       u32 property, u32 *value)
> +{
> +	struct qcom_rtc_glink_msg msg = {
> +		.hdr.owner = cpu_to_le32(PMIC_GLINK_OWNER_RTC),
> +		.hdr.type = cpu_to_le32(PMIC_GLINK_REQ_RESP),
> +		.hdr.opcode = cpu_to_le32(RTC_GLINK_GET_PROPERTY),
> +		.property = cpu_to_le32(property),
> +	};
> +	int ret;
> +
> +	dev_dbg(rtc_glink->dev,
> +		"TX opcode=0x%x property=%u\n",
> +		RTC_GLINK_GET_PROPERTY, property);
> +	mutex_lock(&rtc_glink->lock);
> +	ret = qcom_rtc_glink_request(rtc_glink, &msg, sizeof(msg));
> +	if (!ret) {

The idiomatic form in the Linux kernel is to handle the error. I'd find
the following form more natural to read:

	ret = qcom_rtc_glink_request(rtc_glink, &msg, sizeof(msg));
	if (ret < 0) {
		goto out_unlock;
	} else if (!rtc_glink->resp_valid) {
		ret = -EIO;
		goto out_unlock;
	}
	
	*value = rtc_glink->resp_value;

out_unlock:
	mutex_unlock(&rtc_glink->lock);
	return ret;



> +		if (!rtc_glink->resp_valid)
> +			ret = -EIO;
> +		else
> +			*value = rtc_glink->resp_value;
> +	}
> +	mutex_unlock(&rtc_glink->lock);
> +	return ret;
> +}
> +
> +static int qcom_rtc_glink_get_ticks(struct qcom_rtc_glink *rtc_glink,
> +				    u32 *ticks)
> +{
> +	struct qcom_rtc_glink_generic_req msg = {
> +		.hdr.owner = cpu_to_le32(PMIC_GLINK_OWNER_RTC),
> +		.hdr.type = cpu_to_le32(PMIC_GLINK_REQ_RESP),
> +		.hdr.opcode = cpu_to_le32(RTC_GLINK_GET_RTC_TICKS),
> +	};
> +	int ret;
> +
> +	dev_dbg(rtc_glink->dev, "TX opcode=0x%x\n", RTC_GLINK_GET_RTC_TICKS);
> +	mutex_lock(&rtc_glink->lock);
> +	ret = qcom_rtc_glink_request(rtc_glink, &msg, sizeof(msg));
> +	if (!ret) {
> +		if (!rtc_glink->resp_valid)
> +			ret = -EIO;
> +		else
> +			*ticks = rtc_glink->resp_value;
> +	}

Ditto

> +	mutex_unlock(&rtc_glink->lock);
> +	return ret;
> +}
> +
> +static int qcom_rtc_glink_get_real_time(struct qcom_rtc_glink *rtc_glink,
> +					struct rtc_time *tm)
> +{
> +	struct qcom_rtc_glink_generic_req msg = {
> +		.hdr.owner = cpu_to_le32(PMIC_GLINK_OWNER_RTC),
> +		.hdr.type = cpu_to_le32(PMIC_GLINK_REQ_RESP),
> +		.hdr.opcode = cpu_to_le32(RTC_GLINK_GET_REAL_TIME),
> +	};
> +	int ret;
> +
> +	dev_dbg(rtc_glink->dev, "TX opcode=0x%x\n", RTC_GLINK_GET_REAL_TIME);
> +	mutex_lock(&rtc_glink->lock);
> +	ret = qcom_rtc_glink_request(rtc_glink, &msg, sizeof(msg));
> +	if (!ret) {
> +		if (!rtc_glink->resp_valid)
> +			ret = -EIO;
> +		else
> +			*tm = rtc_glink->resp_tm;
> +	}

Ditto.

> +	mutex_unlock(&rtc_glink->lock);
> +	return ret;
> +}
> +
> +static int qcom_rtc_glink_set_real_time(struct qcom_rtc_glink *rtc_glink,
> +					struct rtc_time *tm)
> +{
> +	struct qcom_rtc_glink_set_real_time_req msg = {};
> +	u32 w0, w1;
> +	int ret;
> +
> +	w0 = ((tm->tm_year + 1900) & 0xffff) |
> +	     (((tm->tm_mon + 1) & 0xff) << 16) |
> +	     ((tm->tm_mday & 0xff) << 24);

Can't this be written as:

	w0 = FIELD_PREP(0x0000ffff, tm->tm_year + 1900) |
	     FIELD_PREP(0x00ff0000, tm->tm_mod + 1) |
	     FIELD_PREP(0xff000000, tm->tm_mday);

> +	w1 = (tm->tm_hour & 0xff) |
> +	     ((tm->tm_min & 0xff) << 8) |
> +	     ((tm->tm_sec & 0xff) << 16) |
> +	     (1U << 24);
> +	msg.hdr.owner = cpu_to_le32(PMIC_GLINK_OWNER_RTC);
> +	msg.hdr.type = cpu_to_le32(PMIC_GLINK_REQ_RESP);
> +	msg.hdr.opcode = cpu_to_le32(RTC_GLINK_SET_REAL_TIME);
> +	msg.real_time_data[0] = cpu_to_le32(w0);
> +	msg.real_time_data[1] = cpu_to_le32(w1);
> +	msg.real_time_data[2] = 0;
> +	msg.real_time_data[3] = 0;
> +	dev_dbg(rtc_glink->dev,
> +		"TX opcode=0x%x data=%08x %08x\n",
> +		RTC_GLINK_SET_REAL_TIME, w0, w1);
> +	mutex_lock(&rtc_glink->lock);
> +	ret = qcom_rtc_glink_request(rtc_glink, &msg, sizeof(msg));
> +	mutex_unlock(&rtc_glink->lock);
> +	return ret;

It certainly wouldn't be bad to insert one or two empty lines in this 28
line block.

> +}
> +
> +static int qcom_rtc_glink_get_time(struct qcom_rtc_glink *rtc_glink,
> +				   struct rtc_time *time)
> +{
> +	u32 ticks;
> +	int ret;
> +
> +	ret = qcom_rtc_glink_get_real_time(rtc_glink, time);
> +	if (!ret)
> +		return 0;
> +	dev_warn(rtc_glink->dev, "0x68 failed (%d), falling back\n", ret);

0x68 failed you say...how about writing an error message that a human
can read?

> +
> +	ret = qcom_rtc_glink_get_ticks(rtc_glink, &ticks);
> +	if (!ret) {
> +		rtc_time64_to_tm((time64_t)ticks + rtc_glink->offset, time);
> +		return 0;
> +	}
> +
> +	ret = qcom_rtc_glink_get_property(rtc_glink, QCOM_RTC_GLINK_TIME,
> +					  &ticks);

Don't wrap lines when they are easier to read in their unwrapped form
and still less than 100 characters.

> +	if (!ret && ticks != U32_MAX) {
> +		rtc_time64_to_tm((time64_t)ticks + rtc_glink->offset, time);
> +		return 0;
> +	}
> +
> +	dev_err(rtc_glink->dev, "all time sources failed\n");
> +	return -EIO;
> +}
> +
> +static int qcom_rtc_glink_set_time(struct qcom_rtc_glink *rtc_glink,
> +				   struct rtc_time *time)
> +{
> +	time64_t t = rtc_tm_to_time64(time);
> +	u32 ticks;
> +	int ret;
> +
> +	if (!rtc_glink->allow_set_time)
> +		return -EOPNOTSUPP;
> +
> +	ret = qcom_rtc_glink_set_real_time(rtc_glink, time);
> +	if (ret)
> +		dev_warn(rtc_glink->dev, "0x69 failed (%d)\n", ret);

Write English error messages.

> +
> +	ret = qcom_rtc_glink_set_property(rtc_glink, QCOM_RTC_GLINK_TIME, (u32)t);
> +	if (ret)
> +		return ret;
> +
> +	ret = qcom_rtc_glink_get_ticks(rtc_glink, &ticks);
> +	if (!ret && ticks != U32_MAX) {
> +		rtc_glink->offset = (u32)t - ticks;

There's no reason to if (success) { dostuff(); } else {return -EIO; }.
Handle the error and then continue with the successful case.

> +	} else {
> +		dev_warn(rtc_glink->dev, "no tick source for offset\n");
> +		return -EIO;
> +	}
> +
> +	return 0;
> +}
> +
> +static int qcom_rtc_glink_set_alarm_en(struct qcom_rtc_glink *rtc_glink,
> +				       int enabled)
> +{
> +	return qcom_rtc_glink_set_property(rtc_glink,
> +					   QCOM_RTC_GLINK_ALARM_ENABLE,
> +					   enabled);
> +}
> +
> +static int qcom_rtc_glink_set_alarm(struct qcom_rtc_glink *rtc_glink,
> +				    struct rtc_wkalrm *alarm)
> +{
> +	time64_t alarm_t = rtc_tm_to_time64(&alarm->time);
> +	struct rtc_time now_tm;
> +	time64_t now_real, secs_until_alarm;
> +	u32 fw_current, fw_alarm;
> +	int ret;
> +
> +	ret = qcom_rtc_glink_get_time(rtc_glink, &now_tm);
> +	if (ret)
> +		return ret;
> +	now_real = rtc_tm_to_time64(&now_tm);
> +	secs_until_alarm = alarm_t - now_real;
> +	dev_dbg(rtc_glink->dev,
> +		"set_alarm: now_real=%lld alarm_t=%lld delta=%lld\n",
> +		now_real, alarm_t, secs_until_alarm);

Here's 12 lines that get the time and calculate now and calculate alarm
time and print a debug message and checks for invalid value.

If you really need the debug print in here, give it a space before and
after to separate it from the actual logic.

> +	if (secs_until_alarm < 0)
> +		return -EINVAL;
> +	if (secs_until_alarm > U32_MAX)
> +		return -ERANGE;
> +
> +	ret = qcom_rtc_glink_get_property(rtc_glink, QCOM_RTC_GLINK_TIME,
> +					  &fw_current);

Again, don't unnecessarily wrap lines that aren't too long.

> +	if (ret)
> +		return ret;
> +	if (fw_current == U32_MAX)
> +		fw_current = 0;
> +	fw_alarm = fw_current + (u32)secs_until_alarm;
> +	dev_dbg(rtc_glink->dev,
> +		"set_alarm: fw_current=%u fw_alarm=%u\n",
> +		fw_current, fw_alarm);
> +
> +	ret = qcom_rtc_glink_set_alarm_en(rtc_glink, 0);
> +	if (ret)
> +		return ret;
> +
> +	ret = qcom_rtc_glink_set_property(rtc_glink,
> +					  QCOM_RTC_GLINK_ALARM_TIME, fw_alarm);
> +	if (ret)
> +		return ret;
> +
> +	return qcom_rtc_glink_set_alarm_en(rtc_glink, alarm->enabled);
> +}
> +
> +static int qcom_rtc_glink_get_alarm(struct qcom_rtc_glink *rtc_glink,
> +				    struct rtc_wkalrm *alarm)
> +{
> +	u32 fw_alarm_time, fw_current, alarm_en;
> +	struct rtc_time now_tm;
> +	time64_t now_real, secs_until_alarm, alarm_real;
> +	int ret;
> +
> +	ret = qcom_rtc_glink_get_property(rtc_glink,
> +					  QCOM_RTC_GLINK_ALARM_TIME,
> +					  &fw_alarm_time);
> +	if (ret)
> +		return ret;
> +
> +	ret = qcom_rtc_glink_get_property(rtc_glink, QCOM_RTC_GLINK_TIME,
> +					  &fw_current);
> +	if (ret)
> +		return ret;
> +
> +	ret = qcom_rtc_glink_get_property(rtc_glink,
> +					  QCOM_RTC_GLINK_ALARM_ENABLE,
> +					  &alarm_en);
> +	if (ret)
> +		return ret;
> +	if (fw_current == U32_MAX)
> +		fw_current = 0;
> +
> +	ret = qcom_rtc_glink_get_time(rtc_glink, &now_tm);
> +	if (ret)
> +		return ret;
> +	now_real = rtc_tm_to_time64(&now_tm);
> +	secs_until_alarm = (time64_t)fw_alarm_time - (time64_t)fw_current;
> +	alarm_real = now_real + secs_until_alarm;
> +	dev_dbg(rtc_glink->dev,
> +		"get_alarm: now_real=%lld fw_current=%u fw_alarm=%u delta=%lld\n",
> +		now_real, fw_current, fw_alarm_time, secs_until_alarm);
> +	rtc_time64_to_tm(alarm_real, &alarm->time);
> +	alarm->enabled = !!alarm_en;
> +	return 0;
> +}
> +
> +static void qcom_rtc_glink_alarm_work(struct work_struct *work)
> +{
> +	struct qcom_rtc_glink *rtc_glink =
> +		container_of(work, struct qcom_rtc_glink, alarm_work);
> +	int ret;
> +
> +	ret = qcom_rtc_glink_set_alarm_en(rtc_glink, 0);
> +	if (ret)
> +		dev_err(rtc_glink->dev,
> +			"failed to disable alarm after expiry (%d)\n", ret);
> +	rtc_update_irq(rtc_glink->rtc, 1, RTC_IRQF | RTC_AF);
> +}
> +
> +static void qcom_rtc_glink_callback(const void *data, size_t len, void *priv)
> +{
> +	struct qcom_rtc_glink *rtc_glink = priv;
> +	const struct pmic_glink_hdr *hdr = data;
> +	unsigned long flags;
> +	bool pending;
> +
> +	if (len < sizeof(*hdr))
> +		return;
> +
> +	dev_dbg(rtc_glink->dev,
> +		"RX opcode=0x%x type=0x%x owner=0x%x len=%zu\n",
> +		le32_to_cpu(hdr->opcode), le32_to_cpu(hdr->type),
> +		le32_to_cpu(hdr->owner), len);

I don't fancy all your debug prints, but I love the fact that this isn't
crammed into logical chunks of code.

> +
> +	if (le32_to_cpu(hdr->opcode) == RTC_GLINK_ALARM_EXPIRED) {
> +		dev_info(rtc_glink->dev, "alarm expired\n");

Is this useful to the end user?

> +		schedule_work(&rtc_glink->alarm_work);
> +		return;
> +	}
> +
> +	/*
> +	 * Hold state_lock across the pending check and the response fields
> +	 * below: qcom_rtc_glink_pdr_notify() writes the same error/resp_*
> +	 * fields under this lock when it force-completes a request on an
> +	 * SSR down-transition, and the two must not race. Dropping the
> +	 * response here if !pending also covers requests that already
> +	 * timed out or were aborted; qcom_rtc_glink_request() will call
> +	 * reinit_completion() again before this response could be mistaken
> +	 * for a later request's.
> +	 */
> +	spin_lock_irqsave(&rtc_glink->state_lock, flags);
> +	pending = rtc_glink->request_pending;
> +
> +	if (!pending) {
> +		spin_unlock_irqrestore(&rtc_glink->state_lock, flags);
> +		dev_dbg(rtc_glink->dev,
> +			"dropping stale response opcode=0x%x\n",
> +			le32_to_cpu(hdr->opcode));
> +		return;
> +	}
> +
> +	switch (le32_to_cpu(hdr->opcode)) {
> +	case RTC_GLINK_GET_PROPERTY:
> +		fallthrough;
> +	case RTC_GLINK_SET_PROPERTY: {
> +		const struct qcom_rtc_glink_msg *msg = data;
> +
> +		if (len < sizeof(struct pmic_glink_hdr) + sizeof(__le32)) {
> +			rtc_glink->error = -EINVAL;
> +			break;
> +		}
> +		rtc_glink->resp_value = le32_to_cpu(msg->property);
> +		rtc_glink->resp_valid = true;
> +		rtc_glink->error = 0;
> +		dev_dbg(rtc_glink->dev, "ACK opcode=0x%x resp_value=%u\n",
> +			le32_to_cpu(hdr->opcode), rtc_glink->resp_value);
> +		break;
> +	}
> +
> +	case RTC_GLINK_GET_RTC_TICKS: {
> +		const struct qcom_rtc_glink_ticks_resp *resp = data;
> +
> +		if (len < sizeof(*resp)) {
> +			rtc_glink->error = -EINVAL;
> +			break;
> +		}
> +		if (le32_to_cpu(resp->return_status)) {
> +			rtc_glink->error = -EIO;
> +			break;
> +		}
> +		rtc_glink->resp_value = le32_to_cpu(resp->rtc_ticks);
> +		rtc_glink->resp_valid = true;
> +		rtc_glink->error = 0;
> +		dev_dbg(rtc_glink->dev, "ACK opcode=0x%x ticks=%u\n",
> +			RTC_GLINK_GET_RTC_TICKS, rtc_glink->resp_value);
> +		break;
> +	}
> +
> +	case RTC_GLINK_GET_REAL_TIME: {
> +		const struct qcom_rtc_glink_real_time_resp *resp = data;
> +		u32 w0, w1;
> +
> +		if (len < sizeof(*resp)) {
> +			rtc_glink->error = -EINVAL;
> +			break;
> +		}
> +		if (le32_to_cpu(resp->return_status)) {
> +			rtc_glink->error = -EIO;
> +			break;
> +		}
> +		w0 = le32_to_cpu(resp->real_time_data[0]);
> +		w1 = le32_to_cpu(resp->real_time_data[1]);
> +		rtc_glink->resp_tm.tm_year = (w0 & 0xffff) - 1900;
> +		rtc_glink->resp_tm.tm_mon = ((w0 >> 16) & 0xff) - 1;
> +		rtc_glink->resp_tm.tm_mday = (w0 >> 24) & 0xff;
> +		rtc_glink->resp_tm.tm_hour = w1 & 0xff;
> +		rtc_glink->resp_tm.tm_min = (w1 >> 8) & 0xff;
> +		rtc_glink->resp_tm.tm_sec = (w1 >> 16) & 0xff;

Can't you FIELD_GET() these?

> +		rtc_glink->resp_valid = true;
> +		rtc_glink->error = 0;
> +		dev_dbg(rtc_glink->dev,
> +			"ACK opcode=0x%x %04d-%02d-%02d %02d:%02d:%02d\n",
> +			RTC_GLINK_GET_REAL_TIME,
> +			rtc_glink->resp_tm.tm_year + 1900,
> +			rtc_glink->resp_tm.tm_mon + 1,
> +			rtc_glink->resp_tm.tm_mday,
> +			rtc_glink->resp_tm.tm_hour,
> +			rtc_glink->resp_tm.tm_min,
> +			rtc_glink->resp_tm.tm_sec);
> +		break;
> +	}
> +
> +	case RTC_GLINK_SET_REAL_TIME: {
> +		const struct qcom_rtc_glink_status_resp *resp = data;
> +
> +		if (len < sizeof(*resp)) {
> +			rtc_glink->error = -EINVAL;
> +			break;
> +		}
> +		rtc_glink->error = le32_to_cpu(resp->return_status) ? -EIO : 0;
> +		dev_dbg(rtc_glink->dev, "ACK opcode=0x%x status=%u\n",
> +			RTC_GLINK_SET_REAL_TIME,
> +			le32_to_cpu(resp->return_status));
> +		break;
> +	}
> +
> +	default:
> +		spin_unlock_irqrestore(&rtc_glink->state_lock, flags);
> +		dev_warn(rtc_glink->dev,
> +			 "unhandled RX opcode=0x%x len=%zu\n",
> +			 le32_to_cpu(hdr->opcode), len);
> +		return;
> +	}
> +
> +	spin_unlock_irqrestore(&rtc_glink->state_lock, flags);
> +	complete(&rtc_glink->ack);
> +}
> +
> +static void qcom_rtc_glink_pdr_notify(void *priv, int state)
> +{
> +	struct qcom_rtc_glink *rtc_glink = priv;
> +	unsigned long flags;
> +	bool up = (state == SERVREG_SERVICE_STATE_UP);
> +
> +	spin_lock_irqsave(&rtc_glink->state_lock, flags);
> +	rtc_glink->service_up = up;
> +	if (!up && rtc_glink->request_pending) {
> +		rtc_glink->error = -ECONNRESET;
> +		rtc_glink->resp_valid = false;
> +		complete(&rtc_glink->ack);
> +	}
> +	spin_unlock_irqrestore(&rtc_glink->state_lock, flags);
> +}
> +
> +static int qcom_rtc_glink_read_time(struct device *dev, struct rtc_time *tm)
> +{
> +	return qcom_rtc_glink_get_time(dev_get_drvdata(dev), tm);
> +}
> +
> +static int qcom_rtc_glink_set_time_dev(struct device *dev, struct rtc_time *tm)
> +{
> +	return qcom_rtc_glink_set_time(dev_get_drvdata(dev), tm);
> +}
> +
> +static int qcom_rtc_glink_read_alarm(struct device *dev,
> +				     struct rtc_wkalrm *alrm)
> +{
> +	return qcom_rtc_glink_get_alarm(dev_get_drvdata(dev), alrm);
> +}
> +
> +static int qcom_rtc_glink_set_alarm_dev(struct device *dev,
> +					struct rtc_wkalrm *alrm)
> +{
> +	return qcom_rtc_glink_set_alarm(dev_get_drvdata(dev), alrm);
> +}
> +
> +static int qcom_rtc_glink_alarm_irq_enable(struct device *dev,
> +					   unsigned int enabled)
> +{
> +	return qcom_rtc_glink_set_alarm_en(dev_get_drvdata(dev), enabled);
> +}
> +
> +static const struct rtc_class_ops qcom_rtc_glink_rtc_ops = {
> +	.read_time = qcom_rtc_glink_read_time,
> +	.set_time = qcom_rtc_glink_set_time_dev,
> +	.read_alarm = qcom_rtc_glink_read_alarm,
> +	.set_alarm = qcom_rtc_glink_set_alarm_dev,
> +	.alarm_irq_enable = qcom_rtc_glink_alarm_irq_enable,
> +};
> +
> +static const struct of_device_id qcom_rtc_glink_of_variants[] = {
> +	{ .compatible = "qcom,glymur-pmic-glink" },
> +	{}
> +};
> +
> +static int qcom_rtc_glink_probe(struct auxiliary_device *adev,
> +				const struct auxiliary_device_id *id)
> +{
> +	struct device *dev = &adev->dev;
> +	struct qcom_rtc_glink *rtc_glink;
> +
> +	if (!of_match_device(qcom_rtc_glink_of_variants, dev->parent))
> +		return -ENXIO;

So in order to support this on a new target we should update both the
client_mask and qcom_rtc_glink_of_variants[]?

Why is this needed?

> +
> +	rtc_glink = devm_kzalloc(dev, sizeof(*rtc_glink), GFP_KERNEL);
> +	if (!rtc_glink)
> +		return -ENOMEM;
> +
> +	rtc_glink->dev            = dev;
> +	rtc_glink->allow_set_time = device_property_read_bool(dev->parent,
> +							      "allow-set-time");

Why do we have this property? Why would we not allow time to be set?

Regards,
Bjorn

> +
> +	mutex_init(&rtc_glink->lock);
> +	spin_lock_init(&rtc_glink->state_lock);
> +	init_completion(&rtc_glink->ack);
> +	INIT_WORK(&rtc_glink->alarm_work, qcom_rtc_glink_alarm_work);
> +	dev_set_drvdata(dev, rtc_glink);
> +
> +	rtc_glink->client = devm_pmic_glink_client_alloc(dev,
> +							 PMIC_GLINK_OWNER_RTC,
> +							 qcom_rtc_glink_callback,
> +							 qcom_rtc_glink_pdr_notify,
> +							 rtc_glink);
> +	if (IS_ERR(rtc_glink->client))
> +		return dev_err_probe(dev, PTR_ERR(rtc_glink->client),
> +				     "failed to allocate glink client\n");
> +
> +	pmic_glink_client_register(rtc_glink->client);
> +
> +	device_init_wakeup(dev, true);
> +
> +	rtc_glink->rtc = devm_rtc_allocate_device(dev);
> +	if (IS_ERR(rtc_glink->rtc))
> +		return dev_err_probe(dev, PTR_ERR(rtc_glink->rtc),
> +				     "failed to allocate RTC device\n");
> +
> +	rtc_glink->rtc->ops      = &qcom_rtc_glink_rtc_ops;
> +	rtc_glink->rtc->range_min = 0;
> +	rtc_glink->rtc->range_max = U32_MAX;
> +
> +	return devm_rtc_register_device(rtc_glink->rtc);
> +}
> +
> +static void qcom_rtc_glink_remove(struct auxiliary_device *adev)
> +{
> +	struct qcom_rtc_glink *rtc_glink = dev_get_drvdata(&adev->dev);
> +	/*
> +	 * Nothing left to arm alarm_work after this returns: the glink
> +	 * client (and its callback/pdr_notify) is torn down by devm after
> +	 * .remove() returns, and rtc_update_irq() below still targets a
> +	 * live devm-managed rtc device.
> +	 */
> +	cancel_work_sync(&rtc_glink->alarm_work);
> +}
> +
> +static const struct auxiliary_device_id qcom_rtc_glink_id_table[] = {
> +	{ .name = "pmic_glink.rtc-glink" },
> +	{}
> +};
> +
> +MODULE_DEVICE_TABLE(auxiliary, qcom_rtc_glink_id_table);
> +
> +static struct auxiliary_driver qcom_rtc_glink_driver = {
> +	.name = "qcom_pmic_rtc_glink",
> +	.id_table = qcom_rtc_glink_id_table,
> +	.probe = qcom_rtc_glink_probe,
> +	.remove = qcom_rtc_glink_remove,
> +};
> +
> +module_auxiliary_driver(qcom_rtc_glink_driver);
> +MODULE_DESCRIPTION("Qualcomm PMIC GLINK RTC driver");
> +MODULE_LICENSE("GPL");
> diff --git a/drivers/soc/qcom/pmic_glink.c b/drivers/soc/qcom/pmic_glink.c
> index 3042261578aa..7ee70a31ba15 100644
> --- a/drivers/soc/qcom/pmic_glink.c
> +++ b/drivers/soc/qcom/pmic_glink.c
> @@ -21,6 +21,7 @@ enum {
>  	PMIC_GLINK_CLIENT_BATT = 0,
>  	PMIC_GLINK_CLIENT_ALTMODE,
>  	PMIC_GLINK_CLIENT_UCSI,
> +	PMIC_GLINK_CLIENT_RTC,
>  };
>  
>  struct pmic_glink_data {
> @@ -40,6 +41,7 @@ struct pmic_glink {
>  	struct auxiliary_device altmode_aux;
>  	struct auxiliary_device ps_aux;
>  	struct auxiliary_device ucsi_aux;
> +	struct auxiliary_device rtc_aux;
>  
>  	/* serializing client_state and pdr_state updates */
>  	struct mutex state_lock;
> @@ -341,6 +343,12 @@ static int pmic_glink_probe(struct platform_device *pdev)
>  			goto out_release_altmode_aux;
>  	}
>  
> +	if (pg->data->client_mask & BIT(PMIC_GLINK_CLIENT_RTC)) {
> +		ret = pmic_glink_add_aux_device(pg, &pg->rtc_aux, "rtc-glink");
> +		if (ret)
> +			goto out_release_ps_aux;
> +	}
> +
>  	if (pg->data->charger_pdr_service_name && pg->data->charger_pdr_service_path) {
>  		service = pdr_add_lookup(pg->pdr, pg->data->charger_pdr_service_name,
>  					 pg->data->charger_pdr_service_path);
> @@ -358,6 +366,9 @@ static int pmic_glink_probe(struct platform_device *pdev)
>  	return 0;
>  
>  out_release_aux_devices:
> +	if (pg->data->client_mask & BIT(PMIC_GLINK_CLIENT_RTC))
> +		pmic_glink_del_aux_device(pg, &pg->rtc_aux);
> +out_release_ps_aux:
>  	if (pg->data->client_mask & BIT(PMIC_GLINK_CLIENT_BATT))
>  		pmic_glink_del_aux_device(pg, &pg->ps_aux);
>  out_release_altmode_aux:
> @@ -384,11 +395,20 @@ static void pmic_glink_remove(struct platform_device *pdev)
>  		pmic_glink_del_aux_device(pg, &pg->altmode_aux);
>  	if (pg->data->client_mask & BIT(PMIC_GLINK_CLIENT_UCSI))
>  		pmic_glink_del_aux_device(pg, &pg->ucsi_aux);
> +	if (pg->data->client_mask & BIT(PMIC_GLINK_CLIENT_RTC))
> +		pmic_glink_del_aux_device(pg, &pg->rtc_aux);
>  
>  	guard(mutex)(&__pmic_glink_lock);
>  	__pmic_glink = NULL;
>  }
>  
> +static const struct pmic_glink_data pmic_glink_glymur_data = {
> +	.client_mask = BIT(PMIC_GLINK_CLIENT_BATT) |
> +		       BIT(PMIC_GLINK_CLIENT_ALTMODE) |
> +		       BIT(PMIC_GLINK_CLIENT_UCSI) |
> +		       BIT(PMIC_GLINK_CLIENT_RTC),
> +};
> +
>  static const struct pmic_glink_data pmic_glink_adsp_data = {
>  	.client_mask = BIT(PMIC_GLINK_CLIENT_BATT) |
>  		       BIT(PMIC_GLINK_CLIENT_ALTMODE) |
> @@ -404,7 +424,7 @@ static const struct pmic_glink_data pmic_glink_soccp_data = {
>  };
>  
>  static const struct of_device_id pmic_glink_of_match[] = {
> -	{ .compatible = "qcom,glymur-pmic-glink", .data = &pmic_glink_soccp_data },
> +	{ .compatible = "qcom,glymur-pmic-glink", .data = &pmic_glink_glymur_data },
>  	{ .compatible = "qcom,kaanapali-pmic-glink", .data = &pmic_glink_soccp_data },
>  	{ .compatible = "qcom,pmic-glink", .data = &pmic_glink_adsp_data },
>  	{}
> diff --git a/include/linux/soc/qcom/pmic_glink.h b/include/linux/soc/qcom/pmic_glink.h
> index 7cddf1027752..11031b06b810 100644
> --- a/include/linux/soc/qcom/pmic_glink.h
> +++ b/include/linux/soc/qcom/pmic_glink.h
> @@ -11,6 +11,7 @@ struct pmic_glink_client;
>  #define PMIC_GLINK_OWNER_BATTMGR	32778
>  #define PMIC_GLINK_OWNER_USBC		32779
>  #define PMIC_GLINK_OWNER_USBC_PAN	32780
> +#define PMIC_GLINK_OWNER_RTC		32784
>  
>  #define PMIC_GLINK_REQ_RESP		1
>  #define PMIC_GLINK_NOTIFY		2
> 
> -- 
> 2.43.0
> 

Re: [PATCH 1/2] dt-bindings: soc: qcom: qcom,pmic-glink: Document RTC client properties

[Bjorn Andersson]

On Fri, Aug 07, 2026 at 02:26:30AM +0530, Kamal Wadhwa wrote:
> The pmic_glink driver can now register an RTC auxiliary device on
> glymur, whose PMIC exposes an RTC over GLINK (see rtc-glink driver).
> Document the allow-set-time flag it consumes from the parent
> pmic-glink node.

The driver functionality has no relevance to the DeviceTree binding.

The binding defines the contract between the DeviceTree (DTB) and the
implementation, such that the implementation knows what to expect from
and how to interpret the DTB.

> 
> Assisted-by: Claude:claude-sonnet-5
> Signed-off-by: Raj Aryan <raryan@qti.qualcomm.com>
> Signed-off-by: Kamal Wadhwa <kamal.wadhwa@oss.qualcomm.com>

As with the driver, the author/s-o-b chain is wrong.

> ---
>  Documentation/devicetree/bindings/soc/qcom/qcom,pmic-glink.yaml | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,pmic-glink.yaml b/Documentation/devicetree/bindings/soc/qcom/qcom,pmic-glink.yaml
> index 16ad4f5698fc..38db1ff473c0 100644
> --- a/Documentation/devicetree/bindings/soc/qcom/qcom,pmic-glink.yaml
> +++ b/Documentation/devicetree/bindings/soc/qcom/qcom,pmic-glink.yaml
> @@ -79,6 +79,11 @@ properties:
>        - const: charge_limit_end
>        - const: charge_limit_delta
>  
> +  allow-set-time:
> +    $ref: /schemas/types.yaml#/definitions/flag
> +    description:
> +      Indicates that the setting of RTC time is allowed by the host CPU.

What property of the hardware/firmware that this describes would cause
you to set or not set this?

Also, what would the expected outcome be if I declare that my Hamoa
hardware has pmic-glink with allow-set-time?

Regards,
Bjorn

> +
>  patternProperties:
>    '^connector@\d$':
>      $ref: /schemas/connector/usb-connector.yaml#
> 
> -- 
> 2.43.0
> 

Re: [PATCH 2/2] rtc: qcom: rtc-glink: Add PMIC GLINK RTC driver and extend pmic_glink

[Alexandre Belloni]

On 07/08/2026 02:26:31+0530, Kamal Wadhwa wrote:
> Add a new RTC driver that communicates with the PMIC firmware over
> the GLINK transport layer. The driver supports reading time and
> alarm functionality via GET_PROPERTY and SET_PROPERTY opcodes.
> 
> Time is read via GET_PROPERTY (opcode 0x62) using QCOM_RTC_GLINK_TIME
> property (0x00). Alarm set and enable are handled via SET_PROPERTY
> (opcode 0x63) using QCOM_RTC_GLINK_ALARM and QCOM_RTC_GLINK_ALARM_ENABLE
> properties respectively.
> 
> GET_PROPERTY and SET_PROPERTY responses share the same firmware message
> layout, with the firmware echoing back the property value. Both opcodes
> are handled by a common callback path accordingly.
> 
> Also extend pmic_glink to register the RTC as a PMIC GLINK client and
> add PMIC GLINK RTC client support for glymur.
> 
> Assisted-by: Claude:claude-sonnet-5
> Signed-off-by: Raj Aryan <raryan@qti.qualcomm.com>
> Signed-off-by: Kamal Wadhwa <kamal.wadhwa@oss.qualcomm.com>
> ---
>  MAINTAINERS                         |   7 +
>  drivers/rtc/Kconfig                 |  10 +
>  drivers/rtc/Makefile                |   1 +
>  drivers/rtc/rtc-qcom-glink.c        | 690 ++++++++++++++++++++++++++++++++++++
>  drivers/soc/qcom/pmic_glink.c       |  22 +-
>  include/linux/soc/qcom/pmic_glink.h |   1 +
>  6 files changed, 730 insertions(+), 1 deletion(-)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 5f178a5ff026..752cf667b8ad 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -22505,6 +22505,13 @@ S:	Maintained
>  F:	Documentation/devicetree/bindings/power/supply/qcom,pmi8998-charger.yaml
>  F:	drivers/power/supply/qcom_smbx.c
>  
> +QUALCOMM PMIC GLINK RTC DRIVER
> +M:	Kamal Wadhwa <kamal.wadhwa@oss.qualcomm.com>
> +L:	linux-rtc@vger.kernel.org
> +L:	linux-arm-msm@vger.kernel.org
> +S:	Maintained
> +F:	drivers/rtc/rtc-qcom-glink.c
> +
>  QUALCOMM PPE DRIVER
>  M:	Luo Jie <jie.luo@oss.qualcomm.com>
>  L:	netdev@vger.kernel.org
> diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
> index 01def8231873..3ac39704b2e1 100644
> --- a/drivers/rtc/Kconfig
> +++ b/drivers/rtc/Kconfig
> @@ -1836,6 +1836,16 @@ config RTC_DRV_PM8XXX
>  	  To compile this driver as a module, choose M here: the
>  	  module will be called rtc-pm8xxx.
>  
> +config RTC_DRV_QCOM_GLINK
> +	tristate "Qualcomm PMIC GLINK RTC"
> +	depends on QCOM_PMIC_GLINK
> +	help
> +	  Say Y here to enable support for the RTC exposed by the PMIC
> +	  over the GLINK transport on Qualcomm platforms.
> +
> +	  This driver can also be built as a module. If so, the module
> +	  will be called rtc-qcom-glink.
> +
>  config RTC_DRV_TEGRA
>  	tristate "NVIDIA Tegra Internal RTC driver"
>  	depends on ARCH_TEGRA || COMPILE_TEST
> diff --git a/drivers/rtc/Makefile b/drivers/rtc/Makefile
> index 0347645b021f..8215a1fc29fb 100644
> --- a/drivers/rtc/Makefile
> +++ b/drivers/rtc/Makefile
> @@ -73,6 +73,7 @@ obj-$(CONFIG_RTC_DRV_FM3130)	+= rtc-fm3130.o
>  obj-$(CONFIG_RTC_DRV_FSL_FTM_ALARM)	+= rtc-fsl-ftm-alarm.o
>  obj-$(CONFIG_RTC_DRV_FTRTC010)	+= rtc-ftrtc010.o
>  obj-$(CONFIG_RTC_DRV_GENERIC)	+= rtc-generic.o
> +obj-$(CONFIG_RTC_DRV_QCOM_GLINK) += rtc-qcom-glink.o
>  obj-$(CONFIG_RTC_DRV_GOLDFISH)	+= rtc-goldfish.o
>  obj-$(CONFIG_RTC_DRV_HID_SENSOR_TIME) += rtc-hid-sensor-time.o
>  obj-$(CONFIG_RTC_DRV_HYM8563)	+= rtc-hym8563.o
> diff --git a/drivers/rtc/rtc-qcom-glink.c b/drivers/rtc/rtc-qcom-glink.c
> new file mode 100644
> index 000000000000..611f06a3cc5c
> --- /dev/null
> +++ b/drivers/rtc/rtc-qcom-glink.c
> @@ -0,0 +1,690 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
> + */
> +#include <linux/auxiliary_bus.h>
> +#include <linux/completion.h>
> +#include <linux/device.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
> +#include <linux/property.h>
> +#include <linux/rtc.h>
> +#include <linux/slab.h>
> +#include <linux/soc/qcom/pdr.h>
> +#include <linux/soc/qcom/pmic_glink.h>
> +#include <linux/timekeeping.h>
> +#include <linux/workqueue.h>
> +
> +#define RTC_GLINK_SET_PROPERTY		0x63
> +#define RTC_GLINK_GET_PROPERTY		0x65
> +#define RTC_GLINK_GET_RTC_TICKS		0x67
> +#define RTC_GLINK_GET_REAL_TIME		0x68
> +#define RTC_GLINK_SET_REAL_TIME		0x69
> +#define RTC_GLINK_ALARM_EXPIRED		0x6A
> +
> +enum qcom_rtc_glink_properties {
> +	QCOM_RTC_GLINK_TIME = 0,
> +	QCOM_RTC_GLINK_ALARM_TIME,
> +	QCOM_RTC_GLINK_ALARM_ENABLE,
> +};
> +
> +struct qcom_rtc_glink_msg {
> +	struct pmic_glink_hdr	hdr;
> +	__le32			property;
> +	__le32			value;
> +};
> +
> +struct qcom_rtc_glink_generic_req {
> +	struct pmic_glink_hdr	hdr;
> +};
> +
> +struct qcom_rtc_glink_status_resp {
> +	struct pmic_glink_hdr	hdr;
> +	__le32			return_status;
> +};
> +
> +struct qcom_rtc_glink_ticks_resp {
> +	struct pmic_glink_hdr	hdr;
> +	__le32			return_status;
> +	__le32			rtc_ticks;
> +};
> +
> +struct qcom_rtc_glink_real_time_resp {
> +	struct pmic_glink_hdr	hdr;
> +	__le32			return_status;
> +	__le32			real_time_data[4];
> +};
> +
> +struct qcom_rtc_glink_set_real_time_req {
> +	struct pmic_glink_hdr	hdr;
> +	__le32			real_time_data[4];
> +};
> +
> +struct qcom_rtc_glink {
> +	struct device		*dev;
> +	struct pmic_glink_client *client;
> +	struct rtc_device	*rtc;
> +
> +	/* Serializes requests: only one may be in flight at a time */
> +	struct mutex		lock;

This mutex is useless as all the RTC operations are already serialized.
Simply rtc_lock/rtc_unlock in qcom_rtc_glink_alarm_work.

> +	struct completion	ack;
> +	struct work_struct	alarm_work;
> +
> +	/* Protects service_up and request_pending across callback contexts */
> +	spinlock_t		state_lock;
> +	bool			service_up;
> +	bool			request_pending;
> +
> +	int			error;
> +	u32			resp_value;
> +	u32			offset;
> +	struct rtc_time		resp_tm;
> +	bool			resp_valid;
> +	bool			allow_set_time;
> +};
> +
> +static int qcom_rtc_glink_request(struct qcom_rtc_glink *rtc_glink,
> +				  void *data, size_t len)
> +{
> +	unsigned long flags;
> +	unsigned long left;
> +	int ret;
> +
> +	/*
> +	 * Reinit the completion before request_pending becomes visible to
> +	 * qcom_rtc_glink_pdr_tify(), so a concurrent SSR down-transition
> +	 * can't complete() a stale completion that gets wiped out by
> +	 * reinit_completion() right after.
> +	 */
> +	reinit_completion(&rtc_glink->ack);
> +	rtc_glink->error = 0;
> +	rtc_glink->resp_valid = false;
> +
> +	spin_lock_irqsave(&rtc_glink->state_lock, flags);
> +	if (!rtc_glink->service_up) {
> +		spin_unlock_irqrestore(&rtc_glink->state_lock, flags);
> +		return -ECONNRESET;
> +	}
> +	rtc_glink->request_pending = true;
> +	spin_unlock_irqrestore(&rtc_glink->state_lock, flags);
> +
> +	ret = pmic_glink_send(rtc_glink->client, data, len);
> +	if (ret < 0) {
> +		spin_lock_irqsave(&rtc_glink->state_lock, flags);
> +		rtc_glink->request_pending = false;
> +		spin_unlock_irqrestore(&rtc_glink->state_lock, flags);
> +		return ret;
> +	}
> +
> +	left = wait_for_completion_timeout(&rtc_glink->ack, HZ);
> +	spin_lock_irqsave(&rtc_glink->state_lock, flags);
> +	rtc_glink->request_pending = false;
> +	spin_unlock_irqrestore(&rtc_glink->state_lock, flags);
> +	if (!left)
> +		return -ETIMEDOUT;
> +
> +	return rtc_glink->error;
> +}
> +
> +static int qcom_rtc_glink_set_property(struct qcom_rtc_glink *rtc_glink,
> +				       u32 property, u32 value)
> +{
> +	struct qcom_rtc_glink_msg msg = {
> +		.hdr.owner = cpu_to_le32(PMIC_GLINK_OWNER_RTC),
> +		.hdr.type = cpu_to_le32(PMIC_GLINK_REQ_RESP),
> +		.hdr.opcode = cpu_to_le32(RTC_GLINK_SET_PROPERTY),
> +		.property = cpu_to_le32(property),
> +		.value = cpu_to_le32(value),
> +	};
> +	int ret;
> +
> +	dev_dbg(rtc_glink->dev,
> +		"TX opcode=0x%x property=%u value=%u\n",
> +		RTC_GLINK_SET_PROPERTY, property, value);
> +	mutex_lock(&rtc_glink->lock);
> +	ret = qcom_rtc_glink_request(rtc_glink, &msg, sizeof(msg));
> +	mutex_unlock(&rtc_glink->lock);
> +	return ret;
> +}
> +
> +static int qcom_rtc_glink_get_property(struct qcom_rtc_glink *rtc_glink,
> +				       u32 property, u32 *value)
> +{
> +	struct qcom_rtc_glink_msg msg = {
> +		.hdr.owner = cpu_to_le32(PMIC_GLINK_OWNER_RTC),
> +		.hdr.type = cpu_to_le32(PMIC_GLINK_REQ_RESP),
> +		.hdr.opcode = cpu_to_le32(RTC_GLINK_GET_PROPERTY),
> +		.property = cpu_to_le32(property),
> +	};
> +	int ret;
> +
> +	dev_dbg(rtc_glink->dev,
> +		"TX opcode=0x%x property=%u\n",
> +		RTC_GLINK_GET_PROPERTY, property);
> +	mutex_lock(&rtc_glink->lock);
> +	ret = qcom_rtc_glink_request(rtc_glink, &msg, sizeof(msg));
> +	if (!ret) {
> +		if (!rtc_glink->resp_valid)
> +			ret = -EIO;
> +		else
> +			*value = rtc_glink->resp_value;
> +	}
> +	mutex_unlock(&rtc_glink->lock);
> +	return ret;
> +}
> +
> +static int qcom_rtc_glink_get_ticks(struct qcom_rtc_glink *rtc_glink,
> +				    u32 *ticks)
> +{
> +	struct qcom_rtc_glink_generic_req msg = {
> +		.hdr.owner = cpu_to_le32(PMIC_GLINK_OWNER_RTC),
> +		.hdr.type = cpu_to_le32(PMIC_GLINK_REQ_RESP),
> +		.hdr.opcode = cpu_to_le32(RTC_GLINK_GET_RTC_TICKS),
> +	};
> +	int ret;
> +
> +	dev_dbg(rtc_glink->dev, "TX opcode=0x%x\n", RTC_GLINK_GET_RTC_TICKS);
> +	mutex_lock(&rtc_glink->lock);
> +	ret = qcom_rtc_glink_request(rtc_glink, &msg, sizeof(msg));
> +	if (!ret) {
> +		if (!rtc_glink->resp_valid)
> +			ret = -EIO;
> +		else
> +			*ticks = rtc_glink->resp_value;
> +	}
> +	mutex_unlock(&rtc_glink->lock);
> +	return ret;
> +}
> +
> +static int qcom_rtc_glink_get_real_time(struct qcom_rtc_glink *rtc_glink,
> +					struct rtc_time *tm)
> +{
> +	struct qcom_rtc_glink_generic_req msg = {
> +		.hdr.owner = cpu_to_le32(PMIC_GLINK_OWNER_RTC),
> +		.hdr.type = cpu_to_le32(PMIC_GLINK_REQ_RESP),
> +		.hdr.opcode = cpu_to_le32(RTC_GLINK_GET_REAL_TIME),
> +	};
> +	int ret;
> +
> +	dev_dbg(rtc_glink->dev, "TX opcode=0x%x\n", RTC_GLINK_GET_REAL_TIME);
> +	mutex_lock(&rtc_glink->lock);
> +	ret = qcom_rtc_glink_request(rtc_glink, &msg, sizeof(msg));
> +	if (!ret) {
> +		if (!rtc_glink->resp_valid)
> +			ret = -EIO;
> +		else
> +			*tm = rtc_glink->resp_tm;
> +	}
> +	mutex_unlock(&rtc_glink->lock);
> +	return ret;
> +}
> +
> +static int qcom_rtc_glink_set_real_time(struct qcom_rtc_glink *rtc_glink,
> +					struct rtc_time *tm)
> +{
> +	struct qcom_rtc_glink_set_real_time_req msg = {};
> +	u32 w0, w1;
> +	int ret;
> +
> +	w0 = ((tm->tm_year + 1900) & 0xffff) |
> +	     (((tm->tm_mon + 1) & 0xff) << 16) |
> +	     ((tm->tm_mday & 0xff) << 24);
> +	w1 = (tm->tm_hour & 0xff) |
> +	     ((tm->tm_min & 0xff) << 8) |
> +	     ((tm->tm_sec & 0xff) << 16) |
> +	     (1U << 24);
> +	msg.hdr.owner = cpu_to_le32(PMIC_GLINK_OWNER_RTC);
> +	msg.hdr.type = cpu_to_le32(PMIC_GLINK_REQ_RESP);
> +	msg.hdr.opcode = cpu_to_le32(RTC_GLINK_SET_REAL_TIME);
> +	msg.real_time_data[0] = cpu_to_le32(w0);
> +	msg.real_time_data[1] = cpu_to_le32(w1);
> +	msg.real_time_data[2] = 0;
> +	msg.real_time_data[3] = 0;
> +	dev_dbg(rtc_glink->dev,
> +		"TX opcode=0x%x data=%08x %08x\n",
> +		RTC_GLINK_SET_REAL_TIME, w0, w1);
> +	mutex_lock(&rtc_glink->lock);
> +	ret = qcom_rtc_glink_request(rtc_glink, &msg, sizeof(msg));
> +	mutex_unlock(&rtc_glink->lock);
> +	return ret;
> +}
> +
> +static int qcom_rtc_glink_get_time(struct qcom_rtc_glink *rtc_glink,
> +				   struct rtc_time *time)
> +{
> +	u32 ticks;
> +	int ret;
> +
> +	ret = qcom_rtc_glink_get_real_time(rtc_glink, time);
> +	if (!ret)
> +		return 0;
> +	dev_warn(rtc_glink->dev, "0x68 failed (%d), falling back\n", ret);

This message is useless, either fail or fallback silently

> +
> +	ret = qcom_rtc_glink_get_ticks(rtc_glink, &ticks);
> +	if (!ret) {
> +		rtc_time64_to_tm((time64_t)ticks + rtc_glink->offset, time);
> +		return 0;
> +	}
> +
> +	ret = qcom_rtc_glink_get_property(rtc_glink, QCOM_RTC_GLINK_TIME,
> +					  &ticks);
> +	if (!ret && ticks != U32_MAX) {
> +		rtc_time64_to_tm((time64_t)ticks + rtc_glink->offset, time);
> +		return 0;
> +	}
> +
> +	dev_err(rtc_glink->dev, "all time sources failed\n");

Ditto

> +	return -EIO;
> +}
> +
> +static int qcom_rtc_glink_set_time(struct qcom_rtc_glink *rtc_glink,
> +				   struct rtc_time *time)
> +{
> +	time64_t t = rtc_tm_to_time64(time);
> +	u32 ticks;
> +	int ret;
> +
> +	if (!rtc_glink->allow_set_time)
> +		return -EOPNOTSUPP;
> +
> +	ret = qcom_rtc_glink_set_real_time(rtc_glink, time);
> +	if (ret)
> +		dev_warn(rtc_glink->dev, "0x69 failed (%d)\n", ret);

Ditto

> +
> +	ret = qcom_rtc_glink_set_property(rtc_glink, QCOM_RTC_GLINK_TIME, (u32)t);
> +	if (ret)
> +		return ret;
> +
> +	ret = qcom_rtc_glink_get_ticks(rtc_glink, &ticks);
> +	if (!ret && ticks != U32_MAX) {
> +		rtc_glink->offset = (u32)t - ticks;
> +	} else {
> +		dev_warn(rtc_glink->dev, "no tick source for offset\n");

This is also probably useless. No one is going to read those messages or
do anything about them.

> +		return -EIO;
> +	}
> +
> +	return 0;
> +}
> +
> +static int qcom_rtc_glink_set_alarm_en(struct qcom_rtc_glink *rtc_glink,
> +				       int enabled)
> +{
> +	return qcom_rtc_glink_set_property(rtc_glink,
> +					   QCOM_RTC_GLINK_ALARM_ENABLE,
> +					   enabled);
> +}
> +
> +static int qcom_rtc_glink_set_alarm(struct qcom_rtc_glink *rtc_glink,
> +				    struct rtc_wkalrm *alarm)
> +{
> +	time64_t alarm_t = rtc_tm_to_time64(&alarm->time);
> +	struct rtc_time now_tm;
> +	time64_t now_real, secs_until_alarm;
> +	u32 fw_current, fw_alarm;
> +	int ret;
> +
> +	ret = qcom_rtc_glink_get_time(rtc_glink, &now_tm);
> +	if (ret)
> +		return ret;
> +	now_real = rtc_tm_to_time64(&now_tm);
> +	secs_until_alarm = alarm_t - now_real;
> +	dev_dbg(rtc_glink->dev,
> +		"set_alarm: now_real=%lld alarm_t=%lld delta=%lld\n",
> +		now_real, alarm_t, secs_until_alarm);
> +	if (secs_until_alarm < 0)
> +		return -EINVAL;
> +	if (secs_until_alarm > U32_MAX)
> +		return -ERANGE;
> +
> +	ret = qcom_rtc_glink_get_property(rtc_glink, QCOM_RTC_GLINK_TIME,
> +					  &fw_current);
> +	if (ret)
> +		return ret;
> +	if (fw_current == U32_MAX)
> +		fw_current = 0;
> +	fw_alarm = fw_current + (u32)secs_until_alarm;
> +	dev_dbg(rtc_glink->dev,
> +		"set_alarm: fw_current=%u fw_alarm=%u\n",
> +		fw_current, fw_alarm);
> +
> +	ret = qcom_rtc_glink_set_alarm_en(rtc_glink, 0);
> +	if (ret)
> +		return ret;
> +
> +	ret = qcom_rtc_glink_set_property(rtc_glink,
> +					  QCOM_RTC_GLINK_ALARM_TIME, fw_alarm);
> +	if (ret)
> +		return ret;
> +
> +	return qcom_rtc_glink_set_alarm_en(rtc_glink, alarm->enabled);
> +}
> +
> +static int qcom_rtc_glink_get_alarm(struct qcom_rtc_glink *rtc_glink,
> +				    struct rtc_wkalrm *alarm)
> +{
> +	u32 fw_alarm_time, fw_current, alarm_en;
> +	struct rtc_time now_tm;
> +	time64_t now_real, secs_until_alarm, alarm_real;
> +	int ret;
> +
> +	ret = qcom_rtc_glink_get_property(rtc_glink,
> +					  QCOM_RTC_GLINK_ALARM_TIME,
> +					  &fw_alarm_time);
> +	if (ret)
> +		return ret;
> +
> +	ret = qcom_rtc_glink_get_property(rtc_glink, QCOM_RTC_GLINK_TIME,
> +					  &fw_current);
> +	if (ret)
> +		return ret;
> +
> +	ret = qcom_rtc_glink_get_property(rtc_glink,
> +					  QCOM_RTC_GLINK_ALARM_ENABLE,
> +					  &alarm_en);
> +	if (ret)
> +		return ret;
> +	if (fw_current == U32_MAX)
> +		fw_current = 0;
> +
> +	ret = qcom_rtc_glink_get_time(rtc_glink, &now_tm);
> +	if (ret)
> +		return ret;
> +	now_real = rtc_tm_to_time64(&now_tm);
> +	secs_until_alarm = (time64_t)fw_alarm_time - (time64_t)fw_current;
> +	alarm_real = now_real + secs_until_alarm;
> +	dev_dbg(rtc_glink->dev,
> +		"get_alarm: now_real=%lld fw_current=%u fw_alarm=%u delta=%lld\n",
> +		now_real, fw_current, fw_alarm_time, secs_until_alarm);
> +	rtc_time64_to_tm(alarm_real, &alarm->time);
> +	alarm->enabled = !!alarm_en;
> +	return 0;
> +}
> +
> +static void qcom_rtc_glink_alarm_work(struct work_struct *work)
> +{
> +	struct qcom_rtc_glink *rtc_glink =
> +		container_of(work, struct qcom_rtc_glink, alarm_work);
> +	int ret;
> +
> +	ret = qcom_rtc_glink_set_alarm_en(rtc_glink, 0);
> +	if (ret)
> +		dev_err(rtc_glink->dev,
> +			"failed to disable alarm after expiry (%d)\n", ret);

So what is the user supposed to do with this message? Also, do you
actually have to disable the alarm? I guess it is not going to fire again.

> +	rtc_update_irq(rtc_glink->rtc, 1, RTC_IRQF | RTC_AF);
> +}
> +
> +static void qcom_rtc_glink_callback(const void *data, size_t len, void *priv)
> +{
> +	struct qcom_rtc_glink *rtc_glink = priv;
> +	const struct pmic_glink_hdr *hdr = data;
> +	unsigned long flags;
> +	bool pending;
> +
> +	if (len < sizeof(*hdr))
> +		return;
> +
> +	dev_dbg(rtc_glink->dev,
> +		"RX opcode=0x%x type=0x%x owner=0x%x len=%zu\n",
> +		le32_to_cpu(hdr->opcode), le32_to_cpu(hdr->type),
> +		le32_to_cpu(hdr->owner), len);
> +
> +	if (le32_to_cpu(hdr->opcode) == RTC_GLINK_ALARM_EXPIRED) {
> +		dev_info(rtc_glink->dev, "alarm expired\n");

This is just noise

> +		schedule_work(&rtc_glink->alarm_work);
> +		return;
> +	}
> +
> +	/*
> +	 * Hold state_lock across the pending check and the response fields
> +	 * below: qcom_rtc_glink_pdr_notify() writes the same error/resp_*
> +	 * fields under this lock when it force-completes a request on an
> +	 * SSR down-transition, and the two must not race. Dropping the
> +	 * response here if !pending also covers requests that already
> +	 * timed out or were aborted; qcom_rtc_glink_request() will call
> +	 * reinit_completion() again before this response could be mistaken
> +	 * for a later request's.
> +	 */
> +	spin_lock_irqsave(&rtc_glink->state_lock, flags);
> +	pending = rtc_glink->request_pending;
> +
> +	if (!pending) {
> +		spin_unlock_irqrestore(&rtc_glink->state_lock, flags);
> +		dev_dbg(rtc_glink->dev,
> +			"dropping stale response opcode=0x%x\n",
> +			le32_to_cpu(hdr->opcode));
> +		return;
> +	}
> +
> +	switch (le32_to_cpu(hdr->opcode)) {
> +	case RTC_GLINK_GET_PROPERTY:
> +		fallthrough;
> +	case RTC_GLINK_SET_PROPERTY: {
> +		const struct qcom_rtc_glink_msg *msg = data;
> +
> +		if (len < sizeof(struct pmic_glink_hdr) + sizeof(__le32)) {
> +			rtc_glink->error = -EINVAL;
> +			break;
> +		}
> +		rtc_glink->resp_value = le32_to_cpu(msg->property);
> +		rtc_glink->resp_valid = true;
> +		rtc_glink->error = 0;
> +		dev_dbg(rtc_glink->dev, "ACK opcode=0x%x resp_value=%u\n",
> +			le32_to_cpu(hdr->opcode), rtc_glink->resp_value);
> +		break;
> +	}
> +
> +	case RTC_GLINK_GET_RTC_TICKS: {
> +		const struct qcom_rtc_glink_ticks_resp *resp = data;
> +
> +		if (len < sizeof(*resp)) {
> +			rtc_glink->error = -EINVAL;
> +			break;
> +		}
> +		if (le32_to_cpu(resp->return_status)) {
> +			rtc_glink->error = -EIO;
> +			break;
> +		}
> +		rtc_glink->resp_value = le32_to_cpu(resp->rtc_ticks);
> +		rtc_glink->resp_valid = true;
> +		rtc_glink->error = 0;
> +		dev_dbg(rtc_glink->dev, "ACK opcode=0x%x ticks=%u\n",
> +			RTC_GLINK_GET_RTC_TICKS, rtc_glink->resp_value);
> +		break;
> +	}
> +
> +	case RTC_GLINK_GET_REAL_TIME: {
> +		const struct qcom_rtc_glink_real_time_resp *resp = data;
> +		u32 w0, w1;
> +
> +		if (len < sizeof(*resp)) {
> +			rtc_glink->error = -EINVAL;
> +			break;
> +		}
> +		if (le32_to_cpu(resp->return_status)) {
> +			rtc_glink->error = -EIO;
> +			break;
> +		}
> +		w0 = le32_to_cpu(resp->real_time_data[0]);
> +		w1 = le32_to_cpu(resp->real_time_data[1]);
> +		rtc_glink->resp_tm.tm_year = (w0 & 0xffff) - 1900;
> +		rtc_glink->resp_tm.tm_mon = ((w0 >> 16) & 0xff) - 1;
> +		rtc_glink->resp_tm.tm_mday = (w0 >> 24) & 0xff;
> +		rtc_glink->resp_tm.tm_hour = w1 & 0xff;
> +		rtc_glink->resp_tm.tm_min = (w1 >> 8) & 0xff;
> +		rtc_glink->resp_tm.tm_sec = (w1 >> 16) & 0xff;
> +		rtc_glink->resp_valid = true;
> +		rtc_glink->error = 0;
> +		dev_dbg(rtc_glink->dev,
> +			"ACK opcode=0x%x %04d-%02d-%02d %02d:%02d:%02d\n",
> +			RTC_GLINK_GET_REAL_TIME,
> +			rtc_glink->resp_tm.tm_year + 1900,
> +			rtc_glink->resp_tm.tm_mon + 1,
> +			rtc_glink->resp_tm.tm_mday,
> +			rtc_glink->resp_tm.tm_hour,
> +			rtc_glink->resp_tm.tm_min,
> +			rtc_glink->resp_tm.tm_sec);
> +		break;
> +	}
> +
> +	case RTC_GLINK_SET_REAL_TIME: {
> +		const struct qcom_rtc_glink_status_resp *resp = data;
> +
> +		if (len < sizeof(*resp)) {
> +			rtc_glink->error = -EINVAL;
> +			break;
> +		}
> +		rtc_glink->error = le32_to_cpu(resp->return_status) ? -EIO : 0;
> +		dev_dbg(rtc_glink->dev, "ACK opcode=0x%x status=%u\n",
> +			RTC_GLINK_SET_REAL_TIME,
> +			le32_to_cpu(resp->return_status));
> +		break;
> +	}
> +
> +	default:
> +		spin_unlock_irqrestore(&rtc_glink->state_lock, flags);
> +		dev_warn(rtc_glink->dev,
> +			 "unhandled RX opcode=0x%x len=%zu\n",
> +			 le32_to_cpu(hdr->opcode), len);

Useless message

> +		return;
> +	}
> +
> +	spin_unlock_irqrestore(&rtc_glink->state_lock, flags);
> +	complete(&rtc_glink->ack);
> +}
> +
> +static void qcom_rtc_glink_pdr_notify(void *priv, int state)
> +{
> +	struct qcom_rtc_glink *rtc_glink = priv;
> +	unsigned long flags;
> +	bool up = (state == SERVREG_SERVICE_STATE_UP);
> +
> +	spin_lock_irqsave(&rtc_glink->state_lock, flags);
> +	rtc_glink->service_up = up;
> +	if (!up && rtc_glink->request_pending) {
> +		rtc_glink->error = -ECONNRESET;
> +		rtc_glink->resp_valid = false;
> +		complete(&rtc_glink->ack);
> +	}
> +	spin_unlock_irqrestore(&rtc_glink->state_lock, flags);
> +}
> +
> +static int qcom_rtc_glink_read_time(struct device *dev, struct rtc_time *tm)
> +{
> +	return qcom_rtc_glink_get_time(dev_get_drvdata(dev), tm);
> +}
> +
> +static int qcom_rtc_glink_set_time_dev(struct device *dev, struct rtc_time *tm)
> +{
> +	return qcom_rtc_glink_set_time(dev_get_drvdata(dev), tm);
> +}
> +
> +static int qcom_rtc_glink_read_alarm(struct device *dev,
> +				     struct rtc_wkalrm *alrm)
> +{
> +	return qcom_rtc_glink_get_alarm(dev_get_drvdata(dev), alrm);
> +}
> +
> +static int qcom_rtc_glink_set_alarm_dev(struct device *dev,
> +					struct rtc_wkalrm *alrm)
> +{
> +	return qcom_rtc_glink_set_alarm(dev_get_drvdata(dev), alrm);
> +}
> +
> +static int qcom_rtc_glink_alarm_irq_enable(struct device *dev,
> +					   unsigned int enabled)
> +{
> +	return qcom_rtc_glink_set_alarm_en(dev_get_drvdata(dev), enabled);
> +}
> +
> +static const struct rtc_class_ops qcom_rtc_glink_rtc_ops = {
> +	.read_time = qcom_rtc_glink_read_time,
> +	.set_time = qcom_rtc_glink_set_time_dev,
> +	.read_alarm = qcom_rtc_glink_read_alarm,
> +	.set_alarm = qcom_rtc_glink_set_alarm_dev,
> +	.alarm_irq_enable = qcom_rtc_glink_alarm_irq_enable,
> +};
> +
> +static const struct of_device_id qcom_rtc_glink_of_variants[] = {
> +	{ .compatible = "qcom,glymur-pmic-glink" },
> +	{}
> +};
> +
> +static int qcom_rtc_glink_probe(struct auxiliary_device *adev,
> +				const struct auxiliary_device_id *id)
> +{
> +	struct device *dev = &adev->dev;
> +	struct qcom_rtc_glink *rtc_glink;
> +
> +	if (!of_match_device(qcom_rtc_glink_of_variants, dev->parent))
> +		return -ENXIO;
> +
> +	rtc_glink = devm_kzalloc(dev, sizeof(*rtc_glink), GFP_KERNEL);
> +	if (!rtc_glink)
> +		return -ENOMEM;
> +
> +	rtc_glink->dev            = dev;
> +	rtc_glink->allow_set_time = device_property_read_bool(dev->parent,
> +							      "allow-set-time");
> +
> +	mutex_init(&rtc_glink->lock);
> +	spin_lock_init(&rtc_glink->state_lock);
> +	init_completion(&rtc_glink->ack);
> +	INIT_WORK(&rtc_glink->alarm_work, qcom_rtc_glink_alarm_work);
> +	dev_set_drvdata(dev, rtc_glink);
> +
> +	rtc_glink->client = devm_pmic_glink_client_alloc(dev,
> +							 PMIC_GLINK_OWNER_RTC,
> +							 qcom_rtc_glink_callback,
> +							 qcom_rtc_glink_pdr_notify,
> +							 rtc_glink);
> +	if (IS_ERR(rtc_glink->client))
> +		return dev_err_probe(dev, PTR_ERR(rtc_glink->client),
> +				     "failed to allocate glink client\n");
> +
> +	pmic_glink_client_register(rtc_glink->client);
> +
> +	device_init_wakeup(dev, true);
> +
> +	rtc_glink->rtc = devm_rtc_allocate_device(dev);
> +	if (IS_ERR(rtc_glink->rtc))
> +		return dev_err_probe(dev, PTR_ERR(rtc_glink->rtc),
> +				     "failed to allocate RTC device\n");
> +

Useless message, there is no failure path without a message in
devm_rtc_allocate_device.


> +	rtc_glink->rtc->ops      = &qcom_rtc_glink_rtc_ops;
> +	rtc_glink->rtc->range_min = 0;
> +	rtc_glink->rtc->range_max = U32_MAX;
> +
> +	return devm_rtc_register_device(rtc_glink->rtc);
> +}
> +
> +static void qcom_rtc_glink_remove(struct auxiliary_device *adev)
> +{
> +	struct qcom_rtc_glink *rtc_glink = dev_get_drvdata(&adev->dev);
> +	/*
> +	 * Nothing left to arm alarm_work after this returns: the glink
> +	 * client (and its callback/pdr_notify) is torn down by devm after
> +	 * .remove() returns, and rtc_update_irq() below still targets a
> +	 * live devm-managed rtc device.
> +	 */
> +	cancel_work_sync(&rtc_glink->alarm_work);
> +}
> +
> +static const struct auxiliary_device_id qcom_rtc_glink_id_table[] = {
> +	{ .name = "pmic_glink.rtc-glink" },
> +	{}
> +};
> +
> +MODULE_DEVICE_TABLE(auxiliary, qcom_rtc_glink_id_table);
> +
> +static struct auxiliary_driver qcom_rtc_glink_driver = {
> +	.name = "qcom_pmic_rtc_glink",
> +	.id_table = qcom_rtc_glink_id_table,
> +	.probe = qcom_rtc_glink_probe,
> +	.remove = qcom_rtc_glink_remove,
> +};
> +
> +module_auxiliary_driver(qcom_rtc_glink_driver);
> +MODULE_DESCRIPTION("Qualcomm PMIC GLINK RTC driver");
> +MODULE_LICENSE("GPL");
> diff --git a/drivers/soc/qcom/pmic_glink.c b/drivers/soc/qcom/pmic_glink.c
> index 3042261578aa..7ee70a31ba15 100644
> --- a/drivers/soc/qcom/pmic_glink.c
> +++ b/drivers/soc/qcom/pmic_glink.c
> @@ -21,6 +21,7 @@ enum {
>  	PMIC_GLINK_CLIENT_BATT = 0,
>  	PMIC_GLINK_CLIENT_ALTMODE,
>  	PMIC_GLINK_CLIENT_UCSI,
> +	PMIC_GLINK_CLIENT_RTC,
>  };
>  
>  struct pmic_glink_data {
> @@ -40,6 +41,7 @@ struct pmic_glink {
>  	struct auxiliary_device altmode_aux;
>  	struct auxiliary_device ps_aux;
>  	struct auxiliary_device ucsi_aux;
> +	struct auxiliary_device rtc_aux;
>  
>  	/* serializing client_state and pdr_state updates */
>  	struct mutex state_lock;
> @@ -341,6 +343,12 @@ static int pmic_glink_probe(struct platform_device *pdev)
>  			goto out_release_altmode_aux;
>  	}
>  
> +	if (pg->data->client_mask & BIT(PMIC_GLINK_CLIENT_RTC)) {
> +		ret = pmic_glink_add_aux_device(pg, &pg->rtc_aux, "rtc-glink");
> +		if (ret)
> +			goto out_release_ps_aux;
> +	}
> +
>  	if (pg->data->charger_pdr_service_name && pg->data->charger_pdr_service_path) {
>  		service = pdr_add_lookup(pg->pdr, pg->data->charger_pdr_service_name,
>  					 pg->data->charger_pdr_service_path);
> @@ -358,6 +366,9 @@ static int pmic_glink_probe(struct platform_device *pdev)
>  	return 0;
>  
>  out_release_aux_devices:
> +	if (pg->data->client_mask & BIT(PMIC_GLINK_CLIENT_RTC))
> +		pmic_glink_del_aux_device(pg, &pg->rtc_aux);
> +out_release_ps_aux:
>  	if (pg->data->client_mask & BIT(PMIC_GLINK_CLIENT_BATT))
>  		pmic_glink_del_aux_device(pg, &pg->ps_aux);
>  out_release_altmode_aux:
> @@ -384,11 +395,20 @@ static void pmic_glink_remove(struct platform_device *pdev)
>  		pmic_glink_del_aux_device(pg, &pg->altmode_aux);
>  	if (pg->data->client_mask & BIT(PMIC_GLINK_CLIENT_UCSI))
>  		pmic_glink_del_aux_device(pg, &pg->ucsi_aux);
> +	if (pg->data->client_mask & BIT(PMIC_GLINK_CLIENT_RTC))
> +		pmic_glink_del_aux_device(pg, &pg->rtc_aux);
>  
>  	guard(mutex)(&__pmic_glink_lock);
>  	__pmic_glink = NULL;
>  }
>  
> +static const struct pmic_glink_data pmic_glink_glymur_data = {
> +	.client_mask = BIT(PMIC_GLINK_CLIENT_BATT) |
> +		       BIT(PMIC_GLINK_CLIENT_ALTMODE) |
> +		       BIT(PMIC_GLINK_CLIENT_UCSI) |
> +		       BIT(PMIC_GLINK_CLIENT_RTC),
> +};
> +
>  static const struct pmic_glink_data pmic_glink_adsp_data = {
>  	.client_mask = BIT(PMIC_GLINK_CLIENT_BATT) |
>  		       BIT(PMIC_GLINK_CLIENT_ALTMODE) |
> @@ -404,7 +424,7 @@ static const struct pmic_glink_data pmic_glink_soccp_data = {
>  };
>  
>  static const struct of_device_id pmic_glink_of_match[] = {
> -	{ .compatible = "qcom,glymur-pmic-glink", .data = &pmic_glink_soccp_data },
> +	{ .compatible = "qcom,glymur-pmic-glink", .data = &pmic_glink_glymur_data },
>  	{ .compatible = "qcom,kaanapali-pmic-glink", .data = &pmic_glink_soccp_data },
>  	{ .compatible = "qcom,pmic-glink", .data = &pmic_glink_adsp_data },
>  	{}
> diff --git a/include/linux/soc/qcom/pmic_glink.h b/include/linux/soc/qcom/pmic_glink.h
> index 7cddf1027752..11031b06b810 100644
> --- a/include/linux/soc/qcom/pmic_glink.h
> +++ b/include/linux/soc/qcom/pmic_glink.h
> @@ -11,6 +11,7 @@ struct pmic_glink_client;
>  #define PMIC_GLINK_OWNER_BATTMGR	32778
>  #define PMIC_GLINK_OWNER_USBC		32779
>  #define PMIC_GLINK_OWNER_USBC_PAN	32780
> +#define PMIC_GLINK_OWNER_RTC		32784
>  
>  #define PMIC_GLINK_REQ_RESP		1
>  #define PMIC_GLINK_NOTIFY		2
> 
> -- 
> 2.43.0
> 

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