On 09.11.2023 12:49, Paolo Abeni wrote:
> On Thu, 2023-11-09 at 02:48 +0800, ChunHao Lin wrote:
>> For devices that support DASH, even DASH is disabled, there may still
>> exist a default firmware that will influence device behavior.
>> So driver needs to handle DASH for devices that support DASH, no
>> matter the DASH status is.
>>
>> This patch also prepare for "fix DASH deviceis network lost issue".
>>
>> Signed-off-by: ChunHao Lin <hau@xxxxxxxxxxx>
>
> You should include the fixes tag you already added in v1 and your Sob
> should come as the last tag
>
> The same applies to the next patch
>
>> Reviewed-by: Heiner Kallweit <hkallweit1@xxxxxxxxx>
>
> It's not clear where/when Heiner provided the above tag for this patch.
> I hope that was off-list.
>
Right, so far I added my Rb for patch 2 only. Will have a look at patch 1
again once there's a version with your review comments having been addressed.
>> Cc: stable@xxxxxxxxxxxxxxx
>> ---
>> drivers/net/ethernet/realtek/r8169_main.c | 35 ++++++++++++++++-------
>> 1 file changed, 25 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
>> index 0c76c162b8a9..108dc75050ba 100644
>> --- a/drivers/net/ethernet/realtek/r8169_main.c
>> +++ b/drivers/net/ethernet/realtek/r8169_main.c
>> @@ -624,6 +624,7 @@ struct rtl8169_private {
>>
>> unsigned supports_gmii:1;
>> unsigned aspm_manageable:1;
>> + unsigned dash_enabled:1;
>> dma_addr_t counters_phys_addr;
>> struct rtl8169_counters *counters;
>> struct rtl8169_tc_offsets tc_offset;
>> @@ -1253,14 +1254,26 @@ static bool r8168ep_check_dash(struct rtl8169_private *tp)
>> return r8168ep_ocp_read(tp, 0x128) & BIT(0);
>> }
>>
>> -static enum rtl_dash_type rtl_check_dash(struct rtl8169_private *tp)
>> +static bool rtl_dash_is_enabled(struct rtl8169_private *tp)
>> +{
>> + switch (tp->dash_type) {
>> + case RTL_DASH_DP:
>> + return r8168dp_check_dash(tp);
>> + case RTL_DASH_EP:
>> + return r8168ep_check_dash(tp);
>> + default:
>> + return false;
>> + }
>> +}
>> +
>> +static enum rtl_dash_type rtl_get_dash_type(struct rtl8169_private *tp)
>> {
>> switch (tp->mac_version) {
>> case RTL_GIGA_MAC_VER_28:
>> case RTL_GIGA_MAC_VER_31:
>> - return r8168dp_check_dash(tp) ? RTL_DASH_DP : RTL_DASH_NONE;
>> + return RTL_DASH_DP;
>> case RTL_GIGA_MAC_VER_51 ... RTL_GIGA_MAC_VER_53:
>> - return r8168ep_check_dash(tp) ? RTL_DASH_EP : RTL_DASH_NONE;
>> + return RTL_DASH_EP;
>> default:
>> return RTL_DASH_NONE;
>> }
>> @@ -1453,7 +1466,7 @@ static void __rtl8169_set_wol(struct rtl8169_private *tp, u32 wolopts)
>>
>> device_set_wakeup_enable(tp_to_dev(tp), wolopts);
>>
>> - if (tp->dash_type == RTL_DASH_NONE) {
>> + if (!tp->dash_enabled) {
>> rtl_set_d3_pll_down(tp, !wolopts);
>> tp->dev->wol_enabled = wolopts ? 1 : 0;
>> }
>> @@ -2512,7 +2525,7 @@ static void rtl_wol_enable_rx(struct rtl8169_private *tp)
>>
>> static void rtl_prepare_power_down(struct rtl8169_private *tp)
>> {
>> - if (tp->dash_type != RTL_DASH_NONE)
>> + if (tp->dash_enabled)
>> return;
>>
>> if (tp->mac_version == RTL_GIGA_MAC_VER_32 ||
>> @@ -4869,7 +4882,7 @@ static int rtl8169_runtime_idle(struct device *device)
>> {
>> struct rtl8169_private *tp = dev_get_drvdata(device);
>>
>> - if (tp->dash_type != RTL_DASH_NONE)
>> + if (tp->dash_enabled)
>> return -EBUSY;
>>
>> if (!netif_running(tp->dev) || !netif_carrier_ok(tp->dev))
>> @@ -4896,7 +4909,7 @@ static void rtl_shutdown(struct pci_dev *pdev)
>> rtl_rar_set(tp, tp->dev->perm_addr);
>>
>> if (system_state == SYSTEM_POWER_OFF &&
>> - tp->dash_type == RTL_DASH_NONE) {
>> + !tp->dash_enabled) {
>
> Since you have to repost, please maintain the correct indentation
> above:
>
> if (system_state == SYSTEM_POWER_OFF &&
> !tp->dash_enabled) {
>
> ^^^^
> spaces here.
>
>
> Cheers,
>
> Paolo
>
