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

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.

I've rebased to net-next since that is the faster moving tree. If I3C 
maintainers would prefer I can submit the I3C bus enumeration patch 
by itself and let it take another cycle to get into net-next, though 
it wouldn't have any in-tree user. I'll leave that to maintainers.

Thanks,
Matt

---

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                   | 777 ++++++++++++++++++
 include/linux/i3c/master.h                    |  11 +
 6 files changed, 839 insertions(+)
 create mode 100644 drivers/net/mctp/mctp-i3c.c

On Mon, 17 Jul 2023 12:06:38 +0800 Matt Johnston wrote:
> +	dev_net_set(ndev, current->nsproxy->net_ns);

This is a bit odd, we may have missed similar code in earlier mctp
drivers. Are you actually making use of auto-assigning netns?

> +	mbus->tx_thread = kthread_create(mctp_i3c_tx_thread, mbus,
> +					 "%s/tx", ndev->name);
> +	if (IS_ERR(mbus->tx_thread)) {
> +		dev_warn(&ndev->dev, "Error creating thread: %pe\n",
> +			mbus->tx_thread);
> +		rc = PTR_ERR(mbus->tx_thread);
> +		mbus->tx_thread = NULL;
> +		goto err;
> +	}
> +	wake_up_process(mbus->tx_thread);

kthread_run()

On Mon, 17 Jul 2023 12:06:37 +0800 Matt Johnston wrote:
> From: Jeremy Kerr <jk@codeconstruct.com.au>
> 
> This allows other drivers to be notified when new i3c busses are
> attached, referring to a whole i3c bus as opposed to individual
> devices.
> 
> Signed-off-by: Jeremy Kerr <jk@codeconstruct.com.au>
> Signed-off-by: Matt Johnston <matt@codeconstruct.com.au>

We need one of:
 - sign-off from Alexandre that he's okay with this code going via
   netdev; or
 - stable branch from Alexandre based on an upstream -rc tag which 
   we can pull into net-next; or
 - wait until 6.6 merge window for the change to propagate.
Until then we can't do much on our end, so I'll mark the patches as
deferred from netdev perspective.

On Mon, 17 Jul 2023 12:06:38 +0800 Matt Johnston wrote:
> +/* Returns the 48 bit Provisioned Id from an i3c_device_info.pid */
> +static void pid_to_addr(u64 pid, u8 addr[PID_SIZE])
> +{
> +	pid = cpu_to_be64(pid);
> +	memcpy(addr, ((u8 *)&pid) + 2, PID_SIZE);
> +}

put_unaligned_be48() ? That or you need a local variable or something,
because otherwise sparse is not happy (build test with C=1).

On 17/07/2023 12:06:37+0800, Matt Johnston wrote:
> From: Jeremy Kerr <jk@codeconstruct.com.au>
> 
> This allows other drivers to be notified when new i3c busses are
> attached, referring to a whole i3c bus as opposed to individual
> devices.
> 
> Signed-off-by: Jeremy Kerr <jk@codeconstruct.com.au>
> Signed-off-by: Matt Johnston <matt@codeconstruct.com.au>

Acked-by: Alexandre Belloni <alexandre.belloni@bootlin.com>

