Re: [U-Boot] [PATCH v3] net/designware: make driver compatible with data cache

2014-01-22 Thread Mischa Jonker
Hello Alexey,

In general, a very nice, clean patch.

 + /* Flush modified buffer descriptor */
 + flush_dcache_range((unsigned long)desc_p,
 +(unsigned long)desc_p + sizeof(struct dmamacdescr));
 +

If I remember correctly, there is some bit that tells you if the DMA descriptor 
is owned by CPU or by GMAC. What if the descriptor size is smaller than the 
cache line size of the CPU? How do you prevent the CPU from overwriting 
adiacent DMA descriptors that may be owned by the GMAC?

As far as I can remember, in Linux they solve this by mapping the descriptors 
(not the packet buffers, these are always cacheline aligned) in uncached 
memory, but we cannot do that in u-boot as the MMU is still disabled. OTOH, as 
we may not need to have the performance benefits of the CPU and GMAC 
concurrently accessing the descriptor table, we may be able to work around it 
by handing off multiple descriptors at once from GMAC to CPU and vice versa 
(maybe depending on cache line size?). 

I remember that a similar patch (that looked a lot uglier BTW) solved it by 
doing uncached accesses to the descriptors, but that would require using 
arch-specific accessor macro's (and I'm not sure if all architectures support 
an 'uncached access' attribute/flag with load/store instructions).

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


Re: [U-Boot] [PATCH] net/designware: make driver compatible with data cache

2013-12-24 Thread Mischa Jonker
Hi Alexey,

  * Implement all accesses to shared structures between CPU and GMAC via
 uncached reads/writes (readl/writel).

I don't know how ARC exactly implements this for u-boot, but AFAIK, 
readl/writel are meant for 'strongly ordered' I/O writes, not necessarily 
uncached. The uncached part of it us usually achieved by mapping it into an 
uncached area, but this is not always possible without using the MMU. So you 
may need to allocate descriptors on cache-line boundaries and do manually 
flushing/invalidating.

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


Re: [U-Boot] [PATCH v2] drivers/net/designware - fix alignment of buffer descriptors

2013-09-25 Thread Mischa Jonker
Vipin wrote:
 I have also faced this problem before. May be a better solution is to
 place all the struct and buffer declarations at the very start of
 dw_eth_dev structure (off-course with a comment that these should not
 be moved). It may avoid the problem in later modifications

