| Message ID | 20221216070632.11444-7-yanhong.wang@starfivetech.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Add Ethernet driver for StarFive JH7110 SoC | expand |
| Context | Check | Description |
|---|---|---|
| conchuod/tree_selection | fail | Guessing tree name failed |
Hi Yanhong, On Fri, 2022-12-16 at 15:06 +0800, Yanhong Wang wrote: > This adds basic support for the Motorcomm YT8531 > Gigabit Ethernet PHY. > > Signed-off-by: Yanhong Wang <yanhong.wang@starfivetech.com> > --- > drivers/net/phy/Kconfig | 3 +- > drivers/net/phy/motorcomm.c | 202 > ++++++++++++++++++++++++++++++++++++ > 2 files changed, 204 insertions(+), 1 deletion(-) > > > +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; You are returning the error value as well as the read value in the function. But in the config_init, you are not checking the error value, just using the value directly. So instead of returning both the value and error, you may consider return error and for value use call by reference. > + > + val = __phy_read(phydev, YT8511_PAGE); > + > + return val; > +} > + > + > 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; i initialized here as well as in for loop. > + > + of_node = phydev->mdio.dev.of_node; > + if (of_node) { to reduce the ident level, you may consider returning here by checking if(!of_node) return -EINVAL; > + 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); > + } > + > + 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"); > + > + return -EINVAL; > +} > + > +static void ytphy_link_change_notify(struct phy_device *phydev) > +{ > + u32 val; > + struct ytphy_priv_t *ytphy_priv = phydev->priv; reverse christmas tree > + > + if (phydev->speed < 0) > + return; > + > + > +static int yt8531_probe(struct phy_device *phydev) > +{ > + struct ytphy_priv_t *priv; > + const struct device_node *of_node; Reverse christmas tree. > + 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; > +} > + >
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@starfivetech.com> > --- > 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@gmail.com> > + * > */ > > +#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 */ } > }; >
> Signed-off-by: Yanhong Wang <yanhong.wang@starfivetech.com> > --- > 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 Please don't do this. Look at the other PHY drivers. How many have a default like this? Generally, if you are doing something which no other driver does, you are doing something wrong. > --- a/drivers/net/phy/motorcomm.c > +++ b/drivers/net/phy/motorcomm.c > @@ -3,13 +3,17 @@ > * Driver for Motorcomm PHYs > * > * Author: Peter Geis <pgwipeout@gmail.com> > + * > */ Please avoid changes like this. It distracts from the real code changes. > > +#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 */ > +}; This should be next to where it is used. But once you have delay in ps, i suspect you will throw this away. > > +static int ytphy_config_init(struct phy_device *phydev) Is this specific to the 8531? If so, put 8531 in the function name? Do any of these delay configurations also apply to the 8511? > +{ > + struct device_node *of_node; > + u32 val; > + u32 mask; > + u32 cfg; > + int ret; > + int i = 0; Reverse Christmas tree. i needs to be earlier, val needs to be later. > + 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); > + } > + > + 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"); What about the case of the PHY is used on a board without device tree? ACPI could be used, or there could be nothing because it is on a USB dongle etc. You should of defined in your device tree binding what the defaults are when properties are missing. So apply them when there is no DT available. Then we at least have the PHY in a known state. Andrew
On 2022/12/16 19:58, Heiner Kallweit wrote: > 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@starfivetech.com> >> --- >> 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. > Thanks. Parts of this patch exist already, after discussion unanimity was achieved, i will remove the parts of YT8531 in the next version. >> 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@gmail.com> >> + * >> */ >> >> +#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 */ } >> }; >> >
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 help Enables support for Motorcomm network PHYs. - Currently supports the YT8511 gigabit PHY. + Currently supports the YT8511 and YT8531 gigabit PHYs. 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@gmail.com> + * */ +#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 } +}; + +/* 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); + } + + 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"); + + 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 */ } };
This adds basic support for the Motorcomm YT8531 Gigabit Ethernet PHY. Signed-off-by: Yanhong Wang <yanhong.wang@starfivetech.com> --- drivers/net/phy/Kconfig | 3 +- drivers/net/phy/motorcomm.c | 202 ++++++++++++++++++++++++++++++++++++ 2 files changed, 204 insertions(+), 1 deletion(-)