* [PATCH v7 0/7] I3C mastership handover support
@ 2020-05-11 13:11 Parshuram Thombare
2020-05-11 13:12 ` [PATCH v7 1/7] i3c: master: secondary master initialization document Parshuram Thombare
` (6 more replies)
0 siblings, 7 replies; 17+ messages in thread
From: Parshuram Thombare @ 2020-05-11 13:11 UTC (permalink / raw)
To: bbrezillon, vitor.soares
Cc: mparab, Parshuram Thombare, praneeth, linux-kernel, pgaj, linux-i3c
Hi,
As per the discussion with Boris sending v7 of
patchset with changes mentioned in below changelog v6->v7.
Main changes between v6 and v7 are:
- Added separate functions for main and secondary
master initialization
- Secondary master initialization don't wait for
DEFSLSVS.
- Change to use I2C device information from DTS,
and corresponding changes in controller driver
and I3C core DEFSLVS processing to ignore I2C
devices received in DEFSLVS
- Reverted bus_init split
- Fixed formatting issues in document
Main changes between v5 and v6 are:
- Moved populate_bus() hook to master subsystem code.
- For secondary master initialization i3c_master_register
spawan separate threads, as secondary master may have to
wait for DEFSLVS and bus mastership.
- Populate bus info is based on DEFSLVS data and take care
of hot plugged / unplugged I3C devices.
- Split bus_init into bus_init and master_set_info callbacks
- Moved mastership aquire and handover to separate state
machines.
- Added DEFSLVS processing code.
- Moved back all locks in side the subsystem code.
- Secondary mastership support to Cadence I3C master
controller driver
- Sysfs key 'i3c_acquire_bus' to acauire bus.
- NULL check for pool pointer in i3c_generic_ibi_free_pool.
Main changes between v4 and v5 are:
- Add populate_bus() hook
- Split i3c_master_register into init and register pair
- Split device information retrieval, let add partialy discovered devices
- Make i3c_master_set_info private
- Add separate function to register secondary master
- Reworked secondary master register in CDNS driver
- Export i3c_bus_set_mode
Main changes between v3 and v4 are:
- Reworked acquire bus ownership
- Refactored the code
Main changes between v2 and v3 are:
- Added DEFSLVS devices are registered from master driver
- Reworked I2C registering on secondary master side
- Reworked Mastership event is enabled/disabled globally (for all devices)
Main changes between initial version and v2 are:
- Reworked devices registration on secondary master side
- Reworked mastership event disabling/enabling
- Reworked bus locking during mastership takeover process
- Added DEFSLVS devices registration during initialization
- Fixed style issues
Regards,
Parshuram Thombare
Parshuram Thombare (7):
i3c: master: secondary master initialization document
i3c: master: use i3c_master_register only for main master
i3c: master: add i3c_secondary_master_register
i3c: master: add mastership handover support
i3c: master: add defslvs processing
i3c: master: sysfs key for acquire bus
i3c: master: mastership handover, defslvs processing in cdns
controller driver
Documentation/driver-api/i3c/index.rst | 1 +
.../i3c/secondary-master-initialization.rst | 118 ++++
drivers/i3c/master.c | 566 ++++++++++++++++--
drivers/i3c/master/dw-i3c-master.c | 2 +-
drivers/i3c/master/i3c-master-cdns.c | 377 +++++++++++-
include/linux/i3c/master.h | 36 +-
6 files changed, 1038 insertions(+), 62 deletions(-)
create mode 100644 Documentation/driver-api/i3c/secondary-master-initialization.rst
--
2.17.1
_______________________________________________
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v7 1/7] i3c: master: secondary master initialization document
2020-05-11 13:11 [PATCH v7 0/7] I3C mastership handover support Parshuram Thombare
@ 2020-05-11 13:12 ` Parshuram Thombare
2020-05-11 16:05 ` Boris Brezillon
2020-05-11 13:13 ` [PATCH v7 2/7] i3c: master: use i3c_master_register only for main master Parshuram Thombare
` (5 subsequent siblings)
6 siblings, 1 reply; 17+ messages in thread
From: Parshuram Thombare @ 2020-05-11 13:12 UTC (permalink / raw)
To: bbrezillon, vitor.soares
Cc: mparab, Parshuram Thombare, praneeth, linux-kernel, pgaj, linux-i3c
Document describing secondary master initialization,
mastership handover and DEFSLVS handling processes.
Signed-off-by: Parshuram Thombare <pthombar@cadence.com>
---
Documentation/driver-api/i3c/index.rst | 1 +
.../i3c/secondary-master-initialization.rst | 118 ++++++++++++++++++
2 files changed, 119 insertions(+)
create mode 100644 Documentation/driver-api/i3c/secondary-master-initialization.rst
diff --git a/Documentation/driver-api/i3c/index.rst b/Documentation/driver-api/i3c/index.rst
index 783d6dad054b..af2a0aa68f5b 100644
--- a/Documentation/driver-api/i3c/index.rst
+++ b/Documentation/driver-api/i3c/index.rst
@@ -9,3 +9,4 @@ I3C subsystem
protocol
device-driver-api
master-driver-api
+ secondary-master-initialization
diff --git a/Documentation/driver-api/i3c/secondary-master-initialization.rst b/Documentation/driver-api/i3c/secondary-master-initialization.rst
new file mode 100644
index 000000000000..9d1869550807
--- /dev/null
+++ b/Documentation/driver-api/i3c/secondary-master-initialization.rst
@@ -0,0 +1,118 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+===================================
+I3C Secondary Master Initialization
+===================================
+
++-----------------------------------------+-------------------------------------------+
+| **Main master** | **Secondary master** |
++=========================================+===========================================+
+| | |
+| | Do I3C master controller specific | | Do I3C master controller specefic |
+| initialization. | initialization, except enabling |
+| | the DEFSLVS interrupt. |
+| | Call i3c_master_register | | Call i3c_secondary_master_register |
+| | |
+| *i3c_master_register* | *i3c_secondary_master_register* |
+| | Initialize I3C master controller | | Initialize I3C master controller |
+| object. | object. |
+| | Scan I3C and I2C devices from DTS. | | Scan I2C devices from DTS. |
+| | Set appropriate bus mode based on | | Set appropriate bus mode based on |
+| I2C devices information. | I3C and I2C devices information. |
+| | Create a work queue. | | Create a work queue. |
+| | Call i3c_master_bus_init | | Call i3c_secondary_master_bus_init |
+| | |
+| *i3c_master_bus_init* | *i3c_secondary_master_bus_init* |
+| | Call bus_init to do controller | | Call bus_init to do controller |
+| specific bus initialization and | specific bus initialization and |
+| enabling the controller. | enabling the controller. |
+| | Create I3C device representing a| | Create I3C device representing a |
+| master and add it to the I3C | master and add it to the I3C |
+| device list. | device list. |
+| | Set current master to the device| |
+| created to represent I3C master | | Allocate memory for 'defslvs_data', |
+| device. | that will be used to pass I3C |
+| | Reset all dynamic address that | device list received in DEFSLVS |
+| may have been assigned before. | to I3C core DEFSLVS processing |
+| | Disable all slave events before | | Add I3C device representing this |
+| starting DAA. | master to the system. |
+| | Pre-assign dynamic address and | | Expose our I3C bus as an I2C adapter |
+| retrieve device information if | so that I2C devices are exposed |
+| needed. | through the I2C subsystem. |
+| | Do dynamic address assignment to| |
+| all I3C devices currenly present| | Enable DEFSLVS interrupt. |
+| on the bus. | |
+| | Create I3C devices representing | |
+| those found during DAA. +-------------------------------------------+
+| | Send DEFSVLS message | | *DEFSLVS interrupt* |
+| containing information about all| | Controller driver can chose how to |
+| known I3C and I2C devices. | to handle I2C devices received in |
+| | DEFSLVS e.g. Cadence's controller |
+| | driver ignore I2C devices from |
+| | DEFSLVS and only uses I2C device |
+| | information from DTS. |
+| | | Read all I3C devices information |
+| | from DEFSLVS message in hardware to |
+| | defslvs_data in master object. |
+| | | Call i3c_master_process_defslvs |
+| | |
+| | *i3c_master_process_defslvs* |
+| | | Acquire I3c bus |
+| | |
+| | *i3c_master_acquire_bus* |
+| | Add I3C device representing this | | If device is already holding the |
+| master to the system. | mastership, just broadcast DISEC |
+| | Expose our I3C bus as an I2C | MR, HJ message and return. |
+| adapter so that I2C devices are | | Check if device has got a address |
+| exposed through the I2C subsystem. | by polling with a timeout. |
+| | Register all I3C devices. | |
+| | | Send MR request: Controller driver|
+| | should check if it is already in |
+| | master mode, to handle the case |
+| | of mastership yielded but due to |
+| | poll timeout acquire failed. |
+| | | If not a master, wait until MR |
+| | ENEC is received if currently it |
+| | is disabled. |
+| | Broadcast ENEC MR, HJ message. | | Send MR request. |
++-----------------------------------------+ |
+| | *MR request interrupt* | |
+| | |
+| *i3c_master_yield_bus* | |
+| | Check if this device is still a | |
+| master to handle a case of | |
+| multiple MR requests from different| |
+| devices at a same time. | |
+| | Broadcast DISEC MR, HJ message. | |
+| New master should broadcast ENEC | |
+| MR, HJ once it's usage of bus is | |
+| done. | |
+| | Get accept mastership acknowldege | |
+| from requesting master. | |
+| | Mastership hand over is done. | | Check if device enter master |
+| | In case of failure reenable | mode by polling with a timeout. |
+| MR requests by broadcasting ENEC | |
+| MR, HJ. | Handle I3C device list from DEFSLVS. |
+| | |
+| | *i3c_master_populate_bus* |
+| | | Free up all I3C addresses to handle |
+| | address re assignment by main |
+| | master. |
+| | | Move all devices from I3C list to a |
+| | temporary list. |
+| | | For every device from defslvs_data |
+| | list except the receiving master |
+| | device, retrieve pid and compare it |
+| | with already known I3C devices from |
+| | I3C list. If match is found, |
+| | allocate new address and move the |
+| | device to the original I3C device |
+| | list. If no match is found, it is a |
+| | new device. Register and add it to |
+| | the original I3C list. |
+| | | At the end if temporary list is not |
+| | empty, it contains unplugged I3C |
+| | device. Deregister and delete them. |
+| | |
+| | Broadcast ENEC MR, HJ message. |
++-----------------------------------------+-------------------------------------------+
--
2.17.1
_______________________________________________
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v7 2/7] i3c: master: use i3c_master_register only for main master
2020-05-11 13:11 [PATCH v7 0/7] I3C mastership handover support Parshuram Thombare
2020-05-11 13:12 ` [PATCH v7 1/7] i3c: master: secondary master initialization document Parshuram Thombare
@ 2020-05-11 13:13 ` Parshuram Thombare
2020-05-11 15:44 ` Boris Brezillon
2020-05-11 13:13 ` [PATCH v7 3/7] i3c: master: add i3c_secondary_master_register Parshuram Thombare
` (4 subsequent siblings)
6 siblings, 1 reply; 17+ messages in thread
From: Parshuram Thombare @ 2020-05-11 13:13 UTC (permalink / raw)
To: bbrezillon, vitor.soares
Cc: mparab, Parshuram Thombare, praneeth, linux-kernel, pgaj, linux-i3c
Removed last argument 'secondary' and refactored
i3c_master_register to move code that can be common
to i3c_secondary_master_register to separate function
i3c_master_init.
Signed-off-by: Parshuram Thombare <pthombar@cadence.com>
---
drivers/i3c/master.c | 69 +++++++++++++++++-----------
drivers/i3c/master/dw-i3c-master.c | 2 +-
drivers/i3c/master/i3c-master-cdns.c | 2 +-
include/linux/i3c/master.h | 3 +-
4 files changed, 46 insertions(+), 30 deletions(-)
diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
index 5f4bd52121fe..ba07a7d49633 100644
--- a/drivers/i3c/master.c
+++ b/drivers/i3c/master.c
@@ -2391,31 +2391,10 @@ static int i3c_master_check_ops(const struct i3c_master_controller_ops *ops)
return 0;
}
-/**
- * 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
- *
- * This function takes care of everything for you:
- *
- * - creates and initializes the I3C bus
- * - populates the bus with static I2C devs if @parent->of_node is not
- * NULL
- * - registers all I3C devices added by the controller during bus
- * initialization
- * - registers the I2C adapter and all I2C devices
- *
- * Return: 0 in case of success, a negative error code otherwise.
- */
-int i3c_master_register(struct i3c_master_controller *master,
- struct device *parent,
- const struct i3c_master_controller_ops *ops,
- bool secondary)
+static int i3c_master_init(struct i3c_master_controller *master,
+ struct device *parent,
+ const struct i3c_master_controller_ops *ops,
+ bool secondary)
{
struct i3c_bus *i3cbus = i3c_master_get_bus(master);
enum i3c_bus_mode mode = I3C_BUS_MODE_PURE;
@@ -2482,6 +2461,45 @@ int i3c_master_register(struct i3c_master_controller *master,
if (ret)
goto err_put_dev;
+ return 0;
+
+err_put_dev:
+ put_device(&master->dev);
+
+ return ret;
+}
+
+/**
+ * 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
+ *
+ * This function takes care of everything for you:
+ *
+ * - creates and initializes the I3C bus
+ * - populates the bus with static I2C devs if @parent->of_node is not
+ * NULL
+ * - registers all I3C devices added by the controller during bus
+ * initialization
+ * - registers the I2C adapter and all I2C devices
+ *
+ * Return: 0 in case of success, a negative error code otherwise.
+ */
+int i3c_master_register(struct i3c_master_controller *master,
+ struct device *parent,
+ const struct i3c_master_controller_ops *ops)
+{
+ int ret;
+
+ ret = i3c_master_init(master, parent, ops, false);
+ if (ret)
+ return ret;
+
ret = device_add(&master->dev);
if (ret)
goto err_cleanup_bus;
@@ -2511,7 +2529,6 @@ int i3c_master_register(struct i3c_master_controller *master,
err_cleanup_bus:
i3c_master_bus_cleanup(master);
-err_put_dev:
put_device(&master->dev);
return ret;
diff --git a/drivers/i3c/master/dw-i3c-master.c b/drivers/i3c/master/dw-i3c-master.c
index 1d83c97431c7..5d5a8a90ec06 100644
--- a/drivers/i3c/master/dw-i3c-master.c
+++ b/drivers/i3c/master/dw-i3c-master.c
@@ -1158,7 +1158,7 @@ static int dw_i3c_probe(struct platform_device *pdev)
master->free_pos = GENMASK(master->maxdevs - 1, 0);
ret = i3c_master_register(&master->base, &pdev->dev,
- &dw_mipi_i3c_ops, false);
+ &dw_mipi_i3c_ops);
if (ret)
goto err_assert_rst;
diff --git a/drivers/i3c/master/i3c-master-cdns.c b/drivers/i3c/master/i3c-master-cdns.c
index 8889a4fdb454..ed4f43807f9e 100644
--- a/drivers/i3c/master/i3c-master-cdns.c
+++ b/drivers/i3c/master/i3c-master-cdns.c
@@ -1615,7 +1615,7 @@ static int cdns_i3c_master_probe(struct platform_device *pdev)
writel(DEVS_CTRL_DEV_CLR_ALL, master->regs + DEVS_CTRL);
ret = i3c_master_register(&master->base, &pdev->dev,
- &cdns_i3c_master_ops, false);
+ &cdns_i3c_master_ops);
if (ret)
goto err_disable_sysclk;
diff --git a/include/linux/i3c/master.h b/include/linux/i3c/master.h
index f13fd8b1dd79..f5ba82c390bc 100644
--- a/include/linux/i3c/master.h
+++ b/include/linux/i3c/master.h
@@ -533,8 +533,7 @@ int i3c_master_set_info(struct i3c_master_controller *master,
int i3c_master_register(struct i3c_master_controller *master,
struct device *parent,
- const struct i3c_master_controller_ops *ops,
- bool secondary);
+ const struct i3c_master_controller_ops *ops);
int i3c_master_unregister(struct i3c_master_controller *master);
/**
--
2.17.1
_______________________________________________
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v7 3/7] i3c: master: add i3c_secondary_master_register
2020-05-11 13:11 [PATCH v7 0/7] I3C mastership handover support Parshuram Thombare
2020-05-11 13:12 ` [PATCH v7 1/7] i3c: master: secondary master initialization document Parshuram Thombare
2020-05-11 13:13 ` [PATCH v7 2/7] i3c: master: use i3c_master_register only for main master Parshuram Thombare
@ 2020-05-11 13:13 ` Parshuram Thombare
2020-05-11 16:14 ` Boris Brezillon
2020-05-11 13:14 ` [PATCH v7 4/7] i3c: master: add mastership handover support Parshuram Thombare
` (3 subsequent siblings)
6 siblings, 1 reply; 17+ messages in thread
From: Parshuram Thombare @ 2020-05-11 13:13 UTC (permalink / raw)
To: bbrezillon, vitor.soares
Cc: mparab, Parshuram Thombare, praneeth, linux-kernel, pgaj, linux-i3c
add i3c_secondary_master_register which is used
to register secondary masters.
Signed-off-by: Parshuram Thombare <pthombar@cadence.com>
---
drivers/i3c/master.c | 154 ++++++++++++++++++++++++++++++++++++-
include/linux/i3c/master.h | 3 +
2 files changed, 156 insertions(+), 1 deletion(-)
diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
index ba07a7d49633..669bd7e45810 100644
--- a/drivers/i3c/master.c
+++ b/drivers/i3c/master.c
@@ -1768,6 +1768,90 @@ static int i3c_master_bus_init(struct i3c_master_controller *master)
return ret;
}
+/**
+ * i3c_secondary_master_bus_init() - initialize an I3C bus for secondary
+ * master
+ * @master: secondary master initializing the bus
+ *
+ * This function does
+ *
+ * 1. Attach I2C devs to the master
+ *
+ * 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)
+ *
+ * Once this is done, I2C devices should be usable.
+ *
+ * Return: a 0 in case of success, an negative error code otherwise.
+ */
+static int i3c_secondary_master_bus_init(struct i3c_master_controller *master)
+{
+ enum i3c_addr_slot_status status;
+ struct i2c_dev_boardinfo *i2cboardinfo;
+ 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;
+ }
+
+ 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;
+ }
+
+ ret = i3c_master_attach_i2c_dev(master, i2cdev);
+ if (ret) {
+ i3c_master_free_i2c_dev(i2cdev);
+ goto err_detach_devs;
+ }
+ }
+
+ /*
+ * Now execute the controller specific ->bus_init() routine, which
+ * might configure its internal logic to match the bus limitations.
+ */
+ ret = master->ops->bus_init(master);
+ if (ret)
+ goto err_detach_devs;
+
+ /*
+ * The master device should have been instantiated in ->bus_init(),
+ * complain if this was not the case.
+ */
+ if (!master->this) {
+ dev_err(&master->dev,
+ "master_set_info() was not called in ->bus_init()\n");
+ ret = -EINVAL;
+ goto err_bus_cleanup;
+ }
+
+ return 0;
+
+err_bus_cleanup:
+ if (master->ops->bus_cleanup)
+ master->ops->bus_cleanup(master);
+
+err_detach_devs:
+ i3c_master_detach_free_devs(master);
+
+ return ret;
+}
+
static void i3c_master_bus_cleanup(struct i3c_master_controller *master)
{
if (master->ops->bus_cleanup)
@@ -2457,7 +2541,10 @@ static int i3c_master_init(struct i3c_master_controller *master,
goto err_put_dev;
}
- ret = i3c_master_bus_init(master);
+ if (secondary)
+ ret = i3c_secondary_master_bus_init(master);
+ else
+ ret = i3c_master_bus_init(master);
if (ret)
goto err_put_dev;
@@ -2535,6 +2622,71 @@ int i3c_master_register(struct i3c_master_controller *master,
}
EXPORT_SYMBOL_GPL(i3c_master_register);
+/**
+ * i3c_secondary_master_register() - register an I3C secondary 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
+ *
+ * This function does minimal required initialization for secondary
+ * master, rest functionality like creating and registering I2C
+ * and I3C devices is done in defslvs processing.
+ *
+ * i3c_secondary_master_register() does following things -
+ * - creates and initializes the I3C bus
+ * - populates the bus with static I2C devs if @parent->of_node is not
+ * NULL
+ * initialization
+ * - allocate memory for defslvs_data.devs, which is used to receive
+ * defslvs list
+ * - create I3C device representing this master
+ * - registers the I2C adapter and all I2C devices
+ *
+ * Return: 0 in case of success, a negative error code otherwise.
+ */
+int i3c_secondary_master_register(struct i3c_master_controller *master,
+ struct device *parent,
+ const struct i3c_master_controller_ops *ops)
+{
+ int ret;
+
+ ret = i3c_master_init(master, parent, ops, true);
+ if (ret)
+ return ret;
+
+ ret = device_add(&master->dev);
+ if (ret)
+ goto err_cleanup_bus;
+
+ /*
+ * Expose our I3C bus as an I2C adapter so that I2C devices are exposed
+ * through the I2C subsystem.
+ */
+ ret = i3c_master_i2c_adapter_init(master);
+ if (ret)
+ goto err_del_dev;
+
+ /*
+ * We're done initializing the bus and the controller, we can now
+ * register I3C devices from defslvs list.
+ */
+ master->init_done = true;
+
+ return 0;
+
+err_del_dev:
+ device_del(&master->dev);
+
+err_cleanup_bus:
+ i3c_master_bus_cleanup(master);
+
+ put_device(&master->dev);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(i3c_secondary_master_register);
+
/**
* i3c_master_unregister() - unregister an I3C master
* @master: master used to send frames on the bus
diff --git a/include/linux/i3c/master.h b/include/linux/i3c/master.h
index f5ba82c390bc..5124ff4831eb 100644
--- a/include/linux/i3c/master.h
+++ b/include/linux/i3c/master.h
@@ -534,6 +534,9 @@ int i3c_master_set_info(struct i3c_master_controller *master,
int i3c_master_register(struct i3c_master_controller *master,
struct device *parent,
const struct i3c_master_controller_ops *ops);
+int i3c_secondary_master_register(struct i3c_master_controller *master,
+ struct device *parent,
+ const struct i3c_master_controller_ops *ops);
int i3c_master_unregister(struct i3c_master_controller *master);
/**
--
2.17.1
_______________________________________________
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v7 4/7] i3c: master: add mastership handover support
2020-05-11 13:11 [PATCH v7 0/7] I3C mastership handover support Parshuram Thombare
` (2 preceding siblings ...)
2020-05-11 13:13 ` [PATCH v7 3/7] i3c: master: add i3c_secondary_master_register Parshuram Thombare
@ 2020-05-11 13:14 ` Parshuram Thombare
2020-05-11 16:38 ` Boris Brezillon
2020-05-11 13:15 ` [PATCH v7 5/7] i3c: master: add defslvs processing Parshuram Thombare
` (2 subsequent siblings)
6 siblings, 1 reply; 17+ messages in thread
From: Parshuram Thombare @ 2020-05-11 13:14 UTC (permalink / raw)
To: bbrezillon, vitor.soares
Cc: mparab, Parshuram Thombare, praneeth, linux-kernel, pgaj, linux-i3c
Added mastership acquire and yield functions.
Signed-off-by: Parshuram Thombare <pthombar@cadence.com>
---
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 <linux/slab.h>
#include <linux/spinlock.h>
#include <linux/workqueue.h>
+#include <linux/iopoll.h>
#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(&master->bus);
+ ret = i3c_master_enec_locked(master, I3C_BROADCAST_ADDR,
+ I3C_CCC_EVENT_MR | I3C_CCC_EVENT_HJ);
+ i3c_bus_maintenance_unlock(&master->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(&m->bus);
+ i3c_bus_maintenance_lock(&m->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, 1000000);
+ 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, 1000000);
+ if (!ret)
+ m->bus.cur_master = m->this;
+
+mr_req_done:
+ i3c_bus_maintenance_unlock(&m->bus);
+ i3c_bus_normaluse_lock(&m->bus);
+ 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(&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;
+}
+
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(&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;
@@ -1570,7 +1667,9 @@ 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->secondary)
+ master->bus.cur_master = master->this;
ret = i3c_master_attach_i3c_dev(master, i3cdev);
if (ret)
@@ -1612,6 +1711,73 @@ static void i3c_master_detach_free_devs(struct i3c_master_controller *master)
}
}
+/**
+ * i3c_master_yield_bus() - yield I3C bus mastership
+ * @m: I3C master object
+ * @sec_mst_dyn_addr: address of device requesting mastership
+ *
+ * This function may sleep.
+ * It is expected to be called with normaluse_lock.
+ */
+void
+i3c_master_yield_bus(struct i3c_master_controller *m, u8 sec_mst_dyn_addr)
+{
+ struct i3c_dev_desc *i3cdev;
+ int ret = 0;
+
+ i3c_bus_maintenance_lock(&m->bus);
+ if (m->this != m->bus.cur_master)
+ goto mr_yield_done;
+
+ /*
+ * Maintenance lock and master check above is used to
+ * avoid race amongst devices sending MR requests
+ * at the same time, as soon as ENEC MST is sent by the current
+ * master. It ensure that only one MR request is processed,
+ * rest MR requests on losing devices will timeout in wait MR
+ * DONE state. And next MR requests are blocked due to DISEC MST
+ * sent by current master in yield operation.
+ * New master should send ENEC MST once it's work is done.
+ * maintainanace lock is also needed for i3c_master_get_accmst_locked.
+ */
+
+ ret = i3c_master_disec_locked(m, I3C_BROADCAST_ADDR,
+ I3C_CCC_EVENT_MR |
+ I3C_CCC_EVENT_HJ);
+ /*
+ * Once mastership is given to the new master, it is expected that
+ * MR is disabled prior to that and new master is responsible to
+ * enable it by broadcasting ENEC MR when it's work is done.
+ * If DISEC MR fails and we still go ahead with handover, chances
+ * are new master will get interrupted by unexpected MR requests.
+ */
+ if (ret)
+ goto mr_yield_done;
+
+ ret = i3c_master_get_accmst_locked(m, sec_mst_dyn_addr);
+ if (ret)
+ goto mr_yield_done;
+
+ i3c_bus_for_each_i3cdev(&m->bus, i3cdev) {
+ if (sec_mst_dyn_addr == i3cdev->info.dyn_addr) {
+ m->bus.cur_master = i3cdev;
+ break;
+ }
+ }
+
+ /* Requesting device not found on i3c list. This should never happen. */
+ if (m->this == m->bus.cur_master) {
+ ret = -EAGAIN;
+ WARN_ON(1);
+ }
+
+mr_yield_done:
+ i3c_bus_maintenance_unlock(&m->bus);
+ if (ret)
+ i3c_master_enable_mr_events(m);
+}
+EXPORT_SYMBOL_GPL(i3c_master_yield_bus);
+
/**
* i3c_master_bus_init() - initialize an I3C bus
* @master: main master initializing the bus
@@ -2472,6 +2638,10 @@ 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->check_event_set))
+ return -EINVAL;
+
return 0;
}
@@ -2485,10 +2655,6 @@ static int i3c_master_init(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;
@@ -2608,6 +2774,9 @@ int i3c_master_register(struct i3c_master_controller *master,
i3c_master_register_new_i3c_devs(master);
i3c_bus_normaluse_unlock(&master->bus);
+ if (ops->request_mastership)
+ ret = i3c_master_enable_mr_events(master);
+
return 0;
err_del_dev:
diff --git a/include/linux/i3c/master.h b/include/linux/i3c/master.h
index 5124ff4831eb..dd67497ad8b1 100644
--- a/include/linux/i3c/master.h
+++ b/include/linux/i3c/master.h
@@ -259,6 +259,16 @@ enum i3c_bus_mode {
I3C_BUS_MODE_MIXED_SLOW,
};
+/**
+ * enum i3c_event - I3C master (currently acting as a slave) controller events
+ * @I3C_SLV_DA_UPDATE: I3C master has dynamic address
+ * @I3C_SLV_MR_DONE: I3C mastership request completed successfully
+ */
+enum i3c_event {
+ I3C_SLV_DA_UPDATE,
+ I3C_SLV_MR_DONE,
+};
+
/**
* enum i3c_addr_slot_status - I3C address slot status
* @I3C_ADDR_SLOT_FREE: address is free
@@ -418,6 +428,12 @@ struct i3c_bus {
* for a future IBI
* This method is mandatory only if ->request_ibi is not
* NULL.
+ * @request_mastership: send mastership request to the current master
+ * @enable_mr_events: enable mastership request handling by the controller
+ * @check_event_set: check events (enum i3c_event) such as device has dynamic
+ * address, mastership request is completed successfully.
+ * @sec_mst_dyn_addr: read current dynamic address of the I3C device from
+ * hardware.
*/
struct i3c_master_controller_ops {
int (*bus_init)(struct i3c_master_controller *master);
@@ -445,6 +461,11 @@ 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);
+ int (*request_mastership)(struct i3c_master_controller *master);
+ void (*enable_mr_events)(struct i3c_master_controller *m);
+ bool (*check_event_set)(struct i3c_master_controller *m,
+ enum i3c_event);
+ int (*sec_mst_dyn_addr)(struct i3c_master_controller *m);
};
/**
@@ -510,6 +531,8 @@ struct i3c_master_controller {
#define i3c_bus_for_each_i3cdev(bus, dev) \
list_for_each_entry(dev, &(bus)->devs.i3c, common.node)
+void i3c_master_yield_bus(struct i3c_master_controller *m,
+ u8 sec_mst_dyn_addr);
int i3c_master_do_i2c_xfers(struct i3c_master_controller *master,
const struct i2c_msg *xfers,
int nxfers);
--
2.17.1
_______________________________________________
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v7 5/7] i3c: master: add defslvs processing
2020-05-11 13:11 [PATCH v7 0/7] I3C mastership handover support Parshuram Thombare
` (3 preceding siblings ...)
2020-05-11 13:14 ` [PATCH v7 4/7] i3c: master: add mastership handover support Parshuram Thombare
@ 2020-05-11 13:15 ` Parshuram Thombare
2020-05-11 13:16 ` [PATCH v7 6/7] i3c: master: sysfs key for acquire bus Parshuram Thombare
2020-05-11 13:17 ` [PATCH v7 7/7] i3c: master: mastership handover, defslvs processing in cdns controller driver Parshuram Thombare
6 siblings, 0 replies; 17+ messages in thread
From: Parshuram Thombare @ 2020-05-11 13:15 UTC (permalink / raw)
To: bbrezillon, vitor.soares
Cc: mparab, Parshuram Thombare, praneeth, linux-kernel, pgaj, linux-i3c
Added defslvs processing code to the I3C master subsystem.
Signed-off-by: Parshuram Thombare <pthombar@cadence.com>
---
drivers/i3c/master.c | 142 ++++++++++++++++++++++++++++++++++++-
include/linux/i3c/master.h | 7 ++
2 files changed, 147 insertions(+), 2 deletions(-)
diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
index 9c8250a6a2b0..ea53fadeed99 100644
--- a/drivers/i3c/master.c
+++ b/drivers/i3c/master.c
@@ -2639,7 +2639,8 @@ static int i3c_master_check_ops(const struct i3c_master_controller_ops *ops)
return -EINVAL;
if (ops->request_mastership &&
- (!ops->enable_mr_events || !ops->check_event_set))
+ (!ops->enable_mr_events || !ops->check_event_set ||
+ !ops->sec_mst_dyn_addr))
return -EINVAL;
return 0;
@@ -2818,12 +2819,20 @@ int i3c_secondary_master_register(struct i3c_master_controller *master,
struct device *parent,
const struct i3c_master_controller_ops *ops)
{
- int ret;
+ int ret, sz;
ret = i3c_master_init(master, parent, ops, true);
if (ret)
return ret;
+ sz = sizeof(struct i3c_ccc_dev_desc) * I3C_BUS_MAX_DEVS;
+ master->defslvs_data.devs = devm_kzalloc(&master->dev, sz,
+ GFP_KERNEL);
+ if (!master->defslvs_data.devs) {
+ ret = -ENOMEM;
+ goto err_cleanup_bus;
+ }
+
ret = device_add(&master->dev);
if (ret)
goto err_cleanup_bus;
@@ -2856,6 +2865,135 @@ int i3c_secondary_master_register(struct i3c_master_controller *master,
}
EXPORT_SYMBOL_GPL(i3c_secondary_master_register);
+static int i3c_master_populate_bus(struct i3c_master_controller *master)
+{
+ struct i3c_dev_desc *i3cdev, *olddev, *tmp;
+ struct i3c_ccc_dev_desc *desc;
+ struct list_head i3c_old;
+ struct i3c_bus *i3cbus;
+ int slot, dyn_addr, ret;
+
+ i3cbus = i3c_master_get_bus(master);
+
+ INIT_LIST_HEAD(&i3c_old);
+ list_for_each_entry_safe(olddev, tmp, &i3cbus->devs.i3c, common.node) {
+ i3c_master_put_i3c_addrs(olddev);
+ list_del(&olddev->common.node);
+ list_add(&olddev->common.node, &i3c_old);
+ }
+
+ dyn_addr = master->ops->sec_mst_dyn_addr(master);
+ master->this->info.dyn_addr = dyn_addr;
+ i3c_master_get_i3c_addrs(master->this);
+ list_del(&master->this->common.node);
+ list_add(&master->this->common.node, &i3cbus->devs.i3c);
+
+ desc = master->defslvs_data.devs;
+ for (slot = 0; slot < master->defslvs_data.ndevs; slot++, desc++) {
+ struct i3c_device_info info = {
+ .dyn_addr = desc->dyn_addr
+ };
+
+ if (dyn_addr == info.dyn_addr)
+ continue;
+
+ i3cdev = i3c_master_alloc_i3c_dev(master, &info);
+ if (IS_ERR(i3cdev)) {
+ ret = PTR_ERR(i3cdev);
+ goto populate_bus_fail;
+ }
+
+ i3c_master_get_i3c_addrs(i3cdev);
+ ret = i3c_master_retrieve_dev_info(i3cdev);
+ i3c_master_put_i3c_addrs(i3cdev);
+ if (ret) {
+ i3c_master_free_i3c_dev(i3cdev);
+ goto populate_bus_fail;
+ }
+
+ list_for_each_entry_safe(olddev, tmp, &i3c_old, common.node) {
+ if (olddev->info.pid == i3cdev->info.pid) {
+ olddev->info.dyn_addr = info.dyn_addr;
+ i3c_master_get_i3c_addrs(olddev);
+ list_del(&olddev->common.node);
+ list_add(&olddev->common.node,
+ &i3cbus->devs.i3c);
+ i3c_master_free_i3c_dev(i3cdev);
+ i3cdev = NULL;
+ break;
+ }
+ }
+
+ if (i3cdev) {
+ ret = i3c_master_attach_i3c_dev(master, i3cdev);
+ if (ret) {
+ i3c_master_free_i3c_dev(i3cdev);
+ goto populate_bus_fail;
+ }
+ }
+ }
+
+ list_for_each_entry_safe(olddev, tmp, &i3c_old, common.node) {
+ if (olddev->dev) {
+ olddev->dev->desc = NULL;
+ if (device_is_registered(&olddev->dev->dev))
+ device_unregister(&olddev->dev->dev);
+ else
+ put_device(&olddev->dev->dev);
+ kfree(olddev->dev);
+ }
+ list_del(&olddev->common.node);
+ i3c_master_free_i3c_dev(olddev);
+ }
+
+ return 0;
+
+populate_bus_fail:
+ /*
+ * Try to restore i3cbus->devs.i3c list, so far no i3c
+ * device is deleted, only moved or added to the original
+ * i3c list. Move rest of the i3c devices from old list,
+ * to correctly process defslvs in rety.
+ */
+ list_for_each_entry_safe(olddev, tmp, &i3c_old, common.node) {
+ list_del(&olddev->common.node);
+ list_add(&olddev->common.node, &i3cbus->devs.i3c);
+ }
+
+ return ret;
+}
+
+/**
+ * i3c_master_process_defslvs() - process I3C device list received in
+ * DEFSLVS for device plug/unplug and address change.
+ * @m: I3C master object
+ *
+ * This function may sleep, so should not be called in the atomic context.
+ */
+int i3c_master_process_defslvs(struct i3c_master_controller *m)
+{
+ int ret;
+
+ i3c_bus_normaluse_lock(&m->bus);
+ ret = i3c_master_acquire_bus(m);
+ i3c_bus_normaluse_unlock(&m->bus);
+ if (ret)
+ return ret;
+
+ i3c_bus_maintenance_lock(&m->bus);
+ ret = i3c_master_populate_bus(m);
+ i3c_bus_maintenance_unlock(&m->bus);
+ if (!ret) {
+ i3c_bus_normaluse_lock(&m->bus);
+ i3c_master_register_new_i3c_devs(m);
+ i3c_bus_normaluse_unlock(&m->bus);
+ }
+ i3c_master_enable_mr_events(m);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(i3c_master_process_defslvs);
+
/**
* i3c_master_unregister() - unregister an I3C master
* @master: master used to send frames on the bus
diff --git a/include/linux/i3c/master.h b/include/linux/i3c/master.h
index dd67497ad8b1..688487c4a62a 100644
--- a/include/linux/i3c/master.h
+++ b/include/linux/i3c/master.h
@@ -488,6 +488,8 @@ 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
+ * @defslvs_data: list used to pass i3c device list received in DEFSLVS message,
+ * from DEFSLVS controller driver to I3C core
*
* A &struct i3c_master_controller has to be registered to the I3C subsystem
* through i3c_master_register(). None of &struct i3c_master_controller fields
@@ -507,6 +509,10 @@ struct i3c_master_controller {
} boardinfo;
struct i3c_bus bus;
struct workqueue_struct *wq;
+ struct {
+ u32 ndevs;
+ struct i3c_ccc_dev_desc *devs;
+ } defslvs_data;
};
/**
@@ -533,6 +539,7 @@ struct i3c_master_controller {
void i3c_master_yield_bus(struct i3c_master_controller *m,
u8 sec_mst_dyn_addr);
+int i3c_master_process_defslvs(struct i3c_master_controller *m);
int i3c_master_do_i2c_xfers(struct i3c_master_controller *master,
const struct i2c_msg *xfers,
int nxfers);
--
2.17.1
_______________________________________________
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v7 6/7] i3c: master: sysfs key for acquire bus
2020-05-11 13:11 [PATCH v7 0/7] I3C mastership handover support Parshuram Thombare
` (4 preceding siblings ...)
2020-05-11 13:15 ` [PATCH v7 5/7] i3c: master: add defslvs processing Parshuram Thombare
@ 2020-05-11 13:16 ` Parshuram Thombare
2020-05-11 13:17 ` [PATCH v7 7/7] i3c: master: mastership handover, defslvs processing in cdns controller driver Parshuram Thombare
6 siblings, 0 replies; 17+ messages in thread
From: Parshuram Thombare @ 2020-05-11 13:16 UTC (permalink / raw)
To: bbrezillon, vitor.soares
Cc: mparab, Parshuram Thombare, praneeth, linux-kernel, pgaj, linux-i3c
Added support to acquire I3C bus through sysfs interface.
Signed-off-by: Parshuram Thombare <pthombar@cadence.com>
---
drivers/i3c/master.c | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)
diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
index ea53fadeed99..ec27196e987c 100644
--- a/drivers/i3c/master.c
+++ b/drivers/i3c/master.c
@@ -607,6 +607,23 @@ static ssize_t i2c_scl_frequency_show(struct device *dev,
}
static DEVICE_ATTR_RO(i2c_scl_frequency);
+static ssize_t i3c_acquire_bus_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct i3c_master_controller *master = dev_to_i3cmaster(dev);
+ int ret;
+
+ i3c_bus_normaluse_lock(&master->bus);
+ ret = i3c_master_acquire_bus(master);
+ i3c_bus_normaluse_unlock(&master->bus);
+ if (!ret)
+ i3c_master_enable_mr_events(master);
+
+ return ret ?: count;
+}
+static DEVICE_ATTR_WO(i3c_acquire_bus);
+
static struct attribute *i3c_masterdev_attrs[] = {
&dev_attr_mode.attr,
&dev_attr_current_master.attr,
@@ -617,6 +634,7 @@ static struct attribute *i3c_masterdev_attrs[] = {
&dev_attr_pid.attr,
&dev_attr_dynamic_address.attr,
&dev_attr_hdrcap.attr,
+ &dev_attr_i3c_acquire_bus.attr,
NULL,
};
ATTRIBUTE_GROUPS(i3c_masterdev);
--
2.17.1
_______________________________________________
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v7 7/7] i3c: master: mastership handover, defslvs processing in cdns controller driver
2020-05-11 13:11 [PATCH v7 0/7] I3C mastership handover support Parshuram Thombare
` (5 preceding siblings ...)
2020-05-11 13:16 ` [PATCH v7 6/7] i3c: master: sysfs key for acquire bus Parshuram Thombare
@ 2020-05-11 13:17 ` Parshuram Thombare
6 siblings, 0 replies; 17+ messages in thread
From: Parshuram Thombare @ 2020-05-11 13:17 UTC (permalink / raw)
To: bbrezillon, vitor.soares
Cc: mparab, Parshuram Thombare, praneeth, linux-kernel, pgaj, linux-i3c
Added I3C bus mastership handover and DEFSLVS message handling
code to Cadence's I3C master controller driver.
Signed-off-by: Parshuram Thombare <pthombar@cadence.com>
---
drivers/i3c/master/i3c-master-cdns.c | 377 +++++++++++++++++++++++++--
1 file changed, 354 insertions(+), 23 deletions(-)
diff --git a/drivers/i3c/master/i3c-master-cdns.c b/drivers/i3c/master/i3c-master-cdns.c
index ed4f43807f9e..1155aa327404 100644
--- a/drivers/i3c/master/i3c-master-cdns.c
+++ b/drivers/i3c/master/i3c-master-cdns.c
@@ -157,6 +157,7 @@
#define SLV_IMR 0x48
#define SLV_ICR 0x4c
#define SLV_ISR 0x50
+#define SLV_INT_DEFSLVS BIT(21)
#define SLV_INT_TM BIT(20)
#define SLV_INT_ERROR BIT(19)
#define SLV_INT_EVENT_UP BIT(18)
@@ -189,7 +190,7 @@
#define SLV_STATUS1_HJ_DIS BIT(18)
#define SLV_STATUS1_MR_DIS BIT(17)
#define SLV_STATUS1_PROT_ERR BIT(16)
-#define SLV_STATUS1_DA(x) (((s) & GENMASK(15, 9)) >> 9)
+#define SLV_STATUS1_DA(s) (((s) & GENMASK(15, 9)) >> 9)
#define SLV_STATUS1_HAS_DA BIT(8)
#define SLV_STATUS1_DDR_RX_FULL BIT(7)
#define SLV_STATUS1_DDR_TX_FULL BIT(6)
@@ -390,6 +391,8 @@ struct cdns_i3c_xfer {
struct cdns_i3c_master {
struct work_struct hj_work;
+ struct work_struct mr_yield_work;
+ struct work_struct defslvs_work;
struct i3c_master_controller base;
u32 free_rr_slots;
unsigned int maxdevs;
@@ -408,6 +411,7 @@ struct cdns_i3c_master {
struct clk *pclk;
struct cdns_i3c_master_caps caps;
unsigned long i3c_scl_lim;
+ u8 mr_addr;
};
static inline struct cdns_i3c_master *
@@ -1187,10 +1191,6 @@ static int cdns_i3c_master_do_daa(struct i3c_master_controller *m)
cdns_i3c_master_upd_i3c_scl_lim(master);
- /* Unmask Hot-Join and Mastership request interrupts. */
- i3c_master_enec_locked(m, I3C_BROADCAST_ADDR,
- I3C_CCC_EVENT_HJ | I3C_CCC_EVENT_MR);
-
return 0;
}
@@ -1199,21 +1199,21 @@ static int cdns_i3c_master_bus_init(struct i3c_master_controller *m)
struct cdns_i3c_master *master = to_cdns_i3c_master(m);
unsigned long pres_step, sysclk_rate, max_i2cfreq;
struct i3c_bus *bus = i3c_master_get_bus(m);
- u32 ctrl, prescl0, prescl1, pres, low;
+ u32 ctrl, prescl0, prescl1, pres, low, bus_mode;
struct i3c_device_info info = { };
int ret, ncycles;
switch (bus->mode) {
case I3C_BUS_MODE_PURE:
- ctrl = CTRL_PURE_BUS_MODE;
+ bus_mode = CTRL_PURE_BUS_MODE;
break;
case I3C_BUS_MODE_MIXED_FAST:
- ctrl = CTRL_MIXED_FAST_BUS_MODE;
+ bus_mode = CTRL_MIXED_FAST_BUS_MODE;
break;
case I3C_BUS_MODE_MIXED_SLOW:
- ctrl = CTRL_MIXED_SLOW_BUS_MODE;
+ bus_mode = CTRL_MIXED_SLOW_BUS_MODE;
break;
default:
@@ -1244,7 +1244,6 @@ static int cdns_i3c_master_bus_init(struct i3c_master_controller *m)
bus->scl_rate.i2c = sysclk_rate / ((pres + 1) * 5);
prescl0 |= PRESCL_CTRL0_I2C(pres);
- writel(prescl0, master->regs + PRESCL_CTRL0);
/* Calculate OD and PP low. */
pres_step = 1000000000 / (bus->scl_rate.i3c * 4);
@@ -1252,7 +1251,6 @@ static int cdns_i3c_master_bus_init(struct i3c_master_controller *m)
if (ncycles < 0)
ncycles = 0;
prescl1 = PRESCL_CTRL1_OD_LOW(ncycles);
- writel(prescl1, master->regs + PRESCL_CTRL1);
/* Get an address for the master. */
ret = i3c_master_get_free_addr(m, 0);
@@ -1270,13 +1268,21 @@ static int cdns_i3c_master_bus_init(struct i3c_master_controller *m)
if (ret)
return ret;
+ ctrl = readl(master->regs + CTRL);
+ if (ctrl & CTRL_DEV_EN)
+ cdns_i3c_master_disable(master);
+ writel(prescl0, master->regs + PRESCL_CTRL0);
+ writel(prescl1, master->regs + PRESCL_CTRL1);
+ ctrl &= ~CTRL_BUS_MODE_MASK;
+ ctrl |= bus_mode | CTRL_HALT_EN | CTRL_MCS_EN;
/*
* Enable Hot-Join, and, when a Hot-Join request happens, disable all
* events coming from this device.
*
* We will issue ENTDAA afterwards from the threaded IRQ handler.
*/
- ctrl |= CTRL_HJ_ACK | CTRL_HJ_DISEC | CTRL_HALT_EN | CTRL_MCS_EN;
+ if (!m->secondary)
+ ctrl |= CTRL_HJ_ACK | CTRL_HJ_DISEC;
writel(ctrl, master->regs + CTRL);
cdns_i3c_master_enable(master);
@@ -1340,6 +1346,7 @@ static void cdns_i3c_master_handle_ibi(struct cdns_i3c_master *master,
static void cnds_i3c_master_demux_ibis(struct cdns_i3c_master *master)
{
+ struct i3c_dev_desc *dev;
u32 status0;
writel(MST_INT_IBIR_THR, master->regs + MST_ICR);
@@ -1361,6 +1368,14 @@ static void cnds_i3c_master_demux_ibis(struct cdns_i3c_master *master)
case IBIR_TYPE_MR:
WARN_ON(IBIR_XFER_BYTES(ibir) || (ibir & IBIR_ERROR));
+ if (ibir & IBIR_ACKED) {
+ dev = master->ibi.slots[IBIR_SLVID(ibir)];
+ master->mr_addr = dev->info.dyn_addr;
+ queue_work(master->base.wq,
+ &master->mr_yield_work);
+ }
+ break;
+
default:
break;
}
@@ -1372,16 +1387,40 @@ static irqreturn_t cdns_i3c_master_interrupt(int irq, void *data)
struct cdns_i3c_master *master = data;
u32 status;
- status = readl(master->regs + MST_ISR);
- if (!(status & readl(master->regs + MST_IMR)))
- return IRQ_NONE;
+ if (master->base.this &&
+ master->base.this == master->base.bus.cur_master) {
+ status = readl(master->regs + MST_ISR);
+ if (!(status & readl(master->regs + MST_IMR)))
+ return IRQ_NONE;
+
+ spin_lock(&master->xferqueue.lock);
+ cdns_i3c_master_end_xfer_locked(master, status);
+ spin_unlock(&master->xferqueue.lock);
+
+ if (status & MST_INT_IBIR_THR)
+ cnds_i3c_master_demux_ibis(master);
+
+ if (status & MST_INT_MR_DONE) {
+ writel(FLUSH_RX_FIFO | FLUSH_TX_FIFO,
+ master->regs + FLUSH_CTRL);
+ writel(MST_INT_MR_DONE, master->regs + MST_ICR);
+ }
+ } else {
+ status = (readl(master->regs + SLV_ISR) &
+ readl(master->regs + SLV_IMR));
+
+ if (!status)
+ return IRQ_NONE;
+
+ if (status & SLV_INT_MR_DONE)
+ writel(FLUSH_RX_FIFO | FLUSH_TX_FIFO,
+ master->regs + FLUSH_CTRL);
- spin_lock(&master->xferqueue.lock);
- cdns_i3c_master_end_xfer_locked(master, status);
- spin_unlock(&master->xferqueue.lock);
+ if (status & SLV_INT_DEFSLVS)
+ queue_work(master->base.wq, &master->defslvs_work);
- if (status & MST_INT_IBIR_THR)
- cnds_i3c_master_demux_ibis(master);
+ writel(status, master->regs + SLV_ICR);
+ }
return IRQ_HANDLED;
}
@@ -1505,6 +1544,138 @@ static void cdns_i3c_master_recycle_ibi_slot(struct i3c_dev_desc *dev,
i3c_generic_ibi_recycle_slot(data->ibi_pool, slot);
}
+static int cdns_i3c_master_find_ibi_slot(struct cdns_i3c_master *master,
+ struct i3c_dev_desc *dev,
+ s16 *slot)
+{
+ unsigned long flags;
+ unsigned int i;
+ int ret = -ENOENT;
+
+ spin_lock_irqsave(&master->ibi.lock, flags);
+ for (i = 0; i < master->ibi.num_slots; i++) {
+ if (master->ibi.slots[i] == dev) {
+ *slot = i;
+ ret = 0;
+ break;
+ }
+ }
+
+ if (ret) {
+ for (i = 0; i < master->ibi.num_slots; i++) {
+ if (!master->ibi.slots[i]) {
+ master->ibi.slots[i] = dev;
+ *slot = i;
+ ret = 0;
+ break;
+ }
+ }
+ }
+ spin_unlock_irqrestore(&master->ibi.lock, flags);
+
+ return ret;
+}
+
+static int cdns_i3c_request_mastership(struct i3c_master_controller *m)
+{
+ struct cdns_i3c_master *master = to_cdns_i3c_master(m);
+ int status;
+
+ status = readl(master->regs + MST_STATUS0);
+
+ if (status & MST_STATUS0_MASTER_MODE)
+ return 0;
+
+ status = readl(master->regs + SLV_STATUS1);
+
+ if (status & SLV_STATUS1_MR_DIS)
+ return -EACCES;
+
+ writel(readl(master->regs + CTRL) | CTRL_MST_INIT | CTRL_MST_ACK,
+ master->regs + CTRL);
+
+ return 0;
+}
+
+static void
+cdns_i3c_master_enable_mastership_events(struct i3c_master_controller *m)
+{
+ struct cdns_i3c_master *master = to_cdns_i3c_master(m);
+ struct cdns_i3c_i2c_dev_data *data;
+ struct i3c_dev_desc *i3cdev;
+ unsigned long flags;
+ u32 sircfg, sirmap;
+ int ret;
+
+ i3c_bus_for_each_i3cdev(&m->bus, i3cdev) {
+ if (I3C_BCR_DEVICE_ROLE(i3cdev->info.bcr) !=
+ I3C_BCR_I3C_MASTER ||
+ m->this == i3cdev)
+ continue;
+
+ data = i3c_dev_get_master_data(i3cdev);
+ if (!data)
+ continue;
+
+ ret = cdns_i3c_master_find_ibi_slot(master, i3cdev, &data->ibi);
+ if (ret)
+ continue;
+
+ spin_lock_irqsave(&master->ibi.lock, flags);
+ sirmap = readl(master->regs + SIR_MAP_DEV_REG(data->ibi));
+ sirmap &= ~SIR_MAP_DEV_CONF_MASK(data->ibi);
+ sircfg = SIR_MAP_DEV_ROLE(i3cdev->info.bcr >> 6) |
+ SIR_MAP_DEV_DA(i3cdev->info.dyn_addr) |
+ SIR_MAP_DEV_PL(i3cdev->info.max_ibi_len) |
+ SIR_MAP_DEV_ACK;
+
+ if (i3cdev->info.bcr & I3C_BCR_MAX_DATA_SPEED_LIM)
+ sircfg |= SIR_MAP_DEV_SLOW;
+
+ sirmap |= SIR_MAP_DEV_CONF(data->ibi, sircfg);
+ writel(sirmap, master->regs + SIR_MAP_DEV_REG(data->ibi));
+ spin_unlock_irqrestore(&master->ibi.lock, flags);
+ }
+}
+
+static bool
+cdns_i3c_master_check_event_set(struct i3c_master_controller *m,
+ enum i3c_event event)
+{
+ struct cdns_i3c_master *master = to_cdns_i3c_master(m);
+ bool ret = false;
+
+ switch (event) {
+ case I3C_SLV_DA_UPDATE:
+ if (readl(master->regs + SLV_STATUS1) & SLV_STATUS1_HAS_DA)
+ ret = true;
+ break;
+
+ case I3C_SLV_MR_DONE:
+ if (readl(master->regs + MST_STATUS0) & MST_STATUS0_MASTER_MODE)
+ ret = true;
+ break;
+
+ default:
+ break;
+ }
+
+ return ret;
+}
+
+int cdns_i3c_sec_master_dyn_addr(struct i3c_master_controller *m)
+{
+ struct cdns_i3c_master *master = to_cdns_i3c_master(m);
+ int dyn_addr = -1;
+ u32 status;
+
+ status = readl(master->regs + SLV_STATUS1);
+ if (status & SLV_STATUS1_HAS_DA)
+ dyn_addr = SLV_STATUS1_DA(status);
+
+ return dyn_addr;
+}
+
static const struct i3c_master_controller_ops cdns_i3c_master_ops = {
.bus_init = cdns_i3c_master_bus_init,
.bus_cleanup = cdns_i3c_master_bus_cleanup,
@@ -1524,6 +1695,10 @@ static const struct i3c_master_controller_ops cdns_i3c_master_ops = {
.request_ibi = cdns_i3c_master_request_ibi,
.free_ibi = cdns_i3c_master_free_ibi,
.recycle_ibi_slot = cdns_i3c_master_recycle_ibi_slot,
+ .request_mastership = cdns_i3c_request_mastership,
+ .enable_mr_events = cdns_i3c_master_enable_mastership_events,
+ .check_event_set = cdns_i3c_master_check_event_set,
+ .sec_mst_dyn_addr = cdns_i3c_sec_master_dyn_addr,
};
static void cdns_i3c_master_hj(struct work_struct *work)
@@ -1535,10 +1710,152 @@ static void cdns_i3c_master_hj(struct work_struct *work)
i3c_master_do_daa(&master->base);
}
+static void cdns_i3c_master_yield(struct work_struct *work)
+{
+ struct cdns_i3c_master *master = container_of(work,
+ struct cdns_i3c_master,
+ mr_yield_work);
+
+ i3c_master_yield_bus(&master->base, master->mr_addr);
+}
+
+static void cdns_i3c_sec_master_defslvs(struct work_struct *work)
+{
+ struct cdns_i3c_master *master = container_of(work,
+ struct cdns_i3c_master,
+ defslvs_work);
+ struct i3c_master_controller *m = &master->base;
+ struct i3c_ccc_dev_desc *desc;
+ struct cdns_i3c_i2c_dev_data *data;
+ struct i2c_dev_desc *i2cdev;
+ struct i3c_dev_desc *i3cdev;
+ u32 devs, val, rr, slot;
+ u32 r0, r1, r2;
+ u8 saddr;
+
+ devs = readl(master->regs + DEVS_CTRL) & DEVS_CTRL_DEVS_ACTIVE_MASK;
+ master->free_rr_slots = GENMASK(master->maxdevs, 1) & ~devs;
+
+ /*
+ * We chose to ignore I2C devices received from
+ * main master and use I2C device info from boardinfo.
+ * Since I2C device from boardinfo are already
+ * registered during bus_init, we just use same slot
+ * and if any I3C device is received in DEFSLVS in that
+ * place, just move that I3C device to other free slot.
+ * If there is no free slot, then such I3C devices
+ * are ignored.
+ * Master controller driver can chose how to handle I2C
+ * devices in DEFSLVS and pass only I3C devices list to
+ * I3C core DEFSVLS processing to handle hotplug and
+ * I3C device address changes.
+ */
+ for (slot = 0; slot < master->maxdevs; slot++) {
+ if (!(devs & BIT(slot)))
+ continue;
+
+ val = readl(master->regs + DEV_ID_RR0(slot));
+ if (!(val & DEV_ID_RR0_IS_I3C)) {
+ writel(readl(master->regs + DEVS_CTRL) |
+ DEVS_CTRL_DEV_CLR(slot),
+ master->regs + DEVS_CTRL);
+ master->free_rr_slots |= BIT(slot);
+ }
+ }
+
+ val = 0;
+ devs = readl(master->regs + DEVS_CTRL) & DEVS_CTRL_DEVS_ACTIVE_MASK;
+ i3c_bus_for_each_i2cdev(&m->bus, i2cdev) {
+ data = i2c_dev_get_master_data(i2cdev);
+ if (devs & BIT(data->id)) {
+ rr = readl(master->regs + DEV_ID_RR0(data->id));
+ saddr = DEV_ID_RR0_GET_DEV_ADDR(rr);
+ if (saddr != i2cdev->boardinfo->base.addr) {
+ r0 = readl(master->regs + DEV_ID_RR0(data->id));
+ r1 = readl(master->regs + DEV_ID_RR1(data->id));
+ r2 = readl(master->regs + DEV_ID_RR2(data->id));
+ slot = ffs(master->free_rr_slots) - 1;
+ if (slot > 0) {
+ writel(r0,
+ master->regs + DEV_ID_RR0(slot));
+ writel(r1,
+ master->regs + DEV_ID_RR1(slot));
+ writel(r2,
+ master->regs + DEV_ID_RR2(slot));
+ writel(readl(master->regs + DEVS_CTRL) |
+ DEVS_CTRL_DEV_ACTIVE(slot),
+ master->regs + DEVS_CTRL);
+ master->free_rr_slots &= ~BIT(slot);
+ }
+ } else {
+ continue;
+ }
+ }
+ writel(readl(master->regs + DEVS_CTRL) |
+ DEVS_CTRL_DEV_CLR(data->id),
+ master->regs + DEVS_CTRL);
+ writel(readl(master->regs + DEVS_CTRL) |
+ DEVS_CTRL_DEV_ACTIVE(data->id),
+ master->regs + DEVS_CTRL);
+ writel(prepare_rr0_dev_address(i2cdev->boardinfo->base.addr) |
+ (i2cdev->boardinfo->base.flags & I2C_CLIENT_TEN ?
+ DEV_ID_RR0_LVR_EXT_ADDR : 0),
+ master->regs + DEV_ID_RR0(data->id));
+ writel(i2cdev->boardinfo->lvr,
+ master->regs + DEV_ID_RR2(data->id));
+ master->free_rr_slots &= ~BIT(data->id);
+ }
+
+ master->base.defslvs_data.ndevs = 0;
+ desc = master->base.defslvs_data.devs;
+ devs = readl(master->regs + DEVS_CTRL) & DEVS_CTRL_DEVS_ACTIVE_MASK;
+
+ for (slot = 0; slot < master->maxdevs; slot++) {
+ if (!(devs & BIT(slot)))
+ continue;
+
+ val = readl(master->regs + DEV_ID_RR0(slot));
+ if (val & DEV_ID_RR0_IS_I3C) {
+ memset(desc, 0, sizeof(struct i3c_ccc_dev_desc));
+ rr = readl(master->regs + DEV_ID_RR0(slot));
+ desc->dyn_addr = DEV_ID_RR0_GET_DEV_ADDR(rr);
+ rr = readl(master->regs + DEV_ID_RR2(slot));
+ desc->dcr = rr;
+ desc->bcr = rr >> 8;
+ master->base.defslvs_data.ndevs++;
+ desc++;
+ }
+ }
+
+ if (i3c_master_process_defslvs(m)) {
+ queue_work(master->base.wq, work);
+ return;
+ }
+
+ /*
+ * Fix data->id for any changes due to mismatch in number
+ * of I2C devices on main and secondary master, causing
+ * I3C devices received in DEFSLVS in a slot which was used
+ * for I2C device on sec master to be moved to other free
+ * slot. And then if any I3C device get unplugged
+ * next DEFSLVS processing would I3C devices in original
+ * I2C slot to be moved to different slot than it
+ * was moved at the first DEFSLVS processing.
+ */
+ i3c_bus_for_each_i3cdev(&m->bus, i3cdev) {
+ data = i3c_dev_get_master_data(i3cdev);
+ if (!data)
+ continue;
+ data->id = cdns_i3c_master_get_rr_slot(master,
+ i3cdev->info.dyn_addr);
+ }
+}
+
static int cdns_i3c_master_probe(struct platform_device *pdev)
{
struct cdns_i3c_master *master;
struct resource *res;
+ bool secondary;
int ret, irq;
u32 val;
@@ -1579,6 +1896,7 @@ static int cdns_i3c_master_probe(struct platform_device *pdev)
spin_lock_init(&master->xferqueue.lock);
INIT_LIST_HEAD(&master->xferqueue.list);
+ INIT_WORK(&master->mr_yield_work, cdns_i3c_master_yield);
INIT_WORK(&master->hj_work, cdns_i3c_master_hj);
writel(0xffffffff, master->regs + MST_IDR);
writel(0xffffffff, master->regs + SLV_IDR);
@@ -1590,6 +1908,7 @@ static int cdns_i3c_master_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, master);
val = readl(master->regs + CONF_STATUS0);
+ secondary = (val & CONF_STATUS0_SEC_MASTER) ? true : false;
/* Device ID0 is reserved to describe this master. */
master->maxdevs = CONF_STATUS0_DEVS_NUM(val);
@@ -1610,12 +1929,24 @@ static int cdns_i3c_master_probe(struct platform_device *pdev)
if (!master->ibi.slots)
goto err_disable_sysclk;
+ if (secondary)
+ INIT_WORK(&master->defslvs_work, cdns_i3c_sec_master_defslvs);
+
writel(IBIR_THR(1), master->regs + CMD_IBI_THR_CTRL);
- writel(MST_INT_IBIR_THR, master->regs + MST_IER);
+ writel(MST_INT_IBIR_THR | MST_INT_MR_DONE, master->regs + MST_IER);
writel(DEVS_CTRL_DEV_CLR_ALL, master->regs + DEVS_CTRL);
- ret = i3c_master_register(&master->base, &pdev->dev,
- &cdns_i3c_master_ops);
+ if (secondary) {
+ ret = i3c_secondary_master_register(&master->base, &pdev->dev,
+ &cdns_i3c_master_ops);
+ if (!ret)
+ writel(SLV_INT_DEFSLVS | SLV_INT_MR_DONE,
+ master->regs + SLV_IER);
+ } else {
+ ret = i3c_master_register(&master->base, &pdev->dev,
+ &cdns_i3c_master_ops);
+ }
+
if (ret)
goto err_disable_sysclk;
--
2.17.1
_______________________________________________
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH v7 2/7] i3c: master: use i3c_master_register only for main master
2020-05-11 13:13 ` [PATCH v7 2/7] i3c: master: use i3c_master_register only for main master Parshuram Thombare
@ 2020-05-11 15:44 ` Boris Brezillon
2020-05-12 4:57 ` Parshuram Raju Thombare
0 siblings, 1 reply; 17+ messages in thread
From: Boris Brezillon @ 2020-05-11 15:44 UTC (permalink / raw)
To: Parshuram Thombare
Cc: mparab, bbrezillon, praneeth, linux-kernel, vitor.soares, pgaj,
linux-i3c
On Mon, 11 May 2020 15:13:05 +0200
Parshuram Thombare <pthombar@cadence.com> wrote:
> Removed last argument 'secondary' and refactored
> i3c_master_register to move code that can be common
> to i3c_secondary_master_register to separate function
> i3c_master_init.
>
> Signed-off-by: Parshuram Thombare <pthombar@cadence.com>
> ---
> drivers/i3c/master.c | 69 +++++++++++++++++-----------
> drivers/i3c/master/dw-i3c-master.c | 2 +-
> drivers/i3c/master/i3c-master-cdns.c | 2 +-
> include/linux/i3c/master.h | 3 +-
> 4 files changed, 46 insertions(+), 30 deletions(-)
>
> diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
> index 5f4bd52121fe..ba07a7d49633 100644
> --- a/drivers/i3c/master.c
> +++ b/drivers/i3c/master.c
> @@ -2391,31 +2391,10 @@ static int i3c_master_check_ops(const struct i3c_master_controller_ops *ops)
> return 0;
> }
>
> -/**
> - * 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
> - *
> - * This function takes care of everything for you:
> - *
> - * - creates and initializes the I3C bus
> - * - populates the bus with static I2C devs if @parent->of_node is not
> - * NULL
> - * - registers all I3C devices added by the controller during bus
> - * initialization
> - * - registers the I2C adapter and all I2C devices
> - *
> - * Return: 0 in case of success, a negative error code otherwise.
> - */
> -int i3c_master_register(struct i3c_master_controller *master,
> - struct device *parent,
> - const struct i3c_master_controller_ops *ops,
> - bool secondary)
> +static int i3c_master_init(struct i3c_master_controller *master,
> + struct device *parent,
> + const struct i3c_master_controller_ops *ops,
> + bool secondary)
> {
> struct i3c_bus *i3cbus = i3c_master_get_bus(master);
> enum i3c_bus_mode mode = I3C_BUS_MODE_PURE;
> @@ -2482,6 +2461,45 @@ int i3c_master_register(struct i3c_master_controller *master,
> if (ret)
> goto err_put_dev;
>
> + return 0;
> +
> +err_put_dev:
> + put_device(&master->dev);
> +
> + return ret;
> +}
> +
> +/**
> + * i3c_master_register() - register an I3C master
The function should be renamed and the doc updated to reflect the fact
that it only works for primary masters:
i3c_primary_master_register() - register a primary 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
This argument no longer exists.
> + *
> + * This function takes care of everything for you:
> + *
> + * - creates and initializes the I3C bus
> + * - populates the bus with static I2C devs if @parent->of_node is not
> + * NULL
> + * - registers all I3C devices added by the controller during bus
> + * initialization
> + * - registers the I2C adapter and all I2C devices
> + *
> + * Return: 0 in case of success, a negative error code otherwise.
> + */
> +int i3c_master_register(struct i3c_master_controller *master,
> + struct device *parent,
> + const struct i3c_master_controller_ops *ops)
> +{
> + int ret;
> +
> + ret = i3c_master_init(master, parent, ops, false);
> + if (ret)
> + return ret;
> +
> ret = device_add(&master->dev);
> if (ret)
> goto err_cleanup_bus;
> @@ -2511,7 +2529,6 @@ int i3c_master_register(struct i3c_master_controller *master,
> err_cleanup_bus:
> i3c_master_bus_cleanup(master);
>
> -err_put_dev:
> put_device(&master->dev);
>
> return ret;
> diff --git a/drivers/i3c/master/dw-i3c-master.c b/drivers/i3c/master/dw-i3c-master.c
> index 1d83c97431c7..5d5a8a90ec06 100644
> --- a/drivers/i3c/master/dw-i3c-master.c
> +++ b/drivers/i3c/master/dw-i3c-master.c
> @@ -1158,7 +1158,7 @@ static int dw_i3c_probe(struct platform_device *pdev)
> master->free_pos = GENMASK(master->maxdevs - 1, 0);
>
> ret = i3c_master_register(&master->base, &pdev->dev,
> - &dw_mipi_i3c_ops, false);
> + &dw_mipi_i3c_ops);
> if (ret)
> goto err_assert_rst;
>
> diff --git a/drivers/i3c/master/i3c-master-cdns.c b/drivers/i3c/master/i3c-master-cdns.c
> index 8889a4fdb454..ed4f43807f9e 100644
> --- a/drivers/i3c/master/i3c-master-cdns.c
> +++ b/drivers/i3c/master/i3c-master-cdns.c
> @@ -1615,7 +1615,7 @@ static int cdns_i3c_master_probe(struct platform_device *pdev)
> writel(DEVS_CTRL_DEV_CLR_ALL, master->regs + DEVS_CTRL);
>
> ret = i3c_master_register(&master->base, &pdev->dev,
> - &cdns_i3c_master_ops, false);
> + &cdns_i3c_master_ops);
> if (ret)
> goto err_disable_sysclk;
>
> diff --git a/include/linux/i3c/master.h b/include/linux/i3c/master.h
> index f13fd8b1dd79..f5ba82c390bc 100644
> --- a/include/linux/i3c/master.h
> +++ b/include/linux/i3c/master.h
> @@ -533,8 +533,7 @@ int i3c_master_set_info(struct i3c_master_controller *master,
>
> int i3c_master_register(struct i3c_master_controller *master,
> struct device *parent,
> - const struct i3c_master_controller_ops *ops,
> - bool secondary);
> + const struct i3c_master_controller_ops *ops);
> int i3c_master_unregister(struct i3c_master_controller *master);
>
> /**
_______________________________________________
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v7 1/7] i3c: master: secondary master initialization document
2020-05-11 13:12 ` [PATCH v7 1/7] i3c: master: secondary master initialization document Parshuram Thombare
@ 2020-05-11 16:05 ` Boris Brezillon
2020-05-12 5:03 ` Parshuram Raju Thombare
0 siblings, 1 reply; 17+ messages in thread
From: Boris Brezillon @ 2020-05-11 16:05 UTC (permalink / raw)
To: Parshuram Thombare
Cc: mparab, bbrezillon, praneeth, linux-kernel, vitor.soares, pgaj,
linux-i3c
On Mon, 11 May 2020 15:12:39 +0200
Parshuram Thombare <pthombar@cadence.com> wrote:
> Document describing secondary master initialization,
> mastership handover and DEFSLVS handling processes.
Thanks for doing that, but you probably didn't try to compile the doc
(the formatting is all messed up).
# make htmldocs
and then check the output in Documentation/output/ (open index.html
with a web-browser and go to the i3c section).
>
> Signed-off-by: Parshuram Thombare <pthombar@cadence.com>
> ---
> Documentation/driver-api/i3c/index.rst | 1 +
> .../i3c/secondary-master-initialization.rst | 118 ++++++++++++++++++
> 2 files changed, 119 insertions(+)
> create mode 100644 Documentation/driver-api/i3c/secondary-master-initialization.rst
>
> diff --git a/Documentation/driver-api/i3c/index.rst b/Documentation/driver-api/i3c/index.rst
> index 783d6dad054b..af2a0aa68f5b 100644
> --- a/Documentation/driver-api/i3c/index.rst
> +++ b/Documentation/driver-api/i3c/index.rst
> @@ -9,3 +9,4 @@ I3C subsystem
> protocol
> device-driver-api
> master-driver-api
> + secondary-master-initialization
> diff --git a/Documentation/driver-api/i3c/secondary-master-initialization.rst b/Documentation/driver-api/i3c/secondary-master-initialization.rst
> new file mode 100644
> index 000000000000..9d1869550807
> --- /dev/null
> +++ b/Documentation/driver-api/i3c/secondary-master-initialization.rst
> @@ -0,0 +1,118 @@
> +.. SPDX-License-Identifier: GPL-2.0
> +
> +===================================
> +I3C Secondary Master Initialization
> +===================================
> +
> ++-----------------------------------------+-------------------------------------------+
> +| **Main master** | **Secondary master** |
> ++=========================================+===========================================+
> +| | |
> +| | Do I3C master controller specific | | Do I3C master controller specefic |
> +| initialization. | initialization, except enabling |
> +| | the DEFSLVS interrupt. |
> +| | Call i3c_master_register | | Call i3c_secondary_master_register |
> +| | |
> +| *i3c_master_register* | *i3c_secondary_master_register* |
> +| | Initialize I3C master controller | | Initialize I3C master controller |
> +| object. | object. |
> +| | Scan I3C and I2C devices from DTS. | | Scan I2C devices from DTS. |
> +| | Set appropriate bus mode based on | | Set appropriate bus mode based on |
> +| I2C devices information. | I3C and I2C devices information. |
> +| | Create a work queue. | | Create a work queue. |
> +| | Call i3c_master_bus_init | | Call i3c_secondary_master_bus_init |
> +| | |
> +| *i3c_master_bus_init* | *i3c_secondary_master_bus_init* |
> +| | Call bus_init to do controller | | Call bus_init to do controller |
> +| specific bus initialization and | specific bus initialization and |
> +| enabling the controller. | enabling the controller. |
> +| | Create I3C device representing a| | Create I3C device representing a |
> +| master and add it to the I3C | master and add it to the I3C |
> +| device list. | device list. |
> +| | Set current master to the device| |
> +| created to represent I3C master | | Allocate memory for 'defslvs_data', |
> +| device. | that will be used to pass I3C |
> +| | Reset all dynamic address that | device list received in DEFSLVS |
> +| may have been assigned before. | to I3C core DEFSLVS processing |
> +| | Disable all slave events before | | Add I3C device representing this |
> +| starting DAA. | master to the system. |
> +| | Pre-assign dynamic address and | | Expose our I3C bus as an I2C adapter |
> +| retrieve device information if | so that I2C devices are exposed |
> +| needed. | through the I2C subsystem. |
> +| | Do dynamic address assignment to| |
> +| all I3C devices currenly present| | Enable DEFSLVS interrupt. |
> +| on the bus. | |
> +| | Create I3C devices representing | |
> +| those found during DAA. +-------------------------------------------+
> +| | Send DEFSVLS message | | *DEFSLVS interrupt* |
> +| containing information about all| | Controller driver can chose how to |
> +| known I3C and I2C devices. | to handle I2C devices received in |
> +| | DEFSLVS e.g. Cadence's controller |
> +| | driver ignore I2C devices from |
> +| | DEFSLVS and only uses I2C device |
> +| | information from DTS. |
> +| | | Read all I3C devices information |
> +| | from DEFSLVS message in hardware to |
> +| | defslvs_data in master object. |
> +| | | Call i3c_master_process_defslvs |
> +| | |
> +| | *i3c_master_process_defslvs* |
> +| | | Acquire I3c bus |
> +| | |
> +| | *i3c_master_acquire_bus* |
> +| | Add I3C device representing this | | If device is already holding the |
> +| master to the system. | mastership, just broadcast DISEC |
> +| | Expose our I3C bus as an I2C | MR, HJ message and return. |
> +| adapter so that I2C devices are | | Check if device has got a address |
> +| exposed through the I2C subsystem. | by polling with a timeout. |
> +| | Register all I3C devices. | |
> +| | | Send MR request: Controller driver|
> +| | should check if it is already in |
> +| | master mode, to handle the case |
> +| | of mastership yielded but due to |
> +| | poll timeout acquire failed. |
> +| | | If not a master, wait until MR |
> +| | ENEC is received if currently it |
> +| | is disabled. |
> +| | Broadcast ENEC MR, HJ message. | | Send MR request. |
> ++-----------------------------------------+ |
> +| | *MR request interrupt* | |
> +| | |
> +| *i3c_master_yield_bus* | |
> +| | Check if this device is still a | |
> +| master to handle a case of | |
> +| multiple MR requests from different| |
> +| devices at a same time. | |
> +| | Broadcast DISEC MR, HJ message. | |
> +| New master should broadcast ENEC | |
> +| MR, HJ once it's usage of bus is | |
> +| done. | |
> +| | Get accept mastership acknowldege | |
> +| from requesting master. | |
> +| | Mastership hand over is done. | | Check if device enter master |
> +| | In case of failure reenable | mode by polling with a timeout. |
> +| MR requests by broadcasting ENEC | |
> +| MR, HJ. | Handle I3C device list from DEFSLVS. |
> +| | |
> +| | *i3c_master_populate_bus* |
> +| | | Free up all I3C addresses to handle |
> +| | address re assignment by main |
> +| | master. |
> +| | | Move all devices from I3C list to a |
> +| | temporary list. |
> +| | | For every device from defslvs_data |
> +| | list except the receiving master |
> +| | device, retrieve pid and compare it |
> +| | with already known I3C devices from |
> +| | I3C list. If match is found, |
> +| | allocate new address and move the |
> +| | device to the original I3C device |
> +| | list. If no match is found, it is a |
> +| | new device. Register and add it to |
> +| | the original I3C list. |
> +| | | At the end if temporary list is not |
> +| | empty, it contains unplugged I3C |
> +| | device. Deregister and delete them. |
> +| | |
> +| | Broadcast ENEC MR, HJ message. |
> ++-----------------------------------------+-------------------------------------------+
_______________________________________________
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v7 3/7] i3c: master: add i3c_secondary_master_register
2020-05-11 13:13 ` [PATCH v7 3/7] i3c: master: add i3c_secondary_master_register Parshuram Thombare
@ 2020-05-11 16:14 ` Boris Brezillon
2020-05-12 6:42 ` Parshuram Raju Thombare
0 siblings, 1 reply; 17+ messages in thread
From: Boris Brezillon @ 2020-05-11 16:14 UTC (permalink / raw)
To: Parshuram Thombare
Cc: mparab, bbrezillon, praneeth, linux-kernel, vitor.soares, pgaj,
linux-i3c
On Mon, 11 May 2020 15:13:53 +0200
Parshuram Thombare <pthombar@cadence.com> wrote:
> add i3c_secondary_master_register which is used
> to register secondary masters.
>
> Signed-off-by: Parshuram Thombare <pthombar@cadence.com>
> ---
> drivers/i3c/master.c | 154 ++++++++++++++++++++++++++++++++++++-
> include/linux/i3c/master.h | 3 +
> 2 files changed, 156 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
> index ba07a7d49633..669bd7e45810 100644
> --- a/drivers/i3c/master.c
> +++ b/drivers/i3c/master.c
> @@ -1768,6 +1768,90 @@ static int i3c_master_bus_init(struct i3c_master_controller *master)
> return ret;
> }
>
> +/**
> + * i3c_secondary_master_bus_init() - initialize an I3C bus for secondary
> + * master
> + * @master: secondary master initializing the bus
> + *
> + * This function does
> + *
> + * 1. Attach I2C devs to the master
> + *
> + * 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)
Can you really select the bus mode without knowing the I3C devices you
have on the bus? Or maybe that's a preliminary initialization which is
then updated when you receive DEFSLVS events.
> + *
> + * Once this is done, I2C devices should be usable.
I suspect you'll have to request bus ownership first, which means
they're not really usable, just registered to the I2C subsystem. This
might lead to a whole bunch of problems when drivers will try to send
messages to the I2C devices and receive ETIMEDOUT/EBUSY errors. We'll
probably need some updates to the I2C framework if we want I2C to play
nicely with bus handover, but I think we can keep that for later. I'd
suggest to forget about I2C for now and mark that as a limitation (no
I2C devs on secondary masters).
> + *
> + * Return: a 0 in case of success, an negative error code otherwise.
> + */
> +static int i3c_secondary_master_bus_init(struct i3c_master_controller *master)
> +{
> + enum i3c_addr_slot_status status;
> + struct i2c_dev_boardinfo *i2cboardinfo;
> + 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;
> + }
> +
> + 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;
> + }
> +
> + ret = i3c_master_attach_i2c_dev(master, i2cdev);
> + if (ret) {
> + i3c_master_free_i2c_dev(i2cdev);
> + goto err_detach_devs;
> + }
> + }
> +
> + /*
> + * Now execute the controller specific ->bus_init() routine, which
> + * might configure its internal logic to match the bus limitations.
> + */
> + ret = master->ops->bus_init(master);
> + if (ret)
> + goto err_detach_devs;
> +
> + /*
> + * The master device should have been instantiated in ->bus_init(),
> + * complain if this was not the case.
> + */
> + if (!master->this) {
> + dev_err(&master->dev,
> + "master_set_info() was not called in ->bus_init()\n");
> + ret = -EINVAL;
> + goto err_bus_cleanup;
> + }
> +
> + return 0;
> +
> +err_bus_cleanup:
> + if (master->ops->bus_cleanup)
> + master->ops->bus_cleanup(master);
> +
> +err_detach_devs:
> + i3c_master_detach_free_devs(master);
> +
> + return ret;
> +}
> +
> static void i3c_master_bus_cleanup(struct i3c_master_controller *master)
> {
> if (master->ops->bus_cleanup)
> @@ -2457,7 +2541,10 @@ static int i3c_master_init(struct i3c_master_controller *master,
> goto err_put_dev;
> }
>
> - ret = i3c_master_bus_init(master);
> + if (secondary)
> + ret = i3c_secondary_master_bus_init(master);
> + else
> + ret = i3c_master_bus_init(master);
> if (ret)
> goto err_put_dev;
>
> @@ -2535,6 +2622,71 @@ int i3c_master_register(struct i3c_master_controller *master,
> }
> EXPORT_SYMBOL_GPL(i3c_master_register);
>
> +/**
> + * i3c_secondary_master_register() - register an I3C secondary 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
> + *
> + * This function does minimal required initialization for secondary
> + * master, rest functionality like creating and registering I2C
> + * and I3C devices is done in defslvs processing.
> + *
> + * i3c_secondary_master_register() does following things -
> + * - creates and initializes the I3C bus
> + * - populates the bus with static I2C devs if @parent->of_node is not
> + * NULL
> + * initialization
> + * - allocate memory for defslvs_data.devs, which is used to receive
> + * defslvs list
> + * - create I3C device representing this master
> + * - registers the I2C adapter and all I2C devices
> + *
> + * Return: 0 in case of success, a negative error code otherwise.
> + */
> +int i3c_secondary_master_register(struct i3c_master_controller *master,
> + struct device *parent,
> + const struct i3c_master_controller_ops *ops)
> +{
> + int ret;
> +
> + ret = i3c_master_init(master, parent, ops, true);
> + if (ret)
> + return ret;
> +
> + ret = device_add(&master->dev);
> + if (ret)
> + goto err_cleanup_bus;
> +
> + /*
> + * Expose our I3C bus as an I2C adapter so that I2C devices are exposed
> + * through the I2C subsystem.
> + */
> + ret = i3c_master_i2c_adapter_init(master);
> + if (ret)
> + goto err_del_dev;
> +
> + /*
> + * We're done initializing the bus and the controller, we can now
> + * register I3C devices from defslvs list.
> + */
> + master->init_done = true;
> +
> + return 0;
> +
> +err_del_dev:
> + device_del(&master->dev);
> +
> +err_cleanup_bus:
> + i3c_master_bus_cleanup(master);
> +
> + put_device(&master->dev);
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(i3c_secondary_master_register);
> +
> /**
> * i3c_master_unregister() - unregister an I3C master
> * @master: master used to send frames on the bus
> diff --git a/include/linux/i3c/master.h b/include/linux/i3c/master.h
> index f5ba82c390bc..5124ff4831eb 100644
> --- a/include/linux/i3c/master.h
> +++ b/include/linux/i3c/master.h
> @@ -534,6 +534,9 @@ int i3c_master_set_info(struct i3c_master_controller *master,
> int i3c_master_register(struct i3c_master_controller *master,
> struct device *parent,
> const struct i3c_master_controller_ops *ops);
> +int i3c_secondary_master_register(struct i3c_master_controller *master,
> + struct device *parent,
> + const struct i3c_master_controller_ops *ops);
> int i3c_master_unregister(struct i3c_master_controller *master);
>
> /**
_______________________________________________
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v7 4/7] i3c: master: add mastership handover support
2020-05-11 13:14 ` [PATCH v7 4/7] i3c: master: add mastership handover support Parshuram Thombare
@ 2020-05-11 16:38 ` Boris Brezillon
2020-05-12 6:24 ` Parshuram Raju Thombare
0 siblings, 1 reply; 17+ messages in thread
From: Boris Brezillon @ 2020-05-11 16:38 UTC (permalink / raw)
To: Parshuram Thombare
Cc: mparab, bbrezillon, praneeth, linux-kernel, vitor.soares, pgaj,
linux-i3c
On Mon, 11 May 2020 15:14:49 +0200
Parshuram Thombare <pthombar@cadence.com> wrote:
> Added mastership acquire and yield functions.
>
> Signed-off-by: Parshuram Thombare <pthombar@cadence.com>
> ---
> 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 <linux/slab.h>
> #include <linux/spinlock.h>
> #include <linux/workqueue.h>
> +#include <linux/iopoll.h>
>
> #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(&master->bus);
> + ret = i3c_master_enec_locked(master, I3C_BROADCAST_ADDR,
> + I3C_CCC_EVENT_MR | I3C_CCC_EVENT_HJ);
> + i3c_bus_maintenance_unlock(&master->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(&m->bus);
> + i3c_bus_maintenance_lock(&m->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, 1000000);
> + 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, 1000000);
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(&m->bus);
> + i3c_bus_normaluse_lock(&m->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(&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;
> +}
> +
> 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(&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;
>
> @@ -1570,7 +1667,9 @@ 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->secondary)
> + master->bus.cur_master = master->this;
This change doesn't seem related to this patch. Looks like this should
instead go to patch 3.
>
> ret = i3c_master_attach_i3c_dev(master, i3cdev);
> if (ret)
> @@ -1612,6 +1711,73 @@ static void i3c_master_detach_free_devs(struct i3c_master_controller *master)
> }
> }
>
> +/**
> + * i3c_master_yield_bus() - yield I3C bus mastership
> + * @m: I3C master object
> + * @sec_mst_dyn_addr: address of device requesting mastership
> + *
> + * This function may sleep.
> + * It is expected to be called with normaluse_lock.
> + */
> +void
> +i3c_master_yield_bus(struct i3c_master_controller *m, u8 sec_mst_dyn_addr)
> +{
> + struct i3c_dev_desc *i3cdev;
> + int ret = 0;
> +
> + i3c_bus_maintenance_lock(&m->bus);
> + if (m->this != m->bus.cur_master)
> + goto mr_yield_done;
> +
> + /*
> + * Maintenance lock and master check above is used to
> + * avoid race amongst devices sending MR requests
> + * at the same time, as soon as ENEC MST is sent by the current
> + * master. It ensure that only one MR request is processed,
> + * rest MR requests on losing devices will timeout in wait MR
> + * DONE state. And next MR requests are blocked due to DISEC MST
> + * sent by current master in yield operation.
> + * New master should send ENEC MST once it's work is done.
> + * maintainanace lock is also needed for i3c_master_get_accmst_locked.
> + */
> +
> + ret = i3c_master_disec_locked(m, I3C_BROADCAST_ADDR,
> + I3C_CCC_EVENT_MR |
> + I3C_CCC_EVENT_HJ);
Isn't it taken care of at the controller level? I'm fine sending it
here, but I suspect some controllers might actually auto-disable MRs
once they receive one.
> + /*
> + * Once mastership is given to the new master, it is expected that
> + * MR is disabled prior to that and new master is responsible to
> + * enable it by broadcasting ENEC MR when it's work is done.
> + * If DISEC MR fails and we still go ahead with handover, chances
> + * are new master will get interrupted by unexpected MR requests.
> + */
> + if (ret)
> + goto mr_yield_done;
> +
> + ret = i3c_master_get_accmst_locked(m, sec_mst_dyn_addr);
> + if (ret)
> + goto mr_yield_done;
> +
> + i3c_bus_for_each_i3cdev(&m->bus, i3cdev) {
> + if (sec_mst_dyn_addr == i3cdev->info.dyn_addr) {
> + m->bus.cur_master = i3cdev;
> + break;
> + }
> + }
> +
> + /* Requesting device not found on i3c list. This should never happen. */
> + if (m->this == m->bus.cur_master) {
if (WARN_ON(m->this == m->bus.cur_master)) {
And you should probably check that before send DISEC.
> + ret = -EAGAIN;
> + WARN_ON(1);
> + }
> +
> +mr_yield_done:
> + i3c_bus_maintenance_unlock(&m->bus);
> + if (ret)
> + i3c_master_enable_mr_events(m);
> +}
> +EXPORT_SYMBOL_GPL(i3c_master_yield_bus);
> +
> /**
> * i3c_master_bus_init() - initialize an I3C bus
> * @master: main master initializing the bus
> @@ -2472,6 +2638,10 @@ 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->check_event_set))
> + return -EINVAL;
> +
> return 0;
> }
>
> @@ -2485,10 +2655,6 @@ static int i3c_master_init(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;
> @@ -2608,6 +2774,9 @@ int i3c_master_register(struct i3c_master_controller *master,
> i3c_master_register_new_i3c_devs(master);
> i3c_bus_normaluse_unlock(&master->bus);
>
> + if (ops->request_mastership)
> + ret = i3c_master_enable_mr_events(master);
> +
> return 0;
>
> err_del_dev:
> diff --git a/include/linux/i3c/master.h b/include/linux/i3c/master.h
> index 5124ff4831eb..dd67497ad8b1 100644
> --- a/include/linux/i3c/master.h
> +++ b/include/linux/i3c/master.h
> @@ -259,6 +259,16 @@ enum i3c_bus_mode {
> I3C_BUS_MODE_MIXED_SLOW,
> };
>
> +/**
> + * enum i3c_event - I3C master (currently acting as a slave) controller events
> + * @I3C_SLV_DA_UPDATE: I3C master has dynamic address
> + * @I3C_SLV_MR_DONE: I3C mastership request completed successfully
> + */
> +enum i3c_event {
> + I3C_SLV_DA_UPDATE,
> + I3C_SLV_MR_DONE,
> +};
> +
> /**
> * enum i3c_addr_slot_status - I3C address slot status
> * @I3C_ADDR_SLOT_FREE: address is free
> @@ -418,6 +428,12 @@ struct i3c_bus {
> * for a future IBI
> * This method is mandatory only if ->request_ibi is not
> * NULL.
> + * @request_mastership: send mastership request to the current master
> + * @enable_mr_events: enable mastership request handling by the controller
> + * @check_event_set: check events (enum i3c_event) such as device has dynamic
> + * address, mastership request is completed successfully.
> + * @sec_mst_dyn_addr: read current dynamic address of the I3C device from
> + * hardware.
> */
> struct i3c_master_controller_ops {
> int (*bus_init)(struct i3c_master_controller *master);
> @@ -445,6 +461,11 @@ 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);
> + int (*request_mastership)(struct i3c_master_controller *master);
> + void (*enable_mr_events)(struct i3c_master_controller *m);
Those 2 helper sound good.
> + bool (*check_event_set)(struct i3c_master_controller *m,
> + enum i3c_event);
> + int (*sec_mst_dyn_addr)(struct i3c_master_controller *m);
I feel like this is HW-specific. I would just hide that in the driver
mastership-handover/DA-assignment handling. If we see enough
similarities in how other hardware handle those sequences we might want
to revise that, but I think it's a bit premature.
> };
>
> /**
> @@ -510,6 +531,8 @@ struct i3c_master_controller {
> #define i3c_bus_for_each_i3cdev(bus, dev) \
> list_for_each_entry(dev, &(bus)->devs.i3c, common.node)
>
> +void i3c_master_yield_bus(struct i3c_master_controller *m,
> + u8 sec_mst_dyn_addr);
> int i3c_master_do_i2c_xfers(struct i3c_master_controller *master,
> const struct i2c_msg *xfers,
> int nxfers);
_______________________________________________
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c
^ permalink raw reply [flat|nested] 17+ messages in thread
* RE: [PATCH v7 2/7] i3c: master: use i3c_master_register only for main master
2020-05-11 15:44 ` Boris Brezillon
@ 2020-05-12 4:57 ` Parshuram Raju Thombare
0 siblings, 0 replies; 17+ messages in thread
From: Parshuram Raju Thombare @ 2020-05-12 4:57 UTC (permalink / raw)
To: Boris Brezillon
Cc: Milind Parab, bbrezillon, praneeth, linux-kernel, vitor.soares,
Przemyslaw Gaj, linux-i3c
>> +/**
>> + * i3c_master_register() - register an I3C master
>
>The function should be renamed and the doc updated to reflect the fact
>that it only works for primary masters:
>
>i3c_primary_master_register() - register a primary I3C master
Sure, I will do that.
>> + * @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
>
>This argument no longer exists.
Thanks, I missed that comment. It should be removed.
Regards,
Parshuram Thombare
_______________________________________________
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c
^ permalink raw reply [flat|nested] 17+ messages in thread
* RE: [PATCH v7 1/7] i3c: master: secondary master initialization document
2020-05-11 16:05 ` Boris Brezillon
@ 2020-05-12 5:03 ` Parshuram Raju Thombare
2020-05-12 7:35 ` Boris Brezillon
0 siblings, 1 reply; 17+ messages in thread
From: Parshuram Raju Thombare @ 2020-05-12 5:03 UTC (permalink / raw)
To: Boris Brezillon
Cc: Milind Parab, bbrezillon, praneeth, linux-kernel, vitor.soares,
Przemyslaw Gaj, linux-i3c
>> Document describing secondary master initialization,
>> mastership handover and DEFSLVS handling processes.
>
>Thanks for doing that, but you probably didn't try to compile the doc
>(the formatting is all messed up).
>
># make htmldocs
Yes, it looks messed in email but I built html format of doc and formatting was ok.
May be because some tab/space issue it is looking messed up in email.
I will check that.
Regards,
Parshuram Thombare
_______________________________________________
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c
^ permalink raw reply [flat|nested] 17+ messages in thread
* RE: [PATCH v7 4/7] i3c: master: add mastership handover support
2020-05-11 16:38 ` Boris Brezillon
@ 2020-05-12 6:24 ` Parshuram Raju Thombare
0 siblings, 0 replies; 17+ messages in thread
From: Parshuram Raju Thombare @ 2020-05-12 6:24 UTC (permalink / raw)
To: Boris Brezillon, Przemyslaw Gaj
Cc: Milind Parab, bbrezillon, praneeth, linux-kernel, vitor.soares,
linux-i3c
>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.
Ok, I will move these pollings, check master has DA and MR done to
master driver method request_mastership. Then we will not need
check_events method in master driver. Every master driver can
handle mastership request process in request_mastership method.
>> + if (!ret)
>> + m->bus.cur_master = m->this;
>> +
>> +mr_req_done:
>> + i3c_bus_maintenance_unlock(&m->bus);
>> + i3c_bus_normaluse_lock(&m->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.
That locking/unlocking dance was apparently because caller of
acquire_bus holds read lock to avoid device on which it is
operating does not disappear or get modified due to
maintenance activities happening inside maintenance (write) lock.
Hence, here first we unlock normaluse lock and then gets
maintenance lock.
Przemek: Was there any other reason behind that ?
Yes, I agree it is safer to combine maintenance (write) unlock and then
normaluse (read) lock, into downgrading maintenance lock to normaluse
lock in single step using downgrade_lock. I will make that change.
>> +
>> + if (!master->secondary)
>> + master->bus.cur_master = master->this;
>
>This change doesn't seem related to this patch. Looks like this should
>instead go to patch 3.
This is to initialize cur_master pointer in master object pointing to
I3C object for current master on the bus. Only main master is by default
current master at the beginning, so cur_master is initialized only
for the main master. And since set_info happens in bus_init during
master initialization this change is included in this patch.
>> + /*
>> + * Maintenance lock and master check above is used to
>> + * avoid race amongst devices sending MR requests
>> + * at the same time, as soon as ENEC MST is sent by the current
>> + * master. It ensure that only one MR request is processed,
>> + * rest MR requests on losing devices will timeout in wait MR
>> + * DONE state. And next MR requests are blocked due to DISEC MST
>> + * sent by current master in yield operation.
>> + * New master should send ENEC MST once it's work is done.
>> + * maintainanace lock is also needed for i3c_master_get_accmst_locked.
>> + */
>> +
>> + ret = i3c_master_disec_locked(m, I3C_BROADCAST_ADDR,
>> + I3C_CCC_EVENT_MR |
>> + I3C_CCC_EVENT_HJ);
>
>Isn't it taken care of at the controller level? I'm fine sending it
>here, but I suspect some controllers might actually auto-disable MRs
>once they receive one.
I think auto disable MR on receiving one, handles the case of multiple
devices sending MR requests at a same time. That is handled above using
maintenance lock and current master check.
DISEC MR, HJ is to avoid new master getting interrupted by MR or HJ
before it's use of bus is done. Once it's work is done ENEC MR, HJ is
sent using i3c_master_enable_mr_events(), may be we can
rename it to i3c_master_release_bus. It is not actual hand over of bus
but just allowing other master to acquire it.
I am not disabling MR and HJ at the hardware level. But I can add that
in acquire_bus as one more level of safety, if DISEC is disregarded for
some reason by any device.
> if (WARN_ON(m->this == m->bus.cur_master)) {
>
>And you should probably check that before send DISEC.
Sure, I will do that.
Regards,
Parshuram Thombare
_______________________________________________
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c
^ permalink raw reply [flat|nested] 17+ messages in thread
* RE: [PATCH v7 3/7] i3c: master: add i3c_secondary_master_register
2020-05-11 16:14 ` Boris Brezillon
@ 2020-05-12 6:42 ` Parshuram Raju Thombare
0 siblings, 0 replies; 17+ messages in thread
From: Parshuram Raju Thombare @ 2020-05-12 6:42 UTC (permalink / raw)
To: Boris Brezillon
Cc: Milind Parab, bbrezillon, praneeth, linux-kernel, vitor.soares,
Przemyslaw Gaj, linux-i3c
>Can you really select the bus mode without knowing the I3C devices you
>have on the bus? Or maybe that's a preliminary initialization which is
>then updated when you receive DEFSLVS events.
I think we can select bus mode based on knowledge of I2C devices on the
bus. I was expecting to support different I2C devices information
on main master and secondary masters. But that seems problematic
for mastership requests. As IMO for mastership acquire and yield to work,
all master capable devices should be operating in same bus mode.
In previous patch set I tried approach of all devices using same (Pure)
bus mode during initialization and updating it on DEFSLVS reception.
But this can cause issue for hot joining devices as current master would
have already switched it's bus mode to something other than pure mode,
and hot join would not work.
To me it's seems for mastership handover to work, we need all masters
to have complete I2C device list on the bus.
>I suspect you'll have to request bus ownership first, which means
>they're not really usable, just registered to the I2C subsystem. This
>might lead to a whole bunch of problems when drivers will try to send
>messages to the I2C devices and receive ETIMEDOUT/EBUSY errors. We'll
>probably need some updates to the I2C framework if we want I2C to play
>nicely with bus handover, but I think we can keep that for later. I'd
>suggest to forget about I2C for now and mark that as a limitation (no
>I2C devs on secondary masters).
Correct, here I2C devices are just registered. Irrespective of the device type (I2C/I3C)
to which master want to communicate with, bus has to be acquired.
Regards,
Parshuram Thombare
_______________________________________________
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v7 1/7] i3c: master: secondary master initialization document
2020-05-12 5:03 ` Parshuram Raju Thombare
@ 2020-05-12 7:35 ` Boris Brezillon
0 siblings, 0 replies; 17+ messages in thread
From: Boris Brezillon @ 2020-05-12 7:35 UTC (permalink / raw)
To: Parshuram Raju Thombare
Cc: Milind Parab, bbrezillon, praneeth, linux-kernel, vitor.soares,
Przemyslaw Gaj, linux-i3c
On Tue, 12 May 2020 05:03:32 +0000
Parshuram Raju Thombare <pthombar@cadence.com> wrote:
> >> Document describing secondary master initialization,
> >> mastership handover and DEFSLVS handling processes.
> >
> >Thanks for doing that, but you probably didn't try to compile the doc
> >(the formatting is all messed up).
> >
> ># make htmldocs
>
> Yes, it looks messed in email but I built html format of doc and formatting was ok.
I did build the html doc and it's not ok here.
> May be because some tab/space issue it is looking messed up in email.
No, it's really the html output I'm complaining about.
_______________________________________________
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c
^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2020-05-12 7:36 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-11 13:11 [PATCH v7 0/7] I3C mastership handover support Parshuram Thombare
2020-05-11 13:12 ` [PATCH v7 1/7] i3c: master: secondary master initialization document Parshuram Thombare
2020-05-11 16:05 ` Boris Brezillon
2020-05-12 5:03 ` Parshuram Raju Thombare
2020-05-12 7:35 ` Boris Brezillon
2020-05-11 13:13 ` [PATCH v7 2/7] i3c: master: use i3c_master_register only for main master Parshuram Thombare
2020-05-11 15:44 ` Boris Brezillon
2020-05-12 4:57 ` Parshuram Raju Thombare
2020-05-11 13:13 ` [PATCH v7 3/7] i3c: master: add i3c_secondary_master_register Parshuram Thombare
2020-05-11 16:14 ` Boris Brezillon
2020-05-12 6:42 ` Parshuram Raju Thombare
2020-05-11 13:14 ` [PATCH v7 4/7] i3c: master: add mastership handover support Parshuram Thombare
2020-05-11 16:38 ` Boris Brezillon
2020-05-12 6:24 ` Parshuram Raju Thombare
2020-05-11 13:15 ` [PATCH v7 5/7] i3c: master: add defslvs processing Parshuram Thombare
2020-05-11 13:16 ` [PATCH v7 6/7] i3c: master: sysfs key for acquire bus Parshuram Thombare
2020-05-11 13:17 ` [PATCH v7 7/7] i3c: master: mastership handover, defslvs processing in cdns controller driver Parshuram Thombare
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).