All of lore.kernel.org
 help / color / mirror / Atom feed
From: David Daney <david.daney@cavium.com>
To: Andy Fleming <afleming@freescale.com>
Cc: davem@davemloft.net, netdev@vger.kernel.org
Subject: Re: [PATCH v3 1/3] phylib: Convert MDIO and PHY Lib drivers to support 10G
Date: Thu, 13 Oct 2011 09:00:03 -0700	[thread overview]
Message-ID: <4E970B03.9060200@cavium.com> (raw)
In-Reply-To: <1318516660-25452-2-git-send-email-afleming@freescale.com>

On 10/13/2011 07:37 AM, Andy Fleming wrote:
> 10G MDIO is a totally different protocol (clause 45 of 802.3).
> Supporting this new protocol requires a couple of changes:
>
> * Add a new parameter to the mdiobus_read functions to specify the
>    "device address" inside the PHY.
> * Add a phy45_read/write function which takes advantage of that
>    new parameter
> * Convert all of the existing drivers to use the new format
>
> I created a new clause-45-specific read/write functions because:
> 1) phy_read and phy_write are highly overloaded functions, and
>     finding every instance which is actually the PHY Lib version
>     was quite difficult
> 2) Most code which invokes phy_read/phy_write inside PHY Lib is
>     Clause-22-specific. None of the phy_read/phy_write invocations
>     were useable on 10G PHYs
>

I think converting all these phy_read/phy_write to take an extra
parameter is a mistake.  99% of the users have no need for the "device
address".  Also you are still passing the protocol mode as a high
order bit in the register address, so that part is still quite ugly.

The existing infrastructure where we pass the "device address" in bits
16..20 of the register number is much less disruptive.

If you don't like it, an easy and much less intrusive approach might
be a simple (untested) wrapper:

static inline int phy45_read(struct phy_device *phydev,
                              int devad, u16 regnum)
{
	u32 c45_reg = MII_ADDR_C45 | ((devad & 0x1f) << 16) | regnum;
	return phy_read(phydev, c45_reg)
}

static inline int phy45_write(struct phy_device *phydev,
                               int devad, u16 regnum, u16 val)
{
	u32 c45_reg = MII_ADDR_C45 | ((devad & 0x1f) << 16) | regnum;
	return phy_write(phydev, c45_reg, val)
}


> Signed-off-by: Andy Fleming<afleming@freescale.com>
> ---
> v2: Convert newer buses, split out generic PHY support
> v3: Make patch series more coherent
>
>   Documentation/networking/phy.txt                  |   15 +++--
>   arch/powerpc/platforms/pasemi/gpio_mdio.c         |    6 +-
>   drivers/net/ethernet/adi/bfin_mac.c               |    7 +-
>   drivers/net/ethernet/aeroflex/greth.c             |    5 +-
>   drivers/net/ethernet/amd/au1000_eth.c             |    7 +-
>   drivers/net/ethernet/broadcom/bcm63xx_enet.c      |    4 +-
>   drivers/net/ethernet/broadcom/sb1250-mac.c        |    7 +-
>   drivers/net/ethernet/broadcom/tg3.c               |    5 +-
>   drivers/net/ethernet/cadence/macb.c               |    7 +-
>   drivers/net/ethernet/dnet.c                       |    7 +-
>   drivers/net/ethernet/ethoc.c                      |    5 +-
>   drivers/net/ethernet/faraday/ftgmac100.c          |    5 +-
>   drivers/net/ethernet/freescale/fec.c              |    7 +-
>   drivers/net/ethernet/freescale/fec_mpc52xx_phy.c  |    7 +-
>   drivers/net/ethernet/freescale/fs_enet/mii-fec.c  |    6 +-
>   drivers/net/ethernet/freescale/fsl_pq_mdio.c      |   13 ++--
>   drivers/net/ethernet/freescale/fsl_pq_mdio.h      |   11 ++-
>   drivers/net/ethernet/lantiq_etop.c                |    5 +-
>   drivers/net/ethernet/marvell/mv643xx_eth.c        |    5 +-
>   drivers/net/ethernet/marvell/pxa168_eth.c         |    7 +-
>   drivers/net/ethernet/rdc/r6040.c                  |    5 +-
>   drivers/net/ethernet/s6gmac.c                     |    5 +-
>   drivers/net/ethernet/smsc/smsc911x.c              |   22 ++++---
>   drivers/net/ethernet/smsc/smsc9420.c              |   10 ++-
>   drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c |    9 ++-
>   drivers/net/ethernet/ti/cpmac.c                   |    4 +-
>   drivers/net/ethernet/ti/davinci_mdio.c            |    5 +-
>   drivers/net/ethernet/toshiba/tc35815.c            |    5 +-
>   drivers/net/ethernet/xilinx/ll_temac_mdio.c       |    5 +-
>   drivers/net/ethernet/xilinx/xilinx_emaclite.c     |    9 ++-
>   drivers/net/ethernet/xscale/ixp4xx_eth.c          |    7 +-
>   drivers/net/phy/fixed.c                           |    5 +-
>   drivers/net/phy/icplus.c                          |   17 +++--
>   drivers/net/phy/mdio-bitbang.c                    |    5 +-
>   drivers/net/phy/mdio-octeon.c                     |    5 +-
>   drivers/net/phy/mdio_bus.c                        |    8 +-
>   drivers/net/phy/phy.c                             |    5 +-
>   drivers/net/phy/phy_device.c                      |   64 +++++++++++++------
>   include/linux/phy.h                               |   70 ++++++++++++++++++---
>   net/dsa/slave.c                                   |    5 +-
>   40 files changed, 270 insertions(+), 141 deletions(-)
>

  parent reply	other threads:[~2011-10-13 16:00 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-10-13 14:37 [PATCH v3 0/3] Add support for 10G to PHY Lib Andy Fleming
2011-10-13 14:37 ` [PATCH v3 1/3] phylib: Convert MDIO and PHY Lib drivers to support 10G Andy Fleming
2011-10-13 15:46   ` Ben Hutchings
2011-10-13 15:59     ` Andy Fleming
2011-10-13 16:00   ` David Daney [this message]
2011-10-14 21:17     ` Andy Fleming
2011-10-14 21:52       ` David Daney
2011-10-13 16:18   ` David Daney
2011-10-13 14:37 ` [PATCH v3 2/3] phylib: Convert MDIO bitbang to new MDIO 45 format Andy Fleming
2011-10-13 14:37 ` [PATCH v3 3/3] phylib: Add rudimentary Generic 10G support Andy Fleming
2011-10-13 15:51   ` Ben Hutchings

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=4E970B03.9060200@cavium.com \
    --to=david.daney@cavium.com \
    --cc=afleming@freescale.com \
    --cc=davem@davemloft.net \
    --cc=netdev@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.