| 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: <1452612514.3694.46.camel@pengutronix.de>
Date: Tue, 12 Jan 2016 16:28:34 +0100
From: Lucas Stach <l.stach@...gutronix.de>
To: Lothar Waßmann <LW@...O-electronics.de>
Cc: Andrew Lunn <andrew@...n.ch>,
"David S. Miller" <davem@...emloft.net>,
Fabio Estevam <fabio.estevam@...escale.com>,
Greg Ungerer <gerg@...inux.org>,
Kevin Hao <haokexin@...il.com>,
Nimrod Andy <B38611@...escale.com>,
Philippe Reynes <tremyfr@...il.com>,
Richard Cochran <richardcochran@...il.com>,
Russell King <rmk+kernel@....linux.org.uk>,
Sascha Hauer <s.hauer@...gutronix.de>,
Stefan Agner <stefan@...er.ch>, linux-kernel@...r.kernel.org,
netdev@...r.kernel.org, Jeff Kirsher <jeffrey.t.kirsher@...el.com>,
Uwe Kleine-König
<u.kleine-koenig@...gutronix.de>,
Ian Campbell <ijc+devicetree@...lion.org.uk>,
Kumar Gala <galak@...eaurora.org>,
Mark Rutland <mark.rutland@....com>,
Pawel Moll <pawel.moll@....com>,
Rob Herring <robh+dt@...nel.org>,
Russell King <linux@....linux.org.uk>,
Sascha Hauer <kernel@...gutronix.de>,
Shawn Guo <shawnguo@...nel.org>, devicetree@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCHv2 2/2] net: fec: Reset ethernet PHY whenever the
enet_out clock is being enabled
Am Dienstag, den 12.01.2016, 16:17 +0100 schrieb Lothar Waßmann:
> If a PHY uses ENET_OUT as reference clock, it may need a RESET to get
> functional after the clock had been disabled.
>
> Failure to do this results in the link state constantly toggling
> between up and down:
> fec 02188000.ethernet eth0: Link is Up - 100Mbps/Full - flow control rx/tx
> fec 02188000.ethernet eth0: Link is Down
> fec 02188000.ethernet eth0: Link is Up - 100Mbps/Full - flow control rx/tx
> fec 02188000.ethernet eth0: Link is Down
> [...]
>
> Signed-off-by: Lothar Waßmann <LW@...O-electronics.de>
> ---
> drivers/net/ethernet/freescale/fec.h | 1 +
> drivers/net/ethernet/freescale/fec_main.c | 38 +++++++++++++++++++++++++------
> 2 files changed, 32 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/net/ethernet/freescale/fec.h b/drivers/net/ethernet/freescale/fec.h
> index 99d33e2..33228df 100644
> --- a/drivers/net/ethernet/freescale/fec.h
> +++ b/drivers/net/ethernet/freescale/fec.h
> @@ -519,6 +519,7 @@ struct fec_enet_private {
> int pause_flag;
> int wol_flag;
> u32 quirks;
> + int phy_reset_gpio;
>
> struct napi_struct napi;
> int csum_flags;
> diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
> index 592a7a4..fb0b0eb 100644
> --- a/drivers/net/ethernet/freescale/fec_main.c
> +++ b/drivers/net/ethernet/freescale/fec_main.c
> @@ -67,6 +67,7 @@
>
> static void set_multicast_list(struct net_device *ndev);
> static void fec_enet_itr_coal_init(struct net_device *ndev);
> +static void fec_reset_phy(struct platform_device *pdev);
>
> #define DRIVER_NAME "fec"
>
> @@ -1862,6 +1863,8 @@ static int fec_enet_clk_enable(struct net_device *ndev, bool enable)
> ret = clk_prepare_enable(fep->clk_enet_out);
> if (ret)
> goto failed_clk_enet_out;
> +
> + fec_reset_phy(fep->pdev);
> }
> if (fep->clk_ptp) {
> mutex_lock(&fep->ptp_clk_mutex);
> @@ -3216,11 +3219,12 @@ static int fec_enet_init(struct net_device *ndev)
> #ifdef CONFIG_OF
> static void fec_reset_phy(struct platform_device *pdev)
> {
> - int err, phy_reset;
> - int msec = 1;
> + struct net_device *ndev = platform_get_drvdata(pdev);
> + struct fec_enet_private *fep = netdev_priv(ndev);
> struct device_node *np = pdev->dev.of_node;
> + int msec = 1;
>
> - if (!np)
> + if (!gpio_is_valid(fep->phy_reset_gpio))
> return;
>
> of_property_read_u32(np, "phy-reset-duration", &msec);
While this isn't in a hot path I still want to see this
of_property_read() moved to the fec_get_reset_gpio() function below.
It's really inconsistent otherwise.
> @@ -3228,18 +3232,27 @@ static void fec_reset_phy(struct platform_device *pdev)
> if (msec > 1000)
> msec = 1;
>
> + gpio_set_value_cansleep(fep->phy_reset_gpio, 0);
> + msleep(msec);
> + gpio_set_value_cansleep(fep->phy_reset_gpio, 1);
> +}
> +
> +static int fec_get_reset_gpio(struct platform_device *pdev)
> +{
> + int err, phy_reset;
> + struct device_node *np = pdev->dev.of_node;
> +
> phy_reset = of_get_named_gpio(np, "phy-reset-gpios", 0);
> if (!gpio_is_valid(phy_reset))
> - return;
> + return phy_reset;
>
> err = devm_gpio_request_one(&pdev->dev, phy_reset,
> GPIOF_OUT_INIT_LOW, "phy-reset");
> if (err) {
> dev_err(&pdev->dev, "failed to get phy-reset-gpios: %d\n", err);
> - return;
> + return err;
> }
> - msleep(msec);
> - gpio_set_value_cansleep(phy_reset, 1);
> + return phy_reset;
> }
> #else /* CONFIG_OF */
> static void fec_reset_phy(struct platform_device *pdev)
> @@ -3249,6 +3262,11 @@ static void fec_reset_phy(struct platform_device *pdev)
> * by machine code.
> */
> }
> +
> +static inline int fec_get_reset_gpio(struct platform_device *pdev)
> +{
> + return -EINVAL;
> +}
> #endif /* CONFIG_OF */
>
> static void
> @@ -3339,6 +3357,11 @@ fec_probe(struct platform_device *pdev)
>
> platform_set_drvdata(pdev, ndev);
>
> + ret = fec_get_reset_gpio(pdev);
> + if (ret == -EPROBE_DEFER)
> + goto gpio_defer;
> + fep->phy_reset_gpio = ret;
> +
> if (of_get_property(np, "fsl,magic-packet", NULL))
> fep->wol_flag |= FEC_WOL_HAS_MAGIC_PACKET;
>
> @@ -3493,6 +3516,7 @@ failed_clk_ipg:
> failed_clk:
> failed_phy:
> of_node_put(phy_node);
> +gpio_defer:
> failed_ioremap:
> free_netdev(ndev);
>
--
Pengutronix e.K. | Lucas Stach |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Powered by blists - more mailing lists