From 993cc6a5149e0e6b25b5f094b6be89315889121e Mon Sep 17 00:00:00 2001 From: Subbaraman Narayanamurthy Date: Wed, 18 Jan 2017 20:10:51 -0800 Subject: qcom: fg-memif: Return error properly when IMA read/write fails In the SRAM masked write we read the register first, make modification to the value and write it back. If the read fails we bail out of the masked write function. However there is a bug in the read api where the error code is not properly returned. This causes the masked write to proceed with a failed read and corruption ensues. A similar bug is present in write api too where the error is not returned correctly to the caller. Fix this. Change-Id: Ic7651c5cb2e054a0b8b2dfe201463063ce9e167b Signed-off-by: Subbaraman Narayanamurthy --- drivers/power/supply/qcom/fg-memif.c | 27 +++++++++++++++++---------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/drivers/power/supply/qcom/fg-memif.c b/drivers/power/supply/qcom/fg-memif.c index a98ff7d765e3..e66adb956ad2 100644 --- a/drivers/power/supply/qcom/fg-memif.c +++ b/drivers/power/supply/qcom/fg-memif.c @@ -1,4 +1,4 @@ -/* Copyright (c) 2016, The Linux Foundation. All rights reserved. +/* Copyright (c) 2016-2017, 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 @@ -544,7 +544,7 @@ static int fg_get_beat_count(struct fg_chip *chip, u8 *count) int fg_interleaved_mem_read(struct fg_chip *chip, u16 address, u8 offset, u8 *val, int len) { - int rc = 0; + int rc = 0, ret; u8 start_beat_count, end_beat_count, count = 0; bool retry_once = false; @@ -597,13 +597,17 @@ retry: } out: /* Release IMA access */ - rc = fg_masked_write(chip, MEM_IF_MEM_INTF_CFG(chip), + ret = fg_masked_write(chip, MEM_IF_MEM_INTF_CFG(chip), MEM_ACCESS_REQ_BIT | IACS_SLCT_BIT, 0); - if (rc < 0) { - pr_err("failed to reset IMA access bit rc = %d\n", rc); - return rc; + if (ret < 0) { + pr_err("failed to reset IMA access bit ret = %d\n", ret); + return ret; } + /* Return the error we got before releasing memory access */ + if (rc < 0) + return rc; + if (retry_once) { retry_once = false; goto retry; @@ -615,7 +619,7 @@ out: int fg_interleaved_mem_write(struct fg_chip *chip, u16 address, u8 offset, u8 *val, int len, bool atomic_access) { - int rc = 0; + int rc = 0, ret; u8 start_beat_count, end_beat_count, count = 0; if (offset > 3) { @@ -662,11 +666,14 @@ retry: start_beat_count, end_beat_count); out: /* Release IMA access */ - rc = fg_masked_write(chip, MEM_IF_MEM_INTF_CFG(chip), + ret = fg_masked_write(chip, MEM_IF_MEM_INTF_CFG(chip), MEM_ACCESS_REQ_BIT | IACS_SLCT_BIT, 0); - if (rc < 0) - pr_err("failed to reset IMA access bit rc = %d\n", rc); + if (ret < 0) { + pr_err("failed to reset IMA access bit ret = %d\n", ret); + return ret; + } + /* Return the error we got before releasing memory access */ return rc; } -- cgit v1.2.3 From 234089eca072e3813727baffdb0573c6336a0b64 Mon Sep 17 00:00:00 2001 From: Subbaraman Narayanamurthy Date: Fri, 20 Jan 2017 18:26:25 -0800 Subject: qcom: fg-memif: improve retry mechanism for IMA read/write FG SRAM read/write APIs consist of peripheral/supporting read/writes and the core interleaved_mem_access functions. The current implementation retries only when the core functions return an error and in case of a read, additionally when the beat count changes. This works except if the peripheral/supporting access fail, we do not even retry. Introduce retries for all possible failures while reading/writing a SRAM location. Change-Id: I99ad9acae3ef0dbc3941094076f124d16099468c Signed-off-by: Subbaraman Narayanamurthy --- drivers/power/supply/qcom/fg-memif.c | 59 +++++++++++++++++++++++++++--------- 1 file changed, 44 insertions(+), 15 deletions(-) diff --git a/drivers/power/supply/qcom/fg-memif.c b/drivers/power/supply/qcom/fg-memif.c index e66adb956ad2..2dc76182ed15 100644 --- a/drivers/power/supply/qcom/fg-memif.c +++ b/drivers/power/supply/qcom/fg-memif.c @@ -289,7 +289,7 @@ static int fg_check_iacs_ready(struct fg_chip *chip) } if (!tries) { - pr_err("IACS_RDY not set\n"); + pr_err("IACS_RDY not set, opr_sts: %d\n", ima_opr_sts); /* check for error condition */ rc = fg_clear_ima_errors_if_any(chip, false); if (rc < 0) { @@ -546,7 +546,7 @@ int fg_interleaved_mem_read(struct fg_chip *chip, u16 address, u8 offset, { int rc = 0, ret; u8 start_beat_count, end_beat_count, count = 0; - bool retry_once = false; + bool retry = false; if (offset > 3) { pr_err("offset too large %d\n", offset); @@ -554,10 +554,18 @@ int fg_interleaved_mem_read(struct fg_chip *chip, u16 address, u8 offset, } retry: + if (count >= RETRY_COUNT) { + pr_err("Tried %d times\n", RETRY_COUNT); + retry = false; + goto out; + } + rc = fg_interleaved_mem_config(chip, val, address, offset, len, FG_READ); if (rc < 0) { pr_err("failed to configure SRAM for IMA rc = %d\n", rc); + count++; + retry = true; goto out; } @@ -565,18 +573,21 @@ retry: rc = fg_get_beat_count(chip, &start_beat_count); if (rc < 0) { pr_err("failed to read beat count rc=%d\n", rc); + count++; + retry = true; goto out; } /* read data */ rc = __fg_interleaved_mem_read(chip, address, offset, val, len); if (rc < 0) { - if ((rc == -EAGAIN) && (count < RETRY_COUNT)) { - count++; + count++; + if (rc == -EAGAIN) { pr_err("IMA access failed retry_count = %d\n", count); goto retry; } pr_err("failed to read SRAM address rc = %d\n", rc); + retry = true; goto out; } @@ -584,32 +595,31 @@ retry: rc = fg_get_beat_count(chip, &end_beat_count); if (rc < 0) { pr_err("failed to read beat count rc=%d\n", rc); + count++; + retry = true; goto out; } fg_dbg(chip, FG_SRAM_READ, "Start beat_count = %x End beat_count = %x\n", start_beat_count, end_beat_count); - if (start_beat_count != end_beat_count && !retry_once) { + if (start_beat_count != end_beat_count) { fg_dbg(chip, FG_SRAM_READ, "Beat count(%d/%d) do not match - retry transaction\n", start_beat_count, end_beat_count); - retry_once = true; + count++; + retry = true; } out: /* Release IMA access */ ret = fg_masked_write(chip, MEM_IF_MEM_INTF_CFG(chip), MEM_ACCESS_REQ_BIT | IACS_SLCT_BIT, 0); - if (ret < 0) { + if (rc < 0 && ret < 0) { pr_err("failed to reset IMA access bit ret = %d\n", ret); return ret; } - /* Return the error we got before releasing memory access */ - if (rc < 0) - return rc; - - if (retry_once) { - retry_once = false; + if (retry) { + retry = false; goto retry; } @@ -621,6 +631,7 @@ int fg_interleaved_mem_write(struct fg_chip *chip, u16 address, u8 offset, { int rc = 0, ret; u8 start_beat_count, end_beat_count, count = 0; + bool retry = false; if (offset > 3) { pr_err("offset too large %d\n", offset); @@ -628,10 +639,18 @@ int fg_interleaved_mem_write(struct fg_chip *chip, u16 address, u8 offset, } retry: + if (count >= RETRY_COUNT) { + pr_err("Tried %d times\n", RETRY_COUNT); + retry = false; + goto out; + } + rc = fg_interleaved_mem_config(chip, val, address, offset, len, FG_WRITE); if (rc < 0) { pr_err("failed to configure SRAM for IMA rc = %d\n", rc); + count++; + retry = true; goto out; } @@ -639,18 +658,21 @@ retry: rc = fg_get_beat_count(chip, &start_beat_count); if (rc < 0) { pr_err("failed to read beat count rc=%d\n", rc); + count++; + retry = true; goto out; } /* write data */ rc = __fg_interleaved_mem_write(chip, address, offset, val, len); if (rc < 0) { + count++; if ((rc == -EAGAIN) && (count < RETRY_COUNT)) { - count++; pr_err("IMA access failed retry_count = %d\n", count); goto retry; } pr_err("failed to write SRAM address rc = %d\n", rc); + retry = true; goto out; } @@ -658,6 +680,8 @@ retry: rc = fg_get_beat_count(chip, &end_beat_count); if (rc < 0) { pr_err("failed to read beat count rc=%d\n", rc); + count++; + retry = true; goto out; } @@ -668,11 +692,16 @@ out: /* Release IMA access */ ret = fg_masked_write(chip, MEM_IF_MEM_INTF_CFG(chip), MEM_ACCESS_REQ_BIT | IACS_SLCT_BIT, 0); - if (ret < 0) { + if (rc < 0 && ret < 0) { pr_err("failed to reset IMA access bit ret = %d\n", ret); return ret; } + if (retry) { + retry = false; + goto retry; + } + /* Return the error we got before releasing memory access */ return rc; } -- cgit v1.2.3