| Message ID | 20170608092653.25221-8-antoine.tenart@free-electrons.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Jun 08, 2017 at 11:26:52AM +0200, Antoine Tenart wrote: > +#define MVMDIO_XSMI_MGNT_REG 0x0 > +#define MVMDIO_XSMI_READ_VALID BIT(29) > +#define MVMDIO_XSMI_BUSY BIT(30) > +#define MVMDIO_XSMI_ADDR_REG 0x8 > +#define MVMDIO_XSMI_PHYADDR_SHIFT 16 > +#define MVMDIO_XSMI_DEVADDR_SHIFT 21 > +#define MVMDIO_XSMI_READ_OPERATION (0x7 << 26) > +#define MVMDIO_XSMI_WRITE_OPERATION (0x5 << 27) Hi Antoine These two operations seem odd. Generally ops have the same shift. Andrew
On 06/08/2017 02:26 AM, Antoine Tenart wrote: > This patch adds the xMDIO interface support in the mvmdio driver. This > interface is used in Ethernet controllers on Marvell 370, 7k and 8k (as > of now). The xSMI interface supported by this driver complies with the > IEEE 802.3 clause 45 (while the SMI interface complies with the clause > 22). The xSMI interface is used by 10GbE devices. In the previous version you were properly defining a new compatibles strings for xmdio, but now you don't and instead you runtime select the operations based on whether MII_ADDR_C45 is set in the register which is fine from a functional perspective. If I get this right, the xMDIO controller is actually a superset of the MDIO controller and has an extra MVMDIO_XSMI_ADDR_REG register to preform C45 accesses? If that is the case (and looking at patch 8 that seems to be the case), you probably still need to define a new compatible string for that block, because it has a different register layout than its predecessor. [snip] > static int orion_mdio_wait_ready(const struct orion_mdio_ops *ops, > @@ -164,7 +236,7 @@ static int orion_mdio_read(struct mii_bus *bus, int mii_id, > int regnum) > { > struct orion_mdio_dev *dev = bus->priv; > - const struct orion_mdio_ops *ops = &orion_mdio_smi_ops; > + const struct orion_mdio_ops *ops = orion_mdio_get_ops(regnum); > int ret; > > mutex_lock(&dev->lock); > @@ -195,7 +267,7 @@ static int orion_mdio_write(struct mii_bus *bus, int mii_id, > int regnum, u16 value) > { > struct orion_mdio_dev *dev = bus->priv; > - const struct orion_mdio_ops *ops = &orion_mdio_smi_ops; > + const struct orion_mdio_ops *ops = orion_mdio_get_ops(regnum); > int ret; ok, that seems to work since you get the operation based on MII_ADDR_C45. Thanks! -- Florian
On Thu, Jun 08, 2017 at 09:42:21AM -0700, Florian Fainelli wrote: > On 06/08/2017 02:26 AM, Antoine Tenart wrote: > > This patch adds the xMDIO interface support in the mvmdio driver. This > > interface is used in Ethernet controllers on Marvell 370, 7k and 8k (as > > of now). The xSMI interface supported by this driver complies with the > > IEEE 802.3 clause 45 (while the SMI interface complies with the clause > > 22). The xSMI interface is used by 10GbE devices. > > In the previous version you were properly defining a new compatibles > strings for xmdio, but now you don't and instead you runtime select the > operations based on whether MII_ADDR_C45 is set in the register which is > fine from a functional perspective. > > If I get this right, the xMDIO controller is actually a superset of the > MDIO controller and has an extra MVMDIO_XSMI_ADDR_REG register to > preform C45 accesses? > > If that is the case (and looking at patch 8 that seems to be the case), > you probably still need to define a new compatible string for that > block, because it has a different register layout than its predecessor. Yes, i think you need the compatible string to return -EOPNOSUP when somebody tries to do a C45 access on the older IP which only has C22. Andrew
Hello Florian, Andrew, On Thu, Jun 08, 2017 at 06:55:46PM +0200, Andrew Lunn wrote: > On Thu, Jun 08, 2017 at 09:42:21AM -0700, Florian Fainelli wrote: > > On 06/08/2017 02:26 AM, Antoine Tenart wrote: > > > This patch adds the xMDIO interface support in the mvmdio driver. This > > > interface is used in Ethernet controllers on Marvell 370, 7k and 8k (as > > > of now). The xSMI interface supported by this driver complies with the > > > IEEE 802.3 clause 45 (while the SMI interface complies with the clause > > > 22). The xSMI interface is used by 10GbE devices. > > > > In the previous version you were properly defining a new compatibles > > strings for xmdio, but now you don't and instead you runtime select the > > operations based on whether MII_ADDR_C45 is set in the register which is > > fine from a functional perspective. > > > > If I get this right, the xMDIO controller is actually a superset of the > > MDIO controller and has an extra MVMDIO_XSMI_ADDR_REG register to > > preform C45 accesses? > > > > If that is the case (and looking at patch 8 that seems to be the case), > > you probably still need to define a new compatible string for that > > block, because it has a different register layout than its predecessor. > > Yes, i think you need the compatible string to return -EOPNOSUP when > somebody tries to do a C45 access on the older IP which only has C22. That's a very good point. I'll update the series to fix this. Thanks! Antoine
Hi Andrew, On Thu, Jun 08, 2017 at 06:03:31PM +0200, Andrew Lunn wrote: > On Thu, Jun 08, 2017 at 11:26:52AM +0200, Antoine Tenart wrote: > > +#define MVMDIO_XSMI_MGNT_REG 0x0 > > +#define MVMDIO_XSMI_READ_VALID BIT(29) > > +#define MVMDIO_XSMI_BUSY BIT(30) > > +#define MVMDIO_XSMI_ADDR_REG 0x8 > > +#define MVMDIO_XSMI_PHYADDR_SHIFT 16 > > +#define MVMDIO_XSMI_DEVADDR_SHIFT 21 > > +#define MVMDIO_XSMI_READ_OPERATION (0x7 << 26) > > +#define MVMDIO_XSMI_WRITE_OPERATION (0x5 << 27) > > These two operations seem odd. Generally ops have the same shift. Indeed, this is odd. I'll have a look at this. Thanks, Antoine
On Thu, Jun 08, 2017 at 09:42:21AM -0700, Florian Fainelli wrote: > On 06/08/2017 02:26 AM, Antoine Tenart wrote: > > This patch adds the xMDIO interface support in the mvmdio driver. This > > interface is used in Ethernet controllers on Marvell 370, 7k and 8k (as > > of now). The xSMI interface supported by this driver complies with the > > IEEE 802.3 clause 45 (while the SMI interface complies with the clause > > 22). The xSMI interface is used by 10GbE devices. > > In the previous version you were properly defining a new compatibles > strings for xmdio, but now you don't and instead you runtime select the > operations based on whether MII_ADDR_C45 is set in the register which is > fine from a functional perspective. > > If I get this right, the xMDIO controller is actually a superset of the > MDIO controller and has an extra MVMDIO_XSMI_ADDR_REG register to > preform C45 accesses? This is also a mistake. It's not a superset as the register offsets are different. So we can't use the same smi operations in both cases. We would need dedicated xmdio smi operations, but I don't think there is a board where a c22 PHY is connected to the xMDIO interface. I'll add smi and xsmi flags and return -ENOTSUPP based on the MII_ADDR_C45 bit and flags combination. If the need to support smi operations on the xMDIO bus arise it will be fairly easy to implement in the future. I'll rename the ops functions as well to differentiate mdio and xmdio operations. Thanks, Antoine
On Fri, Jun 09, 2017 at 10:25:41AM +0200, Antoine Tenart wrote: > On Thu, Jun 08, 2017 at 09:42:21AM -0700, Florian Fainelli wrote: > > On 06/08/2017 02:26 AM, Antoine Tenart wrote: > > > This patch adds the xMDIO interface support in the mvmdio driver. This > > > interface is used in Ethernet controllers on Marvell 370, 7k and 8k (as > > > of now). The xSMI interface supported by this driver complies with the > > > IEEE 802.3 clause 45 (while the SMI interface complies with the clause > > > 22). The xSMI interface is used by 10GbE devices. > > > > In the previous version you were properly defining a new compatibles > > strings for xmdio, but now you don't and instead you runtime select the > > operations based on whether MII_ADDR_C45 is set in the register which is > > fine from a functional perspective. > > > > If I get this right, the xMDIO controller is actually a superset of the > > MDIO controller and has an extra MVMDIO_XSMI_ADDR_REG register to > > preform C45 accesses? > > This is also a mistake. It's not a superset as the register offsets are > different. So we can't use the same smi operations in both cases. We > would need dedicated xmdio smi operations, but I don't think there is a > board where a c22 PHY is connected to the xMDIO interface. Hi Antoine I don't have the datasheet... Is it described as one device, which can do c22 via one set of registers, and c45 by another set of registers? Or are there two separate devices. In particular, does each device have there own MDC and MDIO pins? Do we have an C22 only device/bus, and a C45 only device/bus? Andrew
Hi Andrew, On Fri, Jun 09, 2017 at 03:26:24PM +0200, Andrew Lunn wrote: > On Fri, Jun 09, 2017 at 10:25:41AM +0200, Antoine Tenart wrote: > > On Thu, Jun 08, 2017 at 09:42:21AM -0700, Florian Fainelli wrote: > > > > > > If I get this right, the xMDIO controller is actually a superset of the > > > MDIO controller and has an extra MVMDIO_XSMI_ADDR_REG register to > > > preform C45 accesses? > > > > This is also a mistake. It's not a superset as the register offsets are > > different. So we can't use the same smi operations in both cases. We > > would need dedicated xmdio smi operations, but I don't think there is a > > board where a c22 PHY is connected to the xMDIO interface. > > Is it described as one device, which can do c22 via one set of > registers, and c45 by another set of registers? > > Or are there two separate devices. In particular, does each device > have there own MDC and MDIO pins? Do we have an C22 only device/bus, > and a C45 only device/bus? The MDIO/xMDIO registers are embedded into the network controller. The mvmdio driver was created at first to abstract this functionality outside the network controller driver because it is shared between all ports and used in different IPs. So it's not really devices per say. Looking at the datasheet/schematics there are two hardware buses, one for c22 and one for c45. So we should keep two separate nodes to describe the two interfaces. From what I read c45 is backward compatible with c22 so the xSMI interface should be capable to speak to c22 PHYs as well. And by looking at the datasheet this seems possible, although it's not completely clear. But anyway this wouldn't impact the dt bindings. What I'll propose in v3 is two separate nodes, with their own compatibles. The driver will have smi ops for the marvell,orion-mdio and xsmi ops for the marvell,xmdio. I won't implement for now the smi ops for marvell,xmdio as I can't test them and can't be 100% sure it would work. (But again this won't impact the dt and can be updated in the driver later). Does that sound right? Thanks! Antoine
On Fri, Jun 09, 2017 at 04:09:22PM +0200, Antoine Tenart wrote: > Hi Andrew, > > On Fri, Jun 09, 2017 at 03:26:24PM +0200, Andrew Lunn wrote: > > On Fri, Jun 09, 2017 at 10:25:41AM +0200, Antoine Tenart wrote: > > > On Thu, Jun 08, 2017 at 09:42:21AM -0700, Florian Fainelli wrote: > > > > > > > > If I get this right, the xMDIO controller is actually a superset of the > > > > MDIO controller and has an extra MVMDIO_XSMI_ADDR_REG register to > > > > preform C45 accesses? > > > > > > This is also a mistake. It's not a superset as the register offsets are > > > different. So we can't use the same smi operations in both cases. We > > > would need dedicated xmdio smi operations, but I don't think there is a > > > board where a c22 PHY is connected to the xMDIO interface. > > > > Is it described as one device, which can do c22 via one set of > > registers, and c45 by another set of registers? > > > > Or are there two separate devices. In particular, does each device > > have there own MDC and MDIO pins? Do we have an C22 only device/bus, > > and a C45 only device/bus? > > The MDIO/xMDIO registers are embedded into the network controller. The > mvmdio driver was created at first to abstract this functionality > outside the network controller driver because it is shared between all > ports and used in different IPs. So it's not really devices per say. > > Looking at the datasheet/schematics there are two hardware buses, one > for c22 and one for c45. So we should keep two separate nodes to > describe the two interfaces. From what I read c45 is backward > compatible with c22 so the xSMI interface should be capable to speak to > c22 PHYs as well. The on the wire protocol of c45 is backwards compatible with c22, in that a c22 device will not get confused by a c45 transaction on the bus. A c22 device will just ignore it. You cannot talk to a c22 device using c45. From what you are saying, you have a hardware block generating c45 transactions and a hardware block which generates c22 transactions. What is not clear to me is if these two hardware blocks are using the same physical MDC/MDIO pins, i.e. there is just one MDIO bus to the outside world, or are there two busses? > And by looking at the datasheet this seems possible, although it's > not completely clear. But anyway this wouldn't impact the dt > bindings. What i'm worried about is there being one set of MDC/MDIO lines. You should not expose that to linux as two mdio busses. It is one bus. The phylib will poll each phy on the bus once per second to check its state. The phylib serialises the read/writes at a bus level. So if you have one physical bus registered as two logical bus, at some point you are going to get simultaneous read/writes, and you are going to need a mutex low down to serialise this for physical bus access. And in the end, this would affect the dt binding. If it is one physical bus, you want one binding representing both c22 and c45 transactions. Andrew
Hi Andrew, On Fri, Jun 09, 2017 at 04:49:36PM +0200, Andrew Lunn wrote: > On Fri, Jun 09, 2017 at 04:09:22PM +0200, Antoine Tenart wrote: > > > > The MDIO/xMDIO registers are embedded into the network controller. The > > mvmdio driver was created at first to abstract this functionality > > outside the network controller driver because it is shared between all > > ports and used in different IPs. So it's not really devices per say. > > > > Looking at the datasheet/schematics there are two hardware buses, one > > for c22 and one for c45. So we should keep two separate nodes to > > describe the two interfaces. From what I read c45 is backward > > compatible with c22 so the xSMI interface should be capable to speak to > > c22 PHYs as well. > > The on the wire protocol of c45 is backwards compatible with c22, in > that a c22 device will not get confused by a c45 transaction on the > bus. A c22 device will just ignore it. You cannot talk to a c22 device > using c45. I see. > From what you are saying, you have a hardware block generating c45 > transactions and a hardware block which generates c22 transactions. > What is not clear to me is if these two hardware blocks are using the > same physical MDC/MDIO pins, i.e. there is just one MDIO bus to the > outside world, or are there two busses? There are two busses, one generating c22 transactions and one generating c45 transactions. Each bus has its own MDC/MDIO pins. > > And by looking at the datasheet this seems possible, although it's > > not completely clear. But anyway this wouldn't impact the dt > > bindings. > > What i'm worried about is there being one set of MDC/MDIO lines. You > should not expose that to linux as two mdio busses. It is one bus. > > The phylib will poll each phy on the bus once per second to check its > state. The phylib serialises the read/writes at a bus level. So if you > have one physical bus registered as two logical bus, at some point you > are going to get simultaneous read/writes, and you are going to need a > mutex low down to serialise this for physical bus access. > > And in the end, this would affect the dt binding. If it is one > physical bus, you want one binding representing both c22 and c45 > transactions. Of course. See above. Thanks! Antoine
> There are two busses, one generating c22 transactions and one generating > c45 transactions. Each bus has its own MDC/MDIO pins. O.K. That is what i wanted to know. So we want two completely separate device tree bindings, busses registered with Linux, etc. Thanks for clarification. Andrew
On Fri, Jun 09, 2017 at 05:03:40PM +0200, Andrew Lunn wrote: > > There are two busses, one generating c22 transactions and one generating > > c45 transactions. Each bus has its own MDC/MDIO pins. > > O.K. That is what i wanted to know. So we want two completely separate > device tree bindings, busses registered with Linux, etc. > > Thanks for clarification. So in the end I need one change in v3: to bind the xSMI usage to marvell,xmdio and the SMI one to marvell,orion-mdio. (Plus the GENMASK and offset comments). Antoine
On Fri, Jun 09, 2017 at 04:49:36PM +0200, Andrew Lunn wrote: > On Fri, Jun 09, 2017 at 04:09:22PM +0200, Antoine Tenart wrote: > > The MDIO/xMDIO registers are embedded into the network controller. The > > mvmdio driver was created at first to abstract this functionality > > outside the network controller driver because it is shared between all > > ports and used in different IPs. So it's not really devices per say. > > > > Looking at the datasheet/schematics there are two hardware buses, one > > for c22 and one for c45. So we should keep two separate nodes to > > describe the two interfaces. From what I read c45 is backward > > compatible with c22 so the xSMI interface should be capable to speak to > > c22 PHYs as well. > > The on the wire protocol of c45 is backwards compatible with c22, in > that a c22 device will not get confused by a c45 transaction on the > bus. A c22 device will just ignore it. You cannot talk to a c22 device > using c45. From what I can tell, having 'scoped the MDIO line and tried writing several different values to the XSMI registers, it is not possible for the XSMI block to generate C22 frame structures - the "start" bits are always "00", and I can't make them the required "01" for C22. However, I can confirm that bits 26 and 27 of the XSMI register are used directly for the OP field (so 0 << 26 produces a C45 address frame.) I suspect, although I haven't delved that deeply (yet) that bit 28 sets whether XSMI produces an address cycle itself along with the data cycle. Bit 31 appears to be writable, but has no effect on the frame structure. > What i'm worried about is there being one set of MDC/MDIO lines. You > should not expose that to linux as two mdio busses. It is one bus. We're independent - the SMI and XSMI blocks are two entirely separate interfaces with entirely separate hardware MDC/MDIO lines. The XSMI MDC/MDIO lines remain at logic '1' while phylib polls the PHY via the SMI interface.
On Fri, Jun 09, 2017 at 06:22:16PM +0200, Antoine Tenart wrote: > On Fri, Jun 09, 2017 at 05:03:40PM +0200, Andrew Lunn wrote: > > > There are two busses, one generating c22 transactions and one generating > > > c45 transactions. Each bus has its own MDC/MDIO pins. > > > > O.K. That is what i wanted to know. So we want two completely separate > > device tree bindings, busses registered with Linux, etc. > > > > Thanks for clarification. > > So in the end I need one change in v3: to bind the xSMI usage to > marvell,xmdio and the SMI one to marvell,orion-mdio. (Plus the GENMASK > and offset comments). Also, you need to 1. trap out on incorrect MII_ADDR_C45 in regnum for the interface. 2. mask the dev_addr with GENMASK(4, 0) (as merely shifting will leave the MII_ADDR_C45 bit set.) 3. moving MVMDIO_XSMI_PHYADDR_SHIFT / MVMDIO_XSMI_DEVADDR_SHIFT / MVMDIO_XSMI_READ_OPERATION / MVMDIO_XSMI_WRITE_OPERATION under the MVMDIO_XSMI_MGNT_REG reg - these definitions are nothing to do with MVMDIO_XSMI_ADDR_REG. 4. fixing MVMDIO_XSMI_WRITE_OPERATION to be 5 << 26, not 5 << 27.
On Fri, Jun 09, 2017 at 08:40:19AM +0200, Antoine Tenart wrote: > Hi Andrew, > > On Thu, Jun 08, 2017 at 06:03:31PM +0200, Andrew Lunn wrote: > > On Thu, Jun 08, 2017 at 11:26:52AM +0200, Antoine Tenart wrote: > > > +#define MVMDIO_XSMI_MGNT_REG 0x0 > > > +#define MVMDIO_XSMI_READ_VALID BIT(29) > > > +#define MVMDIO_XSMI_BUSY BIT(30) > > > +#define MVMDIO_XSMI_ADDR_REG 0x8 > > > +#define MVMDIO_XSMI_PHYADDR_SHIFT 16 > > > +#define MVMDIO_XSMI_DEVADDR_SHIFT 21 > > > +#define MVMDIO_XSMI_READ_OPERATION (0x7 << 26) > > > +#define MVMDIO_XSMI_WRITE_OPERATION (0x5 << 27) > > > > These two operations seem odd. Generally ops have the same shift. > > Indeed, this is odd. I'll have a look at this. The Marvell driver uses 5 << 26: +#define XOPCODE_OFFS 26 +#define XOPCODE_ADDR_READ (7 << XOPCODE_OFFS) +#define XOPCODE_ADDR_WRITE (5 << XOPCODE_OFFS) What this means is that with the incorrect shift in your driver, although writes appeared to work, they actually resulted in a post-read-increment-address frame (and hence no error.)
diff --git a/Documentation/devicetree/bindings/net/marvell-orion-mdio.txt b/Documentation/devicetree/bindings/net/marvell-orion-mdio.txt index ccdabdcc8618..36be4ca5dab2 100644 --- a/Documentation/devicetree/bindings/net/marvell-orion-mdio.txt +++ b/Documentation/devicetree/bindings/net/marvell-orion-mdio.txt @@ -1,9 +1,9 @@ * Marvell MDIO Ethernet Controller interface The Ethernet controllers of the Marvel Kirkwood, Dove, Orion5x, -MV78xx0, Armada 370 and Armada XP have an identical unit that provides -an interface with the MDIO bus. This driver handles this MDIO -interface. +MV78xx0, Armada 370, Armada XP, Armada 7k and Armada 8k have an +identical unit that provides an interface with the MDIO bus or to +the xMDIO bus. This driver handles these interfaces. Required properties: - compatible: "marvell,orion-mdio" diff --git a/drivers/net/ethernet/marvell/Kconfig b/drivers/net/ethernet/marvell/Kconfig index da6fb825afea..205bb7e683b7 100644 --- a/drivers/net/ethernet/marvell/Kconfig +++ b/drivers/net/ethernet/marvell/Kconfig @@ -35,9 +35,9 @@ config MVMDIO depends on HAS_IOMEM select PHYLIB ---help--- - This driver supports the MDIO interface found in the network - interface units of the Marvell EBU SoCs (Kirkwood, Orion5x, - Dove, Armada 370 and Armada XP). + This driver supports the MDIO and xMDIO interfaces found in + the network interface units of the Marvell EBU SoCs (Kirkwood, + Orion5x, Dove, Armada 370, Armada XP, Armada 7k and Armada 8k). This driver is used by the MV643XX_ETH and MVNETA drivers. diff --git a/drivers/net/ethernet/marvell/mvmdio.c b/drivers/net/ethernet/marvell/mvmdio.c index 07b18f60da2a..7b650ef67347 100644 --- a/drivers/net/ethernet/marvell/mvmdio.c +++ b/drivers/net/ethernet/marvell/mvmdio.c @@ -41,6 +41,15 @@ #define MVMDIO_ERR_INT_SMI_DONE 0x00000010 #define MVMDIO_ERR_INT_MASK 0x0080 +#define MVMDIO_XSMI_MGNT_REG 0x0 +#define MVMDIO_XSMI_READ_VALID BIT(29) +#define MVMDIO_XSMI_BUSY BIT(30) +#define MVMDIO_XSMI_ADDR_REG 0x8 +#define MVMDIO_XSMI_PHYADDR_SHIFT 16 +#define MVMDIO_XSMI_DEVADDR_SHIFT 21 +#define MVMDIO_XSMI_READ_OPERATION (0x7 << 26) +#define MVMDIO_XSMI_WRITE_OPERATION (0x5 << 27) + /* * SMI Timeout measurements: * - Kirkwood 88F6281 (Globalscale Dreamplug): 45us to 95us (Interrupt) @@ -50,6 +59,9 @@ #define MVMDIO_SMI_POLL_INTERVAL_MIN 45 #define MVMDIO_SMI_POLL_INTERVAL_MAX 55 +#define MVMDIO_XSMI_POLL_INTERVAL_MIN 150 +#define MVMDIO_XSMI_POLL_INTERVAL_MAX 160 + struct orion_mdio_dev { struct mutex lock; void __iomem *regs; @@ -75,6 +87,7 @@ struct orion_mdio_ops { unsigned int poll_interval_max; }; +/* smi */ static int orion_mdio_smi_is_done(struct orion_mdio_dev *dev) { return !(readl(dev->regs) & MVMDIO_SMI_BUSY); @@ -120,6 +133,65 @@ static const struct orion_mdio_ops orion_mdio_smi_ops = { .poll_interval_max = MVMDIO_SMI_POLL_INTERVAL_MAX, }; +/* xsmi */ +static int orion_mdio_xsmi_is_done(struct orion_mdio_dev *dev) +{ + return !(readl(dev->regs + MVMDIO_XSMI_MGNT_REG) & MVMDIO_XSMI_BUSY); +} + +static int orion_mdio_xsmi_is_read_valid(struct orion_mdio_dev *dev) +{ + return readl(dev->regs + MVMDIO_XSMI_MGNT_REG) & MVMDIO_XSMI_READ_VALID; +} + +static void orion_mdio_xsmi_start_read_op(struct orion_mdio_dev *dev, + int mii_id, int regnum) +{ + u16 dev_addr = regnum >> 16; + + writel(regnum & GENMASK(15,0), dev->regs + MVMDIO_XSMI_ADDR_REG); + writel((mii_id << MVMDIO_XSMI_PHYADDR_SHIFT) | + (dev_addr << MVMDIO_XSMI_DEVADDR_SHIFT) | + MVMDIO_XSMI_READ_OPERATION, + dev->regs + MVMDIO_XSMI_MGNT_REG); +} + +static u16 orion_mdio_xsmi_read_op(struct orion_mdio_dev *dev) +{ + return readl(dev->regs + MVMDIO_XSMI_MGNT_REG) & GENMASK(15,0); +} + +static void orion_mdio_xsmi_write_op(struct orion_mdio_dev *dev, int mii_id, + int regnum, u16 value) +{ + u16 dev_addr = regnum >> 16; + + writel(regnum & GENMASK(15,0), dev->regs + MVMDIO_XSMI_ADDR_REG); + writel((mii_id << MVMDIO_XSMI_PHYADDR_SHIFT) | + (dev_addr << MVMDIO_XSMI_DEVADDR_SHIFT) | + MVMDIO_XSMI_WRITE_OPERATION | value, + dev->regs + MVMDIO_XSMI_MGNT_REG); +} + +static const struct orion_mdio_ops orion_mdio_xsmi_ops = { + .is_done = orion_mdio_xsmi_is_done, + .is_read_valid = orion_mdio_xsmi_is_read_valid, + .start_read = orion_mdio_xsmi_start_read_op, + .read = orion_mdio_xsmi_read_op, + .write = orion_mdio_xsmi_write_op, + + .poll_interval_min = MVMDIO_XSMI_POLL_INTERVAL_MIN, + .poll_interval_max = MVMDIO_XSMI_POLL_INTERVAL_MAX, +}; + +static const struct orion_mdio_ops *orion_mdio_get_ops(int regnum) +{ + if (regnum & MII_ADDR_C45) + return &orion_mdio_xsmi_ops; + + return &orion_mdio_smi_ops; +} + /* Wait for the SMI unit to be ready for another operation */ static int orion_mdio_wait_ready(const struct orion_mdio_ops *ops, @@ -164,7 +236,7 @@ static int orion_mdio_read(struct mii_bus *bus, int mii_id, int regnum) { struct orion_mdio_dev *dev = bus->priv; - const struct orion_mdio_ops *ops = &orion_mdio_smi_ops; + const struct orion_mdio_ops *ops = orion_mdio_get_ops(regnum); int ret; mutex_lock(&dev->lock); @@ -195,7 +267,7 @@ static int orion_mdio_write(struct mii_bus *bus, int mii_id, int regnum, u16 value) { struct orion_mdio_dev *dev = bus->priv; - const struct orion_mdio_ops *ops = &orion_mdio_smi_ops; + const struct orion_mdio_ops *ops = orion_mdio_get_ops(regnum); int ret; mutex_lock(&dev->lock);
This patch adds the xMDIO interface support in the mvmdio driver. This interface is used in Ethernet controllers on Marvell 370, 7k and 8k (as of now). The xSMI interface supported by this driver complies with the IEEE 802.3 clause 45 (while the SMI interface complies with the clause 22). The xSMI interface is used by 10GbE devices. Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com> --- .../devicetree/bindings/net/marvell-orion-mdio.txt | 6 +- drivers/net/ethernet/marvell/Kconfig | 6 +- drivers/net/ethernet/marvell/mvmdio.c | 76 +++++++++++++++++++++- 3 files changed, 80 insertions(+), 8 deletions(-)