[PATCH RFC V1 0/3] da9062: Add DA9062 RTC support using the existing DA9063 RTC driver

[PATCH RFC V1 0/3] da9062: Add DA9062 RTC support using the existing DA9063 RTC driver

[S Twiss]

From: S Twiss <stwiss.opensource@diasemi.com>

This patch set adds RTC support for the Dialog DA9062 Power Management IC.
Changes are made to the existing DA9063 RTC component so that functionality
in this device driver can be re-used to support the DA9062 RTC.

This following patch set is the proposed method for re-using existing kernel
RTC drivers for chips whose RTC blocks are functionally similar to the
DA9063 RTC.

There is already a precedence for reuse of this type in the kernel, see
linux-mainline/v4.1 drivers/rtc/rtc-pm8xxx.c +34
This patch set will follow that example.

The main points for the MFD core and device tree changes are as follows.

- Alteration of the DA9063 RTC Kconfig needs to be updated to depend on
  both MFD_DA9063 "or" MFD_DA9062. There is no explicit DA9062 RTC Kconfig.
- The DA9062 MFD core should add a new RTC resource as usual and an entry
  in the mfd_cell to support a component name and of_compatible for
  "da9062-rtc" and "dlg,da9062-rtc".
- The device tree binding support should include a compatible string for
  "dlg,da9062-rtc"

The main points for the RTC changes are as follows:

A generic structure is used (called da9063_compatible_rtc_regmap) to hold
all generic registers and bitmasks for use with this type of RTC component.

Functions in the DA9063 RTC will be refactored to use this compatibility
struct and all accesses to generic registers/masks will be made through
this table look-up instead of using defines from the register header files
directly

Linkage between the DA9062 MFD and the DA9063 RTC driver is created through
the use of an of_match_table entry in the platform_driver structure.
A re-work of da9063_rtc_probe() is necessary to use the of_match_node() and
dev_get_regmap() functions: this will provide initialisation of the generic
registers and masks and allow access to the regmap according to the correct
device tree specification.

The addition of a of_device_id table for DA9063 and DA9062 default data
is created. This method of initialisation for the DA9063 RTC component
works well for different chip types, and is only slightly complicated when
distinguishing the difference between support for DA9063-AD and DA9063-BB.

In this patch set the following is provided:
 - [PATCH V1 1/3]: MFD changes in DA9062 to support RTC
 - [PATCH V1 2/3]: Update existing DA9063 RTC to add DA9062 support
 - [PATCH V1 3/3]: Device tree bindings for DA9062 RTC component

This patch applies against linux-next and next-20150708 

Thank you,
Steve Twiss, Dialog Semiconductor Ltd.

S Twiss (3):
  mfd: da9062: Support for the DA9063 RTC in the DA9062 core
  rtc: da9063: Add DA9062 RTC capability to DA9063 RTC driver
  devicetree: da9062: Add device tree bindings for DA9062 RTC

 Documentation/devicetree/bindings/mfd/da9062.txt |   9 +
 drivers/mfd/da9062-core.c                        |  11 +
 drivers/rtc/Kconfig                              |   6 +-
 drivers/rtc/rtc-da9063.c                         | 435 ++++++++++++++++-------
 4 files changed, 330 insertions(+), 131 deletions(-)

-- 
end-of-patch for PATCH RFC V1

[PATCH RFC V1 2/3] rtc: da9063: Add DA9062 RTC capability to DA9063 RTC driver

[S Twiss]

From: S Twiss <stwiss.opensource@diasemi.com>

Add DA9062 RTC support into the existing DA9063 RTC driver component by
using generic access tables for common register and bit mask definitions.

The following change will add generic register and bit mask support to the
DA9063 RTC. The changes are slightly complicated by requiring support for
three register sets: DA9063-AD, DA9063-BB and DA9062-AA.

The following alterations have been made to the DA9063 RTC:

- Addition of a da9063_compatible_rtc_regmap structure to hold all generic
  registers and bitmasks for this type of RTC component.
- A re-write of struct da9063 to use pointers for regmap and compatible
  registers/masks definitions
- Addition of a of_device_id table for DA9063 and DA9062 defaults
- Refactoring functions to use struct da9063_compatible_rtc accesses to
  generic registers/masks instead of using defines from registers.h
- Addition of a re-try when reading the RTC inside da9063_rtc_read_time()
- Re-work of da9063_rtc_probe() to use of_match_node() and dev_get_regmap()
  to provide initialisation of generic registers and masks and access to
  regmap

Signed-off-by: Steve Twiss <stwiss.opensource@diasemi.com>

---
Checks performed with linux-next/next-20150708/scripts/checkpatch.pl
 Kconfig                   total: 0 errors, 15 warnings, 1600 lines checked
 rtc-da9063.c              total: 0 errors, 0 warnings, 531 lines checked
This patch applies against linux-next and next-20150708 


 drivers/rtc/Kconfig      |   6 +-
 drivers/rtc/rtc-da9063.c | 435 +++++++++++++++++++++++++++++++++--------------
 2 files changed, 310 insertions(+), 131 deletions(-)

diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
index 83b4b89..1eaa268 100644
--- a/drivers/rtc/Kconfig
+++ b/drivers/rtc/Kconfig
@@ -945,11 +945,11 @@ config RTC_DRV_DA9055
 	  will be called rtc-da9055
 
 config RTC_DRV_DA9063
-	tristate "Dialog Semiconductor DA9063 RTC"
-	depends on MFD_DA9063
+	tristate "Dialog Semiconductor DA9063/DA9062 RTC"
+	depends on MFD_DA9063 || MFD_DA9062
 	help
 	  If you say yes here you will get support for the RTC subsystem
-	  of the Dialog Semiconductor DA9063.
+	  for the Dialog Semiconductor PMIC chips DA9063 and DA9062.
 
 	  This driver can also be built as a module. If so, the module
 	  will be called "rtc-da9063".
diff --git a/drivers/rtc/rtc-da9063.c b/drivers/rtc/rtc-da9063.c
index 7ffc570..e94fb6d 100644
--- a/drivers/rtc/rtc-da9063.c
+++ b/drivers/rtc/rtc-da9063.c
@@ -1,127 +1,274 @@
-/* rtc-da9063.c - Real time clock device driver for DA9063
- * Copyright (C) 2013-14  Dialog Semiconductor Ltd.
+/*
+ * Real time clock device driver for DA9063/DA9062
+ * Copyright (C) 2013-15  Dialog Semiconductor Ltd.
  *
- * This library is free software; you can redistribute it and/or
- * modify it under the terms of the GNU Library General Public
- * License as published by the Free Software Foundation; either
- * version 2 of the License, or (at your option) any later version.
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
  *
- * This library is distributed in the hope that it will be useful,
+ * 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
- * Library General Public License for more details.
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
  */
 
+#include <linux/delay.h>
+#include <linux/init.h>
+#include <linux/interrupt.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
-#include <linux/init.h>
+#include <linux/of.h>
 #include <linux/platform_device.h>
-#include <linux/interrupt.h>
+#include <linux/regmap.h>
 #include <linux/rtc.h>
 #include <linux/slab.h>
-#include <linux/delay.h>
-#include <linux/regmap.h>
+
+#include <linux/mfd/da9062/registers.h>
 #include <linux/mfd/da9063/registers.h>
 #include <linux/mfd/da9063/core.h>
 
+#define DA9063_GET_TIME_RETRIES	5
 #define YEARS_TO_DA9063(year)		((year) - 100)
 #define MONTHS_TO_DA9063(month)		((month) + 1)
 #define YEARS_FROM_DA9063(year)		((year) + 100)
 #define MONTHS_FROM_DA9063(month)	((month) - 1)
 
