diff mbox series

[2/2] dt-bindings: input: cirrus,cs40l26: Add bindings

Message ID 1680819613-29256-1-git-send-email-fred.treven@cirrus.com
State New
Headers show
Series [1/2] Input: cs40l26: Support for CS40L26 Boosted Haptic Amplifier | expand

Commit Message

Fred Treven April 6, 2023, 10:20 p.m. UTC
Add devicetree bindings for CS40L26 driver.

Signed-off-by: Fred Treven <fred.treven@cirrus.com>
---
 .../devicetree/bindings/input/cs40l26.yaml         | 92 ++++++++++++++++++++++
 MAINTAINERS                                        |  1 +
 2 files changed, 93 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/input/cs40l26.yaml

Comments

Krzysztof Kozlowski April 7, 2023, 9:16 a.m. UTC | #1
On 07/04/2023 00:20, Fred Treven wrote:
> Add devicetree bindings for CS40L26 driver.

I appreciate the try to write my name manually, but there is no need to
struggle. :) You will just make a mistake.

Just copy-paste or use scripts/get_maintainers.pl.
You can automate everything with something like:
https://github.com/krzk/tools/blob/master/linux/.bash_aliases_linux#L91

> 
> Signed-off-by: Fred Treven <fred.treven@cirrus.com>
> ---
>  .../devicetree/bindings/input/cs40l26.yaml         | 92 ++++++++++++++++++++++
>  MAINTAINERS                                        |  1 +
>  2 files changed, 93 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/input/cs40l26.yaml

Filename matching compatible, so you need vendor-prefix.

> 
> diff --git a/Documentation/devicetree/bindings/input/cs40l26.yaml b/Documentation/devicetree/bindings/input/cs40l26.yaml
> new file mode 100644
> index 000000000000..1036a374baa0
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/cs40l26.yaml
> @@ -0,0 +1,92 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/input/cs40l26.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Cirrus Logic CS40L26 Boosted Haptic Amplifier
> +
> +maintainers:
> +  - Fred Treven <fred.treven@cirrus.com>
> +
> +description:
> +  CS40L26 is a Boosted Haptic Driver with Integrated DSP and Waveform Memory
> +  with Advanced Closed Loop Algorithms and LRA protection
> +
> +properties:
> +  compatible:
> +    enum:
> +      - cirrus,cs40l26a
> +      - cirrus,cs40l26b
> +      - cirrus,cs40l27a
> +      - cirrus,cs40l27b
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +    description:
> +      Property describing the interrupt line the devices /ALERT pin is connected to.
> +      The device only has one interrupt source.

Drop description - it is almost useless. You could just mention ALERT pin.

> +
> +  VA-supply:
> +    description: Regulator for VA analog voltage
> +
> +  VP-supply:
> +    description: Regulator for VP peak voltage
> +
> +  cirrus,bst-ipk-microamp:
> +    description:
> +      Maximum amount of current that can be drawn by the device's boost
> +      converter in uA. Accepted values are between 1600000 uA and 4800000 uA in
> +      50000 uA increments.
> +    minimum: 1600000
> +    maximum: 4800000
> +    default: 4500000

Isn't this property of regulator? Why do you need it here?

> +
> +  cirrus,bst-ctl-microvolt:
> +    description:
> +      Maximum target voltage to which the class H algorithm may increase the
> +      VBST supply, expressed in uV. Valid values range from 2550000 to 11000000
> +      (inclusive) in steps of 50000. If this value is specified as zero or VP
> +      rises above this value, VBST is bypassed to VP. If this value is omitted,
> +      the maximum target voltage remains at 11 V.

Don't repeat constraints in free form text - drop last sentence.

> +    minimum: 2550000
> +    maximum: 11000000
> +    default: 11000000
> +
> +  cirrus,bst-exploratory-mode-disable:
> +    description:
> +      Disable boost exploratory mode if this boolean is present in the
> +      devicetree. 

Don't explain how DT works. Explain how hardware works instead.

