From mboxrd@z Thu Jan 1 00:00:00 1970 From: Eric Miao Subject: Re: [RFC 2.6.26-rc3 06/10] gumstix: conversion to MFP support and add bluetooth support Date: Fri, 13 Jun 2008 10:01:13 +0800 Message-ID: <4851D4E9.3080008@gmail.com> References: <1213289961-1562-1-git-send-email-jayakumar.lkml@gmail.com> <1213289961-1562-7-git-send-email-jayakumar.lkml@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from sc8-sf-mx1-b.sourceforge.net ([10.3.1.91] helo=mail.sourceforge.net) by sc8-sf-list1-new.sourceforge.net with esmtp (Exim 4.43) id 1K6ybU-0003Bx-32 for linux-fbdev-devel@lists.sourceforge.net; Thu, 12 Jun 2008 19:01:24 -0700 Received: from rv-out-0708.google.com ([209.85.198.244]) by mail.sourceforge.net with esmtp (Exim 4.44) id 1K6ybS-0004AE-Jx for linux-fbdev-devel@lists.sourceforge.net; Thu, 12 Jun 2008 19:01:24 -0700 Received: by rv-out-0708.google.com with SMTP id f25so3498731rvb.22 for ; Thu, 12 Jun 2008 19:01:22 -0700 (PDT) In-Reply-To: <1213289961-1562-7-git-send-email-jayakumar.lkml@gmail.com> List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: linux-fbdev-devel-bounces@lists.sourceforge.net Errors-To: linux-fbdev-devel-bounces@lists.sourceforge.net To: Jaya Kumar Cc: ymiao3@marvell.com, linux-fbdev-devel@lists.sourceforge.net, linux-arm-kernel@lists.arm.linux.org.uk Jaya Kumar wrote: > This patch converts gumstix over to the MFP configuration system and adds > bluetooth support. Added a CLK_32K clock to enable bluetooth. > Overall looks ok to me, some concerns about the 32K timer, see my comments below. > Signed-off-by: Jaya Kumar > --- > arch/arm/mach-pxa/gumstix.c | 79 ++++++++++++++++++++++++++------- > arch/arm/mach-pxa/pxa25x.c | 31 +++++++++++++ > include/asm-arm/arch-pxa/mfp-pxa25x.h | 1 + > 3 files changed, 95 insertions(+), 16 deletions(-) > > diff --git a/arch/arm/mach-pxa/gumstix.c b/arch/arm/mach-pxa/gumstix.c > index bdf2397..3e0a962 100644 > --- a/arch/arm/mach-pxa/gumstix.c > +++ b/arch/arm/mach-pxa/gumstix.c > @@ -20,8 +20,12 @@ > #include > #include > #include > +#include > #include > #include > +#include > +#include > +#include > > #include > #include > @@ -40,7 +44,7 @@ > > #include > #include > -#include > +#include > > #include "generic.h" > > @@ -85,21 +89,8 @@ static struct platform_device *devices[] __initdata = { > }; > > #ifdef CONFIG_MMC_PXA I'd suggest we use #if defined(CONFIG_MMC_PXA) || defined(CONFIG_MMC_PXA_MODULE) so that even when PXA MMCI is built as a module, the platform data is still available for later bind. Same below to the BT. > -static struct pxamci_platform_data gumstix_mci_platform_data; > - > -static int gumstix_mci_init(struct device *dev, irq_handler_t detect_int, > - void *data) > -{ > - pxa_gpio_mode(GPIO6_MMCCLK_MD); > - pxa_gpio_mode(GPIO53_MMCCLK_MD); > - pxa_gpio_mode(GPIO8_MMCCS0_MD); > - > - return 0; > -} > - > static struct pxamci_platform_data gumstix_mci_platform_data = { > .ocr_mask = MMC_VDD_32_33|MMC_VDD_33_34, > - .init = gumstix_mci_init, > }; > > static void __init gumstix_mmc_init(void) > @@ -109,7 +100,7 @@ static void __init gumstix_mmc_init(void) > #else > static void __init gumstix_mmc_init(void) > { > - printk(KERN_INFO "Gumstix mmc disabled\n"); > + pr_debug("Gumstix mmc disabled\n"); > } > #endif > > @@ -126,12 +117,68 @@ static void __init gumstix_udc_init(void) > #else > static void gumstix_udc_init(void) > { > - printk(KERN_INFO "Gumstix udc is disabled\n"); > + pr_debug("Gumstix udc is disabled\n"); > +} > +#endif > + > +#ifdef CONFIG_BT > +static void __init gumstix_bluetooth_init(void) > +{ > + int err; > + struct clk *clk32k; > + > + clk32k = clk_get(NULL, "CLK_32K"); > + if (IS_ERR(clk32k)) { > + pr_err("gumstix: failed to find 32kHz clock\n"); > + return; > + } > + clk_enable(clk32k); The 32KHz oscillator should be on by default, so I'm thinking of: 1. remove this clock source or 2. enable/disable the clk by enable/disable the pin configuration (i.e. switch between GPIO12_32KHz and GPIO12_GPIO) > + > + err = gpio_direction_output(GPIO_GUMSTIX_BTRESET, 1); > + if (err) { > + pr_err("gumstix: can't reset bluetooth\n"); > + return; > + } > + gpio_set_value(GPIO_GUMSTIX_BTRESET, 0); > + udelay(100); > + gpio_set_value(GPIO_GUMSTIX_BTRESET, 1); > +} > +#else > +static void gumstix_bluetooth_init(void) > +{ > + pr_debug("Gumstix Bluetooth is disabled\n"); > } > #endif > > +static unsigned long gumstix_pin_config[] __initdata = { > +#ifdef CONFIG_BT > + GPIO12_32KHz, > + /* BTUART */ > + GPIO42_HWUART_RXD, > + GPIO43_HWUART_TXD, > + GPIO44_HWUART_CTS, > + GPIO45_HWUART_RTSL, > +#endif > +#ifdef CONFIG_MMC_PXA > + /* MMC */ > + GPIO6_MMC_CLK, > + GPIO53_MMC_CLK, > + GPIO8_MMC_CS0, > +#endif > + /* these are used by AM200EPD */ > + GPIO51_GPIO, > + GPIO49_GPIO, > + GPIO48_GPIO, > + GPIO32_GPIO, > + GPIO17_GPIO, > + GPIO16_GPIO, > +}; > + > static void __init gumstix_init(void) > { > + pxa2xx_mfp_config(ARRAY_AND_SIZE(gumstix_pin_config)); > + > + gumstix_bluetooth_init(); > gumstix_udc_init(); > gumstix_mmc_init(); > (void) platform_add_devices(devices, ARRAY_SIZE(devices)); > diff --git a/arch/arm/mach-pxa/pxa25x.c b/arch/arm/mach-pxa/pxa25x.c > index e5b417d..1ffe773 100644 > --- a/arch/arm/mach-pxa/pxa25x.c > +++ b/arch/arm/mach-pxa/pxa25x.c > @@ -22,6 +22,7 @@ > #include > #include > #include > +#include > > #include > #include > @@ -108,6 +109,30 @@ static const struct clkops clk_pxa25x_lcd_ops = { > .getrate = clk_pxa25x_lcd_getrate, > }; > > +static void clk_32k_enable(struct clk *clk) > +{ > + int timeout = 500; > + > + OSCC |= OSCC_OON; > + do { > + if (OSCC & OSCC_OOK) > + break; > + udelay(1); > + } while (--timeout); > + if (!timeout) > + pr_err("Failed to start 32kHz clock\n"); > +} As said, the 32K timer should really be started on the very begining, I'd prefer a some kind of warning here if OSCC_OOK isn't set. > + > +static void clk_32k_disable(struct clk *clk) > +{ > + OSCC &= ~OSCC_OON; > +} > + The 32K timer cannot be disabled once started, until a power-on or hardware reset occurs. > +static const struct clkops clk_32k_ops = { > + .enable = clk_32k_enable, > + .disable = clk_32k_disable, > +}; > + > /* > * 3.6864MHz -> OST, GPIO, SSP, PWM, PLLs (95.842MHz, 147.456MHz) > * 95.842MHz -> MMC 19.169MHz, I2C 31.949MHz, FICP 47.923MHz, USB 47.923MHz > @@ -118,6 +143,12 @@ static struct clk pxa25x_hwuart_clk = > ; > > static struct clk pxa25x_clks[] = { > + { > + .name = "CLK_32K", > + .ops = &clk_32k_ops, > + .rate = 32000, > + .delay = 70, > + }, > INIT_CK("LCDCLK", LCD, &clk_pxa25x_lcd_ops, &pxa_device_fb.dev), > INIT_CKEN("UARTCLK", FFUART, 14745600, 1, &pxa_device_ffuart.dev), > INIT_CKEN("UARTCLK", BTUART, 14745600, 1, &pxa_device_btuart.dev), > diff --git a/include/asm-arm/arch-pxa/mfp-pxa25x.h b/include/asm-arm/arch-pxa/mfp-pxa25x.h > index 0499323..6c82a74 100644 > --- a/include/asm-arm/arch-pxa/mfp-pxa25x.h > +++ b/include/asm-arm/arch-pxa/mfp-pxa25x.h > @@ -79,6 +79,7 @@ > #define GPIO43_HWUART_TXD MFP_CFG_OUT(GPIO43, AF3, DRIVE_HIGH) > #define GPIO44_HWUART_CTS MFP_CFG_IN(GPIO44, AF3) > #define GPIO45_HWUART_RTS MFP_CFG_OUT(GPIO45, AF3, DRIVE_HIGH) > +#define GPIO45_HWUART_RTSL MFP_CFG_OUT(GPIO45, AF3, DRIVE_LOW) Is this board specific or globally applicable? ISTR, RTS is low active, so a HIGH in low power mode would be more appropriate that the remote DCE will hold its data to send. > #define GPIO48_HWUART_TXD MFP_CFG_OUT(GPIO48, AF1, DRIVE_HIGH) > #define GPIO49_HWUART_RXD MFP_CFG_IN(GPIO49, AF1) > #define GPIO50_HWUART_CTS MFP_CFG_IN(GPIO50, AF1) ------------------------------------------------------------------------- Check out the new SourceForge.net Marketplace. It's the best place to buy or sell services for just about anything Open Source. http://sourceforge.net/services/buy/index.php