diff mbox series

[1/4] regulator: da9062: refactor buck modes into header

Message ID 1573121050-4728-2-git-send-email-chf.fritz@googlemail.com (mailing list archive)
State New, archived
Headers show
Series regulator da9062: support setting buck modes | expand

Commit Message

Christoph Fritz Nov. 7, 2019, 10:04 a.m. UTC 
This patch refactors buck modes into a header so that device trees
can make use of these mode constants.

Signed-off-by: Christoph Fritz <chf.fritz@googlemail.com>
---
 drivers/regulator/da9062-regulator.c               | 28 ++++++++--------------
 .../dt-bindings/regulator/dlg,da906x-regulator.h   | 16 +++++++++++++
 2 files changed, 26 insertions(+), 18 deletions(-)
 create mode 100644 include/dt-bindings/regulator/dlg,da906x-regulator.h

Comments

Adam Thomson Nov. 8, 2019, 10:37 a.m. UTC | #1 
On 07 November 2019 10:04, Christoph Fritz wrote: 

> This patch refactors buck modes into a header so that device trees
> can make use of these mode constants.
> 
> Signed-off-by: Christoph Fritz <chf.fritz@googlemail.com>
> ---
>  drivers/regulator/da9062-regulator.c               | 28 ++++++++--------------
>  .../dt-bindings/regulator/dlg,da906x-regulator.h   | 16 +++++++++++++
>  2 files changed, 26 insertions(+), 18 deletions(-)
>  create mode 100644 include/dt-bindings/regulator/dlg,da906x-regulator.h
> 
> diff --git a/drivers/regulator/da9062-regulator.c b/drivers/regulator/da9062-
> regulator.c
> index 710e670..1a95982 100644
> --- a/drivers/regulator/da9062-regulator.c
> +++ b/drivers/regulator/da9062-regulator.c
> @@ -16,6 +16,7 @@
>  #include <linux/regulator/of_regulator.h>
>  #include <linux/mfd/da9062/core.h>
>  #include <linux/mfd/da9062/registers.h>
> +#include <dt-bindings/regulator/dlg,da906x-regulator.h>

Can we please rename this file to use da9063 instead of da906x as this header
wouldn't necessarily be valid for other da906x prefixed devices outside of
DA9061/2/3. DA9063 was the earlier chip so it makes sense to use that name over
DA9062, and DA9063 driver code will want updating at some point in a similar
manner.

