On Thu, 2015-09-10 at 14:04 +0000, Tirdea, Irina wrote: > > > -----Original Message----- > > From: Bastien Nocera [mailto:hadess@xxxxxxxxxx] > > Sent: 09 September, 2015 20:03 > > To: Tirdea, Irina; linux-input@xxxxxxxxxxxxxxx > > Cc: linux-kernel@xxxxxxxxxxxxxxx; Rob Herring; Pawel Moll; Ian > > Campbell; Kumar Gala; Purdila, Octavian; Dmitry Torokhov; Mark > > Rutland; devicetree@xxxxxxxxxxxxxxx > > 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 devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html