* Re: [PATCH v3 2/2] arm64: dts: qcom: sc8280xp-x13s: Fix/enable touchscreen
From: Johan Hovold @ 2024-01-29 16:59 UTC (permalink / raw)
To: Bjorn Andersson
Cc: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Benjamin Tissoires, Jiri Kosina, Bjorn Andersson, Konrad Dybcio,
Johan Hovold, linux-arm-msm, linux-input, devicetree,
linux-kernel, Konrad Dybcio, Krzysztof Kozlowski, Daniel Thompson
In-Reply-To: <20240129-x13s-touchscreen-v3-2-c4a933034145@quicinc.com>
On Mon, Jan 29, 2024 at 08:47:48AM -0800, Bjorn Andersson wrote:
> The touchscreen present on some SKUs of Lenovo Thinkpad X13s is never
> detected by Linux. Power is applied and the device is brought out of
> reset using the pinconfig in DeviceTree, but the read-test in
> __i2c_hid_core_probe() fails to access the device, which result in probe
> being aborted.
>
> Some users have reported success after rebinding the device.
>
> Looking to the ACPI tables, there's a 5ms after-power and a 200ms
> after-reset delay. The power-supply is shared with other components, so
> this is active all the way through boot. The reset GPIO, on the other
> hand, is low (reset asserted) at boot, so this is first deasserted by
> the implicit application of the pinconf state.
>
> This means the time between reset deassert and __i2c_hid_core_probe() is
> significantly below the value documented in the ACPI tables.
>
> As the I2C HID binding and driver support specifying a reset gpio,
> replace the pinconf-based scheme to pull the device out of reset. Then
> specify the after-reset time.
>
> The shared power rail is currently always on, but in case this ever
> change, the after-power delay is added as well, to not violate the
> power-on to reset-deassert timing requirement.
>
> Fixes: 32c231385ed4 ("arm64: dts: qcom: sc8280xp: add Lenovo Thinkpad X13s devicetree")
> Tested-by: Daniel Thompson <daniel.thompson@linaro.org>
> Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com>
Thanks for the update.
Reviewed-by: Johan Hovold <johan+linaro@kernel.org>
^ permalink raw reply
* [PATCH] HID: wacom: Do not register input devices until after hid_hw_start
From: Gerecke, Jason @ 2024-01-29 22:35 UTC (permalink / raw)
To: linux-input, Jiri Kosina, Benjamin Tissoires
Cc: Ping Cheng, Joshua Dickens, Aaron Armstrong Skomra,
Tobita, Tatsunosuke, Jason Gerecke, stable, Dmitry Torokhov,
Jason Gerecke
From: Jason Gerecke <killertofu@gmail.com>
If a input device is opened before hid_hw_start is called, events may
not be received from the hardware. In the case of USB-backed devices,
for example, the hid_hw_start function is responsible for filling in
the URB which is submitted when the input device is opened. If a device
is opened prematurely, polling will never start because the device will
not have been in the correct state to send the URB.
Because the wacom driver registers its input devices before calling
hid_hw_start, there is a window of time where a device can be opened
and end up in an inoperable state. Some ARM-based Chromebooks in particular
reliably trigger this bug.
This commit splits the wacom_register_inputs function into two pieces.
One which is responsible for setting up the allocated inputs (and runs
prior to hid_hw_start so that devices are ready for any input events
they may end up receiving) and another which only registers the devices
(and runs after hid_hw_start to ensure devices can be immediately opened
without issue). Note that the functions to initialize the LEDs and remotes
are also moved after hid_hw_start to maintain their own dependency chains.
Fixes: 7704ac937345 ("HID: wacom: implement generic HID handling for pen generic devices")
Cc: stable@vger.kernel.org # v3.18+
Suggested-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Signed-off-by: Jason Gerecke <jason.gerecke@wacom.com>
---
drivers/hid/wacom_sys.c | 63 ++++++++++++++++++++++++++++-------------
1 file changed, 43 insertions(+), 20 deletions(-)
diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
index b613f11ed9498..2bc45b24075c3 100644
--- a/drivers/hid/wacom_sys.c
+++ b/drivers/hid/wacom_sys.c
@@ -2087,7 +2087,7 @@ static int wacom_allocate_inputs(struct wacom *wacom)
return 0;
}
-static int wacom_register_inputs(struct wacom *wacom)
+static int wacom_setup_inputs(struct wacom *wacom)
{
struct input_dev *pen_input_dev, *touch_input_dev, *pad_input_dev;
struct wacom_wac *wacom_wac = &(wacom->wacom_wac);
@@ -2106,10 +2106,6 @@ static int wacom_register_inputs(struct wacom *wacom)
input_free_device(pen_input_dev);
wacom_wac->pen_input = NULL;
pen_input_dev = NULL;
- } else {
- error = input_register_device(pen_input_dev);
- if (error)
- goto fail;
}
error = wacom_setup_touch_input_capabilities(touch_input_dev, wacom_wac);
@@ -2118,10 +2114,6 @@ static int wacom_register_inputs(struct wacom *wacom)
input_free_device(touch_input_dev);
wacom_wac->touch_input = NULL;
touch_input_dev = NULL;
- } else {
- error = input_register_device(touch_input_dev);
- if (error)
- goto fail;
}
error = wacom_setup_pad_input_capabilities(pad_input_dev, wacom_wac);
@@ -2130,7 +2122,34 @@ static int wacom_register_inputs(struct wacom *wacom)
input_free_device(pad_input_dev);
wacom_wac->pad_input = NULL;
pad_input_dev = NULL;
- } else {
+ }
+
+ return 0;
+}
+
+static int wacom_register_inputs(struct wacom *wacom)
+{
+ struct input_dev *pen_input_dev, *touch_input_dev, *pad_input_dev;
+ struct wacom_wac *wacom_wac = &(wacom->wacom_wac);
+ int error = 0;
+
+ pen_input_dev = wacom_wac->pen_input;
+ touch_input_dev = wacom_wac->touch_input;
+ pad_input_dev = wacom_wac->pad_input;
+
+ if (pen_input_dev) {
+ error = input_register_device(pen_input_dev);
+ if (error)
+ goto fail;
+ }
+
+ if (touch_input_dev) {
+ error = input_register_device(touch_input_dev);
+ if (error)
+ goto fail;
+ }
+
+ if (pad_input_dev) {
error = input_register_device(pad_input_dev);
if (error)
goto fail;
@@ -2383,6 +2402,20 @@ static int wacom_parse_and_register(struct wacom *wacom, bool wireless)
if (error)
goto fail;
+ error = wacom_setup_inputs(wacom);
+ if (error)
+ goto fail;
+
+ if (features->type == HID_GENERIC)
+ connect_mask |= HID_CONNECT_DRIVER;
+
+ /* Regular HID work starts now */
+ error = hid_hw_start(hdev, connect_mask);
+ if (error) {
+ hid_err(hdev, "hw start failed\n");
+ goto fail;
+ }
+
error = wacom_register_inputs(wacom);
if (error)
goto fail;
@@ -2397,16 +2430,6 @@ static int wacom_parse_and_register(struct wacom *wacom, bool wireless)
goto fail;
}
- if (features->type == HID_GENERIC)
- connect_mask |= HID_CONNECT_DRIVER;
-
- /* Regular HID work starts now */
- error = hid_hw_start(hdev, connect_mask);
- if (error) {
- hid_err(hdev, "hw start failed\n");
- goto fail;
- }
-
if (!wireless) {
/* Note that if query fails it is not a hard failure */
wacom_query_tablet_data(wacom);
--
2.43.0
^ permalink raw reply related
* Re: [PATCH] HID: wacom: Do not register input devices until after hid_hw_start
From: Dmitry Torokhov @ 2024-01-30 0:03 UTC (permalink / raw)
To: Gerecke, Jason
Cc: linux-input, Jiri Kosina, Benjamin Tissoires, Ping Cheng,
Joshua Dickens, Aaron Armstrong Skomra, Tobita, Tatsunosuke,
stable, Jason Gerecke
In-Reply-To: <20240129223545.219878-1-jason.gerecke@wacom.com>
On Mon, Jan 29, 2024 at 02:35:45PM -0800, Gerecke, Jason wrote:
> From: Jason Gerecke <killertofu@gmail.com>
>
> If a input device is opened before hid_hw_start is called, events may
> not be received from the hardware. In the case of USB-backed devices,
> for example, the hid_hw_start function is responsible for filling in
> the URB which is submitted when the input device is opened. If a device
> is opened prematurely, polling will never start because the device will
> not have been in the correct state to send the URB.
>
> Because the wacom driver registers its input devices before calling
> hid_hw_start, there is a window of time where a device can be opened
> and end up in an inoperable state. Some ARM-based Chromebooks in particular
> reliably trigger this bug.
>
> This commit splits the wacom_register_inputs function into two pieces.
> One which is responsible for setting up the allocated inputs (and runs
> prior to hid_hw_start so that devices are ready for any input events
> they may end up receiving) and another which only registers the devices
> (and runs after hid_hw_start to ensure devices can be immediately opened
> without issue). Note that the functions to initialize the LEDs and remotes
> are also moved after hid_hw_start to maintain their own dependency chains.
>
> Fixes: 7704ac937345 ("HID: wacom: implement generic HID handling for pen generic devices")
> Cc: stable@vger.kernel.org # v3.18+
> Suggested-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> Signed-off-by: Jason Gerecke <jason.gerecke@wacom.com>
Tested-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Thanks.
--
Dmitry
^ permalink raw reply
* [PATCH v2] HID: logitech-dj: allow mice to report multimedia keycodes
From: Yaraslau Furman @ 2024-01-30 11:17 UTC (permalink / raw)
To: Benjamin Tissoires
Cc: Yaraslau Furman, Filipe Laíns, Jiri Kosina,
open list:HID LOGITECH DRIVERS, open list
In-Reply-To: <20240128214906.60606-1-yaro330@gmail.com>
Multimedia buttons can be bound to the mouse's extra keys in Windows application.
Let Linux receive those keycodes.
Signed-off-by: Yaraslau Furman <yaro330@gmail.com>
---
drivers/hid/hid-logitech-dj.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/hid/hid-logitech-dj.c b/drivers/hid/hid-logitech-dj.c
index e6a8b6d8eab7..3c3c497b6b91 100644
--- a/drivers/hid/hid-logitech-dj.c
+++ b/drivers/hid/hid-logitech-dj.c
@@ -965,9 +965,7 @@ static void logi_hidpp_dev_conn_notif_equad(struct hid_device *hdev,
}
break;
case REPORT_TYPE_MOUSE:
- workitem->reports_supported |= STD_MOUSE | HIDPP;
- if (djrcv_dev->type == recvr_type_mouse_only)
- workitem->reports_supported |= MULTIMEDIA;
+ workitem->reports_supported |= STD_MOUSE | HIDPP | MULTIMEDIA;
break;
}
}
--
2.43.0
^ permalink raw reply related
* Re: Implement per-key keyboard backlight as auxdisplay?
From: Werner Sembach @ 2024-01-30 11:12 UTC (permalink / raw)
To: Hans de Goede, Pavel Machek
Cc: Jani Nikula, jikos, Jelle van der Waa, Miguel Ojeda, Lee Jones,
linux-kernel, dri-devel@lists.freedesktop.org, linux-input, ojeda,
linux-leds
In-Reply-To: <7228f2c6-fbdd-4e19-b703-103b8535d77d@redhat.com>
Hi Hans,
Am 29.01.24 um 14:24 schrieb Hans de Goede:
> Hi Werner,
>
> On 1/19/24 17:04, Werner Sembach wrote:
>> Am 19.01.24 um 09:44 schrieb Hans de Goede:
> <snip>
>
>>> So my proposal would be an ioctl interface (ioctl only no r/w)
>>> using /dev/rgbkbd0 /dev/rgbkdb1, etc. registered as a misc chardev.
>>>
>>> For per key controllable rgb LEDs we need to discuss a coordinate
>>> system. I propose using a fixed size of 16 rows of 64 keys,
>>> so 64x16 in standard WxH notation.
>>>
>>> And then storing RGB in separate bytes, so userspace will then
>>> always send a buffer of 192 bytes per line (64x3) x 14 rows
>>> = 3072 bytes. With the kernel driver ignoring parts of
>>> the buffer where there are no actual keys.
>> The be sure the "14 rows" is a typo? And should be 16 rows?
> Yes that should be 16.
>
> <snip>
>
>>> This way we can address all the possible keys in the various
>>> standard layouts in one standard wat and then the drivers can
>>> just skip keys which are not there when preparing the buffer
>>> to send to the hw / fw.
>> Some remarks here:
>>
>> - Some keyboards might have two or more leds for big keys like (iso-)enter, shift, capslock, num+, etc. that in theory are individually controllable by the firmware. In windows drivers this is usually abstracted away, but could be interesting for effects (e.g. if the top of iso-enter is separate from the bottom of iso-enter like with one of our devices).
>>
>> - In combination with this: The driver might not be able to tell if the actual physical keyboard is ISO or ANSI, so it might not be able the correctly assign the leds around enter correctly as being an own key or being part of ANSI- or ISO-enter.
>>
>> - Should the interface have different addresses for the different enter and num+ styles (or even the different length shifts and spacebars)?
>>
>> One idea for this: Actually assign 1 value per line for tall keys per line, 3 (or maybe even 4, to have one spare) values per line for wide keys and 6 (or 8) values for space. e.g.:
> That sounds workable OTOH combined with your remarks about also supporting
> lightbars. I'm starting to think that we need to just punt this to userspace.
>
> So basically change things from trying to present a standardized address
> space where say the 'Q' key is always in the same place just model
> a keyboard as a string of LEDs (1 dimensional / so an array) and leave
> mapping which address in the array is which key to userspace, then userspace
> can have json or whatever files for this per keyboard.
>
> This keeps the kernel interface much more KISS which I think is what
> we need to strive for.
>
> So instead of having /dev/rgbkbd we get a /dev/rgbledstring and then
> that can be used for rbb-kbds and also your lightbar example as well
> as actual RGB LED strings, which depending on the controller may
> also have zones / effects, etc. just like the keyboards.
>
>
>
>> - Right shift would have 3 values in row 10. The first value might be the left side of shift or the additional ABNT/JIS key. The 2nd Key might be the left side or middle of shift and the third key might be the right side of shift or the only value for the whole key. The additional ABNT/JIS key still also has a dedicated value which is used by drivers which can differentiate between physical layouts.
>>
>> - Enter would have 3 values in row 8 and 3 values in row 9. With the same disambiguation as the additional ABNT/JIS but this time for ansii-/ and iso-#
>>
>> - Num+ would have 2 values, one row 8 and one in row 9. The one in row 9 might control the whole key or might just control the lower half. The one in row 8 might be another key or the upper half
>>
>> For the left half if the main block the leftmost value should be the "might be the only relevant"-value while the right most value should be the "might be another key"-value. For the right side of the main block this should be swapped. Unused values should be adjacent to the "might be another key"-value, e.g.:
>>
>> | Left shift value 1 | Left shift value 2 | Left shift value 3 | Left shift value 4 | 102nd key value
>> ISO/ANSI aware | Left shift color | Unused | Unused | Unused | 102nd key color
>> ISO non aware 1 led under shift | Left shift color | Unused | Unused | 102nd key color | Unused
>> ANSI non aware 1 led under shift | Left shift color | Unused | Unused | Unused | Unused
>> ISO non aware 2 leds under shift | Left shift left color | Left shift right color | Unused | 102nd key color | Unused
>> ANSI non aware 2 leds under shift | Left shift left color | Left shift right color | Unused | Unused | Unused
>> ISO non aware 3 leds under shift | Left shift left color | Left shift middle color | Left shift right color | 102nd key color | Unused
>> ANSI non aware 3 leds under shift | Left shift left color | Left shift middle color | Unused | Left shift right color | Unused
>> ANSI non aware 4 leds under shift | Left shift left color | Left shift middle left color | Left shift middle right color | Left shift right color | Unused
>>
>> Like this with no information you can still reliable target the ANSI-shift space, if you know it's an ISO keyboard from user space you can target shift and 102nd key, and if you have even more information you can have multi color shift if the firmware supports it.
> Right, so see above I think we need to push all these complications
> into userspace. And simple come up for a kernel interface
> for RGB LED strings with zones / effects / possibly individual
> addressable LEDs.
>
> Also we should really only use whatever kernel interface we come up
> with for devices which cannot be supported directly from userspace
> through e.g. hidraw access. Looking at keyboards then the openrgb project:
>
> https://openrgb.org/devices_0.9.html
>
> Currently already supports 398 keyboard modes, we really do not want
> to be adding support for all those to the kernel.
I think that are mostly external keyboards, so in theory a possible cut could
also between built-in and external devices.
>
> Further down in the thread you mention Mice with RGB LEDs,
> Mice are almost always HID devices and already have extensive support,
> including for their LEDs in userspace through libratbag and the piper UI,
> see the screenshots here (click on the camera icon):
> https://linux.softpedia.com/get/Utilities/Piper-libratbag-104168.shtml
>
> Again we really don't want to be re-doing all this work in the kernel
> only to end up conflicting with the existing userspace support.
>
> <snip>
>
>>> 5. A set_effect ioctl which takes a struct with the following members:
>>>
>>> {
>>> long size; /* Size of passed in struct including the size member itself */
>>> int effect_nr; /* enum value of the effect to enable, 0 for disable effect */
>>> int zone; /* zone to apply the effect to */
>> Don't know if this is necessary, the keyboards I have seen so far apply firmware effects globally.
>>> int speed; /* cycle speed of the effect in milli-hz */
>> I would split this into speed and speed_max and don't specify an actual unit. The firmwares effects I have seen so far: If they have a speed value, it's some low number interpreted as a proportional x/n * the max speed of this effect, with n being some low number like 8 or 10.
>>
>> But i don't know if such clearly named properties are even sensefull, see below.
>>
>>> char color1[3]; /* effect dependend may be unused. */
>>> char color2[3]; /* effect dependend may be unused. */
>>> }
>> We can not predetermine how many colors we might need in the future.
>>
>> Firmware effects can vary vastly in complexity, e.g. breathing can be a single bit switch that just varies the brightness of whatever color setting is currently applied. It could have an optional speed argument. It could have nth additional color arguments to cycle through, it could have an optional randomize bit that either randomizes the order of the defined colors or means that it is picking completely random color ignoring the color settings if set.
>>
>> Like this we could have a very fast explosion of the effects enum e.g.: breathing, breathing_2_colors, breathing_3_colors, ... breathing_n_colors, breathing_speed_controlled, breathing_speed_controlled_2_colors, ... breathing_speed_controlled_n_colors_random_bit, etc.
>>
>> Or we give up on generic names and just make something like: tongfang_breathing_1, tongfang_scan_1, tongfang_breathing_2, clevo_breathing_1
>>
>> Each with an own struct defined in a big .h file.
>>
>> Otherwise I think the config struct needs to be dynamically created out of information the driver gives to userspace.
> Given that as mentioned above I believe that we should only use a kernel
> driver where direct userspace access is impossible I believe that having
> things like tongfang_breathing_1, tongfang_scan_1, tongfang_breathing_2,
> clevo_breathing_1, etc. for the hopefully small set of devices which
> needs an actual kernel driver to be reasonable.
So also no basic driver? Or still the concept from before with a basic 1 zone
only driver via leds subsystem to have something working, but it is unregistered
by userspace, if open rgb wants to take over for fine granular support?
>
> Talking about existing RGB LED support I believe that we should also
> reach out to and get feedback on (or even an ack for) the new rgbledstring
> API from the OpenRGB folks: https://openrgb.org
>
> Maybe they already have a nice abstraction to deal with different
> kind of effects which we can copy for the kernel API ?
I opened an issue regarding this:
https://gitlab.com/CalcProgrammer1/OpenRGB/-/issues/3916
>
> Regards,
>
> Hans
>
>
>
Kind regards,
Werner
^ permalink raw reply
* Re: Implement per-key keyboard backlight as auxdisplay?
From: Hans de Goede @ 2024-01-30 17:10 UTC (permalink / raw)
To: Werner Sembach, Pavel Machek
Cc: Jani Nikula, jikos, Jelle van der Waa, Miguel Ojeda, Lee Jones,
linux-kernel, dri-devel@lists.freedesktop.org, linux-input, ojeda,
linux-leds
In-Reply-To: <730bead8-6e1d-4d21-90d2-4ee73155887a@tuxedocomputers.com>
Hi Werner,
On 1/30/24 12:12, Werner Sembach wrote:
> Hi Hans,
>
> Am 29.01.24 um 14:24 schrieb Hans de Goede:
<snip>
>> That sounds workable OTOH combined with your remarks about also supporting
>> lightbars. I'm starting to think that we need to just punt this to userspace.
>>
>> So basically change things from trying to present a standardized address
>> space where say the 'Q' key is always in the same place just model
>> a keyboard as a string of LEDs (1 dimensional / so an array) and leave
>> mapping which address in the array is which key to userspace, then userspace
>> can have json or whatever files for this per keyboard.
>>
>> This keeps the kernel interface much more KISS which I think is what
>> we need to strive for.
>>
>> So instead of having /dev/rgbkbd we get a /dev/rgbledstring and then
>> that can be used for rbb-kbds and also your lightbar example as well
>> as actual RGB LED strings, which depending on the controller may
>> also have zones / effects, etc. just like the keyboards.
<snip>
>> Right, so see above I think we need to push all these complications
>> into userspace. And simple come up for a kernel interface
>> for RGB LED strings with zones / effects / possibly individual
>> addressable LEDs.
>>
>> Also we should really only use whatever kernel interface we come up
>> with for devices which cannot be supported directly from userspace
>> through e.g. hidraw access. Looking at keyboards then the openrgb project:
>>
>> https://openrgb.org/devices_0.9.html
>>
>> Currently already supports 398 keyboard modes, we really do not want
>> to be adding support for all those to the kernel.
> I think that are mostly external keyboards, so in theory a possible cut could also between built-in and external devices.
IMHO it would be better to limit /dev/rgbledstring use to only
cases where direct userspace control is not possible and thus
have the cut be based on whether direct userspace control
(e.g. /dev/hidraw access) is possible or not.
>> Further down in the thread you mention Mice with RGB LEDs,
>> Mice are almost always HID devices and already have extensive support,
>> including for their LEDs in userspace through libratbag and the piper UI,
>> see the screenshots here (click on the camera icon):
>> https://linux.softpedia.com/get/Utilities/Piper-libratbag-104168.shtml
>>
>> Again we really don't want to be re-doing all this work in the kernel
>> only to end up conflicting with the existing userspace support.
>>
>> <snip>
>>
>>>> 5. A set_effect ioctl which takes a struct with the following members:
>>>>
>>>> {
>>>> long size; /* Size of passed in struct including the size member itself */
>>>> int effect_nr; /* enum value of the effect to enable, 0 for disable effect */
>>>> int zone; /* zone to apply the effect to */
>>> Don't know if this is necessary, the keyboards I have seen so far apply firmware effects globally.
>>>> int speed; /* cycle speed of the effect in milli-hz */
>>> I would split this into speed and speed_max and don't specify an actual unit. The firmwares effects I have seen so far: If they have a speed value, it's some low number interpreted as a proportional x/n * the max speed of this effect, with n being some low number like 8 or 10.
>>>
>>> But i don't know if such clearly named properties are even sensefull, see below.
>>>
>>>> char color1[3]; /* effect dependend may be unused. */
>>>> char color2[3]; /* effect dependend may be unused. */
>>>> }
>>> We can not predetermine how many colors we might need in the future.
>>>
>>> Firmware effects can vary vastly in complexity, e.g. breathing can be a single bit switch that just varies the brightness of whatever color setting is currently applied. It could have an optional speed argument. It could have nth additional color arguments to cycle through, it could have an optional randomize bit that either randomizes the order of the defined colors or means that it is picking completely random color ignoring the color settings if set.
>>>
>>> Like this we could have a very fast explosion of the effects enum e.g.: breathing, breathing_2_colors, breathing_3_colors, ... breathing_n_colors, breathing_speed_controlled, breathing_speed_controlled_2_colors, ... breathing_speed_controlled_n_colors_random_bit, etc.
>>>
>>> Or we give up on generic names and just make something like: tongfang_breathing_1, tongfang_scan_1, tongfang_breathing_2, clevo_breathing_1
>>>
>>> Each with an own struct defined in a big .h file.
>>>
>>> Otherwise I think the config struct needs to be dynamically created out of information the driver gives to userspace.
>> Given that as mentioned above I believe that we should only use a kernel
>> driver where direct userspace access is impossible I believe that having
>> things like tongfang_breathing_1, tongfang_scan_1, tongfang_breathing_2,
>> clevo_breathing_1, etc. for the hopefully small set of devices which
>> needs an actual kernel driver to be reasonable.
> So also no basic driver? Or still the concept from before with a basic 1 zone only driver via leds subsystem to have something working, but it is unregistered by userspace, if open rgb wants to take over for fine granular support?
Ah good point, no I think that a basic driver just for kbd backlight
brightness support which works with the standard desktop environment
controls for this makes sense.
Combined with some mechanism for e.g. openrgb to fully take over
control as discussed. It is probably a good idea to file a separate
issue with the openrgb project to discuss the takeover API.
>> Talking about existing RGB LED support I believe that we should also
>> reach out to and get feedback on (or even an ack for) the new rgbledstring
>> API from the OpenRGB folks: https://openrgb.org
>>
>> Maybe they already have a nice abstraction to deal with different
>> kind of effects which we can copy for the kernel API ?
> I opened an issue regarding this: https://gitlab.com/CalcProgrammer1/OpenRGB/-/issues/3916
Great, thank you.
Regards,
Hans
^ permalink raw reply
* Re: [PATCH v6 3/3] Input: Add TouchNetix axiom i2c touchscreen driver
From: POPESCU Catalin @ 2024-01-30 17:32 UTC (permalink / raw)
To: Kamel Bouhara, Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Henrik Rydberg, linux-input@vger.kernel.org,
linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
Marco Felsch, Jeff LaBundy
Cc: mark.satterthwaite@touchnetix.com, Thomas Petazzoni,
Gregory Clement, GEO-CHHER-bsp-development
In-Reply-To: <20240125165823.996910-4-kamel.bouhara@bootlin.com>
Hi Kamel,
I have a few remarks regarding the population of the usage table
function, see inline.
On 25.01.24 17:58, Kamel Bouhara wrote:
> [Some people who received this message don't often get email from kamel.bouhara@bootlin.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
>
> This email is not from Hexagon’s Office 365 instance. Please be careful while clicking links, opening attachments, or replying to this email.
>
>
> Add a new driver for the TouchNetix's axiom family of
> touchscreen controllers. This driver only supports i2c
> and can be later adapted for SPI and USB support.
>
> Signed-off-by: Kamel Bouhara <kamel.bouhara@bootlin.com>
> ---
> MAINTAINERS | 1 +
> drivers/input/touchscreen/Kconfig | 12 +
> drivers/input/touchscreen/Makefile | 1 +
> drivers/input/touchscreen/touchnetix_axiom.c | 664 +++++++++++++++++++
> 4 files changed, 678 insertions(+)
> create mode 100644 drivers/input/touchscreen/touchnetix_axiom.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 4752d8436dbb..337ddac6c74b 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -21436,6 +21436,7 @@ M: Kamel Bouhara <kamel.bouhara@bootlin.com>
> L: linux-input@vger.kernel.org
> S: Maintained
> F: Documentation/devicetree/bindings/input/touchscreen/touchnetix,ax54a.yaml
> +F: drivers/input/touchscreen/touchnetix_axiom.c
>
> THUNDERBOLT DMA TRAFFIC TEST DRIVER
> M: Isaac Hazan <isaac.hazan@intel.com>
> diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig
> index e3e2324547b9..f36bee8d8696 100644
> --- a/drivers/input/touchscreen/Kconfig
> +++ b/drivers/input/touchscreen/Kconfig
> @@ -803,6 +803,18 @@ config TOUCHSCREEN_MIGOR
> To compile this driver as a module, choose M here: the
> module will be called migor_ts.
>
> +config TOUCHSCREEN_TOUCHNETIX_AXIOM
> + tristate "TouchNetix AXIOM based touchscreen controllers"
> + depends on I2C
> + help
> + Say Y here if you have a axiom touchscreen connected to
> + your system.
> +
> + If unsure, say N.
> +
> + To compile this driver as a module, choose M here: the
> + module will be called axiom.
> +
> config TOUCHSCREEN_TOUCHRIGHT
> tristate "Touchright serial touchscreen"
> select SERIO
> diff --git a/drivers/input/touchscreen/Makefile b/drivers/input/touchscreen/Makefile
> index 62bd24f3ac8e..8e32a2df5e18 100644
> --- a/drivers/input/touchscreen/Makefile
> +++ b/drivers/input/touchscreen/Makefile
> @@ -88,6 +88,7 @@ obj-$(CONFIG_TOUCHSCREEN_SUR40) += sur40.o
> obj-$(CONFIG_TOUCHSCREEN_SURFACE3_SPI) += surface3_spi.o
> obj-$(CONFIG_TOUCHSCREEN_TI_AM335X_TSC) += ti_am335x_tsc.o
> obj-$(CONFIG_TOUCHSCREEN_TOUCHIT213) += touchit213.o
> +obj-$(CONFIG_TOUCHSCREEN_TOUCHNETIX_AXIOM) += touchnetix_axiom.o
> obj-$(CONFIG_TOUCHSCREEN_TOUCHRIGHT) += touchright.o
> obj-$(CONFIG_TOUCHSCREEN_TOUCHWIN) += touchwin.o
> obj-$(CONFIG_TOUCHSCREEN_TS4800) += ts4800-ts.o
> diff --git a/drivers/input/touchscreen/touchnetix_axiom.c b/drivers/input/touchscreen/touchnetix_axiom.c
> new file mode 100644
> index 000000000000..cbb5525b83f5
> --- /dev/null
> +++ b/drivers/input/touchscreen/touchnetix_axiom.c
> @@ -0,0 +1,664 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * TouchNetix axiom Touchscreen Driver
> + *
> + * Copyright (C) 2020-2023 TouchNetix Ltd.
> + *
> + * Author(s): Bart Prescott <bartp@baasheep.co.uk>
> + * Pedro Torruella <pedro.torruella@touchnetix.com>
> + * Mark Satterthwaite <mark.satterthwaite@touchnetix.com>
> + * Hannah Rossiter <hannah.rossiter@touchnetix.com>
> + * Kamel Bouhara <kamel.bouhara@bootlin.com>
> + *
> + */
> +#include <linux/bitfield.h>
> +#include <linux/crc16.h>
> +#include <linux/delay.h>
> +#include <linux/device.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/i2c.h>
> +#include <linux/input.h>
> +#include <linux/input/mt.h>
> +#include <linux/input/touchscreen.h>
> +#include <linux/interrupt.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/regmap.h>
> +
> +#include <asm/unaligned.h>
> +#define AXIOM_PROX_LEVEL -128
> +#define AXIOM_DMA_OPS_DELAY_USEC 250
> +/*
> + * Register group u31 has 2 pages for usage table entries.
> + */
> +#define AXIOM_U31_MAX_USAGES 0xff
> +#define AXIOM_U31_BYTES_PER_USAGE 6
> +#define AXIOM_U31_PAGE0_LENGTH 0x0C
> +#define AXIOM_U31_BOOTMODE_MASK BIT(7)
> +#define AXIOM_U31_DEVID_MASK GENMASK(14, 0)
> +
> +#define AXIOM_MAX_REPORT_LEN 0x7f
> +
> +#define AXIOM_CMD_HEADER_READ_MASK BIT(15)
> +#define AXIOM_U41_MAX_TARGETS 10
> +
> +#define AXIOM_U46_AUX_CHANNELS 4
> +#define AXIOM_U46_AUX_MASK GENMASK(11, 0)
> +
> +#define AXIOM_COMMS_MAX_USAGE_PAGES 3
> +#define AXIOM_COMMS_PAGE_SIZE 256
> +#define AXIOM_COMMS_REPORT_LEN_MASK GENMASK(6, 0)
> +
> +#define AXIOM_REPORT_USAGE_ID 0x34
> +#define AXIOM_DEVINFO_USAGE_ID 0x31
> +#define AXIOM_USAGE_2HB_REPORT_ID 0x01
> +#define AXIOM_USAGE_2AUX_REPORT_ID 0x46
> +#define AXIOM_USAGE_2DCTS_REPORT_ID 0x41
> +
> +#define AXIOM_PAGE_OFFSET_MASK GENMASK(6, 0)
> +
> +struct axiom_devinfo {
> + __le16 device_id;
> + u8 fw_minor;
> + u8 fw_major;
> + u8 fw_info_extra;
> + u8 tcp_revision;
> + u8 bootloader_fw_minor;
> + u8 bootloader_fw_major;
> + __le16 jedec_id;
> + u8 num_usages;
> +} __packed;
> +
> +/*
> + * Describes parameters of a specific usage, essentially a single element of
> + * the "Usage Table"
> + */
> +struct axiom_usage_entry {
> + u8 id;
> + u8 is_report;
> + u8 start_page;
> + u8 num_pages;
> +};
> +
> +/*
> + * Represents state of a touch or target when detected prior to a touch (eg.
> + * hover or proximity events).
> + */
> +enum axiom_target_state {
> + AXIOM_TARGET_STATE_NOT_PRESENT = 0,
> + AXIOM_TARGET_STATE_PROX = 1,
> + AXIOM_TARGET_STATE_HOVER = 2,
> + AXIOM_TARGET_STATE_TOUCHING = 3,
> +};
> +
> +struct axiom_u41_target {
> + enum axiom_target_state state;
> + u16 x;
> + u16 y;
> + s8 z;
> + bool insert;
> + bool touch;
> +};
> +
> +struct axiom_target_report {
> + u8 index;
> + u8 present;
> + u16 x;
> + u16 y;
> + s8 z;
> +};
> +
> +struct axiom_cmd_header {
> + __le16 target_address;
> + __le16 length;
> +} __packed;
> +
> +struct axiom_data {
> + struct axiom_devinfo devinfo;
> + struct device *dev;
> + struct gpio_desc *reset_gpio;
> + struct i2c_client *client;
> + struct input_dev *input_dev;
> + u32 max_report_len;
> + u8 rx_buf[AXIOM_COMMS_MAX_USAGE_PAGES * AXIOM_COMMS_PAGE_SIZE];
> + struct axiom_u41_target targets[AXIOM_U41_MAX_TARGETS];
> + struct axiom_usage_entry usage_table[AXIOM_U31_MAX_USAGES];
> + bool usage_table_populated;
> + struct regulator *vdda;
> + struct regulator *vddi;
> + struct regmap *regmap;
> + struct touchscreen_properties prop;
> +};
> +
> +static const struct regmap_config axiom_i2c_regmap_config = {
> + .reg_bits = 32,
> + .reg_format_endian = REGMAP_ENDIAN_LITTLE,
> + .val_bits = 8,
> + .val_format_endian = REGMAP_ENDIAN_LITTLE,
> +};
> +
> +/*
> + * axiom devices are typically configured to report touches at a rate
> + * of 100Hz (10ms) for systems that require polling for reports.
> + * When reports are polled, it will be expected to occasionally
> + * observe the overflow bit being set in the reports.
> + * This indicates that reports are not being read fast enough.
> + */
> +#define POLL_INTERVAL_DEFAULT_MS 10
> +
> +/* Translate usage/page/offset triplet into physical address. */
> +static u16 axiom_usage_to_target_address(struct axiom_data *ts, u8 usage, u8 page,
> + char offset)
> +{
> + /* At the moment the convention is that u31 is always at physical address 0x0 */
> + if (!ts->usage_table_populated) {
> + if (usage == AXIOM_DEVINFO_USAGE_ID)
> + return ((page << 8) + offset);
> + else
> + return 0xffff;
> + }
> +
> + if (page >= ts->usage_table[usage].num_pages) {
> + dev_err(ts->dev, "Invalid usage table! usage: u%02x, page: %02x, offset: %02x\n",
> + usage, page, offset);
> + return 0xffff;
> + }
> +
> + return ((ts->usage_table[usage].start_page + page) << 8) + offset;
> +}
> +
> +static int axiom_read(struct axiom_data *ts, u8 usage, u8 page, void *buf, u16 len)
> +{
> + struct axiom_cmd_header cmd_header;
> + int ret;
> +
> + cmd_header.target_address = cpu_to_le16(axiom_usage_to_target_address(ts, usage, page, 0));
> + cmd_header.length = cpu_to_le16(len | AXIOM_CMD_HEADER_READ_MASK);
> +
> + __le32 preamble = get_unaligned_le32((u8 *)&cmd_header);
> +
> + ret = regmap_write(ts->regmap, preamble, 0);
> + if (ret) {
> + dev_err(ts->dev, "failed to write preamble, error %d\n", ret);
> + return ret;
> + }
> +
> + ret = regmap_raw_read(ts->regmap, 0, buf, len);
> + if (ret) {
> + dev_err(ts->dev, "failed to read target address %04x, error %d\n",
> + cmd_header.target_address, ret);
> + return ret;
> + }
> +
> + /* Wait device's DMA operations */
> + usleep_range(AXIOM_DMA_OPS_DELAY_USEC, AXIOM_DMA_OPS_DELAY_USEC + 50);
> +
> + return 0;
> +}
> +
> +/*
> + * One of the main purposes for reading the usage table is to identify
> + * which usages reside at which target address.
> + * When performing subsequent reads or writes to AXIOM, the target address
> + * is used to specify which usage is being accessed.
> + * Consider the following discovery code which will build up the usage table.
> + */
> +static u32 axiom_populate_usage_table(struct axiom_data *ts)
> +{
> + struct axiom_usage_entry *usage_table;
> + u8 *rx_data = ts->rx_buf;
> + u32 max_report_len;
please force initialization of max_report_len to 0.
> + u32 usage_id;
> + int error;
> +
> + usage_table = ts->usage_table;
> +
> + /* Read the second page of usage u31 to get the usage table */
> + error = axiom_read(ts, AXIOM_DEVINFO_USAGE_ID, 1, rx_data,
> + (AXIOM_U31_BYTES_PER_USAGE * ts->devinfo.num_usages));
> +
> + if (error)
> + return error;
> +
> + for (usage_id = 1; usage_id < AXIOM_U31_MAX_USAGES; usage_id++) {
usage_id should start at 0, since it's used to compute the offset in the
page 1 of u31.
if you start with usage_id=1, then whatever is the 1st usage of the
usage table gets jumped out...
also, you should stop the loop at num_usages, otherwise you're going to
read garbage after the end of rx_data buffer...
this garbage would end up in wrong computation of max_report_len then in
failure of reading any touch report.
> + u16 offset = (usage_id * AXIOM_U31_BYTES_PER_USAGE);
> + u8 id = rx_data[offset + 0];
> + u8 start_page = rx_data[offset + 1];
> + u8 num_pages = rx_data[offset + 2];
> + u32 max_offset = ((rx_data[offset + 3] & AXIOM_PAGE_OFFSET_MASK) + 1) * 2;
> +
> + if (!num_pages)
> + usage_table[id].is_report = true;
please add an else statement to set is_report to false.
> +
> + /* Store the entry into the usage table */
> + usage_table[id].id = id;
> + usage_table[id].start_page = start_page;
> + usage_table[id].num_pages = num_pages;
> +
> + dev_dbg(ts->dev, "Usage u%02x Info: %*ph\n", id, AXIOM_U31_BYTES_PER_USAGE,
> + &rx_data[offset]);
> +
> + /* Identify the max report length the module will receive */
> + if (usage_table[id].is_report && max_offset > max_report_len)
> + max_report_len = max_offset;
> + }
> +
> + ts->usage_table_populated = true;
> +
> + return max_report_len;
> +}
> +
> +static int axiom_discover(struct axiom_data *ts)
> +{
> + int error;
> +
> + /*
> + * Fetch the first page of usage u31 to get the
> + * device information and the number of usages
> + */
> + error = axiom_read(ts, AXIOM_DEVINFO_USAGE_ID, 0, &ts->devinfo, AXIOM_U31_PAGE0_LENGTH);
> + if (error)
> + return error;
> +
> + dev_dbg(ts->dev, " Boot Mode : %s\n",
> + FIELD_GET(AXIOM_U31_BOOTMODE_MASK,
> + le16_to_cpu(ts->devinfo.device_id)) ? "BLP" : "TCP");
> + dev_dbg(ts->dev, " Device ID : %04lx\n",
> + FIELD_GET(AXIOM_U31_DEVID_MASK, le16_to_cpu(ts->devinfo.device_id)));
> + dev_dbg(ts->dev, " Firmware Rev : %02x.%02x\n", ts->devinfo.fw_major,
> + ts->devinfo.fw_minor);
> + dev_dbg(ts->dev, " Bootloader Rev : %02x.%02x\n", ts->devinfo.bootloader_fw_major,
> + ts->devinfo.bootloader_fw_minor);
> + dev_dbg(ts->dev, " FW Extra Info : %04x\n", ts->devinfo.fw_info_extra);
> + dev_dbg(ts->dev, " Silicon : %04x\n", le16_to_cpu(ts->devinfo.jedec_id));
> + dev_dbg(ts->dev, " Number usages : %04x\n", ts->devinfo.num_usages);
> +
> + ts->max_report_len = axiom_populate_usage_table(ts);
> + if (!ts->max_report_len || !ts->devinfo.num_usages ||
> + ts->max_report_len > AXIOM_MAX_REPORT_LEN) {
> + dev_err(ts->dev, "Invalid report length or usages number");
> + return -EINVAL;
> + }
> +
> + dev_dbg(ts->dev, "Max Report Length: %u\n", ts->max_report_len);
> +
> + return 0;
> +}
> +
> +/*
> + * Support function to axiom_process_u41_report.
> + * Generates input-subsystem events for every target.
> + * After calling this function the caller shall issue
> + * a Sync to the input sub-system.
> + */
> +static bool axiom_process_u41_report_target(struct axiom_data *ts,
> + struct axiom_target_report *target)
> +{
> + struct input_dev *input_dev = ts->input_dev;
> + struct axiom_u41_target *target_prev_state;
> + enum axiom_target_state current_state;
> + int slot;
> +
> + /* Verify the target index */
> + if (target->index >= AXIOM_U41_MAX_TARGETS) {
> + dev_err(ts->dev, "Invalid target index! %u\n", target->index);
> + return false;
> + }
> +
> + target_prev_state = &ts->targets[target->index];
> +
> + current_state = AXIOM_TARGET_STATE_NOT_PRESENT;
> +
> + if (target->present) {
> + if (target->z >= 0)
> + current_state = AXIOM_TARGET_STATE_TOUCHING;
> + else if (target->z > AXIOM_PROX_LEVEL && target->z < 0)
> + current_state = AXIOM_TARGET_STATE_HOVER;
> + else if (target->z == AXIOM_PROX_LEVEL)
> + current_state = AXIOM_TARGET_STATE_PROX;
> + }
> +
> + if (target_prev_state->state == current_state &&
> + target_prev_state->x == target->x &&
> + target_prev_state->y == target->y &&
> + target_prev_state->z == target->z)
> + return false;
> +
> + slot = target->index;
> +
> + dev_dbg(ts->dev, "U41 Target T%u, slot:%u present:%u, x:%u, y:%u, z:%d\n",
> + target->index, slot, target->present,
> + target->x, target->y, target->z);
> +
> + switch (current_state) {
> + case AXIOM_TARGET_STATE_NOT_PRESENT:
> + case AXIOM_TARGET_STATE_PROX:
> + if (!target_prev_state->insert)
> + break;
> + target_prev_state->insert = false;
> + input_mt_slot(input_dev, slot);
> +
> + if (!slot)
> + input_report_key(input_dev, BTN_TOUCH, 0);
> +
> + input_mt_report_slot_inactive(input_dev);
> + /*
> + * make sure the previous coordinates are
> + * all off screen when the finger comes back
> + */
> + target->x = 65535;
> + target->y = 65535;
> + target->z = AXIOM_PROX_LEVEL;
> + break;
> + case AXIOM_TARGET_STATE_HOVER:
> + case AXIOM_TARGET_STATE_TOUCHING:
> + target_prev_state->insert = true;
> + input_mt_slot(input_dev, slot);
> + input_report_abs(input_dev, ABS_MT_TRACKING_ID, slot);
> + input_report_abs(input_dev, ABS_MT_POSITION_X, target->x);
> + input_report_abs(input_dev, ABS_MT_POSITION_Y, target->y);
> +
> + if (current_state == AXIOM_TARGET_STATE_TOUCHING) {
> + input_report_abs(input_dev, ABS_MT_DISTANCE, 0);
> + input_report_abs(input_dev, ABS_DISTANCE, 0);
> + input_report_abs(input_dev, ABS_MT_PRESSURE, target->z);
> + input_report_abs(input_dev, ABS_PRESSURE, target->z);
> + } else {
> + input_report_abs(input_dev, ABS_MT_DISTANCE, -target->z);
> + input_report_abs(input_dev, ABS_DISTANCE, -target->z);
> + input_report_abs(input_dev, ABS_MT_PRESSURE, 0);
> + input_report_abs(input_dev, ABS_PRESSURE, 0);
> + }
> +
> + if (!slot)
> + input_report_key(input_dev, BTN_TOUCH, (current_state ==
> + AXIOM_TARGET_STATE_TOUCHING));
> + break;
> + default:
> + break;
> + }
> +
> + target_prev_state->state = current_state;
> + target_prev_state->x = target->x;
> + target_prev_state->y = target->y;
> + target_prev_state->z = target->z;
> +
> + return true;
> +}
> +
> +/*
> + * U41 is the output report of the 2D CTS and contains the status of targets
> + * (including contacts and pre-contacts) along with their X,Y,Z values.
> + * When a target has been removed (no longer detected),
> + * the corresponding X,Y,Z values will be zeroed.
> + */
> +static bool axiom_process_u41_report(struct axiom_data *ts, u8 *rx_buf)
> +{
> + struct axiom_target_report target;
> + bool update_done = false;
> + u16 target_status;
> + int i;
> +
> + target_status = get_unaligned_le16(rx_buf + 1);
> +
> + for (i = 0; i < AXIOM_U41_MAX_TARGETS; i++) {
> + u8 *target_step = &rx_buf[i * 4];
> +
> + target.index = i;
> + target.present = ((target_status & (1 << i)) != 0) ? 1 : 0;
> + target.x = get_unaligned_le16(target_step + 3);
> + target.y = get_unaligned_le16(target_step + 5);
> + target.z = (s8)(rx_buf[i + 43]);
> + update_done |= axiom_process_u41_report_target(ts, &target);
> + }
> +
> + return update_done;
> +}
> +
> +/*
> + * U46 report contains a low level measurement data generated by the capacitive
> + * displacement sensor (CDS) algorithms from the auxiliary channels.
> + * This information is useful when tuning multi-press to assess mechanical
> + * consistency in the unit's construction.
> + */
> +static void axiom_process_u46_report(struct axiom_data *ts, u8 *rx_buf)
> +{
> + struct input_dev *input_dev = ts->input_dev;
> + u32 event_value;
> + u16 aux_value;
> + int i;
> +
> + for (i = 0; i < AXIOM_U46_AUX_CHANNELS; i++) {
> + u8 *target_step = &rx_buf[i * 2];
> +
> + aux_value = get_unaligned_le16(target_step + 1) & AXIOM_U46_AUX_MASK;
> + event_value = (i << 16) | (aux_value);
> + input_event(input_dev, EV_MSC, MSC_RAW, event_value);
> + }
> +}
> +
> +/*
> + * Validates the crc and demultiplexes the axiom reports to the appropriate
> + * report handler
> + */
> +static int axiom_handle_events(struct axiom_data *ts)
> +{
> + struct input_dev *input_dev = ts->input_dev;
> + u8 *report_data = ts->rx_buf;
> + struct device *dev = ts->dev;
> + u16 crc_report;
> + u16 crc_calc;
> + int error;
> + u8 len;
> +
> + error = axiom_read(ts, AXIOM_REPORT_USAGE_ID, 0, report_data, ts->max_report_len);
> + if (error)
> + return error;
> +
> + len = (report_data[0] & AXIOM_COMMS_REPORT_LEN_MASK) << 1;
> + if (len <= 2) {
> + dev_err(dev, "Zero length report discarded.\n");
> + return -ENODATA;
> + }
> +
> + /* Validate the report CRC */
> + u8 *crc_bytes = &report_data[len];
> +
> + crc_report = get_unaligned_le16(crc_bytes - 2);
> + /* Length is in 16 bit words and remove the size of the CRC16 itself */
> + crc_calc = crc16(0, report_data, (len - 2));
> +
> + if (crc_calc != crc_report) {
> + dev_err(dev,
> + "CRC mismatch! Expected: %#x, Calculated CRC: %#x.\n",
> + crc_report, crc_calc);
> + return -EINVAL;
> + }
> +
> + switch (report_data[1]) {
> + case AXIOM_USAGE_2DCTS_REPORT_ID:
> + if (axiom_process_u41_report(ts, &report_data[1])) {
> + input_mt_sync_frame(input_dev);
> + input_sync(input_dev);
> + }
> + break;
> +
> + case AXIOM_USAGE_2AUX_REPORT_ID:
> + /* This is an aux report (force) */
> + axiom_process_u46_report(ts, &report_data[1]);
> + input_mt_sync(input_dev);
> + input_sync(input_dev);
> + break;
> +
> + case AXIOM_USAGE_2HB_REPORT_ID:
> + /* This is a heartbeat report */
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> +static void axiom_i2c_poll(struct input_dev *input_dev)
> +{
> + struct axiom_data *ts = input_get_drvdata(input_dev);
> +
> + axiom_handle_events(ts);
> +}
> +
> +static irqreturn_t axiom_irq(int irq, void *dev_id)
> +{
> + struct axiom_data *ts = dev_id;
> +
> + axiom_handle_events(ts);
> +
> + return IRQ_HANDLED;
> +}
> +
> +static void axiom_reset(struct gpio_desc *reset_gpio)
> +{
> + gpiod_set_value_cansleep(reset_gpio, 1);
> + usleep_range(1000, 2000);
> + gpiod_set_value_cansleep(reset_gpio, 0);
> + msleep(110);
> +}
> +
> +static int axiom_i2c_probe(struct i2c_client *client)
> +{
> + u32 poll_interval = POLL_INTERVAL_DEFAULT_MS;
> + struct device *dev = &client->dev;
> + struct input_dev *input_dev;
> + struct axiom_data *ts;
> + u32 startup_delay_ms;
> + int target;
> + int error;
> +
> + ts = devm_kzalloc(dev, sizeof(*ts), GFP_KERNEL);
> + if (!ts)
> + return -ENOMEM;
> +
> + i2c_set_clientdata(client, ts);
> + ts->client = client;
> + ts->dev = dev;
> +
> + ts->regmap = devm_regmap_init_i2c(client, &axiom_i2c_regmap_config);
> + error = PTR_ERR_OR_ZERO(ts->regmap);
> + if (error) {
> + dev_err(dev, "Failed to initialize regmap: %d\n", error);
> + return error;
> + }
> +
> + ts->reset_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
> + if (IS_ERR(ts->reset_gpio))
> + return dev_err_probe(dev, PTR_ERR(ts->reset_gpio), "failed to get reset GPIO\n");
> +
> + if (ts->reset_gpio)
> + axiom_reset(ts->reset_gpio);
> +
> + ts->vddi = devm_regulator_get_optional(dev, "vddi");
> + if (!IS_ERR(ts->vddi)) {
> + error = devm_regulator_get_enable(dev, "vddi");
> + if (error)
> + return dev_err_probe(&client->dev, error,
> + "Failed to enable vddi regulator\n");
> + }
> +
> + ts->vdda = devm_regulator_get_optional(dev, "vdda");
> + if (!IS_ERR(ts->vdda)) {
> + error = devm_regulator_get_enable(dev, "vdda");
> + if (error)
> + return dev_err_probe(&client->dev, error,
> + "Failed to enable vdda regulator\n");
> + if (!device_property_read_u32(dev, "startup-time-ms", &startup_delay_ms))
> + msleep(startup_delay_ms);
> + }
> +
> + error = axiom_discover(ts);
> + if (error)
> + return dev_err_probe(dev, error, "Failed touchscreen discover\n");
> +
> + input_dev = devm_input_allocate_device(ts->dev);
> + if (!input_dev)
> + return -ENOMEM;
> +
> + input_dev->name = "TouchNetix axiom Touchscreen";
> + input_dev->phys = "input/axiom_ts";
> +
> + input_set_abs_params(input_dev, ABS_MT_POSITION_X, 0, 65535, 0, 0);
> + input_set_abs_params(input_dev, ABS_MT_POSITION_Y, 0, 65535, 0, 0);
> + input_set_abs_params(input_dev, ABS_MT_TOOL_TYPE, 0, MT_TOOL_MAX, 0, 0);
> + input_set_abs_params(input_dev, ABS_MT_DISTANCE, 0, 127, 0, 0);
> + input_set_abs_params(input_dev, ABS_MT_PRESSURE, 0, 127, 0, 0);
> +
> + touchscreen_parse_properties(input_dev, true, &ts->prop);
> +
> + /* Registers the axiom device as a touchscreen instead of a mouse pointer */
> + error = input_mt_init_slots(input_dev, AXIOM_U41_MAX_TARGETS, INPUT_MT_DIRECT);
> + if (error)
> + return error;
> +
> + /* Enables the raw data for up to 4 force channels to be sent to the input subsystem */
> + set_bit(EV_REL, input_dev->evbit);
> + set_bit(EV_MSC, input_dev->evbit);
> + /* Declare that we support "RAW" Miscellaneous events */
> + set_bit(MSC_RAW, input_dev->mscbit);
> +
> + ts->input_dev = input_dev;
> + input_set_drvdata(ts->input_dev, ts);
> +
> + /* Ensure that all reports are initialised to not be present. */
> + for (target = 0; target < AXIOM_U41_MAX_TARGETS; target++)
> + ts->targets[target].state = AXIOM_TARGET_STATE_NOT_PRESENT;
> +
> + error = input_register_device(input_dev);
> + if (error)
> + return dev_err_probe(ts->dev, error,
> + "Could not register with Input Sub-system.\n");
> +
> + error = devm_request_threaded_irq(dev, client->irq, NULL,
> + axiom_irq, IRQF_ONESHOT, dev_name(dev), ts);
> + if (error) {
> + dev_info(dev, "Request irq failed, falling back to polling mode");
> +
> + error = input_setup_polling(input_dev, axiom_i2c_poll);
> + if (error)
> + return dev_err_probe(ts->dev, error, "Unable to set up polling mode\n");
> +
> + if (!device_property_read_u32(ts->dev, "poll-interval", &poll_interval))
> + input_set_poll_interval(input_dev, poll_interval);
> + }
> +
> + return 0;
> +}
> +
> +static const struct i2c_device_id axiom_i2c_id_table[] = {
> + { "ax54a" },
> + { },
> +};
> +MODULE_DEVICE_TABLE(i2c, axiom_i2c_id_table);
> +
> +static const struct of_device_id axiom_i2c_of_match[] = {
> + { .compatible = "touchnetix,ax54a", },
> + { }
> +};
> +MODULE_DEVICE_TABLE(of, axiom_i2c_of_match);
> +
> +static struct i2c_driver axiom_i2c_driver = {
> + .driver = {
> + .name = "axiom",
> + .of_match_table = axiom_i2c_of_match,
> + },
> + .id_table = axiom_i2c_id_table,
> + .probe = axiom_i2c_probe,
> +};
> +module_i2c_driver(axiom_i2c_driver);
> +
> +MODULE_AUTHOR("Bart Prescott <bartp@baasheep.co.uk>");
> +MODULE_AUTHOR("Pedro Torruella <pedro.torruella@touchnetix.com>");
> +MODULE_AUTHOR("Mark Satterthwaite <mark.satterthwaite@touchnetix.com>");
> +MODULE_AUTHOR("Hannah Rossiter <hannah.rossiter@touchnetix.com>");
> +MODULE_AUTHOR("Kamel Bouhara <kamel.bouhara@bootlin.com>");
> +MODULE_DESCRIPTION("TouchNetix axiom touchscreen I2C bus driver");
> +MODULE_LICENSE("GPL");
> --
> 2.25.1
>
^ permalink raw reply
* Re: Implement per-key keyboard backlight as auxdisplay?
From: Werner Sembach @ 2024-01-30 18:09 UTC (permalink / raw)
To: Hans de Goede, Pavel Machek
Cc: Jani Nikula, jikos, Jelle van der Waa, Miguel Ojeda, Lee Jones,
linux-kernel, dri-devel@lists.freedesktop.org, linux-input, ojeda,
linux-leds
In-Reply-To: <952409e1-2f0e-4d7a-a7a9-3b78f2eafec7@redhat.com>
Hi Hans,
resend because Thunderbird htmlified the mail :/
Am 30.01.24 um 18:10 schrieb Hans de Goede:
> Hi Werner,
>
> On 1/30/24 12:12, Werner Sembach wrote:
>> Hi Hans,
>>
>> Am 29.01.24 um 14:24 schrieb Hans de Goede:
<snip>
>> I think that are mostly external keyboards, so in theory a possible cut could also between built-in and external devices.
> IMHO it would be better to limit /dev/rgbledstring use to only
> cases where direct userspace control is not possible and thus
> have the cut be based on whether direct userspace control
> (e.g. /dev/hidraw access) is possible or not.
Ack
<snip>
>> So also no basic driver? Or still the concept from before with a basic 1 zone only driver via leds subsystem to have something working, but it is unregistered by userspace, if open rgb wants to take over for fine granular support?
> Ah good point, no I think that a basic driver just for kbd backlight
> brightness support which works with the standard desktop environment
> controls for this makes sense.
>
> Combined with some mechanism for e.g. openrgb to fully take over
> control as discussed. It is probably a good idea to file a separate
> issue with the openrgb project to discuss the takeover API.
I think the OpenRGB maintainers are pretty flexible at that point, after all
it's similar to enable commands a lot of rgb devices need anyway. I would
include it in a full api proposal.
On this note: Any particular reason you suggested an ioctl interface instead of
a sysfs one? (Open question as, for example, I have no idea what performance
implications both have)
<snip>
>> I opened an issue regarding this:https://gitlab.com/CalcProgrammer1/OpenRGB/-/issues/3916
> Great, thank you.
First replies are in.
> Regards,
>
> Hans
Kind regards,
Werner
^ permalink raw reply
* Re: [PATCH v4 4/5] dt-bindings: input/touchscreen: imagis: add compatible for IST3032C
From: Rob Herring @ 2024-01-30 18:16 UTC (permalink / raw)
To: Karel Balej
Cc: Henrik Rydberg, ~postmarketos/upstreaming, Duje Mihanović,
linux-input, Conor Dooley, Rob Herring, linux-kernel,
Dmitry Torokhov, phone-devel, Markuss Broks, Krzysztof Kozlowski,
devicetree
In-Reply-To: <20240120191940.3631-5-karelb@gimli.ms.mff.cuni.cz>
On Sat, 20 Jan 2024 20:11:15 +0100, Karel Balej wrote:
> From: Karel Balej <balejk@matfyz.cz>
>
> IST3032C is a touchscreen IC which seems mostly compatible with IST3038C
> except that it reports a different chip ID value.
>
> Signed-off-by: Karel Balej <balejk@matfyz.cz>
> ---
>
> Notes:
> v4:
> * Reword commit description to mention how this IC differs from the
> already supported.
>
> .../devicetree/bindings/input/touchscreen/imagis,ist3038c.yaml | 1 +
> 1 file changed, 1 insertion(+)
>
Acked-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* Re: Implement per-key keyboard backlight as auxdisplay?
From: Hans de Goede @ 2024-01-30 18:35 UTC (permalink / raw)
To: Werner Sembach, Pavel Machek
Cc: Jani Nikula, jikos, Jelle van der Waa, Miguel Ojeda, Lee Jones,
linux-kernel, dri-devel@lists.freedesktop.org, linux-input, ojeda,
linux-leds
In-Reply-To: <9851a06d-956e-4b57-be63-e10ff1fce8b4@tuxedocomputers.com>
Hi,
On 1/30/24 19:09, Werner Sembach wrote:
> Hi Hans,
>
> resend because Thunderbird htmlified the mail :/
I use thunderbird too. If you right click on the server name
and then go to "Settings" -> "Composition & Addressing"
and then uncheck "Compose messages in HTML format"
I think that should do the trick.
> Am 30.01.24 um 18:10 schrieb Hans de Goede:
>> Hi Werner,
>>
>> On 1/30/24 12:12, Werner Sembach wrote:
>>> Hi Hans,
>>>
>>> Am 29.01.24 um 14:24 schrieb Hans de Goede:
> <snip>
>>> I think that are mostly external keyboards, so in theory a possible cut could also between built-in and external devices.
>> IMHO it would be better to limit /dev/rgbledstring use to only
>> cases where direct userspace control is not possible and thus
>> have the cut be based on whether direct userspace control
>> (e.g. /dev/hidraw access) is possible or not.
>
> Ack
>
> <snip>
>
>>> So also no basic driver? Or still the concept from before with a basic 1 zone only driver via leds subsystem to have something working, but it is unregistered by userspace, if open rgb wants to take over for fine granular support?
>> Ah good point, no I think that a basic driver just for kbd backlight
>> brightness support which works with the standard desktop environment
>> controls for this makes sense.
>>
>> Combined with some mechanism for e.g. openrgb to fully take over
>> control as discussed. It is probably a good idea to file a separate
>> issue with the openrgb project to discuss the takeover API.
>
> I think the OpenRGB maintainers are pretty flexible at that point, after all it's similar to enable commands a lot of rgb devices need anyway. I would include it in a full api proposal.
Ack.
> On this note: Any particular reason you suggested an ioctl interface instead of a sysfs one? (Open question as, for example, I have no idea what performance implications both have)
sysfs APIs typically have a one file per setting approach,
so for effects with speed and multiple-color settings you
would need a whole bunch of different files and then you
would either need to immediately apply every setting,
needing multiple writes to the hw for a single effect
update, or have some sort of "commit" sysfs attribute.
With ioctls you can simply provide all the settings
in one call, which is why I suggested using ioctls.
Regards,
Hans
^ permalink raw reply
* Re: Implement per-key keyboard backlight as auxdisplay?
From: Werner Sembach @ 2024-01-30 19:08 UTC (permalink / raw)
To: Hans de Goede, Pavel Machek
Cc: Jani Nikula, jikos, Jelle van der Waa, Miguel Ojeda, Lee Jones,
linux-kernel, dri-devel@lists.freedesktop.org, linux-input, ojeda,
linux-leds
In-Reply-To: <1bc6d6f0-a13d-4148-80cb-9c13dec7ed32@redhat.com>
Hi,
Am 30.01.24 um 19:35 schrieb Hans de Goede:
> Hi,
>
> On 1/30/24 19:09, Werner Sembach wrote:
>> Hi Hans,
>>
>> resend because Thunderbird htmlified the mail :/
> I use thunderbird too. If you right click on the server name
> and then go to "Settings" -> "Composition & Addressing"
> and then uncheck "Compose messages in HTML format"
> I think that should do the trick.
Can't set this globally or other people will complain that my replies delete
company logos in signatures xD. But usually the auto detection of Thunderbird works.
>
>> Am 30.01.24 um 18:10 schrieb Hans de Goede:
>>> Hi Werner,
>>>
>>> On 1/30/24 12:12, Werner Sembach wrote:
>>>> Hi Hans,
>>>>
>>>> Am 29.01.24 um 14:24 schrieb Hans de Goede:
>> <snip>
>>>> I think that are mostly external keyboards, so in theory a possible cut could also between built-in and external devices.
>>> IMHO it would be better to limit /dev/rgbledstring use to only
>>> cases where direct userspace control is not possible and thus
>>> have the cut be based on whether direct userspace control
>>> (e.g. /dev/hidraw access) is possible or not.
>> Ack
>>
>> <snip>
>>
>>>> So also no basic driver? Or still the concept from before with a basic 1 zone only driver via leds subsystem to have something working, but it is unregistered by userspace, if open rgb wants to take over for fine granular support?
>>> Ah good point, no I think that a basic driver just for kbd backlight
>>> brightness support which works with the standard desktop environment
>>> controls for this makes sense.
>>>
>>> Combined with some mechanism for e.g. openrgb to fully take over
>>> control as discussed. It is probably a good idea to file a separate
>>> issue with the openrgb project to discuss the takeover API.
>> I think the OpenRGB maintainers are pretty flexible at that point, after all it's similar to enable commands a lot of rgb devices need anyway. I would include it in a full api proposal.
> Ack.
>
>> On this note: Any particular reason you suggested an ioctl interface instead of a sysfs one? (Open question as, for example, I have no idea what performance implications both have)
> sysfs APIs typically have a one file per setting approach,
> so for effects with speed and multiple-color settings you
> would need a whole bunch of different files and then you
> would either need to immediately apply every setting,
> needing multiple writes to the hw for a single effect
> update, or have some sort of "commit" sysfs attribute.
>
> With ioctls you can simply provide all the settings
> in one call, which is why I suggested using ioctls.
Ack
If the static mode update is fast enough to have userspace controlled
animations, OpenRGB is calling that direct mode. Is it feasible to send 30 or
more ioctls per second for such an direct mode? Or should this spawn a special
purpose sysfs file that is kept open by userspace to continuously update the
keyboard?
>
> Regards,
>
> Hans
>
>
>
Regards,
Werner
^ permalink raw reply
* Re: Implement per-key keyboard backlight as auxdisplay?
From: Hans de Goede @ 2024-01-30 19:46 UTC (permalink / raw)
To: Werner Sembach, Pavel Machek
Cc: Jani Nikula, jikos, Jelle van der Waa, Miguel Ojeda, Lee Jones,
linux-kernel, dri-devel@lists.freedesktop.org, linux-input, ojeda,
linux-leds
In-Reply-To: <b70b2ea8-abfd-4d41-b336-3e34e5bdb8c6@tuxedocomputers.com>
Hi,
On 1/30/24 20:08, Werner Sembach wrote:
> Hi,
>
> Am 30.01.24 um 19:35 schrieb Hans de Goede:
>> Hi,
>>
>> On 1/30/24 19:09, Werner Sembach wrote:
>>> Hi Hans,
>>>
>>> resend because Thunderbird htmlified the mail :/
>> I use thunderbird too. If you right click on the server name
>> and then go to "Settings" -> "Composition & Addressing"
>> and then uncheck "Compose messages in HTML format"
>> I think that should do the trick.
> Can't set this globally or other people will complain that my replies delete company logos in signatures xD. But usually the auto detection of Thunderbird works.
>>
>>> Am 30.01.24 um 18:10 schrieb Hans de Goede:
>>>> Hi Werner,
>>>>
>>>> On 1/30/24 12:12, Werner Sembach wrote:
>>>>> Hi Hans,
>>>>>
>>>>> Am 29.01.24 um 14:24 schrieb Hans de Goede:
>>> <snip>
>>>>> I think that are mostly external keyboards, so in theory a possible cut could also between built-in and external devices.
>>>> IMHO it would be better to limit /dev/rgbledstring use to only
>>>> cases where direct userspace control is not possible and thus
>>>> have the cut be based on whether direct userspace control
>>>> (e.g. /dev/hidraw access) is possible or not.
>>> Ack
>>>
>>> <snip>
>>>
>>>>> So also no basic driver? Or still the concept from before with a basic 1 zone only driver via leds subsystem to have something working, but it is unregistered by userspace, if open rgb wants to take over for fine granular support?
>>>> Ah good point, no I think that a basic driver just for kbd backlight
>>>> brightness support which works with the standard desktop environment
>>>> controls for this makes sense.
>>>>
>>>> Combined with some mechanism for e.g. openrgb to fully take over
>>>> control as discussed. It is probably a good idea to file a separate
>>>> issue with the openrgb project to discuss the takeover API.
>>> I think the OpenRGB maintainers are pretty flexible at that point, after all it's similar to enable commands a lot of rgb devices need anyway. I would include it in a full api proposal.
>> Ack.
>>
>>> On this note: Any particular reason you suggested an ioctl interface instead of a sysfs one? (Open question as, for example, I have no idea what performance implications both have)
>> sysfs APIs typically have a one file per setting approach,
>> so for effects with speed and multiple-color settings you
>> would need a whole bunch of different files and then you
>> would either need to immediately apply every setting,
>> needing multiple writes to the hw for a single effect
>> update, or have some sort of "commit" sysfs attribute.
>>
>> With ioctls you can simply provide all the settings
>> in one call, which is why I suggested using ioctls.
>
> Ack
>
> If the static mode update is fast enough to have userspace controlled animations, OpenRGB is calling that direct mode. Is it feasible to send 30 or more ioctls per second for such an direct mode? Or should this spawn a special purpose sysfs file that is kept open by userspace to continuously update the keyboard?
ioctls are quite fast and another advantage of ioctls is
you open the /dev/rgbledstring# device only once and
then re-use the fd for as many ioctls as you want.
Regards,
Hans
^ permalink raw reply
* Re: [PATCH v1 2/7] HID: playstation: DS4: Don't fail on MAC address request
From: Roderick Colenbrander @ 2024-01-30 20:59 UTC (permalink / raw)
To: Max Staudt
Cc: Roderick Colenbrander, Jiri Kosina, Benjamin Tissoires,
linux-input, linux-kernel
In-Reply-To: <cb8290a0-1937-4937-ace5-e2a22a1f6e41@enpas.org>
On Sat, Jan 27, 2024 at 12:51 AM Max Staudt <max@enpas.org> wrote:
>
> On 1/25/24 09:39, Roderick Colenbrander wrote:
> > On Mon, Jan 15, 2024 at 6:58 AM Max Staudt <max@enpas.org> wrote:
> >>
> >> Some third-party controllers can't report their MAC address.
> >>
> > For what type of devices is this not working? This one example of this
> > request which is very foundational for a controller working on even
> > the game console. Are this perhaps USB-only devices? If the case maybe
> > some kind of error is only needed for USB connections.
>
> IIRC I've only seen this quirk with the oddball VID/PID 7545:0104 that my patch 7/7 adds. It is indeed a USB-only controller. I have not tried this device (or any, really) with an actual console, my focus is just on Linux. Admittedly, I'd also like to know what happens on a real PS4 :)
>
> My intention was to keep the error message and the solution as universal as possible, in case a controller comes along that has both this quirk *and* Bluetooth. It's a bit of an ugly workaround though - if you (or anyone else) have an idea for a nicer solution, I'd be really glad.
>
>
> Max
>
I remember on the console side that we support a number of controllers
including our official model and some licensed controllers. I recall
them taking some different codepaths and HID reports differently. It
has been a while, so I don't recall the details. If I remember it
could be that all of the licensed ones were USB-only (of course there
are some Bluetooth capable clones).
For this reason I think not all DS4-compatible devices have a MAC
address or handle this request. (We made some fixes in hid-playstation
relative to hid-sony, where hid-sony used another less known HID
report for the MAC address. Now we use the more commonly known one and
that helped other clone devices).
I'm not sure about the best way to handle this. I have kind of been
leaning towards doing a vid/pid like check for this case even though I
really hate it. It could be within dualshock4_get_mac_address as we do
some other special handling there too (although having the caller of
dualshock4_get_mac_address do it is an option too, but I think within
get_mac_address is slightly nicer for now).
Thanks,
Roderick
^ permalink raw reply
* Re: [PATCH v1 7/7] HID: playstation: DS4: Add VID/PID for SZ-MYPOWER controllers
From: Roderick Colenbrander @ 2024-01-30 21:07 UTC (permalink / raw)
To: Max Staudt
Cc: Roderick Colenbrander, Jiri Kosina, Benjamin Tissoires,
linux-input, linux-kernel
In-Reply-To: <e107b202-5843-41a7-b61e-68dd92128176@enpas.org>
On Sat, Jan 27, 2024 at 3:16 AM Max Staudt <max@enpas.org> wrote:
>
> On 1/25/24 10:03, Roderick Colenbrander wrote:
> > I'm not familiar with this device, but if it indeed works. Then I'm
> > okay with it.
>
> Thanks!
>
>
> I've just tried this patch on real hardware again, and there's a tradeoff here - it improves the situation for one 7545:0104 controller, and worsens it for another.
>
> Up to you, and if you don't want to think about it, then let's shelve this patch :)
>
>
>
> Details follow, if you're curious.
>
>
> I have two controllers with VID/PID 7545:0104, and they're both very quirky multi-emulation devices. One is shaped like a PS4 controller, the other like a hybrid between a PS4 and a Switch controller. Since these controllers exhibit all of the USB related quirks in this series, I've kept them as reproducers. Other controllers that passed through my hands only had a subset of the quirks.
>
> Up until now, both controllers worked with hid-sony as PS3 controllers. With this patch, the PS4 controller gains LED support and fine-grained control of the weak rumble motor. The "Switch (?) controller" on the other hand errors out, becomes 0079:181c, and loses the Home key and the accelerometer. This is a user facing change, and the question is how much we really care about these controllers.
>
>
>
> More details, if you're still reading:
>
>
> Both are "multi-purpose" controllers, appearing as PS4/PS3/Switch/other controllers in sequence. They advertise themselves as one USB device, and if there is no driver sending whatever init sequence they expect, they disconnect and try emulating a different controller.
>
> The PS4 controller has rumble and an RGB LED, and this patch series improves its functionality. It cannot emulate a Switch controller.
>
> The Switch (?) controller has no rumble and four multicolour player LEDs, but it adds Switch compatibility including accelerometer and gyro.
>
>
> For the PS4 mode, which is the first that they try, and which would unify most functions, they use 7545:0104 instead of cloning a DS4 VID/PID. So I took a guess and found that it works fine with hid-playstation if I add the VID/PID and the init quirks in patches 2/3/4. Well, to be precise, I've only made the DS4 shaped one work in PS4 mode, the Switch controller isn't happy and errors out, see below.
>
>
> On the PS4 controller, this makes the RGB LED work, rumble works, but the gyro and touchpad don't send HID updates. The touchpad can click though, so maybe the controller I have has a hardware defect.
>
> The Switch (?) controller is where things get weird. It disconnects, even though it is initialised by hid-playstation, and transitions into a generic controller with VID/PID 0079:181c. This mode is *not* on the list of emulations it usually tries. It's as if the "unfinished" PS4 initialisation transitions it into a hidden fifth emulation mode. In this mode, the home key does not send any HID event, and there are no accelerometer updates that hid-sony would receive in PS3 mode.
>
>
> So, with this patch, the PS4 controller works better on Linux, while the Switch controller works worse. Both were seen as PS3 controllers up until now. I see no way to discern them at driver probe time.
>
> Any preference on what to do...?
>
>
>
> Max
>
Hmpf, euhm euhm I'm not entirely sure what makes sense. From the
sounds of it are somewhat broken devices (buggy firmwares on them) or
perhaps one of your controllers (the one with not working touch) is
perhaps broken.
Some of the patches like handling report id 0x1 (minimal) won't hurt,
the LED fixes won't either. It makes it easier to add more devices. I
wonder if we have fully have enough data.
Need to think a bit...
Roderick
^ permalink raw reply
* Re: [PATCH v3] Input: bcm5974 - check endpoint type before starting traffic
From: Javier Carrasco @ 2024-01-30 22:11 UTC (permalink / raw)
To: Javier Carrasco, John Horan, Henrik Rydberg, Dmitry Torokhov
Cc: linux-input, linux-kernel, syzbot+348331f63b034f89b622
In-Reply-To: <20231007-topic-bcm5974_bulk-v3-1-d0f38b9d2935@gmail.com>
On 14.10.23 12:20, Javier Carrasco wrote:
> syzbot has found a type mismatch between a USB pipe and the transfer
> endpoint, which is triggered by the bcm5974 driver[1].
>
> This driver expects the device to provide input interrupt endpoints and
> if that is not the case, the driver registration should terminate.
>
> Repros are available to reproduce this issue with a certain setup for
> the dummy_hcd, leading to an interrupt/bulk mismatch which is caught in
> the USB core after calling usb_submit_urb() with the following message:
> "BOGUS urb xfer, pipe 1 != type 3"
>
> Some other device drivers (like the appletouch driver bcm5974 is mainly
> based on) provide some checking mechanism to make sure that an IN
> interrupt endpoint is available. In this particular case the endpoint
> addresses are provided by a config table, so the checking can be
> targeted to the provided endpoints.
>
> Add some basic checking to guarantee that the endpoints available match
> the expected type for both the trackpad and button endpoints.
>
> This issue was only found for the trackpad endpoint, but the checking
> has been added to the button endpoint as well for the same reasons.
>
> Given that there was never a check for the endpoint type, this bug has
> been there since the first implementation of the driver (f89bd95c5c94).
>
> [1] https://eur04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fsyzkaller.appspot.com%2Fbug%3Fextid%3D348331f63b034f89b622&data=05%7C01%7Cjavier.carrasco%40wolfvision.net%7C1880f48b0ac1493b40ff08dbcc9f2ea8%7Ce94ec9da9183471e83b351baa8eb804f%7C1%7C0%7C638328756279240780%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=IOsiHpWoIkog8HHkYIY8Ljh762bPZiqgm5xd5oAbK3s%3D&reserved=0
>
> Fixes: f89bd95c5c94 ("Input: bcm5974 - add driver for Macbook Air and Pro Penryn touchpads")
> Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>
> Reported-and-tested-by: syzbot+348331f63b034f89b622@syzkaller.appspotmail.com
> ---
> Changes in v3:
> - Use usb_check_int_endpoints() to validate the endpoints.
> - Link to v2: https://eur04.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Fr%2F20231007-topic-bcm5974_bulk-v2-1-021131c83efb%40gmail.com&data=05%7C01%7Cjavier.carrasco%40wolfvision.net%7C1880f48b0ac1493b40ff08dbcc9f2ea8%7Ce94ec9da9183471e83b351baa8eb804f%7C1%7C0%7C638328756279240780%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=vqLE9mUP0ehBIgtI%2F52ONRsF1wOrikc0VVLrp6MMjqQ%3D&reserved=0
>
> Changes in v2:
> - Keep error = -ENOMEM for the rest of the probe and return -ENODEV if
> the endpoint check fails.
> - Check function returns now bool and was renamed (_is_ for
> bool-returning functions).
> - Link to v1: https://eur04.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Fr%2F20231007-topic-bcm5974_bulk-v1-1-355be9f8ad80%40gmail.com&data=05%7C01%7Cjavier.carrasco%40wolfvision.net%7C1880f48b0ac1493b40ff08dbcc9f2ea8%7Ce94ec9da9183471e83b351baa8eb804f%7C1%7C0%7C638328756279240780%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=Qf6kg4M2AvwSEpkwClGPpVdo1PO96WfUfTsiy6z28UI%3D&reserved=0
> ---
> drivers/input/mouse/bcm5974.c | 20 ++++++++++++++++++++
> 1 file changed, 20 insertions(+)
>
> diff --git a/drivers/input/mouse/bcm5974.c b/drivers/input/mouse/bcm5974.c
> index ca150618d32f..953992b458e9 100644
> --- a/drivers/input/mouse/bcm5974.c
> +++ b/drivers/input/mouse/bcm5974.c
> @@ -19,6 +19,7 @@
> * Copyright (C) 2006 Nicolas Boichat (nicolas@boichat.ch)
> */
>
> +#include "linux/usb.h"
> #include <linux/kernel.h>
> #include <linux/errno.h>
> #include <linux/slab.h>
> @@ -193,6 +194,8 @@ enum tp_type {
>
> /* list of device capability bits */
> #define HAS_INTEGRATED_BUTTON 1
> +/* maximum number of supported endpoints (currently trackpad and button) */
> +#define MAX_ENDPOINTS 2
>
> /* trackpad finger data block size */
> #define FSIZE_TYPE1 (14 * sizeof(__le16))
> @@ -891,6 +894,18 @@ static int bcm5974_resume(struct usb_interface *iface)
> return error;
> }
>
> +static bool bcm5974_check_endpoints(struct usb_interface *iface,
> + const struct bcm5974_config *cfg)
> +{
> + u8 ep_addr[MAX_ENDPOINTS + 1] = {0};
> +
> + ep_addr[0] = cfg->tp_ep;
> + if (cfg->tp_type == TYPE1)
> + ep_addr[1] = cfg->bt_ep;
> +
> + return usb_check_int_endpoints(iface, ep_addr);
> +}
> +
> static int bcm5974_probe(struct usb_interface *iface,
> const struct usb_device_id *id)
> {
> @@ -903,6 +918,11 @@ static int bcm5974_probe(struct usb_interface *iface,
> /* find the product index */
> cfg = bcm5974_get_config(udev);
>
> + if (!bcm5974_check_endpoints(iface, cfg)) {
> + dev_err(&iface->dev, "Unexpected non-int endpoint\n");
> + return -ENODEV;
> + }
> +
> /* allocate memory for our device state and initialize it */
> dev = kzalloc(sizeof(struct bcm5974), GFP_KERNEL);
> input_dev = input_allocate_device();
>
> ---
> base-commit: 401644852d0b2a278811de38081be23f74b5bb04
> change-id: 20231007-topic-bcm5974_bulk-c66b743ba7ba
>
> Best regards,
Gentle reminder: this bug keeps on being found by syzbot and it was
included in the last monthly input report (Jan 2024):
https://lore.kernel.org/all/0000000000001df937060f20c585@google.com/T/
Best regards,
Javier Carrasco
^ permalink raw reply
* Re: [PATCH v6 2/3] dt-bindings: input: Add TouchNetix axiom touchscreen
From: Rob Herring @ 2024-01-30 22:28 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Kamel Bouhara, Dmitry Torokhov, Krzysztof Kozlowski, Conor Dooley,
Henrik Rydberg, linux-input, linux-kernel, devicetree,
Marco Felsch, Jeff LaBundy, catalin.popescu, mark.satterthwaite,
Thomas Petazzoni, Gregory Clement, bsp-development.geo
In-Reply-To: <2c8961ff-9fcc-402c-b048-744ae9473164@linaro.org>
On Fri, Jan 26, 2024 at 12:46:16PM +0100, Krzysztof Kozlowski wrote:
> On 25/01/2024 17:58, Kamel Bouhara wrote:
> > + reset-gpios:
> > + maxItems: 1
> > +
> > + vdda-supply:
> > + description: Analog power supply regulator on VDDA pin
> > +
> > + vddi-supply:
> > + description: I/O power supply regulator on VDDI pin
> > +
> > + startup-time-ms:
> > + description: delay after power supply regulator is applied in ms
>
> That's a regulator property - ramp up time.
I'm sure there's an existing property name that could be used.
But why is it needed? Is it variable per board with the same device? If
not, it should be implied by the compatible.
Rob
^ permalink raw reply
* Re: [PATCH] Input: xpad - add Lenovo Legion Go controllers
From: Dmitry Torokhov @ 2024-01-30 22:32 UTC (permalink / raw)
To: Brenton Simpson
Cc: Vicki Pfau, Hans de Goede, Cameron Gutman, Erica Taylor,
Ismael Ferreras Morezuelas, Jonathan Frederick, Matthias Benkmann,
Matthias Berndt, nate, Sam Lantinga, linux-input, linux-kernel,
trivial
In-Reply-To: <CAAL3-=-RRWyCbq_B=Lh7tnG2i3MOLL+2bqPOUS54oTC4+vVk_g@mail.gmail.com>
Hi Brenton,
On Fri, Jan 19, 2024 at 12:22:45AM -0800, Brenton Simpson wrote:
> Thanks Vicki. I didn't realize they were meant to be sorted.
Please try avoid top posting.
>
> Would it be appropriate to add comments explaining the sorting? The
> second stanza, in particular, is sorted by the IDs rather than
> lexicographically. If someone sorted it naively, they'd end up with a
> bigger diff than expected.
Yes, we usually sort by VID/PID rather than the name of the device.
>
> It looks like a few others have escaped sorting; for instance,
> "Microsoft X-Box One Elite 2 pad" appears in the wrong place.
Yep, sometimes we mess up.
>
> If Dmitry wants to land this and then follow on with a sort + comment
> commit (or do that first and then rebase this on top), that would be
> great. I can take a stab too if that's helpful.
I applied the patch (moving the entires to the right place). If someone
would send a patch fixing the Elite 2 entry and noting the sorting rules
I'd be happy to apply it.
Thanks.
--
Dmitry
^ permalink raw reply
* [PATCH] Input: xpad - sort xpad_device by vendor and product ID
From: Brenton Simpson @ 2024-01-30 23:19 UTC (permalink / raw)
To: Dmitry Torokhov, Hans de Goede
Cc: Cameron Gutman, Erica Taylor, Ismael Ferreras Morezuelas,
Jonathan Frederick, Matthias Benkmann, Matthias Berndt, nate,
Sam Lantinga, Vicki Pfau, linux-input, linux-kernel, trivial,
Brenton Simpson
In-Reply-To: <Zbl49VAMZx2qrz-p@google.com>
Signed-off-by: Brenton Simpson <appsforartists@google.com>
---
drivers/input/joystick/xpad.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/drivers/input/joystick/xpad.c b/drivers/input/joystick/xpad.c
index 7c4b2a5cc1b5..c67f3d65192d 100644
--- a/drivers/input/joystick/xpad.c
+++ b/drivers/input/joystick/xpad.c
@@ -127,6 +127,7 @@ static const struct xpad_device {
u8 mapping;
u8 xtype;
} xpad_device[] = {
+ /* Please keep this list sorted by vendor and product ID. */
{ 0x0079, 0x18d4, "GPD Win 2 X-Box Controller", 0, XTYPE_XBOX360 },
{ 0x03eb, 0xff01, "Wooting One (Legacy)", 0, XTYPE_XBOX360 },
{ 0x03eb, 0xff02, "Wooting Two (Legacy)", 0, XTYPE_XBOX360 },
@@ -147,9 +148,9 @@ static const struct xpad_device {
{ 0x045e, 0x02d1, "Microsoft X-Box One pad", 0, XTYPE_XBOXONE },
{ 0x045e, 0x02dd, "Microsoft X-Box One pad (Firmware 2015)", 0, XTYPE_XBOXONE },
{ 0x045e, 0x02e3, "Microsoft X-Box One Elite pad", MAP_PADDLES, XTYPE_XBOXONE },
- { 0x045e, 0x0b00, "Microsoft X-Box One Elite 2 pad", MAP_PADDLES, XTYPE_XBOXONE },
{ 0x045e, 0x02ea, "Microsoft X-Box One S pad", 0, XTYPE_XBOXONE },
{ 0x045e, 0x0719, "Xbox 360 Wireless Receiver", MAP_DPAD_TO_BUTTONS, XTYPE_XBOX360W },
+ { 0x045e, 0x0b00, "Microsoft X-Box One Elite 2 pad", MAP_PADDLES, XTYPE_XBOXONE },
{ 0x045e, 0x0b0a, "Microsoft X-Box Adaptive Controller", MAP_PROFILE_BUTTON, XTYPE_XBOXONE },
{ 0x045e, 0x0b12, "Microsoft Xbox Series S|X Controller", MAP_SELECT_BUTTON, XTYPE_XBOXONE },
{ 0x046d, 0xc21d, "Logitech Gamepad F310", 0, XTYPE_XBOX360 },
@@ -335,7 +336,6 @@ static const struct xpad_device {
{ 0x20d6, 0x2001, "BDA Xbox Series X Wired Controller", 0, XTYPE_XBOXONE },
{ 0x20d6, 0x2009, "PowerA Enhanced Wired Controller for Xbox Series X|S", 0, XTYPE_XBOXONE },
{ 0x20d6, 0x281f, "PowerA Wired Controller For Xbox 360", 0, XTYPE_XBOX360 },
- { 0x2e24, 0x0652, "Hyperkin Duke X-Box One pad", 0, XTYPE_XBOXONE },
{ 0x24c6, 0x5000, "Razer Atrox Arcade Stick", MAP_TRIGGERS_TO_BUTTONS, XTYPE_XBOX360 },
{ 0x24c6, 0x5300, "PowerA MINI PROEX Controller", 0, XTYPE_XBOX360 },
{ 0x24c6, 0x5303, "Xbox Airflo wired controller", 0, XTYPE_XBOX360 },
@@ -350,9 +350,9 @@ static const struct xpad_device {
{ 0x24c6, 0x5502, "Hori Fighting Stick VX Alt", MAP_TRIGGERS_TO_BUTTONS, XTYPE_XBOX360 },
{ 0x24c6, 0x5503, "Hori Fighting Edge", MAP_TRIGGERS_TO_BUTTONS, XTYPE_XBOX360 },
{ 0x24c6, 0x5506, "Hori SOULCALIBUR V Stick", 0, XTYPE_XBOX360 },
- { 0x24c6, 0x5510, "Hori Fighting Commander ONE (Xbox 360/PC Mode)", MAP_TRIGGERS_TO_BUTTONS, XTYPE_XBOX360 },
{ 0x24c6, 0x550d, "Hori GEM Xbox controller", 0, XTYPE_XBOX360 },
{ 0x24c6, 0x550e, "Hori Real Arcade Pro V Kai 360", MAP_TRIGGERS_TO_BUTTONS, XTYPE_XBOX360 },
+ { 0x24c6, 0x5510, "Hori Fighting Commander ONE (Xbox 360/PC Mode)", MAP_TRIGGERS_TO_BUTTONS, XTYPE_XBOX360 },
{ 0x24c6, 0x551a, "PowerA FUSION Pro Controller", 0, XTYPE_XBOXONE },
{ 0x24c6, 0x561a, "PowerA FUSION Controller", 0, XTYPE_XBOXONE },
{ 0x24c6, 0x5b00, "ThrustMaster Ferrari 458 Racing Wheel", 0, XTYPE_XBOX360 },
@@ -363,6 +363,7 @@ static const struct xpad_device {
{ 0x2563, 0x058d, "OneXPlayer Gamepad", 0, XTYPE_XBOX360 },
{ 0x2dc8, 0x2000, "8BitDo Pro 2 Wired Controller fox Xbox", 0, XTYPE_XBOXONE },
{ 0x2dc8, 0x3106, "8BitDo Pro 2 Wired Controller", 0, XTYPE_XBOX360 },
+ { 0x2e24, 0x0652, "Hyperkin Duke X-Box One pad", 0, XTYPE_XBOXONE },
{ 0x31e3, 0x1100, "Wooting One", 0, XTYPE_XBOX360 },
{ 0x31e3, 0x1200, "Wooting Two", 0, XTYPE_XBOX360 },
{ 0x31e3, 0x1210, "Wooting Lekker", 0, XTYPE_XBOX360 },
@@ -460,6 +461,9 @@ static const signed short xpad_btn_paddles[] = {
{ XPAD_XBOXONE_VENDOR_PROTOCOL((vend), 208) }
static const struct usb_device_id xpad_table[] = {
+ /* Please keep this list sorted by vendor ID. Because the lines use different
+ * macros, you may need to sort it by hand.
+ */
{ USB_INTERFACE_INFO('X', 'B', 0) }, /* Xbox USB-IF not-approved class */
XPAD_XBOX360_VENDOR(0x0079), /* GPD Win 2 controller */
XPAD_XBOX360_VENDOR(0x03eb), /* Wooting Keyboards (Legacy) */
--
2.43.0.429.g432eaa2c6b-goog
^ permalink raw reply related
* Re: [PATCH v15 0/4] Input: add initial support for Goodix Berlin touchscreen IC
From: Dmitry Torokhov @ 2024-01-30 23:22 UTC (permalink / raw)
To: Neil Armstrong
Cc: linux-input, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Bastien Nocera, Hans de Goede, Henrik Rydberg, Jeff LaBundy,
linux-arm-msm, devicetree, linux-kernel, Rob Herring
In-Reply-To: <20240129-topic-goodix-berlin-upstream-initial-v15-0-6f7d096c0a0a@linaro.org>
On Mon, Jan 29, 2024 at 10:16:34AM +0100, Neil Armstrong wrote:
> These touchscreen ICs support SPI, I2C and I3C interface, up to
> 10 finger touch, stylus and gestures events.
>
> This initial driver is derived from the Goodix goodix_ts_berlin
> available at [1] and [2] and only supports the GT9916 IC
> present on the Qualcomm SM8550/SM8650 MTP & QRD touch panel.
>
> The current implementation only supports BerlinD, aka GT9916.
>
> Support for advanced features like:
> - Firmware & config update
> - Stylus events
> - Gestures events
> - Previous revisions support (BerlinA or BerlinB)
> is not included in current version.
>
> The current support will work with currently flashed firmware
> and config, and bail out if firmware or config aren't flashed yet.
>
> [1] https://github.com/goodix/goodix_ts_berlin
> [2] https://git.codelinaro.org/clo/la/platform/vendor/opensource/touch-drivers
>
> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
Applied the lot, thank you.
--
Dmitry
^ permalink raw reply
* Re: [PATCH v6 3/3] Input: Add TouchNetix axiom i2c touchscreen driver
From: Dmitry Torokhov @ 2024-01-30 23:24 UTC (permalink / raw)
To: POPESCU Catalin
Cc: Kamel Bouhara, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Henrik Rydberg, linux-input@vger.kernel.org,
linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
Marco Felsch, Jeff LaBundy, mark.satterthwaite@touchnetix.com,
Thomas Petazzoni, Gregory Clement, GEO-CHHER-bsp-development
In-Reply-To: <b466d75c-944f-4c45-80f3-993b1fe40d7a@leica-geosystems.com>
On Tue, Jan 30, 2024 at 05:32:54PM +0000, POPESCU Catalin wrote:
> On 25.01.24 17:58, Kamel Bouhara wrote:
> > + u16 offset = (usage_id * AXIOM_U31_BYTES_PER_USAGE);
> > + u8 id = rx_data[offset + 0];
> > + u8 start_page = rx_data[offset + 1];
> > + u8 num_pages = rx_data[offset + 2];
> > + u32 max_offset = ((rx_data[offset + 3] & AXIOM_PAGE_OFFSET_MASK) + 1) * 2;
> > +
> > + if (!num_pages)
> > + usage_table[id].is_report = true;
> please add an else statement to set is_report to false.
Better written as:
usage_table[id].is_report = num_pages == 0;
or
usage_table[id].is_report = !num_pages;
Thanks.
--
Dmitry
^ permalink raw reply
* Suspected bug in hid-microsoft.c
From: Taco Jerkface @ 2024-01-31 6:26 UTC (permalink / raw)
To: Benjamin Tissoires, linux-input, jikos
In-Reply-To: <CAEQPD4T+C_RaP_z96XRXj1teGiDMZu1MsPn8hAQ5FhSoaajZaA@mail.gmail.com>
Resending without HTML:
Hi,
I hope this is the correct contact for this report, I found you as the
maintainer in the hid-microsoft.c.
I believe there is a bug in the microsoft bluetooth driver for the
Xbox Elite Series 2 controller. I have been experiencing issues with
it that I initially thought were SDL related. However the SDL team
seems to think this is driver related. My SDL bug report information
is here:
https://github.com/libsdl-org/SDL/issues/8907
Basically, SDL reads the controller correctly when connected by USB,
and if I run "controllermap" with root permission, but with user
permissions it misreads the number of buttons as 122, the first paddle
button on the back seems to act like the "screenshot" button from the
1914 controller, and the other paddle buttons are not read. All
buttons read fine with evites, but the paddle buttons "KEY_UNKNOWN"
type 1 (EV_KEY), code 240 (KEY_UNKNOWN), value 0
Please let me know if there is a better contact for this, or if there
is anything I can do to help identify the problem.
Cheers
^ permalink raw reply
* Re: [PATCH v6 2/3] dt-bindings: input: Add TouchNetix axiom touchscreen
From: Krzysztof Kozlowski @ 2024-01-31 7:28 UTC (permalink / raw)
To: Rob Herring
Cc: Kamel Bouhara, Dmitry Torokhov, Krzysztof Kozlowski, Conor Dooley,
Henrik Rydberg, linux-input, linux-kernel, devicetree,
Marco Felsch, Jeff LaBundy, catalin.popescu, mark.satterthwaite,
Thomas Petazzoni, Gregory Clement, bsp-development.geo
In-Reply-To: <20240130222844.GA2527859-robh@kernel.org>
On 30/01/2024 23:28, Rob Herring wrote:
> On Fri, Jan 26, 2024 at 12:46:16PM +0100, Krzysztof Kozlowski wrote:
>> On 25/01/2024 17:58, Kamel Bouhara wrote:
>>> + reset-gpios:
>>> + maxItems: 1
>>> +
>>> + vdda-supply:
>>> + description: Analog power supply regulator on VDDA pin
>>> +
>>> + vddi-supply:
>>> + description: I/O power supply regulator on VDDI pin
>>> +
>>> + startup-time-ms:
>>> + description: delay after power supply regulator is applied in ms
>>
>> That's a regulator property - ramp up time.
>
> I'm sure there's an existing property name that could be used.
>
> But why is it needed? Is it variable per board with the same device? If
> not, it should be implied by the compatible.
I meant, that regulators have such property. Unless this is coming from
the device needs, not from the regulator?
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH v6 0/8] Convert DA906{1,2} bindings to json-schema
From: Lee Jones @ 2024-01-31 8:49 UTC (permalink / raw)
To: Biju Das
Cc: Wim Van Sebroeck, Guenter Roeck, Dmitry Torokhov, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Support Opensource,
Rafael J. Wysocki, Daniel Lezcano, Zhang Rui, Lukasz Luba,
Steve Twiss, linux-input@vger.kernel.org,
devicetree@vger.kernel.org, linux-pm@vger.kernel.org,
linux-watchdog@vger.kernel.org, Geert Uytterhoeven,
Prabhakar Mahadev Lad, biju.das.au,
linux-renesas-soc@vger.kernel.org
In-Reply-To: <TYCPR01MB1126921A54B9260CC33505FCA867E2@TYCPR01MB11269.jpnprd01.prod.outlook.com>
On Mon, 29 Jan 2024, Biju Das wrote:
> Hi Lee Jones,
>
> > -----Original Message-----
> > From: Lee Jones <lee@kernel.org>
> > Sent: Thursday, December 21, 2023 2:33 PM
> > Subject: Re: [PATCH v6 0/8] Convert DA906{1,2} bindings to json-schema
> >
> > On Thu, 21 Dec 2023, Lee Jones wrote:
> >
> > > On Thu, 14 Dec 2023 08:09:03 +0000, Biju Das wrote:
> > > > Convert the below bindings to json-schema
> > > > 1) DA906{1,2} mfd bindings
> > > > 2) DA906{1,2,3} onkey bindings
> > > > 3) DA906{1,2,3} thermal bindings
> > > >
> > > > Also add fallback for DA9061 watchdog device and document
> > > > DA9063 watchdog device.
> > > >
> > > > [...]
> > >
> > > Applied, thanks!
> > >
> > > [1/8] dt-bindings: mfd: da9062: Update watchdog description
> > > commit: 9e7b13b805bcbe5335c2936d4c7ea0323ac69a81
> > > [2/8] dt-bindings: watchdog: dlg,da9062-watchdog: Add fallback for
> > DA9061 watchdog
> > > commit: 28d34db7772f18490b52328f04a3bf69ed5390d2
> > > [3/8] dt-bindings: watchdog: dlg,da9062-watchdog: Document DA9063
> > watchdog
> > > commit: d2a7dbb808870c17cffa2749ea877f61f298d098
> > > [4/8] dt-bindings: mfd: dlg,da9063: Update watchdog child node
> > > commit: d4018547a15a94c7e865b2cef82bff1cd43a32b3
> > > [5/8] dt-bindings: input: Convert da906{1,2,3} onkey to json-schema
> > > commit: db459d3da7bb9c37cb86897c7b321a49f8e40968
> > > [6/8] dt-bindings: thermal: Convert da906{1,2} thermal to json-schema
> > > commit: 998f499c843e360bcd9ee1fe9addc3b5d32f1234
> > > [7/8] dt-bindings: mfd: dlg,da9063: Sort child devices
> > > commit: 2bbf9d2a8e3bc933703dfda87cac953bed458496
> > > [8/8] dt-bindings: mfd: dlg,da9063: Convert da9062 to json-schema
> > > commit: 522225161830f6a93f2aaabda99226c1ffddc8c4
> >
> > Submitted for testing. Pull-request to follow.
>
> The commit dc805ea058c0e ("MAINTAINERS: rectify entry for DIALOG SEMICONDUCTOR DRIVERS")
> in mainline will give a conflict for patch#1.
>
> Patch#2 and patch#3 are already in mainline.
>
>
> Please let me know if you want me to rebase and resend the patch series
That would be helpful, thanks.
Please ensure all of the patches have my:
Acked-by: Lee Jones <lee@kernel.org>
... applied, then I'll know to just apply them again.
--
Lee Jones [李琼斯]
^ permalink raw reply
* Re: [PATCH v6 2/3] dt-bindings: input: Add TouchNetix axiom touchscreen
From: Kamel Bouhara @ 2024-01-31 9:06 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Rob Herring, Dmitry Torokhov, Krzysztof Kozlowski, Conor Dooley,
Henrik Rydberg, linux-input, linux-kernel, devicetree,
Marco Felsch, Jeff LaBundy, catalin.popescu, mark.satterthwaite,
Thomas Petazzoni, Gregory Clement, bsp-development.geo
In-Reply-To: <f07d7557-c190-4154-8fc7-6a3850b1d162@linaro.org>
Hello,
On Wed, Jan 31, 2024 at 08:28:43AM +0100, Krzysztof Kozlowski wrote:
> On 30/01/2024 23:28, Rob Herring wrote:
> > On Fri, Jan 26, 2024 at 12:46:16PM +0100, Krzysztof Kozlowski wrote:
> >> On 25/01/2024 17:58, Kamel Bouhara wrote:
> >>> + reset-gpios:
> >>> + maxItems: 1
> >>> +
> >>> + vdda-supply:
> >>> + description: Analog power supply regulator on VDDA pin
> >>> +
> >>> + vddi-supply:
> >>> + description: I/O power supply regulator on VDDI pin
> >>> +
> >>> + startup-time-ms:
> >>> + description: delay after power supply regulator is applied in ms
> >>
> >> That's a regulator property - ramp up time.
> >
> > I'm sure there's an existing property name that could be used.
> >
> > But why is it needed? Is it variable per board with the same device? If
> > not, it should be implied by the compatible.
>
> I meant, that regulators have such property. Unless this is coming from
> the device needs, not from the regulator?
>
After looking again into the device's datasheet [1], the delay (startup
time) is not really optional and it shouldn't be set through devicetree.
IIUC, it have to be set unconditionally after a device reset or
a vdda assertion.
[1]: https://www.touchnetix.com/media/dgnjohor/tnxd00394-a3-axiom_ax54a_2d_touch_controller_datasheet.pdf
--
Kamel Bouhara, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com
^ permalink raw reply
* Re: [PATCH bpf-next v4 3/3] bpf: treewide: Annotate BPF kfuncs in BTF
From: Benjamin Tissoires @ 2024-01-31 9:24 UTC (permalink / raw)
To: Daniel Xu
Cc: mhiramat, daniel, edumazet, fw, hannes, tytso, ast, eddyz87, kuba,
tj, steffen.klassert, yonghong.song, hawk, rostedt,
john.fastabend, pablo, pabeni, jikos, davem, alexandre.torgue,
Herbert Xu, song, dsahern, mcoquelin.stm32, corbet, lizefan.x,
andrii, martin.lau, benjamin.tissoires, ebiggers, kadlec, shuah,
alexei.starovoitov, olsajiri, quentin, alan.maguire, memxor,
kpsingh, sdf, haoluo, jolsa, mathieu.desnoyers, mykolal, bpf,
linux-doc, linux-kernel, linux-input, fsverity, cgroups,
linux-trace-kernel, netdev, netfilter-devel, coreteam,
linux-kselftest, linux-stm32, linux-arm-kernel
In-Reply-To: <e55150ceecbf0a5d961e608941165c0bee7bc943.1706491398.git.dxu@dxuuu.xyz>
On Jan 28 2024, Daniel Xu wrote:
> This commit marks kfuncs as such inside the .BTF_ids section. The upshot
> of these annotations is that we'll be able to automatically generate
> kfunc prototypes for downstream users. The process is as follows:
>
> 1. In source, use BTF_KFUNCS_START/END macro pair to mark kfuncs
> 2. During build, pahole injects into BTF a "bpf_kfunc" BTF_DECL_TAG for
> each function inside BTF_KFUNCS sets
> 3. At runtime, vmlinux or module BTF is made available in sysfs
> 4. At runtime, bpftool (or similar) can look at provided BTF and
> generate appropriate prototypes for functions with "bpf_kfunc" tag
>
> To ensure future kfunc are similarly tagged, we now also return error
> inside kfunc registration for untagged kfuncs. For vmlinux kfuncs,
> we also WARN(), as initcall machinery does not handle errors.
>
> Signed-off-by: Daniel Xu <dxu@dxuuu.xyz>
> ---
> Documentation/bpf/kfuncs.rst | 8 ++++----
> drivers/hid/bpf/hid_bpf_dispatch.c | 8 ++++----
For the HID changes (they shouldn't conflict with our current branch):
Acked-by: Benjamin Tissoires <bentiss@kernel.org>
Cheers,
Benjamin
> fs/verity/measure.c | 4 ++--
> kernel/bpf/btf.c | 8 ++++++++
> kernel/bpf/cpumask.c | 4 ++--
> kernel/bpf/helpers.c | 8 ++++----
> kernel/bpf/map_iter.c | 4 ++--
> kernel/cgroup/rstat.c | 4 ++--
> kernel/trace/bpf_trace.c | 8 ++++----
> net/bpf/test_run.c | 8 ++++----
> net/core/filter.c | 20 +++++++++----------
> net/core/xdp.c | 4 ++--
> net/ipv4/bpf_tcp_ca.c | 4 ++--
> net/ipv4/fou_bpf.c | 4 ++--
> net/ipv4/tcp_bbr.c | 4 ++--
> net/ipv4/tcp_cubic.c | 4 ++--
> net/ipv4/tcp_dctcp.c | 4 ++--
> net/netfilter/nf_conntrack_bpf.c | 4 ++--
> net/netfilter/nf_nat_bpf.c | 4 ++--
> net/xfrm/xfrm_interface_bpf.c | 4 ++--
> net/xfrm/xfrm_state_bpf.c | 4 ++--
> .../selftests/bpf/bpf_testmod/bpf_testmod.c | 8 ++++----
> 22 files changed, 70 insertions(+), 62 deletions(-)
>
> diff --git a/Documentation/bpf/kfuncs.rst b/Documentation/bpf/kfuncs.rst
> index 7985c6615f3c..a8f5782bd833 100644
> --- a/Documentation/bpf/kfuncs.rst
> +++ b/Documentation/bpf/kfuncs.rst
> @@ -177,10 +177,10 @@ In addition to kfuncs' arguments, verifier may need more information about the
> type of kfunc(s) being registered with the BPF subsystem. To do so, we define
> flags on a set of kfuncs as follows::
>
> - BTF_SET8_START(bpf_task_set)
> + BTF_KFUNCS_START(bpf_task_set)
> BTF_ID_FLAGS(func, bpf_get_task_pid, KF_ACQUIRE | KF_RET_NULL)
> BTF_ID_FLAGS(func, bpf_put_pid, KF_RELEASE)
> - BTF_SET8_END(bpf_task_set)
> + BTF_KFUNCS_END(bpf_task_set)
>
> This set encodes the BTF ID of each kfunc listed above, and encodes the flags
> along with it. Ofcourse, it is also allowed to specify no flags.
> @@ -347,10 +347,10 @@ Once the kfunc is prepared for use, the final step to making it visible is
> registering it with the BPF subsystem. Registration is done per BPF program
> type. An example is shown below::
>
> - BTF_SET8_START(bpf_task_set)
> + BTF_KFUNCS_START(bpf_task_set)
> BTF_ID_FLAGS(func, bpf_get_task_pid, KF_ACQUIRE | KF_RET_NULL)
> BTF_ID_FLAGS(func, bpf_put_pid, KF_RELEASE)
> - BTF_SET8_END(bpf_task_set)
> + BTF_KFUNCS_END(bpf_task_set)
>
> static const struct btf_kfunc_id_set bpf_task_kfunc_set = {
> .owner = THIS_MODULE,
> diff --git a/drivers/hid/bpf/hid_bpf_dispatch.c b/drivers/hid/bpf/hid_bpf_dispatch.c
> index d9ef45fcaeab..02c441aaa217 100644
> --- a/drivers/hid/bpf/hid_bpf_dispatch.c
> +++ b/drivers/hid/bpf/hid_bpf_dispatch.c
> @@ -172,9 +172,9 @@ hid_bpf_get_data(struct hid_bpf_ctx *ctx, unsigned int offset, const size_t rdwr
> * The following set contains all functions we agree BPF programs
> * can use.
> */
> -BTF_SET8_START(hid_bpf_kfunc_ids)
> +BTF_KFUNCS_START(hid_bpf_kfunc_ids)
> BTF_ID_FLAGS(func, hid_bpf_get_data, KF_RET_NULL)
> -BTF_SET8_END(hid_bpf_kfunc_ids)
> +BTF_KFUNCS_END(hid_bpf_kfunc_ids)
>
> static const struct btf_kfunc_id_set hid_bpf_kfunc_set = {
> .owner = THIS_MODULE,
> @@ -440,12 +440,12 @@ static const struct btf_kfunc_id_set hid_bpf_fmodret_set = {
> };
>
> /* for syscall HID-BPF */
> -BTF_SET8_START(hid_bpf_syscall_kfunc_ids)
> +BTF_KFUNCS_START(hid_bpf_syscall_kfunc_ids)
> BTF_ID_FLAGS(func, hid_bpf_attach_prog)
> BTF_ID_FLAGS(func, hid_bpf_allocate_context, KF_ACQUIRE | KF_RET_NULL)
> BTF_ID_FLAGS(func, hid_bpf_release_context, KF_RELEASE)
> BTF_ID_FLAGS(func, hid_bpf_hw_request)
> -BTF_SET8_END(hid_bpf_syscall_kfunc_ids)
> +BTF_KFUNCS_END(hid_bpf_syscall_kfunc_ids)
>
> static const struct btf_kfunc_id_set hid_bpf_syscall_kfunc_set = {
> .owner = THIS_MODULE,
> diff --git a/fs/verity/measure.c b/fs/verity/measure.c
> index bf7a5f4cccaf..3969d54158d1 100644
> --- a/fs/verity/measure.c
> +++ b/fs/verity/measure.c
> @@ -159,9 +159,9 @@ __bpf_kfunc int bpf_get_fsverity_digest(struct file *file, struct bpf_dynptr_ker
>
> __bpf_kfunc_end_defs();
>
> -BTF_SET8_START(fsverity_set_ids)
> +BTF_KFUNCS_START(fsverity_set_ids)
> BTF_ID_FLAGS(func, bpf_get_fsverity_digest, KF_TRUSTED_ARGS)
> -BTF_SET8_END(fsverity_set_ids)
> +BTF_KFUNCS_END(fsverity_set_ids)
>
> static int bpf_get_fsverity_digest_filter(const struct bpf_prog *prog, u32 kfunc_id)
> {
> diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
> index edef96ceffa3..bc446f37530c 100644
> --- a/kernel/bpf/btf.c
> +++ b/kernel/bpf/btf.c
> @@ -8041,6 +8041,14 @@ int register_btf_kfunc_id_set(enum bpf_prog_type prog_type,
> {
> enum btf_kfunc_hook hook;
>
> + /* All kfuncs need to be tagged as such in BTF.
> + * WARN() for initcall registrations that do not check errors.
> + */
> + if (!(kset->set->flags & BTF_SET8_KFUNCS)) {
> + WARN_ON(!kset->owner);
> + return -EINVAL;
> + }
> +
> hook = bpf_prog_type_to_kfunc_hook(prog_type);
> return __register_btf_kfunc_id_set(hook, kset);
> }
> diff --git a/kernel/bpf/cpumask.c b/kernel/bpf/cpumask.c
> index 2e73533a3811..dad0fb1c8e87 100644
> --- a/kernel/bpf/cpumask.c
> +++ b/kernel/bpf/cpumask.c
> @@ -424,7 +424,7 @@ __bpf_kfunc u32 bpf_cpumask_weight(const struct cpumask *cpumask)
>
> __bpf_kfunc_end_defs();
>
> -BTF_SET8_START(cpumask_kfunc_btf_ids)
> +BTF_KFUNCS_START(cpumask_kfunc_btf_ids)
> BTF_ID_FLAGS(func, bpf_cpumask_create, KF_ACQUIRE | KF_RET_NULL)
> BTF_ID_FLAGS(func, bpf_cpumask_release, KF_RELEASE)
> BTF_ID_FLAGS(func, bpf_cpumask_acquire, KF_ACQUIRE | KF_TRUSTED_ARGS)
> @@ -450,7 +450,7 @@ BTF_ID_FLAGS(func, bpf_cpumask_copy, KF_RCU)
> BTF_ID_FLAGS(func, bpf_cpumask_any_distribute, KF_RCU)
> BTF_ID_FLAGS(func, bpf_cpumask_any_and_distribute, KF_RCU)
> BTF_ID_FLAGS(func, bpf_cpumask_weight, KF_RCU)
> -BTF_SET8_END(cpumask_kfunc_btf_ids)
> +BTF_KFUNCS_END(cpumask_kfunc_btf_ids)
>
> static const struct btf_kfunc_id_set cpumask_kfunc_set = {
> .owner = THIS_MODULE,
> diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
> index bcb951a2ecf4..4db1c658254c 100644
> --- a/kernel/bpf/helpers.c
> +++ b/kernel/bpf/helpers.c
> @@ -2544,7 +2544,7 @@ __bpf_kfunc void bpf_throw(u64 cookie)
>
> __bpf_kfunc_end_defs();
>
> -BTF_SET8_START(generic_btf_ids)
> +BTF_KFUNCS_START(generic_btf_ids)
> #ifdef CONFIG_KEXEC_CORE
> BTF_ID_FLAGS(func, crash_kexec, KF_DESTRUCTIVE)
> #endif
> @@ -2573,7 +2573,7 @@ BTF_ID_FLAGS(func, bpf_task_get_cgroup1, KF_ACQUIRE | KF_RCU | KF_RET_NULL)
> #endif
> BTF_ID_FLAGS(func, bpf_task_from_pid, KF_ACQUIRE | KF_RET_NULL)
> BTF_ID_FLAGS(func, bpf_throw)
> -BTF_SET8_END(generic_btf_ids)
> +BTF_KFUNCS_END(generic_btf_ids)
>
> static const struct btf_kfunc_id_set generic_kfunc_set = {
> .owner = THIS_MODULE,
> @@ -2589,7 +2589,7 @@ BTF_ID(struct, cgroup)
> BTF_ID(func, bpf_cgroup_release_dtor)
> #endif
>
> -BTF_SET8_START(common_btf_ids)
> +BTF_KFUNCS_START(common_btf_ids)
> BTF_ID_FLAGS(func, bpf_cast_to_kern_ctx)
> BTF_ID_FLAGS(func, bpf_rdonly_cast)
> BTF_ID_FLAGS(func, bpf_rcu_read_lock)
> @@ -2618,7 +2618,7 @@ BTF_ID_FLAGS(func, bpf_dynptr_is_null)
> BTF_ID_FLAGS(func, bpf_dynptr_is_rdonly)
> BTF_ID_FLAGS(func, bpf_dynptr_size)
> BTF_ID_FLAGS(func, bpf_dynptr_clone)
> -BTF_SET8_END(common_btf_ids)
> +BTF_KFUNCS_END(common_btf_ids)
>
> static const struct btf_kfunc_id_set common_kfunc_set = {
> .owner = THIS_MODULE,
> diff --git a/kernel/bpf/map_iter.c b/kernel/bpf/map_iter.c
> index 6abd7c5df4b3..9575314f40a6 100644
> --- a/kernel/bpf/map_iter.c
> +++ b/kernel/bpf/map_iter.c
> @@ -213,9 +213,9 @@ __bpf_kfunc s64 bpf_map_sum_elem_count(const struct bpf_map *map)
>
> __bpf_kfunc_end_defs();
>
> -BTF_SET8_START(bpf_map_iter_kfunc_ids)
> +BTF_KFUNCS_START(bpf_map_iter_kfunc_ids)
> BTF_ID_FLAGS(func, bpf_map_sum_elem_count, KF_TRUSTED_ARGS)
> -BTF_SET8_END(bpf_map_iter_kfunc_ids)
> +BTF_KFUNCS_END(bpf_map_iter_kfunc_ids)
>
> static const struct btf_kfunc_id_set bpf_map_iter_kfunc_set = {
> .owner = THIS_MODULE,
> diff --git a/kernel/cgroup/rstat.c b/kernel/cgroup/rstat.c
> index a8350d2d63e6..07e2284bb499 100644
> --- a/kernel/cgroup/rstat.c
> +++ b/kernel/cgroup/rstat.c
> @@ -562,10 +562,10 @@ void cgroup_base_stat_cputime_show(struct seq_file *seq)
> }
>
> /* Add bpf kfuncs for cgroup_rstat_updated() and cgroup_rstat_flush() */
> -BTF_SET8_START(bpf_rstat_kfunc_ids)
> +BTF_KFUNCS_START(bpf_rstat_kfunc_ids)
> BTF_ID_FLAGS(func, cgroup_rstat_updated)
> BTF_ID_FLAGS(func, cgroup_rstat_flush, KF_SLEEPABLE)
> -BTF_SET8_END(bpf_rstat_kfunc_ids)
> +BTF_KFUNCS_END(bpf_rstat_kfunc_ids)
>
> static const struct btf_kfunc_id_set bpf_rstat_kfunc_set = {
> .owner = THIS_MODULE,
> diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> index 64fdaf79d113..241ddf5e3895 100644
> --- a/kernel/trace/bpf_trace.c
> +++ b/kernel/trace/bpf_trace.c
> @@ -1412,14 +1412,14 @@ __bpf_kfunc int bpf_verify_pkcs7_signature(struct bpf_dynptr_kern *data_ptr,
>
> __bpf_kfunc_end_defs();
>
> -BTF_SET8_START(key_sig_kfunc_set)
> +BTF_KFUNCS_START(key_sig_kfunc_set)
> BTF_ID_FLAGS(func, bpf_lookup_user_key, KF_ACQUIRE | KF_RET_NULL | KF_SLEEPABLE)
> BTF_ID_FLAGS(func, bpf_lookup_system_key, KF_ACQUIRE | KF_RET_NULL)
> BTF_ID_FLAGS(func, bpf_key_put, KF_RELEASE)
> #ifdef CONFIG_SYSTEM_DATA_VERIFICATION
> BTF_ID_FLAGS(func, bpf_verify_pkcs7_signature, KF_SLEEPABLE)
> #endif
> -BTF_SET8_END(key_sig_kfunc_set)
> +BTF_KFUNCS_END(key_sig_kfunc_set)
>
> static const struct btf_kfunc_id_set bpf_key_sig_kfunc_set = {
> .owner = THIS_MODULE,
> @@ -1475,9 +1475,9 @@ __bpf_kfunc int bpf_get_file_xattr(struct file *file, const char *name__str,
>
> __bpf_kfunc_end_defs();
>
> -BTF_SET8_START(fs_kfunc_set_ids)
> +BTF_KFUNCS_START(fs_kfunc_set_ids)
> BTF_ID_FLAGS(func, bpf_get_file_xattr, KF_SLEEPABLE | KF_TRUSTED_ARGS)
> -BTF_SET8_END(fs_kfunc_set_ids)
> +BTF_KFUNCS_END(fs_kfunc_set_ids)
>
> static int bpf_get_file_xattr_filter(const struct bpf_prog *prog, u32 kfunc_id)
> {
> diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c
> index dfd919374017..5535f9adc658 100644
> --- a/net/bpf/test_run.c
> +++ b/net/bpf/test_run.c
> @@ -617,21 +617,21 @@ CFI_NOSEAL(bpf_kfunc_call_memb_release_dtor);
>
> __bpf_kfunc_end_defs();
>
> -BTF_SET8_START(bpf_test_modify_return_ids)
> +BTF_KFUNCS_START(bpf_test_modify_return_ids)
> BTF_ID_FLAGS(func, bpf_modify_return_test)
> BTF_ID_FLAGS(func, bpf_modify_return_test2)
> BTF_ID_FLAGS(func, bpf_fentry_test1, KF_SLEEPABLE)
> -BTF_SET8_END(bpf_test_modify_return_ids)
> +BTF_KFUNCS_END(bpf_test_modify_return_ids)
>
> static const struct btf_kfunc_id_set bpf_test_modify_return_set = {
> .owner = THIS_MODULE,
> .set = &bpf_test_modify_return_ids,
> };
>
> -BTF_SET8_START(test_sk_check_kfunc_ids)
> +BTF_KFUNCS_START(test_sk_check_kfunc_ids)
> BTF_ID_FLAGS(func, bpf_kfunc_call_test_release, KF_RELEASE)
> BTF_ID_FLAGS(func, bpf_kfunc_call_memb_release, KF_RELEASE)
> -BTF_SET8_END(test_sk_check_kfunc_ids)
> +BTF_KFUNCS_END(test_sk_check_kfunc_ids)
>
> static void *bpf_test_init(const union bpf_attr *kattr, u32 user_size,
> u32 size, u32 headroom, u32 tailroom)
> diff --git a/net/core/filter.c b/net/core/filter.c
> index 358870408a51..524adf1fa6d0 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
> @@ -11982,21 +11982,21 @@ int bpf_dynptr_from_skb_rdonly(struct sk_buff *skb, u64 flags,
> return 0;
> }
>
> -BTF_SET8_START(bpf_kfunc_check_set_skb)
> +BTF_KFUNCS_START(bpf_kfunc_check_set_skb)
> BTF_ID_FLAGS(func, bpf_dynptr_from_skb)
> -BTF_SET8_END(bpf_kfunc_check_set_skb)
> +BTF_KFUNCS_END(bpf_kfunc_check_set_skb)
>
> -BTF_SET8_START(bpf_kfunc_check_set_xdp)
> +BTF_KFUNCS_START(bpf_kfunc_check_set_xdp)
> BTF_ID_FLAGS(func, bpf_dynptr_from_xdp)
> -BTF_SET8_END(bpf_kfunc_check_set_xdp)
> +BTF_KFUNCS_END(bpf_kfunc_check_set_xdp)
>
> -BTF_SET8_START(bpf_kfunc_check_set_sock_addr)
> +BTF_KFUNCS_START(bpf_kfunc_check_set_sock_addr)
> BTF_ID_FLAGS(func, bpf_sock_addr_set_sun_path)
> -BTF_SET8_END(bpf_kfunc_check_set_sock_addr)
> +BTF_KFUNCS_END(bpf_kfunc_check_set_sock_addr)
>
> -BTF_SET8_START(bpf_kfunc_check_set_tcp_reqsk)
> +BTF_KFUNCS_START(bpf_kfunc_check_set_tcp_reqsk)
> BTF_ID_FLAGS(func, bpf_sk_assign_tcp_reqsk, KF_TRUSTED_ARGS)
> -BTF_SET8_END(bpf_kfunc_check_set_tcp_reqsk)
> +BTF_KFUNCS_END(bpf_kfunc_check_set_tcp_reqsk)
>
> static const struct btf_kfunc_id_set bpf_kfunc_set_skb = {
> .owner = THIS_MODULE,
> @@ -12075,9 +12075,9 @@ __bpf_kfunc int bpf_sock_destroy(struct sock_common *sock)
>
> __bpf_kfunc_end_defs();
>
> -BTF_SET8_START(bpf_sk_iter_kfunc_ids)
> +BTF_KFUNCS_START(bpf_sk_iter_kfunc_ids)
> BTF_ID_FLAGS(func, bpf_sock_destroy, KF_TRUSTED_ARGS)
> -BTF_SET8_END(bpf_sk_iter_kfunc_ids)
> +BTF_KFUNCS_END(bpf_sk_iter_kfunc_ids)
>
> static int tracing_iter_filter(const struct bpf_prog *prog, u32 kfunc_id)
> {
> diff --git a/net/core/xdp.c b/net/core/xdp.c
> index 4869c1c2d8f3..034fb80f3fbe 100644
> --- a/net/core/xdp.c
> +++ b/net/core/xdp.c
> @@ -771,11 +771,11 @@ __bpf_kfunc int bpf_xdp_metadata_rx_vlan_tag(const struct xdp_md *ctx,
>
> __bpf_kfunc_end_defs();
>
> -BTF_SET8_START(xdp_metadata_kfunc_ids)
> +BTF_KFUNCS_START(xdp_metadata_kfunc_ids)
> #define XDP_METADATA_KFUNC(_, __, name, ___) BTF_ID_FLAGS(func, name, KF_TRUSTED_ARGS)
> XDP_METADATA_KFUNC_xxx
> #undef XDP_METADATA_KFUNC
> -BTF_SET8_END(xdp_metadata_kfunc_ids)
> +BTF_KFUNCS_END(xdp_metadata_kfunc_ids)
>
> static const struct btf_kfunc_id_set xdp_metadata_kfunc_set = {
> .owner = THIS_MODULE,
> diff --git a/net/ipv4/bpf_tcp_ca.c b/net/ipv4/bpf_tcp_ca.c
> index 834edc18463a..7f518ea5f4ac 100644
> --- a/net/ipv4/bpf_tcp_ca.c
> +++ b/net/ipv4/bpf_tcp_ca.c
> @@ -201,13 +201,13 @@ bpf_tcp_ca_get_func_proto(enum bpf_func_id func_id,
> }
> }
>
> -BTF_SET8_START(bpf_tcp_ca_check_kfunc_ids)
> +BTF_KFUNCS_START(bpf_tcp_ca_check_kfunc_ids)
> BTF_ID_FLAGS(func, tcp_reno_ssthresh)
> BTF_ID_FLAGS(func, tcp_reno_cong_avoid)
> BTF_ID_FLAGS(func, tcp_reno_undo_cwnd)
> BTF_ID_FLAGS(func, tcp_slow_start)
> BTF_ID_FLAGS(func, tcp_cong_avoid_ai)
> -BTF_SET8_END(bpf_tcp_ca_check_kfunc_ids)
> +BTF_KFUNCS_END(bpf_tcp_ca_check_kfunc_ids)
>
> static const struct btf_kfunc_id_set bpf_tcp_ca_kfunc_set = {
> .owner = THIS_MODULE,
> diff --git a/net/ipv4/fou_bpf.c b/net/ipv4/fou_bpf.c
> index 4da03bf45c9b..06e5572f296f 100644
> --- a/net/ipv4/fou_bpf.c
> +++ b/net/ipv4/fou_bpf.c
> @@ -100,10 +100,10 @@ __bpf_kfunc int bpf_skb_get_fou_encap(struct __sk_buff *skb_ctx,
>
> __bpf_kfunc_end_defs();
>
> -BTF_SET8_START(fou_kfunc_set)
> +BTF_KFUNCS_START(fou_kfunc_set)
> BTF_ID_FLAGS(func, bpf_skb_set_fou_encap)
> BTF_ID_FLAGS(func, bpf_skb_get_fou_encap)
> -BTF_SET8_END(fou_kfunc_set)
> +BTF_KFUNCS_END(fou_kfunc_set)
>
> static const struct btf_kfunc_id_set fou_bpf_kfunc_set = {
> .owner = THIS_MODULE,
> diff --git a/net/ipv4/tcp_bbr.c b/net/ipv4/tcp_bbr.c
> index 22358032dd48..05dc2d05bc7c 100644
> --- a/net/ipv4/tcp_bbr.c
> +++ b/net/ipv4/tcp_bbr.c
> @@ -1155,7 +1155,7 @@ static struct tcp_congestion_ops tcp_bbr_cong_ops __read_mostly = {
> .set_state = bbr_set_state,
> };
>
> -BTF_SET8_START(tcp_bbr_check_kfunc_ids)
> +BTF_KFUNCS_START(tcp_bbr_check_kfunc_ids)
> #ifdef CONFIG_X86
> #ifdef CONFIG_DYNAMIC_FTRACE
> BTF_ID_FLAGS(func, bbr_init)
> @@ -1168,7 +1168,7 @@ BTF_ID_FLAGS(func, bbr_min_tso_segs)
> BTF_ID_FLAGS(func, bbr_set_state)
> #endif
> #endif
> -BTF_SET8_END(tcp_bbr_check_kfunc_ids)
> +BTF_KFUNCS_END(tcp_bbr_check_kfunc_ids)
>
> static const struct btf_kfunc_id_set tcp_bbr_kfunc_set = {
> .owner = THIS_MODULE,
> diff --git a/net/ipv4/tcp_cubic.c b/net/ipv4/tcp_cubic.c
> index 0fd78ecb67e7..44869ea089e3 100644
> --- a/net/ipv4/tcp_cubic.c
> +++ b/net/ipv4/tcp_cubic.c
> @@ -485,7 +485,7 @@ static struct tcp_congestion_ops cubictcp __read_mostly = {
> .name = "cubic",
> };
>
> -BTF_SET8_START(tcp_cubic_check_kfunc_ids)
> +BTF_KFUNCS_START(tcp_cubic_check_kfunc_ids)
> #ifdef CONFIG_X86
> #ifdef CONFIG_DYNAMIC_FTRACE
> BTF_ID_FLAGS(func, cubictcp_init)
> @@ -496,7 +496,7 @@ BTF_ID_FLAGS(func, cubictcp_cwnd_event)
> BTF_ID_FLAGS(func, cubictcp_acked)
> #endif
> #endif
> -BTF_SET8_END(tcp_cubic_check_kfunc_ids)
> +BTF_KFUNCS_END(tcp_cubic_check_kfunc_ids)
>
> static const struct btf_kfunc_id_set tcp_cubic_kfunc_set = {
> .owner = THIS_MODULE,
> diff --git a/net/ipv4/tcp_dctcp.c b/net/ipv4/tcp_dctcp.c
> index bb23bb5b387a..e33fbe4933e4 100644
> --- a/net/ipv4/tcp_dctcp.c
> +++ b/net/ipv4/tcp_dctcp.c
> @@ -260,7 +260,7 @@ static struct tcp_congestion_ops dctcp_reno __read_mostly = {
> .name = "dctcp-reno",
> };
>
> -BTF_SET8_START(tcp_dctcp_check_kfunc_ids)
> +BTF_KFUNCS_START(tcp_dctcp_check_kfunc_ids)
> #ifdef CONFIG_X86
> #ifdef CONFIG_DYNAMIC_FTRACE
> BTF_ID_FLAGS(func, dctcp_init)
> @@ -271,7 +271,7 @@ BTF_ID_FLAGS(func, dctcp_cwnd_undo)
> BTF_ID_FLAGS(func, dctcp_state)
> #endif
> #endif
> -BTF_SET8_END(tcp_dctcp_check_kfunc_ids)
> +BTF_KFUNCS_END(tcp_dctcp_check_kfunc_ids)
>
> static const struct btf_kfunc_id_set tcp_dctcp_kfunc_set = {
> .owner = THIS_MODULE,
> diff --git a/net/netfilter/nf_conntrack_bpf.c b/net/netfilter/nf_conntrack_bpf.c
> index 475358ec8212..d2492d050fe6 100644
> --- a/net/netfilter/nf_conntrack_bpf.c
> +++ b/net/netfilter/nf_conntrack_bpf.c
> @@ -467,7 +467,7 @@ __bpf_kfunc int bpf_ct_change_status(struct nf_conn *nfct, u32 status)
>
> __bpf_kfunc_end_defs();
>
> -BTF_SET8_START(nf_ct_kfunc_set)
> +BTF_KFUNCS_START(nf_ct_kfunc_set)
> BTF_ID_FLAGS(func, bpf_xdp_ct_alloc, KF_ACQUIRE | KF_RET_NULL)
> BTF_ID_FLAGS(func, bpf_xdp_ct_lookup, KF_ACQUIRE | KF_RET_NULL)
> BTF_ID_FLAGS(func, bpf_skb_ct_alloc, KF_ACQUIRE | KF_RET_NULL)
> @@ -478,7 +478,7 @@ BTF_ID_FLAGS(func, bpf_ct_set_timeout, KF_TRUSTED_ARGS)
> BTF_ID_FLAGS(func, bpf_ct_change_timeout, KF_TRUSTED_ARGS)
> BTF_ID_FLAGS(func, bpf_ct_set_status, KF_TRUSTED_ARGS)
> BTF_ID_FLAGS(func, bpf_ct_change_status, KF_TRUSTED_ARGS)
> -BTF_SET8_END(nf_ct_kfunc_set)
> +BTF_KFUNCS_END(nf_ct_kfunc_set)
>
> static const struct btf_kfunc_id_set nf_conntrack_kfunc_set = {
> .owner = THIS_MODULE,
> diff --git a/net/netfilter/nf_nat_bpf.c b/net/netfilter/nf_nat_bpf.c
> index 6e3b2f58855f..481be15609b1 100644
> --- a/net/netfilter/nf_nat_bpf.c
> +++ b/net/netfilter/nf_nat_bpf.c
> @@ -54,9 +54,9 @@ __bpf_kfunc int bpf_ct_set_nat_info(struct nf_conn___init *nfct,
>
> __bpf_kfunc_end_defs();
>
> -BTF_SET8_START(nf_nat_kfunc_set)
> +BTF_KFUNCS_START(nf_nat_kfunc_set)
> BTF_ID_FLAGS(func, bpf_ct_set_nat_info, KF_TRUSTED_ARGS)
> -BTF_SET8_END(nf_nat_kfunc_set)
> +BTF_KFUNCS_END(nf_nat_kfunc_set)
>
> static const struct btf_kfunc_id_set nf_bpf_nat_kfunc_set = {
> .owner = THIS_MODULE,
> diff --git a/net/xfrm/xfrm_interface_bpf.c b/net/xfrm/xfrm_interface_bpf.c
> index 7d5e920141e9..5ea15037ebd1 100644
> --- a/net/xfrm/xfrm_interface_bpf.c
> +++ b/net/xfrm/xfrm_interface_bpf.c
> @@ -93,10 +93,10 @@ __bpf_kfunc int bpf_skb_set_xfrm_info(struct __sk_buff *skb_ctx, const struct bp
>
> __bpf_kfunc_end_defs();
>
> -BTF_SET8_START(xfrm_ifc_kfunc_set)
> +BTF_KFUNCS_START(xfrm_ifc_kfunc_set)
> BTF_ID_FLAGS(func, bpf_skb_get_xfrm_info)
> BTF_ID_FLAGS(func, bpf_skb_set_xfrm_info)
> -BTF_SET8_END(xfrm_ifc_kfunc_set)
> +BTF_KFUNCS_END(xfrm_ifc_kfunc_set)
>
> static const struct btf_kfunc_id_set xfrm_interface_kfunc_set = {
> .owner = THIS_MODULE,
> diff --git a/net/xfrm/xfrm_state_bpf.c b/net/xfrm/xfrm_state_bpf.c
> index 9e20d4a377f7..2248eda741f8 100644
> --- a/net/xfrm/xfrm_state_bpf.c
> +++ b/net/xfrm/xfrm_state_bpf.c
> @@ -117,10 +117,10 @@ __bpf_kfunc void bpf_xdp_xfrm_state_release(struct xfrm_state *x)
>
> __bpf_kfunc_end_defs();
>
> -BTF_SET8_START(xfrm_state_kfunc_set)
> +BTF_KFUNCS_START(xfrm_state_kfunc_set)
> BTF_ID_FLAGS(func, bpf_xdp_get_xfrm_state, KF_RET_NULL | KF_ACQUIRE)
> BTF_ID_FLAGS(func, bpf_xdp_xfrm_state_release, KF_RELEASE)
> -BTF_SET8_END(xfrm_state_kfunc_set)
> +BTF_KFUNCS_END(xfrm_state_kfunc_set)
>
> static const struct btf_kfunc_id_set xfrm_state_xdp_kfunc_set = {
> .owner = THIS_MODULE,
> diff --git a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c
> index 8befaf17d454..53f73fc86baa 100644
> --- a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c
> +++ b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c
> @@ -343,12 +343,12 @@ static struct bin_attribute bin_attr_bpf_testmod_file __ro_after_init = {
> .write = bpf_testmod_test_write,
> };
>
> -BTF_SET8_START(bpf_testmod_common_kfunc_ids)
> +BTF_KFUNCS_START(bpf_testmod_common_kfunc_ids)
> BTF_ID_FLAGS(func, bpf_iter_testmod_seq_new, KF_ITER_NEW)
> BTF_ID_FLAGS(func, bpf_iter_testmod_seq_next, KF_ITER_NEXT | KF_RET_NULL)
> BTF_ID_FLAGS(func, bpf_iter_testmod_seq_destroy, KF_ITER_DESTROY)
> BTF_ID_FLAGS(func, bpf_kfunc_common_test)
> -BTF_SET8_END(bpf_testmod_common_kfunc_ids)
> +BTF_KFUNCS_END(bpf_testmod_common_kfunc_ids)
>
> static const struct btf_kfunc_id_set bpf_testmod_common_kfunc_set = {
> .owner = THIS_MODULE,
> @@ -494,7 +494,7 @@ __bpf_kfunc static u32 bpf_kfunc_call_test_static_unused_arg(u32 arg, u32 unused
> return arg;
> }
>
> -BTF_SET8_START(bpf_testmod_check_kfunc_ids)
> +BTF_KFUNCS_START(bpf_testmod_check_kfunc_ids)
> BTF_ID_FLAGS(func, bpf_testmod_test_mod_kfunc)
> BTF_ID_FLAGS(func, bpf_kfunc_call_test1)
> BTF_ID_FLAGS(func, bpf_kfunc_call_test2)
> @@ -520,7 +520,7 @@ BTF_ID_FLAGS(func, bpf_kfunc_call_test_ref, KF_TRUSTED_ARGS | KF_RCU)
> BTF_ID_FLAGS(func, bpf_kfunc_call_test_destructive, KF_DESTRUCTIVE)
> BTF_ID_FLAGS(func, bpf_kfunc_call_test_static_unused_arg)
> BTF_ID_FLAGS(func, bpf_kfunc_call_test_offset)
> -BTF_SET8_END(bpf_testmod_check_kfunc_ids)
> +BTF_KFUNCS_END(bpf_testmod_check_kfunc_ids)
>
> static int bpf_testmod_ops_init(struct btf *btf)
> {
> --
> 2.42.1
>
^ 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