All of lore.kernel.org
 help / color / mirror / Atom feed
From: Felipe Balbi <balbi@kernel.org>
To: Dmitry Antipov <dmanti@microsoft.com>
Cc: Benjamin Tissoires <benjamin.tissoires@redhat.com>,
	Dmitry Antipov <daantipov@gmail.com>,
	Jiri Kosina <jikos@kernel.org>,
	"open list:HID CORE LAYER" <linux-input@vger.kernel.org>,
	Mark Brown <broonie@kernel.org>,
	"linux-spi@vger.kernel.org" <linux-spi@vger.kernel.org>
Subject: Re: [EXTERNAL] Re: [PATCH v1 5/5] HID: add spi-hid, transport driver for HID over SPI bus
Date: Sat, 15 Jan 2022 13:16:30 +0200	[thread overview]
Message-ID: <87zgnx2qp6.fsf@kernel.org> (raw)
In-Reply-To: <MW2PR2101MB1065322A52B88BDBF0807058DA559@MW2PR2101MB1065.namprd21.prod.outlook.com>


Hi,

Dmitry Antipov <dmanti@microsoft.com> writes:
>> > +       /*
>> > +        * FIXME: below call returning 0 doesn't mean that the report descriptor
>> > +        * is good. We might be caching a crc32 of a corrupted r. d. or who
>> > +        * knows what the FW sent. Need to have a feedback loop about r. d.
>> > +        * being ok and only then cache it.
>> 
>> Shouldn't you check for the CRC before submitting to hid_parse_report then?
>
> I recently tested a touch digitizer firmware with a bungled report
> descriptor. Nothing in the HID stack returned an error, but the hidraw
> device was not installed. Short of traversing the /dev/ location I am
> not sure how to confirm that hid_add_device() did what we expect it to.
> 		
> We keep the device (/dev/hidraw# in our case) installed during suspend
> sessions (saves time and kernel memory on resumes from sleep), but if
> the report descriptor changes, its CRC will not match the cached one (we
> check in spi_hid_refresh_device_work) and we will reinstall the device,
> so we won't be surprised if the device starts sending unexpected
> reports.

note that the same behavior can potentially trigger after a FW upgrade
of the digitizer if either device or report descriptors change. That's
the only reason why the CRC exists today, so the driver can detect that
the other side "is another device".

-- 
balbi

  reply	other threads:[~2022-01-15 11:17 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-12-29 23:11 [PATCH v1 0/5] Add spi-hid, transport for HID over SPI bus Dmitry Antipov
2021-12-29 23:11 ` [PATCH v1 1/5] HID: Add BUS_SPI support when printing out device info in hid_connect() Dmitry Antipov
2022-01-03 15:18   ` Benjamin Tissoires
2021-12-29 23:11 ` [PATCH v1 2/5] HID: define HID_SPI_DEVICE macro in hid.h Dmitry Antipov
2022-01-03 15:18   ` Benjamin Tissoires
2021-12-29 23:11 ` [PATCH v1 3/5] HID: add on_transport_error() field to struct hid_driver Dmitry Antipov
2022-01-03 15:26   ` Benjamin Tissoires
2022-01-04  2:07     ` [EXTERNAL] " Dmitry Antipov
2022-01-04 15:51       ` Benjamin Tissoires
2022-01-08  1:10         ` Dmitry Antipov
2022-01-13 10:02           ` Benjamin Tissoires
2022-01-14  6:22             ` Felipe Balbi
2021-12-29 23:11 ` [PATCH v1 4/5] HID: add reset() field to struct hid_ll_driver Dmitry Antipov
2022-01-03 15:32   ` Benjamin Tissoires
2021-12-29 23:11 ` [PATCH v1 5/5] HID: add spi-hid, transport driver for HID over SPI bus Dmitry Antipov
2022-01-03 17:26   ` Benjamin Tissoires
2022-01-15  2:06     ` [EXTERNAL] " Dmitry Antipov
2022-01-15 11:16       ` Felipe Balbi [this message]
2022-01-03 15:17 ` [PATCH v1 0/5] Add spi-hid, transport " Benjamin Tissoires

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87zgnx2qp6.fsf@kernel.org \
    --to=balbi@kernel.org \
    --cc=benjamin.tissoires@redhat.com \
    --cc=broonie@kernel.org \
    --cc=daantipov@gmail.com \
    --cc=dmanti@microsoft.com \
    --cc=jikos@kernel.org \
    --cc=linux-input@vger.kernel.org \
    --cc=linux-spi@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.