| Message ID | 1507266491-73971-5-git-send-email-preid@electromag.com.au |
|---|---|
| State | New |
| Headers | show |
| Series | pinctrl: mcp32s08: add support for mcp23018 | expand |
Hi, On Fri, Oct 06, 2017 at 01:08:08PM +0800, Phil Reid wrote: > This flag set the mcp23s08 device series irq type to open drain active low. > > Signed-off-by: Phil Reid <preid@electromag.com.au> Reviewed-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk> -- Sebastian > --- > Documentation/devicetree/bindings/pinctrl/pinctrl-mcp23s08.txt | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/Documentation/devicetree/bindings/pinctrl/pinctrl-mcp23s08.txt b/Documentation/devicetree/bindings/pinctrl/pinctrl-mcp23s08.txt > index 8a5bf99..7a0808f 100644 > --- a/Documentation/devicetree/bindings/pinctrl/pinctrl-mcp23s08.txt > +++ b/Documentation/devicetree/bindings/pinctrl/pinctrl-mcp23s08.txt > @@ -58,6 +58,8 @@ Optional device specific properties: > On devices with only one interrupt output this property is useless. > - microchip,irq-active-high: Sets the INTPOL flag in the IOCON register. This > configures the IRQ output polarity as active high. > +- microchip,irq-open-drain: Sets the ODR flag in the IOCON register. This > + configures the IRQ output as open drain active low. > > Example I2C (with interrupt): > gpiom1: gpio@20 { > -- > 1.8.3.1 >
So I applied patches to this point. On Fri, Oct 6, 2017 at 7:08 AM, Phil Reid <preid@electromag.com.au> wrote: > This flag set the mcp23s08 device series irq type to open drain active low. > > Signed-off-by: Phil Reid <preid@electromag.com.au> > --- (...) > - microchip,irq-active-high: Sets the INTPOL flag in the IOCON register. This > configures the IRQ output polarity as active high. > +- microchip,irq-open-drain: Sets the ODR flag in the IOCON register. This > + configures the IRQ output as open drain active low. I have cold feet on these two. I do not think either of these flags should be there or even be used. It is a bit tricky wrt device tree because it is Linuxisms, but I hope the DT maintainers know what is applicable also for other operating systems. So the MCP23xxx outputs an IRQ. Someone, somewhere, is using this IRQ. If that *user* needs to have the IRQ fireing active high or set as open drain (because there are more clients on the line) then it needs this set up. Look at this code from drivers/iio/gyro/mpu3050-core.c: /* Check if IRQ is open drain */ if (of_property_read_bool(mpu3050->dev->of_node, "drive-open-drain")) mpu3050->irq_opendrain = true; irq_trig = irqd_get_trigger_type(irq_get_irq_data(irq)); /* * Configure the interrupt generator hardware to supply whatever * the interrupt is configured for, edges low/high level low/high, * we can provide it all. */ switch (irq_trig) { case IRQF_TRIGGER_RISING: dev_info(&indio_dev->dev, "pulse interrupts on the rising edge\n"); break; case IRQF_TRIGGER_FALLING: mpu3050->irq_actl = true; dev_info(&indio_dev->dev, "pulse interrupts on the falling edge\n"); break; case IRQF_TRIGGER_HIGH: mpu3050->irq_latch = true; dev_info(&indio_dev->dev, "interrupts active high level\n"); /* * With level IRQs, we mask the IRQ until it is processed, * but with edge IRQs (pulses) we can queue several interrupts * in the top half. */ irq_trig |= IRQF_ONESHOT; break; case IRQF_TRIGGER_LOW: mpu3050->irq_latch = true; mpu3050->irq_actl = true; irq_trig |= IRQF_ONESHOT; dev_info(&indio_dev->dev, "interrupts active low level\n"); break; default: /* This is the most preferred mode, if possible */ dev_err(&indio_dev->dev, "unsupported IRQ trigger specified (%lx), enforce " "rising edge\n", irq_trig); irq_trig = IRQF_TRIGGER_RISING; break; } /* An open drain line can be shared with several devices */ if (mpu3050->irq_opendrain) irq_trig |= IRQF_SHARED; ret = request_threaded_irq(irq, mpu3050_irq_handler, mpu3050_irq_thread, irq_trig, mpu3050->trig->name, mpu3050->trig); if (ret) { dev_err(mpu3050->dev, "can't get IRQ %d, error %d\n", irq, ret); return ret; } Notice that the IIO gyro here is also a *producer* of IRQs sending then to some *consumer* such as a GPIO pin or dedicated IRQ in on a SoC. Lessons learned: 1) Use the standard binding "drive-open-drain" for this, not any mcp,* custom namespaced things. 2) WRT active high: please deprecate that flag, and ignore it completely in the driver or at least print a warning. Instead look up the trigger type from the irq descriptor just like the MPU3050 IIO driver does. The information is already inside the kernel, from the consumer side. The reciever states what kind of interrupt it wants. It is fair to add a flag for open drain (with the standard binding), as the IRQ subsystem has no idea about such things, but with that comes the inevitable consequence of requesting a shared IRQ. I will check if I can anyways apply some more patches not related to this. Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/Documentation/devicetree/bindings/pinctrl/pinctrl-mcp23s08.txt b/Documentation/devicetree/bindings/pinctrl/pinctrl-mcp23s08.txt index 8a5bf99..7a0808f 100644 --- a/Documentation/devicetree/bindings/pinctrl/pinctrl-mcp23s08.txt +++ b/Documentation/devicetree/bindings/pinctrl/pinctrl-mcp23s08.txt @@ -58,6 +58,8 @@ Optional device specific properties: On devices with only one interrupt output this property is useless. - microchip,irq-active-high: Sets the INTPOL flag in the IOCON register. This configures the IRQ output polarity as active high. +- microchip,irq-open-drain: Sets the ODR flag in the IOCON register. This + configures the IRQ output as open drain active low. Example I2C (with interrupt): gpiom1: gpio@20 {
This flag set the mcp23s08 device series irq type to open drain active low. Signed-off-by: Phil Reid <preid@electromag.com.au> --- Documentation/devicetree/bindings/pinctrl/pinctrl-mcp23s08.txt | 2 ++ 1 file changed, 2 insertions(+)