From: "Tan, Tee Min" <tee.min.tan@linux.intel.com>
Add cur_link_an_mode into phy_device struct for PHY drivers to
communicate the in-band AN mode setting with phylink framework.
As there is a mechanism in PHY drivers to switch the PHY interface
between SGMII and 2500BaseX according to link speed. In this case,
the in-band AN mode should be switching based on the PHY interface
as well, if the PHY interface has been changed/updated by PHY driver.
For e.g., disable in-band AN in 2500BaseX mode, or enable in-band AN
back for SGMII mode (10/100/1000Mbps).
Signed-off-by: Tan, Tee Min <tee.min.tan@linux.intel.com>
Signed-off-by: Choong Yong Liang <yong.liang.choong@linux.intel.com>
---
drivers/net/phy/marvell10g.c | 6 ++++++
drivers/net/phy/phylink.c | 3 +++
include/linux/phy.h | 2 ++
3 files changed, 11 insertions(+)
diff --git a/drivers/net/phy/marvell10g.c b/drivers/net/phy/marvell10g.c
index 55d9d7acc32e..5cf26d02841d 100644
--- a/drivers/net/phy/marvell10g.c
+++ b/drivers/net/phy/marvell10g.c
@@ -30,6 +30,7 @@
#include <linux/phy.h>
#include <linux/sfp.h>
#include <linux/netdevice.h>
+#include <linux/phylink.h>
#define MV_PHY_ALASKA_NBT_QUIRK_MASK 0xfffffffe
#define MV_PHY_ALASKA_NBT_QUIRK_REV (MARVELL_PHY_ID_88X3310 | 0xa)
@@ -939,6 +940,9 @@ static void mv3310_update_interface(struct phy_device *phydev)
* xaui / rxaui modes according to the speed.
* Florian suggests setting phydev->interface to communicate this to the
* MAC. Only do this if we are already in one of the above modes.
+ * In-band Auto-negotiation is not supported in 2500BASE-X.
+ * Setting phydev->cur_link_an_mode to communicate this to the
+ * phylink framework.
*/
switch (phydev->speed) {
case SPEED_10000:
@@ -949,11 +953,13 @@ static void mv3310_update_interface(struct phy_device *phydev)
break;
case SPEED_2500:
phydev->interface = PHY_INTERFACE_MODE_2500BASEX;
+ phydev->cur_link_an_mode = MLO_AN_PHY;
break;
case SPEED_1000:
case SPEED_100:
case SPEED_10:
phydev->interface = PHY_INTERFACE_MODE_SGMII;
+ phydev->cur_link_an_mode = MLO_AN_INBAND;
break;
default:
break;
diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
index 97c15e1f81de..d5783ea95eee 100644
--- a/drivers/net/phy/phylink.c
+++ b/drivers/net/phy/phylink.c
@@ -1654,6 +1654,7 @@ static void phylink_phy_change(struct phy_device *phydev, bool up)
pl->phy_state.pause |= MLO_PAUSE_RX;
pl->phy_state.interface = phydev->interface;
pl->phy_state.link = up;
+ pl->cur_link_an_mode = phydev->cur_link_an_mode;
mutex_unlock(&pl->state_mutex);
phylink_run_resolve(pl);
@@ -1758,6 +1759,8 @@ static int phylink_bringup_phy(struct phylink *pl, struct phy_device *phy,
if (pl->config->mac_managed_pm)
phy->mac_managed_pm = true;
+ pl->phydev->cur_link_an_mode = pl->cur_link_an_mode;
+
return 0;
}
diff --git a/include/linux/phy.h b/include/linux/phy.h
index 11c1e91563d4..c685b526e307 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -756,6 +756,8 @@ struct phy_device {
/* MACsec management functions */
const struct macsec_ops *macsec_ops;
#endif
+ /* For communicate the AN mode setting with phylink framework. */
+ u8 cur_link_an_mode;
};
/* Generic phy_device::dev_flags */
--
2.25.1On Thu, Jun 22, 2023 at 12:19:02PM +0800, Choong Yong Liang wrote:
> From: "Tan, Tee Min" <tee.min.tan@linux.intel.com>
>
> Add cur_link_an_mode into phy_device struct for PHY drivers to
> communicate the in-band AN mode setting with phylink framework.
>
> As there is a mechanism in PHY drivers to switch the PHY interface
> between SGMII and 2500BaseX according to link speed. In this case,
> the in-band AN mode should be switching based on the PHY interface
> as well, if the PHY interface has been changed/updated by PHY driver.
>
> For e.g., disable in-band AN in 2500BaseX mode, or enable in-band AN
> back for SGMII mode (10/100/1000Mbps).
>
> Signed-off-by: Tan, Tee Min <tee.min.tan@linux.intel.com>
> Signed-off-by: Choong Yong Liang <yong.liang.choong@linux.intel.com>
...
> diff --git a/include/linux/phy.h b/include/linux/phy.h
> index 11c1e91563d4..c685b526e307 100644
> --- a/include/linux/phy.h
> +++ b/include/linux/phy.h
> @@ -756,6 +756,8 @@ struct phy_device {
> /* MACsec management functions */
> const struct macsec_ops *macsec_ops;
> #endif
> + /* For communicate the AN mode setting with phylink framework. */
> + u8 cur_link_an_mode;
> };
Hi Choong Yong Liang,
Please consider adding cur_link_an_mode to the kernel doc
for struct phy_device - which is above the definition of struct phy_device.
Hi Simon,
I will update it in v2. Thank you.
On 22/6/2023 10:43 pm, Simon Horman wrote:
> On Thu, Jun 22, 2023 at 12:19:02PM +0800, Choong Yong Liang wrote:
>> From: "Tan, Tee Min" <tee.min.tan@linux.intel.com>
>>
>> Add cur_link_an_mode into phy_device struct for PHY drivers to
>> communicate the in-band AN mode setting with phylink framework.
>>
>> As there is a mechanism in PHY drivers to switch the PHY interface
>> between SGMII and 2500BaseX according to link speed. In this case,
>> the in-band AN mode should be switching based on the PHY interface
>> as well, if the PHY interface has been changed/updated by PHY driver.
>>
>> For e.g., disable in-band AN in 2500BaseX mode, or enable in-band AN
>> back for SGMII mode (10/100/1000Mbps).
>>
>> Signed-off-by: Tan, Tee Min <tee.min.tan@linux.intel.com>
>> Signed-off-by: Choong Yong Liang <yong.liang.choong@linux.intel.com>
>
> ...
>
>> diff --git a/include/linux/phy.h b/include/linux/phy.h
>> index 11c1e91563d4..c685b526e307 100644
>> --- a/include/linux/phy.h
>> +++ b/include/linux/phy.h
>> @@ -756,6 +756,8 @@ struct phy_device {
>> /* MACsec management functions */
>> const struct macsec_ops *macsec_ops;
>> #endif
>> + /* For communicate the AN mode setting with phylink framework. */
>> + u8 cur_link_an_mode;
>> };
>
> Hi Choong Yong Liang,
>
> Please consider adding cur_link_an_mode to the kernel doc
> for struct phy_device - which is above the definition of struct phy_device.
On Thu, Jun 22, 2023 at 04:43:51PM +0200, Simon Horman wrote:
> On Thu, Jun 22, 2023 at 12:19:02PM +0800, Choong Yong Liang wrote:
> > From: "Tan, Tee Min" <tee.min.tan@linux.intel.com>
> >
> > Add cur_link_an_mode into phy_device struct for PHY drivers to
> > communicate the in-band AN mode setting with phylink framework.
> >
> > As there is a mechanism in PHY drivers to switch the PHY interface
> > between SGMII and 2500BaseX according to link speed. In this case,
> > the in-band AN mode should be switching based on the PHY interface
> > as well, if the PHY interface has been changed/updated by PHY driver.
> >
> > For e.g., disable in-band AN in 2500BaseX mode, or enable in-band AN
> > back for SGMII mode (10/100/1000Mbps).
> >
> > Signed-off-by: Tan, Tee Min <tee.min.tan@linux.intel.com>
> > Signed-off-by: Choong Yong Liang <yong.liang.choong@linux.intel.com>
>
> ...
>
> > diff --git a/include/linux/phy.h b/include/linux/phy.h
> > index 11c1e91563d4..c685b526e307 100644
> > --- a/include/linux/phy.h
> > +++ b/include/linux/phy.h
> > @@ -756,6 +756,8 @@ struct phy_device {
> > /* MACsec management functions */
> > const struct macsec_ops *macsec_ops;
> > #endif
> > + /* For communicate the AN mode setting with phylink framework. */
> > + u8 cur_link_an_mode;
> > };
>
> Hi Choong Yong Liang,
>
> Please consider adding cur_link_an_mode to the kernel doc
> for struct phy_device - which is above the definition of struct phy_device.
This looks like it's grabbing something from phylink and stuffing it
into phylib. However, I have no idea, because I don't seem to have
received the original patches. I'm guessing the reason is:
2023-06-22 05:21:24 1qCBoy-0003ji-G9 H=mga03.intel.com
[134.134.136.65]:57703 I=[78.32.30.218]:25
X=TLS1.2:ECDHE_SECP521R1__RSA_SHA512__AES_256_GCM:256
F=<yong.liang.choong@linux.intel.com> rejected after DATA: unqualified
address not permitted: failing address in "Cc:" header is: Tan
Which I suspect came from:
Tan, Tee Min <tee.min.tan@linux.intel.com>
and someone doesn't realise that a "," in the display-name part of
an address *must* be quoted, otherwise "," is taken to be a separator
in the address list.
Consequently, it has now become:
Tan@web.codeaurora.org, Tee Min <tee.min.tan@linux.intel.com>,
It should have been:
"Tan, Tee Min" <tee.min.tan@linux.intel.com>
with the double-quotes.
Please do not review this series further, but instead, please can the
author repost it forthwith with correct conformant headers so that a
proper review can be undertaken by all?
Thanks.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
Hi Russell,
Thank you for pointing that out.
I will fix it and send out version 2.
On 22/6/2023 11:06 pm, Russell King (Oracle) wrote:
> On Thu, Jun 22, 2023 at 04:43:51PM +0200, Simon Horman wrote:
>> On Thu, Jun 22, 2023 at 12:19:02PM +0800, Choong Yong Liang wrote:
>>> From: "Tan, Tee Min" <tee.min.tan@linux.intel.com>
>>>
>>> Add cur_link_an_mode into phy_device struct for PHY drivers to
>>> communicate the in-band AN mode setting with phylink framework.
>>>
>>> As there is a mechanism in PHY drivers to switch the PHY interface
>>> between SGMII and 2500BaseX according to link speed. In this case,
>>> the in-band AN mode should be switching based on the PHY interface
>>> as well, if the PHY interface has been changed/updated by PHY driver.
>>>
>>> For e.g., disable in-band AN in 2500BaseX mode, or enable in-band AN
>>> back for SGMII mode (10/100/1000Mbps).
>>>
>>> Signed-off-by: Tan, Tee Min <tee.min.tan@linux.intel.com>
>>> Signed-off-by: Choong Yong Liang <yong.liang.choong@linux.intel.com>
>>
>> ...
>>
>>> diff --git a/include/linux/phy.h b/include/linux/phy.h
>>> index 11c1e91563d4..c685b526e307 100644
>>> --- a/include/linux/phy.h
>>> +++ b/include/linux/phy.h
>>> @@ -756,6 +756,8 @@ struct phy_device {
>>> /* MACsec management functions */
>>> const struct macsec_ops *macsec_ops;
>>> #endif
>>> + /* For communicate the AN mode setting with phylink framework. */
>>> + u8 cur_link_an_mode;
>>> };
>>
>> Hi Choong Yong Liang,
>>
>> Please consider adding cur_link_an_mode to the kernel doc
>> for struct phy_device - which is above the definition of struct phy_device.
>
> This looks like it's grabbing something from phylink and stuffing it
> into phylib. However, I have no idea, because I don't seem to have
> received the original patches. I'm guessing the reason is:
>
> 2023-06-22 05:21:24 1qCBoy-0003ji-G9 H=mga03.intel.com
> [134.134.136.65]:57703 I=[78.32.30.218]:25
> X=TLS1.2:ECDHE_SECP521R1__RSA_SHA512__AES_256_GCM:256
> F=<yong.liang.choong@linux.intel.com> rejected after DATA: unqualified
> address not permitted: failing address in "Cc:" header is: Tan
>
> Which I suspect came from:
>
> Tan, Tee Min <tee.min.tan@linux.intel.com>
>
> and someone doesn't realise that a "," in the display-name part of
> an address *must* be quoted, otherwise "," is taken to be a separator
> in the address list.
>
> Consequently, it has now become:
>
> Tan@web.codeaurora.org, Tee Min <tee.min.tan@linux.intel.com>,
>
> It should have been:
>
> "Tan, Tee Min" <tee.min.tan@linux.intel.com>
>
> with the double-quotes.
>
> Please do not review this series further, but instead, please can the
> author repost it forthwith with correct conformant headers so that a
> proper review can be undertaken by all?
>
> Thanks.
>
On Thu, Jun 22, 2023 at 04:06:31PM +0100, Russell King (Oracle) wrote:
> On Thu, Jun 22, 2023 at 04:43:51PM +0200, Simon Horman wrote:
> > On Thu, Jun 22, 2023 at 12:19:02PM +0800, Choong Yong Liang wrote:
> > > From: "Tan, Tee Min" <tee.min.tan@linux.intel.com>
> > >
> > > Add cur_link_an_mode into phy_device struct for PHY drivers to
> > > communicate the in-band AN mode setting with phylink framework.
> > >
> > > As there is a mechanism in PHY drivers to switch the PHY interface
> > > between SGMII and 2500BaseX according to link speed. In this case,
> > > the in-band AN mode should be switching based on the PHY interface
> > > as well, if the PHY interface has been changed/updated by PHY driver.
> > >
> > > For e.g., disable in-band AN in 2500BaseX mode, or enable in-band AN
> > > back for SGMII mode (10/100/1000Mbps).
> > >
> > > Signed-off-by: Tan, Tee Min <tee.min.tan@linux.intel.com>
> > > Signed-off-by: Choong Yong Liang <yong.liang.choong@linux.intel.com>
> >
> > ...
> >
> > > diff --git a/include/linux/phy.h b/include/linux/phy.h
> > > index 11c1e91563d4..c685b526e307 100644
> > > --- a/include/linux/phy.h
> > > +++ b/include/linux/phy.h
> > > @@ -756,6 +756,8 @@ struct phy_device {
> > > /* MACsec management functions */
> > > const struct macsec_ops *macsec_ops;
> > > #endif
> > > + /* For communicate the AN mode setting with phylink framework. */
> > > + u8 cur_link_an_mode;
> > > };
> >
> > Hi Choong Yong Liang,
> >
> > Please consider adding cur_link_an_mode to the kernel doc
> > for struct phy_device - which is above the definition of struct phy_device.
>
> This looks like it's grabbing something from phylink and stuffing it
> into phylib. However, I have no idea, because I don't seem to have
> received the original patches. I'm guessing the reason is:
>
> 2023-06-22 05:21:24 1qCBoy-0003ji-G9 H=mga03.intel.com
> [134.134.136.65]:57703 I=[78.32.30.218]:25
> X=TLS1.2:ECDHE_SECP521R1__RSA_SHA512__AES_256_GCM:256
> F=<yong.liang.choong@linux.intel.com> rejected after DATA: unqualified
> address not permitted: failing address in "Cc:" header is: Tan
>
> Which I suspect came from:
>
> Tan, Tee Min <tee.min.tan@linux.intel.com>
>
> and someone doesn't realise that a "," in the display-name part of
> an address *must* be quoted, otherwise "," is taken to be a separator
> in the address list.
>
> Consequently, it has now become:
>
> Tan@web.codeaurora.org, Tee Min <tee.min.tan@linux.intel.com>,
>
> It should have been:
>
> "Tan, Tee Min" <tee.min.tan@linux.intel.com>
>
> with the double-quotes.
>
> Please do not review this series further, but instead, please can the
> author repost it forthwith with correct conformant headers so that a
> proper review can be undertaken by all?
Hi Russell,
Sorry for not noticing this myself.
I agree that we should wait for a properly formed post as you suggest.
© 2016 - 2026 Red Hat, Inc.