Re: [PATCH net-next v6 01/10] r8152: split r8156_init per chip and factor out wait_autoload_done

Hi Chih Kai,

On 08/10/2026 10:15 am, Chih Kai Hsu wrote:

RTL8156, RTL8156B, RTL8157 and RTL8159 shared a single r8156_init(),
gated by a dense set of tp->version range checks. That made the init
sequence hard to follow.

Split r8156_init() into one function per chip family:
- r8156_init  (VER_10/11)
- r8156b_init (VER_12/13/15)
- r8157_init  (VER_16)
- r8159_init  (VER_17)

I have a hard time to understand why you want to separate out one init function into 4 different ones, for each chip one. Especially, if, as you say no functional changes are intended, which might justify the re-factoring in this manner. By multiplying the functions, it will become more difficult to maintain the code: if an issue is found in the init-sequence, it may now be necessary to fix it 4-times and future developments touching the init-sequence will also need to be replicated multiple times. In total, there would afterwards be 8(!) different init
sequences including the other ones for the RTL8152 and different versions of the RTL8153. The same pattern repeats with _up(), _down() and other configuration functions.

Would it not make more sense to try to further combine the init sequences or try to disentangle the code by establishing common library routines put into a _lib.c file and possibly different .c files for the different generations of the chips? For example, it appears that the rtl8157/9 belong to one generation of chips, based on the rtl8156 design. On the other hand, rtl8152/3 appear to be a different generation with various family members. That would allow to build a basis for future development like to rely on using phylink to handle the PHYs integrated in the chips, which all appear to be already supported by the Realtek-PHY driver.

[...]

No functional change intended.

Signed-off-by: Chih Kai Hsu <redacted>

Best,
  Birger