| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <7039d3f0-8102-4d0d-afec-1b96e3709aa6@broadcom.com>
Date: Tue, 21 Oct 2025 09:36:02 -0700
From: Florian Fainelli <florian.fainelli@...adcom.com>
To: Balázs Triszka <info@...ika011.hu>,
bcm-kernel-feedback-list@...adcom.com, Russell King <linux@...linux.org.uk>,
netdev@...r.kernel.org
Subject: Re: [PATCH] net-phy-bcm84881: add support for other 2.5G / 5G / 10G
phys
Hello,
On 10/20/2025 3:25 PM, 'Balázs Triszka' via BCM-KERNEL-FEEDBACK-LIST,PDL
wrote:
> This patch add support for mako, orca, blackfin, shortfin and
> longfin phys to bcm84881 driver.
>
> Signed-off-by: Balázs Triszka <info@...ika011.hu>
> Cc:florian.fainelli@...adcom.com
> Cc:andrew@...n.ch
You need to run ./scripts/get_maintainer.pl to get the proper
recipients, right now this patch has not been sent to all of them,
specifically the driver author, Russell, is critically missing from the
recipients list.
> ---
> drivers/net/phy/bcm84881.c | 1127 +++++++++++++++++++++++++++++++++++-
> 1 file changed, 1110 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/net/phy/bcm84881.c b/drivers/net/phy/bcm84881.c
> index d7f7cc44c532..d8dc32aa4ada 100644
> --- a/drivers/net/phy/bcm84881.c
> +++ b/drivers/net/phy/bcm84881.c
> @@ -16,6 +16,106 @@
> #include <linux/module.h>
> #include <linux/phy.h>
>
> +#define PHYID_BCM4912 0x359050cd
> +#define PHYID_BCM4912M 0x359051cd
> +#define PHYID_BCM50991EL_A0 0x359050c8
> +#define PHYID_BCM50991EL_B0 0x359050c9
> +#define PHYID_BCM50991ELM_B0 0x3590518d
> +#define PHYID_BCM50994E_A0 0x359050f8
> +#define PHYID_BCM50994E_B0 0x359050f9
> +#define PHYID_BCM54991E_A0 0x35905098
> +#define PHYID_BCM54991E_B0 0x35905099
> +#define PHYID_BCM54991EL_A0 0x35905088
> +#define PHYID_BCM54991EL_B0 0x35905089
> +#define PHYID_BCM54991ELM_A0 0x35905188
> +#define PHYID_BCM54991ELM_B0 0x35905189
> +#define PHYID_BCM54991EM_A0 0x35905198
> +#define PHYID_BCM54991EM_B0 0x35905199
> +#define PHYID_BCM54992E_A0 0x359050a8
> +#define PHYID_BCM54992E_B0 0x359050a9
> +#define PHYID_BCM54992EM_A0 0x359051a8
> +#define PHYID_BCM54992EM_B0 0x359051a9
> +#define PHYID_BCM54994E_A0 0x359050b8
> +#define PHYID_BCM54994E_B0 0x359050b9
> +#define PHYID_BCM54994EM_A0 0x359051b8
> +#define PHYID_BCM54994EM_B0 0x359051b9
> +
> +#define PHYID_BCM49418 0x359050c1
> +#define PHYID_BCM49418M 0x359051c1
> +#define PHYID_BCM54991_A0 0x35905094
> +#define PHYID_BCM54991_B0 0x35905095
> +#define PHYID_BCM54991L_A0 0x35905084
> +#define PHYID_BCM54991L_B0 0x35905085
> +#define PHYID_BCM54991M_A0 0x35905194
> +#define PHYID_BCM54991M_B0 0x35905195
> +#define PHYID_BCM54992_A0 0x359050a4
> +#define PHYID_BCM54992_B0 0x359050a5
> +#define PHYID_BCM54992M_A0 0x359051a4
> +#define PHYID_BCM54992M_B0 0x359051a5
> +#define PHYID_BCM54994_A0 0x359050b4
> +#define PHYID_BCM54994_B0 0x359050b5
> +#define PHYID_BCM54994M_A0 0x359051b4
> +#define PHYID_BCM54994M_B0 0x359051b5
> +#define PHYID_BCM84860_A0 0xae025048
> +#define PHYID_BCM84861_A0 0xae025040
> +#define PHYID_BCM84880_A0 0xae025158
> +#define PHYID_BCM84880_B0 0xae025159
> +#define PHYID_BCM84884_A0 0xae025148
> +#define PHYID_BCM84884_B0 0xae025149
> +#define PHYID_BCM84884E_A0 0xae025168
> +#define PHYID_BCM84884E_B0 0xae025169
> +#define PHYID_BCM84885_A0 0xae025178
> +#define PHYID_BCM84885_B0 0xae025179
> +
> +#define PHYID_BCM54991H_A0 0x359050d0
> +#define PHYID_BCM54991H_A1 0x359051d0
> +#define PHYID_BCM54991H_B0 0x359050d1
> +#define PHYID_BCM54991H_B1 0x359051d1
> +#define PHYID_BCM54991LM_A0 0x35905184
> +#define PHYID_BCM54991LM_B0 0x35905185
> +#define PHYID_BCM54991SK_B0 0x359051d5
> +#define PHYID_BCM54994EL_B0 0x3590501d
> +#define PHYID_BCM54994H_A0 0x359050f0
> +#define PHYID_BCM54994H_A1 0x359051f0
> +#define PHYID_BCM54994H_B0 0x359050f1
> +#define PHYID_BCM54994H_B1 0x359051f1
> +#define PHYID_BCM54994L_B0 0x35905019
> +#define PHYID_BCM54994SK_B0 0x359051f5
> +#define PHYID_BCM54998_B0 0x35905011
> +#define PHYID_BCM54998E_B0 0x35905015
> +#define PHYID_BCM54998ES_B0 0x3590500d
> +#define PHYID_BCM54998S_B0 0x35905009
> +#define PHYID_BCM84881_A0 0xae025150
> +#define PHYID_BCM84881_B0 0xae025151
> +#define PHYID_BCM84886_A0 0xae025170
> +#define PHYID_BCM84886_B0 0xae025171
> +#define PHYID_BCM84887_A0 0xae025144
> +#define PHYID_BCM84887_B0 0xae025145
> +#define PHYID_BCM84888_A0 0xae025140
> +#define PHYID_BCM84888_B0 0xae025141
> +#define PHYID_BCM84888E_A0 0xae025160
> +#define PHYID_BCM84888E_B0 0xae025161
> +#define PHYID_BCM84888S_A0 0xae025174
> +#define PHYID_BCM84888S_B0 0xae025175
> +#define PHYID_BCM84891_A0 0x35905090
> +#define PHYID_BCM84891_B0 0x35905091
> +#define PHYID_BCM84891L_A0 0x35905080
> +#define PHYID_BCM84891L_B0 0x35905081
> +#define PHYID_BCM84891LM_A0 0x35905180
> +#define PHYID_BCM84891LM_B0 0x35905181
> +#define PHYID_BCM84891M_A0 0x35905190
> +#define PHYID_BCM84891M_B0 0x35905191
> +#define PHYID_BCM84892_A0 0x359050a0
> +#define PHYID_BCM84892_B0 0x359050a1
> +#define PHYID_BCM84892M_A0 0x359051a0
> +#define PHYID_BCM84892M_B0 0x359051a1
> +#define PHYID_BCM84894_A0 0x359050b0
> +#define PHYID_BCM84894_B0 0x359050b1
> +#define PHYID_BCM84894M_A0 0x359051b0
> +#define PHYID_BCM84894M_B0 0x359051b1
> +#define PHYID_BCM84896_B0 0x35905005
> +#define PHYID_BCM84898_B0 0x35905001
PHY identifiers are all maintained in include/linux/brcmphy.h, also
there is no need to be maintaining per-revision PHY identifiers, just
the main chip identifiers are enough.
> +
> enum {
> MDIO_AN_C22 = 0xffe0,
> };
> @@ -29,22 +129,57 @@ static int bcm84881_wait_init(struct phy_device *phydev)
> 100000, 2000000, false);
> }
>
> -static void bcm84881_fill_possible_interfaces(struct phy_device *phydev)
> +static int bcm84881_config_init_2500(struct phy_device *phydev)
> {
> unsigned long *possible = phydev->possible_interfaces;
>
> __set_bit(PHY_INTERFACE_MODE_SGMII, possible);
> __set_bit(PHY_INTERFACE_MODE_2500BASEX, possible);
> - __set_bit(PHY_INTERFACE_MODE_10GBASER, possible);
> +
> + switch (phydev->interface) {
> + case PHY_INTERFACE_MODE_SGMII:
> + case PHY_INTERFACE_MODE_2500BASEX:
> + break;
> + default:
> + return -ENODEV;
> + }
> +
> + return 0;
> +}
> +
> +static int bcm84881_config_init_5G(struct phy_device *phydev)
Why not bcm84881_config_init_5000() for consistenty with
bcm84881_config_init_2500()?
> +{
> + unsigned long *possible = phydev->possible_interfaces;
> +
> + __set_bit(PHY_INTERFACE_MODE_SGMII, possible);
> + __set_bit(PHY_INTERFACE_MODE_2500BASEX, possible);
> + __set_bit(PHY_INTERFACE_MODE_5GBASER, possible);
> +
> + switch (phydev->interface) {
> + case PHY_INTERFACE_MODE_SGMII:
> + case PHY_INTERFACE_MODE_2500BASEX:
> + case PHY_INTERFACE_MODE_5GBASER:
> + break;
> + default:
> + return -ENODEV;
> + }
> +
> + return 0;
> }
>
> -static int bcm84881_config_init(struct phy_device *phydev)
> +static int bcm84881_config_init_10G(struct phy_device *phydev)
Likewise.
> {
> - bcm84881_fill_possible_interfaces(phydev);
> + unsigned long *possible = phydev->possible_interfaces;
> +
> + __set_bit(PHY_INTERFACE_MODE_SGMII, possible);
> + __set_bit(PHY_INTERFACE_MODE_2500BASEX, possible);
> + __set_bit(PHY_INTERFACE_MODE_5GBASER, possible);
> + __set_bit(PHY_INTERFACE_MODE_10GBASER, possible);
>
> switch (phydev->interface) {
> case PHY_INTERFACE_MODE_SGMII:
> case PHY_INTERFACE_MODE_2500BASEX:
> + case PHY_INTERFACE_MODE_5GBASER:
> case PHY_INTERFACE_MODE_10GBASER:
> break;
> default:
> @@ -208,26 +343,25 @@ static int bcm84881_read_status(struct phy_device *phydev)
> */
> val = phy_read_mmd(phydev, MDIO_MMD_VEND1, 0x4011);
> mode = (val & 0x1e) >> 1;
> - if (mode == 1 || mode == 2)
> - phydev->interface = PHY_INTERFACE_MODE_SGMII;
> - else if (mode == 3)
> - phydev->interface = PHY_INTERFACE_MODE_10GBASER;
> - else if (mode == 4)
> - phydev->interface = PHY_INTERFACE_MODE_2500BASEX;
> switch (mode & 7) {
> case 1:
It would be a good idea to get some defines for these numbers.
> + phydev->interface = PHY_INTERFACE_MODE_SGMII;
> phydev->speed = SPEED_100;
> break;
> case 2:
> + phydev->interface = PHY_INTERFACE_MODE_SGMII;
> phydev->speed = SPEED_1000;
> break;
> case 3:
> + phydev->interface = PHY_INTERFACE_MODE_10GBASER;
> phydev->speed = SPEED_10000;
> break;
> case 4:
> + phydev->interface = PHY_INTERFACE_MODE_2500BASEX;
> phydev->speed = SPEED_2500;
> break;
> case 5:
> + phydev->interface = PHY_INTERFACE_MODE_5GBASER;
> phydev->speed = SPEED_5000;
> break;
> }
> @@ -246,11 +380,970 @@ static unsigned int bcm84881_inband_caps(struct phy_device *phydev,
>
> static struct phy_driver bcm84881_drivers[] = {
> {
> - .phy_id = 0xae025150,
> - .phy_id_mask = 0xfffffff0,
> - .name = "Broadcom BCM84881",
> + PHY_ID_MATCH_EXACT(PHYID_BCM4912),
> + .name = "Broadcom BCM4912",
> + .inband_caps = bcm84881_inband_caps,
> + .config_init = bcm84881_config_init_2500,
> + .probe = bcm84881_probe,
> + .get_features = bcm84881_get_features,
> + .config_aneg = bcm84881_config_aneg,
> + .aneg_done = bcm84881_aneg_done,
> + .read_status = bcm84881_read_status,
Please create a macro that simplifies the creation of such entries:
#define BCM84881_PHY_ENTRY(id, speed) \
{ \
PHY_ID_MATCH_EXACT(PHYID_##id), \
.name = "Broadcom BCM" # __stringify(id), \
.inband_caps = bcm84881_inband_caps, \
.config_init = bcm84881_config_init_## speed, \
.probe = bcm84881_probe, \
.get_Features = bcm84881_get_features, \
.aneg_done = bcm84881_aneg_done, \
.read_status = bcm84881_read_status, \
}
And then it just becomes:
BCM84881_PHY_ENTRY(4912, 2500),
BCM84881_PHY_ENTRY(4912M, 2500),
etc.
--
Florian
Powered by blists - more mailing lists