netdev - Re: net: fec: Separate C22 and C45 transactions

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-next>] [day] [month] [year] [list]

Date:   Mon, 3 Apr 2023 14:41:46 +1000
From:   Greg Ungerer <gerg@...ux-m68k.org>
To:     Andrew Lunn <andrew@...n.ch>
Cc:     "netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: Re: net: fec: Separate C22 and C45 transactions

Hi Andrew,

On Mon, Jan 9, at 16:30 Andrew Lunn wrote:
> net: fec: Separate C22 and C45 transactions
>     
> The fec MDIO bus driver can perform both C22 and C45 transfers.
> Create separate functions for each and register the C45 versions using
> the new API calls where appropriate.

Are you sure that all FEC hardware blocks MDIO bus units support C45
transactions?

I ask because none of the ColdFire SoC parts with the FEC block mention
supporting C45 transactions. They quote those OP bits in the MMFR register
as generating "non-compliant" MII frames - for whatever that is supposed
to mean. They are not described any further. And it looks like some of
the older iMX parts have similar wording (at least I see it in the iMX25RM
and iMX50RM). No mention of supporting C45 transactions, and that is spelt out
explicitly in various iMX6, iMX8, and other Reference Manuals.

The MMFR register bits have some variation of this for ColdFire SoCs:

     29–28      OP Operation code.
       OP       00 Write frame operation, but not MII compliant.
                01 Write frame operation for a valid MII management frame.
                10 Read frame operation for a valid MII management frame.
                11 Read frame operation, but not MII compliant.

It is fairly easy to quirk this out and only register the C45 MDIO methods
if we know they are supported. Attached patch for example.

Regards
Greg



>     Signed-off-by: Andrew Lunn <andrew@...n.ch>
>     Signed-off-by: Michael Walle <michael@...le.cc>
>     Reviewed-by: Wei Fang <wei.fang@....com>
>     Signed-off-by: Jakub Kicinski <kuba@...nel.org>
> 
> diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
> index 644f3c963730..e6238e53940d 100644
> --- a/drivers/net/ethernet/freescale/fec_main.c
> +++ b/drivers/net/ethernet/freescale/fec_main.c
> @@ -1987,47 +1987,74 @@ static int fec_enet_mdio_wait(struct fec_enet_private *fep)
>  	return ret;
>  }
>  
> -static int fec_enet_mdio_read(struct mii_bus *bus, int mii_id, int regnum)
> +static int fec_enet_mdio_read_c22(struct mii_bus *bus, int mii_id, int regnum)
>  {
>  	struct fec_enet_private *fep = bus->priv;
>  	struct device *dev = &fep->pdev->dev;
>  	int ret = 0, frame_start, frame_addr, frame_op;
> -	bool is_c45 = !!(regnum & MII_ADDR_C45);
>  
>  	ret = pm_runtime_resume_and_get(dev);
>  	if (ret < 0)
>  		return ret;
>  
...

View attachment "0001-net-fec-make-use-of-MDIO-C45-a-quirk.patch" of type "text/x-patch" (6360 bytes)