I think that's why Alexey added the alignment to the struct dmamacdescr 
declaration, to make sure that it always aligned on a boundary of 16 bytes (so 
even 128-bit busses don't face this issue).

I don't know though whether the __aligned attribute should be at the type 
definition of the struct or at the declaration of the struct dmamacdescr 
inside struct dw_eth_dev. I'm guessing the declaration inside struct 
dw_eth_dev will inherit the alignment requirements of the type def though, but 
not sure.

Mischa


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


[U-Boot] [PATCH] mmc/dw_mmc: Fix DMA descriptor corruption

2013-07-26 Thread Mischa Jonker
In dwmci_prepare_data, the descriptors are allocated for DMA transfer.
These are allocated using the ALLOC_CACHE_ALIGN_BUFFER. This macro uses
the stack to allocate these descriptors. This becomes a problem if the
DMA transfer continues after the processor leaves the function in which
the descriptors were allocated.

Therefore, I have moved the allocated of the buffers up one level, to
dwmci_send_cmd(). The DMA transfer should be complete when leaving this
function.

Signed-off-by: Mischa Jonker mjon...@synopsys.com
Cc: Alexey Brodkin abrod...@synopsys.com
Cc: Jaehoon Chung jh80.ch...@samsung.com
Cc: Andy Fleming aflem...@gmail.com
---
 drivers/mmc/dw_mmc.c |7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/mmc/dw_mmc.c b/drivers/mmc/dw_mmc.c
index a82ee17..796a811 100644
--- a/drivers/mmc/dw_mmc.c
+++ b/drivers/mmc/dw_mmc.c
@@ -41,12 +41,11 @@ static void dwmci_set_idma_desc(struct dwmci_idmac *idmac,
 }
 
 static void dwmci_prepare_data(struct dwmci_host *host,
-   struct mmc_data *data)
+   struct mmc_data *data, struct dwmci_idmac *cur_idmac)
 {
unsigned long ctrl;
unsigned int i = 0, flags, cnt, blk_cnt;
ulong data_start, data_end, start_addr;
-   ALLOC_CACHE_ALIGN_BUFFER(struct dwmci_idmac, cur_idmac, data-blocks);
 
 
blk_cnt = data-blocks;
@@ -111,6 +110,8 @@ static int dwmci_send_cmd(struct mmc *mmc, struct mmc_cmd 
*cmd,
struct mmc_data *data)
 {
struct dwmci_host *host = (struct dwmci_host *)mmc-priv;
+   ALLOC_CACHE_ALIGN_BUFFER(struct dwmci_idmac, cur_idmac,
+data ? data-blocks : 0);
int flags = 0, i;
unsigned int timeout = 10;
u32 retry = 1;
@@ -127,7 +128,7 @@ static int dwmci_send_cmd(struct mmc *mmc, struct mmc_cmd 
*cmd,
dwmci_writel(host, DWMCI_RINTSTS, DWMCI_INTMSK_ALL);
 
if (data)
-   dwmci_prepare_data(host, data);
+   dwmci_prepare_data(host, data, cur_idmac);
 
dwmci_writel(host, DWMCI_CMDARG, cmd-cmdarg);
 
-- 
1.7.9.5

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


[U-Boot] [PATCH 2/2] Add parentheses to ALLOC_ALIGN_BUFFER macro's

2013-07-26 Thread Mischa Jonker
Without those it's very easy to make mistakes when for instance
the 'size' field is more than just a constant.

Signed-off-by: Mischa Jonker mjon...@synopsys.com
Cc: Alexey Brodkin abrod...@synopsys.com
Cc: Marek Vasut ma...@denx.de
Cc: Anton Staaf robot...@chromium.org
Cc: Tom Rini tr...@ti.com
Cc: Wolfgang Denk w...@denx.de
---
 include/common.h |4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/common.h b/include/common.h
index 8addf43..cc7454a 100644
--- a/include/common.h
+++ b/include/common.h
@@ -1015,10 +1015,10 @@ static inline phys_addr_t map_to_sysmem(void *ptr)
  * of a function scoped static buffer.  It can not be used to create a cache
  * line aligned global buffer.
  */
-#define PAD_COUNT(s, pad) ((s - 1) / pad + 1)
+#define PAD_COUNT(s, pad) (((s) - 1) / (pad) + 1)
 #define PAD_SIZE(s, pad) (PAD_COUNT(s, pad) * pad)
 #define ALLOC_ALIGN_BUFFER_PAD(type, name, size, align, pad)   \
-   char __##name[ROUND(PAD_SIZE(size * sizeof(type), pad), align)  \
+   char __##name[ROUND(PAD_SIZE((size) * sizeof(type), pad), align)  \
  + (align - 1)];   \
\
type *name = (type *) ALIGN((uintptr_t)__##name, align)
-- 
1.7.9.5

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


[U-Boot] [PATCH 1/2] mmc/dw_mmc: Allocate the correct amount of descriptors

2013-07-26 Thread Mischa Jonker
This fixes two issues:
 * a descriptor was allocated for every block, while a descriptor can
   take 8 blocks
 * there was an off-by-one error in the descriptor preparation: there
   were two last descriptors, one with length==0

Signed-off-by: Mischa Jonker mjon...@synopsys.com
Cc: Alexey Brodkin abrod...@synopsys.com
Cc: Jaehoon Chung jh80.ch...@samsung.com
Cc: Andy Fleming aflem...@gmail.com
---
 drivers/mmc/dw_mmc.c |4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/dw_mmc.c b/drivers/mmc/dw_mmc.c
index 796a811..9a803a0 100644
--- a/drivers/mmc/dw_mmc.c
+++ b/drivers/mmc/dw_mmc.c
@@ -72,7 +72,7 @@ static void dwmci_prepare_data(struct dwmci_host *host,
dwmci_set_idma_desc(cur_idmac, flags, cnt,
start_addr + (i * PAGE_SIZE));
 
-   if(blk_cnt  8)
+   if (blk_cnt = 8)
break;
blk_cnt -= 8;
cur_idmac++;
@@ -111,7 +111,7 @@ static int dwmci_send_cmd(struct mmc *mmc, struct mmc_cmd 
*cmd,
 {
struct dwmci_host *host = (struct dwmci_host *)mmc-priv;
ALLOC_CACHE_ALIGN_BUFFER(struct dwmci_idmac, cur_idmac,
-data ? data-blocks : 0);
+data ? DIV_ROUND_UP(data-blocks, 8) : 0);
int flags = 0, i;
unsigned int timeout = 10;
u32 retry = 1;
-- 
1.7.9.5

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