2010-01-07 23:27:54

by Hartley Sweeten

Subject: [PATCH] drivers/net/skfp/skfddi.c: use %pMF to show MAC address

Use the %pMF kernel extension to display the MAC address.

The address will still be displayed in the FDDI Canonical format.

Signed-off-by: H Hartley Sweeten <[email protected]>
Cc: David S. Miller <[email protected]>

---

diff --git a/drivers/net/skfp/skfddi.c b/drivers/net/skfp/skfddi.c
index db216a7..1f9698c 100644
--- a/drivers/net/skfp/skfddi.c
+++ b/drivers/net/skfp/skfddi.c
@@ -435,13 +435,7 @@ static int skfp_driver_init(struct net_device *dev)
goto fail;
}
read_address(smc, NULL);
- pr_debug(KERN_INFO "HW-Addr: %02x %02x %02x %02x %02x %02x\n",
- smc->hw.fddi_canon_addr.a[0],
- smc->hw.fddi_canon_addr.a[1],
- smc->hw.fddi_canon_addr.a[2],
- smc->hw.fddi_canon_addr.a[3],
- smc->hw.fddi_canon_addr.a[4],
- smc->hw.fddi_canon_addr.a[5]);
+ pr_debug(KERN_INFO "HW-Addr: %pMF\n", smc->hw.fddi_canon_addr.a);
memcpy(dev->dev_addr, smc->hw.fddi_canon_addr.a, 6);

smt_reset_defaults(smc, 0);
@@ -890,15 +884,8 @@ static void skfp_ctl_set_multicast_list_wo_lock(struct net_device *dev)
(struct fddi_addr *)dmi->dmi_addr,
1);

- pr_debug(KERN_INFO "ENABLE MC ADDRESS:");
- pr_debug(" %02x %02x %02x ",
- dmi->dmi_addr[0],
- dmi->dmi_addr[1],
- dmi->dmi_addr[2]);
- pr_debug("%02x %02x %02x\n",
- dmi->dmi_addr[3],
- dmi->dmi_addr[4],
- dmi->dmi_addr[5]);
+ pr_debug(KERN_INFO "ENABLE MC ADDRESS: %pMF\n",
+ dmi->dmi_addr);
dmi = dmi->next;
} // for

2010-01-08 00:02:28

by Simon Horman

Subject: Re: [PATCH] drivers/net/skfp/skfddi.c: use %pMF to show MAC address

On Thu, Jan 07, 2010 at 04:27:46PM -0700, H Hartley Sweeten wrote:
> Use the %pMF kernel extension to display the MAC address.
>
> The address will still be displayed in the FDDI Canonical format.

I'm not sure that it matters, but prior to this patch
the address was displayed with octets delimited by spaces,
and afterwards its delimited by hyphens. So perhaps the comment
should read:

The address will now be displayed in the FDDI Canonical format.

>
> Signed-off-by: H Hartley Sweeten <[email protected]>
> Cc: David S. Miller <[email protected]>
>
> ---
>
> diff --git a/drivers/net/skfp/skfddi.c b/drivers/net/skfp/skfddi.c
> index db216a7..1f9698c 100644
> --- a/drivers/net/skfp/skfddi.c
> +++ b/drivers/net/skfp/skfddi.c
> @@ -435,13 +435,7 @@ static int skfp_driver_init(struct net_device *dev)
> goto fail;
> }
> read_address(smc, NULL);
> - pr_debug(KERN_INFO "HW-Addr: %02x %02x %02x %02x %02x %02x\n",
> - smc->hw.fddi_canon_addr.a[0],
> - smc->hw.fddi_canon_addr.a[1],
> - smc->hw.fddi_canon_addr.a[2],
> - smc->hw.fddi_canon_addr.a[3],
> - smc->hw.fddi_canon_addr.a[4],
> - smc->hw.fddi_canon_addr.a[5]);
> + pr_debug(KERN_INFO "HW-Addr: %pMF\n", smc->hw.fddi_canon_addr.a);
> memcpy(dev->dev_addr, smc->hw.fddi_canon_addr.a, 6);
>
> smt_reset_defaults(smc, 0);
> @@ -890,15 +884,8 @@ static void skfp_ctl_set_multicast_list_wo_lock(struct net_device *dev)
> (struct fddi_addr *)dmi->dmi_addr,
> 1);
>
> - pr_debug(KERN_INFO "ENABLE MC ADDRESS:");
> - pr_debug(" %02x %02x %02x ",
> - dmi->dmi_addr[0],
> - dmi->dmi_addr[1],
> - dmi->dmi_addr[2]);
> - pr_debug("%02x %02x %02x\n",
> - dmi->dmi_addr[3],
> - dmi->dmi_addr[4],
> - dmi->dmi_addr[5]);
> + pr_debug(KERN_INFO "ENABLE MC ADDRESS: %pMF\n",
> + dmi->dmi_addr);
> dmi = dmi->next;
> } // for
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html

2010-01-08 00:59:00

by David Miller

