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: <20200612141611.GI5396@sirena.org.uk>
Date:   Fri, 12 Jun 2020 15:16:11 +0100
From:   Mark Brown <broonie@...nel.org>
To:     Robin Gong <yibin.gong@....com>
Cc:     Vinod Koul <vkoul@...nel.org>,
        "shawnguo@...nel.org" <shawnguo@...nel.org>,
        "s.hauer@...gutronix.de" <s.hauer@...gutronix.de>,
        "festevam@...il.com" <festevam@...il.com>,
        "robin.murphy@....com" <robin.murphy@....com>,
        "matthias.schiffer@...tq-group.com" 
        <matthias.schiffer@...tq-group.com>,
        "kernel@...gutronix.de" <kernel@...gutronix.de>,
        dl-linux-imx <linux-imx@....com>,
        "linux-spi@...r.kernel.org" <linux-spi@...r.kernel.org>,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v1 RFC 1/2] spi: introduce fallback to pio

On Fri, Jun 12, 2020 at 01:48:41PM +0000, Robin Gong wrote:
> On 2020/06/12 18:14 Mark Brown <broonie@...nel.org> wrote: 

> > Please look at the formatting of your e-mails - they're really hard to read.  The
> > line length is over 80 columns and there's no breaks between paragraphs.

> Sorry for that, seems my outlook format issue, hope it's ok now this time :)

Yes, looks good thanks!

> > Client could enable this feature by choosing SPI_MASTER_FALLBACK freely
> > without any impact on others.

> > SPI_MASTER_FALLBACK.  If this works why would any driver not enable the
> > flag?

> Just make sure little impact if it's not good enough and potential issue may
> come out after it's merged into mainline. TBH, I'm not sure if it has taken
> care all in spi core. Besides, I don't know if other spi client need this feature.

It's not something that's going to come up a lot for most devices, it'd
be a mapping failure due to running out of memory or something, but your
point about that being possible is valid.

> > > Any error happen in DMA could fallback to PIO , seems a nice to have,
> > because it could
> > > give chance to run in PIO which is more reliable. But if there is also error in

> > PIO, thus may loop here, it's better adding limit try times here?

> > An error doesn't mean nothing happened on the bus, an error could for
> > example also be something like a FIFO overrun which corrupts data.

> Do you mean fallback to PIO may cause FIFO overrun since some latency
> involved so that this patch seems not useful as expected?

No, I mean that the reason the DMA transfer fails may be something that
happens after we've started putting things on the bus - the bit about
FIFOs is just a random example of an error that could happen.

> > It *could* but only in extreme situations, and again this isn't just handling
> > errors from failure to prepare the hardware but also anything that happens
> > after it.

> Okay, understood your point. You prefer to some interface provided by dma
> engine before dmaengine_prep_slave_sg so that can_dma() can know if
> this dma channel is ready indeed. But unfortunately, seems there is no one....

Well, this is free software and everything can be modified!  The other
option would be framework changes in SPI that allowed us to indicate
from the driver that an error occured before we started doing anything
to the hardware (like happens here) through something like a special
error code or splitting up the API.

Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