Skip to contentSkip to conversation

da9062: Add DA9062 RTC support using the existing DA9063 RTC driver

RFC PATCH In review S Twiss 3 participants 15 messages Jul 9, 2015 – Jul 21, 2015

Subsystems (5)
lore
Share & tools
lore

S Twiss

author
#0 lore raw

cover da9062: Add DA9062 RTC support using the existing DA9063 RTC driver

From: S Twiss <[email protected]>

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(-)
Signature · 1 line
end-of-patch for PATCH RFC V1
author
#1 lore raw

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

From: S Twiss <[email protected]>

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"

---
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(+)
drivers/mfd/da9062-core.c +11 -0
index 4cf0643..b43cd24 100644
@@ -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)
Signature · 1 line
end-of-patch for PATCH RFC V1
author
#2 lore raw

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

In reply to S Twiss

From: S Twiss <[email protected]>

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

---
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(-)
drivers/rtc/Kconfig +3 -3
index 83b4b89..1eaa268 100644
@@ -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".
drivers/rtc/rtc-da9063.c +307 -128

615 lines not on this page · raw message

Signature · 1 line
end-of-patch for PATCH RFC V1
#3 lore raw

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

Hi,

Quoted 11 lines · S Twiss

From: S Twiss <[email protected]>

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:

Quoted 8 lines

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

Quoted 10 lines

- 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

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

Quoted 19 lines
drivers/rtc/rtc-da9063.c +7 -6
index 7ffc570..e94fb6d 100644
@@ -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.

drivers/rtc/rtc-da9063.c line 15

Quoted 10 lines
drivers/rtc/rtc-da9063.c +3 -3
  *
- * 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.

[...]

Quoted 6 lines
drivers/rtc/rtc-da9063.c +2 -2
 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;

