headers
Krystian Garbaciak | 11 Jul 2012 14:10

[PATCH] regulator: Fix bug in regulator_mode_to_status() core function.

Fix typo for case REGULATOR_STATUS_STANDBY -> REGULATOR_MODE_STANDBY.
For undefined mode, return REGULATOR_STATUS_ERROR (0 is not valid status).

Signed-off-by: Krystian Garbaciak <krystian.garbaciak <at> diasemi.com>
---
 drivers/regulator/core.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index 09a737c..b821a9f 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
 <at>  <at>  -2909,10 +2909,10  <at>  <at>  int regulator_mode_to_status(unsigned int mode)
 		return REGULATOR_STATUS_NORMAL;
 	case REGULATOR_MODE_IDLE:
 		return REGULATOR_STATUS_IDLE;
-	case REGULATOR_STATUS_STANDBY:
+	case REGULATOR_MODE_STANDBY:
 		return REGULATOR_STATUS_STANDBY;
 	default:
-		return 0;
+		return REGULATOR_STATUS_ERROR;
 	}
 }
 EXPORT_SYMBOL_GPL(regulator_mode_to_status);
--

-- 
1.7.0.4
Permalink | Reply |
headers
Mark Brown | 11 Jul 2012 15:28
Favicon
Gravatar

Re: [PATCH] regulator: Fix bug in regulator_mode_to_status() core function.

On Wed, Jul 11, 2012 at 01:10:01PM +0100, Krystian Garbaciak wrote:
> Fix typo for case REGULATOR_STATUS_STANDBY -> REGULATOR_MODE_STANDBY.
> For undefined mode, return REGULATOR_STATUS_ERROR (0 is not valid status).

This is deliberate.  It's not reporting an error, it's reporting an
indeterminate status which is a different thing.
Permalink | Reply |
headers
Krystian Garbaciak | 11 Jul 2012 16:18

Re: [PATCH] regulator: Fix bug in regulator_mode_to_status() core function.

> From: Mark Brown [mailto:broonie <at> opensource.wolfsonmicro.com]
> Sent: 11 July 2012 14:29
> 
> On Wed, Jul 11, 2012 at 01:10:01PM +0100, Krystian Garbaciak wrote:
> > Fix typo for case REGULATOR_STATUS_STANDBY -> REGULATOR_MODE_STANDBY.
> > For undefined mode, return REGULATOR_STATUS_ERROR (0 is not valid status).
> 
> This is deliberate.  It's not reporting an error, it's reporting an
> indeterminate status which is a different thing.

Ok, then I would propose to use REGULATOR_STATUS_OFF instead of 0, to present
your deliberate decision here.
Permalink | Reply |
headers
Mark Brown | 11 Jul 2012 16:26
Favicon
Gravatar

Re: [PATCH] regulator: Fix bug in regulator_mode_to_status() core function.

On Wed, Jul 11, 2012 at 03:18:00PM +0100, Krystian Garbaciak wrote:

> > This is deliberate.  It's not reporting an error, it's reporting an
> > indeterminate status which is a different thing.

> Ok, then I would propose to use REGULATOR_STATUS_OFF instead of 0, to present
> your deliberate decision here.

I don't think you're fully understanding "indeterminate" there...  the
whole point is that we can't adequately represent the state, making
something up isn't helping things.
Permalink | Reply |
headers
Krystian Garbaciak | 11 Jul 2012 17:13

Re: [PATCH] regulator: Fix bug in regulator_mode_to_status() core function.

> > > This is deliberate.  It's not reporting an error, it's reporting an
> > > indeterminate status which is a different thing.
> 
> > Ok, then I would propose to use REGULATOR_STATUS_OFF instead of 0, to present
> > your deliberate decision here.
> 
> I don't think you're fully understanding "indeterminate" there...  the
> whole point is that we can't adequately represent the state, making
> something up isn't helping things.

Would it make more sense to have some special enum value for that case, let say
there would be REGULATOR_STATUS_UNDEFINED?

Returning 0 is interpreted as REGULATOR_STATUS_OFF outside the function.
But it may change, if ever the enumeration changes.
Permalink | Reply |
headers
Mark Brown | 11 Jul 2012 19:42
Favicon
Gravatar

