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

[Tony Lindgren <tony@atomide.com>]

* Kevin Hilman <khilman@deeprootsystems.com> [100312 16:38]:
> "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.

Yeah me too considering the amount of code. I also agree that most of
the smartreflex code should be done under drivers/ to allow loading it
as a module.

Regards,

Tony