linux-kernel - Re: [PATCH v3 1/5] Input: goodix

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>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Fri, 25 Sep 2015 16:44:11 +0200
From:	Bastien Nocera <hadess@...ess.net>
To:	"Tirdea, Irina" <irina.tirdea@...el.com>,
	"linux-input@...r.kernel.org" <linux-input@...r.kernel.org>
Cc:	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Rob Herring <robh+dt@...nel.org>,
	Pawel Moll <pawel.moll@....com>,
	Ian Campbell <ijc+devicetree@...lion.org.uk>,
	Kumar Gala <galak@...eaurora.org>,
	"Purdila, Octavian" <octavian.purdila@...el.com>,
	Dmitry Torokhov <dmitry.torokhov@...il.com>,
	Mark Rutland <mark.rutland@....com>,
	"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>
Subject: Re: [PATCH v3 1/5] Input: goodix - reset device at init

On Thu, 2015-09-10 at 14:04 +0000, Tirdea, Irina wrote:
> 
> > -----Original Message-----
> > From: Bastien Nocera [mailto:hadess@...ess.net]
> > Sent: 09 September, 2015 20:03
> > To: Tirdea, Irina; linux-input@...r.kernel.org
> > Cc: linux-kernel@...r.kernel.org; Rob Herring; Pawel Moll; Ian
> > Campbell; Kumar Gala; Purdila, Octavian; Dmitry Torokhov; Mark
> > Rutland; devicetree@...r.kernel.org
> > Subject: Re: [PATCH v3 1/5] Input: goodix - reset device at init
> > 
> > On Thu, 2015-07-30 at 11:27 +0000, Tirdea, Irina wrote:
> > > I can send some additional patches that will simplify testing the
> > > configuration update to the Goodix device. I think this feature
> > > is
> > > the easiest
> > > to test so we can determine if writing to the interrupt pin
> > > actually
> > > works.
> > > However, even if it is a BIOS problem and the code will work, the
> > > warning
> > > will still be printed in dmesg.
> > 
> > 
> > Somehow missed this mail before replying to the current patchset.
> > I'd
> > be fine with that, though it's still not clear to me whether the
> > BIOS/hardware is at fault, or the code that's being added to the
> > driver
> > ;)
> > 
> 
> The reset procedure is described in the Goodix GT911 datasheet [1]
> and is
> used for power-on reset and power management. The power-on sequence
> is described in chapter 6.1. I2C Timing, in the Power-on Timing
> diagram.
> The sequence for putting the device to sleep is described in chapter
> 7.1. Operating Modes, c) Sleep mode. These sequences use the
> interrupt
> pin as output.
> 
> The warning you mentioned comes from the following code in the goodix
> driver, which sets the interrupt to be used as output:
> 
> +       error = gpiod_direction_output(ts->gpiod_int, ts->client-
> >addr == 0x14);
> 
> The gpiod_direction_output() call ends up in
> drivers/pinctrl/intel/pinctrl-baytrail.c:
> /*
>  * Before making any direction modifications, do a check if gpio
>  * is set for direct IRQ.  On baytrail, setting GPIO to output does
>  * not make sense, so let's at least warn the caller before they
> shoot
>  * themselves in the foot.
>  */
> WARN(readl(conf_reg) & BYT_DIRECT_IRQ_EN,
> 	"Potential Error: Setting GPIO with direct_irq_en to output");
> 
> So the problem comes from using the gpio interrupt pin as output,
> which 
> should not work on Baytrail if BYT_DIRECT_IRQ_EN is set by BIOS.
> The above warning is introduced and discussed in [2] and [3]. As I
> mentioned,
> this could be a real HW issue or the BIOS sets BYT_DIRECT_IRQ_EN when
> it should not. I have also tested these patches on a Baytrail
> plarform
> (that uses the same pinctrl driver), but I did not see any issues
> since
> BYT_DIRECT_IRQ_EN is not set in my case for the interrupt gpio pin.

Do we have a way to work-around this in the GPIO driver?

> To determine if using the interrupt pin as output works, you can test
> updating
> the goodix configuration [4].

Right, the problem being that it's a later patch in the branch, and
that the driver will fail to probe if there's an error from the GPIO
call.

Would you have a patch for me to test that would bypass this error, or
at least fallback gracefully to not resetting, not probing GPIOs if
they're badly setup?

Cheers
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/