mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Javen <javen_xu@realsil.com.cn>
To: Maxime Chevallier <maxime.chevallier@bootlin.com>,
	"hkallweit1@gmail.com" <hkallweit1@gmail.com>,
	"nic_swsd@realtek.com" <nic_swsd@realtek.com>,
	"andrew+netdev@lunn.ch" <andrew+netdev@lunn.ch>,
	"davem@davemloft.net" <davem@davemloft.net>,
	"edumazet@google.com" <edumazet@google.com>,
	"kuba@kernel.org" <kuba@kernel.org>,
	"pabeni@redhat.com" <pabeni@redhat.com>,
	"horms@kernel.org" <horms@kernel.org>
Cc: "netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"daniel@makrotopia.org" <daniel@makrotopia.org>,
	"linux@armlinux.org.uk" <linux@armlinux.org.uk>,
	"enelsonmoore@gmail.com" <enelsonmoore@gmail.com>,
	"daniel@thingy.jp" <daniel@thingy.jp>
Subject: RE: [PATCH net-next v6 2/7] net: phy: phylink: add helper to modify pause
Date: Tue, 21 Jul 2026 10:37:40 +0000	[thread overview]
Message-ID: <9ca915e4384a48f1832e4d0e24a977d8@realsil.com.cn> (raw)
In-Reply-To: <c1e3f0c6-89a7-42de-ad77-aede3cd93ea5@bootlin.com>

>
>Hi Javen,
>
>On 7/21/26 11:52, Javen wrote:
>> Hi Maxime,
>>
>>>
>>> Hi Javen,
>>>
>>> On 7/13/26 09:51, Javen wrote:
>>>> Hi,
>>>>
>>>>> There's a change in the MAC's ability to support Pause, so we should :
>>>>>
>>>>> - Recompute the pl->supported field. Update the
>>>>> config.mac_capabilities with the
>>>>>   new pause settings, calling phylink_validate() should do the
>>>>> trick I think,
>>> this
>>>>>   will rebuild the capability list:
>>>>>
>>>>>   phylink_validate(pl, pl->supported, &pl->link_config);
>>>>>
>>>>> - Then update the pl->link_config.pause,
>>>>>
>>>>> - Then update the pause advertising, like done in phylink_setpauseparam
>>>>>   ( I think, everything that comes after pl->state_mutex gets released in
>>>>>    phylink_ethtool_set_pauseparam)
>>>>>
>>>>> Ideally, the logic to update the advertising and re-trigger a
>>>>> negociation should be factored out in a private helper, then reused
>>>>> from both this path (MAC updates pause support) and the
>>> phylink_ethtool_set_pauseparam path.
>>>>>
>>>>> Maxime
>>>>
>>>> Thanks for review and helpful suggestions.
>>>>
>>>> I agree with your suggestion to factor out the logic into a private
>>>> helper and
>>> reuse it for both phylink_ethtool_set_pauseparam() and
>>> phylink_update_mac_pause_capabilities().
>>>>
>>>> Here is the refactored logic. I want to share this specific part
>>>> with you for a
>>> quick check before I submit v7 patch.
>>>
>>> I'm currently attending the netdev conference, It'll take a few days
>>> for me to look at this, sorry about that :/
>>>
>>> Maxime
>>
>> Just a gentle ping on this thread. Could you please take a quick look at this
>refactored logic whenever you are free? Any suggestion would be greatly
>appreciated.
>
>Thanks for the ping, I just looked at it from a high level and this is the thing I
>had in mind yes :)
>
>I think you can post a new revision then.
>
>Thanks for that work,
>
>Maxime

Thanks for review. I will resend the next version.

BRs,
Javen


  reply	other threads:[~2026-07-21 10:38 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-09 10:02 [PATCH net-next v6 0/7] r8169: add support for phylink javen
2026-07-09 10:02 ` [PATCH net-next v6 1/7] r8169: add speed in private struct javen
2026-07-09 10:02 ` [PATCH net-next v6 2/7] net: phy: phylink: add helper to modify pause javen
2026-07-10 10:05   ` Maxime Chevallier
2026-07-10 17:02     ` Andrew Lunn
2026-07-11 19:33       ` Maxime Chevallier
2026-07-13  7:51     ` Javen
2026-07-14  9:16       ` Maxime Chevallier
2026-07-21  9:52         ` Javen
2026-07-21 10:01           ` Maxime Chevallier
2026-07-21 10:37             ` Javen [this message]
2026-07-09 10:02 ` [PATCH net-next v6 3/7] r8169: add support for phylink javen
2026-07-09 10:02 ` [PATCH net-next v6 4/7] r8169: add support for RTL8116af javen
2026-07-09 10:02 ` [PATCH net-next v6 5/7] r8169: add support for RTL8127atf javen
2026-07-09 10:02 ` [PATCH net-next v6 6/7] r8169: add ltr support for RTL8117 series javen
2026-07-09 10:02 ` [PATCH net-next v6 7/7] r8169: fix RTL8116af can not enter s0idle and c10 javen
2026-07-21 15:57   ` Jakub Kicinski

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=9ca915e4384a48f1832e4d0e24a977d8@realsil.com.cn \
    --to=javen_xu@realsil.com.cn \
    --cc=andrew+netdev@lunn.ch \
    --cc=daniel@makrotopia.org \
    --cc=daniel@thingy.jp \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=enelsonmoore@gmail.com \
    --cc=hkallweit1@gmail.com \
    --cc=horms@kernel.org \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@armlinux.org.uk \
    --cc=maxime.chevallier@bootlin.com \
    --cc=netdev@vger.kernel.org \
    --cc=nic_swsd@realtek.com \
    --cc=pabeni@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®