Message ID | 20220810173733.795897-1-wei.fang@nxp.com (mailing list archive) |
---|---|
State | Changes Requested |
Delegated to: | Netdev Maintainers |
Headers | show |
Series | [net-next] net: phy: realtek: add support for RTL8821F(D)(I)-VD-CG | expand |
Context | Check | Description |
---|---|---|
netdev/tree_selection | success | Clearly marked for net-next |
netdev/fixes_present | success | Fixes tag not required for -next series |
netdev/subject_prefix | success | Link |
netdev/cover_letter | success | Single patches do not need cover letters |
netdev/patch_count | success | Link |
netdev/header_inline | success | No static functions without inline keyword in header files |
netdev/build_32bit | success | Errors and warnings before: 0 this patch: 0 |
netdev/cc_maintainers | success | CCed 8 of 8 maintainers |
netdev/build_clang | success | Errors and warnings before: 0 this patch: 0 |
netdev/module_param | success | Was 0 now: 0 |
netdev/verify_signedoff | success | Signed-off-by tag matches author and committer |
netdev/check_selftest | success | No net selftest shell script |
netdev/verify_fixes | success | No Fixes tag |
netdev/build_allmodconfig_warn | success | Errors and warnings before: 0 this patch: 0 |
netdev/checkpatch | warning | WARNING: line length of 82 exceeds 80 columns |
netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
netdev/source_inline | success | Was 0 now: 0 |
On 10.08.2022 19:37, wei.fang@nxp.com wrote: > From: Clark Wang <xiaoning.wang@nxp.com> > > RTL8821F(D)(I)-VD-CG is the pin-to-pin upgrade chip from > RTL8821F(D)(I)-CG. > Don't you mean 8211 instead of 8821 here? > Add new PHY ID for this chip. > It does not support RTL8211F_PHYCR2 anymore, so remove the w/r operation > of this register. > > Signed-off-by: Clark Wang <xiaoning.wang@nxp.com> > --- > drivers/net/phy/realtek.c | 48 +++++++++++++++++++++++++++++---------- > 1 file changed, 36 insertions(+), 12 deletions(-) > > diff --git a/drivers/net/phy/realtek.c b/drivers/net/phy/realtek.c > index a5671ab896b3..bfde22dc85f5 100644 > --- a/drivers/net/phy/realtek.c > +++ b/drivers/net/phy/realtek.c > @@ -70,6 +70,7 @@ > #define RTLGEN_SPEED_MASK 0x0630 > > #define RTL_GENERIC_PHYID 0x001cc800 > +#define RTL_8211FVD_PHYID 0x001cc878 > > MODULE_DESCRIPTION("Realtek PHY driver"); > MODULE_AUTHOR("Johnson Leung"); > @@ -80,6 +81,11 @@ struct rtl821x_priv { > u16 phycr2; > }; > > +static bool is_rtl8211fvd(u32 phy_id) Better add a has_phycr2 to struct rtl821x_priv. Then you have: if (priv->has_phycr2) do_something_with(priv->phycr2); > +{ > + return phy_id == RTL_8211FVD_PHYID; > +} > + > static int rtl821x_read_page(struct phy_device *phydev) > { > return __phy_read(phydev, RTL821x_PAGE_SELECT); > @@ -94,6 +100,7 @@ static int rtl821x_probe(struct phy_device *phydev) > { > struct device *dev = &phydev->mdio.dev; > struct rtl821x_priv *priv; > + u32 phy_id = phydev->drv->phy_id; > int ret; > > priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL); > @@ -108,13 +115,15 @@ static int rtl821x_probe(struct phy_device *phydev) > if (of_property_read_bool(dev->of_node, "realtek,aldps-enable")) > priv->phycr1 |= RTL8211F_ALDPS_PLL_OFF | RTL8211F_ALDPS_ENABLE | RTL8211F_ALDPS_XTAL_OFF; > > - ret = phy_read_paged(phydev, 0xa43, RTL8211F_PHYCR2); > - if (ret < 0) > - return ret; > + if (!is_rtl8211fvd(phy_id)) { > + ret = phy_read_paged(phydev, 0xa43, RTL8211F_PHYCR2); > + if (ret < 0) > + return ret; > > - priv->phycr2 = ret & RTL8211F_CLKOUT_EN; > - if (of_property_read_bool(dev->of_node, "realtek,clkout-disable")) > - priv->phycr2 &= ~RTL8211F_CLKOUT_EN; > + priv->phycr2 = ret & RTL8211F_CLKOUT_EN; > + if (of_property_read_bool(dev->of_node, "realtek,clkout-disable")) > + priv->phycr2 &= ~RTL8211F_CLKOUT_EN; > + } > > phydev->priv = priv; > > @@ -333,6 +342,7 @@ static int rtl8211f_config_init(struct phy_device *phydev) > { > struct rtl821x_priv *priv = phydev->priv; > struct device *dev = &phydev->mdio.dev; > + u32 phy_id = phydev->drv->phy_id; > u16 val_txdly, val_rxdly; > int ret; > > @@ -400,12 +410,14 @@ static int rtl8211f_config_init(struct phy_device *phydev) > val_rxdly ? "enabled" : "disabled"); > } > > - ret = phy_modify_paged(phydev, 0xa43, RTL8211F_PHYCR2, > - RTL8211F_CLKOUT_EN, priv->phycr2); > - if (ret < 0) { > - dev_err(dev, "clkout configuration failed: %pe\n", > - ERR_PTR(ret)); > - return ret; > + if (!is_rtl8211fvd(phy_id)) { > + ret = phy_modify_paged(phydev, 0xa43, RTL8211F_PHYCR2, > + RTL8211F_CLKOUT_EN, priv->phycr2); > + if (ret < 0) { > + dev_err(dev, "clkout configuration failed: %pe\n", > + ERR_PTR(ret)); > + return ret; > + } > } > > return genphy_soft_reset(phydev); > @@ -923,6 +935,18 @@ static struct phy_driver realtek_drvs[] = { > .resume = rtl821x_resume, > .read_page = rtl821x_read_page, > .write_page = rtl821x_write_page, > + }, { > + PHY_ID_MATCH_EXACT(RTL_8211FVD_PHYID), > + .name = "RTL8211F-VD Gigabit Ethernet", > + .probe = rtl821x_probe, > + .config_init = &rtl8211f_config_init, > + .read_status = rtlgen_read_status, > + .config_intr = &rtl8211f_config_intr, > + .handle_interrupt = rtl8211f_handle_interrupt, > + .suspend = genphy_suspend, > + .resume = rtl821x_resume, > + .read_page = rtl821x_read_page, > + .write_page = rtl821x_write_page, > }, { > .name = "Generic FE-GE Realtek PHY", > .match_phy_device = rtlgen_match_phy_device, And by the way, net-next is closed currently.
> -----Original Message----- > From: Heiner Kallweit <hkallweit1@gmail.com> > Sent: 2022年8月11日 3:55 > To: Wei Fang <wei.fang@nxp.com>; andrew@lunn.ch; linux@armlinux.org.uk; > davem@davemloft.net; edumazet@google.com; kuba@kernel.org; > pabeni@redhat.com; netdev@vger.kernel.org > Cc: Clark Wang <xiaoning.wang@nxp.com> > Subject: Re: [PATCH net-next] net: phy: realtek: add support for > RTL8821F(D)(I)-VD-CG > > On 10.08.2022 19:37, wei.fang@nxp.com wrote: > > From: Clark Wang <xiaoning.wang@nxp.com> > > > > RTL8821F(D)(I)-VD-CG is the pin-to-pin upgrade chip from > > RTL8821F(D)(I)-CG. > > > > Don't you mean 8211 instead of 8821 here? > Sorry, It’s written error, I'll correct it in next version. > > Add new PHY ID for this chip. > > It does not support RTL8211F_PHYCR2 anymore, so remove the w/r > > operation of this register. > > > > Signed-off-by: Clark Wang <xiaoning.wang@nxp.com> > > --- > > drivers/net/phy/realtek.c | 48 > > +++++++++++++++++++++++++++++---------- > > 1 file changed, 36 insertions(+), 12 deletions(-) > > > > diff --git a/drivers/net/phy/realtek.c b/drivers/net/phy/realtek.c > > index a5671ab896b3..bfde22dc85f5 100644 > > --- a/drivers/net/phy/realtek.c > > +++ b/drivers/net/phy/realtek.c > > @@ -70,6 +70,7 @@ > > #define RTLGEN_SPEED_MASK 0x0630 > > > > #define RTL_GENERIC_PHYID 0x001cc800 > > +#define RTL_8211FVD_PHYID 0x001cc878 > > > > MODULE_DESCRIPTION("Realtek PHY driver"); > MODULE_AUTHOR("Johnson > > Leung"); @@ -80,6 +81,11 @@ struct rtl821x_priv { > > u16 phycr2; > > }; > > > > +static bool is_rtl8211fvd(u32 phy_id) > > Better add a has_phycr2 to struct rtl821x_priv. Then you have: > > if (priv->has_phycr2) > do_something_with(priv->phycr2); > Thanks for your suggestion, I will apply this method in next version. > > +{ > > + return phy_id == RTL_8211FVD_PHYID; > > +} > > + > > static int rtl821x_read_page(struct phy_device *phydev) { > > return __phy_read(phydev, RTL821x_PAGE_SELECT); @@ -94,6 +100,7 > @@ > > static int rtl821x_probe(struct phy_device *phydev) { > > struct device *dev = &phydev->mdio.dev; > > struct rtl821x_priv *priv; > > + u32 phy_id = phydev->drv->phy_id; > > int ret; > > > > priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL); @@ -108,13 > > +115,15 @@ static int rtl821x_probe(struct phy_device *phydev) > > if (of_property_read_bool(dev->of_node, "realtek,aldps-enable")) > > priv->phycr1 |= RTL8211F_ALDPS_PLL_OFF | > RTL8211F_ALDPS_ENABLE | > > RTL8211F_ALDPS_XTAL_OFF; > > > > - ret = phy_read_paged(phydev, 0xa43, RTL8211F_PHYCR2); > > - if (ret < 0) > > - return ret; > > + if (!is_rtl8211fvd(phy_id)) { > > + ret = phy_read_paged(phydev, 0xa43, RTL8211F_PHYCR2); > > + if (ret < 0) > > + return ret; > > > > - priv->phycr2 = ret & RTL8211F_CLKOUT_EN; > > - if (of_property_read_bool(dev->of_node, "realtek,clkout-disable")) > > - priv->phycr2 &= ~RTL8211F_CLKOUT_EN; > > + priv->phycr2 = ret & RTL8211F_CLKOUT_EN; > > + if (of_property_read_bool(dev->of_node, "realtek,clkout-disable")) > > + priv->phycr2 &= ~RTL8211F_CLKOUT_EN; > > + } > > > > phydev->priv = priv; > > > > @@ -333,6 +342,7 @@ static int rtl8211f_config_init(struct phy_device > > *phydev) { > > struct rtl821x_priv *priv = phydev->priv; > > struct device *dev = &phydev->mdio.dev; > > + u32 phy_id = phydev->drv->phy_id; > > u16 val_txdly, val_rxdly; > > int ret; > > > > @@ -400,12 +410,14 @@ static int rtl8211f_config_init(struct phy_device > *phydev) > > val_rxdly ? "enabled" : "disabled"); > > } > > > > - ret = phy_modify_paged(phydev, 0xa43, RTL8211F_PHYCR2, > > - RTL8211F_CLKOUT_EN, priv->phycr2); > > - if (ret < 0) { > > - dev_err(dev, "clkout configuration failed: %pe\n", > > - ERR_PTR(ret)); > > - return ret; > > + if (!is_rtl8211fvd(phy_id)) { > > + ret = phy_modify_paged(phydev, 0xa43, RTL8211F_PHYCR2, > > + RTL8211F_CLKOUT_EN, priv->phycr2); > > + if (ret < 0) { > > + dev_err(dev, "clkout configuration failed: %pe\n", > > + ERR_PTR(ret)); > > + return ret; > > + } > > } > > > > return genphy_soft_reset(phydev); > > @@ -923,6 +935,18 @@ static struct phy_driver realtek_drvs[] = { > > .resume = rtl821x_resume, > > .read_page = rtl821x_read_page, > > .write_page = rtl821x_write_page, > > + }, { > > + PHY_ID_MATCH_EXACT(RTL_8211FVD_PHYID), > > + .name = "RTL8211F-VD Gigabit Ethernet", > > + .probe = rtl821x_probe, > > + .config_init = &rtl8211f_config_init, > > + .read_status = rtlgen_read_status, > > + .config_intr = &rtl8211f_config_intr, > > + .handle_interrupt = rtl8211f_handle_interrupt, > > + .suspend = genphy_suspend, > > + .resume = rtl821x_resume, > > + .read_page = rtl821x_read_page, > > + .write_page = rtl821x_write_page, > > }, { > > .name = "Generic FE-GE Realtek PHY", > > .match_phy_device = rtlgen_match_phy_device, > > And by the way, net-next is closed currently. Okay, I will post the V2 when the net-next is re-opened, Thanks for your kindly reminder.
diff --git a/drivers/net/phy/realtek.c b/drivers/net/phy/realtek.c index a5671ab896b3..bfde22dc85f5 100644 --- a/drivers/net/phy/realtek.c +++ b/drivers/net/phy/realtek.c @@ -70,6 +70,7 @@ #define RTLGEN_SPEED_MASK 0x0630 #define RTL_GENERIC_PHYID 0x001cc800 +#define RTL_8211FVD_PHYID 0x001cc878 MODULE_DESCRIPTION("Realtek PHY driver"); MODULE_AUTHOR("Johnson Leung"); @@ -80,6 +81,11 @@ struct rtl821x_priv { u16 phycr2; }; +static bool is_rtl8211fvd(u32 phy_id) +{ + return phy_id == RTL_8211FVD_PHYID; +} + static int rtl821x_read_page(struct phy_device *phydev) { return __phy_read(phydev, RTL821x_PAGE_SELECT); @@ -94,6 +100,7 @@ static int rtl821x_probe(struct phy_device *phydev) { struct device *dev = &phydev->mdio.dev; struct rtl821x_priv *priv; + u32 phy_id = phydev->drv->phy_id; int ret; priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL); @@ -108,13 +115,15 @@ static int rtl821x_probe(struct phy_device *phydev) if (of_property_read_bool(dev->of_node, "realtek,aldps-enable")) priv->phycr1 |= RTL8211F_ALDPS_PLL_OFF | RTL8211F_ALDPS_ENABLE | RTL8211F_ALDPS_XTAL_OFF; - ret = phy_read_paged(phydev, 0xa43, RTL8211F_PHYCR2); - if (ret < 0) - return ret; + if (!is_rtl8211fvd(phy_id)) { + ret = phy_read_paged(phydev, 0xa43, RTL8211F_PHYCR2); + if (ret < 0) + return ret; - priv->phycr2 = ret & RTL8211F_CLKOUT_EN; - if (of_property_read_bool(dev->of_node, "realtek,clkout-disable")) - priv->phycr2 &= ~RTL8211F_CLKOUT_EN; + priv->phycr2 = ret & RTL8211F_CLKOUT_EN; + if (of_property_read_bool(dev->of_node, "realtek,clkout-disable")) + priv->phycr2 &= ~RTL8211F_CLKOUT_EN; + } phydev->priv = priv; @@ -333,6 +342,7 @@ static int rtl8211f_config_init(struct phy_device *phydev) { struct rtl821x_priv *priv = phydev->priv; struct device *dev = &phydev->mdio.dev; + u32 phy_id = phydev->drv->phy_id; u16 val_txdly, val_rxdly; int ret; @@ -400,12 +410,14 @@ static int rtl8211f_config_init(struct phy_device *phydev) val_rxdly ? "enabled" : "disabled"); } - ret = phy_modify_paged(phydev, 0xa43, RTL8211F_PHYCR2, - RTL8211F_CLKOUT_EN, priv->phycr2); - if (ret < 0) { - dev_err(dev, "clkout configuration failed: %pe\n", - ERR_PTR(ret)); - return ret; + if (!is_rtl8211fvd(phy_id)) { + ret = phy_modify_paged(phydev, 0xa43, RTL8211F_PHYCR2, + RTL8211F_CLKOUT_EN, priv->phycr2); + if (ret < 0) { + dev_err(dev, "clkout configuration failed: %pe\n", + ERR_PTR(ret)); + return ret; + } } return genphy_soft_reset(phydev); @@ -923,6 +935,18 @@ static struct phy_driver realtek_drvs[] = { .resume = rtl821x_resume, .read_page = rtl821x_read_page, .write_page = rtl821x_write_page, + }, { + PHY_ID_MATCH_EXACT(RTL_8211FVD_PHYID), + .name = "RTL8211F-VD Gigabit Ethernet", + .probe = rtl821x_probe, + .config_init = &rtl8211f_config_init, + .read_status = rtlgen_read_status, + .config_intr = &rtl8211f_config_intr, + .handle_interrupt = rtl8211f_handle_interrupt, + .suspend = genphy_suspend, + .resume = rtl821x_resume, + .read_page = rtl821x_read_page, + .write_page = rtl821x_write_page, }, { .name = "Generic FE-GE Realtek PHY", .match_phy_device = rtlgen_match_phy_device,