Linux Input/HID development
 help / color / mirror / Atom feed
* Re: [PATCH] HID: logitech-hidpp: Handle timeout differently from busy
From: Linux regression tracking (Thorsten Leemhuis) @ 2023-06-05  8:44 UTC (permalink / raw)
  To: Jiri Kosina, Bastien Nocera
  Cc: linux-input, linux-kernel, Benjamin Tissoires,
	Peter F . Patel-Schneider, Filipe Laíns, Nestor Lopez Casado,
	Mark Lord, Linux kernel regressions list
In-Reply-To: <nycvar.YFH.7.76.2306031440380.29760@cbobk.fhfr.pm>

On 03.06.23 14:41, Jiri Kosina wrote:
> On Wed, 31 May 2023, Jiri Kosina wrote:
> 
>>> If an attempt at contacting a receiver or a device fails because the
>>> receiver or device never responds, don't restart the communication, only
>>> restart it if the receiver or device answers that it's busy, as originally
>>> intended.
>>>
>>> This was the behaviour on communication timeout before commit 586e8fede795
>>> ("HID: logitech-hidpp: Retry commands when device is busy").
>>>
>>> This fixes some overly long waits in a critical path on boot, when
>>> checking whether the device is connected by getting its HID++ version.
>>>
>>> Signed-off-by: Bastien Nocera <hadess@hadess.net>
>>> Suggested-by: Mark Lord <mlord@pobox.com>
>>> Fixes: 586e8fede795 ("HID: logitech-hidpp: Retry commands when device is busy")
>>> Link: https://bugzilla.kernel.org/show_bug.cgi?id=217412
> [...]  
>>
>> I have applied this even before getting confirmation from the reporters in 
>> bugzilla, as it's the right thing to do anyway.
> 
> Unfortunately it doesn't seem to cure the reported issue (while reverting 
> 586e8fede79 does):

BTW, remind me again: was fixing this by reverting 586e8fede79 for now a
option? I guess it's not, but if I'm wrong I wonder if that might at
this point be the best way forward.

> https://bugzilla.kernel.org/show_bug.cgi?id=217523#c2

FWIW, another comment showed up there:

```
> --- Comment #6 from vova7890 ---
> Same problem. I researched this some time ago. I noticed that if I add a small
> delay between commands to the dongle - everything goes fine. Repeated
> request(586e8fede7953b1695b5ccc6112eff9b052e79ac) made the situation more
> visible
```

Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
--
Everything you wanna know about Linux kernel regression tracking:
https://linux-regtracking.leemhuis.info/about/#tldr
If I did something stupid, please tell me, as explained on that page.

#regzbot ^backmonitor:
https://lore.kernel.org/all/15e5d50f-95fc-c7c9-0918-015f24c6fc6d@leemhuis.info/

^ permalink raw reply

* [PATCH v2 2/2] HID: i2c-hid: elan: Add ili9882t timing
From: Cong Yang @ 2023-06-05  6:05 UTC (permalink / raw)
  To: robh+dt, krzysztof.kozlowski+dt, conor+dt, dmitry.torokhov, jikos,
	benjamin.tissoires, dianders, hsinyi
  Cc: linux-input, devicetree, linux-kernel, Cong Yang
In-Reply-To: <20230605060524.1178164-1-yangcong5@huaqin.corp-partner.google.com>

The ili9882t is a TDDI IC (Touch with Display Driver). The datasheet
specifies there should be 60ms between touch SDA sleep and panel RESX.
Doug's series[1] allows panels and touchscreens to power on/off together,
so we can add the 65 ms delay in i2c_hid_core_suspend before panel_unprepare.

[1]: https: //lore.kernel.org/all/20230523193017.4109557-1-dianders@chromium.org/

Signed-off-by: Cong Yang <yangcong5@huaqin.corp-partner.google.com>
---
 drivers/hid/i2c-hid/i2c-hid-of-elan.c | 20 ++++++++++++++++----
 1 file changed, 16 insertions(+), 4 deletions(-)

diff --git a/drivers/hid/i2c-hid/i2c-hid-of-elan.c b/drivers/hid/i2c-hid/i2c-hid-of-elan.c
index 76ddc8be1cbb..411d7ea2725d 100644
--- a/drivers/hid/i2c-hid/i2c-hid-of-elan.c
+++ b/drivers/hid/i2c-hid/i2c-hid-of-elan.c
@@ -18,7 +18,8 @@
 #include "i2c-hid.h"
 
 struct elan_i2c_hid_chip_data {
-	unsigned int post_gpio_reset_delay_ms;
+	unsigned int post_gpio_reset_on_delay_ms;
+	unsigned int post_gpio_reset_off_delay_ms;
 	unsigned int post_power_delay_ms;
 	u16 hid_descriptor_address;
 };
@@ -52,8 +53,8 @@ static int elan_i2c_hid_power_up(struct i2chid_ops *ops)
 		msleep(ihid_elan->chip_data->post_power_delay_ms);
 
 	gpiod_set_value_cansleep(ihid_elan->reset_gpio, 0);
-	if (ihid_elan->chip_data->post_gpio_reset_delay_ms)
-		msleep(ihid_elan->chip_data->post_gpio_reset_delay_ms);
+	if (ihid_elan->chip_data->post_gpio_reset_on_delay_ms)
+		msleep(ihid_elan->chip_data->post_gpio_reset_on_delay_ms);
 
 	return 0;
 }
@@ -64,6 +65,9 @@ static void elan_i2c_hid_power_down(struct i2chid_ops *ops)
 		container_of(ops, struct i2c_hid_of_elan, ops);
 
 	gpiod_set_value_cansleep(ihid_elan->reset_gpio, 1);
+	if (ihid_elan->chip_data->post_gpio_reset_off_delay_ms)
+		msleep(ihid_elan->chip_data->post_gpio_reset_off_delay_ms);
+
 	regulator_disable(ihid_elan->vccio);
 	regulator_disable(ihid_elan->vcc33);
 }
@@ -101,12 +105,20 @@ static int i2c_hid_of_elan_probe(struct i2c_client *client)
 
 static const struct elan_i2c_hid_chip_data elan_ekth6915_chip_data = {
 	.post_power_delay_ms = 1,
-	.post_gpio_reset_delay_ms = 300,
+	.post_gpio_reset_on_delay_ms = 300,
+	.hid_descriptor_address = 0x0001,
+};
+
+static const struct elan_i2c_hid_chip_data ilitek_ili9882t_chip_data = {
+	.post_power_delay_ms = 1,
+	.post_gpio_reset_on_delay_ms = 200,
+	.post_gpio_reset_off_delay_ms = 65,
 	.hid_descriptor_address = 0x0001,
 };
 
 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 },
 	{ }
 };
 MODULE_DEVICE_TABLE(of, elan_i2c_hid_of_match);
-- 
2.25.1


^ permalink raw reply related

* [PATCH v2 1/2] dt-bindings: input: touchscreen: Add ilitek 9882T touchscreen chip
From: Cong Yang @ 2023-06-05  6:05 UTC (permalink / raw)
  To: robh+dt, krzysztof.kozlowski+dt, conor+dt, dmitry.torokhov, jikos,
	benjamin.tissoires, dianders, hsinyi
  Cc: linux-input, devicetree, linux-kernel, Cong Yang
In-Reply-To: <20230605060524.1178164-1-yangcong5@huaqin.corp-partner.google.com>

Add an ilitek touch screen chip ili9882t.

Signed-off-by: Cong Yang <yangcong5@huaqin.corp-partner.google.com>
---
 .../bindings/input/elan,ekth6915.yaml         | 23 ++++++++++++++++---
 1 file changed, 20 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/input/elan,ekth6915.yaml b/Documentation/devicetree/bindings/input/elan,ekth6915.yaml
index 05e6f2df604c..f0e7ffdce605 100644
--- a/Documentation/devicetree/bindings/input/elan,ekth6915.yaml
+++ b/Documentation/devicetree/bindings/input/elan,ekth6915.yaml
@@ -15,11 +15,14 @@ description:
 
 properties:
   compatible:
-    items:
-      - const: elan,ekth6915
+    enum:
+      - elan,ekth6915
+      - ilitek,ili9882t
 
   reg:
-    const: 0x10
+    enum:
+      - 0x10
+      - 0x41
 
   interrupts:
     maxItems: 1
@@ -29,11 +32,13 @@ properties:
 
   vcc33-supply:
     description: The 3.3V supply to the touchscreen.
+                 If using ili9882t then this supply will not be needed.
 
   vccio-supply:
     description:
       The IO supply to the touchscreen. Need not be specified if this is the
       same as the 3.3V supply.
+      If using ili9882t, the IO supply is required.
 
 required:
   - compatible
@@ -41,6 +46,18 @@ required:
   - interrupts
   - vcc33-supply
 
+if:
+  properties:
+    compatible:
+      contains:
+        const: ilitek,ili9882t
+then:
+  required:
+    - compatible
+    - reg
+    - interrupts
+    - vccio-supply
+
 additionalProperties: false
 
 examples:
-- 
2.25.1


^ permalink raw reply related

* [PATCH v2 0/2] Add ili9882t timing
From: Cong Yang @ 2023-06-05  6:05 UTC (permalink / raw)
  To: robh+dt, krzysztof.kozlowski+dt, conor+dt, dmitry.torokhov, jikos,
	benjamin.tissoires, dianders, hsinyi
  Cc: linux-input, devicetree, linux-kernel, Cong Yang

Add ili9882t dt-bindings and timing

Changes in v2:
- PATCH 1/2: fix ran make dt_binding_check warnings/errors.
- PATCH 1/2: remove oneOf,just enum.
- Link to v1: https://lore.kernel.org/all/20230602140948.2138668-1-yangcong5@huaqin.corp-partner.google.com/

