linux-kernel - Re: [PATCH NET] net/hns:bugfix of ethtool -t phy self

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:   Wed, 21 Jun 2017 15:34:49 +0200
From:   Andrew Lunn <andrew@...n.ch>
To:     l00371289 <linyunsheng@...wei.com>
Cc:     Florian Fainelli <f.fainelli@...il.com>, davem@...emloft.net,
        huangdaode@...ilicon.com, xuwei5@...ilicon.com,
        liguozhu@...ilicon.com, Yisen.Zhuang@...wei.com,
        gabriele.paoloni@...wei.com, john.garry@...wei.com,
        linuxarm@...wei.com, salil.mehta@...wei.com, lipeng321@...wei.com,
        yankejian@...wei.com, tremyfr@...il.com, xieqianqian@...wei.com,
        netdev@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH NET] net/hns:bugfix of ethtool -t phy self_test

> drivers/net/ph/marvell.c
> marvell_set_loopback(struct phy_device *dev, bool enable)
> {
> 	/* do some device specific setting */
> 	........
> 
> 	return genphy_loopback(dev, enable);
> }
> 
> I don't know if this makes sense or not?

Nope, you want something in phy.c like this. Not compiled, not
tested....

int phy_loopback(struct phy_device *dev, bool enable)
{

	int err;

	mutex_lock(dev->mutex);

	if (enable && dev->loopback_enabled) {
	   err = -EBUSY;
	   goto out;
	}

	if (!enable && !dev->loopback_enabled) {
	   err = -EINVAL;
	   goto out;
	}

	if (dev->drv->loopback)
	   err = dev->drv->loopback(dev, enable);

	if (!err)
	   dev->loopback_enabled = enable

	mutex_unlock(dev->mutex);

out:
	return err;
}
EXPORT_SYMBOL(phy_loopback);

The interesting part is how to handle two attempts to enable loopback.
Should the MAC driver be responsible for preventing that? Or do it in
the PHY driver? And what will the MAC driver do if it gets EBUSY?
Return it to user space?

       Andrew