From: Mark Brown <broonie@kernel.org>
To: Arnd Bergmann <arnd@arndb.de>
Cc: linux-arm-kernel@lists.infradead.org,
Liam Girdwood <lgirdwood@gmail.com>,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH] regulator: return NULL for dummy bulk_get operation
Date: Tue, 26 Jan 2016 12:04:32 +0000 [thread overview]
Message-ID: <20160126120432.GF6588@sirena.org.uk> (raw)
In-Reply-To: <1453737537-1960121-1-git-send-email-arnd@arndb.de>
[-- Attachment #1: Type: text/plain, Size: 1606 bytes --]
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.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
prev parent reply other threads:[~2016-01-26 12:04 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-01-25 15:58 Arnd Bergmann
2016-01-26 12:04 ` Mark Brown [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20160126120432.GF6588@sirena.org.uk \
--to=broonie@kernel.org \
--cc=arnd@arndb.de \
--cc=lgirdwood@gmail.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Powered by JetHome