* [PATCH v3 06/12] HID: logitech-hidpp: Move hidpp_overwrite_name() to before connect check
From: Hans de Goede @ 2023-10-10 10:20 UTC (permalink / raw)
To: Filipe Laíns, Bastien Nocera, Jiri Kosina,
Benjamin Tissoires
Cc: Hans de Goede, linux-input
In-Reply-To: <20231010102029.111003-1-hdegoede@redhat.com>
Move the hidpp_overwrite_name() call to before the connect check, this
puts it at the same place in the probe() order as hidpp_serial_init()
which seems more logical. This should not make a difference since this
is in the non-unifying path and only unifying devices can be probed
in non-connected state.
This is a preparation patch for moving all connected state handling
to hidpp_connect_event().
Doing this before the connect check requires dropping the protocol
version check since protocol_major is not set yet now. Instead
this relies on hidpp_root_get_feature(HIDPP_PAGE_GET_DEVICE_NAME_TYPE)
failing on older devices, just like how hidpp_get_serial() relies on
hidpp_root_get_feature(HIDPP_PAGE_DEVICE_INFORMATION) failing on older
devices. So this again makes the code more consistent.
Also stop printing an error on failure now, since with the proto
version check gone failures are expected to happen on older devices.
This series has been tested on the following devices:
Logitech Bluetooth Laser Travel Mouse (bluetooth, HID++ 1.0)
Logitech M720 Triathlon (bluetooth, HID++ 4.5)
Logitech M720 Triathlon (unifying, HID++ 4.5)
Logitech K400 Pro (unifying, HID++ 4.1)
Logitech K270 (eQUAD nano Lite, HID++ 2.0)
Logitech M185 (eQUAD nano Lite, HID++ 4.5)
Logitech LX501 keyboard (27 Mhz, HID++ builtin scroll-wheel, HID++ 1.0)
Logitech M-RAZ105 mouse (27 Mhz, HID++ extra mouse buttons, HID++ 1.0)
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Note: The weird unbalanced curly braces in the "else { ... }" go away
in the next patch in the series.
---
drivers/hid/hid-logitech-hidpp.c | 21 ++++++++-------------
1 file changed, 8 insertions(+), 13 deletions(-)
diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
index dd67749301a8..799d1a429d81 100644
--- a/drivers/hid/hid-logitech-hidpp.c
+++ b/drivers/hid/hid-logitech-hidpp.c
@@ -4135,19 +4135,12 @@ static void hidpp_overwrite_name(struct hid_device *hdev)
struct hidpp_device *hidpp = hid_get_drvdata(hdev);
char *name;
- if (hidpp->protocol_major < 2)
- return;
-
name = hidpp_get_device_name(hidpp);
-
- if (!name) {
- hid_err(hdev, "unable to retrieve the name of the device");
- } else {
+ if (name) {
dbg_hid("HID++: Got name: %s\n", name);
snprintf(hdev->name, sizeof(hdev->name), "%s", name);
+ kfree(name);
}
-
- kfree(name);
}
static int hidpp_input_open(struct input_dev *dev)
@@ -4483,8 +4476,12 @@ static int hidpp_probe(struct hid_device *hdev, const struct hid_device_id *id)
if (hidpp->quirks & HIDPP_QUIRK_UNIFYING)
hidpp_unifying_init(hidpp);
- else if (hid_is_usb(hidpp->hid_dev))
- hidpp_serial_init(hidpp);
+ else {
+ if (hid_is_usb(hidpp->hid_dev))
+ hidpp_serial_init(hidpp);
+
+ hidpp_overwrite_name(hdev);
+ }
connected = hidpp_root_get_protocol_version(hidpp) == 0;
atomic_set(&hidpp->connected, connected);
@@ -4494,8 +4491,6 @@ static int hidpp_probe(struct hid_device *hdev, const struct hid_device_id *id)
hid_err(hdev, "Device not connected");
goto hid_hw_init_fail;
}
-
- hidpp_overwrite_name(hdev);
}
schedule_work(&hidpp->work);
--
2.41.0
^ permalink raw reply related
* [PATCH v3 08/12] HID: logitech-hidpp: Remove connected check for non-unifying devices
From: Hans de Goede @ 2023-10-10 10:20 UTC (permalink / raw)
To: Filipe Laíns, Bastien Nocera, Jiri Kosina,
Benjamin Tissoires
Cc: Hans de Goede, linux-input
In-Reply-To: <20231010102029.111003-1-hdegoede@redhat.com>
Non-unifying devices (USB, Bluetooth) are always connected during
probe(), remove the unnecessary connected check.
This is a preparation patch for moving all connected state handling
to hidpp_connect_event().
This series has been tested on the following devices:
Logitech Bluetooth Laser Travel Mouse (bluetooth, HID++ 1.0)
Logitech M720 Triathlon (bluetooth, HID++ 4.5)
Logitech M720 Triathlon (unifying, HID++ 4.5)
Logitech K400 Pro (unifying, HID++ 4.1)
Logitech K270 (eQUAD nano Lite, HID++ 2.0)
Logitech M185 (eQUAD nano Lite, HID++ 4.5)
Logitech LX501 keyboard (27 Mhz, HID++ builtin scroll-wheel, HID++ 1.0)
Logitech M-RAZ105 mouse (27 Mhz, HID++ extra mouse buttons, HID++ 1.0)
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
drivers/hid/hid-logitech-hidpp.c | 7 -------
1 file changed, 7 deletions(-)
diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
index 55fc80f3be05..21f41c147f9b 100644
--- a/drivers/hid/hid-logitech-hidpp.c
+++ b/drivers/hid/hid-logitech-hidpp.c
@@ -4487,13 +4487,6 @@ static int hidpp_probe(struct hid_device *hdev, const struct hid_device_id *id)
connected = hidpp_root_get_protocol_version(hidpp) == 0;
atomic_set(&hidpp->connected, connected);
- if (!(hidpp->quirks & HIDPP_QUIRK_UNIFYING)) {
- if (!connected) {
- ret = -ENODEV;
- hid_err(hdev, "Device not connected");
- goto hid_hw_init_fail;
- }
- }
schedule_work(&hidpp->work);
flush_work(&hidpp->work);
--
2.41.0
^ permalink raw reply related
* [PATCH v3 00/12] HID: logitech-hidpp: Avoid hidpp_connect_event() running while probe() restarts IO
From: Hans de Goede @ 2023-10-10 10:20 UTC (permalink / raw)
To: Filipe Laíns, Bastien Nocera, Jiri Kosina,
Benjamin Tissoires
Cc: Hans de Goede, linux-input
Hi Benjamin,
As dicussed here is v3 of my series to rework / cleanup the hidpp
probing code.
Note the $subject of the cover-letter is not entirely accurate anymore,
but I kept it the same since this is the successor of series with
the same subject.
Changes in v3:
- Call hid_connect() to connect the hid-input and hidraw drivers
after retrieving the name + serial instead of restarting IO
- Shuffle the series order to put patches with Fixes tags at the start
- Squash 2 changes to G920 handling into one
- Since probe() now no longer restarts IO the following patches have been dropped:
"HID: logitech-hidpp: Add a hidpp_connect_and_start() helper"
"HID: logitech-hidpp: Move the connected check to after restarting IO"
"HID: logitech-hidpp: Avoid hidpp_connect_event() running while probe() restarts IO"
- Add "HID: logitech-hidpp: Drop HIDPP_QUIRK_UNIFYING"
Regards,
Hans
Hans de Goede (12):
HID: logitech-hidpp: Don't restart IO, instead defer hid_connect()
only
HID: logitech-hidpp: Revert "Don't restart communication if not
necessary"
HID: logitech-hidpp: Move get_wireless_feature_index() check to
hidpp_connect_event()
HID: logitech-hidpp: Remove wtp_get_config() call from probe()
HID: logitech-hidpp: Move g920_get_config() to just before
hidpp_ff_init()
HID: logitech-hidpp: Move hidpp_overwrite_name() to before connect
check
HID: logitech-hidpp: Add hidpp_non_unifying_init() helper
HID: logitech-hidpp: Remove connected check for non-unifying devices
HID: logitech-hidpp: Remove unused connected param from *_connect()
HID: logitech-hidpp: Fix connect event race
HID: logitech-hidpp: Drop delayed_work_cb()
HID: logitech-hidpp: Drop HIDPP_QUIRK_UNIFYING
drivers/hid/hid-logitech-hidpp.c | 173 +++++++++++--------------------
1 file changed, 61 insertions(+), 112 deletions(-)
--
2.41.0
^ permalink raw reply
* [PATCH v3 07/12] HID: logitech-hidpp: Add hidpp_non_unifying_init() helper
From: Hans de Goede @ 2023-10-10 10:20 UTC (permalink / raw)
To: Filipe Laíns, Bastien Nocera, Jiri Kosina,
Benjamin Tissoires
Cc: Hans de Goede, linux-input
In-Reply-To: <20231010102029.111003-1-hdegoede@redhat.com>
Turn hidpp_overwrite_name() into a hidpp_non_unifying_init() helper
which takes care of setting both the name and the serial for non
unifying devices.
This mirrors the hidpp_unifying_init() helper for unifying devices.
This series has been tested on the following devices:
Logitech Bluetooth Laser Travel Mouse (bluetooth, HID++ 1.0)
Logitech M720 Triathlon (bluetooth, HID++ 4.5)
Logitech M720 Triathlon (unifying, HID++ 4.5)
Logitech K400 Pro (unifying, HID++ 4.1)
Logitech K270 (eQUAD nano Lite, HID++ 2.0)
Logitech M185 (eQUAD nano Lite, HID++ 4.5)
Logitech LX501 keyboard (27 Mhz, HID++ builtin scroll-wheel, HID++ 1.0)
Logitech M-RAZ105 mouse (27 Mhz, HID++ extra mouse buttons, HID++ 1.0)
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
drivers/hid/hid-logitech-hidpp.c | 18 ++++++++++--------
1 file changed, 10 insertions(+), 8 deletions(-)
diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
index 799d1a429d81..55fc80f3be05 100644
--- a/drivers/hid/hid-logitech-hidpp.c
+++ b/drivers/hid/hid-logitech-hidpp.c
@@ -4130,11 +4130,16 @@ static int hidpp_initialize_battery(struct hidpp_device *hidpp)
return ret;
}
-static void hidpp_overwrite_name(struct hid_device *hdev)
+/* Get name + serial for USB and Bluetooth HID++ devices */
+static void hidpp_non_unifying_init(struct hidpp_device *hidpp)
{
- struct hidpp_device *hidpp = hid_get_drvdata(hdev);
+ struct hid_device *hdev = hidpp->hid_dev;
char *name;
+ /* Bluetooth devices already have their serialnr set */
+ if (hid_is_usb(hdev))
+ hidpp_serial_init(hidpp);
+
name = hidpp_get_device_name(hidpp);
if (name) {
dbg_hid("HID++: Got name: %s\n", name);
@@ -4474,14 +4479,11 @@ static int hidpp_probe(struct hid_device *hdev, const struct hid_device_id *id)
/* Allow incoming packets */
hid_device_io_start(hdev);
+ /* Get name + serial, store in hdev->name + hdev->uniq */
if (hidpp->quirks & HIDPP_QUIRK_UNIFYING)
hidpp_unifying_init(hidpp);
- else {
- if (hid_is_usb(hidpp->hid_dev))
- hidpp_serial_init(hidpp);
-
- hidpp_overwrite_name(hdev);
- }
+ else
+ hidpp_non_unifying_init(hidpp);
connected = hidpp_root_get_protocol_version(hidpp) == 0;
atomic_set(&hidpp->connected, connected);
--
2.41.0
^ permalink raw reply related
* [PATCH v3 01/12] HID: logitech-hidpp: Don't restart IO, instead defer hid_connect() only
From: Hans de Goede @ 2023-10-10 10:20 UTC (permalink / raw)
To: Filipe Laíns, Bastien Nocera, Jiri Kosina,
Benjamin Tissoires
Cc: Hans de Goede, linux-input, Benjamin Tissoires
In-Reply-To: <20231010102029.111003-1-hdegoede@redhat.com>
Restarting IO causes 2 problems:
1. Some devices do not like IO being restarted this was addressed in
commit 498ba2069035 ("HID: logitech-hidpp: Don't restart communication
if not necessary"), but that change has issues of its own and needs to
be reverted.
2. Restarting IO and specifically calling hid_device_io_stop() causes
received packets to be missed, which may cause connect-events to
get missed.
Restarting IO was introduced in commit 91cf9a98ae41 ("HID: logitech-hidpp:
make .probe usbhid capable") to allow to retrieve the device's name and
serial number and store these in hdev->name and hdev->uniq before
connecting any hid subdrivers (hid-input, hidraw) exporting this info
to userspace.
But this does not require restarting IO, this merely requires deferring
calling hid_connect(). Calling hid_hw_start() with a connect-mask of
0 makes it skip calling hid_connect(), so hidpp_probe() can simply call
hid_connect() later without needing to restart IO.
Remove the stop + restart of IO and instead just call hid_connect() later
to avoid the issues caused by restarting IO.
Now that IO is no longer stopped, hid_hw_close() must be called at the end
of probe() to balance the hid_hw_open() done at the beginning probe().
This series has been tested on the following devices:
Logitech Bluetooth Laser Travel Mouse (bluetooth, HID++ 1.0)
Logitech M720 Triathlon (bluetooth, HID++ 4.5)
Logitech M720 Triathlon (unifying, HID++ 4.5)
Logitech K400 Pro (unifying, HID++ 4.1)
Logitech K270 (eQUAD nano Lite, HID++ 2.0)
Logitech M185 (eQUAD nano Lite, HID++ 4.5)
Logitech LX501 keyboard (27 Mhz, HID++ builtin scroll-wheel, HID++ 1.0)
Logitech M-RAZ105 mouse (27 Mhz, HID++ extra mouse buttons, HID++ 1.0)
Fixes: 498ba2069035 ("HID: logitech-hidpp: Don't restart communication if not necessary")
Suggested-by: Benjamin Tissoires <bentiss@kernel.org>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
drivers/hid/hid-logitech-hidpp.c | 22 ++++++++++++----------
1 file changed, 12 insertions(+), 10 deletions(-)
diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
index a209d51bd247..aa4f232c4518 100644
--- a/drivers/hid/hid-logitech-hidpp.c
+++ b/drivers/hid/hid-logitech-hidpp.c
@@ -4460,8 +4460,10 @@ static int hidpp_probe(struct hid_device *hdev, const struct hid_device_id *id)
hdev->name);
/*
- * Plain USB connections need to actually call start and open
- * on the transport driver to allow incoming data.
+ * First call hid_hw_start(hdev, 0) to allow IO without connecting any
+ * hid subdrivers (hid-input, hidraw). This allows retrieving the dev's
+ * name and serial number and store these in hdev->name and hdev->uniq,
+ * before the hid-input and hidraw drivers expose these to userspace.
*/
ret = hid_hw_start(hdev, will_restart ? 0 : connect_mask);
if (ret) {
@@ -4519,19 +4521,14 @@ static int hidpp_probe(struct hid_device *hdev, const struct hid_device_id *id)
flush_work(&hidpp->work);
if (will_restart) {
- /* Reset the HID node state */
- hid_device_io_stop(hdev);
- hid_hw_close(hdev);
- hid_hw_stop(hdev);
-
if (hidpp->quirks & HIDPP_QUIRK_DELAYED_INIT)
connect_mask &= ~HID_CONNECT_HIDINPUT;
/* Now export the actual inputs and hidraw nodes to the world */
- ret = hid_hw_start(hdev, connect_mask);
+ ret = hid_connect(hdev, connect_mask);
if (ret) {
- hid_err(hdev, "%s:hid_hw_start returned error\n", __func__);
- goto hid_hw_start_fail;
+ hid_err(hdev, "%s:hid_connect returned error %d\n", __func__, ret);
+ goto hid_hw_init_fail;
}
}
@@ -4543,6 +4540,11 @@ static int hidpp_probe(struct hid_device *hdev, const struct hid_device_id *id)
ret);
}
+ /*
+ * This relies on logi_dj_ll_close() being a no-op so that DJ connection
+ * events will still be received.
+ */
+ hid_hw_close(hdev);
return ret;
hid_hw_init_fail:
--
2.41.0
^ permalink raw reply related
* [PATCH v3 03/12] HID: logitech-hidpp: Move get_wireless_feature_index() check to hidpp_connect_event()
From: Hans de Goede @ 2023-10-10 10:20 UTC (permalink / raw)
To: Filipe Laíns, Bastien Nocera, Jiri Kosina,
Benjamin Tissoires
Cc: Hans de Goede, linux-input
In-Reply-To: <20231010102029.111003-1-hdegoede@redhat.com>
Calling get_wireless_feature_index() from probe() causes
the wireless_feature_index to only get set for unifying devices which
are already connected at probe() time. It does not get set for devices
which connect later.
Fix this by moving get_wireless_feature_index() to hidpp_connect_event(),
this does not make a difference for devices connected at probe() since
probe() will queue the hidpp_connect_event() for those at probe time.
This series has been tested on the following devices:
Logitech Bluetooth Laser Travel Mouse (bluetooth, HID++ 1.0)
Logitech M720 Triathlon (bluetooth, HID++ 4.5)
Logitech M720 Triathlon (unifying, HID++ 4.5)
Logitech K400 Pro (unifying, HID++ 4.1)
Logitech K270 (eQUAD nano Lite, HID++ 2.0)
Logitech M185 (eQUAD nano Lite, HID++ 4.5)
Logitech LX501 keyboard (27 Mhz, HID++ builtin scroll-wheel, HID++ 1.0)
Logitech M-RAZ105 mouse (27 Mhz, HID++ extra mouse buttons, HID++ 1.0)
Fixes: 0da0a63b7cba ("HID: logitech-hidpp: Support WirelessDeviceStatus connect events")
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
drivers/hid/hid-logitech-hidpp.c | 20 +++++++++-----------
1 file changed, 9 insertions(+), 11 deletions(-)
diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
index c582d19b11d6..7bf12ca0eb4a 100644
--- a/drivers/hid/hid-logitech-hidpp.c
+++ b/drivers/hid/hid-logitech-hidpp.c
@@ -1835,15 +1835,14 @@ static int hidpp_battery_get_property(struct power_supply *psy,
/* -------------------------------------------------------------------------- */
#define HIDPP_PAGE_WIRELESS_DEVICE_STATUS 0x1d4b
-static int hidpp_set_wireless_feature_index(struct hidpp_device *hidpp)
+static int hidpp_get_wireless_feature_index(struct hidpp_device *hidpp, u8 *feature_index)
{
u8 feature_type;
int ret;
ret = hidpp_root_get_feature(hidpp,
HIDPP_PAGE_WIRELESS_DEVICE_STATUS,
- &hidpp->wireless_feature_index,
- &feature_type);
+ feature_index, &feature_type);
return ret;
}
@@ -4249,6 +4248,13 @@ static void hidpp_connect_event(struct hidpp_device *hidpp)
}
}
+ if (hidpp->protocol_major >= 2) {
+ u8 feature_index;
+
+ if (!hidpp_get_wireless_feature_index(hidpp, &feature_index))
+ hidpp->wireless_feature_index = feature_index;
+ }
+
if (hidpp->name == hdev->name && hidpp->protocol_major >= 2) {
name = hidpp_get_device_name(hidpp);
if (name) {
@@ -4493,14 +4499,6 @@ static int hidpp_probe(struct hid_device *hdev, const struct hid_device_id *id)
hidpp_overwrite_name(hdev);
}
- if (connected && hidpp->protocol_major >= 2) {
- ret = hidpp_set_wireless_feature_index(hidpp);
- if (ret == -ENOENT)
- hidpp->wireless_feature_index = 0;
- else if (ret)
- goto hid_hw_init_fail;
- }
-
if (connected && (hidpp->quirks & HIDPP_QUIRK_CLASS_WTP)) {
ret = wtp_get_config(hidpp);
if (ret)
--
2.41.0
^ permalink raw reply related
* [PATCH v3 05/12] HID: logitech-hidpp: Move g920_get_config() to just before hidpp_ff_init()
From: Hans de Goede @ 2023-10-10 10:20 UTC (permalink / raw)
To: Filipe Laíns, Bastien Nocera, Jiri Kosina,
Benjamin Tissoires
Cc: Hans de Goede, linux-input
In-Reply-To: <20231010102029.111003-1-hdegoede@redhat.com>
The data retrieved by g920_get_config() is only used by hidpp_ff_init().
Now that the hw is kept open till the end of probe() the g920_get_config()
call can be moved to just before hidpp_ff_init() to have all
the HIDPP_QUIRK_CLASS_G920 together in a single place.
This series has been tested on the following devices:
Logitech Bluetooth Laser Travel Mouse (bluetooth, HID++ 1.0)
Logitech M720 Triathlon (bluetooth, HID++ 4.5)
Logitech M720 Triathlon (unifying, HID++ 4.5)
Logitech K400 Pro (unifying, HID++ 4.1)
Logitech K270 (eQUAD nano Lite, HID++ 2.0)
Logitech M185 (eQUAD nano Lite, HID++ 4.5)
Logitech LX501 keyboard (27 Mhz, HID++ builtin scroll-wheel, HID++ 1.0)
Logitech M-RAZ105 mouse (27 Mhz, HID++ extra mouse buttons, HID++ 1.0)
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
drivers/hid/hid-logitech-hidpp.c | 14 ++++++--------
1 file changed, 6 insertions(+), 8 deletions(-)
diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
index 707fb9616fc5..dd67749301a8 100644
--- a/drivers/hid/hid-logitech-hidpp.c
+++ b/drivers/hid/hid-logitech-hidpp.c
@@ -4399,7 +4399,6 @@ static int hidpp_probe(struct hid_device *hdev, const struct hid_device_id *id)
int ret;
bool connected;
unsigned int connect_mask = HID_CONNECT_DEFAULT;
- struct hidpp_ff_private_data data;
/* report_fixup needs drvdata to be set before we call hid_parse */
hidpp = devm_kzalloc(&hdev->dev, sizeof(*hidpp), GFP_KERNEL);
@@ -4499,12 +4498,6 @@ static int hidpp_probe(struct hid_device *hdev, const struct hid_device_id *id)
hidpp_overwrite_name(hdev);
}
- if (connected && (hidpp->quirks & HIDPP_QUIRK_CLASS_G920)) {
- ret = g920_get_config(hidpp, &data);
- if (ret)
- goto hid_hw_init_fail;
- }
-
schedule_work(&hidpp->work);
flush_work(&hidpp->work);
@@ -4519,7 +4512,12 @@ static int hidpp_probe(struct hid_device *hdev, const struct hid_device_id *id)
}
if (hidpp->quirks & HIDPP_QUIRK_CLASS_G920) {
- ret = hidpp_ff_init(hidpp, &data);
+ struct hidpp_ff_private_data data;
+
+ ret = g920_get_config(hidpp, &data);
+ if (!ret)
+ ret = hidpp_ff_init(hidpp, &data);
+
if (ret)
hid_warn(hidpp->hid_dev,
"Unable to initialize force feedback support, errno %d\n",
--
2.41.0
^ permalink raw reply related
* [PATCH v3 02/12] HID: logitech-hidpp: Revert "Don't restart communication if not necessary"
From: Hans de Goede @ 2023-10-10 10:20 UTC (permalink / raw)
To: Filipe Laíns, Bastien Nocera, Jiri Kosina,
Benjamin Tissoires
Cc: Hans de Goede, linux-input
In-Reply-To: <20231010102029.111003-1-hdegoede@redhat.com>
Commit 91cf9a98ae41 ("HID: logitech-hidpp: make .probe usbhid capable")
makes hidpp_probe() first call hid_hw_start(hdev, 0) to allow IO
without connecting any hid subdrivers (hid-input, hidraw).
This is done to allow to retrieve the device's name and serial number
and store these in hdev->name and hdev->uniq.
Then later on IO was stopped and started again with hid_hw_start(hdev,
HID_CONNECT_DEFAULT) connecting hid-input and hidraw after the name
and serial number have been setup.
Commit 498ba2069035 ("HID: logitech-hidpp: Don't restart communication
if not necessary") changed the probe() code to only do the start with
a 0 connect-mask + restart later for unifying devices.
But for non unifying devices hdev->name and hdev->uniq are updated too.
So this change re-introduces the problem for which the start with
a 0 connect-mask + restart later behavior was introduced.
The previous patch in this series changes the unifying path to instead of
restarting IO only call hid_connect() later. This avoids possible issues
with restarting IO seen on non unifying devices.
Revert the change to limit the restart behavior to unifying devices to
fix hdev->name changing after userspace facing devices have already been
registered.
This series has been tested on the following devices:
Logitech Bluetooth Laser Travel Mouse (bluetooth, HID++ 1.0)
Logitech M720 Triathlon (bluetooth, HID++ 4.5)
Logitech M720 Triathlon (unifying, HID++ 4.5)
Logitech K400 Pro (unifying, HID++ 4.1)
Logitech K270 (eQUAD nano Lite, HID++ 2.0)
Logitech M185 (eQUAD nano Lite, HID++ 4.5)
Logitech LX501 keyboard (27 Mhz, HID++ builtin scroll-wheel, HID++ 1.0)
Logitech M-RAZ105 mouse (27 Mhz, HID++ extra mouse buttons, HID++ 1.0)
Fixes: 498ba2069035 ("HID: logitech-hidpp: Don't restart communication if not necessary")
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
drivers/hid/hid-logitech-hidpp.c | 24 ++++++++----------------
1 file changed, 8 insertions(+), 16 deletions(-)
diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
index aa4f232c4518..c582d19b11d6 100644
--- a/drivers/hid/hid-logitech-hidpp.c
+++ b/drivers/hid/hid-logitech-hidpp.c
@@ -4394,7 +4394,6 @@ static int hidpp_probe(struct hid_device *hdev, const struct hid_device_id *id)
bool connected;
unsigned int connect_mask = HID_CONNECT_DEFAULT;
struct hidpp_ff_private_data data;
- bool will_restart = false;
/* report_fixup needs drvdata to be set before we call hid_parse */
hidpp = devm_kzalloc(&hdev->dev, sizeof(*hidpp), GFP_KERNEL);
@@ -4445,10 +4444,6 @@ static int hidpp_probe(struct hid_device *hdev, const struct hid_device_id *id)
return ret;
}
- if (hidpp->quirks & HIDPP_QUIRK_DELAYED_INIT ||
- hidpp->quirks & HIDPP_QUIRK_UNIFYING)
- will_restart = true;
-
INIT_WORK(&hidpp->work, delayed_work_cb);
mutex_init(&hidpp->send_mutex);
init_waitqueue_head(&hidpp->wait);
@@ -4465,7 +4460,7 @@ static int hidpp_probe(struct hid_device *hdev, const struct hid_device_id *id)
* name and serial number and store these in hdev->name and hdev->uniq,
* before the hid-input and hidraw drivers expose these to userspace.
*/
- ret = hid_hw_start(hdev, will_restart ? 0 : connect_mask);
+ ret = hid_hw_start(hdev, 0);
if (ret) {
hid_err(hdev, "hw start failed\n");
goto hid_hw_start_fail;
@@ -4504,7 +4499,6 @@ static int hidpp_probe(struct hid_device *hdev, const struct hid_device_id *id)
hidpp->wireless_feature_index = 0;
else if (ret)
goto hid_hw_init_fail;
- ret = 0;
}
if (connected && (hidpp->quirks & HIDPP_QUIRK_CLASS_WTP)) {
@@ -4520,16 +4514,14 @@ static int hidpp_probe(struct hid_device *hdev, const struct hid_device_id *id)
schedule_work(&hidpp->work);
flush_work(&hidpp->work);
- if (will_restart) {
- if (hidpp->quirks & HIDPP_QUIRK_DELAYED_INIT)
- connect_mask &= ~HID_CONNECT_HIDINPUT;
+ if (hidpp->quirks & HIDPP_QUIRK_DELAYED_INIT)
+ connect_mask &= ~HID_CONNECT_HIDINPUT;
- /* Now export the actual inputs and hidraw nodes to the world */
- ret = hid_connect(hdev, connect_mask);
- if (ret) {
- hid_err(hdev, "%s:hid_connect returned error %d\n", __func__, ret);
- goto hid_hw_init_fail;
- }
+ /* Now export the actual inputs and hidraw nodes to the world */
+ ret = hid_connect(hdev, connect_mask);
+ if (ret) {
+ hid_err(hdev, "%s:hid_connect returned error %d\n", __func__, ret);
+ goto hid_hw_init_fail;
}
if (hidpp->quirks & HIDPP_QUIRK_CLASS_G920) {
--
2.41.0
^ permalink raw reply related
* Re: [PATCH v2 00/14] HID: logitech-hidpp: Avoid hidpp_connect_event() running while probe() restarts IO
From: Hans de Goede @ 2023-10-10 9:40 UTC (permalink / raw)
To: Benjamin Tissoires
Cc: Filipe Laíns, Bastien Nocera, Jiri Kosina,
Benjamin Tissoires, linux-input
In-Reply-To: <hjcpffahcxfqxfzyd7as2v75wpqbta2arj7dy3d2xwg7pz7q4g@4lv2dry2gcp7>
Hi,
On 10/10/23 11:36, Benjamin Tissoires wrote:
> On Oct 09 2023, Hans de Goede wrote:
<snip>
>>> However, the thing I am afraid is that commit 498ba2069035 ("HID:
>>> logitech-hidpp: Don't restart communication if not necessary") was
>>> fixing devices that did not like the hid_hw_stop/start. I can't find the
>>> bug numbers however... So with your series, we might breaking those
>>> once again.
>>>
>>> How about we do the following (in pseudo code):
>>> probe():
>>> hidpp_connect_and_start(connect_mask = 0)
>>> // retrieve name and serial
>>> hid_connect(connect_mask) // with connect_mask ensuring we don't
>>> // create inputs if HIDPP_QUIRK_DELAYED_INIT
>>> // is set, instead of stop/start
>>> hid_hw_close(hdev); // to balance hidpp_connect_and_start()
>>>
>>> I think the above should even remove the need for the
>>> enable_connect_event atomic_t given that now we are not restarting the
>>> devices at all.
>>
>> Interesting yes that looks good, any idea why this was not done
>> in commit 91cf9a98ae41 ("HID: logitech-hidpp: make .probe usbhid capable")
>> right away ?
>
> I tried to do a little bit of digging. Initially I thought that's
> because I was always doing that stop/start dance, and so I continued
> with that.
>
> But looking slightly more carefully, I think I understand why:
> - calling twice hid_connect should fail, because it'll create twice the
> sysfs nodes for the HID device itself
> - so we need to call hid_disconnect() first
> - this was done through hid_hw_stop() and given that the dj driver has
> empty stubs for start/stop, this was semantically the same.
>
> So using hid_hw_stop/start was a shortcut because DJ had empty stubs.
> But when I enabled direct use of hidpp from other transport layers, I
> did not realized that there was this glitch.
>
> And then I completely forgot to clean up that because "at the end of a
> HID driver we are supposed to call hid_hw_start()".
>
>>
>> Let me rework the series to use that tomorrow. This will probably also
>> allow dropping a bunch of the patches.
>
> Yeah, I think we should be able to remove the sync mechanism.
> And be careful to call hid_disconnect() before
> hid_connect(connect_mask), or you'll probably have errors if not when
> plugging the device, but when unplugging it for sure.
Note that hid_hw_start() never calls hid_connect() when called with
a connect_mask of 0, so just simply calling hid_connect() later should
be fine without needing to do a hid_disconnect() before:
int hid_hw_start(struct hid_device *hdev, unsigned int connect_mask)
{
int error;
error = hdev->ll_driver->start(hdev);
if (error)
return error;
if (connect_mask) {
error = hid_connect(hdev, connect_mask);
if (error) {
hdev->ll_driver->stop(hdev);
return error;
}
}
return 0;
}
EXPORT_SYMBOL_GPL(hid_hw_start);
Regards,
Hans
^ permalink raw reply
* Re: [PATCH v2 00/14] HID: logitech-hidpp: Avoid hidpp_connect_event() running while probe() restarts IO
From: Benjamin Tissoires @ 2023-10-10 9:36 UTC (permalink / raw)
To: Hans de Goede
Cc: Filipe Laíns, Bastien Nocera, Jiri Kosina,
Benjamin Tissoires, linux-input
In-Reply-To: <f28d42de-e2f3-bb2e-ed69-b7f21bcf06f9@redhat.com>
On Oct 09 2023, Hans de Goede wrote:
> Hi Benjamin,
>
> On 10/9/23 10:13, Benjamin Tissoires wrote:
> >
> > Hi Hans,
> >
> > On Oct 08 2023, Hans de Goede wrote:
> >> Hi,
> >>
> >> On 10/8/23 11:54, Hans de Goede wrote:
> >>> Hi Benjamin,
> >>>
> >>> Here is a v2 of my series to fix issues with hidpp_connect_event() running
> >>> while restarting IO, which now also fixes the issues you mentioned with
> >>> potentially missing connect events.
> >
> > Great, thanks a lot for this hard work.
>
> You're welcome.
>
> >>> This series is best explained by a brief sketch of how probe()
> >>> looks at the end of the series (1):
> >
> > TBH, I couldn't parse the following yesterday evening, but after looking
> > at all patches one by one I can now get it :)
>
> I'm glad you get it now :)
>
> >
> >>>
> >>> Prep work:
> >>>
> >>> 1. All code depending on a device being in connected state is moved to
> >>> the hidpp_connect_event() workqueue item
> >>>
> >>> 2. hidpp_connect_event() now checks the connected state itself by
> >>> checking that hidpp_root_get_protocol_version() succeeds, instead
> >>> of relying on possibly stale (racy) data in struct hidpp_device
> >>>
> >>> With this in place the new probe() sequence looks like this:
> >>>
> >>> 1. enable_connect_event flag starts at 0, this filters out / ignores any
> >>> connect-events in hidpp_raw_hidpp_event() avoiding
> >>> hidpp_connect_event() getting queued before the IO restart
> >>>
> >>> 2. IO is started with connect-mask = 0
> >>> this avoids hid-input and hidraw connecting while probe() is setting
> >>> hdev->name and hdev->uniq
> >>>
> >>> 3. Name and serialnr are retrieved and stored in hdev
> >>>
> >>> 4. IO is fully restarted (including hw_open + io_start, not just hw_start)
> >>> with the actual connect-mask.
> >>>
> >>> 5. enable_connect_event atomic_t is set to 1 to enable processing of
> >>> connect events.
> >>>
> >>> 6. hidpp_connect_event() is queued + flushed to query the connected state
> >>> and do the connect work if the device is connected.
> >>>
> >>> 7. probe() now ends with:
> >>>
> >>> /*
> >>> * This relies on logi_dj_ll_close() being a no-op so that
> >>> * DJ connection events will still be received.
> >>> */
> >>> hid_hw_close(hdev);
> >>>
> >>> Since on restarting IO it now is fully restarted so the hid_hw_open()
> >>> there needs to be balanced.
> >>>
> >>> This series now obviously is no longer 6.6 material, instead I hope we
> >>> can get this rework (and IMHO nice cleanup) into 6.7 .
> >
> > Yeah, not 6.6 anymore, but should be doable for 6.7.
> >
> >>>
> >>> Regards,
> >>>
> >>> Hans
> >>
> >> I forgot to add info on the list of devices I tested this on:
> >>
> >> Logitech Bluetooth Laser Travel Mouse (bluetooth, HID++ 1.0)
> >> Logitech M720 Triathlon (bluetooth, HID++ 4.5)
> >> Logitech K400 Pro (unifying, HID++ 4.1)
> >> Logitech K270 (eQUAD nano Lite, HID++ 2.0)
> >> Logitech M185 (eQUAD nano Lite, HID++ 4.5)
> >> Logitech Keyboard LX501 (27 Mhz, HID++ builtin scroll-wheel, HID++ 1.0)
> >> Logitech 27Mhz mouse (27 Mhz, HID++ extra mouse buttons, HID++ 1.0)
> >
> > We should probably add this list to the commit messages.
> > I'll need to test also myself on some problematic devices that have a
> > special case (WTP, USB wired, BLE).
>
> So you want to add this to all 14 commit messages ?
I'd like to have this list recorded in the git tree. In case we have to
do forensic once again. Whether it's added in all messages is up to you
I think. But honestly, maybe add those only when you actually tested the
patch. No?
>
> > Anyway, I'll have to test everything, but this looks like it's in a
> > better shape than previously.
> >
> > However, the thing I am afraid is that commit 498ba2069035 ("HID:
> > logitech-hidpp: Don't restart communication if not necessary") was
> > fixing devices that did not like the hid_hw_stop/start. I can't find the
> > bug numbers however... So with your series, we might breaking those
> > once again.
> >
> > How about we do the following (in pseudo code):
> > probe():
> > hidpp_connect_and_start(connect_mask = 0)
> > // retrieve name and serial
> > hid_connect(connect_mask) // with connect_mask ensuring we don't
> > // create inputs if HIDPP_QUIRK_DELAYED_INIT
> > // is set, instead of stop/start
> > hid_hw_close(hdev); // to balance hidpp_connect_and_start()
> >
> > I think the above should even remove the need for the
> > enable_connect_event atomic_t given that now we are not restarting the
> > devices at all.
>
> Interesting yes that looks good, any idea why this was not done
> in commit 91cf9a98ae41 ("HID: logitech-hidpp: make .probe usbhid capable")
> right away ?
I tried to do a little bit of digging. Initially I thought that's
because I was always doing that stop/start dance, and so I continued
with that.
But looking slightly more carefully, I think I understand why:
- calling twice hid_connect should fail, because it'll create twice the
sysfs nodes for the HID device itself
- so we need to call hid_disconnect() first
- this was done through hid_hw_stop() and given that the dj driver has
empty stubs for start/stop, this was semantically the same.
So using hid_hw_stop/start was a shortcut because DJ had empty stubs.
But when I enabled direct use of hidpp from other transport layers, I
did not realized that there was this glitch.
And then I completely forgot to clean up that because "at the end of a
HID driver we are supposed to call hid_hw_start()".
>
> Let me rework the series to use that tomorrow. This will probably also
> allow dropping a bunch of the patches.
Yeah, I think we should be able to remove the sync mechanism.
And be careful to call hid_disconnect() before
hid_connect(connect_mask), or you'll probably have errors if not when
plugging the device, but when unplugging it for sure.
Cheers,
Benjamin
>
> Regards,
>
> Hans
>
>
>
>
> >
> >>
> >> Regards,
> >>
> >> Hans
> >>
> >>
> >>
> >>> 1) For reviewing you may also want to apply the entire series and look
> >>> at the end result to help you understand why various intermediate steps
> >>> are taken.
> >>>
> >>>
> >>> Hans de Goede (14):
> >>> HID: logitech-hidpp: Revert "Don't restart communication if not
> >>> necessary"
> >>> HID: logitech-hidpp: Move hidpp_overwrite_name() to before connect
> >>> check
> >>> HID: logitech-hidpp: Add hidpp_non_unifying_init() helper
> >>> HID: logitech-hidpp: Remove connected check for non-unifying devices
> >>> HID: logitech-hidpp: Move get_wireless_feature_index() check to
> >>> hidpp_connect_event()
> >>> HID: logitech-hidpp: Remove wtp_get_config() call from probe()
> >>> HID: logitech-hidpp: Remove connected check for g920_get_config() call
> >>> HID: logitech-hidpp: Add a hidpp_connect_and_start() helper
> >>> HID: logitech-hidpp: Move the connected check to after restarting IO
> >>> HID: logitech-hidpp: Move g920_get_config() to just before
> >>> hidpp_ff_init()
> >>> HID: logitech-hidpp: Remove unused connected param from *_connect()
> >>> HID: logitech-hidpp: Fix connect event race
> >>> HID: logitech-hidpp: Avoid hidpp_connect_event() running while probe()
> >>> restarts IO
> >>> HID: logitech-hidpp: Drop delayed_work_cb()
> >>>
> >>> drivers/hid/hid-logitech-hidpp.c | 211 +++++++++++++------------------
> >>> 1 file changed, 91 insertions(+), 120 deletions(-)
> >>>
> >
> > I like when the total number of deletions is higher than the additions
> > :)
> >
> > Cheers,
> > Benjamin
> >
> >>
> >
>
^ permalink raw reply
* Re: [PATCH v2 2/3] dt-bindings: input: Add bindings for TouchNetix axiom touchscreen
From: kamel.bouhara @ 2023-10-10 7:15 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Henrik Rydberg, linux-input, linux-kernel, devicetree,
mark.satterthwaite, pedro.torruella, bartp, hannah.rossiter,
Thomas Petazzoni, Gregory Clement, bsp-development.geo
Hello Krzysztof,
Le 2023-10-09 17:05, Krzysztof Kozlowski a écrit :
> On 09/10/2023 15:44, Kamel Bouhara wrote:
>> Add the TouchNetix axiom I2C touchscreen device tree bindings
>> documentation.
>
> A nit, subject: drop second/last, redundant "bindings for". The
> "dt-bindings" prefix is already stating that these are bindings.
>
OK.
>> Signed-off-by: Kamel Bouhara <kamel.bouhara@bootlin.com>
>> ---
>> .../touchscreen/touchnetix,axiom-ax54a.yaml | 51
>> +++++++++++++++++++
>> MAINTAINERS | 6 +++
>> 2 files changed, 57 insertions(+)
>> create mode 100644
>> Documentation/devicetree/bindings/input/touchscreen/touchnetix,axiom-ax54a.yaml
>>
>> diff --git
>> a/Documentation/devicetree/bindings/input/touchscreen/touchnetix,axiom-ax54a.yaml
>> b/Documentation/devicetree/bindings/input/touchscreen/touchnetix,axiom-ax54a.yaml
>> new file mode 100644
>> index 000000000000..41201d7112a6
>> --- /dev/null
>> +++
>> b/Documentation/devicetree/bindings/input/touchscreen/touchnetix,axiom-ax54a.yaml
>> @@ -0,0 +1,51 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id:
>> http://devicetree.org/schemas/input/touchscreen/touchnetix,axiom-ax54a.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: TouchNetix Axiom series touchscreen controller
>> +
>> +maintainers:
>> + - Kamel Bouhara <kamel.bouhara@bootlin.com>
>> +
>> +properties:
>> + compatible:
>> + const: touchnetix,axiom-ax54a
>> +
>> + reg:
>> + const: 0x66
>> +
>> + interrupts:
>> + maxItems: 1
>> +
>> + irq-gpios:
>> + maxItems: 1
>
> Why these are GPIOs? Interrupts are usually just interrupts... You need
> to clearly describe this.
>
I've been using this for some specific acpi stuff hence it need to be
removed.
>
>> +
>> + reset-gpios:
>> + maxItems: 1
>> +
>> +additionalProperties: false
>
> This goes after required: block.
>
>> +
>> +required:
>> + - compatible
>> + - reg
>> +
>> +examples:
>> + - |
>> + #include <dt-bindings/gpio/gpio.h>
>> + #include <dt-bindings/interrupt-controller/arm-gic.h>
>> + i2c {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> + axiom@66 {
>
> Node names should be generic. See also an explanation and list of
> examples (not exhaustive) in DT specification:
> https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation
>
Ack.
>
>
>> + compatible = "touchnetix,axiom-ax54a";
>> + reg = <0x66>;
>> + interrupt-parent = <&gpio2>;
>> + interrupts = <2 IRQ_TYPE_EDGE_FALLING>;
>> + irq-gpios = <&gpio2 0 GPIO_ACTIVE_LOW>;
>
> Eh? This looks really wrong.
OK let's clean that as well.
Thanks
>
>
> Best regards,
> Krzysztof
^ permalink raw reply
* uinput: waiting for UI_FF_UPLOAD events will not inform user when allocation is required
From: John Salamon @ 2023-10-10 7:08 UTC (permalink / raw)
To: dmitry.torokhov, rydberg; +Cc: linux-input, linux-kernel
Currently the "fake" input events generated by uinput in response to
effect uploads will return an effect with an id that has already been
handled by input_ff_upload in ff-core.c, which can modify the effect
id. This causes a problem specifically when the effect originally
uploaded via the EVIOCSFF ioctl contained an effect with -1, as the
userspace code handling UI_FF_UPLOAD receives an effect with an id
other than -1, and therefore will not know an allocation was
requested.
I notice that the "old" field on the ff_effect struct is set to NULL
when the -1 id is changed (in input_ff_upload), which can serve as a
flag that an allocation was requested. If it is the intention is that
uinput users check if old == NULL to know when allocations are needed
I think uinput documentation should describe this.
I first noticed this using python-evdev, see my issue report here:
https://github.com/gvalkov/python-evdev/issues/199
^ permalink raw reply
* Re: [PATCH] Input: elantech - fix fast_reconnect callback in ps2 mode
From: Thorsten Leemhuis @ 2023-10-10 7:08 UTC (permalink / raw)
To: Jeffery Miller, Dmitry Torokhov
Cc: regressions, benjamin.tissoires, Andrew Duggan, Andrew Duggan,
loic.poulain, Greg Kroah-Hartman, linux-input, linux-kernel,
Linux kernel regressions list
In-Reply-To: <CAAzPG9Pp6mHfEziJiUuhDRmkKMfiiPD6axtfAMaCJcEAcuQPiA@mail.gmail.com>
On 05.10.23 02:13, Jeffery Miller wrote:
>
> On Wed, Oct 4, 2023 at 9:11 AM Dmitry Torokhov
> <dmitry.torokhov@gmail.com> wrote:
>>
>> In fact, now that I think about it more, we should rework the original
>> patch that added the delay, so that we do not wait these 30 msec in the
>> "fast" reconnect handler. It turns out your original approach was
>> better, but we should not be using retries, but rather the existing
>> reset_delay_ms already defined in rmi platform data. I would appreciate
>> if you try the draft patch at the end of this email (to be applied after
>> reverting your original one adding the delay in psmouse-smbus.c).
> I tested the draft patch and it works. I did revert the previous delay
> patch while testing it.
>
>> I think we need a similar change in synaptics.c as that one also can
>> fall back to PS/2 mode.
>>
> Ah, good point, yes it does appear this needs to be done as well.
> I have tested and will post an new version of the patch to include
> the fix in synaptics.c as well.
As I'm affected by this problem (and somebody else reported to me in
private to be affected as well) and nothing afaics happened in the past
few days a quick question:
What's the way forward here now that -rc6 slowly comes into sight? Apply
Jeff's patch to fix my problem? Revert the culprit and fix this properly
up with Dmitry's and Jeff's patches in the next cycle? Something else?
Ciao, Thorsten
^ permalink raw reply
* Re: [PATCH v2 3/3] Input: Add TouchNetix aXiom i2c touchscreen driver
From: kernel test robot @ 2023-10-09 19:36 UTC (permalink / raw)
To: Kamel Bouhara, Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Henrik Rydberg
Cc: oe-kbuild-all, linux-input, linux-kernel, devicetree,
mark.satterthwaite, pedro.torruella, bartp, hannah.rossiter,
Thomas Petazzoni, Gregory Clement, bsp-development.geo,
Kamel Bouhara
In-Reply-To: <20231009134435.36311-4-kamel.bouhara@bootlin.com>
Hi Kamel,
kernel test robot noticed the following build warnings:
[auto build test WARNING on dtor-input/next]
[also build test WARNING on dtor-input/for-linus robh/for-next krzk-dt/for-next linus/master v6.6-rc5 next-20231009]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Kamel-Bouhara/dt-bindings-vendor-prefixes-Add-TouchNetix-AS/20231009-214751
base: https://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git next
patch link: https://lore.kernel.org/r/20231009134435.36311-4-kamel.bouhara%40bootlin.com
patch subject: [PATCH v2 3/3] Input: Add TouchNetix aXiom i2c touchscreen driver
config: sparc-allyesconfig (https://download.01.org/0day-ci/archive/20231010/202310100300.oAC2M62R-lkp@intel.com/config)
compiler: sparc64-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231010/202310100300.oAC2M62R-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202310100300.oAC2M62R-lkp@intel.com/
All warnings (new ones prefixed by >>):
In file included from include/linux/byteorder/big_endian.h:5,
from arch/sparc/include/uapi/asm/byteorder.h:5,
from include/asm-generic/qrwlock_types.h:6,
from arch/sparc/include/asm/spinlock_types.h:17,
from include/linux/spinlock_types_raw.h:7,
from include/linux/ratelimit_types.h:7,
from include/linux/printk.h:9,
from include/asm-generic/bug.h:22,
from arch/sparc/include/asm/bug.h:25,
from include/linux/bug.h:5,
from include/linux/thread_info.h:13,
from arch/sparc/include/asm/current.h:15,
from include/linux/sched.h:12,
from include/linux/delay.h:23,
from drivers/input/touchscreen/touchnetix_axiom_i2c.c:16:
drivers/input/touchscreen/touchnetix_axiom_i2c.c: In function 'axiom_i2c_read':
>> include/uapi/linux/byteorder/big_endian.h:36:26: warning: conversion from 'short unsigned int' to 'unsigned char:1' changes value from '256' to '0' [-Woverflow]
36 | #define __cpu_to_le16(x) ((__force __le16)__swab16((x)))
| ^
include/linux/byteorder/generic.h:90:21: note: in expansion of macro '__cpu_to_le16'
90 | #define cpu_to_le16 __cpu_to_le16
| ^~~~~~~~~~~~~
drivers/input/touchscreen/touchnetix_axiom_i2c.c:214:27: note: in expansion of macro 'cpu_to_le16'
214 | cmd_header.read = cpu_to_le16(1);
| ^~~~~~~~~~~
drivers/input/touchscreen/touchnetix_axiom_i2c.c: At top level:
>> drivers/input/touchscreen/touchnetix_axiom_i2c.c:527:6: warning: no previous prototype for 'axiom_handle_events' [-Wmissing-prototypes]
527 | void axiom_handle_events(struct axiom_data *ts)
| ^~~~~~~~~~~~~~~~~~~
--
>> drivers/input/touchscreen/touchnetix_axiom_i2c.c:164: warning: This comment starts with '/**', but isn't a kernel-doc comment. Refer Documentation/doc-guide/kernel-doc.rst
* Translate usage/page/offset triplet into physical address.
vim +/axiom_handle_events +527 drivers/input/touchscreen/touchnetix_axiom_i2c.c
522
523 /*
524 * Validates the crc and demultiplexes the axiom reports to the appropriate
525 * report handler
526 */
> 527 void axiom_handle_events(struct axiom_data *ts)
528 {
529 char *report_data = ts->rx_buf;
530 struct device *dev = ts->dev;
531 char usage = report_data[1];
532 u16 crc_report;
533 u16 crc_calc;
534 char len;
535
536 axiom_i2c_read(ts->client, AXIOM_REPORT_USAGE_ID, 0, report_data, ts->max_report_len);
537
538 if ((report_data[0] & AXIOM_COMMS_OVERFLOW_MASK) != 0)
539 ts->report_overflow_counter++;
540
541 len = (report_data[0] & AXIOM_COMMS_REPORT_LEN_MASK) * 2;
542 if (!len) {
543 dev_err(dev, "Zero length report discarded.\n");
544 return;
545 }
546
547 dev_dbg(dev, "Payload Data %*ph\n", len, report_data);
548
549 /* Validate the report CRC */
550 crc_report = (report_data[len - 1] << 8) | (report_data[len - 2]);
551 /* Length is in 16 bit words and remove the size of the CRC16 itself */
552 crc_calc = crc16(0, report_data, (len - 2));
553
554 if (crc_calc != crc_report) {
555 dev_err(dev,
556 "CRC mismatch! Expected: %#x, Calculated CRC: %#x.\n",
557 crc_report, crc_calc);
558 return;
559 }
560
561 switch (usage) {
562 case AXIOM_USAGE_2DCTS_REPORT_ID:
563 axiom_process_u41_report(ts, &report_data[1]);
564 break;
565
566 case AXIOM_USAGE_2AUX_REPORT_ID:
567 /* This is an aux report (force) */
568 axiom_process_u46_report(ts, &report_data[1]);
569 break;
570
571 case AXIOM_USAGE_2HB_REPORT_ID:
572 /* This is a heartbeat report */
573 break;
574 }
575
576 ts->report_counter++;
577 }
578
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* [PATCH RFT v7 6/6] input: ads7846: Move wait_for_sync() logic to driver
From: Duje Mihanović @ 2023-10-09 18:34 UTC (permalink / raw)
To: Daniel Mack, Haojian Zhuang, Robert Jarzmik, Russell King,
Alan Stern, Greg Kroah-Hartman, Linus Walleij,
Bartosz Golaszewski, Andy Shevchenko, Dmitry Torokhov, Mark Brown
Cc: linux-arm-kernel, linux-kernel, linux-usb, linux-gpio,
linux-input, linux-spi, Duje Mihanović, Bartosz Golaszewski
In-Reply-To: <20231009-pxa-gpio-v7-0-c8f5f403e856@skole.hr>
If this code is left in the board file, the sync GPIO would have to be
separated into another lookup table during conversion to the GPIO
descriptor API (which is also done in this patch).
The only user of this code (Sharp Spitz) is also converted in this
patch.
Suggested-by: Linus Walleij <linus.walleij@linaro.org>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Acked-by: Mark Brown <broonie@kernel.org>
Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Signed-off-by: Duje Mihanović <duje.mihanovic@skole.hr>
---
arch/arm/mach-pxa/spitz.c | 12 ++----------
drivers/input/touchscreen/ads7846.c | 22 +++++++++++++++-------
include/linux/spi/ads7846.h | 1 -
3 files changed, 17 insertions(+), 18 deletions(-)
diff --git a/arch/arm/mach-pxa/spitz.c b/arch/arm/mach-pxa/spitz.c
index 4b6360821396..44336a498699 100644
--- a/arch/arm/mach-pxa/spitz.c
+++ b/arch/arm/mach-pxa/spitz.c
@@ -522,22 +522,12 @@ static inline void spitz_leds_init(void) {}
* SSP Devices
******************************************************************************/
#if defined(CONFIG_SPI_PXA2XX) || defined(CONFIG_SPI_PXA2XX_MODULE)
-static void spitz_ads7846_wait_for_hsync(void)
-{
- while (gpio_get_value(SPITZ_GPIO_HSYNC))
- cpu_relax();
-
- while (!gpio_get_value(SPITZ_GPIO_HSYNC))
- cpu_relax();
-}
-
static struct ads7846_platform_data spitz_ads7846_info = {
.model = 7846,
.vref_delay_usecs = 100,
.x_plate_ohms = 419,
.y_plate_ohms = 486,
.pressure_max = 1024,
- .wait_for_sync = spitz_ads7846_wait_for_hsync,
};
static struct gpiod_lookup_table spitz_ads7846_gpio_table = {
@@ -545,6 +535,8 @@ static struct gpiod_lookup_table spitz_ads7846_gpio_table = {
.table = {
GPIO_LOOKUP("gpio-pxa", SPITZ_GPIO_TP_INT,
"pendown", GPIO_ACTIVE_LOW),
+ GPIO_LOOKUP("gpio-pxa", SPITZ_GPIO_HSYNC,
+ "sync", GPIO_ACTIVE_LOW),
{ }
},
};
diff --git a/drivers/input/touchscreen/ads7846.c b/drivers/input/touchscreen/ads7846.c
index faea40dd66d0..139b0f3735d0 100644
--- a/drivers/input/touchscreen/ads7846.c
+++ b/drivers/input/touchscreen/ads7846.c
@@ -138,8 +138,7 @@ struct ads7846 {
void *filter_data;
int (*get_pendown_state)(void);
struct gpio_desc *gpio_pendown;
-
- void (*wait_for_sync)(void);
+ struct gpio_desc *sync;
};
enum ads7846_filter {
@@ -636,9 +635,15 @@ static const struct attribute_group ads784x_attr_group = {
};
/*--------------------------------------------------------------------------*/
-
-static void null_wait_for_sync(void)
+static void ads7846_wait_for_sync_gpio(struct ads7846 *ts)
{
+ if (!ts->sync)
+ return;
+ while (!gpiod_get_value(ts->sync))
+ cpu_relax();
+
+ while (gpiod_get_value(ts->sync))
+ cpu_relax();
}
static int ads7846_debounce_filter(void *ads, int data_idx, int *val)
@@ -803,7 +808,7 @@ static void ads7846_read_state(struct ads7846 *ts)
packet->last_cmd_idx = 0;
while (true) {
- ts->wait_for_sync();
+ ads7846_wait_for_sync_gpio(ts);
m = &ts->msg[msg_idx];
error = spi_sync(ts->spi, m);
@@ -1261,8 +1266,6 @@ static int ads7846_probe(struct spi_device *spi)
ts->penirq_recheck_delay_usecs =
pdata->penirq_recheck_delay_usecs;
- ts->wait_for_sync = pdata->wait_for_sync ? : null_wait_for_sync;
-
snprintf(ts->phys, sizeof(ts->phys), "%s/input0", dev_name(dev));
snprintf(ts->name, sizeof(ts->name), "ADS%d Touchscreen", ts->model);
@@ -1361,6 +1364,11 @@ static int ads7846_probe(struct spi_device *spi)
if (err)
return err;
+ ts->sync = devm_gpiod_get_optional(dev, "sync", GPIOD_IN);
+ if (IS_ERR(ts->sync))
+ return dev_err_probe(dev, PTR_ERR(ts->sync),
+ "Failed to get sync GPIO");
+
err = input_register_device(input_dev);
if (err)
return err;
diff --git a/include/linux/spi/ads7846.h b/include/linux/spi/ads7846.h
index a04c1c34c344..fa7c4f119023 100644
--- a/include/linux/spi/ads7846.h
+++ b/include/linux/spi/ads7846.h
@@ -38,7 +38,6 @@ struct ads7846_platform_data {
int gpio_pendown_debounce; /* platform specific debounce time for
* the gpio_pendown */
int (*get_pendown_state)(void);
- void (*wait_for_sync)(void);
bool wakeup;
unsigned long irq_flags;
};
--
2.42.0
^ permalink raw reply related
* [PATCH RFT v7 5/6] ARM: pxa: Convert gumstix Bluetooth to GPIO descriptors
From: Duje Mihanović @ 2023-10-09 18:34 UTC (permalink / raw)
To: Daniel Mack, Haojian Zhuang, Robert Jarzmik, Russell King,
Alan Stern, Greg Kroah-Hartman, Linus Walleij,
Bartosz Golaszewski, Andy Shevchenko, Dmitry Torokhov, Mark Brown
Cc: linux-arm-kernel, linux-kernel, linux-usb, linux-gpio,
linux-input, linux-spi, Duje Mihanović, Bartosz Golaszewski
In-Reply-To: <20231009-pxa-gpio-v7-0-c8f5f403e856@skole.hr>
Gumstix still uses the legacy GPIO interface for resetting the Bluetooth
device.
Convert it to use the GPIO descriptor interface.
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Signed-off-by: Duje Mihanović <duje.mihanovic@skole.hr>
---
arch/arm/mach-pxa/gumstix.c | 22 +++++++++++-----------
1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/arch/arm/mach-pxa/gumstix.c b/arch/arm/mach-pxa/gumstix.c
index c9f0f62187bd..0bca6e2c80a9 100644
--- a/arch/arm/mach-pxa/gumstix.c
+++ b/arch/arm/mach-pxa/gumstix.c
@@ -20,8 +20,8 @@
#include <linux/delay.h>
#include <linux/mtd/mtd.h>
#include <linux/mtd/partitions.h>
+#include <linux/gpio/consumer.h>
#include <linux/gpio/machine.h>
-#include <linux/gpio.h>
#include <linux/err.h>
#include <linux/clk.h>
@@ -129,6 +129,11 @@ static void gumstix_udc_init(void)
#endif
#ifdef CONFIG_BT
+GPIO_LOOKUP_SINGLE(gumstix_bt_gpio_table, "pxa2xx-uart.1", "pxa-gpio",
+ GPIO_GUMSTIX_BTRESET, "BTRST", GPIO_ACTIVE_LOW);
+
+static struct gpio_desc *bt_reset;
+
/* Normally, the bootloader would have enabled this 32kHz clock but many
** boards still have u-boot 1.1.4 so we check if it has been turned on and
** if not, we turn it on with a warning message. */
@@ -153,24 +158,19 @@ static void gumstix_setup_bt_clock(void)
static void __init gumstix_bluetooth_init(void)
{
- int err;
+ gpiod_add_lookup_table(&gumstix_bt_gpio_table);
gumstix_setup_bt_clock();
- err = gpio_request(GPIO_GUMSTIX_BTRESET, "BTRST");
- if (err) {
+ bt_reset = gpiod_get(&pxa_device_btuart.dev, "BTRST", GPIOD_OUT_HIGH);
+ if (IS_ERR(bt_reset)) {
pr_err("gumstix: failed request gpio for bluetooth reset\n");
return;
}
- err = gpio_direction_output(GPIO_GUMSTIX_BTRESET, 1);
- if (err) {
- pr_err("gumstix: can't reset bluetooth\n");
- return;
- }
- gpio_set_value(GPIO_GUMSTIX_BTRESET, 0);
+ gpiod_set_value(bt_reset, 1);
udelay(100);
- gpio_set_value(GPIO_GUMSTIX_BTRESET, 1);
+ gpiod_set_value(bt_reset, 0);
}
#else
static void gumstix_bluetooth_init(void)
--
2.42.0
^ permalink raw reply related
* [PATCH RFT v7 4/6] ARM: pxa: Convert reset driver to GPIO descriptors
From: Duje Mihanović @ 2023-10-09 18:34 UTC (permalink / raw)
To: Daniel Mack, Haojian Zhuang, Robert Jarzmik, Russell King,
Alan Stern, Greg Kroah-Hartman, Linus Walleij,
Bartosz Golaszewski, Andy Shevchenko, Dmitry Torokhov, Mark Brown
Cc: linux-arm-kernel, linux-kernel, linux-usb, linux-gpio,
linux-input, linux-spi, Duje Mihanović, Bartosz Golaszewski
In-Reply-To: <20231009-pxa-gpio-v7-0-c8f5f403e856@skole.hr>
The PXA reset driver still uses the legacy GPIO interface for
configuring and asserting the reset pin.
Convert it to use the GPIO descriptor interface.
Acked-by: Linus Walleij <linus.walleij@linaro.org>
Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Signed-off-by: Duje Mihanović <duje.mihanovic@skole.hr>
---
arch/arm/mach-pxa/reset.c | 39 +++++++++++++--------------------------
arch/arm/mach-pxa/reset.h | 3 +--
arch/arm/mach-pxa/spitz.c | 6 +++++-
3 files changed, 19 insertions(+), 29 deletions(-)
diff --git a/arch/arm/mach-pxa/reset.c b/arch/arm/mach-pxa/reset.c
index 27293549f8ad..08bc104b9411 100644
--- a/arch/arm/mach-pxa/reset.c
+++ b/arch/arm/mach-pxa/reset.c
@@ -2,7 +2,7 @@
#include <linux/kernel.h>
#include <linux/module.h>
#include <linux/delay.h>
-#include <linux/gpio.h>
+#include <linux/gpio/consumer.h>
#include <linux/io.h>
#include <asm/proc-fns.h>
#include <asm/system_misc.h>
@@ -14,33 +14,20 @@
static void do_hw_reset(void);
-static int reset_gpio = -1;
+static struct gpio_desc *reset_gpio;
-int init_gpio_reset(int gpio, int output, int level)
+int init_gpio_reset(int output, int level)
{
- int rc;
-
- rc = gpio_request(gpio, "reset generator");
- if (rc) {
- printk(KERN_ERR "Can't request reset_gpio\n");
- goto out;
+ reset_gpio = gpiod_get(NULL, "reset", GPIOD_ASIS);
+ if (IS_ERR(reset_gpio)) {
+ pr_err("Can't request reset_gpio: %pe\n", reset_gpio);
+ return PTR_ERR(reset_gpio);
}
if (output)
- rc = gpio_direction_output(gpio, level);
+ return gpiod_direction_output(reset_gpio, level);
else
- rc = gpio_direction_input(gpio);
- if (rc) {
- printk(KERN_ERR "Can't configure reset_gpio\n");
- gpio_free(gpio);
- goto out;
- }
-
-out:
- if (!rc)
- reset_gpio = gpio;
-
- return rc;
+ return gpiod_direction_input(reset_gpio);
}
/*
@@ -50,16 +37,16 @@ int init_gpio_reset(int gpio, int output, int level)
*/
static void do_gpio_reset(void)
{
- BUG_ON(reset_gpio == -1);
+ BUG_ON(IS_ERR(reset_gpio));
/* drive it low */
- gpio_direction_output(reset_gpio, 0);
+ gpiod_direction_output(reset_gpio, 0);
mdelay(2);
/* rising edge or drive high */
- gpio_set_value(reset_gpio, 1);
+ gpiod_set_value(reset_gpio, 1);
mdelay(2);
/* falling edge */
- gpio_set_value(reset_gpio, 0);
+ gpiod_set_value(reset_gpio, 0);
/* give it some time */
mdelay(10);
diff --git a/arch/arm/mach-pxa/reset.h b/arch/arm/mach-pxa/reset.h
index 963dd190bc13..5864f61a0e94 100644
--- a/arch/arm/mach-pxa/reset.h
+++ b/arch/arm/mach-pxa/reset.h
@@ -13,10 +13,9 @@ extern void pxa_register_wdt(unsigned int reset_status);
/**
* init_gpio_reset() - register GPIO as reset generator
- * @gpio: gpio nr
* @output: set gpio as output instead of input during normal work
* @level: output level
*/
-extern int init_gpio_reset(int gpio, int output, int level);
+extern int init_gpio_reset(int output, int level);
#endif /* __ASM_ARCH_RESET_H */
diff --git a/arch/arm/mach-pxa/spitz.c b/arch/arm/mach-pxa/spitz.c
index c8fc333c2017..4b6360821396 100644
--- a/arch/arm/mach-pxa/spitz.c
+++ b/arch/arm/mach-pxa/spitz.c
@@ -1026,9 +1026,13 @@ static void spitz_restart(enum reboot_mode mode, const char *cmd)
spitz_poweroff();
}
+GPIO_LOOKUP_SINGLE(spitz_reset_gpio_table, NULL, "pxa-gpio",
+ SPITZ_GPIO_ON_RESET, "reset", GPIO_ACTIVE_HIGH);
+
static void __init spitz_init(void)
{
- init_gpio_reset(SPITZ_GPIO_ON_RESET, 1, 0);
+ gpiod_add_lookup_table(&spitz_reset_gpio_table);
+ init_gpio_reset(1, 0);
pm_power_off = spitz_poweroff;
PMCR = 0x00;
--
2.42.0
^ permalink raw reply related
* [PATCH RFT v7 3/6] ARM: pxa: Convert Spitz CF power control to GPIO descriptors
From: Duje Mihanović @ 2023-10-09 18:34 UTC (permalink / raw)
To: Daniel Mack, Haojian Zhuang, Robert Jarzmik, Russell King,
Alan Stern, Greg Kroah-Hartman, Linus Walleij,
Bartosz Golaszewski, Andy Shevchenko, Dmitry Torokhov, Mark Brown
Cc: linux-arm-kernel, linux-kernel, linux-usb, linux-gpio,
linux-input, linux-spi, Duje Mihanović, Bartosz Golaszewski
In-Reply-To: <20231009-pxa-gpio-v7-0-c8f5f403e856@skole.hr>
Sharp's Spitz board still uses the legacy GPIO interface for controlling
the power supply to its CF and SD card slots.
Convert it to use the GPIO descriptor interface.
Acked-by: Linus Walleij <linus.walleij@linaro.org>
Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Signed-off-by: Duje Mihanović <duje.mihanovic@skole.hr>
---
arch/arm/mach-pxa/spitz.c | 20 ++++++++++++++++++--
1 file changed, 18 insertions(+), 2 deletions(-)
diff --git a/arch/arm/mach-pxa/spitz.c b/arch/arm/mach-pxa/spitz.c
index 29907abc4513..c8fc333c2017 100644
--- a/arch/arm/mach-pxa/spitz.c
+++ b/arch/arm/mach-pxa/spitz.c
@@ -133,6 +133,10 @@ static unsigned long spitz_pin_config[] __initdata = {
* Scoop GPIO expander
******************************************************************************/
#if defined(CONFIG_SHARP_SCOOP) || defined(CONFIG_SHARP_SCOOP_MODULE)
+GPIO_LOOKUP_SINGLE(spitz_card_pwr_ctrl_gpio_table, "pxa2xx-mci.0",
+ "sharp-scoop", SPITZ_GPIO_CF_POWER, "cf_power",
+ GPIO_ACTIVE_HIGH);
+
/* SCOOP Device #1 */
static struct resource spitz_scoop_1_resources[] = {
[0] = {
@@ -190,6 +194,7 @@ struct platform_device spitz_scoop_2_device = {
static void __init spitz_scoop_init(void)
{
platform_device_register(&spitz_scoop_1_device);
+ gpiod_add_lookup_table(&spitz_card_pwr_ctrl_gpio_table);
/* Akita doesn't have the second SCOOP chip */
if (!machine_is_akita())
@@ -201,9 +206,18 @@ static void __maybe_unused spitz_card_pwr_ctrl(uint8_t enable, uint8_t new_cpr)
{
unsigned short cpr;
unsigned long flags;
+ struct gpio_desc *cf_power;
+
+ cf_power = gpiod_get(&pxa_device_mci.dev, "cf_power", GPIOD_ASIS);
+ if (IS_ERR(cf_power)) {
+ dev_err(&pxa_device_mci.dev,
+ "failed to get power control GPIO with %ld\n",
+ PTR_ERR(cf_power));
+ return;
+ }
if (new_cpr & 0x7) {
- gpio_set_value(SPITZ_GPIO_CF_POWER, 1);
+ gpiod_direction_output(cf_power, 1);
mdelay(5);
}
@@ -222,8 +236,10 @@ static void __maybe_unused spitz_card_pwr_ctrl(uint8_t enable, uint8_t new_cpr)
if (!(cpr & 0x7)) {
mdelay(1);
- gpio_set_value(SPITZ_GPIO_CF_POWER, 0);
+ gpiod_direction_output(cf_power, 0);
}
+
+ gpiod_put(cf_power);
}
#else
--
2.42.0
^ permalink raw reply related
* [PATCH RFT v7 2/6] ARM: pxa: Convert Spitz LEDs to GPIO descriptors
From: Duje Mihanović @ 2023-10-09 18:33 UTC (permalink / raw)
To: Daniel Mack, Haojian Zhuang, Robert Jarzmik, Russell King,
Alan Stern, Greg Kroah-Hartman, Linus Walleij,
Bartosz Golaszewski, Andy Shevchenko, Dmitry Torokhov, Mark Brown
Cc: linux-arm-kernel, linux-kernel, linux-usb, linux-gpio,
linux-input, linux-spi, Duje Mihanović
In-Reply-To: <20231009-pxa-gpio-v7-0-c8f5f403e856@skole.hr>
Sharp's Spitz board still uses the legacy GPIO interface for configuring
its two onboard LEDs.
Convert them to use the GPIO descriptor interface.
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Duje Mihanović <duje.mihanovic@skole.hr>
---
arch/arm/mach-pxa/spitz.c | 20 ++++++++++++++++++--
1 file changed, 18 insertions(+), 2 deletions(-)
diff --git a/arch/arm/mach-pxa/spitz.c b/arch/arm/mach-pxa/spitz.c
index 535e2b2e997b..29907abc4513 100644
--- a/arch/arm/mach-pxa/spitz.c
+++ b/arch/arm/mach-pxa/spitz.c
@@ -452,16 +452,25 @@ static inline void spitz_keys_init(void) {}
* LEDs
******************************************************************************/
#if defined(CONFIG_LEDS_GPIO) || defined(CONFIG_LEDS_GPIO_MODULE)
+static struct gpiod_lookup_table spitz_led_gpio_table = {
+ .dev_id = "leds-gpio",
+ .table = {
+ GPIO_LOOKUP_IDX("pxa-gpio", SPITZ_GPIO_LED_ORANGE, NULL, 0,
+ GPIO_ACTIVE_HIGH),
+ GPIO_LOOKUP_IDX("pxa-gpio", SPITZ_GPIO_LED_GREEN, NULL, 1,
+ GPIO_ACTIVE_HIGH),
+ { }
+ }
+};
+
static struct gpio_led spitz_gpio_leds[] = {
{
.name = "spitz:amber:charge",
.default_trigger = "sharpsl-charge",
- .gpio = SPITZ_GPIO_LED_ORANGE,
},
{
.name = "spitz:green:hddactivity",
.default_trigger = "disk-activity",
- .gpio = SPITZ_GPIO_LED_GREEN,
},
};
@@ -478,9 +487,16 @@ static struct platform_device spitz_led_device = {
},
};
+static struct gpio_descs *leds;
+
static void __init spitz_leds_init(void)
{
+ gpiod_add_lookup_table(&spitz_led_gpio_table);
platform_device_register(&spitz_led_device);
+ leds = gpiod_get_array_optional(&spitz_led_device.dev,
+ NULL, GPIOD_ASIS);
+ spitz_gpio_leds[0].gpiod = leds->desc[0];
+ spitz_gpio_leds[1].gpiod = leds->desc[1];
}
#else
static inline void spitz_leds_init(void) {}
--
2.42.0
^ permalink raw reply related
* [PATCH RFT v7 0/6] ARM: pxa: GPIO descriptor conversions
From: Duje Mihanović @ 2023-10-09 18:33 UTC (permalink / raw)
To: Daniel Mack, Haojian Zhuang, Robert Jarzmik, Russell King,
Alan Stern, Greg Kroah-Hartman, Linus Walleij,
Bartosz Golaszewski, Andy Shevchenko, Dmitry Torokhov, Mark Brown
Cc: linux-arm-kernel, linux-kernel, linux-usb, linux-gpio,
linux-input, linux-spi, Duje Mihanović, Bartosz Golaszewski
Hello,
Small series to convert some of the board files in the mach-pxa directory
to use the new GPIO descriptor interface.
Most notably, the am200epd, am300epd and Spitz matrix keypad among
others are not converted in this series.
Signed-off-by: Duje Mihanović <duje.mihanovic@skole.hr>
---
Changes in v7:
- Address maintainer comments:
- Drop gpiod_put in OHCI
- Make "struct gpio_descs *leds" in Spitz LEDs global
- Link to v6: https://lore.kernel.org/r/20231006-pxa-gpio-v6-0-981b4910d599@skole.hr
Changes in v6:
- Address maintainer comments:
- Use devm_gpiod_get_optional() in OHCI
- Use gpiod_get_array() in Spitz LEDs
- Update trailers
- Link to v5: https://lore.kernel.org/r/20231004-pxa-gpio-v5-0-d99ae6fceea8@skole.hr
Changes in v5:
- Address maintainer comments:
- Rename "reset generator" GPIO to "reset"
- Rename ads7846_wait_for_sync() to ads7846_wait_for_sync_gpio()
- Properly bail out when requesting USB host GPIO fails
- Use dev_err_probe() when requesting touchscreen sync GPIO fails
- Use static gpio_desc for gumstix bluetooth reset
- Pulse gumstix bluetooth reset line correctly (assert, then deassert)
- Fix style issue in ads7846_wait_for_sync_gpio()
- Update trailers
- Link to v4: https://lore.kernel.org/r/20231001-pxa-gpio-v4-0-0f3b975e6ed5@skole.hr
Changes in v4:
- Address maintainer comments:
- Move wait_for_sync() from spitz.c to driver
- Register LED platform device before getting its gpiod-s
- Add Linus' Reviewed-by
- Link to v3: https://lore.kernel.org/r/20230929-pxa-gpio-v3-0-af8d5e5d1f34@skole.hr
Changes in v3:
- Address maintainer comments:
- Use GPIO_LOOKUP_IDX for LEDs
- Drop unnecessary NULL assignments
- Don't give up on *all* SPI devices if hsync cannot be set up
- Add Linus' Acked-by
- Link to v2: https://lore.kernel.org/r/20230926-pxa-gpio-v2-0-984464d165dd@skole.hr
Changes in v2:
- Address maintainer comments:
- Change mentions of function to function()
- Drop cast in OHCI driver dev_warn() call
- Use %pe in OHCI and reset drivers
- Use GPIO _optional() API in OHCI driver
- Drop unnecessary not-null check in OHCI driver
- Use pr_err() instead of printk() in reset driver
- Rebase on v6.6-rc3
- Link to v1: https://lore.kernel.org/r/20230924-pxa-gpio-v1-0-2805b87d8894@skole.hr
---
Duje Mihanović (6):
ARM: pxa: Convert Spitz OHCI to GPIO descriptors
ARM: pxa: Convert Spitz LEDs to GPIO descriptors
ARM: pxa: Convert Spitz CF power control to GPIO descriptors
ARM: pxa: Convert reset driver to GPIO descriptors
ARM: pxa: Convert gumstix Bluetooth to GPIO descriptors
input: ads7846: Move wait_for_sync() logic to driver
arch/arm/mach-pxa/gumstix.c | 22 ++++++------
arch/arm/mach-pxa/reset.c | 39 +++++++-------------
arch/arm/mach-pxa/reset.h | 3 +-
arch/arm/mach-pxa/spitz.c | 71 +++++++++++++++++++++++++------------
drivers/input/touchscreen/ads7846.c | 22 ++++++++----
drivers/usb/host/ohci-pxa27x.c | 5 +++
include/linux/spi/ads7846.h | 1 -
7 files changed, 94 insertions(+), 69 deletions(-)
---
base-commit: 8a749fd1a8720d4619c91c8b6e7528c0a355c0aa
change-id: 20230807-pxa-gpio-3ce25d574814
Best regards,
--
Duje Mihanović <duje.mihanovic@skole.hr>
^ permalink raw reply
* [PATCH RFT v7 1/6] ARM: pxa: Convert Spitz OHCI to GPIO descriptors
From: Duje Mihanović @ 2023-10-09 18:33 UTC (permalink / raw)
To: Daniel Mack, Haojian Zhuang, Robert Jarzmik, Russell King,
Alan Stern, Greg Kroah-Hartman, Linus Walleij,
Bartosz Golaszewski, Andy Shevchenko, Dmitry Torokhov, Mark Brown
Cc: linux-arm-kernel, linux-kernel, linux-usb, linux-gpio,
linux-input, linux-spi, Duje Mihanović
In-Reply-To: <20231009-pxa-gpio-v7-0-c8f5f403e856@skole.hr>
Sharp's Spitz board still uses the legacy GPIO interface for controlling
a GPIO pin related to the USB host controller.
Convert this function to use the new GPIO descriptor interface.
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Duje Mihanović <duje.mihanovic@skole.hr>
---
arch/arm/mach-pxa/spitz.c | 13 ++++++-------
drivers/usb/host/ohci-pxa27x.c | 5 +++++
2 files changed, 11 insertions(+), 7 deletions(-)
diff --git a/arch/arm/mach-pxa/spitz.c b/arch/arm/mach-pxa/spitz.c
index cc691b199429..535e2b2e997b 100644
--- a/arch/arm/mach-pxa/spitz.c
+++ b/arch/arm/mach-pxa/spitz.c
@@ -649,23 +649,22 @@ static inline void spitz_mmc_init(void) {}
* USB Host
******************************************************************************/
#if defined(CONFIG_USB_OHCI_HCD) || defined(CONFIG_USB_OHCI_HCD_MODULE)
+GPIO_LOOKUP_SINGLE(spitz_usb_host_gpio_table, "pxa27x-ohci", "gpio-pxa",
+ SPITZ_GPIO_USB_HOST, "usb-host", GPIO_ACTIVE_LOW);
+
static int spitz_ohci_init(struct device *dev)
{
- int err;
-
- err = gpio_request(SPITZ_GPIO_USB_HOST, "USB_HOST");
- if (err)
- return err;
+ gpiod_add_lookup_table(&spitz_usb_host_gpio_table);
/* Only Port 2 is connected, setup USB Port 2 Output Control Register */
UP2OCR = UP2OCR_HXS | UP2OCR_HXOE | UP2OCR_DPPDE | UP2OCR_DMPDE;
- return gpio_direction_output(SPITZ_GPIO_USB_HOST, 1);
+ return 0;
}
static void spitz_ohci_exit(struct device *dev)
{
- gpio_free(SPITZ_GPIO_USB_HOST);
+ gpiod_remove_lookup_table(&spitz_usb_host_gpio_table);
}
static struct pxaohci_platform_data spitz_ohci_platform_data = {
diff --git a/drivers/usb/host/ohci-pxa27x.c b/drivers/usb/host/ohci-pxa27x.c
index 357d9aee38a3..a809ba0bb25e 100644
--- a/drivers/usb/host/ohci-pxa27x.c
+++ b/drivers/usb/host/ohci-pxa27x.c
@@ -121,6 +121,7 @@ struct pxa27x_ohci {
void __iomem *mmio_base;
struct regulator *vbus[3];
bool vbus_enabled[3];
+ struct gpio_desc *usb_host;
};
#define to_pxa27x_ohci(hcd) (struct pxa27x_ohci *)(hcd_to_ohci(hcd)->priv)
@@ -447,6 +448,10 @@ static int ohci_hcd_pxa27x_probe(struct platform_device *pdev)
pxa_ohci = to_pxa27x_ohci(hcd);
pxa_ohci->clk = usb_clk;
pxa_ohci->mmio_base = (void __iomem *)hcd->regs;
+ pxa_ohci->usb_host = devm_gpiod_get_optional(&pdev->dev, "usb-host", GPIOD_OUT_LOW);
+ if (IS_ERR(pxa_ohci->usb_host))
+ return dev_err_probe(&pdev->dev, PTR_ERR(pxa_ohci->usb_host),
+ "failed to get USB host GPIO\n");
for (i = 0; i < 3; ++i) {
char name[6];
--
2.42.0
^ permalink raw reply related
* Re: [PATCH V2 1/2] dt-bindings: input: Introduce Himax HID-over-SPI device
From: Conor Dooley @ 2023-10-09 17:52 UTC (permalink / raw)
To: yang tylor
Cc: Conor Dooley, dmitry.torokhov, robh+dt, krzysztof.kozlowski+dt,
conor+dt, jikos, benjamin.tissoires, linux-input, devicetree,
linux-kernel, poyuan_chang, hbarnor, jingyliang@chromium.org,
wuxy23, luolm1, hung poyu
In-Reply-To: <CAGD2q_b1gn8XAfgfzuNn3Jo6gEguBEacxERyRM5ms-V=+hWS+g@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 3374 bytes --]
On Mon, Oct 02, 2023 at 06:44:41PM +0800, yang tylor wrote:
> On Fri, Sep 29, 2023 at 12:56 AM Conor Dooley <conor@kernel.org> wrote:
> >
> > On Thu, Sep 28, 2023 at 10:12:41AM +0800, yang tylor wrote:
> > > On Tue, Sep 26, 2023 at 8:53 PM Conor Dooley <conor@kernel.org> wrote:
> > > > On Tue, Sep 26, 2023 at 05:52:39PM +0800, yang tylor wrote:
> > > > > On Tue, Sep 26, 2023 at 5:02 PM Conor Dooley <conor@kernel.org> wrote:
> > > > > > On Mon, Sep 25, 2023 at 06:16:29PM +0800, yang tylor wrote:
> > > > > > > On Mon, Sep 25, 2023 at 4:41 PM Conor Dooley <conor.dooley@microchip.com> wrote:
> > > > > > > We have a default prefix firmware name(like himax_xxxx.bin) in the driver code.
> > > > > >
> > > > > > How do you intend generating the name of the firmware file? I assume the
> > > > > > same firmware doesn't work on every IC, so you'll need to pick a
> > > > > > different one depending on the compatible?
> > > > > >
> > > > > If considering a firmware library line-up for all the incoming panels
> > > > > of this driver.
> > > > > We would use PID as part of the file name. Because all the support panels would
> > > > > have a unique PID associated. Which will make the firmware name like
> > > > > himax_xxx_{$PID}.bin. The problem is, we need to know PID before firmware load
> > > > > at no flash condition. Thus PID information is required in dts when
> > > > > no-flash-flag
> > > > > is specified.
> > > >
> > > > Firstly, where does the "xxx" come from?
> > > > And you're making it sound more like having firmware-name is suitable
> > > > for this use case, given you need to determine the name of the file to
> > > > use based on something that is hardware specific but is not
> > > > dynamically detectable.
> > > Current driver patch uses a prefix name "himax_i2chid" which comes
> > > from the previous project
> > > and seems not suitable for this condition, so I use "xxx" and plan to
> > > replace it in the next version.
> > > For finding firmware, I think both solutions are reasonable.
> > > - provide firmware name directly: implies no-flash and use user
> > > specified firmware, no PID info.
> > > - provide no-flash-flag and PID info: loading firmware from organized
> > > names with PID info.
> > > I prefer the 2nd solution, but it needs more properties in dts. 1st
> > > has less properties and more
> > > intuitive.
> > >
> > > I don't know which one is more acceptable by the community, as you
> > > know I'm a newbie here.
> >
> > To be honest, I am not all that sure either! Does the panel id have
> > value in its own right, or is that only used to determine the firmware
> > filename?
> Currently, PID stands for Panel/Project ID and is used for determining
> the firmware filename only. We haven't come up with any new attribute that
> may attach to it. The differences between panels are handled in firmware
> dedicated to its PID.
>
> > Also, if it does have value in its own right, rather than a "pid",
> > should the panel be a child node of this hid device with its own
> > compatible?
> It may need a child node if we find it necessary to add attributes to each PID.
> But currently we have no idea about it.
To be honest, it seems to me like you are using "PID" in place of a
compatible for the panel, since it needs to be provided via DT anyway.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: [PATCH 7/7] block, drivers: make lots of attribute_group globals const
From: Greg Kroah-Hartman @ 2023-10-09 17:30 UTC (permalink / raw)
To: Max Kellermann
Cc: Jens Axboe, Rafael J. Wysocki, Ilya Dryomov, Dongsheng Yang,
Dmitry Torokhov, Bjorn Helgaas, Rodolfo Giometti,
Alessandro Zummo, Alexandre Belloni, Jiri Slaby, Mark Fasheh,
Joel Becker, Joseph Qi, Andrew Morton, linux-block, linux-kernel,
ceph-devel, linux-input, linux-pci, linux-rtc, linux-serial,
ocfs2-devel, linux-mm
In-Reply-To: <20231009165741.746184-7-max.kellermann@ionos.com>
On Mon, Oct 09, 2023 at 06:57:40PM +0200, Max Kellermann wrote:
> This moves those variables to the ".rodata" section which reduces the
> kernel size a bit and protects the variables by putting them on
> read-only pages at runtime.
The kernel size should still be the same overall, you are just moving
pointers from one section to another, right?
If not, what are the numbers, show them please.
But step back, are you SURE you can make these attribute group pointers
const? They are modified by some subsystems by adding or removing items
from the lists, so why does the core need to change for that?
thanks,
greg k-h
^ permalink raw reply
* [PATCH 7/7] block, drivers: make lots of attribute_group globals const
From: Max Kellermann @ 2023-10-09 16:57 UTC (permalink / raw)
To: Jens Axboe, Greg Kroah-Hartman, Rafael J. Wysocki, Ilya Dryomov,
Dongsheng Yang, Dmitry Torokhov, Bjorn Helgaas, Rodolfo Giometti,
Alessandro Zummo, Alexandre Belloni, Jiri Slaby, Mark Fasheh,
Joel Becker, Joseph Qi, Andrew Morton
Cc: Max Kellermann, linux-block, linux-kernel, ceph-devel,
linux-input, linux-pci, linux-rtc, linux-serial, ocfs2-devel,
linux-mm
In-Reply-To: <20231009165741.746184-1-max.kellermann@ionos.com>
This moves those variables to the ".rodata" section which reduces the
kernel size a bit and protects the variables by putting them on
read-only pages at runtime.
Signed-off-by: Max Kellermann <max.kellermann@ionos.com>
---
block/blk-sysfs.c | 6 +++---
block/genhd.c | 4 ++--
block/partitions/core.c | 2 +-
drivers/base/cacheinfo.c | 2 +-
drivers/block/loop.c | 2 +-
drivers/block/rbd.c | 4 ++--
drivers/input/input.c | 2 +-
drivers/input/serio/serio.c | 2 +-
drivers/pci/pci-sysfs.c | 8 ++++----
drivers/pci/pci.h | 6 +++---
drivers/pps/sysfs.c | 2 +-
drivers/rtc/sysfs.c | 2 +-
drivers/tty/serial/8250/8250_port.c | 2 +-
fs/ocfs2/cluster/sys.c | 2 +-
include/linux/khugepaged.h | 2 +-
include/linux/pps_kernel.h | 2 +-
mm/khugepaged.c | 2 +-
17 files changed, 26 insertions(+), 26 deletions(-)
diff --git a/block/blk-sysfs.c b/block/blk-sysfs.c
index 63e481262336..feea5d68b5a1 100644
--- a/block/blk-sysfs.c
+++ b/block/blk-sysfs.c
@@ -699,12 +699,12 @@ static umode_t blk_mq_queue_attr_visible(struct kobject *kobj,
return attr->mode;
}
-static struct attribute_group queue_attr_group = {
+static const struct attribute_group queue_attr_group = {
.attrs = queue_attrs,
.is_visible = queue_attr_visible,
};
-static struct attribute_group blk_mq_queue_attr_group = {
+static const struct attribute_group blk_mq_queue_attr_group = {
.attrs = blk_mq_queue_attrs,
.is_visible = blk_mq_queue_attr_visible,
};
@@ -750,7 +750,7 @@ static const struct sysfs_ops queue_sysfs_ops = {
.store = queue_attr_store,
};
-static const struct attribute_group *blk_queue_attr_groups[] = {
+static const struct attribute_group *const blk_queue_attr_groups[] = {
&queue_attr_group,
&blk_mq_queue_attr_group,
NULL
diff --git a/block/genhd.c b/block/genhd.c
index d82560a79b04..9fa16e5de6d2 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -1115,12 +1115,12 @@ static umode_t disk_visible(struct kobject *kobj, struct attribute *a, int n)
return a->mode;
}
-static struct attribute_group disk_attr_group = {
+static const struct attribute_group disk_attr_group = {
.attrs = disk_attrs,
.is_visible = disk_visible,
};
-static const struct attribute_group *disk_attr_groups[] = {
+static const struct attribute_group *const disk_attr_groups[] = {
&disk_attr_group,
#ifdef CONFIG_BLK_DEV_IO_TRACE
&blk_trace_attr_group,
diff --git a/block/partitions/core.c b/block/partitions/core.c
index e137a87f4db0..463298e26757 100644
--- a/block/partitions/core.c
+++ b/block/partitions/core.c
@@ -232,7 +232,7 @@ static const struct attribute_group part_attr_group = {
.attrs = part_attrs,
};
-static const struct attribute_group *part_attr_groups[] = {
+static const struct attribute_group *const part_attr_groups[] = {
&part_attr_group,
#ifdef CONFIG_BLK_DEV_IO_TRACE
&blk_trace_attr_group,
diff --git a/drivers/base/cacheinfo.c b/drivers/base/cacheinfo.c
index b91c31c2a393..3642eed8ef74 100644
--- a/drivers/base/cacheinfo.c
+++ b/drivers/base/cacheinfo.c
@@ -786,7 +786,7 @@ static const struct attribute_group cache_default_group = {
.is_visible = cache_default_attrs_is_visible,
};
-static const struct attribute_group *cache_default_groups[] = {
+static const struct attribute_group *const cache_default_groups[] = {
&cache_default_group,
NULL,
};
diff --git a/drivers/block/loop.c b/drivers/block/loop.c
index 9f2d412fc560..c1718b17b5ef 100644
--- a/drivers/block/loop.c
+++ b/drivers/block/loop.c
@@ -736,7 +736,7 @@ static struct attribute *loop_attrs[] = {
NULL,
};
-static struct attribute_group loop_attribute_group = {
+static const struct attribute_group loop_attribute_group = {
.name = "loop",
.attrs= loop_attrs,
};
diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
index a999b698b131..73e616453c34 100644
--- a/drivers/block/rbd.c
+++ b/drivers/block/rbd.c
@@ -5246,11 +5246,11 @@ static struct attribute *rbd_attrs[] = {
NULL
};
-static struct attribute_group rbd_attr_group = {
+static const struct attribute_group rbd_attr_group = {
.attrs = rbd_attrs,
};
-static const struct attribute_group *rbd_attr_groups[] = {
+static const struct attribute_group *const rbd_attr_groups[] = {
&rbd_attr_group,
NULL
};
diff --git a/drivers/input/input.c b/drivers/input/input.c
index 8c5fdb0f858a..d97126d54947 100644
--- a/drivers/input/input.c
+++ b/drivers/input/input.c
@@ -1597,7 +1597,7 @@ static const struct attribute_group input_dev_caps_attr_group = {
.attrs = input_dev_caps_attrs,
};
-static const struct attribute_group *input_dev_attr_groups[] = {
+static const struct attribute_group *const input_dev_attr_groups[] = {
&input_dev_attr_group,
&input_dev_id_attr_group,
&input_dev_caps_attr_group,
diff --git a/drivers/input/serio/serio.c b/drivers/input/serio/serio.c
index 767fc9efb4a8..ef82d20572b0 100644
--- a/drivers/input/serio/serio.c
+++ b/drivers/input/serio/serio.c
@@ -474,7 +474,7 @@ static const struct attribute_group serio_device_attr_group = {
.attrs = serio_device_attrs,
};
-static const struct attribute_group *serio_device_attr_groups[] = {
+static const struct attribute_group *const serio_device_attr_groups[] = {
&serio_id_attr_group,
&serio_device_attr_group,
NULL
diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
index d9eede2dbc0e..f2147da6e4a5 100644
--- a/drivers/pci/pci-sysfs.c
+++ b/drivers/pci/pci-sysfs.c
@@ -455,7 +455,7 @@ static const struct attribute_group pci_bus_group = {
.attrs = pci_bus_attrs,
};
-const struct attribute_group *pci_bus_groups[] = {
+const struct attribute_group *const pci_bus_groups[] = {
&pci_bus_group,
NULL,
};
@@ -647,7 +647,7 @@ static const struct attribute_group pcibus_group = {
.attrs = pcibus_attrs,
};
-const struct attribute_group *pcibus_groups[] = {
+const struct attribute_group *const pcibus_groups[] = {
&pcibus_group,
NULL,
};
@@ -1604,7 +1604,7 @@ static const struct attribute_group pci_dev_group = {
.attrs = pci_dev_attrs,
};
-const struct attribute_group *pci_dev_groups[] = {
+const struct attribute_group *const pci_dev_groups[] = {
&pci_dev_group,
&pci_dev_config_attr_group,
&pci_dev_rom_attr_group,
@@ -1641,7 +1641,7 @@ static const struct attribute_group pcie_dev_attr_group = {
.is_visible = pcie_dev_attrs_are_visible,
};
-static const struct attribute_group *pci_dev_attr_groups[] = {
+static const struct attribute_group *const pci_dev_attr_groups[] = {
&pci_dev_attr_group,
&pci_dev_hp_attr_group,
#ifdef CONFIG_PCI_IOV
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index 39a8932dc340..046d6c9944cd 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -182,10 +182,10 @@ static inline int pci_no_d1d2(struct pci_dev *dev)
return (dev->no_d1d2 || parent_dstates);
}
-extern const struct attribute_group *pci_dev_groups[];
-extern const struct attribute_group *pcibus_groups[];
+extern const struct attribute_group *const pci_dev_groups[];
+extern const struct attribute_group *const pcibus_groups[];
extern const struct device_type pci_dev_type;
-extern const struct attribute_group *pci_bus_groups[];
+extern const struct attribute_group *const pci_bus_groups[];
extern unsigned long pci_hotplug_io_size;
extern unsigned long pci_hotplug_mmio_size;
diff --git a/drivers/pps/sysfs.c b/drivers/pps/sysfs.c
index 134bc33f6ad0..355ce20b6e53 100644
--- a/drivers/pps/sysfs.c
+++ b/drivers/pps/sysfs.c
@@ -93,7 +93,7 @@ static const struct attribute_group pps_group = {
.attrs = pps_attrs,
};
-const struct attribute_group *pps_groups[] = {
+const struct attribute_group *const pps_groups[] = {
&pps_group,
NULL,
};
diff --git a/drivers/rtc/sysfs.c b/drivers/rtc/sysfs.c
index 9c45c2557e28..c126cb706b27 100644
--- a/drivers/rtc/sysfs.c
+++ b/drivers/rtc/sysfs.c
@@ -303,7 +303,7 @@ static struct attribute_group rtc_attr_group = {
.attrs = rtc_attrs,
};
-static const struct attribute_group *rtc_attr_groups[] = {
+static const struct attribute_group *const rtc_attr_groups[] = {
&rtc_attr_group,
NULL
};
diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
index 141627370aab..7af8f196e00f 100644
--- a/drivers/tty/serial/8250/8250_port.c
+++ b/drivers/tty/serial/8250/8250_port.c
@@ -3187,7 +3187,7 @@ static struct attribute *serial8250_dev_attrs[] = {
NULL
};
-static struct attribute_group serial8250_dev_attr_group = {
+static const struct attribute_group serial8250_dev_attr_group = {
.attrs = serial8250_dev_attrs,
};
diff --git a/fs/ocfs2/cluster/sys.c b/fs/ocfs2/cluster/sys.c
index 022f716c74ff..63e14ef53610 100644
--- a/fs/ocfs2/cluster/sys.c
+++ b/fs/ocfs2/cluster/sys.c
@@ -31,7 +31,7 @@ static struct attribute *o2cb_attrs[] = {
NULL,
};
-static struct attribute_group o2cb_attr_group = {
+static const struct attribute_group o2cb_attr_group = {
.attrs = o2cb_attrs,
};
diff --git a/include/linux/khugepaged.h b/include/linux/khugepaged.h
index f68865e19b0b..85b442e9e638 100644
--- a/include/linux/khugepaged.h
+++ b/include/linux/khugepaged.h
@@ -5,7 +5,7 @@
#include <linux/sched/coredump.h> /* MMF_VM_HUGEPAGE */
#ifdef CONFIG_TRANSPARENT_HUGEPAGE
-extern struct attribute_group khugepaged_attr_group;
+extern const struct attribute_group khugepaged_attr_group;
extern int khugepaged_init(void);
extern void khugepaged_destroy(void);
diff --git a/include/linux/pps_kernel.h b/include/linux/pps_kernel.h
index 78c8ac4951b5..996db99f983f 100644
--- a/include/linux/pps_kernel.h
+++ b/include/linux/pps_kernel.h
@@ -66,7 +66,7 @@ struct pps_device {
* Global variables
*/
-extern const struct attribute_group *pps_groups[];
+extern const struct attribute_group *const pps_groups[];
/*
* Internal functions.
diff --git a/mm/khugepaged.c b/mm/khugepaged.c
index 88433cc25d8a..cd1b26075d1b 100644
--- a/mm/khugepaged.c
+++ b/mm/khugepaged.c
@@ -339,7 +339,7 @@ static struct attribute *khugepaged_attr[] = {
NULL,
};
-struct attribute_group khugepaged_attr_group = {
+const struct attribute_group khugepaged_attr_group = {
.attrs = khugepaged_attr,
.name = "khugepaged",
};
--
2.39.2
^ permalink raw reply related
* Re: [PATCH v3 1/2] dt-bindings: input: bindings for Adafruit Seesaw Gamepad
From: Conor Dooley @ 2023-10-09 16:21 UTC (permalink / raw)
To: Anshul Dalal
Cc: linux-input, devicetree, Dmitry Torokhov, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Shuah Khan,
linux-kernel-mentees
In-Reply-To: <20231008185709.2448423-1-anshulusr@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 2703 bytes --]
On Mon, Oct 09, 2023 at 12:27:06AM +0530, Anshul Dalal wrote:
> A simple driver for a mini gamepad that communicates over i2c, the gamepad
> has bidirectional thumb stick input and six buttons.
>
> The gamepad chip utilizes the open framework from Adafruit called 'Seesaw'
> to transmit the ADC data for the joystick and digital pin state for the
> buttons. I have only implemented the functionality required to receive the
> thumb stick and button state.
>
> Product page:
> https://www.adafruit.com/product/5743
> Arduino driver:
> https://github.com/adafruit/Adafruit_Seesaw
>
> For testing on a RPi Zero 2W, I utilized the following devicetree overlay:
>
> /dts-v1/;
> /plugin/;
> / {
> compatible = "brcm,bcm2835";
> fragment@0 {
> target = <&i2c1>;
> __overlay__ {
> #address-cells = <1>;
> #size-cells = <0>;
> joystick@50 {
> compatible = "adafruit,seesaw-gamepad";
> reg = <0x50>;
> };
> };
> };
> };
>
> I used the above overlay as reference for writing this binding. Though the
> gamepad also has an interrupt pin that needs to be enabled explicitly (not
> currently implemented in driver). The pin triggers a rising edge when a
> button is pressed or joystick is moved which can be detected on a GPIO
> of the Microcontroller.
>
> I wasn't sure how to represent that functionality in the binding so I have
> left it out for now.
>
> Signed-off-by: Anshul Dalal <anshulusr@gmail.com>
> ---
>
> Changes for v3:
> - Updated id field to reflect updated file name from previous version
> - Added `reg` property
>
> Changes for v2:
> - Renamed file to `adafruit,seesaw-gamepad.yaml`
> - Removed quotes for `$id` and `$schema`
> - Removed "Bindings for" from the description
> - Changed node name to the generic name "joystick"
> - Changed compatible to 'adafruit,seesaw-gamepad' instead of 'adafruit,seesaw_gamepad'
>
> .../input/adafruit,seesaw-gamepad.yaml | 54 +++++++++++++++++++
> 1 file changed, 54 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/input/adafruit,seesaw-gamepad.yaml
>
> diff --git a/Documentation/devicetree/bindings/input/adafruit,seesaw-gamepad.yaml b/Documentation/devicetree/bindings/input/adafruit,seesaw-gamepad.yaml
> new file mode 100644
> index 000000000000..610c99594439
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/adafruit,seesaw-gamepad.yaml
> +$id: http://devicetree.org/schemas/input/adafruit-seesaw.yaml#
You're at version 3 now, but do not seem to have figured out how to test
the bindings?
https://docs.kernel.org/devicetree/bindings/writing-schema.html#running-checks
Thanks,
Conor.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ 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