linux-kernel - Re: [PATCH v2 4/5] net: hpe: Add GXP UMAC Driver

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: <61c541c9-be30-4a43-aa85-53816d5848f9@lunn.ch>
Date:   Sat, 5 Aug 2023 08:45:40 +0200
From:   Andrew Lunn <andrew@...n.ch>
To:     "Hawkins, Nick" <nick.hawkins@....com>
Cc:     "christophe.jaillet@...adoo.fr" <christophe.jaillet@...adoo.fr>,
        "simon.horman@...igine.com" <simon.horman@...igine.com>,
        "Verdun, Jean-Marie" <verdun@....com>,
        "davem@...emloft.net" <davem@...emloft.net>,
        "edumazet@...gle.com" <edumazet@...gle.com>,
        "kuba@...nel.org" <kuba@...nel.org>,
        "pabeni@...hat.com" <pabeni@...hat.com>,
        "robh+dt@...nel.org" <robh+dt@...nel.org>,
        "krzysztof.kozlowski+dt@...aro.org" 
        <krzysztof.kozlowski+dt@...aro.org>,
        "conor+dt@...nel.org" <conor+dt@...nel.org>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2 4/5] net: hpe: Add GXP UMAC Driver

On Fri, Aug 04, 2023 at 08:55:58PM +0000, Hawkins, Nick wrote:
> Greetings Andrew,
> 
> For some reason I do not see your replies for v1 of this patch or
> the mdio driver on lore.kernel. Apologies as I did not intend to
> not address your previous review comments. My mistake.
> 
> >> +static int umac_int_phy_init(struct umac_priv *umac)
> >> +{
> >> + struct phy_device *phy_dev = umac->int_phy_dev;
> >> + unsigned int value;
> >> +
> >> + value = phy_read(phy_dev, 0);
> >> + if (value & 0x4000)
> >> + pr_info("Internal PHY loopback is enabled - clearing\n");
> 
> > How is the PHY getting into loopback mode? The MAC driver should never
> > touch the PHY, because you have no idea what the PHY actually is,
> > unless it is internal. 
> 
> It would only be in loopback mode if it was previously configured
> that way. I will remove it. The PHY is internal to the ASIC 
> and is always the same. Given that information is it acceptable

Hi Nick

So what you call a PHY is probably a PCS. Please look at the API used
in driver/net/pcs/. The real PHYs are external.

Given that this is a BMC, you probably have lots of i2c busses. So you
can support an SFP on the SERDES. So it would be better if you used
the phylink interface, not phylib. This should also solve your
interface mode switching.

    Andrew