Re: [PATCH 0/8 RFC] OMAP: GPIO: Split OMAP1 and OMAP2PLUS

[Kevin Hilman <khilman@deeprootsystems.com>]

Charulatha V <charu@ti.com> writes:

> This patch series is in preparation to adapt GPIO to HWMOD FW.
> It creates OMAP architecture specific gpio files to handle
> SoC specific gpio_init. The common plat-omap/gpio.c handles all
> common GPIO APIs.
>
> OMAP2PLUS GPIO module is implemented as early platform device and
> OMAP1 GPIO is still handled in old way via gpio_init from board files.
>
> Save/restore context, gpio_prepare_for_retention and
> gpio_resume_after_retention APIs are also handled in plat-omap layer.
> These APIs are currently not used in OMAP1, but still they might
> become common for different OMAP architectures in the future.
> Hence they are handled in plat-omap layer. If they need to be moved
> to mach-omap2 layer, additional patches may be sent during
> next version of this patch series.
>
> This patch series is generated on top of linux-omap-2.6 branch: master
> and tested on 3430SDP, 4430SDP & zoom3.

I have many problems with this series, but without going into the
specifics of each patch, I have a big problem with the general
approach being taken here.

You say this is in preparation for adaptation to hwmod, but in fact,
this series is duplicating a lot of the work you would get for free by
starting with hwmod.

You're generating a bunch of code that will be completely thrown away
with a hwmod conversion.  Instead, you should *start* by using hwmod +
omap_device to generate the platform_devices for you.  Doing this, you
will drop patches 2, 3 and 4 entirely (they are mostly identical
anyways, except for the search-and-replace) and also eliminate the
need for patch 5.

What is needed here is a way to introduce the platform_device
conversion without introducing signifcant functional changes at the
same time.  Here's a proposal to do that, and which should also be
considerably easier to review for sanity:

- use hwmod + omap_device to generate (early) platform_devices
- modify plat-omap/gpio.c to use base addr and IRQ from platform_device
- modify plat-omap/gpio.c to get clocks based on 'struct device'

For an example, you can see the pm-wip/hwmods branch in my tree for
how this was done for UART and the pm-wip/mmc branch for how this was
done for MMC.

Kevin