[...]
> > With register offsets now defined for respective OMAP versions we can
> get rid
> > of cpu_class_* checks. This function now has common initialization code
> for
> > all OMAP versions. Initialization specific to OMAP16xx has been moved
> within
> > omap16xx_gpio_init().
>
> This patch also adds the ->do_raw_write() stuff which is not described
> here, and is an unrelated change that wants its own patch. More on that
> below.
Ok.
>
> > Signed-off-by: Tarun Kanti DebBarma <tarun.ka...@ti.com>
> > Signed-off-by: Charulatha V <ch...@ti.com>
> > ---
> > arch/arm/mach-omap1/gpio16xx.c | 19 ++++++-
> > arch/arm/plat-omap/include/plat/gpio.h | 2 +
> > drivers/gpio/gpio-omap.c | 96 +++++++++++++++----------
> -------
> > 3 files changed, 64 insertions(+), 53 deletions(-)
> >
> > diff --git a/arch/arm/mach-omap1/gpio16xx.c b/arch/arm/mach-
> omap1/gpio16xx.c
> > index 08dda3c..cfd86b9 100644
> > --- a/arch/arm/mach-omap1/gpio16xx.c
> > +++ b/arch/arm/mach-omap1/gpio16xx.c
> > @@ -93,6 +93,7 @@ static struct omap_gpio_reg_offs omap16xx_gpio_regs =
> {
> > .wkup_status = OMAP1610_GPIO_WAKEUPENABLE,
> > .edgectrl1 = OMAP1610_GPIO_EDGE_CTRL1,
> > .edgectrl2 = OMAP1610_GPIO_EDGE_CTRL2,
> > + .sysconfig = OMAP1610_GPIO_SYSCONFIG,
> > };
> >
> > static struct __initdata omap_gpio_platform_data omap16xx_gpio1_config
> = {
> > @@ -218,12 +219,28 @@ static struct __initdata platform_device *
> omap16xx_gpio_dev[] = {
> > static int __init omap16xx_gpio_init(void)
> > {
> > int i;
> > + void __iomem *base;
> > + struct resource *res;
> > + struct platform_device *pdev;
> > + struct omap_gpio_platform_data *pdata;
> >
> > if (!cpu_is_omap16xx())
> > return -EINVAL;
> >
> > - for (i = 0; i < ARRAY_SIZE(omap16xx_gpio_dev); i++)
> > + for (i = 0; i < ARRAY_SIZE(omap16xx_gpio_dev); i++) {
> > + pdev = omap16xx_gpio_dev[i];
> > + pdata = pdev->dev.platform_data;
> > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > + base = ioremap(res->start, resource_size(res));
> > +
> > + if (pdata->regs->sysconfig)
>
> using field from regs-> here...
>
> > + __raw_writew(0x0014, base + OMAP1610_GPIO_SYSCONFIG);
>
> but the #define here.
>
> Since you don't really need this field in reg_offs (because it's not
> used in SoC-independent code), just drop the if and always write
> sysconfig.
Yes. In fact I thought the same where it is redundant to add 16xx specific
Register in reg_offs. Thanks.
I will make the change.
>
> > + omap_writel(omap_readl(ULPD_CAM_CLK_CTRL) | 0x04,
> > + ULPD_CAM_CLK_CTRL);
>
> Please preserve the comments from the original code, since the CAM
> access does lok rather confusing.
Sure.
>
> > platform_device_register(omap16xx_gpio_dev[i]);
> > + }
> >
> > return 0;
> > }
> > diff --git a/arch/arm/plat-omap/include/plat/gpio.h b/arch/arm/plat-
> omap/include/plat/gpio.h
> > index 4f584de..3f7b028 100644
> > --- a/arch/arm/plat-omap/include/plat/gpio.h
> > +++ b/arch/arm/plat-omap/include/plat/gpio.h
> > @@ -198,6 +198,8 @@ struct omap_gpio_reg_offs {
> > u16 irqctrl;
> > u16 edgectrl1;
> > u16 edgectrl2;
> > + /* Not applicable for OMAP2+ as hwmod layer takes care of sysconfig
> */
> > + u16 sysconfig;
> >
> > bool irqenable_inv;
> > };
> > diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
> > index 769fae7..3acbaa9 100644
> > --- a/drivers/gpio/gpio-omap.c
> > +++ b/drivers/gpio/gpio-omap.c
> > @@ -75,6 +75,7 @@ struct gpio_bank {
> >
> > void (*set_dataout)(struct gpio_bank *bank, int gpio, int enable);
> > int (*get_context_loss_count)(struct device *dev);
> > + void (*do_raw_write)(void __iomem *reg, int set);
> >
> > struct omap_gpio_reg_offs *regs;
> > };
> > @@ -864,67 +865,53 @@ static void __init omap_gpio_show_rev(struct
> gpio_bank *bank)
> > called = true;
> > }
> >
> > +static void __do_raw_writel(void __iomem *reg, int set)
> > +{
> > + if (set)
> > + __raw_writel(0xffffffff, reg);
> > + else
> > + __raw_writel(0x00000000, reg);
> > +
> > +}
> > +
> > +static void __do_raw_writew(void __iomem *reg, int set)
> > +{
> > + if (set)
> > + __raw_writew(0xffff, reg);
> > + else
> > + __raw_writew(0x0000, reg);
> > +}
> > +
> > /* This lock class tells lockdep that GPIO irqs are in a different
> > * category than their parents, so it won't report false recursion.
> > */
> > static struct lock_class_key gpio_lock_class;
> >
> > -/* TODO: Cleanup cpu_is_* checks */
> > static void omap_gpio_mod_init(struct gpio_bank *bank)
> > {
> > - if (cpu_class_is_omap2()) {
> > - if (cpu_is_omap44xx()) {
> > - __raw_writel(0xffffffff, bank->base +
> > - OMAP4_GPIO_IRQSTATUSCLR0);
> > - __raw_writel(0x00000000, bank->base +
> > - OMAP4_GPIO_DEBOUNCENABLE);
> > - /* Initialize interface clk ungated, module enabled */
> > - __raw_writel(0, bank->base + OMAP4_GPIO_CTRL);
> > - } else if (cpu_is_omap34xx()) {
> > - __raw_writel(0x00000000, bank->base +
> > - OMAP24XX_GPIO_IRQENABLE1);
> > - __raw_writel(0xffffffff, bank->base +
> > - OMAP24XX_GPIO_IRQSTATUS1);
> > - __raw_writel(0x00000000, bank->base +
> > - OMAP24XX_GPIO_DEBOUNCE_EN);
> > -
> > - /* Initialize interface clk ungated, module enabled */
> > - __raw_writel(0, bank->base + OMAP24XX_GPIO_CTRL);
> > - }
> > - } else if (cpu_class_is_omap1()) {
> > - if (bank_is_mpuio(bank)) {
> > - __raw_writew(0xffff, bank->base +
> > - OMAP_MPUIO_GPIO_MASKIT / bank->stride);
> > + if (bank_is_mpuio(bank)) {
> > + bank->do_raw_write(bank->base + bank->regs->irqenable, 1);
> > + if (bank->regs->wkup_status)
> > mpuio_init(bank);
> > - }
> > - if (cpu_is_omap15xx() && bank->method == METHOD_GPIO_1510) {
> > - __raw_writew(0xffff, bank->base
> > - + OMAP1510_GPIO_INT_MASK);
> > - __raw_writew(0x0000, bank->base
> > - + OMAP1510_GPIO_INT_STATUS);
> > - }
> > - if (cpu_is_omap16xx() && bank->method == METHOD_GPIO_1610) {
> > - __raw_writew(0x0000, bank->base
> > - + OMAP1610_GPIO_IRQENABLE1);
> > - __raw_writew(0xffff, bank->base
> > - + OMAP1610_GPIO_IRQSTATUS1);
> > - __raw_writew(0x0014, bank->base
> > - + OMAP1610_GPIO_SYSCONFIG);
> > -
> > - /*
> > - * Enable system clock for GPIO module.
> > - * The CAM_CLK_CTRL *is* really the right place.
> > - */
> > - omap_writel(omap_readl(ULPD_CAM_CLK_CTRL) | 0x04,
> > - ULPD_CAM_CLK_CTRL);
> > - }
> > - if (cpu_is_omap7xx() && bank->method == METHOD_GPIO_7XX) {
> > - __raw_writel(0xffffffff, bank->base
> > - + OMAP7XX_GPIO_INT_MASK);
> > - __raw_writel(0x00000000, bank->base
> > - + OMAP7XX_GPIO_INT_STATUS);
> > - }
> > + return;
> > }
> > +
> > + if (bank->regs->ctrl)
> > + /* Initialize interface clk ungated, module enabled */
> > + bank->do_raw_write(bank->base + bank->regs->ctrl, 0);
> > +
> > + if (bank->regs->clr_irqenable && bank->width == 32)
> > + bank->do_raw_write(bank->base + bank->regs->clr_irqenable, 1);
> > + else if (bank->regs->irqenable)
> > + bank->do_raw_write(bank->base + bank->regs->irqenable,
> > + bank->regs->irqenable_inv);
> > +
> > + if (bank->regs->irqstatus)
> > + bank->do_raw_write(bank->base + bank->regs->irqstatus,
> > + bank->regs->irqenable_inv == 0);
> > +
> > + if (bank->regs->debounce_en)
> > + bank->do_raw_write(bank->base + bank->regs->debounce_en, 0);
> > }
> >
> > static __init void
> > @@ -1041,6 +1028,11 @@ static int __devinit omap_gpio_probe(struct
> platform_device *pdev)
> > bank->get_context_loss_count = pdata->get_context_loss_count;
> > bank->regs = pdata->regs;
> >
> > + if (bank->width == 32)
> > + bank->do_raw_write = __do_raw_writel;
> > + else
> > + bank->do_raw_write = __do_raw_writew;
> > +
> > if (bank->regs->set_dataout && bank->regs->clr_dataout)
> > bank->set_dataout = _set_gpio_dataout_reg;
> > else
>
> Actually, I think this driver should just be converted so *all* access
> are 4-byte accesses.
>
> Current code already uses a mix of readw and readl for register access
> OMAP1, so let's just drop all the readw and use readl throughout. All
> registers on OMAP1 are 4-byte aligned.
Yes, you had similar comment on another patch. I will modify.
--
Tarun
>
> Kevin
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
