ISL12057 alarm support and isil vs isl fix

Hi Andrew, Uwe,

Andrew, the first patch of this series is a new version of the one you have in your tree, providing the changes asked by Uwe (i.e. you can just replace yours by that one). If you prefer an additional commit over what you already have, do not hesitate to tell me. Below, you have a diff between previous (the one you have) and this first patch, w/ some inline comments. Uwe, tell me if it is what you expected.

The three patches for RN102, RN104 and RN2120 .dts files just add a boolean property to the ISL12057 node to mark it has a wakeup source, as proposed by Uwe. If first patch is ok, I think those three can either be merged by Jason or directly by you, Andrew.

The last patch tries and fix the isl vs isil issue which led to the conflict between the previous version of this patch and two patches merged upstream by Darshana, *by using isil everywhere and deprecating isl*. Arnd is also in copy to provide some feedback. At the moment, this last patch and the first one of this series can be applied in whatever order but I think this is just luck (first and last patch both touch rtc-isl12057.c file) so we'll have to be careful if it's merged by different maintainers.

Cheers,

a+

Some comments on the changes in first patch:

-diff -puN drivers/rtc/rtc-isl12057.c~rtc-rtc-isl12057-add-alarm-support-to-intersil-isl12057-rtc-driver drivers/rtc/rtc-isl12057.c ---- a/drivers/rtc/rtc-isl12057.c~rtc-rtc-isl12057-add-alarm-support-to-intersil-isl12057-rtc-driver
-+++ a/drivers/rtc/rtc-isl12057.c
+Index: linux/drivers/rtc/rtc-isl12057.c

+===================================================================
+--- linux.orig/drivers/rtc/rtc-isl12057.c

