| Message ID | 1511252491-79952-2-git-send-email-preid@electromag.com.au |
|---|---|
| State | New |
| Headers | show |
| Series | pinctrl: mcp32s08: add open drain config for irq | expand |
Hi, On Tue, Nov 21, 2017 at 04:21:27PM +0800, Phil Reid wrote: > The polarity of the irq should be defined in the configuration > for the irq. eg The device tree bind already allows for either > active high / low interrupt configuration. > > Signed-off-by: Phil Reid <preid@electromag.com.au> > --- I think the patch is right, but the long patch description is not. I would expect something like this: This changes the driver, so that the "microchip,irq-active-high" property only configures the mcp23017 interrupt output, but not the host interrupt input. The host interrupt should be configured using the standard interrupt flags. -- Sebastian > drivers/pinctrl/pinctrl-mcp23s08.c | 14 ++++---------- > 1 file changed, 4 insertions(+), 10 deletions(-) > > diff --git a/drivers/pinctrl/pinctrl-mcp23s08.c b/drivers/pinctrl/pinctrl-mcp23s08.c > index 0aef30e..cc1f9f6 100644 > --- a/drivers/pinctrl/pinctrl-mcp23s08.c > +++ b/drivers/pinctrl/pinctrl-mcp23s08.c > @@ -56,7 +56,6 @@ > > struct mcp23s08 { > u8 addr; > - bool irq_active_high; > bool reg_shift; > > u16 irq_rise; > @@ -627,11 +626,6 @@ static int mcp23s08_irq_setup(struct mcp23s08 *mcp) > int err; > unsigned long irqflags = IRQF_ONESHOT | IRQF_SHARED; > > - if (mcp->irq_active_high) > - irqflags |= IRQF_TRIGGER_HIGH; > - else > - irqflags |= IRQF_TRIGGER_LOW; > - > err = devm_request_threaded_irq(chip->parent, mcp->irq, NULL, > mcp23s08_irq, > irqflags, dev_name(chip->parent), mcp); > @@ -777,12 +771,12 @@ static int mcp23s08_probe_one(struct mcp23s08 *mcp, struct device *dev, > { > int status, ret; > bool mirror = false; > + bool irq_active_high = false; > > mutex_init(&mcp->lock); > > mcp->dev = dev; > mcp->addr = addr; > - mcp->irq_active_high = false; > > mcp->chip.direction_input = mcp23s08_direction_input; > mcp->chip.get = mcp23s08_get; > @@ -868,7 +862,7 @@ static int mcp23s08_probe_one(struct mcp23s08 *mcp, struct device *dev, > mcp->irq_controller = > device_property_read_bool(dev, "interrupt-controller"); > if (mcp->irq && mcp->irq_controller) { > - mcp->irq_active_high = > + irq_active_high = > device_property_read_bool(dev, > "microchip,irq-active-high"); > > @@ -876,11 +870,11 @@ static int mcp23s08_probe_one(struct mcp23s08 *mcp, struct device *dev, > } > > if ((status & IOCON_SEQOP) || !(status & IOCON_HAEN) || mirror || > - mcp->irq_active_high) { > + irq_active_high) { > /* mcp23s17 has IOCON twice, make sure they are in sync */ > status &= ~(IOCON_SEQOP | (IOCON_SEQOP << 8)); > status |= IOCON_HAEN | (IOCON_HAEN << 8); > - if (mcp->irq_active_high) > + if (irq_active_high) > status |= IOCON_INTPOL | (IOCON_INTPOL << 8); > else > status &= ~(IOCON_INTPOL | (IOCON_INTPOL << 8)); > -- > 1.8.3.1 >
diff --git a/drivers/pinctrl/pinctrl-mcp23s08.c b/drivers/pinctrl/pinctrl-mcp23s08.c index 0aef30e..cc1f9f6 100644 --- a/drivers/pinctrl/pinctrl-mcp23s08.c +++ b/drivers/pinctrl/pinctrl-mcp23s08.c @@ -56,7 +56,6 @@ struct mcp23s08 { u8 addr; - bool irq_active_high; bool reg_shift; u16 irq_rise; @@ -627,11 +626,6 @@ static int mcp23s08_irq_setup(struct mcp23s08 *mcp) int err; unsigned long irqflags = IRQF_ONESHOT | IRQF_SHARED; - if (mcp->irq_active_high) - irqflags |= IRQF_TRIGGER_HIGH; - else - irqflags |= IRQF_TRIGGER_LOW; - err = devm_request_threaded_irq(chip->parent, mcp->irq, NULL, mcp23s08_irq, irqflags, dev_name(chip->parent), mcp); @@ -777,12 +771,12 @@ static int mcp23s08_probe_one(struct mcp23s08 *mcp, struct device *dev, { int status, ret; bool mirror = false; + bool irq_active_high = false; mutex_init(&mcp->lock); mcp->dev = dev; mcp->addr = addr; - mcp->irq_active_high = false; mcp->chip.direction_input = mcp23s08_direction_input; mcp->chip.get = mcp23s08_get; @@ -868,7 +862,7 @@ static int mcp23s08_probe_one(struct mcp23s08 *mcp, struct device *dev, mcp->irq_controller = device_property_read_bool(dev, "interrupt-controller"); if (mcp->irq && mcp->irq_controller) { - mcp->irq_active_high = + irq_active_high = device_property_read_bool(dev, "microchip,irq-active-high"); @@ -876,11 +870,11 @@ static int mcp23s08_probe_one(struct mcp23s08 *mcp, struct device *dev, } if ((status & IOCON_SEQOP) || !(status & IOCON_HAEN) || mirror || - mcp->irq_active_high) { + irq_active_high) { /* mcp23s17 has IOCON twice, make sure they are in sync */ status &= ~(IOCON_SEQOP | (IOCON_SEQOP << 8)); status |= IOCON_HAEN | (IOCON_HAEN << 8); - if (mcp->irq_active_high) + if (irq_active_high) status |= IOCON_INTPOL | (IOCON_INTPOL << 8); else status &= ~(IOCON_INTPOL | (IOCON_INTPOL << 8));
The polarity of the irq should be defined in the configuration for the irq. eg The device tree bind already allows for either active high / low interrupt configuration. Signed-off-by: Phil Reid <preid@electromag.com.au> --- drivers/pinctrl/pinctrl-mcp23s08.c | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-)