HID: cp2112: Cleanups and refactorings

PATCH In review Andy Shevchenko 6 participants 30 messages Jul 3, 2023 – Aug 28, 2023

Subsystems (2)

HID Generic string library

lore

Share & tools

lore

Andy Shevchenko

author

2023-07-03 18:52 UTC #0 lore raw

cover HID: cp2112: Cleanups and refactorings

After I updated GPIO library for the case Benjamin has with CP2112, I have a brief look into the CP2112 driver itself.

From GPIO perspective it has two main (maitenance) issues:
- usage of ->to_irq() with IRQ chip present;
- having IRQ chip not immutable.

Besides that there are plenty small cleanups here and there. Hence this series.

Compile tested only.

Andy Shevchenko (12):
lib/string_choices: Add str_write_read() helper
HID: cp2112: Use str_write_read() and str_read_write()
HID: cp2112: Make irq_chip immutable
HID: cp2112: Switch to for_each_set_bit() to simplify the code
HID: cp2112: Don't call ->to_irq() explicitly
HID: cp2112: Remove dead code
HID: cp2112: Define maximum GPIO constant and use it
HID: cp2112: Define all GPIO mask and use it
HID: cp2112: Use BIT() in GPIO setter and getter
HID: cp2112: Use sysfs_emit() to instead of scnprintf()
HID: cp2112: Convert to DEVICE_ATTR_RW()
HID: cp2112: Use octal permissions

 drivers/hid/hid-cp2112.c       | 169 +++++++++++----------------------
 include/linux/string_choices.h |   1 +
 2 files changed, 58 insertions(+), 112 deletions(-)

Signature · 1 line

2.40.0.1.gaa8946217a0b

Andy Shevchenko

author

2023-07-03 18:52 UTC #1 lore raw

PATCH 1/12 lib/string_choices: Add str_write_read() helper

Add an inversed variant of str_read_write(), i.e. str_write_read().

Signed-off-by Andy Shevchenko <[email protected]>

---
 include/linux/string_choices.h | 1 +
 1 file changed, 1 insertion(+)

include/linux/string_choices.h +1 -0

index 48120222b9b2..3c1091941eb8 100644
@@ -30,6 +30,7 @@ static inline const char *str_read_write(bool v)
 {
 	return v ? "read" : "write";
 }
