[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <DB9PR04MB81062AB20D311BEAEE0514DA88649@DB9PR04MB8106.eurprd04.prod.outlook.com>
Date: Thu, 11 Aug 2022 01:48:56 +0000
From: Wei Fang <wei.fang@....com>
To: Heiner Kallweit <hkallweit1@...il.com>,
"andrew@...n.ch" <andrew@...n.ch>,
"linux@...linux.org.uk" <linux@...linux.org.uk>,
"davem@...emloft.net" <davem@...emloft.net>,
"edumazet@...gle.com" <edumazet@...gle.com>,
"kuba@...nel.org" <kuba@...nel.org>,
"pabeni@...hat.com" <pabeni@...hat.com>,
"netdev@...r.kernel.org" <netdev@...r.kernel.org>
CC: Clark Wang <xiaoning.wang@....com>
Subject: RE: [PATCH net-next] net: phy: realtek: add support for
RTL8821F(D)(I)-VD-CG
> -----Original Message-----
> From: Heiner Kallweit <hkallweit1@...il.com>
> Sent: 2022年8月11日 3:55
> To: Wei Fang <wei.fang@....com>; andrew@...n.ch; linux@...linux.org.uk;
> davem@...emloft.net; edumazet@...gle.com; kuba@...nel.org;
> pabeni@...hat.com; netdev@...r.kernel.org
> Cc: Clark Wang <xiaoning.wang@....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@....com wrote:
> > From: Clark Wang <xiaoning.wang@....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@....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.
Powered by blists - more mailing lists