All of lore.kernel.org
 help / color / mirror / Atom feed
From: Michal Simek <monstr@monstr.eu>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH v3] net: ll_temac: Add LL TEMAC driver to u-boot
Date: Thu, 01 Sep 2011 15:17:14 +0200	[thread overview]
Message-ID: <4E5F85DA.4080403@monstr.eu> (raw)
In-Reply-To: <20110901130605.A31D2166C886@gemini.denx.de>

Wolfgang Denk wrote:
> Dear Michal Simek,
> 
> In message <1314877154-14536-1-git-send-email-monstr@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

  reply	other threads:[~2011-09-01 13:17 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-09-01 11:39 [U-Boot] [PATCH v3] net: ll_temac: Add LL TEMAC driver to u-boot Michal Simek
2011-09-01 11:39 ` [U-Boot] [PATCH v2] net: axi_ethernet: Add " Michal Simek
2011-09-01 13:17   ` Wolfgang Denk
2011-09-02  9:02     ` Michal Simek
2011-09-01 13:06 ` [U-Boot] [PATCH v3] net: ll_temac: Add LL TEMAC " Wolfgang Denk
2011-09-01 13:17   ` Michal Simek [this message]
2011-09-01 14:09     ` Wolfgang Denk
2011-09-02  6:39       ` Michal Simek
2011-09-02  8:19         ` Wolfgang Denk
2011-09-02  8:40           ` Michal Simek
2011-09-02  9:24             ` Wolfgang Denk
2011-09-02 10:38               ` Michal Simek
2011-09-02 12:53                 ` Wolfgang Denk
2011-09-02 13:29                   ` Michal Simek
2011-09-02 14:08                     ` Wolfgang Denk
2011-09-05  7:15                       ` Michal Simek
2011-09-05  8:00                         ` Wolfgang Denk

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4E5F85DA.4080403@monstr.eu \
    --to=monstr@monstr.eu \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.