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: <20160512193229.GA10499@chewinlnx07.qualcomm.com>
Date:	Fri, 13 May 2016 01:02:29 +0530
From:	Abhishek Sahu <absahu@...eaurora.org>
To:	Andy Gross <andy.gross@...aro.org>
Cc:	Sricharan <sricharan@...eaurora.org>, agross@...eaurora.org,
	architt@...eaurora.org, linux-arm-msm@...r.kernel.org,
	ntelkar@...eaurora.org, linux-kernel@...r.kernel.org,
	linux-i2c@...r.kernel.org, dmaengine@...r.kernel.org,
	linux-arm-kernel@...ts.infradead.org, wsa@...-dreams.de
Subject: Re: [PATCH 1/2] i2c: qup: Cleared the error bits in ISR

On Thu, May 12, 2016 at 12:58:30PM -0500, Andy Gross wrote:
> On Thu, May 12, 2016 at 11:48:43AM +0530, Abhishek Sahu wrote:
> > On Thu, May 12, 2016 at 12:13:47AM -0500, Andy Gross wrote:
> > > On Wed, May 11, 2016 at 11:04:17PM +0530, Abhishek Sahu wrote:
> > > 
> > > <snip>
> > > 
> > > > >       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.
> > > 
> > > It'd be better to not reset the QUP inside the ISR at all.  I think the better
> > > solution is making the reset occur in the xfer function.  So just clear the bits
> > > like you should in the isr, and defer reset till later.
> > 
> > This is a HW workaround for QUP where we need to RESET the core in
> > ISR. When interrupt is level triggerd, more than one interrupt may
> > fire in error cases and QUP will generate these interrupts after
> > completion of current interrupt. Thus we reset the core immediately
> > in the ISR to ward off the next interrupt.
> 
> I wonder if that was just an expedient solution.  And it seems like it could be
> a little racy as a poll is not being done to make sure the QUP state machine
> transitions to reset.  Is there any documentation/errata on this?  Or is this
> just from commit comments?
> 
> 
> Regards,
> Andy

Hi Andy,

Thanks for your comments. We have only added the patch for clearing
the error bits. This resetting of QUP is present in the base code
itself. Since this QUP is being used by all the QCOM chips that's why
we have not changed the existing QUP reset logic. We will look
into reset polling logic separately and will post the patch for the
same.

We got the input for QUP RESET by referring our working internal
drivers and we also faced the BAM hang issue in base upstream
driver, if we don't do this RESET. If we schedule any transfer
with DMA mode then the DMA Engine (BAM) will never get any interrupt
for these errors, since the QUP interrupt handles the error cases.
The BAM will generate the interrupt only when it will receive the
requested amount of data. This reset will clear the QUP FIFO and
we will get the BAM transfer completion interrupt by the existing
logic present in the base bam transfer function.

-- 
Abhishek Sahu