> Boost exploratory mode allows the device to overshoot
> +      the set peak current limit. This has potential to damage the boost
> +      inductor. Disabling this mode will prevent this from happening; it will
> +      also prevent the device from detecting boost inductor short errors.
> +      (Default) Enabled

Why this is suitable for DT? Why would anyone need to disable it per board?

> +    type: boolean
> +
> +

Just one blank line.

> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    i2c {
> +      cs40l26: cs40l26@58 {

Node names should be generic.
https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation

> +        compatible = "cirrus,cs40l26a";
> +        reg = <0x58>;
> +        interrupt-parent = <&gpio0>;
> +        interrupts = <57 8>;

If 8 is interrupt flag, use appropriate define.

> +        reset-gpios = <&gpio0 54 0>;

Same for GPIO flag.

> +        VA-supply = <&dummy_vreg>;
> +        VP-supply = <&dummy_vreg>;
> +        cirrus,bst-ctl-microvolt = <2600000>; // Max boost voltage = 2.6V
> +        cirrus,bst-ipk-microamp = <1650000>; // Max boost current = 1.65A
> +        cirrus,bst-exploratory-mode-disabled; // Disable exploratory mode

The comments are not useful - they copy the property. Instead you could
explain WHY. Or just drop the comments.

>  

Best regards,
Krzysztof
Rob Herring (Arm) April 7, 2023, 2:21 p.m. UTC | #2
On Thu, 06 Apr 2023 17:20:13 -0500, Fred Treven wrote:
> Add devicetree bindings for CS40L26 driver.
> 
> Signed-off-by: Fred Treven <fred.treven@cirrus.com>
> ---
>  .../devicetree/bindings/input/cs40l26.yaml         | 92 ++++++++++++++++++++++
>  MAINTAINERS                                        |  1 +
>  2 files changed, 93 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/input/cs40l26.yaml
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
Documentation/devicetree/bindings/input/cs40l26.example.dts:24.13-26: Warning (reg_format): /example-0/i2c/cs40l26@58:reg: property has invalid length (4 bytes) (#address-cells == 2, #size-cells == 1)
Documentation/devicetree/bindings/input/cs40l26.example.dtb: Warning (pci_device_reg): Failed prerequisite 'reg_format'
Documentation/devicetree/bindings/input/cs40l26.example.dtb: Warning (pci_device_bus_num): Failed prerequisite 'reg_format'
Documentation/devicetree/bindings/input/cs40l26.example.dtb: Warning (simple_bus_reg): Failed prerequisite 'reg_format'
Documentation/devicetree/bindings/input/cs40l26.example.dts:21.13-34.11: Warning (i2c_bus_bridge): /example-0/i2c: incorrect #address-cells for I2C bus
Documentation/devicetree/bindings/input/cs40l26.example.dts:21.13-34.11: Warning (i2c_bus_bridge): /example-0/i2c: incorrect #size-cells for I2C bus
Documentation/devicetree/bindings/input/cs40l26.example.dtb: Warning (i2c_bus_reg): Failed prerequisite 'reg_format'
Documentation/devicetree/bindings/input/cs40l26.example.dtb: Warning (i2c_bus_reg): Failed prerequisite 'i2c_bus_bridge'
Documentation/devicetree/bindings/input/cs40l26.example.dtb: Warning (spi_bus_reg): Failed prerequisite 'reg_format'
Documentation/devicetree/bindings/input/cs40l26.example.dts:22.31-33.13: Warning (avoid_default_addr_size): /example-0/i2c/cs40l26@58: Relying on default #address-cells value
Documentation/devicetree/bindings/input/cs40l26.example.dts:22.31-33.13: Warning (avoid_default_addr_size): /example-0/i2c/cs40l26@58: Relying on default #size-cells value
Documentation/devicetree/bindings/input/cs40l26.example.dtb: Warning (unique_unit_address_if_enabled): Failed prerequisite 'avoid_default_addr_size'
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/input/cs40l26.example.dtb: cs40l26@58: 'cirrus,bst-exploratory-mode-disabled', 'reset-gpios' do not match any of the regexes: 'pinctrl-[0-9]+'
	From schema: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/input/cs40l26.yaml

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/1680819613-29256-1-git-send-email-fred.treven@cirrus.com

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/input/cs40l26.yaml b/Documentation/devicetree/bindings/input/cs40l26.yaml
new file mode 100644
index 000000000000..1036a374baa0
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/cs40l26.yaml
@@ -0,0 +1,92 @@ 
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/input/cs40l26.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Cirrus Logic CS40L26 Boosted Haptic Amplifier
+
+maintainers:
+  - Fred Treven <fred.treven@cirrus.com>
+
+description:
+  CS40L26 is a Boosted Haptic Driver with Integrated DSP and Waveform Memory
+  with Advanced Closed Loop Algorithms and LRA protection
+
+properties:
+  compatible:
+    enum:
+      - cirrus,cs40l26a
+      - cirrus,cs40l26b
+      - cirrus,cs40l27a
+      - cirrus,cs40l27b
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+    description:
+      Property describing the interrupt line the devices /ALERT pin is connected to.
+      The device only has one interrupt source.
+
+  VA-supply:
+    description: Regulator for VA analog voltage
+
+  VP-supply:
+    description: Regulator for VP peak voltage
+
+  cirrus,bst-ipk-microamp:
+    description:
+      Maximum amount of current that can be drawn by the device's boost
+      converter in uA. Accepted values are between 1600000 uA and 4800000 uA in
+      50000 uA increments.
+    minimum: 1600000
+    maximum: 4800000
+    default: 4500000
+
+  cirrus,bst-ctl-microvolt:
+    description:
+      Maximum target voltage to which the class H algorithm may increase the
+      VBST supply, expressed in uV. Valid values range from 2550000 to 11000000
+      (inclusive) in steps of 50000. If this value is specified as zero or VP
+      rises above this value, VBST is bypassed to VP. If this value is omitted,
+      the maximum target voltage remains at 11 V.
+    minimum: 2550000
+    maximum: 11000000
+    default: 11000000
+
+  cirrus,bst-exploratory-mode-disable:
+    description:
+      Disable boost exploratory mode if this boolean is present in the
+      devicetree. Boost exploratory mode allows the device to overshoot
+      the set peak current limit. This has potential to damage the boost
+      inductor. Disabling this mode will prevent this from happening; it will
+      also prevent the device from detecting boost inductor short errors.
+      (Default) Enabled
+    type: boolean
+
+
+required:
+  - compatible
+  - reg
+  - interrupts
+
+additionalProperties: false
+
+examples:
+  - |
+    i2c {
+      cs40l26: cs40l26@58 {
+        compatible = "cirrus,cs40l26a";
+        reg = <0x58>;
+        interrupt-parent = <&gpio0>;
+        interrupts = <57 8>;
+        reset-gpios = <&gpio0 54 0>;
+        VA-supply = <&dummy_vreg>;
+        VP-supply = <&dummy_vreg>;
+        cirrus,bst-ctl-microvolt = <2600000>; // Max boost voltage = 2.6V
+        cirrus,bst-ipk-microamp = <1650000>; // Max boost current = 1.65A
+        cirrus,bst-exploratory-mode-disabled; // Disable exploratory mode
+      };
+    };
diff --git a/MAINTAINERS b/MAINTAINERS
index 7051386d5a13..3cd9f0c8e38d 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4934,6 +4934,7 @@  L:	patches@opensource.cirrus.com
 S:	Supported
 W:	https://github.com/CirrusLogic/linux-drivers/wiki
 T:	git https://github.com/CirrusLogic/linux-drivers.git
+F:	Documentation/devicetree/bindings/input/cs40l26.yaml
 F:	drivers/input/misc/cs40l*
 F:	include/linux/input/cs40l*