All of lore.kernel.org
 help / color / mirror / Atom feed
From: Johan Hovold <johan+linaro@kernel.org>
To: Alexandre Belloni <alexandre.belloni@bootlin.com>,
	Bjorn Andersson <andersson@kernel.org>
Cc: Andy Gross <agross@kernel.org>,
	Konrad Dybcio <konrad.dybcio@linaro.org>,
	Alessandro Zummo <a.zummo@towertech.it>,
	Rob Herring <robh+dt@kernel.org>,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
	Maximilian Luz <luzmaximilian@gmail.com>,
	linux-arm-msm@vger.kernel.org, linux-rtc@vger.kernel.org,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
	Johan Hovold <johan+linaro@kernel.org>
Subject: [PATCH v2 04/22] rtc: pm8xxx: drop bogus locking
Date: Thu,  2 Feb 2023 16:54:30 +0100	[thread overview]
Message-ID: <20230202155448.6715-5-johan+linaro@kernel.org> (raw)
In-Reply-To: <20230202155448.6715-1-johan+linaro@kernel.org>

Since commit c8d523a4b053 ("drivers/rtc/rtc-pm8xxx.c: rework to support
pm8941 rtc") which removed the shadow control register there is no need
for a driver lock.

Specifically, the rtc ops are serialised by rtc core and the interrupt
handler only unconditionally disables the alarm using the alarm_ctrl
register.

Note that since only the alarm enable bit of alarm_ctrl is used after
enabling the RTC at probe, the locking was not needed when doing open
coded read-modify-write cycles either.

Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
---
 drivers/rtc/rtc-pm8xxx.c | 67 +++++++++++++---------------------------
 1 file changed, 21 insertions(+), 46 deletions(-)

diff --git a/drivers/rtc/rtc-pm8xxx.c b/drivers/rtc/rtc-pm8xxx.c
index 8c2847ac64f4..053a04f74a91 100644
--- a/drivers/rtc/rtc-pm8xxx.c
+++ b/drivers/rtc/rtc-pm8xxx.c
@@ -53,7 +53,6 @@ struct pm8xxx_rtc_regs {
  * @rtc_alarm_irq:	rtc alarm irq number.
  * @regs:		rtc registers description.
  * @rtc_dev:		device structure.
- * @ctrl_reg_lock:	spinlock protecting access to ctrl_reg.
  */
 struct pm8xxx_rtc {
 	struct rtc_device *rtc;
@@ -62,7 +61,6 @@ struct pm8xxx_rtc {
 	int rtc_alarm_irq;
 	const struct pm8xxx_rtc_regs *regs;
 	struct device *rtc_dev;
-	spinlock_t ctrl_reg_lock;
 };
 
 /*
@@ -77,11 +75,11 @@ struct pm8xxx_rtc {
 static int pm8xxx_rtc_set_time(struct device *dev, struct rtc_time *tm)
 {
 	int rc, i;
-	unsigned long secs, irq_flags;
 	struct pm8xxx_rtc *rtc_dd = dev_get_drvdata(dev);
 	const struct pm8xxx_rtc_regs *regs = rtc_dd->regs;
 	u8 value[NUM_8_BIT_RTC_REGS];
 	bool alarm_enabled;
+	unsigned long secs;
 
 	if (!rtc_dd->allow_set_time)
 		return -ENODEV;
@@ -95,51 +93,46 @@ static int pm8xxx_rtc_set_time(struct device *dev, struct rtc_time *tm)
 		secs >>= 8;
 	}
 
-	spin_lock_irqsave(&rtc_dd->ctrl_reg_lock, irq_flags);
-
 	rc = regmap_update_bits_check(rtc_dd->regmap, regs->alarm_ctrl,
 				      regs->alarm_en, 0, &alarm_enabled);
 	if (rc)
-		goto rtc_rw_fail;
+		return rc;
 
 	/* Disable RTC H/w before writing on RTC register */
 	rc = regmap_update_bits(rtc_dd->regmap, regs->ctrl, PM8xxx_RTC_ENABLE, 0);
 	if (rc)
-		goto rtc_rw_fail;
+		return rc;
 
 	/* Write 0 to Byte[0] */
 	rc = regmap_write(rtc_dd->regmap, regs->write, 0);
 	if (rc)
-		goto rtc_rw_fail;
+		return rc;
 
 	/* Write Byte[1], Byte[2], Byte[3] */
 	rc = regmap_bulk_write(rtc_dd->regmap, regs->write + 1,
 			       &value[1], sizeof(value) - 1);
 	if (rc)
-		goto rtc_rw_fail;
+		return rc;
 
 	/* Write Byte[0] */
 	rc = regmap_write(rtc_dd->regmap, regs->write, value[0]);
 	if (rc)
-		goto rtc_rw_fail;
+		return rc;
 
 	/* Enable RTC H/w after writing on RTC register */
 	rc = regmap_update_bits(rtc_dd->regmap, regs->ctrl, PM8xxx_RTC_ENABLE,
 				PM8xxx_RTC_ENABLE);
 	if (rc)
-		goto rtc_rw_fail;
+		return rc;
 
 	if (alarm_enabled) {
 		rc = regmap_update_bits(rtc_dd->regmap, regs->alarm_ctrl,
 					regs->alarm_en, regs->alarm_en);
 		if (rc)
-			goto rtc_rw_fail;
+			return rc;
 	}
 
-rtc_rw_fail:
-	spin_unlock_irqrestore(&rtc_dd->ctrl_reg_lock, irq_flags);
-
-	return rc;
+	return 0;
 }
 
 static int pm8xxx_rtc_read_time(struct device *dev, struct rtc_time *tm)
@@ -184,9 +177,9 @@ static int pm8xxx_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alarm)
 {
 	int rc, i;
 	u8 value[NUM_8_BIT_RTC_REGS];
-	unsigned long secs, irq_flags;
 	struct pm8xxx_rtc *rtc_dd = dev_get_drvdata(dev);
 	const struct pm8xxx_rtc_regs *regs = rtc_dd->regs;
+	unsigned long secs;
 
 	secs = rtc_tm_to_time64(&alarm->time);
 
@@ -200,25 +193,22 @@ static int pm8xxx_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alarm)
 	if (rc)
 		return rc;
 
-	spin_lock_irqsave(&rtc_dd->ctrl_reg_lock, irq_flags);
-
 	rc = regmap_bulk_write(rtc_dd->regmap, regs->alarm_rw, value,
 			       sizeof(value));
 	if (rc)
-		goto rtc_rw_fail;
+		return rc;
 
 	if (alarm->enabled) {
 		rc = regmap_update_bits(rtc_dd->regmap, regs->alarm_ctrl,
 					regs->alarm_en, regs->alarm_en);
 		if (rc)
-			goto rtc_rw_fail;
+			return rc;
 	}
 
 	dev_dbg(dev, "Alarm Set for h:m:s=%ptRt, y-m-d=%ptRdr\n",
 		&alarm->time, &alarm->time);
-rtc_rw_fail:
-	spin_unlock_irqrestore(&rtc_dd->ctrl_reg_lock, irq_flags);
-	return rc;
+
+	return 0;
 }
 
 static int pm8xxx_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alarm)
