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

[Wolfgang Denk]

Dear Michal Simek,

In message <1314877154-14536-1-git-send-email-monstr at monstr.eu> you wrote:
> LL Temac driver can be used by microblaze, xilinx ppc405/440
> in sdma and fifo mode. DCR or XPS bus can be used.
> 
> The driver uses and requires PHYLIB.


> +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);
> +}
> +
> +static u32 sdma_in_be32(struct ll_priv *priv, u32 offset)
> +{
> +	if (priv->mode & DCR_BIT)
> +		return mfdcr_local(priv->ctrl + offset);
> +
> +	return in_be32((u32 *)(priv->ctrl + offset * 4));
> +}

Can we please get rid of these functions?  As mentioned many, many
times before, we discourage all use of "base address plus offset" to
access any device registers etc.

These functions here re-introduce such accesses, and this is something
I will not accept.


> +	/* Soft reset the DMA.  */
> +	sdma_out_be32(priv, DMA_CONTROL_REG, DMA_CONTROL_RESET);
> +	while (sdma_in_be32(priv, DMA_CONTROL_REG) & DMA_CONTROL_RESET)
> +		;

I think this has been pointed out before: we do not accept such
potentially endless loops. Please fix this  (and integrate theother
previously made review remarks).  Thanks.

> +	do {
> +		flush_cache((u32)&tx_bd, sizeof(tx_bd));
> +	} while (!(tx_bd.stat & BDSTAT_COMPLETED_MASK));

Ditto. Please fix globally.

> +	/* Read out the packet info and start the DMA
> +	   onto the second buffer to enable the ethernet rx
> +	   path to run in parallel with sw processing
> +	   packets.  */

Incorrect multiline comment style; please fix globally.

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
The project was large enough and management communication poor enough
to prompt many members of the team to see themselves  as  contestants
making  brownie  points,  rather  than as builders making programming
products. Each suboptimized  his  piece  to  meet  his  targets;  few
stopped to think about the total effect on the customer.
                              - Fred Brooks, "The Mythical Man Month"