RE: [PATCH v5 3/9] Input: goodix - reset device at init

2015-09-15 Thread Tirdea, Irina 


> -Original Message-
> From: Aleksei Mamlin [mailto:mamli...@gmail.com]
> Sent: 15 September, 2015 12:48
> To: Tirdea, Irina
> Cc: Dmitry Torokhov; Bastien Nocera; linux-in...@vger.kernel.org; Mark 
> Rutland; Purdila, Octavian; linux-kernel@vger.kernel.org;
> devicet...@vger.kernel.org
> Subject: Re: [PATCH v5 3/9] Input: goodix - reset device at init
> 
> On Mon,  7 Sep 2015 17:36:17 +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.
> >
> > Signed-off-by: Octavian Purdila 
> > Signed-off-by: Irina Tirdea 
> > ---
> >  .../bindings/input/touchscreen/goodix.txt  |   5 +
> >  drivers/input/touchscreen/goodix.c | 136 
> > +
> >  2 files changed, 141 insertions(+)
> >

> > +/**
> > + * goodix_reset - Reset device during power on
> > + *
> > + * @ts: goodix_ts_data pointer
> > + */
> > +static int goodix_reset(struct goodix_ts_data *ts)
> > +{
> > +   int error;
> > +
> > +   /* begin select I2C slave addr */
> > +   error = gpiod_direction_output(ts->gpiod_rst, 0);
> > +   if (error)
> > +   return error;
> > +   msleep(20); /* T2: > 10ms */
> > +   /* HIGH: 0x28/0x29, LOW: 0xBA/0xBB */
> > +   error = gpiod_direction_output(ts->gpiod_int, ts->client->addr == 0x14);
> > +   if (error)
> > +   return error;
> > +   usleep_range(100, 2000);/* T3: > 100us */
> > +   error = gpiod_direction_output(ts->gpiod_rst, 1);
> > +   if (error)
> > +   return error;
> > +   usleep_range(6000, 1);  /* T4: > 5ms */
> > +   /* end select I2C slave addr */
> > +   error = gpiod_direction_input(ts->gpiod_rst);
> > +   if (error)
> > +   return error;
> > +   return goodix_int_sync(ts);
> > +}
> > +

> >  /**
> >   * goodix_read_config - Read the embedded configuration of the panel
> >   *
> > @@ -419,6 +542,19 @@ static int goodix_ts_probe(struct i2c_client *client,
> >
> > ts->cfg_len = goodix_get_cfg_len(id_info);
> >
> > +   error = goodix_get_gpio_config(ts, id);
> > +   if (error)
> > +   return error;
> > +
> > +   if (ts->gpiod_int && ts->gpiod_rst) {
> > +   /* reset the controller */
> > +   error = goodix_reset(ts);
> > +   if (error) {
> > +   dev_err(>dev, "Controller reset failed.\n");
> > +   return error;
> > +   }
> > +   }
> > +
> 
> On devices with devicetree, such as ARM tablets, we can set I2C address via 
> DT, so driver should reset controller and set right address.
> If we don't do this we get "I2C communication failure: -6".
>

This is exactly what this patch tries to do. The address set in ACPI or DT will 
be available
in ts->client->addr. The reset code checks for the address set by ACPI/DT and 
configures
the device to use that address.
 
> Also, most of touchscreen drivers tries to reset controllers before start 
> communicating, so we need do the same.

Good catch! goodix_reset should be called before goodix_i2c_test. I'll send a 
new version with this fix.

Thanks,
Irina

> 
> > goodix_read_config(ts);
> >
> > error = goodix_request_input_dev(ts, version_info, id_info);
> > --
> > 1.9.1
> >
> 
> 
> --
> Thanks and regards,
> Aleksei Mamlin
--
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 v5 3/9] Input: goodix - reset device at init