drivers/rtc/rtc-da9063.c static const struct rtc_class_ops da9063_rtc_ops = { · line 417

Quoted 10 lines
drivers/rtc/rtc-da9063.c +4 -2
+	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.

drivers/rtc/rtc-da9063.c static const struct rtc_class_ops da9063_rtc_ops = { · line 424

Quoted 7 lines
drivers/rtc/rtc-da9063.c +7 -0
+		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.

Quoted 6 lines
drivers/rtc/rtc-da9063.c +6 -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;
Signature · 3 lines
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
#4 lore raw

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

Quoted 11 lines · Alexandre Belloni
Quoted 10 lines · S Twiss

From: S Twiss <[email protected]>

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:

[...]

Quoted 5 lines
Quoted 1 line

- 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

[...]

Quoted 11 lines
Quoted 3 lines

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.

[...]

Quoted 24 lines
Quoted 24 lines
drivers/rtc/rtc-da9063.c +8 -8
index 7ffc570..e94fb6d 100644
@@ -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
Quoted 9 lines

drivers/rtc/rtc-da9063.c line 15

Quoted 5 lines
drivers/rtc/rtc-da9063.c +2 -1
- * 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

Quoted 13 lines

[...]

Quoted 11 lines

static int da9063_rtc_probe(struct platform_device *pdev) {

drivers/rtc/rtc-da9063.c +3 -2
-	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;
Quoted 8 lines

drivers/rtc/rtc-da9063.c static const struct rtc_class_ops da9063_rtc_ops = { · line 417

Quoted 5 lines
drivers/rtc/rtc-da9063.c +3 -2
-	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.

Quoted 11 lines

drivers/rtc/rtc-da9063.c static const struct rtc_class_ops da9063_rtc_ops = { · line 424

Quoted 7 lines
drivers/rtc/rtc-da9063.c +7 -0
+		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

#5 lore raw

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

Quoted 59 lines · Opensource [Steve Twiss]
Quoted 6 lines · Alexandre Belloni
Quoted 1 line · S Twiss

- 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

[...]

Quoted 24 lines
Quoted 24 lines
drivers/rtc/rtc-da9063.c +8 -8
index 7ffc570..e94fb6d 100644
@@ -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
Quoted 9 lines

drivers/rtc/rtc-da9063.c line 15

Quoted 5 lines
drivers/rtc/rtc-da9063.c +2 -1
- * 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.

Quoted 6 lines

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.

Quoted 13 lines
Quoted 12 lines

drivers/rtc/rtc-da9063.c static const struct rtc_class_ops da9063_rtc_ops = { · line 424

Quoted 8 lines
drivers/rtc/rtc-da9063.c +7 -0
+		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.

Quoted 3 lines

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().

Quoted 3 lines

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.

Signature · 3 lines
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
#6 lore raw

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

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.

Quoted 2 lines · On 20 July 2015 22:51 Alexandre Belloni wrote:
Quoted 1 line · Opensource [Steve Twiss]

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

[...]

Quoted 10 lines
Quoted 10 lines
Quoted 10 lines
Quoted 10 lines
drivers/rtc/rtc-da9063.c +3 -2
index 7ffc570..e94fb6d 100644
@@ -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.

[...]

Quoted 7 lines
Quoted 7 lines
Quoted 2 lines

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.

[...]

Quoted 2 lines

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

Thanks,
I'll send this later.

Quoted 24 lines
Quoted 17 lines
Quoted 8 lines
Quoted 8 lines
drivers/rtc/rtc-da9063.c +8 -0
+
+	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);
+
Quoted 8 lines

drivers/rtc/rtc-da9063.c static const struct rtc_class_ops da9063_rtc_ops = { · line 430

Quoted 4 lines
drivers/rtc/rtc-da9063.c +4 -0
+		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.

Quoted 3 lines

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().

Quoted 7 lines
Quoted 3 lines

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

#7 lore raw

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

Hi,

Quoted 32 lines · Opensource [Steve Twiss]
Quoted 24 lines
Quoted 17 lines
Quoted 8 lines
Quoted 8 lines
drivers/rtc/rtc-da9063.c +8 -0
+
+	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);
+
Quoted 8 lines

drivers/rtc/rtc-da9063.c static const struct rtc_class_ops da9063_rtc_ops = { · line 430

Quoted 4 lines
drivers/rtc/rtc-da9063.c +4 -0
+		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.

Quoted 3 lines

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().

Quoted 7 lines
Quoted 3 lines

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.

Quoted 7 lines

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

Signature · 3 lines
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
author
#8 lore raw

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

In reply to S Twiss

From: S Twiss <[email protected]>

Add device tree bindings for the DA9062 RTC driver component

---
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(+)
Documentation/devicetree/bindings/mfd/da9062.txt +9 -0
index 5765ed9..38802b5 100644
@@ -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";
 		};
Signature · 1 line
end-of-patch for PATCH RFC V1
#9 lore raw

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

Quoted 39 lines · S Twiss

From: S Twiss <[email protected]>

Add device tree bindings for the DA9062 RTC driver component

---
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(+)
Documentation/devicetree/bindings/mfd/da9062.txt +5 -0
index 5765ed9..38802b5 100644
@@ -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.

Documentation/devicetree/bindings/mfd/da9062.txt Example: · line 66

Quoted 7 lines
Documentation/devicetree/bindings/mfd/da9062.txt +4 -0
@@ -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

Quoted 6 lines
Documentation/devicetree/bindings/mfd/da9062.txt +0 -0
 		watchdog {
 			compatible = "dlg,da9062-watchdog";
 		};
Signature · 1 line
end-of-patch for PATCH RFC V1
Signature · 3 lines
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
#10 lore raw

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

Quoted 6 lines · Alexandre Belloni
Quoted 5 lines · S Twiss

From: S Twiss <[email protected]>

Add device tree bindings for the DA9062 RTC driver component

[...]

Quoted 3 lines
Quoted 3 lines

+- rtc : This node defines settings required for the Real-Time Clock associated

Documentation/devicetree/bindings/mfd/da9062.txt +2 -0
+  with the DA9062. There are currently no entries in this binding, however
+  compatible = "dlg,da9062-rtc" should be added if a node is created.

[...]

Quoted 4 lines
Quoted 4 lines
Documentation/devicetree/bindings/mfd/da9062.txt +0 -0
@@ -55,6 +60,10 @@ Example:
 		interrupts = <11 IRQ_TYPE_LEVEL_LOW>;
 		interrupt-controller;
 
Quoted 8 lines

Documentation/devicetree/bindings/mfd/da9062.txt Example: · line 66

Quoted 4 lines
Documentation/devicetree/bindings/mfd/da9062.txt +4 -0
+		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

#11 lore raw

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

Hi,

Quoted 5 lines · Opensource [Steve Twiss]

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.

Signature · 3 lines
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
#12 lore raw
In reply to S Twiss

Hi Steve,

Quoted 5 lines · S Twiss

From: S Twiss <[email protected]>

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.

[...]

Quoted 2 lines

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

"DA9062 RTC"?

Quoted 2 lines

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 -- [email protected]

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

#13 lore raw

T24gMDkgSnVseSAyMDE1IDA5OjAyLCBHZWVydCBVeXR0ZXJob2V2ZW4gd3JvdGU6DQoNCj4gU3Vi amVjdDogUmU6IFtQQVRDSCBSRkMgVjEgMC8zXSBkYTkwNjI6IEFkZCBEQTkwNjIgUlRDIHN1cHBv cnQgdXNpbmcgdGhlDQo+IGV4aXN0aW5nIERBOTA2MyBSVEMgZHJpdmVyDQo+IA0KPiBIaSBTdGV2 ZSwNCj4gDQo+IE9uIFRodSwgSnVsIDksIDIwMTUgYXQgOTo0NSBBTSwgUyBUd2lzcyA8c3R3aXNz Lm9wZW5zb3VyY2VAZGlhc2VtaS5jb20+DQo+IHdyb3RlOg0KPiA+IEZyb206IFMgVHdpc3MgPHN0 d2lzcy5vcGVuc291cmNlQGRpYXNlbWkuY29tPg0KPiA+DQo+ID4gVGhpcyBwYXRjaCBzZXQgYWRk cyBSVEMgc3VwcG9ydCBmb3IgdGhlIERpYWxvZyBEQTkwNjIgUG93ZXIgTWFuYWdlbWVudCBJQy4N Cj4gPiBDaGFuZ2VzIGFyZSBtYWRlIHRvIHRoZSBleGlzdGluZyBEQTkwNjMgUlRDIGNvbXBvbmVu dCBzbyB0aGF0IGZ1bmN0aW9uYWxpdHkNCj4gPiBpbiB0aGlzIGRldmljZSBkcml2ZXIgY2FuIGJl IHJlLXVzZWQgdG8gc3VwcG9ydCB0aGUgREE5MDYyIFJUQy4NCj4gDQo+IFsuLi5dDQo+IA0KPiA+ IFMgVHdpc3MgKDMpOg0KPiA+ICAgbWZkOiBkYTkwNjI6IFN1cHBvcnQgZm9yIHRoZSBEQTkwNjMg UlRDIGluIHRoZSBEQTkwNjIgY29yZQ0KPiANCj4gIkRBOTA2MiBSVEMiPw0KPiANCg0KSGkgR2Vl cnQsDQoNCkl0J3MgdGhlIHN1cHBvcnQgZm9yIHRoZSAiREE5MDYzIFJUQyAqZHJpdmVyKiBpbiB0 aGUgREE5MDYyIE1GRCBkcml2ZXIgY29yZSIuDQoNCkknbSByZS13b3JraW5nIHRoZSBEQTkwNjMg UlRDIGRyaXZlciB0byBzdXBwb3J0IERBOTA2MiBSVEMgaGFyZHdhcmUuIFRoaXMgd2lsbCBtYWtl DQp0aGUgbmFtZSAiREE5MDYzIFJUQyBkcml2ZXIiIGludGVyY2hhbmdlYWJsZSB3aXRoICJEQTkw NjIgUlRDIGRyaXZlciIsIGJlY2F1c2UgaWYgdGhpcw0KcGF0Y2ggZ29lcyB0aHJvdWdoIHRoZXkg d2lsbCBib3RoIGJlIHVzaW5nIHRoZSBvcmlnaW5hbCAiREE5MDYzIFJUQyBkcml2ZXIiLg0KDQpD b25mdXNlZD8gIDotLw0KDQpJIHN1cHBvc2UgSSBjb3VsZCBhbHNvIGhhdmUgY2FsbGVkIHRoaXMg cGF0Y2ggInN1cHBvcnQgZm9yIHRoZSBEQTkwNjIgUlRDIEgvVyBpbiB0aGUNCkRBOTA2MiBNRkQg ZHJpdmVyIGNvcmUiLg0KDQpUaGlzIGNvbWVzIGZyb20gYSBwcmV2aW91cyB0aHJlYWQgaGVyZToN Cmh0dHBzOi8vbGttbC5vcmcvbGttbC8yMDE1LzUvMTMvNDA2DQpodHRwczovL2xrbWwub3JnL2xr bWwvMjAxNS81LzEzLzUyOA0KDQpSZWdhcmRzLA0KU3RldmUNCg0KWy4uLl0NCg== --
To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

#14 lore raw
Quoted 9 lines · Opensource [Steve Twiss]
Quoted 5 lines
Quoted 2 lines

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 -- [email protected]

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 [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html