linux-kernel - Re: [PATCH v5 3/3] regulator: add s2dos05 regulator support

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] [day] [month] [year] [list]

Message-ID: <j7ly4fzqiyw2umxepskd3bo6aithspkuvfu7au4qtybr2tmbgm@liwmxothkxax>
Date: Fri, 27 Sep 2024 10:54:16 +0200
From: Krzysztof Kozlowski <krzk@...nel.org>
To: Dzmitry Sankouski <dsankouski@...il.com>
Cc: Lee Jones <lee@...nel.org>, Rob Herring <robh@...nel.org>, 
	Conor Dooley <conor+dt@...nel.org>, Liam Girdwood <lgirdwood@...il.com>, 
	Mark Brown <broonie@...nel.org>, linux-kernel@...r.kernel.org, linux-samsung-soc@...r.kernel.org, 
	devicetree@...r.kernel.org
Subject: Re: [PATCH v5 3/3] regulator: add s2dos05 regulator support

On Fri, Sep 27, 2024 at 10:51:08AM +0200, Krzysztof Kozlowski wrote:
> On Thu, Sep 26, 2024 at 12:47:32PM +0300, Dzmitry Sankouski wrote:
> > +static int s2dos05_pmic_probe(struct platform_device *pdev)
> > +{
> > +	struct device *dev = &pdev->dev;
> > +	struct sec_pmic_dev *iodev = dev_get_drvdata(pdev->dev.parent);
> > +	struct of_regulator_match *rdata = NULL;
> > +	struct s2dos05_data *s2dos05;
> > +	struct regulator_config config = { };
> > +	unsigned int rdev_num = ARRAY_SIZE(regulators);
> > +	int i, ret;
> > +
> > +	s2dos05 = devm_kzalloc(dev, sizeof(*s2dos05), GFP_KERNEL);
> > +	if (!s2dos05)
> > +		return -ENOMEM;
> > +
> > +	platform_set_drvdata(pdev, s2dos05);
> > +
> > +	rdata = devm_kcalloc(dev, rdev_num, sizeof(*rdata), GFP_KERNEL);
> > +	if (!rdata)
> > +		return -ENOMEM;
> > +
> > +	for (i = 0; i < rdev_num; i++)
> > +		rdata[i].name = regulators[i].name;
> > +
> > +	s2dos05->regmap = iodev->regmap_pmic;
> > +	s2dos05->dev = dev;
> > +	if (!dev->of_node)
> > +		dev->of_node = dev->parent->of_node;
> > +
> > +	for (i = 0; i < rdev_num; i++) {
> > +		struct regulator_dev *regulator;
> > +
> > +		config.init_data = rdata[i].init_data;
> > +		config.of_node = rdata[i].of_node;
> > +		config.dev = dev;
> > +		config.driver_data = s2dos05;
> > +		regulator = devm_regulator_register(&pdev->dev,
> > +						&regulators[i], &config);
> > +		if (IS_ERR(regulator)) {
> > +			ret = PTR_ERR(regulator);
> > +			dev_err(&pdev->dev, "regulator init failed for %d\n",
> > +				i);
> > +		}
> > +	}
> > +
> > +	return ret;
> 
> ret is uninitialized. Please test your code with smatch and sparse.
> 
> Since I expect a new version, I will have a comment on the bindings as well.

Ah, no, looks fine. I was thinking you missed interrupts, but aparently
device is quite simple and I coould not find interrupts in downstream
source.

Still please investigate above 'ret' and run smatch and sparse.

Best regards,
Krzysztof