2015-09-15 Thread Aleksei Mamlin 
On Mon,  7 Sep 2015 17:36:17 +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.
> 
> Signed-off-by: Octavian Purdila 
> Signed-off-by: Irina Tirdea 
> ---
>  .../bindings/input/touchscreen/goodix.txt  |   5 +
>  drivers/input/touchscreen/goodix.c | 136 
> +
>  2 files changed, 141 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/input/touchscreen/goodix.txt 
> b/Documentation/devicetree/bindings/input/touchscreen/goodix.txt
> index 8ba98ee..c0715f8 100644
> --- a/Documentation/devicetree/bindings/input/touchscreen/goodix.txt
> +++ b/Documentation/devicetree/bindings/input/touchscreen/goodix.txt
> @@ -12,6 +12,8 @@ Required properties:
>   - reg   : I2C address of the chip. Should be 0x5d or 
> 0x14
>   - interrupt-parent  : Interrupt controller to which the chip is connected
>   - interrupts: Interrupt to which the chip is connected
> + - gpios : GPIOS the chip is connected to: first one is the
> +   interrupt gpio and second one the reset gpio.
>  
>  Example:
>  
> @@ -23,6 +25,9 @@ Example:
>   reg = <0x5d>;
>   interrupt-parent = <>;
>   interrupts = <0 0>;
> +
> + gpios = < 0 0>, /* INT */
> + < 1 0>; /* RST */
>   };
>  
>   /* ... */
> diff --git a/drivers/input/touchscreen/goodix.c 
> b/drivers/input/touchscreen/goodix.c
> index 7be6eab..8edfc06 100644
> --- a/drivers/input/touchscreen/goodix.c
> +++ b/drivers/input/touchscreen/goodix.c
> @@ -17,6 +17,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -37,6 +38,8 @@ struct goodix_ts_data {
>   unsigned int int_trigger_type;
>   bool rotated_screen;
>   int cfg_len;
> + struct gpio_desc *gpiod_int;
> + struct gpio_desc *gpiod_rst;
>  };
>  
>  #define GOODIX_MAX_HEIGHT4096
> @@ -89,6 +92,30 @@ static const struct dmi_system_id rotated_screen[] = {
>   {}
>  };
>  
> +/*
> + * ACPI table specifies gpio pins in this order: first rst pin and
> + * then interrupt pin.
> + */
> +static const struct dmi_system_id goodix_rst_pin_first[] = {
> +#if defined(CONFIG_DMI) && defined(CONFIG_X86)
> + {
> + .ident = "WinBook TW100",
> + .matches = {
> + DMI_MATCH(DMI_SYS_VENDOR, "WinBook"),
> + DMI_MATCH(DMI_PRODUCT_NAME, "TW100")
> + }
> + },
> + {
> + .ident = "WinBook TW700",
> + .matches = {
> + DMI_MATCH(DMI_SYS_VENDOR, "WinBook"),
> + DMI_MATCH(DMI_PRODUCT_NAME, "TW700")
> + },
> + },
> +#endif
> + {}
> +};
> +
>  /**
>   * goodix_i2c_read - read data from a register of the i2c slave device.
>   *
> @@ -237,6 +264,102 @@ static irqreturn_t goodix_ts_irq_handler(int irq, void 
> *dev_id)
>   return IRQ_HANDLED;
>  }
>  
> +static int goodix_int_sync(struct goodix_ts_data *ts)
> +{
> + int error;
> +
> + error = gpiod_direction_output(ts->gpiod_int, 0);
> + if (error)
> + return error;
> + msleep(50); /* T5: 50ms */
> +
> + return gpiod_direction_input(ts->gpiod_int);
> +}
> +
> +/**
> + * goodix_reset - Reset device during power on
> + *
> + * @ts: goodix_ts_data pointer
> + */
> +static int goodix_reset(struct goodix_ts_data *ts)
> +{
> + int error;
> +
> + /* begin select I2C slave addr */
> + error = gpiod_direction_output(ts->gpiod_rst, 0);
> + if (error)
> + return error;
> + msleep(20); /* T2: > 10ms */
> + /* HIGH: 0x28/0x29, LOW: 0xBA/0xBB */
> + error = gpiod_direction_output(ts->gpiod_int, ts->client->addr == 0x14);
> + if (error)
> + return error;
> + usleep_range(100, 2000);/* T3: > 100us */
> + error = gpiod_direction_output(ts->gpiod_rst, 1);
> + if (error)
> + return error;
> +

Re: [PATCH v5 3/9] Input: goodix - reset device at init

2015-09-15 Thread Aleksei Mamlin 
On Mon,  7 Sep 2015 17:36:17 +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.
> 
> Signed-off-by: Octavian Purdila 
> Signed-off-by: Irina Tirdea 
> ---
>  .../bindings/input/touchscreen/goodix.txt  |   5 +
>  drivers/input/touchscreen/goodix.c | 136 
> +
>  2 files changed, 141 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/input/touchscreen/goodix.txt 
> b/Documentation/devicetree/bindings/input/touchscreen/goodix.txt
> index 8ba98ee..c0715f8 100644
> --- a/Documentation/devicetree/bindings/input/touchscreen/goodix.txt
> +++ b/Documentation/devicetree/bindings/input/touchscreen/goodix.txt
> @@ -12,6 +12,8 @@ Required properties:
>   - reg   : I2C address of the chip. Should be 0x5d or 
> 0x14
>   - interrupt-parent  : Interrupt controller to which the chip is connected
>   - interrupts: Interrupt to which the chip is connected
> + - gpios : GPIOS the chip is connected to: first one is the
> +   interrupt gpio and second one the reset gpio.
>  
>  Example:
>  
> @@ -23,6 +25,9 @@ Example:
>   reg = <0x5d>;
>   interrupt-parent = <>;
>   interrupts = <0 0>;
> +
> + gpios = < 0 0>, /* INT */
> + < 1 0>; /* RST */
>   };
>  
>   /* ... */
> diff --git a/drivers/input/touchscreen/goodix.c 
> b/drivers/input/touchscreen/goodix.c
> index 7be6eab..8edfc06 100644
> --- a/drivers/input/touchscreen/goodix.c
> +++ b/drivers/input/touchscreen/goodix.c
> @@ -17,6 +17,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -37,6 +38,8 @@ struct goodix_ts_data {
>   unsigned int int_trigger_type;
>   bool rotated_screen;
>   int cfg_len;
> + struct gpio_desc *gpiod_int;
> + struct gpio_desc *gpiod_rst;
>  };
>  
>  #define GOODIX_MAX_HEIGHT4096
> @@ -89,6 +92,30 @@ static const struct dmi_system_id rotated_screen[] = {
>   {}
>  };
>  
> +/*
> + * ACPI table specifies gpio pins in this order: first rst pin and
> + * then interrupt pin.
> + */
> +static const struct dmi_system_id goodix_rst_pin_first[] = {
> +#if defined(CONFIG_DMI) && defined(CONFIG_X86)
> + {
> + .ident = "WinBook TW100",
> + .matches = {
> + DMI_MATCH(DMI_SYS_VENDOR, "WinBook"),
> + DMI_MATCH(DMI_PRODUCT_NAME, "TW100")
> + }
> + },
> + {
> + .ident = "WinBook TW700",
> + .matches = {
> + DMI_MATCH(DMI_SYS_VENDOR, "WinBook"),
> + DMI_MATCH(DMI_PRODUCT_NAME, "TW700")
> + },
> + },
> +#endif
> + {}
> +};
> +
>  /**
>   * goodix_i2c_read - read data from a register of the i2c slave device.
>   *
> @@ -237,6 +264,102 @@ static irqreturn_t goodix_ts_irq_handler(int irq, void 
> *dev_id)
>   return IRQ_HANDLED;
>  }
>  
> +static int goodix_int_sync(struct goodix_ts_data *ts)
> +{
> + int error;
> +
> + error = gpiod_direction_output(ts->gpiod_int, 0);
> + if (error)
> + return error;
> + msleep(50); /* T5: 50ms */
> +
> + return gpiod_direction_input(ts->gpiod_int);
> +}
> +
> +/**
> + * goodix_reset - Reset device during power on
> + *
> + * @ts: goodix_ts_data pointer
> + */
> +static int goodix_reset(struct goodix_ts_data *ts)
> +{
> + int error;
> +
> + /* begin select I2C slave addr */
> + error = gpiod_direction_output(ts->gpiod_rst, 0);
> + if (error)
> + return error;
> + msleep(20); /* T2: > 10ms */
> + /* HIGH: 0x28/0x29, LOW: 0xBA/0xBB */
> + error = gpiod_direction_output(ts->gpiod_int, ts->client->addr == 0x14);
> + if (error)
> + return error;
> + usleep_range(100, 2000);/* T3: > 100us */
> + error =

RE: [PATCH v5 3/9] Input: goodix - reset device at init

2015-09-15 Thread Tirdea, Irina 


> -Original Message-
> From: Aleksei Mamlin [mailto:mamli...@gmail.com]
> Sent: 15 September, 2015 12:48
> To: Tirdea, Irina
> Cc: Dmitry Torokhov; Bastien Nocera; linux-in...@vger.kernel.org; Mark 
> Rutland; Purdila, Octavian; linux-kernel@vger.kernel.org;
> devicet...@vger.kernel.org
> Subject: Re: [PATCH v5 3/9] Input: goodix - reset device at init
> 
> On Mon,  7 Sep 2015 17:36:17 +0300
> Irina Tirdea <irina.tir...@intel.com> 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.
> >
> > Signed-off-by: Octavian Purdila <octavian.purd...@intel.com>
> > Signed-off-by: Irina Tirdea <irina.tir...@intel.com>
> > ---
> >  .../bindings/input/touchscreen/goodix.txt  |   5 +
> >  drivers/input/touchscreen/goodix.c | 136 
> > +
> >  2 files changed, 141 insertions(+)
> >

> > +/**
> > + * goodix_reset - Reset device during power on
> > + *
> > + * @ts: goodix_ts_data pointer
> > + */
> > +static int goodix_reset(struct goodix_ts_data *ts)
> > +{
> > +   int error;
> > +
> > +   /* begin select I2C slave addr */
> > +   error = gpiod_direction_output(ts->gpiod_rst, 0);
> > +   if (error)
> > +   return error;
> > +   msleep(20); /* T2: > 10ms */
> > +   /* HIGH: 0x28/0x29, LOW: 0xBA/0xBB */
> > +   error = gpiod_direction_output(ts->gpiod_int, ts->client->addr == 0x14);
> > +   if (error)
> > +   return error;
> > +   usleep_range(100, 2000);/* T3: > 100us */
> > +   error = gpiod_direction_output(ts->gpiod_rst, 1);
> > +   if (error)
> > +   return error;
> > +   usleep_range(6000, 1);  /* T4: > 5ms */
> > +   /* end select I2C slave addr */
> > +   error = gpiod_direction_input(ts->gpiod_rst);
> > +   if (error)
> > +   return error;
> > +   return goodix_int_sync(ts);
> > +}
> > +

> >  /**
> >   * goodix_read_config - Read the embedded configuration of the panel
> >   *
> > @@ -419,6 +542,19 @@ static int goodix_ts_probe(struct i2c_client *client,
> >
> > ts->cfg_len = goodix_get_cfg_len(id_info);
> >
> > +   error = goodix_get_gpio_config(ts, id);
> > +   if (error)
> > +   return error;
> > +
> > +   if (ts->gpiod_int && ts->gpiod_rst) {
> > +   /* reset the controller */
> > +   error = goodix_reset(ts);
> > +   if (error) {
> > +   dev_err(>dev, "Controller reset failed.\n");
> > +   return error;
> > +   }
> > +   }
> > +
> 
> On devices with devicetree, such as ARM tablets, we can set I2C address via 
> DT, so driver should reset controller and set right address.
> If we don't do this we get "I2C communication failure: -6".
>

