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

[Mark Brown <broonie@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 902 bytes --]

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?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]