All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sebastian Reichel <sre@kernel.org>
To: Arnd Bergmann <arnd@arndb.de>
Cc: Phil Reid <preid@electromag.com.au>,
	YH Huang <yh.huang@mediatek.com>,
	Joshua Clayton <stillcompiling@gmail.com>,
	linux-pm@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] power: supply: sbs-battery: simplify DT parsing
Date: Wed, 7 Sep 2016 01:55:23 +0200	[thread overview]
Message-ID: <20160906235523.ff4opq56gg4ln6xv@earth> (raw)
In-Reply-To: <20160906131643.1930011-1-arnd@arndb.de>

[-- Attachment #1: Type: text/plain, Size: 1749 bytes --]

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

  reply	other threads:[~2016-09-06 23:55 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-09-06 13:16 [PATCH] power: supply: sbs-battery: simplify DT parsing Arnd Bergmann
2016-09-06 23:55 ` Sebastian Reichel [this message]
2016-09-07  7:58   ` Arnd Bergmann
2016-09-07 13:17     ` Sebastian Reichel
2016-09-19  9:20 ` Phil Reid

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=20160906235523.ff4opq56gg4ln6xv@earth \
    --to=sre@kernel.org \
    --cc=arnd@arndb.de \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=preid@electromag.com.au \
    --cc=stillcompiling@gmail.com \
    --cc=yh.huang@mediatek.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.