On Sun, Sep 21, 2014 at 05:28:05PM -0700, Matt Ranostay wrote:
> On Sun, Sep 21, 2014 at 2:58 AM, Daniel Mack <daniel@xxxxxxxxxx> wrote:
> > 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 <mranostay@xxxxxxxxx>
> >> ---
> >> 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.
> >
> >> 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.
> >
> >> @@ -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.
> >
> >> + 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.
> >
> > Use devm_kcalloc() to allocate an array.
> >
> >> @@ -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
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
