| Message ID | 1327616505-11669-3-git-send-email-troy.kisky@boundarydevices.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Stefano Babic |
| Headers | show |
On Thursday 26 January 2012 17:21:44 Troy Kisky wrote: > +/* This is used to set board specific things like clock skew */ > +unsigned short ksz9021_por_cmds[] = { static const > +int ksz9021_send_phy_cmds(struct phy_device *phydev, unsigned short* p) static > + for (;;) { personally, i'd prefer: while (1) { -mike
On 1/26/2012 7:54 PM, Mike Frysinger wrote: > On Thursday 26 January 2012 17:21:44 Troy Kisky wrote: >> +/* This is used to set board specific things like clock skew */ >> +unsigned short ksz9021_por_cmds[] = { > static const Thanks > >> +int ksz9021_send_phy_cmds(struct phy_device *phydev, unsigned short* p) > static Thanks > >> + for (;;) { > personally, i'd prefer: > while (1) { > -mike I used to prefer while (1), but then Microsoft started warning about it. I doubt GCC ever makes the same decision, but it is easier to use the same style. And I can't see a downside. see http://msdn.microsoft.com/en-us/library/6t66728h%28VS.80%29.aspx
NAK, for reasons listed below. On Thu, Jan 26, 2012 at 4:21 PM, Troy Kisky <troy.kisky@boundarydevices.com> wrote: > Add the gigabit phy KSZ9021. > > Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com> These commit messages are a bit over-brief. Please explain a bit more if there's anything non-trivial in the patch. > --- > drivers/net/phy/Makefile | 1 + > drivers/net/phy/micrel_ksz9021.c | 124 ++++++++++++++++++++++++++++++++++++++ > drivers/net/phy/phy.c | 3 + > 3 files changed, 128 insertions(+), 0 deletions(-) > create mode 100644 drivers/net/phy/micrel_ksz9021.c > [...] > diff --git a/drivers/net/phy/micrel_ksz9021.c b/drivers/net/phy/micrel_ksz9021.c > + > +/* This is used to set board specific things like clock skew */ > +unsigned short ksz9021_por_cmds[] = { > +#ifdef CONFIG_PHY_MICREL_KSZ9021_INIT_CMDS > + CONFIG_PHY_MICREL_KSZ9021_INIT_CMDS > +#endif > + 0, 0 > +}; > + > +int ksz9021_send_phy_cmds(struct phy_device *phydev, unsigned short* p) > +{ > + for (;;) { > + unsigned reg = *p++; > + unsigned val = *p++; > + if (reg == 0 && val == 0) > + break; > + if (reg < 32) { > + phy_write(phydev, MDIO_DEVAD_NONE, reg, val); > + } else { > + phy_write(phydev, MDIO_DEVAD_NONE, > + MII_KSZ9021_EXTENDED_CTRL, reg); > + phy_write(phydev, MDIO_DEVAD_NONE, > + MII_KSZ9021_EXTENDED_DATAW, val); > + } > + } > + return 0; > +} For instance, some wacky data-driven initializer function which totally works around any attempts the community might make at dealing with initialization in a transparent and functional way, and is essentially a whole (undocumented) API in and of itself. Basically, this is the sort of capability that all PHYs need, and if the current framework isn't sufficient to this PHY's needs, then we should look at modifying PHY Lib to provide better board-level initialization capabilities. An earlier version of something like PHY Lib had a similar type of mechanism, but a list of register writes is often insufficient to the task of performing necessary initialization. Sometimes, one needs to wait until a write takes effect, or do a different write based on link state information, or make other sorts of decisions. > diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c > index eb55180..7e1e4b6 100644 > --- a/drivers/net/phy/phy.c > +++ b/drivers/net/phy/phy.c > @@ -438,6 +438,9 @@ int phy_init(void) > #ifdef CONFIG_PHY_MICREL > phy_micrel_init(); > #endif > +#ifdef CONFIG_PHY_MICREL_KSZ9021 > + phy_ksz9021_init(); > +#endif I believe we're sorting this list alphabetically.... And now I see that this is actually a Micrel PHY. Why are you making a separate file for it? Put this code in the micrel.c file. > #ifdef CONFIG_PHY_NATSEMI > phy_natsemi_init(); > #endif
On 1/29/2012 7:26 PM, Andy Fleming wrote: > NAK, for reasons listed below. > > On Thu, Jan 26, 2012 at 4:21 PM, Troy Kisky > <troy.kisky@boundarydevices.com> wrote: >> Add the gigabit phy KSZ9021. >> >> Signed-off-by: Troy Kisky<troy.kisky@boundarydevices.com> > These commit messages are a bit over-brief. Please explain a bit more > if there's anything non-trivial in the patch. > > >> --- >> drivers/net/phy/Makefile | 1 + >> drivers/net/phy/micrel_ksz9021.c | 124 ++++++++++++++++++++++++++++++++++++++ >> drivers/net/phy/phy.c | 3 + >> 3 files changed, 128 insertions(+), 0 deletions(-) >> create mode 100644 drivers/net/phy/micrel_ksz9021.c >> > [...] > >> diff --git a/drivers/net/phy/micrel_ksz9021.c b/drivers/net/phy/micrel_ksz9021.c >> + >> +/* This is used to set board specific things like clock skew */ >> +unsigned short ksz9021_por_cmds[] = { >> +#ifdef CONFIG_PHY_MICREL_KSZ9021_INIT_CMDS >> + CONFIG_PHY_MICREL_KSZ9021_INIT_CMDS >> +#endif >> + 0, 0 >> +}; >> + >> +int ksz9021_send_phy_cmds(struct phy_device *phydev, unsigned short* p) >> +{ >> + for (;;) { >> + unsigned reg = *p++; >> + unsigned val = *p++; >> + if (reg == 0&& val == 0) >> + break; >> + if (reg< 32) { >> + phy_write(phydev, MDIO_DEVAD_NONE, reg, val); >> + } else { >> + phy_write(phydev, MDIO_DEVAD_NONE, >> + MII_KSZ9021_EXTENDED_CTRL, reg); >> + phy_write(phydev, MDIO_DEVAD_NONE, >> + MII_KSZ9021_EXTENDED_DATAW, val); >> + } >> + } >> + return 0; >> +} > > For instance, some wacky data-driven initializer function which > totally works around any attempts the community might make at dealing > with initialization in a transparent and functional way, and is > essentially a whole (undocumented) API in and of itself. This should be a separate patch including documentation. But that doesn't sound sufficient for you. > > Basically, this is the sort of capability that all PHYs need, and if > the current framework isn't sufficient to this PHY's needs, then we > should look at modifying PHY Lib to provide better board-level > initialization capabilities. > > An earlier version of something like PHY Lib had a similar type of > mechanism, but a list of register writes is often insufficient to the > task of performing necessary initialization. Sometimes, one needs to > wait until a write takes effect, or do a different write based on link > state information, or make other sorts of decisions. I thought that was a clean method of keeping board specific things in the board.h file, without needing to duplicate code in the board.c file. Please suggest an alternative method. Right now, I'm just dead in the water and cannot see the way forward. Do you want a more complete API? Or do you want want this as a function in the board file? I could use board_phy_config, but that would mean duplicated code in other board files. The only sticking point is that board_phy_config is called after drv->config, not before or instead of. If it was changed to instead of, I could add the call to phydev->drv->config(phydev) to all existing board_phy_config functions. Currently that is board/freescale/corenet_ds/eth_p4080.c board/freescale/mpc8544ds/mpc8544ds.c > > >> diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c >> index eb55180..7e1e4b6 100644 >> --- a/drivers/net/phy/phy.c >> +++ b/drivers/net/phy/phy.c >> @@ -438,6 +438,9 @@ int phy_init(void) >> #ifdef CONFIG_PHY_MICREL >> phy_micrel_init(); >> #endif >> +#ifdef CONFIG_PHY_MICREL_KSZ9021 >> + phy_ksz9021_init(); >> +#endif > I believe we're sorting this list alphabetically.... > > And now I see that this is actually a Micrel PHY. Why are you making a > separate file for it? Put this code in the micrel.c file. So, you want ifdefs in the micrel.c file? That seems unnecessarily ugly. The micrel file currently supports only the ksz804 and uses only genphy_xxx functions. Perhaps renaming this file to micrel_ksz804.c would be more appropriate. Alternatively, change the name to genphy.c and change the structure to static struct phy_driver genphy_driver = { .name = CONFIG_GENPHY_NAME, .uid = CONFIG_GENPHY_UID, .mask = CONFIG_GENPHY_MASK, .features = PHY_BASIC_FEATURES, .config = &genphy_config, .startup = &genphy_startup, .shutdown = &genphy_shutdown, }; So that all phys which are generic can be easily defined by the board which uses it. I think phy_init should be changed to traverse a section created by the linker so that all these ifdefs could disappear. Something like #define __phy_init_section __attribute__ ((__section__ (".data.phy_init_section"))) typedef int(*phy_init_rtn)(void); static phy_init_rtn ksz9021_phy_init_rtn __phy_init_section = phy_ksz9021_init; extern phy_init_rtn __phy_init_section_start, __phy_init_section_end; int phy_init(void) { phy_init_rtn* rtn = &__phy_init_section_start; while (rtn < &__phy_init_section_end) { rtn(); rtn++; } } But that is beyond the scope of this patch. Would you like a patch to convert to this before this patch? > >> #ifdef CONFIG_PHY_NATSEMI >> phy_natsemi_init(); >> #endif
On Mon, Jan 30, 2012 at 3:30 PM, Troy Kisky <troy.kisky@boundarydevices.com> wrote: > On 1/29/2012 7:26 PM, Andy Fleming wrote: >> >> NAK, for reasons listed below. >> >> An earlier version of something like PHY Lib had a similar type of >> mechanism, but a list of register writes is often insufficient to the >> task of performing necessary initialization. Sometimes, one needs to >> wait until a write takes effect, or do a different write based on link >> state information, or make other sorts of decisions. > > I thought that was a clean method of keeping board specific things in the > board.h file, > without needing to duplicate code in the board.c file. It seems that way, but, as I said, it can lead to more problems in the future. It also makes the reasons for the initialization sequences much less comprehensible. > > Please suggest an alternative method. Right now, I'm just dead in the water > and > cannot see the way forward. Do you want a more complete API? Or do you want > want this as a function in the board file? > > I could use board_phy_config, but that would mean duplicated code in other > board files. > The only sticking point is that board_phy_config is called after > drv->config, not before or > instead of. If it was changed to instead of, I could add the call to > phydev->drv->config(phydev) > to all existing board_phy_config functions. Currently that is > > board/freescale/corenet_ds/eth_p4080.c > board/freescale/mpc8544ds/mpc8544ds.c Calling board_phy_config() instead of drv->config() might be the most straightforward solution. You don't have to have duplicate code, though. If a given set of boards will have the same initialization sequence, then share the code between them. Also, as these sequences are all probably either enabling/disabling well-understood features, or configuration, you could put those functions in the PHY driver code, and your board code ends up looking like: /* Muxing on bus adds extra delay on RX path */ ksz9021_set_rx_delay(phydev, 12ns); /* New isolinear circuits go to 11 */ ksz9021_adjust_resonance_frequency(11); /* Accidental feedback loop caused PHY to achieve homicidal rage */ ksz9021_disable_emotion_chip(phydev); And there are other factors, which could be handled *generically* by the config() function for your PHY: if (phydev->interface == PHY_INTERFACE_MODE_SGMII) { int cfg = phy_read(phydev, KSZ9021_CONFIG_SPECIAL); cfg |= KSZ021_SGMII_MODE; phy_write(phydev, KSZ9021_CONFIG_SPECIAL, cfg); } else ... That's the best solution, as it helps to contain knowledge about the PHY inside the driver. However, there are definitely times when the PHY requires some very specific configuration steps, which are best contained in board code. Summary: Yes, I think it would be a good idea to convert all the current calls to phydev->drv->config to board_phy_config(), which should default to just calling drv->config(). > > >> >> >>> diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c >>> index eb55180..7e1e4b6 100644 >>> --- a/drivers/net/phy/phy.c >>> +++ b/drivers/net/phy/phy.c >>> @@ -438,6 +438,9 @@ int phy_init(void) >>> #ifdef CONFIG_PHY_MICREL >>> phy_micrel_init(); >>> #endif >>> +#ifdef CONFIG_PHY_MICREL_KSZ9021 >>> + phy_ksz9021_init(); >>> +#endif >> >> I believe we're sorting this list alphabetically.... >> >> And now I see that this is actually a Micrel PHY. Why are you making a >> separate file for it? Put this code in the micrel.c file. > > > So, you want ifdefs in the micrel.c file? That seems unnecessarily ugly. > The micrel file currently supports only the ksz804 and uses only > genphy_xxx functions. Perhaps renaming this file to micrel_ksz804.c > would be more appropriate. Alternatively, change the name to genphy.c > and change the structure to > static struct phy_driver genphy_driver = { > .name = CONFIG_GENPHY_NAME, > .uid = CONFIG_GENPHY_UID, > .mask = CONFIG_GENPHY_MASK, > .features = PHY_BASIC_FEATURES, > .config = &genphy_config, > .startup = &genphy_startup, > .shutdown = &genphy_shutdown, > }; > > So that all phys which are generic can be easily defined by the board which > uses it. I meant you should follow the example of marvell.c or vitesse.c, and put all Micrel PHY code in the micrel.c file. Frequently, the different PHYs from the same companies share common features. We may divide them up differently, later, but, if anything, this driver should be the "micrel" driver, and the existing one is just a stub. The idea of allowing the Micrel stub PHY to exist was so that someone might come along one day, and decide to add to its functionality. It seems likely that your code may get repurposed by some other developer to better support the 804 on her board. > > > I think phy_init should be changed to traverse a section created by the > linker so that all these ifdefs could disappear. Something like > > #define __phy_init_section __attribute__ ((__section__ > (".data.phy_init_section"))) > typedef int(*phy_init_rtn)(void); > > static phy_init_rtn ksz9021_phy_init_rtn __phy_init_section = > phy_ksz9021_init; > > extern phy_init_rtn __phy_init_section_start, __phy_init_section_end; > > int phy_init(void) > { > phy_init_rtn* rtn = &__phy_init_section_start; > while (rtn < &__phy_init_section_end) { > rtn(); > rtn++; > } > } > > But that is beyond the scope of this patch. Would you like a patch to > convert to this > before this patch? That's fine by me, though I think it's a bit overkill. As I said, there are other examples of collecting multiple PHY drivers into one file. It wastes a little space and time, but only a little, to my mind. If we want to be more precise, we can always add more specific #ifdefs. I just want to avoid a situation where we have hundreds or thousands of PHY drivers, and right now we've standardized on one init function and one file per PHY vendor. Andy
On 1/30/2012 5:05 PM, Andy Fleming wrote: > On Mon, Jan 30, 2012 at 3:30 PM, Troy Kisky > <troy.kisky@boundarydevices.com> wrote: >> On 1/29/2012 7:26 PM, Andy Fleming wrote: >>> NAK, for reasons listed below. >>> >>> An earlier version of something like PHY Lib had a similar type of >>> mechanism, but a list of register writes is often insufficient to the >>> task of performing necessary initialization. Sometimes, one needs to >>> wait until a write takes effect, or do a different write based on link >>> state information, or make other sorts of decisions. >> I thought that was a clean method of keeping board specific things in the >> board.h file, >> without needing to duplicate code in the board.c file. > > It seems that way, but, as I said, it can lead to more problems in the > future. It also makes the reasons for the initialization sequences > much less comprehensible. I'll take your word, as I have more difficulty imagining your future. > > >> Please suggest an alternative method. Right now, I'm just dead in the water >> and >> cannot see the way forward. Do you want a more complete API? Or do you want >> want this as a function in the board file? >> >> I could use board_phy_config, but that would mean duplicated code in other >> board files. >> The only sticking point is that board_phy_config is called after >> drv->config, not before or >> instead of. If it was changed to instead of, I could add the call to >> phydev->drv->config(phydev) >> to all existing board_phy_config functions. Currently that is >> >> board/freescale/corenet_ds/eth_p4080.c >> board/freescale/mpc8544ds/mpc8544ds.c > > Calling board_phy_config() instead of drv->config() might be the most > straightforward solution. You don't have to have duplicate code, > though. If a given set of boards will have the same initialization > sequence, then share the code between them. Also, as these sequences > are all probably either enabling/disabling well-understood features, > or configuration, you could put those functions in the PHY driver > code, and your board code ends up looking like: > > /* Muxing on bus adds extra delay on RX path */ > ksz9021_set_rx_delay(phydev, 12ns); > > /* New isolinear circuits go to 11 */ > ksz9021_adjust_resonance_frequency(11); > > /* Accidental feedback loop caused PHY to achieve homicidal rage */ > ksz9021_disable_emotion_chip(phydev); > > > And there are other factors, which could be handled *generically* by > the config() function for your PHY: > > if (phydev->interface == PHY_INTERFACE_MODE_SGMII) { > int cfg = phy_read(phydev, KSZ9021_CONFIG_SPECIAL); > cfg |= KSZ021_SGMII_MODE; > phy_write(phydev, KSZ9021_CONFIG_SPECIAL, cfg); > } else ... > > > That's the best solution, as it helps to contain knowledge about the > PHY inside the driver. However, there are definitely times when the > PHY requires some very specific configuration steps, which are best > contained in board code. > > Summary: Yes, I think it would be a good idea to convert all the > current calls to phydev->drv->config to board_phy_config(), which > should default to just calling drv->config(). Can do. Thanks for providing a direction. > >> >>> >>>> diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c >>>> index eb55180..7e1e4b6 100644 >>>> --- a/drivers/net/phy/phy.c >>>> +++ b/drivers/net/phy/phy.c >>>> @@ -438,6 +438,9 @@ int phy_init(void) >>>> #ifdef CONFIG_PHY_MICREL >>>> phy_micrel_init(); >>>> #endif >>>> +#ifdef CONFIG_PHY_MICREL_KSZ9021 >>>> + phy_ksz9021_init(); >>>> +#endif >>> I believe we're sorting this list alphabetically.... >>> >>> And now I see that this is actually a Micrel PHY. Why are you making a >>> separate file for it? Put this code in the micrel.c file. >> >> So, you want ifdefs in the micrel.c file? That seems unnecessarily ugly. >> The micrel file currently supports only the ksz804 and uses only >> genphy_xxx functions. Perhaps renaming this file to micrel_ksz804.c >> would be more appropriate. Alternatively, change the name to genphy.c >> and change the structure to >> static struct phy_driver genphy_driver = { >> .name = CONFIG_GENPHY_NAME, >> .uid = CONFIG_GENPHY_UID, >> .mask = CONFIG_GENPHY_MASK, >> .features = PHY_BASIC_FEATURES, >> .config =&genphy_config, >> .startup =&genphy_startup, >> .shutdown =&genphy_shutdown, >> }; >> >> So that all phys which are generic can be easily defined by the board which >> uses it. > > I meant you should follow the example of marvell.c or vitesse.c, and > put all Micrel PHY code in the micrel.c file. Frequently, the > different PHYs from the same companies share common features. We may > divide them up differently, later, but, if anything, this driver > should be the "micrel" driver, and the existing one is just a stub. > > The idea of allowing the Micrel stub PHY to exist was so that someone > might come along one day, and decide to add to its functionality. It > seems likely that your code may get repurposed by some other developer > to better support the 804 on her board. I'm doubtful, since that code has not changed since your initial commit back in April. Is anyone still using it today? Seems only powerPC platforms with CONFIG_TSEC_ENET have had a chance. But I guess that is an irrelevant tangent. > > >> >> I think phy_init should be changed to traverse a section created by the >> linker so that all these ifdefs could disappear. Something like >> >> #define __phy_init_section __attribute__ ((__section__ >> (".data.phy_init_section"))) >> typedef int(*phy_init_rtn)(void); >> >> static phy_init_rtn ksz9021_phy_init_rtn __phy_init_section = >> phy_ksz9021_init; >> >> extern phy_init_rtn __phy_init_section_start, __phy_init_section_end; >> >> int phy_init(void) >> { >> phy_init_rtn* rtn =&__phy_init_section_start; >> while (rtn< &__phy_init_section_end) { >> rtn(); >> rtn++; >> } >> } >> >> But that is beyond the scope of this patch. Would you like a patch to >> convert to this >> before this patch? > > That's fine by me, though I think it's a bit overkill. As I said, > there are other examples of collecting multiple PHY drivers into one > file. It wastes a little space and time, but only a little, to my > mind. If we want to be more precise, we can always add more specific > #ifdefs. I just want to avoid a situation where we have hundreds or > thousands of PHY drivers, and right now we've standardized on one init > function and one file per PHY vendor. Yes, if some code is shared. I does make sense to me to have both in the same file or have a separate file with the shared code rather than cut-n-pasting the code. However, a hard rule of only one file per vendor doesn't make sense to me. If Micrel buys out vitesse will you combine the files? I doubt it. On the other hand, as this is way way way down on the list of things I care about, and seems much higher on your list, putting it in micrel.c is fine with me. > > Andy >
diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile index feced39..86ec3c5 100644 --- a/drivers/net/phy/Makefile +++ b/drivers/net/phy/Makefile @@ -36,6 +36,7 @@ COBJS-$(CONFIG_PHY_DAVICOM) += davicom.o COBJS-$(CONFIG_PHY_LXT) += lxt.o COBJS-$(CONFIG_PHY_MARVELL) += marvell.o COBJS-$(CONFIG_PHY_MICREL) += micrel.o +COBJS-$(CONFIG_PHY_MICREL_KSZ9021) += micrel_ksz9021.o COBJS-$(CONFIG_PHY_NATSEMI) += natsemi.o COBJS-$(CONFIG_PHY_REALTEK) += realtek.o COBJS-$(CONFIG_PHY_SMSC) += smsc.o diff --git a/drivers/net/phy/micrel_ksz9021.c b/drivers/net/phy/micrel_ksz9021.c new file mode 100644 index 0000000..f17b798 --- /dev/null +++ b/drivers/net/phy/micrel_ksz9021.c @@ -0,0 +1,124 @@ +/* + * Micrel KSZ9021 PHY driver + * + * 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. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, + * MA 02111-1307 USA + * + * Copyright 2012 Boundary Devices + * + */ +#include <config.h> +#include <common.h> +#include <phy.h> + +/* ksz9021 PHY Registers */ +#define MII_KSZ9021_EXTENDED_CTRL 0x0b +#define MII_KSZ9021_EXTENDED_DATAW 0x0c +#define MII_KSZ9021_PHY_CTL 0x1f +#define MIIM_KSZ9021_PHYCTL_1000 (1 << 6) +#define MIIM_KSZ9021_PHYCTL_100 (1 << 5) +#define MIIM_KSZ9021_PHYCTL_10 (1 << 4) +#define MIIM_KSZ9021_PHYCTL_DUPLEX (1 << 3) + +#define CTRL1000_PREFER_MASTER (1 << 10) +#define CTRL1000_CONFIG_MASTER (1 << 11) +#define CTRL1000_MANUAL_CONFIG (1 << 12) + +/* This is used to set board specific things like clock skew */ +unsigned short ksz9021_por_cmds[] = { +#ifdef CONFIG_PHY_MICREL_KSZ9021_INIT_CMDS + CONFIG_PHY_MICREL_KSZ9021_INIT_CMDS +#endif + 0, 0 +}; + +int ksz9021_send_phy_cmds(struct phy_device *phydev, unsigned short* p) +{ + for (;;) { + unsigned reg = *p++; + unsigned val = *p++; + if (reg == 0 && val == 0) + break; + if (reg < 32) { + phy_write(phydev, MDIO_DEVAD_NONE, reg, val); + } else { + phy_write(phydev, MDIO_DEVAD_NONE, + MII_KSZ9021_EXTENDED_CTRL, reg); + phy_write(phydev, MDIO_DEVAD_NONE, + MII_KSZ9021_EXTENDED_DATAW, val); + } + } + return 0; +} + +/* Micrel ksz9021 */ +static int ksz9021_config(struct phy_device *phydev) +{ + unsigned ctrl1000 = 0; + const unsigned master = CTRL1000_PREFER_MASTER | + CTRL1000_CONFIG_MASTER | CTRL1000_MANUAL_CONFIG; + unsigned features = phydev->drv->features; + + ksz9021_send_phy_cmds(phydev, ksz9021_por_cmds); + if (getenv("disable_giga")) + features &= ~(SUPPORTED_1000baseT_Half | + SUPPORTED_1000baseT_Full); + /* force master mode for 1000BaseT due to chip errata */ + if (features & SUPPORTED_1000baseT_Half) + ctrl1000 |= ADVERTISE_1000HALF | master; + if (features & SUPPORTED_1000baseT_Full) + ctrl1000 |= ADVERTISE_1000FULL | master; + phydev->advertising = phydev->supported = features; + phy_write(phydev, MDIO_DEVAD_NONE, MII_CTRL1000, ctrl1000); + genphy_config_aneg(phydev); + genphy_restart_aneg(phydev); + return 0; +} + +static int ksz9021_startup(struct phy_device *phydev) +{ + unsigned phy_ctl; + genphy_update_link(phydev); + phy_ctl = phy_read(phydev, MDIO_DEVAD_NONE, MII_KSZ9021_PHY_CTL); + + if (phy_ctl & MIIM_KSZ9021_PHYCTL_DUPLEX) + phydev->duplex = DUPLEX_FULL; + else + phydev->duplex = DUPLEX_HALF; + + if (phy_ctl & MIIM_KSZ9021_PHYCTL_1000) + phydev->speed = SPEED_1000; + else if (phy_ctl & MIIM_KSZ9021_PHYCTL_100) + phydev->speed = SPEED_100; + else if (phy_ctl & MIIM_KSZ9021_PHYCTL_10) + phydev->speed = SPEED_10; + return 0; +} + +static struct phy_driver ksz9021_driver = { + .name = "Micrel ksz9021", + .uid = 0x221610, + .mask = 0xfffff0, + .features = PHY_GBIT_FEATURES, + .config = &ksz9021_config, + .startup = &ksz9021_startup, + .shutdown = &genphy_shutdown, +}; + +int phy_ksz9021_init(void) +{ + phy_register(&ksz9021_driver); + return 0; +} diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c index eb55180..7e1e4b6 100644 --- a/drivers/net/phy/phy.c +++ b/drivers/net/phy/phy.c @@ -438,6 +438,9 @@ int phy_init(void) #ifdef CONFIG_PHY_MICREL phy_micrel_init(); #endif +#ifdef CONFIG_PHY_MICREL_KSZ9021 + phy_ksz9021_init(); +#endif #ifdef CONFIG_PHY_NATSEMI phy_natsemi_init(); #endif
Add the gigabit phy KSZ9021. Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com> --- drivers/net/phy/Makefile | 1 + drivers/net/phy/micrel_ksz9021.c | 124 ++++++++++++++++++++++++++++++++++++++ drivers/net/phy/phy.c | 3 + 3 files changed, 128 insertions(+), 0 deletions(-) create mode 100644 drivers/net/phy/micrel_ksz9021.c