[PATCH v10 0/7] I3C mastership handover support

[PATCH v10 0/7] I3C mastership handover support

[Parshuram Thombare]

Main changes between v9 and v10 are:
- Fix build failure reported by kernel test robot

Main changes between v8 and v9 are:
- Fix NULL dereference issue in current_master_show when
  cat'ing sysfs key current_master for secondary master
  before primary master gets initialized.

Main changes between v7 and v8 are:
- Document format changed from table to DOT diagram
- Appropriate names for few functions
- Moved mastership request process entirely to the driver
- Reuse of i3c_master_add_i3c_dev_locked in core defslvs
  processing

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

Parshuram Thombare (7):
  i3c: master: master initialization flow 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 +
 .../driver-api/i3c/master-initialization-flow.rst  |  187 ++++++++
 drivers/i3c/master.c                               |  497 ++++++++++++++++++--
 drivers/i3c/master/dw-i3c-master.c                 |    4 +-
 drivers/i3c/master/i3c-master-cdns.c               |  329 ++++++++++++-
 drivers/i3c/master/mipi-i3c-hci/core.c             |    4 +-
 include/linux/i3c/master.h                         |   23 +-
 7 files changed, 970 insertions(+), 75 deletions(-)
 create mode 100644 Documentation/driver-api/i3c/master-initialization-flow.rst


-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

[PATCH v10 1/7] i3c: master: master initialization flow document

[Parshuram Thombare]

Document describing master initialization, mastership handover
and DEFSLVS handling processes.

Signed-off-by: Parshuram Thombare <pthombar@cadence.com>
---
 Documentation/driver-api/i3c/index.rst             |    1 +
 .../driver-api/i3c/master-initialization-flow.rst  |  187 ++++++++++++++++++++
 2 files changed, 188 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/driver-api/i3c/master-initialization-flow.rst

