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

[Jagan Teki]

On 20 October 2014 00:13, Marek Vasut <marex at 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 at denx.de>
> Cc: Chin Liang See <clsee at altera.com>
> Cc: Dinh Nguyen <dinguyen at altera.com>
> Cc: Albert Aribaud <albert.u.boot at aribaud.net>
> Cc: Tom Rini <trini at ti.com>
> Cc: Wolfgang Denk <wd at denx.de>
> Cc: Pavel Machek <pavel at denx.de>
> Cc: Jagannadha Sutradharudu Teki <jagannadh.teki at 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.