subject:"\[PATCH\] rtc\: Add a driver for Micro Crystal RV8803"

Re: [PATCH] rtc: Add a driver for Micro Crystal RV8803

2015-09-28 Thread kbuild test robot

Hi Alexandre,

[auto build test results on v4.3-rc2 -- if it's inappropriate base, please 
ignore]


coccinelle warnings: (new ones prefixed by >>)

>> drivers/rtc/rtc-rv8803.c:494:3-8: No need to set .owner here. The core will 
>> do it.
--
>> drivers/rtc/rtc-rv8803.c:350:2-5: WARNING: end returns can be simpified if 
>> negative or 0 value

Please review and possibly fold the followup patch.

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] rtc: Add a driver for Micro Crystal RV8803

2015-09-28 Thread kbuild test robot

Hi Alexandre,

[auto build test results on v4.3-rc2 -- if it's inappropriate base, please 
ignore]


coccinelle warnings: (new ones prefixed by >>)

>> drivers/rtc/rtc-rv8803.c:494:3-8: No need to set .owner here. The core will 
>> do it.
--
>> drivers/rtc/rtc-rv8803.c:350:2-5: WARNING: end returns can be simpified if 
>> negative or 0 value

Please review and possibly fold the followup patch.

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] rtc: Add a driver for Micro Crystal RV8803

2015-09-27 Thread Josh Cartwright

Hello Alexandre-

Few comments below.

