[U-Boot] [PATCH v2 3/4] common/lcd_console: move single static variables into common (static) structure

Igor Grinberg grinberg at compulab.co.il
Wed Mar 18 16:23:10 CET 2015


On 03/18/15 17:07, Hannes Petermaier wrote:
> On 2015-03-18 13:47, Igor Grinberg wrote:
>> On 03/18/15 14:14, Hannes Petermaier wrote:
>>> On 2015-03-18 13:11, Igor Grinberg wrote:
>>>> [...]
>>>>> +struct console_t {
>>>>> +    short curr_col, curr_row;
>>>>> +    short cols, rows;
>>>>> +    void *lcd_address;
>>>> Shouldn't this be fbbase?
>>>>
>>>>> +};
>>>>> +static struct console_t cons;
>>>> [...]
>>>>
>>> Hi Igor,
>>> my plan was/is to make a cleanup patch after this story is closed.
>>> - remove empty lines in code
>> Yes indeed, but...
>>
>>> - rename some variables
>> This depends. For example the lcd_address, you are introducing
>> a lot of new places where this variable is used.
>> So, IMO, it is better to rename it before you are using it, otherwise
>> the renaming patch will be much bigger and that is called needless churn...
>>
>>> would it be better to do that immediately ?
>> Well, I think that there are certain levels of cleanup...
>> In case of the lcd_address, I think it is better to rename it before use.
>>
>> I think the rule of thumb (meaning this is not cast in stone) is first
>> do a clean up and then add a functionality to already clean code.
> Hi Igor,
> 
> Okay,
> thanks - i will do the renanaming within my implementation of the rotation feature.

Yep. Looks like a good place is this patch as you rename/move it here already...


-- 
Regards,
Igor.


More information about the U-Boot mailing list