Re: [PATCH] regulator: Fix bug in regulator_mode_to_status() core function.

On Wed, Jul 11, 2012 at 04:13:00PM +0100, Krystian Garbaciak wrote:

> Would it make more sense to have some special enum value for that case, let say
> there would be REGULATOR_STATUS_UNDEFINED?

> Returning 0 is interpreted as REGULATOR_STATUS_OFF outside the function.
> But it may change, if ever the enumeration changes.

That'd be fine.

Also, please do submit separate changes separately.
Permalink | Reply |
headers
Krystian Garbaciak | 12 Jul 2012 12:50

[PATCH] regulator: Fix a typo in regulator_mode_to_status() core function.

Case REGULATOR_STATUS_STANDBY -> REGULATOR_MODE_STANDBY.

Signed-off-by: Krystian Garbaciak <krystian.garbaciak <at> diasemi.com>
---
 drivers/regulator/core.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index 09a737c..af44b94 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
 <at>  <at>  -2909,7 +2909,7  <at>  <at>  int regulator_mode_to_status(unsigned int mode)
 		return REGULATOR_STATUS_NORMAL;
 	case REGULATOR_MODE_IDLE:
 		return REGULATOR_STATUS_IDLE;
-	case REGULATOR_STATUS_STANDBY:
+	case REGULATOR_MODE_STANDBY:
 		return REGULATOR_STATUS_STANDBY;
 	default:
 		return 0;
--

-- 
1.7.0.4
Permalink | Reply |
headers
Krystian Garbaciak | 12 Jul 2012 14:53

[PATCH] regulator: Add REGULATOR_STATUS_UNDEFINED.

REGULATOR_STATUS_UNDEFINED is to be returned by regulator, if any other state
doesn't really apply.

Signed-off-by: Krystian Garbaciak <krystian.garbaciak <at> diasemi.com>
---
 drivers/regulator/core.c         |    5 ++++-
 include/linux/regulator/driver.h |    2 ++
 2 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index af44b94..f042a8a 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
 <at>  <at>  -427,6 +427,9  <at>  <at>  static ssize_t regulator_status_show(struct device *dev,
 	case REGULATOR_STATUS_STANDBY:
 		label = "standby";
 		break;
+	case REGULATOR_STATUS_UNDEFINED:
+		label = "undefined";
+		break;
 	default:
 		return -ERANGE;
 	}
 <at>  <at>  -2912,7 +2915,7  <at>  <at>  int regulator_mode_to_status(unsigned int mode)
 	case REGULATOR_MODE_STANDBY:
 		return REGULATOR_STATUS_STANDBY;
 	default:
-		return 0;
+		return REGULATOR_STATUS_UNDEFINED;
 	}
(Continue reading)

Permalink | Reply |
headers
Mark Brown | 12 Jul 2012 19:20
Favicon
Gravatar

Re: [PATCH] regulator: Add REGULATOR_STATUS_UNDEFINED.

On Thu, Jul 12, 2012 at 01:53:35PM +0100, Krystian Garbaciak wrote:
> REGULATOR_STATUS_UNDEFINED is to be returned by regulator, if any other state
> doesn't really apply.

Applied, thanks.
Permalink | Reply |
headers
Mark Brown | 12 Jul 2012 19:18
Favicon
Gravatar

Re: [PATCH] regulator: Fix a typo in regulator_mode_to_status() core function.

On Thu, Jul 12, 2012 at 11:50:38AM +0100, Krystian Garbaciak wrote:
> Case REGULATOR_STATUS_STANDBY -> REGULATOR_MODE_STANDBY.
> 
> Signed-off-by: Krystian Garbaciak <krystian.garbaciak <at> diasemi.com>

This doesn't apply against -next (or the topic/core branch).  Please
check what's going on there.
Permalink | Reply |
headers
Mark Brown | 12 Jul 2012 19:20
Favicon
Gravatar

Re: [PATCH] regulator: Fix a typo in regulator_mode_to_status() core function.

On Thu, Jul 12, 2012 at 11:50:38AM +0100, Krystian Garbaciak wrote:
> Case REGULATOR_STATUS_STANDBY -> REGULATOR_MODE_STANDBY.

Gah, actually this was due to git am applying your two patches in the
wrong order - applied now, thanks.
Permalink | Reply |

Gmane