All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] power: supply: Fix static batteries
@ 2022-03-01  0:30 Linus Walleij
  2022-03-04 21:22 ` Sebastian Reichel
  0 siblings, 1 reply; 2+ messages in thread
From: Linus Walleij @ 2022-03-01  0:30 UTC (permalink / raw)
  To: Sebastian Reichel, Marcus Cooper; +Cc: linux-pm, Matti Vaittinen, Linus Walleij

The code for looking up static battery data was erroneously
rebased and assumed we were only using device tree, but we have
added fwnode support.

Augment the code properly to just pass in a compatible as a
string when looking up a static Samsung battery, also move
the fwnode parsing code up so we can still check this before
we move on to simple-battery and start allocating stuff.

Fixes: ffb983d31519 ("power: supply: Static data for Samsung batteries")
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/power/supply/power_supply_core.c   | 63 +++++++++++-----------
 drivers/power/supply/samsung-sdi-battery.c |  5 +-
 drivers/power/supply/samsung-sdi-battery.h |  4 +-
 3 files changed, 36 insertions(+), 36 deletions(-)

diff --git a/drivers/power/supply/power_supply_core.c b/drivers/power/supply/power_supply_core.c
index 44816d41b4ec..ea02c8dcd748 100644
--- a/drivers/power/supply/power_supply_core.c
+++ b/drivers/power/supply/power_supply_core.c
@@ -579,16 +579,42 @@ int power_supply_get_battery_info(struct power_supply *psy,
 	const __be32 *list;
 	u32 min_max[2];
 
+	if (psy->of_node) {
+		battery_np = of_parse_phandle(psy->of_node, "monitored-battery", 0);
+		if (!battery_np)
+			return -ENODEV;
+
+		fwnode = fwnode_handle_get(of_fwnode_handle(battery_np));
+	} else {
+		err = fwnode_property_get_reference_args(
+					dev_fwnode(psy->dev.parent),
+					"monitored-battery", NULL, 0, 0, &args);
+		if (err)
+			return err;
+
+		fwnode = args.fwnode;
+	}
+
+	err = fwnode_property_read_string(fwnode, "compatible", &value);
+	if (err)
+		goto out_put_node;
+
+
 	/* Try static batteries first */
-	err = samsung_sdi_battery_get_info(&psy->dev, battery_np, &info);
-	if (!err) {
-		*info_out = info;
-		return err;
+	err = samsung_sdi_battery_get_info(&psy->dev, value, &info);
+	if (!err)
+		goto out_ret_pointer;
+
+	if (strcmp("simple-battery", value)) {
+		err = -ENODEV;
+		goto out_put_node;
 	}
 
 	info = devm_kmalloc(&psy->dev, sizeof(*info), GFP_KERNEL);
-	if (!info)
-		return -ENOMEM;
+	if (!info) {
+		err = -ENOMEM;
+		goto out_put_node;
+	}
 
 	info->technology                     = POWER_SUPPLY_TECHNOLOGY_UNKNOWN;
 	info->energy_full_design_uwh         = -EINVAL;
@@ -625,31 +651,6 @@ int power_supply_get_battery_info(struct power_supply *psy,
 		info->ocv_table_size[index]  = -EINVAL;
 	}
 
-	if (psy->of_node) {
-		battery_np = of_parse_phandle(psy->of_node, "monitored-battery", 0);
-		if (!battery_np)
-			return -ENODEV;
-
-		fwnode = fwnode_handle_get(of_fwnode_handle(battery_np));
-	} else {
-		err = fwnode_property_get_reference_args(
-					dev_fwnode(psy->dev.parent),
-					"monitored-battery", NULL, 0, 0, &args);
-		if (err)
-			return err;
-
-		fwnode = args.fwnode;
-	}
-
-	err = fwnode_property_read_string(fwnode, "compatible", &value);
-	if (err)
-		goto out_put_node;
-
-	if (strcmp("simple-battery", value)) {
-		err = -ENODEV;
-		goto out_put_node;
-	}
-
 	/* The property and field names below must correspond to elements
 	 * in enum power_supply_property. For reasoning, see
 	 * Documentation/power/power_supply_class.rst.
diff --git a/drivers/power/supply/samsung-sdi-battery.c b/drivers/power/supply/samsung-sdi-battery.c
index 64ec34a41c5b..9d59f277f519 100644
--- a/drivers/power/supply/samsung-sdi-battery.c
+++ b/drivers/power/supply/samsung-sdi-battery.c
@@ -10,7 +10,6 @@
  * the BTI resistance with a multimeter on the battery.
  */
 #include <linux/module.h>
-#include <linux/of.h>
 #include <linux/power_supply.h>
 #include "samsung-sdi-battery.h"
 
@@ -895,7 +894,7 @@ static struct samsung_sdi_battery samsung_sdi_batteries[] = {
 };
 
 int samsung_sdi_battery_get_info(struct device *dev,
-				 struct device_node *np,
+				 const char *compatible,
 				 struct power_supply_battery_info **info)
 {
 	struct samsung_sdi_battery *batt;
@@ -903,7 +902,7 @@ int samsung_sdi_battery_get_info(struct device *dev,
 
 	for (i = 0; i < ARRAY_SIZE(samsung_sdi_batteries); i++) {
 		batt = &samsung_sdi_batteries[i];
-		if (of_device_is_compatible(np, batt->compatible))
+		if (!strcmp(compatible, batt->compatible))
 			break;
 	}
 
diff --git a/drivers/power/supply/samsung-sdi-battery.h b/drivers/power/supply/samsung-sdi-battery.h
index 08783847dfcb..365ab6e85b26 100644
--- a/drivers/power/supply/samsung-sdi-battery.h
+++ b/drivers/power/supply/samsung-sdi-battery.h
@@ -1,11 +1,11 @@
 /* SPDX-License-Identifier: GPL-2.0 */
 #if IS_ENABLED(CONFIG_BATTERY_SAMSUNG_SDI)
 extern int samsung_sdi_battery_get_info(struct device *dev,
-				struct device_node *np,
+				const char *compatible,
 				struct power_supply_battery_info **info);
 #else
 static inline int samsung_sdi_battery_get_info(struct device *dev,
-				struct device_node *np,
+				const char *compatible,
 				struct power_supply_battery_info **info)
 {
 	return -ENODEV;
-- 
2.34.1


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

* Re: [PATCH] power: supply: Fix static batteries
  2022-03-01  0:30 [PATCH] power: supply: Fix static batteries Linus Walleij
@ 2022-03-04 21:22 ` Sebastian Reichel
  0 siblings, 0 replies; 2+ messages in thread
