* Re: [PATCH RFC 1/4] dt-bindings: input: touchscreen: Add Z2 controller bindings.
From: Sven Peter @ 2023-02-24 11:08 UTC (permalink / raw)
To: Sasha Finkelstein, Mark Kettenis
Cc: Hector Martin, Alyssa Rosenzweig, Dmitry Torokhov, Rob Herring,
Krzysztof Kozlowski, asahi, Henrik Rydberg, linux-arm-kernel,
linux-input, devicetree, linux-kernel
In-Reply-To: <CAMT+MTQOUd0aSDJ3DPBMfkVwaic=nbRPtfGgu2nduSdCdydcgg@mail.gmail.com>
Hi,
On Fri, Feb 24, 2023, at 12:04, Sasha Finkelstein wrote:
> On Fri, 24 Feb 2023 at 11:55, Mark Kettenis <mark.kettenis@xs4all.nl> wrote:
>
>> What is the motivation for including the firmware name in the device
>> tree rather than constructing it in the driver like what is done for
>> the broadcom wireless?
> There is no way to identify the device subtype before the firmware is
> uploaded, and so i need some way of figuring out which firmware to use.
Some Broadcom bluetooth boards use the compatible of the root node (see
btbcm_get_board_name in drivers/bluetooth/btbcm.c) which would be "apple,jXXX"
for Apple Silicon. I believe the Broadcom WiFi driver has similar logic as well
which marcan had to extend to instead of "brcm,board-type" because different
WiFi boards can me matched to different Apple Silicon boards. I don't think
that's the case for this touchscreen though.
Sven
^ permalink raw reply
* Re: [PATCH RFC 1/4] dt-bindings: input: touchscreen: Add Z2 controller bindings.
From: Sven Peter @ 2023-02-24 11:03 UTC (permalink / raw)
To: Sasha Finkelstein, Hector Martin, Alyssa Rosenzweig,
Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, -,
Henrik Rydberg
Cc: linux-arm-kernel, linux-input, devicetree, linux-kernel
In-Reply-To: <20230223-z2-for-ml-v1-1-028f2b85dc15@gmail.com>
On Fri, Feb 24, 2023, at 11:20, Sasha Finkelstein via B4 Relay wrote:
> From: Sasha Finkelstein <fnkl.kernel@gmail.com>
>
> Add bindings for touchscreen controllers attached using the Z2 protocol.
> Those are present in most Apple devices.
>
> Signed-off-by: Sasha Finkelstein <fnkl.kernel@gmail.com>
> ---
> .../input/touchscreen/apple,z2-touchscreen.yaml | 81 ++++++++++++++++++++++
> 1 file changed, 81 insertions(+)
>
> diff --git
> a/Documentation/devicetree/bindings/input/touchscreen/apple,z2-touchscreen.yaml
> b/Documentation/devicetree/bindings/input/touchscreen/apple,z2-touchscreen.yaml
> new file mode 100644
> index 000000000000..695594494b1e
> --- /dev/null
> +++
> b/Documentation/devicetree/bindings/input/touchscreen/apple,z2-touchscreen.yaml
> @@ -0,0 +1,81 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id:
> http://devicetree.org/schemas/input/touchscreen/apple,z2-touchscreen.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Apple touchscreens attached using the Z2 protocol.
> +
> +maintainers:
> + - asahi@lists.linux.dev
> + - Sasha Finkelstein <fnkl.kernel@gmail.com>
> +
> +description: A series of touschscreen controllers used in Apple
> products.
> +
> +allOf:
> + - $ref: touchscreen.yaml#
> + - $ref: /schemas/spi/spi-peripheral-props.yaml#
> +
> +properties:
> + compatible:
> + const: apple,z2-touchscreen
> +
> + reg:
> + maxItems: 1
> +
> + interrupts-extended:
> + maxItems: 1
> +
> + reset-gpios:
> + maxItems: 1
> +
> + cs-gpios:
> + maxItems: 1
> +
> + firmware-name:
> + maxItems: 1
> +
> + apple,z2-device-name:
> + description: The name to be used for the input device
> + $ref: /schemas/types.yaml#/definitions/string
Now that I thought about this again after the brief discussion we already had:
Do we even need to specify the device name? Is there any reason we can't just
always use something like "Apple Z2 TouchBar"?
Best,
Sven
^ permalink raw reply
* Re: [PATCH RFC 1/4] dt-bindings: input: touchscreen: Add Z2 controller bindings.
From: Sasha Finkelstein @ 2023-02-24 11:04 UTC (permalink / raw)
To: Mark Kettenis
Cc: marcan, sven, alyssa, dmitry.torokhov, robh+dt,
krzysztof.kozlowski+dt, asahi, rydberg, linux-arm-kernel,
linux-input, devicetree, linux-kernel
In-Reply-To: <87r0ufs574.fsf@bloch.sibelius.xs4all.nl>
On Fri, 24 Feb 2023 at 11:55, Mark Kettenis <mark.kettenis@xs4all.nl> wrote:
> What is the motivation for including the firmware name in the device
> tree rather than constructing it in the driver like what is done for
> the broadcom wireless?
There is no way to identify the device subtype before the firmware is
uploaded, and so i need some way of figuring out which firmware to use.
^ permalink raw reply
* Re: [PATCH RFC 1/4] dt-bindings: input: touchscreen: Add Z2 controller bindings.
From: Mark Kettenis @ 2023-02-24 10:55 UTC (permalink / raw)
To: fnkl.kernel
Cc: marcan, sven, alyssa, dmitry.torokhov, robh+dt,
krzysztof.kozlowski+dt, asahi, rydberg, linux-arm-kernel,
linux-input, devicetree, linux-kernel, fnkl.kernel
In-Reply-To: <20230223-z2-for-ml-v1-1-028f2b85dc15@gmail.com>
> Date: Fri, 24 Feb 2023 11:20:06 +0100
> From: Sasha Finkelstein <fnkl.kernel@gmail.com>
Hi Sasha,
> Add bindings for touchscreen controllers attached using the Z2 protocol.
> Those are present in most Apple devices.
>
> Signed-off-by: Sasha Finkelstein <fnkl.kernel@gmail.com>
> ---
> .../input/touchscreen/apple,z2-touchscreen.yaml | 81 ++++++++++++++++++++++
> 1 file changed, 81 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/input/touchscreen/apple,z2-touchscreen.yaml b/Documentation/devicetree/bindings/input/touchscreen/apple,z2-touchscreen.yaml
> new file mode 100644
> index 000000000000..695594494b1e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/touchscreen/apple,z2-touchscreen.yaml
> @@ -0,0 +1,81 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/input/touchscreen/apple,z2-touchscreen.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Apple touchscreens attached using the Z2 protocol.
> +
> +maintainers:
> + - asahi@lists.linux.dev
> + - Sasha Finkelstein <fnkl.kernel@gmail.com>
> +
> +description: A series of touschscreen controllers used in Apple products.
> +
> +allOf:
> + - $ref: touchscreen.yaml#
> + - $ref: /schemas/spi/spi-peripheral-props.yaml#
> +
> +properties:
> + compatible:
> + const: apple,z2-touchscreen
> +
> + reg:
> + maxItems: 1
> +
> + interrupts-extended:
> + maxItems: 1
> +
> + reset-gpios:
> + maxItems: 1
> +
> + cs-gpios:
> + maxItems: 1
> +
> + firmware-name:
> + maxItems: 1
What is the motivation for including the firmware name in the device
tree rather than constructing it in the driver like what is done for
the broadcom wireless?
In your example the firmware name includes the directory name. I
think that is a bad idea since it makes assumptions about the
directory layout used for the firmware files on the OS level. And in
particular, forcing the firmware to be in a subdirectory named "apple"
would be awkward for the way we handle firmware in OpenBSD.
> +
> + apple,z2-device-name:
> + description: The name to be used for the input device
> + $ref: /schemas/types.yaml#/definitions/string
> +
> + touchscreen-size-x: true
> + touchscreen-size-y: true
> + spi-max-frequency: true
> +
> +required:
> + - compatible
> + - interrupts-extended
> + - reset-gpios
> + - cs-gpios
> + - firmware-name
> + - apple,z2-device-name
> + - touchscreen-size-x
> + - touchscreen-size-y
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/gpio/gpio.h>
> + #include <dt-bindings/interrupt-controller/irq.h>
> +
> + spi {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + touchscreen@0 {
> + compatible = "apple,z2-touchscreen";
> + reg = <0>;
> + spi-max-frequency = <11500000>;
> + reset-gpios = <&pinctrl_ap 139 0>;
> + cs-gpios = <&pinctrl_ap 109 0>;
> + interrupts-extended = <&pinctrl_ap 194 IRQ_TYPE_EDGE_FALLING>;
> + firmware-name = "apple/dfrmtfw-j293.bin";
> + touchscreen-size-x = <23045>;
> + touchscreen-size-y = <640>;
> + apple,z2-device-name = "MacBookPro17,1 Touch Bar";
> + };
> + };
> +
> +...
>
> --
> Git-137.1)
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
^ permalink raw reply
* [PATCH RFC 2/4] input: apple_z2: Add a driver for Apple Z2 touchscreens
From: Sasha Finkelstein via B4 Relay @ 2023-02-24 10:20 UTC (permalink / raw)
To: Hector Martin, Sven Peter, Alyssa Rosenzweig, Dmitry Torokhov,
Rob Herring, Krzysztof Kozlowski, -, Henrik Rydberg
Cc: linux-arm-kernel, linux-input, devicetree, linux-kernel,
Sasha Finkelstein
In-Reply-To: <20230223-z2-for-ml-v1-0-028f2b85dc15@gmail.com>
From: Sasha Finkelstein <fnkl.kernel@gmail.com>
Adds a driver for Apple touchscreens using the Z2 protocol.
Signed-off-by: Sasha Finkelstein <fnkl.kernel@gmail.com>
---
drivers/input/touchscreen/Kconfig | 13 +
drivers/input/touchscreen/Makefile | 1 +
drivers/input/touchscreen/apple_z2.c | 465 +++++++++++++++++++++++++++++++++++
3 files changed, 479 insertions(+)
diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig
index 68d99a112e14..76aeea837ab9 100644
--- a/drivers/input/touchscreen/Kconfig
+++ b/drivers/input/touchscreen/Kconfig
@@ -103,6 +103,19 @@ config TOUCHSCREEN_ADC
To compile this driver as a module, choose M here: the
module will be called resistive-adc-touch.ko.
+config TOUCHSCREEN_APPLE_Z2
+ tristate "Apple Z2 touchscreens"
+ default ARCH_APPLE
+ depends on SPI && INPUT && OF
+ help
+ Say Y here if you have an Apple device with
+ a touchscreen or a touchbar.
+
+ If unsure, say N.
+
+ To compile this driver as a module, choose M here: the
+ module will be called apple_z2.
+
config TOUCHSCREEN_AR1021_I2C
tristate "Microchip AR1020/1021 i2c touchscreen"
depends on I2C && OF
diff --git a/drivers/input/touchscreen/Makefile b/drivers/input/touchscreen/Makefile
index 4968c370479a..d885b2c4e3a6 100644
--- a/drivers/input/touchscreen/Makefile
+++ b/drivers/input/touchscreen/Makefile
@@ -15,6 +15,7 @@ obj-$(CONFIG_TOUCHSCREEN_AD7879_I2C) += ad7879-i2c.o
obj-$(CONFIG_TOUCHSCREEN_AD7879_SPI) += ad7879-spi.o
obj-$(CONFIG_TOUCHSCREEN_ADC) += resistive-adc-touch.o
obj-$(CONFIG_TOUCHSCREEN_ADS7846) += ads7846.o
+obj-$(CONFIG_TOUCHSCREEN_APPLE_Z2) += apple_z2.o
obj-$(CONFIG_TOUCHSCREEN_AR1021_I2C) += ar1021_i2c.o
obj-$(CONFIG_TOUCHSCREEN_ATMEL_MXT) += atmel_mxt_ts.o
obj-$(CONFIG_TOUCHSCREEN_AUO_PIXCIR) += auo-pixcir-ts.o
diff --git a/drivers/input/touchscreen/apple_z2.c b/drivers/input/touchscreen/apple_z2.c
new file mode 100644
index 000000000000..06f1d864a137
--- /dev/null
+++ b/drivers/input/touchscreen/apple_z2.c
@@ -0,0 +1,465 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Apple Z2 touchscreen driver
+ *
+ * Copyright (C) The Asahi Linux Contributors
+ */
+
+#include <linux/module.h>
+#include <linux/spi/spi.h>
+#include <linux/interrupt.h>
+#include <linux/delay.h>
+#include <linux/firmware.h>
+#include <linux/input.h>
+#include <linux/input/mt.h>
+
+#define Z2_NUM_FINGERS_OFFSET 16
+#define Z2_FINGERS_OFFSET 24
+#define Z2_TOUCH_STARTED 3
+#define Z2_TOUCH_MOVED 4
+#define Z2_CMD_READ_INTERRUPT_DATA 0xEB
+#define Z2_HBPP_CMD_BLOB 0x3001
+#define Z2_FW_MAGIC 0x5746325A
+#define LOAD_COMMAND_INIT_PAYLOAD 0
+#define LOAD_COMMAND_SEND_BLOB 1
+#define LOAD_COMMAND_SEND_CALIBRATION 2
+
+struct apple_z2 {
+ struct spi_device *spidev;
+ struct gpio_desc *cs_gpio;
+ struct gpio_desc *reset_gpio;
+ struct input_dev *input_dev;
+ struct completion boot_irq;
+ int booted;
+ int counter;
+ int y_size;
+ const char *fw_name;
+ const char *cal_blob;
+ int cal_size;
+};
+
+struct z2_finger {
+ u8 finger;
+ u8 state;
+ __le16 unknown2;
+ __le16 abs_x;
+ __le16 abs_y;
+ __le16 rel_x;
+ __le16 rel_y;
+ __le16 tool_major;
+ __le16 tool_minor;
+ __le16 orientation;
+ __le16 touch_major;
+ __le16 touch_minor;
+ __le16 unused[2];
+ __le16 pressure;
+ __le16 multi;
+} __packed;
+
+struct z2_hbpp_blob_hdr {
+ u16 cmd;
+ u16 len;
+ u32 addr;
+ u16 checksum;
+} __packed;
+
+struct z2_fw_hdr {
+ u32 magic;
+ u32 version;
+} __packed;
+
+struct z2_read_interrupt_cmd {
+ u8 cmd;
+ u8 counter;
+ u8 unused[12];
+ __le16 checksum;
+} __packed;
+
+static void apple_z2_parse_touches(struct apple_z2 *z2, char *msg, size_t msg_len)
+{
+ int i;
+ int nfingers;
+ int slot;
+ int slot_valid;
+ struct z2_finger *fingers;
+
+ if (msg_len <= Z2_NUM_FINGERS_OFFSET)
+ return;
+ nfingers = msg[Z2_NUM_FINGERS_OFFSET];
+ fingers = (struct z2_finger *)(msg + Z2_FINGERS_OFFSET);
+ for (i = 0; i < nfingers; i++) {
+ slot = input_mt_get_slot_by_key(z2->input_dev, fingers[i].finger);
+ if (slot < 0) {
+ dev_warn(&z2->spidev->dev, "unable to get slot for finger");
+ continue;
+ }
+ slot_valid = fingers[i].state == Z2_TOUCH_STARTED ||
+ fingers[i].state == Z2_TOUCH_MOVED;
+ input_mt_slot(z2->input_dev, slot);
+ input_mt_report_slot_state(z2->input_dev, MT_TOOL_FINGER, slot_valid);
+ if (!slot_valid)
+ continue;
+ input_report_abs(z2->input_dev, ABS_MT_POSITION_X,
+ le16_to_cpu(fingers[i].abs_x));
+ input_report_abs(z2->input_dev, ABS_MT_POSITION_Y,
+ z2->y_size - le16_to_cpu(fingers[i].abs_y));
+ input_report_abs(z2->input_dev, ABS_MT_WIDTH_MAJOR,
+ le16_to_cpu(fingers[i].tool_major));
+ input_report_abs(z2->input_dev, ABS_MT_WIDTH_MINOR,
+ le16_to_cpu(fingers[i].tool_minor));
+ input_report_abs(z2->input_dev, ABS_MT_ORIENTATION,
+ le16_to_cpu(fingers[i].orientation));
+ input_report_abs(z2->input_dev, ABS_MT_TOUCH_MAJOR,
+ le16_to_cpu(fingers[i].touch_major));
+ input_report_abs(z2->input_dev, ABS_MT_TOUCH_MINOR,
+ le16_to_cpu(fingers[i].touch_minor));
+ }
+ input_mt_sync_frame(z2->input_dev);
+ input_sync(z2->input_dev);
+}
+
+static int apple_z2_spi_sync(struct apple_z2 *z2, struct spi_message *msg)
+{
+ int err;
+ gpiod_direction_output(z2->cs_gpio, 0);
+ usleep_range(1000, 2000);
+ err = spi_sync(z2->spidev, msg);
+ usleep_range(1000, 2000);
+ gpiod_direction_output(z2->cs_gpio, 1);
+ return err;
+}
+
+static int apple_z2_read_packet(struct apple_z2 *z2)
+{
+ struct spi_message msg;
+ struct spi_transfer xfer;
+ struct z2_read_interrupt_cmd len_cmd;
+ char len_rx[16];
+ size_t pkt_len;
+ char *pkt_rx;
+ int err = 0;
+
+ spi_message_init(&msg);
+ memset(&xfer, 0, sizeof(xfer));
+ memset(&len_cmd, 0, sizeof(len_cmd));
+ len_cmd.cmd = Z2_CMD_READ_INTERRUPT_DATA;
+ len_cmd.counter = z2->counter + 1;
+ len_cmd.checksum = cpu_to_le16(Z2_CMD_READ_INTERRUPT_DATA + 1 + z2->counter);
+ z2->counter = 1 - z2->counter;
+ xfer.tx_buf = &len_cmd;
+ xfer.rx_buf = len_rx;
+ xfer.len = sizeof(len_cmd);
+ spi_message_add_tail(&xfer, &msg);
+ err = apple_z2_spi_sync(z2, &msg);
+ if (err)
+ return err;
+
+ pkt_len = ((len_rx[1] | len_rx[2] << 8) + 8) & (-4);
+ pkt_rx = kzalloc(pkt_len, GFP_KERNEL);
+ if (!pkt_rx)
+ return -ENOMEM;
+
+ spi_message_init(&msg);
+ xfer.rx_buf = pkt_rx;
+ xfer.len = pkt_len;
+ spi_message_add_tail(&xfer, &msg);
+ err = apple_z2_spi_sync(z2, &msg);
+
+ if (!err)
+ apple_z2_parse_touches(z2, pkt_rx + 5, pkt_len - 5);
+
+ kfree(pkt_rx);
+ return err;
+}
+
+static irqreturn_t apple_z2_irq(int irq, void *data)
+{
+ struct spi_device *spi = data;
+ struct apple_z2 *z2 = spi_get_drvdata(spi);
+
+ if (!z2->booted)
+ complete(&z2->boot_irq);
+ else
+ apple_z2_read_packet(z2);
+
+ return IRQ_HANDLED;
+}
+
+
+// Return value must be freed with kfree
+static char *apple_z2_build_cal_blob(struct apple_z2 *z2, u32 address, u32 *size)
+{
+ u16 len_words = (z2->cal_size + 3) / 4;
+ u32 checksum = 0;
+ u16 checksum_hdr = 0;
+ char *data;
+ int i;
+ struct z2_hbpp_blob_hdr *hdr;
+
+ *size = z2->cal_size + sizeof(struct z2_hbpp_blob_hdr) + 4;
+
+ data = kzalloc(*size, GFP_KERNEL);
+ hdr = (struct z2_hbpp_blob_hdr *)data;
+ hdr->cmd = Z2_HBPP_CMD_BLOB;
+ hdr->len = len_words;
+ hdr->addr = address;
+
+ for (i = 2; i < 8; i++)
+ checksum_hdr += data[i];
+
+ hdr->checksum = checksum_hdr;
+ memcpy(data + 10, z2->cal_blob, z2->cal_size);
+
+ for (i = 0; i < z2->cal_size; i++)
+ checksum += z2->cal_blob[i];
+
+ *(u32 *)(data + z2->cal_size + 10) = checksum;
+
+ return data;
+}
+
+static int apple_z2_send_firmware_blob(struct apple_z2 *z2, const char *data, u32 size, u8 bpw)
+{
+ struct spi_message msg;
+ struct spi_transfer blob_xfer, ack_xfer;
+ char int_ack[] = {0x1a, 0xa1};
+ char ack_rsp[] = {0, 0};
+ int err;
+
+ spi_message_init(&msg);
+ memset(&blob_xfer, 0, sizeof(blob_xfer));
+ memset(&ack_xfer, 0, sizeof(ack_xfer));
+ blob_xfer.tx_buf = data;
+ blob_xfer.len = size;
+ blob_xfer.bits_per_word = bpw;
+ spi_message_add_tail(&blob_xfer, &msg);
+ ack_xfer.tx_buf = int_ack;
+ ack_xfer.rx_buf = ack_rsp;
+ ack_xfer.len = 2;
+ spi_message_add_tail(&ack_xfer, &msg);
+ reinit_completion(&z2->boot_irq);
+ err = apple_z2_spi_sync(z2, &msg);
+ if (err)
+ return err;
+ wait_for_completion_timeout(&z2->boot_irq, msecs_to_jiffies(20));
+ return 0;
+}
+
+static int apple_z2_upload_firmware(struct apple_z2 *z2)
+{
+ const struct firmware *fw;
+ struct z2_fw_hdr *fw_hdr;
+ size_t fw_idx = sizeof(struct z2_fw_hdr);
+ int err;
+ u32 load_cmd;
+ u32 size;
+ u32 address;
+ char *data;
+
+ err = request_firmware(&fw, z2->fw_name, &z2->spidev->dev);
+ if (err)
+ return dev_err_probe(&z2->spidev->dev, err, "unable to load firmware");
+
+ fw_hdr = (struct z2_fw_hdr *)fw->data;
+ if (fw_hdr->magic != Z2_FW_MAGIC || fw_hdr->version != 1)
+ return dev_err_probe(&z2->spidev->dev, -EINVAL, "invalid firmware header");
+
+ while (fw_idx < fw->size) {
+ if (fw->size - fw_idx < 8) {
+ err = dev_err_probe(&z2->spidev->dev, -EINVAL, "firmware malformed");
+ goto error;
+ }
+
+ load_cmd = *(u32 *)(fw->data + fw_idx);
+ fw_idx += 4;
+ if (load_cmd == LOAD_COMMAND_INIT_PAYLOAD || load_cmd == LOAD_COMMAND_SEND_BLOB) {
+ size = *(u32 *)(fw->data + fw_idx);
+ fw_idx += 4;
+ if (fw->size - fw_idx < size) {
+ err = dev_err_probe(&z2->spidev->dev,
+ -EINVAL, "firmware malformed");
+ goto error;
+ }
+ err = apple_z2_send_firmware_blob(z2, fw->data + fw_idx,
+ size, load_cmd == LOAD_COMMAND_SEND_BLOB ? 16 : 8);
+ if (err)
+ goto error;
+ fw_idx += size;
+ } else if (load_cmd == 2) {
+ address = *(u32 *)(fw->data + fw_idx);
+ fw_idx += 4;
+ data = apple_z2_build_cal_blob(z2, address, &size);
+ err = apple_z2_send_firmware_blob(z2, data, size, 16);
+ kfree(data);
+ if (err)
+ goto error;
+ } else {
+ err = dev_err_probe(&z2->spidev->dev, -EINVAL, "firmware malformed");
+ goto error;
+ }
+ if (fw_idx % 4 != 0)
+ fw_idx += 4 - (fw_idx % 4);
+ }
+
+
+ z2->booted = 1;
+ apple_z2_read_packet(z2);
+ error:
+ release_firmware(fw);
+ return err;
+}
+
+static int apple_z2_boot(struct apple_z2 *z2)
+{
+ int err;
+ enable_irq(z2->spidev->irq);
+ gpiod_direction_output(z2->reset_gpio, 1);
+ wait_for_completion_timeout(&z2->boot_irq, msecs_to_jiffies(20));
+ err = apple_z2_upload_firmware(z2);
+ if (err) { // Boot failed, let's put the device into reset just in case.
+ gpiod_direction_output(z2->reset_gpio, 0);
+ disable_irq(z2->spidev->irq);
+ }
+ return err;
+}
+
+static int apple_z2_probe(struct spi_device *spi)
+{
+ struct device *dev = &spi->dev;
+ struct apple_z2 *z2;
+ int err;
+ int x_size;
+ const char *device_name;
+
+ z2 = devm_kzalloc(dev, sizeof(*z2), GFP_KERNEL);
+ if (!z2)
+ return -ENOMEM;
+
+ z2->spidev = spi;
+ init_completion(&z2->boot_irq);
+ spi_set_drvdata(spi, z2);
+
+ z2->cs_gpio = devm_gpiod_get_index(dev, "cs", 0, 0);
+ if (IS_ERR(z2->cs_gpio))
+ return dev_err_probe(dev, PTR_ERR(z2->cs_gpio), "unable to get cs");
+
+ z2->reset_gpio = devm_gpiod_get_index(dev, "reset", 0, 0);
+ if (IS_ERR(z2->reset_gpio))
+ return dev_err_probe(dev, PTR_ERR(z2->reset_gpio), "unable to get reset");
+
+ err = devm_request_threaded_irq(dev, z2->spidev->irq, NULL,
+ apple_z2_irq, IRQF_ONESHOT | IRQF_NO_AUTOEN,
+ "apple-z2-irq", spi);
+ if (err < 0)
+ return dev_err_probe(dev, z2->spidev->irq, "unable to request irq");
+
+ err = device_property_read_u32(dev, "touchscreen-size-x", &x_size);
+ if (err)
+ return dev_err_probe(dev, err, "unable to get touchscreen size");
+
+ err = device_property_read_u32(dev, "touchscreen-size-y", &z2->y_size);
+ if (err)
+ return dev_err_probe(dev, err, "unable to get touchscreen size");
+
+ err = device_property_read_string(dev, "apple,z2-device-name", &device_name);
+ if (err)
+ return dev_err_probe(dev, err, "unable to get device name");
+
+ err = device_property_read_string(dev, "firmware-name", &z2->fw_name);
+ if (err)
+ return dev_err_probe(dev, err, "unable to get firmware name");
+
+ z2->cal_blob = of_get_property(dev->of_node, "apple,z2-cal-blob", &z2->cal_size);
+ if (!z2->cal_blob)
+ return dev_err_probe(dev, -EINVAL, "unable to get calibration");
+
+ z2->input_dev = devm_input_allocate_device(dev);
+ if (!z2->input_dev)
+ return -ENOMEM;
+ z2->input_dev->name = device_name;
+ z2->input_dev->phys = "apple_z2";
+ z2->input_dev->dev.parent = dev;
+ z2->input_dev->id.bustype = BUS_SPI;
+ input_set_abs_params(z2->input_dev, ABS_MT_POSITION_X, 0, x_size, 0, 0);
+ input_abs_set_res(z2->input_dev, ABS_MT_POSITION_X, 1);
+ input_set_abs_params(z2->input_dev, ABS_MT_POSITION_Y, 0, z2->y_size, 0, 0);
+ input_abs_set_res(z2->input_dev, ABS_MT_POSITION_Y, 1);
+ input_set_abs_params(z2->input_dev, ABS_MT_WIDTH_MAJOR, 0, 65535, 0, 0);
+ input_set_abs_params(z2->input_dev, ABS_MT_WIDTH_MINOR, 0, 65535, 0, 0);
+ input_set_abs_params(z2->input_dev, ABS_MT_TOUCH_MAJOR, 0, 65535, 0, 0);
+ input_set_abs_params(z2->input_dev, ABS_MT_TOUCH_MINOR, 0, 65535, 0, 0);
+ input_set_abs_params(z2->input_dev, ABS_MT_ORIENTATION, -32768, 32767, 0, 0);
+ input_mt_init_slots(z2->input_dev, 256, INPUT_MT_DIRECT);
+
+ err = input_register_device(z2->input_dev);
+ if (err < 0)
+ return dev_err_probe(dev, err, "unable to register input device");
+
+
+ // Reset the device on probe
+ gpiod_direction_output(z2->reset_gpio, 0);
+ usleep_range(5000, 10000);
+ return apple_z2_boot(z2);
+}
+
+static void apple_z2_remove(struct spi_device *spi)
+{
+ struct apple_z2 *z2 = spi_get_drvdata(spi);
+
+ disable_irq(z2->spidev->irq);
+ gpiod_direction_output(z2->reset_gpio, 0);
+}
+
+static void apple_z2_shutdown(struct spi_device *spi)
+{
+ struct apple_z2 *z2 = spi_get_drvdata(spi);
+
+ disable_irq(z2->spidev->irq);
+ gpiod_direction_output(z2->reset_gpio, 0);
+}
+
+static int apple_z2_suspend(struct device *dev)
+{
+ apple_z2_shutdown(to_spi_device(dev));
+ return 0;
+}
+
+static int apple_z2_resume(struct device *dev)
+{
+ struct apple_z2 *z2 = spi_get_drvdata(to_spi_device(dev));
+
+ return apple_z2_boot(z2);
+}
+
+static DEFINE_SIMPLE_DEV_PM_OPS(apple_z2_pm, apple_z2_suspend, apple_z2_resume);
+
+static const struct of_device_id apple_z2_of_match[] = {
+ { .compatible = "apple,z2-touchscreen" },
+ {},
+};
+MODULE_DEVICE_TABLE(of, apple_z2_of_match);
+
+static struct spi_device_id apple_z2_of_id[] = {
+ { .name = "z2-touchscreen" },
+ {}
+};
+MODULE_DEVICE_TABLE(spi, apple_z2_of_id);
+
+static struct spi_driver apple_z2_driver = {
+ .driver = {
+ .name = "apple-z2",
+ .pm = pm_sleep_ptr(&apple_z2_pm),
+ .owner = THIS_MODULE,
+ .of_match_table = of_match_ptr(apple_z2_of_match),
+ },
+
+ .id_table = apple_z2_of_id,
+ .probe = apple_z2_probe,
+ .remove = apple_z2_remove,
+ .shutdown = apple_z2_shutdown,
+};
+
+module_spi_driver(apple_z2_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_FIRMWARE("apple/mtfw-*.bin");
--
Git-137.1)
^ permalink raw reply related
* [PATCH RFC 3/4] arm64: dts: apple: t8103: Add touchbar bindings
From: Sasha Finkelstein via B4 Relay @ 2023-02-24 10:20 UTC (permalink / raw)
To: Hector Martin, Sven Peter, Alyssa Rosenzweig, Dmitry Torokhov,
Rob Herring, Krzysztof Kozlowski, -, Henrik Rydberg
Cc: linux-arm-kernel, linux-input, devicetree, linux-kernel,
Sasha Finkelstein
In-Reply-To: <20230223-z2-for-ml-v1-0-028f2b85dc15@gmail.com>
From: Sasha Finkelstein <fnkl.kernel@gmail.com>
Adds device tree entries for the touchbar digitizer
Signed-off-by: Sasha Finkelstein <fnkl.kernel@gmail.com>
---
arch/arm64/boot/dts/apple/t8103-j293.dts | 20 ++++++++++++++++++++
arch/arm64/boot/dts/apple/t8103.dtsi | 12 ++++++++++++
2 files changed, 32 insertions(+)
diff --git a/arch/arm64/boot/dts/apple/t8103-j293.dts b/arch/arm64/boot/dts/apple/t8103-j293.dts
index 151074109a11..c369493b1c06 100644
--- a/arch/arm64/boot/dts/apple/t8103-j293.dts
+++ b/arch/arm64/boot/dts/apple/t8103-j293.dts
@@ -15,6 +15,9 @@
/ {
compatible = "apple,j293", "apple,t8103", "apple,arm-platform";
model = "Apple MacBook Pro (13-inch, M1, 2020)";
+ aliases {
+ touchbar0 = &touchbar0;
+ };
};
&bluetooth0 {
@@ -25,6 +28,23 @@ &wifi0 {
brcm,board-type = "apple,honshu";
};
+&spi0 {
+ status = "okay";
+
+ touchbar0: touchbar@0 {
+ compatible = "apple,z2-touchscreen";
+ reg = <0>;
+ spi-max-frequency = <11500000>;
+ reset-gpios = <&pinctrl_ap 139 0>;
+ cs-gpios = <&pinctrl_ap 109 0>;
+ interrupts-extended = <&pinctrl_ap 194 IRQ_TYPE_EDGE_FALLING>;
+ firmware-name = "apple/dfrmtfw-j293.bin";
+ touchscreen-size-x = <23045>;
+ touchscreen-size-y = <640>;
+ apple,z2-device-name = "MacBookPro17,1 Touch Bar";
+ };
+};
+
/*
* Remove unused PCIe ports and disable the associated DARTs.
*/
diff --git a/arch/arm64/boot/dts/apple/t8103.dtsi b/arch/arm64/boot/dts/apple/t8103.dtsi
index 9859219699f4..70d3183f72bf 100644
--- a/arch/arm64/boot/dts/apple/t8103.dtsi
+++ b/arch/arm64/boot/dts/apple/t8103.dtsi
@@ -432,6 +432,18 @@ i2c4: i2c@235020000 {
status = "disabled"; /* only used in J293 */
};
+ spi0: spi@235100000 {
+ compatible = "apple,t8103-spi", "apple,spi";
+ reg = <0x2 0x35100000 0x0 0x4000>;
+ interrupt-parent = <&aic>;
+ interrupts = <AIC_IRQ 614 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&clk_200m>;
+ power-domains = <&ps_spi0>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ status = "disabled"; /* only used in J293 */
+ };
+
serial0: serial@235200000 {
compatible = "apple,s5l-uart";
reg = <0x2 0x35200000 0x0 0x1000>;
--
Git-137.1)
^ permalink raw reply related
* [PATCH RFC 0/4] Driver for Apple Z2 touchscreens.
From: Sasha Finkelstein via B4 Relay @ 2023-02-24 10:20 UTC (permalink / raw)
To: Hector Martin, Sven Peter, Alyssa Rosenzweig, Dmitry Torokhov,
Rob Herring, Krzysztof Kozlowski, -, Henrik Rydberg
Cc: linux-arm-kernel, linux-input, devicetree, linux-kernel,
Sasha Finkelstein
Hi.
This series adds support for Apple touchscreens using the Z2 protocol.
Those are used as the primary touchscreen on mobile Apple devices, and for the
touchbar on laptops using the M-series chips. (T1/T2 laptops have a coprocessor
in charge of speaking Z2 to the touchbar).
Sending this as a RFC for now, since this series requires the SPI controller
support which is not upstream yet:
https://lore.kernel.org/all/20211212034726.26306-1-marcan@marcan.st/
Signed-off-by: Sasha Finkelstein <fnkl.kernel@gmail.com>
---
Sasha Finkelstein (4):
dt-bindings: input: touchscreen: Add Z2 controller bindings.
input: apple_z2: Add a driver for Apple Z2 touchscreens
arm64: dts: apple: t8103: Add touchbar bindings
MAINTAINERS: Add entries for Apple Z2 touchscreen driver
.../input/touchscreen/apple,z2-touchscreen.yaml | 81 ++++
MAINTAINERS | 2 +
arch/arm64/boot/dts/apple/t8103-j293.dts | 20 +
arch/arm64/boot/dts/apple/t8103.dtsi | 12 +
drivers/input/touchscreen/Kconfig | 13 +
drivers/input/touchscreen/Makefile | 1 +
drivers/input/touchscreen/apple_z2.c | 465 +++++++++++++++++++++
7 files changed, 594 insertions(+)
---
base-commit: c9c3395d5e3dcc6daee66c6908354d47bf98cb0c
change-id: 20230223-z2-for-ml-18fb5246f4b8
Best regards,
--
Sasha Finkelstein <fnkl.kernel@gmail.com>
^ permalink raw reply
* [PATCH RFC 1/4] dt-bindings: input: touchscreen: Add Z2 controller bindings.
From: Sasha Finkelstein via B4 Relay @ 2023-02-24 10:20 UTC (permalink / raw)
To: Hector Martin, Sven Peter, Alyssa Rosenzweig, Dmitry Torokhov,
Rob Herring, Krzysztof Kozlowski, -, Henrik Rydberg
Cc: linux-arm-kernel, linux-input, devicetree, linux-kernel,
Sasha Finkelstein
In-Reply-To: <20230223-z2-for-ml-v1-0-028f2b85dc15@gmail.com>
From: Sasha Finkelstein <fnkl.kernel@gmail.com>
Add bindings for touchscreen controllers attached using the Z2 protocol.
Those are present in most Apple devices.
Signed-off-by: Sasha Finkelstein <fnkl.kernel@gmail.com>
---
.../input/touchscreen/apple,z2-touchscreen.yaml | 81 ++++++++++++++++++++++
1 file changed, 81 insertions(+)
diff --git a/Documentation/devicetree/bindings/input/touchscreen/apple,z2-touchscreen.yaml b/Documentation/devicetree/bindings/input/touchscreen/apple,z2-touchscreen.yaml
new file mode 100644
index 000000000000..695594494b1e
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/touchscreen/apple,z2-touchscreen.yaml
@@ -0,0 +1,81 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/input/touchscreen/apple,z2-touchscreen.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Apple touchscreens attached using the Z2 protocol.
+
+maintainers:
+ - asahi@lists.linux.dev
+ - Sasha Finkelstein <fnkl.kernel@gmail.com>
+
+description: A series of touschscreen controllers used in Apple products.
+
+allOf:
+ - $ref: touchscreen.yaml#
+ - $ref: /schemas/spi/spi-peripheral-props.yaml#
+
+properties:
+ compatible:
+ const: apple,z2-touchscreen
+
+ reg:
+ maxItems: 1
+
+ interrupts-extended:
+ maxItems: 1
+
+ reset-gpios:
+ maxItems: 1
+
+ cs-gpios:
+ maxItems: 1
+
+ firmware-name:
+ maxItems: 1
+
+ apple,z2-device-name:
+ description: The name to be used for the input device
+ $ref: /schemas/types.yaml#/definitions/string
+
+ touchscreen-size-x: true
+ touchscreen-size-y: true
+ spi-max-frequency: true
+
+required:
+ - compatible
+ - interrupts-extended
+ - reset-gpios
+ - cs-gpios
+ - firmware-name
+ - apple,z2-device-name
+ - touchscreen-size-x
+ - touchscreen-size-y
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/gpio/gpio.h>
+ #include <dt-bindings/interrupt-controller/irq.h>
+
+ spi {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ touchscreen@0 {
+ compatible = "apple,z2-touchscreen";
+ reg = <0>;
+ spi-max-frequency = <11500000>;
+ reset-gpios = <&pinctrl_ap 139 0>;
+ cs-gpios = <&pinctrl_ap 109 0>;
+ interrupts-extended = <&pinctrl_ap 194 IRQ_TYPE_EDGE_FALLING>;
+ firmware-name = "apple/dfrmtfw-j293.bin";
+ touchscreen-size-x = <23045>;
+ touchscreen-size-y = <640>;
+ apple,z2-device-name = "MacBookPro17,1 Touch Bar";
+ };
+ };
+
+...
--
Git-137.1)
^ permalink raw reply related
* [PATCH RFC 4/4] MAINTAINERS: Add entries for Apple Z2 touchscreen driver
From: Sasha Finkelstein via B4 Relay @ 2023-02-24 10:20 UTC (permalink / raw)
To: Hector Martin, Sven Peter, Alyssa Rosenzweig, Dmitry Torokhov,
Rob Herring, Krzysztof Kozlowski, -, Henrik Rydberg
Cc: linux-arm-kernel, linux-input, devicetree, linux-kernel,
Sasha Finkelstein
In-Reply-To: <20230223-z2-for-ml-v1-0-028f2b85dc15@gmail.com>
From: Sasha Finkelstein <fnkl.kernel@gmail.com>
Add the MAINTAINERS entries for the driver
Signed-off-by: Sasha Finkelstein <fnkl.kernel@gmail.com>
---
MAINTAINERS | 2 ++
1 file changed, 2 insertions(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index 135d93368d36..12811aa1bd01 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1958,6 +1958,7 @@ F: Documentation/devicetree/bindings/clock/apple,nco.yaml
F: Documentation/devicetree/bindings/cpufreq/apple,cluster-cpufreq.yaml
F: Documentation/devicetree/bindings/dma/apple,admac.yaml
F: Documentation/devicetree/bindings/i2c/apple,i2c.yaml
+F: Documentation/devicetree/bindings/input/touchscreen/apple,z2-touchscreen.yaml
F: Documentation/devicetree/bindings/interrupt-controller/apple,*
F: Documentation/devicetree/bindings/iommu/apple,dart.yaml
F: Documentation/devicetree/bindings/iommu/apple,sart.yaml
@@ -1976,6 +1977,7 @@ F: drivers/cpufreq/apple-soc-cpufreq.c
F: drivers/dma/apple-admac.c
F: drivers/i2c/busses/i2c-pasemi-core.c
F: drivers/i2c/busses/i2c-pasemi-platform.c
+F: drivers/input/touchscreen/apple_z2.c
F: drivers/iommu/apple-dart.c
F: drivers/iommu/io-pgtable-dart.c
F: drivers/irqchip/irq-apple-aic.c
--
Git-137.1)
^ permalink raw reply related
* [PATCH] input: keyboard: adp5588: make use of devm_regulator_get_enable()
From: Nuno Sá @ 2023-02-24 9:20 UTC (permalink / raw)
To: linux-input; +Cc: Dmitry Torokhov, Michael Hennerich, Nuno Sá
Using devm_regulator_get_enable() allow us to save some boiler plate
code.
No functional changes intended...
Signed-off-by: Nuno Sá <nuno.sa@analog.com>
---
drivers/input/keyboard/adp5588-keys.c | 21 +++------------------
1 file changed, 3 insertions(+), 18 deletions(-)
diff --git a/drivers/input/keyboard/adp5588-keys.c b/drivers/input/keyboard/adp5588-keys.c
index 72ae5ce72956..a9138810f8ef 100644
--- a/drivers/input/keyboard/adp5588-keys.c
+++ b/drivers/input/keyboard/adp5588-keys.c
@@ -713,17 +713,11 @@ static int adp5588_fw_parse(struct adp5588_kpad *kpad)
return 0;
}
-static void adp5588_disable_regulator(void *reg)
-{
- regulator_disable(reg);
-}
-
static int adp5588_probe(struct i2c_client *client)
{
struct adp5588_kpad *kpad;
struct input_dev *input;
struct gpio_desc *gpio;
- struct regulator *vcc;
unsigned int revid;
int ret;
int error;
@@ -749,18 +743,9 @@ static int adp5588_probe(struct i2c_client *client)
if (error)
return error;
- vcc = devm_regulator_get(&client->dev, "vcc");
- if (IS_ERR(vcc))
- return PTR_ERR(vcc);
-
- error = regulator_enable(vcc);
- if (error)
- return error;
-
- error = devm_add_action_or_reset(&client->dev,
- adp5588_disable_regulator, vcc);
- if (error)
- return error;
+ ret = devm_regulator_get_enable(&client->dev, "vcc");
+ if (ret)
+ return ret;
gpio = devm_gpiod_get_optional(&client->dev, "reset", GPIOD_OUT_HIGH);
if (IS_ERR(gpio))
--
2.39.2
^ permalink raw reply related
* Re: [PATCH] HID: usbhid: enable remote wakeup for mice
From: Michael Wu @ 2023-02-24 7:02 UTC (permalink / raw)
To: Greg KH
Cc: jikos, benjamin.tissoires, linux-usb, linux-input, linux-kernel,
mario.limonciello, richard.gong
In-Reply-To: <Y/dMq2KKYfdMdrjh@kroah.com>
Dear Greg：
On 2/23/2023 7:23 PM, Greg KH wrote:
> On Thu, Feb 23, 2023 at 07:18:12PM +0800, Michael Wu wrote:
>> Dear Greg,
>>
>> On 2/22/2023 2:04 PM, Greg KH wrote:
>>> On Wed, Feb 22, 2023 at 09:39:44AM +0800, Michael Wu wrote:
>>>> This patch fixes a problem that USB mouse can't wake up the device that
>>>> enters standby.
>>>
>>> This not a problem, it is that way by design.
>>
>> I got it, maybe it's a little problem to say that.
>
> It is.
>
>>>> At present, the kernel only checks whether certain USB manufacturers
>>>> support wake-up, which will easily cause inconvenience to the
>>>> development work of other manufacturers and add unnecessary work to the
>>>> maintenance of kernel.
>>>>
>>>> The USB protocol supports judging whether a usb supports the wake-up
>>>> function, so it should be more reasonable to add a wake-up source by
>>>> directly checking the settings from the USB protocol.
>>>
>>> But you do not do that in this patch, why not?
>>
>> I just want to explain the background of my patch, to prove we could use a
>> similar way to avoid such a "disturbing" situation.
>> To reduce the influence, my patch enables remote wakeup for USB mouse
>> devices refer to what keyboard do.
>
> Keyboards are not mice :)
Sorry, What I wanted to say is that we registered the mouse wake-up
source by referring to the practice of the keyboard.
>
>>>> There was a similar issue on the keyboard before, which was fixed by
>>>> this patch (3d61510f4eca), but now the problem happened on the mouse.
>>>> This patch uses a similar idea to fix this problem.
>>>>
>>>> Signed-off-by: Michael Wu <michael@allwinnertech.com>
>>>> ---
>>>> drivers/hid/usbhid/hid-core.c | 8 ++++++++
>>>> drivers/hid/usbhid/usbmouse.c | 1 +
>>>> 2 files changed, 9 insertions(+)
>>>>
>>>> diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c
>>>> index be4c731aaa65..d3a6755cca09 100644
>>>> --- a/drivers/hid/usbhid/hid-core.c
>>>> +++ b/drivers/hid/usbhid/hid-core.c
>>>> @@ -1189,6 +1189,14 @@ static int usbhid_start(struct hid_device *hid)
>>>> device_set_wakeup_enable(&dev->dev, 1);
>>>> }
>>>> + /**
>>>> + * NOTE: enable remote wakeup by default for all mouse devices
>>>> + * supporting the boot protocol.
>>>> + */
>>>> + if (interface->desc.bInterfaceSubClass == USB_INTERFACE_SUBCLASS_BOOT &&
>>>> + interface->desc.bInterfaceProtocol == USB_INTERFACE_PROTOCOL_MOUSE)
>>>> + device_set_wakeup_enable(&dev->dev, 1);
>>>
>>> Sorry, but we can not take this unless it is proven that this will work
>>> properly for all of these devices. Other operating systems do not do
>>> this last I checked, so there will be problems.
>>
>> As Mario Limonciello says, they has confirmed that the Microsoft Windows
>> does set a similar policy as well. Can we talk about more in this topic: why
>> does Linux not support it?
>> Of course, if you have other great idea, I will appreciate that if we can
>> have some further discussion.
>
> You need to provide some sort of "proof" that this has been heavily
> tested on a huge range of devices before we can change this.
>
> When this was first implemented, Windows did not work this way and many
> devices on the market were broken if this were to be enabled. I'm sure
> the mailing list archives from 20+ years ago have many more details,
> please dig around there for specifics.
>
> If you feel strongly that this is the way forward, why not do it in
> userspace today for your systems as part of testing this out? It should
> not require a kernel change, right?
Thanks for your advises. I'm clear now. I will try it in userspace.
>
> thanks,
>
> greg k-h
--
Regards,
Michael Wu
^ permalink raw reply
* [git pull] Input updates for v6.3-rc0
From: Dmitry Torokhov @ 2023-02-24 6:44 UTC (permalink / raw)
To: Linus Torvalds; +Cc: linux-kernel, linux-input
Hi Linus,
Please pull from:
git://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git tags/input-for-v6.3-rc0
to receive updates for the input subsystem. You will get:
- a set of tweaks to iqs269a touch controller driver
- a fix for ads7846 driver to properly handle 7845 chip
- cap11xx driver will support cap1203, cap1293 and cap1298 models
- xpad driver will support 8BitDo Pro 2 Wired Controller
- input drivers have been switched to DEFINE_SIMPLE_DEV_PM_OPS()
and pm_sleep_ptr()
- other miscellaneous fixes and tweaks
Note that you will get a merge conflict with ucb1400_ts driver that was
removed by Arnd.
Changelog:
---------
Arnd Bergmann (1):
Input: ipaq-micro-ts - fix DEFINE_SIMPLE_DEV_PM_OPS typo
Dario Binacchi (1):
Input: edt-ft5x06 - fix typo in a comment
Dmitry Torokhov (1):
Input: exc3000 - properly stop timer on shutdown
Jeff LaBundy (7):
Input: iqs269a - drop unused device node references
Input: iqs269a - increase interrupt handler return delay
Input: iqs269a - configure device with a single block write
Input: iqs269a - do not poll during suspend or resume
Input: iqs269a - do not poll during ATI
Input: iqs626a - drop unused device node references
dt-bindings: input: iqs626a: Redefine trackpad property types
Jiri Valek - 2N (2):
dt-bindings: input: microchip,cap11xx: add cap1203, cap1293 and cap1298
Input: cap11xx - add support for cap1203, cap1293 and cap1298
John Butler (1):
Input: xpad - add 8BitDo Pro 2 Wired Controller support
Jonathan Cameron (85):
Input: 88pm860x - switch to DEFINE_SIMPLE_DEV_PM_OPS() and pm_sleep_ptr()
Input: da7280 - switch to DEFINE_SIMPLE_DEV_PM_OPS() and pm_sleep_ptr()
Input: drv260x - switch to DEFINE_SIMPLE_DEV_PM_OPS() and pm_sleep_ptr()
Input: drv2665 - switch to DEFINE_SIMPLE_DEV_PM_OPS() and pm_sleep_ptr()
Input: drv2667 - switch to DEFINE_SIMPLE_DEV_PM_OPS() and pm_sleep_ptr()
Input: e3x0-button - switch to DEFINE_SIMPLE_DEV_PM_OPS() and pm_sleep_ptr()
Input: gpio-vibra - switch to DEFINE_SIMPLE_DEV_PM_OPS() and pm_sleep_ptr()
Input: iqs269a - switch to DEFINE_SIMPLE_DEV_PM_OPS() and pm_sleep_ptr()
Input: iqs626a - switch to DEFINE_SIMPLE_DEV_PM_OPS() and pm_sleep_ptr()
Input: kxtj9 - switch to DEFINE_SIMPLE_DEV_PM_OPS() and pm_sleep_ptr()
Input: max77693-haptic - switch to DEFINE_SIMPLE_DEV_PM_OPS() and pm_sleep_ptr()
Input: max8925_onkey - switch to DEFINE_SIMPLE_DEV_PM_OPS() and pm_sleep_ptr()
Input: max8997_haptic - switch to DEFINE_SIMPLE_DEV_PM_OPS() and pm_sleep_ptr()
Input: palmas-pwrbutton - switch to DEFINE_SIMPLE_DEV_PM_OPS() and pm_sleep_ptr()
Input: pcfg8574_keypad - switch to DEFINE_SIMPLE_DEV_PM_OPS() and pm_sleep_ptr()
Input: pm8941-pwrkey - switch to DEFINE_SIMPLE_DEV_PM_OPS() and pm_sleep_ptr()
Input: pm8xxx-vibrator - switch to DEFINE_SIMPLE_DEV_PM_OPS() and pm_sleep_ptr()
Input: pmic8xxx-pwrkey - switch to DEFINE_SIMPLE_DEV_PM_OPS() and pm_sleep_ptr()
Input: pwm-beeper - switch to DEFINE_SIMPLE_DEV_PM_OPS() and pm_sleep_ptr()
Input: pwm-vibra - switch to DEFINE_SIMPLE_DEV_PM_OPS() and pm_sleep_ptr()
Input: regulator-haptic - switch to DEFINE_SIMPLE_DEV_PM_OPS() and pm_sleep_ptr()
Input: rotary-encoder - switch to DEFINE_SIMPLE_DEV_PM_OPS() and pm_sleep_ptr()
Input: stpmic1_onekey - switch to DEFINE_SIMPLE_DEV_PM_OPS() and pm_sleep_ptr()
Input: twl4030-vibra - switch to DEFINE_SIMPLE_DEV_PM_OPS() and pm_sleep_ptr()
Input: twl6030-vibra - switch to DEFINE_SIMPLE_DEV_PM_OPS() and pm_sleep_ptr()
Input: elan_i2c_core - switch to DEFINE_SIMPLE_DEV_PM_OPS() and pm_sleep_ptr()
Input: navpoint - switch to DEFINE_SIMPLE_DEV_PM_OPS() and pm_sleep_ptr()
Input: synaptics_i2c - switch to DEFINE_SIMPLE_DEV_PM_OPS() and pm_sleep_ptr()
Input: ambakmi - switch to DEFINE_SIMPLE_DEV_PM_OPS() and pm_sleep_ptr()
Input: ad7877 - switch to DEFINE_SIMPLE_DEV_PM_OPS() and pm_sleep_ptr()
Input: ads7846 - switch to DEFINE_SIMPLE_DEV_PM_OPS() and pm_sleep_ptr()
Input: ar1021 - switch to DEFINE_SIMPLE_DEV_PM_OPS() and pm_sleep_ptr()
Input: atmel_mxt_ts - switch to DEFINE_SIMPLE_DEV_PM_OPS() and pm_sleep_ptr()
Input: auo-pixcir-ts - switch to DEFINE_SIMPLE_DEV_PM_OPS() and pm_sleep_ptr()
Input: bu21013_ts - switch to DEFINE_SIMPLE_DEV_PM_OPS() and pm_sleep_ptr()
Input: bu21029 - switch to DEFINE_SIMPLE_DEV_PM_OPS() and pm_sleep_ptr()
Input: chipone_in8318 - switch to DEFINE_SIMPLE_DEV_PM_OPS() and pm_sleep_ptr()
Input: chipone_icn8505 - switch to DEFINE_SIMPLE_DEV_PM_OPS() and pm_sleep_ptr()
Input: cy8ctma140 - switch to DEFINE_SIMPLE_DEV_PM_OPS() and pm_sleep_ptr()
Input: cy8ctmg110_ts - switch to DEFINE_SIMPLE_DEV_PM_OPS() and pm_sleep_ptr()
Input: edt-ft5x06 - switch to DEFINE_SIMPLE_DEV_PM_OPS() and pm_sleep_ptr()
Input: eeti_ts - switch to DEFINE_SIMPLE_DEV_PM_OPS() and pm_sleep_ptr()
Input: egalax_ts - switch to DEFINE_SIMPLE_DEV_PM_OPS() and pm_sleep_ptr()
input: ektf2127 - switch to DEFINE_SIMPLE_DEV_PM_OPS() and pm_sleep_ptr()
Input: elants_i2c - switch to DEFINE_SIMPLE_DEV_PM_OPS() and pm_sleep_ptr()
Input: goodix - switch to DEFINE_SIMPLE_DEV_PM_OPS() and pm_sleep_ptr()
Input: hideep - switch to DEFINE_SIMPLE_DEV_PM_OPS() and pm_sleep_ptr()
Input: ilitek_ts_i2c - switch to DEFINE_SIMPLE_DEV_PM_OPS() and pm_sleep_ptr()
Input: imagis - switch to DEFINE_SIMPLE_DEV_PM_OPS() and pm_sleep_ptr()
Input: imx6ul_tsc - switch to DEFINE_SIMPLE_DEV_PM_OPS() and pm_sleep_ptr()
Input: iqs5xx - switch to DEFINE_SIMPLE_DEV_PM_OPS() and pm_sleep_ptr()
Input: mcs5000_ts - switch to DEFINE_SIMPLE_DEV_PM_OPS() and pm_sleep_ptr()
Input: melfas_mip4 - switch to DEFINE_SIMPLE_DEV_PM_OPS() and pm_sleep_ptr()
input: migor_ts - switch to DEFINE_SIMPLE_DEV_PM_OPS() and pm_sleep_ptr()
Input: mms114 - switch to DEFINE_SIMPLE_DEV_PM_OPS() and pm_sleep_ptr()
Input: msg2638 - switch to DEFINE_SIMPLE_DEV_PM_OPS() and pm_sleep_ptr()
Input: pixcir_i2c_ts - switch to DEFINE_SIMPLE_DEV_PM_OPS() and pm_sleep_ptr()
Input: raydium_i2c_ts - switch to DEFINE_SIMPLE_DEV_PM_OPS() and pm_sleep_ptr()
Input: silead - switch to DEFINE_SIMPLE_DEV_PM_OPS() and pm_sleep_ptr()
Input: st1232 - switch to DEFINE_SIMPLE_DEV_PM_OPS() and pm_sleep_ptr()
Input: surface3_spi - switch to DEFINE_SIMPLE_DEV_PM_OPS() and pm_sleep_ptr()
Input: ti_am335x_tsc - switch to DEFINE_SIMPLE_DEV_PM_OPS() and pm_sleep_ptr()
Input: ucb1400_ts - switch to DEFINE_SIMPLE_DEV_PM_OPS() and pm_sleep_ptr()
Input: wacom_i2c - switch to DEFINE_SIMPLE_DEV_PM_OPS() and pm_sleep_ptr()
Input: wdt87xx_i2c - switch to DEFINE_SIMPLE_DEV_PM_OPS() and pm_sleep_ptr()
Input: wm97xx - switch to DEFINE_SIMPLE_DEV_PM_OPS() and pm_sleep_ptr()
Input: zforce_ts - switch to DEFINE_SIMPLE_DEV_PM_OPS() and pm_sleep_ptr()
Input: zinitix - switch to DEFINE_SIMPLE_DEV_PM_OPS() and pm_sleep_ptr()
Input: ipaq-micro-ts - use DEFINE_SIMPLE_DEV_PM_OPS() and pm_sleep_ptr()
Input: cyapa - switch to SYSTEM_SLEEP/RUNTIME_PM_OPS() and pm_ptr()
Input: axp20x-pek - switch to SYSTEM_SLEEP_PM_OPS() and pm_sleep_ptr()
Input: samsung-keypad - switch to pm_ptr() and SYSTEM_SLEEP/RUNTIME_PM_OPS()
Input: s6sy761 - switch to SYSTEM_SLEEP_/RUNTIME_PM_OPS() and pm_ptr()
Input: rmi4 - switch to SYSTEM_SLEEP/RUNTIME_PM_OPS() and pm_ptr()
Input: stmfts - switch to SYSTEM_SLEEP_/RUNTIME_PM_OPS() and pm_ptr()
Input: ad714x - unify dev_pm_ops using EXPORT_SIMPLE_DEV_PM_OPS()
Input: adxl34x - unify dev_pm_ops using EXPORT_SIMPLE_DEV_PM_OPS()
Input: tsc200x - use EXPORT_GPL_SIMPLE_DEV_PM_OPS()
Input: cyttsp4 - use EXPORT_GPL_RUNTIME_DEV_PM_OPS()
Input: cyttsp - use EXPORT_GPL_SIMPLE_DEV_PM_OPS()
Input: applespi - use pm_sleep_ptr() and SYSTEM_SLEEP_PM_OPS()
Input: omap4-keyad - use pm_ptr() and RUNTIME_DEV_PM_OPS()
Input: Use pm_sleep_ptr() to avoid need for ifdef CONFIG_PM_SLEEP
Input: cma3000 - use pm_sleep_ptr() to allow removal of ifdef CONFIG_PM guards
Input: wistron_btns - use pm_sleep_ptr() to allow removal of ifdef CONFIG_PM guards
Krzysztof Kozlowski (1):
dt-bindings: input: touchscreen: st,stmfts: convert to dtschema
Luca Ellero (3):
Input: ads7846 - don't report pressure for ads7845
Input: ads7846 - always set last command to PWRDOWN
Input: ads7846 - don't check penirq immediately for 7845
Randy Dunlap (1):
Input: pmic8xxx-keypad - fix a Kconfig spelling mistake & hyphenation
Rayyan Ansari (1):
Input: synaptics-rmi4 - fix SPI device ID
Richard Kjerstadius (1):
Input: cyttsp5 - fix bitmask for touch buttons
joewu (吳仲振) (1):
Input: cros_ec_keyb - add 3 buttons for monitor function
ye xingchen (7):
Input: altera_ps2 - use devm_platform_get_and_ioremap_resource()
Input: apbps2 - use devm_platform_get_and_ioremap_resource()
Input: arc_ps2 - use devm_platform_get_and_ioremap_resource()
Input: olpc_apsp - use devm_platform_get_and_ioremap_resource()
Input: spear-keyboard - use devm_platform_get_and_ioremap_resource()
Input: st-keyscan - use devm_platform_get_and_ioremap_resource()
Input: tegra-kbc - use devm_platform_get_and_ioremap_resource()
Diffstat:
--------
.../devicetree/bindings/input/iqs626a.yaml | 94 ++++--
.../bindings/input/microchip,cap11xx.yaml | 3 +
.../bindings/input/touchscreen/st,stmfts.txt | 41 ---
.../bindings/input/touchscreen/st,stmfts.yaml | 72 +++++
drivers/input/input.c | 7 +-
drivers/input/joystick/xpad.c | 2 +
drivers/input/keyboard/Kconfig | 2 +-
drivers/input/keyboard/applespi.c | 10 +-
drivers/input/keyboard/cap11xx.c | 19 +-
drivers/input/keyboard/cros_ec_keyb.c | 15 +
drivers/input/keyboard/omap4-keypad.c | 6 +-
drivers/input/keyboard/samsung-keypad.c | 12 +-
drivers/input/keyboard/spear-keyboard.c | 4 +-
drivers/input/keyboard/st-keyscan.c | 4 +-
drivers/input/keyboard/tegra-kbc.c | 4 +-
drivers/input/misc/88pm860x_onkey.c | 9 +-
drivers/input/misc/ad714x-i2c.c | 14 +-
drivers/input/misc/ad714x-spi.c | 14 +-
drivers/input/misc/ad714x.c | 12 +-
drivers/input/misc/ad714x.h | 4 +-
drivers/input/misc/adxl34x-i2c.c | 25 +-
drivers/input/misc/adxl34x-spi.c | 25 +-
drivers/input/misc/adxl34x.c | 16 +-
drivers/input/misc/adxl34x.h | 4 +-
drivers/input/misc/axp20x-pek.c | 12 +-
drivers/input/misc/cma3000_d0x_i2c.c | 6 +-
drivers/input/misc/da7280.c | 8 +-
drivers/input/misc/drv260x.c | 8 +-
drivers/input/misc/drv2665.c | 8 +-
drivers/input/misc/drv2667.c | 8 +-
drivers/input/misc/e3x0-button.c | 10 +-
drivers/input/misc/gpio-vibra.c | 10 +-
drivers/input/misc/iqs269a.c | 335 ++++++++-------------
drivers/input/misc/iqs626a.c | 164 +++++-----
drivers/input/misc/kxtj9.c | 8 +-
drivers/input/misc/max77693-haptic.c | 11 +-
drivers/input/misc/max8925_onkey.c | 9 +-
drivers/input/misc/max8997_haptic.c | 7 +-
drivers/input/misc/palmas-pwrbutton.c | 10 +-
drivers/input/misc/pcf8574_keypad.c | 16 +-
drivers/input/misc/pm8941-pwrkey.c | 10 +-
drivers/input/misc/pm8xxx-vibrator.c | 6 +-
drivers/input/misc/pmic8xxx-pwrkey.c | 8 +-
drivers/input/misc/pwm-beeper.c | 10 +-
drivers/input/misc/pwm-vibra.c | 10 +-
drivers/input/misc/regulator-haptic.c | 8 +-
drivers/input/misc/rotary_encoder.c | 10 +-
drivers/input/misc/stpmic1_onkey.c | 12 +-
drivers/input/misc/twl4030-vibra.c | 10 +-
drivers/input/misc/twl6040-vibra.c | 7 +-
drivers/input/misc/wistron_btns.c | 6 +-
drivers/input/mouse/cyapa.c | 14 +-
drivers/input/mouse/elan_i2c_core.c | 8 +-
drivers/input/mouse/navpoint.c | 9 +-
drivers/input/mouse/synaptics_i2c.c | 10 +-
drivers/input/rmi4/rmi_i2c.c | 11 +-
drivers/input/rmi4/rmi_smbus.c | 15 +-
drivers/input/rmi4/rmi_spi.c | 13 +-
drivers/input/serio/altera_ps2.c | 4 +-
drivers/input/serio/ambakmi.c | 6 +-
drivers/input/serio/apbps2.c | 4 +-
drivers/input/serio/arc_ps2.c | 4 +-
drivers/input/serio/olpc_apsp.c | 4 +-
drivers/input/touchscreen/ad7877.c | 8 +-
drivers/input/touchscreen/ads7846.c | 31 +-
drivers/input/touchscreen/ar1021_i2c.c | 9 +-
drivers/input/touchscreen/atmel_mxt_ts.c | 8 +-
drivers/input/touchscreen/auo-pixcir-ts.c | 10 +-
drivers/input/touchscreen/bu21013_ts.c | 8 +-
drivers/input/touchscreen/bu21029_ts.c | 8 +-
drivers/input/touchscreen/chipone_icn8318.c | 6 +-
drivers/input/touchscreen/chipone_icn8505.c | 8 +-
drivers/input/touchscreen/cy8ctma140.c | 9 +-
drivers/input/touchscreen/cy8ctmg110_ts.c | 9 +-
drivers/input/touchscreen/cyttsp4_core.c | 9 +-
drivers/input/touchscreen/cyttsp4_i2c.c | 2 +-
drivers/input/touchscreen/cyttsp4_spi.c | 2 +-
drivers/input/touchscreen/cyttsp5.c | 2 +-
drivers/input/touchscreen/cyttsp_core.c | 7 +-
drivers/input/touchscreen/cyttsp_i2c.c | 2 +-
drivers/input/touchscreen/cyttsp_spi.c | 2 +-
drivers/input/touchscreen/edt-ft5x06.c | 12 +-
drivers/input/touchscreen/eeti_ts.c | 8 +-
drivers/input/touchscreen/egalax_ts.c | 9 +-
drivers/input/touchscreen/ektf2127.c | 10 +-
drivers/input/touchscreen/elants_i2c.c | 10 +-
drivers/input/touchscreen/exc3000.c | 10 +
drivers/input/touchscreen/goodix.c | 8 +-
drivers/input/touchscreen/hideep.c | 8 +-
drivers/input/touchscreen/ilitek_ts_i2c.c | 8 +-
drivers/input/touchscreen/imagis.c | 8 +-
drivers/input/touchscreen/imx6ul_tsc.c | 10 +-
drivers/input/touchscreen/ipaq-micro-ts.c | 11 +-
drivers/input/touchscreen/iqs5xx.c | 8 +-
drivers/input/touchscreen/mcs5000_ts.c | 9 +-
drivers/input/touchscreen/melfas_mip4.c | 8 +-
drivers/input/touchscreen/migor_ts.c | 8 +-
drivers/input/touchscreen/mms114.c | 8 +-
drivers/input/touchscreen/msg2638.c | 8 +-
drivers/input/touchscreen/pixcir_i2c_ts.c | 10 +-
drivers/input/touchscreen/raydium_i2c_ts.c | 12 +-
drivers/input/touchscreen/s6sy761.c | 15 +-
drivers/input/touchscreen/silead.c | 8 +-
drivers/input/touchscreen/st1232.c | 10 +-
drivers/input/touchscreen/stmfts.c | 14 +-
drivers/input/touchscreen/surface3_spi.c | 12 +-
drivers/input/touchscreen/ti_am335x_tsc.c | 8 +-
drivers/input/touchscreen/tsc2004.c | 2 +-
drivers/input/touchscreen/tsc2005.c | 2 +-
drivers/input/touchscreen/tsc200x-core.c | 7 +-
drivers/input/touchscreen/wacom_i2c.c | 8 +-
drivers/input/touchscreen/wdt87xx_i2c.c | 8 +-
drivers/input/touchscreen/wm97xx-core.c | 10 +-
drivers/input/touchscreen/zforce_ts.c | 8 +-
drivers/input/touchscreen/zinitix.c | 8 +-
include/linux/platform_data/cros_ec_commands.h | 3 +
116 files changed, 810 insertions(+), 883 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/input/touchscreen/st,stmfts.txt
create mode 100644 Documentation/devicetree/bindings/input/touchscreen/st,stmfts.yaml
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH 4/5] USB: core: Add API to change the wireless_status
From: Alan Stern @ 2023-02-24 2:34 UTC (permalink / raw)
To: Bastien Nocera
Cc: linux-usb, linux-input, Greg Kroah-Hartman, Benjamin Tissoires,
Filipe Laíns, Nestor Lopez Casado
In-Reply-To: <ba9eec1f2f0d41e3af8b1e73e4c804074e429cc4.camel@hadess.net>
On Fri, Feb 24, 2023 at 12:04:12AM +0100, Bastien Nocera wrote:
> On Thu, 2023-02-23 at 12:07 -0500, Alan Stern wrote:
> > The refcounting in your patch guarantees that when the work function
> > runs, the interface structure will still exist. But refcounting does
> > not guarantee that the interface will still be registered in sysfs,
> > and
> > this can actually happen if the work is scheduled immediately before
> > the
> > interface is unregistered.
> >
> > So my question is: What will happen when sysfs_update_group(),
> > sysfs_notify(), and kobject_uevent() are called after the interface
> > has
> > been unregistered from sysfs? Maybe they will work okay -- I simply
> > don't know, and I wanted to find out whether you had considered the
> > issue.
>
> A long week-end started for me a couple of hours ago, but I wanted to
> dump my thoughts before either I forgot, or it took over my whole week-
> end ;)
>
> I had thought about the problem, and didn't think that sysfs files
> would get removed before the interface got released/unref'ed and
> usb_remove_sysfs_intf_files() got called.
>
> If the device gets removed from the device bus before it's released,
> then this patch should fix it:
> --- a/drivers/usb/core/message.c
> +++ b/drivers/usb/core/message.c
> @@ -1917,7 +1917,8 @@ static void __usb_wireless_status_intf(struct work_struct *ws)
> struct usb_interface *iface =
> container_of(ws, struct usb_interface, wireless_status_work);
>
> - usb_update_wireless_status_attr(iface);
> + if (intf->sysfs_files_created)
> + usb_update_wireless_status_attr(iface);
> usb_put_intf(iface); /* Undo _get_ in usb_set_wireless_status() */
>
> The callback would be a no-op if the device's sysfs is already
> unregistered, just unref'ing the reference it held.
>
> What do you think? I'll amend that into my patchset on Monday.
That's a good way to do it, but it does race with
usb_remove_sysfs_intf_files(). To prevent this race, you can protect
the test and function call with device_lock(iface->dev.parent) (that is,
lock the interface's parent usb_device).
Alan Stern
^ permalink raw reply
* Re: [PATCH 4/5] USB: core: Add API to change the wireless_status
From: Bastien Nocera @ 2023-02-23 23:04 UTC (permalink / raw)
To: Alan Stern
Cc: linux-usb, linux-input, Greg Kroah-Hartman, Benjamin Tissoires,
Filipe Laíns, Nestor Lopez Casado
In-Reply-To: <Y/edOwVwfHVkoq9j@rowland.harvard.edu>
On Thu, 2023-02-23 at 12:07 -0500, Alan Stern wrote:
> On Thu, Feb 23, 2023 at 05:51:48PM +0100, Bastien Nocera wrote:
> > On Thu, 2023-02-23 at 11:25 -0500, Alan Stern wrote:
> > > On Thu, Feb 23, 2023 at 05:17:13PM +0100, Bastien Nocera wrote:
> > > > On Thu, 2023-02-23 at 10:41 -0500, Alan Stern wrote:
> > > > > On Thu, Feb 23, 2023 at 02:24:51PM +0100, Bastien Nocera
> > > > > wrote:
> > > > > > Allow device specific drivers to change the wireless status
> > > > > > of
> > > > > > a
> > > > > > device.
> > > > > > This will allow user-space to know whether the device is
> > > > > > available,
> > > > > > whether or not specific USB interfaces can detect it.
> > > > > >
> > > > > > This can be used by wireless headsets with USB receivers to
> > > > > > propagate to
> > > > > > user-space whether or not the headset is turned on, so as
> > > > > > to
> > > > > > consider it
> > > > > > as unavailable, and not switch to it just because the
> > > > > > receiver
> > > > > > is
> > > > > > plugged in.
> > > > > >
> > > > > > Signed-off-by: Bastien Nocera <hadess@hadess.net>
> > > > > > ---
> > > > > > drivers/usb/core/message.c | 13 +++++++++++++
> > > > > > drivers/usb/core/usb.c | 24 ++++++++++++++++++++++++
> > > > > > include/linux/usb.h | 4 ++++
> > > > > > 3 files changed, 41 insertions(+)
> > > > > >
> > > > > > diff --git a/drivers/usb/core/message.c
> > > > > > b/drivers/usb/core/message.c
> > > > > > index 127fac1af676..d5c7749d515e 100644
> > > > > > --- a/drivers/usb/core/message.c
> > > > > > +++ b/drivers/usb/core/message.c
> > > > > > @@ -1908,6 +1908,18 @@ static void
> > > > > > __usb_queue_reset_device(struct
> > > > > > work_struct *ws)
> > > > > > usb_put_intf(iface); /* Undo _get_ in
> > > > > > usb_queue_reset_device() */
> > > > > > }
> > > > > >
> > > > > > +/*
> > > > > > + * Internal function to set the wireless_status sysfs
> > > > > > attribute
> > > > > > + * See usb_set_wireless_status() for more details
> > > > > > + */
> > > > > > +static void __usb_wireless_status_intf(struct work_struct
> > > > > > *ws)
> > > > > > +{
> > > > > > + struct usb_interface *iface =
> > > > > > + container_of(ws, struct usb_interface,
> > > > > > wireless_status_work);
> > > > > > +
> > > > > > + usb_update_wireless_status_attr(iface);
> > > > > > + usb_put_intf(iface); /* Undo _get_ in
> > > > > > usb_set_wireless_status() */
> > > > > > +}
> > > > >
> > > > > Have you thought about what will happen if this routine ends
> > > > > up
> > > > > running
> > > > > after the interface has been deleted?
> > > >
> > > > I believe that usb_release_interface() will only be called once
> > > > the
> > > > last reference to the interface is dropped, so bar any
> > > > refcounting
> > > > bugs, the interface should always exist when this function is
> > > > called.
> > >
> > > Yes, but what about the calls made by
> > > usb_update_wireless_status_attr():
> > > sysfs_update_group(), sysfs_notify(), and kobject_uevent()? Will
> > > they
> > > work properly when called for an object that has been
> > > unregistered
> > > from
> > > sysfs?
> >
> > Those calls are made before the last reference to the interface can
> > be
> > dropped by our own call to usb_put_intf(). So, in effect, the
> > interface
> > is kept alive until we're done with our work so we can't ever be
> > poking
> > at objects that disappeared.
> >
> > Am I missing something about the object model, or the refcounting
> > here?
>
> Yes, you are.
>
> There's a difference between object _existence_ and object
> _registration_. An object (kobject, struct device, whatever) exists
> for
> as long as its memory is still allocated and in use. This is what
> refcounting protects.
>
> An object is registered in sysfs during the time between calls to
> device_add() and device_del(). During this time it shows up as a
> directory under /sys, along with its various attribute files.
> Calling
> device_del() gets rid of all that -- James Bottomley calls it
> "removing
> an object from visibility". The object still exists, but it isn't
> part
> of sysfs any more.
>
> The refcounting in your patch guarantees that when the work function
> runs, the interface structure will still exist. But refcounting does
> not guarantee that the interface will still be registered in sysfs,
> and
> this can actually happen if the work is scheduled immediately before
> the
> interface is unregistered.
>
> So my question is: What will happen when sysfs_update_group(),
> sysfs_notify(), and kobject_uevent() are called after the interface
> has
> been unregistered from sysfs? Maybe they will work okay -- I simply
> don't know, and I wanted to find out whether you had considered the
> issue.
A long week-end started for me a couple of hours ago, but I wanted to
dump my thoughts before either I forgot, or it took over my whole week-
end ;)
I had thought about the problem, and didn't think that sysfs files
would get removed before the interface got released/unref'ed and
usb_remove_sysfs_intf_files() got called.
If the device gets removed from the device bus before it's released,
then this patch should fix it:
--- a/drivers/usb/core/message.c
+++ b/drivers/usb/core/message.c
@@ -1917,7 +1917,8 @@ static void __usb_wireless_status_intf(struct work_struct *ws)
struct usb_interface *iface =
container_of(ws, struct usb_interface, wireless_status_work);
- usb_update_wireless_status_attr(iface);
+ if (intf->sysfs_files_created)
+ usb_update_wireless_status_attr(iface);
usb_put_intf(iface); /* Undo _get_ in usb_set_wireless_status() */
The callback would be a no-op if the device's sysfs is already
unregistered, just unref'ing the reference it held.
What do you think? I'll amend that into my patchset on Monday.
Cheers
^ permalink raw reply
* Re: [PATCH v3 00/17] Self-encapsulate the thermal zone device structure
From: Daniel Lezcano @ 2023-02-23 22:56 UTC (permalink / raw)
To: rafael
Cc: linux-pm, linux-kernel, Zhang Rui, Len Brown, Damien Le Moal,
Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
NXP Linux Team, Jean Delvare, Guenter Roeck, Jonathan Cameron,
Lars-Peter Clausen, Chen-Yu Tsai, Jernej Skrabec, Samuel Holland,
Dmitry Torokhov, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Ido Schimmel, Petr Machata, Gregory Greenman,
Kalle Valo, Sebastian Reichel, Liam Girdwood, Mark Brown,
Miquel Raynal, Amit Kucheria, Florian Fainelli,
Broadcom internal kernel review list, Ray Jui, Scott Branden,
Markus Mayer, Support Opensource, Andy Gross, Bjorn Andersson,
Konrad Dybcio, Thara Gopinath, Niklas Söderlund,
Heiko Stuebner, Bartlomiej Zolnierkiewicz, Krzysztof Kozlowski,
Alim Akhtar, Orson Zhai, Baolin Wang, Chunyan Zhang,
Vasily Khoruzhick, Yangtao Li, Thierry Reding, Jonathan Hunter,
Talel Shenhar, Eduardo Valentin, Keerthy, Kunihiko Hayashi,
Masami Hiramatsu, Matthias Brugger, AngeloGioacchino Del Regno,
Stefan Wahren, Zheng Yongjun, Yang Li, Srinivas Pandruvada,
Daniel Golle, Balsam CHIHI, Mikko Perttunen, linux-acpi,
linux-ide, linux-arm-kernel, linux-hwmon, linux-iio, linux-sunxi,
linux-input, netdev, linux-wireless, linux-rpi-kernel,
linux-arm-msm, linux-renesas-soc, linux-rockchip,
linux-samsung-soc, linux-tegra, linux-omap, linux-mediatek
In-Reply-To: <20230223224844.3491251-1-daniel.lezcano@linaro.org>
On 23/02/2023 23:48, Daniel Lezcano wrote:
> The exported thermal headers expose the thermal core structure while those
> should be private to the framework. The initial idea was the thermal sensor
> drivers use the thermal zone device structure pointer to pass it around from
> the ops to the thermal framework API like a handler.
>
> Unfortunately, different drivers are using and abusing the internals of this
> structure to hook the associated struct device, read the internals values, take
> the lock, etc ...
>
> rn order to fix this situation, let's encapsulate the structure leaking the
> more in the different drivers: the thermal_zone_device structure.
>
> This series revisit the existing drivers using the thermal zone private
> structure internals to change the access to something else. For instance, the
> get_temp() ops is using the tz->dev to write a debug trace. Despite the trace
> is not helpful, we can check the return value for the get_temp() ops in the
> call site and show the message in this place.
>
> With this set of changes, the thermal_zone_device is almost self-encapsulated.
> As usual, the acpi driver needs a more complex changes, so that will come in a
> separate series along with the structure moved the private core headers.
>
> Changelog:
> - V3:
> - Collected more tags
> - Added missing changes for ->devdata in some drivers
> - Added a 'type' accessor
> - Replaced the 'type' to 'id' changes by the 'type' accessor
> - Used the 'type' accessor in the drivers
> - V2:
> - Collected tags
> - Added missing changes for ->devdata for the tsens driver
> - Renamed thermal_zone_device_get_data() to thermal_zone_priv()
> - Added stubs when CONFIG_THERMAL is not set
> - Dropped hwmon change where we remove the tz->lock usage
>
> Thank you all for your comments
The series has been blocked by gsmtp because the next patch has too many
Cc. I'll sort out this and resend.
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
^ permalink raw reply
* [PATCH v3 00/17] Self-encapsulate the thermal zone device structure
From: Daniel Lezcano @ 2023-02-23 22:48 UTC (permalink / raw)
To: rafael, daniel.lezcano
Cc: linux-pm, linux-kernel, Zhang Rui, Len Brown, Damien Le Moal,
Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
NXP Linux Team, Jean Delvare, Guenter Roeck, Jonathan Cameron,
Lars-Peter Clausen, Chen-Yu Tsai, Jernej Skrabec, Samuel Holland,
Dmitry Torokhov, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Ido Schimmel, Petr Machata, Gregory Greenman,
Kalle Valo, Sebastian Reichel, Liam Girdwood, Mark Brown,
Miquel Raynal, Amit Kucheria, Florian Fainelli,
Broadcom internal kernel review list, Ray Jui, Scott Branden,
Markus Mayer, Support Opensource, Andy Gross, Bjorn Andersson,
Konrad Dybcio, Thara Gopinath, Niklas Söderlund,
Heiko Stuebner, Bartlomiej Zolnierkiewicz, Krzysztof Kozlowski,
Alim Akhtar, Orson Zhai, Baolin Wang, Chunyan Zhang,
Vasily Khoruzhick, Yangtao Li, Thierry Reding, Jonathan Hunter,
Talel Shenhar, Eduardo Valentin, Keerthy, Kunihiko Hayashi,
Masami Hiramatsu, Matthias Brugger, AngeloGioacchino Del Regno,
Stefan Wahren, Zheng Yongjun, Yang Li, Srinivas Pandruvada,
Daniel Golle, Balsam CHIHI, Mikko Perttunen, linux-acpi,
linux-ide, linux-arm-kernel, linux-hwmon, linux-iio, linux-sunxi,
linux-input, netdev, linux-wireless, linux-rpi-kernel,
linux-arm-msm, linux-renesas-soc, linux-rockchip,
linux-samsung-soc, linux-tegra, linux-omap, linux-mediatek
The exported thermal headers expose the thermal core structure while those
should be private to the framework. The initial idea was the thermal sensor
drivers use the thermal zone device structure pointer to pass it around from
the ops to the thermal framework API like a handler.
Unfortunately, different drivers are using and abusing the internals of this
structure to hook the associated struct device, read the internals values, take
the lock, etc ...
rn order to fix this situation, let's encapsulate the structure leaking the
more in the different drivers: the thermal_zone_device structure.
This series revisit the existing drivers using the thermal zone private
structure internals to change the access to something else. For instance, the
get_temp() ops is using the tz->dev to write a debug trace. Despite the trace
is not helpful, we can check the return value for the get_temp() ops in the
call site and show the message in this place.
With this set of changes, the thermal_zone_device is almost self-encapsulated.
As usual, the acpi driver needs a more complex changes, so that will come in a
separate series along with the structure moved the private core headers.
Changelog:
- V3:
- Collected more tags
- Added missing changes for ->devdata in some drivers
- Added a 'type' accessor
- Replaced the 'type' to 'id' changes by the 'type' accessor
- Used the 'type' accessor in the drivers
- V2:
- Collected tags
- Added missing changes for ->devdata for the tsens driver
- Renamed thermal_zone_device_get_data() to thermal_zone_priv()
- Added stubs when CONFIG_THERMAL is not set
- Dropped hwmon change where we remove the tz->lock usage
Thank you all for your comments
Cc: "Rafael J. Wysocki" <rafael@kernel.org>
Cc: Zhang Rui <rui.zhang@intel.com>
Cc: Len Brown <lenb@kernel.org>
Cc: Damien Le Moal <damien.lemoal@opensource.wdc.com>
Cc: Shawn Guo <shawnguo@kernel.org>
Cc: Sascha Hauer <s.hauer@pengutronix.de>
Cc: Pengutronix Kernel Team <kernel@pengutronix.de>
Cc: Fabio Estevam <festevam@gmail.com>
Cc: NXP Linux Team <linux-imx@nxp.com>
Cc: Jean Delvare <jdelvare@suse.com>
Cc: Guenter Roeck <linux@roeck-us.net>
Cc: Jonathan Cameron <jic23@kernel.org>
Cc: Lars-Peter Clausen <lars@metafoo.de>
Cc: Chen-Yu Tsai <wens@csie.org>
Cc: Jernej Skrabec <jernej.skrabec@gmail.com>
Cc: Samuel Holland <samuel@sholland.org>
Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Eric Dumazet <edumazet@google.com>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: Paolo Abeni <pabeni@redhat.com>
Cc: Ido Schimmel <idosch@nvidia.com>
Cc: Petr Machata <petrm@nvidia.com>
Cc: Gregory Greenman <gregory.greenman@intel.com>
Cc: Kalle Valo <kvalo@kernel.org>
Cc: Sebastian Reichel <sre@kernel.org>
Cc: Liam Girdwood <lgirdwood@gmail.com>
Cc: Mark Brown <broonie@kernel.org>
Cc: Miquel Raynal <miquel.raynal@bootlin.com>
Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
Cc: Amit Kucheria <amitk@kernel.org>
Cc: Florian Fainelli <f.fainelli@gmail.com>
Cc: Broadcom internal kernel review list <bcm-kernel-feedback-list@broadcom.com>
Cc: Ray Jui <rjui@broadcom.com>
Cc: Scott Branden <sbranden@broadcom.com>
Cc: Markus Mayer <mmayer@broadcom.com>
Cc: Support Opensource <support.opensource@diasemi.com>
Cc: Andy Gross <agross@kernel.org>
Cc: Bjorn Andersson <andersson@kernel.org>
Cc: Konrad Dybcio <konrad.dybcio@linaro.org>
Cc: Thara Gopinath <thara.gopinath@gmail.com>
Cc: "Niklas Söderlund" <niklas.soderlund@ragnatech.se>
Cc: Heiko Stuebner <heiko@sntech.de>
Cc: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
Cc: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Cc: Alim Akhtar <alim.akhtar@samsung.com>
Cc: Orson Zhai <orsonzhai@gmail.com>
Cc: Baolin Wang <baolin.wang@linux.alibaba.com>
Cc: Chunyan Zhang <zhang.lyra@gmail.com>
Cc: Vasily Khoruzhick <anarsoul@gmail.com>
Cc: Yangtao Li <tiny.windzz@gmail.com>
Cc: Thierry Reding <thierry.reding@gmail.com>
Cc: Jonathan Hunter <jonathanh@nvidia.com>
Cc: Talel Shenhar <talel@amazon.com>
Cc: Eduardo Valentin <edubezval@gmail.com>
Cc: Keerthy <j-keerthy@ti.com>
Cc: Kunihiko Hayashi <hayashi.kunihiko@socionext.com>
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Matthias Brugger <matthias.bgg@gmail.com>
Cc: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
Cc: Stefan Wahren <stefan.wahren@i2se.com>
Cc: Zheng Yongjun <zhengyongjun3@huawei.com>
Cc: Yang Li <yang.lee@linux.alibaba.com>
Cc: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
Cc: Daniel Golle <daniel@makrotopia.org>
Cc: Balsam CHIHI <bchihi@baylibre.com>
Cc: Mikko Perttunen <mperttunen@nvidia.com>
Cc: linux-acpi@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: linux-ide@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-hwmon@vger.kernel.org
Cc: linux-iio@vger.kernel.org
Cc: linux-sunxi@lists.linux.dev
Cc: linux-input@vger.kernel.org
Cc: netdev@vger.kernel.org
Cc: linux-wireless@vger.kernel.org
Cc: linux-pm@vger.kernel.org
Cc: linux-rpi-kernel@lists.infradead.org
Cc: linux-arm-msm@vger.kernel.org
Cc: linux-renesas-soc@vger.kernel.org
Cc: linux-rockchip@lists.infradead.org
Cc: linux-samsung-soc@vger.kernel.org
Cc: linux-tegra@vger.kernel.org
Cc: linux-omap@vger.kernel.org
Cc: linux-mediatek@lists.infradead.org
Daniel Lezcano (17):
thermal/core: Add a thermal zone 'devdata' accessor
thermal/core: Show a debug message when get_temp() fails
thermal: Remove debug or error messages in get_temp() ops
thermal/hwmon: Do not set no_hwmon before calling
thermal_add_hwmon_sysfs()
thermal/hwmon: Use the right device for devm_thermal_add_hwmon_sysfs()
thermal: Don't use 'device' internal thermal zone structure field
thermal/core: Add 'type' accessor
thermal/drivers/spear: Don't use tz->device but pdev->dev
thermal: Add a thermal zone id accessor
thermal: Use thermal_zone_device_type() accessor
thermal/drivers/da9062: Don't access the thermal zone device fields
thermal/hwmon: Use the thermal_core.h header
thermal/drivers/tegra: Remove unneeded lock when setting a trip point
thermal/tegra: Do not enable the thermal zone, it is already enabled
thermal/drivers/acerhdf: Make interval setting only at module load
time
thermal/drivers/acerhdf: Remove pointless governor test
thermal/traces: Replace the thermal zone structure parameter with the
field value
drivers/acpi/thermal.c | 18 +++----
drivers/ata/ahci_imx.c | 2 +-
drivers/hwmon/hwmon.c | 4 +-
drivers/hwmon/pmbus/pmbus_core.c | 2 +-
drivers/hwmon/scmi-hwmon.c | 4 +-
drivers/hwmon/scpi-hwmon.c | 2 +-
drivers/iio/adc/sun4i-gpadc-iio.c | 2 +-
drivers/input/touchscreen/sun4i-ts.c | 2 +-
.../ethernet/chelsio/cxgb4/cxgb4_thermal.c | 2 +-
.../ethernet/mellanox/mlxsw/core_thermal.c | 16 +++----
drivers/net/wireless/intel/iwlwifi/mvm/tt.c | 4 +-
drivers/platform/x86/acerhdf.c | 19 ++------
drivers/power/supply/power_supply_core.c | 2 +-
drivers/regulator/max8973-regulator.c | 2 +-
drivers/thermal/amlogic_thermal.c | 4 +-
drivers/thermal/armada_thermal.c | 14 ++----
drivers/thermal/broadcom/bcm2711_thermal.c | 3 +-
drivers/thermal/broadcom/bcm2835_thermal.c | 3 +-
drivers/thermal/broadcom/brcmstb_thermal.c | 8 ++--
drivers/thermal/broadcom/ns-thermal.c | 2 +-
drivers/thermal/broadcom/sr-thermal.c | 2 +-
drivers/thermal/da9062-thermal.c | 13 +++--
drivers/thermal/db8500_thermal.c | 2 +-
drivers/thermal/dove_thermal.c | 7 +--
drivers/thermal/gov_fair_share.c | 4 +-
drivers/thermal/gov_power_allocator.c | 6 ++-
drivers/thermal/gov_step_wise.c | 4 +-
drivers/thermal/hisi_thermal.c | 5 +-
drivers/thermal/imx8mm_thermal.c | 4 +-
drivers/thermal/imx_sc_thermal.c | 9 ++--
drivers/thermal/imx_thermal.c | 47 +++++--------------
.../intel/int340x_thermal/int3400_thermal.c | 2 +-
.../int340x_thermal/int340x_thermal_zone.c | 4 +-
.../processor_thermal_device_pci.c | 4 +-
drivers/thermal/intel/intel_pch_thermal.c | 2 +-
.../thermal/intel/intel_quark_dts_thermal.c | 6 +--
drivers/thermal/intel/intel_soc_dts_iosf.c | 13 ++---
drivers/thermal/intel/x86_pkg_temp_thermal.c | 4 +-
drivers/thermal/k3_bandgap.c | 4 +-
drivers/thermal/k3_j72xx_bandgap.c | 2 +-
drivers/thermal/kirkwood_thermal.c | 7 +--
drivers/thermal/max77620_thermal.c | 6 +--
drivers/thermal/mediatek/auxadc_thermal.c | 4 +-
drivers/thermal/mediatek/lvts_thermal.c | 10 ++--
drivers/thermal/qcom/qcom-spmi-adc-tm5.c | 6 +--
drivers/thermal/qcom/qcom-spmi-temp-alarm.c | 6 +--
drivers/thermal/qcom/tsens.c | 6 +--
drivers/thermal/qoriq_thermal.c | 4 +-
drivers/thermal/rcar_gen3_thermal.c | 5 +-
drivers/thermal/rcar_thermal.c | 8 +---
drivers/thermal/rockchip_thermal.c | 8 +---
drivers/thermal/rzg2l_thermal.c | 3 +-
drivers/thermal/samsung/exynos_tmu.c | 4 +-
drivers/thermal/spear_thermal.c | 10 ++--
drivers/thermal/sprd_thermal.c | 2 +-
drivers/thermal/st/st_thermal.c | 4 +-
drivers/thermal/st/stm_thermal.c | 4 +-
drivers/thermal/sun8i_thermal.c | 4 +-
drivers/thermal/tegra/soctherm.c | 6 +--
drivers/thermal/tegra/tegra-bpmp-thermal.c | 6 ++-
drivers/thermal/tegra/tegra30-tsensor.c | 31 ++++++------
drivers/thermal/thermal-generic-adc.c | 7 ++-
drivers/thermal/thermal_core.c | 26 +++++++++-
drivers/thermal/thermal_helpers.c | 3 ++
drivers/thermal/thermal_hwmon.c | 9 ++--
drivers/thermal/thermal_hwmon.h | 4 +-
drivers/thermal/thermal_mmio.c | 2 +-
.../ti-soc-thermal/ti-thermal-common.c | 10 ++--
drivers/thermal/uniphier_thermal.c | 2 +-
include/linux/thermal.h | 19 ++++++++
include/trace/events/thermal.h | 24 +++++-----
.../trace/events/thermal_power_allocator.h | 12 ++---
72 files changed, 251 insertions(+), 270 deletions(-)
--
2.34.1
^ permalink raw reply
* [PATCH v7 3/3] HID: cp2112: Fwnode Support
From: Danny Kaehn @ 2023-02-23 21:31 UTC (permalink / raw)
To: robh+dt, krzysztof.kozlowski+dt, jikos, benjamin.tissoires
Cc: bartosz.golaszewski, andriy.shevchenko, dmitry.torokhov,
devicetree, linux-input, ethan.twardy
In-Reply-To: <20230223213147.268-1-kaehndan@gmail.com>
Bind I2C and GPIO interfaces to subnodes with names
"i2c" and "gpio" if they exist, respectively. This
allows the GPIO and I2C controllers to be described
in firmware as usual. Additionally, support configuring the
I2C bus speed from the clock-frequency device property.
Signed-off-by: Danny Kaehn <kaehndan@gmail.com>
---
drivers/hid/hid-cp2112.c | 15 +++++++++++++--
1 file changed, 13 insertions(+), 2 deletions(-)
diff --git a/drivers/hid/hid-cp2112.c b/drivers/hid/hid-cp2112.c
index 27cadadda7c9..491e3c83af12 100644
--- a/drivers/hid/hid-cp2112.c
+++ b/drivers/hid/hid-cp2112.c
@@ -1234,6 +1234,7 @@ static int cp2112_probe(struct hid_device *hdev, const struct hid_device_id *id)
u8 buf[3];
struct cp2112_smbus_config_report config;
struct gpio_irq_chip *girq;
+ struct i2c_timings timings;
int ret;
dev = devm_kzalloc(&hdev->dev, sizeof(*dev), GFP_KERNEL);
@@ -1292,6 +1293,10 @@ static int cp2112_probe(struct hid_device *hdev, const struct hid_device_id *id)
goto err_power_normal;
}
+ device_set_node(&dev->adap.dev, device_get_named_child_node(&hdev->dev, "i2c"));
+ i2c_parse_fw_timings(&dev->adap.dev, &timings, true);
+
+ config.clock_speed = cpu_to_be32(timings.bus_freq_hz);
config.retry_time = cpu_to_be16(1);
ret = cp2112_hid_output(hdev, (u8 *)&config, sizeof(config),
@@ -1300,7 +1305,7 @@ static int cp2112_probe(struct hid_device *hdev, const struct hid_device_id *id)
hid_err(hdev, "error setting SMBus config\n");
if (ret >= 0)
ret = -EIO;
- goto err_power_normal;
+ goto err_free_i2c_of;
}
hid_set_drvdata(hdev, (void *)dev);
@@ -1322,7 +1327,7 @@ static int cp2112_probe(struct hid_device *hdev, const struct hid_device_id *id)
if (ret) {
hid_err(hdev, "error registering i2c adapter\n");
- goto err_power_normal;
+ goto err_free_i2c_of;
}
hid_dbg(hdev, "adapter registered\n");
@@ -1336,6 +1341,7 @@ static int cp2112_probe(struct hid_device *hdev, const struct hid_device_id *id)
dev->gc.ngpio = 8;
dev->gc.can_sleep = 1;
dev->gc.parent = &hdev->dev;
+ dev->gc.fwnode = device_get_named_child_node(&hdev->dev, "gpio");
dev->irq.name = "cp2112-gpio";
dev->irq.irq_startup = cp2112_gpio_irq_startup;
@@ -1376,7 +1382,10 @@ static int cp2112_probe(struct hid_device *hdev, const struct hid_device_id *id)
err_gpiochip_remove:
gpiochip_remove(&dev->gc);
err_free_i2c:
+ fwnode_handle_put(dev->gc.fwnode);
i2c_del_adapter(&dev->adap);
+err_free_i2c_of:
+ fwnode_handle_put(dev_fwnode(&dev->adap.dev));
err_power_normal:
hid_hw_power(hdev, PM_HINT_NORMAL);
err_hid_close:
@@ -1391,6 +1400,8 @@ static void cp2112_remove(struct hid_device *hdev)
struct cp2112_device *dev = hid_get_drvdata(hdev);
int i;
+ fwnode_handle_put(dev->gc.fwnode);
+ fwnode_handle_put(dev_fwnode(&dev->adap.dev));
sysfs_remove_group(&hdev->dev.kobj, &cp2112_attr_group);
i2c_del_adapter(&dev->adap);
--
2.25.1
^ permalink raw reply related
* [PATCH v7 2/3] HID: usbhid: Share USB device firmware node with child HID device
From: Danny Kaehn @ 2023-02-23 21:31 UTC (permalink / raw)
To: robh+dt, krzysztof.kozlowski+dt, jikos, benjamin.tissoires
Cc: bartosz.golaszewski, andriy.shevchenko, dmitry.torokhov,
devicetree, linux-input, ethan.twardy
In-Reply-To: <20230223213147.268-1-kaehndan@gmail.com>
USB HID core now shares its fwnode with its child HID device.
Since there can only be one HID device on a USB interface, it is redundant
to specify a hid node under the USB device. This allows usb HID device
drivers to be described in firmware and make use of device properties.
Signed-off-by: Danny Kaehn <kaehndan@gmail.com>
---
drivers/hid/usbhid/hid-core.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c
index 257dd73e37bf..090260d99c84 100644
--- a/drivers/hid/usbhid/hid-core.c
+++ b/drivers/hid/usbhid/hid-core.c
@@ -19,6 +19,7 @@
#include <linux/list.h>
#include <linux/mm.h>
#include <linux/mutex.h>
+#include <linux/property.h>
#include <linux/spinlock.h>
#include <asm/unaligned.h>
#include <asm/byteorder.h>
@@ -1374,6 +1375,7 @@ static int usbhid_probe(struct usb_interface *intf, const struct usb_device_id *
hid->hiddev_report_event = hiddev_report_event;
#endif
hid->dev.parent = &intf->dev;
+ device_set_node(&hid->dev, dev_fwnode(&intf->dev));
hid->bus = BUS_USB;
hid->vendor = le16_to_cpu(dev->descriptor.idVendor);
hid->product = le16_to_cpu(dev->descriptor.idProduct);
--
2.25.1
^ permalink raw reply related
* [PATCH v7 1/3] dt-bindings: i2c: Add CP2112 HID USB to SMBus Bridge
From: Danny Kaehn @ 2023-02-23 21:31 UTC (permalink / raw)
To: robh+dt, krzysztof.kozlowski+dt, jikos, benjamin.tissoires
Cc: bartosz.golaszewski, andriy.shevchenko, dmitry.torokhov,
devicetree, linux-input, ethan.twardy
In-Reply-To: <20230223213147.268-1-kaehndan@gmail.com>
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.
Signed-off-by: Danny Kaehn <kaehndan@gmail.com>
---
.../bindings/i2c/silabs,cp2112.yaml | 113 ++++++++++++++++++
1 file changed, 113 insertions(+)
create mode 100644 Documentation/devicetree/bindings/i2c/silabs,cp2112.yaml
diff --git a/Documentation/devicetree/bindings/i2c/silabs,cp2112.yaml b/Documentation/devicetree/bindings/i2c/silabs,cp2112.yaml
new file mode 100644
index 000000000000..a27509627804
--- /dev/null
+++ b/Documentation/devicetree/bindings/i2c/silabs,cp2112.yaml
@@ -0,0 +1,113 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/i2c/silabs,cp2112.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: CP2112 HID USB to SMBus/I2C Bridge
+
+maintainers:
+ - Danny Kaehn <kaehndan@gmail.com>
+
+description:
+ The CP2112 is a USB HID device which includes an integrated I2C controller
+ and 8 GPIO pins. Its GPIO pins can each be configured as inputs, open-drain
+ outputs, or push-pull outputs.
+
+properties:
+ compatible:
+ const: usb10c4,ea90
+
+ reg:
+ maxItems: 1
+ description: The USB port number on the host controller
+
+ i2c:
+ description: The SMBus/I2C controller node for the CP2112
+ $ref: /schemas/i2c/i2c-controller.yaml#
+ unevaluatedProperties: false
+
+ properties:
+ sda-gpios:
+ maxItems: 1
+
+ scl-gpios:
+ maxItems: 1
+
+ clock-frequency:
+ minimum: 10000
+ default: 100000
+ maximum: 400000
+
+ gpio:
+ description: The GPIO controller node for the CP2112
+ type: object
+ unevaluatedProperties: false
+
+ properties:
+ interrupt-controller: true
+ "#interrupt-cells":
+ const: 2
+
+ gpio-controller: true
+ "#gpio-cells":
+ const: 2
+
+ gpio-line-names:
+ minItems: 1
+ maxItems: 8
+
+ patternProperties:
+ "-hog(-[0-9]+)?$":
+ type: object
+
+ required:
+ - gpio-hog
+
+required:
+ - compatible
+ - reg
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/interrupt-controller/irq.h>
+ #include <dt-bindings/gpio/gpio.h>
+
+ usb {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ device@1 {
+ compatible = "usb10c4,ea90";
+ reg = <1>;
+
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ sda-gpios = <&cp2112_gpio 0 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
+ scl-gpios = <&cp2112_gpio 1 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
+
+ temp@48 {
+ compatible = "national,lm75";
+ reg = <0x48>;
+ };
+ };
+
+ cp2112_gpio: gpio {
+ gpio-controller;
+ interrupt-controller;
+ #gpio-cells = <2>;
+ gpio-line-names = "CP2112_SDA", "CP2112_SCL", "TEST2",
+ "TEST3","TEST4", "TEST5", "TEST6";
+
+ fan-rst-hog {
+ gpio-hog;
+ gpios = <7 GPIO_ACTIVE_HIGH>;
+ output-high;
+ line-name = "FAN_RST";
+ };
+ };
+ };
+ };
--
2.25.1
^ permalink raw reply related
* [PATCH v7 0/3] Firmware Support for USB-HID Devices and CP2112
From: Danny Kaehn @ 2023-02-23 21:31 UTC (permalink / raw)
To: robh+dt, krzysztof.kozlowski+dt, jikos, benjamin.tissoires
Cc: bartosz.golaszewski, andriy.shevchenko, dmitry.torokhov,
devicetree, linux-input, ethan.twardy
This patchset allows USB-HID devices to have DeviceTree bindings through sharing
the USB fwnode with the HID driver, and adds such a binding and driver
implementation for the CP2112 USB to SMBus Bridge (which necessitated the
USB-HID change). This change allows a CP2112 permanently attached in hardware to
be described in DT and interoperate with other drivers.
Changes in v7:
- Use dev_fwnode when calling fwnod_handle_put in i2c_adapter in hid-cp2112.c
- Capitalize I2C and GPIO in commit message for patch 0003
Changes in v6:
- Fix fwnode_handle reference leaks in hid-cp21112.c
- Simplify hog node pattern in silabs,cp2112.yaml
Changes in v5:
- Use fwnode API instead of of_node api in hid-core.c and hid-cp2112.c
- Include sda-gpios and scl-gpios in silabs,cp2112.yaml
- Additional fixups to silabs,cp2112.yaml to address comments
- Submit threaded interrupt bugfix separately from this patchset, as requested
Changes in v4:
- Moved silabs,cp2112.yaml to /Documentation/devicetree/bindings/i2c
Changes in v3:
- Additional fixups to silabs,cp2112.yaml to address comments
Changes in v2:
- Added more detail to silabs,cp2112.yaml dt-binding
- Moved silabs,cp2112.yaml to /Documentation/devicetree/bindings/input
- Added support for setting smbus clock-frequency from DT in hid-cp2112.c
- Added freeing of of_nodes on error paths of _probe in hid-cp2112.c
Danny Kaehn (3):
dt-bindings: i2c: Add CP2112 HID USB to SMBus Bridge
HID: usbhid: Share USB device firmware node with child HID device
HID: cp2112: Fwnode Support
.../bindings/i2c/silabs,cp2112.yaml | 113 ++++++++++++++++++
drivers/hid/hid-cp2112.c | 15 ++-
drivers/hid/usbhid/hid-core.c | 2 +
3 files changed, 128 insertions(+), 2 deletions(-)
create mode 100644 Documentation/devicetree/bindings/i2c/silabs,cp2112.yaml
--
2.25.1
^ permalink raw reply
* RE: [PATCH] HID: usbhid: enable remote wakeup for mice
From: Limonciello, Mario @ 2023-02-23 19:41 UTC (permalink / raw)
To: Oliver Neukum, Greg KH, Michael Wu
Cc: jikos@kernel.org, benjamin.tissoires@redhat.com,
linux-usb@vger.kernel.org, linux-input@vger.kernel.org,
linux-kernel@vger.kernel.org, Gong, Richard
In-Reply-To: <801338c3-7c0d-6b3e-eac8-872ee5add466@suse.com>
[AMD Official Use Only - General]
> -----Original Message-----
> From: Oliver Neukum <oneukum@suse.com>
> Sent: Thursday, February 23, 2023 06:02
> To: Greg KH <gregkh@linuxfoundation.org>; Michael Wu
> <michael@allwinnertech.com>
> Cc: jikos@kernel.org; benjamin.tissoires@redhat.com; linux-
> usb@vger.kernel.org; linux-input@vger.kernel.org; linux-
> kernel@vger.kernel.org; Limonciello, Mario <Mario.Limonciello@amd.com>;
> Gong, Richard <Richard.Gong@amd.com>
> Subject: Re: [PATCH] HID: usbhid: enable remote wakeup for mice
>
> On 23.02.23 12:23, Greg KH wrote:
>
> >> I just want to explain the background of my patch, to prove we could use
> a
> >> similar way to avoid such a "disturbing" situation.
> >> To reduce the influence, my patch enables remote wakeup for USB
> mouse
> >> devices refer to what keyboard do.
> >
> > Keyboards are not mice :)
> >
>
> OK, let me explain, why I never proposed switching on autosuspend
> for mice or using them as a system wakeup source. The reasons are very
> similar.
>
> Basically the standard gives us no way to ask a device what constitutes
> a wakeup event for it. We just get the blanket statement of support
> or no support.
>
> For runtime PM I would want my mouse to generate a remote wakeup
> whenever a button is pressed or the mouse is moved. Under this
> premise runtime PM with a mouse works wonderfully. Testing that,
> however, is a challenge.
> It turns out that mice that claim that they support remote wakeup
> by and large deactivate their LED/laser when you send them into
> suspension. Hence they react only to buttons being pressed or mouse
> wheels moved.
>
> As a system wakeup source a mouse that generates events when
> it is moved, however, would make the system unsuspendable, whenever
> even
> a bit of vibration is acting on the system.
> And as S4 is used in many setups to prevent an uncontrolled shutdown
> at low power, this must work.
At least in my version of the series, this is part of the reason that it was
only intended to be used with s2idle.
The kernel driver is well aware of what power state you're in the suspend
callback (pm_suspend_target_state).
What about if we agreed to treat this one special by examining that?
If the sysfs is set to "enabled"
* During suspend if your target is s2idle -> program it
* During suspend if your target is mem -> disable it
* During suspend if your target is hibernate -> disable it
With that type of policy on how to handle the suspend call in place
perhaps we could set it to enabled by default?
>
> I suspect that most, but _not_ _all_ mice, are designed for use
> with a system that ties power management to the screen saver.
> That is a mode of operation that due to the lack of coupling
> between GUI and kernel is hard to copy.
>
> Frankly given these constraints the only default safe in every
> case seems to me to leave the kernel's default at off and
> put the work into udev's hwdb. Not that that is a good solution,
> merely the best I can come up with.
>
> Regards
> Oliver
Turning on "autosuspend" for USB mice makes them behave pretty
similarly to how they work when they're marked for remote wakeup.
On some mice the lasers turn off, and they only wakeup when you
press a button or roll a wheel.
^ permalink raw reply
* Re: [PATCH 4/5] USB: core: Add API to change the wireless_status
From: Alan Stern @ 2023-02-23 17:07 UTC (permalink / raw)
To: Bastien Nocera
Cc: linux-usb, linux-input, Greg Kroah-Hartman, Benjamin Tissoires,
Filipe Laíns, Nestor Lopez Casado
In-Reply-To: <16aaaa1a6207e7da07faa932ecac0dcc9e5f10e3.camel@hadess.net>
On Thu, Feb 23, 2023 at 05:51:48PM +0100, Bastien Nocera wrote:
> On Thu, 2023-02-23 at 11:25 -0500, Alan Stern wrote:
> > On Thu, Feb 23, 2023 at 05:17:13PM +0100, Bastien Nocera wrote:
> > > On Thu, 2023-02-23 at 10:41 -0500, Alan Stern wrote:
> > > > On Thu, Feb 23, 2023 at 02:24:51PM +0100, Bastien Nocera wrote:
> > > > > Allow device specific drivers to change the wireless status of
> > > > > a
> > > > > device.
> > > > > This will allow user-space to know whether the device is
> > > > > available,
> > > > > whether or not specific USB interfaces can detect it.
> > > > >
> > > > > This can be used by wireless headsets with USB receivers to
> > > > > propagate to
> > > > > user-space whether or not the headset is turned on, so as to
> > > > > consider it
> > > > > as unavailable, and not switch to it just because the receiver
> > > > > is
> > > > > plugged in.
> > > > >
> > > > > Signed-off-by: Bastien Nocera <hadess@hadess.net>
> > > > > ---
> > > > > drivers/usb/core/message.c | 13 +++++++++++++
> > > > > drivers/usb/core/usb.c | 24 ++++++++++++++++++++++++
> > > > > include/linux/usb.h | 4 ++++
> > > > > 3 files changed, 41 insertions(+)
> > > > >
> > > > > diff --git a/drivers/usb/core/message.c
> > > > > b/drivers/usb/core/message.c
> > > > > index 127fac1af676..d5c7749d515e 100644
> > > > > --- a/drivers/usb/core/message.c
> > > > > +++ b/drivers/usb/core/message.c
> > > > > @@ -1908,6 +1908,18 @@ static void
> > > > > __usb_queue_reset_device(struct
> > > > > work_struct *ws)
> > > > > usb_put_intf(iface); /* Undo _get_ in
> > > > > usb_queue_reset_device() */
> > > > > }
> > > > >
> > > > > +/*
> > > > > + * Internal function to set the wireless_status sysfs
> > > > > attribute
> > > > > + * See usb_set_wireless_status() for more details
> > > > > + */
> > > > > +static void __usb_wireless_status_intf(struct work_struct *ws)
> > > > > +{
> > > > > + struct usb_interface *iface =
> > > > > + container_of(ws, struct usb_interface,
> > > > > wireless_status_work);
> > > > > +
> > > > > + usb_update_wireless_status_attr(iface);
> > > > > + usb_put_intf(iface); /* Undo _get_ in
> > > > > usb_set_wireless_status() */
> > > > > +}
> > > >
> > > > Have you thought about what will happen if this routine ends up
> > > > running
> > > > after the interface has been deleted?
> > >
> > > I believe that usb_release_interface() will only be called once the
> > > last reference to the interface is dropped, so bar any refcounting
> > > bugs, the interface should always exist when this function is
> > > called.
> >
> > Yes, but what about the calls made by
> > usb_update_wireless_status_attr():
> > sysfs_update_group(), sysfs_notify(), and kobject_uevent()? Will
> > they
> > work properly when called for an object that has been unregistered
> > from
> > sysfs?
>
> Those calls are made before the last reference to the interface can be
> dropped by our own call to usb_put_intf(). So, in effect, the interface
> is kept alive until we're done with our work so we can't ever be poking
> at objects that disappeared.
>
> Am I missing something about the object model, or the refcounting here?
Yes, you are.
There's a difference between object _existence_ and object
_registration_. An object (kobject, struct device, whatever) exists for
as long as its memory is still allocated and in use. This is what
refcounting protects.
An object is registered in sysfs during the time between calls to
device_add() and device_del(). During this time it shows up as a
directory under /sys, along with its various attribute files. Calling
device_del() gets rid of all that -- James Bottomley calls it "removing
an object from visibility". The object still exists, but it isn't part
of sysfs any more.
The refcounting in your patch guarantees that when the work function
runs, the interface structure will still exist. But refcounting does
not guarantee that the interface will still be registered in sysfs, and
this can actually happen if the work is scheduled immediately before the
interface is unregistered.
So my question is: What will happen when sysfs_update_group(),
sysfs_notify(), and kobject_uevent() are called after the interface has
been unregistered from sysfs? Maybe they will work okay -- I simply
don't know, and I wanted to find out whether you had considered the
issue.
Alan Stern
^ permalink raw reply
* Re: [PATCH 4/5] USB: core: Add API to change the wireless_status
From: Bastien Nocera @ 2023-02-23 16:51 UTC (permalink / raw)
To: Alan Stern
Cc: linux-usb, linux-input, Greg Kroah-Hartman, Benjamin Tissoires,
Filipe Laíns, Nestor Lopez Casado
In-Reply-To: <Y/eTkXYlflDWllHZ@rowland.harvard.edu>
On Thu, 2023-02-23 at 11:25 -0500, Alan Stern wrote:
> On Thu, Feb 23, 2023 at 05:17:13PM +0100, Bastien Nocera wrote:
> > On Thu, 2023-02-23 at 10:41 -0500, Alan Stern wrote:
> > > On Thu, Feb 23, 2023 at 02:24:51PM +0100, Bastien Nocera wrote:
> > > > Allow device specific drivers to change the wireless status of
> > > > a
> > > > device.
> > > > This will allow user-space to know whether the device is
> > > > available,
> > > > whether or not specific USB interfaces can detect it.
> > > >
> > > > This can be used by wireless headsets with USB receivers to
> > > > propagate to
> > > > user-space whether or not the headset is turned on, so as to
> > > > consider it
> > > > as unavailable, and not switch to it just because the receiver
> > > > is
> > > > plugged in.
> > > >
> > > > Signed-off-by: Bastien Nocera <hadess@hadess.net>
> > > > ---
> > > > drivers/usb/core/message.c | 13 +++++++++++++
> > > > drivers/usb/core/usb.c | 24 ++++++++++++++++++++++++
> > > > include/linux/usb.h | 4 ++++
> > > > 3 files changed, 41 insertions(+)
> > > >
> > > > diff --git a/drivers/usb/core/message.c
> > > > b/drivers/usb/core/message.c
> > > > index 127fac1af676..d5c7749d515e 100644
> > > > --- a/drivers/usb/core/message.c
> > > > +++ b/drivers/usb/core/message.c
> > > > @@ -1908,6 +1908,18 @@ static void
> > > > __usb_queue_reset_device(struct
> > > > work_struct *ws)
> > > > usb_put_intf(iface); /* Undo _get_ in
> > > > usb_queue_reset_device() */
> > > > }
> > > >
> > > > +/*
> > > > + * Internal function to set the wireless_status sysfs
> > > > attribute
> > > > + * See usb_set_wireless_status() for more details
> > > > + */
> > > > +static void __usb_wireless_status_intf(struct work_struct *ws)
> > > > +{
> > > > + struct usb_interface *iface =
> > > > + container_of(ws, struct usb_interface,
> > > > wireless_status_work);
> > > > +
> > > > + usb_update_wireless_status_attr(iface);
> > > > + usb_put_intf(iface); /* Undo _get_ in
> > > > usb_set_wireless_status() */
> > > > +}
> > >
> > > Have you thought about what will happen if this routine ends up
> > > running
> > > after the interface has been deleted?
> >
> > I believe that usb_release_interface() will only be called once the
> > last reference to the interface is dropped, so bar any refcounting
> > bugs, the interface should always exist when this function is
> > called.
>
> Yes, but what about the calls made by
> usb_update_wireless_status_attr():
> sysfs_update_group(), sysfs_notify(), and kobject_uevent()? Will
> they
> work properly when called for an object that has been unregistered
> from
> sysfs?
Those calls are made before the last reference to the interface can be
dropped by our own call to usb_put_intf(). So, in effect, the interface
is kept alive until we're done with our work so we can't ever be poking
at objects that disappeared.
Am I missing something about the object model, or the refcounting here?
^ permalink raw reply
* Re: [PATCH 4/5] USB: core: Add API to change the wireless_status
From: Alan Stern @ 2023-02-23 16:25 UTC (permalink / raw)
To: Bastien Nocera
Cc: linux-usb, linux-input, Greg Kroah-Hartman, Benjamin Tissoires,
Filipe Laíns, Nestor Lopez Casado
In-Reply-To: <f94acf520e62676c741c0959f91c8d951d74a36c.camel@hadess.net>
On Thu, Feb 23, 2023 at 05:17:13PM +0100, Bastien Nocera wrote:
> On Thu, 2023-02-23 at 10:41 -0500, Alan Stern wrote:
> > On Thu, Feb 23, 2023 at 02:24:51PM +0100, Bastien Nocera wrote:
> > > Allow device specific drivers to change the wireless status of a
> > > device.
> > > This will allow user-space to know whether the device is available,
> > > whether or not specific USB interfaces can detect it.
> > >
> > > This can be used by wireless headsets with USB receivers to
> > > propagate to
> > > user-space whether or not the headset is turned on, so as to
> > > consider it
> > > as unavailable, and not switch to it just because the receiver is
> > > plugged in.
> > >
> > > Signed-off-by: Bastien Nocera <hadess@hadess.net>
> > > ---
> > > drivers/usb/core/message.c | 13 +++++++++++++
> > > drivers/usb/core/usb.c | 24 ++++++++++++++++++++++++
> > > include/linux/usb.h | 4 ++++
> > > 3 files changed, 41 insertions(+)
> > >
> > > diff --git a/drivers/usb/core/message.c
> > > b/drivers/usb/core/message.c
> > > index 127fac1af676..d5c7749d515e 100644
> > > --- a/drivers/usb/core/message.c
> > > +++ b/drivers/usb/core/message.c
> > > @@ -1908,6 +1908,18 @@ static void __usb_queue_reset_device(struct
> > > work_struct *ws)
> > > usb_put_intf(iface); /* Undo _get_ in
> > > usb_queue_reset_device() */
> > > }
> > >
> > > +/*
> > > + * Internal function to set the wireless_status sysfs attribute
> > > + * See usb_set_wireless_status() for more details
> > > + */
> > > +static void __usb_wireless_status_intf(struct work_struct *ws)
> > > +{
> > > + struct usb_interface *iface =
> > > + container_of(ws, struct usb_interface,
> > > wireless_status_work);
> > > +
> > > + usb_update_wireless_status_attr(iface);
> > > + usb_put_intf(iface); /* Undo _get_ in
> > > usb_set_wireless_status() */
> > > +}
> >
> > Have you thought about what will happen if this routine ends up
> > running
> > after the interface has been deleted?
>
> I believe that usb_release_interface() will only be called once the
> last reference to the interface is dropped, so bar any refcounting
> bugs, the interface should always exist when this function is called.
Yes, but what about the calls made by usb_update_wireless_status_attr():
sysfs_update_group(), sysfs_notify(), and kobject_uevent()? Will they
work properly when called for an object that has been unregistered from
sysfs?
Alan Stern
^ permalink raw reply
* Re: [PATCH 4/5] USB: core: Add API to change the wireless_status
From: Bastien Nocera @ 2023-02-23 16:17 UTC (permalink / raw)
To: Alan Stern
Cc: linux-usb, linux-input, Greg Kroah-Hartman, Benjamin Tissoires,
Filipe Laíns, Nestor Lopez Casado
In-Reply-To: <Y/eJDDPXJfYgfdfI@rowland.harvard.edu>
On Thu, 2023-02-23 at 10:41 -0500, Alan Stern wrote:
> On Thu, Feb 23, 2023 at 02:24:51PM +0100, Bastien Nocera wrote:
> > Allow device specific drivers to change the wireless status of a
> > device.
> > This will allow user-space to know whether the device is available,
> > whether or not specific USB interfaces can detect it.
> >
> > This can be used by wireless headsets with USB receivers to
> > propagate to
> > user-space whether or not the headset is turned on, so as to
> > consider it
> > as unavailable, and not switch to it just because the receiver is
> > plugged in.
> >
> > Signed-off-by: Bastien Nocera <hadess@hadess.net>
> > ---
> > drivers/usb/core/message.c | 13 +++++++++++++
> > drivers/usb/core/usb.c | 24 ++++++++++++++++++++++++
> > include/linux/usb.h | 4 ++++
> > 3 files changed, 41 insertions(+)
> >
> > diff --git a/drivers/usb/core/message.c
> > b/drivers/usb/core/message.c
> > index 127fac1af676..d5c7749d515e 100644
> > --- a/drivers/usb/core/message.c
> > +++ b/drivers/usb/core/message.c
> > @@ -1908,6 +1908,18 @@ static void __usb_queue_reset_device(struct
> > work_struct *ws)
> > usb_put_intf(iface); /* Undo _get_ in
> > usb_queue_reset_device() */
> > }
> >
> > +/*
> > + * Internal function to set the wireless_status sysfs attribute
> > + * See usb_set_wireless_status() for more details
> > + */
> > +static void __usb_wireless_status_intf(struct work_struct *ws)
> > +{
> > + struct usb_interface *iface =
> > + container_of(ws, struct usb_interface,
> > wireless_status_work);
> > +
> > + usb_update_wireless_status_attr(iface);
> > + usb_put_intf(iface); /* Undo _get_ in
> > usb_set_wireless_status() */
> > +}
>
> Have you thought about what will happen if this routine ends up
> running
> after the interface has been deleted?
I believe that usb_release_interface() will only be called once the
last reference to the interface is dropped, so bar any refcounting
bugs, the interface should always exist when this function is called.
> > /*
> > * usb_set_configuration - Makes a particular device setting be
> > current
> > @@ -2100,6 +2112,7 @@ int usb_set_configuration(struct usb_device
> > *dev, int configuration)
> > intf->dev.type = &usb_if_device_type;
> > intf->dev.groups = usb_interface_groups;
> > INIT_WORK(&intf->reset_ws,
> > __usb_queue_reset_device);
> > + INIT_WORK(&intf->wireless_status_work,
> > __usb_wireless_status_intf);
> > intf->minor = -1;
> > device_initialize(&intf->dev);
> > pm_runtime_no_callbacks(&intf->dev);
> > diff --git a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c
> > index 11b15d7b357a..5f42c5b9d209 100644
> > --- a/drivers/usb/core/usb.c
> > +++ b/drivers/usb/core/usb.c
> > @@ -871,6 +871,30 @@ int usb_get_current_frame_number(struct
> > usb_device *dev)
> > }
> > EXPORT_SYMBOL_GPL(usb_get_current_frame_number);
> >
> > +/**
> > + * usb_set_wireless_status - sets the wireless_status struct
> > member
> > + * @dev: the device to modify
> > + * @status: the new wireless status
> > + *
> > + * Set the wireless_status struct member to the new value, and
> > emit
> > + * sysfs changes as necessary.
> > + *
> > + * Returns: 0 on success, -EALREADY if already set.
> > + */
> > +int usb_set_wireless_status(struct usb_interface *iface,
> > + enum usb_wireless_status status)
> > +{
> > + if (iface->wireless_status == status)
> > + return -EALREADY;
> > +
> > + usb_get_intf(iface);
> > + iface->wireless_status = status;
> > + schedule_work(&iface->wireless_status_work);
> > +
> > + return 0;
> > +}
> > +EXPORT_SYMBOL_GPL(usb_set_wireless_status);
>
> This routine belongs in message.c, next to
> __usb_wireless_status_intf().
Sure, done locally.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox