[U-Boot] [PATCH v2 5/8] lpc32xx: add LPC32xx SSP support (SPI mode)

Jagan Teki jagannadh.teki at gmail.com
Tue Feb 17 14:15:44 CET 2015


Hi Albert,

On 17 February 2015 at 15:21, Albert ARIBAUD <albert.aribaud at 3adev.fr> wrote:
> Bonjour Jagan,
>
> Le Tue, 17 Feb 2015 15:03:16 +0530, Jagan Teki
> <jagannadh.teki at gmail.com> a écrit :
>
>> On 12 February 2015 at 23:07, Albert ARIBAUD (3ADEV)
>> <albert.aribaud at 3adev.fr> wrote:
>> > Signed-off-by: Albert ARIBAUD (3ADEV) <albert.aribaud at 3adev.fr>
>> > ---
>> >
>> > Changes in v2:
>> > - added MUX setting for SSP0
>> >
>> >  arch/arm/cpu/arm926ejs/lpc32xx/devices.c      |  14 +++
>> >  arch/arm/include/asm/arch-lpc32xx/clk.h       |   3 +
>> >  arch/arm/include/asm/arch-lpc32xx/sys_proto.h |   1 +
>> >  drivers/spi/Makefile                          |   1 +
>> >  drivers/spi/lpc32xx_ssp.c                     | 132 ++++++++++++++++++++++++++
>> >  5 files changed, 151 insertions(+)
>> >  create mode 100644 drivers/spi/lpc32xx_ssp.c
>> >
>> > diff --git a/arch/arm/cpu/arm926ejs/lpc32xx/devices.c b/arch/arm/cpu/arm926ejs/lpc32xx/devices.c
>> > index a407098..5a453e3 100644
>> > --- a/arch/arm/cpu/arm926ejs/lpc32xx/devices.c
>> > +++ b/arch/arm/cpu/arm926ejs/lpc32xx/devices.c
>> > @@ -8,11 +8,13 @@
>> >  #include <asm/arch/cpu.h>
>> >  #include <asm/arch/clk.h>
>> >  #include <asm/arch/uart.h>
>> > +#include <asm/arch/mux.h>
>> >  #include <asm/io.h>
>> >  #include <dm.h>
>> >
>> >  static struct clk_pm_regs    *clk  = (struct clk_pm_regs *)CLK_PM_BASE;
>> >  static struct uart_ctrl_regs *ctrl = (struct uart_ctrl_regs *)UART_CTRL_BASE;
>> > +static struct mux_regs *mux = (struct mux_regs *)MUX_BASE;
>> >
>> >  void lpc32xx_uart_init(unsigned int uart_id)
>> >  {
>> > @@ -66,3 +68,15 @@ void lpc32xx_i2c_init(unsigned int devnum)
>> >  U_BOOT_DEVICE(lpc32xx_gpios) = {
>> >         .name = "gpio_lpc32xx"
>> >  };
>> > +
>> > +/* Mux for SCK0, MISO0, MOSI0. We do not use SSEL0. */
>> > +
>> > +#define P_MUX_SET_SSP0 0x1600
>> > +
>> > +void lpc32xx_ssp_init(void)
>> > +{
>> > +       /* Enable SSP0 interface */
>> > +       writel(CLK_SSP0_ENABLE_CLOCK, &clk->ssp_ctrl);
>> > +       /* Mux SSP0 pins */
>> > +       writel(P_MUX_SET_SSP0, &mux->p_mux_set);
>> > +}
>> > diff --git a/arch/arm/include/asm/arch-lpc32xx/clk.h b/arch/arm/include/asm/arch-lpc32xx/clk.h
>> > index 781ac07..2cb5703 100644
>> > --- a/arch/arm/include/asm/arch-lpc32xx/clk.h
>> > +++ b/arch/arm/include/asm/arch-lpc32xx/clk.h
>> > @@ -155,6 +155,9 @@ struct clk_pm_regs {
>> >  #define CLK_NAND_MLC                   (1 << 1)
>> >  #define CLK_NAND_MLC_INT               (1 << 5)
>> >
>> > +/* SSP Clock Control Register bits */
>> > +#define CLK_SSP0_ENABLE_CLOCK          (1 << 0)
>> > +
>> >  unsigned int get_sys_clk_rate(void);
>> >  unsigned int get_hclk_pll_rate(void);
>> >  unsigned int get_hclk_clk_div(void);
>> > diff --git a/arch/arm/include/asm/arch-lpc32xx/sys_proto.h b/arch/arm/include/asm/arch-lpc32xx/sys_proto.h
>> > index a4a05d1..86d5ee9 100644
>> > --- a/arch/arm/include/asm/arch-lpc32xx/sys_proto.h
>> > +++ b/arch/arm/include/asm/arch-lpc32xx/sys_proto.h
>> > @@ -11,5 +11,6 @@ void lpc32xx_uart_init(unsigned int uart_id);
>> >  void lpc32xx_mac_init(void);
>> >  void lpc32xx_mlc_nand_init(void);
>> >  void lpc32xx_i2c_init(unsigned int devnum);
>> > +void lpc32xx_ssp_init(void);
>> >
>> >  #endif /* _LPC32XX_SYS_PROTO_H */
>> > diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile
>> > index edbd520..ce6f1cc 100644
>> > --- a/drivers/spi/Makefile
>> > +++ b/drivers/spi/Makefile
>> > @@ -32,6 +32,7 @@ obj-$(CONFIG_EXYNOS_SPI) += exynos_spi.o
>> >  obj-$(CONFIG_FTSSP010_SPI) += ftssp010_spi.o
>> >  obj-$(CONFIG_ICH_SPI) +=  ich.o
>> >  obj-$(CONFIG_KIRKWOOD_SPI) += kirkwood_spi.o
>> > +obj-$(CONFIG_LPC32XX_SSP) += lpc32xx_ssp.o
>> >  obj-$(CONFIG_MPC52XX_SPI) += mpc52xx_spi.o
>> >  obj-$(CONFIG_MPC8XXX_SPI) += mpc8xxx_spi.o
>> >  obj-$(CONFIG_MXC_SPI) += mxc_spi.o
>> > diff --git a/drivers/spi/lpc32xx_ssp.c b/drivers/spi/lpc32xx_ssp.c
>> > new file mode 100644
>> > index 0000000..40270df
>> > --- /dev/null
>> > +++ b/drivers/spi/lpc32xx_ssp.c
>> > @@ -0,0 +1,132 @@
>> > +/*
>> > + * LPC32xx SSP interface (SPI mode)
>> > + *
>> > + * (C) Copyright 2014  DENX Software Engineering GmbH
>> > + * Written-by: Albert ARIBAUD <albert.aribaud at 3adev.fr>
>> > + *
>> > + * SPDX-License-Identifier:     GPL-2.0+
>> > + */
>> > +
>> > +#include <common.h>
>> > +#include <linux/compat.h>
>> > +#include <asm/io.h>
>> > +#include <malloc.h>
>> > +#include <spi.h>
>> > +#include <asm/arch/clk.h>
>> > +
>> > +/* SSP chip registers */
>> > +struct ssp_regs {
>> > +       u32 cr0;
>> > +       u32 cr1;
>> > +       u32 data;
>> > +       u32 sr;
>> > +       u32 cpsr;
>> > +       u32 imsc;
>> > +       u32 ris;
>> > +       u32 mis;
>> > +       u32 icr;
>> > +       u32 dmacr;
>> > +};
>> > +
>> > +/* CR1 register defines  */
>> > +#define SSP_CR1_SSP_ENABLE 0x0002
>> > +
>> > +/* SR register defines  */
>> > +#define SSP_SR_TNF 0x0002
>> > +/* SSP status RX FIFO not empty bit */
>> > +#define SSP_SR_RNE 0x0004
>> > +
>> > +static struct ssp_regs *ssp0_regs = (struct ssp_regs *)SSP0_BASE;
>>
>> Pls- use static inline instead of direct macro assignment.
>
> Hmm... What's the benefit of going through a static inline function
> when all that is performed is an assignment?

Sorry, it's my mistake i thought the driver handling more than one
controller base.
Your macro assignment is correct in that sense.

All I'm explained below is that changes in spi driver like how well it
uses the generic spi calls
and the format of the driver resembles same as others (reff. driver
drivers/spi/zynq_spi.c)

All (most) of the drivers will follow the same.

If you still confused, may be I will look at it but pls- understand.

>
>> > +
>> > +static struct spi_slave ssp0_slave = {
>> > +       .bus = 0,
>> > +       .cs = 0,
>> > +       .op_mode_rx = 0,
>> > +       .op_mode_tx = 0,
>> > +       .wordlen = 8,
>> > +       .max_write_size = 1, /* this is for SPI FLASHes -- don't care */
>> > +       .memory_map = NULL, /* for SPI FLASHes too */
>> > +       .option = 0,
>> > +       .flags = 0
>> > +};
>>
>> This wouldn't require - spi_alloc_slave() is a generic func to do all the stuff.
>> See comments below...
>>
>> > +
>> > +/* spi_init is called during boot when CONFIG_CMD_SPI is defined */
>> > +void spi_init(void)
>> > +{
>> > +       /*
>> > +        *  nothing to do: clocking was enabled in lpc32xx_ssp_enable()
>> > +        * and configuration will be done in spi_setup_slave()
>> > +       */
>> > +}
>> > +
>> > +/* the following is called in sequence by do_spi_xfer() */
>> > +
>> > +struct spi_slave *spi_setup_slave(uint bus, uint cs, uint max_hz, uint mode)
>> > +{
>>
>> Define spi_cs_is_valid() that wikk ensure the controlling bus and cs
>> options command prompt
>
> Not sure I understand what you mean here.
>
>> use spi_alloc_slave() and members will be having reg_base and slave that's.
>> See drivers/spi/zynq_spi.c for more info.
>>
>> Suggested code:
>> static struct ssp0_spi_slave {
>>      struct spi_slave slave;
>>      struct ssp_regs *base;
>> };
>>
>> static inline struct ssp0_regs *get_ssp0_spi_base(void)
>> {
>>            return (struct ssp0_regs *) SSP0_BASE;
>> }
>>
>> in spi_setup_slave()
>>
>> {
>>     struct ssp0_spi_slave *ssp0;
>>
>>     /* valid bus and cs */
>>
>>     /* allocate */
>>     ssp0 = spi_alloc_slave(struct ssp0_spi_slave, bus, cs);
>>
>>     ssp0->base = get_ssp0_spi_base();
>>
>> }
>
> Ditto -- I don't see in what respect the code you suggest is supposed to
> improve things. Can you please develop?
>
>> > +       /* we only set up SSP0 for now, so ignore bus */
>> > +
>> > +       if (mode & SPI_3WIRE) {
>> > +               error("3-wire mode not supported");
>> > +               return NULL;
>> > +       }
>> > +
>> > +       if (mode & SPI_SLAVE) {
>> > +               error("slave mode not supported\n");
>> > +               return NULL;
>> > +       }
>> > +
>> > +       if (mode & SPI_PREAMBLE) {
>> > +               error("preamble byte skipping not supported\n");
>> > +               return NULL;
>> > +       }
>> > +
>> > +       /*
>> > +        * 8 bit frame, SPI fmt, 500kbps -> clock divider is 26.
>> > +        * Set SCR to 0 and CPSDVSR to 26.
>> > +        */
>> > +
>> > +       writel(0x7, &ssp0_regs->cr0); /* 8-bit chunks, SPI, 1 clk/bit */
>> > +       writel(26, &ssp0_regs->cpsr); /* SSP clock = HCLK/26 = 500kbps */
>> > +       writel(0, &ssp0_regs->imsc); /* do not raise any interrupts */
>> > +       writel(0, &ssp0_regs->icr); /* clear any pending interrupt */
>> > +       writel(0, &ssp0_regs->dmacr); /* do not do DMAs */
>> > +       writel(SSP_CR1_SSP_ENABLE, &ssp0_regs->cr1); /* enable SSP0 */
>> > +       return &ssp0_slave;
>> > +}
>> > +
>> > +int spi_claim_bus(struct spi_slave *slave)
>> > +{
>> > +       /* only one bus and slave so far, always available */
>> > +       return 0;
>> > +}
>> > +
>> > +int spi_xfer(struct spi_slave *slave, unsigned int bitlen,
>> > +       const void *dout, void *din, unsigned long flags)
>> > +{
>> > +       int bytelen = bitlen >> 3;
>> > +       int idx_out = 0;
>> > +       int idx_in = 0;
>> > +       int start_time;
>> > +
>> > +       start_time = get_timer(0);
>> > +       while ((idx_out < bytelen) || (idx_in < bytelen)) {
>> > +               int status = readl(&ssp0_regs->sr);
>> > +               if ((idx_out < bytelen) && (status & SSP_SR_TNF))
>> > +                       writel(((u8 *)dout)[idx_out++], &ssp0_regs->data);
>> > +               if ((idx_in < bytelen) && (status & status & SSP_SR_RNE))
>> > +                       ((u8 *)din)[idx_in++] = readl(&ssp0_regs->data);
>> > +               if (get_timer(start_time) >= CONFIG_LPC32XX_SSP_TIMEOUT)
>> > +                       return -1;
>> > +       }
>> > +       return 0;
>> > +}
>> > +
>> > +void spi_release_bus(struct spi_slave *slave)
>> > +{
>> > +       /* do nothing */
>> > +}
>> > +
>> > +void spi_free_slave(struct spi_slave *slave)
>> > +{
>> > +       /* do nothing */
>> > +}
>> > --
>> > 2.1.0
>> >
>>
>> thanks!

thanks!
-- 
Jagan.


More information about the U-Boot mailing list