RE: [bug report] spi: lpspi: Improve the stability of lpspi data transmission

[Clark Wang <xiaoning.wang@xxxxxxx>]

> -----Original Message-----
> From: Dan Carpenter <dan.carpenter@xxxxxxxxxx>
> Sent: Wednesday, January 9, 2019 16:25
> To: Clark Wang <xiaoning.wang@xxxxxxx>
> Cc: linux-spi@xxxxxxxxxxxxxxx
> Subject: [bug report] spi: lpspi: Improve the stability of lpspi data transmission
> 
> Hello Clark Wang,
> 
> The patch c23fdef891ac: "spi: lpspi: Improve the stability of lpspi data
> transmission" from Jan 7, 2019, leads to the following static checker warning:
> 
> 	drivers/spi/spi-fsl-lpspi.c:463 fsl_lpspi_isr()
> 	warn: inconsistent indenting
> 
> drivers/spi/spi-fsl-lpspi.c
>     438 static irqreturn_t fsl_lpspi_isr(int irq, void *dev_id)
>     439 {
>     440 	u32 temp_SR, temp_IER;
>     441 	struct fsl_lpspi_data *fsl_lpspi = dev_id;
>     442
>     443 	temp_IER = readl(fsl_lpspi->base + IMX7ULP_IER);
>     444 	fsl_lpspi_intctrl(fsl_lpspi, 0);
>     445 	temp_SR = readl(fsl_lpspi->base + IMX7ULP_SR);
>     446
>     447 	fsl_lpspi_read_rx_fifo(fsl_lpspi);
>     448
>     449 	if ((temp_SR & SR_TDF) && (temp_IER & IER_TDIE)) {
>     450 		fsl_lpspi_write_tx_fifo(fsl_lpspi);
>     451 		return IRQ_HANDLED;
>     452 	}
>     453
>     454 	if (temp_SR & SR_MBF ||
>     455 	    readl(fsl_lpspi->base + IMX7ULP_FSR) & FSR_RXCOUNT) {
>     456 		writel(SR_FCF, fsl_lpspi->base + IMX7ULP_SR);
>     457 		fsl_lpspi_intctrl(fsl_lpspi, IER_FCIE);
>     458 		return IRQ_HANDLED;
>     459 	}
>     460
>     461 	if (temp_SR & SR_FCF && (temp_IER & IER_FCIE)) {
>     462 		writel(SR_FCF, fsl_lpspi->base + IMX7ULP_SR);
> --> 463 			complete(&fsl_lpspi->xfer_done);
> 
> There used to be an "if (!fsl_lpspi->remain)" statement, and that's why the
> complete() line was indented.  Was it deleted accidentally?
> 

Hi Dan,

Thank you for your suggestion. 

I delete the "if (!fsl_lpspi->remain)" statement because this is only a count
number by software, so it might go wrong under some circumstance.  In the
previous transmission process, it sometimes happens that fsl_lpspi->remain
has been reduced to zero but the data received on the hardware is actually
smaller than tx->len. The cause of this problem may be the order between
tx and rx or sometimes hardware reasons. It will cause data loss.

In order to avoid this problem, fsl_lpspi->remain is only for TX now.
I will open the IER_FCIE when the controller sends the last frame of one
transfer. The hardware will set IER_FCIE when it finishes sending a frame.
So, when the program goes in isr function becase IER_FCIE is set, it means the
all the transmissions have done. All of the tx data will be sent and all of the rx
data will be received at this time.

The program has passed the test, even let it communicates with
spi-nor@40MHz when this patch series are all applied. The bug which
happened when we don't use CONT has been fixed by " spi: lpspi: Fix wrong
transmission when don't use CONT "(6a130448498c151).

Regards,
Clark Wang

>     464 		return IRQ_HANDLED;
>     465 	}
>     466
>     467 	return IRQ_NONE;
>     468 }
> 
> regards,
> dan carpenter