* Re: [PATCH net-next v5 0/5] Introduce NETIF_F_GRO_HW
From: Alexander Duyck @ 2017-12-29 15:12 UTC (permalink / raw)
To: Sabrina Dubroca; +Cc: Michael Chan, David Miller, Netdev, Andrew Gospodarek
In-Reply-To: <20171229124332.GA20090@bistromath.localdomain>
On Fri, Dec 29, 2017 at 4:43 AM, Sabrina Dubroca <sd@queasysnail.net> wrote:
> 2017-12-22, 10:14:32 -0800, Alexander Duyck wrote:
>> On Fri, Dec 22, 2017 at 6:57 AM, Sabrina Dubroca <sd@queasysnail.net> wrote:
>> > IIUC, with the patches that were applied, each driver can define
>> > whether GRO_HW depends on GRO? From a user's perspective, this
>> > inconsistent behavior is going to be quite confusing.
>> >
>> > Worse than inconsistent behavior, it looks like a driver deciding that
>> > GRO_HW doesn't depend on GRO is going to introduce a change of
>> > behavior. Previously, when GRO was disabled, there wouldn't be any
>> > packet over MTU handed to the network stack. Now, even if GRO is
>> > disabled, GRO_HW might still be enabled, so we might get over-MTU
>> > packets because of hardware GRO.
>>
>> This isn't actually true. LRO was still handling packets larger than
>> MTU over even when GRO was disabled.
>
> Sure, LRO will also cause that, but we're speaking in the context of
> GRO here, which means no LRO.
We are talking about GRO_HW. Which is basically aggregation being
performed in hardware. The choice of name is unfortunate though since
it implies this is GRO when what is actually happening is just
GRO-like. Really the only difference between LRO and GRO_HW is that
the frames are better formed in the final result. It is a matter of
what is performed versus where it is performed.
>> > I don't think drivers should be allowed to say "GRO_HW doesn't depend
>> > on GRO".
>>
>> Why not, it doesn't. In my mind GRO_HW is closer to LRO than it is to
>> GRO.
>
> Why do you say that? It looks like GRO to me. These drivers are
> calling tcp_gro_complete() for example.
The final frame output in the case of frames that are aggregated will
be the same as GRO. However LRO could theoretically do the same thing
if the hardware had been implemented correctly in the first place.
>> The only ugly bit as I see it is that these devices were exposing
>> the feature via the GRO flag in the first place. So for the sake of
>> legacy they might want to carry around the dependency.
>>
>> > I think it's reasonable to be able to disable software GRO even if
>> > hardware GRO is enabled. Thus, I would propose:
>> > - keep the current GRO flag
>> > - add a GRO_HW flag, depending on GRO, enforced by the core as in
>> > earlier versions of these patches
>> > - add a GRO_SW flag, also depending on GRO
>>
>> This seems like a bunch of extra overhead for not much gain. Do we
>> really need to fork GRO into 3 bits? I would argue that GRO_HW really
>> should have been branded something like FORWARDABLE_LRO, but nobody
>> wanted to touch the name LRO since it apparently has some negative
>> stigma to it. If we had used a name like that we probably wouldn't be
>> going through all these extra hoops. The only real reason why this is
>> even being associated with GRO in the first place is that is how this
>> feature was hidden by the drivers so they got around having to deal
>> with the LRO being disabled for routing/forwarding issue. Those are
>> the parts that want to keep it associated with GRO since that is how
>> they exposed it in their devices originally.
>
> I think it wouldn't have hidden behind GRO if it wasn't GRO. Again,
> why do you think it's not GRO?
The only real piece I see as missing the propagation bits for GRO_HW
to upper devices. The argument was that GRO doesn't have to do that,
but I think we are going to have to get there at some point so that
when we encounter the first piece of hardware that does this wrong we
have a switch ready to allow us to disable it for debugging. If we are
insistent on having a switch that binds together GRO and GRO_HW one
thought I had was to change the meaning of GRO_HW for virtual devices
to indicate that we allow GRO to happen on the lower devices
associated with this device. My thought was that GRO_HW allows GRO to
take place on the physical hardware below the netdev, so why couldn't
we say that GRO_HW also applies to upper devices and them indicating
they don't want GRO, GRO_HW, or LRO for lower devices. Thoughts? The
only reason it occurred to me is that there is currently no way to
propagate a disable of GRO so GRO_HW might be a way to do that for
virtual devices. It would make GRO_HW more about the location of the
GRO feature instead of the feature itself. Basically if you clear it
then nothing below that point should be doing any sort of aggregation.
Thanks.
- Alex
^ permalink raw reply
* 3c59x: pci_unmap_single() oops
From: tedheadster @ 2017-12-29 14:51 UTC (permalink / raw)
To: netdev, nhorman
In the 4.15.0-rc5 kernel (and likely earlier) I get the following oops.
3c59x 0000:00:0c.0 enp0s12: renamed from eth0
enp0s12: setting half-duplex.
------------[ cut here ]------------
3c59x 0000:00:0c.0: DMA-API: device driver failed to check map
error[device address=0x0000000009e1b040] [size=1536 bytes] [mapped as
single]
WARNING: CPU: 0 PID: 1 at check_unmap+0x559/0x695
Modules linked in: ohci_pci ohci_hcd ehci_pci ehci_hcd usbcore pcspkr
serio_raw 3c59x mii usb_common ipv6
CPU: 0 PID: 1 Comm: systemd Not tainted 4.15.0-rc5.i486 #10
EIP: check_unmap+0x559/0x695
EFLAGS: 00010096 CPU: 0
EAX: 0000008c EBX: cb8a8660 ECX: c0881544 EDX: 00000001
ESI: cb8e5280 EDI: c06e8b9f EBP: cb821e50 ESP: cb821df8
DS: 007b ES: 007b FS: 00d8 GS: 0033 SS: 0068
CR0: 80050033 CR2: 004f1700 CR3: 0b2f9000 CR4: 00000000
Call Trace:
? mntput_no_expire+0x13/0x105
debug_dma_unmap_page+0x61/0x69
pci_unmap_single+0x4c/0x56 [3c59x]
boomerang_rx+0x250/0x42a [3c59x]
boomerang_interrupt+0xde/0x3ea [3c59x]
__handle_irq_event_percpu+0x2a/0xaf
handle_irq_event_percpu+0x17/0x3d
handle_irq_event+0x22/0x3b
handle_level_irq+0x55/0x7a
handle_irq+0x4f/0x58
do_IRQ+0x35/0x95
common_interrupt+0x34/0x40
EIP: 0xb7ae6970
EFLAGS: 00000246 CPU: 0
EAX: b7e1c3d8 EBX: b7eef344 ECX: 00000000 EDX: 007048c4
ESI: 00000013 EDI: 007048c4 EBP: bf8d81c8 ESP: bf8d8094
DS: 007b ES: 007b FS: 0000 GS: 0033 SS: 007b
Code: 01 00 00 8b 58 08 e9 4a 01 00 00 bb 1b 2f 70 c0 89 d8 57 ff 75
e4 ff 75 e0 ff 75 dc ff 75 d8 53 50 68 bd 16 71 c0 e8 1f 3c e1 ff <0f>
ff 83 c4 20 83 3d 44 d5 7e c0 00 75 0f a1 f0 e6 7b c0 85 c0
---[ end trace 8b519628d8703199 ]---
This may relate to "3c59x: Add dma error checking and recovery"
- Matthew Whitehead
^ permalink raw reply
* Compliment of the day
From: Kabore Zongo @ 2017-12-29 13:45 UTC (permalink / raw)
--
Compliment of the day , I am a professional banker with International
and Qualified Experience. Please i seek your Urgent Attention and
Cooperation to transfer the sum of $8.5M into your account. Risk free
and Beneficial. Waiting response so that we can proceed to the next
level(zongokab2017@yahoo.com
^ permalink raw reply
* [PATCH net-next 5/5] net: phy: marvell10g: add support for half duplex 100M and 10M
From: Russell King @ 2017-12-29 12:46 UTC (permalink / raw)
To: Andrew Lunn, Florian Fainelli; +Cc: netdev
In-Reply-To: <20171229124412.GA10595@n2100.armlinux.org.uk>
Add support for half-duplex 100M and 10M copper connections by parsing
the advertisment results rather than trying to decode the negotiated
speed from one of the PHYs "vendor" registers. This allows us to
decode the duplex as well, which means we can support half-duplex mode
for the slower speeds.
Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
drivers/net/phy/marvell10g.c | 37 ++++++++++++-------------------------
1 file changed, 12 insertions(+), 25 deletions(-)
diff --git a/drivers/net/phy/marvell10g.c b/drivers/net/phy/marvell10g.c
index c01bb2654d69..26220593c529 100644
--- a/drivers/net/phy/marvell10g.c
+++ b/drivers/net/phy/marvell10g.c
@@ -42,13 +42,6 @@ enum {
*/
MV_AN_CTRL1000 = 0x8000, /* 1000base-T control register */
MV_AN_STAT1000 = 0x8001, /* 1000base-T status register */
-
- /* This register appears to reflect the copper status */
- MV_AN_RESULT = 0xa016,
- MV_AN_RESULT_SPD_10 = BIT(12),
- MV_AN_RESULT_SPD_100 = BIT(13),
- MV_AN_RESULT_SPD_1000 = BIT(14),
- MV_AN_RESULT_SPD_10000 = BIT(15),
};
static int mv3310_modify(struct phy_device *phydev, int devad, u16 reg,
@@ -239,12 +232,18 @@ static int mv3310_config_init(struct phy_device *phydev)
if (val & MDIO_PMA_EXTABLE_1000BKX)
__set_bit(ETHTOOL_LINK_MODE_1000baseKX_Full_BIT,
supported);
- if (val & MDIO_PMA_EXTABLE_100BTX)
+ if (val & MDIO_PMA_EXTABLE_100BTX) {
__set_bit(ETHTOOL_LINK_MODE_100baseT_Full_BIT,
supported);
- if (val & MDIO_PMA_EXTABLE_10BT)
+ __set_bit(ETHTOOL_LINK_MODE_100baseT_Half_BIT,
+ supported);
+ }
+ if (val & MDIO_PMA_EXTABLE_10BT) {
__set_bit(ETHTOOL_LINK_MODE_10baseT_Full_BIT,
supported);
+ __set_bit(ETHTOOL_LINK_MODE_10baseT_Half_BIT,
+ supported);
+ }
}
if (!ethtool_convert_link_mode_to_legacy_u32(&mask, supported))
@@ -412,22 +411,8 @@ static int mv3310_read_status(struct phy_device *phydev)
phydev->lp_advertising |= mii_stat1000_to_ethtool_lpa_t(val);
- if (phydev->autoneg == AUTONEG_ENABLE) {
- val = phy_read_mmd(phydev, MDIO_MMD_AN, MV_AN_RESULT);
- if (val < 0)
- return val;
-
- if (val & MV_AN_RESULT_SPD_10000)
- phydev->speed = SPEED_10000;
- else if (val & MV_AN_RESULT_SPD_1000)
- phydev->speed = SPEED_1000;
- else if (val & MV_AN_RESULT_SPD_100)
- phydev->speed = SPEED_100;
- else if (val & MV_AN_RESULT_SPD_10)
- phydev->speed = SPEED_10;
-
- phydev->duplex = DUPLEX_FULL;
- }
+ if (phydev->autoneg == AUTONEG_ENABLE)
+ phy_resolve_aneg_linkmode(phydev);
}
if (phydev->autoneg != AUTONEG_ENABLE) {
@@ -469,7 +454,9 @@ static struct phy_driver mv3310_drivers[] = {
.phy_id_mask = MARVELL_PHY_ID_MASK,
.name = "mv88x3310",
.features = SUPPORTED_10baseT_Full |
+ SUPPORTED_10baseT_Half |
SUPPORTED_100baseT_Full |
+ SUPPORTED_100baseT_Half |
SUPPORTED_1000baseT_Full |
SUPPORTED_Autoneg |
SUPPORTED_TP |
--
2.7.4
^ permalink raw reply related
* [PATCH net-next 4/5] net: phy: add helper to convert negotiation result to phy settings
From: Russell King @ 2017-12-29 12:46 UTC (permalink / raw)
To: Andrew Lunn, Florian Fainelli; +Cc: netdev
In-Reply-To: <20171229124412.GA10595@n2100.armlinux.org.uk>
Add a helper to convert the result of the autonegotiation advertisment
into the PHYs speed and duplex settings. If the result is full duplex,
also extract the pause mode settings from the link partner advertisment.
Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
drivers/net/phy/phy-core.c | 43 +++++++++++++++++++++++++++++++++++++++++++
include/linux/phy.h | 2 ++
2 files changed, 45 insertions(+)
diff --git a/drivers/net/phy/phy-core.c b/drivers/net/phy/phy-core.c
index 9c08850eed16..3d67f182b844 100644
--- a/drivers/net/phy/phy-core.c
+++ b/drivers/net/phy/phy-core.c
@@ -189,6 +189,49 @@ size_t phy_speeds(unsigned int *speeds, size_t size,
return count;
}
+/**
+ * phy_resolve_aneg_linkmode - resolve the advertisments into phy settings
+ * @phydev: The phy_device struct
+ *
+ * Resolve our and the link partner advertisments into their corresponding
+ * speed and duplex. If full duplex was negotiated, extract the pause mode
+ * from the link partner mask.
+ */
+void phy_resolve_aneg_linkmode(struct phy_device *phydev)
+{
+ u32 common = phydev->lp_advertising & phydev->advertising;
+
+ if (common & ADVERTISED_10000baseT_Full) {
+ phydev->speed = SPEED_10000;
+ phydev->duplex = DUPLEX_FULL;
+ } else if (common & ADVERTISED_1000baseT_Full) {
+ phydev->speed = SPEED_1000;
+ phydev->duplex = DUPLEX_FULL;
+ } else if (common & ADVERTISED_1000baseT_Half) {
+ phydev->speed = SPEED_1000;
+ phydev->duplex = DUPLEX_HALF;
+ } else if (common & ADVERTISED_100baseT_Full) {
+ phydev->speed = SPEED_100;
+ phydev->duplex = DUPLEX_FULL;
+ } else if (common & ADVERTISED_100baseT_Half) {
+ phydev->speed = SPEED_100;
+ phydev->duplex = DUPLEX_HALF;
+ } else if (common & ADVERTISED_10baseT_Full) {
+ phydev->speed = SPEED_10;
+ phydev->duplex = DUPLEX_FULL;
+ } else if (common & ADVERTISED_10baseT_Half) {
+ phydev->speed = SPEED_10;
+ phydev->duplex = DUPLEX_HALF;
+ }
+
+ if (phydev->duplex == DUPLEX_FULL) {
+ phydev->pause = !!(phydev->lp_advertising & ADVERTISED_Pause);
+ phydev->asym_pause = !!(phydev->lp_advertising &
+ ADVERTISED_Asym_Pause);
+ }
+}
+EXPORT_SYMBOL_GPL(phy_resolve_aneg_linkmode);
+
static void mmd_phy_indirect(struct mii_bus *bus, int phy_addr, int devad,
u16 regnum)
{
diff --git a/include/linux/phy.h b/include/linux/phy.h
index bb89a60716c2..0db52b272fd7 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -693,6 +693,8 @@ phy_lookup_setting(int speed, int duplex, const unsigned long *mask,
size_t phy_speeds(unsigned int *speeds, size_t size,
unsigned long *mask, size_t maxbit);
+void phy_resolve_aneg_linkmode(struct phy_device *phydev);
+
/**
* phy_read_mmd - Convenience function for reading a register
* from an MMD on a given PHY.
--
2.7.4
^ permalink raw reply related
* [PATCH net-next 3/5] net: phy: marvell10g: clean up interface mode switching
From: Russell King @ 2017-12-29 12:46 UTC (permalink / raw)
To: Andrew Lunn, Florian Fainelli; +Cc: netdev
In-Reply-To: <20171229124412.GA10595@n2100.armlinux.org.uk>
Centralise the PHY interface mode switching, rather than having it in
two places.
Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
drivers/net/phy/marvell10g.c | 36 ++++++++++++++++++++----------------
1 file changed, 20 insertions(+), 16 deletions(-)
diff --git a/drivers/net/phy/marvell10g.c b/drivers/net/phy/marvell10g.c
index 632f2ec15e39..c01bb2654d69 100644
--- a/drivers/net/phy/marvell10g.c
+++ b/drivers/net/phy/marvell10g.c
@@ -333,6 +333,24 @@ static int mv3310_aneg_done(struct phy_device *phydev)
return genphy_c45_aneg_done(phydev);
}
+static void mv3310_update_interface(struct phy_device *phydev)
+{
+ if ((phydev->interface == PHY_INTERFACE_MODE_SGMII ||
+ phydev->interface == PHY_INTERFACE_MODE_10GKR) && phydev->link) {
+ /* The PHY automatically switches its serdes interface (and
+ * active PHYXS instance) between Cisco SGMII and 10GBase-KR
+ * modes according to the speed. Florian suggests setting
+ * phydev->interface to communicate this to the MAC. Only do
+ * this if we are already in either SGMII or 10GBase-KR mode.
+ */
+ if (phydev->speed == SPEED_10000)
+ phydev->interface = PHY_INTERFACE_MODE_10GKR;
+ else if (phydev->speed >= SPEED_10 &&
+ phydev->speed < SPEED_10000)
+ phydev->interface = PHY_INTERFACE_MODE_SGMII;
+ }
+}
+
/* 10GBASE-ER,LR,LRM,SR do not support autonegotiation. */
static int mv3310_read_10gbr_status(struct phy_device *phydev)
{
@@ -340,8 +358,7 @@ static int mv3310_read_10gbr_status(struct phy_device *phydev)
phydev->speed = SPEED_10000;
phydev->duplex = DUPLEX_FULL;
- if (phydev->interface == PHY_INTERFACE_MODE_SGMII)
- phydev->interface = PHY_INTERFACE_MODE_10GKR;
+ mv3310_update_interface(phydev);
return 0;
}
@@ -441,20 +458,7 @@ static int mv3310_read_status(struct phy_device *phydev)
}
}
- if ((phydev->interface == PHY_INTERFACE_MODE_SGMII ||
- phydev->interface == PHY_INTERFACE_MODE_10GKR) && phydev->link) {
- /* The PHY automatically switches its serdes interface (and
- * active PHYXS instance) between Cisco SGMII and 10GBase-KR
- * modes according to the speed. Florian suggests setting
- * phydev->interface to communicate this to the MAC. Only do
- * this if we are already in either SGMII or 10GBase-KR mode.
- */
- if (phydev->speed == SPEED_10000)
- phydev->interface = PHY_INTERFACE_MODE_10GKR;
- else if (phydev->speed >= SPEED_10 &&
- phydev->speed < SPEED_10000)
- phydev->interface = PHY_INTERFACE_MODE_SGMII;
- }
+ mv3310_update_interface(phydev);
return 0;
}
--
2.7.4
^ permalink raw reply related
* [PATCH net-next 2/5] net: phy: marvell10g: add MDI swap reporting
From: Russell King @ 2017-12-29 12:46 UTC (permalink / raw)
To: Andrew Lunn, Florian Fainelli; +Cc: netdev
In-Reply-To: <20171229124412.GA10595@n2100.armlinux.org.uk>
Add reporting of the MDI swap to the Marvell 10G PHY driver by providing
a generic implementation for the standard 10GBASE-T pair swap register
and polarity register. We also support reading the MDI swap status for
1G and below from a PCS register.
Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
drivers/net/phy/marvell10g.c | 31 +++++++++++++++++++++++++++++++
drivers/net/phy/phy-c45.c | 33 +++++++++++++++++++++++++++++++++
include/linux/phy.h | 1 +
3 files changed, 65 insertions(+)
diff --git a/drivers/net/phy/marvell10g.c b/drivers/net/phy/marvell10g.c
index 0d503493ac14..632f2ec15e39 100644
--- a/drivers/net/phy/marvell10g.c
+++ b/drivers/net/phy/marvell10g.c
@@ -31,6 +31,11 @@ enum {
MV_PCS_BASE_R = 0x1000,
MV_PCS_1000BASEX = 0x2000,
+ MV_PCS_PAIRSWAP = 0x8182,
+ MV_PCS_PAIRSWAP_MASK = 0x0003,
+ MV_PCS_PAIRSWAP_AB = 0x0002,
+ MV_PCS_PAIRSWAP_NONE = 0x0003,
+
/* These registers appear at 0x800X and 0xa00X - the 0xa00X control
* registers appear to set themselves to the 0x800X when AN is
* restarted, but status registers appear readable from either.
@@ -267,6 +272,9 @@ static int mv3310_config_aneg(struct phy_device *phydev)
u32 advertising;
int ret;
+ /* We don't support manual MDI control */
+ phydev->mdix_ctrl = ETH_TP_MDI_AUTO;
+
if (phydev->autoneg == AUTONEG_DISABLE) {
ret = genphy_c45_pma_setup_forced(phydev);
if (ret < 0)
@@ -356,6 +364,7 @@ static int mv3310_read_status(struct phy_device *phydev)
phydev->link = 0;
phydev->pause = 0;
phydev->asym_pause = 0;
+ phydev->mdix = 0;
val = phy_read_mmd(phydev, MDIO_MMD_PCS, MV_PCS_BASE_R + MDIO_STAT1);
if (val < 0)
@@ -410,6 +419,28 @@ static int mv3310_read_status(struct phy_device *phydev)
return val;
}
+ if (phydev->speed == SPEED_10000) {
+ val = genphy_c45_read_mdix(phydev);
+ if (val < 0)
+ return val;
+ } else {
+ val = phy_read_mmd(phydev, MDIO_MMD_PCS, MV_PCS_PAIRSWAP);
+ if (val < 0)
+ return val;
+
+ switch (val & MV_PCS_PAIRSWAP_MASK) {
+ case MV_PCS_PAIRSWAP_AB:
+ phydev->mdix = ETH_TP_MDI_X;
+ break;
+ case MV_PCS_PAIRSWAP_NONE:
+ phydev->mdix = ETH_TP_MDI;
+ break;
+ default:
+ phydev->mdix = ETH_TP_MDI_INVALID;
+ break;
+ }
+ }
+
if ((phydev->interface == PHY_INTERFACE_MODE_SGMII ||
phydev->interface == PHY_INTERFACE_MODE_10GKR) && phydev->link) {
/* The PHY automatically switches its serdes interface (and
diff --git a/drivers/net/phy/phy-c45.c b/drivers/net/phy/phy-c45.c
index dada819c6b78..a4576859afae 100644
--- a/drivers/net/phy/phy-c45.c
+++ b/drivers/net/phy/phy-c45.c
@@ -233,6 +233,39 @@ int genphy_c45_read_pma(struct phy_device *phydev)
}
EXPORT_SYMBOL_GPL(genphy_c45_read_pma);
+/**
+ * genphy_c45_read_mdix - read mdix status from PMA
+ * @phydev: target phy_device struct
+ */
+int genphy_c45_read_mdix(struct phy_device *phydev)
+{
+ int val;
+
+ if (phydev->speed == SPEED_10000) {
+ val = phy_read_mmd(phydev, MDIO_MMD_PMAPMD,
+ MDIO_PMA_10GBT_SWAPPOL);
+ if (val < 0)
+ return val;
+
+ switch (val) {
+ case MDIO_PMA_10GBT_SWAPPOL_ABNX | MDIO_PMA_10GBT_SWAPPOL_CDNX:
+ phydev->mdix = ETH_TP_MDI;
+ break;
+
+ case 0:
+ phydev->mdix = ETH_TP_MDI_X;
+ break;
+
+ default:
+ phydev->mdix = ETH_TP_MDI_INVALID;
+ break;
+ }
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(genphy_c45_read_mdix);
+
/* The gen10g_* functions are the old Clause 45 stub */
static int gen10g_config_aneg(struct phy_device *phydev)
diff --git a/include/linux/phy.h b/include/linux/phy.h
index 0ee4ece312da..bb89a60716c2 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -943,6 +943,7 @@ int genphy_c45_read_lpa(struct phy_device *phydev);
int genphy_c45_read_pma(struct phy_device *phydev);
int genphy_c45_pma_setup_forced(struct phy_device *phydev);
int genphy_c45_an_disable_aneg(struct phy_device *phydev);
+int genphy_c45_read_mdix(struct phy_device *phydev);
static inline int phy_read_status(struct phy_device *phydev)
{
--
2.7.4
^ permalink raw reply related
* [PATCH net-next 1/5] net: phy: marvell10g: update header comments
From: Russell King @ 2017-12-29 12:46 UTC (permalink / raw)
To: Andrew Lunn, Florian Fainelli; +Cc: netdev
In-Reply-To: <20171229124412.GA10595@n2100.armlinux.org.uk>
Update header comments to indicate the newly found behaviour with XAUI
interfaces.
Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
drivers/net/phy/marvell10g.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/net/phy/marvell10g.c b/drivers/net/phy/marvell10g.c
index 87f18cee1533..0d503493ac14 100644
--- a/drivers/net/phy/marvell10g.c
+++ b/drivers/net/phy/marvell10g.c
@@ -6,12 +6,18 @@
*
* There appears to be several different data paths through the PHY which
* are automatically managed by the PHY. The following has been determined
- * via observation and experimentation:
+ * via observation and experimentation for a setup using single-lane Serdes:
*
* SGMII PHYXS -- BASE-T PCS -- 10G PMA -- AN -- Copper (for <= 1G)
* 10GBASE-KR PHYXS -- BASE-T PCS -- 10G PMA -- AN -- Copper (for 10G)
* 10GBASE-KR PHYXS -- BASE-R PCS -- Fiber
*
+ * With XAUI, observation shows:
+ *
+ * XAUI PHYXS -- <appropriate PCS as above>
+ *
+ * and no switching of the host interface mode occurs.
+ *
* If both the fiber and copper ports are connected, the first to gain
* link takes priority and the other port is completely locked out.
*/
--
2.7.4
^ permalink raw reply related
* [PATCH net-next 0/5] marvell10g updates
From: Russell King - ARM Linux @ 2017-12-29 12:44 UTC (permalink / raw)
To: Andrew Lunn, Florian Fainelli; +Cc: netdev
Hi,
This series:
- adds MDI/MDIX reporting
- adds support for 10/100Mbps half-duplex link modes
- adds a comment describing the setup on VF610 ZII boards (where
the phy interface mode doesn't change.)
- cleans up the phy interace mode switching
drivers/net/phy/marvell10g.c | 110 +++++++++++++++++++++++++++----------------
drivers/net/phy/phy-c45.c | 33 +++++++++++++
drivers/net/phy/phy-core.c | 43 +++++++++++++++++
include/linux/phy.h | 3 ++
4 files changed, 148 insertions(+), 41 deletions(-)
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up
^ permalink raw reply
* Re: [PATCH net-next v5 0/5] Introduce NETIF_F_GRO_HW
From: Sabrina Dubroca @ 2017-12-29 12:43 UTC (permalink / raw)
To: Alexander Duyck; +Cc: Michael Chan, David Miller, Netdev, Andrew Gospodarek
In-Reply-To: <CAKgT0UexohK4Mup928VqLDfzuafB-geqhbHA_Y9dJL3Yo4goKw@mail.gmail.com>
2017-12-22, 10:14:32 -0800, Alexander Duyck wrote:
> On Fri, Dec 22, 2017 at 6:57 AM, Sabrina Dubroca <sd@queasysnail.net> wrote:
> > IIUC, with the patches that were applied, each driver can define
> > whether GRO_HW depends on GRO? From a user's perspective, this
> > inconsistent behavior is going to be quite confusing.
> >
> > Worse than inconsistent behavior, it looks like a driver deciding that
> > GRO_HW doesn't depend on GRO is going to introduce a change of
> > behavior. Previously, when GRO was disabled, there wouldn't be any
> > packet over MTU handed to the network stack. Now, even if GRO is
> > disabled, GRO_HW might still be enabled, so we might get over-MTU
> > packets because of hardware GRO.
>
> This isn't actually true. LRO was still handling packets larger than
> MTU over even when GRO was disabled.
Sure, LRO will also cause that, but we're speaking in the context of
GRO here, which means no LRO.
> > I don't think drivers should be allowed to say "GRO_HW doesn't depend
> > on GRO".
>
> Why not, it doesn't. In my mind GRO_HW is closer to LRO than it is to
> GRO.
Why do you say that? It looks like GRO to me. These drivers are
calling tcp_gro_complete() for example.
> The only ugly bit as I see it is that these devices were exposing
> the feature via the GRO flag in the first place. So for the sake of
> legacy they might want to carry around the dependency.
>
> > I think it's reasonable to be able to disable software GRO even if
> > hardware GRO is enabled. Thus, I would propose:
> > - keep the current GRO flag
> > - add a GRO_HW flag, depending on GRO, enforced by the core as in
> > earlier versions of these patches
> > - add a GRO_SW flag, also depending on GRO
>
> This seems like a bunch of extra overhead for not much gain. Do we
> really need to fork GRO into 3 bits? I would argue that GRO_HW really
> should have been branded something like FORWARDABLE_LRO, but nobody
> wanted to touch the name LRO since it apparently has some negative
> stigma to it. If we had used a name like that we probably wouldn't be
> going through all these extra hoops. The only real reason why this is
> even being associated with GRO in the first place is that is how this
> feature was hidden by the drivers so they got around having to deal
> with the LRO being disabled for routing/forwarding issue. Those are
> the parts that want to keep it associated with GRO since that is how
> they exposed it in their devices originally.
I think it wouldn't have hidden behind GRO if it wasn't GRO. Again,
why do you think it's not GRO?
--
Sabrina
^ permalink raw reply
* [PATCH net-next 2/2] net: dsa: lan9303: Adjust phy_addr_base expressions
From: Egil Hjelmeland @ 2017-12-29 12:38 UTC (permalink / raw)
To: andrew, vivien.didelot, f.fainelli, netdev, linux-kernel; +Cc: Egil Hjelmeland
In-Reply-To: <20171229123824.28452-1-privat@egil-hjelmeland.no>
Simplify calculation of chip->phy_addr_base in lan9303_detect_phy_setup().
Use GENMASK to calculate phys_mii_mask from LAN9303_NUM_PORTS and
phy_addr_base.
Signed-off-by: Egil Hjelmeland <privat@egil-hjelmeland.no>
---
drivers/net/dsa/lan9303-core.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/net/dsa/lan9303-core.c b/drivers/net/dsa/lan9303-core.c
index 3088cdc5d205..4efb772dbc7e 100644
--- a/drivers/net/dsa/lan9303-core.c
+++ b/drivers/net/dsa/lan9303-core.c
@@ -495,10 +495,7 @@ static int lan9303_detect_phy_setup(struct lan9303 *chip)
return reg;
}
- if ((reg != 0) && (reg != 0xffff))
- chip->phy_addr_base = 1;
- else
- chip->phy_addr_base = 0;
+ chip->phy_addr_base = reg != 0 && reg != 0xffff;
dev_dbg(chip->dev, "Phy setup '%s' detected\n",
chip->phy_addr_base ? "1-2-3" : "0-1-2");
@@ -1283,13 +1280,16 @@ static const struct dsa_switch_ops lan9303_switch_ops = {
static int lan9303_register_switch(struct lan9303 *chip)
{
+ int base;
+
chip->ds = dsa_switch_alloc(chip->dev, LAN9303_NUM_PORTS);
if (!chip->ds)
return -ENOMEM;
chip->ds->priv = chip;
chip->ds->ops = &lan9303_switch_ops;
- chip->ds->phys_mii_mask = chip->phy_addr_base ? 0xe : 0x7;
+ base = chip->phy_addr_base;
+ chip->ds->phys_mii_mask = GENMASK(LAN9303_NUM_PORTS - 1 + base, base);
return dsa_register_switch(chip->ds);
}
--
2.14.1
^ permalink raw reply related
* [PATCH net-next 1/2] net: dsa: lan9303: phy_addr_sel_strap rename and retype
From: Egil Hjelmeland @ 2017-12-29 12:38 UTC (permalink / raw)
To: andrew, vivien.didelot, f.fainelli, netdev, linux-kernel; +Cc: Egil Hjelmeland
In-Reply-To: <20171229123824.28452-1-privat@egil-hjelmeland.no>
chip->phy_addr_sel_strap is declared as a bool, but is also used as an
integer address base.
Rename 'phy_addr_sel_strap' to 'phy_addr_base', and change type to int.
Signed-off-by: Egil Hjelmeland <privat@egil-hjelmeland.no>
---
drivers/net/dsa/lan9303-core.c | 20 ++++++++++----------
include/linux/dsa/lan9303.h | 2 +-
2 files changed, 11 insertions(+), 11 deletions(-)
diff --git a/drivers/net/dsa/lan9303-core.c b/drivers/net/dsa/lan9303-core.c
index 944901f03f8b..3088cdc5d205 100644
--- a/drivers/net/dsa/lan9303-core.c
+++ b/drivers/net/dsa/lan9303-core.c
@@ -479,7 +479,8 @@ static int lan9303_detect_phy_setup(struct lan9303 *chip)
{
int reg;
- /* depending on the 'phy_addr_sel_strap' setting, the three phys are
+ /* Calculate chip->phy_addr_base:
+ * Depending on the 'phy_addr_sel_strap' setting, the three phys are
* using IDs 0-1-2 or IDs 1-2-3. We cannot read back the
* 'phy_addr_sel_strap' setting directly, so we need a test, which
* configuration is active:
@@ -495,12 +496,12 @@ static int lan9303_detect_phy_setup(struct lan9303 *chip)
}
if ((reg != 0) && (reg != 0xffff))
- chip->phy_addr_sel_strap = 1;
+ chip->phy_addr_base = 1;
else
- chip->phy_addr_sel_strap = 0;
+ chip->phy_addr_base = 0;
dev_dbg(chip->dev, "Phy setup '%s' detected\n",
- chip->phy_addr_sel_strap ? "1-2-3" : "0-1-2");
+ chip->phy_addr_base ? "1-2-3" : "0-1-2");
return 0;
}
@@ -1019,7 +1020,7 @@ static int lan9303_get_sset_count(struct dsa_switch *ds)
static int lan9303_phy_read(struct dsa_switch *ds, int phy, int regnum)
{
struct lan9303 *chip = ds->priv;
- int phy_base = chip->phy_addr_sel_strap;
+ int phy_base = chip->phy_addr_base;
if (phy == phy_base)
return lan9303_virt_phy_reg_read(chip, regnum);
@@ -1033,7 +1034,7 @@ static int lan9303_phy_write(struct dsa_switch *ds, int phy, int regnum,
u16 val)
{
struct lan9303 *chip = ds->priv;
- int phy_base = chip->phy_addr_sel_strap;
+ int phy_base = chip->phy_addr_base;
if (phy == phy_base)
return lan9303_virt_phy_reg_write(chip, regnum, val);
@@ -1070,7 +1071,7 @@ static void lan9303_adjust_link(struct dsa_switch *ds, int port,
res = lan9303_phy_write(ds, port, MII_BMCR, ctl);
- if (port == chip->phy_addr_sel_strap) {
+ if (port == chip->phy_addr_base) {
/* Virtual Phy: Remove Turbo 200Mbit mode */
lan9303_read(chip->regmap, LAN9303_VIRT_SPECIAL_CTRL, &ctl);
@@ -1094,8 +1095,7 @@ static void lan9303_port_disable(struct dsa_switch *ds, int port,
struct lan9303 *chip = ds->priv;
lan9303_disable_processing_port(chip, port);
- lan9303_phy_write(ds, chip->phy_addr_sel_strap + port,
- MII_BMCR, BMCR_PDOWN);
+ lan9303_phy_write(ds, chip->phy_addr_base + port, MII_BMCR, BMCR_PDOWN);
}
static int lan9303_port_bridge_join(struct dsa_switch *ds, int port,
@@ -1289,7 +1289,7 @@ static int lan9303_register_switch(struct lan9303 *chip)
chip->ds->priv = chip;
chip->ds->ops = &lan9303_switch_ops;
- chip->ds->phys_mii_mask = chip->phy_addr_sel_strap ? 0xe : 0x7;
+ chip->ds->phys_mii_mask = chip->phy_addr_base ? 0xe : 0x7;
return dsa_register_switch(chip->ds);
}
diff --git a/include/linux/dsa/lan9303.h b/include/linux/dsa/lan9303.h
index b6514c29563f..b4f22112ba75 100644
--- a/include/linux/dsa/lan9303.h
+++ b/include/linux/dsa/lan9303.h
@@ -23,7 +23,7 @@ struct lan9303 {
struct regmap_irq_chip_data *irq_data;
struct gpio_desc *reset_gpio;
u32 reset_duration; /* in [ms] */
- bool phy_addr_sel_strap;
+ int phy_addr_base;
struct dsa_switch *ds;
struct mutex indirect_mutex; /* protect indexed register access */
struct mutex alr_mutex; /* protect ALR access */
--
2.14.1
^ permalink raw reply related
* [PATCH net-next 0/2] net: dsa: lan9303: phy_addr_sel_strap rename and retype
From: Egil Hjelmeland @ 2017-12-29 12:38 UTC (permalink / raw)
To: andrew, vivien.didelot, f.fainelli, netdev, linux-kernel; +Cc: Egil Hjelmeland
Non functional cleanups involving chip->phy_addr_sel_strap.
As promised in https://lkml.org/lkml/2017/11/6/273
Egil Hjelmeland (2):
net: dsa: lan9303: phy_addr_sel_strap rename and retype
net: dsa: lan9303: Adjust phy_addr_base expressions
drivers/net/dsa/lan9303-core.c | 24 ++++++++++++------------
include/linux/dsa/lan9303.h | 2 +-
2 files changed, 13 insertions(+), 13 deletions(-)
--
2.14.1
^ permalink raw reply
* Re: [PATCH bpf-next v3 3/3] libbpf: add missing SPDX-License-Identifier
From: Philippe Ombredanne @ 2017-12-29 12:35 UTC (permalink / raw)
To: Eric Leblond; +Cc: daniel, Alexei Starovoitov, netdev, LKML
In-Reply-To: <20171228080437.16933-4-eric@regit.org>
Eric,
On Thu, Dec 28, 2017 at 9:04 AM, Eric Leblond <eric@regit.org> wrote:
> Signed-off-by: Eric Leblond <eric@regit.org>
> Acked-by: Alexei Starovoitov <ast@kernel.org>
> ---
> tools/lib/bpf/bpf.c | 2 ++
> tools/lib/bpf/bpf.h | 2 ++
> tools/lib/bpf/libbpf.c | 2 ++
> tools/lib/bpf/libbpf.h | 2 ++
> 4 files changed, 8 insertions(+)
>
> diff --git a/tools/lib/bpf/bpf.c b/tools/lib/bpf/bpf.c
> index cdfabbe118cc..9e53dbbca2bd 100644
> --- a/tools/lib/bpf/bpf.c
> +++ b/tools/lib/bpf/bpf.c
> @@ -1,3 +1,5 @@
> +/* SPDX-License-Identifier: LGPL-2.1 */
In a .c file these should using C++-style comments as in //.
As requested by Linus, discussed on list and documented in Thomas doc patches.
> +
> /*
> * common eBPF ELF operations.
> *
> diff --git a/tools/lib/bpf/bpf.h b/tools/lib/bpf/bpf.h
> index 9f44c196931e..8d18fb73d7fb 100644
> --- a/tools/lib/bpf/bpf.h
> +++ b/tools/lib/bpf/bpf.h
> @@ -1,3 +1,5 @@
> +/* SPDX-License-Identifier: LGPL-2.1 */
And this is correct for a .h
> +
> /*
> * common eBPF ELF operations.
> *
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 5fe8aaa2123e..878e240a681b 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -1,3 +1,5 @@
> +/* SPDX-License-Identifier: LGPL-2.1 */
Use // here.
> +
> /*
> * Common eBPF ELF object loading operations.
> *
> diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
> index e42f96900318..f85906533cdd 100644
> --- a/tools/lib/bpf/libbpf.h
> +++ b/tools/lib/bpf/libbpf.h
> @@ -1,3 +1,5 @@
> +/* SPDX-License-Identifier: LGPL-2.1 */
> +
> /*
> * Common eBPF ELF object loading operations.
> *
> --
> 2.15.1
>
--
Cordially
Philippe Ombredanne
^ permalink raw reply
* Re: [PATCH net-next 2/2] tuntap: XDP transmission
From: Jesper Dangaard Brouer @ 2017-12-29 12:32 UTC (permalink / raw)
To: Jason Wang; +Cc: brouer, netdev, linux-kernel, mst
In-Reply-To: <1514541604-12728-3-git-send-email-jasowang@redhat.com>
On Fri, 29 Dec 2017 18:00:04 +0800
Jason Wang <jasowang@redhat.com> wrote:
> This patch implements XDP transmission for TAP. Since we can't create
> new queues for TAP during XDP set, exist ptr_ring was reused for
> queuing XDP buffers. To differ xdp_buff from sk_buff, TUN_XDP_FLAG
> (0x1ULL) was encoded into lowest bit of xpd_buff pointer during
> ptr_ring_produce, and was decoded during consuming. XDP metadata was
> stored in the headroom of the packet which should work in most of
> cases since driver usually reserve enough headroom. Very minor changes
> were done for vhost_net: it just need to peek the length depends on
> the type of pointer.
>
> Tests was done on two Intel E5-2630 2.40GHz machines connected back to
> back through two 82599ES. Traffic were generated through MoonGen and
> testpmd(rxonly) in guest reports 2.97Mpps when xdp_redirect_map is
> doing redirection from ixgbe to TAP.
IMHO a performance measurement without something to compare against is
useless. What was the performance before?
> Cc: Jesper Dangaard Brouer <brouer@redhat.com>
> Signed-off-by: Jason Wang <jasowang@redhat.com>
> ---
> drivers/net/tun.c | 205 ++++++++++++++++++++++++++++++++++++++++---------
> drivers/vhost/net.c | 13 +++-
> include/linux/if_tun.h | 17 ++++
> 3 files changed, 197 insertions(+), 38 deletions(-)
>
> diff --git a/drivers/net/tun.c b/drivers/net/tun.c
> index 2c89efe..be6d993 100644
> --- a/drivers/net/tun.c
> +++ b/drivers/net/tun.c
> @@ -240,6 +240,24 @@ struct tun_struct {
> struct tun_steering_prog __rcu *steering_prog;
> };
>
> +bool tun_is_xdp_buff(void *ptr)
> +{
> + return (unsigned long)ptr & TUN_XDP_FLAG;
> +}
> +EXPORT_SYMBOL(tun_is_xdp_buff);
> +
> +void *tun_xdp_to_ptr(void *ptr)
> +{
> + return (void *)((unsigned long)ptr | TUN_XDP_FLAG);
> +}
> +EXPORT_SYMBOL(tun_xdp_to_ptr);
> +
> +void *tun_ptr_to_xdp(void *ptr)
> +{
> + return (void *)((unsigned long)ptr & ~TUN_XDP_FLAG);
> +}
> +EXPORT_SYMBOL(tun_ptr_to_xdp);
> +
> static int tun_napi_receive(struct napi_struct *napi, int budget)
> {
> struct tun_file *tfile = container_of(napi, struct tun_file, napi);
> @@ -630,12 +648,25 @@ static struct tun_struct *tun_enable_queue(struct tun_file *tfile)
> return tun;
> }
>
> +static void tun_ptr_free(void *ptr)
> +{
> + if (!ptr)
> + return;
> + if (tun_is_xdp_buff(ptr)) {
> + struct xdp_buff *xdp = tun_ptr_to_xdp(ptr);
> +
> + put_page(virt_to_head_page(xdp->data));
(Yet another XDP-free call point, I need to convert to use an accessor
later to transition driver into an xdp_buff return API)
> + } else {
> + __skb_array_destroy_skb(ptr);
> + }
> +}
> +
> static void tun_queue_purge(struct tun_file *tfile)
> {
> - struct sk_buff *skb;
> + void *ptr;
>
> - while ((skb = ptr_ring_consume(&tfile->tx_ring)) != NULL)
> - kfree_skb(skb);
> + while ((ptr = ptr_ring_consume(&tfile->tx_ring)) != NULL)
> + tun_ptr_free(ptr);
>
> skb_queue_purge(&tfile->sk.sk_write_queue);
> skb_queue_purge(&tfile->sk.sk_error_queue);
> @@ -688,8 +719,7 @@ static void __tun_detach(struct tun_file *tfile, bool clean)
> unregister_netdevice(tun->dev);
> }
> if (tun)
> - ptr_ring_cleanup(&tfile->tx_ring,
> - __skb_array_destroy_skb);
> + ptr_ring_cleanup(&tfile->tx_ring, tun_ptr_free);
> sock_put(&tfile->sk);
> }
> }
> @@ -1201,6 +1231,54 @@ static const struct net_device_ops tun_netdev_ops = {
> .ndo_get_stats64 = tun_net_get_stats64,
> };
>
> +static int tun_xdp_xmit(struct net_device *dev, struct xdp_buff *xdp)
> +{
> + struct tun_struct *tun = netdev_priv(dev);
> + struct xdp_buff *buff = xdp->data_hard_start;
> + int headroom = xdp->data - xdp->data_hard_start;
> + struct tun_file *tfile;
> + u32 numqueues;
> + int ret = 0;
> +
> + /* Assure headroom is available and buff is properly aligned */
> + if (unlikely(headroom < sizeof(*xdp) || tun_is_xdp_buff(xdp)))
> + return -ENOSPC;
> +
> + *buff = *xdp;
> +
> + rcu_read_lock();
> +
> + numqueues = READ_ONCE(tun->numqueues);
> + if (!numqueues) {
> + ret = -ENOSPC;
> + goto out;
> + }
> + tfile = rcu_dereference(tun->tfiles[smp_processor_id() %
> + numqueues]);
Several concurrent CPUs can get the same 'tfile'.
> + /* Encode the XDP flag into lowest bit for consumer to differ
> + * XDP buffer from sk_buff.
> + */
> + if (ptr_ring_produce(&tfile->tx_ring, tun_xdp_to_ptr(buff))) {
> + this_cpu_inc(tun->pcpu_stats->tx_dropped);
> + ret = -ENOSPC;
> + }
The ptr_ring_produce() will take a lock per packet, limiting the
performance. (Again a case where I would have liked a bulk API for
ndo_xdp_xmit()).
> +
> +out:
> + rcu_read_unlock();
> + return ret;
> +}
> +
> +static void tun_xdp_flush(struct net_device *dev)
> +{
> + struct tun_struct *tun = netdev_priv(dev);
> + struct tun_file *tfile = tun->tfiles[0];
> +
> + /* Notify and wake up reader process */
> + if (tfile->flags & TUN_FASYNC)
> + kill_fasync(&tfile->fasync, SIGIO, POLL_IN);
> + tfile->socket.sk->sk_data_ready(tfile->socket.sk);
> +}
> +
> static const struct net_device_ops tap_netdev_ops = {
> .ndo_uninit = tun_net_uninit,
> .ndo_open = tun_net_open,
> @@ -1218,6 +1296,8 @@ static const struct net_device_ops tap_netdev_ops = {
> .ndo_set_rx_headroom = tun_set_headroom,
> .ndo_get_stats64 = tun_net_get_stats64,
> .ndo_bpf = tun_xdp,
> + .ndo_xdp_xmit = tun_xdp_xmit,
> + .ndo_xdp_flush = tun_xdp_flush,
> };
>
> static void tun_flow_init(struct tun_struct *tun)
> @@ -1841,6 +1921,40 @@ static ssize_t tun_chr_write_iter(struct kiocb *iocb, struct iov_iter *from)
> return result;
> }
>
> +static ssize_t tun_put_user_xdp(struct tun_struct *tun,
> + struct tun_file *tfile,
> + struct xdp_buff *xdp,
> + struct iov_iter *iter)
> +{
> + int vnet_hdr_sz = 0;
> + size_t size = xdp->data_end - xdp->data;
> + struct tun_pcpu_stats *stats;
> + size_t ret;
> +
> + if (tun->flags & IFF_VNET_HDR) {
> + struct virtio_net_hdr gso = { 0 };
> +
> + vnet_hdr_sz = READ_ONCE(tun->vnet_hdr_sz);
> + if (unlikely(iov_iter_count(iter) < vnet_hdr_sz))
> + return -EINVAL;
> + if (unlikely(copy_to_iter(&gso, sizeof(gso), iter) !=
> + sizeof(gso)))
> + return -EFAULT;
> + iov_iter_advance(iter, vnet_hdr_sz - sizeof(gso));
> + }
> +
> + ret = copy_to_iter(xdp->data, size, iter) + vnet_hdr_sz;
> +
> + stats = get_cpu_ptr(tun->pcpu_stats);
> + u64_stats_update_begin(&stats->syncp);
> + stats->tx_packets++;
> + stats->tx_bytes += ret;
> + u64_stats_update_end(&stats->syncp);
> + put_cpu_ptr(tun->pcpu_stats);
> +
> + return ret;
> +}
> +
> /* Put packet to the user space buffer */
> static ssize_t tun_put_user(struct tun_struct *tun,
> struct tun_file *tfile,
[...]
> error = -ERESTARTSYS;
> @@ -1977,36 +2090,42 @@ static struct sk_buff *tun_ring_recv(struct tun_file *tfile, int noblock,
>
> out:
> *err = error;
> - return skb;
> + return ptr;
> }
>
> static ssize_t tun_do_read(struct tun_struct *tun, struct tun_file *tfile,
> struct iov_iter *to,
> - int noblock, struct sk_buff *skb)
> + int noblock, void *ptr)
> {
> ssize_t ret;
> int err;
>
> tun_debug(KERN_INFO, tun, "tun_do_read\n");
>
> - if (!iov_iter_count(to)) {
> - if (skb)
> - kfree_skb(skb);
> - return 0;
> - }
> + if (!iov_iter_count(to))
> + tun_ptr_free(ptr);
>
> - if (!skb) {
> + if (!ptr) {
> /* Read frames from ring */
> - skb = tun_ring_recv(tfile, noblock, &err);
> - if (!skb)
> + ptr = tun_ring_recv(tfile, noblock, &err);
> + if (!ptr)
> return err;
> }
>
> - ret = tun_put_user(tun, tfile, skb, to);
> - if (unlikely(ret < 0))
> - kfree_skb(skb);
> - else
> - consume_skb(skb);
> + if (tun_is_xdp_buff(ptr)) {
> + struct xdp_buff *xdp = tun_ptr_to_xdp(ptr);
> +
> + ret = tun_put_user_xdp(tun, tfile, xdp, to);
> + put_page(virt_to_head_page(xdp->data));
This again tie us info a page-refcnt based XDP scheme.
> + } else {
> + struct sk_buff *skb = ptr;
> +
> + ret = tun_put_user(tun, tfile, skb, to);
> + if (unlikely(ret < 0))
> + kfree_skb(skb);
> + else
> + consume_skb(skb);
> + }
>
> return ret;
> }
[...]
> @@ -3191,8 +3323,7 @@ struct socket *tun_get_socket(struct file *file)
> struct tun_file *tfile;
> if (file->f_op != &tun_fops)
> return ERR_PTR(-EINVAL);
> - tfile = file->private_data;
> - if (!tfile)
> + tfile = file->private_data; if (!tfile)
This change looks like a typo...
> return ERR_PTR(-EBADFD);
> return &tfile->socket;
> }
--
Best regards,
Jesper Dangaard Brouer
MSc.CS, Principal Kernel Engineer at Red Hat
LinkedIn: http://www.linkedin.com/in/brouer
^ permalink raw reply
* [PATCH net-next 7/7] net: phy: convert read-modify-write to phy_modify()
From: Russell King @ 2017-12-29 12:31 UTC (permalink / raw)
To: Andrew Lunn, Florian Fainelli; +Cc: netdev
In-Reply-To: <20171229123024.GZ10595@n2100.armlinux.org.uk>
Convert read-modify-write sequences in at803x, Marvell and core phylib
to use phy_modify() to ensure safety.
Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
drivers/net/phy/at803x.c | 20 +++--------
drivers/net/phy/marvell.c | 82 ++++++++++++++++----------------------------
drivers/net/phy/phy_device.c | 50 +++++----------------------
3 files changed, 43 insertions(+), 109 deletions(-)
diff --git a/drivers/net/phy/at803x.c b/drivers/net/phy/at803x.c
index e911e4990b20..e86c1b8b1b51 100644
--- a/drivers/net/phy/at803x.c
+++ b/drivers/net/phy/at803x.c
@@ -216,34 +216,22 @@ static int at803x_suspend(struct phy_device *phydev)
int value;
int wol_enabled;
- mutex_lock(&phydev->lock);
-
value = phy_read(phydev, AT803X_INTR_ENABLE);
wol_enabled = value & AT803X_INTR_ENABLE_WOL;
- value = phy_read(phydev, MII_BMCR);
-
if (wol_enabled)
- value |= BMCR_ISOLATE;
+ value = BMCR_ISOLATE;
else
- value |= BMCR_PDOWN;
+ value = BMCR_PDOWN;
- phy_write(phydev, MII_BMCR, value);
-
- mutex_unlock(&phydev->lock);
+ phy_modify(phydev, MII_BMCR, 0, value);
return 0;
}
static int at803x_resume(struct phy_device *phydev)
{
- int value;
-
- value = phy_read(phydev, MII_BMCR);
- value &= ~(BMCR_PDOWN | BMCR_ISOLATE);
- phy_write(phydev, MII_BMCR, value);
-
- return 0;
+ return phy_modify(phydev, MII_BMCR, ~(BMCR_PDOWN | BMCR_ISOLATE), 0);
}
static int at803x_probe(struct phy_device *phydev)
diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c
index 1cb84064d658..6129ab1000f9 100644
--- a/drivers/net/phy/marvell.c
+++ b/drivers/net/phy/marvell.c
@@ -664,19 +664,14 @@ static int m88e1116r_config_init(struct phy_device *phydev)
static int m88e3016_config_init(struct phy_device *phydev)
{
- int reg;
+ int ret;
/* Enable Scrambler and Auto-Crossover */
- reg = phy_read(phydev, MII_88E3016_PHY_SPEC_CTRL);
- if (reg < 0)
- return reg;
-
- reg &= ~MII_88E3016_DISABLE_SCRAMBLER;
- reg |= MII_88E3016_AUTO_MDIX_CROSSOVER;
-
- reg = phy_write(phydev, MII_88E3016_PHY_SPEC_CTRL, reg);
- if (reg < 0)
- return reg;
+ ret = phy_modify(phydev, MII_88E3016_PHY_SPEC_CTRL,
+ ~MII_88E3016_DISABLE_SCRAMBLER,
+ MII_88E3016_AUTO_MDIX_CROSSOVER);
+ if (ret < 0)
+ return ret;
return marvell_config_init(phydev);
}
@@ -685,42 +680,34 @@ static int m88e1111_config_init_hwcfg_mode(struct phy_device *phydev,
u16 mode,
int fibre_copper_auto)
{
- int temp;
-
- temp = phy_read(phydev, MII_M1111_PHY_EXT_SR);
- if (temp < 0)
- return temp;
-
- temp &= ~(MII_M1111_HWCFG_MODE_MASK |
- MII_M1111_HWCFG_FIBER_COPPER_AUTO |
- MII_M1111_HWCFG_FIBER_COPPER_RES);
- temp |= mode;
-
if (fibre_copper_auto)
- temp |= MII_M1111_HWCFG_FIBER_COPPER_AUTO;
+ mode |= MII_M1111_HWCFG_FIBER_COPPER_AUTO;
- return phy_write(phydev, MII_M1111_PHY_EXT_SR, temp);
+ return phy_modify(phydev, MII_M1111_PHY_EXT_SR,
+ (u16)~(MII_M1111_HWCFG_MODE_MASK |
+ MII_M1111_HWCFG_FIBER_COPPER_AUTO |
+ MII_M1111_HWCFG_FIBER_COPPER_RES),
+ mode);
}
static int m88e1111_config_init_rgmii_delays(struct phy_device *phydev)
{
- int temp;
-
- temp = phy_read(phydev, MII_M1111_PHY_EXT_CR);
- if (temp < 0)
- return temp;
+ int delay;
if (phydev->interface == PHY_INTERFACE_MODE_RGMII_ID) {
- temp |= (MII_M1111_RGMII_RX_DELAY | MII_M1111_RGMII_TX_DELAY);
+ delay = MII_M1111_RGMII_RX_DELAY | MII_M1111_RGMII_TX_DELAY;
} else if (phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID) {
- temp &= ~MII_M1111_RGMII_TX_DELAY;
- temp |= MII_M1111_RGMII_RX_DELAY;
+ delay = MII_M1111_RGMII_RX_DELAY;
} else if (phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID) {
- temp &= ~MII_M1111_RGMII_RX_DELAY;
- temp |= MII_M1111_RGMII_TX_DELAY;
+ delay = MII_M1111_RGMII_TX_DELAY;
+ } else {
+ delay = 0;
}
- return phy_write(phydev, MII_M1111_PHY_EXT_CR, temp);
+ return phy_modify(phydev, MII_M1111_PHY_EXT_CR,
+ (u16)~(MII_M1111_RGMII_RX_DELAY |
+ MII_M1111_RGMII_TX_DELAY),
+ delay);
}
static int m88e1111_config_init_rgmii(struct phy_device *phydev)
@@ -834,7 +821,6 @@ static int m88e1121_config_init(struct phy_device *phydev)
static int m88e1510_config_init(struct phy_device *phydev)
{
int err;
- int temp;
/* SGMII-to-Copper mode initialization */
if (phydev->interface == PHY_INTERFACE_MODE_SGMII) {
@@ -846,16 +832,15 @@ static int m88e1510_config_init(struct phy_device *phydev)
return err;
/* In reg 20, write MODE[2:0] = 0x1 (SGMII to Copper) */
- temp = phy_read(phydev, MII_88E1510_GEN_CTRL_REG_1);
- temp &= ~MII_88E1510_GEN_CTRL_REG_1_MODE_MASK;
- temp |= MII_88E1510_GEN_CTRL_REG_1_MODE_SGMII;
- err = phy_write(phydev, MII_88E1510_GEN_CTRL_REG_1, temp);
+ err = phy_modify(phydev, MII_88E1510_GEN_CTRL_REG_1,
+ ~MII_88E1510_GEN_CTRL_REG_1_MODE_MASK,
+ MII_88E1510_GEN_CTRL_REG_1_MODE_SGMII);
if (err < 0)
return err;
/* PHY reset is necessary after changing MODE[2:0] */
- temp |= MII_88E1510_GEN_CTRL_REG_1_RESET;
- err = phy_write(phydev, MII_88E1510_GEN_CTRL_REG_1, temp);
+ err = phy_modify(phydev, MII_88E1510_GEN_CTRL_REG_1, 0,
+ MII_88E1510_GEN_CTRL_REG_1_RESET);
if (err < 0)
return err;
@@ -961,7 +946,6 @@ static int m88e1149_config_init(struct phy_device *phydev)
static int m88e1145_config_init_rgmii(struct phy_device *phydev)
{
- int temp;
int err;
err = m88e1111_config_init_rgmii_delays(phydev);
@@ -973,15 +957,9 @@ static int m88e1145_config_init_rgmii(struct phy_device *phydev)
if (err < 0)
return err;
- temp = phy_read(phydev, 0x1e);
- if (temp < 0)
- return temp;
-
- temp &= 0xf03f;
- temp |= 2 << 9; /* 36 ohm */
- temp |= 2 << 6; /* 39 ohm */
-
- err = phy_write(phydev, 0x1e, temp);
+ err = phy_modify(phydev, 0x1e, 0xf03f,
+ 2 << 9 | /* 36 ohm */
+ 2 << 6); /* 39 ohm */
if (err < 0)
return err;
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index b15b31ca2618..e5ddc5ae56d1 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -1328,9 +1328,8 @@ static int genphy_config_eee_advert(struct phy_device *phydev)
*/
int genphy_setup_forced(struct phy_device *phydev)
{
- int ctl = phy_read(phydev, MII_BMCR);
+ u16 ctl = 0;
- ctl &= BMCR_LOOPBACK | BMCR_ISOLATE | BMCR_PDOWN;
phydev->pause = 0;
phydev->asym_pause = 0;
@@ -1342,7 +1341,8 @@ int genphy_setup_forced(struct phy_device *phydev)
if (DUPLEX_FULL == phydev->duplex)
ctl |= BMCR_FULLDPLX;
- return phy_write(phydev, MII_BMCR, ctl);
+ return phy_modify(phydev, MII_BMCR,
+ BMCR_LOOPBACK | BMCR_ISOLATE | BMCR_PDOWN, ctl);
}
EXPORT_SYMBOL(genphy_setup_forced);
@@ -1352,17 +1352,9 @@ EXPORT_SYMBOL(genphy_setup_forced);
*/
int genphy_restart_aneg(struct phy_device *phydev)
{
- int ctl = phy_read(phydev, MII_BMCR);
-
- if (ctl < 0)
- return ctl;
-
- ctl |= BMCR_ANENABLE | BMCR_ANRESTART;
-
/* Don't isolate the PHY if we're negotiating */
- ctl &= ~BMCR_ISOLATE;
-
- return phy_write(phydev, MII_BMCR, ctl);
+ return phy_modify(phydev, MII_BMCR, ~BMCR_ISOLATE,
+ BMCR_ANENABLE | BMCR_ANRESTART);
}
EXPORT_SYMBOL(genphy_restart_aneg);
@@ -1628,44 +1620,20 @@ EXPORT_SYMBOL(genphy_config_init);
int genphy_suspend(struct phy_device *phydev)
{
- int value;
-
- mutex_lock(&phydev->lock);
-
- value = phy_read(phydev, MII_BMCR);
- phy_write(phydev, MII_BMCR, value | BMCR_PDOWN);
-
- mutex_unlock(&phydev->lock);
-
- return 0;
+ return phy_modify(phydev, MII_BMCR, 0, BMCR_PDOWN);
}
EXPORT_SYMBOL(genphy_suspend);
int genphy_resume(struct phy_device *phydev)
{
- int value;
-
- value = phy_read(phydev, MII_BMCR);
- phy_write(phydev, MII_BMCR, value & ~BMCR_PDOWN);
-
- return 0;
+ return phy_modify(phydev, MII_BMCR, ~BMCR_PDOWN, 0);
}
EXPORT_SYMBOL(genphy_resume);
int genphy_loopback(struct phy_device *phydev, bool enable)
{
- int value;
-
- value = phy_read(phydev, MII_BMCR);
- if (value < 0)
- return value;
-
- if (enable)
- value |= BMCR_LOOPBACK;
- else
- value &= ~BMCR_LOOPBACK;
-
- return phy_write(phydev, MII_BMCR, value);
+ return phy_modify(phydev, MII_BMCR, ~BMCR_LOOPBACK,
+ enable ? BMCR_LOOPBACK : 0);
}
EXPORT_SYMBOL(genphy_loopback);
--
2.7.4
^ permalink raw reply related
* [PATCH net-next 6/7] net: phy: add phy_modify() accessor
From: Russell King @ 2017-12-29 12:31 UTC (permalink / raw)
To: Andrew Lunn, Florian Fainelli; +Cc: netdev
In-Reply-To: <20171229123024.GZ10595@n2100.armlinux.org.uk>
Add phy_modify() convenience accessor to complement the mdiobus
counterpart.
Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
drivers/net/phy/phy-core.c | 23 +++++++++++++++++++++++
include/linux/phy.h | 1 +
2 files changed, 24 insertions(+)
diff --git a/drivers/net/phy/phy-core.c b/drivers/net/phy/phy-core.c
index df39e6711b76..9c08850eed16 100644
--- a/drivers/net/phy/phy-core.c
+++ b/drivers/net/phy/phy-core.c
@@ -306,6 +306,29 @@ int __phy_modify(struct phy_device *phydev, u32 regnum, u16 mask, u16 set)
}
EXPORT_SYMBOL_GPL(__phy_modify);
+/**
+ * phy_modify - Convenience function for modifying a given PHY register
+ * @phydev: the phy_device struct
+ * @regnum: register number to write
+ * @mask: bit mask of bits to clear
+ * @set: new value of bits set in mask to write to @regnum
+ *
+ * NOTE: MUST NOT be called from interrupt context,
+ * because the bus read/write functions may wait for an interrupt
+ * to conclude the operation.
+ */
+int phy_modify(struct phy_device *phydev, u32 regnum, u16 mask, u16 set)
+{
+ int ret;
+
+ mutex_lock(&phydev->mdio.bus->mdio_lock);
+ ret = __phy_modify(phydev, regnum, mask, set);
+ mutex_unlock(&phydev->mdio.bus->mdio_lock);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(phy_modify);
+
static int __phy_read_page(struct phy_device *phydev)
{
return phydev->drv->read_page(phydev);
diff --git a/include/linux/phy.h b/include/linux/phy.h
index f408220d95ec..0ee4ece312da 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -760,6 +760,7 @@ static inline int __phy_write(struct phy_device *phydev, u32 regnum, u16 val)
}
int __phy_modify(struct phy_device *phydev, u32 regnum, u16 mask, u16 set);
+int phy_modify(struct phy_device *phydev, u32 regnum, u16 mask, u16 set);
/**
* phy_interrupt_is_valid - Convenience function for testing a given PHY irq
--
2.7.4
^ permalink raw reply related
* [PATCH net-next 5/7] net: phy: marvell: fix paged access races
From: Russell King @ 2017-12-29 12:31 UTC (permalink / raw)
To: Andrew Lunn, Florian Fainelli; +Cc: netdev
In-Reply-To: <20171229123024.GZ10595@n2100.armlinux.org.uk>
For paged accesses to be truely safe, we need to hold the bus lock to
prevent anyone else gaining access to the registers while we modify
them.
The phydev->lock mutex does not do this: userspace via the MII ioctl
can still sneak in and read or write any register while we are on a
different page, and the suspend/resume methods can be called by a
thread different to the thread polling the phy status.
Races have been observed with mvneta on SolidRun Clearfog with phylink,
particularly between the phylib worker reading the PHYs status, and
the thread resuming mvneta, calling phy_start() which then calls
through to m88e1121_config_aneg_rgmii_delays(), which tries to
read-modify-write the MSCR register:
CPU0 CPU1
marvell_read_status_page()
marvell_set_page(phydev, MII_MARVELL_FIBER_PAGE)
...
m88e1121_config_aneg_rgmii_delays()
set_page(MII_MARVELL_MSCR_PAGE)
phy_read(phydev, MII_88E1121_PHY_MSCR_REG)
marvell_set_page(phydev, MII_MARVELL_COPPER_PAGE);
...
phy_write(phydev, MII_88E1121_PHY_MSCR_REG)
The result of this is we end up writing the copper page register 21,
which causes the copper PHY to be disabled, and the link partner sees
the link immediately go down.
Solve this by taking the bus lock instead of the PHY lock, thereby
preventing other accesses to the PHY while we are accessing other PHY
pages.
Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
drivers/net/phy/marvell.c | 354 ++++++++++++++++++----------------------------
1 file changed, 137 insertions(+), 217 deletions(-)
diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c
index 82104edca393..1cb84064d658 100644
--- a/drivers/net/phy/marvell.c
+++ b/drivers/net/phy/marvell.c
@@ -83,7 +83,7 @@
#define MII_88E1121_PHY_MSCR_REG 21
#define MII_88E1121_PHY_MSCR_RX_DELAY BIT(5)
#define MII_88E1121_PHY_MSCR_TX_DELAY BIT(4)
-#define MII_88E1121_PHY_MSCR_DELAY_MASK (~(BIT(5) | BIT(4)))
+#define MII_88E1121_PHY_MSCR_DELAY_MASK (BIT(5) | BIT(4))
#define MII_88E1121_MISC_TEST 0x1a
#define MII_88E1510_MISC_TEST_TEMP_THRESHOLD_MASK 0x1f00
@@ -177,27 +177,19 @@ struct marvell_priv {
struct device *hwmon_dev;
};
-static int marvell_get_page(struct phy_device *phydev)
+static int marvell_read_page(struct phy_device *phydev)
{
- return phy_read(phydev, MII_MARVELL_PHY_PAGE);
+ return __phy_read(phydev, MII_MARVELL_PHY_PAGE);
}
-static int marvell_set_page(struct phy_device *phydev, int page)
+static int marvell_write_page(struct phy_device *phydev, int page)
{
- return phy_write(phydev, MII_MARVELL_PHY_PAGE, page);
+ return __phy_write(phydev, MII_MARVELL_PHY_PAGE, page);
}
-static int marvell_get_set_page(struct phy_device *phydev, int page)
+static int marvell_set_page(struct phy_device *phydev, int page)
{
- int oldpage = marvell_get_page(phydev);
-
- if (oldpage < 0)
- return oldpage;
-
- if (page != oldpage)
- return marvell_set_page(phydev, page);
-
- return 0;
+ return phy_write(phydev, MII_MARVELL_PHY_PAGE, page);
}
static int marvell_ack_interrupt(struct phy_device *phydev)
@@ -399,7 +391,7 @@ static int m88e1111_config_aneg(struct phy_device *phydev)
static int marvell_of_reg_init(struct phy_device *phydev)
{
const __be32 *paddr;
- int len, i, saved_page, current_page, ret;
+ int len, i, saved_page, current_page, ret = 0;
if (!phydev->mdio.dev.of_node)
return 0;
@@ -409,12 +401,11 @@ static int marvell_of_reg_init(struct phy_device *phydev)
if (!paddr || len < (4 * sizeof(*paddr)))
return 0;
- saved_page = marvell_get_page(phydev);
+ saved_page = phy_save_page(phydev);
if (saved_page < 0)
- return saved_page;
+ goto err;
current_page = saved_page;
- ret = 0;
len /= sizeof(*paddr);
for (i = 0; i < len - 3; i += 4) {
u16 page = be32_to_cpup(paddr + i);
@@ -425,14 +416,14 @@ static int marvell_of_reg_init(struct phy_device *phydev)
if (page != current_page) {
current_page = page;
- ret = marvell_set_page(phydev, page);
+ ret = marvell_write_page(phydev, page);
if (ret < 0)
goto err;
}
val = 0;
if (mask) {
- val = phy_read(phydev, reg);
+ val = __phy_read(phydev, reg);
if (val < 0) {
ret = val;
goto err;
@@ -441,17 +432,12 @@ static int marvell_of_reg_init(struct phy_device *phydev)
}
val |= val_bits;
- ret = phy_write(phydev, reg, val);
+ ret = __phy_write(phydev, reg, val);
if (ret < 0)
goto err;
}
err:
- if (current_page != saved_page) {
- i = marvell_set_page(phydev, saved_page);
- if (ret == 0)
- ret = i;
- }
- return ret;
+ return phy_restore_page(phydev, saved_page, ret);
}
#else
static int marvell_of_reg_init(struct phy_device *phydev)
@@ -462,34 +448,21 @@ static int marvell_of_reg_init(struct phy_device *phydev)
static int m88e1121_config_aneg_rgmii_delays(struct phy_device *phydev)
{
- int err, oldpage, mscr;
-
- oldpage = marvell_get_set_page(phydev, MII_MARVELL_MSCR_PAGE);
- if (oldpage < 0)
- return oldpage;
-
- mscr = phy_read(phydev, MII_88E1121_PHY_MSCR_REG);
- if (mscr < 0) {
- err = mscr;
- goto out;
- }
-
- mscr &= MII_88E1121_PHY_MSCR_DELAY_MASK;
+ int mscr;
if (phydev->interface == PHY_INTERFACE_MODE_RGMII_ID)
- mscr |= (MII_88E1121_PHY_MSCR_RX_DELAY |
- MII_88E1121_PHY_MSCR_TX_DELAY);
+ mscr = MII_88E1121_PHY_MSCR_RX_DELAY |
+ MII_88E1121_PHY_MSCR_TX_DELAY;
else if (phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID)
- mscr |= MII_88E1121_PHY_MSCR_RX_DELAY;
+ mscr = MII_88E1121_PHY_MSCR_RX_DELAY;
else if (phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID)
- mscr |= MII_88E1121_PHY_MSCR_TX_DELAY;
-
- err = phy_write(phydev, MII_88E1121_PHY_MSCR_REG, mscr);
-
-out:
- marvell_set_page(phydev, oldpage);
+ mscr = MII_88E1121_PHY_MSCR_TX_DELAY;
+ else
+ mscr = 0;
- return err;
+ return phy_modify_paged(phydev, MII_MARVELL_MSCR_PAGE,
+ MII_88E1121_PHY_MSCR_REG,
+ MII_88E1121_PHY_MSCR_DELAY_MASK, mscr);
}
static int m88e1121_config_aneg(struct phy_device *phydev)
@@ -515,20 +488,11 @@ static int m88e1121_config_aneg(struct phy_device *phydev)
static int m88e1318_config_aneg(struct phy_device *phydev)
{
- int err, oldpage, mscr;
-
- oldpage = marvell_get_set_page(phydev, MII_MARVELL_MSCR_PAGE);
- if (oldpage < 0)
- return oldpage;
-
- mscr = phy_read(phydev, MII_88E1318S_PHY_MSCR1_REG);
- mscr |= MII_88E1318S_PHY_MSCR1_PAD_ODD;
-
- err = phy_write(phydev, MII_88E1318S_PHY_MSCR1_REG, mscr);
- if (err < 0)
- return err;
+ int err;
- err = marvell_set_page(phydev, oldpage);
+ err = phy_modify_paged(phydev, MII_MARVELL_MSCR_PAGE,
+ MII_88E1318S_PHY_MSCR1_REG,
+ 0, MII_88E1318S_PHY_MSCR1_PAD_ODD);
if (err < 0)
return err;
@@ -854,20 +818,15 @@ static int m88e1111_config_init(struct phy_device *phydev)
static int m88e1121_config_init(struct phy_device *phydev)
{
- int err, oldpage;
-
- oldpage = marvell_get_set_page(phydev, MII_MARVELL_LED_PAGE);
- if (oldpage < 0)
- return oldpage;
+ int err;
/* Default PHY LED config: LED[0] .. Link, LED[1] .. Activity */
- err = phy_write(phydev, MII_88E1121_PHY_LED_CTRL,
- MII_88E1121_PHY_LED_DEF);
+ err = phy_write_paged(phydev, MII_MARVELL_LED_PAGE,
+ MII_88E1121_PHY_LED_CTRL,
+ MII_88E1121_PHY_LED_DEF);
if (err < 0)
return err;
- marvell_set_page(phydev, oldpage);
-
/* Set marvell,reg-init configuration from device tree */
return marvell_config_init(phydev);
}
@@ -1398,100 +1357,98 @@ static int m88e1121_did_interrupt(struct phy_device *phydev)
static void m88e1318_get_wol(struct phy_device *phydev,
struct ethtool_wolinfo *wol)
{
+ int oldpage, ret = 0;
+
wol->supported = WAKE_MAGIC;
wol->wolopts = 0;
- if (marvell_set_page(phydev, MII_MARVELL_WOL_PAGE) < 0)
- return;
+ oldpage = phy_select_page(phydev, MII_MARVELL_WOL_PAGE);
+ if (oldpage < 0)
+ goto error;
- if (phy_read(phydev, MII_88E1318S_PHY_WOL_CTRL) &
- MII_88E1318S_PHY_WOL_CTRL_MAGIC_PACKET_MATCH_ENABLE)
+ ret = __phy_read(phydev, MII_88E1318S_PHY_WOL_CTRL);
+ if (ret & MII_88E1318S_PHY_WOL_CTRL_MAGIC_PACKET_MATCH_ENABLE)
wol->wolopts |= WAKE_MAGIC;
- if (marvell_set_page(phydev, MII_MARVELL_COPPER_PAGE) < 0)
- return;
+error:
+ phy_restore_page(phydev, oldpage, ret);
}
static int m88e1318_set_wol(struct phy_device *phydev,
struct ethtool_wolinfo *wol)
{
- int err, oldpage, temp;
+ int err = 0, oldpage;
- oldpage = marvell_get_page(phydev);
+ oldpage = phy_save_page(phydev);
+ if (oldpage < 0)
+ goto error;
if (wol->wolopts & WAKE_MAGIC) {
/* Explicitly switch to page 0x00, just to be sure */
- err = marvell_set_page(phydev, MII_MARVELL_COPPER_PAGE);
+ err = marvell_write_page(phydev, MII_MARVELL_COPPER_PAGE);
if (err < 0)
- return err;
+ goto error;
/* Enable the WOL interrupt */
- temp = phy_read(phydev, MII_88E1318S_PHY_CSIER);
- temp |= MII_88E1318S_PHY_CSIER_WOL_EIE;
- err = phy_write(phydev, MII_88E1318S_PHY_CSIER, temp);
+ err = __phy_modify(phydev, MII_88E1318S_PHY_CSIER, 0,
+ MII_88E1318S_PHY_CSIER_WOL_EIE);
if (err < 0)
- return err;
+ goto error;
- err = marvell_set_page(phydev, MII_MARVELL_LED_PAGE);
+ err = marvell_write_page(phydev, MII_MARVELL_LED_PAGE);
if (err < 0)
- return err;
+ goto error;
/* Setup LED[2] as interrupt pin (active low) */
- temp = phy_read(phydev, MII_88E1318S_PHY_LED_TCR);
- temp &= ~MII_88E1318S_PHY_LED_TCR_FORCE_INT;
- temp |= MII_88E1318S_PHY_LED_TCR_INTn_ENABLE;
- temp |= MII_88E1318S_PHY_LED_TCR_INT_ACTIVE_LOW;
- err = phy_write(phydev, MII_88E1318S_PHY_LED_TCR, temp);
+ err = __phy_modify(phydev, MII_88E1318S_PHY_LED_TCR,
+ (u16)~MII_88E1318S_PHY_LED_TCR_FORCE_INT,
+ MII_88E1318S_PHY_LED_TCR_INTn_ENABLE |
+ MII_88E1318S_PHY_LED_TCR_INT_ACTIVE_LOW);
if (err < 0)
- return err;
+ goto error;
- err = marvell_set_page(phydev, MII_MARVELL_WOL_PAGE);
+ err = marvell_write_page(phydev, MII_MARVELL_WOL_PAGE);
if (err < 0)
- return err;
+ goto error;
/* Store the device address for the magic packet */
- err = phy_write(phydev, MII_88E1318S_PHY_MAGIC_PACKET_WORD2,
+ err = __phy_write(phydev, MII_88E1318S_PHY_MAGIC_PACKET_WORD2,
((phydev->attached_dev->dev_addr[5] << 8) |
phydev->attached_dev->dev_addr[4]));
if (err < 0)
- return err;
- err = phy_write(phydev, MII_88E1318S_PHY_MAGIC_PACKET_WORD1,
+ goto error;
+ err = __phy_write(phydev, MII_88E1318S_PHY_MAGIC_PACKET_WORD1,
((phydev->attached_dev->dev_addr[3] << 8) |
phydev->attached_dev->dev_addr[2]));
if (err < 0)
- return err;
- err = phy_write(phydev, MII_88E1318S_PHY_MAGIC_PACKET_WORD0,
+ goto error;
+ err = __phy_write(phydev, MII_88E1318S_PHY_MAGIC_PACKET_WORD0,
((phydev->attached_dev->dev_addr[1] << 8) |
phydev->attached_dev->dev_addr[0]));
if (err < 0)
- return err;
+ goto error;
/* Clear WOL status and enable magic packet matching */
- temp = phy_read(phydev, MII_88E1318S_PHY_WOL_CTRL);
- temp |= MII_88E1318S_PHY_WOL_CTRL_CLEAR_WOL_STATUS;
- temp |= MII_88E1318S_PHY_WOL_CTRL_MAGIC_PACKET_MATCH_ENABLE;
- err = phy_write(phydev, MII_88E1318S_PHY_WOL_CTRL, temp);
+ err = __phy_modify(phydev, MII_88E1318S_PHY_WOL_CTRL, 0,
+ MII_88E1318S_PHY_WOL_CTRL_CLEAR_WOL_STATUS |
+ MII_88E1318S_PHY_WOL_CTRL_MAGIC_PACKET_MATCH_ENABLE);
if (err < 0)
- return err;
+ goto error;
} else {
- err = marvell_set_page(phydev, MII_MARVELL_WOL_PAGE);
+ err = marvell_write_page(phydev, MII_MARVELL_WOL_PAGE);
if (err < 0)
- return err;
+ goto error;
/* Clear WOL status and disable magic packet matching */
- temp = phy_read(phydev, MII_88E1318S_PHY_WOL_CTRL);
- temp |= MII_88E1318S_PHY_WOL_CTRL_CLEAR_WOL_STATUS;
- temp &= ~MII_88E1318S_PHY_WOL_CTRL_MAGIC_PACKET_MATCH_ENABLE;
- err = phy_write(phydev, MII_88E1318S_PHY_WOL_CTRL, temp);
+ err = __phy_modify(phydev, MII_88E1318S_PHY_WOL_CTRL,
+ (u16)~MII_88E1318S_PHY_WOL_CTRL_MAGIC_PACKET_MATCH_ENABLE,
+ MII_88E1318S_PHY_WOL_CTRL_CLEAR_WOL_STATUS);
if (err < 0)
- return err;
+ goto error;
}
- err = marvell_set_page(phydev, oldpage);
- if (err < 0)
- return err;
-
- return 0;
+error:
+ return phy_restore_page(phydev, oldpage, err);
}
static int marvell_get_sset_count(struct phy_device *phydev)
@@ -1519,14 +1476,10 @@ static u64 marvell_get_stat(struct phy_device *phydev, int i)
{
struct marvell_hw_stat stat = marvell_hw_stats[i];
struct marvell_priv *priv = phydev->priv;
- int oldpage, val;
+ int val;
u64 ret;
- oldpage = marvell_get_set_page(phydev, stat.page);
- if (oldpage < 0)
- return UINT64_MAX;
-
- val = phy_read(phydev, stat.reg);
+ val = phy_read_paged(phydev, stat.page, stat.reg);
if (val < 0) {
ret = UINT64_MAX;
} else {
@@ -1535,8 +1488,6 @@ static u64 marvell_get_stat(struct phy_device *phydev, int i)
ret = priv->stats[i];
}
- marvell_set_page(phydev, oldpage);
-
return ret;
}
@@ -1553,51 +1504,44 @@ static void marvell_get_stats(struct phy_device *phydev,
static int m88e1121_get_temp(struct phy_device *phydev, long *temp)
{
int oldpage;
- int ret;
+ int ret = 0;
int val;
*temp = 0;
- mutex_lock(&phydev->lock);
-
- oldpage = marvell_get_set_page(phydev, MII_MARVELL_MISC_TEST_PAGE);
- if (oldpage < 0) {
- mutex_unlock(&phydev->lock);
- return oldpage;
- }
+ oldpage = phy_select_page(phydev, MII_MARVELL_MISC_TEST_PAGE);
+ if (oldpage < 0)
+ goto error;
/* Enable temperature sensor */
- ret = phy_read(phydev, MII_88E1121_MISC_TEST);
+ ret = __phy_read(phydev, MII_88E1121_MISC_TEST);
if (ret < 0)
goto error;
- ret = phy_write(phydev, MII_88E1121_MISC_TEST,
- ret | MII_88E1121_MISC_TEST_TEMP_SENSOR_EN);
+ ret = __phy_write(phydev, MII_88E1121_MISC_TEST,
+ ret | MII_88E1121_MISC_TEST_TEMP_SENSOR_EN);
if (ret < 0)
goto error;
/* Wait for temperature to stabilize */
usleep_range(10000, 12000);
- val = phy_read(phydev, MII_88E1121_MISC_TEST);
+ val = __phy_read(phydev, MII_88E1121_MISC_TEST);
if (val < 0) {
ret = val;
goto error;
}
/* Disable temperature sensor */
- ret = phy_write(phydev, MII_88E1121_MISC_TEST,
- ret & ~MII_88E1121_MISC_TEST_TEMP_SENSOR_EN);
+ ret = __phy_write(phydev, MII_88E1121_MISC_TEST,
+ ret & ~MII_88E1121_MISC_TEST_TEMP_SENSOR_EN);
if (ret < 0)
goto error;
*temp = ((val & MII_88E1121_MISC_TEST_TEMP_MASK) - 5) * 5000;
error:
- marvell_set_page(phydev, oldpage);
- mutex_unlock(&phydev->lock);
-
- return ret;
+ return phy_restore_page(phydev, oldpage, ret);
}
static int m88e1121_hwmon_read(struct device *dev,
@@ -1671,118 +1615,64 @@ static const struct hwmon_chip_info m88e1121_hwmon_chip_info = {
static int m88e1510_get_temp(struct phy_device *phydev, long *temp)
{
- int oldpage;
int ret;
*temp = 0;
- mutex_lock(&phydev->lock);
-
- oldpage = marvell_get_set_page(phydev, MII_MARVELL_MISC_TEST_PAGE);
- if (oldpage < 0) {
- mutex_unlock(&phydev->lock);
- return oldpage;
- }
-
- ret = phy_read(phydev, MII_88E1510_TEMP_SENSOR);
+ ret = phy_read_paged(phydev, MII_MARVELL_MISC_TEST_PAGE,
+ MII_88E1510_TEMP_SENSOR);
if (ret < 0)
- goto error;
+ return ret;
*temp = ((ret & MII_88E1510_TEMP_SENSOR_MASK) - 25) * 1000;
-error:
- marvell_set_page(phydev, oldpage);
- mutex_unlock(&phydev->lock);
-
- return ret;
+ return 0;
}
static int m88e1510_get_temp_critical(struct phy_device *phydev, long *temp)
{
- int oldpage;
int ret;
*temp = 0;
- mutex_lock(&phydev->lock);
-
- oldpage = marvell_get_set_page(phydev, MII_MARVELL_MISC_TEST_PAGE);
- if (oldpage < 0) {
- mutex_unlock(&phydev->lock);
- return oldpage;
- }
-
- ret = phy_read(phydev, MII_88E1121_MISC_TEST);
+ ret = phy_read_paged(phydev, MII_MARVELL_MISC_TEST_PAGE,
+ MII_88E1121_MISC_TEST);
if (ret < 0)
- goto error;
+ return ret;
*temp = (((ret & MII_88E1510_MISC_TEST_TEMP_THRESHOLD_MASK) >>
MII_88E1510_MISC_TEST_TEMP_THRESHOLD_SHIFT) * 5) - 25;
/* convert to mC */
*temp *= 1000;
-error:
- marvell_set_page(phydev, oldpage);
- mutex_unlock(&phydev->lock);
-
- return ret;
+ return 0;
}
static int m88e1510_set_temp_critical(struct phy_device *phydev, long temp)
{
- int oldpage;
- int ret;
-
- mutex_lock(&phydev->lock);
-
- oldpage = marvell_get_set_page(phydev, MII_MARVELL_MISC_TEST_PAGE);
- if (oldpage < 0) {
- mutex_unlock(&phydev->lock);
- return oldpage;
- }
-
- ret = phy_read(phydev, MII_88E1121_MISC_TEST);
- if (ret < 0)
- goto error;
-
temp = temp / 1000;
temp = clamp_val(DIV_ROUND_CLOSEST(temp, 5) + 5, 0, 0x1f);
- ret = phy_write(phydev, MII_88E1121_MISC_TEST,
- (ret & ~MII_88E1510_MISC_TEST_TEMP_THRESHOLD_MASK) |
- (temp << MII_88E1510_MISC_TEST_TEMP_THRESHOLD_SHIFT));
-
-error:
- marvell_set_page(phydev, oldpage);
- mutex_unlock(&phydev->lock);
- return ret;
+ return phy_modify_paged(phydev, MII_MARVELL_MISC_TEST_PAGE,
+ MII_88E1121_MISC_TEST,
+ MII_88E1510_MISC_TEST_TEMP_THRESHOLD_MASK,
+ temp << MII_88E1510_MISC_TEST_TEMP_THRESHOLD_SHIFT);
}
static int m88e1510_get_temp_alarm(struct phy_device *phydev, long *alarm)
{
- int oldpage;
int ret;
*alarm = false;
- mutex_lock(&phydev->lock);
-
- oldpage = marvell_get_set_page(phydev, MII_MARVELL_MISC_TEST_PAGE);
- if (oldpage < 0) {
- mutex_unlock(&phydev->lock);
- return oldpage;
- }
-
- ret = phy_read(phydev, MII_88E1121_MISC_TEST);
+ ret = phy_read_paged(phydev, MII_MARVELL_MISC_TEST_PAGE,
+ MII_88E1121_MISC_TEST);
if (ret < 0)
- goto error;
- *alarm = !!(ret & MII_88E1510_MISC_TEST_TEMP_IRQ);
+ return ret;
-error:
- marvell_set_page(phydev, oldpage);
- mutex_unlock(&phydev->lock);
+ *alarm = !!(ret & MII_88E1510_MISC_TEST_TEMP_IRQ);
- return ret;
+ return 0;
}
static int m88e1510_hwmon_read(struct device *dev,
@@ -1979,6 +1869,8 @@ static struct phy_driver marvell_drivers[] = {
.config_intr = &marvell_config_intr,
.resume = &genphy_resume,
.suspend = &genphy_suspend,
+ .read_page = marvell_read_page,
+ .write_page = marvell_write_page,
.get_sset_count = marvell_get_sset_count,
.get_strings = marvell_get_strings,
.get_stats = marvell_get_stats,
@@ -1997,6 +1889,8 @@ static struct phy_driver marvell_drivers[] = {
.config_intr = &marvell_config_intr,
.resume = &genphy_resume,
.suspend = &genphy_suspend,
+ .read_page = marvell_read_page,
+ .write_page = marvell_write_page,
.get_sset_count = marvell_get_sset_count,
.get_strings = marvell_get_strings,
.get_stats = marvell_get_stats,
@@ -2015,6 +1909,8 @@ static struct phy_driver marvell_drivers[] = {
.config_intr = &marvell_config_intr,
.resume = &genphy_resume,
.suspend = &genphy_suspend,
+ .read_page = marvell_read_page,
+ .write_page = marvell_write_page,
.get_sset_count = marvell_get_sset_count,
.get_strings = marvell_get_strings,
.get_stats = marvell_get_stats,
@@ -2033,6 +1929,8 @@ static struct phy_driver marvell_drivers[] = {
.config_intr = &marvell_config_intr,
.resume = &genphy_resume,
.suspend = &genphy_suspend,
+ .read_page = marvell_read_page,
+ .write_page = marvell_write_page,
.get_sset_count = marvell_get_sset_count,
.get_strings = marvell_get_strings,
.get_stats = marvell_get_stats,
@@ -2052,6 +1950,8 @@ static struct phy_driver marvell_drivers[] = {
.did_interrupt = &m88e1121_did_interrupt,
.resume = &genphy_resume,
.suspend = &genphy_suspend,
+ .read_page = marvell_read_page,
+ .write_page = marvell_write_page,
.get_sset_count = marvell_get_sset_count,
.get_strings = marvell_get_strings,
.get_stats = marvell_get_stats,
@@ -2073,6 +1973,8 @@ static struct phy_driver marvell_drivers[] = {
.set_wol = &m88e1318_set_wol,
.resume = &genphy_resume,
.suspend = &genphy_suspend,
+ .read_page = marvell_read_page,
+ .write_page = marvell_write_page,
.get_sset_count = marvell_get_sset_count,
.get_strings = marvell_get_strings,
.get_stats = marvell_get_stats,
@@ -2091,6 +1993,8 @@ static struct phy_driver marvell_drivers[] = {
.config_intr = &marvell_config_intr,
.resume = &genphy_resume,
.suspend = &genphy_suspend,
+ .read_page = marvell_read_page,
+ .write_page = marvell_write_page,
.get_sset_count = marvell_get_sset_count,
.get_strings = marvell_get_strings,
.get_stats = marvell_get_stats,
@@ -2109,6 +2013,8 @@ static struct phy_driver marvell_drivers[] = {
.config_intr = &marvell_config_intr,
.resume = &genphy_resume,
.suspend = &genphy_suspend,
+ .read_page = marvell_read_page,
+ .write_page = marvell_write_page,
.get_sset_count = marvell_get_sset_count,
.get_strings = marvell_get_strings,
.get_stats = marvell_get_stats,
@@ -2127,6 +2033,8 @@ static struct phy_driver marvell_drivers[] = {
.config_intr = &marvell_config_intr,
.resume = &genphy_resume,
.suspend = &genphy_suspend,
+ .read_page = marvell_read_page,
+ .write_page = marvell_write_page,
.get_sset_count = marvell_get_sset_count,
.get_strings = marvell_get_strings,
.get_stats = marvell_get_stats,
@@ -2145,6 +2053,8 @@ static struct phy_driver marvell_drivers[] = {
.config_intr = &marvell_config_intr,
.resume = &genphy_resume,
.suspend = &genphy_suspend,
+ .read_page = marvell_read_page,
+ .write_page = marvell_write_page,
.get_sset_count = marvell_get_sset_count,
.get_strings = marvell_get_strings,
.get_stats = marvell_get_stats,
@@ -2166,6 +2076,8 @@ static struct phy_driver marvell_drivers[] = {
.set_wol = &m88e1318_set_wol,
.resume = &marvell_resume,
.suspend = &marvell_suspend,
+ .read_page = marvell_read_page,
+ .write_page = marvell_write_page,
.get_sset_count = marvell_get_sset_count,
.get_strings = marvell_get_strings,
.get_stats = marvell_get_stats,
@@ -2186,6 +2098,8 @@ static struct phy_driver marvell_drivers[] = {
.did_interrupt = &m88e1121_did_interrupt,
.resume = &genphy_resume,
.suspend = &genphy_suspend,
+ .read_page = marvell_read_page,
+ .write_page = marvell_write_page,
.get_sset_count = marvell_get_sset_count,
.get_strings = marvell_get_strings,
.get_stats = marvell_get_stats,
@@ -2205,6 +2119,8 @@ static struct phy_driver marvell_drivers[] = {
.did_interrupt = &m88e1121_did_interrupt,
.resume = &genphy_resume,
.suspend = &genphy_suspend,
+ .read_page = marvell_read_page,
+ .write_page = marvell_write_page,
.get_sset_count = marvell_get_sset_count,
.get_strings = marvell_get_strings,
.get_stats = marvell_get_stats,
@@ -2225,6 +2141,8 @@ static struct phy_driver marvell_drivers[] = {
.did_interrupt = &m88e1121_did_interrupt,
.resume = &genphy_resume,
.suspend = &genphy_suspend,
+ .read_page = marvell_read_page,
+ .write_page = marvell_write_page,
.get_sset_count = marvell_get_sset_count,
.get_strings = marvell_get_strings,
.get_stats = marvell_get_stats,
@@ -2244,6 +2162,8 @@ static struct phy_driver marvell_drivers[] = {
.did_interrupt = &m88e1121_did_interrupt,
.resume = &genphy_resume,
.suspend = &genphy_suspend,
+ .read_page = marvell_read_page,
+ .write_page = marvell_write_page,
.get_sset_count = marvell_get_sset_count,
.get_strings = marvell_get_strings,
.get_stats = marvell_get_stats,
--
2.7.4
^ permalink raw reply related
* [PATCH net-next 4/7] net: phy: add paged phy register accessors
From: Russell King @ 2017-12-29 12:31 UTC (permalink / raw)
To: Andrew Lunn, Florian Fainelli; +Cc: netdev
In-Reply-To: <20171229123024.GZ10595@n2100.armlinux.org.uk>
Add a set of paged phy register accessors which are inherently safe in
their design against other accesses interfering with the paged access.
Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
drivers/net/phy/phy-core.c | 157 +++++++++++++++++++++++++++++++++++++++++++++
include/linux/phy.h | 11 ++++
2 files changed, 168 insertions(+)
diff --git a/drivers/net/phy/phy-core.c b/drivers/net/phy/phy-core.c
index 37c039da0c16..df39e6711b76 100644
--- a/drivers/net/phy/phy-core.c
+++ b/drivers/net/phy/phy-core.c
@@ -305,3 +305,160 @@ int __phy_modify(struct phy_device *phydev, u32 regnum, u16 mask, u16 set)
return ret;
}
EXPORT_SYMBOL_GPL(__phy_modify);
+
+static int __phy_read_page(struct phy_device *phydev)
+{
+ return phydev->drv->read_page(phydev);
+}
+
+static int __phy_write_page(struct phy_device *phydev, int page)
+{
+ return phydev->drv->write_page(phydev, page);
+}
+
+/**
+ * phy_save_page() - take the bus lock and save the current page
+ * @phydev: a pointer to a &struct phy_device
+ *
+ * Take the MDIO bus lock, and return the current page number. On error,
+ * returns a negative errno. phy_restore_page() must be called after this
+ * to release the lock even on failure.
+ */
+int phy_save_page(struct phy_device *phydev)
+{
+ mutex_lock(&phydev->mdio.bus->mdio_lock);
+ return __phy_read_page(phydev);
+}
+EXPORT_SYMBOL_GPL(phy_save_page);
+
+/**
+ * phy_select_page() - take the bus lock, save the current page, and set a page
+ * @phydev: a pointer to a &struct phy_device
+ * @page: desired page
+ *
+ * Take the MDIO bus lock to protect against concurrent access, save the
+ * current PHY page, and set the current page. On error, returns a
+ * negative errno, otherwise returns the previous page number.
+ * phy_restore_page() must be called after this to restore the page
+ * number (if this call was successful) and release the lock.
+ */
+int phy_select_page(struct phy_device *phydev, int page)
+{
+ int ret, oldpage;
+
+ oldpage = ret = phy_save_page(phydev);
+ if (ret < 0)
+ return ret;
+
+ if (oldpage != page) {
+ ret = __phy_write_page(phydev, page);
+ if (ret < 0)
+ return ret;
+ }
+
+ return oldpage;
+}
+EXPORT_SYMBOL_GPL(phy_select_page);
+
+/**
+ * phy_restore_page() - restore the page register and release the bus lock
+ * @phydev: a pointer to a &struct phy_device
+ * @oldpage: the old page, return value from phy_save_page() or phy_select_page()
+ * @ret: operation's return code
+ *
+ * Release the MDIO bus lock, restoring @oldpage if it is a valid page.
+ * This function propagates the earliest error code from the group of
+ * operations.
+ *
+ * Returns:
+ * @oldpage if it was a negative value, otherwise
+ * @ret if it was a negative errno value, otherwise
+ * phy_write_page()'s negative value if it were in error, otherwise
+ * @ret.
+ */
+int phy_restore_page(struct phy_device *phydev, int oldpage, int ret)
+{
+ int r;
+
+ if (oldpage >= 0) {
+ r = __phy_write_page(phydev, oldpage);
+
+ /* Propagate the operation return code if the page write
+ * was successful.
+ */
+ if (ret >= 0 && r < 0)
+ ret = r;
+ } else {
+ /* Propagate the phy page selection error code */
+ ret = oldpage;
+ }
+
+ mutex_unlock(&phydev->mdio.bus->mdio_lock);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(phy_restore_page);
+
+/**
+ * phy_read_paged() - Convenience function for reading a paged register
+ * @phydev: a pointer to a &struct phy_device
+ * @page: the page for the phy
+ * @regnum: register number
+ *
+ * Same rules as for phy_read();
+ */
+int phy_read_paged(struct phy_device *phydev, int page, u32 regnum)
+{
+ int ret = 0, oldpage;
+
+ oldpage = phy_select_page(phydev, page);
+ if (oldpage >= 0)
+ ret = __phy_read(phydev, regnum);
+
+ return phy_restore_page(phydev, oldpage, ret);
+}
+EXPORT_SYMBOL(phy_read_paged);
+
+/**
+ * phy_write_paged() - Convenience function for writing a paged register
+ * @phydev: a pointer to a &struct phy_device
+ * @page: the page for the phy
+ * @regnum: register number
+ * @val: value to write
+ *
+ * Same rules as for phy_write();
+ */
+int phy_write_paged(struct phy_device *phydev, int page, u32 regnum, u16 val)
+{
+ int ret = 0, oldpage;
+
+ oldpage = phy_select_page(phydev, page);
+ if (oldpage >= 0)
+ ret = __phy_write(phydev, regnum, val);
+
+ return phy_restore_page(phydev, oldpage, ret);
+}
+EXPORT_SYMBOL(phy_write_paged);
+
+/**
+ * phy_modify_paged() - Convenience function for modifying a paged register
+ * @phydev: a pointer to a &struct phy_device
+ * @page: the page for the phy
+ * @regnum: register number
+ * @mask: bit mask of bits to clear
+ * @set: bit mask of bits to set
+ *
+ * Same rules as for phy_read() and phy_write();
+ */
+int phy_modify_paged(struct phy_device *phydev, int page, u32 regnum,
+ u16 mask, u16 set)
+{
+ int ret = 0, oldpage;
+
+ oldpage = phy_select_page(phydev, page);
+ if (oldpage >= 0)
+ ret = __phy_modify(phydev, regnum, mask, set);
+
+ return phy_restore_page(phydev, oldpage, ret);
+}
+EXPORT_SYMBOL(phy_modify_paged);
diff --git a/include/linux/phy.h b/include/linux/phy.h
index 3dae5b7408b4..f408220d95ec 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -634,6 +634,9 @@ struct phy_driver {
int (*write_mmd)(struct phy_device *dev, int devnum, u16 regnum,
u16 val);
+ int (*read_page)(struct phy_device *dev);
+ int (*write_page)(struct phy_device *dev, int page);
+
/* Get the size and type of the eeprom contained within a plug-in
* module */
int (*module_info)(struct phy_device *dev,
@@ -836,6 +839,14 @@ static inline bool phy_is_pseudo_fixed_link(struct phy_device *phydev)
*/
int phy_write_mmd(struct phy_device *phydev, int devad, u32 regnum, u16 val);
+int phy_save_page(struct phy_device *phydev);
+int phy_select_page(struct phy_device *phydev, int page);
+int phy_restore_page(struct phy_device *phydev, int oldpage, int ret);
+int phy_read_paged(struct phy_device *phydev, int page, u32 regnum);
+int phy_write_paged(struct phy_device *phydev, int page, u32 regnum, u16 val);
+int phy_modify_paged(struct phy_device *phydev, int page, u32 regnum,
+ u16 mask, u16 set);
+
struct phy_device *phy_device_create(struct mii_bus *bus, int addr, int phy_id,
bool is_c45,
struct phy_c45_device_ids *c45_ids);
--
2.7.4
^ permalink raw reply related
* [PATCH net-next 3/7] net: phy: add unlocked accessors
From: Russell King @ 2017-12-29 12:30 UTC (permalink / raw)
To: Andrew Lunn, Florian Fainelli; +Cc: netdev
In-Reply-To: <20171229123024.GZ10595@n2100.armlinux.org.uk>
Add unlocked versions of the bus accessors, which allows access to the
bus with all the tracing. These accessors validate that the bus mutex
is held, which is a basic requirement for all mii bus accesses.
Also added is a read-modify-write unlocked accessor with the same
locking requirements.
Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
drivers/net/phy/phy-core.c | 25 +++++++++++++++++++++++++
include/linux/phy.h | 28 ++++++++++++++++++++++++++++
2 files changed, 53 insertions(+)
diff --git a/drivers/net/phy/phy-core.c b/drivers/net/phy/phy-core.c
index 83d32644cb4d..37c039da0c16 100644
--- a/drivers/net/phy/phy-core.c
+++ b/drivers/net/phy/phy-core.c
@@ -280,3 +280,28 @@ int phy_write_mmd(struct phy_device *phydev, int devad, u32 regnum, u16 val)
return ret;
}
EXPORT_SYMBOL(phy_write_mmd);
+
+/**
+ * __phy_modify() - Convenience function for modifying a PHY register
+ * @phydev: a pointer to a &struct phy_device
+ * @regnum: register number
+ * @mask: bit mask of bits to clear
+ * @set: bit mask of bits to set
+ *
+ * Unlocked helper function which allows a PHY register to be modified as
+ * new register value = (old register value & mask) | set
+ */
+int __phy_modify(struct phy_device *phydev, u32 regnum, u16 mask, u16 set)
+{
+ int ret, res;
+
+ ret = __phy_read(phydev, regnum);
+ if (ret >= 0) {
+ res = __phy_write(phydev, regnum, (ret & ~mask) | set);
+ if (res < 0)
+ ret = res;
+ }
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(__phy_modify);
diff --git a/include/linux/phy.h b/include/linux/phy.h
index 71d777fe6c3d..3dae5b7408b4 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -716,6 +716,18 @@ static inline int phy_read(struct phy_device *phydev, u32 regnum)
}
/**
+ * __phy_read - convenience function for reading a given PHY register
+ * @phydev: the phy_device struct
+ * @regnum: register number to read
+ *
+ * The caller must have taken the MDIO bus lock.
+ */
+static inline int __phy_read(struct phy_device *phydev, u32 regnum)
+{
+ return __mdiobus_read(phydev->mdio.bus, phydev->mdio.addr, regnum);
+}
+
+/**
* phy_write - Convenience function for writing a given PHY register
* @phydev: the phy_device struct
* @regnum: register number to write
@@ -731,6 +743,22 @@ static inline int phy_write(struct phy_device *phydev, u32 regnum, u16 val)
}
/**
+ * __phy_write - Convenience function for writing a given PHY register
+ * @phydev: the phy_device struct
+ * @regnum: register number to write
+ * @val: value to write to @regnum
+ *
+ * The caller must have taken the MDIO bus lock.
+ */
+static inline int __phy_write(struct phy_device *phydev, u32 regnum, u16 val)
+{
+ return __mdiobus_write(phydev->mdio.bus, phydev->mdio.addr, regnum,
+ val);
+}
+
+int __phy_modify(struct phy_device *phydev, u32 regnum, u16 mask, u16 set);
+
+/**
* phy_interrupt_is_valid - Convenience function for testing a given PHY irq
* @phydev: the phy_device struct
*
--
2.7.4
^ permalink raw reply related
* [PATCH net-next 2/7] net: phy: use unlocked accessors for indirect MMD accesses
From: Russell King @ 2017-12-29 12:30 UTC (permalink / raw)
To: Andrew Lunn, Florian Fainelli; +Cc: netdev
In-Reply-To: <20171229123024.GZ10595@n2100.armlinux.org.uk>
Use unlocked accessors for indirect MMD accesses to clause 22 PHYs.
This permits tracing of these accesses.
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
drivers/net/phy/phy-core.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/net/phy/phy-core.c b/drivers/net/phy/phy-core.c
index 21f75ae244b3..83d32644cb4d 100644
--- a/drivers/net/phy/phy-core.c
+++ b/drivers/net/phy/phy-core.c
@@ -193,13 +193,14 @@ static void mmd_phy_indirect(struct mii_bus *bus, int phy_addr, int devad,
u16 regnum)
{
/* Write the desired MMD Devad */
- bus->write(bus, phy_addr, MII_MMD_CTRL, devad);
+ __mdiobus_write(bus, phy_addr, MII_MMD_CTRL, devad);
/* Write the desired MMD register address */
- bus->write(bus, phy_addr, MII_MMD_DATA, regnum);
+ __mdiobus_write(bus, phy_addr, MII_MMD_DATA, regnum);
/* Select the Function : DATA with no post increment */
- bus->write(bus, phy_addr, MII_MMD_CTRL, devad | MII_MMD_CTRL_NOINCR);
+ __mdiobus_write(bus, phy_addr, MII_MMD_CTRL,
+ devad | MII_MMD_CTRL_NOINCR);
}
/**
@@ -232,7 +233,7 @@ int phy_read_mmd(struct phy_device *phydev, int devad, u32 regnum)
mmd_phy_indirect(bus, phy_addr, devad, regnum);
/* Read the content of the MMD's selected register */
- val = bus->read(bus, phy_addr, MII_MMD_DATA);
+ val = __mdiobus_read(bus, phy_addr, MII_MMD_DATA);
mutex_unlock(&bus->mdio_lock);
}
return val;
@@ -271,7 +272,7 @@ int phy_write_mmd(struct phy_device *phydev, int devad, u32 regnum, u16 val)
mmd_phy_indirect(bus, phy_addr, devad, regnum);
/* Write the data into MMD's selected register */
- bus->write(bus, phy_addr, MII_MMD_DATA, val);
+ __mdiobus_write(bus, phy_addr, MII_MMD_DATA, val);
mutex_unlock(&bus->mdio_lock);
ret = 0;
--
2.7.4
^ permalink raw reply related
* [PATCH net-next 1/7] net: mdiobus: add unlocked accessors
From: Russell King @ 2017-12-29 12:30 UTC (permalink / raw)
To: Andrew Lunn, Florian Fainelli; +Cc: netdev
In-Reply-To: <20171229123024.GZ10595@n2100.armlinux.org.uk>
Add unlocked versions of the bus accessors, which allows access to the
bus with all the tracing. These accessors validate that the bus mutex
is held, which is a basic requirement for all mii bus accesses.
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
drivers/net/phy/mdio_bus.c | 65 +++++++++++++++++++++++++++++++++++++---------
include/linux/mdio.h | 3 +++
2 files changed, 56 insertions(+), 12 deletions(-)
diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c
index 54d00a1d2bef..75be8be0d169 100644
--- a/drivers/net/phy/mdio_bus.c
+++ b/drivers/net/phy/mdio_bus.c
@@ -494,6 +494,55 @@ struct phy_device *mdiobus_scan(struct mii_bus *bus, int addr)
EXPORT_SYMBOL(mdiobus_scan);
/**
+ * __mdiobus_read - Unlocked version of the mdiobus_read function
+ * @bus: the mii_bus struct
+ * @addr: the phy address
+ * @regnum: register number to read
+ *
+ * Read a MDIO bus register. Caller must hold the mdio bus lock.
+ *
+ * NOTE: MUST NOT be called from interrupt context.
+ */
+int __mdiobus_read(struct mii_bus *bus, int addr, u32 regnum)
+{
+ int retval;
+
+ WARN_ON_ONCE(!mutex_is_locked(&bus->mdio_lock));
+
+ retval = bus->read(bus, addr, regnum);
+
+ trace_mdio_access(bus, 1, addr, regnum, retval, retval);
+
+ return retval;
+}
+EXPORT_SYMBOL(__mdiobus_read);
+
+/**
+ * __mdiobus_write - Unlocked version of the mdiobus_write function
+ * @bus: the mii_bus struct
+ * @addr: the phy address
+ * @regnum: register number to write
+ * @val: value to write to @regnum
+ *
+ * Write a MDIO bus register. Caller must hold the mdio bus lock.
+ *
+ * NOTE: MUST NOT be called from interrupt context.
+ */
+int __mdiobus_write(struct mii_bus *bus, int addr, u32 regnum, u16 val)
+{
+ int err;
+
+ WARN_ON_ONCE(!mutex_is_locked(&bus->mdio_lock));
+
+ err = bus->write(bus, addr, regnum, val);
+
+ trace_mdio_access(bus, 0, addr, regnum, val, err);
+
+ return err;
+}
+EXPORT_SYMBOL(__mdiobus_write);
+
+/**
* mdiobus_read_nested - Nested version of the mdiobus_read function
* @bus: the mii_bus struct
* @addr: the phy address
@@ -513,11 +562,9 @@ int mdiobus_read_nested(struct mii_bus *bus, int addr, u32 regnum)
BUG_ON(in_interrupt());
mutex_lock_nested(&bus->mdio_lock, MDIO_MUTEX_NESTED);
- retval = bus->read(bus, addr, regnum);
+ retval = __mdiobus_read(bus, addr, regnum);
mutex_unlock(&bus->mdio_lock);
- trace_mdio_access(bus, 1, addr, regnum, retval, retval);
-
return retval;
}
EXPORT_SYMBOL(mdiobus_read_nested);
@@ -539,11 +586,9 @@ int mdiobus_read(struct mii_bus *bus, int addr, u32 regnum)
BUG_ON(in_interrupt());
mutex_lock(&bus->mdio_lock);
- retval = bus->read(bus, addr, regnum);
+ retval = __mdiobus_read(bus, addr, regnum);
mutex_unlock(&bus->mdio_lock);
- trace_mdio_access(bus, 1, addr, regnum, retval, retval);
-
return retval;
}
EXPORT_SYMBOL(mdiobus_read);
@@ -569,11 +614,9 @@ int mdiobus_write_nested(struct mii_bus *bus, int addr, u32 regnum, u16 val)
BUG_ON(in_interrupt());
mutex_lock_nested(&bus->mdio_lock, MDIO_MUTEX_NESTED);
- err = bus->write(bus, addr, regnum, val);
+ err = __mdiobus_write(bus, addr, regnum, val);
mutex_unlock(&bus->mdio_lock);
- trace_mdio_access(bus, 0, addr, regnum, val, err);
-
return err;
}
EXPORT_SYMBOL(mdiobus_write_nested);
@@ -596,11 +639,9 @@ int mdiobus_write(struct mii_bus *bus, int addr, u32 regnum, u16 val)
BUG_ON(in_interrupt());
mutex_lock(&bus->mdio_lock);
- err = bus->write(bus, addr, regnum, val);
+ err = __mdiobus_write(bus, addr, regnum, val);
mutex_unlock(&bus->mdio_lock);
- trace_mdio_access(bus, 0, addr, regnum, val, err);
-
return err;
}
EXPORT_SYMBOL(mdiobus_write);
diff --git a/include/linux/mdio.h b/include/linux/mdio.h
index ca08ab16ecdc..34796e29c90c 100644
--- a/include/linux/mdio.h
+++ b/include/linux/mdio.h
@@ -257,6 +257,9 @@ static inline u16 ethtool_adv_to_mmd_eee_adv_t(u32 adv)
return reg;
}
+int __mdiobus_read(struct mii_bus *bus, int addr, u32 regnum);
+int __mdiobus_write(struct mii_bus *bus, int addr, u32 regnum, u16 val);
+
int mdiobus_read(struct mii_bus *bus, int addr, u32 regnum);
int mdiobus_read_nested(struct mii_bus *bus, int addr, u32 regnum);
int mdiobus_write(struct mii_bus *bus, int addr, u32 regnum, u16 val);
--
2.7.4
^ permalink raw reply related
* [PATCH net-next 0/7] Resolve races in phy accessors
From: Russell King - ARM Linux @ 2017-12-29 12:30 UTC (permalink / raw)
To: Andrew Lunn, Florian Fainelli; +Cc: netdev
Hi,
This series resolves races with various accesses to PHY registers.
The first five patches are necessary before we add phylink support
to mvneta, the remaining three are merely cleanups for unobserved
races, and hence are less critical.
There are two possible classes of races that can occur: where we
write to a page register that changes the meaning of a group of
other registers, and where we read-modify-write a register.
Resolve these races by performing the accesses under the mdio bus
lock, ensuring that no other user can access the bus while the
series of atomic operations are being performed.
These patches have been posted before, and have been modified
along the lines of previous feedback:
- The third patch was originally reviewed by Florian, but as I've
added __phy_modify() to it, I've removed that attributation.
- Included generic page-based accessors as suggested last time
around.
- Since we have the unlocked __phy_modify() in this patch series,
it is sensible to include the changes for this to marvell.c -
these accessors have to change anyway to avoid deadlocks on the
mdio bus lock.
I haven't been able to test the at803x.c changes yet beyond compile
testing - although I do have systems with an ar8035 PHY. However,
they should be straight forward to review.
This is targetted for net-next because the races have not been
found in existing drivers, but have been observed with phylink
integrated into mvneta - that's not to say that the races do not
exist today, they are just unobserved (probably through lack of
rigorous enough testing.) The race provoking condition is detailed
in patch 5.
drivers/net/phy/at803x.c | 20 +-
drivers/net/phy/marvell.c | 436 +++++++++++++++++--------------------------
drivers/net/phy/mdio_bus.c | 65 +++++--
drivers/net/phy/phy-core.c | 216 ++++++++++++++++++++-
drivers/net/phy/phy_device.c | 50 +----
include/linux/mdio.h | 3 +
include/linux/phy.h | 40 ++++
7 files changed, 487 insertions(+), 343 deletions(-)
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up
^ permalink raw reply
* [PATCH net-next A 5/5] phylink: remove 'mode' variable from phylink_sfp_module_insert()
From: Russell King @ 2017-12-29 12:15 UTC (permalink / raw)
To: Andrew Lunn, Florian Fainelli; +Cc: netdev
In-Reply-To: <20171229121451.GY10595@n2100.armlinux.org.uk>
'mode' is actually constant through phylink_sfp_module_insert(), so
remove it and replace it with the enumerated constant.
Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
drivers/net/phy/phylink.c | 15 ++++++++-------
1 file changed, 8 insertions(+), 7 deletions(-)
diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
index 073530949677..4e8c459bf062 100644
--- a/drivers/net/phy/phylink.c
+++ b/drivers/net/phy/phylink.c
@@ -1578,7 +1578,7 @@ static int phylink_sfp_module_insert(void *upstream,
__ETHTOOL_DECLARE_LINK_MODE_MASK(support) = { 0, };
struct phylink_link_state config;
phy_interface_t iface;
- int mode, ret = 0;
+ int ret = 0;
bool changed;
u8 port;
@@ -1593,7 +1593,6 @@ static int phylink_sfp_module_insert(void *upstream,
case PHY_INTERFACE_MODE_1000BASEX:
case PHY_INTERFACE_MODE_2500BASEX:
case PHY_INTERFACE_MODE_10GKR:
- mode = MLO_AN_INBAND;
break;
default:
return -EINVAL;
@@ -1611,13 +1610,15 @@ static int phylink_sfp_module_insert(void *upstream,
ret = phylink_validate(pl, support, &config);
if (ret) {
netdev_err(pl->netdev, "validation of %s/%s with support %*pb failed: %d\n",
- phylink_an_mode_str(mode), phy_modes(config.interface),
+ phylink_an_mode_str(MLO_AN_INBAND),
+ phy_modes(config.interface),
__ETHTOOL_LINK_MODE_MASK_NBITS, support, ret);
return ret;
}
netdev_dbg(pl->netdev, "requesting link mode %s/%s with support %*pb\n",
- phylink_an_mode_str(mode), phy_modes(config.interface),
+ phylink_an_mode_str(MLO_AN_INBAND),
+ phy_modes(config.interface),
__ETHTOOL_LINK_MODE_MASK_NBITS, support);
if (phy_interface_mode_is_8023z(iface) && pl->phydev)
@@ -1630,15 +1631,15 @@ static int phylink_sfp_module_insert(void *upstream,
linkmode_copy(pl->link_config.advertising, config.advertising);
}
- if (pl->link_an_mode != mode ||
+ if (pl->link_an_mode != MLO_AN_INBAND ||
pl->link_config.interface != config.interface) {
pl->link_config.interface = config.interface;
- pl->link_an_mode = mode;
+ pl->link_an_mode = MLO_AN_INBAND;
changed = true;
netdev_info(pl->netdev, "switched to %s/%s link mode\n",
- phylink_an_mode_str(mode),
+ phylink_an_mode_str(MLO_AN_INBAND),
phy_modes(config.interface));
}
--
2.7.4
^ permalink raw reply related
* [PATCH net-next A 4/5] sfp: improve support for direct-attach copper cables
From: Russell King @ 2017-12-29 12:15 UTC (permalink / raw)
To: Andrew Lunn, Florian Fainelli; +Cc: netdev
In-Reply-To: <20171229121451.GY10595@n2100.armlinux.org.uk>
Improve the support for direct-attach copper so that we avoid kernel
warning messages, and report the appropriate PORT_DA type to userspace.
Direct Attach cables can use a number of protocols depending on their
range of speeds.
Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
drivers/net/phy/sfp-bus.c | 51 ++++++++++++++++++++++++++++++++++++++++-------
include/linux/sfp.h | 36 ++++++++++++++++++++++++++++++++-
2 files changed, 79 insertions(+), 8 deletions(-)
diff --git a/drivers/net/phy/sfp-bus.c b/drivers/net/phy/sfp-bus.c
index 6c05acd5b1d4..bdc4bb3c8288 100644
--- a/drivers/net/phy/sfp-bus.c
+++ b/drivers/net/phy/sfp-bus.c
@@ -57,21 +57,19 @@ int sfp_parse_port(struct sfp_bus *bus, const struct sfp_eeprom_id *id,
case SFP_CONNECTOR_MT_RJ:
case SFP_CONNECTOR_MU:
case SFP_CONNECTOR_OPTICAL_PIGTAIL:
- if (support)
- phylink_set(support, FIBRE);
port = PORT_FIBRE;
break;
case SFP_CONNECTOR_RJ45:
- if (support)
- phylink_set(support, TP);
port = PORT_TP;
break;
+ case SFP_CONNECTOR_COPPER_PIGTAIL:
+ port = PORT_DA;
+ break;
+
case SFP_CONNECTOR_UNSPEC:
if (id->base.e1000_base_t) {
- if (support)
- phylink_set(support, TP);
port = PORT_TP;
break;
}
@@ -80,7 +78,6 @@ int sfp_parse_port(struct sfp_bus *bus, const struct sfp_eeprom_id *id,
case SFP_CONNECTOR_MPO_1X12:
case SFP_CONNECTOR_MPO_2X16:
case SFP_CONNECTOR_HSSDC_II:
- case SFP_CONNECTOR_COPPER_PIGTAIL:
case SFP_CONNECTOR_NOSEPARATE:
case SFP_CONNECTOR_MXC_2X16:
port = PORT_OTHER;
@@ -92,6 +89,18 @@ int sfp_parse_port(struct sfp_bus *bus, const struct sfp_eeprom_id *id,
break;
}
+ if (support) {
+ switch (port) {
+ case PORT_FIBRE:
+ phylink_set(support, FIBRE);
+ break;
+
+ case PORT_TP:
+ phylink_set(support, TP);
+ break;
+ }
+ }
+
return port;
}
EXPORT_SYMBOL_GPL(sfp_parse_port);
@@ -143,6 +152,11 @@ phy_interface_t sfp_parse_interface(struct sfp_bus *bus,
break;
default:
+ if (id->base.e1000_base_cx) {
+ iface = PHY_INTERFACE_MODE_1000BASEX;
+ break;
+ }
+
iface = PHY_INTERFACE_MODE_NA;
dev_err(bus->sfp_dev,
"SFP module encoding does not support 8b10b nor 64b66b\n");
@@ -208,6 +222,29 @@ void sfp_parse_support(struct sfp_bus *bus, const struct sfp_eeprom_id *id,
br_min <= 1300 && br_max >= 1200)
phylink_set(support, 1000baseX_Full);
+ /* For active or passive cables, select the link modes
+ * based on the bit rates and the cable compliance bytes.
+ */
+ if ((id->base.sfp_ct_passive || id->base.sfp_ct_active) && br_nom) {
+ /* This may look odd, but some manufacturers use 12000MBd */
+ if (br_min <= 12000 && br_max >= 10300)
+ phylink_set(support, 10000baseCR_Full);
+ if (br_min <= 3200 && br_max >= 3100)
+ phylink_set(support, 2500baseX_Full);
+ if (br_min <= 1300 && br_max >= 1200)
+ phylink_set(support, 1000baseX_Full);
+ }
+ if (id->base.sfp_ct_passive) {
+ if (id->base.passive.sff8431_app_e)
+ phylink_set(support, 10000baseCR_Full);
+ }
+ if (id->base.sfp_ct_active) {
+ if (id->base.active.sff8431_app_e ||
+ id->base.active.sff8431_lim) {
+ phylink_set(support, 10000baseCR_Full);
+ }
+ }
+
switch (id->base.extended_cc) {
case 0x00: /* Unspecified */
break;
diff --git a/include/linux/sfp.h b/include/linux/sfp.h
index 0c5c5f6ae1ec..e724d5a3dd80 100644
--- a/include/linux/sfp.h
+++ b/include/linux/sfp.h
@@ -165,7 +165,41 @@ struct sfp_eeprom_base {
char vendor_rev[4];
union {
__be16 optical_wavelength;
- u8 cable_spec;
+ __be16 cable_compliance;
+ struct {
+#if defined __BIG_ENDIAN_BITFIELD
+ u8 reserved60_2:6;
+ u8 fc_pi_4_app_h:1;
+ u8 sff8431_app_e:1;
+ u8 reserved61:8;
+#elif defined __LITTLE_ENDIAN_BITFIELD
+ u8 sff8431_app_e:1;
+ u8 fc_pi_4_app_h:1;
+ u8 reserved60_2:6;
+ u8 reserved61:8;
+#else
+#error Unknown Endian
+#endif
+ } __packed passive;
+ struct {
+#if defined __BIG_ENDIAN_BITFIELD
+ u8 reserved60_4:4;
+ u8 fc_pi_4_lim:1;
+ u8 sff8431_lim:1;
+ u8 fc_pi_4_app_h:1;
+ u8 sff8431_app_e:1;
+ u8 reserved61:8;
+#elif defined __LITTLE_ENDIAN_BITFIELD
+ u8 sff8431_app_e:1;
+ u8 fc_pi_4_app_h:1;
+ u8 sff8431_lim:1;
+ u8 fc_pi_4_lim:1;
+ u8 reserved60_4:4;
+ u8 reserved61:8;
+#else
+#error Unknown Endian
+#endif
+ } __packed active;
} __packed;
u8 reserved62;
u8 cc_base;
--
2.7.4
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox