Re: [PATCH v4 3/6] i3c: Add support for mastership request to I3C subsystem

vitor
2019-04-01T18:41:54Z

Hi Przemek,

On 10/03/19 13:58, Przemyslaw Gaj wrote:
> This patch adds support for mastership request to I3C subsystem.
>
> Mastership event is enabled globally.
>
> Mastership is requested automatically when device driver
> tries to transfer data using master controller in slave mode.
>
> There is still some limitation:
> - I2C devices are registered on secondary master side if boardinfo
> entry matching the info transmitted through the DEFSLVS frame.
>
> Signed-off-by: Przemyslaw Gaj <pgaj@cadence.com>
>
> ---
>
> Main changes between v3 and v4:
> - Add i3c_master_acquire_bus_ownership to acquire the bus
> - Refactored the code
>
> Main changes between v2 and v3:
> - Add i3c_bus_downgrade_maintenance_lock() for downgrading the bus
> lock from maintenance to normal use
> - Add additional fields to i2c_dev_desc for DEFSLVS purpose (addr, lvr)
> - Add i3c_master_register_new_i2c_devs() function to register I2C devices
> - Reworked I2C devices registration on secondary master side
>
> Changes in v2:
> - Add mastership disable event hook
> - Changed name of mastership enable event hook
> - Add function to test if master owns the bus
> - Add function to test if master owns the bus
> - Removed op_mode
> - Changed parameter of i3c_master_get_accmst_locked, no need to
> pass full i3c_device_info
> - Changed name of mastership enable event hook
> - Add function to test if master owns the bus
> - Removed op_mode
> - Changed parameter of i3c_master_get_accmst_locked, no need to
> pass full i3c_device_info
> - Removed redundant DEFSLVS command before GETACCMST
> - Add i3c_master_bus_takeover function. There is a need to lock
> the bus before adding devices and no matter of the controller
> devices have to be added after mastership takeover.
> - Add device registration during initialization on secondary master
> side. Devices received by DEFSLVS (if occured). If not, device
> initialization is deffered untill next mastership request.
> ---
> drivers/i3c/device.c | 26 +++
> drivers/i3c/internals.h | 4 +
> drivers/i3c/master.c | 417 +++++++++++++++++++++++++++++++++++++--------
> include/linux/i3c/master.h | 17 ++
> 4 files changed, 391 insertions(+), 73 deletions(-)
>
> diff --git a/drivers/i3c/device.c b/drivers/i3c/device.c
> index 69cc040..b60f637 100644
> --- a/drivers/i3c/device.c
> +++ b/drivers/i3c/device.c
> @@ -43,7 +43,13 @@ int i3c_device_do_priv_xfers(struct i3c_device *dev,
> }
>
> i3c_bus_normaluse_lock(dev->bus);
> + ret = i3c_master_acquire_bus_ownership(dev->desc->common.master);
> + if (ret)
> + goto err_unlock_bus;
> +
> ret = i3c_dev_do_priv_xfers_locked(dev->desc, xfers, nxfers);
> +
> +err_unlock_bus:
> i3c_bus_normaluse_unlock(dev->bus);
>
> return ret;
> @@ -114,11 +120,17 @@ int i3c_device_enable_ibi(struct i3c_device *dev)
> int ret = -ENOENT;
>
> i3c_bus_normaluse_lock(dev->bus);
> + ret = i3c_master_acquire_bus_ownership(dev->desc->common.master);
> + if (ret)
> + goto err_unlock_bus;
> +
> if (dev->desc) {
> mutex_lock(&dev->desc->ibi_lock);
> ret = i3c_dev_enable_ibi_locked(dev->desc);
> mutex_unlock(&dev->desc->ibi_lock);
> }
> +
> +err_unlock_bus:
> i3c_bus_normaluse_unlock(dev->bus);
>
> return ret;
> @@ -145,11 +157,17 @@ int i3c_device_request_ibi(struct i3c_device *dev,
> return -EINVAL;
>
> i3c_bus_normaluse_lock(dev->bus);
> + ret = i3c_master_acquire_bus_ownership(dev->desc->common.master);
> + if (ret)
> + goto err_unlock_bus;
> +
> if (dev->desc) {
> mutex_lock(&dev->desc->ibi_lock);
> ret = i3c_dev_request_ibi_locked(dev->desc, req);
> mutex_unlock(&dev->desc->ibi_lock);
> }
> +
> +err_unlock_bus:
> i3c_bus_normaluse_unlock(dev->bus);
>
> return ret;
> @@ -166,12 +184,20 @@ EXPORT_SYMBOL_GPL(i3c_device_request_ibi);
> */
> void i3c_device_free_ibi(struct i3c_device *dev)
> {
> + int ret;
> +
> i3c_bus_normaluse_lock(dev->bus);
> + ret = i3c_master_acquire_bus_ownership(dev->desc->common.master);
> + if (ret)
> + goto err_unlock_bus;
> +
> if (dev->desc) {
> mutex_lock(&dev->desc->ibi_lock);
> i3c_dev_free_ibi_locked(dev->desc);
> mutex_unlock(&dev->desc->ibi_lock);
> }
> +
> +err_unlock_bus:
> i3c_bus_normaluse_unlock(dev->bus);
> }
> EXPORT_SYMBOL_GPL(i3c_device_free_ibi);

Can't we verify if it is the current master no master.c side?

> diff --git a/drivers/i3c/internals.h b/drivers/i3c/internals.h
> index 86b7b44..929ca6b 100644
> --- a/drivers/i3c/internals.h
> +++ b/drivers/i3c/internals.h
> @@ -14,6 +14,10 @@ extern struct bus_type i3c_bus_type;
>
> void i3c_bus_normaluse_lock(struct i3c_bus *bus);
> void i3c_bus_normaluse_unlock(struct i3c_bus *bus);
> +void i3c_bus_downgrade_maintenance_lock(struct i3c_bus *bus);
> +int i3c_master_acquire_bus_ownership(struct i3c_master_controller *master);
> +int i3c_master_request_mastership_locked(struct i3c_master_controller *master);
> +
>
> int i3c_dev_do_priv_xfers_locked(struct i3c_dev_desc *dev,
> struct i3c_priv_xfer *xfers,
> diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
> index aea4309..7a84158 100644
> --- a/drivers/i3c/master.c
> +++ b/drivers/i3c/master.c
> @@ -93,6 +93,20 @@ void i3c_bus_normaluse_unlock(struct i3c_bus *bus)
> up_read(&bus->lock);
> }
>
> +/**
> + * i3c_bus_downgrade_maintenance_lock - Downgrade the bus lock to normal
> + * operation
> + * @bus: I3C bus to downgrade the lock on
> + *
> + * Should be called when a maintenance operation is done and normal
> + * operation is planned. See i3c_bus_maintenance_lock() and
> + * i3c_bus_normaluse_lock() for more details.
> + */
> +void i3c_bus_downgrade_maintenance_lock(struct i3c_bus *bus)
> +{
> + downgrade_write(&bus->lock);
> +}
> +
> static struct i3c_master_controller *dev_to_i3cmaster(struct device *dev)
> {
> return container_of(dev, struct i3c_master_controller, dev);
> @@ -341,6 +355,22 @@ static int i3c_device_probe(struct device *dev)
> return driver->probe(i3cdev);
> }
>
> +static void i3c_master_enable_mr_events(struct i3c_master_controller *master)
> +{
> + if (!master->ops->enable_mr_events)
> + return;
> +
> + master->ops->enable_mr_events(master);
> +}
> +
> +static void i3c_master_disable_mr_events(struct i3c_master_controller *master)
> +{
> + if (!master->ops->disable_mr_events)
> + return;
> +
> + master->ops->disable_mr_events(master);
> +}
> +

do we want to do this in broadcast? Is it save to give mastership capabilities to all devices?

> static int i3c_device_remove(struct device *dev)
> {
> struct i3c_device *i3cdev = dev_to_i3cdev(dev);
> @@ -462,6 +492,36 @@ static int i3c_bus_init(struct i3c_bus *i3cbus)
> return 0;
> }
>
> +int i3c_master_request_mastership_locked(struct i3c_master_controller *master)
> +{
> + if (WARN_ON(master->init_done &&
> + !rwsem_is_locked(&master->bus.lock)))
> + return -EINVAL;
> +
> + if (!master->ops->request_mastership)
> + return -ENOTSUPP;
> +
> + return master->ops->request_mastership(master);
> +}
> +
> +int i3c_master_acquire_bus_ownership(struct i3c_master_controller *master)
> +{
> + int ret;
> +
> + if (master->bus.cur_master != master->this) {
> + i3c_bus_normaluse_unlock(&master->bus);
> + i3c_bus_maintenance_lock(&master->bus);
> +
> + ret = i3c_master_request_mastership_locked(master);
> + if (ret) {
> + i3c_bus_maintenance_unlock(&master->bus);
> + return ret;
> + }
> + i3c_bus_downgrade_maintenance_lock(&master->bus);
> + }
> + return 0;
> +}
> +
> static const char * const i3c_bus_mode_strings[] = {
> [I3C_BUS_MODE_PURE] = "pure",
> [I3C_BUS_MODE_MIXED_FAST] = "mixed-fast",
> @@ -620,6 +680,25 @@ i3c_master_alloc_i2c_dev(struct i3c_master_controller *master,
>
> dev->common.master = master;
> dev->boardinfo = boardinfo;
> + dev->addr = boardinfo->base.addr;
> + dev->lvr = boardinfo->lvr;
> +
> + return dev;
> +}
> +
> +static struct i2c_dev_desc *
> +i3c_master_alloc_i2c_dev_no_boardinfo(struct i3c_master_controller *master,
> + u16 addr, u8 lvr)
> +{
> + struct i2c_dev_desc *dev;
> +
> + dev = kzalloc(sizeof(*dev), GFP_KERNEL);
> + if (!dev)
> + return ERR_PTR(-ENOMEM);
> +
> + dev->common.master = master;
> + dev->addr = addr;
> + dev->lvr = lvr;
>
> return dev;
> }
> @@ -693,6 +772,9 @@ i3c_master_find_i2c_dev_by_addr(const struct i3c_master_controller *master,
> struct i2c_dev_desc *dev;
>
> i3c_bus_for_each_i2cdev(&master->bus, dev) {
> + if (!dev->boardinfo)
> + continue;
> +
> if (dev->boardinfo->base.addr == addr)
> return dev;
> }
> @@ -939,8 +1021,8 @@ int i3c_master_defslvs_locked(struct i3c_master_controller *master)
>
> desc = defslvs->slaves;
> i3c_bus_for_each_i2cdev(bus, i2cdev) {
> - desc->lvr = i2cdev->boardinfo->lvr;
> - desc->static_addr = i2cdev->boardinfo->base.addr << 1;
> + desc->lvr = i2cdev->lvr;
> + desc->static_addr = i2cdev->addr << 1;
> desc++;
> }
>
> @@ -1492,6 +1574,83 @@ i3c_master_register_new_i3c_devs(struct i3c_master_controller *master)
> }
> }
>
> +static struct i2c_dev_boardinfo *
> +i3c_master_find_i2c_boardinfo(const struct i3c_master_controller *master,
> + u16 addr, u8 lvr)
> +{
> + struct i2c_dev_boardinfo *i2cboardinfo;
> +
> + list_for_each_entry(i2cboardinfo, &master->boardinfo.i2c, node) {
> + if (i2cboardinfo->base.addr == addr &&
> + i2cboardinfo->lvr == lvr)
> + return i2cboardinfo;
> + }
> +
> + return NULL;
> +}
> +
> +static void
> +i3c_master_register_new_i2c_devs(struct i3c_master_controller *master)
> +{
> + struct i2c_adapter *adap = i3c_master_to_i2c_adapter(master);
> + struct i2c_dev_desc *i2cdev;
> +
> + if (!master->init_done)
> + return;
> +
> + i3c_bus_for_each_i2cdev(&master->bus, i2cdev) {
> + if (i2cdev->dev)
> + continue;
> +
> + if (!i2cdev->boardinfo)
> + continue;
> +
> + i2cdev->dev = i2c_new_device(adap, &i2cdev->boardinfo->base);
> + }
> +}
> +
> +/**
> + * i3c_master_get_accmst_locked() - send a GETACCMST CCC command
> + * @master: master used to send frames on the bus
> + * @info: I3C device information
> + *
> + * Send a GETACCMST CCC command.
> + *
> + * This should be called if the curent master acknowledges bus takeover.
> + *
> + * This function must be called with the bus lock held in write mode.
> + *
> + * Return: 0 in case of success, a positive I3C error code if the error is
> + * one of the official Mx error codes, and a negative error code otherwise.
> + */
> +int i3c_master_get_accmst_locked(struct i3c_master_controller *master,
> + u8 addr)
> +{
> + struct i3c_ccc_getaccmst *accmst;
> + struct i3c_ccc_cmd_dest dest;
> + struct i3c_ccc_cmd cmd;
> + int ret;
> +
> + accmst = i3c_ccc_cmd_dest_init(&dest, addr, sizeof(*accmst));
> + if (!accmst)
> + return -ENOMEM;
> +
> + i3c_ccc_cmd_init(&cmd, true, I3C_CCC_GETACCMST, &dest, 1);
> +
> + ret = i3c_master_send_ccc_cmd_locked(master, &cmd);
> + if (ret)
> + goto out;
> +
> + if (dest.payload.len != sizeof(*accmst))
> + ret = -EIO;
> +
> +out:
> + i3c_ccc_cmd_dest_cleanup(&dest);
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(i3c_master_get_accmst_locked);
> +
> /**
> * i3c_master_do_daa() - do a DAA (Dynamic Address Assignment)
> * @master: master doing the DAA
> @@ -1559,10 +1718,6 @@ int i3c_master_set_info(struct i3c_master_controller *master,
> if (!i3c_bus_dev_addr_is_avail(&master->bus, info->dyn_addr))
> return -EINVAL;
>
> - if (I3C_BCR_DEVICE_ROLE(info->bcr) == I3C_BCR_I3C_MASTER &&
> - master->secondary)
> - return -EINVAL;
> -
> if (master->this)
> return -EINVAL;
>
> @@ -1607,43 +1762,13 @@ static void i3c_master_detach_free_devs(struct i3c_master_controller *master)
> common.node) {
> i3c_master_detach_i2c_dev(i2cdev);
> i3c_bus_set_addr_slot_status(&master->bus,
> - i2cdev->boardinfo->base.addr,
> - I3C_ADDR_SLOT_FREE);
> + i2cdev->addr,
> + I3C_ADDR_SLOT_FREE);
> i3c_master_free_i2c_dev(i2cdev);
> }
> }
>
> -/**
> - * i3c_master_bus_init() - initialize an I3C bus
> - * @master: main master initializing the bus
> - *
> - * This function is following all initialisation steps described in the I3C
> - * specification:
> - *
> - * 1. Attach I2C and statically defined I3C devs to the master so that the
> - * master can fill its internal device table appropriately
> - *
> - * 2. Call &i3c_master_controller_ops->bus_init() method to initialize
> - * the master controller. That's usually where the bus mode is selected
> - * (pure bus or mixed fast/slow bus)
> - *
> - * 3. Instruct all devices on the bus to drop their dynamic address. This is
> - * particularly important when the bus was previously configured by someone
> - * else (for example the bootloader)
> - *
> - * 4. Disable all slave events.
> - *
> - * 5. Pre-assign dynamic addresses requested by the FW with SETDASA for I3C
> - * devices that have a static address
> - *
> - * 6. Do a DAA (Dynamic Address Assignment) to assign dynamic addresses to all
> - * remaining I3C devices
> - *
> - * Once this is done, all I3C and I2C devices should be usable.
> - *
> - * Return: a 0 in case of success, an negative error code otherwise.
> - */
> -static int i3c_master_bus_init(struct i3c_master_controller *master)
> +static int i3c_master_attach_static_devs(struct i3c_master_controller *master)
> {
> enum i3c_addr_slot_status status;
> struct i2c_dev_boardinfo *i2cboardinfo;
> @@ -1652,32 +1777,24 @@ static int i3c_master_bus_init(struct i3c_master_controller *master)
> struct i2c_dev_desc *i2cdev;
> int ret;
>
> - /*
> - * First attach all devices with static definitions provided by the
> - * FW.
> - */
> list_for_each_entry(i2cboardinfo, &master->boardinfo.i2c, node) {
> status = i3c_bus_get_addr_slot_status(&master->bus,
> i2cboardinfo->base.addr);
> - if (status != I3C_ADDR_SLOT_FREE) {
> - ret = -EBUSY;
> - goto err_detach_devs;
> - }
> + if (status != I3C_ADDR_SLOT_FREE)
> + return -EBUSY;
>
> i3c_bus_set_addr_slot_status(&master->bus,
> i2cboardinfo->base.addr,
> I3C_ADDR_SLOT_I2C_DEV);
>
> i2cdev = i3c_master_alloc_i2c_dev(master, i2cboardinfo);
> - if (IS_ERR(i2cdev)) {
> - ret = PTR_ERR(i2cdev);
> - goto err_detach_devs;
> - }
> + if (IS_ERR(i2cdev))
> + return PTR_ERR(i2cdev);
>
> ret = i3c_master_attach_i2c_dev(master, i2cdev);
> if (ret) {
> i3c_master_free_i2c_dev(i2cdev);
> - goto err_detach_devs;
> + return ret;
> }
> }
> list_for_each_entry(i3cboardinfo, &master->boardinfo.i3c, node) {
> @@ -1687,28 +1804,71 @@ static int i3c_master_bus_init(struct i3c_master_controller *master)
>
> if (i3cboardinfo->init_dyn_addr) {
> status = i3c_bus_get_addr_slot_status(&master->bus,
> - i3cboardinfo->init_dyn_addr);
> - if (status != I3C_ADDR_SLOT_FREE) {
> - ret = -EBUSY;
> - goto err_detach_devs;
> - }
> + i3cboardinfo->init_dyn_addr);
> + if (status != I3C_ADDR_SLOT_FREE)
> + return -EBUSY;
> }
>
> i3cdev = i3c_master_alloc_i3c_dev(master, &info);
> - if (IS_ERR(i3cdev)) {
> - ret = PTR_ERR(i3cdev);
> - goto err_detach_devs;
> - }
> + if (IS_ERR(i3cdev))
> + return PTR_ERR(i3cdev);
>
> i3cdev->boardinfo = i3cboardinfo;
>
> ret = i3c_master_attach_i3c_dev(master, i3cdev);
> if (ret) {
> i3c_master_free_i3c_dev(i3cdev);
> - goto err_detach_devs;
> + return ret;
> }
> }
>
> + return 0;
> +}
> +
> +/**
> + * i3c_master_bus_init() - initialize an I3C bus
> + * @master: main master initializing the bus
> + *
> + * This function is following all initialisation steps described in the I3C
> + * specification:
> + *
> + * 1. Attach I2C and statically defined I3C devs to the master so that the
> + * master can fill its internal device table appropriately
> + *
> + * 2. Call &i3c_master_controller_ops->bus_init() method to initialize
> + * the master controller. That's usually where the bus mode is selected
> + * (pure bus or mixed fast/slow bus)
> + *
> + * 3. Instruct all devices on the bus to drop their dynamic address. This is
> + * particularly important when the bus was previously configured by someone
> + * else (for example the bootloader)
> + *
> + * 4. Disable all slave events.
> + *
> + * 5. Pre-assign dynamic addresses requested by the FW with SETDASA for I3C
> + * devices that have a static address
> + *
> + * 6. Do a DAA (Dynamic Address Assignment) to assign dynamic addresses to all
> + * remaining I3C devices
> + *
> + * Once this is done, all I3C and I2C devices should be usable.
> + *
> + * Return: a 0 in case of success, an negative error code otherwise.
> + */
> +static int i3c_master_bus_init(struct i3c_master_controller *master)
> +{
> + struct i3c_dev_desc *i3cdev;
> + int ret;
> +
> + /*
> + * First attach all devices with static definitions provided by the
> + * FW.
> + */
> + if (!master->secondary) {
> + ret = i3c_master_attach_static_devs(master);
> + if (ret)
> + goto err_detach_devs;
> + }

When do we get the i3c_dev_info()?

> /*
> * Now execute the controller specific ->bus_init() routine, which
> * might configure its internal logic to match the bus limitations.
> @@ -1729,6 +1889,13 @@ static int i3c_master_bus_init(struct i3c_master_controller *master)
> }
>
> /*
> + * Don't reset addresses if this is secondary master.
> + * Secondary masters can't do DAA.
> + */
> + if (master->secondary)
> + return 0;
> +
> + /*
> * Reset all dynamic address that may have been assigned before
> * (assigned by the bootloader for example).
> */
> @@ -1791,6 +1958,49 @@ i3c_master_search_i3c_dev_duplicate(struct i3c_dev_desc *refdev)
> return NULL;
> }
>
> +int i3c_master_add_i2c_dev(struct i3c_master_controller *master,
> + u16 addr, u8 lvr)
> +{
> + enum i3c_addr_slot_status status;
> + struct i2c_dev_desc *i2cdev;
> + int ret;
> +
> + if (!master)
> + return -EINVAL;
> +
> + status = i3c_bus_get_addr_slot_status(&master->bus,
> + addr);
> + if (status != I3C_ADDR_SLOT_FREE)
> + return -EBUSY;
> +
> + i3c_bus_set_addr_slot_status(&master->bus, addr,
> + I3C_ADDR_SLOT_I2C_DEV);
> +
> + i2cdev = i3c_master_alloc_i2c_dev_no_boardinfo(master, addr, lvr);
> + if (IS_ERR(i2cdev)) {
> + ret = PTR_ERR(i2cdev);
> + goto err_free_dev;
> + }
> +
> + i2cdev->boardinfo = i3c_master_find_i2c_boardinfo(master, addr, lvr);
> +
> + ret = i3c_master_attach_i2c_dev(master, i2cdev);
> + if (ret) {
> + ret = PTR_ERR(i2cdev);
> + goto err_free_dev;
> + }
> +
> + return 0;
> +
> +err_free_dev:
> + i3c_bus_set_addr_slot_status(&master->bus, addr,
> + I3C_ADDR_SLOT_FREE);
> + i3c_master_free_i2c_dev(i2cdev);
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(i3c_master_add_i2c_dev);
> +
> /**
> * i3c_master_add_i3c_dev_locked() - add an I3C slave to the bus
> * @master: master used to send frames on the bus
> @@ -2113,11 +2323,17 @@ static int i3c_master_i2c_adapter_xfer(struct i2c_adapter *adap,
> }
>
> i3c_bus_normaluse_lock(&master->bus);
> + ret = i3c_master_acquire_bus_ownership(master);
> + if (ret)
> + goto err_unlock_bus;
> +
> dev = i3c_master_find_i2c_dev_by_addr(master, addr);
> if (!dev)
> ret = -ENOENT;
> else
> ret = master->ops->i2c_xfers(dev, xfers, nxfers);
> +
> +err_unlock_bus:
> i3c_bus_normaluse_unlock(&master->bus);
>
> return ret ? ret : nxfers;
> @@ -2151,8 +2367,12 @@ static int i3c_master_i2c_adapter_init(struct i3c_master_controller *master)
> * We silently ignore failures here. The bus should keep working
> * correctly even if one or more i2c devices are not registered.
> */
> - i3c_bus_for_each_i2cdev(&master->bus, i2cdev)
> + i3c_bus_for_each_i2cdev(&master->bus, i2cdev) {
> + if (!i2cdev->boardinfo)
> + continue;
> +
> i2cdev->dev = i2c_new_device(adap, &i2cdev->boardinfo->base);
> + }
>
> return 0;
> }
> @@ -2393,18 +2613,43 @@ static int i3c_master_check_ops(const struct i3c_master_controller_ops *ops)
> !ops->recycle_ibi_slot))
> return -EINVAL;
>
> + if (ops->request_mastership &&
> + (!ops->enable_mr_events || !ops->disable_mr_events))
> + return -EINVAL;
> +
> return 0;
> }
>
> /**
> + * i3c_master_register_new_devs() - register new devices
> + * @master: master used to send frames on the bus
> + *
> + * This function is useful when devices were not added
> + * during initialization or when new device joined the bus
> + * which was under control of different master.
> + */
> +void i3c_master_register_new_devs(struct i3c_master_controller *master)
> +{
> + /*
> + * We can register I3C devices received from master by DEFSLVS.
> + */
> + i3c_bus_normaluse_lock(&master->bus);
> + i3c_master_register_new_i3c_devs(master);
> + i3c_bus_normaluse_unlock(&master->bus);
> +
> + i3c_bus_normaluse_lock(&master->bus);
> + i3c_master_register_new_i2c_devs(master);
> + i3c_bus_normaluse_unlock(&master->bus);
> +}
> +EXPORT_SYMBOL_GPL(i3c_master_register_new_devs);
> +

