diff mbox

[2/3] cap1106: support for active-high interrupt option

Message ID 1411268469-21283-3-git-send-email-mranostay@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Matt Ranostay Sept. 21, 2014, 3:01 a.m. UTC
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(+)

Comments

Daniel Mack Sept. 21, 2014, 10:06 a.m. UTC | #1
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

--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Dmitry Torokhov Sept. 22, 2014, 5:56 a.m. UTC | #2
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.
Daniel Mack Sept. 22, 2014, 7:44 a.m. UTC | #3
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

--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

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;