* Re: [PATCH 05/33] Input: synaptics-rmi4 - follow renaming of SPI "master" to "controller"
From: Dmitry Torokhov @ 2024-01-16 19:14 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: Mark Brown, Geert Uytterhoeven, linux-spi, kernel, Ulf Hansson,
Greg Kroah-Hartman, Rayyan Ansari, Andy Shevchenko,
Jonathan Cameron, linux-input
In-Reply-To: <5257de51fe406cf8405310dd638f648a232f4a6c.1705348269.git.u.kleine-koenig@pengutronix.de>
On Mon, Jan 15, 2024 at 09:12:51PM +0100, Uwe Kleine-König wrote:
> In commit 8caab75fd2c2 ("spi: Generalize SPI "master" to "controller"")
> some functions and struct members were renamed. To not break all drivers
> compatibility macros were provided.
>
> To be able to remove these compatibility macros push the renaming into
> this driver.
>
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Acked-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Please feel free to merge with the rest of the series.
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH v2] Input: atkbd - Skip ATKBD_CMD_GETID in translated mode
From: Hans de Goede @ 2024-01-16 19:33 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Barnabás Pőcze, linux-input, Shang Ye, gurevitch,
Egor Ignatov, Anton Zhilyaev
In-Reply-To: <ZabTi1PbTl-uF5aM@google.com>
Hi,
On 1/16/24 20:05, Dmitry Torokhov wrote:
> On Tue, Jan 16, 2024 at 03:43:10PM +0100, Hans de Goede wrote:
>> Hi,
>>
>> On 1/16/24 14:32, Barnabás Pőcze wrote:
>>>
>>> After:
>>>
>>> evdev:input:b0011v0001p0001*
>>> KEYBOARD_KEY_f8=fn
>>> KEYBOARD_KEY_76=f21
>>>
>>> I: Bus=0011 Vendor=0001 Product=0001 Version=abba
>>> N: Name="AT Translated Set 2 keyboard"
>>> P: Phys=isa0060/serio0/input0
>>> S: Sysfs=/devices/platform/i8042/serio0/input/input4
>>
>> I see, thank you. There are no v0001p0001 matches
>> in the hwdb.d/60-keyboard.hwdb shipped with systems.
>>
>> Typically laptop builtin keyboards use another match-type
>> so that they can do DMI matching e.g.:
>>
>> evdev:atkbd:dmi:bvn*:bvr*:bd*:svnAcer*:pn*:*
>>
>> So luckily for almost all users the e field in the match
>> rule changing should not be an issue. Sorry that this
>> was a problem for you.
>
> Hans, I wonder, if we skip "GET ID" command because it is a
> portable/laptop, maybe we should assume that it is the standard "0xab83"
> instead of "0xabba" that we assign if GET ID fails but SET LEDS
> succeeds. What do you think?
That sounds like a good idea to me. I was already wondering
if there was a standard response.
Do you plan to write a fix yourself or shall I propose one ?
Regards,
Hans
^ permalink raw reply
* Re: [PATCH 2/2] input/navpoint: remove driver
From: Duje Mihanović @ 2024-01-16 19:45 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Russell King, Uwe Kleine-König, linux-arm-kernel,
linux-kernel, linux-input
In-Reply-To: <ZabS88WnVz9avRv2@google.com>
Hello Dmitry,
On Tuesday, January 16, 2024 8:03:15 PM CET Dmitry Torokhov wrote:
> On Tue, Jan 16, 2024 at 07:17:48PM +0100, Duje Mihanović wrote:
> > This driver does not use the SPI core as it should, instead tampering
> > with the SSP registers manually. Besides this, it relies on the global
> > GPIO namespace. Refactoring the driver is almost certainly not worth it
> > as it was seemingly only implemented in the HP iPAQ hx4700 removed more
> > than a year ago in d6df7df7ae5a ("ARM: pxa: remove unused board files"),
> > so let's remove it.
>
> I think this clashed with some recent cleanups to the navpoint driver.
> Could you please re-generate the patch against linux-next or my "next"
> branch please?
I have done it. Should I wait a bit (how much?) before resending or is it okay
to send v2 right away in this case?
Regards,
--
Duje
^ permalink raw reply
* Re: [PATCH 2/2] input/navpoint: remove driver
From: Dmitry Torokhov @ 2024-01-16 19:47 UTC (permalink / raw)
To: Duje Mihanović
Cc: Russell King, Uwe Kleine-König, linux-arm-kernel,
linux-kernel, linux-input
In-Reply-To: <12362783.O9o76ZdvQC@radijator>
On Tue, Jan 16, 2024 at 08:45:43PM +0100, Duje Mihanović wrote:
> Hello Dmitry,
>
> On Tuesday, January 16, 2024 8:03:15 PM CET Dmitry Torokhov wrote:
> > On Tue, Jan 16, 2024 at 07:17:48PM +0100, Duje Mihanović wrote:
> > > This driver does not use the SPI core as it should, instead tampering
> > > with the SSP registers manually. Besides this, it relies on the global
> > > GPIO namespace. Refactoring the driver is almost certainly not worth it
> > > as it was seemingly only implemented in the HP iPAQ hx4700 removed more
> > > than a year ago in d6df7df7ae5a ("ARM: pxa: remove unused board files"),
> > > so let's remove it.
> >
> > I think this clashed with some recent cleanups to the navpoint driver.
> > Could you please re-generate the patch against linux-next or my "next"
> > branch please?
>
> I have done it. Should I wait a bit (how much?) before resending or is it okay
> to send v2 right away in this case?
Please send it, there is no reason to wait.
Thanks.
--
Dmitry
^ permalink raw reply
* [PATCH v2 1/2] ARM: pxa: drop MOUSE_NAVPOINT_PXA27x from defconfig
From: Duje Mihanović @ 2024-01-16 19:48 UTC (permalink / raw)
To: Russell King, Dmitry Torokhov
Cc: Uwe Kleine-König, linux-arm-kernel, linux-kernel,
linux-input, Duje Mihanović
In-Reply-To: <20240116-navpoint-removal-v2-0-e566806f1009@skole.hr>
Drop the Synaptics NavPoint touchpad from pxa_defconfig in preparation
for removal of the driver.
Signed-off-by: Duje Mihanović <duje.mihanovic@skole.hr>
---
arch/arm/configs/pxa_defconfig | 1 -
1 file changed, 1 deletion(-)
diff --git a/arch/arm/configs/pxa_defconfig b/arch/arm/configs/pxa_defconfig
index b0c3355e2599..7d6d0551a870 100644
--- a/arch/arm/configs/pxa_defconfig
+++ b/arch/arm/configs/pxa_defconfig
@@ -245,7 +245,6 @@ CONFIG_MOUSE_PS2_ELANTECH=y
CONFIG_MOUSE_SERIAL=m
CONFIG_MOUSE_CYAPA=m
CONFIG_MOUSE_ELAN_I2C=m
-CONFIG_MOUSE_NAVPOINT_PXA27x=m
CONFIG_INPUT_TOUCHSCREEN=y
CONFIG_TOUCHSCREEN_ADS7846=m
CONFIG_TOUCHSCREEN_ATMEL_MXT=m
--
2.43.0
^ permalink raw reply related
* [PATCH v2 0/2] input/navpoint: remove driver
From: Duje Mihanović @ 2024-01-16 19:48 UTC (permalink / raw)
To: Russell King, Dmitry Torokhov
Cc: Uwe Kleine-König, linux-arm-kernel, linux-kernel,
linux-input, Duje Mihanović
Hello,
Small series to drop the navpoint driver. The rationale is explained in
the commit message of patch 2/2.
Signed-off-by: Duje Mihanović <duje.mihanovic@skole.hr>
---
Changes in v2:
- Rebase to input tree (also edit commit message to reflect GPIO cleanup
done there)
- Mention hardware's apparent exclusiveness to hx4700 in commit message
- Link to v1: https://lore.kernel.org/r/20240116-navpoint-removal-v1-0-593ae4954a83@skole.hr
---
Duje Mihanović (2):
ARM: pxa: drop MOUSE_NAVPOINT_PXA27x from defconfig
input/navpoint: remove driver
arch/arm/configs/pxa_defconfig | 1 -
drivers/input/mouse/Kconfig | 12 --
drivers/input/mouse/Makefile | 1 -
drivers/input/mouse/navpoint.c | 350 -----------------------------------------
include/linux/input/navpoint.h | 8 -
5 files changed, 372 deletions(-)
---
base-commit: 52c4e5985a730796a3fa555b83b404708b960f9d
change-id: 20240106-navpoint-removal-38225f48733a
Best regards,
--
Duje Mihanović <duje.mihanovic@skole.hr>
^ permalink raw reply
* [PATCH v2 2/2] input/navpoint: remove driver
From: Duje Mihanović @ 2024-01-16 19:48 UTC (permalink / raw)
To: Russell King, Dmitry Torokhov
Cc: Uwe Kleine-König, linux-arm-kernel, linux-kernel,
linux-input, Duje Mihanović
In-Reply-To: <20240116-navpoint-removal-v2-0-e566806f1009@skole.hr>
This driver does not use the SPI core as it should, instead tampering
with the SSP registers manually. Refactoring the driver is almost
certainly not worth it as the hardware seems to have been designed for
and used only in the HP iPAQ hx4700 removed more than a year ago in
d6df7df7ae5a ("ARM: pxa: remove unused board files"), so let's remove
it.
Signed-off-by: Duje Mihanović <duje.mihanovic@skole.hr>
---
drivers/input/mouse/Kconfig | 12 --
drivers/input/mouse/Makefile | 1 -
drivers/input/mouse/navpoint.c | 350 -----------------------------------------
include/linux/input/navpoint.h | 8 -
4 files changed, 371 deletions(-)
diff --git a/drivers/input/mouse/Kconfig b/drivers/input/mouse/Kconfig
index 32cc4c62a716..833b643f0616 100644
--- a/drivers/input/mouse/Kconfig
+++ b/drivers/input/mouse/Kconfig
@@ -439,16 +439,4 @@ config MOUSE_SYNAPTICS_USB
To compile this driver as a module, choose M here: the
module will be called synaptics_usb.
-config MOUSE_NAVPOINT_PXA27x
- tristate "Synaptics NavPoint (PXA27x SSP/SPI)"
- depends on PXA27x && PXA_SSP
- help
- This driver adds support for the Synaptics NavPoint touchpad connected
- to a PXA27x SSP port in SPI slave mode. The device emulates a mouse;
- a tap or tap-and-a-half drag gesture emulates the left mouse button.
- For example, use the xf86-input-evdev driver for an X pointing device.
-
- To compile this driver as a module, choose M here: the
- module will be called navpoint.
-
endif
diff --git a/drivers/input/mouse/Makefile b/drivers/input/mouse/Makefile
index 92b3204ce84e..a1336d5bee6f 100644
--- a/drivers/input/mouse/Makefile
+++ b/drivers/input/mouse/Makefile
@@ -15,7 +15,6 @@ obj-$(CONFIG_MOUSE_GPIO) += gpio_mouse.o
obj-$(CONFIG_MOUSE_INPORT) += inport.o
obj-$(CONFIG_MOUSE_LOGIBM) += logibm.o
obj-$(CONFIG_MOUSE_MAPLE) += maplemouse.o
-obj-$(CONFIG_MOUSE_NAVPOINT_PXA27x) += navpoint.o
obj-$(CONFIG_MOUSE_PC110PAD) += pc110pad.o
obj-$(CONFIG_MOUSE_PS2) += psmouse.o
obj-$(CONFIG_MOUSE_RISCPC) += rpcmouse.o
diff --git a/drivers/input/mouse/navpoint.c b/drivers/input/mouse/navpoint.c
deleted file mode 100644
index ba757783c258..000000000000
--- a/drivers/input/mouse/navpoint.c
+++ /dev/null
@@ -1,350 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0-only
-/*
- * Synaptics NavPoint (PXA27x SSP/SPI) driver.
- *
- * Copyright (C) 2012 Paul Parsons <lost.distance@yahoo.com>
- */
-
-#include <linux/kernel.h>
-#include <linux/module.h>
-#include <linux/platform_device.h>
-#include <linux/clk.h>
-#include <linux/delay.h>
-#include <linux/gpio/consumer.h>
-#include <linux/input.h>
-#include <linux/input/navpoint.h>
-#include <linux/interrupt.h>
-#include <linux/mutex.h>
-#include <linux/pxa2xx_ssp.h>
-#include <linux/slab.h>
-
-/*
- * Synaptics Modular Embedded Protocol: Module Packet Format.
- * Module header byte 2:0 = Length (# bytes that follow)
- * Module header byte 4:3 = Control
- * Module header byte 7:5 = Module Address
- */
-#define HEADER_LENGTH(byte) ((byte) & 0x07)
-#define HEADER_CONTROL(byte) (((byte) >> 3) & 0x03)
-#define HEADER_ADDRESS(byte) ((byte) >> 5)
-
-struct navpoint {
- struct ssp_device *ssp;
- struct input_dev *input;
- struct device *dev;
- struct gpio_desc *gpiod;
- int index;
- u8 data[1 + HEADER_LENGTH(0xff)];
-};
-
-/*
- * Initialization values for SSCR0_x, SSCR1_x, SSSR_x.
- */
-static const u32 sscr0 = 0
- | SSCR0_TUM /* TIM = 1; No TUR interrupts */
- | SSCR0_RIM /* RIM = 1; No ROR interrupts */
- | SSCR0_SSE /* SSE = 1; SSP enabled */
- | SSCR0_Motorola /* FRF = 0; Motorola SPI */
- | SSCR0_DataSize(16) /* DSS = 15; Data size = 16-bit */
- ;
-static const u32 sscr1 = 0
- | SSCR1_SCFR /* SCFR = 1; SSPSCLK only during transfers */
- | SSCR1_SCLKDIR /* SCLKDIR = 1; Slave mode */
- | SSCR1_SFRMDIR /* SFRMDIR = 1; Slave mode */
- | SSCR1_RWOT /* RWOT = 1; Receive without transmit mode */
- | SSCR1_RxTresh(1) /* RFT = 0; Receive FIFO threshold = 1 */
- | SSCR1_SPH /* SPH = 1; SSPSCLK inactive 0.5 + 1 cycles */
- | SSCR1_RIE /* RIE = 1; Receive FIFO interrupt enabled */
- ;
-static const u32 sssr = 0
- | SSSR_BCE /* BCE = 1; Clear BCE */
- | SSSR_TUR /* TUR = 1; Clear TUR */
- | SSSR_EOC /* EOC = 1; Clear EOC */
- | SSSR_TINT /* TINT = 1; Clear TINT */
- | SSSR_PINT /* PINT = 1; Clear PINT */
- | SSSR_ROR /* ROR = 1; Clear ROR */
- ;
-
-/*
- * MEP Query $22: Touchpad Coordinate Range Query is not supported by
- * the NavPoint module, so sampled values provide the default limits.
- */
-#define NAVPOINT_X_MIN 1278
-#define NAVPOINT_X_MAX 5340
-#define NAVPOINT_Y_MIN 1572
-#define NAVPOINT_Y_MAX 4396
-#define NAVPOINT_PRESSURE_MIN 0
-#define NAVPOINT_PRESSURE_MAX 255
-
-static void navpoint_packet(struct navpoint *navpoint)
-{
- int finger;
- int gesture;
- int x, y, z;
-
- switch (navpoint->data[0]) {
- case 0xff: /* Garbage (packet?) between reset and Hello packet */
- case 0x00: /* Module 0, NULL packet */
- break;
-
- case 0x0e: /* Module 0, Absolute packet */
- finger = (navpoint->data[1] & 0x01);
- gesture = (navpoint->data[1] & 0x02);
- x = ((navpoint->data[2] & 0x1f) << 8) | navpoint->data[3];
- y = ((navpoint->data[4] & 0x1f) << 8) | navpoint->data[5];
- z = navpoint->data[6];
- input_report_key(navpoint->input, BTN_TOUCH, finger);
- input_report_abs(navpoint->input, ABS_X, x);
- input_report_abs(navpoint->input, ABS_Y, y);
- input_report_abs(navpoint->input, ABS_PRESSURE, z);
- input_report_key(navpoint->input, BTN_TOOL_FINGER, finger);
- input_report_key(navpoint->input, BTN_LEFT, gesture);
- input_sync(navpoint->input);
- break;
-
- case 0x19: /* Module 0, Hello packet */
- if ((navpoint->data[1] & 0xf0) == 0x10)
- break;
- fallthrough;
- default:
- dev_warn(navpoint->dev,
- "spurious packet: data=0x%02x,0x%02x,...\n",
- navpoint->data[0], navpoint->data[1]);
- break;
- }
-}
-
-static irqreturn_t navpoint_irq(int irq, void *dev_id)
-{
- struct navpoint *navpoint = dev_id;
- struct ssp_device *ssp = navpoint->ssp;
- irqreturn_t ret = IRQ_NONE;
- u32 status;
-
- status = pxa_ssp_read_reg(ssp, SSSR);
- if (status & sssr) {
- dev_warn(navpoint->dev,
- "unexpected interrupt: status=0x%08x\n", status);
- pxa_ssp_write_reg(ssp, SSSR, (status & sssr));
- ret = IRQ_HANDLED;
- }
-
- while (status & SSSR_RNE) {
- u32 data;
-
- data = pxa_ssp_read_reg(ssp, SSDR);
- navpoint->data[navpoint->index + 0] = (data >> 8);
- navpoint->data[navpoint->index + 1] = data;
- navpoint->index += 2;
- if (HEADER_LENGTH(navpoint->data[0]) < navpoint->index) {
- navpoint_packet(navpoint);
- navpoint->index = 0;
- }
- status = pxa_ssp_read_reg(ssp, SSSR);
- ret = IRQ_HANDLED;
- }
-
- return ret;
-}
-
-static void navpoint_up(struct navpoint *navpoint)
-{
- struct ssp_device *ssp = navpoint->ssp;
- int timeout;
-
- clk_prepare_enable(ssp->clk);
-
- pxa_ssp_write_reg(ssp, SSCR1, sscr1);
- pxa_ssp_write_reg(ssp, SSSR, sssr);
- pxa_ssp_write_reg(ssp, SSTO, 0);
- pxa_ssp_write_reg(ssp, SSCR0, sscr0); /* SSCR0_SSE written last */
-
- /* Wait until SSP port is ready for slave clock operations */
- for (timeout = 100; timeout != 0; --timeout) {
- if (!(pxa_ssp_read_reg(ssp, SSSR) & SSSR_CSS))
- break;
- msleep(1);
- }
-
- if (timeout == 0)
- dev_err(navpoint->dev,
- "timeout waiting for SSSR[CSS] to clear\n");
-
- gpiod_set_value(navpoint->gpiod, 1);
-}
-
-static void navpoint_down(struct navpoint *navpoint)
-{
- struct ssp_device *ssp = navpoint->ssp;
-
- gpiod_set_value(navpoint->gpiod, 0);
-
- pxa_ssp_write_reg(ssp, SSCR0, 0);
-
- clk_disable_unprepare(ssp->clk);
-}
-
-static int navpoint_open(struct input_dev *input)
-{
- struct navpoint *navpoint = input_get_drvdata(input);
-
- navpoint_up(navpoint);
-
- return 0;
-}
-
-static void navpoint_close(struct input_dev *input)
-{
- struct navpoint *navpoint = input_get_drvdata(input);
-
- navpoint_down(navpoint);
-}
-
-static int navpoint_probe(struct platform_device *pdev)
-{
- const struct navpoint_platform_data *pdata =
- dev_get_platdata(&pdev->dev);
- struct ssp_device *ssp;
- struct input_dev *input;
- struct navpoint *navpoint;
- int error;
-
- if (!pdata) {
- dev_err(&pdev->dev, "no platform data\n");
- return -EINVAL;
- }
-
- ssp = pxa_ssp_request(pdata->port, pdev->name);
- if (!ssp)
- return -ENODEV;
-
- /* HaRET does not disable devices before jumping into Linux */
- if (pxa_ssp_read_reg(ssp, SSCR0) & SSCR0_SSE) {
- pxa_ssp_write_reg(ssp, SSCR0, 0);
- dev_warn(&pdev->dev, "ssp%d already enabled\n", pdata->port);
- }
-
- navpoint = kzalloc(sizeof(*navpoint), GFP_KERNEL);
- input = input_allocate_device();
- if (!navpoint || !input) {
- error = -ENOMEM;
- goto err_free_mem;
- }
-
- navpoint->gpiod = gpiod_get_optional(&pdev->dev,
- NULL, GPIOD_OUT_LOW);
- if (IS_ERR(navpoint->gpiod)) {
- error = PTR_ERR(navpoint->gpiod);
- dev_err(&pdev->dev, "error getting GPIO\n");
- goto err_free_mem;
- }
- gpiod_set_consumer_name(navpoint->gpiod, "SYNAPTICS_ON");
-
- navpoint->ssp = ssp;
- navpoint->input = input;
- navpoint->dev = &pdev->dev;
-
- input->name = pdev->name;
- input->dev.parent = &pdev->dev;
-
- __set_bit(EV_KEY, input->evbit);
- __set_bit(EV_ABS, input->evbit);
- __set_bit(BTN_LEFT, input->keybit);
- __set_bit(BTN_TOUCH, input->keybit);
- __set_bit(BTN_TOOL_FINGER, input->keybit);
-
- input_set_abs_params(input, ABS_X,
- NAVPOINT_X_MIN, NAVPOINT_X_MAX, 0, 0);
- input_set_abs_params(input, ABS_Y,
- NAVPOINT_Y_MIN, NAVPOINT_Y_MAX, 0, 0);
- input_set_abs_params(input, ABS_PRESSURE,
- NAVPOINT_PRESSURE_MIN, NAVPOINT_PRESSURE_MAX,
- 0, 0);
-
- input->open = navpoint_open;
- input->close = navpoint_close;
-
- input_set_drvdata(input, navpoint);
-
- error = request_irq(ssp->irq, navpoint_irq, 0, pdev->name, navpoint);
- if (error)
- goto err_free_mem;
-
- error = input_register_device(input);
- if (error)
- goto err_free_irq;
-
- platform_set_drvdata(pdev, navpoint);
- dev_dbg(&pdev->dev, "ssp%d, irq %d\n", pdata->port, ssp->irq);
-
- return 0;
-
-err_free_irq:
- free_irq(ssp->irq, navpoint);
-err_free_mem:
- input_free_device(input);
- kfree(navpoint);
- pxa_ssp_free(ssp);
-
- return error;
-}
-
-static void navpoint_remove(struct platform_device *pdev)
-{
- struct navpoint *navpoint = platform_get_drvdata(pdev);
- struct ssp_device *ssp = navpoint->ssp;
-
- free_irq(ssp->irq, navpoint);
-
- input_unregister_device(navpoint->input);
- kfree(navpoint);
-
- pxa_ssp_free(ssp);
-}
-
-static int navpoint_suspend(struct device *dev)
-{
- struct platform_device *pdev = to_platform_device(dev);
- struct navpoint *navpoint = platform_get_drvdata(pdev);
- struct input_dev *input = navpoint->input;
-
- mutex_lock(&input->mutex);
- if (input_device_enabled(input))
- navpoint_down(navpoint);
- mutex_unlock(&input->mutex);
-
- return 0;
-}
-
-static int navpoint_resume(struct device *dev)
-{
- struct platform_device *pdev = to_platform_device(dev);
- struct navpoint *navpoint = platform_get_drvdata(pdev);
- struct input_dev *input = navpoint->input;
-
- mutex_lock(&input->mutex);
- if (input_device_enabled(input))
- navpoint_up(navpoint);
- mutex_unlock(&input->mutex);
-
- return 0;
-}
-
-static DEFINE_SIMPLE_DEV_PM_OPS(navpoint_pm_ops,
- navpoint_suspend, navpoint_resume);
-
-static struct platform_driver navpoint_driver = {
- .probe = navpoint_probe,
- .remove_new = navpoint_remove,
- .driver = {
- .name = "navpoint",
- .pm = pm_sleep_ptr(&navpoint_pm_ops),
- },
-};
-
-module_platform_driver(navpoint_driver);
-
-MODULE_AUTHOR("Paul Parsons <lost.distance@yahoo.com>");
-MODULE_DESCRIPTION("Synaptics NavPoint (PXA27x SSP/SPI) driver");
-MODULE_LICENSE("GPL");
-MODULE_ALIAS("platform:navpoint");
diff --git a/include/linux/input/navpoint.h b/include/linux/input/navpoint.h
deleted file mode 100644
index 5192ae3f5ec1..000000000000
--- a/include/linux/input/navpoint.h
+++ /dev/null
@@ -1,8 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0-only */
-/*
- * Copyright (C) 2012 Paul Parsons <lost.distance@yahoo.com>
- */
-
-struct navpoint_platform_data {
- int port; /* PXA SSP port for pxa_ssp_request() */
-};
--
2.43.0
^ permalink raw reply related
* Re: [PATCH v2] Input: atkbd - Skip ATKBD_CMD_GETID in translated mode
From: Dmitry Torokhov @ 2024-01-16 19:50 UTC (permalink / raw)
To: Hans de Goede
Cc: Barnabás Pőcze, linux-input, Shang Ye, gurevitch,
Egor Ignatov, Anton Zhilyaev
In-Reply-To: <5207ea8a-722b-4431-8d50-8cec8356f1ad@redhat.com>
On Tue, Jan 16, 2024 at 08:33:39PM +0100, Hans de Goede wrote:
> Hi,
>
> On 1/16/24 20:05, Dmitry Torokhov wrote:
> > On Tue, Jan 16, 2024 at 03:43:10PM +0100, Hans de Goede wrote:
> >> Hi,
> >>
> >> On 1/16/24 14:32, Barnabás Pőcze wrote:
> >>>
> >>> After:
> >>>
> >>> evdev:input:b0011v0001p0001*
> >>> KEYBOARD_KEY_f8=fn
> >>> KEYBOARD_KEY_76=f21
> >>>
> >>> I: Bus=0011 Vendor=0001 Product=0001 Version=abba
> >>> N: Name="AT Translated Set 2 keyboard"
> >>> P: Phys=isa0060/serio0/input0
> >>> S: Sysfs=/devices/platform/i8042/serio0/input/input4
> >>
> >> I see, thank you. There are no v0001p0001 matches
> >> in the hwdb.d/60-keyboard.hwdb shipped with systems.
> >>
> >> Typically laptop builtin keyboards use another match-type
> >> so that they can do DMI matching e.g.:
> >>
> >> evdev:atkbd:dmi:bvn*:bvr*:bd*:svnAcer*:pn*:*
> >>
> >> So luckily for almost all users the e field in the match
> >> rule changing should not be an issue. Sorry that this
> >> was a problem for you.
> >
> > Hans, I wonder, if we skip "GET ID" command because it is a
> > portable/laptop, maybe we should assume that it is the standard "0xab83"
> > instead of "0xabba" that we assign if GET ID fails but SET LEDS
> > succeeds. What do you think?
>
> That sounds like a good idea to me. I was already wondering
> if there was a standard response.
>
> Do you plan to write a fix yourself or shall I propose one ?
Please propose a patch.
Thanks.
--
Dmitry
^ permalink raw reply
* [PATCH] Input: atkbd - Use ab83 as id when skipping the getid command
From: Hans de Goede @ 2024-01-16 20:43 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: Hans de Goede, linux-input, Barnabás Pőcze
Barnabás reported that the change to skip the getid command
when the controller is in translated mode on laptops caused
the Version field of his "AT Translated Set 2 keyboard"
input device to change from ab83 to abba, breaking a custom
hwdb entry for this keyboard.
Use the standard ab83 id for keyboards when getid is skipped
(rather then that getid fails) to avoid reporting a different
Version to userspace then before skipping the getid.
Fixes: 936e4d49ecbc ("Input: atkbd - skip ATKBD_CMD_GETID in translated mode")
Reported-by: Barnabás Pőcze <pobrn@protonmail.com>
Closes: https://lore.kernel.org/linux-input/W1ydwoG2fYv85Z3C3yfDOJcVpilEvGge6UGa9kZh8zI2-qkHXp7WLnl2hSkFz63j-c7WupUWI5TLL6n7Lt8DjRuU-yJBwLYWrreb1hbnd6A=@protonmail.com/
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
drivers/input/keyboard/atkbd.c | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/drivers/input/keyboard/atkbd.c b/drivers/input/keyboard/atkbd.c
index 786f00f6b7fd..13ef6284223d 100644
--- a/drivers/input/keyboard/atkbd.c
+++ b/drivers/input/keyboard/atkbd.c
@@ -791,9 +791,9 @@ static bool atkbd_is_portable_device(void)
* not work. So in this case simply assume a keyboard is connected to avoid
* confusing some laptop keyboards.
*
- * Skipping ATKBD_CMD_GETID ends up using a fake keyboard id. Using a fake id is
- * ok in translated mode, only atkbd_select_set() checks atkbd->id and in
- * translated mode that is a no-op.
+ * Skipping ATKBD_CMD_GETID ends up using a fake keyboard id. Using the standard
+ * 0xab83 id is ok in translated mode, only atkbd_select_set() checks atkbd->id
+ * and in translated mode that is a no-op.
*/
static bool atkbd_skip_getid(struct atkbd *atkbd)
{
@@ -811,6 +811,7 @@ static int atkbd_probe(struct atkbd *atkbd)
{
struct ps2dev *ps2dev = &atkbd->ps2dev;
unsigned char param[2];
+ bool skip_getid;
/*
* Some systems, where the bit-twiddling when testing the io-lines of the
@@ -832,7 +833,8 @@ static int atkbd_probe(struct atkbd *atkbd)
*/
param[0] = param[1] = 0xa5; /* initialize with invalid values */
- if (atkbd_skip_getid(atkbd) || ps2_command(ps2dev, param, ATKBD_CMD_GETID)) {
+ skip_getid = atkbd_skip_getid(atkbd);
+ if (skip_getid || ps2_command(ps2dev, param, ATKBD_CMD_GETID)) {
/*
* If the get ID command was skipped or failed, we check if we can at least set
@@ -842,7 +844,7 @@ static int atkbd_probe(struct atkbd *atkbd)
param[0] = 0;
if (ps2_command(ps2dev, param, ATKBD_CMD_SETLEDS))
return -1;
- atkbd->id = 0xabba;
+ atkbd->id = skip_getid ? 0xab83 : 0xabba;
return 0;
}
--
2.43.0
^ permalink raw reply related
* Re: [PATCH] Input: atkbd - Use ab83 as id when skipping the getid command
From: Dmitry Torokhov @ 2024-01-16 21:32 UTC (permalink / raw)
To: Hans de Goede; +Cc: linux-input, Barnabás Pőcze
In-Reply-To: <20240116204325.7719-1-hdegoede@redhat.com>
On Tue, Jan 16, 2024 at 09:43:25PM +0100, Hans de Goede wrote:
> Barnabás reported that the change to skip the getid command
> when the controller is in translated mode on laptops caused
> the Version field of his "AT Translated Set 2 keyboard"
> input device to change from ab83 to abba, breaking a custom
> hwdb entry for this keyboard.
>
> Use the standard ab83 id for keyboards when getid is skipped
> (rather then that getid fails) to avoid reporting a different
> Version to userspace then before skipping the getid.
>
> Fixes: 936e4d49ecbc ("Input: atkbd - skip ATKBD_CMD_GETID in translated mode")
> Reported-by: Barnabás Pőcze <pobrn@protonmail.com>
> Closes: https://lore.kernel.org/linux-input/W1ydwoG2fYv85Z3C3yfDOJcVpilEvGge6UGa9kZh8zI2-qkHXp7WLnl2hSkFz63j-c7WupUWI5TLL6n7Lt8DjRuU-yJBwLYWrreb1hbnd6A=@protonmail.com/
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Applied, thank you.
--
Dmitry
^ permalink raw reply
* Re: [PATCH v2 2/2] input/navpoint: remove driver
From: Dmitry Torokhov @ 2024-01-16 21:54 UTC (permalink / raw)
To: Duje Mihanović
Cc: Russell King, Uwe Kleine-König, linux-arm-kernel,
linux-kernel, linux-input
In-Reply-To: <20240116-navpoint-removal-v2-2-e566806f1009@skole.hr>
On Tue, Jan 16, 2024 at 08:48:07PM +0100, Duje Mihanović wrote:
> This driver does not use the SPI core as it should, instead tampering
> with the SSP registers manually. Refactoring the driver is almost
> certainly not worth it as the hardware seems to have been designed for
> and used only in the HP iPAQ hx4700 removed more than a year ago in
> d6df7df7ae5a ("ARM: pxa: remove unused board files"), so let's remove
> it.
>
> Signed-off-by: Duje Mihanović <duje.mihanovic@skole.hr>
Applied, thank you.
--
Dmitry
^ permalink raw reply
* Re: [PATCH] Input: synaptics-rmi4: Fix NULL pointer dereference in rmi_driver_probe
From: Kunwu Chan @ 2024-01-17 7:07 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: aduggan, cheiny, linux-input, linux-kernel
In-Reply-To: <ZabU_lsGCuki1dSY@google.com>
On 2024/1/17 03:11, Dmitry Torokhov wrote:
> On Tue, Jan 16, 2024 at 04:38:47PM +0800, Kunwu Chan wrote:
>> devm_kasprintf() returns a pointer to dynamically allocated memory
>> which can be NULL upon failure. Ensure the allocation was successful
>> by checking the pointer validity.
>
> It is perfectly valid to not set "input->phys" and leave it at NULL. So
> while I agree that having error handling is good I do not believe
> there's chance for NULL pointer dereference, so please adjust your patch
> title.
Thanks for your suggestions.
I'll change patch title to "Input: synaptics-rmi4: Add a null pointer
check to the rmi_driver_probe".
>>
>> Fixes: 2b6a321da9a2 ("Input: synaptics-rmi4 - add support for Synaptics RMI4 devices")
>> Signed-off-by: Kunwu Chan <chentao@kylinos.cn>
>> ---
>> drivers/input/rmi4/rmi_driver.c | 6 ++++++
>> 1 file changed, 6 insertions(+)
>>
>> diff --git a/drivers/input/rmi4/rmi_driver.c b/drivers/input/rmi4/rmi_driver.c
>> index 258d5fe3d395..d3a601ff51e6 100644
>> --- a/drivers/input/rmi4/rmi_driver.c
>> +++ b/drivers/input/rmi4/rmi_driver.c
>> @@ -1197,6 +1197,12 @@ static int rmi_driver_probe(struct device *dev)
>> rmi_driver_set_input_params(rmi_dev, data->input);
>> data->input->phys = devm_kasprintf(dev, GFP_KERNEL,
>> "%s/input0", dev_name(dev));
>> + if (!data->input->phys) {
>> + dev_err(dev, "%s: Failed to allocate memory.\n",
>
> No need to log the error here, memory allocation will already log the
> failure.
Thanks, I'll remove the dev_err.
>
> Thanks.
>
--
Thanks,
Kunwu
^ permalink raw reply
* [PATCH v2] Input: synaptics-rmi4: Add a null pointer check to the rmi_driver_probe
From: Kunwu Chan @ 2024-01-17 7:31 UTC (permalink / raw)
To: dmitry.torokhov, aduggan, cheiny; +Cc: linux-input, linux-kernel, Kunwu Chan
devm_kasprintf() returns a pointer to dynamically allocated memory
which can be NULL upon failure. Ensure the allocation was successful
by checking the pointer validity.
Signed-off-by: Kunwu Chan <chentao@kylinos.cn>
Suggested-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
v2: Change title and remove dev_err
---
drivers/input/rmi4/rmi_driver.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/input/rmi4/rmi_driver.c b/drivers/input/rmi4/rmi_driver.c
index 258d5fe3d395..8da104e99e7b 100644
--- a/drivers/input/rmi4/rmi_driver.c
+++ b/drivers/input/rmi4/rmi_driver.c
@@ -1197,6 +1197,10 @@ static int rmi_driver_probe(struct device *dev)
rmi_driver_set_input_params(rmi_dev, data->input);
data->input->phys = devm_kasprintf(dev, GFP_KERNEL,
"%s/input0", dev_name(dev));
+ if (!data->input->phys) {
+ retval = -ENOMEM;
+ goto err;
+ }
}
retval = rmi_init_functions(data);
--
2.39.2
^ permalink raw reply related
* [syzbot] Monthly input report (Jan 2024)
From: syzbot @ 2024-01-17 9:21 UTC (permalink / raw)
To: linux-input, linux-kernel, syzkaller-bugs
Hello input maintainers/developers,
This is a 31-day syzbot report for the input subsystem.
All related reports/information can be found at:
https://syzkaller.appspot.com/upstream/s/input
During the period, 0 new issues were detected and 0 were fixed.
In total, 12 issues are still open and 51 have been fixed so far.
Some of the still happening issues:
Ref Crashes Repro Title
<1> 4133 Yes WARNING in input_mt_init_slots
https://syzkaller.appspot.com/bug?extid=0122fa359a69694395d5
<2> 947 Yes WARNING in implement
https://syzkaller.appspot.com/bug?extid=38e7237add3712479d65
<3> 353 No possible deadlock in evdev_pass_values (2)
https://syzkaller.appspot.com/bug?extid=13d3cb2a3dc61e6092f5
<4> 149 Yes WARNING in cm109_urb_irq_callback/usb_submit_urb
https://syzkaller.appspot.com/bug?extid=2d6d691af5ab4b7e66df
<5> 90 Yes INFO: task hung in uhid_char_release
https://syzkaller.appspot.com/bug?extid=8fe2d362af0e1cba8735
<6> 39 Yes WARNING in bcm5974_start_traffic/usb_submit_urb
https://syzkaller.appspot.com/bug?extid=348331f63b034f89b622
---
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.
To disable reminders for individual bugs, reply with the following command:
#syz set <Ref> no-reminders
To change bug's subsystems, reply with:
#syz set <Ref> subsystems: new-subsystem
You may send multiple commands in a single email message.
^ permalink raw reply
* [PATCH] selftests/hid: wacom: fix confidence tests
From: Benjamin Tissoires @ 2024-01-17 13:27 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, Shuah Khan, Jason Gerecke
Cc: linux-input, linux-kselftest, linux-kernel, Benjamin Tissoires
The device is exported with a fuzz of 4, meaning that the `+ t` here
is removed by the fuzz algorithm, making those tests failing.
Not sure why, but when I run this locally it was passing, but not in the
VM.
Link: https://gitlab.freedesktop.org/bentiss/hid/-/jobs/53692957#L3315
Signed-off-by: Benjamin Tissoires <bentiss@kernel.org>
---
Over the break the test suite wasn't properly running on my runner,
and this small issue sneaked in.
---
tools/testing/selftests/hid/tests/test_wacom_generic.py | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/tools/testing/selftests/hid/tests/test_wacom_generic.py b/tools/testing/selftests/hid/tests/test_wacom_generic.py
index 352fc39f3c6c..b62c7dba6777 100644
--- a/tools/testing/selftests/hid/tests/test_wacom_generic.py
+++ b/tools/testing/selftests/hid/tests/test_wacom_generic.py
@@ -880,8 +880,8 @@ class TestDTH2452Tablet(test_multitouch.BaseTest.TestMultitouch, TouchTabletTest
does not overlap with other contacts. The value of `t` may be
incremented over time to move the point along a linear path.
"""
- x = 50 + 10 * contact_id + t
- y = 100 + 100 * contact_id + t
+ x = 50 + 10 * contact_id + t * 11
+ y = 100 + 100 * contact_id + t * 11
return test_multitouch.Touch(contact_id, x, y)
def make_contacts(self, n, t=0):
@@ -902,8 +902,8 @@ class TestDTH2452Tablet(test_multitouch.BaseTest.TestMultitouch, TouchTabletTest
tracking_id = contact_ids.tracking_id
slot_num = contact_ids.slot_num
- x = 50 + 10 * contact_id + t
- y = 100 + 100 * contact_id + t
+ x = 50 + 10 * contact_id + t * 11
+ y = 100 + 100 * contact_id + t * 11
# If the data isn't supposed to be stored in any slots, there is
# nothing we can check for in the evdev stream.
---
base-commit: 80d5a73edcfbd1d8d6a4c2b755873c5d63a1ebd7
change-id: 20240117-b4-wip-wacom-tests-fixes-298b50bea47f
Best regards,
--
Benjamin Tissoires <bentiss@kernel.org>
^ permalink raw reply related
* Re: Implement per-key keyboard backlight as auxdisplay?
From: Hans de Goede @ 2024-01-17 15:26 UTC (permalink / raw)
To: Werner Sembach, Pavel Machek, Jani Nikula, jikos,
Jelle van der Waa
Cc: Miguel Ojeda, Lee Jones, linux-kernel,
dri-devel@lists.freedesktop.org, linux-input, ojeda, linux-leds
In-Reply-To: <ac02143c-d417-49e5-9c6e-150cbda71ba7@tuxedocomputers.com>
Hi Werner,
Once again, sorry for the very slow response here.
On 11/27/23 11:59, Werner Sembach wrote:
> Hi Hans,
>
> Am 22.11.23 um 19:34 schrieb Hans de Goede:
>> Hi Werner,
> [snip]
>>>>> Another idea I want to throw in the mix:
>>>>>
>>>>> Maybe the kernel is not the right place to implement this at all. RGB stuff is not at all standardized and every vendor is doing completely different interfaces, which does not fit the kernel userpsace apis desire to be uniformal and fixed. e.g. Auxdisplay might fit static setting of RGB values, but it does not fit the snake-effect mode, or the raindrops mode, or the 4-different-colors-in-the-edges-breathing-and-color-cycling mode.
>>>>>
>>>>> So my current idea: Implement these keyboards as a single zone RGB kbd_backlight in the leds interface to have something functional out of the box, but make it runtime disable-able if something like https://gitlab.com/CalcProgrammer1/OpenRGB wants to take over more fine granular control from userspace via hidraw.
>>>> That sounds like a good approach to me. We are seeing the same with game controllers where steam and wine/proton also sometimes use hidraw mode to get access to all the crazy^W interesting features.
>>>>
>>>> That would mean that all we need to standardize and the kernel <-> userspace API level is adding a standard way to disable the single zone RGB kbd_backlight support in the kernel.
>>> I would suggest a simple "enable" entry. Default is 1. When set to 0 the kernel driver no longer does anything.
>> I'm not in favor of using "enable" as sysfs attribute for this,
>> I would like to see a more descriptive name, how about:
>>
>> "disable_kernel_kbd_backlight_support"
>>
>> And then maybe also have the driver actually unregister
>> the LED class device ?
>>
>> Or just make the support inactive when writing 1 to
>> this and allow re-enabling it by writing 0?
>
> Unregistering would mean that it can't be reenabled without module reload/reboot?
Yes.
> I would prefer that the userspace driver could easily give back control to the leds interface.
Hmm, the problem here is that leaving a non-working LED class device
behind may confuse things like upower.
So maybe the disable_kbd_backlight_support sysfs attr should
not sit under /sys/class/leds/foobar::kbd_backlight, but rather
it should sit under the sysfs dir of the parent-device ?
So if we are talking [USB|I2C]-HID keyboards and userspace
using hidraw to takeover kbd_backlight control through,
then have "disable_kbd_backlight_support" sit under
/sys/bus/hid/devices/0003:xxxx:xxxx.xxxx/disable_kbd_backlight_support
and then re-register the LED class device for the keyboard
when 0 gets written to disable_kbd_backlight_support ?
That seems better to me then leaving a non-working LED
class device behind and this will not require any changes
to the LED subsystem.
>>> Questions:
>>>
>>> - Should the driver try to reset the settings to boot default? Or just leave the device in the current state? With the former I could see issues that they keyboard is flashing when changing from kernelspace control to userspace control. With the later the burden on bringing the device to a know state lies with the userspace driver.
>> My vote would go to leave the state as is. Even if the hw
>> does not support state readback, then the userspace code
>> can readback the state before writing 1 to
>> "disable_kernel_kbd_backlight_support"
> ack
>>
>>> - Should this be a optional entry that only shows up on drivers supporting it, or could this implemented in a generic way affecting all current led entries?
>> IMHO this should be optional. If we go with the variant
>> where writing 1 to "disable_kernel_kbd_backlight_support"
>> just disables support and 0 re-enables it then I guess
>> we could have support for this in the LED-core, enabled
>> by a flag set by the driver.
>>
>> If we go with unregistering the led class device,
>> then this needs to be mostly handled in the driver.
>>
>> Either way the kernel driver should know about this even
>> if it is mostly handled in the LED core so that e.g.
>> it does not try to restore settings on resume from suspend.
>
> So a generic implementation would still require all current led drivers to be touched?
>
> For the sake of simplicity I would then prefer the optional variant.
See above, I think we need to put the sysfs-attr to disable
the kernel's builtin kbd-backlight support in the parents
sysfs-dir and then actually unregister the LED class device,
this way we don't need any LED subsytem changes at all.
>>> - I guess UPower integration for the userspace driver could be archived with https://www.kernel.org/doc/html/latest/leds/uleds.html however this limited to brightness atm, so when accent colors actually come to UPower this would also need some expansion to be able to pass a preferred color to the userspace driver (regardless of what that driver is then doing with that information).
>> Using uleds is an interesting suggestion, but upower atm
>> does not support LED class kbd_backlight devices getting
>> hot-plugged. It only scans for them once at boot.
>>
>> Jelle van der Waa (a colleague of mine, added to the Cc)
>> has indicated he is interested in maybe working on fixing
>> this upower short-coming as a side project, once his
>> current side-projects are finished.
> Nice to hear.
>>
>>> On a different note: This approach does currently not cover the older EC controlled 3 zone keyboards from clevo. Here only the kernel has access access to the device so the kernel driver has to expose all functionality somehow. Should this be done by an arbitrarily designed platform device?
>> Interesting question, this reminds there was a discussion
>> about how to handle zoned keyboards using plain LED class
>> APIs here:
>>
>> https://lore.kernel.org/linux-leds/544484b9-c0ac-2fd0-1f41-8fa94cb94d4b@redhat.com/
>>
>> Basically the idea discussed there is to create
>> separate multi-color LED sysfs devices for each zone,
>> using :rgb:kbd_zoned_backlight-xxx as postfix, e.g. :
>>
>> :rgb:kbd_zoned_backlight-left
>> :rgb:kbd_zoned_backlight-middle
>> :rgb:kbd_zoned_backlight-right
>> :rgb:kbd_zoned_backlight-wasd
>>
>> As postfixes for the 4 per zone LED class devices
>> and then teach upower to just treat this as
>> a single kbd-backlight for the existing upower
>> DBUS API and maybe later extend the DBUS API.
>>
>> Would something like this work for the Clevo
>> case you are describing?
>
> Not entirely as some concept for the special modes would still be required.
Right, that can be done with some custom sysfs attr added
to the LED class device, like how dell-laptop.c sets
the .groups member of the "dell::kbd_backlight"
"struct led_classdev kbd_led" to add some extra
sysfs_attr to configure the timeout after which
the kbd_backlight automatically turns off when
no keys are pressed.
> Also it would be nice to be able to set the whole keyboard with a singular file access so that the keyboard changes at once and not zone by zone.
That is an interesting point. This could be implemented
by adding an "enable_atomic_commit" sysfs attr to
all 4:
:rgb:kbd_zoned_backlight-left
:rgb:kbd_zoned_backlight-middle
:rgb:kbd_zoned_backlight-right
:rgb:kbd_zoned_backlight-wasd
LED class devices, which is backed by only
1 variable in the kernel (so changing it
in one place changes it everywhere) and
then also have a "commit" sysfs attr and
writing say "1" to that will then commit
all changes at once.
So normally changes are still applied directly
(for compatibility with the usual sysfs API),
but then when "enable_atomic_commit" is set to 1,
writes only update in kernel variables and then
once "commit" is written all changes are send
out in 1 go.
I think we had the same issue where there was
a single WMI call to change all zones at once
(and having some sort of atomic API was desirable)
the last time the suggestion to use 4 LED class
devices for zoned kbds:
:rgb:kbd_zoned_backlight-left
:rgb:kbd_zoned_backlight-middle
:rgb:kbd_zoned_backlight-right
:rgb:kbd_zoned_backlight-wasd
came up, so we could start a new:
Documentation/ABI/testing/sysfs-class-led-zoned-kbd-backlight
document extending the standard:
Documentation/ABI/testing/sysfs-class-led
which documents both using the:
:rgb:kbd_zoned_backlight-left
:rgb:kbd_zoned_backlight-middle
:rgb:kbd_zoned_backlight-right
:rgb:kbd_zoned_backlight-wasd
suffixes there, as well as document some sort
of atomically change all 4 zones at once API.
Werner, if this sounds like something which would
work for you, then it would probably be best to
first submit a RFC patch introducing a:
Documentation/ABI/testing/sysfs-class-led-zoned-kbd-backlight
and then first discuss that with the LED subsys
maintainers, so that we have buy-in from the LED
subsys maintainers before you start actually
implementing this.
I'll reply to your "I also stumbled across a new Problem"
in another reply as it seems best to start a separate
thread for this.
Regards,
Hans
^ permalink raw reply
* Re: Userspace API for per key backlight for non HID (no hidraw) keyboards
From: Hans de Goede @ 2024-01-17 16:50 UTC (permalink / raw)
To: Werner Sembach, Pavel Machek, Jani Nikula, jikos,
Jelle van der Waa, Armin Wolf
Cc: Miguel Ojeda, Lee Jones, linux-kernel,
dri-devel@lists.freedesktop.org, linux-input, ojeda, linux-leds
In-Reply-To: <ac02143c-d417-49e5-9c6e-150cbda71ba7@tuxedocomputers.com>
Hi All,
On 11/27/23 11:59, Werner Sembach wrote:
<snip>
> I also stumbled across a new Problem:
>
> We have an upcoming device that has a per-key keyboard backlight, but does the control completely via a wmi/acpi interface. So no usable hidraw here for a potential userspace driver implementation ...
>
> So a quick summary for the ideas floating in this thread so far:
>
> 1. Expand leds interface allowing arbitrary modes with semi arbitrary optional attributes:
>
> - Pro:
>
> - Still offers all default attributes for use with UPower
>
> - Fairly simple to implement from the preexisting codebase
>
> - Could be implemented for all (to me) known internal keyboard backlights
>
> - Con:
>
> - Violates the simplicity paradigm of the leds interface (e.g. with this one leds entry controls possible multiple leds)
So what you are suggesting here is having some way (a-z + other sysfs attr?)
to use a single LED class device and then extend that to allow setting all
keys ?
This does not seem like a good idea to me and this will also cause issues
when doing animations in software, since this API will likely be slow.
And if the API is not slow, then it will likely involve some sort
of binary sysfs file for setting multiple keys rather then 1
file per key which would break the normal 1 file per setting sysfs
paradigm.
> 2. Implement per-key keyboards as auxdisplay
>
> - Pro:
>
> - Already has a concept for led positions
With a "concept" you mean simple x,y positioning or is
there something more advanced here that I'm aware of ?
> - Is conceptually closer to "multiple leds forming a singular entity"
>
> - Con:
>
> - No preexisting UPower support
>
> - No concept for special hardware lightning modes
>
> - No support for arbitrary led outlines yet (e.g. ISO style enter-key)
Hmm, so there is very little documentation on this and what
docs there is: Documentation/admin-guide/auxdisplay/cfag12864b.rst
as well as the example program how to uses this suggests that
this is using the old /dev/fb# interface which we are sorta
trying to retire.
> 3. Implement in input subsystem
>
> - Pro:
>
> - Preexisting concept for keys and key purpose
>
> - Con:
>
> - Not in scope for subsystem
>
> - No other preexisting light infrastructure
Dmitry actually recently nacked the addition of
a LED_MIC_MUTE define to include/uapi/linux/input-event-codes.h
which was intended to be able to allow the input LED support
with standard HID mic-mute leds (spk-mute is already supported
this way).
Dmitry was very clear that no new LEDs must be added and
that any new LED support should be done through the LED
subsytem, so I do not think that something like this
is going to fly.
> 4. Implement a simple leds driver only supporting a small subset of the capabilities and make it disable-able for a userspace driver to take over
>
> - Pro:
>
> - Most simple to implement basic support
>
> - In scope for led subsystem simplicity paradigm
>
> - Con:
>
> - Not all built in keyboard backlights can be implemented in a userspace only driver
Right, so this is basically what we have been discussing in the other
part of the thread with the:
/sys/bus/hid/devices/0003:xxxx:xxxx.xxxx/disable_kbd_backlight_support
proposal to unregister the kernel's LED class device and then
allow userspace to do whatever it wants through /dev/hidraw
without the kernel also trying to access the backlight
functionality at the same time.
AFAIK there already is a bunch of userspace support for
per key addressable kbd RGB backlights using hidraw support,
so this way we can use the momentum / code of these existing
projects, at least for existing hidraw keyboards and adding
support for:
/sys/bus/hid/devices/0003:xxxx:xxxx.xxxx/disable_kbd_backlight_support
to these existing projects should be simple.
Yet this will not work for your mentioned "control completely
via a wmi/acpi interface". Still I think we should go the same
route for those adding a misc-char device or something like
that to allow making WMI calls from userspace (like Windows
can do). Maybe with an allow list per GUID to only allow
specific calls, so that we can avoid possible dangerous calls.
Armin Wolf recently became the WMI bus maintainer.
Armin, we are discussing how to deal with (laptop) keyboards
which have a separate RGB LED per key and how to control
those LEDs.
So far per key addressable keyboard backlighting has always
been HID based, so any existing support is just userspace
based using /dev/hidraw. In my experience the problem with
supporting gaming peripherals is that there is interest in it,
but not really enough interest to keep a sustained momentum
behind projects, especially not when it comes to taking code
from a fun weekend hack to upstreaming them into bigger
projects like the kernel.
So I would like to offer some sort of easy accessible
API to userspace for accessing this, basically allowing
userspace drivers for the LED part of the keyboard which
in some cases will involve making WMI calls from userspace.
So, Armin, what do you think about a way of allowing
(filtered) WMI calls from userspace through say
a misc-char device + ioctls or something like that?
Werner atm I personally do think that option 4. from
your list is the way to go. Mainly because designing
a generic kernel API for all bells and whistles of gaming
hw is very tricky and would require a large ongoing
effort which I just don't see happening (based on
past experience).
Regards,
Hans
^ permalink raw reply
* Re: Flood of logitech-hidpp-device messages in v6.7
From: Jiri Kosina @ 2024-01-17 19:01 UTC (permalink / raw)
To: Hans de Goede
Cc: Oleksandr Natalenko, linux-kernel, linux-input, Filipe Laíns,
Bastien Nocera, Benjamin Tissoires
In-Reply-To: <824573bb-ae01-41b9-8f97-a760ae8f3f18@redhat.com>
On Tue, 9 Jan 2024, Hans de Goede wrote:
> > Jan 09 10:05:06 spock kernel: logitech-hidpp-device 0003:046D:4051.0006: Disconnected
> > Jan 09 10:07:15 spock kernel: logitech-hidpp-device 0003:046D:408A.0005: Disconnected
> > Jan 09 10:16:51 spock kernel: logitech-hidpp-device 0003:046D:4051.0006: HID++ 4.5 device connected.
> > Jan 09 10:16:55 spock kernel: logitech-hidpp-device 0003:046D:408A.0005: HID++ 4.5 device connected.
> > Jan 09 10:16:55 spock kernel: logitech-hidpp-device 0003:046D:408A.0005: HID++ 4.5 device connected.
> > Jan 09 10:36:31 spock kernel: logitech-hidpp-device 0003:046D:4051.0006: Disconnected
> > Jan 09 10:37:07 spock kernel: logitech-hidpp-device 0003:046D:4051.0006: HID++ 4.5 device connected.
> > Jan 09 10:46:21 spock kernel: logitech-hidpp-device 0003:046D:4051.0006: Disconnected
> > Jan 09 10:48:23 spock kernel: logitech-hidpp-device 0003:046D:408A.0005: Disconnected
> > Jan 09 11:12:27 spock kernel: logitech-hidpp-device 0003:046D:4051.0006: HID++ 4.5 device connected.
> > Jan 09 11:12:47 spock kernel: logitech-hidpp-device 0003:046D:408A.0005: HID++ 4.5 device connected.
> > Jan 09 11:12:47 spock kernel: logitech-hidpp-device 0003:046D:408A.0005: HID++ 4.5 device connected.
> > Jan 09 11:38:32 spock kernel: logitech-hidpp-device 0003:046D:4051.0006: Disconnected
> > Jan 09 11:43:32 spock kernel: logitech-hidpp-device 0003:046D:408A.0005: Disconnected
> > Jan 09 11:45:10 spock kernel: logitech-hidpp-device 0003:046D:4051.0006: HID++ 4.5 device connected.
> > Jan 09 11:45:11 spock kernel: logitech-hidpp-device 0003:046D:408A.0005: HID++ 4.5 device connected.
> > Jan 09 11:45:11 spock kernel: logitech-hidpp-device 0003:046D:408A.0005: HID++ 4.5 device connected.
> > Jan 09 12:31:48 spock kernel: logitech-hidpp-device 0003:046D:4051.0006: Disconnected
> > Jan 09 12:33:21 spock kernel: logitech-hidpp-device 0003:046D:4051.0006: HID++ 4.5 device connected.
> > ```
> >
> > I've got the following hardware:
> >
> > * Bus 006 Device 004: ID 046d:c52b Logitech, Inc. Unifying Receiver
> > * Logitech MX Keys
> > * Logitech M510v2
> >
> > With v6.6 I do not get those messages.
> >
> > I think this is related to 680ee411a98e ("HID: logitech-hidpp: Fix connect event race").
> >
> > My speculation is that some of the devices enter powersaving state after being idle for some time (5 mins?), and then wake up and reconnect once I touch either keyboard or mouse. I should highlight that everything works just fine, it is the flood of messages that worries me.
> >
> > Is it expected?
>
> Yes this is expected, looking at your logs I see about 10 messages per
> hour which IMHO is not that bad.
>
> I guess we could change things to track we have logged the connect
> message once and if yes then log future connect messages (and all
> disconnect messages) at debug level.
>
> Jiri, Benjamin, do you have any opinion on this ?
Works for me, thanks. Do you plan to submit the patch implementing this?
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: Userspace API for per key backlight for non HID (no hidraw) keyboards
From: Armin Wolf @ 2024-01-17 19:03 UTC (permalink / raw)
To: Hans de Goede, Werner Sembach, Pavel Machek, Jani Nikula, jikos,
Jelle van der Waa
Cc: Miguel Ojeda, Lee Jones, linux-kernel,
dri-devel@lists.freedesktop.org, linux-input, ojeda, linux-leds
In-Reply-To: <3040290f-9b26-4fd3-8e64-a03ec59921d6@redhat.com>
Am 17.01.24 um 17:50 schrieb Hans de Goede:
> Hi All,
>
> On 11/27/23 11:59, Werner Sembach wrote:
>
> <snip>
>
>> I also stumbled across a new Problem:
>>
>> We have an upcoming device that has a per-key keyboard backlight, but does the control completely via a wmi/acpi interface. So no usable hidraw here for a potential userspace driver implementation ...
>>
>> So a quick summary for the ideas floating in this thread so far:
>>
>> 1. Expand leds interface allowing arbitrary modes with semi arbitrary optional attributes:
>>
>> - Pro:
>>
>> - Still offers all default attributes for use with UPower
>>
>> - Fairly simple to implement from the preexisting codebase
>>
>> - Could be implemented for all (to me) known internal keyboard backlights
>>
>> - Con:
>>
>> - Violates the simplicity paradigm of the leds interface (e.g. with this one leds entry controls possible multiple leds)
> So what you are suggesting here is having some way (a-z + other sysfs attr?)
> to use a single LED class device and then extend that to allow setting all
> keys ?
>
> This does not seem like a good idea to me and this will also cause issues
> when doing animations in software, since this API will likely be slow.
>
> And if the API is not slow, then it will likely involve some sort
> of binary sysfs file for setting multiple keys rather then 1
> file per key which would break the normal 1 file per setting sysfs
> paradigm.
>
>> 2. Implement per-key keyboards as auxdisplay
>>
>> - Pro:
>>
>> - Already has a concept for led positions
> With a "concept" you mean simple x,y positioning or is
> there something more advanced here that I'm aware of ?
>
>> - Is conceptually closer to "multiple leds forming a singular entity"
>>
>> - Con:
>>
>> - No preexisting UPower support
>>
>> - No concept for special hardware lightning modes
>>
>> - No support for arbitrary led outlines yet (e.g. ISO style enter-key)
> Hmm, so there is very little documentation on this and what
> docs there is: Documentation/admin-guide/auxdisplay/cfag12864b.rst
> as well as the example program how to uses this suggests that
> this is using the old /dev/fb# interface which we are sorta
> trying to retire.
>
>
>> 3. Implement in input subsystem
>>
>> - Pro:
>>
>> - Preexisting concept for keys and key purpose
>>
>> - Con:
>>
>> - Not in scope for subsystem
>>
>> - No other preexisting light infrastructure
> Dmitry actually recently nacked the addition of
> a LED_MIC_MUTE define to include/uapi/linux/input-event-codes.h
> which was intended to be able to allow the input LED support
> with standard HID mic-mute leds (spk-mute is already supported
> this way).
>
> Dmitry was very clear that no new LEDs must be added and
> that any new LED support should be done through the LED
> subsytem, so I do not think that something like this
> is going to fly.
>
>
>> 4. Implement a simple leds driver only supporting a small subset of the capabilities and make it disable-able for a userspace driver to take over
>>
>> - Pro:
>>
>> - Most simple to implement basic support
>>
>> - In scope for led subsystem simplicity paradigm
>>
>> - Con:
>>
>> - Not all built in keyboard backlights can be implemented in a userspace only driver
> Right, so this is basically what we have been discussing in the other
> part of the thread with the:
>
> /sys/bus/hid/devices/0003:xxxx:xxxx.xxxx/disable_kbd_backlight_support
>
> proposal to unregister the kernel's LED class device and then
> allow userspace to do whatever it wants through /dev/hidraw
> without the kernel also trying to access the backlight
> functionality at the same time.
>
> AFAIK there already is a bunch of userspace support for
> per key addressable kbd RGB backlights using hidraw support,
> so this way we can use the momentum / code of these existing
> projects, at least for existing hidraw keyboards and adding
> support for:
>
> /sys/bus/hid/devices/0003:xxxx:xxxx.xxxx/disable_kbd_backlight_support
>
> to these existing projects should be simple.
>
> Yet this will not work for your mentioned "control completely
> via a wmi/acpi interface". Still I think we should go the same
> route for those adding a misc-char device or something like
> that to allow making WMI calls from userspace (like Windows
> can do). Maybe with an allow list per GUID to only allow
> specific calls, so that we can avoid possible dangerous calls.
>
> Armin Wolf recently became the WMI bus maintainer.
>
> Armin, we are discussing how to deal with (laptop) keyboards
> which have a separate RGB LED per key and how to control
> those LEDs.
>
> So far per key addressable keyboard backlighting has always
> been HID based, so any existing support is just userspace
> based using /dev/hidraw. In my experience the problem with
> supporting gaming peripherals is that there is interest in it,
> but not really enough interest to keep a sustained momentum
> behind projects, especially not when it comes to taking code
> from a fun weekend hack to upstreaming them into bigger
> projects like the kernel.
>
> So I would like to offer some sort of easy accessible
> API to userspace for accessing this, basically allowing
> userspace drivers for the LED part of the keyboard which
> in some cases will involve making WMI calls from userspace.
>
> So, Armin, what do you think about a way of allowing
> (filtered) WMI calls from userspace through say
> a misc-char device + ioctls or something like that?
>
> Werner atm I personally do think that option 4. from
> your list is the way to go. Mainly because designing
> a generic kernel API for all bells and whistles of gaming
> hw is very tricky and would require a large ongoing
> effort which I just don't see happening (based on
> past experience).
>
> Regards,
>
> Hans
>
Hi,
i can understand your concerns, but i strongly advise against a generic WMI userspace API.
The reasons for this are:
1. We are still unable to parse (and use) the binary MOF buffers describing the WMI devices,
so all of that would require the driver parsing a raw byte buffer. In this case a separate
misc device managed by the driver would basically do the same.
2. Many WMI implementations are like RWEverything implemented inside the ACPI firmware, so
most devices will require the driver to do excessive filtering. Many implementations also do
no proper input validation either so the driver has to know all possible use cases since he
has to protect the buggy ACPI firmware from userspace attacks.
Regarding point number 2, i just had to contact Asus so that they remove a broken WMI interface
from my motherboard or else a simple application could crash the Windows kernel. This firmware
is (sadly) being designed as an internal API and thus unstable beyond all imagination.
For HID devices, a userspace driver might be OK since they are somewhat isolated from the remaining
hardware, but WMI is basically a kernel bypass for ACPI firmware calls, so userspace could easily
attack the kernel is way.
Personally, i would prefer extending the LED subsystem to support zone-like devices with many LEDs,
as this would prevent userspace from having to tinker with the hardware behind the kernels back.
Other highly device-specific features could be implemented with a driver-specific misc device.
Regarding the speed, it depends on the underlying WMI interface design if smooth animations are
even possible, since many WMI interfaces are quite slow. Can you share the Binary MOF buffers
describing the WMI interfaces in question?
Thanks,
Armin Wolf
^ permalink raw reply
* Re: [PATCH v9 3/3] HID: cp2112: Fwnode Support
From: Danny Kaehn @ 2024-01-17 18:48 UTC (permalink / raw)
To: andriy.shevchenko@linux.intel.com
Cc: kaehndan@gmail.com, bartosz.golaszewski@linaro.org,
benjamin.tissoires@redhat.com, Ethan Twardy,
devicetree@vger.kernel.org, dmitry.torokhov@gmail.com,
jikos@kernel.org, linux-input@vger.kernel.org,
niyas.sait@linaro.org, krzysztof.kozlowski+dt@linaro.org,
robh+dt@kernel.org
In-Reply-To: <ZKKpknBH3Pa9mLS1@smile.fi.intel.com>
Hello folks, wanted to give one more follow up on this
patch/discussion. Would a reasonable next step for me
to help nudge this forward be to submit a v10 addressing
Andy's most recent code comments? Again hoping I'm not being
rude or stepping on toes; just want to make sure I'm doing my
dilligence to move things forward. I'll assume that going ahead
and submitting a v10 with unresolved discussion here isn't a
terrible offense if I don't end up getting a response here in
the next week or so.
Leaving some links to some of the more key points of the discussion
across the versions of this patch, since it's been ~5 months since
the last activity here.
Discussion began with discussion of using child nodes by name
across DT with ACPI, for binding fwnodes for the CP2112's I2C
and GPIO controllers; since ACPI requires uppercase names (and
names should specifically not be meaningful in ACPI)
https://lore.kernel.org/all/Y%2F9oO1AE6GK6CQmp@smile.fi.intel.com/
Andy suggested I use _ADR to identify the child node by index
for ACPI
https://lore.kernel.org/all/ZAi4NjqXTbLpVhPo@smile.fi.intel.com/
v9 implemented matching by child node name OR by address depnding
on the type of firmware used
https://lore.kernel.org/all/20230319204802.1364-4-kaehndan@gmail.com/
Some additional discussion on whether matching child nodes by name
is the best approach even for the DT side
(also within the in-line body of this email)
https://lore.kernel.org/all/ZBhoHzTr5l38u%2FkX@smile.fi.intel.com/
The DT binding patch in question
https://lore.kernel.org/all/20230319204802.1364-2-kaehndan@gmail.com/
Thanks,
Danny Kaehn
On Mon, Jul 3 2023 at 13:57:22 +0300 Andy Shevchenko write:
> On Mon, May 01, 2023 at 06:35:44PM -0500, Daniel Kaehn wrote:
> > On Mon, Mar 20, 2023 at 9:10 AM Andy Shevchenko
> > <andriy.shevchenko@linux.intel.com> wrote:
> > > On Mon, Mar 20, 2023 at 08:40:07AM -0500, Daniel Kaehn wrote:
> > > > On Mon, Mar 20, 2023 at 8:00 AM Andy Shevchenko
> > > > <andriy.shevchenko@linux.intel.com> wrote:
> > > > > On Mon, Mar 20, 2023 at 02:58:07PM +0200, Andy Shevchenko
Wrote:
> > > > > > On Sun, Mar 19, 2023 at 03:48:02PM -0500, Danny Kaehn
wrote:
> +Cc: Niyas, who is working a lot on filling the gaps in ACPI in
comparison
> to DT in the Linux kernel. Perhaps he has some ideas or even
better
> solutions.
>
>
> ...
>
> > > > > > > + device_for_each_child_node(&hdev->dev, child) {
> > > > > > > + name = fwnode_get_name(child);
> > > > > > > + ret =
acpi_get_local_address(ACPI_HANDLE_FWNODE(child), &addr);
> > > > > > > +
> > > > > > > + if ((name && strcmp("i2c", name) == 0) ||
(!ret && addr == 0))
> > > > > > > + device_set_node(&dev->adap.dev,
child);
> > > > > > > + else if ((name && strcmp("gpio", name)) == 0
||
> > > > > > > + (!ret && addr == 1))
> > > > > > > + dev->gc.fwnode = child;
> > > > > > > + }
> > > > > >
> > > > > > Please, make addresses defined explicitly. You may also do
it with node naming
> > > > > > schema:
> > > > > >
> > > > > > #define CP2112_I2C_ADR 0
> > > > > > #define CP2112_GPIO_ADR 1
> > > > > >
> > > > > > static const char * const cp2112_cell_names[] = {
> > > > > > [CP2112_I2C_ADR] = "i2c",
> > > > > > [CP2112_GPIO_ADR] = "gpio",
> > > > > > };
> > > > > >
> > > > > > device_for_each_child_node(&hdev->dev, child) {
> > > > > > name = fwnode_get_name(child);
> > > > > > if (name) {
> > > > > > ret = match_string(cp2112_cell_names,
ARRAY_SIZE(cp2112_cell_names), name);
> > > > > > if (ret >= 0)
> > > > > > addr = ret;
> > > > > > } else
> > > > > > ret =
acpi_get_local_address(ACPI_HANDLE_FWNODE(child), &addr);
> > > > > > if (ret < 0)
> > > > > > ...error handling if needed...
> > > > > >
> > > > > > switch (addr) {
> > > > > > case CP2112_I2C_ADR:
> > > > > > device_set_node(&dev->adap.dev,
child);
> > > > > > break;
> > > > > > case CP2112_GPIO_ADR:
> > > > > > dev->gc.fwnode = child;
> > > > > > break;
> > > > > > default:
> > > > > > ...error handling...
> > > > > > }
> > > > > > }
> > > > >
> > > > > Btw, don't you use "reg" property for the child nodes? It
would be better from
> > > > > de facto used patterns (we have a couple of mode drivers that
have a common
> > > > > code to read "reg" or _ADR() and that code can be split into
a helper and used
> > > > > here).
> > > >
> > > > Named nodes _seem_ to be preferred in DT for when there isn't a
logical /
> > > > natural numbering to the child nodes. A.e. for USB, reg is used
to specify
> > > > which port, for I2C, which address on the bus, but for two
parallel and
> > > > independent functions on the same device, it seems named nodes
would make
> > > > more sense in DT. Many examples exist in mainline where named
nodes are used
> > > > in DT in this way.
> > >
> > > Okay, I'm not an expert in the DT preferable schemas, so I
believe DT people
> > > should answer on this.
> >
> > Hello,
> >
> > Thanks for all the time spent reviewing this thus far. Following up
to
> > see what my next steps might be.
> >
> > It sounds like we might want some DT folks to weigh in on the
strategy
> > used for identifying the child I2C and GPIO nodes for the CP2112
> > device before moving further toward applying this.
> >
> > Since the DT list is on this thread (as well as Rob+Krzystof), and
> > this has sat for a little while, I'm assuming that the ball is in
my
> > court to seek out an answer/opinion here. (I know folks get a lot
of
> > email, so apologies if the correct move would have been to wait a
bit
> > longer before following up! Not intending to be rude.)
> >
> > Would it be appropriate / expected that I send a separate email
thread
> > to the DT mailing list on their opinion here? Or would that create
> > more confusion/complexity in adding yet another thread? I did
create a
> > separate email thread for the initial DT vs. ACPI conversation we
had
> > about accessing children by name or index in a unified way due to
the
> > differences in upper/lower case and use-cases, but that
> > (understandably) didn't seem to gain any traction.
> >
> > Thanks for any insights!
> >
> > Thanks,
> > Danny Kaehn
> >
> > > > One example is network cards which provide an mdio bus
> > > > bind through the child "mdio". One example of a specifically a
> > > > child i2c controller being bound to "i2c" can be found in
> > > > pine64,pinephone-keyboard.yaml.
> > > > But it's certainly possible this isn't the desired direction
moving forward
> > > > in DT -- my opinion should definitely be taken with a grain of
salt. Maybe
> > > > this is something I should follow up on with DT folks on that
DT vs. ACPI
> > > > thread made earlier.
> > > >
> > > > One thing I did notice when looking at the mfd subsystem is
that most DT
> > > > drivers actually match on the compatible string of the child
nodes, a.e.
> > > > "silabs,cp2112", "silabs,cp2112-gpio". "silabs,cp2112-i2c". We
could
> > > > implement that here, but I think that would make more sense if
we were to
> > > > actually split the cp2112 into mfd & platform drivers, and
additionally split
> > > > the DT binding by function.
> > >
> > > IIRC (but might be very well mistaken) the compatible strings for
children
> > > are discouraged.
>
^ permalink raw reply
* Re: [PATCH v2] HID: i2c-hid: Skip SET_POWER SLEEP for Cirque touchpad on system suspend
From: Doug Anderson @ 2024-01-17 20:35 UTC (permalink / raw)
To: Kai-Heng Feng
Cc: jikos, benjamin.tissoires, Hans de Goede, Maxime Ripard,
Thomas Weißschuh, Johan Hovold, linux-input, linux-kernel
In-Reply-To: <20240115045054.1170294-1-kai.heng.feng@canonical.com>
Hi,
On Sun, Jan 14, 2024 at 8:51 PM Kai-Heng Feng
<kai.heng.feng@canonical.com> wrote:
>
> There's a Cirque touchpad that wakes system up without anything touched
> the touchpad. The input report is empty when this happens.
> The reason is stated in HID over I2C spec, 7.2.8.2:
> "If the DEVICE wishes to wake the HOST from its low power state, it can
> issue a wake by asserting the interrupt."
>
> This is fine if OS can put system back to suspend by identifying input
> wakeup count stays the same on resume, like Chrome OS Dark Resume [0].
> But for regular distro such policy is lacking.
>
> Though the change doesn't bring any impact on power consumption for
> touchpad is minimal, other i2c-hid device may depends on SLEEP control
> power. So use a quirk to limit the change scope.
>
> [0] https://chromium.googlesource.com/chromiumos/platform2/+/HEAD/power_manager/docs/dark_resume.md
>
> Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
> ---
> v2:
> - Use quirk instead of applying the change universally.
>
> drivers/hid/hid-ids.h | 3 +++
> drivers/hid/i2c-hid/i2c-hid-core.c | 6 +++++-
> 2 files changed, 8 insertions(+), 1 deletion(-)
This seems OK to me. As per my repy to v1, it doesn't feel fully
explained what's going on. Why does it only wake the system without
touches if you put it to sleep first? Is it truly a hardware quirk
with the Cirque touchpad or is it a bug on the board you have it
hooked up to? In any case, perhaps it doesn't matter since you said
you measured power here and, on this touchpad it doesn't seem to save
significant extra power to go into sleep mode. ...so I guess I'd be OK
w/
Reviewed-by: Douglas Anderson <dianders@chromium.org>
[1] https://lore.kernel.org/r/20240115045054.1170294-1-kai.heng.feng@canonical.com
^ permalink raw reply
* Re: Userspace API for per key backlight for non HID (no hidraw) keyboards
From: Werner Sembach @ 2024-01-18 0:58 UTC (permalink / raw)
To: Armin Wolf, Hans de Goede, Pavel Machek, Jani Nikula, jikos,
Jelle van der Waa, Christoffer Sandberg
Cc: Miguel Ojeda, Lee Jones, linux-kernel,
dri-devel@lists.freedesktop.org, linux-input, ojeda, linux-leds
In-Reply-To: <6b32fb73-0544-4a68-95ba-e82406a4b188@gmx.de>
Hi Hans and Armin,
Am 17.01.24 um 20:03 schrieb Armin Wolf:
> Am 17.01.24 um 17:50 schrieb Hans de Goede:
>
>> Hi All,
>>
>> On 11/27/23 11:59, Werner Sembach wrote:
>>
>> <snip>
>>
>>> I also stumbled across a new Problem:
>>>
>>> We have an upcoming device that has a per-key keyboard backlight, but does
>>> the control completely via a wmi/acpi interface. So no usable hidraw here
>>> for a potential userspace driver implementation ...
>>>
>>> So a quick summary for the ideas floating in this thread so far:
>>>
>>> 1. Expand leds interface allowing arbitrary modes with semi arbitrary
>>> optional attributes:
>>>
>>> - Pro:
>>>
>>> - Still offers all default attributes for use with UPower
>>>
>>> - Fairly simple to implement from the preexisting codebase
>>>
>>> - Could be implemented for all (to me) known internal keyboard
>>> backlights
>>>
>>> - Con:
>>>
>>> - Violates the simplicity paradigm of the leds interface (e.g. with
>>> this one leds entry controls possible multiple leds)
>> So what you are suggesting here is having some way (a-z + other sysfs attr?)
>> to use a single LED class device and then extend that to allow setting all
>> keys ?
>>
>> This does not seem like a good idea to me and this will also cause issues
>> when doing animations in software, since this API will likely be slow.
>>
>> And if the API is not slow, then it will likely involve some sort
>> of binary sysfs file for setting multiple keys rather then 1
>> file per key which would break the normal 1 file per setting sysfs
>> paradigm.
>>
>>> 2. Implement per-key keyboards as auxdisplay
>>>
>>> - Pro:
>>>
>>> - Already has a concept for led positions
>> With a "concept" you mean simple x,y positioning or is
>> there something more advanced here that I'm aware of ?
>>
>>> - Is conceptually closer to "multiple leds forming a singular entity"
>>>
>>> - Con:
>>>
>>> - No preexisting UPower support
>>>
>>> - No concept for special hardware lightning modes
>>>
>>> - No support for arbitrary led outlines yet (e.g. ISO style enter-key)
>> Hmm, so there is very little documentation on this and what
>> docs there is: Documentation/admin-guide/auxdisplay/cfag12864b.rst
>> as well as the example program how to uses this suggests that
>> this is using the old /dev/fb# interface which we are sorta
>> trying to retire.
>>
>>
>>> 3. Implement in input subsystem
>>>
>>> - Pro:
>>>
>>> - Preexisting concept for keys and key purpose
>>>
>>> - Con:
>>>
>>> - Not in scope for subsystem
>>>
>>> - No other preexisting light infrastructure
>> Dmitry actually recently nacked the addition of
>> a LED_MIC_MUTE define to include/uapi/linux/input-event-codes.h
>> which was intended to be able to allow the input LED support
>> with standard HID mic-mute leds (spk-mute is already supported
>> this way).
>>
>> Dmitry was very clear that no new LEDs must be added and
>> that any new LED support should be done through the LED
>> subsytem, so I do not think that something like this
>> is going to fly.
>>
>>
>>> 4. Implement a simple leds driver only supporting a small subset of the
>>> capabilities and make it disable-able for a userspace driver to take over
>>>
>>> - Pro:
>>>
>>> - Most simple to implement basic support
>>>
>>> - In scope for led subsystem simplicity paradigm
>>>
>>> - Con:
>>>
>>> - Not all built in keyboard backlights can be implemented in a
>>> userspace only driver
>> Right, so this is basically what we have been discussing in the other
>> part of the thread with the:
>>
>> /sys/bus/hid/devices/0003:xxxx:xxxx.xxxx/disable_kbd_backlight_support
>>
>> proposal to unregister the kernel's LED class device and then
>> allow userspace to do whatever it wants through /dev/hidraw
>> without the kernel also trying to access the backlight
>> functionality at the same time.
>>
>> AFAIK there already is a bunch of userspace support for
>> per key addressable kbd RGB backlights using hidraw support,
>> so this way we can use the momentum / code of these existing
>> projects, at least for existing hidraw keyboards and adding
>> support for:
>>
>> /sys/bus/hid/devices/0003:xxxx:xxxx.xxxx/disable_kbd_backlight_support
>>
>> to these existing projects should be simple.
>>
>> Yet this will not work for your mentioned "control completely
>> via a wmi/acpi interface". Still I think we should go the same
>> route for those adding a misc-char device or something like
>> that to allow making WMI calls from userspace (like Windows
>> can do). Maybe with an allow list per GUID to only allow
>> specific calls, so that we can avoid possible dangerous calls.
>>
>> Armin Wolf recently became the WMI bus maintainer.
>>
>> Armin, we are discussing how to deal with (laptop) keyboards
>> which have a separate RGB LED per key and how to control
>> those LEDs.
>>
>> So far per key addressable keyboard backlighting has always
>> been HID based, so any existing support is just userspace
>> based using /dev/hidraw. In my experience the problem with
>> supporting gaming peripherals is that there is interest in it,
>> but not really enough interest to keep a sustained momentum
>> behind projects, especially not when it comes to taking code
>> from a fun weekend hack to upstreaming them into bigger
>> projects like the kernel.
>>
>> So I would like to offer some sort of easy accessible
>> API to userspace for accessing this, basically allowing
>> userspace drivers for the LED part of the keyboard which
>> in some cases will involve making WMI calls from userspace.
>>
>> So, Armin, what do you think about a way of allowing
>> (filtered) WMI calls from userspace through say
>> a misc-char device + ioctls or something like that?
>>
>> Werner atm I personally do think that option 4. from
>> your list is the way to go. Mainly because designing
>> a generic kernel API for all bells and whistles of gaming
>> hw is very tricky and would require a large ongoing
>> effort which I just don't see happening (based on
>> past experience).
>>
>> Regards,
>>
>> Hans
>>
> Hi,
>
> i can understand your concerns, but i strongly advise against a generic WMI
> userspace API.
> The reasons for this are:
>
> 1. We are still unable to parse (and use) the binary MOF buffers describing
> the WMI devices,
> so all of that would require the driver parsing a raw byte buffer. In this
> case a separate
> misc device managed by the driver would basically do the same.
>
> 2. Many WMI implementations are like RWEverything implemented inside the ACPI
> firmware, so
> most devices will require the driver to do excessive filtering. Many
> implementations also do
> no proper input validation either so the driver has to know all possible use
> cases since he
> has to protect the buggy ACPI firmware from userspace attacks.
Or the WMI has a straight forward arbitrary read/write function into EC ram
(e.g. all Uniwill/TongFang devices).
The filtering would need to be explicit whitelisting of wmi-calls+arguments.
Don't know if this would reduce complexity for the kernel.
>
> Regarding point number 2, i just had to contact Asus so that they remove a
> broken WMI interface
> from my motherboard or else a simple application could crash the Windows
> kernel. This firmware
> is (sadly) being designed as an internal API and thus unstable beyond all
> imagination.
>
> For HID devices, a userspace driver might be OK since they are somewhat
> isolated from the remaining
> hardware, but WMI is basically a kernel bypass for ACPI firmware calls, so
> userspace could easily
> attack the kernel is way.
>
> Personally, i would prefer extending the LED subsystem to support zone-like
> devices with many LEDs,
> as this would prevent userspace from having to tinker with the hardware behind
> the kernels back.
> Other highly device-specific features could be implemented with a
> driver-specific misc device.
Something like my earlier suggestion "[...] adds a new entry zones_count.
multi_intensity has now colors count * zones_count entries. aka a RGB keyboard
with 126 leds would take 378 values for multi_intensity [...]"?
Setting all with one file access to multi_intensity could make it somewhat
performant as Hans already mentioned, but also would violate the one file one
led paradigm.
Or formulated differently: How should the sysfs folder look:
leds/
rgb:kbd_backlight_a/
brightness
multi_intensity
rgb:kbd_backlight_b/
brightness
multi_intensity
...
or
leds/
rgb:kbd_backlight/
brightness
multi_intensity_a
multi_intensity_b
...
or
leds/
rgb:kbd_backlight/
brightness
zones_count
multi_intensity
Personally I don't really like the idea of having the color set in
/sys/class/leds/*:rgb:kbd_backlight/multi_intensity and e.g. the breathing mode
in /sys/class/misc/<some_random_name>/<some_random_attribute>. Or at least there
should be a hint in /sys/class/leds/*:rgb:kbd_backlight/ for the userspace to
know where to look for associated additional attributes.
>
> Regarding the speed, it depends on the underlying WMI interface design if
> smooth animations are
> even possible, since many WMI interfaces are quite slow. Can you share the
> Binary MOF buffers
> describing the WMI interfaces in question?
Taking a colleague in the loop who currently has the device at hand.
@Christoffer can you extract it? Is it one wmi call per key or is there a "set
all" wmi call (because performance)?
>
> Thanks,
> Armin Wolf
>
Kind regards,
Werner
^ permalink raw reply
* [dtor-input:for-linus] BUILD SUCCESS 58f65f9db7e0de366a5a115c2e2c0703858bba69
From: kernel test robot @ 2024-01-18 3:10 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: 58f65f9db7e0de366a5a115c2e2c0703858bba69 Input: atkbd - use ab83 as id when skipping the getid command
elapsed time: 1765m
configs tested: 104
configs skipped: 0
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 randconfig-001-20240118 gcc
arc randconfig-002-20240118 gcc
arm allmodconfig gcc
arm allnoconfig gcc
arm defconfig clang
arm randconfig-001-20240118 clang
arm randconfig-002-20240118 clang
arm randconfig-003-20240118 clang
arm randconfig-004-20240118 clang
arm64 allnoconfig gcc
arm64 defconfig gcc
arm64 randconfig-001-20240118 clang
arm64 randconfig-002-20240118 clang
arm64 randconfig-003-20240118 clang
arm64 randconfig-004-20240118 clang
csky allnoconfig gcc
csky defconfig gcc
csky randconfig-001-20240118 gcc
csky randconfig-002-20240118 gcc
hexagon allnoconfig clang
hexagon defconfig clang
hexagon randconfig-001-20240118 clang
hexagon randconfig-002-20240118 clang
i386 allmodconfig clang
i386 allnoconfig clang
i386 allyesconfig clang
i386 buildonly-randconfig-001-20240117 gcc
i386 buildonly-randconfig-002-20240117 gcc
i386 buildonly-randconfig-003-20240117 gcc
i386 buildonly-randconfig-004-20240117 gcc
i386 buildonly-randconfig-005-20240117 gcc
i386 buildonly-randconfig-006-20240117 gcc
i386 defconfig gcc
i386 randconfig-001-20240117 gcc
i386 randconfig-002-20240117 gcc
i386 randconfig-003-20240117 gcc
i386 randconfig-004-20240117 gcc
i386 randconfig-005-20240117 gcc
i386 randconfig-006-20240117 gcc
i386 randconfig-011-20240117 clang
i386 randconfig-012-20240117 clang
i386 randconfig-013-20240117 clang
i386 randconfig-014-20240117 clang
i386 randconfig-015-20240117 clang
i386 randconfig-016-20240117 clang
loongarch allmodconfig gcc
loongarch allnoconfig gcc
loongarch defconfig gcc
loongarch randconfig-001-20240118 gcc
loongarch randconfig-002-20240118 gcc
m68k allmodconfig gcc
m68k allnoconfig gcc
m68k allyesconfig gcc
microblaze allmodconfig gcc
microblaze allyesconfig gcc
mips allyesconfig gcc
nios2 allmodconfig gcc
nios2 allyesconfig gcc
nios2 randconfig-001-20240118 gcc
nios2 randconfig-002-20240118 gcc
openrisc allyesconfig gcc
parisc allmodconfig gcc
parisc allyesconfig gcc
parisc randconfig-001-20240118 gcc
parisc randconfig-002-20240118 gcc
powerpc allmodconfig clang
powerpc allyesconfig clang
powerpc randconfig-001-20240118 clang
powerpc randconfig-002-20240118 clang
powerpc randconfig-003-20240118 clang
powerpc64 randconfig-001-20240118 clang
powerpc64 randconfig-002-20240118 clang
powerpc64 randconfig-003-20240118 clang
riscv allmodconfig gcc
riscv allyesconfig gcc
riscv randconfig-001-20240118 clang
riscv randconfig-002-20240118 clang
riscv rv32_defconfig clang
s390 allmodconfig gcc
s390 allyesconfig gcc
s390 randconfig-001-20240118 gcc
s390 randconfig-002-20240118 gcc
sh allmodconfig gcc
sh allyesconfig gcc
sh randconfig-001-20240118 gcc
sh randconfig-002-20240118 gcc
sparc allmodconfig gcc
sparc64 allmodconfig gcc
sparc64 allyesconfig gcc
sparc64 randconfig-001-20240118 gcc
sparc64 randconfig-002-20240118 gcc
um allmodconfig clang
um allyesconfig clang
x86_64 allnoconfig gcc
x86_64 allyesconfig clang
x86_64 defconfig gcc
x86_64 rhel-8.3-rust clang
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* Re: [PATCH] selftests/hid: wacom: fix confidence tests
From: Benjamin Tissoires @ 2024-01-18 8:13 UTC (permalink / raw)
To: Jason Gerecke
Cc: Benjamin Tissoires, Jiri Kosina, Shuah Khan, Jason Gerecke,
linux-input, linux-kselftest, linux-kernel
In-Reply-To: <CANRwn3QhYtuiGemwrf6_bYDDeAU0GrPm7T9ca3OzQaX8vGKPEQ@mail.gmail.com>
On Wed, Jan 17, 2024 at 7:12 PM Jason Gerecke <killertofu@gmail.com> wrote:
>
> LGTM. Acked-by: Jason Gerecke <jason.gerecke@wacom.com>
Thanks!
I'll add a:
Fixes: b0fb904d074e ("HID: wacom: Add additional tests of confidence behavior")
And send to Linus in the next round for 6.8 so we also fix the future
for-6.9 branches
Cheers,
Benjamin
>
>
> Jason
> ---
> Now instead of four in the eights place /
> you’ve got three, ‘Cause you added one /
> (That is to say, eight) to the two, /
> But you can’t take seven from three, /
> So you look at the sixty-fours....
>
>
>
> On Wed, Jan 17, 2024 at 5:27 AM Benjamin Tissoires <bentiss@kernel.org> wrote:
>>
>> The device is exported with a fuzz of 4, meaning that the `+ t` here
>> is removed by the fuzz algorithm, making those tests failing.
>>
>> Not sure why, but when I run this locally it was passing, but not in the
>> VM.
>>
>> Link: https://gitlab.freedesktop.org/bentiss/hid/-/jobs/53692957#L3315
>> Signed-off-by: Benjamin Tissoires <bentiss@kernel.org>
>> ---
>> Over the break the test suite wasn't properly running on my runner,
>> and this small issue sneaked in.
>> ---
>> tools/testing/selftests/hid/tests/test_wacom_generic.py | 8 ++++----
>> 1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/tools/testing/selftests/hid/tests/test_wacom_generic.py b/tools/testing/selftests/hid/tests/test_wacom_generic.py
>> index 352fc39f3c6c..b62c7dba6777 100644
>> --- a/tools/testing/selftests/hid/tests/test_wacom_generic.py
>> +++ b/tools/testing/selftests/hid/tests/test_wacom_generic.py
>> @@ -880,8 +880,8 @@ class TestDTH2452Tablet(test_multitouch.BaseTest.TestMultitouch, TouchTabletTest
>> does not overlap with other contacts. The value of `t` may be
>> incremented over time to move the point along a linear path.
>> """
>> - x = 50 + 10 * contact_id + t
>> - y = 100 + 100 * contact_id + t
>> + x = 50 + 10 * contact_id + t * 11
>> + y = 100 + 100 * contact_id + t * 11
>> return test_multitouch.Touch(contact_id, x, y)
>>
>> def make_contacts(self, n, t=0):
>> @@ -902,8 +902,8 @@ class TestDTH2452Tablet(test_multitouch.BaseTest.TestMultitouch, TouchTabletTest
>> tracking_id = contact_ids.tracking_id
>> slot_num = contact_ids.slot_num
>>
>> - x = 50 + 10 * contact_id + t
>> - y = 100 + 100 * contact_id + t
>> + x = 50 + 10 * contact_id + t * 11
>> + y = 100 + 100 * contact_id + t * 11
>>
>> # If the data isn't supposed to be stored in any slots, there is
>> # nothing we can check for in the evdev stream.
>>
>> ---
>> base-commit: 80d5a73edcfbd1d8d6a4c2b755873c5d63a1ebd7
>> change-id: 20240117-b4-wip-wacom-tests-fixes-298b50bea47f
>>
>> Best regards,
>> --
>> Benjamin Tissoires <bentiss@kernel.org>
>>
>>
^ permalink raw reply
* [PATCH v7 00/39] ep93xx device tree conversion
From: Nikita Shubin via B4 Relay @ 2024-01-18 8:20 UTC (permalink / raw)
To: Hartley Sweeten, Alexander Sverdlin, Russell King,
Lukasz Majewski, Linus Walleij, Bartosz Golaszewski,
Andy Shevchenko, Michael Turquette, Stephen Boyd,
Sebastian Reichel, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Nikita Shubin, Vinod Koul, Wim Van Sebroeck, Guenter Roeck,
Thierry Reding, Uwe Kleine-König, Mark Brown,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Miquel Raynal, Richard Weinberger, Vignesh Raghavendra,
Damien Le Moal, Sergey Shtylyov, Dmitry Torokhov, Liam Girdwood,
Jaroslav Kysela, Takashi Iwai, Ralf Baechle, Wu, Aaron, Lee Jones,
Olof Johansson, Niklas Cassel
Cc: linux-arm-kernel, linux-kernel, linux-gpio, linux-clk, linux-pm,
devicetree, dmaengine, linux-watchdog, linux-pwm, linux-spi,
netdev, linux-mtd, linux-ide, linux-input, linux-sound,
Arnd Bergmann, Andy Shevchenko, Bartosz Golaszewski,
Krzysztof Kozlowski, Andy Shevchenko, Andrew Lunn,
Andy Shevchenko
The goal is to recieve ACKs for all patches in series to merge it via Arnd branch.
No major changes since last version (v6) all changes are cometic.
Following patches require attention from Stephen Boyd, as they were converted to aux_dev as suggested:
- ARM: ep93xx: add regmap aux_dev
- clk: ep93xx: add DT support for Cirrus EP93xx
Following patches require attention from Vinod Koul:
- dma: cirrus: Convert to DT for Cirrus EP93xx
- dma: cirrus: remove platform code
Following patches are dropped:
- dt-bindings: wdt: Add ts72xx (pulled requested by Wim Van Sebroeck)
Big Thanks to Andy Shevchenko once again.
Patches should be formated with '--histogram'
---
Changes in v7:
mtd: rawnand: add support for ts72xx
- fixed KConfig description
ARM: ep93xx: Add terminator to gpiod_lookup_table
- + Reported-by, Fixes
ARM: ep93xx: add regmap aux_dev
- + trailing comma
- - #include <linux/spinlock.h>
clk: ep93xx: add DT support for Cirrus EP93xx
- dropped unused defines
- return from default in ep93xx_mux_get_parent()
- use guard() in ep93xx_mux_set_parent_lock()
- <math.h> header for abs_diff()
- fixed comments
pinctrl: add a Cirrus ep93xx SoC pin controller
- dropped comments for DEVCFG defines
- <linux/array_size.h> for ARRAY_SIZE()
- + default in ep93xx_get_group_name()
- correct cast for id->driver_data
- s/device_set_of_node_from_dev()/device_set_node()/
power: reset: Add a driver for the ep93xx reset
- Add <linux/container_of.h>, <linux/errno.h>, <linux/slab.h>
- Add <linux/module.h>, <linux/mod_devicetable.h>
- Remove <platform_device.h>
spi: ep93xx: add DT support for Cirrus EP93xx
- Replace with ret = dev_err_probe(...);
ata: pata_ep93xx: add device tree support
- fixed wrong rebase with some partes leaked in "ata: pata_ep93xx: remove legacy pinctrl use"
- fix dma_request_chan() error processing
dma: cirrus: Convert to DT for Cirrus EP93xx
- fixed commit message (dropped explicit "only")
- fixed clk_get() processing to defer probe and log spamming
- refactor ep93xx_m2p_dma_filter()
- dropped blank line in ep93xx_m2p_dma_of_xlate()
- refactor ep93xx_m2m_dma_of_xlate()
dma: cirrus: remove platform code
- s/dma/DMA/ in commit message
soc: Add SoC driver for Cirrus ep93xx
- add period
- use cleanup and guard() for spinlocking
- correct cast for device_get_match_data()
- dropped dev_info() with SoC revision - i can't find it anywhere since 2.6 :/,
don't know why i was so sured that ep93xx always printed that
ata: pata_ep93xx: remove legacy pinctrl use
- made error handling in DMA as Uwe suggested
- Link to v6: https://lore.kernel.org/r/20231212-ep93xx-v6-0-c307b8ac9aa8@maquefel.me
Changes in v6:
- clk: ep93xx: add DT support for Cirrus EP93xx
- s/spin_lock_irqsave()/guard()/
- refactor index check in ep93xx_mux_set_parent_lock() to something more readable
- use in_range in ep93xx_mux_set_parent_lock()/ep93xx_ddiv_set_rate()
- use GENMASK() in ep93xx_ddiv_recalc_rate()
- comment reserved bit in ep93xx_ddiv_set_rate()
- move out from loop ClkDiv value assigment
- some style fixes
Andy, i was i asked to set index of XTALI explicitly, i am not setting ddiv_pdata
there becouse only XTALI is jnown in advance, and i think setting them in one place is more convenient.
- pinctrl: add a Cirrus ep93xx SoC pin controller
- drop OF from Kconfig
- droped linux/of.h include
- add space to */ where it is applicable
- add coma in multiline assigment
- "return NULL" as default case in ep93xx_get_group_name()
- fixed casting id->driver_data
- use device_set_of_node_from_dev()
- use dev_err_probe()
- power: reset: Add a driver for the ep93xx reset
- drop linux/of.h include
- soc: Add SoC driver for Cirrus ep93xx
- s/GPL-2.0/GPL-2.0-only/
- drop linux/kernel.h include
- + blank line before linux/soc/cirrus/ep93xx.h
- + blank line after ep93xx_get_soc_rev()
- + coma for pinctrl_names
- valid casting to int for of_device_get_match_data() return value
- mtd: rawnand: add support for ts72xx
- return as part of switch case
- s/iowrite8/iowrite8_rep/
- net: cirrus: add DT support for Cirrus EP93xx
- fix header sorting
- dma: cirrus: Convert to DT for Cirrus EP93xx
- use devm_clk_get
- use is_slave_direction
Changes in v5:
- gpio: ep93xx: split device in multiple
- ordered headers
- use irqd_to_hwirq()
- s/platform_get_irq()/platform_get_irq_optional()/
- [PATCH v4 02/42] ARM: ep93xx: add swlocked prototypes
- replaced with ARM: ep93xx: add regmap aux_dev
- [PATCH v4 03/42] dt-bindings: clock: Add Cirrus EP93xx
- fixed identation
- removed EP93XX_CLK_END
- and dropped it
- clock bindings moved to syscon with renaming to cirrus,ep9301-syscon.h
- clk: ep93xx: add DT support for Cirrus EP93xx
- convert to auxiliary and use parent device tree node
- moved all clocks except XTALI here
- used devm version everywhere and *_parent_hw() instead of passing name where it's possible
- unfortunately devm_clk_hw_register_fixed_rate doesn't have a parent index version
- [PATCH v4 05/42] dt-bindings: pinctrl: Add Cirrus EP93xx
- "unevaluatedProperties: false" for pins
- returned "additionalProperties: false" where it was
- and dropped it
- pinctrl: add a Cirrus ep93xx SoC pin controller
- sorted includes
- convert to auxiliary and use parent device tree node
- power: reset: Add a driver for the ep93xx reset
- convert to auxiliary device
- dt-bindings: soc: Add Cirrus EP93xx
- dropped all ref to reboot, clk, pinctrl subnodes
- added pins, as it's now used for pinctrl
- added #clock-cells, as it's now used for clk
- dt-bindings: pwm: Add Cirrus EP93xx
- $ref to pwm.yaml
- fixed 'pwm-cells'
- s/additionalProperties/unevaluatedProperties/
- soc: Add SoC driver for Cirrus ep93xx
- removed clocks, they are moved to clk auxiliary driver, as we dropped the clk dt node
- removed all swlocked exported functions
- dropped static spinlock
- added instantiating auxiliary reboot, clk, pinctrl
- dt-bindings: spi: Add Cirrus EP93xx
- Document DMA support
- spi: ep93xx: add DT support for Cirrus EP93xx
- dropped CONFIG_OF and SPI/DMA platform data entirely
- s/master/host/
- reworked DMA setup so we can use probe defer
- dt-bindings: dma: Add Cirrus EP93xx
- dropped bindings header (moved ports description to YAML)
- changed '#dma-cells' to 2, we use port, direction in cells so we can drop platform code completely
- dma: cirrus: add DT support for Cirrus EP93xx
- dropped platform probing completely
- dropped struct ep93xx_dma_data replaced with internal struct ep93xx_dma_chan_cfg with port/direction
- added xlate functions for m2m/m2p
- we require filters to set dma_cfg before hw_setup
- dt-bindings: ata: Add Cirrus EP93xx
- Document DMA support
- ata: pata_ep93xx: add device tree support
- drop DMA platform header with data
- use DMA OF so we can defer probing until DMA is up
- ARM: dts: add Cirrus EP93XX SoC .dtsi
- ARM: dts: ep93xx: add ts7250 board
- ARM: dts: ep93xx: Add EDB9302 DT
- replaced "eclk: clock-controller" to syscon reference
- replaced "pinctrl: pinctrl" to syscon reference
- gpios are now "enabled" by default
- reworked i2s node
- change all dma nodes and refs
- new additions to I2S
- Document DMA
- Document Audio Port usage
- drop legacy DMA support
- Link to v4: https://lore.kernel.org/r/20230915-ep93xx-v4-0-a1d779dcec10@maquefel.me
Changes in v4:
- gpio: ep93xx: split device in multiple
- s/generic_handle_irq/generic_handle_domain_irq/
- s/int offset/irq_hw_number_t offset/ though now it looks a bit odd to me
- drop i = 0
- drop 'error'
- use dev_err_probe withour printing devname once again
dt-bindings: clock: Add Cirrus EP93xx
- renamed cirrus,ep93xx-clock.h -> cirrus,ep9301-clk.h
clk: ep93xx: add DT support for Cirrus EP93xx
- drop unused includes
- use .name only for xtali, pll1, pll2 parents
- convert // to /*
- pass clk_parent_data instead of char* clock name
dt-bindings: pinctrl: Add Cirrus EP93xx
- s/additionalProperties/unevaluatedProperties/
dt-bindings: soc: Add Cirrus EP93xx
- move syscon to soc directory
- add vendor prefix
- make reboot same style as pinctrl, clk
- use absolute path for ref
- expand example
soc: Add SoC driver for Cirrus ep93xx
- s/0xf0000000/GENMASK(31, 28)/
- s/ret/ep93xx_chip_revision(map)/
- drop symbol exports
- convert to platform driver
dt-bindings: rtc: Add Cirrus EP93xx
- allOf: with $ref to rtc.yaml
- s/additionalProperties/unevaluatedProperties/
dt-bindings: watchdog: Add Cirrus EP93x
- drop description
- reword
power: reset: Add a driver for the ep93xx reset
- lets use 'GPL-2.0+' instead of '(GPL-2.0)'
- s/of_device/of/
- drop mdelay with warning
- return 0 at the end
net: cirrus: add DT support for Cirrus EP93xx
- fix leaking np
mtd: nand: add support for ts72xx
- +bits.h
- drop comment
- ok to fwnode_get_next_child_node
- use goto to put handle and nand and report error
ARM: dts: add Cirrus EP93XX SoC .dtsi
- add simple-bus for ebi, as we don't require to setup anything
- add arm,pl011 compatible to uart nodes
- drop i2c-gpio, as it's isn't used anywhere
ARM: dts: ep93xx: add ts7250 board
- generic node name for temperature-sensor
- drop i2c
- move nand, rtc, watchdog to ebi node
- Link to v3: https://lore.kernel.org/r/20230605-ep93xx-v3-0-3d63a5f1103e@maquefel.me
---
Alexander Sverdlin (3):
ASoC: ep93xx: Drop legacy DMA support
ARM: dts: ep93xx: Add EDB9302 DT
ASoC: cirrus: edb93xx: Delete driver
Nikita Shubin (36):
ARM: ep93xx: Add terminator to gpiod_lookup_table
gpio: ep93xx: split device in multiple
ARM: ep93xx: add regmap aux_dev
clk: ep93xx: add DT support for Cirrus EP93xx
pinctrl: add a Cirrus ep93xx SoC pin controller
power: reset: Add a driver for the ep93xx reset
dt-bindings: soc: Add Cirrus EP93xx
soc: Add SoC driver for Cirrus ep93xx
dt-bindings: dma: Add Cirrus EP93xx
dma: cirrus: Convert to DT for Cirrus EP93xx
dt-bindings: watchdog: Add Cirrus EP93x
watchdog: ep93xx: add DT support for Cirrus EP93xx
dt-bindings: pwm: Add Cirrus EP93xx
pwm: ep93xx: add DT support for Cirrus EP93xx
dt-bindings: spi: Add Cirrus EP93xx
spi: ep93xx: add DT support for Cirrus EP93xx
dt-bindings: net: Add Cirrus EP93xx
net: cirrus: add DT support for Cirrus EP93xx
dt-bindings: mtd: Add ts7200 nand-controller
mtd: rawnand: add support for ts72xx
dt-bindings: ata: Add Cirrus EP93xx
ata: pata_ep93xx: add device tree support
dt-bindings: input: Add Cirrus EP93xx keypad
input: keypad: ep93xx: add DT support for Cirrus EP93xx
wdt: ts72xx: add DT support for ts72xx
gpio: ep93xx: add DT support for gpio-ep93xx
ASoC: dt-bindings: ep93xx: Document DMA support
ASoC: dt-bindings: ep93xx: Document Audio Port support
ARM: dts: add Cirrus EP93XX SoC .dtsi
ARM: dts: ep93xx: add ts7250 board
ARM: ep93xx: DT for the Cirrus ep93xx SoC platforms
pwm: ep93xx: drop legacy pinctrl
ata: pata_ep93xx: remove legacy pinctrl use
ARM: ep93xx: delete all boardfiles
ARM: ep93xx: soc: drop defines
dma: cirrus: remove platform code
.../bindings/arm/cirrus/cirrus,ep9301.yaml | 38 +
.../bindings/ata/cirrus,ep9312-pata.yaml | 42 +
.../bindings/dma/cirrus,ep9301-dma-m2m.yaml | 84 ++
.../bindings/dma/cirrus,ep9301-dma-m2p.yaml | 144 ++
.../bindings/input/cirrus,ep9307-keypad.yaml | 87 ++
.../devicetree/bindings/mtd/technologic,nand.yaml | 45 +
.../devicetree/bindings/net/cirrus,ep9301-eth.yaml | 59 +
.../devicetree/bindings/pwm/cirrus,ep9301-pwm.yaml | 53 +
.../bindings/soc/cirrus/cirrus,ep9301-syscon.yaml | 94 ++
.../bindings/sound/cirrus,ep9301-i2s.yaml | 16 +
.../devicetree/bindings/spi/cirrus,ep9301-spi.yaml | 70 +
.../bindings/watchdog/cirrus,ep9301-wdt.yaml | 42 +
arch/arm/Makefile | 1 -
arch/arm/boot/dts/cirrus/Makefile | 4 +
arch/arm/boot/dts/cirrus/ep93xx-bk3.dts | 126 ++
arch/arm/boot/dts/cirrus/ep93xx-edb9302.dts | 182 +++
arch/arm/boot/dts/cirrus/ep93xx-ts7250.dts | 145 ++
arch/arm/boot/dts/cirrus/ep93xx.dtsi | 441 ++++++
arch/arm/mach-ep93xx/Kconfig | 20 +-
arch/arm/mach-ep93xx/Makefile | 11 -
arch/arm/mach-ep93xx/clock.c | 733 ----------
arch/arm/mach-ep93xx/core.c | 1017 --------------
arch/arm/mach-ep93xx/dma.c | 114 --
arch/arm/mach-ep93xx/edb93xx.c | 368 -----
arch/arm/mach-ep93xx/ep93xx-regs.h | 38 -
arch/arm/mach-ep93xx/gpio-ep93xx.h | 111 --
arch/arm/mach-ep93xx/hardware.h | 25 -
arch/arm/mach-ep93xx/irqs.h | 76 --
arch/arm/mach-ep93xx/platform.h | 42 -
arch/arm/mach-ep93xx/soc.h | 212 ---
arch/arm/mach-ep93xx/timer-ep93xx.c | 143 --
arch/arm/mach-ep93xx/ts72xx.c | 422 ------
arch/arm/mach-ep93xx/ts72xx.h | 94 --
arch/arm/mach-ep93xx/vision_ep9307.c | 321 -----
drivers/ata/pata_ep93xx.c | 106 +-
drivers/clk/Kconfig | 8 +
drivers/clk/Makefile | 1 +
drivers/clk/clk-ep93xx.c | 840 ++++++++++++
drivers/dma/ep93xx_dma.c | 287 +++-
drivers/gpio/gpio-ep93xx.c | 345 ++---
drivers/input/keyboard/ep93xx_keypad.c | 74 +-
drivers/mtd/nand/raw/Kconfig | 6 +
drivers/mtd/nand/raw/Makefile | 1 +
drivers/mtd/nand/raw/technologic-nand-controller.c | 220 +++
drivers/net/ethernet/cirrus/ep93xx_eth.c | 63 +-
drivers/pinctrl/Kconfig | 7 +
drivers/pinctrl/Makefile | 1 +
drivers/pinctrl/pinctrl-ep93xx.c | 1434 ++++++++++++++++++++
drivers/power/reset/Kconfig | 10 +
drivers/power/reset/Makefile | 1 +
drivers/power/reset/ep93xx-restart.c | 84 ++
drivers/pwm/pwm-ep93xx.c | 26 +-
drivers/soc/Kconfig | 1 +
drivers/soc/Makefile | 1 +
drivers/soc/cirrus/Kconfig | 13 +
drivers/soc/cirrus/Makefile | 2 +
drivers/soc/cirrus/soc-ep93xx.c | 241 ++++
drivers/spi/spi-ep93xx.c | 66 +-
drivers/watchdog/ep93xx_wdt.c | 8 +
drivers/watchdog/ts72xx_wdt.c | 8 +
include/dt-bindings/clock/cirrus,ep9301-syscon.h | 46 +
include/linux/platform_data/dma-ep93xx.h | 94 --
include/linux/platform_data/eth-ep93xx.h | 10 -
include/linux/platform_data/keypad-ep93xx.h | 32 -
include/linux/platform_data/spi-ep93xx.h | 15 -
include/linux/soc/cirrus/ep93xx.h | 47 +-
sound/soc/cirrus/Kconfig | 9 -
sound/soc/cirrus/Makefile | 4 -
sound/soc/cirrus/edb93xx.c | 116 --
sound/soc/cirrus/ep93xx-i2s.c | 19 -
sound/soc/cirrus/ep93xx-pcm.c | 19 +-
71 files changed, 5137 insertions(+), 4548 deletions(-)
---
base-commit: 052d534373b7ed33712a63d5e17b2b6cdbce84fd
change-id: 20230605-ep93xx-01c76317e2d2
Best regards,
--
Nikita Shubin <nikita.shubin@maquefel.me>
^ 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