* [PATCH v6 5/5] MAINTAINERS: add myself for Marvell 88PM886 PMIC
From: Karel Balej @ 2024-05-04 19:37 UTC (permalink / raw)
To: Lee Jones, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Dmitry Torokhov, Liam Girdwood, Mark Brown, devicetree,
linux-kernel, linux-input
Cc: Duje Mihanović, ~postmarketos/upstreaming, phone-devel,
balejk
In-Reply-To: <20240504194632.2456-1-balejk@matfyz.cz>
Add an entry to MAINTAINERS for the Marvell 88PM886 PMIC MFD, onkey and
regulator drivers.
Signed-off-by: Karel Balej <balejk@matfyz.cz>
---
Notes:
RFC v3:
- Remove onkey bindings file.
RFC v2:
- Only mention 88PM886 in the commit message.
- Add regulator driver.
- Rename the entry.
MAINTAINERS | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index f6dc90559341..e1a0e02e098d 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -13044,6 +13044,15 @@ F: drivers/net/dsa/mv88e6xxx/
F: include/linux/dsa/mv88e6xxx.h
F: include/linux/platform_data/mv88e6xxx.h
+MARVELL 88PM886 PMIC DRIVER
+M: Karel Balej <balejk@matfyz.cz>
+S: Maintained
+F: Documentation/devicetree/bindings/mfd/marvell,88pm886-a1.yaml
+F: drivers/input/misc/88pm886-onkey.c
+F: drivers/mfd/88pm886.c
+F: drivers/regulators/88pm886-regulator.c
+F: include/linux/mfd/88pm886.h
+
MARVELL ARMADA 3700 PHY DRIVERS
M: Miquel Raynal <miquel.raynal@bootlin.com>
S: Maintained
--
2.45.0
^ permalink raw reply related
* [PATCH v6 0/5] initial support for Marvell 88PM886 PMIC
From: Karel Balej @ 2024-05-04 19:37 UTC (permalink / raw)
To: Lee Jones, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Dmitry Torokhov, Liam Girdwood, Mark Brown, devicetree,
linux-kernel, linux-input
Cc: Duje Mihanović, ~postmarketos/upstreaming, phone-devel,
balejk
Hello,
the following implements basic support for Marvell's 88PM886 PMIC which
is found for instance as a component of the samsung,coreprimevelte
smartphone which inspired this and also serves as a testing platform.
The code for the MFD is based primarily on this old series [1] with the
addition of poweroff based on the smartphone's downstream kernel tree
[2]. The onkey and regulators drivers are based on the latter. I am not
in possesion of the datasheet.
[1] https://lore.kernel.org/all/1434098601-3498-1-git-send-email-yizhang@marvell.com/
[2] https://github.com/CoderCharmander/g361f-kernel
Thank you and kind regards,
K. B.
---
v6:
- Rebase to v6.9-rc6.
- Fix patchset versioning: the previous version was marked as v1 because I
considered RFC to be its own thing. Thank you to Krzysztof for
explaining that that is not the case. The previous version is thus now
marked as v5 and this is v6, sorry for any confusion.
- v5: https://lore.kernel.org/r/20240331105608.7338-2-balejk@matfyz.cz/
v5:
- RFC v4: https://lore.kernel.org/r/20240311160110.32185-1-karelb@gimli.ms.mff.cuni.cz/
- Rebase to v6.9-rc1.
- Thank you to everybody for their feedback on the RFC!
RFC v4:
- RFC v3: https://lore.kernel.org/all/20240303101506.4187-1-karelb@gimli.ms.mff.cuni.cz/
RFC v3:
- Address Rob's feedback:
- Drop onkey bindings patch.
- Rename PM88X -> PM886 everywhere.
- RFC v2: https://lore.kernel.org/all/20240211094609.2223-1-karelb@gimli.ms.mff.cuni.cz/
RFC v2:
- Merge with the regulators series to have multiple devices and thus
justify the use of the MFD framework.
- Rebase on v6.8-rc3.
- Reorder patches.
- MFD RFC v1: https://lore.kernel.org/all/20231217131838.7569-1-karelb@gimli.ms.mff.cuni.cz/
- regulators RFC v1: https://lore.kernel.org/all/20231228100208.2932-1-karelb@gimli.ms.mff.cuni.cz/
Karel Balej (5):
dt-bindings: mfd: add entry for Marvell 88PM886 PMIC
mfd: add driver for Marvell 88PM886 PMIC
regulator: add regulators driver for Marvell 88PM886 PMIC
input: add onkey driver for Marvell 88PM886 PMIC
MAINTAINERS: add myself for Marvell 88PM886 PMIC
.../bindings/mfd/marvell,88pm886-a1.yaml | 76 +++
MAINTAINERS | 9 +
drivers/input/misc/88pm886-onkey.c | 98 ++++
drivers/input/misc/Kconfig | 7 +
drivers/input/misc/Makefile | 1 +
drivers/mfd/88pm886.c | 148 ++++++
drivers/mfd/Kconfig | 12 +
drivers/mfd/Makefile | 1 +
drivers/regulator/88pm886-regulator.c | 476 ++++++++++++++++++
drivers/regulator/Kconfig | 6 +
drivers/regulator/Makefile | 1 +
include/linux/mfd/88pm886.h | 69 +++
12 files changed, 904 insertions(+)
create mode 100644 Documentation/devicetree/bindings/mfd/marvell,88pm886-a1.yaml
create mode 100644 drivers/input/misc/88pm886-onkey.c
create mode 100644 drivers/mfd/88pm886.c
create mode 100644 drivers/regulator/88pm886-regulator.c
create mode 100644 include/linux/mfd/88pm886.h
--
2.45.0
^ permalink raw reply
* [PATCH v6 4/5] input: add onkey driver for Marvell 88PM886 PMIC
From: Karel Balej @ 2024-05-04 19:37 UTC (permalink / raw)
To: Lee Jones, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Dmitry Torokhov, Liam Girdwood, Mark Brown, devicetree,
linux-kernel, linux-input
Cc: Duje Mihanović, ~postmarketos/upstreaming, phone-devel,
balejk
In-Reply-To: <20240504194632.2456-1-balejk@matfyz.cz>
Marvell 88PM886 PMIC provides onkey among other things. Add client
driver to handle it. The driver currently only provides a basic support
omitting additional functions found in the vendor version, such as long
onkey and GPIO integration.
Acked-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Signed-off-by: Karel Balej <balejk@matfyz.cz>
---
Notes:
v5:
- Remove kernel.h include.
RFC v4:
- Reflect MFD driver changes:
- chip->regmaps[...] -> chip->regmap
- Address Dmitry's feedback:
- Add ID table.
- Add Ack.
RFC v3:
- Drop wakeup-source.
RFC v2:
- Address Dmitry's feedback:
- Sort includes alphabetically.
- Drop onkey->irq.
- ret -> err in irq_handler and no initialization.
- Break long lines and other formatting.
- Do not clobber platform_get_irq error.
- Do not set device parent manually.
- Use input_set_capability.
- Use the wakeup-source DT property.
- Drop of_match_table.
- Use more temporaries.
- Use dev_err_probe.
- Modify Kconfig description.
drivers/input/misc/88pm886-onkey.c | 98 ++++++++++++++++++++++++++++++
drivers/input/misc/Kconfig | 7 +++
drivers/input/misc/Makefile | 1 +
3 files changed, 106 insertions(+)
create mode 100644 drivers/input/misc/88pm886-onkey.c
diff --git a/drivers/input/misc/88pm886-onkey.c b/drivers/input/misc/88pm886-onkey.c
new file mode 100644
index 000000000000..284ff5190b6e
--- /dev/null
+++ b/drivers/input/misc/88pm886-onkey.c
@@ -0,0 +1,98 @@
+// SPDX-License-Identifier: GPL-2.0-only
+#include <linux/input.h>
+#include <linux/interrupt.h>
+#include <linux/irq.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+
+#include <linux/mfd/88pm886.h>
+
+struct pm886_onkey {
+ struct input_dev *idev;
+ struct pm886_chip *chip;
+};
+
+static irqreturn_t pm886_onkey_irq_handler(int irq, void *data)
+{
+ struct pm886_onkey *onkey = data;
+ struct regmap *regmap = onkey->chip->regmap;
+ struct input_dev *idev = onkey->idev;
+ struct device *parent = idev->dev.parent;
+ unsigned int val;
+ int err;
+
+ err = regmap_read(regmap, PM886_REG_STATUS1, &val);
+ if (err) {
+ dev_err(parent, "Failed to read status: %d\n", err);
+ return IRQ_NONE;
+ }
+ val &= PM886_ONKEY_STS1;
+
+ input_report_key(idev, KEY_POWER, val);
+ input_sync(idev);
+
+ return IRQ_HANDLED;
+}
+
+static int pm886_onkey_probe(struct platform_device *pdev)
+{
+ struct pm886_chip *chip = dev_get_drvdata(pdev->dev.parent);
+ struct device *dev = &pdev->dev;
+ struct pm886_onkey *onkey;
+ struct input_dev *idev;
+ int irq, err;
+
+ onkey = devm_kzalloc(dev, sizeof(*onkey), GFP_KERNEL);
+ if (!onkey)
+ return -ENOMEM;
+
+ onkey->chip = chip;
+
+ irq = platform_get_irq(pdev, 0);
+ if (irq < 0)
+ return dev_err_probe(dev, irq, "Failed to get IRQ\n");
+
+ idev = devm_input_allocate_device(dev);
+ if (!idev) {
+ dev_err(dev, "Failed to allocate input device\n");
+ return -ENOMEM;
+ }
+ onkey->idev = idev;
+
+ idev->name = "88pm886-onkey";
+ idev->phys = "88pm886-onkey/input0";
+ idev->id.bustype = BUS_I2C;
+
+ input_set_capability(idev, EV_KEY, KEY_POWER);
+
+ err = devm_request_threaded_irq(dev, irq, NULL, pm886_onkey_irq_handler,
+ IRQF_ONESHOT | IRQF_NO_SUSPEND, "onkey",
+ onkey);
+ if (err)
+ return dev_err_probe(dev, err, "Failed to request IRQ\n");
+
+ err = input_register_device(idev);
+ if (err)
+ return dev_err_probe(dev, err, "Failed to register input device\n");
+
+ return 0;
+}
+
+static const struct platform_device_id pm886_onkey_id_table[] = {
+ { "88pm886-onkey", },
+ { }
+};
+MODULE_DEVICE_TABLE(platform, pm886_onkey_id_table);
+
+static struct platform_driver pm886_onkey_driver = {
+ .driver = {
+ .name = "88pm886-onkey",
+ },
+ .probe = pm886_onkey_probe,
+ .id_table = pm886_onkey_id_table,
+};
+module_platform_driver(pm886_onkey_driver);
+
+MODULE_DESCRIPTION("Marvell 88PM886 onkey driver");
+MODULE_AUTHOR("Karel Balej <balejk@matfyz.cz>");
+MODULE_LICENSE("GPL");
diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
index 6ba984d7f0b1..16a079d9f0f2 100644
--- a/drivers/input/misc/Kconfig
+++ b/drivers/input/misc/Kconfig
@@ -33,6 +33,13 @@ config INPUT_88PM80X_ONKEY
To compile this driver as a module, choose M here: the module
will be called 88pm80x_onkey.
+config INPUT_88PM886_ONKEY
+ tristate "Marvell 88PM886 onkey support"
+ depends on MFD_88PM886_PMIC
+ help
+ Support the onkey of Marvell 88PM886 PMIC as an input device
+ reporting power button status.
+
config INPUT_AB8500_PONKEY
tristate "AB8500 Pon (PowerOn) Key"
depends on AB8500_CORE
diff --git a/drivers/input/misc/Makefile b/drivers/input/misc/Makefile
index 04296a4abe8e..054a6dc1ac27 100644
--- a/drivers/input/misc/Makefile
+++ b/drivers/input/misc/Makefile
@@ -7,6 +7,7 @@
obj-$(CONFIG_INPUT_88PM860X_ONKEY) += 88pm860x_onkey.o
obj-$(CONFIG_INPUT_88PM80X_ONKEY) += 88pm80x_onkey.o
+obj-$(CONFIG_INPUT_88PM886_ONKEY) += 88pm886-onkey.o
obj-$(CONFIG_INPUT_AB8500_PONKEY) += ab8500-ponkey.o
obj-$(CONFIG_INPUT_AD714X) += ad714x.o
obj-$(CONFIG_INPUT_AD714X_I2C) += ad714x-i2c.o
--
2.45.0
^ permalink raw reply related
* [PATCH v2] HID: playstation: DS4: Fix calibration workaround for clone devices
From: Max Staudt @ 2024-05-04 16:55 UTC (permalink / raw)
To: Roderick Colenbrander, Jiri Kosina, Benjamin Tissoires
Cc: linux-input, linux-kernel, max
The logic in dualshock4_get_calibration_data() used uninitialised data
in case of a failed kzalloc() for the transfer buffer.
The solution is to group all business logic and all sanity checks
together, and jump only to the latter in case of an error.
While we're at it, factor out the axes' labelling, since it must happen
either way for input_report_abs() to succeed later on.
Thanks to Dan Carpenter for the Smatch static checker warning.
Fixes: a48a7cd85f55 ("HID: playstation: DS4: Don't fail on calibration data request")
Signed-off-by: Max Staudt <max@enpas.org>
---
Changes in v1 -> v2:
- Restored assignments to .abs_code in their original location
- Added assignments to .abs_code in the error handling loops
---
drivers/hid/hid-playstation.c | 52 +++++++++++++++++++----------------
1 file changed, 28 insertions(+), 24 deletions(-)
diff --git a/drivers/hid/hid-playstation.c b/drivers/hid/hid-playstation.c
index edc46fc02e9a..e7c309cfe3a0 100644
--- a/drivers/hid/hid-playstation.c
+++ b/drivers/hid/hid-playstation.c
@@ -1787,7 +1787,7 @@ static int dualshock4_get_calibration_data(struct dualshock4 *ds4)
buf = kzalloc(DS4_FEATURE_REPORT_CALIBRATION_SIZE, GFP_KERNEL);
if (!buf) {
ret = -ENOMEM;
- goto no_buffer_tail_check;
+ goto transfer_failed;
}
/* We should normally receive the feature report data we asked
@@ -1807,6 +1807,7 @@ static int dualshock4_get_calibration_data(struct dualshock4 *ds4)
hid_warn(hdev, "Failed to retrieve DualShock4 calibration info: %d\n", ret);
ret = -EILSEQ;
+ goto transfer_failed;
} else {
break;
}
@@ -1815,17 +1816,19 @@ static int dualshock4_get_calibration_data(struct dualshock4 *ds4)
buf = kzalloc(DS4_FEATURE_REPORT_CALIBRATION_BT_SIZE, GFP_KERNEL);
if (!buf) {
ret = -ENOMEM;
- goto no_buffer_tail_check;
+ goto transfer_failed;
}
ret = ps_get_report(hdev, DS4_FEATURE_REPORT_CALIBRATION_BT, buf,
DS4_FEATURE_REPORT_CALIBRATION_BT_SIZE, true);
- if (ret)
+ if (ret) {
hid_warn(hdev, "Failed to retrieve DualShock4 calibration info: %d\n", ret);
+ goto transfer_failed;
+ }
}
- /* Parse buffer. If the transfer failed, this safely copies zeroes. */
+ /* Transfer succeeded - parse the calibration data received. */
gyro_pitch_bias = get_unaligned_le16(&buf[1]);
gyro_yaw_bias = get_unaligned_le16(&buf[3]);
gyro_roll_bias = get_unaligned_le16(&buf[5]);
@@ -1854,6 +1857,9 @@ static int dualshock4_get_calibration_data(struct dualshock4 *ds4)
acc_z_plus = get_unaligned_le16(&buf[31]);
acc_z_minus = get_unaligned_le16(&buf[33]);
+ /* Done parsing the buffer, so let's free it. */
+ kfree(buf);
+
/*
* Set gyroscope calibration and normalization parameters.
* Data values will be normalized to 1/DS4_GYRO_RES_PER_DEG_S degree/s.
@@ -1877,26 +1883,6 @@ static int dualshock4_get_calibration_data(struct dualshock4 *ds4)
ds4->gyro_calib_data[2].sens_denom = abs(gyro_roll_plus - gyro_roll_bias) +
abs(gyro_roll_minus - gyro_roll_bias);
- /* Done parsing the buffer, so let's free it. */
- kfree(buf);
-
-no_buffer_tail_check:
-
- /*
- * Sanity check gyro calibration data. This is needed to prevent crashes
- * during report handling of virtual, clone or broken devices not implementing
- * calibration data properly.
- */
- for (i = 0; i < ARRAY_SIZE(ds4->gyro_calib_data); i++) {
- if (ds4->gyro_calib_data[i].sens_denom == 0) {
- hid_warn(hdev, "Invalid gyro calibration data for axis (%d), disabling calibration.",
- ds4->gyro_calib_data[i].abs_code);
- ds4->gyro_calib_data[i].bias = 0;
- ds4->gyro_calib_data[i].sens_numer = DS4_GYRO_RANGE;
- ds4->gyro_calib_data[i].sens_denom = S16_MAX;
- }
- }
-
/*
* Set accelerometer calibration and normalization parameters.
* Data values will be normalized to 1/DS4_ACC_RES_PER_G g.
@@ -1919,6 +1905,23 @@ static int dualshock4_get_calibration_data(struct dualshock4 *ds4)
ds4->accel_calib_data[2].sens_numer = 2*DS4_ACC_RES_PER_G;
ds4->accel_calib_data[2].sens_denom = range_2g;
+transfer_failed:
+ /*
+ * Sanity check gyro calibration data. This is needed to prevent crashes
+ * during report handling of virtual, clone or broken devices not implementing
+ * calibration data properly.
+ */
+ for (i = 0; i < ARRAY_SIZE(ds4->gyro_calib_data); i++) {
+ if (ds4->gyro_calib_data[i].sens_denom == 0) {
+ ds4->gyro_calib_data[i].abs_code = ABS_RX + i;
+ hid_warn(hdev, "Invalid gyro calibration data for axis (%d), disabling calibration.",
+ ds4->gyro_calib_data[i].abs_code);
+ ds4->gyro_calib_data[i].bias = 0;
+ ds4->gyro_calib_data[i].sens_numer = DS4_GYRO_RANGE;
+ ds4->gyro_calib_data[i].sens_denom = S16_MAX;
+ }
+ }
+
/*
* Sanity check accelerometer calibration data. This is needed to prevent crashes
* during report handling of virtual, clone or broken devices not implementing calibration
@@ -1926,6 +1929,7 @@ static int dualshock4_get_calibration_data(struct dualshock4 *ds4)
*/
for (i = 0; i < ARRAY_SIZE(ds4->accel_calib_data); i++) {
if (ds4->accel_calib_data[i].sens_denom == 0) {
+ ds4->accel_calib_data[i].abs_code = ABS_X + i;
hid_warn(hdev, "Invalid accelerometer calibration data for axis (%d), disabling calibration.",
ds4->accel_calib_data[i].abs_code);
ds4->accel_calib_data[i].bias = 0;
--
2.39.2
^ permalink raw reply related
* Re: [PATCH v3 03/11] tty: serdev: Add flag buffer aware receive_buf_fp()
From: Greg Kroah-Hartman @ 2024-05-04 16:00 UTC (permalink / raw)
To: Christoph Fritz
Cc: Jiri Slaby, Oliver Hartkopp, Marc Kleine-Budde, Vincent Mailhol,
David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Jiri Kosina,
Benjamin Tissoires, Sebastian Reichel, Linus Walleij,
Andreas Lauser, Jonathan Corbet, Pavel Pisa, linux-can, netdev,
devicetree, linux-input, linux-serial
In-Reply-To: <20240502182804.145926-4-christoph.fritz@hexdev.de>
On Thu, May 02, 2024 at 08:27:56PM +0200, Christoph Fritz wrote:
> This patch introduces an additional receive buffer callback variation
> besides the already existing receive_buf(). This new callback function
> also passes the flag buffer (TTY_NORMAL, TTY_BREAK, and friends).
>
> If defined, this function gets prioritized and called instead of the
> standard receive_buf().
>
> An alternative approach could have been to enhance the receive_buf()
> function and update all drivers that use it.
Please, let's do that instead of adding random letters at the end of a
function pointer :)
> Signed-off-by: Christoph Fritz <christoph.fritz@hexdev.de>
> ---
> drivers/tty/serdev/serdev-ttyport.c | 2 +-
> include/linux/serdev.h | 17 ++++++++++++++---
> 2 files changed, 15 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/tty/serdev/serdev-ttyport.c b/drivers/tty/serdev/serdev-ttyport.c
> index 3d7ae7fa50186..bb47691afdb21 100644
> --- a/drivers/tty/serdev/serdev-ttyport.c
> +++ b/drivers/tty/serdev/serdev-ttyport.c
> @@ -32,7 +32,7 @@ static size_t ttyport_receive_buf(struct tty_port *port, const u8 *cp,
> if (!test_bit(SERPORT_ACTIVE, &serport->flags))
> return 0;
>
> - ret = serdev_controller_receive_buf(ctrl, cp, count);
> + ret = serdev_controller_receive_buf(ctrl, cp, fp, count);
>
> dev_WARN_ONCE(&ctrl->dev, ret > count,
> "receive_buf returns %zu (count = %zu)\n",
> diff --git a/include/linux/serdev.h b/include/linux/serdev.h
> index ff78efc1f60df..c6ef5a8988e07 100644
> --- a/include/linux/serdev.h
> +++ b/include/linux/serdev.h
> @@ -23,11 +23,17 @@ struct serdev_device;
> * struct serdev_device_ops - Callback operations for a serdev device
> * @receive_buf: Function called with data received from device;
> * returns number of bytes accepted; may sleep.
> + * @receive_buf_fp: Function called with data and flag buffer received
> + * from device; If defined, this function gets called
> + * instead of @receive_buf;
> + * returns number of bytes accepted; may sleep.
I don't remember waht "fp" means here, and you don't document it, so
let's just have one recieve_buf() callback please.
thanks,
greg k-h
^ permalink raw reply
* [PATCH v3] HID: logitech-dj: Add support for a new lightspeed receiver iteration
From: Mavroudis Chatzilazaridis @ 2024-05-04 14:40 UTC (permalink / raw)
To: jikos; +Cc: linux-input, benjamin.tissoires, hadess, lains, mavchatz
The lightspeed receiver for the Pro X Superlight uses 13 byte mouse reports
without a report id. The workaround for such cases has been adjusted to
handle these larger packets.
The device now reports the status of its battery in wireless mode and
libratbag now recognizes the device and it can be configured with Piper.
This receiver has new descriptors, which have been added. The mouse
descriptor has 5 extra vendor defined bytes at the end, while the keyboard
one has a different report layout and minimums/maximums. As such, mice
with key press macros and keyboards that use this receiver misbehave
without them.
Fixes: 9d1bd9346241 (HID: logitech-dj: Add support for a new lightspeed receiver iteration)
Link: https://bugzilla.kernel.org/show_bug.cgi?id=218172
Link: https://bugzilla.kernel.org/show_bug.cgi?id=218094
Link: https://lore.kernel.org/r/CAOEevLOrTSugnLePJwpcqx2_AacNbBxFZDTqp0Qm_jjVpWKgFQ@mail.gmail.com/
Link: https://lore.kernel.org/r/6929ebbf-f2e0-4cd4-addc-1e33ecf3277f@gmail.com/
Co-developed-by: Filipe Laíns <lains@riseup.net>
Signed-off-by: Filipe Laíns <lains@riseup.net>
Signed-off-by: Mavroudis Chatzilazaridis <mavchatz@protonmail.com>
---
V2 -> V3: Fixed regression which broke keyboard related input
V1 -> V2: Addressed review comment for commit message
drivers/hid/hid-ids.h | 1 +
drivers/hid/hid-logitech-dj.c | 105 ++++++++++++++++++++++++++++++++--
2 files changed, 102 insertions(+), 4 deletions(-)
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index 8376fb5e2d0b..258fa27746c8 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -873,6 +873,7 @@
#define USB_DEVICE_ID_LOGITECH_NANO_RECEIVER_2 0xc534
#define USB_DEVICE_ID_LOGITECH_NANO_RECEIVER_LIGHTSPEED_1 0xc539
#define USB_DEVICE_ID_LOGITECH_NANO_RECEIVER_LIGHTSPEED_1_1 0xc53f
+#define USB_DEVICE_ID_LOGITECH_NANO_RECEIVER_LIGHTSPEED_1_2 0xc547
#define USB_DEVICE_ID_LOGITECH_NANO_RECEIVER_POWERPLAY 0xc53a
#define USB_DEVICE_ID_SPACETRAVELLER 0xc623
#define USB_DEVICE_ID_SPACENAVIGATOR 0xc626
diff --git a/drivers/hid/hid-logitech-dj.c b/drivers/hid/hid-logitech-dj.c
index e6a8b6d8eab7..f25e076405e0 100644
--- a/drivers/hid/hid-logitech-dj.c
+++ b/drivers/hid/hid-logitech-dj.c
@@ -116,6 +116,7 @@ enum recvr_type {
recvr_type_dj,
recvr_type_hidpp,
recvr_type_gaming_hidpp,
+ recvr_type_gaming_hidpp_ls_1_2,
recvr_type_mouse_only,
recvr_type_27mhz,
recvr_type_bluetooth,
@@ -211,6 +212,45 @@ static const char kbd_descriptor[] = {
0xC0
};
+/* Gaming Keyboard descriptor (1) */
+static const char gaming_kbd_descriptor[] = {
+ 0x05, 0x01, /* Usage Page (Generic Desktop) */
+ 0x09, 0x06, /* Usage (Keyboard) */
+ 0xA1, 0x01, /* Collection (Application) */
+ 0x85, 0x01, /* Report ID (1) */
+ 0x05, 0x07, /* Usage Page (Kbrd/Keypad) */
+ 0x19, 0xE0, /* Usage Minimum (0xE0) */
+ 0x29, 0xE7, /* Usage Maximum (0xE7) */
+ 0x15, 0x00, /* Logical Minimum (0) */
+ 0x25, 0x01, /* Logical Maximum (1) */
+ 0x75, 0x01, /* Report Size (1) */
+ 0x95, 0x08, /* Report Count (8) */
+ 0x81, 0x02, /* Input (Data,Var) */
+ 0x95, 0x05, /* Report Count (5) */
+ 0x05, 0x08, /* Usage Page (LEDs) */
+ 0x19, 0x01, /* Usage Minimum (Num Lock) */
+ 0x29, 0x05, /* Usage Maximum (Kana) */
+ 0x91, 0x02, /* Output (Data,Var,Abs) */
+ 0x95, 0x01, /* Report Count (1) */
+ 0x75, 0x03, /* Report Size (3) */
+ 0x91, 0x03, /* Output (Const,Var,Abs) */
+ 0x95, 0x70, /* Report Count (112) */
+ 0x75, 0x01, /* Report Size (1) */
+ 0x05, 0x07, /* Usage Page (Kbrd/Keypad) */
+ 0x19, 0x04, /* Usage Minimum (0x04) */
+ 0x29, 0x73, /* Usage Maximum (0x73) */
+ 0x81, 0x02, /* Input (Data,Var,Abs) */
+ 0x95, 0x05, /* Report Count (5) */
+ 0x19, 0x87, /* Usage Minimum (0x87) */
+ 0x29, 0x8B, /* Usage Maximum (0x8B) */
+ 0x81, 0x02, /* Input (Data,Var,Abs) */
+ 0x95, 0x03, /* Report Count (3) */
+ 0x19, 0x90, /* Usage Minimum (0x90) */
+ 0x29, 0x92, /* Usage Maximum (0x92) */
+ 0x81, 0x02, /* Input (Data,Var,Abs) */
+ 0xC0, /* End Collection */
+};
+
/* Mouse descriptor (2) */
static const char mse_descriptor[] = {
0x05, 0x01, /* USAGE_PAGE (Generic Desktop) */
@@ -415,6 +455,51 @@ static const char mse_high_res_descriptor[] = {
0xC0, /* END_COLLECTION */
};
+/* Gaming Mouse descriptor with vendor data (2) */
+static const char mse_high_res_ls_1_2_descriptor[] = {
+ 0x05, 0x01, /* Usage Page (Generic Desktop) */
+ 0x09, 0x02, /* Usage (Mouse) */
+ 0xA1, 0x01, /* Collection (Application) */
+ 0x85, 0x02, /* Report ID (2) */
+ 0x09, 0x01, /* Usage (Pointer) */
+ 0xA1, 0x00, /* Collection (Physical) */
+ 0x95, 0x10, /* Report Count (16) */
+ 0x75, 0x01, /* Report Size (1) */
+ 0x15, 0x00, /* Logical Minimum (0) */
+ 0x25, 0x01, /* Logical Maximum (1) */
+ 0x05, 0x09, /* Usage Page (Button) */
+ 0x19, 0x01, /* Usage Minimum (0x01) */
+ 0x29, 0x10, /* Usage Maximum (0x10) */
+ 0x81, 0x02, /* Input (Data,Var,Abs) */
+ 0x95, 0x02, /* Report Count (2) */
+ 0x75, 0x10, /* Report Size (16) */
+ 0x16, 0x01, 0x80, /* Logical Minimum (-32767) */
+ 0x26, 0xFF, 0x7F, /* Logical Maximum (32767) */
+ 0x05, 0x01, /* Usage Page (Generic Desktop) */
+ 0x09, 0x30, /* Usage (X) */
+ 0x09, 0x31, /* Usage (Y) */
+ 0x81, 0x06, /* Input (Data,Var,Rel) */
+ 0x95, 0x01, /* Report Count (1) */
+ 0x75, 0x08, /* Report Size (8) */
+ 0x15, 0x81, /* Logical Minimum (-127) */
+ 0x25, 0x7F, /* Logical Maximum (127) */
+ 0x09, 0x38, /* Usage (Wheel) */
+ 0x81, 0x06, /* Input (Data,Var,Rel) */
+ 0x95, 0x01, /* Report Count (1) */
+ 0x05, 0x0C, /* Usage Page (Consumer) */
+ 0x0A, 0x38, 0x02, /* Usage (AC Pan) */
+ 0x81, 0x06, /* Input (Data,Var,Rel) */
+ 0xC0, /* End Collection */
+ 0x06, 0x00, 0xFF, /* Usage Page (Vendor Defined 0xFF00) */
+ 0x09, 0xF1, /* Usage (0xF1) */
+ 0x75, 0x08, /* Report Size (8) */
+ 0x95, 0x05, /* Report Count (5) */
+ 0x15, 0x00, /* Logical Minimum (0) */
+ 0x26, 0xFF, 0x00, /* Logical Maximum (255) */
+ 0x81, 0x00, /* Input (Data,Array,Abs) */
+ 0xC0, /* End Collection */
+};
+
/* Consumer Control descriptor (3) */
static const char consumer_descriptor[] = {
0x05, 0x0C, /* USAGE_PAGE (Consumer Devices) */
@@ -1426,7 +1511,10 @@ static int logi_dj_ll_parse(struct hid_device *hid)
if (djdev->reports_supported & STD_KEYBOARD) {
dbg_hid("%s: sending a kbd descriptor, reports_supported: %llx\n",
__func__, djdev->reports_supported);
- rdcat(rdesc, &rsize, kbd_descriptor, sizeof(kbd_descriptor));
+ if (djdev->dj_receiver_dev->type == recvr_type_gaming_hidpp_ls_1_2)
+ rdcat(rdesc, &rsize, gaming_kbd_descriptor, sizeof(gaming_kbd_descriptor));
+ else
+ rdcat(rdesc, &rsize, kbd_descriptor, sizeof(kbd_descriptor));
}
if (djdev->reports_supported & STD_MOUSE) {
@@ -1436,6 +1524,9 @@ static int logi_dj_ll_parse(struct hid_device *hid)
djdev->dj_receiver_dev->type == recvr_type_mouse_only)
rdcat(rdesc, &rsize, mse_high_res_descriptor,
sizeof(mse_high_res_descriptor));
+ else if (djdev->dj_receiver_dev->type == recvr_type_gaming_hidpp_ls_1_2)
+ rdcat(rdesc, &rsize, mse_high_res_ls_1_2_descriptor,
+ sizeof(mse_high_res_ls_1_2_descriptor));
else if (djdev->dj_receiver_dev->type == recvr_type_27mhz)
rdcat(rdesc, &rsize, mse_27mhz_descriptor,
sizeof(mse_27mhz_descriptor));
@@ -1695,11 +1786,12 @@ static int logi_dj_raw_event(struct hid_device *hdev,
}
/*
* Mouse-only receivers send unnumbered mouse data. The 27 MHz
- * receiver uses 6 byte packets, the nano receiver 8 bytes.
+ * receiver uses 6 byte packets, the nano receiver 8 bytes,
+ * the lightspeed receiver (Pro X Superlight) 13 bytes.
*/
if (djrcv_dev->unnumbered_application == HID_GD_MOUSE &&
- size <= 8) {
- u8 mouse_report[9];
+ size <= 13){
+ u8 mouse_report[14];
/* Prepend report id */
mouse_report[0] = REPORT_TYPE_MOUSE;
@@ -1776,6 +1868,7 @@ static int logi_dj_probe(struct hid_device *hdev,
case recvr_type_dj: no_dj_interfaces = 3; break;
case recvr_type_hidpp: no_dj_interfaces = 2; break;
case recvr_type_gaming_hidpp: no_dj_interfaces = 3; break;
+ case recvr_type_gaming_hidpp_ls_1_2: no_dj_interfaces = 3; break;
case recvr_type_mouse_only: no_dj_interfaces = 2; break;
case recvr_type_27mhz: no_dj_interfaces = 2; break;
case recvr_type_bluetooth: no_dj_interfaces = 2; break;
@@ -1983,6 +2076,10 @@ static const struct hid_device_id logi_dj_receivers[] = {
HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH,
USB_DEVICE_ID_LOGITECH_NANO_RECEIVER_LIGHTSPEED_1_1),
.driver_data = recvr_type_gaming_hidpp},
+ { /* Logitech lightspeed receiver (0xc547) */
+ HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH,
+ USB_DEVICE_ID_LOGITECH_NANO_RECEIVER_LIGHTSPEED_1_2),
+ .driver_data = recvr_type_gaming_hidpp_ls_1_2},
{ /* Logitech 27 MHz HID++ 1.0 receiver (0xc513) */
HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_MX3000_RECEIVER),
--
2.34.1
^ permalink raw reply related
* Re: [PATCH v2 07/12] can: Add support for serdev LIN adapters
From: Simon Horman @ 2024-05-04 13:13 UTC (permalink / raw)
To: Christoph Fritz
Cc: Oliver Hartkopp, Marc Kleine-Budde, Vincent Mailhol,
David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Jiri Kosina,
Benjamin Tissoires, Greg Kroah-Hartman, Jiri Slaby,
Sebastian Reichel, Linus Walleij, Andreas Lauser, Jonathan Corbet,
Pavel Pisa, linux-can, netdev, devicetree, linux-input,
linux-serial
In-Reply-To: <20240502075534.882628-8-christoph.fritz@hexdev.de>
On Thu, May 02, 2024 at 09:55:29AM +0200, Christoph Fritz wrote:
> This commit introduces LIN-Bus support for UART devices equipped with
> LIN transceivers, utilizing the Serial Device Bus (serdev) interface.
>
> For more details on an adapter, visit: https://hexdev.de/hexlin#tty
>
> Signed-off-by: Christoph Fritz <christoph.fritz@hexdev.de>
...
> diff --git a/drivers/net/can/lin-serdev.c b/drivers/net/can/lin-serdev.c
...
> +static int linser_probe(struct serdev_device *serdev)
> +{
> + struct linser_priv *priv;
> + int ret;
> +
> + priv = devm_kzalloc(&serdev->dev, sizeof(*priv), GFP_KERNEL);
> + if (!priv)
> + return -ENOMEM;
> +
> + ret = kfifo_alloc(&priv->rx_fifo, LINSER_RX_FIFO_SIZE, GFP_KERNEL);
> + if (ret)
> + return ret;
> +
> + INIT_DELAYED_WORK(&priv->rx_work, linser_process_delayed);
> +
> + priv->serdev = serdev;
> + serdev_device_set_drvdata(serdev, priv);
> + serdev_device_set_client_ops(serdev, &linser_ops);
> +
> + ret = serdev_device_open(serdev);
> + if (ret) {
> + dev_err(&serdev->dev, "Unable to open device\n");
> + goto err_open;
> + }
> +
> + serdev_device_set_flow_control(serdev, false);
> + serdev_device_set_break_detection(serdev, true);
> + linser_derive_timings(priv, LIN_DEFAULT_BAUDRATE);
> +
> + mutex_init(&priv->resp_lock);
> +
> + priv->lin_dev = register_lin(&serdev->dev, &linser_lindev_ops);
> + if (IS_ERR_OR_NULL(priv->lin_dev)) {
> + ret = PTR_ERR(priv->lin_dev);
As per my feedback on an earlier patch in the series,
in the case where priv->lin_dev is NULL,
this will result in the function returning 0.
Is that ok?
Flagged by Smatch
> + goto err_register_lin;
> + }
> +
> + serdev_device_close(serdev);
> + priv->is_stopped = true;
> +
> + return 0;
> +
> +err_register_lin:
> + serdev_device_close(serdev);
> +err_open:
> + kfifo_free(&priv->rx_fifo);
> + return ret;
> +}
...
^ permalink raw reply
* Re: [PATCH v2 02/12] HID: hexLIN: Add support for USB LIN bus adapter
From: Simon Horman @ 2024-05-04 12:58 UTC (permalink / raw)
To: Christoph Fritz
Cc: Oliver Hartkopp, Marc Kleine-Budde, Vincent Mailhol,
David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Jiri Kosina,
Benjamin Tissoires, Greg Kroah-Hartman, Jiri Slaby,
Sebastian Reichel, Linus Walleij, Andreas Lauser, Jonathan Corbet,
Pavel Pisa, linux-can, netdev, devicetree, linux-input,
linux-serial
In-Reply-To: <20240502075534.882628-3-christoph.fritz@hexdev.de>
On Thu, May 02, 2024 at 09:55:24AM +0200, Christoph Fritz wrote:
> This patch introduces driver support for the hexLIN USB LIN bus adapter,
> enabling LIN communication over USB for both controller and responder
> modes. The driver interfaces with the CAN_LIN framework for userland
> connectivity.
>
> For more details on the adapter, visit: https://hexdev.de/hexlin/
>
> Tested-by: Andreas Lauser <andreas.lauser@mercedes-benz.com>
> Signed-off-by: Christoph Fritz <christoph.fritz@hexdev.de>
Hi Christoph,
Some minor feedback from my side.
...
> diff --git a/drivers/hid/hid-hexdev-hexlin.c b/drivers/hid/hid-hexdev-hexlin.c
...
> +static int hexlin_queue_frames_insert(struct hexlin_priv_data *priv,
> + const u8 *raw_data, int sz)
> +{
> + struct hid_device *hdev = priv->hid_dev;
> + struct hexlin_frame hxf;
> + struct lin_frame lf;
> +
> + if (sz != sizeof(struct hexlin_frame))
> + return -EREMOTEIO;
> +
> + memcpy(&hxf, raw_data, sz);
> + le32_to_cpus(hxf.flags);
> +
> + lf.len = hxf.len;
> + lf.lin_id = hxf.lin_id;
> + memcpy(lf.data, hxf.data, LIN_MAX_DLEN);
> + lf.checksum = hxf.checksum;
> + lf.checksum_mode = hxf.checksum_mode;
> +
> + hid_dbg(hdev, "id:%02x, len:%u, data:%*ph, checksum:%02x (%s)\n",
> + lf.lin_id, lf.len, lf.len, lf.data, lf.checksum,
> + lf.checksum_mode ? "enhanced" : "classic");
nit: The indentation above is not quite right.
It should align to inside the opening '(' like this.
hid_dbg(hdev, "id:%02x, len:%u, data:%*ph, checksum:%02x (%s)\n",
lf.lin_id, lf.len, lf.len, lf.data, lf.checksum,
lf.checksum_mode ? "enhanced" : "classic");
Flagged by checkpatch.
...
> +static int hexlin_set_baudrate(struct hexlin_priv_data *priv, u16 baudrate)
> +{
> + struct hexlin_baudrate_req req;
> + int ret;
> +
> + if (baudrate < LIN_MIN_BAUDRATE || baudrate > LIN_MAX_BAUDRATE)
> + return -EINVAL;
> +
> + req.cmd = HEXLIN_SET_BAUDRATE;
> + req.baudrate = cpu_to_le16(baudrate);
The type of req.baudrate is u16, a host byte-order type.
But it is being assigned a little endian value.
This does not seem right.
Flagged by Smatch.
> +
> + ret = hexlin_tx_req_status(priv, &req,
> + sizeof(struct hexlin_baudrate_req));
> + if (ret)
> + hid_err(priv->hid_dev, "%s failed with %d\n", __func__, ret);
> +
> + return ret;
> +}
...
> +static int hexlin_probe(struct hid_device *hdev,
> + const struct hid_device_id *id)
> +{
> + struct hexlin_priv_data *priv;
> + int ret;
> +
> + priv = devm_kzalloc(&hdev->dev, sizeof(*priv), GFP_KERNEL);
> + if (!priv)
> + return -ENOMEM;
> +
> + priv->hid_dev = hdev;
> + hid_set_drvdata(hdev, priv);
> +
> + mutex_init(&priv->tx_lock);
> +
> + ret = hid_parse(hdev);
> + if (ret) {
> + hid_err(hdev, "hid parse failed with %d\n", ret);
> + goto fail_and_free;
> + }
> +
> + ret = hid_hw_start(hdev, HID_CONNECT_DRIVER);
> + if (ret) {
> + hid_err(hdev, "hid hw start failed with %d\n", ret);
> + goto fail_and_stop;
> + }
> +
> + ret = hid_hw_open(hdev);
> + if (ret) {
> + hid_err(hdev, "hid hw open failed with %d\n", ret);
> + goto fail_and_close;
> + }
> +
> + init_completion(&priv->wait_in_report);
> +
> + hid_device_io_start(hdev);
> +
> + ret = init_hw(priv);
> + if (ret)
> + goto fail_and_close;
> +
> + priv->ldev = register_lin(&hdev->dev, &hexlin_ldo);
> + if (IS_ERR_OR_NULL(priv->ldev)) {
> + ret = PTR_ERR(priv->ldev);
> + goto fail_and_close;
Is it intentional that when priv->ldev is NULL here,
the function will return 0?
Flagged by Smatch.
> + }
> +
> + hid_hw_close(hdev);
> +
> + hid_info(hdev, "hexLIN (fw-version: %u) probed\n", priv->fw_version);
> +
> + return 0;
> +
> +fail_and_close:
> + hid_hw_close(hdev);
> +fail_and_stop:
> + hid_hw_stop(hdev);
> +fail_and_free:
> + mutex_destroy(&priv->tx_lock);
> + return ret;
> +}
...
^ permalink raw reply
* Re: [PATCH v2 01/12] can: Add LIN bus as CAN abstraction
From: Simon Horman @ 2024-05-04 12:49 UTC (permalink / raw)
To: Christoph Fritz
Cc: Oliver Hartkopp, Marc Kleine-Budde, Vincent Mailhol,
David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Jiri Kosina,
Benjamin Tissoires, Greg Kroah-Hartman, Jiri Slaby,
Sebastian Reichel, Linus Walleij, Andreas Lauser, Jonathan Corbet,
Pavel Pisa, linux-can, netdev, devicetree, linux-input,
linux-serial
In-Reply-To: <20240502075534.882628-2-christoph.fritz@hexdev.de>
On Thu, May 02, 2024 at 09:55:23AM +0200, Christoph Fritz wrote:
> This patch adds a LIN (local interconnect network) bus abstraction on
> top of CAN. It is a glue driver adapting CAN on one side while offering
> LIN abstraction on the other side. So that upcoming LIN device drivers
> can make use of it.
>
> Tested-by: Andreas Lauser <andreas.lauser@mercedes-benz.com>
> Signed-off-by: Christoph Fritz <christoph.fritz@hexdev.de>
...
> diff --git a/drivers/net/can/lin.c b/drivers/net/can/lin.c
...
> +struct lin_device *register_lin(struct device *dev,
> + const struct lin_device_ops *ldops)
> +{
> + struct net_device *ndev;
> + struct lin_device *ldev;
> + int ret;
> +
> + if (!ldops || !ldops->ldo_tx || !ldops->update_bitrate ||
> + !ldops->ldo_open || !ldops->ldo_stop) {
> + netdev_err(ndev, "missing mandatory lin_device_ops\n");
Hi Christoph,
The line above uses ndev, but ndev is not initialised
until a few lines further down.
Flagged by Smatch.
> + return ERR_PTR(-EINVAL);
> + }
> +
> + ndev = alloc_candev(sizeof(struct lin_device), 1);
> + if (!ndev)
> + return ERR_PTR(-ENOMEM);
> +
> + ldev = netdev_priv(ndev);
> +
> + ldev->ldev_ops = ldops;
> + ndev->netdev_ops = &lin_netdev_ops;
> + ndev->flags |= IFF_ECHO;
> + ndev->mtu = CANFD_MTU;
> + ldev->can.bittiming.bitrate = LIN_DEFAULT_BAUDRATE;
> + ldev->can.ctrlmode = CAN_CTRLMODE_LIN;
> + ldev->can.ctrlmode_supported = 0;
> + ldev->can.bitrate_const = lin_bitrate;
> + ldev->can.bitrate_const_cnt = ARRAY_SIZE(lin_bitrate);
> + ldev->can.do_set_bittiming = lin_set_bittiming;
> + ldev->ndev = ndev;
> + ldev->dev = dev;
> +
> + SET_NETDEV_DEV(ndev, dev);
> +
> + ret = lin_set_bittiming(ndev);
> + if (ret) {
> + netdev_err(ndev, "set bittiming failed\n");
> + goto exit_candev;
> + }
> +
> + ret = register_candev(ndev);
> + if (ret)
> + goto exit_candev;
> +
> + ldev->lin_ids_kobj = kobject_create_and_add("lin_ids", &ndev->dev.kobj);
> + if (!ldev->lin_ids_kobj) {
> + netdev_err(ndev, "Failed to create sysfs directory\n");
> + ret = -ENOMEM;
> + goto exit_unreg;
> + }
> +
> + ret = lin_create_sysfs_id_files(ndev);
> + if (ret) {
> + netdev_err(ndev, "Failed to create sysfs entry: %d\n", ret);
> + goto exit_kobj_put;
> + }
> +
> + /* Using workqueue as tx over USB/SPI/... may sleep */
> + ldev->wq = alloc_workqueue(dev_name(dev), WQ_FREEZABLE | WQ_MEM_RECLAIM,
> + 0);
> + if (!ldev->wq)
> + goto exit_rm_files;
The goto above will result in: return ERR_PTR(ret)
But ret is 0 here. Should it be set to a negative error value?
Also flagged by Smatch.
> +
> + INIT_WORK(&ldev->tx_work, lin_tx_work_handler);
> +
> + netdev_info(ndev, "LIN initialized.\n");
> +
> + return ldev;
> +
> +exit_rm_files:
> + lin_remove_sysfs_id_files(ndev);
> +exit_kobj_put:
> + kobject_put(ldev->lin_ids_kobj);
> +exit_unreg:
> + unregister_candev(ndev);
> +exit_candev:
> + free_candev(ndev);
> + return ERR_PTR(ret);
> +}
> +EXPORT_SYMBOL_GPL(register_lin);
...
^ permalink raw reply
* Re: [PATCH 2/6] input: himax_hx83112b: add regulator handling
From: Krzysztof Kozlowski @ 2024-05-04 12:37 UTC (permalink / raw)
To: Felix Kaechele, Job Noorman, Dmitry Torokhov, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, linux-input, devicetree,
linux-kernel
In-Reply-To: <20240504020745.68525-3-felix@kaechele.ca>
On 04/05/2024 04:04, Felix Kaechele wrote:
> Handle regulators used on this chip family, namely AVDD and VDD. These
> definitions are taken from the GPLv2 licensed vendor driver.
>
> Signed-off-by: Felix Kaechele <felix@kaechele.ca>
> Link: https://github.com/HimaxSoftware/HX83112_Android_Driver
> ---
> drivers/input/touchscreen/himax_hx83112b.c | 48 ++++++++++++++++++++--
> 1 file changed, 44 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/input/touchscreen/himax_hx83112b.c b/drivers/input/touchscreen/himax_hx83112b.c
> index 4f6609dcdef3..0a797789e548 100644
> --- a/drivers/input/touchscreen/himax_hx83112b.c
> +++ b/drivers/input/touchscreen/himax_hx83112b.c
> @@ -19,10 +19,12 @@
> #include <linux/interrupt.h>
> #include <linux/kernel.h>
> #include <linux/regmap.h>
> +#include <linux/regulator/consumer.h>
>
> #define HIMAX_ID_83112B 0x83112b
>
> #define HIMAX_MAX_POINTS 10
> +#define HIMAX_MAX_SUPPLIES 2
>
> #define HIMAX_REG_CFG_SET_ADDR 0x00
> #define HIMAX_REG_CFG_INIT_READ 0x0c
> @@ -50,6 +52,7 @@ struct himax_event {
> static_assert(sizeof(struct himax_event) == 56);
>
> struct himax_ts_data {
> + struct regulator_bulk_data supplies[HIMAX_MAX_SUPPLIES];
> struct gpio_desc *gpiod_rst;
> struct input_dev *input_dev;
> struct i2c_client *client;
> @@ -63,6 +66,11 @@ static const struct regmap_config himax_regmap_config = {
> .val_format_endian = REGMAP_ENDIAN_LITTLE,
> };
>
> +static const char *const himax_supply_names[] = {
> + "avdd",
> + "vdd",
> +};
> +
That's confusing. Binding said only HX83100A family has regulators, but
you request for everyone.
> static int himax_read_config(struct himax_ts_data *ts, u32 address, u32 *dst)
> {
> int error;
> @@ -267,7 +275,7 @@ static irqreturn_t himax_irq_handler(int irq, void *dev_id)
>
> static int himax_probe(struct i2c_client *client)
> {
> - int error;
> + int error, i;
> struct device *dev = &client->dev;
> struct himax_ts_data *ts;
>
> @@ -290,11 +298,31 @@ static int himax_probe(struct i2c_client *client)
> return error;
> }
>
> + int num_supplies = ARRAY_SIZE(himax_supply_names);
> +
> + for (i = 0; i < num_supplies; i++)
> + ts->supplies[i].supply = himax_supply_names[i];
> +
> + error = devm_regulator_bulk_get(dev,
devm_regulator_bulk_get_enable and drop rest of the code here.
> + num_supplies,
> + ts->supplies);
Wrap it properly at 80, not one argument in one line.
> + if (error) {
> + dev_err(dev, "Failed to get supplies: %d\n", error);
return dev_err_probe()
> + return error;
> + }
> +
> + error = regulator_bulk_enable(num_supplies,
> + ts->supplies);
> + if (error) {
> + dev_err(dev, "Failed to enable supplies: %d\n", error);
> + goto error_out;
> + }
> +
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH 1/6] dt-bindings: input: touchscreen: himax,hx83112b: add HX83100A
From: Krzysztof Kozlowski @ 2024-05-04 12:34 UTC (permalink / raw)
To: Felix Kaechele, Job Noorman, Dmitry Torokhov, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, linux-input, devicetree,
linux-kernel
In-Reply-To: <20240504020745.68525-2-felix@kaechele.ca>
On 04/05/2024 04:04, Felix Kaechele wrote:
> This adds a compatible string for the Himax HX83100A touch controller
Please do not use "This commit/patch/change", but imperative mood. See
longer explanation here:
https://elixir.bootlin.com/linux/v5.17.1/source/Documentation/process/submitting-patches.rst#L95
> including the AVDD and VDD supply nodes used by this chip family.
>
> Signed-off-by: Felix Kaechele <felix@kaechele.ca>
> ---
> .../bindings/input/touchscreen/himax,hx83112b.yaml | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/input/touchscreen/himax,hx83112b.yaml b/Documentation/devicetree/bindings/input/touchscreen/himax,hx83112b.yaml
> index f42b23d532eb..5809afedb9a2 100644
> --- a/Documentation/devicetree/bindings/input/touchscreen/himax,hx83112b.yaml
> +++ b/Documentation/devicetree/bindings/input/touchscreen/himax,hx83112b.yaml
> @@ -15,6 +15,7 @@ allOf:
> properties:
> compatible:
> enum:
> + - himax,hx83100a
> - himax,hx83112b
>
> reg:
> @@ -26,6 +27,12 @@ properties:
> reset-gpios:
> maxItems: 1
>
> + avdd-supply:
> + description: Analog power supply regulator
> +
> + vdd-supply:
> + description: Digital power supply regulator
These should not be allowed for other variant, so either you need
allOf:if:then disallowing them (: false) or just create another binding
file.
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH v2 4/7] iio: addac: ad74115: Use devm_regulator_get_enable_read_voltage()
From: Jonathan Cameron @ 2024-05-04 11:25 UTC (permalink / raw)
To: David Lechner
Cc: Liam Girdwood, Mark Brown, Jean Delvare, Guenter Roeck,
Dmitry Torokhov, Jonathan Corbet, Support Opensource,
Cosmin Tanislav, Lars-Peter Clausen, Michael Hennerich,
Antoniu Miclaus, Greg Kroah-Hartman, linux-doc, linux-kernel,
linux-hwmon, linux-iio, linux-staging, linux-input
In-Reply-To: <20240429-regulator-get-enable-get-votlage-v2-4-b1f11ab766c1@baylibre.com>
On Mon, 29 Apr 2024 18:40:12 -0500
David Lechner <dlechner@baylibre.com> wrote:
> We can reduce boilerplate code by using
> devm_regulator_get_enable_read_voltage().
>
> To maintain backwards compatibility in the case a DT does not provide
> an avdd-supply, we fall back to calling devm_regulator_get_enable()
> so that there is no change in user-facing behavior (e.g. dummy regulator
> will still be in sysfs).
>
> Also add an informative error message when we failed to get the voltage
> and knowing the voltage is required while we are touching this.
>
> Signed-off-by: David Lechner <dlechner@baylibre.com>
A somewhat fiddly case. I think you've done it the best way possible though.
Acked-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
^ permalink raw reply
* [PATCH 3/6] input: himax_hx83112b: use more descriptive register defines
From: Felix Kaechele @ 2024-05-04 2:04 UTC (permalink / raw)
To: Job Noorman, Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, linux-input, devicetree, linux-kernel
In-Reply-To: <20240504020745.68525-1-felix@kaechele.ca>
Himax uses an AHB-style bus to communicate with different parts of the
display driver and touch controller system.
Use more descriptive names for the register and address defines.
The names were taken from a driver submission for the similar HX83102J
chip.
Signed-off-by: Felix Kaechele <felix@kaechele.ca>
Link: https://lore.kernel.org/all/TY0PR06MB561105A3386E9D76F429110D9E0F2@TY0PR06MB5611.apcprd06.prod.outlook.com/
---
drivers/input/touchscreen/himax_hx83112b.c | 23 ++++++++++++----------
1 file changed, 13 insertions(+), 10 deletions(-)
diff --git a/drivers/input/touchscreen/himax_hx83112b.c b/drivers/input/touchscreen/himax_hx83112b.c
index 0a797789e548..ba5442cc0a24 100644
--- a/drivers/input/touchscreen/himax_hx83112b.c
+++ b/drivers/input/touchscreen/himax_hx83112b.c
@@ -26,12 +26,14 @@
#define HIMAX_MAX_POINTS 10
#define HIMAX_MAX_SUPPLIES 2
-#define HIMAX_REG_CFG_SET_ADDR 0x00
-#define HIMAX_REG_CFG_INIT_READ 0x0c
-#define HIMAX_REG_CFG_READ_VALUE 0x08
-#define HIMAX_REG_READ_EVENT 0x30
+#define HIMAX_AHB_ADDR_BYTE_0 0x00
+#define HIMAX_AHB_ADDR_RDATA_BYTE_0 0x08
+#define HIMAX_AHB_ADDR_ACCESS_DIRECTION 0x0c
+#define HIMAX_AHB_ADDR_EVENT_STACK 0x30
-#define HIMAX_CFG_PRODUCT_ID 0x900000d0
+#define HIMAX_AHB_CMD_ACCESS_DIRECTION_READ 0x00
+
+#define HIMAX_REG_ADDR_ICID 0x900000d0
#define HIMAX_INVALID_COORD 0xffff
@@ -75,15 +77,16 @@ static int himax_read_config(struct himax_ts_data *ts, u32 address, u32 *dst)
{
int error;
- error = regmap_write(ts->regmap, HIMAX_REG_CFG_SET_ADDR, address);
+ error = regmap_write(ts->regmap, HIMAX_AHB_ADDR_BYTE_0, address);
if (error)
return error;
- error = regmap_write(ts->regmap, HIMAX_REG_CFG_INIT_READ, 0x0);
+ error = regmap_write(ts->regmap, HIMAX_AHB_ADDR_ACCESS_DIRECTION,
+ HIMAX_AHB_CMD_ACCESS_DIRECTION_READ);
if (error)
return error;
- error = regmap_read(ts->regmap, HIMAX_REG_CFG_READ_VALUE, dst);
+ error = regmap_read(ts->regmap, HIMAX_AHB_ADDR_RDATA_BYTE_0, dst);
if (error)
return error;
@@ -109,7 +112,7 @@ static int himax_read_product_id(struct himax_ts_data *ts, u32 *product_id)
{
int error;
- error = himax_read_config(ts, HIMAX_CFG_PRODUCT_ID, product_id);
+ error = himax_read_config(ts, HIMAX_REG_ADDR_ICID, product_id);
if (error)
return error;
@@ -243,7 +246,7 @@ static int himax_handle_input(struct himax_ts_data *ts)
int error;
struct himax_event event;
- error = regmap_raw_read(ts->regmap, HIMAX_REG_READ_EVENT, &event,
+ error = regmap_raw_read(ts->regmap, HIMAX_AHB_ADDR_EVENT_STACK, &event,
sizeof(event));
if (error) {
dev_err(&ts->client->dev, "Failed to read input event: %d\n",
--
2.44.0
^ permalink raw reply related
* [PATCH 1/6] dt-bindings: input: touchscreen: himax,hx83112b: add HX83100A
From: Felix Kaechele @ 2024-05-04 2:04 UTC (permalink / raw)
To: Job Noorman, Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, linux-input, devicetree, linux-kernel
In-Reply-To: <20240504020745.68525-1-felix@kaechele.ca>
This adds a compatible string for the Himax HX83100A touch controller
including the AVDD and VDD supply nodes used by this chip family.
Signed-off-by: Felix Kaechele <felix@kaechele.ca>
---
.../bindings/input/touchscreen/himax,hx83112b.yaml | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/Documentation/devicetree/bindings/input/touchscreen/himax,hx83112b.yaml b/Documentation/devicetree/bindings/input/touchscreen/himax,hx83112b.yaml
index f42b23d532eb..5809afedb9a2 100644
--- a/Documentation/devicetree/bindings/input/touchscreen/himax,hx83112b.yaml
+++ b/Documentation/devicetree/bindings/input/touchscreen/himax,hx83112b.yaml
@@ -15,6 +15,7 @@ allOf:
properties:
compatible:
enum:
+ - himax,hx83100a
- himax,hx83112b
reg:
@@ -26,6 +27,12 @@ properties:
reset-gpios:
maxItems: 1
+ avdd-supply:
+ description: Analog power supply regulator
+
+ vdd-supply:
+ description: Digital power supply regulator
+
touchscreen-inverted-x: true
touchscreen-inverted-y: true
touchscreen-size-x: true
@@ -54,6 +61,8 @@ examples:
reg = <0x48>;
interrupt-parent = <&tlmm>;
interrupts = <65 IRQ_TYPE_LEVEL_LOW>;
+ avdd-supply = <&avdd_reg>;
+ vdd-supply = <&vdd_reg>;
touchscreen-size-x = <1080>;
touchscreen-size-y = <2160>;
reset-gpios = <&tlmm 64 GPIO_ACTIVE_LOW>;
--
2.44.0
^ permalink raw reply related
* [PATCH 4/6] input: himax_hx83112b: implement MCU register reading
From: Felix Kaechele @ 2024-05-04 2:04 UTC (permalink / raw)
To: Job Noorman, Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, linux-input, devicetree, linux-kernel
In-Reply-To: <20240504020745.68525-1-felix@kaechele.ca>
Implement reading from the MCU in a more universal fashion. This allows
properly handling reads of more than 4 bytes using the AHB FIFO
implemented in the chip.
Signed-off-by: Felix Kaechele <felix@kaechele.ca>
---
drivers/input/touchscreen/himax_hx83112b.c | 51 ++++++++++++++++++++--
1 file changed, 48 insertions(+), 3 deletions(-)
diff --git a/drivers/input/touchscreen/himax_hx83112b.c b/drivers/input/touchscreen/himax_hx83112b.c
index ba5442cc0a24..0173ff394a99 100644
--- a/drivers/input/touchscreen/himax_hx83112b.c
+++ b/drivers/input/touchscreen/himax_hx83112b.c
@@ -29,9 +29,13 @@
#define HIMAX_AHB_ADDR_BYTE_0 0x00
#define HIMAX_AHB_ADDR_RDATA_BYTE_0 0x08
#define HIMAX_AHB_ADDR_ACCESS_DIRECTION 0x0c
+#define HIMAX_AHB_ADDR_INCR4 0x0d
+#define HIMAX_AHB_ADDR_CONTI 0x13
#define HIMAX_AHB_ADDR_EVENT_STACK 0x30
#define HIMAX_AHB_CMD_ACCESS_DIRECTION_READ 0x00
+#define HIMAX_AHB_CMD_INCR4 0x10
+#define HIMAX_AHB_CMD_CONTI 0x31
#define HIMAX_REG_ADDR_ICID 0x900000d0
@@ -73,10 +77,34 @@ static const char *const himax_supply_names[] = {
"vdd",
};
-static int himax_read_config(struct himax_ts_data *ts, u32 address, u32 *dst)
+static int himax_bus_enable_burst(struct himax_ts_data *ts)
{
int error;
+ error = regmap_write(ts->regmap, HIMAX_AHB_ADDR_CONTI,
+ HIMAX_AHB_CMD_CONTI);
+ if (error)
+ return error;
+
+ error = regmap_write(ts->regmap, HIMAX_AHB_ADDR_INCR4,
+ HIMAX_AHB_CMD_INCR4);
+ if (error)
+ return error;
+
+ return 0;
+}
+
+static int himax_bus_read(struct himax_ts_data *ts, u32 address, void *dst,
+ size_t length)
+{
+ int error;
+
+ if (length > 4) {
+ error = himax_bus_enable_burst(ts);
+ if (error)
+ return error;
+ }
+
error = regmap_write(ts->regmap, HIMAX_AHB_ADDR_BYTE_0, address);
if (error)
return error;
@@ -86,7 +114,24 @@ static int himax_read_config(struct himax_ts_data *ts, u32 address, u32 *dst)
if (error)
return error;
- error = regmap_read(ts->regmap, HIMAX_AHB_ADDR_RDATA_BYTE_0, dst);
+ if (length > 4)
+ error = regmap_noinc_read(ts->regmap,
+ HIMAX_AHB_ADDR_RDATA_BYTE_0,
+ dst, length);
+ else
+ error = regmap_read(ts->regmap,
+ HIMAX_AHB_ADDR_RDATA_BYTE_0, dst);
+ if (error)
+ return error;
+
+ return 0;
+}
+
+static int himax_read_mcu(struct himax_ts_data *ts, u32 address, u32 *dst)
+{
+ int error;
+
+ error = himax_bus_read(ts, address, dst, sizeof(dst));
if (error)
return error;
@@ -112,7 +157,7 @@ static int himax_read_product_id(struct himax_ts_data *ts, u32 *product_id)
{
int error;
- error = himax_read_config(ts, HIMAX_REG_ADDR_ICID, product_id);
+ error = himax_read_mcu(ts, HIMAX_REG_ADDR_ICID, product_id);
if (error)
return error;
--
2.44.0
^ permalink raw reply related
* [PATCH 2/6] input: himax_hx83112b: add regulator handling
From: Felix Kaechele @ 2024-05-04 2:04 UTC (permalink / raw)
To: Job Noorman, Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, linux-input, devicetree, linux-kernel
In-Reply-To: <20240504020745.68525-1-felix@kaechele.ca>
Handle regulators used on this chip family, namely AVDD and VDD. These
definitions are taken from the GPLv2 licensed vendor driver.
Signed-off-by: Felix Kaechele <felix@kaechele.ca>
Link: https://github.com/HimaxSoftware/HX83112_Android_Driver
---
drivers/input/touchscreen/himax_hx83112b.c | 48 ++++++++++++++++++++--
1 file changed, 44 insertions(+), 4 deletions(-)
diff --git a/drivers/input/touchscreen/himax_hx83112b.c b/drivers/input/touchscreen/himax_hx83112b.c
index 4f6609dcdef3..0a797789e548 100644
--- a/drivers/input/touchscreen/himax_hx83112b.c
+++ b/drivers/input/touchscreen/himax_hx83112b.c
@@ -19,10 +19,12 @@
#include <linux/interrupt.h>
#include <linux/kernel.h>
#include <linux/regmap.h>
+#include <linux/regulator/consumer.h>
#define HIMAX_ID_83112B 0x83112b
#define HIMAX_MAX_POINTS 10
+#define HIMAX_MAX_SUPPLIES 2
#define HIMAX_REG_CFG_SET_ADDR 0x00
#define HIMAX_REG_CFG_INIT_READ 0x0c
@@ -50,6 +52,7 @@ struct himax_event {
static_assert(sizeof(struct himax_event) == 56);
struct himax_ts_data {
+ struct regulator_bulk_data supplies[HIMAX_MAX_SUPPLIES];
struct gpio_desc *gpiod_rst;
struct input_dev *input_dev;
struct i2c_client *client;
@@ -63,6 +66,11 @@ static const struct regmap_config himax_regmap_config = {
.val_format_endian = REGMAP_ENDIAN_LITTLE,
};
+static const char *const himax_supply_names[] = {
+ "avdd",
+ "vdd",
+};
+
static int himax_read_config(struct himax_ts_data *ts, u32 address, u32 *dst)
{
int error;
@@ -267,7 +275,7 @@ static irqreturn_t himax_irq_handler(int irq, void *dev_id)
static int himax_probe(struct i2c_client *client)
{
- int error;
+ int error, i;
struct device *dev = &client->dev;
struct himax_ts_data *ts;
@@ -290,11 +298,31 @@ static int himax_probe(struct i2c_client *client)
return error;
}
+ int num_supplies = ARRAY_SIZE(himax_supply_names);
+
+ for (i = 0; i < num_supplies; i++)
+ ts->supplies[i].supply = himax_supply_names[i];
+
+ error = devm_regulator_bulk_get(dev,
+ num_supplies,
+ ts->supplies);
+ if (error) {
+ dev_err(dev, "Failed to get supplies: %d\n", error);
+ return error;
+ }
+
+ error = regulator_bulk_enable(num_supplies,
+ ts->supplies);
+ if (error) {
+ dev_err(dev, "Failed to enable supplies: %d\n", error);
+ goto error_out;
+ }
+
ts->gpiod_rst = devm_gpiod_get(dev, "reset", GPIOD_OUT_HIGH);
error = PTR_ERR_OR_ZERO(ts->gpiod_rst);
if (error) {
dev_err(dev, "Failed to get reset GPIO: %d\n", error);
- return error;
+ goto error_out;
}
himax_reset(ts);
@@ -305,15 +333,26 @@ static int himax_probe(struct i2c_client *client)
error = himax_input_register(ts);
if (error)
- return error;
+ goto error_out;
error = devm_request_threaded_irq(dev, client->irq, NULL,
himax_irq_handler, IRQF_ONESHOT,
client->name, ts);
if (error)
- return error;
+ goto error_out;
return 0;
+
+error_out:
+ regulator_bulk_disable(num_supplies, ts->supplies);
+ return error;
+}
+
+static void himax_remove(struct i2c_client *client)
+{
+ struct himax_ts_data *ts = i2c_get_clientdata(client);
+
+ regulator_bulk_disable(ARRAY_SIZE(himax_supply_names), ts->supplies);
}
static int himax_suspend(struct device *dev)
@@ -350,6 +389,7 @@ MODULE_DEVICE_TABLE(of, himax_of_match);
static struct i2c_driver himax_ts_driver = {
.probe = himax_probe,
+ .remove = himax_remove,
.id_table = himax_ts_id,
.driver = {
.name = "Himax-hx83112b-TS",
--
2.44.0
^ permalink raw reply related
* [PATCH 0/6] input: himax_hx83112b: add support for HX83100A
From: Felix Kaechele @ 2024-05-04 2:04 UTC (permalink / raw)
To: Job Noorman, Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, linux-input, devicetree, linux-kernel
This set of patches brings support for the Himax HX83100A touch
controller.
To properly bring the chip up, support for regulator handling is also
added.
I have no access to datasheets. So, like the original driver code
that's being extended here, this code is mostly based on the quite
convoluted, GPLv2 licensed manufacturer drivers for Android.
I included links to sources and references where appropriate.
A number of people tested this patch set on Lenovo ThinkSmart View
(CD-18781Y) devices. That device has a variant utilizing a Innolux
P080DDD-AB2 LCM. This LCM comes with the HX83100A.
I would really appreciate if people using HX83112B chips could give this
set a run to ensure nothing broke.
Thanks,
Felix
Felix Kaechele (6):
dt-bindings: input: touchscreen: himax,hx83112b: add HX83100A
input: himax_hx83112b: add regulator handling
input: himax_hx83112b: use more descriptive register defines
input: himax_hx83112b: implement MCU register reading
input: himax_hx83112b: add himax_chip struct for multi-chip support
input: himax_hx83112b: add support for HX83100A
.../input/touchscreen/himax,hx83112b.yaml | 9 +
drivers/input/touchscreen/himax_hx83112b.c | 187 +++++++++++++++---
2 files changed, 166 insertions(+), 30 deletions(-)
base-commit: 7b4e0b39182cf5e677c1fc092a3ec40e621c25b6
--
2.44.0
^ permalink raw reply
* [PATCH 6/6] input: himax_hx83112b: add support for HX83100A
From: Felix Kaechele @ 2024-05-04 2:04 UTC (permalink / raw)
To: Job Noorman, Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, linux-input, devicetree, linux-kernel
Cc: Paul Gale
In-Reply-To: <20240504020745.68525-1-felix@kaechele.ca>
The HX83100A is a bit of an outlier in the Himax HX831xxx series of
touch controllers as it requires reading touch events through the AHB
interface of the MCU rather than providing a dedicated FIFO address like
the other chips do.
This patch implements the specific read function and introduces the
HX83100A chip with an appropriate i2c ID and DT compatible string.
The HX83100A doesn't have a straightforward way to do chip
identification, which is why it is not implemented in this patch.
Tested on: Lenovo ThinkSmart View (CD-18781Y) / Innolux P080DDD-AB2 LCM
Signed-off-by: Felix Kaechele <felix@kaechele.ca>
Tested-by: Paul Gale <paul@siliconpixel.com>
---
drivers/input/touchscreen/himax_hx83112b.c | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)
diff --git a/drivers/input/touchscreen/himax_hx83112b.c b/drivers/input/touchscreen/himax_hx83112b.c
index 667611c5a018..984baef495e1 100644
--- a/drivers/input/touchscreen/himax_hx83112b.c
+++ b/drivers/input/touchscreen/himax_hx83112b.c
@@ -4,6 +4,9 @@
*
* Copyright (C) 2022 Job Noorman <job@noorman.info>
*
+ * HX83100A support
+ * Copyright (C) 2024 Felix Kaechele <felix@kaechele.ca>
+ *
* This code is based on "Himax Android Driver Sample Code for QCT platform":
*
* Copyright (C) 2017 Himax Corporation.
@@ -37,6 +40,8 @@
#define HIMAX_REG_ADDR_ICID 0x900000d0
+#define HX83100A_REG_FW_EVENT_STACK 0x90060000
+
#define HIMAX_INVALID_COORD 0xffff
struct himax_event_point {
@@ -291,6 +296,13 @@ static bool himax_verify_checksum(struct himax_ts_data *ts,
return true;
}
+static int hx83100a_read_events(struct himax_ts_data *ts,
+ struct himax_event *event,
+ size_t length)
+{
+ return himax_bus_read(ts, HX83100A_REG_FW_EVENT_STACK, event, length);
+};
+
static int himax_read_events(struct himax_ts_data *ts,
struct himax_event *event,
size_t length)
@@ -436,6 +448,10 @@ static int himax_resume(struct device *dev)
static DEFINE_SIMPLE_DEV_PM_OPS(himax_pm_ops, himax_suspend, himax_resume);
+static const struct himax_chip hx83100a_chip = {
+ .read_events = hx83100a_read_events,
+};
+
static const struct himax_chip hx83112b_chip = {
.id = 0x83112b,
.check_id = himax_check_product_id,
@@ -443,6 +459,7 @@ static const struct himax_chip hx83112b_chip = {
};
static const struct i2c_device_id himax_ts_id[] = {
+ { "hx83100a", (kernel_ulong_t) &hx83100a_chip },
{ "hx83112b", (kernel_ulong_t) &hx83112b_chip },
{ /* sentinel */ }
};
@@ -450,6 +467,7 @@ MODULE_DEVICE_TABLE(i2c, himax_ts_id);
#ifdef CONFIG_OF
static const struct of_device_id himax_of_match[] = {
+ { .compatible = "himax,hx83100a", .data = &hx83100a_chip },
{ .compatible = "himax,hx83112b", .data = &hx83112b_chip },
{ /* sentinel */ }
};
--
2.44.0
^ permalink raw reply related
* [PATCH 5/6] input: himax_hx83112b: add himax_chip struct for multi-chip support
From: Felix Kaechele @ 2024-05-04 2:04 UTC (permalink / raw)
To: Job Noorman, Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, linux-input, devicetree, linux-kernel
In-Reply-To: <20240504020745.68525-1-felix@kaechele.ca>
In preparation for HX83100A support allow defining separate functions
for specific chip operations.
Signed-off-by: Felix Kaechele <felix@kaechele.ca>
---
drivers/input/touchscreen/himax_hx83112b.c | 53 +++++++++++++++-------
1 file changed, 37 insertions(+), 16 deletions(-)
diff --git a/drivers/input/touchscreen/himax_hx83112b.c b/drivers/input/touchscreen/himax_hx83112b.c
index 0173ff394a99..667611c5a018 100644
--- a/drivers/input/touchscreen/himax_hx83112b.c
+++ b/drivers/input/touchscreen/himax_hx83112b.c
@@ -21,8 +21,6 @@
#include <linux/regmap.h>
#include <linux/regulator/consumer.h>
-#define HIMAX_ID_83112B 0x83112b
-
#define HIMAX_MAX_POINTS 10
#define HIMAX_MAX_SUPPLIES 2
@@ -57,7 +55,17 @@ struct himax_event {
static_assert(sizeof(struct himax_event) == 56);
+struct himax_ts_data;
+struct himax_chip {
+ u32 id;
+ int (*check_id)(struct himax_ts_data *ts);
+ int (*read_events)(struct himax_ts_data *ts,
+ struct himax_event *event,
+ size_t length);
+};
+
struct himax_ts_data {
+ const struct himax_chip *chip;
struct regulator_bulk_data supplies[HIMAX_MAX_SUPPLIES];
struct gpio_desc *gpiod_rst;
struct input_dev *input_dev;
@@ -176,15 +184,12 @@ static int himax_check_product_id(struct himax_ts_data *ts)
dev_dbg(&ts->client->dev, "Product id: %x\n", product_id);
- switch (product_id) {
- case HIMAX_ID_83112B:
+ if (product_id == ts->chip->id)
return 0;
- default:
- dev_err(&ts->client->dev,
- "Unknown product id: %x\n", product_id);
- return -EINVAL;
- }
+ dev_err(&ts->client->dev, "Unknown product id: %x\n",
+ product_id);
+ return -EINVAL;
}
static int himax_input_register(struct himax_ts_data *ts)
@@ -286,13 +291,20 @@ static bool himax_verify_checksum(struct himax_ts_data *ts,
return true;
}
+static int himax_read_events(struct himax_ts_data *ts,
+ struct himax_event *event,
+ size_t length)
+{
+ return regmap_raw_read(ts->regmap, HIMAX_AHB_ADDR_EVENT_STACK, event,
+ length);
+}
+
static int himax_handle_input(struct himax_ts_data *ts)
{
int error;
struct himax_event event;
- error = regmap_raw_read(ts->regmap, HIMAX_AHB_ADDR_EVENT_STACK, &event,
- sizeof(event));
+ error = ts->chip->read_events(ts, &event, sizeof(event));
if (error) {
dev_err(&ts->client->dev, "Failed to read input event: %d\n",
error);
@@ -338,6 +350,7 @@ static int himax_probe(struct i2c_client *client)
i2c_set_clientdata(client, ts);
ts->client = client;
+ ts->chip = i2c_get_match_data(client);
ts->regmap = devm_regmap_init_i2c(client, &himax_regmap_config);
error = PTR_ERR_OR_ZERO(ts->regmap);
@@ -375,9 +388,11 @@ static int himax_probe(struct i2c_client *client)
himax_reset(ts);
- error = himax_check_product_id(ts);
- if (error)
- return error;
+ if (ts->chip->check_id) {
+ error = himax_check_product_id(ts);
+ if (error)
+ return error;
+ }
error = himax_input_register(ts);
if (error)
@@ -421,15 +436,21 @@ static int himax_resume(struct device *dev)
static DEFINE_SIMPLE_DEV_PM_OPS(himax_pm_ops, himax_suspend, himax_resume);
+static const struct himax_chip hx83112b_chip = {
+ .id = 0x83112b,
+ .check_id = himax_check_product_id,
+ .read_events = himax_read_events,
+};
+
static const struct i2c_device_id himax_ts_id[] = {
- { "hx83112b", 0 },
+ { "hx83112b", (kernel_ulong_t) &hx83112b_chip },
{ /* sentinel */ }
};
MODULE_DEVICE_TABLE(i2c, himax_ts_id);
#ifdef CONFIG_OF
static const struct of_device_id himax_of_match[] = {
- { .compatible = "himax,hx83112b" },
+ { .compatible = "himax,hx83112b", .data = &hx83112b_chip },
{ /* sentinel */ }
};
MODULE_DEVICE_TABLE(of, himax_of_match);
--
2.44.0
^ permalink raw reply related
* Re: [PATCH v3 06/11] dt-bindings: net/can: Add serial (serdev) LIN adapter
From: Christoph Fritz @ 2024-05-03 18:29 UTC (permalink / raw)
To: Conor Dooley
Cc: Jiri Slaby, Oliver Hartkopp, Marc Kleine-Budde, Vincent Mailhol,
David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Jiri Kosina,
Benjamin Tissoires, Greg Kroah-Hartman, Sebastian Reichel,
Linus Walleij, Andreas Lauser, Jonathan Corbet, Pavel Pisa,
linux-can, netdev, devicetree, linux-input, linux-serial
In-Reply-To: <20240503-fading-extruding-2105bbd8b479@spud>
Hello Conor,
thanks for having an eye on this, please see my answer below.
On Fri, 2024-05-03 at 18:12 +0100, Conor Dooley wrote:
> On Thu, May 02, 2024 at 08:27:59PM +0200, Christoph Fritz wrote:
> > Add dt-bindings for serial LIN bus adapters. These adapters are
> > basically just LIN transceivers that are hard-wired to serial devices.
> >
> > Signed-off-by: Christoph Fritz <christoph.fritz@hexdev.de>
> > ---
> > .../bindings/net/can/hexdev,lin-serdev.yaml | 32 +++++++++++++++++++
> > 1 file changed, 32 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/net/can/hexdev,lin-serdev.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/net/can/hexdev,lin-serdev.yaml b/Documentation/devicetree/bindings/net/can/hexdev,lin-serdev.yaml
> > new file mode 100644
> > index 0000000000000..c178eb9be1391
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/net/can/hexdev,lin-serdev.yaml
> > @@ -0,0 +1,32 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/net/can/hexdev,lin-serdev.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Serial LIN Adapter
> > +
> > +description:
> > + LIN transceiver, mostly hard-wired to a serial device, used for communication
> > + on a LIN bus.
> > + For more details on an adapter, visit <https://hexdev.de/hexlin#tty>.
> > +
> > +maintainers:
> > + - Christoph Fritz <christoph.fritz@hexdev.de>
> > +
> > +properties:
> > + compatible:
> > + const: hexdev,lin-serdev
>
> Maybe I've just missed something on earlier versions that I didn't
> read, but the name of the device on the website you link is "hexLIN",
> so why is "lin-serdev" used here instead?
The USB one is called hexLIN and has it's own HID driver.
This serial LIN adapter doesn't really have a product name. Currently
on our website it's generically called 'UART LIN Adapter'.
This LIN adapter is basically just a LIN transceiver and very generic,
so that one could solder it to any single-board computer with an uart.
I think 'lin-serdev' for LIN and serial device fits great, also serdev
is the name of the used kernel infrastructure (besides the LIN glue
driver).
If you still don't like it, I'm open to other names. What about
"hexlin-uart" or "linser"?
Thanks
-- Christoph
>
^ permalink raw reply
* Re: [PATCH v3 06/11] dt-bindings: net/can: Add serial (serdev) LIN adapter
From: Conor Dooley @ 2024-05-03 17:12 UTC (permalink / raw)
To: Christoph Fritz
Cc: Jiri Slaby, Oliver Hartkopp, Marc Kleine-Budde, Vincent Mailhol,
David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Jiri Kosina,
Benjamin Tissoires, Greg Kroah-Hartman, Sebastian Reichel,
Linus Walleij, Andreas Lauser, Jonathan Corbet, Pavel Pisa,
linux-can, netdev, devicetree, linux-input, linux-serial
In-Reply-To: <20240502182804.145926-7-christoph.fritz@hexdev.de>
[-- Attachment #1: Type: text/plain, Size: 1827 bytes --]
On Thu, May 02, 2024 at 08:27:59PM +0200, Christoph Fritz wrote:
> Add dt-bindings for serial LIN bus adapters. These adapters are
> basically just LIN transceivers that are hard-wired to serial devices.
>
> Signed-off-by: Christoph Fritz <christoph.fritz@hexdev.de>
> ---
> .../bindings/net/can/hexdev,lin-serdev.yaml | 32 +++++++++++++++++++
> 1 file changed, 32 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/net/can/hexdev,lin-serdev.yaml
>
> diff --git a/Documentation/devicetree/bindings/net/can/hexdev,lin-serdev.yaml b/Documentation/devicetree/bindings/net/can/hexdev,lin-serdev.yaml
> new file mode 100644
> index 0000000000000..c178eb9be1391
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/can/hexdev,lin-serdev.yaml
> @@ -0,0 +1,32 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/net/can/hexdev,lin-serdev.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Serial LIN Adapter
> +
> +description:
> + LIN transceiver, mostly hard-wired to a serial device, used for communication
> + on a LIN bus.
> + For more details on an adapter, visit <https://hexdev.de/hexlin#tty>.
> +
> +maintainers:
> + - Christoph Fritz <christoph.fritz@hexdev.de>
> +
> +properties:
> + compatible:
> + const: hexdev,lin-serdev
Maybe I've just missed something on earlier versions that I didn't
read, but the name of the device on the website you link is "hexLIN",
so why is "lin-serdev" used here instead?
> +
> +required:
> + - compatible
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + serial {
> + linbus {
> + compatible = "hexdev,lin-serdev";
> + };
> + };
> --
> 2.39.2
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* [PATCH v2] Input: elantech - fix touchpad state on resume for Lenovo N24
From: Jonathan Denose @ 2024-05-03 16:12 UTC (permalink / raw)
To: dmitry.torokhov; +Cc: gregkh, jdenose, jefferymiller, linux-input, linux-kernel
In-Reply-To: <ZjQeE643YAbK1hq5@google.com>
The Lenovo N24 on resume becomes stuck in a state where it
sends incorrect packets, causing elantech_packet_check_v4 to fail.
The only way for the device to resume sending the correct packets is for
it to be disabled and then re-enabled.
This change adds a dmi check to trigger this behavior on resume.
Signed-off-by: Jonathan Denose <jdenose@google.com>
---
Changes in v2:
- change ps2_sendbyte() calls to ps2_command()
drivers/input/mouse/elantech.c | 33 +++++++++++++++++++++++++++++++++
1 file changed, 33 insertions(+)
diff --git a/drivers/input/mouse/elantech.c b/drivers/input/mouse/elantech.c
index 4e38229404b4b..18f26315cae25 100644
--- a/drivers/input/mouse/elantech.c
+++ b/drivers/input/mouse/elantech.c
@@ -1476,6 +1476,23 @@ static void elantech_disconnect(struct psmouse *psmouse)
psmouse->private = NULL;
}
+/*
+ * Some hw_version 4 models fail to properly activate absolute mode on
+ * resume without going through disable/enable cycle.
+ */
+static const struct dmi_system_id elantech_needs_reenable[] = {
+#if defined(CONFIG_DMI) && defined(CONFIG_X86)
+ {
+ /* Lenovo N24 */
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
+ DMI_MATCH(DMI_PRODUCT_NAME, "81AF"),
+ },
+ },
+#endif
+ { }
+};
+
/*
* Put the touchpad back into absolute mode when reconnecting
*/
@@ -1486,6 +1503,22 @@ static int elantech_reconnect(struct psmouse *psmouse)
if (elantech_detect(psmouse, 0))
return -1;
+ if (dmi_check_system(elantech_needs_reenable)) {
+ int err;
+
+ err = ps2_command(&psmouse->ps2dev, NULL, PSMOUSE_CMD_DISABLE);
+
+ if (err)
+ psmouse_warn(psmouse, "Failed to deactivate mouse on %s: %d\n",
+ psmouse->ps2dev.serio->phys, err);
+
+ err = ps2_command(&psmouse->ps2dev, NULL, PSMOUSE_CMD_ENABLE);
+
+ if (err)
+ psmouse_warn(psmouse, "Failed to reactivate mouse on %s: %d\n",
+ psmouse->ps2dev.serio->phys, err);
+ }
+
if (elantech_set_absolute_mode(psmouse)) {
psmouse_err(psmouse,
"failed to put touchpad back into absolute mode.\n");
--
2.45.0.rc1.225.g2a3ae87e7f-goog
^ permalink raw reply related
* Re: [PATCH RESEND v10 4/5] Input: cs40l50 - Add support for the CS40L50 haptic driver
From: James Ogletree @ 2024-05-03 15:25 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: James Ogletree, robh+dt@kernel.org, Krzysztof Kozlowski,
Conor Dooley, Lee Jones, Mark Brown, Jeff LaBundy,
open list:CIRRUS LOGIC HAPTIC DRIVERS,
open list:SOUND - SOC LAYER / DYNAMIC AUDIO POWER MANAGEM...,
open list:INPUT (KEYBOARD, MOUSE, JOYSTICK, TOUCHSCREEN)...,
devicetree@vger.kernel.org
In-Reply-To: <A0925B5F-F8D0-4270-B510-385FE4A38F84@cirrus.com>
Hi Dmitry,
Trimming down my last reply to just the points that need your attention/ack.
I made some small edits for the sake of clarity.
> On Apr 16, 2024, at 6:28 PM, Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:
>
> On Mon, Apr 08, 2024 at 03:32:13PM +0000, James Ogletree wrote:
>>
>> +/* Describes an area in DSP memory populated by effects */
>> +struct cs40l50_bank {
>> + enum cs40l50_bank_type type;
>> + u32 base_index;
>> + u32 max_index;
>
> This looks like it is written to the device, so I think this needs
> proper endianness annotation. Is there any concern about padding between
> the type and base_index?
The structure itself is never written, so there is no concern about padding.
Only base_index is written, and the endianness conversion is handled by regmap.
>> +static void cs40l50_start_worker(struct work_struct *work)
>> +{
>> + struct cs40l50_work *work_data = container_of(work, struct cs40l50_work, work);
>> + struct cs40l50_vibra *vibra = work_data->vibra;
>> + struct cs40l50_effect *start_effect;
>> +
>> + if (pm_runtime_resume_and_get(vibra->dev) < 0)
>> + goto err_free;
>> +
>> + mutex_lock(&vibra->lock);
>> +
>> + start_effect = cs40l50_find_effect(work_data->effect->id, &vibra->effect_head);
>> + if (start_effect) {
>> + while (--work_data->count >= 0) {
>> + vibra->dsp.write(vibra->dev, vibra->regmap, start_effect->index);
>> + usleep_range(work_data->effect->replay.length,
>> + work_data->effect->replay.length + 100);
>
> If (I am reading this right you are spinning here playing the effect. It
> would be much better if you tracked outstanding number of replays for an
> effect and had a work re-scheduled that would play an instance.
> Similarly to what code in ff-memless.c is doing.
Yes, you are reading it right.
It appears that ff-memless.c handles repeats atomically, however for
reasons explained below, this driver must queue work for playback
executions.
This results in a possible scenario where if a playback is issued with a
high enough repeat value, an erase operation could arrive in the middle of
the chain of re-scheduling and disrupt the playbacks which have yet to be
queued. But with the current approach, the effect is guaranteed to repeat
any number of times without risk of being erased in the middle.
That’s my concern with adopting the re-scheduling approach for this
driver. Please let me know your thoughts.
>> +static int cs40l50_erase(struct input_dev *dev, int effect_id)
>> +{
>> + struct cs40l50_vibra *vibra = input_get_drvdata(dev);
>> + struct cs40l50_work work_data;
>> +
>> + work_data.vibra = vibra;
>> + work_data.effect = &dev->ff->effects[effect_id];
>> +
>> + INIT_WORK(&work_data.work, cs40l50_erase_worker);
>> +
>> + /* Push to workqueue to serialize with playbacks */
>> + queue_work(vibra->vibe_wq, &work_data.work);
>> + flush_work(&work_data.work);
>
> You already take the lock when you play, upload or erase an effect. Why
> do we need additional single-thread workqueue for this? Why do we need
> additional ordering and synchronization?
The workqueue is necessary is because playback blocks (via
pm_runtime_resume_and_get), and so playback must defer work. Upload
and erase are not called in atomic context, but without queueing those
executions, they could cut in front of playbacks waiting in the
workqueue.
But as the callbacks are already serialized via the workqueue, then I do
think the locks are excessive. That’s my thinking, let me know if it is
sound.
Best,
James
^ permalink raw reply
* Re: [PATCH v2 00/19] backlight: Constify lcd_ops
From: Lee Jones @ 2024-05-03 9:55 UTC (permalink / raw)
To: Daniel Thompson, Jingoo Han, Helge Deller, Bruno Prémont,
Jiri Kosina, Benjamin Tissoires, Alexander Shiyan, Sascha Hauer,
Pengutronix Kernel Team, Shawn Guo, Fabio Estevam,
Krzysztof Kozlowski
Cc: dri-devel, linux-fbdev, linux-kernel, linux-input,
linux-arm-kernel, imx, linux-omap, Thomas Zimmermann,
Thomas Weißschuh
In-Reply-To: <20240503094319.GP1227636@google.com>
On Fri, 03 May 2024, Lee Jones wrote:
> On Fri, 03 May 2024, Lee Jones wrote:
>
> > On Wed, 24 Apr 2024 08:33:26 +0200, Krzysztof Kozlowski wrote:
> > > Changes in v2:
> > > - Collect tags, including wrongly places Thomas' tag (which requires me
> > > to manually edit 15 other patches to drop it).
> > > - Combine here checkpatch patch:
> > > https://lore.kernel.org/all/20240414185440.288812-1-krzk@kernel.org/
> > > - Link to v1: https://lore.kernel.org/r/20240414-video-backlight-lcd-ops-v1-0-9b37fcbf546a@kernel.org
> > >
> > > [...]
> >
> > Applied, thanks!
> >
> > [01/19] backlight: Constify lcd_ops
> > commit: 9258e31adebf0ccf0797867841f3f9800695dba2
> > [02/19] backlight: ams369fg06: Constify lcd_ops
> > commit: 8053c4fa200c3fef07859bac9469ad3f26f5aba1
> > [03/19] backlight: corgi_lcd: Constify lcd_ops
> > commit: 18c5d4ab9f6312f2c9c6c409287d552112db810a
> > [04/19] backlight: hx8357: Constify lcd_ops
> > commit: 1d669c1998b559393ec2eaac0449f4989a255049
> > [05/19] backlight: ili922x: Constify lcd_ops
> > commit: e77fef89de954b1557cb91b64696cd4fc06c80ad
> > [06/19] backlight: ili9320: Constify lcd_ops
> > commit: 06cfc92faa1eabb2ea226c58d6fd0b5ab117ee39
> > [07/19] backlight: jornada720_lcd: Constify lcd_ops
> > commit: a54b4999dc204bc5839bb70602078c7c8e4a5010
> > [08/19] backlight: l4f00242t03: Constify lcd_ops
> > commit: 657e6c1b270e9f4a890059f5d08a08ea842fa1a8
> > [09/19] backlight: lms283gf05: Constify lcd_ops
> > commit: 66e5a10818fd332e973d36429e36f4c436a86a91
> > [10/19] backlight: lms501kf03: Constify lcd_ops
> > commit: 31c205d1e8426dd0cce0143c500ff1ff71fe64d1
> > [11/19] backlight: ltv350qv: Constify lcd_ops
> > commit: 24424f84d7568d9d794657622e080b1cba1e9290
> > [12/19] backlight: otm3225a: Constify lcd_ops
> > commit: 02949072ee8fb6141cd8ac2be9867ef466580ddb
> > [13/19] backlight: platform_lcd: Constify lcd_ops
> > commit: d217a8d5a39851caa16996756682715c9debb4a9
> > [14/19] backlight: tdo24m: Constify lcd_ops
> > commit: c7a1809d1982f671e66a4b1c1ffd8bdd5ba260aa
> > [15/19] HID: picoLCD: Constify lcd_ops
> > commit: 238724635763e7c5d82c0581b0c49e5dfdd5505a
> > [16/19] fbdev: clps711x: Constify lcd_ops
> > commit: 55d9a955375af3b3fd5725a9b5cbc658d4bdd244
> > [17/19] fbdev: imx: Constify lcd_ops
> > commit: a6abbb5783345c4c7cc9fbd583b81e167bd0207d
> > [18/19] fbdev: omap: lcd_ams_delta: Constify lcd_ops
> > commit: ca991e8e096c9f0cff0300289e2d4813192b8ef3
> > [19/19] const_structs.checkpatch: add lcd_ops
> > commit: f02aeccbec6108d768f54d31e7cb48b06c0e3814
>
> Please ensure you use checkpatch.pl before submitting:
>
> ====
>
> ERROR: Does not appear to be a unified-diff format patch
>
> total: 1 errors, 0 warnings, 0 lines checked
>
> NOTE: For some of the reported defects, checkpatch may be able to
> mechanically convert to the typical style using --fix or --fix-inplace.
>
> "[PATCH v2 19/19] const_structs.checkpatch: add lcd_ops" has style problems, please review.
>
> NOTE: If any of the errors are false positives, please report
> them to the maintainer, see CHECKPATCH in MAINTAINERS.
> WARNING: Missing commit description - Add an appropriate one
>
> ERROR: Missing Signed-off-by: line(s)
>
> total: 1 errors, 1 warnings, 7 lines checked
>
> NOTE: For some of the reported defects, checkpatch may be able to
> mechanically convert to the typical style using --fix or --fix-inplace.
>
> Your patch has style problems, please review.
>
> NOTE: If any of the errors are false positives, please report
> them to the maintainer, see CHECKPATCH in MAINTAINERS.
>
> ====
>
> Some of them are a little odd. I fixed the relevant ones
Change of plan - I actually can't find anything to fix.
All of this reports seem bogus!
--
Lee Jones [李琼斯]
^ permalink raw reply
* Re: [PATCH v2 00/19] backlight: Constify lcd_ops
From: Lee Jones @ 2024-05-03 9:43 UTC (permalink / raw)
To: Daniel Thompson, Jingoo Han, Helge Deller, Bruno Prémont,
Jiri Kosina, Benjamin Tissoires, Alexander Shiyan, Sascha Hauer,
Pengutronix Kernel Team, Shawn Guo, Fabio Estevam,
Krzysztof Kozlowski
Cc: dri-devel, linux-fbdev, linux-kernel, linux-input,
linux-arm-kernel, imx, linux-omap, Thomas Zimmermann,
Thomas Weißschuh
In-Reply-To: <171472928415.1323021.3458121588308140519.b4-ty@kernel.org>
On Fri, 03 May 2024, Lee Jones wrote:
> On Wed, 24 Apr 2024 08:33:26 +0200, Krzysztof Kozlowski wrote:
> > Changes in v2:
> > - Collect tags, including wrongly places Thomas' tag (which requires me
> > to manually edit 15 other patches to drop it).
> > - Combine here checkpatch patch:
> > https://lore.kernel.org/all/20240414185440.288812-1-krzk@kernel.org/
> > - Link to v1: https://lore.kernel.org/r/20240414-video-backlight-lcd-ops-v1-0-9b37fcbf546a@kernel.org
> >
> > [...]
>
> Applied, thanks!
>
> [01/19] backlight: Constify lcd_ops
> commit: 9258e31adebf0ccf0797867841f3f9800695dba2
> [02/19] backlight: ams369fg06: Constify lcd_ops
> commit: 8053c4fa200c3fef07859bac9469ad3f26f5aba1
> [03/19] backlight: corgi_lcd: Constify lcd_ops
> commit: 18c5d4ab9f6312f2c9c6c409287d552112db810a
> [04/19] backlight: hx8357: Constify lcd_ops
> commit: 1d669c1998b559393ec2eaac0449f4989a255049
> [05/19] backlight: ili922x: Constify lcd_ops
> commit: e77fef89de954b1557cb91b64696cd4fc06c80ad
> [06/19] backlight: ili9320: Constify lcd_ops
> commit: 06cfc92faa1eabb2ea226c58d6fd0b5ab117ee39
> [07/19] backlight: jornada720_lcd: Constify lcd_ops
> commit: a54b4999dc204bc5839bb70602078c7c8e4a5010
> [08/19] backlight: l4f00242t03: Constify lcd_ops
> commit: 657e6c1b270e9f4a890059f5d08a08ea842fa1a8
> [09/19] backlight: lms283gf05: Constify lcd_ops
> commit: 66e5a10818fd332e973d36429e36f4c436a86a91
> [10/19] backlight: lms501kf03: Constify lcd_ops
> commit: 31c205d1e8426dd0cce0143c500ff1ff71fe64d1
> [11/19] backlight: ltv350qv: Constify lcd_ops
> commit: 24424f84d7568d9d794657622e080b1cba1e9290
> [12/19] backlight: otm3225a: Constify lcd_ops
> commit: 02949072ee8fb6141cd8ac2be9867ef466580ddb
> [13/19] backlight: platform_lcd: Constify lcd_ops
> commit: d217a8d5a39851caa16996756682715c9debb4a9
> [14/19] backlight: tdo24m: Constify lcd_ops
> commit: c7a1809d1982f671e66a4b1c1ffd8bdd5ba260aa
> [15/19] HID: picoLCD: Constify lcd_ops
> commit: 238724635763e7c5d82c0581b0c49e5dfdd5505a
> [16/19] fbdev: clps711x: Constify lcd_ops
> commit: 55d9a955375af3b3fd5725a9b5cbc658d4bdd244
> [17/19] fbdev: imx: Constify lcd_ops
> commit: a6abbb5783345c4c7cc9fbd583b81e167bd0207d
> [18/19] fbdev: omap: lcd_ams_delta: Constify lcd_ops
> commit: ca991e8e096c9f0cff0300289e2d4813192b8ef3
> [19/19] const_structs.checkpatch: add lcd_ops
> commit: f02aeccbec6108d768f54d31e7cb48b06c0e3814
Please ensure you use checkpatch.pl before submitting:
====
ERROR: Does not appear to be a unified-diff format patch
total: 1 errors, 0 warnings, 0 lines checked
NOTE: For some of the reported defects, checkpatch may be able to
mechanically convert to the typical style using --fix or --fix-inplace.
"[PATCH v2 19/19] const_structs.checkpatch: add lcd_ops" has style problems, please review.
NOTE: If any of the errors are false positives, please report
them to the maintainer, see CHECKPATCH in MAINTAINERS.
WARNING: Missing commit description - Add an appropriate one
ERROR: Missing Signed-off-by: line(s)
total: 1 errors, 1 warnings, 7 lines checked
NOTE: For some of the reported defects, checkpatch may be able to
mechanically convert to the typical style using --fix or --fix-inplace.
Your patch has style problems, please review.
NOTE: If any of the errors are false positives, please report
them to the maintainer, see CHECKPATCH in MAINTAINERS.
====
Some of them are a little odd. I fixed the relevant ones
Submitted for build testing.
If successful, I'll submit a PR for the other maintainers soon.
--
Lee Jones [李琼斯]
^ 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