From mboxrd@z Thu Jan 1 00:00:00 1970 From: Hannes Petermaier Date: Wed, 18 Mar 2015 16:07:29 +0100 Subject: [U-Boot] [PATCH v2 3/4] common/lcd_console: move single static variables into common (static) structure In-Reply-To: <550973CD.8010501@compulab.co.il> References: <1426664243-30998-1-git-send-email-oe5hpm@oevsv.at> <1426664243-30998-4-git-send-email-oe5hpm@oevsv.at> <55096B7F.3090707@compulab.co.il> <55096C16.3010907@petermaier.org> <550973CD.8010501@compulab.co.il> Message-ID: <550994B1.5090803@petermaier.org> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de 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. best regards, Hannes