[U-Boot] [PATCH V2 6/6] arm: mx5: Add support for DENX M53EVK

[Marek Vasut <marex@denx.de>]

Dear Fabio Estevam,

> On Sun, Apr 21, 2013 at 12:52 PM, Marek Vasut <marex@denx.de> wrote:
> > +u32 get_board_rev(void)
> > +{
> > +       struct iim_regs *iim = (struct iim_regs *)IMX_IIM_BASE;
> > +       struct fuse_bank *bank = &iim->bank[0];
> > +       struct fuse_bank0_regs *fuse =
> > +               (struct fuse_bank0_regs *)bank->fuse_regs;
> > +       int rev = readl(&fuse->gp[6]);
> > +
> > +       return (get_cpu_rev() & ~(0xF << 8)) | (rev & 0xF) << 8;
> 
> Do you really need to read the fuses on m53evk?
> 
> It would be better to do the same as we did for mx6:
> 
> Put it into a common mx5 file:
> 
> #ifdef CONFIG_REVISION_TAG
> u32 __weak get_board_rev(void)
> {
> 	return get_cpu_rev();
> }
> #endif
> 
> ,and then remove get_board_rev from your board file.

We don't have that on MX5. Or do you mean I should do the work and submit this 
patch afterwards ?

> > +static void m53_set_nand(void)
> > +{
> > +       u32 i;
> > +
> > +       #define M4IF_GENP_WEIM_MM               0x00000001
> > +       #define WEIM_GCR2_MUX16_BYP_GRANT       0x00001000
> 
> Please put these defines into a common file. mx53ard also uses them.

Which one? Or do you mean generate two files full of register sets because of 
these two bits?

> > +
> > +       /* NAND flash is muxed on ATA pins */
> > +       setbits_le32(M4IF_BASE_ADDR + 0xc, M4IF_GENP_WEIM_MM);
> > +
> > +       /* Wait for Grant/Ack sequence (see EIM_CSnGCR2:MUX16_BYP_GRANT)
> > */ +       for (i = 0x4; i < 0x94; i += 0x18)
> > +               clrbits_le32(WEIM_BASE_ADDR + i,
> > WEIM_GCR2_MUX16_BYP_GRANT); +
> > +       mxc_set_clock(0, 33, MXC_NFC_CLK);
> > +       enable_nfc_clk(1);
> 
> Shouldn't this function be placed into a common mx5 location? mx53ard
> uses the same.

The WEIM and M4IF configuration is board-specific.

Best regards,
Marek Vasut