-#define RTC_ALARM_DATA_LEN (DA9063_AD_REG_ALARM_Y - DA9063_AD_REG_ALARM_MI + 1)
-
-#define RTC_DATA_LEN	(DA9063_REG_COUNT_Y - DA9063_REG_COUNT_S + 1)
-#define RTC_SEC		0
-#define RTC_MIN		1
-#define RTC_HOUR	2
-#define RTC_DAY		3
-#define RTC_MONTH	4
-#define RTC_YEAR	5
-
-struct da9063_rtc {
-	struct rtc_device	*rtc_dev;
-	struct da9063		*hw;
-	struct rtc_time		alarm_time;
-	bool			rtc_sync;
-	int			alarm_year;
-	int			alarm_start;
-	int			alarm_len;
-	int			data_start;
+enum {
+	RTC_SEC	= 0,
+	RTC_MIN	= 1,
+	RTC_HOUR = 2,
+	RTC_DAY	= 3,
+	RTC_MONTH = 4,
+	RTC_YEAR = 5,
+	RTC_DATA_LEN
+};
+
+struct da9063_compatible_rtc_regmap {
+	/* REGS */
+	int rtc_enable_reg;
+	int rtc_enable_32k_crystal_reg;
+	int rtc_alarm_secs_reg;
+	int rtc_alarm_year_reg;
+	int rtc_count_secs_reg;
+	int rtc_count_year_reg;
+	int rtc_event_reg;
+	/* MASKS */
+	int rtc_enable_mask;
+	int rtc_crystal_mask;
+	int rtc_event_alarm_mask;
+	int rtc_alarm_on_mask;
+	int rtc_alarm_status_mask;
+	int rtc_tick_on_mask;
+	int rtc_ready_to_read_mask;
+	int rtc_count_sec_mask;
+	int rtc_count_min_mask;
+	int rtc_count_hour_mask;
+	int rtc_count_day_mask;
+	int rtc_count_month_mask;
+	int rtc_count_year_mask;
+	/* ALARM CONFIG */
+	int rtc_data_start;
+	int rtc_alarm_len;
+};
+
+struct da9063_compatible_rtc {
+	struct rtc_device *rtc_dev;
+	struct rtc_time alarm_time;
+	struct regmap *regmap;
+	const struct da9063_compatible_rtc_regmap *config;
+	bool rtc_sync;
 };
 
-static void da9063_data_to_tm(u8 *data, struct rtc_time *tm)
+static const struct da9063_compatible_rtc_regmap da9063_ad_regs = {
+	/* REGS */
+	.rtc_enable_reg             = DA9063_REG_CONTROL_E,
+	.rtc_alarm_secs_reg         = DA9063_AD_REG_ALARM_MI,
+	.rtc_alarm_year_reg         = DA9063_AD_REG_ALARM_Y,
+	.rtc_count_secs_reg         = DA9063_REG_COUNT_S,
+	.rtc_count_year_reg         = DA9063_REG_COUNT_Y,
+	.rtc_event_reg              = DA9063_REG_EVENT_A,
+	/* MASKS */
+	.rtc_enable_mask            = DA9063_RTC_EN,
+	.rtc_crystal_mask           = DA9063_CRYSTAL,
+	.rtc_enable_32k_crystal_reg = DA9063_REG_EN_32K,
+	.rtc_event_alarm_mask       = DA9063_E_ALARM,
+	.rtc_alarm_on_mask          = DA9063_ALARM_ON,
+	.rtc_alarm_status_mask      = DA9063_ALARM_STATUS_ALARM |
+				      DA9063_ALARM_STATUS_TICK,
+	.rtc_tick_on_mask           = DA9063_TICK_ON,
+	.rtc_ready_to_read_mask     = DA9063_RTC_READ,
+	.rtc_count_sec_mask         = DA9063_COUNT_SEC_MASK,
+	.rtc_count_min_mask         = DA9063_COUNT_MIN_MASK,
+	.rtc_count_hour_mask        = DA9063_COUNT_HOUR_MASK,
+	.rtc_count_day_mask         = DA9063_COUNT_DAY_MASK,
+	.rtc_count_month_mask       = DA9063_COUNT_MONTH_MASK,
+	.rtc_count_year_mask        = DA9063_COUNT_YEAR_MASK,
+	/* ALARM CONFIG */
+	.rtc_data_start             = RTC_MIN,
+	.rtc_alarm_len              = RTC_DATA_LEN - 1,
+};
+
+static const struct da9063_compatible_rtc_regmap da9063_bb_regs = {
+	/* REGS */
+	.rtc_enable_reg             = DA9063_REG_CONTROL_E,
+	.rtc_alarm_secs_reg         = DA9063_BB_REG_ALARM_S,
+	.rtc_alarm_year_reg         = DA9063_BB_REG_ALARM_Y,
+	.rtc_count_secs_reg         = DA9063_REG_COUNT_S,
+	.rtc_count_year_reg         = DA9063_REG_COUNT_Y,
+	.rtc_event_reg              = DA9063_REG_EVENT_A,
+	/* MASKS */
+	.rtc_enable_mask            = DA9063_RTC_EN,
+	.rtc_crystal_mask           = DA9063_CRYSTAL,
+	.rtc_enable_32k_crystal_reg = DA9063_REG_EN_32K,
+	.rtc_event_alarm_mask       = DA9063_E_ALARM,
+	.rtc_alarm_on_mask          = DA9063_ALARM_ON,
+	.rtc_alarm_status_mask      = DA9063_ALARM_STATUS_ALARM |
+				      DA9063_ALARM_STATUS_TICK,
+	.rtc_tick_on_mask           = DA9063_TICK_ON,
+	.rtc_ready_to_read_mask     = DA9063_RTC_READ,
+	.rtc_count_sec_mask         = DA9063_COUNT_SEC_MASK,
+	.rtc_count_min_mask         = DA9063_COUNT_MIN_MASK,
+	.rtc_count_hour_mask        = DA9063_COUNT_HOUR_MASK,
+	.rtc_count_day_mask         = DA9063_COUNT_DAY_MASK,
+	.rtc_count_month_mask       = DA9063_COUNT_MONTH_MASK,
+	.rtc_count_year_mask        = DA9063_COUNT_YEAR_MASK,
+	/* ALARM CONFIG */
+	.rtc_data_start             = RTC_SEC,
+	.rtc_alarm_len              = RTC_DATA_LEN,
+};
+
+static const struct da9063_compatible_rtc_regmap da9062_aa_regs = {
+	/* REGS */
+	.rtc_enable_reg             = DA9062AA_CONTROL_E,
+	.rtc_alarm_secs_reg         = DA9062AA_ALARM_S,
+	.rtc_alarm_year_reg         = DA9062AA_ALARM_Y,
+	.rtc_count_secs_reg         = DA9062AA_COUNT_S,
+	.rtc_count_year_reg         = DA9062AA_COUNT_Y,
+	.rtc_event_reg              = DA9062AA_EVENT_A,
+	/* MASKS */
+	.rtc_enable_mask            = DA9062AA_RTC_EN_MASK,
+	.rtc_crystal_mask           = DA9062AA_CRYSTAL_MASK,
+	.rtc_enable_32k_crystal_reg = DA9062AA_EN_32K,
+	.rtc_event_alarm_mask       = DA9062AA_M_ALARM_MASK,
+	.rtc_alarm_on_mask          = DA9062AA_ALARM_ON_MASK,
+	.rtc_alarm_status_mask      = (0x02 << 6),
+	.rtc_tick_on_mask           = DA9062AA_TICK_ON_MASK,
+	.rtc_ready_to_read_mask     = DA9062AA_RTC_READ_MASK,
+	.rtc_count_sec_mask         = DA9062AA_COUNT_SEC_MASK,
+	.rtc_count_min_mask         = DA9062AA_COUNT_MIN_MASK,
+	.rtc_count_hour_mask        = DA9062AA_COUNT_HOUR_MASK,
+	.rtc_count_day_mask         = DA9062AA_COUNT_DAY_MASK,
+	.rtc_count_month_mask       = DA9062AA_COUNT_MONTH_MASK,
+	.rtc_count_year_mask        = DA9062AA_COUNT_YEAR_MASK,
+	/* ALARM CONFIG */
+	.rtc_data_start             = RTC_SEC,
+	.rtc_alarm_len              = RTC_DATA_LEN,
+};
+
+static const struct of_device_id da9063_compatible_reg_id_table[] = {
+	{ .compatible = "dlg,da9063-rtc", .data = &da9063_bb_regs },
+	{ .compatible = "dlg,da9062-rtc", .data = &da9062_aa_regs },
+	{ },
+};
+
+static void da9063_data_to_tm(u8 *data, struct rtc_time *tm,
+			      struct da9063_compatible_rtc *rtc)
 {
-	tm->tm_sec  = data[RTC_SEC]  & DA9063_COUNT_SEC_MASK;
-	tm->tm_min  = data[RTC_MIN]  & DA9063_COUNT_MIN_MASK;
-	tm->tm_hour = data[RTC_HOUR] & DA9063_COUNT_HOUR_MASK;
-	tm->tm_mday = data[RTC_DAY]  & DA9063_COUNT_DAY_MASK;
+	const struct da9063_compatible_rtc_regmap *config = rtc->config;
+
+	tm->tm_sec  = data[RTC_SEC]  & config->rtc_count_sec_mask;
+	tm->tm_min  = data[RTC_MIN]  & config->rtc_count_min_mask;
+	tm->tm_hour = data[RTC_HOUR] & config->rtc_count_hour_mask;
+	tm->tm_mday = data[RTC_DAY]  & config->rtc_count_day_mask;
 	tm->tm_mon  = MONTHS_FROM_DA9063(data[RTC_MONTH] &
-					 DA9063_COUNT_MONTH_MASK);
+					 config->rtc_count_month_mask);
 	tm->tm_year = YEARS_FROM_DA9063(data[RTC_YEAR] &
-					DA9063_COUNT_YEAR_MASK);
+					config->rtc_count_year_mask);
 }
 
