* Re: [syzbot] [input?] INFO: task hung in __input_unregister_device (5)
From: Ricardo B. Marliere @ 2023-09-18 20:12 UTC (permalink / raw)
To: syzbot
Cc: dmitry.torokhov, linux-input, linux-kernel, linux-usb, rydberg,
syzkaller-bugs
In-Reply-To: <0000000000003d63410605a18363@google.com>
#syz test: git://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git next-20230918
^ permalink raw reply
* Re: [PATCH] Input: add driver for TouchNetix aXiom touchscreen
From: Marco Felsch @ 2023-09-18 19:38 UTC (permalink / raw)
To: Kamel Bouhara
Cc: linux-input, mark.satterthwaite, pedro.torruella, bartp,
hannah.rossiter, Thomas Petazzoni, Gregory Clement,
bsp-development.geo, kernel
In-Reply-To: <20230908153203.122062-2-kamel.bouhara@bootlin.com>
On 23-09-08, Kamel Bouhara wrote:
> Add a new driver for the TouchNetix's aXiom family of
> multi-touch controller. This driver only support i2c
> and can be later adapted for SPI and USB support.
>
> Signed-off-by: Kamel Bouhara <kamel.bouhara@bootlin.com>
> ---
> .../devicetree/bindings/vendor-prefixes.yaml | 2 +
> MAINTAINERS | 7 +
> drivers/input/touchscreen/Kconfig | 11 +
> drivers/input/touchscreen/Makefile | 1 +
> drivers/input/touchscreen/axiom_core.c | 382 ++++++++++++++++++
> drivers/input/touchscreen/axiom_core.h | 140 +++++++
> drivers/input/touchscreen/axiom_i2c.c | 349 ++++++++++++++++
> 7 files changed, 892 insertions(+)
> create mode 100644 drivers/input/touchscreen/axiom_core.c
> create mode 100644 drivers/input/touchscreen/axiom_core.h
> create mode 100644 drivers/input/touchscreen/axiom_i2c.c
Do we really care about the usb/spi case? For simplicity I would merge
the axiom_core.* into the axiom_i2c.c.
> diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml
> index 573578db9509..b0a3ed451f15 100644
> --- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
> +++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
> @@ -175,6 +175,8 @@ patternProperties:
> "^awinic,.*":
> description: Shanghai Awinic Technology Co., Ltd.
> "^axentia,.*":
> + description: TouchNetix
> + "^axiom,.*":
This looks odd, also the product is called Axiom but the manufacturer is
TouchNetix, so the compatible should be "^touchnetix,.*".
> description: Axentia Technologies AB
> "^axis,.*":
> description: Axis Communications AB
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 389fe9e38884..43add48257d8 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -3373,6 +3373,13 @@ W: https://ez.analog.com/linux-software-drivers
> F: Documentation/devicetree/bindings/hwmon/adi,axi-fan-control.yaml
> F: drivers/hwmon/axi-fan-control.c
>
> +AXIOM I2C TOUCHSCREEN DRIVER
> +M: Kamel Bouhara <kamel.bouhara@bootlin.com>
> +L: linux-input@vger.kernel.org
> +S: Maintained
> +F: drivers/input/touchscreen/axiom_core.c
> +F: drivers/input/touchscreen/axiom_i2.c
> +
> AXXIA I2C CONTROLLER
> M: Krzysztof Adamski <krzysztof.adamski@nokia.com>
> L: linux-i2c@vger.kernel.org
> diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig
> index e3e2324547b9..08a770a0c5e5 100644
> --- a/drivers/input/touchscreen/Kconfig
> +++ b/drivers/input/touchscreen/Kconfig
> @@ -150,6 +150,17 @@ config TOUCHSCREEN_AUO_PIXCIR
> To compile this driver as a module, choose M here: the
> module will be called auo-pixcir-ts.
>
> +config TOUCHSCREEN_AXIOM_I2C
> + tristate "AXIOM based multi-touch panel controllers"
"TouchNetix Axiom multi-touch controller" ?
> + help
> + Say Y here if you have a axiom touchscreen connected to
> + your system.
> +
> + If unsure, say N.
> +
> + To compile this driver as a module, choose M here: the
> + module will be called axiom_i2c.
> +
> config TOUCHSCREEN_BU21013
> tristate "BU21013 based touch panel controllers"
> depends on I2C
> diff --git a/drivers/input/touchscreen/Makefile b/drivers/input/touchscreen/Makefile
> index 62bd24f3ac8e..59a3234ddb09 100644
> --- a/drivers/input/touchscreen/Makefile
> +++ b/drivers/input/touchscreen/Makefile
> @@ -18,6 +18,7 @@ obj-$(CONFIG_TOUCHSCREEN_ADS7846) += ads7846.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
> +obj-$(CONFIG_TOUCHSCREEN_AXIOM_I2C) += axiom_core.o axiom_i2c.o
> obj-$(CONFIG_TOUCHSCREEN_BU21013) += bu21013_ts.o
> obj-$(CONFIG_TOUCHSCREEN_BU21029) += bu21029_ts.o
> obj-$(CONFIG_TOUCHSCREEN_CHIPONE_ICN8318) += chipone_icn8318.o
> diff --git a/drivers/input/touchscreen/axiom_core.c b/drivers/input/touchscreen/axiom_core.c
> new file mode 100644
> index 000000000000..d381afd7fb84
> --- /dev/null
> +++ b/drivers/input/touchscreen/axiom_core.c
> @@ -0,0 +1,382 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * TouchNetix aXiom Touchscreen Driver
> + *
> + * Copyright (C) 2020-2023 TouchNetix Ltd.
> + *
> + * Author(s): Mark Satterthwaite <mark.satterthwaite@touchnetix.com>
> + * Pedro Torruella <pedro.torruella@touchnetix.com>
> + * Bart Prescott <bartp@baasheep.co.uk>
> + * Hannah Rossiter <hannah.rossiter@touchnetix.com>
> + *
> + */
> +
> +#include <linux/crc16.h>
> +#include <linux/device.h>
> +#include <linux/input.h>
> +#include <linux/input/mt.h>
> +#include <linux/module.h>
> +#include <linux/kernel.h>
> +#include <linux/slab.h>
> +
> +#include "axiom_core.h"
> +
> +/**
> + * Decodes and populates the local u31 structure.
> + * Given a buffer of data read from page 0 of u31 in an aXiom device.
> + */
> +void axiom_get_dev_info(struct axiom_data *ts, char *info)
If we squash all together we can make this static, as well as all the
other helpers below. IMHO the function could be renamed too, e.g.
axiom_decode_info(). This allow us to drop the comment.
> +{
> + struct axiom_devinfo *devinfo = &ts->devinfo;
> +
> + if (devinfo) {
if (!devinfo)
return;
> + devinfo->bootloader_mode = ((info[1] & 0x80) != 0) ? 1 : 0;
> + devinfo->device_id = ((info[1] & 0x7F) << 8) | info[0];
> + devinfo->fw_minor = info[2];
> + devinfo->fw_major = info[3];
> + devinfo->fw_info_extra = (info[4]) | (info[5] << 8);
> + devinfo->bootloader_fw_ver_minor = info[6];
> + devinfo->bootloader_fw_ver_major = info[7];
> + devinfo->jedec_id = (info[8]) | (info[9] << 8);
> + devinfo->num_usages = info[10];
> + devinfo->silicon_revision = info[11];
> + }
> +}
> +EXPORT_SYMBOL_GPL(axiom_get_dev_info);
> +
> +/**
> + * Decodes and populates the local Usage Table.
> + * Given a buffer of data read from page 1 onwards of u31 from an aXiom
^
u31 isn't really intuitive, since we use u(signed) but instead this
stands for u(sage) here and there is a u(sage)32 as well which is even
worse.
> + * device.
> + */
> +char axiom_populate_usage_table(struct axiom_data *ts, char *rx_data)
> +{
> + u32 usage_id = 0;
> + char max_report_len = 0;
> + struct axiom_devinfo *device_info;
> + struct usage_entry *usage_table;
> +
> + device_info = &ts->devinfo;
> + usage_table = ts->usage_table;
> +
> + for (usage_id = 0; usage_id < device_info->num_usages; usage_id++) {
> + u16 offset = (usage_id * U31_BYTES_PER_USAGE);
> + char id = rx_data[offset + 0];
> + char start_page = rx_data[offset + 1];
> + char num_pages = rx_data[offset + 2];
> + char max_offset = ((rx_data[offset + 3] & 0x7F) + 1) * 2;
> +
> + /* Store the entry into the usage table */
> + usage_table[usage_id].id = id;
> + usage_table[usage_id].is_report = ((num_pages == 0) ? 1 : 0);
> + usage_table[usage_id].start_page = start_page;
> + usage_table[usage_id].num_pages = num_pages;
> +
> + dev_dbg(ts->pdev, "Usage %2u Info: %*ph\n", usage_id,
> + U31_BYTES_PER_USAGE, &rx_data[offset]);
> +
> + /* Identify the max report length the module will receive */
> + if ((usage_table[usage_id].is_report)
> + && (max_offset > max_report_len))
> + max_report_len = max_offset;
> + }
> + ts->usage_table_populated = true;
> +
> + return max_report_len;
> +}
> +EXPORT_SYMBOL_GPL(axiom_populate_usage_table);
With my suggestion below I'm not sure if we need this here.
> +
> +/**
> + * Translate usage/page/offset triplet into physical address.
> + *
> + * @usage: groups of registers
> + * @page: page to which the usage belongs to offset
> + * @offset of the usage
> + */
> +u16
> +usage_to_target_address(struct axiom_data *ts, char usage, char page,
> + char offset)
> +{
> + struct axiom_devinfo *device_info;
> + struct usage_entry *usage_table;
> + u16 target_address;
> + u32 i;
> +
> + device_info = &ts->devinfo;
> + usage_table = ts->usage_table;
> +
> + /* At the moment the convention is that u31 is always at physical address 0x0 */
> + if (!ts->usage_table_populated && (usage == DEVINFO_USAGE_ID)) {
> + target_address = ((page << 8) + offset);
> + } else if (ts->usage_table_populated) {
> + for (i = 0; i < device_info->num_usages; i++) {
> + if (usage_table[i].id == usage) {
> + if (page < usage_table[i].num_pages) {
> + target_address =
> + ((usage_table[i].start_page + page) << 8) + offset;
> + } else {
> + target_address = 0xFFFF;
> + dev_err(ts->pdev,
> + "Invalid usage table! usage: %u, page: %u, offset: %u\n",
> + usage, page, offset);
> + }
> + break;
> + }
> + }
> + } else {
> + target_address = 0xFFFF;
> + dev_err(ts->pdev, "Unpopulated usage table for usage: %u\n",
> + usage);
> + }
> +
> + dev_dbg(ts->pdev, "target_address is 0x%04x for usage: %u page %u\n",
> + target_address, usage, page);
> +
> + return target_address;
> +}
> +EXPORT_SYMBOL_GPL(usage_to_target_address);
and this.
> +void axiom_remove(struct axiom_data *ts)
> +{
> + if (ts->usage_table)
> + ts->usage_table_populated = false;
> +
> + if (ts->input_dev)
> + input_unregister_device(ts->input_dev);
> +}
> +EXPORT_SYMBOL_GPL(axiom_remove);
Just squash the drivers into one to keep it simple. Once there is a need
for SPI/USB we can it it.
> +
> +/*
> + * Support function to axiom_process_u41_report.
> + * It generates input-subsystem events for every target.
> + * After calling this function the caller shall issue
> + * a Sync to the input sub-system.
> + */
> +static bool
> +axiom_process_u41_report_target(struct axiom_data *ts,
> + struct axiom_target_report *target)
> +{
> + struct input_dev *input_dev = ts->input_dev;
> + enum axiom_target_state current_state;
> + struct u41_target *target_prev_state;
> + struct device *pdev = ts->pdev;
> + bool update = false;
> + int slot;
> +
> + /* Verify the target index */
> + if (target->index >= U41_MAX_TARGETS) {
> + dev_dbg(pdev, "Invalid target index! %u\n", target->index);
> + return false;
> + }
> +
> + target_prev_state = &ts->targets[target->index];
> +
> + current_state =
> + ((target->present == 0) ? TARGET_STATE_NOT_PRESENT : (target->z >=
> + 0) ?
> + TARGET_STATE_TOUCHING : (target->z > PROX_LEVEL)
> + && (target->z < 0) ? TARGET_STATE_HOVER : (target->z ==
> + PROX_LEVEL) ?
> + TARGET_STATE_PROX : TARGET_STATE_NOT_PRESENT);
> + if ((target_prev_state->state == current_state)
> + && (target_prev_state->x == target->x)
> + && (target_prev_state->y == target->y)
> + && (target_prev_state->z == target->z)) {
> + return false;
> + }
> +
> + slot = target->index;
> +
> + dev_dbg(pdev, "U41 Target T%u, slot:%u present:%u, x:%u, y:%u, z:%d\n",
> + target->index, slot, target->present,
> + target->x, target->y, target->z);
> +
> + switch (current_state) {
> + default:
> + case TARGET_STATE_NOT_PRESENT:
> + case TARGET_STATE_PROX:
> + {
> + if (target_prev_state->insert) {
> + update = true;
> + target_prev_state->insert = false;
> + input_mt_slot(input_dev, slot);
> +
> + if (slot == 0)
> + input_report_key(input_dev, BTN_LEFT,
> + 0);
> +
> + input_mt_report_slot_inactive(input_dev);
> + /* make sure the previous coordinates are all off */
> + /* screen when the finger comes back */
> + target->x = target->y = 65535;
> + target->z = PROX_LEVEL;
> + }
> + break;
> + }
> + case TARGET_STATE_HOVER:
> + case TARGET_STATE_TOUCHING:
> + {
> + target_prev_state->insert = true;
> + update = true;
> + input_mt_slot(input_dev, slot);
> + input_report_abs(input_dev, ABS_MT_TRACKING_ID, slot);
> + input_report_abs(input_dev, ABS_MT_POSITION_X,
> + target->x);
> + input_report_abs(input_dev, ABS_X, target->x);
> + input_report_abs(input_dev, ABS_MT_POSITION_Y,
> + target->y);
> + input_report_abs(input_dev, ABS_Y, target->y);
> +
> + if (current_state == TARGET_STATE_TOUCHING) {
> + input_report_abs(input_dev, ABS_MT_DISTANCE, 0);
> + input_report_abs(input_dev, ABS_DISTANCE, 0);
> + input_report_abs(input_dev, ABS_MT_PRESSURE,
> + target->z);
> + input_report_abs(input_dev, ABS_PRESSURE,
> + target->z);
> + } else {
> + input_report_abs(input_dev, ABS_MT_DISTANCE,
> + -target->z);
> + input_report_abs(input_dev, ABS_DISTANCE,
> + -target->z);
> + input_report_abs(input_dev, ABS_MT_PRESSURE, 0);
> + input_report_abs(input_dev, ABS_PRESSURE, 0);
> + }
> +
> + if (slot == 0)
> + input_report_key(input_dev, BTN_LEFT,
> + (current_state ==
> + TARGET_STATE_TOUCHING));
> +
> + break;
> + }
> + }
> +
> + target_prev_state->state = current_state;
> + target_prev_state->x = target->x;
> + target_prev_state->y = target->y;
> + target_prev_state->z = target->z;
> +
> + if (update)
> + input_mt_sync_frame(input_dev);
> +
> + return update;
> +}
> +
> +/**
> + * Take a raw buffer with u41 report data and decode it.
> + * Also generate input events if needed.
> + * @rx_buf: ptr to a byte array [0]: Usage number [1]: Status LSB [2]: Status MSB
> + */
> +void axiom_process_u41_report(struct axiom_data *ts, char *rx_buf)
> +{
> + struct axiom_target_report target;
> + struct input_dev *input_dev = ts->input_dev;
> + bool update_done = false;
> + u16 target_status;
> + u32 i;
> +
> + if (rx_buf[0] != 0x41) {
> + dev_err(ts->pdev,
> + "Data in buffer does not have expected u41 format.\n");
> + return;
> + }
> +
> + target_status = ((rx_buf[1]) | (rx_buf[2] << 8));
> +
> + for (i = 0; i < U41_MAX_TARGETS; i++) {
> + target.index = i;
> + target.present = ((target_status & (1 << i)) != 0) ? 1 : 0;
> + target.x = (rx_buf[(i * 4) + 3]) | (rx_buf[(i * 4) + 4] << 8);
> + target.y = (rx_buf[(i * 4) + 5]) | (rx_buf[(i * 4) + 6] << 8);
> + target.z = (s8) (rx_buf[i + 43]);
> + update_done |= axiom_process_u41_report_target(ts, &target);
> + }
> +
> + if (update_done)
> + input_sync(input_dev);
> +}
> +EXPORT_SYMBOL_GPL(axiom_process_u41_report);
> +
> +void axiom_process_u46_report(struct axiom_data *ts, char *rx_buf)
> +{
> + struct input_dev *input_dev = ts->input_dev;
> + u16 aux_value;
> + u32 event_value;
> + u32 i = 0;
> +
> + for (i = 0; i < U46_AUX_CHANNELS; i++) {
> + aux_value =
> + ((rx_buf[(i * 2) + 2] << 8) | (rx_buf[(i * 2) + 1])) &
> + U46_AUX_MASK;
> + event_value = (i << 16) | (aux_value);
> + input_event(input_dev, EV_MSC, MSC_RAW, event_value);
> + }
> +
> + input_mt_sync(input_dev);
> + input_sync(input_dev);
> +}
> +EXPORT_SYMBOL_GPL(axiom_process_u46_report);
> +
> +/**
> + * Support function to axiom_process_report.
> + * It validates the crc and multiplexes the axiom reports to the appropriate
> + * report handler
> + */
> +void axiom_process_report(struct axiom_data *ts, char *report_data)
> +{
> + struct device *pdev = ts->pdev;
> + char usage = report_data[1];
> + u16 crc_report;
> + u16 crc_calc;
> + char len;
> +
> + if ((report_data[0] & COMMS_OVERFLOW_MSK) != 0)
> + ts->report_overflow_counter++;
> +
> + len = (report_data[0] & COMMS_REPORT_LEN_MSK) << 1;
> + if (len == 0) {
> + dev_err(pdev, "Zero length report discarded.\n");
> + return;
> + }
> +
> + dev_dbg(pdev, "Payload Data %*ph\n", len, report_data);
> +
> + /* Validate the report CRC */
> + crc_report = (report_data[len - 1] << 8) | (report_data[len - 2]);
> + /* Length is in 16 bit words and remove the size of the CRC16 itself */
> + crc_calc = crc16(0, report_data, (len - 2));
> +
> + if (crc_calc != crc_report) {
> + dev_err(pdev,
> + "CRC mismatch! Expected: %#x, Calculated CRC: %#x.\n",
> + crc_report, crc_calc);
> + return;
> + }
> +
> + switch (usage) {
> + case USAGE_2DCTS_REPORT_ID:
> + axiom_process_u41_report(ts, &report_data[1]);
> + break;
> +
> + case USAGE_2AUX_REPORT_ID:
> + /* This is an aux report (force) */
> + axiom_process_u46_report(ts, &report_data[1]);
> + break;
> +
> + case USAGE_2HB_REPORT_ID:
> + /* This is a heartbeat report */
> + break;
> +
> + default:
> + break;
> + }
> +
> + ts->report_counter++;
> +}
> +EXPORT_SYMBOL_GPL(axiom_process_report);
> +
> +MODULE_AUTHOR("Kamel Bouhara <kamel.bouhara@bootlin.com>");
> +MODULE_DESCRIPTION("aXiom touchscreen core logic");
> +MODULE_LICENSE("GPL");
> +MODULE_ALIAS("axiom");
> diff --git a/drivers/input/touchscreen/axiom_core.h b/drivers/input/touchscreen/axiom_core.h
> new file mode 100644
> index 000000000000..f129d28671ff
> --- /dev/null
> +++ b/drivers/input/touchscreen/axiom_core.h
> @@ -0,0 +1,140 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * TouchNetix aXiom Touchscreen Driver
> + *
> + * Copyright (C) 2020-2023 TouchNetix Ltd.
> + *
> + * Author(s): Mark Satterthwaite <mark.satterthwaite@touchnetix.com>
> + * Pedro Torruella <pedro.torruella@touchnetix.com>
> + * Bart Prescott <bartp@baasheep.co.uk>
> + * Hannah Rossiter <hannah.rossiter@touchnetix.com>
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License as published by the
> + * Free Software Foundation; either version 2 of the License, or (at your
> + * option) any later version.
> + *
> + */
> +
> +#ifndef __AXIOM_CORE_H
> +#define __AXIOM_CORE_H
> +
> +/* Enables the raw data for up to 4 force channels to be sent to the input subsystem */
> +#define U46_ENABLE_RAW_FORCE_DATA
> +
> +/**
> + * u31 has 2 pages for usage table entries.
> + * (2 * AX_COMMS_PAGE_SIZE) / U31_BYTES_PER_USAGE = 85
> + */
> +#define U31_MAX_USAGES (85U)
> +#define U41_MAX_TARGETS (10U)
> +#define U46_AUX_CHANNELS (4U)
> +#define U46_AUX_MASK (0xFFFU)
> +#define U31_BYTES_PER_USAGE (6U)
> +#define USAGE_2DCTS_REPORT_ID (0x41U)
> +#define USAGE_2AUX_REPORT_ID (0x46U)
> +#define USAGE_2HB_REPORT_ID (0x01U)
> +#define PROX_LEVEL (-128)
> +#define AX_U31_PAGE0_LENGTH (0x0C)
> +#define AX_COMMS_WRITE (0x00U)
> +#define AX_COMMS_READ (0x80U)
> +#define AX_COMMS_BYTES_MASK (0xFFU)
> +
> +#define DEVINFO_USAGE_ID 0x31
> +#define REPORT_USAGE_ID 0x34
> +
> +#define REBASELINE_CMD 0x03
> +
> +#define COMMS_MAX_USAGE_PAGES (3)
> +#define AX_COMMS_PAGE_SIZE (256)
> +
> +#define COMMS_OVERFLOW_MSK (0x80)
> +#define COMMS_REPORT_LEN_MSK (0x7F)
The defines look not good, please use proper kernel coding style. Also
I'm not sure if we should follow the downstream solution here. Of course
there is this concept of usages, pages and offsets:
/ reg0 (0x0)
/ page-0 (0x0) -------+ reg1 (0x1)
| | ...
u(sage)31 --+ page-1 (0x1) \ reg127 (0xff)
|
\ page-2 (0x2)
But in the end it is just a 16bit register and we can access is
partially. We only need to know the register, the len-bytes we have to
read/write and the reg-mask we may need to apply.
#define AXIOM_PAGE_MASK GENMASK(15, 8)
#define AXIOM_PAGE_OFFSET_MASK GENMASK(7, 0)
struct axiom_reg {
u16 reg;
u16 len;
u32 mask;
}
#define AXIOM_REG(_page, _offset_bytes, _len_bytes, _mask) { \
.reg = FIELD_PREP(AXIOM_PAGE_MASK, _page) | \
FIELD_PREP(AXIOM_PAGE_OFFSET_MASK, _offset_bytes), \
.len = _len_bytes, \
.mask =_ mask, \
}
enum axiom_reg_desc {
AXIOM_U31_DEVICE_ID,
AXIOM_U31_MODE,
AXIOM_U31_RUTNIME_FW_VARIANT,
AXIOM_U31_RUTNIME_FW_STATUS,
};
static struct axiom_reg axiom_reg[] = {
[AXIOM_U31_DEVICE_ID] = AXIOM_REG(0, 0, 2, GENMASK(14, 0)),
[AXIOM_U31_MODE] = AXIOM_REG(0, 1, 1, BIT(7)),
[AXIOM_U31_RUTNIME_FW_VARIANT] = AXIOM_REG(0, 4, 1, GENMASK(6, 0)),
[AXIOM_U31_RUTNIME_FW_STATUS] = AXIOM_REG(0, 4, 1, BIT(7)),
};
Of course this does not cover the event read case but all the other
cases and would simplify the decoding or just use the common pattern
like:
#define AXIOM_REG(page, offset) \
(FIELD_PREP(AXIOM_PAGE_MASK, (page)) | \
FIELD_PREP(AXIOM_PAGE_OFFSET_MASK, (offset)))
#define AXIOM_U31_DEVICE_ID_REG AXIOM_REG(0, 0)
#define AXIOM_U31_DEVICE_ID_MASK GEMASK(14, 0)
#define AXIOM_U31_MODE_REG AXIOM_REG(0, 1)
#define AXIOM_U31_MODE_MASK BIT(7)
#define AXIOM_U31_RUTNIME_FW_REG AXIOM_REG(0, 4)
#define AXIOM_U31_RUTNIME_FW_STATUS_MASK BIT(7)
#define AXIOM_U31_RUTNIME_FW_VARIANT_MASK GENMASK(6, 0)
so in the end we can could use:
- axiom_read(priv, AXIOM_U31_DEVICE_ID) or
- axiom_read(priv, AXIOM_U31_DEVICE_ID_REG, 2);
not sure which one does fit better but IMHO just copy the downstream
version isn't the way we should go.
Regards,
Marco
> +#include <linux/input.h>
> +
> +struct axiom_devinfo {
> + char bootloader_fw_ver_major;
> + char bootloader_fw_ver_minor;
> + char bootloader_mode;
> + u16 device_id;
> + char fw_major;
> + char fw_minor;
> + u16 fw_info_extra;
> + u16 jedec_id;
> + char num_usages;
> + char silicon_revision;
> +};
> +
> +/**
> + * Describes parameters of a specific usage, essenstially a single element of
> + * the "Usage Table"
> + */
> +struct usage_entry {
> + char id;
> + char is_report;
> + char start_page;
> + char num_pages;
> +};
> +
> +/**
> + * Holds state of a "Target", A.K.A. as a "touch", but called a target as it
> + * can be a detected "target" prior to touch, eg, hovering.
> + */
> +enum axiom_target_state {
> + TARGET_STATE_NOT_PRESENT = 0,
> + TARGET_STATE_PROX = 1,
> + TARGET_STATE_HOVER = 2,
> + TARGET_STATE_TOUCHING = 3,
> + TARGET_STATE_MIN = TARGET_STATE_NOT_PRESENT,
> + TARGET_STATE_MAX = TARGET_STATE_TOUCHING,
> +};
> +
> +struct u41_target {
> + enum axiom_target_state state;
> + u16 x;
> + u16 y;
> + s8 z;
> + bool insert;
> + bool touch;
> +};
> +
> +struct axiom_target_report {
> + u8 index;
> + u8 present;
> + u16 x;
> + u16 y;
> + s8 z;
> +};
> +
> +struct axiom_cmd_header {
> + u16 target_address;
> + u16 length:15;
> + u16 read:1;
> + char write_data[];
> +};
> +
> +struct axiom_data {
> + struct axiom_devinfo devinfo;
> + struct device *pdev;
> + struct gpio_desc *reset_gpio;
> + struct gpio_desc *irq_gpio;
> + struct i2c_client *client;
> + struct input_dev *input_dev;
> + char max_report_len;
> + u32 report_overflow_counter;
> + u32 report_counter;
> + char rx_buf[COMMS_MAX_USAGE_PAGES * AX_COMMS_PAGE_SIZE];
> + struct u41_target targets[U41_MAX_TARGETS];
> + struct usage_entry usage_table[U31_MAX_USAGES];
> + bool usage_table_populated;
> +};
> +
> +extern u16 usage_to_target_address(struct axiom_data *ts, char usage,
> + char page, char offset);
> +extern void axiom_process_report(struct axiom_data *ts, char *report_data);
> +extern char axiom_populate_usage_table(struct axiom_data *ts, char *rx_data);
> +extern void axiom_remove(struct axiom_data *ts);
> +extern void axiom_get_dev_info(struct axiom_data *ts, char *info);
> +
> +#endif /* __AXIOM_CORE_H */
> diff --git a/drivers/input/touchscreen/axiom_i2c.c b/drivers/input/touchscreen/axiom_i2c.c
> new file mode 100644
> index 000000000000..ddb898ad3744
> --- /dev/null
> +++ b/drivers/input/touchscreen/axiom_i2c.c
> @@ -0,0 +1,349 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * TouchNetix aXiom Touchscreen Driver
> + *
> + * Copyright (C) 2020-2023 TouchNetix Ltd.
> + *
> + * Author(s): Bart Prescott <bartp@baasheep.co.uk>
> + * Pedro Torruella <pedro.torruella@touchnetix.com>
> + * Mark Satterthwaite <mark.satterthwaite@touchnetix.com>
> + * Hannah Rossiter <hannah.rossiter@touchnetix.com>
> + *
> + */
> +
> +#include "axiom_core.h"
> +
> +#include <linux/delay.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/i2c.h>
> +#include <linux/input.h>
> +#include <linux/input/mt.h>
> +#include <linux/interrupt.h>
> +#include <linux/irqreturn.h>
> +#include <linux/kernel.h>
> +#include <linux/kobject.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/pm.h>
> +#include <linux/string.h>
> +
> +/**
> + * aXiom devices are typically configured to report
> + * touches at a rate of 100Hz (10ms). For systems
> + * that require polling for reports, 100ms seems like
> + * an acceptable polling rate.
> + * When reports are polled, it will be expected to
> + * occasionally observe the overflow bit being set
> + * in the reports. This indicates that reports are not
> + * being read fast enough.
> + */
> +#define POLL_INTERVAL_DEFAULT_MS 100
> +
> +static int
> +axiom_i2c_read(struct i2c_client *client, u8 usage, u8 page, u8 *buf, u16 len)
> +{
> + struct axiom_cmd_header cmd_header;
> + struct axiom_data *ts = i2c_get_clientdata(client);
> + struct i2c_msg msg[2];
> + int ret;
> +
> + /* Build the header */
> + cmd_header.target_address = usage_to_target_address(ts, usage, page, 0);
> + cmd_header.length = len;
> + cmd_header.read = 1;
> +
> + msg[0].addr = client->addr;
> + msg[0].flags = 0;
> + msg[0].len = sizeof(cmd_header);
> + msg[0].buf = (u8 *)&cmd_header;
> +
> + msg[1].addr = client->addr;
> + msg[1].flags = I2C_M_RD;
> + msg[1].len = len;
> + msg[1].buf = (char *)buf;
> +
> + ret = i2c_transfer(client->adapter, msg, 2);
> + if (ret != ARRAY_SIZE(msg)) {
> + dev_err(&client->dev,
> + "Failed reading usage %#x page %#x, error=%d\n", usage,
> + page, ret);
> + return -EIO;
> + }
> +
> + return 0;
> +}
> +
> +static int
> +axiom_i2c_write(struct i2c_client *client, u8 usage, u8 page, u8 *buf, u16 len)
> +{
> + struct axiom_cmd_header cmd_header;
> + struct axiom_data *ts = i2c_get_clientdata(client);
> + struct i2c_msg msg[2];
> + int ret;
> +
> + cmd_header.target_address = usage_to_target_address(ts, usage, page, 0);
> + cmd_header.length = len;
> + cmd_header.read = 0;
> +
> + msg[0].addr = client->addr;
> + msg[0].flags = 0;
> + msg[0].len = sizeof(cmd_header);
> + msg[0].buf = (u8 *)&cmd_header;
> +
> + msg[1].addr = client->addr;
> + msg[1].flags = 0;
> + msg[1].len = len;
> + msg[1].buf = (char *)buf;
> +
> + ret = i2c_transfer(client->adapter, msg, 2);
> + if (ret != 2) {
> + dev_err(&client->dev,
> + "Failed to write usage %#x page %#x, error=%d\n", usage,
> + page, ret);
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static void axiom_i2c_poll(struct input_dev *input_dev)
> +{
> + struct axiom_data *ts = input_get_drvdata(input_dev);
> + char *rx_data = ts->rx_buf;
> +
> + axiom_i2c_read(ts->client, REPORT_USAGE_ID, 0, rx_data,
> + ts->max_report_len);
> + axiom_process_report(ts, rx_data);
> +}
> +
> +/**
> + * Retrieve, store and print the axiom device information
> + */
> +int axiom_discover(struct axiom_data *ts)
> +{
> + char *rx_data = &ts->rx_buf[0];
> + struct device *pdev = ts->pdev;
> + int ret;
> +
> + /* First the first page of u31 to get the device information and */
> + /* the number of usages */
> + ret =
> + axiom_i2c_read(ts->client, DEVINFO_USAGE_ID, 0, rx_data,
> + AX_U31_PAGE0_LENGTH);
> + if (ret)
> + return ret;
> +
> + axiom_get_dev_info(ts, rx_data);
> +
> + dev_dbg(pdev, "Data Decode:\n");
> + dev_dbg(pdev, " Bootloader Mode: %u\n", ts->devinfo.bootloader_mode);
> + dev_dbg(pdev, " Device ID : %04x\n", ts->devinfo.device_id);
> + dev_dbg(pdev, " Firmware Rev : %02x.%02x\n", ts->devinfo.fw_major,
> + ts->devinfo.fw_minor);
> + dev_dbg(pdev, " Bootloader Rev : %02x.%02x\n",
> + ts->devinfo.bootloader_fw_ver_major,
> + ts->devinfo.bootloader_fw_ver_minor);
> + dev_dbg(pdev, " FW Extra Info : %04x\n", ts->devinfo.fw_info_extra);
> + dev_dbg(pdev, " Silicon : %02x\n", ts->devinfo.jedec_id);
> + dev_dbg(pdev, " Num Usages : %04x\n", ts->devinfo.num_usages);
> +
> + /* Read the second page of u31 to get the usage table */
> + ret = axiom_i2c_read(ts->client, DEVINFO_USAGE_ID, 1, rx_data,
> + (U31_BYTES_PER_USAGE * ts->devinfo.num_usages));
> + if (ret)
> + return ret;
> +
> + ts->max_report_len = axiom_populate_usage_table(ts, rx_data);
> + dev_dbg(pdev, "Max Report Length: %u\n", ts->max_report_len);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(axiom_discover);
> +
> +/**
> + * Rebaseline the touchscreen, effectively zero-ing it
> + */
> +void axiom_rebaseline(struct axiom_data *ts)
> +{
> + struct device *pdev = ts->pdev;
> + char buffer[8] = { 0 };
> + int ret;
> +
> + memset(buffer, 0, sizeof(buffer));
> +
> + buffer[0] = REBASELINE_CMD;
> +
> + ret = axiom_i2c_write(ts->client, 0x02, 0, buffer, sizeof(buffer));
> + if (ret)
> + dev_err(pdev, "Rebaseline failed\n");
> +
> + dev_info(pdev, "Capture Baseline Requested\n");
> +}
> +EXPORT_SYMBOL_GPL(axiom_rebaseline);
> +
> +static irqreturn_t axiom_irq(int irq, void *handle)
> +{
> + struct axiom_data *ts = handle;
> + u8 *rx_data = &ts->rx_buf[0];
> +
> + axiom_i2c_read(ts->client, REPORT_USAGE_ID, 0, rx_data, ts->max_report_len);
> + axiom_process_report(ts, rx_data);
> +
> + return IRQ_HANDLED;
> +}
> +
> +static int axiom_i2c_probe(struct i2c_client *client)
> +{
> + struct axiom_data *ts;
> + struct device *pdev = &client->dev;
> + struct input_dev *input_dev;
> + int error;
> + int target;
> +
> + ts = devm_kzalloc(pdev, sizeof(*ts), GFP_KERNEL);
> + if (!ts)
> + return -ENOMEM;
> +
> + ts->irq_gpio = devm_gpiod_get_optional(pdev, "irq", GPIOD_IN);
> + if (IS_ERR(ts->irq_gpio)) {
> + error = PTR_ERR(ts->irq_gpio);
> + dev_err(pdev, "failed to request irq GPIO: %d", error);
> + return error;
> + }
> +
> + ts->reset_gpio = devm_gpiod_get_optional(pdev, "reset", GPIOD_OUT_HIGH);
> + if (IS_ERR(ts->reset_gpio)) {
> + error = PTR_ERR(ts->reset_gpio);
> + dev_err(pdev, "failed to request reset GPIO: %d", error);
> + return error;
> + }
> +
> + if (ts->irq_gpio) {
> + error =
> + devm_request_threaded_irq(pdev, client->irq, NULL,
> + axiom_irq,
> + IRQF_TRIGGER_LOW | IRQF_ONESHOT,
> + "axiom_irq", ts);
> + if (error != 0) {
> + dev_err(pdev, "Failed to request IRQ %u (error: %d)\n",
> + client->irq, error);
> + return error;
> + }
> + }
> +
> + ts->client = client;
> + ts->pdev = pdev;
> + ts->usage_table_populated = false;
> +
> + i2c_set_clientdata(client, ts);
> +
> + axiom_discover(ts);
> + axiom_rebaseline(ts);
> +
> + input_dev = devm_input_allocate_device(ts->pdev);
> + if (!input_dev) {
> + dev_err(pdev, "Failed to allocate input device\n");
> + return -ENOMEM;
> + }
> +
> + input_dev->name = "TouchNetix aXiom Touchscreen";
> + input_dev->phys = "input/axiom_ts";
> +
> + /* Single Touch */
> + input_set_abs_params(input_dev, ABS_X, 0, 65535, 0, 0);
> + input_set_abs_params(input_dev, ABS_Y, 0, 65535, 0, 0);
> +
> + /* Multi Touch */
> + /* Min, Max, Fuzz (expected noise in px, try 4?) and Flat */
> + input_set_abs_params(input_dev, ABS_MT_POSITION_X, 0, 65535, 0, 0);
> + /* Min, Max, Fuzz (expected noise in px, try 4?) and Flat */
> + input_set_abs_params(input_dev, ABS_MT_POSITION_Y, 0, 65535, 0, 0);
> + input_set_abs_params(input_dev, ABS_MT_TOOL_TYPE, 0, MT_TOOL_MAX, 0, 0);
> + input_set_abs_params(input_dev, ABS_MT_DISTANCE, 0, 127, 0, 0);
> + input_set_abs_params(input_dev, ABS_MT_PRESSURE, 0, 127, 0, 0);
> +
> + /* Registers the axiom device as a touch screen instead of as a mouse pointer */
> + input_mt_init_slots(input_dev, U41_MAX_TARGETS, INPUT_MT_DIRECT);
> +
> + input_set_capability(input_dev, EV_KEY, BTN_LEFT);
> +
> + /* Enables the raw data for up to 4 force channels to be sent to the */
> + /* input subsystem */
> + set_bit(EV_REL, input_dev->evbit);
> + set_bit(EV_MSC, input_dev->evbit);
> + /* Declare that we support "RAW" Miscellaneous events */
> + set_bit(MSC_RAW, input_dev->mscbit);
> +
> + if (!ts->irq_gpio) {
> + error = input_setup_polling(input_dev, axiom_i2c_poll);
> + if (error) {
> + dev_err(ts->pdev, "Unable to set up polling: %d\n",
> + error);
> + return error;
> + }
> + input_set_poll_interval(input_dev, POLL_INTERVAL_DEFAULT_MS);
> + }
> +
> + error = input_register_device(input_dev);
> + if (error != 0) {
> + dev_err(ts->pdev,
> + "Could not register with Input Sub-system., error=%d\n",
> + error);
> + return error;
> + }
> +
> + ts->input_dev = input_dev;
> + input_set_drvdata(ts->input_dev, ts);
> +
> + dev_info(pdev, "AXIOM: I2C driver registered with Input Sub-System.\n");
> +
> + /* Delay just a smidge before enabling the IRQ */
> + udelay(10);
> +
> + /* Ensure that all reports are initialised to not be present. */
> + for (target = 0; target < U41_MAX_TARGETS; target++)
> + ts->targets[target].state = TARGET_STATE_NOT_PRESENT;
> +
> + return 0;
> +}
> +
> +static void axiom_i2c_remove(struct i2c_client *client)
> +{
> + struct axiom_data *ts = i2c_get_clientdata(client);
> +
> + axiom_remove(ts);
> +}
> +
> +static const struct i2c_device_id axiom_i2c_id_table[] = {
> + {"ax54a"},
> + {},
> +};
> +
> +MODULE_DEVICE_TABLE(i2c, axiom_i2c_id_table);
> +
> +static const struct of_device_id axiom_i2c_dt_ids[] = {
> + {
> + .compatible = "axiom,ax54a",
> + .data = "axiom",
> + },
> + {}
> +};
> +
> +MODULE_DEVICE_TABLE(of, axiom_i2c_dt_ids);
> +
> +static struct i2c_driver axiom_i2c_driver = {
> + .driver = {
> + .name = "axiom_i2c",
> + .of_match_table = of_match_ptr(axiom_i2c_dt_ids),
> + },
> + .id_table = axiom_i2c_id_table,
> + .probe = axiom_i2c_probe,
> + .remove = axiom_i2c_remove,
> +};
> +
> +module_i2c_driver(axiom_i2c_driver);
> +
> +MODULE_AUTHOR("Kamel Bouhara <kamel.bouhara@bootlin.com>");
> +MODULE_DESCRIPTION("aXiom touchscreen I2C bus driver");
> +MODULE_LICENSE("GPL");
> +MODULE_ALIAS("axiom");
> --
> 2.25.1
>
^ permalink raw reply
* [dtor-input:for-linus] BUILD SUCCESS e28a0974d749e5105d77233c0a84d35c37da047e
From: kernel test robot @ 2023-09-18 17:54 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: linux-input
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git for-linus
branch HEAD: e28a0974d749e5105d77233c0a84d35c37da047e Input: xpad - add HyperX Clutch Gladiate Support
elapsed time: 726m
configs tested: 148
configs skipped: 2
The following configs have been built successfully.
More configs may be tested in the coming days.
tested configs:
alpha allnoconfig gcc
alpha allyesconfig gcc
alpha defconfig gcc
arc allmodconfig gcc
arc allnoconfig gcc
arc allyesconfig gcc
arc defconfig gcc
arc haps_hs_defconfig gcc
arc randconfig-001-20230918 gcc
arm allmodconfig gcc
arm allnoconfig gcc
arm allyesconfig gcc
arm defconfig gcc
arm randconfig-001-20230918 gcc
arm64 allmodconfig gcc
arm64 allnoconfig gcc
arm64 allyesconfig gcc
arm64 defconfig gcc
csky allmodconfig gcc
csky allnoconfig gcc
csky allyesconfig gcc
csky defconfig gcc
i386 allmodconfig gcc
i386 allnoconfig gcc
i386 allyesconfig gcc
i386 buildonly-randconfig-001-20230918 gcc
i386 buildonly-randconfig-002-20230918 gcc
i386 buildonly-randconfig-003-20230918 gcc
i386 buildonly-randconfig-004-20230918 gcc
i386 buildonly-randconfig-005-20230918 gcc
i386 buildonly-randconfig-006-20230918 gcc
i386 debian-10.3 gcc
i386 defconfig gcc
i386 randconfig-001-20230918 gcc
i386 randconfig-002-20230918 gcc
i386 randconfig-003-20230918 gcc
i386 randconfig-004-20230918 gcc
i386 randconfig-005-20230918 gcc
i386 randconfig-006-20230918 gcc
i386 randconfig-011-20230918 gcc
i386 randconfig-012-20230918 gcc
i386 randconfig-013-20230918 gcc
i386 randconfig-014-20230918 gcc
i386 randconfig-015-20230918 gcc
i386 randconfig-016-20230918 gcc
loongarch allmodconfig gcc
loongarch allnoconfig gcc
loongarch allyesconfig gcc
loongarch defconfig gcc
loongarch randconfig-001-20230918 gcc
loongarch randconfig-001-20230919 gcc
m68k allmodconfig gcc
m68k allnoconfig gcc
m68k allyesconfig gcc
m68k defconfig gcc
microblaze allmodconfig gcc
microblaze allnoconfig gcc
microblaze allyesconfig gcc
microblaze defconfig gcc
mips allmodconfig gcc
mips allnoconfig gcc
mips allyesconfig gcc
mips decstation_64_defconfig gcc
nios2 allmodconfig gcc
nios2 allnoconfig gcc
nios2 allyesconfig gcc
nios2 defconfig gcc
openrisc allmodconfig gcc
openrisc allnoconfig gcc
openrisc allyesconfig gcc
openrisc defconfig gcc
parisc allmodconfig gcc
parisc allnoconfig gcc
parisc allyesconfig gcc
parisc defconfig gcc
parisc64 defconfig gcc
powerpc allmodconfig gcc
powerpc allnoconfig gcc
powerpc allyesconfig clang
powerpc allyesconfig gcc
powerpc ppa8548_defconfig clang
riscv allmodconfig gcc
riscv allnoconfig gcc
riscv allyesconfig gcc
riscv defconfig gcc
riscv randconfig-001-20230918 gcc
riscv rv32_defconfig gcc
s390 allmodconfig gcc
s390 allnoconfig gcc
s390 allyesconfig gcc
s390 defconfig gcc
s390 randconfig-001-20230918 gcc
sh allmodconfig gcc
sh allnoconfig gcc
sh allyesconfig gcc
sh defconfig gcc
sh sh7710voipgw_defconfig gcc
sh sh7785lcr_32bit_defconfig gcc
sparc allmodconfig gcc
sparc allnoconfig gcc
sparc allyesconfig gcc
sparc defconfig gcc
sparc randconfig-001-20230918 gcc
sparc64 allmodconfig gcc
sparc64 allyesconfig gcc
sparc64 defconfig gcc
um allmodconfig clang
um allnoconfig clang
um allyesconfig clang
um defconfig gcc
um i386_defconfig gcc
um x86_64_defconfig gcc
x86_64 allnoconfig gcc
x86_64 allyesconfig gcc
x86_64 buildonly-randconfig-001-20230918 gcc
x86_64 buildonly-randconfig-002-20230918 gcc
x86_64 buildonly-randconfig-003-20230918 gcc
x86_64 buildonly-randconfig-004-20230918 gcc
x86_64 buildonly-randconfig-005-20230918 gcc
x86_64 buildonly-randconfig-006-20230918 gcc
x86_64 defconfig gcc
x86_64 kexec gcc
x86_64 randconfig-001-20230918 gcc
x86_64 randconfig-002-20230918 gcc
x86_64 randconfig-003-20230918 gcc
x86_64 randconfig-004-20230918 gcc
x86_64 randconfig-005-20230918 gcc
x86_64 randconfig-006-20230918 gcc
x86_64 randconfig-011-20230918 gcc
x86_64 randconfig-012-20230918 gcc
x86_64 randconfig-013-20230918 gcc
x86_64 randconfig-014-20230918 gcc
x86_64 randconfig-015-20230918 gcc
x86_64 randconfig-016-20230918 gcc
x86_64 randconfig-071-20230918 gcc
x86_64 randconfig-072-20230918 gcc
x86_64 randconfig-073-20230918 gcc
x86_64 randconfig-074-20230918 gcc
x86_64 randconfig-075-20230918 gcc
x86_64 randconfig-076-20230918 gcc
x86_64 rhel-8.3-bpf gcc
x86_64 rhel-8.3-func gcc
x86_64 rhel-8.3-kselftests gcc
x86_64 rhel-8.3-kunit gcc
x86_64 rhel-8.3-ltp gcc
x86_64 rhel-8.3-rust clang
x86_64 rhel-8.3 gcc
xtensa allnoconfig gcc
xtensa allyesconfig gcc
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* Re: [PATCH v2 2/3] selftests/hid: do not manually call headers_install
From: Shuah Khan @ 2023-09-18 17:24 UTC (permalink / raw)
To: Justin Stitt, Jiri Kosina, Benjamin Tissoires, Shuah Khan
Cc: Eduard Zingerman, linux-input, linux-kselftest, linux-kernel, bpf,
Benjamin Tissoires, Shuah Khan
In-Reply-To: <20230908-kselftest-09-08-v2-2-0def978a4c1b@google.com>
On 9/8/23 16:22, Justin Stitt wrote:
> From: Benjamin Tissoires <bentiss@kernel.org>
>
> "make headers" is a requirement before calling make on the selftests
> dir, so we should not have to manually install those headers
>
> Signed-off-by: Benjamin Tissoires <bentiss@kernel.org>
Thank for making this change. Just check bpf continues to
compile and run though.
Acked-by: Shuah Khan <skhan@linuxfoundation.org>
thanks,
-- Shuah
^ permalink raw reply
* [PATCH RESEND] HID: i2c-hid: fix handling of unpopulated devices
From: Johan Hovold @ 2023-09-18 16:35 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires
Cc: linux-input, linux-kernel, Johan Hovold, Douglas Anderson,
Maxime Ripard
In-Reply-To: <CAD=FV=Wfwvp-SbGrdO5VJcjG42njkApJPB7wnY-YYa1_-O0JWQ@mail.gmail.com>
A recent commit reordered probe so that the interrupt line is now
requested before making sure that the device exists.
This breaks machines like the Lenovo ThinkPad X13s which rely on the
HID driver to probe second-source devices and only register the variant
that is actually populated. Specifically, the interrupt line may now
already be (temporarily) claimed when doing asynchronous probing of the
touchpad:
genirq: Flags mismatch irq 191. 00082008 (hid-over-i2c) vs. 00082008 (hid-over-i2c)
i2c_hid_of 21-0015: Could not register for hid-over-i2c interrupt, irq = 191, ret = -16
i2c_hid_of: probe of 21-0015 failed with error -16
Fix this by restoring the old behaviour of first making sure the device
exists before requesting the interrupt line.
Note that something like this should probably be implemented also for
"panel followers", whose actual probe is currently effectively deferred
until the DRM panel is probed (e.g. by powering down the device after
making sure it exists and only then register it as a follower).
Fixes: 675cd877c952 ("HID: i2c-hid: Rearrange probe() to power things up later")
Cc: Douglas Anderson <dianders@chromium.org>
Cc: Maxime Ripard <mripard@kernel.org>
Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
---
This one still hasn't shown up in lore so resending as a reply to Doug's
reply for completeness.
Johan
drivers/hid/i2c-hid/i2c-hid-core.c | 142 ++++++++++++++++-------------
1 file changed, 80 insertions(+), 62 deletions(-)
diff --git a/drivers/hid/i2c-hid/i2c-hid-core.c b/drivers/hid/i2c-hid/i2c-hid-core.c
index 9601c0605fd9..e66c058a4b00 100644
--- a/drivers/hid/i2c-hid/i2c-hid-core.c
+++ b/drivers/hid/i2c-hid/i2c-hid-core.c
@@ -998,45 +998,29 @@ static int i2c_hid_core_resume(struct i2c_hid *ihid)
return hid_driver_reset_resume(hid);
}
-/**
- * __do_i2c_hid_core_initial_power_up() - First time power up of the i2c-hid device.
- * @ihid: The ihid object created during probe.
- *
- * This function is called at probe time.
- *
- * The initial power on is where we do some basic validation that the device
- * exists, where we fetch the HID descriptor, and where we create the actual
- * HID devices.
- *
- * Return: 0 or error code.
+/*
+ * Check that the device exists and parse the HID descriptor.
*/
-static int __do_i2c_hid_core_initial_power_up(struct i2c_hid *ihid)
+static int __i2c_hid_core_probe(struct i2c_hid *ihid)
{
struct i2c_client *client = ihid->client;
struct hid_device *hid = ihid->hid;
int ret;
- ret = i2c_hid_core_power_up(ihid);
- if (ret)
- return ret;
-
/* Make sure there is something at this address */
ret = i2c_smbus_read_byte(client);
if (ret < 0) {
i2c_hid_dbg(ihid, "nothing at this address: %d\n", ret);
- ret = -ENXIO;
- goto err;
+ return -ENXIO;
}
ret = i2c_hid_fetch_hid_descriptor(ihid);
if (ret < 0) {
dev_err(&client->dev,
"Failed to fetch the HID Descriptor\n");
- goto err;
+ return ret;
}
- enable_irq(client->irq);
-
hid->version = le16_to_cpu(ihid->hdesc.bcdVersion);
hid->vendor = le16_to_cpu(ihid->hdesc.wVendorID);
hid->product = le16_to_cpu(ihid->hdesc.wProductID);
@@ -1050,17 +1034,49 @@ static int __do_i2c_hid_core_initial_power_up(struct i2c_hid *ihid)
ihid->quirks = i2c_hid_lookup_quirk(hid->vendor, hid->product);
+ return 0;
+}
+
+static int i2c_hid_core_register_hid(struct i2c_hid *ihid)
+{
+ struct i2c_client *client = ihid->client;
+ struct hid_device *hid = ihid->hid;
+ int ret;
+
+ enable_irq(client->irq);
+
ret = hid_add_device(hid);
if (ret) {
if (ret != -ENODEV)
hid_err(client, "can't add hid device: %d\n", ret);
- goto err;
+ disable_irq(client->irq);
+ return ret;
}
return 0;
+}
+
+static int i2c_hid_core_probe_panel_follower(struct i2c_hid *ihid)
+{
+ int ret;
-err:
+ ret = i2c_hid_core_power_up(ihid);
+ if (ret)
+ return ret;
+
+ ret = __i2c_hid_core_probe(ihid);
+ if (ret)
+ goto err_power_down;
+
+ ret = i2c_hid_core_register_hid(ihid);
+ if (ret)
+ goto err_power_down;
+
+ return 0;
+
+err_power_down:
i2c_hid_core_power_down(ihid);
+
return ret;
}
@@ -1077,7 +1093,7 @@ static void ihid_core_panel_prepare_work(struct work_struct *work)
* steps.
*/
if (!hid->version)
- ret = __do_i2c_hid_core_initial_power_up(ihid);
+ ret = i2c_hid_core_probe_panel_follower(ihid);
else
ret = i2c_hid_core_resume(ihid);
@@ -1156,30 +1172,6 @@ static int i2c_hid_core_register_panel_follower(struct i2c_hid *ihid)
return 0;
}
-static int i2c_hid_core_initial_power_up(struct i2c_hid *ihid)
-{
- /*
- * If we're a panel follower, we'll register and do our initial power
- * up when the panel turns on; otherwise we do it right away.
- */
- if (drm_is_panel_follower(&ihid->client->dev))
- return i2c_hid_core_register_panel_follower(ihid);
- else
- return __do_i2c_hid_core_initial_power_up(ihid);
-}
-
-static void i2c_hid_core_final_power_down(struct i2c_hid *ihid)
-{
- /*
- * If we're a follower, the act of unfollowing will cause us to be
- * powered down. Otherwise we need to manually do it.
- */
- if (ihid->is_panel_follower)
- drm_panel_remove_follower(&ihid->panel_follower);
- else
- i2c_hid_core_suspend(ihid, true);
-}
-
int i2c_hid_core_probe(struct i2c_client *client, struct i2chid_ops *ops,
u16 hid_descriptor_address, u32 quirks)
{
@@ -1224,14 +1216,10 @@ int i2c_hid_core_probe(struct i2c_client *client, struct i2chid_ops *ops,
return ret;
device_enable_async_suspend(&client->dev);
- ret = i2c_hid_init_irq(client);
- if (ret < 0)
- goto err_buffers_allocated;
-
hid = hid_allocate_device();
if (IS_ERR(hid)) {
ret = PTR_ERR(hid);
- goto err_irq;
+ goto err_free_buffers;
}
ihid->hid = hid;
@@ -1242,19 +1230,42 @@ int i2c_hid_core_probe(struct i2c_client *client, struct i2chid_ops *ops,
hid->bus = BUS_I2C;
hid->initial_quirks = quirks;
- ret = i2c_hid_core_initial_power_up(ihid);
+ /* Power on and probe unless device is a panel follower. */
+ if (!drm_is_panel_follower(&ihid->client->dev)) {
+ ret = i2c_hid_core_power_up(ihid);
+ if (ret < 0)
+ goto err_destroy_device;
+
+ ret = __i2c_hid_core_probe(ihid);
+ if (ret < 0)
+ goto err_power_down;
+ }
+
+ ret = i2c_hid_init_irq(client);
+ if (ret < 0)
+ goto err_power_down;
+
+ /*
+ * If we're a panel follower, we'll register when the panel turns on;
+ * otherwise we do it right away.
+ */
+ if (drm_is_panel_follower(&ihid->client->dev))
+ ret = i2c_hid_core_register_panel_follower(ihid);
+ else
+ ret = i2c_hid_core_register_hid(ihid);
if (ret)
- goto err_mem_free;
+ goto err_free_irq;
return 0;
-err_mem_free:
- hid_destroy_device(hid);
-
-err_irq:
+err_free_irq:
free_irq(client->irq, ihid);
-
-err_buffers_allocated:
+err_power_down:
+ if (!drm_is_panel_follower(&ihid->client->dev))
+ i2c_hid_core_power_down(ihid);
+err_destroy_device:
+ hid_destroy_device(hid);
+err_free_buffers:
i2c_hid_free_buffers(ihid);
return ret;
@@ -1266,7 +1277,14 @@ void i2c_hid_core_remove(struct i2c_client *client)
struct i2c_hid *ihid = i2c_get_clientdata(client);
struct hid_device *hid;
- i2c_hid_core_final_power_down(ihid);
+ /*
+ * If we're a follower, the act of unfollowing will cause us to be
+ * powered down. Otherwise we need to manually do it.
+ */
+ if (ihid->is_panel_follower)
+ drm_panel_remove_follower(&ihid->panel_follower);
+ else
+ i2c_hid_core_suspend(ihid, true);
hid = ihid->hid;
hid_destroy_device(hid);
--
2.41.0
^ permalink raw reply related
* [PATCH] HID: lenovo: Add middleclick_workaround sysfs knob for cptkbd
From: Mikhail Khvainitski @ 2023-09-18 14:50 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires
Cc: linux-input, linux-kernel, Mikhail Khvainitski
Built-in firmware of cptkbd handles scrolling by itself (when middle
button is pressed) but with issues: it does not support horizontal and
hi-res scrolling and upon middle button release it sends middle button
click even if there was a scrolling event. Commit 3cb5ff0220e3 ("HID:
lenovo: Hide middle-button press until release") workarounds last
issue but it's impossible to workaround scrolling-related issues
without firmware modification.
Likely, Dennis Schneider has reverse engineered the firmware and
provided an instruction on how to patch it [1]. However,
aforementioned workaround prevents userspace (libinput) from knowing
exact moment when middle button has been pressed down and performing
"On-Button scrolling". This commit makes it possible to disable this
workaround by sysfs knob.
Link: https://hohlerde.org/rauch/en/elektronik/projekte/tpkbd-fix/ [1]
Signed-off-by: Mikhail Khvainitski <me@khvoinitsky.org>
---
drivers/hid/hid-lenovo.c | 84 +++++++++++++++++++++++++++++-----------
1 file changed, 62 insertions(+), 22 deletions(-)
diff --git a/drivers/hid/hid-lenovo.c b/drivers/hid/hid-lenovo.c
index 44763c0da444..8993fa1ce66a 100644
--- a/drivers/hid/hid-lenovo.c
+++ b/drivers/hid/hid-lenovo.c
@@ -53,6 +53,7 @@ struct lenovo_drvdata {
int press_speed;
u8 middlebutton_state; /* 0:Up, 1:Down (undecided), 2:Scrolling */
bool fn_lock;
+ bool middleclick_workaround_cptkbd;
};
#define map_key_clear(c) hid_map_usage_clear(hi, usage, bit, max, EV_KEY, (c))
@@ -603,6 +604,36 @@ static ssize_t attr_sensitivity_store_cptkbd(struct device *dev,
return count;
}
+static ssize_t attr_middleclick_workaround_show_cptkbd(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct hid_device *hdev = to_hid_device(dev);
+ struct lenovo_drvdata *cptkbd_data = hid_get_drvdata(hdev);
+
+ return snprintf(buf, PAGE_SIZE, "%u\n",
+ cptkbd_data->middleclick_workaround_cptkbd);
+}
+
+static ssize_t attr_middleclick_workaround_store_cptkbd(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf,
+ size_t count)
+{
+ struct hid_device *hdev = to_hid_device(dev);
+ struct lenovo_drvdata *cptkbd_data = hid_get_drvdata(hdev);
+ int value;
+
+ if (kstrtoint(buf, 10, &value))
+ return -EINVAL;
+ if (value < 0 || value > 1)
+ return -EINVAL;
+
+ cptkbd_data->middleclick_workaround_cptkbd = !!value;
+
+ return count;
+}
+
static struct device_attribute dev_attr_fn_lock =
__ATTR(fn_lock, S_IWUSR | S_IRUGO,
@@ -614,10 +645,16 @@ static struct device_attribute dev_attr_sensitivity_cptkbd =
attr_sensitivity_show_cptkbd,
attr_sensitivity_store_cptkbd);
+static struct device_attribute dev_attr_middleclick_workaround_cptkbd =
+ __ATTR(middleclick_workaround, S_IWUSR | S_IRUGO,
+ attr_middleclick_workaround_show_cptkbd,
+ attr_middleclick_workaround_store_cptkbd);
+
static struct attribute *lenovo_attributes_cptkbd[] = {
&dev_attr_fn_lock.attr,
&dev_attr_sensitivity_cptkbd.attr,
+ &dev_attr_middleclick_workaround_cptkbd.attr,
NULL
};
@@ -668,31 +705,33 @@ static int lenovo_event_cptkbd(struct hid_device *hdev,
{
struct lenovo_drvdata *cptkbd_data = hid_get_drvdata(hdev);
- /* "wheel" scroll events */
- if (usage->type == EV_REL && (usage->code == REL_WHEEL ||
- usage->code == REL_HWHEEL)) {
- /* Scroll events disable middle-click event */
- cptkbd_data->middlebutton_state = 2;
- return 0;
- }
+ if (cptkbd_data->middleclick_workaround_cptkbd) {
+ /* "wheel" scroll events */
+ if (usage->type == EV_REL && (usage->code == REL_WHEEL ||
+ usage->code == REL_HWHEEL)) {
+ /* Scroll events disable middle-click event */
+ cptkbd_data->middlebutton_state = 2;
+ return 0;
+ }
- /* Middle click events */
- if (usage->type == EV_KEY && usage->code == BTN_MIDDLE) {
- if (value == 1) {
- cptkbd_data->middlebutton_state = 1;
- } else if (value == 0) {
- if (cptkbd_data->middlebutton_state == 1) {
- /* No scrolling inbetween, send middle-click */
- input_event(field->hidinput->input,
- EV_KEY, BTN_MIDDLE, 1);
- input_sync(field->hidinput->input);
- input_event(field->hidinput->input,
- EV_KEY, BTN_MIDDLE, 0);
- input_sync(field->hidinput->input);
+ /* Middle click events */
+ if (usage->type == EV_KEY && usage->code == BTN_MIDDLE) {
+ if (value == 1) {
+ cptkbd_data->middlebutton_state = 1;
+ } else if (value == 0) {
+ if (cptkbd_data->middlebutton_state == 1) {
+ /* No scrolling inbetween, send middle-click */
+ input_event(field->hidinput->input,
+ EV_KEY, BTN_MIDDLE, 1);
+ input_sync(field->hidinput->input);
+ input_event(field->hidinput->input,
+ EV_KEY, BTN_MIDDLE, 0);
+ input_sync(field->hidinput->input);
+ }
+ cptkbd_data->middlebutton_state = 0;
}
- cptkbd_data->middlebutton_state = 0;
+ return 1;
}
- return 1;
}
if (usage->type == EV_KEY && usage->code == KEY_FN_ESC && value == 1) {
@@ -1146,6 +1185,7 @@ static int lenovo_probe_cptkbd(struct hid_device *hdev)
cptkbd_data->middlebutton_state = 0;
cptkbd_data->fn_lock = true;
cptkbd_data->sensitivity = 0x05;
+ cptkbd_data->middleclick_workaround_cptkbd = true;
lenovo_features_set_cptkbd(hdev);
ret = sysfs_create_group(&hdev->dev.kobj, &lenovo_attr_group_cptkbd);
--
2.42.0
^ permalink raw reply related
* [PATCH] HID: i2c-hid: fix handling of unpopulated devices
From: Johan Hovold @ 2023-09-18 12:58 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires
Cc: linux-input, linux-kernel, Johan Hovold, Douglas Anderson,
Maxime Ripard
A recent commit reordered probe so that the interrupt line is now
requested before making sure that the device exists.
This breaks machines like the Lenovo ThinkPad X13s which rely on the
HID driver to probe second-source devices and only register the variant
that is actually populated. Specifically, the interrupt line may now
already be (temporarily) claimed when doing asynchronous probing of the
touchpad:
genirq: Flags mismatch irq 191. 00082008 (hid-over-i2c) vs. 00082008 (hid-over-i2c)
i2c_hid_of 21-0015: Could not register for hid-over-i2c interrupt, irq = 191, ret = -16
i2c_hid_of: probe of 21-0015 failed with error -16
Fix this by restoring the old behaviour of first making sure the device
exists before requesting the interrupt line.
Note that something like this should probably be implemented also for
"panel followers", whose actual probe is currently effectively deferred
until the DRM panel is probed (e.g. by powering down the device after
making sure it exists and only then register it as a follower).
Fixes: 675cd877c952 ("HID: i2c-hid: Rearrange probe() to power things up later")
Cc: Douglas Anderson <dianders@chromium.org>
Cc: Maxime Ripard <mripard@kernel.org>
Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
---
drivers/hid/i2c-hid/i2c-hid-core.c | 142 ++++++++++++++++-------------
1 file changed, 80 insertions(+), 62 deletions(-)
diff --git a/drivers/hid/i2c-hid/i2c-hid-core.c b/drivers/hid/i2c-hid/i2c-hid-core.c
index 9601c0605fd9..e66c058a4b00 100644
--- a/drivers/hid/i2c-hid/i2c-hid-core.c
+++ b/drivers/hid/i2c-hid/i2c-hid-core.c
@@ -998,45 +998,29 @@ static int i2c_hid_core_resume(struct i2c_hid *ihid)
return hid_driver_reset_resume(hid);
}
-/**
- * __do_i2c_hid_core_initial_power_up() - First time power up of the i2c-hid device.
- * @ihid: The ihid object created during probe.
- *
- * This function is called at probe time.
- *
- * The initial power on is where we do some basic validation that the device
- * exists, where we fetch the HID descriptor, and where we create the actual
- * HID devices.
- *
- * Return: 0 or error code.
+/*
+ * Check that the device exists and parse the HID descriptor.
*/
-static int __do_i2c_hid_core_initial_power_up(struct i2c_hid *ihid)
+static int __i2c_hid_core_probe(struct i2c_hid *ihid)
{
struct i2c_client *client = ihid->client;
struct hid_device *hid = ihid->hid;
int ret;
- ret = i2c_hid_core_power_up(ihid);
- if (ret)
- return ret;
-
/* Make sure there is something at this address */
ret = i2c_smbus_read_byte(client);
if (ret < 0) {
i2c_hid_dbg(ihid, "nothing at this address: %d\n", ret);
- ret = -ENXIO;
- goto err;
+ return -ENXIO;
}
ret = i2c_hid_fetch_hid_descriptor(ihid);
if (ret < 0) {
dev_err(&client->dev,
"Failed to fetch the HID Descriptor\n");
- goto err;
+ return ret;
}
- enable_irq(client->irq);
-
hid->version = le16_to_cpu(ihid->hdesc.bcdVersion);
hid->vendor = le16_to_cpu(ihid->hdesc.wVendorID);
hid->product = le16_to_cpu(ihid->hdesc.wProductID);
@@ -1050,17 +1034,49 @@ static int __do_i2c_hid_core_initial_power_up(struct i2c_hid *ihid)
ihid->quirks = i2c_hid_lookup_quirk(hid->vendor, hid->product);
+ return 0;
+}
+
+static int i2c_hid_core_register_hid(struct i2c_hid *ihid)
+{
+ struct i2c_client *client = ihid->client;
+ struct hid_device *hid = ihid->hid;
+ int ret;
+
+ enable_irq(client->irq);
+
ret = hid_add_device(hid);
if (ret) {
if (ret != -ENODEV)
hid_err(client, "can't add hid device: %d\n", ret);
- goto err;
+ disable_irq(client->irq);
+ return ret;
}
return 0;
+}
+
+static int i2c_hid_core_probe_panel_follower(struct i2c_hid *ihid)
+{
+ int ret;
-err:
+ ret = i2c_hid_core_power_up(ihid);
+ if (ret)
+ return ret;
+
+ ret = __i2c_hid_core_probe(ihid);
+ if (ret)
+ goto err_power_down;
+
+ ret = i2c_hid_core_register_hid(ihid);
+ if (ret)
+ goto err_power_down;
+
+ return 0;
+
+err_power_down:
i2c_hid_core_power_down(ihid);
+
return ret;
}
@@ -1077,7 +1093,7 @@ static void ihid_core_panel_prepare_work(struct work_struct *work)
* steps.
*/
if (!hid->version)
- ret = __do_i2c_hid_core_initial_power_up(ihid);
+ ret = i2c_hid_core_probe_panel_follower(ihid);
else
ret = i2c_hid_core_resume(ihid);
@@ -1156,30 +1172,6 @@ static int i2c_hid_core_register_panel_follower(struct i2c_hid *ihid)
return 0;
}
-static int i2c_hid_core_initial_power_up(struct i2c_hid *ihid)
-{
- /*
- * If we're a panel follower, we'll register and do our initial power
- * up when the panel turns on; otherwise we do it right away.
- */
- if (drm_is_panel_follower(&ihid->client->dev))
- return i2c_hid_core_register_panel_follower(ihid);
- else
- return __do_i2c_hid_core_initial_power_up(ihid);
-}
-
-static void i2c_hid_core_final_power_down(struct i2c_hid *ihid)
-{
- /*
- * If we're a follower, the act of unfollowing will cause us to be
- * powered down. Otherwise we need to manually do it.
- */
- if (ihid->is_panel_follower)
- drm_panel_remove_follower(&ihid->panel_follower);
- else
- i2c_hid_core_suspend(ihid, true);
-}
-
int i2c_hid_core_probe(struct i2c_client *client, struct i2chid_ops *ops,
u16 hid_descriptor_address, u32 quirks)
{
@@ -1224,14 +1216,10 @@ int i2c_hid_core_probe(struct i2c_client *client, struct i2chid_ops *ops,
return ret;
device_enable_async_suspend(&client->dev);
- ret = i2c_hid_init_irq(client);
- if (ret < 0)
- goto err_buffers_allocated;
-
hid = hid_allocate_device();
if (IS_ERR(hid)) {
ret = PTR_ERR(hid);
- goto err_irq;
+ goto err_free_buffers;
}
ihid->hid = hid;
@@ -1242,19 +1230,42 @@ int i2c_hid_core_probe(struct i2c_client *client, struct i2chid_ops *ops,
hid->bus = BUS_I2C;
hid->initial_quirks = quirks;
- ret = i2c_hid_core_initial_power_up(ihid);
+ /* Power on and probe unless device is a panel follower. */
+ if (!drm_is_panel_follower(&ihid->client->dev)) {
+ ret = i2c_hid_core_power_up(ihid);
+ if (ret < 0)
+ goto err_destroy_device;
+
+ ret = __i2c_hid_core_probe(ihid);
+ if (ret < 0)
+ goto err_power_down;
+ }
+
+ ret = i2c_hid_init_irq(client);
+ if (ret < 0)
+ goto err_power_down;
+
+ /*
+ * If we're a panel follower, we'll register when the panel turns on;
+ * otherwise we do it right away.
+ */
+ if (drm_is_panel_follower(&ihid->client->dev))
+ ret = i2c_hid_core_register_panel_follower(ihid);
+ else
+ ret = i2c_hid_core_register_hid(ihid);
if (ret)
- goto err_mem_free;
+ goto err_free_irq;
return 0;
-err_mem_free:
- hid_destroy_device(hid);
-
-err_irq:
+err_free_irq:
free_irq(client->irq, ihid);
-
-err_buffers_allocated:
+err_power_down:
+ if (!drm_is_panel_follower(&ihid->client->dev))
+ i2c_hid_core_power_down(ihid);
+err_destroy_device:
+ hid_destroy_device(hid);
+err_free_buffers:
i2c_hid_free_buffers(ihid);
return ret;
@@ -1266,7 +1277,14 @@ void i2c_hid_core_remove(struct i2c_client *client)
struct i2c_hid *ihid = i2c_get_clientdata(client);
struct hid_device *hid;
- i2c_hid_core_final_power_down(ihid);
+ /*
+ * If we're a follower, the act of unfollowing will cause us to be
+ * powered down. Otherwise we need to manually do it.
+ */
+ if (ihid->is_panel_follower)
+ drm_panel_remove_follower(&ihid->panel_follower);
+ else
+ i2c_hid_core_suspend(ihid, true);
hid = ihid->hid;
hid_destroy_device(hid);
--
2.41.0
^ permalink raw reply related
* Re: [PATCH] HID: nvidia-shield: Select POWER_SUPPLY Kconfig option
From: Rahul Rameshbabu @ 2023-09-18 16:18 UTC (permalink / raw)
To: Jiri Kosina; +Cc: Benjamin Tissoires, linux-input
In-Reply-To: <nycvar.YFH.7.76.2309181656130.14216@cbobk.fhfr.pm>
On Mon, 18 Sep, 2023 16:56:49 +0200 Jiri Kosina <jikos@kernel.org> wrote:
> On Sun, 17 Sep 2023, Rahul Rameshbabu wrote:
>
>> Battery information reported by the driver depends on the power supply
>> subsystem. Select the required subsystem when the HID_NVIDIA_SHIELD
>> Kconfig option is enabled.
>>
>> Fixes: 3ab196f88237 ("HID: nvidia-shield: Add battery support for Thunderstrike")
>> Signed-off-by: Rahul Rameshbabu <rrameshbabu@nvidia.com>
>> ---
>> drivers/hid/Kconfig | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
>> index e11c1c803676..dc227f477601 100644
>> --- a/drivers/hid/Kconfig
>> +++ b/drivers/hid/Kconfig
>> @@ -792,6 +792,7 @@ config HID_NVIDIA_SHIELD
>> tristate "NVIDIA SHIELD devices"
>> depends on USB_HID
>> depends on BT_HIDP
>> + select POWER_SUPPLY
>
> Is there a reason not to do it the standard way using 'depends on', and
> not vice versa?
I originally used 'depends on' for POWER_SUPPLY. I took a look at
drivers/hid/Kconfig and saw that all modules that depended on
POWER_SUPPLY in the hid subsystem used 'select' instead. I figured I
should follow the trend.
--
Thanks,
Rahul Rameshbabu
^ permalink raw reply
* Re: [PATCH] HID: intel-ish-hid: ipc: Rework EHL OOB wakeup
From: srinivas pandruvada @ 2023-09-18 15:57 UTC (permalink / raw)
To: Kai-Heng Feng, Xu, Even
Cc: jikos@kernel.org, benjamin.tissoires@redhat.com,
linux-pm@vger.kernel.org, linux-pci@vger.kernel.org,
Lee, Jian Hui, Zhang, Lixu, Ba, Najumon,
linux-input@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <CAAd53p4o1pB-yzpvUCYsvuYEvQQK0my=u-ogrByRCx_Lvns=hw@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 9819 bytes --]
Hi Kai-Heng,
On Mon, 2023-09-18 at 09:17 +0800, Kai-Heng Feng wrote:
> Hi Even,
>
> On Mon, Sep 18, 2023 at 8:33 AM Xu, Even <even.xu@intel.com> wrote:
> >
> > Hi, Kai-Heng,
> >
> > I just got feedback, for testing EHL S5 wakeup feature, you need
> > several steps to setup and access
> > "https://portal.devicewise.com/things/browse" to trigger wake.
> > But currently, our test account of this website are all out of
> > data.
> > So maybe you need double check with the team who required you
> > preparing the patch for the verification.
>
> The patch is to solve the GPE refcount overflow, while maintaining S5
> wakeup. I don't have any mean to test S5 wake.
>
The issue is not calling acpi_disable_gpe(). To reduce the scope of
change can we just add that instead of a adding new callbacks. This way
scope is reduced.
Something like the attached
Thanks,
Srinivas
> So if you also don't have ways to verify S5 wake functionality, maybe
> we can simply revert 2e23a70edabe ("HID: intel-ish-hid: ipc: finish
> power flow for EHL OOB") as alternative?
>
> Kai-Heng
>
> > Thanks!
> >
> > Best Regards,
> > Even Xu
> >
> > -----Original Message-----
> > From: Xu, Even
> > Sent: Friday, September 15, 2023 3:27 PM
> > To: Kai-Heng Feng <kai.heng.feng@canonical.com>
> > Cc: srinivas pandruvada <srinivas.pandruvada@linux.intel.com>;
> > jikos@kernel.org; benjamin.tissoires@redhat.com;
> > linux-pm@vger.kernel.org; linux-pci@vger.kernel.org; Lee, Jian Hui
> > <jianhui.lee@canonical.com>; Zhang, Lixu <Lixu.Zhang@intel.com>;
> > Ba, Najumon <najumon.ba@intel.com>; linux-input@vger.kernel.org;
> > linux-kernel@vger.kernel.org
> > Subject: RE: [PATCH] HID: intel-ish-hid: ipc: Rework EHL OOB wakeup
> >
> > Hi, Kai-Heng,
> >
> > I am also not familiar with this S5 wakeup test case.
> > I already sent out mails to ask for help on it.
> > Will come back to you once I get feedback.
> > Thanks!
> >
> > Best Regards,
> > Even Xu
> >
> > -----Original Message-----
> > From: Kai-Heng Feng <kai.heng.feng@canonical.com>
> > Sent: Friday, September 15, 2023 2:01 PM
> > To: Xu, Even <even.xu@intel.com>
> > Cc: srinivas pandruvada <srinivas.pandruvada@linux.intel.com>;
> > jikos@kernel.org; benjamin.tissoires@redhat.com;
> > linux-pm@vger.kernel.org; linux-pci@vger.kernel.org; Lee, Jian Hui
> > <jianhui.lee@canonical.com>; Zhang, Lixu <lixu.zhang@intel.com>;
> > Ba, Najumon <najumon.ba@intel.com>; linux-input@vger.kernel.org;
> > linux-kernel@vger.kernel.org
> > Subject: Re: [PATCH] HID: intel-ish-hid: ipc: Rework EHL OOB wakeup
> >
> > Hi Even,
> >
> > On Fri, Sep 15, 2023 at 1:31 PM Xu, Even <even.xu@intel.com> wrote:
> > >
> > > Hi, Srinivas,
> > >
> > > Sure, I will test it.
> > > As long term not working on EHL, I doesn't have EHL board on hand
> > > right now, I can test this patch on other ISH related platforms.
> > > From the patch, it's focus on EHL platform, I assume Kai-Heng
> > > already verified the function on EHL board.
> >
> > I only made sure the GPE overflow issue is fixed by the patch, but
> > I didn't test the S5 wakeup.
> > That's because I don't know how to test it on the EHL system I
> > have.
> > I'll test it if you can let me know how to test the S5 wakeup.
> >
> > Kai-Heng
> >
> > > I don't think it will take effect on other platforms, anyway, I
> > > will test it on the platforms I have to provide cross platform
> > > verification.
> > >
> > > Thanks!
> > >
> > > Best Regards,
> > > Even Xu
> > >
> > > -----Original Message-----
> > > From: srinivas pandruvada <srinivas.pandruvada@linux.intel.com>
> > > Sent: Friday, September 15, 2023 12:11 AM
> > > To: Kai-Heng Feng <kai.heng.feng@canonical.com>;
> > > jikos@kernel.org;
> > > benjamin.tissoires@redhat.com
> > > Cc: linux-pm@vger.kernel.org; linux-pci@vger.kernel.org; Lee,
> > > Jian Hui
> > > <jianhui.lee@canonical.com>; Xu, Even <even.xu@intel.com>; Zhang,
> > > Lixu
> > > <lixu.zhang@intel.com>; Ba, Najumon <najumon.ba@intel.com>;
> > > linux-input@vger.kernel.org; linux-kernel@vger.kernel.org
> > > Subject: Re: [PATCH] HID: intel-ish-hid: ipc: Rework EHL OOB
> > > wakeup
> > >
> > > Hi Even,
> > >
> > > On Thu, 2023-09-14 at 12:18 +0800, Kai-Heng Feng wrote:
> > > > System cannot suspend more than 255 times because the driver
> > > > doesn't
> > > > have corresponding acpi_disable_gpe() for acpi_enable_gpe(), so
> > > > the
> > > > GPE refcount overflows.
> > > >
> > > > Since PCI core and ACPI core already handles PCI PME wake and
> > > > GPE
> > > > wake when the device has wakeup capability, use
> > > > device_init_wakeup()
> > > > to let them do the wakeup setting work.
> > > >
> > > > Also add a shutdown callback which uses pci_prepare_to_sleep()
> > > > to
> > > > let PCI and ACPI set OOB wakeup for S5.
> > > >
> > > Please test this change.
> > >
> > > Thanks,
> > > Srinivas
> > >
> > > > Fixes: 2e23a70edabe ("HID: intel-ish-hid: ipc: finish power
> > > > flow for
> > > > EHL OOB")
> > > > Cc: Jian Hui Lee <jianhui.lee@canonical.com>
> > > > Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
> > > > ---
> > > > drivers/hid/intel-ish-hid/ipc/pci-ish.c | 59
> > > > +++++++----------------
> > > > --
> > > > 1 file changed, 15 insertions(+), 44 deletions(-)
> > > >
> > > > diff --git a/drivers/hid/intel-ish-hid/ipc/pci-ish.c
> > > > b/drivers/hid/intel-ish-hid/ipc/pci-ish.c
> > > > index 55cb25038e63..65e7eeb2fa64 100644
> > > > --- a/drivers/hid/intel-ish-hid/ipc/pci-ish.c
> > > > +++ b/drivers/hid/intel-ish-hid/ipc/pci-ish.c
> > > > @@ -119,42 +119,6 @@ static inline bool
> > > > ish_should_leave_d0i3(struct
> > > > pci_dev *pdev)
> > > > return !pm_resume_via_firmware() || pdev->device ==
> > > > CHV_DEVICE_ID; }
> > > >
> > > > -static int enable_gpe(struct device *dev) -{ -#ifdef
> > > > CONFIG_ACPI
> > > > - acpi_status acpi_sts;
> > > > - struct acpi_device *adev;
> > > > - struct acpi_device_wakeup *wakeup;
> > > > -
> > > > - adev = ACPI_COMPANION(dev);
> > > > - if (!adev) {
> > > > - dev_err(dev, "get acpi handle failed\n");
> > > > - return -ENODEV;
> > > > - }
> > > > - wakeup = &adev->wakeup;
> > > > -
> > > > - acpi_sts = acpi_enable_gpe(wakeup->gpe_device, wakeup-
> > > > > gpe_number);
> > > > - if (ACPI_FAILURE(acpi_sts)) {
> > > > - dev_err(dev, "enable ose_gpe failed\n");
> > > > - return -EIO;
> > > > - }
> > > > -
> > > > - return 0;
> > > > -#else
> > > > - return -ENODEV;
> > > > -#endif
> > > > -}
> > > > -
> > > > -static void enable_pme_wake(struct pci_dev *pdev) -{
> > > > - if ((pci_pme_capable(pdev, PCI_D0) ||
> > > > - pci_pme_capable(pdev, PCI_D3hot) ||
> > > > - pci_pme_capable(pdev, PCI_D3cold)) &&
> > > > !enable_gpe(&pdev-
> > > > > dev)) {
> > > > - pci_pme_active(pdev, true);
> > > > - dev_dbg(&pdev->dev, "ish ipc driver pme wake
> > > > enabled\n");
> > > > - }
> > > > -}
> > > > -
> > > > /**
> > > > * ish_probe() - PCI driver probe callback
> > > > * @pdev: pci device
> > > > @@ -225,7 +189,7 @@ static int ish_probe(struct pci_dev *pdev,
> > > > const
> > > > struct pci_device_id *ent)
> > > >
> > > > /* Enable PME for EHL */
> > > > if (pdev->device == EHL_Ax_DEVICE_ID)
> > > > - enable_pme_wake(pdev);
> > > > + device_init_wakeup(dev, true);
> > > >
> > > > ret = ish_init(ishtp);
> > > > if (ret)
> > > > @@ -248,6 +212,19 @@ static void ish_remove(struct pci_dev
> > > > *pdev)
> > > > ish_device_disable(ishtp_dev); }
> > > >
> > > > +
> > > > +/**
> > > > + * ish_shutdown() - PCI driver shutdown callback
> > > > + * @pdev: pci device
> > > > + *
> > > > + * This function sets up wakeup for S5 */ static void
> > > > +ish_shutdown(struct pci_dev *pdev) {
> > > > + if (pdev->device == EHL_Ax_DEVICE_ID)
> > > > + pci_prepare_to_sleep(pdev); }
> > > > +
> > > > static struct device __maybe_unused *ish_resume_device;
> > > >
> > > > /* 50ms to get resume response */
> > > > @@ -370,13 +347,6 @@ static int __maybe_unused
> > > > ish_resume(struct
> > > > device *device)
> > > > struct pci_dev *pdev = to_pci_dev(device);
> > > > struct ishtp_device *dev = pci_get_drvdata(pdev);
> > > >
> > > > - /* add this to finish power flow for EHL */
> > > > - if (dev->pdev->device == EHL_Ax_DEVICE_ID) {
> > > > - pci_set_power_state(pdev, PCI_D0);
> > > > - enable_pme_wake(pdev);
> > > > - dev_dbg(dev->devc, "set power state to D0 for
> > > > ehl\n");
> > > > - }
> > > > -
> > > > ish_resume_device = device;
> > > > dev->resume_flag = 1;
> > > >
> > > > @@ -392,6 +362,7 @@ static struct pci_driver ish_driver = {
> > > > .id_table = ish_pci_tbl,
> > > > .probe = ish_probe,
> > > > .remove = ish_remove,
> > > > + .shutdown = ish_shutdown,
> > > > .driver.pm = &ish_pm_ops,
> > > > };
> > > >
> > >
[-- Attachment #2: ehl_gpe.diff --]
[-- Type: text/x-patch, Size: 2393 bytes --]
diff --git a/drivers/hid/intel-ish-hid/ipc/pci-ish.c b/drivers/hid/intel-ish-hid/ipc/pci-ish.c
index 55cb25038e63..0d854e4f1f7c 100644
--- a/drivers/hid/intel-ish-hid/ipc/pci-ish.c
+++ b/drivers/hid/intel-ish-hid/ipc/pci-ish.c
@@ -119,7 +119,7 @@ static inline bool ish_should_leave_d0i3(struct pci_dev *pdev)
return !pm_resume_via_firmware() || pdev->device == CHV_DEVICE_ID;
}
-static int enable_gpe(struct device *dev)
+static int enable_gpe(struct device *dev, bool status)
{
#ifdef CONFIG_ACPI
acpi_status acpi_sts;
@@ -133,7 +133,11 @@ static int enable_gpe(struct device *dev)
}
wakeup = &adev->wakeup;
- acpi_sts = acpi_enable_gpe(wakeup->gpe_device, wakeup->gpe_number);
+ if (status)
+ acpi_sts = acpi_enable_gpe(wakeup->gpe_device, wakeup->gpe_number);
+ else
+ acpi_sts = acpi_disable_gpe(wakeup->gpe_device, wakeup->gpe_number);
+
if (ACPI_FAILURE(acpi_sts)) {
dev_err(dev, "enable ose_gpe failed\n");
return -EIO;
@@ -145,13 +149,13 @@ static int enable_gpe(struct device *dev)
#endif
}
-static void enable_pme_wake(struct pci_dev *pdev)
+static void enable_pme_wake(struct pci_dev *pdev, status)
{
if ((pci_pme_capable(pdev, PCI_D0) ||
pci_pme_capable(pdev, PCI_D3hot) ||
- pci_pme_capable(pdev, PCI_D3cold)) && !enable_gpe(&pdev->dev)) {
- pci_pme_active(pdev, true);
- dev_dbg(&pdev->dev, "ish ipc driver pme wake enabled\n");
+ pci_pme_capable(pdev, PCI_D3cold)) && !enable_gpe(&pdev->dev), status) {
+ pci_pme_active(pdev, status);
+ dev_dbg(&pdev->dev, "ish ipc driver pme wake enabled state %d\n", status);
}
}
@@ -225,7 +229,7 @@ static int ish_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
/* Enable PME for EHL */
if (pdev->device == EHL_Ax_DEVICE_ID)
- enable_pme_wake(pdev);
+ enable_pme_wake(pdev, true);
ret = ish_init(ishtp);
if (ret)
@@ -353,6 +357,9 @@ static int __maybe_unused ish_suspend(struct device *device)
ish_disable_dma(dev);
}
+ if (dev->pdev->device == EHL_Ax_DEVICE_ID)
+ enable_pme_wake(pdev, false);
+
return 0;
}
@@ -373,7 +380,7 @@ static int __maybe_unused ish_resume(struct device *device)
/* add this to finish power flow for EHL */
if (dev->pdev->device == EHL_Ax_DEVICE_ID) {
pci_set_power_state(pdev, PCI_D0);
- enable_pme_wake(pdev);
+ enable_pme_wake(pdev, true);
dev_dbg(dev->devc, "set power state to D0 for ehl\n");
}
^ permalink raw reply related
* Re: [PATCH] HID: nvidia-shield: Select POWER_SUPPLY Kconfig option
From: Jiri Kosina @ 2023-09-18 14:56 UTC (permalink / raw)
To: Rahul Rameshbabu; +Cc: Benjamin Tissoires, linux-input
In-Reply-To: <20230917151850.62505-1-rrameshbabu@nvidia.com>
On Sun, 17 Sep 2023, Rahul Rameshbabu wrote:
> Battery information reported by the driver depends on the power supply
> subsystem. Select the required subsystem when the HID_NVIDIA_SHIELD
> Kconfig option is enabled.
>
> Fixes: 3ab196f88237 ("HID: nvidia-shield: Add battery support for Thunderstrike")
> Signed-off-by: Rahul Rameshbabu <rrameshbabu@nvidia.com>
> ---
> drivers/hid/Kconfig | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
> index e11c1c803676..dc227f477601 100644
> --- a/drivers/hid/Kconfig
> +++ b/drivers/hid/Kconfig
> @@ -792,6 +792,7 @@ config HID_NVIDIA_SHIELD
> tristate "NVIDIA SHIELD devices"
> depends on USB_HID
> depends on BT_HIDP
> + select POWER_SUPPLY
Is there a reason not to do it the standard way using 'depends on', and
not vice versa?
Thanks,
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH] HID: logitech-hidpp: Add Bluetooth ID for the Logitech M720 Triathlon mouse
From: Jiri Kosina @ 2023-09-18 14:48 UTC (permalink / raw)
To: Hans de Goede
Cc: Filipe Laíns, Bastien Nocera, Benjamin Tissoires,
linux-input
In-Reply-To: <20230827222438.43717-1-hdegoede@redhat.com>
On Mon, 28 Aug 2023, Hans de Goede wrote:
> Using hidpp for the M720 adds battery info reporting and hires
> scrolling support.
>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
> drivers/hid/hid-logitech-hidpp.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
> index 129b01be488d..3786fcc93da0 100644
> --- a/drivers/hid/hid-logitech-hidpp.c
> +++ b/drivers/hid/hid-logitech-hidpp.c
> @@ -4640,6 +4640,8 @@ static const struct hid_device_id hidpp_devices[] = {
> HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_LOGITECH, 0xb008) },
> { /* MX Master mouse over Bluetooth */
> HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_LOGITECH, 0xb012) },
> + { /* M720 Triathlon mouse over Bluetooth */
> + HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_LOGITECH, 0xb015) },
> { /* MX Ergo trackball over Bluetooth */
> HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_LOGITECH, 0xb01d) },
> { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_LOGITECH, 0xb01e) },
Applied, thanks.
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH] HID: nvidia-shield: add LEDS_CLASS dependency
From: Jiri Kosina @ 2023-09-18 14:55 UTC (permalink / raw)
To: Randy Dunlap
Cc: linux-kernel, Rahul Rameshbabu, Benjamin Tissoires, linux-input
In-Reply-To: <20230914000517.16252-1-rdunlap@infradead.org>
On Wed, 13 Sep 2023, Randy Dunlap wrote:
> The hid-nvidia-shield driver uses functions that are built
> only when LEDS_CLASS is set, so make the driver depend on that
> symbol to prevent build errors.
>
> riscv32-linux-ld: drivers/hid/hid-nvidia-shield.o: in function `.L11':
> hid-nvidia-shield.c:(.text+0x192): undefined reference to `led_classdev_unregister'
> riscv32-linux-ld: drivers/hid/hid-nvidia-shield.o: in function `.L113':
> hid-nvidia-shield.c:(.text+0xfa4): undefined reference to `led_classdev_register_ext'
>
> Fixes: 09308562d4af ("HID: nvidia-shield: Initial driver implementation with Thunderstrike support")
> Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
> Cc: Rahul Rameshbabu <rrameshbabu@nvidia.com>
> Cc: Jiri Kosina <jkosina@suse.cz>
> Cc: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> Cc: linux-input@vger.kernel.org
> ---
> drivers/hid/Kconfig | 1 +
> 1 file changed, 1 insertion(+)
>
> diff -- a/drivers/hid/Kconfig b/drivers/hid/Kconfig
> --- a/drivers/hid/Kconfig
> +++ b/drivers/hid/Kconfig
> @@ -799,6 +799,7 @@ config HID_NVIDIA_SHIELD
> tristate "NVIDIA SHIELD devices"
> depends on USB_HID
> depends on BT_HIDP
> + depends on LEDS_CLASS
Aplied, thanks.
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH] HID: i2c-hid: fix handling of unpopulated devices
From: Doug Anderson @ 2023-09-18 15:00 UTC (permalink / raw)
To: Johan Hovold
Cc: Jiri Kosina, Benjamin Tissoires, linux-input, linux-kernel,
Maxime Ripard, Dmitry Torokhov, LinusW, Rob Herring,
Krzysztof Kozlowski, Conor Dooley,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
open list:GPIO SUBSYSTEM
In-Reply-To: <20230918125851.310-1-johan+linaro@kernel.org>
Hi,
On Mon, Sep 18, 2023 at 6:00 AM Johan Hovold <johan+linaro@kernel.org> wrote:
>
> A recent commit reordered probe so that the interrupt line is now
> requested before making sure that the device exists.
>
> This breaks machines like the Lenovo ThinkPad X13s which rely on the
> HID driver to probe second-source devices and only register the variant
> that is actually populated. Specifically, the interrupt line may now
> already be (temporarily) claimed when doing asynchronous probing of the
> touchpad:
>
> genirq: Flags mismatch irq 191. 00082008 (hid-over-i2c) vs. 00082008 (hid-over-i2c)
> i2c_hid_of 21-0015: Could not register for hid-over-i2c interrupt, irq = 191, ret = -16
> i2c_hid_of: probe of 21-0015 failed with error -16
>
> Fix this by restoring the old behaviour of first making sure the device
> exists before requesting the interrupt line.
>
> Note that something like this should probably be implemented also for
> "panel followers", whose actual probe is currently effectively deferred
> until the DRM panel is probed (e.g. by powering down the device after
> making sure it exists and only then register it as a follower).
>
> Fixes: 675cd877c952 ("HID: i2c-hid: Rearrange probe() to power things up later")
> Cc: Douglas Anderson <dianders@chromium.org>
> Cc: Maxime Ripard <mripard@kernel.org>
> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
> ---
> drivers/hid/i2c-hid/i2c-hid-core.c | 142 ++++++++++++++++-------------
> 1 file changed, 80 insertions(+), 62 deletions(-)
Ugh, sorry for the regression. :( It actually turns out that I've been
digging into this same issue on a different device (see
mt8173-elm-hana). I hadn't realized that it was a regression caused by
my recent change, though.
I haven't yet reviewed your change in detail, but to me it seems like
at most a short term fix. Specifically, I think the way that this has
been working has been partially via hacks and partially via luck. Let
me explain...
Currently, to make this work the `sc8280xp-lenovo-thinkpad-x13s.dts`
file has a hack in it. You can see that the `tpad_default` pinctrl
entry has been moved up to the i2c bus level even though it doesn't
belong there (it should be in each trackpad). This is because,
otherwise, you would have run into similar type problems as the device
core would have failed to claim the pin for one of the devices.
Currently, we're getting a bit lucky with
`sc8280xp-lenovo-thinkpad-x13s.dts` that there are no other shared
resources between the two devices besides the interrupt. Specifically
a number of trackpads / touchscreens also have a "reset" GPIO that
needs to be power sequenced properly in order to talk to the
touchscreen. In this case we'll be stuck again because both instances
would need to grab the "reset" GPIO before being able to confirm if
the device is there.
This is an old problem. The first I remember running into it was back
in 2015 on rk3288-veryron-minnie. We had a downstream hack to make
this work with -EPROBE_DEFER. https://crrev.com/c/266224. By the time
we shipped, though, we decided not to do the 2nd sourcing. After that
I always NAKed HW designs like this, but I guess that didn't help with
Mediatek hardware I wasn't involved with. :( ...and, of course, it
didn't help with devices that aren't Chromebooks like the Thinkpad
X13S.
FWIW: as a short term solution, we ended up forcing synchronous probe
in <https://crrev.com/c/4857566>. This has some pretty serious boot
time implications, but it's also very simple.
I'm actively working on coming up with a better solution here. My
current thought is that that maybe we want to do:
1. Undo the hack in the device tree and have each "2nd source" have
its own pinctrl entry.
2. In core pinctrl / device probing code detect the pinctrl conflict
and only probe one of the devices at a time.
...that sounds like a nice/elegant solution and I'm trying to make it
work, though it does have some downsides. Namely:
a) It requires "dts" changes to work. Namely we've got to undo the
hack that pushed the pinctrl up to the controller level (or, in the
case of mt8173-elm-hana, that just totally skipped the "pinctrl" entry
altogether). Unfortunately those same "dts" changes will actually make
things _worse_ if you don't have the code change. :(
b) It only handles the case where the resources shared by 2nd sourcing
are expressed by pinctrl. In a practical sense this seems to be most
cases, but conceivably you could imagine running into this situation
with a non-pin-related shared resource.
c) To solve this in the core, we have to make sure we properly handle
(without hanging/failing) multiple partially-conflicting devices and
devices that might acquire resources in arbitrary orders.
Though the above solution detecting the pinctrl conflicts sounds
appealing and I'm currently working on prototyping it, I'm still not
100% convinced. I'm worried about the above downsides.
Personally, I feel like we could add information to the device tree
that would help us out. The question is: is this an abuse of device
tree for something that Linux ought to be able to figure out on its
own, or is it OK? To make it concrete, I was thinking about something
like this:
/ {
tp_ex_group: trackpad-exclusion-group {
members = <&tp1>, <&tp2>, <&tp3>;
};
};
&i2c_bus {
tp1: trackpad@10 {
...
mutual-exclusion-group = <&tp_ex_group>;
};
tp2: trackpad@20 {
...
mutual-exclusion-group = <&tp_ex_group>;
};
tp3: trackpad@30 {
...
mutual-exclusion-group = <&tp_ex_group>;
};
};
Then the device core would know not to probe devices in the same
"mutual-exclusion-group" at the same time.
If DT folks are OK with the "mutual-exclusion-group" idea then I'll
probably backburner my attempt to make this work on the pinctrl level
and go with that.
^ permalink raw reply
* Re: [PATCH v2] HID: steelseries: Fix signedness bug in steelseries_headset_arctis_1_fetch_battery()
From: Jiri Kosina @ 2023-09-18 14:44 UTC (permalink / raw)
To: Dan Carpenter
Cc: Bastien Nocera, Benjamin Tissoires, linux-input, kernel-janitors
In-Reply-To: <6e0a33e3-1564-419a-9946-b2d0afa0f98d@moroto.mountain>
On Fri, 15 Sep 2023, Dan Carpenter wrote:
> The hid_hw_raw_request() function returns negative error codes or the
> number bytes transferred. The problem is that when it returns negative
> error codes and we check if "ret < sizeof(arctis_1_battery_request)",
> then the negative values are type promoted from int to high unsigned long
> values and treated as success.
>
> This was detected using Smatch:
>
> drivers/hid/hid-steelseries.c:393 steelseries_headset_arctis_1_fetch_battery()
> warn: error code type promoted to positive: 'ret'
>
> Fixes: a0c76896c3fb ("HID: steelseries: Add support for Arctis 1 XBox")
> Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
> ---
> v2: Re-word the commit message. Add the Smatch warning. Use a cast
> instead of an explicit check for negatives.
>
> drivers/hid/hid-steelseries.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/hid/hid-steelseries.c b/drivers/hid/hid-steelseries.c
> index 43d2cf7153d7..b3edadf42d6d 100644
> --- a/drivers/hid/hid-steelseries.c
> +++ b/drivers/hid/hid-steelseries.c
> @@ -390,7 +390,7 @@ static int steelseries_headset_arctis_1_fetch_battery(struct hid_device *hdev)
> ret = hid_hw_raw_request(hdev, arctis_1_battery_request[0],
> write_buf, sizeof(arctis_1_battery_request),
> HID_OUTPUT_REPORT, HID_REQ_SET_REPORT);
> - if (ret < sizeof(arctis_1_battery_request)) {
> + if (ret < (int)sizeof(arctis_1_battery_request)) {
> hid_err(hdev, "hid_hw_raw_request() failed with %d\n", ret);
> ret = -ENODATA;
> }
Applied, thanks.
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH] HID: intel-ish-hid: ipc: Add Arrow Lake PCI device ID
From: Jiri Kosina @ 2023-09-18 14:46 UTC (permalink / raw)
To: Even Xu; +Cc: linux-input, srinivas.pandruvada, benjamin.tissoires
In-Reply-To: <1694572419-10981-1-git-send-email-even.xu@intel.com>
On Wed, 13 Sep 2023, Even Xu wrote:
> Add device ID of Arrow Lake-S into ishtp support list.
>
> Signed-off-by: Even Xu <even.xu@intel.com>
> ---
> drivers/hid/intel-ish-hid/ipc/hw-ish.h | 1 +
> drivers/hid/intel-ish-hid/ipc/pci-ish.c | 1 +
> 2 files changed, 2 insertions(+)
>
> diff --git a/drivers/hid/intel-ish-hid/ipc/hw-ish.h b/drivers/hid/intel-ish-hid/ipc/hw-ish.h
> index e99f3a3..f89b300 100644
> --- a/drivers/hid/intel-ish-hid/ipc/hw-ish.h
> +++ b/drivers/hid/intel-ish-hid/ipc/hw-ish.h
> @@ -34,6 +34,7 @@
> #define RPL_S_DEVICE_ID 0x7A78
> #define MTL_P_DEVICE_ID 0x7E45
> #define ARL_H_DEVICE_ID 0x7745
> +#define ARL_S_DEVICE_ID 0x7F78
>
> #define REVISION_ID_CHT_A0 0x6
> #define REVISION_ID_CHT_Ax_SI 0x0
> diff --git a/drivers/hid/intel-ish-hid/ipc/pci-ish.c b/drivers/hid/intel-ish-hid/ipc/pci-ish.c
> index 55cb250..ae3c6c1 100644
> --- a/drivers/hid/intel-ish-hid/ipc/pci-ish.c
> +++ b/drivers/hid/intel-ish-hid/ipc/pci-ish.c
> @@ -45,6 +45,7 @@ static const struct pci_device_id ish_pci_tbl[] = {
> {PCI_DEVICE(PCI_VENDOR_ID_INTEL, RPL_S_DEVICE_ID)},
> {PCI_DEVICE(PCI_VENDOR_ID_INTEL, MTL_P_DEVICE_ID)},
> {PCI_DEVICE(PCI_VENDOR_ID_INTEL, ARL_H_DEVICE_ID)},
> + {PCI_DEVICE(PCI_VENDOR_ID_INTEL, ARL_S_DEVICE_ID)},
> {0, }
Applied, thanks.
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* [PATCH v6] HID: mcp2200: added driver for GPIOs of MCP2200
From: Johannes Roith @ 2023-09-18 15:14 UTC (permalink / raw)
To: sergeantsagara
Cc: andi.shyti, benjamin.tissoires, christophe.jaillet, jikos,
johannes, linux-input, linux-kernel, rdunlap
In-Reply-To: <87ledpvhgm.fsf@protonmail.com>
Added a gpiochip compatible driver to control the 8 GPIOs of
the MCP2200 by using the HID interface.
Using GPIOs with alternative functions (GP0<->SSPND, GP1<->USBCFG,
GP6<->RXLED, GP7<->TXLED) will reset the functions, if set (unset by
default).
The driver was tested while also using the UART of the chip. Setting
and reading the GPIOs has no effect on the UART communication. However,
a reset is triggered after the CONFIGURE command. If the GPIO Direction
is constantly changed, this will affect the communication at low baud
rates. This is a hardware problem of the MCP2200 and is not caused by
the driver.
Signed-off-by: Johannes Roith <johannes@gnu-linux.rocks>
---
drivers/hid/Kconfig | 9 +
drivers/hid/Makefile | 1 +
drivers/hid/hid-ids.h | 1 +
drivers/hid/hid-mcp2200.c | 390 ++++++++++++++++++++++++++++++++++++++
4 files changed, 401 insertions(+)
create mode 100644 drivers/hid/hid-mcp2200.c
diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
index 0cea301cc9a9..3c14644b593d 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -1298,6 +1298,15 @@ config HID_ALPS
Say Y here if you have a Alps touchpads over i2c-hid or usbhid
and want support for its special functionalities.
+config HID_MCP2200
+ tristate "Microchip MCP2200 HID USB-to-GPIO bridge"
+ depends on USB_HID && GPIOLIB
+ help
+ Provides GPIO functionality over USB-HID through MCP2200 device.
+
+ To compile this driver as a module, choose M here: the module
+ will be called hid-mcp2200.ko.
+
config HID_MCP2221
tristate "Microchip MCP2221 HID USB-to-I2C/SMbus host support"
depends on USB_HID && I2C
diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
index 8a06d0f840bc..082a728eac60 100644
--- a/drivers/hid/Makefile
+++ b/drivers/hid/Makefile
@@ -79,6 +79,7 @@ obj-$(CONFIG_HID_LOGITECH_HIDPP) += hid-logitech-hidpp.o
obj-$(CONFIG_HID_MACALLY) += hid-macally.o
obj-$(CONFIG_HID_MAGICMOUSE) += hid-magicmouse.o
obj-$(CONFIG_HID_MALTRON) += hid-maltron.o
+obj-$(CONFIG_HID_MCP2200) += hid-mcp2200.o
obj-$(CONFIG_HID_MCP2221) += hid-mcp2221.o
obj-$(CONFIG_HID_MAYFLASH) += hid-mf.o
obj-$(CONFIG_HID_MEGAWORLD_FF) += hid-megaworld.o
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index 7e499992a793..bb87ad4eb2aa 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -915,6 +915,7 @@
#define USB_DEVICE_ID_PICK16F1454 0x0042
#define USB_DEVICE_ID_PICK16F1454_V2 0xf2f7
#define USB_DEVICE_ID_LUXAFOR 0xf372
+#define USB_DEVICE_ID_MCP2200 0x00df
#define USB_DEVICE_ID_MCP2221 0x00dd
#define USB_VENDOR_ID_MICROSOFT 0x045e
diff --git a/drivers/hid/hid-mcp2200.c b/drivers/hid/hid-mcp2200.c
new file mode 100644
index 000000000000..477a3915d2f0
--- /dev/null
+++ b/drivers/hid/hid-mcp2200.c
@@ -0,0 +1,390 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * MCP2200 - Microchip USB to GPIO bridge
+ *
+ * Copyright (c) 2023, Johannes Roith <johannes@gnu-linux.rocks>
+ *
+ * Datasheet: https://ww1.microchip.com/downloads/en/DeviceDoc/22228A.pdf
+ * App Note for HID: https://ww1.microchip.com/downloads/en/DeviceDoc/93066A.pdf
+ */
+#include <linux/completion.h>
+#include <linux/delay.h>
+#include <linux/err.h>
+#include <linux/gpio/driver.h>
+#include <linux/hid.h>
+#include <linux/hidraw.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include "hid-ids.h"
+
+/* Commands codes in a raw output report */
+#define SET_CLEAR_OUTPUTS 0x08
+#define CONFIGURE 0x10
+#define READ_EE 0x20
+#define WRITE_EE 0x40
+#define READ_ALL 0x80
+
+/* MCP GPIO direction encoding */
+enum MCP_IO_DIR {
+ MCP2200_DIR_OUT = 0x00,
+ MCP2200_DIR_IN = 0x01,
+};
+
+/* Altternative pin assignments */
+#define TXLED 2
+#define RXLED 3
+#define USBCFG 6
+#define SSPND 7
+#define MCP_NGPIO 8
+
+/* CMD to set or clear a GPIO output */
+struct mcp_set_clear_outputs {
+ u8 cmd;
+ u8 dummys1[10];
+ u8 set_bmap;
+ u8 clear_bmap;
+ u8 dummys2[3];
+} __packed;
+
+/* CMD to configure the IOs */
+struct mcp_configure {
+ u8 cmd;
+ u8 dummys1[3];
+ u8 io_bmap;
+ u8 config_alt_pins;
+ u8 io_default_val_bmap;
+ u8 config_alt_options;
+ u8 baud_h;
+ u8 baud_l;
+ u8 dummys2[6];
+} __packed;
+
+/* CMD to read all parameters */
+struct mcp_read_all {
+ u8 cmd;
+ u8 dummys[15];
+} __packed;
+
+/* Response to the read all cmd */
+struct mcp_read_all_resp {
+ u8 cmd;
+ u8 eep_addr;
+ u8 dummy;
+ u8 eep_val;
+ u8 io_bmap;
+ u8 config_alt_pins;
+ u8 io_default_val_bmap;
+ u8 config_alt_options;
+ u8 baud_h;
+ u8 baud_l;
+ u8 io_port_val_bmap;
+ u8 dummys[5];
+} __packed;
+
+struct mcp2200 {
+ struct hid_device *hdev;
+ struct mutex lock;
+ struct completion wait_in_report;
+ u8 gpio_dir;
+ u8 gpio_val;
+ u8 gpio_inval;
+ u8 baud_h;
+ u8 baud_l;
+ u8 config_alt_pins;
+ u8 gpio_reset_val;
+ u8 config_alt_options;
+ int status;
+ struct gpio_chip gc;
+ u8 hid_report[16];
+};
+
+/* this executes the READ_ALL cmd */
+static int mcp_cmd_read_all(struct mcp2200 *mcp)
+{
+ struct mcp_read_all *read_all;
+ int len, t;
+
+ reinit_completion(&mcp->wait_in_report);
+
+ mutex_lock(&mcp->lock);
+
+ read_all = (struct mcp_read_all *) mcp->hid_report;
+ read_all->cmd = READ_ALL;
+ len = hid_hw_output_report(mcp->hdev, (u8 *) read_all,
+ sizeof(struct mcp_read_all));
+
+ mutex_unlock(&mcp->lock);
+
+ if (len != sizeof(struct mcp_read_all))
+ return -EINVAL;
+
+ t = wait_for_completion_timeout(&mcp->wait_in_report,
+ msecs_to_jiffies(4000));
+ if (!t)
+ return -ETIMEDOUT;
+
+ /* return status, negative value if wrong response was received */
+ return mcp->status;
+}
+
+static void mcp_set_multiple(struct gpio_chip *gc, unsigned long *mask,
+ unsigned long *bits)
+{
+ struct mcp2200 *mcp = gpiochip_get_data(gc);
+ u8 value;
+ int status;
+ struct mcp_set_clear_outputs *cmd;
+
+ mutex_lock(&mcp->lock);
+ cmd = (struct mcp_set_clear_outputs *) mcp->hid_report;
+
+ value = mcp->gpio_val & ~*mask;
+ value |= (*mask & *bits);
+
+ cmd->cmd = SET_CLEAR_OUTPUTS;
+ cmd->set_bmap = value;
+ cmd->clear_bmap = ~(value);
+
+ status = hid_hw_output_report(mcp->hdev, (u8 *) cmd,
+ sizeof(struct mcp_set_clear_outputs));
+
+ if (status == sizeof(struct mcp_set_clear_outputs))
+ mcp->gpio_val = value;
+
+ mutex_unlock(&mcp->lock);
+}
+
+static void mcp_set(struct gpio_chip *gc, unsigned int gpio_nr, int value)
+{
+ unsigned long mask = 1 << gpio_nr;
+ unsigned long bmap_value = value << gpio_nr;
+
+ mcp_set_multiple(gc, &mask, &bmap_value);
+}
+
+static int mcp_get_multiple(struct gpio_chip *gc, unsigned long *mask,
+ unsigned long *bits)
+{
+ u32 val;
+ struct mcp2200 *mcp = gpiochip_get_data(gc);
+ int status;
+
+ status = mcp_cmd_read_all(mcp);
+ if (status)
+ return status;
+
+ val = mcp->gpio_inval;
+ *bits = (val & *mask);
+ return 0;
+}
+
+static int mcp_get(struct gpio_chip *gc, unsigned int gpio_nr)
+{
+ unsigned long mask = 0, bits = 0;
+
+ mask = (1 << gpio_nr);
+ mcp_get_multiple(gc, &mask, &bits);
+ return bits > 0;
+}
+
+static int mcp_get_direction(struct gpio_chip *gc, unsigned int gpio_nr)
+{
+ struct mcp2200 *mcp = gpiochip_get_data(gc);
+
+ return (mcp->gpio_dir & (MCP2200_DIR_IN << gpio_nr))
+ ? GPIO_LINE_DIRECTION_IN : GPIO_LINE_DIRECTION_OUT;
+}
+
+static int mcp_set_direction(struct gpio_chip *gc, unsigned int gpio_nr,
+ enum MCP_IO_DIR io_direction)
+{
+ struct mcp2200 *mcp = gpiochip_get_data(gc);
+ struct mcp_configure *conf;
+ int status;
+ /* after the configure cmd we will need to set the outputs again */
+ unsigned long mask = ~(mcp->gpio_dir); /* only set outputs */
+ unsigned long bits = mcp->gpio_val;
+ /* Offsets of alternative pins in config_alt_pins, 0 is not used */
+ u8 alt_pin_conf[8] = {SSPND, USBCFG, 0, 0, 0, 0, RXLED, TXLED};
+ u8 config_alt_pins = mcp->config_alt_pins;
+
+ /* Read in the reset baudrate first, we need it later */
+ status = mcp_cmd_read_all(mcp);
+ if (status != 0)
+ return status;
+
+ mutex_lock(&mcp->lock);
+ conf = (struct mcp_configure *) mcp->hid_report;
+
+ /* configure will reset the chip! */
+ conf->cmd = CONFIGURE;
+ conf->io_bmap = (mcp->gpio_dir & ~(1 << gpio_nr))
+ | (io_direction << gpio_nr);
+ /* Don't overwrite the reset parameters */
+ conf->baud_h = mcp->baud_h;
+ conf->baud_l = mcp->baud_l;
+ conf->config_alt_options = mcp->config_alt_options;
+ conf->io_default_val_bmap = mcp->gpio_reset_val;
+ /* Adjust alt. func if necessary */
+ if (alt_pin_conf[gpio_nr])
+ config_alt_pins &= ~(1 << alt_pin_conf[gpio_nr]);
+ conf->config_alt_pins = config_alt_pins;
+
+ status = hid_hw_output_report(mcp->hdev, (u8 *) conf,
+ sizeof(struct mcp_set_clear_outputs));
+
+ if (status == sizeof(struct mcp_set_clear_outputs)) {
+ mcp->gpio_dir = conf->io_bmap;
+ mcp->config_alt_pins = config_alt_pins;
+ } else {
+ mutex_unlock(&mcp->lock);
+ return -EIO;
+ }
+
+ mutex_unlock(&mcp->lock);
+
+ /* Configure CMD will clear all IOs -> rewrite them */
+ mcp_set_multiple(gc, &mask, &bits);
+ return 0;
+}
+
+static int mcp_direction_input(struct gpio_chip *gc, unsigned int gpio_nr)
+{
+ return mcp_set_direction(gc, gpio_nr, MCP2200_DIR_IN);
+}
+
+static int mcp_direction_output(struct gpio_chip *gc, unsigned int gpio_nr,
+ int value)
+{
+ int ret;
+ unsigned long mask, bmap_value;
+
+ mask = 1 << gpio_nr;
+ bmap_value = value << gpio_nr;
+
+ ret = mcp_set_direction(gc, gpio_nr, MCP2200_DIR_OUT);
+ if (!ret)
+ mcp_set_multiple(gc, &mask, &bmap_value);
+ return ret;
+}
+
+static const struct gpio_chip template_chip = {
+ .label = "mcp2200",
+ .owner = THIS_MODULE,
+ .get_direction = mcp_get_direction,
+ .direction_input = mcp_direction_input,
+ .direction_output = mcp_direction_output,
+ .set = mcp_set,
+ .set_multiple = mcp_set_multiple,
+ .get = mcp_get,
+ .get_multiple = mcp_get_multiple,
+ .base = -1,
+ .ngpio = MCP_NGPIO,
+ .can_sleep = true,
+};
+
+/*
+ * MCP2200 uses interrupt endpoint for input reports. This function
+ * is called by HID layer when it receives i/p report from mcp2200,
+ * which is actually a response to the previously sent command.
+ */
+static int mcp2200_raw_event(struct hid_device *hdev, struct hid_report *report,
+ u8 *data, int size)
+{
+ struct mcp2200 *mcp = hid_get_drvdata(hdev);
+ struct mcp_read_all_resp *all_resp;
+
+ switch (data[0]) {
+ case READ_ALL:
+ all_resp = (struct mcp_read_all_resp *) data;
+ mcp->status = 0;
+ mcp->gpio_inval = all_resp->io_port_val_bmap;
+ mcp->baud_h = all_resp->baud_h;
+ mcp->baud_l = all_resp->baud_l;
+ mcp->gpio_reset_val = all_resp->io_default_val_bmap;
+ mcp->config_alt_pins = all_resp->config_alt_pins;
+ mcp->config_alt_options = all_resp->config_alt_options;
+ break;
+ default:
+ mcp->status = -EIO;
+ break;
+ }
+
+ complete(&mcp->wait_in_report);
+ return 0;
+}
+
+static int mcp2200_probe(struct hid_device *hdev, const struct hid_device_id *id)
+{
+ int ret;
+ struct mcp2200 *mcp;
+
+ mcp = devm_kzalloc(&hdev->dev, sizeof(*mcp), GFP_KERNEL);
+ if (!mcp)
+ return -ENOMEM;
+
+ ret = hid_parse(hdev);
+ if (ret) {
+ hid_err(hdev, "can't parse reports\n");
+ return ret;
+ }
+
+ ret = hid_hw_start(hdev, 0);
+ if (ret) {
+ hid_err(hdev, "can't start hardware\n");
+ return ret;
+ }
+
+ hid_info(hdev, "USB HID v%x.%02x Device [%s] on %s\n", hdev->version >> 8,
+ hdev->version & 0xff, hdev->name, hdev->phys);
+
+ ret = hid_hw_open(hdev);
+ if (ret) {
+ hid_err(hdev, "can't open device\n");
+ hid_hw_stop(hdev);
+ return ret;
+ }
+
+ mutex_init(&mcp->lock);
+ init_completion(&mcp->wait_in_report);
+ hid_set_drvdata(hdev, mcp);
+ mcp->hdev = hdev;
+
+ mcp->gc = template_chip;
+ mcp->gc.parent = &hdev->dev;
+
+ ret = devm_gpiochip_add_data(&hdev->dev, &mcp->gc, mcp);
+ if (ret < 0) {
+ hid_err(hdev, "Unable to register gpiochip\n");
+ return ret;
+ }
+
+ return 0;
+}
+
+static void mcp2200_remove(struct hid_device *hdev)
+{
+ hid_hw_close(hdev);
+ hid_hw_stop(hdev);
+}
+
+static const struct hid_device_id mcp2200_devices[] = {
+ { HID_USB_DEVICE(USB_VENDOR_ID_MICROCHIP, USB_DEVICE_ID_MCP2200) },
+ { }
+};
+MODULE_DEVICE_TABLE(hid, mcp2200_devices);
+
+static struct hid_driver mcp2200_driver = {
+ .name = "mcp2200",
+ .id_table = mcp2200_devices,
+ .probe = mcp2200_probe,
+ .remove = mcp2200_remove,
+ .raw_event = mcp2200_raw_event,
+};
+
+/* Register with HID core */
+module_hid_driver(mcp2200_driver);
+
+MODULE_AUTHOR("Johannes Roith <johannes@gnu-linux.rocks>");
+MODULE_DESCRIPTION("MCP2200 Microchip HID USB to GPIO bridge");
+MODULE_LICENSE("GPL");
--
2.42.0
^ permalink raw reply related
* [PATCH v6] HID: mcp2200: added driver for GPIOs of MCP2200
From: Johannes Roith @ 2023-09-18 15:16 UTC (permalink / raw)
To: jikos
Cc: sergeantsagara, andi.shyti, benjamin.tissoires,
christophe.jaillet, johannes, linux-input, linux-kernel, rdunlap
Added a gpiochip compatible driver to control the 8 GPIOs of
the MCP2200 by using the HID interface.
Using GPIOs with alternative functions (GP0<->SSPND, GP1<->USBCFG,
GP6<->RXLED, GP7<->TXLED) will reset the functions, if set (unset by
default).
The driver was tested while also using the UART of the chip. Setting
and reading the GPIOs has no effect on the UART communication. However,
a reset is triggered after the CONFIGURE command. If the GPIO Direction
is constantly changed, this will affect the communication at low baud
rates. This is a hardware problem of the MCP2200 and is not caused by
the driver.
Signed-off-by: Johannes Roith <johannes@gnu-linux.rocks>
---
drivers/hid/Kconfig | 9 +
drivers/hid/Makefile | 1 +
drivers/hid/hid-ids.h | 1 +
drivers/hid/hid-mcp2200.c | 390 ++++++++++++++++++++++++++++++++++++++
4 files changed, 401 insertions(+)
create mode 100644 drivers/hid/hid-mcp2200.c
diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
index 0cea301cc9a9..3c14644b593d 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -1298,6 +1298,15 @@ config HID_ALPS
Say Y here if you have a Alps touchpads over i2c-hid or usbhid
and want support for its special functionalities.
+config HID_MCP2200
+ tristate "Microchip MCP2200 HID USB-to-GPIO bridge"
+ depends on USB_HID && GPIOLIB
+ help
+ Provides GPIO functionality over USB-HID through MCP2200 device.
+
+ To compile this driver as a module, choose M here: the module
+ will be called hid-mcp2200.ko.
+
config HID_MCP2221
tristate "Microchip MCP2221 HID USB-to-I2C/SMbus host support"
depends on USB_HID && I2C
diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
index 8a06d0f840bc..082a728eac60 100644
--- a/drivers/hid/Makefile
+++ b/drivers/hid/Makefile
@@ -79,6 +79,7 @@ obj-$(CONFIG_HID_LOGITECH_HIDPP) += hid-logitech-hidpp.o
obj-$(CONFIG_HID_MACALLY) += hid-macally.o
obj-$(CONFIG_HID_MAGICMOUSE) += hid-magicmouse.o
obj-$(CONFIG_HID_MALTRON) += hid-maltron.o
+obj-$(CONFIG_HID_MCP2200) += hid-mcp2200.o
obj-$(CONFIG_HID_MCP2221) += hid-mcp2221.o
obj-$(CONFIG_HID_MAYFLASH) += hid-mf.o
obj-$(CONFIG_HID_MEGAWORLD_FF) += hid-megaworld.o
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index 7e499992a793..bb87ad4eb2aa 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -915,6 +915,7 @@
#define USB_DEVICE_ID_PICK16F1454 0x0042
#define USB_DEVICE_ID_PICK16F1454_V2 0xf2f7
#define USB_DEVICE_ID_LUXAFOR 0xf372
+#define USB_DEVICE_ID_MCP2200 0x00df
#define USB_DEVICE_ID_MCP2221 0x00dd
#define USB_VENDOR_ID_MICROSOFT 0x045e
diff --git a/drivers/hid/hid-mcp2200.c b/drivers/hid/hid-mcp2200.c
new file mode 100644
index 000000000000..477a3915d2f0
--- /dev/null
+++ b/drivers/hid/hid-mcp2200.c
@@ -0,0 +1,390 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * MCP2200 - Microchip USB to GPIO bridge
+ *
+ * Copyright (c) 2023, Johannes Roith <johannes@gnu-linux.rocks>
+ *
+ * Datasheet: https://ww1.microchip.com/downloads/en/DeviceDoc/22228A.pdf
+ * App Note for HID: https://ww1.microchip.com/downloads/en/DeviceDoc/93066A.pdf
+ */
+#include <linux/completion.h>
+#include <linux/delay.h>
+#include <linux/err.h>
+#include <linux/gpio/driver.h>
+#include <linux/hid.h>
+#include <linux/hidraw.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include "hid-ids.h"
+
+/* Commands codes in a raw output report */
+#define SET_CLEAR_OUTPUTS 0x08
+#define CONFIGURE 0x10
+#define READ_EE 0x20
+#define WRITE_EE 0x40
+#define READ_ALL 0x80
+
+/* MCP GPIO direction encoding */
+enum MCP_IO_DIR {
+ MCP2200_DIR_OUT = 0x00,
+ MCP2200_DIR_IN = 0x01,
+};
+
+/* Altternative pin assignments */
+#define TXLED 2
+#define RXLED 3
+#define USBCFG 6
+#define SSPND 7
+#define MCP_NGPIO 8
+
+/* CMD to set or clear a GPIO output */
+struct mcp_set_clear_outputs {
+ u8 cmd;
+ u8 dummys1[10];
+ u8 set_bmap;
+ u8 clear_bmap;
+ u8 dummys2[3];
+} __packed;
+
+/* CMD to configure the IOs */
+struct mcp_configure {
+ u8 cmd;
+ u8 dummys1[3];
+ u8 io_bmap;
+ u8 config_alt_pins;
+ u8 io_default_val_bmap;
+ u8 config_alt_options;
+ u8 baud_h;
+ u8 baud_l;
+ u8 dummys2[6];
+} __packed;
+
+/* CMD to read all parameters */
+struct mcp_read_all {
+ u8 cmd;
+ u8 dummys[15];
+} __packed;
+
+/* Response to the read all cmd */
+struct mcp_read_all_resp {
+ u8 cmd;
+ u8 eep_addr;
+ u8 dummy;
+ u8 eep_val;
+ u8 io_bmap;
+ u8 config_alt_pins;
+ u8 io_default_val_bmap;
+ u8 config_alt_options;
+ u8 baud_h;
+ u8 baud_l;
+ u8 io_port_val_bmap;
+ u8 dummys[5];
+} __packed;
+
+struct mcp2200 {
+ struct hid_device *hdev;
+ struct mutex lock;
+ struct completion wait_in_report;
+ u8 gpio_dir;
+ u8 gpio_val;
+ u8 gpio_inval;
+ u8 baud_h;
+ u8 baud_l;
+ u8 config_alt_pins;
+ u8 gpio_reset_val;
+ u8 config_alt_options;
+ int status;
+ struct gpio_chip gc;
+ u8 hid_report[16];
+};
+
+/* this executes the READ_ALL cmd */
+static int mcp_cmd_read_all(struct mcp2200 *mcp)
+{
+ struct mcp_read_all *read_all;
+ int len, t;
+
+ reinit_completion(&mcp->wait_in_report);
+
+ mutex_lock(&mcp->lock);
+
+ read_all = (struct mcp_read_all *) mcp->hid_report;
+ read_all->cmd = READ_ALL;
+ len = hid_hw_output_report(mcp->hdev, (u8 *) read_all,
+ sizeof(struct mcp_read_all));
+
+ mutex_unlock(&mcp->lock);
+
+ if (len != sizeof(struct mcp_read_all))
+ return -EINVAL;
+
+ t = wait_for_completion_timeout(&mcp->wait_in_report,
+ msecs_to_jiffies(4000));
+ if (!t)
+ return -ETIMEDOUT;
+
+ /* return status, negative value if wrong response was received */
+ return mcp->status;
+}
+
+static void mcp_set_multiple(struct gpio_chip *gc, unsigned long *mask,
+ unsigned long *bits)
+{
+ struct mcp2200 *mcp = gpiochip_get_data(gc);
+ u8 value;
+ int status;
+ struct mcp_set_clear_outputs *cmd;
+
+ mutex_lock(&mcp->lock);
+ cmd = (struct mcp_set_clear_outputs *) mcp->hid_report;
+
+ value = mcp->gpio_val & ~*mask;
+ value |= (*mask & *bits);
+
+ cmd->cmd = SET_CLEAR_OUTPUTS;
+ cmd->set_bmap = value;
+ cmd->clear_bmap = ~(value);
+
+ status = hid_hw_output_report(mcp->hdev, (u8 *) cmd,
+ sizeof(struct mcp_set_clear_outputs));
+
+ if (status == sizeof(struct mcp_set_clear_outputs))
+ mcp->gpio_val = value;
+
+ mutex_unlock(&mcp->lock);
+}
+
+static void mcp_set(struct gpio_chip *gc, unsigned int gpio_nr, int value)
+{
+ unsigned long mask = 1 << gpio_nr;
+ unsigned long bmap_value = value << gpio_nr;
+
+ mcp_set_multiple(gc, &mask, &bmap_value);
+}
+
+static int mcp_get_multiple(struct gpio_chip *gc, unsigned long *mask,
+ unsigned long *bits)
+{
+ u32 val;
+ struct mcp2200 *mcp = gpiochip_get_data(gc);
+ int status;
+
+ status = mcp_cmd_read_all(mcp);
+ if (status)
+ return status;
+
+ val = mcp->gpio_inval;
+ *bits = (val & *mask);
+ return 0;
+}
+
+static int mcp_get(struct gpio_chip *gc, unsigned int gpio_nr)
+{
+ unsigned long mask = 0, bits = 0;
+
+ mask = (1 << gpio_nr);
+ mcp_get_multiple(gc, &mask, &bits);
+ return bits > 0;
+}
+
+static int mcp_get_direction(struct gpio_chip *gc, unsigned int gpio_nr)
+{
+ struct mcp2200 *mcp = gpiochip_get_data(gc);
+
+ return (mcp->gpio_dir & (MCP2200_DIR_IN << gpio_nr))
+ ? GPIO_LINE_DIRECTION_IN : GPIO_LINE_DIRECTION_OUT;
+}
+
+static int mcp_set_direction(struct gpio_chip *gc, unsigned int gpio_nr,
+ enum MCP_IO_DIR io_direction)
+{
+ struct mcp2200 *mcp = gpiochip_get_data(gc);
+ struct mcp_configure *conf;
+ int status;
+ /* after the configure cmd we will need to set the outputs again */
+ unsigned long mask = ~(mcp->gpio_dir); /* only set outputs */
+ unsigned long bits = mcp->gpio_val;
+ /* Offsets of alternative pins in config_alt_pins, 0 is not used */
+ u8 alt_pin_conf[8] = {SSPND, USBCFG, 0, 0, 0, 0, RXLED, TXLED};
+ u8 config_alt_pins = mcp->config_alt_pins;
+
+ /* Read in the reset baudrate first, we need it later */
+ status = mcp_cmd_read_all(mcp);
+ if (status != 0)
+ return status;
+
+ mutex_lock(&mcp->lock);
+ conf = (struct mcp_configure *) mcp->hid_report;
+
+ /* configure will reset the chip! */
+ conf->cmd = CONFIGURE;
+ conf->io_bmap = (mcp->gpio_dir & ~(1 << gpio_nr))
+ | (io_direction << gpio_nr);
+ /* Don't overwrite the reset parameters */
+ conf->baud_h = mcp->baud_h;
+ conf->baud_l = mcp->baud_l;
+ conf->config_alt_options = mcp->config_alt_options;
+ conf->io_default_val_bmap = mcp->gpio_reset_val;
+ /* Adjust alt. func if necessary */
+ if (alt_pin_conf[gpio_nr])
+ config_alt_pins &= ~(1 << alt_pin_conf[gpio_nr]);
+ conf->config_alt_pins = config_alt_pins;
+
+ status = hid_hw_output_report(mcp->hdev, (u8 *) conf,
+ sizeof(struct mcp_set_clear_outputs));
+
+ if (status == sizeof(struct mcp_set_clear_outputs)) {
+ mcp->gpio_dir = conf->io_bmap;
+ mcp->config_alt_pins = config_alt_pins;
+ } else {
+ mutex_unlock(&mcp->lock);
+ return -EIO;
+ }
+
+ mutex_unlock(&mcp->lock);
+
+ /* Configure CMD will clear all IOs -> rewrite them */
+ mcp_set_multiple(gc, &mask, &bits);
+ return 0;
+}
+
+static int mcp_direction_input(struct gpio_chip *gc, unsigned int gpio_nr)
+{
+ return mcp_set_direction(gc, gpio_nr, MCP2200_DIR_IN);
+}
+
+static int mcp_direction_output(struct gpio_chip *gc, unsigned int gpio_nr,
+ int value)
+{
+ int ret;
+ unsigned long mask, bmap_value;
+
+ mask = 1 << gpio_nr;
+ bmap_value = value << gpio_nr;
+
+ ret = mcp_set_direction(gc, gpio_nr, MCP2200_DIR_OUT);
+ if (!ret)
+ mcp_set_multiple(gc, &mask, &bmap_value);
+ return ret;
+}
+
+static const struct gpio_chip template_chip = {
+ .label = "mcp2200",
+ .owner = THIS_MODULE,
+ .get_direction = mcp_get_direction,
+ .direction_input = mcp_direction_input,
+ .direction_output = mcp_direction_output,
+ .set = mcp_set,
+ .set_multiple = mcp_set_multiple,
+ .get = mcp_get,
+ .get_multiple = mcp_get_multiple,
+ .base = -1,
+ .ngpio = MCP_NGPIO,
+ .can_sleep = true,
+};
+
+/*
+ * MCP2200 uses interrupt endpoint for input reports. This function
+ * is called by HID layer when it receives i/p report from mcp2200,
+ * which is actually a response to the previously sent command.
+ */
+static int mcp2200_raw_event(struct hid_device *hdev, struct hid_report *report,
+ u8 *data, int size)
+{
+ struct mcp2200 *mcp = hid_get_drvdata(hdev);
+ struct mcp_read_all_resp *all_resp;
+
+ switch (data[0]) {
+ case READ_ALL:
+ all_resp = (struct mcp_read_all_resp *) data;
+ mcp->status = 0;
+ mcp->gpio_inval = all_resp->io_port_val_bmap;
+ mcp->baud_h = all_resp->baud_h;
+ mcp->baud_l = all_resp->baud_l;
+ mcp->gpio_reset_val = all_resp->io_default_val_bmap;
+ mcp->config_alt_pins = all_resp->config_alt_pins;
+ mcp->config_alt_options = all_resp->config_alt_options;
+ break;
+ default:
+ mcp->status = -EIO;
+ break;
+ }
+
+ complete(&mcp->wait_in_report);
+ return 0;
+}
+
+static int mcp2200_probe(struct hid_device *hdev, const struct hid_device_id *id)
+{
+ int ret;
+ struct mcp2200 *mcp;
+
+ mcp = devm_kzalloc(&hdev->dev, sizeof(*mcp), GFP_KERNEL);
+ if (!mcp)
+ return -ENOMEM;
+
+ ret = hid_parse(hdev);
+ if (ret) {
+ hid_err(hdev, "can't parse reports\n");
+ return ret;
+ }
+
+ ret = hid_hw_start(hdev, 0);
+ if (ret) {
+ hid_err(hdev, "can't start hardware\n");
+ return ret;
+ }
+
+ hid_info(hdev, "USB HID v%x.%02x Device [%s] on %s\n", hdev->version >> 8,
+ hdev->version & 0xff, hdev->name, hdev->phys);
+
+ ret = hid_hw_open(hdev);
+ if (ret) {
+ hid_err(hdev, "can't open device\n");
+ hid_hw_stop(hdev);
+ return ret;
+ }
+
+ mutex_init(&mcp->lock);
+ init_completion(&mcp->wait_in_report);
+ hid_set_drvdata(hdev, mcp);
+ mcp->hdev = hdev;
+
+ mcp->gc = template_chip;
+ mcp->gc.parent = &hdev->dev;
+
+ ret = devm_gpiochip_add_data(&hdev->dev, &mcp->gc, mcp);
+ if (ret < 0) {
+ hid_err(hdev, "Unable to register gpiochip\n");
+ return ret;
+ }
+
+ return 0;
+}
+
+static void mcp2200_remove(struct hid_device *hdev)
+{
+ hid_hw_close(hdev);
+ hid_hw_stop(hdev);
+}
+
+static const struct hid_device_id mcp2200_devices[] = {
+ { HID_USB_DEVICE(USB_VENDOR_ID_MICROCHIP, USB_DEVICE_ID_MCP2200) },
+ { }
+};
+MODULE_DEVICE_TABLE(hid, mcp2200_devices);
+
+static struct hid_driver mcp2200_driver = {
+ .name = "mcp2200",
+ .id_table = mcp2200_devices,
+ .probe = mcp2200_probe,
+ .remove = mcp2200_remove,
+ .raw_event = mcp2200_raw_event,
+};
+
+/* Register with HID core */
+module_hid_driver(mcp2200_driver);
+
+MODULE_AUTHOR("Johannes Roith <johannes@gnu-linux.rocks>");
+MODULE_DESCRIPTION("MCP2200 Microchip HID USB to GPIO bridge");
+MODULE_LICENSE("GPL");
--
2.42.0
^ permalink raw reply related
* [syzbot] [input?] INFO: task hung in __input_unregister_device (5)
From: syzbot @ 2023-09-18 12:45 UTC (permalink / raw)
To: dmitry.torokhov, linux-input, linux-kernel, linux-usb, rydberg,
syzkaller-bugs
Hello,
syzbot found the following issue on:
HEAD commit: 0bb80ecc33a8 Linux 6.6-rc1
git tree: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing
console output: https://syzkaller.appspot.com/x/log.txt?x=146fd272680000
kernel config: https://syzkaller.appspot.com/x/.config?x=13b8d0222a8e5a19
dashboard link: https://syzkaller.appspot.com/bug?extid=78e2288f58b881ed3c45
compiler: gcc (Debian 12.2.0-14) 12.2.0, GNU ld (GNU Binutils for Debian) 2.40
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=151e9d62680000
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=17971b44680000
Downloadable assets:
disk image: https://storage.googleapis.com/syzbot-assets/a5fdb25d37ac/disk-0bb80ecc.raw.xz
vmlinux: https://storage.googleapis.com/syzbot-assets/dc964206a779/vmlinux-0bb80ecc.xz
kernel image: https://storage.googleapis.com/syzbot-assets/541dfc8b614c/bzImage-0bb80ecc.xz
IMPORTANT: if you fix the issue, please add the following tag to the commit:
Reported-by: syzbot+78e2288f58b881ed3c45@syzkaller.appspotmail.com
INFO: task kworker/1:0:22 blocked for more than 143 seconds.
Not tainted 6.6.0-rc1-syzkaller #0
"echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
task:kworker/1:0 state:D
stack:22752 pid:22 ppid:2 flags:0x00004000
Workqueue: usb_hub_wq hub_event
Call Trace:
<TASK>
context_switch kernel/sched/core.c:5382 [inline]
__schedule+0xc79/0x30a0 kernel/sched/core.c:6695
schedule+0xe7/0x1b0 kernel/sched/core.c:6771
schedule_preempt_disabled+0x13/0x20 kernel/sched/core.c:6830
__mutex_lock_common kernel/locking/mutex.c:679 [inline]
__mutex_lock+0x967/0x1340 kernel/locking/mutex.c:747
__input_unregister_device+0x136/0x450 drivers/input/input.c:2219
input_unregister_device+0xb9/0x100 drivers/input/input.c:2440
hidinput_disconnect+0x160/0x3e0 drivers/hid/hid-input.c:2386
hid_disconnect+0x143/0x1b0 drivers/hid/hid-core.c:2273
hid_hw_stop drivers/hid/hid-core.c:2322 [inline]
hid_device_remove+0x1a5/0x250 drivers/hid/hid-core.c:2684
device_remove+0xc8/0x170 drivers/base/dd.c:567
__device_release_driver drivers/base/dd.c:1272 [inline]
device_release_driver_internal+0x44a/0x610 drivers/base/dd.c:1295
bus_remove_device+0x22c/0x420 drivers/base/bus.c:574
device_del+0x39a/0xa50 drivers/base/core.c:3811
hid_remove_device drivers/hid/hid-core.c:2859 [inline]
hid_destroy_device+0xe5/0x150 drivers/hid/hid-core.c:2879
usbhid_disconnect+0xa0/0xe0 drivers/hid/usbhid/hid-core.c:1456
usb_unbind_interface+0x1dd/0x8d0 drivers/usb/core/driver.c:458
device_remove drivers/base/dd.c:569 [inline]
device_remove+0x11f/0x170 drivers/base/dd.c:561
__device_release_driver drivers/base/dd.c:1272 [inline]
device_release_driver_internal+0x44a/0x610 drivers/base/dd.c:1295
bus_remove_device+0x22c/0x420 drivers/base/bus.c:574
device_del+0x39a/0xa50 drivers/base/core.c:3811
usb_disable_device+0x36c/0x7f0 drivers/usb/core/message.c:1416
usb_disconnect+0x2e1/0x890 drivers/usb/core/hub.c:2252
hub_port_connect drivers/usb/core/hub.c:5280 [inline]
hub_port_connect_change drivers/usb/core/hub.c:5580 [inline]
port_event drivers/usb/core/hub.c:5740 [inline]
hub_event+0x1be0/0x4f30 drivers/usb/core/hub.c:5822
process_one_work+0x887/0x15d0 kernel/workqueue.c:2630
process_scheduled_works kernel/workqueue.c:2703 [inline]
worker_thread+0x8bb/0x1290 kernel/workqueue.c:2784
kthread+0x33a/0x430 kernel/kthread.c:388
ret_from_fork+0x45/0x80 arch/x86/kernel/process.c:147
ret_from_fork_asm+0x11/0x20 arch/x86/entry/entry_64.S:304
</TASK>
INFO: task jbd2/sda1-8:2348 blocked for more than 143 seconds.
Not tainted 6.6.0-rc1-syzkaller #0
"echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
task:jbd2/sda1-8 state:D stack:26656 pid:2348 ppid:2 flags:0x00004000
Call Trace:
<TASK>
context_switch kernel/sched/core.c:5382 [inline]
__schedule+0xc79/0x30a0 kernel/sched/core.c:6695
schedule+0xe7/0x1b0 kernel/sched/core.c:6771
io_schedule+0xbe/0x130 kernel/sched/core.c:9026
bit_wait_io+0x16/0xe0 kernel/sched/wait_bit.c:209
__wait_on_bit+0x62/0x170 kernel/sched/wait_bit.c:49
out_of_line_wait_on_bit+0xdb/0x110 kernel/sched/wait_bit.c:64
wait_on_bit_io include/linux/wait_bit.h:101 [inline]
__wait_on_buffer+0x64/0x70 fs/buffer.c:123
wait_on_buffer include/linux/buffer_head.h:370 [inline]
journal_wait_on_commit_record fs/jbd2/commit.c:171 [inline]
jbd2_journal_commit_transaction+0x4885/0x63d0 fs/jbd2/commit.c:890
kjournald2+0x1fb/0x900 fs/jbd2/journal.c:201
kthread+0x33a/0x430 kernel/kthread.c:388
ret_from_fork+0x45/0x80 arch/x86/kernel/process.c:147
ret_from_fork_asm+0x11/0x20 arch/x86/entry/entry_64.S:304
</TASK>
INFO: task udevd:2385 blocked for more than 143 seconds.
Not tainted 6.6.0-rc1-syzkaller #0
"echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
task:udevd state:D
stack:26672 pid:2385 ppid:1 flags:0x00004002
Call Trace:
<TASK>
context_switch kernel/sched/core.c:5382 [inline]
__schedule+0xc79/0x30a0 kernel/sched/core.c:6695
schedule+0xe7/0x1b0 kernel/sched/core.c:6771
schedule_preempt_disabled+0x13/0x20 kernel/sched/core.c:6830
rwsem_down_read_slowpath+0x625/0xb20 kernel/locking/rwsem.c:1086
__down_read_common kernel/locking/rwsem.c:1250 [inline]
__down_read kernel/locking/rwsem.c:1263 [inline]
down_read+0xf0/0x470 kernel/locking/rwsem.c:1522
kernfs_dop_revalidate+0xf0/0x5a0 fs/kernfs/dir.c:1138
d_revalidate fs/namei.c:861 [inline]
d_revalidate fs/namei.c:858 [inline]
lookup_fast+0x232/0x520 fs/namei.c:1654
walk_component+0x5b/0x5a0 fs/namei.c:1997
link_path_walk.part.0.constprop.0+0x71f/0xce0 fs/namei.c:2328
link_path_walk fs/namei.c:2253 [inline]
path_openat+0x231/0x29c0 fs/namei.c:3792
do_filp_open+0x1de/0x430 fs/namei.c:3823
do_sys_openat2+0x176/0x1e0 fs/open.c:1422
do_sys_open fs/open.c:1437 [inline]
__do_sys_openat fs/open.c:1453 [inline]
__se_sys_openat fs/open.c:1448 [inline]
__x64_sys_openat+0x175/0x210 fs/open.c:1448
do_syscall_x64 arch/x86/entry/common.c:50 [inline]
do_syscall_64+0x38/0xb0 arch/x86/entry/common.c:80
entry_SYSCALL_64_after_hwframe+0x63/0xcd
RIP: 0033:0x7ff1d282a9a4
RSP: 002b:00007ffc58f5b980 EFLAGS: 00000246 ORIG_RAX: 0000000000000101
RAX: ffffffffffffffda RBX: 0000000000000008 RCX: 00007ff1d282a9a4
RDX: 0000000000080000 RSI: 00007ffc58f5bab8 RDI: 00000000ffffff9c
RBP: 00007ffc58f5bab8 R08: 0000000000000008 R09: 0000000000000001
R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000080000
R13: 000055dec8f90b42 R14: 0000000000000001 R15: 0000000000000000
</TASK>
INFO: task udevd:2500 blocked for more than 143 seconds.
Not tainted 6.6.0-rc1-syzkaller #0
"echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
task:udevd state:D
stack:27568 pid:2500 ppid:2385 flags:0x00004002
Call Trace:
<TASK>
context_switch kernel/sched/core.c:5382 [inline]
__schedule+0xc79/0x30a0 kernel/sched/core.c:6695
schedule+0xe7/0x1b0 kernel/sched/core.c:6771
schedule_preempt_disabled+0x13/0x20 kernel/sched/core.c:6830
rwsem_down_read_slowpath+0x625/0xb20 kernel/locking/rwsem.c:1086
__down_read_common kernel/locking/rwsem.c:1250 [inline]
__down_read kernel/locking/rwsem.c:1263 [inline]
down_read+0xf0/0x470 kernel/locking/rwsem.c:1522
kernfs_dop_revalidate+0xf0/0x5a0 fs/kernfs/dir.c:1138
d_revalidate fs/namei.c:861 [inline]
d_revalidate fs/namei.c:858 [inline]
lookup_fast+0x232/0x520 fs/namei.c:1654
walk_component+0x5b/0x5a0 fs/namei.c:1997
link_path_walk.part.0.constprop.0+0x71f/0xce0 fs/namei.c:2328
link_path_walk fs/namei.c:2252 [inline]
path_lookupat+0x93/0x770 fs/namei.c:2481
filename_lookup+0x1e7/0x5b0 fs/namei.c:2511
vfs_statx+0x160/0x430 fs/stat.c:277
vfs_fstatat+0xb3/0x140 fs/stat.c:332
__do_sys_newfstatat+0x98/0x110 fs/stat.c:502
do_syscall_x64 arch/x86/entry/common.c:50 [inline]
do_syscall_64+0x38/0xb0 arch/x86/entry/common.c:80
entry_SYSCALL_64_after_hwframe+0x63/0xcd
RIP: 0033:0x7ff1d282a5f4
RSP: 002b:00007ffc58f59b38 EFLAGS: 00000206
ORIG_RAX: 0000000000000106
RAX: ffffffffffffffda RBX: 000055decac27a40 RCX: 00007ff1d282a5f4
RDX: 00007ffc58f59b48 RSI: 00007ffc58f59bd8 RDI: 00000000ffffff9c
RBP: 000055decac3d3df R08: 000055decac3d3df R09: 0000000000000000
R10: 0000000000000100 R11: 0000000000000206 R12: 0000000000000000
R13: 000055decac27b30 R14: 00007ffc58f59bd8 R15: 000055dec8f91a04
</TASK>
INFO: task kworker/1:3:2503 blocked for more than 143 seconds.
Not tainted 6.6.0-rc1-syzkaller #0
"echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
task:kworker/1:3 state:D stack:22416 pid:2503 ppid:2 flags:0x00004000
Workqueue: events console_callback
Call Trace:
<TASK>
context_switch kernel/sched/core.c:5382 [inline]
__schedule+0xc79/0x30a0 kernel/sched/core.c:6695
schedule+0xe7/0x1b0 kernel/sched/core.c:6771
schedule_timeout+0x27a/0x2c0 kernel/time/timer.c:2143
___down_common kernel/locking/semaphore.c:225 [inline]
__down_common+0x328/0x6c0 kernel/locking/semaphore.c:246
down+0x74/0xa0 kernel/locking/semaphore.c:63
console_lock+0x96/0x150 kernel/printk/printk.c:2652
console_callback+0x63/0x4c0 drivers/tty/vt/vt.c:2933
process_one_work+0x887/0x15d0 kernel/workqueue.c:2630
process_scheduled_works kernel/workqueue.c:2703 [inline]
worker_thread+0x8bb/0x1290 kernel/workqueue.c:2784
kthread+0x33a/0x430 kernel/kthread.c:388
ret_from_fork+0x45/0x80 arch/x86/kernel/process.c:147
ret_from_fork_asm+0x11/0x20 arch/x86/entry/entry_64.S:304
</TASK>
INFO: task udevd:2507 blocked for more than 143 seconds.
Not tainted 6.6.0-rc1-syzkaller #0
"echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
task:udevd state:D
stack:27712 pid:2507 ppid:2385 flags:0x00000002
Call Trace:
<TASK>
context_switch kernel/sched/core.c:5382 [inline]
__schedule+0xc79/0x30a0 kernel/sched/core.c:6695
schedule+0xe7/0x1b0 kernel/sched/core.c:6771
schedule_preempt_disabled+0x13/0x20 kernel/sched/core.c:6830
rwsem_down_read_slowpath+0x625/0xb20 kernel/locking/rwsem.c:1086
__down_read_common kernel/locking/rwsem.c:1250 [inline]
__down_read kernel/locking/rwsem.c:1263 [inline]
down_read+0xf0/0x470 kernel/locking/rwsem.c:1522
kernfs_dop_revalidate+0xf0/0x5a0 fs/kernfs/dir.c:1138
d_revalidate fs/namei.c:861 [inline]
d_revalidate fs/namei.c:858 [inline]
lookup_fast+0x232/0x520 fs/namei.c:1654
walk_component+0x5b/0x5a0 fs/namei.c:1997
link_path_walk.part.0.constprop.0+0x71f/0xce0 fs/namei.c:2328
link_path_walk fs/namei.c:2252 [inline]
path_lookupat+0x93/0x770 fs/namei.c:2481
filename_lookup+0x1e7/0x5b0 fs/namei.c:2511
vfs_statx+0x160/0x430 fs/stat.c:277
vfs_fstatat+0xb3/0x140 fs/stat.c:332
__do_sys_newfstatat+0x98/0x110 fs/stat.c:502
do_syscall_x64 arch/x86/entry/common.c:50 [inline]
do_syscall_64+0x38/0xb0 arch/x86/entry/common.c:80
entry_SYSCALL_64_after_hwframe+0x63/0xcd
RIP: 0033:0x7ff1d282a5f4
RSP: 002b:00007ffc58f58e98 EFLAGS: 00000206
ORIG_RAX: 0000000000000106
RAX: ffffffffffffffda RBX: 000055decac14d40 RCX: 00007ff1d282a5f4
RDX: 00007ffc58f58ea8 RSI: 00007ffc58f58f38 RDI: 00000000ffffff9c
RBP: 00007ffc58f5a390 R08: 00007ffc58f5a390 R09: 0000000000000000
R10: 0000000000000100 R11: 0000000000000206 R12: 0000000000000000
R13: 000055decac14e30 R14: 00007ffc58f58f38 R15: 0000000000000001
</TASK>
INFO: task udevd:2508 blocked for more than 143 seconds.
Not tainted 6.6.0-rc1-syzkaller #0
"echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
task:udevd state:D stack:26768 pid:2508 ppid:2385 flags:0x00004002
Call Trace:
<TASK>
context_switch kernel/sched/core.c:5382 [inline]
__schedule+0xc79/0x30a0 kernel/sched/core.c:6695
schedule+0xe7/0x1b0 kernel/sched/core.c:6771
schedule_preempt_disabled+0x13/0x20 kernel/sched/core.c:6830
rwsem_down_read_slowpath+0x625/0xb20 kernel/locking/rwsem.c:1086
__down_read_common kernel/locking/rwsem.c:1250 [inline]
__down_read kernel/locking/rwsem.c:1263 [inline]
down_read+0xf0/0x470 kernel/locking/rwsem.c:1522
kernfs_dop_revalidate+0xf0/0x5a0 fs/kernfs/dir.c:1138
d_revalidate fs/namei.c:861 [inline]
d_revalidate fs/namei.c:858 [inline]
lookup_fast+0x232/0x520 fs/namei.c:1654
walk_component+0x5b/0x5a0 fs/namei.c:1997
link_path_walk.part.0.constprop.0+0x71f/0xce0 fs/namei.c:2328
link_path_walk fs/namei.c:2252 [inline]
path_lookupat+0x93/0x770 fs/namei.c:2481
filename_lookup+0x1e7/0x5b0 fs/namei.c:2511
vfs_statx+0x160/0x430 fs/stat.c:277
vfs_fstatat+0xb3/0x140 fs/stat.c:332
__do_sys_newfstatat+0x98/0x110 fs/stat.c:502
do_syscall_x64 arch/x86/entry/common.c:50 [inline]
do_syscall_64+0x38/0xb0 arch/x86/entry/common.c:80
entry_SYSCALL_64_after_hwframe+0x63/0xcd
RIP: 0033:0x7ff1d282a5f4
RSP: 002b:00007ffc58f59b38 EFLAGS: 00000206
ORIG_RAX: 0000000000000106
RAX: ffffffffffffffda RBX: 000055decac33730 RCX: 00007ff1d282a5f4
RDX: 00007ffc58f59b48 RSI: 00007ffc58f59bd8 RDI: 00000000ffffff9c
RBP: 000055decac3dcbb R08: 000055decac3dcbb R09: 0000000000000000
R10: 0000000000000100 R11: 0000000000000206 R12: 0000000000000000
R13: 000055decac33820 R14: 00007ffc58f59bd8 R15: 000055dec8f91a04
</TASK>
INFO: task kworker/0:6:2525 blocked for more than 143 seconds.
Not tainted 6.6.0-rc1-syzkaller #0
"echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
task:kworker/0:6 state:D
stack:22912 pid:2525 ppid:2 flags:0x00004000
Workqueue: usb_hub_wq hub_event
Call Trace:
<TASK>
context_switch kernel/sched/core.c:5382 [inline]
__schedule+0xc79/0x30a0 kernel/sched/core.c:6695
schedule+0xe7/0x1b0 kernel/sched/core.c:6771
schedule_preempt_disabled+0x13/0x20 kernel/sched/core.c:6830
rwsem_down_read_slowpath+0x625/0xb20 kernel/locking/rwsem.c:1086
__down_read_common kernel/locking/rwsem.c:1250 [inline]
__down_read kernel/locking/rwsem.c:1263 [inline]
down_read+0xf0/0x470 kernel/locking/rwsem.c:1522
kernfs_find_and_get_ns+0x71/0xc0 fs/kernfs/dir.c:897
kernfs_find_and_get include/linux/kernfs.h:601 [inline]
sysfs_unmerge_group+0x61/0x170 fs/sysfs/group.c:369
dpm_sysfs_remove+0x68/0xb0 drivers/base/power/sysfs.c:833
device_del+0x1a8/0xa50 drivers/base/core.c:3786
device_unregister+0x1d/0xc0 drivers/base/core.c:3852
usb_remove_ep_devs+0x42/0x80 drivers/usb/core/endpoint.c:188
remove_intf_ep_devs drivers/usb/core/message.c:1264 [inline]
usb_disable_device+0x322/0x7f0 drivers/usb/core/message.c:1415
usb_disconnect+0x2e1/0x890 drivers/usb/core/hub.c:2252
hub_port_connect drivers/usb/core/hub.c:5280 [inline]
hub_port_connect_change drivers/usb/core/hub.c:5580 [inline]
port_event drivers/usb/core/hub.c:5740 [inline]
hub_event+0x1be0/0x4f30 drivers/usb/core/hub.c:5822
process_one_work+0x887/0x15d0 kernel/workqueue.c:2630
process_scheduled_works kernel/workqueue.c:2703 [inline]
worker_thread+0x8bb/0x1290 kernel/workqueue.c:2784
kthread+0x33a/0x430 kernel/kthread.c:388
ret_from_fork+0x45/0x80 arch/x86/kernel/process.c:147
ret_from_fork_asm+0x11/0x20 arch/x86/entry/entry_64.S:304
</TASK>
INFO: task udevd:2537 blocked for more than 143 seconds.
Not tainted 6.6.0-rc1-syzkaller #0
"echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
task:udevd state:D
stack:27712 pid:2537 ppid:2385 flags:0x00004002
Call Trace:
<TASK>
context_switch kernel/sched/core.c:5382 [inline]
__schedule+0xc79/0x30a0 kernel/sched/core.c:6695
schedule+0xe7/0x1b0 kernel/sched/core.c:6771
schedule_preempt_disabled+0x13/0x20 kernel/sched/core.c:6830
rwsem_down_read_slowpath+0x625/0xb20 kernel/locking/rwsem.c:1086
__down_read_common kernel/locking/rwsem.c:1250 [inline]
__down_read kernel/locking/rwsem.c:1263 [inline]
down_read+0xf0/0x470 kernel/locking/rwsem.c:1522
kernfs_dop_revalidate+0xf0/0x5a0 fs/kernfs/dir.c:1138
d_revalidate fs/namei.c:861 [inline]
d_revalidate fs/namei.c:858 [inline]
lookup_fast+0x232/0x520 fs/namei.c:1654
walk_component+0x5b/0x5a0 fs/namei.c:1997
link_path_walk.part.0.constprop.0+0x71f/0xce0 fs/namei.c:2328
link_path_walk fs/namei.c:2252 [inline]
path_lookupat+0x93/0x770 fs/namei.c:2481
filename_lookup+0x1e7/0x5b0 fs/namei.c:2511
user_path_at_empty+0x42/0x60 fs/namei.c:2910
do_readlinkat+0xdd/0x2f0 fs/stat.c:533
__do_sys_readlink fs/stat.c:566 [inline]
__se_sys_readlink fs/stat.c:563 [inline]
__x64_sys_readlink+0x78/0xb0 fs/stat.c:563
do_syscall_x64 arch/x86/entry/common.c:50 [inline]
do_syscall_64+0x38/0xb0 arch/x86/entry/common.c:80
entry_SYSCALL_64_after_hwframe+0x63/0xcd
RIP: 0033:0x7ff1d282bd47
RSP: 002b:00007ffc58f59a48 EFLAGS: 00000246
ORIG_RAX: 0000000000000059
RAX: ffffffffffffffda RBX: 00007ffc58f59a58 RCX: 00007ff1d282bd47
RDX: 0000000000000400 RSI: 00007ffc58f59a58 RDI: 00007ffc58f59f38
RBP: 0000000000000400 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000010 R11: 0000000000000246 R12: 00007ffc58f59f38
R13: 00007ffc58f59ea8 R14: 000055decac14910 R15: 00007ffc58f5b049
</TASK>
INFO: task syz-executor212:2596 blocked for more than 143 seconds.
Not tainted 6.6.0-rc1-syzkaller #0
"echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
task:syz-executor212 state:D stack:28864 pid:2596 ppid:2487 flags:0x00004006
Call Trace:
<TASK>
context_switch kernel/sched/core.c:5382 [inline]
__schedule+0xc79/0x30a0 kernel/sched/core.c:6695
schedule+0xe7/0x1b0 kernel/sched/core.c:6771
schedule_preempt_disabled+0x13/0x20 kernel/sched/core.c:6830
rwsem_down_write_slowpath+0x53e/0x1290 kernel/locking/rwsem.c:1178
__down_write_common kernel/locking/rwsem.c:1306 [inline]
__down_write kernel/locking/rwsem.c:1315 [inline]
down_write+0x1d3/0x200 kernel/locking/rwsem.c:1574
kernfs_add_one+0xb1/0x510 fs/kernfs/dir.c:757
kernfs_create_dir_ns+0x181/0x210 fs/kernfs/dir.c:1050
sysfs_create_dir_ns+0x13b/0x2a0 fs/sysfs/dir.c:59
create_dir lib/kobject.c:73 [inline]
kobject_add_internal+0x2c8/0x960 lib/kobject.c:238
kobject_add_varg lib/kobject.c:372 [inline]
kobject_init_and_add+0x11c/0x190 lib/kobject.c:455
bus_add_driver+0x186/0x630 drivers/base/bus.c:666
driver_register+0x15c/0x4a0 drivers/base/driver.c:246
usb_gadget_register_driver_owner+0xfd/0x2d0 drivers/usb/gadget/udc/core.c:1674
raw_ioctl_run drivers/usb/gadget/legacy/raw_gadget.c:548 [inline]
raw_ioctl+0x172f/0x2b80 drivers/usb/gadget/legacy/raw_gadget.c:1255
vfs_ioctl fs/ioctl.c:51 [inline]
__do_sys_ioctl fs/ioctl.c:871 [inline]
__se_sys_ioctl fs/ioctl.c:857 [inline]
__x64_sys_ioctl+0x18f/0x210 fs/ioctl.c:857
do_syscall_x64 arch/x86/entry/common.c:50 [inline]
do_syscall_64+0x38/0xb0 arch/x86/entry/common.c:80
entry_SYSCALL_64_after_hwframe+0x63/0xcd
RIP: 0033:0x7ff0f81f082b
RSP: 002b:00007ffdd1f948b0 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
RAX: ffffffffffffffda RBX: 0000000000000003 RCX: 00007ff0f81f082b
RDX: 0000000000000000 RSI: 0000000000005501 RDI: 0000000000000003
RBP: 00007ffdd1f95970 R08: 0000000000000010 R09: 00312e6364755f79
R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000000
R13: 00007ffdd1f94940 R14: 00007ffdd1f969e0 R15: 00007ff0f82693c0
</TASK>
Future hung task reports are suppressed, see sysctl kernel.hung_task_warnings
Showing all locks held in the system:
7 locks held by kworker/0:0/8:
#0: ffff88810aa6dd38 ((wq_completion)usb_hub_wq
){+.+.}-{0:0}, at: process_one_work+0x774/0x15d0 kernel/workqueue.c:2602
#1: ffffc9000008fd88 ((work_completion)(&hub->events)){+.+.}-{0:0}
, at: process_one_work+0x7a7/0x15d0 kernel/workqueue.c:2605
#2: ffff888106335190 (
&dev->mutex){....}-{3:3}
, at: device_lock include/linux/device.h:992 [inline]
, at: hub_event+0x1be/0x4f30 drivers/usb/core/hub.c:5768
#3: ffff88811bed8190 (&dev->mutex){....}-{3:3}
, at: device_lock include/linux/device.h:992 [inline]
, at: __device_attach+0x7f/0x4b0 drivers/base/dd.c:1005
#4: ffff88811bfbd160 (&dev->mutex){....}-{3:3}
, at: device_lock include/linux/device.h:992 [inline]
, at: __device_attach+0x7f/0x4b0 drivers/base/dd.c:1005
#5:
ffff88811d4b1a20 (&dev->mutex){....}-{3:3}
, at: device_lock include/linux/device.h:992 [inline]
, at: __device_attach+0x7f/0x4b0 drivers/base/dd.c:1005
#6: ffffffff88aa3328
(input_mutex
){+.+.}-{3:3}
, at: input_register_device+0xa27/0x1130 drivers/input/input.c:2389
7 locks held by kworker/1:0/22:
#0:
ffff88810aa6dd38 ((wq_completion)usb_hub_wq
){+.+.}-{0:0}
, at: process_one_work+0x774/0x15d0 kernel/workqueue.c:2602
#1: ffffc9000017fd88 ((work_completion)(&hub->events)
){+.+.}-{0:0}, at: process_one_work+0x7a7/0x15d0 kernel/workqueue.c:2605
#2: ffff888105bf5190 (
&dev->mutex){....}-{3:3}
, at: device_lock include/linux/device.h:992 [inline]
, at: hub_event+0x1be/0x4f30 drivers/usb/core/hub.c:5768
#3: ffff8881158c8190
(&dev->mutex){....}-{3:3}
, at: device_lock include/linux/device.h:992 [inline]
, at: usb_disconnect+0x10a/0x890 drivers/usb/core/hub.c:2243
#4:
ffff88811beda160 (&dev->mutex
){....}-{3:3}, at: device_lock include/linux/device.h:992 [inline]
){....}-{3:3}, at: __device_driver_lock drivers/base/dd.c:1095 [inline]
){....}-{3:3}, at: device_release_driver_internal+0xa4/0x610 drivers/base/dd.c:1292
#5: ffff888109fc1a20 (
&dev->mutex){....}-{3:3}
, at: device_lock include/linux/device.h:992 [inline]
, at: __device_driver_lock drivers/base/dd.c:1095 [inline]
, at: device_release_driver_internal+0xa4/0x610 drivers/base/dd.c:1292
#6: ffffffff88aa3328 (input_mutex){+.+.}-{3:3}
, at: __input_unregister_device+0x136/0x450 drivers/input/input.c:2219
1 lock held by khungtaskd/29:
#0: ffffffff87eac820 (rcu_read_lock
){....}-{1:2}, at: debug_show_all_locks+0x55/0x340 kernel/locking/lockdep.c:6607
7 locks held by kworker/1:2/1781:
#0:
ffff88810aa6dd38 ((wq_completion)usb_hub_wq){+.+.}-{0:0}
, at: process_one_work+0x774/0x15d0 kernel/workqueue.c:2602
#1: ffffc900035bfd88
((work_completion)(&hub->events)){+.+.}-{0:0}
, at: process_one_work+0x7a7/0x15d0 kernel/workqueue.c:2605
#2:
ffff888105fe5190 (&dev->mutex){....}-{3:3}
, at: device_lock include/linux/device.h:992 [inline]
, at: hub_event+0x1be/0x4f30 drivers/usb/core/hub.c:5768
#3: ffff88811b744190
(&dev->mutex){....}-{3:3}
, at: device_lock include/linux/device.h:992 [inline]
, at: __device_attach+0x7f/0x4b0 drivers/base/dd.c:1005
#4:
ffff88811b84e160 (&dev->mutex){....}-{3:3}
, at: device_lock include/linux/device.h:992 [inline]
, at: __device_attach+0x7f/0x4b0 drivers/base/dd.c:1005
#5:
ffff8881123fda20 (&dev->mutex){....}-{3:3}
, at: device_lock include/linux/device.h:992 [inline]
, at: __device_attach+0x7f/0x4b0 drivers/base/dd.c:1005
#6:
ffffffff88aa3328 (
input_mutex
){+.+.}-{3:3}
, at: input_register_device+0xa27/0x1130 drivers/input/input.c:2389
1 lock held by udevd/2385:
#0:
ffff88810165f948 (&root->kernfs_rwsem
){++++}-{3:3}
, at: kernfs_dop_revalidate+0xf0/0x5a0 fs/kernfs/dir.c:1138
2 locks held by getty/2443:
#0:
ffff88810bb160a0 (&tty->ldisc_sem
){++++}-{0:0}, at: tty_ldisc_ref_wait+0x24/0x80 drivers/tty/tty_ldisc.c:243
#1: ffffc900000432f0 (
&ldata->atomic_read_lock){+.+.}-{3:3}
, at: n_tty_read+0xfc5/0x1480 drivers/tty/n_tty.c:2206
1 lock held by udevd/2500:
#0: ffff88810165f948 (&root->kernfs_rwsem
){++++}-{3:3}, at: kernfs_dop_revalidate+0xf0/0x5a0 fs/kernfs/dir.c:1138
2 locks held by kworker/1:3/2503:
#0: ffff888100070d38 ((wq_completion)events
){+.+.}-{0:0}, at: process_one_work+0x774/0x15d0 kernel/workqueue.c:2602
#1: ffffc9000156fd88
(console_work){+.+.}-{0:0}
, at: process_one_work+0x7a7/0x15d0 kernel/workqueue.c:2605
1 lock held by udevd/2507:
#0: ffff88810165f948 (&root->kernfs_rwsem
){++++}-{3:3}, at: kernfs_dop_revalidate+0xf0/0x5a0 fs/kernfs/dir.c:1138
1 lock held by udevd/2508:
#0:
ffff88810165f948 (&root->kernfs_rwsem){++++}-{3:3}
, at: kernfs_dop_revalidate+0xf0/0x5a0 fs/kernfs/dir.c:1138
9 locks held by kworker/1:4/2514:
10 locks held by kworker/1:6/2517:
5 locks held by kworker/0:6/2525:
#0:
ffff88810aa6dd38 ((wq_completion)usb_hub_wq){+.+.}-{0:0}
, at: process_one_work+0x774/0x15d0 kernel/workqueue.c:2602
#1: ffffc9000148fd88 (
(work_completion)(&hub->events)){+.+.}-{0:0}
, at: process_one_work+0x7a7/0x15d0 kernel/workqueue.c:2605
#2: ffff888105fbd190 (
&dev->mutex){....}-{3:3}, at: device_lock include/linux/device.h:992 [inline]
&dev->mutex){....}-{3:3}, at: hub_event+0x1be/0x4f30 drivers/usb/core/hub.c:5768
#3: ffff88811b743190 (
&dev->mutex){....}-{3:3}, at: device_lock include/linux/device.h:992 [inline]
&dev->mutex){....}-{3:3}, at: usb_disconnect+0x10a/0x890 drivers/usb/core/hub.c:2243
#4: ffff88810165f948 (
&root->kernfs_rwsem){++++}-{3:3}, at: kernfs_find_and_get_ns+0x71/0xc0 fs/kernfs/dir.c:897
1 lock held by udevd/2537:
#0: ffff88810165f948
(&root->kernfs_rwsem){++++}-{3:3}
, at: kernfs_dop_revalidate+0xf0/0x5a0 fs/kernfs/dir.c:1138
1 lock held by syz-executor212/2596:
#0: ffff88810165f948 (
&root->kernfs_rwsem
){++++}-{3:3}
, at: kernfs_add_one+0xb1/0x510 fs/kernfs/dir.c:757
=============================================
NMI backtrace for cpu 0
CPU: 0 PID: 29 Comm: khungtaskd Not tainted 6.6.0-rc1-syzkaller #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 08/04/2023
Call Trace:
<TASK>
__dump_stack lib/dump_stack.c:88 [inline]
dump_stack_lvl+0xd9/0x1b0 lib/dump_stack.c:106
nmi_cpu_backtrace+0x277/0x380 lib/nmi_backtrace.c:113
nmi_trigger_cpumask_backtrace+0x26c/0x2d0 lib/nmi_backtrace.c:62
trigger_all_cpu_backtrace include/linux/nmi.h:160 [inline]
check_hung_uninterruptible_tasks kernel/hung_task.c:222 [inline]
watchdog+0xfac/0x1230 kernel/hung_task.c:379
kthread+0x33a/0x430 kernel/kthread.c:388
ret_from_fork+0x45/0x80 arch/x86/kernel/process.c:147
ret_from_fork_asm+0x11/0x20 arch/x86/entry/entry_64.S:304
</TASK>
Sending NMI from CPU 0 to CPUs 1:
NMI backtrace for cpu 1
CPU: 1 PID: 2517 Comm: kworker/1:6 Not tainted 6.6.0-rc1-syzkaller #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 08/04/2023
Workqueue: usb_hub_wq hub_event
RIP: 0010:__sanitizer_cov_trace_const_cmp2+0x8/0x20 kernel/kcov.c:297
Code: 00 00 f3 0f 1e fa 48 8b 0c 24 40 0f b6 d6 40 0f b6 f7 bf 01 00 00 00 e9 d6 fe ff ff 66 0f 1f 44 00 00 f3 0f 1e fa 48 8b 0c 24 <0f> b7 d6 0f b7 f7 bf 03 00 00 00 e9 b8 fe ff ff 0f 1f 84 00 00 00
RSP: 0018:ffffc90000198510 EFLAGS: 00000046
RAX: 0000000000000002 RBX: 0000000000000002 RCX: ffffffff813144a0
RDX: ffff88810fff3a00 RSI: 0000000000000002 RDI: 0000000000000000
RBP: ffffffff8bfc2610 R08: 0000000000000001 R09: 0000000000000001
R10: 0000000000000020 R11: 000000000bf26868 R12: 0000000000000000
R13: 000000000000001a R14: ffffc900001985b0 R15: 000000000000000a
FS: 0000000000000000(0000) GS:ffff8881f6700000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000555557349ca8 CR3: 000000010a34a000 CR4: 00000000003506e0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Call Trace:
<NMI>
</NMI>
<IRQ>
printk_sprint+0x220/0x2f0 kernel/printk/printk.c:2141
vprintk_store+0x4d8/0xb80 kernel/printk/printk.c:2242
vprintk_emit+0x154/0x630 kernel/printk/printk.c:2288
vprintk+0x7b/0x90 kernel/printk/printk_safe.c:45
_printk+0xc8/0x100 kernel/printk/printk.c:2332
printk_stack_address arch/x86/kernel/dumpstack.c:72 [inline]
show_trace_log_lvl+0x2ac/0x4f0 arch/x86/kernel/dumpstack.c:285
sched_show_task kernel/sched/core.c:9182 [inline]
sched_show_task+0x3f4/0x600 kernel/sched/core.c:9156
show_state_filter+0xeb/0x310 kernel/sched/core.c:9227
k_spec drivers/tty/vt/keyboard.c:667 [inline]
k_spec+0xea/0x140 drivers/tty/vt/keyboard.c:656
kbd_keycode drivers/tty/vt/keyboard.c:1524 [inline]
kbd_event+0xcc8/0x17c0 drivers/tty/vt/keyboard.c:1543
input_to_handler+0x382/0x4c0 drivers/input/input.c:132
input_pass_values.part.0+0x536/0x7a0 drivers/input/input.c:161
input_pass_values drivers/input/input.c:148 [inline]
input_event_dispose+0x5ee/0x770 drivers/input/input.c:378
input_handle_event+0x11c/0xd80 drivers/input/input.c:406
input_repeat_key+0x251/0x340 drivers/input/input.c:2263
call_timer_fn+0x1a0/0x580 kernel/time/timer.c:1700
expire_timers kernel/time/timer.c:1751 [inline]
__run_timers+0x764/0xb10 kernel/time/timer.c:2022
run_timer_softirq+0x58/0xd0 kernel/time/timer.c:2035
__do_softirq+0x20b/0x94e kernel/softirq.c:553
invoke_softirq kernel/softirq.c:427 [inline]
__irq_exit_rcu kernel/softirq.c:632 [inline]
irq_exit_rcu+0xa7/0x110 kernel/softirq.c:644
sysvec_apic_timer_interrupt+0x8e/0xb0 arch/x86/kernel/apic/apic.c:1074
</IRQ>
<TASK>
asm_sysvec_apic_timer_interrupt+0x1a/0x20 arch/x86/include/asm/idtentry.h:645
RIP: 0010:console_flush_all+0x9d2/0xf70 kernel/printk/printk.c:2972
Code: b4 0f 23 00 9c 5b 81 e3 00 02 00 00 31 ff 48 89 de e8 42 64 1c 00 48 85 db 0f 85 95 03 00 00 e8 a4 68 1c 00 fb 48 8b 44 24 08 <48> 8b 14 24 0f b6 00 83 e2 07 38 d0 7f 08 84 c0 0f 85 9e 04 00 00
RSP: 0018:ffffc900016bf5a0 EFLAGS: 00000293
RAX: fffff520002d7edf RBX: 0000000000000000 RCX: 0000000000000000
RDX: ffff88810fff3a00 RSI: ffffffff813107bc RDI: 0000000000000007
RBP: ffffffff88353b80 R08: 0000000000000007 R09: 0000000000000000
R10: 0000000000000000 R11: 205d314320202020 R12: 0000000000000000
R13: dffffc0000000000 R14: ffffffff88353bd8 R15: 0000000000000001
console_unlock+0x10c/0x260 kernel/printk/printk.c:3035
vprintk_emit+0x189/0x630 kernel/printk/printk.c:2307
dev_vprintk_emit drivers/base/core.c:4847 [inline]
dev_printk_emit+0xfb/0x140 drivers/base/core.c:4858
__dev_printk+0xf5/0x270 drivers/base/core.c:4870
_dev_info+0xe5/0x120 drivers/base/core.c:4916
usb_disconnect+0xec/0x890 drivers/usb/core/hub.c:2234
hub_port_connect drivers/usb/core/hub.c:5280 [inline]
hub_port_connect_change drivers/usb/core/hub.c:5580 [inline]
port_event drivers/usb/core/hub.c:5740 [inline]
hub_event+0x1be0/0x4f30 drivers/usb/core/hub.c:5822
process_one_work+0x887/0x15d0 kernel/workqueue.c:2630
process_scheduled_works kernel/workqueue.c:2703 [inline]
worker_thread+0x8bb/0x1290 kernel/workqueue.c:2784
kthread+0x33a/0x430 kernel/kthread.c:388
ret_from_fork+0x45/0x80 arch/x86/kernel/process.c:147
ret_from_fork_asm+0x11/0x20 arch/x86/entry/entry_64.S:304
</TASK>
INFO: NMI handler (nmi_cpu_backtrace_handler) took too long to run: 3.678 msecs
---
This report is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.
syzbot will keep track of this issue. See:
https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
If the bug is already fixed, let syzbot know by replying with:
#syz fix: exact-commit-title
If you want syzbot to run the reproducer, reply with:
#syz test: git://repo/address.git branch-or-commit-hash
If you attach or paste a git patch, syzbot will apply it before testing.
If you want to overwrite bug's subsystems, reply with:
#syz set subsystems: new-subsystem
(See the list of subsystem names on the web dashboard)
If the bug is a duplicate of another bug, reply with:
#syz dup: exact-subject-of-another-report
If you want to undo deduplication, reply with:
#syz undup
^ permalink raw reply
* [PATCH v3 2/3] HID: nvidia-shield: Fix some missing function calls() in the probe error handling path
From: Rahul Rameshbabu @ 2023-09-18 11:54 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires
Cc: Christophe JAILLET, kernel-janitors, linux-input, linux-kernel,
Rahul Rameshbabu
In-Reply-To: <20230918115432.30076-1-rrameshbabu@nvidia.com>
From: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
The commit in Fixes updated the error handling path of
thunderstrike_create() and the remove function but not the error handling
path of shield_probe(), should an error occur after a successful
thunderstrike_create() call.
Add the missing calls.
Fixes: 3ab196f88237 ("HID: nvidia-shield: Add battery support for Thunderstrike")
Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
Reviewed-by: Rahul Rameshbabu <rrameshbabu@nvidia.com>
---
Notes:
Changes:
v1->v2:
- Rename err_haptics label to err_ts_create to make the label name more
accurate.
v2->v3:
- No changes.
drivers/hid/hid-nvidia-shield.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/hid/hid-nvidia-shield.c b/drivers/hid/hid-nvidia-shield.c
index c144641452d3..a566f9cdc97d 100644
--- a/drivers/hid/hid-nvidia-shield.c
+++ b/drivers/hid/hid-nvidia-shield.c
@@ -1058,7 +1058,7 @@ static int shield_probe(struct hid_device *hdev, const struct hid_device_id *id)
ret = hid_hw_start(hdev, HID_CONNECT_HIDINPUT);
if (ret) {
hid_err(hdev, "Failed to start HID device\n");
- goto err_haptics;
+ goto err_ts_create;
}
ret = hid_hw_open(hdev);
@@ -1073,10 +1073,12 @@ static int shield_probe(struct hid_device *hdev, const struct hid_device_id *id)
err_stop:
hid_hw_stop(hdev);
-err_haptics:
+err_ts_create:
+ power_supply_unregister(ts->base.battery_dev.psy);
if (ts->haptics_dev)
input_unregister_device(ts->haptics_dev);
led_classdev_unregister(&ts->led_dev);
+ ida_free(&thunderstrike_ida, ts->id);
return ret;
}
--
2.40.1
^ permalink raw reply related
* [PATCH v3 1/3] HID: nvidia-shield: Fix a missing led_classdev_unregister() in the probe error handling path
From: Rahul Rameshbabu @ 2023-09-18 11:54 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires
Cc: Christophe JAILLET, kernel-janitors, linux-input, linux-kernel,
Rahul Rameshbabu
In-Reply-To: <20230918115432.30076-1-rrameshbabu@nvidia.com>
From: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
The commit in Fixes updated the error handling path of
thunderstrike_create() and the remove function but not the error handling
path of shield_probe(), should an error occur after a successful
thunderstrike_create() call.
Add the missing call. Make sure it is safe to call in the probe error
handling path by preventing the led_classdev from attempting to set the LED
brightness to the off state on unregister.
Fixes: f88af60e74a5 ("HID: nvidia-shield: Support LED functionality for Thunderstrike")
Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
Reviewed-by: Rahul Rameshbabu <rrameshbabu@nvidia.com>
---
Notes:
Changes:
v1->v2:
- Add the LED_RETAIN_AT_SHUTDOWN flag to prevent
led_classdev_unregister from trying to set the LED to off before a
successful call to hid_hw_start.
v2->v3:
- No changes.
drivers/hid/hid-nvidia-shield.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/hid/hid-nvidia-shield.c b/drivers/hid/hid-nvidia-shield.c
index 43784bb57d3f..c144641452d3 100644
--- a/drivers/hid/hid-nvidia-shield.c
+++ b/drivers/hid/hid-nvidia-shield.c
@@ -801,7 +801,7 @@ static inline int thunderstrike_led_create(struct thunderstrike *ts)
led->name = devm_kasprintf(&ts->base.hdev->dev, GFP_KERNEL,
"thunderstrike%d:blue:led", ts->id);
led->max_brightness = 1;
- led->flags = LED_CORE_SUSPENDRESUME;
+ led->flags = LED_CORE_SUSPENDRESUME | LED_RETAIN_AT_SHUTDOWN;
led->brightness_get = &thunderstrike_led_get_brightness;
led->brightness_set = &thunderstrike_led_set_brightness;
@@ -1076,6 +1076,7 @@ static int shield_probe(struct hid_device *hdev, const struct hid_device_id *id)
err_haptics:
if (ts->haptics_dev)
input_unregister_device(ts->haptics_dev);
+ led_classdev_unregister(&ts->led_dev);
return ret;
}
--
2.40.1
^ permalink raw reply related
* [PATCH v3 0/3] HID: nvidia-shield: Fix the error handling path of shield_probe()
From: Rahul Rameshbabu @ 2023-09-18 11:54 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires
Cc: Christophe JAILLET, kernel-janitors, linux-input, linux-kernel,
Rahul Rameshbabu
This series fixes some missing clean-up function calls in the error handling of
the probe.
Patch 1 and 2 fix some similar issues introduced in 2 different commits (hence 2
patches)
Patch 3 is an enhancement that creates a common function for cleaning up
thunderstrike instances.
Changes:
v1->v2:
- Add the LED_RETAIN_AT_SHUTDOWN flag to prevent
led_classdev_unregister from trying to set the LED to off before a
successful call to hid_hw_start.
- Rename err_haptics label to err_ts_create to make the label name more
accurate.
- Re-order operations in thunderstrike_destroy to be in LIFO order with
regards to the operations in thunderstrike_create.
v2->v3:
- Refactor thunderstrike_destroy to take a thunderstrike instance
pointer as a parameter and prevent a variable from being unused
in shield_probe.
Link: https://lore.kernel.org/linux-input/cover.1693070958.git.christophe.jaillet@wanadoo.fr/
Link: https://lore.kernel.org/linux-input/20230918041345.59859-1-rrameshbabu@nvidia.com/
Notes from Rahul:
- Thank you so much Christophe for these patches.
- Sent v2 without accounting for the fact that thunderstrike_destroy in v1
makes the thunderstrike instance in shield_probe unused. Tested v3 with W=1.
Christophe JAILLET (3):
HID: nvidia-shield: Fix a missing led_classdev_unregister() in the
probe error handling path
HID: nvidia-shield: Fix some missing function calls() in the probe
error handling path
HID: nvidia-shield: Introduce thunderstrike_destroy()
drivers/hid/hid-nvidia-shield.c | 24 ++++++++++++++----------
1 file changed, 14 insertions(+), 10 deletions(-)
--
2.40.1
^ permalink raw reply
* [PATCH v3 3/3] HID: nvidia-shield: Introduce thunderstrike_destroy()
From: Rahul Rameshbabu @ 2023-09-18 11:54 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires
Cc: Christophe JAILLET, kernel-janitors, linux-input, linux-kernel,
Rahul Rameshbabu
In-Reply-To: <20230918115432.30076-1-rrameshbabu@nvidia.com>
From: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
In order to simplify some error handling paths and avoid code duplication,
introduce thunderstrike_destroy() which undoes thunderstrike_create().
Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
Reviewed-by: Rahul Rameshbabu <rrameshbabu@nvidia.com>
---
Notes:
Changes:
v1->v2:
- Re-order operations in thunderstrike_destroy to be in LIFO order with
regards to the operations in thunderstrike_create.
v2->v3:
- Refactor thunderstrike_destroy to take a thunderstrike instance
pointer as a parameter and prevent a variable from being unused
in shield_probe.
drivers/hid/hid-nvidia-shield.c | 21 +++++++++++----------
1 file changed, 11 insertions(+), 10 deletions(-)
diff --git a/drivers/hid/hid-nvidia-shield.c b/drivers/hid/hid-nvidia-shield.c
index a566f9cdc97d..bc47e19ef117 100644
--- a/drivers/hid/hid-nvidia-shield.c
+++ b/drivers/hid/hid-nvidia-shield.c
@@ -915,6 +915,15 @@ static struct shield_device *thunderstrike_create(struct hid_device *hdev)
return ERR_PTR(ret);
}
+static void thunderstrike_destroy(struct thunderstrike *ts)
+{
+ led_classdev_unregister(&ts->led_dev);
+ power_supply_unregister(ts->base.battery_dev.psy);
+ if (ts->haptics_dev)
+ input_unregister_device(ts->haptics_dev);
+ ida_free(&thunderstrike_ida, ts->id);
+}
+
static int android_input_mapping(struct hid_device *hdev, struct hid_input *hi,
struct hid_field *field,
struct hid_usage *usage, unsigned long **bit,
@@ -1074,11 +1083,7 @@ static int shield_probe(struct hid_device *hdev, const struct hid_device_id *id)
err_stop:
hid_hw_stop(hdev);
err_ts_create:
- power_supply_unregister(ts->base.battery_dev.psy);
- if (ts->haptics_dev)
- input_unregister_device(ts->haptics_dev);
- led_classdev_unregister(&ts->led_dev);
- ida_free(&thunderstrike_ida, ts->id);
+ thunderstrike_destroy(ts);
return ret;
}
@@ -1090,11 +1095,7 @@ static void shield_remove(struct hid_device *hdev)
ts = container_of(dev, struct thunderstrike, base);
hid_hw_close(hdev);
- power_supply_unregister(dev->battery_dev.psy);
- if (ts->haptics_dev)
- input_unregister_device(ts->haptics_dev);
- led_classdev_unregister(&ts->led_dev);
- ida_free(&thunderstrike_ida, ts->id);
+ thunderstrike_destroy(ts);
del_timer_sync(&ts->psy_stats_timer);
cancel_work_sync(&ts->hostcmd_req_work);
hid_hw_stop(hdev);
--
2.40.1
^ permalink raw reply related
* Re: [PATCH 2/8] iio: hid-sensor-als: Add light color temperature support
From: Jonathan Cameron @ 2023-09-18 10:58 UTC (permalink / raw)
To: Basavaraj Natikar
Cc: Jonathan Cameron, Basavaraj Natikar, jikos, benjamin.tissoires,
lars, srinivas.pandruvada, linux-input, linux-iio
In-Reply-To: <dbc3d5ed-f824-d428-923e-3f44011c44ad@amd.com>
On Sun, 17 Sep 2023 20:13:46 +0530
Basavaraj Natikar <bnatikar@amd.com> wrote:
> On 9/17/2023 4:30 PM, Jonathan Cameron wrote:
> > On Fri, 15 Sep 2023 10:46:57 +0530
> > Basavaraj Natikar <Basavaraj.Natikar@amd.com> wrote:
> >
> >> In most cases, ambient color sensors also support light color temperature.
> >> As a result, add support of light color temperature.
> >>
> >> Signed-off-by: Basavaraj Natikar <Basavaraj.Natikar@amd.com>
> >> ---
> >> drivers/iio/light/hid-sensor-als.c | 33 ++++++++++++++++++++++++++++++
> >> include/linux/hid-sensor-ids.h | 1 +
> >> 2 files changed, 34 insertions(+)
> >>
> >> diff --git a/drivers/iio/light/hid-sensor-als.c b/drivers/iio/light/hid-sensor-als.c
> >> index 48879e233aec..220fb93fea6d 100644
> >> --- a/drivers/iio/light/hid-sensor-als.c
> >> +++ b/drivers/iio/light/hid-sensor-als.c
> >> @@ -16,6 +16,7 @@
> >> enum {
> >> CHANNEL_SCAN_INDEX_INTENSITY = 0,
> >> CHANNEL_SCAN_INDEX_ILLUM = 1,
> > Either drop, the = 1 or keep consistency for TEMP.
> > I don't think the = 1 is useful so I'd drop it.
> >
> >> + CHANNEL_SCAN_INDEX_COLOR_TEMP,
> >> CHANNEL_SCAN_INDEX_MAX
> >> };
> >>
> >> @@ -65,6 +66,18 @@ static const struct iio_chan_spec als_channels[] = {
> >> BIT(IIO_CHAN_INFO_HYSTERESIS_RELATIVE),
> >> .scan_index = CHANNEL_SCAN_INDEX_ILLUM,
> >> },
> >> + {
> >> + .type = IIO_TEMP,
> > Using a temperature channel for color temp is a bit of a stretch,
> > Particularly as it's likely we will see light sensors with actual
> > air temperature sensors in them at somepoint even if we don't have
> > any already.
> >
> > So this needs a new channel type
> > IIO_COLORTEMP or similar.
> >
> > Units for this probably want to be kelvin unlike the mili decrees centigrade
> > used for IIO_TEMP.
> >
> >> + .modified = 1,
> >> + .channel2 = IIO_MOD_TEMP_AMBIENT,
> > I don't really see the modifier as useful here. That exists for thermocouple
> > type systems where it is necessary to know ambient vs sample temperatures.
>
> Sure Jonathan, I will address all comments in this series in v2.
> Also, can i add new channel type IIO_COLORTEMP with following channel index
> for light color temperature ?
Looks good.
Jonathan
> {
> .type = IIO_COLORTEMP,
> .info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
> .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_OFFSET) |
> BIT(IIO_CHAN_INFO_SCALE) |
> BIT(IIO_CHAN_INFO_SAMP_FREQ) |
> BIT(IIO_CHAN_INFO_HYSTERESIS) |
> BIT(IIO_CHAN_INFO_HYSTERESIS_RELATIVE),
> .scan_index = CHANNEL_SCAN_INDEX_COLOR_TEMP,
> }
>
> Thanks,
> --
> Basavaraj
>
> >
> >
> >> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
> >> + .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_OFFSET) |
> >> + BIT(IIO_CHAN_INFO_SCALE) |
> >> + BIT(IIO_CHAN_INFO_SAMP_FREQ) |
> >> + BIT(IIO_CHAN_INFO_HYSTERESIS) |
> >> + BIT(IIO_CHAN_INFO_HYSTERESIS_RELATIVE),
> >> + .scan_index = CHANNEL_SCAN_INDEX_COLOR_TEMP,
> >> + },
> >> IIO_CHAN_SOFT_TIMESTAMP(CHANNEL_SCAN_INDEX_TIMESTAMP)
> >> };
> >>
> >> @@ -103,6 +116,11 @@ static int als_read_raw(struct iio_dev *indio_dev,
> >> min = als_state->als_illum[chan->scan_index].logical_minimum;
> >> address = HID_USAGE_SENSOR_LIGHT_ILLUM;
> >> break;
> >> + case CHANNEL_SCAN_INDEX_COLOR_TEMP:
> >> + report_id = als_state->als_illum[chan->scan_index].report_id;
> >> + min = als_state->als_illum[chan->scan_index].logical_minimum;
> >> + address = HID_USAGE_SENSOR_LIGHT_COLOR_TEMPERATURE;
> >> + break;
> >> default:
> >> report_id = -1;
> >> break;
> >> @@ -223,6 +241,10 @@ static int als_capture_sample(struct hid_sensor_hub_device *hsdev,
> >> als_state->scan.illum[CHANNEL_SCAN_INDEX_ILLUM] = sample_data;
> >> ret = 0;
> >> break;
> >> + case HID_USAGE_SENSOR_LIGHT_COLOR_TEMPERATURE:
> >> + als_state->scan.illum[CHANNEL_SCAN_INDEX_COLOR_TEMP] = sample_data;
> >> + ret = 0;
> >> + break;
> >> case HID_USAGE_SENSOR_TIME_TIMESTAMP:
> >> als_state->timestamp = hid_sensor_convert_timestamp(&als_state->common_attributes,
> >> *(s64 *)raw_data);
> >> @@ -256,6 +278,17 @@ static int als_parse_report(struct platform_device *pdev,
> >> st->als_illum[i].report_id);
> >> }
> >>
> >> + ret = sensor_hub_input_get_attribute_info(hsdev, HID_INPUT_REPORT, usage_id,
> >> + HID_USAGE_SENSOR_LIGHT_COLOR_TEMPERATURE,
> >> + &st->als_illum[CHANNEL_SCAN_INDEX_COLOR_TEMP]);
> >> + if (ret < 0)
> >> + return ret;
> >> + als_adjust_channel_bit_mask(channels, CHANNEL_SCAN_INDEX_COLOR_TEMP,
> >> + st->als_illum[CHANNEL_SCAN_INDEX_COLOR_TEMP].size);
> >> +
> >> + dev_dbg(&pdev->dev, "als %x:%x\n", st->als_illum[CHANNEL_SCAN_INDEX_COLOR_TEMP].index,
> >> + st->als_illum[CHANNEL_SCAN_INDEX_COLOR_TEMP].report_id);
> >> +
> >> st->scale_precision = hid_sensor_format_scale(usage_id,
> >> &st->als_illum[CHANNEL_SCAN_INDEX_INTENSITY],
> >> &st->scale_pre_decml, &st->scale_post_decml);
> >> diff --git a/include/linux/hid-sensor-ids.h b/include/linux/hid-sensor-ids.h
> >> index 13b1e65fbdcc..8af4fb3e0254 100644
> >> --- a/include/linux/hid-sensor-ids.h
> >> +++ b/include/linux/hid-sensor-ids.h
> >> @@ -21,6 +21,7 @@
> >> #define HID_USAGE_SENSOR_ALS 0x200041
> >> #define HID_USAGE_SENSOR_DATA_LIGHT 0x2004d0
> >> #define HID_USAGE_SENSOR_LIGHT_ILLUM 0x2004d1
> >> +#define HID_USAGE_SENSOR_LIGHT_COLOR_TEMPERATURE 0x2004d2
> >>
> >> /* PROX (200011) */
> >> #define HID_USAGE_SENSOR_PROX 0x200011
>
^ permalink raw reply
* Re: [PATCH v4 00/42] ep93xx device tree conversion
From: Andy Shevchenko @ 2023-09-18 7:39 UTC (permalink / raw)
To: nikita.shubin
Cc: Hartley Sweeten, Alexander Sverdlin, Russell King,
Lukasz Majewski, Linus Walleij, Bartosz Golaszewski,
Michael Turquette, Stephen Boyd, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Sebastian Reichel, Daniel Lezcano, Thomas Gleixner,
Alessandro Zummo, Alexandre Belloni, Wim Van Sebroeck,
Guenter Roeck, Thierry Reding, Uwe Kleine-König, Mark Brown,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Vinod Koul, Miquel Raynal, Richard Weinberger,
Vignesh Raghavendra, Damien Le Moal, Sergey Shtylyov,
Dmitry Torokhov, Liam Girdwood, Jaroslav Kysela, Takashi Iwai,
linux-arm-kernel, linux-kernel, linux-gpio, linux-clk, devicetree,
linux-pm, linux-rtc, linux-watchdog, linux-pwm, linux-spi, netdev,
dmaengine, linux-mtd, linux-ide, linux-input, alsa-devel,
Arnd Bergmann, Bartosz Golaszewski, Krzysztof Kozlowski,
Andrew Lunn
In-Reply-To: <20230915-ep93xx-v4-0-a1d779dcec10@maquefel.me>
On Fri, Sep 15, 2023 at 11:10:42AM +0300, Nikita Shubin via B4 Relay wrote:
> This series aims to convert ep93xx from platform to full device tree support.
>
> The main goal is to receive ACK's to take it via Arnd's arm-soc branch.
>
> Major changes:
> - drop newline at the end from each YAML files
> - rename dma and clk bindings headers to match first compatible
> - shrink SoC exported functions number to only 2
> - dropped some ep93xx_pata fixes from these series
> - dropped m48t86 stuff from these series
>
> Bit thanks to Andy Shevchenko for thorough review.
You are welcome!
Dunno if you have used --patience when formatted the patches, but I think
you should, if hadn't, for the next version. It will help a lot in reviewing
and understanding the changes.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH] HID: uhid: refactor deprecated strncpy
From: David Rheinsberg @ 2023-09-18 7:37 UTC (permalink / raw)
To: Kees Cook
Cc: Justin Stitt, Jiri Kosina, Benjamin Tissoires, linux-input,
linux-kernel, linux-hardening, David Herrmann
In-Reply-To: <202309151342.DFA6CA5C7@keescook>
Hey
On Fri, Sep 15, 2023, at 10:48 PM, Kees Cook wrote:
> On Fri, Sep 15, 2023 at 09:36:23AM +0200, David Rheinsberg wrote:
>> Hi
>>
>> On Fri, Sep 15, 2023, at 7:13 AM, Kees Cook wrote:
>> >> - /* @hid is zero-initialized, strncpy() is correct, strlcpy() not */
>> >> - len = min(sizeof(hid->name), sizeof(ev->u.create2.name)) - 1;
>> >> - strncpy(hid->name, ev->u.create2.name, len);
>> >> - len = min(sizeof(hid->phys), sizeof(ev->u.create2.phys)) - 1;
>> >> - strncpy(hid->phys, ev->u.create2.phys, len);
>> >> - len = min(sizeof(hid->uniq), sizeof(ev->u.create2.uniq)) - 1;
>> >> - strncpy(hid->uniq, ev->u.create2.uniq, len);
>> >
>> > ev->u.create2 is:
>> > struct uhid_create2_req {
>> > __u8 name[128];
>> > __u8 phys[64];
>> > __u8 uniq[64];
>> > ...
>> >
>> > hid is:
>> > struct hid_device { /* device report descriptor */
>> > ...
>> > char name[128]; /* Device name */
>> > char phys[64]; /* Device physical location */
>> > char uniq[64]; /* Device unique identifier (serial #) */
>> >
>> > So these "min" calls are redundant -- it wants to copy at most 1 less so
>> > it can be %NUL terminated. Which is what strscpy() already does. And
>> > source and dest are the same size, so we can't over-read source if it
>> > weren't terminated (since strscpy won't overread like strlcpy).
>>
>> I *really* think we should keep the `min` calls. The compiler
>> should already optimize them away, as both arguments are compile-time
>> constants. There is no inherent reason why source and target are equal in
>> size. Yes, it is unlikely to change, but I don't understand why we would
>> want to implicitly rely on it, rather than make the compiler verify it for
>> us. And `struct hid_device` is very much allowed to change in the future.
>>
>> As an alternative, you can use BUILD_BUG_ON() and verify both are equal in length.
>
> If we can't depend on ev->u.create2.name/phys/uniq being %NUL-terminated,
> we've already done the "min" calculations, and we've already got the
> dest zeroed, then I suspect the thing to do is just use memcpy instead
> of strncpy (or strscpy).
If you use memcpy, you might copy garbage trailing the terminating zero. This is not particularly wrong, but also not really nice if user-space relies on the kernel to treat it as a string. You don't know whether a query of the string returns trailing bytes, and thus might expose data that user-space did not intend to share.
I mean, this is why the code uses strncpy().
Thanks
David
^ 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