[PATCH 0/3] cap1106: add support for cap11xx variants

These patches enable using cap11xx devices that have different
number of capacitance channels, and using active-high on the
interrupt output.

Matt Ranostay (3):
  cap1106: Add support for various cap11xx devices
  cap1106: support for active-high interrupt option
  dt: cap1106 active-high property addition

 .../devicetree/bindings/input/cap1106.txt          |  4 ++
 drivers/input/keyboard/cap1106.c                   | 58 ++++++++++++----------
 2 files changed, 35 insertions(+), 27 deletions(-)

-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Several other variants of the cap11xx device exists with a varying
number of capacitance detection channels. Add support for creating
the channels dynamically.

Signed-off-by: Matt Ranostay <redacted>
---
 drivers/input/keyboard/cap1106.c | 54 +++++++++++++++++++---------------------
 1 file changed, 26 insertions(+), 28 deletions(-)

diff --git a/drivers/input/keyboard/cap1106.c b/drivers/input/keyboard/cap1106.c
index d70b65a..b9c43b5 100644
--- a/drivers/input/keyboard/cap1106.c
+++ b/drivers/input/keyboard/cap1106.c

@@ -55,8 +55,6 @@
 #define CAP1106_REG_MANUFACTURER_ID	0xfe
 #define CAP1106_REG_REVISION		0xff
 
