On Fri, Aug 26, 2022 at 03:54:49PM +0200, Maxime Chevallier wrote:
> +
> +/* SGMII PCS register addresses
> + */
> +#define SGMII_PCS_SCRATCH 0x10
> +#define SGMII_PCS_REV 0x11
> +#define SGMII_PCS_LINK_TIMER_0 0x12
> +#define SGMII_PCS_LINK_TIMER_REG(x) (0x12 + (x))
> +#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_SPEED_MASK GENMASK(3, 2)
> +#define PCS_IF_MODE_SGMI_SPEED_10 (0 << 2)
> +#define PCS_IF_MODE_SGMI_SPEED_100 (1 << 2)
> +#define PCS_IF_MODE_SGMI_SPEED_1000 (2 << 2)
> +#define PCS_IF_MODE_SGMI_HALF_DUPLEX BIT(4)
> +#define PCS_IF_MODE_SGMI_PHY_AN BIT(5)
This looks very similar to pcs-lynx's register layout. I wonder if it's
the same underlying hardware.
> +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;
> +
> + if (interface != PHY_INTERFACE_MODE_SGMII &&
> + interface != PHY_INTERFACE_MODE_1000BASEX)
> + return 0;
I would suggest doing this check in .pcs_validate() to catch anyone
attaching the PCS with an unsupported interface mode.
> +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);
> +
> + tse_pcs_reset(tse_pcs);
Any ideas why a reset is necessary after setting BMCR_ANRESTART?
Normally, this is not required.
> diff --git a/include/linux/pcs-altera-tse.h b/include/linux/pcs-altera-tse.h
> new file mode 100644
> index 000000000000..9c85e7c8ef70
> --- /dev/null
> +++ b/include/linux/pcs-altera-tse.h
> @@ -0,0 +1,17 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (C) 2022 Bootlin
> + *
> + * Maxime Chevallier <maxime.chevallier@xxxxxxxxxxx>
> + */
> +
> +#ifndef __LINUX_PCS_ALTERA_TSE_H
> +#define __LINUX_PCS_ALTERA_TSE_H
> +
> +struct phylink;
Don't you want "struct phylink_pcs;" here?
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
