* [PATCH v2 0/3] net: ethtool: add ethtool_op_{get|set}_link_ksettings
@ 2016-05-08 21:44 Philippe Reynes
2016-05-08 21:44 ` [PATCH v2 1/3] net: core: " Philippe Reynes
` (3 more replies)
0 siblings, 4 replies; 9+ messages in thread
From: Philippe Reynes @ 2016-05-08 21:44 UTC (permalink / raw)
To: fugang.duan, davem, ben, kan.liang, decot, aduyck, jiri,
jacob.e.keller, tom, andrew
Cc: netdev, linux-kernel, Philippe Reynes
Ethtool callbacks {get|set}_link_ksettings may be the
same for many drivers. So we add two generics callbacks
ethtool_op_{get|set}_link_ksettings.
To use those generics callbacks, the ethernet driver must
use the pointer phydev contained in struct net_device, and
not use a private structure to store this pointer.
Changelog:
v2:
- use generic function instead of macro
- ethernet driver use the pointer phydev provided by struct net_device
Those idea were provided by Ben Hutchings,
and Florian Fainelli acknowledge them.
Philippe Reynes (3):
net: core: ethtool: add ethtool_op_{get|set}_link_ksettings
net: ethernet: fec: use phydev from struct net_device
net: ethernet: fec: use ethtool_op_{get|set}_link_ksettings
drivers/net/ethernet/freescale/fec.h | 1 -
drivers/net/ethernet/freescale/fec_main.c | 71 +++++++++--------------------
include/linux/ethtool.h | 5 ++
net/core/ethtool.c | 24 ++++++++++
4 files changed, 50 insertions(+), 51 deletions(-)
--
1.7.4.4
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2 1/3] net: core: ethtool: add ethtool_op_{get|set}_link_ksettings
2016-05-08 21:44 [PATCH v2 0/3] net: ethtool: add ethtool_op_{get|set}_link_ksettings Philippe Reynes
@ 2016-05-08 21:44 ` Philippe Reynes
2016-05-09 16:44 ` David Decotigny
2016-05-08 21:44 ` [PATCH v2 2/3] net: ethernet: fec: use phydev from struct net_device Philippe Reynes
` (2 subsequent siblings)
3 siblings, 1 reply; 9+ messages in thread
From: Philippe Reynes @ 2016-05-08 21:44 UTC (permalink / raw)
To: fugang.duan, davem, ben, kan.liang, decot, aduyck, jiri,
jacob.e.keller, tom, andrew
Cc: netdev, linux-kernel, Philippe Reynes
Ethtool callbacks {get|set}_link_ksettings are often the same, so
we add two generics functions ethtool_op_{get|set}_link_ksettings
to avoid writing severals times the same function.
Signed-off-by: Philippe Reynes <tremyfr@gmail.com>
---
include/linux/ethtool.h | 5 +++++
net/core/ethtool.c | 24 ++++++++++++++++++++++++
2 files changed, 29 insertions(+), 0 deletions(-)
diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
index 9ded8c6..9a55836 100644
--- a/include/linux/ethtool.h
+++ b/include/linux/ethtool.h
@@ -157,6 +157,11 @@ void ethtool_convert_legacy_u32_to_link_mode(unsigned long *dst,
bool ethtool_convert_link_mode_to_legacy_u32(u32 *legacy_u32,
const unsigned long *src);
+int ethtool_op_get_link_ksettings(struct net_device *ndev,
+ struct ethtool_link_ksettings *cmd);
+int ethtool_op_set_link_ksettings(struct net_device *ndev,
+ const struct ethtool_link_ksettings *cmd);
+
/**
* struct ethtool_ops - optional netdev operations
* @get_settings: DEPRECATED, use %get_link_ksettings/%set_link_ksettings
diff --git a/net/core/ethtool.c b/net/core/ethtool.c
index bdb4013..b605fb1 100644
--- a/net/core/ethtool.c
+++ b/net/core/ethtool.c
@@ -852,6 +852,30 @@ static int ethtool_set_settings(struct net_device *dev, void __user *useraddr)
return dev->ethtool_ops->set_settings(dev, &cmd);
}
+int ethtool_op_get_link_ksettings(struct net_device *ndev,
+ struct ethtool_link_ksettings *cmd)
+{
+ struct phy_device *phydev = ndev->phydev;
+
+ if (!phydev)
+ return -ENODEV;
+
+ return phy_ethtool_ksettings_get(phydev, cmd);
+}
+EXPORT_SYMBOL(ethtool_op_get_link_ksettings);
+
+int ethtool_op_set_link_ksettings(struct net_device *ndev,
+ const struct ethtool_link_ksettings *cmd)
+{
+ struct phy_device *phydev = ndev->phydev;
+
+ if (!phydev)
+ return -ENODEV;
+
+ return phy_ethtool_ksettings_set(phydev, cmd);
+}
+EXPORT_SYMBOL(ethtool_op_set_link_ksettings);
+
static noinline_for_stack int ethtool_get_drvinfo(struct net_device *dev,
void __user *useraddr)
{
--
1.7.4.4
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v2 2/3] net: ethernet: fec: use phydev from struct net_device
2016-05-08 21:44 [PATCH v2 0/3] net: ethtool: add ethtool_op_{get|set}_link_ksettings Philippe Reynes
2016-05-08 21:44 ` [PATCH v2 1/3] net: core: " Philippe Reynes
@ 2016-05-08 21:44 ` Philippe Reynes
2016-05-08 22:22 ` Ben Hutchings
2016-05-08 21:44 ` [PATCH v2 3/3] net: ethernet: fec: use ethtool_op_{get|set}_link_ksettings Philippe Reynes
2016-05-09 1:57 ` [PATCH v2 0/3] net: ethtool: add ethtool_op_{get|set}_link_ksettings Fugang Duan
3 siblings, 1 reply; 9+ messages in thread
From: Philippe Reynes @ 2016-05-08 21:44 UTC (permalink / raw)
To: fugang.duan, davem, ben, kan.liang, decot, aduyck, jiri,
jacob.e.keller, tom, andrew
Cc: netdev, linux-kernel, Philippe Reynes
The private structure contain a pointer to phydev, but the structure
net_device already contain such pointer. So we can remove the pointer
phydev in the private structure, and update the driver to use the one
contained in struct net_device.
Signed-off-by: Philippe Reynes <tremyfr@gmail.com>
---
drivers/net/ethernet/freescale/fec.h | 1 -
drivers/net/ethernet/freescale/fec_main.c | 49 ++++++++++++----------------
2 files changed, 21 insertions(+), 29 deletions(-)
diff --git a/drivers/net/ethernet/freescale/fec.h b/drivers/net/ethernet/freescale/fec.h
index 195122e..f58f9ea 100644
--- a/drivers/net/ethernet/freescale/fec.h
+++ b/drivers/net/ethernet/freescale/fec.h
@@ -517,7 +517,6 @@ struct fec_enet_private {
/* Phylib and MDIO interface */
struct mii_bus *mii_bus;
- struct phy_device *phy_dev;
int mii_timeout;
uint phy_speed;
phy_interface_t phy_interface;
diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index bfa10c3..e1d24ff 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -967,10 +967,10 @@ fec_restart(struct net_device *ndev)
rcntl &= ~(1 << 8);
/* 1G, 100M or 10M */
- if (fep->phy_dev) {
- if (fep->phy_dev->speed == SPEED_1000)
+ if (ndev->phydev) {
+ if (ndev->phydev->speed == SPEED_1000)
ecntl |= (1 << 5);
- else if (fep->phy_dev->speed == SPEED_100)
+ else if (ndev->phydev->speed == SPEED_100)
rcntl &= ~(1 << 9);
else
rcntl |= (1 << 9);
@@ -991,7 +991,7 @@ fec_restart(struct net_device *ndev)
*/
cfgr = (fep->phy_interface == PHY_INTERFACE_MODE_RMII)
? BM_MIIGSK_CFGR_RMII : BM_MIIGSK_CFGR_MII;
- if (fep->phy_dev && fep->phy_dev->speed == SPEED_10)
+ if (ndev->phydev && ndev->phydev->speed == SPEED_10)
cfgr |= BM_MIIGSK_CFGR_FRCONT_10M;
writel(cfgr, fep->hwp + FEC_MIIGSK_CFGR);
@@ -1005,7 +1005,7 @@ fec_restart(struct net_device *ndev)
/* enable pause frame*/
if ((fep->pause_flag & FEC_PAUSE_FLAG_ENABLE) ||
((fep->pause_flag & FEC_PAUSE_FLAG_AUTONEG) &&
- fep->phy_dev && fep->phy_dev->pause)) {
+ ndev->phydev && ndev->phydev->pause)) {
rcntl |= FEC_ENET_FCE;
/* set FIFO threshold parameter to reduce overrun */
@@ -1679,7 +1679,7 @@ static void fec_get_mac(struct net_device *ndev)
static void fec_enet_adjust_link(struct net_device *ndev)
{
struct fec_enet_private *fep = netdev_priv(ndev);
- struct phy_device *phy_dev = fep->phy_dev;
+ struct phy_device *phy_dev = ndev->phydev;
int status_change = 0;
/* Prevent a state halted on mii error */
@@ -1879,8 +1879,6 @@ static int fec_enet_mii_probe(struct net_device *ndev)
int phy_id;
int dev_id = fep->dev_id;
- fep->phy_dev = NULL;
-
if (fep->phy_node) {
phy_dev = of_phy_connect(ndev, fep->phy_node,
&fec_enet_adjust_link, 0,
@@ -1928,7 +1926,6 @@ static int fec_enet_mii_probe(struct net_device *ndev)
phy_dev->advertising = phy_dev->supported;
- fep->phy_dev = phy_dev;
fep->link = 0;
fep->full_duplex = 0;
@@ -2061,8 +2058,7 @@ static void fec_enet_mii_remove(struct fec_enet_private *fep)
static int fec_enet_get_link_ksettings(struct net_device *ndev,
struct ethtool_link_ksettings *cmd)
{
- struct fec_enet_private *fep = netdev_priv(ndev);
- struct phy_device *phydev = fep->phy_dev;
+ struct phy_device *phydev = ndev->phydev;
if (!phydev)
return -ENODEV;
@@ -2073,8 +2069,7 @@ static int fec_enet_get_link_ksettings(struct net_device *ndev,
static int fec_enet_set_link_ksettings(struct net_device *ndev,
const struct ethtool_link_ksettings *cmd)
{
- struct fec_enet_private *fep = netdev_priv(ndev);
- struct phy_device *phydev = fep->phy_dev;
+ struct phy_device *phydev = ndev->phydev;
if (!phydev)
return -ENODEV;
@@ -2214,7 +2209,7 @@ static int fec_enet_set_pauseparam(struct net_device *ndev,
{
struct fec_enet_private *fep = netdev_priv(ndev);
- if (!fep->phy_dev)
+ if (!ndev->phydev)
return -ENODEV;
if (pause->tx_pause != pause->rx_pause) {
@@ -2230,17 +2225,17 @@ static int fec_enet_set_pauseparam(struct net_device *ndev,
fep->pause_flag |= pause->autoneg ? FEC_PAUSE_FLAG_AUTONEG : 0;
if (pause->rx_pause || pause->autoneg) {
- fep->phy_dev->supported |= ADVERTISED_Pause;
- fep->phy_dev->advertising |= ADVERTISED_Pause;
+ ndev->phydev->supported |= ADVERTISED_Pause;
+ ndev->phydev->advertising |= ADVERTISED_Pause;
} else {
- fep->phy_dev->supported &= ~ADVERTISED_Pause;
- fep->phy_dev->advertising &= ~ADVERTISED_Pause;
+ ndev->phydev->supported &= ~ADVERTISED_Pause;
+ ndev->phydev->advertising &= ~ADVERTISED_Pause;
}
if (pause->autoneg) {
if (netif_running(ndev))
fec_stop(ndev);
- phy_start_aneg(fep->phy_dev);
+ phy_start_aneg(ndev->phydev);
}
if (netif_running(ndev)) {
napi_disable(&fep->napi);
@@ -2356,8 +2351,7 @@ static int fec_enet_get_sset_count(struct net_device *dev, int sset)
static int fec_enet_nway_reset(struct net_device *dev)
{
- struct fec_enet_private *fep = netdev_priv(dev);
- struct phy_device *phydev = fep->phy_dev;
+ struct phy_device *phydev = dev->phydev;
if (!phydev)
return -ENODEV;
@@ -2588,7 +2582,7 @@ static const struct ethtool_ops fec_enet_ethtool_ops = {
static int fec_enet_ioctl(struct net_device *ndev, struct ifreq *rq, int cmd)
{
struct fec_enet_private *fep = netdev_priv(ndev);
- struct phy_device *phydev = fep->phy_dev;
+ struct phy_device *phydev = ndev->phydev;
if (!netif_running(ndev))
return -EINVAL;
@@ -2843,7 +2837,7 @@ fec_enet_open(struct net_device *ndev)
goto err_enet_mii_probe;
napi_enable(&fep->napi);
- phy_start(fep->phy_dev);
+ phy_start(ndev->phydev);
netif_tx_start_all_queues(ndev);
device_set_wakeup_enable(&ndev->dev, fep->wol_flag &
@@ -2867,7 +2861,7 @@ fec_enet_close(struct net_device *ndev)
{
struct fec_enet_private *fep = netdev_priv(ndev);
- phy_stop(fep->phy_dev);
+ phy_stop(ndev->phydev);
if (netif_device_present(ndev)) {
napi_disable(&fep->napi);
@@ -2875,8 +2869,7 @@ fec_enet_close(struct net_device *ndev)
fec_stop(ndev);
}
- phy_disconnect(fep->phy_dev);
- fep->phy_dev = NULL;
+ phy_disconnect(ndev->phydev);
fec_enet_clk_enable(ndev, false);
pinctrl_pm_select_sleep_state(&fep->pdev->dev);
@@ -3504,7 +3497,7 @@ static int __maybe_unused fec_suspend(struct device *dev)
if (netif_running(ndev)) {
if (fep->wol_flag & FEC_WOL_FLAG_ENABLE)
fep->wol_flag |= FEC_WOL_FLAG_SLEEP_ON;
- phy_stop(fep->phy_dev);
+ phy_stop(ndev->phydev);
napi_disable(&fep->napi);
netif_tx_lock_bh(ndev);
netif_device_detach(ndev);
@@ -3564,7 +3557,7 @@ static int __maybe_unused fec_resume(struct device *dev)
netif_device_attach(ndev);
netif_tx_unlock_bh(ndev);
napi_enable(&fep->napi);
- phy_start(fep->phy_dev);
+ phy_start(ndev->phydev);
}
rtnl_unlock();
--
1.7.4.4
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v2 3/3] net: ethernet: fec: use ethtool_op_{get|set}_link_ksettings
2016-05-08 21:44 [PATCH v2 0/3] net: ethtool: add ethtool_op_{get|set}_link_ksettings Philippe Reynes
2016-05-08 21:44 ` [PATCH v2 1/3] net: core: " Philippe Reynes
2016-05-08 21:44 ` [PATCH v2 2/3] net: ethernet: fec: use phydev from struct net_device Philippe Reynes
@ 2016-05-08 21:44 ` Philippe Reynes
2016-05-09 1:57 ` [PATCH v2 0/3] net: ethtool: add ethtool_op_{get|set}_link_ksettings Fugang Duan
3 siblings, 0 replies; 9+ messages in thread
From: Philippe Reynes @ 2016-05-08 21:44 UTC (permalink / raw)
To: fugang.duan, davem, ben, kan.liang, decot, aduyck, jiri,
jacob.e.keller, tom, andrew
Cc: netdev, linux-kernel, Philippe Reynes
There are two generics functions ethtool_op_{get|set}_link_ksettings,
so we can use them instead of defining the same code in the driver.
Signed-off-by: Philippe Reynes <tremyfr@gmail.com>
---
drivers/net/ethernet/freescale/fec_main.c | 26 ++------------------------
1 files changed, 2 insertions(+), 24 deletions(-)
diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index e1d24ff..f2dae6c 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -2055,28 +2055,6 @@ static void fec_enet_mii_remove(struct fec_enet_private *fep)
}
}
-static int fec_enet_get_link_ksettings(struct net_device *ndev,
- struct ethtool_link_ksettings *cmd)
-{
- struct phy_device *phydev = ndev->phydev;
-
- if (!phydev)
- return -ENODEV;
-
- return phy_ethtool_ksettings_get(phydev, cmd);
-}
-
-static int fec_enet_set_link_ksettings(struct net_device *ndev,
- const struct ethtool_link_ksettings *cmd)
-{
- struct phy_device *phydev = ndev->phydev;
-
- if (!phydev)
- return -ENODEV;
-
- return phy_ethtool_ksettings_set(phydev, cmd);
-}
-
static void fec_enet_get_drvinfo(struct net_device *ndev,
struct ethtool_drvinfo *info)
{
@@ -2575,8 +2553,8 @@ static const struct ethtool_ops fec_enet_ethtool_ops = {
.set_tunable = fec_enet_set_tunable,
.get_wol = fec_enet_get_wol,
.set_wol = fec_enet_set_wol,
- .get_link_ksettings = fec_enet_get_link_ksettings,
- .set_link_ksettings = fec_enet_set_link_ksettings,
+ .get_link_ksettings = ethtool_op_get_link_ksettings,
+ .set_link_ksettings = ethtool_op_set_link_ksettings,
};
static int fec_enet_ioctl(struct net_device *ndev, struct ifreq *rq, int cmd)
--
1.7.4.4
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v2 2/3] net: ethernet: fec: use phydev from struct net_device
2016-05-08 21:44 ` [PATCH v2 2/3] net: ethernet: fec: use phydev from struct net_device Philippe Reynes
@ 2016-05-08 22:22 ` Ben Hutchings
2016-05-08 22:47 ` Philippe Reynes
0 siblings, 1 reply; 9+ messages in thread
From: Ben Hutchings @ 2016-05-08 22:22 UTC (permalink / raw)
To: Philippe Reynes, fugang.duan, davem, kan.liang, decot, aduyck,
jiri, jacob.e.keller, tom, andrew
Cc: netdev, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 936 bytes --]
On Sun, 2016-05-08 at 23:44 +0200, Philippe Reynes wrote:
> The private structure contain a pointer to phydev, but the structure
> net_device already contain such pointer. So we can remove the pointer
> phydev in the private structure, and update the driver to use the one
> contained in struct net_device.
But there is no central code that updates the pointer, so:
[...]
> @@ -1928,7 +1926,6 @@ static int fec_enet_mii_probe(struct net_device *ndev)
>
> phy_dev->advertising = phy_dev->supported;
>
> - fep->phy_dev = phy_dev;
you need to assign ndev->phydev here
[...]
> @@ -2875,8 +2869,7 @@ fec_enet_close(struct net_device *ndev)
> fec_stop(ndev);
> }
>
> - phy_disconnect(fep->phy_dev);
> - fep->phy_dev = NULL;
> + phy_disconnect(ndev->phydev);
[...]
and you need to set it to NULL here.
Ben.
--
Ben Hutchings
I haven't lost my mind; it's backed up on tape somewhere.
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 2/3] net: ethernet: fec: use phydev from struct net_device
2016-05-08 22:22 ` Ben Hutchings
@ 2016-05-08 22:47 ` Philippe Reynes
2016-05-08 23:01 ` Ben Hutchings
0 siblings, 1 reply; 9+ messages in thread
From: Philippe Reynes @ 2016-05-08 22:47 UTC (permalink / raw)
To: Ben Hutchings
Cc: fugang.duan, davem, kan.liang, decot, aduyck, jiri,
jacob.e.keller, tom, andrew, netdev, linux-kernel
On 09/05/16 00:22, Ben Hutchings wrote:
> On Sun, 2016-05-08 at 23:44 +0200, Philippe Reynes wrote:
>> The private structure contain a pointer to phydev, but the structure
>> net_device already contain such pointer. So we can remove the pointer
>> phydev in the private structure, and update the driver to use the one
>> contained in struct net_device.
>
> But there is no central code that updates the pointer, so:
The function phy_attach_direct and phy_detach update the pointer
phydev in the struct net_device.
>
> [...]
>> @@ -1928,7 +1926,6 @@ static int fec_enet_mii_probe(struct net_device *ndev)
>>
>> phy_dev->advertising = phy_dev->supported;
>>
>> - fep->phy_dev = phy_dev;
>
> you need to assign ndev->phydev here
The function fec_enet_mii_probe call of_phy_connect,
which call phy_connect_direct, which call phy_attach_direct.
This last function update the pointer phydev of struct net_device.
> [...]
>> @@ -2875,8 +2869,7 @@ fec_enet_close(struct net_device *ndev)
>> fec_stop(ndev);
>> }
>>
>> - phy_disconnect(fep->phy_dev);
>> - fep->phy_dev = NULL;
>> + phy_disconnect(ndev->phydev);
> [...]
>
> and you need to set it to NULL here.
The function fec_enet_close call phy_disconnect, which call phy_detach.
This last function set the pointer phydev in the struct net_device to NULL.
So from my understanding, those two lines aren't usefull.
May you confirm that I'm on the right way please ?
> Ben.
>
Philippe
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 2/3] net: ethernet: fec: use phydev from struct net_device
2016-05-08 22:47 ` Philippe Reynes
@ 2016-05-08 23:01 ` Ben Hutchings
0 siblings, 0 replies; 9+ messages in thread
From: Ben Hutchings @ 2016-05-08 23:01 UTC (permalink / raw)
To: Philippe Reynes
Cc: fugang.duan, davem, kan.liang, decot, aduyck, jiri,
jacob.e.keller, tom, andrew, netdev, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 849 bytes --]
On Mon, 2016-05-09 at 00:47 +0200, Philippe Reynes wrote:
> On 09/05/16 00:22, Ben Hutchings wrote:
> >
> > On Sun, 2016-05-08 at 23:44 +0200, Philippe Reynes wrote:
> > >
> > > The private structure contain a pointer to phydev, but the structure
> > > net_device already contain such pointer. So we can remove the pointer
> > > phydev in the private structure, and update the driver to use the one
> > > contained in struct net_device.
> > But there is no central code that updates the pointer, so:
> The function phy_attach_direct and phy_detach update the pointer
> phydev in the struct net_device.
[...]
> So from my understanding, those two lines aren't usefull.
> May you confirm that I'm on the right way please ?
Sorry, you're right.
Ben.
--
Ben Hutchings
I haven't lost my mind; it's backed up on tape somewhere.
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread
* RE: [PATCH v2 0/3] net: ethtool: add ethtool_op_{get|set}_link_ksettings
2016-05-08 21:44 [PATCH v2 0/3] net: ethtool: add ethtool_op_{get|set}_link_ksettings Philippe Reynes
` (2 preceding siblings ...)
2016-05-08 21:44 ` [PATCH v2 3/3] net: ethernet: fec: use ethtool_op_{get|set}_link_ksettings Philippe Reynes
@ 2016-05-09 1:57 ` Fugang Duan
3 siblings, 0 replies; 9+ messages in thread
From: Fugang Duan @ 2016-05-09 1:57 UTC (permalink / raw)
To: Philippe Reynes, davem, ben, kan.liang, decot, aduyck, jiri,
jacob.e.keller, tom, andrew
Cc: netdev, linux-kernel
Fom: Philippe Reynes <tremyfr@gmail.com> Sent: Monday, May 09, 2016 5:45 AM
> To: Fugang Duan <fugang.duan@nxp.com>; davem@davemloft.net;
> ben@decadent.org.uk; kan.liang@intel.com; decot@googlers.com;
> aduyck@mirantis.com; jiri@mellanox.com; jacob.e.keller@intel.com;
> tom@herbertland.com; andrew@lunn.ch
> Cc: netdev@vger.kernel.org; linux-kernel@vger.kernel.org; Philippe Reynes
> <tremyfr@gmail.com>
> Subject: [PATCH v2 0/3] net: ethtool: add ethtool_op_{get|set}_link_ksettings
>
> Ethtool callbacks {get|set}_link_ksettings may be the same for many drivers. So
> we add two generics callbacks ethtool_op_{get|set}_link_ksettings.
>
> To use those generics callbacks, the ethernet driver must use the pointer
> phydev contained in struct net_device, and not use a private structure to store
> this pointer.
>
> Changelog:
> v2:
> - use generic function instead of macro
> - ethernet driver use the pointer phydev provided by struct net_device
> Those idea were provided by Ben Hutchings,
> and Florian Fainelli acknowledge them.
>
> Philippe Reynes (3):
> net: core: ethtool: add ethtool_op_{get|set}_link_ksettings
> net: ethernet: fec: use phydev from struct net_device
> net: ethernet: fec: use ethtool_op_{get|set}_link_ksettings
>
> drivers/net/ethernet/freescale/fec.h | 1 -
> drivers/net/ethernet/freescale/fec_main.c | 71 +++++++++--------------------
> include/linux/ethtool.h | 5 ++
> net/core/ethtool.c | 24 ++++++++++
> 4 files changed, 50 insertions(+), 51 deletions(-)
>
> --
> 1.7.4.4
Acked-by: Fugang Duan <fugang.duan@nxp.com>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 1/3] net: core: ethtool: add ethtool_op_{get|set}_link_ksettings
2016-05-08 21:44 ` [PATCH v2 1/3] net: core: " Philippe Reynes
@ 2016-05-09 16:44 ` David Decotigny
0 siblings, 0 replies; 9+ messages in thread
From: David Decotigny @ 2016-05-09 16:44 UTC (permalink / raw)
To: Philippe Reynes
Cc: fugang.duan, David S. Miller, Ben Hutchings, Liang, Kan, aduyck,
jiri, jacob.e.keller, tom, andrew, netdev, linux-kernel
Are you sure this belongs to ethtool.h? For one, I believe you need an
#include phy.h somewhere. I would suggest instead to keep this in
phy.h, presumably with a more explicit name suggesting that phydev is
involved.
On Sun, May 8, 2016 at 2:44 PM, Philippe Reynes <tremyfr@gmail.com> wrote:
> Ethtool callbacks {get|set}_link_ksettings are often the same, so
> we add two generics functions ethtool_op_{get|set}_link_ksettings
> to avoid writing severals times the same function.
>
> Signed-off-by: Philippe Reynes <tremyfr@gmail.com>
> ---
> include/linux/ethtool.h | 5 +++++
> net/core/ethtool.c | 24 ++++++++++++++++++++++++
> 2 files changed, 29 insertions(+), 0 deletions(-)
>
> diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
> index 9ded8c6..9a55836 100644
> --- a/include/linux/ethtool.h
> +++ b/include/linux/ethtool.h
> @@ -157,6 +157,11 @@ void ethtool_convert_legacy_u32_to_link_mode(unsigned long *dst,
> bool ethtool_convert_link_mode_to_legacy_u32(u32 *legacy_u32,
> const unsigned long *src);
>
> +int ethtool_op_get_link_ksettings(struct net_device *ndev,
> + struct ethtool_link_ksettings *cmd);
> +int ethtool_op_set_link_ksettings(struct net_device *ndev,
> + const struct ethtool_link_ksettings *cmd);
> +
> /**
> * struct ethtool_ops - optional netdev operations
> * @get_settings: DEPRECATED, use %get_link_ksettings/%set_link_ksettings
> diff --git a/net/core/ethtool.c b/net/core/ethtool.c
> index bdb4013..b605fb1 100644
> --- a/net/core/ethtool.c
> +++ b/net/core/ethtool.c
> @@ -852,6 +852,30 @@ static int ethtool_set_settings(struct net_device *dev, void __user *useraddr)
> return dev->ethtool_ops->set_settings(dev, &cmd);
> }
>
> +int ethtool_op_get_link_ksettings(struct net_device *ndev,
> + struct ethtool_link_ksettings *cmd)
> +{
> + struct phy_device *phydev = ndev->phydev;
> +
> + if (!phydev)
> + return -ENODEV;
> +
> + return phy_ethtool_ksettings_get(phydev, cmd);
> +}
> +EXPORT_SYMBOL(ethtool_op_get_link_ksettings);
> +
> +int ethtool_op_set_link_ksettings(struct net_device *ndev,
> + const struct ethtool_link_ksettings *cmd)
> +{
> + struct phy_device *phydev = ndev->phydev;
> +
> + if (!phydev)
> + return -ENODEV;
> +
> + return phy_ethtool_ksettings_set(phydev, cmd);
> +}
> +EXPORT_SYMBOL(ethtool_op_set_link_ksettings);
> +
> static noinline_for_stack int ethtool_get_drvinfo(struct net_device *dev,
> void __user *useraddr)
> {
> --
> 1.7.4.4
>
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2016-05-09 16:45 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-05-08 21:44 [PATCH v2 0/3] net: ethtool: add ethtool_op_{get|set}_link_ksettings Philippe Reynes
2016-05-08 21:44 ` [PATCH v2 1/3] net: core: " Philippe Reynes
2016-05-09 16:44 ` David Decotigny
2016-05-08 21:44 ` [PATCH v2 2/3] net: ethernet: fec: use phydev from struct net_device Philippe Reynes
2016-05-08 22:22 ` Ben Hutchings
2016-05-08 22:47 ` Philippe Reynes
2016-05-08 23:01 ` Ben Hutchings
2016-05-08 21:44 ` [PATCH v2 3/3] net: ethernet: fec: use ethtool_op_{get|set}_link_ksettings Philippe Reynes
2016-05-09 1:57 ` [PATCH v2 0/3] net: ethtool: add ethtool_op_{get|set}_link_ksettings Fugang Duan
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.