-static void da9063_tm_to_data(struct rtc_time *tm, u8 *data)
+static void da9063_tm_to_data(struct rtc_time *tm, u8 *data,
+			      struct da9063_compatible_rtc *rtc)
 {
-	data[RTC_SEC] &= ~DA9063_COUNT_SEC_MASK;
-	data[RTC_SEC] |= tm->tm_sec & DA9063_COUNT_SEC_MASK;
+	const struct da9063_compatible_rtc_regmap *config = rtc->config;
+
+	data[RTC_SEC] &= ~config->rtc_count_sec_mask;
+	data[RTC_SEC] |= tm->tm_sec & config->rtc_count_sec_mask;
 
-	data[RTC_MIN] &= ~DA9063_COUNT_MIN_MASK;
-	data[RTC_MIN] |= tm->tm_min & DA9063_COUNT_MIN_MASK;
+	data[RTC_MIN] &= ~config->rtc_count_min_mask;
+	data[RTC_MIN] |= tm->tm_min & config->rtc_count_min_mask;
 
-	data[RTC_HOUR] &= ~DA9063_COUNT_HOUR_MASK;
-	data[RTC_HOUR] |= tm->tm_hour & DA9063_COUNT_HOUR_MASK;
+	data[RTC_HOUR] &= ~config->rtc_count_hour_mask;
+	data[RTC_HOUR] |= tm->tm_hour & config->rtc_count_hour_mask;
 
-	data[RTC_DAY] &= ~DA9063_COUNT_DAY_MASK;
-	data[RTC_DAY] |= tm->tm_mday & DA9063_COUNT_DAY_MASK;
+	data[RTC_DAY] &= ~config->rtc_count_day_mask;
+	data[RTC_DAY] |= tm->tm_mday & config->rtc_count_day_mask;
 
-	data[RTC_MONTH] &= ~DA9063_COUNT_MONTH_MASK;
+	data[RTC_MONTH] &= ~config->rtc_count_month_mask;
 	data[RTC_MONTH] |= MONTHS_TO_DA9063(tm->tm_mon) &
-				DA9063_COUNT_MONTH_MASK;
+				config->rtc_count_month_mask;
 
-	data[RTC_YEAR] &= ~DA9063_COUNT_YEAR_MASK;
+	data[RTC_YEAR] &= ~config->rtc_count_year_mask;
 	data[RTC_YEAR] |= YEARS_TO_DA9063(tm->tm_year) &
-				DA9063_COUNT_YEAR_MASK;
+				config->rtc_count_year_mask;
 }
 
 static int da9063_rtc_stop_alarm(struct device *dev)
 {
-	struct da9063_rtc *rtc = dev_get_drvdata(dev);
+	struct da9063_compatible_rtc *rtc = dev_get_drvdata(dev);
+	const struct da9063_compatible_rtc_regmap *config = rtc->config;
 
-	return regmap_update_bits(rtc->hw->regmap, rtc->alarm_year,
-				  DA9063_ALARM_ON, 0);
+	return regmap_update_bits(rtc->regmap,
+				  config->rtc_alarm_year_reg,
+				  config->rtc_alarm_on_mask,
+				  0);
 }
 
 static int da9063_rtc_start_alarm(struct device *dev)
 {
-	struct da9063_rtc *rtc = dev_get_drvdata(dev);
+	struct da9063_compatible_rtc *rtc = dev_get_drvdata(dev);
+	const struct da9063_compatible_rtc_regmap *config = rtc->config;
 
-	return regmap_update_bits(rtc->hw->regmap, rtc->alarm_year,
-				  DA9063_ALARM_ON, DA9063_ALARM_ON);
+	return regmap_update_bits(rtc->regmap,
+				  config->rtc_alarm_year_reg,
+				  config->rtc_alarm_on_mask,
+				  config->rtc_alarm_on_mask);
 }
 
 static int da9063_rtc_read_time(struct device *dev, struct rtc_time *tm)
 {
-	struct da9063_rtc *rtc = dev_get_drvdata(dev);
+	struct da9063_compatible_rtc *rtc = dev_get_drvdata(dev);
+	const struct da9063_compatible_rtc_regmap *config = rtc->config;
 	unsigned long tm_secs;
 	unsigned long al_secs;
 	u8 data[RTC_DATA_LEN];
+	int timeout = DA9063_GET_TIME_RETRIES;
+	bool read_done = false;
 	int ret;
 
-	ret = regmap_bulk_read(rtc->hw->regmap, DA9063_REG_COUNT_S,
-			       data, RTC_DATA_LEN);
-	if (ret < 0) {
-		dev_err(dev, "Failed to read RTC time data: %d\n", ret);
-		return ret;
+	while (timeout--) {
+		ret = regmap_bulk_read(rtc->regmap,
+				       config->rtc_count_secs_reg,
+				       data, RTC_DATA_LEN);
+		if (ret < 0) {
+			dev_err(dev, "Failed to read RTC time data: %d\n",
+				ret);
+			return ret;
+		}
+
+		if (data[RTC_SEC] & config->rtc_ready_to_read_mask) {
+			timeout = 0;
+			read_done = true;
+		} else
+			msleep(20);
 	}
 
-	if (!(data[RTC_SEC] & DA9063_RTC_READ)) {
-		dev_dbg(dev, "RTC not yet ready to be read by the host\n");
-		return -EINVAL;
+	if (!read_done) {
+		dev_err(dev, "Failed to read the RTC time\n");
+		return -EIO;
 	}
 
-	da9063_data_to_tm(data, tm);
+	da9063_data_to_tm(data, tm, rtc);
 
 	rtc_tm_to_time(tm, &tm_secs);
 	rtc_tm_to_time(&rtc->alarm_time, &al_secs);
@@ -137,12 +284,14 @@ static int da9063_rtc_read_time(struct device *dev, struct rtc_time *tm)
 
 static int da9063_rtc_set_time(struct device *dev, struct rtc_time *tm)
 {
-	struct da9063_rtc *rtc = dev_get_drvdata(dev);
+	struct da9063_compatible_rtc *rtc = dev_get_drvdata(dev);
+	const struct da9063_compatible_rtc_regmap *config = rtc->config;
 	u8 data[RTC_DATA_LEN];
 	int ret;
 
-	da9063_tm_to_data(tm, data);
-	ret = regmap_bulk_write(rtc->hw->regmap, DA9063_REG_COUNT_S,
+	da9063_tm_to_data(tm, data, rtc);
+	ret = regmap_bulk_write(rtc->regmap,
+				config->rtc_count_secs_reg,
 				data, RTC_DATA_LEN);
 	if (ret < 0)
 		dev_err(dev, "Failed to set RTC time data: %d\n", ret);
@@ -152,26 +301,31 @@ static int da9063_rtc_set_time(struct device *dev, struct rtc_time *tm)
 
 static int da9063_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alrm)
 {
-	struct da9063_rtc *rtc = dev_get_drvdata(dev);
+	struct da9063_compatible_rtc *rtc = dev_get_drvdata(dev);
+	const struct da9063_compatible_rtc_regmap *config = rtc->config;
 	u8 data[RTC_DATA_LEN];
 	int ret;
 	unsigned int val;
 
 	data[RTC_SEC] = 0;
-	ret = regmap_bulk_read(rtc->hw->regmap, rtc->alarm_start,
-			       &data[rtc->data_start], rtc->alarm_len);
+	ret = regmap_bulk_read(rtc->regmap,
+			       config->rtc_alarm_secs_reg,
+			       &data[config->rtc_data_start],
+			       config->rtc_alarm_len);
 	if (ret < 0)
 		return ret;
 
-	da9063_data_to_tm(data, &alrm->time);
+	da9063_data_to_tm(data, &alrm->time, rtc);
 
-	alrm->enabled = !!(data[RTC_YEAR] & DA9063_ALARM_ON);
+	alrm->enabled = !!(data[RTC_YEAR] & config->rtc_alarm_on_mask);
 
-	ret = regmap_read(rtc->hw->regmap, DA9063_REG_EVENT_A, &val);
+	ret = regmap_read(rtc->regmap,
+			  config->rtc_event_reg,
+			  &val);
 	if (ret < 0)
 		return ret;
 
-	if (val & (DA9063_E_ALARM))
+	if (val & (config->rtc_event_alarm_mask))
 		alrm->pending = 1;
 	else
 		alrm->pending = 0;
@@ -181,11 +335,12 @@ static int da9063_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alrm)
 
 static int da9063_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alrm)
 {
-	struct da9063_rtc *rtc = dev_get_drvdata(dev);
+	struct da9063_compatible_rtc *rtc = dev_get_drvdata(dev);
+	const struct da9063_compatible_rtc_regmap *config = rtc->config;
 	u8 data[RTC_DATA_LEN];
 	int ret;
 
-	da9063_tm_to_data(&alrm->time, data);
+	da9063_tm_to_data(&alrm->time, data, rtc);
 
 	ret = da9063_rtc_stop_alarm(dev);
 	if (ret < 0) {
@@ -193,14 +348,16 @@ static int da9063_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alrm)
 		return ret;
 	}
 
-	ret = regmap_bulk_write(rtc->hw->regmap, rtc->alarm_start,
-			       &data[rtc->data_start], rtc->alarm_len);
+	ret = regmap_bulk_write(rtc->regmap,
+				config->rtc_alarm_secs_reg,
+				&data[config->rtc_data_start],
+				config->rtc_alarm_len);
 	if (ret < 0) {
 		dev_err(dev, "Failed to write alarm: %d\n", ret);
 		return ret;
 	}
 
-	da9063_data_to_tm(data, &rtc->alarm_time);
+	da9063_data_to_tm(data, &rtc->alarm_time, rtc);
 
 	if (alrm->enabled) {
 		ret = da9063_rtc_start_alarm(dev);
@@ -213,7 +370,8 @@ static int da9063_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alrm)
 	return ret;
 }
 
-static int da9063_rtc_alarm_irq_enable(struct device *dev, unsigned int enabled)
+static int da9063_rtc_alarm_irq_enable(struct device *dev,
+				       unsigned int enabled)
 {
 	if (enabled)
 		return da9063_rtc_start_alarm(dev);
@@ -223,10 +381,13 @@ static int da9063_rtc_alarm_irq_enable(struct device *dev, unsigned int enabled)
 
 static irqreturn_t da9063_alarm_event(int irq, void *data)
 {
-	struct da9063_rtc *rtc = data;
+	struct da9063_compatible_rtc *rtc = data;
+	const struct da9063_compatible_rtc_regmap *config = rtc->config;
 
-	regmap_update_bits(rtc->hw->regmap, rtc->alarm_year,
-			   DA9063_ALARM_ON, 0);
+	regmap_update_bits(rtc->regmap,
+			   config->rtc_alarm_year_reg,
+			   config->rtc_alarm_on_mask,
+			   0);
 
 	rtc->rtc_sync = true;
 	rtc_update_irq(rtc->rtc_dev, 1, RTC_IRQF | RTC_AF);
@@ -244,72 +405,91 @@ static const struct rtc_class_ops da9063_rtc_ops = {
 
 static int da9063_rtc_probe(struct platform_device *pdev)
 {
-	struct da9063 *da9063 = dev_get_drvdata(pdev->dev.parent);
-	struct da9063_rtc *rtc;
+	struct da9063_compatible_rtc *rtc;
+	const struct da9063_compatible_rtc_regmap *config;
+	const struct of_device_id *match;
 	int irq_alarm;
 	u8 data[RTC_DATA_LEN];
 	int ret;
 
-	ret = regmap_update_bits(da9063->regmap, DA9063_REG_CONTROL_E,
-				 DA9063_RTC_EN, DA9063_RTC_EN);
+	match = of_match_node(da9063_compatible_reg_id_table,
+			      pdev->dev.of_node);
+	if (!match)
+		return -ENXIO;
+
+	rtc = devm_kzalloc(&pdev->dev, sizeof(*rtc), GFP_KERNEL);
+	if (!rtc)
+		return -ENOMEM;
+
+	if (strncmp(match->name, "dlg,da9063-rtc", 14) == 0) {
+		struct da9063 *chip = dev_get_drvdata(pdev->dev.parent);
+
+		if (chip->variant_code == PMIC_DA9063_AD)
+			rtc->config = &da9063_ad_regs;
+	} else
+		rtc->config = match->data;
+
+	rtc->regmap = dev_get_regmap(pdev->dev.parent, NULL);
+	if (!rtc->regmap) {
+		dev_warn(&pdev->dev, "Parent regmap unavailable.\n");
+		return -ENXIO;
+	}
+
+	config = rtc->config;
+	ret = regmap_update_bits(rtc->regmap,
+				 config->rtc_enable_reg,
+				 config->rtc_enable_mask,
+				 config->rtc_enable_mask);
 	if (ret < 0) {
 		dev_err(&pdev->dev, "Failed to enable RTC\n");
-		goto err;
+		return ret;
 	}
 
-	ret = regmap_update_bits(da9063->regmap, DA9063_REG_EN_32K,
-				 DA9063_CRYSTAL, DA9063_CRYSTAL);
+	ret = regmap_update_bits(rtc->regmap,
+				 config->rtc_enable_32k_crystal_reg,
+				 config->rtc_crystal_mask,
+				 config->rtc_crystal_mask);
 	if (ret < 0) {
 		dev_err(&pdev->dev, "Failed to run 32kHz oscillator\n");
-		goto err;
-	}
-
-	rtc = devm_kzalloc(&pdev->dev, sizeof(*rtc), GFP_KERNEL);
-	if (!rtc)
-		return -ENOMEM;
-
-	if (da9063->variant_code == PMIC_DA9063_AD) {
-		rtc->alarm_year = DA9063_AD_REG_ALARM_Y;
-		rtc->alarm_start = DA9063_AD_REG_ALARM_MI;
-		rtc->alarm_len = RTC_ALARM_DATA_LEN;
-		rtc->data_start = RTC_MIN;
-	} else {
-		rtc->alarm_year = DA9063_BB_REG_ALARM_Y;
-		rtc->alarm_start = DA9063_BB_REG_ALARM_S;
-		rtc->alarm_len = RTC_DATA_LEN;
-		rtc->data_start = RTC_SEC;
+		return ret;
 	}
 
-	ret = regmap_update_bits(da9063->regmap, rtc->alarm_start,
-			DA9063_ALARM_STATUS_TICK | DA9063_ALARM_STATUS_ALARM,
-			0);
+	ret = regmap_update_bits(rtc->regmap,
+				 config->rtc_alarm_secs_reg,
+				 config->rtc_alarm_status_mask,
+				 0);
 	if (ret < 0) {
 		dev_err(&pdev->dev, "Failed to access RTC alarm register\n");
-		goto err;
+		return ret;
 	}
 
-	ret = regmap_update_bits(da9063->regmap, rtc->alarm_start,
+	ret = regmap_update_bits(rtc->regmap,
+				 config->rtc_alarm_secs_reg,
 				 DA9063_ALARM_STATUS_ALARM,
 				 DA9063_ALARM_STATUS_ALARM);
 	if (ret < 0) {
 		dev_err(&pdev->dev, "Failed to access RTC alarm register\n");
-		goto err;
+		return ret;
 	}
 
-	ret = regmap_update_bits(da9063->regmap, rtc->alarm_year,
-				 DA9063_TICK_ON, 0);
+	ret = regmap_update_bits(rtc->regmap,
+				 config->rtc_alarm_year_reg,
+				 config->rtc_tick_on_mask,
+				 0);
 	if (ret < 0) {
 		dev_err(&pdev->dev, "Failed to disable TICKs\n");
-		goto err;
+		return ret;
 	}
 
 	data[RTC_SEC] = 0;
-	ret = regmap_bulk_read(da9063->regmap, rtc->alarm_start,
-			       &data[rtc->data_start], rtc->alarm_len);
+	ret = regmap_bulk_read(rtc->regmap,
+			       config->rtc_alarm_secs_reg,
+			       &data[config->rtc_data_start],
+			       config->rtc_alarm_len);
 	if (ret < 0) {
 		dev_err(&pdev->dev, "Failed to read initial alarm data: %d\n",
 			ret);
-		goto err;
+		return ret;
 	}
 
 	platform_set_drvdata(pdev, rtc);
@@ -322,18 +502,16 @@ static int da9063_rtc_probe(struct platform_device *pdev)
 	if (ret) {
 		dev_err(&pdev->dev, "Failed to request ALARM IRQ %d: %d\n",
 			irq_alarm, ret);
-		goto err;
+		return ret;
 	}
 
-	rtc->hw = da9063;
 	rtc->rtc_dev = devm_rtc_device_register(&pdev->dev, DA9063_DRVNAME_RTC,
 					   &da9063_rtc_ops, THIS_MODULE);
 	if (IS_ERR(rtc->rtc_dev))
 		return PTR_ERR(rtc->rtc_dev);
 
-	da9063_data_to_tm(data, &rtc->alarm_time);
+	da9063_data_to_tm(data, &rtc->alarm_time, rtc);
 	rtc->rtc_sync = false;
-err:
 	return ret;
 }
 
@@ -341,6 +519,7 @@ static struct platform_driver da9063_rtc_driver = {
 	.probe		= da9063_rtc_probe,
 	.driver		= {
 		.name	= DA9063_DRVNAME_RTC,
+		.of_match_table = da9063_compatible_reg_id_table,
 	},
 };
 
@@ -348,5 +527,5 @@ module_platform_driver(da9063_rtc_driver);
 
 MODULE_AUTHOR("S Twiss <stwiss.opensource@diasemi.com>");
 MODULE_DESCRIPTION("Real time clock device driver for Dialog DA9063");
-MODULE_LICENSE("GPL v2");
+MODULE_LICENSE("GPL");
 MODULE_ALIAS("platform:" DA9063_DRVNAME_RTC);
-- 
end-of-patch for PATCH RFC V1

Re: [PATCH RFC V1 2/3] rtc: da9063: Add DA9062 RTC capability to DA9063 RTC driver

[Alexandre Belloni]

Hi,

On 09/07/2015 at 08:45:53 +0100, S Twiss wrote :
> From: S Twiss <stwiss.opensource@diasemi.com>
> 
> Add DA9062 RTC support into the existing DA9063 RTC driver component by
> using generic access tables for common register and bit mask definitions.
> 
> The following change will add generic register and bit mask support to the
> DA9063 RTC. The changes are slightly complicated by requiring support for
> three register sets: DA9063-AD, DA9063-BB and DA9062-AA.
> 
> The following alterations have been made to the DA9063 RTC:
> 
> - Addition of a da9063_compatible_rtc_regmap structure to hold all generic
>   registers and bitmasks for this type of RTC component.
> - A re-write of struct da9063 to use pointers for regmap and compatible
>   registers/masks definitions
> - Addition of a of_device_id table for DA9063 and DA9062 defaults
> - Refactoring functions to use struct da9063_compatible_rtc accesses to
>   generic registers/masks instead of using defines from registers.h
> - Addition of a re-try when reading the RTC inside da9063_rtc_read_time()

Can you separate that change in another patch as it is a change in the
behaviour of the driver? And maybe give a word or two on why this is
needed.

> - Re-work of da9063_rtc_probe() to use of_match_node() and dev_get_regmap()
>   to provide initialisation of generic registers and masks and access to
>   regmap
> 
> Signed-off-by: Steve Twiss <stwiss.opensource@diasemi.com>
> 
> ---
> Checks performed with linux-next/next-20150708/scripts/checkpatch.pl
>  Kconfig                   total: 0 errors, 15 warnings, 1600 lines checked
>  rtc-da9063.c              total: 0 errors, 0 warnings, 531 lines checked

This is not true, there is a warning:
WARNING: DT compatible string "dlg,da9062-rtc" appears un-documented
-- check /home/alex/M/linux/Documentation/devicetree/bindings/
#275: FILE: drivers/rtc/rtc-da9063.c:171:
+     { .compatible = "dlg,da9062-rtc", .data = &da9062_aa_regs },

patch 3/3 should come before 2/3 if you want to avoid that.

> diff --git a/drivers/rtc/rtc-da9063.c b/drivers/rtc/rtc-da9063.c
> index 7ffc570..e94fb6d 100644
> --- a/drivers/rtc/rtc-da9063.c
> +++ b/drivers/rtc/rtc-da9063.c
> @@ -1,127 +1,274 @@
> -/* rtc-da9063.c - Real time clock device driver for DA9063
> - * Copyright (C) 2013-14  Dialog Semiconductor Ltd.
> +/*
> + * Real time clock device driver for DA9063/DA9062
> + * Copyright (C) 2013-15  Dialog Semiconductor Ltd.
>   *
> - * This library is free software; you can redistribute it and/or
> - * modify it under the terms of the GNU Library General Public
> - * License as published by the Free Software Foundation; either
> - * version 2 of the License, or (at your option) any later version.
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * as published by the Free Software Foundation; either version 2
> + * of the License, or (at your option) any later version.
>   *
> - * This library is distributed in the hope that it will be useful,
> + * 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
> - * Library General Public License for more details.
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
>   */
>  

Please also list that license change in the commit log. It should also
probably be separated in another patch.

[...]

>  static int da9063_rtc_probe(struct platform_device *pdev)
>  {
> -	struct da9063 *da9063 = dev_get_drvdata(pdev->dev.parent);
> -	struct da9063_rtc *rtc;
> +	struct da9063_compatible_rtc *rtc;
> +	const struct da9063_compatible_rtc_regmap *config;
> +	const struct of_device_id *match;
>  	int irq_alarm;
>  	u8 data[RTC_DATA_LEN];
>  	int ret;
>  
> -	ret = regmap_update_bits(da9063->regmap, DA9063_REG_CONTROL_E,
> -				 DA9063_RTC_EN, DA9063_RTC_EN);
> +	match = of_match_node(da9063_compatible_reg_id_table,
> +			      pdev->dev.of_node);
> +	if (!match)

This will never happen if you are only probed from DT and this is waht
you expect now.

> +		return -ENXIO;
> +
> +	rtc = devm_kzalloc(&pdev->dev, sizeof(*rtc), GFP_KERNEL);
> +	if (!rtc)
> +		return -ENOMEM;
> +
> +	if (strncmp(match->name, "dlg,da9063-rtc", 14) == 0) {

You must not do that.

You should add a new compatible and change the of_compatible string of
the mfd_cell in drivers/mfd/da9063-core.c onc you know the variant.

> +		struct da9063 *chip = dev_get_drvdata(pdev->dev.parent);
> +
> +		if (chip->variant_code == PMIC_DA9063_AD)
> +			rtc->config = &da9063_ad_regs;
> +	} else
> +		rtc->config = match->data;


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

RE: [PATCH RFC V1 2/3] rtc: da9063: Add DA9062 RTC capability to DA9063 RTC driver

[Opensource [Steve Twiss]]

On 18 July 2015 00:45, Alexandre Belloni wrote:

> On 09/07/2015 at 08:45:53 +0100, S Twiss wrote :
> > From: S Twiss <stwiss.opensource@diasemi.com>
> >
> > Add DA9062 RTC support into the existing DA9063 RTC driver component by
> > using generic access tables for common register and bit mask definitions.
> >
> > The following change will add generic register and bit mask support to the
> > DA9063 RTC. The changes are slightly complicated by requiring support for
> > three register sets: DA9063-AD, DA9063-BB and DA9062-AA.
> >
> > The following alterations have been made to the DA9063 RTC:

[...]

> > - Addition of a re-try when reading the RTC inside da9063_rtc_read_time()
> 
> Can you separate that change in another patch as it is a change in the
> behaviour of the driver? And maybe give a word or two on why this is
> needed.

Sure.

I will separate this into a new patch and add it at a later date. It is a recommendation
to have this in for both DA9062/3 now, although I have not seen this happen during
my testing -- it should be added.
Something similar happened with the RTC previously -- everything tested fine 
with earlier kernels until the RTC core functions were improved and I picked up a
synchronisation problem with my testing:

http://lxr.free-electrons.com/source/drivers/rtc/rtc-da9063.c#L129

[...]

> > Checks performed with linux-next/next-20150708/scripts/checkpatch.pl
> >  Kconfig                   total: 0 errors, 15 warnings, 1600 lines checked
> >  rtc-da9063.c              total: 0 errors, 0 warnings, 531 lines checked
> 
> This is not true, there is a warning:
> WARNING: DT compatible string "dlg,da9062-rtc" appears un-documented
> -- check /home/alex/M/linux/Documentation/devicetree/bindings/
> #275: FILE: drivers/rtc/rtc-da9063.c:171:
> +     { .compatible = "dlg,da9062-rtc", .data = &da9062_aa_regs },
> 
> patch 3/3 should come before 2/3 if you want to avoid that.

I will re-order the patches accordingly.

[...]

> > diff --git a/drivers/rtc/rtc-da9063.c b/drivers/rtc/rtc-da9063.c
> > index 7ffc570..e94fb6d 100644
> > --- a/drivers/rtc/rtc-da9063.c
> > +++ b/drivers/rtc/rtc-da9063.c
> > @@ -1,127 +1,274 @@
> > -/* rtc-da9063.c - Real time clock device driver for DA9063
> > - * Copyright (C) 2013-14  Dialog Semiconductor Ltd.
> > +/*
> > + * Real time clock device driver for DA9063/DA9062
> > + * Copyright (C) 2013-15  Dialog Semiconductor Ltd.
> >   *
> > - * This library is free software; you can redistribute it and/or
> > - * modify it under the terms of the GNU Library General Public
> > - * License as published by the Free Software Foundation; either
> > - * version 2 of the License, or (at your option) any later version.
> > + * This program is free software; you can redistribute it and/or
> > + * modify it under the terms of the GNU General Public License
> > + * as published by the Free Software Foundation; either version 2
> > + * of the License, or (at your option) any later version.
> >   *
> > - * This library is distributed in the hope that it will be useful,
> > + * 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
> > - * Library General Public License for more details.
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > + * GNU General Public License for more details.
> >   */
> >
> 
> Please also list that license change in the commit log. It should also
> probably be separated in another patch.
> 

I can add this to a different patch and change it at a later date.
It is to fix a minor wording problem with the GPL header so it matches the correct
MODULE_LICENSE(""); macro. 

https://lkml.org/lkml/2015/4/18/19

The info at the top of the file needs to remove the LGPL worded text where appropriate:
words such as "Library" from the line "GNU Library General Public License"; and replace
the word "library" with "program" in several places

> [...]
> 
> >  static int da9063_rtc_probe(struct platform_device *pdev)
> >  {
> > -	struct da9063 *da9063 = dev_get_drvdata(pdev->dev.parent);
> > -	struct da9063_rtc *rtc;
> > +	struct da9063_compatible_rtc *rtc;
> > +	const struct da9063_compatible_rtc_regmap *config;
> > +	const struct of_device_id *match;
> >  	int irq_alarm;
> >  	u8 data[RTC_DATA_LEN];
> >  	int ret;
> >
> > -	ret = regmap_update_bits(da9063->regmap, DA9063_REG_CONTROL_E,
> > -				 DA9063_RTC_EN, DA9063_RTC_EN);
> > +	match = of_match_node(da9063_compatible_reg_id_table,
> > +			      pdev->dev.of_node);
> > +	if (!match)
> 
> This will never happen if you are only probed from DT and this is waht
> you expect now.

Yes, I will change this.

> > +		return -ENXIO;
> > +
> > +	rtc = devm_kzalloc(&pdev->dev, sizeof(*rtc), GFP_KERNEL);
> > +	if (!rtc)
> > +		return -ENOMEM;
> > +
> > +	if (strncmp(match->name, "dlg,da9063-rtc", 14) == 0) {
> 
> You must not do that.
> You should add a new compatible and change the of_compatible string of
> the mfd_cell in drivers/mfd/da9063-core.c onc you know the variant.

I can check for a binary comparison against the address of the 
static const struct of_device_id da9063_compatible_reg_id_table[] = {}
entry for DA9063.

The DA9063 driver already associates "dlg,da9063-rtc" with both BB and AD
register maps. I think that altering the string at this point would break backwards
compatibility in the device tree for the DA9063.

The DA9063 core dynamically checks for the chip variant at run-time and then saves
it in the "variant_code". This comes from the original submission of the driver.
Distinguishing DT compatible strings was suggested, but the method conflicted with the
automatic variant detection of the DA9063 chip, and so it was decided against.

https://lkml.org/lkml/2015/1/19/144

[...]

Regards,
Steve

Re: [PATCH RFC V1 2/3] rtc: da9063: Add DA9062 RTC capability to DA9063 RTC driver

[Alexandre Belloni]

On 20/07/2015 at 17:57:50 +0000, Opensource [Steve Twiss] wrote :
> On 18 July 2015 00:45, Alexandre Belloni wrote:
> > On 09/07/2015 at 08:45:53 +0100, S Twiss wrote :
> > > - Addition of a re-try when reading the RTC inside da9063_rtc_read_time()
> > 
> > Can you separate that change in another patch as it is a change in the
> > behaviour of the driver? And maybe give a word or two on why this is
> > needed.
> 
> Sure.
> 
> I will separate this into a new patch and add it at a later date. It is a recommendation
> to have this in for both DA9062/3 now, although I have not seen this happen during
> my testing -- it should be added.
> Something similar happened with the RTC previously -- everything tested fine 
> with earlier kernels until the RTC core functions were improved and I picked up a
> synchronisation problem with my testing:
> 
> http://lxr.free-electrons.com/source/drivers/rtc/rtc-da9063.c#L129
> 
> [...]
> 
> > > diff --git a/drivers/rtc/rtc-da9063.c b/drivers/rtc/rtc-da9063.c
> > > index 7ffc570..e94fb6d 100644
> > > --- a/drivers/rtc/rtc-da9063.c
> > > +++ b/drivers/rtc/rtc-da9063.c
> > > @@ -1,127 +1,274 @@
> > > -/* rtc-da9063.c - Real time clock device driver for DA9063
> > > - * Copyright (C) 2013-14  Dialog Semiconductor Ltd.
> > > +/*
> > > + * Real time clock device driver for DA9063/DA9062
> > > + * Copyright (C) 2013-15  Dialog Semiconductor Ltd.
> > >   *
> > > - * This library is free software; you can redistribute it and/or
> > > - * modify it under the terms of the GNU Library General Public
> > > - * License as published by the Free Software Foundation; either
> > > - * version 2 of the License, or (at your option) any later version.
> > > + * This program is free software; you can redistribute it and/or
> > > + * modify it under the terms of the GNU General Public License
> > > + * as published by the Free Software Foundation; either version 2
> > > + * of the License, or (at your option) any later version.
> > >   *
> > > - * This library is distributed in the hope that it will be useful,
> > > + * 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
> > > - * Library General Public License for more details.
> > > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > > + * GNU General Public License for more details.
> > >   */
> > >
> > 
> > Please also list that license change in the commit log. It should also
> > probably be separated in another patch.
> > 
> 
> I can add this to a different patch and change it at a later date.
> It is to fix a minor wording problem with the GPL header so it matches the correct
> MODULE_LICENSE(""); macro. 
> 
> https://lkml.org/lkml/2015/4/18/19
> 
> The info at the top of the file needs to remove the LGPL worded text where appropriate:
> words such as "Library" from the line "GNU Library General Public License"; and replace
> the word "library" with "program" in several places
> 

Yeah, I understood the change, it can go in as soon as you send the
patch.

> 
> > > +		return -ENXIO;
> > > +
> > > +	rtc = devm_kzalloc(&pdev->dev, sizeof(*rtc), GFP_KERNEL);
> > > +	if (!rtc)
> > > +		return -ENOMEM;
> > > +
> > > +	if (strncmp(match->name, "dlg,da9063-rtc", 14) == 0) {
> > 
> > You must not do that.
> > You should add a new compatible and change the of_compatible string of
> > the mfd_cell in drivers/mfd/da9063-core.c onc you know the variant.
> 
> I can check for a binary comparison against the address of the 
> static const struct of_device_id da9063_compatible_reg_id_table[] = {}
> entry for DA9063.

You also must not compare pointers that way. You can use
of_device_is_compatible().

> The DA9063 driver already associates "dlg,da9063-rtc" with both BB and AD
> register maps. I think that altering the string at this point would break backwards
> compatibility in the device tree for the DA9063.

I'm not fond of the backward compatibility exactly for that kind of
issue. The proper way to handle it is to have tow different compatible
strings because obviously, the BB and AD variants are not compatible.

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

RE: [PATCH RFC V1 2/3] rtc: da9063: Add DA9062 RTC capability to DA9063 RTC driver

[Opensource [Steve Twiss]]

Hi Alexandre,

Thank you for your quick responses on these patches.
I will probably need your further advice on the  of_compatible issue with the
DA9063 RTC  ... I've made some quick replies below.

On 20 July 2015 22:51 Alexandre Belloni wrote:

> On 20/07/2015 at 17:57:50 +0000, Opensource [Steve Twiss] wrote :
> > On 18 July 2015 00:45, Alexandre Belloni wrote:

[...]

> > > > diff --git a/drivers/rtc/rtc-da9063.c b/drivers/rtc/rtc-da9063.c
> > > > index 7ffc570..e94fb6d 100644
> > > > --- a/drivers/rtc/rtc-da9063.c
> > > > +++ b/drivers/rtc/rtc-da9063.c
> > > > @@ -1,127 +1,274 @@
> > > > -/* rtc-da9063.c - Real time clock device driver for DA9063
> > > > - * Copyright (C) 2013-14  Dialog Semiconductor Ltd.
> > > > +/*
> > > > + * Real time clock device driver for DA9063/DA9062
> > > > + * Copyright (C) 2013-15  Dialog Semiconductor Ltd.

[...]

> > > Please also list that license change in the commit log. It should also
> > > probably be separated in another patch.
> >
> > I can add this to a different patch and change it at a later date.
> > It is to fix a minor wording problem with the GPL header so it matches the
> > correct MODULE_LICENSE(""); macro.
> >

[...]

> Yeah, I understood the change, it can go in as soon as you send the
> patch.

Thanks,
I'll send this later.

> > > > +
> > > > +	rtc = devm_kzalloc(&pdev->dev, sizeof(*rtc), GFP_KERNEL);
> > > > +	if (!rtc)
> > > > +		return -ENOMEM;
> > > > +
> > > > +	if (strncmp(match->name, "dlg,da9063-rtc", 14) == 0) {
> > > > +		struct da9063 *chip = dev_get_drvdata(pdev->dev.parent);
> > > > +
> > > > +		if (chip->variant_code == PMIC_DA9063_AD)
> > > > +			rtc->config = &da9063_ad_regs;
> > > > +	} else
> > > > +		rtc->config = match->data;
> > >
> > > You must not do that.
> > > You should add a new compatible and change the of_compatible string of
> > > the mfd_cell in drivers/mfd/da9063-core.c onc you know the variant.
> >
> > I can check for a binary comparison against the address of the
> > static const struct of_device_id da9063_compatible_reg_id_table[] = {}
> > entry for DA9063.
> 
> You also must not compare pointers that way. You can use
> of_device_is_compatible().
> 
> > The DA9063 driver already associates "dlg,da9063-rtc" with both BB and AD
> > register maps. I think that altering the string at this point would break backwards
> > compatibility in the device tree for the DA9063.
> 
> I'm not fond of the backward compatibility exactly for that kind of
> issue. The proper way to handle it is to have tow different compatible
> strings because obviously, the BB and AD variants are not compatible.

I think I understand what you are saying about modifying the mfd_cell in the
DA9063 core driver at run-time so that it changes the of_compatible string to [something like]
"dlg,da9063-ad-rtc". That way the RTC driver can be probed using the correct
string.

Ok, I think I see now .. I'll make a patch v2 and send it soon.

Regards,
Steve

CC:
I've swapped Lee Jones into the TO field of this patch so he can start to have some
context on the next change to the da9063-core.c

Re: [PATCH RFC V1 2/3] rtc: da9063: Add DA9062 RTC capability to DA9063 RTC driver

[Alexandre Belloni]

Hi,

On 21/07/2015 at 08:05:32 +0000, Opensource [Steve Twiss] wrote :
> > > > > +
> > > > > +	rtc = devm_kzalloc(&pdev->dev, sizeof(*rtc), GFP_KERNEL);
> > > > > +	if (!rtc)
> > > > > +		return -ENOMEM;
> > > > > +
> > > > > +	if (strncmp(match->name, "dlg,da9063-rtc", 14) == 0) {
> > > > > +		struct da9063 *chip = dev_get_drvdata(pdev->dev.parent);
> > > > > +
> > > > > +		if (chip->variant_code == PMIC_DA9063_AD)
> > > > > +			rtc->config = &da9063_ad_regs;
> > > > > +	} else
> > > > > +		rtc->config = match->data;
> > > >
> > > > You must not do that.
> > > > You should add a new compatible and change the of_compatible string of
> > > > the mfd_cell in drivers/mfd/da9063-core.c onc you know the variant.
> > >
> > > I can check for a binary comparison against the address of the
> > > static const struct of_device_id da9063_compatible_reg_id_table[] = {}
> > > entry for DA9063.
> > 
> > You also must not compare pointers that way. You can use
> > of_device_is_compatible().
> > 
> > > The DA9063 driver already associates "dlg,da9063-rtc" with both BB and AD
> > > register maps. I think that altering the string at this point would break backwards
> > > compatibility in the device tree for the DA9063.
> > 
> > I'm not fond of the backward compatibility exactly for that kind of
> > issue. The proper way to handle it is to have tow different compatible
> > strings because obviously, the BB and AD variants are not compatible.
> 
> I think I understand what you are saying about modifying the mfd_cell in the
> DA9063 core driver at run-time so that it changes the of_compatible string to [something like]
> "dlg,da9063-ad-rtc". That way the RTC driver can be probed using the correct
> string.
> 
> Ok, I think I see now .. I'll make a patch v2 and send it soon.
> 

This is not needed if you simply change your strncmp with
of_device_is_compatible

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

[PATCH RFC V1 1/3] mfd: da9062: Support for the DA9063 RTC in the DA9062 core

[S Twiss]

From: S Twiss <stwiss.opensource@diasemi.com>

Add MFD core driver support for a RTC component

- MFD core adds the RTC resources da9062_rtc_resources[] for the RTC
  alarm and tick timer IRQ
- An appropriate mfd_cell has been added into da9062_devs[] to support
  a component .name = "da9062-rtc" and .of_compatible  = "dlg,da9062-rtc"

Signed-off-by: Steve Twiss <stwiss.opensource@diasemi.com>

---
Checks performed with linux-next/next-20150708/scripts/checkpatch.pl
 da9062-core.c             total: 0 errors, 0 warnings, 523 lines checked
This patch applies against linux-next and next-20150708 


 drivers/mfd/da9062-core.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/mfd/da9062-core.c b/drivers/mfd/da9062-core.c
index 4cf0643..b43cd24 100644
--- a/drivers/mfd/da9062-core.c
+++ b/drivers/mfd/da9062-core.c
@@ -118,6 +118,11 @@ static struct resource da9062_wdt_resources[] = {
 	DEFINE_RES_NAMED(DA9062_IRQ_WDG_WARN, 1, "WD_WARN", IORESOURCE_IRQ),
 };
 
+static struct resource da9062_rtc_resources[] = {
+	DEFINE_RES_NAMED(DA9062_IRQ_ALARM, 1, "ALARM", IORESOURCE_IRQ),
+	DEFINE_RES_NAMED(DA9062_IRQ_TICK, 1, "TICK", IORESOURCE_IRQ),
+};
+
 static const struct mfd_cell da9062_devs[] = {
 	{
 		.name		= "da9062-core",
@@ -141,6 +146,12 @@ static const struct mfd_cell da9062_devs[] = {
 		.resources	= da9062_thermal_resources,
 		.of_compatible  = "dlg,da9062-thermal",
 	},
+	{
+		.name		= "da9062-rtc",
+		.num_resources	= ARRAY_SIZE(da9062_rtc_resources),
+		.resources	= da9062_rtc_resources,
+		.of_compatible  = "dlg,da9062-rtc",
+	},
 };
 
 static int da9062_clear_fault_log(struct da9062 *chip)
-- 
end-of-patch for PATCH RFC V1

[PATCH RFC V1 3/3] devicetree: da9062: Add device tree bindings for DA9062 RTC

[S Twiss]

From: S Twiss <stwiss.opensource@diasemi.com>

Add device tree bindings for the DA9062 RTC driver component

Signed-off-by: Steve Twiss <stwiss.opensource@diasemi.com>

---
Checks performed with linux-next/next-20150708/scripts/checkpatch.pl
 da9062.txt                total: 0 errors, 0 warnings, 88 lines checked
This patch applies against linux-next and next-20150708 


 Documentation/devicetree/bindings/mfd/da9062.txt | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/Documentation/devicetree/bindings/mfd/da9062.txt b/Documentation/devicetree/bindings/mfd/da9062.txt
index 5765ed9..38802b5 100644
--- a/Documentation/devicetree/bindings/mfd/da9062.txt
+++ b/Documentation/devicetree/bindings/mfd/da9062.txt
@@ -5,6 +5,7 @@ DA9062 consists of a large and varied group of sub-devices:
 Device                   Supply Names    Description
 ------                   ------------    -----------
 da9062-regulator        :               : LDOs & BUCKs
+da9062-rtc              :               : Real-Time Clock
 da9062-watchdog         :               : Watchdog Timer
 
 ======
@@ -41,6 +42,10 @@ Sub-nodes:
   Documentation/devicetree/bindings/regulator/regulator.txt
 
 
+- rtc : This node defines settings required for the Real-Time Clock associated
+  with the DA9062. There are currently no entries in this binding, however
+  compatible = "dlg,da9062-rtc" should be added if a node is created.
+
 - watchdog: This node defines the settings for the watchdog driver associated
   with the DA9062 PMIC. The compatible = "dlg,da9062-watchdog" should be added
   if a node is created.
@@ -55,6 +60,10 @@ Example:
 		interrupts = <11 IRQ_TYPE_LEVEL_LOW>;
 		interrupt-controller;
 
+		rtc {
+			compatible = "dlg,da9062-rtc";
+		};
+
 		watchdog {
 			compatible = "dlg,da9062-watchdog";
 		};
-- 
end-of-patch for PATCH RFC V1

Re: [PATCH RFC V1 3/3] devicetree: da9062: Add device tree bindings for DA9062 RTC

[Alexandre Belloni]

On 09/07/2015 at 08:45:54 +0100, S Twiss wrote :
> From: S Twiss <stwiss.opensource@diasemi.com>
> 
> Add device tree bindings for the DA9062 RTC driver component
> 
> Signed-off-by: Steve Twiss <stwiss.opensource@diasemi.com>
> 
> ---
> Checks performed with linux-next/next-20150708/scripts/checkpatch.pl
>  da9062.txt                total: 0 errors, 0 warnings, 88 lines checked
> This patch applies against linux-next and next-20150708 
> 
> 
>  Documentation/devicetree/bindings/mfd/da9062.txt | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/mfd/da9062.txt b/Documentation/devicetree/bindings/mfd/da9062.txt
> index 5765ed9..38802b5 100644
> --- a/Documentation/devicetree/bindings/mfd/da9062.txt
> +++ b/Documentation/devicetree/bindings/mfd/da9062.txt
> @@ -5,6 +5,7 @@ DA9062 consists of a large and varied group of sub-devices:
>  Device                   Supply Names    Description
>  ------                   ------------    -----------
>  da9062-regulator        :               : LDOs & BUCKs
> +da9062-rtc              :               : Real-Time Clock
>  da9062-watchdog         :               : Watchdog Timer
>  
>  ======
> @@ -41,6 +42,10 @@ Sub-nodes:
>    Documentation/devicetree/bindings/regulator/regulator.txt
>  
>  
> +- rtc : This node defines settings required for the Real-Time Clock associated
> +  with the DA9062. There are currently no entries in this binding, however
> +  compatible = "dlg,da9062-rtc" should be added if a node is created.
> +
>  - watchdog: This node defines the settings for the watchdog driver associated
>    with the DA9062 PMIC. The compatible = "dlg,da9062-watchdog" should be added
>    if a node is created.
> @@ -55,6 +60,10 @@ Example:
>  		interrupts = <11 IRQ_TYPE_LEVEL_LOW>;
>  		interrupt-controller;
>  
> +		rtc {
> +			compatible = "dlg,da9062-rtc";
> +		};
> +

Are those subnodes even parsed? As the parent node is not compatible
with "simple-bus" or "simple-mfd", I guess not. You driver is probed
from the mfd_cells in da9063-core.c

>  		watchdog {
>  			compatible = "dlg,da9062-watchdog";
>  		};
> -- 
> end-of-patch for PATCH RFC V1
> 

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

RE: [PATCH RFC V1 3/3] devicetree: da9062: Add device tree bindings for DA9062 RTC

[Opensource [Steve Twiss]]

On 18 July 2015 00:48, Alexandre Belloni wrote:

> On 09/07/2015 at 08:45:54 +0100, S Twiss wrote :
> > From: S Twiss <stwiss.opensource@diasemi.com>
> >
> > Add device tree bindings for the DA9062 RTC driver component
> >
> > Signed-off-by: Steve Twiss <stwiss.opensource@diasemi.com>

[...]

> > +- rtc : This node defines settings required for the Real-Time Clock associated
> > +  with the DA9062. There are currently no entries in this binding, however
> > +  compatible = "dlg,da9062-rtc" should be added if a node is created.

[...]

> > @@ -55,6 +60,10 @@ Example:
> >  		interrupts = <11 IRQ_TYPE_LEVEL_LOW>;
> >  		interrupt-controller;
> >
> > +		rtc {
> > +			compatible = "dlg,da9062-rtc";
> > +		};
> > +
> 
> Are those subnodes even parsed? As the parent node is not compatible
> with "simple-bus" or "simple-mfd", I guess not. You driver is probed
> from the mfd_cells in da9063-core.c

Hi Alexandre,

If I miss out the compatible line for the RTC in the DTSI for DA9062, or is I alter it
so it does not match the string specified in the DA9063 RTC driver, the DA9063 RTC
driver will not be probed for the DA9062 compatibility.
The RTC compatible = "dlg,da9062-rtc"  is essential in this case.

[...]

Regards,
Stephen

Re: [PATCH RFC V1 3/3] devicetree: da9062: Add device tree bindings for DA9062 RTC

[Alexandre Belloni]

Hi,

On 20/07/2015 at 17:08:00 +0000, Opensource [Steve Twiss] wrote :
> If I miss out the compatible line for the RTC in the DTSI for DA9062, or is I alter it
> so it does not match the string specified in the DA9063 RTC driver, the DA9063 RTC
> driver will not be probed for the DA9062 compatibility.
> The RTC compatible = "dlg,da9062-rtc"  is essential in this case.
> 

You are right. Disregard my comment.

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

Re: [PATCH RFC V1 0/3] da9062: Add DA9062 RTC support using the existing DA9063 RTC driver

[Geert Uytterhoeven]

Hi Steve,

On Thu, Jul 9, 2015 at 9:45 AM, S Twiss <stwiss.opensource@diasemi.com> wrote:
> From: S Twiss <stwiss.opensource@diasemi.com>
>
> This patch set adds RTC support for the Dialog DA9062 Power Management IC.
> Changes are made to the existing DA9063 RTC component so that functionality
> in this device driver can be re-used to support the DA9062 RTC.

[...]

> S Twiss (3):
>   mfd: da9062: Support for the DA9063 RTC in the DA9062 core

"DA9062 RTC"?

>   rtc: da9063: Add DA9062 RTC capability to DA9063 RTC driver
>   devicetree: da9062: Add device tree bindings for DA9062 RTC

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

RE: [PATCH RFC V1 0/3] da9062: Add DA9062 RTC support using the existing DA9063 RTC driver

[Opensource [Steve Twiss]]

On 09 July 2015 09:02, Geert Uytterhoeven wrote:

> Subject: Re: [PATCH RFC V1 0/3] da9062: Add DA9062 RTC support using the
> existing DA9063 RTC driver
> 
> Hi Steve,
> 
> On Thu, Jul 9, 2015 at 9:45 AM, S Twiss <stwiss.opensource@diasemi.com>
> wrote:
> > From: S Twiss <stwiss.opensource@diasemi.com>
> >
> > This patch set adds RTC support for the Dialog DA9062 Power Management IC.
> > Changes are made to the existing DA9063 RTC component so that functionality
> > in this device driver can be re-used to support the DA9062 RTC.
> 
> [...]
> 
> > S Twiss (3):
> >   mfd: da9062: Support for the DA9063 RTC in the DA9062 core
> 
> "DA9062 RTC"?
> 

Hi Geert,

It's the support for the "DA9063 RTC *driver* in the DA9062 MFD driver core".

I'm re-working the DA9063 RTC driver to support DA9062 RTC hardware. This will make
the name "DA9063 RTC driver" interchangeable with "DA9062 RTC driver", because if this
patch goes through they will both be using the original "DA9063 RTC driver".

Confused?  :-/

I suppose I could also have called this patch "support for the DA9062 RTC H/W in the
DA9062 MFD driver core".

This comes from a previous thread here:
https://lkml.org/lkml/2015/5/13/406
https://lkml.org/lkml/2015/5/13/528

Regards,
Steve

[...]

Re: [PATCH RFC V1 0/3] da9062: Add DA9062 RTC support using the existing DA9063 RTC driver

[Geert Uytterhoeven]

On Thu, Jul 9, 2015 at 10:42 AM, Opensource [Steve Twiss]
<stwiss.opensource-WBD+wuPFNBhBDgjK7y7TUQ@public.gmane.org> wrote:
>> > S Twiss (3):
>> >   mfd: da9062: Support for the DA9063 RTC in the DA9062 core
>>
>> "DA9062 RTC"?
>>
>
> Hi Geert,
>
> It's the support for the "DA9063 RTC *driver* in the DA9062 MFD driver core".

Ah, I read "core" as in "hardware IP core".

And as the full patch description for [1/3] doesn't say "da9063" elsewhere,
I thought it was a typo.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html