Hi,
On Sun, Jan 17, 2021 at 10:41:58PM +0100, Carl Philipp Klemm wrote:
> This avoids reimplementing the detection logic twice and removes the
> possibility of activating charging with 500mA even if a battery is not
> detected.
>
> Signed-off-by: Carl Philipp Klemm <philipp@xxxxxxxx>
> ---
> drivers/power/supply/cpcap-charger.c | 41 ++++++++++++++--------------
> 1 file changed, 21 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/power/supply/cpcap-charger.c b/drivers/power/supply/cpcap-charger.c
> index 823d666f09e0..152090faf5b2 100644
> --- a/drivers/power/supply/cpcap-charger.c
> +++ b/drivers/power/supply/cpcap-charger.c
> @@ -26,8 +26,8 @@
> #include <linux/gpio/consumer.h>
> #include <linux/usb/phy_companion.h>
> #include <linux/phy/omap_usb.h>
> -#include <linux/usb/otg.h>
> #include <linux/iio/consumer.h>
> +#include <linux/usb/otg.h>
> #include <linux/mfd/motorola-cpcap.h>
why?
>
> /*
> @@ -173,23 +173,6 @@ static enum power_supply_property cpcap_charger_props[] = {
> POWER_SUPPLY_PROP_CURRENT_NOW,
> };
>
> -/* No battery always shows temperature of -40000 */
> -static bool cpcap_charger_battery_found(struct cpcap_charger_ddata *ddata)
> -{
> - struct iio_channel *channel;
> - int error, temperature;
> -
> - channel = ddata->channels[CPCAP_CHARGER_IIO_BATTDET];
> - error = iio_read_channel_processed(channel, &temperature);
> - if (error < 0) {
> - dev_warn(ddata->dev, "%s failed: %i\n", __func__, error);
> -
> - return false;
> - }
> -
> - return temperature > -20000 && temperature < 60000;
> -}
> -
> static int cpcap_charger_get_charge_voltage(struct cpcap_charger_ddata *ddata)
> {
> struct iio_channel *channel;
> @@ -696,11 +679,29 @@ static void cpcap_usb_detect(struct work_struct *work)
>
> if (!ddata->feeding_vbus && cpcap_charger_vbus_valid(ddata) &&
> s.chrgcurr1) {
> - int max_current = 532000;
> + int max_current;
> int vchrg, ichrg;
> + union power_supply_propval val;
> + struct power_supply *battery;
>
> - if (cpcap_charger_battery_found(ddata))
> + battery = power_supply_get_by_phandle(ddata->dev->of_node, "battery");
Let's not add DT properties, that are not really required. For now
you can just use power_supply_get_by_name("battery"), which should
always return the cpcap-battery on cpcap based systems. In the
future the power-supply core framework will hopefully provide a
function to get the devices supplied by a charger.
> + if (IS_ERR_OR_NULL(battery)) {
> + dev_err(ddata->dev, "battery described by phandle not available %li\n",
> + PTR_ERR(battery));
Don't forget to update the error message.
> + return;
> + }
> +
> + error = power_supply_get_property(battery, POWER_SUPPLY_PROP_PRESENT, &val);
> + power_supply_put(battery);
> + if (error)
> + goto out_err;
> +
> + if (val.intval) {
> max_current = 1596000;
> + } else {
> + dev_info(ddata->dev, "battery not inserted charging disabled\n");
> + max_current = 0;
> + }
>
> if (max_current > ddata->limit_current)
> max_current = ddata->limit_current;
Otherwise LGTM.
-- Sebastian
Attachment:
signature.asc
Description: PGP signature
