| Message ID | 20230126142057.25715-17-johan+linaro@kernel.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | rtc: pm8xxx: add support for setting time using nvmem | expand |
On 26/01/2023 14:20, Johan Hovold wrote: > On many Qualcomm platforms the PMIC RTC control and time registers are > read-only so that the RTC time can not be updated. Instead an offset > needs be stored in some machine-specific non-volatile memory, which the > driver can take into account. > > Add support for storing a 32-bit offset from the Epoch in an nvmem cell > so that the RTC time can be set on such platforms. > > Signed-off-by: Johan Hovold <johan+linaro@kernel.org> > --- > drivers/rtc/rtc-pm8xxx.c | 134 +++++++++++++++++++++++++++++++++++---- > 1 file changed, 123 insertions(+), 11 deletions(-) > > diff --git a/drivers/rtc/rtc-pm8xxx.c b/drivers/rtc/rtc-pm8xxx.c > index 922aef0f0241..09816b9f6282 100644 > --- a/drivers/rtc/rtc-pm8xxx.c > +++ b/drivers/rtc/rtc-pm8xxx.c > @@ -3,6 +3,7 @@ > */ > +static int pm8xxx_rtc_read_nvmem_offset(struct pm8xxx_rtc *rtc_dd) > +{ > + size_t len; > + void *buf; > + int rc; > + > + buf = nvmem_cell_read(rtc_dd->nvmem_cell, &len); > + if (IS_ERR(buf)) { > + rc = PTR_ERR(buf); > + dev_err(rtc_dd->dev, "failed to read nvmem offset: %d\n", rc); > + return rc; > + } > + > + if (len != sizeof(u32)) { > + dev_err(rtc_dd->dev, "unexpected nvmem cell size %zu\n", len); > + kfree(buf); > + return -EINVAL; > + } how about us nvmem_cell_read_u32() > + > + rtc_dd->offset = get_unaligned_le32(buf); > + > + kfree(buf); > + > + return 0; > +} > +
On 26/01/2023 15:20:49+0100, Johan Hovold wrote: > On many Qualcomm platforms the PMIC RTC control and time registers are > read-only so that the RTC time can not be updated. Instead an offset > needs be stored in some machine-specific non-volatile memory, which the > driver can take into account. > > Add support for storing a 32-bit offset from the Epoch in an nvmem cell > so that the RTC time can be set on such platforms. > > Signed-off-by: Johan Hovold <johan+linaro@kernel.org> > --- > drivers/rtc/rtc-pm8xxx.c | 134 +++++++++++++++++++++++++++++++++++---- > 1 file changed, 123 insertions(+), 11 deletions(-) > > diff --git a/drivers/rtc/rtc-pm8xxx.c b/drivers/rtc/rtc-pm8xxx.c > index 922aef0f0241..09816b9f6282 100644 > --- a/drivers/rtc/rtc-pm8xxx.c > +++ b/drivers/rtc/rtc-pm8xxx.c > @@ -3,6 +3,7 @@ > */ > #include <linux/of.h> > #include <linux/module.h> > +#include <linux/nvmem-consumer.h> > #include <linux/init.h> > #include <linux/rtc.h> > #include <linux/platform_device.h> > @@ -49,6 +50,8 @@ struct pm8xxx_rtc_regs { > * @alarm_irq: alarm irq number > * @regs: register description > * @dev: device structure > + * @nvmem_cell: nvmem cell for offset > + * @offset: offset from epoch in seconds > */ > struct pm8xxx_rtc { > struct rtc_device *rtc; > @@ -57,8 +60,60 @@ struct pm8xxx_rtc { > int alarm_irq; > const struct pm8xxx_rtc_regs *regs; > struct device *dev; > + struct nvmem_cell *nvmem_cell; > + u32 offset; > }; > > +static int pm8xxx_rtc_read_nvmem_offset(struct pm8xxx_rtc *rtc_dd) > +{ > + size_t len; > + void *buf; > + int rc; > + > + buf = nvmem_cell_read(rtc_dd->nvmem_cell, &len); > + if (IS_ERR(buf)) { > + rc = PTR_ERR(buf); > + dev_err(rtc_dd->dev, "failed to read nvmem offset: %d\n", rc); You removed many dev_err strings in your previous patch and now this is verbose. Honestly, there is not much to do apart from reying the operation so I don't think the strings are worth it. > + return rc; > + } > + > + if (len != sizeof(u32)) { > + dev_err(rtc_dd->dev, "unexpected nvmem cell size %zu\n", len); > + kfree(buf); > + return -EINVAL; > + } > + > + rtc_dd->offset = get_unaligned_le32(buf); > + > + kfree(buf); > + > + return 0; > +} > + > +static int pm8xxx_rtc_write_nvmem_offset(struct pm8xxx_rtc *rtc_dd, u32 offset) > +{ > + u8 buf[sizeof(u32)]; > + int rc; > + > + put_unaligned_le32(offset, buf); > + > + rc = nvmem_cell_write(rtc_dd->nvmem_cell, buf, sizeof(buf)); > + if (rc < 0) { > + dev_err(rtc_dd->dev, "failed to write nvmem offset: %d\n", rc); > + return rc; > + } > + > + return 0; > +} > + > +static int pm8xxx_rtc_read_offset(struct pm8xxx_rtc *rtc_dd) > +{ > + if (!rtc_dd->nvmem_cell) > + return 0; > + > + return pm8xxx_rtc_read_nvmem_offset(rtc_dd); > +} > + > static int pm8xxx_rtc_read_raw(struct pm8xxx_rtc *rtc_dd, u32 *secs) > { > const struct pm8xxx_rtc_regs *regs = rtc_dd->regs; > @@ -90,6 +145,33 @@ static int pm8xxx_rtc_read_raw(struct pm8xxx_rtc *rtc_dd, u32 *secs) > return 0; > } > > +static int pm8xxx_rtc_update_offset(struct pm8xxx_rtc *rtc_dd, u32 secs) > +{ > + u32 raw_secs; > + u32 offset; > + int rc; > + > + if (!rtc_dd->nvmem_cell) > + return -ENODEV; > + > + rc = pm8xxx_rtc_read_raw(rtc_dd, &raw_secs); > + if (rc) > + return rc; > + > + offset = secs - raw_secs; > + > + if (offset == rtc_dd->offset) > + return 0; > + > + rc = pm8xxx_rtc_write_nvmem_offset(rtc_dd, offset); > + if (rc) > + return rc; > + > + rtc_dd->offset = offset; > + > + return 0; > +} > + > /* > * Steps to write the RTC registers. > * 1. Disable alarm if enabled. > @@ -99,23 +181,15 @@ static int pm8xxx_rtc_read_raw(struct pm8xxx_rtc *rtc_dd, u32 *secs) > * 5. Enable rtc if disabled in step 2. > * 6. Enable alarm if disabled in step 1. > */ > -static int pm8xxx_rtc_set_time(struct device *dev, struct rtc_time *tm) > +static int __pm8xxx_rtc_set_time(struct pm8xxx_rtc *rtc_dd, u32 secs) > { > - 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; > - u32 secs; > int rc; > > - if (!rtc_dd->allow_set_time) > - return -ENODEV; > - > - secs = rtc_tm_to_time64(tm); > put_unaligned_le32(secs, value); > > - dev_dbg(dev, "set time: %ptRd %ptRt (%u)\n", tm, tm, secs); > - > rc = regmap_update_bits_check(rtc_dd->regmap, regs->alarm_ctrl, > regs->alarm_en, 0, &alarm_enabled); > if (rc) > @@ -158,6 +232,27 @@ static int pm8xxx_rtc_set_time(struct device *dev, struct rtc_time *tm) > return 0; > } > > +static int pm8xxx_rtc_set_time(struct device *dev, struct rtc_time *tm) > +{ > + struct pm8xxx_rtc *rtc_dd = dev_get_drvdata(dev); > + u32 secs; > + int rc; > + > + secs = rtc_tm_to_time64(tm); > + > + if (rtc_dd->allow_set_time) > + rc = __pm8xxx_rtc_set_time(rtc_dd, secs); > + else > + rc = pm8xxx_rtc_update_offset(rtc_dd, secs); > + > + if (rc) > + return rc; > + > + dev_dbg(dev, "set time: %ptRd %ptRt (%u + %u)\n", tm, tm, > + secs - rtc_dd->offset, rtc_dd->offset); > + return 0; > +} > + > static int pm8xxx_rtc_read_time(struct device *dev, struct rtc_time *tm) > { > struct pm8xxx_rtc *rtc_dd = dev_get_drvdata(dev); > @@ -168,10 +263,11 @@ static int pm8xxx_rtc_read_time(struct device *dev, struct rtc_time *tm) > if (rc) > return rc; > > + secs += rtc_dd->offset; > rtc_time64_to_tm(secs, tm); > > - dev_dbg(dev, "read time: %ptRd %ptRt (%u)\n", tm, tm, secs); > - > + dev_dbg(dev, "read time: %ptRd %ptRt (%u + %u)\n", tm, tm, > + secs - rtc_dd->offset, rtc_dd->offset); > return 0; > } > > @@ -184,6 +280,7 @@ static int pm8xxx_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alarm) > int rc; > > secs = rtc_tm_to_time64(&alarm->time); > + secs -= rtc_dd->offset; > put_unaligned_le32(secs, value); > > rc = regmap_update_bits(rtc_dd->regmap, regs->alarm_ctrl, > @@ -223,6 +320,7 @@ static int pm8xxx_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alarm) > return rc; > > secs = get_unaligned_le32(value); > + secs += rtc_dd->offset; > rtc_time64_to_tm(secs, &alarm->time); > > rc = regmap_read(rtc_dd->regmap, regs->alarm_ctrl, &ctrl_reg); > @@ -380,9 +478,23 @@ static int pm8xxx_rtc_probe(struct platform_device *pdev) > rtc_dd->allow_set_time = of_property_read_bool(pdev->dev.of_node, > "allow-set-time"); > > + rtc_dd->nvmem_cell = devm_nvmem_cell_get(&pdev->dev, "offset"); Maybe we should get something more specific than just "offset" so this could be parsed in the RTC core at some point (this is the second RTC to behave like this) > + if (IS_ERR(rtc_dd->nvmem_cell)) { > + rc = PTR_ERR(rtc_dd->nvmem_cell); > + if (rc != -ENOENT) > + return rc; > + rtc_dd->nvmem_cell = NULL; > + } > + > rtc_dd->regs = match->data; > rtc_dd->dev = &pdev->dev; > > + if (!rtc_dd->allow_set_time) { > + rc = pm8xxx_rtc_read_offset(rtc_dd); > + if (rc) > + return rc; > + } > + > rc = pm8xxx_rtc_enable(rtc_dd); > if (rc) > return rc; > -- > 2.39.1 >
On Fri, Jan 27, 2023 at 02:13:21PM +0000, Srinivas Kandagatla wrote: > On 26/01/2023 14:20, Johan Hovold wrote: > > On many Qualcomm platforms the PMIC RTC control and time registers are > > read-only so that the RTC time can not be updated. Instead an offset > > needs be stored in some machine-specific non-volatile memory, which the > > driver can take into account. > > > > Add support for storing a 32-bit offset from the Epoch in an nvmem cell > > so that the RTC time can be set on such platforms. > > > > Signed-off-by: Johan Hovold <johan+linaro@kernel.org> > > --- > > drivers/rtc/rtc-pm8xxx.c | 134 +++++++++++++++++++++++++++++++++++---- > > 1 file changed, 123 insertions(+), 11 deletions(-) > > > > diff --git a/drivers/rtc/rtc-pm8xxx.c b/drivers/rtc/rtc-pm8xxx.c > > index 922aef0f0241..09816b9f6282 100644 > > --- a/drivers/rtc/rtc-pm8xxx.c > > +++ b/drivers/rtc/rtc-pm8xxx.c > > @@ -3,6 +3,7 @@ > > */ > > +static int pm8xxx_rtc_read_nvmem_offset(struct pm8xxx_rtc *rtc_dd) > > +{ > > + size_t len; > > + void *buf; > > + int rc; > > + > > + buf = nvmem_cell_read(rtc_dd->nvmem_cell, &len); > > + if (IS_ERR(buf)) { > > + rc = PTR_ERR(buf); > > + dev_err(rtc_dd->dev, "failed to read nvmem offset: %d\n", rc); > > + return rc; > > + } > > + > > + if (len != sizeof(u32)) { > > + dev_err(rtc_dd->dev, "unexpected nvmem cell size %zu\n", len); > > + kfree(buf); > > + return -EINVAL; > > + } > how about us nvmem_cell_read_u32() I considered that, but did not like the asymmetry of the interface. Specifically, nvmem_cell_read_u32() would go out and look up the nvmem cell again even though I already have and need a reference for nvmem_cell_write(). nvmem_cell_read_u32() seems to be a better fit as a convenience wrapper for drivers that only need to do a single read at probe. Johan
On Fri, Jan 27, 2023 at 04:09:54PM +0100, Alexandre Belloni wrote: > On 26/01/2023 15:20:49+0100, Johan Hovold wrote: > > On many Qualcomm platforms the PMIC RTC control and time registers are > > read-only so that the RTC time can not be updated. Instead an offset > > needs be stored in some machine-specific non-volatile memory, which the > > driver can take into account. > > > > Add support for storing a 32-bit offset from the Epoch in an nvmem cell > > so that the RTC time can be set on such platforms. > > > > Signed-off-by: Johan Hovold <johan+linaro@kernel.org> > > --- > > drivers/rtc/rtc-pm8xxx.c | 134 +++++++++++++++++++++++++++++++++++---- > > 1 file changed, 123 insertions(+), 11 deletions(-) > > > > diff --git a/drivers/rtc/rtc-pm8xxx.c b/drivers/rtc/rtc-pm8xxx.c > > index 922aef0f0241..09816b9f6282 100644 > > --- a/drivers/rtc/rtc-pm8xxx.c > > +++ b/drivers/rtc/rtc-pm8xxx.c > > @@ -3,6 +3,7 @@ > > */ > > #include <linux/of.h> > > #include <linux/module.h> > > +#include <linux/nvmem-consumer.h> > > #include <linux/init.h> > > #include <linux/rtc.h> > > #include <linux/platform_device.h> > > @@ -49,6 +50,8 @@ struct pm8xxx_rtc_regs { > > * @alarm_irq: alarm irq number > > * @regs: register description > > * @dev: device structure > > + * @nvmem_cell: nvmem cell for offset > > + * @offset: offset from epoch in seconds > > */ > > struct pm8xxx_rtc { > > struct rtc_device *rtc; > > @@ -57,8 +60,60 @@ struct pm8xxx_rtc { > > int alarm_irq; > > const struct pm8xxx_rtc_regs *regs; > > struct device *dev; > > + struct nvmem_cell *nvmem_cell; > > + u32 offset; > > }; > > > > +static int pm8xxx_rtc_read_nvmem_offset(struct pm8xxx_rtc *rtc_dd) > > +{ > > + size_t len; > > + void *buf; > > + int rc; > > + > > + buf = nvmem_cell_read(rtc_dd->nvmem_cell, &len); > > + if (IS_ERR(buf)) { > > + rc = PTR_ERR(buf); > > + dev_err(rtc_dd->dev, "failed to read nvmem offset: %d\n", rc); > > You removed many dev_err strings in your previous patch and now this is > verbose. Honestly, there is not much to do apart from reying the > operation so I don't think the strings are worth it. There's a difference. The SPMI ones are basically equivalent to mmio reads, which we also don't expect to fail (and other spmi drivers also ignore them). These nvmem error paths I actually hit during development and it could help someone trying to enable this feature on a new platform. > > + return rc; > > + } > > + > > + if (len != sizeof(u32)) { > > + dev_err(rtc_dd->dev, "unexpected nvmem cell size %zu\n", len); > > + kfree(buf); > > + return -EINVAL; > > + } > > + > > + rtc_dd->offset = get_unaligned_le32(buf); > > + > > + kfree(buf); > > + > > + return 0; > > +} > > @@ -380,9 +478,23 @@ static int pm8xxx_rtc_probe(struct platform_device *pdev) > > rtc_dd->allow_set_time = of_property_read_bool(pdev->dev.of_node, > > "allow-set-time"); > > > > + rtc_dd->nvmem_cell = devm_nvmem_cell_get(&pdev->dev, "offset"); > > Maybe we should get something more specific than just "offset" so this > could be parsed in the RTC core at some point (this is the second RTC to > behave like this) Yes, that thought crossed my mind, but it's an nvmem cell name (label) and not a generic devicetree property. If you look at the binding document I think the name makes sense given the current description, and I'm not sure changing to something like 'base' would be much of an improvement. I also don't expect there to be more broken RTCs out there like these ones. Hopefully Qualcomm will even get this fixed at some point themselves. And I assume you were think of the old Atmel driver which uses a timer counter and a scratch register as a base? That one is also a bit different in that the timer can be reset, just not set. > > + if (IS_ERR(rtc_dd->nvmem_cell)) { > > + rc = PTR_ERR(rtc_dd->nvmem_cell); > > + if (rc != -ENOENT) > > + return rc; > > + rtc_dd->nvmem_cell = NULL; > > + } Johan
On 27/01/2023 16:51:27+0100, Johan Hovold wrote: > > > +static int pm8xxx_rtc_read_nvmem_offset(struct pm8xxx_rtc *rtc_dd) > > > +{ > > > + size_t len; > > > + void *buf; > > > + int rc; > > > + > > > + buf = nvmem_cell_read(rtc_dd->nvmem_cell, &len); > > > + if (IS_ERR(buf)) { > > > + rc = PTR_ERR(buf); > > > + dev_err(rtc_dd->dev, "failed to read nvmem offset: %d\n", rc); > > > > You removed many dev_err strings in your previous patch and now this is > > verbose. Honestly, there is not much to do apart from reying the > > operation so I don't think the strings are worth it. > > There's a difference. The SPMI ones are basically equivalent to mmio > reads, which we also don't expect to fail (and other spmi drivers also > ignore them). > > These nvmem error paths I actually hit during development and it could > help someone trying to enable this feature on a new platform. > then consider using dev_dbg, the end user will never see or act on those error messages anyway. I'm on a quest to cut down the number of strings in the kernel, at least in the RTC subsystem ;) > > > + return rc; > > > + } > > > + > > > + if (len != sizeof(u32)) { > > > + dev_err(rtc_dd->dev, "unexpected nvmem cell size %zu\n", len); > > > + kfree(buf); > > > + return -EINVAL; > > > + } > > > + > > > + rtc_dd->offset = get_unaligned_le32(buf); > > > + > > > + kfree(buf); > > > + > > > + return 0; > > > +} > > > > @@ -380,9 +478,23 @@ static int pm8xxx_rtc_probe(struct platform_device *pdev) > > > rtc_dd->allow_set_time = of_property_read_bool(pdev->dev.of_node, > > > "allow-set-time"); > > > > > > + rtc_dd->nvmem_cell = devm_nvmem_cell_get(&pdev->dev, "offset"); > > > > Maybe we should get something more specific than just "offset" so this > > could be parsed in the RTC core at some point (this is the second RTC to > > behave like this) > > Yes, that thought crossed my mind, but it's an nvmem cell name (label) > and not a generic devicetree property. If you look at the binding > document I think the name makes sense given the current description, and > I'm not sure changing to something like 'base' would be much of an > improvement. > > I also don't expect there to be more broken RTCs out there like these > ones. Hopefully Qualcomm will even get this fixed at some point > themselves. > > And I assume you were think of the old Atmel driver which uses a timer > counter and a scratch register as a base? That one is also a bit > different in that the timer can be reset, just not set. Nope, I'm thinking about the gamecube one and probably the nintendo switch one which seems to behave similarly (no driver in the kernel though).
On Fri, Jan 27, 2023 at 05:05:03PM +0100, Alexandre Belloni wrote: > On 27/01/2023 16:51:27+0100, Johan Hovold wrote: > > > > @@ -380,9 +478,23 @@ static int pm8xxx_rtc_probe(struct platform_device *pdev) > > > > rtc_dd->allow_set_time = of_property_read_bool(pdev->dev.of_node, > > > > "allow-set-time"); > > > > > > > > + rtc_dd->nvmem_cell = devm_nvmem_cell_get(&pdev->dev, "offset"); > > > > > > Maybe we should get something more specific than just "offset" so this > > > could be parsed in the RTC core at some point (this is the second RTC to > > > behave like this) > > > > Yes, that thought crossed my mind, but it's an nvmem cell name (label) > > and not a generic devicetree property. If you look at the binding > > document I think the name makes sense given the current description, and > > I'm not sure changing to something like 'base' would be much of an > > improvement. > > > > I also don't expect there to be more broken RTCs out there like these > > ones. Hopefully Qualcomm will even get this fixed at some point > > themselves. > > > > And I assume you were think of the old Atmel driver which uses a timer > > counter and a scratch register as a base? That one is also a bit > > different in that the timer can be reset, just not set. > > Nope, I'm thinking about the gamecube one and probably the nintendo > switch one which seems to behave similarly (no driver in the kernel > though). Found the gamecube one now (misread you comment above to imply that it was also out of tree). That one is also different in that the timer in that RTC can also be set (e.g. like the atmel one), but for consistency with some firmware an offset also needs to be read from SRAM (not NVRAM) and applied. That offset is also never updated by Linux. Johan
On 02/02/2023 16:12:33+0100, Johan Hovold wrote: > On Fri, Jan 27, 2023 at 05:05:03PM +0100, Alexandre Belloni wrote: > > On 27/01/2023 16:51:27+0100, Johan Hovold wrote: > > > > > > @@ -380,9 +478,23 @@ static int pm8xxx_rtc_probe(struct platform_device *pdev) > > > > > rtc_dd->allow_set_time = of_property_read_bool(pdev->dev.of_node, > > > > > "allow-set-time"); > > > > > > > > > > + rtc_dd->nvmem_cell = devm_nvmem_cell_get(&pdev->dev, "offset"); > > > > > > > > Maybe we should get something more specific than just "offset" so this > > > > could be parsed in the RTC core at some point (this is the second RTC to > > > > behave like this) > > > > > > Yes, that thought crossed my mind, but it's an nvmem cell name (label) > > > and not a generic devicetree property. If you look at the binding > > > document I think the name makes sense given the current description, and > > > I'm not sure changing to something like 'base' would be much of an > > > improvement. > > > > > > I also don't expect there to be more broken RTCs out there like these > > > ones. Hopefully Qualcomm will even get this fixed at some point > > > themselves. > > > > > > And I assume you were think of the old Atmel driver which uses a timer > > > counter and a scratch register as a base? That one is also a bit > > > different in that the timer can be reset, just not set. > > > > Nope, I'm thinking about the gamecube one and probably the nintendo > > switch one which seems to behave similarly (no driver in the kernel > > though). > > Found the gamecube one now (misread you comment above to imply that it > was also out of tree). > > That one is also different in that the timer in that RTC can also be > set (e.g. like the atmel one), but for consistency with some firmware an > offset also needs to be read from SRAM (not NVRAM) and applied. That > offset is also never updated by Linux. Yeah, I deally, the gamecube counter shouldn't be updated, the switch one doesn't seem to be updatable. I guess the idea being that games need to be able to know if you are messing with the RTC to get an advantage.
diff --git a/drivers/rtc/rtc-pm8xxx.c b/drivers/rtc/rtc-pm8xxx.c index 922aef0f0241..09816b9f6282 100644 --- a/drivers/rtc/rtc-pm8xxx.c +++ b/drivers/rtc/rtc-pm8xxx.c @@ -3,6 +3,7 @@ */ #include <linux/of.h> #include <linux/module.h> +#include <linux/nvmem-consumer.h> #include <linux/init.h> #include <linux/rtc.h> #include <linux/platform_device.h> @@ -49,6 +50,8 @@ struct pm8xxx_rtc_regs { * @alarm_irq: alarm irq number * @regs: register description * @dev: device structure + * @nvmem_cell: nvmem cell for offset + * @offset: offset from epoch in seconds */ struct pm8xxx_rtc { struct rtc_device *rtc; @@ -57,8 +60,60 @@ struct pm8xxx_rtc { int alarm_irq; const struct pm8xxx_rtc_regs *regs; struct device *dev; + struct nvmem_cell *nvmem_cell; + u32 offset; }; +static int pm8xxx_rtc_read_nvmem_offset(struct pm8xxx_rtc *rtc_dd) +{ + size_t len; + void *buf; + int rc; + + buf = nvmem_cell_read(rtc_dd->nvmem_cell, &len); + if (IS_ERR(buf)) { + rc = PTR_ERR(buf); + dev_err(rtc_dd->dev, "failed to read nvmem offset: %d\n", rc); + return rc; + } + + if (len != sizeof(u32)) { + dev_err(rtc_dd->dev, "unexpected nvmem cell size %zu\n", len); + kfree(buf); + return -EINVAL; + } + + rtc_dd->offset = get_unaligned_le32(buf); + + kfree(buf); + + return 0; +} + +static int pm8xxx_rtc_write_nvmem_offset(struct pm8xxx_rtc *rtc_dd, u32 offset) +{ + u8 buf[sizeof(u32)]; + int rc; + + put_unaligned_le32(offset, buf); + + rc = nvmem_cell_write(rtc_dd->nvmem_cell, buf, sizeof(buf)); + if (rc < 0) { + dev_err(rtc_dd->dev, "failed to write nvmem offset: %d\n", rc); + return rc; + } + + return 0; +} + +static int pm8xxx_rtc_read_offset(struct pm8xxx_rtc *rtc_dd) +{ + if (!rtc_dd->nvmem_cell) + return 0; + + return pm8xxx_rtc_read_nvmem_offset(rtc_dd); +} + static int pm8xxx_rtc_read_raw(struct pm8xxx_rtc *rtc_dd, u32 *secs) { const struct pm8xxx_rtc_regs *regs = rtc_dd->regs; @@ -90,6 +145,33 @@ static int pm8xxx_rtc_read_raw(struct pm8xxx_rtc *rtc_dd, u32 *secs) return 0; } +static int pm8xxx_rtc_update_offset(struct pm8xxx_rtc *rtc_dd, u32 secs) +{ + u32 raw_secs; + u32 offset; + int rc; + + if (!rtc_dd->nvmem_cell) + return -ENODEV; + + rc = pm8xxx_rtc_read_raw(rtc_dd, &raw_secs); + if (rc) + return rc; + + offset = secs - raw_secs; + + if (offset == rtc_dd->offset) + return 0; + + rc = pm8xxx_rtc_write_nvmem_offset(rtc_dd, offset); + if (rc) + return rc; + + rtc_dd->offset = offset; + + return 0; +} + /* * Steps to write the RTC registers. * 1. Disable alarm if enabled. @@ -99,23 +181,15 @@ static int pm8xxx_rtc_read_raw(struct pm8xxx_rtc *rtc_dd, u32 *secs) * 5. Enable rtc if disabled in step 2. * 6. Enable alarm if disabled in step 1. */ -static int pm8xxx_rtc_set_time(struct device *dev, struct rtc_time *tm) +static int __pm8xxx_rtc_set_time(struct pm8xxx_rtc *rtc_dd, u32 secs) { - 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; - u32 secs; int rc; - if (!rtc_dd->allow_set_time) - return -ENODEV; - - secs = rtc_tm_to_time64(tm); put_unaligned_le32(secs, value); - dev_dbg(dev, "set time: %ptRd %ptRt (%u)\n", tm, tm, secs); - rc = regmap_update_bits_check(rtc_dd->regmap, regs->alarm_ctrl, regs->alarm_en, 0, &alarm_enabled); if (rc) @@ -158,6 +232,27 @@ static int pm8xxx_rtc_set_time(struct device *dev, struct rtc_time *tm) return 0; } +static int pm8xxx_rtc_set_time(struct device *dev, struct rtc_time *tm) +{ + struct pm8xxx_rtc *rtc_dd = dev_get_drvdata(dev); + u32 secs; + int rc; + + secs = rtc_tm_to_time64(tm); + + if (rtc_dd->allow_set_time) + rc = __pm8xxx_rtc_set_time(rtc_dd, secs); + else + rc = pm8xxx_rtc_update_offset(rtc_dd, secs); + + if (rc) + return rc; + + dev_dbg(dev, "set time: %ptRd %ptRt (%u + %u)\n", tm, tm, + secs - rtc_dd->offset, rtc_dd->offset); + return 0; +} + static int pm8xxx_rtc_read_time(struct device *dev, struct rtc_time *tm) { struct pm8xxx_rtc *rtc_dd = dev_get_drvdata(dev); @@ -168,10 +263,11 @@ static int pm8xxx_rtc_read_time(struct device *dev, struct rtc_time *tm) if (rc) return rc; + secs += rtc_dd->offset; rtc_time64_to_tm(secs, tm); - dev_dbg(dev, "read time: %ptRd %ptRt (%u)\n", tm, tm, secs); - + dev_dbg(dev, "read time: %ptRd %ptRt (%u + %u)\n", tm, tm, + secs - rtc_dd->offset, rtc_dd->offset); return 0; } @@ -184,6 +280,7 @@ static int pm8xxx_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alarm) int rc; secs = rtc_tm_to_time64(&alarm->time); + secs -= rtc_dd->offset; put_unaligned_le32(secs, value); rc = regmap_update_bits(rtc_dd->regmap, regs->alarm_ctrl, @@ -223,6 +320,7 @@ static int pm8xxx_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alarm) return rc; secs = get_unaligned_le32(value); + secs += rtc_dd->offset; rtc_time64_to_tm(secs, &alarm->time); rc = regmap_read(rtc_dd->regmap, regs->alarm_ctrl, &ctrl_reg); @@ -380,9 +478,23 @@ static int pm8xxx_rtc_probe(struct platform_device *pdev) rtc_dd->allow_set_time = of_property_read_bool(pdev->dev.of_node, "allow-set-time"); + rtc_dd->nvmem_cell = devm_nvmem_cell_get(&pdev->dev, "offset"); + if (IS_ERR(rtc_dd->nvmem_cell)) { + rc = PTR_ERR(rtc_dd->nvmem_cell); + if (rc != -ENOENT) + return rc; + rtc_dd->nvmem_cell = NULL; + } + rtc_dd->regs = match->data; rtc_dd->dev = &pdev->dev; + if (!rtc_dd->allow_set_time) { + rc = pm8xxx_rtc_read_offset(rtc_dd); + if (rc) + return rc; + } + rc = pm8xxx_rtc_enable(rtc_dd); if (rc) return rc;
On many Qualcomm platforms the PMIC RTC control and time registers are read-only so that the RTC time can not be updated. Instead an offset needs be stored in some machine-specific non-volatile memory, which the driver can take into account. Add support for storing a 32-bit offset from the Epoch in an nvmem cell so that the RTC time can be set on such platforms. Signed-off-by: Johan Hovold <johan+linaro@kernel.org> --- drivers/rtc/rtc-pm8xxx.c | 134 +++++++++++++++++++++++++++++++++++---- 1 file changed, 123 insertions(+), 11 deletions(-)