[alsa-devel] [PATCH 1/3] lx6464es: remove useles pci_set_drvdata(pdev, NULL)
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); }
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,
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
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?
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
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.
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
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);
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
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.
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
participants (2)
-
Andy Shevchenko -
Takashi Iwai