Re: [PATCH net-next 2/4] net: mvpp2: add mvpp2_phylink_to_port() helper

Marcin Wojtas
2020-10-09T03:44:16Z

Hi,

sob., 20 cze 2020 o 11:21 Russell King <rmk+kernel@armlinux.org.uk> napisał(a):
>
> Add a helper to convert the struct phylink_config pointer passed in
> from phylink to the drivers internal struct mvpp2_port.
>
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> ---
> .../net/ethernet/marvell/mvpp2/mvpp2_main.c | 29 +++++++++----------
> 1 file changed, 14 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> index 7653277d03b7..313f5a60a605 100644
> --- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> +++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> @@ -4767,12 +4767,16 @@ static void mvpp2_port_copy_mac_addr(struct net_device *dev, struct mvpp2 *priv,
> eth_hw_addr_random(dev);
> }
>
> +static struct mvpp2_port *mvpp2_phylink_to_port(struct phylink_config *config)
> +{
> + return container_of(config, struct mvpp2_port, phylink_config);
> +}
> +
> static void mvpp2_phylink_validate(struct phylink_config *config,
> unsigned long *supported,
> struct phylink_link_state *state)
> {
> - struct mvpp2_port *port = container_of(config, struct mvpp2_port,
> - phylink_config);
> + struct mvpp2_port *port = mvpp2_phylink_to_port(config);
> __ETHTOOL_DECLARE_LINK_MODE_MASK(mask) = { 0, };
>
> /* Invalid combinations */
> @@ -4913,8 +4917,7 @@ static void mvpp2_gmac_pcs_get_state(struct mvpp2_port *port,
> static void mvpp2_phylink_mac_pcs_get_state(struct phylink_config *config,
> struct phylink_link_state *state)
> {
> - struct mvpp2_port *port = container_of(config, struct mvpp2_port,
> - phylink_config);
> + struct mvpp2_port *port = mvpp2_phylink_to_port(config);
>
> if (port->priv->hw_version == MVPP22 && port->gop_id == 0) {
> u32 mode = readl(port->base + MVPP22_XLG_CTRL3_REG);
> @@ -4931,8 +4934,7 @@ static void mvpp2_phylink_mac_pcs_get_state(struct phylink_config *config,
>
> static void mvpp2_mac_an_restart(struct phylink_config *config)
> {
> - struct mvpp2_port *port = container_of(config, struct mvpp2_port,
> - phylink_config);
> + struct mvpp2_port *port = mvpp2_phylink_to_port(config);
> u32 val = readl(port->base + MVPP2_GMAC_AUTONEG_CONFIG);
>
> writel(val | MVPP2_GMAC_IN_BAND_RESTART_AN,
> @@ -5105,13 +5107,12 @@ static void mvpp2_gmac_config(struct mvpp2_port *port, unsigned int mode,
> static void mvpp2_mac_config(struct phylink_config *config, unsigned int mode,
> const struct phylink_link_state *state)
> {
> - struct net_device *dev = to_net_dev(config->dev);
> - struct mvpp2_port *port = netdev_priv(dev);
> + struct mvpp2_port *port = mvpp2_phylink_to_port(config);
> bool change_interface = port->phy_interface != state->interface;
>
> /* Check for invalid configuration */
> if (mvpp2_is_xlg(state->interface) && port->gop_id != 0) {
> - netdev_err(dev, "Invalid mode on %s\n", dev->name);
> + netdev_err(port->dev, "Invalid mode on %s\n", port->dev->name);
> return;
> }
>
> @@ -5151,8 +5152,7 @@ static void mvpp2_mac_link_up(struct phylink_config *config,
> int speed, int duplex,
> bool tx_pause, bool rx_pause)
> {
> - struct net_device *dev = to_net_dev(config->dev);
> - struct mvpp2_port *port = netdev_priv(dev);
> + struct mvpp2_port *port = mvpp2_phylink_to_port(config);
> u32 val;
>
> if (mvpp2_is_xlg(interface)) {
> @@ -5199,14 +5199,13 @@ static void mvpp2_mac_link_up(struct phylink_config *config,
>
> mvpp2_egress_enable(port);
> mvpp2_ingress_enable(port);
> - netif_tx_wake_all_queues(dev);
> + netif_tx_wake_all_queues(port->dev);
> }
>
> static void mvpp2_mac_link_down(struct phylink_config *config,
> unsigned int mode, phy_interface_t interface)
> {
> - struct net_device *dev = to_net_dev(config->dev);
> - struct mvpp2_port *port = netdev_priv(dev);
> + struct mvpp2_port *port = mvpp2_phylink_to_port(config);
> u32 val;
>
> if (!phylink_autoneg_inband(mode)) {
> @@ -5223,7 +5222,7 @@ static void mvpp2_mac_link_down(struct phylink_config *config,
> }
> }
>
> - netif_tx_stop_all_queues(dev);
> + netif_tx_stop_all_queues(port->dev);
> mvpp2_egress_disable(port);
> mvpp2_ingress_disable(port);
>
> --
> 2.20.1
>

This patch fixes a regression that was introduced in v5.3:
Commit 44cc27e43fa3 ("net: phylink: Add struct phylink_config to PHYLINK API")

Above results in a NULL pointer dereference when booting the
Armada7k8k/CN913x with ACPI between 5.3 and 5.8, which will be
problematic especially for the distros using LTSv5.4 and above (the
issue was reported on Fedora 32).

Please help with backporting to the stable v5.3+ branches (it applies
smoothly on v5.4/v5.6/v5.8).

Best regards,
Marcin