* [PATCH 1/2] net: cadence: macb: add support for the WOL
@ 2014-07-14 9:02 Jongsung Kim
2014-07-14 10:15 ` Varka Bhadram
0 siblings, 1 reply; 6+ messages in thread
From: Jongsung Kim @ 2014-07-14 9:02 UTC (permalink / raw)
To: nicolas.ferre; +Cc: netdev, linux-kernel, davem, Jongsung Kim
This patch enables the ethtool utility to control the WOL function
of the PHY connected to the GEM/MACB. (if supported)
Signed-off-by: Jongsung Kim <neidhard.kim@lge.com>
---
drivers/net/ethernet/cadence/macb.c | 26 ++++++++++++++++++++++++++
1 files changed, 26 insertions(+), 0 deletions(-)
diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c
index e9daa07..7ad8909 100644
--- a/drivers/net/ethernet/cadence/macb.c
+++ b/drivers/net/ethernet/cadence/macb.c
@@ -1742,11 +1742,37 @@ static void macb_get_regs(struct net_device *dev, struct ethtool_regs *regs,
}
}
+static void macb_get_wol(struct net_device *netdev, struct ethtool_wolinfo *wol)
+{
+ struct macb *bp = netdev_priv(netdev);
+ struct phy_device *phydev = bp->phy_dev;
+
+ wol->supported = 0;
+ wol->wolopts = 0;
+
+ if (phydev)
+ phy_ethtool_get_wol(phydev, wol);
+}
+
+static int macb_set_wol(struct net_device *netdev, struct ethtool_wolinfo *wol)
+{
+ struct macb *bp = netdev_priv(netdev);
+ struct phy_device *phydev = bp->phy_dev;
+ int err = -ENODEV;
+
+ if (phydev)
+ err = phy_ethtool_set_wol(phydev, wol);
+
+ return err;
+}
+
const struct ethtool_ops macb_ethtool_ops = {
.get_settings = macb_get_settings,
.set_settings = macb_set_settings,
.get_regs_len = macb_get_regs_len,
.get_regs = macb_get_regs,
+ .get_wol = macb_get_wol,
+ .set_wol = macb_set_wol,
.get_link = ethtool_op_get_link,
.get_ts_info = ethtool_op_get_ts_info,
};
--
1.7.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH 1/2] net: cadence: macb: add support for the WOL
2014-07-14 9:02 [PATCH 1/2] net: cadence: macb: add support for the WOL Jongsung Kim
@ 2014-07-14 10:15 ` Varka Bhadram
2014-07-14 10:49 ` David Laight
2014-07-15 7:02 ` Jongsung Kim
0 siblings, 2 replies; 6+ messages in thread
From: Varka Bhadram @ 2014-07-14 10:15 UTC (permalink / raw)
To: Jongsung Kim, nicolas.ferre; +Cc: netdev, linux-kernel, davem
On 07/14/2014 02:32 PM, Jongsung Kim wrote:
> This patch enables the ethtool utility to control the WOL function
> of the PHY connected to the GEM/MACB. (if supported)
>
> Signed-off-by: Jongsung Kim <neidhard.kim@lge.com>
> ---
> drivers/net/ethernet/cadence/macb.c | 26 ++++++++++++++++++++++++++
> 1 files changed, 26 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c
> index e9daa07..7ad8909 100644
> --- a/drivers/net/ethernet/cadence/macb.c
> +++ b/drivers/net/ethernet/cadence/macb.c
> @@ -1742,11 +1742,37 @@ static void macb_get_regs(struct net_device *dev, struct ethtool_regs *regs,
> }
> }
>
> +static void macb_get_wol(struct net_device *netdev, struct ethtool_wolinfo *wol)
> +{
> + struct macb *bp = netdev_priv(netdev);
> + struct phy_device *phydev = bp->phy_dev;
> +
> + wol->supported = 0;
> + wol->wolopts = 0;
> +
> + if (phydev)
> + phy_ethtool_get_wol(phydev, wol);
> +}
> +
> +static int macb_set_wol(struct net_device *netdev, struct ethtool_wolinfo *wol)
> +{
> + struct macb *bp = netdev_priv(netdev);
> + struct phy_device *phydev = bp->phy_dev;
> + int err = -ENODEV;
> +
> + if (phydev)
> + err = phy_ethtool_set_wol(phydev, wol);
> +
> + return err;
> +}
> +
I think we can do in this way:
if (phydev)
return phy_ethtool_set_wol(phydev, wol);
else
return -ENODEV;
we can save err. What do you say ...?
> const struct ethtool_ops macb_ethtool_ops = {
> .get_settings = macb_get_settings,
> .set_settings = macb_set_settings,
> .get_regs_len = macb_get_regs_len,
> .get_regs = macb_get_regs,
> + .get_wol = macb_get_wol,
> + .set_wol = macb_set_wol,
> .get_link = ethtool_op_get_link,
> .get_ts_info = ethtool_op_get_ts_info,
> };
--
Regards,
Varka Bhadram.
^ permalink raw reply [flat|nested] 6+ messages in thread* RE: [PATCH 1/2] net: cadence: macb: add support for the WOL
2014-07-14 10:15 ` Varka Bhadram
@ 2014-07-14 10:49 ` David Laight
2014-07-14 10:53 ` Varka Bhadram
2014-07-15 6:58 ` Jongsung Kim
2014-07-15 7:02 ` Jongsung Kim
1 sibling, 2 replies; 6+ messages in thread
From: David Laight @ 2014-07-14 10:49 UTC (permalink / raw)
To: 'Varka Bhadram', Jongsung Kim, nicolas.ferre@atmel.com
Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
davem@davemloft.net
From: Varka Bhadram
> On 07/14/2014 02:32 PM, Jongsung Kim wrote:
> > This patch enables the ethtool utility to control the WOL function
> > of the PHY connected to the GEM/MACB. (if supported)
...
> > +static int macb_set_wol(struct net_device *netdev, struct ethtool_wolinfo *wol)
> > +{
> > + struct macb *bp = netdev_priv(netdev);
> > + struct phy_device *phydev = bp->phy_dev;
> > + int err = -ENODEV;
> > +
> > + if (phydev)
> > + err = phy_ethtool_set_wol(phydev, wol);
> > +
> > + return err;
> > +}
> > +
>
> I think we can do in this way:
>
> if (phydev)
> return phy_ethtool_set_wol(phydev, wol);
> else
> return -ENODEV;
>
>
> we can save err. What do you say ...?
I would do:
if (!phydev)
return -ENODEV;
return phy_ethtool_set_wol(phydev, wol);
Although it might even be worth moving the NULL test into the function.
(sort of depends on style and the number of callers who need to do the test.)
David
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 1/2] net: cadence: macb: add support for the WOL
2014-07-14 10:49 ` David Laight
@ 2014-07-14 10:53 ` Varka Bhadram
2014-07-15 6:58 ` Jongsung Kim
1 sibling, 0 replies; 6+ messages in thread
From: Varka Bhadram @ 2014-07-14 10:53 UTC (permalink / raw)
To: David Laight, Jongsung Kim, nicolas.ferre@atmel.com
Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
davem@davemloft.net
On 07/14/2014 04:19 PM, David Laight wrote:
> From: Varka Bhadram
>> On 07/14/2014 02:32 PM, Jongsung Kim wrote:
>>> This patch enables the ethtool utility to control the WOL function
>>> of the PHY connected to the GEM/MACB. (if supported)
> ...
>>> +static int macb_set_wol(struct net_device *netdev, struct ethtool_wolinfo *wol)
>>> +{
>>> + struct macb *bp = netdev_priv(netdev);
>>> + struct phy_device *phydev = bp->phy_dev;
>>> + int err = -ENODEV;
>>> +
>>> + if (phydev)
>>> + err = phy_ethtool_set_wol(phydev, wol);
>>> +
>>> + return err;
>>> +}
>>> +
>> I think we can do in this way:
>>
>> if (phydev)
>> return phy_ethtool_set_wol(phydev, wol);
>> else
>> return -ENODEV;
>>
>>
>> we can save err. What do you say ...?
> I would do:
> if (!phydev)
> return -ENODEV;
> return phy_ethtool_set_wol(phydev, wol);
I will agree with this.... :-)
if (!phydev) {
netdev_err("bla bla...");
return -ENODEV;
}
return phy_ethtool_set_wol(phydev, wol);
--
Regards,
Varka Bhadram.
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 1/2] net: cadence: macb: add support for the WOL
2014-07-14 10:49 ` David Laight
2014-07-14 10:53 ` Varka Bhadram
@ 2014-07-15 6:58 ` Jongsung Kim
1 sibling, 0 replies; 6+ messages in thread
From: Jongsung Kim @ 2014-07-15 6:58 UTC (permalink / raw)
To: David Laight, 'Varka Bhadram', nicolas.ferre@atmel.com
Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
davem@davemloft.net
On 07/14/2014 07:49 PM, David Laight wrote:
> From: Varka Bhadram
>> On 07/14/2014 02:32 PM, Jongsung Kim wrote:
>>> This patch enables the ethtool utility to control the WOL function
>>> of the PHY connected to the GEM/MACB. (if supported)
> ...
>>> +static int macb_set_wol(struct net_device *netdev, struct ethtool_wolinfo *wol)
>>> +{
>>> + struct macb *bp = netdev_priv(netdev);
>>> + struct phy_device *phydev = bp->phy_dev;
>>> + int err = -ENODEV;
>>> +
>>> + if (phydev)
>>> + err = phy_ethtool_set_wol(phydev, wol);
>>> +
>>> + return err;
>>> +}
>>> +
>>
>> I think we can do in this way:
>>
>> if (phydev)
>> return phy_ethtool_set_wol(phydev, wol);
>> else
>> return -ENODEV;
>>
>>
>> we can save err. What do you say ...?
>
> I would do:
> if (!phydev)
> return -ENODEV;
> return phy_ethtool_set_wol(phydev, wol);
>
> Although it might even be worth moving the NULL test into the function.
> (sort of depends on style and the number of callers who need to do the test.)
>
Totally agreed. I'd be better to submit a patch about it first.
> David
>
>
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] net: cadence: macb: add support for the WOL
2014-07-14 10:15 ` Varka Bhadram
2014-07-14 10:49 ` David Laight
@ 2014-07-15 7:02 ` Jongsung Kim
1 sibling, 0 replies; 6+ messages in thread
From: Jongsung Kim @ 2014-07-15 7:02 UTC (permalink / raw)
To: Varka Bhadram, nicolas.ferre; +Cc: netdev, linux-kernel, davem
On 07/14/2014 07:15 PM, Varka Bhadram wrote:
>
> I think we can do in this way:
>
> if (phydev)
> return phy_ethtool_set_wol(phydev, wol);
> else
> return -ENODEV;
>
>
> we can save err. What do you say ...?
>
Thank you for your comment. I just wanted the function to return
at one position. However, I'll think about your point..
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-07-15 7:02 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-07-14 9:02 [PATCH 1/2] net: cadence: macb: add support for the WOL Jongsung Kim
2014-07-14 10:15 ` Varka Bhadram
2014-07-14 10:49 ` David Laight
2014-07-14 10:53 ` Varka Bhadram
2014-07-15 6:58 ` Jongsung Kim
2014-07-15 7:02 ` Jongsung Kim
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).