On Wed, Jan 26, 2022 at 12:14:55AM +0100, Ansuel Smith wrote:

quoted

At some point, you'll have to do something about those sequence numbers.
Hardcoding 200 and 400 isn't going to get you very far, it's prone to
errors. How about dealing with it now? If treating them as actual
sequence numbers isn't useful because you can't have multiple packets in
flight due to reordering concerns, at least create a macro for each
sequence number used by the driver for packet identification.

Is documenting define and adding some inline function acceptable? That
should make the separation more clear and also prepare for a future
implementation. The way I see an use for the seq number is something
like a global workqueue that would handle all this stuff and be the one
that handle the seq number.
I mean another way would be just use a counter that will overflow and
remove all this garbage with hardcoded seq number.
(think will follow this path and just implement a correct seq number...)

Cleanest would be, I think, to just treat the sequence number as a
rolling counter and use it to match the request to the response.
But I didn't object to your use of fixed numbers per packet type, just
to the lack of a #define behind those numbers.

quoted

quoted

+	mutex_lock(&phy_hdr_data->mutex);

Shouldn't qca8k_master_change() also take phy_hdr_data->mutex?

Is actually the normal mgmg_hdr_data. 

phy_hdr_data = &priv->mgmt_hdr_data;

Should I remove this and use mgmt_hdr_data directly to remove any
confusion? 

I am not thrilled by the naming of this data structure anyway
(why "hdr"?), but yes, I also got tricked by inconsistent naming.
Please choose a consistent name and stick with it.