Linux RTC
 help / color / mirror / Atom feed
* [PATCH 11/11] rtc: ds1307: remove ds1307_remove
From: Alexandre Belloni @ 2017-07-06  9:42 UTC (permalink / raw)
  To: linux-rtc; +Cc: linux-kernel, Alexandre Belloni
In-Reply-To: <20170706094208.16240-1-alexandre.belloni@free-electrons.com>

ds1307_remove() is now empty, remove it

Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
---
 drivers/rtc/rtc-ds1307.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/drivers/rtc/rtc-ds1307.c b/drivers/rtc/rtc-ds1307.c
index 6f91c11a5f03..ea83ddc5a513 100644
--- a/drivers/rtc/rtc-ds1307.c
+++ b/drivers/rtc/rtc-ds1307.c
@@ -1701,11 +1701,6 @@ static int ds1307_probe(struct i2c_client *client,
 	return err;
 }
 
-static int ds1307_remove(struct i2c_client *client)
-{
-	return 0;
-}
-
 static struct i2c_driver ds1307_driver = {
 	.driver = {
 		.name	= "rtc-ds1307",
@@ -1713,7 +1708,6 @@ static struct i2c_driver ds1307_driver = {
 		.acpi_match_table = ACPI_PTR(ds1307_acpi_ids),
 	},
 	.probe		= ds1307_probe,
-	.remove		= ds1307_remove,
 	.id_table	= ds1307_id,
 };
 
-- 
2.11.0

^ permalink raw reply related

* Re: [rtc-linux] [PATCH 2/2] rtc: ds1307: disable square wave output from ds1308
From: Alexandre Belloni @ 2017-07-06  9:55 UTC (permalink / raw)
  To: Sean Nyekjaer; +Cc: rtc-linux
In-Reply-To: <20170608103655.20043-2-sean.nyekjaer@prevas.dk>

Hi,

On 08/06/2017 at 12:36:55 +0200, Sean Nyekjaer wrote:
> Disable sqw output from ds1308 in order to reduce
> the power consumption by ~50%.
> 
> Signed-off-by: Sean Nyekjaer <sean.nyekjaer@prevas.dk>
> ---
>  drivers/rtc/rtc-ds1307.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/rtc/rtc-ds1307.c b/drivers/rtc/rtc-ds1307.c
> index a4ef03ad9395..8d9f25a3c757 100644
> --- a/drivers/rtc/rtc-ds1307.c
> +++ b/drivers/rtc/rtc-ds1307.c
> @@ -78,6 +78,7 @@ enum ds_type {
>  #	define DS1307_BIT_OUT		0x80
>  #	define DS1338_BIT_OSF		0x20
>  #	define DS1307_BIT_SQWE		0x10
> +#	define DS1308_BIT_BBCLK		0x04
>  #	define DS1307_BIT_RS1		0x02
>  #	define DS1307_BIT_RS0		0x01
>  #define DS1337_REG_CONTROL	0x0e
> @@ -1454,6 +1455,11 @@ static int ds1307_probe(struct i2c_client *client,
>  		}
>  		break;
>  	case ds_1308:
> +		/* disable sqw when driven from battery backup */
> +		if (ds1307->regs[DS1307_REG_CONTROL] & DS1308_BIT_BBCLK)
> +			i2c_smbus_write_byte_data(client, DS1307_REG_CONTROL,
> +					ds1307->regs[DS1307_REG_CONTROL]
> +					& ~DS1338_BIT_BBCLK);

This should be done using the common clock framework, please have a look
at what is done for the ds3231 in ds1307_clks_register.

>  	case ds_1338:
>  		/* clock halted?  turn it on, so clock can tick. */
>  		if (tmp & DS1307_BIT_CH)
> -- 
> 2.13.0
> 
> -- 
> 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.

-- 
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

^ permalink raw reply

* Re: [rtc-linux] [PATCH 1/2] rtc: ds1307: add ds1308 variant
From: Alexandre Belloni @ 2017-07-06  9:57 UTC (permalink / raw)
  To: Sean Nyekjaer; +Cc: rtc-linux
In-Reply-To: <20170608103655.20043-1-sean.nyekjaer@prevas.dk>

On 08/06/2017 at 12:36:54 +0200, Sean Nyekjaer wrote:
> The ds1308 variant is very similar to the already supported ds1338
> variant, it have more debug registers and a square wave clock output.
> 
> Signed-off-by: Sean Nyekjaer <sean.nyekjaer@prevas.dk>
> ---
>  drivers/rtc/rtc-ds1307.c | 12 ++++++++++++
>  1 file changed, 12 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 v2 02/10] MIPS: ranchu: Add Goldfish RTC driver
From: Miodrag Dinic @ 2017-07-06 13:25 UTC (permalink / raw)
  To: Alexandre Belloni, Aleksandar Markovic
  Cc: linux-mips@linux-mips.org, Aleksandar Markovic, Goran Ferenc,
	Alessandro Zummo, David S. Miller, Douglas Leung,
	Greg Kroah-Hartman, James Hogan, linux-kernel@vger.kernel.org,
	linux-rtc@vger.kernel.org, Martin K. Petersen,
	Mauro Carvalho Chehab, Paul Burton, Petar Jovanovic,
	Raghu Gandham, jinqian@google.com, Bo Hu, lfy@google.com
In-Reply-To: <20170705215602.vihwoio2dagxy2fc@piout.net>

cc-ing Jin Quian, Bo Hu & Lingfeng Yang from Google

Hi Alexandre,

thank you for your comments, answers are inline:

> 
> On 28/06/2017 at 17:46:55 +0200, Aleksandar Markovic wrote:
> > 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".
> > 
> 
> Is it really MIPS specific? I would expect the same driver to work on
> the ARM based emulator too.

This driver can be made to work for ARM/Intel emulator but it is currently
used only by MIPS emulator, so I would prefer to keep it guarded with "MIPS".
If ARM or Intel decide to use this driver for their emulators it can be easily
enabled.

> > +config RTC_DRV_GOLDFISH
> > +     tristate "Goldfish Real Time Clock"
> > +     depends on MIPS
> > +     depends on GOLDFISH
> 
> This should be made buildable with COMPILE_TEST to extend coverage.

It will be included in the next version.

> > +     help
> > +       Say yes here to build support for the Goldfish RTC.
> 
> Please, don't expect anybody to actually know what is goldfish can you
> add a sentence or two?

It will be better documented in the next version. Thank you.

> > +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);
> 
> I'd say that events is not needed you can pass the flags directly to
> rtc_update_irq

It will be corrected in the next version.

> > +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);
> 
> I'm not sure why you need that loop as the comments for TIMER_TIME_LOW
> and TIMER_TIME_HIGH indicate that TIMER_TIME_HIGH is latched when
> TIMER_TIME_LOW is read. Note that the original driver from google
> doesn't do that.

This is the way how other HW drivers are doing it, so we used this
approach to make it more in-line with other HW, and it also does not
make any assumptions regarding TIMER_TIME_HIGH is latched or not.
This is the relevant part of code on the RTC device side which emulates these reads:

static uint64_t goldfish_timer_read(void *opaque, hwaddr offset, unsigned size)
{
    struct timer_state *s = (struct timer_state *)opaque;
    switch(offset) {
        case TIMER_TIME_LOW:
            s->now_ns = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
            return s->now_ns;
        case TIMER_TIME_HIGH:
            return s->now_ns >> 32;
        default:
            cpu_abort(current_cpu,
                      "goldfish_timer_read: Bad offset %" HWADDR_PRIx "\n",
                      offset);
            return 0;
    }
}

So theoretically speaking, we could imagine the situation that the kernel pre-empts after the
first TIMER_TIME_LOW read and another request for reading the time gets processed, so
the previous call would end-up having stale TIMER_TIME_LOW value.
This is however very unlikely to happen, but one extra read in the loop doesn't hurt and
actually makes the code less prone to error.

> > +     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);
> 
> devm_ioremap_resource ?

Thanks, it will be fixed in next version to use devm_ioremap_resource().

> > +     if (IS_ERR(qrtc->base))
> > +             return -ENODEV;
> > +
> > +     qrtc->irq = platform_get_irq(pdev, 0);
> > +     if (qrtc->irq < 0)
> > +             return -ENODEV;
> > +
> 
> Is the irq so important that you have to fail here even if the driver
> doesn't support any alarm?

Well currently it does not support alarm features, but we are considering
to implement it in some other iteration. Maybe we will introduce it in the next version
if not we will remove the IRQ handling. Thanks.

