Re: [U-Boot] [PATCH v3] net/designware: make driver compatible with data cache
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
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
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
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
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
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