diff --git a/Documentation/driver-api/i3c/index.rst b/Documentation/driver-api/i3c/index.rst
index 783d6da..604f1c5 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
+   master-initialization-flow
diff --git a/Documentation/driver-api/i3c/master-initialization-flow.rst b/Documentation/driver-api/i3c/master-initialization-flow.rst
new file mode 100644
index 0000000..e02b90d
--- /dev/null
+++ b/Documentation/driver-api/i3c/master-initialization-flow.rst
@@ -0,0 +1,187 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+==============================
+I3C Master Initialization Flow
+==============================
+
+.. kernel-render:: DOT
+   :alt: I3C Master Initialization digraph
+   :caption: I3C Master Initialization Flow
+
+   digraph master_init {
+       ranksep=.25; size="35,35";
+       subgraph cluster_0 {
+           style=dashed
+           x0[shape=ellipse, label="I3C driver probe", style="filled"]
+           x1[shape=diamond, label="Secondary Master ?"]
+           a1[shape=box, label="Do I3C master controller specific initialization"]
+           b1[shape=box, label="Do I3C master controller specific initialization,\n
+           except enabling the DEFSLVS interrupt."]
+           a2[shape=box, label="Call i3c_primary_master_register"]
+           b2[shape=box, label="Call i3c_secondary_master_register"]
+           x0->x1;
+           x1->a1[label="No"];
+           x1->b1[label="Yes"];
+           a1->a2; b1->b2;
+       }
+
+       subgraph cluster_1 {
+           style=dashed
+           label="Current Master";
+           a3[shape=ellipse, label="i3c_primary_master_register", style="filled"]
+           a4[shape=box, label="Initialize I3C master controller object."]
+           a5[shape=box, label="Create I2C objects for devices in DTS and add to I2C list."]
+           a6[shape=box, label="Set appropriate bus mode based on I2C devices information."]
+           a7[shape=box, label="Create a work queue."]
+           a8[shape=box, label="Call i3c_primary_master_bus_init"]
+           a19[shape=box, label="Add I3C object representing this master to the system."]
+           a20[shape=box, label="Expose our I3C bus as an I2C adapter so that I2C devices\n
+           are exposed through the I2C subsystem."]
+           a21[shape=box, label="Register all I3C devices."]
+           a22[shape=box, label="Broadcast ENEC MR, HJ message."]
+           a3->a4->a5->a6->a7->a8; a19->a20->a21->a22;
+           a8->a19[style="invisible"];
+       }
+
+       subgraph cluster_2 {
+           style=dashed
+           label="Current Master";
+           a9[shape=ellipse, label="i3c_primary_master_bus_init", style="filled"]
+           a10[shape=box, label="Call bus_init to do controller specific bus initialization\n
+           and enabling the controller."]
+           a11[shape=box, label="Create I3C object representing the master and add it to\n
+           the I3C device list."]
+           a12[shape=box, label="Set current master to the object created to represent I3C\n
+           master device."]
+           a13[shape=box, label="Reset all dynamic address that may have been assigned before."]
+           a14[shape=box, label="Disable all slave events before starting DAA."]
+           a15[shape=box, label="Pre-assign dynamic address and retrieve device information."]
+           a16[shape=box, label="Do dynamic address assignment to all I3C devices currenly\n
+           present on the bus."]
+           a17[shape=box, label="Create I3C objects representing devices found during DAA."]
+           a18[shape=box, label="Send DEFSVLS message containing information about all\n
+           known I3C and I2C devices."]
+           a9->a10->a11->a12->a13->a14->a15->a16->a17->a18;
+       }
+
+       subgraph cluster_3 {
+           style=dashed
+           label="Current Master";
+           h1[shape=ellipse, label="MR request interrupt", style="filled"]
+           h2[shape=box, label="Bottom half i3c_master_yield_bus"]
+           h1->h2;
+       }
+
+       subgraph cluster_4 {
+           style=dashed
+           label="Current Master";
+           i1[shape=ellipse, label="i3c_master_yield_bus", style="filled"]
+           i2[shape=box, label="Check if this device is still a master to handle a case of\n
+           multiple MR requests from different devices at a same time."]
+           i3[shape=box, label="Broadcast DISEC MR, HJ message.\n
+           New master should broadcast ENEC MR, HJ once its usage of bus is done."]
+           i4[shape=box, label="Get accept mastership acknowldege from requesting master."]
+           i5[shape=box, label="In case of failure reenable MR requests by broadcasting ENEC MR, HJ."]
+           i6[shape=box, label="Mastership hand over is done."]
+           i1->i2->i3->i4->i5->i6;
+       }
+
+       subgraph cluster_5 {
+           style=dashed
+           b3[shape=ellipse, label="i3c_secondary_master_register", style="filled"]
+           b4[shape=box, label="Initialize I3C master controller object."]
+           b5[shape=box, label="Create I2C objects for devices in DTS and add to I2C list."]
+           b6[shape=box, label="Set appropriate bus mode based on I2C devices information."]
+           b7[shape=box, label="Create a work queue."]
+           b8[shape=box, label="Call i3c_secondary_master_bus_init."]
+           b12[shape=box, label="Allocate memory for defslvs_data, that will be used to pass I3C\n
+           device list received in DEFSLVS to the I3C core DEFSLVS processing."]
+           b13[shape=box, label="Add I3C object representing this master to the system."]
+           b14[shape=box, label="Expose our I3C bus as an I2C adapter so that I2C devices are\n
+           exposed through the I2C subsystem."]
+           b3->b4->b5->b6->b7->b8; b12->b13->b14;
+           b8->b12[style="invisible"]
+       }
+
+       subgraph cluster_6 {
+           style=dashed
+           b9[shape=ellipse, label="i3c_secondary_master_bus_init", style="filled"]
+           b10[shape=box, label="Call bus_init to do controller specific bus initialization\n
+           and enabling the controller."]
+           b11[shape=box, label="Create I3C object representing the master and add it to\n
+           the I3C device list."]
+           b9->b10->b11;
+       }
+
+       subgraph cluster_7 {
+           style=dashed
+           d1[shape=ellipse, label="DEFSLVS interrupt", style="filled"]
+           d2[shape=box, label="Controller driver can chose how to handle I2C devices received\n
+           in DEFSLVS."]
+           d3[shape=box, label="Read all I3C devices information from DEFSLVS message\n
+           to defslvs_data structure in the master object."]
+           d4[shape=box, label="Call i3c_master_process_defslvs."]
+           d1->d2->d3->d4;
+       }
+
+       subgraph cluster_8 {
+           style=dashed
+           d5[shape=ellipse, label="i3c_master_process_defslvs", style="filled"]
+           d6[shape=box, label="Acquire I3C bus mastership."]
+           d7[shape=diamond, label="Bus acquired ?"]
+           d8[shape=box, label="Handle I3C device list from DEFSLVS."]
+           d9[shape=box, label="Call i3c_master_populate_bus"]
+           d10[shape=box, label="Queue defslvs processing task for retry."]
+           d5->d6;
+           d7->d8[label="Yes"];
+           d8->d9;
+           d7->d10[label="No"];
+           d6->d7[style="invisible"];
+           d10->d5[style="dotted", constraint=false];
+       }
+
+       subgraph cluster_9 {
+           style=dashed
+           e1[shape=ellipse, label="i3c_master_acquire_bus", style="filled"]
+           e2[shape=box, label="If device is already holding the mastership,\n
+           just broadcast DISEC MR, HJ message and return success."]
+           e3[shape=box, label="Call request_mastership method"]
+           e1->e2->e3;
+       }
+
+       subgraph cluster_10 {
+           style=dashed
+           f1[shape=ellipse, label="request_mastership", style="filled"]
+           f2[shape=box, label="Return success if device is already in master mode."]
+           f3[shape=box, label="Return error if device don't have dyn_addr."]
+           f4[shape=box, label="Return error if MR requests are disabled by current master."]
+           f5[shape=box, label="Send MR request"]
+           f6[shape=box, label="Wait with timeout until MR_DONE interrupt is received."]
+           f1->f2->f3->f4->f5->f6;
+       }
+
+       subgraph cluster_11 {
+           style=dashed
+           g1[shape=ellipse, label="i3c_master_populate_bus", style="filled"]
+           g2[shape=box, label="Free up all I3C addresses to handle address\n
+           re assignment by main master."]
+           g3[shape=box, label="Master acquire dyn_addr received from the driver."]
+           g4[shape=box, label="For every I3C device in the defslvs_data\n
+           call i3c_master_add_i3c_dev_locked."]
+           g1->g2->g3->g4;
+       }
+
+       a2->a3; a8->a9; a18->a19;
+       b2->b3; b8->b9; b11->b12;
+       b14->d1[style=invisible];
+       a22->h1[style=invisible];
+       d4->d5;
+       d6->e1;
+       e3->f1;
+       f6->d7[constraint=false];
+       h2->i1;
+       d9->g1;
+       a18->d1[style="dotted", constraint=false];
+       f5->h1[style="dotted", constraint=false];
+       i4->f6[style="dotted", constraint=false];
+   }
-- 
1.7.1


-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

[PATCH v10 2/7] i3c: master: use i3c_master_register only for main master

[Parshuram Thombare]

Removed last argument 'secondary' and restructured 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                   |   78 +++++++++++++++++++-------------
 drivers/i3c/master/dw-i3c-master.c     |    4 +-
 drivers/i3c/master/i3c-master-cdns.c   |    4 +-
 drivers/i3c/master/mipi-i3c-hci/core.c |    4 +-
 include/linux/i3c/master.h             |    7 +--
 5 files changed, 56 insertions(+), 41 deletions(-)

diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
index b61bf53..56e8fe4 100644
--- a/drivers/i3c/master.c
+++ b/drivers/i3c/master.c
@@ -1637,7 +1637,7 @@ static void i3c_master_detach_free_devs(struct i3c_master_controller *master)
 }
 
 /**
- * i3c_master_bus_init() - initialize an I3C bus
+ * i3c_primary_master_bus_init() - initialize an I3C bus
  * @master: main master initializing the bus
  *
  * This function is following all initialisation steps described in the I3C
@@ -1668,7 +1668,7 @@ static void i3c_master_detach_free_devs(struct i3c_master_controller *master)
  *
  * Return: a 0 in case of success, an negative error code otherwise.
  */
-static int i3c_master_bus_init(struct i3c_master_controller *master)
+static int i3c_primary_master_bus_init(struct i3c_master_controller *master)
 {
 	enum i3c_addr_slot_status status;
 	struct i2c_dev_boardinfo *i2cboardinfo;
@@ -2441,31 +2441,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)
 {
 	unsigned long i2c_scl_rate = I3C_BUS_I2C_FM_PLUS_SCL_RATE;
 	struct i3c_bus *i3cbus = i3c_master_get_bus(master);
@@ -2535,10 +2514,49 @@ int i3c_master_register(struct i3c_master_controller *master,
 		goto err_put_dev;
 	}
 
-	ret = i3c_master_bus_init(master);
+	ret = i3c_primary_master_bus_init(master);
 	if (ret)
 		goto err_destroy_wq;
 
+	return 0;
+
+err_destroy_wq:
+	destroy_workqueue(master->wq);
+
+err_put_dev:
+	put_device(&master->dev);
+
+	return ret;
+}
+
+/**
+ * i3c_primary_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
+ *
+ * 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_primary_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;
@@ -2568,15 +2586,13 @@ int i3c_master_register(struct i3c_master_controller *master,
 err_cleanup_bus:
 	i3c_master_bus_cleanup(master);
 
-err_destroy_wq:
 	destroy_workqueue(master->wq);
 
-err_put_dev:
 	put_device(&master->dev);
 
 	return ret;
 }
-EXPORT_SYMBOL_GPL(i3c_master_register);
+EXPORT_SYMBOL_GPL(i3c_primary_master_register);
 
 /**
  * i3c_master_unregister() - unregister an I3C master
diff --git a/drivers/i3c/master/dw-i3c-master.c b/drivers/i3c/master/dw-i3c-master.c
index 8513bd3..4f4d41f 100644
--- a/drivers/i3c/master/dw-i3c-master.c
+++ b/drivers/i3c/master/dw-i3c-master.c
@@ -1166,8 +1166,8 @@ static int dw_i3c_probe(struct platform_device *pdev)
 	master->maxdevs = ret >> 16;
 	master->free_pos = GENMASK(master->maxdevs - 1, 0);
 
-	ret = i3c_master_register(&master->base, &pdev->dev,
-				  &dw_mipi_i3c_ops, false);
+	ret = i3c_primary_master_register(&master->base, &pdev->dev,
+					  &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 3f22269..f1d6d68 100644
--- a/drivers/i3c/master/i3c-master-cdns.c
+++ b/drivers/i3c/master/i3c-master-cdns.c
@@ -1644,8 +1644,8 @@ static int cdns_i3c_master_probe(struct platform_device *pdev)
 	writel(MST_INT_IBIR_THR, 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, false);
+	ret = i3c_primary_master_register(&master->base, &pdev->dev,
+					  &cdns_i3c_master_ops);
 	if (ret)
 		goto err_disable_sysclk;
 
diff --git a/drivers/i3c/master/mipi-i3c-hci/core.c b/drivers/i3c/master/mipi-i3c-hci/core.c
index 113c4c9..b288245 100644
--- a/drivers/i3c/master/mipi-i3c-hci/core.c
+++ b/drivers/i3c/master/mipi-i3c-hci/core.c
@@ -757,8 +757,8 @@ static int i3c_hci_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
-	ret = i3c_master_register(&hci->master, &pdev->dev,
-				  &i3c_hci_ops, false);
+	ret = i3c_primary_master_register(&hci->master, &pdev->dev,
+					  &i3c_hci_ops);
 	if (ret)
 		return ret;
 
diff --git a/include/linux/i3c/master.h b/include/linux/i3c/master.h
index 9cb39d9..e543f68 100644
--- a/include/linux/i3c/master.h
+++ b/include/linux/i3c/master.h
@@ -538,10 +538,9 @@ int i3c_master_add_i3c_dev_locked(struct i3c_master_controller *master,
 int i3c_master_set_info(struct i3c_master_controller *master,
 			const struct i3c_device_info *info);
 
-int i3c_master_register(struct i3c_master_controller *master,
-			struct device *parent,
-			const struct i3c_master_controller_ops *ops,
-			bool secondary);
+int i3c_primary_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);
 
 /**
-- 
1.7.1


-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

[PATCH v10 3/7] i3c: master: add i3c_secondary_master_register

[Parshuram Thombare]

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 deletions(-)

diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
index 56e8fe4..af0630a 100644
--- a/drivers/i3c/master.c
+++ b/drivers/i3c/master.c
@@ -1797,6 +1797,90 @@ static int i3c_primary_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)
@@ -2514,7 +2598,10 @@ static int i3c_master_init(struct i3c_master_controller *master,
 		goto err_put_dev;
 	}
 
-	ret = i3c_primary_master_bus_init(master);
+	if (secondary)
+		ret = i3c_secondary_master_bus_init(master);
+	else
+		ret = i3c_primary_master_bus_init(master);
 	if (ret)
 		goto err_destroy_wq;
 
@@ -2595,6 +2682,71 @@ int i3c_primary_master_register(struct i3c_master_controller *master,
 EXPORT_SYMBOL_GPL(i3c_primary_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 e543f68..e186d53 100644
--- a/include/linux/i3c/master.h
+++ b/include/linux/i3c/master.h
@@ -541,6 +541,9 @@ int i3c_master_set_info(struct i3c_master_controller *master,
 int i3c_primary_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);
 
 /**
-- 
1.7.1


-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

[PATCH v10 4/7] i3c: master: add mastership handover support

[Parshuram Thombare]

Added mastership acquire and yield functions.

Signed-off-by: Parshuram Thombare <pthombar@cadence.com>
---
 drivers/i3c/master.c       |  183 +++++++++++++++++++++++++++++++++++++++++---
 include/linux/i3c/master.h |    6 ++
 2 files changed, 177 insertions(+), 12 deletions(-)

diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
index af0630a..51ef706 100644
--- a/drivers/i3c/master.c
+++ b/drivers/i3c/master.c
@@ -44,6 +44,16 @@ static void i3c_bus_maintenance_lock(struct i3c_bus *bus)
 }
 
 /**
+ * i3c_bus_maintenance_downgrade_lock - Downgrade maintenance lock to
+ * normaluse lock.
+ * @bus: I3C bus to take the lock on
+ */
+static void i3c_bus_maintenance_downgrade_lock(struct i3c_bus *bus)
+{
+	downgrade_write(&bus->lock);
+}
+
+/**
  * i3c_bus_maintenance_unlock - Release the bus lock after a maintenance
  *			      operation
  * @bus: I3C bus to release the lock on
@@ -451,6 +461,59 @@ static int i3c_bus_init(struct i3c_bus *i3cbus)
 	[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;
+}
+
+/**
+ * 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;
+
+	/*
+	 * 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 = m->ops->request_mastership(m);
+	if (ret)
+		goto mr_req_done;
+
+	m->bus.cur_master = m->this;
+
+mr_req_done:
+	i3c_bus_maintenance_downgrade_lock(&m->bus);
+	return ret;
+}
+
 static ssize_t mode_show(struct device *dev,
 			 struct device_attribute *da,
 			 char *buf)
@@ -476,11 +539,12 @@ static ssize_t current_master_show(struct device *dev,
 				   char *buf)
 {
 	struct i3c_bus *i3cbus = dev_to_i3cbus(dev);
-	ssize_t ret;
+	ssize_t ret = 0;
 
 	i3c_bus_normaluse_lock(i3cbus);
-	ret = sprintf(buf, "%d-%llx\n", i3cbus->id,
-		      i3cbus->cur_master->info.pid);
+	if (i3cbus->cur_master)
+		ret = sprintf(buf, "%d-%llx\n", i3cbus->id,
+			      i3cbus->cur_master->info.pid);
 	i3c_bus_normaluse_unlock(i3cbus);
 
 	return ret;
@@ -690,6 +754,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)
@@ -1582,10 +1673,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;
 
@@ -1594,7 +1681,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)
@@ -1637,6 +1726,74 @@ 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;
+	bool dev_found =  false;
+	int ret = 0;
+
+	i3c_bus_maintenance_lock(&m->bus);
+	if (m->this != m->bus.cur_master)
+		goto mr_yield_done;
+
+	i3c_bus_for_each_i3cdev(&m->bus, i3cdev) {
+		if (sec_mst_dyn_addr == i3cdev->info.dyn_addr) {
+			dev_found = true;
+			break;
+		}
+	}
+
+	/* Requesting device not found on i3c list. This should never happen. */
+	if (!dev_found)
+		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;
+
+	m->bus.cur_master = i3cdev;
+
+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_primary_master_bus_init() - initialize an I3C bus
  * @master: main master initializing the bus
  *
@@ -2522,6 +2679,9 @@ 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)
+		return -EINVAL;
+
 	return 0;
 }
 
@@ -2536,10 +2696,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;
@@ -2665,6 +2821,9 @@ int i3c_primary_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 e186d53..77c0422 100644
--- a/include/linux/i3c/master.h
+++ b/include/linux/i3c/master.h
@@ -426,6 +426,8 @@ 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
  */
 struct i3c_master_controller_ops {
 	int (*bus_init)(struct i3c_master_controller *master);
@@ -452,6 +454,8 @@ 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);
 };
 
 /**
@@ -517,6 +521,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);
-- 
1.7.1


-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

[PATCH v10 5/7] i3c: master: add defslvs processing

[Parshuram Thombare]

Added defslvs processing code to the I3C master subsystem.

Signed-off-by: Parshuram Thombare <pthombar@cadence.com>
---
 drivers/i3c/master.c       |   68 ++++++++++++++++++++++++++++++++++++++++++-
 include/linux/i3c/master.h |    7 ++++
 2 files changed, 73 insertions(+), 2 deletions(-)

diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
index 51ef706..c01ba00 100644
--- a/drivers/i3c/master.c
+++ b/drivers/i3c/master.c
@@ -2172,7 +2172,7 @@ int i3c_master_add_i3c_dev_locked(struct i3c_master_controller *master,
 	else
 		expected_dyn_addr = newdev->info.dyn_addr;
 
-	if (newdev->info.dyn_addr != expected_dyn_addr) {
+	if (!master->secondary && newdev->info.dyn_addr != expected_dyn_addr) {
 		/*
 		 * Try to apply the expected dynamic address. If it fails, keep
 		 * the address assigned by the master.
@@ -2867,12 +2867,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;
@@ -2905,6 +2913,62 @@ 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, u8 dyn_addr)
+{
+	struct i3c_dev_desc *olddev;
+	struct i3c_ccc_dev_desc *desc;
+	int ret, slot;
+
+	i3c_bus_for_each_i3cdev(&master->bus, olddev)
+		i3c_master_put_i3c_addrs(olddev);
+
+	master->this->info.dyn_addr = dyn_addr;
+	i3c_master_get_i3c_addrs(master->this);
+
+	desc = master->defslvs_data.devs;
+	for (slot = 1; slot < master->defslvs_data.ndevs; slot++) {
+		ret = i3c_master_add_i3c_dev_locked(master,
+						    desc[slot].dyn_addr);
+		if (ret)
+			break;
+	}
+
+	return ret;
+}
+
+/**
+ * i3c_master_process_defslvs() - process I3C device list received in
+ * DEFSLVS for device plug/unplug and address change.
+ * @m: I3C master object
+ * @dyn_addr: Current dynamic address of this device
+ *
+ * This function may sleep, so should not be called in the atomic context.
+ */
+int i3c_master_process_defslvs(struct i3c_master_controller *m, u8 dyn_addr)
+{
+	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, dyn_addr);
+	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 77c0422..835f823 100644
--- a/include/linux/i3c/master.h
+++ b/include/linux/i3c/master.h
@@ -478,6 +478,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
@@ -497,6 +499,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;
 };
 
 /**
@@ -523,6 +529,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, u8 dyn_addr);
 int i3c_master_do_i2c_xfers(struct i3c_master_controller *master,
 			    const struct i2c_msg *xfers,
 			    int nxfers);
-- 
1.7.1


-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

[PATCH v10 6/7] i3c: master: sysfs key for acquire bus

[Parshuram Thombare]

Added support to acquire I3C bus through sysfs interface.

Signed-off-by: Parshuram Thombare <pthombar@cadence.com>
---
 drivers/i3c/master.c |   18 ++++++++++++++++++
 1 files changed, 18 insertions(+), 0 deletions(-)

diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
index c01ba00..beb7495 100644
--- a/drivers/i3c/master.c
+++ b/drivers/i3c/master.c
@@ -581,6 +581,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,
@@ -591,6 +608,7 @@ static ssize_t i2c_scl_frequency_show(struct device *dev,
 	&dev_attr_pid.attr,
 	&dev_attr_dynamic_address.attr,
 	&dev_attr_hdrcap.attr,
+	&dev_attr_i3c_acquire_bus.attr,
 	NULL,
 };
 ATTRIBUTE_GROUPS(i3c_masterdev);
-- 
1.7.1


-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

[PATCH v10 7/7] i3c: master: mastership handover, defslvs processing in cdns controller driver

[Parshuram Thombare]

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 |  329 +++++++++++++++++++++++++++++++---
 1 files changed, 306 insertions(+), 23 deletions(-)

diff --git a/drivers/i3c/master/i3c-master-cdns.c b/drivers/i3c/master/i3c-master-cdns.c
index f1d6d68..ff07862 100644
--- a/drivers/i3c/master/i3c-master-cdns.c
+++ b/drivers/i3c/master/i3c-master-cdns.c
@@ -160,6 +160,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)
@@ -192,7 +193,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)
@@ -397,6 +398,9 @@ struct cdns_i3c_data {
 
 struct cdns_i3c_master {
 	struct work_struct hj_work;
+	struct work_struct mr_yield_work;
+	struct work_struct defslvs_work;
+	struct completion mr_done;
 	struct i3c_master_controller base;
 	u32 free_rr_slots;
 	unsigned int maxdevs;
@@ -416,6 +420,7 @@ struct cdns_i3c_master {
 	struct cdns_i3c_master_caps caps;
 	unsigned long i3c_scl_lim;
 	const struct cdns_i3c_data *devdata;
+	u8 mr_addr;
 };
 
 static inline struct cdns_i3c_master *
@@ -1182,10 +1187,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;
 }
 
@@ -1208,21 +1209,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:
@@ -1253,7 +1254,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);
@@ -1261,7 +1261,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);
@@ -1279,13 +1278,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;
 
 	/*
 	 * Configure data hold delay based on device-specific data.
@@ -1358,6 +1365,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);
@@ -1379,6 +1387,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;
 		}
@@ -1390,16 +1406,42 @@ 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);
+			complete(&master->mr_done);
+		}
 
-	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;
 }
@@ -1523,6 +1565,106 @@ 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_HAS_DA))
+		return -EACCES;
+
+	if (status & SLV_STATUS1_MR_DIS)
+		return -EACCES;
+
+	writel(readl(master->regs + CTRL) | CTRL_MST_INIT | CTRL_MST_ACK,
+	       master->regs + CTRL);
+
+	init_completion(&master->mr_done);
+	if (!wait_for_completion_timeout(&master->mr_done,
+					 msecs_to_jiffies(1000)))
+		return -ETIMEDOUT;
+
+	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 const struct i3c_master_controller_ops cdns_i3c_master_ops = {
 	.bus_init = cdns_i3c_master_bus_init,
 	.bus_cleanup = cdns_i3c_master_bus_cleanup,
@@ -1541,6 +1683,8 @@ static void cdns_i3c_master_recycle_ibi_slot(struct i3c_dev_desc *dev,
 	.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,
 };
 
 static void cdns_i3c_master_hj(struct work_struct *work)
@@ -1561,9 +1705,134 @@ static void cdns_i3c_master_hj(struct work_struct *work)
 	{ /* sentinel */ },
 };
 
+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;
+	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++;
+		}
+	}
+
+	val = readl(master->regs + SLV_STATUS1);
+	if (!(val & SLV_STATUS1_HAS_DA) ||
+	    i3c_master_process_defslvs(m, SLV_STATUS1_DA(val))) {
+		queue_work(master->base.wq, work);
+		return;
+	}
+}
+
 static int cdns_i3c_master_probe(struct platform_device *pdev)
 {
 	struct cdns_i3c_master *master;
+	bool secondary;
 	int ret, irq;
 	u32 val;
 
@@ -1607,6 +1876,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);
@@ -1618,6 +1888,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);
@@ -1640,12 +1911,24 @@ static int cdns_i3c_master_probe(struct platform_device *pdev)
 		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_primary_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_primary_master_register(&master->base, &pdev->dev,
+						  &cdns_i3c_master_ops);
+	}
+
 	if (ret)
 		goto err_disable_sysclk;
 
-- 
1.7.1


-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

Re: [PATCH v10 2/7] i3c: master: use i3c_master_register only for main master

[Nicolas Pitre]

On Mon, 30 Nov 2020, Parshuram Thombare wrote:

> Removed last argument 'secondary' and restructured 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>

[...]

> +static int i3c_master_init(struct i3c_master_controller *master,
> +			   struct device *parent,
> +			   const struct i3c_master_controller_ops *ops,
> +			   bool secondary)
>  {
>  	unsigned long i2c_scl_rate = I3C_BUS_I2C_FM_PLUS_SCL_RATE;
>  	struct i3c_bus *i3cbus = i3c_master_get_bus(master);
> @@ -2535,10 +2514,49 @@ int i3c_master_register(struct i3c_master_controller *master,
>  		goto err_put_dev;
>  	}
>  
> -	ret = i3c_master_bus_init(master);
> +	ret = i3c_primary_master_bus_init(master);
>  	if (ret)
>  		goto err_destroy_wq;
>  
> +	return 0;
> +
> +err_destroy_wq:
> +	destroy_workqueue(master->wq);
> +
> +err_put_dev:
> +	put_device(&master->dev);
> +
> +	return ret;
> +}

[...]

> +int i3c_primary_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;
> @@ -2568,15 +2586,13 @@ int i3c_master_register(struct i3c_master_controller *master,
>  err_cleanup_bus:
>  	i3c_master_bus_cleanup(master);
>  
> -err_destroy_wq:
>  	destroy_workqueue(master->wq);
>  
> -err_put_dev:
>  	put_device(&master->dev);
>  
>  	return ret;
>  }

This looks a bit confusing. Here you're rolling back detailss in 
i3c_primary_master_register() that were factored out in 
i3c_master_init(). If i3c_master_init() is successful, then you 
shouldn't be undoing its things openly in i3c_primary_master_register(). 
Instead, there should be another function that does the reverse of 
i3c_master_init() here.


Nicolas

-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

Re: [PATCH v10 3/7] i3c: master: add i3c_secondary_master_register

[Nicolas Pitre]

On Mon, 30 Nov 2020, Parshuram Thombare wrote:

> add i3c_secondary_master_register which is used
> to register secondary masters.

I'm not sure about the logic here. Why would the secondary master 
initialize the bus? If you make a distinction between primary and 
secondary, then the primary should be the owner of the bus and it should 
have enumerated it already. You should populate the bus structure with 
info provided by the primary master not from DT?


> 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 deletions(-)
> 
> diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
> index 56e8fe4..af0630a 100644
> --- a/drivers/i3c/master.c
> +++ b/drivers/i3c/master.c
> @@ -1797,6 +1797,90 @@ static int i3c_primary_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)
> @@ -2514,7 +2598,10 @@ static int i3c_master_init(struct i3c_master_controller *master,
>  		goto err_put_dev;
>  	}
>  
> -	ret = i3c_primary_master_bus_init(master);
> +	if (secondary)
> +		ret = i3c_secondary_master_bus_init(master);
> +	else
> +		ret = i3c_primary_master_bus_init(master);
>  	if (ret)
>  		goto err_destroy_wq;
>  
> @@ -2595,6 +2682,71 @@ int i3c_primary_master_register(struct i3c_master_controller *master,
>  EXPORT_SYMBOL_GPL(i3c_primary_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 e543f68..e186d53 100644
> --- a/include/linux/i3c/master.h
> +++ b/include/linux/i3c/master.h
> @@ -541,6 +541,9 @@ int i3c_master_set_info(struct i3c_master_controller *master,
>  int i3c_primary_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);
>  
>  /**
> -- 
> 1.7.1
> 
> 
> -- 
> linux-i3c mailing list
> linux-i3c@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-i3c
> 

-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

RE: [PATCH v10 2/7] i3c: master: use i3c_master_register only for main master

[Parshuram Raju Thombare]

>This looks a bit confusing. Here you're rolling back detailss in
>i3c_primary_master_register() that were factored out in
>i3c_master_init(). If i3c_master_init() is successful, then you
>shouldn't be undoing its things openly in i3c_primary_master_register().
>Instead, there should be another function that does the reverse of
>i3c_master_init() here.

Every function do its cleanup in case of failures.
And if any failure occur after successful i3c_master_init(), we have
function i3c_master_bus_cleanup() which does the major cleanup.

Regards,
Parshuram Thombare

-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

RE: [PATCH v10 3/7] i3c: master: add i3c_secondary_master_register

[Parshuram Raju Thombare]

>I'm not sure about the logic here. Why would the secondary master
>initialize the bus? If you make a distinction between primary and
>secondary, then the primary should be the owner of the bus and it should
>have enumerated it already. You should populate the bus structure with
>info provided by the primary master not from DT?

Here the bus initialization means programming HW for communicating over
I3C bus and initializing i3c_master_controller object, which is needed for both
primary and secondary masters. Yes, primary master is initial bus master,
it assign addresses to each device detected on I3C bus in DAA and broadcast
the list through DEFSLVS, which is used by secondary masters during their
remaining initialization. 

Initial approach was to allow secondary master to get information about
devices on bus from DEFSLVS only, but it was later decided that secondary
master should parse DT as well for any I2C and I3C device information. 

Regards,
Parshuram Thombare

-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

RE: [PATCH v10 0/7] I3C mastership handover support

[Parshuram Raju Thombare]

Ping !

>-----Original Message-----
>From: Parshuram Thombare <pthombar@cadence.com>
>Sent: Monday, November 30, 2020 11:46 AM
>To: alexandre.belloni@bootlin.com; slongerbeam@gmail.com;
>vitor.soares@synopsys.com
>Cc: linux-i3c@lists.infradead.org; linux-kernel@vger.kernel.org; Milind Parab
><mparab@cadence.com>; praneeth@ti.com; Parshuram Raju Thombare
><pthombar@cadence.com>
>Subject: [PATCH v10 0/7] I3C mastership handover support
>
>Main changes between v9 and v10 are:
>- Fix build failure reported by kernel test robot
>
>Main changes between v8 and v9 are:
>- Fix NULL dereference issue in current_master_show when
>  cat'ing sysfs key current_master for secondary master
>  before primary master gets initialized.
>
>Main changes between v7 and v8 are:
>- Document format changed from table to DOT diagram
>- Appropriate names for few functions
>- Moved mastership request process entirely to the driver
>- Reuse of i3c_master_add_i3c_dev_locked in core defslvs
>  processing
>
>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
>
>Parshuram Thombare (7):
>  i3c: master: master initialization flow 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 +
> .../driver-api/i3c/master-initialization-flow.rst  |  187 ++++++++
> drivers/i3c/master.c                               |  497 ++++++++++++++++++--
> drivers/i3c/master/dw-i3c-master.c                 |    4 +-
> drivers/i3c/master/i3c-master-cdns.c               |  329 ++++++++++++-
> drivers/i3c/master/mipi-i3c-hci/core.c             |    4 +-
> include/linux/i3c/master.h                         |   23 +-
> 7 files changed, 970 insertions(+), 75 deletions(-)
> create mode 100644 Documentation/driver-api/i3c/master-initialization-flow.rst


-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

Re: [PATCH v10 2/7] i3c: master: use i3c_master_register only for main master

[Alexandre Belloni]

On 08/12/2020 05:47:51+0000, Parshuram Raju Thombare wrote:
> >This looks a bit confusing. Here you're rolling back detailss in
> >i3c_primary_master_register() that were factored out in
> >i3c_master_init(). If i3c_master_init() is successful, then you
> >shouldn't be undoing its things openly in i3c_primary_master_register().
> >Instead, there should be another function that does the reverse of
> >i3c_master_init() here.
> 
> Every function do its cleanup in case of failures.
> And if any failure occur after successful i3c_master_init(), we have
> function i3c_master_bus_cleanup() which does the major cleanup.
> 

The point from Nicolas here was that the workqueue is allocated in
i3c_master_init so you should have a function to destroy it instead of
having to do that separately in i3c_primary_master_register. The same is
true for the put_device. Or you have to ensure i3c_masterdev_release
is called when i3c_primary_master_register fails.

-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

Re: [PATCH v10 6/7] i3c: master: sysfs key for acquire bus

[Alexandre Belloni]

On 30/11/2020 07:20:02+0100, Parshuram Thombare wrote:
> Added support to acquire I3C bus through sysfs interface.
> 
> Signed-off-by: Parshuram Thombare <pthombar@cadence.com>
> ---
>  drivers/i3c/master.c |   18 ++++++++++++++++++
>  1 files changed, 18 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
> index c01ba00..beb7495 100644
> --- a/drivers/i3c/master.c
> +++ b/drivers/i3c/master.c
> @@ -581,6 +581,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);
> +

I'm wondering whether we should allow userspace to actually control
that. Shouldn't we simply request mastership when a driver needs to talk
to a device on the bus? Is it really useful to have that until there is
an i3c-dev userspace interface?

>  static struct attribute *i3c_masterdev_attrs[] = {
>  	&dev_attr_mode.attr,
>  	&dev_attr_current_master.attr,
> @@ -591,6 +608,7 @@ static ssize_t i2c_scl_frequency_show(struct device *dev,
>  	&dev_attr_pid.attr,
>  	&dev_attr_dynamic_address.attr,
>  	&dev_attr_hdrcap.attr,
> +	&dev_attr_i3c_acquire_bus.attr,
>  	NULL,
>  };
>  ATTRIBUTE_GROUPS(i3c_masterdev);
> -- 
> 1.7.1
> 

-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

Re: [PATCH v10 7/7] i3c: master: mastership handover, defslvs processing in cdns controller driver

[Alexandre Belloni]

Hello,

On 30/11/2020 07:20:33+0100, Parshuram Thombare wrote:
> 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 |  329 +++++++++++++++++++++++++++++++---
>  1 files changed, 306 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/i3c/master/i3c-master-cdns.c b/drivers/i3c/master/i3c-master-cdns.c
> index f1d6d68..ff07862 100644
> --- a/drivers/i3c/master/i3c-master-cdns.c
> +++ b/drivers/i3c/master/i3c-master-cdns.c
> @@ -160,6 +160,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)
> @@ -192,7 +193,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)
> @@ -397,6 +398,9 @@ struct cdns_i3c_data {
>  
>  struct cdns_i3c_master {
>  	struct work_struct hj_work;
> +	struct work_struct mr_yield_work;
> +	struct work_struct defslvs_work;
> +	struct completion mr_done;
>  	struct i3c_master_controller base;
>  	u32 free_rr_slots;
>  	unsigned int maxdevs;
> @@ -416,6 +420,7 @@ struct cdns_i3c_master {
>  	struct cdns_i3c_master_caps caps;
>  	unsigned long i3c_scl_lim;
>  	const struct cdns_i3c_data *devdata;
> +	u8 mr_addr;
>  };
>  
>  static inline struct cdns_i3c_master *
> @@ -1182,10 +1187,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;
>  }
>  
> @@ -1208,21 +1209,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:
> @@ -1253,7 +1254,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);
> @@ -1261,7 +1261,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);
> @@ -1279,13 +1278,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;
>  
>  	/*
>  	 * Configure data hold delay based on device-specific data.
> @@ -1358,6 +1365,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);
> @@ -1379,6 +1387,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;
>  		}
> @@ -1390,16 +1406,42 @@ 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);
> +			complete(&master->mr_done);
> +		}
>  
> -	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;
>  }
> @@ -1523,6 +1565,106 @@ 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_HAS_DA))
> +		return -EACCES;
> +
> +	if (status & SLV_STATUS1_MR_DIS)
> +		return -EACCES;
> +
> +	writel(readl(master->regs + CTRL) | CTRL_MST_INIT | CTRL_MST_ACK,
> +	       master->regs + CTRL);
> +
> +	init_completion(&master->mr_done);
> +	if (!wait_for_completion_timeout(&master->mr_done,
> +					 msecs_to_jiffies(1000)))
> +		return -ETIMEDOUT;
> +
> +	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 const struct i3c_master_controller_ops cdns_i3c_master_ops = {
>  	.bus_init = cdns_i3c_master_bus_init,
>  	.bus_cleanup = cdns_i3c_master_bus_cleanup,
> @@ -1541,6 +1683,8 @@ static void cdns_i3c_master_recycle_ibi_slot(struct i3c_dev_desc *dev,
>  	.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,
>  };
>  
>  static void cdns_i3c_master_hj(struct work_struct *work)
> @@ -1561,9 +1705,134 @@ static void cdns_i3c_master_hj(struct work_struct *work)
>  	{ /* sentinel */ },
>  };
>  
> +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;
> +	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++;
> +		}
> +	}
> +
> +	val = readl(master->regs + SLV_STATUS1);
> +	if (!(val & SLV_STATUS1_HAS_DA) ||
> +	    i3c_master_process_defslvs(m, SLV_STATUS1_DA(val))) {
> +		queue_work(master->base.wq, work);
> +		return;
> +	}
> +}
> +
>  static int cdns_i3c_master_probe(struct platform_device *pdev)
>  {
>  	struct cdns_i3c_master *master;
> +	bool secondary;
>  	int ret, irq;
>  	u32 val;
>  
> @@ -1607,6 +1876,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);
> @@ -1618,6 +1888,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;
>  

I don't really get why being a secondary master would be hardcoded in
the IP. What happens if multiple cadence masters have
CONF_STATUS0_SEC_MASTER unset and are on the same bus?

Also, it feels weird to let that kind of decision to the master driver.
I would really think the use case would be that each master on the bus
is running on a different system. I don't really see the point of having
the primary and the secondary on the bus on the same system.

Then, we definitively want to let the decision of which master is the
current master at boot to the HW designer which will depend on what is
present on the board

>  	/* Device ID0 is reserved to describe this master. */
>  	master->maxdevs = CONF_STATUS0_DEVS_NUM(val);
> @@ -1640,12 +1911,24 @@ static int cdns_i3c_master_probe(struct platform_device *pdev)
>  		goto err_disable_sysclk;
>  	}
>  
> +	if (secondary)
> +		INIT_WORK(&master->defslvs_work, cdns_i3c_sec_master_defslvs);
> +

Don't you need any master to be able to handle defslvs to be able to
request mastership?

>  	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_primary_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_primary_master_register(&master->base, &pdev->dev,
> +						  &cdns_i3c_master_ops);
> +	}
> +
>  	if (ret)
>  		goto err_disable_sysclk;
>  
> -- 
> 1.7.1
> 

