Re: [PATCH v2 net 2/4] net: aquantia: when cleaning hw cache it should be toggled

[Igor Russkikh <Igor.Russkikh@aquantia.com>]

Hello Jakub,

>> workaround when stopping the device), register bit should actually
>> be toggled.
> 
> Does the bit get set by the driver or HW?
> 
> If it gets set by HW there is still a tiny race from reading to
> writing.. Perhaps doing two writes -> to 0 and to 1 would be a better
> option?  

No, set is done by the driver, not HW. HW just tracks for the toggle.

>> It was previosly always just set. Due to the way driver stops HW this
>> never actually caused any issues, but it still may, so cleaning this up.
> 
> Hm. So is it a cleanup of fix? Does the way code is written guarantee
> it will never cause issues?

Yes, thats a cleanup. We just had other products where this cache reset had to
be done multiple times. Obviously doing that second time was just no-op for
hardware ;)
On linux this always gets called on deinit only once - thus it was safe.
We just aligning here the linux driver with actual HW specification.

>> +	if (err)
>> +		goto err_exit;
>> +
>> +	readx_poll_timeout_atomic(hw_atl_rdm_rx_dma_desc_cache_init_done_get,
>> +				  self, val, val == 1, 1000U, 10000U);
> 
> It's a little strange to toggle, yet wait for it to be of a specific
> value..

Notice thats a different value - 'cache_init_done' bit.
This is used by HW to indicate completion of cache reset operation.

>> +err_exit:
>> +	return err;
> 
> Just return err instead of doing this pointless goto. It make the code
> harder to follow.

Sure

>> +#define RDM_RX_DMA_DESC_CACHE_INIT_DONE_DEFAULT 0x0
>> +
>> +
> 
> two empty lines here?

Will fix.

Regards,
  Igor