[U-Boot] [PATCH 07/10] spi: sun4: Add A31 spi controller support

[Chen-Yu Tsai]

On Wed, Feb 13, 2019 at 9:21 AM André Przywara <andre.przywara at arm.com> wrote:
>
> On 09/02/2019 13:14, Jagan Teki wrote:
> > Add A31 spi controller support for existing sun4i_spi driver via driver
> > data, this would simply add A31 register along with proper register bits
> > via enum sets.
> >
> > Signed-off-by: Jagan Teki <jagan at amarulasolutions.com>
> > ---
> >  drivers/spi/Kconfig     |  4 +-
> >  drivers/spi/sun4i_spi.c | 84 ++++++++++++++++++++++++++++++++++++++++-
> >  2 files changed, 85 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
> > index ac7fbab841..15207d23c1 100644
> > --- a/drivers/spi/Kconfig
> > +++ b/drivers/spi/Kconfig
> > @@ -223,9 +223,9 @@ config STM32_QSPI
> >         this ST IP core.
> >
> >  config SUN4I_SPI
> > -     bool "Allwinner A10 SoCs SPI controller"
> > +     bool "Allwinner A10/A31 SoCs SPI controller"
> >       help
> > -       SPI driver for Allwinner sun4i, sun5i and sun7i SoCs
> > +       This enables using the SPI controller on the Allwinner A10/A31 SoCs.
> >
> >  config TEGRA114_SPI
> >       bool "nVidia Tegra114 SPI driver"
> > diff --git a/drivers/spi/sun4i_spi.c b/drivers/spi/sun4i_spi.c
> > index aeed68764c..36f2215f7d 100644
> > --- a/drivers/spi/sun4i_spi.c
> > +++ b/drivers/spi/sun4i_spi.c
> > @@ -24,6 +24,7 @@
> >  #include <spi.h>
> >  #include <errno.h>
> >  #include <fdt_support.h>
> > +#include <reset.h>
> >  #include <wait_bit.h>
> >
> >  #include <asm/bitops.h>
> > @@ -84,6 +85,18 @@
> >  #define SUN4I_FIFO_STA_TF_CNT_MASK   0x7f
> >  #define SUN4I_FIFO_STA_TF_CNT_BITS   16
> >
> > +/* sun6i spi registers */
> > +#define SUN6I_GBL_CTL_REG            0x04
> > +#define SUN6I_TFR_CTL_REG            0x08
> > +#define SUN6I_FIFO_CTL_REG           0x18
> > +#define SUN6I_FIFO_STA_REG           0x1c
> > +#define SUN6I_CLK_CTL_REG            0x24
> > +#define SUN6I_BURST_CNT_REG          0x30
> > +#define SUN6I_XMIT_CNT_REG           0x34
> > +#define SUN6I_BURST_CTL_REG          0x38
> > +#define SUN6I_TXDATA_REG             0x200
> > +#define SUN6I_RXDATA_REG             0x300
> > +
> >  #define SUN4I_SPI_MAX_RATE   24000000
> >  #define SUN4I_SPI_MIN_RATE   3000
> >  #define SUN4I_SPI_DEFAULT_RATE       1000000
> > @@ -106,6 +119,7 @@ enum sun4i_spi_regs {
> >  /* sun spi register bits */
> >  enum sun4i_spi_bits {
> >       SPI_GCR_TP,
> > +     SPI_GCR_SRST,
> >       SPI_TCR_CPHA,
> >       SPI_TCR_CPOL,
> >       SPI_TCR_CS_ACTIVE_LOW,
> > @@ -133,6 +147,7 @@ struct sun4i_spi_platdata {
> >  struct sun4i_spi_priv {
> >       struct sun4i_spi_variant *variant;
> >       struct clk clk_ahb, clk_mod;
> > +     struct reset_ctl reset;
> >       u32 base_addr;
> >       u32 freq;
> >       u32 mode;
> > @@ -255,7 +270,10 @@ static int sun4i_spi_parse_pins(struct udevice *dev)
> >                       if (pin < 0)
> >                               break;
> >
> > -                     sunxi_gpio_set_cfgpin(pin, SUNXI_GPC_SPI0);
> > +                     if (IS_ENABLED(CONFIG_MACH_SUN50I))
> > +                             sunxi_gpio_set_cfgpin(pin, SUN50I_GPC_SPI0);
> > +                     else
> > +                             sunxi_gpio_set_cfgpin(pin, SUNXI_GPC_SPI0);
> >                       sunxi_gpio_set_drv(pin, drive);
> >                       sunxi_gpio_set_pull(pin, pull);
> >               }
> > @@ -271,6 +289,8 @@ static inline int sun4i_spi_set_clock(struct udevice *dev, bool enable)
> >       if (!enable) {
> >               clk_disable(&priv->clk_ahb);
> >               clk_disable(&priv->clk_mod);
> > +             if (reset_valid(&priv->reset))
> > +                     reset_assert(&priv->reset);
> >               return 0;
> >       }
> >
> > @@ -286,8 +306,18 @@ static inline int sun4i_spi_set_clock(struct udevice *dev, bool enable)
> >               goto err_ahb;
> >       }
> >
> > +     if (reset_valid(&priv->reset)) {
> > +             ret = reset_deassert(&priv->reset);
> > +             if (ret) {
> > +                     dev_err(dev, "failed to deassert reset\n");
> > +                     goto err_mod;
> > +             }
> > +     }
> > +
> >       return 0;
> >
> > +err_mod:
> > +     clk_disable(&priv->clk_mod);
> >  err_ahb:
> >       clk_disable(&priv->clk_ahb);
> >       return ret;
> > @@ -328,6 +358,12 @@ static int sun4i_spi_probe(struct udevice *bus)
> >               return ret;
> >       }
> >
> > +     ret = reset_get_by_index(bus, 0, &priv->reset);
> > +     if (ret && ret != -ENOENT) {
> > +             dev_err(dev, "failed to get reset\n");
> > +             return ret;
> > +     }
> > +
> >       sun4i_spi_parse_pins(bus);
> >
> >       priv->variant = plat->variant;
> > @@ -351,6 +387,10 @@ static int sun4i_spi_claim_bus(struct udevice *dev)
> >                    SUN4I_CTL_ENABLE | SUN4I_CTL_MASTER |
> >                    variant->bits[SPI_GCR_TP]);
> >
> > +     if (IS_ENABLED(CONFIG_SUNXI_GEN_SUN6I))
> > +             setbits_le32(priv->base_addr + variant->regs[SPI_GCR],
> > +                          variant->bits[SPI_GCR_SRST]);
> > +
>
> Just a note: This makes the driver only support one class of device, set
> at compile time. Which means we could have used an #ifdef approach from
> the beginning ....
> Can you make this dependent on some bits in variant? That should solve it.

I believe the argument for IS_ENABLED from working in the kernel is that
it still gives you compile-time checks, whereas #ifdef doesn't.

Though I believe your ultimate goal is to have one image for all? In that
case checking against the variants at runtime is better.

ChenYu