Re: [PATCH 02/11] dt-bindings: i2c: Add NVIDIA LSTP I2C

[Krzysztof Kozlowski <krzk@kernel.org>]

On 12/10/2026 19:36, Adrian Ambrozewicz wrote:
> Add the binding for an I2C controller channel of an NVIDIA LSTP USB
> adapter. Peripherals on the bus are child nodes, as for any I2C
> controller. The device sets the bus speed and the protocol cannot
> change it, so clock-frequency is not supported.
> 
> nvidia,retries sets how many times the host re-issues a transfer that
> failed on arbitration loss or a busy bus (default 3). The generic I2C
> controller schema has no property for this. A device that reports its
> own arbitration retries handles both cases itself, and the property is
> ignored for it.
> 
> Signed-off-by: Adrian Ambrozewicz <aambrozewicz@nvidia.com>
> ---
>  .../bindings/i2c/nvidia,lstp-i2c.yaml         | 71 +++++++++++++++++++
>  MAINTAINERS                                   |  1 +
>  2 files changed, 72 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/i2c/nvidia,lstp-i2c.yaml
> 
> diff --git a/Documentation/devicetree/bindings/i2c/nvidia,lstp-i2c.yaml b/Documentation/devicetree/bindings/i2c/nvidia,lstp-i2c.yaml
> new file mode 100644
> index 000000000000..eacabed52cd1
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/i2c/nvidia,lstp-i2c.yaml
> @@ -0,0 +1,71 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.

We do not use that tag. Drop it everywhere.

> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/i2c/nvidia,lstp-i2c.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: NVIDIA LSTP I2C Channel
> +
> +maintainers:
> +  - Adrian Ambrozewicz <aambrozewicz@nvidia.com>
> +  - Donald Shannon <donalds@nvidia.com>
> +
> +description: |
> +  I2C controller channel on an NVIDIA LSTP USB device.

Say something useful.

> +
> +  Peripherals on the bus are described as child nodes.  An i2c alias
> +  gives the bus a fixed number.

What are you describing here?

> +
> +  The bus speed is set by the device.  The LSTP protocol has no way to
> +  change it, so clock-frequency is not supported and is ignored.
> +
> +allOf:
> +  - $ref: /schemas/i2c/i2c-controller.yaml#
> +
> +properties:
> +  compatible:
> +    const: nvidia,lstp-i2c
> +
> +  reg:
> +    description: Channel ID reported by the device firmware.
> +    maxItems: 1
> +
> +  nvidia,retries:

You described the desired Linux feature or behavior, not the actual
hardware. The bindings are about the latter, so instead you need to
rephrase the property and its description to match actual hardware
capabilities/features/configuration etc.

> +    description: |
> +      Number of times the host re-issues a transfer that fails on
> +      arbitration loss or a busy bus before giving up.
> +
> +      Ignored when the channel config reports non-zero bus arbitration
> +      retries.  Such a device is expected to retry both arbitration loss and
> +      a busy bus itself before reporting failure, so the host adds no
> +      attempts on top.
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    maximum: 255
> +    default: 3
> +
> +required:
> +  - compatible
> +  - reg

Incomplete, where are any resources? If you do not have any, then this
should be folded into the parent node.

> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    interface {
> +        #address-cells = <1>;
> +        #size-cells = <0>;

Why do you need that wrapping node and what is "interface"?

> +
Best regards,
Krzysztof