Re: [RFC PATCH v2 0/2] MCTP I2C devicetree binding

[Rob Herring <robh@kernel.org>]

On Mon, Aug 23, 2021 at 2:52 AM Jeremy Kerr <jk@codeconstruct.com.au> wrote:
>
> Hi Rob,
>
> > > This ends up describing something like a network interface, which
> > > happens to use I2C as a transport in this case. (There are other
> > > transports like MCTP-over-serial, but those don't require DT
> > > topology
> > > data). For other network-type DT bindings (say, ethernet@), we
> > > don't
> > > describe remote network endpoints either, so we're proposing the
> > > same
> > > pattern for MCTP.
> >
> > When a switch becomes integrated in, we do.
>
> OK, we'll allow for cases like that, where we do need a representation
> of a "remote" endpoint. However, we don't *currently* have a scenario
> where that is necessary.

The issue here tends to be we design things based on not having nodes
in DT and then eventually evolve to the point where we should have had
a separate node. Connectors or per slot PCI properties are some
examples. Just something to keep in mind.

> > > > >     reg = <(0x50 | I2C_OWN_SLAVE_ADDRESS)>;
> > > > >     attach-bus = <&i2c1 &i2c6>;
> > > >
> > > > Why do you need to say you are attached to yourself?
> > >
> > > This indicates that the top-level MCTP controller needs to talk to
> > > MCTP
> > > endpoints, eg mctpA on the directly attached bus i2c1. In some
> > > topologies
> > > there will be no directly-attached endpoints, in which case we
> > > would omit
> > > i2c1 from the list. We need to specify the attach-bus property
> > > since we
> > > don't have a list of external device endpoints to walk.
> >
> > Okay, so it's a 'what I2C buses should be scanned for MCTP devices'.
>
> Not quite "scanned", more "marked as MCTP-capable". The indication that
> an i2c bus is a MCTP controller doesn't initiate any scanning, but
> rather provides a facility for software further up the stack to perform
> any scanning / monitoring for hotplug devices / setting up fixed remote
> endpoints - whatever is suitable for the system.
>
> > Why can't that just be all the buses under i2c1 in this example?
> > Limiting it seems like an optimization only.
>
> It's not so much an optimisation, rather a way to avoid overly complex
> network topologies. We may have on the order of 100 i2c busses
> (including both root busses and mux subordinates) on some platforms.
> Since physical addressing requires knowing both the SMBus address plus
> the MUX state, any software/user that deals with physical addreses
> will need to know about those ~100 busses.

Any system with muxes has them described in DT as I'm not aware of any
way muxes are discoverable.

> If I can use the Linux implementation as an example: flagging an i2c
> controller as MCTP-capable will create a MCTP netdev, which allows
> communicating with specific physaddrs on that segment of the bus. I'd
> like to avoid creating ~100 netdevs, all visible to userspace, when only
> a small subset of those can carry actual MCTP data.
>
> If we can limit the possible MCTP controllers to just the i2c busses
> that host MCTP hardware downstream, that makes things much easier for
> any OS implementation to deal with. While we can do the i2c/MCTP mapping
> at a higher level (ie. userspace), representing this in the DT does
> keep the local-hardware-specific data all in the one place.
>
> > In any case, 'attach-bus' sounds very generic and I'm not sure this
> > is. I'd like to hear from others familiar with I2C on this aspect at
> > least.
>
> We're certainly open to other structures for flagging busses as
> MCTP-capable; we can use a more representative name for this phandle
> list, or switch to boolean properties on the subordinate nodes
> themselves (something like the gpio-controller boolean props, perhaps?
> though that seems harder to confine to a schema for mctp-i2c...)

Either option is fine with me. A per bus property scales better (you
can add buses without changing the root MCTP node). We already have
per bus properties such as 'smbus' and 'multi-master'.

Rob