* [PATCH 0/3] cap1106: add support for cap11xx variants
@ 2014-09-21 3:01 Matt Ranostay
2014-09-21 3:01 ` [PATCH 1/3] cap1106: Add support for various cap11xx devices Matt Ranostay
` (2 more replies)
0 siblings, 3 replies; 13+ messages in thread
From: Matt Ranostay @ 2014-09-21 3:01 UTC (permalink / raw)
To: galak, linux-input, linux-kernel, dmitry.torokhov, zonque, robh+dt
Cc: devicetree, Matt Ranostay
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
^ permalink raw reply [flat|nested] 13+ messages in thread* [PATCH 1/3] cap1106: Add support for various cap11xx devices 2014-09-21 3:01 [PATCH 0/3] cap1106: add support for cap11xx variants Matt Ranostay @ 2014-09-21 3:01 ` Matt Ranostay 2014-09-21 9:58 ` Daniel Mack 2014-09-21 10:53 ` Daniel Mack 2014-09-21 3:01 ` [PATCH 2/3] cap1106: support for active-high interrupt option Matt Ranostay 2014-09-21 3:01 ` [PATCH 3/3] dt: cap1106 active-high property addition Matt Ranostay 2 siblings, 2 replies; 13+ messages in thread From: Matt Ranostay @ 2014-09-21 3:01 UTC (permalink / raw) To: galak, linux-input, linux-kernel, dmitry.torokhov, zonque, robh+dt Cc: devicetree, Matt Ranostay 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 <mranostay@gmail.com> --- 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 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 1/3] cap1106: Add support for various cap11xx devices 2014-09-21 3:01 ` [PATCH 1/3] cap1106: Add support for various cap11xx devices Matt Ranostay @ 2014-09-21 9:58 ` Daniel Mack 2014-09-21 22:46 ` Matt Ranostay 2014-09-22 0:28 ` Matt Ranostay 2014-09-21 10:53 ` Daniel Mack 1 sibling, 2 replies; 13+ messages in thread From: Daniel Mack @ 2014-09-21 9:58 UTC (permalink / raw) To: Matt Ranostay, galak, linux-input, linux-kernel, dmitry.torokhov, robh+dt Cc: devicetree 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@gmail.com> > --- > 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); 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. > @@ -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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/3] cap1106: Add support for various cap11xx devices 2014-09-21 9:58 ` Daniel Mack @ 2014-09-21 22:46 ` Matt Ranostay 2014-09-22 7:36 ` Daniel Mack 2014-09-22 0:28 ` Matt Ranostay 1 sibling, 1 reply; 13+ messages in thread From: Matt Ranostay @ 2014-09-21 22:46 UTC (permalink / raw) To: Daniel Mack Cc: Kumar Gala, linux-input, linux-kernel, Dmitry Torokhov, Rob Herring, devicetree On Sun, Sep 21, 2014 at 2:58 AM, Daniel Mack <daniel@zonque.org> 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@gmail.com> > > --- > > 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. Oops. Will fix.. > > > + priv->keycodes = devm_kzalloc(dev, > > + sizeof(u32) * priv->num_channels, GFP_KERNEL); > > 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. > Ok I'l test that. > > @@ -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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/3] cap1106: Add support for various cap11xx devices 2014-09-21 22:46 ` Matt Ranostay @ 2014-09-22 7:36 ` Daniel Mack 0 siblings, 0 replies; 13+ messages in thread From: Daniel Mack @ 2014-09-22 7:36 UTC (permalink / raw) To: Matt Ranostay Cc: Kumar Gala, linux-input, linux-kernel, Dmitry Torokhov, Rob Herring, devicetree On 09/22/2014 12:46 AM, Matt Ranostay wrote: > On Sun, Sep 21, 2014 at 2:58 AM, Daniel Mack <daniel@zonque.org> wrote: >> On 09/21/2014 05:01 AM, Matt Ranostay wrote: >>> @@ -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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/3] cap1106: Add support for various cap11xx devices 2014-09-21 9:58 ` Daniel Mack 2014-09-21 22:46 ` Matt Ranostay @ 2014-09-22 0:28 ` Matt Ranostay 2014-09-22 5:59 ` Dmitry Torokhov 1 sibling, 1 reply; 13+ messages in thread From: Matt Ranostay @ 2014-09-22 0:28 UTC (permalink / raw) To: Daniel Mack Cc: Kumar Gala, linux-input, linux-kernel, Dmitry Torokhov, Rob Herring, devicetree On Sun, Sep 21, 2014 at 2:58 AM, Daniel Mack <daniel@zonque.org> 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@gmail.com> >> --- >> 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); > > 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 >> @@ -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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/3] cap1106: Add support for various cap11xx devices 2014-09-22 0:28 ` Matt Ranostay @ 2014-09-22 5:59 ` Dmitry Torokhov 0 siblings, 0 replies; 13+ messages in thread From: Dmitry Torokhov @ 2014-09-22 5:59 UTC (permalink / raw) To: Matt Ranostay Cc: Daniel Mack, Kumar Gala, linux-input, linux-kernel, Rob Herring, devicetree 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@zonque.org> 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@gmail.com> > >> --- > >> 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/3] cap1106: Add support for various cap11xx devices 2014-09-21 3:01 ` [PATCH 1/3] cap1106: Add support for various cap11xx devices Matt Ranostay 2014-09-21 9:58 ` Daniel Mack @ 2014-09-21 10:53 ` Daniel Mack 1 sibling, 0 replies; 13+ messages in thread From: Daniel Mack @ 2014-09-21 10:53 UTC (permalink / raw) To: Matt Ranostay, galak, linux-input, linux-kernel, dmitry.torokhov, robh+dt Cc: devicetree 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 2/3] cap1106: support for active-high interrupt option 2014-09-21 3:01 [PATCH 0/3] cap1106: add support for cap11xx variants Matt Ranostay 2014-09-21 3:01 ` [PATCH 1/3] cap1106: Add support for various cap11xx devices Matt Ranostay @ 2014-09-21 3:01 ` Matt Ranostay 2014-09-21 10:06 ` Daniel Mack 2014-09-21 3:01 ` [PATCH 3/3] dt: cap1106 active-high property addition Matt Ranostay 2 siblings, 1 reply; 13+ messages in thread From: Matt Ranostay @ 2014-09-21 3:01 UTC (permalink / raw) To: galak, linux-input, linux-kernel, dmitry.torokhov, zonque, robh+dt Cc: devicetree, Matt Ranostay 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 <mranostay@gmail.com> --- 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 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 2/3] cap1106: support for active-high interrupt option 2014-09-21 3:01 ` [PATCH 2/3] cap1106: support for active-high interrupt option Matt Ranostay @ 2014-09-21 10:06 ` Daniel Mack 2014-09-22 5:56 ` Dmitry Torokhov 0 siblings, 1 reply; 13+ messages in thread From: Daniel Mack @ 2014-09-21 10:06 UTC (permalink / raw) To: Matt Ranostay, galak, linux-input, linux-kernel, dmitry.torokhov, robh+dt Cc: devicetree On 09/21/2014 05:01 AM, Matt Ranostay wrote: > 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 <mranostay@gmail.com> > --- > 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"? 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/3] cap1106: support for active-high interrupt option 2014-09-21 10:06 ` Daniel Mack @ 2014-09-22 5:56 ` Dmitry Torokhov 2014-09-22 7:44 ` Daniel Mack 0 siblings, 1 reply; 13+ messages in thread From: Dmitry Torokhov @ 2014-09-22 5:56 UTC (permalink / raw) To: Daniel Mack Cc: Matt Ranostay, galak, linux-input, linux-kernel, robh+dt, devicetree On Sun, Sep 21, 2014 at 12:06:30PM +0200, Daniel Mack wrote: > On 09/21/2014 05:01 AM, Matt Ranostay wrote: > > 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 <mranostay@gmail.com> > > --- > > 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/3] cap1106: support for active-high interrupt option 2014-09-22 5:56 ` Dmitry Torokhov @ 2014-09-22 7:44 ` Daniel Mack 0 siblings, 0 replies; 13+ messages in thread From: Daniel Mack @ 2014-09-22 7:44 UTC (permalink / raw) To: Dmitry Torokhov Cc: Matt Ranostay, galak, linux-input, linux-kernel, robh+dt, devicetree On 09/22/2014 07:56 AM, Dmitry Torokhov wrote: > On Sun, Sep 21, 2014 at 12:06:30PM +0200, Daniel Mack wrote: >> On 09/21/2014 05:01 AM, Matt Ranostay wrote: >>> 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 <mranostay@gmail.com> >>> --- >>> 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 3/3] dt: cap1106 active-high property addition 2014-09-21 3:01 [PATCH 0/3] cap1106: add support for cap11xx variants Matt Ranostay 2014-09-21 3:01 ` [PATCH 1/3] cap1106: Add support for various cap11xx devices Matt Ranostay 2014-09-21 3:01 ` [PATCH 2/3] cap1106: support for active-high interrupt option Matt Ranostay @ 2014-09-21 3:01 ` Matt Ranostay 2 siblings, 0 replies; 13+ messages in thread From: Matt Ranostay @ 2014-09-21 3:01 UTC (permalink / raw) To: galak, linux-input, linux-kernel, dmitry.torokhov, zonque, robh+dt Cc: devicetree, Matt Ranostay Documenting the microchip,active-high property for interrupt pin behavior. Signed-off-by: Matt Ranostay <mranostay@gmail.com> --- 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 ^ permalink raw reply related [flat|nested] 13+ messages in thread
end of thread, other threads:[~2014-09-22 7:44 UTC | newest] Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2014-09-21 3:01 [PATCH 0/3] cap1106: add support for cap11xx variants Matt Ranostay 2014-09-21 3:01 ` [PATCH 1/3] cap1106: Add support for various cap11xx devices Matt Ranostay 2014-09-21 9:58 ` Daniel Mack 2014-09-21 22:46 ` Matt Ranostay 2014-09-22 7:36 ` Daniel Mack 2014-09-22 0:28 ` Matt Ranostay 2014-09-22 5:59 ` Dmitry Torokhov 2014-09-21 10:53 ` Daniel Mack 2014-09-21 3:01 ` [PATCH 2/3] cap1106: support for active-high interrupt option Matt Ranostay 2014-09-21 10:06 ` Daniel Mack 2014-09-22 5:56 ` Dmitry Torokhov 2014-09-22 7:44 ` Daniel Mack 2014-09-21 3:01 ` [PATCH 3/3] dt: cap1106 active-high property addition Matt Ranostay
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).