Re: [PATCHv6 02/16] drivers: thermal: introduce device tree parser

[Eduardo Valentin]

On 08-10-2013 04:45, Hongbo Zhang wrote:
> On 09/25/2013 10:15 PM, Eduardo Valentin wrote:
>> On 25-09-2013 03:13, Hongbo Zhang wrote:
>>> On 09/19/2013 05:35 AM, Eduardo Valentin wrote:
>>>>    [...]
>>>> +
>>>> +/***   sensor API   ***/
>>>> +
>>> You are introducing new concept here, the original framework and drivers
>>> cannot use this, right? any further plan to update original framework
>>> for this new feature?
>>>
>> Well, not new as such. Just a specific way to register sensors to
>> thermal framework. What is really new is the fact that we really need to
>> have sensors decoupled from thermal zone devices, and today we have
>> these concepts pretty merged together.
>> To answer your question, for now I am more concerned with the bindings
>> definition. Once that is at least agreed, then we can follow up with the
>> migration of existing drivers. For now, there are two examples in this
>> series, first one is using one existing thermal driver, which is the TI
>> SoC thermal driver, and the second one is the hwmon drivers, which are
>> existing sensor drivers, but are not thermal drivers.
>>
>> The plan forward, once this series is accepted is to migrate existing
>> drivers, yes, so that they can use device tree uniformly. Of course,
>> this needs help from driver authors.
>>
>> My proposal will be to follow up this series with a two fold migration.
>> First step to change the existing thermal drivers to have both, the
>> current support and the device tree support. And second step, for those
>> who wish to, we could remove the old code containing thermal data and
>> have only dt support. Of course, this requires drivers authors input.
>>
>>
>>
>>>> +static struct thermal_zone_device *
>>>> +thermal_zone_of_add_sensor(struct device_node *zone,
>>>> +               struct device_node *sensor, void *data,
>>>> +               int (*get_temp)(void *, long *),
>>>> +               int (*get_trend)(void *, long *))
>>>> +{
>>>> +    struct thermal_zone_device *tzd;
>>>> +    struct __thermal_zone *tz;
>>>> +
>>>> +    tzd = thermal_zone_get_zone_by_name(zone->name);
>>>> +    if (IS_ERR(tzd))
>>>> +        return ERR_PTR(-EPROBE_DEFER);
>>>> +
>>>> [...]
>>>> +
>>>> +/*
>>>> + * Here are the thermal trip types. This must
>>>> + * match with enum thermal_trip_type at
>>>> + * include/linux/thermal.h
>>>> + */
>>>> +#define THERMAL_TRIP_ACTIVE        0
>>>> +#define THERMAL_TRIP_PASSIVE        1
>>>> +#define THERMAL_TRIP_HOT        2
>>>> +#define THERMAL_TRIP_CRITICAL        3
>>>> +
>>> These macros seem duplicated with enum thermal_trip_type in thermal.h,
>>> do you have further plan to merge them?
>>> Or by using string "active",  "passive" etc in the dts, then you can
>>> reuse the original enum definition.
>> I am changing this so that in DT we have string constants, and we keep a
>> map from string to enum, just like we have for phy-mode, as suggested by
>> Mark.
>>
>> You can have a taste of it here:
>> https://git.kernel.org/cgit/linux/kernel/git/evalenti/linux.git/commit/?h=thermal_work/thermal_core/dt_parser_rfc_v4&id=73f16c27fc763495188fba7d6e17b9c986efc6ac
>>
>>
>> I will be reposting this version once we are done with this thread
>> discussion and I am finished with my current test.
>>
>> If you have the time, I would appreciate if you could try the series on
>> your board, as I am don't have access to your hardware. It would be
>> really nice to see how this work is behaving in other environments then
>> the one I have.
> 
> We have thermal management plan in next year, currently I don't have
> proper board to test this.
> I would like to do it when I have time and board, but I will track these
> thermal treads.

OK Zhang, no worries then.

>>
>> Thanks for your interest in this work.
>>
>>>> +/* On cooling devices upper and lower limits */
>>>> +#define THERMAL_NO_LIMIT        (-1UL)
>>>> +
>>>> +#endif
>>>> [...]
>>>
>>>
>>>
>>
> 
> 
> 
> 
> 


-- 
You have got to be excited about what you are doing. (L. Lamport)

Eduardo Valentin

Attachment: signature.asc
Description: OpenPGP digital signature