dmaengine: fsldma: Add 64-bit I/O accessors for powerpc64

Commit Message

Crystal Wood Dec. 22, 2018, 4:34 a.m. UTC

Otherwise 64-bit PPC builds fail with undefined references
to these accessors.

Cc: Peng Ma <[e-mail]>
Cc: Wen He <[e-mail]>
Fixes: 68997fff94afa (" dmaengine: fsldma: Adding macro FSL_DMA_IN/OUT implement for ARM platform")
Signed-off-by: Scott Wood <[e-mail]>
---
Is there any reason why ioreadXXbe() etc can't be used on PPC as well?

 drivers/dma/fsldma.h | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

Comments

Peng Ma Dec. 24, 2018, 3:42 a.m. UTC | #1

Hi Scott,

You are right, we should support powerpc64, so could I changed it as fallows:

diff --git a/drivers/dma/fsldma.h b/drivers/dma/fsldma.h
index 88db939..057babf 100644
--- a/drivers/dma/fsldma.h
+++ b/drivers/dma/fsldma.h
@@ -202,35 +202,10 @@ struct fsldma_chan {
 #define fsl_iowrite32(v, p)    out_le32(p, v)
 #define fsl_iowrite32be(v, p)  out_be32(p, v)
 
-#ifndef __powerpc64__
-static u64 fsl_ioread64(const u64 __iomem *addr)
-{
-       u32 fsl_addr = lower_32_bits(addr);
-       u64 fsl_addr_hi = (u64)in_le32((u32 *)(fsl_addr + 1)) << 32;
-
-       return fsl_addr_hi | in_le32((u32 *)fsl_addr);
-}
-
-static void fsl_iowrite64(u64 val, u64 __iomem *addr)
-{
-       out_le32((u32 __iomem *)addr + 1, val >> 32);
-       out_le32((u32 __iomem *)addr, (u32)val);
-}
-
-static u64 fsl_ioread64be(const u64 __iomem *addr)
-{
-       u32 fsl_addr = lower_32_bits(addr);
-       u64 fsl_addr_hi = (u64)in_be32((u32 *)fsl_addr) << 32;
-
-       return fsl_addr_hi | in_be32((u32 *)(fsl_addr + 1));
-}
-
-static void fsl_iowrite64be(u64 val, u64 __iomem *addr)
-{
-       out_be32((u32 __iomem *)addr, val >> 32);
-       out_be32((u32 __iomem *)addr + 1, (u32)val);
-}
-#endif
+#define fsl_ioread64(p)                in_le64(p)
+#define fsl_ioread64be(p)      in_be64(p)
+#define fsl_iowrite64(v, p)    out_le64(p, v)
+#define fsl_iowrite64be(v, p)  out_be64(p, v)
 #endif

Best Regards,
Peng
>-----Original Message-----
>From: Scott Wood <[e-mail]>
>Sent: 2018年12月22日 12:35
>To: Leo Li <[e-mail]>; Zhang Wei <[e-mail]>
>Cc: [e-mail]; [e-mail]; Peng Ma
><[e-mail]>; Wen He <[e-mail]>
>Subject: [PATCH] dmaengine: fsldma: Add 64-bit I/O accessors for powerpc64
>
>Otherwise 64-bit PPC builds fail with undefined references to these accessors.
>
>Cc: Peng Ma <[e-mail]>
>Cc: Wen He <[e-mail]>
>Fixes: 68997fff94afa (" dmaengine: fsldma: Adding macro FSL_DMA_IN/OUT
>implement for ARM platform")
>Signed-off-by: Scott Wood <[e-mail]>
>---
>Is there any reason why ioreadXXbe() etc can't be used on PPC as well?
>
> drivers/dma/fsldma.h | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
>diff --git a/drivers/dma/fsldma.h b/drivers/dma/fsldma.h index
>88db939c04a1..a9b12f82b5c3 100644
>--- a/drivers/dma/fsldma.h
>+++ b/drivers/dma/fsldma.h
>@@ -202,7 +202,12 @@ struct fsldma_chan {
> #define fsl_iowrite32(v, p)	out_le32(p, v)
> #define fsl_iowrite32be(v, p)	out_be32(p, v)
>
>-#ifndef __powerpc64__
>+#ifdef __powerpc64__
>+#define fsl_ioread64(p)		in_le64(p)
>+#define fsl_ioread64be(p)	in_be64(p)
>+#define fsl_iowrite64(v, p)	out_le64(p, v)
>+#define fsl_iowrite64be(v, p)	out_be64(p, v)
>+#else
> static u64 fsl_ioread64(const u64 __iomem *addr)  {
> 	u32 fsl_addr = lower_32_bits(addr);
>--
>2.17.1

Crystal Wood Dec. 24, 2018, 4:45 a.m. UTC | #2

On Mon, 2018-12-24 at 03:42 +0000, Peng Ma wrote:
> Hi Scott,
> 
> You are right, we should support powerpc64, so could I changed it as
> fallows:
> 
> diff --git a/drivers/dma/fsldma.h b/drivers/dma/fsldma.h
> index 88db939..057babf 100644
> --- a/drivers/dma/fsldma.h
> +++ b/drivers/dma/fsldma.h
> @@ -202,35 +202,10 @@ struct fsldma_chan {
>  #define fsl_iowrite32(v, p)    out_le32(p, v)
>  #define fsl_iowrite32be(v, p)  out_be32(p, v)
>  
> -#ifndef __powerpc64__
> -static u64 fsl_ioread64(const u64 __iomem *addr)
> -{
> -       u32 fsl_addr = lower_32_bits(addr);
> -       u64 fsl_addr_hi = (u64)in_le32((u32 *)(fsl_addr + 1)) << 32;
> -
> -       return fsl_addr_hi | in_le32((u32 *)fsl_addr);
> -}
> -
> -static void fsl_iowrite64(u64 val, u64 __iomem *addr)
> -{
> -       out_le32((u32 __iomem *)addr + 1, val >> 32);
> -       out_le32((u32 __iomem *)addr, (u32)val);
> -}
> -
> -static u64 fsl_ioread64be(const u64 __iomem *addr)
> -{
> -       u32 fsl_addr = lower_32_bits(addr);
> -       u64 fsl_addr_hi = (u64)in_be32((u32 *)fsl_addr) << 32;
> -
> -       return fsl_addr_hi | in_be32((u32 *)(fsl_addr + 1));
> -}
> -
> -static void fsl_iowrite64be(u64 val, u64 __iomem *addr)
> -{
> -       out_be32((u32 __iomem *)addr, val >> 32);
> -       out_be32((u32 __iomem *)addr + 1, (u32)val);
> -}
> -#endif
> +#define fsl_ioread64(p)                in_le64(p)
> +#define fsl_ioread64be(p)      in_be64(p)
> +#define fsl_iowrite64(v, p)    out_le64(p, v)
> +#define fsl_iowrite64be(v, p)  out_be64(p, v)
>  #endif

Then you'll break 32-bit, assuming those fake-it-with-two-32-bit-accesses were
actually needed.

-Scott

Peng Ma Dec. 24, 2018, 5:29 a.m. UTC | #3

Hi Scott,

Oh, I did not see the in_XX64/out_XX64 supported only __powerpc64__ just now.
Thanks for your reminder.

#ifdef __powerpc64__

#ifdef __BIG_ENDIAN__
DEF_MMIO_OUT_D(out_be64, 64, std);
DEF_MMIO_IN_D(in_be64, 64, ld);

/* There is no asm instructions for 64 bits reverse loads and stores */
static inline u64 in_le64(const volatile u64 __iomem *addr)
{
        return swab64(in_be64(addr));
}

static inline void out_le64(volatile u64 __iomem *addr, u64 val)
{
        out_be64(addr, swab64(val));
}
#else
DEF_MMIO_OUT_D(out_le64, 64, std);
DEF_MMIO_IN_D(in_le64, 64, ld);

/* There is no asm instructions for 64 bits reverse loads and stores */
static inline u64 in_be64(const volatile u64 __iomem *addr)
{
        return swab64(in_le64(addr));
}

static inline void out_be64(volatile u64 __iomem *addr, u64 val)
{
        out_le64(addr, swab64(val));
}

#endif
#endif /* __powerpc64__ */

Best Regards,
Peng
>-----Original Message-----
>From: Scott Wood <[e-mail]>
>Sent: 2018年12月24日 12:46
>To: Peng Ma <[e-mail]>; Leo Li <[e-mail]>; Zhang Wei
><[e-mail]>
>Cc: [e-mail]; [e-mail]; Wen He
><[e-mail]>
>Subject: Re: [PATCH] dmaengine: fsldma: Add 64-bit I/O accessors for
>powerpc64
>
>On Mon, 2018-12-24 at 03:42 +0000, Peng Ma wrote:
>> Hi Scott,
>>
>> You are right, we should support powerpc64, so could I changed it as
>> fallows:
>>
>> diff --git a/drivers/dma/fsldma.h b/drivers/dma/fsldma.h index
>> 88db939..057babf 100644
>> --- a/drivers/dma/fsldma.h
>> +++ b/drivers/dma/fsldma.h
>> @@ -202,35 +202,10 @@ struct fsldma_chan {
>>  #define fsl_iowrite32(v, p)    out_le32(p, v)
>>  #define fsl_iowrite32be(v, p)  out_be32(p, v)
>>
>> -#ifndef __powerpc64__
>> -static u64 fsl_ioread64(const u64 __iomem *addr) -{
>> -       u32 fsl_addr = lower_32_bits(addr);
>> -       u64 fsl_addr_hi = (u64)in_le32((u32 *)(fsl_addr + 1)) << 32;
>> -
>> -       return fsl_addr_hi | in_le32((u32 *)fsl_addr);
>> -}
>> -
>> -static void fsl_iowrite64(u64 val, u64 __iomem *addr) -{
>> -       out_le32((u32 __iomem *)addr + 1, val >> 32);
>> -       out_le32((u32 __iomem *)addr, (u32)val);
>> -}
>> -
>> -static u64 fsl_ioread64be(const u64 __iomem *addr) -{
>> -       u32 fsl_addr = lower_32_bits(addr);
>> -       u64 fsl_addr_hi = (u64)in_be32((u32 *)fsl_addr) << 32;
>> -
>> -       return fsl_addr_hi | in_be32((u32 *)(fsl_addr + 1));
>> -}
>> -
>> -static void fsl_iowrite64be(u64 val, u64 __iomem *addr) -{
>> -       out_be32((u32 __iomem *)addr, val >> 32);
>> -       out_be32((u32 __iomem *)addr + 1, (u32)val);
>> -}
>> -#endif
>> +#define fsl_ioread64(p)                in_le64(p)
>> +#define fsl_ioread64be(p)      in_be64(p)
>> +#define fsl_iowrite64(v, p)    out_le64(p, v)
>> +#define fsl_iowrite64be(v, p)  out_be64(p, v)
>>  #endif
>
>Then you'll break 32-bit, assuming those fake-it-with-two-32-bit-accesses
>were actually needed.
>
>-Scott
>

Vinod Koul Jan. 19, 2019, 12:58 p.m. UTC | #4

On 24-12-18, 05:29, Peng Ma wrote:
> Hi Scott,
> 
> Oh, I did not see the in_XX64/out_XX64 supported only __powerpc64__ just now.
> Thanks for your reminder.

Can you send the formal patch for this...

FWIW, fsl patches were not merged last cycle because of reported
regression...

> 
> #ifdef __powerpc64__
> 
> #ifdef __BIG_ENDIAN__
> DEF_MMIO_OUT_D(out_be64, 64, std);
> DEF_MMIO_IN_D(in_be64, 64, ld);
> 
> /* There is no asm instructions for 64 bits reverse loads and stores */
> static inline u64 in_le64(const volatile u64 __iomem *addr)
> {
>         return swab64(in_be64(addr));
> }
> 
> static inline void out_le64(volatile u64 __iomem *addr, u64 val)
> {
>         out_be64(addr, swab64(val));
> }
> #else
> DEF_MMIO_OUT_D(out_le64, 64, std);
> DEF_MMIO_IN_D(in_le64, 64, ld);
> 
> /* There is no asm instructions for 64 bits reverse loads and stores */
> static inline u64 in_be64(const volatile u64 __iomem *addr)
> {
>         return swab64(in_le64(addr));
> }
> 
> static inline void out_be64(volatile u64 __iomem *addr, u64 val)
> {
>         out_le64(addr, swab64(val));
> }
> 
> #endif
> #endif /* __powerpc64__ */
> 
> Best Regards,
> Peng
> >-----Original Message-----
> >From: Scott Wood <[e-mail]>
> >Sent: 2018年12月24日 12:46
> >To: Peng Ma <[e-mail]>; Leo Li <[e-mail]>; Zhang Wei
> ><[e-mail]>
> >Cc: [e-mail]; [e-mail]; Wen He
> ><[e-mail]>
> >Subject: Re: [PATCH] dmaengine: fsldma: Add 64-bit I/O accessors for
> >powerpc64
> >
> >On Mon, 2018-12-24 at 03:42 +0000, Peng Ma wrote:
> >> Hi Scott,
> >>
> >> You are right, we should support powerpc64, so could I changed it as
> >> fallows:
> >>
> >> diff --git a/drivers/dma/fsldma.h b/drivers/dma/fsldma.h index
> >> 88db939..057babf 100644
> >> --- a/drivers/dma/fsldma.h
> >> +++ b/drivers/dma/fsldma.h
> >> @@ -202,35 +202,10 @@ struct fsldma_chan {
> >>  #define fsl_iowrite32(v, p)    out_le32(p, v)
> >>  #define fsl_iowrite32be(v, p)  out_be32(p, v)
> >>
> >> -#ifndef __powerpc64__
> >> -static u64 fsl_ioread64(const u64 __iomem *addr) -{
> >> -       u32 fsl_addr = lower_32_bits(addr);
> >> -       u64 fsl_addr_hi = (u64)in_le32((u32 *)(fsl_addr + 1)) << 32;
> >> -
> >> -       return fsl_addr_hi | in_le32((u32 *)fsl_addr);
> >> -}
> >> -
> >> -static void fsl_iowrite64(u64 val, u64 __iomem *addr) -{
> >> -       out_le32((u32 __iomem *)addr + 1, val >> 32);
> >> -       out_le32((u32 __iomem *)addr, (u32)val);
> >> -}
> >> -
> >> -static u64 fsl_ioread64be(const u64 __iomem *addr) -{
> >> -       u32 fsl_addr = lower_32_bits(addr);
> >> -       u64 fsl_addr_hi = (u64)in_be32((u32 *)fsl_addr) << 32;
> >> -
> >> -       return fsl_addr_hi | in_be32((u32 *)(fsl_addr + 1));
> >> -}
> >> -
> >> -static void fsl_iowrite64be(u64 val, u64 __iomem *addr) -{
> >> -       out_be32((u32 __iomem *)addr, val >> 32);
> >> -       out_be32((u32 __iomem *)addr + 1, (u32)val);
> >> -}
> >> -#endif
> >> +#define fsl_ioread64(p)                in_le64(p)
> >> +#define fsl_ioread64be(p)      in_be64(p)
> >> +#define fsl_iowrite64(v, p)    out_le64(p, v)
> >> +#define fsl_iowrite64be(v, p)  out_be64(p, v)
> >>  #endif
> >
> >Then you'll break 32-bit, assuming those fake-it-with-two-32-bit-accesses
> >were actually needed.
> >
> >-Scott
> >
>

Peng Ma Jan. 25, 2019, 5:54 a.m. UTC | #5

Hi Vinod,

Sorry to replay late.
1:This patch has already send to the patchwork.
	Please see the patch link: https://patchwork.kernel.org/patch/10741521/
2:I have already compile the fsl patches on arm and powerpc after patched https://patchwork.kernel.org/patch/10741521/
	The compile will successful, please let me know the reported regression results, thanks very much.

Best Regards,
Peng

>-----Original Message-----
>From: Vinod Koul <[e-mail]>
>Sent: 2019年1月19日 20:59
>To: Peng Ma <[e-mail]>
>Cc: Scott Wood <[e-mail]>; Leo Li <[e-mail]>; Zhang Wei
><[e-mail]>; [e-mail];
>[e-mail]; Wen He <[e-mail]>
>Subject: Re: [PATCH] dmaengine: fsldma: Add 64-bit I/O accessors for
>powerpc64
>
>On 24-12-18, 05:29, Peng Ma wrote:
>> Hi Scott,
>>
>> Oh, I did not see the in_XX64/out_XX64 supported only __powerpc64__ just
>now.
>> Thanks for your reminder.
>
>Can you send the formal patch for this...
>
>FWIW, fsl patches were not merged last cycle because of reported regression...
>
>>
>> #ifdef __powerpc64__
>>
>> #ifdef __BIG_ENDIAN__
>> DEF_MMIO_OUT_D(out_be64, 64, std);
>> DEF_MMIO_IN_D(in_be64, 64, ld);
>>
>> /* There is no asm instructions for 64 bits reverse loads and stores
>> */ static inline u64 in_le64(const volatile u64 __iomem *addr) {
>>         return swab64(in_be64(addr));
>> }
>>
>> static inline void out_le64(volatile u64 __iomem *addr, u64 val) {
>>         out_be64(addr, swab64(val));
>> }
>> #else
>> DEF_MMIO_OUT_D(out_le64, 64, std);
>> DEF_MMIO_IN_D(in_le64, 64, ld);
>>
>> /* There is no asm instructions for 64 bits reverse loads and stores
>> */ static inline u64 in_be64(const volatile u64 __iomem *addr) {
>>         return swab64(in_le64(addr));
>> }
>>
>> static inline void out_be64(volatile u64 __iomem *addr, u64 val) {
>>         out_le64(addr, swab64(val));
>> }
>>
>> #endif
>> #endif /* __powerpc64__ */
>>
>> Best Regards,
>> Peng
>> >-----Original Message-----
>> >From: Scott Wood <[e-mail]>
>> >Sent: 2018年12月24日 12:46
>> >To: Peng Ma <[e-mail]>; Leo Li <[e-mail]>; Zhang
>Wei
>> ><[e-mail]>
>> >Cc: [e-mail]; [e-mail]; Wen He
>> ><[e-mail]>
>> >Subject: Re: [PATCH] dmaengine: fsldma: Add 64-bit I/O accessors for
>> >powerpc64
>> >
>> >On Mon, 2018-12-24 at 03:42 +0000, Peng Ma wrote:
>> >> Hi Scott,
>> >>
>> >> You are right, we should support powerpc64, so could I changed it
>> >> as
>> >> fallows:
>> >>
>> >> diff --git a/drivers/dma/fsldma.h b/drivers/dma/fsldma.h index
>> >> 88db939..057babf 100644
>> >> --- a/drivers/dma/fsldma.h
>> >> +++ b/drivers/dma/fsldma.h
>> >> @@ -202,35 +202,10 @@ struct fsldma_chan {
>> >>  #define fsl_iowrite32(v, p)    out_le32(p, v)
>> >>  #define fsl_iowrite32be(v, p)  out_be32(p, v)
>> >>
>> >> -#ifndef __powerpc64__
>> >> -static u64 fsl_ioread64(const u64 __iomem *addr) -{
>> >> -       u32 fsl_addr = lower_32_bits(addr);
>> >> -       u64 fsl_addr_hi = (u64)in_le32((u32 *)(fsl_addr + 1)) << 32;
>> >> -
>> >> -       return fsl_addr_hi | in_le32((u32 *)fsl_addr);
>> >> -}
>> >> -
>> >> -static void fsl_iowrite64(u64 val, u64 __iomem *addr) -{
>> >> -       out_le32((u32 __iomem *)addr + 1, val >> 32);
>> >> -       out_le32((u32 __iomem *)addr, (u32)val);
>> >> -}
>> >> -
>> >> -static u64 fsl_ioread64be(const u64 __iomem *addr) -{
>> >> -       u32 fsl_addr = lower_32_bits(addr);
>> >> -       u64 fsl_addr_hi = (u64)in_be32((u32 *)fsl_addr) << 32;
>> >> -
>> >> -       return fsl_addr_hi | in_be32((u32 *)(fsl_addr + 1));
>> >> -}
>> >> -
>> >> -static void fsl_iowrite64be(u64 val, u64 __iomem *addr) -{
>> >> -       out_be32((u32 __iomem *)addr, val >> 32);
>> >> -       out_be32((u32 __iomem *)addr + 1, (u32)val);
>> >> -}
>> >> -#endif
>> >> +#define fsl_ioread64(p)                in_le64(p)
>> >> +#define fsl_ioread64be(p)      in_be64(p)
>> >> +#define fsl_iowrite64(v, p)    out_le64(p, v)
>> >> +#define fsl_iowrite64be(v, p)  out_be64(p, v)
>> >>  #endif
>> >
>> >Then you'll break 32-bit, assuming those
>> >fake-it-with-two-32-bit-accesses were actually needed.
>> >
>> >-Scott
>> >
>>
>
>--
>~Vinod

Vinod Koul Feb. 4, 2019, 7:27 a.m. UTC | #6

On 25-01-19, 05:54, Peng Ma wrote:
> Hi Vinod,
> 
> Sorry to replay late.
> 1:This patch has already send to the patchwork.
> 	Please see the patch link: https://patchwork.kernel.org/patch/10741521/
> 2:I have already compile the fsl patches on arm and powerpc after patched https://patchwork.kernel.org/patch/10741521/
> 	The compile will successful, please let me know the reported regression results, thanks very much.

And I thought there were further comments  on this patch. I have applied
this, please watch for failures reported if any..

Peng Ma Feb. 11, 2019, 2:33 a.m. UTC | #7

Hi Vinod,

Got it.

Best Regards
Peng

>-----Original Message-----
>From: Vinod Koul <[e-mail]>
>Sent: 2019年2月4日 15:28
>To: Peng Ma <[e-mail]>
>Cc: Scott Wood <[e-mail]>; Leo Li <[e-mail]>; Zhang Wei
><[e-mail]>; [e-mail];
>[e-mail]; Wen He <[e-mail]>
>Subject: Re: [PATCH] dmaengine: fsldma: Add 64-bit I/O accessors for
>powerpc64
>
>On 25-01-19, 05:54, Peng Ma wrote:
>> Hi Vinod,
>>
>> Sorry to replay late.
>> 1:This patch has already send to the patchwork.
>> 	Please see the patch link:
>> https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpat
>>
>chwork.kernel.org%2Fpatch%2F10741521%2F&amp;data=02%7C01%7Cpeng.
>ma%40n
>>
>xp.com%7C5d169e9bc5684a9d14c008d68a727589%7C686ea1d3bc2b4c6fa92
>cd99c5c
>>
>301635%7C0%7C0%7C636848621546833095&amp;sdata=q1sTaC%2F4SqEtM
>yFymzyvXe
>> qBOhCnBjyMcjPz8A9QRI4%3D&amp;reserved=0
>> 2:I have already compile the fsl patches on arm and powerpc after patched
>https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatch
>work.kernel.org%2Fpatch%2F10741521%2F&amp;data=02%7C01%7Cpeng.ma
>%40nxp.com%7C5d169e9bc5684a9d14c008d68a727589%7C686ea1d3bc2b4c
>6fa92cd99c5c301635%7C0%7C0%7C636848621546833095&amp;sdata=q1sTa
>C%2F4SqEtMyFymzyvXeqBOhCnBjyMcjPz8A9QRI4%3D&amp;reserved=0
>> 	The compile will successful, please let me know the reported regression
>results, thanks very much.
>
>And I thought there were further comments  on this patch. I have applied this,
>please watch for failures reported if any..
>--
>~Vinod

Patch

diff --git a/drivers/dma/fsldma.h b/drivers/dma/fsldma.h
index 88db939c04a1..a9b12f82b5c3 100644
--- a/drivers/dma/fsldma.h
+++ b/drivers/dma/fsldma.h
@@ -202,7 +202,12 @@  struct fsldma_chan {
 #define fsl_iowrite32(v, p)	out_le32(p, v)
 #define fsl_iowrite32be(v, p)	out_be32(p, v)
 
-#ifndef __powerpc64__
+#ifdef __powerpc64__
+#define fsl_ioread64(p)		in_le64(p)
+#define fsl_ioread64be(p)	in_be64(p)
+#define fsl_iowrite64(v, p)	out_le64(p, v)
+#define fsl_iowrite64be(v, p)	out_be64(p, v)
+#else
 static u64 fsl_ioread64(const u64 __iomem *addr)
 {
 	u32 fsl_addr = lower_32_bits(addr);