On 01/24/13 00:36, Jeroen Hofstee wrote:
> Hi Nikita,
>
> On 01/21/2013 09:25 AM, Nikita Kiryanov wrote:
>> Hi Jeroen,
>>
>> On 01/20/2013 11:08 PM, Jeroen Hofstee wrote:
>>> On 12/23/2012 08:03 AM, Nikita Kiryanov wrote:
>> [...]
>>>> + * Returns -1 on failure, 0 on success.
>>>> + */
>>>> +static int parse_customlcd(char *custom_lcd_params)
>>>> +{
>>>> + char params_cpy[160];
>>>> + char *setting;
>>>> +
>>>> + strncpy(params_cpy, custom_lcd_params, 160);
>>> I fail to understand why you want to copy this.
>>
>> strtok modifies the string it operates on. The documentation for
>> getenv states that you must not modify the string it returns.
>>
> that seems to make sense...
>>>> + setting = strtok(params_cpy, ",");
>>>> + while (setting) {
>>>> + if (parse_setting(setting) < 0)
>>>> + return -1;
>>>> +
>>>> + setting = strtok(NULL, ",");
>>>> + }
>>>> +
>>>> + /* Currently we don't support changing this via custom lcd params */
>>>> + panel_cfg.data_lines = LCD_INTERFACE_24_BIT;
>>>> +
>>> again, if you only support 24 panels, why not drive them as such?
>>
>> Can you please elaborate on this comment? I'm not entirely sure what
>> inconsistencies you are referring to.
> You're driving only 24 bit panels, yet you want the software to drive 16 bit
> panels.
> Why not keep the software frame buffer at 32 bits?
>>
>>>> + return 0;
>>>> +}
>>>> +
>>> Is above really board specific or should it be in omap_videomodes.c or
>>> whatever?
>>
>> Well, most of it is parsing for a custom feature, so I would say this is
>> board specific.
> I can't understand how custom settings can be board specific, unless you
> mess with timer of course (but even then....).
I fail to understand which timer are you talking about...
Probably, you mean the DPLLs and the clocks?
--
Regards,
Igor.
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
