* [PATCH] greth: some driver cleanups
@ 2010-02-19 11:14 Denis Kirjanov <kirjanov@gmail.com
2010-02-19 12:04 ` Jiri Pirko
0 siblings, 1 reply; 6+ messages in thread
From: Denis Kirjanov <kirjanov@gmail.com @ 2010-02-19 11:14 UTC (permalink / raw)
To: davem; +Cc: kristoffer, jpirko, netdev
Some driver cleanups:
* convert to use phy_find_first/phy_direct_connect
* convert to use netdev_mc_* helpers
* fixed missing validate_addr hook
* removed netdev_priv castings
Signed-off-by: Denis Kirjanov <kirjanov@gmail.com>
---
drivers/net/greth.c | 38 +++++++++++++++-----------------------
1 files changed, 15 insertions(+), 23 deletions(-)
diff --git a/drivers/net/greth.c b/drivers/net/greth.c
index 457da1c..38620da 100644
--- a/drivers/net/greth.c
+++ b/drivers/net/greth.c
@@ -965,7 +965,7 @@ static int greth_set_mac_add(struct net_device *dev, void *p)
struct greth_private *greth;
struct greth_regs *regs;
- greth = (struct greth_private *) netdev_priv(dev);
+ greth = netdev_priv(dev);
regs = (struct greth_regs *) greth->regs;
if (!is_valid_ether_addr(addr->sa_data))
@@ -988,16 +988,14 @@ static u32 greth_hash_get_index(__u8 *addr)
static void greth_set_hash_filter(struct net_device *dev)
{
struct dev_mc_list *curr;
- struct greth_private *greth = (struct greth_private *) netdev_priv(dev);
+ struct greth_private *greth = netdev_priv(dev);
struct greth_regs *regs = (struct greth_regs *) greth->regs;
u32 mc_filter[2];
- unsigned int i, bitnr;
+ unsigned int bitnr;
mc_filter[0] = mc_filter[1] = 0;
- curr = dev->mc_list;
-
- for (i = 0; i < dev->mc_count; i++, curr = curr->next) {
+ netdev_for_each_mc_addr(curr, dev) {
if (!curr)
break; /* unexpected end of list */
@@ -1031,7 +1029,7 @@ static void greth_set_multicast_list(struct net_device *dev)
return;
}
- if (dev->mc_count == 0) {
+ if (!netdev_mc_count(dev)) {
cfg &= ~GRETH_CTRL_MCEN;
GRETH_REGSAVE(regs->control, cfg);
return;
@@ -1160,6 +1158,7 @@ static struct net_device_ops greth_netdev_ops = {
.ndo_stop = greth_close,
.ndo_start_xmit = greth_start_xmit,
.ndo_set_mac_address = greth_set_mac_add,
+ .ndo_validate_addr = eth_validate_addr,
};
static inline int wait_for_mdio(struct greth_private *greth)
@@ -1275,28 +1274,21 @@ static int greth_mdio_probe(struct net_device *dev)
{
struct greth_private *greth = netdev_priv(dev);
struct phy_device *phy = NULL;
- u32 interface;
- int i;
+ int ret;
/* Find the first PHY */
- for (i = 0; i < PHY_MAX_ADDR; i++) {
- if (greth->mdio->phy_map[i]) {
- phy = greth->mdio->phy_map[i];
- break;
- }
- }
+ phy = phy_find_first(greth->mdio);
+
if (!phy) {
if (netif_msg_probe(greth))
dev_err(&dev->dev, "no PHY found\n");
return -ENXIO;
}
- if (greth->gbit_mac)
- interface = PHY_INTERFACE_MODE_GMII;
- else
- interface = PHY_INTERFACE_MODE_MII;
-
- phy = phy_connect(dev, dev_name(&phy->dev), &greth_link_change, 0, interface);
+ ret = phy_connect_direct(dev, phy, &greth_link_change,
+ 0, greth->gbit_mac ?
+ PHY_INTERFACE_MODE_GMII :
+ PHY_INTERFACE_MODE_MII);
if (greth->gbit_mac)
phy->supported &= PHY_GBIT_FEATURES;
@@ -1305,10 +1297,10 @@ static int greth_mdio_probe(struct net_device *dev)
phy->advertising = phy->supported;
- if (IS_ERR(phy)) {
+ if (ret) {
if (netif_msg_ifup(greth))
dev_err(&dev->dev, "could not attach to PHY\n");
- return PTR_ERR(phy);
+ return ret;
}
greth->link = 0;
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] greth: some driver cleanups
2010-02-19 11:14 [PATCH] greth: some driver cleanups Denis Kirjanov <kirjanov@gmail.com
@ 2010-02-19 12:04 ` Jiri Pirko
2010-02-19 12:51 ` Jiri Pirko
0 siblings, 1 reply; 6+ messages in thread
From: Jiri Pirko @ 2010-02-19 12:04 UTC (permalink / raw)
To: kirjanov; +Cc: davem, kristoffer, netdev
Fri, Feb 19, 2010 at 12:14:39PM CET, kirjanov@gmail.com wrote:
>Some driver cleanups:
>* convert to use phy_find_first/phy_direct_connect
>* convert to use netdev_mc_* helpers
>* fixed missing validate_addr hook
>* removed netdev_priv castings
>
>Signed-off-by: Denis Kirjanov <kirjanov@gmail.com>
>
>---
> drivers/net/greth.c | 38 +++++++++++++++-----------------------
> 1 files changed, 15 insertions(+), 23 deletions(-)
>
>diff --git a/drivers/net/greth.c b/drivers/net/greth.c
>index 457da1c..38620da 100644
>--- a/drivers/net/greth.c
>+++ b/drivers/net/greth.c
>@@ -965,7 +965,7 @@ static int greth_set_mac_add(struct net_device *dev, void *p)
> struct greth_private *greth;
> struct greth_regs *regs;
>
>- greth = (struct greth_private *) netdev_priv(dev);
>+ greth = netdev_priv(dev);
> regs = (struct greth_regs *) greth->regs;
>
> if (!is_valid_ether_addr(addr->sa_data))
>@@ -988,16 +988,14 @@ static u32 greth_hash_get_index(__u8 *addr)
> static void greth_set_hash_filter(struct net_device *dev)
> {
> struct dev_mc_list *curr;
>- struct greth_private *greth = (struct greth_private *) netdev_priv(dev);
>+ struct greth_private *greth = netdev_priv(dev);
> struct greth_regs *regs = (struct greth_regs *) greth->regs;
> u32 mc_filter[2];
>- unsigned int i, bitnr;
>+ unsigned int bitnr;
>
> mc_filter[0] = mc_filter[1] = 0;
>
>- curr = dev->mc_list;
>-
>- for (i = 0; i < dev->mc_count; i++, curr = curr->next) {
>+ netdev_for_each_mc_addr(curr, dev) {
>
Please also remove following 2 lines:
> if (!curr)
> break; /* unexpected end of list */
------------------------------------
>@@ -1031,7 +1029,7 @@ static void greth_set_multicast_list(struct net_device *dev)
> return;
> }
>
>- if (dev->mc_count == 0) {
>+ if (!netdev_mc_count(dev)) {
> cfg &= ~GRETH_CTRL_MCEN;
> GRETH_REGSAVE(regs->control, cfg);
> return;
>@@ -1160,6 +1158,7 @@ static struct net_device_ops greth_netdev_ops = {
> .ndo_stop = greth_close,
> .ndo_start_xmit = greth_start_xmit,
> .ndo_set_mac_address = greth_set_mac_add,
>+ .ndo_validate_addr = eth_validate_addr,
> };
>
> static inline int wait_for_mdio(struct greth_private *greth)
>@@ -1275,28 +1274,21 @@ static int greth_mdio_probe(struct net_device *dev)
> {
> struct greth_private *greth = netdev_priv(dev);
> struct phy_device *phy = NULL;
>- u32 interface;
>- int i;
>+ int ret;
>
> /* Find the first PHY */
>- for (i = 0; i < PHY_MAX_ADDR; i++) {
>- if (greth->mdio->phy_map[i]) {
>- phy = greth->mdio->phy_map[i];
>- break;
>- }
>- }
>+ phy = phy_find_first(greth->mdio);
>+
> if (!phy) {
> if (netif_msg_probe(greth))
> dev_err(&dev->dev, "no PHY found\n");
> return -ENXIO;
> }
>
>- if (greth->gbit_mac)
>- interface = PHY_INTERFACE_MODE_GMII;
>- else
>- interface = PHY_INTERFACE_MODE_MII;
>-
>- phy = phy_connect(dev, dev_name(&phy->dev), &greth_link_change, 0, interface);
>+ ret = phy_connect_direct(dev, phy, &greth_link_change,
>+ 0, greth->gbit_mac ?
>+ PHY_INTERFACE_MODE_GMII :
>+ PHY_INTERFACE_MODE_MII);
>
> if (greth->gbit_mac)
> phy->supported &= PHY_GBIT_FEATURES;
>@@ -1305,10 +1297,10 @@ static int greth_mdio_probe(struct net_device *dev)
>
> phy->advertising = phy->supported;
>
When you are in this, I would rather move return value checking right after
phy_connect_direct() call
Regards,
Jirka
>- if (IS_ERR(phy)) {
>+ if (ret) {
> if (netif_msg_ifup(greth))
> dev_err(&dev->dev, "could not attach to PHY\n");
>- return PTR_ERR(phy);
>+ return ret;
> }
>
> greth->link = 0;
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] greth: some driver cleanups
2010-02-19 12:04 ` Jiri Pirko
@ 2010-02-19 12:51 ` Jiri Pirko
2010-02-19 15:00 ` Denis Kirjanov <kirjanov@gmail.com
0 siblings, 1 reply; 6+ messages in thread
From: Jiri Pirko @ 2010-02-19 12:51 UTC (permalink / raw)
To: kirjanov; +Cc: davem, kristoffer, netdev
<snip>
>>@@ -1031,7 +1029,7 @@ static void greth_set_multicast_list(struct net_device *dev)
>> return;
>> }
>>
>>- if (dev->mc_count == 0) {
>>+ if (!netdev_mc_count(dev)) {
also please use netdev_mc_empty() here.
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] greth: some driver cleanups
2010-02-19 12:51 ` Jiri Pirko
@ 2010-02-19 15:00 ` Denis Kirjanov <kirjanov@gmail.com
2010-02-19 15:25 ` Jiri Pirko
0 siblings, 1 reply; 6+ messages in thread
From: Denis Kirjanov <kirjanov@gmail.com @ 2010-02-19 15:00 UTC (permalink / raw)
To: Jiri Pirko; +Cc: davem, kristoffer, netdev
On Fri, Feb 19, 2010 at 13:51 +0100, Jiri Pirko wrote:
>
> <snip>
> >>@@ -1031,7 +1029,7 @@ static void greth_set_multicast_list(struct net_device *dev)
> >> return;
> >> }
> >>
> >>- if (dev->mc_count == 0) {
> >>+ if (!netdev_mc_count(dev)) {
> also please use netdev_mc_empty() here.
Some driver cleanups:
* convert to use phy_find_first/phy_direct_connect
* convert to use netdev_mc_* helpers
* fixed missing validate_addr hook
* removed netdev_priv castings
Signed-off-by: Denis Kirjanov <kirjanov@gmail.com>
---
drivers/net/greth.c | 49 ++++++++++++++++++-------------------------------
1 files changed, 18 insertions(+), 31 deletions(-)
diff --git a/drivers/net/greth.c b/drivers/net/greth.c
index 457da1c..d203233 100644
--- a/drivers/net/greth.c
+++ b/drivers/net/greth.c
@@ -965,7 +965,7 @@ static int greth_set_mac_add(struct net_device *dev, void *p)
struct greth_private *greth;
struct greth_regs *regs;
- greth = (struct greth_private *) netdev_priv(dev);
+ greth = netdev_priv(dev);
regs = (struct greth_regs *) greth->regs;
if (!is_valid_ether_addr(addr->sa_data))
@@ -988,20 +988,14 @@ static u32 greth_hash_get_index(__u8 *addr)
static void greth_set_hash_filter(struct net_device *dev)
{
struct dev_mc_list *curr;
- struct greth_private *greth = (struct greth_private *) netdev_priv(dev);
+ struct greth_private *greth = netdev_priv(dev);
struct greth_regs *regs = (struct greth_regs *) greth->regs;
u32 mc_filter[2];
- unsigned int i, bitnr;
+ unsigned int bitnr;
mc_filter[0] = mc_filter[1] = 0;
- curr = dev->mc_list;
-
- for (i = 0; i < dev->mc_count; i++, curr = curr->next) {
-
- if (!curr)
- break; /* unexpected end of list */
-
+ netdev_for_each_mc_addr(curr, dev) {
bitnr = greth_hash_get_index(curr->dmi_addr);
mc_filter[bitnr >> 5] |= 1 << (bitnr & 31);
}
@@ -1031,7 +1025,7 @@ static void greth_set_multicast_list(struct net_device *dev)
return;
}
- if (dev->mc_count == 0) {
+ if (netdev_mc_empty(dev)) {
cfg &= ~GRETH_CTRL_MCEN;
GRETH_REGSAVE(regs->control, cfg);
return;
@@ -1160,6 +1154,7 @@ static struct net_device_ops greth_netdev_ops = {
.ndo_stop = greth_close,
.ndo_start_xmit = greth_start_xmit,
.ndo_set_mac_address = greth_set_mac_add,
+ .ndo_validate_addr = eth_validate_addr,
};
static inline int wait_for_mdio(struct greth_private *greth)
@@ -1275,28 +1270,26 @@ static int greth_mdio_probe(struct net_device *dev)
{
struct greth_private *greth = netdev_priv(dev);
struct phy_device *phy = NULL;
- u32 interface;
- int i;
+ int ret;
/* Find the first PHY */
- for (i = 0; i < PHY_MAX_ADDR; i++) {
- if (greth->mdio->phy_map[i]) {
- phy = greth->mdio->phy_map[i];
- break;
- }
- }
+ phy = phy_find_first(greth->mdio);
+
if (!phy) {
if (netif_msg_probe(greth))
dev_err(&dev->dev, "no PHY found\n");
return -ENXIO;
}
- if (greth->gbit_mac)
- interface = PHY_INTERFACE_MODE_GMII;
- else
- interface = PHY_INTERFACE_MODE_MII;
-
- phy = phy_connect(dev, dev_name(&phy->dev), &greth_link_change, 0, interface);
+ ret = phy_connect_direct(dev, phy, &greth_link_change,
+ 0, greth->gbit_mac ?
+ PHY_INTERFACE_MODE_GMII :
+ PHY_INTERFACE_MODE_MII);
+ if (ret) {
+ if (netif_msg_ifup(greth))
+ dev_err(&dev->dev, "could not attach to PHY\n");
+ return ret;
+ }
if (greth->gbit_mac)
phy->supported &= PHY_GBIT_FEATURES;
@@ -1305,12 +1298,6 @@ static int greth_mdio_probe(struct net_device *dev)
phy->advertising = phy->supported;
- if (IS_ERR(phy)) {
- if (netif_msg_ifup(greth))
- dev_err(&dev->dev, "could not attach to PHY\n");
- return PTR_ERR(phy);
- }
-
greth->link = 0;
greth->speed = 0;
greth->duplex = -1;
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] greth: some driver cleanups
2010-02-19 15:00 ` Denis Kirjanov <kirjanov@gmail.com
@ 2010-02-19 15:25 ` Jiri Pirko
2010-02-19 21:18 ` David Miller
0 siblings, 1 reply; 6+ messages in thread
From: Jiri Pirko @ 2010-02-19 15:25 UTC (permalink / raw)
To: kirjanov; +Cc: davem, kristoffer, netdev
Fri, Feb 19, 2010 at 04:00:52PM CET, kirjanov@gmail.com wrote:
>On Fri, Feb 19, 2010 at 13:51 +0100, Jiri Pirko wrote:
>>
>> <snip>
>> >>@@ -1031,7 +1029,7 @@ static void greth_set_multicast_list(struct net_device *dev)
>> >> return;
>> >> }
>> >>
>> >>- if (dev->mc_count == 0) {
>> >>+ if (!netdev_mc_count(dev)) {
>> also please use netdev_mc_empty() here.
>Some driver cleanups:
>* convert to use phy_find_first/phy_direct_connect
>* convert to use netdev_mc_* helpers
>* fixed missing validate_addr hook
>* removed netdev_priv castings
>
>Signed-off-by: Denis Kirjanov <kirjanov@gmail.com>
>---
>
> drivers/net/greth.c | 49 ++++++++++++++++++-------------------------------
> 1 files changed, 18 insertions(+), 31 deletions(-)
>
>diff --git a/drivers/net/greth.c b/drivers/net/greth.c
>index 457da1c..d203233 100644
>--- a/drivers/net/greth.c
>+++ b/drivers/net/greth.c
>@@ -965,7 +965,7 @@ static int greth_set_mac_add(struct net_device *dev, void *p)
> struct greth_private *greth;
> struct greth_regs *regs;
>
>- greth = (struct greth_private *) netdev_priv(dev);
>+ greth = netdev_priv(dev);
> regs = (struct greth_regs *) greth->regs;
>
> if (!is_valid_ether_addr(addr->sa_data))
>@@ -988,20 +988,14 @@ static u32 greth_hash_get_index(__u8 *addr)
> static void greth_set_hash_filter(struct net_device *dev)
> {
> struct dev_mc_list *curr;
>- struct greth_private *greth = (struct greth_private *) netdev_priv(dev);
>+ struct greth_private *greth = netdev_priv(dev);
> struct greth_regs *regs = (struct greth_regs *) greth->regs;
> u32 mc_filter[2];
>- unsigned int i, bitnr;
>+ unsigned int bitnr;
>
> mc_filter[0] = mc_filter[1] = 0;
>
>- curr = dev->mc_list;
>-
>- for (i = 0; i < dev->mc_count; i++, curr = curr->next) {
>-
>- if (!curr)
>- break; /* unexpected end of list */
>-
>+ netdev_for_each_mc_addr(curr, dev) {
> bitnr = greth_hash_get_index(curr->dmi_addr);
> mc_filter[bitnr >> 5] |= 1 << (bitnr & 31);
> }
>@@ -1031,7 +1025,7 @@ static void greth_set_multicast_list(struct net_device *dev)
> return;
> }
>
>- if (dev->mc_count == 0) {
>+ if (netdev_mc_empty(dev)) {
> cfg &= ~GRETH_CTRL_MCEN;
> GRETH_REGSAVE(regs->control, cfg);
> return;
>@@ -1160,6 +1154,7 @@ static struct net_device_ops greth_netdev_ops = {
> .ndo_stop = greth_close,
> .ndo_start_xmit = greth_start_xmit,
> .ndo_set_mac_address = greth_set_mac_add,
>+ .ndo_validate_addr = eth_validate_addr,
> };
>
> static inline int wait_for_mdio(struct greth_private *greth)
>@@ -1275,28 +1270,26 @@ static int greth_mdio_probe(struct net_device *dev)
> {
> struct greth_private *greth = netdev_priv(dev);
> struct phy_device *phy = NULL;
>- u32 interface;
>- int i;
>+ int ret;
>
> /* Find the first PHY */
>- for (i = 0; i < PHY_MAX_ADDR; i++) {
>- if (greth->mdio->phy_map[i]) {
>- phy = greth->mdio->phy_map[i];
>- break;
>- }
>- }
>+ phy = phy_find_first(greth->mdio);
>+
> if (!phy) {
> if (netif_msg_probe(greth))
> dev_err(&dev->dev, "no PHY found\n");
> return -ENXIO;
> }
>
>- if (greth->gbit_mac)
>- interface = PHY_INTERFACE_MODE_GMII;
>- else
>- interface = PHY_INTERFACE_MODE_MII;
>-
>- phy = phy_connect(dev, dev_name(&phy->dev), &greth_link_change, 0, interface);
>+ ret = phy_connect_direct(dev, phy, &greth_link_change,
>+ 0, greth->gbit_mac ?
>+ PHY_INTERFACE_MODE_GMII :
>+ PHY_INTERFACE_MODE_MII);
>+ if (ret) {
>+ if (netif_msg_ifup(greth))
>+ dev_err(&dev->dev, "could not attach to PHY\n");
>+ return ret;
>+ }
>
> if (greth->gbit_mac)
> phy->supported &= PHY_GBIT_FEATURES;
>@@ -1305,12 +1298,6 @@ static int greth_mdio_probe(struct net_device *dev)
>
> phy->advertising = phy->supported;
>
>- if (IS_ERR(phy)) {
>- if (netif_msg_ifup(greth))
>- dev_err(&dev->dev, "could not attach to PHY\n");
>- return PTR_ERR(phy);
>- }
>-
> greth->link = 0;
> greth->speed = 0;
> greth->duplex = -1;
Looks good.
Reviewed-by: Jiri Pirko <jpirko@redhat.com>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] greth: some driver cleanups
2010-02-19 15:25 ` Jiri Pirko
@ 2010-02-19 21:18 ` David Miller
0 siblings, 0 replies; 6+ messages in thread
From: David Miller @ 2010-02-19 21:18 UTC (permalink / raw)
To: jpirko; +Cc: kirjanov, kristoffer, netdev
From: Jiri Pirko <jpirko@redhat.com>
Date: Fri, 19 Feb 2010 16:25:02 +0100
> Fri, Feb 19, 2010 at 04:00:52PM CET, kirjanov@gmail.com wrote:
>>On Fri, Feb 19, 2010 at 13:51 +0100, Jiri Pirko wrote:
>>>
>>> <snip>
>>> >>@@ -1031,7 +1029,7 @@ static void greth_set_multicast_list(struct net_device *dev)
>>> >> return;
>>> >> }
>>> >>
>>> >>- if (dev->mc_count == 0) {
>>> >>+ if (!netdev_mc_count(dev)) {
>>> also please use netdev_mc_empty() here.
>>Some driver cleanups:
>>* convert to use phy_find_first/phy_direct_connect
>>* convert to use netdev_mc_* helpers
>>* fixed missing validate_addr hook
>>* removed netdev_priv castings
>>
>>Signed-off-by: Denis Kirjanov <kirjanov@gmail.com>
...
> Looks good.
>
> Reviewed-by: Jiri Pirko <jpirko@redhat.com>
Applied, thanks everyone.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2010-02-19 21:18 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-02-19 11:14 [PATCH] greth: some driver cleanups Denis Kirjanov <kirjanov@gmail.com
2010-02-19 12:04 ` Jiri Pirko
2010-02-19 12:51 ` Jiri Pirko
2010-02-19 15:00 ` Denis Kirjanov <kirjanov@gmail.com
2010-02-19 15:25 ` Jiri Pirko
2010-02-19 21:18 ` David Miller
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).