> ---
>  drivers/i3c/master.c       | 35 +++++++++++++++++++++++++++++++++++
>  include/linux/i3c/master.h | 11 +++++++++++
>  2 files changed, 46 insertions(+)
> 
> diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
> index 08aeb69a7800..2276abe38bdc 100644
> --- a/drivers/i3c/master.c
> +++ b/drivers/i3c/master.c
> @@ -22,6 +22,7 @@
>  static DEFINE_IDR(i3c_bus_idr);
>  static DEFINE_MUTEX(i3c_core_lock);
>  static int __i3c_first_dynamic_bus_num;
> +static BLOCKING_NOTIFIER_HEAD(i3c_bus_notifier);
>  
>  /**
>   * i3c_bus_maintenance_lock - Lock the bus for a maintenance operation
> @@ -453,6 +454,36 @@ static int i3c_bus_init(struct i3c_bus *i3cbus, struct device_node *np)
>  	return 0;
>  }
>  
> +void i3c_for_each_bus_locked(int (*fn)(struct i3c_bus *bus, void *data),
> +			     void *data)
> +{
> +	struct i3c_bus *bus;
> +	int id;
> +
> +	mutex_lock(&i3c_core_lock);
> +	idr_for_each_entry(&i3c_bus_idr, bus, id)
> +		fn(bus, data);
> +	mutex_unlock(&i3c_core_lock);
> +}
> +EXPORT_SYMBOL_GPL(i3c_for_each_bus_locked);
> +
> +int i3c_register_notifier(struct notifier_block *nb)
> +{
> +	return blocking_notifier_chain_register(&i3c_bus_notifier, nb);
> +}
> +EXPORT_SYMBOL_GPL(i3c_register_notifier);
> +
> +int i3c_unregister_notifier(struct notifier_block *nb)
> +{
> +	return blocking_notifier_chain_unregister(&i3c_bus_notifier, nb);
> +}
> +EXPORT_SYMBOL_GPL(i3c_unregister_notifier);
> +
> +static void i3c_bus_notify(struct i3c_bus *bus, unsigned int action)
> +{
> +	blocking_notifier_call_chain(&i3c_bus_notifier, action, bus);
> +}
> +
>  static const char * const i3c_bus_mode_strings[] = {
>  	[I3C_BUS_MODE_PURE] = "pure",
>  	[I3C_BUS_MODE_MIXED_FAST] = "mixed-fast",
> @@ -2678,6 +2709,8 @@ int i3c_master_register(struct i3c_master_controller *master,
>  	if (ret)
>  		goto err_del_dev;
>  
> +	i3c_bus_notify(i3cbus, I3C_NOTIFY_BUS_ADD);
> +
>  	/*
>  	 * We're done initializing the bus and the controller, we can now
>  	 * register I3C devices discovered during the initial DAA.
> @@ -2710,6 +2743,8 @@ EXPORT_SYMBOL_GPL(i3c_master_register);
>   */
>  void i3c_master_unregister(struct i3c_master_controller *master)
>  {
> +	i3c_bus_notify(&master->bus, I3C_NOTIFY_BUS_REMOVE);
> +
>  	i3c_master_i2c_adapter_cleanup(master);
>  	i3c_master_unregister_i3c_devs(master);
>  	i3c_master_bus_cleanup(master);
> diff --git a/include/linux/i3c/master.h b/include/linux/i3c/master.h
> index 0b52da4f2346..db909ef79be4 100644
> --- a/include/linux/i3c/master.h
> +++ b/include/linux/i3c/master.h
> @@ -24,6 +24,12 @@
>  
>  struct i2c_client;
>  
> +/* notifier actions. notifier call data is the struct i3c_bus */
> +enum {
> +	I3C_NOTIFY_BUS_ADD,
> +	I3C_NOTIFY_BUS_REMOVE,
> +};
> +
>  struct i3c_master_controller;
>  struct i3c_bus;
>  struct i3c_device;
> @@ -652,4 +658,9 @@ void i3c_master_queue_ibi(struct i3c_dev_desc *dev, struct i3c_ibi_slot *slot);
>  
>  struct i3c_ibi_slot *i3c_master_get_free_ibi_slot(struct i3c_dev_desc *dev);
>  
> +void i3c_for_each_bus_locked(int (*fn)(struct i3c_bus *bus, void *data),
> +			     void *data);
> +int i3c_register_notifier(struct notifier_block *nb);
> +int i3c_unregister_notifier(struct notifier_block *nb);
> +
>  #endif /* I3C_MASTER_H */
> -- 
> 2.37.2
>

On 18/07/2023 18:57:57-0700, Jakub Kicinski wrote:
> On Mon, 17 Jul 2023 12:06:37 +0800 Matt Johnston wrote:
> > From: Jeremy Kerr <jk@codeconstruct.com.au>
> > 
> > This allows other drivers to be notified when new i3c busses are
> > attached, referring to a whole i3c bus as opposed to individual
> > devices.
> > 
> > Signed-off-by: Jeremy Kerr <jk@codeconstruct.com.au>
> > Signed-off-by: Matt Johnston <matt@codeconstruct.com.au>
> 
> We need one of:
>  - sign-off from Alexandre that he's okay with this code going via
>    netdev; or
>  - stable branch from Alexandre based on an upstream -rc tag which 
>    we can pull into net-next; or
>  - wait until 6.6 merge window for the change to propagate.
> Until then we can't do much on our end, so I'll mark the patches as
> deferred from netdev perspective.

I'm fine with the series going through netdev. Matt, please carry my ack
on the next versions.

> -- 
> pw-bot: defer

On Mon, Jul 17, 2023 at 12:06:38PM +0800, Matt Johnston wrote:

...

Hi Matt,

> +/* Returns the 48 bit Provisioned Id from an i3c_device_info.pid */
> +static void pid_to_addr(u64 pid, u8 addr[PID_SIZE])
> +{
> +	pid = cpu_to_be64(pid);

This assigns a be64 value to a u64 variable,
which is incorrect from a Sparse annotation perspective.

> +	memcpy(addr, ((u8 *)&pid) + 2, PID_SIZE);
> +}

...