subject:"\[U\-Boot\] \[PATCH 1\/2\] drivers\/net\/designware, do an explicit memory access instead of implicit, re\-written assignments to use readl\(\) and writel\(\), all of this as preperation for making the driver "

Re: [U-Boot] [PATCH 1/2] drivers/net/designware, do an explicit memory access instead of implicit, re-written assignments to use readl() and writel(), all of this as preperation for making the driver

2013-01-24 Thread Frank Dols

 Hi Vipin,
 On Wed, 23 Jan 2013 15:46:31 +0530, Vipin Kumar vipin.ku...@st.com
 wrote:
  My first feeling is that the descriptors are allocated as Normal  
  Cachabale memory and it would not help to access them using readl/writel
  ...
  And no, we don't need to allocate them non-cacheable, although in this case 
  we need to use cache flush and invalidate calls. I would 
  suggest doing so rather than allocating the descriptors none cacheable, 
  because using non-cacheable memory makes the dependency 
  between the driver and cache codes implicit (and thus more prone to 
  improperly thought out changes in either code) and the memory usage 
  more complex, while explicit cache operations make the relationship 
  explicit.
 Yes, got it. Thanks Albert
 Frank, so in that case rather changing the code to use readl/writel, cache 
 flush and invalidate operations need to be performed at appropriate places
 Vipin
 I believe patch 2/2 adds explicit cache ops, though I haven't read it in 
 detail and thus don't know if everything needed is present and ok.
 Amicalement, Albert.

Hi Vipin and Albert,
Sorry, I have to clarify here a bit more.
The descriptors are 16 bytes in length and a cache line is in most 
architectures more than 16 bytes in length (in our case either 32 or 64).
This means that cached accesses is not an option for these descriptors. 
Background, two adjacent descriptors as be on one cache line may be owned by 
different entities (host cpu / network ip).
Explicit cache calls that we are added in patch 2/2 are meant for payload of 
the package. And these are made cache line aligned with patch 1/2.
Unfortunately we can't align the descriptors on cache line boundaries due to 
hardware limitations (for architectures with cache line longer than 16 bytes) !
With kind regards, greetings, Frank.

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH 1/2] drivers/net/designware, do an explicit memory access instead of implicit, re-written assignments to use readl() and writel(), all of this as preperation for making the driver

2013-01-23 Thread Vipin Kumar


On 1/23/2013 12:25 PM, Albert ARIBAUD wrote:

Hi Vipin,


My first feeling is that the descriptors are allocated as Normal
Cachabale memory and it would not help to access them using readl/writel

Should the desciptors be allocated as non-cachable memory. If yes then
how to do that in u-boot

I suppose the rest of the code would be better reviewed if we know about
this

Vipin


I would say that yes, descriptors are allocated in DRAM, so they are
cacheable.

And no, we don't need to allocate them non-cacheable, although in this
case we need to use cache flush and invalidate calls. I would suggest
doing so rather than allocating the descriptors non cacheable, because
using non-cacheable memory makes the dependency between the driver and
cache codes implicit (and thus more prone to improperly thought out
changes in either code) and the memory usage more complex, while
explicit cache operations make the relationship explicit.



Yes, got it. Thanks Albert

Frank, so in that case rather changing the code to use readl/writel, 
cache flush and invalidate operations need to be performed at 
appropriate places


Regards
Vipin


One can always not cache descriptors if you one wants to, without this
decision breaking driver functionality.

Amicalement,


___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH 1/2] drivers/net/designware, do an explicit memory access instead of implicit, re-written assignments to use readl() and writel(), all of this as preperation for making the driver

2013-01-23 Thread Albert ARIBAUD

Hi Vipin,

