[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 13:47:09 CET 2015


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.

-- 
Regards,
Igor.


More information about the U-Boot mailing list