Re: [RFC PATCH 1/2] dt-bindings: net: New binding for mctp-i2c

[Rob Herring <robh@kernel.org>]

On Sun, Aug 1, 2021 at 10:12 PM Matt Johnston <matt@codeconstruct.com.au> wrote:
>
> Used to define an endpoint to communicate with MCTP peripherals attached
> to an I2C bus.
>
> Signed-off-by: Matt Johnston <matt@codeconstruct.com.au>
> ---
>  .../devicetree/bindings/net/mctp-i2c.yaml     | 44 +++++++++++++++++++
>  1 file changed, 44 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/net/mctp-i2c.yaml
>
> diff --git a/Documentation/devicetree/bindings/net/mctp-i2c.yaml b/Documentation/devicetree/bindings/net/mctp-i2c.yaml
> new file mode 100644
> index 000000000000..f9378cd845d4
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/mctp-i2c.yaml
> @@ -0,0 +1,44 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/net/mctp-i2c.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: MCTP I2C transport binding
> +
> +maintainers:
> +  - Matt Johnston <matt@codeconstruct.com.au>
> +
> +description:
> +  The MCTP I2C binding defines an MCTP endpoint on the I2C bus to
> +  communicate with I2C peripherals using MCTP (DMTF specification DSP0237).

The general problem with bindings for a protocol/interface is that
those are not complete devices. What if there's something specific
about the implementation that has to be handled? For example, if the
device has to be powered on, brought out of reset or has some buggy
behavior to work around? Or perhaps there's additional functionality.
The exception is if the specification covers all of those things.

Having a specific binding doesn't preclude having a generic driver
either. With a specific compatible, either a generic or device
specific driver could be bound and the kernel could switch between
those whenever it wants.

> +
> +  An mctp-i2c device must be attached to a hardware bus adapter which supports
> +  slave functionality. The reg address must include I2C_OWN_SLAVE_ADDRESS.

This constraint can be a schema.

> +
> +
> +properties:
> +  compatible:
> +    const: mctp-i2c
> +
> +  reg:
> +    maxItems: 1
> +
> +additionalProperties: true

This is only allowed to be 'true' for common, incomplete bindings.
What other properties do you expect?

> +
> +required:
> +  - compatible
> +  - reg
> +
> +examples:
> +  - |
> +    #include <dt-bindings/i2c/i2c.h>
> +    i2c {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        mctp@50 {
> +            compatible = "mctp-i2c";
> +            reg = <(0x50 | I2C_OWN_SLAVE_ADDRESS)>;
> +        };
> +    };
> --
> 2.30.2
>