[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <83E62581-EB88-4FC1-9DF9-8F725577F825@kernel.crashing.org>
Date: Fri, 11 May 2007 09:44:23 -0500
From: Kumar Gala <galak@...nel.crashing.org>
To: matvejchikov@...il.com
Cc: "Jeff Garzik" <jeff@...zik.org>,
"Randy Dunlap" <randy.dunlap@...cle.com>, netdev@...r.kernel.org
Subject: Re: [PATCH] phylib: phy_ethtool_{sset,gset} -- check phydev for NULL
On May 11, 2007, at 8:49 AM, Matvejchikov Ilya wrote:
> 2007/5/11, Kumar Gala <galak@...nel.crashing.org>:
>>
>> On May 11, 2007, at 5:58 AM, Matvejchikov Ilya wrote:
>>
>> > Signed-off-by: Matvejchikov Ilya <matvejchikov@...il.com>
>> > ---
>> >
>> > diff -purN linux-2.6.21-clean/drivers/net/phy/phy.c
>> > linux-2.6.21/drivers/net/phy/phy.c
>> > --- linux-2.6.21-clean/drivers/net/phy/phy.c 2007-04-26
>> > 07:08:32.000000000 +0400
>> > +++ linux-2.6.21/drivers/net/phy/phy.c 2007-05-04
>> > 08:22:01.000000000 +0400
>> > @@ -245,6 +245,9 @@ EXPORT_SYMBOL(phy_sanitize_settings);
>> > */
>> > int phy_ethtool_sset(struct phy_device *phydev, struct ethtool_cmd
>> > *cmd)
>> > {
>> > + if (unlikely(!phydev))
>> > + return -EINVAL;
>>
>> Should this be -ENODEV?
>
> If we get the ENODEV error, we are suppose that there is no device at
> all. PHY device may not be connected to the NET device. But if it
> exists we can't say that there is NODEV...
>
> I think it should be -EINVAL.
Hmm, if the phy isn't connected to the NET device doesn't that mean
we don't have a PHY device (from the point of view of the netdevice),
thus NODEV?
Whatever we should cleanup the current users of phy_ethtool_sset/
phy_ethtool_gset.
>> > +
>> > if (cmd->phy_address != phydev->addr)
>> > return -EINVAL;
>> >
>> > @@ -289,6 +292,9 @@ EXPORT_SYMBOL(phy_ethtool_sset);
>> >
>> > int phy_ethtool_gset(struct phy_device *phydev, struct ethtool_cmd
>> > *cmd)
>> > {
>> > + if (unlikely(!phydev))
>> > + return -EINVAL;
>>
>> same question.
>
> The same answer :)
>
>> > +
>> > cmd->supported = phydev->supported;
>> >
>> > cmd->advertising = phydev->advertising;
>> > -
>> > To unsubscribe from this list: send the line "unsubscribe
>> netdev" in
>> > the body of a message to majordomo@...r.kernel.org
>> > More majordomo info at http://vger.kernel.org/majordomo-info.html
>>
>>
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists