netdev - Re: [PATCH net-next] Add Mellanox BlueField Gigabit Ethernet 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: <20200812213401.GI2141651@lunn.ch>
Date:   Wed, 12 Aug 2020 23:34:01 +0200
From:   Andrew Lunn <andrew@...n.ch>
To:     Asmaa Mnebhi <asmaa@...dia.com>
Cc:     David Thompson <dthompson@...lanox.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "davem@...emloft.net" <davem@...emloft.net>,
        "kuba@...nel.org" <kuba@...nel.org>,
        Jiri Pirko <jiri@...lanox.com>,
        Asmaa Mnebhi <Asmaa@...lanox.com>
Subject: Re: [PATCH net-next] Add Mellanox BlueField Gigabit Ethernet driver

Hi Asmaa

Please wrap your emails at about 75 characters.

> So let me explain further and would greatly appreciate your input.

> Technically, when this driver gets loaded, we shouldn't need the interrupt when bringing up the link for the first time, do we?
> Correct me if I am wrong, "phy_start" should bring up the link. Phy_start calls phy_start_aneg, which eventually calls phy_check_link_status.
> phy_check_link_status , reads the link state bit of the BMSR register (only twice),  and based on that determines whether to bring up/down the link. In our case, that bit is still 0 when the read is donw. A little bit later, it gets set to 1.
> 
> This is why polling works in this case. Phy_start fails to bring up the link but the polling eventually bring it up. If we choose to use the interrupt, we should make sure that the 
> Interrupt is enabled a little bit after phy_start, otherwise, it would just be wasted.

When the PHY is connected to the MAC, phy_request_interrupt() is
called. That sets up the SoC side of the interrupt, so that
phy_interrupt() will be called on interrupt. It then calls
phy_enable_interrupts()->phy_config_interrupt() which calls into the
PHY driver to enable interrupts within the PHY.  It is then expected
that the PHY interrupts whenever there is a change in link status.

Sometime later phy_start() will be called which should kick off an
autoneg. That should result in the link going up, maybe 1.5 seconds
later if the link peer is present, maybe later if there is no
peer. The link up will trigger an interrupt, and the new status will
be read. If the link goes down, the interrupt should also trigger, and
the status will be updated.

   Andrew