@@ -255,14 +245,11 @@ static int pm8xxx_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alarm)
 static int pm8xxx_rtc_alarm_irq_enable(struct device *dev, unsigned int enable)
 {
 	int rc;
-	unsigned long irq_flags;
 	struct pm8xxx_rtc *rtc_dd = dev_get_drvdata(dev);
 	const struct pm8xxx_rtc_regs *regs = rtc_dd->regs;
 	u8 value[NUM_8_BIT_RTC_REGS] = {0};
 	unsigned int val;
 
-	spin_lock_irqsave(&rtc_dd->ctrl_reg_lock, irq_flags);
-
 	if (enable)
 		val = regs->alarm_en;
 	else
@@ -271,19 +258,17 @@ static int pm8xxx_rtc_alarm_irq_enable(struct device *dev, unsigned int enable)
 	rc = regmap_update_bits(rtc_dd->regmap, regs->alarm_ctrl,
 				regs->alarm_en, val);
 	if (rc)
-		goto rtc_rw_fail;
+		return rc;
 
 	/* Clear Alarm register */
 	if (!enable) {
 		rc = regmap_bulk_write(rtc_dd->regmap, regs->alarm_rw, value,
 				       sizeof(value));
 		if (rc)
-			goto rtc_rw_fail;
+			return rc;
 	}
 
-rtc_rw_fail:
-	spin_unlock_irqrestore(&rtc_dd->ctrl_reg_lock, irq_flags);
-	return rc;
+	return 0;
 }
 
 static const struct rtc_class_ops pm8xxx_rtc_ops = {
@@ -302,25 +287,18 @@ static irqreturn_t pm8xxx_alarm_trigger(int irq, void *dev_id)
 
 	rtc_update_irq(rtc_dd->rtc, 1, RTC_IRQF | RTC_AF);
 
-	spin_lock(&rtc_dd->ctrl_reg_lock);
-
 	/* Clear the alarm enable bit */
 	rc = regmap_update_bits(rtc_dd->regmap, regs->alarm_ctrl,
 				regs->alarm_en, 0);
-	if (rc) {
-		spin_unlock(&rtc_dd->ctrl_reg_lock);
-		goto rtc_alarm_handled;
-	}
-
-	spin_unlock(&rtc_dd->ctrl_reg_lock);
+	if (rc)
+		goto out;
 
 	/* Clear RTC alarm register */
 	rc = regmap_update_bits(rtc_dd->regmap, regs->alarm_ctrl2,
 				PM8xxx_RTC_ALARM_CLEAR, 0);
 	if (rc)
-		goto rtc_alarm_handled;
-
-rtc_alarm_handled:
+		goto out;
+out:
 	return IRQ_HANDLED;
 }
 