-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

RE: [PATCH v10 2/7] i3c: master: use i3c_master_register only for main master

[Parshuram Raju Thombare]

Hi Alexandre,

Thank you for review comments.

>On 08/12/2020 05:47:51+0000, Parshuram Raju Thombare wrote:
>> >This looks a bit confusing. Here you're rolling back detailss in
>> >i3c_primary_master_register() that were factored out in
>> >i3c_master_init(). If i3c_master_init() is successful, then you
>> >shouldn't be undoing its things openly in i3c_primary_master_register().
>> >Instead, there should be another function that does the reverse of
>> >i3c_master_init() here.
>>
>> Every function do its cleanup in case of failures.
>> And if any failure occur after successful i3c_master_init(), we have
>> function i3c_master_bus_cleanup() which does the major cleanup.
>>
>
>The point from Nicolas here was that the workqueue is allocated in
>i3c_master_init so you should have a function to destroy it instead of
>having to do that separately in i3c_primary_master_register. The same is
>true for the put_device. Or you have to ensure i3c_masterdev_release
>is called when i3c_primary_master_register fails.

Ok, IIUC, it is just that we need separate clean up function. 
i3c_master_bus_cleanup is used here to call driver cleanup function and
detach + release devices from bus, and I am not sure i3c_masterdev_release
can be used for that.
Better alternative is i3c_master_unregister(). However, it doesn't handle
the case where device_add fails, as this function unconditionally calls
device_unregister which call both device_del and put_device.

