linux-input.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v4 0/6] Add spi-hid, transport for HID over SPI bus
@ 2022-02-25  0:59 Dmitry Antipov
  2022-02-25  0:59 ` [PATCH v4 1/6] HID: Add BUS_SPI support when printing out device info in hid_connect() Dmitry Antipov
                   ` (5 more replies)
  0 siblings, 6 replies; 10+ messages in thread
From: Dmitry Antipov @ 2022-02-25  0:59 UTC (permalink / raw)
  To: Jiri Kosina, Benjamin Tissoires, Dmitry Torokhov, Rob Herring,
	Mark Brown, Felipe Balbi
  Cc: linux-input, devicetree, linux-spi, Dmitry Antipov

Surface Duo devices use a touch digitizer that communicates to the main
SoC via SPI and presents itself as a HID device. This patch's goal is to
add the spi-hid transport driver to drivers/hid. The driver follows the
publically available HID Over SPI Protocol Specification version 1.0.

The specification is available at
https://www.microsoft.com/en-us/download/details.aspx?id=103325.

In the initial commits there are some HID core changes to support a SPI
device, a change to HID documentation, HID over SPI Device Tree
bindings, and finally the SPI HID transport driver.

Dmitry Antipov (6):
  HID: Add BUS_SPI support when printing out device info in
    hid_connect()
  HID: define HID_SPI_DEVICE macro in hid.h
  Documentation: DT bindings for Microsoft G6 Touch Digitizer
  Documentation: Correction in HID output_report callback description.
  HID: add spi-hid, transport driver for HID over SPI bus
  Defconfig: add CONFIG_SPI_HID=m

 .../input/microsoft,g6-touch-digitizer.yaml   |  105 ++
 Documentation/hid/hid-transport.rst           |    4 +-
 arch/arm64/configs/defconfig                  |    1 +
 drivers/hid/Kconfig                           |    2 +
 drivers/hid/Makefile                          |    1 +
 drivers/hid/hid-core.c                        |    3 +
 drivers/hid/spi-hid/Kconfig                   |   25 +
 drivers/hid/spi-hid/Makefile                  |   12 +
 drivers/hid/spi-hid/spi-hid-core.c            | 1326 +++++++++++++++++
 drivers/hid/spi-hid/spi-hid-core.h            |  188 +++
 drivers/hid/spi-hid/spi-hid-of.c              |  141 ++
 drivers/hid/spi-hid/spi-hid-of.h              |   30 +
 drivers/hid/spi-hid/spi-hid_trace.h           |  194 +++
 drivers/hid/spi-hid/trace.c                   |    9 +
 include/linux/hid.h                           |    2 +
 15 files changed, 2041 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/input/microsoft,g6-touch-digitizer.yaml
 create mode 100644 drivers/hid/spi-hid/Kconfig
 create mode 100644 drivers/hid/spi-hid/Makefile
 create mode 100644 drivers/hid/spi-hid/spi-hid-core.c
 create mode 100644 drivers/hid/spi-hid/spi-hid-core.h
 create mode 100644 drivers/hid/spi-hid/spi-hid-of.c
 create mode 100644 drivers/hid/spi-hid/spi-hid-of.h
 create mode 100644 drivers/hid/spi-hid/spi-hid_trace.h
 create mode 100644 drivers/hid/spi-hid/trace.c

-- 
2.25.1


^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: [PATCH v4 3/6] Documentation: DT bindings for Microsoft G6 Touch Digitizer
@ 2022-06-16 18:14 Dmitry Antipov
  0 siblings, 0 replies; 10+ messages in thread
From: Dmitry Antipov @ 2022-06-16 18:14 UTC (permalink / raw)
  To: Rob Herring
  Cc: Jiri Kosina, Benjamin Tissoires, Dmitry Torokhov, Mark Brown,
	Felipe Balbi, linux-input@vger.kernel.org,
	devicetree@vger.kernel.org, linux-spi@vger.kernel.org,
	Jarrett Schultz

On Fri, Feb 25, 2022 at 06:59AM, Rob Herring wrote:
> 
> On Thu, Feb 24, 2022 at 04:59:33PM -0800, Dmitry Antipov wrote:
> > From: Dmitry Antipov <dmanti@microsoft.com>
> 
> Please follow the conventions of the subsystem for the subject:
> 
> dt-bindings: input: ...

Will do, thank you.

> 
> >
> > Documentation describes the required and optional properties for 
> > implementing Device Tree for a Microsoft G6 Touch Digitizer that 
> > supports HID over SPI Protocol 1.0 specification.
> >
> > Signed-off-by: Dmitry Antipov <dmanti@microsoft.com>
> > ---
> >  .../input/microsoft,g6-touch-digitizer.yaml   | 105 ++++++++++++++++++
> >  1 file changed, 105 insertions(+)
> >  create mode 100644
> > Documentation/devicetree/bindings/input/microsoft,g6-touch-digitizer
> > .y
> > aml
> >
> > diff --git
> > a/Documentation/devicetree/bindings/input/microsoft,g6-touch-digitiz
> > er
> > .yaml
> > b/Documentation/devicetree/bindings/input/microsoft,g6-touch-digitiz
> > er
> > .yaml
> > new file mode 100644
> > index 000000000000..e516717527e9
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/input/microsoft,g6-touch-dig
> > +++ it
> > +++ izer.yaml
> > +
> > +properties:
> > +  compatible:
> > +    oneOf:
> > +      - const: microsoft,g6-touch-digitizer
> > +      - items:
> > +        - const: microsoft,g6-touch-digitizer
> > +        - const: hid-over-spi
> 
> Why are both cases needed?
> 
> Assuming you keep the 2nd case, you will need a custom 'select' to 
> avoid applying this schema to another binding using 'hid-over-spi':
> 
> select:
>   properties:
>     compatible:
>       contains:
> 	const: microsoft,g6-touch-digitizer
> 
>   required:
>     - compatible
> 

