[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <779dccd7-0573-43d1-b2f3-cf9efdc76e06@axis.com>
Date: Fri, 27 Jun 2025 09:39:24 +0200
From: Kamil Horák (2N) <kamilh@...s.com>
To: netdev <netdev@...r.kernel.org>
Subject: Re: [PATCH 1/3] net: phy: MII-Lite PHY interface mode
On 6/26/25 15:02, Russell King (Oracle) wrote:
> On Thu, Jun 26, 2025 at 01:56:17PM +0200, Kamil Horák - 2N wrote:
>> diff --git a/drivers/net/phy/broadcom.c b/drivers/net/phy/broadcom.c
>> index 9b1de54fd483..7d3b85a07b8c 100644
>> --- a/drivers/net/phy/broadcom.c
>> +++ b/drivers/net/phy/broadcom.c
>> @@ -423,6 +423,13 @@ static int bcm54811_config_init(struct
phy_device *phydev)
>> /* With BCM54811, BroadR-Reach implies no autoneg */
>> if (priv->brr_mode)
>> phydev->autoneg = 0;
>
> Blank line here to aid readability please.
OK
anyway, this is removed in the next commit handling auto-negotiation
>
>> + /* Enable MII Lite (No TXER, RXER, CRS, COL) if configured */
>> + err = bcm_phy_modify_exp(phydev, BCM_EXP_SYNC_ETHERNET,
>> + BCM_EXP_SYNC_ETHERNET_MII_LITE,
>> + phydev->interface == PHY_INTERFACE_MODE_MIILITE ?
>> + BCM_EXP_SYNC_ETHERNET_MII_LITE : 0);
>
> In cases like this, where the ternary op leads to less readable code,
> it's better to do:
>
> if (phydev->interface == PHY_INTERFACE_MODE_MIILITE)
> exp_sync_ethernet = BCM_EXP_SYNC_ETHERNET_MII_LITE;
> else
> exp_sync_ethernet = 0;
>
> err = bcm_phy_modify_exp(phydev, BCM_EXP_SYNC_ETHERNET,
> BCM_EXP_SYNC_ETHERNET_MII_LITE,
> exp_sync_ethernet);
>
>> + if (err < 0)
>> + return err;
>> return bcm5481x_set_brrmode(phydev, priv->brr_mode);
>> }
OK done so
>
> I'd include this with the above change:
>
>> diff --git a/include/linux/brcmphy.h b/include/linux/brcmphy.h
>> index 028b3e00378e..15c35655f482 100644
>> --- a/include/linux/brcmphy.h
>> +++ b/include/linux/brcmphy.h
>> @@ -182,6 +182,12 @@
>> #define BCM_LED_MULTICOLOR_ACT 0x9
>> #define BCM_LED_MULTICOLOR_PROGRAM 0xa
>> +/*
>> + * Broadcom Synchronous Ethernet Controls (expansion register 0x0E)
>> + */
>> +#define BCM_EXP_SYNC_ETHERNET (MII_BCM54XX_EXP_SEL_ER + 0x0E)
>> +#define BCM_EXP_SYNC_ETHERNET_MII_LITE BIT(11)
>> +
>> /*
>> * BCM5482: Shadow registers
>> * Shadow values go into bits [14:10] of register 0x1c to select a
shadow
>
> ... and send the changes below as a separate patch as these changes
> below are modifying generic code.
>
>> diff --git a/drivers/net/phy/phy-core.c b/drivers/net/phy/phy-core.c
>> index e177037f9110..b2df06343b7e 100644
>> --- a/drivers/net/phy/phy-core.c
>> +++ b/drivers/net/phy/phy-core.c
>> @@ -115,6 +115,7 @@ int phy_interface_num_ports(phy_interface_t
interface)
>> return 0;
>> case PHY_INTERFACE_MODE_INTERNAL:
>> case PHY_INTERFACE_MODE_MII:
>> + case PHY_INTERFACE_MODE_MIILITE:
>> case PHY_INTERFACE_MODE_GMII:
>> case PHY_INTERFACE_MODE_TBI:
>> case PHY_INTERFACE_MODE_REVMII:
>> diff --git a/drivers/net/phy/phy_caps.c b/drivers/net/phy/phy_caps.c
>> index 38417e288611..b4a4dea3e756 100644
>> --- a/drivers/net/phy/phy_caps.c
>> +++ b/drivers/net/phy/phy_caps.c
>> @@ -316,6 +316,10 @@ unsigned long
phy_caps_from_interface(phy_interface_t interface)
>> link_caps |= BIT(LINK_CAPA_100HD) | BIT(LINK_CAPA_100FD);
>> break;
>> + case PHY_INTERFACE_MODE_MIILITE:
>> + link_caps |= BIT(LINK_CAPA_10FD) | BIT(LINK_CAPA_100FD);
>> + break;
>> +
>> case PHY_INTERFACE_MODE_TBI:
>> case PHY_INTERFACE_MODE_MOCA:
>> case PHY_INTERFACE_MODE_RTBI:
>> diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
>> index 0faa3d97e06b..766cad40f1b8 100644
>> --- a/drivers/net/phy/phylink.c
>> +++ b/drivers/net/phy/phylink.c
>> @@ -234,6 +234,7 @@ static int
phylink_interface_max_speed(phy_interface_t interface)
>> case PHY_INTERFACE_MODE_SMII:
>> case PHY_INTERFACE_MODE_REVMII:
>> case PHY_INTERFACE_MODE_MII:
>> + case PHY_INTERFACE_MODE_MIILITE:
>> return SPEED_100;
>> case PHY_INTERFACE_MODE_TBI:
>> diff --git a/include/linux/phy.h b/include/linux/phy.h
>> index e194dad1623d..6aad4b741c01 100644
>> --- a/include/linux/phy.h
>> +++ b/include/linux/phy.h
>> @@ -103,6 +103,7 @@ extern const int phy_basic_ports_array[3];
>> * @PHY_INTERFACE_MODE_QUSGMII: Quad Universal SGMII
>> * @PHY_INTERFACE_MODE_1000BASEKX: 1000Base-KX - with Clause 73 AN
>> * @PHY_INTERFACE_MODE_10G_QXGMII: 10G-QXGMII - 4 ports over 10G
USXGMII
>> + * @PHY_INTERFACE_MODE_MIILITE: MII-Lite - MII without RXER TXER
CRS COL
>> * @PHY_INTERFACE_MODE_MAX: Book keeping
>> *
>> * Describes the interface between the MAC and PHY.
>> @@ -144,6 +145,7 @@ typedef enum {
>> PHY_INTERFACE_MODE_QUSGMII,
>> PHY_INTERFACE_MODE_1000BASEKX,
>> PHY_INTERFACE_MODE_10G_QXGMII,
>> + PHY_INTERFACE_MODE_MIILITE,
>> PHY_INTERFACE_MODE_MAX,
>> } phy_interface_t;
>> @@ -260,6 +262,8 @@ static inline const char
*phy_modes(phy_interface_t interface)
>> return "qusgmii";
>> case PHY_INTERFACE_MODE_10G_QXGMII:
>> return "10g-qxgmii";
>> + case PHY_INTERFACE_MODE_MIILITE:
>> + return "mii-lite";
>> default:
>> return "unknown";
>> }
>
OK, separated into "MII-Lite introduction" and "bcm5481x MII-Lite
activation" patches
> Otherwise, I think this is fine.
>
> Please remember netdev's rules, which can be found in the tl;dr on:
> https://www.kernel.org/doc/html/v6.1/process/maintainer-netdev.html
>
> (There's probably an updated version, but I can never remember the URL.)
Kamil
>
Powered by blists - more mailing lists