From: Wolfgang Grandegger <wg@grandegger.com>
To: u-boot@lists.denx.de
Subject: [U-Boot-Users] [PATCH] Added support for multiple serial for MPC8XX (resubmit)
Date: Mon, 18 Jun 2007 07:18:17 +0200 [thread overview]
Message-ID: <46761599.90003@grandegger.com> (raw)
In-Reply-To: <200706171531.33232.sbabic@denx.de>
Stefano Babic wrote:
> On Friday 15 June 2007 21:38, Wolfgang Grandegger wrote:
>> #if defined(CONFIG_8xx_CONS_SMC2) && (defined(CONFIG_MPC823) ||
>> defined(CONFIG_MPC850))
>> volatile iop8xx_t *ip = (iop8xx_t *)&(im->im_ioport);
>> #endif
>
> I see, on MPC823 the port for SMC2 is not configured if both SMCs are used :(.
>
>> you could simple write:
>>
>> if (smc_index == SMC1_INDEX) {
>> /* Use Port B for SMC1 instead of other functions.
>> */
>> cp->cp_pbpar |= 0x000000c0;
>> cp->cp_pbdir &= ~0x000000c0;
>> cp->cp_pbodr &= ~0x000000c0;
>> }
>
> You are right, but my intention was to assure the highest backward
> compatibility, because I am aware that a lot of boards are currently using
> this driver. Changes as you suggest have a (small) impact on the runtime
> behavior, because, to clean up the code, parameter values as
> PROFF_SMC,CPM_CR_CH_SMC must be taken from the structure I fill in. I could
> simplify other parts of code as allocating the buffers from DPRAM from:
>
> #ifdef CONFIG_SERIAL_MULTI
> dpaddr=ALIGN(CPM_SERIAL_BASE+(sizeof(cbd_t)*2+2)*smc_index,8);
> #else
> dpaddr=CPM_SERIAL_BASE;
> #endif
>
> into simply:
> dpaddr=ALIGN(CPM_SERIAL_BASE+(sizeof(cbd_t)*2+2)*smc_index,8);
>
> Theoretically, no problem, there is enough place in DPRAM to do this. To be
> really sure, it should be tested on all boards :(.
I did not propose to use more DPRAM space, but writing the driver in a
way, that it can handle both, the compile and run-time selection with
less #ifdef's.
> I know, we are talking about small changes but I gave compatibility the
> highest priority doing this job.
>
> Now if CONFIG_SERIAL_MULTI is not set, we have (quite) the same driver as in
> the past.
>
> However, I agree with you that this makes code less readable :(.
If we go on like that, U-Boot code becomes more and more unreadable. To
improve code quality, take the risk. Nevertheless, I think what is
really missing is a clever device interface.
Wolfgang.
next prev parent reply other threads:[~2007-06-18 5:18 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2007-06-05 12:47 [U-Boot-Users] [PATCH] Added support for multiple serial for MPC8XX Stefano Babic
2007-06-05 13:53 ` Wolfgang Denk
2007-06-05 19:39 ` Stefano Babic
2007-06-05 23:49 ` Wolfgang Denk
2007-06-06 5:08 ` Stefan Roese
2007-06-06 13:03 ` Stefano Babic
2007-06-06 13:13 ` Stefan Roese
2007-06-06 13:27 ` Stefano Babic
2007-06-06 13:55 ` Wolfgang Denk
2007-06-06 14:20 ` Stefano Babic
2007-06-06 21:35 ` [U-Boot-Users] [PATCH] Added support for multiple serial for MPC8XX (resubmit) Stefano Babic
2007-06-15 16:03 ` Detlev Zundel
2007-06-15 19:38 ` Wolfgang Grandegger
2007-06-17 13:31 ` Stefano Babic
2007-06-17 21:16 ` Wolfgang Denk
2007-06-18 5:18 ` Wolfgang Grandegger [this message]
2007-06-17 11:35 ` Stefano Babic
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=46761599.90003@grandegger.com \
--to=wg@grandegger.com \
--cc=u-boot@lists.denx.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.