Re: [EXTERNAL] Re: [PATCH v1 5/5] HID: add spi-hid, transport driver for HID over SPI bus

[Felipe Balbi <balbi@xxxxxxxxxx>]

Hi,

Dmitry Antipov <dmanti@xxxxxxxxxxxxx> 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