From mboxrd@z Thu Jan 1 00:00:00 1970 From: Krzysztof Kozlowski Subject: Re: [PATCH v4 07/10] power: bq24257: Add input DPM voltage threshold setting support Date: Wed, 16 Sep 2015 15:06:04 +0900 Message-ID: <55F906CC.8090609@samsung.com> References: <1442339914-25843-1-git-send-email-dannenberg@ti.com> <1442339914-25843-8-git-send-email-dannenberg@ti.com> Mime-Version: 1.0 Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Return-path: In-reply-to: <1442339914-25843-8-git-send-email-dannenberg@ti.com> Sender: linux-pm-owner@vger.kernel.org To: Andreas Dannenberg , Sebastian Reichel , Dmitry Eremin-Solenikov , David Woodhouse , Laurentiu Palcu Cc: Ramakrishna Pallala , linux-pm@vger.kernel.org, devicetree@vger.kernel.org List-Id: devicetree@vger.kernel.org On 16.09.2015 02:58, Andreas Dannenberg wrote: > A new optional device property called "ti,in-dpm-voltage" is introduced > to allow configuring the input voltage threshold for the devices' > dynamic power path management (DPM) feature. In short, it can be used to > prevent the input voltage from dropping below a certain value as current > is drawn to charge the battery or supply the system. > > Signed-off-by: Andreas Dannenberg > --- > drivers/power/bq24257_charger.c | 44 +++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 44 insertions(+) > > diff --git a/drivers/power/bq24257_charger.c b/drivers/power/bq24257_charger.c > index 47af858..84a8945 100644 > --- a/drivers/power/bq24257_charger.c > +++ b/drivers/power/bq24257_charger.c > @@ -76,6 +76,7 @@ struct bq24257_init_data { > u8 iterm; /* termination current */ > u8 in_ilimit; /* input current limit */ > u8 vovp; /* over voltage protection voltage */ > + u8 vindpm; /* VDMP input threshold voltage */ > }; > > struct bq24257_state { > @@ -206,6 +207,13 @@ static const u32 bq24257_vovp_map[] = { > > #define BQ24257_VOVP_MAP_SIZE ARRAY_SIZE(bq24257_vovp_map) > > +static const u32 bq24257_vindpm_map[] = { > + 4200000, 4280000, 4360000, 4440000, 4520000, 4600000, 4680000, > + 4760000 > +}; > + > +#define BQ24257_VINDPM_MAP_SIZE ARRAY_SIZE(bq24257_vindpm_map) > + > static int bq24257_field_read(struct bq24257_device *bq, > enum bq24257_fields field_id) > { > @@ -432,6 +440,17 @@ enum bq24257_vovp { > VOVP_10500 > }; > > +enum bq24257_vindpm { > + VINDPM_4200, > + VINDPM_4280, > + VINDPM_4360, > + VINDPM_4440, > + VINDPM_4520, > + VINDPM_4600, > + VINDPM_4680, > + VINDPM_4760 > +}; Same as patch 6/10... but now see the use case - to choose default value not by index of array but from this enum. I see the idea behind so skip that comment from 6/10. > + > enum bq24257_port_type { > PORT_TYPE_DCP, /* Dedicated Charging Port */ > PORT_TYPE_CDP, /* Charging Downstream Port */ > @@ -615,6 +634,7 @@ static int bq24257_hw_init(struct bq24257_device *bq) > {F_VBAT, bq->init_data.vbat}, > {F_ITERM, bq->init_data.iterm}, > {F_VOVP, bq->init_data.vovp}, > + {F_VINDPM, bq->init_data.vindpm}, > }; > > /* > @@ -648,6 +668,7 @@ static int bq24257_hw_init(struct bq24257_device *bq) > /* program fixed input current limit */ > ret = bq24257_field_write(bq, F_IILIMIT, > bq->init_data.in_ilimit); > + Not part of this patch. It confuses the reviewer of patch. If you need cleanups, make them separate. > if (ret < 0) > return ret; > } else if (!state.power_good) > @@ -695,10 +716,23 @@ static ssize_t bq24257_show_ovp_voltage(struct device *dev, > bq24257_vovp_map[bq->init_data.vovp]); > } > > +static ssize_t bq24257_show_in_dpm_voltage(struct device *dev, > + struct device_attribute *attr, > + char *buf) > +{ > + struct power_supply *psy = dev_get_drvdata(dev); > + struct bq24257_device *bq = power_supply_get_drvdata(psy); > + > + return scnprintf(buf, PAGE_SIZE, "%d\n", > + bq24257_vindpm_map[bq->init_data.vindpm]); Same as patch 6/10. Best regards, Krzysztof > +} > + > static DEVICE_ATTR(ovp_voltage, S_IRUGO, bq24257_show_ovp_voltage, NULL); > +static DEVICE_ATTR(in_dpm_voltage, S_IRUGO, bq24257_show_in_dpm_voltage, NULL); > > static struct attribute *bq24257_charger_attr[] = { > &dev_attr_ovp_voltage.attr, > + &dev_attr_in_dpm_voltage.attr, > NULL, > }; > > @@ -801,6 +835,16 @@ static int bq24257_fw_probe(struct bq24257_device *bq) > bq24257_vovp_map, > BQ24257_VOVP_MAP_SIZE); > > + ret = device_property_read_u32(bq->dev, "ti,in-dpm-voltage", > + &property); > + if (ret < 0) > + bq->init_data.vindpm = VINDPM_4360; > + else > + bq->init_data.vindpm = > + bq24257_find_idx(property, > + bq24257_vindpm_map, > + BQ24257_VINDPM_MAP_SIZE); > + > return 0; > } > >