> On Fri, 2012-01-13 at 13:41 +0100, Marek Vasut wrote: > > > +#define KPDK_DK1 (0x1 << 1) > > > +#define KPDK_DK0 (0x1 << 0) > > > > Drop those two spaces here > > Two spaces? It's tabs! Btw, it's for good looking formatting and > it was taken from pxa-regs.h. I'd like to preserve it like it is.
Then change it to space ... (1 <<[TAB]3) is even weirder. And on my side, it still looks like two spaces. > > > > +static unsigned char queue[64] = {0}; > > > +static int queue_len; > > > +static struct pxa_keypad_regs *regs; > > > > You can as well assign it here. > > Ok > > > > +static int scan_keys(int scan_modif, uint32_t kpasmkp[4]) > > > +{ > > > + uint32_t reg = 0; > > > + int col, row; > > > + static int mod = MOD_NONE; > > > > Are you sure you know what you're doing using all those static vars all > > around? > > I'm sure I know what I'm doing, right here it preserves modifier between > two function calls, i.e. scan_keys(1, ...); scan_keys(0, ...); > > > > + if ((reg >> 16) & (1 << row)) { > > > + if (scan_modif) { > > > + mod = kbd_get_mdf(row, col + 1); > > > + if (mod != MOD_NONE) > > > + return mod; > > > + } else { > > > + key = kbd_lookup(row, col + 1, mod); > > > + if (key != 0xff) > > > + return key; > > > + } > > > > Can you do some elegant solution of this code duplication? > > Ok > > > > + } > > > + } > > > + } > > > > Newline > > Ok > > > > + /* wait for scan to finish */ > > > + while (timeout--) { > > > + if (!(readl(®s->kpc) & KPC_AS)) > > > + break; > > > + udelay(10); > > > > Use WATCHDOG_RESET() > > Why the heck you want to reset here? It's not fatal. We can handle this > timeout gracefully, without resetting and messing up all the stuff. WATCHDOG_RESET() reloads the WDT ... > > > > + } else { > > > + key_counter = 0; > > > + last_key_row = last_key_col = 0xff; > > > + scan_keys(0, kpasmkp_diff); > > > + serial_printf("New key\n"); > > > > What's this dammit? And above too! Use debug() if you need it. > > Sorry, left it occasionally. > > > > + kbddev.getc = kbd_getc ; > > > + kbddev.tstc = kbd_testc ; > > > > Drop spaces before semicolon > > Ok > > Thanks for review! > > Regards, > Vasily _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot