Linux Input/HID development
 help / color / mirror / Atom feed
* [PATCH v3 12/12] gpiolib: Clean up headers
From: Andy Shevchenko @ 2023-02-07 14:29 UTC (permalink / raw)
  To: Andy Shevchenko, Linus Walleij, Arnd Bergmann,
	Bartosz Golaszewski, Tony Lindgren, Thomas Gleixner,
	Krzysztof Kozlowski, Geert Uytterhoeven, Christophe Leroy,
	Michael Ellerman, Devarsh Thakkar, Dmitry Torokhov,
	Stefan Schmidt, Miquel Raynal, linux-gpio, linux-doc,
	linux-kernel, linux-doc-tw-discuss, linux-arm-kernel, linux-omap,
	linux-samsung-soc, linux-m68k, linuxppc-dev, linux-sh, linux-acpi,
	linux-input, linux-media, linux-wpan, netdev, linux-wireless,
	brcm80211-dev-list.pdl, SHA-cyfmac-dev-list, linux-arch,
	devicetree
  Cc: Bartosz Golaszewski, Jonathan Corbet, Alex Shi, Yanteng Si,
	Hu Haowen, Russell King, Aaro Koskinen, Janusz Krzysztofik,
	Andrew Lunn, Sebastian Hesselbarth, Gregory Clement, Alim Akhtar,
	Nicholas Piggin, Yoshinori Sato, Rich Felker, Mun Yew Tham,
	Keerthy, Mika Westerberg, Mauro Carvalho Chehab, Alexander Aring,
	David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Arend van Spriel, Franky Lin, Hante Meuleman, Kalle Valo,
	Qiang Zhao, Li Yang, Lee Jones, Rob Herring, Frank Rowand
In-Reply-To: <20230207142952.51844-1-andriy.shevchenko@linux.intel.com>

There is a few things done:
- include only the headers we are direct user of
- when pointer is in use, provide a forward declaration
- add missing headers
- group generic headers and subsystem headers
- sort each group alphabetically

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/gpio/gpiolib-acpi.c   | 10 ++++++----
 drivers/gpio/gpiolib-acpi.h   |  1 -
 drivers/gpio/gpiolib-of.c     |  6 ++++--
 drivers/gpio/gpiolib-of.h     |  1 -
 drivers/gpio/gpiolib-swnode.c |  5 +++--
 drivers/gpio/gpiolib-sysfs.c  | 21 ++++++++++++++++-----
 drivers/gpio/gpiolib.c        |  9 ++++++---
 include/linux/gpio.h          |  9 +++------
 include/linux/gpio/consumer.h | 14 ++++++++++----
 include/linux/gpio/driver.h   | 30 +++++++++++++++++++++++-------
 10 files changed, 71 insertions(+), 35 deletions(-)

diff --git a/drivers/gpio/gpiolib-acpi.c b/drivers/gpio/gpiolib-acpi.c
index bb583cea366c..3871dade186a 100644
--- a/drivers/gpio/gpiolib-acpi.c
+++ b/drivers/gpio/gpiolib-acpi.c
@@ -7,17 +7,19 @@
  *          Mika Westerberg <mika.westerberg@linux.intel.com>
  */
 
+#include <linux/acpi.h>
 #include <linux/dmi.h>
 #include <linux/errno.h>
-#include <linux/gpio/consumer.h>
-#include <linux/gpio/driver.h>
-#include <linux/gpio/machine.h>
 #include <linux/export.h>
-#include <linux/acpi.h>
 #include <linux/interrupt.h>
+#include <linux/irq.h>
 #include <linux/mutex.h>
 #include <linux/pinctrl/pinctrl.h>
 
+#include <linux/gpio/consumer.h>
+#include <linux/gpio/driver.h>
+#include <linux/gpio/machine.h>
+
 #include "gpiolib.h"
 #include "gpiolib-acpi.h"
 
diff --git a/drivers/gpio/gpiolib-acpi.h b/drivers/gpio/gpiolib-acpi.h
index 5fa315b3c912..a6f3be0bb921 100644
--- a/drivers/gpio/gpiolib-acpi.h
+++ b/drivers/gpio/gpiolib-acpi.h
@@ -9,7 +9,6 @@
 #define GPIOLIB_ACPI_H
 
 #include <linux/err.h>
-#include <linux/errno.h>
 #include <linux/types.h>
 
 #include <linux/gpio/consumer.h>
diff --git a/drivers/gpio/gpiolib-of.c b/drivers/gpio/gpiolib-of.c
index 0f699af438b0..1436cdb5fa26 100644
--- a/drivers/gpio/gpiolib-of.c
+++ b/drivers/gpio/gpiolib-of.c
@@ -10,14 +10,16 @@
 #include <linux/device.h>
 #include <linux/err.h>
 #include <linux/errno.h>
-#include <linux/module.h>
 #include <linux/io.h>
-#include <linux/gpio/consumer.h>
+#include <linux/module.h>
 #include <linux/of.h>
 #include <linux/of_address.h>
 #include <linux/of_gpio.h>
 #include <linux/pinctrl/pinctrl.h>
 #include <linux/slab.h>
+#include <linux/string.h>
+
+#include <linux/gpio/consumer.h>
 #include <linux/gpio/machine.h>
 
 #include "gpiolib.h"
diff --git a/drivers/gpio/gpiolib-of.h b/drivers/gpio/gpiolib-of.h
index e5bb065d82ef..6b3a5347c5d9 100644
--- a/drivers/gpio/gpiolib-of.h
+++ b/drivers/gpio/gpiolib-of.h
@@ -4,7 +4,6 @@
 #define GPIOLIB_OF_H
 
 #include <linux/err.h>
-#include <linux/errno.h>
 #include <linux/types.h>
 
 #include <linux/notifier.h>
diff --git a/drivers/gpio/gpiolib-swnode.c b/drivers/gpio/gpiolib-swnode.c
index dd9ccac214d1..b5a6eaf3729b 100644
--- a/drivers/gpio/gpiolib-swnode.c
+++ b/drivers/gpio/gpiolib-swnode.c
@@ -6,13 +6,14 @@
  */
 #include <linux/err.h>
 #include <linux/errno.h>
-#include <linux/gpio/consumer.h>
-#include <linux/gpio/driver.h>
 #include <linux/kernel.h>
 #include <linux/printk.h>
 #include <linux/property.h>
 #include <linux/string.h>
 
+#include <linux/gpio/consumer.h>
+#include <linux/gpio/driver.h>
+
 #include "gpiolib.h"
 #include "gpiolib-swnode.h"
 
diff --git a/drivers/gpio/gpiolib-sysfs.c b/drivers/gpio/gpiolib-sysfs.c
index 6e4267944f80..c1cbf71329f0 100644
--- a/drivers/gpio/gpiolib-sysfs.c
+++ b/drivers/gpio/gpiolib-sysfs.c
@@ -1,18 +1,29 @@
 // SPDX-License-Identifier: GPL-2.0
+
+#include <linux/bitops.h>
+#include <linux/device.h>
 #include <linux/idr.h>
+#include <linux/init.h>
+#include <linux/interrupt.h>
+#include <linux/kdev_t.h>
+#include <linux/kstrtox.h>
+#include <linux/list.h>
 #include <linux/mutex.h>
-#include <linux/device.h>
+#include <linux/printk.h>
+#include <linux/slab.h>
+#include <linux/spinlock.h>
+#include <linux/string.h>
 #include <linux/sysfs.h>
+#include <linux/types.h>
+
 #include <linux/gpio/consumer.h>
 #include <linux/gpio/driver.h>
-#include <linux/interrupt.h>
-#include <linux/kdev_t.h>
-#include <linux/slab.h>
-#include <linux/ctype.h>
 
 #include "gpiolib.h"
 #include "gpiolib-sysfs.h"
 
+struct kernfs_node;
+
 #define GPIO_IRQF_TRIGGER_NONE		0
 #define GPIO_IRQF_TRIGGER_FALLING	BIT(0)
 #define GPIO_IRQF_TRIGGER_RISING	BIT(1)
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 99a2c77c3711..900f6573c070 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -6,22 +6,25 @@
 #include <linux/debugfs.h>
 #include <linux/device.h>
 #include <linux/err.h>
+#include <linux/errno.h>
 #include <linux/file.h>
 #include <linux/fs.h>
-#include <linux/gpio.h>
-#include <linux/gpio/driver.h>
-#include <linux/gpio/machine.h>
 #include <linux/idr.h>
 #include <linux/interrupt.h>
 #include <linux/irq.h>
 #include <linux/kernel.h>
 #include <linux/list.h>
 #include <linux/module.h>
+#include <linux/of.h>
 #include <linux/pinctrl/consumer.h>
 #include <linux/seq_file.h>
 #include <linux/slab.h>
 #include <linux/spinlock.h>
 
+#include <linux/gpio.h>
+#include <linux/gpio/driver.h>
+#include <linux/gpio/machine.h>
+
 #include <uapi/linux/gpio.h>
 
 #include "gpiolib-acpi.h"
diff --git a/include/linux/gpio.h b/include/linux/gpio.h
index 84bb49939d6e..574c45be924b 100644
--- a/include/linux/gpio.h
+++ b/include/linux/gpio.h
@@ -12,7 +12,7 @@
 #ifndef __LINUX_GPIO_H
 #define __LINUX_GPIO_H
 
-#include <linux/errno.h>
+struct device;
 
 /* see Documentation/driver-api/gpio/legacy.rst */
 
@@ -132,20 +132,17 @@ void gpio_free_array(const struct gpio *array, size_t num);
 
 /* CONFIG_GPIOLIB: bindings for managed devices that want to request gpios */
 
-struct device;
-
 int devm_gpio_request(struct device *dev, unsigned gpio, const char *label);
 int devm_gpio_request_one(struct device *dev, unsigned gpio,
 			  unsigned long flags, const char *label);
 
 #else /* ! CONFIG_GPIOLIB */
 
-#include <linux/bug.h>
 #include <linux/kernel.h>
 #include <linux/types.h>
 
