RE: [PATCH v3 1/5] Input: goodix - reset device at init
> -Original Message- > From: mrgarna...@gmail.com [mailto:mrgarna...@gmail.com] On Behalf Of Carlos > Garnacho > Sent: 30 September, 2015 17:02 > To: Bastien Nocera > Cc: Tirdea, Irina; linux-in...@vger.kernel.org; Cosimo Cecchi; Christian > Hergert; linux-kernel@vger.kernel.org; Rob Herring; Pawel > Moll; Ian Campbell; Kumar Gala; Purdila, Octavian; Dmitry Torokhov; Mark > Rutland; devicet...@vger.kernel.org > Subject: Re: [PATCH v3 1/5] Input: goodix - reset device at init > > Hey, > Hi Carlos and Bastien, Thanks for your help and for testing these patches! > >> > I'm not sure how we can detect, and blacklist, those devices. At > >> > least > >> > my original device, the Onda v975w, and the WinBook TW100 would > >> > have > >> > those problems. > >> > > >> > >> I can use DMI quirks to exclude these devices from using the features > >> that > >> depend on the gpio pins. I already have the DMI information for > >> WinBook TW100 > >> and WinBook TW700. Could you tell me the DMI_SYS_VENDOR and > >> DMI_PRODUCT_NAME for Onda v975w so I can add it as well? > > > > I don't have access to the Onda v975w anymore, but let me CC: a few > > people that could also help with testing. > > > > Carlos, Cosimo, Christian, there's a patchset for you to test on the > > Onda v975w at: > > https://github.com/hadess/gt9xx/commits/irina-tirdea > > > > Doing an "rmmod goodix ; insmod ./goodix_backport.ko" should be enough > > to test whether the patch set works. If it doesn't work correctly, > > you'll need to reboot the machine, swap the irq_idx and rst_idx values > > at: > > https://github.com/hadess/gt9xx/commit/c27de79f494c2b2e7a198ff4d27976ae93669dbd#diff- > dddc2849e36327439530f3e2faacec4fR321 > > > > and try again. > > Unswapped does fail with: > > sep 30 15:37:29 tablet kernel: Goodix-TS i2c-GDIX1001:00: i2c test > failed attempt 1: -121 > sep 30 15:37:29 tablet kernel: Goodix-TS i2c-GDIX1001:00: i2c test > failed attempt 2: -121 > sep 30 15:37:29 tablet kernel: Goodix-TS i2c-GDIX1001:00: I2C > communication failure: -121 > sep 30 15:37:29 tablet kernel: Goodix-TS: probe of i2c-GDIX1001:00 > failed with error -121 > > Swapping the values triggers some errors: > > sep 30 15:48:17 tablet kernel: [ cut here ] > sep 30 15:48:17 tablet kernel: WARNING: CPU: 1 PID: 2341 at > drivers/pinctrl/intel/pinctrl-baytrail.c:342 > byt_gpio_direction_output+0xa1/0xb0() > sep 30 15:48:17 tablet kernel: Potential Error: Setting GPIO with > direct_irq_en to output > sep 30 15:48:17 tablet kernel: Modules linked in: > sep 30 15:48:17 tablet kernel: goodix_backport(OE+) r8723bs(OE) nfsd > lockd grace sunrpc [last unloaded: goodix] > sep 30 15:48:17 tablet kernel: CPU: 1 PID: 2341 Comm: insmod Tainted: > G OE 4.3.0-rc1+ #36 > sep 30 15:48:17 tablet kernel: Hardware name: To be filled by O.E.M. > To be filled by O.E.M./Aptio CRB, BIOS 5.6.5 07/25/2014 > sep 30 15:48:17 tablet kernel: d3f61c08 c13b347f > d3f61c48 d3f61c38 c10a9e1d c20f10e0 > sep 30 15:48:17 tablet kernel: d3f61c64 0925 c20f111c 0156 > c13e7ce1 c13e7ce1 f80580b8 f53a4b0c > sep 30 15:48:17 tablet kernel: 0156 d3f61c50 c10a9e93 0009 > d3f61c48 c20f10e0 d3f61c64 d3f61c78 > sep 30 15:48:17 tablet kernel: Call Trace: > sep 30 15:48:17 tablet kernel: [] dump_stack+0x48/0x69 > sep 30 15:48:17 tablet kernel: [] warn_slowpath_common+0x8d/0xd0 > sep 30 15:48:17 tablet kernel: [] ? > byt_gpio_direction_output+0xa1/0xb0 > sep 30 15:48:17 tablet kernel: [] ? > byt_gpio_direction_output+0xa1/0xb0 > sep 30 15:48:17 tablet kernel: [] warn_slowpath_fmt+0x33/0x40 > sep 30 15:48:17 tablet kernel: [] > byt_gpio_direction_output+0xa1/0xb0 > sep 30 15:48:17 tablet kernel: [] ? byt_gpio_irq_handler+0xb0/0xb0 > sep 30 15:48:17 tablet kernel: [] > _gpiod_direction_output_raw+0x63/0x350 > sep 30 15:48:17 tablet kernel: [] gpiod_direction_output+0x2a/0x50 > sep 30 15:48:17 tablet kernel: [] ? msleep+0x36/0x40 > sep 30 15:48:17 tablet kernel: [] goodix_reset+0x3e/0x90 > [goodix_backport] > sep 30 15:48:17 tablet kernel: [] > goodix_ts_probe+0x12a/0x5aa [goodix_backport] > sep 30 15:48:17 tablet kernel: [] ? acpi_device_wakeup+0x7a/0x80 > sep 30 15:48:17 tablet kernel: [] ? acpi_dev_pm_attach+0x71/0x87 > sep 30 15:48:17 tablet kernel: [] i2c_device_probe+0x121/0x1d0 > sep 30 15:48:17 tablet kernel: [] ? _raw_spin_unlock+0x22/0x30 > sep 30 15:48:17 tablet kernel: [] ? > goodix_config_cb+0xc0/0xc0 [goodix_backport] > sep 30 15:48:17 tablet kernel:
RE: [PATCH v3 1/5] Input: goodix - reset device at init
> -Original Message- > From: mrgarna...@gmail.com [mailto:mrgarna...@gmail.com] On Behalf Of Carlos > Garnacho > Sent: 30 September, 2015 17:02 > To: Bastien Nocera > Cc: Tirdea, Irina; linux-in...@vger.kernel.org; Cosimo Cecchi; Christian > Hergert; linux-kernel@vger.kernel.org; Rob Herring; Pawel > Moll; Ian Campbell; Kumar Gala; Purdila, Octavian; Dmitry Torokhov; Mark > Rutland; devicet...@vger.kernel.org > Subject: Re: [PATCH v3 1/5] Input: goodix - reset device at init > > Hey, > Hi Carlos and Bastien, Thanks for your help and for testing these patches! > >> > I'm not sure how we can detect, and blacklist, those devices. At > >> > least > >> > my original device, the Onda v975w, and the WinBook TW100 would > >> > have > >> > those problems. > >> > > >> > >> I can use DMI quirks to exclude these devices from using the features > >> that > >> depend on the gpio pins. I already have the DMI information for > >> WinBook TW100 > >> and WinBook TW700. Could you tell me the DMI_SYS_VENDOR and > >> DMI_PRODUCT_NAME for Onda v975w so I can add it as well? > > > > I don't have access to the Onda v975w anymore, but let me CC: a few > > people that could also help with testing. > > > > Carlos, Cosimo, Christian, there's a patchset for you to test on the > > Onda v975w at: > > https://github.com/hadess/gt9xx/commits/irina-tirdea > > > > Doing an "rmmod goodix ; insmod ./goodix_backport.ko" should be enough > > to test whether the patch set works. If it doesn't work correctly, > > you'll need to reboot the machine, swap the irq_idx and rst_idx values > > at: > > https://github.com/hadess/gt9xx/commit/c27de79f494c2b2e7a198ff4d27976ae93669dbd#diff- > dddc2849e36327439530f3e2faacec4fR321 > > > > and try again. > > Unswapped does fail with: > > sep 30 15:37:29 tablet kernel: Goodix-TS i2c-GDIX1001:00: i2c test > failed attempt 1: -121 > sep 30 15:37:29 tablet kernel: Goodix-TS i2c-GDIX1001:00: i2c test > failed attempt 2: -121 > sep 30 15:37:29 tablet kernel: Goodix-TS i2c-GDIX1001:00: I2C > communication failure: -121 > sep 30 15:37:29 tablet kernel: Goodix-TS: probe of i2c-GDIX1001:00 > failed with error -121 > > Swapping the values triggers some errors: > > sep 30 15:48:17 tablet kernel: [ cut here ] > sep 30 15:48:17 tablet kernel: WARNING: CPU: 1 PID: 2341 at > drivers/pinctrl/intel/pinctrl-baytrail.c:342 > byt_gpio_direction_output+0xa1/0xb0() > sep 30 15:48:17 tablet kernel: Potential Error: Setting GPIO with > direct_irq_en to output > sep 30 15:48:17 tablet kernel: Modules linked in: > sep 30 15:48:17 tablet kernel: goodix_backport(OE+) r8723bs(OE) nfsd > lockd grace sunrpc [last unloaded: goodix] > sep 30 15:48:17 tablet kernel: CPU: 1 PID: 2341 Comm: insmod Tainted: > G OE 4.3.0-rc1+ #36 > sep 30 15:48:17 tablet kernel: Hardware name: To be filled by O.E.M. > To be filled by O.E.M./Aptio CRB, BIOS 5.6.5 07/25/2014 > sep 30 15:48:17 tablet kernel: d3f61c08 c13b347f > d3f61c48 d3f61c38 c10a9e1d c20f10e0 > sep 30 15:48:17 tablet kernel: d3f61c64 0925 c20f111c 0156 > c13e7ce1 c13e7ce1 f80580b8 f53a4b0c > sep 30 15:48:17 tablet kernel: 0156 d3f61c50 c10a9e93 0009 > d3f61c48 c20f10e0 d3f61c64 d3f61c78 > sep 30 15:48:17 tablet kernel: Call Trace: > sep 30 15:48:17 tablet kernel: [] dump_stack+0x48/0x69 > sep 30 15:48:17 tablet kernel: [] warn_slowpath_common+0x8d/0xd0 > sep 30 15:48:17 tablet kernel: [] ? > byt_gpio_direction_output+0xa1/0xb0 > sep 30 15:48:17 tablet kernel: [] ? > byt_gpio_direction_output+0xa1/0xb0 > sep 30 15:48:17 tablet kernel: [] warn_slowpath_fmt+0x33/0x40 > sep 30 15:48:17 tablet kernel: [] > byt_gpio_direction_output+0xa1/0xb0 > sep 30 15:48:17 tablet kernel: [] ? byt_gpio_irq_handler+0xb0/0xb0 > sep 30 15:48:17 tablet kernel: [] > _gpiod_direction_output_raw+0x63/0x350 > sep 30 15:48:17 tablet kernel: [] gpiod_direction_output+0x2a/0x50 > sep 30 15:48:17 tablet kernel: [] ? msleep+0x36/0x40 > sep 30 15:48:17 tablet kernel: [] goodix_reset+0x3e/0x90 > [goodix_backport] > sep 30 15:48:17 tablet kernel: [] > goodix_ts_probe+0x12a/0x5aa [goodix_backport] > sep 30 15:48:17 tablet kernel: [] ? acpi_device_wakeup+0x7a/0x80 > sep 30 15:48:17 tablet kernel: [] ? acpi_dev_pm_attach+0x71/0x87 > sep 30 15:48:17 tablet kernel: [] i2c_device_probe+0x121/0x1d0 > sep 30 15:48:17 tablet kernel: [] ? _raw_spin_unlock+0x22/0x30 > sep 30 15:48:17 tablet kernel: [] ? > goodix_config_cb+0xc0/0xc0 [goodix_backport] > sep 30 15:48:17 tablet kernel:
Re: [PATCH v3 1/5] Input: goodix - reset device at init
Hey, On Wed, Sep 30, 2015 at 1:15 PM, Bastien Nocera wrote: > On Tue, 2015-09-29 at 17:47 +, Tirdea, Irina wrote: >> >> > -Original Message- >> > From: Bastien Nocera [mailto:had...@hadess.net] >> > Sent: 29 September, 2015 5:04 >> > To: Tirdea, Irina; linux-in...@vger.kernel.org >> > Cc: linux-kernel@vger.kernel.org; Rob Herring; Pawel Moll; Ian >> > Campbell; Kumar Gala; Purdila, Octavian; Dmitry Torokhov; Mark >> > Rutland; devicet...@vger.kernel.org >> > Subject: Re: [PATCH v3 1/5] Input: goodix - reset device at init >> > >> > On Fri, 2015-09-25 at 21:04 +, Tirdea, Irina wrote: >> > > >> >> > > >> > > The warning from your dmesg output will not cause probe to fail. >> > > If you look at the code for byt_gpio_direction_output, it will >> > > just >> > > print >> > > a warning and continue [1]: >> > > WARN(readl(conf_reg) & BYT_DIRECT_IRQ_EN, >> > > "Potential Error: Setting GPIO with direct_irq_en to >> > > output"); >> > > I thought probe finishes successfully, but due to the warning in >> > > dmesg you >> > > are not sure whether the IRQ GPIO pin can be used as output. >> > > If probe fails, it must be for another reason than the >> > > direct_irq_en >> > > warning. >> > > >> > > > 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? >> > > >> > > If the driver fails to initialize the GPIOs, it will at least >> > > print >> > > some >> > > "Failed to get GPIO" warnings in dmesg. Do you have such messages >> > > in >> > > dmesg or any additional information on why probe fails? >> > > >> > > The current code will ignore GPIOs if they are not defined in >> > > ACPI >> > > (see the check for -ENOENT), but does not ignore other error >> > > codes. >> > > If you want to bypass all GPIO errors, you can use the code >> > > below. >> > >> > The failure isn't there, it's when running goodix_i2c_test(): >> > Sep 25 16:39:20 winbook kernel: Goodix-TS i2c-GDIX1001:00: i2c test >> > failed attempt 1: -121 >> > Sep 25 16:39:20 winbook kernel: Goodix-TS i2c-GDIX1001:00: i2c test >> > failed attempt 2: -121 >> > Sep 25 16:39:20 winbook kernel: Goodix-TS i2c-GDIX1001:00: I2C >> > communication failure: -121 >> > Sep 25 16:39:20 winbook kernel: Goodix-TS: probe of i2c-GDIX1001:00 >> > failed with error -121 >> > >> >> Are you using v6 of the patches? There was an issue with reset that >> Aleksei reported >> and was fixed in v6 (although he had a different i2c error and a >> different scenario). > > Pretty certain. Your current patchset is at: > https://github.com/hadess/gt9xx/tree/irina-tirdea > > And the patches are yours, with the prefix and Documentation removed. > >> > The GPIO setup seems to work (bar the warnings), and the reset as >> > well, >> > but then the device fails to communicate. Likely a fallout from the >> > reset actually failing. >> > >> > Swapping around the RST and INT pins leads to the same problem. >> > Either >> > this device's GPIO PINs aren't actually functional, and the >> > firmware >> > contains garbage, or something else is wrong. >> > >> >> I agree. Either the interrupt pin cannot be used as output in your >> configuration >> or there are some specifics in the ACPI tables that prevent using >> these pins. >> >> > I'm not sure how we can detect, and blacklist, those devices. At >> > least >> > my original device, the Onda v975w, and the WinBook TW100 would >> > have >> > those problems. >> > >> >> I can use DMI quirks to exclude these devices from using the features >> that >> depend on the gpio pins. I already have the DMI information for >> WinBook TW100 >> and WinBook TW700. Could you tell me the DMI_SYS_VENDOR and >> DMI_PRODUCT_NAME for Onda v975w so I can add it as well? > > I don't have access to the Onda v975w anymore, but let me CC: a few > people that could also help with testing. > > Carlos, Cosimo, Christian, there's a patchset
Re: [PATCH v3 1/5] Input: goodix - reset device at init
On Tue, 2015-09-29 at 17:47 +, Tirdea, Irina wrote: > > > -Original Message- > > From: Bastien Nocera [mailto:had...@hadess.net] > > Sent: 29 September, 2015 5:04 > > To: Tirdea, Irina; linux-in...@vger.kernel.org > > Cc: linux-kernel@vger.kernel.org; Rob Herring; Pawel Moll; Ian > > Campbell; Kumar Gala; Purdila, Octavian; Dmitry Torokhov; Mark > > Rutland; devicet...@vger.kernel.org > > Subject: Re: [PATCH v3 1/5] Input: goodix - reset device at init > > > > On Fri, 2015-09-25 at 21:04 +, Tirdea, Irina wrote: > > > > > > > > > > The warning from your dmesg output will not cause probe to fail. > > > If you look at the code for byt_gpio_direction_output, it will > > > just > > > print > > > a warning and continue [1]: > > > WARN(readl(conf_reg) & BYT_DIRECT_IRQ_EN, > > > "Potential Error: Setting GPIO with direct_irq_en to > > > output"); > > > I thought probe finishes successfully, but due to the warning in > > > dmesg you > > > are not sure whether the IRQ GPIO pin can be used as output. > > > If probe fails, it must be for another reason than the > > > direct_irq_en > > > warning. > > > > > > > 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? > > > > > > If the driver fails to initialize the GPIOs, it will at least > > > print > > > some > > > "Failed to get GPIO" warnings in dmesg. Do you have such messages > > > in > > > dmesg or any additional information on why probe fails? > > > > > > The current code will ignore GPIOs if they are not defined in > > > ACPI > > > (see the check for -ENOENT), but does not ignore other error > > > codes. > > > If you want to bypass all GPIO errors, you can use the code > > > below. > > > > The failure isn't there, it's when running goodix_i2c_test(): > > Sep 25 16:39:20 winbook kernel: Goodix-TS i2c-GDIX1001:00: i2c test > > failed attempt 1: -121 > > Sep 25 16:39:20 winbook kernel: Goodix-TS i2c-GDIX1001:00: i2c test > > failed attempt 2: -121 > > Sep 25 16:39:20 winbook kernel: Goodix-TS i2c-GDIX1001:00: I2C > > communication failure: -121 > > Sep 25 16:39:20 winbook kernel: Goodix-TS: probe of i2c-GDIX1001:00 > > failed with error -121 > > > > Are you using v6 of the patches? There was an issue with reset that > Aleksei reported > and was fixed in v6 (although he had a different i2c error and a > different scenario). Pretty certain. Your current patchset is at: https://github.com/hadess/gt9xx/tree/irina-tirdea And the patches are yours, with the prefix and Documentation removed. > > The GPIO setup seems to work (bar the warnings), and the reset as > > well, > > but then the device fails to communicate. Likely a fallout from the > > reset actually failing. > > > > Swapping around the RST and INT pins leads to the same problem. > > Either > > this device's GPIO PINs aren't actually functional, and the > > firmware > > contains garbage, or something else is wrong. > > > > I agree. Either the interrupt pin cannot be used as output in your > configuration > or there are some specifics in the ACPI tables that prevent using > these pins. > > > I'm not sure how we can detect, and blacklist, those devices. At > > least > > my original device, the Onda v975w, and the WinBook TW100 would > > have > > those problems. > > > > I can use DMI quirks to exclude these devices from using the features > that > depend on the gpio pins. I already have the DMI information for > WinBook TW100 > and WinBook TW700. Could you tell me the DMI_SYS_VENDOR and > DMI_PRODUCT_NAME for Onda v975w so I can add it as well? I don't have access to the Onda v975w anymore, but let me CC: a few people that could also help with testing. Carlos, Cosimo, Christian, there's a patchset for you to test on the Onda v975w at: https://github.com/hadess/gt9xx/commits/irina-tirdea Doing an "rmmod goodix ; insmod ./goodix_backport.ko" should be enough to test whether the patch set works. If it doesn't work correctly, you'll need to reboot the machine, swap the irq_idx and rst_idx values at: https://github.com/hadess/gt9xx/commit/c27de79f494c2b2e7a198ff4d27976ae93669dbd#diff-dddc2849e36327439530f3e2faacec4fR321 and try again. If all that fails, could you please send the output of "dmidecode" to Irina? Cheers -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3 1/5] Input: goodix - reset device at init
On Tue, 2015-09-29 at 17:47 +, Tirdea, Irina wrote: > > > -Original Message- > > From: Bastien Nocera [mailto:had...@hadess.net] > > Sent: 29 September, 2015 5:04 > > To: Tirdea, Irina; linux-in...@vger.kernel.org > > Cc: linux-kernel@vger.kernel.org; Rob Herring; Pawel Moll; Ian > > Campbell; Kumar Gala; Purdila, Octavian; Dmitry Torokhov; Mark > > Rutland; devicet...@vger.kernel.org > > Subject: Re: [PATCH v3 1/5] Input: goodix - reset device at init > > > > On Fri, 2015-09-25 at 21:04 +, Tirdea, Irina wrote: > > > > > > > > > > The warning from your dmesg output will not cause probe to fail. > > > If you look at the code for byt_gpio_direction_output, it will > > > just > > > print > > > a warning and continue [1]: > > > WARN(readl(conf_reg) & BYT_DIRECT_IRQ_EN, > > > "Potential Error: Setting GPIO with direct_irq_en to > > > output"); > > > I thought probe finishes successfully, but due to the warning in > > > dmesg you > > > are not sure whether the IRQ GPIO pin can be used as output. > > > If probe fails, it must be for another reason than the > > > direct_irq_en > > > warning. > > > > > > > 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? > > > > > > If the driver fails to initialize the GPIOs, it will at least > > > print > > > some > > > "Failed to get GPIO" warnings in dmesg. Do you have such messages > > > in > > > dmesg or any additional information on why probe fails? > > > > > > The current code will ignore GPIOs if they are not defined in > > > ACPI > > > (see the check for -ENOENT), but does not ignore other error > > > codes. > > > If you want to bypass all GPIO errors, you can use the code > > > below. > > > > The failure isn't there, it's when running goodix_i2c_test(): > > Sep 25 16:39:20 winbook kernel: Goodix-TS i2c-GDIX1001:00: i2c test > > failed attempt 1: -121 > > Sep 25 16:39:20 winbook kernel: Goodix-TS i2c-GDIX1001:00: i2c test > > failed attempt 2: -121 > > Sep 25 16:39:20 winbook kernel: Goodix-TS i2c-GDIX1001:00: I2C > > communication failure: -121 > > Sep 25 16:39:20 winbook kernel: Goodix-TS: probe of i2c-GDIX1001:00 > > failed with error -121 > > > > Are you using v6 of the patches? There was an issue with reset that > Aleksei reported > and was fixed in v6 (although he had a different i2c error and a > different scenario). Pretty certain. Your current patchset is at: https://github.com/hadess/gt9xx/tree/irina-tirdea And the patches are yours, with the prefix and Documentation removed. > > The GPIO setup seems to work (bar the warnings), and the reset as > > well, > > but then the device fails to communicate. Likely a fallout from the > > reset actually failing. > > > > Swapping around the RST and INT pins leads to the same problem. > > Either > > this device's GPIO PINs aren't actually functional, and the > > firmware > > contains garbage, or something else is wrong. > > > > I agree. Either the interrupt pin cannot be used as output in your > configuration > or there are some specifics in the ACPI tables that prevent using > these pins. > > > I'm not sure how we can detect, and blacklist, those devices. At > > least > > my original device, the Onda v975w, and the WinBook TW100 would > > have > > those problems. > > > > I can use DMI quirks to exclude these devices from using the features > that > depend on the gpio pins. I already have the DMI information for > WinBook TW100 > and WinBook TW700. Could you tell me the DMI_SYS_VENDOR and > DMI_PRODUCT_NAME for Onda v975w so I can add it as well? I don't have access to the Onda v975w anymore, but let me CC: a few people that could also help with testing. Carlos, Cosimo, Christian, there's a patchset for you to test on the Onda v975w at: https://github.com/hadess/gt9xx/commits/irina-tirdea Doing an "rmmod goodix ; insmod ./goodix_backport.ko" should be enough to test whether the patch set works. If it doesn't work correctly, you'll need to reboot the machine, swap the irq_idx and rst_idx values at: https://github.com/hadess/gt9xx/commit/c27de79f494c2b2e7a198ff4d27976ae93669dbd#diff-dddc2849e36327439530f3e2faacec4fR321 and try again. If all that fails, could you please send the output of "dmidecode" to Irina? Cheers -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3 1/5] Input: goodix - reset device at init
Hey, On Wed, Sep 30, 2015 at 1:15 PM, Bastien Nocera <had...@hadess.net> wrote: > On Tue, 2015-09-29 at 17:47 +, Tirdea, Irina wrote: >> >> > -Original Message- >> > From: Bastien Nocera [mailto:had...@hadess.net] >> > Sent: 29 September, 2015 5:04 >> > To: Tirdea, Irina; linux-in...@vger.kernel.org >> > Cc: linux-kernel@vger.kernel.org; Rob Herring; Pawel Moll; Ian >> > Campbell; Kumar Gala; Purdila, Octavian; Dmitry Torokhov; Mark >> > Rutland; devicet...@vger.kernel.org >> > Subject: Re: [PATCH v3 1/5] Input: goodix - reset device at init >> > >> > On Fri, 2015-09-25 at 21:04 +, Tirdea, Irina wrote: >> > > >> >> > > >> > > The warning from your dmesg output will not cause probe to fail. >> > > If you look at the code for byt_gpio_direction_output, it will >> > > just >> > > print >> > > a warning and continue [1]: >> > > WARN(readl(conf_reg) & BYT_DIRECT_IRQ_EN, >> > > "Potential Error: Setting GPIO with direct_irq_en to >> > > output"); >> > > I thought probe finishes successfully, but due to the warning in >> > > dmesg you >> > > are not sure whether the IRQ GPIO pin can be used as output. >> > > If probe fails, it must be for another reason than the >> > > direct_irq_en >> > > warning. >> > > >> > > > 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? >> > > >> > > If the driver fails to initialize the GPIOs, it will at least >> > > print >> > > some >> > > "Failed to get GPIO" warnings in dmesg. Do you have such messages >> > > in >> > > dmesg or any additional information on why probe fails? >> > > >> > > The current code will ignore GPIOs if they are not defined in >> > > ACPI >> > > (see the check for -ENOENT), but does not ignore other error >> > > codes. >> > > If you want to bypass all GPIO errors, you can use the code >> > > below. >> > >> > The failure isn't there, it's when running goodix_i2c_test(): >> > Sep 25 16:39:20 winbook kernel: Goodix-TS i2c-GDIX1001:00: i2c test >> > failed attempt 1: -121 >> > Sep 25 16:39:20 winbook kernel: Goodix-TS i2c-GDIX1001:00: i2c test >> > failed attempt 2: -121 >> > Sep 25 16:39:20 winbook kernel: Goodix-TS i2c-GDIX1001:00: I2C >> > communication failure: -121 >> > Sep 25 16:39:20 winbook kernel: Goodix-TS: probe of i2c-GDIX1001:00 >> > failed with error -121 >> > >> >> Are you using v6 of the patches? There was an issue with reset that >> Aleksei reported >> and was fixed in v6 (although he had a different i2c error and a >> different scenario). > > Pretty certain. Your current patchset is at: > https://github.com/hadess/gt9xx/tree/irina-tirdea > > And the patches are yours, with the prefix and Documentation removed. > >> > The GPIO setup seems to work (bar the warnings), and the reset as >> > well, >> > but then the device fails to communicate. Likely a fallout from the >> > reset actually failing. >> > >> > Swapping around the RST and INT pins leads to the same problem. >> > Either >> > this device's GPIO PINs aren't actually functional, and the >> > firmware >> > contains garbage, or something else is wrong. >> > >> >> I agree. Either the interrupt pin cannot be used as output in your >> configuration >> or there are some specifics in the ACPI tables that prevent using >> these pins. >> >> > I'm not sure how we can detect, and blacklist, those devices. At >> > least >> > my original device, the Onda v975w, and the WinBook TW100 would >> > have >> > those problems. >> > >> >> I can use DMI quirks to exclude these devices from using the features >> that >> depend on the gpio pins. I already have the DMI information for >> WinBook TW100 >> and WinBook TW700. Could you tell me the DMI_SYS_VENDOR and >> DMI_PRODUCT_NAME for Onda v975w so I can add it as well? > > I don't have access to the Onda v975w anymore, but let me CC: a few > people that could also help with testing. > > Carlos, Cosimo, Christi
RE: [PATCH v3 1/5] Input: goodix - reset device at init
> -Original Message- > From: Bastien Nocera [mailto:had...@hadess.net] > Sent: 29 September, 2015 5:04 > To: Tirdea, Irina; linux-in...@vger.kernel.org > Cc: linux-kernel@vger.kernel.org; Rob Herring; Pawel Moll; Ian Campbell; > Kumar Gala; Purdila, Octavian; Dmitry Torokhov; Mark > Rutland; devicet...@vger.kernel.org > Subject: Re: [PATCH v3 1/5] Input: goodix - reset device at init > > On Fri, 2015-09-25 at 21:04 +, Tirdea, Irina wrote: > > > > > > The warning from your dmesg output will not cause probe to fail. > > If you look at the code for byt_gpio_direction_output, it will just > > print > > a warning and continue [1]: > > WARN(readl(conf_reg) & BYT_DIRECT_IRQ_EN, > > "Potential Error: Setting GPIO with direct_irq_en to > > output"); > > I thought probe finishes successfully, but due to the warning in > > dmesg you > > are not sure whether the IRQ GPIO pin can be used as output. > > If probe fails, it must be for another reason than the direct_irq_en > > warning. > > > > > 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? > > > > If the driver fails to initialize the GPIOs, it will at least print > > some > > "Failed to get GPIO" warnings in dmesg. Do you have such messages in > > dmesg or any additional information on why probe fails? > > > > The current code will ignore GPIOs if they are not defined in ACPI > > (see the check for -ENOENT), but does not ignore other error codes. > > If you want to bypass all GPIO errors, you can use the code below. > > The failure isn't there, it's when running goodix_i2c_test(): > Sep 25 16:39:20 winbook kernel: Goodix-TS i2c-GDIX1001:00: i2c test failed > attempt 1: -121 > Sep 25 16:39:20 winbook kernel: Goodix-TS i2c-GDIX1001:00: i2c test failed > attempt 2: -121 > Sep 25 16:39:20 winbook kernel: Goodix-TS i2c-GDIX1001:00: I2C communication > failure: -121 > Sep 25 16:39:20 winbook kernel: Goodix-TS: probe of i2c-GDIX1001:00 failed > with error -121 > Are you using v6 of the patches? There was an issue with reset that Aleksei reported and was fixed in v6 (although he had a different i2c error and a different scenario). > The GPIO setup seems to work (bar the warnings), and the reset as well, > but then the device fails to communicate. Likely a fallout from the > reset actually failing. > > Swapping around the RST and INT pins leads to the same problem. Either > this device's GPIO PINs aren't actually functional, and the firmware > contains garbage, or something else is wrong. > I agree. Either the interrupt pin cannot be used as output in your configuration or there are some specifics in the ACPI tables that prevent using these pins. > I'm not sure how we can detect, and blacklist, those devices. At least > my original device, the Onda v975w, and the WinBook TW100 would have > those problems. > I can use DMI quirks to exclude these devices from using the features that depend on the gpio pins. I already have the DMI information for WinBook TW100 and WinBook TW700. Could you tell me the DMI_SYS_VENDOR and DMI_PRODUCT_NAME for Onda v975w so I can add it as well? Thanks, Irina > Cheers
RE: [PATCH v3 1/5] Input: goodix - reset device at init
> -Original Message- > From: Bastien Nocera [mailto:had...@hadess.net] > Sent: 29 September, 2015 5:04 > To: Tirdea, Irina; linux-in...@vger.kernel.org > Cc: linux-kernel@vger.kernel.org; Rob Herring; Pawel Moll; Ian Campbell; > Kumar Gala; Purdila, Octavian; Dmitry Torokhov; Mark > Rutland; devicet...@vger.kernel.org > Subject: Re: [PATCH v3 1/5] Input: goodix - reset device at init > > On Fri, 2015-09-25 at 21:04 +, Tirdea, Irina wrote: > > > > > > The warning from your dmesg output will not cause probe to fail. > > If you look at the code for byt_gpio_direction_output, it will just > > print > > a warning and continue [1]: > > WARN(readl(conf_reg) & BYT_DIRECT_IRQ_EN, > > "Potential Error: Setting GPIO with direct_irq_en to > > output"); > > I thought probe finishes successfully, but due to the warning in > > dmesg you > > are not sure whether the IRQ GPIO pin can be used as output. > > If probe fails, it must be for another reason than the direct_irq_en > > warning. > > > > > 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? > > > > If the driver fails to initialize the GPIOs, it will at least print > > some > > "Failed to get GPIO" warnings in dmesg. Do you have such messages in > > dmesg or any additional information on why probe fails? > > > > The current code will ignore GPIOs if they are not defined in ACPI > > (see the check for -ENOENT), but does not ignore other error codes. > > If you want to bypass all GPIO errors, you can use the code below. > > The failure isn't there, it's when running goodix_i2c_test(): > Sep 25 16:39:20 winbook kernel: Goodix-TS i2c-GDIX1001:00: i2c test failed > attempt 1: -121 > Sep 25 16:39:20 winbook kernel: Goodix-TS i2c-GDIX1001:00: i2c test failed > attempt 2: -121 > Sep 25 16:39:20 winbook kernel: Goodix-TS i2c-GDIX1001:00: I2C communication > failure: -121 > Sep 25 16:39:20 winbook kernel: Goodix-TS: probe of i2c-GDIX1001:00 failed > with error -121 > Are you using v6 of the patches? There was an issue with reset that Aleksei reported and was fixed in v6 (although he had a different i2c error and a different scenario). > The GPIO setup seems to work (bar the warnings), and the reset as well, > but then the device fails to communicate. Likely a fallout from the > reset actually failing. > > Swapping around the RST and INT pins leads to the same problem. Either > this device's GPIO PINs aren't actually functional, and the firmware > contains garbage, or something else is wrong. > I agree. Either the interrupt pin cannot be used as output in your configuration or there are some specifics in the ACPI tables that prevent using these pins. > I'm not sure how we can detect, and blacklist, those devices. At least > my original device, the Onda v975w, and the WinBook TW100 would have > those problems. > I can use DMI quirks to exclude these devices from using the features that depend on the gpio pins. I already have the DMI information for WinBook TW100 and WinBook TW700. Could you tell me the DMI_SYS_VENDOR and DMI_PRODUCT_NAME for Onda v975w so I can add it as well? Thanks, Irina > Cheers
Re: [PATCH v3 1/5] Input: goodix - reset device at init
On Fri, 2015-09-25 at 21:04 +, Tirdea, Irina wrote: > > > -Original Message- > > From: Bastien Nocera [mailto:had...@hadess.net] > > Sent: 25 September, 2015 17:44 > > To: Tirdea, Irina; linux-in...@vger.kernel.org > > Cc: linux-kernel@vger.kernel.org; Rob Herring; Pawel Moll; Ian > > Campbell; Kumar Gala; Purdila, Octavian; Dmitry Torokhov; Mark > > Rutland; devicet...@vger.kernel.org > > Subject: Re: [PATCH v3 1/5] Input: goodix - reset device at init > > > > On Thu, 2015-09-10 at 14:04 +, Tirdea, Irina wrote: > > > > > > > -Original Message- > > > > From: Bastien Nocera [mailto:had...@hadess.net] > > > > Sent: 09 September, 2015 20:03 > > > > To: Tirdea, Irina; linux-in...@vger.kernel.org > > > > Cc: linux-kernel@vger.kernel.org; Rob Herring; Pawel Moll; Ian > > > > Campbell; Kumar Gala; Purdila, Octavian; Dmitry Torokhov; Mark > > > > Rutland; devicet...@vger.kernel.org > > > > Subject: Re: [PATCH v3 1/5] Input: goodix - reset device at > > > > init > > > > > > > > On Thu, 2015-07-30 at 11:27 +, 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 d
Re: [PATCH v3 1/5] Input: goodix - reset device at init
On Fri, 2015-09-25 at 21:04 +, Tirdea, Irina wrote: > > > -Original Message- > > From: Bastien Nocera [mailto:had...@hadess.net] > > Sent: 25 September, 2015 17:44 > > To: Tirdea, Irina; linux-in...@vger.kernel.org > > Cc: linux-kernel@vger.kernel.org; Rob Herring; Pawel Moll; Ian > > Campbell; Kumar Gala; Purdila, Octavian; Dmitry Torokhov; Mark > > Rutland; devicet...@vger.kernel.org > > Subject: Re: [PATCH v3 1/5] Input: goodix - reset device at init > > > > On Thu, 2015-09-10 at 14:04 +, Tirdea, Irina wrote: > > > > > > > -Original Message- > > > > From: Bastien Nocera [mailto:had...@hadess.net] > > > > Sent: 09 September, 2015 20:03 > > > > To: Tirdea, Irina; linux-in...@vger.kernel.org > > > > Cc: linux-kernel@vger.kernel.org; Rob Herring; Pawel Moll; Ian > > > > Campbell; Kumar Gala; Purdila, Octavian; Dmitry Torokhov; Mark > > > > Rutland; devicet...@vger.kernel.org > > > > Subject: Re: [PATCH v3 1/5] Input: goodix - reset device at > > > > init > > > > > > > > On Thu, 2015-07-30 at 11:27 +, 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 d
RE: [PATCH v3 1/5] Input: goodix - reset device at init
> -Original Message- > From: Bastien Nocera [mailto:had...@hadess.net] > Sent: 25 September, 2015 17:44 > To: Tirdea, Irina; linux-in...@vger.kernel.org > Cc: linux-kernel@vger.kernel.org; Rob Herring; Pawel Moll; Ian Campbell; > Kumar Gala; Purdila, Octavian; Dmitry Torokhov; Mark > Rutland; devicet...@vger.kernel.org > Subject: Re: [PATCH v3 1/5] Input: goodix - reset device at init > > On Thu, 2015-09-10 at 14:04 +, Tirdea, Irina wrote: > > > > > -Original Message- > > > From: Bastien Nocera [mailto:had...@hadess.net] > > > Sent: 09 September, 2015 20:03 > > > To: Tirdea, Irina; linux-in...@vger.kernel.org > > > Cc: linux-kernel@vger.kernel.org; Rob Herring; Pawel Moll; Ian > > > Campbell; Kumar Gala; Purdila, Octavian; Dmitry Torokhov; Mark > > > Rutland; devicet...@vger.kernel.org > > > Subject: Re: [PATCH v3 1/5] Input: goodix - reset device at init > > > > > > On Thu, 2015-07-30 at 11:27 +, 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. > The warning from your dmesg output will not cause probe to fail. If you look at the code for byt_gpio_direction_output, it will just print a warning and continue [1]: WARN(readl(conf_reg) & BYT_DIRECT_IRQ_EN, "Potential Error: Setting GPIO with direct_irq_en to output"); I thought probe finishes successfully, but due to the warning in dmesg you are not sure whether the IRQ GPIO pin can be used as output. If probe fails, it must be for another reason than the direct_irq_en warning. > 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? If the driver fails to i
Re: [PATCH v3 1/5] Input: goodix - reset device at init
On Thu, 2015-09-10 at 14:04 +, Tirdea, Irina wrote: > > > -Original Message- > > From: Bastien Nocera [mailto:had...@hadess.net] > > Sent: 09 September, 2015 20:03 > > To: Tirdea, Irina; linux-in...@vger.kernel.org > > Cc: linux-kernel@vger.kernel.org; Rob Herring; Pawel Moll; Ian > > Campbell; Kumar Gala; Purdila, Octavian; Dmitry Torokhov; Mark > > Rutland; devicet...@vger.kernel.org > > Subject: Re: [PATCH v3 1/5] Input: goodix - reset device at init > > > > On Thu, 2015-07-30 at 11:27 +, 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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3 1/5] Input: goodix - reset device at init
On Thu, 2015-09-10 at 14:04 +, Tirdea, Irina wrote: > > > -Original Message- > > From: Bastien Nocera [mailto:had...@hadess.net] > > Sent: 09 September, 2015 20:03 > > To: Tirdea, Irina; linux-in...@vger.kernel.org > > Cc: linux-kernel@vger.kernel.org; Rob Herring; Pawel Moll; Ian > > Campbell; Kumar Gala; Purdila, Octavian; Dmitry Torokhov; Mark > > Rutland; devicet...@vger.kernel.org > > Subject: Re: [PATCH v3 1/5] Input: goodix - reset device at init > > > > On Thu, 2015-07-30 at 11:27 +, 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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH v3 1/5] Input: goodix - reset device at init
> -Original Message- > From: Bastien Nocera [mailto:had...@hadess.net] > Sent: 25 September, 2015 17:44 > To: Tirdea, Irina; linux-in...@vger.kernel.org > Cc: linux-kernel@vger.kernel.org; Rob Herring; Pawel Moll; Ian Campbell; > Kumar Gala; Purdila, Octavian; Dmitry Torokhov; Mark > Rutland; devicet...@vger.kernel.org > Subject: Re: [PATCH v3 1/5] Input: goodix - reset device at init > > On Thu, 2015-09-10 at 14:04 +, Tirdea, Irina wrote: > > > > > -Original Message- > > > From: Bastien Nocera [mailto:had...@hadess.net] > > > Sent: 09 September, 2015 20:03 > > > To: Tirdea, Irina; linux-in...@vger.kernel.org > > > Cc: linux-kernel@vger.kernel.org; Rob Herring; Pawel Moll; Ian > > > Campbell; Kumar Gala; Purdila, Octavian; Dmitry Torokhov; Mark > > > Rutland; devicet...@vger.kernel.org > > > Subject: Re: [PATCH v3 1/5] Input: goodix - reset device at init > > > > > > On Thu, 2015-07-30 at 11:27 +, 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. > The warning from your dmesg output will not cause probe to fail. If you look at the code for byt_gpio_direction_output, it will just print a warning and continue [1]: WARN(readl(conf_reg) & BYT_DIRECT_IRQ_EN, "Potential Error: Setting GPIO with direct_irq_en to output"); I thought probe finishes successfully, but due to the warning in dmesg you are not sure whether the IRQ GPIO pin can be used as output. If probe fails, it must be for another reason than the direct_irq_en warning. > 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? If the driver fails to i
RE: [PATCH v3 1/5] Input: goodix - reset device at init
> -Original Message-
> From: Bastien Nocera [mailto:had...@hadess.net]
> Sent: 09 September, 2015 20:03
> To: Tirdea, Irina; linux-in...@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org; Rob Herring; Pawel Moll; Ian Campbell;
> Kumar Gala; Purdila, Octavian; Dmitry Torokhov; Mark
> Rutland; devicet...@vger.kernel.org
> Subject: Re: [PATCH v3 1/5] Input: goodix - reset device at init
>
> On Thu, 2015-07-30 at 11:27 +, 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.
To determine if using the interrupt pin as output works, you can test updating
the goodix configuration [4]. Normally if something goes wrong with the reset
procedure, the configuration will not be updated. To make that easier, I have
already included a sysfs interface to dump the current configuration [5] and
a script to help generate a new configuration [6] (with details on how to use it
in the commit message). Please let me know if you need something more to
test this.
Thanks,
Irina
[1]
https://drive.google.com/a/intel.com/folderview?id=0BxCVOQS3ZymGfmJyY2RKbE5XbVlKNlktVTlwV0lxNEdxd2dzeWZER094cmJPVnMxN1F0Yzg
[2] https://lkml.org/lkml/2014/6/2/654
[3] https://lkml.org/lkml/2014/9/18/129
[4] https://lkml.org/lkml/2015/9/7/339
[5] https://lkml.org/lkml/2015/9/7/337
[6] https://lkml.org/lkml/2015/7/31/706
N�r��yb�X��ǧv�^�){.n�+{zX����ܨ}���Ơz�:+v���zZ+��+zf���h���~i���z� �w���?�&�)ߢf��^jǫy�m��@A�a���
0��h���i
RE: [PATCH v3 1/5] Input: goodix - reset device at init
> -Original Message-
> From: Bastien Nocera [mailto:had...@hadess.net]
> Sent: 09 September, 2015 20:03
> To: Tirdea, Irina; linux-in...@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org; Rob Herring; Pawel Moll; Ian Campbell;
> Kumar Gala; Purdila, Octavian; Dmitry Torokhov; Mark
> Rutland; devicet...@vger.kernel.org
> Subject: Re: [PATCH v3 1/5] Input: goodix - reset device at init
>
> On Thu, 2015-07-30 at 11:27 +, 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.
To determine if using the interrupt pin as output works, you can test updating
the goodix configuration [4]. Normally if something goes wrong with the reset
procedure, the configuration will not be updated. To make that easier, I have
already included a sysfs interface to dump the current configuration [5] and
a script to help generate a new configuration [6] (with details on how to use it
in the commit message). Please let me know if you need something more to
test this.
Thanks,
Irina
[1]
https://drive.google.com/a/intel.com/folderview?id=0BxCVOQS3ZymGfmJyY2RKbE5XbVlKNlktVTlwV0lxNEdxd2dzeWZER094cmJPVnMxN1F0Yzg
[2] https://lkml.org/lkml/2014/6/2/654
[3] https://lkml.org/lkml/2014/9/18/129
[4] https://lkml.org/lkml/2015/9/7/339
[5] https://lkml.org/lkml/2015/9/7/337
[6] https://lkml.org/lkml/2015/7/31/706
N�r��yb�X��ǧv�^�){.n�+{zX����ܨ}���Ơz�:+v���zZ+��+zf���h���~i���z� �w���?�&�)ߢf��^jǫy�m��@A�a���
0��h���i
Re: [PATCH v3 1/5] Input: goodix - reset device at init
On Thu, 2015-07-30 at 11:27 +, 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 ;) Cheers -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3 1/5] Input: goodix - reset device at init
On Thu, 2015-07-30 at 11:27 +, 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 ;) Cheers -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH v3 1/5] Input: goodix - reset device at init
> -Original Message- > From: Bastien Nocera [mailto:had...@hadess.net] > Sent: 30 June, 2015 18:57 > To: Tirdea, Irina; Dmitry Torokhov; Mark Rutland; > linux-in...@vger.kernel.org; devicet...@vger.kernel.org > Cc: linux-kernel@vger.kernel.org; Rob Herring; Pawel Moll; Ian Campbell; > Kumar Gala; Purdila, Octavian > Subject: Re: [PATCH v3 1/5] Input: goodix - reset device at init > > On Mon, 2015-06-29 at 19:28 +0300, Irina Tirdea wrote: > > After power on, it is recommended that the driver resets the device. > > The reset procedure timing is described in the datasheet and is used > > at device init (before writing device configuration) and > > for power management. It is a sequence of setting the interrupt > > and reset pins high/low at specific timing intervals. This procedure > > also includes setting the slave address to the one specified in the > > ACPI/device tree. > > > > This is based on Goodix datasheets for GT911 and GT9271 and on Goodix > > driver gt9xx.c for Android (publicly available in Android kernel > > trees for various devices). > > > > For reset the driver needs to control the interrupt and > > reset gpio pins (configured through ACPI/device tree). For devices > > that do not have the gpio pins declared, the functionality depending > > on these pins will not be available, but the device can still be used > > with basic functionality. > > > I'm having a little trouble with this first patch, on a 4.2 "pre" Linus > tree and on a 4.1 kernel. > > [6.720214] [ cut here ] > [6.720230] WARNING: CPU: 2 PID: 475 at > drivers/pinctrl/intel/pinctrl-baytrail.c:338 > byt_gpio_direction_output+0x97/0xa0() > [6.720234] Potential Error: Setting GPIO with direct_irq_en to output > [6.720238] Modules linked in: > [6.720241] regmap_i2c intel_soc_dts_iosf int340x_thermal_zone soundcore > industrialio battery iosf_mbi acpi_thermal_rel > dell_smo8800 snd_soc_sst_acpi goodix_backport(OE+) i2c_hid acpi_pad ac > rfkill_gpio i2c_designware_platform rfkill > pwm_lpss_platform pwm_lpss i2c_designware_core nfsd auth_rpcgss nfs_acl lockd > grace sunrpc i915 mmc_block i2c_algo_bit > drm_kms_helper drm video sdhci_acpi sdhci mmc_core > [6.720292] CPU: 2 PID: 475 Comm: systemd-udevd Tainted: GW OE > 4.2.0-0.rc0.git2.2.fc22.i686 #1 > [6.720295] Hardware name: To be filled by O.E.M. To be filled by > O.E.M./Aptio CRB, BIOS 5.6.5 07/25/2014 > [6.720299] c0d39967 11197204 f6d37bc8 c0a9ce3c f6d37c08 > f6d37bf8 c045c677 > [6.720311] c0cb62a4 f6d37c28 01db c0cb62e0 0152 c073bfc7 > c073bfc7 f7c580b8 > [6.720321] f441409c f7c580b0 f6d37c14 c045c6ee 0009 f6d37c08 > c0cb62a4 f6d37c28 > [6.720331] Call Trace: > [6.720342] [] dump_stack+0x41/0x52 > [6.720349] [] warn_slowpath_common+0x87/0xc0 > [6.720355] [] ? byt_gpio_direction_output+0x97/0xa0 > [6.720360] [] ? byt_gpio_direction_output+0x97/0xa0 > [6.720365] [] warn_slowpath_fmt+0x3e/0x60 > [6.720370] [] byt_gpio_direction_output+0x97/0xa0 > [6.720376] [] ? byt_gpio_irq_handler+0xc0/0xc0 > [6.720382] [] _gpiod_direction_output_raw+0x59/0x1c0 > [6.720388] [] ? _raw_spin_unlock_irqrestore+0xd/0x10 > [6.720393] [] ? byt_gpio_direction_input+0x43/0x50 > [6.720398] [] ? byt_gpio_set+0x70/0x70 > [6.720404] [] gpiod_direction_output+0x2a/0x50 > [6.720413] [] goodix_ts_probe+0x2fb/0x5fd [goodix_backport] > [6.720422] [] i2c_device_probe+0x101/0x1b0 > [6.720428] [] ? sysfs_create_link+0x25/0x50 > [6.720436] [] ? goodix_ts_irq_handler+0x1f0/0x1f0 > [goodix_backport] > [6.720442] [] ? driver_sysfs_add+0x62/0x80 > [6.720448] [] driver_probe_device+0x1ca/0x460 > [6.720454] [] ? driver_probe_device+0x460/0x460 > [6.720461] [] ? acpi_driver_match_device+0x36/0x3f > [6.720467] [] __driver_attach+0x79/0x80 > [6.720473] [] ? driver_probe_device+0x460/0x460 > [6.720478] [] bus_for_each_dev+0x57/0xa0 > [6.720484] [] driver_attach+0x1e/0x20 > [6.720489] [] ? driver_probe_device+0x460/0x460 > [6.720494] [] bus_add_driver+0x1ef/0x290 > [6.720501] [] ? 0xf7ca7000 > [6.720506] [] ? 0xf7ca7000 > [6.720512] [] driver_register+0x5d/0xf0 > [6.720518] [] i2c_register_driver+0x2a/0xa0 > [6.720524] [] ? do_one_initcall+0x9f/0x200 > [6.720531] [] goodix_ts_driver_init+0x12/0x1000 > [goodix_backport] > [6.720536] [] do_one_initcall+0xaa/0x200 > [6.720541] [] ? 0xf7ca7000 > [6.720547] [] ? free_vmap_area_noflush+0x38/0x80 > [6.720554] [] ? kmem_cache_alloc_trace+0x175/0
RE: [PATCH v3 1/5] Input: goodix - reset device at init
-Original Message- From: Bastien Nocera [mailto:had...@hadess.net] Sent: 30 June, 2015 18:57 To: Tirdea, Irina; Dmitry Torokhov; Mark Rutland; linux-in...@vger.kernel.org; devicet...@vger.kernel.org Cc: linux-kernel@vger.kernel.org; Rob Herring; Pawel Moll; Ian Campbell; Kumar Gala; Purdila, Octavian Subject: Re: [PATCH v3 1/5] Input: goodix - reset device at init On Mon, 2015-06-29 at 19:28 +0300, Irina Tirdea wrote: After power on, it is recommended that the driver resets the device. The reset procedure timing is described in the datasheet and is used at device init (before writing device configuration) and for power management. It is a sequence of setting the interrupt and reset pins high/low at specific timing intervals. This procedure also includes setting the slave address to the one specified in the ACPI/device tree. This is based on Goodix datasheets for GT911 and GT9271 and on Goodix driver gt9xx.c for Android (publicly available in Android kernel trees for various devices). For reset the driver needs to control the interrupt and reset gpio pins (configured through ACPI/device tree). For devices that do not have the gpio pins declared, the functionality depending on these pins will not be available, but the device can still be used with basic functionality. I'm having a little trouble with this first patch, on a 4.2 pre Linus tree and on a 4.1 kernel. [6.720214] [ cut here ] [6.720230] WARNING: CPU: 2 PID: 475 at drivers/pinctrl/intel/pinctrl-baytrail.c:338 byt_gpio_direction_output+0x97/0xa0() [6.720234] Potential Error: Setting GPIO with direct_irq_en to output [6.720238] Modules linked in: [6.720241] regmap_i2c intel_soc_dts_iosf int340x_thermal_zone soundcore industrialio battery iosf_mbi acpi_thermal_rel dell_smo8800 snd_soc_sst_acpi goodix_backport(OE+) i2c_hid acpi_pad ac rfkill_gpio i2c_designware_platform rfkill pwm_lpss_platform pwm_lpss i2c_designware_core nfsd auth_rpcgss nfs_acl lockd grace sunrpc i915 mmc_block i2c_algo_bit drm_kms_helper drm video sdhci_acpi sdhci mmc_core [6.720292] CPU: 2 PID: 475 Comm: systemd-udevd Tainted: GW OE 4.2.0-0.rc0.git2.2.fc22.i686 #1 [6.720295] Hardware name: To be filled by O.E.M. To be filled by O.E.M./Aptio CRB, BIOS 5.6.5 07/25/2014 [6.720299] c0d39967 11197204 f6d37bc8 c0a9ce3c f6d37c08 f6d37bf8 c045c677 [6.720311] c0cb62a4 f6d37c28 01db c0cb62e0 0152 c073bfc7 c073bfc7 f7c580b8 [6.720321] f441409c f7c580b0 f6d37c14 c045c6ee 0009 f6d37c08 c0cb62a4 f6d37c28 [6.720331] Call Trace: [6.720342] [c0a9ce3c] dump_stack+0x41/0x52 [6.720349] [c045c677] warn_slowpath_common+0x87/0xc0 [6.720355] [c073bfc7] ? byt_gpio_direction_output+0x97/0xa0 [6.720360] [c073bfc7] ? byt_gpio_direction_output+0x97/0xa0 [6.720365] [c045c6ee] warn_slowpath_fmt+0x3e/0x60 [6.720370] [c073bfc7] byt_gpio_direction_output+0x97/0xa0 [6.720376] [c073bf30] ? byt_gpio_irq_handler+0xc0/0xc0 [6.720382] [c073e939] _gpiod_direction_output_raw+0x59/0x1c0 [6.720388] [c0aa202d] ? _raw_spin_unlock_irqrestore+0xd/0x10 [6.720393] [c073bb73] ? byt_gpio_direction_input+0x43/0x50 [6.720398] [c073bb30] ? byt_gpio_set+0x70/0x70 [6.720404] [c073eb0a] gpiod_direction_output+0x2a/0x50 [6.720413] [f7fe768b] goodix_ts_probe+0x2fb/0x5fd [goodix_backport] [6.720422] [c0909691] i2c_device_probe+0x101/0x1b0 [6.720428] [c0612e95] ? sysfs_create_link+0x25/0x50 [6.720436] [f7fe7390] ? goodix_ts_irq_handler+0x1f0/0x1f0 [goodix_backport] [6.720442] [c0819c52] ? driver_sysfs_add+0x62/0x80 [6.720448] [c081a56a] driver_probe_device+0x1ca/0x460 [6.720454] [c081a800] ? driver_probe_device+0x460/0x460 [6.720461] [c078d0b1] ? acpi_driver_match_device+0x36/0x3f [6.720467] [c081a879] __driver_attach+0x79/0x80 [6.720473] [c081a800] ? driver_probe_device+0x460/0x460 [6.720478] [c0818467] bus_for_each_dev+0x57/0xa0 [6.720484] [c0819dfe] driver_attach+0x1e/0x20 [6.720489] [c081a800] ? driver_probe_device+0x460/0x460 [6.720494] [c08199bf] bus_add_driver+0x1ef/0x290 [6.720501] [f7ca7000] ? 0xf7ca7000 [6.720506] [f7ca7000] ? 0xf7ca7000 [6.720512] [c081b07d] driver_register+0x5d/0xf0 [6.720518] [c090a3ca] i2c_register_driver+0x2a/0xa0 [6.720524] [c040046f] ? do_one_initcall+0x9f/0x200 [6.720531] [f7ca7012] goodix_ts_driver_init+0x12/0x1000 [goodix_backport] [6.720536] [c040047a] do_one_initcall+0xaa/0x200 [6.720541] [f7ca7000] ? 0xf7ca7000 [6.720547] [c05801c8] ? free_vmap_area_noflush+0x38/0x80 [6.720554] [c0594b95] ? kmem_cache_alloc_trace+0x175/0x1f0 [6.720560] [c0a9c64f] ? do_init_module+0x21/0x1a1 [6.720565] [c0a9c64f] ? do_init_module+0x21/0x1a1 [6.720572] [c0a9c67e] do_init_module+0x50/0x1a1
Re: [PATCH v3 1/5] Input: goodix - reset device at init
On Mon, 2015-06-29 at 19:28 +0300, Irina Tirdea wrote: > After power on, it is recommended that the driver resets the device. > The reset procedure timing is described in the datasheet and is used > at device init (before writing device configuration) and > for power management. It is a sequence of setting the interrupt > and reset pins high/low at specific timing intervals. This procedure > also includes setting the slave address to the one specified in the > ACPI/device tree. > > This is based on Goodix datasheets for GT911 and GT9271 and on Goodix > driver gt9xx.c for Android (publicly available in Android kernel > trees for various devices). > > For reset the driver needs to control the interrupt and > reset gpio pins (configured through ACPI/device tree). For devices > that do not have the gpio pins declared, the functionality depending > on these pins will not be available, but the device can still be used > with basic functionality. I'm having a little trouble with this first patch, on a 4.2 "pre" Linus tree and on a 4.1 kernel. [6.720214] [ cut here ] [6.720230] WARNING: CPU: 2 PID: 475 at drivers/pinctrl/intel/pinctrl-baytrail.c:338 byt_gpio_direction_output+0x97/0xa0() [6.720234] Potential Error: Setting GPIO with direct_irq_en to output [6.720238] Modules linked in: [6.720241] regmap_i2c intel_soc_dts_iosf int340x_thermal_zone soundcore industrialio battery iosf_mbi acpi_thermal_rel dell_smo8800 snd_soc_sst_acpi goodix_backport(OE+) i2c_hid acpi_pad ac rfkill_gpio i2c_designware_platform rfkill pwm_lpss_platform pwm_lpss i2c_designware_core nfsd auth_rpcgss nfs_acl lockd grace sunrpc i915 mmc_block i2c_algo_bit drm_kms_helper drm video sdhci_acpi sdhci mmc_core [6.720292] CPU: 2 PID: 475 Comm: systemd-udevd Tainted: GW OE 4.2.0-0.rc0.git2.2.fc22.i686 #1 [6.720295] Hardware name: To be filled by O.E.M. To be filled by O.E.M./Aptio CRB, BIOS 5.6.5 07/25/2014 [6.720299] c0d39967 11197204 f6d37bc8 c0a9ce3c f6d37c08 f6d37bf8 c045c677 [6.720311] c0cb62a4 f6d37c28 01db c0cb62e0 0152 c073bfc7 c073bfc7 f7c580b8 [6.720321] f441409c f7c580b0 f6d37c14 c045c6ee 0009 f6d37c08 c0cb62a4 f6d37c28 [6.720331] Call Trace: [6.720342] [] dump_stack+0x41/0x52 [6.720349] [] warn_slowpath_common+0x87/0xc0 [6.720355] [] ? byt_gpio_direction_output+0x97/0xa0 [6.720360] [] ? byt_gpio_direction_output+0x97/0xa0 [6.720365] [] warn_slowpath_fmt+0x3e/0x60 [6.720370] [] byt_gpio_direction_output+0x97/0xa0 [6.720376] [] ? byt_gpio_irq_handler+0xc0/0xc0 [6.720382] [] _gpiod_direction_output_raw+0x59/0x1c0 [6.720388] [] ? _raw_spin_unlock_irqrestore+0xd/0x10 [6.720393] [] ? byt_gpio_direction_input+0x43/0x50 [6.720398] [] ? byt_gpio_set+0x70/0x70 [6.720404] [] gpiod_direction_output+0x2a/0x50 [6.720413] [] goodix_ts_probe+0x2fb/0x5fd [goodix_backport] [6.720422] [] i2c_device_probe+0x101/0x1b0 [6.720428] [] ? sysfs_create_link+0x25/0x50 [6.720436] [] ? goodix_ts_irq_handler+0x1f0/0x1f0 [goodix_backport] [6.720442] [] ? driver_sysfs_add+0x62/0x80 [6.720448] [] driver_probe_device+0x1ca/0x460 [6.720454] [] ? driver_probe_device+0x460/0x460 [6.720461] [] ? acpi_driver_match_device+0x36/0x3f [6.720467] [] __driver_attach+0x79/0x80 [6.720473] [] ? driver_probe_device+0x460/0x460 [6.720478] [] bus_for_each_dev+0x57/0xa0 [6.720484] [] driver_attach+0x1e/0x20 [6.720489] [] ? driver_probe_device+0x460/0x460 [6.720494] [] bus_add_driver+0x1ef/0x290 [6.720501] [] ? 0xf7ca7000 [6.720506] [] ? 0xf7ca7000 [6.720512] [] driver_register+0x5d/0xf0 [6.720518] [] i2c_register_driver+0x2a/0xa0 [6.720524] [] ? do_one_initcall+0x9f/0x200 [6.720531] [] goodix_ts_driver_init+0x12/0x1000 [goodix_backport] [6.720536] [] do_one_initcall+0xaa/0x200 [6.720541] [] ? 0xf7ca7000 [6.720547] [] ? free_vmap_area_noflush+0x38/0x80 [6.720554] [] ? kmem_cache_alloc_trace+0x175/0x1f0 [6.720560] [] ? do_init_module+0x21/0x1a1 [6.720565] [] ? do_init_module+0x21/0x1a1 [6.720572] [] do_init_module+0x50/0x1a1 [6.720578] [] load_module+0x1ceb/0x22f0 [6.720585] [] ? copy_module_from_fd.isra.47+0xf9/0x190 [6.720592] [] SyS_finit_module+0xa5/0xf0 [6.720598] [] ? vm_mmap_pgoff+0x9b/0xc0 [6.720605] [] sysenter_do_call+0x12/0x12 [6.720610] ---[ end trace d5183b3e60f0f675 ]--- -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3 1/5] Input: goodix - reset device at init
On Mon, 2015-06-29 at 19:28 +0300, Irina Tirdea wrote: After power on, it is recommended that the driver resets the device. The reset procedure timing is described in the datasheet and is used at device init (before writing device configuration) and for power management. It is a sequence of setting the interrupt and reset pins high/low at specific timing intervals. This procedure also includes setting the slave address to the one specified in the ACPI/device tree. This is based on Goodix datasheets for GT911 and GT9271 and on Goodix driver gt9xx.c for Android (publicly available in Android kernel trees for various devices). For reset the driver needs to control the interrupt and reset gpio pins (configured through ACPI/device tree). For devices that do not have the gpio pins declared, the functionality depending on these pins will not be available, but the device can still be used with basic functionality. I'm having a little trouble with this first patch, on a 4.2 pre Linus tree and on a 4.1 kernel. [6.720214] [ cut here ] [6.720230] WARNING: CPU: 2 PID: 475 at drivers/pinctrl/intel/pinctrl-baytrail.c:338 byt_gpio_direction_output+0x97/0xa0() [6.720234] Potential Error: Setting GPIO with direct_irq_en to output [6.720238] Modules linked in: [6.720241] regmap_i2c intel_soc_dts_iosf int340x_thermal_zone soundcore industrialio battery iosf_mbi acpi_thermal_rel dell_smo8800 snd_soc_sst_acpi goodix_backport(OE+) i2c_hid acpi_pad ac rfkill_gpio i2c_designware_platform rfkill pwm_lpss_platform pwm_lpss i2c_designware_core nfsd auth_rpcgss nfs_acl lockd grace sunrpc i915 mmc_block i2c_algo_bit drm_kms_helper drm video sdhci_acpi sdhci mmc_core [6.720292] CPU: 2 PID: 475 Comm: systemd-udevd Tainted: GW OE 4.2.0-0.rc0.git2.2.fc22.i686 #1 [6.720295] Hardware name: To be filled by O.E.M. To be filled by O.E.M./Aptio CRB, BIOS 5.6.5 07/25/2014 [6.720299] c0d39967 11197204 f6d37bc8 c0a9ce3c f6d37c08 f6d37bf8 c045c677 [6.720311] c0cb62a4 f6d37c28 01db c0cb62e0 0152 c073bfc7 c073bfc7 f7c580b8 [6.720321] f441409c f7c580b0 f6d37c14 c045c6ee 0009 f6d37c08 c0cb62a4 f6d37c28 [6.720331] Call Trace: [6.720342] [c0a9ce3c] dump_stack+0x41/0x52 [6.720349] [c045c677] warn_slowpath_common+0x87/0xc0 [6.720355] [c073bfc7] ? byt_gpio_direction_output+0x97/0xa0 [6.720360] [c073bfc7] ? byt_gpio_direction_output+0x97/0xa0 [6.720365] [c045c6ee] warn_slowpath_fmt+0x3e/0x60 [6.720370] [c073bfc7] byt_gpio_direction_output+0x97/0xa0 [6.720376] [c073bf30] ? byt_gpio_irq_handler+0xc0/0xc0 [6.720382] [c073e939] _gpiod_direction_output_raw+0x59/0x1c0 [6.720388] [c0aa202d] ? _raw_spin_unlock_irqrestore+0xd/0x10 [6.720393] [c073bb73] ? byt_gpio_direction_input+0x43/0x50 [6.720398] [c073bb30] ? byt_gpio_set+0x70/0x70 [6.720404] [c073eb0a] gpiod_direction_output+0x2a/0x50 [6.720413] [f7fe768b] goodix_ts_probe+0x2fb/0x5fd [goodix_backport] [6.720422] [c0909691] i2c_device_probe+0x101/0x1b0 [6.720428] [c0612e95] ? sysfs_create_link+0x25/0x50 [6.720436] [f7fe7390] ? goodix_ts_irq_handler+0x1f0/0x1f0 [goodix_backport] [6.720442] [c0819c52] ? driver_sysfs_add+0x62/0x80 [6.720448] [c081a56a] driver_probe_device+0x1ca/0x460 [6.720454] [c081a800] ? driver_probe_device+0x460/0x460 [6.720461] [c078d0b1] ? acpi_driver_match_device+0x36/0x3f [6.720467] [c081a879] __driver_attach+0x79/0x80 [6.720473] [c081a800] ? driver_probe_device+0x460/0x460 [6.720478] [c0818467] bus_for_each_dev+0x57/0xa0 [6.720484] [c0819dfe] driver_attach+0x1e/0x20 [6.720489] [c081a800] ? driver_probe_device+0x460/0x460 [6.720494] [c08199bf] bus_add_driver+0x1ef/0x290 [6.720501] [f7ca7000] ? 0xf7ca7000 [6.720506] [f7ca7000] ? 0xf7ca7000 [6.720512] [c081b07d] driver_register+0x5d/0xf0 [6.720518] [c090a3ca] i2c_register_driver+0x2a/0xa0 [6.720524] [c040046f] ? do_one_initcall+0x9f/0x200 [6.720531] [f7ca7012] goodix_ts_driver_init+0x12/0x1000 [goodix_backport] [6.720536] [c040047a] do_one_initcall+0xaa/0x200 [6.720541] [f7ca7000] ? 0xf7ca7000 [6.720547] [c05801c8] ? free_vmap_area_noflush+0x38/0x80 [6.720554] [c0594b95] ? kmem_cache_alloc_trace+0x175/0x1f0 [6.720560] [c0a9c64f] ? do_init_module+0x21/0x1a1 [6.720565] [c0a9c64f] ? do_init_module+0x21/0x1a1 [6.720572] [c0a9c67e] do_init_module+0x50/0x1a1 [6.720578] [c04d917b] load_module+0x1ceb/0x22f0 [6.720585] [c04d6159] ? copy_module_from_fd.isra.47+0xf9/0x190 [6.720592] [c04d99a5] SyS_finit_module+0xa5/0xf0 [6.720598] [c056490b] ? vm_mmap_pgoff+0x9b/0xc0 [6.720605] [c0aa26df] sysenter_do_call+0x12/0x12 [6.720610] ---[ end trace d5183b3e60f0f675 ]--- -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More
