[PATCH 00/11] print MAC via printk format specifier

[PATCH 00/11] print MAC via printk format specifier

[Danny Kukawka]

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

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

[Danny Kukawka]

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

Signed-off-by: Danny Kukawka <danny.kukawka@bisect.de>
---
 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

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

[Danny Kukawka]

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

Signed-off-by: Danny Kukawka <danny.kukawka@bisect.de>
---
 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

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

[Danny Kukawka]

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 <danny.kukawka@bisect.de>
---
 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

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

[Danny Kukawka]

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

Signed-off-by: Danny Kukawka <danny.kukawka@bisect.de>
---
 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

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

[Danny Kukawka]

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

Signed-off-by: Danny Kukawka <danny.kukawka@bisect.de>
---
 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

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

[Danny Kukawka]

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

Signed-off-by: Danny Kukawka <danny.kukawka@bisect.de>
---
 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

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

[Danny Kukawka]

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

Signed-off-by: Danny Kukawka <danny.kukawka@bisect.de>
---
 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

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

[Danny Kukawka]

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

Signed-off-by: Danny Kukawka <danny.kukawka@bisect.de>
---
 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

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

[Danny Kukawka]

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

Signed-off-by: Danny Kukawka <danny.kukawka@bisect.de>
---
 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

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

[Danny Kukawka]

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

Signed-off-by: Danny Kukawka <danny.kukawka@bisect.de>
---
 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

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

[Danny Kukawka]

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

Signed-off-by: Danny Kukawka <danny.kukawka@bisect.de>
---
 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

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

[Oliver Neukum]

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 <danny.kukawka@bisect.de>
Acked-by: Oliver Neukum <oliver@neukum.name>

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

[Lennert Buytenhek]

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 <danny.kukawka@bisect.de>

Acked-by: Lennert Buytenhek <kernel@wantstofly.org>

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

[David Miller]

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.

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

[David Miller]

From: Danny Kukawka <danny.kukawka@bisect.de>
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.

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

[Danny Kukawka]

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

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

[Geert Uytterhoeven]

On Fri, Feb 24, 2012 at 14:45, Danny Kukawka <danny.kukawka@bisect.de> 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 <danny.kukawka@bisect.de>
> ---
>  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 -- geert@linux-m68k.org

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

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

[Geert Uytterhoeven]

On Sat, Feb 25, 2012 at 11:15, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> On Fri, Feb 24, 2012 at 14:45, Danny Kukawka <danny.kukawka@bisect.de> 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 <danny.kukawka@bisect.de>
>> ---
>>  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 -- geert@linux-m68k.org

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

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

[David Miller]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: Text/Plain; charset=iso-8859-7, Size: 1962 bytes --]

From: Geert Uytterhoeven <geert@linux-m68k.org>
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 <geert@linux-m68k.org>
Signed-off-by: David S. Miller <davem@davemloft.net>
---
 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Ç+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

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

[Geert Uytterhoeven]

Hi David,

2012/2/28 David Miller <davem@davemloft.net>:
> From: Geert Uytterhoeven <geert@linux-m68k.org>
> 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 <geert@linux-m68k.org>
> Signed-off-by: David S. Miller <davem@davemloft.net>

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 -- geert@linux-m68k.org

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

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

[David Miller]

From: Geert Uytterhoeven <geert@linux-m68k.org>
Date: Tue, 28 Feb 2012 22:00:50 +0100

> What about the is_valid_ether_addr() check?

Patches welcome.

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

[Danny Kukawka]

On Dienstag, 28. Februar 2012, Geert Uytterhoeven wrote:
> Hi David,
>
> 2012/2/28 David Miller <davem@davemloft.net>:
> > From: Geert Uytterhoeven <geert@linux-m68k.org>
> > 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 <geert@linux-m68k.org>
> > Signed-off-by: David S. Miller <davem@davemloft.net>
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