Subject: Re: [PATCH] drivers/net/skfp/skfddi.c: use %pMF to show MAC address

From: H Hartley Sweeten <[email protected]>
Date: Thu, 7 Jan 2010 16:27:46 -0700

> Use the %pMF kernel extension to display the MAC address.
>
> The address will still be displayed in the FDDI Canonical format.
>
> Signed-off-by: H Hartley Sweeten <[email protected]>

Applied.

2010-01-08 01:43:28

by Joe Perches

Subject: Re: [PATCH] drivers/net/skfp/skfddi.c: use %pMF to show MAC address

On Fri, 2010-01-08 at 11:02 +1100, Simon Horman wrote:
> On Thu, Jan 07, 2010 at 04:27:46PM -0700, H Hartley Sweeten wrote:
> > Use the %pMF kernel extension to display the MAC address.
> > The address will still be displayed in the FDDI Canonical format.

> I'm not sure that it matters, but prior to this patch
> the address was displayed with octets delimited by spaces,
> and afterwards its delimited by hyphens. So perhaps the comment
> should read:
>
> The address will now be displayed in the FDDI Canonical format.

And it probably wasn't bit reversed.

> > diff --git a/drivers/net/skfp/skfddi.c b/drivers/net/skfp/skfddi.c
> > index db216a7..1f9698c 100644
> > --- a/drivers/net/skfp/skfddi.c
> > +++ b/drivers/net/skfp/skfddi.c
> > @@ -435,13 +435,7 @@ static int skfp_driver_init(struct net_device *dev)
> > goto fail;
> > }
> > read_address(smc, NULL);
> > - pr_debug(KERN_INFO "HW-Addr: %02x %02x %02x %02x %02x %02x\n",
> > - smc->hw.fddi_canon_addr.a[0],
> > - smc->hw.fddi_canon_addr.a[1],
> > - smc->hw.fddi_canon_addr.a[2],
> > - smc->hw.fddi_canon_addr.a[3],
> > - smc->hw.fddi_canon_addr.a[4],
> > - smc->hw.fddi_canon_addr.a[5]);
> > + pr_debug(KERN_INFO "HW-Addr: %pMF\n", smc->hw.fddi_canon_addr.a);

Is fddi_canon_addr already bit reversed?
It's memcpy'd to dev->dev_addr later, so probably not.

> > smt_reset_defaults(smc, 0);
> > @@ -890,15 +884,8 @@ static void skfp_ctl_set_multicast_list_wo_lock(struct net_device *dev)
> > (struct fddi_addr *)dmi->dmi_addr,
> > 1);
> >
> > - pr_debug(KERN_INFO "ENABLE MC ADDRESS:");
> > - pr_debug(" %02x %02x %02x ",
> > - dmi->dmi_addr[0],
> > - dmi->dmi_addr[1],
> > - dmi->dmi_addr[2]);
> > - pr_debug("%02x %02x %02x\n",
> > - dmi->dmi_addr[3],
> > - dmi->dmi_addr[4],
> > - dmi->dmi_addr[5]);
> > + pr_debug(KERN_INFO "ENABLE MC ADDRESS: %pMF\n",
> > + dmi->dmi_addr);

I think you want %pM here, not the bit-reversed %pMF form.

2010-01-08 02:52:12

by Simon Horman

Subject: Re: [PATCH] drivers/net/skfp/skfddi.c: use %pMF to show MAC address

On Thu, Jan 07, 2010 at 05:43:25PM -0800, Joe Perches wrote:
> On Fri, 2010-01-08 at 11:02 +1100, Simon Horman wrote:
> > On Thu, Jan 07, 2010 at 04:27:46PM -0700, H Hartley Sweeten wrote:
> > > Use the %pMF kernel extension to display the MAC address.
> > > The address will still be displayed in the FDDI Canonical format.
>
> > I'm not sure that it matters, but prior to this patch
> > the address was displayed with octets delimited by spaces,
> > and afterwards its delimited by hyphens. So perhaps the comment
> > should read:
> >
> > The address will now be displayed in the FDDI Canonical format.
>
> And it probably wasn't bit reversed.
>
> > > diff --git a/drivers/net/skfp/skfddi.c b/drivers/net/skfp/skfddi.c
> > > index db216a7..1f9698c 100644
> > > --- a/drivers/net/skfp/skfddi.c
> > > +++ b/drivers/net/skfp/skfddi.c
> > > @@ -435,13 +435,7 @@ static int skfp_driver_init(struct net_device *dev)
> > > goto fail;
> > > }
> > > read_address(smc, NULL);
> > > - pr_debug(KERN_INFO "HW-Addr: %02x %02x %02x %02x %02x %02x\n",
> > > - smc->hw.fddi_canon_addr.a[0],
> > > - smc->hw.fddi_canon_addr.a[1],
> > > - smc->hw.fddi_canon_addr.a[2],
> > > - smc->hw.fddi_canon_addr.a[3],
> > > - smc->hw.fddi_canon_addr.a[4],
> > > - smc->hw.fddi_canon_addr.a[5]);
> > > + pr_debug(KERN_INFO "HW-Addr: %pMF\n", smc->hw.fddi_canon_addr.a);
>
> Is fddi_canon_addr already bit reversed?
> It's memcpy'd to dev->dev_addr later, so probably not.
>
> > > smt_reset_defaults(smc, 0);
> > > @@ -890,15 +884,8 @@ static void skfp_ctl_set_multicast_list_wo_lock(struct net_device *dev)
> > > (struct fddi_addr *)dmi->dmi_addr,
> > > 1);
> > >
> > > - pr_debug(KERN_INFO "ENABLE MC ADDRESS:");
> > > - pr_debug(" %02x %02x %02x ",
> > > - dmi->dmi_addr[0],
> > > - dmi->dmi_addr[1],
> > > - dmi->dmi_addr[2]);
> > > - pr_debug("%02x %02x %02x\n",
> > > - dmi->dmi_addr[3],
> > > - dmi->dmi_addr[4],
> > > - dmi->dmi_addr[5]);
> > > + pr_debug(KERN_INFO "ENABLE MC ADDRESS: %pMF\n",
> > > + dmi->dmi_addr);
>
> I think you want %pM here, not the bit-reversed %pMF form.
>

