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 <[email protected]>
---
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
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 <[email protected]>
> ---
> 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.
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
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.
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
>
>
>
>
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..