linux-kernel - Re: [PATCH net-next 3/3] net: phy: realtek: add RTL8201F phy-id and functions

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Sat, 9 Sep 2017 17:55:17 +0200
From:   Andrew Lunn <andrew@...n.ch>
To:     Jassi Brar <jaswinder.singh@...aro.org>
Cc:     Florian Fainelli <f.fainelli@...il.com>,
        Kunihiko Hayashi <hayashi.kunihiko@...ionext.com>,
        netdev@...r.kernel.org, "David S. Miller" <davem@...emloft.net>,
        Rob Herring <robh+dt@...nel.org>,
        Mark Rutland <mark.rutland@....com>,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>,
        lkml <linux-kernel@...r.kernel.org>,
        Devicetree List <devicetree@...r.kernel.org>,
        Masahiro Yamada <yamada.masahiro@...ionext.com>,
        Masami Hiramatsu <masami.hiramatsu@...aro.org>,
        Jongsung Kim <neidhard.kim@....com>
Subject: Re: [PATCH net-next 3/3] net: phy: realtek: add RTL8201F phy-id and
 functions

On Sat, Sep 09, 2017 at 09:03:05AM +0530, Jassi Brar wrote:
> On 9 September 2017 at 00:21, Florian Fainelli <f.fainelli@...il.com> wrote:
> > On 09/08/2017 06:02 AM, Kunihiko Hayashi wrote:
> >> From: Jassi Brar <jaswinder.singh@...aro.org>
> >>
> >> 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@....com>
> >> Signed-off-by: Jassi Brar <jaswinder.singh@...aro.org>
> >> Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@...ionext.com>
> >> ---
> >>  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...

If you look at all the other supported PHYs, they all consistently use
the same page register across models. Marvell is always 22, mscc is
always 31, vitesse is always 31.

I would say it is a safe bet that all realtek PHYs will use 0x1f for
page select. So please add a patch which renames RTL8211F_PAGE_SELECT
to RTL821x_PAGE_SELECT.

It is best to do this now. I spent a while cleaning up the mess the
Marvell driver had got into with its page select code. Lots of
duplicate code and defines doing the same thing.

	  Andrew