+#define str_write_read(v)		str_read_write(!(v))
 
 static inline const char *str_on_off(bool v)
 {

Signature · 1 line

2.40.0.1.gaa8946217a0b

Andy Shevchenko

author

2023-07-03 18:52 UTC #2 lore raw

PATCH 2/12 HID: cp2112: Use str_write_read() and str_read_write()

In reply to Andy Shevchenko

Use str_write_read() and str_read_write() from string_choices.h.

Signed-off-by Andy Shevchenko <[email protected]>

---
 drivers/hid/hid-cp2112.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

drivers/hid/hid-cp2112.c +4 -5

index 27cadadda7c9..37ccf4714ad1 100644
@@ -24,6 +24,7 @@
 #include <linux/i2c.h>
 #include <linux/module.h>
 #include <linux/nls.h>
+#include <linux/string_choices.h>
 #include <linux/usb/ch9.h>
 #include "hid-ids.h"

@@ -532,15 +533,13 @@ static int cp2112_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs,
 	hid_dbg(hdev, "I2C %d messages\n", num);
 
 	if (num == 1) {
+		hid_dbg(hdev, "I2C %s %#04x len %d\n",
+			str_read_write(msgs->flags & I2C_M_RD), msgs->addr, msgs->len);
 		if (msgs->flags & I2C_M_RD) {
-			hid_dbg(hdev, "I2C read %#04x len %d\n",
-				msgs->addr, msgs->len);
 			read_length = msgs->len;
 			read_buf = msgs->buf;
 			count = cp2112_read_req(buf, msgs->addr, msgs->len);
 		} else {
-			hid_dbg(hdev, "I2C write %#04x len %d\n",
-				msgs->addr, msgs->len);
 			count = cp2112_i2c_write_req(buf, msgs->addr,
 						     msgs->buf, msgs->len);
 		}

@@ -648,7 +647,7 @@ static int cp2112_xfer(struct i2c_adapter *adap, u16 addr,
 	int ret;
 
 	hid_dbg(hdev, "%s addr 0x%x flags 0x%x cmd 0x%x size %d\n",
-		read_write == I2C_SMBUS_WRITE ? "write" : "read",
+		str_write_read(read_write == I2C_SMBUS_WRITE),
 		addr, flags, command, size);
 
 	switch (size) {

Signature · 1 line

2.40.0.1.gaa8946217a0b

Andy Shevchenko

author

2023-07-03 18:52 UTC #3 lore raw

PATCH 3/12 HID: cp2112: Make irq_chip immutable

In reply to Andy Shevchenko

Since recently, the kernel is nagging about mutable irq_chips:

"not an immutable chip, please consider fixing it!"

Drop the unneeded copy, flag it as IRQCHIP_IMMUTABLE, add the new helper functions and call the appropriate gpiolib functions.

Signed-off-by Andy Shevchenko <[email protected]>

---
 drivers/hid/hid-cp2112.c | 33 ++++++++++++++++++++-------------
 1 file changed, 20 insertions(+), 13 deletions(-)

drivers/hid/hid-cp2112.c +20 ~~-13~~

index 37ccf4714ad1..51399b231d36 100644
@@ -164,7 +164,6 @@ struct cp2112_device {
 	atomic_t read_avail;
 	atomic_t xfer_avail;
 	struct gpio_chip gc;
-	struct irq_chip irq;
 	u8 *in_out_buffer;
 	struct mutex lock;

@@ -1079,16 +1078,20 @@ static void cp2112_gpio_irq_mask(struct irq_data *d)
 {
 	struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
 	struct cp2112_device *dev = gpiochip_get_data(gc);
+	irq_hw_number_t hwirq = irqd_to_hwirq(d);
 
-	__clear_bit(d->hwirq, &dev->irq_mask);
+	__clear_bit(hwirq, &dev->irq_mask);
+	gpiochip_disable_irq(gc, hwirq);
 }
 
 static void cp2112_gpio_irq_unmask(struct irq_data *d)
 {
 	struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
 	struct cp2112_device *dev = gpiochip_get_data(gc);
+	irq_hw_number_t hwirq = irqd_to_hwirq(d);
 
-	__set_bit(d->hwirq, &dev->irq_mask);
+	gpiochip_enable_irq(gc, hwirq);
+	__set_bit(hwirq, &dev->irq_mask);
 }
 
 static void cp2112_gpio_poll_callback(struct work_struct *work)

@@ -1174,6 +1177,7 @@ static void cp2112_gpio_irq_shutdown(struct irq_data *d)
 	struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
 	struct cp2112_device *dev = gpiochip_get_data(gc);
 
+	cp2112_gpio_irq_mask(d);
 	cancel_delayed_work_sync(&dev->gpio_poll_worker);
 }

@@ -1227,6 +1231,18 @@ static int __maybe_unused cp2112_allocate_irq(struct cp2112_device *dev,
 	return ret;
 }
 
+static const struct irq_chip cp2112_gpio_irqchip = {
+	.name = "cp2112-gpio",
+	.irq_startup = cp2112_gpio_irq_startup,
+	.irq_shutdown = cp2112_gpio_irq_shutdown,
+	.irq_ack = cp2112_gpio_irq_ack,
+	.irq_mask = cp2112_gpio_irq_mask,
+	.irq_unmask = cp2112_gpio_irq_unmask,
+	.irq_set_type = cp2112_gpio_irq_type,
+	.flags = IRQCHIP_MASK_ON_SUSPEND | IRQCHIP_IMMUTABLE,
+	GPIOCHIP_IRQ_RESOURCE_HELPERS,
+};
+
 static int cp2112_probe(struct hid_device *hdev, const struct hid_device_id *id)
 {
 	struct cp2112_device *dev;

@@ -1336,17 +1352,8 @@ static int cp2112_probe(struct hid_device *hdev, const struct hid_device_id *id)
 	dev->gc.can_sleep		= 1;
 	dev->gc.parent			= &hdev->dev;
 
-	dev->irq.name = "cp2112-gpio";
-	dev->irq.irq_startup = cp2112_gpio_irq_startup;
-	dev->irq.irq_shutdown = cp2112_gpio_irq_shutdown;
-	dev->irq.irq_ack = cp2112_gpio_irq_ack;
-	dev->irq.irq_mask = cp2112_gpio_irq_mask;
-	dev->irq.irq_unmask = cp2112_gpio_irq_unmask;
-	dev->irq.irq_set_type = cp2112_gpio_irq_type;
-	dev->irq.flags = IRQCHIP_MASK_ON_SUSPEND;
-
 	girq = &dev->gc.irq;
-	girq->chip = &dev->irq;
+	gpio_irq_chip_set_chip(girq, &cp2112_gpio_irqchip);
 	/* The event comes from the outside so no parent handler */
 	girq->parent_handler = NULL;
 	girq->num_parents = 0;

Signature · 1 line

2.40.0.1.gaa8946217a0b

Andy Shevchenko

author

2023-07-03 18:52 UTC #4 lore raw

PATCH 4/12 HID: cp2112: Switch to for_each_set_bit() to simplify the code

In reply to Andy Shevchenko

It's cleaner to use for_each_set_bit() than open coding it.

Signed-off-by Andy Shevchenko <[email protected]>

---
 drivers/hid/hid-cp2112.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

drivers/hid/hid-cp2112.c +2 -6

index 51399b231d36..fb4548feb0c8 100644
@@ -16,6 +16,7 @@
  *   https://www.silabs.com/documents/public/application-notes/an495-cp2112-interface-specification.pdf
  */
 
+#include <linux/bitops.h>
 #include <linux/gpio/consumer.h>
 #include <linux/gpio/machine.h>
 #include <linux/gpio/driver.h>

@@ -1100,7 +1101,6 @@ static void cp2112_gpio_poll_callback(struct work_struct *work)
 						 gpio_poll_worker.work);
 	struct irq_data *d;
 	u8 gpio_mask;
-	u8 virqs = (u8)dev->irq_mask;
 	u32 irq_type;
 	int irq, virq, ret;

@@ -1111,11 +1111,7 @@ static void cp2112_gpio_poll_callback(struct work_struct *work)
 		goto exit;
 
 	gpio_mask = ret;
-
-	while (virqs) {
-		virq = ffs(virqs) - 1;
-		virqs &= ~BIT(virq);
-
+	for_each_set_bit(virq, &dev->irq_mask, 8) {
 		if (!dev->gc.to_irq)
 			break;

Signature · 1 line

2.40.0.1.gaa8946217a0b

Andy Shevchenko

author

2023-07-03 18:52 UTC #5 lore raw

PATCH 5/12 HID: cp2112: Don't call ->to_irq() explicitly

In reply to Andy Shevchenko

GPIO library guarantees that ->to_irq() is always exists. Moreover, it tending to become a nische thingy and has to not be used in ordinary drivers. Hence, replace that by irq_find_mapping().

Signed-off-by Andy Shevchenko <[email protected]>

---
 drivers/hid/hid-cp2112.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

drivers/hid/hid-cp2112.c +3 -4

index fb4548feb0c8..15b626359281 100644
@@ -1112,10 +1112,9 @@ static void cp2112_gpio_poll_callback(struct work_struct *work)
 
 	gpio_mask = ret;
 	for_each_set_bit(virq, &dev->irq_mask, 8) {
-		if (!dev->gc.to_irq)
-			break;
-
-		irq = dev->gc.to_irq(&dev->gc, virq);
+		irq = irq_find_mapping(dev->gc.irq.domain, virq);
+		if (!irq)
+			continue;
 
 		d = irq_get_irq_data(irq);
 		if (!d)

Signature · 1 line

2.40.0.1.gaa8946217a0b

Andy Shevchenko

author

2023-07-03 18:52 UTC #6 lore raw

PATCH 6/12 HID: cp2112: Remove dead code

In reply to Andy Shevchenko

Remove cp2112_allocate_irq() and counterparts that seems to be a dead code from day 1. In case somebody needs it, it can be retrieved from Git index.

Signed-off-by Andy Shevchenko <[email protected]>

---
 drivers/hid/hid-cp2112.c | 54 ----------------------------------------
 1 file changed, 54 deletions(-)

drivers/hid/hid-cp2112.c +0 ~~-54~~

index 15b626359281..45cd0d2fd3fd 100644
@@ -17,8 +17,6 @@
  */
 
 #include <linux/bitops.h>
-#include <linux/gpio/consumer.h>
-#include <linux/gpio/machine.h>
 #include <linux/gpio/driver.h>
 #include <linux/hid.h>
 #include <linux/hidraw.h>

@@ -168,7 +166,6 @@ struct cp2112_device {
 	u8 *in_out_buffer;
 	struct mutex lock;
 
-	struct gpio_desc *desc[8];
 	bool gpio_poll;
 	struct delayed_work gpio_poll_worker;
 	unsigned long irq_mask;

@@ -1181,51 +1178,6 @@ static int cp2112_gpio_irq_type(struct irq_data *d, unsigned int type)
 	return 0;
 }
 
-static int __maybe_unused cp2112_allocate_irq(struct cp2112_device *dev,
-					      int pin)
-{
-	int ret;
-
-	if (dev->desc[pin])
-		return -EINVAL;
-
-	dev->desc[pin] = gpiochip_request_own_desc(&dev->gc, pin,
-						   "HID/I2C:Event",
-						   GPIO_ACTIVE_HIGH,
-						   GPIOD_IN);
-	if (IS_ERR(dev->desc[pin])) {
-		dev_err(dev->gc.parent, "Failed to request GPIO\n");
-		return PTR_ERR(dev->desc[pin]);
-	}
-
-	ret = cp2112_gpio_direction_input(&dev->gc, pin);
-	if (ret < 0) {
-		dev_err(dev->gc.parent, "Failed to set GPIO to input dir\n");
-		goto err_desc;
-	}
-
-	ret = gpiochip_lock_as_irq(&dev->gc, pin);
-	if (ret) {
-		dev_err(dev->gc.parent, "Failed to lock GPIO as interrupt\n");
-		goto err_desc;
-	}
-
-	ret = gpiod_to_irq(dev->desc[pin]);
-	if (ret < 0) {
-		dev_err(dev->gc.parent, "Failed to translate GPIO to IRQ\n");
-		goto err_lock;
-	}
-
-	return ret;
-
-err_lock:
-	gpiochip_unlock_as_irq(&dev->gc, pin);
-err_desc:
-	gpiochip_free_own_desc(dev->desc[pin]);
-	dev->desc[pin] = NULL;
-	return ret;
-}
-
 static const struct irq_chip cp2112_gpio_irqchip = {
 	.name = "cp2112-gpio",
 	.irq_startup = cp2112_gpio_irq_startup,

@@ -1390,7 +1342,6 @@ static int cp2112_probe(struct hid_device *hdev, const struct hid_device_id *id)
 static void cp2112_remove(struct hid_device *hdev)
 {
 	struct cp2112_device *dev = hid_get_drvdata(hdev);
-	int i;
 
 	sysfs_remove_group(&hdev->dev.kobj, &cp2112_attr_group);
 	i2c_del_adapter(&dev->adap);

@@ -1400,11 +1351,6 @@ static void cp2112_remove(struct hid_device *hdev)
 		cancel_delayed_work_sync(&dev->gpio_poll_worker);
 	}
 
-	for (i = 0; i < ARRAY_SIZE(dev->desc); i++) {
-		gpiochip_unlock_as_irq(&dev->gc, i);
-		gpiochip_free_own_desc(dev->desc[i]);
-	}
-
 	gpiochip_remove(&dev->gc);
 	/* i2c_del_adapter has finished removing all i2c devices from our
 	 * adapter. Well behaved devices should no longer call our cp2112_xfer

Signature · 1 line

2.40.0.1.gaa8946217a0b

Christophe JAILLET

2023-08-26 18:30 UTC #7 lore raw

Re: [PATCH v1 06/12] HID: cp2112: Remove dead code

Le 03/07/2023 à 20:52, Andy Shevchenko a écrit :

Quoted 5 lines

Remove cp2112_allocate_irq() and counterparts that seems to be a dead code from day 1. In case somebody needs it, it can be retrieved from Git index.
Signed-off-by Andy Shevchenko <[email protected]>

Hi,

for the records, just in case it still makesense to keep this code:

https://lore.kernel.org/all/CAO-hwJJfncQ3jgtS=HO0atbzrTNOT_rzU66oG2yRTWTSY-L8KA@mail.gmail.com/

Quoted 97 lines

---
  drivers/hid/hid-cp2112.c | 54 ----------------------------------------
  1 file changed, 54 deletions(-)

drivers/hid/hid-cp2112.c +0 ~~-54~~

index 15b626359281..45cd0d2fd3fd 100644
@@ -17,8 +17,6 @@
   */
 
  #include <linux/bitops.h>
-#include <linux/gpio/consumer.h>
-#include <linux/gpio/machine.h>
  #include <linux/gpio/driver.h>
  #include <linux/hid.h>
  #include <linux/hidraw.h>

@@ -168,7 +166,6 @@ struct cp2112_device {
  	u8 *in_out_buffer;
  	struct mutex lock;
 
-	struct gpio_desc *desc[8];
  	bool gpio_poll;
  	struct delayed_work gpio_poll_worker;
  	unsigned long irq_mask;

@@ -1181,51 +1178,6 @@ static int cp2112_gpio_irq_type(struct irq_data *d, unsigned int type)
  	return 0;
  }
 
-static int __maybe_unused cp2112_allocate_irq(struct cp2112_device *dev,
-					      int pin)
-{
-	int ret;
-
-	if (dev->desc[pin])
-		return -EINVAL;
-
-	dev->desc[pin] = gpiochip_request_own_desc(&dev->gc, pin,
-						   "HID/I2C:Event",
-						   GPIO_ACTIVE_HIGH,
-						   GPIOD_IN);
-	if (IS_ERR(dev->desc[pin])) {
-		dev_err(dev->gc.parent, "Failed to request GPIO\n");
-		return PTR_ERR(dev->desc[pin]);
-	}
-
-	ret = cp2112_gpio_direction_input(&dev->gc, pin);
-	if (ret < 0) {
-		dev_err(dev->gc.parent, "Failed to set GPIO to input dir\n");
-		goto err_desc;
-	}
-
-	ret = gpiochip_lock_as_irq(&dev->gc, pin);
-	if (ret) {
-		dev_err(dev->gc.parent, "Failed to lock GPIO as interrupt\n");
-		goto err_desc;
-	}
-
-	ret = gpiod_to_irq(dev->desc[pin]);
-	if (ret < 0) {
-		dev_err(dev->gc.parent, "Failed to translate GPIO to IRQ\n");
-		goto err_lock;
-	}
-
-	return ret;
-
-err_lock:
-	gpiochip_unlock_as_irq(&dev->gc, pin);
-err_desc:
-	gpiochip_free_own_desc(dev->desc[pin]);
-	dev->desc[pin] = NULL;
-	return ret;
-}
-
  static const struct irq_chip cp2112_gpio_irqchip = {
  	.name = "cp2112-gpio",
  	.irq_startup = cp2112_gpio_irq_startup,

@@ -1390,7 +1342,6 @@ static int cp2112_probe(struct hid_device *hdev, const struct hid_device_id *id)
  static void cp2112_remove(struct hid_device *hdev)
  {
  	struct cp2112_device *dev = hid_get_drvdata(hdev);
-	int i;
 
  	sysfs_remove_group(&hdev->dev.kobj, &cp2112_attr_group);
  	i2c_del_adapter(&dev->adap);

@@ -1400,11 +1351,6 @@ static void cp2112_remove(struct hid_device *hdev)
  		cancel_delayed_work_sync(&dev->gpio_poll_worker);
  	}
 
-	for (i = 0; i < ARRAY_SIZE(dev->desc); i++) {
-		gpiochip_unlock_as_irq(&dev->gc, i);
-		gpiochip_free_own_desc(dev->desc[i]);
-	}
-
  	gpiochip_remove(&dev->gc);
  	/* i2c_del_adapter has finished removing all i2c devices from our
  	 * adapter. Well behaved devices should no longer call our cp2112_xfer

Andy Shevchenko

2023-08-28 08:53 UTC #8 lore raw

Re: [PATCH v1 06/12] HID: cp2112: Remove dead code

Quoted 6 lines · Christophe JAILLET

Quoted 4 lines · Andy Shevchenko
Remove cp2112_allocate_irq() and counterparts that seems to be a dead code from day 1. In case somebody needs it, it can be retrieved from Git index.

Quoted 7 lines

Quoted 1 line
Signed-off-by Andy Shevchenko <[email protected]>
Hi,
for the records, just in case it still makesense to keep this code:
https://lore.kernel.org/all/CAO-hwJJfncQ3jgtS=HO0atbzrTNOT_rzU66oG2yRTWTSY-L8KA@mail.gmail.com/

It's in the Git index, so we can return it, but the rule of thumb is that we do not add dead code to the kernel.

Signature · 2 lines

With Best Regards,
Andy Shevchenko

Andy Shevchenko

author

2023-07-03 18:52 UTC #9 lore raw

PATCH 7/12 HID: cp2112: Define maximum GPIO constant and use it

In reply to Andy Shevchenko

Define maximum GPIO constant and use it in the driver.

Signed-off-by Andy Shevchenko <[email protected]>

---
 drivers/hid/hid-cp2112.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

drivers/hid/hid-cp2112.c +3 -2

index 45cd0d2fd3fd..c8c59d70500e 100644
@@ -31,6 +31,7 @@
 #define CP2112_GPIO_CONFIG_LENGTH		5
 #define CP2112_GPIO_GET_LENGTH			2
 #define CP2112_GPIO_SET_LENGTH			3
+#define CP2112_GPIO_MAX_GPIO			8
 
 enum {
 	CP2112_GPIO_CONFIG		= 0x02,

@@ -1108,7 +1109,7 @@ static void cp2112_gpio_poll_callback(struct work_struct *work)
 		goto exit;
 
 	gpio_mask = ret;
-	for_each_set_bit(virq, &dev->irq_mask, 8) {
+	for_each_set_bit(virq, &dev->irq_mask, CP2112_GPIO_MAX_GPIO) {
 		irq = irq_find_mapping(dev->gc.irq.domain, virq);
 		if (!irq)
 			continue;

@@ -1295,7 +1296,7 @@ static int cp2112_probe(struct hid_device *hdev, const struct hid_device_id *id)
 	dev->gc.set			= cp2112_gpio_set;
 	dev->gc.get			= cp2112_gpio_get;
 	dev->gc.base			= -1;
-	dev->gc.ngpio			= 8;
+	dev->gc.ngpio			= CP2112_GPIO_MAX_GPIO;
 	dev->gc.can_sleep		= 1;
 	dev->gc.parent			= &hdev->dev;

Signature · 1 line

2.40.0.1.gaa8946217a0b

Andy Shevchenko

author

2023-07-03 18:52 UTC #10 lore raw

PATCH 8/12 HID: cp2112: Define all GPIO mask and use it

In reply to Andy Shevchenko

Define all GPIO mask and use it in the driver.

Signed-off-by Andy Shevchenko <[email protected]>

---
 drivers/hid/hid-cp2112.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

drivers/hid/hid-cp2112.c +3 -2

index c8c59d70500e..37ed7fc54770 100644
@@ -32,6 +32,7 @@
 #define CP2112_GPIO_GET_LENGTH			2
 #define CP2112_GPIO_SET_LENGTH			3
 #define CP2112_GPIO_MAX_GPIO			8
+#define CP2112_GPIO_ALL_GPIO_MASK		GENMASK(7, 0)
 
 enum {
 	CP2112_GPIO_CONFIG		= 0x02,

@@ -173,7 +174,7 @@ struct cp2112_device {
 	u8 gpio_prev_state;
 };
 
-static int gpio_push_pull = 0xFF;
+static int gpio_push_pull = CP2112_GPIO_ALL_GPIO_MASK;
 module_param(gpio_push_pull, int, S_IRUGO | S_IWUSR);
 MODULE_PARM_DESC(gpio_push_pull, "GPIO push-pull configuration bitmask");

@@ -226,7 +227,7 @@ static void cp2112_gpio_set(struct gpio_chip *chip, unsigned offset, int value)
 	mutex_lock(&dev->lock);
 
 	buf[0] = CP2112_GPIO_SET;
-	buf[1] = value ? 0xff : 0;
+	buf[1] = value ? CP2112_GPIO_ALL_GPIO_MASK : 0;
 	buf[2] = 1 << offset;
 
 	ret = hid_hw_raw_request(hdev, CP2112_GPIO_SET, buf,

Signature · 1 line

2.40.0.1.gaa8946217a0b

Andy Shevchenko

author

2023-07-03 18:52 UTC #11 lore raw

PATCH 9/12 HID: cp2112: Use BIT() in GPIO setter and getter

In reply to Andy Shevchenko

Use BIT() in GPIO setter and getter for the sake of consistency with GENMASK() usage elsewhere in the driver.

Signed-off-by Andy Shevchenko <[email protected]>

---
 drivers/hid/hid-cp2112.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

drivers/hid/hid-cp2112.c +2 -2

index 37ed7fc54770..2eebb2b19501 100644
@@ -197,7 +197,7 @@ static int cp2112_gpio_direction_input(struct gpio_chip *chip, unsigned offset)
 		goto exit;
 	}
 
-	buf[1] &= ~(1 << offset);
+	buf[1] &= ~BIT(offset);
 	buf[2] = gpio_push_pull;
 
 	ret = hid_hw_raw_request(hdev, CP2112_GPIO_CONFIG, buf,

@@ -228,7 +228,7 @@ static void cp2112_gpio_set(struct gpio_chip *chip, unsigned offset, int value)
 
 	buf[0] = CP2112_GPIO_SET;
 	buf[1] = value ? CP2112_GPIO_ALL_GPIO_MASK : 0;
-	buf[2] = 1 << offset;
+	buf[2] = BIT(offset);
 
 	ret = hid_hw_raw_request(hdev, CP2112_GPIO_SET, buf,
 				 CP2112_GPIO_SET_LENGTH, HID_FEATURE_REPORT,

Signature · 1 line

2.40.0.1.gaa8946217a0b

Andy Shevchenko

author

2023-07-03 18:52 UTC #12 lore raw

PATCH 10/12 HID: cp2112: Use sysfs_emit() to instead of scnprintf()

In reply to Andy Shevchenko

Follow the advice of the Documentation/filesystems/sysfs.rst and show() should only use sysfs_emit() or sysfs_emit_at() when formatting the value to be returned to user space.

Signed-off-by Andy Shevchenko <[email protected]>

---
 drivers/hid/hid-cp2112.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/hid/hid-cp2112.c +1 -1

index 2eebb2b19501..1e45f5407d09 100644
@@ -893,7 +893,7 @@ static ssize_t name##_show(struct device *kdev, \
 	int ret = cp2112_get_usb_config(hdev, &cfg); \
 	if (ret) \
 		return ret; \
-	return scnprintf(buf, PAGE_SIZE, format, ##__VA_ARGS__); \
+	return sysfs_emit(buf, format, ##__VA_ARGS__); \
 } \
 static DEVICE_ATTR_RW(name);

Signature · 1 line

2.40.0.1.gaa8946217a0b

Andy Shevchenko

author

2023-07-03 18:52 UTC #13 lore raw

PATCH 11/12 HID: cp2112: Convert to DEVICE_ATTR_RW()

In reply to Andy Shevchenko

Instead of custom wrapper, use DEVICE_tATTR_RW() directly.

Signed-off-by Andy Shevchenko <[email protected]>

---
 drivers/hid/hid-cp2112.c | 42 ++++++++++++++++++----------------------
 1 file changed, 19 insertions(+), 23 deletions(-)

drivers/hid/hid-cp2112.c +19 ~~-23~~

index 1e45f5407d09..3c6a3be8fc02 100644
@@ -944,18 +944,10 @@ CP2112_CONFIG_ATTR(release_version, ({
 
 #undef CP2112_CONFIG_ATTR
 
-struct cp2112_pstring_attribute {
-	struct device_attribute attr;
-	unsigned char report;
-};
-
-static ssize_t pstr_store(struct device *kdev,
-			  struct device_attribute *kattr, const char *buf,
-			  size_t count)
+static ssize_t pstr_store(struct device *kdev, struct device_attribute *kattr,
+			  const char *buf, size_t count, int number)
 {
 	struct hid_device *hdev = to_hid_device(kdev);
-	struct cp2112_pstring_attribute *attr =
-		container_of(kattr, struct cp2112_pstring_attribute, attr);
 	struct cp2112_string_report report;
 	int ret;

@@ -963,7 +955,7 @@ static ssize_t pstr_store(struct device *kdev,
 
 	ret = utf8s_to_utf16s(buf, count, UTF16_LITTLE_ENDIAN,
 			      report.string, ARRAY_SIZE(report.string));
-	report.report = attr->report;
+	report.report = number;
 	report.length = ret * sizeof(report.string[0]) + 2;
 	report.type = USB_DT_STRING;

@@ -981,17 +973,15 @@ static ssize_t pstr_store(struct device *kdev,
 	return count;
 }
 
-static ssize_t pstr_show(struct device *kdev,
-			 struct device_attribute *kattr, char *buf)
+static ssize_t pstr_show(struct device *kdev, struct device_attribute *kattr,
+			 char *buf, int number)
 {
 	struct hid_device *hdev = to_hid_device(kdev);
-	struct cp2112_pstring_attribute *attr =
-		container_of(kattr, struct cp2112_pstring_attribute, attr);
 	struct cp2112_string_report report;
 	u8 length;
 	int ret;
 
-	ret = cp2112_hid_get(hdev, attr->report, (u8 *)&report.contents,
+	ret = cp2112_hid_get(hdev, number, (u8 *)&report.contents,
 			     sizeof(report.contents), HID_FEATURE_REPORT);
 	if (ret < 3) {
 		hid_err(hdev, "error reading %s string: %d\n", kattr->attr.name,

@@ -1016,10 +1006,16 @@ static ssize_t pstr_show(struct device *kdev,
 }
 
 #define CP2112_PSTR_ATTR(name, _report) \
-static struct cp2112_pstring_attribute dev_attr_##name = { \
-	.attr = __ATTR(name, (S_IWUSR | S_IRUGO), pstr_show, pstr_store), \
-	.report = _report, \
-};
+static ssize_t name##_store(struct device *kdev, struct device_attribute *kattr, \
+			    const char *buf, size_t count) \
+{ \
+	return pstr_store(kdev, kattr, buf, count, _report); \
+} \
+static ssize_t name##_show(struct device *kdev, struct device_attribute *kattr, char *buf) \
+{ \
+	return pstr_show(kdev, kattr, buf, _report); \
+} \
+static DEVICE_ATTR_RW(name);
 
 CP2112_PSTR_ATTR(manufacturer,	CP2112_MANUFACTURER_STRING);
 CP2112_PSTR_ATTR(product,	CP2112_PRODUCT_STRING);

@@ -1034,9 +1030,9 @@ static const struct attribute_group cp2112_attr_group = {
 		&dev_attr_max_power.attr,
 		&dev_attr_power_mode.attr,
 		&dev_attr_release_version.attr,
-		&dev_attr_manufacturer.attr.attr,
-		&dev_attr_product.attr.attr,
-		&dev_attr_serial.attr.attr,
+		&dev_attr_manufacturer.attr,
+		&dev_attr_product.attr,
+		&dev_attr_serial.attr,
 		NULL
 	}
 };

Signature · 1 line

2.40.0.1.gaa8946217a0b

Andy Shevchenko

author

2023-07-03 18:52 UTC #14 lore raw

PATCH 12/12 HID: cp2112: Use octal permissions

In reply to Andy Shevchenko

Octal permissions are preferred as stated in Documentation/dev-tools/checkpatch.rst. Replace symbolic permissions with octal permissions when creating the files.

Signed-off-by Andy Shevchenko <[email protected]>

---
 drivers/hid/hid-cp2112.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

drivers/hid/hid-cp2112.c +2 -2

index 3c6a3be8fc02..adbe8a47cf67 100644
@@ -175,7 +175,7 @@ struct cp2112_device {
 };
 
 static int gpio_push_pull = CP2112_GPIO_ALL_GPIO_MASK;
-module_param(gpio_push_pull, int, S_IRUGO | S_IWUSR);
+module_param(gpio_push_pull, int, 0644);
 MODULE_PARM_DESC(gpio_push_pull, "GPIO push-pull configuration bitmask");
 
 static int cp2112_gpio_direction_input(struct gpio_chip *chip, unsigned offset)

@@ -1057,7 +1057,7 @@ static void chmod_sysfs_attrs(struct hid_device *hdev)
 	}
 
 	for (attr = cp2112_attr_group.attrs; *attr; ++attr) {
-		umode_t mode = (buf[1] & 1) ? S_IWUSR | S_IRUGO : S_IRUGO;
+		umode_t mode = (buf[1] & 1) ? 0644: 0444;
 		ret = sysfs_chmod_file(&hdev->dev.kobj, *attr, mode);
 		if (ret < 0)
 			hid_err(hdev, "error chmoding sysfs file %s\n",

Signature · 1 line

2.40.0.1.gaa8946217a0b

Andy Shevchenko

author

2023-07-27 18:43 UTC #15 lore raw

In reply to Andy Shevchenko

Quoted 9 lines · Andy Shevchenko

After I updated GPIO library for the case Benjamin has with CP2112, I have a brief look into the CP2112 driver itself.
From GPIO perspective it has two main (maitenance) issues:
- usage of ->to_irq() with IRQ chip present;
- having IRQ chip not immutable.
Besides that there are plenty small cleanups here and there. Hence this series.

Any comments on this?

Signature · 2 lines

With Best Regards,
Andy Shevchenko

Andy Shevchenko

author

2023-08-04 06:42 UTC #16 lore raw

Quoted 4 lines · Andy Shevchenko

Quoted 3 lines · Andy Shevchenko
After I updated GPIO library for the case Benjamin has with CP2112, I have a brief look into the CP2112 driver itself.

Quoted 8 lines

Quoted 6 lines
From GPIO perspective it has two main (maitenance) issues:
- usage of ->to_irq() with IRQ chip present;
- having IRQ chip not immutable.
Besides that there are plenty small cleanups here and there. Hence this series.
Any comments on this?

Gentle ping^2 for this...

Anything should I do to improve it or is it okay to go as is?

Signature · 2 lines

With Best Regards,
Andy Shevchenko

Jiri Kosina

maintainer

2023-08-07 11:34 UTC #17 lore raw

Quoted 9 lines · Andy Shevchenko

Quoted 8 lines · Andy Shevchenko
Quoted 7 lines · Andy Shevchenko
After I updated GPIO library for the case Benjamin has with CP2112, I have a brief look into the CP2112 driver itself.
From GPIO perspective it has two main (maitenance) issues:
- usage of ->to_irq() with IRQ chip present;
- having IRQ chip not immutable.

Quoted 8 lines

Quoted 4 lines
Quoted 2 lines
Besides that there are plenty small cleanups here and there. Hence this series.
Any comments on this?
Gentle ping^2 for this...
Anything should I do to improve it or is it okay to go as is?

I have been off pretty much the whole July. I am now back and slowly making my way through everything that accumulated, I will eventually get to this.

Thanks for the patience,

Signature · 2 lines

Jiri Kosina
SUSE Labs

Andy Shevchenko

author

2023-08-07 14:31 UTC #18 lore raw

Quoted 17 lines · Jiri Kosina

Quoted 16 lines · Andy Shevchenko
Quoted 4 lines · Andy Shevchenko
Quoted 3 lines · Andy Shevchenko
After I updated GPIO library for the case Benjamin has with CP2112, I have a brief look into the CP2112 driver itself.
Quoted 8 lines
Quoted 6 lines
From GPIO perspective it has two main (maitenance) issues:
- usage of ->to_irq() with IRQ chip present;
- having IRQ chip not immutable.
Besides that there are plenty small cleanups here and there. Hence this series.
Any comments on this?
Gentle ping^2 for this...

Quoted 7 lines

Quoted 1 line
Anything should I do to improve it or is it okay to go as is?
I have been off pretty much the whole July. I am now back and slowly making my way through everything that accumulated, I will eventually get to this.
Thanks for the patience,

Ah, okay, no worries and take your time!

I was thinking more on Benjamin's answer as last time he had a hw setup to test... Not sure what the status of that now and if he has a chance to test this or busy enough with something else.

Signature · 2 lines

With Best Regards,
Andy Shevchenko

Jiri Kosina

maintainer

2023-08-14 09:29 UTC #19 lore raw

Quoted 20 lines · Andy Shevchenko

Quoted 20 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
After I updated GPIO library for the case Benjamin has with CP2112, I have a brief look into the CP2112 driver itself.
From GPIO perspective it has two main (maitenance) issues:
- usage of ->to_irq() with IRQ chip present;
- having IRQ chip not immutable.
Quoted 8 lines
Quoted 4 lines
Quoted 2 lines
Besides that there are plenty small cleanups here and there. Hence this series.
Any comments on this?
Gentle ping^2 for this...
Anything should I do to improve it or is it okay to go as is?
I have been off pretty much the whole July. I am now back and slowly making my way through everything that accumulated, I will eventually get to this.

Quoted 7 lines

Quoted 1 line
Thanks for the patience,
Ah, okay, no worries and take your time!
I was thinking more on Benjamin's answer as last time he had a hw setup to test... Not sure what the status of that now and if he has a chance to test this or busy enough with something else.

Ah, that would be of course nice. Benjamin?

Thanks,

Signature · 2 lines

Jiri Kosina
SUSE Labs

Andy Shevchenko

author

2023-08-21 08:11 UTC #20 lore raw

Quoted 1 line · Jiri Kosina

On Mon, 7 Aug 2023, Andy Shevchenko wrote:

...

Quoted 22 lines

Quoted 22 lines
Quoted 21 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
After I updated GPIO library for the case Benjamin has with CP2112, I have a brief look into the CP2112 driver itself.
From GPIO perspective it has two main (maitenance) issues:
- usage of ->to_irq() with IRQ chip present;
- having IRQ chip not immutable.
Quoted 8 lines
Quoted 4 lines
Quoted 2 lines
Besides that there are plenty small cleanups here and there. Hence this series.
Any comments on this?
Gentle ping^2 for this...
Anything should I do to improve it or is it okay to go as is?
I have been off pretty much the whole July. I am now back and slowly making my way through everything that accumulated, I will eventually get to this.
Thanks for the patience,

Quoted 7 lines

Quoted 5 lines
Ah, okay, no worries and take your time!
I was thinking more on Benjamin's answer as last time he had a hw setup to test... Not sure what the status of that now and if he has a chance to test this or busy enough with something else.
Ah, that would be of course nice. Benjamin?

Benjamin? It almost full release cycle passed... I understand if you are busy with something, just tell us.

Signature · 2 lines

With Best Regards,
Andy Shevchenko

Benjamin Tissoires

maintainer

2023-08-21 08:51 UTC #21 lore raw

Quoted 29 lines · Andy Shevchenko

Quoted 1 line · Jiri Kosina
On Mon, 7 Aug 2023, Andy Shevchenko wrote:
...
Quoted 24 lines
Quoted 24 lines
Quoted 14 lines
Quoted 14 lines
Quoted 11 lines
Quoted 9 lines
After I updated GPIO library for the case Benjamin has with CP2112, I have a brief look into the CP2112 driver itself.
From GPIO perspective it has two main (maitenance) issues:
- usage of ->to_irq() with IRQ chip present;
- having IRQ chip not immutable.
Besides that there are plenty small cleanups here and there. Hence this series.
Any comments on this?
Gentle ping^2 for this...
Quoted 7 lines
Quoted 1 line
Anything should I do to improve it or is it okay to go as is?
I have been off pretty much the whole July. I am now back and slowly making my way through everything that accumulated, I will eventually get to this.
Thanks for the patience,
Ah, okay, no worries and take your time!

Quoted 8 lines

Quoted 5 lines
Quoted 3 lines
I was thinking more on Benjamin's answer as last time he had a hw setup to test... Not sure what the status of that now and if he has a chance to test this or busy enough with something else.
Ah, that would be of course nice. Benjamin?
Benjamin? It almost full release cycle passed... I understand if you are busy with something, just tell us.

Sorry for not answering, I was off in August until just now.

I tried you series just before taking time off, but the problem was that my automation relies on this driver to not be too far from the current upstream, as I need to patch it to be able to inject a node child in it.

Which is why I was very interested in the ACPI/DT work so that I do not have to patch the driver.

Long story short, I'm not able to test it right now (and I got quite some backlog as you can imagine). IIRC the code was fine, so I think we can just take the series as is, and work on the quirks (if any) later.

Cheers,
Benjamin

Andy Shevchenko

author

2023-08-21 09:34 UTC #22 lore raw

Quoted 3 lines · Benjamin Tissoires

Quoted 2 lines · Andy Shevchenko
Quoted 1 line · Jiri Kosina
On Mon, 7 Aug 2023, Andy Shevchenko wrote:

...

Quoted 39 lines

Quoted 24 lines
Quoted 24 lines
Quoted 24 lines
Quoted 14 lines
Quoted 14 lines
Quoted 11 lines
Quoted 9 lines
After I updated GPIO library for the case Benjamin has with CP2112, I have a brief look into the CP2112 driver itself.
From GPIO perspective it has two main (maitenance) issues:
- usage of ->to_irq() with IRQ chip present;
- having IRQ chip not immutable.
Besides that there are plenty small cleanups here and there. Hence this series.
Any comments on this?
Gentle ping^2 for this...
Quoted 7 lines
Quoted 1 line
Anything should I do to improve it or is it okay to go as is?
I have been off pretty much the whole July. I am now back and slowly making my way through everything that accumulated, I will eventually get to this.
Thanks for the patience,
Ah, okay, no worries and take your time!
Quoted 8 lines
Quoted 5 lines
Quoted 3 lines
I was thinking more on Benjamin's answer as last time he had a hw setup to test... Not sure what the status of that now and if he has a chance to test this or busy enough with something else.
Ah, that would be of course nice. Benjamin?
Benjamin? It almost full release cycle passed... I understand if you are busy with something, just tell us.
Sorry for not answering, I was off in August until just now.
I tried you series just before taking time off, but the problem was that my automation relies on this driver to not be too far from the current upstream, as I need to patch it to be able to inject a node child in it.

Quoted 6 lines

Which is why I was very interested in the ACPI/DT work so that I do not have to patch the driver.
Long story short, I'm not able to test it right now (and I got quite some backlog as you can imagine). IIRC the code was fine, so I think we can just take the series as is, and work on the quirks (if any) later.

Thank you!

The thing that might be broken is interrupts handling. If that works, I'm pretty confident with the rest.

Signature · 2 lines

With Best Regards,
Andy Shevchenko

Andy Shevchenko

author

2023-08-21 09:35 UTC #23 lore raw

Quoted 2 lines · Andy Shevchenko

Quoted 1 line · Benjamin Tissoires
On Aug 21 2023, Andy Shevchenko wrote:

...

Quoted 8 lines

Quoted 3 lines
Long story short, I'm not able to test it right now (and I got quite some backlog as you can imagine). IIRC the code was fine, so I think we can just take the series as is, and work on the quirks (if any) later.
Thank you!
The thing that might be broken is interrupts handling. If that works, I'm pretty confident with the rest.

I.o.w. first 5 patches to test is already 98% of guarantee that everything is fine.

Signature · 2 lines

With Best Regards,
Andy Shevchenko

Benjamin Tissoires

2023-08-21 10:20 UTC #24 lore raw

Quoted 11 lines · Andy Shevchenko

Quoted 2 lines · Andy Shevchenko
Quoted 1 line · Benjamin Tissoires
On Aug 21 2023, Andy Shevchenko wrote:
...
Quoted 4 lines
Quoted 3 lines
Long story short, I'm not able to test it right now (and I got quite some backlog as you can imagine). IIRC the code was fine, so I think we can just take the series as is, and work on the quirks (if any) later.

Quoted 7 lines

Quoted 4 lines
Thank you!
The thing that might be broken is interrupts handling. If that works, I'm pretty confident with the rest.
I.o.w. first 5 patches to test is already 98% of guarantee that everything is fine.

Actually I applied you series locally, and applied Danny's patches on top, and I could run your series in qemu with the cp2112 as USB passthrough.

Everything is working fine, so I can take this one just now.

Cheers,
Benjamin

Quoted 6 lines

--
With Best Regards,
Andy Shevchenko

Benjamin Tissoires

2023-08-21 10:28 UTC #25 lore raw

Quoted 19 lines · Benjamin Tissoires

Quoted 16 lines · Andy Shevchenko
Quoted 2 lines · Andy Shevchenko
Quoted 1 line · Benjamin Tissoires
On Aug 21 2023, Andy Shevchenko wrote:
...
Quoted 8 lines
Quoted 3 lines
Long story short, I'm not able to test it right now (and I got quite some backlog as you can imagine). IIRC the code was fine, so I think we can just take the series as is, and work on the quirks (if any) later.
Thank you!
The thing that might be broken is interrupts handling. If that works, I'm pretty confident with the rest.

Quoted 8 lines

Quoted 2 lines
I.o.w. first 5 patches to test is already 98% of guarantee that everything is fine.
Actually I applied you series locally, and applied Danny's patches on top, and I could run your series in qemu with the cp2112 as USB passthrough.
Everything is working fine, so I can take this one just now.

I've pushed the series to for-6.6/cp2112, but for some reasons, b4 doesn't seem to believe the series is the one you submitted.

Would you mind double checking on your side if everything is good?

https://git.kernel.org/pub/scm/linux/kernel/git/hid/hid.git/log/?h=for-6.6/cp2112

Cheers,
Benjamin

Quoted 10 lines

Cheers,
Benjamin
Quoted 6 lines
--
With Best Regards,
Andy Shevchenko

Andy Shevchenko

author

2023-08-21 10:37 UTC #26 lore raw

Quoted 7 lines · Benjamin Tissoires

Quoted 5 lines · Benjamin Tissoires
Quoted 3 lines · Andy Shevchenko
Quoted 2 lines · Andy Shevchenko
Quoted 1 line · Benjamin Tissoires
On Aug 21 2023, Andy Shevchenko wrote:

...

Quoted 16 lines

Quoted 16 lines
Quoted 11 lines
Quoted 8 lines
Quoted 3 lines
Long story short, I'm not able to test it right now (and I got quite some backlog as you can imagine). IIRC the code was fine, so I think we can just take the series as is, and work on the quirks (if any) later.
Thank you!
The thing that might be broken is interrupts handling. If that works, I'm pretty confident with the rest.
I.o.w. first 5 patches to test is already 98% of guarantee that everything is fine.
Actually I applied you series locally, and applied Danny's patches on top, and I could run your series in qemu with the cp2112 as USB passthrough.

Quoted 8 lines

Quoted 1 line
Everything is working fine, so I can take this one just now.
I've pushed the series to for-6.6/cp2112, but for some reasons, b4 doesn't seem to believe the series is the one you submitted.
Would you mind double checking on your side if everything is good?
https://git.kernel.org/pub/scm/linux/kernel/git/hid/hid.git/log/?h=for-6.6/cp2112

Everything is fine as far as I can tell.

Signature · 2 lines

With Best Regards,
Andy Shevchenko

Benjamin Tissoires

maintainer

2023-08-21 13:56 UTC #27 lore raw

Quoted 29 lines · Andy Shevchenko

Quoted 7 lines · Benjamin Tissoires
Quoted 5 lines · Benjamin Tissoires
Quoted 3 lines · Andy Shevchenko
Quoted 2 lines · Andy Shevchenko
Quoted 1 line · Benjamin Tissoires
On Aug 21 2023, Andy Shevchenko wrote:
...
Quoted 18 lines
Quoted 17 lines
Quoted 11 lines
Quoted 8 lines
Quoted 3 lines
Long story short, I'm not able to test it right now (and I got quite some backlog as you can imagine). IIRC the code was fine, so I think we can just take the series as is, and work on the quirks (if any) later.
Thank you!
The thing that might be broken is interrupts handling. If that works, I'm pretty confident with the rest.
I.o.w. first 5 patches to test is already 98% of guarantee that everything is fine.
Actually I applied you series locally, and applied Danny's patches on top, and I could run your series in qemu with the cp2112 as USB passthrough.
Everything is working fine, so I can take this one just now.

Quoted 8 lines

Quoted 6 lines
I've pushed the series to for-6.6/cp2112, but for some reasons, b4 doesn't seem to believe the series is the one you submitted.
Would you mind double checking on your side if everything is good?
https://git.kernel.org/pub/scm/linux/kernel/git/hid/hid.git/log/?h=for-6.6/cp2112
Everything is fine as far as I can tell.

Great, thanks for double checking.

Cheers,
Benjamin

Quoted 6 lines

Signature · 3 lines
-- 
With Best Regards,
Andy Shevchenko

Andy Shevchenko

author

2023-08-21 10:32 UTC #28 lore raw

In reply to Benjamin Tissoires

Quoted 5 lines · Benjamin Tissoires

Quoted 3 lines · Andy Shevchenko
Quoted 2 lines · Andy Shevchenko
Quoted 1 line · Benjamin Tissoires
On Aug 21 2023, Andy Shevchenko wrote:

...

Quoted 9 lines

Quoted 9 lines
Quoted 8 lines
Quoted 3 lines
Long story short, I'm not able to test it right now (and I got quite some backlog as you can imagine). IIRC the code was fine, so I think we can just take the series as is, and work on the quirks (if any) later.
Thank you!
The thing that might be broken is interrupts handling. If that works, I'm pretty confident with the rest.

Quoted 8 lines

Quoted 2 lines
I.o.w. first 5 patches to test is already 98% of guarantee that everything is fine.
Actually I applied you series locally, and applied Danny's patches on top, and I could run your series in qemu with the cp2112 as USB passthrough.
Everything is working fine, so I can take this one just now.

Thank you! I assume you have some IRQ (like GPIO button) to test with that. If no, it's easily to describe (in ACPI, see [1]) and use a wire to emulate the button presses. In that case the /proc/interrupts should show the different numbers.

[1]: https://github.com/westeri/meta-acpi/blob/master/recipes-bsp/acpi-tables/samples/edison/buttons.asli

Signature · 2 lines

With Best Regards,
Andy Shevchenko

Benjamin Tissoires

2023-08-21 12:07 UTC #29 lore raw

Quoted 22 lines · Andy Shevchenko

Quoted 5 lines · Benjamin Tissoires
Quoted 3 lines · Andy Shevchenko
Quoted 2 lines · Andy Shevchenko
Quoted 1 line · Benjamin Tissoires
On Aug 21 2023, Andy Shevchenko wrote:
...
Quoted 12 lines
Quoted 11 lines
Quoted 8 lines
Quoted 3 lines
Long story short, I'm not able to test it right now (and I got quite some backlog as you can imagine). IIRC the code was fine, so I think we can just take the series as is, and work on the quirks (if any) later.
Thank you!
The thing that might be broken is interrupts handling. If that works, I'm pretty confident with the rest.
I.o.w. first 5 patches to test is already 98% of guarantee that everything is fine.

Quoted 7 lines

Quoted 5 lines
Actually I applied you series locally, and applied Danny's patches on top, and I could run your series in qemu with the cp2112 as USB passthrough.
Everything is working fine, so I can take this one just now.
Thank you! I assume you have some IRQ (like GPIO button) to test with that.

Yeah, binding a test i2c-hid touchpad on top of hid forces you to use GPIOs. Otherwise you are polling, and it's not allowed in i2c-hid anymore IIRC :)

Quoted 3 lines

If no, it's easily to describe (in ACPI, see [1]) and use a wire to emulate the button presses. In that case the /proc/interrupts should show the different numbers.

Thanks, but again, the GPIO is tested just by checking if the touchpad can send events when touched.

Now I need to update my CI to rely on danny's patches and a DSDT overwrite :)

Cheers,
Benjamin

Quoted 8 lines

[1]: https://github.com/westeri/meta-acpi/blob/master/recipes-bsp/acpi-tables/samples/edison/buttons.asli
--
With Best Regards,
Andy Shevchenko