Dear Wolfgang, On Tue, Mar 23, 2010 at 5:04 PM, Wolfgang Denk <w...@denx.de> wrote: > Dear Michael Zaidman, > > In message <660c0f821003230108t579e90femaac28b937e043...@mail.gmail.com> you > wrote: > >> All these chips are treated in the same way by this patch. Only >> frequency of crystal oscillator or external clock can differs from >> UART ports belonging to CPU. I tried to explain it in the description >> of the change placed at the head of the file. For this purpose the >> CONFIG_SYS_NS16550_HI_PORTS_BGN was added which allows us to >> distinguish between "low" ports belonging to CPU chip and "high" ports >> of additional chip while calculating clock divisor. > > I don't like this distinction. I think we should rather use a flag, or > pointer to handler functions, etc. This way we can get rid of the > index checking here and there in the code. > At the first glance I also did not like it... BUT It fits perfectly here for following reasons: 1) We need this distinguishing for clock divisor calculation only, i.e. single check in single place; 2) The clock is property of the UART chip and not the UART port. All ports of the same chip get the same clock; 3) Any pointers or flags addition will increase memory footprint, while the intention is to reduce; 4) The last but not least consideration - do not make changes affecting a wide range of boards configuration files.
I already did most of corrections you pointed to in your initial code review. Should I submit it? Thanks, Michael _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
