[PATCH 06/13] ARM: LPC32XX: Core architecture files
Uwe Kleine-König
u.kleine-koenig at pengutronix.de
Wed Feb 3 10:55:58 EST 2010
Hello Kevin,
On Wed, Jan 27, 2010 at 05:43:24PM -0800, wellsk40 at gmail.com wrote:
> From: Kevin Wells <wellsk40 at gmail.com>
>
> GPIO support functions for gpiolib, irq setup and handling, serial
> port support, and high resolution timer support used in the LPC32XX
> architecture.
>
> Signed-off-by: Kevin Wells <wellsk40 at gmail.com>
> ---
> arch/arm/mach-lpc32xx/gpiolib.c | 418 +++++++++++++++++++++++++++++++++++++++
> arch/arm/mach-lpc32xx/irq.c | 237 ++++++++++++++++++++++
> arch/arm/mach-lpc32xx/serial.c | 200 +++++++++++++++++++
> arch/arm/mach-lpc32xx/timer.c | 187 +++++++++++++++++
> 4 files changed, 1042 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm/mach-lpc32xx/gpiolib.c b/arch/arm/mach-lpc32xx/gpiolib.c
> new file mode 100644
> index 0000000..30cf252
> --- /dev/null
> +++ b/arch/arm/mach-lpc32xx/gpiolib.c
> @@ -0,0 +1,418 @@
> +/*
> + * arch/arm/mach-lpc32xx/gpiolib.c
> + *
> + * Author: Kevin Wells <kevin.wells at nxp.com>
> + *
> + * Copyright (C) 2010 NXP Semiconductors
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
This is an old address of the FSF. I'd recommend skipping the last
paragraph. Ditto for the other files.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/init.h>
> +#include <linux/io.h>
> +#include <linux/errno.h>
> +#include <linux/gpio.h>
> +
> +#include <mach/hardware.h>
> +#include <mach/platform.h>
> +
> +#define GPIOBASE io_p2v(GPIO_BASE)
> +
> +struct gpio_regs {
> + void __iomem *inp_state;
> + void __iomem *outp_set;
> + void __iomem *outp_clr;
> + void __iomem *dir_set;
> + void __iomem *dir_clr;
> +};
> +
> +/*
> + * GPIO names
> + */
> +static char *gpio_p0_names[GPIO_P0_MAX] = {
> + "p0.0", "p0.1", "p0.2", "p0.3",
> + "p0.4", "p0.5", "p0.6", "p0.7"
> +};
> +
> +static char *gpio_p1_names[GPIO_P1_MAX] = {
> + "p1.0", "p1.1", "p1.2", "p1.3",
> + "p1.4", "p1.5", "p1.6", "p1.7",
> + "p1.8", "p1.9", "p1.10", "p1.11",
> + "p1.12", "p1.13", "p1.14", "p1.15",
> + "p1.16", "p1.17", "p1.18", "p1.19",
> + "p1.20", "p1.21", "p1.22", "p1.23",
> +};
> +
> +static char *gpio_p2_names[GPIO_P2_MAX] = {
> + "p2.0", "p2.1", "p2.2", "p2.3",
> + "p2.4", "p2.5", "p2.6", "p2.7",
> + "p2.8", "p2.9", "p2.10", "p2.11",
> + "p2.12"
> +};
> +
> +static char *gpio_p3_names[GPIO_P3_MAX] = {
> + "gpi000", "gpio01", "gpio02", "gpio03",
> + "gpio04", "gpio05"
> +};
> +
> +static char *gpi_p3_names[GPI_P3_MAX] = {
> + "gpi00", "gpi01", "gpi02", "gpi03",
> + "gpi04", "gpi05", "gpi06", "gpi07",
> + "gpi08", "gpi09", "na", "na",
Quoting include/asm-generic/gpio.h:
" [names] must be an array of strings to use as alternative names for
the GPIOs in this chip. Any entry in the array may be NULL if there is
no alias for the GPIO [...]". So if I understand correctly use NULL
instead of "na".
IMHO there should be a few consts. I will take a look into gpiolib
later to eventually add them there to not get a warning if the names
provided are const.
> + "na", "na", "na", "gpi15",
> + "gpi16", "gpi17", "gpi18", "gpi19",
> + "gpi20", "gpi21", "gpi22", "gpi23",
> + "gpi24", "gpi25", "gpi26", "gpi27"
> +};
> +
> +static char *gpo_p3_names[GPO_P3_MAX] = {
> + "gpo00", "gpo01", "gpo02", "gpo03",
> + "gpo04", "gpo05", "gpo06", "gpo07",
> + "gpo08", "gpo09", "gpo10", "gpo11",
> + "gpo12", "gpo13", "gpo14", "gpo15",
> + "gpo16", "gpo17", "gpo18", "gpo19",
> + "gpo20", "gpo21", "gpo22", "gpo23"
> +};
> +
> +static struct gpio_regs gpio_grp_regs[] = {
> + {
> + .inp_state = (void __iomem *) GPIO_P0_INP_STATE(GPIOBASE),
> + .outp_set = (void __iomem *) GPIO_P0_OUTP_SET(GPIOBASE),
> + .outp_clr = (void __iomem *) GPIO_P0_OUTP_CLR(GPIOBASE),
> + .dir_set = (void __iomem *) GPIO_P0_DIR_SET(GPIOBASE),
> + .dir_clr = (void __iomem *) GPIO_P0_DIR_CLR(GPIOBASE),
> + },
> + {
> + .inp_state = (void __iomem *) GPIO_P1_INP_STATE(GPIOBASE),
> + .outp_set = (void __iomem *) GPIO_P1_OUTP_SET(GPIOBASE),
> + .outp_clr = (void __iomem *) GPIO_P1_OUTP_CLR(GPIOBASE),
> + .dir_set = (void __iomem *) GPIO_P1_DIR_SET(GPIOBASE),
> + .dir_clr = (void __iomem *) GPIO_P1_DIR_CLR(GPIOBASE),
> + },
> + {
> + .inp_state = (void __iomem *) GPIO_P2_INP_STATE(GPIOBASE),
> + .outp_set = (void __iomem *) GPIO_P2_OUTP_SET(GPIOBASE),
> + .outp_clr = (void __iomem *) GPIO_P2_OUTP_CLR(GPIOBASE),
> + .dir_set = (void __iomem *) GPIO_P2_DIR_SET(GPIOBASE),
> + .dir_clr = (void __iomem *) GPIO_P2_DIR_CLR(GPIOBASE),
> + },
> + {
> + .inp_state = (void __iomem *) GPIO_P3_INP_STATE(GPIOBASE),
> + .outp_set = (void __iomem *) GPIO_P3_OUTP_SET(GPIOBASE),
> + .outp_clr = (void __iomem *) GPIO_P3_OUTP_CLR(GPIOBASE),
> + .dir_set = (void __iomem *) GPIO_P2_DIR_SET(GPIOBASE),
> + .dir_clr = (void __iomem *) GPIO_P2_DIR_CLR(GPIOBASE),
> + },
> +};
> +
> +struct lpc32xx_gpio_chip {
> + struct gpio_chip chip;
> + struct gpio_regs *gpio_grp;
> +};
> +
> +static inline struct lpc32xx_gpio_chip *to_lpc32xx_gpio(
> + struct gpio_chip *gpc)
> +{
> + return container_of(gpc, struct lpc32xx_gpio_chip, chip);
> +}
> +
> +static void __set_gpio_dir_p012(struct lpc32xx_gpio_chip *group,
> + unsigned pin, int input)
> +{
> + if (input)
> + writel(1 << pin, group->gpio_grp->dir_clr);
readl/writel are used to "perform PCI memory accesses via an ioremap
region" and "are defined to perform little endian accesses". I think
most archs use __raw_readl/__raw_writel here.
> + else
> + writel(1 << pin, group->gpio_grp->dir_set);
> +}
> +
> +static void __set_gpio_dir_p3(struct lpc32xx_gpio_chip *group,
> + unsigned pin, int input)
> +{
> + u32 u;
> +
> + /* P3 GPIO pins are offset in the register to pin mapping */
> + u = (1 << (pin + 25));
Can you use a define for that? e.g.
#define pin_to_bit_p3(pin) (1 << (pin + 25))
and then for the p012 functions, too, for consistency.
> +
> + if (input)
> + writel(u, group->gpio_grp->dir_clr);
> + else
> + writel(u, group->gpio_grp->dir_set);
> +}
> +
> +static void __set_gpio_level_p012(struct lpc32xx_gpio_chip *group,
> + unsigned pin, int high)
> +{
> + if (high)
> + writel(1 << pin, group->gpio_grp->outp_set);
> + else
> + writel(1 << pin, group->gpio_grp->outp_clr);
> +}
> +
> +static void __set_gpio_level_p3(struct lpc32xx_gpio_chip *group,
> + unsigned pin, int high)
> +{
> + u32 u;
> +
> + /* P3 GPIO pins are offset in the register to pin mapping */
> + u = (1 << (pin + 25));
> +
> + if (high)
> + writel(u, group->gpio_grp->outp_set);
> + else
> + writel(u, group->gpio_grp->outp_clr);
> +}
> +
> +static void __set_gpo_level_p3(struct lpc32xx_gpio_chip *group,
> + unsigned pin, int high)
> +{
> + if (high)
> + writel(1 << pin, group->gpio_grp->outp_set);
> + else
> + writel(1 << pin, group->gpio_grp->outp_clr);
> +}
> +
> +static int __get_gpio_state_p012(struct lpc32xx_gpio_chip *group,
> + unsigned pin)
> +{
> + return (readl(group->gpio_grp->inp_state) >> pin) & 1;
> +}
> +
> +static int __get_gpio_state_p3(struct lpc32xx_gpio_chip *group,
> + unsigned pin)
> +{
> + int state;
> +
> + state = readl(group->gpio_grp->inp_state);
> +
> + /* P3 GPIO pin input mapping is not contiguous */
> + if (pin == 5)
> + state = (state >> 24) & 1;
> + else
> + state = (state >> (10 + pin)) & 1;
The mapping here is different from above in __set_gpio_dir_p3?? Please
someone kick the responsible hardware designer from me.
> + return state;
> +}
> +
> +static int __get_gpi_state_p3(struct lpc32xx_gpio_chip *group,
> + unsigned pin)
> +{
> + return (readl(group->gpio_grp->inp_state) >> pin) & 1;
> +}
> +
> +/*
> + * GENERIC_GPIO primitives.
> + */
> +static int lpc32xx_gpio_dir_input_p012(struct gpio_chip *chip,
> + unsigned pin)
> +{
> + struct lpc32xx_gpio_chip *group = to_lpc32xx_gpio(chip);
> +
> + __set_gpio_dir_p012(group, pin, 1);
> +
> + return 0;
> +}
> +
> +static int lpc32xx_gpio_dir_input_p3(struct gpio_chip *chip,
> + unsigned pin)
> +{
> + struct lpc32xx_gpio_chip *group = to_lpc32xx_gpio(chip);
> +
> + __set_gpio_dir_p3(group, pin, 1);
> +
> + return 0;
> +}
> +
> +static int lpc32xx_gpio_dir_in_always(struct gpio_chip *chip,
> + unsigned pin)
> +{
> + return 0;
> +}
> +
> +static int lpc32xx_gpio_get_value_p012(struct gpio_chip *chip, unsigned pin)
> +{
> + struct lpc32xx_gpio_chip *group = to_lpc32xx_gpio(chip);
> +
> + return __get_gpio_state_p012(group, pin);
> +}
> +
> +static int lpc32xx_gpio_get_value_p3(struct gpio_chip *chip, unsigned pin)
> +{
> + struct lpc32xx_gpio_chip *group = to_lpc32xx_gpio(chip);
> +
> + return __get_gpio_state_p3(group, pin);
> +}
> +
> +static int lpc32xx_gpi_get_value(struct gpio_chip *chip, unsigned pin)
> +{
> + struct lpc32xx_gpio_chip *group = to_lpc32xx_gpio(chip);
> +
> + return __get_gpi_state_p3(group, pin);
> +}
> +
> +static int lpc32xx_gpio_dir_output_p012(struct gpio_chip *chip, unsigned pin,
> + int value)
> +{
> + struct lpc32xx_gpio_chip *group = to_lpc32xx_gpio(chip);
> +
> + __set_gpio_dir_p012(group, pin, 0);
> +
> + return 0;
> +}
> +
> +static int lpc32xx_gpio_dir_output_p3(struct gpio_chip *chip, unsigned pin,
> + int value)
> +{
> + struct lpc32xx_gpio_chip *group = to_lpc32xx_gpio(chip);
> +
> + __set_gpio_dir_p3(group, pin, 0);
> +
> + return 0;
> +}
> +
> +static int lpc32xx_gpio_dir_out_always(struct gpio_chip *chip, unsigned pin,
> + int value)
> +{
> + return 0;
> +}
> +
> +static void lpc32xx_gpio_set_value_p012(struct gpio_chip *chip, unsigned pin,
> + int value)
> +{
> + struct lpc32xx_gpio_chip *group = to_lpc32xx_gpio(chip);
> +
> + __set_gpio_level_p012(group, pin, value);
> +}
> +
> +static void lpc32xx_gpio_set_value_p3(struct gpio_chip *chip, unsigned pin,
> + int value)
> +{
> + struct lpc32xx_gpio_chip *group = to_lpc32xx_gpio(chip);
> +
> + __set_gpio_level_p3(group, pin, value);
> +}
> +
> +static void lpc32xx_gpo_set_value(struct gpio_chip *chip, unsigned pin,
> + int value)
> +{
> + struct lpc32xx_gpio_chip *group = to_lpc32xx_gpio(chip);
> +
> + __set_gpo_level_p3(group, pin, value);
> +}
> +
> +static int lpc32xx_gpio_request(struct gpio_chip *chip, unsigned pin)
> +{
> + if (pin < chip->ngpio)
> + return 0;
> +
> + return -EINVAL;
> +}
> +
> +static struct lpc32xx_gpio_chip lpc32xx_gpiochip[] = {
> + {
> + .chip = {
> + .label = "gpio_p0",
> + .direction_input = lpc32xx_gpio_dir_input_p012,
> + .get = lpc32xx_gpio_get_value_p012,
> + .direction_output = lpc32xx_gpio_dir_output_p012,
> + .set = lpc32xx_gpio_set_value_p012,
> + .request = lpc32xx_gpio_request,
> + .base = GPIO_P0_GRP,
> + .ngpio = GPIO_P0_MAX,
> + .names = gpio_p0_names,
> + .can_sleep = 0,
> + },
> + .gpio_grp = &gpio_grp_regs[0],
These absolute array references tend to become wrong. There is no need
to have them in an array, so why not call it gpio_grp_regs_p0 etc.?
> + },
> + {
> + .chip = {
> + .label = "gpio_p1",
> + .direction_input = lpc32xx_gpio_dir_input_p012,
> + .get = lpc32xx_gpio_get_value_p012,
> + .direction_output = lpc32xx_gpio_dir_output_p012,
> + .set = lpc32xx_gpio_set_value_p012,
> + .request = lpc32xx_gpio_request,
> + .base = GPIO_P1_GRP,
> + .ngpio = GPIO_P1_MAX,
> + .names = gpio_p1_names,
> + .can_sleep = 0,
> + },
> + .gpio_grp = &gpio_grp_regs[1],
> + },
> + {
> + .chip = {
> + .label = "gpio_p2",
> + .direction_input = lpc32xx_gpio_dir_input_p012,
> + .get = lpc32xx_gpio_get_value_p012,
> + .direction_output = lpc32xx_gpio_dir_output_p012,
> + .set = lpc32xx_gpio_set_value_p012,
> + .request = lpc32xx_gpio_request,
> + .base = GPIO_P2_GRP,
> + .ngpio = GPIO_P2_MAX,
> + .names = gpio_p2_names,
> + .can_sleep = 0,
> + },
> + .gpio_grp = &gpio_grp_regs[2],
> + },
> + {
> + .chip = {
> + .label = "gpio_p3",
> + .direction_input = lpc32xx_gpio_dir_input_p3,
> + .get = lpc32xx_gpio_get_value_p3,
> + .direction_output = lpc32xx_gpio_dir_output_p3,
> + .set = lpc32xx_gpio_set_value_p3,
> + .request = lpc32xx_gpio_request,
> + .base = GPIO_P3_GRP,
> + .ngpio = GPIO_P3_MAX,
> + .names = gpio_p3_names,
> + .can_sleep = 0,
> + },
> + .gpio_grp = &gpio_grp_regs[3],
> + },
> + {
> + .chip = {
> + .label = "gpi_p3",
> + .direction_input = lpc32xx_gpio_dir_in_always,
> + .get = lpc32xx_gpi_get_value,
> + .request = lpc32xx_gpio_request,
> + .base = GPI_P3_GRP,
> + .ngpio = GPI_P3_MAX,
> + .names = gpi_p3_names,
> + .can_sleep = 0,
> + },
> + .gpio_grp = &gpio_grp_regs[3],
> + },
> + {
> + .chip = {
> + .label = "gpo_p3",
> + .direction_output = lpc32xx_gpio_dir_out_always,
> + .set = lpc32xx_gpo_set_value,
> + .request = lpc32xx_gpio_request,
> + .base = GPO_P3_GRP,
> + .ngpio = GPO_P3_MAX,
> + .names = gpo_p3_names,
> + .can_sleep = 0,
> + },
> + .gpio_grp = &gpio_grp_regs[3],
> + },
> +};
> +
> +void __init lpc32xx_gpio_init(void)
> +{
> + int i;
> +
> + for (i = 0; i < ARRAY_SIZE(lpc32xx_gpiochip); i++)
> + gpiochip_add(&lpc32xx_gpiochip[i].chip);
> +}
> diff --git a/arch/arm/mach-lpc32xx/irq.c b/arch/arm/mach-lpc32xx/irq.c
> new file mode 100644
> index 0000000..8a1b57d
> --- /dev/null
> +++ b/arch/arm/mach-lpc32xx/irq.c
> @@ -0,0 +1,237 @@
> +/*
> + * arch/arm/mach-lpc32xx/irq.c
> + *
> + * Author: Kevin Wells <kevin.wells at nxp.com>
> + *
> + * Copyright (C) 2010 NXP Semiconductors
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/types.h>
> +#include <linux/interrupt.h>
> +#include <linux/irq.h>
> +#include <linux/err.h>
> +#include <linux/io.h>
> +
> +#include <mach/irqs.h>
> +#include <mach/hardware.h>
> +#include <mach/platform.h>
> +
> +/*
> + * Default value represeting the Activation polarity of all internal
s/represeting/reprenseting/
> + * interrupt sources
> + */
> +#define MIC_APR_DEFAULT 0x3FF0EFE0
> +#define SIC1_APR_DEFAULT 0xFBD27186
> +#define SIC2_APR_DEFAULT 0x801810C0
> +
> +/*
> + * Default value represeting the Activation Type of all internal
> + * interrupt sources. All are level senesitive.
s/senesitive/sensitive/
> + */
> +#define MIC_ATR_DEFAULT 0x00000000
> +#define SIC1_ATR_DEFAULT 0x00026000
> +#define SIC2_ATR_DEFAULT 0x00000000
> +
> +static void get_controller(unsigned int irq, unsigned int *base,
> + unsigned int *irqbit)
> +{
> + if (irq < 32) {
> + *base = io_p2v(MIC_BASE);
> + *irqbit = 1 << irq;
> + } else if (irq < 64) {
> + *base = io_p2v(SIC1_BASE);
> + *irqbit = 1 << (irq - 32);
> + } else {
> + *base = io_p2v(SIC2_BASE);
> + *irqbit = 1 << (irq - 64);
> + }
> +}
> +
> +static void lpc32xx_mask_irq(unsigned int irq)
> +{
> + unsigned int reg, ctrl, mask;
> +
> + get_controller(irq, &ctrl, &mask);
> +
> + reg = readl(ctrl + INTC_MASK);
> + reg &= ~mask;
> + writel(reg, (ctrl + INTC_MASK));
as above s/readl/__raw_readl/; s/writel/__raw_writel/?
> +}
> +
> +static void lpc32xx_unmask_irq(unsigned int irq)
> +{
> + unsigned int reg, ctrl, mask;
> +
> + get_controller(irq, &ctrl, &mask);
> +
> + reg = readl(ctrl + INTC_MASK);
> + reg |= mask;
> + writel(reg, (ctrl + INTC_MASK));
> +}
> +
> +static void lpc32xx_mask_ack_irq(unsigned int irq)
> +{
> + unsigned int ctrl, mask;
> +
> + get_controller(irq, &ctrl, &mask);
> +
> + writel(mask, (ctrl + INTC_RAW_STAT));
> +}
> +
> +static int lpc32xx_set_irq_type(unsigned int irq, unsigned int type)
> +{
> + unsigned int reg, ctrl, mask;
> +
> + get_controller(irq, &ctrl, &mask);
> +
> + switch (type) {
> + case IRQ_TYPE_EDGE_RISING:
> + /* Rising edge sensitive */
> + reg = readl(ctrl + INTC_POLAR);
> + reg |= mask;
> + writel(reg, (ctrl + INTC_POLAR));
> + reg = readl(ctrl + INTC_ACT_TYPE);
> + reg |= mask;
> + writel(reg, (ctrl + INTC_ACT_TYPE));
> + set_irq_handler(irq, handle_edge_irq);
> + break;
> +
> + case IRQ_TYPE_EDGE_FALLING:
> + /* Falling edge sensitive */
> + reg = readl(ctrl + INTC_POLAR);
> + reg &= ~mask;
> + writel(reg, (ctrl + INTC_POLAR));
> + reg = readl(ctrl + INTC_ACT_TYPE);
> + reg |= mask;
> + writel(reg, (ctrl + INTC_ACT_TYPE));
> + set_irq_handler(irq, handle_edge_irq);
> + break;
did you test that you really need handle_edge_irq? Sane irq controllers
can (and should) use handle_level_irq for both level and edge sensitive
irqs. You only need handle_edge_irq if your controller doesn't remember
edge transitions while the irq is masked.
> + case IRQ_TYPE_LEVEL_LOW:
> + /* Low level sensitive */
> + reg = readl(ctrl + INTC_POLAR);
> + reg &= ~mask;
> + writel(reg, (ctrl + INTC_POLAR));
> + reg = readl(ctrl + INTC_ACT_TYPE);
> + reg &= ~mask;
> + writel(reg, (ctrl + INTC_ACT_TYPE));
> + set_irq_handler(irq, handle_level_irq);
> + break;
> +
> + case IRQ_TYPE_LEVEL_HIGH:
> + /* High level sensitive */
> + reg = readl(ctrl + INTC_POLAR);
> + reg |= mask;
> + writel(reg, (ctrl + INTC_POLAR));
> + reg = readl(ctrl + INTC_ACT_TYPE);
> + reg &= ~mask;
> + writel(reg, (ctrl + INTC_ACT_TYPE));
> + set_irq_handler(irq, handle_level_irq);
> + break;
You could increase code sharing here with something like:
static inline __set_irq_type(unsigned int ctrl, unsigned mask, unsigned polar, unsigned acttype)
{
unsigned reg = __raw_readl(ctrl + INTC_POLAR);
reg &= ~mask;
reg |= polar;
__raw_writel(reg, ctrl + INTC_POLAR);
reg = __raw_readl(ctrl + INTC_ACT_TYPE);
...
}
and then the different cases would only be:
...
case IRQ_TYPE_LEVEL_HIGH:
__set_irq_type(ctrl, mask, mask, 0);
> +
> + /* Other modes are not supported */
> + default:
> + return -ENOTSUPP;
> + }
I think you need to differenciate between unsupported an invalid types.
I.e.:
case IRQ_TYPE_EDGE_BOTH:
return -ENOTSUPP;
default:
return -EINVAL;
And I'm not sure if ENOTSUPP is the right value to return. I grepped a
bit around and returning -EINVAL for both unsupported and wrong values
seems common. (pnx4008_set_irq_type return -1 which looks worse than
-ENOTSUPP.)
> +
> + return 0;
> +}
> +
> +static void __init lpc32xx_set_default_mappings(unsigned int base,
> + unsigned int apr, unsigned int atr, unsigned int offset)
> +{
> + unsigned int i, lvl, type;
> +
> + /* Set activation levels for each interrupt */
> + i = 0;
> + while (i < 32) {
> + lvl = ((apr >> i) & 0x1) | (((atr >> i) & 0x1) << 1);
> + switch (lvl) {
> + case 0x0: /* Low polarity and level operation */
> + type = IRQ_TYPE_LEVEL_LOW;
> + break;
> +
> + case 0x1: /* High polarity and level operation */
> + type = IRQ_TYPE_LEVEL_HIGH;
> + break;
> +
> + case 0x2: /* Low polarity and edge operation */
> + type = IRQ_TYPE_EDGE_FALLING;
> + break;
> +
> + case 0x3: /* High polarity and edge operation */
> + default:
> + type = IRQ_TYPE_EDGE_RISING;
> + break;
> + }
> +
> + lpc32xx_set_irq_type((offset + i), type);
you could reuse the function I suggested above, here.
> + i++;
> + }
> +}
> +
> +static struct irq_chip lpc32xx_irq_chip = {
> + .ack = lpc32xx_mask_ack_irq,
.ack = ...mask_ack... ?
> + .mask = lpc32xx_mask_irq,
> + .unmask = lpc32xx_unmask_irq,
> + .set_type = lpc32xx_set_irq_type,
> +};
> +
> +void __init lpc32xx_init_irq(void)
> +{
> + unsigned int i, vloc;
> +
> + /* Setup MIC */
> + vloc = io_p2v(MIC_BASE);
> + writel(0, (vloc + INTC_MASK));
> + writel(MIC_APR_DEFAULT, (vloc + INTC_POLAR));
> + writel(MIC_ATR_DEFAULT, (vloc + INTC_ACT_TYPE));
> +
> + /* Setup SIC1 */
> + vloc = io_p2v(SIC1_BASE);
> + writel(0, (vloc + INTC_MASK));
> + writel(SIC1_APR_DEFAULT, (vloc + INTC_POLAR));
> + writel(SIC1_ATR_DEFAULT, (vloc + INTC_ACT_TYPE));
> +
> + /* Setup SIC2 */
> + vloc = io_p2v(SIC2_BASE);
> + writel(0, (vloc + INTC_MASK));
> + writel(SIC2_APR_DEFAULT, (vloc + INTC_POLAR));
> + writel(SIC2_ATR_DEFAULT, (vloc + INTC_ACT_TYPE));
> +
> + /* Configure supported IRQ's */
> + for (i = 0; i < NR_IRQS; i++) {
> + set_irq_flags(i, IRQF_VALID);
> + set_irq_chip(i, &lpc32xx_irq_chip);
> + }
> +
> + /* Set default mappings */
> + lpc32xx_set_default_mappings(io_p2v(MIC_BASE), MIC_APR_DEFAULT,
> + MIC_ATR_DEFAULT, 0);
> + lpc32xx_set_default_mappings(io_p2v(SIC1_BASE), SIC1_APR_DEFAULT,
> + SIC1_ATR_DEFAULT, 32);
> + lpc32xx_set_default_mappings(io_p2v(SIC2_BASE), SIC2_APR_DEFAULT,
> + SIC2_ATR_DEFAULT, 64);
> +
> + /* mask all interrupts except SUBIRQA and SUBFIQ */
Why don't you mask all irqs?
> + writel((1 << IRQ_SUB1IRQ) | (1 << IRQ_SUB2IRQ) |
> + (1 << IRQ_SUB1FIQ) | (1 << IRQ_SUB2FIQ),
> + (io_p2v(MIC_BASE) + INTC_MASK));
I wonder you want s/SUBIRQA/SUBIRQ/ ?
> + writel(0, (io_p2v(SIC1_BASE) + INTC_MASK));
> + writel(0, (io_p2v(SIC2_BASE) + INTC_MASK));
> +}
> diff --git a/arch/arm/mach-lpc32xx/serial.c b/arch/arm/mach-lpc32xx/serial.c
> new file mode 100644
> index 0000000..1c15121
> --- /dev/null
> +++ b/arch/arm/mach-lpc32xx/serial.c
> @@ -0,0 +1,200 @@
> +/*
> + * arch/arm/mach-lpc32xx/serial.c
> + *
> + * Author: Kevin Wells <kevin.wells at nxp.com>
> + *
> + * Copyright (C) 2010 NXP Semiconductors
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/types.h>
> +#include <linux/serial.h>
> +#include <linux/serial_core.h>
> +#include <linux/serial_reg.h>
> +#include <linux/serial_8250.h>
> +#include <linux/clk.h>
> +#include <linux/io.h>
> +
> +#include <mach/hardware.h>
> +#include <mach/platform.h>
> +#include <mach/io.h>
> +#include "common.h"
> +
> +/* Standard 8250/16550 compatible serial ports */
> +static struct plat_serial8250_port serial_std_platform_data[] = {
> +#ifdef CONFIG_ARCH_LPC32XX_UART5_ENABLE
> + {
> + .membase = (void *) io_p2v(UART5_BASE),
> + .mapbase = UART5_BASE,
> + .irq = IRQ_UART_IIR5,
I'd prefix all lpc32xx specific constants with LPC32XX_.
> + .uartclk = MAIN_OSC_FREQ,
MAIN_OSC_FREQ isn't defined yet.
> + .regshift = 2,
> + .iotype = UPIO_MEM32,
> + .flags = UPF_BOOT_AUTOCONF | UPF_BUGGY_UART |
> + UPF_SKIP_TEST,
> + },
> +#endif
> +#ifdef CONFIG_ARCH_LPC32XX_UART3_ENABLE
> + {
> + .membase = (void *) io_p2v(UART3_BASE),
> + .mapbase = UART3_BASE,
> + .irq = IRQ_UART_IIR3,
> + .uartclk = MAIN_OSC_FREQ,
> + .regshift = 2,
> + .iotype = UPIO_MEM32,
> + .flags = UPF_BOOT_AUTOCONF | UPF_BUGGY_UART |
> + UPF_SKIP_TEST,
> + },
> +#endif
> +#ifdef CONFIG_ARCH_LPC32XX_UART4_ENABLE
> + {
> + .membase = (void *) io_p2v(UART4_BASE),
> + .mapbase = UART4_BASE,
> + .irq = IRQ_UART_IIR4,
> + .uartclk = MAIN_OSC_FREQ,
> + .regshift = 2,
> + .iotype = UPIO_MEM32,
> + .flags = UPF_BOOT_AUTOCONF | UPF_BUGGY_UART |
> + UPF_SKIP_TEST,
> + },
> +#endif
> +#ifdef CONFIG_ARCH_LPC32XX_UART6_ENABLE
> + {
> + .membase = (void *) io_p2v(UART6_BASE),
> + .mapbase = UART6_BASE,
> + .irq = IRQ_UART_IIR6,
> + .uartclk = MAIN_OSC_FREQ,
> + .regshift = 2,
> + .iotype = UPIO_MEM32,
> + .flags = UPF_BOOT_AUTOCONF | UPF_BUGGY_UART |
> + UPF_SKIP_TEST,
> + },
> +#endif
> + { },
> +};
> +
> +struct uartinit {
> + char *uart_ck_name;
> + u32 ck_mode_mask;
> + u32 pdiv_clk_reg;
> +};
> +
> +static struct uartinit uartinit_data[] __initdata = {
> +#ifdef CONFIG_ARCH_LPC32XX_UART5_ENABLE
> + {
> + .uart_ck_name = "uart5_ck",
> + .ck_mode_mask = UART_CLKMODE_LOAD(UART_CLKMODE_ON, 5),
> + .pdiv_clk_reg = CLKPWR_UART5_CLK_CTRL(CLKPWR_IOBASE),
> + },
> +#endif
> +#ifdef CONFIG_ARCH_LPC32XX_UART3_ENABLE
> + {
> + .uart_ck_name = "uart3_ck",
> + .ck_mode_mask = UART_CLKMODE_LOAD(UART_CLKMODE_ON, 3),
> + .pdiv_clk_reg = CLKPWR_UART3_CLK_CTRL(CLKPWR_IOBASE),
> + },
> +#endif
> +#ifdef CONFIG_ARCH_LPC32XX_UART4_ENABLE
> + {
> + .uart_ck_name = "uart4_ck",
> + .ck_mode_mask = UART_CLKMODE_LOAD(UART_CLKMODE_ON, 4),
> + .pdiv_clk_reg = CLKPWR_UART4_CLK_CTRL(CLKPWR_IOBASE),
> + },
> +#endif
> +#ifdef CONFIG_ARCH_LPC32XX_UART6_ENABLE
> + {
> + .uart_ck_name = "uart6_ck",
> + .ck_mode_mask = UART_CLKMODE_LOAD(UART_CLKMODE_ON, 6),
> + .pdiv_clk_reg = CLKPWR_UART6_CLK_CTRL(CLKPWR_IOBASE),
> + },
> +#endif
does the order matter here? If not, why use 5,3,4,6?
> +};
> +
> +static struct platform_device serial_std_platform_device = {
> + .name = "serial8250",
> + .id = 0,
> + .dev = {
> + .platform_data = serial_std_platform_data,
> + },
> +};
> +
> +static struct platform_device *lpc32xx_serial_devs[] __initdata = {
> + &serial_std_platform_device,
> +};
> +
> +void __init lpc32xx_serial_init(void)
> +{
> + u32 tmp, clkmodes = 0;
> + struct clk *clk;
> + void *puart;
> + int i;
> +
> + /* UART clocks are off, let clock driver manage them */
> + __raw_writel(0, CLKPWR_UART_CLK_CTRL(CLKPWR_IOBASE));
> +
> + for (i = 0; i < ARRAY_SIZE(uartinit_data); i++) {
> + clk = clk_get(NULL, uartinit_data[i].uart_ck_name);
> + if (IS_ERR(clk)) {
> +#ifdef CONFIG_DEBUG_LL
> + /* A clock get failure can mean the console might
> + not work. It's possible this might not even
> + work. */
> + printascii("Serial port clock get failure!\n");
> +#endif
> + } else {
> + clk_enable(clk);
> + serial_std_platform_data[i].uartclk =
> + clk_get_rate(clk);
IIRC clk_get_rate is only valid for enabled clocks, so you might want to
check if clk_enable returned 0.
> + }
> +
> + /* Setup UART clock modes for all UARTs, disable autoclock */
> + clkmodes |= uartinit_data[i].ck_mode_mask;
Do you really want to do this if clk_enable or clk_get failed?
> +
> + /* pre-UART clock divider set to 1 */
> + writel(0x0101, uartinit_data[i].pdiv_clk_reg);
ditto.
> + }
> +
> + /* This needs to be done after all UART clocks are setup */
> + writel(clkmodes, UARTCTL_CLKMODE(io_p2v(UART_CTRL_BASE)));
> + for (i = 0; i < ARRAY_SIZE(uartinit_data) - 1; i++) {
> + /* Force a flush of the RX FIFOs to work around a HW bug */
> + puart = serial_std_platform_data[i].membase;
> + writel(0xC1, UART_IIR_FCR(puart));
> + writel(0x00, UART_DLL_FIFO(puart));
> + clkmodes = 64;
> + while (clkmodes--)
> + tmp = readl(UART_DLL_FIFO(puart));
The variable clkmodes seems to be misused here. IMHO it should be named
something like j. and can i have a
#define LPC32XX_UART_FIFO_SIZE 64
inclusive usage here?
> + writel(0, UART_IIR_FCR(puart));
> + }
> +
> + /* IrDA pulsing support on UART6. This only enables the IrDA mux */
> + tmp = readl(UARTCTL_CTRL(io_p2v(UART_CTRL_BASE)));
> +#ifdef CONFIG_ARCH_LPC32XX_UART6_IRDAMODE
> + tmp &= ~UART_UART6_IRDAMOD_BYPASS;
> +#else
> + tmp |= UART_UART6_IRDAMOD_BYPASS;
> +#endif
> + writel(tmp, UARTCTL_CTRL(io_p2v(UART_CTRL_BASE)));
> +
> + /* Disable UART5->USB transparent mode or USB won't work */
> + tmp = readl(UARTCTL_CTRL(io_p2v(UART_CTRL_BASE)));
> + tmp &= ~UART_U5_ROUTE_TO_USB;
> + writel(tmp, UARTCTL_CTRL(io_p2v(UART_CTRL_BASE)));
> +
> + platform_add_devices(lpc32xx_serial_devs,
> + ARRAY_SIZE(lpc32xx_serial_devs));
> +}
> diff --git a/arch/arm/mach-lpc32xx/timer.c b/arch/arm/mach-lpc32xx/timer.c
> new file mode 100644
> index 0000000..9c06346
> --- /dev/null
> +++ b/arch/arm/mach-lpc32xx/timer.c
> @@ -0,0 +1,187 @@
> +/*
> + * arch/arm/mach-lpc32xx/timer.c
> + *
> + * Author: Kevin Wells <kevin.wells at nxp.com>
> + *
> + * Copyright (C) 2009 - 2010 NXP Semiconductors
> + * Copyright (C) 2009 Fontys University of Applied Sciences, Eindhoven
> + * Ed Schouten <e.schouten at fontys.nl>
> + * Laurens Timmermans <l.timmermans at fontys.nl>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
> + */
> +
> +#include <linux/interrupt.h>
> +#include <linux/irq.h>
> +#include <linux/time.h>
> +#include <linux/err.h>
> +#include <linux/clockchips.h>
> +
> +#include <asm/mach/time.h>
> +
> +#include <mach/hardware.h>
> +#include <mach/platform.h>
> +#include "common.h"
> +
> +#define TIMER0_IOBASE io_p2v(TIMER0_BASE)
> +#define TIMER1_IOBASE io_p2v(TIMER1_BASE)
I like the register constants already providing a virtual address.
Something like:
#ifndef __REG
#ifndef __ASSEMBLY__
#define __REG(x) ((void __iomem __force *)io_p2v(x))
#else
#define __REG(x) io_p2v(x)
#endif
#define LPC32XX_TIMER0_IOBASE __REG(0x4004c000)
And for the few places where a physical address is needed you can define
__REG(x) to be just x.
> +static cycle_t lpc32xx_clksrc_read(struct clocksource *cs)
> +{
> + return (cycle_t)readl(TIMER_TC(TIMER1_IOBASE));
> +}
> +
> +static struct clocksource lpc32xx_clksrc = {
> + .name = "lpc32xx_clksrc",
> + .shift = 24,
> + .rating = 300,
> + .read = lpc32xx_clksrc_read,
> + .mask = CLOCKSOURCE_MASK(32),
> + .flags = CLOCK_SOURCE_IS_CONTINUOUS,
> +};
> +
> +static int lpc32xx_clkevt_next_event(unsigned long delta,
> + struct clock_event_device *dev)
> +{
> + unsigned long flags;
> +
> + if (delta < 1)
> + return -ETIME;
As you set min_delta_ns below you don't need to test it here.
> + local_irq_save(flags);
No. irqs are already disabled by the clock framework.
> + writel(TIMER_CNTR_TCR_RESET, TIMER_TCR(TIMER0_IOBASE));
> + writel(delta, TIMER_PR(TIMER0_IOBASE));
> + writel(TIMER_CNTR_TCR_EN, TIMER_TCR(TIMER0_IOBASE));
> +
> + local_irq_restore(flags);
> +
> + return 0;
> +}
> +
> +static void lpc32xx_clkevt_mode(enum clock_event_mode mode,
> + struct clock_event_device *dev)
> +{
> + switch (mode) {
> + case CLOCK_EVT_MODE_PERIODIC:
> + WARN_ON(1);
> + break;
> +
> + case CLOCK_EVT_MODE_ONESHOT:
> + case CLOCK_EVT_MODE_SHUTDOWN:
> + /*
> + * Disable the timer. When using oneshot, we must also
> + * disable the timer to wait for the first call to
> + * set_next_event().
> + */
> + writel(0, TIMER_TCR(TIMER0_IOBASE));
> + break;
> +
> + case CLOCK_EVT_MODE_UNUSED:
> + case CLOCK_EVT_MODE_RESUME:
> + break;
> + }
> +}
> +
> +static struct clock_event_device lpc32xx_clkevt = {
> + .name = "lpc32xx_clkevt",
> + .features = CLOCK_EVT_FEAT_ONESHOT,
> + .shift = 32,
> + .rating = 300,
> + .set_next_event = lpc32xx_clkevt_next_event,
> + .set_mode = lpc32xx_clkevt_mode,
> +};
> +
> +static irqreturn_t lpc32xx_timer_interrupt(int irq, void *dev_id)
> +{
> + struct clock_event_device *evt = &lpc32xx_clkevt;
> +
> + /* Clear match */
> + writel(TIMER_CNTR_MTCH_BIT(0), TIMER_IR(TIMER0_IOBASE));
> +
> + evt->event_handler(evt);
> +
> + return IRQ_HANDLED;
> +}
> +
> +static struct irqaction lpc32xx_timer_irq = {
> + .name = "LPC32XX Timer Tick",
> + .flags = IRQF_DISABLED | IRQF_TIMER | IRQF_IRQPOLL,
> + .handler = lpc32xx_timer_interrupt,
> +};
> +
> +/*
> + * The clock management driver isn't initialized at this point, so the
> + * clocks need to be enabled here manually and then tagged as used in
> + * the clock driver initialization
> + */
> +static void __init lpc32xx_timer_init(void)
> +{
> + u32 clkrate, pllreg;
> +
> + /* Enable timer clock */
> + writel(
> + (CLKPWR_TMRPWMCLK_TIMER0_EN | CLKPWR_TMRPWMCLK_TIMER1_EN),
> + CLKPWR_TIMERS_PWMS_CLK_CTRL_1(CLKPWR_IOBASE));
> +
> + /* The clock driver isn't initialized at this point. So determine if
> + the SYSCLK is driven from the PLL397 or main oscillator and then use
> + it to compute the PLL frequency and the PCLK divider to get the base
> + timer rates. This rate is needed to compute the tick rate. */
the common format for multiline comments is
/*
* ...
* ...
*/
> + if (clk_is_sysclk_mainosc() != 0)
> + clkrate = MAIN_OSC_FREQ;
> + else
> + clkrate = 397 * CLOCK_OSC_FREQ;
> +
> + /* Get ARM HCLKPLL register and convert it into a frequency*/
missing space -----------------------------------------------------^
> + pllreg = readl(CLKPWR_HCLKPLL_CTRL(CLKPWR_IOBASE)) & 0x1FFFF;
> + clkrate = clk_get_pllrate_from_reg(clkrate, pllreg);
> +
> + /* Get PCLK divider and divide ARM PLL clock by it to get timer rate */
> + clkrate = clkrate / clk_get_pclk_div();
> +
> + /* Initial timer setup */
> + writel(0, TIMER_TCR(TIMER0_IOBASE));
> + writel(TIMER_CNTR_MTCH_BIT(0), TIMER_IR(TIMER0_IOBASE));
> + writel(1, TIMER_MR0(TIMER0_IOBASE));
> + writel(TIMER_CNTR_MCR_MTCH(0) | TIMER_CNTR_MCR_STOP(0) |
> + TIMER_CNTR_MCR_RESET(0), TIMER_MCR(TIMER0_IOBASE));
> +
> + /* Setup tick interrupt */
> + setup_irq(IRQ_TIMER0, &lpc32xx_timer_irq);
> +
> + /* Setup the clockevent structure. */
> + lpc32xx_clkevt.mult = div_sc(clkrate, NSEC_PER_SEC,
> + lpc32xx_clkevt.shift);
> + lpc32xx_clkevt.max_delta_ns = clockevent_delta2ns(-1,
> + &lpc32xx_clkevt);
> + lpc32xx_clkevt.min_delta_ns = clockevent_delta2ns(1, &lpc32xx_clkevt);
For rounding reasons this might not be enough to assert delta being >= 1
for set_next event. IIRC you need
clockevent_delta2ns(1, &lpc32xx_clkevt) + 1
> + lpc32xx_clkevt.cpumask = cpumask_of(0);
> + clockevents_register_device(&lpc32xx_clkevt);
> +
> + /* Use timer1 as clock source. */
> + writel(TIMER_CNTR_TCR_RESET, TIMER_TCR(TIMER1_IOBASE));
> + writel(0, TIMER_PR(TIMER1_IOBASE));
> + writel(0, TIMER_MCR(TIMER1_IOBASE));
> + writel(TIMER_CNTR_TCR_EN, TIMER_TCR(TIMER1_IOBASE));
> + lpc32xx_clksrc.mult = clocksource_hz2mult(clkrate,
> + lpc32xx_clksrc.shift);
> + clocksource_register(&lpc32xx_clksrc);
> +}
> +
> +struct sys_timer lpc32xx_timer = {
> + .init = &lpc32xx_timer_init,
> +};
Does this work without NO_HZ? I though you need to setup a periodic
mode with CONFIG_HZ irqs per second first?
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | http://www.pengutronix.de/ |
More information about the linux-arm-kernel
mailing list