Hi Mike,

On Fri, 3 Aug 2018 15:38:43 -0600
[off-list ref] wrote:

quoted

+static void
+i3c_master_register_new_i3c_devs(struct i3c_master_controller *master)
+{
+	struct i3c_dev_desc *desc;
+	int ret;
+
+	if (!master->init_done)
+		return;
+
+	i3c_bus_for_each_i3cdev(master->bus, desc) {
+		if (desc->dev || !desc->info.dyn_addr)
+			continue;
+
+		desc->dev = kzalloc(sizeof(*desc->dev), GFP_KERNEL);
+		if (!desc->dev)
+			continue;
+
+		desc->dev->bus = master->bus;
+		desc->dev->desc = desc;
+		desc->dev->dev.parent = &master->bus->dev;
+		if (desc == master->this)
+			desc->dev->dev.type = &i3c_master_type;
+		else
+			desc->dev->dev.type = &i3c_device_type;
+		desc->dev->dev.bus = &i3c_bus_type;
+		desc->dev->dev.release = i3c_device_release;
+		dev_set_name(&desc->dev->dev, "%d-%llx", master->bus-  

quoted

id,  

+			     desc->info.pid);
+
+		if (desc->boardinfo)
+			desc->dev->dev.of_node = desc->boardinfo-  

quoted

of_node;  

+
+		pr_info("%s:%i\n", __func__, __LINE__);
+		if (ret)
+			dev_err(master->parent,
+				"Failed to add I3C device (err = %d)\n",  

ret);

quoted

+	}
+}  

The compiler gives a warning that "ret" may be used uninitialized in the
above function, which is true since "ret" is never assigned a value.  Also,
I wonder about the "pr_info" call here, should there be a more detailed
message here about the device being registered, other than the file name and
line number?

Yes, kbuild already reported that problem. I messed up when selecting
the lines to add to my commit and added this trace instead of

		ret = device_register(&desc->dev->dev);

Will be fixed in v7.

Otherwise, I have gone through the code in the process of writing the first
iteration of the Qualcomm I3C master driver, and overall the framework looks
good.  Will update with any additional feedback I have.

Thanks for looking at the framework code. Your driver is in my TODO
list.

Regards,

Boris