This is exactly what this patch tries to do. The address set in ACPI or DT will 
be available
in ts->client->addr. The reset code checks for the address set by ACPI/DT and 
configures
the device to use that address.
 
> Also, most of touchscreen drivers tries to reset controllers before start 
> communicating, so we need do the same.

Good catch! goodix_reset should be called before goodix_i2c_test. I'll send a 
new version with this fix.

Thanks,
Irina

> 
> > goodix_read_config(ts);
> >
> > error = goodix_request_input_dev(ts, version_info, id_info);
> > --
> > 1.9.1
> >
> 
> 
> --
> Thanks and regards,
> Aleksei Mamlin
--
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 v5 3/9] Input: goodix - reset device at init

2015-09-09 Thread Bastien Nocera 
On Mon, 2015-09-07 at 17:36 +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.

This throws:
Sep 09 18:22:47 winbook kernel: Goodix-TS i2c-GDIX1001:00: ID 9271, version: 
1040
Sep 09 18:22:47 winbook kernel: [ cut here ]
Sep 09 18:22:47 winbook kernel: WARNING: CPU: 3 PID: 3298 at 
drivers/pinctrl/intel/pinctrl-baytrail.c:338 
byt_gpio_direction_output+0x97/0xa0()
Sep 09 18:22:47 winbook kernel: Potential Error: Setting GPIO with 
direct_irq_en to output
Sep 09 18:22:47 winbook kernel: Modules linked in:
Sep 09 18:22:47 winbook kernel:  goodix_backport(OE+) hid_logitech_hidpp 
hid_logitech_dj cdc_mbim cdc_wdm cdc_ncm usbnet mii uvcvideo videobuf2_vmalloc 
videobuf2_core videobuf2_memops v4l2_common snd_usb_audio v
Sep 09 18:22:47 winbook kernel:  snd_soc_sst_byt_rt5640_mach coretemp 
snd_soc_sst_baytrail_pcm iTCO_vendor_support snd_soc_sst_ipc kvm_intel 
snd_soc_sst_dsp gpio_keys kvm snd_intel_sst_acpi snd_intel_sst_core sn
Sep 09 18:22:47 winbook kernel:  lockd grace sunrpc i915 mmc_block i2c_algo_bit 
drm_kms_helper drm sdhci_acpi video sdhci mmc_core i2c_hid [last unloaded: 
goodix]
Sep 09 18:22:47 winbook kernel: CPU: 3 PID: 3298 Comm: insmod Tainted: G
   OE   4.2.0-0.rc3.git4.2.fc22.i686 #1
