Re: [PATCH net-next v6 4/7] stmmac: intel: configure SerDes according to the interface mode

From: Choong Yong Liang
Date: Wed Feb 05 2025 - 21:21:10 EST




On 4/2/2025 4:25 pm, Ilpo Järvinen wrote:
On Tue, 4 Feb 2025, Choong Yong Liang wrote:

The logic could be reversed + return immediately to reduce the indentation
of the block below.

If you just want to have them initialized, it's enough to use {}, no dummy
0 is necessary.

This looks somewhat ugly. Perhaps it would be better if you make the call
on main level of the function and use local variables to hold the regs
array and its number of elements until then.

It would be even better if you could just store the pointer and # of
elements into some platform info structure so that it wouldn't need to be
calculated on the fly here (but I don't know this driver well enough to
know if that's viable/easy to do).

Why are these arrays in a header and not in the C file that uses them???


Hi Ilpo,

Thank you for your detailed review and comments on the patch.

I'll reverse the logic in intel_tsn_lane_is_available, define the magic number, initialize arrays with {}, refactor intel_config_serdes, and move the arrays to the C file.