mbox series

[v2,0/6] Input: edt-ft5x06 - Improve configuration

Message ID 20220217165559.313366-1-dario.binacchi@amarulasolutions.com
Headers show
Series Input: edt-ft5x06 - Improve configuration | expand

Message

Dario Binacchi Feb. 17, 2022, 4:55 p.m. UTC
The series was born from the analysis and mitigation of a crc problem
raised by an M06 type device. The added sysfs attributes were helpful
in debugging the problem. Patches that change the report rate on driver
probing, mitigated crc errors on kernel bootup. The patch to get/set
report rate by sysfs for an M12 device, has been tested.

Changes in v2:
- Add Oliver Graute's 'Acked-by' tag to:
  * Input: edt-ft5x06 - show model name by sysfs
  * Input: edt-ft5x06 - show firmware version by sysfs
- Fix yaml file. Tested with `make DT_CHECKER_FLAGS=-m dt_binding_check'.

Dario Binacchi (6):
  dt-bindings: input: touchscreen: edt-ft5x06: add report-rate
  Input: edt-ft5x06 - get/set M12 report rate by sysfs
  Input: edt-ft5x06 - set report rate by dts property
  Input: edt-ft5x06 - show model name by sysfs
  Input: edt-ft5x06 - show firmware version by sysfs
  Input: edt-ft5x06 - show crc and header errors by sysfs

 .../input/touchscreen/edt-ft5x06.yaml         |  8 ++
 drivers/input/touchscreen/edt-ft5x06.c        | 81 +++++++++++++++++--
 2 files changed, 81 insertions(+), 8 deletions(-)

Comments

Rob Herring (Arm) Feb. 24, 2022, 10:27 p.m. UTC | #1
On Thu, Feb 17, 2022 at 05:55:53PM +0100, Dario Binacchi wrote:
> It allows to change the M06/M12 default scan rate.
> 
> Co-developed-by: Michael Trimarchi <michael@amarulasolutions.com>
> Signed-off-by: Michael Trimarchi <michael@amarulasolutions.com>
> Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com>
> ---
> 
> (no changes since v1)
> 
>  .../devicetree/bindings/input/touchscreen/edt-ft5x06.yaml | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.yaml b/Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.yaml
> index 2e8da7470513..aa8517c6f65b 100644
> --- a/Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.yaml
> +++ b/Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.yaml
> @@ -85,6 +85,14 @@ properties:
>      minimum: 0
>      maximum: 80
>  
> +  report-rate:

Use property unit suffix: report-rate-hz

This should probably be moved to touchscreen.yaml as it seems common.

> +    description: Allows setting the scan rate.
> +                 M06 supports range from 3 (30 Hz) to 14 (140 Hz).

You're using 3 or 30 in this case? Should be 30, but it's not clear. I'd 
just list the range in Hz and leave the conversion detail to the driver.

> +                 M12 supports range from 1 (1 Hz) to 255 (255 Hz).

Use '|' if formatting (newline) is significant.

> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    minimum: 1
> +    maximum: 255
> +
>    touchscreen-size-x: true
>    touchscreen-size-y: true
>    touchscreen-fuzz-x: true
> -- 
> 2.32.0
> 
>
Dario Binacchi March 13, 2022, 4:32 p.m. UTC | #2
Hi Rob,

On Thu, Feb 24, 2022 at 11:27 PM Rob Herring <robh@kernel.org> wrote:
>
> On Thu, Feb 17, 2022 at 05:55:53PM +0100, Dario Binacchi wrote:
> > It allows to change the M06/M12 default scan rate.
> >
> > Co-developed-by: Michael Trimarchi <michael@amarulasolutions.com>
> > Signed-off-by: Michael Trimarchi <michael@amarulasolutions.com>
> > Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com>
> > ---
> >
> > (no changes since v1)
> >
> >  .../devicetree/bindings/input/touchscreen/edt-ft5x06.yaml | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.yaml b/Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.yaml
> > index 2e8da7470513..aa8517c6f65b 100644
> > --- a/Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.yaml
> > +++ b/Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.yaml
> > @@ -85,6 +85,14 @@ properties:
> >      minimum: 0
> >      maximum: 80
> >
> > +  report-rate:
>
> Use property unit suffix: report-rate-hz
>
> This should probably be moved to touchscreen.yaml as it seems common.

I did some checks but IMHO I think it's better to consider it as a
property of the
driver.

Thanks and regards
Dario

>
> > +    description: Allows setting the scan rate.
> > +                 M06 supports range from 3 (30 Hz) to 14 (140 Hz).
>
> You're using 3 or 30 in this case? Should be 30, but it's not clear. I'd
> just list the range in Hz and leave the conversion detail to the driver.
>
> > +                 M12 supports range from 1 (1 Hz) to 255 (255 Hz).
>
> Use '|' if formatting (newline) is significant.
>
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +    minimum: 1
> > +    maximum: 255
> > +
> >    touchscreen-size-x: true
> >    touchscreen-size-y: true
> >    touchscreen-fuzz-x: true
> > --
> > 2.32.0
> >
> >