netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Florian Fainelli <f.fainelli@gmail.com>
To: Ben Hutchings <ben.hutchings@codethink.co.uk>
Cc: "David S. Miller" <davem@davemloft.net>,
	netdev <netdev@vger.kernel.org>,
	linux-kernel <linux-kernel@lists.codethink.co.uk>,
	Nobuhiro Iwamatsu <nobuhiro.iwamatsu.yj@renesas.com>,
	Mitsuhiro Kimura <mitsuhiro.kimura.kc@renesas.com>,
	Hisashi Nakamura <hisashi.nakamura.ak@renesas.com>,
	Yoshihiro Kaneko <ykaneko0929@gmail.com>
Subject: Re: [PATCH net 2/2] sh_eth: Fix ethtool operation crash when net device is down
Date: Mon, 19 Jan 2015 09:28:06 -0800	[thread overview]
Message-ID: <54BD3EA6.9000900@gmail.com> (raw)
In-Reply-To: <1421664085.1222.200.camel@xylophone.i.decadent.org.uk>

On 19/01/15 02:41, Ben Hutchings wrote:
> On Fri, 2015-01-16 at 10:45 -0800, Florian Fainelli wrote:
>> 2015-01-16 9:51 GMT-08:00 Ben Hutchings <ben.hutchings@codethink.co.uk>:
> [...]
>>> --- a/drivers/net/ethernet/renesas/sh_eth.c
>>> +++ b/drivers/net/ethernet/renesas/sh_eth.c
>>> @@ -1827,6 +1827,9 @@ static int sh_eth_get_settings(struct net_device *ndev,
>>>         unsigned long flags;
>>>         int ret;
>>>
>>> +       if (!mdp->phydev)
>>> +               return -ENODEV;
>>
>> Since the PHY is disconnected, would not checking for netif_running()
>> make sense here, unless there is a good reason to still allow
>> phy_ethtool_gset() to be called?
> [...]
> 
> I think those two conditions will be equivalent, won't they?

They are indeed.

>  Writing the condition like this will also work if the driver later supports
> PHY-less configurations.

You could also represent a PHY-less configuration by using the emulated
fixed-PHY, such that the driver is effectively "driving" a PHY device,
even though this is not a real one, up to you.
-- 
Florian

  reply	other threads:[~2015-01-19 17:28 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-01-16 17:49 [PATCH net 0/2] sh_eth fixes Ben Hutchings
2015-01-16 17:51 ` [PATCH net 1/2] sh_eth: Fix promiscuous mode on chips without TSU Ben Hutchings
2015-01-16 19:27   ` Sergei Shtylyov
2015-01-16 19:36     ` Sergei Shtylyov
2015-01-19 10:45     ` Ben Hutchings
2015-01-16 17:51 ` [PATCH net 2/2] sh_eth: Fix ethtool operation crash when net device is down Ben Hutchings
2015-01-16 18:45   ` Florian Fainelli
2015-01-19 10:41     ` Ben Hutchings
2015-01-19 17:28       ` Florian Fainelli [this message]
2015-01-19 20:38 ` [PATCH net 0/2] sh_eth fixes David Miller

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=54BD3EA6.9000900@gmail.com \
    --to=f.fainelli@gmail.com \
    --cc=ben.hutchings@codethink.co.uk \
    --cc=davem@davemloft.net \
    --cc=hisashi.nakamura.ak@renesas.com \
    --cc=linux-kernel@lists.codethink.co.uk \
    --cc=mitsuhiro.kimura.kc@renesas.com \
    --cc=netdev@vger.kernel.org \
    --cc=nobuhiro.iwamatsu.yj@renesas.com \
    --cc=ykaneko0929@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).