lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [day] [month] [year] [list]
Date:   Tue, 23 Apr 2019 12:57:31 +0800
From:   Kai-Heng Feng <kai.heng.feng@...onical.com>
To:     hotwater438@...anota.com
Cc:     Mika Westerberg <mika.westerberg@...ux.intel.com>,
        Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
        linus.walleij@...aro.org, Hdegoede <hdegoede@...hat.com>,
        linux-gpio@...r.kernel.org,
        Linux Kernel <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] pinctrl: intel: Clear interrupt status in unmask callback

Hi,

at 02:22, <hotwater438@...anota.com> <hotwater438@...anota.com> wrote:

> Hi.
> I've just applied this patch, and touchpad woorks smoothly, but suspend  
> issue is still present.
> After suspend, i2c_hid module bursts i2c_hid i2c-ELAN1200:00:  
> i2c_hid_get_input: incomplete report (16/65535) messages (more than 50  
> reports/sec).
> In dmesg I can see a frequency of reporting every 0.0007 - 0.001 dmesg  
> time units.

What’s the default suspend mode on the platform?
This is a common issue for system that defaults to Suspend-to-idle, but S3  
is in use.
The root cause is that the power of the touchpad doesn’t get cut off during  
S3 by platform firmware.

Do you also see this issue if S2I is in use?

Kai-Heng

>
> Though I can sucessfully restart module and after restarting it works as  
> good as it was.
>
> So suspend issue is still present.
>
> Regards,
> Vladislav.
>
>
> Apr 22, 2019, 7:45 AM by kai.heng.feng@...onical.com:
> Commit a939bb57cd47 ("pinctrl: intel: implement gpio_irq_enable") was
> added because clearing interrupt status bit is required to avoid
> unexpected behavior.
>
> Turns out the unmask callback also needs the fix, which can solve weird
> IRQ triggering issues on I2C touchpad ELAN1200.
>
> Signed-off-by: Kai-Heng Feng <kai.heng.feng@...onical.com>
> ---
> drivers/pinctrl/intel/pinctrl-intel.c | 35 ++++-----------------------
> 1 file changed, 5 insertions(+), 30 deletions(-)
>
> diff --git a/drivers/pinctrl/intel/pinctrl-intel.c  
> b/drivers/pinctrl/intel/pinctrl-intel.c
> index 3b1818184207..53878604537e 100644
> --- a/drivers/pinctrl/intel/pinctrl-intel.c
> +++ b/drivers/pinctrl/intel/pinctrl-intel.c
> @@ -913,35 +913,6 @@ static void intel_gpio_irq_ack(struct irq_data *d)
> }
> }
>
> -static void intel_gpio_irq_enable(struct irq_data *d)
> -{
> -	struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
> -	struct intel_pinctrl *pctrl = gpiochip_get_data(gc);
> -	const struct intel_community *community;
> -	const struct intel_padgroup *padgrp;
> -	int pin;
> -
> -	pin = intel_gpio_to_pin(pctrl, irqd_to_hwirq(d), &community, &padgrp);
> -	if (pin >= 0) {
> -	unsigned int gpp, gpp_offset, is_offset;
> -	unsigned long flags;
> -	u32 value;
> -
> -	gpp = padgrp->reg_num;
> -	gpp_offset = padgroup_offset(padgrp, pin);
> -	is_offset = community->is_offset + gpp * 4;
> -
> -	raw_spin_lock_irqsave(&pctrl->lock, flags);
> -	/* Clear interrupt status first to avoid unexpected interrupt */
> -	writel(BIT(gpp_offset), community->regs + is_offset);
> -
> -	value = readl(community->regs + community->ie_offset + gpp * 4);
> -	value |= BIT(gpp_offset);
> -	writel(value, community->regs + community->ie_offset + gpp * 4);
> -	raw_spin_unlock_irqrestore(&pctrl->lock, flags);
> -	}
> -}
> -
> static void intel_gpio_irq_mask_unmask(struct irq_data *d, bool mask)
> {
> struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
> @@ -963,6 +934,11 @@ static void intel_gpio_irq_mask_unmask(struct  
> irq_data *d, bool mask)
> reg = community->regs + community->ie_offset + gpp * 4;
>
> raw_spin_lock_irqsave(&pctrl->lock, flags);
> +
> +	/* Clear interrupt status first to avoid unexpected interrupt */
> +	if (!mask)
> +	writel(BIT(gpp_offset), community->regs + community->is_offset +  
> gpp * 4);
> +
> value = readl(reg);
> if (mask)
> value &= ~BIT(gpp_offset);
> @@ -1106,7 +1082,6 @@ static irqreturn_t intel_gpio_irq(int irq, void  
> *data)
>
> static struct irq_chip intel_gpio_irqchip = {
> .name = "intel-gpio",
> -	.irq_enable = intel_gpio_irq_enable,
> .irq_ack = intel_gpio_irq_ack,
> .irq_mask = intel_gpio_irq_mask,
> .irq_unmask = intel_gpio_irq_unmask,
> -- 
> 2.17.1


Powered by blists - more mailing lists