* [PATCH 1/3] leds-pca9633: Add support for PCA9634
2013-08-08 8:47 [PATCH 0/3] PCA9633: Add support to PCA9634 and fix some problems Ricardo Ribalda Delgado
@ 2013-08-08 8:47 ` Ricardo Ribalda Delgado
2013-08-08 8:47 ` [PATCH 2/3] leds-pca9633: Unique naming of the LEDs Ricardo Ribalda Delgado
2013-08-08 8:47 ` [PATCH 3/3] leds-pca9633: Add mutex to the ledout register Ricardo Ribalda Delgado
2 siblings, 0 replies; 6+ messages in thread
From: Ricardo Ribalda Delgado @ 2013-08-08 8:47 UTC (permalink / raw)
To: Bryan Wu, Richard Purdie, linux-leds, linux-kernel,
Peter Meerwald
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>
---
drivers/leds/Kconfig | 7 ++--
drivers/leds/leds-pca9633.c | 74 +++++++++++++++++++++++++++++++------------
2 files changed, 58 insertions(+), 23 deletions(-)
diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index e43402d..023af58 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -280,12 +280,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 9aae567..070358a 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.)
*
*/
@@ -33,20 +36,42 @@
#define PCA9633_MODE1 0x00
#define PCA9633_MODE2 0x01
#define PCA9633_PWM_BASE 0x02
-#define PCA9633_LEDOUT 0x08
+
+enum pca9633_type {
+ pca9633,
+ pca9634,
+};
+
+struct pca9633_chipdef {
+ u8 ledout_base;
+ int n_leds;
+};
+
+static struct pca9633_chipdef pca9633_chipdefs[] = {
+ [pca9633] = {
+ .ledout_base = 0x8,
+ .n_leds = 4,
+ },
+ [pca9634] = {
+ .ledout_base = 0xc,
+ .n_leds = 8,
+ },
+};
static const struct i2c_device_id pca9633_id[] = {
- { "pca9633", 0 },
+ { "pca9633", pca9633 },
+ { "pca9634", pca9634 },
{ }
};
MODULE_DEVICE_TABLE(i2c, pca9633_id);
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 */
char name[32];
};
@@ -54,24 +79,26 @@ static void pca9633_led_work(struct work_struct *work)
{
struct pca9633_led *pca9633 = container_of(work,
struct pca9633_led, work);
- 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;
}
@@ -98,26 +125,30 @@ 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 = client->dev.platform_data;
- 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) {
@@ -151,7 +182,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;
@@ -169,10 +202,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] 6+ messages in thread* [PATCH 2/3] leds-pca9633: Unique naming of the LEDs
2013-08-08 8:47 [PATCH 0/3] PCA9633: Add support to PCA9634 and fix some problems Ricardo Ribalda Delgado
2013-08-08 8:47 ` [PATCH 1/3] leds-pca9633: Add support for PCA9634 Ricardo Ribalda Delgado
@ 2013-08-08 8:47 ` Ricardo Ribalda Delgado
2013-08-08 9:45 ` Peter Meerwald
2013-08-08 8:47 ` [PATCH 3/3] leds-pca9633: Add mutex to the ledout register Ricardo Ribalda Delgado
2 siblings, 1 reply; 6+ messages in thread
From: Ricardo Ribalda Delgado @ 2013-08-08 8:47 UTC (permalink / raw)
To: Bryan Wu, Richard Purdie, linux-leds, linux-kernel,
Peter Meerwald
Cc: Ricardo Ribalda Delgado
If there are 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 adress 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 | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/leds/leds-pca9633.c b/drivers/leds/leds-pca9633.c
index 070358a..4dd7793 100644
--- a/drivers/leds/leds-pca9633.c
+++ b/drivers/leds/leds-pca9633.c
@@ -159,10 +159,11 @@ 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:%.2x:%d", 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] 6+ messages in thread* Re: [PATCH 2/3] leds-pca9633: Unique naming of the LEDs
2013-08-08 8:47 ` [PATCH 2/3] leds-pca9633: Unique naming of the LEDs Ricardo Ribalda Delgado
@ 2013-08-08 9:45 ` Peter Meerwald
2013-08-08 10:06 ` Ricardo Ribalda Delgado
0 siblings, 1 reply; 6+ messages in thread
From: Peter Meerwald @ 2013-08-08 9:45 UTC (permalink / raw)
To: Ricardo Ribalda Delgado
Cc: Bryan Wu, Richard Purdie, linux-leds, linux-kernel
Hello,
> If there are 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 patch adds the i2c adress to the name of the unnamed LEDs, making
> them unique.
there still could be pca963x chips with the same address on different
I2C busses; the I2C address itself is not unique
spelling/nitpicking of commit message: 'is more than one pca963x chip',
'address'
regards, p.
> [ 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 | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/leds/leds-pca9633.c b/drivers/leds/leds-pca9633.c
> index 070358a..4dd7793 100644
> --- a/drivers/leds/leds-pca9633.c
> +++ b/drivers/leds/leds-pca9633.c
> @@ -159,10 +159,11 @@ 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:%.2x:%d", client->addr, i);
>
> pca9633[i].led_cdev.name = pca9633[i].name;
> pca9633[i].led_cdev.brightness_set = pca9633_led_set;
>
--
Peter Meerwald
+43-664-2444418 (mobile)
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 2/3] leds-pca9633: Unique naming of the LEDs
2013-08-08 9:45 ` Peter Meerwald
@ 2013-08-08 10:06 ` Ricardo Ribalda Delgado
0 siblings, 0 replies; 6+ messages in thread
From: Ricardo Ribalda Delgado @ 2013-08-08 10:06 UTC (permalink / raw)
To: Peter Meerwald; +Cc: Bryan Wu, Richard Purdie, linux-leds, LKML
Hello Peter
Thanks for your comment, I have added also the bus number to the name,
and fix the commit message.
I will wait for some more comments and then I will resend it again.
Thanks!
On Thu, Aug 8, 2013 at 11:45 AM, Peter Meerwald
<p.meerwald@bct-electronic.com> wrote:
> Hello,
>
>> If there are 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 patch adds the i2c adress to the name of the unnamed LEDs, making
>> them unique.
>
> there still could be pca963x chips with the same address on different
> I2C busses; the I2C address itself is not unique
>
> spelling/nitpicking of commit message: 'is more than one pca963x chip',
> 'address'
>
> regards, p.
>
>> [ 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 | 7 ++++---
>> 1 file changed, 4 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/leds/leds-pca9633.c b/drivers/leds/leds-pca9633.c
>> index 070358a..4dd7793 100644
>> --- a/drivers/leds/leds-pca9633.c
>> +++ b/drivers/leds/leds-pca9633.c
>> @@ -159,10 +159,11 @@ 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:%.2x:%d", client->addr, i);
>>
>> pca9633[i].led_cdev.name = pca9633[i].name;
>> pca9633[i].led_cdev.brightness_set = pca9633_led_set;
>>
>
> --
>
> Peter Meerwald
> +43-664-2444418 (mobile)
--
Ricardo Ribalda
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 3/3] leds-pca9633: Add mutex to the ledout register
2013-08-08 8:47 [PATCH 0/3] PCA9633: Add support to PCA9634 and fix some problems Ricardo Ribalda Delgado
2013-08-08 8:47 ` [PATCH 1/3] leds-pca9633: Add support for PCA9634 Ricardo Ribalda Delgado
2013-08-08 8:47 ` [PATCH 2/3] leds-pca9633: Unique naming of the LEDs Ricardo Ribalda Delgado
@ 2013-08-08 8:47 ` Ricardo Ribalda Delgado
2 siblings, 0 replies; 6+ messages in thread
From: Ricardo Ribalda Delgado @ 2013-08-08 8:47 UTC (permalink / raw)
To: Bryan Wu, Richard Purdie, linux-leds, linux-kernel,
Peter Meerwald
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 addes 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 | 63 ++++++++++++++++++++++++++++---------------
1 file changed, 41 insertions(+), 22 deletions(-)
diff --git a/drivers/leds/leds-pca9633.c b/drivers/leds/leds-pca9633.c
index 4dd7793..5e3d36f 100644
--- a/drivers/leds/leds-pca9633.c
+++ b/drivers/leds/leds-pca9633.c
@@ -65,9 +65,17 @@ static const struct i2c_device_id pca9633_id[] = {
};
MODULE_DEVICE_TABLE(i2c, pca9633_id);
-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;
@@ -79,29 +87,32 @@ static void pca9633_led_work(struct work_struct *work)
{
struct pca9633_led *pca9633 = container_of(work,
struct pca9633_led, work);
- 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_led_set(struct led_classdev *led_cdev,
@@ -123,6 +134,7 @@ static void pca9633_led_set(struct led_classdev *led_cdev,
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;
@@ -138,17 +150,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) {
@@ -182,11 +207,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:
@@ -200,14 +220,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] 6+ messages in thread