@@ -398,9 +376,6 @@ static int pm8xxx_rtc_probe(struct platform_device *pdev)
 	if (rtc_dd == NULL)
 		return -ENOMEM;
 
-	/* Initialise spinlock to protect RTC control register */
-	spin_lock_init(&rtc_dd->ctrl_reg_lock);
-
 	rtc_dd->regmap = dev_get_regmap(pdev->dev.parent, NULL);
 	if (!rtc_dd->regmap) {
 		dev_err(&pdev->dev, "Parent regmap unavailable.\n");
-- 
2.39.1


  parent reply	other threads:[~2023-02-02 15:55 UTC|newest]

Thread overview: 57+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-02-02 15:54 [PATCH v2 00/22] rtc: pm8xxx: add support for setting time using nvmem Johan Hovold
2023-02-02 15:54 ` [PATCH v2 01/22] rtc: pm8xxx: fix set-alarm race Johan Hovold
2023-02-07  3:12   ` David Collins
2023-02-07 15:13     ` Johan Hovold
2023-02-07 15:56       ` Johan Hovold
2023-02-02 15:54 ` [PATCH v2 02/22] rtc: pm8xxx: drop spmi error messages Johan Hovold
2023-02-02 15:54 ` [PATCH v2 03/22] rtc: pm8xxx: use regmap_update_bits() Johan Hovold
2023-02-07  3:14   ` David Collins
2023-02-02 15:54 ` Johan Hovold [this message]
2023-02-07  3:14   ` [PATCH v2 04/22] rtc: pm8xxx: drop bogus locking David Collins
2023-02-02 15:54 ` [PATCH v2 05/22] rtc: pm8xxx: return IRQ_NONE on errors Johan Hovold
2023-02-02 15:54 ` [PATCH v2 06/22] rtc: pm8xxx: drop unused register defines Johan Hovold
2023-02-07  3:15   ` David Collins
2023-02-07 15:16     ` Johan Hovold
2023-02-02 15:54 ` [PATCH v2 07/22] rtc: pm8xxx: use unaligned le32 helpers Johan Hovold
2023-02-07  3:15   ` David Collins
2023-02-02 15:54 ` [PATCH v2 08/22] rtc: pm8xxx: clean up time and alarm debugging Johan Hovold
2023-02-07  3:15   ` David Collins
2023-02-02 15:54 ` [PATCH v2 09/22] rtc: pm8xxx: rename struct device pointer Johan Hovold
2023-02-07  3:15   ` David Collins
2023-02-02 15:54 ` [PATCH v2 10/22] rtc: pm8xxx: rename alarm irq variable Johan Hovold
2023-02-07  3:15   ` David Collins
2023-02-02 15:54 ` [PATCH v2 11/22] rtc: pm8xxx: clean up comments Johan Hovold
2023-02-07  3:15   ` David Collins
2023-02-02 15:54 ` [PATCH v2 12/22] rtc: pm8xxx: use u32 for timestamps Johan Hovold
2023-02-02 15:54 ` [PATCH v2 13/22] rtc: pm8xxx: refactor read_time() Johan Hovold
2023-02-07  3:16   ` David Collins
2023-02-07 15:20     ` Johan Hovold
2023-02-02 15:54 ` [PATCH v2 14/22] rtc: pm8xxx: clean up local declarations Johan Hovold
2023-02-07  3:16   ` David Collins
2023-02-02 15:54 ` [PATCH v2 15/22] rtc: pm8xxx: drop error messages Johan Hovold
2023-02-02 15:54 ` [PATCH v2 16/22] dt-bindings: rtc: qcom-pm8xxx: add nvmem-cell offset Johan Hovold
2023-02-07  3:16   ` David Collins
2023-02-07 15:24     ` Johan Hovold
2023-02-10 22:48   ` Alexandre Belloni
2023-02-11  8:22     ` Johan Hovold
2023-02-11 16:44       ` Alexandre Belloni
2023-02-15 16:51         ` Johan Hovold
2023-02-24  8:21           ` Johan Hovold
2023-02-24  9:18             ` Alexandre Belloni
2023-02-24  9:24               ` Johan Hovold
2023-02-02 15:54 ` [PATCH v2 17/22] rtc: pm8xxx: add support for nvmem offset Johan Hovold
2023-02-03  3:31   ` Konrad Dybcio
2023-02-03  7:22     ` Johan Hovold
2023-02-07  3:16   ` David Collins
2023-02-07 15:36     ` Johan Hovold
2023-02-02 15:54 ` [PATCH v2 18/22] arm64: defconfig: enable Qualcomm SDAM nvmem driver Johan Hovold
2023-02-03  9:31   ` Krzysztof Kozlowski
2023-02-02 15:54 ` [PATCH v2 19/22] arm64: dts: qcom: sc8280xp-pmics: add pmk8280 rtc Johan Hovold
2023-02-02 15:54 ` [PATCH v2 20/22] arm64: dts: qcom: sc8280xp-pmics: add pmk8280 sdam nvram Johan Hovold
2023-02-02 15:54 ` [PATCH v2 21/22] arm64: dts: qcom: sc8280xp-crd: enable rtc Johan Hovold
2023-02-02 15:54 ` [PATCH v2 22/22] arm64: dts: qcom: sc8280xp-x13s: " Johan Hovold
2023-02-09  4:22 ` (subset) [PATCH v2 00/22] rtc: pm8xxx: add support for setting time using nvmem Bjorn Andersson
2023-02-09 22:25 ` Alexandre Belloni
2023-02-10  7:53   ` Johan Hovold
2023-02-10  9:04     ` Alexandre Belloni
2023-02-10  9:14       ` Johan Hovold

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230202155448.6715-5-johan+linaro@kernel.org \
    --to=johan+linaro@kernel.org \
    --cc=a.zummo@towertech.it \
    --cc=agross@kernel.org \
    --cc=alexandre.belloni@bootlin.com \
    --cc=andersson@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=konrad.dybcio@linaro.org \
    --cc=krzysztof.kozlowski+dt@linaro.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-rtc@vger.kernel.org \
    --cc=luzmaximilian@gmail.com \
    --cc=robh+dt@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.