lkml.org 
[lkml]   [2014]   [Dec]   [29]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v6] Thermal: introduce INT3406 thermal driver
On 12/23/2014 09:26 AM, Aaron Lu wrote:
> On 12/22/2014 05:53 PM, Lee Jones wrote:
>> 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.
>
> OK, thanks for the suggestion.
>
>>
>>>> -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?
>
> Yes it does, will add it in next revision.

My fault. INT340X_THERMAL does not rely on backlight, INT3406 does.
Since I have marked INT3406 depends on ACPI_VIDEO, which depends on
BACKLIGHT, so we are good here.

Thanks,
Aaron


\
 
 \ /
  Last update: 2014-12-29 07:21    [W:0.053 / U:0.424 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site