All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4 0/5] PCA9633: Add support to PCA9634 and fix some problems
@ 2013-08-14 21:23 Ricardo Ribalda Delgado
  2013-08-14 21:23 ` [PATCH v5 1/5] leds-pca9633: Add support for PCA9634 Ricardo Ribalda Delgado
                   ` (5 more replies)
  0 siblings, 6 replies; 7+ messages in thread
From: Ricardo Ribalda Delgado @ 2013-08-14 21:23 UTC (permalink / raw)
  To: Bryan Wu, Peter Meerwald, Richard Purdie, Linux LED Subsystem, LKML
  Cc: Ricardo Ribalda Delgado

Add Support for the PCA9634 chip. Simimart to the 9633, but with 8 outputs instead of 4.
Fix bug when 2 chips where present on the system, the ledclass will fail and the chip wont probe.
Protect ledout register with a mutex to support updates of more than leds at the same time
Fix device tree parsing

v5: Contains feedback from Bryan Wu
Bryan: Fix device tree bindings documentation

v4: Rebase to latest leds-next and new patch to Fix the dt parsing

v3: Contains feedback from Bryan Wu
Bryan: Rename pca9633 to pca963x

v2: Contains feedback from Peter Meerwald
Peter: Fix typo on commit message. Add bus number to name

Ricardo Ribalda Delgado (5):
  leds-pca9633: Add support for PCA9634
  leds-pca9633: Unique naming of the LEDs
  leds-pca9633: Add mutex to the ledout register
  leds-pca9633: Rename to leds-pca963x
  leds-pca963x: Fix device tree parsing

 Documentation/devicetree/bindings/leds/pca9633.txt |   46 --
 Documentation/devicetree/bindings/leds/pca963x.txt |   47 ++
 drivers/leds/Kconfig                               |    9 +-
 drivers/leds/Makefile                              |    2 +-
 drivers/leds/leds-pca9633.c                        |  393 -----------------
 drivers/leds/leds-pca963x.c                        |  461 ++++++++++++++++++++
 include/linux/platform_data/leds-pca9633.h         |   41 --
 include/linux/platform_data/leds-pca963x.h         |   42 ++
 8 files changed, 556 insertions(+), 485 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/leds/pca9633.txt
 create mode 100644 Documentation/devicetree/bindings/leds/pca963x.txt
 delete mode 100644 drivers/leds/leds-pca9633.c
 create mode 100644 drivers/leds/leds-pca963x.c
 delete mode 100644 include/linux/platform_data/leds-pca9633.h
 create mode 100644 include/linux/platform_data/leds-pca963x.h

-- 
1.7.10.4


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH v5 1/5] leds-pca9633: Add support for PCA9634
  2013-08-14 21:23 [PATCH v4 0/5] PCA9633: Add support to PCA9634 and fix some problems Ricardo Ribalda Delgado
@ 2013-08-14 21:23 ` Ricardo Ribalda Delgado
  2013-08-14 21:23 ` [PATCH v5 2/5] leds-pca9633: Unique naming of the LEDs Ricardo Ribalda Delgado
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Ricardo Ribalda Delgado @ 2013-08-14 21:23 UTC (permalink / raw)
  To: Bryan Wu, Peter Meerwald, Richard Purdie, Linux LED Subsystem, LKML
  Cc: Ricardo Ribalda Delgado

Add support for PCA9634 chip, which belongs to the same family as the
9633 but with support for 8 outputs instead of 4.

Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
---
 Documentation/devicetree/bindings/leds/pca9633.txt |   11 +-
 drivers/leds/Kconfig                               |    7 +-
 drivers/leds/leds-pca9633.c                        |  108 ++++++++++++++------
 3 files changed, 85 insertions(+), 41 deletions(-)

diff --git a/Documentation/devicetree/bindings/leds/pca9633.txt b/Documentation/devicetree/bindings/leds/pca9633.txt
index 6d9e1a9..aece3ea 100644
--- a/Documentation/devicetree/bindings/leds/pca9633.txt
+++ b/Documentation/devicetree/bindings/leds/pca9633.txt
@@ -1,24 +1,25 @@
-LEDs connected to pca9633 or pca9632
+LEDs connected to pca9632, pca9633 or pca9634
 
 Required properties:
-- compatible : should be : "nxp,pca963x"
+- compatible : should be : "nxp,pca9632", "nxp,pca9633" or "nxp,pca9634"
 
 Optional properties:
 - nxp,totem-pole : use totem pole (push-pull) instead of default open-drain
 - nxp,hw-blink : use hardware blinking instead of software blinking
 
-Each led is represented as a sub-node of the nxp,pca9633 device.
+Each led is represented as a sub-node of the nxp,pca963x device.
 
 LED sub-node properties:
 - label : (optional) see Documentation/devicetree/bindings/leds/common.txt
