2012-02-24 13:46:47

by Danny Kukawka

[permalink] [raw]
Subject: [PATCH 00/11] print MAC via printk format specifier

Print MAC/dev_addr via printk extended format specifier %pm/%pM
instead of using custom code.

These patches are against net-next.

Danny Kukawka (11):
arch/ia64/hp/sim/simeth.c: print MAC via printk format specifier
amd/hplance.c: print MAC via printk format specifier
cirrus/mac89x0: print MAC via printk format specifier
dec/tulip/de4x5: print MAC via printk format specifier
ixgbevf: print MAC via printk format specifier
sun/sunqe: print MAC via printk format specifier
xscale/ixp2000/ixpdev: print MAC via printk format specifier
usb/cdc_ncm: print MAC via printk format specifier
usb/kaweth: print MAC via printk format specifier
Staging: ft1000-pcmcia: print MAC via printk format specifier
Staging: wlags49_h2: print MAC via printk format specifier

arch/ia64/hp/sim/simeth.c | 10 +++-------
drivers/net/ethernet/amd/hplance.c | 10 ++--------
drivers/net/ethernet/cirrus/mac89x0.c | 12 ++++++++----
drivers/net/ethernet/dec/tulip/de4x5.c | 6 +-----
drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c | 8 +-------
drivers/net/ethernet/sun/sunqe.c | 10 ++--------
drivers/net/ethernet/xscale/ixp2000/ixpdev.c | 7 ++-----
drivers/net/usb/cdc_ncm.c | 6 +-----
drivers/net/usb/kaweth.c | 8 +-------
drivers/staging/ft1000/ft1000-pcmcia/ft1000_hw.c | 7 ++-----
drivers/staging/wlags49_h2/wl_cs.c | 7 ++-----
11 files changed, 25 insertions(+), 66 deletions(-)

--
1.7.8.3


2012-02-24 13:46:40

by Danny Kukawka

[permalink] [raw]
Subject: [PATCH 04/11] dec/tulip/de4x5: print MAC via printk format specifier

Print MAC/dev_addr via printk extended format specifier %pM
instead of custom code.

Signed-off-by: Danny Kukawka <[email protected]>
---
drivers/net/ethernet/dec/tulip/de4x5.c | 6 +-----
1 files changed, 1 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/dec/tulip/de4x5.c b/drivers/net/ethernet/dec/tulip/de4x5.c
index 9358340..18b106c 100644
--- a/drivers/net/ethernet/dec/tulip/de4x5.c
+++ b/drivers/net/ethernet/dec/tulip/de4x5.c
@@ -5234,11 +5234,7 @@ de4x5_dbg_open(struct net_device *dev)

