On Tue, Jun 11, 2019 at 05:18:46PM +0200, Jose Abreu wrote:
> Start adding the phylink callbacks.
>
> Signed-off-by: Jose Abreu <[email protected]>
> Cc: Joao Pinto <[email protected]>
> Cc: David S. Miller <[email protected]>
> Cc: Giuseppe Cavallaro <[email protected]>
> Cc: Alexandre Torgue <[email protected]>
> Cc: Russell King <[email protected]>
> Cc: Andrew Lunn <[email protected]>
> Cc: Florian Fainelli <[email protected]>
> Cc: Heiner Kallweit <[email protected]>
> ---
> drivers/net/ethernet/stmicro/stmmac/Kconfig | 1 +
> drivers/net/ethernet/stmicro/stmmac/stmmac.h | 4 ++
> .../net/ethernet/stmicro/stmmac/stmmac_main.c | 48 +++++++++++++++++++
> 3 files changed, 53 insertions(+)
>
> diff --git a/drivers/net/ethernet/stmicro/stmmac/Kconfig
> b/drivers/net/ethernet/stmicro/stmmac/Kconfig
> index 0b5c8d74c683..cf0c9f4f347a 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/Kconfig
> +++ b/drivers/net/ethernet/stmicro/stmmac/Kconfig
> @@ -4,6 +4,7 @@ config STMMAC_ETH
> depends on HAS_IOMEM && HAS_DMA
> select MII
> select PHYLIB
> + select PHYLINK
Please replace PHYLIB with PHYLINK here, there's no need to select both.
> select CRC32
> imply PTP_1588_CLOCK
> select RESET_CONTROLLER
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac.h
> b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
> index a16ada8b8507..b8386778f6c6 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac.h
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
> @@ -25,6 +25,7 @@
> #include <linux/clk.h>
> #include <linux/stmmac.h>
> #include <linux/phy.h>
> +#include <linux/phylink.h>
linux/phy.h is unnecessary when you include phylink.h
> #include <linux/pci.h>
> #include "common.h"
> #include <linux/ptp_clock_kernel.h>
> @@ -155,6 +156,9 @@ struct stmmac_priv {
> struct mii_bus *mii;
> int mii_irq[PHY_MAX_ADDR];
>
> + struct phylink_config phylink_config;
> + struct phylink *phylink;
> +
> struct stmmac_extra_stats xstats ____cacheline_aligned_in_smp;
> struct stmmac_safety_stats sstats;
> struct plat_stmmacenet_data *plat;
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index 6a2f072c0ce3..e2e69cb08fef 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -45,6 +45,7 @@
> #include <linux/seq_file.h>
> #endif /* CONFIG_DEBUG_FS */
> #include <linux/net_tstamp.h>
> +#include <linux/phylink.h>
> #include <net/pkt_cls.h>
> #include "stmmac_ptp.h"
> #include "stmmac.h"
> @@ -848,6 +849,39 @@ static void stmmac_mac_flow_ctrl(struct stmmac_priv
> *priv, u32 duplex)
> priv->pause, tx_cnt);
> }
>
> +static void stmmac_validate(struct phylink_config *config,
> + unsigned long *supported,
> + struct phylink_link_state *state)
> +{
> + struct stmmac_priv *priv = netdev_priv(to_net_dev(config->dev));
> + __ETHTOOL_DECLARE_LINK_MODE_MASK(mask) = { 0, };
> + int tx_cnt = priv->plat->tx_queues_to_use;
> + int max_speed = priv->plat->max_speed;
> +
> + /* Cut down 1G if asked to */
> + if ((max_speed > 0) && (max_speed < 1000)) {
> + phylink_set(mask, 1000baseT_Full);
> + phylink_set(mask, 1000baseX_Full);
> + }
> +
> + /* Half-Duplex can only work with single queue */
> + if (tx_cnt > 1) {
> + phylink_set(mask, 10baseT_Half);
> + phylink_set(mask, 100baseT_Half);
> + phylink_set(mask, 1000baseT_Half);
> + }
The logic here looks a little weird - if max_speed is less than 1000, we
can end up with 1000baseT-HD enabled. Surely this is not desirable.
> +
> + bitmap_andnot(supported, supported, mask,
> __ETHTOOL_LINK_MODE_MASK_NBITS);
> + bitmap_andnot(state->advertising, state->advertising, mask,
> + __ETHTOOL_LINK_MODE_MASK_NBITS);
> +}
> +
> +static int stmmac_mac_link_state(struct phylink_config *config,
> + struct phylink_link_state *state)
> +{
> + return -EOPNOTSUPP;
> +}
> +
> static void stmmac_mac_config(struct net_device *dev)
> {
> struct stmmac_priv *priv = netdev_priv(dev);
> @@ -900,6 +934,11 @@ static void stmmac_mac_config(struct net_device *dev)
> writel(ctrl, priv->ioaddr + MAC_CTRL_REG);
> }
>
> +static void stmmac_mac_an_restart(struct phylink_config *config)
> +{
> + /* Not Supported */
> +}
> +
> static void stmmac_mac_link_down(struct net_device *dev, bool autoneg)
> {
> struct stmmac_priv *priv = netdev_priv(dev);
> @@ -914,6 +953,15 @@ static void stmmac_mac_link_up(struct net_device *dev,
> bool autoneg)
> stmmac_mac_set(priv, priv->ioaddr, true);
> }
>
> +static const struct phylink_mac_ops __maybe_unused stmmac_phylink_mac_ops = {
> + .validate = stmmac_validate,
> + .mac_link_state = stmmac_mac_link_state,
> + .mac_config = NULL, /* TO BE FILLED */
> + .mac_an_restart = stmmac_mac_an_restart,
> + .mac_link_down = NULL, /* TO BE FILLED */
> + .mac_link_up = NULL, /* TO BE FILLED */
> +};
> +
If this is not used, I don't really see the point of splitting this from
the rest of the patch. Also, I don't see the point of all those NULL
initialisers either.
> /**
> * stmmac_adjust_link - adjusts the link parameters
> * @dev: net device structure
> --
> 2.21.0
>
>
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up
