Re: [PATCH 2/2] POWERPC: Remove global CPM mappings

[Vitaly Bordug <vbordug@ru.mvista.com>]

On Tue, 27 Mar 2007 18:12:38 -0400
Dan Malek wrote:

> 
> On Mar 27, 2007, at 5:01 PM, Vitaly Bordug wrote:
> 
> > --- a/drivers/serial/cpm_uart/cpm_uart_cpm1.c
> > +++ b/drivers/serial/cpm_uart/cpm_uart_cpm1.c
> > @@ -52,7 +52,7 @@
> >  void cpm_line_cr_cmd(int line, int cmd)
> >  {
> >  	ushort val;
> > -	volatile cpm8xx_t *cp = cpmp;
> > +	volatile cpm8xx_t *cp = (cpm8xx_t *)immr_map(im_cpm);
> 
> I'd kind of prefer you didn't place function calls
> or macros up here with the variable declarations.
> Do the initialization down in the code section, please.
> 

ok.
> > +#ifdef CONFIG_CPM2
> >  #define cpm2_map(member)	(&cpm2_immr->member)
> >  #define cpm2_map_size(member, size)	(&cpm2_immr->member)
> >  #define cpm2_unmap(addr)        do {} while(0)
> > +#endif
> > +
> > +#ifdef CONFIG_8xx
> > +#define immr_map(member)	(&((immap_t *)IMAP_ADDR)->member)
> > +#define immr_map_size(member, size)	(&((immap_t
> > *)IMAP_ADDR)->member) +#define immr_unmap(addr)        do {}
> > while(0) +#endif
> 
> Why is 8xx different?  We should be able to
> devine cpm_immr and use it in both cases here,
> and hopefully use the same macro names
> across all drivers, too.
> 
> 
The upper relates to arch/ppc code, which uses different structures and variables (cpm2_immr and immap respectively).  With this, I don't want to unify  cpm and cpm2 immrs, at least, not now. They are separated intentionally: while doing cpm2_map one gets access to one of the cpm2_map_t member, and doing immr_map, to immap_t member. 

Those 2 as you know better than anyone here are very similar and very different at the same time, and masking the difference is confusion-prone I think.

Thanks for looking into this!

--
Sincerely, Vitaly