[PATCH] regulator: virtual: add devicetree support

[PATCH] regulator: virtual: add devicetree support

[Vincent Whitchurch]

The reg-virt-consumer is very useful for development and testing of
regulator drivers since it allows voltages and modes to be set from
userspace.  However, it currently requires platform data so it cannot be
used on modern platforms.  Add support for probing it from the
devicetree to remedy this.

Since this driver is only meant for testing and is a purely software
construct, no binding documentation is added.

Signed-off-by: Vincent Whitchurch <vincent.whitchurch@axis.com>
---
 drivers/regulator/virtual.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/regulator/virtual.c b/drivers/regulator/virtual.c
index 52c5a0e0acd8..c4d56b484ed5 100644
--- a/drivers/regulator/virtual.c
+++ b/drivers/regulator/virtual.c
@@ -13,6 +13,7 @@
 #include <linux/regulator/consumer.h>
 #include <linux/slab.h>
 #include <linux/module.h>
+#include <linux/of.h>
 
 struct virtual_consumer_data {
 	struct mutex lock;
@@ -281,6 +282,14 @@ static const struct attribute_group regulator_virtual_attr_group = {
 	.attrs	= regulator_virtual_attributes,
 };
 
+#ifdef CONFIG_OF
+static const struct of_device_id regulator_virtual_consumer_of_match[] = {
+	{ .compatible = "regulator-virtual-consumer" },
+	{},
+};
+MODULE_DEVICE_TABLE(of, regulator_virtual_consumer_of_match);
+#endif
+
 static int regulator_virtual_probe(struct platform_device *pdev)
 {
 	char *reg_id = dev_get_platdata(&pdev->dev);
@@ -292,6 +301,9 @@ static int regulator_virtual_probe(struct platform_device *pdev)
 	if (drvdata == NULL)
 		return -ENOMEM;
 
+	if (!reg_id)
+		reg_id = "default";
+
 	mutex_init(&drvdata->lock);
 
 	drvdata->regulator = devm_regulator_get(&pdev->dev, reg_id);
@@ -334,6 +346,7 @@ static struct platform_driver regulator_virtual_consumer_driver = {
 	.remove		= regulator_virtual_remove,
 	.driver		= {
 		.name		= "reg-virt-consumer",
+		.of_match_table = of_match_ptr(regulator_virtual_consumer_of_match),
 	},
 };
 
-- 
2.34.1

Re: [PATCH] regulator: virtual: add devicetree support

[Mark Brown]

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

On Fri, Feb 18, 2022 at 12:06:03PM +0100, Vincent Whitchurch wrote:

> The reg-virt-consumer is very useful for development and testing of
> regulator drivers since it allows voltages and modes to be set from
> userspace.  However, it currently requires platform data so it cannot be
> used on modern platforms.  Add support for probing it from the
> devicetree to remedy this.

Meh, you can add a bit of code on module_init() to register a platform
device or something.

> Since this driver is only meant for testing and is a purely software
> construct, no binding documentation is added.

That's not going to stop anyone.  We should at the very least be
printing very loud warnings if anyone tries to intantiate this.

> +	if (!reg_id)
> +		reg_id = "default";
> +

Oh?

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

Re: [PATCH] regulator: virtual: add devicetree support

[Vincent Whitchurch]

On Wed, Feb 23, 2022 at 02:23:16PM +0000, Mark Brown wrote:
> On Fri, Feb 18, 2022 at 12:06:03PM +0100, Vincent Whitchurch wrote:
> 
> > The reg-virt-consumer is very useful for development and testing of
> > regulator drivers since it allows voltages and modes to be set from
> > userspace.  However, it currently requires platform data so it cannot be
> > used on modern platforms.  Add support for probing it from the
> > devicetree to remedy this.
> 
> Meh, you can add a bit of code on module_init() to register a platform
> device or something.

Something like that could work during manual testing, but I'm hoping to
use this from a test framework where it's rather impractical to patch
individual drivers in that way.

> > Since this driver is only meant for testing and is a purely software
> > construct, no binding documentation is added.
> 
> That's not going to stop anyone.  We should at the very least be
> printing very loud warnings if anyone tries to intantiate this.

I can add a warning.  Maybe something like this, following the style of
kernel/trace/trace.c?

  pr_warn("**********************************************************\n");
  pr_warn("**   NOTICE NOTICE NOTICE NOTICE NOTICE NOTICE NOTICE   **\n");
  pr_warn("**                                                      **\n");
  pr_warn("** regulator-virtual-consumer is only for testing and   **\n");
  pr_warn("** debugging.  Do not use it in a production kernel.    **\n");
  pr_warn("**                                                      **\n");
  pr_warn("**   NOTICE NOTICE NOTICE NOTICE NOTICE NOTICE NOTICE   **\n");
  pr_warn("**********************************************************\n");

> 
> > +	if (!reg_id)
> > +		reg_id = "default";
> > +
> 
> Oh?

As you know, this results in looking for the regulator phandle in a
property named "default-supply".  I wasn't sure what supply name to use
for this virtual client.  Getting the name from another property seemed
unnecessary.  Would you prefer that, or would you suggest some other
name than "default"?

Re: [PATCH] regulator: virtual: add devicetree support

[Mark Brown]

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

On Mon, Feb 28, 2022 at 04:50:27PM +0100, Vincent Whitchurch wrote:
> On Wed, Feb 23, 2022 at 02:23:16PM +0000, Mark Brown wrote:

> > That's not going to stop anyone.  We should at the very least be
> > printing very loud warnings if anyone tries to intantiate this.

> I can add a warning.  Maybe something like this, following the style of
> kernel/trace/trace.c?

>   pr_warn("**********************************************************\n");
>   pr_warn("**   NOTICE NOTICE NOTICE NOTICE NOTICE NOTICE NOTICE   **\n");
>   pr_warn("**                                                      **\n");
>   pr_warn("** regulator-virtual-consumer is only for testing and   **\n");
>   pr_warn("** debugging.  Do not use it in a production kernel.    **\n");
>   pr_warn("**                                                      **\n");
>   pr_warn("**   NOTICE NOTICE NOTICE NOTICE NOTICE NOTICE NOTICE   **\n");
>   pr_warn("**********************************************************\n");

Something along those lines would do it, yes.

> > > +	if (!reg_id)
> > > +		reg_id = "default";
> > > +

> > Oh?

> As you know, this results in looking for the regulator phandle in a
> property named "default-supply".  I wasn't sure what supply name to use
> for this virtual client.  Getting the name from another property seemed
> unnecessary.  Would you prefer that, or would you suggest some other
> name than "default"?

I'd suggest adding a comment.

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