linux-kernel - RE: [PATCH 1/2] i2c: qup: Cleared the error bits in ISR

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 linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <000901d1abf6$0f3ebf50$2dbc3df0$@codeaurora.org>
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