cap118x: add LED support

PATCH Superseded Matt Ranostay 5 participants 15 messages Jun 16, 2015 – Jun 17, 2015 2 messages not in the archive

Subsystems (4)

Device tree bindings HID Input drivers LED

lore

Share & tools

lore

Matt Ranostay

author

2015-06-16 02:45 UTC #0 lore raw

v4 cover cap118x: add LED support

Changes from v3:
* Fix race condition in work_queue
* Make 0 value (0% duty cycle) an invalid brightness option
* Document why LED brightness is only set global but ON/OFF state isn't

Matt Ranostay (2):
dt: add cap11xx LED documentation
cap11xx: add LED support

 .../devicetree/bindings/input/cap11xx.txt          |  23 ++++
 drivers/input/keyboard/cap11xx.c                   | 128 ++++++++++++++++++++-
 2 files changed, 148 insertions(+), 3 deletions(-)

Signature · 1 line

1.9.1

Matt Ranostay

author

2015-06-16 02:45 UTC #1 lore raw

PATCH v4 1/2 dt: add cap11xx LED documentation

Signed-off-by Matt Ranostay <[email protected]>

---
 .../devicetree/bindings/input/cap11xx.txt          | 23 ++++++++++++++++++++++
 1 file changed, 23 insertions(+)

Documentation/devicetree/bindings/input/cap11xx.txt +23 -0

index 7d0a300..01bb5b8 100644
@@ -38,6 +38,11 @@ Optional properties:
 				defaults. The array must have exactly six
 				entries.
 
+	linux,led-brightness:	Defines the ON brightness when the optional LED
+				functionality is used. Valid values are 1-15.
+				By default a value of 15 is set.
+
+
 Example:
 
 i2c_controller {

@@ -55,5 +60,23 @@ i2c_controller {
 				 <105>,		/* KEY_LEFT */
 				 <109>,		/* KEY_PAGEDOWN */
 				 <104>;		/* KEY_PAGEUP */
+
+		linux,led-brightness = <15>;
+
+		usr@0 {
+			label = "cap11xx:green:usr0";
+			reg = <0>;
+		};
+
+		usr@1 {
+			label = "cap11xx:green:usr1";
+			reg = <1>;
+		};
+
+		alive@2 {
+			label = "cap11xx:green:alive";
+			reg = <2>;
+			linux,default_trigger = "heartbeat";
+		};
 	};
 }

Signature · 1 line

1.9.1

Jacek Anaszewski

2015-06-16 07:57 UTC #2 lore raw

Re: [PATCH v4 1/2] dt: add cap11xx LED documentation

Quoted 22 lines · Matt Ranostay

Signed-off-by Matt Ranostay <[email protected]>

---
  .../devicetree/bindings/input/cap11xx.txt          | 23 ++++++++++++++++++++++
  1 file changed, 23 insertions(+)

Documentation/devicetree/bindings/input/cap11xx.txt +5 -0

index 7d0a300..01bb5b8 100644
@@ -38,6 +38,11 @@ Optional properties:
  				defaults. The array must have exactly six
  				entries.
 
