| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20190611153529.z6hlkhtrnd5ksx2n@shell.armlinux.org.uk>
Date: Tue, 11 Jun 2019 16:35:29 +0100
From: Russell King - ARM Linux admin <linux@...linux.org.uk>
To: Jose Abreu <Jose.Abreu@...opsys.com>
Cc: linux-kernel@...r.kernel.org, netdev@...r.kernel.org,
Joao Pinto <Joao.Pinto@...opsys.com>,
"David S . Miller" <davem@...emloft.net>,
Giuseppe Cavallaro <peppe.cavallaro@...com>,
Alexandre Torgue <alexandre.torgue@...com>,
Andrew Lunn <andrew@...n.ch>,
Florian Fainelli <f.fainelli@...il.com>,
Heiner Kallweit <hkallweit1@...il.com>
Subject: Re: [PATCH net-next 2/3] net: stmmac: Start adding phylink support
On Tue, Jun 11, 2019 at 05:18:46PM +0200, Jose Abreu wrote:
> Start adding the phylink callbacks.
>
> Signed-off-by: Jose Abreu <joabreu@...opsys.com>
> Cc: Joao Pinto <jpinto@...opsys.com>
> Cc: David S. Miller <davem@...emloft.net>
> Cc: Giuseppe Cavallaro <peppe.cavallaro@...com>
> Cc: Alexandre Torgue <alexandre.torgue@...com>
> Cc: Russell King <linux@...linux.org.uk>
> Cc: Andrew Lunn <andrew@...n.ch>
> Cc: Florian Fainelli <f.fainelli@...il.com>
> Cc: Heiner Kallweit <hkallweit1@...il.com>
> ---
> 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
Powered by blists - more mailing lists