On 14/01/2026 18:17, Alexandre Belloni wrote:

On 14/01/2026 09:28:53+0100, Krzysztof Kozlowski wrote:

quoted

quoted

+static int p3h2x4x_device_probe_i3c(struct i3c_device *i3cdev)
+{
+	struct p3h2x4x_dev *p3h2x4x;
+	int ret;
+
+	p3h2x4x = devm_kzalloc(&i3cdev->dev, sizeof(*p3h2x4x), GFP_KERNEL);
+	if (!p3h2x4x)
+		return -ENOMEM;
+
+	i3cdev_set_drvdata(i3cdev, p3h2x4x);
+
+	p3h2x4x->regmap = devm_regmap_init_i3c(i3cdev, &p3h2x4x_regmap_config);
+	if (IS_ERR(p3h2x4x->regmap)) {
+		return dev_err_probe(&i3cdev->dev, PTR_ERR(p3h2x4x->regmap),

Look here...

quoted

+				     "Failed to register I3C HUB regmap\n");
+	}
+
+	p3h2x4x->is_p3h2x4x_in_i3c = true;
+	p3h2x4x->i3cdev = i3cdev;
+
+	ret = devm_mfd_add_devices(&i3cdev->dev, PLATFORM_DEVID_NONE,
+				   p3h2x4x_devs, ARRAY_SIZE(p3h2x4x_devs),
+				   NULL, 0, NULL);
+	if (ret)
+		return dev_err_probe(&i3cdev->dev, ret, "Failed to add sub devices\n");
+
+	return 0;
+}
+
+static int p3h2x4x_device_probe_i2c(struct i2c_client *client)
+{
+	struct p3h2x4x_dev *p3h2x4x;
+	int ret;
+
+	p3h2x4x = devm_kzalloc(&client->dev, sizeof(*p3h2x4x), GFP_KERNEL);
+	if (!p3h2x4x)
+		return -ENOMEM;
+
+	i2c_set_clientdata(client, p3h2x4x);
+
+	p3h2x4x->regmap = devm_regmap_init_i2c(client, &p3h2x4x_regmap_config);
+	if (IS_ERR(p3h2x4x->regmap)) {
+		ret = PTR_ERR(p3h2x4x->regmap);

And here!

Drop. This is completely redundant. How is it that in one place you use
one syntax and in other place completely different syntax?

I don't think it is redundant, one is for i2c and the other one for i3c
and the other i2c so I guess the hub can be sitting on any of those bus.

I guess the driver could be made so that we could only compile the i2c
part though.

I am speaking about ret assignment. This is where my comment was placed.



Best regards,
Krzysztof