On Wed, 23 Jan 2013 15:46:31 +0530, Vipin Kumar vipin.ku...@st.com
wrote:

 On 1/23/2013 12:25 PM, Albert ARIBAUD wrote:
  Hi Vipin,
 
  My first feeling is that the descriptors are allocated as Normal
  Cachabale memory and it would not help to access them using readl/writel
 
  Should the desciptors be allocated as non-cachable memory. If yes then
  how to do that in u-boot
 
  I suppose the rest of the code would be better reviewed if we know about
  this
 
  Vipin
 
  I would say that yes, descriptors are allocated in DRAM, so they are
  cacheable.
 
  And no, we don't need to allocate them non-cacheable, although in this
  case we need to use cache flush and invalidate calls. I would suggest
  doing so rather than allocating the descriptors non cacheable, because
  using non-cacheable memory makes the dependency between the driver and
  cache codes implicit (and thus more prone to improperly thought out
  changes in either code) and the memory usage more complex, while
  explicit cache operations make the relationship explicit.
 
 
 Yes, got it. Thanks Albert
 
 Frank, so in that case rather changing the code to use readl/writel, 
 cache flush and invalidate operations need to be performed at 
 appropriate places

I believe patch 2/2 adds explicit cache ops, though I haven't read it in
detail and thus don't know if everything needed is present and ok.

Amicalement,
-- 
Albert.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH 1/2] drivers/net/designware, do an explicit memory access instead of implicit, re-written assignments to use readl() and writel(), all of this as preperation for making the driver able

2013-01-22 Thread Frank Dols

Signed-off-by: Frank Dols frank.d...@synopsys.com
---
 drivers/net/designware.c |  108 +++---
 drivers/net/designware.h |4 +-
 2 files changed, 66 insertions(+), 46 deletions(-)

diff --git a/drivers/net/designware.c b/drivers/net/designware.c
index bf21a08..2f235d5 100644
--- a/drivers/net/designware.c
+++ b/drivers/net/designware.c
@@ -44,29 +44,36 @@ static void tx_descs_init(struct eth_device *dev)
struct dmamacdescr *desc_p;
u32 idx;
 
+   txbuffs = (char *) unsigned long) txbuffs) +
+   CONFIG_SYS_CACHELINE_SIZE) 
+   (~(CONFIG_SYS_CACHELINE_SIZE - 1)));
+
for (idx = 0; idx  CONFIG_TX_DESCR_NUM; idx++) {
desc_p = desc_table_p[idx];
-   desc_p-dmamac_addr = txbuffs[idx * CONFIG_ETH_BUFSIZE];
-   desc_p-dmamac_next = desc_table_p[idx + 1];
+
+   writel((ulong)txbuffs[(idx * CONFIG_ETH_BUFSIZE)],
+   desc_p-dmamac_addr);
+   writel((ulong)desc_table_p[idx + 1], desc_p-dmamac_next);
 
 #if defined(CONFIG_DW_ALTDESCRIPTOR)
-   desc_p-txrx_status = ~(DESC_TXSTS_TXINT | DESC_TXSTS_TXLAST |
-   DESC_TXSTS_TXFIRST | DESC_TXSTS_TXCRCDIS | \
-   DESC_TXSTS_TXCHECKINSCTRL | \
-   DESC_TXSTS_TXRINGEND | DESC_TXSTS_TXPADDIS);
-
-   desc_p-txrx_status |= DESC_TXSTS_TXCHAIN;
-   desc_p-dmamac_cntl = 0;
-   desc_p-txrx_status = ~(DESC_TXSTS_MSK | DESC_TXSTS_OWNBYDMA);
+   writel(readl(desc_p-txrx_status)  ~(DESC_TXSTS_TXINT |
+   DESC_TXSTS_TXLAST | DESC_TXSTS_TXFIRST |
+   DESC_TXSTS_TXCRCDIS | DESC_TXSTS_TXCHECKINSCTRL |
+   DESC_TXSTS_TXRINGEND | DESC_TXSTS_TXPADDIS),
+   desc_p-txrx_status);
+   writel(readl(desc_p-txrx_status) | DESC_TXSTS_TXCHAIN,
+   desc_p-txrx_status);
+   writel(0, desc_p-dmamac_cntl);
+   writel(readl(desc_p-txrx_status)  ~(DESC_TXSTS_MSK |
+   DESC_TXSTS_OWNBYDMA), desc_p-txrx_status);
 #else
-   desc_p-dmamac_cntl = DESC_TXCTRL_TXCHAIN;
-   desc_p-txrx_status = 0;
+   writel(DESC_TXCTRL_TXCHAIN, desc_p-dmamac_cntl);
+   writel(0, desc_p-txrx_status);
 #endif
}
 
