drivers: i3c: cdns: fixup attachment and addr assignment for daa

The ENTDAA does not have a way to assign DA that are with a PID. It will
assign DAs that were in it's RRs in the order that they win arbitration.
Assign only available addresses in to it's RRs before ENTDAA.

Cleanup the attach api to no longer require a addr argument and remove
the helper function `i3c_determine_default_addr`. This now looks at if
it has a static address or if it already has a dynamic address (such as
from DEFTGTS) and will register the address if either exist with
precidence of dynamic addr over static addr.

This also fixes up the look up for if a device already has a dynamic
addr to find which pos of the RRs is it is in.

Signed-off-by: Ryan McClelland <[email protected]>

Author: XenuIsWatching

drivers/i3c/i3c_cdns.c

@@ -1499,6 +1499,7 @@ static int cdns_i3c_do_daa(const struct device *dev)
 	struct cdns_i3c_data *data = dev->data;
 	const struct cdns_i3c_config *config = dev->config;
 	struct i3c_config_controller *ctrl_config = &data->common.ctrl_config;
+	uint8_t last_addr = 0;
 
 	/* DAA should not be done by secondary controllers */
 	if (ctrl_config->is_secondary) {
@@ -1510,6 +1511,23 @@ static int cdns_i3c_do_daa(const struct device *dev)
 	/* ignore the controller register */
 	olddevs |= BIT(0);
 
+	/* Assign dynamic addressses to available RRs */
+	/* Loop through each clear bit */
+	for (uint8_t i = find_lsb_set(~olddevs); i <= data->max_devs; i++) {
+		uint8_t rr_idx = i - 1;
+
+		if (~olddevs & BIT(rr_idx)) {
+			/* Read RRx registers */
+			last_addr = i3c_addr_slots_next_free_find(
+				&data->common.attached_dev.addr_slots, last_addr + 1);
+			/* Write RRx registers */
+			sys_write32(prepare_rr0_dev_address(last_addr) | DEV_ID_RR0_IS_I3C,
+				    config->base + DEV_ID_RR0(rr_idx));
+			sys_write32(0, config->base + DEV_ID_RR1(rr_idx));
+			sys_write32(0, config->base + DEV_ID_RR2(rr_idx));
+		}
+	}
+
 	/* the Cadence I3C IP will assign an address for it from the RR */
 	struct i3c_ccc_payload entdaa_ccc;
 
@@ -1915,7 +1933,10 @@ static int cdns_i3c_master_get_rr_slot(const struct device *dev, uint8_t dyn_add
 {
 	struct cdns_i3c_data *data = dev->data;
 	const struct cdns_i3c_config *config = dev->config;
+	uint8_t rr_idx, i;
+	uint32_t rr, activedevs;
 
+	/* If it does not have a dynamic address, then assign it a free one */
 	if (dyn_addr == 0) {
 		if (!data->free_rr_slots) {
 			return -ENOSPC;
@@ -1924,62 +1945,70 @@ static int cdns_i3c_master_get_rr_slot(const struct device *dev, uint8_t dyn_add
 		return find_lsb_set(data->free_rr_slots) - 1;
 	}
 
-	uint32_t activedevs = sys_read32(config->base + DEVS_CTRL) & DEVS_CTRL_DEVS_ACTIVE_MASK;
-
+	/* Device already has a Dynamic Address, so assume it is already in the RRs */
+	activedevs = sys_read32(config->base + DEVS_CTRL) & DEVS_CTRL_DEVS_ACTIVE_MASK;
+	/* skip itself */
 	activedevs &= ~BIT(0);
 
 	/* loop through each set bit for new devices */
-	for (uint8_t i = find_lsb_set(activedevs); i <= find_msb_set(activedevs); i++) {
-		if (activedevs & BIT(i)) {
-			uint32_t rr = sys_read32(config->base + DEV_ID_RR0(i));
-
-			if (!(rr & DEV_ID_RR0_IS_I3C) || DEV_ID_RR0_GET_DEV_ADDR(rr) != dyn_addr) {
-				continue;
+	for (i = find_lsb_set(activedevs); i <= find_msb_set(activedevs); i++) {
+		rr_idx = i - 1;
+		if (activedevs & BIT(rr_idx)) {
+			rr = sys_read32(config->base + DEV_ID_RR0(rr_idx));
+			if ((rr & DEV_ID_RR0_IS_I3C) && DEV_ID_RR0_GET_DEV_ADDR(rr) == dyn_addr) {
+				return rr_idx;
 			}
-			return i;
 		}
 	}
 
 	return -EINVAL;
 }
 
-static int cdns_i3c_attach_device(const struct device *dev, struct i3c_device_desc *desc,
-				  uint8_t addr)
+static int cdns_i3c_attach_device(const struct device *dev, struct i3c_device_desc *desc)
 {
-	const struct cdns_i3c_config *config = dev->config;
-	struct cdns_i3c_data *data = dev->data;
-	int slot = cdns_i3c_master_get_rr_slot(dev, desc->dynamic_addr);
-
-	if (slot < 0) {
-		LOG_ERR("%s: no space for i3c device: %s", dev->name, desc->dev->name);
-		return slot;
-	}
+	/*
+	 * Mark Devices as active, devices that will be found and marked active during DAA,
+	 * it will be given the exact DA programmed in it's RR, otherwise they get set as active
+	 * here. If dynamic address is set, then it assumed that it was already initialized by the
+	 * primary controller. When assigned through ENTDAA, the dynamic address, bcr, dcr, and pid
+	 * are all set in the RR along with setting the device as active. If it has a static addr,
+	 * then it is assumed that it will be programmed with SETDASA and will need to be marked
+	 * as active before sending out SETDASA.
+	 */
+	if ((desc->static_addr != 0) || (desc->dynamic_addr != 0)) {
+		const struct cdns_i3c_config *config = dev->config;
+		struct cdns_i3c_data *data = dev->data;
 
-	k_mutex_lock(&data->bus_lock, K_FOREVER);
+		int slot = cdns_i3c_master_get_rr_slot(dev, desc->dynamic_addr ? desc->dynamic_addr
+									       : desc->static_addr);
 
-	data->cdns_i3c_i2c_priv_data[slot].id = slot;
-	desc->controller_priv = &(data->cdns_i3c_i2c_priv_data[slot]);
-	data->free_rr_slots &= ~BIT(slot);
-
-	uint32_t dev_id_rr0 = DEV_ID_RR0_IS_I3C | prepare_rr0_dev_address(addr);
-	uint32_t dev_id_rr1 = DEV_ID_RR1_PID_MSB((desc->pid & 0xFFFFFFFF0000) >> 16);
-	uint32_t dev_id_rr2 = DEV_ID_RR2_PID_LSB(desc->pid & 0xFFFF);
+		if (slot < 0) {
+			LOG_ERR("%s: no space for i3c device: %s", dev->name, desc->dev->name);
+			return slot;
+		}
 
-	sys_write32(dev_id_rr0, config->base + DEV_ID_RR0(slot));
-	sys_write32(dev_id_rr1, config->base + DEV_ID_RR1(slot));
-	sys_write32(dev_id_rr2, config->base + DEV_ID_RR2(slot));
+		k_mutex_lock(&data->bus_lock, K_FOREVER);
 
-	/** Mark Devices as active, devices that will be found and marked active during DAA,
-	 * it will be given the exact DA programmed in it's RR if the PID matches and marked
-	 * as active duing ENTDAA, otherwise they get set as active here. If dynamic address
-	 * is set, then it assumed that it was already initialized by the primary controller.
-	 */
-	if ((desc->static_addr != 0) || (desc->dynamic_addr != 0)) {
 		sys_write32(sys_read32(config->base + DEVS_CTRL) | DEVS_CTRL_DEV_ACTIVE(slot),
 			    config->base + DEVS_CTRL);
-	}
 
-	k_mutex_unlock(&data->bus_lock);
+		data->cdns_i3c_i2c_priv_data[slot].id = slot;
+		desc->controller_priv = &(data->cdns_i3c_i2c_priv_data[slot]);
+		data->free_rr_slots &= ~BIT(slot);
+
+		uint32_t dev_id_rr0 =
+			DEV_ID_RR0_IS_I3C |
+			prepare_rr0_dev_address(desc->dynamic_addr ? desc->dynamic_addr
+								   : desc->static_addr);
+		uint32_t dev_id_rr1 = DEV_ID_RR1_PID_MSB((desc->pid & 0xFFFFFFFF0000) >> 16);
+		uint32_t dev_id_rr2 = DEV_ID_RR2_PID_LSB(desc->pid & 0xFFFF);
+
+		sys_write32(dev_id_rr0, config->base + DEV_ID_RR0(slot));
+		sys_write32(dev_id_rr1, config->base + DEV_ID_RR1(slot));
+		sys_write32(dev_id_rr2, config->base + DEV_ID_RR2(slot));
+
+		k_mutex_unlock(&data->bus_lock);
+	}
 
 	return 0;
 }

drivers/i3c/i3c_common.c

@@ -234,92 +234,41 @@ struct i3c_i2c_device_desc *i3c_dev_list_i2c_addr_find(const struct device *dev,
 	return ret;
 }
 
-int i3c_determine_default_addr(struct i3c_device_desc *target, uint8_t *addr)
-{
-	struct i3c_driver_data *data = (struct i3c_driver_data *)target->bus->data;
-
-	/* If dynamic addr is set, then it assumed that it was assigned by a primary controller */
-	if (target->dynamic_addr == 0) {
-		/* It is assumed that SETDASA or ENTDAA will be run after this */
-		if (target->init_dynamic_addr != 0U) {
-			/* initial dynamic address is requested */
-			if (target->static_addr == 0) {
-				/* SA is set to 0, so DA will be set with ENTDAA */
-				if (i3c_addr_slots_is_free(&data->attached_dev.addr_slots,
-							   target->init_dynamic_addr)) {
-					/* Set DA during ENTDAA */
-					*addr = target->init_dynamic_addr;
-				} else {
-					/* address is not free, get the next one */
-					*addr = i3c_addr_slots_next_free_find(
-						&data->attached_dev.addr_slots, 0);
-				}
-			} else {
-				/* Use the init dynamic address as it's DA, but the RR will need to
-				 * be first set with it's SA to run SETDASA, the RR address will
-				 * need be updated after SETDASA with the request dynamic address
-				 */
-				if (i3c_addr_slots_is_free(&data->attached_dev.addr_slots,
-							   target->static_addr)) {
-					*addr = target->static_addr;
-				} else {
-					/* static address has already been taken */
-					return -EINVAL;
-				}
-			}
-		} else {
-			/* no init dynamic address is requested */
-			if (target->static_addr != 0) {
-				if (i3c_addr_slots_is_free(&data->attached_dev.addr_slots,
-							   target->static_addr)) {
-					/* static exists, set DA with same SA during SETDASA*/
-					*addr = target->static_addr;
-				} else {
-					/* static address has already been taken */
-					return -EINVAL;
-				}
-			} else {
-				/* pick a DA to use */
-				*addr = i3c_addr_slots_next_free_find(
-					&data->attached_dev.addr_slots, 0);
-			}
-		}
-	} else {
-		*addr = target->dynamic_addr;
-	}
-
-	return 0;
-}
-
 int i3c_attach_i3c_device(struct i3c_device_desc *target)
 {
 	struct i3c_driver_data *data = (struct i3c_driver_data *)target->bus->data;
 	const struct i3c_driver_api *api = (const struct i3c_driver_api *)target->bus->api;
-	sys_snode_t *node;
 	uint8_t addr = 0;
 	int status = 0;
+	struct i3c_device_desc *i3c_desc;
 
 	/* check to see if the device has already been attached */
-	if (!sys_slist_is_empty(&data->attached_dev.devices.i3c)) {
-		SYS_SLIST_FOR_EACH_NODE(&data->attached_dev.devices.i3c, node) {
-			if (node == &target->node) {
-				return -EINVAL;
-			}
+	I3C_BUS_FOR_EACH_I3CDEV(target->bus, i3c_desc) {
+		if (i3c_desc == target) {
+			return -EINVAL;
 		}
 	}
 
-	status = i3c_determine_default_addr(target, &addr);
-	if (status != 0) {
-		return status;
+	addr = target->dynamic_addr ? target->dynamic_addr : target->static_addr;
+
+	/*
+	 * If it has a dynamic addr already assigned or a static address, check that it is free
+	 */
+	if (addr) {
+		if (!i3c_addr_slots_is_free(&data->attached_dev.addr_slots, addr)) {
+			return -EINVAL;
+		}
 	}
 
 	sys_slist_append(&data->attached_dev.devices.i3c, &target->node);
 
 	if (api->attach_i3c_device != NULL) {
-		status = api->attach_i3c_device(target->bus, target, addr);
+		status = api->attach_i3c_device(target->bus, target);
 	}
 
-	i3c_addr_slots_mark_i3c(&data->attached_dev.addr_slots, addr);
+	if (addr) {
+		i3c_addr_slots_mark_i3c(&data->attached_dev.addr_slots, addr);
+	}
 
 	return status;
 }
@@ -376,15 +325,13 @@ int i3c_attach_i2c_device(struct i3c_i2c_device_desc *target)
 {
 	struct i3c_driver_data *data = (struct i3c_driver_data *)target->bus->data;
 	const struct i3c_driver_api *api = (const struct i3c_driver_api *)target->bus->api;
-	sys_snode_t *node;
 	int status = 0;
+	struct i3c_i2c_device_desc *i3c_i2c_desc;
 
 	/* check to see if the device has already been attached */
-	if (!sys_slist_is_empty(&data->attached_dev.devices.i2c)) {
-		SYS_SLIST_FOR_EACH_NODE(&data->attached_dev.devices.i2c, node) {
-			if (node == &target->node) {
-				return -EINVAL;
-			}
+	I3C_BUS_FOR_EACH_I2CDEV(target->bus, i3c_i2c_desc) {
+		if (i3c_i2c_desc == target) {
+			return -EINVAL;
 		}
 	}
 

include/zephyr/drivers/i3c.h

@@ -639,13 +639,11 @@ __subsystem struct i3c_driver_api {
 	 *
 	 * @param dev Pointer to controller device driver instance.
 	 * @param target Pointer to target device descriptor.
-	 * @param addr Address to attach with
 	 *
 	 * @return See i3c_attach_i3c_device()
 	 */
 	int (*attach_i3c_device)(const struct device *dev,
-			struct i3c_device_desc *target,
-			uint8_t addr);
+			struct i3c_device_desc *target);
 
 	/**
 	 * I3C Address Update
@@ -1273,21 +1271,6 @@ struct i3c_device_desc *i3c_dev_list_i3c_addr_find(const struct device *dev,
 struct i3c_i2c_device_desc *i3c_dev_list_i2c_addr_find(const struct device *dev,
 							   uint16_t addr);
 
-/**
- * @brief Helper function to find the default address an i3c device is attached with
- *
- * This is a helper function to find the default address the
- * device will be loaded with. This could be either it's static
- * address, a requested dynamic address, or just a dynamic address
- * that is available
- * @param[in] target The pointer of the device descriptor
- * @param[out] addr Address to be assigned to target device.
- *
- * @retval 0 if successful.
- * @retval -EINVAL if the expected default address is already in use
- */
-int i3c_determine_default_addr(struct i3c_device_desc *target, uint8_t *addr);
-
 /**
  * @brief Helper function to find a usable address during ENTDAA.
  *