[U-Boot] [PATCH 1/2] Serial support extended up to 6 COMs

Michael Zaidman michael.zaidman at gmail.com
Tue Mar 23 17:33:38 CET 2010


Dear Wolfgang,

On Tue, Mar 23, 2010 at 5:04 PM, Wolfgang Denk <wd at denx.de> wrote:
> Dear Michael Zaidman,
>
> In message <660c0f821003230108t579e90femaac28b937e04329c at 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


More information about the U-Boot mailing list