Re: Ecovec (SH7724) board doesn't work on latest linus tree

[Nobuhiro Iwamatsu <nobuhiro.iwamatsu.yj@renesas.com>]

Hi,

Your patch does not seem to have a meaning.
In the case of 29bit, CAC/UNCAC_ADDR may not return a right address.
I think that it is to use P1SEGADDR, and P2SEGADDR in CAC/UNCAC_ADDR
to easily revise this.
I attached my patch.

Best regards,
  Nobuhiro

From b1f83e75a2dc5a61671d18e8f472450561c9eea7 Mon Sep 17 00:00:00 2001
From: Nobuhiro Iwamatsu <nobuhiro.iwamatsu.yj@renesas.com>
Date: Thu, 8 Sep 2011 17:37:24 +0900
Subject: [PATCH] sh: Fix address calculation of CAC_ADDR and
UNCAC_ADDR in 29bit mode

In the case of 29bit mode, CAC/UNCAC_ADDR does not return a right address.
This revises this problem by using P1SEGADDR and P2SEGADDR in 29bit mode.

Signed-off-by: Nobuhiro Iwamatsu <nobuhiro.iwamatsu.yj@renesas.com>
---
 arch/sh/include/asm/page.h |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/arch/sh/include/asm/page.h b/arch/sh/include/asm/page.h
index 822d608..abcc4dc 100644
--- a/arch/sh/include/asm/page.h
+++ b/arch/sh/include/asm/page.h
@@ -141,8 +141,13 @@ typedef struct page *pgtable_t;
 #endif /* !__ASSEMBLY__ */

 #ifdef CONFIG_UNCACHED_MAPPING
+#if defined(CONFIG_29BIT)
+#define UNCAC_ADDR(addr)	P2SEGADDR(addr)
+#define CAC_ADDR(addr)		P1SEGADDR(addr)
+#else
 #define UNCAC_ADDR(addr)	((addr) - PAGE_OFFSET + uncached_start)
 #define CAC_ADDR(addr)		((addr) - uncached_start + PAGE_OFFSET)
+#endif
 #else
 #define UNCAC_ADDR(addr)	((addr))
 #define CAC_ADDR(addr)		((addr))
-- 
1.7.5.4

2011/9/1 Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>:
>
> Hi Ebihara-san, and Paul
>
> Thank you for your report.
>
>> i think you can run linux-3.0.4 kernel on your Ecovec (SH7724) board
>> in this debug-code.
>>
>>
>>  void dma_cache_sync(struct device *dev, void *vaddr, size_t size,
>>                      enum dma_data_direction direction)
>>  {
>>          void *addr;
>>
>>          addr = __in_29bit_mode() ?
>> -               (void *)CAC_ADDR((unsigned long)vaddr) : vaddr;
>> +               (void *)P1SEGADDR((unsigned long)vaddr) : vaddr;
>
> Paul
>
> I guess your patch tried to share code for sh32/sh64.
> and CAC_ADDR() should be equal P1SEGADDR(), correct ?
>
> I'm not filmier with memory control, but
> on Ecovec case, 1st (and crash case) dma_cache_sync() caller is
> ${LINUX}/arch/sh/mm/consistent.c :: dma_generic_alloc_coherent()
>
> it tried
>
>        ret = (void *)__get_free_pages(gfp, order);
>        (snip)
>        dma_cache_sync(dev, ret, size, DMA_BIDIRECTIONAL);
>
> This "ret" is "8e000000" for now.
>
> but CAC_ADDR(xx)/P1SEGADDR(xx) is defined as below
>
> CAC_ADDR(addr)  ((addr) - uncached_start + PAGE_OFFSET)
> P1SEGADDR(a)    ((__typeof__(a))(((unsigned long)(a) & 0x1fffffff) | P1SEG))
>
> I guess this "ret" should be uncached area if dma_cache_sync(xx) use CAC_ADDR(xx).
>
> If I apply below patch, Ecovec start works without crash, but I'm not sure.
> Is this correct patch ?
>
> --------------------
> diff --git a/arch/sh/mm/consistent.c b/arch/sh/mm/consistent.c
> index f251b5f..198234a 100644
> --- a/arch/sh/mm/consistent.c
> +++ b/arch/sh/mm/consistent.c
> @@ -48,7 +48,7 @@ void *dma_generic_alloc_coherent(struct device *dev, size_t si
>         * Pages from the page allocator may have data present in
>         * cache. So flush the cache before using uncached memory.
>         */
> -       dma_cache_sync(dev, ret, size, DMA_BIDIRECTIONAL);
> +       dma_cache_sync(dev, UNCAC_ADDR(ret), size, DMA_BIDIRECTIONAL);
>
>        ret_nocache = (void __force *)ioremap_nocache(virt_to_phys(ret), size);
>        if (!ret_nocache) {
> -------------------------
>
> But I'm afraid below comment of dma_generic_alloc_coherent()
>
>        /*
>         * Pages from the page allocator may have data present in
>         * cache. So flush the cache before using uncached memory.
>         */
>        dma_cache_sync(dev, ret, size, DMA_BIDIRECTIONAL);
>
>
> Best regards
> ---
> Kuninori Morimoto
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sh" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>



-- 
Nobuhiro Iwamatsu