Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1316887 > unrolled thread

[PATCH] regulator: return NULL for dummy bulk_get operation

Back to article view | Back to linux.kernel

Contents

  [PATCH] regulator: return NULL for dummy bulk_get operation Arnd Bergmann <arnd@arndb.de> - 2016-01-25 17:00 +0100
    Re: [PATCH] regulator: return NULL for dummy bulk_get operation Mark Brown <broonie@kernel.org> - 2016-01-26 13:10 +0100

#1316887 — [PATCH] regulator: return NULL for dummy bulk_get operation

Drivers that call regulator_bulk_get or devm_regulator_bulk_get when
CONFIG_REGULATOR is disabled see the function return successfully, but
cannot use the "consumer" pointers that are meant to be returned from
the functions, as the values are uninitialized. Gcc warns about this:

drivers/usb/phy/phy-qcom-8x16-usb.c: In function 'phy_8x16_probe':
drivers/usb/phy/phy-qcom-8x16-usb.c:284:13: error: 'regs[0].consumer' may be used uninitialized in this function [-Werror=maybe-uninitialized]
drivers/usb/phy/phy-qcom-8x16-usb.c:285:13: error: 'regs[1].consumer' may be used uninitialized in this function [-Werror=maybe-uninitialized]
drivers/usb/phy/phy-qcom-8x16-usb.c:286:12: error: 'regs[2].consumer' may be used uninitialized in this function [-Werror=maybe-uninitialized]

To make the code behave in a reproducible way, this changes the
two dummy function to behave like the non-bulk regulator_get
and devm_regulator_get functions to, returning a NULL pointer with
a successful return code. This also gets rid of the warnings.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 include/linux/regulator/consumer.h | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/include/linux/regulator/consumer.h b/include/linux/regulator/consumer.h
index 48603506f8de..f4774bf707c2 100644
--- a/include/linux/regulator/consumer.h
+++ b/include/linux/regulator/consumer.h
@@ -404,13 +404,17 @@ static inline int regulator_bulk_get(struct device *dev,
 				     int num_consumers,
 				     struct regulator_bulk_data *consumers)
 {
+	int i;
+	for (i=0; i < num_consumers; i++)
+		consumers[i].consumer = NULL;
+
 	return 0;
 }
 
 static inline int devm_regulator_bulk_get(struct device *dev, int num_consumers,
 					  struct regulator_bulk_data *consumers)
 {
-	return 0;
+	return regulator_bulk_get(dev, num_consumers, consumers);
 }
 
 static inline int regulator_bulk_enable(int num_consumers,
-- 
2.7.0

[toc] | [next] | [standalone]

#1317793

[Multipart message — attachments visible in raw view] — view raw

On Mon, Jan 25, 2016 at 04:58:50PM +0100, Arnd Bergmann wrote:
> Drivers that call regulator_bulk_get or devm_regulator_bulk_get when
> CONFIG_REGULATOR is disabled see the function return successfully, but
> cannot use the "consumer" pointers that are meant to be returned from
> the functions, as the values are uninitialized. Gcc warns about this:

> drivers/usb/phy/phy-qcom-8x16-usb.c: In function 'phy_8x16_probe':
> drivers/usb/phy/phy-qcom-8x16-usb.c:284:13: error: 'regs[0].consumer' may be used uninitialized in this function [-Werror=maybe-uninitialized]
> drivers/usb/phy/phy-qcom-8x16-usb.c:285:13: error: 'regs[1].consumer' may be used uninitialized in this function [-Werror=maybe-uninitialized]
> drivers/usb/phy/phy-qcom-8x16-usb.c:286:12: error: 'regs[2].consumer' may be used uninitialized in this function [-Werror=maybe-uninitialized]

The usage in that driver looks a bit dodgy - the disable function is
just an open coding of regulator_bulk_disable() with no error checking
while the enable function is doing a series of set_voltage() calls on
every enable which look a lot like values that I'd expect to come from
constraints rather than being set explicitly (the VDD values are an API
abuse we know about in the Qualcomm drivers).  The fact that it never
varies the voltage is a warning sign that the driver might not want to
be using set_voltage() at all, and it should at least do that once on
init if there's a use case.  A quick glance at in tree DTs suggests that
there's no actual runtime variation.

I'm not 100% convinced this didn't actually warn us about a real
problem.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web