[PATCH 0/4] rtc/scmi: Support multiple RTCs

[PATCH 0/4] rtc/scmi: Support multiple RTCs

[Peng Fan (OSS)]

i.MX95 System Manager(SM) BBM protocol exports two RTCs for EVK board.
one RTC is SoC internal RTC, the other is board RTC.

The current driver only use the 1st RTC. With this patchset, both RTCs
could be used in Linux. To achieve this:

1. Support more event sources for BBM protocol
2. Add bbm_info hook to let users could query the number of RTCs
3. Introduce devm_rtc_allocate_device_priv to support setting rtc device
   private information
4. Update rtc-imx-sm-bbm.c to register both RTCs

Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
Peng Fan (4):
      firmware: arm_scmi: imx: Support more event sources
      firmware: arm_scmi: imx: Introduce bbm_info hook
      rtc: Introduce devm_rtc_allocate_device_priv
      rtc: imx-sm-bbm: Support multiple RTCs

 drivers/firmware/arm_scmi/vendors/imx/imx-sm-bbm.c | 33 ++++++++++-
 drivers/rtc/class.c                                |  9 ++-
 drivers/rtc/dev.c                                  |  8 ++-
 drivers/rtc/interface.c                            | 16 ++---
 drivers/rtc/proc.c                                 |  2 +-
 drivers/rtc/rtc-imx-sm-bbm.c                       | 69 ++++++++++++++--------
 include/linux/rtc.h                                |  2 +
 include/linux/scmi_imx_protocol.h                  |  2 +
 8 files changed, 100 insertions(+), 41 deletions(-)
---
base-commit: e7bb221a638962d487231ac45a6699fb9bb8f9fa
change-id: 20250116-rtc-3834e01786a8

Best regards,
-- 
Peng Fan <peng.fan@nxp.com>

[PATCH 1/4] firmware: arm_scmi: imx: Support more event sources

[Peng Fan (OSS)]

From: Peng Fan <peng.fan@nxp.com>

