[PATCH v1 0/2] Add ili2900 timing

[PATCH v1 0/2] Add ili2900 timing

[lvzhaoxiong]

ILI2900 requires reset to pull down time greater than 10ms,
so the configuration post_power_delay_ms is 10, and the chipset
initial time is required to be greater than 100ms,
so the post_gpio_reset_on_delay_ms is set to 100.

lvzhaoxiong (2):
  dt-bindings: input: i2c-hid: Introduce Ilitek ili2900
  HID: i2c-hid: elan: Add ili2900 timing

 .../devicetree/bindings/input/ilitek,ili9882t.yaml        | 1 +
 drivers/hid/i2c-hid/i2c-hid-of-elan.c                     | 8 ++++++++
 2 files changed, 9 insertions(+)

-- 
2.17.1

[PATCH v1 1/2] dt-bindings: input: i2c-hid: Introduce Ilitek ili2900

[lvzhaoxiong]

The ili2900 touch screen chip same as ilitek ili9882t controller
has a reset gpio.

Signed-off-by: lvzhaoxiong <lvzhaoxiong@huaqin.corp-partner.google.com>
---
 Documentation/devicetree/bindings/input/ilitek,ili9882t.yaml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/input/ilitek,ili9882t.yaml b/Documentation/devicetree/bindings/input/ilitek,ili9882t.yaml
index c5d9e0e919f9..5063ea2c8375 100644
--- a/Documentation/devicetree/bindings/input/ilitek,ili9882t.yaml
+++ b/Documentation/devicetree/bindings/input/ilitek,ili9882t.yaml
@@ -19,6 +19,7 @@ allOf:
 properties:
   compatible:
     const: ilitek,ili9882t
+    const: ilitek,ili2900
 
   reg:
     const: 0x41
-- 
2.17.1

[PATCH v1 2/2] HID: i2c-hid: elan: Add ili2900 timing

[lvzhaoxiong]

ILI2900 requires reset to pull down time greater than 10ms,
so the configuration post_power_delay_ms is 10, and the chipset
initial time is required to be greater than 100ms,
so the post_gpio_reset_on_delay_ms is set to 100.

Signed-off-by: lvzhaoxiong <lvzhaoxiong@huaqin.corp-partner.google.com>
---
 drivers/hid/i2c-hid/i2c-hid-of-elan.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/hid/i2c-hid/i2c-hid-of-elan.c b/drivers/hid/i2c-hid/i2c-hid-of-elan.c
index 5b91fb106cfc..3073620b2dec 100644
--- a/drivers/hid/i2c-hid/i2c-hid-of-elan.c
+++ b/drivers/hid/i2c-hid/i2c-hid-of-elan.c
@@ -137,10 +137,18 @@ static const struct elan_i2c_hid_chip_data ilitek_ili2901_chip_data = {
 	.main_supply_name = "vcc33",
 };
 
+static const struct elan_i2c_hid_chip_data ilitek_ili2900_chip_data = {
+	.post_power_delay_ms = 10,
+	.post_gpio_reset_on_delay_ms = 100,
+	.hid_descriptor_address = 0x0001,
+	.main_supply_name = NULL,
+};
+
 static const struct of_device_id elan_i2c_hid_of_match[] = {
 	{ .compatible = "elan,ekth6915", .data = &elan_ekth6915_chip_data },
 	{ .compatible = "ilitek,ili9882t", .data = &ilitek_ili9882t_chip_data },
 	{ .compatible = "ilitek,ili2901", .data = &ilitek_ili2901_chip_data },
+	{ .compatible = "ilitek,ili2900", .data = &ilitek_ili2900_chip_data },
 	{ }
 };
 MODULE_DEVICE_TABLE(of, elan_i2c_hid_of_match);
-- 
2.17.1

Re: [PATCH v1 1/2] dt-bindings: input: i2c-hid: Introduce Ilitek ili2900

[Rob Herring]

On Thu, 18 Apr 2024 20:48:14 +0800, lvzhaoxiong wrote:
> The ili2900 touch screen chip same as ilitek ili9882t controller
> has a reset gpio.
> 
> Signed-off-by: lvzhaoxiong <lvzhaoxiong@huaqin.corp-partner.google.com>
> ---
>  Documentation/devicetree/bindings/input/ilitek,ili9882t.yaml | 1 +
>  1 file changed, 1 insertion(+)
> 