We decided to make the driver compatible with "microsoft,g6-touch-digitizer" as
well as "hid-over-spi", so the next revision of the patch will list
"microsoft,g6-touch-digitizer" as the only compatible value.

> 
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  interrupts:
> > +    maxItems: 1
> > +
> > +  reset-gpios:
> > +    maxItems: 1
> > +    description:
> > +      GPIO specifier for the digitizer's reset pin (active low). The line must
> > +      be flagged with GPIO_ACTIVE_LOW.
> > +
> > +  vdd-supply:
> > +    description:
> > +      Regulator for the VDD supply voltage.
> > +
> > +  input-report-header-address:
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +    description:
> > +      This property and the rest are described in HID Over SPI 
> > + Protocol Spec 1.0
> 
> Each property needs a description and a more specific spec location.
> 
> No constraints on the values? 0 - 2^32 is valid?
> 

Thank you. Will provide description for each property and will specify the bit
length of each address if outlined by the spec.

> > +
> > +  input-report-body-address:
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +
> > +  output-report-address:
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +
> > +  read-opcode:
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +
> > +  write-opcode:
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +
> > +  hid-over-spi-flags:
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +
> 
> > +  post-power-on-delay-ms:
> > +    description:
> > +      Optional time in ms required by the device after enabling its regulators
> > +      or powering it on, before it is ready for communication.
> > +
> > +  minimal-reset-delay-ms:
> > +    description:
> > +      Optional minimum amount of time in ms that device needs to be 
> > + in
> reset
> > +      state for the reset to take effect.
> 
> These should be implied by the compatible string.

If I'm understanding you correctly here, you are asking for the driver to have
the heuristics to apply specific delays based on which compatible string is
published in the device tree. However, the same touch digitizer but on different
boards may have different delay requirements based on the voltage regulator used
or line capacitance. Therefore, I would argue it is better to have this delay
information provided by the device tree.

> 
> > +
> > +required:
> > +  - compatible
> > +  - interrupts
> > +  - reset-gpios
> 
> It's not allowed to have reset under h/w control?

No, the spec requires a discrete reset line.

> 
> > +  - vdd-supply
> > +  - input-report-header-address
> > +  - input-report-body-address
> > +  - output-report-address
> > +  - read-opcode
> > +  - write-opcode
> > +  - hid-over-spi-flags
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    #include <dt-bindings/interrupt-controller/irq.h>
> > +    #include <dt-bindings/gpio/gpio.h>
> > +    spi-hid-dev0 {
> 
> hid@0
> 
> And you'll need to define a spi bus.

Thank you, will address in the next patch.

> 
> > +      compatible = "microsoft,g6-touch-digitizer", "hid-over-spi";
> > +      reg = <0>;
> > +      interrupts-extended = <&gpio 42 IRQ_TYPE_EDGE_FALLING>;
> > +      reset-gpios = <&gpio 27 GPIO_ACTIVE_LOW>;
> > +      vdd-supply = <&pm8350c_l3>;
> > +      pinctrl-names = "default";
> > +      pinctrl-0 = <&ts_d6_reset_assert &ts_d6_int_bias>;
> > +      input-report-header-address = <0x1000>;
> > +      input-report-body-address = <0x1004>;
> > +      output-report-address = <0x2000>;
> > +      read-opcode = <0x0b>;
> > +      write-opcode = <0x02>;
> > +      hid-over-spi-flags = <0x00>;
> > +      post-power-on-delay-ms = <5>;
> > +      minimal-reset-delay-ms = <5>;
> > +    };
> > --
> > 2.25.1
> >
> >

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2022-06-16 18:14 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-02-25  0:59 [PATCH v4 0/6] Add spi-hid, transport for HID over SPI bus Dmitry Antipov
2022-02-25  0:59 ` [PATCH v4 1/6] HID: Add BUS_SPI support when printing out device info in hid_connect() Dmitry Antipov
2022-02-25  0:59 ` [PATCH v4 2/6] HID: define HID_SPI_DEVICE macro in hid.h Dmitry Antipov
2022-02-25  0:59 ` [PATCH v4 3/6] Documentation: DT bindings for Microsoft G6 Touch Digitizer Dmitry Antipov
2022-02-25 14:16   ` Rob Herring
2022-02-25 14:58   ` Rob Herring
2022-02-25  0:59 ` [PATCH v4 4/6] Documentation: Correction in HID output_report callback description Dmitry Antipov
2022-02-25  0:59 ` [PATCH v4 5/6] HID: add spi-hid, transport driver for HID over SPI bus Dmitry Antipov
2022-02-25  0:59 ` [PATCH v4 6/6] Defconfig: add CONFIG_SPI_HID=m Dmitry Antipov
  -- strict thread matches above, loose matches on Subject: below --
2022-06-16 18:14 [PATCH v4 3/6] Documentation: DT bindings for Microsoft G6 Touch Digitizer Dmitry Antipov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).