Re: [PATCH v4 01/10] i3c: Add core I3C infrastructure

From: Przemyslaw Gaj
Date: Mon Jun 04 2018 - 05:11:37 EST


Hi Boris

It looks great, just one comment to DEFSLVS command:

ïOn 6/4/18, 9:32 AM, "Boris Brezillon" <boris.brezillon@xxxxxxxxxxx> wrote:

+int i3c_master_defslvs_locked(struct i3c_master_controller *master)
+{
+ struct i3c_ccc_cmd_dest dest = {
+ .addr = I3C_BROADCAST_ADDR,
+ };
+ struct i3c_ccc_cmd cmd = {
+ .id = I3C_CCC_DEFSLVS,
+ .dests = &dest,
+ .ndests = 1,
+ };
+ struct i3c_ccc_defslvs *defslvs;
+ struct i3c_ccc_dev_desc *desc;
+ struct i3c_device *i3cdev;
+ struct i2c_device *i2cdev;
+ struct i3c_bus *bus;
+ bool send = false;
+ int ndevs = 0, ret;
+
+ if (!master)
+ return -EINVAL;
+
+ bus = i3c_master_get_bus(master);
+ i3c_bus_for_each_i3cdev(bus, i3cdev) {
+ ndevs++;
+ if (I3C_BCR_DEVICE_ROLE(i3cdev->info.bcr) == I3C_BCR_I3C_MASTER)
+ send = true;
+ }
+
+ /* No other master on the bus, skip DEFSLVS. */
+ if (!send)
+ return 0;
+
+ i3c_bus_for_each_i2cdev(bus, i2cdev)
+ ndevs++;
+
+ dest.payload.len = sizeof(*defslvs) +
+ ((ndevs - 1) * sizeof(struct i3c_ccc_dev_desc));
+ defslvs = kzalloc(dest.payload.len, GFP_KERNEL);
+ if (!defslvs)
+ return -ENOMEM;
+
+ dest.payload.data = defslvs;
+
+ defslvs->count = ndevs;
+ defslvs->master.bcr = master->this->info.bcr;
+ defslvs->master.dcr = master->this->info.dcr;
+ defslvs->master.dyn_addr = master->this->info.dyn_addr;
+ defslvs->master.static_addr = I3C_BROADCAST_ADDR;
+
+ desc = defslvs->slaves;
+ i3c_bus_for_each_i2cdev(bus, i2cdev) {
+ desc->lvr = i2cdev->lvr;
+ desc->static_addr = i2cdev->info.addr;
+ desc++;
+ }
+
+ i3c_bus_for_each_i3cdev(bus, i3cdev) {
+ /* Skip the I3C dev representing this master. */
+ if (i3cdev == master->this)
+ continue;
+
+ desc->bcr = i3cdev->info.bcr;
+ desc->dcr = i3cdev->info.dcr;
+ desc->dyn_addr = i3cdev->info.dyn_addr;
+ desc->static_addr = i3cdev->info.static_addr;
+ desc++;
+ }
+
+ ret = i3c_master_send_ccc_cmd_locked(master, &cmd);
+ kfree(defslvs);
+
+ return ret;
+}

You should shift all the addresses (dynamic and static) one bit left.
Addresses are stored on bits [7:1], this is described in MIPI spec,
section 5.1.9.3.7 Define List of Slaves (DEFSLVS)

Regards,
Przemyslaw Gaj