* [PATCH 0/3] cap1106: add support for cap11xx variants @ 2014-09-21 3:01 ` Matt Ranostay 0 siblings, 0 replies; 15+ messages in thread From: Matt Ranostay @ 2014-09-21 3:01 UTC (permalink / raw) To: galak-sgV2jX0FEOL9JmXXK+q4OQ, linux-input-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA, dmitry.torokhov-Re5JQEeQqe8AvxtiuMwx3w, zonque-Re5JQEeQqe8AvxtiuMwx3w, robh+dt-DgEjT+Ai2ygdnm+yROfE0A Cc: devicetree-u79uwXL29TY76Z2rM5mHXA, 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 -- 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 ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 0/3] cap1106: add support for cap11xx variants @ 2014-09-21 3:01 ` Matt Ranostay 0 siblings, 0 replies; 15+ 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] 15+ messages in thread
* [PATCH 1/3] cap1106: Add support for various cap11xx devices 2014-09-21 3:01 ` Matt Ranostay (?) @ 2014-09-21 3:01 ` Matt Ranostay 2014-09-21 9:58 ` Daniel Mack 2014-09-21 10:53 ` Daniel Mack -1 siblings, 2 replies; 15+ 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] 15+ 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 [not found] ` <541EA149.5000408-cYrQPVfZoowdnm+yROfE0A@public.gmane.org> 2014-09-22 0:28 ` Matt Ranostay 2014-09-21 10:53 ` Daniel Mack 1 sibling, 2 replies; 15+ 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] 15+ messages in thread
[parent not found: <541EA149.5000408-cYrQPVfZoowdnm+yROfE0A@public.gmane.org>]
* 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 1 sibling, 0 replies; 15+ messages in thread From: Matt Ranostay @ 2014-09-21 22:46 UTC (permalink / raw) To: Daniel Mack Cc: Kumar Gala, linux-input-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Dmitry Torokhov, Rob Herring, devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org On Sun, Sep 21, 2014 at 2:58 AM, Daniel Mack <daniel-cYrQPVfZoowdnm+yROfE0A@public.gmane.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-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> > > --- > > 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 -- 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 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/3] cap1106: Add support for various cap11xx devices @ 2014-09-21 22:46 ` Matt Ranostay 0 siblings, 0 replies; 15+ messages in thread From: Matt Ranostay @ 2014-09-21 22:46 UTC (permalink / raw) To: Daniel Mack Cc: Kumar Gala, linux-input, linux-kernel@vger.kernel.org, Dmitry Torokhov, Rob Herring, devicetree@vger.kernel.org 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] 15+ 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 -1 siblings, 0 replies; 15+ messages in thread From: Daniel Mack @ 2014-09-22 7:36 UTC (permalink / raw) To: Matt Ranostay Cc: Kumar Gala, linux-input, linux-kernel@vger.kernel.org, Dmitry Torokhov, Rob Herring, devicetree@vger.kernel.org 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] 15+ messages in thread
* Re: [PATCH 1/3] cap1106: Add support for various cap11xx devices 2014-09-21 9:58 ` Daniel Mack [not found] ` <541EA149.5000408-cYrQPVfZoowdnm+yROfE0A@public.gmane.org> @ 2014-09-22 0:28 ` Matt Ranostay 2014-09-22 5:59 ` Dmitry Torokhov 1 sibling, 1 reply; 15+ messages in thread From: Matt Ranostay @ 2014-09-22 0:28 UTC (permalink / raw) To: Daniel Mack Cc: Kumar Gala, linux-input, linux-kernel@vger.kernel.org, Dmitry Torokhov, Rob Herring, devicetree@vger.kernel.org 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] 15+ 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; 15+ 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@vger.kernel.org, Rob Herring, devicetree@vger.kernel.org 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] 15+ 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; 15+ 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] 15+ messages in thread
* [PATCH 2/3] cap1106: support for active-high interrupt option 2014-09-21 3:01 ` Matt Ranostay (?) (?) @ 2014-09-21 3:01 ` Matt Ranostay 2014-09-21 10:06 ` Daniel Mack -1 siblings, 1 reply; 15+ 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] 15+ 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; 15+ 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] 15+ 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; 15+ 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] 15+ 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; 15+ 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] 15+ messages in thread
* [PATCH 3/3] dt: cap1106 active-high property addition 2014-09-21 3:01 ` Matt Ranostay ` (2 preceding siblings ...) (?) @ 2014-09-21 3:01 ` Matt Ranostay -1 siblings, 0 replies; 15+ 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] 15+ messages in thread
end of thread, other threads:[~2014-09-22 7:44 UTC | newest]
Thread overview: 15+ 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 ` 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
[not found] ` <541EA149.5000408-cYrQPVfZoowdnm+yROfE0A@public.gmane.org>
2014-09-21 22:46 ` Matt Ranostay
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 an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.