On Sat, Sep 26, 2015 at 03:54:39PM +0200, Alexandre Belloni wrote:
> This driver supports the following functions:
>  - reading and settings time
>  - alarms when connected to an IRQ
>  - reading and clearing the voltage low flags
>  - nvram
> 
> Signed-off-by: Alexandre Belloni 
> ---
[..]
> +static irqreturn_t rv8803_handle_irq(int irq, void *dev_id)
> +{
> + struct i2c_client *client = dev_id;
> + struct rv8803_data *rv8803 = i2c_get_clientdata(client);
> + unsigned long events = 0;
> + u8 flags;
> +
> + flags = i2c_smbus_read_byte_data(client, RV8803_FLAG);
> + if (flags <= 0)
> + return IRQ_HANDLED;

Returning IRQ_HANDLED when no interrupt condition is met.  That seems
like a bad idea.

> + if (flags & RV8803_FLAG_V1F)
> + dev_warn(>dev, "Voltage low, temperature compensation 
> stopped.\n");
> +
> + if (flags & RV8803_FLAG_V2F)
> + dev_warn(>dev, "Voltage low, data loss detected.\n");
> +
> + if (flags & RV8803_FLAG_TF) {
> + flags &= ~RV8803_FLAG_TF;
> + rv8803->ctrl &= ~RV8803_CTRL_TIE;
> + events |= RTC_PF;
> + }
> +
> + if (flags & RV8803_FLAG_AF) {
> + flags &= ~RV8803_FLAG_AF;
> + rv8803->ctrl &= ~RV8803_CTRL_AIE;
> + events |= RTC_AF;
> + }
> +
> + if (flags & RV8803_FLAG_UF) {
> + flags &= ~RV8803_FLAG_UF;
> + rv8803->ctrl &= ~RV8803_CTRL_UIE;
> + events |= RTC_UF;
> + }
> +
> + if (events) {
> + rtc_update_irq(rv8803->rtc, 1, events);
> + i2c_smbus_write_byte_data(client, RV8803_FLAG, flags);

How are the many read-modify-write cycles for flags safe without any
form of synchronization?  (Especially given the interrupt handler isn't
under ops_lock).

> + i2c_smbus_write_byte_data(rv8803->client, RV8803_CTRL,
> +   rv8803->ctrl);
> + }
> +
> + return IRQ_HANDLED;
> +}
> +
[..]
> +static int rv8803_get_alarm(struct device *dev, struct rtc_wkalrm *alrm)
> +{
> + struct rv8803_data *rv8803 = dev_get_drvdata(dev);
> + struct i2c_client *client = rv8803->client;
> + u8 alarmvals[3];
> + int flags, ret;
> +
> + if (client->irq <= 0)
> + return -EINVAL;

It'd be cleaner just to have a second set of rtc_class_ops that can be
switched between based on whether a valid interrupt is specified.

  Josh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] rtc: Add a driver for Micro Crystal RV8803

2015-09-27 Thread Josh Cartwright

Hello Alexandre-

Few comments below.

On Sat, Sep 26, 2015 at 03:54:39PM +0200, Alexandre Belloni wrote:
> This driver supports the following functions:
>  - reading and settings time
>  - alarms when connected to an IRQ
>  - reading and clearing the voltage low flags
>  - nvram
> 
> Signed-off-by: Alexandre Belloni 
> ---
[..]
> +static irqreturn_t rv8803_handle_irq(int irq, void *dev_id)
> +{
> + struct i2c_client *client = dev_id;
> + struct rv8803_data *rv8803 = i2c_get_clientdata(client);
> + unsigned long events = 0;
> + u8 flags;
> +
> + flags = i2c_smbus_read_byte_data(client, RV8803_FLAG);
> + if (flags <= 0)
> + return IRQ_HANDLED;

Returning IRQ_HANDLED when no interrupt condition is met.  That seems
like a bad idea.

> + if (flags & RV8803_FLAG_V1F)
> + dev_warn(>dev, "Voltage low, temperature compensation 
> stopped.\n");
> +
> + if (flags & RV8803_FLAG_V2F)
> + dev_warn(>dev, "Voltage low, data loss detected.\n");
> +
> + if (flags & RV8803_FLAG_TF) {
> + flags &= ~RV8803_FLAG_TF;
> + rv8803->ctrl &= ~RV8803_CTRL_TIE;
> + events |= RTC_PF;
> + }
> +
> + if (flags & RV8803_FLAG_AF) {
> + flags &= ~RV8803_FLAG_AF;
> + rv8803->ctrl &= ~RV8803_CTRL_AIE;
> + events |= RTC_AF;
> + }
> +
> + if (flags & RV8803_FLAG_UF) {
> + flags &= ~RV8803_FLAG_UF;
> + rv8803->ctrl &= ~RV8803_CTRL_UIE;
> + events |= RTC_UF;
> + }
> +
> + if (events) {
> + rtc_update_irq(rv8803->rtc, 1, events);
> + i2c_smbus_write_byte_data(client, RV8803_FLAG, flags);

How are the many read-modify-write cycles for flags safe without any
form of synchronization?  (Especially given the interrupt handler isn't
under ops_lock).

> + i2c_smbus_write_byte_data(rv8803->client, RV8803_CTRL,
> +   rv8803->ctrl);
> + }
> +
> + return IRQ_HANDLED;
> +}
> +
[..]
> +static int rv8803_get_alarm(struct device *dev, struct rtc_wkalrm *alrm)
> +{
> + struct rv8803_data *rv8803 = dev_get_drvdata(dev);
> + struct i2c_client *client = rv8803->client;
> + u8 alarmvals[3];
> + int flags, ret;
> +
> + if (client->irq <= 0)
> + return -EINVAL;

It'd be cleaner just to have a second set of rtc_class_ops that can be
switched between based on whether a valid interrupt is specified.

  Josh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] rtc: Add a driver for Micro Crystal RV8803

2015-09-26 Thread Alexandre Belloni

This driver supports the following functions:
 - reading and settings time
 - alarms when connected to an IRQ
 - reading and clearing the voltage low flags
 - nvram

Signed-off-by: Alexandre Belloni 
---
 drivers/rtc/Kconfig  |   9 +
 drivers/rtc/Makefile |   1 +
 drivers/rtc/rtc-rv8803.c | 504 +++
 3 files changed, 514 insertions(+)
 create mode 100644 drivers/rtc/rtc-rv8803.c

diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
index 9d4290617cee..3ae131334a84 100644
--- a/drivers/rtc/Kconfig
+++ b/drivers/rtc/Kconfig
@@ -593,6 +593,15 @@ config RTC_DRV_RV3029C2
  This driver can also be built as a module. If so, the module
  will be called rtc-rv3029c2.
 
+config RTC_DRV_RV8803
+   tristate "Micro Crystal RV8803"
+   help
+ If you say yes here you get support for the Micro Crystal
+ RV8803 RTC chips.
+
+ This driver can also be built as a module. If so, the module
+ will be called rtc-rv8803.
+
 config RTC_DRV_S5M
tristate "Samsung S2M/S5M series"
depends on MFD_SEC_CORE
diff --git a/drivers/rtc/Makefile b/drivers/rtc/Makefile
index e491eb524434..231f76451615 100644
--- a/drivers/rtc/Makefile
+++ b/drivers/rtc/Makefile
@@ -126,6 +126,7 @@ obj-$(CONFIG_RTC_DRV_RS5C313)   += rtc-rs5c313.o
 obj-$(CONFIG_RTC_DRV_RS5C348)  += rtc-rs5c348.o
 obj-$(CONFIG_RTC_DRV_RS5C372)  += rtc-rs5c372.o
 obj-$(CONFIG_RTC_DRV_RV3029C2) += rtc-rv3029c2.o
+obj-$(CONFIG_RTC_DRV_RV8803)   += rtc-rv8803.o
 obj-$(CONFIG_RTC_DRV_RX4581)   += rtc-rx4581.o
 obj-$(CONFIG_RTC_DRV_RX8025)   += rtc-rx8025.o
 obj-$(CONFIG_RTC_DRV_RX8581)   += rtc-rx8581.o
diff --git a/drivers/rtc/rtc-rv8803.c b/drivers/rtc/rtc-rv8803.c
new file mode 100644
index ..45d3393c5f4d
--- /dev/null
+++ b/drivers/rtc/rtc-rv8803.c
@@ -0,0 +1,504 @@
+/*
+ * RTC driver for the Micro Crystal RV8803
+ *
+ * Copyright (C) 2015 Micro Crystal SA
+ *
+ * Alexandre Belloni 
+ *
+ * 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.
+ *
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define RV8803_SEC 0x00
+#define RV8803_MIN 0x01
+#define RV8803_HOUR0x02
+#define RV8803_WEEK0x03
+#define RV8803_DAY 0x04
+#define RV8803_MONTH   0x05
+#define RV8803_YEAR0x06
+#define RV8803_RAM 0x07
+#define RV8803_ALARM_MIN   0x08
+#define RV8803_ALARM_HOUR  0x09
+#define RV8803_ALARM_WEEK_OR_DAY   0x0A
+#define RV8803_TIMER_CNT_0 0x0B
+#define RV8803_TIMER_CNT_1 0x0C
+#define RV8803_EXT 0x0D
+#define RV8803_FLAG0x0E
+#define RV8803_CTRL0x0F
+#define RV8803_OFFSET  0x2C
+
+#define RV8803_EXT_WADABIT(6)
+
+#define RV8803_FLAG_V1FBIT(0)
+#define RV8803_FLAG_V2FBIT(1)
+#define RV8803_FLAG_AF BIT(3)
+#define RV8803_FLAG_TF BIT(4)
+#define RV8803_FLAG_UF BIT(5)
+
+#define RV8803_CTRL_RESET  BIT(0)
+
+#define RV8803_CTRL_EIEBIT(2)
+#define RV8803_CTRL_AIEBIT(3)
+#define RV8803_CTRL_TIEBIT(4)
+#define RV8803_CTRL_UIEBIT(5)
+
+struct rv8803_data {
+   struct i2c_client *client;
+   struct rtc_device *rtc;
+   u8 ctrl;
+};
+
+static irqreturn_t rv8803_handle_irq(int irq, void *dev_id)
+{
+   struct i2c_client *client = dev_id;
+   struct rv8803_data *rv8803 = i2c_get_clientdata(client);
+   unsigned long events = 0;
+   u8 flags;
+
+   flags = i2c_smbus_read_byte_data(client, RV8803_FLAG);
+   if (flags <= 0)
+   return IRQ_HANDLED;
+
+   if (flags & RV8803_FLAG_V1F)
+   dev_warn(>dev, "Voltage low, temperature compensation 
stopped.\n");
+
+   if (flags & RV8803_FLAG_V2F)
+   dev_warn(>dev, "Voltage low, data loss detected.\n");
+
+   if (flags & RV8803_FLAG_TF) {
+   flags &= ~RV8803_FLAG_TF;
+   rv8803->ctrl &= ~RV8803_CTRL_TIE;
+   events |= RTC_PF;
+   }
+
+   if (flags & RV8803_FLAG_AF) {
+   flags &= ~RV8803_FLAG_AF;
+   rv8803->ctrl &= ~RV8803_CTRL_AIE;
+   events |= RTC_AF;
+   }
+
+   if (flags & RV8803_FLAG_UF) {
+   flags &= ~RV8803_FLAG_UF;
+   rv8803->ctrl &= ~RV8803_CTRL_UIE;
+   events |= RTC_UF;
+   }
+
+   if (events) {
+   rtc_update_irq(rv8803->rtc, 1, events);
+

[PATCH] rtc: Add a driver for Micro Crystal RV8803

2015-09-26 Thread Alexandre Belloni

This driver supports the following functions:
 - reading and settings time
 - alarms when connected to an IRQ
 - reading and clearing the voltage low flags
 - nvram

Signed-off-by: Alexandre Belloni 
---
 drivers/rtc/Kconfig  |   9 +
 drivers/rtc/Makefile |   1 +
 drivers/rtc/rtc-rv8803.c | 504 +++
 3 files changed, 514 insertions(+)
 create mode 100644 drivers/rtc/rtc-rv8803.c

diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
index 9d4290617cee..3ae131334a84 100644
--- a/drivers/rtc/Kconfig
+++ b/drivers/rtc/Kconfig
@@ -593,6 +593,15 @@ config RTC_DRV_RV3029C2
  This driver can also be built as a module. If so, the module
  will be called rtc-rv3029c2.
 
+config RTC_DRV_RV8803
+   tristate "Micro Crystal RV8803"
+   help
+ If you say yes here you get support for the Micro Crystal
+ RV8803 RTC chips.
+
+ This driver can also be built as a module. If so, the module
+ will be called rtc-rv8803.
+
 config RTC_DRV_S5M
tristate "Samsung S2M/S5M series"
depends on MFD_SEC_CORE
diff --git a/drivers/rtc/Makefile b/drivers/rtc/Makefile
index e491eb524434..231f76451615 100644
--- a/drivers/rtc/Makefile
+++ b/drivers/rtc/Makefile
@@ -126,6 +126,7 @@ obj-$(CONFIG_RTC_DRV_RS5C313)   += rtc-rs5c313.o
 obj-$(CONFIG_RTC_DRV_RS5C348)  += rtc-rs5c348.o
 obj-$(CONFIG_RTC_DRV_RS5C372)  += rtc-rs5c372.o
 obj-$(CONFIG_RTC_DRV_RV3029C2) += rtc-rv3029c2.o
+obj-$(CONFIG_RTC_DRV_RV8803)   += rtc-rv8803.o
 obj-$(CONFIG_RTC_DRV_RX4581)   += rtc-rx4581.o
 obj-$(CONFIG_RTC_DRV_RX8025)   += rtc-rx8025.o
 obj-$(CONFIG_RTC_DRV_RX8581)   += rtc-rx8581.o
diff --git a/drivers/rtc/rtc-rv8803.c b/drivers/rtc/rtc-rv8803.c
new file mode 100644
index ..45d3393c5f4d
--- /dev/null
+++ b/drivers/rtc/rtc-rv8803.c
@@ -0,0 +1,504 @@
+/*
+ * RTC driver for the Micro Crystal RV8803
+ *
+ * Copyright (C) 2015 Micro Crystal SA
+ *
+ * Alexandre Belloni 
+ *
+ * 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.
+ *
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define RV8803_SEC 0x00
+#define RV8803_MIN 0x01
+#define RV8803_HOUR0x02
+#define RV8803_WEEK0x03
+#define RV8803_DAY 0x04
+#define RV8803_MONTH   0x05
+#define RV8803_YEAR0x06
+#define RV8803_RAM 0x07
+#define RV8803_ALARM_MIN   0x08
+#define RV8803_ALARM_HOUR  0x09
+#define RV8803_ALARM_WEEK_OR_DAY   0x0A
+#define RV8803_TIMER_CNT_0 0x0B
+#define RV8803_TIMER_CNT_1 0x0C
+#define RV8803_EXT 0x0D
+#define RV8803_FLAG0x0E
+#define RV8803_CTRL0x0F
+#define RV8803_OFFSET  0x2C
+
+#define RV8803_EXT_WADABIT(6)
+
+#define RV8803_FLAG_V1FBIT(0)
+#define RV8803_FLAG_V2FBIT(1)
+#define RV8803_FLAG_AF BIT(3)
+#define RV8803_FLAG_TF BIT(4)
+#define RV8803_FLAG_UF BIT(5)
+
+#define RV8803_CTRL_RESET  BIT(0)
+
+#define RV8803_CTRL_EIEBIT(2)
+#define RV8803_CTRL_AIEBIT(3)
+#define RV8803_CTRL_TIEBIT(4)
+#define RV8803_CTRL_UIEBIT(5)
+
+struct rv8803_data {
+   struct i2c_client *client;
+   struct rtc_device *rtc;
+   u8 ctrl;
+};
+
+static irqreturn_t rv8803_handle_irq(int irq, void *dev_id)
+{
+   struct i2c_client *client = dev_id;
+   struct rv8803_data *rv8803 = i2c_get_clientdata(client);
+   unsigned long events = 0;
+   u8 flags;
+
+   flags = i2c_smbus_read_byte_data(client, RV8803_FLAG);
+   if (flags <= 0)
+   return IRQ_HANDLED;
+
+   if (flags & RV8803_FLAG_V1F)
+   dev_warn(>dev, "Voltage low, temperature compensation 
stopped.\n");
+
+   if (flags & RV8803_FLAG_V2F)
+   dev_warn(>dev, "Voltage low, data loss detected.\n");
+
+   if (flags & RV8803_FLAG_TF) {
+   flags &= ~RV8803_FLAG_TF;
+   rv8803->ctrl &= ~RV8803_CTRL_TIE;
+   events |= RTC_PF;
+   }
+
+   if (flags & RV8803_FLAG_AF) {
+   flags &= ~RV8803_FLAG_AF;
+   rv8803->ctrl &= ~RV8803_CTRL_AIE;
+   events |= RTC_AF;
+   }
+
+   if (flags & RV8803_FLAG_UF) {
+   flags &= ~RV8803_FLAG_UF;
+   rv8803->ctrl &= ~RV8803_CTRL_UIE;
+   events |= RTC_UF;
+   }
+
+   if (events) {
+

Re: [PATCH] rtc: Add a driver for Micro Crystal RV8803

Re: [PATCH] rtc: Add a driver for Micro Crystal RV8803

Re: [PATCH] rtc: Add a driver for Micro Crystal RV8803

Re: [PATCH] rtc: Add a driver for Micro Crystal RV8803

[PATCH] rtc: Add a driver for Micro Crystal RV8803

[PATCH] rtc: Add a driver for Micro Crystal RV8803

6 matches

Site Navigation

Mail list logo

Footer information