Keeping in mind that %pM uses ':' as the delimiter and %pMF uses '-',
which by all accounts is not the FDDI canonical format. But then again,
neither is the current use of ' ' as the delimiter.

... seems like there is some discussion surrounding the other patch in the
series and what the format and bit-order should be.

2010-01-08 14:57:27

by Michał Mirosław

Subject: Re: [PATCH] drivers/net/skfp/skfddi.c: use %pMF to show MAC address

2010/1/8 Joe Perches <[email protected]>:
> On Fri, 2010-01-08 at 11:02 +1100, Simon Horman wrote:
>> On Thu, Jan 07, 2010 at 04:27:46PM -0700, H Hartley Sweeten wrote:
>> > Use the %pMF kernel extension to display the MAC address.
>> > The address will still be displayed in the FDDI Canonical format.
>> I'm not sure that it matters, but prior to this patch
>> the address was displayed with octets delimited by spaces,
>> and afterwards its delimited by hyphens. So perhaps the comment
>> should read:
>>
>> The address will now be displayed in the FDDI Canonical format.
> And it probably wasn't bit reversed.
>> > diff --git a/drivers/net/skfp/skfddi.c b/drivers/net/skfp/skfddi.c
>> > index db216a7..1f9698c 100644
>> > --- a/drivers/net/skfp/skfddi.c
>> > +++ b/drivers/net/skfp/skfddi.c
>> > @@ -435,13 +435,7 @@ static ?int skfp_driver_init(struct net_device *dev)
>> > ? ? ? ? ? ? goto fail;
>> > ? ? }
>> > ? ? read_address(smc, NULL);
>> > - ? pr_debug(KERN_INFO "HW-Addr: %02x %02x %02x %02x %02x %02x\n",
>> > - ? ? ? ? ?smc->hw.fddi_canon_addr.a[0],
>> > - ? ? ? ? ?smc->hw.fddi_canon_addr.a[1],
>> > - ? ? ? ? ?smc->hw.fddi_canon_addr.a[2],
>> > - ? ? ? ? ?smc->hw.fddi_canon_addr.a[3],
>> > - ? ? ? ? ?smc->hw.fddi_canon_addr.a[4],
>> > - ? ? ? ? ?smc->hw.fddi_canon_addr.a[5]);
>> > + ? pr_debug(KERN_INFO "HW-Addr: %pMF\n", smc->hw.fddi_canon_addr.a);
> Is fddi_canon_addr already bit reversed?
> It's memcpy'd to dev->dev_addr later, so probably not.
>
>> > ? ? smt_reset_defaults(smc, 0);
>> > @@ -890,15 +884,8 @@ static void skfp_ctl_set_multicast_list_wo_lock(struct net_device *dev)
>> > ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? (struct fddi_addr *)dmi->dmi_addr,
>> > ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? 1);
>> >
>> > - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? pr_debug(KERN_INFO "ENABLE MC ADDRESS:");
>> > - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? pr_debug(" %02x %02x %02x ",
>> > - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?dmi->dmi_addr[0],
>> > - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?dmi->dmi_addr[1],
>> > - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?dmi->dmi_addr[2]);
>> > - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? pr_debug("%02x %02x %02x\n",
>> > - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?dmi->dmi_addr[3],
>> > - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?dmi->dmi_addr[4],
>> > - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?dmi->dmi_addr[5]);
>> > + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? pr_debug(KERN_INFO "ENABLE MC ADDRESS: %pMF\n",
>> > + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? dmi->dmi_addr);
> I think you want %pM here, not the bit-reversed %pMF form.

You could add %pMf for those, that are bit-reversed already? That
should take at most two extra insns for x86. ;)

Best Regards,
Micha? Miros?aw