> -----Original Message-----
> From: Burakov, Anatoly <anatoly.bura...@intel.com>
> Sent: Monday, October 12, 2020 17:29
> To: Wang, Haiyue <haiyue.w...@intel.com>; dev@dpdk.org
> Cc: Ma, Liang J <liang.j...@intel.com>; Guo, Jia <jia....@intel.com>; Hunt,
> David
> <david.h...@intel.com>; Ananyev, Konstantin <konstantin.anan...@intel.com>;
> jerinjac...@gmail.com;
> Richardson, Bruce <bruce.richard...@intel.com>; tho...@monjalon.net;
> McDaniel, Timothy
> <timothy.mcdan...@intel.com>; Eads, Gage <gage.e...@intel.com>; Macnamara,
> Chris
> <chris.macnam...@intel.com>
> Subject: Re: [PATCH v5 06/10] net/ixgbe: implement power management API
>
> On 12-Oct-20 9:09 AM, Wang, Haiyue wrote:
> > Hi Liang,
> >
> >> -----Original Message-----
> >> From: Burakov, Anatoly <anatoly.bura...@intel.com>
> >> Sent: Saturday, October 10, 2020 00:02
> >> To: dev@dpdk.org
> >> Cc: Ma, Liang J <liang.j...@intel.com>; Guo, Jia <jia....@intel.com>;
> >> Wang, Haiyue
> >> <haiyue.w...@intel.com>; Hunt, David <david.h...@intel.com>; Ananyev,
> >> Konstantin
> >> <konstantin.anan...@intel.com>; jerinjac...@gmail.com; Richardson, Bruce
> <bruce.richard...@intel.com>;
> >> tho...@monjalon.net; McDaniel, Timothy <timothy.mcdan...@intel.com>; Eads,
> >> Gage
> <gage.e...@intel.com>;
> >> Macnamara, Chris <chris.macnam...@intel.com>
> >> Subject: [PATCH v5 06/10] net/ixgbe: implement power management API
> >>
> >> From: Liang Ma <liang.j...@intel.com>
> >>
> >> Implement support for the power management API by implementing a
> >> `get_wake_addr` function that will return an address of an RX ring's
> >> status bit.
> >>
> >> Signed-off-by: Anatoly Burakov <anatoly.bura...@intel.com>
> >> Signed-off-by: Liang Ma <liang.j...@intel.com>
> >> ---
> >> drivers/net/ixgbe/ixgbe_ethdev.c | 1 +
> >> drivers/net/ixgbe/ixgbe_rxtx.c | 22 ++++++++++++++++++++++
> >> drivers/net/ixgbe/ixgbe_rxtx.h | 2 ++
> >> 3 files changed, 25 insertions(+)
> >>
> >
> >
> >>
> >> +int ixgbe_get_wake_addr(void *rx_queue, volatile void **tail_desc_addr,
> >> +uint64_t *expected, uint64_t *mask)
> >> +{
> >> +volatile union ixgbe_adv_rx_desc *rxdp;
> >> +struct ixgbe_rx_queue *rxq = rx_queue;
> >> +uint16_t desc;
> >> +
> >> +desc = rxq->rx_tail;
> >> +rxdp = &rxq->rx_ring[desc];
> >> +/* watch for changes in status bit */
> >> +*tail_desc_addr = &rxdp->wb.upper.status_error;
> >> +
> >> +/*
> >> + * we expect the DD bit to be set to 1 if this descriptor was already
> >> + * written to.
> >> + */
> >> +*expected = rte_cpu_to_le_32(IXGBE_RXDADV_STAT_DD);
> >> +*mask = rte_cpu_to_le_32(IXGBE_RXDADV_STAT_DD);
> >> +
> >
> > Seems have one issue about the byte endian:
> > Like for BIG endian:
> > *expected = rte_bswap32(IXGBE_RXDADV_STAT_DD)
> > !=
> > *expected = rte_bswap64(IXGBE_RXDADV_STAT_DD)
> >
> > And in API 'rte_power_monitor', use uint64_t type to access the wake up
> > data:
> >
> > static inline void rte_power_monitor(const volatile void *p,
> > const uint64_t expected_value, const uint64_t value_mask,
> > const uint64_t tsc_timestamp)
> > {
> > if (value_mask) {
> > const uint64_t cur_value = *(const volatile uint64_t *)p;
> > const uint64_t masked = cur_value & value_mask;
> > /* if the masked value is already matching, abort */
> > if (masked == expected_value)
> > return;
> > }
> >
> >
> > So that we need the wake up address type like 16/32/64b ?
>
> Endian differences strike again! You're right of course.
>
> I suspect casting everything to CPU endinanness would fix it, would it not?
But need the same date type, if swap is needed for casting, then
(u64 a = rte_bswap32(1)) != (u64 b = rte_bswap64(1))
>
> >
> >> --
> >> 2.17.1
>
>
> --
> Thanks,
> Anatoly
