linux-pm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/7] power: supply: sbs-battery: use octal permissions on module param
@ 2019-11-01 19:06 Jean-Francois Dagenais
  2019-11-01 19:07 ` [PATCH 2/7] power: supply: sbs-battery: prefix module param variable Jean-Francois Dagenais
                   ` (6 more replies)
  0 siblings, 7 replies; 12+ messages in thread
From: Jean-Francois Dagenais @ 2019-11-01 19:06 UTC (permalink / raw)
  To: sre, linux-pm; +Cc: Jean-Francois Dagenais

Symbolic permissions 'S_IRUSR | S_IRGRP | S_IROTH' are not
preferred. Use octal permissions '0444'.

Signed-off-by: Jean-Francois Dagenais <jeff.dagenais@gmail.com>
---
 drivers/power/supply/sbs-battery.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/power/supply/sbs-battery.c b/drivers/power/supply/sbs-battery.c
index 048d205d7074..0e66968dabc3 100644
--- a/drivers/power/supply/sbs-battery.c
+++ b/drivers/power/supply/sbs-battery.c
@@ -994,6 +994,6 @@ module_i2c_driver(sbs_battery_driver);
 MODULE_DESCRIPTION("SBS battery monitor driver");
 MODULE_LICENSE("GPL");
 
-module_param(force_load, bool, S_IRUSR | S_IRGRP | S_IROTH);
+module_param(force_load, bool, 0444);
 MODULE_PARM_DESC(force_load,
 		 "Attempt to load the driver even if no battery is connected");
-- 
2.23.0


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

* [PATCH 2/7] power: supply: sbs-battery: prefix module param variable
  2019-11-01 19:06 [PATCH 1/7] power: supply: sbs-battery: use octal permissions on module param Jean-Francois Dagenais
@ 2019-11-01 19:07 ` Jean-Francois Dagenais
  2019-11-01 19:07 ` [PATCH 3/7] power: supply: sbs-battery: add force_load as a dts property Jean-Francois Dagenais
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 12+ messages in thread
From: Jean-Francois Dagenais @ 2019-11-01 19:07 UTC (permalink / raw)
  To: sre, linux-pm; +Cc: Jean-Francois Dagenais

This is to prevent confusion with the upcoming devicetree binding for
force_load.

Signed-off-by: Jean-Francois Dagenais <jeff.dagenais@gmail.com>
---
 drivers/power/supply/sbs-battery.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/power/supply/sbs-battery.c b/drivers/power/supply/sbs-battery.c
