Re: [PATCH v6] Thermal: introduce INT3406 thermal driver

[Lee Jones <lee.jones@linaro.org>]

On Mon, 22 Dec 2014, Zhang Rui wrote:

> On Thu, 2014-12-11 at 16:38 +0800, Aaron Lu wrote:
> > INT3406 ACPI device object resembles an ACPI video output device, but its
> > _BCM is said to be deprecated and should not be used. So we will make
> > use of the raw interface to do the actual cooling. Due to this, the
> > backlight core has some modifications. Also, to re-use some of the ACPI
> > video module's code, one function has been exported.
> > 
> > Signed-off-by: Aaron Lu <aaron.lu@intel.com>
> 
> Jingoo and Lee,
> 
> are you okay with the changes in drivers/video/backlight/backlight.c and
> include/linux/backlight.h?

NB: Jingoo still needs to review the crux of the patch.

> > ---
> > v6: Fix an issue that wrongly set error path return value as reported
> > by Olof Johansson.
> > 
> >  drivers/acpi/video.c                              |  77 ++++----
> >  drivers/thermal/Kconfig                           |  26 +--
> >  drivers/thermal/int340x_thermal/Kconfig           |  41 ++++
> >  drivers/thermal/int340x_thermal/Makefile          |   1 +
> >  drivers/thermal/int340x_thermal/int3406_thermal.c | 229 ++++++++++++++++++++++
> >  drivers/video/backlight/backlight.c               |  44 +++--
> >  include/acpi/video.h                              |  20 ++
> >  include/linux/backlight.h                         |   2 +
> >  8 files changed, 366 insertions(+), 74 deletions(-)
> >  create mode 100644 drivers/thermal/int340x_thermal/Kconfig
> >  create mode 100644 drivers/thermal/int340x_thermal/int3406_thermal.c

I gather by the message at the top that you're looking for an Ack so
this can be routed through another subsystem.  Not going to happen.

So you're on v6 already and a) no one has mentioned that introducing a
new driver AND making core framework changes (in a different subsystem
to boot) in one patch is bad and b) this is the first time you've
Cc'ed the maintainers of the aforementioned subsystem?

Moving forward you should split this patch into component parts and
resend -- only this time ensure you Cc all maintainers in the first
instance, rather than as as afterthought.

> > -config INT340X_THERMAL
> > -	tristate "ACPI INT340X thermal drivers"
> > -	depends on X86 && ACPI
> > -	select THERMAL_GOV_USER_SPACE
> > -	select ACPI_THERMAL_REL
> > -	select ACPI_FAN

This patch also relies on backlight as well, no?

[...]

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog