Re: [PATCH v3] thermal: Add QPNP PMIC temperature alarm driver

[Eduardo Valentin]

On Thu, Oct 30, 2014 at 03:55:45PM +0200, Ivan T. Ivanov wrote:
> 
> On Wed, 2014-10-29 at 17:32 +0200, Ivan T. Ivanov wrote:
> > 
> +
> > > > +       chip->tz_dev = thermal_zone_device_register(node->name, TRIP_NUM, 0,
> > > > +       chip, &qpnp_tz_ops, NULL,
> > > 
> > > Have you considered using of-thermal instead of doing your own specific thermal
> > > zone registration? Having a glance look in this driver, most of the
> > > operation are covered by of-thermal. Ahy concerns using of-thermal in
> > > your case?
> > > 
> > 
> > I just followed implementation found in "armada_thermal", "db8500-thermal",
> > "dove_thermal", "imx_thermal", "kirkwood_thermal"...
> > 
> > Will look at of-thermal.
> > 
> 
> Hm, the 15 drivers, which register its own thermal zone, against 4, which 

Yes, and the reasoning is pretty simple: of-thermal is not imposed
standard, and not all platform have decided to move to using it.
However, DT is being used more and more frequently. So, given you
already have DT support, considering using of-thermal is a good thing to do. 

Besides, you are with hands on your driver, might make sense to add its
support while you are there. 

> use of-thermal registration and one of them is OMAP, which have fallback
> to its own zone registration :-). Anyway. I am afraid that if I use just

The fallback is because non-dt booting has not been dropped from OMAP,
AFAIK.

> thermal_zone_of_sensor_register(), driver will lost ability to switch off 
> hardware controlled shutdown sequence, which make it useless IMHO.

Any special sequence or callback you want to consider to improve
of-thermal support?


For instance, getting a callback for mode change events would enable you
to get a better support for hardware controlled shutdown sequence?


> 
> I don't see how driver can benefits from nice things provided by of-thermal.
> There is no colling device associated with PMIC chip, pooling delays are of
> no use, device uses interrupt, trip pints are predefined in hardware...

I see. However, using it may make this driver even simpler, as you would
simply describe it in DT and the notification you want would be taken
care by the core part of the framework.

> 
> Please advice.
> 
> Regards,
> Ivan

Attachment: signature.asc
Description: Digital signature