if (de4x5_debug & DEBUG_OPEN) {
printk("%s: de4x5 opening with irq %d\n",dev->name,dev->irq);
- printk("\tphysical address: ");
- for (i=0;i<6;i++) {
- printk("%2.2x:",(short)dev->dev_addr[i]);
- }
- printk("\n");
+ printk("\tphysical address: %pM\n", dev->dev_addr);
printk("Descriptor head addresses:\n");
printk("\t0x%8.8lx 0x%8.8lx\n",(u_long)lp->rx_ring,(u_long)lp->tx_ring);
printk("Descriptor addresses:\nRX: ");
--
1.7.8.3

2012-02-24 13:47:18

by Danny Kukawka

[permalink] [raw]
Subject: [PATCH 03/11] cirrus/mac89x0: print MAC via printk format specifier

Print MAC/dev_addr via printk extended format specifier %pM instead
of custom code.

Use memcpy to set the address to dev->dev_addr in set_mac_address,
instead of mxing it up in a for loop with printing a debug msg.

Check also if the given address is valid.

Signed-off-by: Danny Kukawka <[email protected]>
---
drivers/net/ethernet/cirrus/mac89x0.c | 12 ++++++++----
1 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/cirrus/mac89x0.c b/drivers/net/ethernet/cirrus/mac89x0.c
index 83781f3..a7324ce 100644
--- a/drivers/net/ethernet/cirrus/mac89x0.c
+++ b/drivers/net/ethernet/cirrus/mac89x0.c
@@ -592,10 +592,14 @@ static void set_multicast_list(struct net_device *dev)
static int set_mac_address(struct net_device *dev, void *addr)
{
int i;
- printk("%s: Setting MAC address to ", dev->name);
- for (i = 0; i < 6; i++)
- printk(" %2.2x", dev->dev_addr[i] = ((unsigned char *)addr)[i]);
- printk(".\n");
+ struct sockaddr *saddr = addr;
+
+ if (!is_valid_ether_addr(addr->sa_data))
+ return -EADDRNOTAVAIL;
+
+ memcpy(dev->dev_addr, addr->sa_data, ETH_ALEN);
+ printk("%s: Setting MAC address to %pM\n", dev->name, dev->dev_addr);
+
/* set the Ethernet address */
for (i=0; i < ETH_ALEN/2; i++)
writereg(dev, PP_IA+i*2, dev->dev_addr[i*2] | (dev->dev_addr[i*2+1] << 8));
--
1.7.8.3

2012-02-24 13:47:16

by Danny Kukawka

[permalink] [raw]
Subject: [PATCH 02/11] amd/hplance.c: print MAC via printk format specifier

Print MAC/dev_addr via printk extended format specifier %pM
instead of custom code.

Signed-off-by: Danny Kukawka <[email protected]>
---
drivers/net/ethernet/amd/hplance.c | 10 ++--------
1 files changed, 2 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ethernet/amd/hplance.c b/drivers/net/ethernet/amd/hplance.c
index 86aa0d5..40f62e8 100644
--- a/drivers/net/ethernet/amd/hplance.c
+++ b/drivers/net/ethernet/amd/hplance.c
@@ -89,7 +89,6 @@ static int __devinit hplance_init_one(struct dio_dev *d,
{
struct net_device *dev;
int err = -ENOMEM;
- int i;

dev = alloc_etherdev(sizeof(struct hplance_private));
if (!dev)
@@ -107,13 +106,8 @@ static int __devinit hplance_init_one(struct dio_dev *d,

dio_set_drvdata(d, dev);

- printk(KERN_INFO "%s: %s; select code %d, addr %2.2x", dev->name, d->name, d->scode, dev->dev_addr[0]);
-
- for (i=1; i<6; i++) {
- printk(":%2.2x", dev->dev_addr[i]);
- }
-
- printk(", irq %d\n", d->ipl);
+ printk(KERN_INFO "%s: %s; select code %d, addr %pM, irq %d\n",
+ dev->name, d->name, d->scode, dev->dev_addr, d->ipl);

return 0;

--
1.7.8.3

2012-02-24 13:47:46

by Danny Kukawka

[permalink] [raw]
Subject: [PATCH 11/11] Staging: wlags49_h2: print MAC via printk format specifier

Print MAC/dev_addr via printk extended format specifier %pM
instead of custom code.

Signed-off-by: Danny Kukawka <[email protected]>
---
drivers/staging/wlags49_h2/wl_cs.c | 7 ++-----
1 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/wlags49_h2/wl_cs.c b/drivers/staging/wlags49_h2/wl_cs.c
index 2faee2d..a2cbb29 100644
--- a/drivers/staging/wlags49_h2/wl_cs.c
+++ b/drivers/staging/wlags49_h2/wl_cs.c
@@ -229,7 +229,6 @@ static int wl_adapter_resume(struct pcmcia_device *link)
void wl_adapter_insert(struct pcmcia_device *link)
{
struct net_device *dev;
- int i;
int ret;
/*--------------------------------------------------------------------*/

@@ -266,10 +265,8 @@ void wl_adapter_insert(struct pcmcia_device *link)

register_wlags_sysfs(dev);

- printk(KERN_INFO "%s: Wireless, io_addr %#03lx, irq %d, ""mac_address ",
- dev->name, dev->base_addr, dev->irq);
- for (i = 0; i < ETH_ALEN; i++)
- printk("%02X%c", dev->dev_addr[i], ((i < (ETH_ALEN-1)) ? ':' : '\n'));
+ printk(KERN_INFO "%s: Wireless, io_addr %#03lx, irq %d, mac_address"
+ " %pM\n", dev->name, dev->base_addr, dev->irq, dev->dev_addr);

DBG_LEAVE(DbgInfo);
return;
--
1.7.8.3

2012-02-24 13:47:50

by Danny Kukawka

[permalink] [raw]
Subject: [PATCH 10/11] Staging: ft1000-pcmcia: print MAC via printk format specifier

Print MAC/dev_addr via printk extended format specifier %pM
instead of custom code.

Signed-off-by: Danny Kukawka <[email protected]>
---
drivers/staging/ft1000/ft1000-pcmcia/ft1000_hw.c | 7 ++-----
1 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/ft1000/ft1000-pcmcia/ft1000_hw.c b/drivers/staging/ft1000/ft1000-pcmcia/ft1000_hw.c
index 917bbb0..7569aa0 100644
--- a/drivers/staging/ft1000/ft1000-pcmcia/ft1000_hw.c
+++ b/drivers/staging/ft1000/ft1000-pcmcia/ft1000_hw.c
@@ -2211,11 +2211,8 @@ struct net_device *init_ft1000_card(struct pcmcia_device *link,
ft1000InitProc(dev);
ft1000_card_present = 1;
SET_ETHTOOL_OPS(dev, &ops);
- printk(KERN_INFO
- "ft1000: %s: addr 0x%04lx irq %d, MAC addr %02x:%02x:%02x:%02x:%02x:%02x\n",
- dev->name, dev->base_addr, dev->irq, dev->dev_addr[0],
- dev->dev_addr[1], dev->dev_addr[2], dev->dev_addr[3],
- dev->dev_addr[4], dev->dev_addr[5]);
+ printk(KERN_INFO "ft1000: %s: addr 0x%04lx irq %d, MAC addr %pM\n",
+ dev->name, dev->base_addr, dev->irq, dev->dev_addr);
return dev;

err_unreg:
--
1.7.8.3

2012-02-24 13:47:14

by Danny Kukawka

[permalink] [raw]
Subject: [PATCH 05/11] ixgbevf: print MAC via printk format specifier

Print MAC/dev_addr via printk extended format specifier %pM
instead of custom code.

Signed-off-by: Danny Kukawka <[email protected]>
---
drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c | 8 +-------
1 files changed, 1 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
index e10221d..581c659 100644
--- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
+++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
@@ -3462,13 +3462,7 @@ static int __devinit ixgbevf_probe(struct pci_dev *pdev,
ixgbevf_init_last_counter_stats(adapter);

/* print the MAC address */
- hw_dbg(hw, "%2.2x:%2.2x:%2.2x:%2.2x:%2.2x:%2.2x\n",
- netdev->dev_addr[0],
- netdev->dev_addr[1],
- netdev->dev_addr[2],
- netdev->dev_addr[3],
- netdev->dev_addr[4],
- netdev->dev_addr[5]);
+ hw_dbg(hw, "%pM\n", netdev->dev_addr);

hw_dbg(hw, "MAC: %d\n", hw->mac.type);

--
1.7.8.3

2012-02-24 13:46:39

by Danny Kukawka

[permalink] [raw]
Subject: [PATCH 07/11] xscale/ixp2000/ixpdev: print MAC via printk format specifier

Print MAC/dev_addr via printk extended format specifier %pM
instead of custom code.

Signed-off-by: Danny Kukawka <[email protected]>
---
drivers/net/ethernet/xscale/ixp2000/ixpdev.c | 7 ++-----
1 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/xscale/ixp2000/ixpdev.c b/drivers/net/ethernet/xscale/ixp2000/ixpdev.c
index e122493..4500837 100644
--- a/drivers/net/ethernet/xscale/ixp2000/ixpdev.c
+++ b/drivers/net/ethernet/xscale/ixp2000/ixpdev.c
@@ -398,11 +398,8 @@ int ixpdev_init(int __nds_count, struct net_device **__nds,
}

for (i = 0; i < nds_count; i++) {
- printk(KERN_INFO "%s: IXP2000 MSF ethernet (port %d), "
- "%.2x:%.2x:%.2x:%.2x:%.2x:%.2x.\n", nds[i]->name, i,
- nds[i]->dev_addr[0], nds[i]->dev_addr[1],
- nds[i]->dev_addr[2], nds[i]->dev_addr[3],
- nds[i]->dev_addr[4], nds[i]->dev_addr[5]);
+ printk(KERN_INFO "%s: IXP2000 MSF ethernet (port %d), %pM.\n",
+ nds[i]->name, i, nds[i]->dev_addr);
}

return 0;
--
1.7.8.3

2012-02-24 13:48:59

by Danny Kukawka

[permalink] [raw]
Subject: [PATCH 01/11] arch/ia64/hp/sim/simeth.c: print MAC via printk format specifier

Print MAC/dev_addr via printk extended format specifier %pm
instead of custom code.

Signed-off-by: Danny Kukawka <[email protected]>
---
arch/ia64/hp/sim/simeth.c | 10 +++-------
1 files changed, 3 insertions(+), 7 deletions(-)

diff --git a/arch/ia64/hp/sim/simeth.c b/arch/ia64/hp/sim/simeth.c
index 47afcc6..440001e 100644
--- a/arch/ia64/hp/sim/simeth.c
+++ b/arch/ia64/hp/sim/simeth.c
@@ -193,7 +193,7 @@ simeth_probe1(void)
unsigned char mac_addr[ETH_ALEN];
struct simeth_local *local;
struct net_device *dev;
- int fd, i, err, rc;
+ int fd, err, rc;

/*
* XXX Fix me
@@ -236,12 +236,8 @@ simeth_probe1(void)
*/
netdev_connect(dev->irq);

- printk(KERN_INFO "%s: hosteth=%s simfd=%d, HwAddr",
- dev->name, simeth_device, local->simfd);
- for(i = 0; i < ETH_ALEN; i++) {
- printk(" %2.2x", dev->dev_addr[i]);
- }
- printk(", IRQ %d\n", dev->irq);
+ printk(KERN_INFO "%s: hosteth=%s simfd=%d, HwAddr=%pm, IRQ %d\n",
+ dev->name, simeth_device, local->simfd, dev->dev_addr, dev->irq);

return 0;
}
--
1.7.8.3

2012-02-24 13:49:20

by Danny Kukawka

[permalink] [raw]
Subject: [PATCH 08/11] usb/cdc_ncm: print MAC via printk format specifier

Print MAC/dev_addr via printk extended format specifier %pM
instead of custom code.

Signed-off-by: Danny Kukawka <[email protected]>
---
drivers/net/usb/cdc_ncm.c | 6 +-----
1 files changed, 1 insertions(+), 5 deletions(-)

diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index 3a539a9..81f86b8 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -579,11 +579,7 @@ advance:
if (temp)
goto error2;

- dev_info(&dev->udev->dev, "MAC-Address: "
- "0x%02x:0x%02x:0x%02x:0x%02x:0x%02x:0x%02x\n",
- dev->net->dev_addr[0], dev->net->dev_addr[1],
- dev->net->dev_addr[2], dev->net->dev_addr[3],
- dev->net->dev_addr[4], dev->net->dev_addr[5]);
+ dev_info(&dev->udev->dev, "MAC-Address: %pM\n", dev->net->dev_addr);

dev->in = usb_rcvbulkpipe(dev->udev,
ctx->in_ep->desc.bEndpointAddress & USB_ENDPOINT_NUMBER_MASK);
--
1.7.8.3

2012-02-24 13:49:40

by Danny Kukawka

[permalink] [raw]
Subject: [PATCH 09/11] usb/kaweth: print MAC via printk format specifier

Print MAC/dev_addr via printk extended format specifier %pM
instead of custom code.

Signed-off-by: Danny Kukawka <[email protected]>
---
drivers/net/usb/kaweth.c | 8 +-------
1 files changed, 1 insertions(+), 7 deletions(-)

diff --git a/drivers/net/usb/kaweth.c b/drivers/net/usb/kaweth.c
index d034d9c..df2a2cf 100644
--- a/drivers/net/usb/kaweth.c
+++ b/drivers/net/usb/kaweth.c
@@ -1098,13 +1098,7 @@ err_fw:
dev_info(&intf->dev, "Statistics collection: %x\n", kaweth->configuration.statistics_mask);
dev_info(&intf->dev, "Multicast filter limit: %x\n", kaweth->configuration.max_multicast_filters & ((1 << 15) - 1));
dev_info(&intf->dev, "MTU: %d\n", le16_to_cpu(kaweth->configuration.segment_size));
- dev_info(&intf->dev, "Read MAC address %2.2x:%2.2x:%2.2x:%2.2x:%2.2x:%2.2x\n",
- (int)kaweth->configuration.hw_addr[0],
- (int)kaweth->configuration.hw_addr[1],
- (int)kaweth->configuration.hw_addr[2],
- (int)kaweth->configuration.hw_addr[3],
- (int)kaweth->configuration.hw_addr[4],
- (int)kaweth->configuration.hw_addr[5]);
+ dev_info(&intf->dev, "Read MAC address %pM\n", kaweth->configuration.hw_addr);

if(!memcmp(&kaweth->configuration.hw_addr,
&bcast_addr,
--
1.7.8.3

2012-02-24 13:49:59

by Danny Kukawka

[permalink] [raw]
Subject: [PATCH 06/11] sun/sunqe: print MAC via printk format specifier

Print MAC/dev_addr via printk extended format specifier %pM
instead of custom code.

Signed-off-by: Danny Kukawka <[email protected]>
---
drivers/net/ethernet/sun/sunqe.c | 10 ++--------
1 files changed, 2 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ethernet/sun/sunqe.c b/drivers/net/ethernet/sun/sunqe.c
index b42d1c5..139d6b4 100644
--- a/drivers/net/ethernet/sun/sunqe.c
+++ b/drivers/net/ethernet/sun/sunqe.c
@@ -907,14 +907,8 @@ static int __devinit qec_ether_init(struct platform_device *op)

dev_set_drvdata(&op->dev, qe);

- printk(KERN_INFO "%s: qe channel[%d] ", dev->name, qe->channel);
- for (i = 0; i < 6; i++)
- printk ("%2.2x%c",
- dev->dev_addr[i],
- i == 5 ? ' ': ':');
- printk("\n");
-
-
+ printk(KERN_INFO "%s: qe channel[%d] %pM\n", dev->name, qe->channel,
+ dev->dev_addr);
return 0;

fail:
--
1.7.8.3

2012-02-24 13:54:04

by Oliver Neukum

[permalink] [raw]
Subject: Re: [PATCH 09/11] usb/kaweth: print MAC via printk format specifier

Am Freitag, 24. Februar 2012, 14:46:00 schrieb Danny Kukawka:
> Print MAC/dev_addr via printk extended format specifier %pM
> instead of custom code.
>
> Signed-off-by: Danny Kukawka <[email protected]>
Acked-by: Oliver Neukum <[email protected]>

2012-02-24 14:00:29

by Lennert Buytenhek

[permalink] [raw]
Subject: Re: [PATCH 07/11] xscale/ixp2000/ixpdev: print MAC via printk format specifier

On Fri, Feb 24, 2012 at 02:45:58PM +0100, Danny Kukawka wrote:

> Print MAC/dev_addr via printk extended format specifier %pM
> instead of custom code.
>
> Signed-off-by: Danny Kukawka <[email protected]>

Acked-by: Lennert Buytenhek <[email protected]>

2012-02-24 19:55:29

by David Miller

[permalink] [raw]
Subject: Re: [PATCH 10/11] Staging: ft1000-pcmcia: print MAC via printk format specifier


Danny, please don't do this.

If you are posting a "patch series" that you want me to apply, then
that entire series must be posted to netdev for review.

Otherwise I get a partial patch series recorded in patchwork which
screws up my work flow completely.

2012-02-24 20:46:18

by David Miller

[permalink] [raw]
Subject: Re: [PATCH 00/11] print MAC via printk format specifier

From: Danny Kukawka <[email protected]>
Date: Fri, 24 Feb 2012 14:45:51 +0100

> Print MAC/dev_addr via printk extended format specifier %pm/%pM
> instead of using custom code.
>
> These patches are against net-next.

All applied, but you absolutely have to start posting patch series
correctly.

If it's a series for net-next, then every single patch must be
CC:'d to netdev so it gets queued up properly in patchwork.

2012-02-25 09:03:59

by Danny Kukawka

[permalink] [raw]
Subject: Re: [PATCH 10/11] Staging: ft1000-pcmcia: print MAC via printk format specifier

On Freitag, 24. Februar 2012, David Miller wrote:
> Danny, please don't do this.
>
> If you are posting a "patch series" that you want me to apply, then
> that entire series must be posted to netdev for review.
>
> Otherwise I get a partial patch series recorded in patchwork which
> screws up my work flow completely.

Okay, I'll keep this in mind for the next time.

Danny

2012-02-25 10:15:24

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: [PATCH 03/11] cirrus/mac89x0: print MAC via printk format specifier

On Fri, Feb 24, 2012 at 14:45, Danny Kukawka <[email protected]> wrote:
> Print MAC/dev_addr via printk extended format specifier %pM instead
> of custom code.
>
> Use memcpy to set the address to dev->dev_addr in set_mac_address,
> instead of mxing it up in a for loop with printing a debug msg.
>
> Check also if the given address is valid.

Why do you sneak in this check in this patch?

> Signed-off-by: Danny Kukawka <[email protected]>
> ---
>  drivers/net/ethernet/cirrus/mac89x0.c |   12 ++++++++----
>  1 files changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/ethernet/cirrus/mac89x0.c b/drivers/net/ethernet/cirrus/mac89x0.c
> index 83781f3..a7324ce 100644
> --- a/drivers/net/ethernet/cirrus/mac89x0.c
> +++ b/drivers/net/ethernet/cirrus/mac89x0.c
> @@ -592,10 +592,14 @@ static void set_multicast_list(struct net_device *dev)
>  static int set_mac_address(struct net_device *dev, void *addr)
>  {
>        int i;
> -       printk("%s: Setting MAC address to ", dev->name);
> -       for (i = 0; i < 6; i++)
> -               printk(" %2.2x", dev->dev_addr[i] = ((unsigned char *)addr)[i]);
> -       printk(".\n");
> +       struct sockaddr *saddr = addr;
> +
> +       if (!is_valid_ether_addr(addr->sa_data))
> +               return -EADDRNOTAVAIL;
> +
> +       memcpy(dev->dev_addr, addr->sa_data, ETH_ALEN);
> +       printk("%s: Setting MAC address to %pM\n", dev->name, dev->dev_addr);
> +
>        /* set the Ethernet address */
>        for (i=0; i < ETH_ALEN/2; i++)
>                writereg(dev, PP_IA+i*2, dev->dev_addr[i*2] | (dev->dev_addr[i*2+1] << 8));

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

2012-02-28 20:45:33

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: [PATCH 03/11] cirrus/mac89x0: print MAC via printk format specifier

On Sat, Feb 25, 2012 at 11:15, Geert Uytterhoeven <[email protected]> wrote:
> On Fri, Feb 24, 2012 at 14:45, Danny Kukawka <[email protected]> wrote:
>> Print MAC/dev_addr via printk extended format specifier %pM instead
>> of custom code.
>>
>> Use memcpy to set the address to dev->dev_addr in set_mac_address,
>> instead of mxing it up in a for loop with printing a debug msg.
>>
>> Check also if the given address is valid.
>
> Why do you sneak in this check in this patch?

And it doesn't compile:

http://kisskb.ellerman.id.au/kisskb/buildresult/5752157/

drivers/net/ethernet/cirrus/mac89x0.c: In function ‘set_mac_address’:
drivers/net/ethernet/cirrus/mac89x0.c:597: warning: dereferencing
‘void *’ pointer
drivers/net/ethernet/cirrus/mac89x0.c:597: error: request for member
‘sa_data’ in something not a structure or union
drivers/net/ethernet/cirrus/mac89x0.c:600: warning: dereferencing
‘void *’ pointer
drivers/net/ethernet/cirrus/mac89x0.c:600: error: request for member
‘sa_data’ in something not a structure or union
drivers/net/ethernet/cirrus/mac89x0.c:595: warning: unused variable ‘saddr’

No patch included as I don't think this should have been applied as-is.

>> Signed-off-by: Danny Kukawka <[email protected]>
>> ---
>>  drivers/net/ethernet/cirrus/mac89x0.c |   12 ++++++++----
>>  1 files changed, 8 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/cirrus/mac89x0.c b/drivers/net/ethernet/cirrus/mac89x0.c
>> index 83781f3..a7324ce 100644
>> --- a/drivers/net/ethernet/cirrus/mac89x0.c
>> +++ b/drivers/net/ethernet/cirrus/mac89x0.c
>> @@ -592,10 +592,14 @@ static void set_multicast_list(struct net_device *dev)
>>  static int set_mac_address(struct net_device *dev, void *addr)
>>  {
>>        int i;
>> -       printk("%s: Setting MAC address to ", dev->name);
>> -       for (i = 0; i < 6; i++)
>> -               printk(" %2.2x", dev->dev_addr[i] = ((unsigned char *)addr)[i]);
>> -       printk(".\n");
>> +       struct sockaddr *saddr = addr;
>> +
>> +       if (!is_valid_ether_addr(addr->sa_data))
>> +               return -EADDRNOTAVAIL;
>> +
>> +       memcpy(dev->dev_addr, addr->sa_data, ETH_ALEN);
>> +       printk("%s: Setting MAC address to %pM\n", dev->name, dev->dev_addr);
>> +
>>        /* set the Ethernet address */
>>        for (i=0; i < ETH_ALEN/2; i++)
>>                writereg(dev, PP_IA+i*2, dev->dev_addr[i*2] | (dev->dev_addr[i*2+1] << 8));

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

2012-02-28 20:52:37

by David Miller

[permalink] [raw]
Subject: Re: [PATCH 03/11] cirrus/mac89x0: print MAC via printk format specifier

From: Geert Uytterhoeven <[email protected]>
Date: Tue, 28 Feb 2012 21:45:30 +0100

> drivers/net/ethernet/cirrus/mac89x0.c: In function ?set_mac_address?:
> drivers/net/ethernet/cirrus/mac89x0.c:597: warning: dereferencing
> ?void *? pointer
> drivers/net/ethernet/cirrus/mac89x0.c:597: error: request for member
> ?sa_data? in something not a structure or union
> drivers/net/ethernet/cirrus/mac89x0.c:600: warning: dereferencing
> ?void *? pointer
> drivers/net/ethernet/cirrus/mac89x0.c:600: error: request for member
> ?sa_data? in something not a structure or union
> drivers/net/ethernet/cirrus/mac89x0.c:595: warning: unused variable ?saddr?

Thanks, I've fixed this as follows and pushed to net-next:

--------------------
mac89x0: Fix build error.

Need to use the new 'saddr' variable not the void 'addr' in
set_mac_address().

Reported-by: Geert Uytterhoeven <[email protected]>
Signed-off-by: David S. Miller <[email protected]>
---
drivers/net/ethernet/cirrus/mac89x0.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/cirrus/mac89x0.c b/drivers/net/ethernet/cirrus/mac89x0.c
index 419825c..932fdcc 100644
--- a/drivers/net/ethernet/cirrus/mac89x0.c
+++ b/drivers/net/ethernet/cirrus/mac89x0.c
@@ -591,13 +591,13 @@ static void set_multicast_list(struct net_device *dev)

static int set_mac_address(struct net_device *dev, void *addr)
{
- int i;
struct sockaddr *saddr = addr;
+ int i;

- if (!is_valid_ether_addr(addr->sa_data))
+ if (!is_valid_ether_addr(saddr->sa_data))
return -EADDRNOTAVAIL;

- memcpy(dev->dev_addr, addr->sa_data, ETH_ALEN);
+ memcpy(dev->dev_addr, saddr->sa_data, ETH_ALEN);
printk("%s: Setting MAC address to %pM\n", dev->name, dev->dev_addr);

/* set the Ethernet address */
--
1.7.7.6

????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?

2012-02-28 21:00:52

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: [PATCH 03/11] cirrus/mac89x0: print MAC via printk format specifier

Hi David,

2012/2/28 David Miller <[email protected]>:
> From: Geert Uytterhoeven <[email protected]>
> Date: Tue, 28 Feb 2012 21:45:30 +0100
>
>> drivers/net/ethernet/cirrus/mac89x0.c: In function ‘set_mac_address’:
>> drivers/net/ethernet/cirrus/mac89x0.c:597: warning: dereferencing
>> ‘void *’ pointer
>> drivers/net/ethernet/cirrus/mac89x0.c:597: error: request for member
>> ‘sa_data’ in something not a structure or union
>> drivers/net/ethernet/cirrus/mac89x0.c:600: warning: dereferencing
>> ‘void *’ pointer
>> drivers/net/ethernet/cirrus/mac89x0.c:600: error: request for member
>> ‘sa_data’ in something not a structure or union
>> drivers/net/ethernet/cirrus/mac89x0.c:595: warning: unused variable ‘saddr’
>
> Thanks, I've fixed this as follows and pushed to net-next:
>
> --------------------
> mac89x0: Fix build error.
>
> Need to use the new 'saddr' variable not the void 'addr' in
> set_mac_address().
>
> Reported-by: Geert Uytterhoeven <[email protected]>
> Signed-off-by: David S. Miller <[email protected]>

Thanks, that fixed the build.

What about the is_valid_ether_addr() check?

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

2012-02-28 21:07:59

by David Miller

[permalink] [raw]
Subject: Re: [PATCH 03/11] cirrus/mac89x0: print MAC via printk format specifier

From: Geert Uytterhoeven <[email protected]>
Date: Tue, 28 Feb 2012 22:00:50 +0100

> What about the is_valid_ether_addr() check?

Patches welcome.

2012-02-29 07:24:09

by Danny Kukawka

[permalink] [raw]
Subject: Re: [PATCH 03/11] cirrus/mac89x0: print MAC via printk format specifier

On Dienstag, 28. Februar 2012, Geert Uytterhoeven wrote:
> Hi David,
>
> 2012/2/28 David Miller <[email protected]>:
> > From: Geert Uytterhoeven <[email protected]>
> > Date: Tue, 28 Feb 2012 21:45:30 +0100
> >
> >> drivers/net/ethernet/cirrus/mac89x0.c: In function ‘set_mac_address’:
> >> drivers/net/ethernet/cirrus/mac89x0.c:597: warning: dereferencing
> >> ‘void *’ pointer
> >> drivers/net/ethernet/cirrus/mac89x0.c:597: error: request for member
> >> ‘sa_data’ in something not a structure or union
> >> drivers/net/ethernet/cirrus/mac89x0.c:600: warning: dereferencing
> >> ‘void *’ pointer
> >> drivers/net/ethernet/cirrus/mac89x0.c:600: error: request for member
> >> ‘sa_data’ in something not a structure or union
> >> drivers/net/ethernet/cirrus/mac89x0.c:595: warning: unused variable
> >> ‘saddr’
> >
> > Thanks, I've fixed this as follows and pushed to net-next:
> >
> > --------------------
> > mac89x0: Fix build error.
> >
> > Need to use the new 'saddr' variable not the void 'addr' in
> > set_mac_address().
> >
> > Reported-by: Geert Uytterhoeven <[email protected]>
> > Signed-off-by: David S. Miller <[email protected]>
Thanks for the fix!

> Thanks, that fixed the build.
>
> What about the is_valid_ether_addr() check?

If you mean the discussion about checking if the MAC is valid in
.ndo_set_mac_address, I'm on that issue and will hopefully send a patch soon.

Danny