This series adds experimental support for Xilinx PCS devices. It is
experimental because while I believe I have the Linux side mostly
sorted, I have yet to achieve any data transfer. Adding support for this
device has required some plumbing work related to PCSs in general, and I
would appreciate feedback in that area. In general, I have not tested
these changes outside of my particular setup, though I do have the
ability to test the macb changes using the internal PCS in the future.
Sean Anderson (16):
dt-bindings: net: Add pcs property
dt-bindings: net: Add binding for Xilinx PCS
net: sfp: Fix typo in state machine debug string
net: phylink: Move phylink_set_pcs before phylink_create
net: phylink: Automatically attach PCS devices
net: phylink: Add function for optionally adding a PCS
net: phylink: Add helpers for c22 registers without MDIO
net: macb: Clean up macb_validate
net: macb: Move most of mac_prepare to mac_config
net: macb: Move PCS settings to PCS callbacks
net: macb: Support restarting PCS autonegotiation
net: macb: Support external PCSs
net: phy: Export get_phy_c22_id
net: mdio: Add helper functions for accessing MDIO devices
net: pcs: Add Xilinx PCS driver
net: sfp: Add quirk to ignore PHYs
.../bindings/net/ethernet-controller.yaml | 5 +
.../devicetree/bindings/net/xilinx,pcs.yaml | 83 ++++
MAINTAINERS | 6 +
drivers/net/ethernet/cadence/macb_main.c | 375 +++++++++++-------
drivers/net/pcs/Kconfig | 19 +
drivers/net/pcs/Makefile | 1 +
drivers/net/pcs/pcs-xilinx.c | 326 +++++++++++++++
drivers/net/phy/phy_device.c | 3 +-
drivers/net/phy/phylink.c | 335 ++++++++++++----
drivers/net/phy/sfp-bus.c | 12 +-
drivers/net/phy/sfp.c | 5 +-
include/linux/mdio.h | 17 +
include/linux/phy.h | 1 +
include/linux/phylink.h | 17 +-
14 files changed, 963 insertions(+), 242 deletions(-)
create mode 100644 Documentation/devicetree/bindings/net/xilinx,pcs.yaml
create mode 100644 drivers/net/pcs/pcs-xilinx.c
--
2.25.1
Some devices expose memory-mapped c22-compliant PHYs. Because these
devices do not have an MDIO bus, we cannot use the existing helpers.
Refactor the existing helpers to allow supplying the values for c22
registers directly, instead of using MDIO to access them. Only get_state
and set_adversisement are converted, since they contain the most complex
logic.
Signed-off-by: Sean Anderson <[email protected]>
---
drivers/net/phy/phylink.c | 154 ++++++++++++++++++++++----------------
include/linux/phylink.h | 5 ++
2 files changed, 96 insertions(+), 63 deletions(-)
diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
index f82dc0f87f40..1b6077557e31 100644
--- a/drivers/net/phy/phylink.c
+++ b/drivers/net/phy/phylink.c
@@ -2639,6 +2639,49 @@ void phylink_decode_usxgmii_word(struct phylink_link_state *state,
}
EXPORT_SYMBOL_GPL(phylink_decode_usxgmii_word);
+/**
+ * phylink_mii_c22_pcs_decode_state() - Decode MAC PCS state from MII registers
+ * @state: a pointer to a &struct phylink_link_state.
+ * @bmsr: The value of the %MII_BMSR register
+ * @lpa: The value of the %MII_LPA register
+ *
+ * Helper for MAC PCS supporting the 802.3 clause 22 register set for
+ * clause 37 negotiation and/or SGMII control.
+ *
+ * Parse the Clause 37 or Cisco SGMII link partner negotiation word into
+ * the phylink @state structure. This is suitable to be used for implementing
+ * the mac_pcs_get_state() member of the struct phylink_mac_ops structure if
+ * accessing @bmsr and @lpa cannot be done with MDIO directly.
+ */
+void phylink_mii_c22_pcs_decode_state(struct phylink_link_state *state,
+ u16 bmsr, u16 lpa)
+{
+ state->link = !!(bmsr & BMSR_LSTATUS);
+ state->an_complete = !!(bmsr & BMSR_ANEGCOMPLETE);
+ if (!state->link)
+ return;
+
+ switch (state->interface) {
+ case PHY_INTERFACE_MODE_1000BASEX:
+ phylink_decode_c37_word(state, lpa, SPEED_1000);
+ break;
+
+ case PHY_INTERFACE_MODE_2500BASEX:
+ phylink_decode_c37_word(state, lpa, SPEED_2500);
+ break;
+
+ case PHY_INTERFACE_MODE_SGMII:
+ case PHY_INTERFACE_MODE_QSGMII:
+ phylink_decode_sgmii_word(state, lpa);
+ break;
+
+ default:
+ state->link = false;
+ break;
+ }
+}
+EXPORT_SYMBOL_GPL(phylink_mii_c22_pcs_decode_state);
+
/**
* phylink_mii_c22_pcs_get_state() - read the MAC PCS state
* @pcs: a pointer to a &struct mdio_device.
@@ -2667,32 +2710,48 @@ void phylink_mii_c22_pcs_get_state(struct mdio_device *pcs,
return;
}
- state->link = !!(bmsr & BMSR_LSTATUS);
- state->an_complete = !!(bmsr & BMSR_ANEGCOMPLETE);
- if (!state->link)
- return;
-
- switch (state->interface) {
- case PHY_INTERFACE_MODE_1000BASEX:
- phylink_decode_c37_word(state, lpa, SPEED_1000);
- break;
-
- case PHY_INTERFACE_MODE_2500BASEX:
- phylink_decode_c37_word(state, lpa, SPEED_2500);
- break;
-
- case PHY_INTERFACE_MODE_SGMII:
- case PHY_INTERFACE_MODE_QSGMII:
- phylink_decode_sgmii_word(state, lpa);
- break;
-
- default:
- state->link = false;
- break;
- }
+ phylink_mii_c22_pcs_decode_state(state, bmsr, lpa);
}
EXPORT_SYMBOL_GPL(phylink_mii_c22_pcs_get_state);
+/**
+ * phylink_mii_c22_pcs_encode_advertisement() - configure the clause 37 PCS
+ * advertisement
+ * @interface: the PHY interface mode being configured
+ * @advertising: the ethtool advertisement mask
+ * @adv: the value of the %MII_ADVERTISE register
+ *
+ * Helper for MAC PCS supporting the 802.3 clause 22 register set for
+ * clause 37 negotiation and/or SGMII control.
+ *
+ * Encode the clause 37 PCS advertisement as specified by @interface and
+ * @advertising. Callers should write @adv if it has been modified.
+ *
+ * Return: The new value for @adv.
+ */
+u16 phylink_mii_c22_pcs_encode_advertisement(phy_interface_t interface,
+ const unsigned long *advertising,
+ u16 adv)
+{
+ switch (interface) {
+ case PHY_INTERFACE_MODE_1000BASEX:
+ case PHY_INTERFACE_MODE_2500BASEX:
+ adv = ADVERTISE_1000XFULL;
+ if (linkmode_test_bit(ETHTOOL_LINK_MODE_Pause_BIT,
+ advertising))
+ adv |= ADVERTISE_1000XPAUSE;
+ if (linkmode_test_bit(ETHTOOL_LINK_MODE_Asym_Pause_BIT,
+ advertising))
+ adv |= ADVERTISE_1000XPSE_ASYM;
+ return adv;
+ case PHY_INTERFACE_MODE_SGMII:
+ return 0x0001;
+ default:
+ /* Nothing to do for other modes */
+ return adv;
+ }
+}
+
/**
* phylink_mii_c22_pcs_set_advertisement() - configure the clause 37 PCS
* advertisement
@@ -2719,48 +2778,17 @@ int phylink_mii_c22_pcs_set_advertisement(struct mdio_device *pcs,
int val, ret;
u16 adv;
- switch (interface) {
- case PHY_INTERFACE_MODE_1000BASEX:
- case PHY_INTERFACE_MODE_2500BASEX:
- adv = ADVERTISE_1000XFULL;
- if (linkmode_test_bit(ETHTOOL_LINK_MODE_Pause_BIT,
- advertising))
- adv |= ADVERTISE_1000XPAUSE;
- if (linkmode_test_bit(ETHTOOL_LINK_MODE_Asym_Pause_BIT,
- advertising))
- adv |= ADVERTISE_1000XPSE_ASYM;
+ val = mdiobus_read(bus, addr, MII_ADVERTISE);
+ if (val < 0)
+ return val;
- val = mdiobus_read(bus, addr, MII_ADVERTISE);
- if (val < 0)
- return val;
-
- if (val == adv)
- return 0;
-
- ret = mdiobus_write(bus, addr, MII_ADVERTISE, adv);
- if (ret < 0)
- return ret;
-
- return 1;
-
- case PHY_INTERFACE_MODE_SGMII:
- val = mdiobus_read(bus, addr, MII_ADVERTISE);
- if (val < 0)
- return val;
-
- if (val == 0x0001)
- return 0;
-
- ret = mdiobus_write(bus, addr, MII_ADVERTISE, 0x0001);
- if (ret < 0)
- return ret;
-
- return 1;
-
- default:
- /* Nothing to do for other modes */
+ adv = phylink_mii_c22_pcs_encode_advertisement(interface, advertising,
+ adv);
+ if (adv == val)
return 0;
- }
+
+ ret = mdiobus_write(bus, addr, MII_ADVERTISE, adv);
+ return ret < 0 ? ret : 1;
}
EXPORT_SYMBOL_GPL(phylink_mii_c22_pcs_set_advertisement);
diff --git a/include/linux/phylink.h b/include/linux/phylink.h
index bd0ce707d098..b28ed8b569ee 100644
--- a/include/linux/phylink.h
+++ b/include/linux/phylink.h
@@ -496,8 +496,13 @@ int phylink_speed_up(struct phylink *pl);
void phylink_set_port_modes(unsigned long *bits);
void phylink_helper_basex_speed(struct phylink_link_state *state);
+void phylink_mii_c22_pcs_decode_state(struct phylink_link_state *state,
+ u16 bmsr, u16 lpa);
void phylink_mii_c22_pcs_get_state(struct mdio_device *pcs,
struct phylink_link_state *state);
+u16 phylink_mii_c22_pcs_encode_advertisement(phy_interface_t interface,
+ const unsigned long *advertising,
+ u16 adv);
int phylink_mii_c22_pcs_set_advertisement(struct mdio_device *pcs,
phy_interface_t interface,
const unsigned long *advertising);
--
2.25.1
Add a property for associating PCS devices with ethernet controllers.
Because PCS has no generic analogue like PHY, I have left off the
-handle suffix.
Signed-off-by: Sean Anderson <[email protected]>
---
.../devicetree/bindings/net/ethernet-controller.yaml | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/Documentation/devicetree/bindings/net/ethernet-controller.yaml b/Documentation/devicetree/bindings/net/ethernet-controller.yaml
index b0933a8c295a..def95fa6a315 100644
--- a/Documentation/devicetree/bindings/net/ethernet-controller.yaml
+++ b/Documentation/devicetree/bindings/net/ethernet-controller.yaml
@@ -116,6 +116,11 @@ properties:
$ref: "#/properties/phy-handle"
deprecated: true
+ pcs:
+ $ref: /schemas/types.yaml#definitions/phandle
+ description:
+ Specifies a reference to a node representing a PCS device.
+
rx-fifo-depth:
$ref: /schemas/types.yaml#/definitions/uint32
description:
--
2.25.1
On Mon, Oct 04, 2021 at 03:15:12PM -0400, Sean Anderson wrote:
> Add a property for associating PCS devices with ethernet controllers.
> Because PCS has no generic analogue like PHY, I have left off the
> -handle suffix.
For PHYs, we used to have phy and phy-device as property names, but the
modern name is "phy-handle". I think we should do the same here, so I
would suggest using "pcs-handle".
We actually already have LX2160A platforms using "pcs-handle", (see
Documentation/devicetree/bindings/net/fsl,qoriq-mc-dpmac.yaml) so we're
in danger of ending up with multiple property names describing the same
thing.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
On 10/5/21 5:39 AM, Russell King (Oracle) wrote:
> On Mon, Oct 04, 2021 at 03:15:12PM -0400, Sean Anderson wrote:
>> Add a property for associating PCS devices with ethernet controllers.
>> Because PCS has no generic analogue like PHY, I have left off the
>> -handle suffix.
>
> For PHYs, we used to have phy and phy-device as property names, but the
> modern name is "phy-handle". I think we should do the same here, so I
> would suggest using "pcs-handle".
>
> We actually already have LX2160A platforms using "pcs-handle", (see
> Documentation/devicetree/bindings/net/fsl,qoriq-mc-dpmac.yaml) so we're
> in danger of ending up with multiple property names describing the same
> thing.
>
Either way is fine by me.
--Sean
On Tue, Oct 05, 2021 at 10:39:36AM +0100, Russell King (Oracle) wrote:
> On Mon, Oct 04, 2021 at 03:15:12PM -0400, Sean Anderson wrote:
> > Add a property for associating PCS devices with ethernet controllers.
> > Because PCS has no generic analogue like PHY, I have left off the
> > -handle suffix.
>
> For PHYs, we used to have phy and phy-device as property names, but the
> modern name is "phy-handle". I think we should do the same here, so I
> would suggest using "pcs-handle".
On 1G and up ethernet, we have 2 PHYs. There's the external (typically)
ethernet PHY which is what the above properties are for. Then there's
the on-chip serdes PHY similar to SATA, PCIe, etc. which includes the
PCS part. For this part, we should use the generic PHY binding. I think
we already have bindings doing that.
Rob
Hi Rob,
On 10/12/21 9:16 AM, Rob Herring wrote:
> On Tue, Oct 05, 2021 at 10:39:36AM +0100, Russell King (Oracle) wrote:
>> On Mon, Oct 04, 2021 at 03:15:12PM -0400, Sean Anderson wrote:
>> > Add a property for associating PCS devices with ethernet controllers.
>> > Because PCS has no generic analogue like PHY, I have left off the
>> > -handle suffix.
>>
>> For PHYs, we used to have phy and phy-device as property names, but the
>> modern name is "phy-handle". I think we should do the same here, so I
>> would suggest using "pcs-handle".
>
> On 1G and up ethernet, we have 2 PHYs. There's the external (typically)
> ethernet PHY which is what the above properties are for. Then there's
> the on-chip serdes PHY similar to SATA, PCIe, etc. which includes the
> PCS part. For this part, we should use the generic PHY binding. I think
> we already have bindings doing that.
In the 802.3 models, there are several components which convert between
the MII (from the MAC) and the MDI (the physical protocol on the wire).
These are the Physical Coding Sublayer (PCS), Physical Medium Attachment
(PMA) sublayer, and Physical Medium Dependent (PMD) sublayer. The PMD
converts between the physical layer signaling and the on-chip (or
on-board) signalling. The PMA performs clock recovery and converts the
serial data from the PMD into parallel data for the PCS. The PCS handles
autonegotiation, CSMA/CD, and conversion to the apripriate MII for
communicating with the MAC.
In the above model, generic serdes devices generally correspond to the
PMA/PMD sublayers. The PCS is generally a separate device, both
on the hardware and software level. It provides an ethernet-specific
layer on top of the more generic underlying encoding. For this reason,
the PCS should be modeled as its own device, which may then contain a
reference to the appropriate serdes.
The above model describes physical layers such as 1000BASE-X or
10GBASE-X where the PCS/PMA/PMD is the last layer before the physical
medium. In that case, the PCS could be modeled as a traditional PHY.
However, when using (e.g.) SGMII, it is common for the "MDI" to be
SGMII, and for another PHY to convert to 1000BASE-T. To model this
correctly, the PCS/PMA/PMD layer must be considered independently from
the PHY which will ultimately convert the MII to the MDI.
--Sean
On Tue, Oct 12, 2021 at 11:18 AM Sean Anderson <[email protected]> wrote:
>
> Hi Rob,
>
> On 10/12/21 9:16 AM, Rob Herring wrote:
> > On Tue, Oct 05, 2021 at 10:39:36AM +0100, Russell King (Oracle) wrote:
> >> On Mon, Oct 04, 2021 at 03:15:12PM -0400, Sean Anderson wrote:
> >> > Add a property for associating PCS devices with ethernet controllers.
> >> > Because PCS has no generic analogue like PHY, I have left off the
> >> > -handle suffix.
> >>
> >> For PHYs, we used to have phy and phy-device as property names, but the
> >> modern name is "phy-handle". I think we should do the same here, so I
> >> would suggest using "pcs-handle".
> >
> > On 1G and up ethernet, we have 2 PHYs. There's the external (typically)
> > ethernet PHY which is what the above properties are for. Then there's
> > the on-chip serdes PHY similar to SATA, PCIe, etc. which includes the
> > PCS part. For this part, we should use the generic PHY binding. I think
> > we already have bindings doing that.
>
> In the 802.3 models, there are several components which convert between
> the MII (from the MAC) and the MDI (the physical protocol on the wire).
> These are the Physical Coding Sublayer (PCS), Physical Medium Attachment
> (PMA) sublayer, and Physical Medium Dependent (PMD) sublayer. The PMD
> converts between the physical layer signaling and the on-chip (or
> on-board) signalling. The PMA performs clock recovery and converts the
> serial data from the PMD into parallel data for the PCS. The PCS handles
> autonegotiation, CSMA/CD, and conversion to the apripriate MII for
> communicating with the MAC.
>
> In the above model, generic serdes devices generally correspond to the
> PMA/PMD sublayers. The PCS is generally a separate device, both
> on the hardware and software level. It provides an ethernet-specific
> layer on top of the more generic underlying encoding. For this reason,
> the PCS should be modeled as its own device, which may then contain a
> reference to the appropriate serdes.
On the h/w I've worked on, PCS was an additional block instantiated
within the PHY, so it looked like one block to s/w. But that's been
almost 10 years ago now.
If you do have 2 h/w blocks, one option is doing something like this:
phys = <&pcs_phy>, <&sgmii_phy>;
I'm okay with 'pcs-handle', but just want to make sure we're not using
it where 'phys' would work.
> The above model describes physical layers such as 1000BASE-X or
> 10GBASE-X where the PCS/PMA/PMD is the last layer before the physical
> medium. In that case, the PCS could be modeled as a traditional PHY.
> However, when using (e.g.) SGMII, it is common for the "MDI" to be
> SGMII, and for another PHY to convert to 1000BASE-T. To model this
> correctly, the PCS/PMA/PMD layer must be considered independently from
> the PHY which will ultimately convert the MII to the MDI.
On 10/12/21 12:44 PM, Rob Herring wrote:
> On Tue, Oct 12, 2021 at 11:18 AM Sean Anderson <[email protected]> wrote:
>>
>> Hi Rob,
>>
>> On 10/12/21 9:16 AM, Rob Herring wrote:
>> > On Tue, Oct 05, 2021 at 10:39:36AM +0100, Russell King (Oracle) wrote:
>> >> On Mon, Oct 04, 2021 at 03:15:12PM -0400, Sean Anderson wrote:
>> >> > Add a property for associating PCS devices with ethernet controllers.
>> >> > Because PCS has no generic analogue like PHY, I have left off the
>> >> > -handle suffix.
>> >>
>> >> For PHYs, we used to have phy and phy-device as property names, but the
>> >> modern name is "phy-handle". I think we should do the same here, so I
>> >> would suggest using "pcs-handle".
>> >
>> > On 1G and up ethernet, we have 2 PHYs. There's the external (typically)
>> > ethernet PHY which is what the above properties are for. Then there's
>> > the on-chip serdes PHY similar to SATA, PCIe, etc. which includes the
>> > PCS part. For this part, we should use the generic PHY binding. I think
>> > we already have bindings doing that.
>>
>> In the 802.3 models, there are several components which convert between
>> the MII (from the MAC) and the MDI (the physical protocol on the wire).
>> These are the Physical Coding Sublayer (PCS), Physical Medium Attachment
>> (PMA) sublayer, and Physical Medium Dependent (PMD) sublayer. The PMD
>> converts between the physical layer signaling and the on-chip (or
>> on-board) signalling. The PMA performs clock recovery and converts the
>> serial data from the PMD into parallel data for the PCS. The PCS handles
>> autonegotiation, CSMA/CD, and conversion to the apripriate MII for
>> communicating with the MAC.
>>
>> In the above model, generic serdes devices generally correspond to the
>> PMA/PMD sublayers. The PCS is generally a separate device, both
>> on the hardware and software level. It provides an ethernet-specific
>> layer on top of the more generic underlying encoding. For this reason,
>> the PCS should be modeled as its own device, which may then contain a
>> reference to the appropriate serdes.
>
> On the h/w I've worked on, PCS was an additional block instantiated
> within the PHY, so it looked like one block to s/w. But that's been
> almost 10 years ago now.
Well, perhaps the line is not as clear as I made it seem above. The
PCS/PMA/PMD separation is mostly a logical one, so different platforms
divide up the work differently. In addition, the naming may not align
with ethernet's idea of what a PCS or PMA is. For example, on the
ZynqMP, the serdes (GTH) contains its own PCS and PMD (as shown on the
datasheet). However, these blocks both correspond to the PMA layer from
ethernet's POV. The ethernet PCS is a block controlled via registers
within the MAC's address space.
> If you do have 2 h/w blocks, one option is doing something like this:
>
> phys = <&pcs_phy>, <&sgmii_phy>;
Generally, PCSs don't export the same interface as PHYs (see e.g.
drivers/net/pcs and include/linux/phylink.h). IMO it would be an abuse
of the phys property to use it for a PCS as well.
> I'm okay with 'pcs-handle', but just want to make sure we're not using
> it where 'phys' would work.
>
>> The above model describes physical layers such as 1000BASE-X or
>> 10GBASE-X where the PCS/PMA/PMD is the last layer before the physical
>> medium. In that case, the PCS could be modeled as a traditional PHY.
>> However, when using (e.g.) SGMII, it is common for the "MDI" to be
>> SGMII, and for another PHY to convert to 1000BASE-T. To model this
>> correctly, the PCS/PMA/PMD layer must be considered independently from
>> the PHY which will ultimately convert the MII to the MDI.
>
On Mon, Oct 04, 2021 at 03:15:18PM -0400, Sean Anderson wrote:
> Some devices expose memory-mapped c22-compliant PHYs. Because these
> devices do not have an MDIO bus, we cannot use the existing helpers.
> Refactor the existing helpers to allow supplying the values for c22
> registers directly, instead of using MDIO to access them. Only get_state
> and set_adversisement are converted, since they contain the most complex
> logic.
I think this patch is useful on its own, there are certainly cases
where being able to hold the MII bus lock while reading the BMSR/LPA
registers would be advantageous. Please can you update the patch
against net-next and submit it?
Thanks.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!