Re: [PATCH v3 1/5] rtc: pm8xxx: implement qcom,no-alarm flag for non-HLOS owned alarm

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 10/16/24 2:42 AM, Johan Hovold wrote:
On Mon, Oct 14, 2024 at 08:47:26PM -0400, Jonathan Marek wrote:
Qualcomm x1e80100 firmware sets the ownership of the RTC alarm to ADSP.
Thus writing to RTC alarm registers and receiving alarm interrupts is not
possible.

Add a qcom,no-alarm flag to support RTC on this platform.

An alternative may be to drop the alarm interrupt from DT and use that
as an indicator.


That wouldn't be right, the registers/interrupt still exist and should be described in DT.

(if you have firmware that allows access to the alarm, now you only have to delete the qcom,no-alarm property in your dts to use it)

Signed-off-by: Jonathan Marek <jonathan@xxxxxxxx>
---
  drivers/rtc/rtc-pm8xxx.c | 44 +++++++++++++++++++++++++++-------------
  1 file changed, 30 insertions(+), 14 deletions(-)

diff --git a/drivers/rtc/rtc-pm8xxx.c b/drivers/rtc/rtc-pm8xxx.c
index c32fba550c8e0..1e78939625622 100644
--- a/drivers/rtc/rtc-pm8xxx.c
+++ b/drivers/rtc/rtc-pm8xxx.c
@@ -61,6 +61,7 @@ struct pm8xxx_rtc {
  	struct rtc_device *rtc;
  	struct regmap *regmap;
  	bool allow_set_time;
+	bool no_alarm;

How about inverting this one and naming it has_alarm or similar to avoid
the double negation in your conditionals (!no_alarm)?


My reasoning is that the DT flag has to be negative, and its better to use the same name as the DT flag, but inverting it is OK.

  	int alarm_irq;
  	const struct pm8xxx_rtc_regs *regs;
  	struct device *dev;
@@ -473,9 +474,14 @@ static int pm8xxx_rtc_probe(struct platform_device *pdev)
  	if (!rtc_dd->regmap)
  		return -ENXIO;
- rtc_dd->alarm_irq = platform_get_irq(pdev, 0);
-	if (rtc_dd->alarm_irq < 0)
-		return -ENXIO;
+	rtc_dd->no_alarm = of_property_read_bool(pdev->dev.of_node,
+						 "qcom,no-alarm");
+

Stray newline.


That's not a stray newline?

+	if (!rtc_dd->no_alarm) {
+		rtc_dd->alarm_irq = platform_get_irq(pdev, 0);
+		if (rtc_dd->alarm_irq < 0)
+			return -ENXIO;
+	}
rtc_dd->allow_set_time = of_property_read_bool(pdev->dev.of_node,
  						      "allow-set-time");
@@ -503,7 +509,8 @@ static int pm8xxx_rtc_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, rtc_dd); - device_init_wakeup(&pdev->dev, 1);
+	if (!rtc_dd->no_alarm)
+		device_init_wakeup(&pdev->dev, 1);
rtc_dd->rtc = devm_rtc_allocate_device(&pdev->dev);
  	if (IS_ERR(rtc_dd->rtc))
@@ -512,27 +519,36 @@ static int pm8xxx_rtc_probe(struct platform_device *pdev)
  	rtc_dd->rtc->ops = &pm8xxx_rtc_ops;
  	rtc_dd->rtc->range_max = U32_MAX;
- rc = devm_request_any_context_irq(&pdev->dev, rtc_dd->alarm_irq,
-					  pm8xxx_alarm_trigger,
-					  IRQF_TRIGGER_RISING,
-					  "pm8xxx_rtc_alarm", rtc_dd);
-	if (rc < 0)
-		return rc;
+	if (!rtc_dd->no_alarm) {
+		rc = devm_request_any_context_irq(&pdev->dev, rtc_dd->alarm_irq,
+						  pm8xxx_alarm_trigger,
+						  IRQF_TRIGGER_RISING,
+						  "pm8xxx_rtc_alarm", rtc_dd);
+		if (rc < 0)
+			return rc;
+	}
rc = devm_rtc_register_device(rtc_dd->rtc);
  	if (rc)
  		return rc;
- rc = dev_pm_set_wake_irq(&pdev->dev, rtc_dd->alarm_irq);
-	if (rc)
-		return rc;
+	if (!rtc_dd->no_alarm) {
+		rc = dev_pm_set_wake_irq(&pdev->dev, rtc_dd->alarm_irq);
+		if (rc)
+			return rc;
+	} else {
+		clear_bit(RTC_FEATURE_ALARM, rtc_dd->rtc->features);

I assume that you should be clearing the feature bit before registering
the RTC.


Right, it just needs to be after devm_rtc_allocate_device, not devm_rtc_register_device.

+	}
return 0;
  }

Johan





[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux