[U-Boot] [PATCH v3] net: ll_temac: Add LL TEMAC driver to u-boot

Wolfgang Denk wd at denx.de
Fri Sep 2 10:19:01 CEST 2011


Dear Michal Simek,

In message <4E607A0B.3040608 at monstr.eu> you wrote:
>
> >>>> +static void sdma_out_be32(struct ll_priv *priv, u32 offset, u32 val)
> >>>> +{
> >>>> +	if (priv->mode & DCR_BIT)
> >>>> +		mtdcr_local(priv->ctrl + offset, val);
> >>>> +	else
> >>>> +		out_be32((u32 *)(priv->ctrl + offset * 4), val);
> >>>> +}
...
> On PPC system with DCR is special connection between memory controller through DCR bus. Handling is done
> with mfdcr_local and mtdcr_local functions.
> 
> DMA : Sdma address ranges (www.xilinx.com/support/documentation/user_guides/ug200.pdf page 261 and 299)
> 0   : 0x80-0x90
> 1   : 0x98-0xA8
> 2   : 0xB0-0xC0
> 3   : 0xC8-0xD8
> 
> The first reg for DMA2 accessed trough DCR is at 0xB0, the second at 0xB1, etc..

This is indeed a good example, as it shows how terribly broken your
code is.

See function sdma_out_be32() above.  It is suppose to write a 32 bit
value ("u32 val") as a 32 bit entity in big endian mode ("_be32") to
some device register - but the register addresses are (1) not aligned
to 32 bit boundaries and (2) not even 32 bits apart.

Oh!  You are NOT writing 32 bit data? You are writing only 8 bit?
But the function claims to be writing 32 bit!!!

And your function mtdcr_local() is actually a NOOP for anything but
PPC?  Without even a warning?


This code is obfuscated in multiple levels.

I will not accept that.

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
What is tolerance? -- it is the consequence of humanity. We  are  all
formed  of frailty and error; let us pardon reciprocally each other's
folly -- that is the first law of nature.                  - Voltaire


More information about the U-Boot mailing list