My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:
./Documentation/devicetree/bindings/input/ilitek,ili9882t.yaml:22:5: [error] duplication of key "const" in mapping (key-duplicates)

dtschema/dtc warnings/errors:
make[2]: *** Deleting file 'Documentation/devicetree/bindings/input/ilitek,ili9882t.example.dts'
Documentation/devicetree/bindings/input/ilitek,ili9882t.yaml:22:5: found duplicate key "const" with value "ilitek,ili2900" (original value: "ilitek,ili9882t")
make[2]: *** [Documentation/devicetree/bindings/Makefile:26: Documentation/devicetree/bindings/input/ilitek,ili9882t.example.dts] Error 1
make[2]: *** Waiting for unfinished jobs....
./Documentation/devicetree/bindings/input/ilitek,ili9882t.yaml:22:5: found duplicate key "const" with value "ilitek,ili2900" (original value: "ilitek,ili9882t")
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/input/ilitek,ili9882t.yaml: ignoring, error parsing file
make[1]: *** [/builds/robherring/dt-review-ci/linux/Makefile:1430: dt_binding_check] Error 2
make: *** [Makefile:240: __sub-make] Error 2

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20240418124815.31897-2-lvzhaoxiong@huaqin.corp-partner.google.com

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.

Re: [PATCH v1 1/2] dt-bindings: input: i2c-hid: Introduce Ilitek ili2900

[Krzysztof Kozlowski]

On 18/04/2024 14:48, lvzhaoxiong wrote:
> The ili2900 touch screen chip same as ilitek ili9882t controller
> has a reset gpio.
> 
> Signed-off-by: lvzhaoxiong <lvzhaoxiong@huaqin.corp-partner.google.com>

Except that this was not tested, please use full name, not login, if
possible.


Best regards,
Krzysztof

Re: [PATCH v1 1/2] dt-bindings: input: i2c-hid: Introduce Ilitek ili2900

[kernel test robot]

Hi lvzhaoxiong,

kernel test robot noticed the following build warnings:

[auto build test WARNING on hid/for-next]
[also build test WARNING on robh/for-next linus/master v6.9-rc4 next-20240418]
[cannot apply to dtor-input/next dtor-input/for-linus]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/lvzhaoxiong/dt-bindings-input-i2c-hid-Introduce-Ilitek-ili2900/20240418-205059
base:   https://git.kernel.org/pub/scm/linux/kernel/git/hid/hid.git for-next
patch link:    https://lore.kernel.org/r/20240418124815.31897-2-lvzhaoxiong%40huaqin.corp-partner.google.com
patch subject: [PATCH v1 1/2] dt-bindings: input: i2c-hid: Introduce Ilitek ili2900
compiler: loongarch64-linux-gcc (GCC) 13.2.0
dtschema version: 2024.3.dev14+g64b72b0
reproduce: (https://download.01.org/0day-ci/archive/20240419/202404190818.TnHkpNZd-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202404190818.TnHkpNZd-lkp@intel.com/

dtcheck warnings: (new ones prefixed by >>)
>> Documentation/devicetree/bindings/input/ilitek,ili9882t.yaml:22:5: [error] duplication of key "const" in mapping (key-duplicates)
--
   Documentation/devicetree/bindings/firmware/xilinx/xlnx,zynqmp-firmware.yaml:
   Error in referenced schema matching $id: http://devicetree.org/schemas/nvmem/xlnx,zynqmp-nvmem.yaml
>> Documentation/devicetree/bindings/input/ilitek,ili9882t.yaml:22:5: found duplicate key "const" with value "ilitek,ili2900" (original value: "ilitek,ili9882t")
--
>> Documentation/devicetree/bindings/input/ilitek,ili9882t.yaml: ignoring, error parsing file

vim +/const +22 Documentation/devicetree/bindings/input/ilitek,ili9882t.yaml

     8	
     9	maintainers:
    10	  - Cong Yang <yangcong5@huaqin.corp-partner.google.com>
    11	
    12	description:
    13	  Supports the Ilitek ili9882t touchscreen controller.
    14	  This touchscreen controller uses the i2c-hid protocol with a reset GPIO.
    15	
    16	allOf:
    17	  - $ref: /schemas/input/touchscreen/touchscreen.yaml#
    18	
    19	properties:
    20	  compatible:
    21	    const: ilitek,ili9882t
  > 22	    const: ilitek,ili2900
    23	
    24	  reg:
    25	    const: 0x41
    26	
    27	  interrupts:
    28	    maxItems: 1
    29	
    30	  panel: true
    31	
    32	  reset-gpios:
    33	    maxItems: 1
    34	    description: Reset GPIO.
    35	
    36	  vccio-supply:
    37	    description: The 1.8V supply to the touchscreen.
    38	
    39	required:
    40	  - compatible
    41	  - reg
    42	  - interrupts
    43	  - panel
    44	  - vccio-supply
    45	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v1 2/2] HID: i2c-hid: elan: Add ili2900 timing

