[U-Boot] [PATCH 3/4] net: phy: add support for Micrel's KSZ9021

[Andy Fleming]

NAK, for reasons listed below.

On Thu, Jan 26, 2012 at 4:21 PM, Troy Kisky
<troy.kisky at boundarydevices.com> wrote:
> Add the gigabit phy KSZ9021.
>
> Signed-off-by: Troy Kisky <troy.kisky at 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