-struct device;
-struct gpio_chip;
+#include <asm/bug.h>
+#include <asm/errno.h>
 
 static inline bool gpio_is_valid(int number)
 {
diff --git a/include/linux/gpio/consumer.h b/include/linux/gpio/consumer.h
index 5432e5d5fbfb..1c4385a00f88 100644
--- a/include/linux/gpio/consumer.h
+++ b/include/linux/gpio/consumer.h
@@ -3,15 +3,14 @@
 #define __LINUX_GPIO_CONSUMER_H
 
 #include <linux/bits.h>
-#include <linux/bug.h>
-#include <linux/compiler_types.h>
-#include <linux/err.h>
+#include <linux/types.h>
 
 struct acpi_device;
 struct device;
 struct fwnode_handle;
-struct gpio_desc;
+
 struct gpio_array;
+struct gpio_desc;
 
 /**
  * struct gpio_descs - Struct containing an array of descriptors that can be
@@ -185,8 +184,11 @@ struct gpio_desc *devm_fwnode_gpiod_get_index(struct device *dev,
 
 #else /* CONFIG_GPIOLIB */
 
+#include <linux/err.h>
 #include <linux/kernel.h>
 
+#include <asm/bug.h>
+
 static inline int gpiod_count(struct device *dev, const char *con_id)
 {
 	return 0;
@@ -616,6 +618,8 @@ struct gpio_desc *acpi_get_and_request_gpiod(char *path, unsigned int pin, char
 
 #else  /* CONFIG_GPIOLIB && CONFIG_ACPI */
 
+#include <linux/err.h>
+
 static inline int acpi_dev_add_driver_gpios(struct acpi_device *adev,
 			      const struct acpi_gpio_mapping *gpios)
 {
@@ -647,6 +651,8 @@ void gpiod_unexport(struct gpio_desc *desc);
 
 #else  /* CONFIG_GPIOLIB && CONFIG_GPIO_SYSFS */
 
+#include <asm/errno.h>
+
 static inline int gpiod_export(struct gpio_desc *desc,
 			       bool direction_may_change)
 {
diff --git a/include/linux/gpio/driver.h b/include/linux/gpio/driver.h
index 262a84ce9bcb..5c6db5533be6 100644
--- a/include/linux/gpio/driver.h
+++ b/include/linux/gpio/driver.h
@@ -2,27 +2,35 @@
 #ifndef __LINUX_GPIO_DRIVER_H
 #define __LINUX_GPIO_DRIVER_H
 
-#include <linux/device.h>
-#include <linux/irq.h>
+#include <linux/bits.h>
 #include <linux/irqchip/chained_irq.h>
 #include <linux/irqdomain.h>
+#include <linux/irqhandler.h>
 #include <linux/lockdep.h>
 #include <linux/pinctrl/pinconf-generic.h>
 #include <linux/pinctrl/pinctrl.h>
 #include <linux/property.h>
+#include <linux/spinlock_types.h>
 #include <linux/types.h>
 
+#ifdef CONFIG_GENERIC_MSI_IRQ
 #include <asm/msi.h>
+#endif
 
-struct gpio_desc;
+struct device;
+struct irq_chip;
+struct irq_data;
+struct module;
 struct of_phandle_args;
+struct pinctrl_dev;
 struct seq_file;
-struct gpio_device;
-struct module;
-enum gpiod_flags;
-enum gpio_lookup_flags;
 
 struct gpio_chip;
+struct gpio_desc;
+struct gpio_device;
+
+enum gpio_lookup_flags;
+enum gpiod_flags;
 
 union gpio_irq_fwspec {
 	struct irq_fwspec	fwspec;
@@ -679,6 +687,10 @@ bool gpiochip_irqchip_irq_valid(const struct gpio_chip *gc,
 int gpiochip_irqchip_add_domain(struct gpio_chip *gc,
 				struct irq_domain *domain);
 #else
+
+#include <asm/bug.h>
+#include <asm/errno.h>
+
 static inline int gpiochip_irqchip_add_domain(struct gpio_chip *gc,
 					      struct irq_domain *domain)
 {
@@ -756,6 +768,10 @@ struct gpio_chip *gpiod_to_chip(const struct gpio_desc *desc);
 
 #else /* CONFIG_GPIOLIB */
 
+#include <linux/err.h>
+
+#include <asm/bug.h>
+
 static inline struct gpio_chip *gpiod_to_chip(const struct gpio_desc *desc)
 {
 	/* GPIO can never have been requested */
-- 
2.39.1


^ permalink raw reply related

* [PATCH v3 09/12] gpiolib: Drop unused forward declaration from driver.h
From: Andy Shevchenko @ 2023-02-07 14:29 UTC (permalink / raw)
  To: Andy Shevchenko, Linus Walleij, Arnd Bergmann,
	Bartosz Golaszewski, Tony Lindgren, Thomas Gleixner,
	Krzysztof Kozlowski, Geert Uytterhoeven, Christophe Leroy,
	Michael Ellerman, Devarsh Thakkar, Dmitry Torokhov,
	Stefan Schmidt, Miquel Raynal, linux-gpio, linux-doc,
	linux-kernel, linux-doc-tw-discuss, linux-arm-kernel, linux-omap,
	linux-samsung-soc, linux-m68k, linuxppc-dev, linux-sh, linux-acpi,
	linux-input, linux-media, linux-wpan, netdev, linux-wireless,
	brcm80211-dev-list.pdl, SHA-cyfmac-dev-list, linux-arch,
	devicetree
  Cc: Bartosz Golaszewski, Jonathan Corbet, Alex Shi, Yanteng Si,
	Hu Haowen, Russell King, Aaro Koskinen, Janusz Krzysztofik,
	Andrew Lunn, Sebastian Hesselbarth, Gregory Clement, Alim Akhtar,
	Nicholas Piggin, Yoshinori Sato, Rich Felker, Mun Yew Tham,
	Keerthy, Mika Westerberg, Mauro Carvalho Chehab, Alexander Aring,
	David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Arend van Spriel, Franky Lin, Hante Meuleman, Kalle Valo,
	Qiang Zhao, Li Yang, Lee Jones, Rob Herring, Frank Rowand
In-Reply-To: <20230207142952.51844-1-andriy.shevchenko@linux.intel.com>

There is no struct device_node pointers anywhere in the header,
drop unused forward declaration.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 include/linux/gpio/driver.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/include/linux/gpio/driver.h b/include/linux/gpio/driver.h
index ccd8a512d854..262a84ce9bcb 100644
--- a/include/linux/gpio/driver.h
+++ b/include/linux/gpio/driver.h
@@ -16,7 +16,6 @@
 
 struct gpio_desc;
 struct of_phandle_args;
-struct device_node;
 struct seq_file;
 struct gpio_device;
 struct module;
-- 
2.39.1


^ permalink raw reply related

* Re: [PATCH v3 06/12] gpiolib: split linux/gpio/driver.h out of linux/gpio.h
From: Linus Walleij @ 2023-02-07 14:55 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Arnd Bergmann, Bartosz Golaszewski, Tony Lindgren,
	Thomas Gleixner, Krzysztof Kozlowski, Geert Uytterhoeven,
	Christophe Leroy, Michael Ellerman, Devarsh Thakkar,
	Dmitry Torokhov, Stefan Schmidt, Miquel Raynal, linux-gpio,
	linux-doc, linux-kernel, linux-doc-tw-discuss, linux-arm-kernel,
	linux-omap, linux-samsung-soc, linux-m68k, linuxppc-dev, linux-sh,
	linux-acpi, linux-input, linux-media, linux-wpan, netdev,
	linux-wireless, brcm80211-dev-list.pdl, SHA-cyfmac-dev-list,
	linux-arch, devicetree, Bartosz Golaszewski, Jonathan Corbet,
	Alex Shi, Yanteng Si, Hu Haowen, Russell King, Aaro Koskinen,
	Janusz Krzysztofik, Andrew Lunn, Sebastian Hesselbarth,
	Gregory Clement, Alim Akhtar, Nicholas Piggin, Yoshinori Sato,
	Rich Felker, Mun Yew Tham, Keerthy, Mika Westerberg,
	Mauro Carvalho Chehab, Alexander Aring, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Arend van Spriel,
	Franky Lin, Hante Meuleman, Kalle Valo, Qiang Zhao, Li Yang,
	Lee Jones, Rob Herring, Frank Rowand
In-Reply-To: <20230207142952.51844-7-andriy.shevchenko@linux.intel.com>

On Tue, Feb 7, 2023 at 3:29 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:

> From: Arnd Bergmann <arnd@arndb.de>
>
> Almost all gpio drivers include linux/gpio/driver.h, and other
> files should not rely on includes from this header.
>
> Remove the indirect include from here and include the correct
> headers directly from where they are used.
>
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Make sure you push this to the kernel.org build servers (zeroday builds),
I think this patch needs to hit some more files, in my tests with a similar
patch at least these:

diff --git a/drivers/hte/hte-tegra194-test.c b/drivers/hte/hte-tegra194-test.c
index 5d776a185bd6..79eb866558d3 100644
--- a/drivers/hte/hte-tegra194-test.c
+++ b/drivers/hte/hte-tegra194-test.c
@@ -6,10 +6,11 @@
  */

 #include <linux/err.h>
+#include <linux/mod_devicetable.h>
 #include <linux/module.h>
 #include <linux/moduleparam.h>
 #include <linux/interrupt.h>
-#include <linux/gpio.h>
+#include <linux/gpio/consumer.h>
 #include <linux/timer.h>
 #include <linux/platform_device.h>
 #include <linux/workqueue.h>


diff --git a/arch/arm/mach-pxa/viper-pcmcia.c b/arch/arm/mach-pxa/viper-pcmcia.c
index 26599dcc49b3..2c7af4ed57d5 100644
--- a/arch/arm/mach-pxa/viper-pcmcia.c
+++ b/arch/arm/mach-pxa/viper-pcmcia.c
@@ -19,6 +19,7 @@
 #include <linux/errno.h>
 #include <linux/interrupt.h>
 #include <linux/platform_device.h>
+#include <linux/mod_devicetable.h>
 #include <linux/gpio.h>

 #include <pcmcia/ss.h>

Yours,
Linus Walleij

^ permalink raw reply related

* Re: [PATCH 1/7] arm64: dts: qcom: sc7280: On QCard, regulator L3C should be 1.8V
From: Matthias Kaehlcke @ 2023-02-07 18:12 UTC (permalink / raw)
  To: Douglas Anderson
  Cc: Bjorn Andersson, Rob Herring, Krzysztof Kozlowski, Jiri Kosina,
	Benjamin Tissoires, linux-input, linux-arm-msm, Dmitry Torokhov,
	devicetree, Stephen Kitt, Konrad Dybcio, Andy Gross, linux-kernel
In-Reply-To: <20230206184744.1.I784f4b3d7e4a06edafff4a3129f52e749889bc05@changeid>

On Mon, Feb 06, 2023 at 06:48:10PM -0800, Douglas Anderson wrote:
> On the first sc7280 QCards the L3C rail was never really used for
> anything. Stuffing options on the QCard meant that the QCard itself
> didn't use this rail for anything. This rail did get sent to the
> mainboard, but no existing mainboards ever did anything with it other
> that route it to a testpoint.

nit: s/that/than/

no need to re-spin just for that

> On later sc7280 QCards, the L3C rail was repurposed. Instead of being
> a (nominally) 3.3V rail, it was decided to make it a 1.8V rail. It is
> now provided to the display connector (which might route it to the
> touchscreen) and also used to power some buffers relating to
> touchscreen IO. This rail is getting the additional tag "ts_avccio",
> though some places still refer to it as "vreg_l3c_3p0" despite the
> fact that the name now specifies the wrong voltage.
> 
> Since it never hurts for this rail to be 1.8V (even on old QCards /
> old boards), let's just change it to 1.8V across the board and add the
> extra "ts_avccio" moniker as a label in the device tree.
> 
> Future patches will start using this rail in their touchscreens.
> 
> Signed-off-by: Douglas Anderson <dianders@chromium.org>

Reviewed-by: Matthias Kaehlcke <mka@chromium.org>

^ permalink raw reply

* Re: [PATCH 2/7] arm64: dts: qcom: sc7280: Add 3ms ramp to herobrine's pp3300_left_in_mlb
From: Matthias Kaehlcke @ 2023-02-07 18:15 UTC (permalink / raw)
  To: Douglas Anderson
  Cc: Bjorn Andersson, Rob Herring, Krzysztof Kozlowski, Jiri Kosina,
	Benjamin Tissoires, linux-input, linux-arm-msm, Dmitry Torokhov,
	devicetree, Stephen Kitt, Konrad Dybcio, Andy Gross, linux-kernel
In-Reply-To: <20230206184744.2.I13814cefc5ab3e0a39ebd09f052e3fd25d4e8f1d@changeid>

On Mon, Feb 06, 2023 at 06:48:11PM -0800, Douglas Anderson wrote:
> The "pp3300_left_in_mlb" rail on herobrine eventually connects up to
> "vreg_edp_3p3" on the qcard. On several herobrine designs this rail
> has been measured to need more than 1ms to turn on.
> 
> While technically a herobrine derivative (defined as anyone including
> the "herobrine.dtsi") could change the board to make the rail rise
> faster or slower, the fact that two boards (evoker and villager) both
> measured it as taking more than 1ms implies that it's probably going
> to be the norm. Thus, let's add a "regulator-enable-ramp-delay"
> straight into the herobrine.dtsi to handle this. If a particular
> derivative board needs a faster or slower one then they can override
> it, though that feels unlikely.
> 
> While we measured something a bit over 1ms, we'll choose 3ms to give
> us a tiny bit of margin. This isn't a rail that turns off and on all
> the time anyway and 3ms is nothing compared to the total amount of
> time to power on a panel.
> 
> Signed-off-by: Douglas Anderson <dianders@chromium.org>

Reviewed-by: Matthias Kaehlcke <mka@chromium.org>

^ permalink raw reply

* Re: [PATCH 3/7] arm64: dts: qcom: sc7280: Hook up the touchscreen IO rail on villager
From: Matthias Kaehlcke @ 2023-02-07 18:18 UTC (permalink / raw)
  To: Douglas Anderson
  Cc: Bjorn Andersson, Rob Herring, Krzysztof Kozlowski, Jiri Kosina,
	Benjamin Tissoires, linux-input, linux-arm-msm, Dmitry Torokhov,
	devicetree, Stephen Kitt, Konrad Dybcio, Andy Gross, linux-kernel
In-Reply-To: <20230206184744.3.I740d409bc5bb69bf4a7b3c4568ea6e7a92f16ccd@changeid>

On Mon, Feb 06, 2023 at 06:48:12PM -0800, Douglas Anderson wrote:
> On never revs of sc7280-herobrine-villager (rev2+) the L3C rail is

nit: s/never/newer/

no need to re-spin just for this.

> provided to the touchscreen as the IO voltage rail. Let's add it in
> the device tree.
> 
> NOTE: Even though this is only really needed on rev2+ villagers (-rev0
> had non-functioning touchscreen and -rev1 had some hacky hardware
> magic), it doesn't actually hurt to do this for old villager revs. As
> talked about in the patch ("arm64: dts: qcom: sc7280: On QCard,
> regulator L3C should be 1.8V") the L3C regulator didn't go anywhere at
> all on older revs. That means that turning it on for older revs
> doesn't hurt other than drawing a tiny bit of extra power. Since -rev0
> and -rev1 villagers will never make it to real customers and it's nice
> not to have too many old device trees, the better tradeoff seems to be
> to enable it everywhere.
> 
> Signed-off-by: Douglas Anderson <dianders@chromium.org>

Reviewed-by: Matthias Kaehlcke <mka@chromium.org>

^ permalink raw reply

* Re: [PATCH 4/7] HID: i2c-hid: goodix: Stop tying the reset line to the regulator
From: Matthias Kaehlcke @ 2023-02-07 18:31 UTC (permalink / raw)
  To: Douglas Anderson
  Cc: Bjorn Andersson, Rob Herring, Krzysztof Kozlowski, Jiri Kosina,
	Benjamin Tissoires, linux-input, linux-arm-msm, Dmitry Torokhov,
	devicetree, Stephen Kitt, Konrad Dybcio, linux-kernel
In-Reply-To: <20230206184744.4.I085b32b6140c7d1ac4e7e97b712bff9dd5962b62@changeid>

On Mon, Feb 06, 2023 at 06:48:13PM -0800, Douglas Anderson wrote:
> In commit 18eeef46d359 ("HID: i2c-hid: goodix: Tie the reset line to
> true state of the regulator"), we started tying the reset line of
> Goodix touchscreens to the regulator.
> 
> The primary motivation for that patch was some pre-production hardware
> (specifically sc7180-trogdor-homestar) where it was proposed to hook
> the touchscreen's main 3.3V power rail to an always-on supply. In such
> a case, when we turned "off" the touchscreen in Linux it was bad to
> assert the "reset" GPIO because that was causing a power drain. The
> patch accomplished that goal and did it in a general sort of way that
> didn't require special properties to be added in the device tree for
> homestar.
> 
> It turns out that the design of using an always-on power rail for the
> touchscreen was rejected soon after the patch was written and long
> before sc7180-trogdor-homestar went into production. The final design
> of homestar actually fully separates the rail for the touchscreen and
> the display panel and both can be powered off and on. That means that
> the original motivation for the feature is gone.
> 
> There are 3 other users of the goodix i2c-hid driver in mainline.
> 
> I'll first talk about 2 of the other users in mainline: coachz and
> mrbland. On both coachz and mrbland the touchscreen power and panel
> power _are_ shared. That means that the patch to tie the reset line to
> the true state of the regulator _is_ doing something on those
> boards. Specifically, the patch reduced power consumption by tens of
> mA in the case where we turned the touchscreen off but left the panel
> on. Other than saving a small bit of power, the patch wasn't truly
> necessary. That being said, even though a small bit of power was saved
> in the state of "panel on + touchscreen off", that's not actually a
> state we ever expect to be in, except perhaps for very short periods
> of time at boot or during suspend/resume. Thus, the patch is truly not
> necessary. It should be further noted that, as documented in the
> original patch, the current code still didn't optimize power for every
> corner case of the "shared rail" situation.
> 
> The last user in mainline was very recently added: evoker. Evoker is
> actually the motivation for me removing this bit of code. It turns out
> that for evoker we need to manage a second power rail for IO to the
> touchscreen. Trying to fit the management of this IO rail into the
> regulator notifiers turns out to be extremely hard. To avoid lockdep
> splats you shouldn't enable/disable other regulators in regulator
> notifiers and trying to find a way around this was going to be fairly
> difficult.
> 
> Given the lack of any true motivation to tie the reset line to the
> regulator, lets go back to the simpler days and remove the code. This
> is, effectively, a revert of commit bdbc65eb77ee ("HID: i2c-hid:
> goodix: Fix a lockdep splat"), commit 25ddd7cfc582 ("HID: i2c-hid:
> goodix: Use the devm variant of regulator_register_notifier()"), and
> commit 18eeef46d359 ("HID: i2c-hid: goodix: Tie the reset line to true
> state of the regulator").
> 
> Signed-off-by: Douglas Anderson <dianders@chromium.org>

Reviewed-by: Matthias Kaehlcke <mka@chromium.org>

^ permalink raw reply

* Re: [PATCH v3 06/12] gpiolib: split linux/gpio/driver.h out of linux/gpio.h
From: Lee Jones @ 2023-02-07 18:43 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Linus Walleij, Arnd Bergmann, Bartosz Golaszewski, Tony Lindgren,
	Thomas Gleixner, Krzysztof Kozlowski, Geert Uytterhoeven,
	Christophe Leroy, Michael Ellerman, Devarsh Thakkar,
	Dmitry Torokhov, Stefan Schmidt, Miquel Raynal, linux-gpio,
	linux-doc, linux-kernel, linux-doc-tw-discuss, linux-arm-kernel,
	linux-omap, linux-samsung-soc, linux-m68k, linuxppc-dev, linux-sh,
	linux-acpi, linux-input, linux-media, linux-wpan, netdev,
	linux-wireless, brcm80211-dev-list.pdl, SHA-cyfmac-dev-list,
	linux-arch, devicetree, Bartosz Golaszewski, Jonathan Corbet,
	Alex Shi, Yanteng Si, Hu Haowen, Russell King, Aaro Koskinen,
	Janusz Krzysztofik, Andrew Lunn, Sebastian Hesselbarth,
	Gregory Clement, Alim Akhtar, Nicholas Piggin, Yoshinori Sato,
	Rich Felker, Mun Yew Tham, Keerthy, Mika Westerberg,
	Mauro Carvalho Chehab, Alexander Aring, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Arend van Spriel,
	Franky Lin, Hante Meuleman, Kalle Valo, Qiang Zhao, Li Yang,
	Rob Herring, Frank Rowand
In-Reply-To: <20230207142952.51844-7-andriy.shevchenko@linux.intel.com>

On Tue, 07 Feb 2023, Andy Shevchenko wrote:

> From: Arnd Bergmann <arnd@arndb.de>
> 
> Almost all gpio drivers include linux/gpio/driver.h, and other
> files should not rely on includes from this header.
> 
> Remove the indirect include from here and include the correct
> headers directly from where they are used.
> 
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  arch/arm/mach-omap1/irq.c                              | 1 +
>  arch/arm/mach-orion5x/board-rd88f5182.c                | 1 +
>  arch/arm/mach-s3c/s3c64xx.c                            | 1 +
>  arch/arm/mach-sa1100/assabet.c                         | 1 +
>  arch/arm/plat-orion/gpio.c                             | 1 +
>  drivers/net/wireless/broadcom/brcm80211/brcmsmac/led.c | 1 +
>  include/linux/gpio.h                                   | 2 --
>  include/linux/mfd/ucb1x00.h                            | 1 +

Acked-by: Lee Jones <lee@kernel.org>

>  8 files changed, 7 insertions(+), 2 deletions(-)

-- 
Lee Jones [李琼斯]

^ permalink raw reply

* Re: [PATCH 6/7] HID: i2c-hid: goodix: Add mainboard-vddio-supply
From: Matthias Kaehlcke @ 2023-02-07 18:46 UTC (permalink / raw)
  To: Douglas Anderson
  Cc: Bjorn Andersson, Rob Herring, Krzysztof Kozlowski, Jiri Kosina,
	Benjamin Tissoires, linux-input, linux-arm-msm, Dmitry Torokhov,
	devicetree, Stephen Kitt, Konrad Dybcio, linux-kernel
In-Reply-To: <20230206184744.6.Ic234b931025d1f920ce9e06fff294643943a65ad@changeid>

On Mon, Feb 06, 2023 at 06:48:15PM -0800, Douglas Anderson wrote:
> As talked about in the patch ("dt-bindings: HID: i2c-hid: goodix: Add
> mainboard-vddio-supply") we may need to power up a 1.8V rail on the
> host associated with touchscreen IO. Let's add support in the driver
> for it.
> 
> Signed-off-by: Douglas Anderson <dianders@chromium.org>

Reviewed-by: Matthias Kaehlcke <mka@chromium.org>

^ permalink raw reply

* Re: [PATCH 7/7] arm64: dts: qcom: sc7280: Hook up the touchscreen IO rail on evoker
From: Matthias Kaehlcke @ 2023-02-07 18:47 UTC (permalink / raw)
  To: Douglas Anderson
  Cc: Bjorn Andersson, Rob Herring, Krzysztof Kozlowski, Jiri Kosina,
	Benjamin Tissoires, linux-input, linux-arm-msm, Dmitry Torokhov,
	devicetree, Stephen Kitt, Konrad Dybcio, Andy Gross, linux-kernel
In-Reply-To: <20230206184744.7.I2d960ed7f2445db0cf3d227fde985fbd740f3c4d@changeid>

On Mon, Feb 06, 2023 at 06:48:16PM -0800, Douglas Anderson wrote:
> On older revisions of evoker, the touchscreen was either
> non-functional or needed special hardware magic to get it talking
> properly. It's been decided that the proper way going forward is to
> use L3C to power some buffers on the QCard and then configure the
> touchscreens for 1.8V. Let's do that.
> 
> Note that this is safe to do even on older revs even if it might not
> make the touchscreen work there (because they didn't have a properly
> stuffed QCard). As talked about in the patch ("arm64: dts: qcom:
> sc7280: On QCard, regulator L3C should be 1.8V") the L3C regulator
> didn't go anywhere at all on older revs.
> 
> This patch relies on the patch ("HID: i2c-hid: goodix: Add
> mainboard-vddio-supply") in order to function properly. Without that
> patch this one won't do any harm but it won't actually accomplish its
> goal.
> 
> Signed-off-by: Douglas Anderson <dianders@chromium.org>

Reviewed-by: Matthias Kaehlcke <mka@chromium.org>

^ permalink raw reply

* Re: [PATCH v4 1/4] dt-bindings: i2c: Add CP2112 HID USB to SMBus Bridge
From: Rob Herring @ 2023-02-07 18:50 UTC (permalink / raw)
  To: Danny Kaehn
  Cc: krzysztof.kozlowski+dt, jikos, benjamin.tissoires, devicetree,
	linux-input, ethan.twardy
In-Reply-To: <20230206135016.6737-2-kaehndan@gmail.com>

On Mon, Feb 06, 2023 at 07:50:13AM -0600, Danny Kaehn wrote:
> This is a USB HID device which includes an I2C controller and 8 GPIO pins.
> 
> The binding allows describing the chip's gpio and i2c controller in DT
> using the subnodes named "gpio" and "i2c", respectively. This is
> intended to be used in configurations where the CP2112 is permanently
> connected in hardware.

My comments on v3 still apply. Please slow down your pace of sending new 
versions so folks have change to review.

Rob

^ permalink raw reply

* [PATCH] Add support for Logitech G923 Xbox Edition steering wheel in hid-logitech-hidpp. We get the same level of features as the regular G920.
From: Walt Holman @ 2023-02-07 19:50 UTC (permalink / raw)
  To: linux-input, linux-kernel; +Cc: benjamin.tissoires, Walt Holman

---
 drivers/hid/hid-ids.h            | 1 +
 drivers/hid/hid-logitech-hidpp.c | 3 +++
 2 files changed, 4 insertions(+)

diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index 0f8c11842a3a..de1f385b0ecc 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -819,6 +819,7 @@
 #define USB_DEVICE_ID_LOGITECH_G510_USB_AUDIO	0xc22e
 #define USB_DEVICE_ID_LOGITECH_G29_WHEEL	0xc24f
 #define USB_DEVICE_ID_LOGITECH_G920_WHEEL	0xc262
+#define USB_DEVICE_ID_LOGITECH_G923_XBOX_WHEEL	0xc26e
 #define USB_DEVICE_ID_LOGITECH_WINGMAN_F3D	0xc283
 #define USB_DEVICE_ID_LOGITECH_FORCE3D_PRO	0xc286
 #define USB_DEVICE_ID_LOGITECH_FLIGHT_SYSTEM_G940	0xc287
diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
index abf2c95e4d0b..4dc833c8a44c 100644
--- a/drivers/hid/hid-logitech-hidpp.c
+++ b/drivers/hid/hid-logitech-hidpp.c
@@ -4347,6 +4347,9 @@ static const struct hid_device_id hidpp_devices[] = {
 	{ /* Logitech G920 Wheel over USB */
 	  HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_LOGITECH_G920_WHEEL),
 		.driver_data = HIDPP_QUIRK_CLASS_G920 | HIDPP_QUIRK_FORCE_OUTPUT_REPORTS},
+	{ /* Logitech G923 Wheel (Xbox version) over USB */
+	  HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_LOGITECH_G923_XBOX_WHEEL),
+		.driver_data = HIDPP_QUIRK_CLASS_G920 | HIDPP_QUIRK_FORCE_OUTPUT_REPORTS },
 	{ /* Logitech G Pro Gaming Mouse over USB */
 	  HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, 0xC088) },
 
-- 
2.37.2

--signed-of-by: Walt Holman (waltholman09@gmail.com)


^ permalink raw reply related

* Re: [PATCH v3 01/12] gpiolib: remove empty asm/gpio.h files
From: Vincenzo Palazzo @ 2023-02-07 20:53 UTC (permalink / raw)
  To: Andy Shevchenko, Linus Walleij, Arnd Bergmann,
	Bartosz Golaszewski, Tony Lindgren, Thomas Gleixner,
	Krzysztof Kozlowski, Geert Uytterhoeven, Christophe Leroy,
	Michael Ellerman, Devarsh Thakkar, Dmitry Torokhov,
	Stefan Schmidt, Miquel Raynal, linux-gpio, linux-doc,
	linux-kernel, linux-doc-tw-discuss, linux-arm-kernel, linux-omap,
	linux-samsung-soc, linux-m68k, linuxppc-dev, linux-sh, linux-acpi,
	linux-input, linux-media, linux-wpan, netdev, linux-wireless,
	brcm80211-dev-list.pdl, SHA-cyfmac-dev-list, linux-arch,
	devicetree
  Cc: Bartosz Golaszewski, Jonathan Corbet, Alex Shi, Yanteng Si,
	Hu Haowen, Russell King, Aaro Koskinen, Janusz Krzysztofik,
	Andrew Lunn, Sebastian Hesselbarth, Gregory Clement, Alim Akhtar,
	Nicholas Piggin, Yoshinori Sato, Rich Felker, Mun Yew Tham,
	Keerthy, Mika Westerberg, Mauro Carvalho Chehab, Alexander Aring,
	David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Arend van Spriel, Franky Lin, Hante Meuleman, Kalle Valo,
	Qiang Zhao, Li Yang, Lee Jones, Rob Herring, Frank Rowand,
	Geert Uytterhoeven
In-Reply-To: <20230207142952.51844-2-andriy.shevchenko@linux.intel.com>

> From: Arnd Bergmann <arnd@arndb.de>
>
> The arm and sh versions of this file are identical to the generic
> versions and can just be removed.
>
> The drivers that actually use the sh3 specific version also include
> cpu/gpio.h directly, with the exception of magicpanelr2, which is
> easily fixed. This leaves coldfire as the only gpio driver
> that needs something custom for gpiolib.
>
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Acked-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Reviewed-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>

^ permalink raw reply

* [PATCH] HID: retain initial quirks set up when creating HID devices
From: Dmitry Torokhov @ 2023-02-07 21:15 UTC (permalink / raw)
  To: Jiri Kosina, Benjamin Tissoires
  Cc: Allen Ballway, Hans de Goede, Alistair Francis, Guenter Roeck,
	linux-input, linux-kernel

In certain circumstances, such as when creating I2C-connected HID
devices, we want to pass and retain some querks (axis inversion, etc).
The source of such quirks may be device tree, or DMI data, or something
else not readily available to the HID core itself and therefore cannot
be reconstructed easily. To allow this introduce "initial_quirks" field
in hid_device structure and use it when determining the final set of
quirks.

This fixes problem with i2c-hid setting up device-tree sourced quirks
too late and losing them on device rebind, and also allows to sever the
tie between hid-code and i2c-hid when applying DMI-based quirks.

Fixes: b60d3c803d76 ("HID: i2c-hid-of: Expose the touchscreen-inverted properties")
Fixes: a2f416bf062a ("HID: multitouch: Add quirks for flipped axes")
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---

Compiled only, not tested on hardware.

 drivers/hid/hid-quirks.c                 | 8 +-------
 drivers/hid/i2c-hid/i2c-hid-core.c       | 6 ++++--
 drivers/hid/i2c-hid/i2c-hid-dmi-quirks.c | 1 -
 include/linux/hid.h                      | 1 +
 4 files changed, 6 insertions(+), 10 deletions(-)

diff --git a/drivers/hid/hid-quirks.c b/drivers/hid/hid-quirks.c
index 30e35f79def4..66e64350f138 100644
--- a/drivers/hid/hid-quirks.c
+++ b/drivers/hid/hid-quirks.c
@@ -19,7 +19,6 @@
 #include <linux/input/elan-i2c-ids.h>
 
 #include "hid-ids.h"
-#include "i2c-hid/i2c-hid.h"
 
 /*
  * Alphabetically sorted by vendor then product.
@@ -1238,7 +1237,7 @@ EXPORT_SYMBOL_GPL(hid_quirks_exit);
 static unsigned long hid_gets_squirk(const struct hid_device *hdev)
 {
 	const struct hid_device_id *bl_entry;
-	unsigned long quirks = 0;
+	unsigned long quirks = hdev->initial_quirks;
 
 	if (hid_match_id(hdev, hid_ignore_list))
 		quirks |= HID_QUIRK_IGNORE;
@@ -1299,11 +1298,6 @@ unsigned long hid_lookup_quirk(const struct hid_device *hdev)
 		quirks = hid_gets_squirk(hdev);
 	mutex_unlock(&dquirks_lock);
 
-	/* Get quirks specific to I2C devices */
-	if (IS_ENABLED(CONFIG_I2C_DMI_CORE) && IS_ENABLED(CONFIG_DMI) &&
-	    hdev->bus == BUS_I2C)
-		quirks |= i2c_hid_get_dmi_quirks(hdev->vendor, hdev->product);
-
 	return quirks;
 }
 EXPORT_SYMBOL_GPL(hid_lookup_quirk);
diff --git a/drivers/hid/i2c-hid/i2c-hid-core.c b/drivers/hid/i2c-hid/i2c-hid-core.c
index 0ab8f47a84e9..efbba0465eef 100644
--- a/drivers/hid/i2c-hid/i2c-hid-core.c
+++ b/drivers/hid/i2c-hid/i2c-hid-core.c
@@ -1025,6 +1025,10 @@ int i2c_hid_core_probe(struct i2c_client *client, struct i2chid_ops *ops,
 	hid->vendor = le16_to_cpu(ihid->hdesc.wVendorID);
 	hid->product = le16_to_cpu(ihid->hdesc.wProductID);
 
+	hid->initial_quirks = quirks;
+	hid->initial_quirks |= i2c_hid_get_dmi_quirks(hid->vendor,
+						      hid->product);
+
 	snprintf(hid->name, sizeof(hid->name), "%s %04X:%04X",
 		 client->name, (u16)hid->vendor, (u16)hid->product);
 	strscpy(hid->phys, dev_name(&client->dev), sizeof(hid->phys));
@@ -1038,8 +1042,6 @@ int i2c_hid_core_probe(struct i2c_client *client, struct i2chid_ops *ops,
 		goto err_mem_free;
 	}
 
-	hid->quirks |= quirks;
-
 	return 0;
 
 err_mem_free:
diff --git a/drivers/hid/i2c-hid/i2c-hid-dmi-quirks.c b/drivers/hid/i2c-hid/i2c-hid-dmi-quirks.c
index 554a7dc28536..210f17c3a0be 100644
--- a/drivers/hid/i2c-hid/i2c-hid-dmi-quirks.c
+++ b/drivers/hid/i2c-hid/i2c-hid-dmi-quirks.c
@@ -492,4 +492,3 @@ u32 i2c_hid_get_dmi_quirks(const u16 vendor, const u16 product)
 
 	return quirks;
 }
-EXPORT_SYMBOL_GPL(i2c_hid_get_dmi_quirks);
diff --git a/include/linux/hid.h b/include/linux/hid.h
index daaac4d7f370..56dac09c99d9 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -622,6 +622,7 @@ struct hid_device {							/* device report descriptor */
 	unsigned long status;						/* see STAT flags above */
 	unsigned claimed;						/* Claimed by hidinput, hiddev? */
 	unsigned quirks;						/* Various quirks the device can pull on us */
+	unsigned initial_quirks;					/* Initial set of quirks supplied when creating device */
 	bool io_started;						/* If IO has started */
 
 	struct list_head inputs;					/* The list of inputs */
-- 
2.39.1.519.gcb327c4b5f-goog


-- 
Dmitry

^ permalink raw reply related

* Re: [PATCH 5/7] dt-bindings: HID: i2c-hid: goodix: Add mainboard-vddio-supply
From: Rob Herring @ 2023-02-07 21:19 UTC (permalink / raw)
  To: Douglas Anderson
  Cc: Krzysztof Kozlowski, linux-input, Stephen Kitt, Bjorn Andersson,
	linux-arm-msm, linux-kernel, Dmitry Torokhov, Benjamin Tissoires,
	devicetree, Konrad Dybcio, Rob Herring, Jiri Kosina
In-Reply-To: <20230206184744.5.Ia77a96c6c5564f9cc25e6220b5a9171d5c2639e8@changeid>


On Mon, 06 Feb 2023 18:48:14 -0800, Douglas Anderson wrote:
> The goodix i2c-hid bindings currently support two models of
> touchscreen: GT7375P and GT7986U. The datasheets of both touchscreens
> show the following things:
> * The mainboard that the touchscreen is connected to is only expected
>   to supply one voltage to the touchscreen: 3.3V.
> * The touchscreen, depending on stuffing options, can accept IO to the
>   touchscreen as either 3.3V or 1.8V. Presumably this means that the
>   touchscreen has its own way internally to make or deal with 1.8V
>   signals when it's configured for 1.8V IO.
> 
> NOTE: you've got to look very carefully at the datasheet for the
> touchscreen to see that the above bullets are true. Specifically, the
> datasheet shows a signal called VDDIO and one might think that this is
> where a mainboard would provide VDDIO to the touchscreen. Upon closer
> inspection, however, a footnote can be found that says "When VDDIO is
> left floating, the logic level is 1.8V [...]; when VDDIO is connected
> to AVDD, the logic level is AVDD.". Thus the VDDIO pin on the
> touchscreen IC is actually a selector and not a pin whre the mainboard
> would pass a reference voltage.
> 
> The fact that the touchscreen isn't supplied 1.8V by the mainboard
> means that when I originally submitted bindings for these touchscreens
> I only listed the 3.3V rail in the bindings. It can be noted that the
> original bindings and driver were added for sc7180-trogdor boards and
> these boards all use 3.3V IO via a level shifter on the mainboard.
> 
> It turns out that with sc7280-herobrine-evoker, we've got a bit of a
> strange monkey on our hands. Due to some very interesting but
> (unfortunately) set-in-stone hardware design, we are doing 1.8V IO to
> the touchscreen but we _also_ have some extra buffers on the mainboard
> that need to be powered up to make the IO lines work. After much
> pondering about this, it seems like the best way to handle this is to
> add an optional "mainboard-vddio" rail to the bindings that is used to
> power up the buffers. Specifically, the fact that the touchscreen
> datasheet documents that its IOs can be at a different voltage level
> than its main power rail means that there truly are two voltage rails
> associated with the touchscreen, even if we don't actually provide the
> IO rail to it. Thus it doesn't feel absurd for the DT node on the host
> to have a 1.8V rail to power up anything related to its 1.8V logic.
> 
> Signed-off-by: Douglas Anderson <dianders@chromium.org>
> ---
> 
>  .../devicetree/bindings/input/goodix,gt7375p.yaml          | 7 +++++++
>  1 file changed, 7 insertions(+)
> 

Acked-by: Rob Herring <robh@kernel.org>


^ permalink raw reply

* Re: [PATCH v3 04/12] gpiolib: remove gpio_set_debounce
From: Dmitry Torokhov @ 2023-02-07 21:32 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Linus Walleij, Arnd Bergmann, Bartosz Golaszewski, Tony Lindgren,
	Thomas Gleixner, Krzysztof Kozlowski, Geert Uytterhoeven,
	Christophe Leroy, Michael Ellerman, Devarsh Thakkar,
	Stefan Schmidt, Miquel Raynal, linux-gpio, linux-doc,
	linux-kernel, linux-doc-tw-discuss, linux-arm-kernel, linux-omap,
	linux-samsung-soc, linux-m68k, linuxppc-dev, linux-sh, linux-acpi,
	linux-input, linux-media, linux-wpan, netdev, linux-wireless,
	brcm80211-dev-list.pdl, SHA-cyfmac-dev-list, linux-arch,
	devicetree, Bartosz Golaszewski, Jonathan Corbet, Alex Shi,
	Yanteng Si, Hu Haowen, Russell King, Aaro Koskinen,
	Janusz Krzysztofik, Andrew Lunn, Sebastian Hesselbarth,
	Gregory Clement, Alim Akhtar, Nicholas Piggin, Yoshinori Sato,
	Rich Felker, Mun Yew Tham, Keerthy, Mika Westerberg,
	Mauro Carvalho Chehab, Alexander Aring, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Arend van Spriel,
	Franky Lin, Hante Meuleman, Kalle Valo, Qiang Zhao, Li Yang,
	Lee Jones, Rob Herring, Frank Rowand
In-Reply-To: <20230207142952.51844-5-andriy.shevchenko@linux.intel.com>

On Tue, Feb 07, 2023 at 04:29:44PM +0200, Andy Shevchenko wrote:
> @@ -1010,14 +1009,21 @@ static int ads7846_setup_pendown(struct spi_device *spi,
>  		}
>  
>  		ts->gpio_pendown = pdata->gpio_pendown;
> -
> -		if (pdata->gpio_pendown_debounce)
> -			gpio_set_debounce(pdata->gpio_pendown,
> -					  pdata->gpio_pendown_debounce);

Can we please change only this to:

			gpiod_set_debounce(gpio_to_desc(pdata->gpio_pendown),
					   pdata->gpio_pendown_debounce);

and not change anything else (i.e. drop the changes below)?

>  	} else {
> -		dev_err(&spi->dev, "no get_pendown_state nor gpio_pendown?\n");
> -		return -EINVAL;
> +		struct gpio_desc *desc;
> +
> +		desc = devm_gpiod_get(&spi->dev, "pendown", GPIOD_IN);
> +		if (IS_ERR(desc)) {
> +			dev_err(&spi->dev, "no get_pendown_state nor gpio_pendown?\n");
> +			return PTR_ERR(desc);
> +		}
> +		gpiod_set_consumer_name(desc, "ads7846_pendown");
> +
> +		ts->gpio_pendown = desc_to_gpio(desc);
>  	}
> +	if (pdata->gpio_pendown_debounce)
> +		gpiod_set_debounce(gpio_to_desc(ts->gpio_pendown),
> +				   pdata->gpio_pendown_debounce);
>  
>  	return 0;

Thanks.

-- 
Dmitry

^ permalink raw reply

* Re: [PATCH] HID: quirks: Add quirk for Logitech G923 Xbox steering wheel
From: Walt Holman @ 2023-02-07 21:42 UTC (permalink / raw)
  To: Benjamin Tissoires; +Cc: linux-input, linux-kernel, Mattijs Korpershoek
In-Reply-To: <20230206095139.xj6ye7iuwnjo2bvj@mail.corp.redhat.com>

On 2/6/23 03:51, Benjamin Tissoires wrote:
> Hi Walt,
> 
> On Jan 27 2023, Walt Holman wrote:
>> On 1/27/23 16:03, Walt Holman wrote:
>>> Hello,
>>>
>>> This patch adds support for the Logitech G923 Xbox edition steering
>>> wheel.
>>>
>>> -Walt
>>>
>>> diff -uprN linux-master-source/drivers/hid/hid-ids.h linux-master-target/drivers/hid/hid-ids.h
>>> --- linux-master-source/drivers/hid/hid-ids.h    2023-01-27 15:18:14.000000000 -0600
>>> +++ linux-master-target/drivers/hid/hid-ids.h    2023-01-27 15:50:24.077639994 -0600
>>> @@ -819,6 +819,7 @@
>>>    #define USB_DEVICE_ID_LOGITECH_G510_USB_AUDIO    0xc22e
>>>    #define USB_DEVICE_ID_LOGITECH_G29_WHEEL    0xc24f
>>>    #define USB_DEVICE_ID_LOGITECH_G920_WHEEL    0xc262
>>> +#define USB_DEVICE_ID_LOGITECH_G923_XBOX_WHEEL    0xc26e
>>>    #define USB_DEVICE_ID_LOGITECH_WINGMAN_F3D    0xc283
>>>    #define USB_DEVICE_ID_LOGITECH_FORCE3D_PRO    0xc286
>>>    #define USB_DEVICE_ID_LOGITECH_FLIGHT_SYSTEM_G940    0xc287
>>> diff -uprN linux-master-source/drivers/hid/hid-logitech-hidpp.c linux-master-target/drivers/hid/hid-logitech-hidpp.c
>>> --- linux-master-source/drivers/hid/hid-logitech-hidpp.c    2023-01-27 15:18:14.000000000 -0600
>>> +++ linux-master-target/drivers/hid/hid-logitech-hidpp.c    2023-01-27 15:50:24.077639994 -0600
>>> @@ -4347,6 +4347,9 @@ static const struct hid_device_id hidpp_
>>>        { /* Logitech G920 Wheel over USB */
>>>          HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_LOGITECH_G920_WHEEL),
>>>            .driver_data = HIDPP_QUIRK_CLASS_G920 | HIDPP_QUIRK_FORCE_OUTPUT_REPORTS},
>>> +    { /* Logitech G923 Wheel (Xbox version) over USB */
>>> +      HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_LOGITECH_G923_XBOX_WHEEL),
>>> +        .driver_data = HIDPP_QUIRK_CLASS_G920 | HIDPP_QUIRK_FORCE_OUTPUT_REPORTS },
>>>        { /* Logitech G Pro Gaming Mouse over USB */
>>>          HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, 0xC088) },
>>>
>> Signed off by: Walt Holman (waltholman09@gmail.com)
>>
> 
> Thanks a lot for your submission. I wanted to quickly include it in the
> kernel, but it seems that the patch is not following the standards:
> - "git am" on your patch refuses to take it, saying it is corrupted
>    (please use "git format-patch" to generate the proper patch format)
> - your patch doesn't have a proper commit description:
>    please add a description like "adds support for the Logitech G923 Xbox
>    edition steering wheel in hid-logitech-hidpp. We get the same level
>    of features than the regular G920". Or anything else as long as we get
>    a sensible commit description.
> 
> As a general rule of thumb, please follow the doc at [1] to submit your
> patches.
> 
> Cheers,
> Benjamin
> 
> [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst
> 
Hello Benjamin. Sorry about the malformed patch. I used Thunderbird and evidently it mangled it. I've since setup a proper git environment and re-sent the patch just recently from git send-email. Hopefully that is clean and ready to apply. Thanks!

-Walt


^ permalink raw reply

* Re: [PATCH] HID: retain initial quirks set up when creating HID devices
From: Allen Ballway @ 2023-02-07 22:11 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Jiri Kosina, Benjamin Tissoires, Hans de Goede, Alistair Francis,
	Guenter Roeck, linux-input, linux-kernel
In-Reply-To: <Y+K/i4kj0WER1Hl0@google.com>

On Tue, Feb 7, 2023 at 1:16 PM Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
>
> In certain circumstances, such as when creating I2C-connected HID
> devices, we want to pass and retain some querks (axis inversion, etc).
> The source of such quirks may be device tree, or DMI data, or something
> else not readily available to the HID core itself and therefore cannot
> be reconstructed easily. To allow this introduce "initial_quirks" field
> in hid_device structure and use it when determining the final set of
> quirks.
>
> This fixes problem with i2c-hid setting up device-tree sourced quirks
> too late and losing them on device rebind, and also allows to sever the
> tie between hid-code and i2c-hid when applying DMI-based quirks.
>
> Fixes: b60d3c803d76 ("HID: i2c-hid-of: Expose the touchscreen-inverted properties")
> Fixes: a2f416bf062a ("HID: multitouch: Add quirks for flipped axes")
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> ---
>
> Compiled only, not tested on hardware.
>

Tested on Dynabook K50 with Elan9034 touchscreen and confirmed quirks are
applied as expected.

>  drivers/hid/hid-quirks.c                 | 8 +-------
>  drivers/hid/i2c-hid/i2c-hid-core.c       | 6 ++++--
>  drivers/hid/i2c-hid/i2c-hid-dmi-quirks.c | 1 -
>  include/linux/hid.h                      | 1 +
>  4 files changed, 6 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/hid/hid-quirks.c b/drivers/hid/hid-quirks.c
> index 30e35f79def4..66e64350f138 100644
> --- a/drivers/hid/hid-quirks.c
> +++ b/drivers/hid/hid-quirks.c
> @@ -19,7 +19,6 @@
>  #include <linux/input/elan-i2c-ids.h>
>
>  #include "hid-ids.h"
> -#include "i2c-hid/i2c-hid.h"
>
>  /*
>   * Alphabetically sorted by vendor then product.
> @@ -1238,7 +1237,7 @@ EXPORT_SYMBOL_GPL(hid_quirks_exit);
>  static unsigned long hid_gets_squirk(const struct hid_device *hdev)
>  {
>         const struct hid_device_id *bl_entry;
> -       unsigned long quirks = 0;
> +       unsigned long quirks = hdev->initial_quirks;
>
>         if (hid_match_id(hdev, hid_ignore_list))
>                 quirks |= HID_QUIRK_IGNORE;
> @@ -1299,11 +1298,6 @@ unsigned long hid_lookup_quirk(const struct hid_device *hdev)
>                 quirks = hid_gets_squirk(hdev);
>         mutex_unlock(&dquirks_lock);
>
> -       /* Get quirks specific to I2C devices */
> -       if (IS_ENABLED(CONFIG_I2C_DMI_CORE) && IS_ENABLED(CONFIG_DMI) &&
> -           hdev->bus == BUS_I2C)
> -               quirks |= i2c_hid_get_dmi_quirks(hdev->vendor, hdev->product);
> -
>         return quirks;
>  }
>  EXPORT_SYMBOL_GPL(hid_lookup_quirk);
> diff --git a/drivers/hid/i2c-hid/i2c-hid-core.c b/drivers/hid/i2c-hid/i2c-hid-core.c
> index 0ab8f47a84e9..efbba0465eef 100644
> --- a/drivers/hid/i2c-hid/i2c-hid-core.c
> +++ b/drivers/hid/i2c-hid/i2c-hid-core.c
> @@ -1025,6 +1025,10 @@ int i2c_hid_core_probe(struct i2c_client *client, struct i2chid_ops *ops,
>         hid->vendor = le16_to_cpu(ihid->hdesc.wVendorID);
>         hid->product = le16_to_cpu(ihid->hdesc.wProductID);
>
> +       hid->initial_quirks = quirks;
> +       hid->initial_quirks |= i2c_hid_get_dmi_quirks(hid->vendor,
> +                                                     hid->product);
> +
>         snprintf(hid->name, sizeof(hid->name), "%s %04X:%04X",
>                  client->name, (u16)hid->vendor, (u16)hid->product);
>         strscpy(hid->phys, dev_name(&client->dev), sizeof(hid->phys));
> @@ -1038,8 +1042,6 @@ int i2c_hid_core_probe(struct i2c_client *client, struct i2chid_ops *ops,
>                 goto err_mem_free;
>         }
>
> -       hid->quirks |= quirks;
> -
>         return 0;
>
>  err_mem_free:
> diff --git a/drivers/hid/i2c-hid/i2c-hid-dmi-quirks.c b/drivers/hid/i2c-hid/i2c-hid-dmi-quirks.c
> index 554a7dc28536..210f17c3a0be 100644
> --- a/drivers/hid/i2c-hid/i2c-hid-dmi-quirks.c
> +++ b/drivers/hid/i2c-hid/i2c-hid-dmi-quirks.c
> @@ -492,4 +492,3 @@ u32 i2c_hid_get_dmi_quirks(const u16 vendor, const u16 product)
>
>         return quirks;
>  }
> -EXPORT_SYMBOL_GPL(i2c_hid_get_dmi_quirks);
> diff --git a/include/linux/hid.h b/include/linux/hid.h
> index daaac4d7f370..56dac09c99d9 100644
> --- a/include/linux/hid.h
> +++ b/include/linux/hid.h
> @@ -622,6 +622,7 @@ struct hid_device {                                                 /* device report descriptor */
>         unsigned long status;                                           /* see STAT flags above */
>         unsigned claimed;                                               /* Claimed by hidinput, hiddev? */
>         unsigned quirks;                                                /* Various quirks the device can pull on us */
> +       unsigned initial_quirks;                                        /* Initial set of quirks supplied when creating device */
>         bool io_started;                                                /* If IO has started */
>
>         struct list_head inputs;                                        /* The list of inputs */
> --
> 2.39.1.519.gcb327c4b5f-goog
>
>
> --
> Dmitry

^ permalink raw reply

* Re: [PATCH] HID: retain initial quirks set up when creating HID devices
From: Guenter Roeck @ 2023-02-07 22:19 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Jiri Kosina, Benjamin Tissoires, Allen Ballway, Hans de Goede,
	Alistair Francis, Guenter Roeck, linux-input, linux-kernel
In-Reply-To: <Y+K/i4kj0WER1Hl0@google.com>

On Tue, Feb 7, 2023 at 1:16 PM Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
>
> In certain circumstances, such as when creating I2C-connected HID
> devices, we want to pass and retain some querks (axis inversion, etc).

quirks

> The source of such quirks may be device tree, or DMI data, or something
> else not readily available to the HID core itself and therefore cannot
> be reconstructed easily. To allow this introduce "initial_quirks" field

this, introduce (says gmail)

> in hid_device structure and use it when determining the final set of
> quirks.
>
> This fixes problem with i2c-hid setting up device-tree sourced quirks

fixes the problem (again, says gmail)

> too late and losing them on device rebind, and also allows to sever the
> tie between hid-code and i2c-hid when applying DMI-based quirks.
>
> Fixes: b60d3c803d76 ("HID: i2c-hid-of: Expose the touchscreen-inverted properties")
> Fixes: a2f416bf062a ("HID: multitouch: Add quirks for flipped axes")
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>

Reviewed-by: Guenter Roeck <groeck@chromium.org>

> ---
>
> Compiled only, not tested on hardware.
>
>  drivers/hid/hid-quirks.c                 | 8 +-------
>  drivers/hid/i2c-hid/i2c-hid-core.c       | 6 ++++--
>  drivers/hid/i2c-hid/i2c-hid-dmi-quirks.c | 1 -
>  include/linux/hid.h                      | 1 +
>  4 files changed, 6 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/hid/hid-quirks.c b/drivers/hid/hid-quirks.c
> index 30e35f79def4..66e64350f138 100644
> --- a/drivers/hid/hid-quirks.c
> +++ b/drivers/hid/hid-quirks.c
> @@ -19,7 +19,6 @@
>  #include <linux/input/elan-i2c-ids.h>
>
>  #include "hid-ids.h"
> -#include "i2c-hid/i2c-hid.h"
>
>  /*
>   * Alphabetically sorted by vendor then product.
> @@ -1238,7 +1237,7 @@ EXPORT_SYMBOL_GPL(hid_quirks_exit);
>  static unsigned long hid_gets_squirk(const struct hid_device *hdev)
>  {
>         const struct hid_device_id *bl_entry;
> -       unsigned long quirks = 0;
> +       unsigned long quirks = hdev->initial_quirks;
>
>         if (hid_match_id(hdev, hid_ignore_list))
>                 quirks |= HID_QUIRK_IGNORE;
> @@ -1299,11 +1298,6 @@ unsigned long hid_lookup_quirk(const struct hid_device *hdev)
>                 quirks = hid_gets_squirk(hdev);
>         mutex_unlock(&dquirks_lock);
>
> -       /* Get quirks specific to I2C devices */
> -       if (IS_ENABLED(CONFIG_I2C_DMI_CORE) && IS_ENABLED(CONFIG_DMI) &&
> -           hdev->bus == BUS_I2C)
> -               quirks |= i2c_hid_get_dmi_quirks(hdev->vendor, hdev->product);
> -
>         return quirks;
>  }
>  EXPORT_SYMBOL_GPL(hid_lookup_quirk);
> diff --git a/drivers/hid/i2c-hid/i2c-hid-core.c b/drivers/hid/i2c-hid/i2c-hid-core.c
> index 0ab8f47a84e9..efbba0465eef 100644
> --- a/drivers/hid/i2c-hid/i2c-hid-core.c
> +++ b/drivers/hid/i2c-hid/i2c-hid-core.c
> @@ -1025,6 +1025,10 @@ int i2c_hid_core_probe(struct i2c_client *client, struct i2chid_ops *ops,
>         hid->vendor = le16_to_cpu(ihid->hdesc.wVendorID);
>         hid->product = le16_to_cpu(ihid->hdesc.wProductID);
>
> +       hid->initial_quirks = quirks;
> +       hid->initial_quirks |= i2c_hid_get_dmi_quirks(hid->vendor,
> +                                                     hid->product);
> +
>         snprintf(hid->name, sizeof(hid->name), "%s %04X:%04X",
>                  client->name, (u16)hid->vendor, (u16)hid->product);
>         strscpy(hid->phys, dev_name(&client->dev), sizeof(hid->phys));
> @@ -1038,8 +1042,6 @@ int i2c_hid_core_probe(struct i2c_client *client, struct i2chid_ops *ops,
>                 goto err_mem_free;
>         }
>
> -       hid->quirks |= quirks;
> -
>         return 0;
>
>  err_mem_free:
> diff --git a/drivers/hid/i2c-hid/i2c-hid-dmi-quirks.c b/drivers/hid/i2c-hid/i2c-hid-dmi-quirks.c
> index 554a7dc28536..210f17c3a0be 100644
> --- a/drivers/hid/i2c-hid/i2c-hid-dmi-quirks.c
> +++ b/drivers/hid/i2c-hid/i2c-hid-dmi-quirks.c
> @@ -492,4 +492,3 @@ u32 i2c_hid_get_dmi_quirks(const u16 vendor, const u16 product)
>
>         return quirks;
>  }
> -EXPORT_SYMBOL_GPL(i2c_hid_get_dmi_quirks);
> diff --git a/include/linux/hid.h b/include/linux/hid.h
> index daaac4d7f370..56dac09c99d9 100644
> --- a/include/linux/hid.h
> +++ b/include/linux/hid.h
> @@ -622,6 +622,7 @@ struct hid_device {                                                 /* device report descriptor */
>         unsigned long status;                                           /* see STAT flags above */
>         unsigned claimed;                                               /* Claimed by hidinput, hiddev? */
>         unsigned quirks;                                                /* Various quirks the device can pull on us */
> +       unsigned initial_quirks;                                        /* Initial set of quirks supplied when creating device */
>         bool io_started;                                                /* If IO has started */
>
>         struct list_head inputs;                                        /* The list of inputs */
> --
> 2.39.1.519.gcb327c4b5f-goog
>
>
> --
> Dmitry

^ permalink raw reply

* Re: [PATCH v3 06/12] gpiolib: split linux/gpio/driver.h out of linux/gpio.h
From: Andy Shevchenko @ 2023-02-07 22:55 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Arnd Bergmann, Bartosz Golaszewski, Geert Uytterhoeven,
	Christophe Leroy, Michael Ellerman, Dmitry Torokhov,
	Stefan Schmidt, Miquel Raynal, linux-gpio, linux-doc,
	linux-kernel, linux-doc-tw-discuss, linux-arm-kernel, linux-omap,
	linux-samsung-soc, linux-m68k, linuxppc-dev, linux-sh, linux-acpi,
	linux-input, linux-media, linux-wpan, netdev, linux-wireless,
	brcm80211-dev-list.pdl, SHA-cyfmac-dev-list, linux-arch,
	devicetree, Bartosz Golaszewski
In-Reply-To: <CACRpkdaPgjDijPjCdinWy5_Rd8g3idv-8K=YPTv5iTfJKFuJfw@mail.gmail.com>

On Tue, Feb 07, 2023 at 03:55:23PM +0100, Linus Walleij wrote:
> On Tue, Feb 7, 2023 at 3:29 PM Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> 
> > From: Arnd Bergmann <arnd@arndb.de>
> >
> > Almost all gpio drivers include linux/gpio/driver.h, and other
> > files should not rely on includes from this header.
> >
> > Remove the indirect include from here and include the correct
> > headers directly from where they are used.

...

> Make sure you push this to the kernel.org build servers (zeroday builds),

Of course, that is the purpose of publishing this before the release (so we
will have some TODO list that eventually this can be applied for v6.4-rc1).

> I think this patch needs to hit some more files, in my tests with a similar
> patch at least these:

Right. I forgot to also incorporate your stuff into this series.
Do you have anything that I can take as is?

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply

* Re: [PATCH v3 04/12] gpiolib: remove gpio_set_debounce
From: Andy Shevchenko @ 2023-02-07 22:56 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Linus Walleij, Arnd Bergmann, Bartosz Golaszewski,
	Geert Uytterhoeven, Christophe Leroy, Michael Ellerman,
	Devarsh Thakkar, Stefan Schmidt, Miquel Raynal, linux-gpio,
	linux-doc, linux-kernel, linux-doc-tw-discuss, linux-arm-kernel,
	linux-omap, linux-samsung-soc, linux-m68k, linuxppc-dev, linux-sh,
	linux-acpi, linux-input, linux-media, linux-wpan, netdev,
	linux-wireless, brcm80211-dev-list.pdl, SHA-cyfmac-dev-list,
	linux-arch, devicetree, Bartosz Golaszewski
In-Reply-To: <Y+LDUTfKgHEJHNXB@google.com>

On Tue, Feb 07, 2023 at 01:32:01PM -0800, Dmitry Torokhov wrote:
> On Tue, Feb 07, 2023 at 04:29:44PM +0200, Andy Shevchenko wrote:
> > @@ -1010,14 +1009,21 @@ static int ads7846_setup_pendown(struct spi_device *spi,
> >  		}
> >  
> >  		ts->gpio_pendown = pdata->gpio_pendown;
> > -
> > -		if (pdata->gpio_pendown_debounce)
> > -			gpio_set_debounce(pdata->gpio_pendown,
> > -					  pdata->gpio_pendown_debounce);
> 
> Can we please change only this to:
> 
> 			gpiod_set_debounce(gpio_to_desc(pdata->gpio_pendown),
> 					   pdata->gpio_pendown_debounce);
> 
> and not change anything else (i.e. drop the changes below)?

Probably. I can try rollback this.

> >  	} else {
> > -		dev_err(&spi->dev, "no get_pendown_state nor gpio_pendown?\n");
> > -		return -EINVAL;
> > +		struct gpio_desc *desc;
> > +
> > +		desc = devm_gpiod_get(&spi->dev, "pendown", GPIOD_IN);
> > +		if (IS_ERR(desc)) {
> > +			dev_err(&spi->dev, "no get_pendown_state nor gpio_pendown?\n");
> > +			return PTR_ERR(desc);
> > +		}
> > +		gpiod_set_consumer_name(desc, "ads7846_pendown");
> > +
> > +		ts->gpio_pendown = desc_to_gpio(desc);
> >  	}
> > +	if (pdata->gpio_pendown_debounce)
> > +		gpiod_set_debounce(gpio_to_desc(ts->gpio_pendown),
> > +				   pdata->gpio_pendown_debounce);
> >  
> >  	return 0;

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply

* [PATCH v2] HID: retain initial quirks set up when creating HID devices
From: Dmitry Torokhov @ 2023-02-07 23:03 UTC (permalink / raw)
  To: Jiri Kosina, Benjamin Tissoires
  Cc: Guenter Roeck, Allen Ballway, Hans de Goede, Alistair Francis,
	linux-input, linux-kernel

In certain circumstances, such as when creating I2C-connected HID
devices, we want to pass and retain some quirks (axis inversion, etc).
The source of such quirks may be device tree, or DMI data, or something
else not readily available to the HID core itself and therefore cannot
be reconstructed easily. To allow this, introduce "initial_quirks" field
in hid_device structure and use it when determining the final set of
quirks.

This fixes the problem with i2c-hid setting up device-tree sourced
quirks too late and losing them on device rebind, and also allows to
sever the tie between hid-code and i2c-hid when applying DMI-based
quirks.

Fixes: b60d3c803d76 ("HID: i2c-hid-of: Expose the touchscreen-inverted properties")
Fixes: a2f416bf062a ("HID: multitouch: Add quirks for flipped axes")
Reviewed-by: Guenter Roeck <groeck@chromium.org>
Tested-by: Allen Ballway <ballway@chromium.org>
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---

v2:
- corrected spelling/grammar in the commit message per Guenter
- added reviewed-by/tested-by tags


 drivers/hid/hid-quirks.c                 | 8 +-------
 drivers/hid/i2c-hid/i2c-hid-core.c       | 6 ++++--
 drivers/hid/i2c-hid/i2c-hid-dmi-quirks.c | 1 -
 include/linux/hid.h                      | 1 +
 4 files changed, 6 insertions(+), 10 deletions(-)

diff --git a/drivers/hid/hid-quirks.c b/drivers/hid/hid-quirks.c
index 30e35f79def4..66e64350f138 100644
--- a/drivers/hid/hid-quirks.c
+++ b/drivers/hid/hid-quirks.c
@@ -19,7 +19,6 @@
 #include <linux/input/elan-i2c-ids.h>
 
 #include "hid-ids.h"
-#include "i2c-hid/i2c-hid.h"
 
 /*
  * Alphabetically sorted by vendor then product.
@@ -1238,7 +1237,7 @@ EXPORT_SYMBOL_GPL(hid_quirks_exit);
 static unsigned long hid_gets_squirk(const struct hid_device *hdev)
 {
 	const struct hid_device_id *bl_entry;
-	unsigned long quirks = 0;
+	unsigned long quirks = hdev->initial_quirks;
 
 	if (hid_match_id(hdev, hid_ignore_list))
 		quirks |= HID_QUIRK_IGNORE;
@@ -1299,11 +1298,6 @@ unsigned long hid_lookup_quirk(const struct hid_device *hdev)
 		quirks = hid_gets_squirk(hdev);
 	mutex_unlock(&dquirks_lock);
 
-	/* Get quirks specific to I2C devices */
-	if (IS_ENABLED(CONFIG_I2C_DMI_CORE) && IS_ENABLED(CONFIG_DMI) &&
-	    hdev->bus == BUS_I2C)
-		quirks |= i2c_hid_get_dmi_quirks(hdev->vendor, hdev->product);
-
 	return quirks;
 }
 EXPORT_SYMBOL_GPL(hid_lookup_quirk);
diff --git a/drivers/hid/i2c-hid/i2c-hid-core.c b/drivers/hid/i2c-hid/i2c-hid-core.c
index 0ab8f47a84e9..efbba0465eef 100644
--- a/drivers/hid/i2c-hid/i2c-hid-core.c
+++ b/drivers/hid/i2c-hid/i2c-hid-core.c
@@ -1025,6 +1025,10 @@ int i2c_hid_core_probe(struct i2c_client *client, struct i2chid_ops *ops,
 	hid->vendor = le16_to_cpu(ihid->hdesc.wVendorID);
 	hid->product = le16_to_cpu(ihid->hdesc.wProductID);
 
+	hid->initial_quirks = quirks;
+	hid->initial_quirks |= i2c_hid_get_dmi_quirks(hid->vendor,
+						      hid->product);
+
 	snprintf(hid->name, sizeof(hid->name), "%s %04X:%04X",
 		 client->name, (u16)hid->vendor, (u16)hid->product);
 	strscpy(hid->phys, dev_name(&client->dev), sizeof(hid->phys));
@@ -1038,8 +1042,6 @@ int i2c_hid_core_probe(struct i2c_client *client, struct i2chid_ops *ops,
 		goto err_mem_free;
 	}
 
-	hid->quirks |= quirks;
-
 	return 0;
 
 err_mem_free:
diff --git a/drivers/hid/i2c-hid/i2c-hid-dmi-quirks.c b/drivers/hid/i2c-hid/i2c-hid-dmi-quirks.c
index 554a7dc28536..210f17c3a0be 100644
--- a/drivers/hid/i2c-hid/i2c-hid-dmi-quirks.c
+++ b/drivers/hid/i2c-hid/i2c-hid-dmi-quirks.c
@@ -492,4 +492,3 @@ u32 i2c_hid_get_dmi_quirks(const u16 vendor, const u16 product)
 
 	return quirks;
 }
-EXPORT_SYMBOL_GPL(i2c_hid_get_dmi_quirks);
diff --git a/include/linux/hid.h b/include/linux/hid.h
index daaac4d7f370..56dac09c99d9 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -622,6 +622,7 @@ struct hid_device {							/* device report descriptor */
 	unsigned long status;						/* see STAT flags above */
 	unsigned claimed;						/* Claimed by hidinput, hiddev? */
 	unsigned quirks;						/* Various quirks the device can pull on us */
+	unsigned initial_quirks;					/* Initial set of quirks supplied when creating device */
 	bool io_started;						/* If IO has started */
 
 	struct list_head inputs;					/* The list of inputs */
-- 
2.39.1.519.gcb327c4b5f-goog


-- 
Dmitry

^ permalink raw reply related

* [PATCH -next] HID: core: clean up some inconsistent indentings
From: Yang Li @ 2023-02-08  1:19 UTC (permalink / raw)
  To: jikos; +Cc: benjamin.tissoires, linux-input, linux-kernel, Yang Li,
	Abaci Robot

./drivers/hid/hid-core.c:2727:2-17: code aligned with following code on line 2732

Reported-by: Abaci Robot <abaci@linux.alibaba.com>
Link: https://bugzilla.openanolis.cn/show_bug.cgi?id=3980
Signed-off-by: Yang Li <yang.lee@linux.alibaba.com>
---
 drivers/hid/hid-core.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index 842afc88a949..f080f132b36c 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -2729,10 +2729,10 @@ int hid_add_device(struct hid_device *hdev)
 	/*
 	 * Check for the mandatory transport channel.
 	 */
-	 if (!hdev->ll_driver->raw_request) {
+	if (!hdev->ll_driver->raw_request) {
 		hid_err(hdev, "transport driver missing .raw_request()\n");
 		return -EINVAL;
-	 }
+	}
 
 	/*
 	 * Read the device report descriptor once and use as template
-- 
2.20.1.7.g153144c


^ permalink raw reply related

* Re: remove arch/sh
From: Randy Dunlap @ 2023-02-08  1:31 UTC (permalink / raw)
  To: John Paul Adrian Glaubitz, Christoph Hellwig
  Cc: Yoshinori Sato, Rich Felker, Arnd Bergmann, Greg Kroah-Hartman,
	Laurent Pinchart, Kieran Bingham, Geert Uytterhoeven,
	linux-kernel, linux-watchdog, devicetree, linux-arch, dmaengine,
	dri-devel, linux-renesas-soc, linux-i2c, linux-input, linux-media,
	linux-mmc, linux-mtd, netdev, linux-gpio, linux-rtc, linux-spi,
	linux-serial, linux-usb, linux-fbdev, alsa-devel, linux-sh
In-Reply-To: <60ed320c8f5286e8dbbf71be29b760339fd25069.camel@physik.fu-berlin.de>



On 2/7/23 01:06, John Paul Adrian Glaubitz wrote:
> Hello Christoph!
> 
> On Fri, 2023-02-03 at 08:14 +0100, Christoph Hellwig wrote:
>> On Mon, Jan 16, 2023 at 09:52:10AM +0100, John Paul Adrian Glaubitz wrote:
>>> We have had a discussion between multiple people invested in the SuperH port and
>>> I have decided to volunteer as a co-maintainer of the port to support Rich Felker
>>> when he isn't available.
>>
>> So, this still isn't reflected in MAINTAINERS in linux-next.  When
>> do you plan to take over?  What platforms will remain supported and
>> what can we start dropping due to being unused and unmaintained?
> 
> I'm getting everything ready now with Geert's help and I have a probably dumb
> question regarding the MAINTAINERS file change: Shall I just add myself as an
> additional maintainer first or shall I also drop Yoshinori Sato?
> 
> Also, is it desirable to add a "T:" entry for the kernel tree?

Yes, definitely.

thanks.
-- 
~Randy

^ permalink raw reply

* Re: [PATCH v2] HID: retain initial quirks set up when creating HID devices
From: Alistair @ 2023-02-08  8:00 UTC (permalink / raw)
  To: Dmitry Torokhov, Jiri Kosina, Benjamin Tissoires
  Cc: Guenter Roeck, Allen Ballway, Hans de Goede, linux-input,
	Linux Kernel Mailing List
In-Reply-To: <Y+LYwu3Zs13hdVDy@google.com>

On Wed, 8 Feb 2023, at 9:03 AM, Dmitry Torokhov wrote:
> In certain circumstances, such as when creating I2C-connected HID
> devices, we want to pass and retain some quirks (axis inversion, etc).
> The source of such quirks may be device tree, or DMI data, or something
> else not readily available to the HID core itself and therefore cannot
> be reconstructed easily. To allow this, introduce "initial_quirks" field
> in hid_device structure and use it when determining the final set of
> quirks.
> 
> This fixes the problem with i2c-hid setting up device-tree sourced
> quirks too late and losing them on device rebind, and also allows to
> sever the tie between hid-code and i2c-hid when applying DMI-based
> quirks.
> 
> Fixes: b60d3c803d76 ("HID: i2c-hid-of: Expose the touchscreen-inverted properties")
> Fixes: a2f416bf062a ("HID: multitouch: Add quirks for flipped axes")
> Reviewed-by: Guenter Roeck <groeck@chromium.org>
> Tested-by: Allen Ballway <ballway@chromium.org>
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>

Reviewed-by: Alistair Francis <alistair@alistair23.me>

Alistair

> ---
> 
> v2:
> - corrected spelling/grammar in the commit message per Guenter
> - added reviewed-by/tested-by tags
> 
> 
> drivers/hid/hid-quirks.c                 | 8 +-------
> drivers/hid/i2c-hid/i2c-hid-core.c       | 6 ++++--
> drivers/hid/i2c-hid/i2c-hid-dmi-quirks.c | 1 -
> include/linux/hid.h                      | 1 +
> 4 files changed, 6 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/hid/hid-quirks.c b/drivers/hid/hid-quirks.c
> index 30e35f79def4..66e64350f138 100644
> --- a/drivers/hid/hid-quirks.c
> +++ b/drivers/hid/hid-quirks.c
> @@ -19,7 +19,6 @@
> #include <linux/input/elan-i2c-ids.h>
>  
> #include "hid-ids.h"
> -#include "i2c-hid/i2c-hid.h"
>  
> /*
>   * Alphabetically sorted by vendor then product.
> @@ -1238,7 +1237,7 @@ EXPORT_SYMBOL_GPL(hid_quirks_exit);
> static unsigned long hid_gets_squirk(const struct hid_device *hdev)
> {
> const struct hid_device_id *bl_entry;
> - unsigned long quirks = 0;
> + unsigned long quirks = hdev->initial_quirks;
>  
> if (hid_match_id(hdev, hid_ignore_list))
> quirks |= HID_QUIRK_IGNORE;
> @@ -1299,11 +1298,6 @@ unsigned long hid_lookup_quirk(const struct hid_device *hdev)
> quirks = hid_gets_squirk(hdev);
> mutex_unlock(&dquirks_lock);
>  
> - /* Get quirks specific to I2C devices */
> - if (IS_ENABLED(CONFIG_I2C_DMI_CORE) && IS_ENABLED(CONFIG_DMI) &&
> -     hdev->bus == BUS_I2C)
> - quirks |= i2c_hid_get_dmi_quirks(hdev->vendor, hdev->product);
> -
> return quirks;
> }
> EXPORT_SYMBOL_GPL(hid_lookup_quirk);
> diff --git a/drivers/hid/i2c-hid/i2c-hid-core.c b/drivers/hid/i2c-hid/i2c-hid-core.c
> index 0ab8f47a84e9..efbba0465eef 100644
> --- a/drivers/hid/i2c-hid/i2c-hid-core.c
> +++ b/drivers/hid/i2c-hid/i2c-hid-core.c
> @@ -1025,6 +1025,10 @@ int i2c_hid_core_probe(struct i2c_client *client, struct i2chid_ops *ops,
> hid->vendor = le16_to_cpu(ihid->hdesc.wVendorID);
> hid->product = le16_to_cpu(ihid->hdesc.wProductID);
>  
> + hid->initial_quirks = quirks;
> + hid->initial_quirks |= i2c_hid_get_dmi_quirks(hid->vendor,
> +       hid->product);
> +
> snprintf(hid->name, sizeof(hid->name), "%s %04X:%04X",
> client->name, (u16)hid->vendor, (u16)hid->product);
> strscpy(hid->phys, dev_name(&client->dev), sizeof(hid->phys));
> @@ -1038,8 +1042,6 @@ int i2c_hid_core_probe(struct i2c_client *client, struct i2chid_ops *ops,
> goto err_mem_free;
> }
>  
> - hid->quirks |= quirks;
> -
> return 0;
>  
> err_mem_free:
> diff --git a/drivers/hid/i2c-hid/i2c-hid-dmi-quirks.c b/drivers/hid/i2c-hid/i2c-hid-dmi-quirks.c
> index 554a7dc28536..210f17c3a0be 100644
> --- a/drivers/hid/i2c-hid/i2c-hid-dmi-quirks.c
> +++ b/drivers/hid/i2c-hid/i2c-hid-dmi-quirks.c
> @@ -492,4 +492,3 @@ u32 i2c_hid_get_dmi_quirks(const u16 vendor, const u16 product)
>  
> return quirks;
> }
> -EXPORT_SYMBOL_GPL(i2c_hid_get_dmi_quirks);
> diff --git a/include/linux/hid.h b/include/linux/hid.h
> index daaac4d7f370..56dac09c99d9 100644
> --- a/include/linux/hid.h
> +++ b/include/linux/hid.h
> @@ -622,6 +622,7 @@ struct hid_device { /* device report descriptor */
> unsigned long status; /* see STAT flags above */
> unsigned claimed; /* Claimed by hidinput, hiddev? */
> unsigned quirks; /* Various quirks the device can pull on us */
> + unsigned initial_quirks; /* Initial set of quirks supplied when creating device */
> bool io_started; /* If IO has started */
>  
> struct list_head inputs; /* The list of inputs */
> -- 
> 2.39.1.519.gcb327c4b5f-goog
> 
> 
> -- 
> Dmitry
> 

^ 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