-#define CAP1106_NUM_CHN 6
-#define CAP1106_PRODUCT_ID	0x55
 #define CAP1106_MANUFACTURER_ID	0x5d
 
 struct cap1106_priv {

@@ -64,7 +62,8 @@ struct cap1106_priv {
 	struct input_dev *idev;
 
 	/* config */
-	unsigned short keycodes[CAP1106_NUM_CHN];
+	u32 *keycodes;
+	unsigned int num_channels;
 };
 
 static const struct reg_default cap1106_reg_defaults[] = {

@@ -151,7 +150,7 @@ static irqreturn_t cap1106_thread_func(int irq_num, void *data)
 	if (ret < 0)
 		goto out;
 
-	for (i = 0; i < CAP1106_NUM_CHN; i++)
+	for (i = 0; i < priv->num_channels; i++)
 		input_report_key(priv->idev, priv->keycodes[i],
 				 status & (1 << i));
 

@@ -189,27 +188,23 @@ static int cap1106_i2c_probe(struct i2c_client *i2c_client,
 	struct cap1106_priv *priv;
 	struct device_node *node;
 	int i, error, irq, gain = 0;
-	unsigned int val, rev;
-	u32 gain32, keycodes[CAP1106_NUM_CHN];
+	unsigned int val, prod, rev;
+	u32 gain32;
 
 	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
 	if (!priv)
 		return -ENOMEM;
 
+	priv->num_channels = (unsigned long) id->driver_data;
+	priv->keycodes = devm_kzalloc(dev,
+		sizeof(u32) * priv->num_channels, GFP_KERNEL);
+	if (!priv->keycodes)
+		return -ENOMEM;
+
 	priv->regmap = devm_regmap_init_i2c(i2c_client, &cap1106_regmap_config);
 	if (IS_ERR(priv->regmap))
 		return PTR_ERR(priv->regmap);
 
-	error = regmap_read(priv->regmap, CAP1106_REG_PRODUCT_ID, &val);
-	if (error)
-		return error;
-
-	if (val != CAP1106_PRODUCT_ID) {
-		dev_err(dev, "Product ID: Got 0x%02x, expected 0x%02x\n",
-			val, CAP1106_PRODUCT_ID);
-		return -ENODEV;
-	}
-
 	error = regmap_read(priv->regmap, CAP1106_REG_MANUFACTURER_ID, &val);
 	if (error)
 		return error;

@@ -220,6 +215,10 @@ static int cap1106_i2c_probe(struct i2c_client *i2c_client,
 		return -ENODEV;
 	}
 
+	error = regmap_read(priv->regmap, CAP1106_REG_PRODUCT_ID, &prod);
+	if (error < 0)
+		return error;
+
 	error = regmap_read(priv->regmap, CAP1106_REG_REVISION, &rev);
 	if (error < 0)
 		return error;

@@ -235,17 +234,12 @@ static int cap1106_i2c_probe(struct i2c_client *i2c_client,
 			dev_err(dev, "Invalid sensor-gain value %d\n", gain32);
 	}
 
-	BUILD_BUG_ON(ARRAY_SIZE(keycodes) != ARRAY_SIZE(priv->keycodes));
-
 	/* Provide some useful defaults */
-	for (i = 0; i < ARRAY_SIZE(keycodes); i++)
-		keycodes[i] = KEY_A + i;
+	for (i = 0; i < priv->num_channels; i++)
+		priv->keycodes[i] = KEY_A + i;
 
 	of_property_read_u32_array(node, "linux,keycodes",
-				   keycodes, ARRAY_SIZE(keycodes));
-
-	for (i = 0; i < ARRAY_SIZE(keycodes); i++)
-		priv->keycodes[i] = keycodes[i];
+				  priv->keycodes, priv->num_channels);
 
 	error = regmap_update_bits(priv->regmap, CAP1106_REG_MAIN_CONTROL,
 				   CAP1106_REG_MAIN_CONTROL_GAIN_MASK,

@@ -269,17 +263,17 @@ static int cap1106_i2c_probe(struct i2c_client *i2c_client,
 	if (of_property_read_bool(node, "autorepeat"))
 		__set_bit(EV_REP, priv->idev->evbit);
 
-	for (i = 0; i < CAP1106_NUM_CHN; i++)
+	for (i = 0; i < priv->num_channels; i++)
 		__set_bit(priv->keycodes[i], priv->idev->keybit);
 
 	__clear_bit(KEY_RESERVED, priv->idev->keybit);
 
 	priv->idev->keycode = priv->keycodes;
 	priv->idev->keycodesize = sizeof(priv->keycodes[0]);
-	priv->idev->keycodemax = ARRAY_SIZE(priv->keycodes);
+	priv->idev->keycodemax = priv->num_channels;
 
 	priv->idev->id.vendor = CAP1106_MANUFACTURER_ID;
-	priv->idev->id.product = CAP1106_PRODUCT_ID;
+	priv->idev->id.product = prod;
 	priv->idev->id.version = rev;
 
 	priv->idev->open = cap1106_input_open;

@@ -313,12 +307,16 @@ static int cap1106_i2c_probe(struct i2c_client *i2c_client,
 
 static const struct of_device_id cap1106_dt_ids[] = {
 	{ .compatible = "microchip,cap1106", },
+	{ .compatible = "microchip,cap1126", },
+	{ .compatible = "microchip,cap1188", },
 	{}
 };
 MODULE_DEVICE_TABLE(of, cap1106_dt_ids);
 
 static const struct i2c_device_id cap1106_i2c_ids[] = {
-	{ "cap1106", 0 },
+	{ "cap1106", 6 },
+	{ "cap1126", 6 },
+	{ "cap1188", 8 },
 	{}
 };
 MODULE_DEVICE_TABLE(i2c, cap1106_i2c_ids);

-- 
1.9.1

Some applications need to use the active-high push-pull interrupt
option. This allows it be enabled in the device tree child node.

Signed-off-by: Matt Ranostay <redacted>
---
 drivers/input/keyboard/cap1106.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/input/keyboard/cap1106.c b/drivers/input/keyboard/cap1106.c
index b9c43b5..33e2590 100644
--- a/drivers/input/keyboard/cap1106.c
+++ b/drivers/input/keyboard/cap1106.c

@@ -234,6 +234,12 @@ static int cap1106_i2c_probe(struct i2c_client *i2c_client,
 			dev_err(dev, "Invalid sensor-gain value %d\n", gain32);
 	}
 
+	if (of_property_read_bool(node, "microchip,active-high")) {
+		error = regmap_write(priv->regmap, CAP1106_REG_CONFIG2, 0);
+		if (error)
+			return error;
+	}
+
 	/* Provide some useful defaults */
 	for (i = 0; i < priv->num_channels; i++)
 		priv->keycodes[i] = KEY_A + i;

-- 
1.9.1

Documenting the microchip,active-high property for interrupt pin
behavior.

Signed-off-by: Matt Ranostay <redacted>
---
 Documentation/devicetree/bindings/input/cap1106.txt | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/input/cap1106.txt b/Documentation/devicetree/bindings/input/cap1106.txt
index 4b46390..7e64fd6 100644
--- a/Documentation/devicetree/bindings/input/cap1106.txt
+++ b/Documentation/devicetree/bindings/input/cap1106.txt

@@ -26,6 +26,10 @@ Optional properties:
 				Valid values are 1, 2, 4, and 8.
 				By default, a gain of 1 is set.
 
+	microchip,active-high:	By default the interrupt pin  is active low open
+				drain. This property allows using the active high
+				push-pull output.
+
 	linux,keycodes:		Specifies an array of numeric keycode values to
 				be used for the channels. If this property is
 				omitted, KEY_A, KEY_B, etc are used as

-- 
1.9.1

Hi,

On 09/21/2014 05:01 AM, Matt Ranostay wrote:

Several other variants of the cap11xx device exists with a varying
number of capacitance detection channels. Add support for creating
the channels dynamically.

Thanks for the patches!

Signed-off-by: Matt Ranostay <redacted>
---
 drivers/input/keyboard/cap1106.c | 54 +++++++++++++++++++---------------------

Please also add a patch to rename the file to cap11xx.c, and make sure
to export the patch with 'git format-patch -M' to detect the rename.

unsigned short *, please. See below.

priv->num_channels is unsigned int.

Also, a BUG_ON(!priv->num_channels) wouldn't harm.

+	priv->keycodes = devm_kzalloc(dev,
+		sizeof(u32) * priv->num_channels, GFP_KERNEL);

Use devm_kcalloc() to allocate an array.

Hmm, no. Internally, you have to store the keycodes as unsigned short,
otherwise EVIOC{G|S}KEYCODE from usespace doesn't work.

of_property_read_u16_array() should work here for unsigned short, I
guess. Otherwise, you'd have to open-code the routine.

Hmm, how can that work unless you set .data to the number of channels
here? Did you test that with a DT-enabled board?


Thanks,
Daniel

On 09/21/2014 05:01 AM, Matt Ranostay wrote:

I think the name of that property should make clear it's only changing
the interrupt output driver configuration. What about
"microchip,irq-active-high"?

Also, patch 3/3, which documents this new property, can be squashed into
this one.

+		error = regmap_write(priv->regmap, CAP1106_REG_CONFIG2, 0);

This register controls a lot more details than that. Overriding it with
0 doesn't seem right. Please use regmap_update_bits() to just update
ALT_POL, and also add a #define for it, so the next reader knows what
the code is doing :)


Thanks,
Daniel

Hi,

On 09/21/2014 05:01 AM, Matt Ranostay wrote:

 	priv->regmap = devm_regmap_init_i2c(i2c_client, &cap1106_regmap_config);
 	if (IS_ERR(priv->regmap))
 		return PTR_ERR(priv->regmap);
 
-	error = regmap_read(priv->regmap, CAP1106_REG_PRODUCT_ID, &val);
-	if (error)
-		return error;
-
-	if (val != CAP1106_PRODUCT_ID) {
-		dev_err(dev, "Product ID: Got 0x%02x, expected 0x%02x\n",
-			val, CAP1106_PRODUCT_ID);
-		return -ENODEV;
-	}
-

Btw - the purpose of this code was to detect board configuration
mismatch. After all, I2C lacks a way to properly identify peripherals,
so the more runtime checks we do at probe time, the more of a chance we
have to detect wrong setups. This device is actually well implemented
and tells us something about itself.

Hence, I'd propose to define a structure like this:

struct cap11xx_hw_model {
	uint8_t product_id;
	unsigned int num_channels;
};

... and attach instances of that to the members of cap1106_dt_ids[] and
cap1106_i2c_ids[]. In the probe function, check that the contents of
CAP1106_PRODUCT_ID match what is expected by the configured model.


Thanks,
Daniel

On Sun, Sep 21, 2014 at 2:58 AM, Daniel Mack [off-list ref] wrote:

Hi,

On 09/21/2014 05:01 AM, Matt Ranostay wrote:

quoted

Several other variants of the cap11xx device exists with a varying
number of capacitance detection channels. Add support for creating
the channels dynamically.

Thanks for the patches!

quoted

Signed-off-by: Matt Ranostay <redacted>
---
 drivers/input/keyboard/cap1106.c | 54 +++++++++++++++++++---------------------

Please also add a patch to rename the file to cap11xx.c, and make sure
to export the patch with 'git format-patch -M' to detect the rename.

quoted

diff --git a/drivers/input/keyboard/cap1106.c b/drivers/input/keyboard/cap1106.c
index d70b65a..b9c43b5 100644
--- a/drivers/input/keyboard/cap1106.c
+++ b/drivers/input/keyboard/cap1106.c

@@ -55,8 +55,6 @@
 #define CAP1106_REG_MANUFACTURER_ID  0xfe
 #define CAP1106_REG_REVISION         0xff

-#define CAP1106_NUM_CHN 6
-#define CAP1106_PRODUCT_ID   0x55
 #define CAP1106_MANUFACTURER_ID      0x5d

 struct cap1106_priv {

@@ -64,7 +62,8 @@ struct cap1106_priv {
      struct input_dev *idev;

      /* config */
-     unsigned short keycodes[CAP1106_NUM_CHN];
+     u32 *keycodes;

unsigned short *, please. See below.

quoted

@@ -189,27 +188,23 @@ static int cap1106_i2c_probe(struct i2c_client *i2c_client,
      struct cap1106_priv *priv;
      struct device_node *node;
      int i, error, irq, gain = 0;
-     unsigned int val, rev;
-     u32 gain32, keycodes[CAP1106_NUM_CHN];
+     unsigned int val, prod, rev;
+     u32 gain32;

      priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
      if (!priv)
              return -ENOMEM;

+     priv->num_channels = (unsigned long) id->driver_data;

priv->num_channels is unsigned int.

Also, a BUG_ON(!priv->num_channels) wouldn't harm.

Oops. Will fix..

quoted

+     priv->keycodes = devm_kzalloc(dev,
+             sizeof(u32) * priv->num_channels, GFP_KERNEL);

Use devm_kcalloc() to allocate an array.

quoted

@@ -235,17 +234,12 @@ static int cap1106_i2c_probe(struct i2c_client *i2c_client,
                      dev_err(dev, "Invalid sensor-gain value %d\n", gain32);
      }

-     BUILD_BUG_ON(ARRAY_SIZE(keycodes) != ARRAY_SIZE(priv->keycodes));
-
      /* Provide some useful defaults */
-     for (i = 0; i < ARRAY_SIZE(keycodes); i++)
-             keycodes[i] = KEY_A + i;
+     for (i = 0; i < priv->num_channels; i++)
+             priv->keycodes[i] = KEY_A + i;

      of_property_read_u32_array(node, "linux,keycodes",
-                                keycodes, ARRAY_SIZE(keycodes));
-
-     for (i = 0; i < ARRAY_SIZE(keycodes); i++)
-             priv->keycodes[i] = keycodes[i];
+                               priv->keycodes, priv->num_channels);

Hmm, no. Internally, you have to store the keycodes as unsigned short,
otherwise EVIOC{G|S}KEYCODE from usespace doesn't work.

of_property_read_u16_array() should work here for unsigned short, I
guess. Otherwise, you'd have to open-code the routine.

Ok I'l test that.

quoted

@@ -313,12 +307,16 @@ static int cap1106_i2c_probe(struct i2c_client *i2c_client,

 static const struct of_device_id cap1106_dt_ids[] = {
      { .compatible = "microchip,cap1106", },
+     { .compatible = "microchip,cap1126", },
+     { .compatible = "microchip,cap1188", },

Hmm, how can that work unless you set .data to the number of channels
here? Did you test that with a DT-enabled board?

Yes it was tested on a BBB.  The num_channels is set from cap1106_i2c_ids

Thanks,
Daniel

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

On Sun, Sep 21, 2014 at 2:58 AM, Daniel Mack [off-list ref] wrote:

Hi,

On 09/21/2014 05:01 AM, Matt Ranostay wrote:

quoted

Several other variants of the cap11xx device exists with a varying
number of capacitance detection channels. Add support for creating
the channels dynamically.

Thanks for the patches!

quoted

Signed-off-by: Matt Ranostay <redacted>
---
 drivers/input/keyboard/cap1106.c | 54 +++++++++++++++++++---------------------

Please also add a patch to rename the file to cap11xx.c, and make sure
to export the patch with 'git format-patch -M' to detect the rename.

quoted

diff --git a/drivers/input/keyboard/cap1106.c b/drivers/input/keyboard/cap1106.c
index d70b65a..b9c43b5 100644
--- a/drivers/input/keyboard/cap1106.c
+++ b/drivers/input/keyboard/cap1106.c

@@ -55,8 +55,6 @@
 #define CAP1106_REG_MANUFACTURER_ID  0xfe
 #define CAP1106_REG_REVISION         0xff

-#define CAP1106_NUM_CHN 6
-#define CAP1106_PRODUCT_ID   0x55
 #define CAP1106_MANUFACTURER_ID      0x5d

 struct cap1106_priv {

@@ -64,7 +62,8 @@ struct cap1106_priv {
      struct input_dev *idev;

      /* config */
-     unsigned short keycodes[CAP1106_NUM_CHN];
+     u32 *keycodes;

unsigned short *, please. See below.

quoted

@@ -189,27 +188,23 @@ static int cap1106_i2c_probe(struct i2c_client *i2c_client,
      struct cap1106_priv *priv;
      struct device_node *node;
      int i, error, irq, gain = 0;
-     unsigned int val, rev;
-     u32 gain32, keycodes[CAP1106_NUM_CHN];
+     unsigned int val, prod, rev;
+     u32 gain32;

      priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
      if (!priv)
              return -ENOMEM;

+     priv->num_channels = (unsigned long) id->driver_data;

priv->num_channels is unsigned int.

Also, a BUG_ON(!priv->num_channels) wouldn't harm.

quoted

+     priv->keycodes = devm_kzalloc(dev,
+             sizeof(u32) * priv->num_channels, GFP_KERNEL);

Use devm_kcalloc() to allocate an array.

quoted

@@ -235,17 +234,12 @@ static int cap1106_i2c_probe(struct i2c_client *i2c_client,
                      dev_err(dev, "Invalid sensor-gain value %d\n", gain32);
      }

-     BUILD_BUG_ON(ARRAY_SIZE(keycodes) != ARRAY_SIZE(priv->keycodes));
-
      /* Provide some useful defaults */
-     for (i = 0; i < ARRAY_SIZE(keycodes); i++)
-             keycodes[i] = KEY_A + i;
+     for (i = 0; i < priv->num_channels; i++)
+             priv->keycodes[i] = KEY_A + i;

      of_property_read_u32_array(node, "linux,keycodes",
-                                keycodes, ARRAY_SIZE(keycodes));
-
-     for (i = 0; i < ARRAY_SIZE(keycodes); i++)
-             priv->keycodes[i] = keycodes[i];
+                               priv->keycodes, priv->num_channels);

Hmm, no. Internally, you have to store the keycodes as unsigned short,
otherwise EVIOC{G|S}KEYCODE from usespace doesn't work.

of_property_read_u16_array() should work here for unsigned short, I
guess. Otherwise, you'd have to open-code the routine.

Problem with u16 is you'll to mark it */bits/ 16* in the device tree
entry.. I doubt that is acceptable

quoted

@@ -313,12 +307,16 @@ static int cap1106_i2c_probe(struct i2c_client *i2c_client,

 static const struct of_device_id cap1106_dt_ids[] = {
      { .compatible = "microchip,cap1106", },
+     { .compatible = "microchip,cap1126", },
+     { .compatible = "microchip,cap1188", },

Hmm, how can that work unless you set .data to the number of channels
here? Did you test that with a DT-enabled board?


Thanks,
Daniel

On Sun, Sep 21, 2014 at 12:06:30PM +0200, Daniel Mack wrote:

On 09/21/2014 05:01 AM, Matt Ranostay wrote:

quoted

Some applications need to use the active-high push-pull interrupt
option. This allows it be enabled in the device tree child node.

Signed-off-by: Matt Ranostay <redacted>
---
 drivers/input/keyboard/cap1106.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/input/keyboard/cap1106.c b/drivers/input/keyboard/cap1106.c
index b9c43b5..33e2590 100644
--- a/drivers/input/keyboard/cap1106.c
+++ b/drivers/input/keyboard/cap1106.c

@@ -234,6 +234,12 @@ static int cap1106_i2c_probe(struct i2c_client *i2c_client,
 			dev_err(dev, "Invalid sensor-gain value %d\n", gain32);
 	}
 
+	if (of_property_read_bool(node, "microchip,active-high")) {

I think the name of that property should make clear it's only changing
the interrupt output driver configuration. What about
"microchip,irq-active-high"?

Can we infer the setting from IRQ flags by chance?

Thanks.

-- 
Dmitry

On Sun, Sep 21, 2014 at 05:28:05PM -0700, Matt Ranostay wrote:

On Sun, Sep 21, 2014 at 2:58 AM, Daniel Mack [off-list ref] wrote:

quoted

Hi,

On 09/21/2014 05:01 AM, Matt Ranostay wrote:

quoted

Several other variants of the cap11xx device exists with a varying
number of capacitance detection channels. Add support for creating
the channels dynamically.

Thanks for the patches!

quoted

Signed-off-by: Matt Ranostay <redacted>
---
 drivers/input/keyboard/cap1106.c | 54 +++++++++++++++++++---------------------

Please also add a patch to rename the file to cap11xx.c, and make sure
to export the patch with 'git format-patch -M' to detect the rename.

quoted

diff --git a/drivers/input/keyboard/cap1106.c b/drivers/input/keyboard/cap1106.c
index d70b65a..b9c43b5 100644
--- a/drivers/input/keyboard/cap1106.c
+++ b/drivers/input/keyboard/cap1106.c

@@ -55,8 +55,6 @@
 #define CAP1106_REG_MANUFACTURER_ID  0xfe
 #define CAP1106_REG_REVISION         0xff

-#define CAP1106_NUM_CHN 6
-#define CAP1106_PRODUCT_ID   0x55
 #define CAP1106_MANUFACTURER_ID      0x5d

 struct cap1106_priv {

@@ -64,7 +62,8 @@ struct cap1106_priv {
      struct input_dev *idev;

      /* config */
-     unsigned short keycodes[CAP1106_NUM_CHN];
+     u32 *keycodes;

unsigned short *, please. See below.

quoted

@@ -189,27 +188,23 @@ static int cap1106_i2c_probe(struct i2c_client *i2c_client,
      struct cap1106_priv *priv;
      struct device_node *node;
      int i, error, irq, gain = 0;
-     unsigned int val, rev;
-     u32 gain32, keycodes[CAP1106_NUM_CHN];
+     unsigned int val, prod, rev;
+     u32 gain32;

      priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
      if (!priv)
              return -ENOMEM;

+     priv->num_channels = (unsigned long) id->driver_data;

priv->num_channels is unsigned int.

Also, a BUG_ON(!priv->num_channels) wouldn't harm.

quoted

+     priv->keycodes = devm_kzalloc(dev,
+             sizeof(u32) * priv->num_channels, GFP_KERNEL);

You do not need to allocate keycodes separately if you make a flexible length
array at the end of the main structure.

quoted

Use devm_kcalloc() to allocate an array.

quoted

@@ -235,17 +234,12 @@ static int cap1106_i2c_probe(struct i2c_client *i2c_client,
                      dev_err(dev, "Invalid sensor-gain value %d\n", gain32);
      }

-     BUILD_BUG_ON(ARRAY_SIZE(keycodes) != ARRAY_SIZE(priv->keycodes));
-
      /* Provide some useful defaults */
-     for (i = 0; i < ARRAY_SIZE(keycodes); i++)
-             keycodes[i] = KEY_A + i;
+     for (i = 0; i < priv->num_channels; i++)
+             priv->keycodes[i] = KEY_A + i;

      of_property_read_u32_array(node, "linux,keycodes",
-                                keycodes, ARRAY_SIZE(keycodes));
-
-     for (i = 0; i < ARRAY_SIZE(keycodes); i++)
-             priv->keycodes[i] = keycodes[i];
+                               priv->keycodes, priv->num_channels);

Hmm, no. Internally, you have to store the keycodes as unsigned short,
otherwise EVIOC{G|S}KEYCODE from usespace doesn't work.

of_property_read_u16_array() should work here for unsigned short, I
guess. Otherwise, you'd have to open-code the routine.

Problem with u16 is you'll to mark it */bits/ 16* in the device tree
entry.. I doubt that is acceptable

You can make keymap u32. As long as you set input->keycodesize appropriately
(and we do) everything should work just fine.

Thanks.

-- 
Dmitry

On 09/22/2014 12:46 AM, Matt Ranostay wrote:

On Sun, Sep 21, 2014 at 2:58 AM, Daniel Mack [off-list ref] wrote:

quoted

On 09/21/2014 05:01 AM, Matt Ranostay wrote:

quoted

quoted

@@ -313,12 +307,16 @@ static int cap1106_i2c_probe(struct i2c_client *i2c_client,

 static const struct of_device_id cap1106_dt_ids[] = {
      { .compatible = "microchip,cap1106", },
+     { .compatible = "microchip,cap1126", },
+     { .compatible = "microchip,cap1188", },

Hmm, how can that work unless you set .data to the number of channels
here? Did you test that with a DT-enabled board?

Yes it was tested on a BBB.  The num_channels is set from cap1106_i2c_ids

Ah ok. I forgot there's this fallback to the i2c ids. What others driver
do is to use of_match_device() in the probe function, and then access
->data of the returned match.

But I'm fine with falling back to cap1106_i2c_ids unless anyone else has
objections.


Thanks,
Daniel

On 09/22/2014 07:56 AM, Dmitry Torokhov wrote:

On Sun, Sep 21, 2014 at 12:06:30PM +0200, Daniel Mack wrote:

quoted

On 09/21/2014 05:01 AM, Matt Ranostay wrote:

quoted

Some applications need to use the active-high push-pull interrupt
option. This allows it be enabled in the device tree child node.

Signed-off-by: Matt Ranostay <redacted>
---
 drivers/input/keyboard/cap1106.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/input/keyboard/cap1106.c b/drivers/input/keyboard/cap1106.c
index b9c43b5..33e2590 100644
--- a/drivers/input/keyboard/cap1106.c
+++ b/drivers/input/keyboard/cap1106.c

@@ -234,6 +234,12 @@ static int cap1106_i2c_probe(struct i2c_client *i2c_client,
 			dev_err(dev, "Invalid sensor-gain value %d\n", gain32);
 	}
 
+	if (of_property_read_bool(node, "microchip,active-high")) {

I think the name of that property should make clear it's only changing
the interrupt output driver configuration. What about
"microchip,irq-active-high"?

Can we infer the setting from IRQ flags by chance?

Hmm, I thought of that as well, but there could be electrical wiring
setups that want the CPU's hardware pin in push/pull mode but the one on
the sensor chip in open-drain. I'd rather not make the assuption the
pins are directly connected and have both sides individually configurable.


Thanks,
Daniel