On Wednesday, April 28, 2010 10:51 AM, Mika Westerberg wrote:
>
> This patch adds platform side support code for the EP93xx SPI driver. This
> includes clock, resources and muxing. There is a new function: 
> ep93xx_register_spi()
> which can be used by board support code to register new SPI devices for the 
> board.
>
> Patch depends on following ARM patch:
>       5998/1 ep93xx: added chip revision reading function
>
> Signed-off-by: Mika Westerberg <[email protected]>
> ---
>  arch/arm/mach-ep93xx/clock.c                    |   13 ++++++
>  arch/arm/mach-ep93xx/core.c                     |   50 
> +++++++++++++++++++++++
>  arch/arm/mach-ep93xx/include/mach/ep93xx-regs.h |    1 +
>  arch/arm/mach-ep93xx/include/mach/platform.h    |    4 ++
>  4 files changed, 68 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/mach-ep93xx/clock.c b/arch/arm/mach-ep93xx/clock.c
> index 5f80092..e29bdef 100644
> --- a/arch/arm/mach-ep93xx/clock.c
> +++ b/arch/arm/mach-ep93xx/clock.c
> @@ -96,6 +96,10 @@ static struct clk clk_keypad = {
>       .enable_mask    = EP93XX_SYSCON_KEYTCHCLKDIV_KEN,
>       .set_rate       = set_keytchclk_rate,
>  };
> +static struct clk clk_spi = {
> +     .parent         = &clk_xtali,
> +     .rate           = EP93XX_EXT_CLK_RATE,
> +};
>  static struct clk clk_pwm = {
>       .parent         = &clk_xtali,
>       .rate           = EP93XX_EXT_CLK_RATE,
> @@ -186,6 +190,7 @@ static struct clk_lookup clocks[] = {
>       INIT_CK("ep93xx-ohci",          NULL,           &clk_usb_host),
>       INIT_CK("ep93xx-keypad",        NULL,           &clk_keypad),
>       INIT_CK("ep93xx-fb",            NULL,           &clk_video),
> +     INIT_CK("ep93xx-spi.0",         NULL,           &clk_spi),
>       INIT_CK(NULL,                   "pwm_clk",      &clk_pwm),
>       INIT_CK(NULL,                   "m2p0",         &clk_m2p0),
>       INIT_CK(NULL,                   "m2p1",         &clk_m2p1),
> @@ -473,6 +478,14 @@ static int __init ep93xx_clock_init(void)
>       /* Initialize the pll2 derived clocks */
>       clk_usb_host.rate = clk_pll2.rate / (((value >> 28) & 0xf) + 1);
>  
> +     /*
> +      * EP93xx SSP clock rate was doubled in version E2. For more information
> +      * see:
> +      *     http://www.cirrus.com/en/pubs/appNote/AN273REV4.pdf
> +      */
> +     if (ep93xx_chip_revision() < EP93XX_CHIP_REV_E2)
> +             clk_spi.rate /= 2;
> +
>       pr_info("PLL1 running at %ld MHz, PLL2 at %ld MHz\n",
>               clk_pll1.rate / 1000000, clk_pll2.rate / 1000000);
>       pr_info("FCLK %ld MHz, HCLK %ld MHz, PCLK %ld MHz\n",

This all looks good.

> diff --git a/arch/arm/mach-ep93xx/core.c b/arch/arm/mach-ep93xx/core.c
> index 90fb591..d57b61e 100644
> --- a/arch/arm/mach-ep93xx/core.c
> +++ b/arch/arm/mach-ep93xx/core.c
> @@ -29,12 +29,14 @@
>  #include <linux/termios.h>
>  #include <linux/amba/bus.h>
>  #include <linux/amba/serial.h>
> +#include <linux/spi/spi.h>

Nitpick... Please put this after the i2c includes below.

>  #include <linux/i2c.h>
>  #include <linux/i2c-gpio.h>
>  
>  #include <mach/hardware.h>
>  #include <mach/fb.h>
>  #include <mach/ep93xx_keypad.h>
> +#include <mach/ep93xx_spi.h>
>  
>  #include <asm/mach/map.h>
>  #include <asm/mach/time.h>
> @@ -363,6 +365,54 @@ void __init ep93xx_register_eth(struct ep93xx_eth_data 
> *data, int copy_addr)
>       platform_device_register(&ep93xx_eth_device);
>  }
>  
> +static struct ep93xx_spi_info ep93xx_spi_master_data;
> +
> +static struct resource ep93xx_spi_resources[] = {
> +     {
> +             .start  = EP93XX_SPI_PHYS_BASE,
> +             .end    = EP93XX_SPI_PHYS_BASE + 0x18 - 1,
> +             .flags  = IORESOURCE_MEM,
> +     },
> +     {
> +             .start  = IRQ_EP93XX_SSP,
> +             .end    = IRQ_EP93XX_SSP,
> +             .flags  = IORESOURCE_IRQ,
> +     },
> +};
> +
> +static struct platform_device ep93xx_spi_device = {
> +     .name           = "ep93xx-spi",
> +     .id             = 0,
> +     .dev            = {
> +             .platform_data = &ep93xx_spi_master_data,
> +     },
> +     .num_resources  = ARRAY_SIZE(ep93xx_spi_resources),
> +     .resource       = ep93xx_spi_resources,
> +};
> +
> +/**
> + * ep93xx_register_spi() - registers spi platform device
> + * @info: ep93xx board specific spi master info (__initdata)
> + * @devices: SPI devices to register

Please add (__initdata) at the end of the line above.

> + * @num: number of SPI devices to register
> + *
> + * This function registers platform device for the EP93xx SPI controller and
> + * also makes sure that SPI pins are muxed so that I2S is not using those 
> pins.
> + * Caller should allocate necessary GPIO lines before before calling this.
> + */
> +void __init ep93xx_register_spi(struct ep93xx_spi_info *info,
> +                             struct spi_board_info *devices, int num)
> +{
> +     /*
> +      * When SPI is used, we need to make sure that I2S is muxed off from
> +      * SPI pins.
> +      */
> +     ep93xx_devcfg_clear_bits(EP93XX_SYSCON_DEVCFG_I2SONSSP);
> +
> +     ep93xx_spi_master_data = *info;
> +     spi_register_board_info(devices, num);
> +     platform_device_register(&ep93xx_spi_device);
> +}

Nitpick... Please move the spi block above after the i2c block below it.

>  /*************************************************************************
>   * EP93xx i2c peripheral handling

Other than the comments above, this all looks good.

> diff --git a/arch/arm/mach-ep93xx/include/mach/ep93xx-regs.h 
> b/arch/arm/mach-ep93xx/include/mach/ep93xx-regs.h
> index 93e2ecc..b1e096f 100644
> --- a/arch/arm/mach-ep93xx/include/mach/ep93xx-regs.h
> +++ b/arch/arm/mach-ep93xx/include/mach/ep93xx-regs.h
> @@ -106,6 +106,7 @@
>  
>  #define EP93XX_AAC_BASE                      EP93XX_APB_IOMEM(0x00080000)
>  
> +#define EP93XX_SPI_PHYS_BASE         EP93XX_APB_PHYS(0x000a0000)
>  #define EP93XX_SPI_BASE                      EP93XX_APB_IOMEM(0x000a0000)
>  
>  #define EP93XX_IRDA_BASE             EP93XX_APB_IOMEM(0x000b0000)

This is good.

> diff --git a/arch/arm/mach-ep93xx/include/mach/platform.h 
> b/arch/arm/mach-ep93xx/include/mach/platform.h
> index c6dc14d..52eebfd 100644
> --- a/arch/arm/mach-ep93xx/include/mach/platform.h
> +++ b/arch/arm/mach-ep93xx/include/mach/platform.h
> @@ -4,11 +4,13 @@
>  
>  #ifndef __ASSEMBLY__
>  
> +struct spi_board_info;
>  struct i2c_gpio_platform_data;
>  struct i2c_board_info;
>  struct platform_device;
>  struct ep93xxfb_mach_info;
>  struct ep93xx_keypad_platform_data;
> +struct ep93xx_spi_info;
>  
>  struct ep93xx_eth_data
>  {
> @@ -34,6 +36,8 @@ static inline void ep93xx_devcfg_clear_bits(unsigned int 
> bits)
>  }
>  
>  void ep93xx_register_eth(struct ep93xx_eth_data *data, int copy_addr);
> +void ep93xx_register_spi(struct ep93xx_spi_info *info,
> +                      struct spi_board_info *devices, int num);
>  void ep93xx_register_i2c(struct i2c_gpio_platform_data *data,
>                        struct i2c_board_info *devices, int num);
>  void ep93xx_register_fb(struct ep93xxfb_mach_info *data);

Nitpick... Again, please move the spi stuff to after the i2c stuff.

I would like them moved since an i2c gpio expander "could" be used to provide
the chip selects for the spi controller.  Putting them in this order provides a
hint to the registration order for the devices to get the bindings correct. I
will provide the necessary patch to you shortly.

Again, this is all just nitpick stuff.

Overall this is all good.

Regards,
Hartley
------------------------------------------------------------------------------
_______________________________________________
spi-devel-general mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/spi-devel-general

Reply via email to