* Re: [PATCH RFC 0/4] input: touchscreen: add initial support for Goodix Berlin touchscreen IC
From: Neil Armstrong @ 2023-06-06 18:55 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Hans de Goede, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Bastien Nocera, Henrik Rydberg, linux-input, linux-arm-msm,
devicetree, linux-kernel
In-Reply-To: <ZH9+ndrF0RIgFhnI@google.com>
Hi Dmitry,
On 06/06/2023 20:44, Dmitry Torokhov wrote:
> On Tue, Jun 06, 2023 at 08:12:04PM +0200, Neil Armstrong wrote:
>> Hi,
>>
>> On 06/06/2023 17:31, Hans de Goede wrote:
>>> Hi Neil,
>>>
>>> On 6/6/23 16:31, Neil Armstrong wrote:
>>>> These touchscreen ICs support SPI, I2C and I3C interface, up to
>>>> 10 finger touch, stylus and gestures events.
>>>>
>>>> This initial driver is derived from the Goodix goodix_ts_berlin
>>>> available at [1] and [2] and only supports the GT9916 IC
>>>> present on the Qualcomm SM8550 MTP & QRD touch panel.
>>>>
>>>> The current implementation only supports BerlinD, aka GT9916.
>>>>
>>>> Support for advanced features like:
>>>> - Firmware & config update
>>>> - Stylus events
>>>> - Gestures events
>>>> - Previous revisions support (BerlinA or BerlinB)
>>>> is not included in current version.
>>>>
>>>> The current support will work with currently flashed firmware
>>>> and config, and bail out if firmware or config aren't flashed yet.
>>>
>>> What I'm missing here / in the commit msg of
>>> "input: touchscreen: add core support for Goodix Berlin Touchscreen IC"
>>>
>>> is an explanation why this is a new driver instead of adding
>>> support to the existing goodix.c code.
>>>
>>> I assume you have good reasons for this, but it would be good
>>> if you can write the reasons for this down.
>>
>> Sure, should I write it down here and/or update the commit message in a new revision ?
>>
>> Anyway, here's the reasons:
>> - globally the event handling "looks like" the current goodix.c, but again the offsets
>> are again different and none of the register address are the same, and unlike the current
>> support all registers are provided by the "ic_info" structure
>> - while with the current code it *could* be possible to merge it, with a lot of changes,
>> the firmware management looks really different, and it would be really hard to merge.
>>
>> But I may be wrong, and may be misleaded by the goodix driver structure (even if it
>> went through a really heavy cleaning process).
>>
>> Globally it seems they tried to match the "event handling" process of the previous
>> generations, but the firmware interface is completely different.
>
> It is not unprecedented for drivers to share event processing and
> implement several ways/generations of firmware update mechanisms.
Thanks for your reply, I'm perfectly aware of that, this is why I posted
this as RFC.
If the event handling is vaguely similar, I'm not sure it's worth refactoring the
current driver since I do not have the old and current IC datasheet nor
HW to check for current support non-regression.
What I'm sure is that not a single register address, flag or struct is even close
to the current upstream defined ones.
Neil
>
> Thanks.
>
^ permalink raw reply
* Re: [PATCH v2 1/2] dt-bindings: input: touchscreen: Add ilitek 9882T touchscreen chip
From: Dmitry Torokhov @ 2023-06-06 18:51 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Cong Yang, robh+dt, krzysztof.kozlowski+dt, conor+dt, jikos,
benjamin.tissoires, dianders, hsinyi, linux-input, devicetree,
linux-kernel
In-Reply-To: <6818f4b0-4222-c3bb-c55f-bc0d26d7a681@linaro.org>
On Mon, Jun 05, 2023 at 12:34:54PM +0200, Krzysztof Kozlowski wrote:
> On 05/06/2023 08:05, Cong Yang wrote:
> > Add an ilitek touch screen chip ili9882t.
> >
> > Signed-off-by: Cong Yang <yangcong5@huaqin.corp-partner.google.com>
> > ---
> > .../bindings/input/elan,ekth6915.yaml | 23 ++++++++++++++++---
> > 1 file changed, 20 insertions(+), 3 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/input/elan,ekth6915.yaml b/Documentation/devicetree/bindings/input/elan,ekth6915.yaml
> > index 05e6f2df604c..f0e7ffdce605 100644
> > --- a/Documentation/devicetree/bindings/input/elan,ekth6915.yaml
> > +++ b/Documentation/devicetree/bindings/input/elan,ekth6915.yaml
> > @@ -15,11 +15,14 @@ description:
> >
> > properties:
> > compatible:
> > - items:
> > - - const: elan,ekth6915
> > + enum:
> > + - elan,ekth6915
> > + - ilitek,ili9882t
> >
> > reg:
> > - const: 0x10
> > + enum:
> > + - 0x10
> > + - 0x41
> >
> > interrupts:
> > maxItems: 1
> > @@ -29,11 +32,13 @@ properties:
> >
> > vcc33-supply:
> > description: The 3.3V supply to the touchscreen.
> > + If using ili9882t then this supply will not be needed.
>
> What does it mean "will not be needed"? Describe the hardware, not your
> drivers.
I do not think it makes sense to merge Ilitek and Elan into a single
binding. The only thing that they have in common is that we are trying
to reuse drivers/hid/i2c-hid/i2c-hid-of-elan.c to handle reset timings
(which is also questionable IMO).
Maybe if we had a single unified binding for HID-over-I2C touchscreens
then combining would make more sense, at least to me...
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH v2 1/2] dt-bindings: input: touchscreen: Add ilitek 9882T touchscreen chip
From: Dmitry Torokhov @ 2023-06-06 18:47 UTC (permalink / raw)
To: cong yang
Cc: Conor Dooley, robh+dt, krzysztof.kozlowski+dt, conor+dt, jikos,
benjamin.tissoires, dianders, hsinyi, linux-input, devicetree,
linux-kernel
In-Reply-To: <CAHwB_NK_j1SJ1BBkVqafFM_+fWSyvwjCpMmHQxjLjnz_KHR=KA@mail.gmail.com>
On Tue, Jun 06, 2023 at 10:06:05AM +0800, cong yang wrote:
> Hi,Conor,
>
> On Mon, Jun 5, 2023 at 6:20 PM Conor Dooley <conor@kernel.org> wrote:
> >
> > Hey Cong Yang,
> >
> > On Mon, Jun 05, 2023 at 02:05:23PM +0800, Cong Yang wrote:
> > > Add an ilitek touch screen chip ili9882t.
> >
> > Could you add a comment here mentioning the relationship between these
> > chips?
>
> Okay, I will add in V3 version.
>
> > On Mon, Jun 05, 2023 at 02:05:23PM +0800, Cong Yang wrote:
> >
> > > Signed-off-by: Cong Yang <yangcong5@huaqin.corp-partner.google.com>
> > > ---
> > > .../bindings/input/elan,ekth6915.yaml | 23 ++++++++++++++++---
> > > 1 file changed, 20 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/Documentation/devicetree/bindings/input/elan,ekth6915.yaml b/Documentation/devicetree/bindings/input/elan,ekth6915.yaml
> > > index 05e6f2df604c..f0e7ffdce605 100644
> > > --- a/Documentation/devicetree/bindings/input/elan,ekth6915.yaml
> > > +++ b/Documentation/devicetree/bindings/input/elan,ekth6915.yaml
> > > @@ -15,11 +15,14 @@ description:
> > >
> > > properties:
> > > compatible:
> > > - items:
> > > - - const: elan,ekth6915
> > > + enum:
> > > + - elan,ekth6915
> > > + - ilitek,ili9882t
> > >
> > > reg:
> > > - const: 0x10
> > > + enum:
> > > + - 0x10
> > > + - 0x41
> >
> > Is 0x10 only valid for the elan,ekth6915 & 0x41 for the ilitek one?
> > If so, please add some enforcement of the values based on the
> > compatible.
>
> I don't think 0x10 is the only address for ekth6915,(nor is 0x41 the
> only address for ili9882t). It depends on the hardware design.
Only a handful of controllers allow switching between addresses, and
if they do they typically have a "main" and an "alternate" one, and not
something completely customizable. I do not believe Elan offers ways to
program different address, do they?
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH RFC 0/4] input: touchscreen: add initial support for Goodix Berlin touchscreen IC
From: Dmitry Torokhov @ 2023-06-06 18:44 UTC (permalink / raw)
To: Neil Armstrong
Cc: Hans de Goede, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Bastien Nocera, Henrik Rydberg, linux-input, linux-arm-msm,
devicetree, linux-kernel
In-Reply-To: <2677ae8c-59d3-b658-dc3f-918838ac0fb6@linaro.org>
On Tue, Jun 06, 2023 at 08:12:04PM +0200, Neil Armstrong wrote:
> Hi,
>
> On 06/06/2023 17:31, Hans de Goede wrote:
> > Hi Neil,
> >
> > On 6/6/23 16:31, Neil Armstrong wrote:
> > > These touchscreen ICs support SPI, I2C and I3C interface, up to
> > > 10 finger touch, stylus and gestures events.
> > >
> > > This initial driver is derived from the Goodix goodix_ts_berlin
> > > available at [1] and [2] and only supports the GT9916 IC
> > > present on the Qualcomm SM8550 MTP & QRD touch panel.
> > >
> > > The current implementation only supports BerlinD, aka GT9916.
> > >
> > > Support for advanced features like:
> > > - Firmware & config update
> > > - Stylus events
> > > - Gestures events
> > > - Previous revisions support (BerlinA or BerlinB)
> > > is not included in current version.
> > >
> > > The current support will work with currently flashed firmware
> > > and config, and bail out if firmware or config aren't flashed yet.
> >
> > What I'm missing here / in the commit msg of
> > "input: touchscreen: add core support for Goodix Berlin Touchscreen IC"
> >
> > is an explanation why this is a new driver instead of adding
> > support to the existing goodix.c code.
> >
> > I assume you have good reasons for this, but it would be good
> > if you can write the reasons for this down.
>
> Sure, should I write it down here and/or update the commit message in a new revision ?
>
> Anyway, here's the reasons:
> - globally the event handling "looks like" the current goodix.c, but again the offsets
> are again different and none of the register address are the same, and unlike the current
> support all registers are provided by the "ic_info" structure
> - while with the current code it *could* be possible to merge it, with a lot of changes,
> the firmware management looks really different, and it would be really hard to merge.
>
> But I may be wrong, and may be misleaded by the goodix driver structure (even if it
> went through a really heavy cleaning process).
>
> Globally it seems they tried to match the "event handling" process of the previous
> generations, but the firmware interface is completely different.
It is not unprecedented for drivers to share event processing and
implement several ways/generations of firmware update mechanisms.
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH] HID: logitech-hidpp: Handle timeout differently from busy
From: Benjamin Tissoires @ 2023-06-06 18:37 UTC (permalink / raw)
To: Linux regressions mailing list
Cc: Jiri Kosina, Bastien Nocera, linux-input, linux-kernel,
Peter F . Patel-Schneider, Filipe Laíns, Nestor Lopez Casado,
Mark Lord, Greg Kroah-Hartman
In-Reply-To: <42b6e582-f642-7521-135a-449140984211@leemhuis.info>
On Tue, Jun 6, 2023 at 8:18 PM Linux regression tracking (Thorsten
Leemhuis) <regressions@leemhuis.info> wrote:
>
>
>
> On 06.06.23 15:27, Jiri Kosina wrote:
> > On Mon, 5 Jun 2023, Linux regression tracking (Thorsten Leemhuis) wrote:
> >
> >>>>> If an attempt at contacting a receiver or a device fails because the
> >>>>> receiver or device never responds, don't restart the communication, only
> >>>>> restart it if the receiver or device answers that it's busy, as originally
> >>>>> intended.
> >>>>>
> >>>>> This was the behaviour on communication timeout before commit 586e8fede795
> >>>>> ("HID: logitech-hidpp: Retry commands when device is busy").
> >>>>>
> >>>>> This fixes some overly long waits in a critical path on boot, when
> >>>>> checking whether the device is connected by getting its HID++ version.
> >>>>>
> >>>>> Signed-off-by: Bastien Nocera <hadess@hadess.net>
> >>>>> Suggested-by: Mark Lord <mlord@pobox.com>
> >>>>> Fixes: 586e8fede795 ("HID: logitech-hidpp: Retry commands when device is busy")
> >>>>> Link: https://bugzilla.kernel.org/show_bug.cgi?id=217412
> >>> [...]
> >>>>
> >>>> I have applied this even before getting confirmation from the reporters in
> >>>> bugzilla, as it's the right thing to do anyway.
> >>>
> >>> Unfortunately it doesn't seem to cure the reported issue (while reverting
> >>> 586e8fede79 does):
> >>
> >> BTW, remind me again: was fixing this by reverting 586e8fede79 for now a
> >> option? I guess it's not, but if I'm wrong I wonder if that might at
> >> this point be the best way forward.
> >
> > This should now all be fixed by
> >
> > https://git.kernel.org/linus/7c28afd5512e371773dbb2bf95a31ed5625651d9
>
> Jiri, Benjamin, many many thx for working on this.
>
> Hmmm. No CC: <stable... tag.
>
> Should we ask Greg to pick this up for 6.3 now, or better wait a few
> days? He currently already has 6199d23c91ce ("HID: logitech-hidpp:
> Handle timeout differently from busy") in his queue for the next 6.3.y
> release.
Well, the Fixes: tag supposedly is enough to let the stable folks to
pick it up. But you are right, let's Cc Greg for a quicker inclusion
in the 6.3 tree.
Greg, would you mind adding the commit above (7c28afd5512e37) onto the
6.3 stable queue? Thanks!
Cheers,
Benjamin
>
> Ciao, Thorsten
>
> P.S.: If the answer is along the lines of "let's backport this quickly",
> please consider directly CCing Greg.
>
^ permalink raw reply
* Re: [PATCH] HID: logitech-hidpp: Handle timeout differently from busy
From: Linux regression tracking (Thorsten Leemhuis) @ 2023-06-06 18:18 UTC (permalink / raw)
To: Jiri Kosina, Linux regressions mailing list
Cc: Bastien Nocera, linux-input, linux-kernel, Benjamin Tissoires,
Peter F . Patel-Schneider, Filipe Laíns, Nestor Lopez Casado,
Mark Lord
In-Reply-To: <nycvar.YFH.7.76.2306061527080.29760@cbobk.fhfr.pm>
On 06.06.23 15:27, Jiri Kosina wrote:
> On Mon, 5 Jun 2023, Linux regression tracking (Thorsten Leemhuis) wrote:
>
>>>>> If an attempt at contacting a receiver or a device fails because the
>>>>> receiver or device never responds, don't restart the communication, only
>>>>> restart it if the receiver or device answers that it's busy, as originally
>>>>> intended.
>>>>>
>>>>> This was the behaviour on communication timeout before commit 586e8fede795
>>>>> ("HID: logitech-hidpp: Retry commands when device is busy").
>>>>>
>>>>> This fixes some overly long waits in a critical path on boot, when
>>>>> checking whether the device is connected by getting its HID++ version.
>>>>>
>>>>> Signed-off-by: Bastien Nocera <hadess@hadess.net>
>>>>> Suggested-by: Mark Lord <mlord@pobox.com>
>>>>> Fixes: 586e8fede795 ("HID: logitech-hidpp: Retry commands when device is busy")
>>>>> Link: https://bugzilla.kernel.org/show_bug.cgi?id=217412
>>> [...]
>>>>
>>>> I have applied this even before getting confirmation from the reporters in
>>>> bugzilla, as it's the right thing to do anyway.
>>>
>>> Unfortunately it doesn't seem to cure the reported issue (while reverting
>>> 586e8fede79 does):
>>
>> BTW, remind me again: was fixing this by reverting 586e8fede79 for now a
>> option? I guess it's not, but if I'm wrong I wonder if that might at
>> this point be the best way forward.
>
> This should now all be fixed by
>
> https://git.kernel.org/linus/7c28afd5512e371773dbb2bf95a31ed5625651d9
Jiri, Benjamin, many many thx for working on this.
Hmmm. No CC: <stable... tag.
Should we ask Greg to pick this up for 6.3 now, or better wait a few
days? He currently already has 6199d23c91ce ("HID: logitech-hidpp:
Handle timeout differently from busy") in his queue for the next 6.3.y
release.
Ciao, Thorsten
P.S.: If the answer is along the lines of "let's backport this quickly",
please consider directly CCing Greg.
^ permalink raw reply
* Re: [PATCH RFC 0/4] input: touchscreen: add initial support for Goodix Berlin touchscreen IC
From: Neil Armstrong @ 2023-06-06 18:12 UTC (permalink / raw)
To: Hans de Goede, Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Bastien Nocera, Henrik Rydberg
Cc: linux-input, linux-arm-msm, devicetree, linux-kernel
In-Reply-To: <f5f20de8-851a-fe20-4664-62b6de14ebd7@redhat.com>
Hi,
On 06/06/2023 17:31, Hans de Goede wrote:
> Hi Neil,
>
> On 6/6/23 16:31, Neil Armstrong wrote:
>> These touchscreen ICs support SPI, I2C and I3C interface, up to
>> 10 finger touch, stylus and gestures events.
>>
>> This initial driver is derived from the Goodix goodix_ts_berlin
>> available at [1] and [2] and only supports the GT9916 IC
>> present on the Qualcomm SM8550 MTP & QRD touch panel.
>>
>> The current implementation only supports BerlinD, aka GT9916.
>>
>> Support for advanced features like:
>> - Firmware & config update
>> - Stylus events
>> - Gestures events
>> - Previous revisions support (BerlinA or BerlinB)
>> is not included in current version.
>>
>> The current support will work with currently flashed firmware
>> and config, and bail out if firmware or config aren't flashed yet.
>
> What I'm missing here / in the commit msg of
> "input: touchscreen: add core support for Goodix Berlin Touchscreen IC"
>
> is an explanation why this is a new driver instead of adding
> support to the existing goodix.c code.
>
> I assume you have good reasons for this, but it would be good
> if you can write the reasons for this down.
Sure, should I write it down here and/or update the commit message in a new revision ?
Anyway, here's the reasons:
- globally the event handling "looks like" the current goodix.c, but again the offsets
are again different and none of the register address are the same, and unlike the current
support all registers are provided by the "ic_info" structure
- while with the current code it *could* be possible to merge it, with a lot of changes,
the firmware management looks really different, and it would be really hard to merge.
But I may be wrong, and may be misleaded by the goodix driver structure (even if it
went through a really heavy cleaning process).
Globally it seems they tried to match the "event handling" process of the previous
generations, but the firmware interface is completely different.
Neil
>
> Regards,
>
> Hans
>
>
>
>>
>> [1] https://github.com/goodix/goodix_ts_berlin
>> [2] https://git.codelinaro.org/clo/la/platform/vendor/opensource/touch-drivers
>>
>> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
>> ---
>> Neil Armstrong (4):
>> dt-bindings: input: document Goodix Berlin Touchscreen IC
>> input: touchscreen: add core support for Goodix Berlin Touchscreen IC
>> input: touchscreen: add I2C support for Goodix Berlin Touchscreen IC
>> input: touchscreen: add SPI support for Goodix Berlin Touchscreen IC
>>
>> .../bindings/input/touchscreen/goodix-berlin.yaml | 81 ++
>> drivers/input/touchscreen/Kconfig | 33 +
>> drivers/input/touchscreen/Makefile | 3 +
>> drivers/input/touchscreen/goodix_berlin.h | 228 +++++
>> drivers/input/touchscreen/goodix_berlin_core.c | 935 +++++++++++++++++++++
>> drivers/input/touchscreen/goodix_berlin_i2c.c | 76 ++
>> drivers/input/touchscreen/goodix_berlin_spi.c | 183 ++++
>> 7 files changed, 1539 insertions(+)
>> ---
>> base-commit: 6db29e14f4fb7bce9eb5290288e71b05c2b0d118
>> change-id: 20230606-topic-goodix-berlin-upstream-initial-ba97e8ec8f4c
>>
>> Best regards,
>
^ permalink raw reply
* [git pull] Input updates for v6.4-rc5
From: Dmitry Torokhov @ 2023-06-06 17:58 UTC (permalink / raw)
To: Linus Torvalds; +Cc: linux-kernel, linux-input
Hi Linus,
Please pull from:
git://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git tags/input-for-v6.4-rc5
to receive updates for the input subsystem. You will receive:
- a fix for unbalanced open count for inhibited input devices
- fixups in Elantech PS/2 and Cyppress TTSP v5 drivers
- a quirk to soc_button_array driver to make it work with
Lenovo Yoga Book X90F / X90L
- a removal of erroneous entry from xpad driver.
Changelog:
---------
Dmitry Torokhov (2):
Input: fix open count when closing inhibited device
Input: psmouse - fix OOB access in Elantech protocol
Hans de Goede (1):
Input: soc_button_array - add invalid acpi_index DMI quirk handling
Ismael Ferreras Morezuelas (1):
Input: xpad - delete a Razer DeathAdder mouse VID/PID entry
Maximilian Weigand (1):
Input: cyttsp5 - fix array length
Diffstat:
--------
drivers/input/input.c | 2 +-
drivers/input/joystick/xpad.c | 1 -
drivers/input/misc/soc_button_array.c | 30 ++++++++++++++++++++++++++++++
drivers/input/mouse/elantech.c | 9 +++++----
drivers/input/touchscreen/cyttsp5.c | 2 +-
5 files changed, 37 insertions(+), 7 deletions(-)
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH v2] Input: xpad - Move Xbox 360 magic packet sending
From: Dmitry Torokhov @ 2023-06-06 17:47 UTC (permalink / raw)
To: Vicki Pfau
Cc: Dongliang Mu, linux-input, Dan Carpenter,
syzbot+a3f758b8d8cb7e49afec
In-Reply-To: <81e284e7-7d2d-6ccd-8bac-e20616ede754@endrift.com>
Hi Vicki,
On Fri, Jun 02, 2023 at 05:35:52PM -0700, Vicki Pfau wrote:
> On 5/1/23 20:12, Vicki Pfau wrote:
> > This moves the sending of the magic packet introduced in db7220c48d8d from
> > xpad_probe to xpad_start_input to ensure that xpad->dev->dev exists in the
> > event that an error occurs. This should also fix issues with suspend that may
> > occur with some controllers.
> >
> > Fixes: db7220c48d8d ("Input: xpad - fix support for some third-party controllers")
> > Reported-by: syzbot+a3f758b8d8cb7e49afec@syzkaller.appspotmail.com
> > Reported-by: Dongliang Mu <dzm91@hust.edu.cn>
> > Link: https://groups.google.com/g/syzkaller-bugs/c/iMhTgpGuIbM
> > Signed-off-by: Vicki Pfau <vi@endrift.com>
> > ---
> > drivers/input/joystick/xpad.c | 21 +++++++++++++++++++++
> > 1 file changed, 21 insertions(+)
> >
> > diff --git a/drivers/input/joystick/xpad.c b/drivers/input/joystick/xpad.c
> > index 50ecff681b89..40abea92c393 100644
> > --- a/drivers/input/joystick/xpad.c
> > +++ b/drivers/input/joystick/xpad.c
> > @@ -1720,6 +1720,27 @@ static int xpad_start_input(struct usb_xpad *xpad)
> > return error;
> > }
> > }
> > + if (xpad->xtype == XTYPE_XBOX360) {
> > + /*
> > + * Some third-party controllers Xbox 360-style controllers
> > + * require this message to finish initialization.
> > + */
> > + u8 dummy[20];
> > +
> > + error = usb_control_msg_recv(xpad->udev, 0,
> > + /* bRequest */ 0x01,
> > + /* bmRequestType */
> > + USB_TYPE_VENDOR | USB_DIR_IN |
> > + USB_RECIP_INTERFACE,
> > + /* wValue */ 0x100,
> > + /* wIndex */ 0x00,
> > + dummy, sizeof(dummy),
> > + 25, GFP_KERNEL);
> > + if (error)
> > + dev_warn(&xpad->dev->dev,
> > + "unable to receive magic message: %d\n",
> > + error);
> > + }
> >
> > return 0;
> > }
>
> It's been a month and this fixed version of the patch never got any
> replies. Did it just get overlooked? Or does the fact that the old
> version got reverted mean I need to change the description in some
> capacity?
Yes, because the original patch was reverted you need to adjust the
patch/description and resubmit as a new patch.
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH] HID: fix an error code in hid_check_device_match()
From: Dmitry Torokhov @ 2023-06-06 17:19 UTC (permalink / raw)
To: Dan Carpenter
Cc: Jiri Kosina, Benjamin Tissoires, linux-input, kernel-janitors
In-Reply-To: <ZH7sZv4PEovkMxNP@moroto>
On Tue, Jun 06, 2023 at 11:20:54AM +0300, Dan Carpenter wrote:
> The hid_check_device_match() returns true if we find a match and false
> if we don't. But here it returns -ENODEV which becomes true instead
> of false.
>
> Fixes: 207733f92661 ("HID: split apart hid_device_probe to make logic more apparent")
> Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
> ---
> drivers/hid/hid-core.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
> index 4b7062dcefec..8992e3c1e769 100644
> --- a/drivers/hid/hid-core.c
> +++ b/drivers/hid/hid-core.c
> @@ -2593,7 +2593,7 @@ static bool hid_check_device_match(struct hid_device *hdev,
> {
> *id = hid_match_device(hdev, hdrv);
> if (!*id)
> - return -ENODEV;
> + return false;
Ugh, I goofed up, thank you for noticing this Dan.
Reviewed-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH RFC 0/4] input: touchscreen: add initial support for Goodix Berlin touchscreen IC
From: Hans de Goede @ 2023-06-06 15:31 UTC (permalink / raw)
To: Neil Armstrong, Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Bastien Nocera, Henrik Rydberg
Cc: linux-input, linux-arm-msm, devicetree, linux-kernel
In-Reply-To: <20230606-topic-goodix-berlin-upstream-initial-v1-0-4a0741b8aefd@linaro.org>
Hi Neil,
On 6/6/23 16:31, Neil Armstrong wrote:
> These touchscreen ICs support SPI, I2C and I3C interface, up to
> 10 finger touch, stylus and gestures events.
>
> This initial driver is derived from the Goodix goodix_ts_berlin
> available at [1] and [2] and only supports the GT9916 IC
> present on the Qualcomm SM8550 MTP & QRD touch panel.
>
> The current implementation only supports BerlinD, aka GT9916.
>
> Support for advanced features like:
> - Firmware & config update
> - Stylus events
> - Gestures events
> - Previous revisions support (BerlinA or BerlinB)
> is not included in current version.
>
> The current support will work with currently flashed firmware
> and config, and bail out if firmware or config aren't flashed yet.
What I'm missing here / in the commit msg of
"input: touchscreen: add core support for Goodix Berlin Touchscreen IC"
is an explanation why this is a new driver instead of adding
support to the existing goodix.c code.
I assume you have good reasons for this, but it would be good
if you can write the reasons for this down.
Regards,
Hans
>
> [1] https://github.com/goodix/goodix_ts_berlin
> [2] https://git.codelinaro.org/clo/la/platform/vendor/opensource/touch-drivers
>
> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
> ---
> Neil Armstrong (4):
> dt-bindings: input: document Goodix Berlin Touchscreen IC
> input: touchscreen: add core support for Goodix Berlin Touchscreen IC
> input: touchscreen: add I2C support for Goodix Berlin Touchscreen IC
> input: touchscreen: add SPI support for Goodix Berlin Touchscreen IC
>
> .../bindings/input/touchscreen/goodix-berlin.yaml | 81 ++
> drivers/input/touchscreen/Kconfig | 33 +
> drivers/input/touchscreen/Makefile | 3 +
> drivers/input/touchscreen/goodix_berlin.h | 228 +++++
> drivers/input/touchscreen/goodix_berlin_core.c | 935 +++++++++++++++++++++
> drivers/input/touchscreen/goodix_berlin_i2c.c | 76 ++
> drivers/input/touchscreen/goodix_berlin_spi.c | 183 ++++
> 7 files changed, 1539 insertions(+)
> ---
> base-commit: 6db29e14f4fb7bce9eb5290288e71b05c2b0d118
> change-id: 20230606-topic-goodix-berlin-upstream-initial-ba97e8ec8f4c
>
> Best regards,
^ permalink raw reply
* Re: amd_sfh driver causes kernel oops during boot
From: Limonciello, Mario @ 2023-06-06 15:25 UTC (permalink / raw)
To: Benjamin Tissoires, Linux regressions mailing list
Cc: Bagas Sanjaya, Malte Starostik, basavaraj.natikar, linux-input,
linux, stable
In-Reply-To: <aci7a4jnosozypn6sffsdoaezg4p42zgjy5dwnjyvnbav7chdm@wettfjwb4enw>
On 6/6/2023 3:08 AM, Benjamin Tissoires wrote:
> On Jun 06 2023, Linux regression tracking (Thorsten Leemhuis) wrote:
>> On 06.06.23 04:36, Bagas Sanjaya wrote:
>>> On Mon, Jun 05, 2023 at 01:24:25PM +0200, Malte Starostik wrote:
>>>> Hello,
>>>>
>>>> chiming in here as I'm experiencing what looks like the exact same issue, also
>>>> on a Lenovo Z13 notebook, also on Arch:
>>>> Oops during startup in task udev-worker followed by udev-worker blocking all
>>>> attempts to suspend or cleanly shutdown/reboot the machine - in fact I first
>>>> noticed because the machine surprised with repeatedly running out of battery
>>>> after it had supposedly been in standby but couldn't. Only then I noticed the
>>>> error on boot.
>>>>
>>>> bisect result:
>>>> 904e28c6de083fa4834cdbd0026470ddc30676fc is the first bad commit
>>>> commit 904e28c6de083fa4834cdbd0026470ddc30676fc
>>>> Merge: a738688177dc 2f7f4efb9411
>>>> Author: Benjamin Tissoires <benjamin.tissoires@redhat.com>
>>>> Date: Wed Feb 22 10:44:31 2023 +0100
>>>>
>>>> Merge branch 'for-6.3/hid-bpf' into for-linus
>>> Hmm, seems like bad bisect (bisected to HID-BPF which IMO isn't related
>>> to amd_sfh). Can you repeat the bisection?
>> Well, amd_sfh afaics apparently interacts with HID (see trace earlier in
>> the thread), so it's not that far away. But it's a merge commit, which
>> is possible, but doesn't happen every day. So a recheck might really be
>> a good idea.
> Let's not rule out that there is a bad interaction between HID-BPF and
> AMD SFH. HID-BPF is able to process any incoming HID event, whether it
> comes from AND SFH, USB, BT, I2C or anything else.
>
> However, looking at the stack trace in the initial report[0], it seems
> we are getting the oops/stack traces while we are still in amd_sfh:
>
> list_add corruption. next is NULL.
> WARNING: CPU: 5 PID: 433 at lib/list_debug.c:25 __list_add_valid+0x57/0xa0
> ...
> RIP: 0010:__list_add_valid+0x57/0xa0
> ...
> Call Trace:
> <TASK>
> amd_sfh_get_report+0xba/0x110 [amd_sfh 78bf82e66cdb2ccf24cbe871a0835ef4eedddb17]
> ...
>
> If HID-BPF were involved, we should see a call to hid_input_report() IMO.
> Also AMD SFH calls hid_input_report() in a workqueue, so I would expect
> a different stack trace.
>
> I have a suspicion on commit 7bcfdab3f0c6 ("HID: amd_sfh: if no sensors are enabled,
> clean up") because the stack trace says that there is a bad list_add,
> which could happen if the object is not correctly initialized.
>
> However, that commit was present in v6.2, so it might not be that one.
>
> Back to the merge commit: the hid-bpf tree was merged in the hid tree
> while it took its branch during the v6.1 cycle. So that might be the
> reason you get this as a result of bisection because the AMD SFH code in
> the hid-bpf branch is the one from the v6.1 kernel, and when you merge
> it to the v6.2+ branch, you get a different code for that driver.
>
> Cheers,
> Benjamin
>
> [0] https://lore.kernel.org/regressions/f40e3897-76f1-2cd0-2d83-e48d87130eab@hexchain.org/#t
If I'm not mistaken the Z13 doesn't actually have any
sensors connected to SFH. So I think the suspicion on
7bcfdab3f0c6 and theory this is triggered by HID init makes
a lot of sense.
Can you try this patch?
diff --git a/drivers/hid/amd-sfh-hid/amd_sfh_client.c
b/drivers/hid/amd-sfh-hid/amd_sfh_client.c
index d9b7b01900b5..fa693a5224c6 100644
--- a/drivers/hid/amd-sfh-hid/amd_sfh_client.c
+++ b/drivers/hid/amd-sfh-hid/amd_sfh_client.c
@@ -324,6 +324,7 @@ int amd_sfh_hid_client_init(struct amd_mp2_dev
*privdata)
devm_kfree(dev, cl_data->report_descr[i]);
}
dev_warn(dev, "Failed to discover, sensors not enabled
is %d\n", cl_data->is_any_sensor_enabled);
+ cl_data->num_hid_devices = 0;
return -EOPNOTSUPP;
}
schedule_delayed_work(&cl_data->work_buffer,
msecs_to_jiffies(AMD_SFH_IDLE_LOOP));
^ permalink raw reply related
* [PATCH RFC 2/4] input: touchscreen: add core support for Goodix Berlin Touchscreen IC
From: Neil Armstrong @ 2023-06-06 14:31 UTC (permalink / raw)
To: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Bastien Nocera, Hans de Goede, Henrik Rydberg
Cc: linux-input, linux-arm-msm, devicetree, linux-kernel,
Neil Armstrong
In-Reply-To: <20230606-topic-goodix-berlin-upstream-initial-v1-0-4a0741b8aefd@linaro.org>
Add initial support for the new Goodix "Berlin" touchscreen ICs.
These touchscreen ICs support SPI, I2C and I3C interface, up to
10 finger touch, stylus and gestures events.
This initial driver is derived from the Goodix goodix_ts_berlin
available at [1] and [2] and only supports the GT9916 IC
present on the Qualcomm SM8550 MTP & QRD touch panel.
The current implementation only supports BerlinD, aka GT9916.
Support for advanced features like:
- Firmware & config update
- Stylus events
- Gestures events
- Previous revisions support (BerlinA or BerlinB)
is not included in current version.
The current support will work with currently flashed firmware
and config, and bail out if firmware or config aren't flashed yet.
[1] https://github.com/goodix/goodix_ts_berlin
[2] https://git.codelinaro.org/clo/la/platform/vendor/opensource/touch-drivers
Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
---
drivers/input/touchscreen/Kconfig | 5 +
drivers/input/touchscreen/Makefile | 1 +
drivers/input/touchscreen/goodix_berlin.h | 228 ++++++
drivers/input/touchscreen/goodix_berlin_core.c | 935 +++++++++++++++++++++++++
4 files changed, 1169 insertions(+)
diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig
index c2cbd332af1d..1a6f6f6da991 100644
--- a/drivers/input/touchscreen/Kconfig
+++ b/drivers/input/touchscreen/Kconfig
@@ -416,6 +416,11 @@ config TOUCHSCREEN_GOODIX
To compile this driver as a module, choose M here: the
module will be called goodix.
+config TOUCHSCREEN_GOODIX_BERLIN_CORE
+ depends on GPIOLIB || COMPILE_TEST
+ depends on REGMAP
+ tristate
+
config TOUCHSCREEN_HIDEEP
tristate "HiDeep Touch IC"
depends on I2C
diff --git a/drivers/input/touchscreen/Makefile b/drivers/input/touchscreen/Makefile
index 159cd5136fdb..29cdb042e104 100644
--- a/drivers/input/touchscreen/Makefile
+++ b/drivers/input/touchscreen/Makefile
@@ -47,6 +47,7 @@ obj-$(CONFIG_TOUCHSCREEN_EGALAX_SERIAL) += egalax_ts_serial.o
obj-$(CONFIG_TOUCHSCREEN_EXC3000) += exc3000.o
obj-$(CONFIG_TOUCHSCREEN_FUJITSU) += fujitsu_ts.o
obj-$(CONFIG_TOUCHSCREEN_GOODIX) += goodix_ts.o
+obj-$(CONFIG_TOUCHSCREEN_GOODIX_BERLIN_CORE) += goodix_berlin_core.o
obj-$(CONFIG_TOUCHSCREEN_HIDEEP) += hideep.o
obj-$(CONFIG_TOUCHSCREEN_HYNITRON_CSTXXX) += hynitron_cstxxx.o
obj-$(CONFIG_TOUCHSCREEN_ILI210X) += ili210x.o
diff --git a/drivers/input/touchscreen/goodix_berlin.h b/drivers/input/touchscreen/goodix_berlin.h
new file mode 100644
index 000000000000..83f16201c354
--- /dev/null
+++ b/drivers/input/touchscreen/goodix_berlin.h
@@ -0,0 +1,228 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * Goodix Touchscreen Driver
+ * Copyright (C) 2020 - 2021 Goodix, Inc.
+ * Copyright (C) 2023 Linaro Ltd.
+ *
+ * Based on goodix_berlin_berlin driver.
+ */
+
+#ifndef __GOODIX_BERLIN_H_
+#define __GOODIX_BERLIN_H_
+
+#include <linux/input.h>
+#include <linux/of_gpio.h>
+#include <linux/input/touchscreen.h>
+#include <linux/regulator/consumer.h>
+
+#define GOODIX_MAX_TOUCH 10
+#define GOODIX_MAX_FRAMEDATA_LEN 1700
+
+#define GOODIX_NORMAL_RESET_DELAY_MS 100
+#define GOODIX_HOLD_CPU_RESET_DELAY_MS 5
+
+enum CHECKSUM_MODE {
+ CHECKSUM_MODE_U8_LE,
+ CHECKSUM_MODE_U16_LE,
+};
+
+#define MAX_SCAN_FREQ_NUM 8
+#define MAX_SCAN_RATE_NUM 8
+#define MAX_FREQ_NUM_STYLUS 8
+
+#define IRQ_EVENT_HEAD_LEN 8
+#define BYTES_PER_POINT 8
+#define COOR_DATA_CHECKSUM_SIZE 2
+
+#define GOODIX_TOUCH_EVENT BIT(7)
+#define GOODIX_REQUEST_EVENT BIT(6)
+#define GOODIX_GESTURE_EVENT BIT(5)
+
+#define POINT_TYPE_STYLUS_HOVER 0x01
+#define POINT_TYPE_STYLUS 0x03
+
+#define DEV_CONFIRM_VAL 0xAA
+#define BOOTOPTION_ADDR 0x10000
+#define FW_VERSION_INFO_ADDR 0x10014
+
+#define GOODIX_IC_INFO_MAX_LEN 1024
+#define GOODIX_IC_INFO_ADDR 0x10070
+
+enum brl_request_code {
+ BRL_REQUEST_CODE_CONFIG = 1,
+ BRL_REQUEST_CODE_REF_ERR = 2,
+ BRL_REQUEST_CODE_RESET = 3,
+ BRL_REQUEST_CODE_CLOCK = 4,
+};
+
+struct goodix_fw_version {
+ u8 rom_pid[6]; /* rom PID */
+ u8 rom_vid[3]; /* Mask VID */
+ u8 rom_vid_reserved;
+ u8 patch_pid[8]; /* Patch PID */
+ u8 patch_vid[4]; /* Patch VID */
+ u8 patch_vid_reserved;
+ u8 sensor_id;
+ u8 reserved[2];
+ u16 checksum;
+} __packed;
+
+struct goodix_ic_info_version {
+ u8 info_customer_id;
+ u8 info_version_id;
+ u8 ic_die_id;
+ u8 ic_version_id;
+ u32 config_id;
+ u8 config_version;
+ u8 frame_data_customer_id;
+ u8 frame_data_version_id;
+ u8 touch_data_customer_id;
+ u8 touch_data_version_id;
+ u8 reserved[3];
+} __packed;
+
+struct goodix_ic_info_feature { /* feature info*/
+ u16 freqhop_feature;
+ u16 calibration_feature;
+ u16 gesture_feature;
+ u16 side_touch_feature;
+ u16 stylus_feature;
+} __packed;
+
+struct goodix_ic_info_param { /* param */
+ u8 drv_num;
+ u8 sen_num;
+ u8 button_num;
+ u8 force_num;
+ u8 active_scan_rate_num;
+ u16 active_scan_rate[MAX_SCAN_RATE_NUM];
+ u8 mutual_freq_num;
+ u16 mutual_freq[MAX_SCAN_FREQ_NUM];
+ u8 self_tx_freq_num;
+ u16 self_tx_freq[MAX_SCAN_FREQ_NUM];
+ u8 self_rx_freq_num;
+ u16 self_rx_freq[MAX_SCAN_FREQ_NUM];
+ u8 stylus_freq_num;
+ u16 stylus_freq[MAX_FREQ_NUM_STYLUS];
+} __packed;
+
+struct goodix_ic_info_misc { /* other data */
+ u32 cmd_addr;
+ u16 cmd_max_len;
+ u32 cmd_reply_addr;
+ u16 cmd_reply_len;
+ u32 fw_state_addr;
+ u16 fw_state_len;
+ u32 fw_buffer_addr;
+ u16 fw_buffer_max_len;
+ u32 frame_data_addr;
+ u16 frame_data_head_len;
+ u16 fw_attr_len;
+ u16 fw_log_len;
+ u8 pack_max_num;
+ u8 pack_compress_version;
+ u16 stylus_struct_len;
+ u16 mutual_struct_len;
+ u16 self_struct_len;
+ u16 noise_struct_len;
+ u32 touch_data_addr;
+ u16 touch_data_head_len;
+ u16 point_struct_len;
+ u16 reserved1;
+ u16 reserved2;
+ u32 mutual_rawdata_addr;
+ u32 mutual_diffdata_addr;
+ u32 mutual_refdata_addr;
+ u32 self_rawdata_addr;
+ u32 self_diffdata_addr;
+ u32 self_refdata_addr;
+ u32 iq_rawdata_addr;
+ u32 iq_refdata_addr;
+ u32 im_rawdata_addr;
+ u16 im_readata_len;
+ u32 noise_rawdata_addr;
+ u16 noise_rawdata_len;
+ u32 stylus_rawdata_addr;
+ u16 stylus_rawdata_len;
+ u32 noise_data_addr;
+ u32 esd_addr;
+} __packed;
+
+struct goodix_ic_info {
+ u16 length;
+ struct goodix_ic_info_version version;
+ struct goodix_ic_info_feature feature;
+ struct goodix_ic_info_param parm;
+ struct goodix_ic_info_misc misc;
+} __packed;
+
+/* interrupt event type */
+enum ts_event_type {
+ EVENT_INVALID,
+ EVENT_TOUCH, /* finger touch event */
+ EVENT_REQUEST,
+};
+
+enum ts_request_type {
+ REQUEST_TYPE_RESET,
+};
+
+enum touch_point_status {
+ TS_NONE,
+ TS_RELEASE,
+ TS_TOUCH,
+};
+
+/* coordinate package */
+struct goodix_berlin_coords {
+ int status; /* NONE, RELEASE, TOUCH */
+ unsigned int x, y, w, p;
+};
+
+/* touch event data */
+struct goodix_touch_data {
+ int touch_num;
+ struct goodix_berlin_coords coords[GOODIX_MAX_TOUCH];
+};
+
+/* touch event struct */
+struct goodix_berlin_event {
+ enum ts_event_type event_type;
+ u8 request_code; /* represent the request type */
+ struct goodix_touch_data touch_data;
+};
+
+struct goodix_berlin_core {
+ struct device *dev;
+ struct regmap *regmap;
+
+ struct regulator *avdd;
+ struct regulator *iovdd;
+ struct gpio_desc *reset_gpio;
+
+ struct touchscreen_properties props;
+
+ struct goodix_fw_version fw_version;
+ struct goodix_ic_info ic_info;
+
+ struct input_dev *input_dev;
+ struct goodix_berlin_event ts_event;
+
+ int irq;
+
+ struct dentry *debugfs_root;
+};
+
+u32 goodix_append_checksum(u8 *data, int len, int mode);
+int checksum_cmp(const u8 *data, int size, int mode);
+int is_risk_data(const u8 *data, int size);
+u32 goodix_get_file_config_id(u8 *ic_config);
+void goodix_rotate_abcd2cbad(int tx, int rx, s16 *data);
+
+int goodix_berlin_probe(struct device *dev, int irq, const struct input_id *id,
+ struct regmap *regmap);
+int goodix_berlin_remove(struct device *dev);
+
+extern const struct dev_pm_ops goodix_berlin_pm_ops;
+
+#endif
diff --git a/drivers/input/touchscreen/goodix_berlin_core.c b/drivers/input/touchscreen/goodix_berlin_core.c
new file mode 100644
index 000000000000..0719690a5bd3
--- /dev/null
+++ b/drivers/input/touchscreen/goodix_berlin_core.c
@@ -0,0 +1,935 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Goodix Touchscreen Driver
+ * Copyright (C) 2020 - 2021 Goodix, Inc.
+ * Copyright (C) 2023 Linaro Ltd.
+ *
+ * Based on goodix_ts_berlin driver.
+ */
+#include <linux/regmap.h>
+#include <linux/input/mt.h>
+#include <linux/input/touchscreen.h>
+#include <linux/debugfs.h>
+
+#include "goodix_berlin.h"
+
+/*
+ * Goodix "Berlin" Touchscreen ID driver
+ *
+ * Currently only handles Multitouch events with already
+ * programmed firmware and "config" for "Revision D" Berlin IC.
+ *
+ * Support is missing for:
+ * - ESD Management
+ * - Firmware update/flashing
+ * - "Config" update/flashing
+ * - PEN Events
+ * - Gesture Events
+ * - Support for older revisions (A, B & C)
+ */
+
+static bool goodix_berlin_check_checksum(const u8 *data, int size)
+{
+ u32 cal_checksum = 0;
+ u32 r_checksum = 0;
+ u32 i;
+
+ if (size < COOR_DATA_CHECKSUM_SIZE)
+ return false;
+
+ for (i = 0; i < size - COOR_DATA_CHECKSUM_SIZE; i++)
+ cal_checksum += data[i];
+
+ r_checksum += data[i++];
+ r_checksum += (data[i] << 8);
+
+ return (cal_checksum & 0xFFFF) == r_checksum;
+}
+
+static bool goodix_berlin_is_risk_data(struct goodix_berlin_core *cd,
+ const u8 *data, int size)
+{
+ int zero_count = 0;
+ int ff_count = 0;
+ int i;
+
+ for (i = 0; i < size; i++) {
+ if (data[i] == 0)
+ zero_count++;
+ else if (data[i] == 0xff)
+ ff_count++;
+ }
+ if (zero_count == size || ff_count == size) {
+ dev_warn(cd->dev, "warning data is all %s\n",
+ zero_count == size ? "zero" : "0xff");
+ return true;
+ }
+
+ return false;
+}
+
+static int goodix_berlin_dev_confirm(struct goodix_berlin_core *cd)
+{
+ u8 tx_buf[8] = { 0 };
+ u8 rx_buf[8] = { 0 };
+ int retry = 3;
+ int ret = 0;
+
+ memset(tx_buf, DEV_CONFIRM_VAL, sizeof(tx_buf));
+ while (retry--) {
+ ret = regmap_raw_write(cd->regmap, BOOTOPTION_ADDR, tx_buf,
+ sizeof(tx_buf));
+ if (ret < 0)
+ return ret;
+ ret = regmap_raw_read(cd->regmap, BOOTOPTION_ADDR, rx_buf,
+ sizeof(rx_buf));
+ if (ret < 0)
+ return ret;
+ if (!memcmp(tx_buf, rx_buf, sizeof(tx_buf)))
+ break;
+ usleep_range(5000, 5100);
+ }
+
+ if (retry < 0) {
+ ret = -EINVAL;
+ dev_err(cd->dev, "device confirm failed, rx_buf: %*ph\n", 8, rx_buf);
+ }
+
+ return ret;
+}
+
+static int goodix_berlin_power_on(struct goodix_berlin_core *cd, bool on)
+{
+ int ret = 0;
+
+ if (on) {
+ ret = regulator_enable(cd->iovdd);
+ if (ret < 0) {
+ dev_err(cd->dev, "Failed to enable iovdd: %d\n", ret);
+ goto power_off;
+ }
+
+ usleep_range(3000, 3100);
+
+ ret = regulator_enable(cd->avdd);
+ if (ret < 0) {
+ dev_err(cd->dev, "Failed to enable avdd: %d\n", ret);
+ goto power_off;
+ }
+
+ usleep_range(15000, 15100);
+
+ gpiod_set_value(cd->reset_gpio, 0);
+
+ usleep_range(4000, 4100);
+
+ ret = goodix_berlin_dev_confirm(cd);
+ if (ret < 0)
+ goto power_off;
+
+ msleep(GOODIX_NORMAL_RESET_DELAY_MS);
+
+ return 0;
+ }
+
+power_off:
+ gpiod_set_value(cd->reset_gpio, 1);
+ regulator_disable(cd->iovdd);
+ regulator_disable(cd->avdd);
+ return ret;
+}
+
+static int goodix_berlin_do_suspend(struct goodix_berlin_core *cd)
+{
+ if (goodix_berlin_power_on(cd, 0))
+ dev_err(cd->dev, "failed power off\n");
+
+ return 0;
+}
+
+static int goodix_berlin_do_resume(struct goodix_berlin_core *cd)
+{
+ int ret = 0;
+
+ ret = goodix_berlin_power_on(cd, 1);
+ if (ret) {
+ dev_err(cd->dev, "failed power on\n");
+ return ret;
+ }
+
+ return ret;
+}
+
+static int goodix_berlin_reset(struct goodix_berlin_core *cd, int delay)
+{
+ gpiod_set_value(cd->reset_gpio, 1);
+ usleep_range(2000, 2100);
+ gpiod_set_value(cd->reset_gpio, 0);
+
+ if (delay < 20)
+ usleep_range(delay * 1000, delay * 1000 + 100);
+ else
+ msleep(delay);
+
+ return 0;
+}
+
+static int goodix_berlin_irq_enable(struct goodix_berlin_core *cd, bool enable)
+{
+ if (enable)
+ enable_irq(cd->irq);
+ else
+ disable_irq(cd->irq);
+
+ return 0;
+}
+
+static int goodix_berlin_read_version(struct goodix_berlin_core *cd,
+ struct goodix_fw_version *version)
+{
+ u8 buf[sizeof(struct goodix_fw_version)] = { 0 };
+ int ret, i;
+
+ for (i = 0; i < 2; i++) {
+ ret = regmap_raw_read(cd->regmap, FW_VERSION_INFO_ADDR, buf, sizeof(buf));
+ if (ret) {
+ dev_dbg(cd->dev, "read fw version: %d, retry %d\n", ret, i);
+ usleep_range(5000, 5100);
+ continue;
+ }
+
+ if (goodix_berlin_check_checksum(buf, sizeof(buf)))
+ break;
+
+ dev_dbg(cd->dev, "invalid fw version: checksum error\n");
+
+ ret = -EINVAL;
+ usleep_range(10000, 11000);
+ }
+
+ if (ret) {
+ dev_err(cd->dev, "failed to get fw version");
+ return ret;
+ }
+
+ memcpy(version, buf, sizeof(*version));
+
+ return 0;
+}
+
+static int goodix_berlin_fw_version_show(struct seq_file *s, void *unused)
+{
+ struct goodix_berlin_core *cd = s->private;
+ struct goodix_fw_version *version = &cd->fw_version;
+ u8 temp_pid[9] = { 0 };
+
+ memcpy(temp_pid, version->rom_pid, sizeof(version->rom_pid));
+ temp_pid[sizeof(version->rom_pid)] = '\0';
+
+ seq_printf(s, "rom_pid: %s\n", temp_pid);
+ seq_printf(s, "rom_vid: %*ph\n",
+ (int)sizeof(version->rom_vid),
+ version->rom_vid);
+
+ memcpy(temp_pid, version->patch_pid, sizeof(version->patch_pid));
+ temp_pid[sizeof(version->patch_pid)] = '\0';
+
+ seq_printf(s, "patch_pid: %s\n", temp_pid);
+ seq_printf(s, "patch_vid: %*ph\n",
+ (int)sizeof(version->patch_vid),
+ version->patch_vid);
+ seq_printf(s, "sensor_id: %d\n",
+ version->sensor_id);
+
+ return 0;
+}
+
+DEFINE_SHOW_ATTRIBUTE(goodix_berlin_fw_version);
+
+static int goodix_berlin_convert_ic_info(struct goodix_berlin_core *cd,
+ struct goodix_ic_info *info,
+ const u8 *data)
+{
+ int i;
+ struct goodix_ic_info_version *version = &info->version;
+ struct goodix_ic_info_feature *feature = &info->feature;
+ struct goodix_ic_info_param *parm = &info->parm;
+ struct goodix_ic_info_misc *misc = &info->misc;
+
+ info->length = le16_to_cpup((__le16 *)data);
+
+ data += 2;
+ memcpy(version, data, sizeof(*version));
+ version->config_id = le32_to_cpu(version->config_id);
+
+ data += sizeof(struct goodix_ic_info_version);
+ memcpy(feature, data, sizeof(*feature));
+ feature->freqhop_feature = le16_to_cpu(feature->freqhop_feature);
+ feature->calibration_feature =
+ le16_to_cpu(feature->calibration_feature);
+ feature->gesture_feature = le16_to_cpu(feature->gesture_feature);
+ feature->side_touch_feature = le16_to_cpu(feature->side_touch_feature);
+ feature->stylus_feature = le16_to_cpu(feature->stylus_feature);
+
+ data += sizeof(struct goodix_ic_info_feature);
+ parm->drv_num = *(data++);
+ parm->sen_num = *(data++);
+ parm->button_num = *(data++);
+ parm->force_num = *(data++);
+ parm->active_scan_rate_num = *(data++);
+
+ if (parm->active_scan_rate_num > MAX_SCAN_RATE_NUM) {
+ dev_err(cd->dev, "invalid scan rate num %d > %d\n",
+ parm->active_scan_rate_num, MAX_SCAN_RATE_NUM);
+ return -EINVAL;
+ }
+
+ for (i = 0; i < parm->active_scan_rate_num; i++)
+ parm->active_scan_rate[i] =
+ le16_to_cpup((__le16 *)(data + i * 2));
+
+ data += parm->active_scan_rate_num * 2;
+ parm->mutual_freq_num = *(data++);
+
+ if (parm->mutual_freq_num > MAX_SCAN_FREQ_NUM) {
+ dev_err(cd->dev, "invalid mntual freq num %d > %d\n",
+ parm->mutual_freq_num, MAX_SCAN_FREQ_NUM);
+ return -EINVAL;
+ }
+
+ for (i = 0; i < parm->mutual_freq_num; i++)
+ parm->mutual_freq[i] = le16_to_cpup((__le16 *)(data + i * 2));
+
+ data += parm->mutual_freq_num * 2;
+ parm->self_tx_freq_num = *(data++);
+
+ if (parm->self_tx_freq_num > MAX_SCAN_FREQ_NUM) {
+ dev_err(cd->dev, "invalid tx freq num %d > %d\n",
+ parm->self_tx_freq_num, MAX_SCAN_FREQ_NUM);
+ return -EINVAL;
+ }
+
+ for (i = 0; i < parm->self_tx_freq_num; i++)
+ parm->self_tx_freq[i] = le16_to_cpup((__le16 *)(data + i * 2));
+
+ data += parm->self_tx_freq_num * 2;
+ parm->self_rx_freq_num = *(data++);
+
+ if (parm->self_rx_freq_num > MAX_SCAN_FREQ_NUM) {
+ dev_err(cd->dev, "invalid rx freq num %d > %d\n",
+ parm->self_rx_freq_num, MAX_SCAN_FREQ_NUM);
+ return -EINVAL;
+ }
+
+ for (i = 0; i < parm->self_rx_freq_num; i++)
+ parm->self_rx_freq[i] = le16_to_cpup((__le16 *)(data + i * 2));
+
+ data += parm->self_rx_freq_num * 2;
+ parm->stylus_freq_num = *(data++);
+
+ if (parm->stylus_freq_num > MAX_FREQ_NUM_STYLUS) {
+ dev_err(cd->dev, "invalid stylus freq num %d > %d\n",
+ parm->stylus_freq_num, MAX_FREQ_NUM_STYLUS);
+ return -EINVAL;
+ }
+
+ for (i = 0; i < parm->stylus_freq_num; i++)
+ parm->stylus_freq[i] = le16_to_cpup((__le16 *)(data + i * 2));
+
+ data += parm->stylus_freq_num * 2;
+ memcpy(misc, data, sizeof(*misc));
+ misc->cmd_addr = le32_to_cpu(misc->cmd_addr);
+ misc->cmd_max_len = le16_to_cpu(misc->cmd_max_len);
+ misc->cmd_reply_addr = le32_to_cpu(misc->cmd_reply_addr);
+ misc->cmd_reply_len = le16_to_cpu(misc->cmd_reply_len);
+ misc->fw_state_addr = le32_to_cpu(misc->fw_state_addr);
+ misc->fw_state_len = le16_to_cpu(misc->fw_state_len);
+ misc->fw_buffer_addr = le32_to_cpu(misc->fw_buffer_addr);
+ misc->fw_buffer_max_len = le16_to_cpu(misc->fw_buffer_max_len);
+ misc->frame_data_addr = le32_to_cpu(misc->frame_data_addr);
+ misc->frame_data_head_len = le16_to_cpu(misc->frame_data_head_len);
+
+ misc->fw_attr_len = le16_to_cpu(misc->fw_attr_len);
+ misc->fw_log_len = le16_to_cpu(misc->fw_log_len);
+ misc->stylus_struct_len = le16_to_cpu(misc->stylus_struct_len);
+ misc->mutual_struct_len = le16_to_cpu(misc->mutual_struct_len);
+ misc->self_struct_len = le16_to_cpu(misc->self_struct_len);
+ misc->noise_struct_len = le16_to_cpu(misc->noise_struct_len);
+ misc->touch_data_addr = le32_to_cpu(misc->touch_data_addr);
+ misc->touch_data_head_len = le16_to_cpu(misc->touch_data_head_len);
+ misc->point_struct_len = le16_to_cpu(misc->point_struct_len);
+ misc->mutual_rawdata_addr = le32_to_cpu(misc->mutual_rawdata_addr);
+ misc->mutual_diffdata_addr = le32_to_cpu(misc->mutual_diffdata_addr);
+ misc->mutual_refdata_addr = le32_to_cpu(misc->mutual_refdata_addr);
+ misc->self_rawdata_addr = le32_to_cpu(misc->self_rawdata_addr);
+ misc->self_diffdata_addr = le32_to_cpu(misc->self_diffdata_addr);
+ misc->self_refdata_addr = le32_to_cpu(misc->self_refdata_addr);
+ misc->iq_rawdata_addr = le32_to_cpu(misc->iq_rawdata_addr);
+ misc->iq_refdata_addr = le32_to_cpu(misc->iq_refdata_addr);
+ misc->im_rawdata_addr = le32_to_cpu(misc->im_rawdata_addr);
+ misc->im_readata_len = le16_to_cpu(misc->im_readata_len);
+ misc->noise_rawdata_addr = le32_to_cpu(misc->noise_rawdata_addr);
+ misc->noise_rawdata_len = le16_to_cpu(misc->noise_rawdata_len);
+ misc->stylus_rawdata_addr = le32_to_cpu(misc->stylus_rawdata_addr);
+ misc->stylus_rawdata_len = le16_to_cpu(misc->stylus_rawdata_len);
+ misc->noise_data_addr = le32_to_cpu(misc->noise_data_addr);
+ misc->esd_addr = le32_to_cpu(misc->esd_addr);
+
+ return 0;
+}
+
+/* Convert to debugfs */
+static int goodix_berlin_ic_info_show(struct seq_file *s, void *unused)
+{
+ struct goodix_berlin_core *cd = s->private;
+ struct goodix_ic_info_version *version = &cd->ic_info.version;
+ struct goodix_ic_info_feature *feature = &cd->ic_info.feature;
+ struct goodix_ic_info_param *parm = &cd->ic_info.parm;
+ struct goodix_ic_info_misc *misc = &cd->ic_info.misc;
+
+ seq_printf(s, "ic_info_length: %d\n", cd->ic_info.length);
+ seq_printf(s, "info_customer_id: 0x%01X\n",
+ version->info_customer_id);
+ seq_printf(s, "info_version_id: 0x%01X\n",
+ version->info_version_id);
+ seq_printf(s, "ic_die_id: 0x%01X\n",
+ version->ic_die_id);
+ seq_printf(s, "ic_version_id: 0x%01X\n",
+ version->ic_version_id);
+ seq_printf(s, "config_id: 0x%4X\n",
+ version->config_id);
+ seq_printf(s, "config_version: 0x%01X\n",
+ version->config_version);
+ seq_printf(s, "frame_data_customer_id: 0x%01X\n",
+ version->frame_data_customer_id);
+ seq_printf(s, "frame_data_version_id: 0x%01X\n",
+ version->frame_data_version_id);
+ seq_printf(s, "touch_data_customer_id: 0x%01X\n",
+ version->touch_data_customer_id);
+ seq_printf(s, "touch_data_version_id: 0x%01X\n",
+ version->touch_data_version_id);
+ seq_printf(s, "freqhop_feature: 0x%04X\n",
+ feature->freqhop_feature);
+ seq_printf(s, "calibration_feature: 0x%04X\n",
+ feature->calibration_feature);
+ seq_printf(s, "gesture_feature: 0x%04X\n",
+ feature->gesture_feature);
+ seq_printf(s, "side_touch_feature: 0x%04X\n",
+ feature->side_touch_feature);
+ seq_printf(s, "stylus_feature: 0x%04X\n",
+ feature->stylus_feature);
+ seq_printf(s, "Drv*Sen,Button,Force num: %d x %d, %d, %d\n",
+ parm->drv_num, parm->sen_num, parm->button_num, parm->force_num);
+ seq_printf(s, "Cmd: 0x%04X, %d\n",
+ misc->cmd_addr, misc->cmd_max_len);
+ seq_printf(s, "Cmd-Reply: 0x%04X, %d\n",
+ misc->cmd_reply_addr, misc->cmd_reply_len);
+ seq_printf(s, "FW-State: 0x%04X, %d\n",
+ misc->fw_state_addr, misc->fw_state_len);
+ seq_printf(s, "FW-Buffer: 0x%04X, %d\n",
+ misc->fw_buffer_addr, misc->fw_buffer_max_len);
+ seq_printf(s, "Touch-Data: 0x%04X, %d\n",
+ misc->touch_data_addr, misc->touch_data_head_len);
+ seq_printf(s, "point_struct_len: %d\n",
+ misc->point_struct_len);
+ seq_printf(s, "mutual_rawdata_addr: 0x%04X\n",
+ misc->mutual_rawdata_addr);
+ seq_printf(s, "mutual_diffdata_addr: 0x%04X\n",
+ misc->mutual_diffdata_addr);
+ seq_printf(s, "self_rawdata_addr: 0x%04X\n",
+ misc->self_rawdata_addr);
+ seq_printf(s, "self_diffdata_addr: 0x%04X\n",
+ misc->self_diffdata_addr);
+ seq_printf(s, "stylus_rawdata_addr: 0x%04X, %d\n",
+ misc->stylus_rawdata_addr, misc->stylus_rawdata_len);
+ seq_printf(s, "esd_addr: 0x%04X\n",
+ misc->esd_addr);
+
+ return 0;
+}
+
+DEFINE_SHOW_ATTRIBUTE(goodix_berlin_ic_info);
+
+static int goodix_berlin_get_ic_info(struct goodix_berlin_core *cd,
+ struct goodix_ic_info *ic_info)
+{
+ int ret, i;
+ u16 length = 0;
+ u32 ic_addr;
+ u8 afe_data[GOODIX_IC_INFO_MAX_LEN] = { 0 };
+
+ ic_addr = GOODIX_IC_INFO_ADDR;
+
+ for (i = 0; i < 3; i++) {
+ ret = regmap_raw_read(cd->regmap, ic_addr, (u8 *)&length, sizeof(length));
+ if (ret) {
+ dev_info(cd->dev, "failed get ic info length, %d\n", ret);
+ usleep_range(5000, 5100);
+ continue;
+ }
+
+ length = le16_to_cpu(length);
+ if (length >= GOODIX_IC_INFO_MAX_LEN) {
+ dev_info(cd->dev, "invalid ic info length %d, retry %d\n", length, i);
+ continue;
+ }
+
+ ret = regmap_raw_read(cd->regmap, ic_addr, afe_data, length);
+ if (ret) {
+ dev_info(cd->dev, "failed get ic info data, %d\n", ret);
+ usleep_range(5000, 5100);
+ continue;
+ }
+
+ /* judge whether the data is valid */
+ if (goodix_berlin_is_risk_data(cd, (const uint8_t *)afe_data,
+ length)) {
+ dev_info(cd->dev, "fw info data invalid\n");
+ usleep_range(5000, 5100);
+ continue;
+ }
+
+ if (!goodix_berlin_check_checksum((const uint8_t *)afe_data, length)) {
+ dev_info(cd->dev, "fw info checksum error\n");
+ usleep_range(5000, 5100);
+ continue;
+ }
+
+ break;
+ }
+ if (i == 3) {
+ dev_err(cd->dev, "failed get ic info\n");
+ return -EINVAL;
+ }
+
+ ret = goodix_berlin_convert_ic_info(cd, ic_info, afe_data);
+ if (ret) {
+ dev_err(cd->dev, "error converting ic info\n");
+ return ret;
+ }
+
+ /* check some key info */
+ if (!ic_info->misc.cmd_addr || !ic_info->misc.fw_buffer_addr ||
+ !ic_info->misc.touch_data_addr) {
+ dev_err(cd->dev, "cmd_addr fw_buf_addr and touch_data_addr is null\n");
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static int goodix_berlin_after_event_handler(struct goodix_berlin_core *cd)
+{
+ struct goodix_ic_info_misc *misc = &cd->ic_info.misc;
+ u8 sync_clean = 0;
+
+ return regmap_raw_write(cd->regmap, misc->touch_data_addr, &sync_clean, 1);
+}
+
+static void goodix_berlin_parse_finger(struct goodix_berlin_core *cd,
+ struct goodix_touch_data *touch_data,
+ u8 *buf, int touch_num)
+{
+ unsigned int id = 0, x = 0, y = 0, w = 0;
+ u8 *coor_data;
+ int i;
+
+ coor_data = &buf[IRQ_EVENT_HEAD_LEN];
+
+ for (i = 0; i < touch_num; i++) {
+ id = (coor_data[0] >> 4) & 0x0F;
+
+ if (id >= GOODIX_MAX_TOUCH) {
+ dev_warn(cd->dev, "invalid finger id %d\n", id);
+
+ touch_data->touch_num = 0;
+ return;
+ }
+
+ x = le16_to_cpup((__le16 *)(coor_data + 2));
+ y = le16_to_cpup((__le16 *)(coor_data + 4));
+ w = le16_to_cpup((__le16 *)(coor_data + 6));
+
+ touch_data->coords[id].status = TS_TOUCH;
+ touch_data->coords[id].x = x;
+ touch_data->coords[id].y = y;
+ touch_data->coords[id].w = w;
+
+ coor_data += BYTES_PER_POINT;
+ }
+
+ touch_data->touch_num = touch_num;
+}
+
+static int goodix_berlin_touch_handler(struct goodix_berlin_core *cd,
+ u8 *pre_buf, u32 pre_buf_len)
+{
+ static u8 buffer[IRQ_EVENT_HEAD_LEN + BYTES_PER_POINT * GOODIX_MAX_TOUCH + 2];
+ struct goodix_touch_data *touch_data = &cd->ts_event.touch_data;
+ struct goodix_ic_info_misc *misc = &cd->ic_info.misc;
+ u8 point_type = 0;
+ u8 touch_num = 0;
+ int ret = 0;
+
+ /* clean event buffer */
+ memset(&cd->ts_event, 0, sizeof(cd->ts_event));
+
+ /* copy pre-data to buffer */
+ memcpy(buffer, pre_buf, pre_buf_len);
+
+ touch_num = buffer[2] & 0x0F;
+
+ if (touch_num > GOODIX_MAX_TOUCH) {
+ dev_warn(cd->dev, "invalid touch num %d\n", touch_num);
+ return -EINVAL;
+ }
+
+ /* read more data if more than 2 touch events */
+ if (unlikely(touch_num > 2)) {
+ ret = regmap_raw_read(cd->regmap,
+ misc->touch_data_addr + pre_buf_len,
+ &buffer[pre_buf_len],
+ (touch_num - 2) * BYTES_PER_POINT);
+ if (ret) {
+ dev_err(cd->dev, "failed get touch data\n");
+ return ret;
+ }
+ }
+
+ /* read done */
+ goodix_berlin_after_event_handler(cd);
+
+ if (touch_num == 0)
+ return 0;
+
+ point_type = buffer[IRQ_EVENT_HEAD_LEN] & 0x0F;
+
+ /* TOFIX: Not handled */
+ if (point_type == POINT_TYPE_STYLUS || point_type == POINT_TYPE_STYLUS_HOVER) {
+ dev_warn_once(cd->dev, "Stylus event type not handled\n");
+ return 0;
+ }
+
+ if (!goodix_berlin_check_checksum(&buffer[IRQ_EVENT_HEAD_LEN],
+ touch_num * BYTES_PER_POINT + 2)) {
+ dev_dbg(cd->dev, "touch data checksum error\n");
+ dev_dbg(cd->dev, "data: %*ph\n",
+ touch_num * BYTES_PER_POINT + 2, &buffer[IRQ_EVENT_HEAD_LEN]);
+ return -EINVAL;
+ }
+
+ cd->ts_event.event_type = EVENT_TOUCH;
+ goodix_berlin_parse_finger(cd, touch_data, buffer, touch_num);
+
+ return 0;
+}
+
+static int goodix_berlin_event_handler(struct goodix_berlin_core *cd)
+{
+ struct goodix_ic_info_misc *misc = &cd->ic_info.misc;
+ int pre_read_len;
+ u8 pre_buf[32];
+ u8 event_status;
+ int ret;
+
+ pre_read_len = IRQ_EVENT_HEAD_LEN + BYTES_PER_POINT * 2 +
+ COOR_DATA_CHECKSUM_SIZE;
+
+ ret = regmap_raw_read(cd->regmap, misc->touch_data_addr, pre_buf,
+ pre_read_len);
+ if (ret) {
+ dev_err(cd->dev, "failed get event head data\n");
+ return ret;
+ }
+
+ if (pre_buf[0] == 0x00)
+ return -EINVAL;
+
+ if (!goodix_berlin_check_checksum(pre_buf, IRQ_EVENT_HEAD_LEN)) {
+ dev_warn(cd->dev, "touch head checksum err : %*ph\n",
+ IRQ_EVENT_HEAD_LEN, pre_buf);
+ return -EINVAL;
+ }
+
+ event_status = pre_buf[0];
+ if (event_status & GOODIX_TOUCH_EVENT)
+ return goodix_berlin_touch_handler(cd, pre_buf, pre_read_len);
+
+ if (event_status & GOODIX_REQUEST_EVENT) {
+ cd->ts_event.event_type = EVENT_REQUEST;
+ if (pre_buf[2] == BRL_REQUEST_CODE_RESET)
+ cd->ts_event.request_code = REQUEST_TYPE_RESET;
+ else
+ dev_warn(cd->dev, "unsupported request code 0x%x\n", pre_buf[2]);
+ }
+
+ goodix_berlin_after_event_handler(cd);
+
+ return 0;
+}
+
+static void goodix_berlin_report_finger(struct goodix_berlin_core *cd)
+{
+ struct goodix_touch_data *touch_data = &cd->ts_event.touch_data;
+ int i;
+
+ mutex_lock(&cd->input_dev->mutex);
+
+ for (i = 0; i < GOODIX_MAX_TOUCH; i++) {
+ if (touch_data->coords[i].status == TS_TOUCH) {
+ dev_dbg(cd->dev, "report: id[%d], x %d, y %d, w %d\n", i,
+ touch_data->coords[i].x,
+ touch_data->coords[i].y,
+ touch_data->coords[i].w);
+
+ input_mt_slot(cd->input_dev, i);
+ input_mt_report_slot_state(cd->input_dev,
+ MT_TOOL_FINGER, true);
+ touchscreen_report_pos(cd->input_dev, &cd->props,
+ touch_data->coords[i].x,
+ touch_data->coords[i].y, true);
+ input_report_abs(cd->input_dev, ABS_MT_TOUCH_MAJOR,
+ touch_data->coords[i].w);
+ } else {
+ input_mt_slot(cd->input_dev, i);
+ input_mt_report_slot_state(cd->input_dev,
+ MT_TOOL_FINGER, false);
+ }
+ }
+
+ input_mt_sync_frame(cd->input_dev);
+ input_sync(cd->input_dev);
+
+ mutex_unlock(&cd->input_dev->mutex);
+}
+
+static int goodix_berlin_request_handle(struct goodix_berlin_core *cd)
+{
+ /* TOFIX: Handle more request codes */
+ if (cd->ts_event.request_code != REQUEST_TYPE_RESET) {
+ dev_info(cd->dev, "can't handle request type 0x%x\n",
+ cd->ts_event.request_code);
+ return -EINVAL;
+ }
+
+ return goodix_berlin_reset(cd, GOODIX_NORMAL_RESET_DELAY_MS);
+}
+
+static irqreturn_t goodix_berlin_threadirq_func(int irq, void *data)
+{
+ struct goodix_berlin_core *cd = data;
+ int ret;
+
+ ret = goodix_berlin_event_handler(cd);
+ if (likely(!ret)) {
+ switch (cd->ts_event.event_type) {
+ case EVENT_TOUCH:
+ goodix_berlin_report_finger(cd);
+ break;
+
+ case EVENT_REQUEST:
+ goodix_berlin_request_handle(cd);
+ break;
+
+ /* TOFIX: Handle more request types */
+ case EVENT_INVALID:
+ break;
+ }
+ }
+
+ return IRQ_HANDLED;
+}
+
+static int goodix_berlin_input_dev_config(struct goodix_berlin_core *cd,
+ const struct input_id *id)
+{
+ struct input_dev *input_dev = devm_input_allocate_device(cd->dev);
+
+ if (!input_dev)
+ return -ENOMEM;
+
+ cd->input_dev = input_dev;
+ input_set_drvdata(input_dev, cd);
+
+ input_dev->name = "Goodix Berlin Capacitive TouchScreen";
+ input_dev->phys = "input/ts";
+ input_dev->dev.parent = cd->dev;
+
+ memcpy(&input_dev->id, id, sizeof(*id));
+
+ /* set input parameters */
+ input_set_abs_params(cd->input_dev, ABS_MT_POSITION_X, 0, SZ_64K - 1, 0, 0);
+ input_set_abs_params(cd->input_dev, ABS_MT_POSITION_Y, 0, SZ_64K - 1, 0, 0);
+ input_set_abs_params(cd->input_dev, ABS_MT_TOUCH_MAJOR, 0, 255, 0, 0);
+
+ /* Override with DT */
+ touchscreen_parse_properties(cd->input_dev, true, &cd->props);
+
+ input_mt_init_slots(cd->input_dev, GOODIX_MAX_TOUCH, INPUT_MT_DIRECT);
+
+ return input_register_device(cd->input_dev);
+}
+
+static void goodix_berlin_release_connects(struct goodix_berlin_core *cd)
+{
+ struct input_dev *input_dev = cd->input_dev;
+ int i;
+
+ mutex_lock(&input_dev->mutex);
+
+ for (i = 0; i < GOODIX_MAX_TOUCH; i++) {
+ input_mt_slot(input_dev, i);
+ input_mt_report_slot_state(input_dev, MT_TOOL_FINGER, false);
+ }
+ input_mt_sync_frame(input_dev);
+ input_sync(input_dev);
+
+ mutex_unlock(&input_dev->mutex);
+}
+
+static int goodix_berlin_suspend(struct goodix_berlin_core *cd)
+{
+ /* disable irq */
+ goodix_berlin_irq_enable(cd, false);
+
+ /* enter sleep mode or power off */
+ goodix_berlin_do_suspend(cd);
+
+ goodix_berlin_release_connects(cd);
+
+ return 0;
+}
+
+static int goodix_berlin_resume(struct goodix_berlin_core *cd)
+{
+ goodix_berlin_irq_enable(cd, false);
+
+ /* reset device or power on*/
+ goodix_berlin_do_resume(cd);
+
+ /* enable irq */
+ goodix_berlin_irq_enable(cd, true);
+
+ return 0;
+}
+
+int goodix_berlin_pm_suspend(struct device *dev)
+{
+ struct goodix_berlin_core *cd = dev_get_drvdata(dev);
+
+ return goodix_berlin_suspend(cd);
+}
+
+int goodix_berlin_pm_resume(struct device *dev)
+{
+ struct goodix_berlin_core *cd = dev_get_drvdata(dev);
+
+ return goodix_berlin_resume(cd);
+}
+
+EXPORT_GPL_SIMPLE_DEV_PM_OPS(goodix_berlin_pm_ops,
+ goodix_berlin_pm_suspend,
+ goodix_berlin_pm_resume);
+
+int goodix_berlin_probe(struct device *dev, int irq, const struct input_id *id,
+ struct regmap *regmap)
+{
+ struct goodix_berlin_core *cd;
+ int ret;
+
+ if (irq <= 0)
+ return -EINVAL;
+
+ cd = devm_kzalloc(dev, sizeof(*cd), GFP_KERNEL);
+ if (!cd)
+ return -ENOMEM;
+
+ cd->dev = dev;
+ cd->regmap = regmap;
+ cd->irq = irq;
+
+ cd->reset_gpio = devm_gpiod_get(cd->dev, "reset", GPIOF_OUT_INIT_HIGH);
+ if (IS_ERR(cd->reset_gpio))
+ return dev_err_probe(cd->dev, PTR_ERR(cd->reset_gpio),
+ "Failed to request reset gpio\n");
+
+ cd->avdd = devm_regulator_get(cd->dev, "avdd");
+ if (IS_ERR(cd->avdd))
+ return dev_err_probe(cd->dev, PTR_ERR(cd->avdd),
+ "Failed to request avdd regulator\n");
+
+ cd->iovdd = devm_regulator_get(cd->dev, "iovdd");
+ if (IS_ERR(cd->iovdd))
+ return dev_err_probe(cd->dev, PTR_ERR(cd->iovdd),
+ "Failed to request iovdd regulator\n");
+
+ ret = goodix_berlin_input_dev_config(cd, id);
+ if (ret < 0) {
+ dev_err(cd->dev, "failed set input device");
+ return ret;
+ }
+
+ ret = devm_request_threaded_irq(dev, irq, NULL,
+ goodix_berlin_threadirq_func,
+ IRQF_ONESHOT, "goodix-berlin", cd);
+ if (ret) {
+ dev_err(dev, "request threaded irq failed: %d\n", ret);
+ return ret;
+ }
+
+ dev_set_drvdata(dev, cd);
+
+ ret = goodix_berlin_power_on(cd, true);
+ if (ret) {
+ dev_err(cd->dev, "failed power on");
+ goto err_out;
+ }
+
+ ret = goodix_berlin_read_version(cd, &cd->fw_version);
+ if (ret < 0) {
+ dev_err(cd->dev, "failed to get version info");
+ goto err_out;
+ }
+
+ ret = goodix_berlin_get_ic_info(cd, &cd->ic_info);
+ if (ret) {
+ dev_err(cd->dev, "invalid ic info, abort");
+ goto err_out;
+ }
+
+ cd->debugfs_root = debugfs_create_dir(dev_name(dev), NULL);
+ debugfs_create_file("fw_version", 0444, cd->debugfs_root,
+ cd, &goodix_berlin_fw_version_fops);
+ debugfs_create_file("ic_info", 0444, cd->debugfs_root,
+ cd, &goodix_berlin_ic_info_fops);
+
+ dev_info(cd->dev, "Goodix Berlin %s Touchscreen Controller", cd->fw_version.patch_pid);
+
+ return 0;
+
+err_out:
+ goodix_berlin_power_on(cd, false);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(goodix_berlin_probe);
+
+int goodix_berlin_remove(struct device *dev)
+{
+ struct goodix_berlin_core *cd = dev_get_drvdata(dev);
+
+ goodix_berlin_release_connects(cd);
+
+ goodix_berlin_irq_enable(cd, false);
+
+ goodix_berlin_power_on(cd, false);
+
+ debugfs_remove(cd->debugfs_root);
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(goodix_berlin_remove);
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("Goodix Berlin Core Touchscreen driver");
+MODULE_AUTHOR("Neil Armstrong <neil.armstrong@linaro.org>");
--
2.34.1
^ permalink raw reply related
* [PATCH RFC 3/4] input: touchscreen: add I2C support for Goodix Berlin Touchscreen IC
From: Neil Armstrong @ 2023-06-06 14:31 UTC (permalink / raw)
To: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Bastien Nocera, Hans de Goede, Henrik Rydberg
Cc: linux-input, linux-arm-msm, devicetree, linux-kernel,
Neil Armstrong
In-Reply-To: <20230606-topic-goodix-berlin-upstream-initial-v1-0-4a0741b8aefd@linaro.org>
Add initial support for the new Goodix "Berlin" touchscreen ICs
over the I2C interface.
This initial driver is derived from the Goodix goodix_ts_berlin
available at [1] and [2] and only supports the GT9916 IC
present on the Qualcomm SM8550 MTP & QRD touch panel.
The current implementation only supports BerlinD, aka GT9916.
[1] https://github.com/goodix/goodix_ts_berlin
[2] https://git.codelinaro.org/clo/la/platform/vendor/opensource/touch-drivers
Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
---
drivers/input/touchscreen/Kconfig | 14 +++++
drivers/input/touchscreen/Makefile | 1 +
drivers/input/touchscreen/goodix_berlin_i2c.c | 76 +++++++++++++++++++++++++++
3 files changed, 91 insertions(+)
diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig
index 1a6f6f6da991..da6d5d75c42d 100644
--- a/drivers/input/touchscreen/Kconfig
+++ b/drivers/input/touchscreen/Kconfig
@@ -421,6 +421,20 @@ config TOUCHSCREEN_GOODIX_BERLIN_CORE
depends on REGMAP
tristate
+config TOUCHSCREEN_GOODIX_BERLIN_I2C
+ tristate "Goodix Berlin I2C touchscreen"
+ depends on I2C
+ depends on REGMAP_I2C
+ select TOUCHSCREEN_GOODIX_BERLIN_CORE
+ help
+ Say Y here if you have the a touchscreen connected to your
+ system using the Goodix Berlin IC connection via I2C.
+
+ If unsure, say N.
+
+ To compile this driver as a module, choose M here: the
+ module will be called goodix_berlin_i2c.
+
config TOUCHSCREEN_HIDEEP
tristate "HiDeep Touch IC"
depends on I2C
diff --git a/drivers/input/touchscreen/Makefile b/drivers/input/touchscreen/Makefile
index 29cdb042e104..921a2da0c2be 100644
--- a/drivers/input/touchscreen/Makefile
+++ b/drivers/input/touchscreen/Makefile
@@ -48,6 +48,7 @@ obj-$(CONFIG_TOUCHSCREEN_EXC3000) += exc3000.o
obj-$(CONFIG_TOUCHSCREEN_FUJITSU) += fujitsu_ts.o
obj-$(CONFIG_TOUCHSCREEN_GOODIX) += goodix_ts.o
obj-$(CONFIG_TOUCHSCREEN_GOODIX_BERLIN_CORE) += goodix_berlin_core.o
+obj-$(CONFIG_TOUCHSCREEN_GOODIX_BERLIN_I2C) += goodix_berlin_i2c.o
obj-$(CONFIG_TOUCHSCREEN_HIDEEP) += hideep.o
obj-$(CONFIG_TOUCHSCREEN_HYNITRON_CSTXXX) += hynitron_cstxxx.o
obj-$(CONFIG_TOUCHSCREEN_ILI210X) += ili210x.o
diff --git a/drivers/input/touchscreen/goodix_berlin_i2c.c b/drivers/input/touchscreen/goodix_berlin_i2c.c
new file mode 100644
index 000000000000..fc32b8077287
--- /dev/null
+++ b/drivers/input/touchscreen/goodix_berlin_i2c.c
@@ -0,0 +1,76 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Goodix Berlin Touchscreen Driver
+ *
+ * Copyright (C) 2020 - 2021 Goodix, Inc.
+ * Copyright (C) 2023 Linaro Ltd.
+ *
+ * Based on goodix_ts_berlin driver.
+ */
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/i2c.h>
+#include <linux/regmap.h>
+#include <asm/unaligned.h>
+
+#include "goodix_berlin.h"
+
+#define I2C_MAX_TRANSFER_SIZE 256
+
+static const struct regmap_config goodix_berlin_i2c_regmap_conf = {
+ .reg_bits = 32,
+ .val_bits = 8,
+ .max_raw_read = I2C_MAX_TRANSFER_SIZE,
+ .max_raw_write = I2C_MAX_TRANSFER_SIZE,
+};
+
+static const struct input_id goodix_berlin_i2c_input_id = {
+ .bustype = BUS_I2C,
+ .vendor = 0x0416,
+ .product = 0x1001,
+};
+
+static int goodix_berlin_i2c_probe(struct i2c_client *client)
+{
+ struct regmap *map;
+
+ map = devm_regmap_init_i2c(client, &goodix_berlin_i2c_regmap_conf);
+ if (IS_ERR(map))
+ return PTR_ERR(map);
+
+ return goodix_berlin_probe(&client->dev, client->irq,
+ &goodix_berlin_i2c_input_id, map);
+}
+
+static void goodix_berlin_i2c_remove(struct i2c_client *client)
+{
+ goodix_berlin_remove(&client->dev);
+}
+
+static const struct i2c_device_id goodix_berlin_i2c_id[] = {
+ { "gt9916", 0 },
+ { }
+};
+
+MODULE_DEVICE_TABLE(i2c, goodix_berlin_i2c_id);
+
+static const struct of_device_id goodix_berlin_i2c_of_match[] = {
+ { .compatible = "goodix,gt9916", },
+ { },
+};
+
+static struct i2c_driver goodix_berlin_i2c_driver = {
+ .driver = {
+ .name = "goodix-berlin-i2c",
+ .of_match_table = goodix_berlin_i2c_of_match,
+ .pm = pm_sleep_ptr(&goodix_berlin_pm_ops),
+ },
+ .probe = goodix_berlin_i2c_probe,
+ .remove = goodix_berlin_i2c_remove,
+ .id_table = goodix_berlin_i2c_id,
+};
+module_i2c_driver(goodix_berlin_i2c_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("Goodix Berlin I2C Touchscreen driver");
+MODULE_AUTHOR("Neil Armstrong <neil.armstrong@linaro.org>");
--
2.34.1
^ permalink raw reply related
* [PATCH RFC 4/4] input: touchscreen: add SPI support for Goodix Berlin Touchscreen IC
From: Neil Armstrong @ 2023-06-06 14:31 UTC (permalink / raw)
To: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Bastien Nocera, Hans de Goede, Henrik Rydberg
Cc: linux-input, linux-arm-msm, devicetree, linux-kernel,
Neil Armstrong
In-Reply-To: <20230606-topic-goodix-berlin-upstream-initial-v1-0-4a0741b8aefd@linaro.org>
Add initial support for the new Goodix "Berlin" touchscreen ICs
over the SPI interface.
The driver doesn't use the regmap_spi code since the SPI messages
needs to be prefixed, thus this custom regmap code.
This initial driver is derived from the Goodix goodix_ts_berlin
available at [1] and [2] and only supports the GT9916 IC
present on the Qualcomm SM8550 MTP & QRD touch panel.
The current implementation only supports BerlinD, aka GT9916.
[1] https://github.com/goodix/goodix_ts_berlin
[2] https://git.codelinaro.org/clo/la/platform/vendor/opensource/touch-drivers
Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
---
drivers/input/touchscreen/Kconfig | 14 ++
drivers/input/touchscreen/Makefile | 1 +
drivers/input/touchscreen/goodix_berlin_spi.c | 183 ++++++++++++++++++++++++++
3 files changed, 198 insertions(+)
diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig
index da6d5d75c42d..ffe0c0a4cd15 100644
--- a/drivers/input/touchscreen/Kconfig
+++ b/drivers/input/touchscreen/Kconfig
@@ -435,6 +435,20 @@ config TOUCHSCREEN_GOODIX_BERLIN_I2C
To compile this driver as a module, choose M here: the
module will be called goodix_berlin_i2c.
+config TOUCHSCREEN_GOODIX_BERLIN_SPI
+ tristate "Goodix Berlin SPI touchscreen"
+ depends on SPI_MASTER
+ depends on REGMAP
+ select TOUCHSCREEN_GOODIX_BERLIN_CORE
+ help
+ Say Y here if you have the a touchscreen connected to your
+ system using the Goodix Berlin IC connection via SPI.
+
+ If unsure, say N.
+
+ To compile this driver as a module, choose M here: the
+ module will be called goodix_berlin_spi.
+
config TOUCHSCREEN_HIDEEP
tristate "HiDeep Touch IC"
depends on I2C
diff --git a/drivers/input/touchscreen/Makefile b/drivers/input/touchscreen/Makefile
index 921a2da0c2be..29524e8a83db 100644
--- a/drivers/input/touchscreen/Makefile
+++ b/drivers/input/touchscreen/Makefile
@@ -49,6 +49,7 @@ obj-$(CONFIG_TOUCHSCREEN_FUJITSU) += fujitsu_ts.o
obj-$(CONFIG_TOUCHSCREEN_GOODIX) += goodix_ts.o
obj-$(CONFIG_TOUCHSCREEN_GOODIX_BERLIN_CORE) += goodix_berlin_core.o
obj-$(CONFIG_TOUCHSCREEN_GOODIX_BERLIN_I2C) += goodix_berlin_i2c.o
+obj-$(CONFIG_TOUCHSCREEN_GOODIX_BERLIN_SPI) += goodix_berlin_spi.o
obj-$(CONFIG_TOUCHSCREEN_HIDEEP) += hideep.o
obj-$(CONFIG_TOUCHSCREEN_HYNITRON_CSTXXX) += hynitron_cstxxx.o
obj-$(CONFIG_TOUCHSCREEN_ILI210X) += ili210x.o
diff --git a/drivers/input/touchscreen/goodix_berlin_spi.c b/drivers/input/touchscreen/goodix_berlin_spi.c
new file mode 100644
index 000000000000..0f4f650fdf3f
--- /dev/null
+++ b/drivers/input/touchscreen/goodix_berlin_spi.c
@@ -0,0 +1,183 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Goodix Berlin Touchscreen Driver
+ *
+ * Copyright (C) 2020 - 2021 Goodix, Inc.
+ * Copyright (C) 2023 Linaro Ltd.
+ *
+ * Based on goodix_ts_berlin driver.
+ */
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/spi/spi.h>
+#include <linux/regmap.h>
+#include <asm/unaligned.h>
+
+#include "goodix_berlin.h"
+
+#define SPI_TRANS_PREFIX_LEN 1
+#define REGISTER_WIDTH 4
+#define SPI_READ_DUMMY_LEN 3
+#define SPI_READ_PREFIX_LEN (SPI_TRANS_PREFIX_LEN + REGISTER_WIDTH + SPI_READ_DUMMY_LEN)
+#define SPI_WRITE_PREFIX_LEN (SPI_TRANS_PREFIX_LEN + REGISTER_WIDTH)
+
+#define SPI_WRITE_FLAG 0xF0
+#define SPI_READ_FLAG 0xF1
+
+static int goodix_berlin_spi_read(void *context, const void *reg_buf,
+ size_t reg_size, void *val_buf,
+ size_t val_size)
+{
+ struct spi_device *spi = context;
+ struct spi_transfer xfers;
+ struct spi_message spi_msg;
+ const u32 *reg = reg_buf; /* reg is stored as native u32 at start of buffer */
+ u8 *buf = NULL;
+ int ret = 0;
+
+ if (reg_size != REGISTER_WIDTH)
+ return -EINVAL;
+
+ buf = kzalloc(SPI_READ_PREFIX_LEN + val_size, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
+
+ spi_message_init(&spi_msg);
+ memset(&xfers, 0, sizeof(xfers));
+
+ /* buffer format: 0xF1 + addr(4bytes) + dummy(3bytes) + data */
+ buf[0] = SPI_READ_FLAG;
+ put_unaligned_be32(*reg, buf + SPI_TRANS_PREFIX_LEN);
+ memset(buf + SPI_TRANS_PREFIX_LEN + REGISTER_WIDTH, 0xff,
+ SPI_READ_DUMMY_LEN);
+
+ xfers.tx_buf = buf;
+ xfers.rx_buf = buf;
+ xfers.len = SPI_READ_PREFIX_LEN + val_size;
+ xfers.cs_change = 0;
+ spi_message_add_tail(&xfers, &spi_msg);
+
+ ret = spi_sync(spi, &spi_msg);
+ if (ret < 0) {
+ dev_err(&spi->dev, "transfer error:%d", ret);
+ goto exit;
+ }
+
+ memcpy(val_buf, buf + SPI_READ_PREFIX_LEN, val_size);
+exit:
+ kfree(buf);
+ return ret;
+}
+
+static int goodix_berlin_spi_write(void *context, const void *data,
+ size_t count)
+{
+ unsigned int len = count - REGISTER_WIDTH;
+ struct spi_device *spi = context;
+ struct spi_transfer xfers;
+ struct spi_message spi_msg;
+ const u32 *reg = data; /* reg is stored as native u32 at start of buffer */
+ u8 *buf = NULL;
+ int ret = 0;
+
+ buf = kzalloc(SPI_WRITE_PREFIX_LEN + len, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
+
+ spi_message_init(&spi_msg);
+ memset(&xfers, 0, sizeof(xfers));
+
+ buf[0] = SPI_WRITE_FLAG;
+ put_unaligned_be32(*reg, buf + SPI_TRANS_PREFIX_LEN);
+ memcpy(buf + SPI_WRITE_PREFIX_LEN, data + REGISTER_WIDTH, len);
+
+ xfers.tx_buf = buf;
+ xfers.len = SPI_WRITE_PREFIX_LEN + len;
+ xfers.cs_change = 0;
+ spi_message_add_tail(&xfers, &spi_msg);
+
+ ret = spi_sync(spi, &spi_msg);
+ if (ret < 0)
+ dev_err(&spi->dev, "transfer error:%d", ret);
+
+ kfree(buf);
+ return ret;
+}
+
+static const struct regmap_config goodix_berlin_spi_regmap_conf = {
+ .reg_bits = 32,
+ .val_bits = 8,
+ .read = goodix_berlin_spi_read,
+ .write = goodix_berlin_spi_write,
+};
+
+static const struct input_id goodix_berlin_spi_input_id = {
+ .bustype = BUS_SPI,
+ .vendor = 0x0416,
+ .product = 0x1001,
+};
+
+static int goodix_berlin_spi_probe(struct spi_device *spi)
+{
+ struct regmap_config *cfg;
+ struct regmap *map;
+ size_t max_size;
+ int ret = 0;
+
+ cfg = devm_kmemdup(&spi->dev, &goodix_berlin_spi_regmap_conf,
+ sizeof(*cfg), GFP_KERNEL);
+ if (!cfg)
+ return -ENOMEM;
+
+ spi->mode = SPI_MODE_0;
+ spi->bits_per_word = 8;
+ ret = spi_setup(spi);
+ if (ret)
+ return ret;
+
+ max_size = spi_max_transfer_size(spi);
+ cfg->max_raw_read = max_size - SPI_READ_PREFIX_LEN;
+ cfg->max_raw_write = max_size - SPI_WRITE_PREFIX_LEN;
+
+ map = devm_regmap_init(&spi->dev, NULL, spi, cfg);
+ if (IS_ERR(map))
+ return PTR_ERR(map);
+
+ return goodix_berlin_probe(&spi->dev, spi->irq,
+ &goodix_berlin_spi_input_id, map);
+}
+
+static void goodix_berlin_spi_remove(struct spi_device *spi)
+{
+ goodix_berlin_remove(&spi->dev);
+}
+
+static const struct of_device_id goodix_berlin_spi_of_match[] = {
+ {
+ .compatible = "goodix,gt9916",
+ },
+ { },
+};
+MODULE_DEVICE_TABLE(of, goodix_berlin_spi_of_match);
+
+static const struct spi_device_id goodix_berlin_spi_ids[] = {
+ { "gt9916" },
+ { },
+};
+MODULE_DEVICE_TABLE(spi, goodix_berlin_spi_ids);
+
+static struct spi_driver goodix_berlin_spi_driver = {
+ .driver = {
+ .name = "goodix-berlin-spi",
+ .of_match_table = goodix_berlin_spi_of_match,
+ .pm = pm_sleep_ptr(&goodix_berlin_pm_ops),
+ },
+ .id_table = goodix_berlin_spi_ids,
+ .probe = goodix_berlin_spi_probe,
+ .remove = goodix_berlin_spi_remove,
+};
+module_spi_driver(goodix_berlin_spi_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("Goodix Berlin SPI Touchscreen driver");
+MODULE_AUTHOR("Neil Armstrong <neil.armstrong@linaro.org>");
--
2.34.1
^ permalink raw reply related
* [PATCH RFC 1/4] dt-bindings: input: document Goodix Berlin Touchscreen IC
From: Neil Armstrong @ 2023-06-06 14:31 UTC (permalink / raw)
To: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Bastien Nocera, Hans de Goede, Henrik Rydberg
Cc: linux-input, linux-arm-msm, devicetree, linux-kernel,
Neil Armstrong
In-Reply-To: <20230606-topic-goodix-berlin-upstream-initial-v1-0-4a0741b8aefd@linaro.org>
Document the Goodix GT9916 wich is part of the "Berlin" serie
of Touchscreen controllers IC from Goodix.
Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
---
.../bindings/input/touchscreen/goodix-berlin.yaml | 81 ++++++++++++++++++++++
1 file changed, 81 insertions(+)
diff --git a/Documentation/devicetree/bindings/input/touchscreen/goodix-berlin.yaml b/Documentation/devicetree/bindings/input/touchscreen/goodix-berlin.yaml
new file mode 100644
index 000000000000..4c24a541e919
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/touchscreen/goodix-berlin.yaml
@@ -0,0 +1,81 @@
+# SPDX-License-Identifier: GPL-2.0
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/input/touchscreen/goodix-berlin.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Goodix Belin series touchscreen controller
+
+maintainers:
+ - Neil Armstrong <neil.armstrong@linaro.org>
+
+allOf:
+ - $ref: touchscreen.yaml#
+
+properties:
+ compatible:
+ enum:
+ - goodix,gt9916
+
+ reg:
+ maxItems: 1
+
+ interrupts:
+ maxItems: 1
+
+ reset-gpios:
+ maxItems: 1
+
+ avdd-supply:
+ description: Analog power supply regulator on AVDD pin
+
+ vddio-supply:
+ description: GPIO power supply regulator on VDDIO pin
+
+ spi-max-frequency: true
+ touchscreen-inverted-x: true
+ touchscreen-inverted-y: true
+ touchscreen-size-x: true
+ touchscreen-size-y: true
+ touchscreen-swapped-x-y: true
+
+additionalProperties: false
+
+required:
+ - compatible
+ - reg
+ - interrupts
+
+examples:
+ - |
+ #include <dt-bindings/interrupt-controller/irq.h>
+ #include <dt-bindings/gpio/gpio.h>
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ gt9916@5d {
+ compatible = "goodix,gt9916";
+ reg = <0x5d>;
+ interrupt-parent = <&gpio>;
+ interrupts = <25 IRQ_TYPE_LEVEL_LOW>;
+ reset-gpios = <&gpio1 1 GPIO_ACTIVE_LOW>;
+ };
+ };
+ - |
+ #include <dt-bindings/interrupt-controller/irq.h>
+ #include <dt-bindings/gpio/gpio.h>
+ spi {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ num-cs = <1>;
+ cs-gpios = <&gpio 2 GPIO_ACTIVE_HIGH>;
+ gt9916@0 {
+ compatible = "goodix,gt9916";
+ reg = <0>;
+ interrupt-parent = <&gpio>;
+ interrupts = <25 IRQ_TYPE_LEVEL_LOW>;
+ reset-gpios = <&gpio1 1 GPIO_ACTIVE_LOW>;
+ };
+ };
+
+...
--
2.34.1
^ permalink raw reply related
* [PATCH RFC 0/4] input: touchscreen: add initial support for Goodix Berlin touchscreen IC
From: Neil Armstrong @ 2023-06-06 14:31 UTC (permalink / raw)
To: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Bastien Nocera, Hans de Goede, Henrik Rydberg
Cc: linux-input, linux-arm-msm, devicetree, linux-kernel,
Neil Armstrong
These touchscreen ICs support SPI, I2C and I3C interface, up to
10 finger touch, stylus and gestures events.
This initial driver is derived from the Goodix goodix_ts_berlin
available at [1] and [2] and only supports the GT9916 IC
present on the Qualcomm SM8550 MTP & QRD touch panel.
The current implementation only supports BerlinD, aka GT9916.
Support for advanced features like:
- Firmware & config update
- Stylus events
- Gestures events
- Previous revisions support (BerlinA or BerlinB)
is not included in current version.
The current support will work with currently flashed firmware
and config, and bail out if firmware or config aren't flashed yet.
[1] https://github.com/goodix/goodix_ts_berlin
[2] https://git.codelinaro.org/clo/la/platform/vendor/opensource/touch-drivers
Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
---
Neil Armstrong (4):
dt-bindings: input: document Goodix Berlin Touchscreen IC
input: touchscreen: add core support for Goodix Berlin Touchscreen IC
input: touchscreen: add I2C support for Goodix Berlin Touchscreen IC
input: touchscreen: add SPI support for Goodix Berlin Touchscreen IC
.../bindings/input/touchscreen/goodix-berlin.yaml | 81 ++
drivers/input/touchscreen/Kconfig | 33 +
drivers/input/touchscreen/Makefile | 3 +
drivers/input/touchscreen/goodix_berlin.h | 228 +++++
drivers/input/touchscreen/goodix_berlin_core.c | 935 +++++++++++++++++++++
drivers/input/touchscreen/goodix_berlin_i2c.c | 76 ++
drivers/input/touchscreen/goodix_berlin_spi.c | 183 ++++
7 files changed, 1539 insertions(+)
---
base-commit: 6db29e14f4fb7bce9eb5290288e71b05c2b0d118
change-id: 20230606-topic-goodix-berlin-upstream-initial-ba97e8ec8f4c
Best regards,
--
Neil Armstrong <neil.armstrong@linaro.org>
^ permalink raw reply
* Re: [PATCH] HID: logitech-hidpp: Handle timeout differently from busy
From: Jiri Kosina @ 2023-06-06 13:27 UTC (permalink / raw)
To: Linux regressions mailing list
Cc: Bastien Nocera, linux-input, linux-kernel, Benjamin Tissoires,
Peter F . Patel-Schneider, Filipe Laíns, Nestor Lopez Casado,
Mark Lord
In-Reply-To: <15bb2507-a145-7f1b-8e84-58aeb02484b9@leemhuis.info>
On Mon, 5 Jun 2023, Linux regression tracking (Thorsten Leemhuis) wrote:
> >>> If an attempt at contacting a receiver or a device fails because the
> >>> receiver or device never responds, don't restart the communication, only
> >>> restart it if the receiver or device answers that it's busy, as originally
> >>> intended.
> >>>
> >>> This was the behaviour on communication timeout before commit 586e8fede795
> >>> ("HID: logitech-hidpp: Retry commands when device is busy").
> >>>
> >>> This fixes some overly long waits in a critical path on boot, when
> >>> checking whether the device is connected by getting its HID++ version.
> >>>
> >>> Signed-off-by: Bastien Nocera <hadess@hadess.net>
> >>> Suggested-by: Mark Lord <mlord@pobox.com>
> >>> Fixes: 586e8fede795 ("HID: logitech-hidpp: Retry commands when device is busy")
> >>> Link: https://bugzilla.kernel.org/show_bug.cgi?id=217412
> > [...]
> >>
> >> I have applied this even before getting confirmation from the reporters in
> >> bugzilla, as it's the right thing to do anyway.
> >
> > Unfortunately it doesn't seem to cure the reported issue (while reverting
> > 586e8fede79 does):
>
> BTW, remind me again: was fixing this by reverting 586e8fede79 for now a
> option? I guess it's not, but if I'm wrong I wonder if that might at
> this point be the best way forward.
This should now all be fixed by
https://git.kernel.org/linus/7c28afd5512e371773dbb2bf95a31ed5625651d9
> > https://bugzilla.kernel.org/show_bug.cgi?id=217523#c2
>
> FWIW, another comment showed up there:
>
> ```
> > --- Comment #6 from vova7890 ---
> > Same problem. I researched this some time ago. I noticed that if I add a small
> > delay between commands to the dongle - everything goes fine. Repeated
> > request(586e8fede7953b1695b5ccc6112eff9b052e79ac) made the situation more
> > visible
>
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: amd_sfh driver causes kernel oops during boot
From: Malte Starostik @ 2023-06-06 9:53 UTC (permalink / raw)
To: Bagas Sanjaya, Benjamin Tissoires, Linux regressions mailing list,
regressions
Cc: basavaraj.natikar, linux-input, linux
In-Reply-To: <46d5acd2-5961-a496-4c1a-68e5c47021fb@leemhuis.info>
Am Dienstag, 6. Juni 2023, 08:56:16 CEST schrieb Linux regression tracking
(Thorsten Leemhuis):
> On 06.06.23 04:36, Bagas Sanjaya wrote:
> > On Mon, Jun 05, 2023 at 01:24:25PM +0200, Malte Starostik wrote:
> >> chiming in here as I'm experiencing what looks like the exact same issue,
> >> also on a Lenovo Z13 notebook, also on Arch:
> >> bisect result:
> >> 904e28c6de083fa4834cdbd0026470ddc30676fc is the first bad commit
> >> commit 904e28c6de083fa4834cdbd0026470ddc30676fc
> >> Merge: a738688177dc 2f7f4efb9411
> >> Author: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> >> Date: Wed Feb 22 10:44:31 2023 +0100
> >>
> >> Merge branch 'for-6.3/hid-bpf' into for-linus
> >
> > Hmm, seems like bad bisect (bisected to HID-BPF which IMO isn't related
> > to amd_sfh). Can you repeat the bisection?
I'm digging further. That merge is what git bisect ended at, but admittedly my
git skills and especially with a large codebase aren't too advanced.
While at 904e28c6de083fa4834cdbd0026470ddc30676fc, git show only shows the diff
for tools/testing/selftests/Makefile which can't really be the culprit.
However, git diff @~..@ has changes in drivers/hid/amd-sfh-hid/Kconfig (seems
innocuous, too), but also some changes to drivers/hid/hid-core.c. Nothing
obvious either, but at least it's not too far from the trace.
> Well, amd_sfh afaics apparently interacts with HID (see trace earlier in
> the thread), so it's not that far away. But it's a merge commit, which
> is possible, but doesn't happen every day. So a recheck might really be
> a good idea.
I will recheck some more, the Oops only happens with roughly 30 % chance
during boot. When it doesn't, there seem to be no other issues until the next
boot either. I made sure to reboot a few times after each bisect step, will
look deeper into the area.
> > Anyway, tl;dr:
> >> A: http://en.wikipedia.org/wiki/Top_post
> >> Q: Were do I find info about this thing called top-posting?
> >
> > [...]
>
> BTW, I'm not sure if this really is helpful. Teaching this to upcoming
> kernel developers is definitely worth it, but I wonder if pushing this
> on all reporters might do more harm than good. I also wonder if asking
> them a bit more kindly might be wiser (e.g. instead of "Anyway, tl;dr:"
> something like "BTW, please do not top-post:" or something like that maybe).
Thanks, and I agree in general. However, my case was in fact even worse :-)
I'm totally aware of the badness of top-posting. It happened because I had a
draft of the reply. Set In-Reply-To from the link in the wev archive and
pasted the previous message from there. Couple days later, I just pasted the
result on top and disregarded the existing text.
BR Malte
^ permalink raw reply
* Re: [PATCH 2/2] Input: tps65219-pwrbutton - Use regmap_set_bits()
From: Markus Schneider-Pargmann @ 2023-06-06 9:20 UTC (permalink / raw)
To: Uwe Kleine-König; +Cc: Dmitry Torokhov, linux-input, kernel
In-Reply-To: <20230605161458.117361-2-u.kleine-koenig@pengutronix.de>
On Mon, Jun 05, 2023 at 06:14:58PM +0200, Uwe Kleine-König wrote:
> regmap_set_bits() is equivalent to regmap_update_bits() if mask == val.
> The probe function uses regmap_clear_bits() to enable irqs, so
> symmetrically make use of regmap_set_bits() to disable them. There is no
> semantic difference.
>
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Reviewed-by: Markus Schneider-Pargmann <msp@baylibre.com>
Best,
Markus
> ---
> drivers/input/misc/tps65219-pwrbutton.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/input/misc/tps65219-pwrbutton.c b/drivers/input/misc/tps65219-pwrbutton.c
> index b2d9e5d2bcfd..eeb9f2334ab4 100644
> --- a/drivers/input/misc/tps65219-pwrbutton.c
> +++ b/drivers/input/misc/tps65219-pwrbutton.c
> @@ -123,9 +123,8 @@ static void tps65219_pb_remove(struct platform_device *pdev)
> int ret;
>
> /* Disable interrupt for the pushbutton */
> - ret = regmap_update_bits(tps->regmap, TPS65219_REG_MASK_CONFIG,
> - TPS65219_REG_MASK_INT_FOR_PB_MASK,
> - TPS65219_REG_MASK_INT_FOR_PB_MASK);
> + ret = regmap_set_bits(tps->regmap, TPS65219_REG_MASK_CONFIG,
> + TPS65219_REG_MASK_INT_FOR_PB_MASK);
> if (ret)
> dev_warn(&pdev->dev, "Failed to disable irq (%pe)\n", ERR_PTR(ret));
> }
> --
> 2.39.2
>
^ permalink raw reply
* Re: [PATCH 1/2] Input: tps65219-pwrbutton - Convert to .remove_new()
From: Markus Schneider-Pargmann @ 2023-06-06 9:19 UTC (permalink / raw)
To: Uwe Kleine-König; +Cc: Dmitry Torokhov, linux-input, kernel
In-Reply-To: <20230605161458.117361-1-u.kleine-koenig@pengutronix.de>
Hi Uwe,
On Mon, Jun 05, 2023 at 06:14:57PM +0200, Uwe Kleine-König wrote:
> The .remove() callback for a platform driver returns an int which makes
> many driver authors wrongly assume it's possible to do error handling by
> returning an error code. However the value returned is ignored (apart from
> emitting a warning).
>
> To improve here there is a quest to make the remove callback return void.
> In the first step of this quest all drivers are converted to .remove_new()
> which already returns void. Eventually after all drivers are converted,
> .remove_new() is renamed to .remove().
>
> Before this driver might have returned an error. In this case emit a
> warning that tells more about the problem than the generic warning by
> the core, and instead of making the remove callback return zero
> unconditionally, convert to .remove_new() which is equivalent.
>
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Reviewed-by: Markus Schneider-Pargmann <msp@baylibre.com>
Thanks,
Markus
> ---
> drivers/input/misc/tps65219-pwrbutton.c | 13 ++++++++-----
> 1 file changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/input/misc/tps65219-pwrbutton.c b/drivers/input/misc/tps65219-pwrbutton.c
> index 245134bdb59e..b2d9e5d2bcfd 100644
> --- a/drivers/input/misc/tps65219-pwrbutton.c
> +++ b/drivers/input/misc/tps65219-pwrbutton.c
> @@ -117,14 +117,17 @@ static int tps65219_pb_probe(struct platform_device *pdev)
> return 0;
> }
>
> -static int tps65219_pb_remove(struct platform_device *pdev)
> +static void tps65219_pb_remove(struct platform_device *pdev)
> {
> struct tps65219 *tps = dev_get_drvdata(pdev->dev.parent);
> + int ret;
>
> /* Disable interrupt for the pushbutton */
> - return regmap_update_bits(tps->regmap, TPS65219_REG_MASK_CONFIG,
> - TPS65219_REG_MASK_INT_FOR_PB_MASK,
> - TPS65219_REG_MASK_INT_FOR_PB_MASK);
> + ret = regmap_update_bits(tps->regmap, TPS65219_REG_MASK_CONFIG,
> + TPS65219_REG_MASK_INT_FOR_PB_MASK,
> + TPS65219_REG_MASK_INT_FOR_PB_MASK);
> + if (ret)
> + dev_warn(&pdev->dev, "Failed to disable irq (%pe)\n", ERR_PTR(ret));
> }
>
> static const struct platform_device_id tps65219_pwrbtn_id_table[] = {
> @@ -135,7 +138,7 @@ MODULE_DEVICE_TABLE(platform, tps65219_pwrbtn_id_table);
>
> static struct platform_driver tps65219_pb_driver = {
> .probe = tps65219_pb_probe,
> - .remove = tps65219_pb_remove,
> + .remove_new = tps65219_pb_remove,
> .driver = {
> .name = "tps65219_pwrbutton",
> },
>
> base-commit: ac9a78681b921877518763ba0e89202254349d1b
> --
> 2.39.2
>
^ permalink raw reply
* Re: [PATCH] HID: fix an error code in hid_check_device_match()
From: Benjamin Tissoires @ 2023-06-06 8:50 UTC (permalink / raw)
To: Dan Carpenter; +Cc: Dmitry Torokhov, Jiri Kosina, linux-input, kernel-janitors
In-Reply-To: <ZH7sZv4PEovkMxNP@moroto>
On Jun 06 2023, Dan Carpenter wrote:
>
> The hid_check_device_match() returns true if we find a match and false
> if we don't. But here it returns -ENODEV which becomes true instead
> of false.
>
> Fixes: 207733f92661 ("HID: split apart hid_device_probe to make logic more apparent")
> Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
> ---
> drivers/hid/hid-core.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
> index 4b7062dcefec..8992e3c1e769 100644
> --- a/drivers/hid/hid-core.c
> +++ b/drivers/hid/hid-core.c
> @@ -2593,7 +2593,7 @@ static bool hid_check_device_match(struct hid_device *hdev,
> {
> *id = hid_match_device(hdev, hdrv);
> if (!*id)
> - return -ENODEV;
> + return false;
Thanks for spotting this!
Reviewed-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
Cheers,
Benjamin
>
> if (hdrv->match)
> return hdrv->match(hdev, hid_ignore_special_drivers);
> --
> 2.30.2
>
^ permalink raw reply
* [bug report] selftests/hid: add tests for bpf_hid_hw_request
From: Dan Carpenter @ 2023-06-06 8:31 UTC (permalink / raw)
To: benjamin.tissoires; +Cc: linux-input
Hello Benjamin Tissoires,
The patch 4f7153cf461e: "selftests/hid: add tests for
bpf_hid_hw_request" from Nov 3, 2022, leads to the following Smatch
static checker warning:
./tools/testing/selftests/hid/hid_bpf.c:214 uhid_event()
warn: assigning (-5) to unsigned variable 'answer.u.get_report_reply.err'
./tools/testing/selftests/hid/hid_bpf.c
203 case UHID_CLOSE:
204 UHID_LOG("UHID_CLOSE from uhid-dev");
205 break;
206 case UHID_OUTPUT:
207 UHID_LOG("UHID_OUTPUT from uhid-dev");
208 break;
209 case UHID_GET_REPORT:
210 UHID_LOG("UHID_GET_REPORT from uhid-dev");
211
212 answer.type = UHID_GET_REPORT_REPLY;
213 answer.u.get_report_reply.id = ev.u.get_report.id;
--> 214 answer.u.get_report_reply.err = ev.u.get_report.rnum == 1 ? 0 : -EIO;
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
This is a u16 so it can't hold negative error codes.
215 answer.u.get_report_reply.size = sizeof(feature_data);
216 memcpy(answer.u.get_report_reply.data, feature_data, sizeof(feature_data));
217
218 uhid_write(_metadata, fd, &answer);
219
220 break;
221 case UHID_SET_REPORT:
222 UHID_LOG("UHID_SET_REPORT from uhid-dev");
223 break;
224 default:
regards,
dan carpenter
^ permalink raw reply
* [PATCH] HID: fix an error code in hid_check_device_match()
From: Dan Carpenter @ 2023-06-06 8:20 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Jiri Kosina, Benjamin Tissoires, linux-input, kernel-janitors
The hid_check_device_match() returns true if we find a match and false
if we don't. But here it returns -ENODEV which becomes true instead
of false.
Fixes: 207733f92661 ("HID: split apart hid_device_probe to make logic more apparent")
Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
---
drivers/hid/hid-core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index 4b7062dcefec..8992e3c1e769 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -2593,7 +2593,7 @@ static bool hid_check_device_match(struct hid_device *hdev,
{
*id = hid_match_device(hdev, hdrv);
if (!*id)
- return -ENODEV;
+ return false;
if (hdrv->match)
return hdrv->match(hdev, hid_ignore_special_drivers);
--
2.30.2
^ permalink raw reply related
* Re: amd_sfh driver causes kernel oops during boot
From: Benjamin Tissoires @ 2023-06-06 8:08 UTC (permalink / raw)
To: Linux regressions mailing list
Cc: Bagas Sanjaya, Malte Starostik, basavaraj.natikar, linux-input,
linux, stable, Mario Limonciello
In-Reply-To: <46d5acd2-5961-a496-4c1a-68e5c47021fb@leemhuis.info>
On Jun 06 2023, Linux regression tracking (Thorsten Leemhuis) wrote:
>
> On 06.06.23 04:36, Bagas Sanjaya wrote:
> > On Mon, Jun 05, 2023 at 01:24:25PM +0200, Malte Starostik wrote:
> >> Hello,
> >>
> >> chiming in here as I'm experiencing what looks like the exact same issue, also
> >> on a Lenovo Z13 notebook, also on Arch:
> >> Oops during startup in task udev-worker followed by udev-worker blocking all
> >> attempts to suspend or cleanly shutdown/reboot the machine - in fact I first
> >> noticed because the machine surprised with repeatedly running out of battery
> >> after it had supposedly been in standby but couldn't. Only then I noticed the
> >> error on boot.
> >>
> >> bisect result:
> >> 904e28c6de083fa4834cdbd0026470ddc30676fc is the first bad commit
> >> commit 904e28c6de083fa4834cdbd0026470ddc30676fc
> >> Merge: a738688177dc 2f7f4efb9411
> >> Author: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> >> Date: Wed Feb 22 10:44:31 2023 +0100
> >>
> >> Merge branch 'for-6.3/hid-bpf' into for-linus
> >
> > Hmm, seems like bad bisect (bisected to HID-BPF which IMO isn't related
> > to amd_sfh). Can you repeat the bisection?
>
> Well, amd_sfh afaics apparently interacts with HID (see trace earlier in
> the thread), so it's not that far away. But it's a merge commit, which
> is possible, but doesn't happen every day. So a recheck might really be
> a good idea.
Let's not rule out that there is a bad interaction between HID-BPF and
AMD SFH. HID-BPF is able to process any incoming HID event, whether it
comes from AND SFH, USB, BT, I2C or anything else.
However, looking at the stack trace in the initial report[0], it seems
we are getting the oops/stack traces while we are still in amd_sfh:
list_add corruption. next is NULL.
WARNING: CPU: 5 PID: 433 at lib/list_debug.c:25 __list_add_valid+0x57/0xa0
...
RIP: 0010:__list_add_valid+0x57/0xa0
...
Call Trace:
<TASK>
amd_sfh_get_report+0xba/0x110 [amd_sfh 78bf82e66cdb2ccf24cbe871a0835ef4eedddb17]
...
If HID-BPF were involved, we should see a call to hid_input_report() IMO.
Also AMD SFH calls hid_input_report() in a workqueue, so I would expect
a different stack trace.
I have a suspicion on commit 7bcfdab3f0c6 ("HID: amd_sfh: if no sensors are enabled,
clean up") because the stack trace says that there is a bad list_add,
which could happen if the object is not correctly initialized.
However, that commit was present in v6.2, so it might not be that one.
Back to the merge commit: the hid-bpf tree was merged in the hid tree
while it took its branch during the v6.1 cycle. So that might be the
reason you get this as a result of bisection because the AMD SFH code in
the hid-bpf branch is the one from the v6.1 kernel, and when you merge
it to the v6.2+ branch, you get a different code for that driver.
Cheers,
Benjamin
[0] https://lore.kernel.org/regressions/f40e3897-76f1-2cd0-2d83-e48d87130eab@hexchain.org/#t
^ 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