On 9/30/22 4:09 PM, Sean Anderson wrote:
> Although not stated in the datasheet, as far as I can tell PCS for mEMACs
> is a "Lynx." By reusing the existing driver, we can remove the PCS
> management code from the memac driver. This requires calling some PCS
> functions manually which phylink would usually do for us, but we will let
> it do that soon.
>
> One problem is that we don't actually have a PCS for QSGMII. We pretend
> that each mEMAC's MDIO bus has four QSGMII PCSs, but this is not the case.
> Only the "base" mEMAC's MDIO bus has the four QSGMII PCSs. This is not an
> issue yet, because we never get the PCS state. However, it will be once the
> conversion to phylink is complete, since the links will appear to never
> come up. To get around this, we allow specifying multiple PCSs in pcsphy.
> This breaks backwards compatibility with old device trees, but only for
> QSGMII. IMO this is the only reasonable way to figure out what the actual
> QSGMII PCS is.
>
> Additionally, we now also support a separate XFI PCS. This can allow the
> SerDes driver to set different addresses for the SGMII and XFI PCSs so they
> can be accessed at the same time.
>
> Signed-off-by: Sean Anderson <sean.ander...@seco.com>
> ---
>
> Changes in v6:
> - Fix 81-character line
>
> Changes in v3:
> - Put the PCS mdiodev only after we are done with it (since the PCS
> does not perform a get itself).
>
> Changes in v2:
> - Move PCS_LYNX dependency to fman Kconfig
>
> drivers/net/ethernet/freescale/fman/Kconfig | 3 +
> .../net/ethernet/freescale/fman/fman_memac.c | 258 +++++++-----------
> 2 files changed, 105 insertions(+), 156 deletions(-)
>
> diff --git a/drivers/net/ethernet/freescale/fman/Kconfig
> b/drivers/net/ethernet/freescale/fman/Kconfig
> index 48bf8088795d..8f5637db41dd 100644
> --- a/drivers/net/ethernet/freescale/fman/Kconfig
> +++ b/drivers/net/ethernet/freescale/fman/Kconfig
> @@ -4,6 +4,9 @@ config FSL_FMAN
> depends on FSL_SOC || ARCH_LAYERSCAPE || COMPILE_TEST
> select GENERIC_ALLOCATOR
> select PHYLIB
> + select PHYLINK
> + select PCS
> + select PCS_LYNX
> select CRC32
> default n
> help
> diff --git a/drivers/net/ethernet/freescale/fman/fman_memac.c
> b/drivers/net/ethernet/freescale/fman/fman_memac.c
> index 56a29f505590..eeb71352603b 100644
> --- a/drivers/net/ethernet/freescale/fman/fman_memac.c
> +++ b/drivers/net/ethernet/freescale/fman/fman_memac.c
> @@ -11,43 +11,12 @@
>
> #include <linux/slab.h>
> #include <linux/io.h>
> +#include <linux/pcs-lynx.h>
> #include <linux/phy.h>
> #include <linux/phy_fixed.h>
> #include <linux/phy/phy.h>
> #include <linux/of_mdio.h>
>
> -/* PCS registers */
> -#define MDIO_SGMII_CR 0x00
> -#define MDIO_SGMII_DEV_ABIL_SGMII 0x04
> -#define MDIO_SGMII_LINK_TMR_L 0x12
> -#define MDIO_SGMII_LINK_TMR_H 0x13
> -#define MDIO_SGMII_IF_MODE 0x14
> -
> -/* SGMII Control defines */
> -#define SGMII_CR_AN_EN 0x1000
> -#define SGMII_CR_RESTART_AN 0x0200
> -#define SGMII_CR_FD 0x0100
> -#define SGMII_CR_SPEED_SEL1_1G 0x0040
> -#define SGMII_CR_DEF_VAL (SGMII_CR_AN_EN | SGMII_CR_FD | \
> - SGMII_CR_SPEED_SEL1_1G)
> -
> -/* SGMII Device Ability for SGMII defines */
> -#define MDIO_SGMII_DEV_ABIL_SGMII_MODE 0x4001
> -#define MDIO_SGMII_DEV_ABIL_BASEX_MODE 0x01A0
> -
> -/* Link timer define */
> -#define LINK_TMR_L 0xa120
> -#define LINK_TMR_H 0x0007
> -#define LINK_TMR_L_BASEX 0xaf08
> -#define LINK_TMR_H_BASEX 0x002f
> -
> -/* SGMII IF Mode defines */
> -#define IF_MODE_USE_SGMII_AN 0x0002
> -#define IF_MODE_SGMII_EN 0x0001
> -#define IF_MODE_SGMII_SPEED_100M 0x0004
> -#define IF_MODE_SGMII_SPEED_1G 0x0008
> -#define IF_MODE_SGMII_DUPLEX_HALF 0x0010
> -
> /* Num of additional exact match MAC adr regs */
> #define MEMAC_NUM_OF_PADDRS 7
>
> @@ -326,7 +295,9 @@ struct fman_mac {
> struct fman_rev_info fm_rev_info;
> bool basex_if;
> struct phy *serdes;
> - struct phy_device *pcsphy;
> + struct phylink_pcs *sgmii_pcs;
> + struct phylink_pcs *qsgmii_pcs;
> + struct phylink_pcs *xfi_pcs;
> bool allmulti_enabled;
> };
>
> @@ -487,91 +458,22 @@ static u32 get_mac_addr_hash_code(u64 eth_addr)
> return xor_val;
> }
>
> -static void setup_sgmii_internal_phy(struct fman_mac *memac,
> - struct fixed_phy_status *fixed_link)
> +static void setup_sgmii_internal(struct fman_mac *memac,
> + struct phylink_pcs *pcs,
> + struct fixed_phy_status *fixed_link)
> {
> - u16 tmp_reg16;
> -
> - if (WARN_ON(!memac->pcsphy))
> - return;
> -
> - /* SGMII mode */
> - tmp_reg16 = IF_MODE_SGMII_EN;
> - if (!fixed_link)
> - /* AN enable */
> - tmp_reg16 |= IF_MODE_USE_SGMII_AN;
> - else {
> - switch (fixed_link->speed) {
> - case 10:
> - /* For 10M: IF_MODE[SPEED_10M] = 0 */
> - break;
> - case 100:
> - tmp_reg16 |= IF_MODE_SGMII_SPEED_100M;
> - break;
> - case 1000:
> - default:
> - tmp_reg16 |= IF_MODE_SGMII_SPEED_1G;
> - break;
> - }
> - if (!fixed_link->duplex)
> - tmp_reg16 |= IF_MODE_SGMII_DUPLEX_HALF;
> - }
> - phy_write(memac->pcsphy, MDIO_SGMII_IF_MODE, tmp_reg16);
> -
> - /* Device ability according to SGMII specification */
> - tmp_reg16 = MDIO_SGMII_DEV_ABIL_SGMII_MODE;
> - phy_write(memac->pcsphy, MDIO_SGMII_DEV_ABIL_SGMII, tmp_reg16);
> -
> - /* Adjust link timer for SGMII -
> - * According to Cisco SGMII specification the timer should be 1.6 ms.
> - * The link_timer register is configured in units of the clock.
> - * - When running as 1G SGMII, Serdes clock is 125 MHz, so
> - * unit = 1 / (125*10^6 Hz) = 8 ns.
> - * 1.6 ms in units of 8 ns = 1.6ms / 8ns = 2*10^5 = 0x30d40
> - * - When running as 2.5G SGMII, Serdes clock is 312.5 MHz, so
> - * unit = 1 / (312.5*10^6 Hz) = 3.2 ns.
> - * 1.6 ms in units of 3.2 ns = 1.6ms / 3.2ns = 5*10^5 = 0x7a120.
> - * Since link_timer value of 1G SGMII will be too short for 2.5 SGMII,
> - * we always set up here a value of 2.5 SGMII.
> - */
> - phy_write(memac->pcsphy, MDIO_SGMII_LINK_TMR_H, LINK_TMR_H);
> - phy_write(memac->pcsphy, MDIO_SGMII_LINK_TMR_L, LINK_TMR_L);
> -
> - if (!fixed_link)
> - /* Restart AN */
> - tmp_reg16 = SGMII_CR_DEF_VAL | SGMII_CR_RESTART_AN;
> + __ETHTOOL_DECLARE_LINK_MODE_MASK(advertising);
> + phy_interface_t iface = memac->basex_if ? PHY_INTERFACE_MODE_1000BASEX :
> + PHY_INTERFACE_MODE_SGMII;
> + unsigned int mode = fixed_link ? MLO_AN_FIXED : MLO_AN_INBAND;
> +
> + linkmode_set_pause(advertising, true, true);
> + pcs->ops->pcs_config(pcs, mode, iface, advertising, true);
> + if (fixed_link)
> + pcs->ops->pcs_link_up(pcs, mode, iface, fixed_link->speed,
> + fixed_link->duplex);
> else
> - /* AN disabled */
> - tmp_reg16 = SGMII_CR_DEF_VAL & ~SGMII_CR_AN_EN;
> - phy_write(memac->pcsphy, 0x0, tmp_reg16);
> -}
> -
> -static void setup_sgmii_internal_phy_base_x(struct fman_mac *memac)
> -{
> - u16 tmp_reg16;
> -
> - /* AN Device capability */
> - tmp_reg16 = MDIO_SGMII_DEV_ABIL_BASEX_MODE;
> - phy_write(memac->pcsphy, MDIO_SGMII_DEV_ABIL_SGMII, tmp_reg16);
> -
> - /* Adjust link timer for SGMII -
> - * For Serdes 1000BaseX auto-negotiation the timer should be 10 ms.
> - * The link_timer register is configured in units of the clock.
> - * - When running as 1G SGMII, Serdes clock is 125 MHz, so
> - * unit = 1 / (125*10^6 Hz) = 8 ns.
> - * 10 ms in units of 8 ns = 10ms / 8ns = 1250000 = 0x1312d0
> - * - When running as 2.5G SGMII, Serdes clock is 312.5 MHz, so
> - * unit = 1 / (312.5*10^6 Hz) = 3.2 ns.
> - * 10 ms in units of 3.2 ns = 10ms / 3.2ns = 3125000 = 0x2faf08.
> - * Since link_timer value of 1G SGMII will be too short for 2.5 SGMII,
> - * we always set up here a value of 2.5 SGMII.
> - */
> - phy_write(memac->pcsphy, MDIO_SGMII_LINK_TMR_H, LINK_TMR_H_BASEX);
> - phy_write(memac->pcsphy, MDIO_SGMII_LINK_TMR_L, LINK_TMR_L_BASEX);
> -
> - /* Restart AN */
> - tmp_reg16 = SGMII_CR_DEF_VAL | SGMII_CR_RESTART_AN;
> - phy_write(memac->pcsphy, 0x0, tmp_reg16);
> + pcs->ops->pcs_an_restart(pcs);
> }
>
> static int check_init_parameters(struct fman_mac *memac)
> @@ -983,7 +885,6 @@ static int memac_set_exception(struct fman_mac *memac,
> static int memac_init(struct fman_mac *memac)
> {
> struct memac_cfg *memac_drv_param;
> - u8 i;
> enet_addr_t eth_addr;
> bool slow_10g_if = false;
> struct fixed_phy_status *fixed_link = NULL;
> @@ -1036,32 +937,10 @@ static int memac_init(struct fman_mac *memac)
> iowrite32be(reg32, &memac->regs->command_config);
> }
>
> - if (memac->phy_if == PHY_INTERFACE_MODE_SGMII) {
> - /* Configure internal SGMII PHY */
> - if (memac->basex_if)
> - setup_sgmii_internal_phy_base_x(memac);
> - else
> - setup_sgmii_internal_phy(memac, fixed_link);
> - } else if (memac->phy_if == PHY_INTERFACE_MODE_QSGMII) {
> - /* Configure 4 internal SGMII PHYs */
> - for (i = 0; i < 4; i++) {
> - u8 qsmgii_phy_addr, phy_addr;
> - /* QSGMII PHY address occupies 3 upper bits of 5-bit
> - * phy_address; the lower 2 bits are used to extend
> - * register address space and access each one of 4
> - * ports inside QSGMII.
> - */
> - phy_addr = memac->pcsphy->mdio.addr;
> - qsmgii_phy_addr = (u8)((phy_addr << 2) | i);
> - memac->pcsphy->mdio.addr = qsmgii_phy_addr;
> - if (memac->basex_if)
> - setup_sgmii_internal_phy_base_x(memac);
> - else
> - setup_sgmii_internal_phy(memac, fixed_link);
> -
> - memac->pcsphy->mdio.addr = phy_addr;
> - }
> - }
> + if (memac->phy_if == PHY_INTERFACE_MODE_SGMII)
> + setup_sgmii_internal(memac, memac->sgmii_pcs, fixed_link);
> + else if (memac->phy_if == PHY_INTERFACE_MODE_QSGMII)
> + setup_sgmii_internal(memac, memac->qsgmii_pcs, fixed_link);
>
> /* Max Frame Length */
> err = fman_set_mac_max_frame(memac->fm, memac->mac_id,
> @@ -1097,12 +976,25 @@ static int memac_init(struct fman_mac *memac)
> return 0;
> }
>
> +static void pcs_put(struct phylink_pcs *pcs)
> +{
> + struct mdio_device *mdiodev;
> +
> + if (!pcs)
This should be IS_ERR_OR_NULL. Will fix for next spin.
--Sean
> + return;
> +
> + mdiodev = lynx_get_mdio_device(pcs);
> + lynx_pcs_destroy(pcs);
> + mdio_device_free(mdiodev);
> +}
> +
> static int memac_free(struct fman_mac *memac)
> {
> free_init_resources(memac);
>
> - if (memac->pcsphy)
> - put_device(&memac->pcsphy->mdio.dev);
> + pcs_put(memac->sgmii_pcs);
> + pcs_put(memac->qsgmii_pcs);
> + pcs_put(memac->xfi_pcs);
>
> kfree(memac->memac_drv_param);
> kfree(memac);
> @@ -1153,12 +1045,31 @@ static struct fman_mac *memac_config(struct
> mac_device *mac_dev,
> return memac;
> }
>
> +static struct phylink_pcs *memac_pcs_create(struct device_node *mac_node,
> + int index)
> +{
> + struct device_node *node;
> + struct mdio_device *mdiodev = NULL;
> + struct phylink_pcs *pcs;
> +
> + node = of_parse_phandle(mac_node, "pcsphy-handle", index);
> + if (node && of_device_is_available(node))
> + mdiodev = of_mdio_find_device(node);
> + of_node_put(node);
> +
> + if (!mdiodev)
> + return ERR_PTR(-EPROBE_DEFER);
> +
> + pcs = lynx_pcs_create(mdiodev);
> + return pcs;
> +}
> +
> int memac_initialization(struct mac_device *mac_dev,
> struct device_node *mac_node,
> struct fman_mac_params *params)
> {
> int err;
> - struct device_node *phy_node;
> + struct phylink_pcs *pcs;
> struct fixed_phy_status *fixed_link;
> struct fman_mac *memac;
>
> @@ -1188,23 +1099,58 @@ int memac_initialization(struct mac_device *mac_dev,
> memac = mac_dev->fman_mac;
> memac->memac_drv_param->max_frame_length = fman_get_max_frm();
> memac->memac_drv_param->reset_on_init = true;
> - if (memac->phy_if == PHY_INTERFACE_MODE_SGMII ||
> - memac->phy_if == PHY_INTERFACE_MODE_QSGMII) {
> - phy_node = of_parse_phandle(mac_node, "pcsphy-handle", 0);
> - if (!phy_node) {
> - pr_err("PCS PHY node is not available\n");
> - err = -EINVAL;
> +
> + err = of_property_match_string(mac_node, "pcs-names", "xfi");
> + if (err >= 0) {
> + memac->xfi_pcs = memac_pcs_create(mac_node, err);
> + if (IS_ERR(memac->xfi_pcs)) {
> + err = PTR_ERR(memac->xfi_pcs);
> + dev_err_probe(mac_dev->dev, err, "missing xfi pcs\n");
> goto _return_fm_mac_free;
> }
> + } else if (err != -EINVAL && err != -ENODATA) {
> + goto _return_fm_mac_free;
> + }
>
> - memac->pcsphy = of_phy_find_device(phy_node);
> - if (!memac->pcsphy) {
> - pr_err("of_phy_find_device (PCS PHY) failed\n");
> - err = -EINVAL;
> + err = of_property_match_string(mac_node, "pcs-names", "qsgmii");
> + if (err >= 0) {
> + memac->qsgmii_pcs = memac_pcs_create(mac_node, err);
> + if (IS_ERR(memac->qsgmii_pcs)) {
> + err = PTR_ERR(memac->qsgmii_pcs);
> + dev_err_probe(mac_dev->dev, err,
> + "missing qsgmii pcs\n");
> goto _return_fm_mac_free;
> }
> + } else if (err != -EINVAL && err != -ENODATA) {
> + goto _return_fm_mac_free;
> + }
> +
> + /* For compatibility, if pcs-names is missing, we assume this phy is
> + * the first one in pcsphy-handle
> + */
> + err = of_property_match_string(mac_node, "pcs-names", "sgmii");
> + if (err == -EINVAL)
> + pcs = memac_pcs_create(mac_node, 0);
> + else if (err < 0)
> + goto _return_fm_mac_free;
> + else
> + pcs = memac_pcs_create(mac_node, err);
> +
> + if (!pcs) {
> + dev_err(mac_dev->dev, "missing pcs\n");
> + err = -ENOENT;
> + goto _return_fm_mac_free;
> }
>
> + /* If err is set here, it means that pcs-names was missing above (and
> + * therefore that xfi_pcs cannot be set). If we are defaulting to
> + * XGMII, assume this is for XFI. Otherwise, assume it is for SGMII.
> + */
> + if (err && mac_dev->phy_if == PHY_INTERFACE_MODE_XGMII)
> + memac->xfi_pcs = pcs;
> + else
> + memac->sgmii_pcs = pcs;
> +
> memac->serdes = devm_of_phy_get(mac_dev->dev, mac_node, "serdes");
> err = PTR_ERR(memac->serdes);
> if (err == -ENODEV || err == -ENOSYS) {
>
