[dpdk-dev] [PATCH] i40e: Fix the endian issue for the i40e read&write registers functions

[dpdk-dev] [PATCH] i40e: Fix the endian issue for the i40e read&write registers functions

[Zhe Tao]

Linux kernel uses the writel and readl related functions to perform the PCI memory access via ioremap region,
those functions will handle the big little endian properly by doing the conversion between little and big endian if necessary,
so Add this conversion in the read&write registers functions for i40e.

Signed-off-by: Zhe Tao <zhe.tao@intel.com>
---
 drivers/net/i40e/base/i40e_osdep.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/i40e/base/i40e_osdep.h b/drivers/net/i40e/base/i40e_osdep.h
index 3ce8057..d35c494 100644
--- a/drivers/net/i40e/base/i40e_osdep.h
+++ b/drivers/net/i40e/base/i40e_osdep.h
@@ -122,10 +122,10 @@ do {                                                            \
 	((volatile uint32_t *)((char *)(a)->hw_addr + (reg)))
 static inline uint32_t i40e_read_addr(volatile void *addr)
 {
-	return I40E_PCI_REG(addr);
+	return rte_cpu_to_le_32(I40E_PCI_REG(addr));
 }
 #define I40E_PCI_REG_WRITE(reg, value) \
-	do {I40E_PCI_REG((reg)) = (value);} while(0)
+	do { I40E_PCI_REG((reg)) = rte_cpu_to_le_32(value); } while (0)
 
 #define I40E_WRITE_FLUSH(a) I40E_READ_REG(a, I40E_GLGEN_STAT)
 #define I40EVF_WRITE_FLUSH(a) I40E_READ_REG(a, I40E_VFGEN_RSTAT)
-- 
1.9.3

Re: [dpdk-dev] [PATCH] i40e: Fix the endian issue for the i40e read&write registers functions

[Thomas Monjalon]

2015-07-14 18:36, Zhe Tao:
> Linux kernel uses the writel and readl related functions to perform the PCI memory access via ioremap region,
> those functions will handle the big little endian properly by doing the conversion between little and big endian if necessary,
> so Add this conversion in the read&write registers functions for i40e.

You compare with the Linux implementation but you forget to describe
the problem which would be a better justification.

Re: [dpdk-dev] [PATCH] i40e: Fix the endian issue for the i40e read&write registers functions

[Wu, Jingjing]

> -----Original Message-----
> From: Tao, Zhe
> Sent: Tuesday, July 14, 2015 6:37 PM
> To: dev@dpdk.org
> Cc: Tao, Zhe; Wu, Jingjing
> Subject: [PATCH] i40e: Fix the endian issue for the i40e read&write registers
> functions
> 
> Linux kernel uses the writel and readl related functions to perform the PCI
> memory access via ioremap region, those functions will handle the big little
> endian properly by doing the conversion between little and big endian if
> necessary, so Add this conversion in the read&write registers functions for
> i40e.
> 
> Signed-off-by: Zhe Tao <zhe.tao@intel.com>
> ---
>  drivers/net/i40e/base/i40e_osdep.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/i40e/base/i40e_osdep.h
> b/drivers/net/i40e/base/i40e_osdep.h
> index 3ce8057..d35c494 100644
> --- a/drivers/net/i40e/base/i40e_osdep.h
> +++ b/drivers/net/i40e/base/i40e_osdep.h
> @@ -122,10 +122,10 @@ do {                                                            \
>  	((volatile uint32_t *)((char *)(a)->hw_addr + (reg)))  static inline
> uint32_t i40e_read_addr(volatile void *addr)  {
> -	return I40E_PCI_REG(addr);
> +	return rte_cpu_to_le_32(I40E_PCI_REG(addr));

I think we need use rte_le_to_cpu_32 but not rte_cpu_to_le_32 for reading.

>  }
>  #define I40E_PCI_REG_WRITE(reg, value) \
> -	do {I40E_PCI_REG((reg)) = (value);} while(0)
> +	do { I40E_PCI_REG((reg)) = rte_cpu_to_le_32(value); } while (0)
> 
>  #define I40E_WRITE_FLUSH(a) I40E_READ_REG(a, I40E_GLGEN_STAT)
> #define I40EVF_WRITE_FLUSH(a) I40E_READ_REG(a, I40E_VFGEN_RSTAT)
> --
> 1.9.3

[dpdk-dev] [PATCH v2] Fix the endian issue for the i40e read&write registers functions

[Zhe Tao]

When using the Power big endian CPU for i40e NIC,
the current i40e related registers operations will cause a problem,
because the i40e registers are little endian which is inconsistent with
big endian CPU. Add the conversion for the inconsistency.
 
Signed-off-by: Zhe Tao <zhe.tao@intel.com>
---
PATCH v2: Edit the comments make it more clear

PATCH v1: Add the endian conversion for registers operations.

 drivers/net/i40e/base/i40e_osdep.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/i40e/base/i40e_osdep.h b/drivers/net/i40e/base/i40e_osdep.h
index 3ce8057..70d2721 100644
--- a/drivers/net/i40e/base/i40e_osdep.h
+++ b/drivers/net/i40e/base/i40e_osdep.h
@@ -122,10 +122,10 @@ do {                                                            \
 	((volatile uint32_t *)((char *)(a)->hw_addr + (reg)))
 static inline uint32_t i40e_read_addr(volatile void *addr)
 {
-	return I40E_PCI_REG(addr);
+	return rte_le_to_cpu_32(I40E_PCI_REG(addr));
 }
 #define I40E_PCI_REG_WRITE(reg, value) \
-	do {I40E_PCI_REG((reg)) = (value);} while(0)
+	do { I40E_PCI_REG((reg)) = rte_cpu_to_le_32(value); } while (0)
 
 #define I40E_WRITE_FLUSH(a) I40E_READ_REG(a, I40E_GLGEN_STAT)
 #define I40EVF_WRITE_FLUSH(a) I40E_READ_REG(a, I40E_VFGEN_RSTAT)
-- 
1.9.3

[dpdk-dev] [PATCH v3] i40e: Fix the endian issue for the i40e read&write registers functions

[Zhe Tao]

Signed-off-by: Zhe Tao <zhe.tao@intel.com>
---
PATCH v3: Edit the subject make it more clear

PATCH v2: Edit the comments make it more clear

PATCH v1: Add the endian conversion for registers operations.

 drivers/net/i40e/base/i40e_osdep.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/i40e/base/i40e_osdep.h b/drivers/net/i40e/base/i40e_osdep.h
index 3ce8057..70d2721 100644
--- a/drivers/net/i40e/base/i40e_osdep.h
+++ b/drivers/net/i40e/base/i40e_osdep.h
@@ -122,10 +122,10 @@ do {                                                            \
 	((volatile uint32_t *)((char *)(a)->hw_addr + (reg)))
 static inline uint32_t i40e_read_addr(volatile void *addr)
 {
-	return I40E_PCI_REG(addr);
+	return rte_le_to_cpu_32(I40E_PCI_REG(addr));
 }
 #define I40E_PCI_REG_WRITE(reg, value) \
-	do {I40E_PCI_REG((reg)) = (value);} while(0)
+	do { I40E_PCI_REG((reg)) = rte_cpu_to_le_32(value); } while (0)
 
 #define I40E_WRITE_FLUSH(a) I40E_READ_REG(a, I40E_GLGEN_STAT)
 #define I40EVF_WRITE_FLUSH(a) I40E_READ_REG(a, I40E_VFGEN_RSTAT)
-- 
1.9.3

Re: [dpdk-dev] [PATCH v3] i40e: Fix the endian issue for the i40e read&write registers functions

[Chao Zhu]

Acked-by: Chao Zhu <chaozhu@linux.vnet.ibm.com>

On 2015/7/17 15:25, Zhe Tao wrote:
> Signed-off-by: Zhe Tao <zhe.tao@intel.com>
> ---
> PATCH v3: Edit the subject make it more clear
>
> PATCH v2: Edit the comments make it more clear
>
> PATCH v1: Add the endian conversion for registers operations.
>
>   drivers/net/i40e/base/i40e_osdep.h | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/i40e/base/i40e_osdep.h b/drivers/net/i40e/base/i40e_osdep.h
> index 3ce8057..70d2721 100644
> --- a/drivers/net/i40e/base/i40e_osdep.h
> +++ b/drivers/net/i40e/base/i40e_osdep.h
> @@ -122,10 +122,10 @@ do {                                                            \
>   	((volatile uint32_t *)((char *)(a)->hw_addr + (reg)))
>   static inline uint32_t i40e_read_addr(volatile void *addr)
>   {
> -	return I40E_PCI_REG(addr);
> +	return rte_le_to_cpu_32(I40E_PCI_REG(addr));
>   }
>   #define I40E_PCI_REG_WRITE(reg, value) \
> -	do {I40E_PCI_REG((reg)) = (value);} while(0)
> +	do { I40E_PCI_REG((reg)) = rte_cpu_to_le_32(value); } while (0)
>
>   #define I40E_WRITE_FLUSH(a) I40E_READ_REG(a, I40E_GLGEN_STAT)
>   #define I40EVF_WRITE_FLUSH(a) I40E_READ_REG(a, I40E_VFGEN_RSTAT)

Re: [dpdk-dev] [PATCH v3] i40e: Fix the endian issue for the i40e read&write registers functions

[Thomas Monjalon]

There was no explanation.
The changed title should be more explicit:
"i40e: fix registers access from big endian CPU"

> > Signed-off-by: Zhe Tao <zhe.tao@intel.com>
> Acked-by: Chao Zhu <chaozhu@linux.vnet.ibm.com>

Applied, thanks