power: bq27xxx: Fix multiple defined but not used warnings

PATCH In review Sebastian Reichel 3 participants 12 messages Oct 16, 2015 – Oct 19, 2015

lore

Share & tools

lore

Sebastian Reichel

author

2015-10-16 13:44 UTC #0 lore raw

cover power: bq27xxx: Fix multiple defined but not used warnings

Hi,

These patches fix multiple "defined but not used" warnings in the bq27xxx driver.

-- Sebastian

Sebastian Reichel (3):
power: bq27xxx_battery: fix platform probe
power: bq27xxx_battery: move irq handler to i2c section
power: bq27xxx_battery: fix defined but not used warnings

 drivers/power/bq27xxx_battery.c | 23 ++++++++++++-----------
 1 file changed, 12 insertions(+), 11 deletions(-)

Signature · 1 line

2.6.1

Sebastian Reichel

author

2015-10-16 13:44 UTC #1 lore raw

PATCH 1/3 power: bq27xxx_battery: fix platform probe

Add missing initialization of register mapping table to platform probe function.

Signed-off-by Sebastian Reichel <[email protected]>

---
 drivers/power/bq27xxx_battery.c | 1 +
 1 file changed, 1 insertion(+)

drivers/power/bq27xxx_battery.c +1 -0

index 994c78d9f426..69e6b371a51c 100644
@@ -1292,6 +1292,7 @@ static int bq27xxx_battery_platform_probe(struct platform_device *pdev)
 
 	di->dev = &pdev->dev;
 	di->chip = pdata->chip;
+	di->regs = bq27xxx_regs[di->chip];
 
 	name = pdata->name ?: dev_name(&pdev->dev);
 	di->bus.read = &bq27xxx_battery_platform_read;

Signature · 1 line

2.6.1

Pali Rohár

Acked-by on v1 1/3

2015-10-16 14:47 UTC #2 lore raw

Re: [PATCH 1/3] power: bq27xxx_battery: fix platform probe

Quoted 13 lines · On Friday 16 October 2015 15:44:10 Sebastian Reichel wrote:

Add missing initialization of register mapping table to platform probe function.
Signed-off-by Sebastian Reichel <[email protected]>
---
 drivers/power/bq27xxx_battery.c | 1 +
 1 file changed, 1 insertion(+)
drivers/power/bq27xxx_battery.c +0 -0
index 994c78d9f426..69e6b371a51c 100644

drivers/power/bq27xxx_battery.c static int bq27xxx_battery_platform_probe(struct platform_device *pdev) · line 1298

Quoted 7 lines

drivers/power/bq27xxx_battery.c +1 -0

@@ -1292,6 +1292,7 @@ static int bq27xxx_battery_platform_probe(struct platform_device *pdev)
 
 	di->dev = &pdev->dev;
 	di->chip = pdata->chip;
+	di->regs = bq27xxx_regs[di->chip];
 
 	name = pdata->name ?: dev_name(&pdev->dev);
 	di->bus.read = &bq27xxx_battery_platform_read;

Acked-by Pali Rohár <[email protected]>

Signature · 2 lines

Pali Rohár
[email protected]

Andrew F. Davis

Acked-by on v1 1/3

2015-10-16 14:53 UTC #3 lore raw

Re: [PATCH 1/3] power: bq27xxx_battery: fix platform probe

In reply to Sebastian Reichel

Quoted 14 lines · Sebastian Reichel

Add missing initialization of register mapping table to platform probe function.
Signed-off-by Sebastian Reichel <[email protected]>
---
  drivers/power/bq27xxx_battery.c | 1 +
  1 file changed, 1 insertion(+)
drivers/power/bq27xxx_battery.c +0 -0
index 994c78d9f426..69e6b371a51c 100644

drivers/power/bq27xxx_battery.c static int bq27xxx_battery_platform_probe(struct platform_device *pdev) · line 1298

Quoted 7 lines

drivers/power/bq27xxx_battery.c +1 -0

@@ -1292,6 +1292,7 @@ static int bq27xxx_battery_platform_probe(struct platform_device *pdev)
 
  	di->dev = &pdev->dev;
  	di->chip = pdata->chip;
+	di->regs = bq27xxx_regs[di->chip];
 
  	name = pdata->name ?: dev_name(&pdev->dev);
  	di->bus.read = &bq27xxx_battery_platform_read;

