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

login
register
mail settings
Submitter Peng Ma
Date Dec. 24, 2018, 3:42 a.m.
Message ID <VI1PR04MB443154AEE42853B62234075EEDBB0@VI1PR04MB4431.eurprd04.prod.outlook.com>
Download mbox | patch
Permalink /patch/689309/
State New
Headers show

Comments

Peng Ma - Dec. 24, 2018, 3:42 a.m.
Hi Scott,

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

Peng
>-----Original Message-----

>From: Scott Wood <oss@buserror.net>

>Sent: 2018年12月22日 12:35

>To: Leo Li <leoyang.li@nxp.com>; Zhang Wei <zw@zh-kernel.org>

>Cc: linuxppc-dev@lists.ozlabs.org; dmaengine@vger.kernel.org; Peng Ma

><peng.ma@nxp.com>; Wen He <wen.he_1@nxp.com>

>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 <peng.ma@nxp.com>

>Cc: Wen He <wen.he_1@nxp.com>

>Fixes: 68997fff94afa (" dmaengine: fsldma: Adding macro FSL_DMA_IN/OUT

>implement for ARM platform")

>Signed-off-by: Scott Wood <oss@buserror.net>

>---

>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
Scott Wood - Dec. 24, 2018, 4:45 a.m.
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.
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 <oss@buserror.net>

>Sent: 2018年12月24日 12:46

>To: Peng Ma <peng.ma@nxp.com>; Leo Li <leoyang.li@nxp.com>; Zhang Wei

><zw@zh-kernel.org>

>Cc: linuxppc-dev@lists.ozlabs.org; dmaengine@vger.kernel.org; Wen He

><wen.he_1@nxp.com>

>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.
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 <oss@buserror.net>
> >Sent: 2018年12月24日 12:46
> >To: Peng Ma <peng.ma@nxp.com>; Leo Li <leoyang.li@nxp.com>; Zhang Wei
> ><zw@zh-kernel.org>
> >Cc: linuxppc-dev@lists.ozlabs.org; dmaengine@vger.kernel.org; Wen He
> ><wen.he_1@nxp.com>
> >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.
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 <vkoul@kernel.org>

>Sent: 2019年1月19日 20:59

>To: Peng Ma <peng.ma@nxp.com>

>Cc: Scott Wood <oss@buserror.net>; Leo Li <leoyang.li@nxp.com>; Zhang Wei

><zw@zh-kernel.org>; linuxppc-dev@lists.ozlabs.org;

>dmaengine@vger.kernel.org; Wen He <wen.he_1@nxp.com>

>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 <oss@buserror.net>

>> >Sent: 2018年12月24日 12:46

>> >To: Peng Ma <peng.ma@nxp.com>; Leo Li <leoyang.li@nxp.com>; Zhang

>Wei

>> ><zw@zh-kernel.org>

>> >Cc: linuxppc-dev@lists.ozlabs.org; dmaengine@vger.kernel.org; Wen He

>> ><wen.he_1@nxp.com>

>> >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.
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.
Hi Vinod,

Got it.

Best Regards
Peng

>-----Original Message-----

>From: Vinod Koul <vkoul@kernel.org>

>Sent: 2019年2月4日 15:28

>To: Peng Ma <peng.ma@nxp.com>

>Cc: Scott Wood <oss@buserror.net>; Leo Li <leoyang.li@nxp.com>; Zhang Wei

><zw@zh-kernel.org>; linuxppc-dev@lists.ozlabs.org;

>dmaengine@vger.kernel.org; Wen He <wen.he_1@nxp.com>

>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 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,