Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1477890
| From | Sebastian Reichel <sre@kernel.org> |
|---|---|
| Newsgroups | linux.kernel |
| Subject | Re: [PATCH] power: supply: sbs-battery: simplify DT parsing |
| Date | 2016-09-07 02:00 +0200 |
| Message-ID | <seyrv-oi-1@gated-at.bofh.it> (permalink) |
| References | <seos9-2uF-27@gated-at.bofh.it> |
| Organization | linux.* mail to news gateway |
[Multipart message — attachments visible in raw view] - view raw
Hi Arnd,
On Tue, Sep 06, 2016 at 03:16:33PM +0200, Arnd Bergmann wrote:
> After the change to use the gpio descriptor interface, we get a warning if
> -Wmaybe-uninitialized is added back to the build flags (it is currently
> disabled:
>
> drivers/power/supply/sbs-battery.c: In function 'sbs_probe':
> drivers/power/supply/sbs-battery.c:760:28: error: 'pdata' may be used uninitialized in this function [-Werror=maybe-uninitialized]
>
> The problem is that if neither the DT properties nor a platform_data
> pointer are provided, the chip->pdata pointer gets set to an uninitialized
> value.
>
> Looking at the code some more, I found that the sbs_of_populate_pdata
> function is more complex than necessary and has confusing calling
> conventions of possibly returning a valid pointer, a NULL pointer
> or an ERR_PTR pointer (in addition to the uninitialized pointer).
>
> To fix all of that, this gets rid of the chip->pdata pointer and
> simply moves the two integers into the sbs_info structure. This
> makes it much clearer from reading sbs_probe() what the precedence
> of the three possible values are (pdata, DT, hardcoded defaults)
> and completely avoids the #ifdef CONFIG_OF guards as
> of_property_read_u32() gets replaced with a compile-time stub
> when that is disabled, and returns an error if passed a NULL of_node
> pointer.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> Fixes: 3b5dd3a49496 ("power: supply: sbs-battery: Use gpio_desc and sleeping calls for battery detect")
Patch looks fine to me. Actually I already asked Phil to
implement your change [0]. I just queued it to power-supply's
for-next branch.
[0] https://marc.info/?l=linux-pm&m=147273382018953&w=2
-- Sebastian
Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread
[PATCH] power: supply: sbs-battery: simplify DT parsing Arnd Bergmann <arnd@arndb.de> - 2016-09-06 15:20 +0200
Re: [PATCH] power: supply: sbs-battery: simplify DT parsing Sebastian Reichel <sre@kernel.org> - 2016-09-07 02:00 +0200
Re: [PATCH] power: supply: sbs-battery: simplify DT parsing Arnd Bergmann <arnd@arndb.de> - 2016-09-07 10:00 +0200
Re: [PATCH] power: supply: sbs-battery: simplify DT parsing Sebastian Reichel <sre@kernel.org> - 2016-09-07 15:20 +0200
csiph-web