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

[Eduardo Valentin <eduardo.valentin@ti.com>]

Hello,

On Thu, Aug 16, 2012 at 03:12:32PM +0800, Zhang Rui wrote:
> On 四, 2012-08-16 at 00:31 -0600, R, Durgadoss wrote:
> > Hi Rui,
> > 
> > [cut.]
> > > > > > +/*
> > > > > > + * 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.

I also second this step. This split makes a lot of sense.

> > 
> > Ok. I will include this change.
> > 
> > > 
> > > > Other things;
> > > >  I was thinking is 'removal of netlink things' from
> > > > thermal_sys.c
> > > 
> > > and where to move it to?
> > 
> > I was thinking of completely removing this, as raw netlink
> > usage is phasing out, and kobj_uevent () is being used
> > increasingly.
> > 
> > But we will keep it as a separate change, and not club with this one.
> > 
> We need to hold this for a while as I'm not sure if someone is using
> this or not.
> IMO, you can introduce a Config option to enable/disable the netlink
> events for now, and marking it as deprecated.


At least from my side, I don't have any legacy application using the netlink :-)

How about you guys, what are the known applications using this link?

In any case, for me, at least we need to have a standard notification system,
for monitoring, debugging and also in case applications need to react on thermal
events.

> 
> thanks,
> rui
> > > >
> > > > 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.
> > 
> > Yes, will wait for Jean's thoughts..

What where the original design decisions to have these two linked together?

> > 
> > Thanks,
> > Durga
> > 
> > > 
> > > 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
--
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