/* Correcting the last pointer of the chain */
-   desc_p-dmamac_next = desc_table_p[0];
-
+   writel((ulong)desc_table_p[0], desc_p-dmamac_next);
writel((ulong)desc_table_p[0], dma_p-txdesclistaddr);
 }
 
@@ -79,21 +86,23 @@ static void rx_descs_init(struct eth_device *dev)
struct dmamacdescr *desc_p;
u32 idx;
 
+   rxbuffs = (char *) unsigned long) rxbuffs) +
+   CONFIG_SYS_CACHELINE_SIZE) 
+   (~(CONFIG_SYS_CACHELINE_SIZE - 1)));
+
for (idx = 0; idx  CONFIG_RX_DESCR_NUM; idx++) {
desc_p = desc_table_p[idx];
-   desc_p-dmamac_addr = rxbuffs[idx * CONFIG_ETH_BUFSIZE];
-   desc_p-dmamac_next = desc_table_p[idx + 1];
 
-   desc_p-dmamac_cntl =
-   (MAC_MAX_FRAME_SZ  DESC_RXCTRL_SIZE1MASK) | \
- DESC_RXCTRL_RXCHAIN;
-
-   desc_p-txrx_status = DESC_RXSTS_OWNBYDMA;
+   writel((ulong)rxbuffs[idx * CONFIG_ETH_BUFSIZE],
+   desc_p-dmamac_addr);
+   writel((ulong)desc_table_p[idx + 1], desc_p-dmamac_next);
+   writel((MAC_MAX_FRAME_SZ  DESC_RXCTRL_SIZE1MASK) |
+   DESC_RXCTRL_RXCHAIN, desc_p-dmamac_cntl);
+   writel(DESC_RXSTS_OWNBYDMA, desc_p-txrx_status);
}
 
/* Correcting the last pointer of the chain */
-   desc_p-dmamac_next = desc_table_p[0];
-
+   writel((ulong)desc_table_p[0], desc_p-dmamac_next);
writel((ulong)desc_table_p[0], dma_p-rxdesclistaddr);
 }
 
@@ -134,7 +143,7 @@ static int dw_write_hwaddr(struct eth_device *dev)
u32 macid_lo, macid_hi;
u8 *mac_id = dev-enetaddr[0];
 
-   macid_lo = mac_id[0] + (mac_id[1]  8) + \
+   macid_lo = mac_id[0] + (mac_id[1]  8) +
   (mac_id[2]  16) + (mac_id[3]  24);
macid_hi = mac_id[4] + (mac_id[5]  8);
 
@@ -198,7 +207,6 @@ static int dw_eth_init(struct eth_device *dev, bd_t *bis)
 */
writel(readl(dma_p-opmode) | RXSTART, dma_p-opmode);
writel(readl(dma_p-opmode) | TXSTART, dma_p-opmode);
-
writel(readl(mac_p-conf) | RXENABLE | TXENABLE, mac_p-conf);
 
return 0;
@@ -212,26 +220,27 @@ static int dw_eth_send(struct eth_device *dev, void 
*packet, int length)
struct dmamacdescr *desc_p = priv-tx_mac_descrtable[desc_num];
 
