Charulatha V <charu@xxxxxx> writes:
> This patch implements GPIO as a early platform device. Also it
> implements OMAP2PLUS specific GPIO as HWMOD FW adapted driver.
Should include a summary explanation of why you're converting to an
early platform device as well.
> Inorder to convert GPIO as platform device, modifications are
> required in clockxxxx_data.c files so that device names can be
> used to obtain clock instead of getting clocks by name/NULL ptr.
ok
> Currently early platform device register does not do device_pm_init.
> Hence pm_runtime functions are not used to enable the GPIO device
> since gpio is early platform device.
OK for now, since this isn't using runtime PM, but maybe we need a
late_initcall() here to do the device_pm_init() + pm_runtime_enable()
This change log needs more description of the intended init sequence.
Right now it seems that there are multiple init paths. Now that GPIO
is an early_platform_device, we should be able to at least make
omap_gpio_init() static and remove its usage from all the board files.
Also, the driver and device separation and init is totally mixed
together and very confusing. The platform_driver is in
plat-omap/gpio.c and should be doing the driver init:
[early_]platform_driver_register() and _probe(). The platform_device
setup is in mach-omapX/gpio*.c and where the device init should be, in
this case early_platform_add_devices().
> Signed-off-by: Charulatha V <charu@xxxxxx>
> Signed-off-by: Rajendra Nayak <rnayak@xxxxxx>
> ---
> arch/arm/mach-omap1/Makefile | 6 +
> arch/arm/mach-omap1/clock_data.c | 2 +-
> arch/arm/mach-omap2/Makefile | 2 +-
> arch/arm/mach-omap2/clock2420_data.c | 10 +-
> arch/arm/mach-omap2/clock2430_data.c | 14 +-
> arch/arm/mach-omap2/clock3xxx_data.c | 24 +-
> arch/arm/mach-omap2/clock44xx_data.c | 24 +-
> arch/arm/plat-omap/gpio.c | 405 ++++++++++++--------------------
> arch/arm/plat-omap/include/plat/gpio.h | 21 ++
> 9 files changed, 220 insertions(+), 288 deletions(-)
[...]
> @@ -1621,6 +1501,34 @@ static void __init omap_gpio_show_rev(void)
> */
> static struct lock_class_key gpio_lock_class;
>
> +static int init_gpio_info(void)
> +{
> + gpio_bank_bits = 32;
> +
> + if (cpu_is_omap15xx()) {
> + gpio_bank_count = 2;
> + gpio_bank_bits = 16;
> + } else if (cpu_is_omap16xx()) {
> + gpio_bank_count = 5;
> + gpio_bank_bits = 16;
> + } else if (cpu_is_omap7xx())
> + gpio_bank_count = 7;
> + else if (cpu_is_omap242x())
> + gpio_bank_count = 4;
> + else if (cpu_is_omap243x())
> + gpio_bank_count = 5;
> + else if (cpu_is_omap34xx() || cpu_is_omap44xx())
> + gpio_bank_count = OMAP34XX_NR_GPIOS;
Both the bank count and bank bits could be part of platform_data
and set in the SoC specific init. This is the GPIO driver part
and we're trying to make this as SoC independent as possible. Anytime
you need to add a cpu_is* or #ifdef in this code indicates something
that should be part of SoC specific init and passed in.
> + gpio_bank = kzalloc(gpio_bank_count * sizeof(struct gpio_bank),
> + GFP_KERNEL);
> + if (!gpio_bank) {
> + pr_err("Memory allocation failed for gpio_bank\n");
> + return -ENOMEM;
> + }
> + return 0;
> +}
> +
> static void omap_gpio_mod_init(struct gpio_bank *bank, int id)
> {
> if (cpu_class_is_omap2()) {
> @@ -1686,16 +1594,9 @@ static void omap_gpio_mod_init(struct gpio_bank *bank, int id)
>
> static void __init omap_gpio_chip_init(struct gpio_bank *bank)
> {
> - int j, gpio_bank_bits = 16;
> + int j;
> static int gpio;
>
> - if (cpu_is_omap7xx() && bank->method == METHOD_GPIO_7XX)
> - gpio_bank_bits = 32; /* 7xx has 32-bit GPIOs */
> -
> - if ((bank->method == METHOD_GPIO_24XX) ||
> - (bank->method == METHOD_GPIO_44XX))
> - gpio_bank_bits = 32;
> -
> bank->mod_usage = 0;
> /* REVISIT eventually switch from OMAP-specific gpio structs
> * over to the generic ones
> @@ -1737,140 +1638,103 @@ static void __init omap_gpio_chip_init(struct gpio_bank *bank)
> set_irq_data(bank->irq, bank);
> }
>
> -static int __init _omap_gpio_init(void)
> +static inline void get_gpio_dbck(struct platform_device *pdev,
> + struct gpio_bank *bank)
> {
> - int i;
> - int gpio = 0;
> + if (cpu_is_omap34xx() || cpu_is_omap44xx()) {
> + bank->dbck = clk_get(&pdev->dev, "dbck");
> + if (IS_ERR(bank->dbck))
> + pr_err("GPIO: Could not get dbck\n");
> + }
> +}
> +static int __devinit omap_gpio_probe(struct platform_device *pdev)
> +{
> + static int gpio_init_done;
> + struct omap_gpio_platform_data *pdata;
> + int id;
> struct gpio_bank *bank;
> - int bank_size = SZ_8K; /* Module 4KB + L4 4KB except on omap1 */
> - char clk_name[11];
> + struct resource *res;
>
> - initialized = 1;
> + if (!gpio_init_done)
> + init_gpio_info();
>
> -#if defined(CONFIG_ARCH_OMAP1)
> - if (cpu_is_omap15xx()) {
> - gpio_ick = clk_get(NULL, "arm_gpio_ck");
> - if (IS_ERR(gpio_ick))
> - printk("Could not get arm_gpio_ck\n");
> - else
> - clk_enable(gpio_ick);
> + if (!pdev || !pdev->dev.platform_data) {
> + pr_err("GPIO device initialize without"
> + "platform data\n");
> + return -EINVAL;
> }
> -#endif
> -#if defined(CONFIG_ARCH_OMAP2)
> - if (cpu_class_is_omap2()) {
> - gpio_ick = clk_get(NULL, "gpios_ick");
> - if (IS_ERR(gpio_ick))
> - printk("Could not get gpios_ick\n");
> - else
> - clk_enable(gpio_ick);
> - gpio_fck = clk_get(NULL, "gpios_fck");
> - if (IS_ERR(gpio_fck))
> - printk("Could not get gpios_fck\n");
> - else
> - clk_enable(gpio_fck);
>
> - /*
> - * On 2430 & 3430 GPIO 5 uses CORE L4 ICLK
> - */
> -#if defined(CONFIG_ARCH_OMAP2430)
> - if (cpu_is_omap2430()) {
> - gpio5_ick = clk_get(NULL, "gpio5_ick");
> - if (IS_ERR(gpio5_ick))
> - printk("Could not get gpio5_ick\n");
> - else
> - clk_enable(gpio5_ick);
> - gpio5_fck = clk_get(NULL, "gpio5_fck");
> - if (IS_ERR(gpio5_fck))
> - printk("Could not get gpio5_fck\n");
> - else
> - clk_enable(gpio5_fck);
> - }
> -#endif
> - }
> -#endif
> + pdata = pdev->dev.platform_data;
>
> -#if defined(CONFIG_ARCH_OMAP3) || defined(CONFIG_ARCH_OMAP4)
> - if (cpu_is_omap34xx() || cpu_is_omap44xx()) {
> - for (i = 0; i < OMAP34XX_NR_GPIOS; i++) {
> - sprintf(clk_name, "gpio%d_ick", i + 1);
> - gpio_iclks[i] = clk_get(NULL, clk_name);
> - if (IS_ERR(gpio_iclks[i]))
> - printk(KERN_ERR "Could not get %s\n", clk_name);
> - else
> - clk_enable(gpio_iclks[i]);
> - }
> + id = pdev->id;
> + if (id > gpio_bank_count) {
> + pr_err("Invalid GPIO device id (%d)\n", id);
> + return -EINVAL;
> }
> -#endif
>
> + bank = &gpio_bank[id];
>
> -#ifdef CONFIG_ARCH_OMAP15XX
> - if (cpu_is_omap15xx()) {
> - gpio_bank_count = 2;
> - gpio_bank = gpio_bank_1510;
> - bank_size = SZ_2K;
> - }
> -#endif
> -#if defined(CONFIG_ARCH_OMAP16XX)
> - if (cpu_is_omap16xx()) {
> - gpio_bank_count = 5;
> - gpio_bank = gpio_bank_1610;
> - bank_size = SZ_2K;
> - }
> -#endif
> -#if defined(CONFIG_ARCH_OMAP730) || defined(CONFIG_ARCH_OMAP850)
> - if (cpu_is_omap7xx()) {
> - gpio_bank_count = 7;
> - gpio_bank = gpio_bank_7xx;
> - bank_size = SZ_2K;
> - }
> -#endif
> -#ifdef CONFIG_ARCH_OMAP2
> - if (cpu_is_omap242x()) {
> - gpio_bank_count = 4;
> - gpio_bank = gpio_bank_242x;
> - }
> - if (cpu_is_omap243x()) {
> - gpio_bank_count = 5;
> - gpio_bank = gpio_bank_243x;
> - }
> -#endif
> -#ifdef CONFIG_ARCH_OMAP3
> - if (cpu_is_omap34xx()) {
> - gpio_bank_count = OMAP34XX_NR_GPIOS;
> - gpio_bank = gpio_bank_34xx;
> + if (bank->initialized == 1) {
is_early_platform_device() can be used to tell if the _probe() is
being called as part of early platform driver probe or "regular".
> + /*
> + * Currently, for early platform_devices,
> + * clk_get() using dev ptr does not seem to be working
> + * Hence getting dbck during regular device probe
> + */
Need a better explanation here. "does not seem to work" is not
quite good enough.
> + get_gpio_dbck(pdev, bank);
In any case, maybe it's better to defer the clk_get() into
the debounce setup and only use if needed.
> + return 0;
> }
> -#endif
> -#ifdef CONFIG_ARCH_OMAP4
> - if (cpu_is_omap44xx()) {
> - gpio_bank_count = OMAP34XX_NR_GPIOS;
> - gpio_bank = gpio_bank_44xx;
> +
> + res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
> + if (unlikely(!res)) {
> + pr_err("GPIO Bank %i Invalid IRQ resource\n", id);
> + return -ENODEV;
> }
> -#endif
> - for (i = 0; i < gpio_bank_count; i++) {
> + bank->irq = res->start;
> + bank->virtual_irq_start = pdata->virtual_irq_start;
> + bank->base = pdata->base;
Why are you using pdata->base for OMAP2+ base addresses...
> + bank->method = pdata->method;
>
> - bank = &gpio_bank[i];
> - spin_lock_init(&bank->lock);
> + spin_lock_init(&bank->lock);
>
> + if (cpu_class_is_omap2()) {
> + bank->device_enable = pdata->device_enable;
> + bank->device_idle = pdata->device_idle;
> + bank->device_shutdown = pdata->device_shutdown;
> + pdata->device_enable(pdev);
> + } else if (cpu_class_is_omap1()) {
> /* Static mapping, never released */
> - bank->base = ioremap(bank->pbase, bank_size);
> - if (!bank->base) {
> - printk(KERN_ERR "Could not ioremap gpio bank%i\n", i);
> - continue;
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
... and doing it the right way here for OMAP1?
platform_get_resource() should be used for both.
> + if (unlikely(!res)) {
> + pr_err("GPIO Bank %i Invalid mem resource\n", id);
> + return -ENODEV;
> }
>
> - omap_gpio_mod_init(bank, i);
> - omap_gpio_chip_init(bank);
> + bank->base = ioremap(res->start, resource_size(res));
> + if (!bank->base) {
> + pr_err("Could not ioremap gpio bank%i\n", id);
> + return -ENOMEM;
> + }
>
> - if (cpu_is_omap34xx() || cpu_is_omap44xx()) {
> - sprintf(clk_name, "gpio%d_dbck", i + 1);
> - bank->dbck = clk_get(NULL, clk_name);
> - if (IS_ERR(bank->dbck))
> - printk(KERN_ERR "Could not get %s\n", clk_name);
> + if (cpu_is_omap15xx() && (id == 0)) {
> + static struct clk *gpio_clk;
> + gpio_clk = clk_get(&pdev->dev, "arm_gpio_ck");
> + if (IS_ERR(gpio_clk))
> + pr_err("Could not get arm_gpio_ck\n");
> + else
> + clk_enable(gpio_clk);
> }
> }
>
> - omap_gpio_show_rev();
> + omap_gpio_mod_init(bank, id);
> + omap_gpio_chip_init(bank);
>
> + if (!gpio_init_done) {
> + omap_gpio_show_rev();
> + gpio_init_done = 1;
> + }
> +
> + bank->initialized = 1;
I think you can drop this flag, and use is_early_platform_device() if
needed.
> return 0;
> }
>
> @@ -2210,16 +2074,42 @@ void omap_gpio_restore_context(void)
> }
> #endif
>
> -/*
> - * This may get called early from board specific init
> - * for boards that have interrupts routed via FPGA.
> - */
> +static struct platform_driver omap_gpio_driver = {
> + .probe = omap_gpio_probe,
> + .driver = {
> + .name = "omap-gpio",
> + },
> +};
> +
> +int __init omap_gpio_drv_reg(void)
> +{
> + return platform_driver_register(&omap_gpio_driver);
> +}
> +
> +early_platform_init("earlygpio", &omap_gpio_driver);
> +
> int __init omap_gpio_init(void)
> {
> - if (!initialized)
> - return _omap_gpio_init();
> - else
> + int ret = 0;
> +
> + if (initialized)
> return 0;
> +
> +#ifdef CONFIG_ARCH_OMAP1
> + if (cpu_is_omap7xx())
> + ret = omap7xx_gpio_init();
> + if (cpu_is_omap15xx())
> + ret = omap15xx_gpio_init();
> + if (cpu_is_omap16xx())
> + ret = omap16xx_gpio_init();
> +#endif
> +#ifdef CONFIG_ARCH_OMAP2PLUS
> + if (cpu_class_is_omap2())
> + ret = omap2_gpio_init();
> +#endif
No thanks. driver init should not be calling device init. The device
init should have it's own initcall or be called by other early device
init.
> + initialized = 1;
> +
> + return ret;
> }
>
> static int __init omap_gpio_sysinit(void)
> @@ -2227,7 +2117,10 @@ static int __init omap_gpio_sysinit(void)
> int ret = 0;
>
> if (!initialized)
> - ret = _omap_gpio_init();
> + ret = omap_gpio_init();
> +
> + if (!ret)
> + ret = omap_gpio_drv_reg();
>
> mpuio_init();
Kevin
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