Sep 09 18:22:47 winbook kernel: Hardware name: WinBook TW100/TW100, BIOS 
1.02.00 08/25/2014
Sep 09 18:22:47 winbook kernel:  c0d439a7 bb4c1aaf  de2f7bb4 c0aa23b9 
de2f7bf4 de2f7be4 c045c677
Sep 09 18:22:47 winbook kernel:  c0cbe3b8 de2f7c14 0ce2 c0cbe3f4 0152 
c073cd87 c073cd87 f7c5e0b8
Sep 09 18:22:47 winbook kernel:  f4bb309c f7c5e0b0 de2f7c00 c045c6ee 0009 
de2f7bf4 c0cbe3b8 de2f7c14
Sep 09 18:22:47 winbook kernel: Call Trace:
Sep 09 18:22:47 winbook kernel:  [] dump_stack+0x41/0x52
Sep 09 18:22:47 winbook kernel:  [] warn_slowpath_common+0x87/0xc0
Sep 09 18:22:47 winbook kernel:  [] ? 
byt_gpio_direction_output+0x97/0xa0
Sep 09 18:22:47 winbook kernel:  [] ? 
byt_gpio_direction_output+0x97/0xa0
Sep 09 18:22:47 winbook kernel:  [] warn_slowpath_fmt+0x3e/0x60
Sep 09 18:22:47 winbook kernel:  [] 
byt_gpio_direction_output+0x97/0xa0
Sep 09 18:22:47 winbook kernel:  [] ? byt_gpio_irq_handler+0xc0/0xc0
Sep 09 18:22:47 winbook kernel:  [] 
_gpiod_direction_output_raw+0x59/0x1c0
Sep 09 18:22:47 winbook kernel:  [] gpiod_direction_output+0x2a/0x50
Sep 09 18:22:47 winbook kernel:  [] ? msleep+0x2b/0x40
Sep 09 18:22:47 winbook kernel:  [] goodix_reset+0x3e/0x90 
[goodix_backport]
Sep 09 18:22:47 winbook kernel:  [] goodix_ts_probe+0x27e/0x5a0 
[goodix_backport]
Sep 09 18:22:47 winbook kernel:  [] i2c_device_probe+0x101/0x1b0
Sep 09 18:22:47 winbook kernel:  [] ? sysfs_create_link+0x25/0x50
Sep 09 18:22:47 winbook kernel:  [] ? 
goodix_configure_dev+0x1e0/0x1e0 [goodix_backport]

