Re: [PATCH 11/13 v3] OMAP: GPIO: Introduce support for OMAP2PLUS chip GPIO init

[Kevin Hilman <khilman@deeprootsystems.com>]

"Varadarajan, Charulatha" <charu@ti.com> writes:

[...]

>> >>
>> >>>
>> >>> What does 'method' mean in that context? Maybe the name should be revisited?
>> >>
>> >> Agree. 'method' is used throughout OMAP GPIO code. As mentioned above, this
>> field would be removed
>> >> once the whole GPIO code is cleaned up. This patch series doesn't bother to
>> clean up GPIO code as the
>> >> changes would be huge and intended only for HWMOD FW adaptation. Cleaning up
>> GPIO code would come as
>> >> a separate series and we can address this then.
>> >>
>> > Sorry if my comment is not aligned but I thought we are addressing the
>> > gpio clean up as well.
>> >
>> > If we are re-vamping the code so much, is it not the right time to clean up as
>> well ??
>> 
>> I agree with Santosh, you are already cleaning a bunch of things, and in
>> that case you can easily take advantage of HWMOD to remove a good amount
>> of useless code.
>> 
>> We'd better do that right now, instead of waiting a next phase that
>> might never happen...
>
> Since hwmod migration would change mainly the init part of the code,
> I started working on hwmod migration as part of the first
> series. Once we agree upon the final patch set for GPIO hwmod
> migration, I can work on top of the hwmod migration patch series to
> clean up the GPIO code and send a dependent series. This will help
> sending the changes in smaller chunks.
>
> I would add a TODO section in patch description outlining the
> cleanup to be done in the next patch series.

At a minimum, a TODO describing the rest of the cleanups would be
helpful.

> Tony,
> Can you add your feedback?
>
> Please refer http://www.mail-archive.com/linux-omap@vger.kernel.org/msg26065.html for the old context.
>
>> 
>> And BTW, this 'method' is a IP version dependent information and
>> not a Soc specific one. You can potentially use the HW revision
>> field, if it is available for the GPIO.
>
> I agree that it is not SoC specific. But I still feel that it is
> better not to have 'method' as part of dev_attr, considering that,
> after clean-up, this information will no longer be needed.

Considering just this 'method' issue, and not the entire cleanup, I
don't like this extra 'user' field past to omap2_init_gpio() used for
the method.

Can you investigate whether or not this flag is even needed and if we
can determine the method based on the REVISION register?

Kevin