The i.MX System Manager BBM protocol has the capability to support
more than one RTCs. Add scmi_imx_bbm_get_num_sources to replace
num_sources which was fixed to 1. Then the 2nd RTC event could be
configured.

Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/firmware/arm_scmi/vendors/imx/imx-sm-bbm.c | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/drivers/firmware/arm_scmi/vendors/imx/imx-sm-bbm.c b/drivers/firmware/arm_scmi/vendors/imx/imx-sm-bbm.c
index aa176c1a5eefe4220e54e366cf3a267de639fa9b..86fadfe8e3560b1cab5876a1029e38d91d938e2f 100644
--- a/drivers/firmware/arm_scmi/vendors/imx/imx-sm-bbm.c
+++ b/drivers/firmware/arm_scmi/vendors/imx/imx-sm-bbm.c
@@ -128,7 +128,7 @@ static int scmi_imx_bbm_notify(const struct scmi_protocol_handle *ph,
 			return ret;
 
 		rtc_notify = t->tx.buf;
-		rtc_notify->rtc_id = cpu_to_le32(0);
+		rtc_notify->rtc_id = cpu_to_le32(src_id);
 		rtc_notify->flags =
 			cpu_to_le32(enable ? SCMI_IMX_BBM_NOTIFY_RTC_FLAG : 0);
 	} else if (message_id == IMX_BBM_BUTTON_NOTIFY) {
@@ -156,6 +156,20 @@ static enum scmi_imx_bbm_protocol_cmd evt_2_cmd[] = {
 	IMX_BBM_BUTTON_NOTIFY
 };
 
+static int scmi_imx_bbm_get_num_sources(const struct scmi_protocol_handle *ph)
+{
+	struct scmi_imx_bbm_info *pi = ph->get_priv(ph);
+
+	if (!pi)
+		return -EINVAL;
+
+	/*
+	 * There is RTC and Button, but there is only one BBM button, and
+	 * at least one RTC, so use nr_rtc as sources number
+	 */
+	return pi->nr_rtc;
+}
+
 static int scmi_imx_bbm_set_notify_enabled(const struct scmi_protocol_handle *ph,
 					   u8 evt_id, u32 src_id, bool enable)
 {
@@ -220,6 +234,7 @@ static const struct scmi_event scmi_imx_bbm_events[] = {
 };
 
 static const struct scmi_event_ops scmi_imx_bbm_event_ops = {
+	.get_num_sources = scmi_imx_bbm_get_num_sources,
 	.set_notify_enabled = scmi_imx_bbm_set_notify_enabled,
 	.fill_custom_report = scmi_imx_bbm_fill_custom_report,
 };
@@ -229,7 +244,6 @@ static const struct scmi_protocol_events scmi_imx_bbm_protocol_events = {
 	.ops = &scmi_imx_bbm_event_ops,
 	.evts = scmi_imx_bbm_events,
 	.num_events = ARRAY_SIZE(scmi_imx_bbm_events),
-	.num_sources = 1,
 };
 
 static int scmi_imx_bbm_rtc_time_set(const struct scmi_protocol_handle *ph,

-- 
2.37.1

[PATCH 2/4] firmware: arm_scmi: imx: Introduce bbm_info hook

[Peng Fan (OSS)]

From: Peng Fan <peng.fan@nxp.com>

Add bbm_info hook to let BBM protocol users could query the
how many RTCs and GPRs are supported by SCMI platform.

Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/firmware/arm_scmi/vendors/imx/imx-sm-bbm.c | 15 +++++++++++++++
 include/linux/scmi_imx_protocol.h                  |  2 ++
 2 files changed, 17 insertions(+)

diff --git a/drivers/firmware/arm_scmi/vendors/imx/imx-sm-bbm.c b/drivers/firmware/arm_scmi/vendors/imx/imx-sm-bbm.c
index 86fadfe8e3560b1cab5876a1029e38d91d938e2f..9d40ea817f4bdd2304c932bf3f52b7673a35eaff 100644
--- a/drivers/firmware/arm_scmi/vendors/imx/imx-sm-bbm.c
+++ b/drivers/firmware/arm_scmi/vendors/imx/imx-sm-bbm.c
@@ -246,6 +246,20 @@ static const struct scmi_protocol_events scmi_imx_bbm_protocol_events = {
 	.num_events = ARRAY_SIZE(scmi_imx_bbm_events),
 };
 
+static int scmi_imx_bbm_info(const struct scmi_protocol_handle *ph, u32 *nr_rtc,
+			     u32 *nr_gpr)
+{
+	struct scmi_imx_bbm_info *pi = ph->get_priv(ph);
+
+	if (nr_rtc)
+		*nr_rtc = pi->nr_rtc;
+
+	if (nr_gpr)
+		*nr_gpr = pi->nr_gpr;
+
+	return 0;
+}
+
 static int scmi_imx_bbm_rtc_time_set(const struct scmi_protocol_handle *ph,
 				     u32 rtc_id, u64 sec)
 {
@@ -351,6 +365,7 @@ static int scmi_imx_bbm_button_get(const struct scmi_protocol_handle *ph, u32 *s
 }
 
 static const struct scmi_imx_bbm_proto_ops scmi_imx_bbm_proto_ops = {
+	.bbm_info = scmi_imx_bbm_info,
 	.rtc_time_get = scmi_imx_bbm_rtc_time_get,
 	.rtc_time_set = scmi_imx_bbm_rtc_time_set,
 	.rtc_alarm_set = scmi_imx_bbm_rtc_alarm_set,
diff --git a/include/linux/scmi_imx_protocol.h b/include/linux/scmi_imx_protocol.h
index 53b356a26414279f4aaaa8287c32209ed1ad57b4..a0e7e99c4f43ba3e735f50b9eadbfa07a7803947 100644
--- a/include/linux/scmi_imx_protocol.h
+++ b/include/linux/scmi_imx_protocol.h
@@ -20,6 +20,8 @@
 #define SCMI_IMX_SUBVENDOR	"IMX"
 
 struct scmi_imx_bbm_proto_ops {
+	int (*bbm_info)(const struct scmi_protocol_handle *ph, u32 *nr_rtc,
+			u32 *nr_gpr);
 	int (*rtc_time_set)(const struct scmi_protocol_handle *ph, u32 id,
 			    uint64_t sec);
 	int (*rtc_time_get)(const struct scmi_protocol_handle *ph, u32 id,

-- 
2.37.1

[PATCH 3/4] rtc: Introduce devm_rtc_allocate_device_priv

[Peng Fan (OSS)]

From: Peng Fan <peng.fan@nxp.com>

Current users of rtc_class_ops->x are using 'rtc->dev.parent', so
there is no way for rtc drivers get rtc private information. Take
i.MX95 for example, i.MX95 SCMI BBM Protocol supports two RTCs
for i.MX95 EVK board. Using 'rtc->dev.parent' causing driver not
not able to know the exact RTC device. So introduce 'priv' and
devm_rtc_allocate_device_priv for driver to set rtc device private
information.

Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/rtc/class.c     |  9 ++++++++-
 drivers/rtc/dev.c       |  8 +++++---
 drivers/rtc/interface.c | 16 ++++++++--------
 drivers/rtc/proc.c      |  2 +-
 include/linux/rtc.h     |  2 ++
 5 files changed, 24 insertions(+), 13 deletions(-)

diff --git a/drivers/rtc/class.c b/drivers/rtc/class.c
index e31fa0ad127e9545afac745a621d4ccbcd5fca28..67413600785d806fe4da441483ce1280357d8791 100644
--- a/drivers/rtc/class.c
+++ b/drivers/rtc/class.c
@@ -361,7 +361,7 @@ static void devm_rtc_release_device(void *res)
 	put_device(&rtc->dev);
 }
 
-struct rtc_device *devm_rtc_allocate_device(struct device *dev)
+struct rtc_device *devm_rtc_allocate_device_priv(struct device *dev, void *priv)
 {
 	struct rtc_device *rtc;
 	int id, err;
@@ -378,6 +378,7 @@ struct rtc_device *devm_rtc_allocate_device(struct device *dev)
 
 	rtc->id = id;
 	rtc->dev.parent = dev;
+	rtc->priv = priv;
 	err = devm_add_action_or_reset(dev, devm_rtc_release_device, rtc);
 	if (err)
 		return ERR_PTR(err);
@@ -388,6 +389,12 @@ struct rtc_device *devm_rtc_allocate_device(struct device *dev)
 
 	return rtc;
 }
+EXPORT_SYMBOL_GPL(devm_rtc_allocate_device_priv);
+
+struct rtc_device *devm_rtc_allocate_device(struct device *dev)
+{
+	return devm_rtc_allocate_device_priv(dev, NULL);
+}
 EXPORT_SYMBOL_GPL(devm_rtc_allocate_device);
 
 int __devm_rtc_register_device(struct module *owner, struct rtc_device *rtc)
diff --git a/drivers/rtc/dev.c b/drivers/rtc/dev.c
index c4a3ab53dcd4b7280a3a2981fe842729603a1feb..e0e1a488b795645d7c9453490d6cdba510cc5db5 100644
--- a/drivers/rtc/dev.c
+++ b/drivers/rtc/dev.c
@@ -410,7 +410,8 @@ static long rtc_dev_ioctl(struct file *file,
 		}
 		default:
 			if (rtc->ops->param_get)
-				err = rtc->ops->param_get(rtc->dev.parent, &param);
+				err = rtc->ops->param_get(rtc->priv ?
+							  &rtc->dev : rtc->dev.parent, &param);
 			else
 				err = -EINVAL;
 		}
@@ -440,7 +441,8 @@ static long rtc_dev_ioctl(struct file *file,
 
 		default:
 			if (rtc->ops->param_set)
-				err = rtc->ops->param_set(rtc->dev.parent, &param);
+				err = rtc->ops->param_set(rtc->priv ?
+							  &rtc->dev : rtc->dev.parent, &param);
 			else
 				err = -EINVAL;
 		}
@@ -450,7 +452,7 @@ static long rtc_dev_ioctl(struct file *file,
 	default:
 		/* Finally try the driver's ioctl interface */
 		if (ops->ioctl) {
-			err = ops->ioctl(rtc->dev.parent, cmd, arg);
+			err = ops->ioctl(rtc->priv ? &rtc->dev : rtc->dev.parent, cmd, arg);
 			if (err == -ENOIOCTLCMD)
 				err = -ENOTTY;
 		} else {
diff --git a/drivers/rtc/interface.c b/drivers/rtc/interface.c
index aaf76406cd7d7d2cfd5479fc1fc892fcb5efbb6b..06d51761900ee5d6cc3916d31d907505c193c6ad 100644
--- a/drivers/rtc/interface.c
+++ b/drivers/rtc/interface.c
@@ -91,7 +91,7 @@ static int __rtc_read_time(struct rtc_device *rtc, struct rtc_time *tm)
 		err = -EINVAL;
 	} else {
 		memset(tm, 0, sizeof(struct rtc_time));
-		err = rtc->ops->read_time(rtc->dev.parent, tm);
+		err = rtc->ops->read_time(rtc->priv ? &rtc->dev : rtc->dev.parent, tm);
 		if (err < 0) {
 			dev_dbg(&rtc->dev, "read_time: fail to read: %d\n",
 				err);
@@ -155,7 +155,7 @@ int rtc_set_time(struct rtc_device *rtc, struct rtc_time *tm)
 	if (!rtc->ops)
 		err = -ENODEV;
 	else if (rtc->ops->set_time)
-		err = rtc->ops->set_time(rtc->dev.parent, tm);
+		err = rtc->ops->set_time(rtc->priv ? &rtc->dev : rtc->dev.parent, tm);
 	else
 		err = -EINVAL;
 
@@ -200,7 +200,7 @@ static int rtc_read_alarm_internal(struct rtc_device *rtc,
 		alarm->time.tm_wday = -1;
 		alarm->time.tm_yday = -1;
 		alarm->time.tm_isdst = -1;
-		err = rtc->ops->read_alarm(rtc->dev.parent, alarm);
+		err = rtc->ops->read_alarm(rtc->priv ? &rtc->dev : rtc->dev.parent, alarm);
 	}
 
 	mutex_unlock(&rtc->ops_lock);
@@ -441,7 +441,7 @@ static int __rtc_set_alarm(struct rtc_device *rtc, struct rtc_wkalrm *alarm)
 	else if (!test_bit(RTC_FEATURE_ALARM, rtc->features))
 		err = -EINVAL;
 	else
-		err = rtc->ops->set_alarm(rtc->dev.parent, alarm);
+		err = rtc->ops->set_alarm(rtc->priv ? &rtc->dev : rtc->dev.parent, alarm);
 
 	trace_rtc_set_alarm(rtc_tm_to_time64(&alarm->time), err);
 	return err;
@@ -545,7 +545,7 @@ int rtc_alarm_irq_enable(struct rtc_device *rtc, unsigned int enabled)
 	else if (!test_bit(RTC_FEATURE_ALARM, rtc->features) || !rtc->ops->alarm_irq_enable)
 		err = -EINVAL;
 	else
-		err = rtc->ops->alarm_irq_enable(rtc->dev.parent, enabled);
+		err = rtc->ops->alarm_irq_enable(rtc->priv ? &rtc->dev : rtc->dev.parent, enabled);
 
 	mutex_unlock(&rtc->ops_lock);
 
@@ -847,7 +847,7 @@ static void rtc_alarm_disable(struct rtc_device *rtc)
 	if (!rtc->ops || !test_bit(RTC_FEATURE_ALARM, rtc->features) || !rtc->ops->alarm_irq_enable)
 		return;
 
-	rtc->ops->alarm_irq_enable(rtc->dev.parent, false);
+	rtc->ops->alarm_irq_enable(rtc->priv ? &rtc->dev : rtc->dev.parent, false);
 	trace_rtc_alarm_irq_enable(0, 0);
 }
 
@@ -1049,7 +1049,7 @@ int rtc_read_offset(struct rtc_device *rtc, long *offset)
 		return -EINVAL;
 
 	mutex_lock(&rtc->ops_lock);
-	ret = rtc->ops->read_offset(rtc->dev.parent, offset);
+	ret = rtc->ops->read_offset(rtc->priv ? &rtc->dev : rtc->dev.parent, offset);
 	mutex_unlock(&rtc->ops_lock);
 
 	trace_rtc_read_offset(*offset, ret);
@@ -1084,7 +1084,7 @@ int rtc_set_offset(struct rtc_device *rtc, long offset)
 		return -EINVAL;
 
 	mutex_lock(&rtc->ops_lock);
-	ret = rtc->ops->set_offset(rtc->dev.parent, offset);
+	ret = rtc->ops->set_offset(rtc->priv ? &rtc->dev : rtc->dev.parent, offset);
 	mutex_unlock(&rtc->ops_lock);
 
 	trace_rtc_set_offset(offset, ret);
diff --git a/drivers/rtc/proc.c b/drivers/rtc/proc.c
index cbcdbb19d848e78e6674bd626833151a99773ef0..94cc4f9d62b7867018d876f7933468fbd1552ffc 100644
--- a/drivers/rtc/proc.c
+++ b/drivers/rtc/proc.c
@@ -73,7 +73,7 @@ static int rtc_proc_show(struct seq_file *seq, void *offset)
 	seq_printf(seq, "24hr\t\t: yes\n");
 
 	if (ops->proc)
-		ops->proc(rtc->dev.parent, seq);
+		ops->proc(rtc->priv ? &rtc->dev : rtc->dev.parent, seq);
 
 	return 0;
 }
diff --git a/include/linux/rtc.h b/include/linux/rtc.h
index 3f4d315aaec9e641e35c1c86a522f2879bec19ba..a6f3c86a08e1e214062b2a68d9a6a437afb186b3 100644
--- a/include/linux/rtc.h
+++ b/include/linux/rtc.h
@@ -110,6 +110,7 @@ struct rtc_device {
 	struct hrtimer pie_timer; /* sub second exp, so needs hrtimer */
 	int pie_enabled;
 	struct work_struct irqwork;
+	void *priv;
 
 	/*
 	 * This offset specifies the update timing of the RTC.
@@ -182,6 +183,7 @@ extern struct rtc_device *devm_rtc_device_register(struct device *dev,
 					const struct rtc_class_ops *ops,
 					struct module *owner);
 struct rtc_device *devm_rtc_allocate_device(struct device *dev);
+struct rtc_device *devm_rtc_allocate_device_priv(struct device *dev, void *priv);
 int __devm_rtc_register_device(struct module *owner, struct rtc_device *rtc);
 
 extern int rtc_read_time(struct rtc_device *rtc, struct rtc_time *tm);

-- 
2.37.1

[PATCH 4/4] rtc: imx-sm-bbm: Support multiple RTCs

[Peng Fan (OSS)]

From: Peng Fan <peng.fan@nxp.com>

i.MX95 EVK has two RTCs exported by SCMI BBM protocol. Current driver
only enables the 1st RTC inside BBNSM module, leaving the board RTC
not used by Linux.

To use the 2nd RTC, use 'bbm_info' to get the number of RTCs, register
them all, and set 'bbnsm' as private info for rtc device to know which
RTC it is when using rtc_class_ops to access rtc device.

Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/rtc/rtc-imx-sm-bbm.c | 69 +++++++++++++++++++++++++++-----------------
 1 file changed, 43 insertions(+), 26 deletions(-)

diff --git a/drivers/rtc/rtc-imx-sm-bbm.c b/drivers/rtc/rtc-imx-sm-bbm.c
index daa472be7c80697aa3cd3432eccef0c877e4c378..a29b30555d0c0581ecaa8b79760209dc780d2f0e 100644
--- a/drivers/rtc/rtc-imx-sm-bbm.c
+++ b/drivers/rtc/rtc-imx-sm-bbm.c
@@ -15,16 +15,18 @@ struct scmi_imx_bbm {
 	struct rtc_device *rtc_dev;
 	struct scmi_protocol_handle *ph;
 	struct notifier_block nb;
+	u32 bbm_rtc_id;
 };
 
 static int scmi_imx_bbm_read_time(struct device *dev, struct rtc_time *tm)
 {
-	struct scmi_imx_bbm *bbnsm = dev_get_drvdata(dev);
+	struct rtc_device *rtc = to_rtc_device(dev);
+	struct scmi_imx_bbm *bbnsm = rtc->priv;
 	struct scmi_protocol_handle *ph = bbnsm->ph;
 	u64 val;
 	int ret;
 
-	ret = bbnsm->ops->rtc_time_get(ph, 0, &val);
+	ret = bbnsm->ops->rtc_time_get(ph, bbnsm->bbm_rtc_id, &val);
 	if (ret)
 		return ret;
 
@@ -35,37 +37,40 @@ static int scmi_imx_bbm_read_time(struct device *dev, struct rtc_time *tm)
 
 static int scmi_imx_bbm_set_time(struct device *dev, struct rtc_time *tm)
 {
-	struct scmi_imx_bbm *bbnsm = dev_get_drvdata(dev);
+	struct rtc_device *rtc = to_rtc_device(dev);
+	struct scmi_imx_bbm *bbnsm = rtc->priv;
 	struct scmi_protocol_handle *ph = bbnsm->ph;
 	u64 val;
 
 	val = rtc_tm_to_time64(tm);
 
-	return bbnsm->ops->rtc_time_set(ph, 0, val);
+	return bbnsm->ops->rtc_time_set(ph, bbnsm->bbm_rtc_id, val);
 }
 
 static int scmi_imx_bbm_alarm_irq_enable(struct device *dev, unsigned int enable)
 {
-	struct scmi_imx_bbm *bbnsm = dev_get_drvdata(dev);
+	struct rtc_device *rtc = to_rtc_device(dev);
+	struct scmi_imx_bbm *bbnsm = rtc->priv;
 	struct scmi_protocol_handle *ph = bbnsm->ph;
 
 	/* scmi_imx_bbm_set_alarm enables the irq, just handle disable here */
 	if (!enable)
-		return bbnsm->ops->rtc_alarm_set(ph, 0, false, 0);
+		return bbnsm->ops->rtc_alarm_set(ph, bbnsm->bbm_rtc_id, false, 0);
 
 	return 0;
 }
 
 static int scmi_imx_bbm_set_alarm(struct device *dev, struct rtc_wkalrm *alrm)
 {
-	struct scmi_imx_bbm *bbnsm = dev_get_drvdata(dev);
+	struct rtc_device *rtc = to_rtc_device(dev);
+	struct scmi_imx_bbm *bbnsm = rtc->priv;
 	struct scmi_protocol_handle *ph = bbnsm->ph;
 	struct rtc_time *alrm_tm = &alrm->time;
 	u64 val;
 
 	val = rtc_tm_to_time64(alrm_tm);
 
-	return bbnsm->ops->rtc_alarm_set(ph, 0, true, val);
+	return bbnsm->ops->rtc_alarm_set(ph, bbnsm->bbm_rtc_id, true, val);
 }
 
 static const struct rtc_class_ops smci_imx_bbm_rtc_ops = {
@@ -83,19 +88,18 @@ static int scmi_imx_bbm_rtc_notifier(struct notifier_block *nb, unsigned long ev
 	if (r->is_rtc)
 		rtc_update_irq(bbnsm->rtc_dev, 1, RTC_AF | RTC_IRQF);
 	else
-		pr_err("Unexpected bbm event: %s\n", __func__);
+		pr_err("Unexpected bbm event: %s, bbm_rtc_id: %u\n", __func__, bbnsm->bbm_rtc_id);
 
 	return 0;
 }
 
-static int scmi_imx_bbm_rtc_init(struct scmi_device *sdev)
+static int scmi_imx_bbm_rtc_init(struct scmi_device *sdev, struct scmi_imx_bbm *bbnsm)
 {
 	const struct scmi_handle *handle = sdev->handle;
 	struct device *dev = &sdev->dev;
-	struct scmi_imx_bbm *bbnsm = dev_get_drvdata(dev);
 	int ret;
 
-	bbnsm->rtc_dev = devm_rtc_allocate_device(dev);
+	bbnsm->rtc_dev = devm_rtc_allocate_device_priv(dev, bbnsm);
 	if (IS_ERR(bbnsm->rtc_dev))
 		return PTR_ERR(bbnsm->rtc_dev);
 
@@ -105,7 +109,7 @@ static int scmi_imx_bbm_rtc_init(struct scmi_device *sdev)
 	bbnsm->nb.notifier_call = &scmi_imx_bbm_rtc_notifier;
 	ret = handle->notify_ops->devm_event_notifier_register(sdev, SCMI_PROTOCOL_IMX_BBM,
 							       SCMI_EVENT_IMX_BBM_RTC,
-							       NULL, &bbnsm->nb);
+							       &bbnsm->bbm_rtc_id, &bbnsm->nb);
 	if (ret)
 		return ret;
 
@@ -118,29 +122,42 @@ static int scmi_imx_bbm_rtc_probe(struct scmi_device *sdev)
 	struct device *dev = &sdev->dev;
 	struct scmi_protocol_handle *ph;
 	struct scmi_imx_bbm *bbnsm;
-	int ret;
+	const struct scmi_imx_bbm_proto_ops *ops;
+	int ret, i;
+	u32 nr_rtc;
 
 	if (!handle)
 		return -ENODEV;
 
-	bbnsm = devm_kzalloc(dev, sizeof(*bbnsm), GFP_KERNEL);
-	if (!bbnsm)
-		return -ENOMEM;
+	ops = handle->devm_protocol_get(sdev, SCMI_PROTOCOL_IMX_BBM, &ph);
+	if (IS_ERR(ops))
+		return PTR_ERR(ops);
 
-	bbnsm->ops = handle->devm_protocol_get(sdev, SCMI_PROTOCOL_IMX_BBM, &ph);
-	if (IS_ERR(bbnsm->ops))
-		return PTR_ERR(bbnsm->ops);
-
-	bbnsm->ph = ph;
+	ret = ops->bbm_info(ph, &nr_rtc, NULL);
+	if (ret)
+		return ret;
 
 	device_init_wakeup(dev, true);
 
-	dev_set_drvdata(dev, bbnsm);
+	for (i = 0; i < nr_rtc; i++) {
+		bbnsm = devm_kzalloc(dev, sizeof(*bbnsm), GFP_KERNEL);
+		if (!bbnsm) {
+			ret = -ENOMEM;
+			goto fail;
+		}
 
-	ret = scmi_imx_bbm_rtc_init(sdev);
-	if (ret)
-		device_init_wakeup(dev, false);
+		bbnsm->ops = ops;
+		bbnsm->ph = ph;
+		bbnsm->bbm_rtc_id = i;
 
+		ret = scmi_imx_bbm_rtc_init(sdev, bbnsm);
+		if (ret)
+			goto fail;
+	}
+
+	return 0;
+fail:
+	device_init_wakeup(dev, false);
 	return ret;
 }
 

-- 
2.37.1

Re: [PATCH 0/4] rtc/scmi: Support multiple RTCs

[Alexandre Belloni]

Hello,

On 20/01/2025 10:25:32+0800, Peng Fan (OSS) wrote:
> i.MX95 System Manager(SM) BBM protocol exports two RTCs for EVK board.
> one RTC is SoC internal RTC, the other is board RTC.
> 
> The current driver only use the 1st RTC. With this patchset, both RTCs
> could be used in Linux. To achieve this:
> 
> 1. Support more event sources for BBM protocol
> 2. Add bbm_info hook to let users could query the number of RTCs
> 3. Introduce devm_rtc_allocate_device_priv to support setting rtc device
>    private information
> 4. Update rtc-imx-sm-bbm.c to register both RTCs
> 

I'm sorry but no, you have to register two RTCs like any other system
would do.

> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---
> Peng Fan (4):
>       firmware: arm_scmi: imx: Support more event sources
>       firmware: arm_scmi: imx: Introduce bbm_info hook
>       rtc: Introduce devm_rtc_allocate_device_priv
>       rtc: imx-sm-bbm: Support multiple RTCs
> 
>  drivers/firmware/arm_scmi/vendors/imx/imx-sm-bbm.c | 33 ++++++++++-
>  drivers/rtc/class.c                                |  9 ++-
>  drivers/rtc/dev.c                                  |  8 ++-
>  drivers/rtc/interface.c                            | 16 ++---
>  drivers/rtc/proc.c                                 |  2 +-
>  drivers/rtc/rtc-imx-sm-bbm.c                       | 69 ++++++++++++++--------
>  include/linux/rtc.h                                |  2 +
>  include/linux/scmi_imx_protocol.h                  |  2 +
>  8 files changed, 100 insertions(+), 41 deletions(-)
> ---
> base-commit: e7bb221a638962d487231ac45a6699fb9bb8f9fa
> change-id: 20250116-rtc-3834e01786a8
> 
> Best regards,
> -- 
> Peng Fan <peng.fan@nxp.com>
> 

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

Re: [PATCH 3/4] rtc: Introduce devm_rtc_allocate_device_priv

[Dan Carpenter]

On Mon, Jan 20, 2025 at 10:25:35AM +0800, Peng Fan (OSS) wrote:
>  int __devm_rtc_register_device(struct module *owner, struct rtc_device *rtc)
> diff --git a/drivers/rtc/dev.c b/drivers/rtc/dev.c
> index c4a3ab53dcd4b7280a3a2981fe842729603a1feb..e0e1a488b795645d7c9453490d6cdba510cc5db5 100644
> --- a/drivers/rtc/dev.c
> +++ b/drivers/rtc/dev.c
> @@ -410,7 +410,8 @@ static long rtc_dev_ioctl(struct file *file,
>  		}
>  		default:
>  			if (rtc->ops->param_get)
> -				err = rtc->ops->param_get(rtc->dev.parent, &param);
> +				err = rtc->ops->param_get(rtc->priv ?
> +							  &rtc->dev : rtc->dev.parent, &param);

This seems kind of horrible...  I can't think of anywhere else which
does something like this.

It would almost be better to do something like:

	err = rtc->ops->param_get(rtc->priv ? (void *)rtc : rtc->dev.parent, &param);

The advatange of this is that it looks totally horrible from the get go
instead of only subtly wrong.  And it would immediately crash if you got
it wrong implementing the ->param_get() function pointer.

regards,
dan carpenter

RE: [PATCH 0/4] rtc/scmi: Support multiple RTCs

[Peng Fan]

Hi Alexandre

> Subject: Re: [PATCH 0/4] rtc/scmi: Support multiple RTCs
> 
> Hello,
> 
> On 20/01/2025 10:25:32+0800, Peng Fan (OSS) wrote:
> > i.MX95 System Manager(SM) BBM protocol exports two RTCs for EVK
> board.
> > one RTC is SoC internal RTC, the other is board RTC.
> >
> > The current driver only use the 1st RTC. With this patchset, both RTCs
> > could be used in Linux. To achieve this:
> >
> > 1. Support more event sources for BBM protocol 2. Add bbm_info
> hook to
> > let users could query the number of RTCs 3. Introduce
> > devm_rtc_allocate_device_priv to support setting rtc device
> >    private information
> > 4. Update rtc-imx-sm-bbm.c to register both RTCs
> >
> 
> I'm sorry but no, you have to register two RTCs like any other system
> would do.

It is the i.MX SCMI Protocol exports two RTCs using one protocol.

Two RTC devices are created, but share one parent device.

Do you mean each RTC device should have a unique parent device?

Thanks,
Peng.




> 
> > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> > ---
> > Peng Fan (4):
> >       firmware: arm_scmi: imx: Support more event sources
> >       firmware: arm_scmi: imx: Introduce bbm_info hook
> >       rtc: Introduce devm_rtc_allocate_device_priv
> >       rtc: imx-sm-bbm: Support multiple RTCs
> >
> >  drivers/firmware/arm_scmi/vendors/imx/imx-sm-bbm.c | 33
> ++++++++++-
> >  drivers/rtc/class.c                                |  9 ++-
> >  drivers/rtc/dev.c                                  |  8 ++-
> >  drivers/rtc/interface.c                            | 16 ++---
> >  drivers/rtc/proc.c                                 |  2 +-
> >  drivers/rtc/rtc-imx-sm-bbm.c                       | 69 ++++++++++++++-------
> -
> >  include/linux/rtc.h                                |  2 +
> >  include/linux/scmi_imx_protocol.h                  |  2 +
> >  8 files changed, 100 insertions(+), 41 deletions(-)
> > ---
> > base-commit: e7bb221a638962d487231ac45a6699fb9bb8f9fa
> > change-id: 20250116-rtc-3834e01786a8
> >
> > Best regards,
> > --
> > Peng Fan <peng.fan@nxp.com>
> >
> 
> --
> Alexandre Belloni, co-owner and COO, Bootlin
> Embedded Linux and Kernel engineering
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2F
> bootlin.com%2F&data=05%7C02%7Cpeng.fan%40nxp.com%7C7b5c28
> 0a03ee47dea25f08dd393c2e53%7C686ea1d3bc2b4c6fa92cd99c5c30
> 1635%7C0%7C0%7C638729652821885462%7CUnknown%7CTWFpbG
> Zsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXa
> W4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata
> =xL23vC4m%2BtiTN8eNs8QptUHgfo%2FuHEUcEewGMdeYWYo%3D&r
> eserved=0

RE: [PATCH 3/4] rtc: Introduce devm_rtc_allocate_device_priv

[Peng Fan]

Hi Dan,

> Subject: Re: [PATCH 3/4] rtc: Introduce devm_rtc_allocate_device_priv
> 
> On Mon, Jan 20, 2025 at 10:25:35AM +0800, Peng Fan (OSS) wrote:
> >  int __devm_rtc_register_device(struct module *owner, struct
> > rtc_device *rtc) diff --git a/drivers/rtc/dev.c b/drivers/rtc/dev.c
> > index
> >
> c4a3ab53dcd4b7280a3a2981fe842729603a1feb..e0e1a488b795645d
> 7c9453490d6c
> > dba510cc5db5 100644
> > --- a/drivers/rtc/dev.c
> > +++ b/drivers/rtc/dev.c
> > @@ -410,7 +410,8 @@ static long rtc_dev_ioctl(struct file *file,
> >  		}
> >  		default:
> >  			if (rtc->ops->param_get)
> > -				err = rtc->ops->param_get(rtc-
> >dev.parent, &param);
> > +				err = rtc->ops->param_get(rtc->priv ?
> > +							  &rtc->dev :
> rtc->dev.parent, &param);
> 
> This seems kind of horrible...  I can't think of anywhere else which does
> something like this.
> 
> It would almost be better to do something like:
> 
> 	err = rtc->ops->param_get(rtc->priv ? (void *)rtc : rtc-
> >dev.parent, &param);
> 
> The advatange of this is that it looks totally horrible from the get go
> instead of only subtly wrong.  And it would immediately crash if you
> got it wrong implementing the ->param_get() function pointer.

Thanks for help improving the code. I will include this in V2 and post
out after we reach a goal on how to support the 2nd RTC on i.MX95.

Thanks,
Peng.

> 
> regards,
> dan carpenter

Re: [PATCH 3/4] rtc: Introduce devm_rtc_allocate_device_priv

[Dan Carpenter]

On Tue, Jan 21, 2025 at 02:35:59PM +0000, Peng Fan wrote:
> Hi Dan,
> 
> > Subject: Re: [PATCH 3/4] rtc: Introduce devm_rtc_allocate_device_priv
> > 
> > On Mon, Jan 20, 2025 at 10:25:35AM +0800, Peng Fan (OSS) wrote:
> > >  int __devm_rtc_register_device(struct module *owner, struct
> > > rtc_device *rtc) diff --git a/drivers/rtc/dev.c b/drivers/rtc/dev.c
> > > index
> > >
> > c4a3ab53dcd4b7280a3a2981fe842729603a1feb..e0e1a488b795645d
> > 7c9453490d6c
> > > dba510cc5db5 100644
> > > --- a/drivers/rtc/dev.c
> > > +++ b/drivers/rtc/dev.c
> > > @@ -410,7 +410,8 @@ static long rtc_dev_ioctl(struct file *file,
> > >  		}
> > >  		default:
> > >  			if (rtc->ops->param_get)
> > > -				err = rtc->ops->param_get(rtc-
> > >dev.parent, &param);
> > > +				err = rtc->ops->param_get(rtc->priv ?
> > > +							  &rtc->dev :
> > rtc->dev.parent, &param);
> > 
> > This seems kind of horrible...  I can't think of anywhere else which does
> > something like this.
> > 
> > It would almost be better to do something like:
> > 
> > 	err = rtc->ops->param_get(rtc->priv ? (void *)rtc : rtc-
> > >dev.parent, &param);
> > 
> > The advatange of this is that it looks totally horrible from the get go
> > instead of only subtly wrong.  And it would immediately crash if you
> > got it wrong implementing the ->param_get() function pointer.
> 
> Thanks for help improving the code. I will include this in V2 and post
> out after we reach a goal on how to support the 2nd RTC on i.MX95.

Don't do what I said actually...  Let's find a better way.  I don't
know why rtc_class_ops function pointers take a device pointer instead
of an rtc_device pointer.  Or if they did take a device pointer why
not the &rtc->dev like you suggested?  But let's not do both like this.

Migrating all the function pointers is a lot of work but not impossible.

regards,
dan carpenter

Re: [PATCH 0/4] rtc/scmi: Support multiple RTCs

[Peng Fan]

Hi Alexandre,

On Tue, Jan 21, 2025 at 02:31:55PM +0000, Peng Fan wrote:
>Hi Alexandre
>
>> Subject: Re: [PATCH 0/4] rtc/scmi: Support multiple RTCs
>> 
>> Hello,
>> 
>> On 20/01/2025 10:25:32+0800, Peng Fan (OSS) wrote:
>> > i.MX95 System Manager(SM) BBM protocol exports two RTCs for EVK
>> board.
>> > one RTC is SoC internal RTC, the other is board RTC.
>> >
>> > The current driver only use the 1st RTC. With this patchset, both RTCs
>> > could be used in Linux. To achieve this:
>> >
>> > 1. Support more event sources for BBM protocol 2. Add bbm_info
>> hook to
>> > let users could query the number of RTCs 3. Introduce
>> > devm_rtc_allocate_device_priv to support setting rtc device
>> >    private information
>> > 4. Update rtc-imx-sm-bbm.c to register both RTCs
>> >
>> 
>> I'm sorry but no, you have to register two RTCs like any other system
>> would do.
>
>It is the i.MX SCMI Protocol exports two RTCs using one protocol.
>
>Two RTC devices are created, but share one parent device.
>
>Do you mean each RTC device should have a unique parent device?

Could you please share your ideas on this?

Thanks,
Peng

>
>Thanks,
>Peng.
>
>
>
>
>> 
>> > Signed-off-by: Peng Fan <peng.fan@nxp.com>
>> > ---
>> > Peng Fan (4):
>> >       firmware: arm_scmi: imx: Support more event sources
>> >       firmware: arm_scmi: imx: Introduce bbm_info hook
>> >       rtc: Introduce devm_rtc_allocate_device_priv
>> >       rtc: imx-sm-bbm: Support multiple RTCs
>> >
>> >  drivers/firmware/arm_scmi/vendors/imx/imx-sm-bbm.c | 33
>> ++++++++++-
>> >  drivers/rtc/class.c                                |  9 ++-
>> >  drivers/rtc/dev.c                                  |  8 ++-
>> >  drivers/rtc/interface.c                            | 16 ++---
>> >  drivers/rtc/proc.c                                 |  2 +-
>> >  drivers/rtc/rtc-imx-sm-bbm.c                       | 69 ++++++++++++++-------
>> -
>> >  include/linux/rtc.h                                |  2 +
>> >  include/linux/scmi_imx_protocol.h                  |  2 +
>> >  8 files changed, 100 insertions(+), 41 deletions(-)
>> > ---
>> > base-commit: e7bb221a638962d487231ac45a6699fb9bb8f9fa
>> > change-id: 20250116-rtc-3834e01786a8
>> >
>> > Best regards,
>> > --
>> > Peng Fan <peng.fan@nxp.com>
>> >
>> 
>> --
>> Alexandre Belloni, co-owner and COO, Bootlin
>> Embedded Linux and Kernel engineering
>> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2F
>> bootlin.com%2F&data=05%7C02%7Cpeng.fan%40nxp.com%7C7b5c28
>> 0a03ee47dea25f08dd393c2e53%7C686ea1d3bc2b4c6fa92cd99c5c30
>> 1635%7C0%7C0%7C638729652821885462%7CUnknown%7CTWFpbG
>> Zsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXa
>> W4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata
>> =xL23vC4m%2BtiTN8eNs8QptUHgfo%2FuHEUcEewGMdeYWYo%3D&r
>> eserved=0

Re: [PATCH 0/4] rtc/scmi: Support multiple RTCs

[Sudeep Holla]

On Tue, Jan 21, 2025 at 02:31:55PM +0000, Peng Fan wrote:
> 
> It is the i.MX SCMI Protocol exports two RTCs using one protocol.
> 
> Two RTC devices are created, but share one parent device.
> 
> Do you mean each RTC device should have a unique parent device?
>

Can you point where is this check for unique parent ? I am not so familiar
with RTC but I couldn't find myself with quick search.

-- 
Regards,
Sudeep

Re: [PATCH 4/4] rtc: imx-sm-bbm: Support multiple RTCs

[Sudeep Holla]

On Mon, Jan 20, 2025 at 10:25:36AM +0800, Peng Fan (OSS) wrote:
> From: Peng Fan <peng.fan@nxp.com>
> 
> i.MX95 EVK has two RTCs exported by SCMI BBM protocol. Current driver
> only enables the 1st RTC inside BBNSM module, leaving the board RTC
> not used by Linux.
> 
> To use the 2nd RTC, use 'bbm_info' to get the number of RTCs, register
> them all, and set 'bbnsm' as private info for rtc device to know which
> RTC it is when using rtc_class_ops to access rtc device.
> 
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---
>  drivers/rtc/rtc-imx-sm-bbm.c | 69 +++++++++++++++++++++++++++-----------------
>  1 file changed, 43 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/rtc/rtc-imx-sm-bbm.c b/drivers/rtc/rtc-imx-sm-bbm.c
> index daa472be7c80697aa3cd3432eccef0c877e4c378..a29b30555d0c0581ecaa8b79760209dc780d2f0e 100644
> --- a/drivers/rtc/rtc-imx-sm-bbm.c
> +++ b/drivers/rtc/rtc-imx-sm-bbm.c
> @@ -15,16 +15,18 @@ struct scmi_imx_bbm {
>  	struct rtc_device *rtc_dev;
>  	struct scmi_protocol_handle *ph;
>  	struct notifier_block nb;
> +	u32 bbm_rtc_id;

Is it not same as rtc_dev->id ? Why do you need a copy in this wrapper/
container structure ?

-- 
Regards,
Sudeep

Re: [PATCH 0/4] rtc/scmi: Support multiple RTCs

[Peng Fan]

On Tue, Feb 11, 2025 at 04:59:53PM +0000, Sudeep Holla wrote:
>On Tue, Jan 21, 2025 at 02:31:55PM +0000, Peng Fan wrote:
>> 
>> It is the i.MX SCMI Protocol exports two RTCs using one protocol.
>> 
>> Two RTC devices are created, but share one parent device.
>> 
>> Do you mean each RTC device should have a unique parent device?
>>
>
>Can you point where is this check for unique parent ? I am not so familiar
>with RTC but I couldn't find myself with quick search.

The RTC ops takes the rtc parent as input parameter
https://elixir.bootlin.com/linux/v6.13.2/source/drivers/rtc/interface.c#L94
"err = rtc->ops->read_time(rtc->dev.parent, tm);"

So in the rtc device driver, there is no way to know which rtc it is just
from the parent device.

However i.MX SCMI BBM exports two RTCs(id: 0, id: 1), so to make it work for
current RTC framework, we could only pick one RTC and pass the id to BBM
server side.

I am not sure whether Alexandre wanna me to update the code following each
parent could only support one RTC or else.

Regards
Peng

>
>-- 
>Regards,
>Sudeep

Re: [PATCH 4/4] rtc: imx-sm-bbm: Support multiple RTCs

[Peng Fan]

On Tue, Feb 11, 2025 at 05:01:12PM +0000, Sudeep Holla wrote:
>On Mon, Jan 20, 2025 at 10:25:36AM +0800, Peng Fan (OSS) wrote:
>> From: Peng Fan <peng.fan@nxp.com>
>> 
>> i.MX95 EVK has two RTCs exported by SCMI BBM protocol. Current driver
>> only enables the 1st RTC inside BBNSM module, leaving the board RTC
>> not used by Linux.
>> 
>> To use the 2nd RTC, use 'bbm_info' to get the number of RTCs, register
>> them all, and set 'bbnsm' as private info for rtc device to know which
>> RTC it is when using rtc_class_ops to access rtc device.
>> 
>> Signed-off-by: Peng Fan <peng.fan@nxp.com>
>> ---
>>  drivers/rtc/rtc-imx-sm-bbm.c | 69 +++++++++++++++++++++++++++-----------------
>>  1 file changed, 43 insertions(+), 26 deletions(-)
>> 
>> diff --git a/drivers/rtc/rtc-imx-sm-bbm.c b/drivers/rtc/rtc-imx-sm-bbm.c
>> index daa472be7c80697aa3cd3432eccef0c877e4c378..a29b30555d0c0581ecaa8b79760209dc780d2f0e 100644
>> --- a/drivers/rtc/rtc-imx-sm-bbm.c
>> +++ b/drivers/rtc/rtc-imx-sm-bbm.c
>> @@ -15,16 +15,18 @@ struct scmi_imx_bbm {
>>  	struct rtc_device *rtc_dev;
>>  	struct scmi_protocol_handle *ph;
>>  	struct notifier_block nb;
>> +	u32 bbm_rtc_id;
>
>Is it not same as rtc_dev->id ? Why do you need a copy in this wrapper/
>container structure ?

In theroy yes. The current system I use that all RTCs are managed by BBM
protocol. So only two RTCs are registered.

In case there is other RTCs that not managed BBM, the rtc_dev->id
will not be equal to bbm_rtc_id.

For example RTC1 is directly managed by Linux, RTC0 is managed by BBM.

The RTC1 is probed first, so its rtc_dev->id is 0. But from BBM protocol,
the RTC0 use id 0 for BBM SCMI server to handle the RTC0.

I maybe overthinking here. But to avoid potential issues, I would like to
keep bbm_rtc_id.

Regards,
Peng

>
>-- 
>Regards,
>Sudeep

Re: [PATCH 0/4] rtc/scmi: Support multiple RTCs

[Sudeep Holla]

On Wed, Feb 12, 2025 at 02:35:32PM +0800, Peng Fan wrote:
> On Tue, Feb 11, 2025 at 04:59:53PM +0000, Sudeep Holla wrote:
> >On Tue, Jan 21, 2025 at 02:31:55PM +0000, Peng Fan wrote:
> >> 
> >> It is the i.MX SCMI Protocol exports two RTCs using one protocol.
> >> 
> >> Two RTC devices are created, but share one parent device.
> >> 
> >> Do you mean each RTC device should have a unique parent device?
> >>
> >
> >Can you point where is this check for unique parent ? I am not so familiar
> >with RTC but I couldn't find myself with quick search.
> 
> The RTC ops takes the rtc parent as input parameter
> https://elixir.bootlin.com/linux/v6.13.2/source/drivers/rtc/interface.c#L94
> "err = rtc->ops->read_time(rtc->dev.parent, tm);"
> 
> So in the rtc device driver, there is no way to know which rtc it is just
> from the parent device.
>

If that is the expectation, you could create a platform or normal device
per instance of RTC on your platform and slap them as parent device.

IIUC on any pure DT based system, a device node exists per RTC and hence
platform device associated with it. And the RTC devices are created with
parent pointing to unique platform device.

> However i.MX SCMI BBM exports two RTCs(id: 0, id: 1), so to make it work for
> current RTC framework, we could only pick one RTC and pass the id to BBM
> server side.
>
> I am not sure whether Alexandre wanna me to update the code following each
> parent could only support one RTC or else.
>

I assume something like my suggestion above.

--
Regards,
Sudeep

Re: [PATCH 4/4] rtc: imx-sm-bbm: Support multiple RTCs

[Sudeep Holla]

On Wed, Feb 12, 2025 at 02:41:17PM +0800, Peng Fan wrote:
> On Tue, Feb 11, 2025 at 05:01:12PM +0000, Sudeep Holla wrote:
> >On Mon, Jan 20, 2025 at 10:25:36AM +0800, Peng Fan (OSS) wrote:
> >> From: Peng Fan <peng.fan@nxp.com>
> >> 
> >> i.MX95 EVK has two RTCs exported by SCMI BBM protocol. Current driver
> >> only enables the 1st RTC inside BBNSM module, leaving the board RTC
> >> not used by Linux.
> >> 
> >> To use the 2nd RTC, use 'bbm_info' to get the number of RTCs, register
> >> them all, and set 'bbnsm' as private info for rtc device to know which
> >> RTC it is when using rtc_class_ops to access rtc device.
> >> 
> >> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> >> ---
> >>  drivers/rtc/rtc-imx-sm-bbm.c | 69 +++++++++++++++++++++++++++-----------------
> >>  1 file changed, 43 insertions(+), 26 deletions(-)
> >> 
> >> diff --git a/drivers/rtc/rtc-imx-sm-bbm.c b/drivers/rtc/rtc-imx-sm-bbm.c
> >> index daa472be7c80697aa3cd3432eccef0c877e4c378..a29b30555d0c0581ecaa8b79760209dc780d2f0e 100644
> >> --- a/drivers/rtc/rtc-imx-sm-bbm.c
> >> +++ b/drivers/rtc/rtc-imx-sm-bbm.c
> >> @@ -15,16 +15,18 @@ struct scmi_imx_bbm {
> >>  	struct rtc_device *rtc_dev;
> >>  	struct scmi_protocol_handle *ph;
> >>  	struct notifier_block nb;
> >> +	u32 bbm_rtc_id;
> >
> >Is it not same as rtc_dev->id ? Why do you need a copy in this wrapper/
> >container structure ?
> 
> In theroy yes. The current system I use that all RTCs are managed by BBM
> protocol. So only two RTCs are registered.
> 
> In case there is other RTCs that not managed BBM, the rtc_dev->id
> will not be equal to bbm_rtc_id.
> 
> For example RTC1 is directly managed by Linux, RTC0 is managed by BBM.
> 
> The RTC1 is probed first, so its rtc_dev->id is 0. But from BBM protocol,
> the RTC0 use id 0 for BBM SCMI server to handle the RTC0.
> 
> I maybe overthinking here. But to avoid potential issues, I would like to
> keep bbm_rtc_id.
> 

Fair enough, I didn't think of this mix(firmware controlled RTC + Linux
controlled ones).

--
Regards,
Sudeep

Re: [PATCH 0/4] rtc/scmi: Support multiple RTCs

[Alexandre Belloni]

On 12/02/2025 10:43:24+0000, Sudeep Holla wrote:
> On Wed, Feb 12, 2025 at 02:35:32PM +0800, Peng Fan wrote:
> > On Tue, Feb 11, 2025 at 04:59:53PM +0000, Sudeep Holla wrote:
> > >On Tue, Jan 21, 2025 at 02:31:55PM +0000, Peng Fan wrote:
> > >> 
> > >> It is the i.MX SCMI Protocol exports two RTCs using one protocol.
> > >> 
> > >> Two RTC devices are created, but share one parent device.
> > >> 
> > >> Do you mean each RTC device should have a unique parent device?
> > >>
> > >
> > >Can you point where is this check for unique parent ? I am not so familiar
> > >with RTC but I couldn't find myself with quick search.
> > 
> > The RTC ops takes the rtc parent as input parameter
> > https://elixir.bootlin.com/linux/v6.13.2/source/drivers/rtc/interface.c#L94
> > "err = rtc->ops->read_time(rtc->dev.parent, tm);"
> > 
> > So in the rtc device driver, there is no way to know which rtc it is just
> > from the parent device.
> >
> 
> If that is the expectation, you could create a platform or normal device
> per instance of RTC on your platform and slap them as parent device.

This would seem like the proper solution, why not using an MFD or
auxiliary bus ?

> 
> IIUC on any pure DT based system, a device node exists per RTC and hence
> platform device associated with it. And the RTC devices are created with
> parent pointing to unique platform device.
> 
> > However i.MX SCMI BBM exports two RTCs(id: 0, id: 1), so to make it work for
> > current RTC framework, we could only pick one RTC and pass the id to BBM
> > server side.
> >
> > I am not sure whether Alexandre wanna me to update the code following each
> > parent could only support one RTC or else.
> >

I want you to keep your changes local to your driver. I already stated
back in 2018 that you were on your own with the imx-sc driver and that I
don't like seeing multiple abstractions for existing RTCs. What is the
actual use case behind needing to access both RTCs using Linux?
Shouldn't this be handled on your firmware side?


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

Re: [PATCH 0/4] rtc/scmi: Support multiple RTCs

[Peng Fan]

On Wed, Feb 12, 2025 at 06:01:47PM +0100, Alexandre Belloni wrote:
>On 12/02/2025 10:43:24+0000, Sudeep Holla wrote:
>> On Wed, Feb 12, 2025 at 02:35:32PM +0800, Peng Fan wrote:
>> > On Tue, Feb 11, 2025 at 04:59:53PM +0000, Sudeep Holla wrote:
>> > >On Tue, Jan 21, 2025 at 02:31:55PM +0000, Peng Fan wrote:
>> > >> 
>> > >> It is the i.MX SCMI Protocol exports two RTCs using one protocol.
>> > >> 
>> > >> Two RTC devices are created, but share one parent device.
>> > >> 
>> > >> Do you mean each RTC device should have a unique parent device?
>> > >>
>> > >
>> > >Can you point where is this check for unique parent ? I am not so familiar
>> > >with RTC but I couldn't find myself with quick search.
>> > 
>> > The RTC ops takes the rtc parent as input parameter
>> > https://elixir.bootlin.com/linux/v6.13.2/source/drivers/rtc/interface.c#L94
>> > "err = rtc->ops->read_time(rtc->dev.parent, tm);"
>> > 
>> > So in the rtc device driver, there is no way to know which rtc it is just
>> > from the parent device.
>> >
>> 
>> If that is the expectation, you could create a platform or normal device
>> per instance of RTC on your platform and slap them as parent device.

thanks for the proposal, Sudeep.

>
>This would seem like the proper solution, why not using an MFD or
>auxiliary bus ?

Not sure which is better. I need give a look which is simpiler
for current BBM RTC.

>
>> 
>> IIUC on any pure DT based system, a device node exists per RTC and hence
>> platform device associated with it. And the RTC devices are created with
>> parent pointing to unique platform device.
>> 
>> > However i.MX SCMI BBM exports two RTCs(id: 0, id: 1), so to make it work for
>> > current RTC framework, we could only pick one RTC and pass the id to BBM
>> > server side.
>> >
>> > I am not sure whether Alexandre wanna me to update the code following each
>> > parent could only support one RTC or else.
>> >
>
>I want you to keep your changes local to your driver. I already stated
>back in 2018 that you were on your own with the imx-sc driver and that I
>don't like seeing multiple abstractions for existing RTCs. What is the
>actual use case behind needing to access both RTCs using Linux?
>Shouldn't this be handled on your firmware side?

The firmware exports two RTCs, RTC0 could be handled by Linux, RTC1
could only be read by Linux and configuable by M7 per current i.MX95 EVK
firmware.

The firmware itself does not serve Linux. And we not know others
how to use the protocol. So the firmware will not be updated for only
Linux.

I got your point to keep changes local to the rtc bbm driver, I will
give a look on how to achieve this.

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

Re: [PATCH 0/4] rtc/scmi: Support multiple RTCs

[Alexandre Belloni]

On 13/02/2025 11:30:33+0800, Peng Fan wrote:
> >> IIUC on any pure DT based system, a device node exists per RTC and hence
> >> platform device associated with it. And the RTC devices are created with
> >> parent pointing to unique platform device.
> >> 
> >> > However i.MX SCMI BBM exports two RTCs(id: 0, id: 1), so to make it work for
> >> > current RTC framework, we could only pick one RTC and pass the id to BBM
> >> > server side.
> >> >
> >> > I am not sure whether Alexandre wanna me to update the code following each
> >> > parent could only support one RTC or else.
> >> >
> >
> >I want you to keep your changes local to your driver. I already stated
> >back in 2018 that you were on your own with the imx-sc driver and that I
> >don't like seeing multiple abstractions for existing RTCs. What is the
> >actual use case behind needing to access both RTCs using Linux?
> >Shouldn't this be handled on your firmware side?
> 
> The firmware exports two RTCs, RTC0 could be handled by Linux, RTC1
> could only be read by Linux and configuable by M7 per current i.MX95 EVK
> firmware.

This doesn't answer the main question, why is this useful? Where is the
time of RTC1 coming from and why would linux set a different time on
RTC0 ? Can't the firwmare just set the same time on both RTC0 and RTC1?
What would someone do if RTC0 and RTC1 don't agree on the time?

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

Re: [PATCH 0/4] rtc/scmi: Support multiple RTCs

[Peng Fan]

On Thu, Feb 13, 2025 at 09:20:32AM +0100, Alexandre Belloni wrote:
>On 13/02/2025 11:30:33+0800, Peng Fan wrote:
>> >> IIUC on any pure DT based system, a device node exists per RTC and hence
>> >> platform device associated with it. And the RTC devices are created with
>> >> parent pointing to unique platform device.
>> >> 
>> >> > However i.MX SCMI BBM exports two RTCs(id: 0, id: 1), so to make it work for
>> >> > current RTC framework, we could only pick one RTC and pass the id to BBM
>> >> > server side.
>> >> >
>> >> > I am not sure whether Alexandre wanna me to update the code following each
>> >> > parent could only support one RTC or else.
>> >> >
>> >
>> >I want you to keep your changes local to your driver. I already stated
>> >back in 2018 that you were on your own with the imx-sc driver and that I
>> >don't like seeing multiple abstractions for existing RTCs. What is the
>> >actual use case behind needing to access both RTCs using Linux?
>> >Shouldn't this be handled on your firmware side?
>> 
>> The firmware exports two RTCs, RTC0 could be handled by Linux, RTC1
>> could only be read by Linux and configuable by M7 per current i.MX95 EVK
>> firmware.
>
>This doesn't answer the main question, why is this useful? Where is the
>time of RTC1 coming from and why would linux set a different time on
>RTC0 ? Can't the firwmare just set the same time on both RTC0 and RTC1?

To current i.MX95 EVK SCMI firmware, RTC0 is SoC internal RTC, RTC1 is
board level RTC which is more acurrate.

There are safety island in i.MX95, M7 safety core is assigned owner of
RTC1. Linux non-safety is assigned owner of RTC0, but Linux could read RTC1
time, Linux not able to set alarm of RTC1.

I need ask firmware developer to see whether RTC1 time could be synced to
RTC0 from firmware level. But considering RTC1 is more accurate, should we
use RTC1?

The current firmware design is RTC0 is always there and exported, because
it is SoC internal RTC. RTC1 is board level one, it could be optional per
board design and firmware design.

The firmware could update to only export RTC1 if no safety need it,
but this needs big change to the firmware BBM part, I need check with
firmware developer. But things may not change.

>What would someone do if RTC0 and RTC1 don't agree on the time?

RTC1 is more accurate if it is there.

Thanks,
Peng

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

Re: [PATCH 0/4] rtc/scmi: Support multiple RTCs

[Alexandre Belloni]

On 13/02/2025 18:52:57+0800, Peng Fan wrote:
> On Thu, Feb 13, 2025 at 09:20:32AM +0100, Alexandre Belloni wrote:
> >On 13/02/2025 11:30:33+0800, Peng Fan wrote:
> >> >> IIUC on any pure DT based system, a device node exists per RTC and hence
> >> >> platform device associated with it. And the RTC devices are created with
> >> >> parent pointing to unique platform device.
> >> >> 
> >> >> > However i.MX SCMI BBM exports two RTCs(id: 0, id: 1), so to make it work for
> >> >> > current RTC framework, we could only pick one RTC and pass the id to BBM
> >> >> > server side.
> >> >> >
> >> >> > I am not sure whether Alexandre wanna me to update the code following each
> >> >> > parent could only support one RTC or else.
> >> >> >
> >> >
> >> >I want you to keep your changes local to your driver. I already stated
> >> >back in 2018 that you were on your own with the imx-sc driver and that I
> >> >don't like seeing multiple abstractions for existing RTCs. What is the
> >> >actual use case behind needing to access both RTCs using Linux?
> >> >Shouldn't this be handled on your firmware side?
> >> 
> >> The firmware exports two RTCs, RTC0 could be handled by Linux, RTC1
> >> could only be read by Linux and configuable by M7 per current i.MX95 EVK
> >> firmware.
> >
> >This doesn't answer the main question, why is this useful? Where is the
> >time of RTC1 coming from and why would linux set a different time on
> >RTC0 ? Can't the firwmare just set the same time on both RTC0 and RTC1?
> 
> To current i.MX95 EVK SCMI firmware, RTC0 is SoC internal RTC, RTC1 is
> board level RTC which is more acurrate.
> 
> There are safety island in i.MX95, M7 safety core is assigned owner of
> RTC1. Linux non-safety is assigned owner of RTC0, but Linux could read RTC1
> time, Linux not able to set alarm of RTC1.
> 
> I need ask firmware developer to see whether RTC1 time could be synced to
> RTC0 from firmware level. But considering RTC1 is more accurate, should we
> use RTC1?
> 
> The current firmware design is RTC0 is always there and exported, because
> it is SoC internal RTC. RTC1 is board level one, it could be optional per
> board design and firmware design.
> 
> The firmware could update to only export RTC1 if no safety need it,
> but this needs big change to the firmware BBM part, I need check with
> firmware developer. But things may not change.
> 
> >What would someone do if RTC0 and RTC1 don't agree on the time?
> 
> RTC1 is more accurate if it is there.
> 

Well, yes, you have your answer here, if the firmware knows RTC1 is more
accurate and will be your source of truth, then simply use this one.


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

Re: [PATCH 0/4] rtc/scmi: Support multiple RTCs

[Alexandre Belloni]

On 13/02/2025 21:35:50+0800, Peng Fan wrote:
> >Well, yes, you have your answer here, if the firmware knows RTC1 is more
> >accurate and will be your source of truth, then simply use this one.
> 
> But issue is RTC1 is only readable to Linux non-safety, Linux not able
> to set alarm. Linux could only use RTC0 for alarm with current i.MX95 EVK
> firmware.
> 
> If RTC1 could be exported to linux for control, we could for sure
> switch to using RTC1 without caring about RTC0. But this is not true.
> 
> RTC0 is free for linux to control, RTC1 not. Switching to RTC1 will make
> us lose RTC alarm to wake up linux feature.
> 

This doesn't make any sense, this limitation is on your firmware side,
either RTC1 has alarm support and the firmware can set the alarm for
linux or it doesn't and then, the firmware must set the time and alarm
on RTC0.

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

Re: [PATCH 0/4] rtc/scmi: Support multiple RTCs

[Peng Fan]

On Thu, Feb 13, 2025 at 12:26:05PM +0100, Alexandre Belloni wrote:
>On 13/02/2025 18:52:57+0800, Peng Fan wrote:
>> On Thu, Feb 13, 2025 at 09:20:32AM +0100, Alexandre Belloni wrote:
>> >On 13/02/2025 11:30:33+0800, Peng Fan wrote:
>> >> >> IIUC on any pure DT based system, a device node exists per RTC and hence
>> >> >> platform device associated with it. And the RTC devices are created with
>> >> >> parent pointing to unique platform device.
>> >> >> 
>> >> >> > However i.MX SCMI BBM exports two RTCs(id: 0, id: 1), so to make it work for
>> >> >> > current RTC framework, we could only pick one RTC and pass the id to BBM
>> >> >> > server side.
>> >> >> >
>> >> >> > I am not sure whether Alexandre wanna me to update the code following each
>> >> >> > parent could only support one RTC or else.
>> >> >> >
>> >> >
>> >> >I want you to keep your changes local to your driver. I already stated
>> >> >back in 2018 that you were on your own with the imx-sc driver and that I
>> >> >don't like seeing multiple abstractions for existing RTCs. What is the
>> >> >actual use case behind needing to access both RTCs using Linux?
>> >> >Shouldn't this be handled on your firmware side?
>> >> 
>> >> The firmware exports two RTCs, RTC0 could be handled by Linux, RTC1
>> >> could only be read by Linux and configuable by M7 per current i.MX95 EVK
>> >> firmware.
>> >
>> >This doesn't answer the main question, why is this useful? Where is the
>> >time of RTC1 coming from and why would linux set a different time on
>> >RTC0 ? Can't the firwmare just set the same time on both RTC0 and RTC1?
>> 
>> To current i.MX95 EVK SCMI firmware, RTC0 is SoC internal RTC, RTC1 is
>> board level RTC which is more acurrate.
>> 
>> There are safety island in i.MX95, M7 safety core is assigned owner of
>> RTC1. Linux non-safety is assigned owner of RTC0, but Linux could read RTC1
>> time, Linux not able to set alarm of RTC1.
>> 
>> I need ask firmware developer to see whether RTC1 time could be synced to
>> RTC0 from firmware level. But considering RTC1 is more accurate, should we
>> use RTC1?
>> 
>> The current firmware design is RTC0 is always there and exported, because
>> it is SoC internal RTC. RTC1 is board level one, it could be optional per
>> board design and firmware design.
>> 
>> The firmware could update to only export RTC1 if no safety need it,
>> but this needs big change to the firmware BBM part, I need check with
>> firmware developer. But things may not change.
>> 
>> >What would someone do if RTC0 and RTC1 don't agree on the time?
>> 
>> RTC1 is more accurate if it is there.
>> 
>
>Well, yes, you have your answer here, if the firmware knows RTC1 is more
>accurate and will be your source of truth, then simply use this one.

But issue is RTC1 is only readable to Linux non-safety, Linux not able
to set alarm. Linux could only use RTC0 for alarm with current i.MX95 EVK
firmware.

If RTC1 could be exported to linux for control, we could for sure
switch to using RTC1 without caring about RTC0. But this is not true.

RTC0 is free for linux to control, RTC1 not. Switching to RTC1 will make
us lose RTC alarm to wake up linux feature.

Thanks,
Peng

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

Re: [PATCH 0/4] rtc/scmi: Support multiple RTCs

[Peng Fan]

On Thu, Feb 13, 2025 at 01:54:42PM +0100, Alexandre Belloni wrote:
>On 13/02/2025 21:35:50+0800, Peng Fan wrote:
>> >Well, yes, you have your answer here, if the firmware knows RTC1 is more
>> >accurate and will be your source of truth, then simply use this one.
>> 
>> But issue is RTC1 is only readable to Linux non-safety, Linux not able
>> to set alarm. Linux could only use RTC0 for alarm with current i.MX95 EVK
>> firmware.
>> 
>> If RTC1 could be exported to linux for control, we could for sure
>> switch to using RTC1 without caring about RTC0. But this is not true.
>> 
>> RTC0 is free for linux to control, RTC1 not. Switching to RTC1 will make
>> us lose RTC alarm to wake up linux feature.
>> 
>
>This doesn't make any sense, this limitation is on your firmware side,
>either RTC1 has alarm support and the firmware can set the alarm for
>linux or it doesn't and then, the firmware must set the time and alarm
>on RTC0.

ok. I thought to export both RTCs to Linux if both are configured for linux
usage. From the discussion, I suppose you prefer linux only use one RTC.

Let me try to update the rtc bbm driver to use RTC1 when RTC1 is exposed
to Linux for usage. If RTC1 is not exposed for Linux, use RTC0.

Regards
Peng.

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