[U-Boot] [PATCH v6 08/10] mips: ath79: add spi driver

[Wills Wang <wills.wang@live.com>]

On 01/06/2016 08:16 PM, Jagan Teki wrote:
> On 4 January 2016 at 16:44, Wills Wang <wills.wang@live.com> wrote:
>> Signed-off-by: Wills Wang <wills.wang@live.com>
>> ---
>>
>> Changes in v6:
>> - Add rrw_delay in ath79_spi_priv for more accurate timing
>> - Remove ath79_spi_delay
>> - Calculate delay in ath79_spi_set_speed
>>
>> Changes in v5:
>> - remove ar933x_spi_platdata
>> - Import document "spi-ath79.txt" from kernel
>> - Add delay for bitbang operation
>>
>> Changes in v4:
>> - Use get_bus_freq instead of hardcode in SPI driver
>>
>> Changes in v3:
>> - Convert spi driver to driver model
>> - Same code style convertion
>>
>> Changes in v2:
>> - Add a compatible spi driver
>>
>>   doc/device-tree-bindings/spi/spi-ath79.txt |  19 +++
>>   drivers/spi/Kconfig                        |   8 +
>>   drivers/spi/Makefile                       |   1 +
>>   drivers/spi/ath79_spi.c                    | 230 +++++++++++++++++++++++++++++
>>   4 files changed, 258 insertions(+)
>>   create mode 100644 doc/device-tree-bindings/spi/spi-ath79.txt
>>   create mode 100644 drivers/spi/ath79_spi.c
>>
>> diff --git a/doc/device-tree-bindings/spi/spi-ath79.txt b/doc/device-tree-bindings/spi/spi-ath79.txt
>> new file mode 100644
>> index 0000000..3fd9d67
>> --- /dev/null
>> +++ b/doc/device-tree-bindings/spi/spi-ath79.txt
>> @@ -0,0 +1,19 @@
>> +Binding for Qualcomm Atheros AR7xxx/AR9xxx SPI controller
>> +
>> +Required properties:
>> +- compatible: has to be "qca,<soc-type>-spi", "qca,ar7100-spi" as fallback.
>> +- reg: Base address and size of the controllers memory area
>> +- #address-cells: <1>, as required by generic SPI binding.
>> +- #size-cells: <0>, also as required by generic SPI binding.
>> +
>> +Child nodes as per the generic SPI binding.
>> +
>> +Example:
>> +
>> +       spi at 1f000000 {
>> +               compatible = "qca,ar9132-spi", "qca,ar7100-spi";
>> +               reg = <0x1f000000 0x10>;
>> +
>> +               #address-cells = <1>;
>> +               #size-cells = <0>;
>> +       };
>> diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
>> index 2cdb110..0ab2741 100644
>> --- a/drivers/spi/Kconfig
>> +++ b/drivers/spi/Kconfig
>> @@ -23,6 +23,14 @@ config ALTERA_SPI
>>            IP core. Please find details on the "Embedded Peripherals IP
>>            User Guide" of Altera.
>>
>> +config ATH79_SPI
>> +       bool "Atheros SPI driver"
>> +       help
>> +         Enable the Atheros ar7xxx/ar9xxx SoC SPI driver, it was used
>> +         to access SPI NOR flash and other SPI peripherals. This driver
>> +         uses driver model and requires a device tree binding to operate.
>> +         please refer to doc/device-tree-bindings/spi/spi-ath79.txt.
>> +
>>   config CADENCE_QSPI
>>          bool "Cadence QSPI driver"
>>          help
>> diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile
>> index 3eca745..7fb2926 100644
>> --- a/drivers/spi/Makefile
>> +++ b/drivers/spi/Makefile
>> @@ -17,6 +17,7 @@ endif
>>
>>   obj-$(CONFIG_ALTERA_SPI) += altera_spi.o
>>   obj-$(CONFIG_ARMADA100_SPI) += armada100_spi.o
>> +obj-$(CONFIG_ATH79_SPI) += ath79_spi.o
>>   obj-$(CONFIG_ATMEL_DATAFLASH_SPI) += atmel_dataflash_spi.o
>>   obj-$(CONFIG_ATMEL_SPI) += atmel_spi.o
>>   obj-$(CONFIG_BFIN_SPI) += bfin_spi.o
>> diff --git a/drivers/spi/ath79_spi.c b/drivers/spi/ath79_spi.c
>> new file mode 100644
>> index 0000000..3912332
>> --- /dev/null
>> +++ b/drivers/spi/ath79_spi.c
>> @@ -0,0 +1,230 @@
>> +/*
>> + * (C) Copyright 2015
>> + * Wills Wang, <wills.wang@live.com>
> Append this above the line with space.
Where?
>> + *
>> + * SPDX-License-Identifier:     GPL-2.0+
>> + */
>> +
>> +#include <common.h>
>> +#include <spi.h>
>> +#include <dm.h>
>> +#include <div64.h>
>> +#include <errno.h>
>> +#include <asm/io.h>
>> +#include <asm/addrspace.h>
>> +#include <asm/types.h>
>> +#include <mach/ar71xx_regs.h>
> Does this regs common for ar71xx or for spi?
Yes
>> +
>> +/* CLOCK_DIVIDER = 3 (SPI clock = 200 / 8 ~ 25 MHz) */
>> +#define ATH79_SPI_CLK_DIV(x)           (((x) >> 1) - 1)
>> +#define ATH79_SPI_RRW_DELAY_FACTOR     12000
>> +#define MHZ                             (1000 * 1000)
> Rename MHZ to ATH79_SPI_MHZ or similar
Ok.
>> +
>> +struct ath79_spi_priv {
>> +       void __iomem *regs;
>> +       u32 rrw_delay;
>> +};
>> +
>> +static inline u32 ath79_spi_read(struct udevice *bus, u32 offset)
>> +{
>> +       struct ath79_spi_priv *priv = dev_get_priv(bus);
>> +       return readl(priv->regs + offset);
>> +}
>> +
>> +static inline void ath79_spi_write(struct udevice *bus,
>> +                                       u32 val, u32 offset)
>> +{
>> +       struct ath79_spi_priv *priv = dev_get_priv(bus);
>> +       writel(val, priv->regs + offset);
>> +}
> Instead of adding these new read|write calls, use readl and writel
> directly since we can get bus any where using dev
>
There is an inline function.
>> +
>> +static int ath79_spi_claim_bus(struct udevice *dev)
>> +{
>> +       return 0;
>> +}
>> +
>> +static int ath79_spi_release_bus(struct udevice *dev)
>> +{
>> +       return 0;
>> +}
>> +
>> +static int ath79_spi_xfer(struct udevice *dev, unsigned int bitlen,
>> +               const void *dout, void *din, unsigned long flags)
>> +{
>> +       struct udevice *bus = dev->parent;
>> +       struct ath79_spi_priv *priv = dev_get_priv(bus);
>> +       struct dm_spi_slave_platdata *slave = dev_get_parent_platdata(dev);
>> +       u8 *rx = din;
>> +       const u8 *tx = dout;
>> +       u8 curbyte, curbitlen, restbits;
>> +       u32 bytes = bitlen / 8;
>> +       u32 out, in;
>> +       u64 tick;
>> +
>> +
>> +       if (flags & SPI_XFER_BEGIN) {
>> +               ath79_spi_write(bus, AR71XX_SPI_FS_GPIO, AR71XX_SPI_REG_FS);
>> +               ath79_spi_write(bus, AR71XX_SPI_IOC_CS_ALL, AR71XX_SPI_REG_IOC);
> Move this code to local spi_cs_activate and use clrbits_xxx or
> setbits_xxxx if required for just to activate or deactivate specific
> bits.
Ok.
>> +       }
>> +
>> +       restbits = (bitlen % 8);
>> +       if (restbits)
>> +               bytes++;
>> +
>> +       out = AR71XX_SPI_IOC_CS_ALL & ~(AR71XX_SPI_IOC_CS(slave->cs));
>> +       while (bytes > 0) {
>> +               bytes--;
>> +               curbyte = 0;
>> +               if (tx)
>> +                       curbyte = *tx++;
>> +
>> +               if (restbits && !bytes) {
>> +                       curbitlen = restbits;
>> +                       curbyte <<= 8 - restbits;
>> +               } else {
>> +                       curbitlen = 8;
>> +               }
>> +
>> +               for (curbyte <<= (8 - curbitlen); curbitlen; curbitlen--) {
>> +                       if (curbyte & 0x80)
>> +                               out |= AR71XX_SPI_IOC_DO;
>> +                       else
>> +                               out &= ~(AR71XX_SPI_IOC_DO);
>> +
>> +                       ath79_spi_write(bus, out, AR71XX_SPI_REG_IOC);
>> +
>> +                       /* delay for low level */
>> +                       if (priv->rrw_delay) {
>> +                               tick = get_ticks() + priv->rrw_delay;
>> +                               while (get_ticks() < tick)
>> +                                       /*NOP*/;
>> +                       }
>> +
>> +                       ath79_spi_write(bus, out | AR71XX_SPI_IOC_CLK,
>> +                                       AR71XX_SPI_REG_IOC);
>> +
>> +                       /* delay for high level */
>> +                       if (priv->rrw_delay) {
>> +                               tick = get_ticks() + priv->rrw_delay;
>> +                               while (get_ticks() < tick)
>> +                                       /*NOP*/;
> Better to do timeout check instead of infinite while.
I think it is a timeout check.
>> +                       }
>> +
>> +                       curbyte <<= 1;
>> +               }
>> +
>> +               if (!bytes)
>> +                       ath79_spi_write(bus, out, AR71XX_SPI_REG_IOC);
>> +
>> +               in = ath79_spi_read(bus, AR71XX_SPI_REG_RDS);
>> +               if (rx) {
>> +                       if (restbits && !bytes)
>> +                               *rx++ = (in << (8 - restbits));
>> +                       else
>> +                               *rx++ = in;
>> +               }
>> +       }
>> +
>> +       if (flags & SPI_XFER_END) {
>> +               ath79_spi_write(bus, AR71XX_SPI_IOC_CS(slave->cs),
>> +                               AR71XX_SPI_REG_IOC);
>> +               ath79_spi_write(bus, AR71XX_SPI_IOC_CS_ALL, AR71XX_SPI_REG_IOC);
>> +               ath79_spi_write(bus, 0, AR71XX_SPI_REG_FS);
> Same as above use spi_cs_deactivate
>
>> +       }
>> +
>> +       return 0;
>> +}
>> +
>> +
>> +static int ath79_spi_set_speed(struct udevice *bus, uint speed)
>> +{
>> +       struct ath79_spi_priv *priv = dev_get_priv(bus);
>> +       u32 val, div = 0;
>> +       u64 time;
>> +
>> +       if (speed)
>> +               div = get_bus_freq(0) / speed;
>> +
>> +       if (div > 63)
>> +               div = 63;
>> +
>> +       if (div < 5)
>> +               div = 5;
> Where are these div comparisons? please add clear short comment.
The lower limitation is an empirical value, upper limitation is the 
maximum value for register filed.
>> +
>> +       /* calculate delay */
>> +       time = get_tbclk();
>> +       do_div(time, speed / 2);
>> +       val = ATH79_SPI_RRW_DELAY_FACTOR / (get_bus_freq(0) / MHZ);
>> +       if (time > val)
>> +               priv->rrw_delay = time - val + 1;
>> +       else
>> +               priv->rrw_delay = 0;
>> +
>> +       ath79_spi_write(bus, AR71XX_SPI_FS_GPIO, AR71XX_SPI_REG_FS);
>> +       val = ath79_spi_read(bus, AR71XX_SPI_REG_CTRL);
>> +       val &= ~AR71XX_SPI_CTRL_DIV_MASK;
>> +       val |= ATH79_SPI_CLK_DIV(div);
>> +       ath79_spi_write(bus, val, AR71XX_SPI_REG_CTRL);
>> +       ath79_spi_write(bus, 0, AR71XX_SPI_REG_FS);
> This write also for speed setting, does this function doing other than
> speed_setup?
>
Calculate SPI bitbang delay according to speed.
>> +       return 0;
>> +}
>> +
>> +static int ath79_spi_set_mode(struct udevice *bus, uint mode)
>> +{
>> +       return 0;
>> +}
>> +
>> +static int ath79_spi_probe(struct udevice *bus)
>> +{
>> +       struct ath79_spi_priv *priv = dev_get_priv(bus);
>> +       fdt_addr_t addr;
>> +
>> +       addr = dev_get_addr(bus);
>> +       if (addr == FDT_ADDR_T_NONE)
>> +               return -EINVAL;
>> +
>> +       priv->regs = map_physmem(addr,
>> +                                AR71XX_SPI_SIZE,
>> +                                MAP_NOCACHE);
>> +
>> +       /* Init SPI Hardware, disable remap, set clock */
>> +       ath79_spi_write(bus, AR71XX_SPI_FS_GPIO, AR71XX_SPI_REG_FS);
>> +       ath79_spi_write(bus, AR71XX_SPI_CTRL_RD | ATH79_SPI_CLK_DIV(8),
>> +                       AR71XX_SPI_REG_CTRL);
>> +       ath79_spi_write(bus, 0, AR71XX_SPI_REG_FS);
>> +
>> +       return 0;
>> +}
>> +
>> +static int ath79_cs_info(struct udevice *bus, uint cs,
>> +                          struct spi_cs_info *info)
>> +{
>> +       /* Always allow activity on CS 0/1/2 */
>> +       if (cs >= 3)
>> +               return -ENODEV;
>> +
>> +       return 0;
>> +}
>> +
>> +static const struct dm_spi_ops ath79_spi_ops = {
>> +       .claim_bus  = ath79_spi_claim_bus,
>> +       .release_bus    = ath79_spi_release_bus,
>> +       .xfer       = ath79_spi_xfer,
>> +       .set_speed  = ath79_spi_set_speed,
>> +       .set_mode   = ath79_spi_set_mode,
>> +       .cs_info    = ath79_cs_info,
>> +};
>> +
>> +static const struct udevice_id ath79_spi_ids[] = {
>> +       { .compatible = "qca,ar7100-spi" },
>> +       {}
>> +};
>> +
>> +U_BOOT_DRIVER(ath79_spi) = {
>> +       .name   = "ath79_spi",
>> +       .id = UCLASS_SPI,
>> +       .of_match = ath79_spi_ids,
>> +       .ops    = &ath79_spi_ops,
>> +       .priv_auto_alloc_size = sizeof(struct ath79_spi_priv),
>> +       .probe  = ath79_spi_probe,
>> +};
>> --
> thanks!

-- 
Best Regards
Wills