diff mbox series

[5.10.y-cip,02/13] regulator: core: Add helper for allow HW access to enable/disable regulator

Message ID 20240729153504.510443-3-biju.das.jz@bp.renesas.com (mailing list archive)
State New
Headers show
Series RZ/G2L enhancements | expand

Commit Message

Biju Das July 29, 2024, 3:34 p.m. UTC 
commit 1cb7d29157603561af4c38535e936850ceb99f0f upstream.

Add a helper function that allow regulator consumers to allow low-level
HW access, in order to enable/disable regulator in atomic context.

The use-case for RZ/G2L SoC is to enable VBUS selection register based
on vbus detection that happens in interrupt context.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Link: https://patch.msgid.link/20240616105402.45211-4-biju.das.jz@bp.renesas.com
Signed-off-by: Mark Brown <broonie@kernel.org>
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
 Documentation/power/regulator/consumer.rst |  6 +++++
 drivers/regulator/core.c                   | 28 ++++++++++++++++++++++
 include/linux/regulator/consumer.h         |  7 ++++++
 3 files changed, 41 insertions(+)

Comments

Pavel Machek July 30, 2024, 12:28 p.m. UTC | #1 
Hi!

> commit 1cb7d29157603561af4c38535e936850ceb99f0f upstream.
> 
> Add a helper function that allow regulator consumers to allow low-level
> HW access, in order to enable/disable regulator in atomic context.
> 
> The use-case for RZ/G2L SoC is to enable VBUS selection register based
> on vbus detection that happens in interrupt context.
> 
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> Link: https://patch.msgid.link/20240616105402.45211-4-biju.das.jz@bp.renesas.com
> Signed-off-by: Mark Brown <broonie@kernel.org>
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> ---
>  Documentation/power/regulator/consumer.rst |  6 +++++
>  drivers/regulator/core.c                   | 28 ++++++++++++++++++++++
>  include/linux/regulator/consumer.h         |  7 ++++++
>  3 files changed, 41 insertions(+)
> 
> diff --git a/Documentation/power/regulator/consumer.rst b/Documentation/power/regulator/consumer.rst
> index 0cd8cc1275a7..ebc4c8076711 100644
> --- a/Documentation/power/regulator/consumer.rst
> +++ b/Documentation/power/regulator/consumer.rst
> @@ -227,3 +227,9 @@ directly written to the voltage selector register, use::
>  
>  	int regulator_list_hardware_vsel(struct regulator *regulator,
>  					 unsigned selector);
> +
> +To access the hardware for enabling/disabling the regulator, consumers must
> +use regulator_get_exclusive(), as it can't work if there's more than one
> +consumer. To enable/disable regulator use::
> +
> +	int regulator_hardware_enable(struct regulator *regulator, bool enable);
> diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
> index 7082cffdd10e..c5b6979b2560 100644
> --- a/drivers/regulator/core.c
> +++ b/drivers/regulator/core.c
> @@ -3244,6 +3244,34 @@ int regulator_list_hardware_vsel(struct regulator *regulator,
>  }
>  EXPORT_SYMBOL_GPL(regulator_list_hardware_vsel);
>  
> +/**
> + * regulator_hardware_enable - access the HW for enable/disable regulator
> + * @regulator: regulator source
> + * @enable: true for enable, false for disable
> + *
> + * Request that the regulator be enabled/disabled with the regulator output at
> + * the predefined voltage or current value.

"Unlinke other functions, this can be called ...".

And fill in the blanks. "from interrupt context"? "with interrupts
disabled"? I'm not sure what real limits are. Probably can't do that
from NMI?

Best regards,
								Pavel
Biju Das July 30, 2024, 12:32 p.m. UTC | #2 
Hi Pavel,

Thanks for the feedback.

> -----Original Message-----
> From: Pavel Machek <pavel@denx.de>
> Sent: Tuesday, July 30, 2024 1:28 PM
> enable/disable regulator
> 
> Hi!
> 
> > commit 1cb7d29157603561af4c38535e936850ceb99f0f upstream.
> >
> > Add a helper function that allow regulator consumers to allow
> > low-level HW access, in order to enable/disable regulator in atomic context.
> >
> > The use-case for RZ/G2L SoC is to enable VBUS selection register based
> > on vbus detection that happens in interrupt context.
> >
> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > Link:
> > https://patch.msgid.link/20240616105402.45211-4-biju.das.jz@bp.renesas
> > .com
> > Signed-off-by: Mark Brown <broonie@kernel.org>
> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > ---
> >  Documentation/power/regulator/consumer.rst |  6 +++++
> >  drivers/regulator/core.c                   | 28 ++++++++++++++++++++++
> >  include/linux/regulator/consumer.h         |  7 ++++++
> >  3 files changed, 41 insertions(+)
> >
> > diff --git a/Documentation/power/regulator/consumer.rst
> > b/Documentation/power/regulator/consumer.rst
> > index 0cd8cc1275a7..ebc4c8076711 100644
> > --- a/Documentation/power/regulator/consumer.rst
> > +++ b/Documentation/power/regulator/consumer.rst
> > @@ -227,3 +227,9 @@ directly written to the voltage selector register, use::
> >
> >  	int regulator_list_hardware_vsel(struct regulator *regulator,
> >  					 unsigned selector);
> > +
> > +To access the hardware for enabling/disabling the regulator,
> > +consumers must use regulator_get_exclusive(), as it can't work if
> > +there's more than one consumer. To enable/disable regulator use::
> > +
> > +	int regulator_hardware_enable(struct regulator *regulator, bool
> > +enable);
> > diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c index
> > 7082cffdd10e..c5b6979b2560 100644
> > --- a/drivers/regulator/core.c
> > +++ b/drivers/regulator/core.c
> > @@ -3244,6 +3244,34 @@ int regulator_list_hardware_vsel(struct
> > regulator *regulator,  }
> > EXPORT_SYMBOL_GPL(regulator_list_hardware_vsel);
> >
> > +/**
> > + * regulator_hardware_enable - access the HW for enable/disable
> > +regulator
> > + * @regulator: regulator source
> > + * @enable: true for enable, false for disable
> > + *
> > + * Request that the regulator be enabled/disabled with the regulator
> > +output at
> > + * the predefined voltage or current value.
> 
> "Unlinke other functions, this can be called ...".
> 
> And fill in the blanks. "from interrupt context"? "with interrupts disabled"? I'm not sure what real
> limits are. Probably can't do that from NMI?

From interrupt context provided the callback function does not uses any sleeping calls.

Cheers,
Biju

> 
> Best regards,
> 								Pavel
> --
> DENX Software Engineering GmbH,        Managing Director: Erika Unter
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Pavel Machek July 31, 2024, 10:06 a.m. UTC | #3 
Hi!

> > > +++ b/drivers/regulator/core.c
> > > @@ -3244,6 +3244,34 @@ int regulator_list_hardware_vsel(struct
> > > regulator *regulator,  }
> > > EXPORT_SYMBOL_GPL(regulator_list_hardware_vsel);
> > >
> > > +/**
> > > + * regulator_hardware_enable - access the HW for enable/disable
> > > +regulator
> > > + * @regulator: regulator source
> > > + * @enable: true for enable, false for disable
> > > + *
> > > + * Request that the regulator be enabled/disabled with the regulator
> > > +output at
> > > + * the predefined voltage or current value.
> > 
> > "Unlinke other functions, this can be called ...".
> > 
> > And fill in the blanks. "from interrupt context"? "with interrupts disabled"? I'm not sure what real
> > limits are. Probably can't do that from NMI?
> 
> From interrupt context provided the callback function does not uses any sleeping calls.

So the contexts allowed differ based on what low-level driver is used?
That's quite unfortunate...

Best regards,
								Pavel
Biju Das July 31, 2024, 1:08 p.m. UTC | #4 
Hi Pavel,

> -----Original Message-----
> From: Pavel Machek <pavel@denx.de>
> Sent: Wednesday, July 31, 2024 11:06 AM
> Subject: Re: [PATCH 5.10.y-cip 02/13] regulator: core: Add helper for
> allow HW access to enable/disable regulator
> 
> Hi!
> 
> > > > +++ b/drivers/regulator/core.c
> > > > @@ -3244,6 +3244,34 @@ int regulator_list_hardware_vsel(struct
> > > > regulator *regulator,  }
> > > > EXPORT_SYMBOL_GPL(regulator_list_hardware_vsel);
> > > >
> > > > +/**
> > > > + * regulator_hardware_enable - access the HW for enable/disable
> > > > +regulator
> > > > + * @regulator: regulator source
> > > > + * @enable: true for enable, false for disable
> > > > + *
> > > > + * Request that the regulator be enabled/disabled with the
> > > > +regulator output at
> > > > + * the predefined voltage or current value.
> > >
> > > "Unlinke other functions, this can be called ...".
> > >
> > > And fill in the blanks. "from interrupt context"? "with interrupts
> > > disabled"? I'm not sure what real limits are. Probably can't do that
> from NMI?
> >
> > From interrupt context provided the callback function does not uses any
> sleeping calls.
> 
> So the contexts allowed differ based on what low-level driver is used?
> That's quite unfortunate...

The only limitation is it has to be exclusive usage. Other than that it is for generic usage.

Ie, can be used for regulator_gpio_enable or regulator_regmap_enable()

Currently regulator API won't allow you to enable or disable() in atomic context.
But this API allows to you do that.


Cheers,
Biju
diff mbox series

Patch

diff --git a/Documentation/power/regulator/consumer.rst b/Documentation/power/regulator/consumer.rst
index 0cd8cc1275a7..ebc4c8076711 100644
--- a/Documentation/power/regulator/consumer.rst
+++ b/Documentation/power/regulator/consumer.rst
@@ -227,3 +227,9 @@  directly written to the voltage selector register, use::
 
 	int regulator_list_hardware_vsel(struct regulator *regulator,
 					 unsigned selector);
+
+To access the hardware for enabling/disabling the regulator, consumers must
+use regulator_get_exclusive(), as it can't work if there's more than one
+consumer. To enable/disable regulator use::
+
+	int regulator_hardware_enable(struct regulator *regulator, bool enable);
diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index 7082cffdd10e..c5b6979b2560 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -3244,6 +3244,34 @@  int regulator_list_hardware_vsel(struct regulator *regulator,
 }
 EXPORT_SYMBOL_GPL(regulator_list_hardware_vsel);
 
+/**
+ * regulator_hardware_enable - access the HW for enable/disable regulator
+ * @regulator: regulator source
+ * @enable: true for enable, false for disable
+ *
+ * Request that the regulator be enabled/disabled with the regulator output at
+ * the predefined voltage or current value.
+ *
+ * On success 0 is returned, otherwise a negative errno is returned.
+ */
+int regulator_hardware_enable(struct regulator *regulator, bool enable)
+{
+	struct regulator_dev *rdev = regulator->rdev;
+	const struct regulator_ops *ops = rdev->desc->ops;
+	int ret = -EOPNOTSUPP;
+
+	if (!rdev->exclusive || !ops || !ops->enable || !ops->disable)
+		return ret;
+
+	if (enable)
+		ret = ops->enable(rdev);
+	else
+		ret = ops->disable(rdev);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(regulator_hardware_enable);
+
 /**
  * regulator_get_linear_step - return the voltage step size between VSEL values
  * @regulator: regulator source
diff --git a/include/linux/regulator/consumer.h b/include/linux/regulator/consumer.h
index 20e84a84fb77..42da800f86ff 100644
--- a/include/linux/regulator/consumer.h
+++ b/include/linux/regulator/consumer.h
@@ -268,6 +268,7 @@  int regulator_get_hardware_vsel_register(struct regulator *regulator,
 					 unsigned *vsel_mask);
 int regulator_list_hardware_vsel(struct regulator *regulator,
 				 unsigned selector);
+int regulator_hardware_enable(struct regulator *regulator, bool enable);
 
 /* regulator notifier block */
 int regulator_register_notifier(struct regulator *regulator,
@@ -565,6 +566,12 @@  static inline int regulator_list_hardware_vsel(struct regulator *regulator,
 	return -EOPNOTSUPP;
 }
 
+static inline int regulator_hardware_enable(struct regulator *regulator,
+					    bool enable)
+{
+	return -EOPNOTSUPP;
+}
+
 static inline int regulator_register_notifier(struct regulator *regulator,
 			      struct notifier_block *nb)
 {