Linux-OMAP Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH 1/5] power: supply: cpcap-charger: get the battery inserted infomation from cpcap-battery
@ 2021-01-17 21:41 Carl Philipp Klemm
  2021-04-05  8:20 ` Sebastian Reichel
  0 siblings, 1 reply; 2+ messages in thread
From: Carl Philipp Klemm @ 2021-01-17 21:41 UTC (permalink / raw)
  To: Sebastian Reichel
  Cc: linux-pm, linux-omap, Arthur Demchenkov, Tony Lindgren,
	Merlijn Wajer, Pavel Machek

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@uvos.xyz>
---
 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>
 
 /*
@@ -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");
+		if (IS_ERR_OR_NULL(battery)) {
+			dev_err(ddata->dev, "battery described by phandle not available %li\n",
+					PTR_ERR(battery));
+			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;
-- 
2.29.2



^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH 1/5] power: supply: cpcap-charger: get the battery inserted infomation from cpcap-battery
  2021-01-17 21:41 [PATCH 1/5] power: supply: cpcap-charger: get the battery inserted infomation from cpcap-battery Carl Philipp Klemm
@ 2021-04-05  8:20 ` Sebastian Reichel
  0 siblings, 0 replies; 2+ messages in thread
From: Sebastian Reichel @ 2021-04-05  8:20 UTC (permalink / raw)
  To: Carl Philipp Klemm
  Cc: linux-pm, linux-omap, Arthur Demchenkov, Tony Lindgren,
	Merlijn Wajer, Pavel Machek


[-- Attachment #1: Type: text/plain, Size: 3197 bytes --]

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@uvos.xyz>
> ---
>  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 #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, back to index

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-17 21:41 [PATCH 1/5] power: supply: cpcap-charger: get the battery inserted infomation from cpcap-battery Carl Philipp Klemm
2021-04-05  8:20 ` Sebastian Reichel

Linux-OMAP Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-omap/0 linux-omap/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-omap linux-omap/ https://lore.kernel.org/linux-omap \
		linux-omap@vger.kernel.org
	public-inbox-index linux-omap

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-omap


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git