2015-04-07 02:43:03

by Eduardo Valentin

[permalink] [raw]
Subject: Re: [PATCH 13/13] thermal: of: implement .set_trips for device tree thermal zones

On Thu, Mar 26, 2015 at 04:54:00PM +0100, Sascha Hauer wrote:
> Signed-off-by: Sascha Hauer <[email protected]>
> ---
> drivers/thermal/of-thermal.c | 12 ++++++++++++
> include/linux/thermal.h | 1 +
> 2 files changed, 13 insertions(+)

Can you please include at least one user of this call back in your patch
series?

>
> diff --git a/drivers/thermal/of-thermal.c b/drivers/thermal/of-thermal.c
> index 9b63193..a3de5de 100644
> --- a/drivers/thermal/of-thermal.c
> +++ b/drivers/thermal/of-thermal.c
> @@ -97,6 +97,17 @@ static int of_thermal_get_temp(struct thermal_zone_device *tz,
> return data->ops->get_temp(data->sensor_data, temp);
> }
>
> +static int of_thermal_set_trips(struct thermal_zone_device *tz,
> + unsigned long low, unsigned long high)
> +{
> + struct __thermal_zone *data = tz->devdata;
> +
> + if (!data->ops || !data->ops->set_trips)
> + return -ENOSYS;
> +
> + return data->ops->set_trips(data->sensor_data, low, high);
> +}
> +
> /**
> * of_thermal_get_ntrips - function to export number of available trip
> * points.
> @@ -367,6 +378,7 @@ static int of_thermal_get_crit_temp(struct thermal_zone_device *tz,
>
> static const struct thermal_zone_device_ops of_thermal_ops = {
> .get_temp = of_thermal_get_temp,
> + .set_trips = of_thermal_set_trips,
> .get_trend = of_thermal_get_trend,
> .set_emul_temp = of_thermal_set_emul_temp,
>
> diff --git a/include/linux/thermal.h b/include/linux/thermal.h
> index b870702..84a5b5d 100644
> --- a/include/linux/thermal.h
> +++ b/include/linux/thermal.h
> @@ -276,6 +276,7 @@ struct thermal_genl_event {
> */
> struct thermal_zone_of_device_ops {
> int (*get_temp)(void *, unsigned long *);
> + int (*set_trips)(void *, unsigned long, unsigned long);

Could you please keep the kernel doc entry up to date? I know we donot
have entries for all structs, but I am working in improving this.

> int (*get_trend)(void *, int trend, enum thermal_trend *);
> int (*set_emul_temp)(void *, unsigned long);
> };
> --
> 2.1.4
>


Attachments:
(No filename) (2.00 kB)
signature.asc (490.00 B)
Digital signature
Download all attachments

2015-04-13 06:30:30

by Sascha Hauer

[permalink] [raw]
Subject: Re: [PATCH 13/13] thermal: of: implement .set_trips for device tree thermal zones

Hi Eduardo,

On Mon, Apr 06, 2015 at 07:43:08PM -0700, Eduardo Valentin wrote:
> On Thu, Mar 26, 2015 at 04:54:00PM +0100, Sascha Hauer wrote:
> > Signed-off-by: Sascha Hauer <[email protected]>
> > ---
> > drivers/thermal/of-thermal.c | 12 ++++++++++++
> > include/linux/thermal.h | 1 +
> > 2 files changed, 13 insertions(+)
>
> Can you please include at least one user of this call back in your patch
> series?

Thanks for the comments on this series. I'll address them in the next
round. The user for this callback is not yet ready to be posted. I'll
try and move this patch to the end of the series so that the series can
be applied without this one.

Thanks
Sascha

--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |

2015-04-15 18:00:07

by Brian Norris

[permalink] [raw]
Subject: Re: [PATCH 13/13] thermal: of: implement .set_trips for device tree thermal zones

On Mon, Apr 13, 2015 at 08:30:18AM +0200, Sascha Hauer wrote:
> On Mon, Apr 06, 2015 at 07:43:08PM -0700, Eduardo Valentin wrote:
> > On Thu, Mar 26, 2015 at 04:54:00PM +0100, Sascha Hauer wrote:
> > > Signed-off-by: Sascha Hauer <[email protected]>
> > > ---
> > > drivers/thermal/of-thermal.c | 12 ++++++++++++
> > > include/linux/thermal.h | 1 +
> > > 2 files changed, 13 insertions(+)
> >
> > Can you please include at least one user of this call back in your patch
> > series?
>
> Thanks for the comments on this series. I'll address them in the next
> round. The user for this callback is not yet ready to be posted. I'll
> try and move this patch to the end of the series so that the series can
> be applied without this one.

I'm actually interested in this patch, as I am developing a thermal
sensor driver that has hardware-assisted temperature-trip interrupts.
I'm testing out your patches now, and will probably end up using them on
my local tree. With help of this (and a few other changes I need to
make), my driver should be ready to post soon.

So, I'll try to post my driver soon as an example, if you'd like. I'd
also appreciate a CC on any future revisions, if you don't mind. I'm
subscribed to the list, but it's not always easy to pick out the signal
from the noise.

Thanks,
Brian

P.S. Time for a mild complaint: I think it seems to be a bit of a plague
in the thermal subsystem; there are API's, and even user-space ABI's
(!!) that have no in-kernel users! e.g., generate_netlink_event()
http://article.gmane.org/gmane.linux.power-management.general/58685

2015-04-17 05:23:01

by Sascha Hauer

[permalink] [raw]
Subject: Re: [PATCH 13/13] thermal: of: implement .set_trips for device tree thermal zones

Hi Brian,

On Wed, Apr 15, 2015 at 10:59:36AM -0700, Brian Norris wrote:
> On Mon, Apr 13, 2015 at 08:30:18AM +0200, Sascha Hauer wrote:
> > On Mon, Apr 06, 2015 at 07:43:08PM -0700, Eduardo Valentin wrote:
> > > On Thu, Mar 26, 2015 at 04:54:00PM +0100, Sascha Hauer wrote:
> > > > Signed-off-by: Sascha Hauer <[email protected]>
> > > > ---
> > > > drivers/thermal/of-thermal.c | 12 ++++++++++++
> > > > include/linux/thermal.h | 1 +
> > > > 2 files changed, 13 insertions(+)
> > >
> > > Can you please include at least one user of this call back in your patch
> > > series?
> >
> > Thanks for the comments on this series. I'll address them in the next
> > round. The user for this callback is not yet ready to be posted. I'll
> > try and move this patch to the end of the series so that the series can
> > be applied without this one.
>
> I'm actually interested in this patch, as I am developing a thermal
> sensor driver that has hardware-assisted temperature-trip interrupts.
> I'm testing out your patches now, and will probably end up using them on
> my local tree. With help of this (and a few other changes I need to
> make), my driver should be ready to post soon.
>
> So, I'll try to post my driver soon as an example, if you'd like. I'd
> also appreciate a CC on any future revisions, if you don't mind. I'm
> subscribed to the list, but it's not always easy to pick out the signal
> from the noise.

Sure, I'll add you to Cc next round. I'm halfway through updating this
series according to the comments I received, but got distracted. I hope
to send an update next week.

Sascha

--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |