* [PATCH 0/4] Add the I3C mastership request
@ 2019-02-18 13:08 Przemyslaw Gaj
2019-02-18 13:08 ` [PATCH 1/4] i3c: Add support for mastership request to I3C subsystem Przemyslaw Gaj
` (3 more replies)
0 siblings, 4 replies; 8+ messages in thread
From: Przemyslaw Gaj @ 2019-02-18 13:08 UTC (permalink / raw)
To: bbrezillon; +Cc: linux-i3c, Przemyslaw Gaj, rafalc, vitor.soares
This patch series adds support for mastership request to I3C
subsystem and Cadence I3C master driver. Mastership request
allows slave to become the master of the I3C bus.
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
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
Przemyslaw Gaj (4):
i3c: Add support for mastership request to I3C subsystem
i3c: master: cdns: add support for mastership request to Cadence I3C
master driver.
i3c: master: Add module author
MAINTAINERS: add myself as co-maintainer of I3C subsystem
MAINTAINERS | 1 +
drivers/i3c/device.c | 47 ++++
drivers/i3c/internals.h | 4 +
drivers/i3c/master.c | 430 ++++++++++++++++++++++++------
drivers/i3c/master/i3c-master-cdns.c | 493 +++++++++++++++++++++++++++++++----
include/linux/i3c/master.h | 22 ++
6 files changed, 872 insertions(+), 125 deletions(-)
--
2.8.3
_______________________________________________
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 1/4] i3c: Add support for mastership request to I3C subsystem
2019-02-18 13:08 [PATCH 0/4] Add the I3C mastership request Przemyslaw Gaj
@ 2019-02-18 13:08 ` Przemyslaw Gaj
2019-02-22 15:30 ` Boris Brezillon
2019-02-18 13:08 ` [PATCH 2/4] i3c: master: cdns: add support for mastership request to Cadence I3C master driver Przemyslaw Gaj
` (2 subsequent siblings)
3 siblings, 1 reply; 8+ messages in thread
From: Przemyslaw Gaj @ 2019-02-18 13:08 UTC (permalink / raw)
To: bbrezillon; +Cc: linux-i3c, Przemyslaw Gaj, rafalc, vitor.soares
This patch adds support for mastership request to I3C subsystem.
Mastership event is enabled globally.
Mastership is requested automatically when device driver
tries to transfer data using master controller in slave mode.
There is still some limitation:
- I2C devices are registered on secondary master side if boardinfo
entry matching the info transmitted through the DEFSLVS frame.
Signed-off-by: Przemyslaw Gaj <pgaj@cadence.com>
Main changes between v2 and v3:
- Add i3c_bus_downgrade_maintenance_lock() for downgrading the bus
lock from maintenance to normal use
- Add additional fields to i2c_dev_desc for DEFSLVS purpose (addr, lvr)
- Add i3c_master_register_new_i2c_devs() function to register I2C devices
- Reworked I2C devices registration on secondary master side
Changes in v2:
- Add mastership disable event hook
- Changed name of mastership enable event hook
- Add function to test if master owns the bus
- Removed op_mode
- Changed parameter of i3c_master_get_accmst_locked, no need to
pass full i3c_device_info
- Removed redundant DEFSLVS command before GETACCMST
- Add i3c_master_bus_takeover function. There is a need to lock
the bus before adding devices and no matter of the controller
devices have to be added after mastership takeover.
- Add device registration during initialization on secondary master
side. Devices received by DEFSLVS (if occured). If not, device
initialization is deffered untill next mastership request.
---
drivers/i3c/device.c | 47 +++++
drivers/i3c/internals.h | 4 +
drivers/i3c/master.c | 428 +++++++++++++++++++++++++++++++++++++--------
include/linux/i3c/master.h | 22 +++
4 files changed, 427 insertions(+), 74 deletions(-)
diff --git a/drivers/i3c/device.c b/drivers/i3c/device.c
index 69cc040..f53d689 100644
--- a/drivers/i3c/device.c
+++ b/drivers/i3c/device.c
@@ -43,6 +43,17 @@ int i3c_device_do_priv_xfers(struct i3c_device *dev,
}
i3c_bus_normaluse_lock(dev->bus);
+ if (!i3c_master_owns_bus_locked(dev->desc->common.master)) {
+ i3c_bus_normaluse_unlock(dev->bus);
+ i3c_bus_maintenance_lock(dev->bus);
+ ret = i3c_master_request_mastership_locked(dev->desc->common.master);
+ if (ret) {
+ i3c_bus_maintenance_unlock(dev->bus);
+ return ret;
+ }
+ i3c_bus_downgrade_maintenance_lock(dev->bus);
+ }
+
ret = i3c_dev_do_priv_xfers_locked(dev->desc, xfers, nxfers);
i3c_bus_normaluse_unlock(dev->bus);
@@ -114,6 +125,17 @@ int i3c_device_enable_ibi(struct i3c_device *dev)
int ret = -ENOENT;
i3c_bus_normaluse_lock(dev->bus);
+ if (!i3c_master_owns_bus_locked(dev->desc->common.master)) {
+ i3c_bus_normaluse_unlock(dev->bus);
+ i3c_bus_maintenance_lock(dev->bus);
+ ret = i3c_master_request_mastership_locked(dev->desc->common.master);
+ if (ret) {
+ i3c_bus_maintenance_unlock(dev->bus);
+ return ret;
+ }
+ i3c_bus_downgrade_maintenance_lock(dev->bus);
+ }
+
if (dev->desc) {
mutex_lock(&dev->desc->ibi_lock);
ret = i3c_dev_enable_ibi_locked(dev->desc);
@@ -145,6 +167,18 @@ int i3c_device_request_ibi(struct i3c_device *dev,
return -EINVAL;
i3c_bus_normaluse_lock(dev->bus);
+
+ if (!i3c_master_owns_bus_locked(dev->desc->common.master)) {
+ i3c_bus_normaluse_unlock(dev->bus);
+ i3c_bus_maintenance_lock(dev->bus);
+ ret = i3c_master_request_mastership_locked(dev->desc->common.master);
+ if (ret) {
+ i3c_bus_maintenance_unlock(dev->bus);
+ return ret;
+ }
+ i3c_bus_downgrade_maintenance_lock(dev->bus);
+ }
+
if (dev->desc) {
mutex_lock(&dev->desc->ibi_lock);
ret = i3c_dev_request_ibi_locked(dev->desc, req);
@@ -166,7 +200,20 @@ EXPORT_SYMBOL_GPL(i3c_device_request_ibi);
*/
void i3c_device_free_ibi(struct i3c_device *dev)
{
+ int ret;
+
i3c_bus_normaluse_lock(dev->bus);
+ if (!i3c_master_owns_bus_locked(dev->desc->common.master)) {
+ i3c_bus_normaluse_unlock(dev->bus);
+ i3c_bus_maintenance_lock(dev->bus);
+ ret = i3c_master_request_mastership_locked(dev->desc->common.master);
+ if (ret) {
+ i3c_bus_maintenance_unlock(dev->bus);
+ return;
+ }
+ i3c_bus_downgrade_maintenance_lock(dev->bus);
+ }
+
if (dev->desc) {
mutex_lock(&dev->desc->ibi_lock);
i3c_dev_free_ibi_locked(dev->desc);
diff --git a/drivers/i3c/internals.h b/drivers/i3c/internals.h
index 86b7b44..7e9f1fb 100644
--- a/drivers/i3c/internals.h
+++ b/drivers/i3c/internals.h
@@ -14,6 +14,10 @@ extern struct bus_type i3c_bus_type;
void i3c_bus_normaluse_lock(struct i3c_bus *bus);
void i3c_bus_normaluse_unlock(struct i3c_bus *bus);
+void i3c_bus_downgrade_maintenance_lock(struct i3c_bus *bus);
+bool i3c_master_owns_bus_locked(struct i3c_master_controller *master);
+int i3c_master_request_mastership_locked(struct i3c_master_controller *master);
+
int i3c_dev_do_priv_xfers_locked(struct i3c_dev_desc *dev,
struct i3c_priv_xfer *xfers,
diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
index 2dc628d..14493e5 100644
--- a/drivers/i3c/master.c
+++ b/drivers/i3c/master.c
@@ -38,10 +38,11 @@ static DEFINE_MUTEX(i3c_core_lock);
* logic to rely on I3C device information that could be changed behind their
* back.
*/
-static void i3c_bus_maintenance_lock(struct i3c_bus *bus)
+void i3c_bus_maintenance_lock(struct i3c_bus *bus)
{
down_write(&bus->lock);
}
+EXPORT_SYMBOL_GPL(i3c_bus_maintenance_lock);
/**
* i3c_bus_maintenance_unlock - Release the bus lock after a maintenance
@@ -52,10 +53,11 @@ static void i3c_bus_maintenance_lock(struct i3c_bus *bus)
* i3c_bus_maintenance_lock() for more details on what these maintenance
* operations are.
*/
-static void i3c_bus_maintenance_unlock(struct i3c_bus *bus)
+void i3c_bus_maintenance_unlock(struct i3c_bus *bus)
{
up_write(&bus->lock);
}
+EXPORT_SYMBOL_GPL(i3c_bus_maintenance_unlock);
/**
* i3c_bus_normaluse_lock - Lock the bus for a normal operation
@@ -91,6 +93,20 @@ void i3c_bus_normaluse_unlock(struct i3c_bus *bus)
up_read(&bus->lock);
}
+/**
+ * i3c_bus_downgrade_maintenance_lock - Downgrade the bus lock to normal
+ * operation
+ * @bus: I3C bus to downgrade the lock on
+ *
+ * Should be called when a maintenance operation is done and normal
+ * operation is planned. See i3c_bus_maintenance_lock() and
+ * i3c_bus_normaluse_lock() for more details.
+ */
+void i3c_bus_downgrade_maintenance_lock(struct i3c_bus *bus)
+{
+ downgrade_write(&bus->lock);
+}
+
static struct i3c_master_controller *dev_to_i3cmaster(struct device *dev)
{
return container_of(dev, struct i3c_master_controller, dev);
@@ -339,6 +355,22 @@ static int i3c_device_probe(struct device *dev)
return driver->probe(i3cdev);
}
+static void i3c_master_enable_mr_events(struct i3c_master_controller *master)
+{
+ if (!master->ops->enable_mr_events)
+ return;
+
+ master->ops->enable_mr_events(master);
+}
+
+static void i3c_master_disable_mr_events(struct i3c_master_controller *master)
+{
+ if (!master->ops->disable_mr_events)
+ return;
+
+ master->ops->disable_mr_events(master);
+}
+
static int i3c_device_remove(struct device *dev)
{
struct i3c_device *i3cdev = dev_to_i3cdev(dev);
@@ -460,6 +492,23 @@ static int i3c_bus_init(struct i3c_bus *i3cbus)
return 0;
}
+int i3c_master_request_mastership_locked(struct i3c_master_controller *master)
+{
+ if (WARN_ON(master->init_done &&
+ !rwsem_is_locked(&master->bus.lock)))
+ return -EINVAL;
+
+ if (!master->ops->request_mastership)
+ return -ENOTSUPP;
+
+ return master->ops->request_mastership(master);
+}
+
+bool i3c_master_owns_bus_locked(struct i3c_master_controller *master)
+{
+ return master->bus.cur_master == master->this;
+}
+
static const char * const i3c_bus_mode_strings[] = {
[I3C_BUS_MODE_PURE] = "pure",
[I3C_BUS_MODE_MIXED_FAST] = "mixed-fast",
@@ -618,6 +667,25 @@ i3c_master_alloc_i2c_dev(struct i3c_master_controller *master,
dev->common.master = master;
dev->boardinfo = boardinfo;
+ dev->addr = boardinfo->base.addr;
+ dev->lvr = boardinfo->lvr;
+
+ return dev;
+}
+
+static struct i2c_dev_desc *
+i3c_master_alloc_i2c_dev_no_boardinfo(struct i3c_master_controller *master,
+ u16 addr, u8 lvr)
+{
+ struct i2c_dev_desc *dev;
+
+ dev = kzalloc(sizeof(*dev), GFP_KERNEL);
+ if (!dev)
+ return ERR_PTR(-ENOMEM);
+
+ dev->common.master = master;
+ dev->addr = addr;
+ dev->lvr = lvr;
return dev;
}
@@ -691,6 +759,9 @@ i3c_master_find_i2c_dev_by_addr(const struct i3c_master_controller *master,
struct i2c_dev_desc *dev;
i3c_bus_for_each_i2cdev(&master->bus, dev) {
+ if (!dev->boardinfo)
+ continue;
+
if (dev->boardinfo->base.addr == addr)
return dev;
}
@@ -937,8 +1008,8 @@ int i3c_master_defslvs_locked(struct i3c_master_controller *master)
desc = defslvs->slaves;
i3c_bus_for_each_i2cdev(bus, i2cdev) {
- desc->lvr = i2cdev->boardinfo->lvr;
- desc->static_addr = i2cdev->boardinfo->base.addr << 1;
+ desc->lvr = i2cdev->lvr;
+ desc->static_addr = i2cdev->addr << 1;
desc++;
}
@@ -1490,6 +1561,83 @@ i3c_master_register_new_i3c_devs(struct i3c_master_controller *master)
}
}
+static struct i2c_dev_boardinfo *
+i3c_master_find_i2c_boardinfo(const struct i3c_master_controller *master,
+ u16 addr, u8 lvr)
+{
+ struct i2c_dev_boardinfo *i2cboardinfo;
+
+ list_for_each_entry(i2cboardinfo, &master->boardinfo.i2c, node) {
+ if (i2cboardinfo->base.addr == addr &&
+ i2cboardinfo->lvr == lvr)
+ return i2cboardinfo;
+ }
+
+ return NULL;
+}
+
+static void
+i3c_master_register_new_i2c_devs(struct i3c_master_controller *master)
+{
+ struct i2c_adapter *adap = i3c_master_to_i2c_adapter(master);
+ struct i2c_dev_desc *i2cdev;
+
+ if (!master->init_done)
+ return;
+
+ i3c_bus_for_each_i2cdev(&master->bus, i2cdev) {
+ if (i2cdev->dev)
+ continue;
+
+ if (!i2cdev->boardinfo)
+ continue;
+
+ i2cdev->dev = i2c_new_device(adap, &i2cdev->boardinfo->base);
+ }
+}
+
+/**
+ * i3c_master_get_accmst_locked() - send a GETACCMST CCC command
+ * @master: master used to send frames on the bus
+ * @info: I3C device information
+ *
+ * Send a GETACCMST CCC command.
+ *
+ * This should be called if the curent master acknowledges bus takeover.
+ *
+ * This function must be called with the bus lock held in write mode.
+ *
+ * Return: 0 in case of success, a positive I3C error code if the error is
+ * one of the official Mx error codes, and a negative error code otherwise.
+ */
+int i3c_master_get_accmst_locked(struct i3c_master_controller *master,
+ u8 addr)
+{
+ struct i3c_ccc_getaccmst *accmst;
+ struct i3c_ccc_cmd_dest dest;
+ struct i3c_ccc_cmd cmd;
+ int ret;
+
+ accmst = i3c_ccc_cmd_dest_init(&dest, addr, sizeof(*accmst));
+ if (!accmst)
+ return -ENOMEM;
+
+ i3c_ccc_cmd_init(&cmd, true, I3C_CCC_GETACCMST, &dest, 1);
+
+ ret = i3c_master_send_ccc_cmd_locked(master, &cmd);
+ if (ret)
+ goto out;
+
+ if (dest.payload.len != sizeof(*accmst))
+ ret = -EIO;
+
+out:
+ i3c_ccc_cmd_dest_cleanup(&dest);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(i3c_master_get_accmst_locked);
+
/**
* i3c_master_do_daa() - do a DAA (Dynamic Address Assignment)
* @master: master doing the DAA
@@ -1554,12 +1702,9 @@ int i3c_master_set_info(struct i3c_master_controller *master,
struct i3c_dev_desc *i3cdev;
int ret;
- if (!i3c_bus_dev_addr_is_avail(&master->bus, info->dyn_addr))
- return -EINVAL;
-
- if (I3C_BCR_DEVICE_ROLE(info->bcr) == I3C_BCR_I3C_MASTER &&
- master->secondary)
- return -EINVAL;
+ if (!master->secondary)
+ if (!i3c_bus_dev_addr_is_avail(&master->bus, info->dyn_addr))
+ return -EINVAL;
if (master->this)
return -EINVAL;
@@ -1605,43 +1750,13 @@ static void i3c_master_detach_free_devs(struct i3c_master_controller *master)
common.node) {
i3c_master_detach_i2c_dev(i2cdev);
i3c_bus_set_addr_slot_status(&master->bus,
- i2cdev->boardinfo->base.addr,
- I3C_ADDR_SLOT_FREE);
+ i2cdev->addr,
+ I3C_ADDR_SLOT_FREE);
i3c_master_free_i2c_dev(i2cdev);
}
}
-/**
- * i3c_master_bus_init() - initialize an I3C bus
- * @master: main master initializing the bus
- *
- * This function is following all initialisation steps described in the I3C
- * specification:
- *
- * 1. Attach I2C and statically defined I3C devs to the master so that the
- * master can fill its internal device table appropriately
- *
- * 2. Call &i3c_master_controller_ops->bus_init() method to initialize
- * the master controller. That's usually where the bus mode is selected
- * (pure bus or mixed fast/slow bus)
- *
- * 3. Instruct all devices on the bus to drop their dynamic address. This is
- * particularly important when the bus was previously configured by someone
- * else (for example the bootloader)
- *
- * 4. Disable all slave events.
- *
- * 5. Pre-assign dynamic addresses requested by the FW with SETDASA for I3C
- * devices that have a static address
- *
- * 6. Do a DAA (Dynamic Address Assignment) to assign dynamic addresses to all
- * remaining I3C devices
- *
- * Once this is done, all I3C and I2C devices should be usable.
- *
- * Return: a 0 in case of success, an negative error code otherwise.
- */
-static int i3c_master_bus_init(struct i3c_master_controller *master)
+static int i3c_master_attach_static_devs(struct i3c_master_controller *master)
{
enum i3c_addr_slot_status status;
struct i2c_dev_boardinfo *i2cboardinfo;
@@ -1650,32 +1765,24 @@ static int i3c_master_bus_init(struct i3c_master_controller *master)
struct i2c_dev_desc *i2cdev;
int ret;
- /*
- * First attach all devices with static definitions provided by the
- * FW.
- */
list_for_each_entry(i2cboardinfo, &master->boardinfo.i2c, node) {
status = i3c_bus_get_addr_slot_status(&master->bus,
i2cboardinfo->base.addr);
- if (status != I3C_ADDR_SLOT_FREE) {
- ret = -EBUSY;
- goto err_detach_devs;
- }
+ if (status != I3C_ADDR_SLOT_FREE)
+ return -EBUSY;
i3c_bus_set_addr_slot_status(&master->bus,
i2cboardinfo->base.addr,
I3C_ADDR_SLOT_I2C_DEV);
i2cdev = i3c_master_alloc_i2c_dev(master, i2cboardinfo);
- if (IS_ERR(i2cdev)) {
- ret = PTR_ERR(i2cdev);
- goto err_detach_devs;
- }
+ if (IS_ERR(i2cdev))
+ return PTR_ERR(i2cdev);
ret = i3c_master_attach_i2c_dev(master, i2cdev);
if (ret) {
i3c_master_free_i2c_dev(i2cdev);
- goto err_detach_devs;
+ return ret;
}
}
list_for_each_entry(i3cboardinfo, &master->boardinfo.i3c, node) {
@@ -1685,28 +1792,71 @@ static int i3c_master_bus_init(struct i3c_master_controller *master)
if (i3cboardinfo->init_dyn_addr) {
status = i3c_bus_get_addr_slot_status(&master->bus,
- i3cboardinfo->init_dyn_addr);
- if (status != I3C_ADDR_SLOT_FREE) {
- ret = -EBUSY;
- goto err_detach_devs;
- }
+ i3cboardinfo->init_dyn_addr);
+ if (status != I3C_ADDR_SLOT_FREE)
+ return -EBUSY;
}
i3cdev = i3c_master_alloc_i3c_dev(master, &info);
- if (IS_ERR(i3cdev)) {
- ret = PTR_ERR(i3cdev);
- goto err_detach_devs;
- }
+ if (IS_ERR(i3cdev))
+ return PTR_ERR(i3cdev);
i3cdev->boardinfo = i3cboardinfo;
ret = i3c_master_attach_i3c_dev(master, i3cdev);
if (ret) {
i3c_master_free_i3c_dev(i3cdev);
- goto err_detach_devs;
+ return ret;
}
}
+ return 0;
+}
+
+/**
+ * i3c_master_bus_init() - initialize an I3C bus
+ * @master: main master initializing the bus
+ *
+ * This function is following all initialisation steps described in the I3C
+ * specification:
+ *
+ * 1. Attach I2C and statically defined I3C devs to the master so that the
+ * master can fill its internal device table appropriately
+ *
+ * 2. Call &i3c_master_controller_ops->bus_init() method to initialize
+ * the master controller. That's usually where the bus mode is selected
+ * (pure bus or mixed fast/slow bus)
+ *
+ * 3. Instruct all devices on the bus to drop their dynamic address. This is
+ * particularly important when the bus was previously configured by someone
+ * else (for example the bootloader)
+ *
+ * 4. Disable all slave events.
+ *
+ * 5. Pre-assign dynamic addresses requested by the FW with SETDASA for I3C
+ * devices that have a static address
+ *
+ * 6. Do a DAA (Dynamic Address Assignment) to assign dynamic addresses to all
+ * remaining I3C devices
+ *
+ * Once this is done, all I3C and I2C devices should be usable.
+ *
+ * Return: a 0 in case of success, an negative error code otherwise.
+ */
+static int i3c_master_bus_init(struct i3c_master_controller *master)
+{
+ struct i3c_dev_desc *i3cdev;
+ int ret;
+
+ /*
+ * First attach all devices with static definitions provided by the
+ * FW.
+ */
+ if (!master->secondary) {
+ ret = i3c_master_attach_static_devs(master);
+ if (ret)
+ goto err_detach_devs;
+ }
/*
* Now execute the controller specific ->bus_init() routine, which
* might configure its internal logic to match the bus limitations.
@@ -1727,6 +1877,13 @@ static int i3c_master_bus_init(struct i3c_master_controller *master)
}
/*
+ * Don't reset addresses if this is secondary master.
+ * Secondary masters can't do DAA.
+ */
+ if (master->secondary)
+ return 0;
+
+ /*
* Reset all dynamic address that may have been assigned before
* (assigned by the bootloader for example).
*/
@@ -1789,6 +1946,56 @@ i3c_master_search_i3c_dev_duplicate(struct i3c_dev_desc *refdev)
return NULL;
}
+int i3c_master_add_i2c_dev(struct i3c_master_controller *master,
+ u16 addr, u8 lvr)
+{
+ enum i3c_addr_slot_status status;
+ struct i2c_dev_boardinfo *boardinfo;
+ struct i2c_dev_desc *i2cdev;
+ int ret;
+
+ if (!master)
+ return -EINVAL;
+
+ status = i3c_bus_get_addr_slot_status(&master->bus,
+ addr);
+ if (status != I3C_ADDR_SLOT_FREE)
+ return -EBUSY;
+
+ i3c_bus_set_addr_slot_status(&master->bus, addr,
+ I3C_ADDR_SLOT_I2C_DEV);
+
+ i2cdev = i3c_master_alloc_i2c_dev_no_boardinfo(master, addr, lvr);
+ if (IS_ERR(i2cdev)) {
+ ret = PTR_ERR(i2cdev);
+ goto err_free_dev;
+ }
+
+ boardinfo = i3c_master_find_i2c_boardinfo(master, addr, lvr);
+ if (!boardinfo) {
+ ret = -ENODEV;
+ goto err_free_dev;
+ }
+
+ i2cdev->boardinfo = boardinfo;
+
+ ret = i3c_master_attach_i2c_dev(master, i2cdev);
+ if (ret) {
+ ret = PTR_ERR(i2cdev);
+ goto err_free_dev;
+ }
+
+ return 0;
+
+err_free_dev:
+ i3c_bus_set_addr_slot_status(&master->bus, addr,
+ I3C_ADDR_SLOT_FREE);
+ i3c_master_free_i2c_dev(i2cdev);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(i3c_master_add_i2c_dev);
+
/**
* i3c_master_add_i3c_dev_locked() - add an I3C slave to the bus
* @master: master used to send frames on the bus
@@ -2101,6 +2308,18 @@ static int i3c_master_i2c_adapter_xfer(struct i2c_adapter *adap,
}
i3c_bus_normaluse_lock(&master->bus);
+ if (!i3c_master_owns_bus_locked(master)) {
+ i3c_bus_normaluse_unlock(&master->bus);
+ i3c_bus_maintenance_lock(&master->bus);
+
+ ret = i3c_master_request_mastership_locked(master);
+ if (ret) {
+ i3c_bus_maintenance_unlock(&master->bus);
+ return ret;
+ }
+ i3c_bus_downgrade_maintenance_lock(&master->bus);
+ }
+
dev = i3c_master_find_i2c_dev_by_addr(master, addr);
if (!dev)
ret = -ENOENT;
@@ -2146,8 +2365,12 @@ static int i3c_master_i2c_adapter_init(struct i3c_master_controller *master)
* We silently ignore failures here. The bus should keep working
* correctly even if one or more i2c devices are not registered.
*/
- i3c_bus_for_each_i2cdev(&master->bus, i2cdev)
+ i3c_bus_for_each_i2cdev(&master->bus, i2cdev) {
+ if (!i2cdev->boardinfo)
+ continue;
+
i2cdev->dev = i2c_new_device(adap, &i2cdev->boardinfo->base);
+ }
return 0;
}
@@ -2388,18 +2611,43 @@ static int i3c_master_check_ops(const struct i3c_master_controller_ops *ops)
!ops->recycle_ibi_slot))
return -EINVAL;
+ if (ops->request_mastership &&
+ (!ops->enable_mr_events || !ops->disable_mr_events))
+ return -EINVAL;
+
return 0;
}
/**
+ * i3c_master_bus_takeover() - register new I3C devices on bus takeover
+ * @master: master used to send frames on the bus
+ *
+ * This function is useful when devices were not added
+ * during initialization or when new device joined the bus
+ * which was under control of different master.
+ */
+void i3c_master_bus_takeover(struct i3c_master_controller *master)
+{
+ /*
+ * We can register I3C devices received from master by DEFSLVS.
+ */
+ i3c_bus_normaluse_lock(&master->bus);
+ i3c_master_register_new_i3c_devs(master);
+ i3c_bus_normaluse_unlock(&master->bus);
+
+ i3c_bus_normaluse_lock(&master->bus);
+ i3c_master_register_new_i2c_devs(master);
+ i3c_bus_normaluse_unlock(&master->bus);
+}
+EXPORT_SYMBOL_GPL(i3c_master_bus_takeover);
+
+/**
* i3c_master_register() - register an I3C master
* @master: master used to send frames on the bus
* @parent: the parent device (the one that provides this I3C master
* controller)
* @ops: the master controller operations
- * @secondary: true if you are registering a secondary master. Will return
- * -ENOTSUPP if set to true since secondary masters are not yet
- * supported
+ * @secondary: true if you are registering a secondary master.
*
* This function takes care of everything for you:
*
@@ -2422,10 +2670,6 @@ int i3c_master_register(struct i3c_master_controller *master,
struct i2c_dev_boardinfo *i2cbi;
int ret;
- /* We do not support secondary masters yet. */
- if (secondary)
- return -ENOTSUPP;
-
ret = i3c_master_check_ops(ops);
if (ret)
return ret;
@@ -2503,6 +2747,14 @@ int i3c_master_register(struct i3c_master_controller *master,
i3c_master_register_new_i3c_devs(master);
i3c_bus_normaluse_unlock(&master->bus);
+ i3c_bus_normaluse_lock(&master->bus);
+ i3c_master_register_new_i2c_devs(master);
+ i3c_bus_normaluse_unlock(&master->bus);
+
+ i3c_bus_maintenance_lock(&master->bus);
+ i3c_master_enable_mr_events(master);
+ i3c_bus_maintenance_unlock(&master->bus);
+
return 0;
err_del_dev:
@@ -2519,6 +2771,30 @@ int i3c_master_register(struct i3c_master_controller *master,
EXPORT_SYMBOL_GPL(i3c_master_register);
/**
+ * i3c_master_mastership_ack() - performs operations before bus handover.
+ * @master: master used to send frames on the bus
+ * @addr: I3C device address
+ *
+ * Basically, it sends DEFSLVS command to ensure new master is taking
+ * the bus with complete list of devices and then acknowledges bus takeover.
+ *
+ * Return: 0 in case of success, a negative error code otherwise.
+ */
+int i3c_master_mastership_ack(struct i3c_master_controller *master,
+ u8 addr)
+{
+ int ret;
+
+ i3c_bus_maintenance_lock(&master->bus);
+ ret = i3c_master_get_accmst_locked(master, addr);
+ i3c_bus_maintenance_unlock(&master->bus);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(i3c_master_mastership_ack);
+
+
+/**
* i3c_master_unregister() - unregister an I3C master
* @master: master used to send frames on the bus
*
@@ -2528,6 +2804,10 @@ EXPORT_SYMBOL_GPL(i3c_master_register);
*/
int i3c_master_unregister(struct i3c_master_controller *master)
{
+ i3c_bus_maintenance_lock(&master->bus);
+ i3c_master_disable_mr_events(master);
+ i3c_bus_maintenance_unlock(&master->bus);
+
i3c_master_i2c_adapter_cleanup(master);
i3c_master_unregister_i3c_devs(master);
i3c_master_bus_cleanup(master);
diff --git a/include/linux/i3c/master.h b/include/linux/i3c/master.h
index f13fd8b..8514680 100644
--- a/include/linux/i3c/master.h
+++ b/include/linux/i3c/master.h
@@ -84,6 +84,8 @@ struct i2c_dev_desc {
struct i3c_i2c_dev_desc common;
const struct i2c_dev_boardinfo *boardinfo;
struct i2c_client *dev;
+ u16 addr;
+ u8 lvr;
};
/**
@@ -418,6 +420,12 @@ struct i3c_bus {
* for a future IBI
* This method is mandatory only if ->request_ibi is not
* NULL.
+ * @request_mastership: requests bus mastership. Mastership is requested
+ * automatically when device driver wants to transfer
+ * data using master in slave mode.
+ * @enable_mr_events: enable the Mastership event.
+ * Mastership does not require handler.
+ * @disable_mr_events: disable the Mastership event.
*/
struct i3c_master_controller_ops {
int (*bus_init)(struct i3c_master_controller *master);
@@ -445,6 +453,10 @@ struct i3c_master_controller_ops {
int (*disable_ibi)(struct i3c_dev_desc *dev);
void (*recycle_ibi_slot)(struct i3c_dev_desc *dev,
struct i3c_ibi_slot *slot);
+ void (*update_devs)(struct i3c_master_controller *master);
+ int (*request_mastership)(struct i3c_master_controller *master);
+ int (*enable_mr_events)(struct i3c_master_controller *master);
+ int (*disable_mr_events)(struct i3c_master_controller *master);
};
/**
@@ -524,6 +536,8 @@ int i3c_master_defslvs_locked(struct i3c_master_controller *master);
int i3c_master_get_free_addr(struct i3c_master_controller *master,
u8 start_addr);
+int i3c_master_add_i2c_dev(struct i3c_master_controller *master,
+ u16 addr, u8 lvr);
int i3c_master_add_i3c_dev_locked(struct i3c_master_controller *master,
u8 addr);
int i3c_master_do_daa(struct i3c_master_controller *master);
@@ -536,6 +550,11 @@ int i3c_master_register(struct i3c_master_controller *master,
const struct i3c_master_controller_ops *ops,
bool secondary);
int i3c_master_unregister(struct i3c_master_controller *master);
+int i3c_master_get_accmst_locked(struct i3c_master_controller *master,
+ u8 addr);
+int i3c_master_mastership_ack(struct i3c_master_controller *master,
+ u8 addr);
+void i3c_master_bus_takeover(struct i3c_master_controller *master);
/**
* i3c_dev_get_master_data() - get master private data attached to an I3C
@@ -645,4 +664,7 @@ void i3c_master_queue_ibi(struct i3c_dev_desc *dev, struct i3c_ibi_slot *slot);
struct i3c_ibi_slot *i3c_master_get_free_ibi_slot(struct i3c_dev_desc *dev);
+void i3c_bus_maintenance_lock(struct i3c_bus *bus);
+void i3c_bus_maintenance_unlock(struct i3c_bus *bus);
+
#endif /* I3C_MASTER_H */
--
2.8.3
_______________________________________________
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 2/4] i3c: master: cdns: add support for mastership request to Cadence I3C master driver.
2019-02-18 13:08 [PATCH 0/4] Add the I3C mastership request Przemyslaw Gaj
2019-02-18 13:08 ` [PATCH 1/4] i3c: Add support for mastership request to I3C subsystem Przemyslaw Gaj
@ 2019-02-18 13:08 ` Przemyslaw Gaj
2019-02-18 13:08 ` [PATCH 3/4] i3c: master: Add module author Przemyslaw Gaj
2019-02-18 13:08 ` [PATCH 4/4] MAINTAINERS: add myself as co-maintainer of I3C subsystem Przemyslaw Gaj
3 siblings, 0 replies; 8+ messages in thread
From: Przemyslaw Gaj @ 2019-02-18 13:08 UTC (permalink / raw)
To: bbrezillon; +Cc: linux-i3c, Przemyslaw Gaj, rafalc, vitor.soares
This patch adds support for mastership request to Cadence I3C master driver.
Mastership is requested automatically after secondary master receives
mastership ENEC event. This allows secondary masters to initialize their bus.
Signed-off-by: Przemyslaw Gaj <pgaj@cadence.com>
Main changes between v2 and v3:
- Add mastership type
- Add postponed master registration
- Add update device definition on bus initialization time
- Removed redundant mastership work structs
- Reworked IBI slot lookup
- Reworked Mastership event enabling/disabling
Changes in v2:
- Add work structs for mastership purpose
- Add missing mastership disable feature
---
drivers/i3c/master/i3c-master-cdns.c | 493 +++++++++++++++++++++++++++++++----
1 file changed, 442 insertions(+), 51 deletions(-)
diff --git a/drivers/i3c/master/i3c-master-cdns.c b/drivers/i3c/master/i3c-master-cdns.c
index 8889a4f..de4c4ad 100644
--- a/drivers/i3c/master/i3c-master-cdns.c
+++ b/drivers/i3c/master/i3c-master-cdns.c
@@ -157,6 +157,7 @@
#define SLV_IMR 0x48
#define SLV_ICR 0x4c
#define SLV_ISR 0x50
+#define SLV_INT_DEFSLVS BIT(21)
#define SLV_INT_TM BIT(20)
#define SLV_INT_ERROR BIT(19)
#define SLV_INT_EVENT_UP BIT(18)
@@ -388,8 +389,19 @@ struct cdns_i3c_xfer {
struct cdns_i3c_cmd cmds[0];
};
+enum cdns_i3c_mr {
+ REQUEST,
+ HANDOFF,
+ TAKEOVER
+};
+
struct cdns_i3c_master {
struct work_struct hj_work;
+ struct {
+ struct work_struct work;
+ enum cdns_i3c_mr mr_type;
+ u32 ibir;
+ } mastership;
struct i3c_master_controller base;
u32 free_rr_slots;
unsigned int maxdevs;
@@ -408,6 +420,8 @@ struct cdns_i3c_master {
struct clk *pclk;
struct cdns_i3c_master_caps caps;
unsigned long i3c_scl_lim;
+ struct device *parent;
+ struct workqueue_struct *wq;
};
static inline struct cdns_i3c_master *
@@ -663,6 +677,88 @@ static void cdns_i3c_master_unqueue_xfer(struct cdns_i3c_master *master,
spin_unlock_irqrestore(&master->xferqueue.lock, flags);
}
+static void
+cdns_i3c_master_i3c_dev_rr_to_info(struct cdns_i3c_master *master,
+ unsigned int slot,
+ struct i3c_device_info *info)
+{
+ u32 rr;
+
+ memset(info, 0, sizeof(*info));
+ rr = readl(master->regs + DEV_ID_RR0(slot));
+ info->dyn_addr = DEV_ID_RR0_GET_DEV_ADDR(rr);
+ rr = readl(master->regs + DEV_ID_RR2(slot));
+ info->dcr = rr;
+ info->bcr = rr >> 8;
+ info->pid = rr >> 16;
+ info->pid |= (u64)readl(master->regs + DEV_ID_RR1(slot)) << 16;
+}
+
+static void
+cdns_i3c_master_i2c_dev_rr_to_info(struct cdns_i3c_master *master,
+ unsigned int slot,
+ u16 *addr, u8 *lvr)
+{
+ u32 rr;
+
+ rr = readl(master->regs + DEV_ID_RR0(slot));
+ *addr = DEV_ID_RR0_GET_DEV_ADDR(rr);
+ rr = readl(master->regs + DEV_ID_RR2(slot));
+ *lvr = rr;
+}
+
+static
+int cdns_i3c_sec_master_request_mastership(struct i3c_master_controller *m)
+{
+ struct cdns_i3c_master *master = to_cdns_i3c_master(m);
+ u32 status;
+ int ret;
+
+ status = readl(master->regs + MST_STATUS0);
+ if (WARN_ON(status & MST_STATUS0_MASTER_MODE))
+ return -EEXIST;
+
+ status = readl(master->regs + SLV_STATUS1);
+ if (status & SLV_STATUS1_MR_DIS)
+ return -EACCES;
+
+ writel(SLV_INT_MR_DONE, master->regs + SLV_IER);
+ writel(readl(master->regs + CTRL) | CTRL_MST_INIT | CTRL_MST_ACK,
+ master->regs + CTRL);
+
+ ret = readl_poll_timeout(master->regs + MST_STATUS0, status,
+ status & MST_STATUS0_MASTER_MODE, 100,
+ 100000);
+ return ret;
+}
+
+static void cdns_i3c_master_update_devs(struct i3c_master_controller *m)
+{
+ struct cdns_i3c_master *master = to_cdns_i3c_master(m);
+ u32 val, newdevs;
+ u16 addr;
+ u8 lvr;
+ int slot;
+ struct i3c_device_info i3c_info;
+
+ newdevs = readl(master->regs + DEVS_CTRL) & DEVS_CTRL_DEVS_ACTIVE_MASK;
+ for (slot = 1; slot <= master->maxdevs; slot++) {
+ val = readl(master->regs + DEV_ID_RR0(slot));
+
+ if ((newdevs & BIT(slot)) && (val & DEV_ID_RR0_IS_I3C)) {
+ cdns_i3c_master_i3c_dev_rr_to_info(master, slot,
+ &i3c_info);
+
+ i3c_master_add_i3c_dev_locked(m, i3c_info.dyn_addr);
+ } else if ((newdevs & BIT(slot)) &&
+ !(val & DEV_ID_RR0_IS_I3C)) {
+ cdns_i3c_master_i2c_dev_rr_to_info(master, slot,
+ &addr, &lvr);
+ i3c_master_add_i2c_dev(m, addr, lvr);
+ }
+ }
+}
+
static enum i3c_error_code cdns_i3c_cmd_get_err(struct cdns_i3c_cmd *cmd)
{
switch (cmd->error) {
@@ -1044,22 +1140,6 @@ static void cdns_i3c_master_bus_cleanup(struct i3c_master_controller *m)
cdns_i3c_master_disable(master);
}
-static void cdns_i3c_master_dev_rr_to_info(struct cdns_i3c_master *master,
- unsigned int slot,
- struct i3c_device_info *info)
-{
- u32 rr;
-
- memset(info, 0, sizeof(*info));
- rr = readl(master->regs + DEV_ID_RR0(slot));
- info->dyn_addr = DEV_ID_RR0_GET_DEV_ADDR(rr);
- rr = readl(master->regs + DEV_ID_RR2(slot));
- info->dcr = rr;
- info->bcr = rr >> 8;
- info->pid = rr >> 16;
- info->pid |= (u64)readl(master->regs + DEV_ID_RR1(slot)) << 16;
-}
-
static void cdns_i3c_master_upd_i3c_scl_lim(struct cdns_i3c_master *master)
{
struct i3c_master_controller *m = &master->base;
@@ -1187,10 +1267,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;
}
@@ -1254,15 +1330,17 @@ static int cdns_i3c_master_bus_init(struct i3c_master_controller *m)
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);
- if (ret < 0)
- return ret;
+ if (!m->secondary) {
+ /* Get an address for the master. */
+ ret = i3c_master_get_free_addr(m, 0);
+ if (ret < 0)
+ return ret;
- writel(prepare_rr0_dev_address(ret) | DEV_ID_RR0_IS_I3C,
- master->regs + DEV_ID_RR0(0));
+ writel(prepare_rr0_dev_address(ret) | DEV_ID_RR0_IS_I3C,
+ master->regs + DEV_ID_RR0(0));
+ }
- cdns_i3c_master_dev_rr_to_info(master, 0, &info);
+ cdns_i3c_master_i3c_dev_rr_to_info(master, 0, &info);
if (info.bcr & I3C_BCR_HDR_CAP)
info.hdr_cap = I3C_CCC_HDR_MODE(I3C_HDR_DDR);
@@ -1281,9 +1359,32 @@ static int cdns_i3c_master_bus_init(struct i3c_master_controller *m)
cdns_i3c_master_enable(master);
+ if (m->secondary) {
+ i3c_bus_maintenance_lock(&master->base.bus);
+ cdns_i3c_master_update_devs(&master->base);
+ i3c_bus_maintenance_unlock(&master->base.bus);
+ }
+
return 0;
}
+static
+struct i3c_dev_desc *cdns_i3c_get_ibi_device(struct cdns_i3c_master *master,
+ u32 ibir)
+{
+ struct i3c_dev_desc *dev;
+ u32 id = IBIR_SLVID(ibir);
+
+ if (id >= master->ibi.num_slots || (ibir & IBIR_ERROR))
+ return NULL;
+
+ dev = master->ibi.slots[id];
+ if (!dev)
+ return NULL;
+
+ return dev;
+}
+
static void cdns_i3c_master_handle_ibi(struct cdns_i3c_master *master,
u32 ibir)
{
@@ -1361,27 +1462,100 @@ 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));
+ master->mastership.ibir = ibir;
+ master->mastership.mr_type = HANDOFF;
+ queue_work(master->base.wq,
+ &master->mastership.work);
+ break;
default:
break;
}
}
}
+static void cdns_i3c_master_bus_handoff(struct cdns_i3c_master *master)
+{
+ struct i3c_dev_desc *dev;
+
+ dev = cdns_i3c_get_ibi_device(master, master->mastership.ibir);
+
+ writel(MST_INT_MR_DONE, master->regs + MST_ICR);
+
+ master->base.bus.cur_master = dev;
+}
+
+static void cdns_i3c_master_mastership_takeover(struct cdns_i3c_master *master)
+{
+ if (master->base.init_done) {
+ i3c_bus_maintenance_lock(&master->base.bus);
+ cdns_i3c_master_update_devs(&master->base);
+ i3c_bus_maintenance_unlock(&master->base.bus);
+
+ i3c_master_bus_takeover(&master->base);
+ }
+
+ writel(readl(master->regs + CTRL) & ~CTRL_MST_ACK, master->regs + CTRL);
+}
+
+static void cdns_i3c_sec_master_event_up(struct cdns_i3c_master *master)
+{
+ u32 status;
+
+ writel(SLV_INT_EVENT_UP, master->regs + SLV_ICR);
+ status = readl(master->regs + SLV_STATUS1);
+ if (!(status & SLV_STATUS1_MR_DIS) &&
+ !master->base.this) {
+ master->mastership.mr_type = REQUEST;
+ queue_work(master->wq, &master->mastership.work);
+ }
+}
+
+static void cdns_i3c_sec_mastership_done(struct cdns_i3c_master *master)
+{
+ writel(SLV_INT_MR_DONE, master->regs + SLV_ICR);
+
+ master->base.bus.cur_master = master->base.this;
+
+ if (master->base.this) {
+ master->mastership.mr_type = TAKEOVER;
+ queue_work(master->base.wq, &master->mastership.work);
+ }
+}
+
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;
+ status = readl(master->regs + MST_STATUS0);
+
+ if (!master->base.this ||
+ master->base.this != master->base.bus.cur_master) {
+ status = readl(master->regs + SLV_ISR);
+
+ if (!(status & readl(master->regs + SLV_IMR)))
+ return IRQ_NONE;
+
+ if (status & SLV_INT_MR_DONE)
+ cdns_i3c_sec_mastership_done(master);
+
+ if (status & SLV_INT_EVENT_UP)
+ cdns_i3c_sec_master_event_up(master);
+ } else {
+ 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);
- 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_IBIR_THR)
- cnds_i3c_master_demux_ibis(master);
+ if (status & MST_INT_MR_DONE)
+ cdns_i3c_master_bus_handoff(master);
+ }
return IRQ_HANDLED;
}
@@ -1450,30 +1624,54 @@ static int cdns_i3c_master_enable_ibi(struct i3c_dev_desc *dev)
return ret;
}
-static int cdns_i3c_master_request_ibi(struct i3c_dev_desc *dev,
- const struct i3c_ibi_setup *req)
+static int cdns_i3c_master_find_ibi_slot(struct cdns_i3c_master *master,
+ struct i3c_dev_desc *dev,
+ s16 *slot)
{
- struct i3c_master_controller *m = i3c_dev_get_master(dev);
- struct cdns_i3c_master *master = to_cdns_i3c_master(m);
- struct cdns_i3c_i2c_dev_data *data = i3c_dev_get_master_data(dev);
unsigned long flags;
unsigned int i;
-
- data->ibi_pool = i3c_generic_ibi_alloc_pool(dev, req);
- if (IS_ERR(data->ibi_pool))
- return PTR_ERR(data->ibi_pool);
+ int ret = -ENOENT;
spin_lock_irqsave(&master->ibi.lock, flags);
for (i = 0; i < master->ibi.num_slots; i++) {
- if (!master->ibi.slots[i]) {
- data->ibi = i;
- master->ibi.slots[i] = dev;
+ /*
+ * We only need 'SIR' slots to describe IBI-capable devices.
+ * This slot may be used by mastership request interrupt,
+ * We can ruse the same 'SIR' map entry.
+ */
+ 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);
- if (i < master->ibi.num_slots)
+ return ret;
+}
+
+static int cdns_i3c_master_request_ibi(struct i3c_dev_desc *dev,
+ const struct i3c_ibi_setup *req)
+{
+ struct i3c_master_controller *m = i3c_dev_get_master(dev);
+ struct cdns_i3c_master *master = to_cdns_i3c_master(m);
+ struct cdns_i3c_i2c_dev_data *data = i3c_dev_get_master_data(dev);
+
+ data->ibi_pool = i3c_generic_ibi_alloc_pool(dev, req);
+ if (IS_ERR(data->ibi_pool))
+ return PTR_ERR(data->ibi_pool);
+
+ if (cdns_i3c_master_find_ibi_slot(master, dev, &data->ibi) == 0)
return 0;
i3c_generic_ibi_free_pool(data->ibi_pool);
@@ -1482,6 +1680,51 @@ static int cdns_i3c_master_request_ibi(struct i3c_dev_desc *dev,
return -ENOSPC;
}
+static int
+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)
+ 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);
+ }
+
+ /* Unmask Hot-Join and Mastership request interrupts. */
+ i3c_master_enec_locked(&master->base, I3C_BROADCAST_ADDR,
+ I3C_CCC_EVENT_HJ | I3C_CCC_EVENT_MR);
+
+ return 0;
+}
+
static void cdns_i3c_master_free_ibi(struct i3c_dev_desc *dev)
{
struct i3c_master_controller *m = i3c_dev_get_master(dev);
@@ -1497,6 +1740,52 @@ static void cdns_i3c_master_free_ibi(struct i3c_dev_desc *dev)
i3c_generic_ibi_free_pool(data->ibi_pool);
}
+static int
+cdns_i3c_master_disable_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 sirmap;
+ int ret;
+
+ ret = i3c_master_disec_locked(m, I3C_BROADCAST_ADDR,
+ I3C_CCC_EVENT_MR);
+ if (ret)
+ return ret;
+
+ i3c_bus_for_each_i3cdev(&m->bus, i3cdev) {
+ if (I3C_BCR_DEVICE_ROLE(i3cdev->info.bcr) != I3C_BCR_I3C_MASTER)
+ continue;
+
+ data = i3c_dev_get_master_data(i3cdev);
+
+ ret = cdns_i3c_master_find_ibi_slot(master, i3cdev, &data->ibi);
+ if (ret)
+ continue;
+
+ /*
+ * Do not modify SIR register and cleanup slots
+ * if regular IBI is enabled for this device.
+ */
+ if (master->ibi.slots[data->ibi]->ibi->handler)
+ 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);
+ sirmap |= SIR_MAP_DEV_CONF(data->ibi,
+ SIR_MAP_DEV_DA(I3C_BROADCAST_ADDR));
+ writel(sirmap, master->regs + SIR_MAP_DEV_REG(data->ibi));
+ spin_unlock_irqrestore(&master->ibi.lock, flags);
+
+ cdns_i3c_master_free_ibi(i3cdev);
+ }
+
+ return ret;
+}
+
static void cdns_i3c_master_recycle_ibi_slot(struct i3c_dev_desc *dev,
struct i3c_ibi_slot *slot)
{
@@ -1524,6 +1813,9 @@ static const struct i3c_master_controller_ops cdns_i3c_master_ops = {
.request_ibi = cdns_i3c_master_request_ibi,
.free_ibi = cdns_i3c_master_free_ibi,
.recycle_ibi_slot = cdns_i3c_master_recycle_ibi_slot,
+ .request_mastership = cdns_i3c_sec_master_request_mastership,
+ .enable_mr_events = cdns_i3c_master_enable_mastership_events,
+ .disable_mr_events = cdns_i3c_master_disable_mastership_events
};
static void cdns_i3c_master_hj(struct work_struct *work)
@@ -1532,13 +1824,84 @@ static void cdns_i3c_master_hj(struct work_struct *work)
struct cdns_i3c_master,
hj_work);
- i3c_master_do_daa(&master->base);
+ if (!master->base.secondary)
+ i3c_master_do_daa(&master->base);
+}
+
+static int cdns_i3c_sec_master_bus_init(struct cdns_i3c_master *master)
+{
+ u32 val;
+ int ret;
+
+ val = readl(master->regs + SLV_STATUS1);
+
+ if (!(val & SLV_STATUS1_HAS_DA))
+ return -EFAULT;
+
+ i3c_bus_maintenance_lock(&master->base.bus);
+ ret = cdns_i3c_sec_master_request_mastership(&master->base);
+ i3c_bus_maintenance_unlock(&master->base.bus);
+
+ return ret;
+}
+
+static void
+cdns_i3c_master_mastership_request(struct cdns_i3c_master *master)
+{
+ int ret;
+
+ ret = cdns_i3c_sec_master_bus_init(master);
+ if (ret)
+ return;
+
+ ret = i3c_master_register(&master->base, master->parent,
+ &cdns_i3c_master_ops, true);
+ if (ret)
+ dev_err(&master->base.dev, "Master register failed\n");
+}
+
+static void cdns_i3c_master_mastership_handoff(struct cdns_i3c_master *master)
+{
+ int ret;
+
+ struct i3c_dev_desc *dev;
+ u32 ibir = master->mastership.ibir;
+
+ dev = cdns_i3c_get_ibi_device(master, ibir);
+ if (!dev)
+ return;
+
+ ret = i3c_master_mastership_ack(&master->base, dev->info.dyn_addr);
+ if (ret)
+ dev_err(&master->base.dev, "Mastership handoff failed\n");
+}
+
+static void cdns_i3c_master_mastership(struct work_struct *work)
+{
+ struct cdns_i3c_master *master = container_of(work,
+ struct cdns_i3c_master,
+ mastership.work);
+
+ switch (master->mastership.mr_type) {
+ case REQUEST:
+ cdns_i3c_master_mastership_request(master);
+ break;
+ case HANDOFF:
+ cdns_i3c_master_mastership_handoff(master);
+ break;
+ case TAKEOVER:
+ cdns_i3c_master_mastership_takeover(master);
+ break;
+ default:
+ break;
+ }
}
static int cdns_i3c_master_probe(struct platform_device *pdev)
{
struct cdns_i3c_master *master;
struct resource *res;
+ bool secondary = false;
int ret, irq;
u32 val;
@@ -1580,6 +1943,9 @@ static int cdns_i3c_master_probe(struct platform_device *pdev)
INIT_LIST_HEAD(&master->xferqueue.list);
INIT_WORK(&master->hj_work, cdns_i3c_master_hj);
+ INIT_WORK(&master->mastership.work,
+ cdns_i3c_master_mastership);
+
writel(0xffffffff, master->regs + MST_IDR);
writel(0xffffffff, master->regs + SLV_IDR);
ret = devm_request_irq(&pdev->dev, irq, cdns_i3c_master_interrupt, 0,
@@ -1589,6 +1955,10 @@ static int cdns_i3c_master_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, master);
+ val = readl(master->regs + MST_STATUS0);
+ if (!(val & MST_STATUS0_MASTER_MODE))
+ secondary = true;
+
val = readl(master->regs + CONF_STATUS0);
/* Device ID0 is reserved to describe this master. */
@@ -1604,6 +1974,7 @@ static int cdns_i3c_master_probe(struct platform_device *pdev)
spin_lock_init(&master->ibi.lock);
master->ibi.num_slots = CONF_STATUS1_IBI_HW_RES(val);
+
master->ibi.slots = devm_kcalloc(&pdev->dev, master->ibi.num_slots,
sizeof(*master->ibi.slots),
GFP_KERNEL);
@@ -1612,15 +1983,31 @@ static int cdns_i3c_master_probe(struct platform_device *pdev)
writel(IBIR_THR(1), master->regs + CMD_IBI_THR_CTRL);
writel(MST_INT_IBIR_THR, master->regs + MST_IER);
- writel(DEVS_CTRL_DEV_CLR_ALL, master->regs + DEVS_CTRL);
+
+ if (secondary) {
+ ret = cdns_i3c_sec_master_bus_init(master);
+ if (ret)
+ goto err_postpone_init;
+ } else
+ writel(DEVS_CTRL_DEV_CLR_ALL, master->regs + DEVS_CTRL);
ret = i3c_master_register(&master->base, &pdev->dev,
- &cdns_i3c_master_ops, false);
+ &cdns_i3c_master_ops, secondary);
if (ret)
goto err_disable_sysclk;
return 0;
+err_postpone_init:
+ master->wq = alloc_workqueue("%s", 0, 0, pdev->name);
+ if (!master->wq)
+ return -ENOMEM;
+
+ master->parent = &pdev->dev;
+ writel(SLV_INT_EVENT_UP, master->regs + SLV_IER);
+
+ return 0;
+
err_disable_sysclk:
clk_disable_unprepare(master->sysclk);
@@ -1635,6 +2022,9 @@ static int cdns_i3c_master_remove(struct platform_device *pdev)
struct cdns_i3c_master *master = platform_get_drvdata(pdev);
int ret;
+ if (master->wq)
+ destroy_workqueue(master->wq);
+
ret = i3c_master_unregister(&master->base);
if (ret)
return ret;
@@ -1661,6 +2051,7 @@ static struct platform_driver cdns_i3c_master = {
module_platform_driver(cdns_i3c_master);
MODULE_AUTHOR("Boris Brezillon <boris.brezillon@bootlin.com>");
+MODULE_AUTHOR("Przemyslaw Gaj <pgaj@cadence.com>");
MODULE_DESCRIPTION("Cadence I3C master driver");
MODULE_LICENSE("GPL v2");
MODULE_ALIAS("platform:cdns-i3c-master");
--
2.8.3
_______________________________________________
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 3/4] i3c: master: Add module author
2019-02-18 13:08 [PATCH 0/4] Add the I3C mastership request Przemyslaw Gaj
2019-02-18 13:08 ` [PATCH 1/4] i3c: Add support for mastership request to I3C subsystem Przemyslaw Gaj
2019-02-18 13:08 ` [PATCH 2/4] i3c: master: cdns: add support for mastership request to Cadence I3C master driver Przemyslaw Gaj
@ 2019-02-18 13:08 ` Przemyslaw Gaj
2019-02-18 13:08 ` [PATCH 4/4] MAINTAINERS: add myself as co-maintainer of I3C subsystem Przemyslaw Gaj
3 siblings, 0 replies; 8+ messages in thread
From: Przemyslaw Gaj @ 2019-02-18 13:08 UTC (permalink / raw)
To: bbrezillon; +Cc: linux-i3c, Przemyslaw Gaj, rafalc, vitor.soares
This adds myself as an author of I3C framework.
Signed-off-by: Przemyslaw Gaj <pgaj@cadence.com>
---
drivers/i3c/master.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
index 14493e5..01c6bf8 100644
--- a/drivers/i3c/master.c
+++ b/drivers/i3c/master.c
@@ -3,6 +3,7 @@
* Copyright (C) 2018 Cadence Design Systems Inc.
*
* Author: Boris Brezillon <boris.brezillon@bootlin.com>
+ * Author: Przemyslaw Gaj <pgaj@cadence.com>
*/
#include <linux/atomic.h>
@@ -2935,5 +2936,6 @@ static void __exit i3c_exit(void)
module_exit(i3c_exit);
MODULE_AUTHOR("Boris Brezillon <boris.brezillon@bootlin.com>");
+MODULE_AUTHOR("Przemyslaw Gaj <pgaj@cadence.com>");
MODULE_DESCRIPTION("I3C core");
MODULE_LICENSE("GPL v2");
--
2.8.3
_______________________________________________
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 4/4] MAINTAINERS: add myself as co-maintainer of I3C subsystem
2019-02-18 13:08 [PATCH 0/4] Add the I3C mastership request Przemyslaw Gaj
` (2 preceding siblings ...)
2019-02-18 13:08 ` [PATCH 3/4] i3c: master: Add module author Przemyslaw Gaj
@ 2019-02-18 13:08 ` Przemyslaw Gaj
3 siblings, 0 replies; 8+ messages in thread
From: Przemyslaw Gaj @ 2019-02-18 13:08 UTC (permalink / raw)
To: bbrezillon; +Cc: linux-i3c, Przemyslaw Gaj, rafalc, vitor.soares
As discussed with Boris Brezillon - I'm adding myself as the co-maintainer.
Signed-off-by: Przemyslaw Gaj <pgaj@cadence.com>
---
MAINTAINERS | 1 +
1 file changed, 1 insertion(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index 41ce5f4..37a25f8 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7168,6 +7168,7 @@ F: drivers/i2c/i2c-stub.c
I3C SUBSYSTEM
M: Boris Brezillon <bbrezillon@kernel.org>
+M: Przemyslaw Gaj <pgaj@cadence.com>
L: linux-i3c@lists.infradead.org
T: git git://git.kernel.org/pub/scm/linux/kernel/git/i3c/linux.git
S: Maintained
--
2.8.3
_______________________________________________
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 1/4] i3c: Add support for mastership request to I3C subsystem
2019-02-18 13:08 ` [PATCH 1/4] i3c: Add support for mastership request to I3C subsystem Przemyslaw Gaj
@ 2019-02-22 15:30 ` Boris Brezillon
2019-02-22 21:09 ` Przemyslaw Gaj
0 siblings, 1 reply; 8+ messages in thread
From: Boris Brezillon @ 2019-02-22 15:30 UTC (permalink / raw)
To: Przemyslaw Gaj; +Cc: linux-i3c, rafalc, vitor.soares
On Mon, 18 Feb 2019 13:08:43 +0000
Przemyslaw Gaj <pgaj@cadence.com> wrote:
> This patch adds support for mastership request to I3C subsystem.
>
> Mastership event is enabled globally.
>
> Mastership is requested automatically when device driver
> tries to transfer data using master controller in slave mode.
>
> There is still some limitation:
> - I2C devices are registered on secondary master side if boardinfo
> entry matching the info transmitted through the DEFSLVS frame.
>
> Signed-off-by: Przemyslaw Gaj <pgaj@cadence.com>
Next time, please add the changelog after a --- delimiter so that it
does not appear when I apply the patch.
>
> Main changes between v2 and v3:
> - Add i3c_bus_downgrade_maintenance_lock() for downgrading the bus
> lock from maintenance to normal use
> - Add additional fields to i2c_dev_desc for DEFSLVS purpose (addr, lvr)
> - Add i3c_master_register_new_i2c_devs() function to register I2C devices
> - Reworked I2C devices registration on secondary master side
>
> Changes in v2:
> - Add mastership disable event hook
> - Changed name of mastership enable event hook
> - Add function to test if master owns the bus
> - Removed op_mode
> - Changed parameter of i3c_master_get_accmst_locked, no need to
> pass full i3c_device_info
> - Removed redundant DEFSLVS command before GETACCMST
> - Add i3c_master_bus_takeover function. There is a need to lock
> the bus before adding devices and no matter of the controller
> devices have to be added after mastership takeover.
> - Add device registration during initialization on secondary master
> side. Devices received by DEFSLVS (if occured). If not, device
> initialization is deffered untill next mastership request.
> ---
> drivers/i3c/device.c | 47 +++++
> drivers/i3c/internals.h | 4 +
> drivers/i3c/master.c | 428 +++++++++++++++++++++++++++++++++++++--------
> include/linux/i3c/master.h | 22 +++
> 4 files changed, 427 insertions(+), 74 deletions(-)
>
> diff --git a/drivers/i3c/device.c b/drivers/i3c/device.c
> index 69cc040..f53d689 100644
> --- a/drivers/i3c/device.c
> +++ b/drivers/i3c/device.c
> @@ -43,6 +43,17 @@ int i3c_device_do_priv_xfers(struct i3c_device *dev,
> }
>
> i3c_bus_normaluse_lock(dev->bus);
> + if (!i3c_master_owns_bus_locked(dev->desc->common.master)) {
> + i3c_bus_normaluse_unlock(dev->bus);
> + i3c_bus_maintenance_lock(dev->bus);
> + ret = i3c_master_request_mastership_locked(dev->desc->common.master);
> + if (ret) {
> + i3c_bus_maintenance_unlock(dev->bus);
> + return ret;
> + }
> + i3c_bus_downgrade_maintenance_lock(dev->bus);
> + }
> +
Looks like this code check is duplicated, please create a helper
(i3c_master_acquire_bus_ownership()?).
> ret = i3c_dev_do_priv_xfers_locked(dev->desc, xfers, nxfers);
> i3c_bus_normaluse_unlock(dev->bus);
>
> @@ -114,6 +125,17 @@ int i3c_device_enable_ibi(struct i3c_device *dev)
> int ret = -ENOENT;
>
> i3c_bus_normaluse_lock(dev->bus);
> + if (!i3c_master_owns_bus_locked(dev->desc->common.master)) {
> + i3c_bus_normaluse_unlock(dev->bus);
> + i3c_bus_maintenance_lock(dev->bus);
> + ret = i3c_master_request_mastership_locked(dev->desc->common.master);
> + if (ret) {
> + i3c_bus_maintenance_unlock(dev->bus);
> + return ret;
> + }
> + i3c_bus_downgrade_maintenance_lock(dev->bus);
> + }
> +
> if (dev->desc) {
> mutex_lock(&dev->desc->ibi_lock);
> ret = i3c_dev_enable_ibi_locked(dev->desc);
> @@ -145,6 +167,18 @@ int i3c_device_request_ibi(struct i3c_device *dev,
> return -EINVAL;
>
> i3c_bus_normaluse_lock(dev->bus);
> +
> + if (!i3c_master_owns_bus_locked(dev->desc->common.master)) {
> + i3c_bus_normaluse_unlock(dev->bus);
> + i3c_bus_maintenance_lock(dev->bus);
> + ret = i3c_master_request_mastership_locked(dev->desc->common.master);
> + if (ret) {
> + i3c_bus_maintenance_unlock(dev->bus);
> + return ret;
> + }
> + i3c_bus_downgrade_maintenance_lock(dev->bus);
> + }
> +
> if (dev->desc) {
> mutex_lock(&dev->desc->ibi_lock);
> ret = i3c_dev_request_ibi_locked(dev->desc, req);
> @@ -166,7 +200,20 @@ EXPORT_SYMBOL_GPL(i3c_device_request_ibi);
> */
> void i3c_device_free_ibi(struct i3c_device *dev)
> {
> + int ret;
> +
> i3c_bus_normaluse_lock(dev->bus);
> + if (!i3c_master_owns_bus_locked(dev->desc->common.master)) {
> + i3c_bus_normaluse_unlock(dev->bus);
> + i3c_bus_maintenance_lock(dev->bus);
> + ret = i3c_master_request_mastership_locked(dev->desc->common.master);
> + if (ret) {
> + i3c_bus_maintenance_unlock(dev->bus);
> + return;
> + }
> + i3c_bus_downgrade_maintenance_lock(dev->bus);
> + }
> +
> if (dev->desc) {
> mutex_lock(&dev->desc->ibi_lock);
> i3c_dev_free_ibi_locked(dev->desc);
> diff --git a/drivers/i3c/internals.h b/drivers/i3c/internals.h
> index 86b7b44..7e9f1fb 100644
> --- a/drivers/i3c/internals.h
> +++ b/drivers/i3c/internals.h
> @@ -14,6 +14,10 @@ extern struct bus_type i3c_bus_type;
>
> void i3c_bus_normaluse_lock(struct i3c_bus *bus);
> void i3c_bus_normaluse_unlock(struct i3c_bus *bus);
> +void i3c_bus_downgrade_maintenance_lock(struct i3c_bus *bus);
> +bool i3c_master_owns_bus_locked(struct i3c_master_controller *master);
> +int i3c_master_request_mastership_locked(struct i3c_master_controller *master);
> +
>
> int i3c_dev_do_priv_xfers_locked(struct i3c_dev_desc *dev,
> struct i3c_priv_xfer *xfers,
> diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
> index 2dc628d..14493e5 100644
> --- a/drivers/i3c/master.c
> +++ b/drivers/i3c/master.c
> @@ -38,10 +38,11 @@ static DEFINE_MUTEX(i3c_core_lock);
> * logic to rely on I3C device information that could be changed behind their
> * back.
> */
> -static void i3c_bus_maintenance_lock(struct i3c_bus *bus)
> +void i3c_bus_maintenance_lock(struct i3c_bus *bus)
> {
> down_write(&bus->lock);
> }
> +EXPORT_SYMBOL_GPL(i3c_bus_maintenance_lock);
>
> /**
> * i3c_bus_maintenance_unlock - Release the bus lock after a maintenance
> @@ -52,10 +53,11 @@ static void i3c_bus_maintenance_lock(struct i3c_bus *bus)
> * i3c_bus_maintenance_lock() for more details on what these maintenance
> * operations are.
> */
> -static void i3c_bus_maintenance_unlock(struct i3c_bus *bus)
> +void i3c_bus_maintenance_unlock(struct i3c_bus *bus)
> {
> up_write(&bus->lock);
> }
> +EXPORT_SYMBOL_GPL(i3c_bus_maintenance_unlock);
This should be done in a separate patch explaining why you need to
export those funcs. Also, functions defined in drivers/i3c/internals.h
should never be exported. If the function is about to be used by master
controller drivers, it should be defined in include/linux/i3c/master.h.
>
> /**
> * i3c_bus_normaluse_lock - Lock the bus for a normal operation
> @@ -91,6 +93,20 @@ void i3c_bus_normaluse_unlock(struct i3c_bus *bus)
> up_read(&bus->lock);
> }
>
> +/**
> + * i3c_bus_downgrade_maintenance_lock - Downgrade the bus lock to normal
> + * operation
> + * @bus: I3C bus to downgrade the lock on
> + *
> + * Should be called when a maintenance operation is done and normal
> + * operation is planned. See i3c_bus_maintenance_lock() and
> + * i3c_bus_normaluse_lock() for more details.
> + */
> +void i3c_bus_downgrade_maintenance_lock(struct i3c_bus *bus)
> +{
> + downgrade_write(&bus->lock);
> +}
> +
> static struct i3c_master_controller *dev_to_i3cmaster(struct device *dev)
> {
> return container_of(dev, struct i3c_master_controller, dev);
> @@ -339,6 +355,22 @@ static int i3c_device_probe(struct device *dev)
> return driver->probe(i3cdev);
> }
>
> +static void i3c_master_enable_mr_events(struct i3c_master_controller *master)
> +{
> + if (!master->ops->enable_mr_events)
> + return;
> +
> + master->ops->enable_mr_events(master);
> +}
> +
> +static void i3c_master_disable_mr_events(struct i3c_master_controller *master)
> +{
> + if (!master->ops->disable_mr_events)
> + return;
> +
> + master->ops->disable_mr_events(master);
> +}
> +
> static int i3c_device_remove(struct device *dev)
> {
> struct i3c_device *i3cdev = dev_to_i3cdev(dev);
> @@ -460,6 +492,23 @@ static int i3c_bus_init(struct i3c_bus *i3cbus)
> return 0;
> }
>
> +int i3c_master_request_mastership_locked(struct i3c_master_controller *master)
> +{
> + if (WARN_ON(master->init_done &&
> + !rwsem_is_locked(&master->bus.lock)))
> + return -EINVAL;
> +
> + if (!master->ops->request_mastership)
> + return -ENOTSUPP;
> +
> + return master->ops->request_mastership(master);
> +}
> +
> +bool i3c_master_owns_bus_locked(struct i3c_master_controller *master)
> +{
> + return master->bus.cur_master == master->this;
> +}
If you create the i3c_master_acquire_bus_ownership() helper, you
should need i3c_master_owns_bus_locked().
> +
> static const char * const i3c_bus_mode_strings[] = {
> [I3C_BUS_MODE_PURE] = "pure",
> [I3C_BUS_MODE_MIXED_FAST] = "mixed-fast",
> @@ -618,6 +667,25 @@ i3c_master_alloc_i2c_dev(struct i3c_master_controller *master,
>
> dev->common.master = master;
> dev->boardinfo = boardinfo;
> + dev->addr = boardinfo->base.addr;
> + dev->lvr = boardinfo->lvr;
Please do that in a separate patch (I mean, the part that adds ->lvr
and ->addr fields to the i3c_dev_desc object).
> +
> + return dev;
> +}
> +
> +static struct i2c_dev_desc *
> +i3c_master_alloc_i2c_dev_no_boardinfo(struct i3c_master_controller *master,
> + u16 addr, u8 lvr)
> +{
> + struct i2c_dev_desc *dev;
> +
> + dev = kzalloc(sizeof(*dev), GFP_KERNEL);
> + if (!dev)
> + return ERR_PTR(-ENOMEM);
> +
> + dev->common.master = master;
> + dev->addr = addr;
> + dev->lvr = lvr;
>
> return dev;
> }
> @@ -691,6 +759,9 @@ i3c_master_find_i2c_dev_by_addr(const struct i3c_master_controller *master,
> struct i2c_dev_desc *dev;
>
> i3c_bus_for_each_i2cdev(&master->bus, dev) {
> + if (!dev->boardinfo)
> + continue;
> +
> if (dev->boardinfo->base.addr == addr)
> return dev;
> }
> @@ -937,8 +1008,8 @@ int i3c_master_defslvs_locked(struct i3c_master_controller *master)
>
> desc = defslvs->slaves;
> i3c_bus_for_each_i2cdev(bus, i2cdev) {
> - desc->lvr = i2cdev->boardinfo->lvr;
> - desc->static_addr = i2cdev->boardinfo->base.addr << 1;
> + desc->lvr = i2cdev->lvr;
> + desc->static_addr = i2cdev->addr << 1;
> desc++;
> }
>
> @@ -1490,6 +1561,83 @@ i3c_master_register_new_i3c_devs(struct i3c_master_controller *master)
> }
> }
>
> +static struct i2c_dev_boardinfo *
> +i3c_master_find_i2c_boardinfo(const struct i3c_master_controller *master,
> + u16 addr, u8 lvr)
> +{
> + struct i2c_dev_boardinfo *i2cboardinfo;
> +
> + list_for_each_entry(i2cboardinfo, &master->boardinfo.i2c, node) {
> + if (i2cboardinfo->base.addr == addr &&
> + i2cboardinfo->lvr == lvr)
> + return i2cboardinfo;
> + }
> +
> + return NULL;
> +}
> +
> +static void
> +i3c_master_register_new_i2c_devs(struct i3c_master_controller *master)
> +{
> + struct i2c_adapter *adap = i3c_master_to_i2c_adapter(master);
> + struct i2c_dev_desc *i2cdev;
> +
> + if (!master->init_done)
> + return;
> +
> + i3c_bus_for_each_i2cdev(&master->bus, i2cdev) {
> + if (i2cdev->dev)
> + continue;
> +
> + if (!i2cdev->boardinfo)
> + continue;
> +
> + i2cdev->dev = i2c_new_device(adap, &i2cdev->boardinfo->base);
> + }
> +}
> +
> +/**
> + * i3c_master_get_accmst_locked() - send a GETACCMST CCC command
> + * @master: master used to send frames on the bus
> + * @info: I3C device information
> + *
> + * Send a GETACCMST CCC command.
> + *
> + * This should be called if the curent master acknowledges bus takeover.
> + *
> + * This function must be called with the bus lock held in write mode.
> + *
> + * Return: 0 in case of success, a positive I3C error code if the error is
> + * one of the official Mx error codes, and a negative error code otherwise.
> + */
> +int i3c_master_get_accmst_locked(struct i3c_master_controller *master,
> + u8 addr)
> +{
> + struct i3c_ccc_getaccmst *accmst;
> + struct i3c_ccc_cmd_dest dest;
> + struct i3c_ccc_cmd cmd;
> + int ret;
> +
> + accmst = i3c_ccc_cmd_dest_init(&dest, addr, sizeof(*accmst));
> + if (!accmst)
> + return -ENOMEM;
> +
> + i3c_ccc_cmd_init(&cmd, true, I3C_CCC_GETACCMST, &dest, 1);
> +
> + ret = i3c_master_send_ccc_cmd_locked(master, &cmd);
> + if (ret)
> + goto out;
> +
> + if (dest.payload.len != sizeof(*accmst))
> + ret = -EIO;
> +
> +out:
> + i3c_ccc_cmd_dest_cleanup(&dest);
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(i3c_master_get_accmst_locked);
> +
> /**
> * i3c_master_do_daa() - do a DAA (Dynamic Address Assignment)
> * @master: master doing the DAA
> @@ -1554,12 +1702,9 @@ int i3c_master_set_info(struct i3c_master_controller *master,
> struct i3c_dev_desc *i3cdev;
> int ret;
>
> - if (!i3c_bus_dev_addr_is_avail(&master->bus, info->dyn_addr))
> - return -EINVAL;
> -
> - if (I3C_BCR_DEVICE_ROLE(info->bcr) == I3C_BCR_I3C_MASTER &&
> - master->secondary)
> - return -EINVAL;
> + if (!master->secondary)
> + if (!i3c_bus_dev_addr_is_avail(&master->bus, info->dyn_addr))
> + return -EINVAL;
Please, add curly braces around the first if block. Also, why don't you
check if the address is valid in that case?
>
> if (master->this)
> return -EINVAL;
> @@ -1605,43 +1750,13 @@ static void i3c_master_detach_free_devs(struct i3c_master_controller *master)
> common.node) {
> i3c_master_detach_i2c_dev(i2cdev);
> i3c_bus_set_addr_slot_status(&master->bus,
> - i2cdev->boardinfo->base.addr,
> - I3C_ADDR_SLOT_FREE);
> + i2cdev->addr,
> + I3C_ADDR_SLOT_FREE);
> i3c_master_free_i2c_dev(i2cdev);
> }
> }
>
> -/**
> - * i3c_master_bus_init() - initialize an I3C bus
> - * @master: main master initializing the bus
> - *
> - * This function is following all initialisation steps described in the I3C
> - * specification:
> - *
> - * 1. Attach I2C and statically defined I3C devs to the master so that the
> - * master can fill its internal device table appropriately
> - *
> - * 2. Call &i3c_master_controller_ops->bus_init() method to initialize
> - * the master controller. That's usually where the bus mode is selected
> - * (pure bus or mixed fast/slow bus)
> - *
> - * 3. Instruct all devices on the bus to drop their dynamic address. This is
> - * particularly important when the bus was previously configured by someone
> - * else (for example the bootloader)
> - *
> - * 4. Disable all slave events.
> - *
> - * 5. Pre-assign dynamic addresses requested by the FW with SETDASA for I3C
> - * devices that have a static address
> - *
> - * 6. Do a DAA (Dynamic Address Assignment) to assign dynamic addresses to all
> - * remaining I3C devices
> - *
> - * Once this is done, all I3C and I2C devices should be usable.
> - *
> - * Return: a 0 in case of success, an negative error code otherwise.
> - */
> -static int i3c_master_bus_init(struct i3c_master_controller *master)
> +static int i3c_master_attach_static_devs(struct i3c_master_controller *master)
> {
> enum i3c_addr_slot_status status;
> struct i2c_dev_boardinfo *i2cboardinfo;
> @@ -1650,32 +1765,24 @@ static int i3c_master_bus_init(struct i3c_master_controller *master)
> struct i2c_dev_desc *i2cdev;
> int ret;
>
> - /*
> - * First attach all devices with static definitions provided by the
> - * FW.
> - */
> list_for_each_entry(i2cboardinfo, &master->boardinfo.i2c, node) {
> status = i3c_bus_get_addr_slot_status(&master->bus,
> i2cboardinfo->base.addr);
> - if (status != I3C_ADDR_SLOT_FREE) {
> - ret = -EBUSY;
> - goto err_detach_devs;
> - }
> + if (status != I3C_ADDR_SLOT_FREE)
> + return -EBUSY;
>
> i3c_bus_set_addr_slot_status(&master->bus,
> i2cboardinfo->base.addr,
> I3C_ADDR_SLOT_I2C_DEV);
>
> i2cdev = i3c_master_alloc_i2c_dev(master, i2cboardinfo);
> - if (IS_ERR(i2cdev)) {
> - ret = PTR_ERR(i2cdev);
> - goto err_detach_devs;
> - }
> + if (IS_ERR(i2cdev))
> + return PTR_ERR(i2cdev);
>
> ret = i3c_master_attach_i2c_dev(master, i2cdev);
> if (ret) {
> i3c_master_free_i2c_dev(i2cdev);
> - goto err_detach_devs;
> + return ret;
> }
> }
> list_for_each_entry(i3cboardinfo, &master->boardinfo.i3c, node) {
> @@ -1685,28 +1792,71 @@ static int i3c_master_bus_init(struct i3c_master_controller *master)
>
> if (i3cboardinfo->init_dyn_addr) {
> status = i3c_bus_get_addr_slot_status(&master->bus,
> - i3cboardinfo->init_dyn_addr);
> - if (status != I3C_ADDR_SLOT_FREE) {
> - ret = -EBUSY;
> - goto err_detach_devs;
> - }
> + i3cboardinfo->init_dyn_addr);
> + if (status != I3C_ADDR_SLOT_FREE)
> + return -EBUSY;
> }
>
> i3cdev = i3c_master_alloc_i3c_dev(master, &info);
> - if (IS_ERR(i3cdev)) {
> - ret = PTR_ERR(i3cdev);
> - goto err_detach_devs;
> - }
> + if (IS_ERR(i3cdev))
> + return PTR_ERR(i3cdev);
>
> i3cdev->boardinfo = i3cboardinfo;
>
> ret = i3c_master_attach_i3c_dev(master, i3cdev);
> if (ret) {
> i3c_master_free_i3c_dev(i3cdev);
> - goto err_detach_devs;
> + return ret;
> }
> }
>
> + return 0;
> +}
> +
> +/**
> + * i3c_master_bus_init() - initialize an I3C bus
> + * @master: main master initializing the bus
> + *
> + * This function is following all initialisation steps described in the I3C
> + * specification:
> + *
> + * 1. Attach I2C and statically defined I3C devs to the master so that the
> + * master can fill its internal device table appropriately
> + *
> + * 2. Call &i3c_master_controller_ops->bus_init() method to initialize
> + * the master controller. That's usually where the bus mode is selected
> + * (pure bus or mixed fast/slow bus)
> + *
> + * 3. Instruct all devices on the bus to drop their dynamic address. This is
> + * particularly important when the bus was previously configured by someone
> + * else (for example the bootloader)
> + *
> + * 4. Disable all slave events.
> + *
> + * 5. Pre-assign dynamic addresses requested by the FW with SETDASA for I3C
> + * devices that have a static address
> + *
> + * 6. Do a DAA (Dynamic Address Assignment) to assign dynamic addresses to all
> + * remaining I3C devices
> + *
> + * Once this is done, all I3C and I2C devices should be usable.
> + *
> + * Return: a 0 in case of success, an negative error code otherwise.
> + */
> +static int i3c_master_bus_init(struct i3c_master_controller *master)
> +{
> + struct i3c_dev_desc *i3cdev;
> + int ret;
> +
> + /*
> + * First attach all devices with static definitions provided by the
> + * FW.
> + */
> + if (!master->secondary) {
> + ret = i3c_master_attach_static_devs(master);
> + if (ret)
> + goto err_detach_devs;
> + }
> /*
> * Now execute the controller specific ->bus_init() routine, which
> * might configure its internal logic to match the bus limitations.
> @@ -1727,6 +1877,13 @@ static int i3c_master_bus_init(struct i3c_master_controller *master)
> }
>
> /*
> + * Don't reset addresses if this is secondary master.
> + * Secondary masters can't do DAA.
> + */
> + if (master->secondary)
> + return 0;
> +
> + /*
> * Reset all dynamic address that may have been assigned before
> * (assigned by the bootloader for example).
> */
> @@ -1789,6 +1946,56 @@ i3c_master_search_i3c_dev_duplicate(struct i3c_dev_desc *refdev)
> return NULL;
> }
>
> +int i3c_master_add_i2c_dev(struct i3c_master_controller *master,
> + u16 addr, u8 lvr)
> +{
> + enum i3c_addr_slot_status status;
> + struct i2c_dev_boardinfo *boardinfo;
> + struct i2c_dev_desc *i2cdev;
> + int ret;
> +
> + if (!master)
> + return -EINVAL;
> +
> + status = i3c_bus_get_addr_slot_status(&master->bus,
> + addr);
> + if (status != I3C_ADDR_SLOT_FREE)
> + return -EBUSY;
> +
> + i3c_bus_set_addr_slot_status(&master->bus, addr,
> + I3C_ADDR_SLOT_I2C_DEV);
> +
> + i2cdev = i3c_master_alloc_i2c_dev_no_boardinfo(master, addr, lvr);
> + if (IS_ERR(i2cdev)) {
> + ret = PTR_ERR(i2cdev);
> + goto err_free_dev;
> + }
> +
> + boardinfo = i3c_master_find_i2c_boardinfo(master, addr, lvr);
> + if (!boardinfo) {
> + ret = -ENODEV;
> + goto err_free_dev;
> + }
Don't you need to keep the i2c_dev_desc around even if there's no
boardinfo associated to it? I thought you had to do that to be able to
send DEFSLVS when the secondary master receives a H-J request and needs
to send a new DEFSLVS frame.
> +
> + i2cdev->boardinfo = boardinfo;
> +
> + ret = i3c_master_attach_i2c_dev(master, i2cdev);
> + if (ret) {
> + ret = PTR_ERR(i2cdev);
> + goto err_free_dev;
> + }
> +
> + return 0;
> +
> +err_free_dev:
> + i3c_bus_set_addr_slot_status(&master->bus, addr,
> + I3C_ADDR_SLOT_FREE);
> + i3c_master_free_i2c_dev(i2cdev);
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(i3c_master_add_i2c_dev);
> +
> /**
> * i3c_master_add_i3c_dev_locked() - add an I3C slave to the bus
> * @master: master used to send frames on the bus
> @@ -2101,6 +2308,18 @@ static int i3c_master_i2c_adapter_xfer(struct i2c_adapter *adap,
> }
>
> i3c_bus_normaluse_lock(&master->bus);
> + if (!i3c_master_owns_bus_locked(master)) {
> + i3c_bus_normaluse_unlock(&master->bus);
> + i3c_bus_maintenance_lock(&master->bus);
> +
> + ret = i3c_master_request_mastership_locked(master);
> + if (ret) {
> + i3c_bus_maintenance_unlock(&master->bus);
> + return ret;
> + }
> + i3c_bus_downgrade_maintenance_lock(&master->bus);
> + }
> +
> dev = i3c_master_find_i2c_dev_by_addr(master, addr);
> if (!dev)
> ret = -ENOENT;
> @@ -2146,8 +2365,12 @@ static int i3c_master_i2c_adapter_init(struct i3c_master_controller *master)
> * We silently ignore failures here. The bus should keep working
> * correctly even if one or more i2c devices are not registered.
> */
> - i3c_bus_for_each_i2cdev(&master->bus, i2cdev)
> + i3c_bus_for_each_i2cdev(&master->bus, i2cdev) {
> + if (!i2cdev->boardinfo)
> + continue;
> +
> i2cdev->dev = i2c_new_device(adap, &i2cdev->boardinfo->base);
> + }
>
> return 0;
> }
> @@ -2388,18 +2611,43 @@ static int i3c_master_check_ops(const struct i3c_master_controller_ops *ops)
> !ops->recycle_ibi_slot))
> return -EINVAL;
>
> + if (ops->request_mastership &&
> + (!ops->enable_mr_events || !ops->disable_mr_events))
> + return -EINVAL;
> +
> return 0;
> }
>
> /**
> + * i3c_master_bus_takeover() - register new I3C devices on bus takeover
> + * @master: master used to send frames on the bus
> + *
> + * This function is useful when devices were not added
> + * during initialization or when new device joined the bus
> + * which was under control of different master.
> + */
> +void i3c_master_bus_takeover(struct i3c_master_controller *master)
Seems like the name does not match what the function does. How about
i3c_master_register_new_devs(). The fact that it's call when a
secondary master acquires the bus is just a detail.
> +{
> + /*
> + * We can register I3C devices received from master by DEFSLVS.
> + */
> + i3c_bus_normaluse_lock(&master->bus);
> + i3c_master_register_new_i3c_devs(master);
> + i3c_bus_normaluse_unlock(&master->bus);
> +
> + i3c_bus_normaluse_lock(&master->bus);
> + i3c_master_register_new_i2c_devs(master);
> + i3c_bus_normaluse_unlock(&master->bus);
> +}
> +EXPORT_SYMBOL_GPL(i3c_master_bus_takeover);
> +
> +/**
> * i3c_master_register() - register an I3C master
> * @master: master used to send frames on the bus
> * @parent: the parent device (the one that provides this I3C master
> * controller)
> * @ops: the master controller operations
> - * @secondary: true if you are registering a secondary master. Will return
> - * -ENOTSUPP if set to true since secondary masters are not yet
> - * supported
> + * @secondary: true if you are registering a secondary master.
> *
> * This function takes care of everything for you:
> *
> @@ -2422,10 +2670,6 @@ int i3c_master_register(struct i3c_master_controller *master,
> struct i2c_dev_boardinfo *i2cbi;
> int ret;
>
> - /* We do not support secondary masters yet. */
> - if (secondary)
> - return -ENOTSUPP;
> -
> ret = i3c_master_check_ops(ops);
> if (ret)
> return ret;
> @@ -2503,6 +2747,14 @@ int i3c_master_register(struct i3c_master_controller *master,
> i3c_master_register_new_i3c_devs(master);
> i3c_bus_normaluse_unlock(&master->bus);
>
> + i3c_bus_normaluse_lock(&master->bus);
> + i3c_master_register_new_i2c_devs(master);
> + i3c_bus_normaluse_unlock(&master->bus);
You could call i3c_master_register_new_devs() here instead of
duplicating the code.
> +
> + i3c_bus_maintenance_lock(&master->bus);
> + i3c_master_enable_mr_events(master);
> + i3c_bus_maintenance_unlock(&master->bus);
We might want to make that configurable per-bus using a DT prop (or a
module param), but let's say we accept MR requests by default.
> +
> return 0;
>
> err_del_dev:
> @@ -2519,6 +2771,30 @@ int i3c_master_register(struct i3c_master_controller *master,
> EXPORT_SYMBOL_GPL(i3c_master_register);
>
> /**
> + * i3c_master_mastership_ack() - performs operations before bus handover.
> + * @master: master used to send frames on the bus
> + * @addr: I3C device address
> + *
> + * Basically, it sends DEFSLVS command to ensure new master is taking
> + * the bus with complete list of devices and then acknowledges bus takeover.
> + *
> + * Return: 0 in case of success, a negative error code otherwise.
> + */
> +int i3c_master_mastership_ack(struct i3c_master_controller *master,
> + u8 addr)
> +{
> + int ret;
> +
> + i3c_bus_maintenance_lock(&master->bus);
> + ret = i3c_master_get_accmst_locked(master, addr);
> + i3c_bus_maintenance_unlock(&master->bus);
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(i3c_master_mastership_ack);
> +
> +
> +/**
> * i3c_master_unregister() - unregister an I3C master
> * @master: master used to send frames on the bus
> *
> @@ -2528,6 +2804,10 @@ EXPORT_SYMBOL_GPL(i3c_master_register);
> */
> int i3c_master_unregister(struct i3c_master_controller *master)
> {
> + i3c_bus_maintenance_lock(&master->bus);
> + i3c_master_disable_mr_events(master);
> + i3c_bus_maintenance_unlock(&master->bus);
> +
> i3c_master_i2c_adapter_cleanup(master);
> i3c_master_unregister_i3c_devs(master);
> i3c_master_bus_cleanup(master);
> diff --git a/include/linux/i3c/master.h b/include/linux/i3c/master.h
> index f13fd8b..8514680 100644
> --- a/include/linux/i3c/master.h
> +++ b/include/linux/i3c/master.h
> @@ -84,6 +84,8 @@ struct i2c_dev_desc {
> struct i3c_i2c_dev_desc common;
> const struct i2c_dev_boardinfo *boardinfo;
> struct i2c_client *dev;
> + u16 addr;
> + u8 lvr;
> };
>
> /**
> @@ -418,6 +420,12 @@ struct i3c_bus {
> * for a future IBI
> * This method is mandatory only if ->request_ibi is not
> * NULL.
> + * @request_mastership: requests bus mastership. Mastership is requested
> + * automatically when device driver wants to transfer
> + * data using master in slave mode.
> + * @enable_mr_events: enable the Mastership event.
> + * Mastership does not require handler.
> + * @disable_mr_events: disable the Mastership event.
> */
> struct i3c_master_controller_ops {
> int (*bus_init)(struct i3c_master_controller *master);
> @@ -445,6 +453,10 @@ struct i3c_master_controller_ops {
> int (*disable_ibi)(struct i3c_dev_desc *dev);
> void (*recycle_ibi_slot)(struct i3c_dev_desc *dev,
> struct i3c_ibi_slot *slot);
> + void (*update_devs)(struct i3c_master_controller *master);
Undocumented hook (I'm not even sure you're using it in the code).
> + int (*request_mastership)(struct i3c_master_controller *master);
> + int (*enable_mr_events)(struct i3c_master_controller *master);
> + int (*disable_mr_events)(struct i3c_master_controller *master);
> };
>
> /**
> @@ -524,6 +536,8 @@ int i3c_master_defslvs_locked(struct i3c_master_controller *master);
> int i3c_master_get_free_addr(struct i3c_master_controller *master,
> u8 start_addr);
>
> +int i3c_master_add_i2c_dev(struct i3c_master_controller *master,
> + u16 addr, u8 lvr);
> int i3c_master_add_i3c_dev_locked(struct i3c_master_controller *master,
> u8 addr);
> int i3c_master_do_daa(struct i3c_master_controller *master);
> @@ -536,6 +550,11 @@ int i3c_master_register(struct i3c_master_controller *master,
> const struct i3c_master_controller_ops *ops,
> bool secondary);
> int i3c_master_unregister(struct i3c_master_controller *master);
> +int i3c_master_get_accmst_locked(struct i3c_master_controller *master,
> + u8 addr);
> +int i3c_master_mastership_ack(struct i3c_master_controller *master,
> + u8 addr);
> +void i3c_master_bus_takeover(struct i3c_master_controller *master);
>
> /**
> * i3c_dev_get_master_data() - get master private data attached to an I3C
> @@ -645,4 +664,7 @@ void i3c_master_queue_ibi(struct i3c_dev_desc *dev, struct i3c_ibi_slot *slot);
>
> struct i3c_ibi_slot *i3c_master_get_free_ibi_slot(struct i3c_dev_desc *dev);
>
> +void i3c_bus_maintenance_lock(struct i3c_bus *bus);
> +void i3c_bus_maintenance_unlock(struct i3c_bus *bus);
> +
> #endif /* I3C_MASTER_H */
_______________________________________________
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/4] i3c: Add support for mastership request to I3C subsystem
2019-02-22 15:30 ` Boris Brezillon
@ 2019-02-22 21:09 ` Przemyslaw Gaj
2019-02-25 8:51 ` Boris Brezillon
0 siblings, 1 reply; 8+ messages in thread
From: Przemyslaw Gaj @ 2019-02-22 21:09 UTC (permalink / raw)
To: Boris Brezillon; +Cc: linux-i3c, rafalc, vitor.soares
Thank you for the review.
The 02/22/2019 16:30, Boris Brezillon wrote:
>
>
> On Mon, 18 Feb 2019 13:08:43 +0000
> Przemyslaw Gaj <pgaj@cadence.com> wrote:
>
> > This patch adds support for mastership request to I3C subsystem.
> >
> > Mastership event is enabled globally.
> >
> > Mastership is requested automatically when device driver
> > tries to transfer data using master controller in slave mode.
> >
> > There is still some limitation:
> > - I2C devices are registered on secondary master side if boardinfo
> > entry matching the info transmitted through the DEFSLVS frame.
> >
> > Signed-off-by: Przemyslaw Gaj <pgaj@cadence.com>
>
> Next time, please add the changelog after a --- delimiter so that it
> does not appear when I apply the patch.
>
Got it.
> >
> > Main changes between v2 and v3:
> > - Add i3c_bus_downgrade_maintenance_lock() for downgrading the bus
> > lock from maintenance to normal use
> > - Add additional fields to i2c_dev_desc for DEFSLVS purpose (addr, lvr)
> > - Add i3c_master_register_new_i2c_devs() function to register I2C devices
> > - Reworked I2C devices registration on secondary master side
> >
> > Changes in v2:
> > - Add mastership disable event hook
> > - Changed name of mastership enable event hook
> > - Add function to test if master owns the bus
> > - Removed op_mode
> > - Changed parameter of i3c_master_get_accmst_locked, no need to
> > pass full i3c_device_info
> > - Removed redundant DEFSLVS command before GETACCMST
> > - Add i3c_master_bus_takeover function. There is a need to lock
> > the bus before adding devices and no matter of the controller
> > devices have to be added after mastership takeover.
> > - Add device registration during initialization on secondary master
> > side. Devices received by DEFSLVS (if occured). If not, device
> > initialization is deffered untill next mastership request.
> > ---
> > drivers/i3c/device.c | 47 +++++
> > drivers/i3c/internals.h | 4 +
> > drivers/i3c/master.c | 428 +++++++++++++++++++++++++++++++++++++--------
> > include/linux/i3c/master.h | 22 +++
> > 4 files changed, 427 insertions(+), 74 deletions(-)
> >
> > diff --git a/drivers/i3c/device.c b/drivers/i3c/device.c
> > index 69cc040..f53d689 100644
> > --- a/drivers/i3c/device.c
> > +++ b/drivers/i3c/device.c
> > @@ -43,6 +43,17 @@ int i3c_device_do_priv_xfers(struct i3c_device *dev,
> > }
> >
> > i3c_bus_normaluse_lock(dev->bus);
> > + if (!i3c_master_owns_bus_locked(dev->desc->common.master)) {
> > + i3c_bus_normaluse_unlock(dev->bus);
> > + i3c_bus_maintenance_lock(dev->bus);
> > + ret = i3c_master_request_mastership_locked(dev->desc->common.master);
> > + if (ret) {
> > + i3c_bus_maintenance_unlock(dev->bus);
> > + return ret;
> > + }
> > + i3c_bus_downgrade_maintenance_lock(dev->bus);
> > + }
> > +
>
> Looks like this code check is duplicated, please create a helper
> (i3c_master_acquire_bus_ownership()?).
>
Sure.
> > ret = i3c_dev_do_priv_xfers_locked(dev->desc, xfers, nxfers);
> > i3c_bus_normaluse_unlock(dev->bus);
> >
> > @@ -114,6 +125,17 @@ int i3c_device_enable_ibi(struct i3c_device *dev)
> > int ret = -ENOENT;
> >
> > i3c_bus_normaluse_lock(dev->bus);
> > + if (!i3c_master_owns_bus_locked(dev->desc->common.master)) {
> > + i3c_bus_normaluse_unlock(dev->bus);
> > + i3c_bus_maintenance_lock(dev->bus);
> > + ret = i3c_master_request_mastership_locked(dev->desc->common.master);
> > + if (ret) {
> > + i3c_bus_maintenance_unlock(dev->bus);
> > + return ret;
> > + }
> > + i3c_bus_downgrade_maintenance_lock(dev->bus);
> > + }
> > +
> > if (dev->desc) {
> > mutex_lock(&dev->desc->ibi_lock);
> > ret = i3c_dev_enable_ibi_locked(dev->desc);
> > @@ -145,6 +167,18 @@ int i3c_device_request_ibi(struct i3c_device *dev,
> > return -EINVAL;
> >
> > i3c_bus_normaluse_lock(dev->bus);
> > +
> > + if (!i3c_master_owns_bus_locked(dev->desc->common.master)) {
> > + i3c_bus_normaluse_unlock(dev->bus);
> > + i3c_bus_maintenance_lock(dev->bus);
> > + ret = i3c_master_request_mastership_locked(dev->desc->common.master);
> > + if (ret) {
> > + i3c_bus_maintenance_unlock(dev->bus);
> > + return ret;
> > + }
> > + i3c_bus_downgrade_maintenance_lock(dev->bus);
> > + }
> > +
> > if (dev->desc) {
> > mutex_lock(&dev->desc->ibi_lock);
> > ret = i3c_dev_request_ibi_locked(dev->desc, req);
> > @@ -166,7 +200,20 @@ EXPORT_SYMBOL_GPL(i3c_device_request_ibi);
> > */
> > void i3c_device_free_ibi(struct i3c_device *dev)
> > {
> > + int ret;
> > +
> > i3c_bus_normaluse_lock(dev->bus);
> > + if (!i3c_master_owns_bus_locked(dev->desc->common.master)) {
> > + i3c_bus_normaluse_unlock(dev->bus);
> > + i3c_bus_maintenance_lock(dev->bus);
> > + ret = i3c_master_request_mastership_locked(dev->desc->common.master);
> > + if (ret) {
> > + i3c_bus_maintenance_unlock(dev->bus);
> > + return;
> > + }
> > + i3c_bus_downgrade_maintenance_lock(dev->bus);
> > + }
> > +
> > if (dev->desc) {
> > mutex_lock(&dev->desc->ibi_lock);
> > i3c_dev_free_ibi_locked(dev->desc);
> > diff --git a/drivers/i3c/internals.h b/drivers/i3c/internals.h
> > index 86b7b44..7e9f1fb 100644
> > --- a/drivers/i3c/internals.h
> > +++ b/drivers/i3c/internals.h
> > @@ -14,6 +14,10 @@ extern struct bus_type i3c_bus_type;
> >
> > void i3c_bus_normaluse_lock(struct i3c_bus *bus);
> > void i3c_bus_normaluse_unlock(struct i3c_bus *bus);
> > +void i3c_bus_downgrade_maintenance_lock(struct i3c_bus *bus);
> > +bool i3c_master_owns_bus_locked(struct i3c_master_controller *master);
> > +int i3c_master_request_mastership_locked(struct i3c_master_controller *master);
> > +
> >
> > int i3c_dev_do_priv_xfers_locked(struct i3c_dev_desc *dev,
> > struct i3c_priv_xfer *xfers,
> > diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
> > index 2dc628d..14493e5 100644
> > --- a/drivers/i3c/master.c
> > +++ b/drivers/i3c/master.c
> > @@ -38,10 +38,11 @@ static DEFINE_MUTEX(i3c_core_lock);
> > * logic to rely on I3C device information that could be changed behind their
> > * back.
> > */
> > -static void i3c_bus_maintenance_lock(struct i3c_bus *bus)
> > +void i3c_bus_maintenance_lock(struct i3c_bus *bus)
> > {
> > down_write(&bus->lock);
> > }
> > +EXPORT_SYMBOL_GPL(i3c_bus_maintenance_lock);
> >
> > /**
> > * i3c_bus_maintenance_unlock - Release the bus lock after a maintenance
> > @@ -52,10 +53,11 @@ static void i3c_bus_maintenance_lock(struct i3c_bus *bus)
> > * i3c_bus_maintenance_lock() for more details on what these maintenance
> > * operations are.
> > */
> > -static void i3c_bus_maintenance_unlock(struct i3c_bus *bus)
> > +void i3c_bus_maintenance_unlock(struct i3c_bus *bus)
> > {
> > up_write(&bus->lock);
> > }
> > +EXPORT_SYMBOL_GPL(i3c_bus_maintenance_unlock);
>
> This should be done in a separate patch explaining why you need to
> export those funcs. Also, functions defined in drivers/i3c/internals.h
> should never be exported. If the function is about to be used by master
> controller drivers, it should be defined in include/linux/i3c/master.h.
>
Ok.
> >
> > /**
> > * i3c_bus_normaluse_lock - Lock the bus for a normal operation
> > @@ -91,6 +93,20 @@ void i3c_bus_normaluse_unlock(struct i3c_bus *bus)
> > up_read(&bus->lock);
> > }
> >
> > +/**
> > + * i3c_bus_downgrade_maintenance_lock - Downgrade the bus lock to normal
> > + * operation
> > + * @bus: I3C bus to downgrade the lock on
> > + *
> > + * Should be called when a maintenance operation is done and normal
> > + * operation is planned. See i3c_bus_maintenance_lock() and
> > + * i3c_bus_normaluse_lock() for more details.
> > + */
> > +void i3c_bus_downgrade_maintenance_lock(struct i3c_bus *bus)
> > +{
> > + downgrade_write(&bus->lock);
> > +}
> > +
> > static struct i3c_master_controller *dev_to_i3cmaster(struct device *dev)
> > {
> > return container_of(dev, struct i3c_master_controller, dev);
> > @@ -339,6 +355,22 @@ static int i3c_device_probe(struct device *dev)
> > return driver->probe(i3cdev);
> > }
> >
> > +static void i3c_master_enable_mr_events(struct i3c_master_controller *master)
> > +{
> > + if (!master->ops->enable_mr_events)
> > + return;
> > +
> > + master->ops->enable_mr_events(master);
> > +}
> > +
> > +static void i3c_master_disable_mr_events(struct i3c_master_controller *master)
> > +{
> > + if (!master->ops->disable_mr_events)
> > + return;
> > +
> > + master->ops->disable_mr_events(master);
> > +}
> > +
> > static int i3c_device_remove(struct device *dev)
> > {
> > struct i3c_device *i3cdev = dev_to_i3cdev(dev);
> > @@ -460,6 +492,23 @@ static int i3c_bus_init(struct i3c_bus *i3cbus)
> > return 0;
> > }
> >
> > +int i3c_master_request_mastership_locked(struct i3c_master_controller *master)
> > +{
> > + if (WARN_ON(master->init_done &&
> > + !rwsem_is_locked(&master->bus.lock)))
> > + return -EINVAL;
> > +
> > + if (!master->ops->request_mastership)
> > + return -ENOTSUPP;
> > +
> > + return master->ops->request_mastership(master);
> > +}
> > +
> > +bool i3c_master_owns_bus_locked(struct i3c_master_controller *master)
> > +{
> > + return master->bus.cur_master == master->this;
> > +}
>
> If you create the i3c_master_acquire_bus_ownership() helper, you
> should need i3c_master_owns_bus_locked().
Should or shouldn't? :-)
>
> > +
> > static const char * const i3c_bus_mode_strings[] = {
> > [I3C_BUS_MODE_PURE] = "pure",
> > [I3C_BUS_MODE_MIXED_FAST] = "mixed-fast",
> > @@ -618,6 +667,25 @@ i3c_master_alloc_i2c_dev(struct i3c_master_controller *master,
> >
> > dev->common.master = master;
> > dev->boardinfo = boardinfo;
> > + dev->addr = boardinfo->base.addr;
> > + dev->lvr = boardinfo->lvr;
>
> Please do that in a separate patch (I mean, the part that adds ->lvr
> and ->addr fields to the i3c_dev_desc object).
>
Ok.
> > +
> > + return dev;
> > +}
> > +
> > +static struct i2c_dev_desc *
> > +i3c_master_alloc_i2c_dev_no_boardinfo(struct i3c_master_controller *master,
> > + u16 addr, u8 lvr)
> > +{
> > + struct i2c_dev_desc *dev;
> > +
> > + dev = kzalloc(sizeof(*dev), GFP_KERNEL);
> > + if (!dev)
> > + return ERR_PTR(-ENOMEM);
> > +
> > + dev->common.master = master;
> > + dev->addr = addr;
> > + dev->lvr = lvr;
> >
> > return dev;
> > }
> > @@ -691,6 +759,9 @@ i3c_master_find_i2c_dev_by_addr(const struct i3c_master_controller *master,
> > struct i2c_dev_desc *dev;
> >
> > i3c_bus_for_each_i2cdev(&master->bus, dev) {
> > + if (!dev->boardinfo)
> > + continue;
> > +
> > if (dev->boardinfo->base.addr == addr)
> > return dev;
> > }
> > @@ -937,8 +1008,8 @@ int i3c_master_defslvs_locked(struct i3c_master_controller *master)
> >
> > desc = defslvs->slaves;
> > i3c_bus_for_each_i2cdev(bus, i2cdev) {
> > - desc->lvr = i2cdev->boardinfo->lvr;
> > - desc->static_addr = i2cdev->boardinfo->base.addr << 1;
> > + desc->lvr = i2cdev->lvr;
> > + desc->static_addr = i2cdev->addr << 1;
> > desc++;
> > }
> >
> > @@ -1490,6 +1561,83 @@ i3c_master_register_new_i3c_devs(struct i3c_master_controller *master)
> > }
> > }
> >
> > +static struct i2c_dev_boardinfo *
> > +i3c_master_find_i2c_boardinfo(const struct i3c_master_controller *master,
> > + u16 addr, u8 lvr)
> > +{
> > + struct i2c_dev_boardinfo *i2cboardinfo;
> > +
> > + list_for_each_entry(i2cboardinfo, &master->boardinfo.i2c, node) {
> > + if (i2cboardinfo->base.addr == addr &&
> > + i2cboardinfo->lvr == lvr)
> > + return i2cboardinfo;
> > + }
> > +
> > + return NULL;
> > +}
> > +
> > +static void
> > +i3c_master_register_new_i2c_devs(struct i3c_master_controller *master)
> > +{
> > + struct i2c_adapter *adap = i3c_master_to_i2c_adapter(master);
> > + struct i2c_dev_desc *i2cdev;
> > +
> > + if (!master->init_done)
> > + return;
> > +
> > + i3c_bus_for_each_i2cdev(&master->bus, i2cdev) {
> > + if (i2cdev->dev)
> > + continue;
> > +
> > + if (!i2cdev->boardinfo)
> > + continue;
> > +
> > + i2cdev->dev = i2c_new_device(adap, &i2cdev->boardinfo->base);
> > + }
> > +}
> > +
> > +/**
> > + * i3c_master_get_accmst_locked() - send a GETACCMST CCC command
> > + * @master: master used to send frames on the bus
> > + * @info: I3C device information
> > + *
> > + * Send a GETACCMST CCC command.
> > + *
> > + * This should be called if the curent master acknowledges bus takeover.
> > + *
> > + * This function must be called with the bus lock held in write mode.
> > + *
> > + * Return: 0 in case of success, a positive I3C error code if the error is
> > + * one of the official Mx error codes, and a negative error code otherwise.
> > + */
> > +int i3c_master_get_accmst_locked(struct i3c_master_controller *master,
> > + u8 addr)
> > +{
> > + struct i3c_ccc_getaccmst *accmst;
> > + struct i3c_ccc_cmd_dest dest;
> > + struct i3c_ccc_cmd cmd;
> > + int ret;
> > +
> > + accmst = i3c_ccc_cmd_dest_init(&dest, addr, sizeof(*accmst));
> > + if (!accmst)
> > + return -ENOMEM;
> > +
> > + i3c_ccc_cmd_init(&cmd, true, I3C_CCC_GETACCMST, &dest, 1);
> > +
> > + ret = i3c_master_send_ccc_cmd_locked(master, &cmd);
> > + if (ret)
> > + goto out;
> > +
> > + if (dest.payload.len != sizeof(*accmst))
> > + ret = -EIO;
> > +
> > +out:
> > + i3c_ccc_cmd_dest_cleanup(&dest);
> > +
> > + return ret;
> > +}
> > +EXPORT_SYMBOL_GPL(i3c_master_get_accmst_locked);
> > +
> > /**
> > * i3c_master_do_daa() - do a DAA (Dynamic Address Assignment)
> > * @master: master doing the DAA
> > @@ -1554,12 +1702,9 @@ int i3c_master_set_info(struct i3c_master_controller *master,
> > struct i3c_dev_desc *i3cdev;
> > int ret;
> >
> > - if (!i3c_bus_dev_addr_is_avail(&master->bus, info->dyn_addr))
> > - return -EINVAL;
> > -
> > - if (I3C_BCR_DEVICE_ROLE(info->bcr) == I3C_BCR_I3C_MASTER &&
> > - master->secondary)
> > - return -EINVAL;
> > + if (!master->secondary)
> > + if (!i3c_bus_dev_addr_is_avail(&master->bus, info->dyn_addr))
> > + return -EINVAL;
>
> Please, add curly braces around the first if block. Also, why don't you
> check if the address is valid in that case?
I should. Again, I left this because it was valid for the early version.
I'll check if address is valid in case of both main and secondary ceontrollers.
>
> >
> > if (master->this)
> > return -EINVAL;
> > @@ -1605,43 +1750,13 @@ static void i3c_master_detach_free_devs(struct i3c_master_controller *master)
> > common.node) {
> > i3c_master_detach_i2c_dev(i2cdev);
> > i3c_bus_set_addr_slot_status(&master->bus,
> > - i2cdev->boardinfo->base.addr,
> > - I3C_ADDR_SLOT_FREE);
> > + i2cdev->addr,
> > + I3C_ADDR_SLOT_FREE);
> > i3c_master_free_i2c_dev(i2cdev);
> > }
> > }
> >
> > -/**
> > - * i3c_master_bus_init() - initialize an I3C bus
> > - * @master: main master initializing the bus
> > - *
> > - * This function is following all initialisation steps described in the I3C
> > - * specification:
> > - *
> > - * 1. Attach I2C and statically defined I3C devs to the master so that the
> > - * master can fill its internal device table appropriately
> > - *
> > - * 2. Call &i3c_master_controller_ops->bus_init() method to initialize
> > - * the master controller. That's usually where the bus mode is selected
> > - * (pure bus or mixed fast/slow bus)
> > - *
> > - * 3. Instruct all devices on the bus to drop their dynamic address. This is
> > - * particularly important when the bus was previously configured by someone
> > - * else (for example the bootloader)
> > - *
> > - * 4. Disable all slave events.
> > - *
> > - * 5. Pre-assign dynamic addresses requested by the FW with SETDASA for I3C
> > - * devices that have a static address
> > - *
> > - * 6. Do a DAA (Dynamic Address Assignment) to assign dynamic addresses to all
> > - * remaining I3C devices
> > - *
> > - * Once this is done, all I3C and I2C devices should be usable.
> > - *
> > - * Return: a 0 in case of success, an negative error code otherwise.
> > - */
> > -static int i3c_master_bus_init(struct i3c_master_controller *master)
> > +static int i3c_master_attach_static_devs(struct i3c_master_controller *master)
> > {
> > enum i3c_addr_slot_status status;
> > struct i2c_dev_boardinfo *i2cboardinfo;
> > @@ -1650,32 +1765,24 @@ static int i3c_master_bus_init(struct i3c_master_controller *master)
> > struct i2c_dev_desc *i2cdev;
> > int ret;
> >
> > - /*
> > - * First attach all devices with static definitions provided by the
> > - * FW.
> > - */
> > list_for_each_entry(i2cboardinfo, &master->boardinfo.i2c, node) {
> > status = i3c_bus_get_addr_slot_status(&master->bus,
> > i2cboardinfo->base.addr);
> > - if (status != I3C_ADDR_SLOT_FREE) {
> > - ret = -EBUSY;
> > - goto err_detach_devs;
> > - }
> > + if (status != I3C_ADDR_SLOT_FREE)
> > + return -EBUSY;
> >
> > i3c_bus_set_addr_slot_status(&master->bus,
> > i2cboardinfo->base.addr,
> > I3C_ADDR_SLOT_I2C_DEV);
> >
> > i2cdev = i3c_master_alloc_i2c_dev(master, i2cboardinfo);
> > - if (IS_ERR(i2cdev)) {
> > - ret = PTR_ERR(i2cdev);
> > - goto err_detach_devs;
> > - }
> > + if (IS_ERR(i2cdev))
> > + return PTR_ERR(i2cdev);
> >
> > ret = i3c_master_attach_i2c_dev(master, i2cdev);
> > if (ret) {
> > i3c_master_free_i2c_dev(i2cdev);
> > - goto err_detach_devs;
> > + return ret;
> > }
> > }
> > list_for_each_entry(i3cboardinfo, &master->boardinfo.i3c, node) {
> > @@ -1685,28 +1792,71 @@ static int i3c_master_bus_init(struct i3c_master_controller *master)
> >
> > if (i3cboardinfo->init_dyn_addr) {
> > status = i3c_bus_get_addr_slot_status(&master->bus,
> > - i3cboardinfo->init_dyn_addr);
> > - if (status != I3C_ADDR_SLOT_FREE) {
> > - ret = -EBUSY;
> > - goto err_detach_devs;
> > - }
> > + i3cboardinfo->init_dyn_addr);
> > + if (status != I3C_ADDR_SLOT_FREE)
> > + return -EBUSY;
> > }
> >
> > i3cdev = i3c_master_alloc_i3c_dev(master, &info);
> > - if (IS_ERR(i3cdev)) {
> > - ret = PTR_ERR(i3cdev);
> > - goto err_detach_devs;
> > - }
> > + if (IS_ERR(i3cdev))
> > + return PTR_ERR(i3cdev);
> >
> > i3cdev->boardinfo = i3cboardinfo;
> >
> > ret = i3c_master_attach_i3c_dev(master, i3cdev);
> > if (ret) {
> > i3c_master_free_i3c_dev(i3cdev);
> > - goto err_detach_devs;
> > + return ret;
> > }
> > }
> >
> > + return 0;
> > +}
> > +
> > +/**
> > + * i3c_master_bus_init() - initialize an I3C bus
> > + * @master: main master initializing the bus
> > + *
> > + * This function is following all initialisation steps described in the I3C
> > + * specification:
> > + *
> > + * 1. Attach I2C and statically defined I3C devs to the master so that the
> > + * master can fill its internal device table appropriately
> > + *
> > + * 2. Call &i3c_master_controller_ops->bus_init() method to initialize
> > + * the master controller. That's usually where the bus mode is selected
> > + * (pure bus or mixed fast/slow bus)
> > + *
> > + * 3. Instruct all devices on the bus to drop their dynamic address. This is
> > + * particularly important when the bus was previously configured by someone
> > + * else (for example the bootloader)
> > + *
> > + * 4. Disable all slave events.
> > + *
> > + * 5. Pre-assign dynamic addresses requested by the FW with SETDASA for I3C
> > + * devices that have a static address
> > + *
> > + * 6. Do a DAA (Dynamic Address Assignment) to assign dynamic addresses to all
> > + * remaining I3C devices
> > + *
> > + * Once this is done, all I3C and I2C devices should be usable.
> > + *
> > + * Return: a 0 in case of success, an negative error code otherwise.
> > + */
> > +static int i3c_master_bus_init(struct i3c_master_controller *master)
> > +{
> > + struct i3c_dev_desc *i3cdev;
> > + int ret;
> > +
> > + /*
> > + * First attach all devices with static definitions provided by the
> > + * FW.
> > + */
> > + if (!master->secondary) {
> > + ret = i3c_master_attach_static_devs(master);
> > + if (ret)
> > + goto err_detach_devs;
> > + }
> > /*
> > * Now execute the controller specific ->bus_init() routine, which
> > * might configure its internal logic to match the bus limitations.
> > @@ -1727,6 +1877,13 @@ static int i3c_master_bus_init(struct i3c_master_controller *master)
> > }
> >
> > /*
> > + * Don't reset addresses if this is secondary master.
> > + * Secondary masters can't do DAA.
> > + */
> > + if (master->secondary)
> > + return 0;
> > +
> > + /*
> > * Reset all dynamic address that may have been assigned before
> > * (assigned by the bootloader for example).
> > */
> > @@ -1789,6 +1946,56 @@ i3c_master_search_i3c_dev_duplicate(struct i3c_dev_desc *refdev)
> > return NULL;
> > }
> >
> > +int i3c_master_add_i2c_dev(struct i3c_master_controller *master,
> > + u16 addr, u8 lvr)
> > +{
> > + enum i3c_addr_slot_status status;
> > + struct i2c_dev_boardinfo *boardinfo;
> > + struct i2c_dev_desc *i2cdev;
> > + int ret;
> > +
> > + if (!master)
> > + return -EINVAL;
> > +
> > + status = i3c_bus_get_addr_slot_status(&master->bus,
> > + addr);
> > + if (status != I3C_ADDR_SLOT_FREE)
> > + return -EBUSY;
> > +
> > + i3c_bus_set_addr_slot_status(&master->bus, addr,
> > + I3C_ADDR_SLOT_I2C_DEV);
> > +
> > + i2cdev = i3c_master_alloc_i2c_dev_no_boardinfo(master, addr, lvr);
> > + if (IS_ERR(i2cdev)) {
> > + ret = PTR_ERR(i2cdev);
> > + goto err_free_dev;
> > + }
> > +
> > + boardinfo = i3c_master_find_i2c_boardinfo(master, addr, lvr);
> > + if (!boardinfo) {
> > + ret = -ENODEV;
> > + goto err_free_dev;
> > + }
>
> Don't you need to keep the i2c_dev_desc around even if there's no
> boardinfo associated to it? I thought you had to do that to be able to
> send DEFSLVS when the secondary master receives a H-J request and needs
> to send a new DEFSLVS frame.
Yes, my mistake. I shouldn't free up i2c_dev_desc. I added it during refactoring.
DEFSLVS frame doesn't look good now.
>
> > +
> > + i2cdev->boardinfo = boardinfo;
> > +
> > + ret = i3c_master_attach_i2c_dev(master, i2cdev);
> > + if (ret) {
> > + ret = PTR_ERR(i2cdev);
> > + goto err_free_dev;
> > + }
> > +
> > + return 0;
> > +
> > +err_free_dev:
> > + i3c_bus_set_addr_slot_status(&master->bus, addr,
> > + I3C_ADDR_SLOT_FREE);
> > + i3c_master_free_i2c_dev(i2cdev);
> > +
> > + return ret;
> > +}
> > +EXPORT_SYMBOL_GPL(i3c_master_add_i2c_dev);
> > +
> > /**
> > * i3c_master_add_i3c_dev_locked() - add an I3C slave to the bus
> > * @master: master used to send frames on the bus
> > @@ -2101,6 +2308,18 @@ static int i3c_master_i2c_adapter_xfer(struct i2c_adapter *adap,
> > }
> >
> > i3c_bus_normaluse_lock(&master->bus);
> > + if (!i3c_master_owns_bus_locked(master)) {
> > + i3c_bus_normaluse_unlock(&master->bus);
> > + i3c_bus_maintenance_lock(&master->bus);
> > +
> > + ret = i3c_master_request_mastership_locked(master);
> > + if (ret) {
> > + i3c_bus_maintenance_unlock(&master->bus);
> > + return ret;
> > + }
> > + i3c_bus_downgrade_maintenance_lock(&master->bus);
> > + }
> > +
> > dev = i3c_master_find_i2c_dev_by_addr(master, addr);
> > if (!dev)
> > ret = -ENOENT;
> > @@ -2146,8 +2365,12 @@ static int i3c_master_i2c_adapter_init(struct i3c_master_controller *master)
> > * We silently ignore failures here. The bus should keep working
> > * correctly even if one or more i2c devices are not registered.
> > */
> > - i3c_bus_for_each_i2cdev(&master->bus, i2cdev)
> > + i3c_bus_for_each_i2cdev(&master->bus, i2cdev) {
> > + if (!i2cdev->boardinfo)
> > + continue;
> > +
> > i2cdev->dev = i2c_new_device(adap, &i2cdev->boardinfo->base);
> > + }
> >
> > return 0;
> > }
> > @@ -2388,18 +2611,43 @@ static int i3c_master_check_ops(const struct i3c_master_controller_ops *ops)
> > !ops->recycle_ibi_slot))
> > return -EINVAL;
> >
> > + if (ops->request_mastership &&
> > + (!ops->enable_mr_events || !ops->disable_mr_events))
> > + return -EINVAL;
> > +
> > return 0;
> > }
> >
> > /**
> > + * i3c_master_bus_takeover() - register new I3C devices on bus takeover
> > + * @master: master used to send frames on the bus
> > + *
> > + * This function is useful when devices were not added
> > + * during initialization or when new device joined the bus
> > + * which was under control of different master.
> > + */
> > +void i3c_master_bus_takeover(struct i3c_master_controller *master)
>
> Seems like the name does not match what the function does. How about
> i3c_master_register_new_devs(). The fact that it's call when a
> secondary master acquires the bus is just a detail.
Yes, it made sense before, but not now. I'll change the name.
>
> > +{
> > + /*
> > + * We can register I3C devices received from master by DEFSLVS.
> > + */
> > + i3c_bus_normaluse_lock(&master->bus);
> > + i3c_master_register_new_i3c_devs(master);
> > + i3c_bus_normaluse_unlock(&master->bus);
> > +
> > + i3c_bus_normaluse_lock(&master->bus);
> > + i3c_master_register_new_i2c_devs(master);
> > + i3c_bus_normaluse_unlock(&master->bus);
> > +}
> > +EXPORT_SYMBOL_GPL(i3c_master_bus_takeover);
> > +
> > +/**
> > * i3c_master_register() - register an I3C master
> > * @master: master used to send frames on the bus
> > * @parent: the parent device (the one that provides this I3C master
> > * controller)
> > * @ops: the master controller operations
> > - * @secondary: true if you are registering a secondary master. Will return
> > - * -ENOTSUPP if set to true since secondary masters are not yet
> > - * supported
> > + * @secondary: true if you are registering a secondary master.
> > *
> > * This function takes care of everything for you:
> > *
> > @@ -2422,10 +2670,6 @@ int i3c_master_register(struct i3c_master_controller *master,
> > struct i2c_dev_boardinfo *i2cbi;
> > int ret;
> >
> > - /* We do not support secondary masters yet. */
> > - if (secondary)
> > - return -ENOTSUPP;
> > -
> > ret = i3c_master_check_ops(ops);
> > if (ret)
> > return ret;
> > @@ -2503,6 +2747,14 @@ int i3c_master_register(struct i3c_master_controller *master,
> > i3c_master_register_new_i3c_devs(master);
> > i3c_bus_normaluse_unlock(&master->bus);
> >
> > + i3c_bus_normaluse_lock(&master->bus);
> > + i3c_master_register_new_i2c_devs(master);
> > + i3c_bus_normaluse_unlock(&master->bus);
>
> You could call i3c_master_register_new_devs() here instead of
> duplicating the code.
>
> > +
> > + i3c_bus_maintenance_lock(&master->bus);
> > + i3c_master_enable_mr_events(master);
> > + i3c_bus_maintenance_unlock(&master->bus);
>
> We might want to make that configurable per-bus using a DT prop (or a
> module param), but let's say we accept MR requests by default.
>
Yes, we can add support for this. Adding it to my todo list :)
> > +
> > return 0;
> >
> > err_del_dev:
> > @@ -2519,6 +2771,30 @@ int i3c_master_register(struct i3c_master_controller *master,
> > EXPORT_SYMBOL_GPL(i3c_master_register);
> >
> > /**
> > + * i3c_master_mastership_ack() - performs operations before bus handover.
> > + * @master: master used to send frames on the bus
> > + * @addr: I3C device address
> > + *
> > + * Basically, it sends DEFSLVS command to ensure new master is taking
> > + * the bus with complete list of devices and then acknowledges bus takeover.
> > + *
> > + * Return: 0 in case of success, a negative error code otherwise.
> > + */
> > +int i3c_master_mastership_ack(struct i3c_master_controller *master,
> > + u8 addr)
> > +{
> > + int ret;
> > +
> > + i3c_bus_maintenance_lock(&master->bus);
> > + ret = i3c_master_get_accmst_locked(master, addr);
> > + i3c_bus_maintenance_unlock(&master->bus);
> > +
> > + return ret;
> > +}
> > +EXPORT_SYMBOL_GPL(i3c_master_mastership_ack);
> > +
> > +
> > +/**
> > * i3c_master_unregister() - unregister an I3C master
> > * @master: master used to send frames on the bus
> > *
> > @@ -2528,6 +2804,10 @@ EXPORT_SYMBOL_GPL(i3c_master_register);
> > */
> > int i3c_master_unregister(struct i3c_master_controller *master)
> > {
> > + i3c_bus_maintenance_lock(&master->bus);
> > + i3c_master_disable_mr_events(master);
> > + i3c_bus_maintenance_unlock(&master->bus);
> > +
> > i3c_master_i2c_adapter_cleanup(master);
> > i3c_master_unregister_i3c_devs(master);
> > i3c_master_bus_cleanup(master);
> > diff --git a/include/linux/i3c/master.h b/include/linux/i3c/master.h
> > index f13fd8b..8514680 100644
> > --- a/include/linux/i3c/master.h
> > +++ b/include/linux/i3c/master.h
> > @@ -84,6 +84,8 @@ struct i2c_dev_desc {
> > struct i3c_i2c_dev_desc common;
> > const struct i2c_dev_boardinfo *boardinfo;
> > struct i2c_client *dev;
> > + u16 addr;
> > + u8 lvr;
> > };
> >
> > /**
> > @@ -418,6 +420,12 @@ struct i3c_bus {
> > * for a future IBI
> > * This method is mandatory only if ->request_ibi is not
> > * NULL.
> > + * @request_mastership: requests bus mastership. Mastership is requested
> > + * automatically when device driver wants to transfer
> > + * data using master in slave mode.
> > + * @enable_mr_events: enable the Mastership event.
> > + * Mastership does not require handler.
> > + * @disable_mr_events: disable the Mastership event.
> > */
> > struct i3c_master_controller_ops {
> > int (*bus_init)(struct i3c_master_controller *master);
> > @@ -445,6 +453,10 @@ struct i3c_master_controller_ops {
> > int (*disable_ibi)(struct i3c_dev_desc *dev);
> > void (*recycle_ibi_slot)(struct i3c_dev_desc *dev,
> > struct i3c_ibi_slot *slot);
> > + void (*update_devs)(struct i3c_master_controller *master);
>
> Undocumented hook (I'm not even sure you're using it in the code).
>
I'm not. Impact of the previous version.
> > + int (*request_mastership)(struct i3c_master_controller *master);
> > + int (*enable_mr_events)(struct i3c_master_controller *master);
> > + int (*disable_mr_events)(struct i3c_master_controller *master);
> > };
> >
> > /**
> > @@ -524,6 +536,8 @@ int i3c_master_defslvs_locked(struct i3c_master_controller *master);
> > int i3c_master_get_free_addr(struct i3c_master_controller *master,
> > u8 start_addr);
> >
> > +int i3c_master_add_i2c_dev(struct i3c_master_controller *master,
> > + u16 addr, u8 lvr);
> > int i3c_master_add_i3c_dev_locked(struct i3c_master_controller *master,
> > u8 addr);
> > int i3c_master_do_daa(struct i3c_master_controller *master);
> > @@ -536,6 +550,11 @@ int i3c_master_register(struct i3c_master_controller *master,
> > const struct i3c_master_controller_ops *ops,
> > bool secondary);
> > int i3c_master_unregister(struct i3c_master_controller *master);
> > +int i3c_master_get_accmst_locked(struct i3c_master_controller *master,
> > + u8 addr);
> > +int i3c_master_mastership_ack(struct i3c_master_controller *master,
> > + u8 addr);
> > +void i3c_master_bus_takeover(struct i3c_master_controller *master);
> >
> > /**
> > * i3c_dev_get_master_data() - get master private data attached to an I3C
> > @@ -645,4 +664,7 @@ void i3c_master_queue_ibi(struct i3c_dev_desc *dev, struct i3c_ibi_slot *slot);
> >
> > struct i3c_ibi_slot *i3c_master_get_free_ibi_slot(struct i3c_dev_desc *dev);
> >
> > +void i3c_bus_maintenance_lock(struct i3c_bus *bus);
> > +void i3c_bus_maintenance_unlock(struct i3c_bus *bus);
> > +
> > #endif /* I3C_MASTER_H */
>
--
--
Przemyslaw Gaj
_______________________________________________
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/4] i3c: Add support for mastership request to I3C subsystem
2019-02-22 21:09 ` Przemyslaw Gaj
@ 2019-02-25 8:51 ` Boris Brezillon
0 siblings, 0 replies; 8+ messages in thread
From: Boris Brezillon @ 2019-02-25 8:51 UTC (permalink / raw)
To: Przemyslaw Gaj; +Cc: linux-i3c, rafalc, vitor.soares
On Fri, 22 Feb 2019 21:09:03 +0000
Przemyslaw Gaj <pgaj@cadence.com> wrote:
> > > +
> > > +bool i3c_master_owns_bus_locked(struct i3c_master_controller *master)
> > > +{
> > > + return master->bus.cur_master == master->this;
> > > +}
> >
> > If you create the i3c_master_acquire_bus_ownership() helper, you
> > should need i3c_master_owns_bus_locked().
>
> Should or shouldn't? :-)
Shouldn't :P.
_______________________________________________
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2019-02-25 8:51 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-18 13:08 [PATCH 0/4] Add the I3C mastership request Przemyslaw Gaj
2019-02-18 13:08 ` [PATCH 1/4] i3c: Add support for mastership request to I3C subsystem Przemyslaw Gaj
2019-02-22 15:30 ` Boris Brezillon
2019-02-22 21:09 ` Przemyslaw Gaj
2019-02-25 8:51 ` Boris Brezillon
2019-02-18 13:08 ` [PATCH 2/4] i3c: master: cdns: add support for mastership request to Cadence I3C master driver Przemyslaw Gaj
2019-02-18 13:08 ` [PATCH 3/4] i3c: master: Add module author Przemyslaw Gaj
2019-02-18 13:08 ` [PATCH 4/4] MAINTAINERS: add myself as co-maintainer of I3C subsystem Przemyslaw Gaj
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).