On 1/19/2022 1:52 AM, Ilpo Järvinen wrote:
On Tue, 18 Jan 2022, Martinez, Ricardo wrote:
On 1/18/2022 6:13 AM, Ilpo Järvinen wrote:
On Thu, 13 Jan 2022, Ricardo Martinez wrote:
...
+static bool t7xx_cldma_qs_are_active(struct t7xx_cldma_hw *hw_info)
+{
+ unsigned int tx_active;
+ unsigned int rx_active;
+
+ tx_active = t7xx_cldma_hw_queue_status(hw_info, CLDMA_ALL_Q, MTK_TX);
+ rx_active = t7xx_cldma_hw_queue_status(hw_info, CLDMA_ALL_Q, MTK_RX);
+ if (tx_active == CLDMA_INVALID_STATUS || rx_active ==
CLDMA_INVALID_STATUS)
These cannot ever be true because of mask in t7xx_cldma_hw_queue_status().
t7xx_cldma_hw_queue_status() shouldn't apply the mask for CLDMA_ALL_Q.
I guess it shouldn't but it currently does apply 0xff (CLDMA_ALL_Q) as
mask in that case. However, this now raises another question, if
0xffffffff (CLDMA_INVALID_STATUS) means status is invalid, should all
callers both single Q and CLDMA_ALL_Q be returned/check/handle that value?
Why would CLDMA_ALL_Q be special in this respect that the INVALID_STATUS
means invalid only with it?
Reading 0xffffffff is used to detect if the PCI link was disconnected,
it is relevant in t7xx_cldma_qs_are_active() because it is a helper function
polled by t7xx_cldma_stop() to wait until the queues are not active anymore.
I think a cleaner implementation would be to use pci_device_is_present()
instead of the CLDMA_INVALID_STATUS check inside t7xx_cldma_qs_are_active()
and keep t7xx_cldma_hw_queue_status() free of that logic.
...
