From: "Russell King (Oracle)" <linux@armlinux.org.uk> To: Choong Yong Liang <yong.liang.choong@linux.intel.com> Cc: Rajneesh Bhardwaj <irenic.rajneesh@gmail.com>, David E Box <david.e.box@linux.intel.com>, Hans de Goede <hdegoede@redhat.com>, Mark Gross <markgross@kernel.org>, Alexandre Torgue <alexandre.torgue@foss.st.com>, Jose Abreu <Jose.Abreu@synopsys.com>, "David S . Miller" <davem@davemloft.net>, Eric Dumazet <edumazet@google.com>, Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>, Maxime Coquelin <mcoquelin.stm32@gmail.com>, Richard Cochran <richardcochran@gmail.com>, Alexei Starovoitov <ast@kernel.org>, Daniel Borkmann <daniel@iogearbox.net>, Jesper Dangaard Brouer <hawk@kernel.org>, John Fastabend <john.fastabend@gmail.com>, Andrew Lunn <andrew@lunn.ch>, Heiner Kallweit <hkallweit1@gmail.com>, Philipp Zabel <p.zabel@pengutronix.de>, Andrew Halaney <ahalaney@redhat.com>, Simon Horman <simon.horman@corigine.com>, Serge Semin <fancer.lancer@gmail.com>, netdev@vger.kernel.org, linux-kernel@vger.kernel.org, linux-stm32@st-md-mailman.stormreply.com, linux-arm-kernel@lists.infradead.org, platform-driver-x86@vger.kernel.org, linux-hwmon@vger.kernel.org, bpf@vger.kernel.org, Voon Wei Feng <weifeng.voon@intel.com>, Michael Sit Wei Hong <michael.wei.hong.sit@intel.com>, Lai Peter Jun Ann <jun.ann.lai@intel.com>, Abdul Rahim Faizal <faizal.abdul.rahim@intel.com> Subject: Re: [PATCH net-next v4 06/11] net: stmmac: resetup XPCS according to the new interface mode Date: Fri, 2 Feb 2024 08:50:34 +0000 [thread overview] Message-ID: <Zbys2orOUikYxeOm@shell.armlinux.org.uk> (raw) In-Reply-To: <2ad1f55c-f361-4439-9174-6af1bb429d55@linux.intel.com> On Fri, Feb 02, 2024 at 11:00:58AM +0800, Choong Yong Liang wrote: > > > On 1/2/2024 4:38 pm, Russell King (Oracle) wrote: > > Note the "This must not modify any state." statement. By reinitialising > > the PCS in this method, you are violating that statement. > > > > This requirement is because this method will be called by > > phylink_validate_mac_and_pcs() at various times, potentially for each > > and every interface that stmmac supports, which will lead to you > > reinitialising the PCS, killing the link, each time we ask the MAC for > > a PCS, whether we are going to make use of it in that mode or not. > > > > You can not do this. Sorry. Hard NAK for this approach. > > > Thank you for taking the time to review, got your concerns, and I'll address > the following concerns before submitting a new patch series: > > 1. Remove allow_switch_interface and have the PHY driver fill in > phydev->possible_interfaces. Yes please. > 2. Rework on the PCS to have similar implementation with the following patch > "net: macb: use .mac_select_pcs() interface" > (https://lore.kernel.org/netdev/E1n568J-002SZX-Gr@rmk-PC.armlinux.org.uk/T/). mac_select_pcs() is about returning to phylink the PCS that the MAC needs to use for the specified interface mode, or NULL if no PCS is required, nothing more, nothing less. Plase do not copy that mac_select_pcs() implementation - changing the "ops" underneath phylink is no longer permitted. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
WARNING: multiple messages have this Message-ID (diff)
From: "Russell King (Oracle)" <linux@armlinux.org.uk> To: Choong Yong Liang <yong.liang.choong@linux.intel.com> Cc: Rajneesh Bhardwaj <irenic.rajneesh@gmail.com>, David E Box <david.e.box@linux.intel.com>, Hans de Goede <hdegoede@redhat.com>, Mark Gross <markgross@kernel.org>, Alexandre Torgue <alexandre.torgue@foss.st.com>, Jose Abreu <Jose.Abreu@synopsys.com>, "David S . Miller" <davem@davemloft.net>, Eric Dumazet <edumazet@google.com>, Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>, Maxime Coquelin <mcoquelin.stm32@gmail.com>, Richard Cochran <richardcochran@gmail.com>, Alexei Starovoitov <ast@kernel.org>, Daniel Borkmann <daniel@iogearbox.net>, Jesper Dangaard Brouer <hawk@kernel.org>, John Fastabend <john.fastabend@gmail.com>, Andrew Lunn <andrew@lunn.ch>, Heiner Kallweit <hkallweit1@gmail.com>, Philipp Zabel <p.zabel@pengutronix.de>, Andrew Halaney <ahalaney@redhat.com>, Simon Horman <simon.horman@corigine.com>, Serge Semin <fancer.lancer@gmail.com>, netdev@vger.kernel.org, linux-kernel@vger.kernel.org, linux-stm32@st-md-mailman.stormreply.com, linux-arm-kernel@lists.infradead.org, platform-driver-x86@vger.kernel.org, linux-hwmon@vger.kernel.org, bpf@vger.kernel.org, Voon Wei Feng <weifeng.voon@intel.com>, Michael Sit Wei Hong <michael.wei.hong.sit@intel.com>, Lai Peter Jun Ann <jun.ann.lai@intel.com>, Abdul Rahim Faizal <faizal.abdul.rahim@intel.com> Subject: Re: [PATCH net-next v4 06/11] net: stmmac: resetup XPCS according to the new interface mode Date: Fri, 2 Feb 2024 08:50:34 +0000 [thread overview] Message-ID: <Zbys2orOUikYxeOm@shell.armlinux.org.uk> (raw) In-Reply-To: <2ad1f55c-f361-4439-9174-6af1bb429d55@linux.intel.com> On Fri, Feb 02, 2024 at 11:00:58AM +0800, Choong Yong Liang wrote: > > > On 1/2/2024 4:38 pm, Russell King (Oracle) wrote: > > Note the "This must not modify any state." statement. By reinitialising > > the PCS in this method, you are violating that statement. > > > > This requirement is because this method will be called by > > phylink_validate_mac_and_pcs() at various times, potentially for each > > and every interface that stmmac supports, which will lead to you > > reinitialising the PCS, killing the link, each time we ask the MAC for > > a PCS, whether we are going to make use of it in that mode or not. > > > > You can not do this. Sorry. Hard NAK for this approach. > > > Thank you for taking the time to review, got your concerns, and I'll address > the following concerns before submitting a new patch series: > > 1. Remove allow_switch_interface and have the PHY driver fill in > phydev->possible_interfaces. Yes please. > 2. Rework on the PCS to have similar implementation with the following patch > "net: macb: use .mac_select_pcs() interface" > (https://lore.kernel.org/netdev/E1n568J-002SZX-Gr@rmk-PC.armlinux.org.uk/T/). mac_select_pcs() is about returning to phylink the PCS that the MAC needs to use for the specified interface mode, or NULL if no PCS is required, nothing more, nothing less. Plase do not copy that mac_select_pcs() implementation - changing the "ops" underneath phylink is no longer permitted. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last! _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2024-02-02 8:51 UTC|newest] Thread overview: 48+ messages / expand[flat|nested] mbox.gz Atom feed top 2024-01-29 13:02 [PATCH net-next v4 00/11] Enable SGMII and 2500BASEX interface mode switching for Intel platforms Choong Yong Liang 2024-01-29 13:02 ` Choong Yong Liang 2024-01-29 13:02 ` [PATCH net-next v4 01/11] net: phylink: publish ethtool link modes that supported and advertised Choong Yong Liang 2024-01-29 13:02 ` Choong Yong Liang 2024-01-30 9:27 ` Russell King (Oracle) 2024-01-30 9:27 ` Russell King (Oracle) 2024-01-29 13:02 ` [PATCH net-next v4 02/11] net: stmmac: provide allow_switch_interface flag Choong Yong Liang 2024-01-29 13:02 ` Choong Yong Liang 2024-01-29 13:02 ` [PATCH net-next v4 03/11] net: phylink: provide mac_get_pcs_neg_mode() function Choong Yong Liang 2024-01-29 13:02 ` Choong Yong Liang 2024-01-29 13:02 ` [PATCH net-next v4 04/11] net: phylink: add phylink_pcs_neg_mode() declaration into phylink.h Choong Yong Liang 2024-01-29 13:02 ` Choong Yong Liang 2024-01-29 13:02 ` [PATCH net-next v4 05/11] net: stmmac: select PCS negotiation mode according to the interface mode Choong Yong Liang 2024-01-29 13:02 ` Choong Yong Liang 2024-01-29 13:02 ` [PATCH net-next v4 06/11] net: stmmac: resetup XPCS according to the new " Choong Yong Liang 2024-01-29 13:02 ` Choong Yong Liang 2024-01-30 10:21 ` Russell King (Oracle) 2024-01-30 10:21 ` Russell King (Oracle) 2024-02-01 5:10 ` Choong Yong Liang 2024-02-01 5:10 ` Choong Yong Liang 2024-02-01 8:38 ` Russell King (Oracle) 2024-02-01 8:38 ` Russell King (Oracle) 2024-02-02 3:00 ` Choong Yong Liang 2024-02-02 3:00 ` Choong Yong Liang 2024-02-02 8:50 ` Russell King (Oracle) [this message] 2024-02-02 8:50 ` Russell King (Oracle) 2024-02-15 3:14 ` Choong Yong Liang 2024-02-15 3:14 ` Choong Yong Liang 2024-01-29 13:02 ` [PATCH net-next v4 07/11] arch: x86: Add IPC mailbox accessor function and add SoC register access Choong Yong Liang 2024-01-29 13:02 ` Choong Yong Liang 2024-01-31 10:54 ` Ilpo Järvinen 2024-01-31 10:54 ` Ilpo Järvinen 2024-02-02 3:04 ` Choong Yong Liang 2024-02-02 3:04 ` Choong Yong Liang 2024-01-29 13:02 ` [PATCH net-next v4 08/11] stmmac: intel: configure SerDes according to the interface mode Choong Yong Liang 2024-01-29 13:02 ` Choong Yong Liang 2024-01-30 8:48 ` kernel test robot 2024-01-30 8:48 ` kernel test robot 2024-01-31 10:58 ` Ilpo Järvinen 2024-01-31 10:58 ` Ilpo Järvinen 2024-02-02 3:06 ` Choong Yong Liang 2024-02-02 3:06 ` Choong Yong Liang 2024-01-29 13:02 ` [PATCH net-next v4 09/11] net: stmmac: configure SerDes on mac_finish Choong Yong Liang 2024-01-29 13:02 ` Choong Yong Liang 2024-01-29 13:02 ` [PATCH net-next v4 10/11] stmmac: intel: interface switching support for EHL platform Choong Yong Liang 2024-01-29 13:02 ` Choong Yong Liang 2024-01-29 13:02 ` [PATCH net-next v4 11/11] stmmac: intel: interface switching support for ADL-N platform Choong Yong Liang 2024-01-29 13:02 ` Choong Yong Liang
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=Zbys2orOUikYxeOm@shell.armlinux.org.uk \ --to=linux@armlinux.org.uk \ --cc=Jose.Abreu@synopsys.com \ --cc=ahalaney@redhat.com \ --cc=alexandre.torgue@foss.st.com \ --cc=andrew@lunn.ch \ --cc=ast@kernel.org \ --cc=bpf@vger.kernel.org \ --cc=daniel@iogearbox.net \ --cc=davem@davemloft.net \ --cc=david.e.box@linux.intel.com \ --cc=edumazet@google.com \ --cc=faizal.abdul.rahim@intel.com \ --cc=fancer.lancer@gmail.com \ --cc=hawk@kernel.org \ --cc=hdegoede@redhat.com \ --cc=hkallweit1@gmail.com \ --cc=irenic.rajneesh@gmail.com \ --cc=john.fastabend@gmail.com \ --cc=jun.ann.lai@intel.com \ --cc=kuba@kernel.org \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-hwmon@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-stm32@st-md-mailman.stormreply.com \ --cc=markgross@kernel.org \ --cc=mcoquelin.stm32@gmail.com \ --cc=michael.wei.hong.sit@intel.com \ --cc=netdev@vger.kernel.org \ --cc=p.zabel@pengutronix.de \ --cc=pabeni@redhat.com \ --cc=platform-driver-x86@vger.kernel.org \ --cc=richardcochran@gmail.com \ --cc=simon.horman@corigine.com \ --cc=weifeng.voon@intel.com \ --cc=yong.liang.choong@linux.intel.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: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.