Hi, On 07 October 2016 06:29, Keerthy [mailto:a0393675@xxxxxx] wrote: > On Thursday 06 October 2016 02:13 PM, Steve Twiss wrote: > > From: Steve Twiss <stwiss.opensource@xxxxxxxxxxx> [...] > > + > > +static int da9062_thermal_get_trip_type(struct thermal_zone_device *z, > > + int trip, > > + enum thermal_trip_type *type) > > +{ > > + struct da9062_thermal *thermal = z->devdata; > > + > > + switch (trip) { > > + case 0: > > + *type = THERMAL_TRIP_HOT; > > 125C is pretty hot. Just a suggestion, do look at THERMAL_TRIP_CRITICAL. > Quite warm. I got this from the Documentation/devicetree/bindings/thermal/thermal.txt "hot": A trip point to notify emergency "critical": Hardware not reliable. The junction temperature supervision characteristics for DA9061 supports three levels and 125degC is the lowest "warning" level. This warning level is intended for non-invasive temperature control where the necessary mitigation is under software control of the system. The other two levels are hotter than 125degC and it is not possible to intervene using software. The hardware will automatically power off. [...] > > +static int da9062_thermal_notify(struct thermal_zone_device *z, > > + int trip, > > + enum thermal_trip_type type) > > +{ > > + struct da9062_thermal *thermal = z->devdata; > > + > > + switch (type) { > > + case THERMAL_TRIP_HOT: > > + dev_warn(thermal->dev, "Reached HOT (125degC) > temperature\n"); > > Any cooling action? What is done once you reach this high temperature? There is nothing to do in the device driver. This is just a stub for board specific additions to mitigate the temperature. There is a similar function in the RCAR thermal driver which contains a FIXME comment. But, apart from helping with my testing, it doesn't add anything in the driver. It can be removed. [...] > > +static const struct da9062_thermal_config da9062_config = { > > + .name = "da9062-thermal", > > +}; > > + > > +static const struct da9062_thermal_config da9061_config = { > > + .name = "da9061-thermal", > > +}; > > + > > +static const struct of_device_id da9062_compatible_reg_id_table[] = { > > + { .compatible = "dlg,da9062-thermal", .data = &da9062_config }, > > + { .compatible = "dlg,da9061-thermal", .data = &da9061_config }, > > Two separate compatible values. Do you have anything different apart > from the name? Why use 2 compatibles when there is absolutely no > difference? Yes. This was a comment for the watchdog device driver as well. My concern was having multiple devices (61 and 62) in the same system -- and allowing the driver to report the hardware difference. There is discussion going on about this in other threads. Not certain of the final outcome yet, apart from my existing proposal should be changed. -- From Guenter Roeck: -- http://www.spinics.net/lists/linux-watchdog/msg10040.html [...] > > + INIT_DELAYED_WORK(&thermal->work, da9062_thermal_poll_on); > > + mutex_init(&thermal->lock); > > thermal_zone_device_register itself does > INIT_DELAYED_WORK(&(tz->poll_queue), thermal_zone_device_check); > > refer: drivers/thermal/thermal_core.c > > It does a periodic monitoring of the temperature as well. Do you really > want to have an additional work for monitoring temperature in your > driver also? I will take a look at this for V2. Regards, Steve -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
