From db1faf0c32adc0d124d532a09c68563d42970b62 Mon Sep 17 00:00:00 2001 From: Gilad Avidov Date: Tue, 13 May 2014 17:40:30 -0600 Subject: [PATCH] spmi-pmic-arb: improve error reporting Add additional information on error return code and log dump. Introduce a new return code for transient failures and add to log dump information such as: slave-id, address, op-code, byte-count, data buffer content, and values of relevant registers. Change-Id: I6ac785cf78a84a448f96057b93c94df0b72e39ce Signed-off-by: Gilad Avidov Signed-off-by: Alok Chauhan --- drivers/spmi/spmi-pmic-arb.c | 108 ++++++++++++++++++++++++++--------- include/linux/spmi.h | 6 +- 2 files changed, 85 insertions(+), 29 deletions(-) diff --git a/drivers/spmi/spmi-pmic-arb.c b/drivers/spmi/spmi-pmic-arb.c index e3284d5ac9e..81640a07289 100644 --- a/drivers/spmi/spmi-pmic-arb.c +++ b/drivers/spmi/spmi-pmic-arb.c @@ -33,7 +33,9 @@ /* PMIC Arbiter configuration registers */ #define PMIC_ARB_VERSION 0x0000 #define PMIC_ARB_INT_EN 0x0004 - +#define PMIC_ARB_PROTOCOL_IRQ_STATUS (0x700 + 0x820) +#define PMIC_ARB_GENI_CTRL 0x0024 +#define PMIC_ARB_GENI_STATUS 0x0028 /* PMIC Arbiter channel registers */ #define PMIC_ARB_CMD(N) (0x0800 + (0x80 * (N))) #define PMIC_ARB_CONFIG(N) (0x0804 + (0x80 * (N))) @@ -125,6 +127,7 @@ struct spmi_pmic_arb_dev { u8 max_apid; u16 periph_id_map[PMIC_ARB_MAX_PERIPHS]; u32 mapping_table[SPMI_MAPPING_TABLE_LEN]; + u32 prev_prtcl_irq_stat; }; static struct spmi_pmic_arb_dev *the_pmic_arb; @@ -143,6 +146,37 @@ static void pmic_arb_write(struct spmi_pmic_arb_dev *dev, u32 offset, u32 val) writel_relaxed(val, dev->base + offset); } +static void pmic_arb_save_stat_before_txn(struct spmi_pmic_arb_dev *dev) +{ + dev->prev_prtcl_irq_stat = + readl_relaxed(dev->cnfg + PMIC_ARB_PROTOCOL_IRQ_STATUS); +} + +static int pmic_arb_diagnosis(struct spmi_pmic_arb_dev *dev, u32 status) +{ + if (status & PMIC_ARB_STATUS_DENIED) { + dev_err(dev->dev, + "wait_for_done: transaction denied by SPMI master (0x%x)\n", + status); + return -EPERM; + } + + if (status & PMIC_ARB_STATUS_FAILURE) { + dev_err(dev->dev, + "wait_for_done: transaction failed (0x%x)\n", status); + return -EIO; + } + + if (status & PMIC_ARB_STATUS_DROPPED) { + dev_err(dev->dev, + "wait_for_done: transaction dropped pmic-arb busy (0x%x)\n", + status); + return -EAGAIN; + } + + return 0; +} + static int pmic_arb_wait_for_done(struct spmi_pmic_arb_dev *dev) { u32 status = 0; @@ -152,34 +186,13 @@ static int pmic_arb_wait_for_done(struct spmi_pmic_arb_dev *dev) while (timeout--) { status = pmic_arb_read(dev, offset); - if (status & PMIC_ARB_STATUS_DONE) { - if (status & PMIC_ARB_STATUS_DENIED) { - dev_err(dev->dev, - "%s: transaction denied (0x%x)\n", - __func__, status); - return -EPERM; - } + if (status & PMIC_ARB_STATUS_DONE) + return pmic_arb_diagnosis(dev, status); - if (status & PMIC_ARB_STATUS_FAILURE) { - dev_err(dev->dev, - "%s: transaction failed (0x%x)\n", - __func__, status); - return -EIO; - } - - if (status & PMIC_ARB_STATUS_DROPPED) { - dev_err(dev->dev, - "%s: transaction dropped (0x%x)\n", - __func__, status); - return -EIO; - } - - return 0; - } udelay(1); } - dev_err(dev->dev, "%s: timeout, status 0x%x\n", __func__, status); + dev_err(dev->dev, "wait_for_done:: timeout, status 0x%x\n", status); return -ETIMEDOUT; } @@ -209,6 +222,29 @@ pa_write_data(struct spmi_pmic_arb_dev *dev, u8 *buf, u32 reg, u8 bc) pmic_arb_write(dev, reg, data); } +static void pmic_arb_dbg_err_dump(struct spmi_pmic_arb_dev *pmic_arb, int ret, + const char *msg, u8 opc, u8 sid, u16 addr, u8 bc, u8 *buf) +{ + u32 irq_stat = readl_relaxed(pmic_arb->cnfg + + PMIC_ARB_PROTOCOL_IRQ_STATUS); + u32 geni_stat = readl_relaxed(pmic_arb->cnfg + PMIC_ARB_GENI_STATUS); + u32 geni_ctrl = readl_relaxed(pmic_arb->cnfg + PMIC_ARB_GENI_CTRL); + + bc += 1; /* actual byte count */ + + if (buf) + dev_err(pmic_arb->dev, + "error:%d on data %s opcode:0x%x sid:%d addr:0x%x bc:%d buf:%*phC\n", + ret, msg, opc, sid, addr, bc, bc, buf); + else + dev_err(pmic_arb->dev, + "error:%d on non-data-cmd opcode:0x%x sid:%d\n", + ret, opc, sid); + dev_err(pmic_arb->dev, + "PROTOCOL_IRQ_STATUS before:0x%x after:0x%x GENI_STATUS:0x%x GENI_CTRL:0x%x\n", + irq_stat, pmic_arb->prev_prtcl_irq_stat, geni_stat, geni_ctrl); +} + /* Non-data command */ static int pmic_arb_cmd(struct spmi_controller *ctrl, u8 opc, u8 sid) { @@ -228,10 +264,13 @@ static int pmic_arb_cmd(struct spmi_controller *ctrl, u8 opc, u8 sid) cmd = (opc << 27) | ((sid & 0xf) << 20); spin_lock_irqsave(&pmic_arb->lock, flags); + pmic_arb_save_stat_before_txn(pmic_arb); pmic_arb_write(pmic_arb, PMIC_ARB_CMD(pmic_arb->channel), cmd); rc = pmic_arb_wait_for_done(pmic_arb); spin_unlock_irqrestore(&pmic_arb->lock, flags); + if (rc) + pmic_arb_dbg_err_dump(pmic_arb, rc, "cmd", opc, sid, 0, 0, 0); return rc; } @@ -249,7 +288,8 @@ static int pmic_arb_read_cmd(struct spmi_controller *ctrl, , PMIC_ARB_MAX_TRANS_BYTES, bc+1); return -EINVAL; } - pr_debug("op:0x%x sid:%d bc:%d addr:0x%x\n", opc, sid, bc, addr); + dev_dbg(pmic_arb->dev, "client-rd op:0x%x sid:%d addr:0x%x bc:%d\n", + opc, sid, addr, bc + 1); /* Check the opcode */ if (opc >= 0x60 && opc <= 0x7F) @@ -264,6 +304,7 @@ static int pmic_arb_read_cmd(struct spmi_controller *ctrl, cmd = (opc << 27) | ((sid & 0xf) << 20) | (addr << 4) | (bc & 0x7); spin_lock_irqsave(&pmic_arb->lock, flags); + pmic_arb_save_stat_before_txn(pmic_arb); pmic_arb_write(pmic_arb, PMIC_ARB_CMD(pmic_arb->channel), cmd); rc = pmic_arb_wait_for_done(pmic_arb); if (rc) @@ -279,6 +320,9 @@ static int pmic_arb_read_cmd(struct spmi_controller *ctrl, done: spin_unlock_irqrestore(&pmic_arb->lock, flags); + if (rc) + pmic_arb_dbg_err_dump(pmic_arb, rc, "read", opc, sid, addr, bc, + buf); return rc; } @@ -296,7 +340,8 @@ static int pmic_arb_write_cmd(struct spmi_controller *ctrl, , PMIC_ARB_MAX_TRANS_BYTES, bc+1); return -EINVAL; } - pr_debug("op:0x%x sid:%d bc:%d addr:0x%x\n", opc, sid, bc, addr); + dev_dbg(pmic_arb->dev, "client-wr op:0x%x sid:%d addr:0x%x bc:%d\n", + opc, sid, addr, bc + 1); /* Check the opcode */ if (opc >= 0x40 && opc <= 0x5F) @@ -314,6 +359,7 @@ static int pmic_arb_write_cmd(struct spmi_controller *ctrl, /* Write data to FIFOs */ spin_lock_irqsave(&pmic_arb->lock, flags); + pmic_arb_save_stat_before_txn(pmic_arb); pa_write_data(pmic_arb, buf, PMIC_ARB_WDATA0(pmic_arb->channel) , min_t(u8, bc, 3)); if (bc > 3) @@ -325,6 +371,10 @@ static int pmic_arb_write_cmd(struct spmi_controller *ctrl, rc = pmic_arb_wait_for_done(pmic_arb); spin_unlock_irqrestore(&pmic_arb->lock, flags); + if (rc) + pmic_arb_dbg_err_dump(pmic_arb, rc, "write", opc, sid, addr, bc, + buf); + return rc; } @@ -501,7 +551,9 @@ periph_interrupt(struct spmi_pmic_arb_dev *pmic_arb, u8 apid, bool show) int i; if (!is_apid_valid(pmic_arb, apid)) { - dev_err(pmic_arb->dev, "unknown peripheral id 0x%x\n", ppid); + dev_err(pmic_arb->dev, + "periph_interrupt(apid:0x%x sid:0x%x pid:0x%x) unknown peripheral\n", + apid, sid, pid); /* return IRQ_NONE; */ } diff --git a/include/linux/spmi.h b/include/linux/spmi.h index e8e932eb066..b581de80bba 100644 --- a/include/linux/spmi.h +++ b/include/linux/spmi.h @@ -1,4 +1,4 @@ -/* Copyright (c) 2012-2013, The Linux Foundation. All rights reserved. +/* Copyright (c) 2012-2014 The Linux Foundation. All rights reserved. * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License version 2 and @@ -382,6 +382,7 @@ extern int spmi_ext_register_writel(struct spmi_controller *ctrl, * -EPERM if the SPMI transaction is denied due to permission issues. * -EIO if the SPMI transaction fails (parity errors, etc). * -ETIMEDOUT if the SPMI transaction times out. + * -EAGAIN if the SPMI transaction is temporarily unavailable */ extern int spmi_command_reset(struct spmi_controller *ctrl, u8 sid); @@ -397,6 +398,7 @@ extern int spmi_command_reset(struct spmi_controller *ctrl, u8 sid); * -EPERM if the SPMI transaction is denied due to permission issues. * -EIO if the SPMI transaction fails (parity errors, etc). * -ETIMEDOUT if the SPMI transaction times out. + * -EAGAIN if the SPMI transaction is temporarily unavailable */ extern int spmi_command_sleep(struct spmi_controller *ctrl, u8 sid); @@ -413,6 +415,7 @@ extern int spmi_command_sleep(struct spmi_controller *ctrl, u8 sid); * -EPERM if the SPMI transaction is denied due to permission issues. * -EIO if the SPMI transaction fails (parity errors, etc). * -ETIMEDOUT if the SPMI transaction times out. + * -EAGAIN if the SPMI transaction is temporarily unavailable */ extern int spmi_command_wakeup(struct spmi_controller *ctrl, u8 sid); @@ -428,6 +431,7 @@ extern int spmi_command_wakeup(struct spmi_controller *ctrl, u8 sid); * -EPERM if the SPMI transaction is denied due to permission issues. * -EIO if the SPMI transaction fails (parity errors, etc). * -ETIMEDOUT if the SPMI transaction times out. + * -EAGAIN if the SPMI transaction is temporarily unavailable */ extern int spmi_command_shutdown(struct spmi_controller *ctrl, u8 sid);