Re: [PATCH 1/8] sunxi: SPL SPI: extract code for doing SPI transfer
On Fri, 14 Oct 2022 11:05:13 +0800
Icenowy Zheng wrote:
Hi,
> To support SPI NAND flashes, more commands than Read (03h) are needed.
>
> Extract the code for doing SPI transfer from the reading code for code
> reuse.
I was looking for a better solution than this inflated function parameter
list, but everything I came up with is actually worse. So it's fine like
you wrote it here.
I think it now even looks a bit better, since that long list is now
wrapped completely by the spi0_xfer() function.
It increases the code size by 20 (Thumb2) and 28 bytes (AArch64), that's
not great, but acceptable.
> Signed-off-by: Icenowy Zheng
With that one array index change that Samuel suggested (I can fix this up
myself):
Acked-by: Andre Przywara
Cheers,
Andre
> ---
> arch/arm/mach-sunxi/spl_spi_sunxi.c | 105
> 1 file changed, 59 insertions(+), 46 deletions(-)
>
> diff --git a/arch/arm/mach-sunxi/spl_spi_sunxi.c
> b/arch/arm/mach-sunxi/spl_spi_sunxi.c
> index 925bf85f2d..7975457758 100644
> --- a/arch/arm/mach-sunxi/spl_spi_sunxi.c
> +++ b/arch/arm/mach-sunxi/spl_spi_sunxi.c
> @@ -243,77 +243,90 @@ static void spi0_deinit(void)
>
> #define SPI_READ_MAX_SIZE 60 /* FIFO size, minus 4 bytes of the header */
>
> -static void sunxi_spi0_read_data(u8 *buf, u32 addr, u32 bufsize,
> - ulong spi_ctl_reg,
> - ulong spi_ctl_xch_bitmask,
> - ulong spi_fifo_reg,
> - ulong spi_tx_reg,
> - ulong spi_rx_reg,
> - ulong spi_bc_reg,
> - ulong spi_tc_reg,
> - ulong spi_bcc_reg)
> +static void sunxi_spi0_xfer(const u8 *txbuf, u32 txlen,
> + u8 *rxbuf, u32 rxlen,
> + ulong spi_ctl_reg,
> + ulong spi_ctl_xch_bitmask,
> + ulong spi_fifo_reg,
> + ulong spi_tx_reg,
> + ulong spi_rx_reg,
> + ulong spi_bc_reg,
> + ulong spi_tc_reg,
> + ulong spi_bcc_reg)
> {
> - writel(4 + bufsize, spi_bc_reg); /* Burst counter (total bytes) */
> - writel(4, spi_tc_reg); /* Transfer counter (bytes to send) */
> + writel(txlen + rxlen, spi_bc_reg); /* Burst counter (total bytes) */
> + writel(txlen, spi_tc_reg); /* Transfer counter (bytes to send)
> */
> if (spi_bcc_reg)
> - writel(4, spi_bcc_reg); /* SUN6I also needs this */
> + writel(txlen, spi_bcc_reg); /* SUN6I also needs this */
>
> - /* Send the Read Data Bytes (03h) command header */
> - writeb(0x03, spi_tx_reg);
> - writeb((u8)(addr >> 16), spi_tx_reg);
> - writeb((u8)(addr >> 8), spi_tx_reg);
> - writeb((u8)(addr), spi_tx_reg);
> + for (u32 i = 0; i < txlen; i++)
> + writeb(*(txbuf++), spi_tx_reg);
>
> /* Start the data transfer */
> setbits_le32(spi_ctl_reg, spi_ctl_xch_bitmask);
>
> /* Wait until everything is received in the RX FIFO */
> - while ((readl(spi_fifo_reg) & 0x7F) < 4 + bufsize)
> + while ((readl(spi_fifo_reg) & 0x7F) < txlen + rxlen)
> ;
>
> - /* Skip 4 bytes */
> - readl(spi_rx_reg);
> + /* Skip txlen bytes */
> + for (u32 i = 0; i < txlen; i++)
> + readb(spi_rx_reg);
>
> /* Read the data */
> - while (bufsize-- > 0)
> - *buf++ = readb(spi_rx_reg);
> + while (rxlen-- > 0)
> + *rxbuf++ = readb(spi_rx_reg);
> +}
> +
> +static void spi0_xfer(const u8 *txbuf, u32 txlen, u8 *rxbuf, u32 rxlen)
> +{
> + uintptr_t base = spi0_base_address();
>
> - /* tSHSL time is up to 100 ns in various SPI flash datasheets */
> - udelay(1);
> + if (is_sun6i_gen_spi()) {
> + sunxi_spi0_xfer(txbuf, txlen, rxbuf, rxlen,
> + base + SUN6I_SPI0_TCR,
> + SUN6I_TCR_XCH,
> + base + SUN6I_SPI0_FIFO_STA,
> + base + SUN6I_SPI0_TXD,
> + base + SUN6I_SPI0_RXD,
> + base + SUN6I_SPI0_MBC,
> + base + SUN6I_SPI0_MTC,
> + base + SUN6I_SPI0_BCC);
> + } else {
> + sunxi_spi0_xfer(txbuf, txlen, rxbuf, rxlen,
> + base + SUN4I_SPI0_CTL,
> + SUN4I_CTL_XCH,
> + base + SUN4I_SPI0_FIFO_STA,
> + base + SUN4I_SPI0_TX,
> + base + SUN4I_SPI0_RX,
> + base + SUN4I_SPI0_BC,
> + base + SUN4I_SPI0_TC,
> + 0);
> + }
> }
>
> static void spi0_read_data(void
Re: [PATCH 1/8] sunxi: SPL SPI: extract code for doing SPI transfer
On 10/13/22 22:05, Icenowy Zheng wrote:
> To support SPI NAND flashes, more commands than Read (03h) are needed.
>
> Extract the code for doing SPI transfer from the reading code for code
> reuse.
>
> Signed-off-by: Icenowy Zheng
One comment below.
Reviewed-by: Samuel Holland
Tested-by: Samuel Holland # Orange Pi Zero Plus
> ---
> arch/arm/mach-sunxi/spl_spi_sunxi.c | 105
> 1 file changed, 59 insertions(+), 46 deletions(-)
>
> diff --git a/arch/arm/mach-sunxi/spl_spi_sunxi.c
> b/arch/arm/mach-sunxi/spl_spi_sunxi.c
> index 925bf85f2d..7975457758 100644
> --- a/arch/arm/mach-sunxi/spl_spi_sunxi.c
> +++ b/arch/arm/mach-sunxi/spl_spi_sunxi.c
> @@ -243,77 +243,90 @@ static void spi0_deinit(void)
>
> #define SPI_READ_MAX_SIZE 60 /* FIFO size, minus 4 bytes of the header */
>
> -static void sunxi_spi0_read_data(u8 *buf, u32 addr, u32 bufsize,
> - ulong spi_ctl_reg,
> - ulong spi_ctl_xch_bitmask,
> - ulong spi_fifo_reg,
> - ulong spi_tx_reg,
> - ulong spi_rx_reg,
> - ulong spi_bc_reg,
> - ulong spi_tc_reg,
> - ulong spi_bcc_reg)
> +static void sunxi_spi0_xfer(const u8 *txbuf, u32 txlen,
> + u8 *rxbuf, u32 rxlen,
> + ulong spi_ctl_reg,
> + ulong spi_ctl_xch_bitmask,
> + ulong spi_fifo_reg,
> + ulong spi_tx_reg,
> + ulong spi_rx_reg,
> + ulong spi_bc_reg,
> + ulong spi_tc_reg,
> + ulong spi_bcc_reg)
> {
> - writel(4 + bufsize, spi_bc_reg); /* Burst counter (total bytes) */
> - writel(4, spi_tc_reg); /* Transfer counter (bytes to send) */
> + writel(txlen + rxlen, spi_bc_reg); /* Burst counter (total bytes) */
> + writel(txlen, spi_tc_reg); /* Transfer counter (bytes to send)
> */
> if (spi_bcc_reg)
> - writel(4, spi_bcc_reg); /* SUN6I also needs this */
> + writel(txlen, spi_bcc_reg); /* SUN6I also needs this */
>
> - /* Send the Read Data Bytes (03h) command header */
> - writeb(0x03, spi_tx_reg);
> - writeb((u8)(addr >> 16), spi_tx_reg);
> - writeb((u8)(addr >> 8), spi_tx_reg);
> - writeb((u8)(addr), spi_tx_reg);
> + for (u32 i = 0; i < txlen; i++)
> + writeb(*(txbuf++), spi_tx_reg);
I think txbuf[i] would be a bit clearer here.
Regards,
Samuel
>
> /* Start the data transfer */
> setbits_le32(spi_ctl_reg, spi_ctl_xch_bitmask);
>
> /* Wait until everything is received in the RX FIFO */
> - while ((readl(spi_fifo_reg) & 0x7F) < 4 + bufsize)
> + while ((readl(spi_fifo_reg) & 0x7F) < txlen + rxlen)
> ;
>
> - /* Skip 4 bytes */
> - readl(spi_rx_reg);
> + /* Skip txlen bytes */
> + for (u32 i = 0; i < txlen; i++)
> + readb(spi_rx_reg);
>
> /* Read the data */
> - while (bufsize-- > 0)
> - *buf++ = readb(spi_rx_reg);
> + while (rxlen-- > 0)
> + *rxbuf++ = readb(spi_rx_reg);
> +}
> +
> +static void spi0_xfer(const u8 *txbuf, u32 txlen, u8 *rxbuf, u32 rxlen)
> +{
> + uintptr_t base = spi0_base_address();
>
> - /* tSHSL time is up to 100 ns in various SPI flash datasheets */
> - udelay(1);
> + if (is_sun6i_gen_spi()) {
> + sunxi_spi0_xfer(txbuf, txlen, rxbuf, rxlen,
> + base + SUN6I_SPI0_TCR,
> + SUN6I_TCR_XCH,
> + base + SUN6I_SPI0_FIFO_STA,
> + base + SUN6I_SPI0_TXD,
> + base + SUN6I_SPI0_RXD,
> + base + SUN6I_SPI0_MBC,
> + base + SUN6I_SPI0_MTC,
> + base + SUN6I_SPI0_BCC);
> + } else {
> + sunxi_spi0_xfer(txbuf, txlen, rxbuf, rxlen,
> + base + SUN4I_SPI0_CTL,
> + SUN4I_CTL_XCH,
> + base + SUN4I_SPI0_FIFO_STA,
> + base + SUN4I_SPI0_TX,
> + base + SUN4I_SPI0_RX,
> + base + SUN4I_SPI0_BC,
> + base + SUN4I_SPI0_TC,
> + 0);
> + }
> }
>
> static void spi0_read_data(void *buf, u32 addr, u32 len)
> {
> u8 *buf8 = buf;
> u32 chunk_len;
> - uintptr_t base = spi0_base_address();
> + u8 txbuf[4];
>
> while (len > 0) {
> chunk_len = len;
> +
> + /* Configure the Read Data Bytes (03h) command header */
> + txbuf[0] = 0x03;
> + txbuf[1] = (u8)(addr >> 16);
> +
[PATCH 1/8] sunxi: SPL SPI: extract code for doing SPI transfer
To support SPI NAND flashes, more commands than Read (03h) are needed.
Extract the code for doing SPI transfer from the reading code for code
reuse.
Signed-off-by: Icenowy Zheng
---
arch/arm/mach-sunxi/spl_spi_sunxi.c | 105
1 file changed, 59 insertions(+), 46 deletions(-)
diff --git a/arch/arm/mach-sunxi/spl_spi_sunxi.c
b/arch/arm/mach-sunxi/spl_spi_sunxi.c
index 925bf85f2d..7975457758 100644
--- a/arch/arm/mach-sunxi/spl_spi_sunxi.c
+++ b/arch/arm/mach-sunxi/spl_spi_sunxi.c
@@ -243,77 +243,90 @@ static void spi0_deinit(void)
#define SPI_READ_MAX_SIZE 60 /* FIFO size, minus 4 bytes of the header */
-static void sunxi_spi0_read_data(u8 *buf, u32 addr, u32 bufsize,
-ulong spi_ctl_reg,
-ulong spi_ctl_xch_bitmask,
-ulong spi_fifo_reg,
-ulong spi_tx_reg,
-ulong spi_rx_reg,
-ulong spi_bc_reg,
-ulong spi_tc_reg,
-ulong spi_bcc_reg)
+static void sunxi_spi0_xfer(const u8 *txbuf, u32 txlen,
+ u8 *rxbuf, u32 rxlen,
+ ulong spi_ctl_reg,
+ ulong spi_ctl_xch_bitmask,
+ ulong spi_fifo_reg,
+ ulong spi_tx_reg,
+ ulong spi_rx_reg,
+ ulong spi_bc_reg,
+ ulong spi_tc_reg,
+ ulong spi_bcc_reg)
{
- writel(4 + bufsize, spi_bc_reg); /* Burst counter (total bytes) */
- writel(4, spi_tc_reg); /* Transfer counter (bytes to send) */
+ writel(txlen + rxlen, spi_bc_reg); /* Burst counter (total bytes) */
+ writel(txlen, spi_tc_reg); /* Transfer counter (bytes to send)
*/
if (spi_bcc_reg)
- writel(4, spi_bcc_reg); /* SUN6I also needs this */
+ writel(txlen, spi_bcc_reg); /* SUN6I also needs this */
- /* Send the Read Data Bytes (03h) command header */
- writeb(0x03, spi_tx_reg);
- writeb((u8)(addr >> 16), spi_tx_reg);
- writeb((u8)(addr >> 8), spi_tx_reg);
- writeb((u8)(addr), spi_tx_reg);
+ for (u32 i = 0; i < txlen; i++)
+ writeb(*(txbuf++), spi_tx_reg);
/* Start the data transfer */
setbits_le32(spi_ctl_reg, spi_ctl_xch_bitmask);
/* Wait until everything is received in the RX FIFO */
- while ((readl(spi_fifo_reg) & 0x7F) < 4 + bufsize)
+ while ((readl(spi_fifo_reg) & 0x7F) < txlen + rxlen)
;
- /* Skip 4 bytes */
- readl(spi_rx_reg);
+ /* Skip txlen bytes */
+ for (u32 i = 0; i < txlen; i++)
+ readb(spi_rx_reg);
/* Read the data */
- while (bufsize-- > 0)
- *buf++ = readb(spi_rx_reg);
+ while (rxlen-- > 0)
+ *rxbuf++ = readb(spi_rx_reg);
+}
+
+static void spi0_xfer(const u8 *txbuf, u32 txlen, u8 *rxbuf, u32 rxlen)
+{
+ uintptr_t base = spi0_base_address();
- /* tSHSL time is up to 100 ns in various SPI flash datasheets */
- udelay(1);
+ if (is_sun6i_gen_spi()) {
+ sunxi_spi0_xfer(txbuf, txlen, rxbuf, rxlen,
+ base + SUN6I_SPI0_TCR,
+ SUN6I_TCR_XCH,
+ base + SUN6I_SPI0_FIFO_STA,
+ base + SUN6I_SPI0_TXD,
+ base + SUN6I_SPI0_RXD,
+ base + SUN6I_SPI0_MBC,
+ base + SUN6I_SPI0_MTC,
+ base + SUN6I_SPI0_BCC);
+ } else {
+ sunxi_spi0_xfer(txbuf, txlen, rxbuf, rxlen,
+ base + SUN4I_SPI0_CTL,
+ SUN4I_CTL_XCH,
+ base + SUN4I_SPI0_FIFO_STA,
+ base + SUN4I_SPI0_TX,
+ base + SUN4I_SPI0_RX,
+ base + SUN4I_SPI0_BC,
+ base + SUN4I_SPI0_TC,
+ 0);
+ }
}
static void spi0_read_data(void *buf, u32 addr, u32 len)
{
u8 *buf8 = buf;
u32 chunk_len;
- uintptr_t base = spi0_base_address();
+ u8 txbuf[4];
while (len > 0) {
chunk_len = len;
+
+ /* Configure the Read Data Bytes (03h) command header */
+ txbuf[0] = 0x03;
+ txbuf[1] = (u8)(addr >> 16);
+ txbuf[2] = (u8)(addr >> 8);
+ txbuf[3] = (u8)(addr);
+
if (chunk_len > SPI_READ_MAX_SIZE)
chunk_len = SPI_READ_MAX_SIZE;
- if (is_sun6i_gen_spi()) {
- sunxi_spi0_read_data(buf8,
