On 9 September 2017 at 00:21, Florian Fainelli <f.fainelli@xxxxxxxxx> wrote: > On 09/08/2017 06:02 AM, Kunihiko Hayashi wrote: >> From: Jassi Brar <jaswinder.singh@xxxxxxxxxx> >> >> Add RTL8201F phy-id and the related functions to the driver. >> >> The original patch is as follows: >> https://patchwork.kernel.org/patch/2538341/ >> >> Signed-off-by: Jongsung Kim <neidhard.kim@xxxxxxx> >> Signed-off-by: Jassi Brar <jaswinder.singh@xxxxxxxxxx> >> Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@xxxxxxxxxxxxx> >> --- >> drivers/net/phy/realtek.c | 45 +++++++++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 45 insertions(+) >> >> diff --git a/drivers/net/phy/realtek.c b/drivers/net/phy/realtek.c >> index 9cbe645..d9974ce 100644 >> --- a/drivers/net/phy/realtek.c >> +++ b/drivers/net/phy/realtek.c >> @@ -29,10 +29,23 @@ >> #define RTL8211F_PAGE_SELECT 0x1f >> #define RTL8211F_TX_DELAY 0x100 >> >> +#define RTL8201F_ISR 0x1e >> +#define RTL8201F_PAGE_SELECT 0x1f > > We have a page select register define for the RTL8211F right above, so > surely we can make that a common definition? > That is just for the sake of consistency. I mean RTL8211 wouldn't look neat among everything else RTL8201. Also the page-select offsets just _happen_ to be same value... RTL8211E_INER_LINK_STATUS and RTL8211F_INER_LINK_STATUS are very different. >> +#define RTL8201F_IER 0x13 >> + >> MODULE_DESCRIPTION("Realtek PHY driver"); >> MODULE_AUTHOR("Johnson Leung"); >> MODULE_LICENSE("GPL"); >> >> +static int rtl8201_ack_interrupt(struct phy_device *phydev) >> +{ >> + int err; >> + >> + err = phy_read(phydev, RTL8201F_ISR); >> + >> + return (err < 0) ? err : 0; >> +} >> + >> static int rtl821x_ack_interrupt(struct phy_device *phydev) >> { >> int err; >> @@ -54,6 +67,25 @@ static int rtl8211f_ack_interrupt(struct phy_device *phydev) >> return (err < 0) ? err : 0; >> } >> >> +static int rtl8201_config_intr(struct phy_device *phydev) >> +{ >> + int err; >> + >> + /* switch to page 7 */ >> + phy_write(phydev, RTL8201F_PAGE_SELECT, 0x7); >> + >> + if (phydev->interrupts == PHY_INTERRUPT_ENABLED) >> + err = phy_write(phydev, RTL8201F_IER, >> + BIT(13) | BIT(12) | BIT(11)); > > Can you detail what bits 11, 12 and 13 do? Do they correspond to link, > duplex and pause changes by any chance? > Sorry no idea. The datasheet would say, and other functions too use such magic values. >> + else >> + err = phy_write(phydev, RTL8201F_IER, 0); >> + >> + /* restore to default page 0 */ >> + phy_write(phydev, RTL8201F_PAGE_SELECT, 0x0); >> + >> + return err; >> +} >> + > > Other than that, LGTM: > > Reviewed-by: Florian Fainelli <f.fainelli@xxxxxxxxx> > Thank you. -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