Cong Yang (2):
  dt-bindings: input: touchscreen: Add ilitek 9882T touchscreen chip
  HID: i2c-hid: elan: Add ili9882t timing

 .../bindings/input/elan,ekth6915.yaml         | 23 ++++++++++++++++---
 drivers/hid/i2c-hid/i2c-hid-of-elan.c         | 20 ++++++++++++----
 2 files changed, 36 insertions(+), 7 deletions(-)

-- 
2.25.1


^ permalink raw reply

* Re: [PATCH 1/2] dt-bindings: input: touchscreen: Add ilitek 9882T touchscreen chip
From: cong yang @ 2023-06-05  6:03 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: robh+dt, krzysztof.kozlowski+dt, conor+dt, dmitry.torokhov, jikos,
	benjamin.tissoires, dianders, hsinyi, linux-input, devicetree,
	linux-kernel
In-Reply-To: <afd610cf-954f-afc2-00da-86da9fe4192d@linaro.org>

Hi,Krzysztof,
   Thanks for Krzysztof.Sorry, I didn't run `make dt_binding_check. I
will check before sending V2 patch.

On Fri, Jun 2, 2023 at 10:19 PM Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 02/06/2023 16:09, Cong Yang wrote:
> > Add an ilitek touch screen chip ili9882t.
> >
> > Signed-off-by: Cong Yang <yangcong5@huaqin.corp-partner.google.com>
>
> It does not look like you tested the bindings, at least after quick
> look. Please run `make dt_binding_check` (see
> Documentation/devicetree/bindings/writing-schema.rst for instructions).
> Maybe you need to update your dtschema and yamllint.
>
> There is no way this would work and if you test you will see error msg.
> Feel free to ping if message is unclear.
>
> > ---
> >  .../bindings/input/elan,ekth6915.yaml         | 36 ++++++++++++++-----
> >  1 file changed, 27 insertions(+), 9 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/input/elan,ekth6915.yaml b/Documentation/devicetree/bindings/input/elan,ekth6915.yaml
> > index 05e6f2df604c..73e94cb6c4e0 100644
> > --- a/Documentation/devicetree/bindings/input/elan,ekth6915.yaml
> > +++ b/Documentation/devicetree/bindings/input/elan,ekth6915.yaml
> > @@ -15,11 +15,15 @@ description:
> >
> >  properties:
> >    compatible:
> > -    items:
> > -      - const: elan,ekth6915
> > +    oneOf:
>
> It's not oneOf. Just enum

Done,thanks.

>
> > +      - enum:
> > +        - elan,ekth6915
> > +        - ilitek,ili9882t
> >
> >    reg:
> > -    const: 0x10
> > +    enum:
> > +      - 0x10
> > +      - 0x41
> >
> >    interrupts:
> >      maxItems: 1
> > @@ -29,17 +33,31 @@ properties:
> >
> >    vcc33-supply:
> >      description: The 3.3V supply to the touchscreen.
> > +                 If using ili9882t then this supply will not be needed.
>
> Are you sure these are compatible devices then? What does it mean "not
> needed"? Is the pin there or is not?

Yes,ili9882t this touch ic not needed 3.3V supply,this pin does not exist.

>
> >
> >    vccio-supply:
> >      description:
> >        The IO supply to the touchscreen. Need not be specified if this is the
> >        same as the 3.3V supply.
> > -
> > -required:
> > -  - compatible
> > -  - reg
> > -  - interrupts
> > -  - vcc33-supply
> > +      If using ili9882t, the IO supply is required.
> > +
> > +  required:
>
> NAK. Really. Test patches before sending.
>
> Best regards,
> Krzysztof
>

^ permalink raw reply

* Re: [PATCH v1 28/43] input: keypad: ep93xx: add DT support for Cirrus EP93xx
From: Nikita Shubin @ 2023-06-04 19:14 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Alexander Sverdlin, Arnd Bergmann, Linus Walleij, Dmitry Torokhov,
	Jonathan Cameron, Michael Peters, Kris Bahnsen, linux-input,
	linux-kernel
In-Reply-To: <ZHjNR1n6tZbTAJWS@smile.fi.intel.com>

Hello Andy!

On Thu, 2023-06-01 at 19:54 +0300, Andy Shevchenko wrote:
> On Thu, Jun 01, 2023 at 08:45:33AM +0300, Nikita Shubin wrote:
> > - get keymap from the device tree
> > - find register range from the device tree
> > - get interrupts from device tree
> 
> ...
> 
> > +/* flags for the ep93xx_keypad driver */
> > +#define EP93XX_KEYPAD_DISABLE_3_KEY    (1<<0)  /* disable 3-key
> > reset */
> > +#define EP93XX_KEYPAD_DIAG_MODE                (1<<1)  /*
> > diagnostic mode */
> > +#define EP93XX_KEYPAD_BACK_DRIVE       (1<<2)  /* back driving
> > mode */
> > +#define EP93XX_KEYPAD_TEST_MODE                (1<<3)  /* scan
> > only column 0 */
> > +#define EP93XX_KEYPAD_AUTOREPEAT       (1<<4)  /* enable key
> > autorepeat */
> 
> > +static int ep93xx_keypad_flags;
> > +module_param(ep93xx_keypad_flags, int, 0);
> > +MODULE_PARM_DESC(ep93xx_keypad_flags, "EP93XX keypad flags.");
> 
> Why? This pretty much looks like missing DT description.

From other patches from this series, i learned NOT to add new DT
entities, not even with vendor prefix, no way!

May be i missing something of course...

Either way 

https://elixir.bootlin.com/linux/v6.4-rc4/source/arch/arm/mach-ep93xx/core.c#L577

static struct ep93xx_keypad_platform_data ep93xx_keypad_data;

