diff mbox series

[v2,13/22] rtc: pm8xxx: refactor read_time()

Message ID 20230202155448.6715-14-johan+linaro@kernel.org (mailing list archive)
State Not Applicable
Headers show
Series rtc: pm8xxx: add support for setting time using nvmem | expand

Commit Message

Johan Hovold Feb. 2, 2023, 3:54 p.m. UTC
In preparation for adding support for setting the time by means of an
externally stored offset, refactor read_time() by adding a new helper
that can be used to retrieve the raw time as stored in the RTC.

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

Comments

David Collins Feb. 7, 2023, 3:16 a.m. UTC | #1
On 2/2/23 07:54, Johan Hovold wrote:
> In preparation for adding support for setting the time by means of an
> externally stored offset, refactor read_time() by adding a new helper
> that can be used to retrieve the raw time as stored in the RTC.
> 
> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
> ---
>  drivers/rtc/rtc-pm8xxx.c | 54 ++++++++++++++++++++++++----------------
>  1 file changed, 33 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/rtc/rtc-pm8xxx.c b/drivers/rtc/rtc-pm8xxx.c
> index b1ce246c501a..2f96a178595c 100644
> --- a/drivers/rtc/rtc-pm8xxx.c
> +++ b/drivers/rtc/rtc-pm8xxx.c
> @@ -59,6 +59,37 @@ struct pm8xxx_rtc {
>  	struct device *dev;
>  };
>  
> +static int pm8xxx_rtc_read_raw(struct pm8xxx_rtc *rtc_dd, u32 *secs)

I think that pm8xxx_rtc_read_time_raw() might be a better name for this
function to avoid any possible confusion if it is being used to read the
RTC time or the alarm time.

The patch looks good to me otherwise.

Take care,
David
Johan Hovold Feb. 7, 2023, 3:20 p.m. UTC | #2
On Mon, Feb 06, 2023 at 07:16:09PM -0800, David Collins wrote:
> On 2/2/23 07:54, Johan Hovold wrote:
> > In preparation for adding support for setting the time by means of an
> > externally stored offset, refactor read_time() by adding a new helper
> > that can be used to retrieve the raw time as stored in the RTC.
> > 
> > Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
> > ---
> >  drivers/rtc/rtc-pm8xxx.c | 54 ++++++++++++++++++++++++----------------
> >  1 file changed, 33 insertions(+), 21 deletions(-)
> > 
> > diff --git a/drivers/rtc/rtc-pm8xxx.c b/drivers/rtc/rtc-pm8xxx.c
> > index b1ce246c501a..2f96a178595c 100644
> > --- a/drivers/rtc/rtc-pm8xxx.c
> > +++ b/drivers/rtc/rtc-pm8xxx.c
> > @@ -59,6 +59,37 @@ struct pm8xxx_rtc {
> >  	struct device *dev;
> >  };
> >  
> > +static int pm8xxx_rtc_read_raw(struct pm8xxx_rtc *rtc_dd, u32 *secs)
> 
> I think that pm8xxx_rtc_read_time_raw() might be a better name for this
> function to avoid any possible confusion if it is being used to read the
> RTC time or the alarm time.

Thanks, that's a good suggestion. I don't think there's any real risk
for confusion here, but if I'm sending a v3 I can rename this one.

> The patch looks good to me otherwise.

Johan
diff mbox series

Patch

diff --git a/drivers/rtc/rtc-pm8xxx.c b/drivers/rtc/rtc-pm8xxx.c
index b1ce246c501a..2f96a178595c 100644
--- a/drivers/rtc/rtc-pm8xxx.c
+++ b/drivers/rtc/rtc-pm8xxx.c
@@ -59,6 +59,37 @@  struct pm8xxx_rtc {
 	struct device *dev;
 };
 
+static int pm8xxx_rtc_read_raw(struct pm8xxx_rtc *rtc_dd, u32 *secs)
+{
+	const struct pm8xxx_rtc_regs *regs = rtc_dd->regs;
+	u8 value[NUM_8_BIT_RTC_REGS];
+	unsigned int reg;
+	int rc;
+
+	rc = regmap_bulk_read(rtc_dd->regmap, regs->read, value, sizeof(value));
+	if (rc)
+		return rc;
+
+	/*
+	 * Read the LSB again and check if there has been a carry over.
+	 * If there has, redo the read operation.
+	 */
+	rc = regmap_read(rtc_dd->regmap, regs->read, &reg);
+	if (rc < 0)
+		return rc;
+
+	if (reg < value[0]) {
+		rc = regmap_bulk_read(rtc_dd->regmap, regs->read, value,
+				      sizeof(value));
+		if (rc)
+			return rc;
+	}
+
+	*secs = get_unaligned_le32(value);
+
+	return 0;
+}
+
 /*
  * Steps to write the RTC registers.
  * 1. Disable alarm if enabled.
@@ -129,33 +160,14 @@  static int pm8xxx_rtc_set_time(struct device *dev, struct rtc_time *tm)
 
 static int pm8xxx_rtc_read_time(struct device *dev, struct rtc_time *tm)
 {
-	int rc;
-	u8 value[NUM_8_BIT_RTC_REGS];
-	unsigned int reg;
 	struct pm8xxx_rtc *rtc_dd = dev_get_drvdata(dev);
-	const struct pm8xxx_rtc_regs *regs = rtc_dd->regs;
 	u32 secs;
+	int rc;
 
-	rc = regmap_bulk_read(rtc_dd->regmap, regs->read, value, sizeof(value));
+	rc = pm8xxx_rtc_read_raw(rtc_dd, &secs);
 	if (rc)
 		return rc;
 
-	/*
-	 * Read the LSB again and check if there has been a carry over.
-	 * If there is, redo the read operation.
-	 */
-	rc = regmap_read(rtc_dd->regmap, regs->read, &reg);
-	if (rc < 0)
-		return rc;
-
-	if (unlikely(reg < value[0])) {
-		rc = regmap_bulk_read(rtc_dd->regmap, regs->read,
-				      value, sizeof(value));
-		if (rc)
-			return rc;
-	}
-
-	secs = get_unaligned_le32(value);
 	rtc_time64_to_tm(secs, tm);
 
 	dev_dbg(dev, "read time: %ptRd %ptRt (%u)\n", tm, tm, secs);