[Doug Anderson]

Hi,

On Thu, Apr 18, 2024 at 5:48 AM lvzhaoxiong
<lvzhaoxiong@huaqin.corp-partner.google.com> wrote:
>
> ILI2900 requires reset to pull down time greater than 10ms,
> so the configuration post_power_delay_ms is 10, and the chipset
> initial time is required to be greater than 100ms,
> so the post_gpio_reset_on_delay_ms is set to 100.
>
> Signed-off-by: lvzhaoxiong <lvzhaoxiong@huaqin.corp-partner.google.com>

Please use a proper name instead of "lvzhaoxiong".


> ---
>  drivers/hid/i2c-hid/i2c-hid-of-elan.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/drivers/hid/i2c-hid/i2c-hid-of-elan.c b/drivers/hid/i2c-hid/i2c-hid-of-elan.c
> index 5b91fb106cfc..3073620b2dec 100644
> --- a/drivers/hid/i2c-hid/i2c-hid-of-elan.c
> +++ b/drivers/hid/i2c-hid/i2c-hid-of-elan.c
> @@ -137,10 +137,18 @@ static const struct elan_i2c_hid_chip_data ilitek_ili2901_chip_data = {
>         .main_supply_name = "vcc33",
>  };
>
> +static const struct elan_i2c_hid_chip_data ilitek_ili2900_chip_data = {
> +       .post_power_delay_ms = 10,
> +       .post_gpio_reset_on_delay_ms = 100,
> +       .hid_descriptor_address = 0x0001,
> +       .main_supply_name = NULL,

There's really no main power supply for this device? It feels likely
that there is one.

Also: other than the main power supply, there is no difference between
this and the ili2901. If you actually do have a main power supply,
then you probably don't need a new table. You probably don't even need
your own compatible string and in the device tree you could just
specify:

compatible = "ilitek,ili2900, "ilitek,ili2901";

...which says "I actually have an ILI 2900, but if you don't have any
special driver for the ILI 2900 it's likely that the driver for the
ILI 2901 will work because the hardware is almost the same."


> +};
> +
>  static const struct of_device_id elan_i2c_hid_of_match[] = {
>         { .compatible = "elan,ekth6915", .data = &elan_ekth6915_chip_data },
>         { .compatible = "ilitek,ili9882t", .data = &ilitek_ili9882t_chip_data },
>         { .compatible = "ilitek,ili2901", .data = &ilitek_ili2901_chip_data },
> +       { .compatible = "ilitek,ili2900", .data = &ilitek_ili2900_chip_data },

If you do need this, these should be sorted. 2900 should come before 2901.


-Doug

Re: [PATCH v1 2/2] HID: i2c-hid: elan: Add ili2900 timing

[zhaoxiong lv]

hi Doug

Sorry, This patch was accidentally sent when sending other patches.
Please ignore this patch.

>  Also: other than the main power supply, there is no difference between

> this and the ili2901. If you actually do have a main power supply,
> then you probably don't need a new table. You probably don't even need
> your own compatible string and in the device tree you could just
> specify:

> compatible = "ilitek,ili2900, "ilitek,ili2901";

> ...which says "I actually have an ILI 2900, but if you don't have any
> special driver for the ILI 2900 it's likely that the driver for the
> ILI 2901 will work because the hardware is almost the same."

In addition, in the previous patch, we have made modifications based
on your suggestions. "ilitek ili2900" and "ilitek ili2901" use the
same driver. Upstream may not be needed in the future. Thank you for
your previous suggestions.
[1] https://lore.kernel.org/r/CAD=FV=X5tk0tCcDa+vLnu0aoas1TDWuqvkMzM-278dOCX8K1gw@mail.gmail.com


On Thu, May 9, 2024 at 10:35 PM Doug Anderson <dianders@google.com> wrote:
>
> Hi,
>
> On Wed, May 8, 2024 at 11:43 PM Zhaoxiong Lv
> <lvzhaoxiong@huaqin.corp-partner.google.com> wrote:
> >
> > From: lvzhaoxiong <lvzhaoxiong@huaqin.corp-partner.google.com>
> >
> > ILI2900 requires reset to pull down time greater than 10ms,
> > so the configuration post_power_delay_ms is 10, and the chipset
> > initial time is required to be greater than 100ms,
> > so the post_gpio_reset_on_delay_ms is set to 100.
> >
> > Signed-off-by: lvzhaoxiong <lvzhaoxiong@huaqin.corp-partner.google.com>
> > ---
> >  drivers/hid/i2c-hid/i2c-hid-of-elan.c | 8 ++++++++
> >  1 file changed, 8 insertions(+)
>
> You silently ignored pretty much all of the feedback from the previous
> version [1], so I'm not planning to review this version.
>
> [1] https://lore.kernel.org/r/CAD=FV=X5tk0tCcDa+vLnu0aoas1TDWuqvkMzM-278dOCX8K1gw@mail.gmail.com

Re: [PATCH v1 2/2] HID: i2c-hid: elan: Add ili2900 timing

[Doug Anderson]

Hi,

On Wed, May 8, 2024 at 11:43 PM Zhaoxiong Lv
<lvzhaoxiong@huaqin.corp-partner.google.com> wrote:
>
> From: lvzhaoxiong <lvzhaoxiong@huaqin.corp-partner.google.com>
>
> ILI2900 requires reset to pull down time greater than 10ms,
> so the configuration post_power_delay_ms is 10, and the chipset
> initial time is required to be greater than 100ms,
> so the post_gpio_reset_on_delay_ms is set to 100.
>
> Signed-off-by: lvzhaoxiong <lvzhaoxiong@huaqin.corp-partner.google.com>
> ---
>  drivers/hid/i2c-hid/i2c-hid-of-elan.c | 8 ++++++++
>  1 file changed, 8 insertions(+)

You silently ignored pretty much all of the feedback from the previous
version [1], so I'm not planning to review this version.

[1] https://lore.kernel.org/r/CAD=FV=X5tk0tCcDa+vLnu0aoas1TDWuqvkMzM-278dOCX8K1gw@mail.gmail.com

[PATCH v1 2/2] HID: i2c-hid: elan: Add ili2900 timing

[Zhaoxiong Lv]

From: lvzhaoxiong <lvzhaoxiong@huaqin.corp-partner.google.com>

ILI2900 requires reset to pull down time greater than 10ms,
so the configuration post_power_delay_ms is 10, and the chipset
initial time is required to be greater than 100ms,
so the post_gpio_reset_on_delay_ms is set to 100.

Signed-off-by: lvzhaoxiong <lvzhaoxiong@huaqin.corp-partner.google.com>
---
 drivers/hid/i2c-hid/i2c-hid-of-elan.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/hid/i2c-hid/i2c-hid-of-elan.c b/drivers/hid/i2c-hid/i2c-hid-of-elan.c
index 5b91fb106cfc..3073620b2dec 100644
--- a/drivers/hid/i2c-hid/i2c-hid-of-elan.c
+++ b/drivers/hid/i2c-hid/i2c-hid-of-elan.c
@@ -137,10 +137,18 @@ static const struct elan_i2c_hid_chip_data ilitek_ili2901_chip_data = {
 	.main_supply_name = "vcc33",
 };
 
+static const struct elan_i2c_hid_chip_data ilitek_ili2900_chip_data = {
+	.post_power_delay_ms = 10,
+	.post_gpio_reset_on_delay_ms = 100,
+	.hid_descriptor_address = 0x0001,
+	.main_supply_name = NULL,
+};
+
 static const struct of_device_id elan_i2c_hid_of_match[] = {
 	{ .compatible = "elan,ekth6915", .data = &elan_ekth6915_chip_data },
 	{ .compatible = "ilitek,ili9882t", .data = &ilitek_ili9882t_chip_data },
 	{ .compatible = "ilitek,ili2901", .data = &ilitek_ili2901_chip_data },
+	{ .compatible = "ilitek,ili2900", .data = &ilitek_ili2900_chip_data },
 	{ }
 };
 MODULE_DEVICE_TABLE(of, elan_i2c_hid_of_match);
-- 
2.17.1