[lkml]   [2012]   [Jan]   [25]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
Patch in this message
SubjectAdd NULL pointer checks to regulator_enable() et. al?

(Please CC me on replies).

I have this pattern coming up a LOT in my code:

struct regulator *reg;

/* probe() */
reg = regulator_get(dev, "VLOGIC");

/* elsewhere() */
if (!IS_ERR_OR_NULL(reg))
ret = regulator_enable(reg);

/* remove() */
if (!IS_ERR_OR_NULL(reg))

It comes up particularly often in drivers that are doing aggressive
runtime power management.

Unlike other kernel API, a NULL pointer from regulator_get() isn't a
true "error": it just means that there is no regulator associated with
the requested pin and device. It seems natural, therefore, for
regulator_enable() and friends to not consider a NULL regulator
pointer to be an error--- and definitely not something OOPS-worthy.
But if I forget even ONE check in the above code, that's exactly what
I get: an OOPS. But all those checks seem to be just tedium and noise
in this case.

Is there something truly objectionable in the following patch? If
not, then I'm happy to post a more complete one.

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index 5b2328d..b6e303a 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -1482,9 +1482,16 @@ static int _regulator_enable(struct regulator_dev *rdev)
int regulator_enable(struct regulator *regulator)
- struct regulator_dev *rdev = regulator->rdev;
+ struct regulator_dev *rdev;
int ret = 0;

+ if (unlikely(!regulator))
+ return 0;
+ if (unlikely(IS_ERR(regulator)))
+ return PTR_ERR(regulator);
+ rdev = regulator->rdev;

if (!regulator_check_voltage_update(rdev)) {

Note that I'm not arguing for such treatment all over the kernel---
just certain places in the regulator framework!

Kindest regards,

Bill Gatliff

 \ /
  Last update: 2012-01-25 08:41    [W:0.037 / U:7.396 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site