RE: [PATCH 02/13] Thermal: Move thermal_instance to thermal.h

[Zhang Rui <rui.zhang@intel.com>]

On 四, 2012-08-16 at 00:19 -0600, R, Durgadoss wrote:
> Hi Rui,
> 
> 
> > -----Original Message-----
> > From: Zhang, Rui
> > Sent: Thursday, August 16, 2012 11:44 AM
> > To: R, Durgadoss
> > Cc: lenb@kernel.org; rjw@sisk.pl; linux-acpi@vger.kernel.org; linux-
> > pm@vger.kernel.org; eduardo.valentin@ti.com; amit.kachhap@linaro.org;
> > wni@nvidia.com
> > Subject: Re: [PATCH 02/13] Thermal: Move thermal_instance to thermal.h
> > 
> > On 四, 2012-08-09 at 18:15 +0530, Durgadoss R wrote:
> > > This patch moves the thermal_instance structure
> > > to thermal.h so that thermal management files
> > > (other than thermal_sys.c) can also access it.
> > >
> > > Signed-off-by: Durgadoss R <durgadoss.r@intel.com>
> > > ---
> > >  drivers/thermal/thermal_sys.c |   21 ---------------------
> > >  include/linux/thermal.h       |   23 +++++++++++++++++++++++
> > >  2 files changed, 23 insertions(+), 21 deletions(-)
> > >
> > > diff --git a/drivers/thermal/thermal_sys.c b/drivers/thermal/thermal_sys.c
> > > index 5be8728..a0e20f9 100644
> > > --- a/drivers/thermal/thermal_sys.c
> > > +++ b/drivers/thermal/thermal_sys.c
> > > @@ -41,27 +41,6 @@ MODULE_AUTHOR("Zhang Rui");
> > >  MODULE_DESCRIPTION("Generic thermal management sysfs support");
> > >  MODULE_LICENSE("GPL");
> > >
> > > -#define THERMAL_NO_TARGET -1UL
> > > -/*
> > > - * This structure is used to describe the behavior of
> > > - * a certain cooling device on a certain trip point
> > > - * in a certain thermal zone
> > > - */
> > > -struct thermal_instance {
> > > -	int id;
> > > -	char name[THERMAL_NAME_LENGTH];
> > > -	struct thermal_zone_device *tz;
> > > -	struct thermal_cooling_device *cdev;
> > > -	int trip;
> > > -	unsigned long upper;	/* Highest cooling state for this trip point */
> > > -	unsigned long lower;	/* Lowest cooling state for this trip point */
> > > -	unsigned long target;	/* expected cooling state */
> > > -	char attr_name[THERMAL_NAME_LENGTH];
> > > -	struct device_attribute attr;
> > > -	struct list_head tz_node; /* node in tz->thermal_instances */
> > > -	struct list_head cdev_node; /* node in cdev->thermal_instances */
> > > -};
> > > -
> > >  static DEFINE_IDR(thermal_tz_idr);
> > >  static DEFINE_IDR(thermal_cdev_idr);
> > >  static DEFINE_MUTEX(thermal_idr_lock);
> > > diff --git a/include/linux/thermal.h b/include/linux/thermal.h
> > > index 8611e3e..f25df23 100644
> > > --- a/include/linux/thermal.h
> > > +++ b/include/linux/thermal.h
> > > @@ -33,6 +33,9 @@
> > >  #define THERMAL_MAX_TRIPS	12
> > >  #define THERMAL_NAME_LENGTH	20
> > >
> > > +/* Initial state of a cooling device during binding */
> > > +#define THERMAL_NO_TARGET	-1UL
> > > +
> > >  /* No upper/lower limit requirement */
> > >  #define THERMAL_NO_LIMIT	-1UL
> > >
> > > @@ -164,6 +167,26 @@ struct thermal_zone_device {
> > >  	struct delayed_work poll_queue;
> > >  };
> > >
> > > +/*
> > > + * This structure is used to describe the behavior of
> > > + * a certain cooling device on a certain trip point
> > > + * in a certain thermal zone
> > > + */
> > > +struct thermal_instance {
> > > +	int id;
> > > +	char name[THERMAL_NAME_LENGTH];
> > > +	struct thermal_zone_device *tz;
> > > +	struct thermal_cooling_device *cdev;
> > > +	int trip;
> > > +	unsigned long upper;	/* Highest cooling state for this trip point */
> > > +	unsigned long lower;	/* Lowest cooling state for this trip point */
> > > +	unsigned long target;	/* expected cooling state */
> > > +	char attr_name[THERMAL_NAME_LENGTH];
> > > +	struct device_attribute attr;
> > > +	struct list_head tz_node; /* node in tz->thermal_instances */
> > > +	struct list_head cdev_node; /* node in cdev->thermal_instances */
> > > +};
> > > +
> > 
> > as this structure is used internally only, I'm thinking if we can rename
> > drivers/thermal/thermal_sys.c to drivers/thermal/thermal_core.c,
> > and introduce drivers/thermal/thermal_core.h for these internal stuff.
> > what do you think?
> 
> Yes agree with you.
> Also, we can keep the sysfs things in thermal_sys.c 
> and rest of the things in thermal_core.c, and have a thermal_core.h also.
> (This is how the power supply subsystem does it)
> 
> I will include this clean up, as part of v2, if you are Ok with this.
> 
yes, please go ahead.

> Other things;
>  I was thinking is 'removal of netlink things' from
> thermal_sys.c

and where to move it to?
> 
> Removing the hwmon related code (the thermal subsystem has grown
> quite a bit and provides more thermal functionalities than Hwmon)
> So, why do we need CONFIG_HWMON inside thermal subsystem ?
> If all of us agree, I am Ok to remove this also.
> 
we need Jean's opinion on this.
But anyway, we can do this at anytime, if really needed.

thanks,
rui

--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html