> > +     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;
> 
> Ditto.

Kind regards,
Miodrag

________________________________________
From: Alexandre Belloni [alexandre.belloni@free-electrons.com]
Sent: Wednesday, July 05, 2017 11:56 PM
To: Aleksandar Markovic
Cc: linux-mips@linux-mips.org; Aleksandar Markovic; Miodrag Dinic; Goran Ferenc; Alessandro Zummo; David S. Miller; Douglas Leung; Greg Kroah-Hartman; James Hogan; linux-kernel@vger.kernel.org; linux-rtc@vger.kernel.org; Martin K. Petersen; Mauro Carvalho Chehab; Paul Burton; Petar Jovanovic; Raghu Gandham
Subject: Re: [PATCH v2 02/10] MIPS: ranchu: Add Goldfish RTC driver

Hi,

The subject doesn't fit the subsystem style, this needs to be changed.

On 28/06/2017 at 17:46:55 +0200, Aleksandar Markovic wrote:
> 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".
>

Is it really MIPS specific? I would expect the same driver to work on
the ARM based emulator too.

> +config RTC_DRV_GOLDFISH
> +     tristate "Goldfish Real Time Clock"
> +     depends on MIPS
> +     depends on GOLDFISH

This should be made buildable with COMPILE_TEST to extend coverage.

> +     help
> +       Say yes here to build support for the Goldfish RTC.

Please, don't expect anybody to actually know what is goldfish can you
add a sentence or two?

> +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);

I'd say that events is not needed you can pass the flags directly to
rtc_update_irq

> +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);

I'm not sure why you need that loop as the comments for TIMER_TIME_LOW
and TIMER_TIME_HIGH indicate that TIMER_TIME_HIGH is latched when
TIMER_TIME_LOW is read. Note that the original driver from google
doesn't do that.

> +     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);

devm_ioremap_resource ?

> +     if (IS_ERR(qrtc->base))
> +             return -ENODEV;
> +
> +     qrtc->irq = platform_get_irq(pdev, 0);
> +     if (qrtc->irq < 0)
> +             return -ENODEV;
> +

Is the irq so important that you have to fail here even if the driver
doesn't support any alarm?

> +     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;

Ditto.


--
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

^ permalink raw reply

* Re: [PATCH v2 02/10] MIPS: ranchu: Add Goldfish RTC driver
From: Alexandre Belloni @ 2017-07-06 15:13 UTC (permalink / raw)
  To: Miodrag Dinic
  Cc: Aleksandar Markovic, linux-mips@linux-mips.org,
	Aleksandar Markovic, Goran Ferenc, Alessandro Zummo,
	David S. Miller, Douglas Leung, Greg Kroah-Hartman, James Hogan,
	linux-kernel@vger.kernel.org, linux-rtc@vger.kernel.org,
	Martin K. Petersen, Mauro Carvalho Chehab, Paul Burton,
	Petar Jovanovic, Raghu Gandham, jinqian@google.com, Bo Hu,
	lfy@google.com
In-Reply-To: <232DDC0A2B605E4F9E85F6904417885F015D929D3D@BADAG02.ba.imgtec.org>

On 06/07/2017 at 13:25:09 +0000, Miodrag Dinic wrote:
> > > +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);
> > 
> > I'm not sure why you need that loop as the comments for TIMER_TIME_LOW
> > and TIMER_TIME_HIGH indicate that TIMER_TIME_HIGH is latched when
> > TIMER_TIME_LOW is read. Note that the original driver from google
> > doesn't do that.
> 
> This is the way how other HW drivers are doing it, so we used this
> approach to make it more in-line with other HW, and it also does not
> make any assumptions regarding TIMER_TIME_HIGH is latched or not.
> This is the relevant part of code on the RTC device side which emulates these reads:
> 
> static uint64_t goldfish_timer_read(void *opaque, hwaddr offset, unsigned size)
> {
>     struct timer_state *s = (struct timer_state *)opaque;
>     switch(offset) {
>         case TIMER_TIME_LOW:
>             s->now_ns = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
>             return s->now_ns;
>         case TIMER_TIME_HIGH:
>             return s->now_ns >> 32;
>         default:
>             cpu_abort(current_cpu,
>                       "goldfish_timer_read: Bad offset %" HWADDR_PRIx "\n",
>                       offset);
>             return 0;
>     }
> }
> 
> So theoretically speaking, we could imagine the situation that the kernel pre-empts after the
> first TIMER_TIME_LOW read and another request for reading the time gets processed, so
> the previous call would end-up having stale TIMER_TIME_LOW value.
> This is however very unlikely to happen, but one extra read in the loop doesn't hurt and
> actually makes the code less prone to error.
> 

Every call to the RTC callbacks are protected by a mutex so this will
never happen.

Most of the RTCs are actually latching the time on the first register
read and don't require specific handling. I'd prefer to keep the driver
simple.


> > > +     qrtc->irq = platform_get_irq(pdev, 0);
> > > +     if (qrtc->irq < 0)
> > > +             return -ENODEV;
> > > +
> > 
> > Is the irq so important that you have to fail here even if the driver
> > doesn't support any alarm?
> 
> Well currently it does not support alarm features, but we are considering
> to implement it in some other iteration. Maybe we will introduce it in the next version
> if not we will remove the IRQ handling. Thanks.
> 

I'd say that you should probably leave out the whole IRQ handling until
you really handle alarms in the driver or do you have a way to generate
alarms (and so interrupts) without using the driver?


-- 
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

^ permalink raw reply

* Hardware specific rtc regression in v4.4.76
From: elseifthen @ 2017-07-06 19:16 UTC (permalink / raw)
  To: rtc
In-Reply-To: <S1752256AbdGFSzp/20170706185546Z+86@vger.kernel.org>


Hello all,

There appears to be a hardware specific rtc regression in v4.4.76.

Introduced in v4.4.52 by:

commit f0414c1f8bb7a4e69064296f460773170c5435ac
Author: Colin Ian King <colin.king@canonical.com>
Date:   Mon May 16 17:22:54 2016 +0100

    rtc: interface: ignore expired timers when enqueuing new timers
    
    commit 2b2f5ff00f63847d95adad6289bd8b05f5983dd5 upstream.

 8< ---

The problem does not appear to be in v4.9.x kernels.

Maybe v4.4.76 is missing one of these?
http://marc.info/?l=linux-rtc&m=149489307123605&w=2
Which also includes the commit in question.

See this thread for more details:
https://www.linuxquestions.org/questions/slackware-14/slow-hwclock-systohc-on-reboot-shutdown-4175609040/

Note: I am not the one having this issue. I'm only reporting it, so any
questions would need to be asked in the above thread. Or perhaps the
person with the problem will follow this email thread. I will post a
link to it for them.

^ permalink raw reply

* rtc: ds1307: fix compile warning if RTC_DRV_DS1307_CENTURY isn't defined
From: Heiner Kallweit @ 2017-07-06 20:01 UTC (permalink / raw)
  To: Alexandre Belloni; +Cc: linux-rtc, Stephen Rothwell

Fix the following warning if RTC_DRV_DS1307_CENTURY isn't defined.

drivers/rtc/rtc-ds1307.c: In function 'ds1307_get_time':
drivers/rtc/rtc-ds1307.c:342:26: warning: unused variable 'chip' [-Wunused-variable]
  const struct chip_desc *chip = &chips[ds1307->type];
                          ^
Fixes: 436c93db60a4 ("rtc: ds1307: factor out century bit handling")
Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/rtc/rtc-ds1307.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/rtc/rtc-ds1307.c b/drivers/rtc/rtc-ds1307.c
index 43e9dc6f..18b6a6af 100644
--- a/drivers/rtc/rtc-ds1307.c
+++ b/drivers/rtc/rtc-ds1307.c
@@ -368,10 +368,9 @@ static int ds1307_get_time(struct device *dev, struct rtc_time *t)
 	t->tm_mon = bcd2bin(tmp) - 1;
 	t->tm_year = bcd2bin(ds1307->regs[DS1307_REG_YEAR]) + 100;
 
-#ifdef CONFIG_RTC_DRV_DS1307_CENTURY
-	if (ds1307->regs[chip->century_reg] & chip->century_bit)
+	if (ds1307->regs[chip->century_reg] & chip->century_bit &&
+	    IS_ENABLED(CONFIG_RTC_DRV_DS1307_CENTURY))
 		t->tm_year += 100;
