[dpdk-dev,v3,18/29] net/cxgbe: use eal I/O device memory read/write API
Checks
Commit Message
From: Santosh Shukla <santosh.shukla@caviumnetworks.com>
Replace the raw I/O device memory read/write access with eal
abstraction for I/O device memory read/write access to fix
portability issues across different architectures.
CC: Rahul Lakkireddy <rahul.lakkireddy@chelsio.com>
Signed-off-by: Santosh Shukla <santosh.shukla@caviumnetworks.com>
Signed-off-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>
---
drivers/net/cxgbe/base/adapter.h | 34 ++++++++++++++++++++++++++++------
drivers/net/cxgbe/cxgbe_compat.h | 8 +++++++-
drivers/net/cxgbe/sge.c | 10 +++++-----
3 files changed, 40 insertions(+), 12 deletions(-)
Comments
On 1/12/2017 9:17 AM, Jerin Jacob wrote:
<...>
>
> -#define CXGBE_PCI_REG_WRITE(reg, value) ({ \
> - CXGBE_PCI_REG((reg)) = (value); })
> +#define CXGBE_PCI_REG_WRITE(reg, value) rte_write32((value), (reg))
Almost all (if not all) PMD write macros' argument order is like
write(address, value), but rte_writeX has rte_writex(value, address)
What is the common usage for this kind of function?
What do you think reverting argument order?
As a similar example, dpdk ether_addr_copy(src, dst) function argument
order is revers according common usage, and keeps confusing people.
<...>
On Thu, Jan 12, 2017 at 07:12:33PM +0000, Ferruh Yigit wrote:
> On 1/12/2017 9:17 AM, Jerin Jacob wrote:
> <...>
>
> >
> > -#define CXGBE_PCI_REG_WRITE(reg, value) ({ \
> > - CXGBE_PCI_REG((reg)) = (value); })
> > +#define CXGBE_PCI_REG_WRITE(reg, value) rte_write32((value), (reg))
>
> Almost all (if not all) PMD write macros' argument order is like
> write(address, value), but rte_writeX has rte_writex(value, address)
>
> What is the common usage for this kind of function?
Arguments order has been taken from Linux kernel readl/writel syntax.
> What do you think reverting argument order?
OMG :-). If it worth it then we can consider. IMHO, let it be in kernel
syntax so that it will easy to port drivers from Linux kernel.
>
> As a similar example, dpdk ether_addr_copy(src, dst) function argument
> order is revers according common usage, and keeps confusing people.
>
> <...>
>
On 1/13/2017 8:37 AM, Jerin Jacob wrote:
> On Thu, Jan 12, 2017 at 07:12:33PM +0000, Ferruh Yigit wrote:
>> On 1/12/2017 9:17 AM, Jerin Jacob wrote:
>> <...>
>>
>>>
>>> -#define CXGBE_PCI_REG_WRITE(reg, value) ({ \
>>> - CXGBE_PCI_REG((reg)) = (value); })
>>> +#define CXGBE_PCI_REG_WRITE(reg, value) rte_write32((value), (reg))
>>
>> Almost all (if not all) PMD write macros' argument order is like
>> write(address, value), but rte_writeX has rte_writex(value, address)
>>
>> What is the common usage for this kind of function?
>
> Arguments order has been taken from Linux kernel readl/writel syntax.
>
>> What do you think reverting argument order?
>
> OMG :-). If it worth it then we can consider. IMHO, let it be in kernel
> syntax so that it will easy to port drivers from Linux kernel.
Fair enough.
>
>>
>> As a similar example, dpdk ether_addr_copy(src, dst) function argument
>> order is revers according common usage, and keeps confusing people.
>>
>> <...>
>>
@@ -37,6 +37,7 @@
#define __T4_ADAPTER_H__
#include <rte_mbuf.h>
+#include <rte_io.h>
#include "cxgbe_compat.h"
#include "t4_regs_values.h"
@@ -324,7 +325,7 @@ struct adapter {
int use_unpacked_mode; /* unpacked rx mode state */
};
-#define CXGBE_PCI_REG(reg) (*((volatile uint32_t *)(reg)))
+#define CXGBE_PCI_REG(reg) rte_read32(reg)
static inline uint64_t cxgbe_read_addr64(volatile void *addr)
{
@@ -350,16 +351,21 @@ static inline uint32_t cxgbe_read_addr(volatile void *addr)
#define CXGBE_READ_REG64(adap, reg) \
cxgbe_read_addr64(CXGBE_PCI_REG_ADDR((adap), (reg)))
-#define CXGBE_PCI_REG_WRITE(reg, value) ({ \
- CXGBE_PCI_REG((reg)) = (value); })
+#define CXGBE_PCI_REG_WRITE(reg, value) rte_write32((value), (reg))
+
+#define CXGBE_PCI_REG_WRITE_RELAXED(reg, value) \
+ rte_write32_relaxed((value), (reg))
#define CXGBE_WRITE_REG(adap, reg, value) \
CXGBE_PCI_REG_WRITE(CXGBE_PCI_REG_ADDR((adap), (reg)), (value))
+#define CXGBE_WRITE_REG_RELAXED(adap, reg, value) \
+ CXGBE_PCI_REG_WRITE_RELAXED(CXGBE_PCI_REG_ADDR((adap), (reg)), (value))
+
static inline uint64_t cxgbe_write_addr64(volatile void *addr, uint64_t val)
{
- CXGBE_PCI_REG(addr) = val;
- CXGBE_PCI_REG(((volatile uint8_t *)(addr) + 4)) = (val >> 32);
+ CXGBE_PCI_REG_WRITE(addr, val);
+ CXGBE_PCI_REG_WRITE(((volatile uint8_t *)(addr) + 4), (val >> 32));
return val;
}
@@ -383,7 +389,7 @@ static inline u32 t4_read_reg(struct adapter *adapter, u32 reg_addr)
}
/**
- * t4_write_reg - write a HW register
+ * t4_write_reg - write a HW register with barrier
* @adapter: the adapter
* @reg_addr: the register address
* @val: the value to write
@@ -398,6 +404,22 @@ static inline void t4_write_reg(struct adapter *adapter, u32 reg_addr, u32 val)
}
/**
+ * t4_write_reg_relaxed - write a HW register with no barrier
+ * @adapter: the adapter
+ * @reg_addr: the register address
+ * @val: the value to write
+ *
+ * Write a 32-bit value into the given HW register.
+ */
+static inline void t4_write_reg_relaxed(struct adapter *adapter, u32 reg_addr,
+ u32 val)
+{
+ CXGBE_DEBUG_REG(adapter, "setting register 0x%x to 0x%x\n", reg_addr,
+ val);
+ CXGBE_WRITE_REG_RELAXED(adapter, reg_addr, val);
+}
+
+/**
* t4_read_reg64 - read a 64-bit HW register
* @adapter: the adapter
* @reg_addr: the register address
@@ -45,6 +45,7 @@
#include <rte_cycles.h>
#include <rte_spinlock.h>
#include <rte_log.h>
+#include <rte_io.h>
#define dev_printf(level, fmt, args...) \
RTE_LOG(level, PMD, "rte_cxgbe_pmd: " fmt, ## args)
@@ -254,7 +255,7 @@ static inline unsigned long ilog2(unsigned long n)
static inline void writel(unsigned int val, volatile void __iomem *addr)
{
- *(volatile unsigned int *)addr = val;
+ rte_write32(val, addr);
}
static inline void writeq(u64 val, volatile void __iomem *addr)
@@ -263,4 +264,9 @@ static inline void writeq(u64 val, volatile void __iomem *addr)
writel(val >> 32, (void *)((uintptr_t)addr + 4));
}
+static inline void writel_relaxed(unsigned int val, volatile void __iomem *addr)
+{
+ rte_write32_relaxed(val, addr);
+}
+
#endif /* _CXGBE_COMPAT_H_ */
@@ -338,12 +338,12 @@ static inline void ring_fl_db(struct adapter *adap, struct sge_fl *q)
* mechanism.
*/
if (unlikely(!q->bar2_addr)) {
- t4_write_reg(adap, MYPF_REG(A_SGE_PF_KDOORBELL),
- val | V_QID(q->cntxt_id));
+ t4_write_reg_relaxed(adap, MYPF_REG(A_SGE_PF_KDOORBELL),
+ val | V_QID(q->cntxt_id));
} else {
- writel(val | V_QID(q->bar2_qid),
- (void *)((uintptr_t)q->bar2_addr +
- SGE_UDB_KDOORBELL));
+ writel_relaxed(val | V_QID(q->bar2_qid),
+ (void *)((uintptr_t)q->bar2_addr +
+ SGE_UDB_KDOORBELL));
/*
* This Write memory Barrier will force the write to