* [PATCH 1/2] net: ethernet: marvell: pxa168_eth: use phydev from struct net_device
@ 2016-07-17 21:30 Philippe Reynes
2016-07-17 21:30 ` [PATCH 2/2] net: ethernet: marvell: pxa168_eth: use phy_ethtool_{get|set}_link_ksettings Philippe Reynes
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Philippe Reynes @ 2016-07-17 21:30 UTC (permalink / raw)
To: davem, jszhang, sergei.shtylyov, mugunthanvnm, a, fw
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/marvell/pxa168_eth.c | 36 +++++++++++++----------------
1 files changed, 16 insertions(+), 20 deletions(-)
diff --git a/drivers/net/ethernet/marvell/pxa168_eth.c b/drivers/net/ethernet/marvell/pxa168_eth.c
index 54d5154..d466326 100644
--- a/drivers/net/ethernet/marvell/pxa168_eth.c
+++ b/drivers/net/ethernet/marvell/pxa168_eth.c
@@ -247,7 +247,6 @@ struct pxa168_eth_private {
*/
struct timer_list timeout;
struct mii_bus *smi_bus;
- struct phy_device *phy;
/* clock */
struct clk *clk;
@@ -644,7 +643,7 @@ static void eth_port_start(struct net_device *dev)
struct pxa168_eth_private *pep = netdev_priv(dev);
int tx_curr_desc, rx_curr_desc;
- phy_start(pep->phy);
+ phy_start(dev->phydev);
/* Assignment of Tx CTRP of given queue */
tx_curr_desc = pep->tx_curr_desc_q;
@@ -700,7 +699,7 @@ static void eth_port_reset(struct net_device *dev)
val &= ~PCR_EN;
wrl(pep, PORT_CONFIG, val);
- phy_stop(pep->phy);
+ phy_stop(dev->phydev);
}
/*
@@ -943,7 +942,7 @@ static int set_port_config_ext(struct pxa168_eth_private *pep)
static void pxa168_eth_adjust_link(struct net_device *dev)
{
struct pxa168_eth_private *pep = netdev_priv(dev);
- struct phy_device *phy = pep->phy;
+ struct phy_device *phy = dev->phydev;
u32 cfg, cfg_o = rdl(pep, PORT_CONFIG);
u32 cfgext, cfgext_o = rdl(pep, PORT_CONFIG_EXT);
@@ -973,16 +972,17 @@ static int pxa168_init_phy(struct net_device *dev)
{
struct pxa168_eth_private *pep = netdev_priv(dev);
struct ethtool_cmd cmd;
+ struct phy_device *phy = NULL;
int err;
- if (pep->phy)
+ if (dev->phydev)
return 0;
- pep->phy = mdiobus_scan(pep->smi_bus, pep->phy_addr);
- if (IS_ERR(pep->phy))
- return PTR_ERR(pep->phy);
+ phy = mdiobus_scan(pep->smi_bus, pep->phy_addr);
+ if (IS_ERR(phy))
+ return PTR_ERR(phy);
- err = phy_connect_direct(dev, pep->phy, pxa168_eth_adjust_link,
+ err = phy_connect_direct(dev, phy, pxa168_eth_adjust_link,
pep->phy_intf);
if (err)
return err;
@@ -1366,30 +1366,26 @@ static int pxa168_smi_write(struct mii_bus *bus, int phy_addr, int regnum,
static int pxa168_eth_do_ioctl(struct net_device *dev, struct ifreq *ifr,
int cmd)
{
- struct pxa168_eth_private *pep = netdev_priv(dev);
- if (pep->phy != NULL)
- return phy_mii_ioctl(pep->phy, ifr, cmd);
+ if (dev->phydev != NULL)
+ return phy_mii_ioctl(dev->phydev, ifr, cmd);
return -EOPNOTSUPP;
}
static int pxa168_get_settings(struct net_device *dev, struct ethtool_cmd *cmd)
{
- struct pxa168_eth_private *pep = netdev_priv(dev);
int err;
- err = phy_read_status(pep->phy);
+ err = phy_read_status(dev->phydev);
if (err == 0)
- err = phy_ethtool_gset(pep->phy, cmd);
+ err = phy_ethtool_gset(dev->phydev, cmd);
return err;
}
static int pxa168_set_settings(struct net_device *dev, struct ethtool_cmd *cmd)
{
- struct pxa168_eth_private *pep = netdev_priv(dev);
-
- return phy_ethtool_sset(pep->phy, cmd);
+ return phy_ethtool_sset(dev->phydev, cmd);
}
static void pxa168_get_drvinfo(struct net_device *dev,
@@ -1569,8 +1565,8 @@ static int pxa168_eth_remove(struct platform_device *pdev)
pep->htpr, pep->htpr_dma);
pep->htpr = NULL;
}
- if (pep->phy)
- phy_disconnect(pep->phy);
+ if (dev->phydev)
+ phy_disconnect(dev->phydev);
if (pep->clk) {
clk_disable_unprepare(pep->clk);
}
--
1.7.4.4
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH 2/2] net: ethernet: marvell: pxa168_eth: use phy_ethtool_{get|set}_link_ksettings
2016-07-17 21:30 [PATCH 1/2] net: ethernet: marvell: pxa168_eth: use phydev from struct net_device Philippe Reynes
@ 2016-07-17 21:30 ` Philippe Reynes
2016-07-18 6:23 ` David Miller
2016-07-18 22:02 ` Florian Fainelli
2016-07-18 6:22 ` [PATCH 1/2] net: ethernet: marvell: pxa168_eth: use phydev from struct net_device David Miller
2016-07-18 10:14 ` Sergei Shtylyov
2 siblings, 2 replies; 7+ messages in thread
From: Philippe Reynes @ 2016-07-17 21:30 UTC (permalink / raw)
To: davem, jszhang, sergei.shtylyov, mugunthanvnm, a, fw
Cc: netdev, linux-kernel, Philippe Reynes
There are two generics functions phy_ethtool_{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/marvell/pxa168_eth.c | 39 +++++++++++++---------------
1 files changed, 18 insertions(+), 21 deletions(-)
diff --git a/drivers/net/ethernet/marvell/pxa168_eth.c b/drivers/net/ethernet/marvell/pxa168_eth.c
index d466326..aeeb2e7 100644
--- a/drivers/net/ethernet/marvell/pxa168_eth.c
+++ b/drivers/net/ethernet/marvell/pxa168_eth.c
@@ -274,8 +274,8 @@ enum hash_table_entry {
HASH_ENTRY_RECEIVE_DISCARD_BIT = 2
};
-static int pxa168_get_settings(struct net_device *dev, struct ethtool_cmd *cmd);
-static int pxa168_set_settings(struct net_device *dev, struct ethtool_cmd *cmd);
+static int pxa168_get_link_ksettings(struct net_device *dev,
+ struct ethtool_link_ksettings *cmd);
static int pxa168_init_hw(struct pxa168_eth_private *pep);
static int pxa168_init_phy(struct net_device *dev);
static void eth_port_reset(struct net_device *dev);
@@ -971,7 +971,7 @@ static void pxa168_eth_adjust_link(struct net_device *dev)
static int pxa168_init_phy(struct net_device *dev)
{
struct pxa168_eth_private *pep = netdev_priv(dev);
- struct ethtool_cmd cmd;
+ struct ethtool_link_ksettings cmd;
struct phy_device *phy = NULL;
int err;
@@ -987,20 +987,21 @@ static int pxa168_init_phy(struct net_device *dev)
if (err)
return err;
- err = pxa168_get_settings(dev, &cmd);
+ err = pxa168_get_link_ksettings(dev, &cmd);
if (err)
return err;
- cmd.phy_address = pep->phy_addr;
- cmd.speed = pep->phy_speed;
- cmd.duplex = pep->phy_duplex;
- cmd.advertising = PHY_BASIC_FEATURES;
- cmd.autoneg = AUTONEG_ENABLE;
+ cmd.base.phy_address = pep->phy_addr;
+ cmd.base.speed = pep->phy_speed;
+ cmd.base.duplex = pep->phy_duplex;
+ ethtool_convert_legacy_u32_to_link_mode(cmd.link_modes.advertising,
+ PHY_BASIC_FEATURES);
+ cmd.base.autoneg = AUTONEG_ENABLE;
- if (cmd.speed != 0)
- cmd.autoneg = AUTONEG_DISABLE;
+ if (cmd.base.speed != 0)
+ cmd.base.autoneg = AUTONEG_DISABLE;
- return pxa168_set_settings(dev, &cmd);
+ return phy_ethtool_set_link_ksettings(dev, &cmd);
}
static int pxa168_init_hw(struct pxa168_eth_private *pep)
@@ -1372,22 +1373,18 @@ static int pxa168_eth_do_ioctl(struct net_device *dev, struct ifreq *ifr,
return -EOPNOTSUPP;
}
-static int pxa168_get_settings(struct net_device *dev, struct ethtool_cmd *cmd)
+static int pxa168_get_link_ksettings(struct net_device *dev,
+ struct ethtool_link_ksettings *cmd)
{
int err;
err = phy_read_status(dev->phydev);
if (err == 0)
- err = phy_ethtool_gset(dev->phydev, cmd);
+ err = phy_ethtool_ksettings_get(dev->phydev, cmd);
return err;
}
-static int pxa168_set_settings(struct net_device *dev, struct ethtool_cmd *cmd)
-{
- return phy_ethtool_sset(dev->phydev, cmd);
-}
-
static void pxa168_get_drvinfo(struct net_device *dev,
struct ethtool_drvinfo *info)
{
@@ -1398,11 +1395,11 @@ static void pxa168_get_drvinfo(struct net_device *dev,
}
static const struct ethtool_ops pxa168_ethtool_ops = {
- .get_settings = pxa168_get_settings,
- .set_settings = pxa168_set_settings,
.get_drvinfo = pxa168_get_drvinfo,
.get_link = ethtool_op_get_link,
.get_ts_info = ethtool_op_get_ts_info,
+ .get_link_ksettings = pxa168_get_link_ksettings,
+ .set_link_ksettings = phy_ethtool_set_link_ksettings,
};
static const struct net_device_ops pxa168_eth_netdev_ops = {
--
1.7.4.4
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH 2/2] net: ethernet: marvell: pxa168_eth: use phy_ethtool_{get|set}_link_ksettings
2016-07-17 21:30 ` [PATCH 2/2] net: ethernet: marvell: pxa168_eth: use phy_ethtool_{get|set}_link_ksettings Philippe Reynes
@ 2016-07-18 6:23 ` David Miller
2016-07-18 22:02 ` Florian Fainelli
1 sibling, 0 replies; 7+ messages in thread
From: David Miller @ 2016-07-18 6:23 UTC (permalink / raw)
To: tremyfr
Cc: jszhang, sergei.shtylyov, mugunthanvnm, a, fw, netdev, linux-kernel
From: Philippe Reynes <tremyfr@gmail.com>
Date: Sun, 17 Jul 2016 23:30:46 +0200
> There are two generics functions phy_ethtool_{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>
Applied.
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH 2/2] net: ethernet: marvell: pxa168_eth: use phy_ethtool_{get|set}_link_ksettings
2016-07-17 21:30 ` [PATCH 2/2] net: ethernet: marvell: pxa168_eth: use phy_ethtool_{get|set}_link_ksettings Philippe Reynes
2016-07-18 6:23 ` David Miller
@ 2016-07-18 22:02 ` Florian Fainelli
1 sibling, 0 replies; 7+ messages in thread
From: Florian Fainelli @ 2016-07-18 22:02 UTC (permalink / raw)
To: Philippe Reynes, davem, jszhang, sergei.shtylyov, mugunthanvnm, a, fw
Cc: netdev, linux-kernel
On 07/17/2016 02:30 PM, Philippe Reynes wrote:
> There are two generics functions phy_ethtool_{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>
> ---
> - cmd.phy_address = pep->phy_addr;
> - cmd.speed = pep->phy_speed;
> - cmd.duplex = pep->phy_duplex;
> - cmd.advertising = PHY_BASIC_FEATURES;
> - cmd.autoneg = AUTONEG_ENABLE;
> + cmd.base.phy_address = pep->phy_addr;
> + cmd.base.speed = pep->phy_speed;
> + cmd.base.duplex = pep->phy_duplex;
> + ethtool_convert_legacy_u32_to_link_mode(cmd.link_modes.advertising,
> + PHY_BASIC_FEATURES);
> + cmd.base.autoneg = AUTONEG_ENABLE;
>
> - if (cmd.speed != 0)
> - cmd.autoneg = AUTONEG_DISABLE;
> + if (cmd.base.speed != 0)
> + cmd.base.autoneg = AUTONEG_DISABLE;
>
> - return pxa168_set_settings(dev, &cmd);
> + return phy_ethtool_set_link_ksettings(dev, &cmd);
This duplicates quite a bit of code that the core PHYLIB already deals
with, you should plan for a subsequent cleanup patch which removes all
of this.
--
Florian
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] net: ethernet: marvell: pxa168_eth: use phydev from struct net_device
2016-07-17 21:30 [PATCH 1/2] net: ethernet: marvell: pxa168_eth: use phydev from struct net_device Philippe Reynes
2016-07-17 21:30 ` [PATCH 2/2] net: ethernet: marvell: pxa168_eth: use phy_ethtool_{get|set}_link_ksettings Philippe Reynes
@ 2016-07-18 6:22 ` David Miller
2016-07-18 10:14 ` Sergei Shtylyov
2 siblings, 0 replies; 7+ messages in thread
From: David Miller @ 2016-07-18 6:22 UTC (permalink / raw)
To: tremyfr
Cc: jszhang, sergei.shtylyov, mugunthanvnm, a, fw, netdev, linux-kernel
From: Philippe Reynes <tremyfr@gmail.com>
Date: Sun, 17 Jul 2016 23:30:45 +0200
> 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>
Applied.
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH 1/2] net: ethernet: marvell: pxa168_eth: use phydev from struct net_device
2016-07-17 21:30 [PATCH 1/2] net: ethernet: marvell: pxa168_eth: use phydev from struct net_device Philippe Reynes
2016-07-17 21:30 ` [PATCH 2/2] net: ethernet: marvell: pxa168_eth: use phy_ethtool_{get|set}_link_ksettings Philippe Reynes
2016-07-18 6:22 ` [PATCH 1/2] net: ethernet: marvell: pxa168_eth: use phydev from struct net_device David Miller
@ 2016-07-18 10:14 ` Sergei Shtylyov
2016-07-18 22:00 ` Philippe Reynes
2 siblings, 1 reply; 7+ messages in thread
From: Sergei Shtylyov @ 2016-07-18 10:14 UTC (permalink / raw)
To: Philippe Reynes, davem, jszhang, mugunthanvnm, a, fw; +Cc: netdev, linux-kernel
Hello.
On 7/18/2016 12:30 AM, 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.
>
> Signed-off-by: Philippe Reynes <tremyfr@gmail.com>
> ---
> drivers/net/ethernet/marvell/pxa168_eth.c | 36 +++++++++++++----------------
> 1 files changed, 16 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/net/ethernet/marvell/pxa168_eth.c b/drivers/net/ethernet/marvell/pxa168_eth.c
> index 54d5154..d466326 100644
> --- a/drivers/net/ethernet/marvell/pxa168_eth.c
> +++ b/drivers/net/ethernet/marvell/pxa168_eth.c
[...]
> @@ -973,16 +972,17 @@ static int pxa168_init_phy(struct net_device *dev)
> {
> struct pxa168_eth_private *pep = netdev_priv(dev);
> struct ethtool_cmd cmd;
> + struct phy_device *phy = NULL;
Initializer not really needed.
> int err;
>
> - if (pep->phy)
> + if (dev->phydev)
> return 0;
>
> - pep->phy = mdiobus_scan(pep->smi_bus, pep->phy_addr);
> - if (IS_ERR(pep->phy))
> - return PTR_ERR(pep->phy);
> + phy = mdiobus_scan(pep->smi_bus, pep->phy_addr);
> + if (IS_ERR(phy))
> + return PTR_ERR(phy);
>
> - err = phy_connect_direct(dev, pep->phy, pxa168_eth_adjust_link,
> + err = phy_connect_direct(dev, phy, pxa168_eth_adjust_link,
> pep->phy_intf);
> if (err)
> return err;
Hm, where do you assign 'dev->phydev'?
[...]
MBR, Sergei
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH 1/2] net: ethernet: marvell: pxa168_eth: use phydev from struct net_device
2016-07-18 10:14 ` Sergei Shtylyov
@ 2016-07-18 22:00 ` Philippe Reynes
0 siblings, 0 replies; 7+ messages in thread
From: Philippe Reynes @ 2016-07-18 22:00 UTC (permalink / raw)
To: Sergei Shtylyov; +Cc: davem, jszhang, mugunthanvnm, a, fw, netdev, linux-kernel
Hi,
On 18/07/16 12:14, Sergei Shtylyov wrote:
> Hello.
>
> On 7/18/2016 12:30 AM, 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.
>>
>> Signed-off-by: Philippe Reynes<tremyfr@gmail.com>
>> ---
>> drivers/net/ethernet/marvell/pxa168_eth.c | 36 +++++++++++++----------------
>> 1 files changed, 16 insertions(+), 20 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/marvell/pxa168_eth.c b/drivers/net/ethernet/marvell/pxa168_eth.c
>> index 54d5154..d466326 100644
>> --- a/drivers/net/ethernet/marvell/pxa168_eth.c
>> +++ b/drivers/net/ethernet/marvell/pxa168_eth.c
> [...]
>> @@ -973,16 +972,17 @@ static int pxa168_init_phy(struct net_device *dev)
>> {
>> struct pxa168_eth_private *pep = netdev_priv(dev);
>> struct ethtool_cmd cmd;
>> + struct phy_device *phy = NULL;
>
> Initializer not really needed.
You're right, the first line using phy is an assign, so it's not really usefull.
>> int err;
>>
>> - if (pep->phy)
>> + if (dev->phydev)
>> return 0;
>>
>> - pep->phy = mdiobus_scan(pep->smi_bus, pep->phy_addr);
>> - if (IS_ERR(pep->phy))
>> - return PTR_ERR(pep->phy);
>> + phy = mdiobus_scan(pep->smi_bus, pep->phy_addr);
>> + if (IS_ERR(phy))
>> + return PTR_ERR(phy);
>>
>> - err = phy_connect_direct(dev, pep->phy, pxa168_eth_adjust_link,
>> + err = phy_connect_direct(dev, phy, pxa168_eth_adjust_link,
>> pep->phy_intf);
>> if (err)
>> return err;
>
> Hm, where do you assign 'dev->phydev'?
dev-> phydev is assigned in phy_connect_direct. In fact, phy_connect_direct call
phy_attach_direct, and this last function assign phydev to dev->phydev.
> [...]
>
> MBR, Sergei
>
Regards,
Philippe
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2016-07-18 22:03 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-07-17 21:30 [PATCH 1/2] net: ethernet: marvell: pxa168_eth: use phydev from struct net_device Philippe Reynes
2016-07-17 21:30 ` [PATCH 2/2] net: ethernet: marvell: pxa168_eth: use phy_ethtool_{get|set}_link_ksettings Philippe Reynes
2016-07-18 6:23 ` David Miller
2016-07-18 22:02 ` Florian Fainelli
2016-07-18 6:22 ` [PATCH 1/2] net: ethernet: marvell: pxa168_eth: use phydev from struct net_device David Miller
2016-07-18 10:14 ` Sergei Shtylyov
2016-07-18 22:00 ` Philippe Reynes
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.