[RFC,7/7] net: pcs: Drop the TSE PCS driver
Commit Message
Now that we can easily create a mdio-device that represents a
memory-mapped device that exposes an MDIO-like register layout, we don't
need the Altera TSE PCS anymore, since we can use the Lynx PCS instead.
Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
---
MAINTAINERS | 7 --
drivers/net/pcs/Kconfig | 6 --
drivers/net/pcs/Makefile | 1 -
drivers/net/pcs/pcs-altera-tse.c | 160 -------------------------------
include/linux/pcs-altera-tse.h | 17 ----
5 files changed, 191 deletions(-)
delete mode 100644 drivers/net/pcs/pcs-altera-tse.c
delete mode 100644 include/linux/pcs-altera-tse.h
Comments
Hi Maxime,
On Fri, Mar 24, 2023 at 10:36:44AM +0100, Maxime Chevallier wrote:
> Now that we can easily create a mdio-device that represents a
> memory-mapped device that exposes an MDIO-like register layout, we don't
> need the Altera TSE PCS anymore, since we can use the Lynx PCS instead.
>
> Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
> ---
> -static int tse_pcs_reset(struct altera_tse_pcs *tse_pcs)
> -{
> - u16 bmcr;
> -
> - /* Reset PCS block */
> - bmcr = tse_pcs_read(tse_pcs, MII_BMCR);
> - bmcr |= BMCR_RESET;
> - tse_pcs_write(tse_pcs, MII_BMCR, bmcr);
> -
> - return read_poll_timeout(tse_pcs_read, bmcr, (bmcr & BMCR_RESET),
> - 10, SGMII_PCS_SW_RESET_TIMEOUT, 1,
> - tse_pcs, MII_BMCR);
> -}
I just noticed this difference between the Lynx PCS and Altera TSE PCS
drivers. The Lynx driver doesn't reset the PCS, and if it did, it would
wait until the opposite condition from yours would be true: BMCR_RESET
should clear: "!(bmcr & BMCR_RESET)".
Is your reset procedure correct, I wonder?
Hello Vlad,
On Mon, 15 May 2023 14:48:09 +0300
Vladimir Oltean <vladimir.oltean@nxp.com> wrote:
> Hi Maxime,
>
> On Fri, Mar 24, 2023 at 10:36:44AM +0100, Maxime Chevallier wrote:
> > Now that we can easily create a mdio-device that represents a
> > memory-mapped device that exposes an MDIO-like register layout, we
> > don't need the Altera TSE PCS anymore, since we can use the Lynx
> > PCS instead.
> >
> > Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
> > ---
> > -static int tse_pcs_reset(struct altera_tse_pcs *tse_pcs)
> > -{
> > - u16 bmcr;
> > -
> > - /* Reset PCS block */
> > - bmcr = tse_pcs_read(tse_pcs, MII_BMCR);
> > - bmcr |= BMCR_RESET;
> > - tse_pcs_write(tse_pcs, MII_BMCR, bmcr);
> > -
> > - return read_poll_timeout(tse_pcs_read, bmcr, (bmcr &
> > BMCR_RESET),
> > - 10, SGMII_PCS_SW_RESET_TIMEOUT, 1,
> > - tse_pcs, MII_BMCR);
> > -}
>
> I just noticed this difference between the Lynx PCS and Altera TSE PCS
> drivers. The Lynx driver doesn't reset the PCS, and if it did, it
> would wait until the opposite condition from yours would be true:
> BMCR_RESET should clear: "!(bmcr & BMCR_RESET)".
>
> Is your reset procedure correct, I wonder?
I also wonder... My understanding from the documentation I had is that
the PCS needed to be reset every time a major configuration was
changed, as the reset would put the internal state machines back to
their init condition, but without resetting the whole IP.
When performing my initial tests, this was indeed necessary. However
when porting to Lynx and running tests, it pretty much worked out of the
box, and I'm quite happy with it being that way. So I didn't dig much
deeper on why the reset was needed with the TSE PCS in the first place.
On a side note, the final dependencies for the final removal of TSE
were merged into the regmap tree, I'll therefore send a new version of
that series shortly.
Thanks for the review Vlad,
Best Regards,
Maxime
@@ -909,13 +909,6 @@ L: netdev@vger.kernel.org
S: Maintained
F: drivers/net/ethernet/altera/
-ALTERA TSE PCS
-M: Maxime Chevallier <maxime.chevallier@bootlin.com>
-L: netdev@vger.kernel.org
-S: Supported
-F: drivers/net/pcs/pcs-altera-tse.c
-F: include/linux/pcs-altera-tse.h
-
ALTERA UART/JTAG UART SERIAL DRIVERS
M: Tobias Klauser <tklauser@distanz.ch>
L: linux-serial@vger.kernel.org
@@ -26,10 +26,4 @@ config PCS_RZN1_MIIC
on RZ/N1 SoCs. This PCS converts MII to RMII/RGMII or can be set in
pass-through mode for MII.
-config PCS_ALTERA_TSE
- tristate
- help
- This module provides helper functions for the Altera Triple Speed
- Ethernet SGMII PCS, that can be found on the Intel Socfpga family.
-
endmenu
@@ -6,4 +6,3 @@ pcs_xpcs-$(CONFIG_PCS_XPCS) := pcs-xpcs.o pcs-xpcs-nxp.o
obj-$(CONFIG_PCS_XPCS) += pcs_xpcs.o
obj-$(CONFIG_PCS_LYNX) += pcs-lynx.o
obj-$(CONFIG_PCS_RZN1_MIIC) += pcs-rzn1-miic.o
-obj-$(CONFIG_PCS_ALTERA_TSE) += pcs-altera-tse.o
deleted file mode 100644
@@ -1,160 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0
-/*
- * Copyright (C) 2022 Bootlin
- *
- * Maxime Chevallier <maxime.chevallier@bootlin.com>
- */
-
-#include <linux/netdevice.h>
-#include <linux/phy.h>
-#include <linux/phylink.h>
-#include <linux/pcs-altera-tse.h>
-
-/* SGMII PCS register addresses
- */
-#define SGMII_PCS_LINK_TIMER_0 0x12
-#define SGMII_PCS_LINK_TIMER_1 0x13
-#define SGMII_PCS_IF_MODE 0x14
-#define PCS_IF_MODE_SGMII_ENA BIT(0)
-#define PCS_IF_MODE_USE_SGMII_AN BIT(1)
-#define PCS_IF_MODE_SGMI_HALF_DUPLEX BIT(4)
-#define PCS_IF_MODE_SGMI_PHY_AN BIT(5)
-#define SGMII_PCS_SW_RESET_TIMEOUT 100 /* usecs */
-
-struct altera_tse_pcs {
- struct phylink_pcs pcs;
- void __iomem *base;
- int reg_width;
-};
-
-static struct altera_tse_pcs *phylink_pcs_to_tse_pcs(struct phylink_pcs *pcs)
-{
- return container_of(pcs, struct altera_tse_pcs, pcs);
-}
-
-static u16 tse_pcs_read(struct altera_tse_pcs *tse_pcs, int regnum)
-{
- if (tse_pcs->reg_width == 4)
- return readl(tse_pcs->base + regnum * 4);
- else
- return readw(tse_pcs->base + regnum * 2);
-}
-
-static void tse_pcs_write(struct altera_tse_pcs *tse_pcs, int regnum,
- u16 value)
-{
- if (tse_pcs->reg_width == 4)
- writel(value, tse_pcs->base + regnum * 4);
- else
- writew(value, tse_pcs->base + regnum * 2);
-}
-
-static int tse_pcs_reset(struct altera_tse_pcs *tse_pcs)
-{
- u16 bmcr;
-
- /* Reset PCS block */
- bmcr = tse_pcs_read(tse_pcs, MII_BMCR);
- bmcr |= BMCR_RESET;
- tse_pcs_write(tse_pcs, MII_BMCR, bmcr);
-
- return read_poll_timeout(tse_pcs_read, bmcr, (bmcr & BMCR_RESET),
- 10, SGMII_PCS_SW_RESET_TIMEOUT, 1,
- tse_pcs, MII_BMCR);
-}
-
-static int alt_tse_pcs_validate(struct phylink_pcs *pcs,
- unsigned long *supported,
- const struct phylink_link_state *state)
-{
- if (state->interface == PHY_INTERFACE_MODE_SGMII ||
- state->interface == PHY_INTERFACE_MODE_1000BASEX)
- return 1;
-
- return -EINVAL;
-}
-
-static int alt_tse_pcs_config(struct phylink_pcs *pcs, unsigned int mode,
- phy_interface_t interface,
- const unsigned long *advertising,
- bool permit_pause_to_mac)
-{
- struct altera_tse_pcs *tse_pcs = phylink_pcs_to_tse_pcs(pcs);
- u32 ctrl, if_mode;
-
- ctrl = tse_pcs_read(tse_pcs, MII_BMCR);
- if_mode = tse_pcs_read(tse_pcs, SGMII_PCS_IF_MODE);
-
- /* Set link timer to 1.6ms, as per the MegaCore Function User Guide */
- tse_pcs_write(tse_pcs, SGMII_PCS_LINK_TIMER_0, 0x0D40);
- tse_pcs_write(tse_pcs, SGMII_PCS_LINK_TIMER_1, 0x03);
-
- if (interface == PHY_INTERFACE_MODE_SGMII) {
- if_mode |= PCS_IF_MODE_USE_SGMII_AN | PCS_IF_MODE_SGMII_ENA;
- } else if (interface == PHY_INTERFACE_MODE_1000BASEX) {
- if_mode &= ~(PCS_IF_MODE_USE_SGMII_AN | PCS_IF_MODE_SGMII_ENA);
- }
-
- ctrl |= (BMCR_SPEED1000 | BMCR_FULLDPLX | BMCR_ANENABLE);
-
- tse_pcs_write(tse_pcs, MII_BMCR, ctrl);
- tse_pcs_write(tse_pcs, SGMII_PCS_IF_MODE, if_mode);
-
- return tse_pcs_reset(tse_pcs);
-}
-
-static void alt_tse_pcs_get_state(struct phylink_pcs *pcs,
- struct phylink_link_state *state)
-{
- struct altera_tse_pcs *tse_pcs = phylink_pcs_to_tse_pcs(pcs);
- u16 bmsr, lpa;
-
- bmsr = tse_pcs_read(tse_pcs, MII_BMSR);
- lpa = tse_pcs_read(tse_pcs, MII_LPA);
-
- phylink_mii_c22_pcs_decode_state(state, bmsr, lpa);
-}
-
-static void alt_tse_pcs_an_restart(struct phylink_pcs *pcs)
-{
- struct altera_tse_pcs *tse_pcs = phylink_pcs_to_tse_pcs(pcs);
- u16 bmcr;
-
- bmcr = tse_pcs_read(tse_pcs, MII_BMCR);
- bmcr |= BMCR_ANRESTART;
- tse_pcs_write(tse_pcs, MII_BMCR, bmcr);
-
- /* This PCS seems to require a soft reset to re-sync the AN logic */
- tse_pcs_reset(tse_pcs);
-}
-
-static const struct phylink_pcs_ops alt_tse_pcs_ops = {
- .pcs_validate = alt_tse_pcs_validate,
- .pcs_get_state = alt_tse_pcs_get_state,
- .pcs_config = alt_tse_pcs_config,
- .pcs_an_restart = alt_tse_pcs_an_restart,
-};
-
-struct phylink_pcs *alt_tse_pcs_create(struct net_device *ndev,
- void __iomem *pcs_base, int reg_width)
-{
- struct altera_tse_pcs *tse_pcs;
-
- if (reg_width != 4 && reg_width != 2)
- return ERR_PTR(-EINVAL);
-
- tse_pcs = devm_kzalloc(&ndev->dev, sizeof(*tse_pcs), GFP_KERNEL);
- if (!tse_pcs)
- return ERR_PTR(-ENOMEM);
-
- tse_pcs->pcs.ops = &alt_tse_pcs_ops;
- tse_pcs->base = pcs_base;
- tse_pcs->reg_width = reg_width;
-
- return &tse_pcs->pcs;
-}
-EXPORT_SYMBOL_GPL(alt_tse_pcs_create);
-
-MODULE_LICENSE("GPL");
-MODULE_DESCRIPTION("Altera TSE PCS driver");
-MODULE_AUTHOR("Maxime Chevallier <maxime.chevallier@bootlin.com>");
deleted file mode 100644
@@ -1,17 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0 */
-/*
- * Copyright (C) 2022 Bootlin
- *
- * Maxime Chevallier <maxime.chevallier@bootlin.com>
- */
-
-#ifndef __LINUX_PCS_ALTERA_TSE_H
-#define __LINUX_PCS_ALTERA_TSE_H
-
-struct phylink_pcs;
-struct net_device;
-
-struct phylink_pcs *alt_tse_pcs_create(struct net_device *ndev,
- void __iomem *pcs_base, int reg_width);
-
-#endif /* __LINUX_PCS_ALTERA_TSE_H */