[U-Boot] [PATCH 2/2] mpc5200, digsy_mtc: add support for rev5 board version

[Wolfgang Denk]

Dear Heiko Schocher,

In message <4D2D7122.60909 at denx.de> you wrote:
> 
> > Global question: do we really need an CONFIG_DIGSY_REV5?  Is there not
> > a way to determine the revision by probing the hardware?  For example,
> > the RTC's show up at different addresses on the bus - but ther emight
> > be even easier ways?
> 
> Good question ... as I know, there is no possibility to detect the
> hardware on an other way then over the RTC ... and I don;t want to
> go this way ... what if the RTC is not responding?

Maybe you can ask the hardware guys again?

> >> +	for (i = 0; i < 2; i++) {
> >> +		dev = &flash_info[i];
> >> +
> >> +		if (dev->size) {
> >> +			/* calculate new base addr for this chipselect */
> >> +			base -= dev->size;
> >> +			out_be32(cs_reg, START_REG(base));
> >> +			cs_reg++;
> >> +			out_be32(cs_reg, STOP_REG(base, dev->size));
> >> +			cs_reg++;
> >> +			/* recalculate the sectoraddr in the cfi driver */
> >> +			size += flash_get_size(base, i);
> >> +		}
> >> +	}
> >> +#if defined(CONFIG_DIGSY_REV5)
> >> +	gd->bd->bi_flashstart = base;
> >> +#endif
> > 
> > Why is this #if needed? Why not always set bi_flashstart ?
> 
> It is set in arch/powerpc/lib/board.c before calling update_flash_size(),
> so this adaption is only needed, if the baseaddr is changing on different
> flash sizes, what is valid for the rev5 board ...

It may not be needed, but I think it should compute the same result,
so you should be able to omit the "#if" and the "#endif".

If you should compute a different result, then I'd wonder if the code
is actually correct?

> >> -#endif /* CONFIG_CMD_IDE */
> > 
> > Ah!  So this is a bug fix?
> 
> Yep. Should I seperate this in an extra patch?

At least mention it in the commit message.

> Hmm.. I have only one flash on the !rev5 board, so I need this
> differentiation here, or?

Yes.

> Hmm.. after looking in code, can your proposal work?:
> Is flash_info[0].size valid, when the cfi driver detects the flash?

For the probing, a temporary address can be used.  You set up the
final mapping only after the sizes are knows, similar to what we do
when we have several banks of RAM.


Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
People are always a lot more complicated than you  think.  It's  very
important to remember that.             - Terry Pratchett, _Truckers_