+	linux,led-brightness:	Defines the ON brightness when the optional LED
+				functionality is used. Valid values are 1-15.
+				By default a value of 15 is set.
+
+
  Example:
 
  i2c_controller {

Documentation/devicetree/bindings/input/cap11xx.txt i2c_controller { · line 64

Quoted 5 lines

Documentation/devicetree/bindings/input/cap11xx.txt +2 -0

@@ -55,5 +60,23 @@ i2c_controller {
  				 <105>,		/* KEY_LEFT */
  				 <109>,		/* KEY_PAGEDOWN */
  				 <104>;		/* KEY_PAGEUP */
+
+		linux,led-brightness = <15>;

You are missing here:

#address-cells = <1>;
#size-cells = <0>;

Otherwise you will get warnings during compilation of dtc file.

Quoted 18 lines

Documentation/devicetree/bindings/input/cap11xx.txt +15 -0

+		usr@0 {
+			label = "cap11xx:green:usr0";
+			reg = <0>;
+		};
+
+		usr@1 {
+			label = "cap11xx:green:usr1";
+			reg = <1>;
+		};
+
+		alive@2 {
+			label = "cap11xx:green:alive";
+			reg = <2>;
+			linux,default_trigger = "heartbeat";
+		};
  	};
  }

Signature · 2 lines

Best Regards,
Jacek Anaszewski

Matt Ranostay

author

2015-06-16 14:31 UTC #3 lore raw

Re: [PATCH v4 1/2] dt: add cap11xx LED documentation

Quoted 37 lines · Jacek Anaszewski

Quoted 27 lines · Matt Ranostay

Signed-off-by Matt Ranostay <[email protected]>

---
  .../devicetree/bindings/input/cap11xx.txt          | 23
++++++++++++++++++++++
  1 file changed, 23 insertions(+)

Documentation/devicetree/bindings/input/cap11xx.txt +1 -0

index 7d0a300..01bb5b8 100644
@@ -38,6 +38,11 @@ Optional properties:
                                defaults. The array must have exactly six
                                entries.
 
+       linux,led-brightness:   Defines the ON brightness when the

optional LED
+ functionality is used. Valid values are
1-15.

Documentation/devicetree/bindings/input/cap11xx.txt +3 -0

+                               By default a value of 15 is set.
+
+
  Example:
 
  i2c_controller {

Documentation/devicetree/bindings/input/cap11xx.txt i2c_controller { · line 64

Quoted 5 lines

Documentation/devicetree/bindings/input/cap11xx.txt +2 -0

@@ -55,5 +60,23 @@ i2c_controller {
                                 <105>,         /* KEY_LEFT */
                                 <109>,         /* KEY_PAGEDOWN */
                                 <104>;         /* KEY_PAGEUP */
+
+               linux,led-brightness = <15>;

You are missing here:

Quoted 4 lines

#address-cells = <1>;
#size-cells = <0>;
Otherwise you will get warnings during compilation of dtc file.

Will fix in v5.

Quoted 25 lines

Quoted 10 lines
Documentation/devicetree/bindings/input/cap11xx.txt +7 -0
+               usr@0 {
+                       label = "cap11xx:green:usr0";
+                       reg = <0>;
+               };
+
+               usr@1 {
+                       label = "cap11xx:green:usr1";
Documentation/devicetree/bindings/input/cap11xx.txt i2c_controller { · line 82
Quoted 10 lines
Documentation/devicetree/bindings/input/cap11xx.txt +8 -0
+                       reg = <1>;
+               };
+
+               alive@2 {
+                       label = "cap11xx:green:alive";
+                       reg = <2>;
+                       linux,default_trigger = "heartbeat";
+               };
        };
  }
--
Best Regards,
Jacek Anaszewski

Matt Ranostay

author

2015-06-16 02:45 UTC #4 lore raw

PATCH v4 2/2 cap11xx: add LED support

In reply to Matt Ranostay

Several cap11xx variants have LEDs that be can be controlled, this patchset implements this functionality.

Signed-off-by Matt Ranostay <[email protected]>

---
 drivers/input/keyboard/cap11xx.c | 128 ++++++++++++++++++++++++++++++++++++++-
 1 file changed, 125 insertions(+), 3 deletions(-)

drivers/input/keyboard/cap11xx.c +125 -3

index f07461a..fc8d365 100644
@@ -12,6 +12,7 @@
 #include <linux/module.h>
 #include <linux/interrupt.h>
 #include <linux/input.h>
+#include <linux/leds.h>
 #include <linux/of_irq.h>
 #include <linux/regmap.h>
 #include <linux/i2c.h>

@@ -47,6 +48,20 @@
 #define CAP11XX_REG_CONFIG2		0x44
 #define CAP11XX_REG_CONFIG2_ALT_POL	BIT(6)
 #define CAP11XX_REG_SENSOR_BASE_CNT(X)	(0x50 + (X))
+#define CAP11XX_REG_LED_POLARITY	0x73
+#define CAP11XX_REG_LED_OUTPUT_CONTROL	0x74
+
+#define CAP11XX_REG_LED_DUTY_CYCLE_1	0x90
+#define CAP11XX_REG_LED_DUTY_CYCLE_2	0x91
+#define CAP11XX_REG_LED_DUTY_CYCLE_3	0x92
+#define CAP11XX_REG_LED_DUTY_CYCLE_4	0x93
+
+#define CAP11XX_REG_LED_DUTY_MIN_MASK	(0x0f)
+#define CAP11XX_REG_LED_DUTY_MIN_MASK_SHIFT	(0)
+#define CAP11XX_REG_LED_DUTY_MAX_MASK	(0xf0)
+#define CAP11XX_REG_LED_DUTY_MAX_MASK_SHIFT	(4)
+#define CAP11XX_REG_LED_DUTY_MAX_VALUE	(15)
+
 #define CAP11XX_REG_SENSOR_CALIB	(0xb1 + (X))
 #define CAP11XX_REG_SENSOR_CALIB_LSB1	0xb9
 #define CAP11XX_REG_SENSOR_CALIB_LSB2	0xba

@@ -56,10 +71,24 @@
 
 #define CAP11XX_MANUFACTURER_ID	0x5d
 
+#ifdef CONFIG_LEDS_CLASS
+struct cap11xx_led {
+	struct cap11xx_priv *priv;
+	struct led_classdev cdev;
+	struct work_struct work;
+	u32 reg;
+	enum led_brightness new_brightness;
+};
+#endif
+
 struct cap11xx_priv {
 	struct regmap *regmap;
 	struct input_dev *idev;
 
+#ifdef CONFIG_LEDS_CLASS
+	struct cap11xx_led *leds;
+	int num_leds;
+#endif
 	/* config */
 	u32 keycodes[];
 };

@@ -67,6 +96,7 @@ struct cap11xx_priv {
 struct cap11xx_hw_model {
 	u8 product_id;
 	unsigned int num_channels;
+	unsigned int num_leds;
 };
 
 enum {

@@ -76,9 +106,9 @@ enum {
 };
 
 static const struct cap11xx_hw_model cap11xx_devices[] = {
-	[CAP1106] = { .product_id = 0x55, .num_channels = 6 },
-	[CAP1126] = { .product_id = 0x53, .num_channels = 6 },
-	[CAP1188] = { .product_id = 0x50, .num_channels = 8 },
+	[CAP1106] = { .product_id = 0x55, .num_channels = 6, .num_leds = 0 },
+	[CAP1126] = { .product_id = 0x53, .num_channels = 6, .num_leds = 2 },
+	[CAP1188] = { .product_id = 0x50, .num_channels = 8, .num_leds = 8 },
 };
 
 static const struct reg_default cap11xx_reg_defaults[] = {

@@ -111,6 +141,7 @@ static const struct reg_default cap11xx_reg_defaults[] = {
 	{ CAP11XX_REG_STANDBY_SENSITIVITY,	0x02 },
 	{ CAP11XX_REG_STANDBY_THRESH,		0x40 },
 	{ CAP11XX_REG_CONFIG2,			0x40 },
+	{ CAP11XX_REG_LED_POLARITY,		0x00 },
 	{ CAP11XX_REG_SENSOR_CALIB_LSB1,	0x00 },
 	{ CAP11XX_REG_SENSOR_CALIB_LSB2,	0x00 },
 };

@@ -196,6 +227,91 @@ static void cap11xx_input_close(struct input_dev *idev)
 	cap11xx_set_sleep(priv, true);
 }
 
+#ifdef CONFIG_LEDS_CLASS
+static void cap11xx_led_work(struct work_struct *work)
+{
+	struct cap11xx_led *led = container_of(work, struct cap11xx_led, work);
+	struct cap11xx_priv *priv = led->priv;
+	int value = led->new_brightness;
+
+	/*
+	 * All LEDs share the same duty cycle as this is a HW limitation.
+	 * Brightness levels per LED are either 0 (OFF) and 1 (ON).
+	 *
+	 * Actual brightness level for the ON state for all LEDS is set via the
+	 * "linux,led-brightness" DT property.
+	 */
+	regmap_update_bits(priv->regmap, CAP11XX_REG_LED_OUTPUT_CONTROL,
+				BIT(led->reg), !!value ? BIT(led->reg) : 0);
+}
+
+static void cap11xx_led_set(struct led_classdev *cdev,
+			   enum led_brightness value)
+{
+	struct cap11xx_led *led = container_of(cdev, struct cap11xx_led, cdev);
+
+	led->new_brightness = value;
+	schedule_work(&led->work);
+}
+
+static int cap11xx_init_leds(struct device *dev, struct cap11xx_priv *priv)
+{
+	struct device_node *node = dev->of_node, *child;
+	struct cap11xx_led *led;
+	int cnt = of_get_child_count(node);
+	u32 reg = 0;
+	int error;
+
+	if (!priv->num_leds || !cnt)
+		return 0;
+	if (cnt > priv->num_leds)
+		return -EINVAL;
+
+	led = devm_kcalloc(dev, cnt,
+		sizeof(struct cap11xx_led), GFP_KERNEL);
+	if (!led)
+		return -ENOMEM;
+
+	priv->leds = led;
+	error = of_property_read_u32(node, "linux,led-brightness", &reg);
+	if (error || reg == 0 || reg > CAP11XX_REG_LED_DUTY_MAX_VALUE)
+		reg = CAP11XX_REG_LED_DUTY_MAX_VALUE;
+
+	error = regmap_update_bits(priv->regmap, CAP11XX_REG_LED_DUTY_CYCLE_4,
+				CAP11XX_REG_LED_DUTY_MAX_MASK,
+				reg << CAP11XX_REG_LED_DUTY_MAX_MASK_SHIFT);
+	if (error)
+		return error;
+
+	for_each_child_of_node(node, child) {
+		led->cdev.name =
+			of_get_property(child, "label", NULL) ? : child->name;
+		led->cdev.default_trigger =
+			of_get_property(child, "linux,default-trigger", NULL);
+		led->cdev.flags = 0;
+		led->cdev.brightness_set = cap11xx_led_set;
+		led->cdev.max_brightness = 1;
+		led->cdev.brightness = LED_OFF;
+
+		error = of_property_read_u32(child, "reg", &reg);
+		if (error != 0 || reg >= priv->num_leds)
+			continue;
+		led->reg = reg;
+		led->priv = priv;
+
+		INIT_WORK(&led->work, cap11xx_led_work);
+		error = devm_led_classdev_register(dev, &led->cdev);
+		if (error < 0)
+			return -EINVAL;
+
+		schedule_work(&led->work);
+		led++;
+	};
+
+	return 0;
+}
+#endif
+
 static int cap11xx_i2c_probe(struct i2c_client *i2c_client,
 			     const struct i2c_device_id *id)
 {

@@ -316,6 +432,12 @@ static int cap11xx_i2c_probe(struct i2c_client *i2c_client,
 	priv->idev->open = cap11xx_input_open;
 	priv->idev->close = cap11xx_input_close;
 
+#ifdef CONFIG_LEDS_CLASS
+	priv->num_leds = cap->num_leds;
+	error = cap11xx_init_leds(dev, priv);
+	if (error)
+		return error;
+#endif
 	input_set_drvdata(priv->idev, priv);
 
 	/*

Signature · 1 line

1.9.1

Jacek Anaszewski

2015-06-16 07:54 UTC #5 lore raw

Re: [PATCH v4 2/2] cap11xx: add LED support

Quoted 105 lines · Matt Ranostay

Several cap11xx variants have LEDs that be can be controlled, this patchset implements this functionality.

Signed-off-by Matt Ranostay <[email protected]>

---
  drivers/input/keyboard/cap11xx.c | 128 ++++++++++++++++++++++++++++++++++++++-
  1 file changed, 125 insertions(+), 3 deletions(-)

drivers/input/keyboard/cap11xx.c +40 -3

index f07461a..fc8d365 100644
@@ -12,6 +12,7 @@
  #include <linux/module.h>
  #include <linux/interrupt.h>
  #include <linux/input.h>
+#include <linux/leds.h>
  #include <linux/of_irq.h>
  #include <linux/regmap.h>
  #include <linux/i2c.h>

@@ -47,6 +48,20 @@
  #define CAP11XX_REG_CONFIG2		0x44
  #define CAP11XX_REG_CONFIG2_ALT_POL	BIT(6)
  #define CAP11XX_REG_SENSOR_BASE_CNT(X)	(0x50 + (X))
+#define CAP11XX_REG_LED_POLARITY	0x73
+#define CAP11XX_REG_LED_OUTPUT_CONTROL	0x74
+
+#define CAP11XX_REG_LED_DUTY_CYCLE_1	0x90
+#define CAP11XX_REG_LED_DUTY_CYCLE_2	0x91
+#define CAP11XX_REG_LED_DUTY_CYCLE_3	0x92
+#define CAP11XX_REG_LED_DUTY_CYCLE_4	0x93
+
+#define CAP11XX_REG_LED_DUTY_MIN_MASK	(0x0f)
+#define CAP11XX_REG_LED_DUTY_MIN_MASK_SHIFT	(0)
+#define CAP11XX_REG_LED_DUTY_MAX_MASK	(0xf0)
+#define CAP11XX_REG_LED_DUTY_MAX_MASK_SHIFT	(4)
+#define CAP11XX_REG_LED_DUTY_MAX_VALUE	(15)
+
  #define CAP11XX_REG_SENSOR_CALIB	(0xb1 + (X))
  #define CAP11XX_REG_SENSOR_CALIB_LSB1	0xb9
  #define CAP11XX_REG_SENSOR_CALIB_LSB2	0xba

@@ -56,10 +71,24 @@
 
  #define CAP11XX_MANUFACTURER_ID	0x5d
 
+#ifdef CONFIG_LEDS_CLASS
+struct cap11xx_led {
+	struct cap11xx_priv *priv;
+	struct led_classdev cdev;
+	struct work_struct work;
+	u32 reg;
+	enum led_brightness new_brightness;
+};
+#endif
+
  struct cap11xx_priv {
  	struct regmap *regmap;
  	struct input_dev *idev;
 
+#ifdef CONFIG_LEDS_CLASS
+	struct cap11xx_led *leds;
+	int num_leds;
+#endif
  	/* config */
  	u32 keycodes[];
  };

@@ -67,6 +96,7 @@ struct cap11xx_priv {
  struct cap11xx_hw_model {
  	u8 product_id;
  	unsigned int num_channels;
+	unsigned int num_leds;
  };
 
  enum {

@@ -76,9 +106,9 @@ enum {
  };
 
  static const struct cap11xx_hw_model cap11xx_devices[] = {
-	[CAP1106] = { .product_id = 0x55, .num_channels = 6 },
-	[CAP1126] = { .product_id = 0x53, .num_channels = 6 },
-	[CAP1188] = { .product_id = 0x50, .num_channels = 8 },
+	[CAP1106] = { .product_id = 0x55, .num_channels = 6, .num_leds = 0 },
+	[CAP1126] = { .product_id = 0x53, .num_channels = 6, .num_leds = 2 },
+	[CAP1188] = { .product_id = 0x50, .num_channels = 8, .num_leds = 8 },
  };
 
  static const struct reg_default cap11xx_reg_defaults[] = {

@@ -111,6 +141,7 @@ static const struct reg_default cap11xx_reg_defaults[] = {
  	{ CAP11XX_REG_STANDBY_SENSITIVITY,	0x02 },
  	{ CAP11XX_REG_STANDBY_THRESH,		0x40 },
  	{ CAP11XX_REG_CONFIG2,			0x40 },
+	{ CAP11XX_REG_LED_POLARITY,		0x00 },
  	{ CAP11XX_REG_SENSOR_CALIB_LSB1,	0x00 },
  	{ CAP11XX_REG_SENSOR_CALIB_LSB2,	0x00 },
  };

@@ -196,6 +227,91 @@ static void cap11xx_input_close(struct input_dev *idev)
  	cap11xx_set_sleep(priv, true);
  }
 
+#ifdef CONFIG_LEDS_CLASS
+static void cap11xx_led_work(struct work_struct *work)
+{
+	struct cap11xx_led *led = container_of(work, struct cap11xx_led, work);
+	struct cap11xx_priv *priv = led->priv;
+	int value = led->new_brightness;

drivers/input/keyboard/cap11xx.c static void cap11xx_input_close(struct input_dev *idev) · line 245

Quoted 10 lines

drivers/input/keyboard/cap11xx.c +10 -0

+
+	/*
+	 * All LEDs share the same duty cycle as this is a HW limitation.
+	 * Brightness levels per LED are either 0 (OFF) and 1 (ON).
+	 *
+	 * Actual brightness level for the ON state for all LEDS is set via the
+	 * "linux,led-brightness" DT property.
+	 */
+	regmap_update_bits(priv->regmap, CAP11XX_REG_LED_OUTPUT_CONTROL,
+				BIT(led->reg), !!value ? BIT(led->reg) : 0);

You don't need '!!' before value. Any non-zero value is evaluated to true in conditions.

Quoted 52 lines

drivers/input/keyboard/cap11xx.c +52 -0

+}
+
+static void cap11xx_led_set(struct led_classdev *cdev,
+			   enum led_brightness value)
+{
+	struct cap11xx_led *led = container_of(cdev, struct cap11xx_led, cdev);
+
+	led->new_brightness = value;
+	schedule_work(&led->work);
+}
+
+static int cap11xx_init_leds(struct device *dev, struct cap11xx_priv *priv)
+{
+	struct device_node *node = dev->of_node, *child;
+	struct cap11xx_led *led;
+	int cnt = of_get_child_count(node);
+	u32 reg = 0;
+	int error;
+
+	if (!priv->num_leds || !cnt)
+		return 0;
+	if (cnt > priv->num_leds)
+		return -EINVAL;
+
+	led = devm_kcalloc(dev, cnt,
+		sizeof(struct cap11xx_led), GFP_KERNEL);
+	if (!led)
+		return -ENOMEM;
+
+	priv->leds = led;
+	error = of_property_read_u32(node, "linux,led-brightness", &reg);
+	if (error || reg == 0 || reg > CAP11XX_REG_LED_DUTY_MAX_VALUE)
+		reg = CAP11XX_REG_LED_DUTY_MAX_VALUE;
+
+	error = regmap_update_bits(priv->regmap, CAP11XX_REG_LED_DUTY_CYCLE_4,
+				CAP11XX_REG_LED_DUTY_MAX_MASK,
+				reg << CAP11XX_REG_LED_DUTY_MAX_MASK_SHIFT);
+	if (error)
+		return error;
+
+	for_each_child_of_node(node, child) {
+		led->cdev.name =
+			of_get_property(child, "label", NULL) ? : child->name;
+		led->cdev.default_trigger =
+			of_get_property(child, "linux,default-trigger", NULL);
+		led->cdev.flags = 0;
+		led->cdev.brightness_set = cap11xx_led_set;
+		led->cdev.max_brightness = 1;
+		led->cdev.brightness = LED_OFF;
+
+		error = of_property_read_u32(child, "reg", &reg);
+		if (error != 0 || reg >= priv->num_leds)

drivers/input/keyboard/cap11xx.c static void cap11xx_input_close(struct input_dev *idev) · line 307

Quoted 10 lines

drivers/input/keyboard/cap11xx.c +10 -0

+			continue;
+		led->reg = reg;
+		led->priv = priv;
+
+		INIT_WORK(&led->work, cap11xx_led_work);
+		error = devm_led_classdev_register(dev, &led->cdev);
+		if (error < 0)
+			return -EINVAL;
+
+		schedule_work(&led->work);

Why do you schedule work here? It should be done only in brightness_set op.

Quoted 24 lines

drivers/input/keyboard/cap11xx.c +13 -0

+		led++;
+	};
+
+	return 0;
+}
+#endif
+
  static int cap11xx_i2c_probe(struct i2c_client *i2c_client,
  			     const struct i2c_device_id *id)
  {

@@ -316,6 +432,12 @@ static int cap11xx_i2c_probe(struct i2c_client *i2c_client,
  	priv->idev->open = cap11xx_input_open;
  	priv->idev->close = cap11xx_input_close;
 
+#ifdef CONFIG_LEDS_CLASS
+	priv->num_leds = cap->num_leds;
+	error = cap11xx_init_leds(dev, priv);
+	if (error)
+		return error;
+#endif
  	input_set_drvdata(priv->idev, priv);
 
  	/*

Signature · 2 lines

Best Regards,
Jacek Anaszewski

Matt Ranostay

author

2015-06-16 14:39 UTC #6 lore raw

Re: [PATCH v4 2/2] cap11xx: add LED support

Quoted 123 lines · Jacek Anaszewski

Quoted 122 lines · Matt Ranostay

Several cap11xx variants have LEDs that be can be controlled, this patchset implements this functionality.

Signed-off-by Matt Ranostay <[email protected]>

---
  drivers/input/keyboard/cap11xx.c | 128
++++++++++++++++++++++++++++++++++++++-
  1 file changed, 125 insertions(+), 3 deletions(-)

drivers/input/keyboard/cap11xx.c +33 -3

index f07461a..fc8d365 100644
@@ -12,6 +12,7 @@
  #include <linux/module.h>
  #include <linux/interrupt.h>
  #include <linux/input.h>
+#include <linux/leds.h>
  #include <linux/of_irq.h>
  #include <linux/regmap.h>
  #include <linux/i2c.h>

@@ -47,6 +48,20 @@
  #define CAP11XX_REG_CONFIG2           0x44
  #define CAP11XX_REG_CONFIG2_ALT_POL   BIT(6)
  #define CAP11XX_REG_SENSOR_BASE_CNT(X)        (0x50 + (X))
+#define CAP11XX_REG_LED_POLARITY       0x73
+#define CAP11XX_REG_LED_OUTPUT_CONTROL 0x74
+
+#define CAP11XX_REG_LED_DUTY_CYCLE_1   0x90
+#define CAP11XX_REG_LED_DUTY_CYCLE_2   0x91
+#define CAP11XX_REG_LED_DUTY_CYCLE_3   0x92
+#define CAP11XX_REG_LED_DUTY_CYCLE_4   0x93
+
+#define CAP11XX_REG_LED_DUTY_MIN_MASK  (0x0f)
+#define CAP11XX_REG_LED_DUTY_MIN_MASK_SHIFT    (0)
+#define CAP11XX_REG_LED_DUTY_MAX_MASK  (0xf0)
+#define CAP11XX_REG_LED_DUTY_MAX_MASK_SHIFT    (4)
+#define CAP11XX_REG_LED_DUTY_MAX_VALUE (15)
+
  #define CAP11XX_REG_SENSOR_CALIB      (0xb1 + (X))
  #define CAP11XX_REG_SENSOR_CALIB_LSB1 0xb9
  #define CAP11XX_REG_SENSOR_CALIB_LSB2 0xba

@@ -56,10 +71,24 @@
 
  #define CAP11XX_MANUFACTURER_ID       0x5d
 
+#ifdef CONFIG_LEDS_CLASS
+struct cap11xx_led {
+       struct cap11xx_priv *priv;
+       struct led_classdev cdev;
+       struct work_struct work;
+       u32 reg;
+       enum led_brightness new_brightness;
+};
+#endif
+
  struct cap11xx_priv {
        struct regmap *regmap;
        struct input_dev *idev;
 
+#ifdef CONFIG_LEDS_CLASS
+       struct cap11xx_led *leds;
+       int num_leds;
+#endif
        /* config */
        u32 keycodes[];
  };

@@ -67,6 +96,7 @@ struct cap11xx_priv {
  struct cap11xx_hw_model {
        u8 product_id;
        unsigned int num_channels;
+       unsigned int num_leds;
  };
 
  enum {

@@ -76,9 +106,9 @@ enum {
  };
 
  static const struct cap11xx_hw_model cap11xx_devices[] = {
-       [CAP1106] = { .product_id = 0x55, .num_channels = 6 },
-       [CAP1126] = { .product_id = 0x53, .num_channels = 6 },
-       [CAP1188] = { .product_id = 0x50, .num_channels = 8 },
+       [CAP1106] = { .product_id = 0x55, .num_channels = 6, .num_leds = 0
 },
+       [CAP1126] = { .product_id = 0x53, .num_channels = 6, .num_leds = 2
 },
+       [CAP1188] = { .product_id = 0x50, .num_channels = 8, .num_leds = 8
 },

  };

  static const struct reg_default cap11xx_reg_defaults[] = {

drivers/input/keyboard/cap11xx.c +1 -0

@@ -111,6 +141,7 @@ static const struct reg_default cap11xx_reg_defaults[]
 = {
        { CAP11XX_REG_STANDBY_SENSITIVITY,      0x02 },
        { CAP11XX_REG_STANDBY_THRESH,           0x40 },
        { CAP11XX_REG_CONFIG2,                  0x40 },
+       { CAP11XX_REG_LED_POLARITY,             0x00 },
        { CAP11XX_REG_SENSOR_CALIB_LSB1,        0x00 },
        { CAP11XX_REG_SENSOR_CALIB_LSB2,        0x00 },

};

drivers/input/keyboard/cap11xx.c +12 -0

@@ -196,6 +227,91 @@ static void cap11xx_input_close(struct input_dev
 *idev)
        cap11xx_set_sleep(priv, true);
  }
 
+#ifdef CONFIG_LEDS_CLASS
+static void cap11xx_led_work(struct work_struct *work)
+{
+       struct cap11xx_led *led = container_of(work, struct cap11xx_led,
 work);
+       struct cap11xx_priv *priv = led->priv;
+       int value = led->new_brightness;
+
+       /*
+        * All LEDs share the same duty cycle as this is a HW limitation.
+        * Brightness levels per LED are either 0 (OFF) and 1 (ON).
+        *
+        * Actual brightness level for the ON state for all LEDS is set

via the
+ * "linux,led-brightness" DT property.

Quoted 9 lines

Quoted 4 lines
drivers/input/keyboard/cap11xx.c +3 -0
+        */
+       regmap_update_bits(priv->regmap, CAP11XX_REG_LED_OUTPUT_CONTROL,
+                               BIT(led->reg), !!value ? BIT(led->reg) :
0);
You don't need '!!' before value. Any non-zero value is evaluated to true in conditions.

Ok old habit of mine. Will remove in v5

Quoted 64 lines

drivers/input/keyboard/cap11xx.c +6 -0

+}
+
+static void cap11xx_led_set(struct led_classdev *cdev,
+                          enum led_brightness value)
+{
+       struct cap11xx_led *led = container_of(cdev, struct cap11xx_led,

cdev);

drivers/input/keyboard/cap11xx.c +6 -0

+
+       led->new_brightness = value;
+       schedule_work(&led->work);
+}
+
+static int cap11xx_init_leds(struct device *dev, struct cap11xx_priv

*priv)

drivers/input/keyboard/cap11xx.c +23 -0

+{
+       struct device_node *node = dev->of_node, *child;
+       struct cap11xx_led *led;
+       int cnt = of_get_child_count(node);
+       u32 reg = 0;
+       int error;
+
+       if (!priv->num_leds || !cnt)
+               return 0;
+       if (cnt > priv->num_leds)
+               return -EINVAL;
+
+       led = devm_kcalloc(dev, cnt,
+               sizeof(struct cap11xx_led), GFP_KERNEL);
+       if (!led)
+               return -ENOMEM;
+
+       priv->leds = led;
+       error = of_property_read_u32(node, "linux,led-brightness", &reg);
+       if (error || reg == 0 || reg > CAP11XX_REG_LED_DUTY_MAX_VALUE)
+               reg = CAP11XX_REG_LED_DUTY_MAX_VALUE;
+
+       error = regmap_update_bits(priv->regmap,

CAP11XX_REG_LED_DUTY_CYCLE_4,

drivers/input/keyboard/cap11xx.c +2 -0

+                               CAP11XX_REG_LED_DUTY_MAX_MASK,
+                               reg <<

CAP11XX_REG_LED_DUTY_MAX_MASK_SHIFT);

drivers/input/keyboard/cap11xx.c +6 -0

+       if (error)
+               return error;
+
+       for_each_child_of_node(node, child) {
+               led->cdev.name =
+                       of_get_property(child, "label", NULL) ? :

child->name;

drivers/input/keyboard/cap11xx.c +2 -0

+               led->cdev.default_trigger =
+                       of_get_property(child, "linux,default-trigger",

NULL);

drivers/input/keyboard/cap11xx.c +12 -0

+               led->cdev.flags = 0;
+               led->cdev.brightness_set = cap11xx_led_set;
+               led->cdev.max_brightness = 1;
+               led->cdev.brightness = LED_OFF;
+
+               error = of_property_read_u32(child, "reg", &reg);
+               if (error != 0 || reg >= priv->num_leds)
+                       continue;
+               led->reg = reg;
+               led->priv = priv;
+
+               INIT_WORK(&led->work, cap11xx_led_work);

Quoted 9 lines

drivers/input/keyboard/cap11xx.c static void cap11xx_input_close(struct input_dev *idev) · line 307
Quoted 5 lines
drivers/input/keyboard/cap11xx.c +5 -0
+               error = devm_led_classdev_register(dev, &led->cdev);
+               if (error < 0)
+                       return -EINVAL;
+
+               schedule_work(&led->work);
Why do you schedule work here? It should be done only in brightness_set op.

The LED needs to be set to initial turned off state, since there is no Power ON/OFF register for the cap11xx devices.. So LEDs state would be persistent across warm reboots.
Thoughts on how to do this better?

Quoted 30 lines

Quoted 23 lines

drivers/input/keyboard/cap11xx.c +13 -0

+               led++;
+       };
+
+       return 0;
+}
+#endif
+
  static int cap11xx_i2c_probe(struct i2c_client *i2c_client,
                             const struct i2c_device_id *id)
  {

@@ -316,6 +432,12 @@ static int cap11xx_i2c_probe(struct i2c_client
 *i2c_client,
        priv->idev->open = cap11xx_input_open;
        priv->idev->close = cap11xx_input_close;
 
+#ifdef CONFIG_LEDS_CLASS
+       priv->num_leds = cap->num_leds;
+       error = cap11xx_init_leds(dev, priv);
+       if (error)
+               return error;
+#endif
        input_set_drvdata(priv->idev, priv);

Quoted 2 lines

/*

--
Best Regards,
Jacek Anaszewski

Daniel Mack

2015-06-16 21:26 UTC #7 lore raw

Re: [PATCH v4 2/2] cap11xx: add LED support

Quoted 32 lines · Matt Ranostay

Quoted 21 lines · Jacek Anaszewski

Quoted 19 lines · Matt Ranostay

drivers/input/keyboard/cap11xx.c +3 -0

+       for_each_child_of_node(node, child) {
+               led->cdev.name =
+                       of_get_property(child, "label", NULL) ? :

child->name;

(quoted diff) +2 -0

+               led->cdev.default_trigger =
+                       of_get_property(child, "linux,default-trigger",

NULL);

drivers/input/keyboard/cap11xx.c +12 -0

+               led->cdev.flags = 0;
+               led->cdev.brightness_set = cap11xx_led_set;
+               led->cdev.max_brightness = 1;
+               led->cdev.brightness = LED_OFF;
+
+               error = of_property_read_u32(child, "reg", &reg);
+               if (error != 0 || reg >= priv->num_leds)
+                       continue;
+               led->reg = reg;
+               led->priv = priv;
+
+               INIT_WORK(&led->work, cap11xx_led_work);

Quoted 9 lines

drivers/input/keyboard/cap11xx.c static void cap11xx_input_close(struct input_dev *idev) · line 307
Quoted 5 lines
drivers/input/keyboard/cap11xx.c +5 -0
+               error = devm_led_classdev_register(dev, &led->cdev);
+               if (error < 0)
+                       return -EINVAL;
+
+               schedule_work(&led->work);
Why do you schedule work here? It should be done only in brightness_set op.

Quoted 4 lines

The LED needs to be set to initial turned off state, since there is no Power ON/OFF register for the cap11xx devices.. So LEDs state would be persistent across warm reboots.
Thoughts on how to do this better?

What's wrong with this?

  regmap_write(priv->regmap, CAP11XX_REG_LED_OUTPUT_CONTROL, 0);

Outside the loop, of course ...

Daniel

Matt Ranostay

author

2015-06-16 21:55 UTC #8 lore raw

Re: [PATCH v4 2/2] cap11xx: add LED support

Quoted 38 lines · Daniel Mack

Quoted 32 lines · Matt Ranostay
Quoted 21 lines · Jacek Anaszewski
Quoted 19 lines · Matt Ranostay
drivers/input/keyboard/cap11xx.c +3 -0
+       for_each_child_of_node(node, child) {
+               led->cdev.name =
+                       of_get_property(child, "label", NULL) ? :
child->name;
(quoted diff) +2 -0
+               led->cdev.default_trigger =
+                       of_get_property(child, "linux,default-trigger",
NULL);
drivers/input/keyboard/cap11xx.c +12 -0
+               led->cdev.flags = 0;
+               led->cdev.brightness_set = cap11xx_led_set;
+               led->cdev.max_brightness = 1;
+               led->cdev.brightness = LED_OFF;
+
+               error = of_property_read_u32(child, "reg", &reg);
+               if (error != 0 || reg >= priv->num_leds)
+                       continue;
+               led->reg = reg;
+               led->priv = priv;
+
+               INIT_WORK(&led->work, cap11xx_led_work);
Quoted 9 lines
drivers/input/keyboard/cap11xx.c static void cap11xx_input_close(struct input_dev *idev) · line 307
Quoted 5 lines
drivers/input/keyboard/cap11xx.c +5 -0
+               error = devm_led_classdev_register(dev, &led->cdev);
+               if (error < 0)
+                       return -EINVAL;
+
+               schedule_work(&led->work);
Why do you schedule work here? It should be done only in brightness_set op.
Quoted 4 lines
The LED needs to be set to initial turned off state, since there is no Power ON/OFF register for the cap11xx devices.. So LEDs state would be persistent across warm reboots.
Thoughts on how to do this better?

Quoted 6 lines

What's wrong with this?
  regmap_write(priv->regmap, CAP11XX_REG_LED_OUTPUT_CONTROL, 0);
Outside the loop, of course ...

Works for me, and prevents the workqueues tasks for blocking each other in regmap_update_bits.

However I just noticed the DLSLEEP state that cap11xx_input_close() requests via cap11xx_set_sleep(true) could disable the LEDs. Need to double check this later tonight...

Quoted 3 lines

Daniel

Matt Ranostay

author

2015-06-17 06:34 UTC #9 lore raw

Re: [PATCH v4 2/2] cap11xx: add LED support

Quoted 43 lines · Matt Ranostay

Quoted 42 lines · Daniel Mack
Quoted 32 lines · Matt Ranostay
Quoted 21 lines · Jacek Anaszewski
Quoted 19 lines · Matt Ranostay
drivers/input/keyboard/cap11xx.c +3 -0
+       for_each_child_of_node(node, child) {
+               led->cdev.name =
+                       of_get_property(child, "label", NULL) ? :
child->name;
(quoted diff) +2 -0
+               led->cdev.default_trigger =
+                       of_get_property(child, "linux,default-trigger",
NULL);
drivers/input/keyboard/cap11xx.c +12 -0
+               led->cdev.flags = 0;
+               led->cdev.brightness_set = cap11xx_led_set;
+               led->cdev.max_brightness = 1;
+               led->cdev.brightness = LED_OFF;
+
+               error = of_property_read_u32(child, "reg", &reg);
+               if (error != 0 || reg >= priv->num_leds)
+                       continue;
+               led->reg = reg;
+               led->priv = priv;
+
+               INIT_WORK(&led->work, cap11xx_led_work);
Quoted 9 lines
drivers/input/keyboard/cap11xx.c static void cap11xx_input_close(struct input_dev *idev) · line 307
Quoted 5 lines
drivers/input/keyboard/cap11xx.c +5 -0
+               error = devm_led_classdev_register(dev, &led->cdev);
+               if (error < 0)
+                       return -EINVAL;
+
+               schedule_work(&led->work);
Why do you schedule work here? It should be done only in brightness_set op.
Quoted 4 lines
The LED needs to be set to initial turned off state, since there is no Power ON/OFF register for the cap11xx devices.. So LEDs state would be persistent across warm reboots.
Thoughts on how to do this better?
What's wrong with this?
  regmap_write(priv->regmap, CAP11XX_REG_LED_OUTPUT_CONTROL, 0);

Quoted 8 lines

Quoted 2 lines
Outside the loop, of course ...
Works for me, and prevents the workqueues tasks for blocking each other in regmap_update_bits.
However I just noticed the DLSLEEP state that cap11xx_input_close() requests via cap11xx_set_sleep(true) could disable the LEDs. Need to double check this later tonight...

Seems deep sleep only effects the breath function. So never mind :)

Quoted 4 lines

Quoted 4 lines
Daniel

A message that is not in the archive (sent off-list or not yet received): CAKzfze8629xyiz1wufjoGbOu+npDRXyCgyGEhxY_1h7XHKeTQg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org

Matt Ranostay

2015-06-17 06:49 UTC #11 lore raw

Re: [PATCH v4 2/2] cap11xx: add LED support

Actually after further reading of the datasheet and testing with i2ctools it seems cap11xx_input_close() isn't putting the device in DEEP SLEEP mode.

Any thoughts why?

Quoted 46 lines · Matt Ranostay

Quoted 45 lines · Matt Ranostay
Quoted 44 lines · Daniel Mack
Quoted 32 lines · Matt Ranostay
Quoted 21 lines · Jacek Anaszewski
Quoted 19 lines · Matt Ranostay
(quoted diff) +3 -0
+       for_each_child_of_node(node, child) {
+               led->cdev.name =
+                       of_get_property(child, "label", NULL) ? :
child->name;
(quoted diff) +2 -0
+               led->cdev.default_trigger =
+                       of_get_property(child, "linux,default-trigger",
NULL);
(quoted diff) +12 -0
+               led->cdev.flags = 0;
+               led->cdev.brightness_set = cap11xx_led_set;
+               led->cdev.max_brightness = 1;
+               led->cdev.brightness = LED_OFF;
+
+               error = of_property_read_u32(child, "reg", &reg);
+               if (error != 0 || reg >= priv->num_leds)
+                       continue;
+               led->reg = reg;
+               led->priv = priv;
+
+               INIT_WORK(&led->work, cap11xx_led_work);
Quoted 9 lines
Review
Quoted 5 lines
(quoted diff) +5 -0
+               error = devm_led_classdev_register(dev, &led->cdev);
+               if (error < 0)
+                       return -EINVAL;
+
+               schedule_work(&led->work);
Why do you schedule work here? It should be done only in brightness_set op.
Quoted 4 lines
The LED needs to be set to initial turned off state, since there is no Power ON/OFF register for the cap11xx devices.. So LEDs state would be persistent across warm reboots.
Thoughts on how to do this better?
What's wrong with this?
  regmap_write(priv->regmap, CAP11XX_REG_LED_OUTPUT_CONTROL, 0);
Outside the loop, of course ...

Quoted 13 lines

Quoted 6 lines
Works for me, and prevents the workqueues tasks for blocking each other in regmap_update_bits.
However I just noticed the DLSLEEP state that cap11xx_input_close() requests via cap11xx_set_sleep(true) could disable the LEDs. Need to double check this later tonight...
Seems deep sleep only effects the breath function. So never mind :)
Quoted 4 lines
Quoted 4 lines
Daniel

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

Daniel Mack

2015-06-17 08:59 UTC #12 lore raw

Re: [PATCH v4 2/2] cap11xx: add LED support

Quoted 5 lines · Matt Ranostay

Actually after further reading of the datasheet and testing with i2ctools it seems cap11xx_input_close() isn't putting the device in DEEP SLEEP mode.
Any thoughts why?

No, sorry. IIRC, the board I wrote the driver for didn't give me the ability to measure power consumption.

Daniel

Matt Ranostay

author

2015-06-17 21:53 UTC #13 lore raw

Re: [PATCH v4 2/2] cap11xx: add LED support

Quoted 10 lines · Daniel Mack

Quoted 5 lines · Matt Ranostay
Actually after further reading of the datasheet and testing with i2ctools it seems cap11xx_input_close() isn't putting the device in DEEP SLEEP mode.
Any thoughts why?
No, sorry. IIRC, the board I wrote the driver for didn't give me the ability to measure power consumption.

I can confirm it never goes to deep sleep or cap11xx_input_close() gets called. Does evdev kernel module increment/input_open_device even if nothing in userspace is accessing the /dev/event/inputX nodes?

- Matt

Quoted 3 lines

Daniel

A message that is not in the archive (sent off-list or not yet received): CAKzfze-H=m55j-1ty4xOkrVzMswU7FpXe=6pHq4ws=P+3vcZNQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org

Dmitry Torokhov

2015-06-17 22:38 UTC #15 lore raw

Re: [PATCH v4 2/2] cap11xx: add LED support

Quoted 8 lines · Matt Ranostay

Quoted 7 lines · Daniel Mack
Quoted 5 lines · Matt Ranostay
Actually after further reading of the datasheet and testing with i2ctools it seems cap11xx_input_close() isn't putting the device in DEEP SLEEP mode.
Any thoughts why?

Quoted 7 lines

Quoted 3 lines
No, sorry. IIRC, the board I wrote the driver for didn't give me the ability to measure power consumption.
I can confirm it never goes to deep sleep or cap11xx_input_close() gets called. Does evdev kernel module increment/input_open_device even if nothing in userspace is accessing the /dev/event/inputX nodes?

No it does not. On the other hand there is VT that does keep all keyboard-like devices open.

Thanks.

Signature · 5 lines

Dmitry
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Matt Ranostay

author

2015-06-17 23:36 UTC #16 lore raw

Re: [PATCH v4 2/2] cap11xx: add LED support

Quoted 13 lines · Dmitry Torokhov

Quoted 12 lines · Matt Ranostay
Quoted 10 lines · Daniel Mack
Quoted 5 lines · Matt Ranostay
Actually after further reading of the datasheet and testing with i2ctools it seems cap11xx_input_close() isn't putting the device in DEEP SLEEP mode.
Any thoughts why?
No, sorry. IIRC, the board I wrote the driver for didn't give me the ability to measure power consumption.

Quoted 6 lines

Quoted 3 lines
I can confirm it never goes to deep sleep or cap11xx_input_close() gets called. Does evdev kernel module increment/input_open_device even if nothing in userspace is accessing the /dev/event/inputX nodes?
No it does not. On the other hand there is VT that does keep all keyboard-like devices open.

Ah yes is that is likely the issue. I'll disable CONFIG_VT, and update in v5 to handle to keep the device from going to deep sleep when one or more LEDS is on.

- Matt

Quoted 5 lines

Thanks.
--
Dmitry