++++ linux/drivers/rtc/rtc-isl12057.c
@@ -1,5 +1,5 @@
/*
- * rtc-isl12057 - Driver for Intersil ISL12057 I2C Real Time Clock
@@ -335,10 +310,44 @@
static int isl12057_rtc_set_time(struct device *dev, struct rtc_time *tm) {
struct isl12057_rtc_data *data = dev_get_drvdata(dev);
-@@ -262,9 +457,48 @@ static int isl12057_check_rtc_status(str +@@ -262,9 +457,82 @@ static int isl12057_check_rtc_status(str
return 0;
}

a small helper. Note: the only way to mark the device is a wakeup source is via system .dts file. Uwe, now is the time to decide if we put a prefix before the property and which ;-)

I have also added a Documentation/devicetree/bindings/rtc/isil,isl12057.txt in first patch to describe the purpose of the property and give some example. I decided to let the device in the list of trivial I2C device though, as this property is not per see required by the device, nor specific to ISL12057 but to the way the chip IRQ line is connected (or not connected) to the SoC.

 .../devicetree/bindings/i2c/trivial-devices.txt    |   5 +-
 .../devicetree/bindings/regulator/isl9305.txt      |   4 +-
 .../devicetree/bindings/rtc/isil,isl12057.txt      |  71 +++++
 .../devicetree/bindings/vendor-prefixes.txt        |   3 +-
 arch/arm/boot/dts/armada-370-netgear-rn102.dts     |   1 +
 arch/arm/boot/dts/armada-370-netgear-rn104.dts     |   1 +
 arch/arm/boot/dts/armada-xp-netgear-rn2120.dts     |   1 +
 arch/arm/boot/dts/tegra30-cardhu.dtsi              |   2 +-
 arch/arm/boot/dts/zynq-parallella.dts              |   2 +-
 drivers/regulator/isl9305.c                        |   6 +-
 drivers/rtc/rtc-isl12022.c                         |   3 +-
 drivers/rtc/rtc-isl12057.c                         | 353 ++++++++++++++++++++-
 drivers/staging/iio/light/isl29028.c               |   4 +-
 13 files changed, 431 insertions(+), 25 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/rtc/isil,isl12057.txt

2.1.1

This patch adds alarm support to Intersil ISL12057 driver. This allows to configure the chip to generate an interrupt when the alarm matches current time value. Alarm can be programmed up to one month in the future and is accurate to the second.

The patch was developed to support two different configurations:
systems w/ and w/o RTC chip IRQ line connected to the main CPU.

The latter is the one found on current 3 kernel users of the chip for which support was initially developed (Netgear ReadyNAS 102, 104 and 2120 NAS). On those devices, the IRQ#2 pin of the chip is not connected to the SoC but to a PMIC. This allows setting an alarm, powering off the device and have it wake up when the alarm rings. To support that configuration the driver does the following:

1. it has alarm_irq_enable() function returns -ENOTTY when no IRQ is passed to the driver.
2. it marks the device as a wakeup source if an IRQ is passed to to the driver or if the boolean "can-wakeup-machine" is set in system .dts file to have 'wakealarm' sysfs entry created.
3. it marks the device has not supporting UIE mode when no IRQ is passed to the driver (see the commmit message of c9f5c7e7a84f)

This specific configuration was tested on a ReadyNAS 102 by setting an alarm, powering off the device and see it reboot as expected when the alarm rang.

The former configuration was tested on a Netgear ReadyNAS 102 after some soldering of the IRQ#2 pin of the RTC chip to a MPP line of the SoC (the one used usually handles the reset button). The test was performed using a modified .dts file reflecting this change (see below) and rtc-test.c program available in Documentation/rtc.txt. This test program ran as expected, which validates alarm supports, including interrupt support.

As a side note, the ISL12057 remains in the list of trivial devices, i.e. no specific DT binding being added by this patch: i2c core automatically handles extraction of IRQ line info from .dts file. For instance, if one wants to reference the interrupt line for the alarm in its .dts file, adding interrupt and interrupt-parent properties works as expected (if the primary function of your interrupt pin is not GPIO, you will need some additional pinctrl properties):

          isl12057: isl12057 at 68 {
                  compatible = "isil,isl12057";
                  interrupt-parent = <&gpio0>;
                  interrupts = <6 IRQ_TYPE_EDGE_FALLING>;
                  reg = <0x68>;
          };

FWIW, if someone is looking for a way to test alarm support on a system on which the chip IRQ line has the ability to boot the system (e.g. ReadyNAS 102, 104, etc):

    # echo `date '+%s' -d '+ 1 minutes'` > /sys/class/rtc/rtc0/wakealarm
    # shutdown -h now

With the commands above, after a minute, the system comes back to life.

As a final note, the driver does not provide support for the second alarm of the chip (this second alarm does not support second).

---
 .../devicetree/bindings/rtc/isil,isl12057.txt      |  71 +++++
 drivers/rtc/rtc-isl12057.c                         | 350 ++++++++++++++++++++-
 2 files changed, 411 insertions(+), 10 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/rtc/isil,isl12057.txt

new file mode 100644
index 000000000000..111bb8a6c274
@@ -0,0 +1,71 @@
+Intersil ISL12057 I2C RTC/Alarm chip
+
+ISL12057 is a trivial I2C device (it has simple device tree bindings, consisting
+of a compatible field, an address and possibly an interrupt line).
+
+Nonetheless, it supports an additional boolean property ("can-wakeup-machine")
+to handle the specific use-case found on the three initial in-tree users of the
+chip (NETGEAR ReadyNAS 102, 104 and 2120 ARM-based NAS); On those devices, the
+IRQ#2 pin of the chip (associated with the alarm supported by the driver) is not
+connected to the SoC but to a PMIC. It allows the device to be powered up when
+RTC alarm rings. In order to mark the device has a wakeup source and get access
+to the 'wakealarm' sysfs entry, this specific property can be set when the IRQ#2
+pin of the chip is not connected to the SoC.
+
+Required properties supported by the device:
+
+ - "compatible": must be "isil,isl12057"
+ - "reg": I2C bus address of the device
+
+Optional properties:
+
+ - "can-wakeup-machine": mark the chip as a wakeup source, independently of
+			 the availability of an IRQ line connected to the SoC.
+
+
+Example isl12057 node without IRQ#2 pin connected (no alarm support):
+
+	isl12057: isl12057 at 68 {
+		compatible = "isil,isl12057";
+		reg = <0x68>;
+	};
+
+
+Example isl12057 node with IRQ#2 pin connected to main SoC via MPP6 (note
+that the pinctrl-related properties below are given for completeness and
+may not be required depending on your system or SoC, and the main function
+of the MPP used as IRQ line, i.e. "interrupt-parent" and "interrupts" are
+usually sufficient):
+
+		    pinctrl {
+				...
+
+				rtc_alarm_pin: rtc_alarm_pin {
+					marvell,pins = "mpp6";
+					marvell,function = "gpio";
+				};
+
+				...
+
+		    };
+
+	...
+
+	isl12057: isl12057 at 68 {
+		compatible = "isil,isl12057";
+		reg = <0x68>;
+		pinctrl-0 = <&rtc_alarm_pin>;
+		pinctrl-names = "default";
+		interrupt-parent = <&gpio0>;
+		interrupts = <6 IRQ_TYPE_EDGE_FALLING>;
+	};
+
+
+Example isl12057 node without IRQ#2 pin connected to the SoC but to a
+PMIC, allowing the device to be started based on configured alarm:
+
+	isl12057: isl12057 at 68 {
+		compatible = "isil,isl12057";
+		reg = <0x68>;
+		can-wakeup-machine;
+	};

2.1.1

Hello,

I adapted the subject line to make the critical element in this patch more obvious.

As I wouldn't be surprised to get some discussion about your approach it would be beneficial to split this patch into:

- add (traditional) alarm support to rtc-isl12057
- allow the driver to wakeup the machine without the irq being reported to the CPU.

The first patch should not be controversial and the patch to discuss gets smaller.

	isl12057: isl12057 at 68 {
		compatible = "isil,isl12057";
		reg = <0x68>;
		interrupt-parent = <&wakeupfoo>;
		interrupts = <???>
	};

	wakeupfoo: ... {
		/*
		 * This controller cannot report irqs to the cpu, but
		 * can wake it up.
		 */
		....
	}

