Re: [PATCH next v4 2/2] dt-bindings: i2c: add entry for hisilicon,hisi-i2c

Yicong Yang
2022-09-13T07:59:15Z

On 2022/9/9 15:48, Weilong Chen wrote:
> Add the new compatible for HiSilicon common i2c.
>
> Signed-off-by: Weilong Chen <chenweilong@huawei.com>
> ---
> .../bindings/i2c/hisilicon,hisi-i2c.yaml | 67 +++++++++++++++++++
> 1 file changed, 67 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/i2c/hisilicon,hisi-i2c.yaml
>
> diff --git a/Documentation/devicetree/bindings/i2c/hisilicon,hisi-i2c.yaml b/Documentation/devicetree/bindings/i2c/hisilicon,hisi-i2c.yaml
> new file mode 100644
> index 000000000000..f1cb6a4c70d1
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/i2c/hisilicon,hisi-i2c.yaml
> @@ -0,0 +1,67 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: "http://devicetree.org/schemas/i2c/hisilicon,hisi-i2c.yaml#"
> +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
> +
> +title: HiSilicon common IIC controller Device Tree Bindings
> +
> +maintainers:
> + - yangyicong@huawei.com
> +
> +allOf:
> + - $ref: /schemas/i2c/i2c-controller.yaml#
> +
> +properties:
> + compatible:
> + const: hisilicon,hisi-i2c
> +
> + reg:
> + maxItems: 1
> +
> + interrupts:
> + maxItems: 1
> +
> + clk_rate:
> + default: 0xEE6B280
> +
> + clock-frequency:
> + default: 400000
> +
> + i2c-sda-falling-time-ns:
> + default: 343
> +
> + i2c-scl-falling-time-ns:
> + default: 203
> +
> + i2c-sda-hold-time-ns:
> + default: 0x33E
> +
> + i2c-scl-rising-time-ns:
> + default: 365
> +
> + i2c-digital-filter-width-ns:
> + default: 0
> +
> +required:
> + - compatible
> + - reg
> + - interrupts
> +
> +unevaluatedProperties: false
> +
> +examples:
> + - |
> + i2c1: i2c@5038B0000{
> + compatible = "hisilicon,hisi-i2c";
> + reg = <0x38B0000 0x10000>;
> + interrupts = <0x0 120 0x4>;
> + i2c-sda-falling-time-ns = <56>;
> + i2c-scl-falling-time-ns = <56>;
> + i2c-sda-hold-time-ns = <56>;
> + i2c-scl-rising-time-ns = <56>;

The values used here are different from above. Are they used on different
products?

> + i2c-digital-filter;

Should we discard the empty properties or is it necessary to have it?

Others looks good to me, but the device tree experts may have some comments.

This binding file should also be listed in the MAINTAINERS file.

Thanks.

> + i2c-digital-filter-width-ns = <0x0>;
> + clk_rate = <0x0 0xEE6B280>;
> + clock-frequency = <400000>;
> + };
>