Re: [PATCH V2 01/20] thermal: exynos: Moving exynos thermal files into samsung directory

[Tomasz Figa <t.figa@samsung.com>]

On Tuesday 07 of May 2013 18:48:46 amit daniel kachhap wrote:
> Hi Tomasz Figa,
> 
> On Tue, May 7, 2013 at 3:51 PM, Tomasz Figa <t.figa@samsung.com> wrote:
> > On Monday 06 of May 2013 23:05:06 Zhang Rui wrote:
> >> On Fri, 2013-04-26 at 16:07 +0530, Amit Daniel Kachhap wrote:
> >> > This movement of files is done for easy maintenance and adding more
> >> > new sensor's support for exynos platform easily . This will also help
> >> > in
> >> > bifurcating exynos common, sensor driver and sensor data related parts.
> >> > 
> >> > Acked-by: Kukjin Kim <kgene.kim@samsung.com>
> >> > Signed-off-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
> >> > ---
> >> > 
> >> >  drivers/thermal/Kconfig                        |   13 +++++--------
> >> >  drivers/thermal/Makefile                       |    2 +-
> >> >  drivers/thermal/samsung/Kconfig                |    9 +++++++++
> >> >  drivers/thermal/samsung/Makefile               |    4 ++++
> >> >  drivers/thermal/{ => samsung}/exynos_thermal.c |    0
> >> >  5 files changed, 19 insertions(+), 9 deletions(-)
> >> >  create mode 100644 drivers/thermal/samsung/Kconfig
> >> >  create mode 100644 drivers/thermal/samsung/Makefile
> >> >  rename drivers/thermal/{ => samsung}/exynos_thermal.c (100%)
> >> > 
> >> > diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
> >> > index 5e3c025..081ddc5 100644
> >> > --- a/drivers/thermal/Kconfig
> >> > +++ b/drivers/thermal/Kconfig
> >> > @@ -114,14 +114,6 @@ config KIRKWOOD_THERMAL
> >> > 
> >> >       Support for the Kirkwood thermal sensor driver into the Linux
> >> >       thermal
> >> >       framework. Only kirkwood 88F6282 and 88F6283 have this sensor.
> >> > 
> >> > -config EXYNOS_THERMAL
> >> > -   tristate "Temperature sensor on Samsung EXYNOS"
> >> > -   depends on (ARCH_EXYNOS4 || ARCH_EXYNOS5)
> >> > -   depends on CPU_THERMAL
> >> > -   help
> >> > -     If you say yes here you get support for TMU (Thermal Management
> >> > -     Unit) on SAMSUNG EXYNOS series of SoC.
> >> > -
> >> > 
> >> >  config DOVE_THERMAL
> >> >  
> >> >     tristate "Temperature sensor on Marvell Dove SoCs"
> >> >     depends on ARCH_DOVE
> >> > 
> >> > @@ -169,4 +161,9 @@ config INTEL_POWERCLAMP
> >> > 
> >> >       enforce idle time which results in more package C-state
> >> >       residency. The
> >> >       user interface is exposed via generic thermal framework.
> >> > 
> >> > +menu "Exynos thermal drivers"
> >> 
> >> would it be more proper to use "Samsung thermal drivers" or "Samsung
> >> Exynos thermal drivers" instead?
> > 
> > +1
> > 
> > Still, I wonder if it really makes sense to create a separate directory
> > and
> > submenu for this.
> 
> Yes separate directory was needed as the current TMU drivers supports
> many exynos SOC's with some differences in the controller itself. This
> has made it unmanageable and unclean and configuration data need to be
> separated from driver. With all these re-structuring, tmu support for
> exynos5440 is added very cleanly. Also many changes in this patchset
> are for making the driver multi-stance which is needed for 5440.

I fully understand the need to restructure this driver. I'm just thinking if 
there is really a reason to have a separate directory with its own Makefile and 
Kconfig just for six source files, which could be placed in drivers/thermal/ 
directly as well. I guess it's just a matter of preference, though.

Best regards,
-- 
Tomasz Figa
Samsung Poland R&D Center
SW Solution Development, Kernel and System Framework