-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

RE: [PATCH v10 6/7] i3c: master: sysfs key for acquire bus

[Parshuram Raju Thombare]

>I'm wondering whether we should allow userspace to actually control
>that. Shouldn't we simply request mastership when a driver needs to talk
>to a device on the bus? Is it really useful to have that until there is
>an i3c-dev userspace interface?

Yes, for now it is useful for nothing more than triggering mastership request
from user space. It can be dropped and added later if needed for i3c-dev
user space interface.

-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

RE: [PATCH v10 7/7] i3c: master: mastership handover, defslvs processing in cdns controller driver

[Parshuram Raju Thombare]

>I don't really get why being a secondary master would be hardcoded in
>the IP. What happens if multiple cadence masters have
>CONF_STATUS0_SEC_MASTER unset and are on the same bus?
>
>Also, it feels weird to let that kind of decision to the master driver.
>I would really think the use case would be that each master on the bus
>is running on a different system. I don't really see the point of having
>the primary and the secondary on the bus on the same system.
>
>Then, we definitively want to let the decision of which master is the
>current master at boot to the HW designer which will depend on what is
>present on the board

Since I3C bus can have only one master at a time, it is necessary to be decide
Which master device is capable of doing initialization which includes assigning
dynamic addresses to all devices and sending DEFLVS, and can be considered
as main master to call i3c_primary_master_register. This decision about which 
master is capable of acting as main master seems controller specific.
 
Cadence's Master controller IP can act as main master or secondary master.
There are differences in their functionalities. However, if other controllers
support both modes equally, their driver can chose any master device to act
as main master, or use any other controller specific methods to decide main
master. 

>> +	if (secondary)
>> +		INIT_WORK(&master->defslvs_work, cdns_i3c_sec_master_defslvs);
>> +
>Don't you need any master to be able to handle defslvs to be able to
>request mastership?

This is for secondary master only. DEFSLVS is sent by current master, after
DAA, to ensure that all masters have information about all devices on the bus.

-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c