RE: [PATCH 13/13] Thermal: Platform layer changes to provide thermal data

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Rui,

> > > -----Original Message-----
> > > From: linux-acpi-owner@xxxxxxxxxxxxxxx [mailto:linux-acpi-
> > > owner@xxxxxxxxxxxxxxx] On Behalf Of Eduardo Valentin
> > > Sent: Tuesday, August 21, 2012 11:10 AM
> > > To: R, Durgadoss
> > > Cc: lenb@xxxxxxxxxx; Zhang, Rui; rjw@xxxxxxx; linux-acpi@xxxxxxxxxxxxxxx;
> > > linux-pm@xxxxxxxxxxxxxxx; eduardo.valentin@xxxxxx;
> > > amit.kachhap@xxxxxxxxxx; wni@xxxxxxxxxx
> > > Subject: Re: [PATCH 13/13] Thermal: Platform layer changes to provide
> > > thermal data
> > >
> > > Hello,
> > >
> > > On Thu, Aug 09, 2012 at 06:16:05PM +0530, Durgadoss R wrote:
> > > > This patch shows how can we add platform specific thermal data
> > > > required by the thermal framework. This is just an example
> > > > patch, and _not_ for merge.
> > > >
> > > > Signed-off-by: Durgadoss R <durgadoss.r@xxxxxxxxx>
> > > > ---
> > > >  arch/x86/platform/mrst/mrst.c |   42
> > > +++++++++++++++++++++++++++++++++++++++++
> > > >  1 file changed, 42 insertions(+)
> > > >
> > > > diff --git a/arch/x86/platform/mrst/mrst.c
> > > b/arch/x86/platform/mrst/mrst.c
> > > > index fd41a92..0440db5 100644
> > > > --- a/arch/x86/platform/mrst/mrst.c
> > > > +++ b/arch/x86/platform/mrst/mrst.c
> > > > @@ -30,6 +30,7 @@
> > > >  #include <linux/mfd/intel_msic.h>
> > > >  #include <linux/gpio.h>
> > > >  #include <linux/i2c/tc35876x.h>
> > > > +#include <linux/thermal.h>
> > > >
> > > >  #include <asm/setup.h>
> > > >  #include <asm/mpspec_def.h>
> > > > @@ -78,6 +79,30 @@ struct sfi_rtc_table_entry
> > > sfi_mrtc_array[SFI_MRTC_MAX];
> > > >  EXPORT_SYMBOL_GPL(sfi_mrtc_array);
> > > >  int sfi_mrtc_num;
> > > >
> > > > +#define MRST_THERMAL_ZONES	3
> > > > +struct thermal_zone_params tzp[MRST_THERMAL_ZONES] = {
> > > > +	{ .thermal_zone_name = "CPU",
> > > > +	.throttle_policy = THERMAL_FAIR_SHARE,
> > > > +	.num_cdevs = 2,
> > > > +	.cdevs_name = {"CPU", "Battery"},
> > > > +	.trip_mask = {0x0F, 0x08},
> > > > +	.weights = {80, 20}, },
> > > > +
> > > > +	{ .thermal_zone_name = "Battery",
> > > > +	.throttle_policy = THERMAL_FAIR_SHARE,
> > > > +	.num_cdevs = 1,
> > > > +	.cdevs_name = {"Battery"},
> > > > +	.trip_mask = {0x0F},
> > > > +	.weights = {100}, },
> > > > +
> > > > +	{ .thermal_zone_name = "Skin",
> > > > +	.throttle_policy = THERMAL_FAIR_SHARE,
> > > > +	.num_cdevs = 2,
> > > > +	.cdevs_name = {"Display", "Battery"},
> > > > +	.trip_mask = {0x0F, 0x0F},
> > > > +	.weights = {50, 50}, }
> > >
> > > Please consider the comment I sent on your data definition and also the
> > > comment I made on this patch on your RFC series.
> >
> > Yes.. I don't know why/how I missed it.
> > Also, saw the same comment on one of the other patches also.
> >
> > Will surely fix this thing in v2.
> >
> > BTW, any suggestion for the 'name' of that structure ? :-)
> 
> hmmm,
> do we still have thermal_zone_platforms in patch v2?
> I do not think we need this if we only bind devices via .bind()
> callback.

We can bind devices via .bind call back, and that will take some load
off the framework code. But even then, we would need this structure
right ? Say, when we obtain platform data from a thermal driver, it
should know 'what format the platform data is' ..correct ?

I theoretically agree with you that individual platform drivers can
have data in their own format, but that will be a heavy loss on
standardization.

So,
I will remove the extra bind code I added to framework, and
(keep it the old way it was) but still prefer to have the structure
put in thermal.h.

Thanks,
Durga
��.n��������+%������w��{.n�����{�����ܨ}���Ơz�j:+v�����w����ޙ��&�)ߡ�a����z�ޗ���ݢj��w�f



[Index of Archives]     [Linux IBM ACPI]     [Linux Power Management]     [Linux Kernel]     [Linux Laptop]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]

  Powered by Linux