From: Arnd Bergmann <arnd@arndb•de>
To: zhangfei <zhangfei.gao@linaro•org>
Cc: linux-arm-kernel@lists•infradead.org,
Sergei Shtylyov <sergei.shtylyov@cogentembedded•com>,
Florian Fainelli <f.fainelli@gmail•com>,
Mark Rutland <mark.rutland@arm•com>,
"devicetree@vger•kernel.org" <devicetree@vger•kernel.org>,
Russell King <linux@arm•linux.org.uk>,
Eric Dumazet <eric.dumazet@gmail•com>,
netdev <netdev@vger•kernel.org>,
xuwei5@hisilicon•com, David Laight <David.Laight@aculab•com>,
David Miller <davem@davemloft•net>
Subject: Re: [PATCH v8 2/3] net: hisilicon: new hip04 MDIO driver
Date: Tue, 22 Apr 2014 16:30:07 +0200 [thread overview]
Message-ID: <201404221630.07865.arnd@arndb.de> (raw)
In-Reply-To: <535679A6.4020103@linaro.org>
On Tuesday 22 April 2014, zhangfei wrote:
> On 04/22/2014 04:22 PM, Arnd Bergmann wrote:
>
> >> It's private register of the phy marvell 88e1512.
> >> To make it clearer using define instead.
> >> #define MII_MARVELL_PHY_PAGE 22
> >>
> >> The registers has been grouped into several pages, access register need
> >> choose which page first.
> >
> > You shouldn't touch the PHY private registers in the main driver though,
> > this should be purely handled by drivers/net/phy/marvell.c.
> >
> > I don't see support for 88e1512 there, only 88e1510 and lots of older
> > ones, but I assume it isn't hard to add.
> >
>
> 88e1512 driver is already supported, same as 88e1510.
> #define MARVELL_PHY_ID_MASK 0xfffffff0
> So it should support 88e151x.
>
> Reset is required here for get_phy_id, otherwise only 0 can be get.
> phy_device_create will not be called, and can not match any driver.
>
> However in the experiment, it is found BMCR_RESET is not required in fact.
> Only hip04_mdio_write(bus, i, MII_MARVELL_PHY_PAGE, 0) has to be set.
> 88e151x registers are divided into pages.
> Generic MII registers is in page 0, including MII_PHYSID1 and MII_PHYSID2.
> Unfortunately the default page is not 0, so get_phy_id will fail.
>
> So bus->reset still required to set the page to 0, prepared for get_phy_id.
But it means that the hip04_mdio driver potentially won't work if connected
to something other than a Marvell PHY.
I noticed that the marvell_of_reg_init() does this at init time:
saved_page = phy_read(phydev, MII_MARVELL_PHY_PAGE);
... /* perform init */
if (page_changed)
phy_write(phydev, MII_MARVELL_PHY_PAGE, saved_page);
Is this a bug? Maybe it should always set page 0 when leaving
this function.
Arnd
next prev parent reply other threads:[~2014-04-22 14:30 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-04-19 1:12 [PATCH v8 0/3] add hisilicon hip04 ethernet driver Zhangfei Gao
2014-04-19 1:12 ` [PATCH v8 1/3] Documentation: add Device tree bindings for Hisilicon hip04 ethernet Zhangfei Gao
2014-04-19 1:12 ` [PATCH v8 2/3] net: hisilicon: new hip04 MDIO driver Zhangfei Gao
2014-04-21 17:58 ` Sergei Shtylyov
2014-04-21 18:03 ` Florian Fainelli
2014-04-21 18:21 ` Sergei Shtylyov
2014-04-22 6:03 ` zhangfei
[not found] ` <5356063C.6070100-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
2014-04-22 8:22 ` Arnd Bergmann
2014-04-22 14:16 ` zhangfei
2014-04-22 14:30 ` Arnd Bergmann [this message]
2014-04-22 14:58 ` zhangfei
[not found] ` <53568398.2030800-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
2014-04-24 12:28 ` Arnd Bergmann
2014-04-24 13:00 ` zhangfei
2014-04-19 1:13 ` [PATCH v8 3/3] net: hisilicon: new hip04 ethernet driver Zhangfei Gao
2014-12-07 0:42 ` Alexander Graf
2014-12-07 3:28 ` Ding Tianhong
2014-12-07 9:49 ` Alexander Graf
2014-12-07 20:09 ` Arnd Bergmann
2014-12-08 1:48 ` Ding Tianhong
2014-12-10 3:51 ` Ding Tianhong
2014-12-10 6:45 ` Ding Tianhong
2014-12-10 9:35 ` Arnd Bergmann
2014-12-10 16:07 ` David Miller
2014-12-10 16:36 ` Arnd Bergmann
2014-12-10 17:02 ` David Miller
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=201404221630.07865.arnd@arndb.de \
--to=arnd@arndb$(echo .)de \
--cc=David.Laight@aculab$(echo .)com \
--cc=davem@davemloft$(echo .)net \
--cc=devicetree@vger$(echo .)kernel.org \
--cc=eric.dumazet@gmail$(echo .)com \
--cc=f.fainelli@gmail$(echo .)com \
--cc=linux-arm-kernel@lists$(echo .)infradead.org \
--cc=linux@arm$(echo .)linux.org.uk \
--cc=mark.rutland@arm$(echo .)com \
--cc=netdev@vger$(echo .)kernel.org \
--cc=sergei.shtylyov@cogentembedded$(echo .)com \
--cc=xuwei5@hisilicon$(echo .)com \
--cc=zhangfei.gao@linaro$(echo .)org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox