Re: [PATCH V2 0/6] thermal: exynos: Add kernel thermal support forexynos platform

[Amit Kachhap]

On 16 April 2012 07:37, Zhang Rui <rui.zhang@xxxxxxxxx> wrote:
> On ä, 2012-04-11 at 18:17 +0530, Amit Kachhap wrote:
>> Hi Rui,
>>
>> Thanks for looking into the patches.
>>
>> On 10 April 2012 06:28, Zhang Rui <rui.zhang@xxxxxxxxx> wrote:
>> > Hi, Amit,
>> >
>> > On ä, 2012-04-04 at 10:02 +0530, Amit Kachhap wrote:
>> >> Hi Len/Rui,
>> >>
>> >> Any comment or feedback from your side about the status of this patch?
>> >> Is it merge-able or major re-work is needed? I have fixed most of the
>> >> comments in this patchset and currently working on some of the minor
>> >> comments received and will submit them shortly.
>> >>
>> > Sorry for the late response.
>> >
>> > First of all, it makes sense to me to introduce the generic cpufrq
>> > cooling implementation.
>> ok thanks
>> > But I still have some questions.
>> > I think the key reason why THERMAL_TRIP_STATE_INSTANCE is introduced is
>> > that the MONIROR_ZONE and WARN_ZONE on exynos4 can not fit into the
>> > current passive handling in the generic thermal layer well, right?
>> > e.g. there is no tc1/tc2 on exynos4.
>> >
>> > If yes, is it possible that we can enhance the passive cooling to
>> > support the generic processor cooling?
>> > say, introduce another way to throttle the processor in
>> > thermal_zone_device_passive when tc1 and tc2 are not available?
>>
>> I agree that this new trip type code can be moved into passive trip
>> type when tc1 and tc2 are 0. but this is special type of cooling
>> devices behaviour where only instances of the same cooling device is
>> binded to a trip point. The order of mapping is the only
>> differentiating criteria and there are some checks used to implement
>> this like
>> 1) The trip points should be in ascending order.(This is missing in my
>> original patch, so I added below)
>> 2) The set_cur_state has to be called for the exact temp range so
>> get_cur_state(&state) and set_cur_state(state ++/state--) logic is not
>> possible.
>> 3) set_cur_state is called as set_cur_state(cdev_instance).
>
> Do you really need two THERMAL_TRIP_STATE_INSTANCE trip points?
Sorry for late reply as I was off for vacation.
Yes we need 2 trip points of type THERMAL_TRIP_STATE_INSTANCE as we
need different cooling for these 2 zones. Anyways Do you feel that
these whole/partial patch series(cpufreq cooling api's, new trip  type
etc) is ack-able or some modification is needed?
>
> I'm not sure if my understanding is right, but IMO, we can have one
> THERMAL_TRIP_STATE_INSTANCE only, for both MONIROR_ZONE and WARN_ZONE,
> and the trip temperature equals MONIROR_ZONE.
> The cpufreq cooling device will work in the same way, no?
>
> thanks,
> rui
>
>> There is a chance that people might confuse that these checks are
>> applicable for passive trip types also which is not the case here.
>>
>> @@ -1187,6 +1228,21 @@ struct thermal_zone_device
>> *thermal_zone_device_register(char *type,
>> Â Â Â Â Â Â Â Â tz->ops->get_trip_type(tz, count, &trip_type);
>> Â Â Â Â Â Â Â Â if (trip_type == THERMAL_TRIP_PASSIVE)
>> Â Â Â Â Â Â Â Â Â Â Â Â passive = 1;
>> + Â Â Â Â Â Â Â /*
>> + Â Â Â Â Â Â Â Â* For THERMAL_TRIP_STATE_INSTANCE trips, thermal zone should
>> + Â Â Â Â Â Â Â Â* be in ascending order.
>> + Â Â Â Â Â Â Â */
>> + Â Â Â Â Â Â Â if (trip_type == THERMAL_TRIP_STATE_INSTANCE) {
>> + Â Â Â Â Â Â Â Â Â Â Â tz->ops->get_trip_temp(tz, count, &trip_temp);
>> + Â Â Â Â Â Â Â Â Â Â Â if (first_trip_temp == 0)
>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â first_trip_temp = trip_temp;
>> + Â Â Â Â Â Â Â Â Â Â Â else if (first_trip_temp < trip_temp)
>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â first_trip_temp = trip_temp;
>> + Â Â Â Â Â Â Â Â Â Â Â else if (first_trip_temp > trip_temp) {
>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â pr_warn("Zone trip points should be in
>> ascending order\n");
>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â goto unregister;
>> + Â Â Â Â Â Â Â Â Â Â Â }
>> + Â Â Â Â Â Â Â }
>> Â Â Â Â }
>>
>> Â Â Â Â if (!passive)
>>
>> Anyway there is other alternative where this new trip type is not
>> needed and I can just use the existing trip type THERMAL_TRIP_ACTIVE
>> and create 2 separate cooling devices for MONITOR_ZONE and WARN_ZONE.
>> I had thought to make this generic and just to manage with 1 cooling
>> device.
>> What is your view?
>>
>> Thanks,
>> Amit Daniel
>>
>>
>> >
>> > thanks,
>> > rui
>> >
>> >> Regards,
>> >> Amit Daniel
>> >>
>> >> On 19 March 2012 11:47, Amit Daniel Kachhap <amit.kachhap@xxxxxxxxxx> wrote:
>> >> > Changes since V1:
>> >> > *Moved the sensor driver to driver/thermal folder from driver/hwmon folder
>> >> > Âas suggested by Mark Brown and Guenter Roeck
>> >> > *Added notifier support to notify the registered drivers of any cpu cooling
>> >> > Âaction. The driver can modify the default cooling behaviour(eg set different
>> >> > Âmax clip frequency).
>> >> > *The percentage based frequency replaced with absolute clipped frequency.
>> >> > *Some more conditional checks when setting max frequency.
>> >> > *Renamed the new trip type THERMAL_TRIP_STATE_ACTIVE to
>> >> > ÂTHERMAL_TRIP_STATE_INSTANCE
>> >> > *Many review comments from R, Durgadoss <durgadoss.r@xxxxxxxxx> and
>> >> > Âeduardo.valentin@xxxxxx implemented.
>> >> > *Removed cooling stats through debugfs patch
>> >> > *The V1 based can be found here,
>> >> > Âhttps://lkml.org/lkml/2012/2/22/123
>> >> > Âhttp://lkml.org/lkml/2012/3/3/32
>> >> >
>> >> > Changes since RFC:
>> >> > *Changed the cpu cooling registration/unregistration API's to instance based
>> >> > *Changed the STATE_ACTIVE trip type to pass correct instance id
>> >> > *Adding support to restore back the policy->max_freq after doing frequency
>> >> > Âclipping.
>> >> > *Moved the trip cooling stats from sysfs node to debugfs node as suggested
>> >> > Âby Greg KH greg@xxxxxxxxx
>> >> > *Incorporated several review comments from eduardo.valentin@xxxxxx
>> >> > *Moved the Temperature sensor driver from driver/hwmon/ to driver/mfd
>> >> > Âas discussed with Guenter Roeck <guenter.roeck@xxxxxxxxxxxx> and
>> >> > ÂDonggeun Kim <dg77.kim@xxxxxxxxxxx> (https://lkml.org/lkml/2012/1/5/7)
>> >> > *Some changes according to the changes in common cpu cooling APIs
>> >> > *The RFC based patches can be found here,
>> >> > Âhttps://lkml.org/lkml/2011/12/13/186
>> >> > Âhttps://lkml.org/lkml/2011/12/21/169
>> >> >
>> >> >
>> >> > Brief Description:
>> >> >
>> >> > 1) The generic cooling devices code is placed inside driver/thermal/* as
>> >> > placing inside acpi folder will need un-necessary enabling of acpi code. This
>> >> > codes is architecture independent.
>> >> >
>> >> > 2) This patchset adds a new trip type THERMAL_TRIP_STATE_INSTANCE which passes
>> >> > cooling device instance number and may be helpful for cpufreq cooling devices
>> >> > to take the correct cooling action. This trip type avoids the temperature
>> >> > comparision check again inside the cooling handler.
>> >> >
>> >> > 3) This patchset adds generic cpu cooling low level implementation through
>> >> > frequency clipping and cpu hotplug. In future, other cpu related cooling
>> >> > devices may be added here. An ACPI version of this already exists
>> >> > (drivers/acpi/processor_thermal.c). But this will be useful for platforms
>> >> > like ARM using the generic thermal interface along with the generic cpu
>> >> > cooling devices. The cooling device registration API's return cooling device
>> >> > pointers which can be easily binded with the thermal zone trip points.
>> >> > The important APIs exposed are,
>> >> > Â a)struct thermal_cooling_device *cpufreq_cooling_register(
>> >> > Â Â Â Âstruct freq_clip_table *tab_ptr, unsigned int tab_size,
>> >> > Â Â Â Âconst struct cpumask *mask_val)
>> >> > Â b)void cpufreq_cooling_unregister(struct thermal_cooling_device *cdev)
>> >> >
>> >> > 4) Samsung exynos platform thermal implementation is done using the generic
>> >> > cpu cooling APIs and the new trip type. The temperature sensor driver present
>> >> > in the hwmon folder(registered as hwmon driver) is moved to thermal folder
>> >> > and registered as a thermal driver.
>> >> >
>> >> > All this patchset is based on Kernel version 3.3-rc7
>> >> >
>> >> > A simple data/control flow diagrams is shown below,
>> >> >
>> >> > Core Linux thermal <-----> ÂExynos thermal interface <----- Temperature Sensor
>> >> > Â Â Â Â Â| Â Â Â Â Â Â Â Â Â Â Â Â Â Â |
>> >> > Â Â Â Â \|/ Â Â Â Â Â Â Â Â Â Â Â Â Â Â|
>> >> > ÂCpufreq cooling device <---------------
>> >> >
>> >> >
>> >> > Amit Daniel Kachhap (6):
>> >> > Âthermal: Add a new trip type to use cooling device instance number
>> >> > Âthermal: Add generic cpufreq cooling implementation
>> >> > Âthermal: Add generic cpuhotplug cooling implementation
>> >> > Âhwmon: exynos4: Move thermal sensor driver to driver/thermal
>> >> > Â Âdirectory
>> >> > Âthermal: exynos4: Register the tmu sensor with the kernel thermal
>> >> > Â Âlayer
>> >> > ÂARM: exynos4: Add thermal sensor driver platform device support
>> >> >
>> >> > ÂDocumentation/hwmon/exynos4_tmu      |  81 ---
>> >> > ÂDocumentation/thermal/cpu-cooling-api.txt | Â 76 +++
>> >> > ÂDocumentation/thermal/exynos4_tmu     |  52 ++
>> >> > ÂDocumentation/thermal/sysfs-api.txt    |  Â4 +-
>> >> > Âarch/arm/mach-exynos/Kconfig       Â|  11 +
>> >> > Âarch/arm/mach-exynos/Makefile       |  Â1 +
>> >> > Âarch/arm/mach-exynos/clock.c       Â|  Â4 +
>> >> > Âarch/arm/mach-exynos/dev-tmu.c      Â|  39 ++
>> >> > Âarch/arm/mach-exynos/include/mach/irqs.h Â| Â Â2 +
>> >> > Âarch/arm/mach-exynos/include/mach/map.h  |  Â1 +
>> >> > Âarch/arm/mach-exynos/mach-origen.c    Â|  Â1 +
>> >> > Âarch/arm/plat-samsung/include/plat/devs.h | Â Â1 +
>> >> > Âdrivers/hwmon/Kconfig           |  10 -
>> >> > Âdrivers/hwmon/Makefile          Â|  Â1 -
>> >> > Âdrivers/hwmon/exynos4_tmu.c        | Â514 -------------------
>> >> > Âdrivers/thermal/Kconfig          |  21 +
>> >> > Âdrivers/thermal/Makefile         Â|  Â2 +
>> >> > Âdrivers/thermal/cpu_cooling.c       | Â529 +++++++++++++++++++
>> >> > Âdrivers/thermal/exynos4_thermal.c     | Â790 +++++++++++++++++++++++++++++
>> >> > Âdrivers/thermal/thermal_sys.c       |  45 ++-
>> >> > Âinclude/linux/cpu_cooling.h        |  78 +++
>> >> > Âinclude/linux/platform_data/exynos4_tmu.h | Â Â7 +
>> >> > Âinclude/linux/thermal.h          |  Â1 +
>> >> > Â23 files changed, 1660 insertions(+), 611 deletions(-)
>> >> > Âdelete mode 100644 Documentation/hwmon/exynos4_tmu
>> >> > Âcreate mode 100644 Documentation/thermal/cpu-cooling-api.txt
>> >> > Âcreate mode 100644 Documentation/thermal/exynos4_tmu
>> >> > Âcreate mode 100644 arch/arm/mach-exynos/dev-tmu.c
>> >> > Âdelete mode 100644 drivers/hwmon/exynos4_tmu.c
>> >> > Âcreate mode 100644 drivers/thermal/cpu_cooling.c
>> >> > Âcreate mode 100644 drivers/thermal/exynos4_thermal.c
>> >> > Âcreate mode 100644 include/linux/cpu_cooling.h
>> >> >
>> >
>> >
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/