Re: [PATCH 03/16] OMAP3: PM: Convert smartreflex driver into a platform driver using hwmods and omap-device layer

From: Kevin Hilman <khilman@deeprootsystems.com>
To: "Gopinath, Thara" <thara@ti.com>
Cc: "linux-omap@vger.kernel.org" <linux-omap@vger.kernel.org>,
	"paul@pwsan.com" <paul@pwsan.com>, "Menon, Nishanth" <nm@ti.com>,
	"Cousson, Benoit" <b-cousson@ti.com>,
	"Sripathy, Vishwanath" <vishwanath.bs@ti.com>,
	"Sawant, Anand" <sawant@ti.com>
Subject: Re: [PATCH 03/16] OMAP3: PM: Convert smartreflex driver into a platform driver using hwmods and omap-device layer
Date: Fri, 12 Mar 2010 16:36:56 -0800	[thread overview]
Message-ID: <87aaud2h1z.fsf@deeprootsystems.com> (raw)
In-Reply-To: <5A47E75E594F054BAF48C5E4FC4B92AB0321EC965D@dbde02.ent.ti.com> (Thara Gopinath's message of "Fri\, 12 Mar 2010 15\:18\:54 +0530")

"Gopinath, Thara" <thara@ti.com> writes:

>>>-----Original Message-----
>>>From: Kevin Hilman [mailto:khilman@deeprootsystems.com]
>>>Sent: Tuesday, March 02, 2010 11:58 PM
>>>To: Gopinath, Thara
>>>Cc: linux-omap@vger.kernel.org; paul@pwsan.com; Menon, Nishanth; Cousson, Benoit; Sripathy,
>>>Vishwanath; Sawant, Anand
>>>Subject: Re: [PATCH 03/16] OMAP3: PM: Convert smartreflex driver into a platform driver using hwmods
>>>and omap-device layer
>>>
>>>Thara Gopinath <thara@ti.com> writes:
>>>
>>>> This patch converts the exisitng smartreflex library into a
>>>> platform driver with device , driver registrations using hardware mods.
>>>> As part of this Ntarget values are passed as platform data.
>>>>
>>>> Signed-off-by: Thara Gopinath <thara@ti.com>
>>>
>>>Nice work Thara.  This is good.  Some minor comments below, but some
>>>general comments first.
>>>
>>>As discussed, eventually (longer term) we'll want to move the driver
>>>parts of this to a regulator driver.  To ease that transition, I think
>>>it would be nice to start now by separating the driver portions of
>>>this file from the device portions.
>>>
>>>You've already done a good job of keeping them separate in this file
>>>(most of the device stuff is at the end) but I think having a separate
>>>file is probably even better.  Maybe move driver stuff to sr_driver.c
>>>and leave device stuff in smartreflex.c?  I don't care much about the
>>>names at this point.
>
> Quick question.. If we are going to separate out smartreflex device
> portions from this file , can't we keep them in devices.c instead of
> going for a separate file?

Ultimately, that's Tony's call, but I would prefer to see a separate
specific file for SR device init.

Kevin