/* Check if the descriptor is owned by CPU */
-   if (desc_p-txrx_status  DESC_TXSTS_OWNBYDMA) {
+   if

Re: [U-Boot] [PATCH 1/2] drivers/net/designware, do an explicit memory access instead of implicit, re-written assignments to use readl() and writel(), all of this as preperation for making the driver

2013-01-22 Thread Vipin Kumar

Provide a short patch title and a longer description in the patch 
itself. that would be much more readable


On 1/22/2013 7:40 PM, Frank Dols wrote:

Signed-off-by: Frank Dolsfrank.d...@synopsys.com
---
  drivers/net/designware.c |  108 +++---
  drivers/net/designware.h |4 +-
  2 files changed, 66 insertions(+), 46 deletions(-)

diff --git a/drivers/net/designware.c b/drivers/net/designware.c
index bf21a08..2f235d5 100644
--- a/drivers/net/designware.c
+++ b/drivers/net/designware.c
@@ -44,29 +44,36 @@ static void tx_descs_init(struct eth_device *dev)
struct dmamacdescr *desc_p;
u32 idx;

+   txbuffs = (char *) unsigned long) txbuffs) +
+   CONFIG_SYS_CACHELINE_SIZE)
+   (~(CONFIG_SYS_CACHELINE_SIZE - 1)));
+
for (idx = 0; idx  CONFIG_TX_DESCR_NUM; idx++) {
desc_p =desc_table_p[idx];
-   desc_p-dmamac_addr =txbuffs[idx * CONFIG_ETH_BUFSIZE];
-   desc_p-dmamac_next =desc_table_p[idx + 1];
+
+   writel((ulong)txbuffs[(idx * CONFIG_ETH_BUFSIZE)],
+   desc_p-dmamac_addr);
+   writel((ulong)desc_table_p[idx + 1],desc_p-dmamac_next);



My first feeling is that the descriptors are allocated as Normal 
Cachabale memory and it would not help to access them using readl/writel


Should the desciptors be allocated as non-cachable memory. If yes then 
how to do that in u-boot


I suppose the rest of the code would be better reviewed if we know about 
this


Vipin


  #if defined(CONFIG_DW_ALTDESCRIPTOR)
-   desc_p-txrx_status= ~(DESC_TXSTS_TXINT | DESC_TXSTS_TXLAST |
-   DESC_TXSTS_TXFIRST | DESC_TXSTS_TXCRCDIS | \
-   DESC_TXSTS_TXCHECKINSCTRL | \
-   DESC_TXSTS_TXRINGEND | DESC_TXSTS_TXPADDIS);
-
-   desc_p-txrx_status |= DESC_TXSTS_TXCHAIN;
-   desc_p-dmamac_cntl = 0;
-   desc_p-txrx_status= ~(DESC_TXSTS_MSK | DESC_TXSTS_OWNBYDMA);
+   writel(readl(desc_p-txrx_status)  ~(DESC_TXSTS_TXINT |
+   DESC_TXSTS_TXLAST | DESC_TXSTS_TXFIRST |
+   DESC_TXSTS_TXCRCDIS | DESC_TXSTS_TXCHECKINSCTRL |
+   DESC_TXSTS_TXRINGEND | DESC_TXSTS_TXPADDIS),
+   desc_p-txrx_status);
+   writel(readl(desc_p-txrx_status) | DESC_TXSTS_TXCHAIN,
+   desc_p-txrx_status);
+   writel(0,desc_p-dmamac_cntl);
+   writel(readl(desc_p-txrx_status)  ~(DESC_TXSTS_MSK |
+   DESC_TXSTS_OWNBYDMA),desc_p-txrx_status);
  #else
-   desc_p-dmamac_cntl = DESC_TXCTRL_TXCHAIN;
-   desc_p-txrx_status = 0;
+   writel(DESC_TXCTRL_TXCHAIN,desc_p-dmamac_cntl);
+   writel(0,desc_p-txrx_status);
  #endif
}

/* Correcting the last pointer of the chain */
-   desc_p-dmamac_next =desc_table_p[0];
-
+   writel((ulong)desc_table_p[0],desc_p-dmamac_next);
writel((ulong)desc_table_p[0],dma_p-txdesclistaddr);
  }

@@ -79,21 +86,23 @@ static void rx_descs_init(struct eth_device *dev)
struct dmamacdescr *desc_p;
u32 idx;