> 
>  /* Regulator IDs */
>  enum {
> @@ -75,14 +76,6 @@ struct da9062_regulators {
>  	struct da9062_regulator			regulator[0];
>  };
> 
> -/* BUCK modes */
> -enum {
> -	BUCK_MODE_MANUAL,	/* 0 */
> -	BUCK_MODE_SLEEP,	/* 1 */
> -	BUCK_MODE_SYNC,		/* 2 */
> -	BUCK_MODE_AUTO		/* 3 */
> -};
> -
>  /* Regulator operations */
> 
>  /* Current limits array (in uA)
> @@ -112,13 +105,13 @@ static int da9062_buck_set_mode(struct regulator_dev
> *rdev, unsigned mode)
> 
>  	switch (mode) {
>  	case REGULATOR_MODE_FAST:
> -		val = BUCK_MODE_SYNC;
> +		val = DA906X_BUCK_MODE_SYNC;
>  		break;
>  	case REGULATOR_MODE_NORMAL:
> -		val = BUCK_MODE_AUTO;
> +		val = DA906X_BUCK_MODE_AUTO;
>  		break;
>  	case REGULATOR_MODE_STANDBY:
> -		val = BUCK_MODE_SLEEP;
> +		val = DA906X_BUCK_MODE_SLEEP;
>  		break;
>  	default:
>  		return -EINVAL;
> @@ -145,15 +138,14 @@ static unsigned da9062_buck_get_mode(struct
> regulator_dev *rdev)
> 
>  	switch (val) {
>  	default:
> -	case BUCK_MODE_MANUAL:
>  		mode = REGULATOR_MODE_FAST |
> REGULATOR_MODE_STANDBY;
>  		/* Sleep flag bit decides the mode */
>  		break;

I'm not sure your code is based on the latest regulator updates as I believe
Axel Lin made some improvements to this bit of code. Checkout Mark's regulator
fork of the kernel.

> -	case BUCK_MODE_SLEEP:
> +	case DA906X_BUCK_MODE_SLEEP:
>  		return REGULATOR_MODE_STANDBY;
> -	case BUCK_MODE_SYNC:
> +	case DA906X_BUCK_MODE_SYNC:
>  		return REGULATOR_MODE_FAST;
> -	case BUCK_MODE_AUTO:
> +	case DA906X_BUCK_MODE_AUTO:
>  		return REGULATOR_MODE_NORMAL;
>  	}
> 
> @@ -282,13 +274,13 @@ static int da9062_buck_set_suspend_mode(struct
> regulator_dev *rdev,
> 
>  	switch (mode) {
>  	case REGULATOR_MODE_FAST:
> -		val = BUCK_MODE_SYNC;
> +		val = DA906X_BUCK_MODE_SYNC;
>  		break;
>  	case REGULATOR_MODE_NORMAL:
> -		val = BUCK_MODE_AUTO;
> +		val = DA906X_BUCK_MODE_AUTO;
>  		break;
>  	case REGULATOR_MODE_STANDBY:
> -		val = BUCK_MODE_SLEEP;
> +		val = DA906X_BUCK_MODE_SLEEP;
>  		break;
>  	default:
>  		return -EINVAL;
> diff --git a/include/dt-bindings/regulator/dlg,da906x-regulator.h b/include/dt-
> bindings/regulator/dlg,da906x-regulator.h
> new file mode 100644
> index 00000000..848a4df
> --- /dev/null
> +++ b/include/dt-bindings/regulator/dlg,da906x-regulator.h
> @@ -0,0 +1,16 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +
> +#ifndef __DLG_DA906X_REGULATOR_H
> +#define __DLG_DA906X_REGULATOR_H

Just to echo previous comment, rename from DA906X to DA9063

> +
> +/*
> + * These buck mode constants may be used to specify values in device tree
> + * properties (e.g. regulator-initial-mode).
> + * A description of the following modes is in the manufacturers datasheet.
> + */
> +
> +#define DA906X_BUCK_MODE_SLEEP		1
> +#define DA906X_BUCK_MODE_SYNC		2
> +#define DA906X_BUCK_MODE_AUTO		3
> +
> +#endif
> --
> 2.1.4
Christoph Fritz Nov. 13, 2019, 11:04 a.m. UTC | #2 
Hi Adam

On Fri, 2019-11-08 at 10:37 +0000, Adam Thomson wrote:
> > --- a/drivers/regulator/da9062-regulator.c
> > +++ b/drivers/regulator/da9062-regulator.c
> > @@ -16,6 +16,7 @@
> >  #include <linux/regulator/of_regulator.h>
> >  #include <linux/mfd/da9062/core.h>
> >  #include <linux/mfd/da9062/registers.h>
> > +#include <dt-bindings/regulator/dlg,da906x-regulator.h>
> 
> Can we please rename this file to use da9063 instead of da906x
[..]

sure

> > @@ -145,15 +138,14 @@ static unsigned da9062_buck_get_mode(struct
> > regulator_dev *rdev)
> > 
> >  	switch (val) {
> >  	default:
> > -	case BUCK_MODE_MANUAL:
> >  		mode = REGULATOR_MODE_FAST |
> > REGULATOR_MODE_STANDBY;
> >  		/* Sleep flag bit decides the mode */
> >  		break;
> 
> I'm not sure your code is based on the latest regulator updates as I believe
> Axel Lin made some improvements to this bit of code. Checkout Mark's regulator
> fork of the kernel.

yes, the line

 mode = REGULATOR_MODE_FAST | REGULATOR_MODE_STANDBY;

is now gone by commit

 be446f183ae35a8c76
 regulator: da9062: Simplify da9062_buck_set_mode for BUCK_MODE_MANUAL case

it's already in linux-next, I'll rebase this patchset


> > diff --git a/include/dt-bindings/regulator/dlg,da906x-regulator.h b/include/dt-
> > bindings/regulator/dlg,da906x-regulator.h
> > new file mode 100644
> > index 00000000..848a4df
> > --- /dev/null
> > +++ b/include/dt-bindings/regulator/dlg,da906x-regulator.h
> > @@ -0,0 +1,16 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +
> > +#ifndef __DLG_DA906X_REGULATOR_H
> > +#define __DLG_DA906X_REGULATOR_H
> 
> Just to echo previous comment, rename from DA906X to DA9063

ok
diff mbox series

Patch

diff --git a/drivers/regulator/da9062-regulator.c b/drivers/regulator/da9062-regulator.c
index 710e670..1a95982 100644
--- a/drivers/regulator/da9062-regulator.c
+++ b/drivers/regulator/da9062-regulator.c
@@ -16,6 +16,7 @@ 
 #include <linux/regulator/of_regulator.h>
 #include <linux/mfd/da9062/core.h>
 #include <linux/mfd/da9062/registers.h>
+#include <dt-bindings/regulator/dlg,da906x-regulator.h>
 
 /* Regulator IDs */
 enum {
@@ -75,14 +76,6 @@  struct da9062_regulators {
 	struct da9062_regulator			regulator[0];
 };
 
-/* BUCK modes */
-enum {
-	BUCK_MODE_MANUAL,	/* 0 */
-	BUCK_MODE_SLEEP,	/* 1 */
-	BUCK_MODE_SYNC,		/* 2 */
-	BUCK_MODE_AUTO		/* 3 */
-};
-
 /* Regulator operations */
 
 /* Current limits array (in uA)
@@ -112,13 +105,13 @@  static int da9062_buck_set_mode(struct regulator_dev *rdev, unsigned mode)
 
 	switch (mode) {
 	case REGULATOR_MODE_FAST:
-		val = BUCK_MODE_SYNC;
+		val = DA906X_BUCK_MODE_SYNC;
 		break;
 	case REGULATOR_MODE_NORMAL:
-		val = BUCK_MODE_AUTO;
+		val = DA906X_BUCK_MODE_AUTO;
 		break;
 	case REGULATOR_MODE_STANDBY:
-		val = BUCK_MODE_SLEEP;
+		val = DA906X_BUCK_MODE_SLEEP;
 		break;
 	default:
 		return -EINVAL;
@@ -145,15 +138,14 @@  static unsigned da9062_buck_get_mode(struct regulator_dev *rdev)
 
 	switch (val) {
 	default:
-	case BUCK_MODE_MANUAL:
 		mode = REGULATOR_MODE_FAST | REGULATOR_MODE_STANDBY;
 		/* Sleep flag bit decides the mode */
 		break;
-	case BUCK_MODE_SLEEP:
+	case DA906X_BUCK_MODE_SLEEP:
 		return REGULATOR_MODE_STANDBY;
-	case BUCK_MODE_SYNC:
+	case DA906X_BUCK_MODE_SYNC:
 		return REGULATOR_MODE_FAST;
-	case BUCK_MODE_AUTO:
+	case DA906X_BUCK_MODE_AUTO:
 		return REGULATOR_MODE_NORMAL;
 	}
 
@@ -282,13 +274,13 @@  static int da9062_buck_set_suspend_mode(struct regulator_dev *rdev,
 
 	switch (mode) {
 	case REGULATOR_MODE_FAST:
-		val = BUCK_MODE_SYNC;
+		val = DA906X_BUCK_MODE_SYNC;
 		break;
 	case REGULATOR_MODE_NORMAL:
-		val = BUCK_MODE_AUTO;
+		val = DA906X_BUCK_MODE_AUTO;
 		break;
 	case REGULATOR_MODE_STANDBY:
-		val = BUCK_MODE_SLEEP;
+		val = DA906X_BUCK_MODE_SLEEP;
 		break;
 	default:
 		return -EINVAL;
diff --git a/include/dt-bindings/regulator/dlg,da906x-regulator.h b/include/dt-bindings/regulator/dlg,da906x-regulator.h
new file mode 100644
index 00000000..848a4df
--- /dev/null
+++ b/include/dt-bindings/regulator/dlg,da906x-regulator.h
@@ -0,0 +1,16 @@ 
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#ifndef __DLG_DA906X_REGULATOR_H
+#define __DLG_DA906X_REGULATOR_H
+
+/*
+ * These buck mode constants may be used to specify values in device tree
+ * properties (e.g. regulator-initial-mode).
+ * A description of the following modes is in the manufacturers datasheet.
+ */
+
+#define DA906X_BUCK_MODE_SLEEP		1
+#define DA906X_BUCK_MODE_SYNC		2
+#define DA906X_BUCK_MODE_AUTO		3
+
+#endif