mbox series

[0/3] 64-bit Digitizer Serial Numbers

Message ID 20210520002249.361821-1-kenalba@google.com (mailing list archive)
Headers show
Series 64-bit Digitizer Serial Numbers | expand

Message

Kenneth Albanowski May 20, 2021, 12:22 a.m. UTC
Still as RFC, here's a patch set to implement support for 64-bit
Digitizer Serial Numbers, following on to the prior discussion of how
we can support Usage(Digitizers.Transducer Serial Number) larger
than 32 bits.

My primary goal is making the serial number available via the
power_supply node interface, and the third patch implements that: if
CONFIG_HID_BATTERY_STRENGTH is enabled, and a hid report includes
both Usage(Digitizers.Battery Strength) and Usage(Digitizers.
Transducer Serial Number) then the generated power_supply node
includes a SERIAL_NUMBER of "DG-ABCDEF..." of the last seen digitizer.

For this patch set, I have not implemented any changes to MSC_SERIAL*,
however everything should be available for a future implementation. This
patch set does not change any events emitted by evdev.

After some conversation with Dmitry, I am leaning towards serial number
integration with the hid-multitouch protocol being the better long term
solution for evdev, as that fits better with combined touch and stylus
operation, as well as providing a clear means to providing multiple
serial numbers for simultaneous styluses.

This set includes general modifications to hid-core to decode and
represent HID fields larger than 32-bits as multiple consecutive 32-bit
words, allowing arbitrarily long fields (or at least as many as can fit
into HID_MAX_BUFFER_SIZE) to be passed upstream and (potentially)
processed by hid-input and other subsystems.

(After trying a 64-bit implementation, I decided staying with 32-bit
words kept the rest of the hid-core cleaner, and it was worth 
supporting arbitrary length fields instead of only upgrading
from 32 -> 64.)

Kenneth Albanowski (3):
  [hid] Minor cleanup
  [hid] Enable processing of fields larger than 32 bits.
  [hid] Emit digitizer serial number through power_supply

 Documentation/hid/hiddev.rst |   6 +-
 drivers/hid/hid-core.c       |  99 +++++++++++++++++++++-----------
 drivers/hid/hid-debug.c      |  27 ++++++---
 drivers/hid/hid-input.c      | 108 +++++++++++++++++++++++++++++++++--
 include/linux/hid-debug.h    |   4 +-
 include/linux/hid.h          |  10 +++-
 6 files changed, 199 insertions(+), 55 deletions(-)

Comments

Kenneth Albanowski June 9, 2021, 8:52 p.m. UTC | #1
To follow up to myself, we've decided that general >32-bit HID field
support isn't justified at this time (and not really supported by the
spec), so this set of patches can be simplified.

At this time, I'm going to switch to a plan of treating a 64-bit
Usage(Digitizer.Transducer Serial Number) as a singular oddity that
needs to be processed, instead of adding general processing for
arbitrary length HID fields throughout the hid-input system.

As to the code, I'm going to be following up with revised patches:

Patch #1 (minor cleanup) remains unchanged.

Patch #2 (large field support) goes away

Patch #3 (power_supply S/N support) is kept, and adds some minor
changes to hid-core.c, particularly in hid_add_field() that
recognize the 64-bit Usage(Digitizer.Transducer Serial Number) and
cuts it into two fields, a 32-bit Usage(Digitizer.Transducer Serial
Number) and a new 32-bit usage for the upper half.

Kind regards,
- Kenneth Albanowski