Acked-by Andrew F. Davis <[email protected]>

Signature · 1 line

Andrew F. Davis

Sebastian Reichel

author

2015-10-16 13:44 UTC #4 lore raw

PATCH 2/3 power: bq27xxx_battery: move irq handler to i2c section

In reply to Sebastian Reichel

The IRQ handler is not used by the platform based code resulting in a 'defined but not used' warning, if CONFIG_BQ27XXX_I2C is not enabled.

Signed-off-by Sebastian Reichel <[email protected]>

---
 drivers/power/bq27xxx_battery.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

drivers/power/bq27xxx_battery.c +9 -9

index 69e6b371a51c..880233ce9343 100644
@@ -762,15 +762,6 @@ static void bq27xxx_battery_update(struct bq27xxx_device_info *di)
 	di->last_update = jiffies;
 }
 
-static irqreturn_t bq27xxx_battery_irq_handler_thread(int irq, void *data)
-{
-	struct bq27xxx_device_info *di = data;
-
-	bq27xxx_battery_update(di);
-
-	return IRQ_HANDLED;
-}
-
 static void bq27xxx_battery_poll(struct work_struct *work)
 {
 	struct bq27xxx_device_info *di =

@@ -1061,6 +1052,15 @@ static void bq27xxx_powersupply_unregister(struct bq27xxx_device_info *di)
 static DEFINE_IDR(battery_id);
 static DEFINE_MUTEX(battery_mutex);
 
+static irqreturn_t bq27xxx_battery_irq_handler_thread(int irq, void *data)
+{
+	struct bq27xxx_device_info *di = data;
+
+	bq27xxx_battery_update(di);
+
+	return IRQ_HANDLED;
+}
+
 static int bq27xxx_battery_i2c_read(struct bq27xxx_device_info *di, u8 reg,
 				    bool single)
 {

Signature · 1 line

2.6.1

Pali Rohár

Acked-by on v1 2/3

2015-10-16 14:48 UTC #5 lore raw

Re: [PATCH 2/3] power: bq27xxx_battery: move irq handler to i2c section

Quoted 35 lines · On Friday 16 October 2015 15:44:11 Sebastian Reichel wrote:

The IRQ handler is not used by the platform based code resulting in a 'defined but not used' warning, if CONFIG_BQ27XXX_I2C is not enabled.

Signed-off-by Sebastian Reichel <[email protected]>

---
 drivers/power/bq27xxx_battery.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

drivers/power/bq27xxx_battery.c +2 -9

index 69e6b371a51c..880233ce9343 100644
@@ -762,15 +762,6 @@ static void bq27xxx_battery_update(struct bq27xxx_device_info *di)
 	di->last_update = jiffies;
 }
 
-static irqreturn_t bq27xxx_battery_irq_handler_thread(int irq, void *data)
-{
-	struct bq27xxx_device_info *di = data;
-
-	bq27xxx_battery_update(di);
-
-	return IRQ_HANDLED;
-}
-
 static void bq27xxx_battery_poll(struct work_struct *work)
 {
 	struct bq27xxx_device_info *di =

@@ -1061,6 +1052,15 @@ static void bq27xxx_powersupply_unregister(struct bq27xxx_device_info *di)
 static DEFINE_IDR(battery_id);
 static DEFINE_MUTEX(battery_mutex);
 
+static irqreturn_t bq27xxx_battery_irq_handler_thread(int irq, void *data)
+{

drivers/power/bq27xxx_battery.c static void bq27xxx_powersupply_unregister(struct bq27xxx_device_info *di) · line 1066

Quoted 10 lines

drivers/power/bq27xxx_battery.c +7 -0

+	struct bq27xxx_device_info *di = data;
+
+	bq27xxx_battery_update(di);
+
+	return IRQ_HANDLED;
+}
+
 static int bq27xxx_battery_i2c_read(struct bq27xxx_device_info *di, u8 reg,
 				    bool single)
 {

Acked-by Pali Rohár <[email protected]>

Signature · 2 lines

Pali Rohár
[email protected]

Andrew F. Davis

Acked-by on v1 2/3

2015-10-16 15:06 UTC #6 lore raw

Re: [PATCH 2/3] power: bq27xxx_battery: move irq handler to i2c section

In reply to Sebastian Reichel

Quoted 36 lines · Sebastian Reichel

The IRQ handler is not used by the platform based code resulting in a 'defined but not used' warning, if CONFIG_BQ27XXX_I2C is not enabled.

Signed-off-by Sebastian Reichel <[email protected]>

---
  drivers/power/bq27xxx_battery.c | 18 +++++++++---------
  1 file changed, 9 insertions(+), 9 deletions(-)

drivers/power/bq27xxx_battery.c +2 -9

index 69e6b371a51c..880233ce9343 100644
@@ -762,15 +762,6 @@ static void bq27xxx_battery_update(struct bq27xxx_device_info *di)
  	di->last_update = jiffies;
  }
 
-static irqreturn_t bq27xxx_battery_irq_handler_thread(int irq, void *data)
-{
-	struct bq27xxx_device_info *di = data;
-
-	bq27xxx_battery_update(di);
-
-	return IRQ_HANDLED;
-}
-
  static void bq27xxx_battery_poll(struct work_struct *work)
  {
  	struct bq27xxx_device_info *di =

@@ -1061,6 +1052,15 @@ static void bq27xxx_powersupply_unregister(struct bq27xxx_device_info *di)
  static DEFINE_IDR(battery_id);
  static DEFINE_MUTEX(battery_mutex);
 
+static irqreturn_t bq27xxx_battery_irq_handler_thread(int irq, void *data)
+{

drivers/power/bq27xxx_battery.c static void bq27xxx_powersupply_unregister(struct bq27xxx_device_info *di) · line 1066

Quoted 10 lines

drivers/power/bq27xxx_battery.c +7 -0

+	struct bq27xxx_device_info *di = data;
+
+	bq27xxx_battery_update(di);
+
+	return IRQ_HANDLED;
+}
+
  static int bq27xxx_battery_i2c_read(struct bq27xxx_device_info *di, u8 reg,
  				    bool single)
  {

Acked-by Andrew F. Davis <[email protected]>

Signature · 1 line

Andrew F. Davis

Sebastian Reichel

author

2015-10-16 13:44 UTC #7 lore raw

PATCH 3/3 power: bq27xxx_battery: fix defined but not used warnings

In reply to Sebastian Reichel

If BQ27XXX is enabled, but neither I2C based initialization, nor platform based initialization are activated, bq27xxx_powersupply_init and bq27xxx_powersupply_unregister are defined but not used.

This configuration doesn't make sense, but there is no easy way to make it unavailable in the build system, so just mark the functions as __maybe_unused instead.

Signed-off-by Sebastian Reichel <[email protected]>

---
 drivers/power/bq27xxx_battery.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

drivers/power/bq27xxx_battery.c +2 -2

index 880233ce9343..452c9f079e0d 100644
@@ -991,7 +991,7 @@ static void bq27xxx_external_power_changed(struct power_supply *psy)
 	schedule_delayed_work(&di->work, 0);
 }
 
-static int bq27xxx_powersupply_init(struct bq27xxx_device_info *di,
+static int __maybe_unused bq27xxx_powersupply_init(struct bq27xxx_device_info *di,
 				    const char *name)
 {
 	int ret;

@@ -1026,7 +1026,7 @@ static int bq27xxx_powersupply_init(struct bq27xxx_device_info *di,
 	return 0;
 }
 
-static void bq27xxx_powersupply_unregister(struct bq27xxx_device_info *di)
+static void __maybe_unused bq27xxx_powersupply_unregister(struct bq27xxx_device_info *di)
 {
 	/*
 	 * power_supply_unregister call bq27xxx_battery_get_property which

Signature · 1 line

2.6.1

Pali Rohár

2015-10-16 14:47 UTC #8 lore raw

Re: [PATCH 3/3] power: bq27xxx_battery: fix defined but not used warnings

Quoted 8 lines · On Friday 16 October 2015 15:44:12 Sebastian Reichel wrote:

If BQ27XXX is enabled, but neither I2C based initialization, nor platform based initialization are activated, bq27xxx_powersupply_init and bq27xxx_powersupply_unregister are defined but not used.
This configuration doesn't make sense, but there is no easy way to make it unavailable in the build system, so just mark the functions as __maybe_unused instead.

What about?

#if defined(CONFIG_1) || defined(CONFIG_2)
#define NEED_POWER_SUPPLY
#endif

And then wrap power supply code into #ifdef NEED_POWER_SUPPLY?

Signature · 2 lines

Pali Rohár
[email protected]

Andrew F. Davis

2015-10-16 15:01 UTC #9 lore raw

Re: [PATCH 3/3] power: bq27xxx_battery: fix defined but not used warnings

Quoted 10 lines · Pali Rohár

Quoted 8 lines · On Friday 16 October 2015 15:44:12 Sebastian Reichel wrote:
If BQ27XXX is enabled, but neither I2C based initialization, nor platform based initialization are activated, bq27xxx_powersupply_init and bq27xxx_powersupply_unregister are defined but not used.
This configuration doesn't make sense, but there is no easy way to make it unavailable in the build system, so just mark the functions as __maybe_unused instead.

Quoted 8 lines

What about?
#if defined(CONFIG_1) || defined(CONFIG_2)
#define NEED_POWER_SUPPLY
#endif
And then wrap power supply code into #ifdef NEED_POWER_SUPPLY?

We would need to wrap everything in that then, so without I2C or platform enabled we end up compiling an empty file. I think the better fix would be to modify the Kconfig, something like:

config BATTERY_BQ27XXX

	tristate
	depends on (BATTERY_BQ27XXX_I2C || BATTERY_BQ27XXX_PLATFORM)
	default y

config BATTERY_BQ27XXX_I2C
bool "BQ27200/BQ27500 support"
depends on I2C
help
Say Y here to enable support for batteries with BQ27x00 (I2C) chips.

config BATTERY_BQ27XXX_PLATFORM

	bool "BQ27000 support"
	help
	  Say Y here to enable support for batteries with BQ27000 (HDQ) chips.

This would be more in line with how other multi-bus devices handle this issue when nether bus is selected.

Signature · 1 line

Andrew F. Davis

Sebastian Reichel

author

2015-10-19 09:37 UTC #10 lore raw

Re: [PATCH 3/3] power: bq27xxx_battery: fix defined but not used warnings

Quoted 11 lines · Andrew F. Davis

Quoted 10 lines · Pali Rohár
Quoted 8 lines · On Friday 16 October 2015 15:44:12 Sebastian Reichel wrote:
If BQ27XXX is enabled, but neither I2C based initialization, nor platform based initialization are activated, bq27xxx_powersupply_init and bq27xxx_powersupply_unregister are defined but not used.
This configuration doesn't make sense, but there is no easy way to make it unavailable in the build system, so just mark the functions as __maybe_unused instead.

Quoted 7 lines

Quoted 7 lines
What about?
#if defined(CONFIG_1) || defined(CONFIG_2)
#define NEED_POWER_SUPPLY
#endif
And then wrap power supply code into #ifdef NEED_POWER_SUPPLY?

I thought about wrapping the whole code in an ifdef, but I don't think compiling an empty file is any better than adding the __maybe_unused flag.

Quoted 3 lines

We would need to wrap everything in that then, so without I2C or platform enabled we end up compiling an empty file. I think the better fix would be to modify the Kconfig, something like:

Handling this in Kconfig is preferred of course.

Quoted 11 lines

config BATTERY_BQ27XXX
	tristate
	depends on (BATTERY_BQ27XXX_I2C || BATTERY_BQ27XXX_PLATFORM)
	default y
config BATTERY_BQ27XXX_I2C
bool "BQ27200/BQ27500 support"
depends on I2C
help
Say Y here to enable support for batteries with BQ27x00 (I2C) chips.

Quoted 7 lines

config BATTERY_BQ27XXX_PLATFORM
	bool "BQ27000 support"
	help
	  Say Y here to enable support for batteries with BQ27000 (HDQ) chips.
This would be more in line with how other multi-bus devices handle this issue when nether bus is selected.

If I'm not mistaken, this way it's impossible to build the code as module. You need to move the tristate to the individual bus options. This would require to split the code, so that two separate modules are built. I checked the tree and found a few examples:

* building one modules for each bus, core support is autoselected, but always built-in
- BMP085

* building a core modules + one module for each bus
- AD525X_DPOT
- ADE7854
- ADT7316
- INPUT_AD714X
- INPUT_ADXL34X
- TOUCHSCREEN_AD7879
- TOUCHSCREEN_CYTTSP_CORE
- TOUCHSCREEN_CYTTSP4_CORE

Instead of restructuring the whole driver, we could also just built the platform code unconditionally. The platform specific code is just a probe + remove function and bq27xxx_battery_platform_read(). The last one could be dropped, if the pdata->read function is modified (effectively moving the code into the w1 driver).

-- Sebastian

Andrew F. Davis

2015-10-19 16:33 UTC #11 lore raw

Re: [PATCH 3/3] power: bq27xxx_battery: fix defined but not used warnings

Quoted 67 lines · Sebastian Reichel

Quoted 11 lines · Andrew F. Davis
Quoted 10 lines · Pali Rohár
Quoted 8 lines · On Friday 16 October 2015 15:44:12 Sebastian Reichel wrote:
If BQ27XXX is enabled, but neither I2C based initialization, nor platform based initialization are activated, bq27xxx_powersupply_init and bq27xxx_powersupply_unregister are defined but not used.
This configuration doesn't make sense, but there is no easy way to make it unavailable in the build system, so just mark the functions as __maybe_unused instead.
Quoted 7 lines
Quoted 7 lines
What about?
#if defined(CONFIG_1) || defined(CONFIG_2)
#define NEED_POWER_SUPPLY
#endif
And then wrap power supply code into #ifdef NEED_POWER_SUPPLY?
I thought about wrapping the whole code in an ifdef, but I don't think compiling an empty file is any better than adding the __maybe_unused flag.
Quoted 3 lines
We would need to wrap everything in that then, so without I2C or platform enabled we end up compiling an empty file. I think the better fix would be to modify the Kconfig, something like:
Handling this in Kconfig is preferred of course.
Quoted 11 lines
config BATTERY_BQ27XXX
	tristate
	depends on (BATTERY_BQ27XXX_I2C || BATTERY_BQ27XXX_PLATFORM)
	default y
config BATTERY_BQ27XXX_I2C
bool "BQ27200/BQ27500 support"
depends on I2C
help
Say Y here to enable support for batteries with BQ27x00 (I2C) chips.
Quoted 7 lines
config BATTERY_BQ27XXX_PLATFORM
	bool "BQ27000 support"
	help
	  Say Y here to enable support for batteries with BQ27000 (HDQ) chips.
This would be more in line with how other multi-bus devices handle this issue when nether bus is selected.
If I'm not mistaken, this way it's impossible to build the code as module. You need to move the tristate to the individual bus options. This would require to split the code, so that two separate modules are built. I checked the tree and found a few examples:
* building one modules for each bus, core support is autoselected, but always built-in
- BMP085
* building a core modules + one module for each bus
- AD525X_DPOT
- ADE7854
- ADT7316
- INPUT_AD714X
- INPUT_ADXL34X
- TOUCHSCREEN_AD7879
- TOUCHSCREEN_CYTTSP_CORE
- TOUCHSCREEN_CYTTSP4_CORE

Quoted 8 lines

Instead of restructuring the whole driver, we could also just built the platform code unconditionally. The platform specific code is just a probe + remove function and bq27xxx_battery_platform_read(). The last one could be dropped, if the pdata->read function is modified (effectively moving the code into the w1 driver).
-- Sebastian

A quick test shows the following Kconfig to work as desired, selecting one or both buses as modules builds the driver as a module, if ether are selected as built-in the whole driver is built-in.

config BATTERY_BQ27XXX

	tristate
	depends on (BATTERY_BQ27XXX_I2C || BATTERY_BQ27XXX_PLATFORM)
	default y

config BATTERY_BQ27XXX_I2C
tristate "BQ27xxx I2C support"
depends on I2C
help
Say Y here to enable support for batteries with BQ27xxx (I2C) chips.

config BATTERY_BQ27XXX_PLATFORM
tristate "BQ27xxx HDQ support"
help
Say Y here to enable support for batteries with BQ27xxx (HDQ) chips.

We would then have to change:
#ifdef CONFIG_BATTERY_BQ27XXX_I2C

#if defined(CONFIG_BATTERY_BQ27XXX_I2C) || defined(CONFIG_BATTERY_BQ27XXX_I2C_MODULE)

(same for CONFIG_BATTERY_BQ27XXX_PLATFORM)

Signature · 1 line

Andrew F. Davis