Mingkai Hu wrote:
> +LIB  := $(obj)libfm.a

Libraries are now .o files, not .a files.  Take a look at the other Makefiles to
see how they've recently changed.

> +u32 fm_muram_alloc(struct fm_muram *mem, u32 size, u32 align)
> +{
> +     u32 ret;
> +     u32 align_mask, off;
> +     u32 save;
> +
> +     align_mask = align - 1;
> +     save = mem->alloc;
> +
> +     if ((off = (save & align_mask)) != 0)
> +             mem->alloc += (align - off);
> +     if ((off = size & align_mask) != 0)
> +             size += (align - off);
> +     if ((mem->alloc + size) >= mem->top) {
> +             mem->alloc = save;
> +             printf("%s: run out of ram.\n", __func__);
> +     }

What exactly happens here?  If you run out of memory, you return a pointer
anyway?  Shouldn't you return NULL here or something?

> +     /* Loop through each microcode. */
> +     for (i = 0; i < firmware->count; i++) {
> +             const struct qe_microcode *ucode = &firmware->microcode[i];
> +
> +             /* Upload a microcode if it's present */
> +             if (ucode->code_offset) {
> +                     printf("Fman: Uploading microcode version %u.%u.%u.\n",
> +                            ucode->major, ucode->minor, ucode->revision);
> +                     fm->ucode = (void *) CONFIG_SYS_FMAN_FW_ADDR +
> +                             ucode->code_offset;

Just FYI, this code will only work on systems that have NOR flash mapped to I/O
space.  If the customer boots from NAND and doesn't have any NOR flash on his
board, then CONFIG_SYS_FMAN_FW_ADDR will be invalid.  In addition, I don't see
any code that passes the value of CONFIG_SYS_FMAN_FW_ADDR to the kernel via
"fman_ucode".

> +     /* setup TxBD */
> +     txbd->buf_ptr_hi = 0;
> +     txbd->buf_ptr_lo = (u32)buf;
> +     txbd->len = len;
> +     __asm__ __volatile__ ("sync");

Why do we sometimes do "asm("sync");" and sometimes we do "__asm__ __volatile__
("sync");"?  And why do we have hard-coded asm anyway?  There is a sync()
function that does this already.


> +     txbd->status = TxBD_READY | TxBD_LAST;
> +     __asm__ __volatile__ ("sync");
> +
> +     /* update TxQD, let RISC to send the packet */
> +     offset_in = muram_readw(&pram->txqd.offset_in);
> +     offset_in += SIZEOFBD;
> +     if (offset_in >= muram_readw(&pram->txqd.bd_ring_size))
> +             offset_in = 0;
> +     muram_writew(&pram->txqd.offset_in, offset_in);
> +     __asm__ __volatile__ ("sync");
> +
> +     /* wait for buffer to be transmitted */
> +     for (i = 0; txbd->status & TxBD_READY; i++) {
> +             udelay(1000);
> +             if (i > 0x10000) {
> +                     printf("%s: Tx error\n", dev->name);
> +                     return 0;
> +             }
> +     }

If you're going to do a timeout, do it the same way everyone else does.  Start
with the timeout value, and decrement the variable.  Something like this:

        timeout = 0x10000; (why a hex number anyway?)
        while (!(txbd->status & TxBD_READY) && --timeout)
                udelay(1000);

> +static void
> +__def_board_ft_fman_fixup_port(void *blob, char * prop, phys_addr_t pa,
> +                             enum fm_port port, int offset)
> +{
> +     return ;

Delete this "return ;".  It doesn't do anything.

-- 
Timur Tabi
Linux kernel developer at Freescale

_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to