On 30.10.2014 13:43, Jonghwa Lee wrote:
> It drops the way of using power_supply interface to reference battery's
> temperature. Then it tries to use thermal subsystem's only. This makes driver
> more simple and also can remove ifdeferies.
> 
> Signed-off-by: Jonghwa Lee <[email protected]>
> ---
>  drivers/power/Kconfig                 |    1 +
>  drivers/power/charger-manager.c       |  113 
> ++++++++-------------------------
>  include/linux/power/charger-manager.h |    3 +-
>  3 files changed, 28 insertions(+), 89 deletions(-)
> 
> diff --git a/drivers/power/Kconfig b/drivers/power/Kconfig
> index 8ff2511..115d153 100644
> --- a/drivers/power/Kconfig
> +++ b/drivers/power/Kconfig
> @@ -317,6 +317,7 @@ config CHARGER_MANAGER
>       bool "Battery charger manager for multiple chargers"
>       depends on REGULATOR
>       select EXTCON
> +     select THERMAL

I think both of "select" here could be dangerous. Select should rather
be used for non-visible errors. Just use "depends on".

>       help
>            Say Y to enable charger-manager support, which allows multiple
>            chargers attached to a battery and multiple batteries attached to a
> diff --git a/drivers/power/charger-manager.c b/drivers/power/charger-manager.c
> index 22246b9..b4b101c 100644
> --- a/drivers/power/charger-manager.c
> +++ b/drivers/power/charger-manager.c
> @@ -28,13 +28,6 @@
>  #include <linux/of.h>
>  #include <linux/thermal.h>
>  
> -/*
> - * Default termperature threshold for charging.
> - * Every temperature units are in tenth of centigrade.
> - */
> -#define CM_DEFAULT_RECHARGE_TEMP_DIFF        50
> -#define CM_DEFAULT_CHARGE_TEMP_MAX   500
> -
>  static const char * const default_event_names[] = {
>       [CM_EVENT_UNKNOWN] = "Unknown",
>       [CM_EVENT_BATT_FULL] = "Battery Full",
> @@ -572,40 +565,18 @@ static int check_charging_duration(struct 
> charger_manager *cm)
>       return ret;
>  }
>  
> -static int cm_get_battery_temperature_by_psy(struct charger_manager *cm,
> -                                     int *temp)
> -{
> -     struct power_supply *fuel_gauge;
> -
> -     fuel_gauge = power_supply_get_by_name(cm->desc->psy_fuel_gauge);
> -     if (!fuel_gauge)
> -             return -ENODEV;
> -
> -     return fuel_gauge->get_property(fuel_gauge,
> -                             POWER_SUPPLY_PROP_TEMP,
> -                             (union power_supply_propval *)temp);
> -}
> -
>  static int cm_get_battery_temperature(struct charger_manager *cm,
>                                       int *temp)
>  {
>       int ret;
>  
> -     if (!cm->desc->measure_battery_temp)
> +     if (!cm->tzd_batt)
>               return -ENODEV;
>  
> -#ifdef CONFIG_THERMAL
> -     if (cm->tzd_batt) {
> -             ret = thermal_zone_get_temp(cm->tzd_batt, (unsigned long 
> *)temp);
> -             if (!ret)
> -                     /* Calibrate temperature unit */
> -                     *temp /= 100;
> -     } else
> -#endif
> -     {
> -             /* if-else continued from CONFIG_THERMAL */
> -             ret = cm_get_battery_temperature_by_psy(cm, temp);
> -     }
> +     ret = thermal_zone_get_temp(cm->tzd_batt, (unsigned long *)temp);
> +     if (!ret)
> +             /* Calibrate temperature unit */
> +             *temp /= 100;
>  
>       return ret;
>  }
> @@ -623,7 +594,7 @@ static int cm_check_thermal_status(struct charger_manager 
> *cm)
>                * occur hazadous result. We have to handle it
>                * depending on battery type.
>                */
> -             dev_err(cm->dev, "Failed to get battery temperature\n");
> +             dev_dbg(cm->dev, "Failed to get battery temperature\n");

A valuable change but not strictly related to the commit. Additionally
that is a user-visible change. Could you split it to separate patch?

Best regards,
Krzysztof

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to