netdev - RE: [RFC PATCH 0/2] Reset PHY in fec

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <DB8PR04MB679536F1DD900564B9A957C4E66E9@DB8PR04MB6795.eurprd04.prod.outlook.com>
Date:   Tue, 7 Dec 2021 01:58:16 +0000
From:   Joakim Zhang <qiangqing.zhang@....com>
To:     Philippe Schenker <philippe.schenker@...adex.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        Fabio Estevam <festevam@...il.com>,
        "David S . Miller" <davem@...emloft.net>,
        Russell King <linux@...linux.org.uk>,
        Andrew Lunn <andrew@...n.ch>, Jakub Kicinski <kuba@...nel.org>
CC:     "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [RFC PATCH 0/2] Reset PHY in fec_resume if it got powered down


Hi Philippe,

> -----Original Message-----
> From: Philippe Schenker <philippe.schenker@...adex.com>
> Sent: 2021年12月6日 18:13
> To: netdev@...r.kernel.org; Joakim Zhang <qiangqing.zhang@....com>;
> Fabio Estevam <festevam@...il.com>; Fugang Duan
> <fugang.duan@....com>; David S . Miller <davem@...emloft.net>; Russell
> King <linux@...linux.org.uk>; Andrew Lunn <andrew@...n.ch>; Jakub
> Kicinski <kuba@...nel.org>
> Cc: Philippe Schenker <philippe.schenker@...adex.com>;
> linux-kernel@...r.kernel.org
> Subject: [RFC PATCH 0/2] Reset PHY in fec_resume if it got powered down
> 
> 
> If a hardware-design is able to control power to the Ethernet PHY and relying
> on software to do a reset, the PHY does no longer work after resuming from
> suspend, given the PHY does need a hardware-reset.
> The Freescale fec driver does currently control the reset-signal of a phy but
> does not issue a reset on resume.
> 
> On Toradex Apalis iMX8 board we do have such a design where we also don't
> place the RC circuit to delay the reset-line by hardware. Hence we fully rely
> on software to do so.
> Since I didn't manage to get the needed parts of Apalis iMX8 working with
> mainline this patchset was only tested on the downstream kernel
> toradex_5.4-2.3.x-imx. [1] This kernel is based on NXP's release
> imx_5.4.70_2.3.0. [2] The affected code is still the same on mainline kernel,
> which would actually make me comfortable merging this patch, but due to
> this fact I'm sending this as RFC maybe someone else is able to test this code.
> 
> This patchset aims to change the behavior by resetting the ethernet PHY in
> fec_resume. A short description of the patches can be found below, please
> find a detailed description in the commit-messages of the respective
> patches.
> 
> [PATCH 2/2] net: fec: reset phy in resume if it was powered down
> 
> This patch calls fec_reset_phy just after regulator enable in fec_resume,
> when the phy is resumed
> 
> [PATCH 1/2] net: fec: make fec_reset_phy not only usable once
> 
> This patch prepares the function fec_reset_phy to be called multiple times. It
> stores the data around the reset-gpio in fec_enet_private.
> This patch aims to do no functional changes.
> 
> [1]
> https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fgit.tor
> adex.com%2Fcgit%2Flinux-toradex.git%2Flog%2F%3Fh%3Dtoradex_5.4-2.3.x
> -imx&amp;data=04%7C01%7Cqiangqing.zhang%40nxp.com%7Cf3c138ed9232
> 4a8d75e708d9b8a11b9a%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%
> 7C637743824364193423%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAw
> MDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&amp;sda
> ta=Bw%2BZdqhAjPXqKJFZCXp0mtId1x9mkX6f6MW2ky6U1ww%3D&amp;res
> erved=0
> [2]
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fsourc
> e.codeaurora.org%2Fexternal%2Fimx%2Flinux-imx%2Flog%2F%3Fh%3Dimx_
> 5.4.70_2.3.0&amp;data=04%7C01%7Cqiangqing.zhang%40nxp.com%7Cf3c13
> 8ed92324a8d75e708d9b8a11b9a%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C
> 0%7C0%7C637743824364193423%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiM
> C4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000
> &amp;sdata=of9z9hfVhHakVScLxCdEo%2BXmd2B9Ad9X8Rry6GjEZE4%3D&a
> mp;reserved=0
> 

In fec driver, it has supported hardware reset for PHY when MAC resume back,

fec_resume() -> phy_init_hw() -> phy_device_reset() de-assert the reset signal, you only need implement
the properties which PHY core provided.

I think you should not use deprecated reset properties provided by fec driver, instead the common
reset properties provided by PHY core.

Please check the dt-bindings for more details:
Documentation/devicetree/bindings/net/fsl,fec.yaml
Documentation/devicetree/bindings/net/ethernet-phy.yaml

Best Regards,
Joakim Zhang
> Philippe Schenker (2):
>   net: fec: make fec_reset_phy not only usable once
>   net: fec: reset phy in resume if it was powered down
> 
>  drivers/net/ethernet/freescale/fec.h      |  6 ++
>  drivers/net/ethernet/freescale/fec_main.c | 98 ++++++++++++++++-------
>  2 files changed, 73 insertions(+), 31 deletions(-)
> 
> --
> 2.34.0