Was never used in different ways than initializing all to zeroes
including flags since 2.6 (didn't look before through), so i would
prefer to drop this completely than moving it into device tree.

May we should drop ep93xx_keypad entirely, i don't have hardware to
test it anyway, neither does Alexander.


> 
> Please, write your commit message better, so we can understand the
> point of
> such decisions w/o asking.
> 


^ permalink raw reply

* Re: [PATCH v1 00/43] ep93xx device tree conversion
From: Alexander Sverdlin @ 2023-06-04 15:54 UTC (permalink / raw)
  To: Nikita Shubin, Arnd Bergmann, Linus Walleij, Alexander Gordeev,
	Alexandre Belloni, Andy Shevchenko, Bartosz Golaszewski,
	Christophe Kerello, Conor Dooley, Dmitry Torokhov,
	Emil Renner Berthing, Florian Fainelli, Hartley Sweeten,
	Heiko Stuebner, Hitomi Hasegawa, Jean Delvare, Joel Stanley,
	Jonathan Cameron, Jonathan Neuschäfer, Krzysztof Kozlowski,
	Le Moal, Liang Yang, Mark Brown, Masahiro Yamada, Miquel Raynal,
	Nathan Chancellor, Neil Armstrong, Nick Desaulniers,
	Nicolas Ferre, Nicolas Saenz Julienne, Richard Weinberger,
	Russell King (Oracle), Sergey Shtylyov, Uwe Kleine-König,
	Vasily Gorbik, Walker Chen, Yinbo Zhu
  Cc: Michael Peters, Kris Bahnsen, alsa-devel, devicetree, dmaengine,
	linux-arm-kernel, linux-clk, linux-gpio, linux-ide, linux-input,
	linux-kernel, linux-mtd, linux-pm, linux-pwm, linux-rtc,
	linux-spi, linux-watchdog, netdev
In-Reply-To: <20230601053546.9574-1-nikita.shubin@maquefel.me>

Hi Nikita,

On Thu, 2023-06-01 at 08:33 +0300, Nikita Shubin wrote:
> This series aims to convert ep93xx from platform to full device tree support.
> 
> Alexander, Kris - there are some significant changes in clk and pinctrl so can i ask you to tests all once again.

I have quickly tested network and sound on EDB9302 and I neither have problems with
these functions, nor did I spot any new error messages, overall looks good to me,
thanks for your efforts!

-- 
Alexander Sverdlin.


^ permalink raw reply

* Re: [PATCH v4 1/4] Input: ads7846 - Convert to use software nodes
From: Christophe JAILLET @ 2023-06-04 15:44 UTC (permalink / raw)
  To: Linus Walleij, Aaro Koskinen, Janusz Krzysztofik, Tony Lindgren,
	Russell King, Daniel Mack, Haojian Zhuang, Robert Jarzmik,
	Thomas Bogendoerfer, Dmitry Torokhov, Mark Brown,
	Bartosz Golaszewski, Andreas Kemnade, Helge Deller, Ulf Hansson
  Cc: linux-omap, linux-arm-kernel, linux-kernel, linux-mips,
	linux-input, linux-spi, linux-fbdev, dri-devel, linux-mmc
In-Reply-To: <20230430-nokia770-regression-v4-1-9b6dc5536b17@linaro.org>

Le 08/05/2023 à 23:20, Linus Walleij a écrit :
> The Nokia 770 is using GPIOs from the global numberspace on the
> CBUS node to pass down to the LCD controller. This regresses when we
> let the OMAP GPIO driver use dynamic GPIO base.
> 
> The Nokia 770 now has dynamic allocation of IRQ numbers, so this
> needs to be fixed for it to work.
> 
> As this is the only user of LCD MIPID we can easily augment the
> driver to use a GPIO descriptor instead and resolve the issue.
> 
> The platform data .shutdown() callback wasn't even used in the
> code, but we encode a shutdown asserting RESET in the remove()
> callback for completeness sake.
> 
> The CBUS also has the ADS7846 touchscreen attached.
> 
> Populate the devices on the Nokia 770 CBUS I2C using software
> nodes instead of platform data quirks. This includes the LCD
> and the ADS7846 touchscreen so the conversion just brings the LCD
> along with it as software nodes is an all-or-nothing design
> pattern.
> 
> The ADS7846 has some limited support for using GPIO descriptors,
> let's convert it over completely to using device properties and then
> fix all remaining boardfile users to provide all platform data using
> software nodes.
> 
> Dump the of includes and of_match_ptr() in the ADS7846 driver as part
> of the job.
> 
> Since we have to move ADS7846 over to obtaining the GPIOs it is
> using exclusively from descriptors, we provide descriptor tables
> for the two remaining in-kernel boardfiles using ADS7846:
> 
> - PXA Spitz
> - MIPS Alchemy DB1000 development board
> 
> It was too hard for me to include software node conversion of
> these two remaining users at this time: the spitz is using a
> hscync callback in the platform data that would require further
> GPIO descriptor conversion of the Spitz, and moving the hsync
> callback down into the driver: it will just become too big of
> a job, but it can be done separately.
> 
> The MIPS Alchemy DB1000 is simply something I cannot test, so take
> the easier approach of just providing some GPIO descriptors in
> this case as I don't want the patch to grow too intrusive.
> 
> As we see that several device trees have incorrect polarity flags
> and just expect to bypass the gpiolib polarity handling, fix up
> all device trees too, in a separate patch.
> 
> Suggested-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> Fixes: 92bf78b33b0b ("gpio: omap: use dynamic allocation of base")
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---

[...]

> diff --git a/drivers/video/fbdev/omap/lcd_mipid.c b/drivers/video/fbdev/omap/lcd_mipid.c
> index 03cff39d392d..e4a7f0b824ff 100644
> --- a/drivers/video/fbdev/omap/lcd_mipid.c
> +++ b/drivers/video/fbdev/omap/lcd_mipid.c
> @@ -7,6 +7,7 @@
>    */
>   #include <linux/device.h>
>   #include <linux/delay.h>
> +#include <linux/gpio/consumer.h>
>   #include <linux/slab.h>
>   #include <linux/workqueue.h>
>   #include <linux/spi/spi.h>
> @@ -41,6 +42,7 @@ struct mipid_device {
>   						   when we can issue the
>   						   next sleep in/out command */
>   	unsigned long	hw_guard_wait;		/* max guard time in jiffies */
> +	struct gpio_desc	*reset;
>   
>   	struct omapfb_device	*fbdev;
>   	struct spi_device	*spi;
> @@ -556,6 +558,12 @@ static int mipid_spi_probe(struct spi_device *spi)
>   		return -ENOMEM;
>   	}
>   
> +	/* This will de-assert RESET if active */
> +	md->reset = gpiod_get(&spi->dev, "reset", GPIOD_OUT_LOW);
> +	if (IS_ERR(md->reset))
> +		return dev_err_probe(&spi->dev, PTR_ERR(md->reset),
> +				     "no reset GPIO line\n");
> +
>   	spi->mode = SPI_MODE_0;
>   	md->spi = spi;
>   	dev_set_drvdata(&spi->dev, md);
> @@ -574,6 +582,8 @@ static void mipid_spi_remove(struct spi_device *spi)
>   {
>   	struct mipid_device *md = dev_get_drvdata(&spi->dev);
>   
> +	/* Asserts RESET */
> +	gpiod_set_value(md->reset, 1);

Hi,

should this also be done in the probe if mipid_detect() fails?

If yes, please also look at [1], that I've just sent, which introduces 
an error handling path in the probe.

CJ

[1]: 
https://lore.kernel.org/all/8b82e34724755b69f34f15dddb288cd373080390.1620505229.git.christophe.jaillet@wanadoo.fr/

>   	mipid_disable(&md->panel);
>   	kfree(md);
>   }

[...]

^ permalink raw reply

* Re: [PATCH] HID: logitech-hidpp: Handle timeout differently from busy
From: Mark Lord @ 2023-06-03 13:17 UTC (permalink / raw)
  To: Jiri Kosina, Bastien Nocera
  Cc: linux-input, linux-kernel, Benjamin Tissoires,
	Peter F . Patel-Schneider, Filipe Laíns, Nestor Lopez Casado
In-Reply-To: <nycvar.YFH.7.76.2306031440380.29760@cbobk.fhfr.pm>

On 2023-06-03 08:41 AM, Jiri Kosina wrote:
> On Wed, 31 May 2023, Jiri Kosina wrote:
> 
>>> If an attempt at contacting a receiver or a device fails because the
>>> receiver or device never responds, don't restart the communication, only
>>> restart it if the receiver or device answers that it's busy, as originally
>>> intended.
>>>
>>> This was the behaviour on communication timeout before commit 586e8fede795
>>> ("HID: logitech-hidpp: Retry commands when device is busy").
>>>
>>> This fixes some overly long waits in a critical path on boot, when
>>> checking whether the device is connected by getting its HID++ version.
>>>
>>> Signed-off-by: Bastien Nocera <hadess@hadess.net>
>>> Suggested-by: Mark Lord <mlord@pobox.com>
>>> Fixes: 586e8fede795 ("HID: logitech-hidpp: Retry commands when device is busy")
>>> Link: https://bugzilla.kernel.org/show_bug.cgi?id=217412
>>> ---
>>>  drivers/hid/hid-logitech-hidpp.c | 1 +
>>>  1 file changed, 1 insertion(+)
>>>
>>> diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
>>> index 0fcfd85fea0f..2246044b1639 100644
>>> --- a/drivers/hid/hid-logitech-hidpp.c
>>> +++ b/drivers/hid/hid-logitech-hidpp.c
>>> @@ -314,6 +314,7 @@ static int hidpp_send_message_sync(struct hidpp_device *hidpp,
>>>  			dbg_hid("%s:timeout waiting for response\n", __func__);
>>>  			memset(response, 0, sizeof(struct hidpp_report));
>>>  			ret = -ETIMEDOUT;
>>> +			goto exit;
>>>  		}
>>>  
>>
>> I have applied this even before getting confirmation from the reporters in 
>> bugzilla, as it's the right thing to do anyway.
> 
> Unfortunately it doesn't seem to cure the reported issue (while reverting 
> 586e8fede79 does): https://bugzilla.kernel.org/show_bug.cgi?id=217523#c2

I wonder if this code could be re-worked to not even do this (waiting)
from the _probe() function?  It ought to be able to throw it on a workqueue
or something, rather than stalling system boot for a minimum of 5-seconds
(or much longer as as-is).
-- 
Mark Lord

^ permalink raw reply

* Re: [PATCH] HID: logitech-hidpp: Handle timeout differently from busy
From: Jiri Kosina @ 2023-06-03 12:41 UTC (permalink / raw)
  To: Bastien Nocera
  Cc: linux-input, linux-kernel, Benjamin Tissoires,
	Peter F . Patel-Schneider, Filipe Laíns, Nestor Lopez Casado,
	Mark Lord
In-Reply-To: <nycvar.YFH.7.76.2305311606160.29760@cbobk.fhfr.pm>

On Wed, 31 May 2023, Jiri Kosina wrote:

> > If an attempt at contacting a receiver or a device fails because the
> > receiver or device never responds, don't restart the communication, only
> > restart it if the receiver or device answers that it's busy, as originally
> > intended.
> > 
> > This was the behaviour on communication timeout before commit 586e8fede795
> > ("HID: logitech-hidpp: Retry commands when device is busy").
> > 
> > This fixes some overly long waits in a critical path on boot, when
> > checking whether the device is connected by getting its HID++ version.
> > 
> > Signed-off-by: Bastien Nocera <hadess@hadess.net>
> > Suggested-by: Mark Lord <mlord@pobox.com>
> > Fixes: 586e8fede795 ("HID: logitech-hidpp: Retry commands when device is busy")
> > Link: https://bugzilla.kernel.org/show_bug.cgi?id=217412
> > ---
> >  drivers/hid/hid-logitech-hidpp.c | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
> > index 0fcfd85fea0f..2246044b1639 100644
> > --- a/drivers/hid/hid-logitech-hidpp.c
> > +++ b/drivers/hid/hid-logitech-hidpp.c
> > @@ -314,6 +314,7 @@ static int hidpp_send_message_sync(struct hidpp_device *hidpp,
> >  			dbg_hid("%s:timeout waiting for response\n", __func__);
> >  			memset(response, 0, sizeof(struct hidpp_report));
> >  			ret = -ETIMEDOUT;
> > +			goto exit;
> >  		}
> >  
> 
> I have applied this even before getting confirmation from the reporters in 
> bugzilla, as it's the right thing to do anyway.

Unfortunately it doesn't seem to cure the reported issue (while reverting 
586e8fede79 does): https://bugzilla.kernel.org/show_bug.cgi?id=217523#c2

-- 
Jiri Kosina
SUSE Labs


^ permalink raw reply

* Wyższa konwersja w e-sklepie 
From: Kamil Durjasz @ 2023-06-02  9:00 UTC (permalink / raw)
  To: linux-input

Dzień dobry,

w jaki sposób docierają Państwo do odbiorców?

Tworzymy potężne narzędzia sprzedaży, które pozwalają kompleksowo rozwiązać problemy potencjalnych klientów i skutecznie wpłynąć na ich decyzje zakupowe. 

Skupiamy się na Państwa potrzebach związanych z obsługą sklepu, oczekiwaniach i planach sprzedażowych. Szczegółowo dopasowujemy grafikę, funkcjonalności, strukturę i mikrointerakcje do Państwa grupy docelowej, co przekłada się na oczekiwane rezultaty.

Chętnie przedstawię dotychczasowe realizacje, aby mogli Państwo przekonać się o naszych możliwościach. Mogę się skontaktować?


Pozdrawiam
Kamil Durjasz

^ permalink raw reply

* Re: [PATCH v2] Input: xpad - Move Xbox 360 magic packet sending
From: Vicki Pfau @ 2023-06-03  0:35 UTC (permalink / raw)
  To: Dmitry Torokhov, Dongliang Mu, linux-input
  Cc: Dan Carpenter, syzbot+a3f758b8d8cb7e49afec
In-Reply-To: <20230502031202.1018440-1-vi@endrift.com>

