| 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: <AM4PR0401MB22606BEB23DB9364E12C0F11FF220@AM4PR0401MB2260.eurprd04.prod.outlook.com>
Date: Mon, 20 Nov 2017 09:35:51 +0000
From: Andy Duan <fugang.duan@....com>
To: Richard Leitner <dev@...l1n.net>,
"f.fainelli@...il.com" <f.fainelli@...il.com>,
"andrew@...n.ch" <andrew@...n.ch>
CC: "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"richard.leitner@...data.com" <richard.leitner@...data.com>
Subject: RE: [PATCH v2 1/3] net: ethernet: freescale: simplify fec_reset_phy
From: Richard Leitner <dev@...l1n.net> Sent: Monday, November 20, 2017 4:34 PM
>The fec_reset_phy function allowed only one execution during probeing.
>To make it more usable move the dt parsing and gpio allocation to the probe
>function. The parameters of the phy reset are added to the fec_enet_private
>struct. As a result the fec_reset_phy function may be called anytime after
>probe.
>
>One checkpatch.pl warning (too long line) is ignored. This is due to the fact a
>string (dt property name) otherwise needs to be split over multiple lines,
>which is counterproductive for the readability.
>
>Signed-off-by: Richard Leitner <richard.leitner@...data.com>
>---
It is better to convert to gpio descriptor, and use dts gpio flag as the gpio polarity instead of extra phy_reset_active_high variable.
Regards,
Andy
> drivers/net/ethernet/freescale/fec.h | 4 ++
> drivers/net/ethernet/freescale/fec_main.c | 88 ++++++++++++++++----------
>-----
> 2 files changed, 50 insertions(+), 42 deletions(-)
>
>diff --git a/drivers/net/ethernet/freescale/fec.h
>b/drivers/net/ethernet/freescale/fec.h
>index 5385074b3b7d..401c4eabf08a 100644
>--- a/drivers/net/ethernet/freescale/fec.h
>+++ b/drivers/net/ethernet/freescale/fec.h
>@@ -539,6 +539,10 @@ struct fec_enet_private {
> int pause_flag;
> int wol_flag;
> u32 quirks;
>+ int phy_reset;
>+ int phy_reset_duration;
>+ int phy_reset_post_delay;
>+ bool phy_reset_active_high;
>
> 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 610573855213..06a7caca0cee 100644
>--- a/drivers/net/ethernet/freescale/fec_main.c
>+++ b/drivers/net/ethernet/freescale/fec_main.c
>@@ -3212,62 +3212,36 @@ static int fec_enet_init(struct net_device *ndev) }
>
> #ifdef CONFIG_OF
>-static int fec_reset_phy(struct platform_device *pdev)
>+static int fec_reset_phy(struct net_device *ndev)
> {
>- int err, phy_reset;
>- bool active_high = false;
>- int msec = 1, phy_post_delay = 0;
>- struct device_node *np = pdev->dev.of_node;
>-
>- if (!np)
>- return 0;
>-
>- err = of_property_read_u32(np, "phy-reset-duration", &msec);
>- /* A sane reset duration should not be longer than 1s */
>- if (!err && msec > 1000)
>- msec = 1;
>+ struct fec_enet_private *fep = netdev_priv(ndev);
>
>- phy_reset = of_get_named_gpio(np, "phy-reset-gpios", 0);
>- if (phy_reset == -EPROBE_DEFER)
>- return phy_reset;
>- else if (!gpio_is_valid(phy_reset))
>+ if (!fep->phy_reset)
> return 0;
>
>- err = of_property_read_u32(np, "phy-reset-post-delay",
>&phy_post_delay);
>- /* valid reset duration should be less than 1s */
>- if (!err && phy_post_delay > 1000)
>- return -EINVAL;
>-
>- active_high = of_property_read_bool(np, "phy-reset-active-high");
>+ gpio_set_value_cansleep(fep->phy_reset, fep-
>>phy_reset_active_high);
>
>- err = devm_gpio_request_one(&pdev->dev, phy_reset,
>- active_high ? GPIOF_OUT_INIT_HIGH :
>GPIOF_OUT_INIT_LOW,
>- "phy-reset");
>- if (err) {
>- dev_err(&pdev->dev, "failed to get phy-reset-gpios: %d\n",
>err);
>- return err;
>- }
>-
>- if (msec > 20)
>- msleep(msec);
>+ if (fep->phy_reset_duration > 20)
>+ msleep(fep->phy_reset_duration);
> else
>- usleep_range(msec * 1000, msec * 1000 + 1000);
>+ usleep_range(fep->phy_reset_duration * 1000,
>+ fep->phy_reset_duration * 1000 + 1000);
>
>- gpio_set_value_cansleep(phy_reset, !active_high);
>+ gpio_set_value_cansleep(fep->phy_reset, !fep-
>>phy_reset_active_high);
>
>- if (!phy_post_delay)
>+ if (!fep->phy_reset_post_delay)
> return 0;
>
>- if (phy_post_delay > 20)
>- msleep(phy_post_delay);
>+ if (fep->phy_reset_post_delay > 20)
>+ msleep(fep->phy_reset_post_delay);
> else
>- usleep_range(phy_post_delay * 1000,
>- phy_post_delay * 1000 + 1000);
>+ usleep_range(fep->phy_reset_post_delay * 1000,
>+ fep->phy_reset_post_delay * 1000 + 1000);
>
> return 0;
> }
> #else /* CONFIG_OF */
>-static int fec_reset_phy(struct platform_device *pdev)
>+static int fec_reset_phy(struct net_device *ndev)
> {
> /*
> * In case of platform probe, the reset has been done @@ -3400,6
>+3374,36 @@ fec_probe(struct platform_device *pdev)
> }
> fep->phy_node = phy_node;
>
>+ fep->phy_reset = of_get_named_gpio(np, "phy-reset-gpios", 0);
>+ if (gpio_is_valid(fep->phy_reset)) {
>+ ret = of_property_read_u32(np, "phy-reset-duration",
>+ &fep->phy_reset_duration);
>+ /* A sane reset duration should not be longer than 1s */
>+ if (!ret && fep->phy_reset_post_delay > 1000)
>+ fep->phy_reset_post_delay = 1;
>+
>+ ret = of_property_read_u32(np, "phy-reset-post-delay",
>+ &fep->phy_reset_post_delay);
>+ /* valid post reset delay should be less than 1s */
>+ if (!ret && fep->phy_reset_post_delay > 1000)
>+ fep->phy_reset_post_delay = 1;
>+
>+ fep->phy_reset_active_high = of_property_read_bool(np,
>+"phy-reset-active-high");
>+
>+ ret = devm_gpio_request_one(&pdev->dev, fep->phy_reset,
>+ fep->phy_reset_active_high ?
>+ GPIOF_OUT_INIT_HIGH :
>+ GPIOF_OUT_INIT_LOW,
>+ "phy-reset");
>+ if (ret) {
>+ dev_err(&pdev->dev, "failed to get reset-
>gpios: %d\n",
>+ ret);
>+ goto failed_phy;
>+ }
>+ } else {
>+ fep->phy_reset = 0;
>+ }
>+
> ret = of_get_phy_mode(pdev->dev.of_node);
> if (ret < 0) {
> pdata = dev_get_platdata(&pdev->dev); @@ -3472,7 +3476,7
>@@ fec_probe(struct platform_device *pdev)
> pm_runtime_set_active(&pdev->dev);
> pm_runtime_enable(&pdev->dev);
>
>- ret = fec_reset_phy(pdev);
>+ ret = fec_reset_phy(ndev);
> if (ret)
> goto failed_reset;
>
>--
>2.11.0
Powered by blists - more mailing lists