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 :-)
> > 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.
> > +/* 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
