Messages in this thread | | | Date | Sun, 29 Jan 2023 09:56:09 +0800 | Subject | Re: [PATCH net-next v2 5/5] net: phy: Add driver for Motorcomm yt8531 gigabit ethernet phy | From | "Frank.Sae" <> |
| |
Hi Andrew,
On 2023/1/28 23:42, Andrew Lunn wrote: > On Sat, Jan 28, 2023 at 11:13:14AM +0800, Frank Sae wrote: >> Add a driver for the motorcomm yt8531 gigabit ethernet phy. We have >> verified the driver on AM335x platform with yt8531 board. On the >> board, yt8531 gigabit ethernet phy works in utp mode, RGMII >> interface, supports 1000M/100M/10M speeds, and wol(magic package). >> >> Signed-off-by: Frank Sae <Frank.Sae@motor-comm.com> >> --- >> drivers/net/phy/Kconfig | 2 +- >> drivers/net/phy/motorcomm.c | 204 +++++++++++++++++++++++++++++++++++- >> 2 files changed, 203 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig >> index f5df2edc94a5..dc2f7d0b0cd8 100644 >> --- a/drivers/net/phy/Kconfig >> +++ b/drivers/net/phy/Kconfig >> @@ -257,7 +257,7 @@ config MOTORCOMM_PHY >> tristate "Motorcomm PHYs" >> help >> Enables support for Motorcomm network PHYs. >> - Currently supports the YT8511, YT8521, YT8531S Gigabit Ethernet PHYs. >> + Currently supports the YT8511, YT8521, YT8531, YT8531S Gigabit Ethernet PHYs. > > This is O.K. for now, but when you add the next PHY, please do this in > some other way, because it does not scale. Maybe just say YT85xx? > >> >> config NATIONAL_PHY >> tristate "National Semiconductor PHYs" >> diff --git a/drivers/net/phy/motorcomm.c b/drivers/net/phy/motorcomm.c >> index 9559fc52814f..f1fc912738e0 100644 >> --- a/drivers/net/phy/motorcomm.c >> +++ b/drivers/net/phy/motorcomm.c >> @@ -1,6 +1,6 @@ >> // SPDX-License-Identifier: GPL-2.0+ >> /* >> - * Motorcomm 8511/8521/8531S PHY driver. >> + * Motorcomm 8511/8521/8531/8531S PHY driver. >> * >> * Author: Peter Geis <pgwipeout@gmail.com> >> * Author: Frank <Frank.Sae@motor-comm.com> >> @@ -14,6 +14,7 @@ >> >> #define PHY_ID_YT8511 0x0000010a >> #define PHY_ID_YT8521 0x0000011A >> +#define PHY_ID_YT8531 0x4f51e91b >> #define PHY_ID_YT8531S 0x4F51E91A >> >> /* YT8521/YT8531S Register Overview >> @@ -517,6 +518,68 @@ static int ytphy_set_wol(struct phy_device *phydev, struct ethtool_wolinfo *wol) >> return phy_restore_page(phydev, old_page, ret); >> } >> >> +static int yt8531_set_wol(struct phy_device *phydev, >> + struct ethtool_wolinfo *wol) >> +{ >> + struct net_device *p_attached_dev; >> + const u16 mac_addr_reg[] = { >> + YTPHY_WOL_MACADDR2_REG, >> + YTPHY_WOL_MACADDR1_REG, >> + YTPHY_WOL_MACADDR0_REG, >> + }; >> + const u8 *mac_addr; >> + u16 mask, val; >> + int ret; >> + u8 i; >> + >> + if (wol->wolopts & WAKE_MAGIC) { >> + p_attached_dev = phydev->attached_dev; >> + if (!p_attached_dev) >> + return -ENODEV; >> + >> + mac_addr = (const u8 *)p_attached_dev->dev_addr; >> + if (!is_valid_ether_addr(mac_addr)) >> + return -EINVAL; > > Have you ever seen that happen? It suggests the MAC driver has a bug, > not validating its MAC address.
I have never seen that happen. Do you mean that I should change the code from
+ if (wol->wolopts & WAKE_MAGIC) { + p_attached_dev = phydev->attached_dev; + if (!p_attached_dev) + return -ENODEV; + + mac_addr = (const u8 *)p_attached_dev->dev_addr; + if (!is_valid_ether_addr(mac_addr)) + return -EINVAL;
to
+ if (wol->wolopts & WAKE_MAGIC) { + mac_addr = phydev->attached_dev->dev_addr;
?
> Also, does the PHY actually care? Will the firmware crash if given a > bad MAC address? >
The PHY actually is not care this. The firmware is not crash if given a bad MAC address.
> Andrew
| |