[U-Boot] [PATCH 1/7] spi: altera: Use struct-based register access

Jagan Teki
2014-10-20T14:53:15Z

On 20 October 2014 00:13, Marek Vasut <marex@denx.de> wrote:
> Zap the offset-based register access and use the struct-based one
> as this is the preferred method.
>
> No functional change, but there are some line-over-80 problems in
> the driver, which will be addressed later.
>
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Chin Liang See <clsee@altera.com>
> Cc: Dinh Nguyen <dinguyen@altera.com>
> Cc: Albert Aribaud <albert.u.boot@aribaud.net>
> Cc: Tom Rini <trini@ti.com>
> Cc: Wolfgang Denk <wd@denx.de>
> Cc: Pavel Machek <pavel@denx.de>
> Cc: Jagannadha Sutradharudu Teki <jagannadh.teki@gmail.com>
> ---
> drivers/spi/altera_spi.c | 49 ++++++++++++++++++++++++------------------------
> 1 file changed, 25 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/spi/altera_spi.c b/drivers/spi/altera_spi.c
> index 5accbb5..13191f3 100644
> --- a/drivers/spi/altera_spi.c
> +++ b/drivers/spi/altera_spi.c
> @@ -12,11 +12,14 @@
> #include <malloc.h>
> #include <spi.h>
>
> -#define ALTERA_SPI_RXDATA 0
> -#define ALTERA_SPI_TXDATA 4
> -#define ALTERA_SPI_STATUS 8
> -#define ALTERA_SPI_CONTROL 12
> -#define ALTERA_SPI_SLAVE_SEL 20
> +struct altera_spi_regs {
> + u32 rxdata;
> + u32 txdata;
> + u32 status;
> + u32 control;
> + u32 _reserved;
> + u32 slave_sel;
> +};

Can you place this structure definition below of all macro defines, i
don't think the
next level patches does that, does they?

>
> #define ALTERA_SPI_STATUS_ROE_MSK (0x8)
> #define ALTERA_SPI_STATUS_TOE_MSK (0x10)
> @@ -39,8 +42,8 @@
> static ulong altera_spi_base_list[] = CONFIG_SYS_ALTERA_SPI_LIST;
>
> struct altera_spi_slave {
> - struct spi_slave slave;
> - ulong base;
> + struct spi_slave slave;
> + struct altera_spi_regs *regs;
> };
> #define to_altera_spi_slave(s) container_of(s, struct altera_spi_slave, slave)
>
> @@ -54,16 +57,16 @@ __attribute__((weak))
> void spi_cs_activate(struct spi_slave *slave)
> {
> struct altera_spi_slave *altspi = to_altera_spi_slave(slave);
> - writel(1 << slave->cs, altspi->base + ALTERA_SPI_SLAVE_SEL);
> - writel(ALTERA_SPI_CONTROL_SSO_MSK, altspi->base + ALTERA_SPI_CONTROL);
> + writel(1 << slave->cs, &altspi->regs->slave_sel);
> + writel(ALTERA_SPI_CONTROL_SSO_MSK, &altspi->regs->control);
> }
>
> __attribute__((weak))
> void spi_cs_deactivate(struct spi_slave *slave)
> {
> struct altera_spi_slave *altspi = to_altera_spi_slave(slave);
> - writel(0, altspi->base + ALTERA_SPI_CONTROL);
> - writel(0, altspi->base + ALTERA_SPI_SLAVE_SEL);
> + writel(0, &altspi->regs->control);
> + writel(0, &altspi->regs->slave_sel);
> }
>
> void spi_init(void)
> @@ -87,9 +90,9 @@ struct spi_slave *spi_setup_slave(unsigned int bus, unsigned int cs,
> if (!altspi)
> return NULL;
>
> - altspi->base = altera_spi_base_list[bus];
> - debug("%s: bus:%i cs:%i base:%lx\n", __func__,
> - bus, cs, altspi->base);
> + altspi->regs = (struct altera_spi_regs *)altera_spi_base_list[bus];
> + debug("%s: bus:%i cs:%i base:%p\n", __func__,
> + bus, cs, altspi->regs);
>
> return &altspi->slave;
> }
> @@ -105,8 +108,8 @@ int spi_claim_bus(struct spi_slave *slave)
> struct altera_spi_slave *altspi = to_altera_spi_slave(slave);
>
> debug("%s: bus:%i cs:%i\n", __func__, slave->bus, slave->cs);
> - writel(0, altspi->base + ALTERA_SPI_CONTROL);
> - writel(0, altspi->base + ALTERA_SPI_SLAVE_SEL);
> + writel(0, &altspi->regs->control);
> + writel(0, &altspi->regs->slave_sel);
> return 0;
> }
>
> @@ -115,7 +118,7 @@ void spi_release_bus(struct spi_slave *slave)
> struct altera_spi_slave *altspi = to_altera_spi_slave(slave);
>
> debug("%s: bus:%i cs:%i\n", __func__, slave->bus, slave->cs);
> - writel(0, altspi->base + ALTERA_SPI_SLAVE_SEL);
> + writel(0, &altspi->regs->slave_sel);
> }
>
> #ifndef CONFIG_ALTERA_SPI_IDLE_VAL
> @@ -142,20 +145,18 @@ int spi_xfer(struct spi_slave *slave, unsigned int bitlen, const void *dout,
> }
>
> /* empty read buffer */
> - if (readl(altspi->base + ALTERA_SPI_STATUS) &
> - ALTERA_SPI_STATUS_RRDY_MSK)
> - readl(altspi->base + ALTERA_SPI_RXDATA);
> + if (readl(&altspi->regs->status) & ALTERA_SPI_STATUS_RRDY_MSK)
> + readl(&altspi->regs->rxdata);
> if (flags & SPI_XFER_BEGIN)
> spi_cs_activate(slave);
>
> while (bytes--) {
> uchar d = txp ? *txp++ : CONFIG_ALTERA_SPI_IDLE_VAL;
> debug("%s: tx:%x ", __func__, d);
> - writel(d, altspi->base + ALTERA_SPI_TXDATA);
> - while (!(readl(altspi->base + ALTERA_SPI_STATUS) &
> - ALTERA_SPI_STATUS_RRDY_MSK))
> + writel(d, &altspi->regs->txdata);
> + while (!(readl(&altspi->regs->status) & ALTERA_SPI_STATUS_RRDY_MSK))
> ;
> - d = readl(altspi->base + ALTERA_SPI_RXDATA);
> + d = readl(&altspi->regs->rxdata);
> if (rxp)
> *rxp++ = d;
> debug("rx:%x\n", d);
> --
> 2.1.1
>

thanks!
--
Jagan.