index 0e66968dabc3..f9e8c14ac830 100644
--- a/drivers/power/supply/sbs-battery.c
+++ b/drivers/power/supply/sbs-battery.c
@@ -168,7 +168,7 @@ struct sbs_info {
 
 static char model_name[I2C_SMBUS_BLOCK_MAX + 1];
 static char manufacturer[I2C_SMBUS_BLOCK_MAX + 1];
-static bool force_load;
+static bool param_force_load;
 
 static int sbs_read_word_data(struct i2c_client *client, u8 address)
 {
@@ -890,7 +890,7 @@ static int sbs_probe(struct i2c_client *client,
 	 * Before we register, we might need to make sure we can actually talk
 	 * to the battery.
 	 */
-	if (!(force_load || chip->gpio_detect)) {
+	if (!(param_force_load || chip->gpio_detect)) {
 		rc = sbs_read_word_data(client, sbs_data[REG_STATUS].addr);
 
 		if (rc < 0) {
@@ -994,6 +994,6 @@ module_i2c_driver(sbs_battery_driver);
 MODULE_DESCRIPTION("SBS battery monitor driver");
 MODULE_LICENSE("GPL");
 
-module_param(force_load, bool, 0444);
-MODULE_PARM_DESC(force_load,
+module_param(param_force_load, bool, 0444);
+MODULE_PARM_DESC(param_force_load,
 		 "Attempt to load the driver even if no battery is connected");
-- 
2.23.0


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

* [PATCH 3/7] power: supply: sbs-battery: add force_load as a dts property
  2019-11-01 19:06 [PATCH 1/7] power: supply: sbs-battery: use octal permissions on module param Jean-Francois Dagenais
  2019-11-01 19:07 ` [PATCH 2/7] power: supply: sbs-battery: prefix module param variable Jean-Francois Dagenais
@ 2019-11-01 19:07 ` Jean-Francois Dagenais
  2019-12-19  1:19   ` Sebastian Reichel
  2019-11-01 19:07 ` [PATCH 4/7] devicetree: bindings: sbs-battery: add sbs,force-load property Jean-Francois Dagenais
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 12+ messages in thread
From: Jean-Francois Dagenais @ 2019-11-01 19:07 UTC (permalink / raw)
  To: sre, linux-pm; +Cc: Jean-Francois Dagenais

This extends the behaviour of the existing module param so that system
integrators can specify the flag in device-tree.

Signed-off-by: Jean-Francois Dagenais <jeff.dagenais@gmail.com>
---
 drivers/power/supply/sbs-battery.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/power/supply/sbs-battery.c b/drivers/power/supply/sbs-battery.c
index f9e8c14ac830..f92b98d900d2 100644
--- a/drivers/power/supply/sbs-battery.c
+++ b/drivers/power/supply/sbs-battery.c
@@ -814,6 +814,7 @@ static int sbs_probe(struct i2c_client *client,
 	struct power_supply_desc *sbs_desc;
 	struct sbs_platform_data *pdata = client->dev.platform_data;
 	struct power_supply_config psy_cfg = {};
+	bool force_load;
 	int rc;
 	int irq;
 
@@ -858,6 +859,9 @@ static int sbs_probe(struct i2c_client *client,
 	}
 	chip->i2c_retry_count = chip->i2c_retry_count + 1;
 
+	force_load = of_property_read_bool(client->dev.of_node,
+					   "sbs,force-load");
+
 	chip->gpio_detect = devm_gpiod_get_optional(&client->dev,
 			"sbs,battery-detect", GPIOD_IN);
 	if (IS_ERR(chip->gpio_detect)) {
@@ -890,7 +894,7 @@ static int sbs_probe(struct i2c_client *client,
 	 * Before we register, we might need to make sure we can actually talk
 	 * to the battery.
 	 */
-	if (!(param_force_load || chip->gpio_detect)) {
+	if (!(param_force_load || force_load || chip->gpio_detect)) {
 		rc = sbs_read_word_data(client, sbs_data[REG_STATUS].addr);
 
 		if (rc < 0) {
-- 
2.23.0


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

* [PATCH 4/7] devicetree: bindings: sbs-battery: add sbs,force-load property
  2019-11-01 19:06 [PATCH 1/7] power: supply: sbs-battery: use octal permissions on module param Jean-Francois Dagenais
  2019-11-01 19:07 ` [PATCH 2/7] power: supply: sbs-battery: prefix module param variable Jean-Francois Dagenais
  2019-11-01 19:07 ` [PATCH 3/7] power: supply: sbs-battery: add force_load as a dts property Jean-Francois Dagenais
@ 2019-11-01 19:07 ` Jean-Francois Dagenais
  2019-12-19  1:23   ` Sebastian Reichel
  2019-11-01 19:07 ` [PATCH 5/7] power: supply: sbs-battery: fix CAPACITY_MODE bit naming Jean-Francois Dagenais
                   ` (3 subsequent siblings)
  6 siblings, 1 reply; 12+ messages in thread
From: Jean-Francois Dagenais @ 2019-11-01 19:07 UTC (permalink / raw)
  To: sre, linux-pm; +Cc: Jean-Francois Dagenais

Signed-off-by: Jean-Francois Dagenais <jeff.dagenais@gmail.com>
---
 .../devicetree/bindings/power/supply/sbs_sbs-battery.txt       | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/power/supply/sbs_sbs-battery.txt b/Documentation/devicetree/bindings/power/supply/sbs_sbs-battery.txt
index 4e78e51018eb..62ec842325b4 100644
--- a/Documentation/devicetree/bindings/power/supply/sbs_sbs-battery.txt
+++ b/Documentation/devicetree/bindings/power/supply/sbs_sbs-battery.txt
@@ -15,6 +15,8 @@ Optional properties :
    after an external change notification.
  - sbs,battery-detect-gpios : The gpio which signals battery detection and
    a flag specifying its polarity.
+ - sbs,force-load : Same as the module param, makes the probe skip the
+   detection of the battery and assume it is (or will be) present.
 
 Example:
 
@@ -24,4 +26,5 @@ Example:
 		sbs,i2c-retry-count = <2>;
 		sbs,poll-retry-count = <10>;
 		sbs,battery-detect-gpios = <&gpio-controller 122 1>;
+		sbs,force-load;
 	}
-- 
2.23.0


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

* [PATCH 5/7] power: supply: sbs-battery: fix CAPACITY_MODE bit naming
  2019-11-01 19:06 [PATCH 1/7] power: supply: sbs-battery: use octal permissions on module param Jean-Francois Dagenais
                   ` (2 preceding siblings ...)
  2019-11-01 19:07 ` [PATCH 4/7] devicetree: bindings: sbs-battery: add sbs,force-load property Jean-Francois Dagenais
@ 2019-11-01 19:07 ` Jean-Francois Dagenais
  2019-12-19  1:22   ` Sebastian Reichel
  2019-11-01 19:07 ` [PATCH 6/7] power: supply: sbs-battery: add ability to disable charger broadcasts Jean-Francois Dagenais
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 12+ messages in thread
From: Jean-Francois Dagenais @ 2019-11-01 19:07 UTC (permalink / raw)
  To: sre, linux-pm; +Cc: Jean-Francois Dagenais

"Battery mode" is the name of the register, the bit manipulated by this
code is "CAPACITY_MODE" (Smart Battery System Specifications).

Signed-off-by: Jean-Francois Dagenais <jeff.dagenais@gmail.com>
---
 drivers/power/supply/sbs-battery.c | 30 +++++++++++++++---------------
 1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/drivers/power/supply/sbs-battery.c b/drivers/power/supply/sbs-battery.c
index f92b98d900d2..46c89dd05f46 100644
--- a/drivers/power/supply/sbs-battery.c
+++ b/drivers/power/supply/sbs-battery.c
@@ -46,10 +46,10 @@ enum {
 
 /* Battery Mode defines */
 #define BATTERY_MODE_OFFSET		0x03
-#define BATTERY_MODE_MASK		0x8000
-enum sbs_battery_mode {
-	BATTERY_MODE_AMPS = 0,
-	BATTERY_MODE_WATTS = 0x8000
+#define BATTERY_MODE_CAPACITY_MASK	(1<<15)
+enum sbs_capacity_mode {
+	CAPACITY_MODE_AMPS = 0,
+	CAPACITY_MODE_WATTS = BATTERY_MODE_CAPACITY_MASK
 };
 
 /* manufacturer access defines */
@@ -513,8 +513,8 @@ static void  sbs_unit_adjustment(struct i2c_client *client,
 	}
 }
 
-static enum sbs_battery_mode sbs_set_battery_mode(struct i2c_client *client,
-	enum sbs_battery_mode mode)
+static enum sbs_capacity_mode sbs_set_capacity_mode(struct i2c_client *client,
+	enum sbs_capacity_mode mode)
 {
 	int ret, original_val;
 
@@ -522,13 +522,13 @@ static enum sbs_battery_mode sbs_set_battery_mode(struct i2c_client *client,
 	if (original_val < 0)
 		return original_val;
 
-	if ((original_val & BATTERY_MODE_MASK) == mode)
+	if ((original_val & BATTERY_MODE_CAPACITY_MASK) == mode)
 		return mode;
 
-	if (mode == BATTERY_MODE_AMPS)
-		ret = original_val & ~BATTERY_MODE_MASK;
+	if (mode == CAPACITY_MODE_AMPS)
+		ret = original_val & ~BATTERY_MODE_CAPACITY_MASK;
 	else
-		ret = original_val | BATTERY_MODE_MASK;
+		ret = original_val | BATTERY_MODE_CAPACITY_MASK;
 
 	ret = sbs_write_word_data(client, BATTERY_MODE_OFFSET, ret);
 	if (ret < 0)
@@ -536,7 +536,7 @@ static enum sbs_battery_mode sbs_set_battery_mode(struct i2c_client *client,
 
 	usleep_range(1000, 2000);
 
-	return original_val & BATTERY_MODE_MASK;
+	return original_val & BATTERY_MODE_CAPACITY_MASK;
 }
 
 static int sbs_get_battery_capacity(struct i2c_client *client,
@@ -544,12 +544,12 @@ static int sbs_get_battery_capacity(struct i2c_client *client,
 	union power_supply_propval *val)
 {
 	s32 ret;
-	enum sbs_battery_mode mode = BATTERY_MODE_WATTS;
+	enum sbs_capacity_mode mode = CAPACITY_MODE_WATTS;
 
 	if (power_supply_is_amp_property(psp))
-		mode = BATTERY_MODE_AMPS;
+		mode = CAPACITY_MODE_AMPS;
 
-	mode = sbs_set_battery_mode(client, mode);
+	mode = sbs_set_capacity_mode(client, mode);
 	if (mode < 0)
 		return mode;
 
@@ -559,7 +559,7 @@ static int sbs_get_battery_capacity(struct i2c_client *client,
 
 	val->intval = ret;
 
-	ret = sbs_set_battery_mode(client, mode);
+	ret = sbs_set_capacity_mode(client, mode);
 	if (ret < 0)
 		return ret;
 
-- 
2.23.0


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

* [PATCH 6/7] power: supply: sbs-battery: add ability to disable charger broadcasts
  2019-11-01 19:06 [PATCH 1/7] power: supply: sbs-battery: use octal permissions on module param Jean-Francois Dagenais
                   ` (3 preceding siblings ...)
  2019-11-01 19:07 ` [PATCH 5/7] power: supply: sbs-battery: fix CAPACITY_MODE bit naming Jean-Francois Dagenais
@ 2019-11-01 19:07 ` Jean-Francois Dagenais
  2019-11-01 19:07 ` [PATCH 7/7] dt-bindings: power: sbs-battery: add disable-charger-broadcasts doc Jean-Francois Dagenais
  2019-12-19  1:13 ` [PATCH 1/7] power: supply: sbs-battery: use octal permissions on module param Sebastian Reichel
  6 siblings, 0 replies; 12+ messages in thread
From: Jean-Francois Dagenais @ 2019-11-01 19:07 UTC (permalink / raw)
  To: sre, linux-pm; +Cc: Jean-Francois Dagenais

In certain designs, it is possible to add a battery on a populated i2c
bus without an sbs compliant charger. In that case, the battery will
un-necessarily and sometimes un-desirably master the bus trying to write
info in the charger.

It is observed in many occasion that these battery "broadcasts" are even
corrupting other ongoing master to slave communication. I.e. the
multi-master support in the battery is inadequate.

Thankfully, the CHARGER_MODE bit allows designers to disable that SBS
battery behaviour.

This needs to be done once when the battery is first seen on the bus.

Signed-off-by: Jean-Francois Dagenais <jeff.dagenais@gmail.com>
---
 drivers/power/supply/sbs-battery.c | 39 +++++++++++++++++++++++++++---
 1 file changed, 36 insertions(+), 3 deletions(-)

diff --git a/drivers/power/supply/sbs-battery.c b/drivers/power/supply/sbs-battery.c
index 46c89dd05f46..fb116ab7752d 100644
--- a/drivers/power/supply/sbs-battery.c
+++ b/drivers/power/supply/sbs-battery.c
@@ -51,6 +51,7 @@ enum sbs_capacity_mode {
 	CAPACITY_MODE_AMPS = 0,
 	CAPACITY_MODE_WATTS = BATTERY_MODE_CAPACITY_MASK
 };
+#define BATTERY_MODE_CHARGER_MASK	(1<<14)
 
 /* manufacturer access defines */
 #define MANUFACTURER_ACCESS_STATUS	0x0006
@@ -157,6 +158,7 @@ struct sbs_info {
 	bool				is_present;
 	struct gpio_desc		*gpio_detect;
 	bool				enable_detection;
+	bool				charger_broadcasts;
 	int				last_state;
 	int				poll_time;
 	u32				i2c_retry_count;
@@ -596,6 +598,34 @@ static int sbs_get_property_index(struct i2c_client *client,
 	return -EINVAL;
 }
 
+static void sbs_disable_charger_broadcasts(struct sbs_info *chip)
+{
+	int val = sbs_read_word_data(chip->client, BATTERY_MODE_OFFSET);
+	if (val < 0)
+		goto exit;
+
+	val |= BATTERY_MODE_CHARGER_MASK;
+
+	val = sbs_write_word_data(chip->client, BATTERY_MODE_OFFSET, val);
+
+exit:
+	if (val < 0)
+		dev_err(&chip->client->dev,
+			"Failed to disable charger broadcasting: %d\n", val);
+	else
+		dev_dbg(&chip->client->dev, "%s\n", __func__);
+}
+
+static void sbs_set_is_present(struct sbs_info *chip, bool is_present)
+{
+	dev_dbg(&chip->client->dev, "%s %d\n", __func__, is_present);
+
+	if (!chip->is_present && is_present && !chip->charger_broadcasts)
+		sbs_disable_charger_broadcasts(chip);
+
+	chip->is_present = is_present;
+}
+
 static int sbs_get_property(struct power_supply *psy,
 	enum power_supply_property psp,
 	union power_supply_propval *val)
@@ -610,7 +640,7 @@ static int sbs_get_property(struct power_supply *psy,
 			return ret;
 		if (psp == POWER_SUPPLY_PROP_PRESENT) {
 			val->intval = ret;
-			chip->is_present = val->intval;
+			sbs_set_is_present(chip, val->intval);
 			return 0;
 		}
 		if (ret == 0)
@@ -706,7 +736,7 @@ static int sbs_get_property(struct power_supply *psy,
 
 	if (!chip->gpio_detect &&
 		chip->is_present != (ret >= 0)) {
-		chip->is_present = (ret >= 0);
+		sbs_set_is_present(chip, ret >= 0);
 		power_supply_changed(chip->power_supply);
 	}
 
@@ -737,7 +767,7 @@ static void sbs_supply_changed(struct sbs_info *chip)
 	ret = gpiod_get_value_cansleep(chip->gpio_detect);
 	if (ret < 0)
 		return;
-	chip->is_present = ret;
+	sbs_set_is_present(chip, ret);
 	power_supply_changed(battery);
 }
 
@@ -862,6 +892,9 @@ static int sbs_probe(struct i2c_client *client,
 	force_load = of_property_read_bool(client->dev.of_node,
 					   "sbs,force-load");
 
+	chip->charger_broadcasts = !of_property_read_bool(client->dev.of_node,
+					"sbs,disable-charger-broadcasts");
+
 	chip->gpio_detect = devm_gpiod_get_optional(&client->dev,
 			"sbs,battery-detect", GPIOD_IN);
 	if (IS_ERR(chip->gpio_detect)) {
-- 
2.23.0


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

* [PATCH 7/7] dt-bindings: power: sbs-battery: add disable-charger-broadcasts doc
  2019-11-01 19:06 [PATCH 1/7] power: supply: sbs-battery: use octal permissions on module param Jean-Francois Dagenais
                   ` (4 preceding siblings ...)
  2019-11-01 19:07 ` [PATCH 6/7] power: supply: sbs-battery: add ability to disable charger broadcasts Jean-Francois Dagenais
@ 2019-11-01 19:07 ` Jean-Francois Dagenais
  2019-12-19  1:22   ` Sebastian Reichel
  2019-12-19  1:13 ` [PATCH 1/7] power: supply: sbs-battery: use octal permissions on module param Sebastian Reichel
  6 siblings, 1 reply; 12+ messages in thread
From: Jean-Francois Dagenais @ 2019-11-01 19:07 UTC (permalink / raw)
  To: sre, linux-pm; +Cc: Jean-Francois Dagenais

Signed-off-by: Jean-Francois Dagenais <jeff.dagenais@gmail.com>
---
 .../devicetree/bindings/power/supply/sbs_sbs-battery.txt        | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/power/supply/sbs_sbs-battery.txt b/Documentation/devicetree/bindings/power/supply/sbs_sbs-battery.txt
index 62ec842325b4..578643f49b0a 100644
--- a/Documentation/devicetree/bindings/power/supply/sbs_sbs-battery.txt
+++ b/Documentation/devicetree/bindings/power/supply/sbs_sbs-battery.txt
@@ -17,6 +17,7 @@ Optional properties :
    a flag specifying its polarity.
  - sbs,force-load : Same as the module param, makes the probe skip the
    detection of the battery and assume it is (or will be) present.
+ - sbs,disable-charger-broadcasts: for systems without sbs compliant chargers
 
 Example:
 
@@ -27,4 +28,5 @@ Example:
 		sbs,poll-retry-count = <10>;
 		sbs,battery-detect-gpios = <&gpio-controller 122 1>;
 		sbs,force-load;
+		sbs,disable-charger-broadcasts;
 	}
-- 
2.23.0


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

* Re: [PATCH 1/7] power: supply: sbs-battery: use octal permissions on module param
  2019-11-01 19:06 [PATCH 1/7] power: supply: sbs-battery: use octal permissions on module param Jean-Francois Dagenais
                   ` (5 preceding siblings ...)
  2019-11-01 19:07 ` [PATCH 7/7] dt-bindings: power: sbs-battery: add disable-charger-broadcasts doc Jean-Francois Dagenais
@ 2019-12-19  1:13 ` Sebastian Reichel
  6 siblings, 0 replies; 12+ messages in thread
From: Sebastian Reichel @ 2019-12-19  1:13 UTC (permalink / raw)
  To: Jean-Francois Dagenais; +Cc: linux-pm

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

Hi,

On Fri, Nov 01, 2019 at 03:06:59PM -0400, Jean-Francois Dagenais wrote:
> Symbolic permissions 'S_IRUSR | S_IRGRP | S_IROTH' are not
> preferred. Use octal permissions '0444'.
> 
> Signed-off-by: Jean-Francois Dagenais <jeff.dagenais@gmail.com>
> ---

Thanks, queued.

-- Sebastian

>  drivers/power/supply/sbs-battery.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/power/supply/sbs-battery.c b/drivers/power/supply/sbs-battery.c
> index 048d205d7074..0e66968dabc3 100644
> --- a/drivers/power/supply/sbs-battery.c
> +++ b/drivers/power/supply/sbs-battery.c
> @@ -994,6 +994,6 @@ module_i2c_driver(sbs_battery_driver);
>  MODULE_DESCRIPTION("SBS battery monitor driver");
>  MODULE_LICENSE("GPL");
>  
> -module_param(force_load, bool, S_IRUSR | S_IRGRP | S_IROTH);
> +module_param(force_load, bool, 0444);
>  MODULE_PARM_DESC(force_load,
>  		 "Attempt to load the driver even if no battery is connected");
> -- 
> 2.23.0
> 

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

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

* Re: [PATCH 3/7] power: supply: sbs-battery: add force_load as a dts property
  2019-11-01 19:07 ` [PATCH 3/7] power: supply: sbs-battery: add force_load as a dts property Jean-Francois Dagenais
@ 2019-12-19  1:19   ` Sebastian Reichel
  0 siblings, 0 replies; 12+ messages in thread
From: Sebastian Reichel @ 2019-12-19  1:19 UTC (permalink / raw)
  To: Jean-Francois Dagenais; +Cc: linux-pm

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

Hi,

On Fri, Nov 01, 2019 at 03:07:01PM -0400, Jean-Francois Dagenais wrote:
> This extends the behaviour of the existing module param so that system
> integrators can specify the flag in device-tree.
> 
> Signed-off-by: Jean-Francois Dagenais <jeff.dagenais@gmail.com>
> ---

sbs,force-load is not a hardware description and does not belong
into DT. OTOH "hot-plug-capable;" would be a hardware description
and carry the same information. Please respin the patch(es) and CC
Rob Herring <robh+dt@kernel.org> and devicetree@vger.kernel.org.

-- Sebastian

>  drivers/power/supply/sbs-battery.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/power/supply/sbs-battery.c b/drivers/power/supply/sbs-battery.c
> index f9e8c14ac830..f92b98d900d2 100644
> --- a/drivers/power/supply/sbs-battery.c
> +++ b/drivers/power/supply/sbs-battery.c
> @@ -814,6 +814,7 @@ static int sbs_probe(struct i2c_client *client,
>  	struct power_supply_desc *sbs_desc;
>  	struct sbs_platform_data *pdata = client->dev.platform_data;
>  	struct power_supply_config psy_cfg = {};
> +	bool force_load;
>  	int rc;
>  	int irq;
>  
> @@ -858,6 +859,9 @@ static int sbs_probe(struct i2c_client *client,
>  	}
>  	chip->i2c_retry_count = chip->i2c_retry_count + 1;
>  
> +	force_load = of_property_read_bool(client->dev.of_node,
> +					   "sbs,force-load");
> +
>  	chip->gpio_detect = devm_gpiod_get_optional(&client->dev,
>  			"sbs,battery-detect", GPIOD_IN);
>  	if (IS_ERR(chip->gpio_detect)) {
> @@ -890,7 +894,7 @@ static int sbs_probe(struct i2c_client *client,
>  	 * Before we register, we might need to make sure we can actually talk
>  	 * to the battery.
>  	 */
> -	if (!(param_force_load || chip->gpio_detect)) {
> +	if (!(param_force_load || force_load || chip->gpio_detect)) {
>  		rc = sbs_read_word_data(client, sbs_data[REG_STATUS].addr);
>  
>  		if (rc < 0) {
> -- 
> 2.23.0
> 

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

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

* Re: [PATCH 7/7] dt-bindings: power: sbs-battery: add disable-charger-broadcasts doc
  2019-11-01 19:07 ` [PATCH 7/7] dt-bindings: power: sbs-battery: add disable-charger-broadcasts doc Jean-Francois Dagenais
@ 2019-12-19  1:22   ` Sebastian Reichel
  0 siblings, 0 replies; 12+ messages in thread
From: Sebastian Reichel @ 2019-12-19  1:22 UTC (permalink / raw)
  To: Jean-Francois Dagenais; +Cc: linux-pm

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

Hi,

This is missing a long patch description and not CC'd to DT binding
maintainers.

-- Sebastian

On Fri, Nov 01, 2019 at 03:07:05PM -0400, Jean-Francois Dagenais wrote:
> Signed-off-by: Jean-Francois Dagenais <jeff.dagenais@gmail.com>
> ---
>  .../devicetree/bindings/power/supply/sbs_sbs-battery.txt        | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/power/supply/sbs_sbs-battery.txt b/Documentation/devicetree/bindings/power/supply/sbs_sbs-battery.txt
> index 62ec842325b4..578643f49b0a 100644
> --- a/Documentation/devicetree/bindings/power/supply/sbs_sbs-battery.txt
> +++ b/Documentation/devicetree/bindings/power/supply/sbs_sbs-battery.txt
> @@ -17,6 +17,7 @@ Optional properties :
>     a flag specifying its polarity.
>   - sbs,force-load : Same as the module param, makes the probe skip the
>     detection of the battery and assume it is (or will be) present.
> + - sbs,disable-charger-broadcasts: for systems without sbs compliant chargers
>  
>  Example:
>  
> @@ -27,4 +28,5 @@ Example:
>  		sbs,poll-retry-count = <10>;
>  		sbs,battery-detect-gpios = <&gpio-controller 122 1>;
>  		sbs,force-load;
> +		sbs,disable-charger-broadcasts;
>  	}
> -- 
> 2.23.0
> 

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

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

* Re: [PATCH 5/7] power: supply: sbs-battery: fix CAPACITY_MODE bit naming
  2019-11-01 19:07 ` [PATCH 5/7] power: supply: sbs-battery: fix CAPACITY_MODE bit naming Jean-Francois Dagenais
@ 2019-12-19  1:22   ` Sebastian Reichel
  0 siblings, 0 replies; 12+ messages in thread
From: Sebastian Reichel @ 2019-12-19  1:22 UTC (permalink / raw)
  To: Jean-Francois Dagenais; +Cc: linux-pm

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

Hi,

On Fri, Nov 01, 2019 at 03:07:03PM -0400, Jean-Francois Dagenais wrote:
> "Battery mode" is the name of the register, the bit manipulated by this
> code is "CAPACITY_MODE" (Smart Battery System Specifications).
> 
> Signed-off-by: Jean-Francois Dagenais <jeff.dagenais@gmail.com>
> ---

Thanks, queued.

-- Sebastian

>  drivers/power/supply/sbs-battery.c | 30 +++++++++++++++---------------
>  1 file changed, 15 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/power/supply/sbs-battery.c b/drivers/power/supply/sbs-battery.c
> index f92b98d900d2..46c89dd05f46 100644
> --- a/drivers/power/supply/sbs-battery.c
> +++ b/drivers/power/supply/sbs-battery.c
> @@ -46,10 +46,10 @@ enum {
>  
>  /* Battery Mode defines */
>  #define BATTERY_MODE_OFFSET		0x03
> -#define BATTERY_MODE_MASK		0x8000
> -enum sbs_battery_mode {
> -	BATTERY_MODE_AMPS = 0,
> -	BATTERY_MODE_WATTS = 0x8000
> +#define BATTERY_MODE_CAPACITY_MASK	(1<<15)
> +enum sbs_capacity_mode {
> +	CAPACITY_MODE_AMPS = 0,
> +	CAPACITY_MODE_WATTS = BATTERY_MODE_CAPACITY_MASK
>  };
>  
>  /* manufacturer access defines */
> @@ -513,8 +513,8 @@ static void  sbs_unit_adjustment(struct i2c_client *client,
>  	}
>  }
>  
> -static enum sbs_battery_mode sbs_set_battery_mode(struct i2c_client *client,
> -	enum sbs_battery_mode mode)
> +static enum sbs_capacity_mode sbs_set_capacity_mode(struct i2c_client *client,
> +	enum sbs_capacity_mode mode)
>  {
>  	int ret, original_val;
>  
> @@ -522,13 +522,13 @@ static enum sbs_battery_mode sbs_set_battery_mode(struct i2c_client *client,
>  	if (original_val < 0)
>  		return original_val;
>  
> -	if ((original_val & BATTERY_MODE_MASK) == mode)
> +	if ((original_val & BATTERY_MODE_CAPACITY_MASK) == mode)
>  		return mode;
>  
> -	if (mode == BATTERY_MODE_AMPS)
> -		ret = original_val & ~BATTERY_MODE_MASK;
> +	if (mode == CAPACITY_MODE_AMPS)
> +		ret = original_val & ~BATTERY_MODE_CAPACITY_MASK;
>  	else
> -		ret = original_val | BATTERY_MODE_MASK;
> +		ret = original_val | BATTERY_MODE_CAPACITY_MASK;
>  
>  	ret = sbs_write_word_data(client, BATTERY_MODE_OFFSET, ret);
>  	if (ret < 0)
> @@ -536,7 +536,7 @@ static enum sbs_battery_mode sbs_set_battery_mode(struct i2c_client *client,
>  
>  	usleep_range(1000, 2000);
>  
> -	return original_val & BATTERY_MODE_MASK;
> +	return original_val & BATTERY_MODE_CAPACITY_MASK;
>  }
>  
>  static int sbs_get_battery_capacity(struct i2c_client *client,
> @@ -544,12 +544,12 @@ static int sbs_get_battery_capacity(struct i2c_client *client,
>  	union power_supply_propval *val)
>  {
>  	s32 ret;
> -	enum sbs_battery_mode mode = BATTERY_MODE_WATTS;
> +	enum sbs_capacity_mode mode = CAPACITY_MODE_WATTS;
>  
>  	if (power_supply_is_amp_property(psp))
> -		mode = BATTERY_MODE_AMPS;
> +		mode = CAPACITY_MODE_AMPS;
>  
> -	mode = sbs_set_battery_mode(client, mode);
> +	mode = sbs_set_capacity_mode(client, mode);
>  	if (mode < 0)
>  		return mode;
>  
> @@ -559,7 +559,7 @@ static int sbs_get_battery_capacity(struct i2c_client *client,
>  
>  	val->intval = ret;
>  
> -	ret = sbs_set_battery_mode(client, mode);
> +	ret = sbs_set_capacity_mode(client, mode);
>  	if (ret < 0)
>  		return ret;
>  
> -- 
> 2.23.0
> 

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

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

* Re: [PATCH 4/7] devicetree: bindings: sbs-battery: add sbs,force-load property
  2019-11-01 19:07 ` [PATCH 4/7] devicetree: bindings: sbs-battery: add sbs,force-load property Jean-Francois Dagenais
@ 2019-12-19  1:23   ` Sebastian Reichel
  0 siblings, 0 replies; 12+ messages in thread
From: Sebastian Reichel @ 2019-12-19  1:23 UTC (permalink / raw)
  To: Jean-Francois Dagenais; +Cc: linux-pm

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

Hi,

Kernel patches should always have a long description.

-- Sebastian

On Fri, Nov 01, 2019 at 03:07:02PM -0400, Jean-Francois Dagenais wrote:
> Signed-off-by: Jean-Francois Dagenais <jeff.dagenais@gmail.com>
> ---
>  .../devicetree/bindings/power/supply/sbs_sbs-battery.txt       | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/power/supply/sbs_sbs-battery.txt b/Documentation/devicetree/bindings/power/supply/sbs_sbs-battery.txt
> index 4e78e51018eb..62ec842325b4 100644
> --- a/Documentation/devicetree/bindings/power/supply/sbs_sbs-battery.txt
> +++ b/Documentation/devicetree/bindings/power/supply/sbs_sbs-battery.txt
> @@ -15,6 +15,8 @@ Optional properties :
>     after an external change notification.
>   - sbs,battery-detect-gpios : The gpio which signals battery detection and
>     a flag specifying its polarity.
> + - sbs,force-load : Same as the module param, makes the probe skip the
> +   detection of the battery and assume it is (or will be) present.
>  
>  Example:
>  
> @@ -24,4 +26,5 @@ Example:
>  		sbs,i2c-retry-count = <2>;
>  		sbs,poll-retry-count = <10>;
>  		sbs,battery-detect-gpios = <&gpio-controller 122 1>;
> +		sbs,force-load;
>  	}
> -- 
> 2.23.0
> 

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

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

end of thread, other threads:[~2019-12-19  1:23 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-01 19:06 [PATCH 1/7] power: supply: sbs-battery: use octal permissions on module param Jean-Francois Dagenais
2019-11-01 19:07 ` [PATCH 2/7] power: supply: sbs-battery: prefix module param variable Jean-Francois Dagenais
2019-11-01 19:07 ` [PATCH 3/7] power: supply: sbs-battery: add force_load as a dts property Jean-Francois Dagenais
2019-12-19  1:19   ` Sebastian Reichel
2019-11-01 19:07 ` [PATCH 4/7] devicetree: bindings: sbs-battery: add sbs,force-load property Jean-Francois Dagenais
2019-12-19  1:23   ` Sebastian Reichel
2019-11-01 19:07 ` [PATCH 5/7] power: supply: sbs-battery: fix CAPACITY_MODE bit naming Jean-Francois Dagenais
2019-12-19  1:22   ` Sebastian Reichel
2019-11-01 19:07 ` [PATCH 6/7] power: supply: sbs-battery: add ability to disable charger broadcasts Jean-Francois Dagenais
2019-11-01 19:07 ` [PATCH 7/7] dt-bindings: power: sbs-battery: add disable-charger-broadcasts doc Jean-Francois Dagenais
2019-12-19  1:22   ` Sebastian Reichel
2019-12-19  1:13 ` [PATCH 1/7] power: supply: sbs-battery: use octal permissions on module param Sebastian Reichel

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).