From: Sebastian Reichel @ 2022-03-04 21:22 UTC (permalink / raw)
  To: Linus Walleij; +Cc: Marcus Cooper, linux-pm, Matti Vaittinen

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

Hi,

On Tue, Mar 01, 2022 at 01:30:20AM +0100, Linus Walleij wrote:
> The code for looking up static battery data was erroneously
> rebased and assumed we were only using device tree, but we have
> added fwnode support.
> 
> Augment the code properly to just pass in a compatible as a
> string when looking up a static Samsung battery, also move
> the fwnode parsing code up so we can still check this before
> we move on to simple-battery and start allocating stuff.
> 
> Fixes: ffb983d31519 ("power: supply: Static data for Samsung batteries")
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---

Thanks, I squashed this into the previous commit.

-- Sebastian

>  drivers/power/supply/power_supply_core.c   | 63 +++++++++++-----------
>  drivers/power/supply/samsung-sdi-battery.c |  5 +-
>  drivers/power/supply/samsung-sdi-battery.h |  4 +-
>  3 files changed, 36 insertions(+), 36 deletions(-)
> 
> diff --git a/drivers/power/supply/power_supply_core.c b/drivers/power/supply/power_supply_core.c
> index 44816d41b4ec..ea02c8dcd748 100644
> --- a/drivers/power/supply/power_supply_core.c
> +++ b/drivers/power/supply/power_supply_core.c
> @@ -579,16 +579,42 @@ int power_supply_get_battery_info(struct power_supply *psy,
>  	const __be32 *list;
>  	u32 min_max[2];
>  
> +	if (psy->of_node) {
> +		battery_np = of_parse_phandle(psy->of_node, "monitored-battery", 0);
> +		if (!battery_np)
> +			return -ENODEV;
> +
> +		fwnode = fwnode_handle_get(of_fwnode_handle(battery_np));
> +	} else {
> +		err = fwnode_property_get_reference_args(
> +					dev_fwnode(psy->dev.parent),
> +					"monitored-battery", NULL, 0, 0, &args);
> +		if (err)
> +			return err;
> +
> +		fwnode = args.fwnode;
> +	}
> +
> +	err = fwnode_property_read_string(fwnode, "compatible", &value);
> +	if (err)
> +		goto out_put_node;
> +
> +
>  	/* Try static batteries first */
> -	err = samsung_sdi_battery_get_info(&psy->dev, battery_np, &info);
> -	if (!err) {
> -		*info_out = info;
> -		return err;
> +	err = samsung_sdi_battery_get_info(&psy->dev, value, &info);
> +	if (!err)
> +		goto out_ret_pointer;
> +
> +	if (strcmp("simple-battery", value)) {
> +		err = -ENODEV;
> +		goto out_put_node;
>  	}
>  
>  	info = devm_kmalloc(&psy->dev, sizeof(*info), GFP_KERNEL);
> -	if (!info)
> -		return -ENOMEM;
> +	if (!info) {
> +		err = -ENOMEM;
> +		goto out_put_node;
> +	}
>  
>  	info->technology                     = POWER_SUPPLY_TECHNOLOGY_UNKNOWN;
>  	info->energy_full_design_uwh         = -EINVAL;
> @@ -625,31 +651,6 @@ int power_supply_get_battery_info(struct power_supply *psy,
>  		info->ocv_table_size[index]  = -EINVAL;
>  	}
>  
> -	if (psy->of_node) {
> -		battery_np = of_parse_phandle(psy->of_node, "monitored-battery", 0);
> -		if (!battery_np)
> -			return -ENODEV;
> -
> -		fwnode = fwnode_handle_get(of_fwnode_handle(battery_np));
> -	} else {
> -		err = fwnode_property_get_reference_args(
> -					dev_fwnode(psy->dev.parent),
> -					"monitored-battery", NULL, 0, 0, &args);
> -		if (err)
> -			return err;
> -
> -		fwnode = args.fwnode;
> -	}
> -
> -	err = fwnode_property_read_string(fwnode, "compatible", &value);
> -	if (err)
> -		goto out_put_node;
> -
> -	if (strcmp("simple-battery", value)) {
> -		err = -ENODEV;
> -		goto out_put_node;
> -	}
> -
>  	/* The property and field names below must correspond to elements
>  	 * in enum power_supply_property. For reasoning, see
>  	 * Documentation/power/power_supply_class.rst.
> diff --git a/drivers/power/supply/samsung-sdi-battery.c b/drivers/power/supply/samsung-sdi-battery.c
> index 64ec34a41c5b..9d59f277f519 100644
> --- a/drivers/power/supply/samsung-sdi-battery.c
> +++ b/drivers/power/supply/samsung-sdi-battery.c
> @@ -10,7 +10,6 @@
>   * the BTI resistance with a multimeter on the battery.
>   */
>  #include <linux/module.h>
> -#include <linux/of.h>
>  #include <linux/power_supply.h>
>  #include "samsung-sdi-battery.h"
>  
> @@ -895,7 +894,7 @@ static struct samsung_sdi_battery samsung_sdi_batteries[] = {
>  };
>  
>  int samsung_sdi_battery_get_info(struct device *dev,
> -				 struct device_node *np,
> +				 const char *compatible,
>  				 struct power_supply_battery_info **info)
>  {
>  	struct samsung_sdi_battery *batt;
> @@ -903,7 +902,7 @@ int samsung_sdi_battery_get_info(struct device *dev,
>  
>  	for (i = 0; i < ARRAY_SIZE(samsung_sdi_batteries); i++) {
>  		batt = &samsung_sdi_batteries[i];
> -		if (of_device_is_compatible(np, batt->compatible))
> +		if (!strcmp(compatible, batt->compatible))
>  			break;
>  	}
>  
> diff --git a/drivers/power/supply/samsung-sdi-battery.h b/drivers/power/supply/samsung-sdi-battery.h
> index 08783847dfcb..365ab6e85b26 100644
> --- a/drivers/power/supply/samsung-sdi-battery.h
> +++ b/drivers/power/supply/samsung-sdi-battery.h
> @@ -1,11 +1,11 @@
>  /* SPDX-License-Identifier: GPL-2.0 */
>  #if IS_ENABLED(CONFIG_BATTERY_SAMSUNG_SDI)
>  extern int samsung_sdi_battery_get_info(struct device *dev,
> -				struct device_node *np,
> +				const char *compatible,
>  				struct power_supply_battery_info **info);
>  #else
>  static inline int samsung_sdi_battery_get_info(struct device *dev,
> -				struct device_node *np,
> +				const char *compatible,
>  				struct power_supply_battery_info **info)
>  {
>  	return -ENODEV;
> -- 
> 2.34.1
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

end of thread, other threads:[~2022-03-04 21:22 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-01  0:30 [PATCH] power: supply: Fix static batteries Linus Walleij
2022-03-04 21:22 ` Sebastian Reichel

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.