Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   Robin Gong <yibin.gong@nxp.com>
To:     Mark Brown <broonie@kernel.org>
CC:     Vinod Koul <vkoul@kernel.org>,
        "shawnguo@kernel.org" <shawnguo@kernel.org>,
        "s.hauer@pengutronix.de" <s.hauer@pengutronix.de>,
        "festevam@gmail.com" <festevam@gmail.com>,
        "robin.murphy@arm.com" <robin.murphy@arm.com>,
        "matthias.schiffer@ew.tq-group.com" 
        <matthias.schiffer@ew.tq-group.com>,
        "kernel@pengutronix.de" <kernel@pengutronix.de>,
        dl-linux-imx <linux-imx@nxp.com>,
        "linux-spi@vger.kernel.org" <linux-spi@vger.kernel.org>,
        "linux-arm-kernel@lists.infradead.org" 
        <linux-arm-kernel@lists.infradead.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: RE: [PATCH v1 RFC 1/2] spi: introduce fallback to pio
Thread-Topic: [PATCH v1 RFC 1/2] spi: introduce fallback to pio
Thread-Index: AQHWP6z7CX2etPvPnk+oddH8JEW5FKjTbHIAgADDYUCAAJUwgIAACjoQgAA5dICAAOmA8A==
Date:   Sun, 14 Jun 2020 13:04:57 +0000
Message-ID: <VE1PR04MB6638B43E3AC83286946DABCD899F0@VE1PR04MB6638.eurprd04.prod.outlook.com>
References: <1591880310-1813-1-git-send-email-yibin.gong@nxp.com>
 <1591880310-1813-2-git-send-email-yibin.gong@nxp.com>
 <20200611134042.GG4671@sirena.org.uk>
 <VE1PR04MB66383245FAD2AE33CFEA76F789810@VE1PR04MB6638.eurprd04.prod.outlook.com>
 <20200612101357.GA5396@sirena.org.uk>
 <VE1PR04MB66384013797FE6B01943F2A889810@VE1PR04MB6638.eurprd04.prod.outlook.com>
 <20200612141611.GI5396@sirena.org.uk>
In-Reply-To: <20200612141611.GI5396@sirena.org.uk>
Accept-Language: en-US
Content-Language: en-US
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-Network-Message-Id: 20e42814-dd67-4257-dc3b-08d810638a3c
X-MS-Exchange-CrossTenant-originalarrivaltime: 14 Jun 2020 13:04:57.8287
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 686ea1d3-bc2b-4c6f-a92c-d99c5c301635
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: RrpdPdsrC09ZxJ0RTJGN15XZELcMyglqDYack75/UUs+gqOJ25LLW61D5gwF2LCfTUO5oRrzt44VTBE0Igag1A==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: VE1PR04MB6365
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 2020/06/12 22:16 Mark Brown <broonie@kernel.org> wrote:=20
> On Fri, Jun 12, 2020 at 01:48:41PM +0000, Robin Gong wrote:
> > On 2020/06/12 18:14 Mark Brown <broonie@kernel.org> wrote:
>=20
> > > 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 be=
tween
> paragraphs.
>=20
> > Sorry for that, seems my outlook format issue, hope it's ok now this
> > time :)
>=20
> Yes, looks good thanks!
>=20
> > > Client could enable this feature by choosing SPI_MASTER_FALLBACK
> > > freely without any impact on others.
>=20
> > > SPI_MASTER_FALLBACK.  If this works why would any driver not enable
> > > the flag?
>=20
> > 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 sp=
i client need
> this feature.
>=20
> It's not something that's going to come up a lot for most devices, it'd b=
e a
> mapping failure due to running out of memory or something, but your point
> about that being possible is valid.
>=20
> > > > 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
>=20
> > > PIO, thus may loop here, it's better adding limit try times here?
>=20
> > > 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=
.
>=20
> > Do you mean fallback to PIO may cause FIFO overrun since some latency
> > involved so that this patch seems not useful as expected?
>=20
> 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 FIF=
Os is
> just a random example of an error that could happen.
>=20
Sorry Mark for that I can't get your point... The bus error such as data co=
rrupt
seems not the spi core's business since it can only be caught in spi contro=
ller
driver or upper level such as mtd driver (spi-nor) which know what's the fa=
ilure
happen at spi bus HW level or what's the correct data/message. In other wor=
ds,
spi core can't detect such error by transfer_one().

> > > 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.
>=20
> > 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....
>=20
> Well, this is free software and everything can be modified!  The other op=
tion
> would be framework changes in SPI that allowed us to indicate from the dr=
iver
> that an error occured before we started doing anything to the hardware (l=
ike
> happens here) through something like a special error code or splitting up=
 the
> API.
Yes, but both assume spi controller driver could detect such dma failure be=
fore
dmaengine_prep_*(). Let's wait Vinod's comment for that if dmaengine_slave_=
config
could keep direction.
But despite of that case, do you think this patch is valid for transfer_one=
() failue
in dma and fallback to pio?