* [rtc-linux] Re: [PATCH v2 0/3] rtc: make st-lpc robust against y2038/2106 bug
From: Shuah Khan @ 2017-06-23 19:40 UTC (permalink / raw)
To: Benjamin Gaignard, john.stultz, tglx, sboyd, linux-kernel,
linux-kselftest, patrice.chotard, a.zummo, alexandre.belloni,
linux-arm-kernel, rtc-linux
Cc: linaro-kernel, Shuah Khan
In-Reply-To: <1497864982-29284-1-git-send-email-benjamin.gaignard@linaro.org>
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.
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.
thanks,
-- Shah
--
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: Alexandre Belloni @ 2017-06-23 22:09 UTC (permalink / raw)
To: Shuah Khan
Cc: Benjamin Gaignard, john.stultz, tglx, sboyd, linux-kernel,
linux-kselftest, patrice.chotard, a.zummo, linux-arm-kernel,
rtc-linux, linaro-kernel
In-Reply-To: <47b45b70-91da-ce41-535d-89d92f380274@kernel.org>
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 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 v2 0/3] rtc: make st-lpc robust against y2038/2106 bug
From: Shuah Khan @ 2017-06-23 22:34 UTC (permalink / raw)
To: Alexandre Belloni
Cc: Benjamin Gaignard, john.stultz, tglx, sboyd, linux-kernel,
linux-kselftest, patrice.chotard, a.zummo, linux-arm-kernel,
rtc-linux, linaro-kernel, Shuah Khan
In-Reply-To: <20170623220907.kxhkwkl6gt7bho5r@piout.net>
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.
>
>
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] rtc: opal: Implement rtc_class_ops.alarm_irq_enable callback
From: Alexandre Belloni @ 2017-06-24 5:48 UTC (permalink / raw)
To: Vaibhav Jain
Cc: Alessandro Zummo, rtc-linux, linux-kernel, Benjamin Herrenschmidt,
Michael Ellerman, Paul Mackerras
In-Reply-To: <20170531130901.31415-1-vaibhav@linux.vnet.ibm.com>
On 31/05/2017 at 18:39:01 +0530, Vaibhav Jain wrote:
> Provide an implementation of the callback
> rtc_class_ops.alarm_irq_enable for rtc-opal driver. This callback is
> called when the wake alarm is disabled via the command:
>
> 'echo 0 > /sys/class/rtc/rtc0/wakealarm'
>
> Without this the Timed-Power-On(TPO) config remains set even when its
> disabled by the above command and FSP will still force machine
> boot at previously configured alarm time.
>
> The callback is implemented as function opal_tpo_alarm_irq_enable()
> which calls opal_set_tpo_time() with alarm.enabled == 0. A branch is
> added to opal_set_tpo_time() to handle this case by passing y_m_d ==
> h_m_s_ms == 0 to opal as arguments for opal_tpo_write() call.
>
> Signed-off-by: Vaibhav Jain <vaibhav@linux.vnet.ibm.com>
> ---
> drivers/rtc/rtc-opal.c | 22 +++++++++++++++++++++-
> 1 file changed, 21 insertions(+), 1 deletion(-)
>
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] drivers: rtc-mxc: remove unused variable
From: Alexandre Belloni @ 2017-06-24 8:52 UTC (permalink / raw)
To: Jose Diaz de Grenu de Pedro; +Cc: a.zummo, rtc-linux, trivial
In-Reply-To: <1494517119-23622-1-git-send-email-Jose.DiazdeGrenu@digi.com>
On 11/05/2017 at 17:38:39 +0200, Jose Diaz de Grenu de Pedro wrote:
> From: "Diaz de Grenu, Jose" <Jose.DiazdeGrenu@digi.com>
>=20
> This variable was never used. With GCC 6.2, we get the following warning:
>=20
> drivers/rtc/rtc-mxc.c:44:18: warning: =E2=80=98PIE_BIT_DEF=E2=80=99 defin=
ed but not used [-Wunused-const-variable=3D]
> static const u32 PIE_BIT_DEF[MAX_PIE_NUM][2] =3D {
>=20
> Signed-off-by: Diaz de Grenu, Jose <Jose.DiazdeGrenu@digi.com>
> ---
> drivers/rtc/rtc-mxc.c | 11 -----------
> 1 file changed, 11 deletions(-)
>=20
Applied, thanks.
--=20
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
--=20
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.
---=20
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 e=
mail to rtc-linux+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
^ permalink raw reply
* [rtc-linux] Re: [PATCH 2/2] rtc: brcmstb-waketimer: Add Broadcom STB wake-timer
From: Alexandre Belloni @ 2017-06-24 18:59 UTC (permalink / raw)
To: Florian Fainelli
Cc: linux-kernel, Brian Norris, Markus Mayer, Alessandro Zummo,
Rob Herring, Mark Rutland, Gregory Fong,
maintainer:BROADCOM BCM7XXX ARM ARCHITECTURE,
open list:REAL TIME CLOCK (RTC) SUBSYSTEM,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
moderated list:BROADCOM BCM7XXX ARM ARCHITECTURE
In-Reply-To: <20170615195904.12653-3-f.fainelli@gmail.com>
Hi,
This seems mostly good.
On 15/06/2017 at 12:59:04 -0700, Florian Fainelli wrote:
> +static void wktmr_read(struct brcmstb_waketmr *timer,
> + struct wktmr_time *t)
> +{
> + u32 tmp;
> +
To be sure, is this IP always 32bit, even on 64bit platforms?
> + do {
> + t->sec = readl_relaxed(timer->base + BRCMSTB_WKTMR_COUNTER);
> + tmp = readl_relaxed(timer->base + BRCMSTB_WKTMR_PRESCALER_VAL);
> + } while (tmp >= timer->rate);
> +
> + t->pre = timer->rate - tmp;
> +}
> +
[...]
> +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;
> +
> + ret = rtc_valid_tm(tm);
> + if (ret)
> + return ret;
> +
There is no way this function can be called without a valid tm. The only
caller checks before calling.
> + rtc_tm_to_time(tm, &sec);
> +
> + dev_dbg(dev, "%s: sec=%ld\n", __func__, sec);
> + writel_relaxed(sec, timer->base + BRCMSTB_WKTMR_COUNTER);
> +
> + return 0;
> +}
> +
> +static int brcmstb_waketmr_getalarm(struct device *dev,
> + struct rtc_wkalrm *alarm)
> +{
> + struct brcmstb_waketmr *timer = dev_get_drvdata(dev);
> + unsigned long sec;
> + u32 reg;
> +
> + sec = readl_relaxed(timer->base + BRCMSTB_WKTMR_ALARM);
> + if (sec == 0) {
> + /* Alarm is disabled */
> + alarm->enabled = 0;
> + alarm->time.tm_mon = -1;
> + alarm->time.tm_mday = -1;
> + alarm->time.tm_year = -1;
> + alarm->time.tm_hour = -1;
> + alarm->time.tm_min = -1;
> + alarm->time.tm_sec = -1;
This is not needed since d68778b80dd7
> + dev_dbg(dev, "%s: alarm is disabled\n", __func__);
> + } else {
> + /* Alarm is enabled */
> + alarm->enabled = 1;
> + rtc_time_to_tm(sec, &alarm->time);
> + dev_dbg(dev, "%s: alarm is enabled\n", __func__);
> + }
> +
> + reg = readl_relaxed(timer->base + BRCMSTB_WKTMR_EVENT);
> + alarm->pending = !!(reg & 1);
> + dev_dbg(dev, "%s: alarm pending=%d\n", __func__, alarm->pending);
> +
> + return 0;
> +}
--
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 6/6] rtc: s3c: Handle clock enable failures
From: Alexandre Belloni @ 2017-06-24 20:55 UTC (permalink / raw)
To: Krzysztof Kozlowski; +Cc: Alessandro Zummo, linux-rtc, linux-kernel
In-Reply-To: <20170616192807.10347-6-krzk@kernel.org>
Hi,
On 16/06/2017 at 21:28:07 +0200, Krzysztof Kozlowski wrote:
> clk_enable() can fail so handle such case.
>
> Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
> ---
> drivers/rtc/rtc-s3c.c | 72 ++++++++++++++++++++++++++++++++++++++++-----------
> 1 file changed, 57 insertions(+), 15 deletions(-)
>
I've applied the whole series. However, quite often, on a platform,
clk_prepare/enable are ensured to never fail and I find that the added
complexity to handle failures is not worth it.
> diff --git a/drivers/rtc/rtc-s3c.c b/drivers/rtc/rtc-s3c.c
> index 0cb2f27a30b4..a8992c227f61 100644
> --- a/drivers/rtc/rtc-s3c.c
> +++ b/drivers/rtc/rtc-s3c.c
> @@ -68,18 +68,32 @@ struct s3c_rtc_data {
> void (*disable) (struct s3c_rtc *info);
> };
>
> -static void s3c_rtc_enable_clk(struct s3c_rtc *info)
> +static int s3c_rtc_enable_clk(struct s3c_rtc *info)
> {
> unsigned long irq_flags;
> + int ret = 0;
>
> spin_lock_irqsave(&info->alarm_clk_lock, irq_flags);
> +
> if (info->clk_disabled) {
> - clk_enable(info->rtc_clk);
> - if (info->data->needs_src_clk)
> - clk_enable(info->rtc_src_clk);
> + ret = clk_enable(info->rtc_clk);
> + if (ret)
> + goto out;
> +
> + if (info->data->needs_src_clk) {
> + ret = clk_enable(info->rtc_src_clk);
> + if (ret) {
> + clk_disable(info->rtc_clk);
> + goto out;
> + }
> + }
> info->clk_disabled = false;
> }
> +
> +out:
> spin_unlock_irqrestore(&info->alarm_clk_lock, irq_flags);
> +
> + return ret;
> }
>
> static void s3c_rtc_disable_clk(struct s3c_rtc *info)
> @@ -122,10 +136,13 @@ static int s3c_rtc_setaie(struct device *dev, unsigned int enabled)
> {
> struct s3c_rtc *info = dev_get_drvdata(dev);
> unsigned int tmp;
> + int ret;
>
> dev_dbg(info->dev, "%s: aie=%d\n", __func__, enabled);
>
> - s3c_rtc_enable_clk(info);
> + ret = s3c_rtc_enable_clk(info);
> + if (ret)
> + return ret;
>
> tmp = readb(info->base + S3C2410_RTCALM) & ~S3C2410_RTCALM_ALMEN;
>
> @@ -136,10 +153,13 @@ static int s3c_rtc_setaie(struct device *dev, unsigned int enabled)
>
> s3c_rtc_disable_clk(info);
>
> - if (enabled)
> - s3c_rtc_enable_clk(info);
> - else
> + if (enabled) {
> + ret = s3c_rtc_enable_clk(info);
> + if (ret)
> + return ret;
> + } else {
> s3c_rtc_disable_clk(info);
> + }
>
> return 0;
> }
> @@ -147,10 +167,14 @@ static int s3c_rtc_setaie(struct device *dev, unsigned int enabled)
> /* Set RTC frequency */
> static int s3c_rtc_setfreq(struct s3c_rtc *info, int freq)
> {
> + int ret;
> +
> if (!is_power_of_2(freq))
> return -EINVAL;
>
> - s3c_rtc_enable_clk(info);
> + ret = s3c_rtc_enable_clk(info);
> + if (ret)
> + return ret;
> spin_lock_irq(&info->pie_lock);
>
> if (info->data->set_freq)
> @@ -167,8 +191,11 @@ static int s3c_rtc_gettime(struct device *dev, struct rtc_time *rtc_tm)
> {
> struct s3c_rtc *info = dev_get_drvdata(dev);
> unsigned int have_retried = 0;
> + int ret;
>
> - s3c_rtc_enable_clk(info);
> + ret = s3c_rtc_enable_clk(info);
> + if (ret)
> + return ret;
>
> retry_get_time:
> rtc_tm->tm_min = readb(info->base + S3C2410_RTCMIN);
> @@ -212,6 +239,7 @@ static int s3c_rtc_settime(struct device *dev, struct rtc_time *tm)
> {
> struct s3c_rtc *info = dev_get_drvdata(dev);
> int year = tm->tm_year - 100;
> + int ret;
>
> dev_dbg(dev, "set time %04d.%02d.%02d %02d:%02d:%02d\n",
> 1900 + tm->tm_year, tm->tm_mon, tm->tm_mday,
> @@ -224,7 +252,9 @@ static int s3c_rtc_settime(struct device *dev, struct rtc_time *tm)
> return -EINVAL;
> }
>
> - s3c_rtc_enable_clk(info);
> + ret = s3c_rtc_enable_clk(info);
> + if (ret)
> + return ret;
>
> writeb(bin2bcd(tm->tm_sec), info->base + S3C2410_RTCSEC);
> writeb(bin2bcd(tm->tm_min), info->base + S3C2410_RTCMIN);
> @@ -243,8 +273,11 @@ static int s3c_rtc_getalarm(struct device *dev, struct rtc_wkalrm *alrm)
> struct s3c_rtc *info = dev_get_drvdata(dev);
> struct rtc_time *alm_tm = &alrm->time;
> unsigned int alm_en;
> + int ret;
>
> - s3c_rtc_enable_clk(info);
> + ret = s3c_rtc_enable_clk(info);
> + if (ret)
> + return ret;
>
> alm_tm->tm_sec = readb(info->base + S3C2410_ALMSEC);
> alm_tm->tm_min = readb(info->base + S3C2410_ALMMIN);
> @@ -293,6 +326,7 @@ static int s3c_rtc_setalarm(struct device *dev, struct rtc_wkalrm *alrm)
> struct s3c_rtc *info = dev_get_drvdata(dev);
> struct rtc_time *tm = &alrm->time;
> unsigned int alrm_en;
> + int ret;
> int year = tm->tm_year - 100;
>
> dev_dbg(dev, "s3c_rtc_setalarm: %d, %04d.%02d.%02d %02d:%02d:%02d\n",
> @@ -300,7 +334,9 @@ static int s3c_rtc_setalarm(struct device *dev, struct rtc_wkalrm *alrm)
> 1900 + tm->tm_year, tm->tm_mon + 1, tm->tm_mday,
> tm->tm_hour, tm->tm_min, tm->tm_sec);
>
> - s3c_rtc_enable_clk(info);
> + ret = s3c_rtc_enable_clk(info);
> + if (ret)
> + return ret;
>
> alrm_en = readb(info->base + S3C2410_RTCALM) & S3C2410_RTCALM_ALMEN;
> writeb(0x00, info->base + S3C2410_RTCALM);
> @@ -349,8 +385,11 @@ static int s3c_rtc_setalarm(struct device *dev, struct rtc_wkalrm *alrm)
> static int s3c_rtc_proc(struct device *dev, struct seq_file *seq)
> {
> struct s3c_rtc *info = dev_get_drvdata(dev);
> + int ret;
>
> - s3c_rtc_enable_clk(info);
> + ret = s3c_rtc_enable_clk(info);
> + if (ret)
> + return ret;
>
> if (info->data->enable_tick)
> info->data->enable_tick(info, seq);
> @@ -589,8 +628,11 @@ static int s3c_rtc_probe(struct platform_device *pdev)
> static int s3c_rtc_suspend(struct device *dev)
> {
> struct s3c_rtc *info = dev_get_drvdata(dev);
> + int ret;
>
> - s3c_rtc_enable_clk(info);
> + ret = s3c_rtc_enable_clk(info);
> + if (ret)
> + return ret;
>
> /* save TICNT for anyone using periodic interrupts */
> if (info->data->save_tick_cnt)
> --
> 2.9.3
>
--
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply
* [rtc-linux] Re: [PATCH V2] rtc: ds1307: Add support for Epson RX8130CE
From: Alexandre Belloni @ 2017-06-24 21:02 UTC (permalink / raw)
To: Marek Vasut
Cc: rtc-linux, Arnd Bergmann, Felipe Balbi, Nishanth Menon,
Tony Lindgren
In-Reply-To: <20170618205528.10088-1-marex@denx.de>
On 18/06/2017 at 22:55:28 +0200, Marek Vasut wrote:
> Add support for yet another RTC chip, Epson RX8130CE. This time around,
> the chip has slightly permutated registers and also the register starts
> at 0x10 instead of 0x0 .
>
> So far, we only support the RTC and NVRAM parts of the chip, Alarm and
> Timer is not supported.
>
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Alexandre Belloni <alexandre.belloni@free-electrons.com>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Felipe Balbi <balbi@ti.com>
> Cc: Nishanth Menon <nm@ti.com>
> Cc: Tony Lindgren <tony@atomide.com>
> ---
> V2: Convert to regmap
> ---
> drivers/rtc/rtc-ds1307.c | 175 +++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 175 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 6/6] rtc: s3c: Handle clock enable failures
From: Krzysztof Kozlowski @ 2017-06-25 7:16 UTC (permalink / raw)
To: Alexandre Belloni; +Cc: Alessandro Zummo, linux-rtc, linux-kernel
In-Reply-To: <20170624205532.eqykr37ynpvprklf@piout.net>
On Sat, Jun 24, 2017 at 10:55 PM, Alexandre Belloni
<alexandre.belloni@free-electrons.com> wrote:
> Hi,
>
> On 16/06/2017 at 21:28:07 +0200, Krzysztof Kozlowski wrote:
>> clk_enable() can fail so handle such case.
>>
>> Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
>> ---
>> drivers/rtc/rtc-s3c.c | 72 ++++++++++++++++++++++++++++++++++++++++-----------
>> 1 file changed, 57 insertions(+), 15 deletions(-)
>>
>
> I've applied the whole series. However, quite often, on a platform,
> clk_prepare/enable are ensured to never fail and I find that the added
> complexity to handle failures is not worth it.
Indeed, the platform clocks usually cannot fail (although not always,
e.g. some PLLs might return ETIMEDOUT on sync timeout) but in this
case one of the clocks is an I2C-controlled clock from PMIC. Its
clk_prepare() can fail. It does not implement clk_enable() but this
might change in the future so to avoid unexpected issues, it is better
just to handle this properly.
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH v2] rtc: ds3232: add temperature support
From: Andy Shevchenko @ 2017-06-25 16:39 UTC (permalink / raw)
To: Kirill Esipov
Cc: Alessandro Zummo, Alexandre Belloni, linux-rtc,
linux-kernel@vger.kernel.org
In-Reply-To: <1498150704-21777-1-git-send-email-yesipov@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?
> +#ifdef CONFIG_RTC_DRV_DS3232_HWMON
IS_BUILTIN() ?
> +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.
> + if (ret < 0)
> + return ret;
> +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:
> + ds3232_hwmon_read_temp(dev, temp);
> + err = 0;
> + break;
> + default:
> + return -EOPNOTSUPP;
> + }
> +
> + return err;
You may do as in previous function. Or what did you mean here by
introducing an err variable?
> +}
> +
> +
Remove one of them.
> +static void ds3232_hwmon_register(struct device *dev, const char *name)
> +{
> + struct ds3232 *ds3232 = dev_get_drvdata(dev);
> + struct device *hwmon_dev;
> +
> + hwmon_dev = devm_hwmon_device_register_with_info(dev, name, ds3232,
> + &ds3232_hwmon_chip_info,
> + NULL);
> +
Remove.
> + if (IS_ERR(hwmon_dev)) {
> + dev_warn(dev, "unable to register hwmon device %ld\n",
> + PTR_ERR(hwmon_dev));
> + }
> +}
> +
> +#else
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.
> +
> +static void ds3232_hwmon_register(struct device *dev, const char *name)
> +{
> +}
> +
> +#endif
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* [rtc-linux] Re: [PATCH 1/2] RTC: s35390a: handle invalid RTC time
From: Fabien Lahoudere @ 2017-06-26 9:51 UTC (permalink / raw)
To: Alexandre Belloni; +Cc: a.zummo, rtc-linux
In-Reply-To: <20170117110016.ids6xheti242lxhe@piout.net>
On Tue, 2017-01-17 at 12:00 +0100, Alexandre Belloni wrote:
> On 17/01/2017 at 09:24:17 +0100, Fabien Lahoudere wrote :
> > On Mon, 2017-01-16 at 18:50 +0100, Alexandre Belloni wrote:
> > > Hi,
> > >=20
> > > On 12/01/2017 at 11:43:37 +0100, Fabien Lahoudere wrote :
> > > > 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 lo=
st.
> > > >=20
> > > > Signed-off-by: Fabien Lahoudere <fabien.lahoudere@collabora.co.uk>
> > > > ---
> > > > =C2=A0drivers/rtc/rtc-s35390a.c | 9 +++++++++
> > > > =C2=A01 file changed, 9 insertions(+)
> > > >=20
> > > > diff --git a/drivers/rtc/rtc-s35390a.c b/drivers/rtc/rtc-s35390a.c
> > > > index 5dab466..ef4ada9 100644
> > > > --- a/drivers/rtc/rtc-s35390a.c
> > > > +++ b/drivers/rtc/rtc-s35390a.c
> > > > @@ -62,6 +62,7 @@ struct s35390a {
> > > > =C2=A0 struct i2c_client *client[8];
> > > > =C2=A0 struct rtc_device *rtc;
> > > > =C2=A0 int twentyfourhour;
> > > > + int isinvalid;
> > > > =C2=A0};
> > > > =C2=A0
> > > > =C2=A0static int s35390a_set_reg(struct s35390a *s35390a, int reg, =
char *buf, int len)
> > > > @@ -135,6 +136,8 @@ static int s35390a_reset(struct s35390a *s35390=
a, char *status1)
> > > > =C2=A0 =C2=A0* The 24H bit is kept over reset, so set it already he=
re.
> > > > =C2=A0 =C2=A0*/
> > > > =C2=A0initialize:
> > > > + /* set the RTC time as invalid */
> > > > + s35390a->isinvalid =3D 1;
> > > > =C2=A0 *status1 =3D S35390A_FLAG_24H;
> > > > =C2=A0 buf =3D S35390A_FLAG_RESET | S35390A_FLAG_24H;
> > > > =C2=A0 ret =3D s35390a_set_reg(s35390a, S35390A_CMD_STATUS1, &buf, =
1);
> > > > @@ -221,6 +224,8 @@ static int s35390a_set_datetime(struct i2c_clie=
nt *client, struct
> > > > rtc_time *tm)
> > > > =C2=A0 buf[i] =3D bitrev8(buf[i]);
> > > > =C2=A0
> > > > =C2=A0 err =3D s35390a_set_reg(s35390a, S35390A_CMD_TIME1, buf, siz=
eof(buf));
> > > > + if (err >=3D 0)
> > > > + s35390a->isinvalid =3D 0;
> > > > =C2=A0
> > > > =C2=A0 return err;
> > > > =C2=A0}
> > > > @@ -231,6 +236,9 @@ static int s35390a_get_datetime(struct i2c_clie=
nt *client, struct
> > > > rtc_time *tm)
> > > > =C2=A0 char buf[7];
> > > > =C2=A0 int i, err;
> > > > =C2=A0
> > > > + if (s35390a->isinvalid)
> > > > + return -EINVAL;
> > > > +
> > >=20
> > > That's fine but what happens if it became invalid between probe and
> > > s35390a_get_datetime()? (This is particularly relevant after patch 2/=
2.
> >=20
> > This is not possible with our design. When the system is on, its power
> > supply replace the backup battery. (See p38 Figure 49).
> >=20
>=20
> Well, it depends on the tolerances of each components (but yeah, I doubt
> your SoC is more tolerant than the RTC).
>=20
>=20
> > If you prefer I can call s35390a_reset if s35390a->isinvalid is set to
> > 0?
> >=20
>=20
> 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.
>=20
isinvalid is also used in s35390a_set_datetime. So if I remove it how can I=
detect that time setting
failed?
> 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.
>=20
--=20
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.
---=20
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 e=
mail to rtc-linux+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
^ permalink raw reply
* [rtc-linux] [PATCH 0/2] rtc: add support for STM32H7 RTC
From: Amelie Delaunay @ 2017-06-26 9:51 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,
Amelie Delaunay
Add support for STM32H7 Real Time Clock variant.
Difference is regarding clocks: there is a 'pclk' clock on STM32H7
for register access.
Amelie Delaunay (2):
dt-bindings: rtc: stm32: add support for STM32H7
rtc: stm32: add STM32H7 RTC support
.../devicetree/bindings/rtc/st,stm32-rtc.txt | 32 ++++++--
drivers/rtc/rtc-stm32.c | 85 +++++++++++++++++-----
2 files changed, 95 insertions(+), 22 deletions(-)
--
1.9.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] [PATCH 2/2] rtc: stm32: add STM32H7 RTC support
From: Amelie Delaunay @ 2017-06-26 9:51 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,
Amelie Delaunay
In-Reply-To: <1498470689-26829-1-git-send-email-amelie.delaunay@st.com>
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>
---
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;
--
1.9.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 1/2] dt-bindings: rtc: stm32: add support for STM32H7
From: Amelie Delaunay @ 2017-06-26 9:51 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,
Amelie Delaunay
In-Reply-To: <1498470689-26829-1-git-send-email-amelie.delaunay@st.com>
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(-)
diff --git a/Documentation/devicetree/bindings/rtc/st,stm32-rtc.txt b/Documentation/devicetree/bindings/rtc/st,stm32-rtc.txt
index e2837b9..0a4c371 100644
--- a/Documentation/devicetree/bindings/rtc/st,stm32-rtc.txt
+++ b/Documentation/devicetree/bindings/rtc/st,stm32-rtc.txt
@@ -1,17 +1,25 @@
STM32 Real Time Clock
Required properties:
-- compatible: "st,stm32-rtc".
+- compatible: can be either "st,stm32-rtc" or "st,stm32h7-rtc", depending on
+ the device is compatible with stm32(f4/f7) or stm32h7.
- reg: address range of rtc register set.
-- clocks: reference to the clock entry ck_rtc.
+- clocks: can use up to two clocks, depending on part used:
+ - "rtc_ck": RTC clock source.
+ It is required on stm32(f4/f7) and stm32h7.
+ - "pclk": RTC APB interface clock.
+ It is not present on stm32(f4/f7).
+ It is required on stm32h7.
+- clock-names: must be "rtc_ck" and "pclk".
+ It is required only on stm32h7.
- interrupt-parent: phandle for the interrupt controller.
- interrupts: rtc alarm interrupt.
- st,syscfg: phandle for pwrcfg, mandatory to disable/enable backup domain
(RTC registers) write protection.
-Optional properties (to override default ck_rtc parent clock):
-- assigned-clocks: reference to the ck_rtc clock entry.
-- assigned-clock-parents: phandle of the new parent clock of ck_rtc.
+Optional properties (to override default rtc_ck parent clock):
+- assigned-clocks: reference to the rtc_ck clock entry.
+- assigned-clock-parents: phandle of the new parent clock of rtc_ck.
Example:
@@ -25,3 +33,17 @@ Example:
interrupts = <17 1>;
st,syscfg = <&pwrcfg>;
};
+
+ rtc: rtc@58004000 {
+ compatible = "st,stm32h7-rtc";
+ reg = <0x58004000 0x400>;
+ clocks = <&rcc RTCAPB_CK>, <&rcc RTC_CK>;
+ clock-names = "pclk", "rtc_ck";
+ assigned-clocks = <&rcc RTC_CK>;
+ assigned-clock-parents = <&rcc LSE_CK>;
+ interrupt-parent = <&exti>;
+ interrupts = <17 1>;
+ interrupt-names = "alarm";
+ st,syscfg = <&pwrcfg>;
+ status = "disabled";
+ };
--
1.9.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] Re: [PATCH 2/2] rtc: brcmstb-waketimer: Add Broadcom STB wake-timer
From: Florian Fainelli @ 2017-06-26 17:15 UTC (permalink / raw)
To: Alexandre Belloni
Cc: linux-kernel, Brian Norris, Markus Mayer, Alessandro Zummo,
Rob Herring, Mark Rutland, Gregory Fong,
maintainer:BROADCOM BCM7XXX ARM ARCHITECTURE,
open list:REAL TIME CLOCK (RTC) SUBSYSTEM,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
moderated list:BROADCOM BCM7XXX ARM ARCHITECTURE
In-Reply-To: <20170624185942.vq656pjxbqnqdhll@piout.net>
On 06/24/2017 11:59 AM, Alexandre Belloni wrote:
> Hi,
>
> This seems mostly good.
>
> On 15/06/2017 at 12:59:04 -0700, Florian Fainelli wrote:
>> +static void wktmr_read(struct brcmstb_waketmr *timer,
>> + struct wktmr_time *t)
>> +{
>> + u32 tmp;
>> +
>
> To be sure, is this IP always 32bit, even on 64bit platforms?
Correct, it's only 32-bit capable (saw the recent discussions about the
2038yr "problem"...).
>
>> + do {
>> + t->sec = readl_relaxed(timer->base + BRCMSTB_WKTMR_COUNTER);
>> + tmp = readl_relaxed(timer->base + BRCMSTB_WKTMR_PRESCALER_VAL);
>> + } while (tmp >= timer->rate);
>> +
>> + t->pre = timer->rate - tmp;
>> +}
>> +
>
> [...]
>
>> +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;
>> +
>> + ret = rtc_valid_tm(tm);
>> + if (ret)
>> + return ret;
>> +
>
> There is no way this function can be called without a valid tm. The only
> caller checks before calling.
OK.
>
>> + rtc_tm_to_time(tm, &sec);
>> +
>> + dev_dbg(dev, "%s: sec=%ld\n", __func__, sec);
>> + writel_relaxed(sec, timer->base + BRCMSTB_WKTMR_COUNTER);
>> +
>> + return 0;
>> +}
>> +
>> +static int brcmstb_waketmr_getalarm(struct device *dev,
>> + struct rtc_wkalrm *alarm)
>> +{
>> + struct brcmstb_waketmr *timer = dev_get_drvdata(dev);
>> + unsigned long sec;
>> + u32 reg;
>> +
>> + sec = readl_relaxed(timer->base + BRCMSTB_WKTMR_ALARM);
>> + if (sec == 0) {
>> + /* Alarm is disabled */
>> + alarm->enabled = 0;
>> + alarm->time.tm_mon = -1;
>> + alarm->time.tm_mday = -1;
>> + alarm->time.tm_year = -1;
>> + alarm->time.tm_hour = -1;
>> + alarm->time.tm_min = -1;
>> + alarm->time.tm_sec = -1;
>
> This is not needed since d68778b80dd7
Great, I will take that out.
Do you care whether some dev_dbg() prints are left in the driver or
should I remove those in v2?
>
>> + dev_dbg(dev, "%s: alarm is disabled\n", __func__);
>> + } else {
>> + /* Alarm is enabled */
>> + alarm->enabled = 1;
>> + rtc_time_to_tm(sec, &alarm->time);
>> + dev_dbg(dev, "%s: alarm is enabled\n", __func__);
>> + }
>> +
>> + reg = readl_relaxed(timer->base + BRCMSTB_WKTMR_EVENT);
>> + alarm->pending = !!(reg & 1);
>> + dev_dbg(dev, "%s: alarm pending=%d\n", __func__, alarm->pending);
>> +
>> + return 0;
>> +}
>
--
Florian
--
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 1/2] dt-bindings: Document the Broadcom STB wake-up timer node
From: Florian Fainelli @ 2017-06-26 21:15 UTC (permalink / raw)
To: linux-arm-kernel
Cc: Florian Fainelli, Alessandro Zummo, Alexandre Belloni,
Rob Herring, Mark Rutland, Brian Norris, Gregory Fong,
maintainer:BROADCOM BCM7XXX ARM ARCHITECTURE,
open list:REAL TIME CLOCK (RTC) SUBSYSTEM,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
open list
In-Reply-To: <20170626211503.7471-1-f.fainelli@gmail.com>
Document the binding for the Broadcom STB SoCs wake-up timer node
allowing the system to generate alarms and exit low power states.
Acked-by: Rob Herring <robh@kernel.org>
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
.../bindings/rtc/brcm,brcmstb-waketimer.txt | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
create mode 100644 Documentation/devicetree/bindings/rtc/brcm,brcmstb-waketimer.txt
diff --git a/Documentation/devicetree/bindings/rtc/brcm,brcmstb-waketimer.txt b/Documentation/devicetree/bindings/rtc/brcm,brcmstb-waketimer.txt
new file mode 100644
index 000000000000..1d990bcc0baf
--- /dev/null
+++ b/Documentation/devicetree/bindings/rtc/brcm,brcmstb-waketimer.txt
@@ -0,0 +1,22 @@
+Broadcom STB wake-up Timer
+
+The Broadcom STB wake-up timer provides a 27Mhz resolution timer, with the
+ability to wake up the system from low-power suspend/standby modes.
+
+Required properties:
+- compatible : should contain "brcm,brcmstb-waketimer"
+- reg : the register start and length for the WKTMR block
+- interrupts : The TIMER interrupt
+- interrupt-parent: The phandle to the Always-On (AON) Power Management (PM) L2
+ interrupt controller node
+- clocks : The phandle to the UPG fixed clock (27Mhz domain)
+
+Example:
+
+waketimer@f0411580 {
+ compatible = "brcm,brcmstb-waketimer";
+ reg = <0xf0411580 0x14>;
+ interrupts = <0x3>;
+ interrupt-parent = <&aon_pm_l2_intc>;
+ clocks = <&upg_fixed>;
+};
--
2.9.3
^ permalink raw reply related
* [rtc-linux] [PATCH v2 0/2] Broadcom STB wake-timer support
From: Florian Fainelli @ 2017-06-26 21:15 UTC (permalink / raw)
To: linux-arm-kernel
Cc: Florian Fainelli, Alessandro Zummo, Alexandre Belloni,
Rob Herring, Mark Rutland, Brian Norris, Gregory Fong,
maintainer:BROADCOM BCM7XXX ARM ARCHITECTURE,
open list:REAL TIME CLOCK (RTC) SUBSYSTEM,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
open list
Hi Alexandre,
This patch series adds support for the Broadcom STB wake-timer. This is a 27Mhz
timer which allows the system to exit S2/S3/S5 sleep states when configured.
The wake-timer has has a long history within our internal/downstream tree, and
Brian authored it in the first place, then Markus added the rtc bits, and then I
cleaned it up for upstream.
Changes in v2:
- added Rob's Acked-by tag to the device tree binding patch
- removed dev_dbg() prints
- removed unnecessary initialization in .read_alarm callback
- rebase against latest rtc-next
Thanks!
Brian Norris (1):
rtc: brcmstb-waketimer: Add Broadcom STB wake-timer
Florian Fainelli (1):
dt-bindings: Document the Broadcom STB wake-up timer node
.../bindings/rtc/brcm,brcmstb-waketimer.txt | 22 ++
drivers/rtc/Kconfig | 11 +
drivers/rtc/Makefile | 1 +
drivers/rtc/rtc-brcmstb-waketimer.c | 345 +++++++++++++++++++++
4 files changed, 379 insertions(+)
create mode 100644 Documentation/devicetree/bindings/rtc/brcm,brcmstb-waketimer.txt
create mode 100644 drivers/rtc/rtc-brcmstb-waketimer.c
--
2.9.3
--
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: brcmstb-waketimer: Add Broadcom STB wake-timer
From: Florian Fainelli @ 2017-06-26 21:15 UTC (permalink / raw)
To: linux-arm-kernel
Cc: Brian Norris, Markus Mayer, Florian Fainelli, Alessandro Zummo,
Alexandre Belloni, Rob Herring, Mark Rutland, Gregory Fong,
maintainer:BROADCOM BCM7XXX ARM ARCHITECTURE,
open list:REAL TIME CLOCK (RTC) SUBSYSTEM,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
open list
In-Reply-To: <20170626211503.7471-1-f.fainelli@gmail.com>
From: Brian Norris <computersforpeace@gmail.com>
This adds support for the Broadcom STB wake-timer which is a timer in
the chip's 27Mhz clock domain that offers the ability to wake the system
(wake-up source) from suspend states (S2, S3, S5). It is supported using
the rtc framework allowing us to configure alarms for system wake-up.
Signed-off-by: Brian Norris <computersforpeace@gmail.com>
Signed-off-by: Markus Mayer <mmayer@broadcom.com>
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/rtc/Kconfig | 11 ++
drivers/rtc/Makefile | 1 +
drivers/rtc/rtc-brcmstb-waketimer.c | 325 ++++++++++++++++++++++++++++++++=
++++
3 files changed, 337 insertions(+)
create mode 100644 drivers/rtc/rtc-brcmstb-waketimer.c
diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
index 8d3b95728326..8e1aa67fe533 100644
--- a/drivers/rtc/Kconfig
+++ b/drivers/rtc/Kconfig
@@ -197,6 +197,17 @@ config RTC_DRV_AC100
This driver can also be built as a module. If so, the module
will be called rtc-ac100.
=20
+config RTC_DRV_BRCMSTB
+ tristate "Broadcom STB wake-timer"
+ depends on ARCH_BRCMSTB || BMIPS_GENERIC || COMPILE_TEST
+ default ARCH_BRCMSTB || BMIPS_GENERIC
+ help
+ If you say yes here you get support for the wake-timer found on
+ Broadcom STB SoCs (BCM7xxx).
+
+ This driver can also be built as a module. If so, the module will
+ be called rtc-brcmstb-waketimer.
+
config RTC_DRV_AS3722
tristate "ams AS3722 RTC driver"
depends on MFD_AS3722
diff --git a/drivers/rtc/Makefile b/drivers/rtc/Makefile
index 13857d2fce09..df89cac1f9ae 100644
--- a/drivers/rtc/Makefile
+++ b/drivers/rtc/Makefile
@@ -36,6 +36,7 @@ obj-$(CONFIG_RTC_DRV_AT91RM9200)+=3D rtc-at91rm9200.o
obj-$(CONFIG_RTC_DRV_AT91SAM9) +=3D rtc-at91sam9.o
obj-$(CONFIG_RTC_DRV_AU1XXX) +=3D rtc-au1xxx.o
obj-$(CONFIG_RTC_DRV_BFIN) +=3D rtc-bfin.o
+obj-$(CONFIG_RTC_DRV_BRCMSTB) +=3D rtc-brcmstb-waketimer.o
obj-$(CONFIG_RTC_DRV_BQ32K) +=3D rtc-bq32k.o
obj-$(CONFIG_RTC_DRV_BQ4802) +=3D rtc-bq4802.o
obj-$(CONFIG_RTC_DRV_CMOS) +=3D rtc-cmos.o
diff --git a/drivers/rtc/rtc-brcmstb-waketimer.c b/drivers/rtc/rtc-brcmstb-=
waketimer.c
new file mode 100644
index 000000000000..5dea6d0627d8
--- /dev/null
+++ b/drivers/rtc/rtc-brcmstb-waketimer.c
@@ -0,0 +1,325 @@
+/*
+ * Copyright =C2=A9 2014-2017 Broadcom
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * 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.
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/clk.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/init.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/irqreturn.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/pm.h>
+#include <linux/pm_wakeup.h>
+#include <linux/reboot.h>
+#include <linux/rtc.h>
+#include <linux/stat.h>
+#include <linux/suspend.h>
+
+struct brcmstb_waketmr {
+ struct rtc_device *rtc;
+ struct device *dev;
+ void __iomem *base;
+ int irq;
+ struct notifier_block reboot_notifier;
+ struct clk *clk;
+ u32 rate;
+};
+
+#define BRCMSTB_WKTMR_EVENT 0x00
+#define BRCMSTB_WKTMR_COUNTER 0x04
+#define BRCMSTB_WKTMR_ALARM 0x08
+#define BRCMSTB_WKTMR_PRESCALER 0x0C
+#define BRCMSTB_WKTMR_PRESCALER_VAL 0x10
+
+#define BRCMSTB_WKTMR_DEFAULT_FREQ 27000000
+
+static inline void brcmstb_waketmr_clear_alarm(struct brcmstb_waketmr *tim=
er)
+{
+ writel_relaxed(1, timer->base + BRCMSTB_WKTMR_EVENT);
+ (void)readl_relaxed(timer->base + BRCMSTB_WKTMR_EVENT);
+}
+
+static void brcmstb_waketmr_set_alarm(struct brcmstb_waketmr *timer,
+ unsigned int secs)
+{
+ brcmstb_waketmr_clear_alarm(timer);
+
+ writel_relaxed(secs + 1, timer->base + BRCMSTB_WKTMR_ALARM);
+}
+
+static irqreturn_t brcmstb_waketmr_irq(int irq, void *data)
+{
+ struct brcmstb_waketmr *timer =3D data;
+
+ pm_wakeup_event(timer->dev, 0);
+
+ return IRQ_HANDLED;
+}
+
+struct wktmr_time {
+ u32 sec;
+ u32 pre;
+};
+
+static void wktmr_read(struct brcmstb_waketmr *timer,
+ struct wktmr_time *t)
+{
+ u32 tmp;
+
+ do {
+ t->sec =3D readl_relaxed(timer->base + BRCMSTB_WKTMR_COUNTER);
+ tmp =3D readl_relaxed(timer->base + BRCMSTB_WKTMR_PRESCALER_VAL);
+ } while (tmp >=3D timer->rate);
+
+ t->pre =3D timer->rate - tmp;
+}
+
+static int brcmstb_waketmr_prepare_suspend(struct brcmstb_waketmr *timer)
+{
+ struct device *dev =3D timer->dev;
+ int ret =3D 0;
+
+ if (device_may_wakeup(dev)) {
+ ret =3D enable_irq_wake(timer->irq);
+ if (ret) {
+ dev_err(dev, "failed to enable wake-up interrupt\n");
+ return ret;
+ }
+ }
+
+ return ret;
+}
+
+/* If enabled as a wakeup-source, arm the timer when powering off */
+static int brcmstb_waketmr_reboot(struct notifier_block *nb,
+ unsigned long action, void *data)
+{
+ struct brcmstb_waketmr *timer;
+
+ timer =3D container_of(nb, struct brcmstb_waketmr, reboot_notifier);
+
+ /* Set timer for cold boot */
+ if (action =3D=3D SYS_POWER_OFF)
+ brcmstb_waketmr_prepare_suspend(timer);
+
+ return NOTIFY_DONE;
+}
+
+static int brcmstb_waketmr_gettime(struct device *dev,
+ struct rtc_time *tm)
+{
+ struct brcmstb_waketmr *timer =3D dev_get_drvdata(dev);
+ struct wktmr_time now;
+
+ wktmr_read(timer, &now);
+
+ rtc_time_to_tm(now.sec, tm);
+
+ return 0;
+}
+
+static int brcmstb_waketmr_settime(struct device *dev,
+ struct rtc_time *tm)
+{
+ struct brcmstb_waketmr *timer =3D dev_get_drvdata(dev);
+ unsigned long sec;
+ int ret;
+
+ rtc_tm_to_time(tm, &sec);
+
+ writel_relaxed(sec, timer->base + BRCMSTB_WKTMR_COUNTER);
+
+ return 0;
+}
+
+static int brcmstb_waketmr_getalarm(struct device *dev,
+ struct rtc_wkalrm *alarm)
+{
+ struct brcmstb_waketmr *timer =3D dev_get_drvdata(dev);
+ unsigned long sec;
+ u32 reg;
+
+ sec =3D readl_relaxed(timer->base + BRCMSTB_WKTMR_ALARM);
+ if (sec !=3D 0) {
+ /* Alarm is enabled */
+ alarm->enabled =3D 1;
+ rtc_time_to_tm(sec, &alarm->time);
+ }
+
+ reg =3D readl_relaxed(timer->base + BRCMSTB_WKTMR_EVENT);
+ alarm->pending =3D !!(reg & 1);
+
+ return 0;
+}
+
+static int brcmstb_waketmr_setalarm(struct device *dev,
+ struct rtc_wkalrm *alarm)
+{
+ struct brcmstb_waketmr *timer =3D dev_get_drvdata(dev);
+ unsigned long sec;
+
+ if (alarm->enabled)
+ rtc_tm_to_time(&alarm->time, &sec);
+ else
+ sec =3D 0;
+
+ brcmstb_waketmr_set_alarm(timer, sec);
+
+ return 0;
+}
+
+/*
+ * Does not do much but keep the RTC class happy. We always support
+ * alarms.
+ */
+static int brcmstb_waketmr_alarm_enable(struct device *dev,
+ unsigned int enabled)
+{
+ return 0;
+}
+
+static const struct rtc_class_ops brcmstb_waketmr_ops =3D {
+ .read_time =3D brcmstb_waketmr_gettime,
+ .set_time =3D brcmstb_waketmr_settime,
+ .read_alarm =3D brcmstb_waketmr_getalarm,
+ .set_alarm =3D brcmstb_waketmr_setalarm,
+ .alarm_irq_enable =3D brcmstb_waketmr_alarm_enable,
+};
+
+static int brcmstb_waketmr_probe(struct platform_device *pdev)
+{
+ struct device *dev =3D &pdev->dev;
+ struct brcmstb_waketmr *timer;
+ struct resource *res;
+ int ret;
+
+ timer =3D devm_kzalloc(dev, sizeof(*timer), GFP_KERNEL);
+ if (!timer)
+ return -ENOMEM;
+
+ platform_set_drvdata(pdev, timer);
+ timer->dev =3D dev;
+
+ res =3D platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ timer->base =3D devm_ioremap_resource(dev, res);
+ if (IS_ERR(timer->base))
+ return PTR_ERR(timer->base);
+
+ /*
+ * Set wakeup capability before requesting wakeup interrupt, so we can
+ * process boot-time "wakeups" (e.g., from S5 soft-off)
+ */
+ device_set_wakeup_capable(dev, true);
+ device_wakeup_enable(dev);
+
+ timer->irq =3D platform_get_irq(pdev, 0);
+ if (timer->irq < 0)
+ return -ENODEV;
+
+ timer->clk =3D devm_clk_get(dev, NULL);
+ if (!IS_ERR(timer->clk)) {
+ ret =3D clk_prepare_enable(timer->clk);
+ if (ret)
+ return ret;
+ timer->rate =3D clk_get_rate(timer->clk);
+ if (!timer->rate)
+ timer->rate =3D BRCMSTB_WKTMR_DEFAULT_FREQ;
+ } else {
+ timer->rate =3D BRCMSTB_WKTMR_DEFAULT_FREQ;
+ timer->clk =3D NULL;
+ }
+
+ ret =3D devm_request_irq(dev, timer->irq, brcmstb_waketmr_irq, 0,
+ "brcmstb-waketimer", timer);
+ if (ret < 0)
+ return ret;
+
+ timer->reboot_notifier.notifier_call =3D brcmstb_waketmr_reboot;
+ register_reboot_notifier(&timer->reboot_notifier);
+
+ timer->rtc =3D rtc_device_register("brcmstb-waketmr", dev,
+ &brcmstb_waketmr_ops, THIS_MODULE);
+ if (IS_ERR(timer->rtc)) {
+ dev_err(dev, "unable to register device\n");
+ unregister_reboot_notifier(&timer->reboot_notifier);
+ return PTR_ERR(timer->rtc);
+ }
+
+ dev_info(dev, "registered, with irq %d\n", timer->irq);
+
+ return ret;
+}
+
+static int brcmstb_waketmr_remove(struct platform_device *pdev)
+{
+ struct brcmstb_waketmr *timer =3D dev_get_drvdata(&pdev->dev);
+
+ unregister_reboot_notifier(&timer->reboot_notifier);
+ rtc_device_unregister(timer->rtc);
+
+ return 0;
+}
+
+#ifdef CONFIG_PM_SLEEP
+static int brcmstb_waketmr_suspend(struct device *dev)
+{
+ struct brcmstb_waketmr *timer =3D dev_get_drvdata(dev);
+
+ return brcmstb_waketmr_prepare_suspend(timer);
+}
+
+static int brcmstb_waketmr_resume(struct device *dev)
+{
+ struct brcmstb_waketmr *timer =3D dev_get_drvdata(dev);
+ int ret;
+
+ if (!device_may_wakeup(dev))
+ return 0;
+
+ ret =3D disable_irq_wake(timer->irq);
+
+ brcmstb_waketmr_clear_alarm(timer);
+
+ return ret;
+}
+#endif /* CONFIG_PM_SLEEP */
+
+static SIMPLE_DEV_PM_OPS(brcmstb_waketmr_pm_ops,
+ brcmstb_waketmr_suspend, brcmstb_waketmr_resume);
+
+static const struct of_device_id brcmstb_waketmr_of_match[] =3D {
+ { .compatible =3D "brcm,brcmstb-waketimer" },
+ { /* sentinel */ },
+};
+
+static struct platform_driver brcmstb_waketmr_driver =3D {
+ .probe =3D brcmstb_waketmr_probe,
+ .remove =3D brcmstb_waketmr_remove,
+ .driver =3D {
+ .name =3D "brcmstb-waketimer",
+ .pm =3D &brcmstb_waketmr_pm_ops,
+ .of_match_table =3D of_match_ptr(brcmstb_waketmr_of_match),
+ }
+};
+module_platform_driver(brcmstb_waketmr_driver);
+
+MODULE_LICENSE("GPL v2");
+MODULE_AUTHOR("Brian Norris");
+MODULE_AUTHOR("Markus Mayer");
+MODULE_DESCRIPTION("Wake-up timer driver for STB chips");
--=20
2.9.3
--=20
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.
---=20
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 e=
mail to rtc-linux+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
^ permalink raw reply related
* [rtc-linux] Re: [PATCH 2/2] rtc: brcmstb-waketimer: Add Broadcom STB wake-timer
From: Alexandre Belloni @ 2017-06-26 21:35 UTC (permalink / raw)
To: Florian Fainelli
Cc: linux-kernel, Brian Norris, Markus Mayer, Alessandro Zummo,
Rob Herring, Mark Rutland, Gregory Fong,
maintainer:BROADCOM BCM7XXX ARM ARCHITECTURE,
open list:REAL TIME CLOCK (RTC) SUBSYSTEM,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
moderated list:BROADCOM BCM7XXX ARM ARCHITECTURE
In-Reply-To: <35c43786-ee58-130c-2ecd-5577197bd5aa@gmail.com>
On 26/06/2017 at 10:15:46 -0700, Florian Fainelli wrote:
> > This is not needed since d68778b80dd7
>
> Great, I will take that out.
>
> Do you care whether some dev_dbg() prints are left in the driver or
> should I remove those in v2?
>
It could have stayed but it seems you removed it from v2.
I'll applay that but it is ok if you change your mind.
--
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/2] rtc: brcmstb-waketimer: Add Broadcom STB wake-timer
From: Florian Fainelli @ 2017-06-26 21:39 UTC (permalink / raw)
To: Alexandre Belloni, Florian Fainelli
Cc: linux-kernel, Brian Norris, Markus Mayer, Alessandro Zummo,
Rob Herring, Mark Rutland, Gregory Fong,
maintainer:BROADCOM BCM7XXX ARM ARCHITECTURE,
open list:REAL TIME CLOCK (RTC) SUBSYSTEM,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
moderated list:BROADCOM BCM7XXX ARM ARCHITECTURE
In-Reply-To: <20170626213534.jur6rggwbzhk6r4t@piout.net>
On 06/26/2017 02:35 PM, Alexandre Belloni wrote:
> On 26/06/2017 at 10:15:46 -0700, Florian Fainelli wrote:
>>> This is not needed since d68778b80dd7
>>
>> Great, I will take that out.
>>
>> Do you care whether some dev_dbg() prints are left in the driver or
>> should I remove those in v2?
>>
>
> It could have stayed but it seems you removed it from v2.
>
> I'll applay that but it is ok if you change your mind.
No, I think they don't really belong in the driver anymore, thanks!
--
Florian
--
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 2/2] rtc: brcmstb-waketimer: Add Broadcom STB wake-timer
From: Florian Fainelli @ 2017-06-26 21:15 UTC (permalink / raw)
To: linux-arm-kernel
Cc: Brian Norris, Markus Mayer, Florian Fainelli, Alessandro Zummo,
Alexandre Belloni, Rob Herring, Mark Rutland, Gregory Fong,
maintainer:BROADCOM BCM7XXX ARM ARCHITECTURE,
open list:REAL TIME CLOCK (RTC) SUBSYSTEM,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
open list
In-Reply-To: <20170626211503.7471-1-f.fainelli@gmail.com>
From: Brian Norris <computersforpeace@gmail.com>
This adds support for the Broadcom STB wake-timer which is a timer in
the chip's 27Mhz clock domain that offers the ability to wake the system
(wake-up source) from suspend states (S2, S3, S5). It is supported using
the rtc framework allowing us to configure alarms for system wake-up.
Signed-off-by: Brian Norris <computersforpeace@gmail.com>
Signed-off-by: Markus Mayer <mmayer@broadcom.com>
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/rtc/Kconfig | 11 ++
drivers/rtc/Makefile | 1 +
drivers/rtc/rtc-brcmstb-waketimer.c | 325 ++++++++++++++++++++++++++++++++++++
3 files changed, 337 insertions(+)
create mode 100644 drivers/rtc/rtc-brcmstb-waketimer.c
diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
index 8d3b95728326..8e1aa67fe533 100644
--- a/drivers/rtc/Kconfig
+++ b/drivers/rtc/Kconfig
@@ -197,6 +197,17 @@ config RTC_DRV_AC100
This driver can also be built as a module. If so, the module
will be called rtc-ac100.
+config RTC_DRV_BRCMSTB
+ tristate "Broadcom STB wake-timer"
+ depends on ARCH_BRCMSTB || BMIPS_GENERIC || COMPILE_TEST
+ default ARCH_BRCMSTB || BMIPS_GENERIC
+ help
+ If you say yes here you get support for the wake-timer found on
+ Broadcom STB SoCs (BCM7xxx).
+
+ This driver can also be built as a module. If so, the module will
+ be called rtc-brcmstb-waketimer.
+
config RTC_DRV_AS3722
tristate "ams AS3722 RTC driver"
depends on MFD_AS3722
diff --git a/drivers/rtc/Makefile b/drivers/rtc/Makefile
index 13857d2fce09..df89cac1f9ae 100644
--- a/drivers/rtc/Makefile
+++ b/drivers/rtc/Makefile
@@ -36,6 +36,7 @@ obj-$(CONFIG_RTC_DRV_AT91RM9200)+= rtc-at91rm9200.o
obj-$(CONFIG_RTC_DRV_AT91SAM9) += rtc-at91sam9.o
obj-$(CONFIG_RTC_DRV_AU1XXX) += rtc-au1xxx.o
obj-$(CONFIG_RTC_DRV_BFIN) += rtc-bfin.o
+obj-$(CONFIG_RTC_DRV_BRCMSTB) += rtc-brcmstb-waketimer.o
obj-$(CONFIG_RTC_DRV_BQ32K) += rtc-bq32k.o
obj-$(CONFIG_RTC_DRV_BQ4802) += rtc-bq4802.o
obj-$(CONFIG_RTC_DRV_CMOS) += rtc-cmos.o
diff --git a/drivers/rtc/rtc-brcmstb-waketimer.c b/drivers/rtc/rtc-brcmstb-waketimer.c
new file mode 100644
index 000000000000..5dea6d0627d8
--- /dev/null
+++ b/drivers/rtc/rtc-brcmstb-waketimer.c
@@ -0,0 +1,325 @@
+/*
+ * Copyright © 2014-2017 Broadcom
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * 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.
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/clk.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/init.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/irqreturn.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/pm.h>
+#include <linux/pm_wakeup.h>
+#include <linux/reboot.h>
+#include <linux/rtc.h>
+#include <linux/stat.h>
+#include <linux/suspend.h>
+
+struct brcmstb_waketmr {
+ struct rtc_device *rtc;
+ struct device *dev;
+ void __iomem *base;
+ int irq;
+ struct notifier_block reboot_notifier;
+ struct clk *clk;
+ u32 rate;
+};
+
+#define BRCMSTB_WKTMR_EVENT 0x00
+#define BRCMSTB_WKTMR_COUNTER 0x04
+#define BRCMSTB_WKTMR_ALARM 0x08
+#define BRCMSTB_WKTMR_PRESCALER 0x0C
+#define BRCMSTB_WKTMR_PRESCALER_VAL 0x10
+
+#define BRCMSTB_WKTMR_DEFAULT_FREQ 27000000
+
+static inline void brcmstb_waketmr_clear_alarm(struct brcmstb_waketmr *timer)
+{
+ writel_relaxed(1, timer->base + BRCMSTB_WKTMR_EVENT);
+ (void)readl_relaxed(timer->base + BRCMSTB_WKTMR_EVENT);
+}
+
+static void brcmstb_waketmr_set_alarm(struct brcmstb_waketmr *timer,
+ unsigned int secs)
+{
+ brcmstb_waketmr_clear_alarm(timer);
+
+ writel_relaxed(secs + 1, timer->base + BRCMSTB_WKTMR_ALARM);
+}
+
+static irqreturn_t brcmstb_waketmr_irq(int irq, void *data)
+{
+ struct brcmstb_waketmr *timer = data;
+
+ pm_wakeup_event(timer->dev, 0);
+
+ return IRQ_HANDLED;
+}
+
+struct wktmr_time {
+ u32 sec;
+ u32 pre;
+};
+
+static void wktmr_read(struct brcmstb_waketmr *timer,
+ struct wktmr_time *t)
+{
+ u32 tmp;
+
+ do {
+ t->sec = readl_relaxed(timer->base + BRCMSTB_WKTMR_COUNTER);
+ tmp = readl_relaxed(timer->base + BRCMSTB_WKTMR_PRESCALER_VAL);
+ } while (tmp >= timer->rate);
+
+ t->pre = timer->rate - tmp;
+}
+
+static int brcmstb_waketmr_prepare_suspend(struct brcmstb_waketmr *timer)
+{
+ struct device *dev = timer->dev;
+ int ret = 0;
+
+ if (device_may_wakeup(dev)) {
+ ret = enable_irq_wake(timer->irq);
+ if (ret) {
+ dev_err(dev, "failed to enable wake-up interrupt\n");
+ return ret;
+ }
+ }
+
+ return ret;
+}
+
+/* If enabled as a wakeup-source, arm the timer when powering off */
+static int brcmstb_waketmr_reboot(struct notifier_block *nb,
+ unsigned long action, void *data)
+{
+ struct brcmstb_waketmr *timer;
+
+ timer = container_of(nb, struct brcmstb_waketmr, reboot_notifier);
+
+ /* Set timer for cold boot */
+ if (action == SYS_POWER_OFF)
+ brcmstb_waketmr_prepare_suspend(timer);
+
+ return NOTIFY_DONE;
+}
+
+static int brcmstb_waketmr_gettime(struct device *dev,
+ struct rtc_time *tm)
+{
+ struct brcmstb_waketmr *timer = dev_get_drvdata(dev);
+ struct wktmr_time now;
+
+ wktmr_read(timer, &now);
+
+ rtc_time_to_tm(now.sec, tm);
+
+ return 0;
+}
+
+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;
+
+ rtc_tm_to_time(tm, &sec);
+
+ writel_relaxed(sec, timer->base + BRCMSTB_WKTMR_COUNTER);
+
+ return 0;
+}
+
+static int brcmstb_waketmr_getalarm(struct device *dev,
+ struct rtc_wkalrm *alarm)
+{
+ struct brcmstb_waketmr *timer = dev_get_drvdata(dev);
+ unsigned long 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);
+ }
+
+ reg = readl_relaxed(timer->base + BRCMSTB_WKTMR_EVENT);
+ alarm->pending = !!(reg & 1);
+
+ return 0;
+}
+
+static int brcmstb_waketmr_setalarm(struct device *dev,
+ struct rtc_wkalrm *alarm)
+{
+ struct brcmstb_waketmr *timer = dev_get_drvdata(dev);
+ unsigned long sec;
+
+ if (alarm->enabled)
+ rtc_tm_to_time(&alarm->time, &sec);
+ else
+ sec = 0;
+
+ brcmstb_waketmr_set_alarm(timer, sec);
+
+ return 0;
+}
+
+/*
+ * Does not do much but keep the RTC class happy. We always support
+ * alarms.
+ */
+static int brcmstb_waketmr_alarm_enable(struct device *dev,
+ unsigned int enabled)
+{
+ return 0;
+}
+
+static const struct rtc_class_ops brcmstb_waketmr_ops = {
+ .read_time = brcmstb_waketmr_gettime,
+ .set_time = brcmstb_waketmr_settime,
+ .read_alarm = brcmstb_waketmr_getalarm,
+ .set_alarm = brcmstb_waketmr_setalarm,
+ .alarm_irq_enable = brcmstb_waketmr_alarm_enable,
+};
+
+static int brcmstb_waketmr_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct brcmstb_waketmr *timer;
+ struct resource *res;
+ int ret;
+
+ timer = devm_kzalloc(dev, sizeof(*timer), GFP_KERNEL);
+ if (!timer)
+ return -ENOMEM;
+
+ platform_set_drvdata(pdev, timer);
+ timer->dev = dev;
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ timer->base = devm_ioremap_resource(dev, res);
+ if (IS_ERR(timer->base))
+ return PTR_ERR(timer->base);
+
+ /*
+ * Set wakeup capability before requesting wakeup interrupt, so we can
+ * process boot-time "wakeups" (e.g., from S5 soft-off)
+ */
+ device_set_wakeup_capable(dev, true);
+ device_wakeup_enable(dev);
+
+ timer->irq = platform_get_irq(pdev, 0);
+ if (timer->irq < 0)
+ return -ENODEV;
+
+ timer->clk = devm_clk_get(dev, NULL);
+ if (!IS_ERR(timer->clk)) {
+ ret = clk_prepare_enable(timer->clk);
+ if (ret)
+ return ret;
+ timer->rate = clk_get_rate(timer->clk);
+ if (!timer->rate)
+ timer->rate = BRCMSTB_WKTMR_DEFAULT_FREQ;
+ } else {
+ timer->rate = BRCMSTB_WKTMR_DEFAULT_FREQ;
+ timer->clk = NULL;
+ }
+
+ ret = devm_request_irq(dev, timer->irq, brcmstb_waketmr_irq, 0,
+ "brcmstb-waketimer", timer);
+ if (ret < 0)
+ return ret;
+
+ timer->reboot_notifier.notifier_call = brcmstb_waketmr_reboot;
+ register_reboot_notifier(&timer->reboot_notifier);
+
+ timer->rtc = rtc_device_register("brcmstb-waketmr", dev,
+ &brcmstb_waketmr_ops, THIS_MODULE);
+ if (IS_ERR(timer->rtc)) {
+ dev_err(dev, "unable to register device\n");
+ unregister_reboot_notifier(&timer->reboot_notifier);
+ return PTR_ERR(timer->rtc);
+ }
+
+ dev_info(dev, "registered, with irq %d\n", timer->irq);
+
+ return ret;
+}
+
+static int brcmstb_waketmr_remove(struct platform_device *pdev)
+{
+ struct brcmstb_waketmr *timer = dev_get_drvdata(&pdev->dev);
+
+ unregister_reboot_notifier(&timer->reboot_notifier);
+ rtc_device_unregister(timer->rtc);
+
+ return 0;
+}
+
+#ifdef CONFIG_PM_SLEEP
+static int brcmstb_waketmr_suspend(struct device *dev)
+{
+ struct brcmstb_waketmr *timer = dev_get_drvdata(dev);
+
+ return brcmstb_waketmr_prepare_suspend(timer);
+}
+
+static int brcmstb_waketmr_resume(struct device *dev)
+{
+ struct brcmstb_waketmr *timer = dev_get_drvdata(dev);
+ int ret;
+
+ if (!device_may_wakeup(dev))
+ return 0;
+
+ ret = disable_irq_wake(timer->irq);
+
+ brcmstb_waketmr_clear_alarm(timer);
+
+ return ret;
+}
+#endif /* CONFIG_PM_SLEEP */
+
+static SIMPLE_DEV_PM_OPS(brcmstb_waketmr_pm_ops,
+ brcmstb_waketmr_suspend, brcmstb_waketmr_resume);
+
+static const struct of_device_id brcmstb_waketmr_of_match[] = {
+ { .compatible = "brcm,brcmstb-waketimer" },
+ { /* sentinel */ },
+};
+
+static struct platform_driver brcmstb_waketmr_driver = {
+ .probe = brcmstb_waketmr_probe,
+ .remove = brcmstb_waketmr_remove,
+ .driver = {
+ .name = "brcmstb-waketimer",
+ .pm = &brcmstb_waketmr_pm_ops,
+ .of_match_table = of_match_ptr(brcmstb_waketmr_of_match),
+ }
+};
+module_platform_driver(brcmstb_waketmr_driver);
+
+MODULE_LICENSE("GPL v2");
+MODULE_AUTHOR("Brian Norris");
+MODULE_AUTHOR("Markus Mayer");
+MODULE_DESCRIPTION("Wake-up timer driver for STB chips");
--
2.9.3
^ permalink raw reply related
* [rtc-linux] Re: [PATCH v2 2/2] rtc: brcmstb-waketimer: Add Broadcom STB wake-timer
From: Alexandre Belloni @ 2017-06-26 21:47 UTC (permalink / raw)
To: Florian Fainelli
Cc: linux-arm-kernel, Brian Norris, Markus Mayer, Alessandro Zummo,
Rob Herring, Mark Rutland, Gregory Fong,
maintainer:BROADCOM BCM7XXX ARM ARCHITECTURE,
open list:REAL TIME CLOCK (RTC) SUBSYSTEM,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
open list
In-Reply-To: <20170626211503.7471-3-f.fainelli@gmail.com>
On 26/06/2017 at 14:15:03 -0700, Florian Fainelli wrote:
> From: Brian Norris <computersforpeace@gmail.com>
>
> This adds support for the Broadcom STB wake-timer which is a timer in
> the chip's 27Mhz clock domain that offers the ability to wake the system
> (wake-up source) from suspend states (S2, S3, S5). It is supported using
> the rtc framework allowing us to configure alarms for system wake-up.
>
> Signed-off-by: Brian Norris <computersforpeace@gmail.com>
> Signed-off-by: Markus Mayer <mmayer@broadcom.com>
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> ---
> drivers/rtc/Kconfig | 11 ++
> drivers/rtc/Makefile | 1 +
> drivers/rtc/rtc-brcmstb-waketimer.c | 325 ++++++++++++++++++++++++++++++++++++
> 3 files changed, 337 insertions(+)
> create mode 100644 drivers/rtc/rtc-brcmstb-waketimer.c
>
Both 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 v2] rtc: ds3232: add temperature support
From: Kirill Esipov @ 2017-06-27 12:24 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Alessandro Zummo, Alexandre Belloni, linux-rtc,
linux-kernel@vger.kernel.org
In-Reply-To: <CAHp75Vd-CoKpt0dhXspCDLSoohEVRTHEEPn9jZtFqUwvAeB1bA@mail.gmail.com>
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?
>> +#ifdef CONFIG_RTC_DRV_DS3232_HWMON
>
> IS_BUILTIN() ?
>
>> +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.
>
>> + if (ret < 0)
>> + return ret;
>
>> +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:
>> + ds3232_hwmon_read_temp(dev, temp);
>> + err = 0;
>> + break;
>> + default:
>> + return -EOPNOTSUPP;
>> + }
>> +
>> + return err;
>
> You may do as in previous function. Or what did you mean here by
> introducing an err variable?
>
>> +}
>
>> +
>> +
>
> Remove one of them.
>
>> +static void ds3232_hwmon_register(struct device *dev, const char *name)
>> +{
>> + struct ds3232 *ds3232 = dev_get_drvdata(dev);
>> + struct device *hwmon_dev;
>> +
>> + hwmon_dev = devm_hwmon_device_register_with_info(dev, name, ds3232,
>> + &ds3232_hwmon_chip_info,
>> + NULL);
>
>> +
>
> Remove.
>
>> + if (IS_ERR(hwmon_dev)) {
>> + dev_warn(dev, "unable to register hwmon device %ld\n",
>> + PTR_ERR(hwmon_dev));
>> + }
>> +}
>> +
>
>> +#else
>
> 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.
>
>> +
>> +static void ds3232_hwmon_register(struct device *dev, const char *name)
>> +{
>> +}
>> +
>> +#endif
>
> --
> With Best Regards,
> Andy Shevchenko
--
Kirill Esipov
^ permalink raw reply
* Re: [PATCH v2] rtc: ds3232: add temperature support
From: Alexandre Belloni @ 2017-06-27 13:00 UTC (permalink / raw)
To: Kirill Esipov
Cc: Andy Shevchenko, Alessandro Zummo, linux-rtc,
linux-kernel@vger.kernel.org
In-Reply-To: <CAJcpCzF1SJ2v-KsJXN7c=BogeQv4LdAZEgZFsgVVy3HKPCcO1Q@mail.gmail.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.
> >> +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
^ 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