Re: [EXTERNAL] Re: [PATCH 2/2] power: supply: bq25980: Moves properties from battery node

[Ricardo Rivera-Matos]

On 2/10/21 2:23 AM, Krzysztof Kozlowski wrote:

On Wed, 10 Feb 2021 at 00:52, Ricardo Rivera-Matos
 wrote:

fix: exposes POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT on the

charger node

Why is this a fix? Why is exposing this property wrong? What is the
problem here? Why do you start sentences with a small letter? Your
commit message should answer such questions.

ACK, I will rebase and reword this commit message


Best regards,
Krzysztof

Best Regards,
Ricardo

Re: [PATCH 2/2] power: supply: bq25980: Moves properties from battery node

[Krzysztof Kozlowski]

On Wed, 10 Feb 2021 at 00:52, Ricardo Rivera-Matos
 wrote:
>
> fix: exposes POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT on the
>
> charger node

Why is this a fix? Why is exposing this property wrong? What is the
problem here? Why do you start sentences with a small letter? Your
commit message should answer such questions.

Best regards,
Krzysztof

[PATCH 2/2] power: supply: bq25980: Moves properties from battery node

[Ricardo Rivera-Matos]

fix: exposes POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT on the

charger node

fix: exposes POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE on the

charger node

fix: eliminates unnecessary set_property for the battery node

Fixes: 5069185fc18e ("power: supply: bq25980: Add support for the BQ259xx 
family")
Signed-off-by: Ricardo Rivera-Matos 
---
 drivers/power/supply/bq25980_charger.c | 40 --
 1 file changed, 12 insertions(+), 28 deletions(-)

diff --git a/drivers/power/supply/bq25980_charger.c 
b/drivers/power/supply/bq25980_charger.c
index 7c489a9e8877..ac73e2c19238 100644
--- a/drivers/power/supply/bq25980_charger.c
+++ b/drivers/power/supply/bq25980_charger.c
@@ -641,33 +641,6 @@ static int bq25980_get_state(struct bq25980_device *bq,
return 0;
 }
 
-static int bq25980_set_battery_property(struct power_supply *psy,
-   enum power_supply_property psp,
-   const union power_supply_propval *val)
-{
-   struct bq25980_device *bq = power_supply_get_drvdata(psy);
-   int ret = 0;
-
-   switch (psp) {
-   case POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT:
-   ret = bq25980_set_const_charge_curr(bq, val->intval);
-   if (ret)
-   return ret;
-   break;
-
-   case POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE:
-   ret = bq25980_set_const_charge_volt(bq, val->intval);
-   if (ret)
-   return ret;
-   break;
-
-   default:
-   return -EINVAL;
-   }
-
-   return ret;
-}
-
 static int bq25980_get_battery_property(struct power_supply *psy,
enum power_supply_property psp,
union power_supply_propval *val)
@@ -736,6 +709,18 @@ static int bq25980_set_charger_property(struct 
power_supply *psy,
return ret;
break;
 
+   case POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT:
+   ret = bq25980_set_const_charge_curr(bq, val->intval);
+   if (ret)
+   return ret;
+   break;
+
+   case POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE:
+   ret = bq25980_set_const_charge_volt(bq, val->intval);
+   if (ret)
+   return ret;
+   break;
+
default:
return -EINVAL;
}
@@ -957,7 +942,6 @@ static struct power_supply_desc bq25980_battery_desc = {
.name   = "bq25980-battery",
.type   = POWER_SUPPLY_TYPE_BATTERY,
.get_property   = bq25980_get_battery_property,
-   .set_property   = bq25980_set_battery_property,
.properties = bq25980_battery_props,
.num_properties = ARRAY_SIZE(bq25980_battery_props),
.property_is_writeable  = bq25980_property_is_writeable,
-- 
2.30.0