On 5/1/23 20:12, Vicki Pfau wrote:
> This moves the sending of the magic packet introduced in db7220c48d8d from
> xpad_probe to xpad_start_input to ensure that xpad->dev->dev exists in the
> event that an error occurs. This should also fix issues with suspend that may
> occur with some controllers.
> 
> Fixes: db7220c48d8d ("Input: xpad - fix support for some third-party controllers")
> Reported-by: syzbot+a3f758b8d8cb7e49afec@syzkaller.appspotmail.com
> Reported-by: Dongliang Mu <dzm91@hust.edu.cn>
> Link: https://groups.google.com/g/syzkaller-bugs/c/iMhTgpGuIbM
> Signed-off-by: Vicki Pfau <vi@endrift.com>
> ---
>  drivers/input/joystick/xpad.c | 21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)
> 
> diff --git a/drivers/input/joystick/xpad.c b/drivers/input/joystick/xpad.c
> index 50ecff681b89..40abea92c393 100644
> --- a/drivers/input/joystick/xpad.c
> +++ b/drivers/input/joystick/xpad.c
> @@ -1720,6 +1720,27 @@ static int xpad_start_input(struct usb_xpad *xpad)
>  			return error;
>  		}
>  	}
> +	if (xpad->xtype == XTYPE_XBOX360) {
> +		/*
> +		 * Some third-party controllers Xbox 360-style controllers
> +		 * require this message to finish initialization.
> +		*/
> +		u8 dummy[20];
> +
> +		error = usb_control_msg_recv(xpad->udev, 0,
> +					     /* bRequest */ 0x01,
> +					     /* bmRequestType */
> +					     USB_TYPE_VENDOR | USB_DIR_IN |
> +						USB_RECIP_INTERFACE,
> +					     /* wValue */ 0x100,
> +					     /* wIndex */ 0x00,
> +					     dummy, sizeof(dummy),
> +					     25, GFP_KERNEL);
> +		if (error)
> +			dev_warn(&xpad->dev->dev,
> +				 "unable to receive magic message: %d\n",
> +				 error);
> +	}
>  
>  	return 0;
>  }

It's been a month and this fixed version of the patch never got any replies. Did it just get overlooked? Or does the fact that the old version got reverted mean I need to change the description in some capacity?

^ permalink raw reply

* Re: [PATCH] HID: i2c-hid: Block a rogue device on ASUS TUF A16
From: Limonciello, Mario @ 2023-06-02 18:43 UTC (permalink / raw)
  To: Friedrich Vock, linux-input, Natikar, Basavaraj,
	S-k, Shyam-sundar
  Cc: Jiri Kosina, Benjamin Tissoires
In-Reply-To: <20230530154058.17594-1-friedrich.vock@gmx.de>

+ some AMD guys

On 5/30/2023 10:40 AM, Friedrich Vock wrote:
> On these laptops, there seems to be a device that, when probed by
> i2c-hid, constantly sends bogus interrupts and interferes with the
> keyboard controller. When the device is enabled, it takes the keyboard
> around 8 seconds to register that keys are being pressed or released.

Do you know what interrupt is firing constantly?
Presumably it is the GPIO controller master interrupt, right?
And it's for GPIO 7 (guessed from acpidump on one of the bug
reports).

To confirm check /proc/interrupts.

If it's not obvious which GPIO is firing there is also a dynamic
debug statement in pinctrl-amd.c that may be helpful to figure
this out.

I would also suspect in Windows this doesn't happen.  If possible
can you confirm that? Check in Device Manager what driver is bound
to this device. Is it "inbox" from Microsoft or is it an ASUS
specific driver?

I wonder if the GPIO controller got programmed differently in
Windows for some reason. We may want to confirm the values for
GPIO registers from /sys/kernel/debug/gpio in Linux against those
that are programmed in Windows.

This can be accomplished using R/W everything in Windows.

>
> Nothing I tried seemed to make the device work, and preventing the
> device from being probed doesn't seem to break any functionality of
> the laptop.
>
> Signed-off-by: Friedrich Vock <friedrich.vock@gmx.de>

There are a few bug reports that popped up around this issue that should
probably also be tagged.

Link: https://bugzilla.kernel.org/show_bug.cgi?id=217336
Link: https://bugzilla.kernel.org/show_bug.cgi?id=217493