Which is the same error I had previously:
https://lkml.org/lkml/2015/6/30/434

I was testing this on a Onda v975w, but I'm now testing it on a WinBook
TW100.


> +/*
> + * ACPI table specifies gpio pins in this order: first rst pin and
> + * then interrupt pin.
> + */
> +static const struct dmi_system_id goodix_rst_pin_first[] = {
> +#if defined(CONFIG_DMI) && defined(CONFIG_X86)
> + {
> + .ident = "WinBook TW100",
> + .matches = {
> + DMI_MATCH(DMI_SYS_VENDOR, "WinBook"),
> + DMI_MATCH(DMI_PRODUCT_NAME, "TW100")
> + }
> + },

The DSDT for the WinBook one is here:
https://people.gnome.org/~hadess/Winbook%20TW100%20DSDT.dsl

For reference, the DSDT for the Onda, the tablet I tested this on some
months ago:
https://bugzilla.kernel.org/attachment.cgi?id=149331

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 v5 3/9] Input: goodix - reset device at init

2015-09-09 Thread Bastien Nocera 
On Mon, 2015-09-07 at 17:36 +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.

This throws:
Sep 09 18:22:47 winbook kernel: Goodix-TS i2c-GDIX1001:00: ID 9271, version: 
1040
Sep 09 18:22:47 winbook kernel: [ cut here ]
Sep 09 18:22:47 winbook kernel: WARNING: CPU: 3 PID: 3298 at 
drivers/pinctrl/intel/pinctrl-baytrail.c:338 
byt_gpio_direction_output+0x97/0xa0()
Sep 09 18:22:47 winbook kernel: Potential Error: Setting GPIO with 
direct_irq_en to output
Sep 09 18:22:47 winbook kernel: Modules linked in:
Sep 09 18:22:47 winbook kernel:  goodix_backport(OE+) hid_logitech_hidpp 
hid_logitech_dj cdc_mbim cdc_wdm cdc_ncm usbnet mii uvcvideo videobuf2_vmalloc 
videobuf2_core videobuf2_memops v4l2_common snd_usb_audio v
Sep 09 18:22:47 winbook kernel:  snd_soc_sst_byt_rt5640_mach coretemp 
snd_soc_sst_baytrail_pcm iTCO_vendor_support snd_soc_sst_ipc kvm_intel 
snd_soc_sst_dsp gpio_keys kvm snd_intel_sst_acpi snd_intel_sst_core sn
Sep 09 18:22:47 winbook kernel:  lockd grace sunrpc i915 mmc_block i2c_algo_bit 
drm_kms_helper drm sdhci_acpi video sdhci mmc_core i2c_hid [last unloaded: 
goodix]
Sep 09 18:22:47 winbook kernel: CPU: 3 PID: 3298 Comm: insmod Tainted: G
   OE   4.2.0-0.rc3.git4.2.fc22.i686 #1
