Thread (13 messages) 13 messages, 3 authors, 2014-09-22

Re: [PATCH 1/3] cap1106: Add support for various cap11xx devices

From: Matt Ranostay <hidden>
Date: 2014-09-21 22:46:51
Also in: linux-devicetree, lkml

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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help