Re: [PATCH v2] pinctrl: baytrail: Clear direct_irq_en flag on broken configs

[Andy Shevchenko <andriy.shevchenko@xxxxxxxxx>]

On Fri, Jan 07, 2022 at 03:23:43PM +0100, Hans de Goede wrote:
> Some boards set the direct_irq_en flag in the conf0 register without
> setting any of the trigger bits. The direct_irq_en flag just means that
> the GPIO will send IRQs directly to the APIC instead of going through
> the shared interrupt for the GPIO controller, in order for the pin to
> be able to actually generate IRQs the trigger flags must still be set.
> 
> So having the direct_irq_en flag set without any trigger flags is
> non-sense, log a FW_BUG warning when encountering this and clear the flag
> so that a driver can actually use the pin as IRQ through gpiod_to_irq().
> 
> Specifically this allows the edt-ft5x06 touchscreen driver to use
> INT33FC:02 pin 3 as touchscreen IRQ on the Nextbook Ares 8 tablet,
> accompanied by the following new log message:
> 
> byt_gpio INT33FC:02: [Firmware Bug]: pin 3: direct_irq_en set without trigger, clearing
> 
> The new byt_direct_irq_sanity_check() function also checks that the
> pin is actually appointed to one of the 16 direct-IRQs which the
> GPIO controller support and on success prints debug msg like these:

supports ?

> byt_gpio INT33FC:02: Pin 0: uses direct IRQ 0 (APIC 67)
> byt_gpio INT33FC:02: Pin 15: uses direct IRQ 2 (APIC 69)
> 
> This is useful to figure out the GPIO pin belonging to ACPI
> resources like this one: "Interrupt () { 0x00000043 }" or
> the other way around.

...

> +static bool byt_direct_irq_sanity_check(struct intel_pinctrl *vg, int pin, u32 value)
> +{
> +	void __iomem *reg;
> +	int i, j;
> +
> +	if (!(value & (BYT_TRIG_POS | BYT_TRIG_NEG))) {
> +		dev_warn(vg->dev,
> +			 FW_BUG "pin %i: direct_irq_en set without trigger, clearing\n", pin);
> +		return false;
> +	}
> +
> +	reg = vg->communities->pad_regs + BYT_DIRECT_IRQ_REG;

> +	for (i = 0; i < 16; i += 4) {
> +		value = readl(reg + i);
> +		for (j = 0; j < 4; j++) {
> +			if (((value >> j * 8) & 0xff) == pin) {

Can it be like

	u32 direct_irq[16];
	void __iomem *reg;
	void *match;


	memcpy_fromio(...);
	match = memchr(...);
	if (match)
		dev_dbg();
	else
		dev_warn();

	return !!match;

?

> +				dev_dbg(vg->dev, "Pin %i: uses direct IRQ %d (APIC %d)\n",
> +					pin, i + j, 0x43 + i + j);
> +				return true;
> +			}
> +		}
> +	}
> +
> +	dev_warn(vg->dev,
> +		 FW_BUG "pin %i: direct_irq_en set but no IRQ assigned, clearing\n", pin);
> +	return false;
> +}

-- 
With Best Regards,
Andy Shevchenko