Sep 09 18:22:47 winbook kernel: Hardware name: WinBook TW100/TW100, BIOS 
1.02.00 08/25/2014
Sep 09 18:22:47 winbook kernel:  c0d439a7 bb4c1aaf  de2f7bb4 c0aa23b9 
de2f7bf4 de2f7be4 c045c677
Sep 09 18:22:47 winbook kernel:  c0cbe3b8 de2f7c14 0ce2 c0cbe3f4 0152 
c073cd87 c073cd87 f7c5e0b8
Sep 09 18:22:47 winbook kernel:  f4bb309c f7c5e0b0 de2f7c00 c045c6ee 0009 
de2f7bf4 c0cbe3b8 de2f7c14
Sep 09 18:22:47 winbook kernel: Call Trace:
Sep 09 18:22:47 winbook kernel:  [] dump_stack+0x41/0x52
Sep 09 18:22:47 winbook kernel:  [] warn_slowpath_common+0x87/0xc0
Sep 09 18:22:47 winbook kernel:  [] ? 
byt_gpio_direction_output+0x97/0xa0
Sep 09 18:22:47 winbook kernel:  [] ? 
byt_gpio_direction_output+0x97/0xa0
Sep 09 18:22:47 winbook kernel:  [] warn_slowpath_fmt+0x3e/0x60
Sep 09 18:22:47 winbook kernel:  [] 
byt_gpio_direction_output+0x97/0xa0
Sep 09 18:22:47 winbook kernel:  [] ? byt_gpio_irq_handler+0xc0/0xc0
Sep 09 18:22:47 winbook kernel:  [] 
_gpiod_direction_output_raw+0x59/0x1c0
Sep 09 18:22:47 winbook kernel:  [] gpiod_direction_output+0x2a/0x50
Sep 09 18:22:47 winbook kernel:  [] ? msleep+0x2b/0x40
Sep 09 18:22:47 winbook kernel:  [] goodix_reset+0x3e/0x90 
[goodix_backport]
Sep 09 18:22:47 winbook kernel:  [] goodix_ts_probe+0x27e/0x5a0 
[goodix_backport]
Sep 09 18:22:47 winbook kernel:  [] i2c_device_probe+0x101/0x1b0
Sep 09 18:22:47 winbook kernel:  [] ? sysfs_create_link+0x25/0x50
Sep 09 18:22:47 winbook kernel:  [] ? 
goodix_configure_dev+0x1e0/0x1e0 [goodix_backport]

Which is the same error I had previously:
https://lkml.org/lkml/2015/6/30/434

I was testing this on a Onda v975w, but I'm now testing it on a WinBook
TW100.


> +/*
> + * ACPI table specifies gpio pins in this order: first rst pin and
> + * then interrupt pin.
> + */
> +static const struct dmi_system_id goodix_rst_pin_first[] = {
> +#if defined(CONFIG_DMI) && defined(CONFIG_X86)
> + {
> + .ident = "WinBook TW100",
> + .matches = {
> + DMI_MATCH(DMI_SYS_VENDOR, "WinBook"),
> + DMI_MATCH(DMI_PRODUCT_NAME, "TW100")
> + }
> + },

The DSDT for the WinBook one is here:
https://people.gnome.org/~hadess/Winbook%20TW100%20DSDT.dsl

For reference, the DSDT for the Onda, the tablet I tested this on some
months ago:
https://bugzilla.kernel.org/attachment.cgi?id=149331

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/