| Message ID | 1397869980-21187-3-git-send-email-zhangfei.gao@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hello. On 04/19/2014 05:12 AM, Zhangfei Gao wrote: > Hisilicon hip04 platform mdio driver > Reuse Marvell phy drivers/net/phy/marvell.c > Signed-off-by: Zhangfei Gao <zhangfei.gao@linaro.org> [...] > diff --git a/drivers/net/ethernet/hisilicon/hip04_mdio.c b/drivers/net/ethernet/hisilicon/hip04_mdio.c > new file mode 100644 > index 0000000..19826a3 > --- /dev/null > +++ b/drivers/net/ethernet/hisilicon/hip04_mdio.c > @@ -0,0 +1,185 @@ > + Empty line not needed here. > +/* Copyright (c) 2014 Linaro Ltd. > + * Copyright (c) 2014 Hisilicon Limited. > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License as published by > + * the Free Software Foundation; either version 2 of the License, or > + * (at your option) any later version. > + */ [...] > +static int hip04_mdio_reset(struct mii_bus *bus) > +{ > + int temp, err, i; > + > + for (i = 0; i < PHY_MAX_ADDR; i++) { > + hip04_mdio_write(bus, i, 22, 0); Why? What kind of a register this is? <uapi/linux/mii.h> tells me it's MII_SREVISION... > + temp = hip04_mdio_read(bus, i, MII_BMCR); You're not checking for error... > + temp |= BMCR_RESET; > + err = hip04_mdio_write(bus, i, MII_BMCR, temp); Hmm, why you're open coding BMCR reset? There's phy_init_hw() doing this correctly... > + if (err < 0) > + return err; > + } > + > + mdelay(500); > + return 0; > +} > + > +static int hip04_mdio_probe(struct platform_device *pdev) > +{ > + struct resource *r; > + struct mii_bus *bus; > + struct hip04_mdio_priv *priv; > + int ret; > + > + bus = mdiobus_alloc_size(sizeof(struct hip04_mdio_priv)); > + if (!bus) { > + dev_err(&pdev->dev, "Cannot allocate MDIO bus\n"); > + return -ENOMEM; > + } > + > + bus->name = "hip04_mdio_bus"; > + bus->read = hip04_mdio_read; > + bus->write = hip04_mdio_write; > + bus->reset = hip04_mdio_reset; Ah... However I don't think it a good implementation of that bus method... [...] WBR, Sergei
2014-04-21 10:58 GMT-07:00 Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>: > Hello. > > > On 04/19/2014 05:12 AM, Zhangfei Gao wrote: > >> Hisilicon hip04 platform mdio driver >> Reuse Marvell phy drivers/net/phy/marvell.c > > >> Signed-off-by: Zhangfei Gao <zhangfei.gao@linaro.org> > > [...] > > >> diff --git a/drivers/net/ethernet/hisilicon/hip04_mdio.c >> b/drivers/net/ethernet/hisilicon/hip04_mdio.c >> new file mode 100644 >> index 0000000..19826a3 >> --- /dev/null >> +++ b/drivers/net/ethernet/hisilicon/hip04_mdio.c >> @@ -0,0 +1,185 @@ >> + > > > Empty line not needed here. > > >> +/* Copyright (c) 2014 Linaro Ltd. >> + * Copyright (c) 2014 Hisilicon Limited. >> + * >> + * This program is free software; you can redistribute it and/or modify >> + * it under the terms of the GNU General Public License as published by >> + * the Free Software Foundation; either version 2 of the License, or >> + * (at your option) any later version. >> + */ > > > [...] > > >> +static int hip04_mdio_reset(struct mii_bus *bus) >> +{ >> + int temp, err, i; >> + >> + for (i = 0; i < PHY_MAX_ADDR; i++) { >> + hip04_mdio_write(bus, i, 22, 0); > > > Why? What kind of a register this is? <uapi/linux/mii.h> tells me it's > MII_SREVISION... I think this rather means clause 22 as opposed to clause 45. > > >> + temp = hip04_mdio_read(bus, i, MII_BMCR); > > > You're not checking for error... > > >> + temp |= BMCR_RESET; >> + err = hip04_mdio_write(bus, i, MII_BMCR, temp); > > > Hmm, why you're open coding BMCR reset? There's phy_init_hw() doing this > correctly... Except that this runs way before we have created the PHY driver, so we can't use that function just yet. I already asked about this, and he explained that this was because the PHY devices he uses are not responding correcty to MII_PHYSID1/2 reads. > > >> + if (err < 0) >> + return err; >> + } >> + >> + mdelay(500); >> + return 0; >> +} >> + >> +static int hip04_mdio_probe(struct platform_device *pdev) >> +{ >> + struct resource *r; >> + struct mii_bus *bus; >> + struct hip04_mdio_priv *priv; >> + int ret; >> + >> + bus = mdiobus_alloc_size(sizeof(struct hip04_mdio_priv)); >> + if (!bus) { >> + dev_err(&pdev->dev, "Cannot allocate MDIO bus\n"); >> + return -ENOMEM; >> + } >> + >> + bus->name = "hip04_mdio_bus"; >> + bus->read = hip04_mdio_read; >> + bus->write = hip04_mdio_write; >> + bus->reset = hip04_mdio_reset; > > > Ah... However I don't think it a good implementation of that bus > method... > > [...] > > WBR, Sergei >
On 04/21/2014 10:03 PM, Florian Fainelli wrote: >>> Hisilicon hip04 platform mdio driver >>> Reuse Marvell phy drivers/net/phy/marvell.c >>> Signed-off-by: Zhangfei Gao <zhangfei.gao@linaro.org> >> [...] >>> diff --git a/drivers/net/ethernet/hisilicon/hip04_mdio.c >>> b/drivers/net/ethernet/hisilicon/hip04_mdio.c >>> new file mode 100644 >>> index 0000000..19826a3 >>> --- /dev/null >>> +++ b/drivers/net/ethernet/hisilicon/hip04_mdio.c >>> @@ -0,0 +1,185 @@ [...] >>> +static int hip04_mdio_reset(struct mii_bus *bus) >>> +{ >>> + int temp, err, i; >>> + >>> + for (i = 0; i < PHY_MAX_ADDR; i++) { >>> + hip04_mdio_write(bus, i, 22, 0); >> Why? What kind of a register this is? <uapi/linux/mii.h> tells me it's >> MII_SREVISION... > I think this rather means clause 22 as opposed to clause 45. No, the corresponding hip04_mdio_write()'s parameter is a register #, so this is a write of 0 to register #22. A comment certainly wouldn't hurt here... >>> + temp = hip04_mdio_read(bus, i, MII_BMCR); >> You're not checking for error... >>> + temp |= BMCR_RESET; >>> + err = hip04_mdio_write(bus, i, MII_BMCR, temp); >> Hmm, why you're open coding BMCR reset? There's phy_init_hw() doing this >> correctly... > Except that this runs way before we have created the PHY driver, so we > can't use that function just yet. Ah, you're right. > I already asked about this, and he > explained that this was because the PHY devices he uses are not > responding correcty to MII_PHYSID1/2 reads. So, this manual reset loop helps with reading the ID registers? A comment wouldn't hurt either... >>> + if (err < 0) >>> + return err; I'm not at all sure we want to leave the reset loop on a first write error. >>> + } >>> + >>> + mdelay(500); I'm not sure this is enough, given that in phy_init_hw() we poll for 600 ms + 1 ms. >>> + return 0; >>> +} >>> + >>> +static int hip04_mdio_probe(struct platform_device *pdev) >>> +{ >>> + struct resource *r; >>> + struct mii_bus *bus; >>> + struct hip04_mdio_priv *priv; >>> + int ret; >>> + >>> + bus = mdiobus_alloc_size(sizeof(struct hip04_mdio_priv)); >>> + if (!bus) { >>> + dev_err(&pdev->dev, "Cannot allocate MDIO bus\n"); >>> + return -ENOMEM; >>> + } >>> + >>> + bus->name = "hip04_mdio_bus"; >>> + bus->read = hip04_mdio_read; >>> + bus->write = hip04_mdio_write; >>> + bus->reset = hip04_mdio_reset; >> Ah... However I don't think it a good implementation of that bus >> method... I assumed this method exists to do a hardware reset of the whole bus, not to do a loop of soft-resetting all PHYs... WBR, Sergei
On 04/22/2014 02:21 AM, Sergei Shtylyov wrote: > On 04/21/2014 10:03 PM, Florian Fainelli wrote: > >>>> Hisilicon hip04 platform mdio driver >>>> Reuse Marvell phy drivers/net/phy/marvell.c > >>>> Signed-off-by: Zhangfei Gao <zhangfei.gao@linaro.org> >>> [...] > >>>> diff --git a/drivers/net/ethernet/hisilicon/hip04_mdio.c >>>> b/drivers/net/ethernet/hisilicon/hip04_mdio.c >>>> new file mode 100644 >>>> index 0000000..19826a3 >>>> --- /dev/null >>>> +++ b/drivers/net/ethernet/hisilicon/hip04_mdio.c >>>> @@ -0,0 +1,185 @@ > > [...] > >>>> +static int hip04_mdio_reset(struct mii_bus *bus) >>>> +{ >>>> + int temp, err, i; >>>> + >>>> + for (i = 0; i < PHY_MAX_ADDR; i++) { >>>> + hip04_mdio_write(bus, i, 22, 0); > >>> Why? What kind of a register this is? <uapi/linux/mii.h> tells me >>> it's >>> MII_SREVISION... > >> I think this rather means clause 22 as opposed to clause 45. > > No, the corresponding hip04_mdio_write()'s parameter is a register > #, so this is a write of 0 to register #22. A comment certainly wouldn't > hurt here... 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. > >>>> + temp = hip04_mdio_read(bus, i, MII_BMCR); > >>> You're not checking for error... > >>>> + temp |= BMCR_RESET; >>>> + err = hip04_mdio_write(bus, i, MII_BMCR, temp); > >>> Hmm, why you're open coding BMCR reset? There's phy_init_hw() >>> doing this >>> correctly... > >> Except that this runs way before we have created the PHY driver, so we >> can't use that function just yet. > > Ah, you're right. > >> I already asked about this, and he >> explained that this was because the PHY devices he uses are not >> responding correcty to MII_PHYSID1/2 reads. > > So, this manual reset loop helps with reading the ID registers? A > comment wouldn't hurt either... Yes, it required for get_phy_id, will add comments. > >>>> + if (err < 0) >>>> + return err; > > I'm not at all sure we want to leave the reset loop on a first write > error. Yes, continue can be used instead. > >>>> + } >>>> + >>>> + mdelay(500); > > I'm not sure this is enough, given that in phy_init_hw() we poll for > 600 ms + 1 ms. Though not find clear delay time required from the phy spec, the experiment shows OK. Also here can not verify the BMCR_RESET bit, 0xffff will returned if the phy is not exists. > >>>> + return 0; >>>> +} >>>> + >>>> +static int hip04_mdio_probe(struct platform_device *pdev) >>>> +{ >>>> + struct resource *r; >>>> + struct mii_bus *bus; >>>> + struct hip04_mdio_priv *priv; >>>> + int ret; >>>> + >>>> + bus = mdiobus_alloc_size(sizeof(struct hip04_mdio_priv)); >>>> + if (!bus) { >>>> + dev_err(&pdev->dev, "Cannot allocate MDIO bus\n"); >>>> + return -ENOMEM; >>>> + } >>>> + >>>> + bus->name = "hip04_mdio_bus"; >>>> + bus->read = hip04_mdio_read; >>>> + bus->write = hip04_mdio_write; >>>> + bus->reset = hip04_mdio_reset; > >>> Ah... However I don't think it a good implementation of that bus >>> method... > > I assumed this method exists to do a hardware reset of the whole > bus, not to do a loop of soft-resetting all PHYs... > It is required for each phy, if no such reset, the specific phy can NOT be detected and get_phy_id returns 0. Thanks
On Tuesday 22 April 2014 14:03:40 zhangfei wrote: > On 04/22/2014 02:21 AM, Sergei Shtylyov wrote: > > On 04/21/2014 10:03 PM, Florian Fainelli wrote: > > > >>>> Hisilicon hip04 platform mdio driver > >>>> Reuse Marvell phy drivers/net/phy/marvell.c > > > >>>> Signed-off-by: Zhangfei Gao <zhangfei.gao@linaro.org> > >>> [...] > > > >>>> diff --git a/drivers/net/ethernet/hisilicon/hip04_mdio.c > >>>> b/drivers/net/ethernet/hisilicon/hip04_mdio.c > >>>> new file mode 100644 > >>>> index 0000000..19826a3 > >>>> --- /dev/null > >>>> +++ b/drivers/net/ethernet/hisilicon/hip04_mdio.c > >>>> @@ -0,0 +1,185 @@ > >>>> +static int hip04_mdio_reset(struct mii_bus *bus) > >>>> +{ > >>>> + int temp, err, i; > >>>> + > >>>> + for (i = 0; i < PHY_MAX_ADDR; i++) { > >>>> + hip04_mdio_write(bus, i, 22, 0); > > > >>> Why? What kind of a register this is? <uapi/linux/mii.h> tells me > >>> it's > >>> MII_SREVISION... > > > >> I think this rather means clause 22 as opposed to clause 45. > > > > No, the corresponding hip04_mdio_write()'s parameter is a register > > #, so this is a write of 0 to register #22. A comment certainly wouldn't > > hurt here... > > 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. Arnd
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. Thanks
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
On 04/22/2014 10:30 PM, Arnd Bergmann wrote: > 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. > It is correct behavior return to the original page. First get_phy_id, then match driver according to the id, then operation in the specific driver have the chance to run, including marvell_of_reg_init etc. Just unlucky the default value of PHY_PAGE after power on is not 0. Thanks
On Tuesday 22 April 2014, zhangfei wrote: > On 04/22/2014 10:30 PM, Arnd Bergmann wrote: > > On Tuesday 22 April 2014, zhangfei wrote: > >> On 04/22/2014 04:22 PM, Arnd Bergmann wrote: > > 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. > > > It is correct behavior return to the original page. > > First get_phy_id, then match driver according to the id, then operation > in the specific driver have the chance to run, including > marvell_of_reg_init etc. > > Just unlucky the default value of PHY_PAGE after power on is not 0. Ok, so it's actually not possible to detect this case prior to having to work around it. How about adding a DT property as a flag to tell the driver whether to set the page to zero or not? That would let the driver work with different types of PHY implementations, or skip the code if there is firmware that can already set it up to page zero. Arnd
On 04/24/2014 08:28 PM, Arnd Bergmann wrote: > On Tuesday 22 April 2014, zhangfei wrote: >> On 04/22/2014 10:30 PM, Arnd Bergmann wrote: >>> On Tuesday 22 April 2014, zhangfei wrote: >>>> On 04/22/2014 04:22 PM, Arnd Bergmann wrote: > >>> 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. >>> >> It is correct behavior return to the original page. >> >> First get_phy_id, then match driver according to the id, then operation >> in the specific driver have the chance to run, including >> marvell_of_reg_init etc. >> >> Just unlucky the default value of PHY_PAGE after power on is not 0. > > Ok, so it's actually not possible to detect this case prior to having > to work around it. > > How about adding a DT property as a flag to tell the driver whether > to set the page to zero or not? That would let the driver work > with different types of PHY implementations, or skip the code if > there is firmware that can already set it up to page zero. > Currently we suspect it maybe caused by bootloader (uefi) has accessed page register and forgot to recover, still in double confirm. Even it not caused by uefi, we can set to page 0 in uefi, and let mdio driver common to other phy. So we may remove the bus->reset, and let uefi do the preparation. Thanks
diff --git a/drivers/net/ethernet/Kconfig b/drivers/net/ethernet/Kconfig index 506b024..1c8dc3d 100644 --- a/drivers/net/ethernet/Kconfig +++ b/drivers/net/ethernet/Kconfig @@ -54,6 +54,7 @@ source "drivers/net/ethernet/neterion/Kconfig" source "drivers/net/ethernet/faraday/Kconfig" source "drivers/net/ethernet/freescale/Kconfig" source "drivers/net/ethernet/fujitsu/Kconfig" +source "drivers/net/ethernet/hisilicon/Kconfig" source "drivers/net/ethernet/hp/Kconfig" source "drivers/net/ethernet/ibm/Kconfig" source "drivers/net/ethernet/intel/Kconfig" diff --git a/drivers/net/ethernet/Makefile b/drivers/net/ethernet/Makefile index c0b8789..da1a435 100644 --- a/drivers/net/ethernet/Makefile +++ b/drivers/net/ethernet/Makefile @@ -29,6 +29,7 @@ obj-$(CONFIG_NET_VENDOR_EXAR) += neterion/ obj-$(CONFIG_NET_VENDOR_FARADAY) += faraday/ obj-$(CONFIG_NET_VENDOR_FREESCALE) += freescale/ obj-$(CONFIG_NET_VENDOR_FUJITSU) += fujitsu/ +obj-$(CONFIG_NET_VENDOR_HISILICON) += hisilicon/ obj-$(CONFIG_NET_VENDOR_HP) += hp/ obj-$(CONFIG_NET_VENDOR_IBM) += ibm/ obj-$(CONFIG_NET_VENDOR_INTEL) += intel/ diff --git a/drivers/net/ethernet/hisilicon/Kconfig b/drivers/net/ethernet/hisilicon/Kconfig new file mode 100644 index 0000000..628537f --- /dev/null +++ b/drivers/net/ethernet/hisilicon/Kconfig @@ -0,0 +1,31 @@ +# +# HISILICON device configuration +# + +config NET_VENDOR_HISILICON + bool "Hisilicon devices" + default y + depends on ARM + ---help--- + If you have a network (Ethernet) card belonging to this class, say Y + and read the Ethernet-HOWTO, available from + <http://www.tldp.org/docs.html#howto>. + + Note that the answer to this question doesn't directly affect the + kernel: saying N will just cause the configurator to skip all + the questions about Hisilicon devices. If you say Y, you will be asked + for your specific card in the following questions. + +if NET_VENDOR_HISILICON + +config HIP04_ETH + tristate "HISILICON P04 Ethernet support" + select PHYLIB + select MARVELL_PHY + select MFD_SYSCON + ---help--- + If you wish to compile a kernel for a hardware with hisilicon p04 SoC and + want to use the internal ethernet then you should answer Y to this. + + +endif # NET_VENDOR_HISILICON diff --git a/drivers/net/ethernet/hisilicon/Makefile b/drivers/net/ethernet/hisilicon/Makefile new file mode 100644 index 0000000..1d6eb6e --- /dev/null +++ b/drivers/net/ethernet/hisilicon/Makefile @@ -0,0 +1,5 @@ +# +# Makefile for the HISILICON network device drivers. +# + +obj-$(CONFIG_HIP04_ETH) += hip04_mdio.o diff --git a/drivers/net/ethernet/hisilicon/hip04_mdio.c b/drivers/net/ethernet/hisilicon/hip04_mdio.c new file mode 100644 index 0000000..19826a3 --- /dev/null +++ b/drivers/net/ethernet/hisilicon/hip04_mdio.c @@ -0,0 +1,185 @@ + +/* Copyright (c) 2014 Linaro Ltd. + * Copyright (c) 2014 Hisilicon Limited. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + */ + +#include <linux/module.h> +#include <linux/platform_device.h> +#include <linux/io.h> +#include <linux/of_mdio.h> +#include <linux/delay.h> + +#define MDIO_CMD_REG 0x0 +#define MDIO_ADDR_REG 0x4 +#define MDIO_WDATA_REG 0x8 +#define MDIO_RDATA_REG 0xc +#define MDIO_STA_REG 0x10 + +#define MDIO_START BIT(14) +#define MDIO_R_VALID BIT(1) +#define MDIO_READ (BIT(12) | BIT(11) | MDIO_START) +#define MDIO_WRITE (BIT(12) | BIT(10) | MDIO_START) + +struct hip04_mdio_priv { + void __iomem *base; +}; + +#define WAIT_TIMEOUT 10 +static int hip04_mdio_wait_ready(struct mii_bus *bus) +{ + struct hip04_mdio_priv *priv = bus->priv; + int i; + + for (i = 0; readl_relaxed(priv->base + MDIO_CMD_REG) & MDIO_START; i++) { + if (i == WAIT_TIMEOUT) + return -ETIMEDOUT; + msleep(20); + } + + return 0; +} + +static int hip04_mdio_read(struct mii_bus *bus, int mii_id, int regnum) +{ + struct hip04_mdio_priv *priv = bus->priv; + u32 val; + int ret; + + ret = hip04_mdio_wait_ready(bus); + if (ret < 0) + goto out; + + val = regnum | (mii_id << 5) | MDIO_READ; + writel_relaxed(val, priv->base + MDIO_CMD_REG); + + ret = hip04_mdio_wait_ready(bus); + if (ret < 0) + goto out; + + val = readl_relaxed(priv->base + MDIO_STA_REG); + if (val & MDIO_R_VALID) { + dev_err(bus->parent, "SMI bus read not valid\n"); + ret = -ENODEV; + goto out; + } + + val = readl_relaxed(priv->base + MDIO_RDATA_REG); + ret = val & 0xFFFF; +out: + return ret; +} + +static int hip04_mdio_write(struct mii_bus *bus, int mii_id, + int regnum, u16 value) +{ + struct hip04_mdio_priv *priv = bus->priv; + u32 val; + int ret; + + ret = hip04_mdio_wait_ready(bus); + if (ret < 0) + goto out; + + writel_relaxed(value, priv->base + MDIO_WDATA_REG); + val = regnum | (mii_id << 5) | MDIO_WRITE; + writel_relaxed(val, priv->base + MDIO_CMD_REG); +out: + return ret; +} + +static int hip04_mdio_reset(struct mii_bus *bus) +{ + int temp, err, i; + + for (i = 0; i < PHY_MAX_ADDR; i++) { + hip04_mdio_write(bus, i, 22, 0); + temp = hip04_mdio_read(bus, i, MII_BMCR); + temp |= BMCR_RESET; + err = hip04_mdio_write(bus, i, MII_BMCR, temp); + if (err < 0) + return err; + } + + mdelay(500); + return 0; +} + +static int hip04_mdio_probe(struct platform_device *pdev) +{ + struct resource *r; + struct mii_bus *bus; + struct hip04_mdio_priv *priv; + int ret; + + bus = mdiobus_alloc_size(sizeof(struct hip04_mdio_priv)); + if (!bus) { + dev_err(&pdev->dev, "Cannot allocate MDIO bus\n"); + return -ENOMEM; + } + + bus->name = "hip04_mdio_bus"; + bus->read = hip04_mdio_read; + bus->write = hip04_mdio_write; + bus->reset = hip04_mdio_reset; + snprintf(bus->id, MII_BUS_ID_SIZE, "%s-mii", dev_name(&pdev->dev)); + bus->parent = &pdev->dev; + priv = bus->priv; + + r = platform_get_resource(pdev, IORESOURCE_MEM, 0); + priv->base = devm_ioremap_resource(&pdev->dev, r); + if (IS_ERR(priv->base)) { + ret = PTR_ERR(priv->base); + goto out_mdio; + } + + ret = of_mdiobus_register(bus, pdev->dev.of_node); + if (ret < 0) { + dev_err(&pdev->dev, "Cannot register MDIO bus (%d)\n", ret); + goto out_mdio; + } + + platform_set_drvdata(pdev, bus); + + return 0; + +out_mdio: + mdiobus_free(bus); + return ret; +} + +static int hip04_mdio_remove(struct platform_device *pdev) +{ + struct mii_bus *bus = platform_get_drvdata(pdev); + + mdiobus_unregister(bus); + mdiobus_free(bus); + + return 0; +} + +static const struct of_device_id hip04_mdio_match[] = { + { .compatible = "hisilicon,hip04-mdio" }, + { } +}; +MODULE_DEVICE_TABLE(of, hip04_mdio_match); + +static struct platform_driver hip04_mdio_driver = { + .probe = hip04_mdio_probe, + .remove = hip04_mdio_remove, + .driver = { + .name = "hip04-mdio", + .owner = THIS_MODULE, + .of_match_table = hip04_mdio_match, + }, +}; + +module_platform_driver(hip04_mdio_driver); + +MODULE_DESCRIPTION("HISILICON P04 MDIO interface driver"); +MODULE_LICENSE("GPL v2"); +MODULE_ALIAS("platform:hip04-mdio");
Hisilicon hip04 platform mdio driver Reuse Marvell phy drivers/net/phy/marvell.c Signed-off-by: Zhangfei Gao <zhangfei.gao@linaro.org> --- drivers/net/ethernet/Kconfig | 1 + drivers/net/ethernet/Makefile | 1 + drivers/net/ethernet/hisilicon/Kconfig | 31 +++++ drivers/net/ethernet/hisilicon/Makefile | 5 + drivers/net/ethernet/hisilicon/hip04_mdio.c | 185 +++++++++++++++++++++++++++ 5 files changed, 223 insertions(+) create mode 100644 drivers/net/ethernet/hisilicon/Kconfig create mode 100644 drivers/net/ethernet/hisilicon/Makefile create mode 100644 drivers/net/ethernet/hisilicon/hip04_mdio.c