Re: [PATCH 5/5] Thermal: Add ST-Ericsson db8500 thermal dirver.

[Hongbo Zhang]

On 18 October 2012 16:07, Viresh Kumar <viresh.kumar@xxxxxxxxxx> wrote:
> On 18 October 2012 13:05, Hongbo Zhang <hongbo.zhang@xxxxxxxxxx> wrote:
>> On 17 October 2012 23:23, Viresh Kumar <viresh.kumar@xxxxxxxxxx> wrote:
>>> On 16 October 2012 17:14, hongbo.zhang <hongbo.zhang@xxxxxxxxxx> wrote:
>
>>>> +static struct db8500_trip_point db8500_trips_table[] = {
>>>> +       [0] = {
>>>> +               .temp = 70000,
>>>> +               .type = THERMAL_TRIP_ACTIVE,
>>>> +               .cooling_dev_name = {
>>>> +                       [0] = "thermal-cpufreq-0",
>>>
>>> If i am not wrong length of cooling_dev_name can't be greater than 8
>> You are wrong this time, it is 20
>> #define THERMAL_NAME_LENGTH 20
>
> Ahh.. Its the array size fixed to 8.. not length of each element within.
>
>>>>  static void __init mop500_init_machine(void)
>>>> @@ -765,6 +834,10 @@ struct of_dev_auxdata u8500_auxdata_lookup[] __initdata = {
>>>>                 "ux500-msp-i2s.2", &msp2_platform_data),
>>>>         OF_DEV_AUXDATA("stericsson,ux500-msp-i2s", 0x80125000,
>>>>                 "ux500-msp-i2s.3", &msp3_platform_data),
>>>> +       OF_DEV_AUXDATA("stericsson,db8500-thermal", 0x801573c0,
>>>> +                       "db8500-thermal", &db8500_thsens_data),
>>>> +       OF_DEV_AUXDATA("stericsson,db8500-cpufreq-cooling", 0,
>>>> +                       "db8500-cpufreq-cooling", NULL),
>>>>         {},
>>>>  };
>
>>> Whatever the case, at-least we should pass data via DT for
>>> u8500_auxdata_lookup[].
>>> As you are adding the driver for the first time here, it must be able to parse
>>> data via DT.
>> Yes, for "db8500-thermal", data &db8500_thsens_data is parsed via DT
>> but there is really no data for "db8500-cpufreq-cooling".
>
> You didn't get my point. You are not parsing pdata via DT here, but setting
> pdata of device node created due to DT.
>
> What i am saying is, you must put in code in thermal driver, which will actually
> parse data from DT and create a pdata structure at run time. And there you
> would be required to add another file in Documentation with bindings for this
> driver.
>
Get the idea.
I will update the DT related codes and sent it out again.
Thanks.

>>>> +static int db8500_cpufreq_cooling_suspend(struct platform_device *pdev,
>>>> +               pm_message_t state)
>>>> +{
>>>> +       return -ENOSYS;
>>>> +}
>>>> +
>>>> +static int db8500_cpufreq_cooling_resume(struct platform_device *pdev)
>>>> +{
>>>> +       return -ENOSYS;
>>>> +}
>>>
>>> Do you need these? Wouldn't it be same if you don't define them?
>> There were not such functions before, I added them after reading
>> Documentation/SubmittingDrivers.
>> Is the document too old? should I follow it?
>
> Probably the document is correct and i am not :)
>
> --
> viresh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/