-- reg : number of LED line (could be from 0 to 4)
+- reg : number of LED line (could be from 0 to 3  in pca9632 or pca9633
+		or 0 to 7 in pca9634)
 - linux,default-trigger : (optional)
    see Documentation/devicetree/bindings/leds/common.txt
 
 Examples:
 
 pca9632: pca9632 {
-	compatible = "nxp,pca9632", "nxp,pca963x";
+	compatible = "nxp,pca9632";
 	#address-cells = <1>;
 	#size-cells = <0>;
 	reg = <0x62>;
diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index f2c738d..e7977aa 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -292,12 +292,13 @@ config LEDS_PCA955X
 	  devices include PCA9550, PCA9551, PCA9552, and PCA9553.
 
 config LEDS_PCA9633
-	tristate "LED support for PCA9633 I2C chip"
+	tristate "LED support for PCA963x I2C chip"
 	depends on LEDS_CLASS
 	depends on I2C
 	help
-	  This option enables support for LEDs connected to the PCA9633
-	  LED driver chip accessed via the I2C bus.
+	  This option enables support for LEDs connected to the PCA963x
+	  LED driver chip accessed via the I2C bus. Supported
+	  devices include PCA9633 and PCA9634
 
 config LEDS_WM831X_STATUS
 	tristate "LED support for status LEDs on WM831x PMICs"
diff --git a/drivers/leds/leds-pca9633.c b/drivers/leds/leds-pca9633.c
index ecd1449..e59ca0a 100644
--- a/drivers/leds/leds-pca9633.c
+++ b/drivers/leds/leds-pca9633.c
@@ -1,7 +1,9 @@
 /*
  * Copyright 2011 bct electronic GmbH
+ * Copyright 2013 Qtechnology/AS
  *
  * Author: Peter Meerwald <p.meerwald@bct-electronic.com>
+ * Author: Ricardo Ribalda <ricardo.ribalda@gmail.com>
  *
  * Based on leds-pca955x.c
  *
@@ -10,6 +12,7 @@
  * directory of this archive for more details.
  *
  * LED driver for the PCA9633 I2C LED driver (7-bit slave address 0x62)
+ * LED driver for the PCA9634 I2C LED driver (7-bit slave address set by hw.)
  *
  * Note that hardware blinking violates the leds infrastructure driver
  * interface since the hardware only supports blinking all LEDs with the
@@ -45,16 +48,42 @@
 #define PCA9633_MODE1		0x00
 #define PCA9633_MODE2		0x01
 #define PCA9633_PWM_BASE	0x02
-#define PCA9633_GRPPWM		0x06
-#define PCA9633_GRPFREQ		0x07
-#define PCA9633_LEDOUT		0x08
+
+enum pca9633_type {
+	pca9633,
+	pca9634,
+};
+
+struct pca9633_chipdef {
+	u8			grppwm;
+	u8			grpfreq;
+	u8			ledout_base;
+	int			n_leds;
+};
+
+static struct pca9633_chipdef pca9633_chipdefs[] = {
+	[pca9633] = {
+		.grppwm		= 0x6,
+		.grpfreq	= 0x7,
+		.ledout_base	= 0x8,
+		.n_leds		= 4,
+	},
+	[pca9634] = {
+		.grppwm		= 0xa,
+		.grpfreq	= 0xb,
+		.ledout_base	= 0xc,
+		.n_leds		= 8,
+	},
+};
 
 /* Total blink period in milliseconds */
 #define PCA9632_BLINK_PERIOD_MIN	42
 #define PCA9632_BLINK_PERIOD_MAX	10667
 
 static const struct i2c_device_id pca9633_id[] = {
-	{ "pca9633", 0 },
+	{ "pca9632", pca9633 },
+	{ "pca9633", pca9633 },
+	{ "pca9634", pca9634 },
 	{ }
 };
 MODULE_DEVICE_TABLE(i2c, pca9633_id);
@@ -66,10 +95,11 @@ enum pca9633_cmd {
 
 struct pca9633_led {
 	struct i2c_client *client;
+	struct pca9633_chipdef *chipdef;
 	struct work_struct work;
 	enum led_brightness brightness;
 	struct led_classdev led_cdev;
-	int led_num; /* 0 .. 3 potentially */
+	int led_num; /* 0 .. 7 potentially */
 	enum pca9633_cmd cmd;
 	char name[32];
 	u8 gdc;
@@ -78,24 +108,26 @@ struct pca9633_led {
 
 static void pca9633_brightness_work(struct pca9633_led *pca9633)
 {
-	u8 ledout = i2c_smbus_read_byte_data(pca9633->client, PCA9633_LEDOUT);
-	int shift = 2 * pca9633->led_num;
+	u8 ledout_addr = pca9633->chipdef->ledout_base + (pca9633->led_num / 4);
+	u8 ledout;
+	int shift = 2 * (pca9633->led_num % 4);
 	u8 mask = 0x3 << shift;
 
+	ledout = i2c_smbus_read_byte_data(pca9633->client, ledout_addr);
 	switch (pca9633->brightness) {
 	case LED_FULL:
-		i2c_smbus_write_byte_data(pca9633->client, PCA9633_LEDOUT,
+		i2c_smbus_write_byte_data(pca9633->client, ledout_addr,
 			(ledout & ~mask) | (PCA9633_LED_ON << shift));
 		break;
 	case LED_OFF:
-		i2c_smbus_write_byte_data(pca9633->client, PCA9633_LEDOUT,
+		i2c_smbus_write_byte_data(pca9633->client, ledout_addr,
 			ledout & ~mask);
 		break;
 	default:
 		i2c_smbus_write_byte_data(pca9633->client,
 			PCA9633_PWM_BASE + pca9633->led_num,
 			pca9633->brightness);
-		i2c_smbus_write_byte_data(pca9633->client, PCA9633_LEDOUT,
+		i2c_smbus_write_byte_data(pca9633->client, ledout_addr,
 			(ledout & ~mask) | (PCA9633_LED_PWM << shift));
 		break;
 	}
@@ -103,15 +135,16 @@ static void pca9633_brightness_work(struct pca9633_led *pca9633)
 
 static void pca9633_blink_work(struct pca9633_led *pca9633)
 {
-	u8 ledout = i2c_smbus_read_byte_data(pca9633->client, PCA9633_LEDOUT);
+	u8 ledout_addr = pca9633->chipdef->ledout_base + (pca9633->led_num / 4);
+	u8 ledout = i2c_smbus_read_byte_data(pca9633->client, ledout_addr);
 	u8 mode2 = i2c_smbus_read_byte_data(pca9633->client, PCA9633_MODE2);
-	int shift = 2 * pca9633->led_num;
+	int shift = 2 * (pca9633->led_num % 4);
 	u8 mask = 0x3 << shift;
 
-	i2c_smbus_write_byte_data(pca9633->client, PCA9633_GRPPWM,
+	i2c_smbus_write_byte_data(pca9633->client, pca9633->chipdef->grppwm,
 		pca9633->gdc);
 
-	i2c_smbus_write_byte_data(pca9633->client, PCA9633_GRPFREQ,
+	i2c_smbus_write_byte_data(pca9633->client, pca9633->chipdef->grpfreq,
 		pca9633->gfrq);
 
 	if (!(mode2 & PCA9633_MODE2_DMBLNK))
@@ -119,7 +152,7 @@ static void pca9633_blink_work(struct pca9633_led *pca9633)
 			mode2 | PCA9633_MODE2_DMBLNK);
 
 	if ((ledout & mask) != (PCA9633_LED_GRP_PWM << shift))
-		i2c_smbus_write_byte_data(pca9633->client, PCA9633_LEDOUT,
+		i2c_smbus_write_byte_data(pca9633->client, ledout_addr,
 			(ledout & ~mask) | (PCA9633_LED_GRP_PWM << shift));
 }
 
@@ -215,7 +248,7 @@ static int pca9633_blink_set(struct led_classdev *led_cdev,
 
 #if IS_ENABLED(CONFIG_OF)
 static struct pca9633_platform_data *
-pca9633_dt_init(struct i2c_client *client)
+pca9633_dt_init(struct i2c_client *client, struct pca9633_chipdef *chip)
 {
 	struct device_node *np = client->dev.of_node, *child;
 	struct pca9633_platform_data *pdata;
@@ -223,11 +256,11 @@ pca9633_dt_init(struct i2c_client *client)
 	int count;
 
 	count = of_get_child_count(np);
-	if (!count || count > 4)
+	if (!count || count > chip->n_leds)
 		return ERR_PTR(-ENODEV);
 
 	pca9633_leds = devm_kzalloc(&client->dev,
-				sizeof(struct led_info) * count, GFP_KERNEL);
+			sizeof(struct led_info) * chip->n_leds, GFP_KERNEL);
 	if (!pca9633_leds)
 		return ERR_PTR(-ENOMEM);
 
@@ -269,12 +302,14 @@ pca9633_dt_init(struct i2c_client *client)
 }
 
 static const struct of_device_id of_pca9633_match[] = {
-	{ .compatible = "nxp,pca963x", },
+	{ .compatible = "nxp,pca9632", },
+	{ .compatible = "nxp,pca9633", },
+	{ .compatible = "nxp,pca9634", },
 	{},
 };
 #else
 static struct pca9633_platform_data *
-pca9633_dt_init(struct i2c_client *client)
+pca9633_dt_init(struct i2c_client *client, struct pca9633_chipdef *chip)
 {
 	return ERR_PTR(-ENODEV);
 }
@@ -285,34 +320,38 @@ static int pca9633_probe(struct i2c_client *client,
 {
 	struct pca9633_led *pca9633;
 	struct pca9633_platform_data *pdata;
+	struct pca9633_chipdef *chip;
 	int i, err;
 
+	chip = &pca9633_chipdefs[id->driver_data];
 	pdata = dev_get_platdata(&client->dev);
 
 	if (!pdata) {
-		pdata = pca9633_dt_init(client);
+		pdata = pca9633_dt_init(client, chip);
 		if (IS_ERR(pdata)) {
 			dev_warn(&client->dev, "could not parse configuration\n");
 			pdata = NULL;
 		}
 	}
 
-	if (pdata) {
-		if (pdata->leds.num_leds <= 0 || pdata->leds.num_leds > 4) {
-			dev_err(&client->dev, "board info must claim at most 4 LEDs");
-			return -EINVAL;
-		}
+	if (pdata && (pdata->leds.num_leds < 1 ||
+				 pdata->leds.num_leds > chip->n_leds)) {
+		dev_err(&client->dev, "board info must claim 1-%d LEDs",
+								chip->n_leds);
+		return -EINVAL;
 	}
 
-	pca9633 = devm_kzalloc(&client->dev, 4 * sizeof(*pca9633), GFP_KERNEL);
+	pca9633 = devm_kzalloc(&client->dev, chip->n_leds * sizeof(*pca9633),
+								GFP_KERNEL);
 	if (!pca9633)
 		return -ENOMEM;
 
 	i2c_set_clientdata(client, pca9633);
 
-	for (i = 0; i < 4; i++) {
+	for (i = 0; i < chip->n_leds; i++) {
 		pca9633[i].client = client;
 		pca9633[i].led_num = i;
+		pca9633[i].chipdef = chip;
 
 		/* Platform data can specify LED names and default triggers */
 		if (pdata && i < pdata->leds.num_leds) {
@@ -349,7 +388,9 @@ static int pca9633_probe(struct i2c_client *client,
 		i2c_smbus_write_byte_data(client, PCA9633_MODE2, 0x01);
 
 	/* Turn off LEDs */
-	i2c_smbus_write_byte_data(client, PCA9633_LEDOUT, 0x00);
+	i2c_smbus_write_byte_data(client, chip->ledout_base, 0x00);
+	if (chip->n_leds > 4)
+		i2c_smbus_write_byte_data(client, chip->ledout_base + 1, 0x00);
 
 	return 0;
 
@@ -367,10 +408,11 @@ static int pca9633_remove(struct i2c_client *client)
 	struct pca9633_led *pca9633 = i2c_get_clientdata(client);
 	int i;
 
-	for (i = 0; i < 4; i++) {
-		led_classdev_unregister(&pca9633[i].led_cdev);
-		cancel_work_sync(&pca9633[i].work);
-	}
+	for (i = 0; i < pca9633->chipdef->n_leds; i++)
+		if (pca9633[i].client != NULL) {
+			led_classdev_unregister(&pca9633[i].led_cdev);
+			cancel_work_sync(&pca9633[i].work);
+		}
 
 	return 0;
 }
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v5 2/5] leds-pca9633: Unique naming of the LEDs
  2013-08-14 21:23 [PATCH v4 0/5] PCA9633: Add support to PCA9634 and fix some problems Ricardo Ribalda Delgado
  2013-08-14 21:23 ` [PATCH v5 1/5] leds-pca9633: Add support for PCA9634 Ricardo Ribalda Delgado
@ 2013-08-14 21:23 ` Ricardo Ribalda Delgado
  2013-08-14 21:23 ` [PATCH v5 3/5] leds-pca9633: Add mutex to the ledout register Ricardo Ribalda Delgado
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Ricardo Ribalda Delgado @ 2013-08-14 21:23 UTC (permalink / raw)
  To: Bryan Wu, Peter Meerwald, Richard Purdie, Linux LED Subsystem, LKML
  Cc: Ricardo Ribalda Delgado

If there is more than one pca963x chips on the system and there are
some LEDs without platform_data names, the driver wont be able to
provide unique naming to them.

This will cause led_class_dev_register to fail, unregistering all the
LEDs of the chip.

This patch adds the i2c address to the name of the unnamed LEDs, making
them unique.

[  555.346827] ------------[ cut here ]------------
[  555.346844] WARNING: at /build/linux-voe0Su/linux-3.9.8/fs/sysfs/dir.c:536 sysfs_add_one+0x8b/0x9d()
[  555.346847] Hardware name: QT5022
[  555.346850] sysfs: cannot create duplicate filename '/class/leds/pca9633:6'
[  555.346853] Modules linked in: qt5038_platform(O+) leds_pca9633(O) hid_generic ledtrig_default_on rfcomm bnep bluetooth binfmt_misc nfsd auth_rpcgss nfs_acl nfs lockd dns_resolver fscache sunrpc nls_utf8 nls_cp437 vfat fat loop fuse joydev hid_multitouch usbhid hid acpi_cpufreq mperf kvm_amd kvm evdev pn533 nfc arc4 microcode pcspkr efivars k10temp ath9k ath9k_common ath9k_hw ath fglrx(PO) mac80211 cfg80211 video rfkill processor thermal_sys sp5100_tco button i2c_piix4 ext4 crc16 jbd2 mbcache sg sd_mod crc_t10dif ahci libahci igb i2c_algo_bit i2c_core dca ptp pps_core ehci_pci ohci_hcd ehci_hcd libata usbcore usb_common scsi_mod [last unloaded: leds_pca963x]
[  555.346940] Pid: 4766, comm: insmod Tainted: P        W  O 3.9-1-amd64 #1 Debian 3.9.8-1
[  555.346943] Call Trace:
[  555.346956]  [<ffffffff8103d153>] ? warn_slowpath_common+0x76/0x8c
[  555.346962]  [<ffffffff8103d202>] ? warn_slowpath_fmt+0x47/0x49
[  555.346968]  [<ffffffff8116005d>] ? sysfs_pathname+0x3b/0x41
[  555.346973]  [<ffffffff81160767>] ? sysfs_add_one+0x8b/0x9d
[  555.346978]  [<ffffffff811610a4>] ? sysfs_do_create_link_sd+0xe8/0x174
[  555.346985]  [<ffffffff81279250>] ? device_add+0x243/0x5ab
[  555.346991]  [<ffffffff81060a16>] ? complete_all+0x31/0x40
[  555.346998]  [<ffffffff8104991a>] ? init_timer_key+0xc/0x56
[  555.347004]  [<ffffffff8127964c>] ? device_create_vargs+0x82/0xb6
[  555.347009]  [<ffffffff812796af>] ? device_create+0x2f/0x31
[  555.347014]  [<ffffffff81060add>] ? should_resched+0x5/0x23
[  555.347021]  [<ffffffff812a3a92>] ? led_classdev_register+0x24/0x103
[  555.347028]  [<ffffffffa09d01c0>] ? pca9633_probe+0x173/0x239 [leds_pca9633]
[  555.347035]  [<ffffffff8127b504>] ? __driver_attach+0x73/0x73
[  555.347049]  [<ffffffffa009dfc9>] ? i2c_device_probe+0x63/0x88 [i2c_core]
[  555.347057]  [<ffffffff8127b373>] ? driver_probe_device+0x92/0x1b0
[  555.347064]  [<ffffffff81279c5c>] ? bus_for_each_drv+0x43/0x7d
[  555.347070]  [<ffffffff8127b2af>] ? device_attach+0x68/0x83
[  555.347078]  [<ffffffff8127a990>] ? bus_probe_device+0x25/0x8d
[  555.347083]  [<ffffffff812793f7>] ? device_add+0x3ea/0x5ab
[  555.347088]  [<ffffffff81060a16>] ? complete_all+0x31/0x40
[  555.347094]  [<ffffffff8104991a>] ? init_timer_key+0xc/0x56
[  555.347104]  [<ffffffffa009d3a1>] ? i2c_new_device+0x10d/0x179 [i2c_core]
[  555.347112]  [<ffffffffa008f036>] ? qt5038_init+0x36/0x1000 [qt5038_platform]
[  555.347119]  [<ffffffffa008f000>] ? 0xffffffffa008efff
[  555.347125]  [<ffffffff8100209e>] ? do_one_initcall+0x74/0x128
[  555.347131]  [<ffffffffa008f000>] ? 0xffffffffa008efff
[  555.347137]  [<ffffffff810836f5>] ? load_module+0x1af7/0x1dfc
[  555.347144]  [<ffffffff810801c5>] ? free_notes_attrs+0x3c/0x3c
[  555.347150]  [<ffffffff81083a98>] ? sys_init_module+0x9e/0xab
[  555.347157]  [<ffffffff8138be29>] ? system_call_fastpath+0x16/0x1b
[  555.347161] ---[ end trace ad00b85794e0de4d ]---
[  555.347448] leds-pca9633: probe of 0-006b failed with error -17

Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
---
 drivers/leds/leds-pca9633.c |    8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/leds/leds-pca9633.c b/drivers/leds/leds-pca9633.c
index e59ca0a..7dd48f8 100644
--- a/drivers/leds/leds-pca9633.c
+++ b/drivers/leds/leds-pca9633.c
@@ -362,10 +362,12 @@ static int pca9633_probe(struct i2c_client *client,
 			if (pdata->leds.leds[i].default_trigger)
 				pca9633[i].led_cdev.default_trigger =
 					pdata->leds.leds[i].default_trigger;
-		} else {
-			snprintf(pca9633[i].name, sizeof(pca9633[i].name),
-				 "pca9633:%d", i);
 		}
+		if (!pdata || i >= pdata->leds.num_leds ||
+						!pdata->leds.leds[i].name)
+			snprintf(pca9633[i].name, sizeof(pca9633[i].name),
+				 "pca9633:%d:%.2x:%d", client->adapter->nr,
+				 client->addr, i);
 
 		pca9633[i].led_cdev.name = pca9633[i].name;
 		pca9633[i].led_cdev.brightness_set = pca9633_led_set;
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v5 3/5] leds-pca9633: Add mutex to the ledout register
  2013-08-14 21:23 [PATCH v4 0/5] PCA9633: Add support to PCA9634 and fix some problems Ricardo Ribalda Delgado
  2013-08-14 21:23 ` [PATCH v5 1/5] leds-pca9633: Add support for PCA9634 Ricardo Ribalda Delgado
  2013-08-14 21:23 ` [PATCH v5 2/5] leds-pca9633: Unique naming of the LEDs Ricardo Ribalda Delgado
@ 2013-08-14 21:23 ` Ricardo Ribalda Delgado
  2013-08-14 21:23 ` [PATCH v5 4/5] leds-pca9633: Rename to leds-pca963x Ricardo Ribalda Delgado
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Ricardo Ribalda Delgado @ 2013-08-14 21:23 UTC (permalink / raw)
  To: Bryan Wu, Peter Meerwald, Richard Purdie, Linux LED Subsystem, LKML
  Cc: Ricardo Ribalda Delgado

To update an LED a register has to be read, updated and writen. If
another LED whas been updated at the same time, this could lead into
wrong updates.

This patch adds a common mutex to all the leds of the same chip to
protect the ledout register.

Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
---
 drivers/leds/leds-pca9633.c |   86 +++++++++++++++++++++++++++----------------
 1 file changed, 55 insertions(+), 31 deletions(-)

diff --git a/drivers/leds/leds-pca9633.c b/drivers/leds/leds-pca9633.c
index 7dd48f8..aaa1c4a 100644
--- a/drivers/leds/leds-pca9633.c
+++ b/drivers/leds/leds-pca9633.c
@@ -93,9 +93,17 @@ enum pca9633_cmd {
 	BLINK_SET,
 };
 
-struct pca9633_led {
-	struct i2c_client *client;
+struct pca9633_led;
+
+struct pca9633 {
 	struct pca9633_chipdef *chipdef;
+	struct mutex mutex;
+	struct i2c_client *client;
+	struct pca9633_led *leds;
+};
+
+struct pca9633_led {
+	struct pca9633 *chip;
 	struct work_struct work;
 	enum led_brightness brightness;
 	struct led_classdev led_cdev;
@@ -108,52 +116,60 @@ struct pca9633_led {
 
 static void pca9633_brightness_work(struct pca9633_led *pca9633)
 {
-	u8 ledout_addr = pca9633->chipdef->ledout_base + (pca9633->led_num / 4);
+	u8 ledout_addr = pca9633->chip->chipdef->ledout_base
+		+ (pca9633->led_num / 4);
 	u8 ledout;
 	int shift = 2 * (pca9633->led_num % 4);
 	u8 mask = 0x3 << shift;
 
-	ledout = i2c_smbus_read_byte_data(pca9633->client, ledout_addr);
+	mutex_lock(&pca9633->chip->mutex);
+	ledout = i2c_smbus_read_byte_data(pca9633->chip->client, ledout_addr);
 	switch (pca9633->brightness) {
 	case LED_FULL:
-		i2c_smbus_write_byte_data(pca9633->client, ledout_addr,
+		i2c_smbus_write_byte_data(pca9633->chip->client, ledout_addr,
 			(ledout & ~mask) | (PCA9633_LED_ON << shift));
 		break;
 	case LED_OFF:
-		i2c_smbus_write_byte_data(pca9633->client, ledout_addr,
+		i2c_smbus_write_byte_data(pca9633->chip->client, ledout_addr,
 			ledout & ~mask);
 		break;
 	default:
-		i2c_smbus_write_byte_data(pca9633->client,
+		i2c_smbus_write_byte_data(pca9633->chip->client,
 			PCA9633_PWM_BASE + pca9633->led_num,
 			pca9633->brightness);
-		i2c_smbus_write_byte_data(pca9633->client, ledout_addr,
+		i2c_smbus_write_byte_data(pca9633->chip->client, ledout_addr,
 			(ledout & ~mask) | (PCA9633_LED_PWM << shift));
 		break;
 	}
+	mutex_unlock(&pca9633->chip->mutex);
 }
 
 static void pca9633_blink_work(struct pca9633_led *pca9633)
 {
-	u8 ledout_addr = pca9633->chipdef->ledout_base + (pca9633->led_num / 4);
-	u8 ledout = i2c_smbus_read_byte_data(pca9633->client, ledout_addr);
-	u8 mode2 = i2c_smbus_read_byte_data(pca9633->client, PCA9633_MODE2);
+	u8 ledout_addr = pca9633->chip->chipdef->ledout_base +
+		(pca9633->led_num / 4);
+	u8 ledout;
+	u8 mode2 = i2c_smbus_read_byte_data(pca9633->chip->client,
+							PCA9633_MODE2);
 	int shift = 2 * (pca9633->led_num % 4);
 	u8 mask = 0x3 << shift;
 
-	i2c_smbus_write_byte_data(pca9633->client, pca9633->chipdef->grppwm,
-		pca9633->gdc);
+	i2c_smbus_write_byte_data(pca9633->chip->client,
+			pca9633->chip->chipdef->grppwm,	pca9633->gdc);
 
-	i2c_smbus_write_byte_data(pca9633->client, pca9633->chipdef->grpfreq,
-		pca9633->gfrq);
+	i2c_smbus_write_byte_data(pca9633->chip->client,
+			pca9633->chip->chipdef->grpfreq, pca9633->gfrq);
 
 	if (!(mode2 & PCA9633_MODE2_DMBLNK))
-		i2c_smbus_write_byte_data(pca9633->client, PCA9633_MODE2,
+		i2c_smbus_write_byte_data(pca9633->chip->client, PCA9633_MODE2,
 			mode2 | PCA9633_MODE2_DMBLNK);
 
+	mutex_lock(&pca9633->chip->mutex);
+	ledout = i2c_smbus_read_byte_data(pca9633->chip->client, ledout_addr);
 	if ((ledout & mask) != (PCA9633_LED_GRP_PWM << shift))
-		i2c_smbus_write_byte_data(pca9633->client, ledout_addr,
+		i2c_smbus_write_byte_data(pca9633->chip->client, ledout_addr,
 			(ledout & ~mask) | (PCA9633_LED_GRP_PWM << shift));
+	mutex_unlock(&pca9633->chip->mutex);
 }
 
 static void pca9633_work(struct work_struct *work)
@@ -318,6 +334,7 @@ pca9633_dt_init(struct i2c_client *client, struct pca9633_chipdef *chip)
 static int pca9633_probe(struct i2c_client *client,
 					const struct i2c_device_id *id)
 {
+	struct pca9633 *pca9633_chip;
 	struct pca9633_led *pca9633;
 	struct pca9633_platform_data *pdata;
 	struct pca9633_chipdef *chip;
@@ -341,17 +358,30 @@ static int pca9633_probe(struct i2c_client *client,
 		return -EINVAL;
 	}
 
+	pca9633_chip = devm_kzalloc(&client->dev, sizeof(*pca9633_chip),
+								GFP_KERNEL);
+	if (!pca9633_chip)
+		return -ENOMEM;
 	pca9633 = devm_kzalloc(&client->dev, chip->n_leds * sizeof(*pca9633),
 								GFP_KERNEL);
 	if (!pca9633)
 		return -ENOMEM;
 
-	i2c_set_clientdata(client, pca9633);
+	i2c_set_clientdata(client, pca9633_chip);
+
+	mutex_init(&pca9633_chip->mutex);
+	pca9633_chip->chipdef = chip;
+	pca9633_chip->client = client;
+	pca9633_chip->leds = pca9633;
+
+	/* Turn off LEDs by default*/
+	i2c_smbus_write_byte_data(client, chip->ledout_base, 0x00);
+	if (chip->n_leds > 4)
+		i2c_smbus_write_byte_data(client, chip->ledout_base + 1, 0x00);
 
 	for (i = 0; i < chip->n_leds; i++) {
-		pca9633[i].client = client;
 		pca9633[i].led_num = i;
-		pca9633[i].chipdef = chip;
+		pca9633[i].chip = pca9633_chip;
 
 		/* Platform data can specify LED names and default triggers */
 		if (pdata && i < pdata->leds.num_leds) {
@@ -389,11 +419,6 @@ static int pca9633_probe(struct i2c_client *client,
 	if (pdata && pdata->outdrv == PCA9633_OPEN_DRAIN)
 		i2c_smbus_write_byte_data(client, PCA9633_MODE2, 0x01);
 
-	/* Turn off LEDs */
-	i2c_smbus_write_byte_data(client, chip->ledout_base, 0x00);
-	if (chip->n_leds > 4)
-		i2c_smbus_write_byte_data(client, chip->ledout_base + 1, 0x00);
-
 	return 0;
 
 exit:
@@ -407,14 +432,13 @@ exit:
 
 static int pca9633_remove(struct i2c_client *client)
 {
-	struct pca9633_led *pca9633 = i2c_get_clientdata(client);
+	struct pca9633 *pca9633 = i2c_get_clientdata(client);
 	int i;
 
-	for (i = 0; i < pca9633->chipdef->n_leds; i++)
-		if (pca9633[i].client != NULL) {
-			led_classdev_unregister(&pca9633[i].led_cdev);
-			cancel_work_sync(&pca9633[i].work);
-		}
+	for (i = 0; i < pca9633->chipdef->n_leds; i++) {
+		led_classdev_unregister(&pca9633->leds[i].led_cdev);
+		cancel_work_sync(&pca9633->leds[i].work);
+	}
 
 	return 0;
 }
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v5 4/5] leds-pca9633: Rename to leds-pca963x
  2013-08-14 21:23 [PATCH v4 0/5] PCA9633: Add support to PCA9634 and fix some problems Ricardo Ribalda Delgado
                   ` (2 preceding siblings ...)
  2013-08-14 21:23 ` [PATCH v5 3/5] leds-pca9633: Add mutex to the ledout register Ricardo Ribalda Delgado
@ 2013-08-14 21:23 ` Ricardo Ribalda Delgado
  2013-08-14 21:23 ` [PATCH v5 5/5] leds-pca963x: Fix device tree parsing Ricardo Ribalda Delgado
  2013-08-17  0:17 ` [PATCH v4 0/5] PCA9633: Add support to PCA9634 and fix some problems Bryan Wu
  5 siblings, 0 replies; 7+ messages in thread
From: Ricardo Ribalda Delgado @ 2013-08-14 21:23 UTC (permalink / raw)
  To: Bryan Wu, Peter Meerwald, Richard Purdie, Linux LED Subsystem, LKML
  Cc: Ricardo Ribalda Delgado

The driver now supports the chips pca9633 and pca9634, therefore we
rename the files to more generic and meaningul names

Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
---
 Documentation/devicetree/bindings/leds/pca9633.txt |   47 --
 Documentation/devicetree/bindings/leds/pca963x.txt |   47 ++
 drivers/leds/Kconfig                               |    2 +-
 drivers/leds/Makefile                              |    2 +-
 drivers/leds/leds-pca9633.c                        |  461 --------------------
 drivers/leds/leds-pca963x.c                        |  461 ++++++++++++++++++++
 include/linux/platform_data/leds-pca9633.h         |   41 --
 include/linux/platform_data/leds-pca963x.h         |   42 ++
 8 files changed, 552 insertions(+), 551 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/leds/pca9633.txt
 create mode 100644 Documentation/devicetree/bindings/leds/pca963x.txt
 delete mode 100644 drivers/leds/leds-pca9633.c
 create mode 100644 drivers/leds/leds-pca963x.c
 delete mode 100644 include/linux/platform_data/leds-pca9633.h
 create mode 100644 include/linux/platform_data/leds-pca963x.h

diff --git a/Documentation/devicetree/bindings/leds/pca9633.txt b/Documentation/devicetree/bindings/leds/pca9633.txt
deleted file mode 100644
index aece3ea..0000000
--- a/Documentation/devicetree/bindings/leds/pca9633.txt
+++ /dev/null
@@ -1,47 +0,0 @@
-LEDs connected to pca9632, pca9633 or pca9634
-
-Required properties:
-- compatible : should be : "nxp,pca9632", "nxp,pca9633" or "nxp,pca9634"
-
-Optional properties:
-- nxp,totem-pole : use totem pole (push-pull) instead of default open-drain
-- nxp,hw-blink : use hardware blinking instead of software blinking
-
-Each led is represented as a sub-node of the nxp,pca963x device.
-
-LED sub-node properties:
-- label : (optional) see Documentation/devicetree/bindings/leds/common.txt
-- reg : number of LED line (could be from 0 to 3  in pca9632 or pca9633
-		or 0 to 7 in pca9634)
-- linux,default-trigger : (optional)
-   see Documentation/devicetree/bindings/leds/common.txt
-
-Examples:
-
-pca9632: pca9632 {
-	compatible = "nxp,pca9632";
-	#address-cells = <1>;
-	#size-cells = <0>;
-	reg = <0x62>;
-
-	red@0 {
-		label = "red";
-		reg = <0>;
-		linux,default-trigger = "none";
-	};
-	green@1 {
-		label = "green";
-		reg = <1>;
-		linux,default-trigger = "none";
-	};
-	blue@2 {
-		label = "blue";
-		reg = <2>;
-		linux,default-trigger = "none";
-	};
-	unused@3 {
-		label = "unused";
-		reg = <3>;
-		linux,default-trigger = "none";
-	};
-};
diff --git a/Documentation/devicetree/bindings/leds/pca963x.txt b/Documentation/devicetree/bindings/leds/pca963x.txt
new file mode 100644
index 0000000..aece3ea
--- /dev/null
+++ b/Documentation/devicetree/bindings/leds/pca963x.txt
@@ -0,0 +1,47 @@
+LEDs connected to pca9632, pca9633 or pca9634
+
+Required properties:
+- compatible : should be : "nxp,pca9632", "nxp,pca9633" or "nxp,pca9634"
+
+Optional properties:
+- nxp,totem-pole : use totem pole (push-pull) instead of default open-drain
+- nxp,hw-blink : use hardware blinking instead of software blinking
+
+Each led is represented as a sub-node of the nxp,pca963x device.
+
+LED sub-node properties:
+- label : (optional) see Documentation/devicetree/bindings/leds/common.txt
+- reg : number of LED line (could be from 0 to 3  in pca9632 or pca9633
+		or 0 to 7 in pca9634)
+- linux,default-trigger : (optional)
+   see Documentation/devicetree/bindings/leds/common.txt
+
+Examples:
+
+pca9632: pca9632 {
+	compatible = "nxp,pca9632";
+	#address-cells = <1>;
+	#size-cells = <0>;
+	reg = <0x62>;
+
+	red@0 {
+		label = "red";
+		reg = <0>;
+		linux,default-trigger = "none";
+	};
+	green@1 {
+		label = "green";
+		reg = <1>;
+		linux,default-trigger = "none";
+	};
+	blue@2 {
+		label = "blue";
+		reg = <2>;
+		linux,default-trigger = "none";
+	};
+	unused@3 {
+		label = "unused";
+		reg = <3>;
+		linux,default-trigger = "none";
+	};
+};
diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index e7977aa..a1a52be 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -291,7 +291,7 @@ config LEDS_PCA955X
 	  LED driver chips accessed via the I2C bus.  Supported
 	  devices include PCA9550, PCA9551, PCA9552, and PCA9553.
 
-config LEDS_PCA9633
+config LEDS_PCA963X
 	tristate "LED support for PCA963x I2C chip"
 	depends on LEDS_CLASS
 	depends on I2C
diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
index 3013113..c7e3542 100644
--- a/drivers/leds/Makefile
+++ b/drivers/leds/Makefile
@@ -35,7 +35,7 @@ obj-$(CONFIG_LEDS_HP6XX)		+= leds-hp6xx.o
 obj-$(CONFIG_LEDS_OT200)		+= leds-ot200.o
 obj-$(CONFIG_LEDS_FSG)			+= leds-fsg.o
 obj-$(CONFIG_LEDS_PCA955X)		+= leds-pca955x.o
-obj-$(CONFIG_LEDS_PCA9633)		+= leds-pca9633.o
+obj-$(CONFIG_LEDS_PCA963X)		+= leds-pca963x.o
 obj-$(CONFIG_LEDS_DA903X)		+= leds-da903x.o
 obj-$(CONFIG_LEDS_DA9052)		+= leds-da9052.o
 obj-$(CONFIG_LEDS_WM831X_STATUS)	+= leds-wm831x-status.o
diff --git a/drivers/leds/leds-pca9633.c b/drivers/leds/leds-pca9633.c
deleted file mode 100644
index aaa1c4a..0000000
--- a/drivers/leds/leds-pca9633.c
+++ /dev/null
@@ -1,461 +0,0 @@
-/*
- * Copyright 2011 bct electronic GmbH
- * Copyright 2013 Qtechnology/AS
- *
- * Author: Peter Meerwald <p.meerwald@bct-electronic.com>
- * Author: Ricardo Ribalda <ricardo.ribalda@gmail.com>
- *
- * Based on leds-pca955x.c
- *
- * This file is subject to the terms and conditions of version 2 of
- * the GNU General Public License.  See the file COPYING in the main
- * directory of this archive for more details.
- *
- * LED driver for the PCA9633 I2C LED driver (7-bit slave address 0x62)
- * LED driver for the PCA9634 I2C LED driver (7-bit slave address set by hw.)
- *
- * Note that hardware blinking violates the leds infrastructure driver
- * interface since the hardware only supports blinking all LEDs with the
- * same delay_on/delay_off rates.  That is, only the LEDs that are set to
- * blink will actually blink but all LEDs that are set to blink will blink
- * in identical fashion.  The delay_on/delay_off values of the last LED
- * that is set to blink will be used for all of the blinking LEDs.
- * Hardware blinking is disabled by default but can be enabled by setting
- * the 'blink_type' member in the platform_data struct to 'PCA9633_HW_BLINK'
- * or by adding the 'nxp,hw-blink' property to the DTS.
- */
-
-#include <linux/module.h>
-#include <linux/delay.h>
-#include <linux/string.h>
-#include <linux/ctype.h>
-#include <linux/leds.h>
-#include <linux/err.h>
-#include <linux/i2c.h>
-#include <linux/workqueue.h>
-#include <linux/slab.h>
-#include <linux/of.h>
-#include <linux/platform_data/leds-pca9633.h>
-
-/* LED select registers determine the source that drives LED outputs */
-#define PCA9633_LED_OFF		0x0	/* LED driver off */
-#define PCA9633_LED_ON		0x1	/* LED driver on */
-#define PCA9633_LED_PWM		0x2	/* Controlled through PWM */
-#define PCA9633_LED_GRP_PWM	0x3	/* Controlled through PWM/GRPPWM */
-
-#define PCA9633_MODE2_DMBLNK	0x20	/* Enable blinking */
-
-#define PCA9633_MODE1		0x00
-#define PCA9633_MODE2		0x01
-#define PCA9633_PWM_BASE	0x02
-
-enum pca9633_type {
-	pca9633,
-	pca9634,
-};
-
-struct pca9633_chipdef {
-	u8			grppwm;
-	u8			grpfreq;
-	u8			ledout_base;
-	int			n_leds;
-};
-
-static struct pca9633_chipdef pca9633_chipdefs[] = {
-	[pca9633] = {
-		.grppwm		= 0x6,
-		.grpfreq	= 0x7,
-		.ledout_base	= 0x8,
-		.n_leds		= 4,
-	},
-	[pca9634] = {
-		.grppwm		= 0xa,
-		.grpfreq	= 0xb,
-		.ledout_base	= 0xc,
-		.n_leds		= 8,
-	},
-};
-
-/* Total blink period in milliseconds */
-#define PCA9632_BLINK_PERIOD_MIN	42
-#define PCA9632_BLINK_PERIOD_MAX	10667
-
-static const struct i2c_device_id pca9633_id[] = {
-	{ "pca9632", pca9633 },
-	{ "pca9633", pca9633 },
-	{ "pca9634", pca9634 },
-	{ }
-};
-MODULE_DEVICE_TABLE(i2c, pca9633_id);
-
-enum pca9633_cmd {
-	BRIGHTNESS_SET,
-	BLINK_SET,
-};
-
-struct pca9633_led;
-
-struct pca9633 {
-	struct pca9633_chipdef *chipdef;
-	struct mutex mutex;
-	struct i2c_client *client;
-	struct pca9633_led *leds;
-};
-
-struct pca9633_led {
-	struct pca9633 *chip;
-	struct work_struct work;
-	enum led_brightness brightness;
-	struct led_classdev led_cdev;
-	int led_num; /* 0 .. 7 potentially */
-	enum pca9633_cmd cmd;
-	char name[32];
-	u8 gdc;
-	u8 gfrq;
-};
-
-static void pca9633_brightness_work(struct pca9633_led *pca9633)
-{
-	u8 ledout_addr = pca9633->chip->chipdef->ledout_base
-		+ (pca9633->led_num / 4);
-	u8 ledout;
-	int shift = 2 * (pca9633->led_num % 4);
-	u8 mask = 0x3 << shift;
-
-	mutex_lock(&pca9633->chip->mutex);
-	ledout = i2c_smbus_read_byte_data(pca9633->chip->client, ledout_addr);
-	switch (pca9633->brightness) {
-	case LED_FULL:
-		i2c_smbus_write_byte_data(pca9633->chip->client, ledout_addr,
-			(ledout & ~mask) | (PCA9633_LED_ON << shift));
-		break;
-	case LED_OFF:
-		i2c_smbus_write_byte_data(pca9633->chip->client, ledout_addr,
-			ledout & ~mask);
-		break;
-	default:
-		i2c_smbus_write_byte_data(pca9633->chip->client,
-			PCA9633_PWM_BASE + pca9633->led_num,
-			pca9633->brightness);
-		i2c_smbus_write_byte_data(pca9633->chip->client, ledout_addr,
-			(ledout & ~mask) | (PCA9633_LED_PWM << shift));
-		break;
-	}
-	mutex_unlock(&pca9633->chip->mutex);
-}
-
-static void pca9633_blink_work(struct pca9633_led *pca9633)
-{
-	u8 ledout_addr = pca9633->chip->chipdef->ledout_base +
-		(pca9633->led_num / 4);
-	u8 ledout;
-	u8 mode2 = i2c_smbus_read_byte_data(pca9633->chip->client,
-							PCA9633_MODE2);
-	int shift = 2 * (pca9633->led_num % 4);
-	u8 mask = 0x3 << shift;
-
-	i2c_smbus_write_byte_data(pca9633->chip->client,
-			pca9633->chip->chipdef->grppwm,	pca9633->gdc);
-
-	i2c_smbus_write_byte_data(pca9633->chip->client,
-			pca9633->chip->chipdef->grpfreq, pca9633->gfrq);
-
-	if (!(mode2 & PCA9633_MODE2_DMBLNK))
-		i2c_smbus_write_byte_data(pca9633->chip->client, PCA9633_MODE2,
-			mode2 | PCA9633_MODE2_DMBLNK);
-
-	mutex_lock(&pca9633->chip->mutex);
-	ledout = i2c_smbus_read_byte_data(pca9633->chip->client, ledout_addr);
-	if ((ledout & mask) != (PCA9633_LED_GRP_PWM << shift))
-		i2c_smbus_write_byte_data(pca9633->chip->client, ledout_addr,
-			(ledout & ~mask) | (PCA9633_LED_GRP_PWM << shift));
-	mutex_unlock(&pca9633->chip->mutex);
-}
-
-static void pca9633_work(struct work_struct *work)
-{
-	struct pca9633_led *pca9633 = container_of(work,
-		struct pca9633_led, work);
-
-	switch (pca9633->cmd) {
-	case BRIGHTNESS_SET:
-		pca9633_brightness_work(pca9633);
-		break;
-	case BLINK_SET:
-		pca9633_blink_work(pca9633);
-		break;
-	}
-}
-
-static void pca9633_led_set(struct led_classdev *led_cdev,
-	enum led_brightness value)
-{
-	struct pca9633_led *pca9633;
-
-	pca9633 = container_of(led_cdev, struct pca9633_led, led_cdev);
-
-	pca9633->cmd = BRIGHTNESS_SET;
-	pca9633->brightness = value;
-
-	/*
-	 * Must use workqueue for the actual I/O since I2C operations
-	 * can sleep.
-	 */
-	schedule_work(&pca9633->work);
-}
-
-static int pca9633_blink_set(struct led_classdev *led_cdev,
-		unsigned long *delay_on, unsigned long *delay_off)
-{
-	struct pca9633_led *pca9633;
-	unsigned long time_on, time_off, period;
-	u8 gdc, gfrq;
-
-	pca9633 = container_of(led_cdev, struct pca9633_led, led_cdev);
-
-	time_on = *delay_on;
-	time_off = *delay_off;
-
-	/* If both zero, pick reasonable defaults of 500ms each */
-	if (!time_on && !time_off) {
-		time_on = 500;
-		time_off = 500;
-	}
-
-	period = time_on + time_off;
-
-	/* If period not supported by hardware, default to someting sane. */
-	if ((period < PCA9632_BLINK_PERIOD_MIN) ||
-	    (period > PCA9632_BLINK_PERIOD_MAX)) {
-		time_on = 500;
-		time_off = 500;
-		period = time_on + time_off;
-	}
-
-	/*
-	 * From manual: duty cycle = (GDC / 256) ->
-	 *	(time_on / period) = (GDC / 256) ->
-	 *		GDC = ((time_on * 256) / period)
-	 */
-	gdc = (time_on * 256) / period;
-
-	/*
-	 * From manual: period = ((GFRQ + 1) / 24) in seconds.
-	 * So, period (in ms) = (((GFRQ + 1) / 24) * 1000) ->
-	 *		GFRQ = ((period * 24 / 1000) - 1)
-	 */
-	gfrq = (period * 24 / 1000) - 1;
-
-	pca9633->cmd = BLINK_SET;
-	pca9633->gdc = gdc;
-	pca9633->gfrq = gfrq;
-
-	/*
-	 * Must use workqueue for the actual I/O since I2C operations
-	 * can sleep.
-	 */
-	schedule_work(&pca9633->work);
-
-	*delay_on = time_on;
-	*delay_off = time_off;
-
-	return 0;
-}
-
-#if IS_ENABLED(CONFIG_OF)
-static struct pca9633_platform_data *
-pca9633_dt_init(struct i2c_client *client, struct pca9633_chipdef *chip)
-{
-	struct device_node *np = client->dev.of_node, *child;
-	struct pca9633_platform_data *pdata;
-	struct led_info *pca9633_leds;
-	int count;
-
-	count = of_get_child_count(np);
-	if (!count || count > chip->n_leds)
-		return ERR_PTR(-ENODEV);
-
-	pca9633_leds = devm_kzalloc(&client->dev,
-			sizeof(struct led_info) * chip->n_leds, GFP_KERNEL);
-	if (!pca9633_leds)
-		return ERR_PTR(-ENOMEM);
-
-	for_each_child_of_node(np, child) {
-		struct led_info led;
-		u32 reg;
-		int res;
-
-		led.name =
-			of_get_property(child, "label", NULL) ? : child->name;
-		led.default_trigger =
-			of_get_property(child, "linux,default-trigger", NULL);
-		res = of_property_read_u32(child, "reg", &reg);
-		if (res != 0)
-			continue;
-		pca9633_leds[reg] = led;
-	}
-	pdata = devm_kzalloc(&client->dev,
-			     sizeof(struct pca9633_platform_data), GFP_KERNEL);
-	if (!pdata)
-		return ERR_PTR(-ENOMEM);
-
-	pdata->leds.leds = pca9633_leds;
-	pdata->leds.num_leds = count;
-
-	/* default to open-drain unless totem pole (push-pull) is specified */
-	if (of_property_read_bool(np, "nxp,totem-pole"))
-		pdata->outdrv = PCA9633_TOTEM_POLE;
-	else
-		pdata->outdrv = PCA9633_OPEN_DRAIN;
-
-	/* default to software blinking unless hardware blinking is specified */
-	if (of_property_read_bool(np, "nxp,hw-blink"))
-		pdata->blink_type = PCA9633_HW_BLINK;
-	else
-		pdata->blink_type = PCA9633_SW_BLINK;
-
-	return pdata;
-}
-
-static const struct of_device_id of_pca9633_match[] = {
-	{ .compatible = "nxp,pca9632", },
-	{ .compatible = "nxp,pca9633", },
-	{ .compatible = "nxp,pca9634", },
-	{},
-};
-#else
-static struct pca9633_platform_data *
-pca9633_dt_init(struct i2c_client *client, struct pca9633_chipdef *chip)
-{
-	return ERR_PTR(-ENODEV);
-}
-#endif
-
-static int pca9633_probe(struct i2c_client *client,
-					const struct i2c_device_id *id)
-{
-	struct pca9633 *pca9633_chip;
-	struct pca9633_led *pca9633;
-	struct pca9633_platform_data *pdata;
-	struct pca9633_chipdef *chip;
-	int i, err;
-
-	chip = &pca9633_chipdefs[id->driver_data];
-	pdata = dev_get_platdata(&client->dev);
-
-	if (!pdata) {
-		pdata = pca9633_dt_init(client, chip);
-		if (IS_ERR(pdata)) {
-			dev_warn(&client->dev, "could not parse configuration\n");
-			pdata = NULL;
-		}
-	}
-
-	if (pdata && (pdata->leds.num_leds < 1 ||
-				 pdata->leds.num_leds > chip->n_leds)) {
-		dev_err(&client->dev, "board info must claim 1-%d LEDs",
-								chip->n_leds);
-		return -EINVAL;
-	}
-
-	pca9633_chip = devm_kzalloc(&client->dev, sizeof(*pca9633_chip),
-								GFP_KERNEL);
-	if (!pca9633_chip)
-		return -ENOMEM;
-	pca9633 = devm_kzalloc(&client->dev, chip->n_leds * sizeof(*pca9633),
-								GFP_KERNEL);
-	if (!pca9633)
-		return -ENOMEM;
-
-	i2c_set_clientdata(client, pca9633_chip);
-
-	mutex_init(&pca9633_chip->mutex);
-	pca9633_chip->chipdef = chip;
-	pca9633_chip->client = client;
-	pca9633_chip->leds = pca9633;
-
-	/* Turn off LEDs by default*/
-	i2c_smbus_write_byte_data(client, chip->ledout_base, 0x00);
-	if (chip->n_leds > 4)
-		i2c_smbus_write_byte_data(client, chip->ledout_base + 1, 0x00);
-
-	for (i = 0; i < chip->n_leds; i++) {
-		pca9633[i].led_num = i;
-		pca9633[i].chip = pca9633_chip;
-
-		/* Platform data can specify LED names and default triggers */
-		if (pdata && i < pdata->leds.num_leds) {
-			if (pdata->leds.leds[i].name)
-				snprintf(pca9633[i].name,
-					 sizeof(pca9633[i].name), "pca9633:%s",
-					 pdata->leds.leds[i].name);
-			if (pdata->leds.leds[i].default_trigger)
-				pca9633[i].led_cdev.default_trigger =
-					pdata->leds.leds[i].default_trigger;
-		}
-		if (!pdata || i >= pdata->leds.num_leds ||
-						!pdata->leds.leds[i].name)
-			snprintf(pca9633[i].name, sizeof(pca9633[i].name),
-				 "pca9633:%d:%.2x:%d", client->adapter->nr,
-				 client->addr, i);
-
-		pca9633[i].led_cdev.name = pca9633[i].name;
-		pca9633[i].led_cdev.brightness_set = pca9633_led_set;
-
-		if (pdata && pdata->blink_type == PCA9633_HW_BLINK)
-			pca9633[i].led_cdev.blink_set = pca9633_blink_set;
-
-		INIT_WORK(&pca9633[i].work, pca9633_work);
-
-		err = led_classdev_register(&client->dev, &pca9633[i].led_cdev);
-		if (err < 0)
-			goto exit;
-	}
-
-	/* Disable LED all-call address and set normal mode */
-	i2c_smbus_write_byte_data(client, PCA9633_MODE1, 0x00);
-
-	/* Configure output: open-drain or totem pole (push-pull) */
-	if (pdata && pdata->outdrv == PCA9633_OPEN_DRAIN)
-		i2c_smbus_write_byte_data(client, PCA9633_MODE2, 0x01);
-
-	return 0;
-
-exit:
-	while (i--) {
-		led_classdev_unregister(&pca9633[i].led_cdev);
-		cancel_work_sync(&pca9633[i].work);
-	}
-
-	return err;
-}
-
-static int pca9633_remove(struct i2c_client *client)
-{
-	struct pca9633 *pca9633 = i2c_get_clientdata(client);
-	int i;
-
-	for (i = 0; i < pca9633->chipdef->n_leds; i++) {
-		led_classdev_unregister(&pca9633->leds[i].led_cdev);
-		cancel_work_sync(&pca9633->leds[i].work);
-	}
-
-	return 0;
-}
-
-static struct i2c_driver pca9633_driver = {
-	.driver = {
-		.name	= "leds-pca9633",
-		.owner	= THIS_MODULE,
-		.of_match_table = of_match_ptr(of_pca9633_match),
-	},
-	.probe	= pca9633_probe,
-	.remove	= pca9633_remove,
-	.id_table = pca9633_id,
-};
-
-module_i2c_driver(pca9633_driver);
-
-MODULE_AUTHOR("Peter Meerwald <p.meerwald@bct-electronic.com>");
-MODULE_DESCRIPTION("PCA9633 LED driver");
-MODULE_LICENSE("GPL v2");
diff --git a/drivers/leds/leds-pca963x.c b/drivers/leds/leds-pca963x.c
new file mode 100644
index 0000000..35d56a6
--- /dev/null
+++ b/drivers/leds/leds-pca963x.c
@@ -0,0 +1,461 @@
+/*
+ * Copyright 2011 bct electronic GmbH
+ * Copyright 2013 Qtechnology/AS
+ *
+ * Author: Peter Meerwald <p.meerwald@bct-electronic.com>
+ * Author: Ricardo Ribalda <ricardo.ribalda@gmail.com>
+ *
+ * Based on leds-pca955x.c
+ *
+ * This file is subject to the terms and conditions of version 2 of
+ * the GNU General Public License.  See the file COPYING in the main
+ * directory of this archive for more details.
+ *
+ * LED driver for the PCA9633 I2C LED driver (7-bit slave address 0x62)
+ * LED driver for the PCA9634 I2C LED driver (7-bit slave address set by hw.)
+ *
+ * Note that hardware blinking violates the leds infrastructure driver
+ * interface since the hardware only supports blinking all LEDs with the
+ * same delay_on/delay_off rates.  That is, only the LEDs that are set to
+ * blink will actually blink but all LEDs that are set to blink will blink
+ * in identical fashion.  The delay_on/delay_off values of the last LED
+ * that is set to blink will be used for all of the blinking LEDs.
+ * Hardware blinking is disabled by default but can be enabled by setting
+ * the 'blink_type' member in the platform_data struct to 'PCA963X_HW_BLINK'
+ * or by adding the 'nxp,hw-blink' property to the DTS.
+ */
+
+#include <linux/module.h>
+#include <linux/delay.h>
+#include <linux/string.h>
+#include <linux/ctype.h>
+#include <linux/leds.h>
+#include <linux/err.h>
+#include <linux/i2c.h>
+#include <linux/workqueue.h>
+#include <linux/slab.h>
+#include <linux/of.h>
+#include <linux/platform_data/leds-pca963x.h>
+
+/* LED select registers determine the source that drives LED outputs */
+#define PCA963X_LED_OFF		0x0	/* LED driver off */
+#define PCA963X_LED_ON		0x1	/* LED driver on */
+#define PCA963X_LED_PWM		0x2	/* Controlled through PWM */
+#define PCA963X_LED_GRP_PWM	0x3	/* Controlled through PWM/GRPPWM */
+
+#define PCA963X_MODE2_DMBLNK	0x20	/* Enable blinking */
+
+#define PCA963X_MODE1		0x00
+#define PCA963X_MODE2		0x01
+#define PCA963X_PWM_BASE	0x02
+
+enum pca963x_type {
+	pca9633,
+	pca9634,
+};
+
+struct pca963x_chipdef {
+	u8			grppwm;
+	u8			grpfreq;
+	u8			ledout_base;
+	int			n_leds;
+};
+
+static struct pca963x_chipdef pca963x_chipdefs[] = {
+	[pca9633] = {
+		.grppwm		= 0x6,
+		.grpfreq	= 0x7,
+		.ledout_base	= 0x8,
+		.n_leds		= 4,
+	},
+	[pca9634] = {
+		.grppwm		= 0xa,
+		.grpfreq	= 0xb,
+		.ledout_base	= 0xc,
+		.n_leds		= 8,
+	},
+};
+
+/* Total blink period in milliseconds */
+#define PCA963X_BLINK_PERIOD_MIN	42
+#define PCA963X_BLINK_PERIOD_MAX	10667
+
+static const struct i2c_device_id pca963x_id[] = {
+	{ "pca9632", pca9633 },
+	{ "pca9633", pca9633 },
+	{ "pca9634", pca9634 },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, pca963x_id);
+
+enum pca963x_cmd {
+	BRIGHTNESS_SET,
+	BLINK_SET,
+};
+
+struct pca963x_led;
+
+struct pca963x {
+	struct pca963x_chipdef *chipdef;
+	struct mutex mutex;
+	struct i2c_client *client;
+	struct pca963x_led *leds;
+};
+
+struct pca963x_led {
+	struct pca963x *chip;
+	struct work_struct work;
+	enum led_brightness brightness;
+	struct led_classdev led_cdev;
+	int led_num; /* 0 .. 7 potentially */
+	enum pca963x_cmd cmd;
+	char name[32];
+	u8 gdc;
+	u8 gfrq;
+};
+
+static void pca963x_brightness_work(struct pca963x_led *pca963x)
+{
+	u8 ledout_addr = pca963x->chip->chipdef->ledout_base
+		+ (pca963x->led_num / 4);
+	u8 ledout;
+	int shift = 2 * (pca963x->led_num % 4);
+	u8 mask = 0x3 << shift;
+
+	mutex_lock(&pca963x->chip->mutex);
+	ledout = i2c_smbus_read_byte_data(pca963x->chip->client, ledout_addr);
+	switch (pca963x->brightness) {
+	case LED_FULL:
+		i2c_smbus_write_byte_data(pca963x->chip->client, ledout_addr,
+			(ledout & ~mask) | (PCA963X_LED_ON << shift));
+		break;
+	case LED_OFF:
+		i2c_smbus_write_byte_data(pca963x->chip->client, ledout_addr,
+			ledout & ~mask);
+		break;
+	default:
+		i2c_smbus_write_byte_data(pca963x->chip->client,
+			PCA963X_PWM_BASE + pca963x->led_num,
+			pca963x->brightness);
+		i2c_smbus_write_byte_data(pca963x->chip->client, ledout_addr,
+			(ledout & ~mask) | (PCA963X_LED_PWM << shift));
+		break;
+	}
+	mutex_unlock(&pca963x->chip->mutex);
+}
+
+static void pca963x_blink_work(struct pca963x_led *pca963x)
+{
+	u8 ledout_addr = pca963x->chip->chipdef->ledout_base +
+		(pca963x->led_num / 4);
+	u8 ledout;
+	u8 mode2 = i2c_smbus_read_byte_data(pca963x->chip->client,
+							PCA963X_MODE2);
+	int shift = 2 * (pca963x->led_num % 4);
+	u8 mask = 0x3 << shift;
+
+	i2c_smbus_write_byte_data(pca963x->chip->client,
+			pca963x->chip->chipdef->grppwm,	pca963x->gdc);
+
+	i2c_smbus_write_byte_data(pca963x->chip->client,
+			pca963x->chip->chipdef->grpfreq, pca963x->gfrq);
+
+	if (!(mode2 & PCA963X_MODE2_DMBLNK))
+		i2c_smbus_write_byte_data(pca963x->chip->client, PCA963X_MODE2,
+			mode2 | PCA963X_MODE2_DMBLNK);
+
+	mutex_lock(&pca963x->chip->mutex);
+	ledout = i2c_smbus_read_byte_data(pca963x->chip->client, ledout_addr);
+	if ((ledout & mask) != (PCA963X_LED_GRP_PWM << shift))
+		i2c_smbus_write_byte_data(pca963x->chip->client, ledout_addr,
+			(ledout & ~mask) | (PCA963X_LED_GRP_PWM << shift));
+	mutex_unlock(&pca963x->chip->mutex);
+}
+
+static void pca963x_work(struct work_struct *work)
+{
+	struct pca963x_led *pca963x = container_of(work,
+		struct pca963x_led, work);
+
+	switch (pca963x->cmd) {
+	case BRIGHTNESS_SET:
+		pca963x_brightness_work(pca963x);
+		break;
+	case BLINK_SET:
+		pca963x_blink_work(pca963x);
+		break;
+	}
+}
+
+static void pca963x_led_set(struct led_classdev *led_cdev,
+	enum led_brightness value)
+{
+	struct pca963x_led *pca963x;
+
+	pca963x = container_of(led_cdev, struct pca963x_led, led_cdev);
+
+	pca963x->cmd = BRIGHTNESS_SET;
+	pca963x->brightness = value;
+
+	/*
+	 * Must use workqueue for the actual I/O since I2C operations
+	 * can sleep.
+	 */
+	schedule_work(&pca963x->work);
+}
+
+static int pca963x_blink_set(struct led_classdev *led_cdev,
+		unsigned long *delay_on, unsigned long *delay_off)
+{
+	struct pca963x_led *pca963x;
+	unsigned long time_on, time_off, period;
+	u8 gdc, gfrq;
+
+	pca963x = container_of(led_cdev, struct pca963x_led, led_cdev);
+
+	time_on = *delay_on;
+	time_off = *delay_off;
+
+	/* If both zero, pick reasonable defaults of 500ms each */
+	if (!time_on && !time_off) {
+		time_on = 500;
+		time_off = 500;
+	}
+
+	period = time_on + time_off;
+
+	/* If period not supported by hardware, default to someting sane. */
+	if ((period < PCA963X_BLINK_PERIOD_MIN) ||
+	    (period > PCA963X_BLINK_PERIOD_MAX)) {
+		time_on = 500;
+		time_off = 500;
+		period = time_on + time_off;
+	}
+
+	/*
+	 * From manual: duty cycle = (GDC / 256) ->
+	 *	(time_on / period) = (GDC / 256) ->
+	 *		GDC = ((time_on * 256) / period)
+	 */
+	gdc = (time_on * 256) / period;
+
+	/*
+	 * From manual: period = ((GFRQ + 1) / 24) in seconds.
+	 * So, period (in ms) = (((GFRQ + 1) / 24) * 1000) ->
+	 *		GFRQ = ((period * 24 / 1000) - 1)
+	 */
+	gfrq = (period * 24 / 1000) - 1;
+
+	pca963x->cmd = BLINK_SET;
+	pca963x->gdc = gdc;
+	pca963x->gfrq = gfrq;
+
+	/*
+	 * Must use workqueue for the actual I/O since I2C operations
+	 * can sleep.
+	 */
+	schedule_work(&pca963x->work);
+
+	*delay_on = time_on;
+	*delay_off = time_off;
+
+	return 0;
+}
+
+#if IS_ENABLED(CONFIG_OF)
+static struct pca963x_platform_data *
+pca963x_dt_init(struct i2c_client *client, struct pca963x_chipdef *chip)
+{
+	struct device_node *np = client->dev.of_node, *child;
+	struct pca963x_platform_data *pdata;
+	struct led_info *pca963x_leds;
+	int count;
+
+	count = of_get_child_count(np);
+	if (!count || count > chip->n_leds)
+		return ERR_PTR(-ENODEV);
+
+	pca963x_leds = devm_kzalloc(&client->dev,
+			sizeof(struct led_info) * chip->n_leds, GFP_KERNEL);
+	if (!pca963x_leds)
+		return ERR_PTR(-ENOMEM);
+
+	for_each_child_of_node(np, child) {
+		struct led_info led;
+		u32 reg;
+		int res;
+
+		led.name =
+			of_get_property(child, "label", NULL) ? : child->name;
+		led.default_trigger =
+			of_get_property(child, "linux,default-trigger", NULL);
+		res = of_property_read_u32(child, "reg", &reg);
+		if (res != 0)
+			continue;
+		pca963x_leds[reg] = led;
+	}
+	pdata = devm_kzalloc(&client->dev,
+			     sizeof(struct pca963x_platform_data), GFP_KERNEL);
+	if (!pdata)
+		return ERR_PTR(-ENOMEM);
+
+	pdata->leds.leds = pca963x_leds;
+	pdata->leds.num_leds = count;
+
+	/* default to open-drain unless totem pole (push-pull) is specified */
+	if (of_property_read_bool(np, "nxp,totem-pole"))
+		pdata->outdrv = PCA963X_TOTEM_POLE;
+	else
+		pdata->outdrv = PCA963X_OPEN_DRAIN;
+
+	/* default to software blinking unless hardware blinking is specified */
+	if (of_property_read_bool(np, "nxp,hw-blink"))
+		pdata->blink_type = PCA963X_HW_BLINK;
+	else
+		pdata->blink_type = PCA963X_SW_BLINK;
+
+	return pdata;
+}
+
+static const struct of_device_id of_pca963x_match[] = {
+	{ .compatible = "nxp,pca9632", },
+	{ .compatible = "nxp,pca9633", },
+	{ .compatible = "nxp,pca9634", },
+	{},
+};
+#else
+static struct pca963x_platform_data *
+pca963x_dt_init(struct i2c_client *client, struct pca963x_chipdef *chip)
+{
+	return ERR_PTR(-ENODEV);
+}
+#endif
+
+static int pca963x_probe(struct i2c_client *client,
+					const struct i2c_device_id *id)
+{
+	struct pca963x *pca963x_chip;
+	struct pca963x_led *pca963x;
+	struct pca963x_platform_data *pdata;
+	struct pca963x_chipdef *chip;
+	int i, err;
+
+	chip = &pca963x_chipdefs[id->driver_data];
+	pdata = dev_get_platdata(&client->dev);
+
+	if (!pdata) {
+		pdata = pca963x_dt_init(client, chip);
+		if (IS_ERR(pdata)) {
+			dev_warn(&client->dev, "could not parse configuration\n");
+			pdata = NULL;
+		}
+	}
+
+	if (pdata && (pdata->leds.num_leds < 1 ||
+				 pdata->leds.num_leds > chip->n_leds)) {
+		dev_err(&client->dev, "board info must claim 1-%d LEDs",
+								chip->n_leds);
+		return -EINVAL;
+	}
+
+	pca963x_chip = devm_kzalloc(&client->dev, sizeof(*pca963x_chip),
+								GFP_KERNEL);
+	if (!pca963x_chip)
+		return -ENOMEM;
+	pca963x = devm_kzalloc(&client->dev, chip->n_leds * sizeof(*pca963x),
+								GFP_KERNEL);
+	if (!pca963x)
+		return -ENOMEM;
+
+	i2c_set_clientdata(client, pca963x_chip);
+
+	mutex_init(&pca963x_chip->mutex);
+	pca963x_chip->chipdef = chip;
+	pca963x_chip->client = client;
+	pca963x_chip->leds = pca963x;
+
+	/* Turn off LEDs by default*/
+	i2c_smbus_write_byte_data(client, chip->ledout_base, 0x00);
+	if (chip->n_leds > 4)
+		i2c_smbus_write_byte_data(client, chip->ledout_base + 1, 0x00);
+
+	for (i = 0; i < chip->n_leds; i++) {
+		pca963x[i].led_num = i;
+		pca963x[i].chip = pca963x_chip;
+
+		/* Platform data can specify LED names and default triggers */
+		if (pdata && i < pdata->leds.num_leds) {
+			if (pdata->leds.leds[i].name)
+				snprintf(pca963x[i].name,
+					 sizeof(pca963x[i].name), "pca963x:%s",
+					 pdata->leds.leds[i].name);
+			if (pdata->leds.leds[i].default_trigger)
+				pca963x[i].led_cdev.default_trigger =
+					pdata->leds.leds[i].default_trigger;
+		}
+		if (!pdata || i >= pdata->leds.num_leds ||
+						!pdata->leds.leds[i].name)
+			snprintf(pca963x[i].name, sizeof(pca963x[i].name),
+				 "pca963x:%d:%.2x:%d", client->adapter->nr,
+				 client->addr, i);
+
+		pca963x[i].led_cdev.name = pca963x[i].name;
+		pca963x[i].led_cdev.brightness_set = pca963x_led_set;
+
+		if (pdata && pdata->blink_type == PCA963X_HW_BLINK)
+			pca963x[i].led_cdev.blink_set = pca963x_blink_set;
+
+		INIT_WORK(&pca963x[i].work, pca963x_work);
+
+		err = led_classdev_register(&client->dev, &pca963x[i].led_cdev);
+		if (err < 0)
+			goto exit;
+	}
+
+	/* Disable LED all-call address and set normal mode */
+	i2c_smbus_write_byte_data(client, PCA963X_MODE1, 0x00);
+
+	/* Configure output: open-drain or totem pole (push-pull) */
+	if (pdata && pdata->outdrv == PCA963X_OPEN_DRAIN)
+		i2c_smbus_write_byte_data(client, PCA963X_MODE2, 0x01);
+
+	return 0;
+
+exit:
+	while (i--) {
+		led_classdev_unregister(&pca963x[i].led_cdev);
+		cancel_work_sync(&pca963x[i].work);
+	}
+
+	return err;
+}
+
+static int pca963x_remove(struct i2c_client *client)
+{
+	struct pca963x *pca963x = i2c_get_clientdata(client);
+	int i;
+
+	for (i = 0; i < pca963x->chipdef->n_leds; i++) {
+		led_classdev_unregister(&pca963x->leds[i].led_cdev);
+		cancel_work_sync(&pca963x->leds[i].work);
+	}
+
+	return 0;
+}
+
+static struct i2c_driver pca963x_driver = {
+	.driver = {
+		.name	= "leds-pca963x",
+		.owner	= THIS_MODULE,
+		.of_match_table = of_match_ptr(of_pca963x_match),
+	},
+	.probe	= pca963x_probe,
+	.remove	= pca963x_remove,
+	.id_table = pca963x_id,
+};
+
+module_i2c_driver(pca963x_driver);
+
+MODULE_AUTHOR("Peter Meerwald <p.meerwald@bct-electronic.com>");
+MODULE_DESCRIPTION("PCA963X LED driver");
+MODULE_LICENSE("GPL v2");
diff --git a/include/linux/platform_data/leds-pca9633.h b/include/linux/platform_data/leds-pca9633.h
deleted file mode 100644
index 3c1037a..0000000
--- a/include/linux/platform_data/leds-pca9633.h
+++ /dev/null
@@ -1,41 +0,0 @@
-/*
- * PCA9633 LED chip driver.
- *
- * Copyright 2012 bct electronic GmbH
- *
- * This program is free software; you can redistribute it and/or
- * modify it under the terms of the GNU General Public License
- * version 2 as published by the Free Software Foundation.
- *
- * 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
- * General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program; if not, write to the Free Software
- * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA
- * 02110-1301 USA
- */
-
-#ifndef __LINUX_PCA9633_H
-#define __LINUX_PCA9633_H
-#include <linux/leds.h>
-
-enum pca9633_outdrv {
-	PCA9633_OPEN_DRAIN,
-	PCA9633_TOTEM_POLE, /* aka push-pull */
-};
-
-enum pca9633_blink_type {
-	PCA9633_SW_BLINK,
-	PCA9633_HW_BLINK,
-};
-
-struct pca9633_platform_data {
-	struct led_platform_data leds;
-	enum pca9633_outdrv outdrv;
-	enum pca9633_blink_type blink_type;
-};
-
-#endif /* __LINUX_PCA9633_H*/
diff --git a/include/linux/platform_data/leds-pca963x.h b/include/linux/platform_data/leds-pca963x.h
new file mode 100644
index 0000000..e731f00
--- /dev/null
+++ b/include/linux/platform_data/leds-pca963x.h
@@ -0,0 +1,42 @@
+/*
+ * PCA963X LED chip driver.
+ *
+ * Copyright 2012 bct electronic GmbH
+ * Copyright 2013 Qtechnology A/S
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * version 2 as published by the Free Software Foundation.
+ *
+ * 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
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA
+ * 02110-1301 USA
+ */
+
+#ifndef __LINUX_PCA963X_H
+#define __LINUX_PCA963X_H
+#include <linux/leds.h>
+
+enum pca963x_outdrv {
+	PCA963X_OPEN_DRAIN,
+	PCA963X_TOTEM_POLE, /* aka push-pull */
+};
+
+enum pca963x_blink_type {
+	PCA963X_SW_BLINK,
+	PCA963X_HW_BLINK,
+};
+
+struct pca963x_platform_data {
+	struct led_platform_data leds;
+	enum pca963x_outdrv outdrv;
+	enum pca963x_blink_type blink_type;
+};
+
+#endif /* __LINUX_PCA963X_H*/
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v5 5/5] leds-pca963x: Fix device tree parsing
  2013-08-14 21:23 [PATCH v4 0/5] PCA9633: Add support to PCA9634 and fix some problems Ricardo Ribalda Delgado
                   ` (3 preceding siblings ...)
  2013-08-14 21:23 ` [PATCH v5 4/5] leds-pca9633: Rename to leds-pca963x Ricardo Ribalda Delgado
@ 2013-08-14 21:23 ` Ricardo Ribalda Delgado
  2013-08-17  0:17 ` [PATCH v4 0/5] PCA9633: Add support to PCA9634 and fix some problems Bryan Wu
  5 siblings, 0 replies; 7+ messages in thread
From: Ricardo Ribalda Delgado @ 2013-08-14 21:23 UTC (permalink / raw)
  To: Bryan Wu, Peter Meerwald, Richard Purdie, Linux LED Subsystem, LKML
  Cc: Ricardo Ribalda Delgado

A malformed device tree could lead into a segmentation fault if the reg
value of a led is bigger than the number of leds.

A valid device tree could have only information about the last led of the
chip. Fix the device tree parsing to handle those cases.

Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
---
 drivers/leds/leds-pca963x.c |    8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/leds/leds-pca963x.c b/drivers/leds/leds-pca963x.c
index 35d56a6..82589c0 100644
--- a/drivers/leds/leds-pca963x.c
+++ b/drivers/leds/leds-pca963x.c
@@ -285,13 +285,13 @@ pca963x_dt_init(struct i2c_client *client, struct pca963x_chipdef *chip)
 		u32 reg;
 		int res;
 
+		res = of_property_read_u32(child, "reg", &reg);
+		if ((res != 0) || (reg >= chip->n_leds))
+			continue;
 		led.name =
 			of_get_property(child, "label", NULL) ? : child->name;
 		led.default_trigger =
 			of_get_property(child, "linux,default-trigger", NULL);
-		res = of_property_read_u32(child, "reg", &reg);
-		if (res != 0)
-			continue;
 		pca963x_leds[reg] = led;
 	}
 	pdata = devm_kzalloc(&client->dev,
@@ -300,7 +300,7 @@ pca963x_dt_init(struct i2c_client *client, struct pca963x_chipdef *chip)
 		return ERR_PTR(-ENOMEM);
 
 	pdata->leds.leds = pca963x_leds;
-	pdata->leds.num_leds = count;
+	pdata->leds.num_leds = chip->n_leds;
 
 	/* default to open-drain unless totem pole (push-pull) is specified */
 	if (of_property_read_bool(np, "nxp,totem-pole"))
-- 
1.7.10.4


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH v4 0/5] PCA9633: Add support to PCA9634 and fix some problems
  2013-08-14 21:23 [PATCH v4 0/5] PCA9633: Add support to PCA9634 and fix some problems Ricardo Ribalda Delgado
                   ` (4 preceding siblings ...)
  2013-08-14 21:23 ` [PATCH v5 5/5] leds-pca963x: Fix device tree parsing Ricardo Ribalda Delgado
@ 2013-08-17  0:17 ` Bryan Wu
  5 siblings, 0 replies; 7+ messages in thread
From: Bryan Wu @ 2013-08-17  0:17 UTC (permalink / raw)
  To: Ricardo Ribalda Delgado
  Cc: Peter Meerwald, Richard Purdie, Linux LED Subsystem, LKML

On Wed, Aug 14, 2013 at 2:23 PM, Ricardo Ribalda Delgado
<ricardo.ribalda@gmail.com> wrote:
> Add Support for the PCA9634 chip. Simimart to the 9633, but with 8 outputs instead of 4.
> Fix bug when 2 chips where present on the system, the ledclass will fail and the chip wont probe.
> Protect ledout register with a mutex to support updates of more than leds at the same time
> Fix device tree parsing
>
> v5: Contains feedback from Bryan Wu
> Bryan: Fix device tree bindings documentation
>

Thanks, I've already merged this new patchset into my tree.

-Bryan

> v4: Rebase to latest leds-next and new patch to Fix the dt parsing
>
> v3: Contains feedback from Bryan Wu
> Bryan: Rename pca9633 to pca963x
>
> v2: Contains feedback from Peter Meerwald
> Peter: Fix typo on commit message. Add bus number to name
>
> Ricardo Ribalda Delgado (5):
>   leds-pca9633: Add support for PCA9634
>   leds-pca9633: Unique naming of the LEDs
>   leds-pca9633: Add mutex to the ledout register
>   leds-pca9633: Rename to leds-pca963x
>   leds-pca963x: Fix device tree parsing
>
>  Documentation/devicetree/bindings/leds/pca9633.txt |   46 --
>  Documentation/devicetree/bindings/leds/pca963x.txt |   47 ++
>  drivers/leds/Kconfig                               |    9 +-
>  drivers/leds/Makefile                              |    2 +-
>  drivers/leds/leds-pca9633.c                        |  393 -----------------
>  drivers/leds/leds-pca963x.c                        |  461 ++++++++++++++++++++
>  include/linux/platform_data/leds-pca9633.h         |   41 --
>  include/linux/platform_data/leds-pca963x.h         |   42 ++
>  8 files changed, 556 insertions(+), 485 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/leds/pca9633.txt
>  create mode 100644 Documentation/devicetree/bindings/leds/pca963x.txt
>  delete mode 100644 drivers/leds/leds-pca9633.c
>  create mode 100644 drivers/leds/leds-pca963x.c
>  delete mode 100644 include/linux/platform_data/leds-pca9633.h
>  create mode 100644 include/linux/platform_data/leds-pca963x.h
>
> --
> 1.7.10.4
>

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2013-08-17  0:17 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-08-14 21:23 [PATCH v4 0/5] PCA9633: Add support to PCA9634 and fix some problems Ricardo Ribalda Delgado
2013-08-14 21:23 ` [PATCH v5 1/5] leds-pca9633: Add support for PCA9634 Ricardo Ribalda Delgado
2013-08-14 21:23 ` [PATCH v5 2/5] leds-pca9633: Unique naming of the LEDs Ricardo Ribalda Delgado
2013-08-14 21:23 ` [PATCH v5 3/5] leds-pca9633: Add mutex to the ledout register Ricardo Ribalda Delgado
2013-08-14 21:23 ` [PATCH v5 4/5] leds-pca9633: Rename to leds-pca963x Ricardo Ribalda Delgado
2013-08-14 21:23 ` [PATCH v5 5/5] leds-pca963x: Fix device tree parsing Ricardo Ribalda Delgado
2013-08-17  0:17 ` [PATCH v4 0/5] PCA9633: Add support to PCA9634 and fix some problems Bryan Wu

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.