* [PATCH 1/3] lx6464es: remove useles pci_set_drvdata(pdev, NULL)
@ 2013-05-29 10:03 Andy Shevchenko
2013-05-29 10:03 ` [PATCH 2/3] lx6464es: dump MAC in standard form Andy Shevchenko
` (2 more replies)
0 siblings, 3 replies; 11+ messages in thread
From: Andy Shevchenko @ 2013-05-29 10:03 UTC (permalink / raw)
To: alsa-devel, Takashi Iwai; +Cc: Andy Shevchenko
The driver core handles this.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
sound/pci/lx6464es/lx6464es.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/sound/pci/lx6464es/lx6464es.c b/sound/pci/lx6464es/lx6464es.c
index 298bc9b..3230e57 100644
--- a/sound/pci/lx6464es/lx6464es.c
+++ b/sound/pci/lx6464es/lx6464es.c
@@ -1139,7 +1139,6 @@ out_free:
static void snd_lx6464es_remove(struct pci_dev *pci)
{
snd_card_free(pci_get_drvdata(pci));
- pci_set_drvdata(pci, NULL);
}
--
1.8.2.rc0.22.gb3600c3
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 2/3] lx6464es: dump MAC in standard form
2013-05-29 10:03 [PATCH 1/3] lx6464es: remove useles pci_set_drvdata(pdev, NULL) Andy Shevchenko
@ 2013-05-29 10:03 ` Andy Shevchenko
2013-05-29 10:15 ` Takashi Iwai
2013-05-29 10:03 ` [PATCH 3/3] lx6464es: use %*phN to print small buffer in hex form Andy Shevchenko
2013-05-29 10:17 ` [PATCH 1/3] lx6464es: remove useles pci_set_drvdata(pdev, NULL) Takashi Iwai
2 siblings, 1 reply; 11+ messages in thread
From: Andy Shevchenko @ 2013-05-29 10:03 UTC (permalink / raw)
To: alsa-devel, Takashi Iwai; +Cc: Andy Shevchenko
Let's use %pM to print MAC wherever we need it.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
sound/pci/lx6464es/lx6464es.c | 8 ++------
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/sound/pci/lx6464es/lx6464es.c b/sound/pci/lx6464es/lx6464es.c
index 3230e57..33eb301 100644
--- a/sound/pci/lx6464es/lx6464es.c
+++ b/sound/pci/lx6464es/lx6464es.c
@@ -798,9 +798,7 @@ static int lx_init_dsp(struct lx6464es *chip)
mac_ready:
snd_printd(LXP "mac address ready read after: %dms\n", i);
- snd_printk(LXP "mac address: %02X.%02X.%02X.%02X.%02X.%02X\n",
- chip->mac_address[0], chip->mac_address[1], chip->mac_address[2],
- chip->mac_address[3], chip->mac_address[4], chip->mac_address[5]);
+ snd_printk(LXP "mac address: %pM\n", chip->mac_address);
err = lx_init_get_version_features(chip);
if (err)
@@ -1113,9 +1111,7 @@ static int snd_lx6464es_probe(struct pci_dev *pci,
sprintf(card->id, "LX6464ES_%02X%02X%02X",
chip->mac_address[3], chip->mac_address[4], chip->mac_address[5]);
- sprintf(card->shortname, "LX6464ES %02X.%02X.%02X.%02X.%02X.%02X",
- chip->mac_address[0], chip->mac_address[1], chip->mac_address[2],
- chip->mac_address[3], chip->mac_address[4], chip->mac_address[5]);
+ sprintf(card->shortname, "LX6464ES %pM", chip->mac_address);
sprintf(card->longname, "%s at 0x%lx, 0x%p, irq %i",
card->shortname, chip->port_plx,
--
1.8.2.rc0.22.gb3600c3
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 3/3] lx6464es: use %*phN to print small buffer in hex form
2013-05-29 10:03 [PATCH 1/3] lx6464es: remove useles pci_set_drvdata(pdev, NULL) Andy Shevchenko
2013-05-29 10:03 ` [PATCH 2/3] lx6464es: dump MAC in standard form Andy Shevchenko
@ 2013-05-29 10:03 ` Andy Shevchenko
2013-05-29 10:16 ` Takashi Iwai
2013-05-29 10:17 ` [PATCH 1/3] lx6464es: remove useles pci_set_drvdata(pdev, NULL) Takashi Iwai
2 siblings, 1 reply; 11+ messages in thread
From: Andy Shevchenko @ 2013-05-29 10:03 UTC (permalink / raw)
To: alsa-devel, Takashi Iwai; +Cc: Andy Shevchenko
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
sound/pci/lx6464es/lx6464es.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/sound/pci/lx6464es/lx6464es.c b/sound/pci/lx6464es/lx6464es.c
index 33eb301..7ad98f8 100644
--- a/sound/pci/lx6464es/lx6464es.c
+++ b/sound/pci/lx6464es/lx6464es.c
@@ -1108,8 +1108,7 @@ static int snd_lx6464es_probe(struct pci_dev *pci,
}
strcpy(card->driver, "LX6464ES");
- sprintf(card->id, "LX6464ES_%02X%02X%02X",
- chip->mac_address[3], chip->mac_address[4], chip->mac_address[5]);
+ sprintf(card->id, "LX6464ES_%3phN", chip->mac_address + 3);
sprintf(card->shortname, "LX6464ES %pM", chip->mac_address);
--
1.8.2.rc0.22.gb3600c3
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 2/3] lx6464es: dump MAC in standard form
2013-05-29 10:03 ` [PATCH 2/3] lx6464es: dump MAC in standard form Andy Shevchenko
@ 2013-05-29 10:15 ` Takashi Iwai
2013-05-29 11:00 ` Andy Shevchenko
0 siblings, 1 reply; 11+ messages in thread
From: Takashi Iwai @ 2013-05-29 10:15 UTC (permalink / raw)
To: Andy Shevchenko; +Cc: alsa-devel
At Wed, 29 May 2013 13:03:21 +0300,
Andy Shevchenko wrote:
>
> Let's use %pM to print MAC wherever we need it.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
> sound/pci/lx6464es/lx6464es.c | 8 ++------
> 1 file changed, 2 insertions(+), 6 deletions(-)
>
> diff --git a/sound/pci/lx6464es/lx6464es.c b/sound/pci/lx6464es/lx6464es.c
> index 3230e57..33eb301 100644
> --- a/sound/pci/lx6464es/lx6464es.c
> +++ b/sound/pci/lx6464es/lx6464es.c
> @@ -798,9 +798,7 @@ static int lx_init_dsp(struct lx6464es *chip)
>
> mac_ready:
> snd_printd(LXP "mac address ready read after: %dms\n", i);
> - snd_printk(LXP "mac address: %02X.%02X.%02X.%02X.%02X.%02X\n",
> - chip->mac_address[0], chip->mac_address[1], chip->mac_address[2],
> - chip->mac_address[3], chip->mac_address[4], chip->mac_address[5]);
> + snd_printk(LXP "mac address: %pM\n", chip->mac_address);
>
> err = lx_init_get_version_features(chip);
> if (err)
> @@ -1113,9 +1111,7 @@ static int snd_lx6464es_probe(struct pci_dev *pci,
> sprintf(card->id, "LX6464ES_%02X%02X%02X",
> chip->mac_address[3], chip->mac_address[4], chip->mac_address[5]);
>
> - sprintf(card->shortname, "LX6464ES %02X.%02X.%02X.%02X.%02X.%02X",
> - chip->mac_address[0], chip->mac_address[1], chip->mac_address[2],
> - chip->mac_address[3], chip->mac_address[4], chip->mac_address[5]);
> + sprintf(card->shortname, "LX6464ES %pM", chip->mac_address);
This will change from the upper to the lower case, thus it'll lead to
an incompatible name string, unfortunately.
Takashi
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 3/3] lx6464es: use %*phN to print small buffer in hex form
2013-05-29 10:03 ` [PATCH 3/3] lx6464es: use %*phN to print small buffer in hex form Andy Shevchenko
@ 2013-05-29 10:16 ` Takashi Iwai
2013-05-29 11:29 ` Andy Shevchenko
0 siblings, 1 reply; 11+ messages in thread
From: Takashi Iwai @ 2013-05-29 10:16 UTC (permalink / raw)
To: Andy Shevchenko; +Cc: alsa-devel
At Wed, 29 May 2013 13:03:22 +0300,
Andy Shevchenko wrote:
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
> sound/pci/lx6464es/lx6464es.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/sound/pci/lx6464es/lx6464es.c b/sound/pci/lx6464es/lx6464es.c
> index 33eb301..7ad98f8 100644
> --- a/sound/pci/lx6464es/lx6464es.c
> +++ b/sound/pci/lx6464es/lx6464es.c
> @@ -1108,8 +1108,7 @@ static int snd_lx6464es_probe(struct pci_dev *pci,
> }
>
> strcpy(card->driver, "LX6464ES");
> - sprintf(card->id, "LX6464ES_%02X%02X%02X",
> - chip->mac_address[3], chip->mac_address[4], chip->mac_address[5]);
> + sprintf(card->id, "LX6464ES_%3phN", chip->mac_address + 3);
The same problem as my previous comment. The case incompatible.
thanks,
Takashi
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/3] lx6464es: remove useles pci_set_drvdata(pdev, NULL)
2013-05-29 10:03 [PATCH 1/3] lx6464es: remove useles pci_set_drvdata(pdev, NULL) Andy Shevchenko
2013-05-29 10:03 ` [PATCH 2/3] lx6464es: dump MAC in standard form Andy Shevchenko
2013-05-29 10:03 ` [PATCH 3/3] lx6464es: use %*phN to print small buffer in hex form Andy Shevchenko
@ 2013-05-29 10:17 ` Takashi Iwai
2 siblings, 0 replies; 11+ messages in thread
From: Takashi Iwai @ 2013-05-29 10:17 UTC (permalink / raw)
To: Andy Shevchenko; +Cc: alsa-devel
At Wed, 29 May 2013 13:03:20 +0300,
Andy Shevchenko wrote:
>
> The driver core handles this.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Thanks, but I already queued a clean-up patch to remove the whole
pci_set_drvdata() with NULL for all sound/* pci drivers.
Takashi
> ---
> sound/pci/lx6464es/lx6464es.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/sound/pci/lx6464es/lx6464es.c b/sound/pci/lx6464es/lx6464es.c
> index 298bc9b..3230e57 100644
> --- a/sound/pci/lx6464es/lx6464es.c
> +++ b/sound/pci/lx6464es/lx6464es.c
> @@ -1139,7 +1139,6 @@ out_free:
> static void snd_lx6464es_remove(struct pci_dev *pci)
> {
> snd_card_free(pci_get_drvdata(pci));
> - pci_set_drvdata(pci, NULL);
> }
>
>
> --
> 1.8.2.rc0.22.gb3600c3
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/3] lx6464es: dump MAC in standard form
2013-05-29 10:15 ` Takashi Iwai
@ 2013-05-29 11:00 ` Andy Shevchenko
2013-05-29 11:03 ` Takashi Iwai
0 siblings, 1 reply; 11+ messages in thread
From: Andy Shevchenko @ 2013-05-29 11:00 UTC (permalink / raw)
To: Takashi Iwai; +Cc: alsa-devel
On Wed, 2013-05-29 at 12:15 +0200, Takashi Iwai wrote:
> At Wed, 29 May 2013 13:03:21 +0300,
> Andy Shevchenko wrote:
> >
> > Let's use %pM to print MAC wherever we need it.
> >
> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > ---
> > sound/pci/lx6464es/lx6464es.c | 8 ++------
> > 1 file changed, 2 insertions(+), 6 deletions(-)
> >
> > diff --git a/sound/pci/lx6464es/lx6464es.c b/sound/pci/lx6464es/lx6464es.c
> > index 3230e57..33eb301 100644
> > --- a/sound/pci/lx6464es/lx6464es.c
> > +++ b/sound/pci/lx6464es/lx6464es.c
> > @@ -798,9 +798,7 @@ static int lx_init_dsp(struct lx6464es *chip)
> >
> > mac_ready:
> > snd_printd(LXP "mac address ready read after: %dms\n", i);
> > - snd_printk(LXP "mac address: %02X.%02X.%02X.%02X.%02X.%02X\n",
> > - chip->mac_address[0], chip->mac_address[1], chip->mac_address[2],
> > - chip->mac_address[3], chip->mac_address[4], chip->mac_address[5]);
> > + snd_printk(LXP "mac address: %pM\n", chip->mac_address);
> >
> > err = lx_init_get_version_features(chip);
> > if (err)
> > @@ -1113,9 +1111,7 @@ static int snd_lx6464es_probe(struct pci_dev *pci,
> > sprintf(card->id, "LX6464ES_%02X%02X%02X",
> > chip->mac_address[3], chip->mac_address[4], chip->mac_address[5]);
> >
> > - sprintf(card->shortname, "LX6464ES %02X.%02X.%02X.%02X.%02X.%02X",
> > - chip->mac_address[0], chip->mac_address[1], chip->mac_address[2],
> > - chip->mac_address[3], chip->mac_address[4], chip->mac_address[5]);
> > + sprintf(card->shortname, "LX6464ES %pM", chip->mac_address);
>
> This will change from the upper to the lower case, thus it'll lead to
> an incompatible name string, unfortunately.
Who is the user of this string?
--
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/3] lx6464es: dump MAC in standard form
2013-05-29 11:00 ` Andy Shevchenko
@ 2013-05-29 11:03 ` Takashi Iwai
2013-05-29 11:31 ` Andy Shevchenko
0 siblings, 1 reply; 11+ messages in thread
From: Takashi Iwai @ 2013-05-29 11:03 UTC (permalink / raw)
To: Andy Shevchenko; +Cc: alsa-devel
At Wed, 29 May 2013 14:00:57 +0300,
Andy Shevchenko wrote:
>
> On Wed, 2013-05-29 at 12:15 +0200, Takashi Iwai wrote:
> > At Wed, 29 May 2013 13:03:21 +0300,
> > Andy Shevchenko wrote:
> > >
> > > Let's use %pM to print MAC wherever we need it.
> > >
> > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > > ---
> > > sound/pci/lx6464es/lx6464es.c | 8 ++------
> > > 1 file changed, 2 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/sound/pci/lx6464es/lx6464es.c b/sound/pci/lx6464es/lx6464es.c
> > > index 3230e57..33eb301 100644
> > > --- a/sound/pci/lx6464es/lx6464es.c
> > > +++ b/sound/pci/lx6464es/lx6464es.c
> > > @@ -798,9 +798,7 @@ static int lx_init_dsp(struct lx6464es *chip)
> > >
> > > mac_ready:
> > > snd_printd(LXP "mac address ready read after: %dms\n", i);
> > > - snd_printk(LXP "mac address: %02X.%02X.%02X.%02X.%02X.%02X\n",
> > > - chip->mac_address[0], chip->mac_address[1], chip->mac_address[2],
> > > - chip->mac_address[3], chip->mac_address[4], chip->mac_address[5]);
> > > + snd_printk(LXP "mac address: %pM\n", chip->mac_address);
> > >
> > > err = lx_init_get_version_features(chip);
> > > if (err)
> > > @@ -1113,9 +1111,7 @@ static int snd_lx6464es_probe(struct pci_dev *pci,
> > > sprintf(card->id, "LX6464ES_%02X%02X%02X",
> > > chip->mac_address[3], chip->mac_address[4], chip->mac_address[5]);
> > >
> > > - sprintf(card->shortname, "LX6464ES %02X.%02X.%02X.%02X.%02X.%02X",
> > > - chip->mac_address[0], chip->mac_address[1], chip->mac_address[2],
> > > - chip->mac_address[3], chip->mac_address[4], chip->mac_address[5]);
> > > + sprintf(card->shortname, "LX6464ES %pM", chip->mac_address);
> >
> > This will change from the upper to the lower case, thus it'll lead to
> > an incompatible name string, unfortunately.
>
> Who is the user of this string?
It's exposed to the user-space via ioctl and it can be used as an id
string. So this will break user-space compatibility.
Takashi
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 3/3] lx6464es: use %*phN to print small buffer in hex form
2013-05-29 10:16 ` Takashi Iwai
@ 2013-05-29 11:29 ` Andy Shevchenko
0 siblings, 0 replies; 11+ messages in thread
From: Andy Shevchenko @ 2013-05-29 11:29 UTC (permalink / raw)
To: Takashi Iwai; +Cc: alsa-devel
On Wed, 2013-05-29 at 12:16 +0200, Takashi Iwai wrote:
> > strcpy(card->driver, "LX6464ES");
> > - sprintf(card->id, "LX6464ES_%02X%02X%02X",
> > - chip->mac_address[3], chip->mac_address[4], chip->mac_address[5]);
> > + sprintf(card->id, "LX6464ES_%3phN", chip->mac_address + 3);
>
> The same problem as my previous comment. The case incompatible.
Here is card->id, which might be used by userspace in defined form.
I rather agree with your objection here.
--
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/3] lx6464es: dump MAC in standard form
2013-05-29 11:03 ` Takashi Iwai
@ 2013-05-29 11:31 ` Andy Shevchenko
2013-05-29 12:15 ` Takashi Iwai
0 siblings, 1 reply; 11+ messages in thread
From: Andy Shevchenko @ 2013-05-29 11:31 UTC (permalink / raw)
To: Takashi Iwai; +Cc: alsa-devel
On Wed, 2013-05-29 at 13:03 +0200, Takashi Iwai wrote:
> > > > - sprintf(card->shortname, "LX6464ES %02X.%02X.%02X.%02X.%02X.%02X",
> > > > - chip->mac_address[0], chip->mac_address[1], chip->mac_address[2],
> > > > - chip->mac_address[3], chip->mac_address[4], chip->mac_address[5]);
> > > > + sprintf(card->shortname, "LX6464ES %pM", chip->mac_address);
> > >
> > > This will change from the upper to the lower case, thus it'll lead to
> > > an incompatible name string, unfortunately.
> >
> > Who is the user of this string?
>
> It's exposed to the user-space via ioctl and it can be used as an id
> string. So this will break user-space compatibility.
MAC is theoretically set of arbitrary 6 bytes.
Only what I can see here is the difference between '.' and ':' as a
delimiter. But the MAC address form is defined in IEEE 802 standards.
Anyway, I am okay if you reject this one.
--
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/3] lx6464es: dump MAC in standard form
2013-05-29 11:31 ` Andy Shevchenko
@ 2013-05-29 12:15 ` Takashi Iwai
0 siblings, 0 replies; 11+ messages in thread
From: Takashi Iwai @ 2013-05-29 12:15 UTC (permalink / raw)
To: Andy Shevchenko; +Cc: alsa-devel
At Wed, 29 May 2013 14:31:07 +0300,
Andy Shevchenko wrote:
>
> On Wed, 2013-05-29 at 13:03 +0200, Takashi Iwai wrote:
>
> > > > > - sprintf(card->shortname, "LX6464ES %02X.%02X.%02X.%02X.%02X.%02X",
> > > > > - chip->mac_address[0], chip->mac_address[1], chip->mac_address[2],
> > > > > - chip->mac_address[3], chip->mac_address[4], chip->mac_address[5]);
> > > > > + sprintf(card->shortname, "LX6464ES %pM", chip->mac_address);
> > > >
> > > > This will change from the upper to the lower case, thus it'll lead to
> > > > an incompatible name string, unfortunately.
> > >
> > > Who is the user of this string?
> >
> > It's exposed to the user-space via ioctl and it can be used as an id
> > string. So this will break user-space compatibility.
>
> MAC is theoretically set of arbitrary 6 bytes.
> Only what I can see here is the difference between '.' and ':' as a
> delimiter. But the MAC address form is defined in IEEE 802 standards.
It doesn't matter whether it's a valid MAC representation or not.
The problem is only that this patch will change the string
representation, thus it becomes incompatible with older kernels.
That's the only point.
> Anyway, I am okay if you reject this one.
Good :)
thanks,
Takashi
> --
> Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Intel Finland Oy
>
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2013-05-29 12:15 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-05-29 10:03 [PATCH 1/3] lx6464es: remove useles pci_set_drvdata(pdev, NULL) Andy Shevchenko
2013-05-29 10:03 ` [PATCH 2/3] lx6464es: dump MAC in standard form Andy Shevchenko
2013-05-29 10:15 ` Takashi Iwai
2013-05-29 11:00 ` Andy Shevchenko
2013-05-29 11:03 ` Takashi Iwai
2013-05-29 11:31 ` Andy Shevchenko
2013-05-29 12:15 ` Takashi Iwai
2013-05-29 10:03 ` [PATCH 3/3] lx6464es: use %*phN to print small buffer in hex form Andy Shevchenko
2013-05-29 10:16 ` Takashi Iwai
2013-05-29 11:29 ` Andy Shevchenko
2013-05-29 10:17 ` [PATCH 1/3] lx6464es: remove useles pci_set_drvdata(pdev, NULL) Takashi Iwai
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.