netdev - Re: [PATCH v2] net: phy: add Maxlinear GPY115/21x/24x 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]

Date:   Thu, 3 Jun 2021 17:21:28 +0200
From:   Andrew Lunn <andrew@...n.ch>
To:     Liang Xu <lxu@...linear.com>
Cc:     "Russell King (Oracle)" <linux@...linux.org.uk>,
        "hkallweit1@...il.com" <hkallweit1@...il.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "davem@...emloft.net" <davem@...emloft.net>,
        "kuba@...nel.org" <kuba@...nel.org>,
        "vee.khee.wong@...ux.intel.com" <vee.khee.wong@...ux.intel.com>,
        Hauke Mehrtens <hmehrtens@...linear.com>,
        Thomas Mohren <tmohren@...linear.com>
Subject: Re: [PATCH v2] net: phy: add Maxlinear GPY115/21x/24x driver

On Thu, Jun 03, 2021 at 03:10:31PM +0000, Liang Xu wrote:
> On 3/6/2021 5:17 pm, Russell King (Oracle) wrote:
> >
> > > +static int gpy_config_init(struct phy_device *phydev)
> > > +{
> > > + int ret, fw_ver;
> > > +
> > > + /* Show GPY PHY FW version in dmesg */
> > > + fw_ver = phy_read(phydev, PHY_FWV);
> > > + if (fw_ver < 0)
> > > + return fw_ver;
> > > +
> > > + phydev_info(phydev, "Firmware Version: 0x%04X (%s)\n", fw_ver,
> > > + (fw_ver & PHY_FWV_REL_MASK) ? "release" : "test");
> >
> > Does this need to print the firmware version each time config_init()
> > is called? Is it likely to change beyond? Would it be more sensible
> > to print it in the probe() method?
> 
> The firmware version can change in device with different firmware 
> loading mechanism.
> 
> I moved the print to probe and tested a few devices, found in some cases 
> it did not print the active version number.

That actually sounds like a real problem. If it is still in the
bootloader when the driver is probed, the driver should not be writing
any configuration registers until the real image is running. So it
sounds like you need a probe function which checks if the PHY has
finished booting, and if not, wait for the real firmware to start
running.

	Andrew