On Wed, May 19, 2021 at 5:23 PM Kenneth Albanowski <kenalba@google.com> wrote:
>
> Still as RFC, here's a patch set to implement support for 64-bit
> Digitizer Serial Numbers, following on to the prior discussion of how
> we can support Usage(Digitizers.Transducer Serial Number) larger
> than 32 bits.
>
> My primary goal is making the serial number available via the
> power_supply node interface, and the third patch implements that: if
> CONFIG_HID_BATTERY_STRENGTH is enabled, and a hid report includes
> both Usage(Digitizers.Battery Strength) and Usage(Digitizers.
> Transducer Serial Number) then the generated power_supply node
> includes a SERIAL_NUMBER of "DG-ABCDEF..." of the last seen digitizer.
>
> For this patch set, I have not implemented any changes to MSC_SERIAL*,
> however everything should be available for a future implementation. This
> patch set does not change any events emitted by evdev.
>
> After some conversation with Dmitry, I am leaning towards serial number
> integration with the hid-multitouch protocol being the better long term
> solution for evdev, as that fits better with combined touch and stylus
> operation, as well as providing a clear means to providing multiple
> serial numbers for simultaneous styluses.
>
> This set includes general modifications to hid-core to decode and
> represent HID fields larger than 32-bits as multiple consecutive 32-bit
> words, allowing arbitrarily long fields (or at least as many as can fit
> into HID_MAX_BUFFER_SIZE) to be passed upstream and (potentially)
> processed by hid-input and other subsystems.
>
> (After trying a 64-bit implementation, I decided staying with 32-bit
> words kept the rest of the hid-core cleaner, and it was worth
> supporting arbitrary length fields instead of only upgrading
> from 32 -> 64.)
>
> Kenneth Albanowski (3):
>   [hid] Minor cleanup
>   [hid] Enable processing of fields larger than 32 bits.
>   [hid] Emit digitizer serial number through power_supply
>
>  Documentation/hid/hiddev.rst |   6 +-
>  drivers/hid/hid-core.c       |  99 +++++++++++++++++++++-----------
>  drivers/hid/hid-debug.c      |  27 ++++++---
>  drivers/hid/hid-input.c      | 108 +++++++++++++++++++++++++++++++++--
>  include/linux/hid-debug.h    |   4 +-
>  include/linux/hid.h          |  10 +++-
>  6 files changed, 199 insertions(+), 55 deletions(-)
>
> --
> 2.31.0
>
Jiri Kosina Sept. 15, 2021, 2:56 p.m. UTC | #2
On Wed, 19 May 2021, Kenneth Albanowski wrote:

> Still as RFC, here's a patch set to implement support for 64-bit
> Digitizer Serial Numbers, following on to the prior discussion of how
> we can support Usage(Digitizers.Transducer Serial Number) larger
> than 32 bits.
> 
> My primary goal is making the serial number available via the
> power_supply node interface, and the third patch implements that: if
> CONFIG_HID_BATTERY_STRENGTH is enabled, and a hid report includes
> both Usage(Digitizers.Battery Strength) and Usage(Digitizers.
> Transducer Serial Number) then the generated power_supply node
> includes a SERIAL_NUMBER of "DG-ABCDEF..." of the last seen digitizer.
> 
> For this patch set, I have not implemented any changes to MSC_SERIAL*,
> however everything should be available for a future implementation. This
> patch set does not change any events emitted by evdev.
> 
> After some conversation with Dmitry, I am leaning towards serial number
> integration with the hid-multitouch protocol being the better long term
> solution for evdev, as that fits better with combined touch and stylus
> operation, as well as providing a clear means to providing multiple
> serial numbers for simultaneous styluses.
> 
> This set includes general modifications to hid-core to decode and
> represent HID fields larger than 32-bits as multiple consecutive 32-bit
> words, allowing arbitrarily long fields (or at least as many as can fit
> into HID_MAX_BUFFER_SIZE) to be passed upstream and (potentially)
> processed by hid-input and other subsystems.
> 
> (After trying a 64-bit implementation, I decided staying with 32-bit
> words kept the rest of the hid-core cleaner, and it was worth 
> supporting arbitrary length fields instead of only upgrading
> from 32 -> 64.)

Sorry for keeping this on hold for so long, it fell in between cracks.

This would need quite an extensive testing by Benjamin's testing 
infrastructure ... Benjamin, could you please put that into your queue?

Other than that, please rework the patchset so that it uses kernel comment 
styles, it can't be applied in this form.

Thanks,