* Re: [PATCH][next] input: ti_am335x_tsc: remove redundant assignment to variable config
From: Dmitry Torokhov @ 2024-02-09 18:53 UTC (permalink / raw)
To: Colin Ian King; +Cc: linux-input, kernel-janitors, linux-kernel
In-Reply-To: <20240205215940.1851349-1-colin.i.king@gmail.com>
On Mon, Feb 05, 2024 at 09:59:40PM +0000, Colin Ian King wrote:
> The variable config is being initialized with a value that is never
> read, it is being re-assigned in the next statement. The initialization
> is redundant and can be removed.
>
> Cleans up clang scan build warning:
> drivers/input/touchscreen/ti_am335x_tsc.c:160:2: warning: Value stored
> to 'config' is never read [deadcode.DeadStores]
>
> Signed-off-by: Colin Ian King <colin.i.king@gmail.com>
Applied, thank you.
--
Dmitry
^ permalink raw reply
* Re: [PATCH] Input: xpad - sort xpad_device by vendor and product ID
From: Dmitry Torokhov @ 2024-02-09 18:56 UTC (permalink / raw)
To: Brenton Simpson
Cc: Hans de Goede, Cameron Gutman, Erica Taylor,
Ismael Ferreras Morezuelas, Jonathan Frederick, Matthias Benkmann,
Matthias Berndt, nate, Sam Lantinga, Vicki Pfau, linux-input,
linux-kernel, trivial
In-Reply-To: <20240130231903.293265-1-appsforartists@google.com>
Hi Brenton,
On Tue, Jan 30, 2024 at 11:19:03PM +0000, Brenton Simpson wrote:
> Signed-off-by: Brenton Simpson <appsforartists@google.com>
Empty patch descriptions are frowned upon. You should be able to add at
least a single sentence why the change is being made.
...
>
> static const struct usb_device_id xpad_table[] = {
> + /* Please keep this list sorted by vendor ID. Because the lines use different
> + * macros, you may need to sort it by hand.
> + */
Kernel style of multi-line comments is:
/*
* line 1
* line 2.
*/
I adjusted and applied, thank you.
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH] Input: psmouse - add resync_on_resume dmi check
From: Dmitry Torokhov @ 2024-02-09 19:01 UTC (permalink / raw)
To: Jonathan Denose
Cc: LKML, jefferymiller, Jonathan Denose, Raul Rangel, linux-input
In-Reply-To: <CALNJtpV0KsOusPQeGv8bQ3jKy2sUj+k=mPHc172f+vMaTDYPfg@mail.gmail.com>
On Wed, Feb 07, 2024 at 10:39:03AM -0600, Jonathan Denose wrote:
> Hi Dmitry,
>
> Thanks for your reply.
>
> On Tue, Feb 6, 2024 at 4:04 PM Dmitry Torokhov
> <dmitry.torokhov@gmail.com> wrote:
> >
> > Hi Jonathan,
> >
> > On Thu, Nov 02, 2023 at 07:52:47AM -0500, Jonathan Denose wrote:
> > > Some elantech touchpads consistently fail after resuming from
> > > suspend at sanity_check in elantech_packet_check_v4. This means
> > > the touchpad is completely unusable after suspend resume.
> > >
> > > With different permutations of i8042 nomux, nopnp, reset, and noloop
> > > kernel options enabled, and with crc_enabled the touchpad fails in
> > > the same way.
> > >
> > > Resyncing the touchpad after receiving the
> > > PACKET_UNKNOWN/PSMOUSE_BAD_DATA return code allows the touchpad to
> > > function correctly on resume. The touchpad fails to reconnect with
> > > the serio reconnect no matter how many times it retries, so this
> > > change skips over that retry sequence and goes directly to resync.
> >
> > Why can't we do this in elantech_reconnect()? I am sure we can make it
> > simpler and more robust than what the generic handler is trying to do
> > with polling and everything.
> >
> > Thanks.
> >
> > --
> > Dmitry
>
> I am fine with anything that would be simpler and more robust, though
> I'm not sure how to implement what you are describing.
>
> Are you suggesting that in this PSMOUSE_BAD_DATA case, instead of
> using psmouse_set_state and psmouse_queue_work to call
> psmouse->reconnect (which calls elantech_reconnect)?
No. From the description it sounds like the device sends wrong/extra
data right after resume. I think you can handle it in
elantech_reconnect() method by draining the buffer or issuing poll
request or something similar.
Can you post the i8042 data stream that happens on suspend/resume?
Toggling i8042.debug option will cause the driver to dump the data to
dmesg.
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH] Input: xpad - sort xpad_device by vendor and product ID
From: Brenton Simpson @ 2024-02-09 19:27 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Hans de Goede, Cameron Gutman, Erica Taylor,
Ismael Ferreras Morezuelas, Jonathan Frederick, Matthias Benkmann,
Matthias Berndt, nate, Sam Lantinga, Vicki Pfau, linux-input,
linux-kernel, trivial
In-Reply-To: <ZcZ1Sn_iPwlgfI3s@google.com>
On Fri, Feb 9, 2024 at 10:56 AM Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> Kernel style of multi-line comments is:
>
> /*
> * line 1
> * line 2.
> */
>
> I adjusted and applied, thank you.
Thanks!
I did scroll up to check comment style before I added those, but I
must have caught one of the ones that's out of spec. 🙃
^ permalink raw reply
* Re: Suspected bug in hid-microsoft.c
From: Taco Jerkface @ 2024-02-10 3:03 UTC (permalink / raw)
To: Benjamin Tissoires; +Cc: open list:HID CORE LAYER
In-Reply-To: <CAO-hwJLQ5rh+kOrOL8XH67WBUjjtVxrXUqFXo29zGSxTvQ6Ovw@mail.gmail.com>
Everything works now and no, the hat difference doesn't seem to be an
issue. It registers the direction pad as 4 buttons, but everything
seems to work fine.
Thanks for all the help!
Chris
On Fri, Feb 9, 2024 at 6:47 AM Benjamin Tissoires
<benjamin.tissoires@redhat.com> wrote:
>
> [Adding back the linux-input ML]
>
> On Wed, Feb 7, 2024 at 6:34 AM Taco Jerkface <tacodog311@gmail.com> wrote:
> >
> > On Tue, Feb 6, 2024 at 8:11 AM Benjamin Tissoires
> > <benjamin.tissoires@redhat.com> wrote:
> > >
> > > On Tue, Feb 6, 2024 at 6:25 AM Taco Jerkface <tacodog311@gmail.com> wrote:
> > > >
> > > > Thanks!
> > > >
> > > > > 1. (easiest) tune your udev rule to also give user access to the
> > > > > hidraw subsystem on this device. SDL should be able to read it
> > > > > directly, and handle it properly, but this won't solve for future
> > > > > users
> > > >
> > > > This udev rule does in fact "fix" the problem
> > > >
> > > > KERNELS=="*:045E:0B22.*", MODE="0666"
> > >
> > > Right :)
> > >
> > > Well you could restrict it further by using the recommendations from SDL:
> > > https://github.com/libsdl-org/SDL/blob/main/src/hidapi/udev/69-hid.rules
> > >
> > > # HIDAPI/hidraw
> > > KERNEL=="hidraw*", ATTRS{idVendor}=="045E", ATTRS{idProduct}=="0B22",
> > > TAG+="uaccess"
> > >
> > > (not entirely sure about the capitals in vendor/product)
> >
> > I tried that and couldn't get it to work. My (incomplete)
> > understanding is that the rule needs to match the fields from "udevadm
> > info --name=/dev/hidraw1 --attribute-walk" which looks like this. The
> > "KERNELS" line was the best I could find with both the vendor and
> > product fields. I wildcarded the 0005 because it changed when I
> > reconnected.
>
> Yeah, so I vaguely remember that some properties are inherited from
> the parents, and VendorID and ProductID might be inherited. And given
> that this is a BLE device and that it's then using uhid, those
> properties are not set.
>
> TL;DR: seems like you got the correct match. You should probably use
> TAG+="uaccess" instead of MODE="0666" to restrict the opening of the
> device from the current user of the current session only.
>
> >
> > looking at device
> > '/devices/virtual/misc/uhid/0005:045E:0B22.0005/hidraw/hidraw1':
> > KERNEL=="hidraw1"
> > SUBSYSTEM=="hidraw"
> > DRIVER==""
> > ATTR{power/control}=="auto"
> > ATTR{power/runtime_active_time}=="0"
> > ATTR{power/runtime_status}=="unsupported"
> > ATTR{power/runtime_suspended_time}=="0"
> >
> > looking at parent device '/devices/virtual/misc/uhid/0005:045E:0B22.0005':
> > KERNELS=="0005:045E:0B22.0005"
> > SUBSYSTEMS=="hid"
> > DRIVERS=="microsoft"
> > ATTRS{country}=="00"
> > ATTRS{power/control}=="auto"
> > ATTRS{power/runtime_active_time}=="0"
> > ATTRS{power/runtime_status}=="unsupported"
> > ATTRS{power/runtime_suspended_time}=="0"
> >
> > looking at parent device '/devices/virtual/misc/uhid':
> > KERNELS=="uhid"
> > SUBSYSTEMS=="misc"
> > DRIVERS==""
> > ATTRS{power/control}=="auto"
> > ATTRS{power/runtime_active_time}=="0"
> >
> > >
> > >
> > > >
> > > >
> > > > > 2. (no kernel compilation required) we can try to fix the report
> > > > > descriptor of the device through HID-BPF. Assuming you have
> > > > > CONFIG_HID_BPF enabled in your kernel, we can relatively easily change
> > > > > the way the device is exported/handled by the kernel, to make it
> > > > > useful hopefully
> > > > > 3. (hardest IMO as you'll have to recompile your kernel for the tests)
> > > > > we can try to tune hid-microsoft.c to properly export these buttons
> > > > >
> > > > > For 2 and 3, I'll need some events from your device with hid-recorder.
> > > > > You only gave me the report descriptor, but no events which are hard
> > > > > to deduce based on the long report descriptor.
> > > >
> > > > With the udev rule, I attached the hid-recorder output when pressing
> > > > the paddles. I see the 01 02 04 08 showing up at the end of the line.
> > >
> > > Thanks for that. And as you can see, they are using the "Assign
> > > Selection" usage, which is... new to me :)
> > >
> > > >
> > > >
> > > > > Also for 2 and 3 we need to have a BTN_* button code to use, and I
> > > > > don't know which ones should be used from the top of my head. HID-BPF
> > > > > would be easiest to use as we can let the user decide of it, while
> > > > > we'll need to have a more formal usage in case we fix hid-microsoft.
> > > >
> > > > When connected with USB, evemu-record shows button names
> > > > "BTN_TRIGGER_HAPPY5" "BTN_TRIGGER_HAPPY6" "BTN_TRIGGER_HAPPY7"
> > > > "BTN_TRIGGER_HAPPY8". I assume these are the button codes we can use?
> > > >
> > > > ################################
> > > > # Waiting for events #
> > > > ################################
> > > > E: 0.000001 0001 02c4 0001 # EV_KEY / BTN_TRIGGER_HAPPY5 1
> > > > E: 0.000001 0000 0000 0000 # ------------ SYN_REPORT (0) ---------- +0ms
> > > > E: 0.143950 0001 02c4 0000 # EV_KEY / BTN_TRIGGER_HAPPY5 0
> > > > E: 0.143950 0000 0000 0000 # ------------ SYN_REPORT (0) ---------- +143ms
> > > > E: 6.543984 0001 02c5 0001 # EV_KEY / BTN_TRIGGER_HAPPY6 1
> > > > E: 6.543984 0000 0000 0000 # ------------ SYN_REPORT (0) ---------- +6400ms
> > > > E: 6.615981 0001 02c5 0000 # EV_KEY / BTN_TRIGGER_HAPPY6 0
> > > > E: 6.615981 0000 0000 0000 # ------------ SYN_REPORT (0) ---------- +72ms
> > > > E: 7.520034 0001 02c6 0001 # EV_KEY / BTN_TRIGGER_HAPPY7 1
> > > > E: 7.520034 0000 0000 0000 # ------------ SYN_REPORT (0) ---------- +905ms
> > > > E: 7.648127 0001 02c6 0000 # EV_KEY / BTN_TRIGGER_HAPPY7 0
> > > > E: 7.648127 0000 0000 0000 # ------------ SYN_REPORT (0) ---------- +128ms
> > > > E: 8.344035 0001 02c7 0001 # EV_KEY / BTN_TRIGGER_HAPPY8 1
> > > > E: 8.344035 0000 0000 0000 # ------------ SYN_REPORT (0) ---------- +696ms
> > > > E: 8.480049 0001 02c7 0000 # EV_KEY / BTN_TRIGGER_HAPPY8 0
> > > > E: 8.480049 0000 0000 0000 # ------------ SYN_REPORT (0) ---------- +136ms
> > > >
> > >
> > > Good point. Then we can easily tweak the device to use those BTN_* instead
> > >
> > > >
> > > > >
> > > > > For 1, maybe SDL (or Steam) already ships some udev rules, and
> > > > > submitting a fix there would make things working for everybody.
> > > > >
> > > > > Anyway, depending on how much you want this to be fixed and what you
> > > > > can do (is CONFIG_HID_BPF enabled in your distro? and can you
> > > > > recompile a kernel module?) we can figure out the next step.
> > > >
> > > > Yes HID-BPF is configured in my kernel:
> > > >
> > > > cat /proc/config.gz | gunzip
> > > > #
> > > > # HID-BPF support
> > > > #
> > > > CONFIG_HID_BPF=y
> > > > # end of HID-BPF support
> > >
> > > Even better, it'll be way easier for the tests :)
> > >
> > > I've opened a new MR with your device fix:
> > > https://gitlab.freedesktop.org/libevdev/udev-hid-bpf/-/merge_requests/32
> > >
> > > You should be able to head to the last job in latest pipeline (in the
> > > pipeline tab) and download the artifact:
> > > https://gitlab.freedesktop.org/libevdev/udev-hid-bpf/-/jobs/54687597/artifacts/browse
> > >
> > > Then unpack and run `sudo ./install --verbose`.
> > >
> > > Then reconnect the device over Bluetooth, and see in evemu-record that
> > > the paddles are now reporting BTN_TRIGGER_HAPPY[5-8].
> > >
> >
> > Thanks. This does indeed work (with my udev rule still active).
>
> \o/
>
> >
> > > I'd then be curious about 2 things:
> > > - are the paddle now reporting the same events over USB and Bluetooth?
> > > (is there no inversions between the BTN_TRIGGER*?, and are all the
> > > buttons correctly handled?)
> >
> > The button reports are nearly identical now, though Bluetooth outputs
> > more information.
>
> \o/
>
> >
> > This is paddle1, paddl2, paddle3, paddle4 pressed in that order each way.
> >
> > USB:
> > E: 0.000001 0001 02c4 0001 # EV_KEY / BTN_TRIGGER_HAPPY5 1
> > E: 0.000001 0000 0000 0000 # ------------ SYN_REPORT (0) ---------- +0ms
> > E: 0.080031 0001 02c4 0000 # EV_KEY / BTN_TRIGGER_HAPPY5 0
> > E: 0.080031 0000 0000 0000 # ------------ SYN_REPORT (0) ---------- +80ms
> > E: 1.520025 0001 02c5 0001 # EV_KEY / BTN_TRIGGER_HAPPY6 1
> > E: 1.520025 0000 0000 0000 # ------------ SYN_REPORT (0) ---------- +1440ms
> > E: 1.631883 0001 02c5 0000 # EV_KEY / BTN_TRIGGER_HAPPY6 0
> > E: 1.631883 0000 0000 0000 # ------------ SYN_REPORT (0) ---------- +111ms
> > E: 2.183887 0001 02c6 0001 # EV_KEY / BTN_TRIGGER_HAPPY7 1
> > E: 2.183887 0000 0000 0000 # ------------ SYN_REPORT (0) ---------- +552ms
> > E: 2.295985 0001 02c6 0000 # EV_KEY / BTN_TRIGGER_HAPPY7 0
> > E: 2.295985 0000 0000 0000 # ------------ SYN_REPORT (0) ---------- +112ms
> > E: 2.800031 0001 02c7 0001 # EV_KEY / BTN_TRIGGER_HAPPY8 1
> > E: 2.800031 0000 0000 0000 # ------------ SYN_REPORT (0) ---------- +505ms
> > E: 2.879964 0001 02c7 0000 # EV_KEY / BTN_TRIGGER_HAPPY8 0
> > E: 2.879964 0000 0000 0000 # ------------ SYN_REPORT (0) ---------- +79ms
> >
> > Bluetooth:
> > E: 0.000001 0004 0004 589845 # EV_MSC / MSC_SCAN 589845
> > E: 0.000001 0001 02c4 0001 # EV_KEY / BTN_TRIGGER_HAPPY5 1
> > E: 0.000001 0000 0000 0000 # ------------ SYN_REPORT (0) ---------- +0ms
> > E: 0.033496 0004 0004 589845 # EV_MSC / MSC_SCAN 589845
> > E: 0.033496 0001 02c4 0000 # EV_KEY / BTN_TRIGGER_HAPPY5 0
> > E: 0.033496 0000 0000 0000 # ------------ SYN_REPORT (0) ---------- +33ms
> > E: 1.726818 0004 0004 589846 # EV_MSC / MSC_SCAN 589846
> > E: 1.726818 0001 02c5 0001 # EV_KEY / BTN_TRIGGER_HAPPY6 1
> > E: 1.726818 0000 0000 0000 # ------------ SYN_REPORT (0) ---------- +1693ms
> > E: 1.843421 0004 0004 589846 # EV_MSC / MSC_SCAN 589846
> > E: 1.843421 0001 02c5 0000 # EV_KEY / BTN_TRIGGER_HAPPY6 0
> > E: 1.843421 0000 0000 0000 # ------------ SYN_REPORT (0) ---------- +117ms
> > E: 2.736883 0004 0004 589847 # EV_MSC / MSC_SCAN 589847
> > E: 2.736883 0001 02c6 0001 # EV_KEY / BTN_TRIGGER_HAPPY7 1
> > E: 2.736883 0000 0000 0000 # ------------ SYN_REPORT (0) ---------- +893ms
> > E: 2.826849 0004 0004 589847 # EV_MSC / MSC_SCAN 589847
> > E: 2.826849 0001 02c6 0000 # EV_KEY / BTN_TRIGGER_HAPPY7 0
> > E: 2.826849 0000 0000 0000 # ------------ SYN_REPORT (0) ---------- +90ms
> > E: 3.300247 0004 0004 589848 # EV_MSC / MSC_SCAN 589848
> > E: 3.300247 0001 02c7 0001 # EV_KEY / BTN_TRIGGER_HAPPY8 1
> > E: 3.300247 0000 0000 0000 # ------------ SYN_REPORT (0) ---------- +474ms
> > E: 3.373377 0004 0004 589848 # EV_MSC / MSC_SCAN 589848
> > E: 3.373377 0001 02c7 0000 # EV_KEY / BTN_TRIGGER_HAPPY8 0
> > E: 3.373377 0000 0000 0000 # ------------ SYN_REPORT (0) ---------- +73ms
> >
> > Also via USB, evemu recognizes the controller as "Microsoft X-Box One
> > Elite 2 pad". With Bluetooth it's seen as "Xbox Wireless Controller".
> > This happens with or without your merge request.
>
> This is expected and also kind of normal. We are not connecting
> through the same transport, so one should at least have the "wireless"
> in it. Rest is just how the device itself has those strings stored, so
> we should probably just say it's fine.
>
> >
> > USB
> >
> > > evemu-record
> > Available devices:
> > /dev/input/event17: Microsoft X-Box One Elite 2 pad
> >
> > Bluetooth
> >
> > > evemu-record
> > Available devices:
> > /dev/input/event16: Xbox Wireless Controller
> >
> >
> > > - does this mess up with SDL over hidraw (so keeping your udev rule in place)
> >
> > SDL seems to recognize the controller as 'Xbox One Elite 2 Controller'
> > over both USB and Bluetooth. Though the ProductID is "000b" with USB
> > and "220b" via bluetooth. Also, SDL, recognized the "hat" via USB,
> > but sees them as 4 more buttons through BT. I'm using
> > Grumbel/sdl-jstest.git to test. This happens with or without your MR.
>
> Is this hat difference an issue? If so we can try to fix it, again,
> and if not we can then leave it as it is.
>
> >
> > USB:
> > Joystick Name: 'Xbox One Elite 2 Controller'
> > Joystick GUID: 030010f85e040000000b000011050000
> > Joystick Number: 0
> > Number of Axes: 6
> > Number of Buttons: 15
> > Number of Hats: 1
> > Number of Balls: 0
> > GameControllerConfig:
> > Name: 'Xbox One Elite 2 Controller'
> > Mapping: '030010f85e040000000b000011050000,Xbox One Elite 2
> > Controller,a:b0,b:b1,x:b2,y:b3,back:b6,guide:b8,start:b7,leftstick:b9,rightstick:b10,leftshoulder:b4,rightshould
> > er:b5,dpup:h0.1,dpdown:h0.4,dpleft:h0.8,dpright:h0.2,paddle1:b11,paddle2:b13,paddle3:b12,paddle4:b14,leftx:a0,lefty:a1,rightx:a3,righty:a4,lefttrigger:a2,righttrigger:a5,crc:
> > f810,platform:Linux'
> >
> > Bluetooth:
> > Joystick Name: 'Xbox One Elite 2 Controller'
> > Joystick GUID: 030018dc5e040000220b000000006800
> > Joystick Number: 0
> > Number of Axes: 6
> > Number of Buttons: 19
> > Number of Hats: 0
> > Number of Balls: 0
> > GameControllerConfig:
> > Name: 'Xbox One Elite 2 Controller'
> > Mapping: '030018dc5e040000220b000000006800,*,a:b0,b:b1,back:b4,dpdown:b12,dpleft:b13,dpright:b14,dpup:b11,guide:b5,leftshoulder:b9,leftstick:b7,lefttrigger:a4,leftx:a0,left
> > y:a1,rightshoulder:b10,rightstick:b8,righttrigger:a5,rightx:a2,righty:a3,start:b6,x:b2,y:b3,paddle1:b15,paddle2:b17,paddle3:b16,paddle4:b18,crc:dc18,platform:Linux'
> >
> >
> > Otherwise, no issues, Steam recognizes all buttons including paddles
> > correctly (with or without your MR). But it also names the controller
> > "Xbox One Elite 2 Controller" via USB and "Xbox One Controller" when
> > using Bluetooth. This doesn't seem to have any effect on registering
> > the paddles correctly though.
>
> \o/
>
> >
> >
> > >
> > > >
> > > > I've never recompiled a module before, but I've compiled lots of other
> > > > stuff before. How hard can it be?
> > > >
> > >
> > > Technically it's not hard, I'm sure there is a proper Arch howto
> > > available. But it's painful because you'll have to maintain &
> > > recompile your kernel until your fix gets in the standard kernel of
> > > your distribution (archlinux if I'm not wrong).
> > >
> > > But the good point is that with HID-BPF that pain is relieved: I can
> > > provide you with the binary, you can check it if you like (the loader
> > > is in rust and the bpf itself is C), and then once installed you can
> > > forget about it.
> >
> > Let's try it!
>
> Well the more I think of it, the more I think we'll stick to the current state:
> - SDL is actually fixed by the udev rule (they just prefer talk over hidraw)
> - for evemu users, we have a bpf filter that works. I'll eventually
> merge those in the kernel so they are automatically shipped.
>
> So I don't really see the point of fixing a driver for the sake of
> fixing it and just losing both of our time.
>
> Thanks a lot for the quick testing.
>
> Cheers,
> Benjamin
>
^ permalink raw reply
* [PATCH 0/2] Input: struct bus_type cleanup
From: Ricardo B. Marliere @ 2024-02-10 15:15 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: linux-input, linux-kernel, Greg Kroah-Hartman,
Ricardo B. Marliere
This series is part of an effort to cleanup the users of the driver
core, as can be seen in many recent patches authored by Greg across the
tree (e.g. [1]).
---
[1]: https://lore.kernel.org/lkml/?q=f%3Agregkh%40linuxfoundation.org+s%3A%22make%22+and+s%3A%22const%22
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Ricardo B. Marliere <ricardo@marliere.net>
---
Ricardo B. Marliere (2):
Input: synaptics-rmi4 - make rmi_bus_type const
Input: serio - make serio_bus const
drivers/input/rmi4/rmi_bus.c | 2 +-
drivers/input/rmi4/rmi_bus.h | 2 +-
drivers/input/serio/serio.c | 2 +-
include/linux/serio.h | 2 +-
4 files changed, 4 insertions(+), 4 deletions(-)
---
base-commit: d03f030115fe930de1222fef294730ba21b93045
change-id: 20240210-bus_cleanup-input2-e7ef5a63e8c4
Best regards,
--
Ricardo B. Marliere <ricardo@marliere.net>
^ permalink raw reply
* [PATCH 1/2] Input: synaptics-rmi4 - make rmi_bus_type const
From: Ricardo B. Marliere @ 2024-02-10 15:15 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: linux-input, linux-kernel, Greg Kroah-Hartman,
Ricardo B. Marliere
In-Reply-To: <20240210-bus_cleanup-input2-v1-0-0daef7e034e0@marliere.net>
Now that the driver core can properly handle constant struct bus_type,
move the variable rmi_bus_type to be a constant structure as well,
placing it into read-only memory which can not be modified at runtime.
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Suggested-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Ricardo B. Marliere <ricardo@marliere.net>
---
drivers/input/rmi4/rmi_bus.c | 2 +-
drivers/input/rmi4/rmi_bus.h | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/input/rmi4/rmi_bus.c b/drivers/input/rmi4/rmi_bus.c
index 1b45b1d3077d..343030290d78 100644
--- a/drivers/input/rmi4/rmi_bus.c
+++ b/drivers/input/rmi4/rmi_bus.c
@@ -344,7 +344,7 @@ static int rmi_bus_match(struct device *dev, struct device_driver *drv)
return physical || rmi_function_match(dev, drv);
}
-struct bus_type rmi_bus_type = {
+const struct bus_type rmi_bus_type = {
.match = rmi_bus_match,
.name = "rmi4",
};
diff --git a/drivers/input/rmi4/rmi_bus.h b/drivers/input/rmi4/rmi_bus.h
index 25df6320f9f1..ea46ad9447ec 100644
--- a/drivers/input/rmi4/rmi_bus.h
+++ b/drivers/input/rmi4/rmi_bus.h
@@ -185,7 +185,7 @@ static inline int rmi_write_block(struct rmi_device *d, u16 addr,
int rmi_for_each_dev(void *data, int (*func)(struct device *dev, void *data));
-extern struct bus_type rmi_bus_type;
+extern const struct bus_type rmi_bus_type;
int rmi_of_property_read_u32(struct device *dev, u32 *result,
const char *prop, bool optional);
--
2.43.0
^ permalink raw reply related
* [PATCH 2/2] Input: serio - make serio_bus const
From: Ricardo B. Marliere @ 2024-02-10 15:15 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: linux-input, linux-kernel, Greg Kroah-Hartman,
Ricardo B. Marliere
In-Reply-To: <20240210-bus_cleanup-input2-v1-0-0daef7e034e0@marliere.net>
Now that the driver core can properly handle constant struct bus_type,
move the serio_bus variable to be a constant structure as well,
placing it into read-only memory which can not be modified at runtime.
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Suggested-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Ricardo B. Marliere <ricardo@marliere.net>
---
drivers/input/serio/serio.c | 2 +-
include/linux/serio.h | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/input/serio/serio.c b/drivers/input/serio/serio.c
index 767fc9efb4a8..a8838b522627 100644
--- a/drivers/input/serio/serio.c
+++ b/drivers/input/serio/serio.c
@@ -1007,7 +1007,7 @@ irqreturn_t serio_interrupt(struct serio *serio,
}
EXPORT_SYMBOL(serio_interrupt);
-struct bus_type serio_bus = {
+const struct bus_type serio_bus = {
.name = "serio",
.drv_groups = serio_driver_groups,
.match = serio_bus_match,
diff --git a/include/linux/serio.h b/include/linux/serio.h
index 6c27d413da92..7ca41af93b37 100644
--- a/include/linux/serio.h
+++ b/include/linux/serio.h
@@ -15,7 +15,7 @@
#include <linux/mod_devicetable.h>
#include <uapi/linux/serio.h>
-extern struct bus_type serio_bus;
+extern const struct bus_type serio_bus;
struct serio {
void *port_data;
--
2.43.0
^ permalink raw reply related
* Re: [PATCH v5 0/4] Add support of color temperature and chromaticity
From: Jonathan Cameron @ 2024-02-10 16:14 UTC (permalink / raw)
To: Srinivas Pandruvada
Cc: jikos, lars, Basavaraj.Natikar, linux-input, linux-iio,
linux-kernel
In-Reply-To: <20240205185926.3030521-1-srinivas.pandruvada@linux.intel.com>
On Mon, 5 Feb 2024 10:59:22 -0800
Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com> wrote:
> The original series submitted to 6.7 (before revert) is modified to
> solve regression issues on several platforms. There are two changes
> introduced before adding support for new features to allow dynamic
> addition of channels.
Applied and pushed out as testing for all the normal reasons (0-day
gets to play with it!)
Thanks,
Jonathan
>
> v5:
> Change als_scan_mask to per state instead of global
> v4:
> Addressed comments from Jonathan and Basavaraj
> v3:
> Addressed comments for v2, details in each patch.
> v2:
> New change to add channels dynamically
> Modified color temperature and chromaticity to skip in case
> of failures
>
>
> Basavaraj Natikar (2):
> iio: hid-sensor-als: Add light color temperature support
> iio: hid-sensor-als: Add light chromaticity support
>
> Srinivas Pandruvada (2):
> iio: hid-sensor-als: Assign channels dynamically
> iio: hid-sensor-als: Remove hardcoding of values for enums
>
> drivers/iio/light/hid-sensor-als.c | 122 ++++++++++++++++++++++++-----
> include/linux/hid-sensor-ids.h | 4 +
> 2 files changed, 108 insertions(+), 18 deletions(-)
>
^ permalink raw reply
* [PATCH v1 0/1] input: touchcreen: tsc2007: make interrupt optional
From: Svyatoslav Ryhel @ 2024-02-10 17:55 UTC (permalink / raw)
To: Dmitry Torokhov, Uwe Kleine-König, Benjamin Bara,
Richard Leitner
Cc: linux-input, linux-kernel
In case tsc2007 is used as an ADC sensor there will be no interrupt
provided at all, so set up an interrupt only if one is present and
remove associated warning.
Svyatoslav Ryhel (1):
input: touchcreen: tsc2007: make interrupt optional
drivers/input/touchscreen/tsc2007_core.c | 30 +++++++++++++-----------
1 file changed, 16 insertions(+), 14 deletions(-)
--
2.40.1
^ permalink raw reply
* [PATCH v1 1/1] input: touchcreen: tsc2007: make interrupt optional
From: Svyatoslav Ryhel @ 2024-02-10 17:55 UTC (permalink / raw)
To: Dmitry Torokhov, Uwe Kleine-König, Benjamin Bara,
Richard Leitner
Cc: linux-input, linux-kernel
In-Reply-To: <20240210175530.137361-1-clamor95@gmail.com>
In case tsc2007 is used as an ADC sensor there will be no interrupt
provided at all, so set up an interrupt only if one is present and
remove associated warning.
Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
---
drivers/input/touchscreen/tsc2007_core.c | 30 +++++++++++++-----------
1 file changed, 16 insertions(+), 14 deletions(-)
diff --git a/drivers/input/touchscreen/tsc2007_core.c b/drivers/input/touchscreen/tsc2007_core.c
index b3655250d4a7..f6bb12ebf97f 100644
--- a/drivers/input/touchscreen/tsc2007_core.c
+++ b/drivers/input/touchscreen/tsc2007_core.c
@@ -178,7 +178,8 @@ static void tsc2007_stop(struct tsc2007 *ts)
mb();
wake_up(&ts->wait);
- disable_irq(ts->irq);
+ if (ts->irq)
+ disable_irq(ts->irq);
}
static int tsc2007_open(struct input_dev *input_dev)
@@ -189,7 +190,8 @@ static int tsc2007_open(struct input_dev *input_dev)
ts->stopped = false;
mb();
- enable_irq(ts->irq);
+ if (ts->irq)
+ enable_irq(ts->irq);
/* Prepare for touch readings - power down ADC and enable PENIRQ */
err = tsc2007_xfer(ts, PWRDOWN);
@@ -253,8 +255,6 @@ static int tsc2007_probe_properties(struct device *dev, struct tsc2007 *ts)
if (ts->gpiod)
ts->get_pendown_state = tsc2007_get_pendown_state_gpio;
- else
- dev_warn(dev, "Pen down GPIO is not specified in properties\n");
return 0;
}
@@ -362,17 +362,19 @@ static int tsc2007_probe(struct i2c_client *client)
pdata->init_platform_hw();
}
- err = devm_request_threaded_irq(&client->dev, ts->irq,
- NULL, tsc2007_soft_irq,
- IRQF_ONESHOT,
- client->dev.driver->name, ts);
- if (err) {
- dev_err(&client->dev, "Failed to request irq %d: %d\n",
- ts->irq, err);
- return err;
- }
+ if (ts->irq) {
+ err = devm_request_threaded_irq(&client->dev, ts->irq,
+ NULL, tsc2007_soft_irq,
+ IRQF_ONESHOT,
+ client->dev.driver->name, ts);
+ if (err) {
+ dev_err(&client->dev, "Failed to request irq %d: %d\n",
+ ts->irq, err);
+ return err;
+ }
- tsc2007_stop(ts);
+ tsc2007_stop(ts);
+ }
/* power down the chip (TSC2007_SETUP does not ACK on I2C) */
err = tsc2007_xfer(ts, PWRDOWN);
--
2.40.1
^ permalink raw reply related
* [PATCH v1 00/19] hid-ft260: Fixes for serial driver patch v4
From: Michael Zaidman @ 2024-02-10 21:51 UTC (permalink / raw)
To: chrysh, daniel.beer, jikos
Cc: linux-kernel, linux-input, linux-serial, ilpo.jarvinen, johan,
gregkh, equinox, michael.zaidman
Modifications on top of "[PATCH v4 RESEND] hid-ft260: Add serial driver"
https://lore.kernel.org/all/20231218093153.192268-1-contact@christina-quast.de/
They are mostly the fixes to the original v4 patch and should be melded into
the v5 patch rather than upstreamed as separate patches.
Michael Zaidman (19):
hid-ft260: fix incompatible-pointer-types error
hid-ft260: fix Wformat warning
hid-ft260: fix i2c driver regression in ft260_raw_event
hid-ft260: remove dead code in ft260_uart_receive_chars
hid-ft260: fix unprotected write_buf concurrent access
hid-ft260: uart: enable wakeup workaround
hid-ft260: depend wakeup workaround activation on uart baud rate
hid-ft260: depend wakeup workaround activation on eeprom config
hid-ft260: uart: wakeup device early to not lose rx data
hid-ft260: uart: do not configure baud rate twice
hid-ft260: uart: do not disable wakeup workaround twice
hid-ft260: uart: use kfifo_avail for fifo write room check
hid-ft260: improve usb interface type detection logic
hid-ft260: uart: cleanup and refactoring
hid-ft260: uart: remove FIXME for wake-up workaround
hid-ft260: uart: suppress unhandled report 0xb1 dmesg
hid-ft260: uart: arm wake-up timer unconditionally on tty session start
hid-ft260: uart: fix rx data loss after device reopening
hid-ft260: uart: improve write performance
drivers/hid/hid-ft260.c | 330 ++++++++++++++++++++++------------------
1 file changed, 185 insertions(+), 145 deletions(-)
--
2.40.1
^ permalink raw reply
* [PATCH v1 01/19] hid-ft260: fix incompatible-pointer-types error
From: Michael Zaidman @ 2024-02-10 21:51 UTC (permalink / raw)
To: chrysh, daniel.beer, jikos
Cc: linux-kernel, linux-input, linux-serial, ilpo.jarvinen, johan,
gregkh, equinox, michael.zaidman
In-Reply-To: <20240210215147.77629-1-michael.zaidman@gmail.com>
Fixed compilation error introduced by "hid-ft260: Add serial driver" patch
https://lore.kernel.org/all/20231218093153.192268-1-contact@christina-quast.de/
You are using: gcc-13 (Ubuntu 13.2.0-4ubuntu3) 13.2.0
CC [M] /home/michael/sw/hid-ft260/hid-ft260.o
/home/michael/sw/hid-ft260/hid-ft260.c:1426:35: error: initialization of ‘int (*)(struct tty_struct *,
const unsigned char *, int)’ from incompatible pointer type ‘ssize_t (*)(struct tty_struct *,
const unsigned char *, size_t)’ {aka ‘long int (*)(struct tty_struct *, const unsigned char *,
long unsigned int)’} [-Werror=incompatible-pointer-types]
1426 | .write = ft260_uart_write,
| ^~~~~~~~~~~~~~~~
/home/michael/sw/hid-ft260/hid-ft260.c:1426:35: note: (near initialization for ‘ft260_uart_ops.write’)
cc1: some warnings being treated as errors
make[3]: *** [scripts/Makefile.build:251: /home/michael/sw/hid-ft260/hid-ft260.o] Error 1
make[2]: *** [/usr/src/linux-headers-6.5.0-14-generic/Makefile:2037: /home/michael/sw/hid-ft260] Error 2
make[1]: *** [Makefile:234: __sub-make] Error 2
Signed-off-by: Michael Zaidman <michael.zaidman@gmail.com>
---
drivers/hid/hid-ft260.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/hid/hid-ft260.c b/drivers/hid/hid-ft260.c
index cc2cce3698e3..7273d401337a 100644
--- a/drivers/hid/hid-ft260.c
+++ b/drivers/hid/hid-ft260.c
@@ -1220,8 +1220,8 @@ static int ft260_uart_receive_chars(struct ft260_device *port,
return ret;
}
-static ssize_t ft260_uart_write(struct tty_struct *tty, const unsigned char *buf,
- size_t count)
+static int ft260_uart_write(struct tty_struct *tty, const unsigned char *buf,
+ int count)
{
struct ft260_device *port = tty->driver_data;
struct hid_device *hdev = port->hdev;
--
2.40.1
^ permalink raw reply related
* [PATCH v1 02/19] hid-ft260: fix Wformat warning
From: Michael Zaidman @ 2024-02-10 21:51 UTC (permalink / raw)
To: chrysh, daniel.beer, jikos
Cc: linux-kernel, linux-input, linux-serial, ilpo.jarvinen, johan,
gregkh, equinox, michael.zaidman
In-Reply-To: <20240210215147.77629-1-michael.zaidman@gmail.com>
Fixed compilation warning introduced by "hid-ft260: Add serial driver" patch
https://lore.kernel.org/all/20231218093153.192268-1-contact@christina-quast.de/
You are using: gcc-13 (Ubuntu 13.2.0-4ubuntu3) 13.2.0
CC [M] /home/michael/sw/hid-ft260/hid-ft260.o
In file included from ./include/linux/kernel.h:30,
from ./arch/x86/include/asm/percpu.h:27,
from ./arch/x86/include/asm/preempt.h:6,
from ./include/linux/preempt.h:79,
from ./include/linux/spinlock.h:56,
from ./include/linux/mmzone.h:8,
from ./include/linux/gfp.h:7,
from ./include/linux/slab.h:16,
from ./include/linux/hid.h:19,
from ./include/uapi/linux/hidraw.h:19,
from ./include/linux/hidraw.h:8,
from /home/michael/sw/hid-ft260/hid-ft260.c:12:
/home/michael/sw/hid-ft260/hid-ft260.c: In function ‘ft260_uart_write’:
./include/linux/kern_levels.h:5:25: warning: format ‘%ld’ expects argument of type ‘long int’,
but argument 3 has type ‘int’ [-Wformat=]
5 | #define KERN_SOH "\001" /* ASCII Start Of Header */
| ^~~~~~
./include/linux/printk.h:427:25: note: in definition of macro ‘printk_index_wrap’
427 | _p_func(_fmt, ##__VA_ARGS__); \
| ^~~~
./include/linux/printk.h:528:9: note: in expansion of macro ‘printk’
528 | printk(KERN_INFO pr_fmt(fmt), ##__VA_ARGS__)
| ^~~~~~
./include/linux/kern_levels.h:14:25: note: in expansion of macro ‘KERN_SOH’
14 | #define KERN_INFO KERN_SOH "6" /* informational */
| ^~~~~~~~
./include/linux/printk.h:528:16: note: in expansion of macro ‘KERN_INFO’
528 | printk(KERN_INFO pr_fmt(fmt), ##__VA_ARGS__)
| ^~~~~~~~~
/home/michael/sw/hid-ft260/hid-ft260.c:38:25: note: in expansion of macro ‘pr_info’
38 | pr_info("%s: " format, __func__, ##arg); \
| ^~~~~~~
/home/michael/sw/hid-ft260/hid-ft260.c:1231:9: note: in expansion of macro ‘ft260_dbg’
1231 | ft260_dbg("count: %ld, len: %d", count, len);
| ^~~~~~~~~
Signed-off-by: Michael Zaidman <michael.zaidman@gmail.com>
---
drivers/hid/hid-ft260.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/hid/hid-ft260.c b/drivers/hid/hid-ft260.c
index 7273d401337a..a67c625c9165 100644
--- a/drivers/hid/hid-ft260.c
+++ b/drivers/hid/hid-ft260.c
@@ -1228,7 +1228,7 @@ static int ft260_uart_write(struct tty_struct *tty, const unsigned char *buf,
int len, ret;
len = kfifo_in_locked(&port->xmit_fifo, buf, count, &port->write_lock);
- ft260_dbg("count: %ld, len: %d", count, len);
+ ft260_dbg("count: %d, len: %d", count, len);
ret = ft260_uart_transmit_chars(port);
if (ret < 0) {
--
2.40.1
^ permalink raw reply related
* [PATCH v1 03/19] hid-ft260: fix i2c driver regression in ft260_raw_event
From: Michael Zaidman @ 2024-02-10 21:51 UTC (permalink / raw)
To: chrysh, daniel.beer, jikos
Cc: linux-kernel, linux-input, linux-serial, ilpo.jarvinen, johan,
gregkh, equinox, michael.zaidman
In-Reply-To: <20240210215147.77629-1-michael.zaidman@gmail.com>
Fix i2c regression in ft260_raw_event introduced by
"[PATCH v4 RESEND] hid-ft260: Add serial driver" patch:
https://lore.kernel.org/all/20231218093153.192268-1-contact@christina-quast.de/
It caused wrong printout per every hid input report:
[21912.237393] ft260 0003:0403:6030.000C: unhandled report 0xde
[21912.247405] ft260 0003:0403:6030.000C: unhandled report 0xde
[21912.249403] ft260 0003:0403:6030.000C: unhandled report 0xd1
[21912.275399] ft260 0003:0403:6030.000C: unhandled report 0xde
[21912.285404] ft260 0003:0403:6030.000C: unhandled report 0xde
[21912.287403] ft260 0003:0403:6030.000C: unhandled report 0xd1
[21912.315406] ft260 0003:0403:6030.000C: unhandled report 0xde
[21912.326390] ft260 0003:0403:6030.000C: unhandled report 0xde
[21912.327402] ft260 0003:0403:6030.000C: unhandled report 0xd1
Signed-off-by: Michael Zaidman <michael.zaidman@gmail.com>
---
drivers/hid/hid-ft260.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/hid/hid-ft260.c b/drivers/hid/hid-ft260.c
index a67c625c9165..a348f11600c6 100644
--- a/drivers/hid/hid-ft260.c
+++ b/drivers/hid/hid-ft260.c
@@ -1755,6 +1755,8 @@ static int ft260_raw_event(struct hid_device *hdev, struct hid_report *report,
if (dev->read_idx == dev->read_len)
complete(&dev->wait);
+ return 0;
+
} else if (xfer->length > FT260_RD_DATA_MAX) {
hid_err(hdev, "Received data too long (%d)\n", xfer->length);
return -EBADR;
--
2.40.1
^ permalink raw reply related
* [PATCH v1 04/19] hid-ft260: remove dead code in ft260_uart_receive_chars
From: Michael Zaidman @ 2024-02-10 21:51 UTC (permalink / raw)
To: chrysh, daniel.beer, jikos
Cc: linux-kernel, linux-input, linux-serial, ilpo.jarvinen, johan,
gregkh, equinox, michael.zaidman
In-Reply-To: <20240210215147.77629-1-michael.zaidman@gmail.com>
Remove conditional expression, which will never be true here since
it is already filtered in the ft260_raw_event procedure.
Signed-off-by: Michael Zaidman <michael.zaidman@gmail.com>
---
drivers/hid/hid-ft260.c | 9 ++-------
1 file changed, 2 insertions(+), 7 deletions(-)
diff --git a/drivers/hid/hid-ft260.c b/drivers/hid/hid-ft260.c
index a348f11600c6..77638cae595e 100644
--- a/drivers/hid/hid-ft260.c
+++ b/drivers/hid/hid-ft260.c
@@ -1202,16 +1202,11 @@ static int ft260_uart_receive_chars(struct ft260_device *port,
u8 *data, u8 length)
{
struct hid_device *hdev = port->hdev;
- int ret = 0;
-
- if (length > FT260_RD_DATA_MAX) {
- hid_err(hdev, "Received too much data (%d)\n", length);
- return -EBADR;
- }
+ int ret;
ret = tty_insert_flip_string(&port->port, data, length);
if (ret != length)
- hid_err(hdev, "%d char not inserted to flip buffer\n", length - ret);
+ hid_dbg(hdev, "%d char not inserted to flip buffer\n", length - ret);
port->icount.rx += ret;
if (ret)
--
2.40.1
^ permalink raw reply related
* [PATCH v1 05/19] hid-ft260: fix unprotected write_buf concurrent access
From: Michael Zaidman @ 2024-02-10 21:51 UTC (permalink / raw)
To: chrysh, daniel.beer, jikos
Cc: linux-kernel, linux-input, linux-serial, ilpo.jarvinen, johan,
gregkh, equinox, michael.zaidman
In-Reply-To: <20240210215147.77629-1-michael.zaidman@gmail.com>
The UART code uses the write_buf unsafely, compromising the data integrity
of both I2C and UART channels.
The I2C channel uses the write_buf to send the HID reports. It uses mutex
to make it atomically. For UART to use this buffer, it should grab the
same mutex first. But then it will degrade the performance of both
channels. The better approach is to have a separate Tx buffer for UART.
I fixed it and briefly tested the data integrity simultaneously writing
via I2C and UART channels.
Signed-off-by: Michael Zaidman <michael.zaidman@gmail.com>
---
drivers/hid/hid-ft260.c | 32 ++++++++++++++++++--------------
1 file changed, 18 insertions(+), 14 deletions(-)
diff --git a/drivers/hid/hid-ft260.c b/drivers/hid/hid-ft260.c
index 77638cae595e..3d6beac0b8b6 100644
--- a/drivers/hid/hid-ft260.c
+++ b/drivers/hid/hid-ft260.c
@@ -38,9 +38,12 @@ MODULE_PARM_DESC(debug, "Toggle FT260 debugging messages");
pr_info("%s: " format, __func__, ##arg); \
} while (0)
-#define FT260_REPORT_MAX_LENGTH (64)
-#define FT260_I2C_DATA_REPORT_ID(len) (FT260_I2C_REPORT_MIN + (len - 1) / 4)
-#define FT260_UART_DATA_REPORT_ID(len) (FT260_UART_REPORT_MIN + (len - 1) / 4)
+#define FT260_REPORT_MAX_LEN (64)
+#define FT260_DATA_REPORT_ID(min, len) (min + (len - 1) / 4)
+#define FT260_I2C_DATA_REPORT_ID(len) \
+ FT260_DATA_REPORT_ID(FT260_I2C_REPORT_MIN, len)
+#define FT260_UART_DATA_REPORT_ID(len) \
+ FT260_DATA_REPORT_ID(FT260_UART_REPORT_MIN, len)
#define FT260_WAKEUP_NEEDED_AFTER_MS (4800) /* 5s minus 200ms margin */
@@ -56,7 +59,8 @@ MODULE_PARM_DESC(debug, "Toggle FT260 debugging messages");
* read payload length to be 180 bytes.
*/
#define FT260_RD_DATA_MAX (180)
-#define FT260_WR_DATA_MAX (60)
+#define FT260_WR_I2C_DATA_MAX (60)
+#define FT260_WR_UART_DATA_MAX (62)
/*
* Device interface configuration.
@@ -229,7 +233,7 @@ struct ft260_i2c_write_request_report {
u8 address; /* 7-bit I2C address */
u8 flag; /* I2C transaction condition */
u8 length; /* data payload length */
- u8 data[FT260_WR_DATA_MAX]; /* data payload */
+ u8 data[FT260_WR_I2C_DATA_MAX]; /* data payload */
} __packed;
struct ft260_i2c_read_request_report {
@@ -249,7 +253,7 @@ struct ft260_input_report {
struct ft260_uart_write_request_report {
u8 report; /* FT260_UART_REPORT */
u8 length; /* data payload length */
- u8 data[] __counted_by(length); /* variable data payload */
+ u8 data[FT260_WR_UART_DATA_MAX]; /* data payload */
} __packed;
struct ft260_configure_uart_request {
@@ -318,10 +322,10 @@ struct ft260_device {
struct work_struct wakeup_work;
bool reschedule_work;
-
struct completion wait;
struct mutex lock;
- u8 write_buf[FT260_REPORT_MAX_LENGTH];
+ u8 i2c_wr_buf[FT260_REPORT_MAX_LEN];
+ u8 uart_wr_buf[FT260_REPORT_MAX_LEN];
unsigned long need_wakeup_at;
u8 *read_buf;
u16 read_idx;
@@ -503,7 +507,7 @@ static int ft260_i2c_write(struct ft260_device *dev, u8 addr, u8 *data,
int ret, wr_len, idx = 0;
struct hid_device *hdev = dev->hdev;
struct ft260_i2c_write_request_report *rep =
- (struct ft260_i2c_write_request_report *)dev->write_buf;
+ (struct ft260_i2c_write_request_report *)dev->i2c_wr_buf;
if (len < 1)
return -EINVAL;
@@ -511,12 +515,12 @@ static int ft260_i2c_write(struct ft260_device *dev, u8 addr, u8 *data,
rep->flag = FT260_FLAG_START;
do {
- if (len <= FT260_WR_DATA_MAX) {
+ if (len <= FT260_WR_I2C_DATA_MAX) {
wr_len = len;
if (flag == FT260_FLAG_START_STOP)
rep->flag |= FT260_FLAG_STOP;
} else {
- wr_len = FT260_WR_DATA_MAX;
+ wr_len = FT260_WR_I2C_DATA_MAX;
}
rep->report = FT260_I2C_DATA_REPORT_ID(wr_len);
@@ -552,7 +556,7 @@ static int ft260_smbus_write(struct ft260_device *dev, u8 addr, u8 cmd,
int len = 4;
struct ft260_i2c_write_request_report *rep =
- (struct ft260_i2c_write_request_report *)dev->write_buf;
+ (struct ft260_i2c_write_request_report *)dev->i2c_wr_buf;
if (data_len >= sizeof(rep->data))
return -EINVAL;
@@ -1167,10 +1171,10 @@ static int ft260_uart_transmit_chars(struct ft260_device *port)
goto tty_out;
}
- rep = (struct ft260_uart_write_request_report *)port->write_buf;
+ rep = (struct ft260_uart_write_request_report *)port->uart_wr_buf;
do {
- len = min(data_len, FT260_WR_DATA_MAX);
+ len = min(data_len, FT260_WR_UART_DATA_MAX);
rep->report = FT260_UART_DATA_REPORT_ID(len);
rep->length = len;
--
2.40.1
^ permalink raw reply related
* [PATCH v1 06/19] hid-ft260: uart: enable wakeup workaround
From: Michael Zaidman @ 2024-02-10 21:51 UTC (permalink / raw)
To: chrysh, daniel.beer, jikos
Cc: linux-kernel, linux-input, linux-serial, ilpo.jarvinen, johan,
gregkh, equinox, michael.zaidman
In-Reply-To: <20240210215147.77629-1-michael.zaidman@gmail.com>
The FT260 has a "power saving mode" that causes the device to switch
to a 30 kHz oscillator if there's no activity for 5 seconds.
Unfortunately, this mode can only be disabled by reprogramming
internal fuses, or external eeprom if exists.
One effect of this mode is to cause data loss on an Rx line at baud
rates higher than 4800 after being idle for longer than 5 seconds.
We work around this by sending a dummy report at least once per 4.8
seconds if the UART is in use. But it is not acctivated in the
"[PATCH v4 RESEND] hid-ft260: Add serial driver"
https://lore.kernel.org/all/20231218093153.192268-1-contact@christina-quast.de/
It causes data loss on the Rx line at the driver's default 9600 baud rate.
Enable periodic dummy report to prevent data loss on Rx line upon exiting
from power saving mode.
Signed-off-by: Michael Zaidman <michael.zaidman@gmail.com>
---
drivers/hid/hid-ft260.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/hid/hid-ft260.c b/drivers/hid/hid-ft260.c
index 3d6beac0b8b6..19599e64c6be 100644
--- a/drivers/hid/hid-ft260.c
+++ b/drivers/hid/hid-ft260.c
@@ -1575,7 +1575,7 @@ static int ft260_uart_probe(struct hid_device *hdev, struct ft260_device *dev)
INIT_WORK(&dev->wakeup_work, ft260_uart_do_wakeup);
// FIXME: Do I need that if I have cancel_work_sync?
// FIXME: are all kfifo access secured by lock? with irq or not?
- dev->reschedule_work = false;
+ dev->reschedule_work = true;
/* Work not started at this point */
timer_setup(&dev->wakeup_timer, ft260_uart_start_wakeup, 0);
--
2.40.1
^ permalink raw reply related
* [PATCH v1 07/19] hid-ft260: depend wakeup workaround activation on uart baud rate
From: Michael Zaidman @ 2024-02-10 21:51 UTC (permalink / raw)
To: chrysh, daniel.beer, jikos
Cc: linux-kernel, linux-input, linux-serial, ilpo.jarvinen, johan,
gregkh, equinox, michael.zaidman
In-Reply-To: <20240210215147.77629-1-michael.zaidman@gmail.com>
The ft260 chip enters power save mode after 5 seconds of being in idle
state. It causes the data loss on the Rx line. The implemented workaround
sends a dummy report every 4.8 seconds, preventing the chip from entering
the power save mode. But the baud rates below 4800 work fine without a
wakeup workaround.
This commit makes the wakeup workaround activation dependent on the UART
rate, allowing entering the power saving mode for the UART rates below
4800 bauds.
[11238.542841] ft260_uart_wakeup_workaraund_enable: Activate wakeup workaround
[11238.542843] ft260_uart_change_speed: Configured termios: flow control: 0, baudrate: 9600,
[11238.542845] ft260_uart_change_speed: data_bit: 8, parity: 0, stop_bit: 0, breaking: 0
[11238.543671] ft260_uart_wakeup_workaraund_enable: Deactivate wakeup workaround
[11238.543683] ft260_uart_change_speed: Configured termios: flow control: 0, baudrate: 4800,
[11238.543684] ft260_uart_change_speed: data_bit: 8, parity: 0, stop_bit: 0, breaking: 0
Signed-off-by: Michael Zaidman <michael.zaidman@gmail.com>
---
drivers/hid/hid-ft260.c | 48 +++++++++++++++++++++++++++++++----------
1 file changed, 37 insertions(+), 11 deletions(-)
diff --git a/drivers/hid/hid-ft260.c b/drivers/hid/hid-ft260.c
index 19599e64c6be..b24998092d22 100644
--- a/drivers/hid/hid-ft260.c
+++ b/drivers/hid/hid-ft260.c
@@ -296,6 +296,8 @@ enum {
FT260_CFG_BAUD_MAX = 12000000,
};
+#define FT260_UART_EN_PW_SAVE_BAUD 4800
+
static const struct hid_device_id ft260_devices[] = {
{ HID_USB_DEVICE(USB_VENDOR_ID_FUTURE_TECHNOLOGY,
USB_DEVICE_ID_FT260) },
@@ -1056,6 +1058,15 @@ static const struct attribute_group ft260_attr_group = {
static DEFINE_MUTEX(ft260_uart_list_lock);
static LIST_HEAD(ft260_uart_device_list);
+static void ft260_uart_wakeup(struct ft260_device *dev);
+
+static void ft260_uart_wakeup_workaraund_enable(struct ft260_device *port,
+ bool enable)
+{
+ port->reschedule_work = enable;
+ ft260_dbg("%s wakeup workaround", enable ? "Activate" : "Deactivate");
+}
+
static struct ft260_device *ft260_dev_by_index(int index)
{
struct ft260_device *port;
@@ -1108,7 +1119,7 @@ static void ft260_uart_port_remove(struct ft260_device *port)
spin_unlock(&port->write_lock);
mutex_lock(&port->port.mutex);
- port->reschedule_work = false;
+ ft260_uart_wakeup_workaraund_enable(port, false);
tty_port_tty_hangup(&port->port, false);
mutex_unlock(&port->port.mutex);
@@ -1266,8 +1277,11 @@ static int ft260_uart_change_speed(struct ft260_device *port,
struct hid_device *hdev = port->hdev;
unsigned int baud;
struct ft260_configure_uart_request req;
+ bool wakeup_workaraund = false;
int ret;
+ ft260_uart_wakeup(port);
+
memset(&req, 0, sizeof(req));
req.report = FT260_SYSTEM_SETTINGS;
@@ -1309,6 +1323,12 @@ static int ft260_uart_change_speed(struct ft260_device *port,
tty_encode_baud_rate(tty, baud, baud);
tty_kref_put(tty);
}
+
+ if (baud > FT260_UART_EN_PW_SAVE_BAUD)
+ wakeup_workaraund = true;
+
+ ft260_uart_wakeup_workaraund_enable(port, wakeup_workaraund);
+
put_unaligned_le32(cpu_to_le32(baud), &req.baudrate);
if (termios->c_cflag & CRTSCTS)
@@ -1435,13 +1455,13 @@ static const struct tty_operations ft260_uart_ops = {
/* The FT260 has a "power saving mode" that causes the device to switch
* to a 30 kHz oscillator if there's no activity for 5 seconds.
- * Unfortunately this mode can only be disabled by reprogramming
+ * Unfortunately, this mode can only be disabled by reprogramming
* internal fuses, which requires an additional programming voltage.
*
- * One effect of this mode is to cause data loss on a fast UART that
- * transmits after being idle for longer than 5 seconds. We work around
- * this by sending a dummy report at least once per 4 seconds if the
- * UART is in use.
+ * One effect of this mode is to cause data loss on an Rx line at baud
+ * rates higher than 4800 after being idle for longer than 5 seconds.
+ * We work around this by sending a dummy report at least once per 4.8
+ * seconds if the UART is in use.
*/
static void ft260_uart_start_wakeup(struct timer_list *t)
{
@@ -1455,10 +1475,8 @@ static void ft260_uart_start_wakeup(struct timer_list *t)
}
}
-static void ft260_uart_do_wakeup(struct work_struct *work)
+static void ft260_uart_wakeup(struct ft260_device *dev)
{
- struct ft260_device *dev =
- container_of(work, struct ft260_device, wakeup_work);
struct ft260_get_chip_version_report version;
int ret;
@@ -1472,12 +1490,20 @@ static void ft260_uart_do_wakeup(struct work_struct *work)
}
}
+static void ft260_uart_do_wakeup(struct work_struct *work)
+{
+ struct ft260_device *dev =
+ container_of(work, struct ft260_device, wakeup_work);
+
+ ft260_uart_wakeup(dev);
+}
+
static void ft260_uart_shutdown(struct tty_port *tport)
{
struct ft260_device *port =
container_of(tport, struct ft260_device, port);
- port->reschedule_work = false;
+ ft260_uart_wakeup_workaraund_enable(port, false);
}
static int ft260_uart_activate(struct tty_port *tport, struct tty_struct *tty)
@@ -1575,7 +1601,7 @@ static int ft260_uart_probe(struct hid_device *hdev, struct ft260_device *dev)
INIT_WORK(&dev->wakeup_work, ft260_uart_do_wakeup);
// FIXME: Do I need that if I have cancel_work_sync?
// FIXME: are all kfifo access secured by lock? with irq or not?
- dev->reschedule_work = true;
+ ft260_uart_wakeup_workaraund_enable(dev, true);
/* Work not started at this point */
timer_setup(&dev->wakeup_timer, ft260_uart_start_wakeup, 0);
--
2.40.1
^ permalink raw reply related
* [PATCH v1 08/19] hid-ft260: depend wakeup workaround activation on eeprom config
From: Michael Zaidman @ 2024-02-10 21:51 UTC (permalink / raw)
To: chrysh, daniel.beer, jikos
Cc: linux-kernel, linux-input, linux-serial, ilpo.jarvinen, johan,
gregkh, equinox, michael.zaidman
In-Reply-To: <20240210215147.77629-1-michael.zaidman@gmail.com>
Do not activate the 4.8-second wakeup workaround if power saving mode is
disabled in EEPROM.
Signed-off-by: Michael Zaidman <michael.zaidman@gmail.com>
---
drivers/hid/hid-ft260.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/hid/hid-ft260.c b/drivers/hid/hid-ft260.c
index b24998092d22..ccd20f590720 100644
--- a/drivers/hid/hid-ft260.c
+++ b/drivers/hid/hid-ft260.c
@@ -323,6 +323,7 @@ struct ft260_device {
struct timer_list wakeup_timer;
struct work_struct wakeup_work;
bool reschedule_work;
+ bool power_saving_en;
struct completion wait;
struct mutex lock;
@@ -889,6 +890,7 @@ static int ft260_get_interface_type(struct hid_device *hdev, struct ft260_device
ft260_dbg("uart_mode: 0x%02x\n", cfg.uart_mode);
dev->ft260_is_serial = false;
+ dev->power_saving_en = cfg.power_saving_en;
switch (cfg.chip_mode) {
case FT260_MODE_ALL:
@@ -1063,8 +1065,11 @@ static void ft260_uart_wakeup(struct ft260_device *dev);
static void ft260_uart_wakeup_workaraund_enable(struct ft260_device *port,
bool enable)
{
- port->reschedule_work = enable;
- ft260_dbg("%s wakeup workaround", enable ? "Activate" : "Deactivate");
+ if (port->power_saving_en) {
+ port->reschedule_work = enable;
+ ft260_dbg("%s wakeup workaround",
+ enable ? "Activate" : "Deactivate");
+ }
}
static struct ft260_device *ft260_dev_by_index(int index)
--
2.40.1
^ permalink raw reply related
* [PATCH v1 09/19] hid-ft260: uart: wakeup device early to not lose rx data
From: Michael Zaidman @ 2024-02-10 21:51 UTC (permalink / raw)
To: chrysh, daniel.beer, jikos
Cc: linux-kernel, linux-input, linux-serial, ilpo.jarvinen, johan,
gregkh, equinox, michael.zaidman
In-Reply-To: <20240210215147.77629-1-michael.zaidman@gmail.com>
Waking up the ft260 device from power saving mode earlier reduces the
probability of incoming data loss.
Signed-off-by: Michael Zaidman <michael.zaidman@gmail.com>
---
drivers/hid/hid-ft260.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/hid/hid-ft260.c b/drivers/hid/hid-ft260.c
index ccd20f590720..6266e4f1100d 100644
--- a/drivers/hid/hid-ft260.c
+++ b/drivers/hid/hid-ft260.c
@@ -1285,8 +1285,6 @@ static int ft260_uart_change_speed(struct ft260_device *port,
bool wakeup_workaraund = false;
int ret;
- ft260_uart_wakeup(port);
-
memset(&req, 0, sizeof(req));
req.report = FT260_SYSTEM_SETTINGS;
@@ -1529,6 +1527,9 @@ static int ft260_uart_activate(struct tty_port *tport, struct tty_struct *tty)
ft260_uart_change_speed(port, &tty->termios, NULL);
clear_bit(TTY_IO_ERROR, &tty->flags);
+ /* Wake up the chip as early as possible to not miss incoming data */
+ ft260_uart_wakeup(port);
+
if (port->reschedule_work) {
mod_timer(&port->wakeup_timer, jiffies +
msecs_to_jiffies(FT260_WAKEUP_NEEDED_AFTER_MS));
--
2.40.1
^ permalink raw reply related
* [PATCH v1 10/19] hid-ft260: uart: do not configure baud rate twice
From: Michael Zaidman @ 2024-02-10 21:51 UTC (permalink / raw)
To: chrysh, daniel.beer, jikos
Cc: linux-kernel, linux-input, linux-serial, ilpo.jarvinen, johan,
gregkh, equinox, michael.zaidman
In-Reply-To: <20240210215147.77629-1-michael.zaidman@gmail.com>
The uart speed settings are configured twice per session:
the old settings - by the ft260_uart_port_activate() and the new -
by the ft260_uart_set_termios() routine. We do not need to configure
the old settings per tty session since they have already been sent
to the device in the probe and passed to the tty via termios in the
ft260_driver_init.
Removed their coonfiguration from the ft260_uart_port_activate routine.
Signed-off-by: Michael Zaidman <michael.zaidman@gmail.com>
---
drivers/hid/hid-ft260.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/hid/hid-ft260.c b/drivers/hid/hid-ft260.c
index 6266e4f1100d..63839f02e9b5 100644
--- a/drivers/hid/hid-ft260.c
+++ b/drivers/hid/hid-ft260.c
@@ -1524,7 +1524,6 @@ static int ft260_uart_activate(struct tty_port *tport, struct tty_struct *tty)
kfifo_reset(&port->xmit_fifo);
spin_unlock(&port->write_lock);
- ft260_uart_change_speed(port, &tty->termios, NULL);
clear_bit(TTY_IO_ERROR, &tty->flags);
/* Wake up the chip as early as possible to not miss incoming data */
--
2.40.1
^ permalink raw reply related
* [PATCH v1 11/19] hid-ft260: uart: do not disable wakeup workaround twice
From: Michael Zaidman @ 2024-02-10 21:51 UTC (permalink / raw)
To: chrysh, daniel.beer, jikos
Cc: linux-kernel, linux-input, linux-serial, ilpo.jarvinen, johan,
gregkh, equinox, michael.zaidman
In-Reply-To: <20240210215147.77629-1-michael.zaidman@gmail.com>
The wakeup workaround is terminated twice: first in the
ft260_uart_port_shutdown and then, in the ft260_uart_port_remove routine.
We do not need to do it in the ft260_uart_port_remove routine since
it's always called when the wakeup mechanism is inactive:
1. Upon the ft260_uart_probe failure.
2. Upon the device/driver removal.
Signed-off-by: Michael Zaidman <michael.zaidman@gmail.com>
---
drivers/hid/hid-ft260.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/hid/hid-ft260.c b/drivers/hid/hid-ft260.c
index 63839f02e9b5..f8d4bf7e6c4f 100644
--- a/drivers/hid/hid-ft260.c
+++ b/drivers/hid/hid-ft260.c
@@ -1124,7 +1124,6 @@ static void ft260_uart_port_remove(struct ft260_device *port)
spin_unlock(&port->write_lock);
mutex_lock(&port->port.mutex);
- ft260_uart_wakeup_workaraund_enable(port, false);
tty_port_tty_hangup(&port->port, false);
mutex_unlock(&port->port.mutex);
--
2.40.1
^ permalink raw reply related
* [PATCH v1 12/19] hid-ft260: uart: use kfifo_avail for fifo write room check
From: Michael Zaidman @ 2024-02-10 21:51 UTC (permalink / raw)
To: chrysh, daniel.beer, jikos
Cc: linux-kernel, linux-input, linux-serial, ilpo.jarvinen, johan,
gregkh, equinox, michael.zaidman
In-Reply-To: <20240210215147.77629-1-michael.zaidman@gmail.com>
Replace uart fifo write room calculation with the kfifo_avail kernel API.
Signed-off-by: Michael Zaidman <michael.zaidman@gmail.com>
---
drivers/hid/hid-ft260.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/hid/hid-ft260.c b/drivers/hid/hid-ft260.c
index f8d4bf7e6c4f..3d1a9ec88cb9 100644
--- a/drivers/hid/hid-ft260.c
+++ b/drivers/hid/hid-ft260.c
@@ -1264,7 +1264,7 @@ static unsigned int ft260_uart_write_room(struct tty_struct *tty)
{
struct ft260_device *port = tty->driver_data;
- return FIFO_SIZE - kfifo_len(&port->xmit_fifo);
+ return kfifo_avail(&port->xmit_fifo);
}
static unsigned int ft260_uart_chars_in_buffer(struct tty_struct *tty)
--
2.40.1
^ permalink raw reply related
* [PATCH v1 13/19] hid-ft260: improve usb interface type detection logic
From: Michael Zaidman @ 2024-02-10 21:51 UTC (permalink / raw)
To: chrysh, daniel.beer, jikos
Cc: linux-kernel, linux-input, linux-serial, ilpo.jarvinen, johan,
gregkh, equinox, michael.zaidman
In-Reply-To: <20240210215147.77629-1-michael.zaidman@gmail.com>
This commit simplifies the ft260_get_interface_type routine by
replacing the ft260_is_serial with iface_type and making use of
its return value as it's in the mainline ft260 driver code.
Signed-off-by: Michael Zaidman <michael.zaidman@gmail.com>
---
drivers/hid/hid-ft260.c | 24 +++++++++---------------
1 file changed, 9 insertions(+), 15 deletions(-)
diff --git a/drivers/hid/hid-ft260.c b/drivers/hid/hid-ft260.c
index 3d1a9ec88cb9..9ecd91d173d2 100644
--- a/drivers/hid/hid-ft260.c
+++ b/drivers/hid/hid-ft260.c
@@ -309,7 +309,7 @@ struct ft260_device {
struct i2c_adapter adap;
struct hid_device *hdev;
- bool ft260_is_serial;
+ int iface_type;
struct list_head device_list;
/* tty_port lifetime is equal to device lifetime */
@@ -889,28 +889,25 @@ static int ft260_get_interface_type(struct hid_device *hdev, struct ft260_device
ft260_dbg("i2c_enable: 0x%02x\n", cfg.i2c_enable);
ft260_dbg("uart_mode: 0x%02x\n", cfg.uart_mode);
- dev->ft260_is_serial = false;
dev->power_saving_en = cfg.power_saving_en;
switch (cfg.chip_mode) {
case FT260_MODE_ALL:
case FT260_MODE_BOTH:
- if (interface == 1) {
+ if (interface == 1)
ret = FT260_IFACE_UART;
- dev->ft260_is_serial = true;
- } else {
+ else
ret = FT260_IFACE_I2C;
- }
break;
case FT260_MODE_UART:
ret = FT260_IFACE_UART;
- dev->ft260_is_serial = true;
break;
case FT260_MODE_I2C:
ret = FT260_IFACE_I2C;
break;
}
+ dev->iface_type = ret;
return ret;
}
@@ -1713,15 +1710,12 @@ static int ft260_probe(struct hid_device *hdev, const struct hid_device_id *id)
mutex_init(&dev->lock);
init_completion(&dev->wait);
- if (!dev->ft260_is_serial) {
+ if (ret == FT260_IFACE_I2C)
ret = ft260_i2c_probe(hdev, dev);
- if (ret)
- goto err_hid_close;
- } else {
+ else
ret = ft260_uart_probe(hdev, dev);
- if (ret)
- goto err_hid_close;
- }
+ if (ret)
+ goto err_hid_close;
return 0;
@@ -1742,7 +1736,7 @@ static void ft260_remove(struct hid_device *hdev)
if (!dev)
return;
- if (dev->ft260_is_serial) {
+ if (dev->iface_type == FT260_IFACE_UART) {
// FIXME:
cancel_work_sync(&dev->wakeup_work);
tty_port_unregister_device(&dev->port, ft260_tty_driver,
--
2.40.1
^ permalink raw reply related
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