From: "Pallala, Ramakrishna" <ramakrishna.pallala@intel.com>
To: Krzysztof Kozlowski <k.kozlowski.k@gmail.com>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"linux-pm@vger.kernel.org" <linux-pm@vger.kernel.org>,
Sebastian Reichel <sre@kernel.org>
Cc: MyungJoo Ham <myungjoo.ham@samsung.com>
Subject: RE: [PATCH] power: max17042_battery: add HEALTH and TEMP_* properties support
Date: Mon, 4 May 2015 15:07:33 +0000 [thread overview]
Message-ID: <D854C92F57B1B347B57E531E78D05EAD578252A3@BGSMSX104.gar.corp.intel.com> (raw)
In-Reply-To: <55478533.9050109@gmail.com>
Hi,
> W dniu 05.05.2015 o 07:18, Ramakrishna Pallala pisze:
> > This patch adds the support for following battery properties to
> > max17042 fuel gauge driver.
>
> The patchset itself looks good. Only minor nits and a question at the end.
>
> >
> > POWER_SUPPLY_PROP_TEMP_ALERT_MIN
> > POWER_SUPPLY_PROP_TEMP_ALERT_MAX
> > POWER_SUPPLY_PROP_TEMP_MIN
> > POWER_SUPPLY_PROP_TEMP_MAX
> > POWER_SUPPLY_PROP_HEALTH
> >
> > Signed-off-by: Ramakrishna Pallala <ramakrishna.pallala@intel.com>
> > ---
> > drivers/power/max17042_battery.c | 190
> ++++++++++++++++++++++++++++++--
> > include/linux/power/max17042_battery.h | 4 +
> > 2 files changed, 183 insertions(+), 11 deletions(-)
> >
> > diff --git a/drivers/power/max17042_battery.c
> > b/drivers/power/max17042_battery.c
> > index 6cc5e87..d8f15ce 100644
> > --- a/drivers/power/max17042_battery.c
> > +++ b/drivers/power/max17042_battery.c
> > @@ -63,6 +63,8 @@
> > #define dP_ACC_100 0x1900
> > #define dP_ACC_200 0x3200
> >
> > +#define MAX17042_VMAX_TOLERENCE 50 /* 50 mV */
>
> s/TOLERENCE/TOLERANCE/
Ok..
[snip]
> > +
> > +static int max17042_get_battery_health(struct max17042_chip *chip,
> > +int *health) {
> > + int temp, vavg, vbatt, ret;
> > + u32 val;
> > +
> > + ret = regmap_read(chip->regmap, MAX17042_AvgVCELL, &val);
> > + if (ret < 0)
> > + goto health_error;
> > +
> > + /* bits [0-3] unused */
> > + vavg = val * 625 / 8;
> > + /* Convert to milli volts */
>
> s/milli volts/millivolts/
Ok..
[snip]
> > @@ -665,6 +829,8 @@ static const struct power_supply_desc
> max17042_psy_desc = {
> > .name = "max170xx_battery",
> > .type = POWER_SUPPLY_TYPE_BATTERY,
> > .get_property = max17042_get_property,
> > + .set_property = max17042_set_property,
> > + .property_is_writeable = max17042_property_is_writeable,
> > .properties = max17042_battery_props,
> > .num_properties = ARRAY_SIZE(max17042_battery_props),
> > };
> > @@ -673,6 +839,8 @@ static const struct power_supply_desc
> max17042_no_current_sense_psy_desc = {
> > .name = "max170xx_battery",
> > .type = POWER_SUPPLY_TYPE_BATTERY,
> > .get_property = max17042_get_property,
> > + .set_property = max17042_set_property,
> > + .property_is_writeable = max17042_property_is_writeable,
> > .properties = max17042_battery_props,
> > .num_properties = ARRAY_SIZE(max17042_battery_props) - 2,
> > };
> > diff --git a/include/linux/power/max17042_battery.h
> > b/include/linux/power/max17042_battery.h
> > index cf112b4..89ca4a8 100644
> > --- a/include/linux/power/max17042_battery.h
> > +++ b/include/linux/power/max17042_battery.h
> > @@ -215,6 +215,10 @@ struct max17042_platform_data {
> > * the datasheet although it can be changed by board designers.
> > */
> > unsigned int r_sns;
> > + int vmin; /* in milli volts */
> > + int vmax; /* in milli volts */
>
> s/milli volts/millivolts/
Ok..
> > + int temp_min; /* in tenths of degree Celsius */
> > + int temp_max; /* in tenths of degree Celsius */
> > };
> >
> > #endif /* __MAX17042_BATTERY_H_ */
>
> The question is who will set these values in pdata? We do not have board files
> anymore so I think you should extend the DT bindings so this could be used.
In our platform we don't use device tree...the enumeration is based on SFI model and
there is board file in our platform.
To complete the platforms which use device tree, I can add of_property_read_u32() calls for
new pdata variables and submit the patch.
Thanks,
Ram
next prev parent reply other threads:[~2015-05-04 15:07 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-05-04 22:18 [PATCH] power: max17042_battery: add HEALTH and TEMP_* properties support Ramakrishna Pallala
2015-05-04 14:41 ` Krzysztof Kozlowski
2015-05-04 15:07 ` Pallala, Ramakrishna [this message]
2015-05-04 15:12 ` Krzysztof Kozlowski
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=D854C92F57B1B347B57E531E78D05EAD578252A3@BGSMSX104.gar.corp.intel.com \
--to=ramakrishna.pallala@intel.com \
--cc=k.kozlowski.k@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pm@vger.kernel.org \
--cc=myungjoo.ham@samsung.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).