All of lore.kernel.org
 help / color / mirror / Atom feed
From: Pavel Machek <pavel@ucw.cz>
To: Krzysztof Kozlowski <k.kozlowski@samsung.com>
Cc: Linus Walleij <linus.walleij@linaro.org>,
	Samuel Ortiz <sameo@linux.intel.com>,
	Lee Jones <lee.jones@linaro.org>,
	Sebastian Reichel <sre@kernel.org>,
	Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>,
	David Woodhouse <dwmw2@infradead.org>,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, linux-pm@vger.kernel.org,
	Anton Vorontsov <anton@enomsg.org>,
	Guenter Roeck <linux@roeck-us.net>,
	Myungjoo Ham <myungjoo.ham@samsung.com>,
	Jonghwa Lee <jonghwa3.lee@samsung.com>,
	Kyungmin Park <kyungmin.park@samsung.com>,
	Marek Szyprowski <m.szyprowski@samsung.com>,
	Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
Subject: Re: [PATCH 2/8] power_supply: sysfs: Use power_supply_*() API for accessing function attrs
Date: Wed, 15 Oct 2014 14:19:29 +0200	[thread overview]
Message-ID: <20141015121929.GB26579@amd> (raw)
In-Reply-To: <1413370199.26771.4.camel@AMDC1943>


> > > @@ -76,7 +76,7 @@ static ssize_t power_supply_show_property(struct device *dev,
> > >  	if (off == POWER_SUPPLY_PROP_TYPE) {
> > >  		value.intval = psy->type;
> > >  	} else {
> > > -		ret = psy->get_property(psy, off, &value);
> > > +		ret = power_supply_get_property(psy, off, &value);
> > >  
> > 
> > One thing.. Your power_supply_get_property (and friends) check for psy
> > == NULL. Is it neccessary / good idea? As far as I can tell, it should
> > not really be NULL...
> 
> It is not necessary. I thought it would be a good behavior of such
> exported function. You're right that this shouldn't be NULL especially
> because previously it was dereferenced.
> 
> Other existing power supply exported functions don't check this so maybe
> I shouldn't introduce inconsistency.
> 
> I'll remove the check and re-spin. I'll found ugly typos in commit
> message anyway.

Thanks!
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

WARNING: multiple messages have this Message-ID (diff)
From: pavel@ucw.cz (Pavel Machek)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 2/8] power_supply: sysfs: Use power_supply_*() API for accessing function attrs
Date: Wed, 15 Oct 2014 14:19:29 +0200	[thread overview]
Message-ID: <20141015121929.GB26579@amd> (raw)
In-Reply-To: <1413370199.26771.4.camel@AMDC1943>


> > > @@ -76,7 +76,7 @@ static ssize_t power_supply_show_property(struct device *dev,
> > >  	if (off == POWER_SUPPLY_PROP_TYPE) {
> > >  		value.intval = psy->type;
> > >  	} else {
> > > -		ret = psy->get_property(psy, off, &value);
> > > +		ret = power_supply_get_property(psy, off, &value);
> > >  
> > 
> > One thing.. Your power_supply_get_property (and friends) check for psy
> > == NULL. Is it neccessary / good idea? As far as I can tell, it should
> > not really be NULL...
> 
> It is not necessary. I thought it would be a good behavior of such
> exported function. You're right that this shouldn't be NULL especially
> because previously it was dereferenced.
> 
> Other existing power supply exported functions don't check this so maybe
> I shouldn't introduce inconsistency.
> 
> I'll remove the check and re-spin. I'll found ugly typos in commit
> message anyway.

Thanks!
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

  reply	other threads:[~2014-10-15 12:19 UTC|newest]

Thread overview: 39+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-10-14 12:20 [PATCH 0/8] power_supply: Add API for safe access of get_property-like function attrs Krzysztof Kozlowski
2014-10-14 12:20 ` Krzysztof Kozlowski
2014-10-14 12:20 ` [PATCH 1/8] power_supply: Add API for safe access of power supply " Krzysztof Kozlowski
2014-10-14 12:20   ` Krzysztof Kozlowski
2014-10-15 10:30   ` Pavel Machek
2014-10-15 10:30     ` Pavel Machek
2014-10-14 12:20 ` [PATCH 2/8] power_supply: sysfs: Use power_supply_*() API for accessing " Krzysztof Kozlowski
2014-10-14 12:20   ` Krzysztof Kozlowski
2014-10-14 12:20   ` Krzysztof Kozlowski
2014-10-15 10:32   ` Pavel Machek
2014-10-15 10:32     ` Pavel Machek
2014-10-15 10:49     ` Krzysztof Kozlowski
2014-10-15 10:49       ` Krzysztof Kozlowski
2014-10-15 12:19       ` Pavel Machek [this message]
2014-10-15 12:19         ` Pavel Machek
2014-10-14 12:20 ` [PATCH 3/8] power: 88pm860x_charger: " Krzysztof Kozlowski
2014-10-14 12:20   ` Krzysztof Kozlowski
2014-10-14 12:20   ` Krzysztof Kozlowski
2014-10-15 10:33   ` Pavel Machek
2014-10-15 10:33     ` Pavel Machek
2014-10-14 12:20 ` [PATCH 4/8] power: ab8500: " Krzysztof Kozlowski
2014-10-14 12:20   ` Krzysztof Kozlowski
2014-10-28  9:58   ` Linus Walleij
2014-10-28  9:58     ` Linus Walleij
2014-10-14 12:20 ` [PATCH 5/8] mfd: " Krzysztof Kozlowski
2014-10-14 12:20   ` Krzysztof Kozlowski
2014-10-14 12:20   ` Krzysztof Kozlowski
2014-10-15  9:07   ` Lee Jones
2014-10-15  9:07     ` Lee Jones
2014-10-14 12:20 ` [PATCH 6/8] power: apm_power: " Krzysztof Kozlowski
2014-10-14 12:20   ` Krzysztof Kozlowski
2014-10-14 12:20 ` [PATCH 7/8] power: bq2415x_charger: " Krzysztof Kozlowski
2014-10-14 12:20   ` Krzysztof Kozlowski
2014-10-14 12:20 ` [PATCH 8/8] power: charger-manager: " Krzysztof Kozlowski
2014-10-14 12:20   ` Krzysztof Kozlowski
2014-10-15  2:22 ` [PATCH 0/8] power_supply: Add API for safe access of get_property-like " jonghwa3.lee
2014-10-15  2:22   ` jonghwa3.lee at samsung.com
2014-10-15 10:35 ` Pavel Machek
2014-10-15 10:35   ` Pavel Machek

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=20141015121929.GB26579@amd \
    --to=pavel@ucw.cz \
    --cc=anton@enomsg.org \
    --cc=b.zolnierkie@samsung.com \
    --cc=dbaryshkov@gmail.com \
    --cc=dwmw2@infradead.org \
    --cc=jonghwa3.lee@samsung.com \
    --cc=k.kozlowski@samsung.com \
    --cc=kyungmin.park@samsung.com \
    --cc=lee.jones@linaro.org \
    --cc=linus.walleij@linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=linux@roeck-us.net \
    --cc=m.szyprowski@samsung.com \
    --cc=myungjoo.ham@samsung.com \
    --cc=sameo@linux.intel.com \
    --cc=sre@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 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.