Thread (32 messages) 32 messages, 5 authors, 2017-05-01

Re: [PATCH v11 05/10] power: bq27xxx_battery: Make error reporting consistent

From: Sebastian Reichel <sre@kernel.org>
Date: 2017-03-23 10:33:20

Hi,

On Thu, Mar 23, 2017 at 03:24:40AM -0700, Liam Breck wrote:
On Thu, Mar 23, 2017 at 3:11 AM, Sebastian Reichel [off-list ref] wrote:
quoted
On Mon, Mar 20, 2017 at 10:59:18AM -0700, Liam Breck wrote:
quoted
On Mon, Mar 20, 2017 at 9:57 AM, Andrew F. Davis [off-list ref] wrote:
quoted
On 03/20/2017 04:43 AM, Liam Breck wrote:
quoted
From: Liam Breck <redacted>

Previously errors were reported in a mix of styles via both dev_err() & dev_dbg().
Report all errors with dev_err() and include error code in message.
Report register ID separately in dev_dbg() message.

Signed-off-by: Liam Breck <redacted>
---
 drivers/power/supply/bq27xxx_battery.c | 35 ++++++++++++++++++----------------
 1 file changed, 19 insertions(+), 16 deletions(-)
diff --git a/drivers/power/supply/bq27xxx_battery.c b/drivers/power/supply/bq27xxx_battery.c
index 398801a..2e2a3a8 100644
--- a/drivers/power/supply/bq27xxx_battery.c
+++ b/drivers/power/supply/bq27xxx_battery.c
@@ -794,11 +794,17 @@ MODULE_PARM_DESC(poll_interval,
 static inline int bq27xxx_read(struct bq27xxx_device_info *di, int reg_index,
                             bool single)
 {
-     /* Reports EINVAL for invalid/missing registers */
+     int ret;
+
      if (!di || di->regs[reg_index] == INVALID_REG_ADDR)
              return -EINVAL;

-     return di->bus.read(di, di->regs[reg_index], single);
+     ret = di->bus.read(di, di->regs[reg_index], single);
+     if (ret < 0)
+             dev_dbg(di->dev, "failed to read register 0x%02x (index %d)\n",
+                     di->regs[reg_index], reg_index);
+
If we print out the error code here we don't have to do that in each of
the below printouts.
This is a dev_dbg, the below are dev_err, and I think you want error
code in dev_err. BTW the above addresses your request to learn which
register was in play in the event of a DM read/write failure.
Just make it dev_err and also print ret. Then we can remove all
other prints. That will result in some information-loss (name
of the register), but
Andrew specifically asked for error reporting that gives the error
context elsewhere in the patchset. In complying with that request, I
filed this patch.

We could pass a context string into read() ...
That's fine with me.

-- Sebastian

Attachments

Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help