On Mon, 11 May 2020 15:14:49 +0200
Parshuram Thombare wrote:
> Added mastership acquire and yield functions.
>
> Signed-off-by: Parshuram Thombare
> ---
> drivers/i3c/master.c | 187 +++--
> include/linux/i3c/master.h | 23 +
> 2 files changed, 201 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
> index 669bd7e45810..9c8250a6a2b0 100644
> --- a/drivers/i3c/master.c
> +++ b/drivers/i3c/master.c
> @@ -16,6 +16,7 @@
> #include
> #include
> #include
> +#include
>
> #include "internals.h"
>
> @@ -467,6 +468,79 @@ static const char * const i3c_bus_mode_strings[] = {
> [I3C_BUS_MODE_MIXED_SLOW] = "mixed-slow",
> };
>
> +static int i3c_master_enable_mr_events(struct i3c_master_controller *master)
> +{
> + int ret;
> +
> + master->ops->enable_mr_events(master);
> + i3c_bus_maintenance_lock(>bus);
> + ret = i3c_master_enec_locked(master, I3C_BROADCAST_ADDR,
> + I3C_CCC_EVENT_MR | I3C_CCC_EVENT_HJ);
> + i3c_bus_maintenance_unlock(>bus);
> +
> + return ret;
> +}
> +
> +static int check_event_da_update(struct i3c_master_controller *m)
> +{
> + return m->ops->check_event_set(m, I3C_SLV_DA_UPDATE);
> +}
> +
> +static int check_event_mr_done(struct i3c_master_controller *m)
> +{
> + return m->ops->check_event_set(m, I3C_SLV_MR_DONE);
> +}
> +
> +/**
> + * i3c_master_acquire_bus() - acquire I3C bus mastership
> + * @m: I3C master object
> + *
> + * This function may sleep.
> + * It is expected to be called with normaluse_lock.
> + */
> +static int i3c_master_acquire_bus(struct i3c_master_controller *m)
> +{
> + int ret = 0;
> + u32 val;
> +
> + /*
> + * Request mastership needs maintenance(write) lock. So, to avoid
> + * deadlock, release normaluse(read) lock, which is expected to be
> + * held before calling this function.
> + * normaluse(read) lock is expected to be held before calling
> + * this function to avoid race with maintenance activities
> + * like DEFSLVS processing etc
> + */
> + i3c_bus_normaluse_unlock(>bus);
> + i3c_bus_maintenance_lock(>bus);
> +
> + if (m->this && m->this == m->bus.cur_master) {
> + i3c_master_disec_locked(m, I3C_BROADCAST_ADDR,
> + I3C_CCC_EVENT_MR |
> + I3C_CCC_EVENT_HJ);
> + goto mr_req_done;
> + }
> +
> + ret = readx_poll_timeout(check_event_da_update, m, val,
> + val, 100, 100);
> + if (ret)
> + goto mr_req_done;
> +
> + ret = m->ops->request_mastership(m);
> + if (ret)
> + goto mr_req_done;
> +
> + ret = readx_poll_timeout(check_event_mr_done, m, val,
> + val, 100, 100);
Those waits should be done in the master driver. Pass a timeout to
->request_master() or make it a property of the i3c_master_controller
if you like, but don't poll the status from the core.
> + if (!ret)
> + m->bus.cur_master = m->this;
> +
> +mr_req_done:
> + i3c_bus_maintenance_unlock(>bus);
> + i3c_bus_normaluse_lock(>bus);
You should downgrade the lock instead of releasing it. I really need to
get my head around this locking scheme because I'm pretty sure we had
good reasons for the locking/unlocking dance Przemek had in his series.
> + return ret;
> +}
> +
> static ssize_t mode_show(struct device *dev,
>struct device_attribute *da,
>char *buf)
> @@ -685,6 +759,33 @@ static int i3c_master_send_ccc_cmd_locked(struct
> i3c_master_controller *master,
> return 0;
> }
>
> +static 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(, addr, sizeof(*accmst));
> + if (!accmst)
> + return -ENOMEM;
> +
> + i3c_ccc_cmd_init(, true, I3C_CCC_GETACCMST, , 1);
> +
> + ret = i3c_master_send_ccc_cmd_locked(master, );
> + if (ret)
> + goto out;
> +
> + if (dest.payload.len != sizeof(*accmst))
> + ret = -EIO;
> +
> +out:
> + i3c_ccc_cmd_dest_cleanup();
> +
> + return ret;
> +}
> +
> static struct i2c_dev_desc *
> i3c_master_find_i2c_dev_by_addr(const struct i3c_master_controller *master,
> u16 addr)
> @@ -1558,10 +1659,6 @@ int i3c_master_set_info(struct i3c_master_controller
> *master,
> if (!i3c_bus_dev_addr_is_avail(>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