> ---
>   drivers/hid/i2c-hid/i2c-hid-core.c       |  5 +++
>   drivers/hid/i2c-hid/i2c-hid-dmi-quirks.c | 48 ++++++++++++++++++++++++
>   drivers/hid/i2c-hid/i2c-hid.h            |  3 ++
>   3 files changed, 56 insertions(+)
>
> diff --git a/drivers/hid/i2c-hid/i2c-hid-core.c b/drivers/hid/i2c-hid/i2c-hid-core.c
> index efbba0465eef..5f0686d058df 100644
> --- a/drivers/hid/i2c-hid/i2c-hid-core.c
> +++ b/drivers/hid/i2c-hid/i2c-hid-core.c
> @@ -1035,6 +1035,11 @@ int i2c_hid_core_probe(struct i2c_client *client, struct i2chid_ops *ops,
>
>   	ihid->quirks = i2c_hid_lookup_quirk(hid->vendor, hid->product);
>
> +	if (i2c_hid_device_blocked(hid->vendor, hid->product)) {
> +		ret = -ENODEV;
> +		goto err_irq;
> +	}
> +
The thing I worry about here is that an unserviced interrupt can prevent the
hardware from going into proper low power states; particularly at runtime.

I think we should better understand what's going on before going down this
path of just ignoring it.

>   	ret = hid_add_device(hid);
>   	if (ret) {
>   		if (ret != -ENODEV)
> diff --git a/drivers/hid/i2c-hid/i2c-hid-dmi-quirks.c b/drivers/hid/i2c-hid/i2c-hid-dmi-quirks.c
> index 210f17c3a0be..d2c2806b64b4 100644
> --- a/drivers/hid/i2c-hid/i2c-hid-dmi-quirks.c
> +++ b/drivers/hid/i2c-hid/i2c-hid-dmi-quirks.c
> @@ -440,6 +440,38 @@ static const struct dmi_system_id i2c_hid_dmi_quirk_table[] = {
>   	{ }	/* Terminate list */
>   };
>
> +static const struct hid_device_id i2c_hid_blocked_ite_device = {
> +	HID_DEVICE(BUS_I2C, HID_GROUP_GENERIC, USB_VENDOR_ID_ITE, 0x8051)
> +};
> +
> +/*
> + * This list contains devices that can't be activated at all, for example
> + * because activating them breaks other important parts of the system.
> + */
> +static const struct dmi_system_id i2c_hid_dmi_block_table[] = {
> +	/*
> +	 * On ASUS TUF Gaming A16 laptops, there is a device that will make the
> +	 * keyboard controller stop working correctly and flood the CPU with bogus
> +	 * interrupts.
> +	 */
> +	{
> +		.ident = "ASUS TUF Gaming A16 (Ryzen 7 7735HS)",
> +		.matches = {
> +			DMI_EXACT_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER INC."),
> +			DMI_MATCH(DMI_PRODUCT_NAME, "FA617NS"),
> +		},
> +		.driver_data = (void *)&i2c_hid_blocked_ite_device,
> +	},
> +	{
> +		.ident = "ASUS TUF Gaming A16 (Ryzen 9 7940HS)",
> +		.matches = {
> +			DMI_EXACT_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER INC."),
> +			DMI_MATCH(DMI_PRODUCT_NAME, "FA617XS"),
> +		},
> +		.driver_data = (void *)&i2c_hid_blocked_ite_device,
> +	},
> +	{ }	/* Terminate list */
If this *does* end up being the best solution, I think it's better
to patch in the DMI to gpiolib-acpi.c similar to other quirks for floating
GPIOs.  Example:

https://github.com/torvalds/linux/blob/master/drivers/gpio/gpiolib-acpi.c#L1654

> +};
>
>   struct i2c_hid_desc *i2c_hid_get_dmi_i2c_hid_desc_override(uint8_t *i2c_name)
>   {
> @@ -492,3 +524,19 @@ u32 i2c_hid_get_dmi_quirks(const u16 vendor, const u16 product)
>
>   	return quirks;
>   }
> +
> +bool i2c_hid_device_blocked(const u16 vendor, const u16 product)
> +{
> +	const struct dmi_system_id *system_id =
> +			dmi_first_match(i2c_hid_dmi_block_table);
> +
> +	if (system_id) {
> +		const struct hid_device_id *device_id =
> +				(struct hid_device_id *)(system_id->driver_data);
> +
> +		if (device_id && device_id->vendor == vendor &&
> +		    device_id->product == product)
> +			return true;
> +	}
> +	return false;
> +}
> diff --git a/drivers/hid/i2c-hid/i2c-hid.h b/drivers/hid/i2c-hid/i2c-hid.h
> index 2c7b66d5caa0..e17bdd758f39 100644
> --- a/drivers/hid/i2c-hid/i2c-hid.h
> +++ b/drivers/hid/i2c-hid/i2c-hid.h
> @@ -10,6 +10,7 @@ struct i2c_hid_desc *i2c_hid_get_dmi_i2c_hid_desc_override(uint8_t *i2c_name);
>   char *i2c_hid_get_dmi_hid_report_desc_override(uint8_t *i2c_name,
>   					       unsigned int *size);
>   u32 i2c_hid_get_dmi_quirks(const u16 vendor, const u16 product);
> +bool i2c_hid_device_blocked(const u16 vendor, const u16 product);
>   #else
>   static inline struct i2c_hid_desc
>   		   *i2c_hid_get_dmi_i2c_hid_desc_override(uint8_t *i2c_name)
> @@ -19,6 +20,8 @@ static inline char *i2c_hid_get_dmi_hid_report_desc_override(uint8_t *i2c_name,
>   { return NULL; }
>   static inline u32 i2c_hid_get_dmi_quirks(const u16 vendor, const u16 product)
>   { return 0; }
> +static inline bool i2c_hid_device_blocked(const u16 vendor, const u16 product)
> +{ return false; }
>   #endif
>
>   /**
> --
> 2.40.1
>
>

^ permalink raw reply

* Re: [PATCH 1/2] dt-bindings: input: touchscreen: Add ilitek 9882T touchscreen chip
From: Rob Herring @ 2023-06-02 15:14 UTC (permalink / raw)
  To: Cong Yang
  Cc: dmitry.torokhov, jikos, conor+dt, linux-input, benjamin.tissoires,
	linux-kernel, devicetree, hsinyi, krzysztof.kozlowski+dt, robh+dt,
	dianders
In-Reply-To: <20230602140948.2138668-2-yangcong5@huaqin.corp-partner.google.com>


On Fri, 02 Jun 2023 22:09:47 +0800, Cong Yang wrote:
> Add an ilitek touch screen chip ili9882t.
> 
> Signed-off-by: Cong Yang <yangcong5@huaqin.corp-partner.google.com>
> ---
>  .../bindings/input/elan,ekth6915.yaml         | 36 ++++++++++++++-----
>  1 file changed, 27 insertions(+), 9 deletions(-)
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:
./Documentation/devicetree/bindings/input/elan,ekth6915.yaml:20:9: [warning] wrong indentation: expected 10 but found 8 (indentation)

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/input/elan,ekth6915.yaml: properties:required: ['compatible', 'reg', 'interrupts', 'vcc33-supply'] is not of type 'object', 'boolean'
	from schema $id: http://json-schema.org/draft-07/schema#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/input/elan,ekth6915.yaml: properties: 'required' should not be valid under {'$ref': '#/definitions/json-schema-prop-names'}
	hint: A json-schema keyword was found instead of a DT property name.
	from schema $id: http://devicetree.org/meta-schemas/keywords.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/input/elan,ekth6915.yaml: ignoring, error in schema: properties: required
Documentation/devicetree/bindings/input/elan,ekth6915.example.dtb: /example-0/i2c/touchscreen@10: failed to match any schema with compatible: ['elan,ekth6915']

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20230602140948.2138668-2-yangcong5@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.


^ permalink raw reply

* Re: [PATCH 1/2] dt-bindings: input: touchscreen: Add ilitek 9882T touchscreen chip
From: Krzysztof Kozlowski @ 2023-06-02 14:19 UTC (permalink / raw)
  To: Cong Yang, robh+dt, krzysztof.kozlowski+dt, conor+dt,
	dmitry.torokhov, jikos, benjamin.tissoires, dianders, hsinyi
  Cc: linux-input, devicetree, linux-kernel
In-Reply-To: <20230602140948.2138668-2-yangcong5@huaqin.corp-partner.google.com>

On 02/06/2023 16:09, Cong Yang wrote:
> Add an ilitek touch screen chip ili9882t.
> 
> Signed-off-by: Cong Yang <yangcong5@huaqin.corp-partner.google.com>

It does not look like you tested the bindings, at least after quick
look. Please run `make dt_binding_check` (see
Documentation/devicetree/bindings/writing-schema.rst for instructions).
Maybe you need to update your dtschema and yamllint.

There is no way this would work and if you test you will see error msg.
Feel free to ping if message is unclear.

> ---
>  .../bindings/input/elan,ekth6915.yaml         | 36 ++++++++++++++-----
>  1 file changed, 27 insertions(+), 9 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/input/elan,ekth6915.yaml b/Documentation/devicetree/bindings/input/elan,ekth6915.yaml
> index 05e6f2df604c..73e94cb6c4e0 100644
> --- a/Documentation/devicetree/bindings/input/elan,ekth6915.yaml
> +++ b/Documentation/devicetree/bindings/input/elan,ekth6915.yaml
> @@ -15,11 +15,15 @@ description:
>  
>  properties:
>    compatible:
> -    items:
> -      - const: elan,ekth6915
> +    oneOf:

It's not oneOf. Just enum

> +      - enum:
> +        - elan,ekth6915
> +        - ilitek,ili9882t
>  
>    reg:
> -    const: 0x10
> +    enum:
> +      - 0x10
> +      - 0x41
>  
>    interrupts:
>      maxItems: 1
> @@ -29,17 +33,31 @@ properties:
>  
>    vcc33-supply:
>      description: The 3.3V supply to the touchscreen.
> +                 If using ili9882t then this supply will not be needed.

Are you sure these are compatible devices then? What does it mean "not
needed"? Is the pin there or is not?

>  
>    vccio-supply:
>      description:
>        The IO supply to the touchscreen. Need not be specified if this is the
>        same as the 3.3V supply.
> -
> -required:
> -  - compatible
> -  - reg
> -  - interrupts
> -  - vcc33-supply
> +      If using ili9882t, the IO supply is required.
> +
> +  required:

NAK. Really. Test patches before sending.

Best regards,
Krzysztof


^ permalink raw reply

* [PATCH 2/2] HID: i2c-hid: elan: Add ili9882t timing
From: Cong Yang @ 2023-06-02 14:09 UTC (permalink / raw)
  To: robh+dt, krzysztof.kozlowski+dt, conor+dt, dmitry.torokhov, jikos,
	benjamin.tissoires, dianders, hsinyi
  Cc: linux-input, devicetree, linux-kernel, Cong Yang
In-Reply-To: <20230602140948.2138668-1-yangcong5@huaqin.corp-partner.google.com>

The ili9882t is a TDDI IC (Touch with Display Driver). The datasheet
specifies there should be 60ms between touch SDA sleep and panel RESX.
Doug's series[1] allows panels and touchscreens to power on/off together,
so we can add the 65 ms delay in i2c_hid_core_suspend before panel_unprepare.

[1]: https: //lore.kernel.org/all/20230523193017.4109557-1-dianders@chromium.org/

Signed-off-by: Cong Yang <yangcong5@huaqin.corp-partner.google.com>
---
 drivers/hid/i2c-hid/i2c-hid-of-elan.c | 20 ++++++++++++++++----
 1 file changed, 16 insertions(+), 4 deletions(-)

diff --git a/drivers/hid/i2c-hid/i2c-hid-of-elan.c b/drivers/hid/i2c-hid/i2c-hid-of-elan.c
index 76ddc8be1cbb..411d7ea2725d 100644
--- a/drivers/hid/i2c-hid/i2c-hid-of-elan.c
+++ b/drivers/hid/i2c-hid/i2c-hid-of-elan.c
@@ -18,7 +18,8 @@
 #include "i2c-hid.h"
 
 struct elan_i2c_hid_chip_data {
-	unsigned int post_gpio_reset_delay_ms;
+	unsigned int post_gpio_reset_on_delay_ms;
+	unsigned int post_gpio_reset_off_delay_ms;
 	unsigned int post_power_delay_ms;
 	u16 hid_descriptor_address;
 };
@@ -52,8 +53,8 @@ static int elan_i2c_hid_power_up(struct i2chid_ops *ops)
 		msleep(ihid_elan->chip_data->post_power_delay_ms);
 
 	gpiod_set_value_cansleep(ihid_elan->reset_gpio, 0);
-	if (ihid_elan->chip_data->post_gpio_reset_delay_ms)
-		msleep(ihid_elan->chip_data->post_gpio_reset_delay_ms);
+	if (ihid_elan->chip_data->post_gpio_reset_on_delay_ms)
+		msleep(ihid_elan->chip_data->post_gpio_reset_on_delay_ms);
 
 	return 0;
 }
@@ -64,6 +65,9 @@ static void elan_i2c_hid_power_down(struct i2chid_ops *ops)
 		container_of(ops, struct i2c_hid_of_elan, ops);
 
 	gpiod_set_value_cansleep(ihid_elan->reset_gpio, 1);
+	if (ihid_elan->chip_data->post_gpio_reset_off_delay_ms)
+		msleep(ihid_elan->chip_data->post_gpio_reset_off_delay_ms);
+
 	regulator_disable(ihid_elan->vccio);
 	regulator_disable(ihid_elan->vcc33);
 }
@@ -101,12 +105,20 @@ static int i2c_hid_of_elan_probe(struct i2c_client *client)
 
 static const struct elan_i2c_hid_chip_data elan_ekth6915_chip_data = {
 	.post_power_delay_ms = 1,
-	.post_gpio_reset_delay_ms = 300,
+	.post_gpio_reset_on_delay_ms = 300,
+	.hid_descriptor_address = 0x0001,
+};
+
+static const struct elan_i2c_hid_chip_data ilitek_ili9882t_chip_data = {
+	.post_power_delay_ms = 1,
+	.post_gpio_reset_on_delay_ms = 200,
+	.post_gpio_reset_off_delay_ms = 65,
 	.hid_descriptor_address = 0x0001,
 };
 
 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 },
 	{ }
 };
 MODULE_DEVICE_TABLE(of, elan_i2c_hid_of_match);
-- 
2.25.1


^ permalink raw reply related

* [PATCH 1/2] dt-bindings: input: touchscreen: Add ilitek 9882T touchscreen chip
From: Cong Yang @ 2023-06-02 14:09 UTC (permalink / raw)
  To: robh+dt, krzysztof.kozlowski+dt, conor+dt, dmitry.torokhov, jikos,
	benjamin.tissoires, dianders, hsinyi
  Cc: linux-input, devicetree, linux-kernel, Cong Yang
In-Reply-To: <20230602140948.2138668-1-yangcong5@huaqin.corp-partner.google.com>

Add an ilitek touch screen chip ili9882t.

Signed-off-by: Cong Yang <yangcong5@huaqin.corp-partner.google.com>
---
 .../bindings/input/elan,ekth6915.yaml         | 36 ++++++++++++++-----
 1 file changed, 27 insertions(+), 9 deletions(-)

diff --git a/Documentation/devicetree/bindings/input/elan,ekth6915.yaml b/Documentation/devicetree/bindings/input/elan,ekth6915.yaml
index 05e6f2df604c..73e94cb6c4e0 100644
--- a/Documentation/devicetree/bindings/input/elan,ekth6915.yaml
+++ b/Documentation/devicetree/bindings/input/elan,ekth6915.yaml
@@ -15,11 +15,15 @@ description:
 
 properties:
   compatible:
-    items:
-      - const: elan,ekth6915
+    oneOf:
+      - enum:
+        - elan,ekth6915
+        - ilitek,ili9882t
 
   reg:
-    const: 0x10
+    enum:
+      - 0x10
+      - 0x41
 
   interrupts:
     maxItems: 1
@@ -29,17 +33,31 @@ properties:
 
   vcc33-supply:
     description: The 3.3V supply to the touchscreen.
+                 If using ili9882t then this supply will not be needed.
 
   vccio-supply:
     description:
       The IO supply to the touchscreen. Need not be specified if this is the
       same as the 3.3V supply.
-
-required:
-  - compatible
-  - reg
-  - interrupts
-  - vcc33-supply
+      If using ili9882t, the IO supply is required.
+
+  required:
+    - compatible
+    - reg
+    - interrupts
+    - vcc33-supply
+
+if:
+  properties:
+    compatible:
+      contains:
+        const: ilitek,ili9882t
+then:
+  required:
+    - compatible
+    - reg
+    - interrupts
+    - vccio-supply
 
 additionalProperties: false
 
-- 
2.25.1


^ permalink raw reply related

* [PATCH 0/2] Add ili9882t timing
From: Cong Yang @ 2023-06-02 14:09 UTC (permalink / raw)
  To: robh+dt, krzysztof.kozlowski+dt, conor+dt, dmitry.torokhov, jikos,
	benjamin.tissoires, dianders, hsinyi
  Cc: linux-input, devicetree, linux-kernel, Cong Yang

Add ili9882t dt-bindings and timing

Cong Yang (2):
  dt-bindings: input: touchscreen: Add ilitek 9882T touchscreen chip
  HID: i2c-hid: elan: Add ili9882t timing

 .../bindings/input/elan,ekth6915.yaml         | 36 ++++++++++++++-----
 drivers/hid/i2c-hid/i2c-hid-of-elan.c         | 20 ++++++++---
 2 files changed, 43 insertions(+), 13 deletions(-)

-- 
2.25.1


^ permalink raw reply

* Re: [PATCH v1 1/7] dt-bindings: mmc: fsl-imx-esdhc: Add imx6ul support
From: Krzysztof Kozlowski @ 2023-06-02  8:27 UTC (permalink / raw)
  To: Oleksij Rempel, Abel Vesa, Michael Turquette, Stephen Boyd,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Shawn Guo,
	Sascha Hauer, Herbert Xu, David S. Miller, Dmitry Torokhov,
	Ulf Hansson
  Cc: kernel, Peng Fan, Fabio Estevam, NXP Linux Team, Daniel Lezcano,
	Thomas Gleixner, Michael Trimarchi, Mark Brown, Dario Binacchi,
	Anson Huang, Marek Vasut, linux-clk, devicetree, linux-arm-kernel,
	linux-kernel, linux-crypto, linux-input, linux-mmc
In-Reply-To: <20230601101451.357662-2-o.rempel@pengutronix.de>

On 01/06/2023 12:14, Oleksij Rempel wrote:
> Add the 'fsl,imx6ul-usdhc' value to the compatible properties list in
> the fsl-imx-esdhc.yaml file. This is required to match the compatible
> strings present in the 'mmc@2190000' node of 'imx6ul-prti6g.dtb'. This
> commit addresses the following dtbs_check warning:
>   imx6ul-prti6g.dtb: mmc@2190000: compatible: 'oneOf' conditional failed,
>     one must be fixed: ['fsl,imx6ul-usdhc', 'fsl,imx6sx-usdhc'] is too long
>     'fsl,imx6ul-usdhc' is not one of ['fsl,imx25-esdhc', 'fsl,imx35-esdhc',
>     'fsl,imx51-esdhc', 'fsl,imx53-esdhc', 'fsl,imx6q-usdhc',
>     'fsl,imx6sl-usdhc', 'fsl,imx6sx-usdhc', 'fsl,imx7d-usdhc',
>     'fsl,imx7ulp-usdhc', 'fsl,imx8mm-usdhc', 'fsl,imxrt1050-usdhc',
>     'nxp,s32g2-usdhc']
>   From schema: Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.yaml

Except what Conor wrote, please don't wrap that much the error log - it
is unreadable. Trim it, remove unneeded parts and keep some decent
one/two lines even if it exceeds the commit msg. This applies to other
patches as well.

Best regards,
Krzysztof


^ permalink raw reply

* Re: [PATCH 1/2] dt-bindings: input: iqs7222: Add properties for Azoteq IQS7222D
From: Krzysztof Kozlowski @ 2023-06-02  7:21 UTC (permalink / raw)
  To: Jeff LaBundy, Krzysztof Kozlowski
  Cc: dmitry.torokhov, robh+dt, linux-input, devicetree,
	krzysztof.kozlowski+dt, conor+dt
In-Reply-To: <ZHfzLUrWZc0Bp+Ap@nixie71>

On 01/06/2023 03:23, Jeff LaBundy wrote:
>>>  
>>>    reg:
>>>      maxItems: 1
>>> @@ -173,6 +174,148 @@ properties:
>>>      maximum: 3000
>>>      description: Specifies the report rate (in ms) during ultra-low-power mode.
>>>  
>>> +  touchscreen-size-x: true
>>> +  touchscreen-size-y: true
>>> +  touchscreen-inverted-x: true
>>> +  touchscreen-inverted-y: true
>>> +  touchscreen-swapped-x-y: true
>>
>> Why? Aren't they coming from common schema?
> 
> Yes, but because additionalProperties is set to false here, we must explicitly
> include the subset of properties from the common schema that are allowed for
> this particular instance. I counted over a dozen other bindings doing the same.
> 
> In case I have misunderstood, please let me know.

If you are listing now most of touchscreen properties, it is a sign you
should use just unevaluatedProperties: false (instead
additionalProperties) and then no need for any of these here.

> 
>>
>>> +
>>> +  trackpad:
>>> +    type: object
>>> +    description: Represents all channels associated with the trackpad.
>>> +
>>> +    properties:
>>> +      azoteq,channel-select:
>>> +        $ref: /schemas/types.yaml#/definitions/uint32-array
>>> +        minItems: 1
>>> +        maxItems: 12
>>> +        items:
>>> +          minimum: 0
>>> +          maximum: 13
>>> +        description:
>>> +          Specifies the order of the channels that participate in the trackpad.
>>> +          Specify 255 to omit a given channel for the purpose of mapping a non-
>>> +          rectangular trackpad.
>>> +
>>> +      azoteq,num-rows:
>>> +        $ref: /schemas/types.yaml#/definitions/uint32
>>> +        minimum: 1
>>> +        maximum: 12
>>> +        description: Specifies the number of rows that comprise the trackpad.
>>> +
>>> +      azoteq,num-cols:
>>> +        $ref: /schemas/types.yaml#/definitions/uint32
>>> +        minimum: 1
>>> +        maximum: 12
>>> +        description: Specifies the number of columns that comprise the trackpad.
>>> +
>>> +      azoteq,top-speed:
>>> +        $ref: /schemas/types.yaml#/definitions/uint32
>>> +        multipleOf: 4
>>> +        minimum: 0
>>> +        maximum: 1020
>>> +        description:
>>> +          Specifies the speed of movement after which coordinate filtering is
>>> +          no longer applied.
>>
>> Units?
> 
> This is a ratiometric, i.e. unitless value that represents a hardware filter
> coefficient. It already exists in this binding prior to this patch under the
> slider-0/1 node and is simply re-used here.

Then mention the ratio (e.g. "speed of movement expressed as ratio
of..."). Description said "speed" and we usually measure speed in very
specific units.

> 
>>
>>> +
>>> +      azoteq,bottom-speed:
>>> +        $ref: /schemas/types.yaml#/definitions/uint32
>>> +        minimum: 0
>>> +        maximum: 255
>>> +        description:
>>> +          Specifies the speed of movement after which coordinate filtering is
>>> +          linearly reduced.
>>
>> Units?
> 
> Same here.
> 
>>
>>> +
>>> +      azoteq,use-prox:
>>> +        type: boolean
>>> +        description:
>>> +          Directs the trackpad to respond to the proximity states of the se-
>>> +          lected channels instead of their corresponding touch states. Note
>>
>> Don't split the words.
> 
> ACK.
> 
>>
>>> +          the trackpad cannot report granular coordinates during a state of
>>> +          proximity.
>>> +
>>> +    patternProperties:
>>> +      "^azoteq,lower-cal-(x|y)$":
>>> +        $ref: /schemas/types.yaml#/definitions/uint32
>>> +        minimum: 0
>>> +        maximum: 255
>>> +        description: Specifies the trackpad's lower starting points.
>>
>> Why would you need this property? Why does this represent hardware property?
> 
> This property and its cousin below define the physical boundaries of the
> touch surface. They are typically used to mask areas that cannot elicit
> an electrical response due to manufacturing tolerances or the presence of
> an overlay. For that reason, they descend directly from properties of the
> hardware.
> 
> Similar properties already exist in this binding for the slider case; this
> device simply extends the functionality to a second dimenstion.

OK

> 
>>
>>> +
>>> +      "^azoteq,upper-cal-(x|y)$":
>>> +        $ref: /schemas/types.yaml#/definitions/uint32
>>> +        minimum: 0
>>> +        maximum: 255
>>> +        description: Specifies the trackpad's upper starting points.
>>> +
>>> +      "^event-(press|tap|(swipe|flick)-(x|y)-(pos|neg))$":
>>> +        type: object
>>> +        $ref: input.yaml#
>>> +        description:
>>> +          Represents a press or gesture event reported by the trackpad. Specify
>>> +          'linux,code' under the press event to report absolute coordinates.
>>> +
>>> +        properties:
>>> +          linux,code: true
>>> +
>>> +          azoteq,gesture-angle-tighten:
>>> +            type: boolean
>>> +            description:
>>> +              Limits the tangent of the gesture angle to 0.5 (axial gestures
>>> +              only). If specified in one direction, the effect is applied in
>>> +              either direction.
>>> +
>>> +          azoteq,gesture-max-ms:
>>> +            multipleOf: 16
>>> +            minimum: 0
>>> +            maximum: 4080
>>> +            description:
>>> +              Specifies the length of time (in ms) within which a tap, swipe
>>> +              or flick gesture must be completed in order to be acknowledged
>>> +              by the device. The number specified for any one swipe or flick
>>> +              gesture applies to all other swipe or flick gestures.
>>> +
>>> +          azoteq,gesture-min-ms:
>>> +            multipleOf: 16
>>> +            minimum: 0
>>> +            maximum: 4080
>>> +            description:
>>> +              Specifies the length of time (in ms) for which a tap gesture must
>>> +              be held in order to be acknowledged by the device.
>>> +
>>> +          azoteq,gesture-dist:
>>> +            $ref: /schemas/types.yaml#/definitions/uint32
>>> +            minimum: 0
>>> +            maximum: 65535
>>> +            description:
>>> +              Specifies the distance across which a tap, swipe or flick gesture
>>> +              must travel in order to be acknowledged by the device. The number
>>> +              specified for any one swipe or flick gesture applies to all other
>>> +              swipe or flick gestures.
>>> +
>>> +          azoteq,gpio-select:
>>> +            $ref: /schemas/types.yaml#/definitions/uint32-array
>>> +            minItems: 1
>>> +            maxItems: 3
>>> +            items:
>>> +              minimum: 0
>>> +              maximum: 2
>>> +            description: |
>>> +              Specifies one or more GPIO mapped to the event as follows:
>>> +              0: GPIO0
>>> +              1: GPIO3
>>> +              2: GPIO4
>>> +
>>> +              Note that although multiple events can be mapped to a single
>>> +              GPIO, they must all be of the same type (proximity, touch or
>>> +              trackpad gesture).
>>> +
>>> +        additionalProperties: false
>>> +
>>> +    required:
>>> +      - azoteq,channel-select
>>> +
>>> +    additionalProperties: false
>>> +
>>>  patternProperties:
>>>    "^cycle-[0-9]$":
>>>      type: object
>>> @@ -288,6 +431,10 @@ patternProperties:
>>>            Activates the reference channel in response to proximity events
>>>            instead of touch events.
>>>  
>>> +      azoteq,counts-filt-enable:
>>> +        type: boolean
>>> +        description: Applies counts filtering to the channel.
>>> +
>>>        azoteq,ati-band:
>>>          $ref: /schemas/types.yaml#/definitions/uint32
>>>          enum: [0, 1, 2, 3]
>>> @@ -432,12 +579,12 @@ patternProperties:
>>>              description: |
>>>                Specifies one or more GPIO mapped to the event as follows:
>>>                0: GPIO0
>>> -              1: GPIO3 (IQS7222C only)
>>> -              2: GPIO4 (IQS7222C only)
>>> +              1: GPIO3
>>> +              2: GPIO4
>>
>> Why changing this? Is it valid for IQS7222A?
> 
> It's not, only for 'C' and now 'D'. However, the restriction for 'A' is already
> conveyed in an if/then schema in the original binding. So rather than updating
> this text to say "(IQS7222C and IQS7222D only)", I opted to drop the open-coded
> text and rely on the existing schema.

OK

> 


Best regards,
Krzysztof


^ permalink raw reply

* Re: [PATCH v1 4/7] dt-bindings: crypto: fsl-dcp: Add i.MX6SL, i.MX6SLL, and i.MX6ULL support
From: Conor Dooley @ 2023-06-01 18:26 UTC (permalink / raw)
  To: Oleksij Rempel
  Cc: Abel Vesa, Michael Turquette, Stephen Boyd, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Shawn Guo, Sascha Hauer,
	Herbert Xu, David S. Miller, Dmitry Torokhov, Ulf Hansson, kernel,
	Peng Fan, Fabio Estevam, NXP Linux Team, Daniel Lezcano,
	Thomas Gleixner, Michael Trimarchi, Mark Brown, Dario Binacchi,
	Anson Huang, Marek Vasut, linux-clk, devicetree, linux-arm-kernel,
	linux-kernel, linux-crypto, linux-input, linux-mmc
In-Reply-To: <20230601101451.357662-5-o.rempel@pengutronix.de>

[-- Attachment #1: Type: text/plain, Size: 2348 bytes --]

On Thu, Jun 01, 2023 at 12:14:48PM +0200, Oleksij Rempel wrote:
> Add support for i.MX6SL, i.MX6SLL, and i.MX6ULL to the 'fsl-dcp' binding
> to address the following dtbs_check warning:
>   imx6ull-jozacp.dtb: crypto@2280000: compatible:0: 'fsl,imx6ull-dcp' is
>     not one of ['fsl,imx23-dcp', 'fsl,imx28-dcp']
>   From schema: Documentation/devicetree/bindings/crypto/fsl-dcp.yaml
> 
>   imx6ull-jozacp.dtb: crypto@2280000: compatible: ['fsl,imx6ull-dcp',
>     'fsl,imx28-dcp'] is too long
>   From schema: Documentation/devicetree/bindings/crypto/fsl-dcp.yaml
> 
> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> ---
>  .../devicetree/bindings/crypto/fsl-dcp.yaml   | 19 ++++++++++++++++---
>  1 file changed, 16 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/crypto/fsl-dcp.yaml b/Documentation/devicetree/bindings/crypto/fsl-dcp.yaml
> index 99be01539fcd..8af393b9f3ca 100644
> --- a/Documentation/devicetree/bindings/crypto/fsl-dcp.yaml
> +++ b/Documentation/devicetree/bindings/crypto/fsl-dcp.yaml
> @@ -11,9 +11,22 @@ maintainers:
>  
>  properties:
>    compatible:
> -    enum:
> -      - fsl,imx23-dcp
> -      - fsl,imx28-dcp
> +    oneOf:
> +      - const: fsl,imx23-dcp
> +      - const: fsl,imx28-dcp
> +      - const: fsl,imx6sl-dcp
> +      - const: fsl,imx6sll-dcp
> +      - const: fsl,imx6ull-dcp

Confused again here chief, why allow these to appear on their own if
their are all compatible with the imx28-dcp?

> +      - items:
> +          - enum:
> +              - fsl,imx6sl-dcp
> +              - fsl,imx6sll-dcp
> +              - fsl,imx6ull-dcp
> +          - const: fsl,imx28-dcp
> +      - items:
> +          - enum:
> +              - fsl,imx28-dcp
> +          - const: fsl,imx23-dcp

I don't get this either. Why set the imx23-dcp as the fallback for the
imx28-dcp, when the imx28-dcp is being used as the fallback for the imx6
stuff?

I get the impression that some of the devicetrees should be fixed up,
rather than adding these sorta odd conditions to the bindings.

To me it'd make sense to swap everything that uses imx28-dcp as a
fallback to use imx23-dcp instead, since that is the most generic one?

What am I missing?

Cheers,
Conor.

>  
>    reg:
>      maxItems: 1
> -- 
> 2.39.2
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply

* Re: [PATCH v1 3/7] dt-bindings: timer: gpt: Support 3rd clock for i.MX6DL
From: Conor Dooley @ 2023-06-01 18:22 UTC (permalink / raw)
  To: Oleksij Rempel
  Cc: Abel Vesa, Michael Turquette, Stephen Boyd, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Shawn Guo, Sascha Hauer,
	Herbert Xu, David S. Miller, Dmitry Torokhov, Ulf Hansson, kernel,
	Peng Fan, Fabio Estevam, NXP Linux Team, Daniel Lezcano,
	Thomas Gleixner, Michael Trimarchi, Mark Brown, Dario Binacchi,
	Anson Huang, Marek Vasut, linux-clk, devicetree, linux-arm-kernel,
	linux-kernel, linux-crypto, linux-input, linux-mmc
In-Reply-To: <20230601101451.357662-4-o.rempel@pengutronix.de>

[-- Attachment #1: Type: text/plain, Size: 1992 bytes --]

On Thu, Jun 01, 2023 at 12:14:47PM +0200, Oleksij Rempel wrote:
> Add support for a 3rd clock, 'osc_per', for i.MX6DL to the 'fsl,imxgpt'
> binding to resolve the following dtbs_check warning:
>   imx6dl-alti6p.dtb: timer@2098000: clocks: [[2, 119], [2, 120], [2, 237]]
>     is too long
>   From schema: Documentation/devicetree/bindings/timer/fsl,imxgpt.yaml
>   imx6dl-alti6p.dtb: timer@2098000: clock-names: ['ipg', 'per', 'osc_per']
>     is too long
>   From schema: Documentation/devicetree/bindings/timer/fsl,imxgpt.yaml
> 
> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> ---
>  .../devicetree/bindings/timer/fsl,imxgpt.yaml | 22 ++++++++++++++-----
>  1 file changed, 16 insertions(+), 6 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/timer/fsl,imxgpt.yaml b/Documentation/devicetree/bindings/timer/fsl,imxgpt.yaml
> index adf617b8f353..21ff51c3f38f 100644
> --- a/Documentation/devicetree/bindings/timer/fsl,imxgpt.yaml
> +++ b/Documentation/devicetree/bindings/timer/fsl,imxgpt.yaml
> @@ -46,14 +46,24 @@ properties:
>      maxItems: 1
>  
>    clocks:
> -    items:
> -      - description: SoC GPT ipg clock
> -      - description: SoC GPT per clock
> +    anyOf:
> +      - items:
> +          - description: SoC GPT ipg clock
> +          - description: SoC GPT per clock
> +      - items:
> +          - description: SoC GPT ipg clock
> +          - description: SoC GPT per clock
> +          - description: SoC GPT osc_per clock
>  
>    clock-names:
> -    items:
> -      - const: ipg
> -      - const: per
> +    anyOf:
> +      - items:
> +          - const: ipg
> +          - const: per
> +      - items:
> +          - const: ipg
> +          - const: per
> +          - const: osc_per

Hmm, should we not do per-compatible enforcement here so that the extra
clock is only used on the appropriate platforms?

Cheers,
Conor.

>  
>  required:
>    - compatible
> -- 
> 2.39.2
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply

* Re: [PATCH v1 2/7] dt-bindings: timer: gpt: Add i.MX6UL support
From: Conor Dooley @ 2023-06-01 18:20 UTC (permalink / raw)
  To: Oleksij Rempel
  Cc: Abel Vesa, Michael Turquette, Stephen Boyd, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Shawn Guo, Sascha Hauer,
	Herbert Xu, David S. Miller, Dmitry Torokhov, Ulf Hansson, kernel,
	Peng Fan, Fabio Estevam, NXP Linux Team, Daniel Lezcano,
	Thomas Gleixner, Michael Trimarchi, Mark Brown, Dario Binacchi,
	Anson Huang, Marek Vasut, linux-clk, devicetree, linux-arm-kernel,
	linux-kernel, linux-crypto, linux-input, linux-mmc
In-Reply-To: <20230601101451.357662-3-o.rempel@pengutronix.de>

[-- Attachment #1: Type: text/plain, Size: 1352 bytes --]

On Thu, Jun 01, 2023 at 12:14:46PM +0200, Oleksij Rempel wrote:
> Add 'fsl,imx6ul-gpt' compatible to resolve the following dtbs_check
> warning:
> /arch/arm/boot/dts/imx6ull-jozacp.dtb: timer@20e8000: compatible:
> 'oneOf' conditional failed, one must be fixed:
>     'fsl,imx6ul-gpt' is not one of ['fsl,imx25-gpt', 'fsl,imx50-gpt',
>     'fsl,imx51-gpt', 'fsl,imx53-gpt', 'fsl,imx6q-gpt']
> 
> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> ---
>  Documentation/devicetree/bindings/timer/fsl,imxgpt.yaml | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/timer/fsl,imxgpt.yaml b/Documentation/devicetree/bindings/timer/fsl,imxgpt.yaml
> index 716c6afcca1f..adf617b8f353 100644
> --- a/Documentation/devicetree/bindings/timer/fsl,imxgpt.yaml
> +++ b/Documentation/devicetree/bindings/timer/fsl,imxgpt.yaml
> @@ -34,6 +34,10 @@ properties:
>                - fsl,imxrt1050-gpt
>                - fsl,imxrt1170-gpt
>            - const: fsl,imx6dl-gpt
> +      - const: fsl,imx6ul-gpt

Again, why add this as an appears-on its own compatible when it seems to
be used only with a fallback to imx6sx? (In-tree at least)

> +      - items:
> +          - const: fsl,imx6ul-gpt
> +          - const: fsl,imx6sx-gpt
>  
>    reg:
>      maxItems: 1
> -- 
> 2.39.2
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply

* Re: [PATCH v1 1/7] dt-bindings: mmc: fsl-imx-esdhc: Add imx6ul support
From: Conor Dooley @ 2023-06-01 18:19 UTC (permalink / raw)
  To: Oleksij Rempel
  Cc: Abel Vesa, Michael Turquette, Stephen Boyd, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Shawn Guo, Sascha Hauer,
	Herbert Xu, David S. Miller, Dmitry Torokhov, Ulf Hansson, kernel,
	Peng Fan, Fabio Estevam, NXP Linux Team, Daniel Lezcano,
	Thomas Gleixner, Michael Trimarchi, Mark Brown, Dario Binacchi,
	Anson Huang, Marek Vasut, linux-clk, devicetree, linux-arm-kernel,
	linux-kernel, linux-crypto, linux-input, linux-mmc
In-Reply-To: <20230601-obnoxious-sterility-e89541412bb4@spud>

[-- Attachment #1: Type: text/plain, Size: 2293 bytes --]

On Thu, Jun 01, 2023 at 07:18:31PM +0100, Conor Dooley wrote:
> On Thu, Jun 01, 2023 at 12:14:45PM +0200, Oleksij Rempel wrote:
> > Add the 'fsl,imx6ul-usdhc' value to the compatible properties list in
> > the fsl-imx-esdhc.yaml file. This is required to match the compatible
> > strings present in the 'mmc@2190000' node of 'imx6ul-prti6g.dtb'. This
> > commit addresses the following dtbs_check warning:
> >   imx6ul-prti6g.dtb: mmc@2190000: compatible: 'oneOf' conditional failed,
> >     one must be fixed: ['fsl,imx6ul-usdhc', 'fsl,imx6sx-usdhc'] is too long
> >     'fsl,imx6ul-usdhc' is not one of ['fsl,imx25-esdhc', 'fsl,imx35-esdhc',
> >     'fsl,imx51-esdhc', 'fsl,imx53-esdhc', 'fsl,imx6q-usdhc',
> >     'fsl,imx6sl-usdhc', 'fsl,imx6sx-usdhc', 'fsl,imx7d-usdhc',
> >     'fsl,imx7ulp-usdhc', 'fsl,imx8mm-usdhc', 'fsl,imxrt1050-usdhc',
> >     'nxp,s32g2-usdhc']
> >   From schema: Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.yaml
> > 
> > Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> > ---
> >  Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.yaml | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.yaml b/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.yaml
> > index fbfd822b9270..090e781705d3 100644
> > --- a/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.yaml
> > +++ b/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.yaml
> > @@ -30,6 +30,7 @@ properties:
> >            - fsl,imx6q-usdhc
> >            - fsl,imx6sl-usdhc
> >            - fsl,imx6sx-usdhc
> > +          - fsl,imx6ul-usdhc
> 
> How come this gets added as a standalone compatible _and_ one that
> should fall back to imx6sx?

Whoops, sent too soon.
I meant to point out that the only user in-tree uses the fallback to
imx6sx.

Cheers,
Conor.

> >            - fsl,imx7d-usdhc
> >            - fsl,imx7ulp-usdhc
> >            - fsl,imx8mm-usdhc
> > @@ -42,6 +43,7 @@ properties:
> >            - enum:
> >                - fsl,imx6sll-usdhc
> >                - fsl,imx6ull-usdhc
> > +              - fsl,imx6ul-usdhc
> >            - const: fsl,imx6sx-usdhc
> >        - items:
> >            - const: fsl,imx7d-usdhc
> > -- 
> > 2.39.2
> > 



[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply

* Re: [PATCH v1 1/7] dt-bindings: mmc: fsl-imx-esdhc: Add imx6ul support
From: Conor Dooley @ 2023-06-01 18:18 UTC (permalink / raw)
  To: Oleksij Rempel
  Cc: Abel Vesa, Michael Turquette, Stephen Boyd, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Shawn Guo, Sascha Hauer,
	Herbert Xu, David S. Miller, Dmitry Torokhov, Ulf Hansson, kernel,
	Peng Fan, Fabio Estevam, NXP Linux Team, Daniel Lezcano,
	Thomas Gleixner, Michael Trimarchi, Mark Brown, Dario Binacchi,
	Anson Huang, Marek Vasut, linux-clk, devicetree, linux-arm-kernel,
	linux-kernel, linux-crypto, linux-input, linux-mmc
In-Reply-To: <20230601101451.357662-2-o.rempel@pengutronix.de>

[-- Attachment #1: Type: text/plain, Size: 2012 bytes --]

On Thu, Jun 01, 2023 at 12:14:45PM +0200, Oleksij Rempel wrote:
> Add the 'fsl,imx6ul-usdhc' value to the compatible properties list in
> the fsl-imx-esdhc.yaml file. This is required to match the compatible
> strings present in the 'mmc@2190000' node of 'imx6ul-prti6g.dtb'. This
> commit addresses the following dtbs_check warning:
>   imx6ul-prti6g.dtb: mmc@2190000: compatible: 'oneOf' conditional failed,
>     one must be fixed: ['fsl,imx6ul-usdhc', 'fsl,imx6sx-usdhc'] is too long
>     'fsl,imx6ul-usdhc' is not one of ['fsl,imx25-esdhc', 'fsl,imx35-esdhc',
>     'fsl,imx51-esdhc', 'fsl,imx53-esdhc', 'fsl,imx6q-usdhc',
>     'fsl,imx6sl-usdhc', 'fsl,imx6sx-usdhc', 'fsl,imx7d-usdhc',
>     'fsl,imx7ulp-usdhc', 'fsl,imx8mm-usdhc', 'fsl,imxrt1050-usdhc',
>     'nxp,s32g2-usdhc']
>   From schema: Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.yaml
> 
> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> ---
>  Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.yaml | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.yaml b/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.yaml
> index fbfd822b9270..090e781705d3 100644
> --- a/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.yaml
> +++ b/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.yaml
> @@ -30,6 +30,7 @@ properties:
>            - fsl,imx6q-usdhc
>            - fsl,imx6sl-usdhc
>            - fsl,imx6sx-usdhc
> +          - fsl,imx6ul-usdhc

How come this gets added as a standalone compatible _and_ one that
should fall back to imx6sx?

>            - fsl,imx7d-usdhc
>            - fsl,imx7ulp-usdhc
>            - fsl,imx8mm-usdhc
> @@ -42,6 +43,7 @@ properties:
>            - enum:
>                - fsl,imx6sll-usdhc
>                - fsl,imx6ull-usdhc
> +              - fsl,imx6ul-usdhc
>            - const: fsl,imx6sx-usdhc
>        - items:
>            - const: fsl,imx7d-usdhc
> -- 
> 2.39.2
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox