lkml.org 
[lkml]   [2012]   [Apr]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    SubjectRe: [PATCH V2 0/6] thermal: exynos: Add kernel thermal support for exynos platform
    From
    On 16 April 2012 07:37, Zhang Rui <rui.zhang@intel.com> 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@intel.com> 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@linaro.org> 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@intel.com> and
    >> >> >  eduardo.valentin@ti.com 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@kroah.com
    >> >> > *Incorporated several review comments from eduardo.valentin@ti.com
    >> >> > *Moved the Temperature sensor driver from driver/hwmon/ to driver/mfd
    >> >> >  as discussed with Guenter Roeck <guenter.roeck@ericsson.com> and
    >> >> >  Donggeun Kim <dg77.kim@samsung.com> (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@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

    \
     
     \ /
      Last update: 2012-04-24 15:27    [W:0.085 / U:29.180 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site