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

Michal Simek monstr at monstr.eu
Thu Sep 1 15:17:14 CEST 2011


Wolfgang Denk wrote:
> 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.

Ok. How to do it?

For bus access it is necessary to use 4B offsets for DCR just 1B
and one system can contains two MACs where the first use 4B offset and the second
1B.


>> +	/* 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.

I forget. Sorry. Will fix it.

> 
>> +	/* 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.

Fixed and checked.

Thanks,
Michal


-- 
Michal Simek, Ing. (M.Eng)
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel 2.6 Microblaze Linux - http://www.monstr.eu/fdt/
Microblaze U-BOOT custodian


More information about the U-Boot mailing list