-#endif
 
 	dev_dbg(dev, "%s secs=%d, mins=%d, "
 		"hours=%d, mday=%d, mon=%d, year=%d, wday=%d\n",
-- 
2.13.1

^ permalink raw reply related

* [PATCH] rtc: ds1307: fix compile warning if RTC_DRV_DS1307_CENTURY isn't defined
From: Heiner Kallweit @ 2017-07-06 20:03 UTC (permalink / raw)
  To: Alexandre Belloni; +Cc: linux-rtc, Stephen Rothwell

Fix the following warning if RTC_DRV_DS1307_CENTURY isn't defined.

drivers/rtc/rtc-ds1307.c: In function 'ds1307_get_time':
drivers/rtc/rtc-ds1307.c:342:26: warning: unused variable 'chip' [-Wunused-variable]
  const struct chip_desc *chip = &chips[ds1307->type];
                          ^
Fixes: 436c93db60a4 ("rtc: ds1307: factor out century bit handling")
Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/rtc/rtc-ds1307.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/rtc/rtc-ds1307.c b/drivers/rtc/rtc-ds1307.c
index 43e9dc6f..18b6a6af 100644
--- a/drivers/rtc/rtc-ds1307.c
+++ b/drivers/rtc/rtc-ds1307.c
@@ -368,10 +368,9 @@ static int ds1307_get_time(struct device *dev, struct rtc_time *t)
 	t->tm_mon = bcd2bin(tmp) - 1;
 	t->tm_year = bcd2bin(ds1307->regs[DS1307_REG_YEAR]) + 100;
 
-#ifdef CONFIG_RTC_DRV_DS1307_CENTURY
-	if (ds1307->regs[chip->century_reg] & chip->century_bit)
+	if (ds1307->regs[chip->century_reg] & chip->century_bit &&
+	    IS_ENABLED(CONFIG_RTC_DRV_DS1307_CENTURY))
 		t->tm_year += 100;
-#endif
 
 	dev_dbg(dev, "%s secs=%d, mins=%d, "
 		"hours=%d, mday=%d, mon=%d, year=%d, wday=%d\n",
-- 
2.13.1

^ permalink raw reply related

* Re: [PATCH] rtc: hctosys: move hctosys functionality to rtc_device_register
From: Heiner Kallweit @ 2017-07-06 20:15 UTC (permalink / raw)
  To: Alexandre Belloni; +Cc: Alessandro Zummo, linux-rtc
In-Reply-To: <14cc1b9c-66af-2e09-1ca5-07aa55025beb@gmail.com>

Am 07.03.2017 um 20:10 schrieb Heiner Kallweit:
> It's a little annoying that rtc driver plus all drivers it depends on
> (e.g. I2C) have to be compiled into the kernel to be used with hctosys.
> 
> I think it would be more logical to set the system clock once the
> rtc driver has been registered. Therefore move the hctosys
> functionality to the end of rtc_device_register.
> 
> Works perfectly fine here.
> 
Do you have an opinion on this one?

Rgds, Heiner


> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> ---
>  drivers/rtc/Makefile  |  1 -
>  drivers/rtc/class.c   | 38 ++++++++++++++++++++++++++++
>  drivers/rtc/hctosys.c | 70 ---------------------------------------------------
>  3 files changed, 38 insertions(+), 71 deletions(-)
>  delete mode 100644 drivers/rtc/hctosys.c
> 
> diff --git a/drivers/rtc/Makefile b/drivers/rtc/Makefile
> index f07297b1..f1d57b06 100644
> --- a/drivers/rtc/Makefile
> +++ b/drivers/rtc/Makefile
> @@ -5,7 +5,6 @@
>  ccflags-$(CONFIG_RTC_DEBUG)	:= -DDEBUG
>  
>  obj-$(CONFIG_RTC_LIB)		+= rtc-lib.o
> -obj-$(CONFIG_RTC_HCTOSYS)	+= hctosys.o
>  obj-$(CONFIG_RTC_SYSTOHC)	+= systohc.o
>  obj-$(CONFIG_RTC_CLASS)		+= rtc-core.o
>  obj-$(CONFIG_RTC_MC146818_LIB)	+= rtc-mc146818-lib.o
> diff --git a/drivers/rtc/class.c b/drivers/rtc/class.c
> index 74fd9746..dcfd2e64 100644
> --- a/drivers/rtc/class.c
> +++ b/drivers/rtc/class.c
> @@ -168,6 +168,13 @@ struct rtc_device *rtc_device_register(const char *name, struct device *dev,
>  	struct rtc_wkalrm alrm;
>  	int of_id = -1, id = -1, err;
>  
> +#if IS_ENABLED(CONFIG_RTC_HCTOSYS)
> +	struct rtc_time tm;
> +	struct timespec64 tv64 = {
> +		.tv_nsec = NSEC_PER_SEC >> 1,
> +	};
> +#endif
> +
>  	if (dev->of_node)
>  		of_id = of_alias_get_id(dev->of_node, "rtc");
>  	else if (dev->parent && dev->parent->of_node)
> @@ -246,6 +253,37 @@ struct rtc_device *rtc_device_register(const char *name, struct device *dev,
>  	dev_info(dev, "rtc core: registered %s as %s\n",
>  			rtc->name, dev_name(&rtc->dev));
>  
> +#if IS_ENABLED(CONFIG_RTC_HCTOSYS)
> +/* IMPORTANT: the RTC only stores whole seconds. It is arbitrary
> + * whether it stores the most close value or the value with partial
> + * seconds truncated. However, it is important that we use it to store
> + * the truncated value. This is because otherwise it is necessary,
> + * in an rtc sync function, to read both xtime.tv_sec and
> + * xtime.tv_nsec. On some processors (i.e. ARM), an atomic read
> + * of >32bits is not possible. So storing the most close value would
> + * slow down the sync API. So here we have the truncated value and
> + * the best guess is to add 0.5s.
> + */
> +	if (!strcmp(CONFIG_RTC_HCTOSYS_DEVICE, dev_name(&rtc->dev))) {
> +		err = rtc_read_time(rtc, &tm);
> +		if (err) {
> +			dev_err(dev, "hctosys: unable to read the hardware clock\n");
> +			goto hctosys_out;
> +		}
> +
> +		tv64.tv_sec = rtc_tm_to_time64(&tm);
> +
> +		err = do_settimeofday64(&tv64);
> +
> +		dev_info(dev, "setting system clock to %d-%02d-%02d %02d:%02d:%02d UTC (%lld)\n",
> +			 tm.tm_year + 1900, tm.tm_mon + 1, tm.tm_mday,
> +			 tm.tm_hour, tm.tm_min, tm.tm_sec,
> +			 (long long) tv64.tv_sec);
> +hctosys_out:
> +		rtc_hctosys_ret = err;
> +	}
> +#endif
> +
>  	return rtc;
>  
>  exit_ida:
> diff --git a/drivers/rtc/hctosys.c b/drivers/rtc/hctosys.c
> deleted file mode 100644
> index e1cfa068..00000000
> --- a/drivers/rtc/hctosys.c
> +++ /dev/null
> @@ -1,70 +0,0 @@
> -/*
> - * RTC subsystem, initialize system time on startup
> - *
> - * Copyright (C) 2005 Tower Technologies
> - * Author: Alessandro Zummo <a.zummo@towertech.it>
> - *
> - * 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.
> -*/
> -
> -#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> -
> -#include <linux/rtc.h>
> -
> -/* IMPORTANT: the RTC only stores whole seconds. It is arbitrary
> - * whether it stores the most close value or the value with partial
> - * seconds truncated. However, it is important that we use it to store
> - * the truncated value. This is because otherwise it is necessary,
> - * in an rtc sync function, to read both xtime.tv_sec and
> - * xtime.tv_nsec. On some processors (i.e. ARM), an atomic read
> - * of >32bits is not possible. So storing the most close value would
> - * slow down the sync API. So here we have the truncated value and
> - * the best guess is to add 0.5s.
> - */
> -
> -static int __init rtc_hctosys(void)
> -{
> -	int err = -ENODEV;
> -	struct rtc_time tm;
> -	struct timespec64 tv64 = {
> -		.tv_nsec = NSEC_PER_SEC >> 1,
> -	};
> -	struct rtc_device *rtc = rtc_class_open(CONFIG_RTC_HCTOSYS_DEVICE);
> -
> -	if (rtc == NULL) {
> -		pr_info("unable to open rtc device (%s)\n",
> -			CONFIG_RTC_HCTOSYS_DEVICE);
> -		goto err_open;
> -	}
> -
> -	err = rtc_read_time(rtc, &tm);
> -	if (err) {
> -		dev_err(rtc->dev.parent,
> -			"hctosys: unable to read the hardware clock\n");
> -		goto err_read;
> -
> -	}
> -
> -	tv64.tv_sec = rtc_tm_to_time64(&tm);
> -
> -	err = do_settimeofday64(&tv64);
> -
> -	dev_info(rtc->dev.parent,
> -		"setting system clock to "
> -		"%d-%02d-%02d %02d:%02d:%02d UTC (%lld)\n",
> -		tm.tm_year + 1900, tm.tm_mon + 1, tm.tm_mday,
> -		tm.tm_hour, tm.tm_min, tm.tm_sec,
> -		(long long) tv64.tv_sec);
> -
> -err_read:
> -	rtc_class_close(rtc);
> -
> -err_open:
> -	rtc_hctosys_ret = err;
> -
> -	return err;
> -}
> -
> -late_initcall(rtc_hctosys);
> 

^ permalink raw reply

* Re: [PATCH] rtc: ds1307: fix compile warning if RTC_DRV_DS1307_CENTURY isn't defined
From: Alexandre Belloni @ 2017-07-06 20:38 UTC (permalink / raw)
  To: Heiner Kallweit; +Cc: linux-rtc, Stephen Rothwell
In-Reply-To: <022ecfbf-c999-b76e-f8cf-1d02fb20237e@gmail.com>

On 06/07/2017 at 22:03:12 +0200, Heiner Kallweit wrote:
> Fix the following warning if RTC_DRV_DS1307_CENTURY isn't defined.
> 
> drivers/rtc/rtc-ds1307.c: In function 'ds1307_get_time':
> drivers/rtc/rtc-ds1307.c:342:26: warning: unused variable 'chip' [-Wunused-variable]
>   const struct chip_desc *chip = &chips[ds1307->type];
>                           ^
> Fixes: 436c93db60a4 ("rtc: ds1307: factor out century bit handling")
> Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> ---
>  drivers/rtc/rtc-ds1307.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 

I've squashed it in the original commit, thanks.


-- 
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

^ permalink raw reply

* [PATCH] rtc: ds1307: remove legacy check for "isil,irq2-can-wakeup-machine" property
From: Heiner Kallweit @ 2017-07-06 20:40 UTC (permalink / raw)
  To: Alexandre Belloni; +Cc: linux-rtc, devicetree@vger.kernel.org

Commit 8b44f5be20fd ("ARM: dts: armada: replace isil,irq2-can-wakeup-machine with wakeup-source property")
removed the last usage of "isil,irq2-can-wakeup-machine" almost
two years ago. So I think we can get rid of supporting this
legacy binding.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 Documentation/devicetree/bindings/power/wakeup-source.txt | 9 ++++-----
 Documentation/devicetree/bindings/rtc/isil,isl12057.txt   | 1 -
 drivers/rtc/rtc-ds1307.c                                  | 8 +-------
 3 files changed, 5 insertions(+), 13 deletions(-)

diff --git a/Documentation/devicetree/bindings/power/wakeup-source.txt b/Documentation/devicetree/bindings/power/wakeup-source.txt
index 963c6dfd..3c81f78b 100644
--- a/Documentation/devicetree/bindings/power/wakeup-source.txt
+++ b/Documentation/devicetree/bindings/power/wakeup-source.txt
@@ -20,13 +20,12 @@ List of legacy properties and respective binding document
 1. "enable-sdio-wakeup"		Documentation/devicetree/bindings/mmc/mmc.txt
 2. "gpio-key,wakeup"		Documentation/devicetree/bindings/input/gpio-keys{,-polled}.txt
 3. "has-tpo"			Documentation/devicetree/bindings/rtc/rtc-opal.txt
-4. "isil,irq2-can-wakeup-machine" Documentation/devicetree/bindings/rtc/isil,isl12057.txt
-5. "linux,wakeup"		Documentation/devicetree/bindings/input/gpio-matrix-keypad.txt
+4. "linux,wakeup"		Documentation/devicetree/bindings/input/gpio-matrix-keypad.txt
 				Documentation/devicetree/bindings/mfd/tc3589x.txt
 				Documentation/devicetree/bindings/input/ads7846.txt
-6. "linux,keypad-wakeup"	Documentation/devicetree/bindings/input/qcom,pm8xxx-keypad.txt
-7. "linux,input-wakeup"		Documentation/devicetree/bindings/input/samsung-keypad.txt
-8. "nvidia,wakeup-source"	Documentation/devicetree/bindings/input/nvidia,tegra20-kbc.txt
+5. "linux,keypad-wakeup"	Documentation/devicetree/bindings/input/qcom,pm8xxx-keypad.txt
+6. "linux,input-wakeup"		Documentation/devicetree/bindings/input/samsung-keypad.txt
+7. "nvidia,wakeup-source"	Documentation/devicetree/bindings/input/nvidia,tegra20-kbc.txt
 
 Examples
 --------
diff --git a/Documentation/devicetree/bindings/rtc/isil,isl12057.txt b/Documentation/devicetree/bindings/rtc/isil,isl12057.txt
index cf83e094..fbbdd92e 100644
--- a/Documentation/devicetree/bindings/rtc/isil,isl12057.txt
+++ b/Documentation/devicetree/bindings/rtc/isil,isl12057.txt
@@ -24,7 +24,6 @@ Optional properties:
 
  - "wakeup-source": mark the chip as a wakeup source, independently of
     the availability of an IRQ line connected to the SoC.
-    (Legacy property supported: "isil,irq2-can-wakeup-machine")
 
  - "interrupt-parent", "interrupts": for passing the interrupt line
    of the SoC connected to IRQ#2 of the RTC chip.
diff --git a/drivers/rtc/rtc-ds1307.c b/drivers/rtc/rtc-ds1307.c
index 18b6a6af..37720242 100644
--- a/drivers/rtc/rtc-ds1307.c
+++ b/drivers/rtc/rtc-ds1307.c
@@ -1396,14 +1396,8 @@ static int ds1307_probe(struct i2c_client *client,
  * This will guarantee the 'wakealarm' sysfs entry is available on the device,
  * if supported by the RTC.
  */
-	if (of_property_read_bool(client->dev.of_node, "wakeup-source")) {
+	if (of_property_read_bool(client->dev.of_node, "wakeup-source"))
 		ds1307_can_wakeup_device = true;
-	}
-	/* Intersil ISL12057 DT backward compatibility */
-	if (of_property_read_bool(client->dev.of_node,
-				  "isil,irq2-can-wakeup-machine")) {
-		ds1307_can_wakeup_device = true;
-	}
 #endif
 
 	switch (ds1307->type) {
-- 
2.13.2

^ permalink raw reply related

* [rtc-linux] Re: [PATCHv2 0/2] rtc: add support for STM32H7 RTC
From: Alexandre Belloni @ 2017-07-06 20:53 UTC (permalink / raw)
  To: Amelie Delaunay
  Cc: Alessandro Zummo, Rob Herring, Mark Rutland, Maxime Coquelin,
	Alexandre Torgue, rtc-linux, devicetree, linux-arm-kernel,
	linux-kernel
In-Reply-To: <1499330865-17232-1-git-send-email-amelie.delaunay@st.com>

On 06/07/2017 at 10:47:43 +0200, Amelie Delaunay wrote:
> 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                            | 82 +++++++++++++++++-----
>  2 files changed, 92 insertions(+), 22 deletions(-)
> 
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 v2 0/3] rtc: make st-lpc robust against y2038/2106 bug
From: Alexandre Belloni @ 2017-07-09 20:33 UTC (permalink / raw)
  To: Benjamin Gaignard
  Cc: john.stultz, tglx, sboyd, shuah, linux-kernel, linux-kselftest,
	patrice.chotard, a.zummo, linux-arm-kernel, rtc-linux,
	linaro-kernel
In-Reply-To: <1497864982-29284-1-git-send-email-benjamin.gaignard@linaro.org>

On 19/06/2017 at 11:36:19 +0200, 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
> 
Applied, thanks.

I've fixed up the date to meaningful ones in patch 2.

-- 
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

* [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

^ permalink raw reply related

* [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

^ permalink raw reply related

* Re: [PATCH] rtc: ds1307: remove legacy check for "isil,irq2-can-wakeup-machine" property
From: Rob Herring @ 2017-07-10 14:08 UTC (permalink / raw)
  To: Heiner Kallweit; +Cc: Alexandre Belloni, linux-rtc, devicetree@vger.kernel.org
In-Reply-To: <ff4ea92b-4af3-1a59-2ea3-056a43125c20@gmail.com>

On Thu, Jul 06, 2017 at 10:40:03PM +0200, Heiner Kallweit wrote:
> Commit 8b44f5be20fd ("ARM: dts: armada: replace isil,irq2-can-wakeup-machine with wakeup-source property")
> removed the last usage of "isil,irq2-can-wakeup-machine" almost
> two years ago. So I think we can get rid of supporting this
> legacy binding.
> 
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> ---
>  Documentation/devicetree/bindings/power/wakeup-source.txt | 9 ++++-----
>  Documentation/devicetree/bindings/rtc/isil,isl12057.txt   | 1 -
>  drivers/rtc/rtc-ds1307.c                                  | 8 +-------
>  3 files changed, 5 insertions(+), 13 deletions(-)

Acked-by: Rob Herring <robh@kernel.org>

^ permalink raw reply

* [PATCH 0/9] rtc: ds1307: series with refactorings / improvements
From: Heiner Kallweit @ 2017-07-12  5:23 UTC (permalink / raw)
  To: Alexandre Belloni; +Cc: linux-rtc

This series includes smaller refactorings and improvements.

Heiner Kallweit (9):
  rtc: ds1307: remove member irq from struct ds1307
  rtc: ds1307: factor out bbsqi bit to struct chip_desc
  rtc: ds1307: improve trickle charger initialization
  rtc: ds1307: constify struct chip_desc variables
  rtc: ds1307: improve irq setup
  rtc: ds1307: factor out irq_handler to struct chip_desc
  rtc: ds1307: factor out rtc_ops to struct chip_desc
  rtc: ds1307: factor out offset to struct chip_desc
  rtc: ds1307: remove member nvram_offset from struct ds1307

 drivers/rtc/rtc-ds1307.c | 165 ++++++++++++++++++++++-------------------------
 1 file changed, 76 insertions(+), 89 deletions(-)

-- 
2.13.2

^ permalink raw reply

* [PATCH 1/9] rtc: ds1307: remove member irq from struct ds1307
From: Heiner Kallweit @ 2017-07-12  5:49 UTC (permalink / raw)
  To: Alexandre Belloni; +Cc: linux-rtc
In-Reply-To: <aded2895-cf4b-12f0-5525-c4389ccf3650@gmail.com>

The irq number is used in the probe function only, so we don't have
to store it in struct ds1307.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/rtc/rtc-ds1307.c | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/drivers/rtc/rtc-ds1307.c b/drivers/rtc/rtc-ds1307.c
index 1cedb21b..adc90f18 100644
--- a/drivers/rtc/rtc-ds1307.c
+++ b/drivers/rtc/rtc-ds1307.c
@@ -126,7 +126,6 @@ struct ds1307 {
 	struct device		*dev;
 	struct regmap		*regmap;
 	const char		*name;
-	int			irq;
 	struct rtc_device	*rtc;
 #ifdef CONFIG_COMMON_CLK
 	struct clk_hw		clks[2];
@@ -1334,7 +1333,6 @@ static int ds1307_probe(struct i2c_client *client,
 	dev_set_drvdata(&client->dev, ds1307);
 	ds1307->dev = &client->dev;
 	ds1307->name = client->name;
-	ds1307->irq = client->irq;
 
 	ds1307->regmap = devm_regmap_init_i2c(client, &regmap_config);
 	if (IS_ERR(ds1307->regmap)) {
@@ -1414,7 +1412,7 @@ static int ds1307_probe(struct i2c_client *client,
 		 * For some variants, be sure alarms can trigger when we're
 		 * running on Vbackup (BBSQI/BBSQW)
 		 */
-		if (chip->alarm && (ds1307->irq > 0 ||
+		if (chip->alarm && (client->irq > 0 ||
 				    ds1307_can_wakeup_device)) {
 			ds1307->regs[0] |= DS1337_BIT_INTCN
 					| bbsqi_bitpos[ds1307->type];
@@ -1499,7 +1497,7 @@ static int ds1307_probe(struct i2c_client *client,
 	case rx_8130:
 		ds1307->offset = 0x10; /* Seconds starts at 0x10 */
 		rtc_ops = &rx8130_rtc_ops;
-		if (chip->alarm && ds1307->irq > 0) {
+		if (chip->alarm && client->irq > 0) {
 			irq_handler = rx8130_irq;
 			want_irq = true;
 		}
@@ -1509,7 +1507,7 @@ static int ds1307_probe(struct i2c_client *client,
 		break;
 	case mcp794xx:
 		rtc_ops = &mcp794xx_rtc_ops;
-		if (chip->alarm && (ds1307->irq > 0 ||
+		if (chip->alarm && (client->irq > 0 ||
 				    ds1307_can_wakeup_device)) {
 			irq_handler = mcp794xx_irq;
 			want_irq = true;
@@ -1655,7 +1653,7 @@ static int ds1307_probe(struct i2c_client *client,
 		return PTR_ERR(ds1307->rtc);
 	}
 
-	if (ds1307_can_wakeup_device && ds1307->irq <= 0) {
+	if (ds1307_can_wakeup_device && client->irq <= 0) {
 		/* Disable request for an IRQ */
 		want_irq = false;
 		dev_info(ds1307->dev,
@@ -1666,7 +1664,7 @@ static int ds1307_probe(struct i2c_client *client,
 
 	if (want_irq) {
 		err = devm_request_threaded_irq(ds1307->dev,
-						ds1307->irq, NULL, irq_handler,
+						client->irq, NULL, irq_handler,
 						IRQF_SHARED | IRQF_ONESHOT,
 						ds1307->name, ds1307);
 		if (err) {
-- 
2.13.2

^ permalink raw reply related

* [PATCH 2/9] rtc: ds1307: factor out bbsqi bit to struct chip_desc
From: Heiner Kallweit @ 2017-07-12  5:49 UTC (permalink / raw)
  To: Alexandre Belloni; +Cc: linux-rtc
In-Reply-To: <aded2895-cf4b-12f0-5525-c4389ccf3650@gmail.com>

Factor out the bbsqi bit to struct chip_desc.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/rtc/rtc-ds1307.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/drivers/rtc/rtc-ds1307.c b/drivers/rtc/rtc-ds1307.c
index adc90f18..eecf6b27 100644
--- a/drivers/rtc/rtc-ds1307.c
+++ b/drivers/rtc/rtc-ds1307.c
@@ -139,6 +139,7 @@ struct chip_desc {
 	u8			century_reg;
 	u8			century_enable_bit;
 	u8			century_bit;
+	u8			bbsqi_bit;
 	u16			trickle_charger_reg;
 	u8			trickle_charger_setup;
 	u8			(*do_trickle_setup)(struct ds1307 *, uint32_t,
@@ -169,6 +170,7 @@ static struct chip_desc chips[last_ds_type] = {
 		.alarm		= 1,
 		.century_reg	= DS1307_REG_MONTH,
 		.century_bit	= DS1337_BIT_CENTURY,
+		.bbsqi_bit	= DS1339_BIT_BBSQI,
 		.trickle_charger_reg = 0x10,
 		.do_trickle_setup = &do_trickle_setup_ds1339,
 	},
@@ -185,6 +187,7 @@ static struct chip_desc chips[last_ds_type] = {
 		.alarm		= 1,
 		.century_reg	= DS1307_REG_MONTH,
 		.century_bit	= DS1337_BIT_CENTURY,
+		.bbsqi_bit	= DS3231_BIT_BBSQW,
 	},
 	[rx_8130] = {
 		.alarm		= 1,
@@ -1319,11 +1322,6 @@ static int ds1307_probe(struct i2c_client *client,
 
 	irq_handler_t	irq_handler = ds1307_irq;
 
-	static const int	bbsqi_bitpos[] = {
-		[ds_1337] = 0,
-		[ds_1339] = DS1339_BIT_BBSQI,
-		[ds_3231] = DS3231_BIT_BBSQW,
-	};
 	const struct rtc_class_ops *rtc_ops = &ds13xx_rtc_ops;
 
 	ds1307 = devm_kzalloc(&client->dev, sizeof(struct ds1307), GFP_KERNEL);
@@ -1414,8 +1412,7 @@ static int ds1307_probe(struct i2c_client *client,
 		 */
 		if (chip->alarm && (client->irq > 0 ||
 				    ds1307_can_wakeup_device)) {
-			ds1307->regs[0] |= DS1337_BIT_INTCN
-					| bbsqi_bitpos[ds1307->type];
+			ds1307->regs[0] |= DS1337_BIT_INTCN | chip->bbsqi_bit;
 			ds1307->regs[0] &= ~(DS1337_BIT_A2IE | DS1337_BIT_A1IE);
 
 			want_irq = true;
-- 
2.13.2

^ permalink raw reply related

* [PATCH 3/9] rtc: ds1307: improve trickle charger initialization
From: Heiner Kallweit @ 2017-07-12  5:49 UTC (permalink / raw)
  To: Alexandre Belloni; +Cc: linux-rtc
In-Reply-To: <aded2895-cf4b-12f0-5525-c4389ccf3650@gmail.com>

Instead of storing the trickle_charger_setup value in struct chip_desc
we can let function ds1307_trickle_init return it because it's used
in the probe function only.
This allows us to constify struct chip_desc variables in a next step.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/rtc/rtc-ds1307.c | 33 ++++++++++++++++-----------------
 1 file changed, 16 insertions(+), 17 deletions(-)

diff --git a/drivers/rtc/rtc-ds1307.c b/drivers/rtc/rtc-ds1307.c
index eecf6b27..d7158c9d 100644
--- a/drivers/rtc/rtc-ds1307.c
+++ b/drivers/rtc/rtc-ds1307.c
@@ -141,7 +141,6 @@ struct chip_desc {
 	u8			century_bit;
 	u8			bbsqi_bit;
 	u16			trickle_charger_reg;
-	u8			trickle_charger_setup;
 	u8			(*do_trickle_setup)(struct ds1307 *, uint32_t,
 						    bool);
 };
@@ -941,23 +940,23 @@ static u8 do_trickle_setup_ds1339(struct ds1307 *ds1307,
 	return setup;
 }
 
-static void ds1307_trickle_init(struct ds1307 *ds1307,
-				struct chip_desc *chip)
+static u8 ds1307_trickle_init(struct ds1307 *ds1307,
+			      struct chip_desc *chip)
 {
-	uint32_t ohms = 0;
+	uint32_t ohms;
 	bool diode = true;
 
 	if (!chip->do_trickle_setup)
-		goto out;
+		return 0;
+
 	if (device_property_read_u32(ds1307->dev, "trickle-resistor-ohms",
 				     &ohms))
-		goto out;
+		return 0;
+
 	if (device_property_read_bool(ds1307->dev, "trickle-diode-disable"))
 		diode = false;
-	chip->trickle_charger_setup = chip->do_trickle_setup(ds1307,
-							     ohms, diode);
-out:
-	return;
+
+	return chip->do_trickle_setup(ds1307, ohms, diode);
 }
 
 /*----------------------------------------------------------------------*/
@@ -1319,6 +1318,7 @@ static int ds1307_probe(struct i2c_client *client,
 	struct ds1307_platform_data *pdata = dev_get_platdata(&client->dev);
 	struct rtc_time		tm;
 	unsigned long		timestamp;
+	u8 			trickle_charger_setup = 0;
 
 	irq_handler_t	irq_handler = ds1307_irq;
 
@@ -1359,18 +1359,17 @@ static int ds1307_probe(struct i2c_client *client,
 	}
 
 	if (!pdata)
-		ds1307_trickle_init(ds1307, chip);
+		trickle_charger_setup = ds1307_trickle_init(ds1307, chip);
 	else if (pdata->trickle_charger_setup)
-		chip->trickle_charger_setup = pdata->trickle_charger_setup;
+		trickle_charger_setup = pdata->trickle_charger_setup;
 
-	if (chip->trickle_charger_setup && chip->trickle_charger_reg) {
+	if (trickle_charger_setup && chip->trickle_charger_reg) {
+		trickle_charger_setup |= DS13XX_TRICKLE_CHARGER_MAGIC;
 		dev_dbg(ds1307->dev,
 			"writing trickle charger info 0x%x to 0x%x\n",
-		    DS13XX_TRICKLE_CHARGER_MAGIC | chip->trickle_charger_setup,
-		    chip->trickle_charger_reg);
+			trickle_charger_setup, chip->trickle_charger_reg);
 		regmap_write(ds1307->regmap, chip->trickle_charger_reg,
-		    DS13XX_TRICKLE_CHARGER_MAGIC |
-		    chip->trickle_charger_setup);
+			     trickle_charger_setup);
 	}
 
 	buf = ds1307->regs;
-- 
2.13.2

^ permalink raw reply related

* [PATCH 4/9] rtc: ds1307: constify struct chip_desc variables
From: Heiner Kallweit @ 2017-07-12  5:49 UTC (permalink / raw)
  To: Alexandre Belloni; +Cc: linux-rtc
In-Reply-To: <aded2895-cf4b-12f0-5525-c4389ccf3650@gmail.com>

Constify struct chip_desc variables.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/rtc/rtc-ds1307.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/rtc/rtc-ds1307.c b/drivers/rtc/rtc-ds1307.c
index d7158c9d..115fe1c5 100644
--- a/drivers/rtc/rtc-ds1307.c
+++ b/drivers/rtc/rtc-ds1307.c
@@ -147,7 +147,7 @@ struct chip_desc {
 
 static u8 do_trickle_setup_ds1339(struct ds1307 *, uint32_t ohms, bool diode);
 
-static struct chip_desc chips[last_ds_type] = {
+static const struct chip_desc chips[last_ds_type] = {
 	[ds_1307] = {
 		.nvram_offset	= 8,
 		.nvram_size	= 56,
@@ -941,7 +941,7 @@ static u8 do_trickle_setup_ds1339(struct ds1307 *ds1307,
 }
 
 static u8 ds1307_trickle_init(struct ds1307 *ds1307,
-			      struct chip_desc *chip)
+			      const struct chip_desc *chip)
 {
 	uint32_t ohms;
 	bool diode = true;
@@ -1311,7 +1311,7 @@ static int ds1307_probe(struct i2c_client *client,
 	struct ds1307		*ds1307;
 	int			err = -ENODEV;
 	int			tmp, wday;
-	struct chip_desc	*chip;
+	const struct chip_desc	*chip;
 	bool			want_irq = false;
 	bool			ds1307_can_wakeup_device = false;
 	unsigned char		*buf;
-- 
2.13.2

^ permalink raw reply related

* [PATCH 5/9] rtc: ds1307: improve irq setup
From: Heiner Kallweit @ 2017-07-12  5:49 UTC (permalink / raw)
  To: Alexandre Belloni; +Cc: linux-rtc
In-Reply-To: <aded2895-cf4b-12f0-5525-c4389ccf3650@gmail.com>

Change the usage of variable want_irq to reflect its name. Don't set
it to true in case wakeup is enabled but no interrupt number is given.
In addition set variable ds1307_can_wakeup_device if chip->alarm
is set only.
This allows to simplify the code and make it better understandable.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/rtc/rtc-ds1307.c | 27 ++++++++++-----------------
 1 file changed, 10 insertions(+), 17 deletions(-)

diff --git a/drivers/rtc/rtc-ds1307.c b/drivers/rtc/rtc-ds1307.c
index 115fe1c5..5aec1761 100644
--- a/drivers/rtc/rtc-ds1307.c
+++ b/drivers/rtc/rtc-ds1307.c
@@ -1312,7 +1312,7 @@ static int ds1307_probe(struct i2c_client *client,
 	int			err = -ENODEV;
 	int			tmp, wday;
 	const struct chip_desc	*chip;
-	bool			want_irq = false;
+	bool			want_irq;
 	bool			ds1307_can_wakeup_device = false;
 	unsigned char		*buf;
 	struct ds1307_platform_data *pdata = dev_get_platdata(&client->dev);
@@ -1358,6 +1358,8 @@ static int ds1307_probe(struct i2c_client *client,
 		ds1307->type = acpi_id->driver_data;
 	}
 
+	want_irq = client->irq > 0 && chip->alarm;
+
 	if (!pdata)
 		trickle_charger_setup = ds1307_trickle_init(ds1307, chip);
 	else if (pdata->trickle_charger_setup)
@@ -1383,7 +1385,8 @@ static int ds1307_probe(struct i2c_client *client,
  * This will guarantee the 'wakealarm' sysfs entry is available on the device,
  * if supported by the RTC.
  */
-	if (of_property_read_bool(client->dev.of_node, "wakeup-source"))
+	if (chip->alarm && of_property_read_bool(client->dev.of_node,
+	    "wakeup-source"))
 		ds1307_can_wakeup_device = true;
 #endif
 
@@ -1409,12 +1412,9 @@ static int ds1307_probe(struct i2c_client *client,
 		 * For some variants, be sure alarms can trigger when we're
 		 * running on Vbackup (BBSQI/BBSQW)
 		 */
-		if (chip->alarm && (client->irq > 0 ||
-				    ds1307_can_wakeup_device)) {
+		if (want_irq || ds1307_can_wakeup_device) {
 			ds1307->regs[0] |= DS1337_BIT_INTCN | chip->bbsqi_bit;
 			ds1307->regs[0] &= ~(DS1337_BIT_A2IE | DS1337_BIT_A1IE);
-
-			want_irq = true;
 		}
 
 		regmap_write(ds1307->regmap, DS1337_REG_CONTROL,
@@ -1493,21 +1493,16 @@ static int ds1307_probe(struct i2c_client *client,
 	case rx_8130:
 		ds1307->offset = 0x10; /* Seconds starts at 0x10 */
 		rtc_ops = &rx8130_rtc_ops;
-		if (chip->alarm && client->irq > 0) {
+		if (want_irq)
 			irq_handler = rx8130_irq;
-			want_irq = true;
-		}
 		break;
 	case ds_1388:
 		ds1307->offset = 1; /* Seconds starts at 1 */
 		break;
 	case mcp794xx:
 		rtc_ops = &mcp794xx_rtc_ops;
-		if (chip->alarm && (client->irq > 0 ||
-				    ds1307_can_wakeup_device)) {
+		if (want_irq || ds1307_can_wakeup_device)
 			irq_handler = mcp794xx_irq;
-			want_irq = true;
-		}
 		break;
 	default:
 		break;
@@ -1639,7 +1634,7 @@ static int ds1307_probe(struct i2c_client *client,
 				   MCP794XX_REG_WEEKDAY_WDAY_MASK,
 				   tm.tm_wday + 1);
 
-	if (want_irq) {
+	if (want_irq || ds1307_can_wakeup_device) {
 		device_set_wakeup_capable(ds1307->dev, true);
 		set_bit(HAS_ALARM, &ds1307->flags);
 	}
@@ -1649,9 +1644,7 @@ static int ds1307_probe(struct i2c_client *client,
 		return PTR_ERR(ds1307->rtc);
 	}
 
-	if (ds1307_can_wakeup_device && client->irq <= 0) {
-		/* Disable request for an IRQ */
-		want_irq = false;
+	if (ds1307_can_wakeup_device && !want_irq) {
 		dev_info(ds1307->dev,
 			 "'wakeup-source' is set, request for an IRQ is disabled!\n");
 		/* We cannot support UIE mode if we do not have an IRQ line */
-- 
2.13.2

^ permalink raw reply related

* [PATCH 6/9] rtc: ds1307: factor out irq_handler to struct chip_desc
From: Heiner Kallweit @ 2017-07-12  5:49 UTC (permalink / raw)
  To: Alexandre Belloni; +Cc: linux-rtc
In-Reply-To: <aded2895-cf4b-12f0-5525-c4389ccf3650@gmail.com>

Factor out irq_handler to struct chip_desc and use ds1307_irq as default.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/rtc/rtc-ds1307.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/drivers/rtc/rtc-ds1307.c b/drivers/rtc/rtc-ds1307.c
index 5aec1761..e64ea250 100644
--- a/drivers/rtc/rtc-ds1307.c
+++ b/drivers/rtc/rtc-ds1307.c
@@ -140,12 +140,15 @@ struct chip_desc {
 	u8			century_enable_bit;
 	u8			century_bit;
 	u8			bbsqi_bit;
+	irq_handler_t		irq_handler;
 	u16			trickle_charger_reg;
 	u8			(*do_trickle_setup)(struct ds1307 *, uint32_t,
 						    bool);
 };
 
 static u8 do_trickle_setup_ds1339(struct ds1307 *, uint32_t ohms, bool diode);
+static irqreturn_t rx8130_irq(int irq, void *dev_id);
+static irqreturn_t mcp794xx_irq(int irq, void *dev_id);
 
 static const struct chip_desc chips[last_ds_type] = {
 	[ds_1307] = {
@@ -193,12 +196,14 @@ static const struct chip_desc chips[last_ds_type] = {
 		/* this is battery backed SRAM */
 		.nvram_offset	= 0x20,
 		.nvram_size	= 4,	/* 32bit (4 word x 8 bit) */
+		.irq_handler = rx8130_irq,
 	},
 	[mcp794xx] = {
 		.alarm		= 1,
 		/* this is battery backed SRAM */
 		.nvram_offset	= 0x20,
 		.nvram_size	= 0x40,
+		.irq_handler = mcp794xx_irq,
 	},
 };
 
@@ -1320,8 +1325,6 @@ static int ds1307_probe(struct i2c_client *client,
 	unsigned long		timestamp;
 	u8 			trickle_charger_setup = 0;
 
-	irq_handler_t	irq_handler = ds1307_irq;
-
 	const struct rtc_class_ops *rtc_ops = &ds13xx_rtc_ops;
 
 	ds1307 = devm_kzalloc(&client->dev, sizeof(struct ds1307), GFP_KERNEL);
@@ -1493,16 +1496,12 @@ static int ds1307_probe(struct i2c_client *client,
 	case rx_8130:
 		ds1307->offset = 0x10; /* Seconds starts at 0x10 */
 		rtc_ops = &rx8130_rtc_ops;
-		if (want_irq)
-			irq_handler = rx8130_irq;
 		break;
 	case ds_1388:
 		ds1307->offset = 1; /* Seconds starts at 1 */
 		break;
 	case mcp794xx:
 		rtc_ops = &mcp794xx_rtc_ops;
-		if (want_irq || ds1307_can_wakeup_device)
-			irq_handler = mcp794xx_irq;
 		break;
 	default:
 		break;
@@ -1652,8 +1651,8 @@ static int ds1307_probe(struct i2c_client *client,
 	}
 
 	if (want_irq) {
-		err = devm_request_threaded_irq(ds1307->dev,
-						client->irq, NULL, irq_handler,
+		err = devm_request_threaded_irq(ds1307->dev, client->irq, NULL,
+						chip->irq_handler ?: ds1307_irq,
 						IRQF_SHARED | IRQF_ONESHOT,
 						ds1307->name, ds1307);
 		if (err) {
-- 
2.13.2

^ permalink raw reply related

* [PATCH 7/9] rtc: ds1307: factor out rtc_ops to struct chip_desc
From: Heiner Kallweit @ 2017-07-12  5:49 UTC (permalink / raw)
  To: Alexandre Belloni; +Cc: linux-rtc
In-Reply-To: <aded2895-cf4b-12f0-5525-c4389ccf3650@gmail.com>

Factor out rtc_ops to struct chip_desc and use ds13xx_rtc_ops as default.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/rtc/rtc-ds1307.c | 51 ++++++++++++++++++++++++++----------------------
 1 file changed, 28 insertions(+), 23 deletions(-)

diff --git a/drivers/rtc/rtc-ds1307.c b/drivers/rtc/rtc-ds1307.c
index e64ea250..4083464f 100644
--- a/drivers/rtc/rtc-ds1307.c
+++ b/drivers/rtc/rtc-ds1307.c
@@ -141,14 +141,39 @@ struct chip_desc {
 	u8			century_bit;
 	u8			bbsqi_bit;
 	irq_handler_t		irq_handler;
+	const struct rtc_class_ops *rtc_ops;
 	u16			trickle_charger_reg;
 	u8			(*do_trickle_setup)(struct ds1307 *, uint32_t,
 						    bool);
 };
 
+static int ds1307_get_time(struct device *dev, struct rtc_time *t);
+static int ds1307_set_time(struct device *dev, struct rtc_time *t);
 static u8 do_trickle_setup_ds1339(struct ds1307 *, uint32_t ohms, bool diode);
 static irqreturn_t rx8130_irq(int irq, void *dev_id);
+static int rx8130_read_alarm(struct device *dev, struct rtc_wkalrm *t);
+static int rx8130_set_alarm(struct device *dev, struct rtc_wkalrm *t);
+static int rx8130_alarm_irq_enable(struct device *dev, unsigned int enabled);
 static irqreturn_t mcp794xx_irq(int irq, void *dev_id);
+static int mcp794xx_read_alarm(struct device *dev, struct rtc_wkalrm *t);
+static int mcp794xx_set_alarm(struct device *dev, struct rtc_wkalrm *t);
+static int mcp794xx_alarm_irq_enable(struct device *dev, unsigned int enabled);
+
+static const struct rtc_class_ops rx8130_rtc_ops = {
+        .read_time      = ds1307_get_time,
+        .set_time       = ds1307_set_time,
+        .read_alarm     = rx8130_read_alarm,
+        .set_alarm      = rx8130_set_alarm,
+        .alarm_irq_enable = rx8130_alarm_irq_enable,
+};
+
+static const struct rtc_class_ops mcp794xx_rtc_ops = {
+        .read_time      = ds1307_get_time,
+        .set_time       = ds1307_set_time,
+        .read_alarm     = mcp794xx_read_alarm,
+        .set_alarm      = mcp794xx_set_alarm,
+        .alarm_irq_enable = mcp794xx_alarm_irq_enable,
+};
 
 static const struct chip_desc chips[last_ds_type] = {
 	[ds_1307] = {
@@ -197,6 +222,7 @@ static const struct chip_desc chips[last_ds_type] = {
 		.nvram_offset	= 0x20,
 		.nvram_size	= 4,	/* 32bit (4 word x 8 bit) */
 		.irq_handler = rx8130_irq,
+		.rtc_ops = &rx8130_rtc_ops,
 	},
 	[mcp794xx] = {
 		.alarm		= 1,
@@ -204,6 +230,7 @@ static const struct chip_desc chips[last_ds_type] = {
 		.nvram_offset	= 0x20,
 		.nvram_size	= 0x40,
 		.irq_handler = mcp794xx_irq,
+		.rtc_ops = &mcp794xx_rtc_ops,
 	},
 };
 
@@ -731,14 +758,6 @@ static int rx8130_alarm_irq_enable(struct device *dev, unsigned int enabled)
 	return regmap_write(ds1307->regmap, RX8130_REG_CONTROL0, reg);
 }
 
-static const struct rtc_class_ops rx8130_rtc_ops = {
-	.read_time	= ds1307_get_time,
-	.set_time	= ds1307_set_time,
-	.read_alarm	= rx8130_read_alarm,
-	.set_alarm	= rx8130_set_alarm,
-	.alarm_irq_enable = rx8130_alarm_irq_enable,
-};
-
 /*----------------------------------------------------------------------*/
 
 /*
@@ -891,14 +910,6 @@ static int mcp794xx_alarm_irq_enable(struct device *dev, unsigned int enabled)
 				  enabled ? MCP794XX_BIT_ALM0_EN : 0);
 }
 
-static const struct rtc_class_ops mcp794xx_rtc_ops = {
-	.read_time	= ds1307_get_time,
-	.set_time	= ds1307_set_time,
-	.read_alarm	= mcp794xx_read_alarm,
-	.set_alarm	= mcp794xx_set_alarm,
-	.alarm_irq_enable = mcp794xx_alarm_irq_enable,
-};
-
 /*----------------------------------------------------------------------*/
 
 static int ds1307_nvram_read(void *priv, unsigned int offset, void *val,
@@ -1325,8 +1336,6 @@ static int ds1307_probe(struct i2c_client *client,
 	unsigned long		timestamp;
 	u8 			trickle_charger_setup = 0;
 
-	const struct rtc_class_ops *rtc_ops = &ds13xx_rtc_ops;
-
 	ds1307 = devm_kzalloc(&client->dev, sizeof(struct ds1307), GFP_KERNEL);
 	if (!ds1307)
 		return -ENOMEM;
@@ -1495,14 +1504,10 @@ static int ds1307_probe(struct i2c_client *client,
 		break;
 	case rx_8130:
 		ds1307->offset = 0x10; /* Seconds starts at 0x10 */
-		rtc_ops = &rx8130_rtc_ops;
 		break;
 	case ds_1388:
 		ds1307->offset = 1; /* Seconds starts at 1 */
 		break;
-	case mcp794xx:
-		rtc_ops = &mcp794xx_rtc_ops;
-		break;
 	default:
 		break;
 	}
@@ -1678,7 +1683,7 @@ static int ds1307_probe(struct i2c_client *client,
 		ds1307->rtc->nvram_old_abi = true;
 	}
 
-	ds1307->rtc->ops = rtc_ops;
+	ds1307->rtc->ops = chip->rtc_ops ?: &ds13xx_rtc_ops;
 	err = rtc_register_device(ds1307->rtc);
 	if (err)
 		return err;
-- 
2.13.2

^ permalink raw reply related

* [PATCH 8/9] rtc: ds1307: factor out offset to struct chip_desc
From: Heiner Kallweit @ 2017-07-12  5:49 UTC (permalink / raw)
  To: Alexandre Belloni; +Cc: linux-rtc
In-Reply-To: <aded2895-cf4b-12f0-5525-c4389ccf3650@gmail.com>

Factor out offset to struct chip_desc and remove it from struct ds1307.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/rtc/rtc-ds1307.c | 18 +++++++-----------
 1 file changed, 7 insertions(+), 11 deletions(-)

diff --git a/drivers/rtc/rtc-ds1307.c b/drivers/rtc/rtc-ds1307.c
index 4083464f..43babd74 100644
--- a/drivers/rtc/rtc-ds1307.c
+++ b/drivers/rtc/rtc-ds1307.c
@@ -115,7 +115,6 @@ enum ds_type {
 
 
 struct ds1307 {
-	u8			offset; /* register's offset */
 	u8			regs[11];
 	u16			nvram_offset;
 	struct nvmem_config	nvmem_cfg;
@@ -136,6 +135,7 @@ struct chip_desc {
 	unsigned		alarm:1;
 	u16			nvram_offset;
 	u16			nvram_size;
+	u8			offset; /* register's offset */
 	u8			century_reg;
 	u8			century_enable_bit;
 	u8			century_bit;
@@ -208,6 +208,7 @@ static const struct chip_desc chips[last_ds_type] = {
 		.trickle_charger_reg = 0x08,
 	},
 	[ds_1388] = {
+		.offset		= 1,
 		.trickle_charger_reg = 0x0a,
 	},
 	[ds_3231] = {
@@ -221,6 +222,7 @@ static const struct chip_desc chips[last_ds_type] = {
 		/* this is battery backed SRAM */
 		.nvram_offset	= 0x20,
 		.nvram_size	= 4,	/* 32bit (4 word x 8 bit) */
+		.offset		= 0x10,
 		.irq_handler = rx8130_irq,
 		.rtc_ops = &rx8130_rtc_ops,
 	},
@@ -387,7 +389,7 @@ static int ds1307_get_time(struct device *dev, struct rtc_time *t)
 	const struct chip_desc *chip = &chips[ds1307->type];
 
 	/* read the RTC date and time registers all at once */
-	ret = regmap_bulk_read(ds1307->regmap, ds1307->offset, ds1307->regs, 7);
+	ret = regmap_bulk_read(ds1307->regmap, chip->offset, ds1307->regs, 7);
 	if (ret) {
 		dev_err(dev, "%s error %d\n", "read", ret);
 		return ret;
@@ -479,7 +481,7 @@ static int ds1307_set_time(struct device *dev, struct rtc_time *t)
 
 	dev_dbg(dev, "%s: %7ph\n", "write", buf);
 
-	result = regmap_bulk_write(ds1307->regmap, ds1307->offset, buf, 7);
+	result = regmap_bulk_write(ds1307->regmap, chip->offset, buf, 7);
 	if (result) {
 		dev_err(dev, "%s error %d\n", "write", result);
 		return result;
@@ -1502,19 +1504,13 @@ static int ds1307_probe(struct i2c_client *client,
 				     DS1307_REG_HOUR << 4 | 0x08, hour);
 		}
 		break;
-	case rx_8130:
-		ds1307->offset = 0x10; /* Seconds starts at 0x10 */
-		break;
-	case ds_1388:
-		ds1307->offset = 1; /* Seconds starts at 1 */
-		break;
 	default:
 		break;
 	}
 
 read_rtc:
 	/* read RTC registers */
-	err = regmap_bulk_read(ds1307->regmap, ds1307->offset, buf, 8);
+	err = regmap_bulk_read(ds1307->regmap, chip->offset, buf, 8);
 	if (err) {
 		dev_dbg(ds1307->dev, "read error %d\n", err);
 		goto exit;
@@ -1615,7 +1611,7 @@ static int ds1307_probe(struct i2c_client *client,
 			tmp = 0;
 		if (ds1307->regs[DS1307_REG_HOUR] & DS1307_BIT_PM)
 			tmp += 12;
-		regmap_write(ds1307->regmap, ds1307->offset + DS1307_REG_HOUR,
+		regmap_write(ds1307->regmap, chip->offset + DS1307_REG_HOUR,
 			     bin2bcd(tmp));
 	}
 
-- 
2.13.2

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox