* Re: [PATCH] rtc: brcmstb-waketimer: fix settime function
From: Alexandre Belloni @ 2017-07-05 21:13 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Alessandro Zummo, Brian Norris, Gregory Fong, Florian Fainelli,
bcm-kernel-feedback-list, Markus Mayer, linux-rtc,
linux-arm-kernel, linux-kernel
In-Reply-To: <20170628200839.3114345-1-arnd@arndb.de>
Hi,
On 28/06/2017 at 22:07:34 +0200, Arnd Bergmann wrote:
> gcc warns about an unused variable in the new driver:
>
> drivers/rtc/rtc-brcmstb-waketimer.c: In function 'brcmstb_waketmr_settime':
> drivers/rtc/rtc-brcmstb-waketimer.c:142:6: error: unused variable 'ret' [-Werror=unused-variable]
>
> The same function also doesn't handle overflow correctly, this makes
> it return -EINVAL when passed a time that doesn't fit within the
> range of the register.
>
> Fixes: 9f4ad359c801 ("rtc: brcmstb-waketimer: Add Broadcom STB wake-timer")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> drivers/rtc/rtc-brcmstb-waketimer.c | 19 ++++++++++++-------
> 1 file changed, 12 insertions(+), 7 deletions(-)
>
I've squashed it in the original commit, I hope this is fine for you.
> diff --git a/drivers/rtc/rtc-brcmstb-waketimer.c b/drivers/rtc/rtc-brcmstb-waketimer.c
> index 5dea6d0627d8..796ac792a381 100644
> --- a/drivers/rtc/rtc-brcmstb-waketimer.c
> +++ b/drivers/rtc/rtc-brcmstb-waketimer.c
> @@ -138,10 +138,12 @@ static int brcmstb_waketmr_settime(struct device *dev,
> struct rtc_time *tm)
> {
> struct brcmstb_waketmr *timer = dev_get_drvdata(dev);
> - unsigned long sec;
> - int ret;
> + time64_t sec;
> +
> + sec = rtc_tm_to_time64(tm);
>
> - rtc_tm_to_time(tm, &sec);
> + if (sec > U32_MAX || sec < 0)
> + return -EINVAL;
>
> writel_relaxed(sec, timer->base + BRCMSTB_WKTMR_COUNTER);
>
> @@ -152,14 +154,14 @@ static int brcmstb_waketmr_getalarm(struct device *dev,
> struct rtc_wkalrm *alarm)
> {
> struct brcmstb_waketmr *timer = dev_get_drvdata(dev);
> - unsigned long sec;
> + time64_t sec;
> u32 reg;
>
> sec = readl_relaxed(timer->base + BRCMSTB_WKTMR_ALARM);
> if (sec != 0) {
> /* Alarm is enabled */
> alarm->enabled = 1;
> - rtc_time_to_tm(sec, &alarm->time);
> + rtc_time64_to_tm(sec, &alarm->time);
> }
>
> reg = readl_relaxed(timer->base + BRCMSTB_WKTMR_EVENT);
> @@ -172,13 +174,16 @@ static int brcmstb_waketmr_setalarm(struct device *dev,
> struct rtc_wkalrm *alarm)
> {
> struct brcmstb_waketmr *timer = dev_get_drvdata(dev);
> - unsigned long sec;
> + time64_t sec;
>
> if (alarm->enabled)
> - rtc_tm_to_time(&alarm->time, &sec);
> + sec = rtc_tm_to_time64(&alarm->time);
> else
> sec = 0;
>
> + if (sec > U32_MAX || sec < 0)
> + return -EINVAL;
> +
> brcmstb_waketmr_set_alarm(timer, sec);
>
> return 0;
> --
> 2.9.0
>
--
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply
* Re: [PATCH] rtc: rtc-nuc900: fix loop timeout test
From: Alexandre Belloni @ 2017-07-05 21:09 UTC (permalink / raw)
To: Dan Carpenter; +Cc: Wan ZongShun, Alessandro Zummo, linux-rtc, kernel-janitors
In-Reply-To: <20170623082900.GB7922@elgon.mountain>
On 23/06/2017 at 11:29:00 +0300, Dan Carpenter wrote:
> We should change this post-op to a pre-op because we want the loop to
> exit with "timeout" set to zero.
>
> Fixes: 0a89b55364e0 ("nuc900/rtc: change the waiting for device ready implement")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
>
Applied, thanks.
--
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply
* [rtc-linux] Re: [PATCH 3/3 v2] rtc: gemini/ftrtc010: rename driver and symbols
From: Alexandre Belloni @ 2017-07-05 21:01 UTC (permalink / raw)
To: Linus Walleij; +Cc: Alessandro Zummo, rtc-linux, Po-Yu Chuang
In-Reply-To: <20170530075332.3740-3-linus.walleij@linaro.org>
On 30/05/2017 at 09:53:32 +0200, Linus Walleij wrote:
> The Gemini RTC is actually a generic IP block from Faraday
> Technology names FTRTC010. Rename the driver file and all
> symbols to match this IP name.
>
> The relationship can be clearly seen in the U-Boot driver
> posted by Po-Yu Chuang for the Faraday A320 board:
> https://lists.denx.de/pipermail/u-boot/2009-September/061326.html
>
> Remove the dependency on ARCH_GEMINI but select the driver
> for ARCH_GEMINI so we get a smooth transition. The IP block
> is synthsized on different silicon and architectures.
>
> Cc: Po-Yu Chuang <ratbert@faraday-tech.com>
> Acked-by: Hans Ulli Kroll <ulli.kroll@googlemail.com>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> ChangeLog v1->v2:
> - Add Hans' ACK.
> ---
> MAINTAINERS | 2 +-
> drivers/rtc/Kconfig | 10 +--
> drivers/rtc/Makefile | 2 +-
> drivers/rtc/{rtc-gemini.c => rtc-ftrtc010.c} | 91 ++++++++++++++--------------
> 4 files changed, 53 insertions(+), 52 deletions(-)
> rename drivers/rtc/{rtc-gemini.c => rtc-ftrtc010.c} (62%)
>
Applied, thanks.
--
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
--
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
---
You received this message because you are subscribed to the Google Groups "rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
^ permalink raw reply
* [rtc-linux] Re: [PATCH 2/3 v2] rtc: gemini: Augment DT bindings for Faraday
From: Alexandre Belloni @ 2017-07-05 21:01 UTC (permalink / raw)
To: Linus Walleij; +Cc: Alessandro Zummo, rtc-linux, devicetree, Po-Yu Chuang
In-Reply-To: <20170530075332.3740-2-linus.walleij@linaro.org>
On 30/05/2017 at 09:53:31 +0200, Linus Walleij wrote:
> The Gemini RTC is actually a standard IP block from Faraday
> Technology called FTRTC010. Rename the bindings, add the
> generic compatible string and add definitions for the two
> available clocks.
>
> Cc: devicetree@vger.kernel.org
> Cc: Po-Yu Chuang <ratbert@faraday-tech.com>
> Acked-by: Rob Herring <robh@kernel.org>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> ChangeLog v1->v2:
> - Add Rob's ACK.
> ---
> .../devicetree/bindings/rtc/cortina,gemini.txt | 14 -----------
> .../devicetree/bindings/rtc/faraday,ftrtc010.txt | 28 ++++++++++++++++++++++
> 2 files changed, 28 insertions(+), 14 deletions(-)
> delete mode 100644 Documentation/devicetree/bindings/rtc/cortina,gemini.txt
> create mode 100644 Documentation/devicetree/bindings/rtc/faraday,ftrtc010.txt
>
Applied, thanks.
--
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
--
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
---
You received this message because you are subscribed to the Google Groups "rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
^ permalink raw reply
* [rtc-linux] Re: [PATCH 1/3 v2] rtc: gemini: Add optional clock handling
From: Alexandre Belloni @ 2017-07-05 21:01 UTC (permalink / raw)
To: Linus Walleij; +Cc: Alessandro Zummo, rtc-linux
In-Reply-To: <20170530075332.3740-1-linus.walleij@linaro.org>
On 30/05/2017 at 09:53:30 +0200, Linus Walleij wrote:
> This makes the Gemini optionally take two clock references to
> the PCLK and EXTCLK. As we are adding a clock framework to the
> Gemini platform we need to make sure that we get the right
> references.
>
> Acked-by: Hans Ulli Kroll <ulli.kroll@googlemail.com>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> ChangeLog v1->v2:
> - Use devm_clk_get() so we do not leave dangling clocks
> behind.
> - Add Hans' ACK.
> ---
> drivers/rtc/rtc-gemini.c | 28 ++++++++++++++++++++++++++++
> 1 file changed, 28 insertions(+)
>
Applied, thanks.
--
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
--
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
---
You received this message because you are subscribed to the Google Groups "rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
^ permalink raw reply
* Re: [PATCH v3 3/4] rtc: ds1307: factor out century bit handling
From: Alexandre Belloni @ 2017-07-05 20:54 UTC (permalink / raw)
To: Heiner Kallweit; +Cc: linux-rtc
In-Reply-To: <cec62c55-1a12-9804-e589-0014eb482ca1@gmail.com>
On 05/06/2017 at 17:57:33 +0200, Heiner Kallweit wrote:
> The driver has lots of places with chip-specific code what doesn't
> necessarily facilitate maintenance.
>
> Let's describe chip-specific differences in century bit handling
> in struct chip_desc to improve this.
>
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> v2:
> - no changes
> v3:
> - add Reviewed-by
> ---
> drivers/rtc/rtc-ds1307.c | 70 ++++++++++++++++++------------------------------
> 1 file changed, 26 insertions(+), 44 deletions(-)
>
Applied, thanks.
--
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply
* Re: [PATCH v3 2/4] rtc: ds1307: use regmap_update_bits where applicable
From: Alexandre Belloni @ 2017-07-05 20:54 UTC (permalink / raw)
To: Heiner Kallweit; +Cc: linux-rtc
In-Reply-To: <df917f73-1dfb-14db-be69-15aa52043938@gmail.com>
On 05/06/2017 at 17:57:29 +0200, Heiner Kallweit wrote:
> After the switch to regmap we can now make use of regmap_update_bits
> to simplify read/modify/write ops.
>
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> v2:
> - no changes
> v3:
> - add Reviewed-by
> ---
> drivers/rtc/rtc-ds1307.c | 82 ++++++++++++------------------------------------
> 1 file changed, 20 insertions(+), 62 deletions(-)
>
Applied, thanks.
--
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply
* [rtc-linux] Re: [PATCH 2/2] rtc: stm32: add STM32H7 RTC support
From: Amelie DELAUNAY @ 2017-07-05 14:19 UTC (permalink / raw)
To: Alessandro Zummo, Alexandre Belloni, Rob Herring, Mark Rutland,
Maxime Coquelin, Alexandre Torgue
Cc: rtc-linux, devicetree, linux-arm-kernel, linux-kernel
In-Reply-To: <1498470689-26829-3-git-send-email-amelie.delaunay@st.com>
On 06/26/2017 11:51 AM, Amelie Delaunay wrote:
> This patch adds support for STM32H7 RTC. On STM32H7, the RTC bus interface
> clock (APB clock) needs to be enabled.
>
> Signed-off-by: Amelie Delaunay <amelie.delaunay@st.com>
> ---
A gentle ping about this patch.
Thanks,
Amelie
> drivers/rtc/rtc-stm32.c | 85 +++++++++++++++++++++++++++++++++++++++----------
> 1 file changed, 68 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/rtc/rtc-stm32.c b/drivers/rtc/rtc-stm32.c
> index bd57eb1..22f988b 100644
> --- a/drivers/rtc/rtc-stm32.c
> +++ b/drivers/rtc/rtc-stm32.c
> @@ -94,11 +94,17 @@
> /* STM32_PWR_CR bit field */
> #define PWR_CR_DBP BIT(8)
>
> +struct stm32_rtc_data {
> + bool has_pclk;
> +};
> +
> struct stm32_rtc {
> struct rtc_device *rtc_dev;
> void __iomem *base;
> struct regmap *dbp;
> - struct clk *ck_rtc;
> + struct stm32_rtc_data *data;
> + struct clk *pclk;
> + struct clk *rtc_ck;
> int irq_alarm;
> };
>
> @@ -122,9 +128,9 @@ static int stm32_rtc_enter_init_mode(struct stm32_rtc *rtc)
> writel_relaxed(isr, rtc->base + STM32_RTC_ISR);
>
> /*
> - * It takes around 2 ck_rtc clock cycles to enter in
> + * It takes around 2 rtc_ck clock cycles to enter in
> * initialization phase mode (and have INITF flag set). As
> - * slowest ck_rtc frequency may be 32kHz and highest should be
> + * slowest rtc_ck frequency may be 32kHz and highest should be
> * 1MHz, we poll every 10 us with a timeout of 100ms.
> */
> return readl_relaxed_poll_timeout_atomic(
> @@ -153,7 +159,7 @@ static int stm32_rtc_wait_sync(struct stm32_rtc *rtc)
>
> /*
> * Wait for RSF to be set to ensure the calendar registers are
> - * synchronised, it takes around 2 ck_rtc clock cycles
> + * synchronised, it takes around 2 rtc_ck clock cycles
> */
> return readl_relaxed_poll_timeout_atomic(rtc->base + STM32_RTC_ISR,
> isr,
> @@ -456,7 +462,7 @@ static int stm32_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alrm)
>
> /*
> * Poll Alarm write flag to be sure that Alarm update is allowed: it
> - * takes around 2 ck_rtc clock cycles
> + * takes around 2 rtc_ck clock cycles
> */
> ret = readl_relaxed_poll_timeout_atomic(rtc->base + STM32_RTC_ISR,
> isr,
> @@ -490,8 +496,17 @@ static int stm32_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alrm)
> .alarm_irq_enable = stm32_rtc_alarm_irq_enable,
> };
>
> +static const struct stm32_rtc_data stm32_rtc_data = {
> + .has_pclk = false,
> +};
> +
> +static const struct stm32_rtc_data stm32h7_rtc_data = {
> + .has_pclk = true,
> +};
> +
> static const struct of_device_id stm32_rtc_of_match[] = {
> - { .compatible = "st,stm32-rtc" },
> + { .compatible = "st,stm32-rtc", .data = &stm32_rtc_data },
> + { .compatible = "st,stm32h7-rtc", .data = &stm32h7_rtc_data },
> {}
> };
> MODULE_DEVICE_TABLE(of, stm32_rtc_of_match);
> @@ -503,7 +518,7 @@ static int stm32_rtc_init(struct platform_device *pdev,
> unsigned int rate;
> int ret = 0;
>
> - rate = clk_get_rate(rtc->ck_rtc);
> + rate = clk_get_rate(rtc->rtc_ck);
>
> /* Find prediv_a and prediv_s to obtain the 1Hz calendar clock */
> pred_a_max = STM32_RTC_PRER_PRED_A >> STM32_RTC_PRER_PRED_A_SHIFT;
> @@ -524,7 +539,7 @@ static int stm32_rtc_init(struct platform_device *pdev,
> pred_a = pred_a_max;
> pred_s = (rate / (pred_a + 1)) - 1;
>
> - dev_warn(&pdev->dev, "ck_rtc is %s\n",
> + dev_warn(&pdev->dev, "rtc_ck is %s\n",
> (rate < ((pred_a + 1) * (pred_s + 1))) ?
> "fast" : "slow");
> }
> @@ -561,6 +576,7 @@ static int stm32_rtc_probe(struct platform_device *pdev)
> {
> struct stm32_rtc *rtc;
> struct resource *res;
> + const struct of_device_id *match;
> int ret;
>
> rtc = devm_kzalloc(&pdev->dev, sizeof(*rtc), GFP_KERNEL);
> @@ -579,15 +595,37 @@ static int stm32_rtc_probe(struct platform_device *pdev)
> return PTR_ERR(rtc->dbp);
> }
>
> - rtc->ck_rtc = devm_clk_get(&pdev->dev, NULL);
> - if (IS_ERR(rtc->ck_rtc)) {
> - dev_err(&pdev->dev, "no ck_rtc clock");
> - return PTR_ERR(rtc->ck_rtc);
> + match = of_match_device(stm32_rtc_of_match, &pdev->dev);
> + if (match && match->data)
> + rtc->data = (struct stm32_rtc_data *)match->data;
> + else
> + return -EINVAL;
> +
> + if (!rtc->data->has_pclk) {
> + rtc->pclk = NULL;
> + rtc->rtc_ck = devm_clk_get(&pdev->dev, NULL);
> + } else {
> + rtc->pclk = devm_clk_get(&pdev->dev, "pclk");
> + if (rtc->data->has_pclk && IS_ERR(rtc->pclk)) {
> + dev_err(&pdev->dev, "no pclk clock");
> + return PTR_ERR(rtc->rtc_ck);
> + }
> + rtc->rtc_ck = devm_clk_get(&pdev->dev, "rtc_ck");
> + }
> + if (IS_ERR(rtc->rtc_ck)) {
> + dev_err(&pdev->dev, "no rtc_ck clock");
> + return PTR_ERR(rtc->rtc_ck);
> }
>
> - ret = clk_prepare_enable(rtc->ck_rtc);
> + if (rtc->data->has_pclk) {
> + ret = clk_prepare_enable(rtc->pclk);
> + if (ret)
> + return ret;
> + }
> +
> + ret = clk_prepare_enable(rtc->rtc_ck);
> if (ret)
> - return ret;
> + goto err;
>
> regmap_update_bits(rtc->dbp, PWR_CR, PWR_CR_DBP, PWR_CR_DBP);
>
> @@ -595,7 +633,7 @@ static int stm32_rtc_probe(struct platform_device *pdev)
> * After a system reset, RTC_ISR.INITS flag can be read to check if
> * the calendar has been initalized or not. INITS flag is reset by a
> * power-on reset (no vbat, no power-supply). It is not reset if
> - * ck_rtc parent clock has changed (so RTC prescalers need to be
> + * rtc_ck parent clock has changed (so RTC prescalers need to be
> * changed). That's why we cannot rely on this flag to know if RTC
> * init has to be done.
> */
> @@ -646,7 +684,9 @@ static int stm32_rtc_probe(struct platform_device *pdev)
>
> return 0;
> err:
> - clk_disable_unprepare(rtc->ck_rtc);
> + if (rtc->data->has_pclk)
> + clk_disable_unprepare(rtc->pclk);
> + clk_disable_unprepare(rtc->rtc_ck);
>
> regmap_update_bits(rtc->dbp, PWR_CR, PWR_CR_DBP, 0);
>
> @@ -667,7 +707,9 @@ static int stm32_rtc_remove(struct platform_device *pdev)
> writel_relaxed(cr, rtc->base + STM32_RTC_CR);
> stm32_rtc_wpr_lock(rtc);
>
> - clk_disable_unprepare(rtc->ck_rtc);
> + clk_disable_unprepare(rtc->rtc_ck);
> + if (rtc->data->has_pclk)
> + clk_disable_unprepare(rtc->pclk);
>
> /* Enable backup domain write protection */
> regmap_update_bits(rtc->dbp, PWR_CR, PWR_CR_DBP, 0);
> @@ -682,6 +724,9 @@ static int stm32_rtc_suspend(struct device *dev)
> {
> struct stm32_rtc *rtc = dev_get_drvdata(dev);
>
> + if (rtc->data->has_pclk)
> + clk_disable_unprepare(rtc->pclk);
> +
> if (device_may_wakeup(dev))
> return enable_irq_wake(rtc->irq_alarm);
>
> @@ -693,6 +738,12 @@ static int stm32_rtc_resume(struct device *dev)
> struct stm32_rtc *rtc = dev_get_drvdata(dev);
> int ret = 0;
>
> + if (rtc->data->has_pclk) {
> + ret = clk_prepare_enable(rtc->pclk);
> + if (ret)
> + return ret;
> + }
> +
> ret = stm32_rtc_wait_sync(rtc);
> if (ret < 0)
> return ret;
>
--
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
---
You received this message because you are subscribed to the Google Groups "rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
^ permalink raw reply
* [rtc-linux] [PATCH v2 2/2] RTC: s35390a: implement ioctls
From: Fabien Lahoudere @ 2017-07-05 8:02 UTC (permalink / raw)
To: alexandre.belloni; +Cc: a.zummo, rtc-linux, Fabien Lahoudere
In-Reply-To: <cover.1499240924.git.fabien.lahoudere@collabora.co.uk>
Implements RTC_VL_READ and RTC_VL_CLR ioctls.
Signed-off-by: Fabien Lahoudere <fabien.lahoudere@collabora.co.uk>
---
drivers/rtc/rtc-s35390a.c | 32 +++++++++++++++++++++++++++++++-
1 file changed, 31 insertions(+), 1 deletion(-)
diff --git a/drivers/rtc/rtc-s35390a.c b/drivers/rtc/rtc-s35390a.c
index 5ebb132..9261ba6 100644
--- a/drivers/rtc/rtc-s35390a.c
+++ b/drivers/rtc/rtc-s35390a.c
@@ -397,12 +397,42 @@ static int s35390a_rtc_set_time(struct device *dev, struct rtc_time *tm)
return s35390a_set_datetime(to_i2c_client(dev), tm);
}
+static int s35390a_rtc_ioctl(struct device *dev, unsigned int cmd,
+ unsigned long arg)
+{
+ struct i2c_client *client = to_i2c_client(dev);
+ struct s35390a *s35390a = i2c_get_clientdata(client);
+ char sts;
+ int err;
+
+ switch (cmd) {
+ case RTC_VL_READ:
+ /* s35390a_reset set lowvoltage flag and init RTC if needed */
+ err = s35390a_read_status(s35390a, &sts);
+ if (err < 0)
+ return err;
+ if (copy_to_user((void __user *)arg, &err, sizeof(int)))
+ return -EFAULT;
+ break;
+ case RTC_VL_CLR:
+ /* update flag and clear register */
+ err = s35390a_init(s35390a);
+ if (err < 0)
+ return err;
+ break;
+ default:
+ return -ENOIOCTLCMD;
+ }
+
+ return 0;
+}
+
static const struct rtc_class_ops s35390a_rtc_ops = {
.read_time = s35390a_rtc_read_time,
.set_time = s35390a_rtc_set_time,
.set_alarm = s35390a_rtc_set_alarm,
.read_alarm = s35390a_rtc_read_alarm,
-
+ .ioctl = s35390a_rtc_ioctl,
};
static struct i2c_driver s35390a_driver;
--
1.8.3.1
--
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
---
You received this message because you are subscribed to the Google Groups "rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
^ permalink raw reply related
* [rtc-linux] [PATCH v2 1/2] RTC: s35390a: handle invalid RTC time
From: Fabien Lahoudere @ 2017-07-05 8:02 UTC (permalink / raw)
To: alexandre.belloni; +Cc: a.zummo, rtc-linux, Fabien Lahoudere
In-Reply-To: <cover.1499240924.git.fabien.lahoudere@collabora.co.uk>
If RTC time have been altered by low voltage, we notify users
that RTC time is invalid by returning -EINVAL.
The RTC time needs to be set correctly to clear the invalid flag.
If the RTC is not set before restarting, the information will be lost.
Signed-off-by: Fabien Lahoudere <fabien.lahoudere@collabora.co.uk>
---
drivers/rtc/rtc-s35390a.c | 72 +++++++++++++++++++++++++++--------------------
1 file changed, 42 insertions(+), 30 deletions(-)
diff --git a/drivers/rtc/rtc-s35390a.c b/drivers/rtc/rtc-s35390a.c
index 5dab466..5ebb132 100644
--- a/drivers/rtc/rtc-s35390a.c
+++ b/drivers/rtc/rtc-s35390a.c
@@ -99,33 +99,12 @@ static int s35390a_get_reg(struct s35390a *s35390a, int reg, char *buf, int len)
return 0;
}
-/*
- * Returns <0 on error, 0 if rtc is setup fine and 1 if the chip was reset.
- * To keep the information if an irq is pending, pass the value read from
- * STATUS1 to the caller.
- */
-static int s35390a_reset(struct s35390a *s35390a, char *status1)
+static int s35390a_init(struct s35390a *s35390a)
{
char buf;
int ret;
unsigned initcount = 0;
- ret = s35390a_get_reg(s35390a, S35390A_CMD_STATUS1, status1, 1);
- if (ret < 0)
- return ret;
-
- if (*status1 & S35390A_FLAG_POC)
- /*
- * Do not communicate for 0.5 seconds since the power-on
- * detection circuit is in operation.
- */
- msleep(500);
- else if (!(*status1 & S35390A_FLAG_BLD))
- /*
- * If both POC and BLD are unset everything is fine.
- */
- return 0;
-
/*
* At least one of POC and BLD are set, so reinitialise chip. Keeping
* this information in the hardware to know later that the time isn't
@@ -135,7 +114,6 @@ static int s35390a_reset(struct s35390a *s35390a, char *status1)
* The 24H bit is kept over reset, so set it already here.
*/
initialize:
- *status1 = S35390A_FLAG_24H;
buf = S35390A_FLAG_RESET | S35390A_FLAG_24H;
ret = s35390a_set_reg(s35390a, S35390A_CMD_STATUS1, &buf, 1);
@@ -158,6 +136,34 @@ static int s35390a_reset(struct s35390a *s35390a, char *status1)
return 1;
}
+/*
+ * Returns <0 on error, 0 if rtc is setup fine and 1 if the chip was reset.
+ * To keep the information if an irq is pending, pass the value read from
+ * STATUS1 to the caller.
+ */
+static int s35390a_read_status(struct s35390a *s35390a, char *status1)
+{
+ int ret;
+
+ ret = s35390a_get_reg(s35390a, S35390A_CMD_STATUS1, status1, 1);
+ if (ret < 0)
+ return ret;
+
+ if (*status1 & S35390A_FLAG_POC) {
+ /*
+ * Do not communicate for 0.5 seconds since the power-on
+ * detection circuit is in operation.
+ */
+ msleep(500);
+ return 1;
+ } else if (*status1 & S35390A_FLAG_BLD)
+ return 1;
+ /*
+ * If both POC and BLD are unset everything is fine.
+ */
+ return 0;
+}
+
static int s35390a_disable_test_mode(struct s35390a *s35390a)
{
char buf[1];
@@ -201,13 +207,16 @@ static int s35390a_set_datetime(struct i2c_client *client, struct rtc_time *tm)
{
struct s35390a *s35390a = i2c_get_clientdata(client);
int i, err;
- char buf[7];
+ char buf[7], status;
dev_dbg(&client->dev, "%s: tm is secs=%d, mins=%d, hours=%d mday=%d, "
"mon=%d, year=%d, wday=%d\n", __func__, tm->tm_sec,
tm->tm_min, tm->tm_hour, tm->tm_mday, tm->tm_mon, tm->tm_year,
tm->tm_wday);
+ if (s35390a_read_status(s35390a, &status) == 1)
+ s35390a_init(s35390a);
+
buf[S35390A_BYTE_YEAR] = bin2bcd(tm->tm_year - 100);
buf[S35390A_BYTE_MONTH] = bin2bcd(tm->tm_mon + 1);
buf[S35390A_BYTE_DAY] = bin2bcd(tm->tm_mday);
@@ -228,9 +237,12 @@ static int s35390a_set_datetime(struct i2c_client *client, struct rtc_time *tm)
static int s35390a_get_datetime(struct i2c_client *client, struct rtc_time *tm)
{
struct s35390a *s35390a = i2c_get_clientdata(client);
- char buf[7];
+ char buf[7], status;
int i, err;
+ if (s35390a_read_status(s35390a, &status) == 1)
+ return -EINVAL;
+
err = s35390a_get_reg(s35390a, S35390A_CMD_TIME1, buf, sizeof(buf));
if (err < 0)
return err;
@@ -398,7 +410,7 @@ static int s35390a_rtc_set_time(struct device *dev, struct rtc_time *tm)
static int s35390a_probe(struct i2c_client *client,
const struct i2c_device_id *id)
{
- int err, err_reset;
+ int err, err_read;
unsigned int i;
struct s35390a *s35390a;
struct rtc_time tm;
@@ -431,9 +443,9 @@ static int s35390a_probe(struct i2c_client *client,
}
}
- err_reset = s35390a_reset(s35390a, &status1);
- if (err_reset < 0) {
- err = err_reset;
+ err_read = s35390a_read_status(s35390a, &status1);
+ if (err_read < 0) {
+ err = err_read;
dev_err(&client->dev, "error resetting chip\n");
goto exit_dummy;
}
@@ -459,7 +471,7 @@ static int s35390a_probe(struct i2c_client *client,
}
}
- if (err_reset > 0 || s35390a_get_datetime(client, &tm) < 0)
+ if (err_read > 0 || s35390a_get_datetime(client, &tm) < 0)
dev_warn(&client->dev, "clock needs to be set\n");
device_set_wakeup_capable(&client->dev, 1);
--
1.8.3.1
--
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
---
You received this message because you are subscribed to the Google Groups "rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
^ permalink raw reply related
* [rtc-linux] [PATCH v2 0/2] RTC: s35390a: Improve low voltage or invalid time detection
From: Fabien Lahoudere @ 2017-07-05 8:02 UTC (permalink / raw)
To: alexandre.belloni; +Cc: a.zummo, rtc-linux, Fabien Lahoudere
This patchset implements feature to detect RTC low voltage or power off.
Changes since v1:
- Read register instead setting flags
Fabien Lahoudere (2):
RTC: s35390a: handle invalid RTC time
RTC: s35390a: implement ioctls
drivers/rtc/rtc-s35390a.c | 104 ++++++++++++++++++++++++++++++++--------------
1 file changed, 73 insertions(+), 31 deletions(-)
--
1.8.3.1
--
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
---
You received this message because you are subscribed to the Google Groups "rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
^ permalink raw reply
* [rtc-linux] Re: [PATCH v2 0/3] rtc: make st-lpc robust against y2038/2106 bug
From: Benjamin Gaignard @ 2017-07-04 11:51 UTC (permalink / raw)
To: shuah
Cc: Alexandre Belloni, John Stultz, Thomas Gleixner, sboyd,
Linux Kernel Mailing List, linux-kselftest, patrice.chotard,
Alessandro Zummo, linux-arm-kernel, rtc-linux,
Linaro Kernel Mailman List
In-Reply-To: <cea926b2-5d84-18a9-9713-52d1495d550f@kernel.org>
2017-06-24 0:34 GMT+02:00 Shuah Khan <shuah@kernel.org>:
> Hi Alexandre,
>
> On 06/23/2017 04:09 PM, Alexandre Belloni wrote:
>> On 23/06/2017 at 13:40:41 -0600, Shuah Khan wrote:
>>> On 06/19/2017 03:36 AM, Benjamin Gaignard wrote:
>>>> On 32bits platforms "struct timeval" or "time_t" are using u32 to code the
>>>> date, this cause tools like "date" or "hwclock" failed even before setting
>>>> the RTC device if the date is superior to year 2038 (or 2106).
>>>>
>>>> To avoid this problem I add one RTC test file which directly use RTC ioctl
>>>> to set and read RTC time and alarm values.
>>>> rtctest_setdate allow to set any date/time given in the command line.
>>>>
>>>> On this version 2 I add check of problematics years in rtctest like suggest
>>>> by Alexandre.
>>>>
>>>> Finally that had allowed me to test and fix rtc-st-lpc driver.
>>>>
>>>> Benjamin Gaignard (3):
>>>> tools: timer: add rtctest_setdate
>>>> tool: timer: rtctest add check for problematic dates
>>>> rtc: st-lpc: make it robust against y2038/2106 bug
>>>>
>>>> drivers/rtc/rtc-st-lpc.c | 19 ++--
>>>> tools/testing/selftests/timers/Makefile | 2 +-
>>>> tools/testing/selftests/timers/rtctest.c | 121 ++++++++++++++++++++++-
>>>> tools/testing/selftests/timers/rtctest_setdate.c | 86 ++++++++++++++++
>>>> 4 files changed, 212 insertions(+), 16 deletions(-)
>>>> create mode 100644 tools/testing/selftests/timers/rtctest_setdate.c
>>>>
>>>
>>> Hi Thomas/John,
>>>
>>> I can take the first two patches in this series through linux-kselftest
>>> with your or John's Ack. Please review and let me know one way or the
>>> other.
>>>>
>> Well, I'm the maintainer for rtctest.c and I'll make sure to also be the
>> one for rtctest_setdate.c>
>>> The third one is a rtc driver patch. Please let me know how do you want
>>> to handle this series soon we can get this into 4.13-rc1.
>>>
>>
>> I'll take the three patches but I still have comment I didn't have time
>> to give yet.
Alexandre, may you had time to give me feedback on this ?
Regards,
Benjamin
>>
>>
>
> Okay. I will drop this off my radar then :)
>
> thanks,
> -- Shuah
>
--
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
---
You received this message because you are subscribed to the Google Groups "rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
^ permalink raw reply
* [rtc-linux] Re: [PATCH 1/2] RTC: s35390a: handle invalid RTC time
From: Alexandre Belloni @ 2017-07-04 11:19 UTC (permalink / raw)
To: Fabien Lahoudere; +Cc: a.zummo, rtc-linux
In-Reply-To: <1499158552.6885.1.camel@collabora.co.uk>
On 04/07/2017 at 10:55:52 +0200, Fabien Lahoudere wrote:
> On Thu, 2017-06-29 at 09:56 +0200, Alexandre Belloni wrote:
> > On 26/06/2017 at 11:51:13 +0200, Fabien Lahoudere wrote:
> > > > Actually, after reading the datasheet, I realize it is only POC that is
> > > > reset to 0 after reading so isinvalid is not needed. Just read status1
> > > > and look for BLD instead of caching it.
> > > >
> > >
> > > isinvalid is also used in s35390a_set_datetime. So if I remove it how can I detect that time
> > > setting
> > > failed?
> > >
> >
> > If it fails, simply don't reset BLD so it is still set when reading the
> > time.
> >
>
> In BLD section, datasheet says "When this flag is "1", be sure to initialize."
> So I think we need those flags.
>
No, you don't. If BLD is set, return -EINVAL in read_time, initialize
the RTC in set_time and that is it. You'll always know when power failed
and you'll always initialize at the correct time (i.e before really
caring about the date/time).
> > > > I think it is probably worth separating s35390a_reset() into two
> > > > functions. One that does the initialization and another one that reads
> > > > status1 and immediately doest the initialization when POC is set. If BLD
> > > > is set, then we can wait for set_time to happen before initializing.
> > > >
> >
> >
--
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
--
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
---
You received this message because you are subscribed to the Google Groups "rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
^ permalink raw reply
* [rtc-linux] Re: [PATCH 1/2] RTC: s35390a: handle invalid RTC time
From: Fabien Lahoudere @ 2017-07-04 8:55 UTC (permalink / raw)
To: Alexandre Belloni; +Cc: a.zummo, rtc-linux
In-Reply-To: <20170629075651.jqa3jvxfer56dvh5@piout.net>
On Thu, 2017-06-29 at 09:56 +0200, Alexandre Belloni wrote:
> On 26/06/2017 at 11:51:13 +0200, Fabien Lahoudere wrote:
> > > Actually, after reading the datasheet, I realize it is only POC that is
> > > reset to 0 after reading so isinvalid is not needed. Just read status1
> > > and look for BLD instead of caching it.
> > >
> >
> > isinvalid is also used in s35390a_set_datetime. So if I remove it how can I detect that time
> > setting
> > failed?
> >
>
> If it fails, simply don't reset BLD so it is still set when reading the
> time.
>
In BLD section, datasheet says "When this flag is "1", be sure to initialize."
So I think we need those flags.
> > > I think it is probably worth separating s35390a_reset() into two
> > > functions. One that does the initialization and another one that reads
> > > status1 and immediately doest the initialization when POC is set. If BLD
> > > is set, then we can wait for set_time to happen before initializing.
> > >
>
>
--
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
---
You received this message because you are subscribed to the Google Groups "rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
^ permalink raw reply
* [rtc-linux] Re: [PATCH 35/51] rtc: pm8xxx: stop using rtc deprecated functions
From: Arnd Bergmann @ 2017-07-03 10:12 UTC (permalink / raw)
To: Benjamin Gaignard
Cc: linaro-kernel, Alessandro Zummo, Alexandre Belloni, rtc-linux,
Linux Kernel Mailing List
In-Reply-To: <1497951359-13334-36-git-send-email-benjamin.gaignard@linaro.org>
On Tue, Jun 20, 2017 at 11:35 AM, Benjamin Gaignard
<benjamin.gaignard@linaro.org> wrote:
> rtc_time_to_tm() and rtc_tm_to_time() are deprecated because they
> rely on 32bits variables and that will make rtc break in y2038/2016.
> Stop using those two functions to safer 64bits ones.
>
> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@linaro.org>
> CC: Alessandro Zummo <a.zummo@towertech.it>
> CC: Alexandre Belloni <alexandre.belloni@free-electrons.com>
> CC: rtc-linux@googlegroups.com
> CC: linux-kernel@vger.kernel.org
> ---
> drivers/rtc/rtc-pm8xxx.c | 22 ++++++++++++----------
> 1 file changed, 12 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/rtc/rtc-pm8xxx.c b/drivers/rtc/rtc-pm8xxx.c
> index fac8355..e6b52bd 100644
> --- a/drivers/rtc/rtc-pm8xxx.c
> +++ b/drivers/rtc/rtc-pm8xxx.c
> @@ -81,7 +81,8 @@ struct pm8xxx_rtc {
> static int pm8xxx_rtc_set_time(struct device *dev, struct rtc_time *tm)
> {
> int rc, i;
> - unsigned long secs, irq_flags;
> + unsigned long long secs;
> + unsigned long irq_flags;
'secs' should be 'time64_t' here, which is signed.
> u8 value[NUM_8_BIT_RTC_REGS], alarm_enabled = 0;
> unsigned int ctrl_reg;
> struct pm8xxx_rtc *rtc_dd = dev_get_drvdata(dev);
> @@ -90,14 +91,14 @@ static int pm8xxx_rtc_set_time(struct device *dev, struct rtc_time *tm)
> if (!rtc_dd->allow_set_time)
> return -EACCES;
>
> - rtc_tm_to_time(tm, &secs);
> + secs = rtc_tm_to_time64(tm);
>
> for (i = 0; i < NUM_8_BIT_RTC_REGS; i++) {
> value[i] = secs & 0xFF;
> secs >>= 8;
> }
>
> - dev_dbg(dev, "Seconds value to be written to RTC = %lu\n", secs);
> + dev_dbg(dev, "Seconds value to be written to RTC = %llu\n", secs);
>
However, note that you only write 32 bits here, since NUM_8_BIT_RTC_REGS
is set to '4'. Is that a hardware constant? If yes, it would be best to return
-EINVAL or -EOVERFLOW for out of range times.
I think should really try to come up with a way to set the policy for
overflows in
RTC drivers globally in this case. There are many drivers that take a 32-bit
unsigned value (and many others that don't), but a nicer policy for the long
run might be to define some kind of windowing where values before e.g.
year 2000 or 2017 are wrapped around and used as future dates.
Unfortunately, the downside of this is that any RTC that defaults to '0'
and is now interpreted as year 1970 would then be interpreted as a future
data that can not be represented in today's 32-bit time_t.
Arnd
--
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
---
You received this message because you are subscribed to the Google Groups "rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
^ permalink raw reply
* [rtc-linux] Re: [PATCH 1/2] RTC: s35390a: handle invalid RTC time
From: Alexandre Belloni @ 2017-06-29 7:56 UTC (permalink / raw)
To: Fabien Lahoudere; +Cc: a.zummo, rtc-linux
In-Reply-To: <1498470673.9477.3.camel@collabora.co.uk>
On 26/06/2017 at 11:51:13 +0200, Fabien Lahoudere wrote:
> > Actually, after reading the datasheet, I realize it is only POC that is
> > reset to 0 after reading so isinvalid is not needed. Just read status1
> > and look for BLD instead of caching it.
> >
>
> isinvalid is also used in s35390a_set_datetime. So if I remove it how can I detect that time setting
> failed?
>
If it fails, simply don't reset BLD so it is still set when reading the
time.
> > I think it is probably worth separating s35390a_reset() into two
> > functions. One that does the initialization and another one that reads
> > status1 and immediately doest the initialization when POC is set. If BLD
> > is set, then we can wait for set_time to happen before initializing.
> >
--
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
--
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
---
You received this message because you are subscribed to the Google Groups "rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
^ permalink raw reply
* Re: [rtc-linux] RTC used as a module
From: Michal Simek @ 2017-06-29 7:34 UTC (permalink / raw)
To: Michal Simek, John Stultz, monstr
Cc: Alexandre Belloni, rtc-linux@googlegroups.com
In-Reply-To: <cc3a8f33-1d68-2ad1-6aba-773f53c29d61@xilinx.com>
Hi John and Alexandre,
On 7.4.2017 09:12, Michal Simek wrote:
> On 16.3.2017 16:35, Michal Simek wrote:
>> Hi John and Alexandre,
>>
>> On 6.3.2017 15:56, Michal Simek wrote:
>>> On 3.3.2017 21:04, John Stultz wrote:
>>>> On Fri, Mar 3, 2017 at 6:46 AM, Michal Simek <monstr@monstr.eu> wrote:
>>>>> Hi Alexandre,
>>>>>
>>>>> On 3.3.2017 10:41, Michal Simek wrote:
>>>>>> Hi Alexandre,
>>>>>>
>>>>>> On 23.2.2017 13:14, Alexandre Belloni wrote:
>>>>>>> Hi Michal,
>>>>>>>
>>>>>>> I've been thinking about this issue (and meaning to actually test).
>>>>>>>
>>>>>>> On 08/12/2016 at 14:02:36 +0100, Michal Simek wrote:
>>>>>>>> Hi guys,
>>>>>>>>
>>>>>>>> I am trying to find out reason for this behavior. If rtc-zynqmp is used
>>>>>>>> as module for the first time it is correctly probed based on aliases
>>>>>>>> setting. (rtc5 for log below). But then driver is removed and add again
>>>>>>>> rtc5 is still not freed. rtc_device_release() is not called that's why
>>>>>>>> rtc->id can't be used again.
>>>>>>>>
>>>>>>>> sh-4.3# modprobe rtc-zynqmp
>>>>>>>> [ 42.468565] rtc_zynqmp ffa60000.rtc: rtc core: registered
>>>>>>>> ffa60000.rtc as rtc5
>>>>>>>> sh-4.3# rmmod rtc-zynqmp
>>>>>>>> sh-4.3# modprobe rtc-zynqmp
>>>>>>>> [ 48.648222] rtc_zynqmp ffa60000.rtc: /aliases ID 5 not available
>>>>>>>> [ 48.654280] rtc_zynqmp ffa60000.rtc: rtc core: registered
>>>>>>>> ffa60000.rtc as rtc0
>>>>>>>>
>>>>>>>>
>>>>>>>> I didn't try different rtc drivers but it looks like that someone keeps
>>>>>>>> reference that's why release function is not called.
>>>>>>>>
>>>>>>>> Can someone check this with different RTC module?
>>>>>>>
>>>>>>> I'll do that soon.
>>>>>>
>>>>>> Have you tried it?
>>>>>>
>>>>>>>
>>>>>>>> Or is this expected behavior?
>>>>>>>>
>>>>>>>
>>>>>>> I don't think so :)
>>>>>>>
>>>>>>> Can you clarify a few things:
>>>>>>> - is this the only rtc with a driver on your system ?
>>>>>>
>>>>>> yes only one.
>>>>>>
>>>>>>> - is it selected as the RTC_SYSTOHC_DEVICE or RTC_HCTOSYS_DEVICE. I've
>>>>>>> checked the code and I don't think this is the issue but it is worth
>>>>>>> testing.
>>>>>>
>>>>>> This is the setting.
>>>>>>
>>>>>> CONFIG_RTC_HCTOSYS=y
>>>>>> CONFIG_RTC_HCTOSYS_DEVICE="rtc0"
>>>>>> CONFIG_RTC_SYSTOHC=y
>>>>>> CONFIG_RTC_SYSTOHC_DEVICE="rtc0"
>>>>>>
>>>>>> What I see is that rtc_device_release is not called which is that
>>>>>> function which contain ida_simple_remove.
>>>>>>
>>>>>> I have alias for rtc0 (rtc0 = &rtc;)
>>>>>> And here is a log with debug. As you see when driver is removed for the
>>>>>> first time rtc_device_release is not called. When this is done for the
>>>>>> second time rtc_device_release is called properly and the same device
>>>>>> allocated via ida is used.
>>>>>>
>>>>>> root@linaro-alip:~# dmesg | grep rtc
>>>>>> [ 3.372715] [drm] Cannot find any crtc or sizes - going 1024x768
>>>>>> [ 3.422099] hctosys: unable to open rtc device (rtc0)
>>>>>> [ 6.578930] rtc_zynqmp ffa60000.rtc: rtc core: registered
>>>>>> ffa60000.rtc as rtc0
>>>>>> root@linaro-alip:~#
>>>>>> root@linaro-alip:~# lsmod
>>>>>> Module Size Used by
>>>>>> rtc_zynqmp 16384 0
>>>>>> uio_pdrv_genirq 16384 0
>>>>>> root@linaro-alip:~# rmmod rtc_zynqmp
>>>>>> [ 26.805172] rtc_core: devm_rtc_device_unregister
>>>>>> [ 26.809794] rtc_core: devm_rtc_device_release
>>>>>> [ 26.814129] rtc_core: rtc_device_unregister
>>>>>> root@linaro-alip:~# modprobe rtc_zynqmp
>>>>>> [ 35.879655] rtc_zynqmp ffa60000.rtc: /aliases ID 0 not available
>>>>>> [ 35.886002] rtc_zynqmp ffa60000.rtc: rtc core: registered
>>>>>> ffa60000.rtc as rtc1
>>>>>> root@linaro-alip:~# rmmod rtc_zynqmp
>>>>>> [ 122.140487] rtc_core: devm_rtc_device_unregister
>>>>>> [ 122.145110] rtc_core: devm_rtc_device_release
>>>>>> [ 122.149443] rtc_core: rtc_device_unregister
>>>>>> [ 122.153770] rtc_core: rtc_device_release
>>>>>> root@linaro-alip:~# modprobe rtc_zynqmp
>>>>>> [ 123.646002] rtc_zynqmp ffa60000.rtc: /aliases ID 0 not available
>>>>>> [ 123.652249] rtc_zynqmp ffa60000.rtc: rtc core: registered
>>>>>> ffa60000.rtc as rtc1
>>>>>> root@linaro-alip:~# rmmod rtc_zynqmp
>>>>>> [ 125.876188] rtc_core: devm_rtc_device_release
>>>>>> [ 125.880554] rtc_core: rtc_device_unregister
>>>>>> [ 125.885163] rtc_core: rtc_device_release
>>>>>> root@linaro-alip:~#
>>>>>>
>>>>>> I have also done experiment to setup rtc alias to rtc1 and this ID is
>>>>>> used only once. It looks like something is not released properly.
>>>>>>
>>>>>> This is out of tree driver but I can't see any specific code which could
>>>>>> improve behavior.
>>>>>> I have tested i2c rtc mcp79410 and there is not a visible problem but it
>>>>>> is not MMIO.
>>>>>
>>>>> This is the function which is making the difference.
>>>>>
>>>>> It was added by John Stultz:
>>>>> "timers: Introduce in-kernel alarm-timer interface"
>>>>> (sha1: ff3ead96d17f47ee70c294a5cc2cce9b61e82f0f)
>>>>>
>>>>> Probe function has device_init_wakeup(&pdev->dev, 1);
>>>>> That's why reference is taken (get_device() below)
>>>>>
>>>>> static int alarmtimer_rtc_add_device(struct device *dev,
>>>>> struct class_interface *class_intf)
>>>>> {
>>>>> unsigned long flags;
>>>>> struct rtc_device *rtc = to_rtc_device(dev);
>>>>>
>>>>> if (rtcdev)
>>>>> return -EBUSY;
>>>>>
>>>>> pr_info("%s\n", __func__);
>>>>>
>>>>> if (!rtc->ops->set_alarm)
>>>>> return -1;
>>>>>
>>>>> pr_info("%s\n", __func__);
>>>>>
>>>>> if (!device_may_wakeup(rtc->dev.parent))
>>>>> return -1;
>>>>>
>>>>> spin_lock_irqsave(&rtcdev_lock, flags);
>>>>> if (!rtcdev) {
>>>>> rtcdev = rtc;
>>>>> /* hold a reference so it doesn't go away */
>>>>> get_device(dev);
>>>>> }
>>>>> spin_unlock_irqrestore(&rtcdev_lock, flags);
>>>>> return 0;
>>>>> }
>>>>>
>>>>> And in remove function device_init_wakeup(&pdev->dev, 0);
>>>>> But because there is still a reference rtc_device_release() is not
>>>>> called and ida is not freed.
>>>>>
>>>>> Generic question which I think should be asked if in this situation
>>>>> driver should be modular or not.
>>>>> If driver can be modular then somewhere should be put_device().
>>>>
>>>> So off the top of my head (sorry, packing for a trip at the moment), I
>>>> think this was because the alarmtimer subsystem needs to hold onto the
>>>> rtcdev so it doesn't disappear under it, as it will be needed when
>>>> setting any alarmtimers on suspend.
>>>>
>>>> Part of the issue is the alarmtimer interface will provide an error to
>>>> userspace if there's no rtcdev, so we don't want a situation where
>>>> timers successfully set then silently never fire because the rtc was
>>>> removed.
>>>
>>> right I fully understand that.
>>>
>>>>
>>>> That said, there may very well be bugs in how it was handled, and I've
>>>> not looked closely at it recently.
>>>
>>> On the other hand you can have multiple alarms in the system and you can
>>> remove them at run time. It means I would expect that you can remove rtc
>>> if alarm is not set. When you run application driver is in use and you
>>> can't do it.
>>>
>>> Anyway by saying this I expect every driver which setup
>>> device_init_wakeup(..,1); and device_init_wakeup(..,0) in remove has the
>>> same issue.
>>> Solution is definitely not making rtc as a module which is in my case
>>> rtc-zynqmp an option but not the best one.
>>
>> I think it is good time to continue on this.
>
> Any update on this one?
It is quite some time from my last ping that's hwy I am sending new one.
If this is not going to be resolve I will send a patch which will simply
disable module functionality which is reasonable workaround for now.
Thanks,
Michal
--
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
---
You received this message because you are subscribed to the Google Groups "rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
^ permalink raw reply
* Re: [PATCH] rtc: brcmstb-waketimer: fix settime function
From: Florian Fainelli @ 2017-06-28 23:14 UTC (permalink / raw)
To: Arnd Bergmann, Alexandre Belloni
Cc: Alessandro Zummo, Brian Norris, Gregory Fong,
bcm-kernel-feedback-list, Markus Mayer, linux-rtc,
linux-arm-kernel, linux-kernel
In-Reply-To: <20170628200839.3114345-1-arnd@arndb.de>
On 06/28/2017 01:07 PM, Arnd Bergmann wrote:
> gcc warns about an unused variable in the new driver:
>
> drivers/rtc/rtc-brcmstb-waketimer.c: In function 'brcmstb_waketmr_settime':
> drivers/rtc/rtc-brcmstb-waketimer.c:142:6: error: unused variable 'ret' [-Werror=unused-variable]
>
> The same function also doesn't handle overflow correctly, this makes
> it return -EINVAL when passed a time that doesn't fit within the
> range of the register.
Yes, thanks for doing that.
>
> Fixes: 9f4ad359c801 ("rtc: brcmstb-waketimer: Add Broadcom STB wake-timer")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
--
Florian
^ permalink raw reply
* [PATCH] rtc: brcmstb-waketimer: fix settime function
From: Arnd Bergmann @ 2017-06-28 20:07 UTC (permalink / raw)
To: Alexandre Belloni
Cc: Arnd Bergmann, Alessandro Zummo, Brian Norris, Gregory Fong,
Florian Fainelli, bcm-kernel-feedback-list, Markus Mayer,
linux-rtc, linux-arm-kernel, linux-kernel
gcc warns about an unused variable in the new driver:
drivers/rtc/rtc-brcmstb-waketimer.c: In function 'brcmstb_waketmr_settime':
drivers/rtc/rtc-brcmstb-waketimer.c:142:6: error: unused variable 'ret' [-Werror=unused-variable]
The same function also doesn't handle overflow correctly, this makes
it return -EINVAL when passed a time that doesn't fit within the
range of the register.
Fixes: 9f4ad359c801 ("rtc: brcmstb-waketimer: Add Broadcom STB wake-timer")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
drivers/rtc/rtc-brcmstb-waketimer.c | 19 ++++++++++++-------
1 file changed, 12 insertions(+), 7 deletions(-)
diff --git a/drivers/rtc/rtc-brcmstb-waketimer.c b/drivers/rtc/rtc-brcmstb-waketimer.c
index 5dea6d0627d8..796ac792a381 100644
--- a/drivers/rtc/rtc-brcmstb-waketimer.c
+++ b/drivers/rtc/rtc-brcmstb-waketimer.c
@@ -138,10 +138,12 @@ static int brcmstb_waketmr_settime(struct device *dev,
struct rtc_time *tm)
{
struct brcmstb_waketmr *timer = dev_get_drvdata(dev);
- unsigned long sec;
- int ret;
+ time64_t sec;
+
+ sec = rtc_tm_to_time64(tm);
- rtc_tm_to_time(tm, &sec);
+ if (sec > U32_MAX || sec < 0)
+ return -EINVAL;
writel_relaxed(sec, timer->base + BRCMSTB_WKTMR_COUNTER);
@@ -152,14 +154,14 @@ static int brcmstb_waketmr_getalarm(struct device *dev,
struct rtc_wkalrm *alarm)
{
struct brcmstb_waketmr *timer = dev_get_drvdata(dev);
- unsigned long sec;
+ time64_t sec;
u32 reg;
sec = readl_relaxed(timer->base + BRCMSTB_WKTMR_ALARM);
if (sec != 0) {
/* Alarm is enabled */
alarm->enabled = 1;
- rtc_time_to_tm(sec, &alarm->time);
+ rtc_time64_to_tm(sec, &alarm->time);
}
reg = readl_relaxed(timer->base + BRCMSTB_WKTMR_EVENT);
@@ -172,13 +174,16 @@ static int brcmstb_waketmr_setalarm(struct device *dev,
struct rtc_wkalrm *alarm)
{
struct brcmstb_waketmr *timer = dev_get_drvdata(dev);
- unsigned long sec;
+ time64_t sec;
if (alarm->enabled)
- rtc_tm_to_time(&alarm->time, &sec);
+ sec = rtc_tm_to_time64(&alarm->time);
else
sec = 0;
+ if (sec > U32_MAX || sec < 0)
+ return -EINVAL;
+
brcmstb_waketmr_set_alarm(timer, sec);
return 0;
--
2.9.0
^ permalink raw reply related
* [rtc-linux] Re: [PATCH 1/2] dt-bindings: rtc: stm32: add support for STM32H7
From: Rob Herring @ 2017-06-28 18:04 UTC (permalink / raw)
To: Amelie Delaunay
Cc: Alessandro Zummo, Alexandre Belloni, Mark Rutland,
Maxime Coquelin, Alexandre Torgue, rtc-linux, devicetree,
linux-arm-kernel, linux-kernel
In-Reply-To: <1498470689-26829-2-git-send-email-amelie.delaunay@st.com>
On Mon, Jun 26, 2017 at 11:51:28AM +0200, Amelie Delaunay wrote:
> This patch documents support for STM32H7 Real Time Clock.
> It introduces a new compatible and rework clock definitions.
> On STM32H7 we have a 'pclk' clock for register access, in addition to
> the 'rtc_ck' clock.
>
> Signed-off-by: Amelie Delaunay <amelie.delaunay@st.com>
> ---
> .../devicetree/bindings/rtc/st,stm32-rtc.txt | 32 ++++++++++++++++++----
> 1 file changed, 27 insertions(+), 5 deletions(-)
Acked-by: Rob Herring <robh@kernel.org>
--
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
---
You received this message because you are subscribed to the Google Groups "rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
^ permalink raw reply
* [PATCH v2 01/10] Documentation: Add device tree binding for Goldfish RTC driver
From: Aleksandar Markovic @ 2017-06-28 15:46 UTC (permalink / raw)
To: linux-mips
Cc: Aleksandar Markovic, Miodrag Dinic, Goran Ferenc,
Alessandro Zummo, Alexandre Belloni, David S. Miller, devicetree,
Douglas Leung, Greg Kroah-Hartman, James Hogan, linux-kernel,
linux-rtc, Mark Rutland, Martin K. Petersen,
Mauro Carvalho Chehab, Paul Burton, Petar Jovanovic,
Raghu Gandham, Rob Herring
In-Reply-To: <1498664922-28493-1-git-send-email-aleksandar.markovic@rt-rk.com>
From: Aleksandar Markovic <aleksandar.markovic@imgtec.com>
Add documentation for DT binding of Goldfish RTC driver. The compatible
string used by OS for binding the driver is "google,goldfish-rtc".
Signed-off-by: Miodrag Dinic <miodrag.dinic@imgtec.com>
Signed-off-by: Goran Ferenc <goran.ferenc@imgtec.com>
Signed-off-by: Aleksandar Markovic <aleksandar.markovic@imgtec.com>
---
.../devicetree/bindings/rtc/google,goldfish-rtc.txt | 17 +++++++++++++++++
MAINTAINERS | 5 +++++
2 files changed, 22 insertions(+)
create mode 100644 Documentation/devicetree/bindings/rtc/google,goldfish-rtc.txt
diff --git a/Documentation/devicetree/bindings/rtc/google,goldfish-rtc.txt b/Documentation/devicetree/bindings/rtc/google,goldfish-rtc.txt
new file mode 100644
index 0000000..634312d
--- /dev/null
+++ b/Documentation/devicetree/bindings/rtc/google,goldfish-rtc.txt
@@ -0,0 +1,17 @@
+Android Goldfish RTC
+
+Android Goldfish RTC device used by Android emulator.
+
+Required properties:
+
+- compatible : should contain "google,goldfish-rtc"
+- reg : <registers mapping>
+- interrupts : <interrupt mapping>
+
+Example:
+
+ goldfish_timer@9020000 {
+ compatible = "google,goldfish-rtc";
+ reg = <0x9020000 0x1000>;
+ interrupts = <0x3>;
+ };
diff --git a/MAINTAINERS b/MAINTAINERS
index 09b5ab6..519cdef 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -841,6 +841,11 @@ S: Supported
F: drivers/android/
F: drivers/staging/android/
+ANDROID GOLDFISH RTC DRIVER
+M: Miodrag Dinic <miodrag.dinic@imgtec.com>
+S: Supported
+F: Documentation/devicetree/bindings/rtc/google,goldfish-rtc.txt
+
ANDROID ION DRIVER
M: Laura Abbott <labbott@redhat.com>
M: Sumit Semwal <sumit.semwal@linaro.org>
--
2.7.4
^ permalink raw reply related
* [PATCH v2 02/10] MIPS: ranchu: Add Goldfish RTC driver
From: Aleksandar Markovic @ 2017-06-28 15:46 UTC (permalink / raw)
To: linux-mips
Cc: Aleksandar Markovic, Miodrag Dinic, Goran Ferenc,
Alessandro Zummo, Alexandre Belloni, David S. Miller,
Douglas Leung, Greg Kroah-Hartman, James Hogan, linux-kernel,
linux-rtc, Martin K. Petersen, Mauro Carvalho Chehab, Paul Burton,
Petar Jovanovic, Raghu Gandham
In-Reply-To: <1498664922-28493-1-git-send-email-aleksandar.markovic@rt-rk.com>
From: Aleksandar Markovic <aleksandar.markovic@imgtec.com>
Add device driver for a virtual Goldfish RTC clock.
The driver can be built only if CONFIG_MIPS and CONFIG_GOLDFISH are
set. The compatible string used by OS for binding the driver is
defined as "google,goldfish-rtc".
Signed-off-by: Miodrag Dinic <miodrag.dinic@imgtec.com>
Signed-off-by: Goran Ferenc <goran.ferenc@imgtec.com>
Signed-off-by: Aleksandar Markovic <aleksandar.markovic@imgtec.com>
---
MAINTAINERS | 1 +
drivers/rtc/Kconfig | 6 ++
drivers/rtc/Makefile | 1 +
drivers/rtc/rtc-goldfish.c | 136 +++++++++++++++++++++++++++++++++++++++++++++
4 files changed, 144 insertions(+)
create mode 100644 drivers/rtc/rtc-goldfish.c
diff --git a/MAINTAINERS b/MAINTAINERS
index 519cdef..26a1267 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -845,6 +845,7 @@ ANDROID GOLDFISH RTC DRIVER
M: Miodrag Dinic <miodrag.dinic@imgtec.com>
S: Supported
F: Documentation/devicetree/bindings/rtc/google,goldfish-rtc.txt
+F: drivers/rtc/rtc-goldfish.c
ANDROID ION DRIVER
M: Laura Abbott <labbott@redhat.com>
diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
index 8d3b957..510c5b7 100644
--- a/drivers/rtc/Kconfig
+++ b/drivers/rtc/Kconfig
@@ -1753,5 +1753,11 @@ config RTC_DRV_HID_SENSOR_TIME
If this driver is compiled as a module, it will be named
rtc-hid-sensor-time.
+config RTC_DRV_GOLDFISH
+ tristate "Goldfish Real Time Clock"
+ depends on MIPS
+ depends on GOLDFISH
+ help
+ Say yes here to build support for the Goldfish RTC.
endif # RTC_CLASS
diff --git a/drivers/rtc/Makefile b/drivers/rtc/Makefile
index 13857d2..dfc38f5 100644
--- a/drivers/rtc/Makefile
+++ b/drivers/rtc/Makefile
@@ -168,3 +168,4 @@ obj-$(CONFIG_RTC_DRV_WM8350) += rtc-wm8350.o
obj-$(CONFIG_RTC_DRV_X1205) += rtc-x1205.o
obj-$(CONFIG_RTC_DRV_XGENE) += rtc-xgene.o
obj-$(CONFIG_RTC_DRV_ZYNQMP) += rtc-zynqmp.o
+obj-$(CONFIG_RTC_DRV_GOLDFISH) += rtc-goldfish.o
diff --git a/drivers/rtc/rtc-goldfish.c b/drivers/rtc/rtc-goldfish.c
new file mode 100644
index 0000000..e206a98
--- /dev/null
+++ b/drivers/rtc/rtc-goldfish.c
@@ -0,0 +1,136 @@
+/* drivers/rtc/rtc-goldfish.c
+ *
+ * Copyright (C) 2007 Google, Inc.
+ * Copyright (C) 2017 Imagination Technologies Ltd.
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/rtc.h>
+
+#define TIMER_TIME_LOW 0x00 /* get low bits of current time */
+ /* and update TIMER_TIME_HIGH */
+#define TIMER_TIME_HIGH 0x04 /* get high bits of time at last */
+ /* TIMER_TIME_LOW read */
+#define TIMER_ALARM_LOW 0x08 /* set low bits of alarm and */
+ /* activate it */
+#define TIMER_ALARM_HIGH 0x0c /* set high bits of next alarm */
+#define TIMER_CLEAR_INTERRUPT 0x10
+#define TIMER_CLEAR_ALARM 0x14
+
+struct goldfish_rtc {
+ void __iomem *base;
+ u32 irq;
+ struct rtc_device *rtc;
+};
+
+static irqreturn_t goldfish_rtc_interrupt(int irq, void *dev_id)
+{
+ struct goldfish_rtc *qrtc = dev_id;
+ unsigned long events = 0;
+ void __iomem *base = qrtc->base;
+
+ writel(1, base + TIMER_CLEAR_INTERRUPT);
+ events = RTC_IRQF | RTC_AF;
+
+ rtc_update_irq(qrtc->rtc, 1, events);
+
+ return IRQ_HANDLED;
+}
+
+static int goldfish_rtc_read_time(struct device *dev, struct rtc_time *tm)
+{
+ u64 time;
+ u64 time_low;
+ u64 time_high;
+ u64 time_high_prev;
+
+ struct goldfish_rtc *qrtc =
+ platform_get_drvdata(to_platform_device(dev));
+ void __iomem *base = qrtc->base;
+
+ time_high = readl(base + TIMER_TIME_HIGH);
+ do {
+ time_high_prev = time_high;
+ time_low = readl(base + TIMER_TIME_LOW);
+ time_high = readl(base + TIMER_TIME_HIGH);
+ } while (time_high != time_high_prev);
+ time = (time_high << 32) | time_low;
+
+ do_div(time, NSEC_PER_SEC);
+
+ rtc_time_to_tm(time, tm);
+
+ return 0;
+}
+
+static const struct rtc_class_ops goldfish_rtc_ops = {
+ .read_time = goldfish_rtc_read_time,
+};
+
+static int goldfish_rtc_probe(struct platform_device *pdev)
+{
+ struct resource *r;
+ struct goldfish_rtc *qrtc;
+ unsigned long rtc_dev_len;
+ unsigned long rtc_dev_addr;
+ int err;
+
+ qrtc = devm_kzalloc(&pdev->dev, sizeof(*qrtc), GFP_KERNEL);
+ if (qrtc == NULL)
+ return -ENOMEM;
+
+ platform_set_drvdata(pdev, qrtc);
+
+ r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ if (r == NULL)
+ return -ENODEV;
+
+ rtc_dev_addr = r->start;
+ rtc_dev_len = resource_size(r);
+ qrtc->base = devm_ioremap(&pdev->dev, rtc_dev_addr, rtc_dev_len);
+ if (IS_ERR(qrtc->base))
+ return -ENODEV;
+
+ qrtc->irq = platform_get_irq(pdev, 0);
+ if (qrtc->irq < 0)
+ return -ENODEV;
+
+ qrtc->rtc = devm_rtc_device_register(&pdev->dev, pdev->name,
+ &goldfish_rtc_ops, THIS_MODULE);
+ if (IS_ERR(qrtc->rtc))
+ return PTR_ERR(qrtc->rtc);
+
+ err = devm_request_irq(&pdev->dev, qrtc->irq, goldfish_rtc_interrupt,
+ 0, pdev->name, qrtc);
+ if (err)
+ return err;
+
+ return 0;
+}
+
+static const struct of_device_id goldfish_rtc_of_match[] = {
+ { .compatible = "google,goldfish-rtc", },
+ {},
+};
+MODULE_DEVICE_TABLE(of, goldfish_rtc_of_match);
+
+static struct platform_driver goldfish_rtc = {
+ .probe = goldfish_rtc_probe,
+ .driver = {
+ .name = "goldfish_rtc",
+ .of_match_table = goldfish_rtc_of_match,
+ }
+};
+
+module_platform_driver(goldfish_rtc);
--
2.7.4
^ permalink raw reply related
* [PATCH v3] rtc: ds3232: add temperature support
From: Kirill Esipov @ 2017-06-28 11:29 UTC (permalink / raw)
To: a.zummo, alexandre.belloni; +Cc: linux-rtc, linux-kernel, Kirill Esipov
DS3232/DS3234 has the temperature registers with a resolution of 0.25
degree celsius. This enables to get the value through hwmon.
# cat /sys/class/hwmon/hwmon0/temp1_input
37250
Signed-off-by: Kirill Esipov <yesipov@gmail.com>
---
drivers/rtc/Kconfig | 8 ++++
drivers/rtc/rtc-ds3232.c | 119 +++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 127 insertions(+)
diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
index 8d3b95728326..ba87c419a2d4 100644
--- a/drivers/rtc/Kconfig
+++ b/drivers/rtc/Kconfig
@@ -791,6 +791,14 @@ config RTC_DRV_DS3232
This driver can also be built as a module. If so, the module
will be called rtc-ds3232.
+config RTC_DRV_DS3232_HWMON
+ bool "HWMON support for Dallas/Maxim DS3232/DS3234"
+ depends on RTC_DRV_DS3232 && HWMON && !(RTC_DRV_DS3232=y && HWMON=m)
+ default y
+ help
+ Say Y here if you want to expose temperature sensor data on
+ rtc-ds3232
+
config RTC_DRV_PCF2127
tristate "NXP PCF2127"
depends on RTC_I2C_AND_SPI
diff --git a/drivers/rtc/rtc-ds3232.c b/drivers/rtc/rtc-ds3232.c
index deff431a37c4..0550f7ba464f 100644
--- a/drivers/rtc/rtc-ds3232.c
+++ b/drivers/rtc/rtc-ds3232.c
@@ -22,6 +22,7 @@
#include <linux/bcd.h>
#include <linux/slab.h>
#include <linux/regmap.h>
+#include <linux/hwmon.h>
#define DS3232_REG_SECONDS 0x00
#define DS3232_REG_MINUTES 0x01
@@ -46,6 +47,8 @@
# define DS3232_REG_SR_A2F 0x02
# define DS3232_REG_SR_A1F 0x01
+#define DS3232_REG_TEMPERATURE 0x11
+
struct ds3232 {
struct device *dev;
struct regmap *regmap;
@@ -275,6 +278,120 @@ static int ds3232_update_alarm(struct device *dev, unsigned int enabled)
return ret;
}
+/*
+ * Temperature sensor support for ds3232/ds3234 devices.
+ * A user-initiated temperature conversion is not started by this function,
+ * so the temperature is updated once every 64 seconds.
+ */
+static int ds3232_hwmon_read_temp(struct device *dev, long int *mC)
+{
+ struct ds3232 *ds3232 = dev_get_drvdata(dev);
+ u8 temp_buf[2];
+ s16 temp;
+ int ret;
+
+ ret = regmap_bulk_read(ds3232->regmap, DS3232_REG_TEMPERATURE, temp_buf,
+ sizeof(temp_buf));
+ if (ret < 0)
+ return ret;
+
+ /*
+ * Temperature is represented as a 10-bit code with a resolution of
+ * 0.25 degree celsius and encoded in two's complement format.
+ */
+ temp = (temp_buf[0] << 8) | temp_buf[1];
+ temp >>= 6;
+ *mC = temp * 250;
+
+ return 0;
+}
+
+static umode_t ds3232_hwmon_is_visible(const void *data,
+ enum hwmon_sensor_types type,
+ u32 attr, int channel)
+{
+ if (type != hwmon_temp)
+ return 0;
+
+ switch (attr) {
+ case hwmon_temp_input:
+ return 0444;
+ default:
+ return 0;
+ }
+}
+
+static int ds3232_hwmon_read(struct device *dev,
+ enum hwmon_sensor_types type,
+ u32 attr, int channel, long *temp)
+{
+ int err;
+
+ switch (attr) {
+ case hwmon_temp_input:
+ err = ds3232_hwmon_read_temp(dev, temp);
+ break;
+ default:
+ err = -EOPNOTSUPP;
+ break;
+ }
+
+ return err;
+}
+
+static u32 ds3232_hwmon_chip_config[] = {
+ HWMON_C_REGISTER_TZ,
+ 0
+};
+
+static const struct hwmon_channel_info ds3232_hwmon_chip = {
+ .type = hwmon_chip,
+ .config = ds3232_hwmon_chip_config,
+};
+
+static u32 ds3232_hwmon_temp_config[] = {
+ HWMON_T_INPUT,
+ 0
+};
+
+static const struct hwmon_channel_info ds3232_hwmon_temp = {
+ .type = hwmon_temp,
+ .config = ds3232_hwmon_temp_config,
+};
+
+static const struct hwmon_channel_info *ds3232_hwmon_info[] = {
+ &ds3232_hwmon_chip,
+ &ds3232_hwmon_temp,
+ NULL
+};
+
+static const struct hwmon_ops ds3232_hwmon_hwmon_ops = {
+ .is_visible = ds3232_hwmon_is_visible,
+ .read = ds3232_hwmon_read,
+};
+
+static const struct hwmon_chip_info ds3232_hwmon_chip_info = {
+ .ops = &ds3232_hwmon_hwmon_ops,
+ .info = ds3232_hwmon_info,
+};
+
+static void ds3232_hwmon_register(struct device *dev, const char *name)
+{
+ struct ds3232 *ds3232 = dev_get_drvdata(dev);
+ struct device *hwmon_dev;
+
+ if (!IS_ENABLED(CONFIG_RTC_DRV_DS3232_HWMON))
+ return;
+
+ hwmon_dev = devm_hwmon_device_register_with_info(dev, name, ds3232,
+ &ds3232_hwmon_chip_info,
+ NULL);
+ if (IS_ERR(hwmon_dev)) {
+ dev_err(dev, "unable to register hwmon device %ld\n",
+ PTR_ERR(hwmon_dev));
+ }
+}
+
static int ds3232_alarm_irq_enable(struct device *dev, unsigned int enabled)
{
struct ds3232 *ds3232 = dev_get_drvdata(dev);
@@ -366,6 +483,8 @@ static int ds3232_probe(struct device *dev, struct regmap *regmap, int irq,
if (ds3232->irq > 0)
device_init_wakeup(dev, 1);
+ ds3232_hwmon_register(dev, name);
+
ds3232->rtc = devm_rtc_device_register(dev, name, &ds3232_rtc_ops,
THIS_MODULE);
if (IS_ERR(ds3232->rtc))
--
2.7.4
^ permalink raw reply related
* Re: [PATCH v2] rtc: ds3232: add temperature support
From: Alexandre Belloni @ 2017-06-27 18:01 UTC (permalink / raw)
To: Kirill Esipov
Cc: Andy Shevchenko, Alessandro Zummo, linux-rtc,
linux-kernel@vger.kernel.org
In-Reply-To: <CAJcpCzEHW8GSkykidpq=GBo-aFbFstmOmQZhAYu-NWBWxTc=uA@mail.gmail.com>
On 27/06/2017 at 18:27:42 +0300, Kirill Esipov wrote:
> 2017-06-27 16:00 GMT+03:00 Alexandre Belloni
> <alexandre.belloni@free-electrons.com>:
> > On 27/06/2017 at 15:24:57 +0300, Kirill Esipov wrote:
> >> 2017-06-25 19:39 GMT+03:00 Andy Shevchenko <andy.shevchenko@gmail.com>:
> >> > On Thu, Jun 22, 2017 at 7:58 PM, Kirill Esipov <yesipov@gmail.com> wrote:
> >> >> DS3232/DS3234 has the temperature registers with a resolution of 0.25
> >> >> degree celsius. This enables to get the value through hwmon.
> >> >>
> >> >> # cat /sys/class/hwmon/hwmon0/temp1_input
> >> >> 37250
> >> >
> >> >> +config RTC_DRV_DS3232_HWMON
> >> >> + bool "HWMON support for Dallas/Maxim DS3232/DS3234"
> >> >
> >> >> + depends on RTC_DRV_DS3232 && HWMON
> >> >> + depends on !(RTC_DRV_DS3232=y && HWMON=m)
> >> >
> >> > Perhaps it might be squeezed into one line (something like that logic
> >> > has been required by I2C related PMIC IIRC)
> >> >
> >> >> + default y
> >> >
> >> > Is it really sane default?
> >> >
> >>
> >> At first sight i thought that yes it is sane default (and others RTC with
> >> hwmon set it "default y" (ds1307, rv3029c2)).
> >> But if it's not sane, then we should turn it off by default in others drivers?
> >>
> >
> > It is definitively sane.
> >
> >>
> >> >> +#ifdef CONFIG_RTC_DRV_DS3232_HWMON
> >> >
> >> > IS_BUILTIN() ?
> >> >
> >
> > I'd use IS_ENABLED in that case.
> >
>
> Why? "RTC_DRV_DS3232_HWMON" is bool, not tristate. So it can't be
> defined as "m".
>
It's clearer and doesn't hurt but really, #ifdef
CONFIG_RTC_DRV_DS3232_HWMON is just fine.
> >> >> +static int ds3232_hwmon_read_temp(struct device *dev, long int *mC)
> >> >> +{
> >> >> + struct ds3232 *ds3232 = dev_get_drvdata(dev);
> >> >> + u8 temp_buf[2];
> >> >> + s16 temp;
> >> >> + int ret;
> >> >> +
> >> >> + ret = regmap_bulk_read(ds3232->regmap, DS3232_REG_TEMPERATURE, temp_buf,
> >> >> + sizeof(temp_buf));
> >> >
> >> >> +
> >> >
> >> > Remove.
> >
> > I'd recommend running checkpatch.pl --strict to remove the remaining
> > whitespace issues too (a few alignments are off).
> >
> >> >
> >> > I dunno which style is preferred, though you may use
> >> > if (IS_BUILTIN(...))
> >> > return;
> >> >
> >> > at the beginning of the function and allow gcc optimizer to take care
> >> > of everything else.
> >> >
> >
> > I don't have a strong opinion there.
> >
> > --
> > Alexandre Belloni, Free Electrons
> > Embedded Linux and Kernel engineering
> > http://free-electrons.com
>
>
>
> --
> Kirill Esipov
--
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply
* Re: [PATCH v2] rtc: ds3232: add temperature support
From: Kirill Esipov @ 2017-06-27 15:27 UTC (permalink / raw)
To: Alexandre Belloni
Cc: Andy Shevchenko, Alessandro Zummo, linux-rtc,
linux-kernel@vger.kernel.org
In-Reply-To: <20170627130057.yhhalp2epmmdicz7@piout.net>
2017-06-27 16:00 GMT+03:00 Alexandre Belloni
<alexandre.belloni@free-electrons.com>:
> On 27/06/2017 at 15:24:57 +0300, Kirill Esipov wrote:
>> 2017-06-25 19:39 GMT+03:00 Andy Shevchenko <andy.shevchenko@gmail.com>:
>> > On Thu, Jun 22, 2017 at 7:58 PM, Kirill Esipov <yesipov@gmail.com> wrote:
>> >> DS3232/DS3234 has the temperature registers with a resolution of 0.25
>> >> degree celsius. This enables to get the value through hwmon.
>> >>
>> >> # cat /sys/class/hwmon/hwmon0/temp1_input
>> >> 37250
>> >
>> >> +config RTC_DRV_DS3232_HWMON
>> >> + bool "HWMON support for Dallas/Maxim DS3232/DS3234"
>> >
>> >> + depends on RTC_DRV_DS3232 && HWMON
>> >> + depends on !(RTC_DRV_DS3232=y && HWMON=m)
>> >
>> > Perhaps it might be squeezed into one line (something like that logic
>> > has been required by I2C related PMIC IIRC)
>> >
>> >> + default y
>> >
>> > Is it really sane default?
>> >
>>
>> At first sight i thought that yes it is sane default (and others RTC with
>> hwmon set it "default y" (ds1307, rv3029c2)).
>> But if it's not sane, then we should turn it off by default in others drivers?
>>
>
> It is definitively sane.
>
>>
>> >> +#ifdef CONFIG_RTC_DRV_DS3232_HWMON
>> >
>> > IS_BUILTIN() ?
>> >
>
> I'd use IS_ENABLED in that case.
>
Why? "RTC_DRV_DS3232_HWMON" is bool, not tristate. So it can't be
defined as "m".
>> >> +static int ds3232_hwmon_read_temp(struct device *dev, long int *mC)
>> >> +{
>> >> + struct ds3232 *ds3232 = dev_get_drvdata(dev);
>> >> + u8 temp_buf[2];
>> >> + s16 temp;
>> >> + int ret;
>> >> +
>> >> + ret = regmap_bulk_read(ds3232->regmap, DS3232_REG_TEMPERATURE, temp_buf,
>> >> + sizeof(temp_buf));
>> >
>> >> +
>> >
>> > Remove.
>
> I'd recommend running checkpatch.pl --strict to remove the remaining
> whitespace issues too (a few alignments are off).
>
>> >
>> > I dunno which style is preferred, though you may use
>> > if (IS_BUILTIN(...))
>> > return;
>> >
>> > at the beginning of the function and allow gcc optimizer to take care
>> > of everything else.
>> >
>
> I don't have a strong opinion there.
>
> --
> Alexandre Belloni, Free Electrons
> Embedded Linux and Kernel engineering
> http://free-electrons.com
--
Kirill Esipov
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox