Re: [PATCH 1/4] dt-bindings: input: Add binding for Qualcomm SPMI PMIC haptics

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Low] The device tree binding description contains Linux-specific driver implementation details, violating the OS-agnostic principle.
- [Low] Stale reference to an undefined property 'qcom,wf-play-rate' in the binding description.
--

commit 24bf9f2257f4052d27a53163621c8f5173fe1da8
Author: Fenglin Wu <fenglin.wu@oss.qualcomm.com>

dt-bindings: input: Add binding for Qualcomm SPMI PMIC haptics

Add binding document for the haptics module inside Qualcomm PMIH010X.

> diff --git a/Documentation/devicetree/bindings/input/qcom,spmi-haptics.yaml b/Documentation/devicetree/bindings/input/qcom,spmi-haptics.yaml
> new file mode 100644
> index 0000000000000..0e26d68563dcd
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/qcom,spmi-haptics.yaml

[ ... ]

> +      In FIFO streaming mode, hardware-based LRA auto-resonance tracking is
> +      disabled by default.  Because this mode is intended to drive arbitrary
> +      waveforms that may not follow the resonant frequency, autonomous hardware
> +      resonance correction would interfere with the intended output.
> +
> +      In the driver, FIFO streaming is implemented using an FF_PERIODIC effect
> +      with an FF_CUSTOM waveform.  The expected custom data layout is:

[Severity: Low]
Are these Linux-specific driver implementation details, such as FF_PERIODIC
and FF_CUSTOM, appropriate for this binding document?

Device tree bindings are typically expected to be strictly OS-agnostic
hardware descriptions. Should the documentation for the driver's ioctl payload
and custom_data memory layout be moved to the Linux driver documentation
instead?

> +        custom_data[0]   = play rate code (see qcom,wf-play-rate values below)

[Severity: Low]
Is there a missing property definition for qcom,wf-play-rate?

This line references qcom,wf-play-rate values, but the property doesn't appear
to be defined anywhere in the schema's properties section. Does this need to
be added, or is this a stale reference that should be removed?

> +        custom_data[1]   = vmax in mV; 0 = use device default (qcom,vmax-mv)
> +        custom_data[2..] = signed 8-bit PCM samples (at least one required)
> +
> +properties:

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260616-qcom-spmi-haptics-v1-0-d24e422de6b4@oss.qualcomm.com?part=1