+   rxbuffs = (char *) unsigned long) rxbuffs) +
+   CONFIG_SYS_CACHELINE_SIZE)
+   (~(CONFIG_SYS_CACHELINE_SIZE - 1)));
+
for (idx = 0; idx  CONFIG_RX_DESCR_NUM; idx++) {
desc_p =desc_table_p[idx];
-   desc_p-dmamac_addr =rxbuffs[idx * CONFIG_ETH_BUFSIZE];
-   desc_p-dmamac_next =desc_table_p[idx + 1];

-   desc_p-dmamac_cntl =
-   (MAC_MAX_FRAME_SZ  DESC_RXCTRL_SIZE1MASK) | \
- DESC_RXCTRL_RXCHAIN;
-
-   desc_p-txrx_status = DESC_RXSTS_OWNBYDMA;
+   writel((ulong)rxbuffs[idx * CONFIG_ETH_BUFSIZE],
+   desc_p-dmamac_addr);
+   writel((ulong)desc_table_p[idx + 1],desc_p-dmamac_next);
+   writel((MAC_MAX_FRAME_SZ  DESC_RXCTRL_SIZE1MASK) |
+   DESC_RXCTRL_RXCHAIN,desc_p-dmamac_cntl);
+   writel(DESC_RXSTS_OWNBYDMA,desc_p-txrx_status);
}

/* Correcting the last pointer of the chain */
-   desc_p-dmamac_next =desc_table_p[0];
-
+   writel((ulong)desc_table_p[0],desc_p-dmamac_next);
writel((ulong)desc_table_p[0],dma_p-rxdesclistaddr);
  }

@@ -134,7 +143,7 @@ static int dw_write_hwaddr(struct eth_device *dev)
u32 macid_lo, macid_hi;
u8 *mac_id =dev-enetaddr[0];

-   macid_lo = mac_id[0] + (mac_id[1]  8) + \
+   macid_lo = mac_id[0] + (mac_id[1]  8) +
   (mac_id[2]  16) + (mac_id[3]  24);
macid_hi = mac_id[4] + (mac_id[5]  8);

@@ -198,7 +207,6 @@ static int dw_eth_init(struct eth_device *dev, bd_t *bis)
 */
writel(readl(dma_p-opmode) | RXSTART,dma_p-opmode);

Re: [U-Boot] [PATCH 1/2] drivers/net/designware, do an explicit memory access instead of implicit, re-written assignments to use readl() and writel(), all of this as preperation for making the driver

2013-01-22 Thread Albert ARIBAUD

Hi Vipin,

 My first feeling is that the descriptors are allocated as Normal 
 Cachabale memory and it would not help to access them using readl/writel
 
 Should the desciptors be allocated as non-cachable memory. If yes then 
 how to do that in u-boot
 
 I suppose the rest of the code would be better reviewed if we know about 
 this
 
 Vipin

I would say that yes, descriptors are allocated in DRAM, so they are
cacheable.

And no, we don't need to allocate them non-cacheable, although in this
case we need to use cache flush and invalidate calls. I would suggest
doing so rather than allocating the descriptors non cacheable, because
using non-cacheable memory makes the dependency between the driver and
cache codes implicit (and thus more prone to improperly thought out
changes in either code) and the memory usage more complex, while
explicit cache operations make the relationship explicit.

One can always not cache descriptors if you one wants to, without this
decision breaking driver functionality.

Amicalement,
-- 
Albert.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH 1/2] drivers/net/designware, do an explicit memory access instead of implicit, re-written assignments to use readl() and writel(), all of this as preperation for making the driver

Re: [U-Boot] [PATCH 1/2] drivers/net/designware, do an explicit memory access instead of implicit, re-written assignments to use readl() and writel(), all of this as preperation for making the driver

Re: [U-Boot] [PATCH 1/2] drivers/net/designware, do an explicit memory access instead of implicit, re-written assignments to use readl() and writel(), all of this as preperation for making the driver

[U-Boot] [PATCH 1/2] drivers/net/designware, do an explicit memory access instead of implicit, re-written assignments to use readl() and writel(), all of this as preperation for making the driver able

Re: [U-Boot] [PATCH 1/2] drivers/net/designware, do an explicit memory access instead of implicit, re-written assignments to use readl() and writel(), all of this as preperation for making the driver

Re: [U-Boot] [PATCH 1/2] drivers/net/designware, do an explicit memory access instead of implicit, re-written assignments to use readl() and writel(), all of this as preperation for making the driver

6 matches

Site Navigation

Mail list logo

Footer information