On 16.12.2022 08:06, Yanhong Wang wrote: > This adds basic support for the Motorcomm YT8531 > Gigabit Ethernet PHY. > > Signed-off-by: Yanhong Wang <yanhong.wang@xxxxxxxxxxxxxxxx> > --- > drivers/net/phy/Kconfig | 3 +- > drivers/net/phy/motorcomm.c | 202 ++++++++++++++++++++++++++++++++++++ > 2 files changed, 204 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig > index c57a0262fb64..86399254d9ff 100644 > --- a/drivers/net/phy/Kconfig > +++ b/drivers/net/phy/Kconfig > @@ -258,9 +258,10 @@ config MICROSEMI_PHY > > config MOTORCOMM_PHY > tristate "Motorcomm PHYs" > + default SOC_STARFIVE Both are completely independent. This default should be removed. > help > Enables support for Motorcomm network PHYs. > - Currently supports the YT8511 gigabit PHY. > + Currently supports the YT8511 and YT8531 gigabit PHYs. > This doesn't apply. Parts of your patch exist already in net-next. Support for YT8531S has been added in the meantime. Please rebase your patch on net-next and annotate your patch as net-next. > config NATIONAL_PHY > tristate "National Semiconductor PHYs" > diff --git a/drivers/net/phy/motorcomm.c b/drivers/net/phy/motorcomm.c > index 7e6ac2c5e27e..bca03185b338 100644 > --- a/drivers/net/phy/motorcomm.c > +++ b/drivers/net/phy/motorcomm.c > @@ -3,13 +3,17 @@ > * Driver for Motorcomm PHYs > * > * Author: Peter Geis <pgwipeout@xxxxxxxxx> > + * > */ > > +#include <linux/bitops.h> > #include <linux/kernel.h> > #include <linux/module.h> > +#include <linux/of.h> > #include <linux/phy.h> > > #define PHY_ID_YT8511 0x0000010a > +#define PHY_ID_YT8531 0x4f51e91b > > #define YT8511_PAGE_SELECT 0x1e > #define YT8511_PAGE 0x1f > @@ -17,6 +21,10 @@ > #define YT8511_EXT_DELAY_DRIVE 0x0d > #define YT8511_EXT_SLEEP_CTRL 0x27 > > +#define YTPHY_EXT_SMI_SDS_PHY 0xa000 > +#define YTPHY_EXT_CHIP_CONFIG 0xa001 > +#define YTPHY_EXT_RGMII_CONFIG1 0xa003 > + > /* 2b00 25m from pll > * 2b01 25m from xtl *default* > * 2b10 62.m from pll > @@ -38,6 +46,51 @@ > #define YT8511_DELAY_FE_TX_EN (0xf << 12) > #define YT8511_DELAY_FE_TX_DIS (0x2 << 12) > > +struct ytphy_reg_field { > + char *name; > + u32 mask; > + u8 dflt; /* Default value */ > +}; > + > +struct ytphy_priv_t { > + u32 tx_inverted_1000; > + u32 tx_inverted_100; > + u32 tx_inverted_10; > +}; > + > +/* rx_delay_sel: RGMII rx clock delay train configuration, about 150ps per > + * step. Delay = 150ps * N > + * > + * tx_delay_sel_fe: RGMII tx clock delay train configuration when speed is > + * 100Mbps or 10Mbps, it's 150ps per step. Delay = 150ps * N > + * > + * tx_delay_sel: RGMII tx clock delay train configuration when speed is > + * 1000Mbps, it's 150ps per step. Delay = 150ps * N > + */ > +static const struct ytphy_reg_field ytphy_rxtxd_grp[] = { > + { "rx_delay_sel", GENMASK(13, 10), 0x0 }, > + { "tx_delay_sel_fe", GENMASK(7, 4), 0xf }, > + { "tx_delay_sel", GENMASK(3, 0), 0x1 } > +}; > + > +/* tx_inverted_x: Use original or inverted RGMII TX_CLK to drive the RGMII > + * TX_CLK delay train configuration when speed is > + * xMbps(10/100/1000Mbps). > + * 0: original, 1: inverted > + */ > +static const struct ytphy_reg_field ytphy_txinver_grp[] = { > + { "tx_inverted_1000", BIT(14), 0x0 }, > + { "tx_inverted_100", BIT(14), 0x0 }, > + { "tx_inverted_10", BIT(14), 0x0 } Copy & Paste error that mask is the same for all entries? > +}; > + > +/* rxc_dly_en: RGMII clk 2ns delay control bit. > + * 0: disable 1: enable > + */ > +static const struct ytphy_reg_field ytphy_rxden_grp[] = { > + { "rxc_dly_en", BIT(8), 0x1 } > +}; > + > static int yt8511_read_page(struct phy_device *phydev) > { > return __phy_read(phydev, YT8511_PAGE_SELECT); > @@ -48,6 +101,33 @@ static int yt8511_write_page(struct phy_device *phydev, int page) > return __phy_write(phydev, YT8511_PAGE_SELECT, page); > }; > > +static int ytphy_read_ext(struct phy_device *phydev, u32 regnum) > +{ > + int ret; > + int val; > + > + ret = __phy_write(phydev, YT8511_PAGE_SELECT, regnum); > + if (ret < 0) > + return ret; > + > + val = __phy_read(phydev, YT8511_PAGE); > + > + return val; > +} > + > +static int ytphy_write_ext(struct phy_device *phydev, u32 regnum, u16 val) > +{ > + int ret; > + > + ret = __phy_write(phydev, YT8511_PAGE_SELECT, regnum); > + if (ret < 0) > + return ret; > + > + ret = __phy_write(phydev, YT8511_PAGE, val); > + > + return ret; > +} > + > static int yt8511_config_init(struct phy_device *phydev) > { > int oldpage, ret = 0; > @@ -111,6 +191,116 @@ static int yt8511_config_init(struct phy_device *phydev) > return phy_restore_page(phydev, oldpage, ret); > } > > +static int ytphy_config_init(struct phy_device *phydev) > +{ > + struct device_node *of_node; > + u32 val; > + u32 mask; > + u32 cfg; > + int ret; > + int i = 0; > + > + of_node = phydev->mdio.dev.of_node; > + if (of_node) { > + ret = of_property_read_u32(of_node, ytphy_rxden_grp[0].name, &cfg); > + if (!ret) { > + mask = ytphy_rxden_grp[0].mask; > + val = ytphy_read_ext(phydev, YTPHY_EXT_CHIP_CONFIG); > + > + /* check the cfg overflow or not */ > + cfg = cfg > mask >> (ffs(mask) - 1) ? mask : cfg; > + > + val &= ~mask; > + val |= FIELD_PREP(mask, cfg); > + ytphy_write_ext(phydev, YTPHY_EXT_CHIP_CONFIG, val); This is the unlocked version. MDIO bus locking is missing. > + } > + > + val = ytphy_read_ext(phydev, YTPHY_EXT_RGMII_CONFIG1); > + for (i = 0; i < ARRAY_SIZE(ytphy_rxtxd_grp); i++) { > + ret = of_property_read_u32(of_node, ytphy_rxtxd_grp[i].name, &cfg); > + if (!ret) { > + mask = ytphy_rxtxd_grp[i].mask; > + > + /* check the cfg overflow or not */ > + cfg = cfg > mask >> (ffs(mask) - 1) ? mask : cfg; > + > + val &= ~mask; > + val |= cfg << (ffs(mask) - 1); > + } > + } > + return ytphy_write_ext(phydev, YTPHY_EXT_RGMII_CONFIG1, val); > + } > + > + phydev_err(phydev, "Get of node fail\n"); > + Please consider that the PHY may be used on non-DT systems. > + return -EINVAL; > +} > + > +static void ytphy_link_change_notify(struct phy_device *phydev) > +{ > + u32 val; > + struct ytphy_priv_t *ytphy_priv = phydev->priv; > + > + if (phydev->speed < 0) > + return; > + > + val = ytphy_read_ext(phydev, YTPHY_EXT_RGMII_CONFIG1); > + switch (phydev->speed) { > + case SPEED_1000: > + val &= ~ytphy_txinver_grp[0].mask; > + val |= FIELD_PREP(ytphy_txinver_grp[0].mask, > + ytphy_priv->tx_inverted_1000); > + break; > + > + case SPEED_100: > + val &= ~ytphy_txinver_grp[1].mask; > + val |= FIELD_PREP(ytphy_txinver_grp[1].mask, > + ytphy_priv->tx_inverted_100); > + break; > + > + case SPEED_10: > + val &= ~ytphy_txinver_grp[2].mask; > + val |= FIELD_PREP(ytphy_txinver_grp[2].mask, > + ytphy_priv->tx_inverted_10); > + break; > + > + default: > + break; > + } > + > + ytphy_write_ext(phydev, YTPHY_EXT_RGMII_CONFIG1, val); > +} > + > +static int yt8531_probe(struct phy_device *phydev) > +{ > + struct ytphy_priv_t *priv; > + const struct device_node *of_node; > + u32 val; > + int ret; > + > + priv = devm_kzalloc(&phydev->mdio.dev, sizeof(*priv), GFP_KERNEL); > + if (!priv) > + return -ENOMEM; > + > + of_node = phydev->mdio.dev.of_node; > + if (of_node) { > + ret = of_property_read_u32(of_node, ytphy_txinver_grp[0].name, &val); > + if (!ret) > + priv->tx_inverted_1000 = val; > + > + ret = of_property_read_u32(of_node, ytphy_txinver_grp[1].name, &val); > + if (!ret) > + priv->tx_inverted_100 = val; > + > + ret = of_property_read_u32(of_node, ytphy_txinver_grp[2].name, &val); > + if (!ret) > + priv->tx_inverted_10 = val; > + } > + phydev->priv = priv; > + > + return 0; > +} > + > static struct phy_driver motorcomm_phy_drvs[] = { > { > PHY_ID_MATCH_EXACT(PHY_ID_YT8511), > @@ -120,6 +310,17 @@ static struct phy_driver motorcomm_phy_drvs[] = { > .resume = genphy_resume, > .read_page = yt8511_read_page, > .write_page = yt8511_write_page, > + }, { > + PHY_ID_MATCH_EXACT(PHY_ID_YT8531), > + .name = "YT8531 Gigabit Ethernet", > + .probe = yt8531_probe, > + .config_init = ytphy_config_init, > + .read_status = genphy_read_status, > + .suspend = genphy_suspend, > + .resume = genphy_resume, > + .read_page = yt8511_read_page, > + .write_page = yt8511_write_page, > + .link_change_notify = ytphy_link_change_notify, > }, > }; > > @@ -131,6 +332,7 @@ MODULE_LICENSE("GPL"); > > static const struct mdio_device_id __maybe_unused motorcomm_tbl[] = { > { PHY_ID_MATCH_EXACT(PHY_ID_YT8511) }, > + { PHY_ID_MATCH_EXACT(PHY_ID_YT8531) }, > { /* sentinal */ } > }; >