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