[U-Boot] [PATCH 05/14] powerpc/qoirq: Add support for FMan ethernet in Independent mode
Timur Tabi
timur at freescale.com
Thu Jan 27 18:40:25 CET 2011
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
More information about the U-Boot
mailing list