mbox series

[net-next,v6,0/3] I3C MCTP net driver

Message ID 20231013040628.354323-1-matt@codeconstruct.com.au
Headers show
Series I3C MCTP net driver | expand

Message

Matt Johnston Oct. 13, 2023, 4:06 a.m. UTC
This series adds an I3C transport for the kernel's MCTP network
protocol. MCTP is a communication protocol between system components
(BMCs, drives, NICs etc), with higher level protocols such as NVMe-MI or
PLDM built on top of it (in userspace). It runs over various transports
such as I2C, PCIe, or I3C.

The mctp-i3c driver follows a similar approach to the kernel's existing
mctp-i2c driver, creating a "mctpi3cX" network interface for each
numbered I3C bus. Busses opt in to support by adding a "mctp-controller"
property to the devicetree:

&i3c0 {
        mctp-controller;
}

The driver will bind to MCTP class devices (DCR 0xCC) that are on a
supported I3C bus. Each bus is represented by a `struct mctp_i3c_bus`
that keeps state for the network device. An individual I3C device
(struct mctp_i3c_device) performs operations using the "parent"
mctp_i3c_bus object. The I3C notify/enumeration patch is needed so that
the mctp-i3c driver can handle creating/removing mctp_i3c_bus objects as
required.

The mctp-i3c driver is using the Provisioned ID as an identifier for
target I3C devices (the neighbour address), as that will be more stable
than the I3C dynamic address. The driver internally translates that to a
dynamic address for bus operations.

The driver has been tested using an AST2600 platform. A remote endpoint 
has been tested against QEMU, as well as using the target mode support 
in Aspeed's vendor tree.

I3C maintainers have acked merging this through net-next tree.

Thanks,
Matt

---

Thank you for the review Paolo, I've made those changes.

v6:

- Use multiple labels for error path handling
- Fix error handling from mctp_i3c_probe()
- Use spinlock_bh() instead for tx_lock
- Remove stale TODO comment (it had been tested)

v5:

- Use #define for constant initializer, fixes older gcc
- Wrap lines at 80 characters, fix parenthesis alignment 

v4:

- Add asm/unaligned.h include

v3:

- Use get_unaligned_be48()
- Use kthread_run()
- Don't set net namespace

v2:

- Rebased to net-next
- Removed unnecessary pr_ printing
- Fixed reverse christmas tree ordering
- Reworded DT property description to match I2C

Jeremy Kerr (1):
  i3c: Add support for bus enumeration & notification

Matt Johnston (2):
  dt-bindings: i3c: Add mctp-controller property
  mctp i3c: MCTP I3C driver

 .../devicetree/bindings/i3c/i3c.yaml          |   6 +
 drivers/i3c/master.c                          |  35 +
 drivers/net/mctp/Kconfig                      |   9 +
 drivers/net/mctp/Makefile                     |   1 +
 drivers/net/mctp/mctp-i3c.c                   | 755 ++++++++++++++++++
 include/linux/i3c/master.h                    |  11 +
 6 files changed, 817 insertions(+)
 create mode 100644 drivers/net/mctp/mctp-i3c.c

Comments

Paolo Abeni Oct. 17, 2023, 10:46 a.m. UTC | #1
On Tue, 2023-10-17 at 10:24 +0200, Simon Horman wrote:
> On Fri, Oct 13, 2023 at 12:06:25PM +0800, Matt Johnston wrote:
> > Provides MCTP network transport over an I3C bus, as specified in
> > DMTF DSP0233.
> > 
> > Each I3C bus (with "mctp-controller" devicetree property) gets an
> > "mctpi3cX" net device created. I3C devices are reachable as remote
> > endpoints through that net device. Link layer addressing uses the
> > I3C PID as a fixed hardware address for neighbour table entries.
> > 
> > The driver matches I3C devices that have the MIPI assigned DCR 0xCC for
> > MCTP.
> > 
> > Signed-off-by: Matt Johnston <matt@codeconstruct.com.au>
> 
> Hi Matt,
> 
> one minor nit below, which you can take, leave, or leave for later
> as far as I am concerned.
> 
> Overall the patch looks good to me and I see that Paolo's review of v5 has
> has been addressed.
> 
> Reviewed-by: Simon Horman <horms@kernel.org>
> 
> > +/* List of mctp_i3c_busdev */
> > +static LIST_HEAD(busdevs);
> > +/* Protects busdevs, as well as mctp_i3c_bus.devs lists */
> > +static DEFINE_MUTEX(busdevs_lock);
> > +
> > +struct mctp_i3c_bus {
> > +	struct net_device *ndev;
> > +
> > +	struct task_struct *tx_thread;
> > +	wait_queue_head_t tx_wq;
> > +	/* tx_lock protects tx_skb and devs */
> > +	spinlock_t tx_lock;
> > +	/* Next skb to transmit */
> > +	struct sk_buff *tx_skb;
> > +	/* Scratch buffer for xmit */
> > +	u8 tx_scratch[MCTP_I3C_MAXBUF];
> > +
> > +	/* Element of busdevs */
> > +	struct list_head list;
> 
> I am unsure if it is important, but I observe that on x86_64
> list spans a cacheline.

It looks like 'list' is only touched on control path, so it's should
not critical.

Cheers,

Paolo
patchwork-bot+netdevbpf@kernel.org Oct. 17, 2023, 11 a.m. UTC | #2
Hello:

This series was applied to netdev/net-next.git (main)
by Paolo Abeni <pabeni@redhat.com>:

On Fri, 13 Oct 2023 12:06:22 +0800 you wrote:
> This series adds an I3C transport for the kernel's MCTP network
> protocol. MCTP is a communication protocol between system components
> (BMCs, drives, NICs etc), with higher level protocols such as NVMe-MI or
> PLDM built on top of it (in userspace). It runs over various transports
> such as I2C, PCIe, or I3C.
> 
> The mctp-i3c driver follows a similar approach to the kernel's existing
> mctp-i2c driver, creating a "mctpi3cX" network interface for each
> numbered I3C bus. Busses opt in to support by adding a "mctp-controller"
> property to the devicetree:
> 
> [...]

Here is the summary with links:
  - [net-next,v6,1/3] dt-bindings: i3c: Add mctp-controller property
    https://git.kernel.org/netdev/net-next/c/ee71d6d5f18b
  - [net-next,v6,2/3] i3c: Add support for bus enumeration & notification
    https://git.kernel.org/netdev/net-next/c/0ac6486e5cbd
  - [net-next,v6,3/3] mctp i3c: MCTP I3C driver
    https://git.kernel.org/netdev/net-next/c/c8755b29b58e

You are awesome, thank you!