* [PATCH 1/4] HID: core: remove #ifdef CONFIG_PM from hid_driver
From: Thomas Weißschuh @ 2023-10-12 10:23 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires
Cc: linux-input, linux-kernel, linux-usb, Thomas Weißschuh
In-Reply-To: <20231012-hid-pm_ptr-v1-0-0a71531ca93b@weissschuh.net>
Allow HID drivers to pass ->suspend, ->resume and ->reset_resume via
pm_ptr().
Through the usage of pm_ptr() the CONFIG_PM-dependent code will always be
compiled, protecting against bitrot.
The linker will then garbage-collect the unused function avoiding any overhead.
The only overhead in the final kernel image and at runtime are a few
extra bytes in 'struct hid_driver'.
The same approach is chosen by 'struct usb_driver' and other subsystems.
Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
---
include/linux/hid.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/include/linux/hid.h b/include/linux/hid.h
index 964ca1f15e3f..5a8387a4a712 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -833,11 +833,11 @@ struct hid_driver {
void (*feature_mapping)(struct hid_device *hdev,
struct hid_field *field,
struct hid_usage *usage);
-#ifdef CONFIG_PM
+
int (*suspend)(struct hid_device *hdev, pm_message_t message);
int (*resume)(struct hid_device *hdev);
int (*reset_resume)(struct hid_device *hdev);
-#endif
+
/* private: */
struct device_driver driver;
};
--
2.42.0
^ permalink raw reply related
* [PATCH 0/4] HID: remove #ifdef CONFIG_PM
From: Thomas Weißschuh @ 2023-10-12 10:23 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires
Cc: linux-input, linux-kernel, linux-usb, Thomas Weißschuh
Through the usage of pm_ptr() the CONFIG_PM-dependent code will always be
compiled, protecting against bitrot.
The linker will then garbage-collect the unused function avoiding any overhead.
This series only converts three users of CONFIG_PM in drivers/hid/ but
most of the others should be convertible, too.
Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
---
Thomas Weißschuh (4):
HID: core: remove #ifdef CONFIG_PM from hid_driver
HID: usbhid: remove #ifdef CONFIG_PM
HID: multitouch: remove #ifdef CONFIG_PM
HID: rmi: remove #ifdef CONFIG_PM
drivers/hid/hid-multitouch.c | 10 +++-------
drivers/hid/hid-rmi.c | 10 +++-------
drivers/hid/usbhid/hid-core.c | 11 +++--------
include/linux/hid.h | 4 ++--
4 files changed, 11 insertions(+), 24 deletions(-)
---
base-commit: 0bb80ecc33a8fb5a682236443c1e740d5c917d1d
change-id: 20231012-hid-pm_ptr-e29ab5ee7ce7
Best regards,
--
Thomas Weißschuh <linux@weissschuh.net>
^ permalink raw reply
* [PATCH 2/4] HID: usbhid: remove #ifdef CONFIG_PM
From: Thomas Weißschuh @ 2023-10-12 10:23 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires
Cc: linux-input, linux-kernel, linux-usb, Thomas Weißschuh
In-Reply-To: <20231012-hid-pm_ptr-v1-0-0a71531ca93b@weissschuh.net>
Through the usage of pm_ptr() the CONFIG_PM-dependent code will always be
compiled, protecting against bitrot.
The linker will then garbage-collect the unused function avoiding any overhead.
Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
---
drivers/hid/usbhid/hid-core.c | 11 +++--------
1 file changed, 3 insertions(+), 8 deletions(-)
diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c
index 257dd73e37bf..a90ed2ceae84 100644
--- a/drivers/hid/usbhid/hid-core.c
+++ b/drivers/hid/usbhid/hid-core.c
@@ -1562,7 +1562,6 @@ static int hid_post_reset(struct usb_interface *intf)
return 0;
}
-#ifdef CONFIG_PM
static int hid_resume_common(struct hid_device *hid, bool driver_suspended)
{
int status = 0;
@@ -1654,8 +1653,6 @@ static int hid_reset_resume(struct usb_interface *intf)
return status;
}
-#endif /* CONFIG_PM */
-
static const struct usb_device_id hid_usb_ids[] = {
{ .match_flags = USB_DEVICE_ID_MATCH_INT_CLASS,
.bInterfaceClass = USB_INTERFACE_CLASS_HID },
@@ -1668,11 +1665,9 @@ static struct usb_driver hid_driver = {
.name = "usbhid",
.probe = usbhid_probe,
.disconnect = usbhid_disconnect,
-#ifdef CONFIG_PM
- .suspend = hid_suspend,
- .resume = hid_resume,
- .reset_resume = hid_reset_resume,
-#endif
+ .suspend = pm_ptr(hid_suspend),
+ .resume = pm_ptr(hid_resume),
+ .reset_resume = pm_ptr(hid_reset_resume),
.pre_reset = hid_pre_reset,
.post_reset = hid_post_reset,
.id_table = hid_usb_ids,
--
2.42.0
^ permalink raw reply related
* [PATCH 4/4] HID: rmi: remove #ifdef CONFIG_PM
From: Thomas Weißschuh @ 2023-10-12 10:23 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires
Cc: linux-input, linux-kernel, linux-usb, Thomas Weißschuh
In-Reply-To: <20231012-hid-pm_ptr-v1-0-0a71531ca93b@weissschuh.net>
Through the usage of pm_ptr() the CONFIG_PM-dependent code will always be
compiled, protecting against bitrot.
The linker will then garbage-collect the unused function avoiding any overhead.
Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
---
drivers/hid/hid-rmi.c | 10 +++-------
1 file changed, 3 insertions(+), 7 deletions(-)
diff --git a/drivers/hid/hid-rmi.c b/drivers/hid/hid-rmi.c
index 84e7ba5314d3..d4af17fdba46 100644
--- a/drivers/hid/hid-rmi.c
+++ b/drivers/hid/hid-rmi.c
@@ -436,7 +436,6 @@ static void rmi_report(struct hid_device *hid, struct hid_report *report)
input_sync(field->hidinput->input);
}
-#ifdef CONFIG_PM
static int rmi_suspend(struct hid_device *hdev, pm_message_t message)
{
struct rmi_data *data = hid_get_drvdata(hdev);
@@ -483,7 +482,6 @@ static int rmi_post_resume(struct hid_device *hdev)
hid_hw_close(hdev);
return ret;
}
-#endif /* CONFIG_PM */
static int rmi_hid_reset(struct rmi_transport_dev *xport, u16 reset_addr)
{
@@ -774,11 +772,9 @@ static struct hid_driver rmi_driver = {
.report = rmi_report,
.input_mapping = rmi_input_mapping,
.input_configured = rmi_input_configured,
-#ifdef CONFIG_PM
- .suspend = rmi_suspend,
- .resume = rmi_post_resume,
- .reset_resume = rmi_post_resume,
-#endif
+ .suspend = pm_ptr(rmi_suspend),
+ .resume = pm_ptr(rmi_post_resume),
+ .reset_resume = pm_ptr(rmi_post_resume),
};
module_hid_driver(rmi_driver);
--
2.42.0
^ permalink raw reply related
* [PATCH 3/4] HID: multitouch: remove #ifdef CONFIG_PM
From: Thomas Weißschuh @ 2023-10-12 10:23 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires
Cc: linux-input, linux-kernel, linux-usb, Thomas Weißschuh
In-Reply-To: <20231012-hid-pm_ptr-v1-0-0a71531ca93b@weissschuh.net>
Through the usage of pm_ptr() the CONFIG_PM-dependent code will always be
compiled, protecting against bitrot.
The linker will then garbage-collect the unused function avoiding any overhead.
Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
---
drivers/hid/hid-multitouch.c | 10 +++-------
1 file changed, 3 insertions(+), 7 deletions(-)
diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index 521b2ffb4244..2cdfc2a70a83 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c
@@ -1802,7 +1802,6 @@ static int mt_probe(struct hid_device *hdev, const struct hid_device_id *id)
return 0;
}
-#ifdef CONFIG_PM
static int mt_suspend(struct hid_device *hdev, pm_message_t state)
{
struct mt_device *td = hid_get_drvdata(hdev);
@@ -1836,7 +1835,6 @@ static int mt_resume(struct hid_device *hdev)
return 0;
}
-#endif
static void mt_remove(struct hid_device *hdev)
{
@@ -2255,10 +2253,8 @@ static struct hid_driver mt_driver = {
.usage_table = mt_grabbed_usages,
.event = mt_event,
.report = mt_report,
-#ifdef CONFIG_PM
- .suspend = mt_suspend,
- .reset_resume = mt_reset_resume,
- .resume = mt_resume,
-#endif
+ .suspend = pm_ptr(mt_suspend),
+ .reset_resume = pm_ptr(mt_reset_resume),
+ .resume = pm_ptr(mt_resume),
};
module_hid_driver(mt_driver);
--
2.42.0
^ permalink raw reply related
* Re: [PATCH v4 1/2] dt-bindings: input: bindings for Adafruit Seesaw Gamepad
From: Anshul Dalal @ 2023-10-12 10:54 UTC (permalink / raw)
To: Krzysztof Kozlowski, linux-input, devicetree
Cc: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Shuah Khan, linux-kernel-mentees, linux-kernel
In-Reply-To: <7c570333-7334-435c-83cd-225817afc51c@linaro.org>
On 10/12/23 14:09, Krzysztof Kozlowski wrote:
> On 10/10/2023 20:48, Anshul Dalal wrote:
>> Adds bindings for the Adafruit Seesaw Gamepad.
>>
>> The gamepad functions as an i2c device with the default address of 0x50
>> and has an IRQ pin that can be enabled in the driver to allow for a rising
>> edge trigger on each button press or joystick movement.
>>
>> Product page:
>> https://www.adafruit.com/product/5743
>> Arduino driver:
>> https://github.com/adafruit/Adafruit_Seesaw
>>
>> Signed-off-by: Anshul Dalal <anshulusr@gmail.com>
>> ---
>>
>> Changes for v4:
>> - Fixed the URI for the id field
>> - Added `interrupts` property
>>
>> Changes for v3:
>> - Updated id field to reflect updated file name from previous version
>> - Added `reg` property
>>
>> Changes for v2:
>> - Renamed file to `adafruit,seesaw-gamepad.yaml`
>> - Removed quotes for `$id` and `$schema`
>> - Removed "Bindings for" from the description
>> - Changed node name to the generic name "joystick"
>> - Changed compatible to 'adafruit,seesaw-gamepad' instead of
>> 'adafruit,seesaw_gamepad'
>>
>> .../input/adafruit,seesaw-gamepad.yaml | 59 +++++++++++++++++++
>> 1 file changed, 59 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/input/adafruit,seesaw-gamepad.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/input/adafruit,seesaw-gamepad.yaml b/Documentation/devicetree/bindings/input/adafruit,seesaw-gamepad.yaml
>> new file mode 100644
>> index 000000000000..e8e676006d2f
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/input/adafruit,seesaw-gamepad.yaml
>> @@ -0,0 +1,59 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/input/adafruit,seesaw-gamepad.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Adafruit Mini I2C Gamepad with seesaw
>> +
>> +maintainers:
>> + - Anshul Dalal <anshulusr@gmail.com>
>> +
>> +description: |
>> + Adafruit Mini I2C Gamepad
>> +
>> + +-----------------------------+
>> + | ___ |
>> + | / \ (X) |
>> + | | S | __ __ (Y) (A) |
>> + | \___/ |ST| |SE| (B) |
>> + | |
>> + +-----------------------------+
>> +
>> + S -> 10-bit percision bidirectional analog joystick
>> + ST -> Start
>> + SE -> Select
>> + X, A, B, Y -> Digital action buttons
>> +
>> + Product page: https://www.adafruit.com/product/5743
>> + Arduino Driver: https://github.com/adafruit/Adafruit_Seesaw
>> +
>> +properties:
>> + compatible:
>> + const: adafruit,seesaw-gamepad
>
> I thought seesaw is a name of a device, but it is not, thus it is quite
> a generic compatible. Are you sure device does not have its name?
> Looking at product page, it indeed might not have.
>
I chose the title from the datasheet:
https://cdn-learn.adafruit.com/downloads/pdf/gamepad-qt.pdf
It lists the product name as "Adafruit Mini I2C STEMMA QT Gamepad with
seesaw" with STEMMA QT being the name of the 4-pin i2c connector that
Adafruit uses on their products. That's the most specific name for the
product I could find.
>
> Best regards,
> Krzysztof
>
Thanks,
Anshul
^ permalink raw reply
* Re: [PATCH v4 1/2] dt-bindings: input: bindings for Adafruit Seesaw Gamepad
From: Krzysztof Kozlowski @ 2023-10-12 12:36 UTC (permalink / raw)
To: Anshul Dalal, linux-input, devicetree
Cc: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Shuah Khan, linux-kernel-mentees, linux-kernel
In-Reply-To: <20231010184827.1213507-1-anshulusr@gmail.com>
On 10/10/2023 20:48, Anshul Dalal wrote:
> Adds bindings for the Adafruit Seesaw Gamepad.
>
> The gamepad functions as an i2c device with the default address of 0x50
> and has an IRQ pin that can be enabled in the driver to allow for a rising
> edge trigger on each button press or joystick movement.
>
> Product page:
> https://www.adafruit.com/product/5743
> Arduino driver:
> https://github.com/adafruit/Adafruit_Seesaw
>
> Signed-off-by: Anshul Dalal <anshulusr@gmail.com>
> ---
>
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH V2 1/2] dt-bindings: input: Introduce Himax HID-over-SPI device
From: Conor Dooley @ 2023-10-12 15:24 UTC (permalink / raw)
To: yang tylor
Cc: Conor Dooley, dmitry.torokhov, robh+dt, krzysztof.kozlowski+dt,
conor+dt, jikos, benjamin.tissoires, linux-input, devicetree,
linux-kernel, poyuan_chang, hbarnor, jingyliang@chromium.org,
wuxy23, luolm1, hung poyu
In-Reply-To: <CAGD2q_aqr+mu4K1SkTVC+65ctL6BsqRP4Ld0HD_H0_rgzFT9MQ@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 4092 bytes --]
On Thu, Oct 12, 2023 at 10:30:03AM +0800, yang tylor wrote:
> On Tue, Oct 10, 2023 at 1:52 AM Conor Dooley <conor@kernel.org> wrote:
> >
> > On Mon, Oct 02, 2023 at 06:44:41PM +0800, yang tylor wrote:
> > > On Fri, Sep 29, 2023 at 12:56 AM Conor Dooley <conor@kernel.org> wrote:
> > > >
> > > > On Thu, Sep 28, 2023 at 10:12:41AM +0800, yang tylor wrote:
> > > > > On Tue, Sep 26, 2023 at 8:53 PM Conor Dooley <conor@kernel.org> wrote:
> > > > > > On Tue, Sep 26, 2023 at 05:52:39PM +0800, yang tylor wrote:
> > > > > > > On Tue, Sep 26, 2023 at 5:02 PM Conor Dooley <conor@kernel.org> wrote:
> > > > > > > > On Mon, Sep 25, 2023 at 06:16:29PM +0800, yang tylor wrote:
> > > > > > > > > On Mon, Sep 25, 2023 at 4:41 PM Conor Dooley <conor.dooley@microchip.com> wrote:
> > > > > > > > > We have a default prefix firmware name(like himax_xxxx.bin) in the driver code.
> > > > > > > >
> > > > > > > > How do you intend generating the name of the firmware file? I assume the
> > > > > > > > same firmware doesn't work on every IC, so you'll need to pick a
> > > > > > > > different one depending on the compatible?
> > > > > > > >
> > > > > > > If considering a firmware library line-up for all the incoming panels
> > > > > > > of this driver.
> > > > > > > We would use PID as part of the file name. Because all the support panels would
> > > > > > > have a unique PID associated. Which will make the firmware name like
> > > > > > > himax_xxx_{$PID}.bin. The problem is, we need to know PID before firmware load
> > > > > > > at no flash condition. Thus PID information is required in dts when
> > > > > > > no-flash-flag
> > > > > > > is specified.
> > > > > >
> > > > > > Firstly, where does the "xxx" come from?
> > > > > > And you're making it sound more like having firmware-name is suitable
> > > > > > for this use case, given you need to determine the name of the file to
> > > > > > use based on something that is hardware specific but is not
> > > > > > dynamically detectable.
> > > > > Current driver patch uses a prefix name "himax_i2chid" which comes
> > > > > from the previous project
> > > > > and seems not suitable for this condition, so I use "xxx" and plan to
> > > > > replace it in the next version.
> > > > > For finding firmware, I think both solutions are reasonable.
> > > > > - provide firmware name directly: implies no-flash and use user
> > > > > specified firmware, no PID info.
> > > > > - provide no-flash-flag and PID info: loading firmware from organized
> > > > > names with PID info.
> > > > > I prefer the 2nd solution, but it needs more properties in dts. 1st
> > > > > has less properties and more
> > > > > intuitive.
> > > > >
> > > > > I don't know which one is more acceptable by the community, as you
> > > > > know I'm a newbie here.
> > > >
> > > > To be honest, I am not all that sure either! Does the panel id have
> > > > value in its own right, or is that only used to determine the firmware
> > > > filename?
> > > Currently, PID stands for Panel/Project ID and is used for determining
> > > the firmware filename only. We haven't come up with any new attribute that
> > > may attach to it. The differences between panels are handled in firmware
> > > dedicated to its PID.
> > >
> > > > Also, if it does have value in its own right, rather than a "pid",
> > > > should the panel be a child node of this hid device with its own
> > > > compatible?
> > > It may need a child node if we find it necessary to add attributes to each PID.
> > > But currently we have no idea about it.
> >
> > To be honest, it seems to me like you are using "PID" in place of a
> > compatible for the panel, since it needs to be provided via DT anyway.
>
> Hmm... So the more formal way is?
> If I add a sub-note inside this spi-device block, such as "panel" and
> add PID inside.
> Will it be more appropriate?
> ...
> spi {
> ...
> hx_spi@0 {
> ...
> panel {
> himax,pid = ...
And this now looks exactly like compatible = "vendor,part" now, no?
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: [PATCH v4 1/2] dt-bindings: input: bindings for Adafruit Seesaw Gamepad
From: Conor Dooley @ 2023-10-12 15:46 UTC (permalink / raw)
To: Anshul Dalal
Cc: linux-input, devicetree, Dmitry Torokhov, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Shuah Khan,
linux-kernel-mentees, linux-kernel
In-Reply-To: <f1796d1a-bcd0-414d-b4e1-806e93eb202b@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 4477 bytes --]
On Thu, Oct 12, 2023 at 12:29:58AM +0530, Anshul Dalal wrote:
> Hello,
>
> On 10/11/23 21:45, Conor Dooley wrote:
> > Hey,
> >
> > On Wed, Oct 11, 2023 at 12:18:23AM +0530, Anshul Dalal wrote:
> >> Adds bindings for the Adafruit Seesaw Gamepad.
> >>
> >> The gamepad functions as an i2c device with the default address of 0x50
> >> and has an IRQ pin that can be enabled in the driver to allow for a rising
> >> edge trigger on each button press or joystick movement.
> >>
> >> Product page:
> >> https://www.adafruit.com/product/5743
> >> Arduino driver:
> >> https://github.com/adafruit/Adafruit_Seesaw
> >>
> >> Signed-off-by: Anshul Dalal <anshulusr@gmail.com>
> >> ---
> >>
> >> Changes for v4:
> >> - Fixed the URI for the id field
> >> - Added `interrupts` property
> >>
> >> Changes for v3:
> >> - Updated id field to reflect updated file name from previous version
> >> - Added `reg` property
> >>
> >> Changes for v2:
> >> - Renamed file to `adafruit,seesaw-gamepad.yaml`
> >> - Removed quotes for `$id` and `$schema`
> >> - Removed "Bindings for" from the description
> >> - Changed node name to the generic name "joystick"
> >> - Changed compatible to 'adafruit,seesaw-gamepad' instead of
> >> 'adafruit,seesaw_gamepad'
> >>
> >> .../input/adafruit,seesaw-gamepad.yaml | 59 +++++++++++++++++++
> >> 1 file changed, 59 insertions(+)
> >> create mode 100644 Documentation/devicetree/bindings/input/adafruit,seesaw-gamepad.yaml
> >>
> >> diff --git a/Documentation/devicetree/bindings/input/adafruit,seesaw-gamepad.yaml b/Documentation/devicetree/bindings/input/adafruit,seesaw-gamepad.yaml
> >> new file mode 100644
> >> index 000000000000..e8e676006d2f
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/input/adafruit,seesaw-gamepad.yaml
> >> @@ -0,0 +1,59 @@
> >> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> >> +%YAML 1.2
> >> +---
> >> +$id: http://devicetree.org/schemas/input/adafruit,seesaw-gamepad.yaml#
> >> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> >> +
> >> +title: Adafruit Mini I2C Gamepad with seesaw
> >
> > Binding mostly looks good to me. My main question is what is a seesaw?
> >
>
> Seesaw is a universal framework that enables extending I/O capabilities
> of the i2c master devices with a compatible breakout board. As it
> relates to the binding, this gamepad uses an AVR ATtiny816
> microcontroller that reads the data from the buttons and the joystick
> and sends the data to the master over i2c using the Seesaw framework.
Right. Not a framework I was aware of, thanks for explaining.
>
> >> +
> >> +maintainers:
> >> + - Anshul Dalal <anshulusr@gmail.com>
> >> +
> >> +description: |
> >> + Adafruit Mini I2C Gamepad
> >> +
> >> + +-----------------------------+
> >> + | ___ |
> >> + | / \ (X) |
> >> + | | S | __ __ (Y) (A) |
> >> + | \___/ |ST| |SE| (B) |
> >> + | |
> >> + +-----------------------------+
> >> +
> >> + S -> 10-bit percision bidirectional analog joystick
> >> + ST -> Start
> >> + SE -> Select
> >> + X, A, B, Y -> Digital action buttons
> >> +
> >> + Product page: https://www.adafruit.com/product/5743
> >> + Arduino Driver: https://github.com/adafruit/Adafruit_Seesaw
> >
> > I'm not really sure what the arduino driver has to do with the binding.
> > Why is a link to it more relevant than the freebsd driver, or the linux
> > driver etc? Is there info about how the pad works in the arduino driver
> >
> > Otherwise, this seems good to me.
> The Arduino driver I linked was the only resource that had the
> implementation of the seesaw framework as well as the example code
> specific to this device:
> https://github.com/adafruit/Adafruit_Seesaw/tree/master/examples/Mini_I2C_Gamepad_QT
> On further thought, a link to the accompanying document from the
> manufacturer (https://cdn-learn.adafruit.com/downloads/pdf/gamepad-qt.pdf)
> might be more relevant for the binding which includes the hardware
> description as well as links to the above-mentioned Arduino driver.
A link to the manufacturer docs would be nice & if, as you say, the
arduino driver was a useful resource for understanding the hardware then
I suppose it can stay too :)
Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
Thanks,
Conor.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* [PATCH v5 1/2] dt-bindings: input: bindings for Adafruit Seesaw Gamepad
From: Anshul Dalal @ 2023-10-12 16:27 UTC (permalink / raw)
To: linux-input, devicetree
Cc: Anshul Dalal, Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Thomas Weißschuh, Shuah Khan,
linux-kernel-mentees, linux-kernel, Conor Dooley,
Krzysztof Kozlowski
Adds bindings for the Adafruit Seesaw Gamepad.
The gamepad functions as an i2c device with the default address of 0x50
and has an IRQ pin that can be enabled in the driver to allow for a rising
edge trigger on each button press or joystick movement.
Product page:
https://www.adafruit.com/product/5743
Arduino driver:
https://github.com/adafruit/Adafruit_Seesaw
Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Signed-off-by: Anshul Dalal <anshulusr@gmail.com>
---
Changes for v5:
- Added link to the datasheet
Changes for v4:
- Fixed the URI for the id field
- Added `interrupts` property
Changes for v3:
- Updated id field to reflect updated file name from previous version
- Added `reg` property
Changes for v2:
- Renamed file to `adafruit,seesaw-gamepad.yaml`
- Removed quotes for `$id` and `$schema`
- Removed "Bindings for" from the description
- Changed node name to the generic name "joystick"
- Changed compatible to 'adafruit,seesaw-gamepad' instead of
'adafruit,seesaw_gamepad'
.../input/adafruit,seesaw-gamepad.yaml | 60 +++++++++++++++++++
1 file changed, 60 insertions(+)
create mode 100644 Documentation/devicetree/bindings/input/adafruit,seesaw-gamepad.yaml
diff --git a/Documentation/devicetree/bindings/input/adafruit,seesaw-gamepad.yaml b/Documentation/devicetree/bindings/input/adafruit,seesaw-gamepad.yaml
new file mode 100644
index 000000000000..3f0d1c5a3b9b
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/adafruit,seesaw-gamepad.yaml
@@ -0,0 +1,60 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/input/adafruit,seesaw-gamepad.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Adafruit Mini I2C Gamepad with seesaw
+
+maintainers:
+ - Anshul Dalal <anshulusr@gmail.com>
+
+description: |
+ Adafruit Mini I2C Gamepad
+
+ +-----------------------------+
+ | ___ |
+ | / \ (X) |
+ | | S | __ __ (Y) (A) |
+ | \___/ |ST| |SE| (B) |
+ | |
+ +-----------------------------+
+
+ S -> 10-bit percision bidirectional analog joystick
+ ST -> Start
+ SE -> Select
+ X, A, B, Y -> Digital action buttons
+
+ Datasheet: https://cdn-learn.adafruit.com/downloads/pdf/gamepad-qt.pdf
+ Product page: https://www.adafruit.com/product/5743
+ Arduino Driver: https://github.com/adafruit/Adafruit_Seesaw
+
+properties:
+ compatible:
+ const: adafruit,seesaw-gamepad
+
+ reg:
+ maxItems: 1
+
+ interrupts:
+ maxItems: 1
+ description:
+ The gamepad's IRQ pin triggers a rising edge if interrupts are enabled.
+
+required:
+ - compatible
+ - reg
+
+additionalProperties: false
+
+examples:
+ - |
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ joystick@50 {
+ compatible = "adafruit,seesaw-gamepad";
+ reg = <0x50>;
+ };
+ };
--
2.42.0
^ permalink raw reply related
* [PATCH v5 2/2] input: joystick: driver for Adafruit Seesaw Gamepad
From: Anshul Dalal @ 2023-10-12 16:27 UTC (permalink / raw)
To: linux-input, devicetree
Cc: Anshul Dalal, Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Thomas Weißschuh, Shuah Khan,
linux-kernel-mentees, linux-kernel
In-Reply-To: <20231012162759.691555-1-anshulusr@gmail.com>
Adds a driver for a mini gamepad that communicates over i2c, the gamepad
has bidirectional thumb stick input and six buttons.
The gamepad chip utilizes the open framework from Adafruit called 'Seesaw'
to transmit the ADC data for the joystick and digital pin state for the
buttons. I have only implemented the functionality required to receive the
thumb stick and button state.
Steps in reading the gamepad state over i2c:
1. Reset the registers
2. Set the pin mode of the pins specified by the `BUTTON_MASK` to input
`BUTTON_MASK`: A bit-map for the six digital pins internally
connected to the joystick buttons.
3. Enable internal pullup resistors for the `BUTTON_MASK`
4. Bulk set the pin state HIGH for `BUTTON_MASK`
5. Poll the device for button and joystick state done by:
`seesaw_read_data(struct i2c_client *client, struct seesaw_data *data)`
Product page:
https://www.adafruit.com/product/5743
Arduino driver:
https://github.com/adafruit/Adafruit_Seesaw
Driver tested on RPi Zero 2W
Reviewed-by: Thomas Weißschuh <linux@weissschuh.net>
Signed-off-by: Anshul Dalal <anshulusr@gmail.com>
---
Changes for v5:
- Added link to the datasheet
- Added debug log message when `seesaw_read_data` fails
Changes for v4:
- Changed `1UL << BUTTON_` to BIT(BUTTON_)
- Removed `hardware_id` field from `struct seesaw_gamepad`
- Removed redundant checks for the number of bytes written and received by
`i2c_master_send` and `i2c_master_recv`
- Used `get_unaligned_be32` to instantiate `u32 result` from `read_buf`
- Changed `result & (1UL << BUTTON_)` to
`test_bit(BUTTON_, (long *)&result)`
- Changed `KBUILD_MODNAME` in id-tables to `SEESAW_DEVICE_NAME`
- Fixed formatting issues
- Changed button reporting:
Since the gamepad had the action buttons in a non-standard layout:
(X)
(Y) (A)
(B)
Therefore moved to using generic directional action button event codes
instead of BTN_[ABXY].
Changes for v3:
- no updates
Changes for v2:
adafruit-seesaw.c:
- Renamed file from 'adafruit_seesaw.c'
- Changed device name from 'seesaw_gamepad' to 'seesaw-gamepad'
- Changed count parameter for receiving joystick x on line 118:
`2` to `sizeof(write_buf)`
- Fixed invalid buffer size on line 123 and 126:
`data->y` to `sizeof(data->y)`
- Added comment for the `mdelay(10)` on line 169
- Changed inconsistent indentation on line 271
Kconfig:
- Fixed indentation for the help text
- Updated module name
Makefile:
- Updated module object file name
MAINTAINERS:
- Updated file name for the driver and bindings
MAINTAINERS | 7 +
drivers/input/joystick/Kconfig | 9 +
drivers/input/joystick/Makefile | 1 +
drivers/input/joystick/adafruit-seesaw.c | 273 +++++++++++++++++++++++
4 files changed, 290 insertions(+)
create mode 100644 drivers/input/joystick/adafruit-seesaw.c
diff --git a/MAINTAINERS b/MAINTAINERS
index 6c4cce45a09d..a314f9b48e21 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -441,6 +441,13 @@ W: http://wiki.analog.com/AD7879
W: https://ez.analog.com/linux-software-drivers
F: drivers/input/touchscreen/ad7879.c
+ADAFRUIT MINI I2C GAMEPAD
+M: Anshul Dalal <anshulusr@gmail.com>
+L: linux-input@vger.kernel.org
+S: Maintained
+F: Documentation/devicetree/bindings/input/adafruit,seesaw-gamepad.yaml
+F: drivers/input/joystick/adafruit-seesaw.c
+
ADDRESS SPACE LAYOUT RANDOMIZATION (ASLR)
M: Jiri Kosina <jikos@kernel.org>
S: Maintained
diff --git a/drivers/input/joystick/Kconfig b/drivers/input/joystick/Kconfig
index ac6925ce8366..df9cd1830b29 100644
--- a/drivers/input/joystick/Kconfig
+++ b/drivers/input/joystick/Kconfig
@@ -412,4 +412,13 @@ config JOYSTICK_SENSEHAT
To compile this driver as a module, choose M here: the
module will be called sensehat_joystick.
+config JOYSTICK_SEESAW
+ tristate "Adafruit Mini I2C Gamepad with Seesaw"
+ depends on I2C
+ help
+ Say Y here if you want to use the Adafruit Mini I2C Gamepad.
+
+ To compile this driver as a module, choose M here: the module will be
+ called adafruit-seesaw.
+
endif
diff --git a/drivers/input/joystick/Makefile b/drivers/input/joystick/Makefile
index 3937535f0098..9976f596a920 100644
--- a/drivers/input/joystick/Makefile
+++ b/drivers/input/joystick/Makefile
@@ -28,6 +28,7 @@ obj-$(CONFIG_JOYSTICK_N64) += n64joy.o
obj-$(CONFIG_JOYSTICK_PSXPAD_SPI) += psxpad-spi.o
obj-$(CONFIG_JOYSTICK_PXRC) += pxrc.o
obj-$(CONFIG_JOYSTICK_QWIIC) += qwiic-joystick.o
+obj-$(CONFIG_JOYSTICK_SEESAW) += adafruit-seesaw.o
obj-$(CONFIG_JOYSTICK_SENSEHAT) += sensehat-joystick.o
obj-$(CONFIG_JOYSTICK_SIDEWINDER) += sidewinder.o
obj-$(CONFIG_JOYSTICK_SPACEBALL) += spaceball.o
diff --git a/drivers/input/joystick/adafruit-seesaw.c b/drivers/input/joystick/adafruit-seesaw.c
new file mode 100644
index 000000000000..2a1eae8d2861
--- /dev/null
+++ b/drivers/input/joystick/adafruit-seesaw.c
@@ -0,0 +1,273 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (C) 2023 Anshul Dalal <anshulusr@gmail.com>
+ *
+ * Driver for Adafruit Mini I2C Gamepad
+ *
+ * Based on the work of:
+ * Oleh Kravchenko (Sparkfun Qwiic Joystick driver)
+ *
+ * Datasheet: https://cdn-learn.adafruit.com/downloads/pdf/gamepad-qt.pdf
+ * Product page: https://www.adafruit.com/product/5743
+ * Firmware and hardware sources: https://github.com/adafruit/Adafruit_Seesaw
+ */
+
+#include <asm-generic/unaligned.h>
+#include <linux/bits.h>
+#include <linux/delay.h>
+#include <linux/i2c.h>
+#include <linux/input.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+
+/* clang-format off */
+#define SEESAW_DEVICE_NAME "seesaw-gamepad"
+
+#define SEESAW_STATUS_BASE 0
+#define SEESAW_GPIO_BASE 1
+#define SEESAW_ADC_BASE 9
+
+#define SEESAW_GPIO_DIRCLR_BULK 3
+#define SEESAW_GPIO_BULK 4
+#define SEESAW_GPIO_BULK_SET 5
+#define SEESAW_GPIO_PULLENSET 11
+
+#define SEESAW_STATUS_HW_ID 1
+#define SEESAW_STATUS_SWRST 127
+
+#define SEESAW_ADC_OFFSET 7
+
+#define BUTTON_A 5
+#define BUTTON_B 1
+#define BUTTON_X 6
+#define BUTTON_Y 2
+#define BUTTON_START 16
+#define BUTTON_SELECT 0
+
+#define ANALOG_X 14
+#define ANALOG_Y 15
+
+#define SEESAW_JOYSTICK_MAX_AXIS 1023
+#define SEESAW_JOYSTICK_FUZZ 2
+#define SEESAW_JOYSTICK_FLAT 4
+
+#define SEESAW_GAMEPAD_POLL_INTERVAL 16
+#define SEESAW_GAMEPAD_POLL_MIN 8
+#define SEESAW_GAMEPAD_POLL_MAX 32
+/* clang-format on */
+
+u32 BUTTON_MASK = BIT(BUTTON_A) | BIT(BUTTON_B) | BIT(BUTTON_X) |
+ BIT(BUTTON_Y) | BIT(BUTTON_START) | BIT(BUTTON_SELECT);
+
+struct seesaw_gamepad {
+ char physical_path[32];
+ struct input_dev *input_dev;
+ struct i2c_client *i2c_client;
+};
+
+struct seesaw_data {
+ __be16 x;
+ __be16 y;
+ u8 button_a, button_b, button_x, button_y, button_start, button_select;
+};
+
+static int seesaw_read_data(struct i2c_client *client, struct seesaw_data *data)
+{
+ int err;
+ unsigned char write_buf[2] = { SEESAW_GPIO_BASE, SEESAW_GPIO_BULK };
+ unsigned char read_buf[4];
+
+ err = i2c_master_send(client, write_buf, sizeof(write_buf));
+ if (err < 0)
+ return err;
+ err = i2c_master_recv(client, read_buf, sizeof(read_buf));
+ if (err < 0)
+ return err;
+
+ u32 result = get_unaligned_be32(&read_buf);
+
+ data->button_a = !test_bit(BUTTON_A, (long *)&result);
+ data->button_b = !test_bit(BUTTON_B, (long *)&result);
+ data->button_x = !test_bit(BUTTON_X, (long *)&result);
+ data->button_y = !test_bit(BUTTON_Y, (long *)&result);
+ data->button_start = !test_bit(BUTTON_START, (long *)&result);
+ data->button_select = !test_bit(BUTTON_SELECT, (long *)&result);
+
+ write_buf[0] = SEESAW_ADC_BASE;
+ write_buf[1] = SEESAW_ADC_OFFSET + ANALOG_X;
+ err = i2c_master_send(client, write_buf, sizeof(write_buf));
+ if (err < 0)
+ return err;
+ err = i2c_master_recv(client, (char *)&data->x, sizeof(data->x));
+ if (err < 0)
+ return err;
+ /*
+ * ADC reads left as max and right as 0, must be reversed since kernel
+ * expects reports in opposite order.
+ */
+ data->x = SEESAW_JOYSTICK_MAX_AXIS - be16_to_cpu(data->x);
+
+ write_buf[1] = SEESAW_ADC_OFFSET + ANALOG_Y;
+ err = i2c_master_send(client, write_buf, sizeof(write_buf));
+ if (err < 0)
+ return err;
+ err = i2c_master_recv(client, (char *)&data->y, sizeof(data->y));
+ if (err < 0)
+ return err;
+ data->y = be16_to_cpu(data->y);
+
+ return 0;
+}
+
+static void seesaw_poll(struct input_dev *input)
+{
+ struct seesaw_gamepad *private = input_get_drvdata(input);
+ struct seesaw_data data;
+ int err;
+
+ err = seesaw_read_data(private->i2c_client, &data);
+ if (err != 0) {
+ dev_dbg(&input->dev, "failed to read joystick state: %d\n",
+ err);
+ return;
+ }
+
+ input_report_abs(input, ABS_X, data.x);
+ input_report_abs(input, ABS_Y, data.y);
+ input_report_key(input, BTN_EAST, data.button_a);
+ input_report_key(input, BTN_SOUTH, data.button_b);
+ input_report_key(input, BTN_NORTH, data.button_x);
+ input_report_key(input, BTN_WEST, data.button_y);
+ input_report_key(input, BTN_START, data.button_start);
+ input_report_key(input, BTN_SELECT, data.button_select);
+ input_sync(input);
+}
+
+static int seesaw_probe(struct i2c_client *client)
+{
+ int err;
+ struct seesaw_gamepad *private;
+ unsigned char register_reset[] = { SEESAW_STATUS_BASE,
+ SEESAW_STATUS_SWRST, 0xFF };
+ unsigned char get_hw_id[] = { SEESAW_STATUS_BASE, SEESAW_STATUS_HW_ID };
+
+ err = i2c_master_send(client, register_reset, sizeof(register_reset));
+ if (err < 0)
+ return err;
+
+ /* Wait for the registers to reset before proceeding */
+ mdelay(10);
+
+ private = devm_kzalloc(&client->dev, sizeof(*private), GFP_KERNEL);
+ if (!private)
+ return -ENOMEM;
+
+ err = i2c_master_send(client, get_hw_id, sizeof(get_hw_id));
+ if (err < 0)
+ return err;
+
+ unsigned char hardware_id;
+
+ err = i2c_master_recv(client, &hardware_id, 1);
+ if (err < 0)
+ return err;
+
+ dev_dbg(&client->dev, "Adafruit Seesaw Gamepad, Hardware ID: %02x\n",
+ hardware_id);
+
+ private->i2c_client = client;
+ scnprintf(private->physical_path, sizeof(private->physical_path),
+ "i2c/%s", dev_name(&client->dev));
+ i2c_set_clientdata(client, private);
+
+ private->input_dev = devm_input_allocate_device(&client->dev);
+ if (!private->input_dev)
+ return -ENOMEM;
+
+ private->input_dev->id.bustype = BUS_I2C;
+ private->input_dev->name = "Adafruit Seesaw Gamepad";
+ private->input_dev->phys = private->physical_path;
+ input_set_drvdata(private->input_dev, private);
+ input_set_abs_params(private->input_dev, ABS_X, 0,
+ SEESAW_JOYSTICK_MAX_AXIS, SEESAW_JOYSTICK_FUZZ,
+ SEESAW_JOYSTICK_FLAT);
+ input_set_abs_params(private->input_dev, ABS_Y, 0,
+ SEESAW_JOYSTICK_MAX_AXIS, SEESAW_JOYSTICK_FUZZ,
+ SEESAW_JOYSTICK_FLAT);
+ input_set_capability(private->input_dev, EV_KEY, BTN_EAST);
+ input_set_capability(private->input_dev, EV_KEY, BTN_SOUTH);
+ input_set_capability(private->input_dev, EV_KEY, BTN_NORTH);
+ input_set_capability(private->input_dev, EV_KEY, BTN_WEST);
+ input_set_capability(private->input_dev, EV_KEY, BTN_START);
+ input_set_capability(private->input_dev, EV_KEY, BTN_SELECT);
+
+ err = input_setup_polling(private->input_dev, seesaw_poll);
+ if (err) {
+ dev_err(&client->dev, "failed to set up polling: %d\n", err);
+ return err;
+ }
+
+ input_set_poll_interval(private->input_dev,
+ SEESAW_GAMEPAD_POLL_INTERVAL);
+ input_set_max_poll_interval(private->input_dev,
+ SEESAW_GAMEPAD_POLL_MAX);
+ input_set_min_poll_interval(private->input_dev,
+ SEESAW_GAMEPAD_POLL_MIN);
+
+ err = input_register_device(private->input_dev);
+ if (err) {
+ dev_err(&client->dev, "failed to register joystick: %d\n", err);
+ return err;
+ }
+
+ /* Set Pin Mode to input and enable pull-up resistors */
+ unsigned char pin_mode[] = { SEESAW_GPIO_BASE, SEESAW_GPIO_DIRCLR_BULK,
+ BUTTON_MASK >> 24, BUTTON_MASK >> 16,
+ BUTTON_MASK >> 8, BUTTON_MASK };
+ err = i2c_master_send(client, pin_mode, sizeof(pin_mode));
+ if (err < 0)
+ return err;
+ pin_mode[1] = SEESAW_GPIO_PULLENSET;
+ err = i2c_master_send(client, pin_mode, sizeof(pin_mode));
+ if (err < 0)
+ return err;
+ pin_mode[1] = SEESAW_GPIO_BULK_SET;
+ err = i2c_master_send(client, pin_mode, sizeof(pin_mode));
+ if (err < 0)
+ return err;
+
+ return 0;
+}
+
+#ifdef CONFIG_OF
+static const struct of_device_id of_seesaw_match[] = {
+ {
+ .compatible = "adafruit,seesaw-gamepad",
+ },
+ { /* Sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, of_seesaw_match);
+#endif /* CONFIG_OF */
+
+/* clang-format off */
+static const struct i2c_device_id seesaw_id_table[] = {
+ { SEESAW_DEVICE_NAME, 0 },
+ { /* Sentinel */ }
+};
+/* clang-format on */
+
+MODULE_DEVICE_TABLE(i2c, seesaw_id_table);
+
+static struct i2c_driver seesaw_driver = {
+ .driver = {
+ .name = SEESAW_DEVICE_NAME,
+ .of_match_table = of_match_ptr(of_seesaw_match),
+ },
+ .id_table = seesaw_id_table,
+ .probe = seesaw_probe,
+};
+module_i2c_driver(seesaw_driver);
+
+MODULE_AUTHOR("Anshul Dalal <anshulusr@gmail.com>");
+MODULE_DESCRIPTION("Adafruit Mini I2C Gamepad driver");
+MODULE_LICENSE("GPL");
--
2.42.0
^ permalink raw reply related
* [PATCH v2] Input: bcm5974 - check endpoint type before starting traffic
From: Javier Carrasco @ 2023-10-12 16:51 UTC (permalink / raw)
To: John Horan, Henrik Rydberg, Dmitry Torokhov
Cc: linux-input, linux-kernel, Javier Carrasco,
syzbot+348331f63b034f89b622
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://syzkaller.appspot.com/bug?extid=348331f63b034f89b622
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 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://lore.kernel.org/r/20231007-topic-bcm5974_bulk-v1-1-355be9f8ad80@gmail.com
---
drivers/input/mouse/bcm5974.c | 27 +++++++++++++++++++++++++++
1 file changed, 27 insertions(+)
diff --git a/drivers/input/mouse/bcm5974.c b/drivers/input/mouse/bcm5974.c
index ca150618d32f..9fc9dd96c96a 100644
--- a/drivers/input/mouse/bcm5974.c
+++ b/drivers/input/mouse/bcm5974.c
@@ -891,6 +891,21 @@ static int bcm5974_resume(struct usb_interface *iface)
return error;
}
+static bool bcm5974_ep_is_int_in(struct usb_host_interface *iface, int addr)
+{
+ struct usb_endpoint_descriptor *endpoint;
+ int i;
+
+ for (i = 0; i < iface->desc.bNumEndpoints; i++) {
+ endpoint = &iface->endpoint[i].desc;
+ if (endpoint->bEndpointAddress == addr) {
+ if (usb_endpoint_is_int_in(endpoint))
+ return true;
+ }
+ }
+ return false;
+}
+
static int bcm5974_probe(struct usb_interface *iface,
const struct usb_device_id *id)
{
@@ -903,6 +918,18 @@ static int bcm5974_probe(struct usb_interface *iface,
/* find the product index */
cfg = bcm5974_get_config(udev);
+ if (cfg->tp_type == TYPE1) {
+ if (!bcm5974_ep_is_int_in(iface->cur_altsetting, cfg->bt_ep)) {
+ dev_err(&iface->dev, "No int-in endpoint for the button\n");
+ return -ENODEV;
+ }
+ }
+
+ if (!bcm5974_ep_is_int_in(iface->cur_altsetting, cfg->tp_ep)) {
+ dev_err(&iface->dev, "No int-in endpoint for the trackpad\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,
--
Javier Carrasco <javier.carrasco.cruz@gmail.com>
^ permalink raw reply related
* Re: [PATCH v2] Input: psmouse - fix fast_reconnect function for PS/2 mode
From: Jeffery Miller @ 2023-10-12 22:33 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: linux-kernel, linux-input, Andrew Duggan, Andrew Duggan,
loic.poulain, linux, benjamin.tissoires
In-Reply-To: <20231005002249.554877-1-jefferymiller@google.com>
On Wed, Oct 4, 2023 at 7:23 PM Jeffery Miller <jefferymiller@google.com> wrote:
>
> drivers/input/mouse/elantech.c | 1 +
> drivers/input/mouse/synaptics.c | 1 +
> 2 files changed, 2 insertions(+)
>
> diff --git a/drivers/input/mouse/elantech.c b/drivers/input/mouse/elantech.c
> index 2118b2075f43..4e38229404b4 100644
> --- a/drivers/input/mouse/elantech.c
> +++ b/drivers/input/mouse/elantech.c
> @@ -2114,6 +2114,7 @@ static int elantech_setup_ps2(struct psmouse *psmouse,
> psmouse->protocol_handler = elantech_process_byte;
> psmouse->disconnect = elantech_disconnect;
> psmouse->reconnect = elantech_reconnect;
> + psmouse->fast_reconnect = NULL;
> psmouse->pktsize = info->hw_version > 1 ? 6 : 4;
>
> return 0;
> diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c
> index ada299ec5bba..cefc74b3b34b 100644
> --- a/drivers/input/mouse/synaptics.c
> +++ b/drivers/input/mouse/synaptics.c
> @@ -1623,6 +1623,7 @@ static int synaptics_init_ps2(struct psmouse *psmouse,
> psmouse->set_rate = synaptics_set_rate;
> psmouse->disconnect = synaptics_disconnect;
> psmouse->reconnect = synaptics_reconnect;
> + psmouse->fast_reconnect = NULL;
> psmouse->cleanup = synaptics_reset;
> /* Synaptics can usually stay in sync without extra help */
> psmouse->resync_time = 0;
> --
> 2.42.0.582.g8ccd20d70d-goog
>
This fast_reconnect function pointer being left over has been here since commit
8eb92e5c9133 ("Input: psmouse - add support for SMBus companions")
It is only recently noticed due to 92e24e0e57f7 ("Input: psmouse - add
delay when deactivating for SMBus mode") which is in the v6.6 rc
branches.
Shouldn't it be OK to merge regardless of a future refactor of
8eb92e5c9133 ("Input: psmouse - add support for SMBus companions")
as described in https://lore.kernel.org/all/ZR1yUFJ8a9Zt606N@penguin/?
This is a v2 from the previous submission at
https://lore.kernel.org/all/20231004005729.3943515-1-jefferymiller@google.com/
Thanks,
Jeff
^ permalink raw reply
* Re: [PATCH] Input: elantech - fix fast_reconnect callback in ps2 mode
From: Dmitry Torokhov @ 2023-10-12 22:53 UTC (permalink / raw)
To: Thorsten Leemhuis
Cc: Jeffery Miller, regressions, benjamin.tissoires, Andrew Duggan,
Andrew Duggan, loic.poulain, Greg Kroah-Hartman, linux-input,
linux-kernel
In-Reply-To: <25ac6b17-e3fa-4e98-95a6-eac12bdbcdd2@leemhuis.info>
Hi Thorsten,
On Tue, Oct 10, 2023 at 09:08:23AM +0200, Thorsten Leemhuis wrote:
> On 05.10.23 02:13, Jeffery Miller wrote:
> >
> > On Wed, Oct 4, 2023 at 9:11 AM Dmitry Torokhov
> > <dmitry.torokhov@gmail.com> wrote:
> >>
> >> In fact, now that I think about it more, we should rework the original
> >> patch that added the delay, so that we do not wait these 30 msec in the
> >> "fast" reconnect handler. It turns out your original approach was
> >> better, but we should not be using retries, but rather the existing
> >> reset_delay_ms already defined in rmi platform data. I would appreciate
> >> if you try the draft patch at the end of this email (to be applied after
> >> reverting your original one adding the delay in psmouse-smbus.c).
> > I tested the draft patch and it works. I did revert the previous delay
> > patch while testing it.
> >
> >> I think we need a similar change in synaptics.c as that one also can
> >> fall back to PS/2 mode.
> >>
> > Ah, good point, yes it does appear this needs to be done as well.
> > I have tested and will post an new version of the patch to include
> > the fix in synaptics.c as well.
>
> As I'm affected by this problem (and somebody else reported to me in
> private to be affected as well) and nothing afaics happened in the past
> few days a quick question:
>
> What's the way forward here now that -rc6 slowly comes into sight? Apply
> Jeff's patch to fix my problem? Revert the culprit and fix this properly
> up with Dmitry's and Jeff's patches in the next cycle? Something else?
I will revert the original patch introducing the delay now that we argee
there is a better way. In the mean time I will merge Jeff's patch to
reset fast_reconnect handlers, as it is right thing to do anyway, and
will get into shape the patch setting reset-delay in RMI code.
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH V2 1/2] dt-bindings: input: Introduce Himax HID-over-SPI device
From: yang tylor @ 2023-10-13 2:15 UTC (permalink / raw)
To: Conor Dooley
Cc: Conor Dooley, dmitry.torokhov, robh+dt, krzysztof.kozlowski+dt,
conor+dt, jikos, benjamin.tissoires, linux-input, devicetree,
linux-kernel, poyuan_chang, hbarnor, jingyliang@chromium.org,
wuxy23, luolm1, hung poyu
In-Reply-To: <20231012-pope-denatured-c1898bc1e44b@spud>
On Thu, Oct 12, 2023 at 11:24 PM Conor Dooley <conor@kernel.org> wrote:
>
> On Thu, Oct 12, 2023 at 10:30:03AM +0800, yang tylor wrote:
> > On Tue, Oct 10, 2023 at 1:52 AM Conor Dooley <conor@kernel.org> wrote:
> > >
> > > On Mon, Oct 02, 2023 at 06:44:41PM +0800, yang tylor wrote:
> > > > On Fri, Sep 29, 2023 at 12:56 AM Conor Dooley <conor@kernel.org> wrote:
> > > > >
> > > > > On Thu, Sep 28, 2023 at 10:12:41AM +0800, yang tylor wrote:
> > > > > > On Tue, Sep 26, 2023 at 8:53 PM Conor Dooley <conor@kernel.org> wrote:
> > > > > > > On Tue, Sep 26, 2023 at 05:52:39PM +0800, yang tylor wrote:
> > > > > > > > On Tue, Sep 26, 2023 at 5:02 PM Conor Dooley <conor@kernel.org> wrote:
> > > > > > > > > On Mon, Sep 25, 2023 at 06:16:29PM +0800, yang tylor wrote:
> > > > > > > > > > On Mon, Sep 25, 2023 at 4:41 PM Conor Dooley <conor.dooley@microchip.com> wrote:
> > > > > > > > > > We have a default prefix firmware name(like himax_xxxx.bin) in the driver code.
> > > > > > > > >
> > > > > > > > > How do you intend generating the name of the firmware file? I assume the
> > > > > > > > > same firmware doesn't work on every IC, so you'll need to pick a
> > > > > > > > > different one depending on the compatible?
> > > > > > > > >
> > > > > > > > If considering a firmware library line-up for all the incoming panels
> > > > > > > > of this driver.
> > > > > > > > We would use PID as part of the file name. Because all the support panels would
> > > > > > > > have a unique PID associated. Which will make the firmware name like
> > > > > > > > himax_xxx_{$PID}.bin. The problem is, we need to know PID before firmware load
> > > > > > > > at no flash condition. Thus PID information is required in dts when
> > > > > > > > no-flash-flag
> > > > > > > > is specified.
> > > > > > >
> > > > > > > Firstly, where does the "xxx" come from?
> > > > > > > And you're making it sound more like having firmware-name is suitable
> > > > > > > for this use case, given you need to determine the name of the file to
> > > > > > > use based on something that is hardware specific but is not
> > > > > > > dynamically detectable.
> > > > > > Current driver patch uses a prefix name "himax_i2chid" which comes
> > > > > > from the previous project
> > > > > > and seems not suitable for this condition, so I use "xxx" and plan to
> > > > > > replace it in the next version.
> > > > > > For finding firmware, I think both solutions are reasonable.
> > > > > > - provide firmware name directly: implies no-flash and use user
> > > > > > specified firmware, no PID info.
> > > > > > - provide no-flash-flag and PID info: loading firmware from organized
> > > > > > names with PID info.
> > > > > > I prefer the 2nd solution, but it needs more properties in dts. 1st
> > > > > > has less properties and more
> > > > > > intuitive.
> > > > > >
> > > > > > I don't know which one is more acceptable by the community, as you
> > > > > > know I'm a newbie here.
> > > > >
> > > > > To be honest, I am not all that sure either! Does the panel id have
> > > > > value in its own right, or is that only used to determine the firmware
> > > > > filename?
> > > > Currently, PID stands for Panel/Project ID and is used for determining
> > > > the firmware filename only. We haven't come up with any new attribute that
> > > > may attach to it. The differences between panels are handled in firmware
> > > > dedicated to its PID.
> > > >
> > > > > Also, if it does have value in its own right, rather than a "pid",
> > > > > should the panel be a child node of this hid device with its own
> > > > > compatible?
> > > > It may need a child node if we find it necessary to add attributes to each PID.
> > > > But currently we have no idea about it.
> > >
> > > To be honest, it seems to me like you are using "PID" in place of a
> > > compatible for the panel, since it needs to be provided via DT anyway.
> >
> > Hmm... So the more formal way is?
> > If I add a sub-note inside this spi-device block, such as "panel" and
> > add PID inside.
> > Will it be more appropriate?
> > ...
> > spi {
> > ...
> > hx_spi@0 {
> > ...
> > panel {
> > himax,pid = ...
>
> And this now looks exactly like compatible = "vendor,part" now, no?
I think it's not the same, I thought "compatible" is used to target
from the driver side.
For finding other information inside the block. But I just store PID
information in this
one, not used for targeting but getting infos from it.
Thanks,
Tylor
^ permalink raw reply
* Re: [PATCH v4 4/5] clk: twl: add clock driver for TWL6032
From: Andreas Kemnade @ 2023-10-13 6:08 UTC (permalink / raw)
To: dmitry.torokhov, robh+dt, krzysztof.kozlowski+dt, conor+dt, lee,
bcousson, tony, mturquette, sboyd, andreas, linux-input,
devicetree, linux-kernel, linux-omap, linux-clk
In-Reply-To: <20230916100515.1650336-5-andreas@kemnade.info>
On Sat, 16 Sep 2023 12:05:14 +0200
Andreas Kemnade <andreas@kemnade.info> wrote:
> The TWL6032 has some clock outputs which are controlled like
> fixed-voltage regulators, in some drivers for these chips
> found in the wild, just the regulator api is abused for controlling
> them, so simply use something similar to the regulator functions.
> Due to a lack of hardware available for testing, leave out the
> TWL6030-specific part of those functions.
>
> Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
> ---
ping...
anything left to do here?
Regards,
Andreas
^ permalink raw reply
* Re: [PATCH v3 04/16] platform/x86/amd/pmf: Add support for PMF Policy Binary
From: Shyam Sundar S K @ 2023-10-13 6:13 UTC (permalink / raw)
To: Mario Limonciello, hdegoede, markgross, ilpo.jarvinen,
basavaraj.natikar, jikos, benjamin.tissoires, alexander.deucher,
christian.koenig, Xinhui.Pan, airlied, daniel
Cc: Patil.Reddy, platform-driver-x86, linux-input, amd-gfx, dri-devel
In-Reply-To: <55a1230c-e9ef-421e-930c-13f5016546b4@amd.com>
On 10/11/2023 8:47 AM, Shyam Sundar S K wrote:
>
>
> On 10/10/2023 9:56 PM, Mario Limonciello wrote:
>> On 10/10/2023 07:59, Shyam Sundar S K wrote:
>>> PMF Policy binary is a encrypted and signed binary that will be part
>>> of the BIOS. PMF driver via the ACPI interface checks the existence
>>> of Smart PC bit. If the advertised bit is found, PMF driver walks
>>> the acpi namespace to find out the policy binary size and the address
>>> which has to be passed to the TA during the TA init sequence.
>>>
>>> The policy binary is comprised of inputs (or the events) and outputs
>>> (or the actions). With the PMF ecosystem, OEMs generate the policy
>>> binary (or could be multiple binaries) that contains a supported set
>>> of inputs and outputs which could be specifically carved out for each
>>> usage segment (or for each user also) that could influence the system
>>> behavior either by enriching the user experience or/and boost/throttle
>>> power limits.
>>>
>>> Once the TA init command succeeds, the PMF driver sends the changing
>>> events in the current environment to the TA for a constant sampling
>>> frequency time (the event here could be a lid close or open) and
>>> if the policy binary has corresponding action built within it, the
>>> TA sends the action for it in the subsequent enact command.
>>>
>>> If the inputs sent to the TA has no output defined in the policy
>>> binary generated by OEMs, there will be no action to be performed
>>> by the PMF driver.
>>>
>>> Example policies:
>>>
>>> 1) if slider is performance ; set the SPL to 40W
>>> Here PMF driver registers with the platform profile interface and
>>> when the slider position is changed, PMF driver lets the TA know
>>> about this. TA sends back an action to update the Sustained
>>> Power Limit (SPL). PMF driver updates this limit via the PMFW mailbox.
>>>
>>> 2) if user_away ; then lock the system
>>> Here PMF driver hooks to the AMD SFH driver to know the user presence
>>> and send the inputs to TA and if the condition is met, the TA sends
>>> the action of locking the system. PMF driver generates a uevent and
>>> based on the udev rule in the userland the system gets locked with
>>> systemctl.
>>>
>>> The intent here is to provide the OEM's to make a policy to lock the
>>> system when the user is away ; but the userland can make a choice to
>>> ignore it.
>>>
>>> and so on.
>>>
>>> The OEMs will have an utility to create numerous such policies and
>>> the policies shall be reviewed by AMD before signing and encrypting
>>> them. Policies are shared between operating systems to have seemless
>>> user
>>> experience.
>>>
>>> Since all this action has to happen via the "amdtee" driver, currently
>>> there is no caller for it in the kernel which can load the amdtee
>>> driver.
>>> Without amdtee driver loading onto the system the "tee" calls shall
>>> fail
>>> from the PMF driver. Hence an explicit "request_module" has been added
>>> to address this.
>>>
>>> Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
>>> ---
>>> drivers/platform/x86/amd/pmf/Kconfig | 2 +-
>>> drivers/platform/x86/amd/pmf/acpi.c | 37 +++++++
>>> drivers/platform/x86/amd/pmf/core.c | 13 +++
>>> drivers/platform/x86/amd/pmf/pmf.h | 136 ++++++++++++++++++++++++
>>> drivers/platform/x86/amd/pmf/tee-if.c | 146
>>> +++++++++++++++++++++++++-
>>> 5 files changed, 331 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/platform/x86/amd/pmf/Kconfig
>>> b/drivers/platform/x86/amd/pmf/Kconfig
>>> index 32a029e8db80..f246252bddd8 100644
>>> --- a/drivers/platform/x86/amd/pmf/Kconfig
>>> +++ b/drivers/platform/x86/amd/pmf/Kconfig
>>> @@ -9,7 +9,7 @@ config AMD_PMF
>>> depends on POWER_SUPPLY
>>> depends on AMD_NB
>>> select ACPI_PLATFORM_PROFILE
>>> - depends on TEE
>>> + depends on TEE && AMDTEE
>>> help
>>> This driver provides support for the AMD Platform Management
>>> Framework.
>>> The goal is to enhance end user experience by making AMD PCs
>>> smarter,
>>> diff --git a/drivers/platform/x86/amd/pmf/acpi.c
>>> b/drivers/platform/x86/amd/pmf/acpi.c
>>> index 3fc5e4547d9f..d0512af2cd42 100644
>>> --- a/drivers/platform/x86/amd/pmf/acpi.c
>>> +++ b/drivers/platform/x86/amd/pmf/acpi.c
>>> @@ -286,6 +286,43 @@ int apmf_install_handler(struct amd_pmf_dev
>>> *pmf_dev)
>>> return 0;
>>> }
>>> +static acpi_status apmf_walk_resources(struct acpi_resource *res,
>>> void *data)
>>> +{
>>> + struct amd_pmf_dev *dev = data;
>>> +
>>> + switch (res->type) {
>>> + case ACPI_RESOURCE_TYPE_ADDRESS64:
>>> + dev->policy_addr = res->data.address64.address.minimum;
>>> + dev->policy_sz = res->data.address64.address.address_length;
>>> + break;
>>> + case ACPI_RESOURCE_TYPE_FIXED_MEMORY32:
>>> + dev->policy_addr = res->data.fixed_memory32.address;
>>> + dev->policy_sz = res->data.fixed_memory32.address_length;
>>> + break;
>>> + }
>>> +
>>> + if (!dev->policy_addr || dev->policy_sz > POLICY_BUF_MAX_SZ ||
>>> dev->policy_sz == 0) {
>>> + pr_err("Incorrect Policy params, possibly a SBIOS bug\n");
>>> + return AE_ERROR;
>>> + }
>>> +
>>> + return AE_OK;
>>> +}
>>> +
>>> +int apmf_check_smart_pc(struct amd_pmf_dev *pmf_dev)
>>> +{
>>> + acpi_handle ahandle = ACPI_HANDLE(pmf_dev->dev);
>>> + acpi_status status;
>>> +
>>> + status = acpi_walk_resources(ahandle, METHOD_NAME__CRS,
>>> apmf_walk_resources, pmf_dev);
>>> + if (ACPI_FAILURE(status)) {
>>> + dev_err(pmf_dev->dev, "acpi_walk_resources failed\n");
>>> + return status;
>>
>> You're returning acpi_status here, but the return for the function is
>> int. It "happens to work" but I think it would be better to do
>> something like:
>>
>> dev_err(pmf_dev->dev, "acpi_walk_resources failed: %d\n, status);
>> return -EINVAL;
>
> OK, I will change this.
>
>>
>>> + }
>>> +
>>> + return 0;
>>> +}
>>> +
>>> void apmf_acpi_deinit(struct amd_pmf_dev *pmf_dev)
>>> {
>>> acpi_handle ahandle = ACPI_HANDLE(pmf_dev->dev);
>>> diff --git a/drivers/platform/x86/amd/pmf/core.c
>>> b/drivers/platform/x86/amd/pmf/core.c
>>> index ffb78e9709d9..96a41e7d4e7d 100644
>>> --- a/drivers/platform/x86/amd/pmf/core.c
>>> +++ b/drivers/platform/x86/amd/pmf/core.c
>>> @@ -395,6 +395,19 @@ static int amd_pmf_probe(struct platform_device
>>> *pdev)
>>> return -ENOMEM;
>>> dev->dev = &pdev->dev;
>>> + err = apmf_check_smart_pc(dev);
>>> + if (!err) {
>>
>> Rather than just failing to init smart PC solution builder, shouldn't
>> you fail probe entirely if an err is set from probing the BIOS
>> resources? This seems fairly fatal.
>>
>> For example I'd think that setting up static slider is relatively
>> pointless on a system intending to use smart PC solution builder if
>> smart PC solution builder isn't working.
>>
>
> What if - the BIOS advertises the smart PC bit and forgets to add
> resources (or a buggy ACPI thing)?
>
> Atleast that way if the static slider is enabled, atleast some amount
> of power saving can happen.
>
> Note that, I have tried the fallback to static slider if the smart pc
> fails and that's working with the current we have in place.
>
>
>>> + /*
>>> + * In order for Smart PC solution to work it has a hard
>>> dependency
>>> + * on the amdtee driver to be loaded first even before the
>>> PMF driver
>>> + * loads. PMF ASL has a _CRS method that advertises the
>>> existence
>>> + * of Smart PC bit. If this information is present, use
>>> this to
>>> + * explicitly probe the amdtee driver, so that "tee"
>>> plumbing is done
>>> + * before the PMF Smart PC init happens.
>>> + */
>>> + if (request_module("amdtee"))
>>> + pr_err("Failed to load amdtee. PMF Smart PC not
>>> enabled!\n");
>>
>> Did that softdep thing Ilpo mentioned not work for modprobe?
>> Hopefully that generally works for everything except the insmod case
>> so this code path is unlikely to be hit in the wild.
>>
>
> unfortunately no. Hence that's the reason I had to retain back the
> request_module() here.
Apologies for creating confusion here. The system which I was using
had some modules blacklisted and that lead to softdep not working.
I tried on couple of other systems and softdep is working. I will
remove request_module() and change it to softdep in v4.
Thanks,
Shyam
>
>>> + }
>>> rdev = pci_get_domain_bus_and_slot(0, 0, PCI_DEVFN(0, 0));
>>> if (!rdev || !pci_match_id(pmf_pci_ids, rdev)) {
>>> diff --git a/drivers/platform/x86/amd/pmf/pmf.h
>>> b/drivers/platform/x86/amd/pmf/pmf.h
>>> index a91c22d9b532..51c0e17f7720 100644
>>> --- a/drivers/platform/x86/amd/pmf/pmf.h
>>> +++ b/drivers/platform/x86/amd/pmf/pmf.h
>>> @@ -14,6 +14,11 @@
>>> #include <linux/acpi.h>
>>> #include <linux/platform_profile.h>
>>> +#define POLICY_BUF_MAX_SZ 0x4b000
>>> +#define POLICY_SIGN_COOKIE 0x31535024
>>> +#define POLICY_COOKIE_OFFSET 0x10
>>> +#define POLICY_COOKIE_LEN 0x14
>>> +
>>> /* APMF Functions */
>>> #define APMF_FUNC_VERIFY_INTERFACE 0
>>> #define APMF_FUNC_GET_SYS_PARAMS 1
>>> @@ -59,8 +64,21 @@
>>> #define ARG_NONE 0
>>> #define AVG_SAMPLE_SIZE 3
>>> +/* Policy Actions */
>>> +#define PMF_POLICY_SPL 2
>>> +#define PMF_POLICY_SPPT 3
>>> +#define PMF_POLICY_FPPT 4
>>> +#define PMF_POLICY_SPPT_APU_ONLY 5
>>> +#define PMF_POLICY_STT_MIN 6
>>> +#define PMF_POLICY_STT_SKINTEMP_APU 7
>>> +#define PMF_POLICY_STT_SKINTEMP_HS2 8
>>> +
>>> /* TA macros */
>>> #define PMF_TA_IF_VERSION_MAJOR 1
>>> +#define TA_PMF_ACTION_MAX 32
>>> +#define TA_PMF_UNDO_MAX 8
>>> +#define TA_OUTPUT_RESERVED_MEM 906
>>> +#define MAX_OPERATION_PARAMS 4
>>> /* AMD PMF BIOS interfaces */
>>> struct apmf_verify_interface {
>>> @@ -183,11 +201,16 @@ struct amd_pmf_dev {
>>> bool cnqf_supported;
>>> struct notifier_block pwr_src_notifier;
>>> /* Smart PC solution builder */
>>> + unsigned char *policy_buf;
>>> + u32 policy_sz;
>>> struct tee_context *tee_ctx;
>>> struct tee_shm *fw_shm_pool;
>>> u32 session_id;
>>> void *shbuf;
>>> struct delayed_work pb_work;
>>> + struct pmf_action_table *prev_data;
>>> + u64 policy_addr;
>>> + void *policy_base;
>>> bool smart_pc_enabled;
>>> };
>>> @@ -399,17 +422,129 @@ struct apmf_dyn_slider_output {
>>> struct apmf_cnqf_power_set ps[APMF_CNQF_MAX];
>>> } __packed;
>>> +/* Smart PC - TA internals */
>>> +enum ta_slider {
>>> + TA_BEST_BATTERY, /* Best Battery */
>>> + TA_BETTER_BATTERY, /* Better Battery */
>>> + TA_BETTER_PERFORMANCE, /* Better Performance */
>>> + TA_BEST_PERFORMANCE, /* Best Performance */
>>> + TA_MAX,
>>> +};
>>
>> The comments above at end of the line don't add any value.
>
> OK.
>
> Thanks,
> Shyam
>
>>
>>> +
>>> /* cmd ids for TA communication */
>>> enum ta_pmf_command {
>>> TA_PMF_COMMAND_POLICY_BUILDER_INITIALIZE,
>>> TA_PMF_COMMAND_POLICY_BUILDER_ENACT_POLICIES,
>>> };
>>> +enum ta_pmf_error_type {
>>> + TA_PMF_TYPE_SUCCESS,
>>> + TA_PMF_ERROR_TYPE_GENERIC,
>>> + TA_PMF_ERROR_TYPE_CRYPTO,
>>> + TA_PMF_ERROR_TYPE_CRYPTO_VALIDATE,
>>> + TA_PMF_ERROR_TYPE_CRYPTO_VERIFY_OEM,
>>> + TA_PMF_ERROR_TYPE_POLICY_BUILDER,
>>> + TA_PMF_ERROR_TYPE_PB_CONVERT,
>>> + TA_PMF_ERROR_TYPE_PB_SETUP,
>>> + TA_PMF_ERROR_TYPE_PB_ENACT,
>>> + TA_PMF_ERROR_TYPE_ASD_GET_DEVICE_INFO,
>>> + TA_PMF_ERROR_TYPE_ASD_GET_DEVICE_PCIE_INFO,
>>> + TA_PMF_ERROR_TYPE_SYS_DRV_FW_VALIDATION,
>>> + TA_PMF_ERROR_TYPE_MAX,
>>> +};
>>> +
>>> +struct pmf_action_table {
>>> + u32 spl; /* in mW */
>>> + u32 sppt; /* in mW */
>>> + u32 sppt_apuonly; /* in mW */
>>> + u32 fppt; /* in mW */
>>> + u32 stt_minlimit; /* in mW */
>>> + u32 stt_skintemp_apu; /* in C */
>>> + u32 stt_skintemp_hs2; /* in C */
>>> +};
>>> +
>>> +/* Input conditions */
>>> +struct ta_pmf_condition_info {
>>> + u32 power_source;
>>> + u32 bat_percentage;
>>> + u32 power_slider;
>>> + u32 lid_state;
>>> + bool user_present;
>>> + u32 rsvd1[2];
>>> + u32 monitor_count;
>>> + u32 rsvd2[2];
>>> + u32 bat_design;
>>> + u32 full_charge_capacity;
>>> + int drain_rate;
>>> + bool user_engaged;
>>> + u32 device_state;
>>> + u32 socket_power;
>>> + u32 skin_temperature;
>>> + u32 rsvd3[5];
>>> + u32 ambient_light;
>>> + u32 length;
>>> + u32 avg_c0residency;
>>> + u32 max_c0residency;
>>> + u32 s0i3_entry;
>>> + u32 gfx_busy;
>>> + u32 rsvd4[7];
>>> + bool camera_state;
>>> + u32 workload_type;
>>> + u32 display_type;
>>> + u32 display_state;
>>> + u32 rsvd5[150];
>>> +};
>>> +
>>> +struct ta_pmf_load_policy_table {
>>> + u32 table_size;
>>> + u8 table[POLICY_BUF_MAX_SZ];
>>> +};
>>> +
>>> +/* TA initialization params */
>>> +struct ta_pmf_init_table {
>>> + u32 frequency; /* SMU sampling frequency */
>>> + bool validate;
>>> + bool sku_check;
>>> + bool metadata_macrocheck;
>>> + struct ta_pmf_load_policy_table policies_table;
>>> +};
>>> +
>>> +/* Everything the TA needs to Enact Policies */
>>> +struct ta_pmf_enact_table {
>>> + struct ta_pmf_condition_info ev_info;
>>> + u32 name;
>>> +};
>>> +
>>> +struct ta_pmf_action {
>>> + u32 action_index;
>>> + u32 value;
>>> +};
>>> +
>>> +/* output actions from TA */
>>> +struct ta_pmf_enact_result {
>>> + u32 actions_count;
>>> + struct ta_pmf_action actions_list[TA_PMF_ACTION_MAX];
>>> + u32 undo_count;
>>> + struct ta_pmf_action undo_list[TA_PMF_UNDO_MAX];
>>> +};
>>> +
>>> +union ta_pmf_input {
>>> + struct ta_pmf_enact_table enact_table;
>>> + struct ta_pmf_init_table init_table;
>>> +};
>>> +
>>> +union ta_pmf_output {
>>> + struct ta_pmf_enact_result policy_apply_table;
>>> + u32 rsvd[TA_OUTPUT_RESERVED_MEM];
>>> +};
>>> +
>>> struct ta_pmf_shared_memory {
>>> int command_id;
>>> int resp_id;
>>> u32 pmf_result;
>>> u32 if_version;
>>> + union ta_pmf_output pmf_output;
>>> + union ta_pmf_input pmf_input;
>>> };
>>> /* Core Layer */
>>> @@ -460,4 +595,5 @@ extern const struct attribute_group
>>> cnqf_feature_attribute_group;
>>> /* Smart PC builder Layer */
>>> int amd_pmf_init_smart_pc(struct amd_pmf_dev *dev);
>>> void amd_pmf_deinit_smart_pc(struct amd_pmf_dev *dev);
>>> +int apmf_check_smart_pc(struct amd_pmf_dev *pmf_dev);
>>> #endif /* PMF_H */
>>> diff --git a/drivers/platform/x86/amd/pmf/tee-if.c
>>> b/drivers/platform/x86/amd/pmf/tee-if.c
>>> index 4036f435f1e2..38f02676261d 100644
>>> --- a/drivers/platform/x86/amd/pmf/tee-if.c
>>> +++ b/drivers/platform/x86/amd/pmf/tee-if.c
>>> @@ -42,9 +42,77 @@ static void amd_pmf_prepare_args(struct
>>> amd_pmf_dev *dev, int cmd,
>>> param[0].u.memref.shm_offs = 0;
>>> }
>>> +static void amd_pmf_apply_policies(struct amd_pmf_dev *dev,
>>> struct ta_pmf_enact_result *out)
>>> +{
>>> + u32 val;
>>> + int idx;
>>> +
>>> + for (idx = 0; idx < out->actions_count; idx++) {
>>> + val = out->actions_list[idx].value;
>>> + switch (out->actions_list[idx].action_index) {
>>> + case PMF_POLICY_SPL:
>>> + if (dev->prev_data->spl != val) {
>>> + amd_pmf_send_cmd(dev, SET_SPL, false, val, NULL);
>>> + dev_dbg(dev->dev, "update SPL : %u\n", val);
>>> + dev->prev_data->spl = val;
>>> + }
>>> + break;
>>> +
>>> + case PMF_POLICY_SPPT:
>>> + if (dev->prev_data->sppt != val) {
>>> + amd_pmf_send_cmd(dev, SET_SPPT, false, val, NULL);
>>> + dev_dbg(dev->dev, "update SPPT : %u\n", val);
>>> + dev->prev_data->sppt = val;
>>> + }
>>> + break;
>>> +
>>> + case PMF_POLICY_FPPT:
>>> + if (dev->prev_data->fppt != val) {
>>> + amd_pmf_send_cmd(dev, SET_FPPT, false, val, NULL);
>>> + dev_dbg(dev->dev, "update FPPT : %u\n", val);
>>> + dev->prev_data->fppt = val;
>>> + }
>>> + break;
>>> +
>>> + case PMF_POLICY_SPPT_APU_ONLY:
>>> + if (dev->prev_data->sppt_apuonly != val) {
>>> + amd_pmf_send_cmd(dev, SET_SPPT_APU_ONLY, false,
>>> val, NULL);
>>> + dev_dbg(dev->dev, "update SPPT_APU_ONLY : %u\n", val);
>>> + dev->prev_data->sppt_apuonly = val;
>>> + }
>>> + break;
>>> +
>>> + case PMF_POLICY_STT_MIN:
>>> + if (dev->prev_data->stt_minlimit != val) {
>>> + amd_pmf_send_cmd(dev, SET_STT_MIN_LIMIT, false,
>>> val, NULL);
>>> + dev_dbg(dev->dev, "update STT_MIN : %u\n", val);
>>> + dev->prev_data->stt_minlimit = val;
>>> + }
>>> + break;
>>> +
>>> + case PMF_POLICY_STT_SKINTEMP_APU:
>>> + if (dev->prev_data->stt_skintemp_apu != val) {
>>> + amd_pmf_send_cmd(dev, SET_STT_LIMIT_APU, false,
>>> val, NULL);
>>> + dev_dbg(dev->dev, "update STT_SKINTEMP_APU : %u\n",
>>> val);
>>> + dev->prev_data->stt_skintemp_apu = val;
>>> + }
>>> + break;
>>> +
>>> + case PMF_POLICY_STT_SKINTEMP_HS2:
>>> + if (dev->prev_data->stt_skintemp_hs2 != val) {
>>> + amd_pmf_send_cmd(dev, SET_STT_LIMIT_HS2, false,
>>> val, NULL);
>>> + dev_dbg(dev->dev, "update STT_SKINTEMP_HS2 : %u\n",
>>> val);
>>> + dev->prev_data->stt_skintemp_hs2 = val;
>>> + }
>>> + break;
>>> + }
>>> + }
>>> +}
>>> +
>>> static int amd_pmf_invoke_cmd_enact(struct amd_pmf_dev *dev)
>>> {
>>> struct ta_pmf_shared_memory *ta_sm = NULL;
>>> + struct ta_pmf_enact_result *out = NULL;
>>> struct tee_param param[MAX_TEE_PARAM];
>>> struct tee_ioctl_invoke_arg arg;
>>> int ret = 0;
>>> @@ -52,7 +120,10 @@ static int amd_pmf_invoke_cmd_enact(struct
>>> amd_pmf_dev *dev)
>>> if (!dev->tee_ctx)
>>> return -ENODEV;
>>> + memset(dev->shbuf, 0, dev->policy_sz);
>>> ta_sm = dev->shbuf;
>>> + out = &ta_sm->pmf_output.policy_apply_table;
>>> +
>>> memset(ta_sm, 0, sizeof(*ta_sm));
>>> ta_sm->command_id = TA_PMF_COMMAND_POLICY_BUILDER_ENACT_POLICIES;
>>> ta_sm->if_version = PMF_TA_IF_VERSION_MAJOR;
>>> @@ -65,6 +136,12 @@ static int amd_pmf_invoke_cmd_enact(struct
>>> amd_pmf_dev *dev)
>>> return ret;
>>> }
>>> + if (ta_sm->pmf_result == TA_PMF_TYPE_SUCCESS &&
>>> out->actions_count) {
>>> + dev_dbg(dev->dev, "action count:%u result:%x\n",
>>> out->actions_count,
>>> + ta_sm->pmf_result);
>>> + amd_pmf_apply_policies(dev, out);
>>> + }
>>> +
>>> return 0;
>>> }
>>> @@ -72,6 +149,7 @@ static int amd_pmf_invoke_cmd_init(struct
>>> amd_pmf_dev *dev)
>>> {
>>> struct ta_pmf_shared_memory *ta_sm = NULL;
>>> struct tee_param param[MAX_TEE_PARAM];
>>> + struct ta_pmf_init_table *in = NULL;
>>> struct tee_ioctl_invoke_arg arg;
>>> int ret = 0;
>>> @@ -80,10 +158,21 @@ static int amd_pmf_invoke_cmd_init(struct
>>> amd_pmf_dev *dev)
>>> return -ENODEV;
>>> }
>>> + dev_dbg(dev->dev, "Policy Binary size: %u bytes\n",
>>> dev->policy_sz);
>>> + memset(dev->shbuf, 0, dev->policy_sz);
>>> ta_sm = dev->shbuf;
>>> + in = &ta_sm->pmf_input.init_table;
>>> +
>>> ta_sm->command_id = TA_PMF_COMMAND_POLICY_BUILDER_INITIALIZE;
>>> ta_sm->if_version = PMF_TA_IF_VERSION_MAJOR;
>>> + in->metadata_macrocheck = false;
>>> + in->sku_check = false;
>>> + in->validate = true;
>>> + in->frequency = pb_actions_ms;
>>> + in->policies_table.table_size = dev->policy_sz;
>>> +
>>> + memcpy(in->policies_table.table, dev->policy_buf, dev->policy_sz);
>>> amd_pmf_prepare_args(dev,
>>> TA_PMF_COMMAND_POLICY_BUILDER_INITIALIZE, &arg, param);
>>> ret = tee_client_invoke_func(dev->tee_ctx, &arg, param);
>>> @@ -103,6 +192,51 @@ static void amd_pmf_invoke_cmd(struct
>>> work_struct *work)
>>> schedule_delayed_work(&dev->pb_work,
>>> msecs_to_jiffies(pb_actions_ms));
>>> }
>>> +static int amd_pmf_start_policy_engine(struct amd_pmf_dev *dev)
>>> +{
>>> + u32 cookie, length;
>>> + int res;
>>> +
>>> + cookie = readl(dev->policy_buf + POLICY_COOKIE_OFFSET);
>>> + length = readl(dev->policy_buf + POLICY_COOKIE_LEN);
>>> +
>>> + if (cookie != POLICY_SIGN_COOKIE || !length)
>>> + return -EINVAL;
>>> +
>>> + /* update the actual length */
>>> + dev->policy_sz = length + 512;
>>> + res = amd_pmf_invoke_cmd_init(dev);
>>> + if (res == TA_PMF_TYPE_SUCCESS) {
>>> + /* now its safe to announce that smart pc is enabled */
>>> + dev->smart_pc_enabled = 1;
>>> + /*
>>> + * Start collecting the data from PMFW after a small delay
>>> + * or else, we might end up getting stale values.
>>> + */
>>> + schedule_delayed_work(&dev->pb_work,
>>> msecs_to_jiffies(pb_actions_ms * 3));
>>> + } else {
>>> + dev_err(dev->dev, "ta invoke cmd init failed err: %x\n", res);
>>> + return res;
>>> + }
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int amd_pmf_get_bios_buffer(struct amd_pmf_dev *dev)
>>> +{
>>> + dev->policy_buf = kzalloc(dev->policy_sz, GFP_KERNEL);
>>> + if (!dev->policy_buf)
>>> + return -ENOMEM;
>>> +
>>> + dev->policy_base = devm_ioremap(dev->dev, dev->policy_addr,
>>> dev->policy_sz);
>>> + if (!dev->policy_base)
>>> + return -ENOMEM;
>>> +
>>> + memcpy(dev->policy_buf, dev->policy_base, dev->policy_sz);
>>> +
>>> + return amd_pmf_start_policy_engine(dev);
>>> +}
>>> +
>>> static int amd_pmf_amdtee_ta_match(struct tee_ioctl_version_data
>>> *ver, const void *data)
>>> {
>>> return ver->impl_id == TEE_IMPL_ID_AMDTEE;
>>> @@ -146,7 +280,7 @@ static int amd_pmf_tee_init(struct amd_pmf_dev
>>> *dev)
>>> goto out_ctx;
>>> }
>>> - size = sizeof(struct ta_pmf_shared_memory);
>>> + size = sizeof(struct ta_pmf_shared_memory) + dev->policy_sz;
>>> dev->fw_shm_pool = tee_shm_alloc_kernel_buf(dev->tee_ctx, size);
>>> if (IS_ERR(dev->fw_shm_pool)) {
>>> dev_err(dev->dev, "Failed to alloc TEE shared memory\n");
>>> @@ -190,11 +324,19 @@ int amd_pmf_init_smart_pc(struct amd_pmf_dev
>>> *dev)
>>> return ret;
>>> INIT_DELAYED_WORK(&dev->pb_work, amd_pmf_invoke_cmd);
>>> - return 0;
>>> + amd_pmf_set_dram_addr(dev);
>>> + amd_pmf_get_bios_buffer(dev);
>>> + dev->prev_data = kzalloc(sizeof(*dev->prev_data), GFP_KERNEL);
>>> + if (!dev->prev_data)
>>> + return -ENOMEM;
>>> +
>>> + return dev->smart_pc_enabled;
>>> }
>>> void amd_pmf_deinit_smart_pc(struct amd_pmf_dev *dev)
>>> {
>>> + kfree(dev->prev_data);
>>> + kfree(dev->policy_buf);
>>> cancel_delayed_work_sync(&dev->pb_work);
>>> amd_pmf_tee_deinit(dev);
>>> }
>>
^ permalink raw reply
* Re: [PATCH v4 2/4] dt-bindings: touchscreen: add overlay-touchscreen and overlay-buttons properties
From: Javier Carrasco @ 2023-10-13 9:54 UTC (permalink / raw)
To: Rob Herring
Cc: Dmitry Torokhov, Krzysztof Kozlowski, Conor Dooley,
Henrik Rydberg, Bastian Hecht, Michael Riesch, linux-kernel,
linux-input, devicetree
In-Reply-To: <20230831114827.GA1941458-robh@kernel.org>
Hi Rob,
On 31.08.23 13:48, Rob Herring wrote:
> On Thu, Aug 24, 2023 at 03:17:10PM +0200, Javier Carrasco wrote:
>> The overlay-touchscreen object defines an area within the touchscreen
>> where touch events are reported and their coordinates get converted to
>> the overlay origin. This object avoids getting events from areas that
>> are physically hidden by overlay frames.
>>
>> For touchscreens where overlay buttons on the touchscreen surface are
>> provided, the overlay-buttons object contains a node for every button
>> and the key event that should be reported when pressed.
>>
>> Signed-off-by: Javier Carrasco <javier.carrasco@wolfvision.net>
>> ---
>> .../bindings/input/touchscreen/touchscreen.yaml | 152 +++++++++++++++++++++
>> 1 file changed, 152 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/input/touchscreen/touchscreen.yaml b/Documentation/devicetree/bindings/input/touchscreen/touchscreen.yaml
>> index 895592da9626..d90cbb4932b5 100644
>> --- a/Documentation/devicetree/bindings/input/touchscreen/touchscreen.yaml
>> +++ b/Documentation/devicetree/bindings/input/touchscreen/touchscreen.yaml
>> @@ -80,6 +80,158 @@ properties:
>> touchscreen-y-plate-ohms:
>> description: Resistance of the Y-plate in Ohms
>>
>> + overlay-touchscreen:
>> + description: Clipped touchscreen area
>> +
>> + This object can be used to describe a frame that restricts the area
>> + within touch events are reported, ignoring the events that occur outside
>> + this area. This is of special interest if the touchscreen is shipped
>> + with a physical overlay on top of it with a frame that hides some part
>> + of the original touchscreen area.
>> +
>> + The x-origin and y-origin properties of this object define the offset of
>> + a new origin from where the touchscreen events are referenced.
>> + This offset is applied to the events accordingly. The x-size and y-size
>> + properties define the size of the overlay-touchscreen (effective area).
>> +
>> + The following example shows the new touchscreen area and the new origin
>> + (0',0') for the touch events generated by the device.
>> +
>> + Touchscreen (full area)
>> + ┌────────────────────────────────────────┐
>> + │ ┌───────────────────────────────┐ │
>> + │ │ │ │
>> + │ ├ y-size │ │
>> + │ │ │ │
>> + │ │ overlay-touchscreen │ │
>> + │ │ │ │
>> + │ │ │ │
>> + │ │ x-size │ │
>> + │ ┌└──────────────┴────────────────┘ │
>> + │(0',0') │
>> + ┌└────────────────────────────────────────┘
>> + (0,0)
>> +
>> + where (0',0') = (0+x-origin,0+y-origin)
>
> What happens if touchscreen-inverted-x/y are set?
>
> Though the existing binding never defines what the non-inverted
> orientation is.
>
This feature acts on the raw input values and the inversion is carried
out afterwards.The events will be inverted within the defined frame,
which is probably the expected behavior.
>> +
>> + type: object
>> +
>> + properties:
>> + x-origin:
>> + description: horizontal origin of the clipped area
>> + $ref: /schemas/types.yaml#/definitions/uint32
>> +
>> + y-origin:
>> + description: vertical origin of the clipped area
>> + $ref: /schemas/types.yaml#/definitions/uint32
>> +
>> + x-size:
>> + description: horizontal resolution of the clipped area
>> + $ref: /schemas/types.yaml#/definitions/uint32
>> +
>> + y-size:
>> + description: vertical resolution of the clipped area
>> + $ref: /schemas/types.yaml#/definitions/uint32
>> +
>> + required:
>> + - x-origin
>> + - y-origin
>> + - x-size
>> + - y-size
>> +
>> + overlay-buttons:
>> + description: list of nodes defining the buttons on the touchscreen
>> +
>> + This object can be used to describe buttons on the touchscreen area,
>> + reporting the touch events on their surface as key events instead of
>> + the original touch events.
>> +
>> + This is of special interest if the touchscreen is shipped with a
>> + physical overlay on top of it where a number of buttons with some
>> + predefined functionality are printed. In that case a specific behavior
>> + is expected from those buttons instead of raw touch events.
>> +
>> + The overlay-buttons properties define a per-button area as well as an
>> + origin relative to the real touchscreen origin. Touch events within the
>> + button area are reported as the key event defined in the linux,code
>> + property. Given that the key events do not provide coordinates, the
>> + button origin is only used to place the button area on the touchscreen
>> + surface. Any event outside the overlay-buttons object is reported as a
>> + touch event with no coordinate transformation.
>> +
>> + The following example shows a touchscreen with a single button on it
>> +
>> + Touchscreen (full area)
>> + ┌───────────────────────────────────┐
>> + │ │
>> + │ │
>> + │ ┌─────────┐ │
>> + │ │button 0 │ │
>> + │ │KEY_POWER│ │
>> + │ └─────────┘ │
>> + │ │
>> + │ │
>> + ┌└───────────────────────────────────┘
>> + (0,0)
>> +
>> + The overlay-buttons object can be combined with the overlay-touchscreen
>> + object as shown in the following example. In that case only the events
>> + within the overlay-touchscreen object are reported as touch events.
>> +
>> + Touchscreen (full area)
>> + ┌─────────┬──────────────────────────────┐
>> + │ │ │
>> + │ │ ┌───────────────────────┐ │
>> + │ button 0│ │ │ │
>> + │KEY_POWER│ │ │ │
>> + │ │ │ │ │
>> + ├─────────┤ │ overlay-touchscreen │ │
>> + │ │ │ │ │
>> + │ │ │ │ │
>> + │ button 1│ │ │ │
>> + │ KEY_INFO│ ┌└───────────────────────┘ │
>> + │ │(0',0') │
>> + ┌└─────────┴──────────────────────────────┘
>> + (0,0)
>> +
>> + type: object
>> +
>> + patternProperties:
>> + '^button-':
>> + type: object
>> + description:
>> + Each button (key) is represented as a sub-node.
>> +
>> + properties:
>> + label:
>> + $ref: /schemas/types.yaml#/definitions/string
>> + description: descriptive name of the button
>> +
>> + linux,code: true
>> +
>> + x-origin:
>> + description: horizontal origin of the button area
>> + $ref: /schemas/types.yaml#/definitions/uint32
>> +
>> + y-origin:
>> + description: vertical origin of the button area
>> + $ref: /schemas/types.yaml#/definitions/uint32
>> +
>> + x-size:
>> + description: horizontal resolution of the button area
>> + $ref: /schemas/types.yaml#/definitions/uint32
>> +
>> + y-size:
>> + description: vertical resolution of the button area
>> + $ref: /schemas/types.yaml#/definitions/uint32
>> +
>> + required:
>> + - linux,code
>> + - x-origin
>> + - y-origin
>> + - x-size
>> + - y-size
>
> We have the same properties defined twice. Move all the common ones to a
> $def entry and then reference it here and in overlay-touchscreen.
>
> $defs:
> overlay-node:
> type: object
> properties:
> x-origin:
> ...
>
> And then here:
>
> '^button-':
> $ref: '#/$defs/overlay-node
> unevaluatedProperties: false
>
> properties:
> label:
> ...
>
>
> Rob
Thank your for your feedback, I will send a next version with this
modification in it.
Best regards,
Javier Carrasco
^ permalink raw reply
* Re: [PATCH v3 06/16] platform/x86/amd/pmf: Add support to get inputs from other subsystems
From: Ilpo Järvinen @ 2023-10-13 12:08 UTC (permalink / raw)
To: Shyam Sundar S K
Cc: Hans de Goede, markgross, basavaraj.natikar, jikos,
benjamin.tissoires, alexander.deucher, christian.koenig,
Xinhui.Pan, airlied, daniel, Patil.Reddy, mario.limonciello,
platform-driver-x86, linux-input, amd-gfx, dri-devel
In-Reply-To: <20231010125917.138225-7-Shyam-sundar.S-k@amd.com>
On Tue, 10 Oct 2023, Shyam Sundar S K wrote:
> PMF driver sends changing inputs from each subystem to TA for evaluating
> the conditions in the policy binary.
>
> Add initial support of plumbing in the PMF driver for Smart PC to get
> information from other subsystems in the kernel.
>
> Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
> ---
> drivers/platform/x86/amd/pmf/Makefile | 2 +-
> drivers/platform/x86/amd/pmf/pmf.h | 18 ++++
> drivers/platform/x86/amd/pmf/spc.c | 119 ++++++++++++++++++++++++++
> drivers/platform/x86/amd/pmf/tee-if.c | 3 +
> 4 files changed, 141 insertions(+), 1 deletion(-)
> create mode 100644 drivers/platform/x86/amd/pmf/spc.c
>
> diff --git a/drivers/platform/x86/amd/pmf/Makefile b/drivers/platform/x86/amd/pmf/Makefile
> index d2746ee7369f..6b26e48ce8ad 100644
> --- a/drivers/platform/x86/amd/pmf/Makefile
> +++ b/drivers/platform/x86/amd/pmf/Makefile
> @@ -7,4 +7,4 @@
> obj-$(CONFIG_AMD_PMF) += amd-pmf.o
> amd-pmf-objs := core.o acpi.o sps.o \
> auto-mode.o cnqf.o \
> - tee-if.o
> + tee-if.o spc.o
> diff --git a/drivers/platform/x86/amd/pmf/pmf.h b/drivers/platform/x86/amd/pmf/pmf.h
> index 51c0e17f7720..88ee3c705913 100644
> --- a/drivers/platform/x86/amd/pmf/pmf.h
> +++ b/drivers/platform/x86/amd/pmf/pmf.h
> @@ -150,6 +150,21 @@ struct smu_pmf_metrics {
> u16 infra_gfx_maxfreq; /* in MHz */
> u16 skin_temp; /* in centi-Celsius */
> u16 device_state;
> + u16 curtemp; /* in centi-Celsius */
> + u16 filter_alpha_value;
> + u16 avg_gfx_clkfrequency;
> + u16 avg_fclk_frequency;
> + u16 avg_gfx_activity;
> + u16 avg_socclk_frequency;
> + u16 avg_vclk_frequency;
> + u16 avg_vcn_activity;
> + u16 avg_dram_reads;
> + u16 avg_dram_writes;
> + u16 avg_socket_power;
> + u16 avg_core_power[2];
> + u16 avg_core_c0residency[16];
> + u16 spare1;
> + u32 metrics_counter;
> } __packed;
>
> enum amd_stt_skin_temp {
> @@ -596,4 +611,7 @@ extern const struct attribute_group cnqf_feature_attribute_group;
> int amd_pmf_init_smart_pc(struct amd_pmf_dev *dev);
> void amd_pmf_deinit_smart_pc(struct amd_pmf_dev *dev);
> int apmf_check_smart_pc(struct amd_pmf_dev *pmf_dev);
> +
> +/* Smart PC - TA interfaces */
> +void amd_pmf_populate_ta_inputs(struct amd_pmf_dev *dev, struct ta_pmf_enact_table *in);
> #endif /* PMF_H */
> diff --git a/drivers/platform/x86/amd/pmf/spc.c b/drivers/platform/x86/amd/pmf/spc.c
> new file mode 100644
> index 000000000000..91a7f1da911c
> --- /dev/null
> +++ b/drivers/platform/x86/amd/pmf/spc.c
> @@ -0,0 +1,119 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * AMD Platform Management Framework Driver - Smart PC Capabilities
> + *
> + * Copyright (c) 2023, Advanced Micro Devices, Inc.
> + * All Rights Reserved.
> + *
> + * Authors: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
> + * Patil Rajesh Reddy <Patil.Reddy@amd.com>
> + */
> +
> +#include <acpi/button.h>
> +#include <linux/power_supply.h>
> +#include <linux/units.h>
> +#include "pmf.h"
> +
> +static void amd_pmf_get_smu_info(struct amd_pmf_dev *dev, struct ta_pmf_enact_table *in)
> +{
> + u16 max, avg = 0;
> + int i;
> +
> + memset(dev->buf, 0, sizeof(dev->m_table));
> + amd_pmf_send_cmd(dev, SET_TRANSFER_TABLE, 0, 7, NULL);
> + memcpy(&dev->m_table, dev->buf, sizeof(dev->m_table));
> +
> + in->ev_info.socket_power = dev->m_table.apu_power + dev->m_table.dgpu_power;
> + in->ev_info.skin_temperature = dev->m_table.skin_temp;
> +
> + /* Get the avg and max C0 residency of all the cores */
> + max = dev->m_table.avg_core_c0residency[0];
> + for (i = 0; i < ARRAY_SIZE(dev->m_table.avg_core_c0residency); i++) {
> + avg += dev->m_table.avg_core_c0residency[i];
> + if (dev->m_table.avg_core_c0residency[i] > max)
> + max = dev->m_table.avg_core_c0residency[i];
> + }
> +
> + in->ev_info.avg_c0residency = avg / ARRAY_SIZE(dev->m_table.avg_core_c0residency);
Not saying the current is wrong and the difference might be
insignificantly small... But you might want to consider using
DIV_ROUND_CLOSEST() instead of the truncating divide (I'm not sure which
is the best here so I leave it up to you).
> + in->ev_info.max_c0residency = max;
> + in->ev_info.gfx_busy = dev->m_table.avg_gfx_activity;
> +}
> +
> +static const char * const pmf_battery_supply_name[] = {
> + "BATT",
> + "BAT0",
> +};
> +
> +static int amd_pmf_get_battery_prop(enum power_supply_property prop)
> +{
> + union power_supply_propval value;
> + struct power_supply *psy;
> + int i, ret = -EINVAL;
Unnecessary to initialize ret here.
> + for (i = 0; i < ARRAY_SIZE(pmf_battery_supply_name); i++) {
> + psy = power_supply_get_by_name(pmf_battery_supply_name[i]);
> + if (!psy)
> + continue;
> +
> + ret = power_supply_get_property(psy, prop, &value);
> + if (ret) {
> + power_supply_put(psy);
> + return ret;
> + }
> + }
> +
> + return value.intval;
> +}
> +
> +static int amd_pmf_get_battery_info(struct amd_pmf_dev *dev, struct ta_pmf_enact_table *in)
> +{
> + int val;
> +
> + val = amd_pmf_get_battery_prop(POWER_SUPPLY_PROP_PRESENT);
> + if (val != 1)
> + return -EINVAL;
If amd_pmf_get_battery_prop() returns an error code, it would be better to
pass that on instead of inventing another code. It's probably better to
split the if into two checks to cover both cases.
For val >= 0 but not 1, -ENODEV might be better justified than -EINVAL
because it doesn't look like input parameters are invalid here.
--
i.
> + in->ev_info.bat_percentage = amd_pmf_get_battery_prop(POWER_SUPPLY_PROP_CAPACITY);
> + /* all values in mWh metrics */
> + in->ev_info.bat_design = amd_pmf_get_battery_prop(POWER_SUPPLY_PROP_ENERGY_FULL_DESIGN) /
> + MILLIWATT_PER_WATT;
> + in->ev_info.full_charge_capacity = amd_pmf_get_battery_prop(POWER_SUPPLY_PROP_ENERGY_FULL) /
> + MILLIWATT_PER_WATT;
> + in->ev_info.drain_rate = amd_pmf_get_battery_prop(POWER_SUPPLY_PROP_POWER_NOW) /
> + MILLIWATT_PER_WATT;
> +
> + return 0;
> +}
> +
> +static int amd_pmf_get_slider_info(struct amd_pmf_dev *dev, struct ta_pmf_enact_table *in)
> +{
> + int val;
> +
> + switch (dev->current_profile) {
> + case PLATFORM_PROFILE_PERFORMANCE:
> + val = TA_BEST_PERFORMANCE;
> + break;
> + case PLATFORM_PROFILE_BALANCED:
> + val = TA_BETTER_PERFORMANCE;
> + break;
> + case PLATFORM_PROFILE_LOW_POWER:
> + val = TA_BEST_BATTERY;
> + break;
> + default:
> + dev_err(dev->dev, "Unknown Platform Profile.\n");
> + return -EOPNOTSUPP;
> + }
> + in->ev_info.power_slider = val;
> +
> + return 0;
> +}
> +
> +void amd_pmf_populate_ta_inputs(struct amd_pmf_dev *dev, struct ta_pmf_enact_table *in)
> +{
> + /* TA side lid open is 1 and close is 0, hence the ! here */
> + in->ev_info.lid_state = !acpi_lid_open();
> + in->ev_info.power_source = amd_pmf_get_power_source();
> + amd_pmf_get_smu_info(dev, in);
> + amd_pmf_get_battery_info(dev, in);
> + amd_pmf_get_slider_info(dev, in);
> +}
> diff --git a/drivers/platform/x86/amd/pmf/tee-if.c b/drivers/platform/x86/amd/pmf/tee-if.c
> index 38f02676261d..277103e4346d 100644
> --- a/drivers/platform/x86/amd/pmf/tee-if.c
> +++ b/drivers/platform/x86/amd/pmf/tee-if.c
> @@ -113,6 +113,7 @@ static int amd_pmf_invoke_cmd_enact(struct amd_pmf_dev *dev)
> {
> struct ta_pmf_shared_memory *ta_sm = NULL;
> struct ta_pmf_enact_result *out = NULL;
> + struct ta_pmf_enact_table *in = NULL;
> struct tee_param param[MAX_TEE_PARAM];
> struct tee_ioctl_invoke_arg arg;
> int ret = 0;
> @@ -123,11 +124,13 @@ static int amd_pmf_invoke_cmd_enact(struct amd_pmf_dev *dev)
> memset(dev->shbuf, 0, dev->policy_sz);
> ta_sm = dev->shbuf;
> out = &ta_sm->pmf_output.policy_apply_table;
> + in = &ta_sm->pmf_input.enact_table;
>
> memset(ta_sm, 0, sizeof(*ta_sm));
> ta_sm->command_id = TA_PMF_COMMAND_POLICY_BUILDER_ENACT_POLICIES;
> ta_sm->if_version = PMF_TA_IF_VERSION_MAJOR;
>
> + amd_pmf_populate_ta_inputs(dev, in);
> amd_pmf_prepare_args(dev, TA_PMF_COMMAND_POLICY_BUILDER_ENACT_POLICIES, &arg, param);
>
> ret = tee_client_invoke_func(dev->tee_ctx, &arg, param);
>
^ permalink raw reply
* Re: [regression] Resume broken on T14s Gen1 (AMD) due to "Input: psmouse - add delay when deactivating for SMBus mode"
From: Linux regression tracking #update (Thorsten Leemhuis) @ 2023-10-13 14:04 UTC (permalink / raw)
To: Linux kernel regressions list; +Cc: linux-input, linux-kernel
In-Reply-To: <ca0109fa-c64b-43c1-a651-75b294d750a1@leemhuis.info>
[TLDR: This mail in primarily relevant for Linux kernel regression
tracking. See link in footer if these mails annoy you.]
On 27.09.23 10:54, Thorsten Leemhuis wrote:
> Hi Jeffery! Your change 92e24e0e57f72e ("Input: psmouse - add delay when
> deactivating for SMBus mode") [merged for v6.6-rc1] broke resume on my
> T14s Gen1 (AMD): the system didn't really resume again at all (the
> display almost always didn't re-initialize) and there was nothing in the
> logs. I found your commit to be the culprit using a bisection and
> confirmed that reverting it on top of Linux 6.6-rc3 makes thing work
> again for me.
>
> #regzbot introduced 92e24e0e57f72e
> #regzbot title Input: psmouse - Resume broken on T14s Gen1 (AMD) due to
> a new delay when deactivating for SMBus mode
#regzbot monitor:
https://lore.kernel.org/all/20231004005729.3943515-1-jefferymiller@google.com/
#regzbot ignore-activity
Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
--
Everything you wanna know about Linux kernel regression tracking:
https://linux-regtracking.leemhuis.info/about/#tldr
That page also explains what to do if mails like this annoy you.
^ permalink raw reply
* Implement per-key keyboard backlight as auxdisplay?
From: Werner Sembach @ 2023-10-13 14:54 UTC (permalink / raw)
To: Pavel Machek, ojeda
Cc: Lee Jones, linux-kernel, linux-leds, linux-input,
dri-devel@lists.freedesktop.org
In-Reply-To: <ZSk16iTBmZ2fLHZ0@duo.ucw.cz>
Hi,
coming from the leds mailing list I'm writing with Pavel how to best handle
per-key RGB keyboards.
His suggestion was that it could be implemented as an aux display, but he also
suggested that I ask first if this fits.
The specific keyboard RGB controller I want to implement takes 6*21 rgb values.
However not every one is actually mapped to a physical key. e.g. the bottom row
needs less entries because of the space bar. Additionally the keys are ofc not
in a straight line from top to bottom.
Best regards,
Werner
^ permalink raw reply
* [PATCH v2] Input: xpad - add PXN V900 support
From: Matthias Berndt @ 2023-10-13 19:01 UTC (permalink / raw)
To: linux-input
[-- Attachment #1: Type: text/plain, Size: 237 bytes --]
Hey,
I'd like to submit again my patch to enable support for the PXN V900 steering
wheel in the xpad driver. It now contains the necessary Signed-off-by line –
thank you Dmitry for pointing that out.
All the best,
Matthias
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Input-xpad-add-PXN-V900-support.patch --]
[-- Type: text/x-patch; charset="x-UTF_8J"; name="0001-Input-xpad-add-PXN-V900-support.patch", Size: 1527 bytes --]
From 79080b54a269071eeb9896ade7bb609c1b88c527 Mon Sep 17 00:00:00 2001
From: Matthias Berndt <matthias_berndt@gmx.de>
Date: Mon, 25 Sep 2023 17:54:13 +0200
Subject: [PATCH] Input: xpad - add PXN V900 support
Add VID and PID to the xpad_device table to allow driver
to use the PXN V900 steering wheel, which is
XTYPE_XBOX360 compatible in xinput mode.
Signed-off-by: Matthias Berndt <matthias_berndt@gmx.de>
---
drivers/input/joystick/xpad.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/input/joystick/xpad.c b/drivers/input/joystick/xpad.c
index ede380551e55..478bf657efc2 100644
--- a/drivers/input/joystick/xpad.c
+++ b/drivers/input/joystick/xpad.c
@@ -272,6 +272,7 @@ static const struct xpad_device {
{ 0x1038, 0x1430, "SteelSeries Stratus Duo", 0, XTYPE_XBOX360 },
{ 0x1038, 0x1431, "SteelSeries Stratus Duo", 0, XTYPE_XBOX360 },
{ 0x11c9, 0x55f0, "Nacon GC-100XF", 0, XTYPE_XBOX360 },
+ { 0x11ff, 0x0511, "PXN V900", 0, XTYPE_XBOX360 },
{ 0x1209, 0x2882, "Ardwiino Controller", 0, XTYPE_XBOX360 },
{ 0x12ab, 0x0004, "Honey Bee Xbox360 dancepad", MAP_DPAD_TO_BUTTONS, XTYPE_XBOX360 },
{ 0x12ab, 0x0301, "PDP AFTERGLOW AX.1", 0, XTYPE_XBOX360 },
@@ -503,6 +504,7 @@ static const struct usb_device_id xpad_table[] = {
XPAD_XBOX360_VENDOR(0x3285), /* Nacon GC-100 */
XPAD_XBOX360_VENDOR(0x3537), /* GameSir Controllers */
XPAD_XBOXONE_VENDOR(0x3537), /* GameSir Controllers */
+ XPAD_XBOX360_VENDOR(0x11ff), /* PXN V900 */
{ }
};
--
2.41.0
^ permalink raw reply related
* Re: Implement per-key keyboard backlight as auxdisplay?
From: Pavel Machek @ 2023-10-13 19:56 UTC (permalink / raw)
To: Werner Sembach
Cc: ojeda, Lee Jones, linux-kernel, linux-leds, linux-input,
dri-devel@lists.freedesktop.org
In-Reply-To: <aac81702-df1e-43a2-bfe9-28e9cb8d2282@tuxedocomputers.com>
[-- Attachment #1: Type: text/plain, Size: 1229 bytes --]
Hi!
> coming from the leds mailing list I'm writing with Pavel how to best handle
> per-key RGB keyboards.
>
> His suggestion was that it could be implemented as an aux display, but he
> also suggested that I ask first if this fits.
Thanks for doing this.
> The specific keyboard RGB controller I want to implement takes 6*21 rgb
> values. However not every one is actually mapped to a physical key. e.g. the
> bottom row needs less entries because of the space bar. Additionally the
> keys are ofc not in a straight line from top to bottom.
So... a bit of rationale. The keyboard does not really fit into the
LED subsystem; LEDs are expected to be independent ("hdd led") and not
a matrix of them.
We do see various strange displays these days -- they commonly have
rounded corners and holes in them. I'm not sure how that's currently
supported, but I believe it is reasonable to view keyboard as a
display with slightly weird placing of pixels.
Plus, I'd really like to play tetris on one of those :-).
So, would presenting them as auxdisplay be acceptable? Or are there
better options?
Best regards,
Pavel
--
People of Russia, stop Putin before his war on Ukraine escalates.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]
^ permalink raw reply
* Re: Implement per-key keyboard backlight as auxdisplay?
From: Pavel Machek @ 2023-10-13 20:03 UTC (permalink / raw)
To: Werner Sembach
Cc: ojeda, Lee Jones, linux-kernel, linux-leds, linux-input,
dri-devel@lists.freedesktop.org
In-Reply-To: <ZSmg4tqXiYiX18K/@duo.ucw.cz>
[-- Attachment #1: Type: text/plain, Size: 1305 bytes --]
Hi!
> > The specific keyboard RGB controller I want to implement takes 6*21 rgb
> > values. However not every one is actually mapped to a physical key. e.g. the
> > bottom row needs less entries because of the space bar. Additionally the
> > keys are ofc not in a straight line from top to bottom.
>
> So... a bit of rationale. The keyboard does not really fit into the
> LED subsystem; LEDs are expected to be independent ("hdd led") and not
> a matrix of them.
>
> We do see various strange displays these days -- they commonly have
> rounded corners and holes in them. I'm not sure how that's currently
> supported, but I believe it is reasonable to view keyboard as a
> display with slightly weird placing of pixels.
>
> Plus, I'd really like to play tetris on one of those :-).
>
> So, would presenting them as auxdisplay be acceptable? Or are there
> better options?
Oh and... My existing keyboard membrane-based Chicony, and it is from
time when PS/2 was still in wide use. I am slowly looking for a new
keyboard. If you know of one with nice mechanical switches, RGB
backlight with known protocol, and hopefully easily available in Czech
republic, let me know.
Best regards,
Pavel
--
People of Russia, stop Putin before his war on Ukraine escalates.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]
^ permalink raw reply
* Re: uinput: waiting for UI_FF_UPLOAD events will not inform user when allocation is required
From: Dmitry Torokhov @ 2023-10-13 21:59 UTC (permalink / raw)
To: John Salamon; +Cc: rydberg, linux-input, linux-kernel
In-Reply-To: <CA+fyA4RABYNPZZSk9+9U51u53kbSzqgwdi1KDDGRxXi8q5TtxQ@mail.gmail.com>
Hi John,
On Tue, Oct 10, 2023 at 05:38:27PM +1030, John Salamon wrote:
> Currently the "fake" input events generated by uinput in response to
> effect uploads will return an effect with an id that has already been
> handled by input_ff_upload in ff-core.c, which can modify the effect
> id. This causes a problem specifically when the effect originally
> uploaded via the EVIOCSFF ioctl contained an effect with -1, as the
> userspace code handling UI_FF_UPLOAD receives an effect with an id
> other than -1, and therefore will not know an allocation was
> requested.
The kernel never changes ID of an existing effect, the only time ID is
changed is when userspace indicates that a new effect should be created
by setting effect ID to -1.
The handler of force feedback effects should know what effects (with
what IDs) have been uploaded to the device so far, so whenever it sees a
request for an effect with previously unseen effect_id it should
recognize this as a signal that a new effect/id has been allocated by
the kernel.
>
> I notice that the "old" field on the ff_effect struct is set to NULL
> when the -1 id is changed (in input_ff_upload), which can serve as a
> flag that an allocation was requested. If it is the intention is that
> uinput users check if old == NULL to know when allocations are needed
> I think uinput documentation should describe this.
No, not really, as explained above.
>
> I first noticed this using python-evdev, see my issue report here:
> https://github.com/gvalkov/python-evdev/issues/199
Thanks.
--
Dmitry
^ 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