lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Thu, 12 May 2016 07:58:59 +0530 From: "Sricharan" <sricharan@...eaurora.org> To: "'Abhishek Sahu'" <absahu@...eaurora.org> Cc: <architt@...eaurora.org>, <wsa@...-dreams.de>, <linux-arm-msm@...r.kernel.org>, <ntelkar@...eaurora.org>, <linux-kernel@...r.kernel.org>, <dmaengine@...r.kernel.org>, <linux-i2c@...r.kernel.org>, <agross@...eaurora.org>, <andy.gross@...aro.org>, <linux-arm-kernel@...ts.infradead.org> Subject: RE: [PATCH 1/2] i2c: qup: Cleared the error bits in ISR Hi, > -----Original Message----- > From: linux-arm-kernel [mailto:linux-arm-kernel- > bounces@...ts.infradead.org] On Behalf Of Abhishek Sahu > Sent: Wednesday, May 11, 2016 11:04 PM > To: Sricharan <sricharan@...eaurora.org> > Cc: architt@...eaurora.org; wsa@...-dreams.de; linux-arm- > msm@...r.kernel.org; ntelkar@...eaurora.org; linux- > kernel@...r.kernel.org; dmaengine@...r.kernel.org; linux- > i2c@...r.kernel.org; agross@...eaurora.org; andy.gross@...aro.org; linux- > arm-kernel@...ts.infradead.org > Subject: RE: [PATCH 1/2] i2c: qup: Cleared the error bits in ISR > > On 2016-05-11 21:27, Sricharan wrote: > > Hi, > > > >> 1. Current QCOM I2C driver hangs when sending data to address > >> 0x03-0x07 > >> in some scenarios. The QUP controller generates invalid write in this > >> case, since these addresses are reserved for different bus formats. > >> > >> 2. Also, the error handling is done by I2C QUP ISR in the case of DMA > >> mode. > >> The state need to be RESET in case of any error for clearing the > >> available data in FIFO, which otherwise leaves the BAM DMA controller > >> in hang state. > >> > >> This patch fixes the above two issues by clearing the error bits from > >> I2C and QUP status in ISR in case of I2C error, QUP error and resets > >> the QUP state to clear the FIFO data. > >> > >> Signed-off-by: Abhishek Sahu <absahu@...eaurora.org> > >> --- > >> drivers/i2c/busses/i2c-qup.c | 14 ++++++++------ > >> 1 file changed, 8 insertions(+), 6 deletions(-) > >> > >> diff --git a/drivers/i2c/busses/i2c-qup.c > >> b/drivers/i2c/busses/i2c-qup.c index 23eaabb..8c2f1bc 100644 > >> --- a/drivers/i2c/busses/i2c-qup.c > >> +++ b/drivers/i2c/busses/i2c-qup.c > >> @@ -213,14 +213,16 @@ static irqreturn_t qup_i2c_interrupt(int irq, > >> void > >> *dev) > >> bus_err &= I2C_STATUS_ERROR_MASK; > >> qup_err &= QUP_STATUS_ERROR_FLAGS; > >> > >> - if (qup_err) { > >> - /* Clear Error interrupt */ > >> + /* Clear the error bits in QUP_ERROR_FLAGS */ > >> + if (qup_err) > >> writel(qup_err, qup->base + QUP_ERROR_FLAGS); > >> - goto done; > >> - } > >> > >> - if (bus_err) { > >> - /* Clear Error interrupt */ > >> + /* Clear the error bits in QUP_I2C_STATUS */ > >> + if (bus_err) > >> + writel(bus_err, qup->base + QUP_I2C_STATUS); > >> + > >> + /* Reset the QUP State in case of error */ > >> + if (qup_err || bus_err) { > >> writel(QUP_RESET_STATE, qup->base + QUP_STATE); > >> goto done; > >> } > > In qup_i2c_xfer and qup_i2c_xfer_v2 state is set to RESET at > > the end, when > > there is no error. So would it be fine if we do it there > > unconditionally ? > > > > Regards, > > Sricharan > > RESET the QUP state wouldn't create any issue in the case of multiple calls. > The existing code also RESET the QUP state for bus_err but it is not clearing > status bits. So is there a difference that you see by setting it in isr for qup errors ? Otherwise, its better to set it in one place unconditionally instead of doing it in two places ? Regards, Sricharan
Powered by blists - more mailing lists