From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Lunn Subject: Re: [PATCH 1/4 RFCv2] net: phy: realtek: Support RTL8366RB variant Date: Tue, 29 May 2018 14:34:44 +0200 Message-ID: <20180529123444.GD10919@lunn.ch> References: <20180528174752.6806-1-linus.walleij@linaro.org> <20180528174752.6806-2-linus.walleij@linaro.org> Mime-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Transfer-Encoding: 8bit Cc: Vivien Didelot , Florian Fainelli , netdev@vger.kernel.org, openwrt-devel@lists.openwrt.org, LEDE Development List , Antti =?iso-8859-1?Q?Sepp=E4l=E4?= , Roman Yeryomin , Colin Leitner , Gabor Juhos To: Linus Walleij Return-path: Received: from vps0.lunn.ch ([185.16.172.187]:48094 "EHLO vps0.lunn.ch" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S933506AbeE2Mer (ORCPT ); Tue, 29 May 2018 08:34:47 -0400 Content-Disposition: inline In-Reply-To: <20180528174752.6806-2-linus.walleij@linaro.org> Sender: netdev-owner@vger.kernel.org List-ID: On Mon, May 28, 2018 at 07:47:49PM +0200, Linus Walleij wrote: > The RTL8366RB is an ASIC with five internal PHYs for > LAN0..LAN3 and WAN. The PHYs are spawn off the main > device so they can be handled in a distributed manner > by the Realtek PHY driver. All that is really needed > is the power save feature enablement and letting the > PHY driver core pick up the IRQ from the switch chip. > > Cc: Antti Seppälä > Cc: Roman Yeryomin > Cc: Colin Leitner > Cc: Gabor Juhos > Cc: Florian Fainelli > Signed-off-by: Linus Walleij > --- > ChangeLog RFCv1->RFCv2: > - No real changes. > --- > drivers/net/phy/realtek.c | 32 ++++++++++++++++++++++++++++++++ > 1 file changed, 32 insertions(+) > > diff --git a/drivers/net/phy/realtek.c b/drivers/net/phy/realtek.c > index 9f48ecf9c627..21624d1c9d38 100644 > --- a/drivers/net/phy/realtek.c > +++ b/drivers/net/phy/realtek.c > @@ -37,6 +37,9 @@ > #define RTL8201F_ISR 0x1e > #define RTL8201F_IER 0x13 > > +#define RTL8366RB_POWER_SAVE 0x21 > +#define RTL8366RB_POWER_SAVE_ON 0x1000 > + > MODULE_DESCRIPTION("Realtek PHY driver"); > MODULE_AUTHOR("Johnson Leung"); > MODULE_LICENSE("GPL"); > @@ -145,6 +148,22 @@ static int rtl8211f_config_init(struct phy_device *phydev) > return phy_modify_paged(phydev, 0xd08, 0x11, RTL8211F_TX_DELAY, val); > } > > +static int rtl8366rb_config_init(struct phy_device *phydev) > +{ > + int ret; > + u16 reg; > + > + ret = genphy_config_init(phydev); > + if (ret < 0) > + return ret; > + > + reg = phy_read(phydev, RTL8366RB_POWER_SAVE); > + reg |= RTL8366RB_POWER_SAVE_ON; > + phy_write(phydev, RTL8366RB_POWER_SAVE, reg); > + > + return 0; > +} > + > static struct phy_driver realtek_drvs[] = { > { > .phy_id = 0x00008201, > @@ -207,6 +226,18 @@ static struct phy_driver realtek_drvs[] = { > .resume = genphy_resume, > .read_page = rtl821x_read_page, > .write_page = rtl821x_write_page, > + }, { > + /* The main part of this DSA is in drivers/net/dsa */ Hi Linus I would not bother with this comment. We don't say, The main part of this driver is in drivers/net/ethernet/... PHY drivers should be completely separate to MAC drivers. Otherwise this looks good. Andrew > + .phy_id = 0x001cc961, > + .name = "RTL8366RB Gigabit Ethernet", > + .phy_id_mask = 0x001fffff, > + .features = PHY_GBIT_FEATURES, > + .flags = PHY_HAS_INTERRUPT, > + .config_aneg = &genphy_config_aneg, > + .config_init = &rtl8366rb_config_init, > + .read_status = &genphy_read_status, > + .suspend = genphy_suspend, > + .resume = genphy_resume, > }, > }; > > @@ -218,6 +249,7 @@ static struct mdio_device_id __maybe_unused realtek_tbl[] = { > { 0x001cc914, 0x001fffff }, > { 0x001cc915, 0x001fffff }, > { 0x001cc916, 0x001fffff }, > + { 0x001cc961, 0x001fffff }, > { } > }; > > -- > 2.17.0 >