Hi Uwe,

Thanks for bearing w/ me,

thanks.

Will do that. But it would be good we end up w/ a solution for current in-tree users of the driver, i.e. those w/ a need for the second patch ;-)

I handled that in details in the documentation (w/ a typo I will fix, I must confess). The answer is: if you put a "can-wakeup-machine" property in your .dts for ISL12057, then it means IRQ#2 can wake up the machine BUT is not hooked up to the SoC. And only that. Alarm2 is not supported (it can generate an interrupt on IRQ#1 or IRQ#2) so the driver does not deal w/ IRQ#1 at all.

That being said, one can still add support later for Alarm2 (via IRQ#1 or IRQ#2) and the "can-wakeup-machine" property you proposed will still handle that w/o issue.

As a side note, on our ReadyNAS, we can easily test a support for alarm2 on IRQ#2 pin. But In the end, the main problem will still be core rtc
interface: how do you handle the existence of two alarms on a RTC chip using current sysfs interface, for instance?

I agree you had a good idea in the first place not to force the rtc as wakeup source and make that configurable via the .dts based on specific system setup. But I think we should keep the knob simple.

What you propose is probably more accurate w/ the existence of a PMIC in the device but IMHO the main purpose of the property is to tell the driver that IRQ#2 (associated w/ the alarm1 it supports) can wake the machine up. It would be more complex than needed for the driver to try and understand that in fact the controller and irq that are passed are placebo and it should the declare the device as a wakeup source.

What about changing the property name from "can-wakeup-machine" to "isil,irq2-can-wakeup-machine" instead? It makes it clear that it is specific to that driver, and that it only works w/ IRQ#2.

Will drop thoses changes.

Cheers,

a+

Now that alarm support for ISL12057 chip is available, let's use a feature of the driver dedicated to NETGEAR ReadyNAS 102 specific routing of RTC IRQ#2 pin; on the device, this pin is not connected to the SoC but to a PMIC, which allows the device to be powered up when RTC alarm rings.

For that to work, the chip needs to be explicitly marked as a device wakeup source using the "can-wakeup-machine" boolean property. This makes 'wakealarm' sysfs entry available to configure the alarm.

---
 arch/arm/boot/dts/armada-370-netgear-rn102.dts | 1 +
 1 file changed, 1 insertion(+)

index 4e24932c6e30..3b0f2b88ce9e 100644
@@ -87,6 +87,7 @@
 				isl12057: isl12057 at 68 {
 					compatible = "isil,isl12057";
 					reg = <0x68>;
+					can-wakeup-machine;
 				};
 
 				g762: g762 at 3e {

2.1.1

Please group patches 2 - 4 into one patch.

thx,

Jason.

Will do.

a+

Now that alarm support for ISL12057 chip is available, let's use a feature of the driver dedicated to NETGEAR ReadyNAS 104 specific routing of RTC IRQ#2 pin; on the device, this pin is not connected to the SoC but to a PMIC, which allows the device to be powered up when RTC alarm rings.

For that to work, the chip needs to be explicitly marked as a device wakeup source using the "can-wakeup-machine" boolean property. This makes 'wakealarm' sysfs entry available to configure the alarm.

---
 arch/arm/boot/dts/armada-370-netgear-rn104.dts | 1 +
 1 file changed, 1 insertion(+)

index 30586e47986a..997a691c9f6c 100644
@@ -93,6 +93,7 @@
 				isl12057: isl12057 at 68 {
 					compatible = "isil,isl12057";
 					reg = <0x68>;
+					can-wakeup-machine;
 				};
 
 				g762: g762 at 3e {

2.1.1

Now that alarm support for ISL12057 chip is available, let's use a feature of the driver dedicated to NETGEAR ReadyNAS 2120 specific routing of RTC IRQ#1 pin; on the device, this pin is not connected to the SoC but to a PMIC, which allows the device to be powered up when RTC alarm rings.

For that to work, the chip needs to be explicitly marked as a device wakeup source using the "can-wakeup-machine" boolean property. This makes 'wakealarm' sysfs entry available to configure the alarm.

---
 arch/arm/boot/dts/armada-xp-netgear-rn2120.dts | 1 +
 1 file changed, 1 insertion(+)

index d81430aa4ab3..f5c7cb52f1f9 100644
@@ -100,6 +100,7 @@
 				isl12057: isl12057 at 68 {
 					compatible = "isil,isl12057";
 					reg = <0x68>;
+					can-wakeup-machine;
 				};
 
 				/* Controller for rear fan #1 of 3 (Protechnic

2.1.1

When Intersil ISL12057 driver was introduced by commit 70e123373c05 ("rtc: Add support for Intersil ISL12057 I2C RTC chip"), the vendor prefix 'isl' was used instead of the expected 'isil' (Intersil NASDAQ symbol) and documented in vendor-prefixes.txt.

Recently, a patch from Philip Zabel (7a6540ca856a, "ARM: mvebu:
Change vendor prefix for Intersil Corporation to isil") fixed that prefix in ReadyNAS devices .dts files (AFAICT, the only users of the driver).

Then, commits 7c75c1d5e72b ("dt-bindings: Document deprecated device vendor name to fix related warning") and b2ea3f82e798 (dt-bindings:
Document correct and deprecated vendor-prefix with device isl29028) decided to go the other way and deprecate isil in vendor-prefixes.txt and in isl29028.c staging driver.

While trying and merge a fix I wrote for ISL12057 drivers to finish Philip's work, it conflicted with the two recently introduced commits, and revealed the issue: at the moment, there are various compatible strings in drivers and .dts files for Intersil products which use either isl or isil:

$ grep -R "isil," .
./Documentation/devicetree/bindings/i2c/trivial-devices.txt:isil,isl29028           (deprecated, use isl)
./drivers/staging/iio/light/isl29028.c:   { .compatible = "isil,isl29028", },/* deprecated, don't use */
./drivers/staging/iio/light/isl29018.c:   { .compatible = "isil,isl29018", },
./drivers/staging/iio/light/isl29018.c:   { .compatible = "isil,isl29023", },
./drivers/staging/iio/light/isl29018.c:   { .compatible = "isil,isl29035", },
./arch/powerpc/boot/dts/p1022rdk.dts:        compatible = "isil,zl6100";
./arch/powerpc/boot/dts/p1022rdk.dts:        compatible = "isil,zl6100";
./arch/powerpc/boot/dts/p1022rdk.dts:        compatible = "isil,zl6100";
./arch/powerpc/boot/dts/p1022rdk.dts:        compatible = "isil,zl6100";
./arch/arm/boot/dts/exynos5800-peach-pi.dts: compatible = "isil,isl29018";
./arch/arm/boot/dts/imx6qdl-nitrogen6x.dtsi: compatible = "isil,isl1208";
./arch/arm/boot/dts/tegra20-ventana.dts:     compatible = "isil,isl29018";
./arch/arm/boot/dts/tegra20-seaboard.dts:    compatible = "isil,isl29018";
./arch/arm/boot/dts/armada-xp-netgear-rn2120.dts: compatible = "isil,isl12057";
./arch/arm/boot/dts/armada-370-netgear-rn104.dts: compatible = "isil,isl12057";
./arch/arm/boot/dts/exynos5420-peach-pit.dts:     compatible = "isil,isl29018";
./arch/arm/boot/dts/armada-370-netgear-rn102.dts: compatible = "isil,isl12057";

$ grep -R "isl," .
./Documentation/devicetree/bindings/regulator/isl9305.txt:- compatible: "isl,isl9305" or "isl,isl9305h"
./Documentation/devicetree/bindings/regulator/isl9305.txt:              compatible = "isl,isl9305";
./Documentation/devicetree/bindings/i2c/trivial-devices.txt:isl,isl12057      Intersil ISL12057 I2C RTC Chip
./Documentation/devicetree/bindings/i2c/trivial-devices.txt:isl,isl29028      Intersil ISL29028 Ambient Light ....
./drivers/regulator/isl9305.c:          { .compatible = "isl,isl9305" },
./drivers/regulator/isl9305.c:          { .compatible = "isl,isl9305h" },
./drivers/staging/iio/light/isl29028.c: { .compatible = "isl,isl29028", },
./drivers/rtc/rtc-isl12057.c:           { .compatible = "isl,isl12057" },
./drivers/rtc/rtc-isl12022.c:           { .compatible = "isl,isl12022" },
./arch/arm/boot/dts/tegra30-cardhu.dtsi:   compatible = "isl,isl29028";
./arch/arm/boot/dts/zynq-parallella.dts:   compatible = "isl,isl9305";

AFAICT, it seems it makes sense to *definitively* settle for isil as the vendor prefix for Intersil, as Philip did in 7a6540ca856a: it's the NASDAQ symbol and this choice requires less changes than opting for isl.

So, this patch changes compatible strings in .dts files to use isil where isl was found before, and modify drivers w/ compatible strings using isl to add one using isil. In those cases, a comment is made that the old compatible string is kept for backward compatibility (w/ out-fo-tree users of those drivers). Additionally, it leaves only isil as prefix in vendor-prefixes.txt. Those changes should prevent any new inclusion of isl compatible strings for Intersil devices due to copy-and-paste.

---
 Documentation/devicetree/bindings/i2c/trivial-devices.txt | 5 ++---
 Documentation/devicetree/bindings/regulator/isl9305.txt   | 4 ++--
 Documentation/devicetree/bindings/vendor-prefixes.txt     | 3 +--
 arch/arm/boot/dts/tegra30-cardhu.dtsi                     | 2 +-
 arch/arm/boot/dts/zynq-parallella.dts                     | 2 +-
 drivers/regulator/isl9305.c                               | 6 ++++--
 drivers/rtc/rtc-isl12022.c                                | 3 ++-
 drivers/rtc/rtc-isl12057.c                                | 3 ++-
 drivers/staging/iio/light/isl29028.c                      | 4 ++--
 9 files changed, 17 insertions(+), 15 deletions(-)

index 605dcca5dbec..213855bb419b 100644
@@ -55,9 +55,8 @@ fsl,sgtl5000		SGTL5000: Ultra Low-Power Audio Codec
 gmt,g751		G751: Digital Temperature Sensor and Thermal Watchdog with Two-Wire Interface
 infineon,slb9635tt	Infineon SLB9635 (Soft-) I2C TPM (old protocol, max 100khz)
 infineon,slb9645tt	Infineon SLB9645 I2C TPM (new protocol, max 400khz)
-isl,isl12057		Intersil ISL12057 I2C RTC Chip
-isil,isl29028           (deprecated, use isl)
-isl,isl29028            Intersil ISL29028 Ambient Light and Proximity Sensor
+isil,isl12057		Intersil ISL12057 I2C RTC/Alarm Chip
+isil,isl29028           Intersil ISL29028 Ambient Light and Proximity Sensor
 maxim,ds1050		5 Bit Programmable, Pulse-Width Modulator
 maxim,max1237		Low-Power, 4-/12-Channel, 2-Wire Serial, 12-Bit ADCs
 maxim,max6625		9-Bit/12-Bit Temperature Sensors with I?C-Compatible Serial Interface

index a626fc1bbf0d..d6e7c9ec9413 100644
@@ -2,7 +2,7 @@ Intersil ISL9305/ISL9305H voltage regulator
 
 Required properties:
 
-- compatible: "isl,isl9305" or "isl,isl9305h"
+- compatible: "isil,isl9305" or "isil,isl9305h"
 - reg: I2C slave address, usually 0x68.
 - regulators: A node that houses a sub-node for each regulator within the
   device. Each sub-node is identified using the node's name, with valid

@@ -19,7 +19,7 @@ Optional properties:
 Example
 
 	pmic: isl9305 at 68 {
-		compatible = "isl,isl9305";
+		compatible = "isil,isl9305";
 		reg = <0x68>;
 
 		VINDCD1-supply = <&system_power>;

index cc6151c431c8..ba574e431c5a 100644
@@ -79,8 +79,7 @@ innolux	Innolux Corporation
 intel	Intel Corporation
 intercontrol	Inter Control Group
 isee	ISEE 2007 S.L.
-isil    Intersil (deprecated, use isl)
-isl	Intersil
+isil	Intersil
 karo	Ka-Ro electronics GmbH
 keymile	Keymile GmbH
 lacie	LaCie

index cbf5a1ae0ca7..a1b682ea01bd 100644
@@ -189,7 +189,7 @@
 
 		/* ALS and Proximity sensor */
 		isl29028 at 44 {
-			compatible = "isl,isl29028";
+			compatible = "isil,isl29028";
 			reg = <0x44>;
 			interrupt-parent = <&gpio>;
 			interrupts = <TEGRA_GPIO(L, 0) IRQ_TYPE_LEVEL_HIGH>;

index ab1dc0a56cdd..174571232ea5 100644
@@ -58,7 +58,7 @@
 	status = "okay";
 
 	isl9305: isl9305 at 68 {
-		compatible = "isl,isl9305";
+		compatible = "isil,isl9305";
 		reg = <0x68>;
 
 		regulators {

index 92fefd98da58..6e3a15fe00f1 100644
@@ -177,8 +177,10 @@ static int isl9305_i2c_probe(struct i2c_client *i2c,
 
 #ifdef CONFIG_OF
 static const struct of_device_id isl9305_dt_ids[] = {
-	{ .compatible = "isl,isl9305" },
-	{ .compatible = "isl,isl9305h" },
+	{ .compatible = "isl,isl9305" }, /* for backward compat., don't use */
+	{ .compatible = "isil,isl9305" },
+	{ .compatible = "isl,isl9305h" }, /* for backward compat., don't use */
+	{ .compatible = "isil,isl9305h" },
 	{},
 };
 #endif

index ee3ba7e6b45e..f9b082784b90 100644
@@ -275,7 +275,8 @@ static int isl12022_probe(struct i2c_client *client,
 
 #ifdef CONFIG_OF
 static const struct of_device_id isl12022_dt_match[] = {
-	{ .compatible = "isl,isl12022" },
+	{ .compatible = "isl,isl12022" }, /* for backward compat., don't use */
+	{ .compatible = "isil,isl12022" },
 	{ },
 };
 #endif

index 98adc2c1bdb0..949121ed0255 100644
@@ -644,7 +644,8 @@ static SIMPLE_DEV_PM_OPS(isl12057_rtc_pm_ops, isl12057_rtc_suspend,
 
 #ifdef CONFIG_OF
 static const struct of_device_id isl12057_dt_match[] = {
-	{ .compatible = "isl,isl12057" },
+	{ .compatible = "isl,isl12057" }, /* for backward compat., don't use */
+	{ .compatible = "isil,isl12057" },
 	{ },
 };
 #endif

index e969107ddb47..6440e3b293ca 100644
@@ -537,8 +537,8 @@ static const struct i2c_device_id isl29028_id[] = {
 MODULE_DEVICE_TABLE(i2c, isl29028_id);
 
 static const struct of_device_id isl29028_of_match[] = {
-	{ .compatible = "isl,isl29028", },
-	{ .compatible = "isil,isl29028", },/* deprecated, don't use */
+	{ .compatible = "isl,isl29028", }, /* for backward compat., don't use */
+	{ .compatible = "isil,isl29028", },
 	{ },
 };
 MODULE_DEVICE_TABLE(of, isl29028_of_match);

2.1.1

Please split the dts{i} changes out into a separate patch. The different maintainers under drivers/ may want separate patches as well.

thx,

Jason.

Hi,

I will prepare that, and then let get_maintainer.pl decide who should be added to the CC: list. But before doing that work, I would like to at least get some feedback that there will not be a big NAK on the whole approach in the end.

Cheers,

a+

As long as you are maintaining compatibility with old dtbs (which you are), then there shouldn't be a problem. I think the original dust up occurred because two different maintainers merged two different solutions and didn't run into each other. This looks like a sane cleanup of the resulting mess. :)

thx,

Jason.