Re: [PATCH v3 1/4] leds-pca9633: Add support for PCA9634

From: Bryan Wu
Date: Tue Aug 13 2013 - 13:12:40 EST


On Tue, Aug 13, 2013 at 7:49 AM, Ricardo Ribalda Delgado
<ricardo.ribalda@xxxxxxxxx> wrote:
> Hello Bryan
>
> Did you have a chance to look at the new series?
>

Sorry for the delay. I have no problem for this series. Could you
please rebase to my tree's for-next branch [1]? And post again?
Because recently there were several pca9533 driver changes merged,
your patches can't be applied cleanly.

Thanks,
-Bryan

[1]: http://git.kernel.org/cgit/linux/kernel/git/cooloney/linux-leds.git
>
> On Fri, Aug 9, 2013 at 8:45 AM, Ricardo Ribalda Delgado
> <ricardo.ribalda@xxxxxxxxx> wrote:
>> 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@xxxxxxxxx>
>> ---
>> 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@xxxxxxxxxxxxxxxxxx>
>> + * Author: Ricardo Ribalda <ricardo.ribalda@xxxxxxxxx>
>> *
>> * 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
>>
>
>
>
> --
> Ricardo Ribalda
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/