diff mbox

[13/13] thermal: of: implement .set_trips for device tree thermal zones

Message ID 1427385240-6086-14-git-send-email-s.hauer@pengutronix.de (mailing list archive)
State New, archived
Headers show

Commit Message

Sascha Hauer March 26, 2015, 3:54 p.m. UTC
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/thermal/of-thermal.c | 12 ++++++++++++
 include/linux/thermal.h      |  1 +
 2 files changed, 13 insertions(+)

Comments

Eduardo Valentin April 7, 2015, 2:43 a.m. UTC | #1
On Thu, Mar 26, 2015 at 04:54:00PM +0100, Sascha Hauer wrote:
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
>  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
>
Sascha Hauer April 13, 2015, 6:30 a.m. UTC | #2
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 <s.hauer@pengutronix.de>
> > ---
> >  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
Brian Norris April 15, 2015, 5:59 p.m. UTC | #3
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 <s.hauer@pengutronix.de>
> > > ---
> > >  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
Sascha Hauer April 17, 2015, 5:22 a.m. UTC | #4
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 <s.hauer@pengutronix.de>
> > > > ---
> > > >  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
diff mbox

Patch

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);
 	int (*get_trend)(void *, int trend, enum thermal_trend *);
 	int (*set_emul_temp)(void *, unsigned long);
 };