> -----Original Message----- > From: Kevin Hilman [mailto:khilman@xxxxxxxxxxxxxxxxxxx] > Sent: Thursday, June 17, 2010 10:05 PM > To: Varadarajan, Charulatha > Cc: Cousson, Benoit; tony@xxxxxxxxxxx; david-b@xxxxxxxxxxx; > broonie@xxxxxxxxxxxxxxxxxxxxxxxxxxx; > akpm@xxxxxxxxxxxxxxxxxxxx; linux-omap@xxxxxxxxxxxxxxx; > paul@xxxxxxxxx; Nayak, Rajendra; Basak, Partha; Shilimkar, Santosh > Subject: Re: [PATCH 11/13 v3] OMAP: GPIO: Introduce support > for OMAP2PLUS chip GPIO init > > "Varadarajan, Charulatha" <charu@xxxxxx> 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. Okay. Will take care in next version of patch series. > > > Tony, > > Can you add your feedback? > > > > Please refer > http://www.mail-archive.com/linux-omap@xxxxxxxxxxxxxxx/msg2606 5.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. Okay, I will avoid passing 'method' as 'user' field in my next version of this patch series. Instead will use 'rev' field of omap_hwmod_class structure and will populate 'method' based on 'rev' value. In hwmod database assign, 'rev' value for OMAP2 as '0' 'rev' value for OMAP3 as '1' 'rev' value for OMAP4 as '2' static int omap2_init_gpio(struct omap_hwmod *oh, void *user) { ... ... switch (oh->class->rev) { case 0: case 1: pdata->method = METHOD_GPIO_24XX; break; case 2: pdata->method = METHOD_GPIO_44XX; break; } ... ... } > > Can you investigate whether or not this flag is even needed and if we > can determine the method based on the REVISION register? AFAIK 'method' is not based on REVISION register. If you see 'method' usage for omap15xx chip, it has METHOD_MPUIO and METHOD_GPIO_1510. Similar for omap16xx and omap 7xx. Even after cleanup we might need a way to find out if the gpio bank falls under MPUIO method. I would do a name change for 'method' as suggested by Benoit during cleanup and it would be used only for identifying MPUIO type after cleanup. > > 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
