Re: [PATCH v3 4/5] pinctrl: mcp23s08: configure irq polarity using irq data

[Sebastian Reichel <sre@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 2320 bytes --]

Hi,

On Tue, Nov 21, 2017 at 04:21:30PM +0800, Phil Reid wrote:
> The irq polarity is already encoded in the irq config. Use that to
> determine the polarity for the mcp32s08 irq output instead of the
> custom microchip,irq-active-high property.
> 
> Signed-off-by: Phil Reid <preid@electromag.com.au>
> ---

I don't like, that we use the flags for configuring the host
interrupt input and the mcp23xxx interrupt output. Usually
when the interrupt line has an inverter on it, board DTS files
just toggle the interrupts polarity. This will not work with
this patch applied. We would need to explicitly add an inverter
in the interrupt line, which is completely different to how its
implemented everywhere else (I know at least some Tegra devices
have implicit inverters on interrupt lines).

In case this is really wanted, this patch and the first patch
should be merged to avoid temporarily exposing the splitted
logic.

-- Sebastian

>  drivers/pinctrl/pinctrl-mcp23s08.c | 11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/pinctrl/pinctrl-mcp23s08.c b/drivers/pinctrl/pinctrl-mcp23s08.c
> index 8ff9b77..6b3f810 100644
> --- a/drivers/pinctrl/pinctrl-mcp23s08.c
> +++ b/drivers/pinctrl/pinctrl-mcp23s08.c
> @@ -773,6 +773,7 @@ static int mcp23s08_probe_one(struct mcp23s08 *mcp, struct device *dev,
>  	bool mirror = false;
>  	bool irq_active_high = false;
>  	bool open_drain = false;
> +	u32 irq_trig;
>  
>  	mutex_init(&mcp->lock);
>  
> @@ -863,9 +864,13 @@ 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) {
> -		irq_active_high =
> -			device_property_read_bool(dev,
> -					      "microchip,irq-active-high");
> +		if (device_property_present(dev, "microchip,irq-active-high"))
> +			dev_warn(dev,
> +				 "microchip,irq-active-high is deprecated\n");
> +
> +		irq_trig = irqd_get_trigger_type(irq_get_irq_data(mcp->irq));
> +		if (irq_trig == IRQF_TRIGGER_HIGH)
> +			irq_active_high = true;
>  
>  		mirror = device_property_read_bool(dev, "microchip,irq-mirror");
>  		open_drain = device_property_read_bool(dev, "drive-open-drain");
> -- 
> 1.8.3.1
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]