Re: [dpdk-dev] [DPDK v2] net/ipn3ke: modifications on AFU configurations

[Ferruh Yigit <ferruh.yigit@intel.com>]

On 5/30/2019 3:58 PM, Dan Wei wrote:
> Modify AFU configurations for new Blue Bitstream of A10 on N3000 card:
> - AFU register access: RTL changes the UPL base address and the read/write
> commands of register indirect access.
> - Add delays to wait for the HW reset completion.
> - Refine log for debug: print UPL_version not only for vBNG bit stream,
> but also for other bit streams
> 
> Fixes: c01c748e4ae6 ("net/ipn3ke: add new driver")
> Cc: rosen.xu@intel.com
> Cc: stable@dpdk.org
> 
> Signed-off-by: Dan Wei <dan.wei@intel.com>
> ---
>  drivers/net/ipn3ke/ipn3ke_ethdev.c | 14 ++++++++++++--
>  drivers/net/ipn3ke/ipn3ke_ethdev.h |  9 +++++----
>  drivers/net/ipn3ke/ipn3ke_flow.c   |  1 +
>  3 files changed, 18 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/net/ipn3ke/ipn3ke_ethdev.c b/drivers/net/ipn3ke/ipn3ke_ethdev.c
> index 9079b57..84eb0e9 100644
> --- a/drivers/net/ipn3ke/ipn3ke_ethdev.c
> +++ b/drivers/net/ipn3ke/ipn3ke_ethdev.c
> @@ -223,15 +223,25 @@
>  				"LineSideMACType", &mac_type);
>  	hw->retimer.mac_type = (int)mac_type;
>  
> +	/* After power on, wait until init done */
> +	while (IPN3KE_READ_REG(hw, IPN3KE_INIT_DONE) != 0x3)
> +		;

Isn't this dangerous? What do you think putting a limit to the loop?
And what do you think creating a define for 0x3?

<...>

> @@ -355,13 +354,13 @@ static inline uint32_t _ipn3ke_indrct_read(struct ipn3ke_hw *hw,
>  
>  	word_offset = (addr & 0x1FFFFFF) >> 2;
>  	indirect_value = RCMD | word_offset << 32;
> -	indirect_addrs = hw->hw_addr + (uint32_t)(UPL_BASE | 0x10);
> +	indirect_addrs = hw->hw_addr + (uint32_t)(0x30);

Can you create a macro for 0x30 hardcoded value? And same for below hardcoded
values.

<...>

> @@ -500,6 +500,7 @@ static inline void _ipn3ke_indrct_write(struct ipn3ke_hw *hw,
>  #define IPN3KE_CLF_RX_TEST                    (IPN3KE_CLASSIFY_OFFSET + 0x0400)
>  
>  #define IPN3KE_CLF_EM_VERSION       (IPN3KE_CLASSIFY_OFFSET + 0x40000 + 0x0000)
> +#define IPN3KE_CLF_EM_SCRATCH       (IPN3KE_CLASSIFY_OFFSET + 0x40000 + 0x0004)

As far as I can see this macro is not used in this patch, why adding it?

<...>