* [PATCH 0/3] leds: add SN3218 LED driver
@ 2016-01-31 12:59 Stefan Wahren
2016-01-31 12:59 ` [PATCH 1/3] of: Add vendor prefix for Si-En Technology Stefan Wahren
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Stefan Wahren @ 2016-01-31 12:59 UTC (permalink / raw)
To: Richard Purdie, Jacek Anaszewski, Rob Herring, Mark Rutland
Cc: devicetree, linux-kernel, linux-leds, Stefan Wahren
This patch series implements support for the Si-En SN3218
18 Channel LED Driver connected through I2C. The SN3218
doesn't support reading so the LED states are cached in a
regmap.
The driver has been tested on a Raspberry Pi B.
Changes since RFC:
* using regmap instead of direct I2C
* add new function to init single LED
* add module name to Kconfig help
* replace of_get_property() with of_property_read_string()
* remove lock, leds_state and name from priv structure
* drop sn3218 LED name prefix
* don't use led_info and led_platform_data
* abort probing if no SN3218 is connected or DT invalid
* fix some style and text issues
* fix license
* move i2c_device_id and of_device_id at the end of file
* use devm_* calls
* use device function as node name in DT binding
Stefan Wahren (3):
of: Add vendor prefix for Si-En Technology
DT: add binding for SN3218 LED driver
leds: add SN3218 LED driver
.../devicetree/bindings/leds/leds-sn3218.txt | 42 +++
.../devicetree/bindings/vendor-prefixes.txt | 1 +
drivers/leds/Kconfig | 12 +
drivers/leds/Makefile | 1 +
drivers/leds/leds-sn3218.c | 297 ++++++++++++++++++++
5 files changed, 353 insertions(+)
create mode 100644 Documentation/devicetree/bindings/leds/leds-sn3218.txt
create mode 100644 drivers/leds/leds-sn3218.c
--
1.7.9.5
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 1/3] of: Add vendor prefix for Si-En Technology
2016-01-31 12:59 [PATCH 0/3] leds: add SN3218 LED driver Stefan Wahren
@ 2016-01-31 12:59 ` Stefan Wahren
2016-01-31 12:59 ` [PATCH 2/3] DT: add binding for SN3218 LED driver Stefan Wahren
2016-01-31 12:59 ` [PATCH 3/3] leds: add " Stefan Wahren
2 siblings, 0 replies; 8+ messages in thread
From: Stefan Wahren @ 2016-01-31 12:59 UTC (permalink / raw)
To: Richard Purdie, Jacek Anaszewski, Rob Herring, Mark Rutland
Cc: devicetree, linux-kernel, linux-leds, Stefan Wahren
Si-En Technology is a fabless design house which offers
audio amplifiers, LED drivers and sensors.
Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
Acked-by: Rob Herring <robh@kernel.org>
---
.../devicetree/bindings/vendor-prefixes.txt | 1 +
1 file changed, 1 insertion(+)
diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt b/Documentation/devicetree/bindings/vendor-prefixes.txt
index 72e2c5a..52f22f9 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.txt
+++ b/Documentation/devicetree/bindings/vendor-prefixes.txt
@@ -204,6 +204,7 @@ seagate Seagate Technology PLC
semtech Semtech Corporation
sgx SGX Sensortech
sharp Sharp Corporation
+si-en Si-En Technology Ltd.
sigma Sigma Designs, Inc.
sil Silicon Image
silabs Silicon Laboratories
--
1.7.9.5
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 2/3] DT: add binding for SN3218 LED driver
2016-01-31 12:59 [PATCH 0/3] leds: add SN3218 LED driver Stefan Wahren
2016-01-31 12:59 ` [PATCH 1/3] of: Add vendor prefix for Si-En Technology Stefan Wahren
@ 2016-01-31 12:59 ` Stefan Wahren
2016-02-01 9:59 ` Jacek Anaszewski
2016-01-31 12:59 ` [PATCH 3/3] leds: add " Stefan Wahren
2 siblings, 1 reply; 8+ messages in thread
From: Stefan Wahren @ 2016-01-31 12:59 UTC (permalink / raw)
To: Richard Purdie, Jacek Anaszewski, Rob Herring, Mark Rutland
Cc: devicetree, linux-kernel, linux-leds, Stefan Wahren
This patch adds the binding for Si-En Technology SN3218
18-Channel LED driver.
Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
Acked-by: Rob Herring <robh@kernel.org>
---
.../devicetree/bindings/leds/leds-sn3218.txt | 42 ++++++++++++++++++++
1 file changed, 42 insertions(+)
create mode 100644 Documentation/devicetree/bindings/leds/leds-sn3218.txt
diff --git a/Documentation/devicetree/bindings/leds/leds-sn3218.txt b/Documentation/devicetree/bindings/leds/leds-sn3218.txt
new file mode 100644
index 0000000..5daa9b0
--- /dev/null
+++ b/Documentation/devicetree/bindings/leds/leds-sn3218.txt
@@ -0,0 +1,42 @@
+* Si-En Technology - SN3218 18-Channel LED Driver
+
+Required properties:
+- compatible :
+ "si-en,sn3218"
+- address-cells : must be 1
+- size-cells : must be 0
+- reg : I2C slave address, typically 0x54
+
+There must be at least 1 LED which is represented as a sub-node
+of the sn3218 device.
+
+LED sub-node properties:
+- label : (optional) see Documentation/devicetree/bindings/leds/common.txt
+- reg : number of LED line (could be from 0 to 17)
+- linux,default-trigger : (optional)
+ see Documentation/devicetree/bindings/leds/common.txt
+
+Example:
+
+sn3218: led-controller@54 {
+ compatible = "si-en,sn3218";
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <0x54>;
+
+ led@0 {
+ label = "led1";
+ reg = <0x0>;
+ };
+
+ led@1 {
+ label = "led2";
+ reg = <0x1>;
+ };
+
+ led@2 {
+ label = "led3";
+ reg = <0x2>;
+ };
+};
+
--
1.7.9.5
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 3/3] leds: add SN3218 LED driver
2016-01-31 12:59 [PATCH 0/3] leds: add SN3218 LED driver Stefan Wahren
2016-01-31 12:59 ` [PATCH 1/3] of: Add vendor prefix for Si-En Technology Stefan Wahren
2016-01-31 12:59 ` [PATCH 2/3] DT: add binding for SN3218 LED driver Stefan Wahren
@ 2016-01-31 12:59 ` Stefan Wahren
2016-02-01 9:58 ` Jacek Anaszewski
2 siblings, 1 reply; 8+ messages in thread
From: Stefan Wahren @ 2016-01-31 12:59 UTC (permalink / raw)
To: Richard Purdie, Jacek Anaszewski, Rob Herring, Mark Rutland
Cc: devicetree, linux-kernel, linux-leds, Stefan Wahren
This patch adds support for Si-En SN3218 18 Channel LED Driver.
Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
---
drivers/leds/Kconfig | 12 ++
drivers/leds/Makefile | 1 +
drivers/leds/leds-sn3218.c | 297 ++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 310 insertions(+)
create mode 100644 drivers/leds/leds-sn3218.c
diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index 7f940c2..920c2da 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -568,6 +568,18 @@ config LEDS_SEAD3
This driver can also be built as a module. If so the module
will be called leds-sead3.
+config LEDS_SN3218
+ tristate "LED support for Si-En SN3218 I2C chip"
+ depends on LEDS_CLASS && I2C
+ depends on OF
+ select REGMAP_I2C
+ help
+ This option enables support for the Si-EN SN3218 LED driver
+ connected through I2C. Say Y to enable support for the SN3218 LED.
+
+ This driver can also be built as a module. If so the module
+ will be called leds-sn3218.
+
comment "LED driver for blink(1) USB RGB LED is under Special HID drivers (HID_THINGM)"
config LEDS_BLINKM
diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
index e9d5309..89c9b6f 100644
--- a/drivers/leds/Makefile
+++ b/drivers/leds/Makefile
@@ -66,6 +66,7 @@ obj-$(CONFIG_LEDS_MENF21BMC) += leds-menf21bmc.o
obj-$(CONFIG_LEDS_KTD2692) += leds-ktd2692.o
obj-$(CONFIG_LEDS_POWERNV) += leds-powernv.o
obj-$(CONFIG_LEDS_SEAD3) += leds-sead3.o
+obj-$(CONFIG_LEDS_SN3218) += leds-sn3218.o
# LED SPI Drivers
obj-$(CONFIG_LEDS_DAC124S085) += leds-dac124s085.o
diff --git a/drivers/leds/leds-sn3218.c b/drivers/leds/leds-sn3218.c
new file mode 100644
index 0000000..7a96323
--- /dev/null
+++ b/drivers/leds/leds-sn3218.c
@@ -0,0 +1,297 @@
+/*
+ * Si-En SN3218 18 Channel LED Driver
+ *
+ * Copyright (C) 2016 Stefan Wahren <stefan.wahren@i2se.com>
+ *
+ * Based on leds-pca963x.c
+ *
+ * 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.
+ *
+ * Datasheet: http://www.si-en.com/uploadpdf/s2011517171720.pdf
+ *
+ */
+
+#include <linux/err.h>
+#include <linux/i2c.h>
+#include <linux/leds.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/regmap.h>
+#include <linux/slab.h>
+
+#define SN3218_MODE 0x00
+#define SN3218_PWM_1 0x01
+#define SN3218_PWM_2 0x02
+#define SN3218_PWM_3 0x03
+#define SN3218_PWM_4 0x04
+#define SN3218_PWM_5 0x05
+#define SN3218_PWM_6 0x06
+#define SN3218_PWM_7 0x07
+#define SN3218_PWM_8 0x08
+#define SN3218_PWM_9 0x09
+#define SN3218_PWM_10 0x0a
+#define SN3218_PWM_11 0x0b
+#define SN3218_PWM_12 0x0c
+#define SN3218_PWM_13 0x0d
+#define SN3218_PWM_14 0x0e
+#define SN3218_PWM_15 0x0f
+#define SN3218_PWM_16 0x10
+#define SN3218_PWM_17 0x11
+#define SN3218_PWM_18 0x12
+#define SN3218_LED_1_6 0x13
+#define SN3218_LED_7_12 0x14
+#define SN3218_LED_13_18 0x15
+#define SN3218_UPDATE 0x16 /* applies to reg 0x01 .. 0x15 */
+#define SN3218_RESET 0x17
+
+#define SN3218_LED_MASK 0x3f
+#define SN3218_LED_ON 0x01
+#define SN3218_LED_OFF 0x00
+
+#define NUM_LEDS 18
+
+struct sn3218_led;
+
+/**
+ * struct sn3218 -
+ * @client - Pointer to the I2C client
+ * @leds - Pointer to the individual LEDs
+ * @num_leds - Actual number of LEDs
+**/
+struct sn3218 {
+ struct i2c_client *client;
+ struct regmap *regmap;
+ struct sn3218_led *leds;
+ int num_leds;
+};
+
+/**
+ * struct sn3218_led -
+ * @chip - Pointer to the container
+ * @led_cdev - led class device pointer
+ * @led_num - LED index ( 0 .. 17 )
+**/
+struct sn3218_led {
+ struct sn3218 *chip;
+ struct led_classdev led_cdev;
+ int led_num;
+};
+
+static int sn3218_led_set(struct led_classdev *led_cdev,
+ enum led_brightness brightness)
+{
+ struct sn3218_led *led =
+ container_of(led_cdev, struct sn3218_led, led_cdev);
+ struct regmap *regmap = led->chip->regmap;
+ u8 bank = led->led_num / 6;
+ u8 mask = 0x1 << (led->led_num % 6);
+ u8 val;
+ int ret;
+
+ if (brightness == LED_OFF)
+ val = 0;
+ else
+ val = mask;
+
+ ret = regmap_update_bits(regmap, SN3218_LED_1_6 + bank, mask, val);
+ if (ret < 0)
+ return ret;
+
+ if (brightness > LED_OFF) {
+ ret = regmap_write(regmap, SN3218_PWM_1 + led->led_num,
+ brightness);
+ if (ret < 0)
+ return ret;
+ }
+
+ ret = regmap_write(regmap, SN3218_UPDATE, 0xff);
+
+ return ret;
+}
+
+void sn3218_led_init(struct sn3218 *sn3218, struct device_node *node, u32 reg)
+{
+ struct sn3218_led *leds = sn3218->leds;
+ struct led_classdev *cdev = &leds[reg].led_cdev;
+
+ leds[reg].led_num = reg;
+ leds[reg].chip = sn3218;
+
+ if (of_property_read_string(node, "label", &cdev->name))
+ cdev->name = node->name;
+
+ of_property_read_string(node, "linux,default-trigger",
+ &cdev->default_trigger);
+
+ cdev->brightness_set_blocking = sn3218_led_set;
+ cdev->max_brightness = LED_FULL;
+}
+
+static const struct reg_default sn3218_reg_defs[] = {
+ { SN3218_MODE, 0x00},
+ { SN3218_PWM_1, 0x00},
+ { SN3218_PWM_2, 0x00},
+ { SN3218_PWM_3, 0x00},
+ { SN3218_PWM_4, 0x00},
+ { SN3218_PWM_5, 0x00},
+ { SN3218_PWM_6, 0x00},
+ { SN3218_PWM_7, 0x00},
+ { SN3218_PWM_8, 0x00},
+ { SN3218_PWM_9, 0x00},
+ { SN3218_PWM_10, 0x00},
+ { SN3218_PWM_11, 0x00},
+ { SN3218_PWM_12, 0x00},
+ { SN3218_PWM_13, 0x00},
+ { SN3218_PWM_14, 0x00},
+ { SN3218_PWM_15, 0x00},
+ { SN3218_PWM_16, 0x00},
+ { SN3218_PWM_17, 0x00},
+ { SN3218_PWM_18, 0x00},
+ { SN3218_LED_1_6, 0x00},
+ { SN3218_LED_7_12, 0x00},
+ { SN3218_LED_13_18, 0x00},
+ { SN3218_UPDATE, 0x00},
+ { SN3218_RESET, 0x00},
+};
+
+static const struct regmap_config sn3218_regmap_config = {
+ .reg_bits = 8,
+ .val_bits = 8,
+
+ .max_register = SN3218_RESET,
+ .reg_defaults = sn3218_reg_defs,
+ .num_reg_defaults = ARRAY_SIZE(sn3218_reg_defs),
+ .cache_type = REGCACHE_RBTREE,
+};
+
+static int sn3218_init(struct i2c_client *client, struct sn3218 *sn3218)
+{
+ struct device_node *np = client->dev.of_node, *child;
+ struct sn3218_led *leds;
+ int ret, count;
+
+ count = of_get_child_count(np);
+ if (!count)
+ return -ENODEV;
+
+ if (count > NUM_LEDS) {
+ dev_err(&client->dev, "Invalid LED count %d\n", count);
+ return -EINVAL;
+ }
+
+ leds = devm_kzalloc(&client->dev, count * sizeof(*leds), GFP_KERNEL);
+ if (!leds)
+ return -ENOMEM;
+
+ sn3218->leds = leds;
+ sn3218->num_leds = count;
+ sn3218->client = client;
+
+ sn3218->regmap = devm_regmap_init_i2c(client, &sn3218_regmap_config);
+ if (IS_ERR(sn3218->regmap)) {
+ ret = PTR_ERR(sn3218->regmap);
+ dev_err(&client->dev, "Failed to allocate register map: %d\n",
+ ret);
+ return ret;
+ }
+
+ for_each_child_of_node(np, child) {
+ u32 reg;
+
+ ret = of_property_read_u32(child, "reg", ®);
+ if (ret)
+ goto fail;
+
+ if (reg >= count) {
+ dev_err(&client->dev, "Invalid LED (%u >= %d)\n", reg,
+ count);
+ ret = -EINVAL;
+ goto fail;
+ }
+
+ sn3218_led_init(sn3218, child, reg);
+ }
+
+ return 0;
+
+fail:
+ of_node_put(np);
+ return ret;
+}
+
+static int sn3218_probe(struct i2c_client *client,
+ const struct i2c_device_id *id)
+{
+ struct sn3218 *sn3218;
+ struct sn3218_led *leds;
+ struct device *dev = &client->dev;
+ int i, ret;
+
+ sn3218 = devm_kzalloc(dev, sizeof(*sn3218), GFP_KERNEL);
+ if (!sn3218)
+ return -ENOMEM;
+
+ ret = sn3218_init(client, sn3218);
+ if (ret)
+ return ret;
+
+ i2c_set_clientdata(client, sn3218);
+ leds = sn3218->leds;
+
+ /*
+ * Since the chip is write-only we need to reset him into
+ * a defined state (all LEDs off).
+ */
+ ret = regmap_write(sn3218->regmap, SN3218_RESET, 0xff);
+ if (ret)
+ return ret;
+
+ for (i = 0; i < sn3218->num_leds; i++) {
+ ret = devm_led_classdev_register(dev, &leds[i].led_cdev);
+ if (ret < 0)
+ return ret;
+ }
+
+ /* Set normal mode */
+ return regmap_write(sn3218->regmap, SN3218_MODE, 0x01);
+}
+
+static int sn3218_remove(struct i2c_client *client)
+{
+ struct sn3218 *sn3218 = i2c_get_clientdata(client);
+
+ /* Set shutdown mode */
+ regmap_write(sn3218->regmap, SN3218_MODE, 0x00);
+
+ return 0;
+}
+
+static const struct i2c_device_id sn3218_id[] = {
+ { "sn3218", 0 },
+ { }
+};
+MODULE_DEVICE_TABLE(i2c, sn3218_id);
+
+static const struct of_device_id of_sn3218_match[] = {
+ { .compatible = "si-en,sn3218", },
+ {},
+};
+MODULE_DEVICE_TABLE(of, of_sn3218_match);
+
+static struct i2c_driver sn3218_driver = {
+ .driver = {
+ .name = "leds-sn3218",
+ .of_match_table = of_match_ptr(of_sn3218_match),
+ },
+ .probe = sn3218_probe,
+ .remove = sn3218_remove,
+ .id_table = sn3218_id,
+};
+
+module_i2c_driver(sn3218_driver);
+
+MODULE_DESCRIPTION("Si-En SN3218 LED Driver");
+MODULE_AUTHOR("Stefan Wahren <stefan.wahren@i2se.com>");
+MODULE_LICENSE("GPL v2");
--
1.7.9.5
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 3/3] leds: add SN3218 LED driver
2016-01-31 12:59 ` [PATCH 3/3] leds: add " Stefan Wahren
@ 2016-02-01 9:58 ` Jacek Anaszewski
2016-02-01 12:53 ` Jacek Anaszewski
0 siblings, 1 reply; 8+ messages in thread
From: Jacek Anaszewski @ 2016-02-01 9:58 UTC (permalink / raw)
To: Stefan Wahren
Cc: Richard Purdie, Rob Herring, Mark Rutland, devicetree,
linux-kernel, linux-leds
Hi Stefan,
Thanks for the update. A few more comments below.
On 01/31/2016 01:59 PM, Stefan Wahren wrote:
> This patch adds support for Si-En SN3218 18 Channel LED Driver.
>
> Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
> ---
> drivers/leds/Kconfig | 12 ++
> drivers/leds/Makefile | 1 +
> drivers/leds/leds-sn3218.c | 297 ++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 310 insertions(+)
> create mode 100644 drivers/leds/leds-sn3218.c
>
> diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
> index 7f940c2..920c2da 100644
> --- a/drivers/leds/Kconfig
> +++ b/drivers/leds/Kconfig
> @@ -568,6 +568,18 @@ config LEDS_SEAD3
> This driver can also be built as a module. If so the module
> will be called leds-sead3.
>
> +config LEDS_SN3218
> + tristate "LED support for Si-En SN3218 I2C chip"
> + depends on LEDS_CLASS && I2C
> + depends on OF
> + select REGMAP_I2C
> + help
> + This option enables support for the Si-EN SN3218 LED driver
> + connected through I2C. Say Y to enable support for the SN3218 LED.
> +
> + This driver can also be built as a module. If so the module
> + will be called leds-sn3218.
> +
> comment "LED driver for blink(1) USB RGB LED is under Special HID drivers (HID_THINGM)"
>
> config LEDS_BLINKM
> diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
> index e9d5309..89c9b6f 100644
> --- a/drivers/leds/Makefile
> +++ b/drivers/leds/Makefile
> @@ -66,6 +66,7 @@ obj-$(CONFIG_LEDS_MENF21BMC) += leds-menf21bmc.o
> obj-$(CONFIG_LEDS_KTD2692) += leds-ktd2692.o
> obj-$(CONFIG_LEDS_POWERNV) += leds-powernv.o
> obj-$(CONFIG_LEDS_SEAD3) += leds-sead3.o
> +obj-$(CONFIG_LEDS_SN3218) += leds-sn3218.o
>
> # LED SPI Drivers
> obj-$(CONFIG_LEDS_DAC124S085) += leds-dac124s085.o
> diff --git a/drivers/leds/leds-sn3218.c b/drivers/leds/leds-sn3218.c
> new file mode 100644
> index 0000000..7a96323
> --- /dev/null
> +++ b/drivers/leds/leds-sn3218.c
> @@ -0,0 +1,297 @@
> +/*
> + * Si-En SN3218 18 Channel LED Driver
> + *
> + * Copyright (C) 2016 Stefan Wahren <stefan.wahren@i2se.com>
> + *
> + * Based on leds-pca963x.c
> + *
> + * 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.
> + *
> + * Datasheet: http://www.si-en.com/uploadpdf/s2011517171720.pdf
> + *
> + */
> +
> +#include <linux/err.h>
> +#include <linux/i2c.h>
> +#include <linux/leds.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/regmap.h>
> +#include <linux/slab.h>
> +
> +#define SN3218_MODE 0x00
> +#define SN3218_PWM_1 0x01
> +#define SN3218_PWM_2 0x02
> +#define SN3218_PWM_3 0x03
> +#define SN3218_PWM_4 0x04
> +#define SN3218_PWM_5 0x05
> +#define SN3218_PWM_6 0x06
> +#define SN3218_PWM_7 0x07
> +#define SN3218_PWM_8 0x08
> +#define SN3218_PWM_9 0x09
> +#define SN3218_PWM_10 0x0a
> +#define SN3218_PWM_11 0x0b
> +#define SN3218_PWM_12 0x0c
> +#define SN3218_PWM_13 0x0d
> +#define SN3218_PWM_14 0x0e
> +#define SN3218_PWM_15 0x0f
> +#define SN3218_PWM_16 0x10
> +#define SN3218_PWM_17 0x11
> +#define SN3218_PWM_18 0x12
> +#define SN3218_LED_1_6 0x13
> +#define SN3218_LED_7_12 0x14
> +#define SN3218_LED_13_18 0x15
> +#define SN3218_UPDATE 0x16 /* applies to reg 0x01 .. 0x15 */
> +#define SN3218_RESET 0x17
> +
> +#define SN3218_LED_MASK 0x3f
> +#define SN3218_LED_ON 0x01
> +#define SN3218_LED_OFF 0x00
> +
> +#define NUM_LEDS 18
> +
> +struct sn3218_led;
> +
> +/**
> + * struct sn3218 -
> + * @client - Pointer to the I2C client
> + * @leds - Pointer to the individual LEDs
> + * @num_leds - Actual number of LEDs
> +**/
> +struct sn3218 {
> + struct i2c_client *client;
> + struct regmap *regmap;
> + struct sn3218_led *leds;
> + int num_leds;
> +};
> +
> +/**
> + * struct sn3218_led -
> + * @chip - Pointer to the container
> + * @led_cdev - led class device pointer
> + * @led_num - LED index ( 0 .. 17 )
> +**/
> +struct sn3218_led {
> + struct sn3218 *chip;
You don't need this if you have led id here. Please refer to
drivers/leds/leds-max77693.c, sub_led_to_led() to check how to get
a pointer to the parent structure in similar case.
> + struct led_classdev led_cdev;
> + int led_num;
> +};
> +
> +static int sn3218_led_set(struct led_classdev *led_cdev,
> + enum led_brightness brightness)
> +{
> + struct sn3218_led *led =
> + container_of(led_cdev, struct sn3218_led, led_cdev);
> + struct regmap *regmap = led->chip->regmap;
> + u8 bank = led->led_num / 6;
> + u8 mask = 0x1 << (led->led_num % 6);
> + u8 val;
> + int ret;
> +
> + if (brightness == LED_OFF)
> + val = 0;
> + else
> + val = mask;
> +
> + ret = regmap_update_bits(regmap, SN3218_LED_1_6 + bank, mask, val);
> + if (ret < 0)
> + return ret;
> +
> + if (brightness > LED_OFF) {
> + ret = regmap_write(regmap, SN3218_PWM_1 + led->led_num,
> + brightness);
> + if (ret < 0)
> + return ret;
> + }
> +
> + ret = regmap_write(regmap, SN3218_UPDATE, 0xff);
> +
> + return ret;
> +}
> +
> +void sn3218_led_init(struct sn3218 *sn3218, struct device_node *node, u32 reg)
s/void/static void/
> +{
> + struct sn3218_led *leds = sn3218->leds;
> + struct led_classdev *cdev = &leds[reg].led_cdev;
> +
> + leds[reg].led_num = reg;
> + leds[reg].chip = sn3218;
> +
> + if (of_property_read_string(node, "label", &cdev->name))
> + cdev->name = node->name;
> +
> + of_property_read_string(node, "linux,default-trigger",
> + &cdev->default_trigger);
> +
> + cdev->brightness_set_blocking = sn3218_led_set;
> + cdev->max_brightness = LED_FULL;
Skip this line. If max_brightness is 0 led_classdev_register()
automatically initializes it to LED_FULL.
> +}
> +
> +static const struct reg_default sn3218_reg_defs[] = {
> + { SN3218_MODE, 0x00},
> + { SN3218_PWM_1, 0x00},
> + { SN3218_PWM_2, 0x00},
> + { SN3218_PWM_3, 0x00},
> + { SN3218_PWM_4, 0x00},
> + { SN3218_PWM_5, 0x00},
> + { SN3218_PWM_6, 0x00},
> + { SN3218_PWM_7, 0x00},
> + { SN3218_PWM_8, 0x00},
> + { SN3218_PWM_9, 0x00},
> + { SN3218_PWM_10, 0x00},
> + { SN3218_PWM_11, 0x00},
> + { SN3218_PWM_12, 0x00},
> + { SN3218_PWM_13, 0x00},
> + { SN3218_PWM_14, 0x00},
> + { SN3218_PWM_15, 0x00},
> + { SN3218_PWM_16, 0x00},
> + { SN3218_PWM_17, 0x00},
> + { SN3218_PWM_18, 0x00},
> + { SN3218_LED_1_6, 0x00},
> + { SN3218_LED_7_12, 0x00},
> + { SN3218_LED_13_18, 0x00},
> + { SN3218_UPDATE, 0x00},
> + { SN3218_RESET, 0x00},
> +};
> +
> +static const struct regmap_config sn3218_regmap_config = {
> + .reg_bits = 8,
> + .val_bits = 8,
> +
> + .max_register = SN3218_RESET,
> + .reg_defaults = sn3218_reg_defs,
> + .num_reg_defaults = ARRAY_SIZE(sn3218_reg_defs),
> + .cache_type = REGCACHE_RBTREE,
> +};
> +
> +static int sn3218_init(struct i2c_client *client, struct sn3218 *sn3218)
> +{
> + struct device_node *np = client->dev.of_node, *child;
> + struct sn3218_led *leds;
> + int ret, count;
> +
> + count = of_get_child_count(np);
> + if (!count)
> + return -ENODEV;
> +
> + if (count > NUM_LEDS) {
> + dev_err(&client->dev, "Invalid LED count %d\n", count);
> + return -EINVAL;
> + }
> +
> + leds = devm_kzalloc(&client->dev, count * sizeof(*leds), GFP_KERNEL);
> + if (!leds)
> + return -ENOMEM;
> +
> + sn3218->leds = leds;
> + sn3218->num_leds = count;
> + sn3218->client = client;
> +
> + sn3218->regmap = devm_regmap_init_i2c(client, &sn3218_regmap_config);
> + if (IS_ERR(sn3218->regmap)) {
> + ret = PTR_ERR(sn3218->regmap);
> + dev_err(&client->dev, "Failed to allocate register map: %d\n",
> + ret);
> + return ret;
> + }
> +
> + for_each_child_of_node(np, child) {
> + u32 reg;
> +
> + ret = of_property_read_u32(child, "reg", ®);
> + if (ret)
> + goto fail;
> +
> + if (reg >= count) {
> + dev_err(&client->dev, "Invalid LED (%u >= %d)\n", reg,
> + count);
> + ret = -EINVAL;
> + goto fail;
> + }
> +
> + sn3218_led_init(sn3218, child, reg);
> + }
> +
> + return 0;
> +
> +fail:
> + of_node_put(np);
s/np/child/
> + return ret;
> +}
> +
> +static int sn3218_probe(struct i2c_client *client,
> + const struct i2c_device_id *id)
> +{
> + struct sn3218 *sn3218;
> + struct sn3218_led *leds;
> + struct device *dev = &client->dev;
> + int i, ret;
> +
> + sn3218 = devm_kzalloc(dev, sizeof(*sn3218), GFP_KERNEL);
> + if (!sn3218)
> + return -ENOMEM;
> +
> + ret = sn3218_init(client, sn3218);
> + if (ret)
> + return ret;
> +
> + i2c_set_clientdata(client, sn3218);
> + leds = sn3218->leds;
> +
> + /*
> + * Since the chip is write-only we need to reset him into
> + * a defined state (all LEDs off).
> + */
> + ret = regmap_write(sn3218->regmap, SN3218_RESET, 0xff);
> + if (ret)
> + return ret;
> +
> + for (i = 0; i < sn3218->num_leds; i++) {
> + ret = devm_led_classdev_register(dev, &leds[i].led_cdev);
> + if (ret < 0)
> + return ret;
> + }
> +
> + /* Set normal mode */
> + return regmap_write(sn3218->regmap, SN3218_MODE, 0x01);
> +}
> +
> +static int sn3218_remove(struct i2c_client *client)
> +{
> + struct sn3218 *sn3218 = i2c_get_clientdata(client);
> +
> + /* Set shutdown mode */
> + regmap_write(sn3218->regmap, SN3218_MODE, 0x00);
> +
> + return 0;
> +}
> +
> +static const struct i2c_device_id sn3218_id[] = {
> + { "sn3218", 0 },
> + { }
> +};
> +MODULE_DEVICE_TABLE(i2c, sn3218_id);
> +
> +static const struct of_device_id of_sn3218_match[] = {
> + { .compatible = "si-en,sn3218", },
> + {},
> +};
> +MODULE_DEVICE_TABLE(of, of_sn3218_match);
> +
> +static struct i2c_driver sn3218_driver = {
> + .driver = {
> + .name = "leds-sn3218",
> + .of_match_table = of_match_ptr(of_sn3218_match),
> + },
> + .probe = sn3218_probe,
> + .remove = sn3218_remove,
> + .id_table = sn3218_id,
> +};
> +
> +module_i2c_driver(sn3218_driver);
> +
> +MODULE_DESCRIPTION("Si-En SN3218 LED Driver");
> +MODULE_AUTHOR("Stefan Wahren <stefan.wahren@i2se.com>");
> +MODULE_LICENSE("GPL v2");
>
--
Best Regards,
Jacek Anaszewski
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/3] DT: add binding for SN3218 LED driver
2016-01-31 12:59 ` [PATCH 2/3] DT: add binding for SN3218 LED driver Stefan Wahren
@ 2016-02-01 9:59 ` Jacek Anaszewski
0 siblings, 0 replies; 8+ messages in thread
From: Jacek Anaszewski @ 2016-02-01 9:59 UTC (permalink / raw)
To: Stefan Wahren
Cc: Richard Purdie, Rob Herring, Mark Rutland, devicetree,
linux-kernel, linux-leds
On 01/31/2016 01:59 PM, Stefan Wahren wrote:
> This patch adds the binding for Si-En Technology SN3218
> 18-Channel LED driver.
>
> Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
> Acked-by: Rob Herring <robh@kernel.org>
> ---
> .../devicetree/bindings/leds/leds-sn3218.txt | 42 ++++++++++++++++++++
> 1 file changed, 42 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/leds/leds-sn3218.txt
>
> diff --git a/Documentation/devicetree/bindings/leds/leds-sn3218.txt b/Documentation/devicetree/bindings/leds/leds-sn3218.txt
> new file mode 100644
> index 0000000..5daa9b0
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/leds/leds-sn3218.txt
> @@ -0,0 +1,42 @@
> +* Si-En Technology - SN3218 18-Channel LED Driver
> +
> +Required properties:
> +- compatible :
> + "si-en,sn3218"
> +- address-cells : must be 1
> +- size-cells : must be 0
> +- reg : I2C slave address, typically 0x54
> +
> +There must be at least 1 LED which is represented as a sub-node
> +of the sn3218 device.
> +
> +LED sub-node properties:
> +- label : (optional) see Documentation/devicetree/bindings/leds/common.txt
> +- reg : number of LED line (could be from 0 to 17)
> +- linux,default-trigger : (optional)
> + see Documentation/devicetree/bindings/leds/common.txt
> +
> +Example:
> +
> +sn3218: led-controller@54 {
> + compatible = "si-en,sn3218";
> + #address-cells = <1>;
> + #size-cells = <0>;
> + reg = <0x54>;
> +
> + led@0 {
> + label = "led1";
> + reg = <0x0>;
> + };
> +
> + led@1 {
> + label = "led2";
> + reg = <0x1>;
> + };
> +
> + led@2 {
> + label = "led3";
> + reg = <0x2>;
> + };
> +};
> +
Please remove this empty line.
--
Best Regards,
Jacek Anaszewski
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 3/3] leds: add SN3218 LED driver
2016-02-01 9:58 ` Jacek Anaszewski
@ 2016-02-01 12:53 ` Jacek Anaszewski
2016-02-01 13:20 ` Stefan Wahren
0 siblings, 1 reply; 8+ messages in thread
From: Jacek Anaszewski @ 2016-02-01 12:53 UTC (permalink / raw)
To: Stefan Wahren
Cc: Richard Purdie, Rob Herring, Mark Rutland, devicetree,
linux-kernel, linux-leds
On 02/01/2016 10:58 AM, Jacek Anaszewski wrote:
> Hi Stefan,
>
> Thanks for the update. A few more comments below.
>
> On 01/31/2016 01:59 PM, Stefan Wahren wrote:
>> This patch adds support for Si-En SN3218 18 Channel LED Driver.
>>
>> Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
>> ---
>> drivers/leds/Kconfig | 12 ++
>> drivers/leds/Makefile | 1 +
>> drivers/leds/leds-sn3218.c | 297
>> ++++++++++++++++++++++++++++++++++++++++++++
>> 3 files changed, 310 insertions(+)
>> create mode 100644 drivers/leds/leds-sn3218.c
>>
>> diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
>> index 7f940c2..920c2da 100644
>> --- a/drivers/leds/Kconfig
>> +++ b/drivers/leds/Kconfig
>> @@ -568,6 +568,18 @@ config LEDS_SEAD3
>> This driver can also be built as a module. If so the module
>> will be called leds-sead3.
>>
>> +config LEDS_SN3218
>> + tristate "LED support for Si-En SN3218 I2C chip"
>> + depends on LEDS_CLASS && I2C
>> + depends on OF
>> + select REGMAP_I2C
>> + help
>> + This option enables support for the Si-EN SN3218 LED driver
>> + connected through I2C. Say Y to enable support for the SN3218 LED.
>> +
>> + This driver can also be built as a module. If so the module
>> + will be called leds-sn3218.
>> +
>> comment "LED driver for blink(1) USB RGB LED is under Special HID
>> drivers (HID_THINGM)"
>>
>> config LEDS_BLINKM
>> diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
>> index e9d5309..89c9b6f 100644
>> --- a/drivers/leds/Makefile
>> +++ b/drivers/leds/Makefile
>> @@ -66,6 +66,7 @@ obj-$(CONFIG_LEDS_MENF21BMC) += leds-menf21bmc.o
>> obj-$(CONFIG_LEDS_KTD2692) += leds-ktd2692.o
>> obj-$(CONFIG_LEDS_POWERNV) += leds-powernv.o
>> obj-$(CONFIG_LEDS_SEAD3) += leds-sead3.o
>> +obj-$(CONFIG_LEDS_SN3218) += leds-sn3218.o
>>
>> # LED SPI Drivers
>> obj-$(CONFIG_LEDS_DAC124S085) += leds-dac124s085.o
>> diff --git a/drivers/leds/leds-sn3218.c b/drivers/leds/leds-sn3218.c
>> new file mode 100644
>> index 0000000..7a96323
>> --- /dev/null
>> +++ b/drivers/leds/leds-sn3218.c
>> @@ -0,0 +1,297 @@
>> +/*
>> + * Si-En SN3218 18 Channel LED Driver
>> + *
>> + * Copyright (C) 2016 Stefan Wahren <stefan.wahren@i2se.com>
>> + *
>> + * Based on leds-pca963x.c
>> + *
>> + * 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.
>> + *
>> + * Datasheet: http://www.si-en.com/uploadpdf/s2011517171720.pdf
>> + *
>> + */
>> +
>> +#include <linux/err.h>
>> +#include <linux/i2c.h>
>> +#include <linux/leds.h>
>> +#include <linux/module.h>
>> +#include <linux/of.h>
>> +#include <linux/regmap.h>
>> +#include <linux/slab.h>
>> +
>> +#define SN3218_MODE 0x00
>> +#define SN3218_PWM_1 0x01
>> +#define SN3218_PWM_2 0x02
>> +#define SN3218_PWM_3 0x03
>> +#define SN3218_PWM_4 0x04
>> +#define SN3218_PWM_5 0x05
>> +#define SN3218_PWM_6 0x06
>> +#define SN3218_PWM_7 0x07
>> +#define SN3218_PWM_8 0x08
>> +#define SN3218_PWM_9 0x09
>> +#define SN3218_PWM_10 0x0a
>> +#define SN3218_PWM_11 0x0b
>> +#define SN3218_PWM_12 0x0c
>> +#define SN3218_PWM_13 0x0d
>> +#define SN3218_PWM_14 0x0e
>> +#define SN3218_PWM_15 0x0f
>> +#define SN3218_PWM_16 0x10
>> +#define SN3218_PWM_17 0x11
>> +#define SN3218_PWM_18 0x12
>> +#define SN3218_LED_1_6 0x13
>> +#define SN3218_LED_7_12 0x14
>> +#define SN3218_LED_13_18 0x15
>> +#define SN3218_UPDATE 0x16 /* applies to reg 0x01 .. 0x15 */
>> +#define SN3218_RESET 0x17
>> +
>> +#define SN3218_LED_MASK 0x3f
>> +#define SN3218_LED_ON 0x01
>> +#define SN3218_LED_OFF 0x00
>> +
>> +#define NUM_LEDS 18
>> +
>> +struct sn3218_led;
>> +
>> +/**
>> + * struct sn3218 -
>> + * @client - Pointer to the I2C client
>> + * @leds - Pointer to the individual LEDs
>> + * @num_leds - Actual number of LEDs
>> +**/
>> +struct sn3218 {
>> + struct i2c_client *client;
>> + struct regmap *regmap;
>> + struct sn3218_led *leds;
>> + int num_leds;
>> +};
>> +
>> +/**
>> + * struct sn3218_led -
>> + * @chip - Pointer to the container
>> + * @led_cdev - led class device pointer
>> + * @led_num - LED index ( 0 .. 17 )
>> +**/
>> +struct sn3218_led {
>> + struct sn3218 *chip;
>
> You don't need this if you have led id here. Please refer to
> drivers/leds/leds-max77693.c, sub_led_to_led() to check how to get
> a pointer to the parent structure in similar case.
Hmm, it would work only if leds was a static array in struct sn3218.
So, let's better leave this "chip" pointer intact.
>> + struct led_classdev led_cdev;
>> + int led_num;
>> +};
>> +
>> +static int sn3218_led_set(struct led_classdev *led_cdev,
>> + enum led_brightness brightness)
>> +{
>> + struct sn3218_led *led =
>> + container_of(led_cdev, struct sn3218_led, led_cdev);
>> + struct regmap *regmap = led->chip->regmap;
>> + u8 bank = led->led_num / 6;
>> + u8 mask = 0x1 << (led->led_num % 6);
>> + u8 val;
>> + int ret;
>> +
>> + if (brightness == LED_OFF)
>> + val = 0;
>> + else
>> + val = mask;
>> +
>> + ret = regmap_update_bits(regmap, SN3218_LED_1_6 + bank, mask, val);
>> + if (ret < 0)
>> + return ret;
>> +
>> + if (brightness > LED_OFF) {
>> + ret = regmap_write(regmap, SN3218_PWM_1 + led->led_num,
>> + brightness);
>> + if (ret < 0)
>> + return ret;
>> + }
>> +
>> + ret = regmap_write(regmap, SN3218_UPDATE, 0xff);
>> +
>> + return ret;
>> +}
>> +
>> +void sn3218_led_init(struct sn3218 *sn3218, struct device_node *node,
>> u32 reg)
>
> s/void/static void/
>
>> +{
>> + struct sn3218_led *leds = sn3218->leds;
>> + struct led_classdev *cdev = &leds[reg].led_cdev;
>> +
>> + leds[reg].led_num = reg;
>> + leds[reg].chip = sn3218;
>> +
>> + if (of_property_read_string(node, "label", &cdev->name))
>> + cdev->name = node->name;
>> +
>> + of_property_read_string(node, "linux,default-trigger",
>> + &cdev->default_trigger);
>> +
>> + cdev->brightness_set_blocking = sn3218_led_set;
>> + cdev->max_brightness = LED_FULL;
>
> Skip this line. If max_brightness is 0 led_classdev_register()
> automatically initializes it to LED_FULL.
>
>> +}
>> +
>> +static const struct reg_default sn3218_reg_defs[] = {
>> + { SN3218_MODE, 0x00},
>> + { SN3218_PWM_1, 0x00},
>> + { SN3218_PWM_2, 0x00},
>> + { SN3218_PWM_3, 0x00},
>> + { SN3218_PWM_4, 0x00},
>> + { SN3218_PWM_5, 0x00},
>> + { SN3218_PWM_6, 0x00},
>> + { SN3218_PWM_7, 0x00},
>> + { SN3218_PWM_8, 0x00},
>> + { SN3218_PWM_9, 0x00},
>> + { SN3218_PWM_10, 0x00},
>> + { SN3218_PWM_11, 0x00},
>> + { SN3218_PWM_12, 0x00},
>> + { SN3218_PWM_13, 0x00},
>> + { SN3218_PWM_14, 0x00},
>> + { SN3218_PWM_15, 0x00},
>> + { SN3218_PWM_16, 0x00},
>> + { SN3218_PWM_17, 0x00},
>> + { SN3218_PWM_18, 0x00},
>> + { SN3218_LED_1_6, 0x00},
>> + { SN3218_LED_7_12, 0x00},
>> + { SN3218_LED_13_18, 0x00},
>> + { SN3218_UPDATE, 0x00},
>> + { SN3218_RESET, 0x00},
>> +};
>> +
>> +static const struct regmap_config sn3218_regmap_config = {
>> + .reg_bits = 8,
>> + .val_bits = 8,
>> +
>> + .max_register = SN3218_RESET,
>> + .reg_defaults = sn3218_reg_defs,
>> + .num_reg_defaults = ARRAY_SIZE(sn3218_reg_defs),
>> + .cache_type = REGCACHE_RBTREE,
>> +};
>> +
>> +static int sn3218_init(struct i2c_client *client, struct sn3218 *sn3218)
>> +{
>> + struct device_node *np = client->dev.of_node, *child;
>> + struct sn3218_led *leds;
>> + int ret, count;
>> +
>> + count = of_get_child_count(np);
>> + if (!count)
>> + return -ENODEV;
>> +
>> + if (count > NUM_LEDS) {
>> + dev_err(&client->dev, "Invalid LED count %d\n", count);
>> + return -EINVAL;
>> + }
>> +
>> + leds = devm_kzalloc(&client->dev, count * sizeof(*leds),
>> GFP_KERNEL);
>> + if (!leds)
>> + return -ENOMEM;
>> +
>> + sn3218->leds = leds;
>> + sn3218->num_leds = count;
>> + sn3218->client = client;
>> +
>> + sn3218->regmap = devm_regmap_init_i2c(client,
>> &sn3218_regmap_config);
>> + if (IS_ERR(sn3218->regmap)) {
>> + ret = PTR_ERR(sn3218->regmap);
>> + dev_err(&client->dev, "Failed to allocate register map: %d\n",
>> + ret);
>> + return ret;
>> + }
>> +
>> + for_each_child_of_node(np, child) {
>> + u32 reg;
>> +
>> + ret = of_property_read_u32(child, "reg", ®);
>> + if (ret)
>> + goto fail;
>> +
>> + if (reg >= count) {
>> + dev_err(&client->dev, "Invalid LED (%u >= %d)\n", reg,
>> + count);
>> + ret = -EINVAL;
>> + goto fail;
>> + }
>> +
>> + sn3218_led_init(sn3218, child, reg);
>> + }
>> +
>> + return 0;
>> +
>> +fail:
>> + of_node_put(np);
>
> s/np/child/
>
>> + return ret;
>> +}
>> +
>> +static int sn3218_probe(struct i2c_client *client,
>> + const struct i2c_device_id *id)
>> +{
>> + struct sn3218 *sn3218;
>> + struct sn3218_led *leds;
>> + struct device *dev = &client->dev;
>> + int i, ret;
>> +
>> + sn3218 = devm_kzalloc(dev, sizeof(*sn3218), GFP_KERNEL);
>> + if (!sn3218)
>> + return -ENOMEM;
>> +
>> + ret = sn3218_init(client, sn3218);
>> + if (ret)
>> + return ret;
>> +
>> + i2c_set_clientdata(client, sn3218);
>> + leds = sn3218->leds;
>> +
>> + /*
>> + * Since the chip is write-only we need to reset him into
>> + * a defined state (all LEDs off).
>> + */
>> + ret = regmap_write(sn3218->regmap, SN3218_RESET, 0xff);
>> + if (ret)
>> + return ret;
>> +
>> + for (i = 0; i < sn3218->num_leds; i++) {
>> + ret = devm_led_classdev_register(dev, &leds[i].led_cdev);
>> + if (ret < 0)
>> + return ret;
>> + }
>> +
>> + /* Set normal mode */
>> + return regmap_write(sn3218->regmap, SN3218_MODE, 0x01);
>> +}
>> +
>> +static int sn3218_remove(struct i2c_client *client)
>> +{
>> + struct sn3218 *sn3218 = i2c_get_clientdata(client);
>> +
>> + /* Set shutdown mode */
>> + regmap_write(sn3218->regmap, SN3218_MODE, 0x00);
>> +
>> + return 0;
>> +}
>> +
>> +static const struct i2c_device_id sn3218_id[] = {
>> + { "sn3218", 0 },
>> + { }
>> +};
>> +MODULE_DEVICE_TABLE(i2c, sn3218_id);
>> +
>> +static const struct of_device_id of_sn3218_match[] = {
>> + { .compatible = "si-en,sn3218", },
>> + {},
>> +};
>> +MODULE_DEVICE_TABLE(of, of_sn3218_match);
>> +
>> +static struct i2c_driver sn3218_driver = {
>> + .driver = {
>> + .name = "leds-sn3218",
>> + .of_match_table = of_match_ptr(of_sn3218_match),
>> + },
>> + .probe = sn3218_probe,
>> + .remove = sn3218_remove,
>> + .id_table = sn3218_id,
>> +};
>> +
>> +module_i2c_driver(sn3218_driver);
>> +
>> +MODULE_DESCRIPTION("Si-En SN3218 LED Driver");
>> +MODULE_AUTHOR("Stefan Wahren <stefan.wahren@i2se.com>");
>> +MODULE_LICENSE("GPL v2");
>>
>
>
--
Best Regards,
Jacek Anaszewski
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 3/3] leds: add SN3218 LED driver
2016-02-01 12:53 ` Jacek Anaszewski
@ 2016-02-01 13:20 ` Stefan Wahren
0 siblings, 0 replies; 8+ messages in thread
From: Stefan Wahren @ 2016-02-01 13:20 UTC (permalink / raw)
To: Jacek Anaszewski
Cc: Richard Purdie, Rob Herring, Mark Rutland, devicetree,
linux-kernel, linux-leds
Am 01.02.2016 um 13:53 schrieb Jacek Anaszewski:
> On 02/01/2016 10:58 AM, Jacek Anaszewski wrote:
>> Hi Stefan,
>>
>> Thanks for the update. A few more comments below.
>>
>> On 01/31/2016 01:59 PM, Stefan Wahren wrote:
>>> +
>>> +/**
>>> + * struct sn3218 -
>>> + * @client - Pointer to the I2C client
>>> + * @leds - Pointer to the individual LEDs
>>> + * @num_leds - Actual number of LEDs
>>> +**/
>>> +struct sn3218 {
>>> + struct i2c_client *client;
>>> + struct regmap *regmap;
>>> + struct sn3218_led *leds;
>>> + int num_leds;
>>> +};
>>> +
>>> +/**
>>> + * struct sn3218_led -
>>> + * @chip - Pointer to the container
>>> + * @led_cdev - led class device pointer
>>> + * @led_num - LED index ( 0 .. 17 )
>>> +**/
>>> +struct sn3218_led {
>>> + struct sn3218 *chip;
>>
>> You don't need this if you have led id here. Please refer to
>> drivers/leds/leds-max77693.c, sub_led_to_led() to check how to get
>> a pointer to the parent structure in similar case.
>
> Hmm, it would work only if leds was a static array in struct sn3218.
> So, let's better leave this "chip" pointer intact.
Okay
I will wait until the end of the week before sending a new version.
Thanks
Stefan
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2016-02-01 13:20 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-01-31 12:59 [PATCH 0/3] leds: add SN3218 LED driver Stefan Wahren
2016-01-31 12:59 ` [PATCH 1/3] of: Add vendor prefix for Si-En Technology Stefan Wahren
2016-01-31 12:59 ` [PATCH 2/3] DT: add binding for SN3218 LED driver Stefan Wahren
2016-02-01 9:59 ` Jacek Anaszewski
2016-01-31 12:59 ` [PATCH 3/3] leds: add " Stefan Wahren
2016-02-01 9:58 ` Jacek Anaszewski
2016-02-01 12:53 ` Jacek Anaszewski
2016-02-01 13:20 ` Stefan Wahren
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).