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

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

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
>

On Mon, 2021-08-02 at 10:45 -0600, Rob Herring wrote:
> On Sun, Aug 1, 2021 at 10:12 PM Matt Johnston <matt@codeconstruct.com.au>

> wrote:

> > 

> > +  slave functionality. The reg address must include

> > I2C_OWN_SLAVE_ADDRESS.

> 

> This constraint can be a schema.


The flag is already described in i2c.txt, is it OK to just make reference
to that?

Cheers,
Matt

On Tue, Aug 10, 2021 at 10:39 PM Matt Johnston
<matt@codeconstruct.com.au> wrote:
>

> On Mon, 2021-08-02 at 10:45 -0600, Rob Herring wrote:

> > On Sun, Aug 1, 2021 at 10:12 PM Matt Johnston <matt@codeconstruct.com.au>

> > wrote:

> > >

> > > +  slave functionality. The reg address must include

> > > I2C_OWN_SLAVE_ADDRESS.

> >

> > This constraint can be a schema.

>

> The flag is already described in i2c.txt, is it OK to just make reference

> to that?


I know it is, but you are saying the only addresses valid must have
that. Sounds like a constraint to me, so it should be a schema. Why
make a user have to debug that they forgot to set that bit?

Rob