[v2,6/7] pinctrl: mcp23s08: remove hardcoded irq polarity in irq_setup

The irq_active_high flag is for controlling the polarity of the output
from the mcp23s08 series devices. The polarity of the irq could be altered
by additional logic (eg inverters) between the device and irq input device.
The device-tree already allows for this as the irq can be specified in the
binding. So hardcoding it in the driver is overly restrictive.

There are no inkernel users of the mcp23s08 driver with irq's.

Signed-off-by: Phil Reid <preid@electromag.com.au>
---
 drivers/pinctrl/pinctrl-mcp23s08.c | 5 -----
 1 file changed, 5 deletions(-)

Hi,

On Fri, Oct 06, 2017 at 01:08:10PM +0800, Phil Reid wrote:
> The irq_active_high flag is for controlling the polarity of the output
> from the mcp23s08 series devices. The polarity of the irq could be altered
> by additional logic (eg inverters) between the device and irq input device.
> The device-tree already allows for this as the irq can be specified in the
> binding. So hardcoding it in the driver is overly restrictive.
> 
> There are no inkernel users of the mcp23s08 driver with irq's.
> 
> Signed-off-by: Phil Reid <preid@electromag.com.au>

Reviewed-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk>

-- Sebastian

> ---
>  drivers/pinctrl/pinctrl-mcp23s08.c | 5 -----
>  1 file changed, 5 deletions(-)
> 
> diff --git a/drivers/pinctrl/pinctrl-mcp23s08.c b/drivers/pinctrl/pinctrl-mcp23s08.c
> index 150f216..8dceaa1 100644
> --- a/drivers/pinctrl/pinctrl-mcp23s08.c
> +++ b/drivers/pinctrl/pinctrl-mcp23s08.c
> @@ -630,11 +630,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);
> -- 
> 1.8.3.1
>

On Fri, Oct 6, 2017 at 7:08 AM, Phil Reid <preid@electromag.com.au> wrote:

> The irq_active_high flag is for controlling the polarity of the output
> from the mcp23s08 series devices. The polarity of the irq could be altered
> by additional logic (eg inverters) between the device and irq input device.
> The device-tree already allows for this as the irq can be specified in the
> binding. So hardcoding it in the driver is overly restrictive.
>
> There are no inkernel users of the mcp23s08 driver with irq's.
>
> Signed-off-by: Phil Reid <preid@electromag.com.au>

Look this up from the irq descriptor as discussed in 4/7.

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

On Fri, Oct 6, 2017 at 7:08 AM, Phil Reid <preid@electromag.com.au> wrote:

> The polarity of the irq could be altered
> by additional logic (eg inverters) between the device and irq input device.

Marc Zyngier has pointed out that inverters should be modeled
using hierarchichal irq domains. You would need to add a new
irqchip to invert the line.

See this PDF:
https://elinux.org/images/8/8c/Zyngier.pdf

drivers/irqchips/irq-uniphier-aidet.c supports inversion of IRQs
for example. For this it uses irq_domain_create_hierarchy()

I guess it would be helpful with a reusable generic "inverter
irq chip", that you can just enable and slam into your device
tree to tell the system that a line is statically inverted, i.e.
non-programmable logic on the board.

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

On 12/10/2017 17:04, Linus Walleij wrote:
> On Fri, Oct 6, 2017 at 7:08 AM, Phil Reid <preid@electromag.com.au> wrote:
> 
>> The polarity of the irq could be altered
>> by additional logic (eg inverters) between the device and irq input device.
> 
> Marc Zyngier has pointed out that inverters should be modeled
> using hierarchichal irq domains. You would need to add a new
> irqchip to invert the line.
> 
> See this PDF:
> https://elinux.org/images/8/8c/Zyngier.pdf
> 
> drivers/irqchips/irq-uniphier-aidet.c supports inversion of IRQs
> for example. For this it uses irq_domain_create_hierarchy()
> 
> I guess it would be helpful with a reusable generic "inverter
> irq chip", that you can just enable and slam into your device
> tree to tell the system that a line is statically inverted, i.e.
> non-programmable logic on the board.
> 
G'day Linus.
Thanks for the pointers, I'll see what I can come up with.