I would say a function to add DEVSLVS. From the master driver you can pack all
received slaves into i3c_ccc_defslvs struct and rely the task of add those
devices to the subsystem.

> +/**
> * i3c_master_register() - register an I3C master
> * @master: master used to send frames on the bus
> * @parent: the parent device (the one that provides this I3C master
> * controller)
> * @ops: the master controller operations
> - * @secondary: true if you are registering a secondary master. Will return
> - * -ENOTSUPP if set to true since secondary masters are not yet
> - * supported
> + * @secondary: true if you are registering a secondary master.
> *
> * This function takes care of everything for you:
> *
> @@ -2427,10 +2672,6 @@ int i3c_master_register(struct i3c_master_controller *master,
> struct i2c_dev_boardinfo *i2cbi;
> int ret;
>
> - /* We do not support secondary masters yet. */
> - if (secondary)
> - return -ENOTSUPP;
> -
> ret = i3c_master_check_ops(ops);
> if (ret)
> return ret;
> @@ -2504,9 +2745,11 @@ int i3c_master_register(struct i3c_master_controller *master,
> * register I3C devices dicovered during the initial DAA.
> */
> master->init_done = true;
> - i3c_bus_normaluse_lock(&master->bus);
> - i3c_master_register_new_i3c_devs(master);
> - i3c_bus_normaluse_unlock(&master->bus);
> + i3c_master_register_new_devs(master);
> +
> + i3c_bus_maintenance_lock(&master->bus);
> + i3c_master_enable_mr_events(master);
> + i3c_bus_maintenance_unlock(&master->bus);
>
> return 0;
>
> @@ -2524,6 +2767,30 @@ int i3c_master_register(struct i3c_master_controller *master,
> EXPORT_SYMBOL_GPL(i3c_master_register);
>
> /**
> + * i3c_master_mastership_ack() - performs operations before bus handover.
> + * @master: master used to send frames on the bus
> + * @addr: I3C device address
> + *
> + * Basically, it sends DEFSLVS command to ensure new master is taking
> + * the bus with complete list of devices and then acknowledges bus takeover.
> + *
> + * Return: 0 in case of success, a negative error code otherwise.
> + */
> +int i3c_master_mastership_ack(struct i3c_master_controller *master,
> + u8 addr)
> +{
> + int ret;
> +
> + i3c_bus_maintenance_lock(&master->bus);
> + ret = i3c_master_get_accmst_locked(master, addr);
> + i3c_bus_maintenance_unlock(&master->bus);
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(i3c_master_mastership_ack);
> +
> +
> +/**
> * i3c_master_unregister() - unregister an I3C master
> * @master: master used to send frames on the bus
> *
> @@ -2533,6 +2800,10 @@ EXPORT_SYMBOL_GPL(i3c_master_register);
> */
> int i3c_master_unregister(struct i3c_master_controller *master)
> {
> + i3c_bus_maintenance_lock(&master->bus);
> + i3c_master_disable_mr_events(master);
> + i3c_bus_maintenance_unlock(&master->bus);
> +
> 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 42bb215..f32afd3 100644
> --- a/include/linux/i3c/master.h
> +++ b/include/linux/i3c/master.h
> @@ -421,6 +421,12 @@ struct i3c_bus {
> * for a future IBI
> * This method is mandatory only if ->request_ibi is not
> * NULL.
> + * @request_mastership: requests bus mastership. Mastership is requested
> + * automatically when device driver wants to transfer
> + * data using master in slave mode.
> + * @enable_mr_events: enable the Mastership event.
> + * Mastership does not require handler.
> + * @disable_mr_events: disable the Mastership event.
> */
> struct i3c_master_controller_ops {
> int (*bus_init)(struct i3c_master_controller *master);
> @@ -447,6 +453,10 @@ struct i3c_master_controller_ops {
> int (*disable_ibi)(struct i3c_dev_desc *dev);
> void (*recycle_ibi_slot)(struct i3c_dev_desc *dev,
> struct i3c_ibi_slot *slot);
> + void (*update_devs)(struct i3c_master_controller *master);
> + int (*request_mastership)(struct i3c_master_controller *master);
> + int (*enable_mr_events)(struct i3c_master_controller *master);
> + int (*disable_mr_events)(struct i3c_master_controller *master);
> };
>
> /**
> @@ -526,6 +536,8 @@ int i3c_master_defslvs_locked(struct i3c_master_controller *master);
> int i3c_master_get_free_addr(struct i3c_master_controller *master,
> u8 start_addr);
>
> +int i3c_master_add_i2c_dev(struct i3c_master_controller *master,
> + u16 addr, u8 lvr);
> int i3c_master_add_i3c_dev_locked(struct i3c_master_controller *master,
> u8 addr);
> int i3c_master_do_daa(struct i3c_master_controller *master);
> @@ -538,6 +550,11 @@ int i3c_master_register(struct i3c_master_controller *master,
> const struct i3c_master_controller_ops *ops,
> bool secondary);
> int i3c_master_unregister(struct i3c_master_controller *master);
> +int i3c_master_get_accmst_locked(struct i3c_master_controller *master,
> + u8 addr);
> +int i3c_master_mastership_ack(struct i3c_master_controller *master,
> + u8 addr);
> +void i3c_master_register_new_devs(struct i3c_master_controller *master);
>
> /**
> * i3c_dev_get_master_data() - get master private data attached to an I3C

Best regards,
Vitor Soares