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), butAndrew 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
- signature.asc [application/pgp-signature] 833 bytes