linux-kernel - Re: [RFC PATCH 1/2] regulator: Add support for software node connections

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]

Date:   Fri, 9 Jul 2021 18:26:47 +0100
From:   Mark Brown <broonie@...nel.org>
To:     Daniel Scally <djrscally@...il.com>
Cc:     linux-kernel@...r.kernel.org, platform-driver-x86@...r.kernel.org,
        hdegoede@...hat.com, mgross@...ux.intel.com,
        luzmaximilian@...il.com, lgirdwood@...il.com,
        andy.shevchenko@...il.com, laurent.pinchart@...asonboard.com,
        kieran.bingham@...asonboard.com
Subject: Re: [RFC PATCH 1/2] regulator: Add support for software node
 connections

On Thu, Jul 08, 2021 at 11:42:25PM +0100, Daniel Scally wrote:

> --- a/drivers/regulator/Makefile
> +++ b/drivers/regulator/Makefile
> @@ -9,6 +9,7 @@ obj-$(CONFIG_OF) += of_regulator.o
>  obj-$(CONFIG_REGULATOR_FIXED_VOLTAGE) += fixed.o
>  obj-$(CONFIG_REGULATOR_VIRTUAL_CONSUMER) += virtual.o
>  obj-$(CONFIG_REGULATOR_USERSPACE_CONSUMER) += userspace-consumer.o
> +obj-$(CONFIG_REGULATOR_SWNODE) += swnode_regulator.o

This appears to be sorted with regulator drivers but as far as I
understand it this is not a driver?  I'd put it next to the OF file.

> @@ -1785,6 +1788,22 @@ static struct regulator_dev *regulator_dev_lookup(struct device *dev,

The line numbers here look off...

> +++ b/drivers/regulator/swnode_regulator.c
> @@ -0,0 +1,111 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Author: Dan Scally <djrscally@...il.com> */
> +

Please make the entire comment a C++ one so things look more
intentional.

> +static struct fwnode_handle *
> +regulator_swnode_get_init_node(struct fwnode_handle *fwnode,
> +			       const struct regulator_desc *desc)
> +{
> +	const struct software_node *parent, *child;
> +
> +	parent = to_software_node(fwnode->secondary);
> +
> +	if (desc->regulators_node)
> +		child = software_node_find_by_name(parent,
> +						   desc->regulators_node);
> +	else
> +		child = software_node_find_by_name(parent, desc->name);
> +
> +	return software_node_fwnode(child);
> +}

Nothing is documenting what the binding for these swnodes is supposed to
be so it's hard to tell if any of this is correct and makes sense, nor
how someone is supposed to take this stuff and integrate it into a
system.  I think this needs some more explicit documentation of what's
going on adding to the tree, this will help with review and help anyone
who needs to use this stuff figure out how to do so.

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