Re: [PATCH net-next v2] r8169: add support for RTL8125BP rev.b

[Heiner Kallweit <hkallweit1@gmail.com>]

On 20.12.2024 10:26, hau@realtek.com wrote:
> From: ChunHao Lin <hau@realtek.com>
> 
> Add support for RTL8125BP rev.b. Its XID is 0x689. This chip supports
> DASH and its dash type is "RTL_DASH_25_BP".
> 
> Signed-off-by: ChunHao Lin <hau@realtek.com>
> ---
> v2:
> - under rtl_hw_config(), add new entry for rtl8125bp
> ---
>  drivers/net/ethernet/realtek/r8169.h          |  1 +
>  drivers/net/ethernet/realtek/r8169_main.c     | 35 +++++++++++++++++++
>  .../net/ethernet/realtek/r8169_phy_config.c   | 23 ++++++++++++
>  3 files changed, 59 insertions(+)
> 
> diff --git a/drivers/net/ethernet/realtek/r8169.h b/drivers/net/ethernet/realtek/r8169.h
> index e0817f2a311a..7a194a8ab989 100644
> --- a/drivers/net/ethernet/realtek/r8169.h
> +++ b/drivers/net/ethernet/realtek/r8169.h
> @@ -70,6 +70,7 @@ enum mac_version {
>  	RTL_GIGA_MAC_VER_63,
>  	RTL_GIGA_MAC_VER_64,
>  	RTL_GIGA_MAC_VER_65,
> +	RTL_GIGA_MAC_VER_66,
>  	RTL_GIGA_MAC_VER_70,
>  	RTL_GIGA_MAC_VER_71,
>  	RTL_GIGA_MAC_NONE
> diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
> index 5724f650f9c6..425b1d7291b8 100644
> --- a/drivers/net/ethernet/realtek/r8169_main.c
> +++ b/drivers/net/ethernet/realtek/r8169_main.c
> @@ -58,6 +58,7 @@
>  #define FIRMWARE_8125B_2	"rtl_nic/rtl8125b-2.fw"
>  #define FIRMWARE_8125D_1	"rtl_nic/rtl8125d-1.fw"
>  #define FIRMWARE_8125D_2	"rtl_nic/rtl8125d-2.fw"
> +#define FIRMWARE_8125BP_2	"rtl_nic/rtl8125bp-2.fw"
>  #define FIRMWARE_8126A_2	"rtl_nic/rtl8126a-2.fw"
>  #define FIRMWARE_8126A_3	"rtl_nic/rtl8126a-3.fw"
>  
> @@ -142,6 +143,7 @@ static const struct {
>  	[RTL_GIGA_MAC_VER_63] = {"RTL8125B",		FIRMWARE_8125B_2},
>  	[RTL_GIGA_MAC_VER_64] = {"RTL8125D",		FIRMWARE_8125D_1},
>  	[RTL_GIGA_MAC_VER_65] = {"RTL8125D",		FIRMWARE_8125D_2},
> +	[RTL_GIGA_MAC_VER_66] = {"RTL8125BP",		FIRMWARE_8125BP_2},
>  	[RTL_GIGA_MAC_VER_70] = {"RTL8126A",		FIRMWARE_8126A_2},
>  	[RTL_GIGA_MAC_VER_71] = {"RTL8126A",		FIRMWARE_8126A_3},
>  };
> @@ -632,6 +634,7 @@ enum rtl_dash_type {
>  	RTL_DASH_NONE,
>  	RTL_DASH_DP,
>  	RTL_DASH_EP,
> +	RTL_DASH_25_BP,
>  };
>  
>  struct rtl8169_private {
> @@ -1361,10 +1364,19 @@ static void rtl8168ep_driver_start(struct rtl8169_private *tp)
>  		rtl_loop_wait_high(tp, &rtl_ep_ocp_read_cond, 10000, 30);
>  }
>  
> +static void rtl8125bp_driver_start(struct rtl8169_private *tp)
> +{
> +	r8168ep_ocp_write(tp, 0x01, 0x14, OOB_CMD_DRIVER_START);
> +	r8168ep_ocp_write(tp, 0x01, 0x18, 0x00);
> +	r8168ep_ocp_write(tp, 0x01, 0x10, 0x01);
> +}
> +
>  static void rtl8168_driver_start(struct rtl8169_private *tp)
>  {
>  	if (tp->dash_type == RTL_DASH_DP)
>  		rtl8168dp_driver_start(tp);
> +	else if (tp->dash_type == RTL_DASH_25_BP)
> +		rtl8125bp_driver_start(tp);
>  	else
>  		rtl8168ep_driver_start(tp);
>  }
> @@ -1385,10 +1397,19 @@ static void rtl8168ep_driver_stop(struct rtl8169_private *tp)
>  		rtl_loop_wait_low(tp, &rtl_ep_ocp_read_cond, 10000, 10);
>  }
>  
> +static void rtl8125bp_driver_stop(struct rtl8169_private *tp)
> +{
> +	r8168ep_ocp_write(tp, 0x01, 0x14, OOB_CMD_DRIVER_STOP);
> +	r8168ep_ocp_write(tp, 0x01, 0x18, 0x00);
> +	r8168ep_ocp_write(tp, 0x01, 0x10, 0x01);
> +}
> +
>  static void rtl8168_driver_stop(struct rtl8169_private *tp)
>  {
>  	if (tp->dash_type == RTL_DASH_DP)
>  		rtl8168dp_driver_stop(tp);
> +	else if (tp->dash_type == RTL_DASH_25_BP)
> +		rtl8125bp_driver_stop(tp);
>  	else
>  		rtl8168ep_driver_stop(tp);
>  }
> @@ -1411,6 +1432,7 @@ static bool rtl_dash_is_enabled(struct rtl8169_private *tp)
>  	case RTL_DASH_DP:
>  		return r8168dp_check_dash(tp);
>  	case RTL_DASH_EP:
> +	case RTL_DASH_25_BP:
>  		return r8168ep_check_dash(tp);
>  	default:
>  		return false;
> @@ -1425,6 +1447,8 @@ static enum rtl_dash_type rtl_get_dash_type(struct rtl8169_private *tp)
>  		return RTL_DASH_DP;
>  	case RTL_GIGA_MAC_VER_51 ... RTL_GIGA_MAC_VER_53:
>  		return RTL_DASH_EP;
> +	case RTL_GIGA_MAC_VER_66:
> +		return RTL_DASH_25_BP;
>  	default:
>  		return RTL_DASH_NONE;
>  	}
> @@ -2261,6 +2285,9 @@ static enum mac_version rtl8169_get_mac_version(u16 xid, bool gmii)
>  		{ 0x7cf, 0x64a,	RTL_GIGA_MAC_VER_71 },
>  		{ 0x7cf, 0x649,	RTL_GIGA_MAC_VER_70 },
>  
> +		/* 8125BP family. */
> +		{ 0x7cf, 0x681,	RTL_GIGA_MAC_VER_66 },
> +
>  		/* 8125D family. */
>  		{ 0x7cf, 0x689,	RTL_GIGA_MAC_VER_65 },
>  		{ 0x7cf, 0x688,	RTL_GIGA_MAC_VER_64 },
> @@ -3793,6 +3820,12 @@ static void rtl_hw_start_8125d(struct rtl8169_private *tp)
>  	rtl_hw_start_8125_common(tp);
>  }
>  
> +static void rtl_hw_start_8125bp(struct rtl8169_private *tp)
> +{
> +	rtl_set_def_aspm_entry_latency(tp);
> +	rtl_hw_start_8125_common(tp);
> +}
> +
>  static void rtl_hw_start_8126a(struct rtl8169_private *tp)
>  {
>  	rtl_set_def_aspm_entry_latency(tp);
> @@ -3842,6 +3875,7 @@ static void rtl_hw_config(struct rtl8169_private *tp)
>  		[RTL_GIGA_MAC_VER_63] = rtl_hw_start_8125b,
>  		[RTL_GIGA_MAC_VER_64] = rtl_hw_start_8125d,
>  		[RTL_GIGA_MAC_VER_65] = rtl_hw_start_8125d,
> +		[RTL_GIGA_MAC_VER_66] = rtl_hw_start_8125bp,
>  		[RTL_GIGA_MAC_VER_70] = rtl_hw_start_8126a,
>  		[RTL_GIGA_MAC_VER_71] = rtl_hw_start_8126a,
>  	};

If the config routine is the same as rtl_hw_start_8125d, then you can use
rtl_hw_start_8125d directly. This also makes clearer that RTL8125BP is derived
from RTL8125D (if that's the case, is it?).
By the way: Why is the new chip version with DASH, if derived from RTL8125D,
called RTL1825BP instead of e.g. RTL8125DP?

And the MODULE_FIRMWARE() entry is missing for the new firmware file.

Note: net-next is closed until end of the year, so you have to resubmit
beginning of January.