From mboxrd@z Thu Jan  1 00:00:00 1970
From: Wolfgang Grandegger <wg@grandegger.com>
Date: Mon, 18 Jun 2007 07:18:17 +0200
Subject: [U-Boot-Users] [PATCH] Added support for multiple serial for
 MPC8XX (resubmit)
In-Reply-To: <200706171531.33232.sbabic@denx.de>
References: <20070606135552.DF3B23535E1@atlas.denx.de>	<m2ejkdtdat.fsf@sowhat.denx.de>
	<4672EAAD.70709@grandegger.com> <200706171531.33232.sbabic@denx.de>
Message-ID: <46761599.90003@grandegger.com>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.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.