> -----Original Message-----
> From: R, Durgadoss
> Sent: Monday, August 27, 2012 11:57 AM
> To: Zhang, Rui; lenb@xxxxxxxxxx
> Cc: linux-acpi@xxxxxxxxxxxxxxx; eduardo.valentin@xxxxxx
> Subject: RE: [PATCHv2 04/14] Thermal: Add platform level information to
> thermal.h
> Importance: High
>
> Hi Rui,
>
> [cut.]
>
> > > +int (*get_platform_thermal_params)(struct thermal_zone_device *);
> > > +EXPORT_SYMBOL(get_platform_thermal_params);
> > > +
> > If this function is used by the thermal layer, and provided by the
> > platform thermal driver, why not make it mandatory when registering a
> thermal zone?
> >
> > Say,
> >
> > +/* Structure to define Thermal Zone parameters */ struct
> > +thermal_zone_params {
> > + int trips,
> > + int mask,
> > + struct thermal_zone_device_ops *ops;
> > + enum thermal_throttle_policy throttle_policy;
> > + int num_tbps; /* Number of tbp entries */
> > + struct thermal_bind_params *tbp;
> > };
> > And modify thermal_zone_device_register to Struct thermal_zone_device
> > *thermal_zone_device_register(const char *type, struct
> > thermal_zone_params *params);
> >
> > The first 3 fields are necessary for registering a zone, the
> > thermal_bind_params can either be filled by platform thermal driver,
> > or be NULL and filled by thermal layer later, when user invokes
> > thermal_zone_bind_cooling_devices.
> >
> > In this way, we do not need this API at all.
>
> We can do it either ways. In this case, we need to modify all the
> tzd_register calls. If we are Ok doing that, I am happy to change.
>
> Just that we are adding one more to the already existing 7 args :-)
No, we are trying to reducing the args by moving them into thermal_zone_params.
>
> > > static int get_idr(struct idr *idr, struct mutex *lock, int *id)
> {
> > > int err;
> > > diff --git a/include/linux/thermal.h b/include/linux/thermal.h
> index
> > > 32af124..b644b8e 100644
> > > --- a/include/linux/thermal.h
> > > +++ b/include/linux/thermal.h
> > > @@ -67,6 +67,12 @@ enum thermal_trend {
> > > THERMAL_TREND_DROPPING, /* temperature is dropping */ };
> > >
> > > +enum thermal_throttle_policy {
> > > + THERMAL_USER_SPACE,
> > > + THERMAL_FAIR_SHARE,
> > > + THERMAL_STEP_WISE,
> > > +};
> > > +
> > > /* Events supported by Thermal Netlink */ enum events {
> > > THERMAL_AUX0,
> > > @@ -162,6 +168,37 @@ struct thermal_zone_device {
> > > struct mutex lock; /* protect thermal_instances list */
> > > struct list_head node;
> > > struct delayed_work poll_queue;
> > > + struct thermal_zone_params *tzp;
> > > +};
> > > +
> > > +/* Structure that holds binding parameters for a zone */ struct
> > > +thermal_bind_params {
> > > + struct thermal_cooling_device *cdev;
> > > +
> > > + /*
> > > + * This is a measure of 'how effectively these devices can
> > > + * cool 'this' thermal zone. The shall be determined by platform
> > > + * characterization. This is on a 'percentage' scale.
> > > + * See Documentation/thermal/sysfs-api.txt for more information.
> > > + */
> > > + int weight;
> > > +
> > > + /*
> > > + * This is a bit mask that gives the binding relation between
> > > this
> > > + * thermal zone and cdev, for a particular trip point.
> > > + * See Documentation/thermal/sysfs-api.txt for more information.
> > > + */
> > > + int trip_mask;
> > > + int (*match) (struct thermal_zone_device *tz,
> > > + struct thermal_cooling_device *cdev); };
> >
> > You should start a new line here.
>
> Again, not sure what you meant here. The new line is already there.
>
struct thermal_cooling_device *cdev);
};
I'm not sure what it is in your original patch, but I see something like
"struct thermal_cooling_device *cdev); };"
May be this is because I'm using outlook?
thanks,
rui
> > > +/* Structure to define Thermal Zone parameters */ struct
> > > +thermal_zone_params {
> > > + const char *zone_name;
> >
> >
> > What is this zone_name used for?
>
> This is required when we retrieve platform data from framework layer.
> Now, that we make it as an argument in tzd_register, we don't need this.
>
> Thanks,
> Durga
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
