> Subject: Re: [PATCH v5 6/7] rtc: support i.MX95 BBM RTC
>
> Hello,
>
> On 21/06/2024 15:04:41+0800, Peng Fan (OSS) wrote:
> > + ret = bbnsm->ops->rtc_time_get(ph, 0, &val);
> > + if (ret) {
> > + dev_err(dev, "%s: %d\n", __func__, ret);
>
> This is not super useful, you should drop the various dev_err or pr_err
> as there is no action the user can take to solve the erro apart from
> retrying.
Sure. I will remove the various dev_err or pr_err in v6.
>
> > + return ret;
> > + }
> > +
> > + rtc_time64_to_tm(val, tm);
> > +
> > + return 0;
> > +}
> > +
> > +static int scmi_imx_bbm_set_time(struct device *dev, struct
> rtc_time
> > +*tm) {
> > + struct scmi_imx_bbm *bbnsm = dev_get_drvdata(dev);
> > + struct scmi_protocol_handle *ph = bbnsm->ph;
> > + u64 val;
> > + int ret;
> > +
> > + val = rtc_tm_to_time64(tm);
> > +
> > + ret = bbnsm->ops->rtc_time_set(ph, 0, val);
> > + if (ret)
> > + dev_err(dev, "%s: %d\n", __func__, ret);
> > +
> > + return ret;
> > +}
> > +
> > +static int scmi_imx_bbm_alarm_irq_enable(struct device *dev,
> unsigned
> > +int enable) {
>
> How can userspace disable the alarm?
The SCMI notify has kernel level enable/disable
drivers/firmware/arm_scmi/notify.c
But indeed, userspace not able to disable the alarm.
I need use
if (!enable) return -EOPNOTSUPP;
>
> > + 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 scmi_protocol_handle *ph = bbnsm->ph;
> > + struct rtc_time *alrm_tm = &alrm->time;
> > + u64 val;
> > + int ret;
> > +
> > + val = rtc_tm_to_time64(alrm_tm);
> > +
> > + ret = bbnsm->ops->rtc_alarm_set(ph, 0, val);
> > + if (ret)
> > + dev_err(dev, "%s: %d\n", __func__, ret);
> > +
> > + return ret;
> > +}
> > +
> > +static const struct rtc_class_ops smci_imx_bbm_rtc_ops = {
> > + .read_time = scmi_imx_bbm_read_time,
> > + .set_time = scmi_imx_bbm_set_time,
> > + .set_alarm = scmi_imx_bbm_set_alarm,
> > + .alarm_irq_enable = scmi_imx_bbm_alarm_irq_enable, };
> > +
> > +static int scmi_imx_bbm_rtc_notifier(struct notifier_block *nb,
> > +unsigned long event, void *data) {
> > + struct scmi_imx_bbm *bbnsm = container_of(nb, struct
> scmi_imx_bbm, nb);
> > + struct scmi_imx_bbm_notif_report *r = data;
> > +
> > + if (r->is_rtc)
> > + rtc_update_irq(bbnsm->rtc_dev, 1, RTC_AF |
> RTC_IRQF);
> > + else
> > + pr_err("Unexpected bbm event: %s\n", __func__);
> > +
> > + return 0;
> > +}
> > +
> > +static int scmi_imx_bbm_rtc_init(struct scmi_device *sdev) {
> > + 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);
> > + if (IS_ERR(bbnsm->rtc_dev))
> > + return PTR_ERR(bbnsm->rtc_dev);
> > +
> > + bbnsm->rtc_dev->ops = &smci_imx_bbm_rtc_ops;
> > + bbnsm->rtc_dev->range_min = 0;
>
> range_min is set to 0 by default, this is not necessary
>
> > + bbnsm->rtc_dev->range_max = U32_MAX;
> > +
> > + ret = devm_rtc_register_device(bbnsm->rtc_dev);
> > + if (ret)
> > + return ret;
> > +
> > + bbnsm->nb.notifier_call = &scmi_imx_bbm_rtc_notifier;
> > + return handle->notify_ops->devm_event_notifier_register(sdev,
> SCMI_PROTOCOL_IMX_BBM,
> > +
> SCMI_EVENT_IMX_BBM_RTC,
> > +
> NULL, &bbnsm->nb);
>
> Note that failing after devm_rtc_register_device opens the driver to a
> race condition as the character device will exist at that time.
Could you please share more info on this?
Won't the devm_rtc_register_device could automatically remove
the rtc is notifier register fails?
Thanks,
Peng.
>
>
> --
> 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%7C91ef3c1
> 72dc04c6ed2b908dca1e8befb%7C686ea1d3bc2b4c6fa92cd99c5c3016
> 35%7C0%7C0%7C638563268206282814%7CUnknown%7CTWFpbGZs
> b3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXV
> CI6Mn0%3D%7C0%7C%7C%7C&sdata=x8j754poti%2Bo07tbvO3p7XM
> QB5jrbkBPWKORIKZdE%2BU%3D&reserved=0
