linux-kernel - Re: [PATCH V4 4/6] regulator: Add a regulator driver for the PM8008 PMIC

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: <YZ+vnV12gDCtia5S@sirena.org.uk>
Date:   Thu, 25 Nov 2021 15:45:33 +0000
From:   Mark Brown <broonie@...nel.org>
To:     Satya Priya <quic_c_skakit@...cinc.com>
Cc:     Bjorn Andersson <bjorn.andersson@...aro.org>,
        Rob Herring <robh+dt@...nel.org>,
        Liam Girdwood <lgirdwood@...il.com>, swboyd@...omium.org,
        collinsd@...eaurora.org, subbaram@...eaurora.org,
        Das Srinagesh <gurus@...eaurora.org>,
        linux-arm-msm@...r.kernel.org, Lee Jones <lee.jones@...aro.org>,
        devicetree@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH V4 4/6] regulator: Add a regulator driver for the PM8008
 PMIC

On Fri, Nov 19, 2021 at 03:12:31PM +0530, Satya Priya wrote:

> +	for (reg = &reg_data[0]; reg->name; reg++) {

Why is this not just iterating from 0 to ARRAY_SIZE() - that's the more
normal way to write things and doesn't require a terminator on the
array.

> +		child_node = of_get_child_by_name(parent_node, reg->name);
> +		if (!child_node) {
> +			dev_err(dev, "child node %s not found\n", reg->name);
> +			return -ENODEV;
> +		}

This could be pulled out of the array.  I think you're also missing an
of_node_put() on the child_node.

> +		rc = of_property_read_u32(child_node, "reg", &base);
> +		if (rc < 0) {
> +			dev_err(dev, "%s: failed to get regulator base rc=%d\n",
> +						reg->name, rc);
> +			return rc;
> +		}

It's not clear to me why this in particular is being read out of the DT
binding, I'd expect this to be in the array describing the regulator the
same as everything else?

Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)