| Message ID | 20250323225439.32400-1-ansuelsmth@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net-next,1/2] net: phy: Add support for new Aeonsemi PHYs | expand |
Hi Christian, kernel test robot noticed the following build warnings: [auto build test WARNING on net-next/main] url: https://github.com/intel-lab-lkp/linux/commits/Christian-Marangi/dt-bindings-net-Document-support-for-Aeonsemi-PHYs/20250324-065920 base: net-next/main patch link: https://lore.kernel.org/r/20250323225439.32400-1-ansuelsmth%40gmail.com patch subject: [net-next PATCH 1/2] net: phy: Add support for new Aeonsemi PHYs config: hexagon-allyesconfig (https://download.01.org/0day-ci/archive/20250324/202503241125.KKDZ7C9F-lkp@intel.com/config) compiler: clang version 21.0.0git (https://github.com/llvm/llvm-project c2692afc0a92cd5da140dfcdfff7818a5b8ce997) reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250324/202503241125.KKDZ7C9F-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202503241125.KKDZ7C9F-lkp@intel.com/ All warnings (new ones prefixed by >>): >> drivers/net/phy/as21xxx.c:767:14: warning: variable 'val' is used uninitialized whenever 'for' loop exits because its condition is false [-Wsometimes-uninitialized] 767 | for (i = 0; i < ARRAY_SIZE(as21xxx_led_supported_pattern); i++) | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ drivers/net/phy/as21xxx.c:775:33: note: uninitialized use occurs here 775 | VEND1_LED_REG_A_EVENT, val); | ^~~ drivers/net/phy/as21xxx.c:767:14: note: remove the condition if it is always true 767 | for (i = 0; i < ARRAY_SIZE(as21xxx_led_supported_pattern); i++) | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ drivers/net/phy/as21xxx.c:761:9: note: initialize the variable 'val' to silence this warning 761 | u16 val; | ^ | = 0 1 warning generated. vim +767 drivers/net/phy/as21xxx.c 757 758 static int as21xxx_led_hw_control_set(struct phy_device *phydev, u8 index, 759 unsigned long rules) 760 { 761 u16 val; 762 int i; 763 764 if (index > AEON_MAX_LDES) 765 return -EINVAL; 766 > 767 for (i = 0; i < ARRAY_SIZE(as21xxx_led_supported_pattern); i++) 768 if (rules == as21xxx_led_supported_pattern[i].pattern) { 769 val = as21xxx_led_supported_pattern[i].val; 770 break; 771 } 772 773 return phy_modify_mmd(phydev, MDIO_MMD_VEND1, 774 VEND1_LED_REG(index), 775 VEND1_LED_REG_A_EVENT, val); 776 } 777
> Supported PHYs AS21011JB1, AS21011PB1, AS21010JB1, AS21010PB1, > AS21511JB1, AS21511PB1, AS21510JB1, AS21510PB1, AS21210JB1, > AS21210PB1 that all register with the PHY ID 0x7500 0x7500 > before the firmware is loaded. Does the value change after the firmware is loaded? Is the same firmware used for all variants? > +++ b/drivers/net/phy/Kconfig > @@ -121,6 +121,18 @@ config AMCC_QT2025_PHY > > source "drivers/net/phy/aquantia/Kconfig" > > +config AS21XXX_PHY > + tristate "Aeonsemi AS21xxx PHYs" The sorting is based on the tristate value, so that when you look at 'make menuconfig' the menu is in alphabetical order. So this goes before aquantia. > +/* 5 LED at step of 0x20 > + * FE: Fast-Ethernet (100) > + * GE: Gigabit-Ethernet (1000) > + * NG: New-Generation (2500/5000/10000) > + * (Lovely ChatGPT managed to translate meaning of NG) It might be a reference to NBase-T Gigabit. Please add a comment somewhere about how locking works for IPCs. As far as i see, the current locking scheme is that IPCs are only called from probe, so no locking is actually required. But: > +#define IPC_CMD_NG_TESTMODE 0x1b /* Set NG test mode and tone */ > +#define IPC_CMD_TEMP_MON 0x15 /* Temperature monitoring function */ > +#define IPC_CMD_SET_LED 0x23 /* Set led */ suggests IPCs might in the future be needed outside of probe, and then a different locking scheme might be needed, particularly for temperature monitoring. > +static int as21xxx_get_features(struct phy_device *phydev) > +{ > + int ret; > + > + ret = genphy_read_abilities(phydev); > + if (ret) > + return ret; > + > + /* AS21xxx supports 100M/1G/2.5G/5G/10G speed. */ > + linkmode_clear_bit(ETHTOOL_LINK_MODE_10baseT_Half_BIT, > + phydev->supported); > + linkmode_clear_bit(ETHTOOL_LINK_MODE_10baseT_Full_BIT, > + phydev->supported); > + linkmode_clear_bit(ETHTOOL_LINK_MODE_100baseT_Half_BIT, > + phydev->supported); Does this mean the registers genphy_read_abilities() reads are broken and report link modes it does not actually support? > + linkmode_set_bit(ETHTOOL_LINK_MODE_100baseT_Full_BIT, > + phydev->supported); > + linkmode_set_bit(ETHTOOL_LINK_MODE_1000baseT_Full_BIT, > + phydev->supported); and it is also not reporting modes it does actually support? Is genphy_read_abilities() actually doing anything useful? Some more comments would be good here. > + linkmode_set_bit(ETHTOOL_LINK_MODE_2500baseT_Full_BIT, > + phydev->supported); > + linkmode_set_bit(ETHTOOL_LINK_MODE_5000baseT_Full_BIT, > + phydev->supported); > + linkmode_set_bit(ETHTOOL_LINK_MODE_10000baseT_Full_BIT, > + phydev->supported); Does this mean genphy_c45_pma_read_abilities() also returns the wrong values? > +static int as21xxx_read_link(struct phy_device *phydev, int *bmcr) > +{ > + int status; > + > + *bmcr = phy_read_mmd(phydev, MDIO_MMD_AN, > + MDIO_AN_C22 + MII_BMCR); > + if (*bmcr < 0) > + return *bmcr; > + > + /* Autoneg is being started, therefore disregard current > + * link status and report link as down. > + */ > + if (*bmcr & BMCR_ANRESTART) { > + phydev->link = 0; > + return 0; > + } > + > + status = phy_read_mmd(phydev, MDIO_MMD_AN, MDIO_STAT1); No MDIO_AN_C22 + here? Maybe add a comment about which C22 registers are mapped into C45 space. Andrew
On Mon, Mar 24, 2025 at 03:03:51PM +0100, Andrew Lunn wrote: > > Supported PHYs AS21011JB1, AS21011PB1, AS21010JB1, AS21010PB1, > > AS21511JB1, AS21511PB1, AS21510JB1, AS21510PB1, AS21210JB1, > > AS21210PB1 that all register with the PHY ID 0x7500 0x7500 > > before the firmware is loaded. > > Does the value change after the firmware is loaded? Is the same > firmware used for all variants? > Yes It does... Can PHY subsystem react on this? Like probe for the generic one and then use the OPs for the real PHY ID? > > +++ b/drivers/net/phy/Kconfig > > @@ -121,6 +121,18 @@ config AMCC_QT2025_PHY > > > > source "drivers/net/phy/aquantia/Kconfig" > > > > +config AS21XXX_PHY > > + tristate "Aeonsemi AS21xxx PHYs" > > The sorting is based on the tristate value, so that when you look at > 'make menuconfig' the menu is in alphabetical order. So this goes > before aquantia. > Tought it was only alphabetical, will move. > > +/* 5 LED at step of 0x20 > > + * FE: Fast-Ethernet (100) > > + * GE: Gigabit-Ethernet (1000) > > + * NG: New-Generation (2500/5000/10000) > > + * (Lovely ChatGPT managed to translate meaning of NG) > > It might be a reference to NBase-T Gigabit. > A better LED table was provided for this confirming ChatGPT assumption. Will update the LED pattern as there were some mistake for activity blink. Also confirmed no way to make the LED only blink... It's sad cause the blink time can be configured... > Please add a comment somewhere about how locking works for IPCs. As > far as i see, the current locking scheme is that IPCs are only called > from probe, so no locking is actually required. But: > > > +#define IPC_CMD_NG_TESTMODE 0x1b /* Set NG test mode and tone */ > > +#define IPC_CMD_TEMP_MON 0x15 /* Temperature monitoring function */ > > +#define IPC_CMD_SET_LED 0x23 /* Set led */ > > suggests IPCs might in the future be needed outside of probe, and then > a different locking scheme might be needed, particularly for > temperature monitoring. > Indeed I will push temperature monitor in a followup patch so yes I will add locking just for preparation of the additional feature. > > +static int as21xxx_get_features(struct phy_device *phydev) > > +{ > > + int ret; > > + > > + ret = genphy_read_abilities(phydev); > > + if (ret) > > + return ret; > > + > > + /* AS21xxx supports 100M/1G/2.5G/5G/10G speed. */ > > + linkmode_clear_bit(ETHTOOL_LINK_MODE_10baseT_Half_BIT, > > + phydev->supported); > > + linkmode_clear_bit(ETHTOOL_LINK_MODE_10baseT_Full_BIT, > > + phydev->supported); > > + linkmode_clear_bit(ETHTOOL_LINK_MODE_100baseT_Half_BIT, > > + phydev->supported); > > Does this mean the registers genphy_read_abilities() reads are broken > and report link modes it does not actually support? > > > + linkmode_set_bit(ETHTOOL_LINK_MODE_100baseT_Full_BIT, > > + phydev->supported); > > + linkmode_set_bit(ETHTOOL_LINK_MODE_1000baseT_Full_BIT, > > + phydev->supported); > > and it is also not reporting modes it does actually support? Is > genphy_read_abilities() actually doing anything useful? Some more > comments would be good here. > > > + linkmode_set_bit(ETHTOOL_LINK_MODE_2500baseT_Full_BIT, > > + phydev->supported); > > + linkmode_set_bit(ETHTOOL_LINK_MODE_5000baseT_Full_BIT, > > + phydev->supported); > > + linkmode_set_bit(ETHTOOL_LINK_MODE_10000baseT_Full_BIT, > > + phydev->supported); > > Does this mean genphy_c45_pma_read_abilities() also returns the wrong > values? > Honestly I followed what the SDK driver did for the get_feature. I will check the register making sure correct bits are reported. Looking at the get_status It might be conflicting as they map C22 in C45 vendor regs. > > +static int as21xxx_read_link(struct phy_device *phydev, int *bmcr) > > +{ > > + int status; > > + > > + *bmcr = phy_read_mmd(phydev, MDIO_MMD_AN, > > + MDIO_AN_C22 + MII_BMCR); > > + if (*bmcr < 0) > > + return *bmcr; > > + > > + /* Autoneg is being started, therefore disregard current > > + * link status and report link as down. > > + */ > > + if (*bmcr & BMCR_ANRESTART) { > > + phydev->link = 0; > > + return 0; > > + } > > + > > + status = phy_read_mmd(phydev, MDIO_MMD_AN, MDIO_STAT1); > > No MDIO_AN_C22 + here? Maybe add a comment about which C22 registers > are mapped into C45 space. > Nope, not a typo... They took the vendor route for some register but for BMSR they used the expected one. Just to make it clear, using the AN_C22 wasn't a choice... the C45 BMCR reports inconsistent data compared to the vendor C22 one.
First, please include a cover message whenever you send a patch series. On Sun, Mar 23, 2025 at 11:54:26PM +0100, Christian Marangi wrote: > Add support for new Aeonsemi 10G C45 PHYs. These PHYs intergate an IPC > to setup some configuration and require special handling to sync with > the parity bit. The parity bit is a way the IPC use to follow correct > order of command sent. > > Supported PHYs AS21011JB1, AS21011PB1, AS21010JB1, AS21010PB1, > AS21511JB1, AS21511PB1, AS21510JB1, AS21510PB1, AS21210JB1, > AS21210PB1 that all register with the PHY ID 0x7500 0x7500 > before the firmware is loaded. Hmm. That behaviour is really not nice for the kernel to deal with. C45 PHYs have multiple IDs (there's registers 2,3 and also 14,15, each is per MMD). Do they all have the same value? Do any of them indicate any kind of valid OUI ? If there is no way to sanely detect this PHY, then I would suggest that it is beyond the ability of the kernel, and at the very least, an initial firmware version needs to be loaded by board boot firmware so the PHY _can_ be properly identified. Basically, it isn't the kernel's job to fix such broken hardware. > +#define VEND1_LED_REG(_n) (0x1800 + ((_n) * 0x10)) > +#define VEND1_LED_REG_A_EVENT GENMASK(15, 11) > +#define VEND1_LED_REG_A_EVENT_ON_10 FIELD_PREP_CONST(VEND1_LED_REG_A_EVENT, 0x0) > +#define VEND1_LED_REG_A_EVENT_ON_100 FIELD_PREP_CONST(VEND1_LED_REG_A_EVENT, 0x1) I really don't like the pattern of "define constants using FIELD_PREP*()". It seems to me it misses the entire point of the bitfield macros, which is to prepare and extract bitfields. When I see: swith (foo & BLAH_MASK) { case BLAH_OPTION_1: ... where BLAH_OPTION_1 is defined using FIELD_PREP*(), it just makes me shudder. SWITCH (FIELD_GET(BLAH_MASK, foo)) { case BLAH_OPTION_1: ... where BLAH_OPTION_1 is defined as the numerical field value is much more how the bitfield stuff is supposed to be used. > +enum { > + MDIO_AN_C22 = 0xffe0, I'd suggest defining this in a driver private namespace, rather than using the MDIO_xxx which is used by linux/mdio.h > + /* Exit condition logic: > + * - Wait for parity bit equal > + * - Wait for status success, error OR ready > + */ > + ret = phy_read_mmd_poll_timeout(phydev, MDIO_MMD_VEND1, VEND1_IPC_STS, val, > + FIELD_GET(AEON_IPC_STS_PARITY, val) == > + curr_parity && > + (val & AEON_IPC_STS_STATUS) != > + AEON_IPC_STS_STATUS_RCVD && > + (val & AEON_IPC_STS_STATUS) != > + AEON_IPC_STS_STATUS_PROCESS && > + (val & AEON_IPC_STS_STATUS) != > + AEON_IPC_STS_STATUS_BUSY, > + 10000, 2000000, false); Use an inline function, and also please wrap a bit tighter, val seems to wrap. ret = phy_read_mmd_poll_timeout(phydev, MDIO_MMD_VEND1, VEND1_IPC_STS, val, aeon_cmd_done(curr_parity, val), 10000, 2000000, false); > + if (ret) > + return ret; > + > + *ret_sts = val; > + if ((val & AEON_IPC_STS_STATUS) != AEON_IPC_STS_STATUS_SUCCESS) > + return -EFAULT; EFAULT means "Bad address". Does not returning successful status mean that there was a bad address? If not, please don't do this. EFAULT is specifically used to return to userspace to tell it that it passed the kernel a bad address. > + > + return 0; > +} > + > +static int aeon_ipc_send_msg(struct phy_device *phydev, u16 opcode, > + u16 *data, unsigned int data_len, u16 *ret_sts) > +{ > + u32 cmd; > + int ret; > + int i; > + > + if (data_len > AEON_IPC_DATA_MAX) > + return -EINVAL; > + > + for (i = 0; i < data_len / sizeof(u16); i++) > + phy_write_mmd(phydev, MDIO_MMD_VEND1, VEND1_IPC_DATA(i), > + data[i]); What ensures that this won't overflow the number of registers? > + > + cmd = FIELD_PREP(AEON_IPC_CMD_SIZE, data_len) | > + FIELD_PREP(AEON_IPC_CMD_OPCODE, opcode); > + ret = aeon_ipc_send_cmd(phydev, cmd, ret_sts); > + if (ret) > + phydev_err(phydev, "failed to send ipc msg for %x: %d\n", opcode, ret); > + > + return ret; > +} > + > +static int aeon_ipc_rcv_msg(struct phy_device *phydev, u16 ret_sts, > + u16 *data) > +{ > + unsigned int size = FIELD_GET(AEON_IPC_STS_SIZE, ret_sts); > + int ret; > + int i; > + > + if ((ret_sts & AEON_IPC_STS_STATUS) == AEON_IPC_STS_STATUS_ERROR) > + return -EINVAL; > + > + for (i = 0; i < DIV_ROUND_UP(size, sizeof(u16)); i++) { > + ret = phy_read_mmd(phydev, MDIO_MMD_VEND1, VEND1_IPC_DATA(i)); > + if (ret < 0) > + return ret; > + > + data[i] = ret; Unsafe. AEON_IPC_STS_SIZE is 5 bits in size, which means this can write indexes 0..31. You pass in a buffer of 8 u16's on the stack. What stops the hardware engaging in stack smashing... nothing. Please code more carefully. > + } > + > + return size; > +} > + > +/* Logic to sync parity bit with IPC. > + * We send 2 NOP cmd with same partity and we wait for IPC > + * to handle the packet only for the second one. This way > + * we make sure we are sync for every next cmd. > + */ > +static int aeon_ipc_sync_parity(struct phy_device *phydev) > +{ > + struct as21xxx_priv *priv = phydev->priv; > + u16 ret_sts; > + u32 cmd; > + int ret; > + > + /* Send NOP with no parity */ > + cmd = FIELD_PREP(AEON_IPC_CMD_SIZE, 0) | > + FIELD_PREP(AEON_IPC_CMD_OPCODE, IPC_CMD_NOOP); > + aeon_ipc_send_cmd(phydev, cmd, NULL); > + > + /* Reset packet parity */ > + priv->parity_status = false; > + > + /* Send second NOP with no parity */ > + ret = aeon_ipc_send_cmd(phydev, cmd, &ret_sts); > + /* We expect to return -EFAULT */ > + if (ret != -EFAULT) > + return ret; > + > + if ((ret_sts & AEON_IPC_STS_STATUS) != AEON_IPC_STS_STATUS_READY) > + return -EINVAL; > + > + return 0; > +} > + > +static int aeon_ipc_get_fw_version(struct phy_device *phydev) > +{ > + u16 ret_data[8], data[1]; > + u16 ret_sts; > + int ret; > + > + data[0] = IPC_INFO_VERSION; > + ret = aeon_ipc_send_msg(phydev, IPC_CMD_INFO, data, sizeof(data), > + &ret_sts); > + if (ret) > + return ret; > + > + ret = aeon_ipc_rcv_msg(phydev, ret_sts, ret_data); > + if (ret < 0) > + return ret; > + > + phydev_info(phydev, "Firmware Version: %s\n", (char *)ret_data); > + > + return 0; > +} > + > +static int aeon_dpc_ra_enable(struct phy_device *phydev) > +{ > + u16 data[2]; > + u16 ret_sts; > + > + data[0] = IPC_CFG_PARAM_DIRECT; > + data[1] = IPC_CFG_PARAM_DIRECT_DPC_RA; > + > + return aeon_ipc_send_msg(phydev, IPC_CMD_CFG_PARAM, data, > + sizeof(data), &ret_sts); > +} > + > +static int as21xxx_probe(struct phy_device *phydev) > +{ > + struct as21xxx_priv *priv; > + int ret; > + > + phydev->priv = devm_kzalloc(&phydev->mdio.dev, > + sizeof(*priv), GFP_KERNEL); > + if (!phydev->priv) > + return -ENOMEM; > + > + ret = aeon_firmware_load(phydev); > + if (ret) > + return ret; > + > + ret = aeon_ipc_sync_parity(phydev); > + if (ret) > + return ret; > + > + /* Enable PTP clk if not already Enabled */ > + ret = phy_set_bits_mmd(phydev, MDIO_MMD_VEND1, VEND1_PTP_CLK, > + VEND1_PTP_CLK_EN); > + if (ret) > + return ret; > + > + ret = aeon_dpc_ra_enable(phydev); > + if (ret) > + return ret; > + > + ret = aeon_ipc_get_fw_version(phydev); > + if (ret) > + return ret; > + > + return 0; > +} > + > +static int as21xxx_get_features(struct phy_device *phydev) > +{ > + int ret; > + > + ret = genphy_read_abilities(phydev); > + if (ret) > + return ret; > + > + /* AS21xxx supports 100M/1G/2.5G/5G/10G speed. */ > + linkmode_clear_bit(ETHTOOL_LINK_MODE_10baseT_Half_BIT, > + phydev->supported); > + linkmode_clear_bit(ETHTOOL_LINK_MODE_10baseT_Full_BIT, > + phydev->supported); > + linkmode_clear_bit(ETHTOOL_LINK_MODE_100baseT_Half_BIT, > + phydev->supported); > + linkmode_set_bit(ETHTOOL_LINK_MODE_100baseT_Full_BIT, > + phydev->supported); > + linkmode_set_bit(ETHTOOL_LINK_MODE_1000baseT_Full_BIT, > + phydev->supported); > + linkmode_set_bit(ETHTOOL_LINK_MODE_2500baseT_Full_BIT, > + phydev->supported); > + linkmode_set_bit(ETHTOOL_LINK_MODE_5000baseT_Full_BIT, > + phydev->supported); > + linkmode_set_bit(ETHTOOL_LINK_MODE_10000baseT_Full_BIT, > + phydev->supported); Given all this, does genphy_read_abilities() actually read anything useful from the PHY? > +static struct phy_driver as21xxx_drivers[] = { > + { > + PHY_ID_MATCH_EXACT(PHY_ID_AS21XXX), > + .name = "Aeonsemi AS21xxx", > + .probe = as21xxx_probe, > + .get_features = as21xxx_get_features, > + .read_status = as21xxx_read_status, > + .led_brightness_set = as21xxx_led_brightness_set, > + .led_hw_is_supported = as21xxx_led_hw_is_supported, > + .led_hw_control_set = as21xxx_led_hw_control_set, > + .led_hw_control_get = as21xxx_led_hw_control_get, > + .led_polarity_set = as21xxx_led_polarity_set, > + }, What if firmware was already loaded? I think you implied that this ID is only present when firmware hasn't been loaded. Thanks.
On Mon, Mar 24, 2025 at 03:16:08PM +0100, Christian Marangi wrote: > On Mon, Mar 24, 2025 at 03:03:51PM +0100, Andrew Lunn wrote: > > > Supported PHYs AS21011JB1, AS21011PB1, AS21010JB1, AS21010PB1, > > > AS21511JB1, AS21511PB1, AS21510JB1, AS21510PB1, AS21210JB1, > > > AS21210PB1 that all register with the PHY ID 0x7500 0x7500 > > > before the firmware is loaded. > > > > Does the value change after the firmware is loaded? Is the same > > firmware used for all variants? > > > > Yes It does... Can PHY subsystem react on this? Like probe for the > generic one and then use the OPs for the real PHY ID? Multiple thoughts here.... If the firmware is in SPI, i assume by the time the MDIO bus is probed, the PHY has its 'real' IDs. So you need entries for those as well as the dummy ID. I think this is the first PHY which changes its IDs at runtime. So we don't have a generic solution to this. USB and PCI probably have support for this, so maybe there is something we can copy. It could be they support hotplug, so the device disappears and returns. That is not really something we have in MDIO. So i don't know if we can reuse ideas from there. When the firmware is running, do the different variants all share the same ID? Is there a way to tell them apart. We always have phy_driver->match_phy_device(). It could look for 0x75007500, download the firmware, and then match on the real IDs. > > > +++ b/drivers/net/phy/Kconfig > > > @@ -121,6 +121,18 @@ config AMCC_QT2025_PHY > > > > > > source "drivers/net/phy/aquantia/Kconfig" > > > > > > +config AS21XXX_PHY > > > + tristate "Aeonsemi AS21xxx PHYs" > > > > The sorting is based on the tristate value, so that when you look at > > 'make menuconfig' the menu is in alphabetical order. So this goes > > before aquantia. > > > > Tought it was only alphabetical, will move. Yes, it is not obvious, it should have a comment added at the top. But the top is so far away, i guess many developers would miss it anyway. > > > +static int as21xxx_get_features(struct phy_device *phydev) > > > +{ > > > + int ret; > > > + > > > + ret = genphy_read_abilities(phydev); > > > + if (ret) > > > + return ret; > > > + > > > + /* AS21xxx supports 100M/1G/2.5G/5G/10G speed. */ > > > + linkmode_clear_bit(ETHTOOL_LINK_MODE_10baseT_Half_BIT, > > > + phydev->supported); > > > + linkmode_clear_bit(ETHTOOL_LINK_MODE_10baseT_Full_BIT, > > > + phydev->supported); > > > + linkmode_clear_bit(ETHTOOL_LINK_MODE_100baseT_Half_BIT, > > > + phydev->supported); > > > > Does this mean the registers genphy_read_abilities() reads are broken > > and report link modes it does not actually support? > > > > > + linkmode_set_bit(ETHTOOL_LINK_MODE_100baseT_Full_BIT, > > > + phydev->supported); > > > + linkmode_set_bit(ETHTOOL_LINK_MODE_1000baseT_Full_BIT, > > > + phydev->supported); > > > > and it is also not reporting modes it does actually support? Is > > genphy_read_abilities() actually doing anything useful? Some more > > comments would be good here. > > > > > + linkmode_set_bit(ETHTOOL_LINK_MODE_2500baseT_Full_BIT, > > > + phydev->supported); > > > + linkmode_set_bit(ETHTOOL_LINK_MODE_5000baseT_Full_BIT, > > > + phydev->supported); > > > + linkmode_set_bit(ETHTOOL_LINK_MODE_10000baseT_Full_BIT, > > > + phydev->supported); > > > > Does this mean genphy_c45_pma_read_abilities() also returns the wrong > > values? > > > > Honestly I followed what the SDK driver did for the get_feature. I will > check the register making sure correct bits are reported. > > Looking at the get_status It might be conflicting as they map C22 in C45 > vendor regs. If all the registers used for automatic feature detection are broken, just comment on it and don't use genphy_read_abilities() etc. > > > +static int as21xxx_read_link(struct phy_device *phydev, int *bmcr) > > > +{ > > > + int status; > > > + > > > + *bmcr = phy_read_mmd(phydev, MDIO_MMD_AN, > > > + MDIO_AN_C22 + MII_BMCR); > > > + if (*bmcr < 0) > > > + return *bmcr; > > > + > > > + /* Autoneg is being started, therefore disregard current > > > + * link status and report link as down. > > > + */ > > > + if (*bmcr & BMCR_ANRESTART) { > > > + phydev->link = 0; > > > + return 0; > > > + } > > > + > > > + status = phy_read_mmd(phydev, MDIO_MMD_AN, MDIO_STAT1); > > > > No MDIO_AN_C22 + here? Maybe add a comment about which C22 registers > > are mapped into C45 space. > > > > Nope, not a typo... They took the vendor route for some register but for > BMSR they used the expected one. > > Just to make it clear, using the AN_C22 wasn't a choice... the C45 BMCR > reports inconsistent data compared to the vendor C22 one. Comments would be good. Is there an open datasheet for this device? Andrew
On Mon, Mar 24, 2025 at 04:16:09PM +0100, Andrew Lunn wrote: > On Mon, Mar 24, 2025 at 03:16:08PM +0100, Christian Marangi wrote: > > On Mon, Mar 24, 2025 at 03:03:51PM +0100, Andrew Lunn wrote: > > > > Supported PHYs AS21011JB1, AS21011PB1, AS21010JB1, AS21010PB1, > > > > AS21511JB1, AS21511PB1, AS21510JB1, AS21510PB1, AS21210JB1, > > > > AS21210PB1 that all register with the PHY ID 0x7500 0x7500 > > > > before the firmware is loaded. > > > > > > Does the value change after the firmware is loaded? Is the same > > > firmware used for all variants? > > > > > > > Yes It does... Can PHY subsystem react on this? Like probe for the > > generic one and then use the OPs for the real PHY ID? > > Multiple thoughts here.... > > If the firmware is in SPI, i assume by the time the MDIO bus is > probed, the PHY has its 'real' IDs. So you need entries for those as > well as the dummy ID. > > I think this is the first PHY which changes its IDs at runtime. So we > don't have a generic solution to this. USB and PCI probably have > support for this, so maybe there is something we can copy. It could be > they support hotplug, so the device disappears and returns. That is > not really something we have in MDIO. So i don't know if we can reuse > ideas from there. > > When the firmware is running, do the different variants all share the > same ID? Is there a way to tell them apart. We always have > phy_driver->match_phy_device(). It could look for 0x75007500, download > the firmware, and then match on the real IDs. > Ok update on this... The PHY report 7500 7500 but on enabling PTP clock, a more specific ""family"" ID is filled in MMD that is 0x7500 0x9410. They all use the same firmware so matching for the family ID might not be a bad idea... The alternative is either load the firmware in match_phy_device or introduce some additional OPs to handle this correctly... Considering how the thing are evolving with PHY I really feel it's time we start introducing specific OP for firmware loading and we might call this OP before PHY ID matching is done (or maybe do it again). Lets see how bad the thing goes API wise tho. > > > > +++ b/drivers/net/phy/Kconfig > > > > @@ -121,6 +121,18 @@ config AMCC_QT2025_PHY > > > > > > > > source "drivers/net/phy/aquantia/Kconfig" > > > > > > > > +config AS21XXX_PHY > > > > + tristate "Aeonsemi AS21xxx PHYs" > > > > > > The sorting is based on the tristate value, so that when you look at > > > 'make menuconfig' the menu is in alphabetical order. So this goes > > > before aquantia. > > > > > > > Tought it was only alphabetical, will move. > > Yes, it is not obvious, it should have a comment added at the top. > But the top is so far away, i guess many developers would miss it > anyway. > > > > > +static int as21xxx_get_features(struct phy_device *phydev) > > > > +{ > > > > + int ret; > > > > + > > > > + ret = genphy_read_abilities(phydev); > > > > + if (ret) > > > > + return ret; > > > > + > > > > + /* AS21xxx supports 100M/1G/2.5G/5G/10G speed. */ > > > > + linkmode_clear_bit(ETHTOOL_LINK_MODE_10baseT_Half_BIT, > > > > + phydev->supported); > > > > + linkmode_clear_bit(ETHTOOL_LINK_MODE_10baseT_Full_BIT, > > > > + phydev->supported); > > > > + linkmode_clear_bit(ETHTOOL_LINK_MODE_100baseT_Half_BIT, > > > > + phydev->supported); > > > > > > Does this mean the registers genphy_read_abilities() reads are broken > > > and report link modes it does not actually support? > > > > > > > + linkmode_set_bit(ETHTOOL_LINK_MODE_100baseT_Full_BIT, > > > > + phydev->supported); > > > > + linkmode_set_bit(ETHTOOL_LINK_MODE_1000baseT_Full_BIT, > > > > + phydev->supported); > > > > > > and it is also not reporting modes it does actually support? Is > > > genphy_read_abilities() actually doing anything useful? Some more > > > comments would be good here. > > > > > > > + linkmode_set_bit(ETHTOOL_LINK_MODE_2500baseT_Full_BIT, > > > > + phydev->supported); > > > > + linkmode_set_bit(ETHTOOL_LINK_MODE_5000baseT_Full_BIT, > > > > + phydev->supported); > > > > + linkmode_set_bit(ETHTOOL_LINK_MODE_10000baseT_Full_BIT, > > > > + phydev->supported); > > > > > > Does this mean genphy_c45_pma_read_abilities() also returns the wrong > > > values? > > > > > > > Honestly I followed what the SDK driver did for the get_feature. I will > > check the register making sure correct bits are reported. > > > > Looking at the get_status It might be conflicting as they map C22 in C45 > > vendor regs. > > If all the registers used for automatic feature detection are broken, > just comment on it and don't use genphy_read_abilities() etc. > > > > > +static int as21xxx_read_link(struct phy_device *phydev, int *bmcr) > > > > +{ > > > > + int status; > > > > + > > > > + *bmcr = phy_read_mmd(phydev, MDIO_MMD_AN, > > > > + MDIO_AN_C22 + MII_BMCR); > > > > + if (*bmcr < 0) > > > > + return *bmcr; > > > > + > > > > + /* Autoneg is being started, therefore disregard current > > > > + * link status and report link as down. > > > > + */ > > > > + if (*bmcr & BMCR_ANRESTART) { > > > > + phydev->link = 0; > > > > + return 0; > > > > + } > > > > + > > > > + status = phy_read_mmd(phydev, MDIO_MMD_AN, MDIO_STAT1); > > > > > > No MDIO_AN_C22 + here? Maybe add a comment about which C22 registers > > > are mapped into C45 space. > > > > > > > Nope, not a typo... They took the vendor route for some register but for > > BMSR they used the expected one. > > > > Just to make it clear, using the AN_C22 wasn't a choice... the C45 BMCR > > reports inconsistent data compared to the vendor C22 one. > > Comments would be good. > > Is there an open datasheet for this device? > Sadly no and I don't even have datasheet, just some contacts to confirm 1-2 thing for the PHY.
On Tue, Mar 25, 2025 at 01:04:30PM +0100, Christian Marangi wrote: > On Mon, Mar 24, 2025 at 04:16:09PM +0100, Andrew Lunn wrote: > > On Mon, Mar 24, 2025 at 03:16:08PM +0100, Christian Marangi wrote: > > > On Mon, Mar 24, 2025 at 03:03:51PM +0100, Andrew Lunn wrote: > > > > > Supported PHYs AS21011JB1, AS21011PB1, AS21010JB1, AS21010PB1, > > > > > AS21511JB1, AS21511PB1, AS21510JB1, AS21510PB1, AS21210JB1, > > > > > AS21210PB1 that all register with the PHY ID 0x7500 0x7500 > > > > > before the firmware is loaded. Do you have details of how these different PHY differ? Do they have different features? > Ok update on this... The PHY report 7500 7500 but on enabling PTP clock, > a more specific ""family"" ID is filled in MMD that is 0x7500 0x9410. Do they all support PTP? > They all use the same firmware so matching for the family ID might not > be a bad idea... The alternative is either load the firmware in > match_phy_device or introduce some additional OPs to handle this > correctly... > > Considering how the thing are evolving with PHY I really feel it's time > we start introducing specific OP for firmware loading and we might call > this OP before PHY ID matching is done (or maybe do it again). You cannot download firmware before doing some sort of match, because you have no idea what PHY you actually have until you do a match, and if the PHY needs firmware. match_phy_device() gives you a bit more flexibility. It will be called for every PHY on the board, independent of the ID registers. So you can read the ID registers, see if it is at least a vendor you know how to download firmware to, do the download, and then look at the ID registers again to see if it is the version of the PHY you want to drive. If not, return -ENODEV, and the core will try the next driver entry. Andrew
On Tue, Mar 25, 2025 at 09:33:26PM +0100, Andrew Lunn wrote: > On Tue, Mar 25, 2025 at 01:04:30PM +0100, Christian Marangi wrote: > > On Mon, Mar 24, 2025 at 04:16:09PM +0100, Andrew Lunn wrote: > > > On Mon, Mar 24, 2025 at 03:16:08PM +0100, Christian Marangi wrote: > > > > On Mon, Mar 24, 2025 at 03:03:51PM +0100, Andrew Lunn wrote: > > > > > > Supported PHYs AS21011JB1, AS21011PB1, AS21010JB1, AS21010PB1, > > > > > > AS21511JB1, AS21511PB1, AS21510JB1, AS21510PB1, AS21210JB1, > > > > > > AS21210PB1 that all register with the PHY ID 0x7500 0x7500 > > > > > > before the firmware is loaded. > > Do you have details of how these different PHY differ? Do they have > different features? > No but I can ask more details. From what I can assume, gigabit, 2.5g 10g and probably a PHY that expose multiple port (PHY package thing) > > Ok update on this... The PHY report 7500 7500 but on enabling PTP clock, > > a more specific ""family"" ID is filled in MMD that is 0x7500 0x9410. > > Do they all support PTP? > With PTP it's not the PTP we think but I guess it's something internal to the PHY to actually start it. From comments it's called PTP Clock... > > They all use the same firmware so matching for the family ID might not > > be a bad idea... The alternative is either load the firmware in > > match_phy_device or introduce some additional OPs to handle this > > correctly... > > > > Considering how the thing are evolving with PHY I really feel it's time > > we start introducing specific OP for firmware loading and we might call > > this OP before PHY ID matching is done (or maybe do it again). > > You cannot download firmware before doing some sort of match, because > you have no idea what PHY you actually have until you do a match, and > if the PHY needs firmware. > > match_phy_device() gives you a bit more flexibility. It will be called > for every PHY on the board, independent of the ID registers. So you > can read the ID registers, see if it is at least a vendor you know how > to download firmware to, do the download, and then look at the ID > registers again to see if it is the version of the PHY you want to > drive. If not, return -ENODEV, and the core will try the next driver > entry. > I'm finishing preparing V2 and I'm curious what you will think of the implementation. The approach I found works good is permit PHY device to register a second time and the PHY driver toggle this. This way in a PHY driver we register OPs for the to-be-init PHY and then we probe the real one.
On Tue, Mar 25, 2025 at 01:04:30PM +0100, Christian Marangi wrote: > On Mon, Mar 24, 2025 at 04:16:09PM +0100, Andrew Lunn wrote: > > On Mon, Mar 24, 2025 at 03:16:08PM +0100, Christian Marangi wrote: > > > On Mon, Mar 24, 2025 at 03:03:51PM +0100, Andrew Lunn wrote: > > > > > Supported PHYs AS21011JB1, AS21011PB1, AS21010JB1, AS21010PB1, > > > > > AS21511JB1, AS21511PB1, AS21510JB1, AS21510PB1, AS21210JB1, > > > > > AS21210PB1 that all register with the PHY ID 0x7500 0x7500 > > > > > before the firmware is loaded. > > > > > > > > Does the value change after the firmware is loaded? Is the same > > > > firmware used for all variants? > > > > > > > > > > Yes It does... Can PHY subsystem react on this? Like probe for the > > > generic one and then use the OPs for the real PHY ID? > > > > Multiple thoughts here.... > > > > If the firmware is in SPI, i assume by the time the MDIO bus is > > probed, the PHY has its 'real' IDs. So you need entries for those as > > well as the dummy ID. > > > > I think this is the first PHY which changes its IDs at runtime. So we > > don't have a generic solution to this. USB and PCI probably have > > support for this, so maybe there is something we can copy. It could be > > they support hotplug, so the device disappears and returns. That is > > not really something we have in MDIO. So i don't know if we can reuse > > ideas from there. > > > > When the firmware is running, do the different variants all share the > > same ID? Is there a way to tell them apart. We always have > > phy_driver->match_phy_device(). It could look for 0x75007500, download > > the firmware, and then match on the real IDs. > > Ok update on this... The PHY report 7500 7500 but on enabling PTP clock, > a more specific ""family"" ID is filled in MMD that is 0x7500 0x9410. > > They all use the same firmware so matching for the family ID might not > be a bad idea... The alternative is either load the firmware in > match_phy_device or introduce some additional OPs to handle this > correctly... > > Considering how the thing are evolving with PHY I really feel it's time > we start introducing specific OP for firmware loading and we might call > this OP before PHY ID matching is done (or maybe do it again). You're basically talking there about modifying the core driver model of the kernel, which I think you're going to have an uphill battle with. The match_phy_device() op is called by the core driver model when a new device/driver is added to find a driver for an unbound device. It does this calling the bus_type's .match() method for each unbound device on the bus_type, and every device driver that is bound to the bus type. In the case of a MDIO device, which phylib's PHYs are a sub-class of, this is populated with mdio_bus_match(), which then goes on to call the MDIO device's ->bus_match() method. For a PHY, that's phy_bus_match(), which will either call the PHY driver's .match_phy_device() method or will attempt to match the hardware ID against what is given in the PHY driver structure. So, the core driver model is responsible for trying each device driver on the bus to find one that matches each device. This isn't a phylib thing. At the moment, I don't see how adding another PHY driver OP helps. We still need to find the right PHY driver struct somehow. It seems to me to be a chicken-and-egg problem. That's why I suggested that maybe board firmware should be loading at least _some_ kind of firmware to get the PHY to the point that the kernel can properly identify it - the kernel can then read the loaded firmware, compare it with the version that's locally available, and if different, replace the firmware. The main thing is that board firmware gets the PHY to the point that it can be properly identified.
Hi Christian,
kernel test robot noticed the following build warnings:
[auto build test WARNING on net-next/main]
url: https://github.com/intel-lab-lkp/linux/commits/Christian-Marangi/dt-bindings-net-Document-support-for-Aeonsemi-PHYs/20250324-065920
base: net-next/main
patch link: https://lore.kernel.org/r/20250323225439.32400-1-ansuelsmth%40gmail.com
patch subject: [net-next PATCH 1/2] net: phy: Add support for new Aeonsemi PHYs
config: riscv-randconfig-r072-20250329 (https://download.01.org/0day-ci/archive/20250330/202503300205.g0FCozVG-lkp@intel.com/config)
compiler: clang version 19.1.7 (https://github.com/llvm/llvm-project cd708029e0b2869e80abe31ddb175f7c35361f90)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202503300205.g0FCozVG-lkp@intel.com/
smatch warnings:
drivers/net/phy/as21xxx.c:744 as21xxx_led_hw_control_get() warn: unsigned 'val' is never less than zero.
drivers/net/phy/as21xxx.c:775 as21xxx_led_hw_control_set() error: uninitialized symbol 'val'.
drivers/net/phy/as21xxx.c:802 as21xxx_led_polarity_set() error: uninitialized symbol 'led_active_low'.
vim +/val +744 drivers/net/phy/as21xxx.c
733
734 static int as21xxx_led_hw_control_get(struct phy_device *phydev, u8 index,
735 unsigned long *rules)
736 {
737 u16 val;
738 int i;
739
740 if (index > AEON_MAX_LDES)
741 return -EINVAL;
742
743 val = phy_read_mmd(phydev, MDIO_MMD_VEND1, VEND1_LED_REG(index));
> 744 if (val < 0)
745 return val;
746
747 val &= VEND1_LED_REG_A_EVENT;
748 for (i = 0; i < ARRAY_SIZE(as21xxx_led_supported_pattern); i++)
749 if (val == as21xxx_led_supported_pattern[i].val) {
750 *rules = as21xxx_led_supported_pattern[i].pattern;
751 return 0;
752 }
753
754 /* Should be impossible */
755 return -EINVAL;
756 }
757
758 static int as21xxx_led_hw_control_set(struct phy_device *phydev, u8 index,
759 unsigned long rules)
760 {
761 u16 val;
762 int i;
763
764 if (index > AEON_MAX_LDES)
765 return -EINVAL;
766
767 for (i = 0; i < ARRAY_SIZE(as21xxx_led_supported_pattern); i++)
768 if (rules == as21xxx_led_supported_pattern[i].pattern) {
769 val = as21xxx_led_supported_pattern[i].val;
770 break;
771 }
772
773 return phy_modify_mmd(phydev, MDIO_MMD_VEND1,
774 VEND1_LED_REG(index),
> 775 VEND1_LED_REG_A_EVENT, val);
776 }
777
778 static int as21xxx_led_polarity_set(struct phy_device *phydev, int index,
779 unsigned long modes)
780 {
781 bool led_active_low;
782 u16 mask, val = 0;
783 u32 mode;
784
785 if (index > AEON_MAX_LDES)
786 return -EINVAL;
787
788 for_each_set_bit(mode, &modes, __PHY_LED_MODES_NUM) {
789 switch (mode) {
790 case PHY_LED_ACTIVE_LOW:
791 led_active_low = true;
792 break;
793 case PHY_LED_ACTIVE_HIGH: /* default mode */
794 led_active_low = false;
795 break;
796 default:
797 return -EINVAL;
798 }
799 }
800
801 mask = VEND1_GLB_CPU_CTRL_LED_POLARITY(index);
> 802 if (led_active_low)
803 val = VEND1_GLB_CPU_CTRL_LED_POLARITY(index);
804
805 return phy_modify_mmd(phydev, MDIO_MMD_VEND1,
806 VEND1_GLB_REG_CPU_CTRL,
807 mask, val);
808 }
809
diff --git a/MAINTAINERS b/MAINTAINERS index 873aa2cce4d7..9a2df6d221bd 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -645,6 +645,12 @@ F: drivers/iio/accel/adxl380.h F: drivers/iio/accel/adxl380_i2c.c F: drivers/iio/accel/adxl380_spi.c +AEONSEMI PHY DRIVER +M: Christian Marangi <ansuelsmth@gmail.com> +L: netdev@vger.kernel.org +S: Maintained +F: drivers/net/phy/as21xxx.c + AF8133J THREE-AXIS MAGNETOMETER DRIVER M: Ondřej Jirman <megi@xff.cz> S: Maintained diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig index 41c15a2c2037..454715c651d6 100644 --- a/drivers/net/phy/Kconfig +++ b/drivers/net/phy/Kconfig @@ -121,6 +121,18 @@ config AMCC_QT2025_PHY source "drivers/net/phy/aquantia/Kconfig" +config AS21XXX_PHY + tristate "Aeonsemi AS21xxx PHYs" + help + Currently supports the Aeonsemi AS21xxx PHY. + + These are C45 PHYs 10G that require all a generic firmware. + + Supported PHYs AS21011JB1, AS21011PB1, AS21010JB1, AS21010PB1, + AS21511JB1, AS21511PB1, AS21510JB1, AS21510PB1, AS21210JB1, + AS21210PB1 that all register with the PHY ID 0x7500 0x7500 + before the firmware is loaded. + config AX88796B_PHY tristate "Asix PHYs" help diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile index c8dac6e92278..14156bf11267 100644 --- a/drivers/net/phy/Makefile +++ b/drivers/net/phy/Makefile @@ -39,6 +39,7 @@ obj-$(CONFIG_AIR_EN8811H_PHY) += air_en8811h.o obj-$(CONFIG_AMD_PHY) += amd.o obj-$(CONFIG_AMCC_QT2025_PHY) += qt2025.o obj-$(CONFIG_AQUANTIA_PHY) += aquantia/ +obj-$(CONFIG_AS21XXX_PHY) += as21xxx.o ifdef CONFIG_AX88796B_RUST_PHY obj-$(CONFIG_AX88796B_PHY) += ax88796b_rust.o else diff --git a/drivers/net/phy/as21xxx.c b/drivers/net/phy/as21xxx.c new file mode 100644 index 000000000000..7aefe2fcf24f --- /dev/null +++ b/drivers/net/phy/as21xxx.c @@ -0,0 +1,834 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Aeonsemi AS21XXxX PHY Driver + * + * Author: Christian Marangi <ansuelsmth@gmail.com> + */ + +#include <linux/bitfield.h> +#include <linux/firmware.h> +#include <linux/module.h> +#include <linux/of.h> +#include <linux/phy.h> + +#define VEND1_GLB_REG_CPU_RESET_ADDR_LO_BASEADDR 0x3 +#define VEND1_GLB_REG_CPU_RESET_ADDR_HI_BASEADDR 0x4 + +#define VEND1_GLB_REG_CPU_CTRL 0xe +#define VEND1_GLB_CPU_CTRL_MASK GENMASK(4, 0) +#define VEND1_GLB_CPU_CTRL_LED_POLARITY_MASK GENMASK(12, 8) +#define VEND1_GLB_CPU_CTRL_LED_POLARITY(_n) FIELD_PREP(VEND1_GLB_CPU_CTRL_LED_POLARITY_MASK, \ + BIT(_n)) + +#define VEND1_FW_START_ADDR 0x100 + +#define VEND1_GLB_REG_MDIO_INDIRECT_ADDRCMD 0x101 +#define VEND1_GLB_REG_MDIO_INDIRECT_LOAD 0x102 + +#define VEND1_GLB_REG_MDIO_INDIRECT_STATUS 0x103 + +#define VEND1_PTP_CLK 0x142 +#define VEND1_PTP_CLK_EN BIT(6) + +/* 5 LED at step of 0x20 + * FE: Fast-Ethernet (100) + * GE: Gigabit-Ethernet (1000) + * NG: New-Generation (2500/5000/10000) + * (Lovely ChatGPT managed to translate meaning of NG) + */ +#define VEND1_LED_REG(_n) (0x1800 + ((_n) * 0x10)) +#define VEND1_LED_REG_A_EVENT GENMASK(15, 11) +#define VEND1_LED_REG_A_EVENT_ON_10 FIELD_PREP_CONST(VEND1_LED_REG_A_EVENT, 0x0) +#define VEND1_LED_REG_A_EVENT_ON_100 FIELD_PREP_CONST(VEND1_LED_REG_A_EVENT, 0x1) +#define VEND1_LED_REG_A_EVENT_ON_1000 FIELD_PREP_CONST(VEND1_LED_REG_A_EVENT, 0x2) +#define VEND1_LED_REG_A_EVENT_ON_2500 FIELD_PREP_CONST(VEND1_LED_REG_A_EVENT, 0x3) +#define VEND1_LED_REG_A_EVENT_ON_5000 FIELD_PREP_CONST(VEND1_LED_REG_A_EVENT, 0x4) +#define VEND1_LED_REG_A_EVENT_ON_10000 FIELD_PREP_CONST(VEND1_LED_REG_A_EVENT, 0x5) +#define VEND1_LED_REG_A_EVENT_ON_FE_GE FIELD_PREP_CONST(VEND1_LED_REG_A_EVENT, 0x6) +#define VEND1_LED_REG_A_EVENT_ON_NG FIELD_PREP_CONST(VEND1_LED_REG_A_EVENT, 0x7) +#define VEND1_LED_REG_A_EVENT_ON_FULL_DUPLEX FIELD_PREP_CONST(VEND1_LED_REG_A_EVENT, 0x8) +#define VEND1_LED_REG_A_EVENT_ON_COLLISION FIELD_PREP_CONST(VEND1_LED_REG_A_EVENT, 0x9) +#define VEND1_LED_REG_A_EVENT_BLINK_TX FIELD_PREP_CONST(VEND1_LED_REG_A_EVENT, 0xa) +#define VEND1_LED_REG_A_EVENT_BLINK_RX FIELD_PREP_CONST(VEND1_LED_REG_A_EVENT, 0xb) +#define VEND1_LED_REG_A_EVENT_BLINK_ACT FIELD_PREP_CONST(VEND1_LED_REG_A_EVENT, 0xc) +#define VEND1_LED_REG_A_EVENT_ON_LINK FIELD_PREP_CONST(VEND1_LED_REG_A_EVENT, 0xd) +#define VEND1_LED_REG_A_EVENT_ON_LINK_BLINK_ACT FIELD_PREP_CONST(VEND1_LED_REG_A_EVENT, 0xe) +#define VEND1_LED_REG_A_EVENT_ON_LINK_BLINK_RX FIELD_PREP_CONST(VEND1_LED_REG_A_EVENT, 0xf) +#define VEND1_LED_REG_A_EVENT_ON_FE_GE_BLINK_ACT FIELD_PREP_CONST(VEND1_LED_REG_A_EVENT, 0x10) +#define VEND1_LED_REG_A_EVENT_ON_NG_BLINK_ACT FIELD_PREP_CONST(VEND1_LED_REG_A_EVENT, 0x11) +#define VEND1_LED_REG_A_EVENT_ON_NG_BLINK_FE_GE FIELD_PREP_CONST(VEND1_LED_REG_A_EVENT, 0x12) +#define VEND1_LED_REG_A_EVENT_ON_FD_BLINK_COLLISION FIELD_PREP_CONST(VEND1_LED_REG_A_EVENT, 0x13) +#define VEND1_LED_REG_A_EVENT_ON FIELD_PREP_CONST(VEND1_LED_REG_A_EVENT, 0x14) +#define VEND1_LED_REG_A_EVENT_OFF FIELD_PREP_CONST(VEND1_LED_REG_A_EVENT, 0x15) +#define VEND1_LED_CONF 0x1881 +#define VEND1_LED_CONFG_BLINK GENMASK(7, 0) + +#define VEND1_SPEED_STATUS 0x4002 +#define VEND1_SPEED_MASK GENMASK(7, 0) +#define VEND1_SPEED_10000 FIELD_PREP_CONST(VEND1_SPEED_MASK, 0x3) +#define VEND1_SPEED_5000 FIELD_PREP_CONST(VEND1_SPEED_MASK, 0x5) +#define VEND1_SPEED_2500 FIELD_PREP_CONST(VEND1_SPEED_MASK, 0x9) +#define VEND1_SPEED_1000 FIELD_PREP_CONST(VEND1_SPEED_MASK, 0x10) +#define VEND1_SPEED_100 FIELD_PREP_CONST(VEND1_SPEED_MASK, 0x20) +#define VEND1_SPEED_10 FIELD_PREP_CONST(VEND1_SPEED_MASK, 0x0) + +#define VEND1_IPC_CMD 0x5801 +#define AEON_IPC_CMD_PARITY BIT(15) +#define AEON_IPC_CMD_SIZE GENMASK(10, 6) +#define AEON_IPC_CMD_OPCODE GENMASK(5, 0) + +#define IPC_CMD_NOOP 0x0 /* Do nothing */ +#define IPC_CMD_INFO 0x1 /* Get Firmware Version */ +#define IPC_CMD_SYS_CPU 0x2 /* SYS_CPU */ +#define IPC_CMD_BULK_DATA 0xa /* Pass bulk data in ipc registers. */ +#define IPC_CMD_BULK_WRITE 0xc /* Write bulk data to memory */ +#define IPC_CMD_CFG_PARAM 0x1a /* Write config parameters to memory */ +#define IPC_CMD_NG_TESTMODE 0x1b /* Set NG test mode and tone */ +#define IPC_CMD_TEMP_MON 0x15 /* Temperature monitoring function */ +#define IPC_CMD_SET_LED 0x23 /* Set led */ + +#define VEND1_IPC_STS 0x5802 +#define AEON_IPC_STS_PARITY BIT(15) +#define AEON_IPC_STS_SIZE GENMASK(14, 10) +#define AEON_IPC_STS_OPCODE GENMASK(9, 4) +#define AEON_IPC_STS_STATUS GENMASK(3, 0) +#define AEON_IPC_STS_STATUS_RCVD FIELD_PREP_CONST(AEON_IPC_STS_STATUS, 0x1) +#define AEON_IPC_STS_STATUS_PROCESS FIELD_PREP_CONST(AEON_IPC_STS_STATUS, 0x2) +#define AEON_IPC_STS_STATUS_SUCCESS FIELD_PREP_CONST(AEON_IPC_STS_STATUS, 0x4) +#define AEON_IPC_STS_STATUS_ERROR FIELD_PREP_CONST(AEON_IPC_STS_STATUS, 0x8) +#define AEON_IPC_STS_STATUS_BUSY FIELD_PREP_CONST(AEON_IPC_STS_STATUS, 0xe) +#define AEON_IPC_STS_STATUS_READY FIELD_PREP_CONST(AEON_IPC_STS_STATUS, 0xf) + +#define VEND1_IPC_DATA0 0x5808 +#define VEND1_IPC_DATA1 0x5809 +#define VEND1_IPC_DATA2 0x580a +#define VEND1_IPC_DATA3 0x580b +#define VEND1_IPC_DATA4 0x580c +#define VEND1_IPC_DATA5 0x580d +#define VEND1_IPC_DATA6 0x580e +#define VEND1_IPC_DATA7 0x580f +#define VEND1_IPC_DATA(_n) (VEND1_IPC_DATA0 + (_n)) + +/* Sub command of CMD_INFO */ +#define IPC_INFO_VERSION 0x1 + +/* Sub command of CMD_SYS_CPU */ +#define IPC_SYS_CPU_REBOOT 0x3 +#define IPC_SYS_CPU_IMAGE_OFST 0x4 +#define IPC_SYS_CPU_IMAGE_CHECK 0x5 +#define IPC_SYS_CPU_PHY_ENABLE 0x6 + +/* Sub command of CMD_CFG_PARAM */ +#define IPC_CFG_PARAM_DIRECT 0x4 + +/* CFG DIRECT sub command */ +#define IPC_CFG_PARAM_DIRECT_NG_PHYCTRL 0x1 +#define IPC_CFG_PARAM_DIRECT_CU_AN 0x2 +#define IPC_CFG_PARAM_DIRECT_SDS_PCS 0x3 +#define IPC_CFG_PARAM_DIRECT_AUTO_EEE 0x4 +#define IPC_CFG_PARAM_DIRECT_SDS_PMA 0x5 +#define IPC_CFG_PARAM_DIRECT_DPC_RA 0x6 +#define IPC_CFG_PARAM_DIRECT_DPC_PKT_CHK 0x7 +#define IPC_CFG_PARAM_DIRECT_DPC_SDS_WAIT_ETH 0x8 +#define IPC_CFG_PARAM_DIRECT_WDT 0x9 +#define IPC_CFG_PARAM_DIRECT_SDS_RESTART_AN 0x10 +#define IPC_CFG_PARAM_DIRECT_TEMP_MON 0x11 +#define IPC_CFG_PARAM_DIRECT_WOL 0x12 + +/* Sub command of CMD_TEMP_MON */ +#define IPC_CMD_TEMP_MON_GET 0x4 + +#define PHY_ID_AS21010JB1 0x75009422 +#define PHY_ID_AS21XXX 0x75007500 +#define PHY_VENDOR_AEONSEMI 0x75009400 + +#define AEON_MAX_LDES 5 +#define AEON_IPC_DATA_MAX (8 * sizeof(u16)) + +#define AEON_BOOT_ADDR 0x1000 +#define AEON_CPU_BOOT_ADDR 0x2000 +#define AEON_CPU_CTRL_FW_LOAD (BIT(4) | BIT(2) | BIT(1) | BIT(0)) +#define AEON_CPU_CTRL_FW_START BIT(0) + +enum { + MDIO_AN_C22 = 0xffe0, +}; + +struct as21xxx_led_pattern_info { + unsigned int pattern; + u16 val; +}; + +struct as21xxx_priv { + bool parity_status; +}; + +static struct as21xxx_led_pattern_info as21xxx_led_supported_pattern[] = { + { + .pattern = BIT(TRIGGER_NETDEV_LINK_10), + .val = VEND1_LED_REG_A_EVENT_ON_10 + }, + { + .pattern = BIT(TRIGGER_NETDEV_LINK_100), + .val = VEND1_LED_REG_A_EVENT_ON_100 + }, + { + .pattern = BIT(TRIGGER_NETDEV_LINK_1000), + .val = VEND1_LED_REG_A_EVENT_ON_1000 + }, + { + .pattern = BIT(TRIGGER_NETDEV_LINK_2500), + .val = VEND1_LED_REG_A_EVENT_ON_2500 + }, + { + .pattern = BIT(TRIGGER_NETDEV_LINK_5000), + .val = VEND1_LED_REG_A_EVENT_ON_5000 + }, + { + .pattern = BIT(TRIGGER_NETDEV_LINK_10000), + .val = VEND1_LED_REG_A_EVENT_ON_10000 + }, + { + .pattern = BIT(TRIGGER_NETDEV_LINK), + .val = VEND1_LED_REG_A_EVENT_ON_LINK + }, + { + .pattern = BIT(TRIGGER_NETDEV_LINK_100) | + BIT(TRIGGER_NETDEV_LINK_1000), + .val = VEND1_LED_REG_A_EVENT_ON_FE_GE + }, + { + .pattern = BIT(TRIGGER_NETDEV_LINK_2500) | + BIT(TRIGGER_NETDEV_LINK_5000) | + BIT(TRIGGER_NETDEV_LINK_10000), + .val = VEND1_LED_REG_A_EVENT_ON_NG + }, + { + .pattern = BIT(TRIGGER_NETDEV_FULL_DUPLEX), + .val = VEND1_LED_REG_A_EVENT_ON_FULL_DUPLEX + }, + { + .pattern = BIT(TRIGGER_NETDEV_TX), + .val = VEND1_LED_REG_A_EVENT_BLINK_TX + }, + { + .pattern = BIT(TRIGGER_NETDEV_RX), + .val = VEND1_LED_REG_A_EVENT_BLINK_RX + }, + { + .pattern = BIT(TRIGGER_NETDEV_TX) | + BIT(TRIGGER_NETDEV_RX), + .val = VEND1_LED_REG_A_EVENT_BLINK_ACT + }, + { + .pattern = BIT(TRIGGER_NETDEV_LINK_10) | + BIT(TRIGGER_NETDEV_LINK_100) | + BIT(TRIGGER_NETDEV_LINK_1000) | + BIT(TRIGGER_NETDEV_LINK_2500) | + BIT(TRIGGER_NETDEV_LINK_5000) | + BIT(TRIGGER_NETDEV_LINK_10000), + .val = VEND1_LED_REG_A_EVENT_ON_LINK + }, + { + .pattern = BIT(TRIGGER_NETDEV_LINK_10) | + BIT(TRIGGER_NETDEV_LINK_100) | + BIT(TRIGGER_NETDEV_LINK_1000) | + BIT(TRIGGER_NETDEV_LINK_2500) | + BIT(TRIGGER_NETDEV_LINK_5000) | + BIT(TRIGGER_NETDEV_LINK_10000) | + BIT(TRIGGER_NETDEV_TX) | + BIT(TRIGGER_NETDEV_RX), + .val = VEND1_LED_REG_A_EVENT_ON_LINK_BLINK_ACT + }, + { + .pattern = BIT(TRIGGER_NETDEV_LINK_10) | + BIT(TRIGGER_NETDEV_LINK_100) | + BIT(TRIGGER_NETDEV_LINK_1000) | + BIT(TRIGGER_NETDEV_LINK_2500) | + BIT(TRIGGER_NETDEV_LINK_5000) | + BIT(TRIGGER_NETDEV_LINK_10000) | + BIT(TRIGGER_NETDEV_RX), + .val = VEND1_LED_REG_A_EVENT_ON_LINK_BLINK_RX + }, + { + .pattern = BIT(TRIGGER_NETDEV_LINK_100) | + BIT(TRIGGER_NETDEV_LINK_1000) | + BIT(TRIGGER_NETDEV_TX) | + BIT(TRIGGER_NETDEV_RX), + .val = VEND1_LED_REG_A_EVENT_ON_FE_GE_BLINK_ACT + }, + { + .pattern = BIT(TRIGGER_NETDEV_LINK_2500) | + BIT(TRIGGER_NETDEV_LINK_5000) | + BIT(TRIGGER_NETDEV_LINK_10000) | + BIT(TRIGGER_NETDEV_TX) | + BIT(TRIGGER_NETDEV_RX), + .val = VEND1_LED_REG_A_EVENT_ON_NG_BLINK_ACT + } +}; + +static int aeon_firmware_boot(struct phy_device *phydev, const u8 *data, size_t size) +{ + int i, ret; + u16 val; + + ret = phy_modify_mmd(phydev, MDIO_MMD_VEND1, VEND1_GLB_REG_CPU_CTRL, + VEND1_GLB_CPU_CTRL_MASK, AEON_CPU_CTRL_FW_LOAD); + if (ret) + return ret; + + ret = phy_write_mmd(phydev, MDIO_MMD_VEND1, VEND1_FW_START_ADDR, + AEON_BOOT_ADDR); + if (ret) + return ret; + + ret = phy_modify_mmd(phydev, MDIO_MMD_VEND1, VEND1_GLB_REG_MDIO_INDIRECT_ADDRCMD, + 0x3ffc, 0xc000); + if (ret) + return ret; + + val = phy_read_mmd(phydev, MDIO_MMD_VEND1, VEND1_GLB_REG_MDIO_INDIRECT_STATUS); + if (val > 1) { + phydev_err(phydev, "wrong origin mdio_indirect_status: %x\n", val); + return -EINVAL; + } + + /* Firmware is always aligned to u16 */ + for (i = 0; i < size; i += 2) { + val = data[i + 1] << 8 | data[i]; + + ret = phy_write_mmd(phydev, MDIO_MMD_VEND1, VEND1_GLB_REG_MDIO_INDIRECT_LOAD, val); + if (ret) + return ret; + } + + ret = phy_write_mmd(phydev, MDIO_MMD_VEND1, VEND1_GLB_REG_CPU_RESET_ADDR_LO_BASEADDR, + lower_16_bits(AEON_CPU_BOOT_ADDR)); + if (ret) + return ret; + + ret = phy_write_mmd(phydev, MDIO_MMD_VEND1, VEND1_GLB_REG_CPU_RESET_ADDR_HI_BASEADDR, + upper_16_bits(AEON_CPU_BOOT_ADDR)); + if (ret) + return ret; + + return phy_modify_mmd(phydev, MDIO_MMD_VEND1, VEND1_GLB_REG_CPU_CTRL, + VEND1_GLB_CPU_CTRL_MASK, AEON_CPU_CTRL_FW_START); +} + +static int aeon_firmware_load(struct phy_device *phydev) +{ + struct device *dev = &phydev->mdio.dev; + const struct firmware *fw; + const char *fw_name; + int ret; + + ret = of_property_read_string(dev->of_node, "firmware-name", + &fw_name); + if (ret) + return ret; + + ret = request_firmware(&fw, fw_name, dev); + if (ret) { + phydev_err(phydev, "failed to find FW file %s (%d)\n", + fw_name, ret); + return ret; + } + + ret = aeon_firmware_boot(phydev, fw->data, fw->size); + + release_firmware(fw); + + return ret; +} + +static int aeon_ipc_send_cmd(struct phy_device *phydev, u32 cmd, + u16 *ret_sts) +{ + struct as21xxx_priv *priv = phydev->priv; + bool curr_parity; + u32 val; + int ret; + + /* The IPC sync by using a single parity bit. + * Each CMD have alternately this bit set or clear + * to understand correct flow and packet order. + */ + curr_parity = priv->parity_status; + if (priv->parity_status) + cmd |= AEON_IPC_CMD_PARITY; + + /* Always update parity for next packet */ + priv->parity_status = !priv->parity_status; + + ret = phy_write_mmd(phydev, MDIO_MMD_VEND1, VEND1_IPC_CMD, cmd); + if (ret) + return ret; + + /* Wait for packet to be processed */ + usleep_range(10000, 15000); + + /* With no ret_sts, ignore waiting for packet completion + * (ipc parity bit sync) + */ + if (!ret_sts) + return 0; + + /* Exit condition logic: + * - Wait for parity bit equal + * - Wait for status success, error OR ready + */ + ret = phy_read_mmd_poll_timeout(phydev, MDIO_MMD_VEND1, VEND1_IPC_STS, val, + FIELD_GET(AEON_IPC_STS_PARITY, val) == + curr_parity && + (val & AEON_IPC_STS_STATUS) != + AEON_IPC_STS_STATUS_RCVD && + (val & AEON_IPC_STS_STATUS) != + AEON_IPC_STS_STATUS_PROCESS && + (val & AEON_IPC_STS_STATUS) != + AEON_IPC_STS_STATUS_BUSY, + 10000, 2000000, false); + if (ret) + return ret; + + *ret_sts = val; + if ((val & AEON_IPC_STS_STATUS) != AEON_IPC_STS_STATUS_SUCCESS) + return -EFAULT; + + return 0; +} + +static int aeon_ipc_send_msg(struct phy_device *phydev, u16 opcode, + u16 *data, unsigned int data_len, u16 *ret_sts) +{ + u32 cmd; + int ret; + int i; + + if (data_len > AEON_IPC_DATA_MAX) + return -EINVAL; + + for (i = 0; i < data_len / sizeof(u16); i++) + phy_write_mmd(phydev, MDIO_MMD_VEND1, VEND1_IPC_DATA(i), + data[i]); + + cmd = FIELD_PREP(AEON_IPC_CMD_SIZE, data_len) | + FIELD_PREP(AEON_IPC_CMD_OPCODE, opcode); + ret = aeon_ipc_send_cmd(phydev, cmd, ret_sts); + if (ret) + phydev_err(phydev, "failed to send ipc msg for %x: %d\n", opcode, ret); + + return ret; +} + +static int aeon_ipc_rcv_msg(struct phy_device *phydev, u16 ret_sts, + u16 *data) +{ + unsigned int size = FIELD_GET(AEON_IPC_STS_SIZE, ret_sts); + int ret; + int i; + + if ((ret_sts & AEON_IPC_STS_STATUS) == AEON_IPC_STS_STATUS_ERROR) + return -EINVAL; + + for (i = 0; i < DIV_ROUND_UP(size, sizeof(u16)); i++) { + ret = phy_read_mmd(phydev, MDIO_MMD_VEND1, VEND1_IPC_DATA(i)); + if (ret < 0) + return ret; + + data[i] = ret; + } + + return size; +} + +/* Logic to sync parity bit with IPC. + * We send 2 NOP cmd with same partity and we wait for IPC + * to handle the packet only for the second one. This way + * we make sure we are sync for every next cmd. + */ +static int aeon_ipc_sync_parity(struct phy_device *phydev) +{ + struct as21xxx_priv *priv = phydev->priv; + u16 ret_sts; + u32 cmd; + int ret; + + /* Send NOP with no parity */ + cmd = FIELD_PREP(AEON_IPC_CMD_SIZE, 0) | + FIELD_PREP(AEON_IPC_CMD_OPCODE, IPC_CMD_NOOP); + aeon_ipc_send_cmd(phydev, cmd, NULL); + + /* Reset packet parity */ + priv->parity_status = false; + + /* Send second NOP with no parity */ + ret = aeon_ipc_send_cmd(phydev, cmd, &ret_sts); + /* We expect to return -EFAULT */ + if (ret != -EFAULT) + return ret; + + if ((ret_sts & AEON_IPC_STS_STATUS) != AEON_IPC_STS_STATUS_READY) + return -EINVAL; + + return 0; +} + +static int aeon_ipc_get_fw_version(struct phy_device *phydev) +{ + u16 ret_data[8], data[1]; + u16 ret_sts; + int ret; + + data[0] = IPC_INFO_VERSION; + ret = aeon_ipc_send_msg(phydev, IPC_CMD_INFO, data, sizeof(data), + &ret_sts); + if (ret) + return ret; + + ret = aeon_ipc_rcv_msg(phydev, ret_sts, ret_data); + if (ret < 0) + return ret; + + phydev_info(phydev, "Firmware Version: %s\n", (char *)ret_data); + + return 0; +} + +static int aeon_dpc_ra_enable(struct phy_device *phydev) +{ + u16 data[2]; + u16 ret_sts; + + data[0] = IPC_CFG_PARAM_DIRECT; + data[1] = IPC_CFG_PARAM_DIRECT_DPC_RA; + + return aeon_ipc_send_msg(phydev, IPC_CMD_CFG_PARAM, data, + sizeof(data), &ret_sts); +} + +static int as21xxx_probe(struct phy_device *phydev) +{ + struct as21xxx_priv *priv; + int ret; + + phydev->priv = devm_kzalloc(&phydev->mdio.dev, + sizeof(*priv), GFP_KERNEL); + if (!phydev->priv) + return -ENOMEM; + + ret = aeon_firmware_load(phydev); + if (ret) + return ret; + + ret = aeon_ipc_sync_parity(phydev); + if (ret) + return ret; + + /* Enable PTP clk if not already Enabled */ + ret = phy_set_bits_mmd(phydev, MDIO_MMD_VEND1, VEND1_PTP_CLK, + VEND1_PTP_CLK_EN); + if (ret) + return ret; + + ret = aeon_dpc_ra_enable(phydev); + if (ret) + return ret; + + ret = aeon_ipc_get_fw_version(phydev); + if (ret) + return ret; + + return 0; +} + +static int as21xxx_get_features(struct phy_device *phydev) +{ + int ret; + + ret = genphy_read_abilities(phydev); + if (ret) + return ret; + + /* AS21xxx supports 100M/1G/2.5G/5G/10G speed. */ + linkmode_clear_bit(ETHTOOL_LINK_MODE_10baseT_Half_BIT, + phydev->supported); + linkmode_clear_bit(ETHTOOL_LINK_MODE_10baseT_Full_BIT, + phydev->supported); + linkmode_clear_bit(ETHTOOL_LINK_MODE_100baseT_Half_BIT, + phydev->supported); + linkmode_set_bit(ETHTOOL_LINK_MODE_100baseT_Full_BIT, + phydev->supported); + linkmode_set_bit(ETHTOOL_LINK_MODE_1000baseT_Full_BIT, + phydev->supported); + linkmode_set_bit(ETHTOOL_LINK_MODE_2500baseT_Full_BIT, + phydev->supported); + linkmode_set_bit(ETHTOOL_LINK_MODE_5000baseT_Full_BIT, + phydev->supported); + linkmode_set_bit(ETHTOOL_LINK_MODE_10000baseT_Full_BIT, + phydev->supported); + + return 0; +} + +static int as21xxx_read_link(struct phy_device *phydev, int *bmcr) +{ + int status; + + *bmcr = phy_read_mmd(phydev, MDIO_MMD_AN, + MDIO_AN_C22 + MII_BMCR); + if (*bmcr < 0) + return *bmcr; + + /* Autoneg is being started, therefore disregard current + * link status and report link as down. + */ + if (*bmcr & BMCR_ANRESTART) { + phydev->link = 0; + return 0; + } + + status = phy_read_mmd(phydev, MDIO_MMD_AN, MDIO_STAT1); + if (status < 0) + return status; + + phydev->link = !!(status & MDIO_STAT1_LSTATUS); + + return 0; +} + +static int as21xxx_read_c22_lpa(struct phy_device *phydev) +{ + int lpagb; + + lpagb = phy_read_mmd(phydev, MDIO_MMD_AN, + MDIO_AN_C22 + MII_STAT1000); + if (lpagb < 0) + return lpagb; + + if (lpagb & LPA_1000MSFAIL) { + int adv = phy_read_mmd(phydev, MDIO_MMD_AN, + MDIO_AN_C22 + MII_CTRL1000); + + if (adv < 0) + return adv; + + if (adv & CTL1000_ENABLE_MASTER) + phydev_err(phydev, "Master/Slave resolution failed, maybe conflicting manual settings?\n"); + else + phydev_err(phydev, "Master/Slave resolution failed\n"); + return -ENOLINK; + } + + mii_stat1000_mod_linkmode_lpa_t(phydev->lp_advertising, + lpagb); + + return 0; +} + +static int as21xxx_read_status(struct phy_device *phydev) +{ + int bmcr, old_link = phydev->link; + int ret; + + ret = as21xxx_read_link(phydev, &bmcr); + if (ret) + return ret; + + /* why bother the PHY if nothing can have changed */ + if (phydev->autoneg == AUTONEG_ENABLE && old_link && phydev->link) + return 0; + + phydev->speed = SPEED_UNKNOWN; + phydev->duplex = DUPLEX_UNKNOWN; + phydev->pause = 0; + phydev->asym_pause = 0; + + if (phydev->autoneg == AUTONEG_ENABLE) { + ret = genphy_c45_read_lpa(phydev); + if (ret) + return ret; + + ret = as21xxx_read_c22_lpa(phydev); + if (ret) + return ret; + + phy_resolve_aneg_linkmode(phydev); + } else { + int speed; + + linkmode_zero(phydev->lp_advertising); + + speed = phy_read_mmd(phydev, MDIO_MMD_VEND1, + VEND1_SPEED_STATUS); + if (speed < 0) + return speed; + + switch (speed & VEND1_SPEED_STATUS) { + case VEND1_SPEED_10000: + phydev->speed = SPEED_10000; + phydev->duplex = DUPLEX_FULL; + break; + case VEND1_SPEED_5000: + phydev->speed = SPEED_5000; + phydev->duplex = DUPLEX_FULL; + break; + case VEND1_SPEED_2500: + phydev->speed = SPEED_2500; + phydev->duplex = DUPLEX_FULL; + break; + case VEND1_SPEED_1000: + phydev->speed = SPEED_1000; + if (bmcr & BMCR_FULLDPLX) + phydev->duplex = DUPLEX_FULL; + else + phydev->duplex = DUPLEX_HALF; + break; + case VEND1_SPEED_100: + phydev->speed = SPEED_100; + phydev->duplex = DUPLEX_FULL; + break; + case VEND1_SPEED_10: + phydev->speed = SPEED_10; + phydev->duplex = DUPLEX_FULL; + break; + default: + return -EINVAL; + } + } + + return 0; +} + +static int as21xxx_led_brightness_set(struct phy_device *phydev, + u8 index, enum led_brightness value) +{ + u16 val = VEND1_LED_REG_A_EVENT_OFF; + + if (index > AEON_MAX_LDES) + return -EINVAL; + + if (value) + val = VEND1_LED_REG_A_EVENT_ON; + + return phy_modify_mmd(phydev, MDIO_MMD_VEND1, + VEND1_LED_REG(index), + VEND1_LED_REG_A_EVENT, val); +} + +static int as21xxx_led_hw_is_supported(struct phy_device *phydev, u8 index, + unsigned long rules) +{ + int i; + + if (index > AEON_MAX_LDES) + return -EINVAL; + + for (i = 0; i < ARRAY_SIZE(as21xxx_led_supported_pattern); i++) + if (rules == as21xxx_led_supported_pattern[i].pattern) + return 0; + + return -EOPNOTSUPP; +} + +static int as21xxx_led_hw_control_get(struct phy_device *phydev, u8 index, + unsigned long *rules) +{ + u16 val; + int i; + + if (index > AEON_MAX_LDES) + return -EINVAL; + + val = phy_read_mmd(phydev, MDIO_MMD_VEND1, VEND1_LED_REG(index)); + if (val < 0) + return val; + + val &= VEND1_LED_REG_A_EVENT; + for (i = 0; i < ARRAY_SIZE(as21xxx_led_supported_pattern); i++) + if (val == as21xxx_led_supported_pattern[i].val) { + *rules = as21xxx_led_supported_pattern[i].pattern; + return 0; + } + + /* Should be impossible */ + return -EINVAL; +} + +static int as21xxx_led_hw_control_set(struct phy_device *phydev, u8 index, + unsigned long rules) +{ + u16 val; + int i; + + if (index > AEON_MAX_LDES) + return -EINVAL; + + for (i = 0; i < ARRAY_SIZE(as21xxx_led_supported_pattern); i++) + if (rules == as21xxx_led_supported_pattern[i].pattern) { + val = as21xxx_led_supported_pattern[i].val; + break; + } + + return phy_modify_mmd(phydev, MDIO_MMD_VEND1, + VEND1_LED_REG(index), + VEND1_LED_REG_A_EVENT, val); +} + +static int as21xxx_led_polarity_set(struct phy_device *phydev, int index, + unsigned long modes) +{ + bool led_active_low; + u16 mask, val = 0; + u32 mode; + + if (index > AEON_MAX_LDES) + return -EINVAL; + + for_each_set_bit(mode, &modes, __PHY_LED_MODES_NUM) { + switch (mode) { + case PHY_LED_ACTIVE_LOW: + led_active_low = true; + break; + case PHY_LED_ACTIVE_HIGH: /* default mode */ + led_active_low = false; + break; + default: + return -EINVAL; + } + } + + mask = VEND1_GLB_CPU_CTRL_LED_POLARITY(index); + if (led_active_low) + val = VEND1_GLB_CPU_CTRL_LED_POLARITY(index); + + return phy_modify_mmd(phydev, MDIO_MMD_VEND1, + VEND1_GLB_REG_CPU_CTRL, + mask, val); +} + +static struct phy_driver as21xxx_drivers[] = { + { + PHY_ID_MATCH_EXACT(PHY_ID_AS21XXX), + .name = "Aeonsemi AS21xxx", + .probe = as21xxx_probe, + .get_features = as21xxx_get_features, + .read_status = as21xxx_read_status, + .led_brightness_set = as21xxx_led_brightness_set, + .led_hw_is_supported = as21xxx_led_hw_is_supported, + .led_hw_control_set = as21xxx_led_hw_control_set, + .led_hw_control_get = as21xxx_led_hw_control_get, + .led_polarity_set = as21xxx_led_polarity_set, + }, +}; +module_phy_driver(as21xxx_drivers); + +static struct mdio_device_id __maybe_unused as21xxx_tbl[] = { + { PHY_ID_MATCH_VENDOR(PHY_VENDOR_AEONSEMI) }, + { } +}; +MODULE_DEVICE_TABLE(mdio, as21xxx_tbl); + +MODULE_DESCRIPTION("Aeonsemi AS21xxx PHY driver"); +MODULE_AUTHOR("Christian Marangi <ansuelsmth@gmail.com>"); +MODULE_LICENSE("GPL");
Add support for new Aeonsemi 10G C45 PHYs. These PHYs intergate an IPC to setup some configuration and require special handling to sync with the parity bit. The parity bit is a way the IPC use to follow correct order of command sent. Supported PHYs AS21011JB1, AS21011PB1, AS21010JB1, AS21010PB1, AS21511JB1, AS21511PB1, AS21510JB1, AS21510PB1, AS21210JB1, AS21210PB1 that all register with the PHY ID 0x7500 0x7500 before the firmware is loaded. They all support up to 5 LEDs with various HW mode supported. While implementing it was found some strange coincidence with using the same logic for implementing C22 in MMD regs in Broadcom PHYs. Signed-off-by: Christian Marangi <ansuelsmth@gmail.com> --- MAINTAINERS | 6 + drivers/net/phy/Kconfig | 12 + drivers/net/phy/Makefile | 1 + drivers/net/phy/as21xxx.c | 834 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 853 insertions(+) create mode 100644 drivers/net/phy/as21xxx.c