From: Przemyslaw Gaj <pgaj@cadence.com>
To: Boris Brezillon <bbrezillon@kernel.org>
Cc: "linux-i3c@lists.infradead.org" <linux-i3c@lists.infradead.org>,
Przemyslaw Sroka <psroka@cadence.com>,
Rafal Ciepiela <rafalc@cadence.com>,
"vitor.soares@synopsys.com" <vitor.soares@synopsys.com>
Subject: Re: [PATCH 1/2] i3c: Add support for mastership request to I3C subsystem
Date: Fri, 14 Dec 2018 07:14:44 +0000 [thread overview]
Message-ID: <DCAE2FA2-D22C-4010-A13C-3413DBCF4721@cadence.com> (raw)
In-Reply-To: <20181213145020.124cb103@bbrezillon>
Hi Boris,
Thank you for reviewing this so quickly.
On 12/13/18, 2:50 PM, "Boris Brezillon" <bbrezillon@kernel.org> wrote:
EXTERNAL MAIL
On Thu, 13 Dec 2018 12:28:00 +0000
Przemyslaw Gaj <pgaj@cadence.com> wrote:
> This patch adds support for mastership request to I3C subsystem.
>
> Mastership event is enabled for all the masters on the I3C bus
> by default.
>
> Mastership request occurs in the following cases:
> - Mastership is requested automatically after secondary master
> receives mastership ENEC event. This allows secondary masters to
> initialize their bus
I guess this only happens when the secondary master received a DEFSLVS
packet, right?
Actually, this happens when current master enables MR event.
He should take care of providing devices list.
Of course, we can consider adding a flag which indicates if slaves are already defined or not.
> - If above procedure fails for some reason, user can force
> mastership request through sysfs. Of course, mastership event
> still has to be enabled on current master side
Sounds like a good idea.
> - Mastership is also requested automatically when device driver
> tries to transfer data using master controller in slave mode.
And that too.
>
> There is still some limitation:
> - I2C devices are not registered on secondary master side. I2C
> devices received in DEFSLVS CCC command are added to device list
> just to send back this information to the other master controllers
> in case of bus handoff. We can add support for this in the future
> if there is such use case.
I2C devices must be described under the bus represented by the
secondary master for that to work properly (we need a way to bind
devices to drivers). So, it's probably a good thing to discard (or
leave them inactive) any devices that do not have a i2c_board_info entry
on the secondary master side.
>
> Signed-off-by: Przemyslaw Gaj <pgaj@cadence.com>
> ---
> drivers/i3c/master.c | 260 ++++++++++++++++++++++++++++++++++++++++++---
> include/linux/i3c/master.h | 43 +++++++-
> 2 files changed, 289 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
> index 68d6553..e98b600 100644
> --- a/drivers/i3c/master.c
> +++ b/drivers/i3c/master.c
> @@ -460,6 +460,15 @@ static int i3c_bus_init(struct i3c_bus *i3cbus)
> return 0;
> }
>
> +static int i3c_master_request_mastership(struct i3c_master_controller *master)
> +{
> + if (!master->ops->request_mastership)
> + return -ENOTSUPP;
Please add a blank line here
> + if (master->ops->request_mastership(master))
> + return -EIO;
and here.
Ok for both.
> + return 0;
> +}
> +
> static const char * const i3c_bus_mode_strings[] = {
> [I3C_BUS_MODE_PURE] = "pure",
> [I3C_BUS_MODE_MIXED_FAST] = "mixed-fast",
> @@ -491,8 +500,15 @@ static ssize_t current_master_show(struct device *dev,
> char *buf)
> {
> struct i3c_bus *i3cbus = dev_to_i3cbus(dev);
> + struct i3c_master_controller *master;
> ssize_t ret;
>
> + if (!i3cbus->cur_master) {
> + master = container_of(i3cbus, struct i3c_master_controller, bus);
> + if (i3c_master_request_mastership(master))
> + return sprintf(buf, "unknown\n");
> + }
Hm, why is that needed? When you are a secondary master ->cur_master
should be set to the i3c_dev_desc representing the main master at init
time.
I do not add devices at initialization time, so secondary master does not have i3c_dev_desc object describing main master.
DEFSLVS is providing us incomplete information, we don’t have PID yet. PID is retrieved from device when secondary master adds device.
This is possible only if current master is ready to relinquish bus control.
I decided to set cur_master after secondary master completely initialized his bus. What do you think?
> +
> i3c_bus_normaluse_lock(i3cbus);
> ret = sprintf(buf, "%d-%llx\n", i3cbus->id,
> i3cbus->cur_master->info.pid);
> @@ -659,6 +675,11 @@ static int i3c_master_send_ccc_cmd_locked(struct i3c_master_controller *master,
> if (!cmd || !master)
> return -EINVAL;
>
> + if (master->op_mode == I3C_SLAVE_MODE) {
> + if (i3c_master_request_mastership(master))
> + return -EIO;
> + }
> +
This shouldn't be needed either. if i3cbus->cur_master != master->this,
that means the master is operating in slave mode.
I'll try that.
> if (WARN_ON(master->init_done &&
> !rwsem_is_locked(&master->bus.lock)))
> return -EINVAL;
> @@ -1371,6 +1392,7 @@ static int i3c_master_reattach_i3c_dev(struct i3c_dev_desc *dev,
> dev->info.dyn_addr);
> if (status != I3C_ADDR_SLOT_FREE)
> return -EBUSY;
> +
Drop this change (or make it part of a separate patch fixing coding
style issues).
Ok.
> i3c_bus_set_addr_slot_status(&master->bus,
> dev->info.dyn_addr,
> I3C_ADDR_SLOT_I3C_DEV);
> @@ -1491,6 +1513,46 @@ i3c_master_register_new_i3c_devs(struct i3c_master_controller *master)
> }
>
> /**
> + * i3c_master_get_accmst_locked() - send a GETACCMST CCC command
> + * @master: master used to send frames on the bus
> + * @info: I3C device information
Maybe you can simply pass the dynamic address instead of the full
device_info struct.
No problem. Now I see it's not needed.
> + *
> + * Send a GETACCMST CCC command.
> + *
> + * This should be called if the curent master acknowledges bus takeover.
I really thought this would be all automated by the master IP, but
apparently it's not :-).
> + *
> + * 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,
> + const struct i3c_device_info *info)
> +{
> + 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, info->dyn_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)
> + return ret;
> +
> + if (dest.payload.len != sizeof(*accmst))
> + return -EIO;
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(i3c_master_get_accmst_locked);
> +
> +/**
> * i3c_master_do_daa() - do a DAA (Dynamic Address Assignment)
> * @master: master doing the DAA
> *
> @@ -1554,11 +1616,8 @@ int i3c_master_set_info(struct i3c_master_controller *master,
> struct i3c_dev_desc *i3cdev;
> int ret;
>
> - 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)
> + if (master->op_mode == I3C_MASTER_MODE &&
> + !i3c_bus_dev_addr_is_avail(&master->bus, info->dyn_addr))
Please place this test in a separate if branch, as they are not really
related.
Ok.
> return -EINVAL;
>
> if (master->this)
> @@ -1569,7 +1628,8 @@ int i3c_master_set_info(struct i3c_master_controller *master,
> return PTR_ERR(i3cdev);
>
> master->this = i3cdev;
> - master->bus.cur_master = master->this;
> + if (master->op_mode == I3C_MASTER_MODE)
> + master->bus.cur_master = master->this;
When you hare a secondary master, you should make ->cur_master point to
an i3c_dev_desc describing the current/main master. Should be possible
thanks to the DEFSLVS info.
As I said before it's not so easy with DEFSLVS.
We can consider creating device list without full information. Do you agree?
>
> ret = i3c_master_attach_i3c_dev(master, i3cdev);
> if (ret)
> @@ -1727,6 +1787,12 @@ 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.
> + */
Hm, I'm not sure how that's supposed to work. What if the secondary
master was initialized by the bootloader. Don't you need to reset
something and maybe trigger a MSTREQ+DAA or a HotJoin?
Even if secondary master does MSTREQ, he can't do DAA. He can't manage the bus, only main master can do that.
I'll point this information in spec below.
I'm wondering about HotJoin. I can't find this use case in my mind.
> + if (master->secondary)
> + return 0;
Add a blank like here.
Ok.
> + /*
> * Reset all dynamic address that may have been assigned before
> * (assigned by the bootloader for example).
> */
> @@ -1738,6 +1804,7 @@ static int i3c_master_bus_init(struct i3c_master_controller *master)
> ret = i3c_master_disec_locked(master, I3C_BROADCAST_ADDR,
> I3C_CCC_EVENT_SIR | I3C_CCC_EVENT_MR |
> I3C_CCC_EVENT_HJ);
> +
Drop this blank line.
Ok.
> if (ret && ret != I3C_ERROR_M2)
> goto err_bus_cleanup;
>
> @@ -1789,6 +1856,44 @@ i3c_master_search_i3c_dev_duplicate(struct i3c_dev_desc *refdev)
> return NULL;
> }
>
> +int i3c_master_add_i2c_dev(struct i3c_master_controller *master,
> + struct i2c_dev_boardinfo *info)
> +{
> + enum i3c_addr_slot_status status;
> + struct device *dev = &master->dev;
> + struct i2c_dev_boardinfo *boardinfo;
> + struct i2c_dev_desc *i2cdev;
> + int ret;
> +
> + status = i3c_bus_get_addr_slot_status(&master->bus,
> + info->base.addr);
> + if (status != I3C_ADDR_SLOT_FREE)
> + return -EBUSY;
> +
> + boardinfo = devm_kzalloc(dev, sizeof(*boardinfo), GFP_KERNEL);
> + if (!boardinfo)
> + return -ENOMEM;
> +
> + i3c_bus_set_addr_slot_status(&master->bus,
> + info->base.addr,
> + I3C_ADDR_SLOT_I2C_DEV);
> +
> + boardinfo->base.addr = info->base.addr;
> + boardinfo->lvr = info->lvr;
> +
> + i2cdev = i3c_master_alloc_i2c_dev(master, boardinfo);
> + if (IS_ERR(i2cdev))
> + return PTR_ERR(i2cdev);
> +
> + ret = i3c_master_attach_i2c_dev(master, i2cdev);
> + if (ret) {
> + i3c_master_free_i2c_dev(i2cdev);
> + return ret;
> + }
> + return 0;
> +}
> +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
> @@ -1832,6 +1937,15 @@ int i3c_master_add_i3c_dev_locked(struct i3c_master_controller *master,
> if (ret)
> goto err_free_dev;
>
> + if (I3C_BCR_DEVICE_ROLE(newdev->info.bcr) == I3C_BCR_I3C_MASTER &&
> + master->ops->enable_mastership) {
> + if (master->ops->enable_mastership(newdev)) {
> + dev_warn(&master->dev,
> + "Failed to enable mastership for device: %d%llx",
> + master->bus.id, newdev->info.pid);
> + }
> + }
> +
> olddev = i3c_master_search_i3c_dev_duplicate(newdev);
> if (olddev) {
> newdev->boardinfo = olddev->boardinfo;
> @@ -2102,6 +2216,11 @@ static int i3c_master_i2c_adapter_xfer(struct i2c_adapter *adap,
> return -ENOTSUPP;
> }
>
> + if (master->op_mode == I3C_SLAVE_MODE) {
> + if (i3c_master_request_mastership(master))
Shouldn't this be called with the lock held?
Oh, and we might have a race here if the master gains bus ownership and
looses it just after that and before it was able to the transfer. I
think we should:
1/ send a GETACCMST
2/ disable MR EVENTS
3/ do the I3C/I2C transfers
4/ enable MR EVENTS
1 and 2 should be sent in the same transaction to avoid cases where
another master tries to acquire the bus, or if that's not possible, the
master that just gained ownership of the bus should reject any incoming
MR until DISEC(MR) has been sent.
I strongly agree with your opinion. I'll implement it this way.
> + return -EIO;
> + }
> +
> i3c_bus_normaluse_lock(&master->bus);
> dev = i3c_master_find_i2c_dev_by_addr(master, addr);
> if (!dev)
> @@ -2393,15 +2512,31 @@ static int i3c_master_check_ops(const struct i3c_master_controller_ops *ops)
> return 0;
> }
>
> +static void i3c_master_bus_takeover(struct work_struct *work)
> +{
> + struct i3c_master_controller *master;
> +
> + master = container_of(work, struct i3c_master_controller, mastership);
> +
> + i3c_bus_maintenance_lock(&master->bus);
> + master->ops->update_devs(master);
> + i3c_bus_maintenance_unlock(&master->bus);
Blank like here please.
Ok
> + /*
> + * We can register I3C devices received from master by DEFSLVS.
> + */
> + master->init_done = true;
I think this should be set before that, in the i3c_master_register()
function. When a secondary master is initialized, it should populate
the dev list based on a previous DEFSLVS frame it has received before
the ->probe() call, or just start with an empty list.
Same here. I do not populate list of devices until I'm able to get full information from devices.
> + i3c_bus_normaluse_lock(&master->bus);
> + i3c_master_register_new_i3c_devs(master);
> + i3c_bus_normaluse_unlock(&master->bus);
> +}
> +
> /**
> * 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:
> *
> @@ -2424,10 +2559,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;
> @@ -2439,6 +2570,7 @@ int i3c_master_register(struct i3c_master_controller *master,
> master->dev.release = i3c_masterdev_release;
> master->ops = ops;
> master->secondary = secondary;
> + master->op_mode = secondary ? I3C_SLAVE_MODE : I3C_MASTER_MODE;
> INIT_LIST_HEAD(&master->boardinfo.i2c);
> INIT_LIST_HEAD(&master->boardinfo.i3c);
>
> @@ -2497,6 +2629,13 @@ int i3c_master_register(struct i3c_master_controller *master,
> goto err_del_dev;
>
> /*
> + * We can stop here. Devices will be attached after bus takeover.
> + * Secondary masters can't do DAA.
> + */
> + if (master->secondary)
> + return 0;
See, I don't think we should stop here. We should provide a hook to let
secondary slaves populate the dev list based on data they might have
received before that.
Do we want to populate list with data received by DEFSLVS?
If yes, I can populate list but we are not able to register devices because we don’t have
PID yet and device drivers are matched by PID.
> +
> + /*
> * We're done initializing the bus and the controller, we can now
> * register I3C devices dicovered during the initial DAA.
> */
> @@ -2521,6 +2660,95 @@ int i3c_master_register(struct i3c_master_controller *master,
> EXPORT_SYMBOL_GPL(i3c_master_register);
>
> /**
> + * i3c_master_switch_operation_mode() - changes operation mode of the controller
> + * @master: master used to send frames on the bus
> + * @new_master: the device that takes control of the bus
> + * @op_mode: the master new operation mode
> + *
> + * Return: 0 in case of success, a negative error code otherwise.
> + */
> +int i3c_master_switch_operation_mode(struct i3c_master_controller *master,
> + struct i3c_dev_desc *new_master,
> + enum i3c_op_mode new_op_mode)
> +{
> + if (master->op_mode == new_op_mode)
> + return -EEXIST;
> +
> + master->op_mode = new_op_mode;
> +
> + if (new_op_mode == I3C_SLAVE_MODE) {
> + master->bus.cur_master = new_master;
> + } else {
> + master->bus.cur_master = master->this;
This is done too early. You should update cur->master only after the
GETACCMST operation succeeds.
Indeed.
> + INIT_WORK(&master->mastership, i3c_master_bus_takeover);
INIT_WORK() should be called once at init time, not everytime you
switch from one mode to another.
My mistake.
> + queue_work(master->wq, &master->mastership);
> + }
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(i3c_master_switch_operation_mode);
> +
> +/**
> + * i3c_master_mastership_ack() - performs operations before bus handover.
> + * @master: master used to send frames on the bus
> + * @info: I3C device information
> + *
> + * 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,
> + const struct i3c_device_info *info)
> +{
> + int ret;
> +
> + i3c_bus_maintenance_lock(&master->bus);
> + ret = i3c_master_defslvs_locked(master);
> + i3c_bus_maintenance_unlock(&master->bus);
> + if (ret)
> + return ret;
> +
Hm, this should have been sent during DAA, no need to do it again here,
right?
I thought that there could be the case when something has changed on the bus but now I think
device list on secondary master side is up-to-date without that. I'll drop this.
> + i3c_bus_maintenance_lock(&master->bus);
> + ret = i3c_master_get_accmst_locked(master, info);
> + i3c_bus_maintenance_unlock(&master->bus);
> + if (ret)
> + return ret;
Hm, will this really work? I thought the mastership handover procedure
had to be done atomically (using Sr between each transfer to avoid
external interruptions). I might be wrong though (need to read the
spec to refresh my memory).
There wouldn't be interruption. Secondary master may ack or nack this command.
If he acks, it's done, he became current master.
If he nacks or transmits incorrect address, he will not acquire mastership.
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(i3c_master_mastership_ack);
> +
> +static void i3c_secondary_master_bus_init(struct work_struct *work)
> +{
> + struct i3c_master_controller *master;
> +
> + master = container_of(work, struct i3c_master_controller, mastership);
> +
> + if (i3c_master_request_mastership(master))
> + dev_err(&master->dev, "Mastership failed.");
> +}
> +
> +/**
> + * i3c_secondary_master_events_enabled() - event from current master
> + * @master: master used to send frames on the bus
> + * @evts: enabled events
> + *
> + * This function allows to perform required operations after current
> + * master enables particular events on the bus.
> + */
> +void i3c_secondary_master_events_enabled(struct i3c_master_controller *master,
> + u8 evts)
> +{
> + if ((evts & I3C_CCC_EVENT_MR) &&
> + !master->init_done) {
> + INIT_WORK(&master->mastership, i3c_secondary_master_bus_init);
> + queue_work(master->wq, &master->mastership);
> + }
> +}
> +EXPORT_SYMBOL_GPL(i3c_secondary_master_events_enabled);
Hm, so you're trying to standardize events handling. I had initially
left that to the driver as it's likely to be controller specific.
I think that I3C spec describes common set of events.
> +
> +/**
> * i3c_master_unregister() - unregister an I3C master
> * @master: master used to send frames on the bus
> *
> @@ -2552,6 +2780,11 @@ int i3c_dev_do_priv_xfers_locked(struct i3c_dev_desc *dev,
> if (!master || !xfers)
> return -EINVAL;
>
> + if (master->op_mode == I3C_SLAVE_MODE) {
> + if (i3c_master_request_mastership(master))
> + return -EIO;
> + }
> +
> if (!master->ops->priv_xfers)
> return -ENOTSUPP;
>
> @@ -2657,5 +2890,6 @@ static void __exit i3c_exit(void)
> module_exit(i3c_exit);
>
> MODULE_AUTHOR("Boris Brezillon <boris.brezillon@bootlin.com>");
> +MODULE_AUTHOR("Przemyslaw Gaj <pgaj@cadence.com>");
Please do that in a separate patch.
Ok.
> MODULE_DESCRIPTION("I3C core");
> MODULE_LICENSE("GPL v2");
> diff --git a/include/linux/i3c/master.h b/include/linux/i3c/master.h
> index f13fd8b..ada956a 100644
> --- a/include/linux/i3c/master.h
> +++ b/include/linux/i3c/master.h
> @@ -282,6 +282,16 @@ enum i3c_addr_slot_status {
> };
>
> /**
> + * enum i3c_op_mode - I3C controller operation mode
> + * @I3C_SLAVE_MODE: I3C controller operates in slave mode
> + * @I3C_MASTER_MODE: I3C controller operates in master mode
> + */
> +enum i3c_op_mode {
> + I3C_SLAVE_MODE,
> + I3C_MASTER_MODE
> +};
> +
> +/**
> * struct i3c_bus - I3C bus object
> * @cur_master: I3C master currently driving the bus. Since I3C is multi-master
> * this can change over the time. Will be used to let a master
> @@ -418,6 +428,20 @@ struct i3c_bus {
> * for a future IBI
> * This method is mandatory only if ->request_ibi is not
> * NULL.
> + * @update_devs: updates device list. Called after bus takeover. Secondary
> + * master can't perform DAA procedure.
Is that really true? Can you point me to the relevant section in the
spec describing this constraint?
Sure, I3C Devices Roles vs Responsibilities, Table in spec. Dynamic Address
Assignment, Secondary master is not capable to do that.
Please also take a look at Hot-Join Dynamic Address Assignment. Some of controllers may
have HJ-DAA implemented in the hardware, maybe it's better to send DEFSLVS right before
GETACCMST?
> This function allows to
> + * update devices received from previous bus owner in DEFSLVS
> + * command. Useful also when new device joins the bus controlled
> + * by secondary master, main master will be able to add
> + * this device after mastership takeover.
> + * @request_mastership: requests bus mastership. By default called by secondary
> + * master after ENEC CCC is received and when devices were
> + * not fully initialized yet. Mastership is also requested
> + * when device driver wants to transfer data using master
> + * in slave mode.
> + * @enable_mastership: enable the Mastership for specified device. Mastership
> + * does not require handler. Mastership is enabled for all
> + * masters by default.
Maybe ->enable_mr_events() (->enable_mastership() is just unclear to
me). Oh, and if you have an enable function you should have a disable
one too.
I agree.
> */
> struct i3c_master_controller_ops {
> int (*bus_init)(struct i3c_master_controller *master);
> @@ -445,6 +469,9 @@ 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_mastership)(struct i3c_dev_desc *dev);
> };
>
> /**
> @@ -458,6 +485,7 @@ struct i3c_master_controller_ops {
> * @ops: master operations. See &struct i3c_master_controller_ops
> * @secondary: true if the master is a secondary master
> * @init_done: true when the bus initialization is done
> + * @op_mode: controller operation mode
As said earlier, I don't think this is needed. Just check ->cur_master
to know whether the master owns the bus or not.
I'll try.
> * @boardinfo.i3c: list of I3C boardinfo objects
> * @boardinfo.i2c: list of I2C boardinfo objects
> * @boardinfo: board-level information attached to devices connected on the bus
> @@ -467,6 +495,7 @@ struct i3c_master_controller_ops {
> * in a thread context. Typical examples are Hot Join processing which
> * requires taking the bus lock in maintenance, which in turn, can only
> * be done from a sleep-able context
> + * @mastership: work for switching operation mode after bus takeover
How about mr_work or something like that. mastership is a bit
ambiguous, and it's not really clear that it's representing a work
object.
I agree.
> *
> * A &struct i3c_master_controller has to be registered to the I3C subsystem
> * through i3c_master_register(). None of &struct i3c_master_controller fields
> @@ -480,12 +509,14 @@ struct i3c_master_controller {
> const struct i3c_master_controller_ops *ops;
> unsigned int secondary : 1;
> unsigned int init_done : 1;
> + enum i3c_op_mode op_mode;
> struct {
> struct list_head i3c;
> struct list_head i2c;
> } boardinfo;
> struct i3c_bus bus;
> struct workqueue_struct *wq;
> + struct work_struct mastership;
> };
>
> /**
> @@ -523,7 +554,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,
> + struct i2c_dev_boardinfo *info);
> int i3c_master_add_i3c_dev_locked(struct i3c_master_controller *master,
> u8 addr);
> int i3c_master_do_daa(struct i3c_master_controller *master);
> @@ -536,6 +568,15 @@ 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,
> + const struct i3c_device_info *info);
> +int i3c_master_switch_operation_mode(struct i3c_master_controller *master,
> + struct i3c_dev_desc *new_master,
> + enum i3c_op_mode new_op_mode);
> +int i3c_master_mastership_ack(struct i3c_master_controller *master,
> + const struct i3c_device_info *info);
> +void i3c_secondary_master_events_enabled(struct i3c_master_controller *master,
> + u8 evts);
>
> /**
> * i3c_dev_get_master_data() - get master private data attached to an I3C
Thanks,
Przemek
_______________________________________________
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c
next prev parent reply other threads:[~2018-12-14 7:15 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-12-13 12:27 [PATCH 0/2] Add the I3C mastership request Przemyslaw Gaj
2018-12-13 12:28 ` [PATCH 1/2] i3c: Add support for mastership request to I3C subsystem Przemyslaw Gaj
2018-12-13 13:50 ` Boris Brezillon
2018-12-14 7:14 ` Przemyslaw Gaj [this message]
2018-12-14 8:24 ` Boris Brezillon
2018-12-14 13:47 ` Przemyslaw Gaj
2018-12-19 14:16 ` Boris Brezillon
2018-12-13 12:28 ` [PATCH 2/2] i3c: master: cdns: add support for mastership request to Cadence I3C master driver Przemyslaw Gaj
2018-12-13 12:47 ` [PATCH 0/2] Add the I3C mastership request Boris Brezillon
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=DCAE2FA2-D22C-4010-A13C-3413DBCF4721@cadence.com \
--to=pgaj@cadence.com \
--cc=bbrezillon@kernel.org \
--cc=linux-i3c@lists.infradead.org \
--cc=psroka@cadence.com \
--cc=rafalc@cadence.com \
--cc=vitor.soares@synopsys.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).