Re: [PATCH V1 01/10] thermal: tegra: move tegra thermal files into tegra directory

[Thierry Reding]

On Wed, Jan 13, 2016 at 03:58:40PM +0800, Wei Ni wrote:
> Move tegra soctherm driver to tegra directory, it's easy to maintain
> and add more new function support for tegra platforms.

Please use the proper spelling "Tegra", except where you refer to
directory or file names.

> This will also help to split soctherm driver into common parts and
> chip specific data related parts.
> 
> Signed-off-by: Wei Ni <wni@xxxxxxxxxx>
> ---
>  drivers/thermal/Kconfig                      | 15 +++++----------
>  drivers/thermal/Makefile                     |  2 +-
>  drivers/thermal/tegra/Kconfig                |  9 +++++++++
>  drivers/thermal/tegra/Makefile               |  6 ++++++
>  drivers/thermal/{ => tegra}/tegra_soctherm.c |  0
>  5 files changed, 21 insertions(+), 11 deletions(-)
>  create mode 100644 drivers/thermal/tegra/Kconfig
>  create mode 100644 drivers/thermal/tegra/Makefile
>  rename drivers/thermal/{ => tegra}/tegra_soctherm.c (100%)
> 
> diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
> index c463c89b90ef..7984cba6b340 100644
> --- a/drivers/thermal/Kconfig
> +++ b/drivers/thermal/Kconfig
> @@ -254,16 +254,6 @@ config ARMADA_THERMAL
>  	  Enable this option if you want to have support for thermal management
>  	  controller present in Armada 370 and Armada XP SoC.
>  
> -config TEGRA_SOCTHERM
> -	tristate "Tegra SOCTHERM thermal management"
> -	depends on ARCH_TEGRA
> -	help
> -	  Enable this option for integrated thermal management support on NVIDIA
> -	  Tegra124 systems-on-chip. The driver supports four thermal zones
> -	  (CPU, GPU, MEM, PLLX). Cooling devices can be bound to the thermal
> -	  zones to manage temperatures. This option is also required for the
> -	  emergency thermal reset (thermtrip) feature to function.
> -
>  config DB8500_CPUFREQ_COOLING
>  	tristate "DB8500 cpufreq cooling"
>  	depends on ARCH_U8500
> @@ -380,6 +370,11 @@ depends on ARCH_STI && OF
>  source "drivers/thermal/st/Kconfig"
>  endmenu
>  
> +menu "Tegra thermal drivers"
> +depends on ARCH_TEGRA
> +source "drivers/thermal/tegra/Kconfig"
> +endmenu
> +

I think it'd be more idiomatic to only source the file here and move the
menu declaration and the dependencies into that file. Perhaps also make
the menu description "NVIDIA Tegra thermal drivers", I think we've been
using that consistently elsewhere.

> diff --git a/drivers/thermal/tegra/Kconfig b/drivers/thermal/tegra/Kconfig
> new file mode 100644
> index 000000000000..a6e6cd4528dc
> --- /dev/null
> +++ b/drivers/thermal/tegra/Kconfig
> @@ -0,0 +1,9 @@
> +config TEGRA_SOCTHERM
> +	tristate "Tegra SOCTHERM thermal management"
> +	depends on ARCH_TEGRA
> +	help
> +	  Enable this option for integrated thermal management support on NVIDIA
> +	  Tegra124 systems-on-chip. The driver supports four thermal zones
> +	  (CPU, GPU, MEM, PLLX). Cooling devices can be bound to the thermal
> +	  zones to manage temperatures. This option is also required for the
> +	  emergency thermal reset (thermtrip) feature to function.
> diff --git a/drivers/thermal/tegra/Makefile b/drivers/thermal/tegra/Makefile
> new file mode 100644
> index 000000000000..8c51076e4b1e
> --- /dev/null
> +++ b/drivers/thermal/tegra/Makefile
> @@ -0,0 +1,6 @@
> +#
> +# Tegra thermal specific Makefile
> +#
> +
> +# Tegra soc thermal drivers
> +obj-$(CONFIG_TEGRA_SOCTHERM)	+= tegra_soctherm.o

I personally don't think these comments are helpful. They're really
redundant given that they're in a Makefile within a subdirectory of
drivers/thermal.

Thierry

Attachment: signature.asc
Description: PGP signature