linux-kernel - Re: [PATCH v4 3/5] mfd: p3h2x4x: Add driver for NXP P3H2x4x i3c hub and on-die regulator

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20260114171732470bd389@mail.local>
Date: Wed, 14 Jan 2026 18:17:32 +0100
From: Alexandre Belloni <alexandre.belloni@...tlin.com>
To: Krzysztof Kozlowski <krzk@...nel.org>
Cc: Aman Kumar Pandey <aman.kumarpandey@....com>,
	linux-kernel@...r.kernel.org, linux-i3c@...ts.infradead.org,
	krzk+dt@...nel.org, robh@...nel.org, conor+dt@...nel.org,
	devicetree@...r.kernel.org, broonie@...nel.org, lee@...nel.org,
	Frank.Li@....com, lgirdwood@...il.com, vikash.bansal@....com,
	priyanka.jain@....com, shashank.rebbapragada@....com
Subject: Re: [PATCH v4 3/5] mfd: p3h2x4x: Add driver for NXP P3H2x4x i3c hub
 and on-die regulator

On 14/01/2026 09:28:53+0100, Krzysztof Kozlowski wrote:
> > +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...
> 
> > +				     "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.

> 
> > +		return dev_err_probe(&client->dev, ret, "Failed to register I3C HUB regmap\n");
> > +	}
> > +
> > +	p3h2x4x->is_p3h2x4x_in_i3c = false;
> > +	p3h2x4x->i2c_client = client;
> > +
> > +	ret = devm_mfd_add_devices(&client->dev, PLATFORM_DEVID_NONE,
> > +				   p3h2x4x_devs, ARRAY_SIZE(p3h2x4x_devs),
> > +				   NULL, 0, NULL);
> > +	if (ret)
> > +		return dev_err_probe(&client->dev, ret, "Failed to add sub devices\n");
> > +
> > +	return 0;
> 
> Best regards,
> Krzysztof
> 

-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com