mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Takashi Iwai <tiwai@suse.de>
To: Chris Rorvick <chris@rorvick.com>
Cc: alsa-devel@alsa-project.org, linux-kernel@vger.kernel.org,
	Stefan Hajnoczi <stefanha@gmail.com>
Subject: Re: [PATCH 3/4] ALSA: line6: Pass driver name to line6_probe()
Date: Fri, 06 Feb 2015 16:04:42 +0100	[thread overview]
Message-ID: <s5hfvajw06t.wl-tiwai@suse.de> (raw)
In-Reply-To: <1423234272-7348-4-git-send-email-chris@rorvick.com>

At Fri,  6 Feb 2015 08:51:11 -0600,
Chris Rorvick wrote:
> 
> Provide a descriptive name for each driver instead of calling all of
> them "line6usb".

This needs to be done carefully.  This string is referred in alsa-lib
to pick up the the configuration file.  So, this change shall break
the compatibility.

If we ever want to pick up a different alsa-lib configuration
depending on each line6 driver type, then yes, we should give the
individual driver name.  If we want to keep rather the common
configuration file (so far there is none, but if any in furture), then
we should keep the common driver name.

And, the decision must be done now.  From now on, basically we are not
allowed to break the user-space compatibility.  That is, this is the
very last chance to do it.

If your patch is supposed to do it with these consideration, I'm
willing to take.  But, I guess it's not, because you chose the string
like "Line 6 POD".  This is usually not ideal as a driver name; think
of $DRIVER.conf file that is used for alsa-lib configuration.

So, from that POV, "line6usb" is no bad name string.  If we want
differentiate per driver, a name like "Line6-Pod" or just "Pod" would
be more appropriate.


Takashi

> 
> Signed-off-by: Chris Rorvick <chris@rorvick.com>
> ---
>  sound/usb/line6/driver.c   | 3 ++-
>  sound/usb/line6/driver.h   | 3 +--
>  sound/usb/line6/pod.c      | 2 +-
>  sound/usb/line6/podhd.c    | 2 +-
>  sound/usb/line6/toneport.c | 2 +-
>  sound/usb/line6/variax.c   | 2 +-
>  6 files changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/sound/usb/line6/driver.c b/sound/usb/line6/driver.c
> index e2a2603..626b0c3 100644
> --- a/sound/usb/line6/driver.c
> +++ b/sound/usb/line6/driver.c
> @@ -480,6 +480,7 @@ static int line6_init_cap_control(struct usb_line6 *line6)
>  */
>  int line6_probe(struct usb_interface *interface,
>  		const struct usb_device_id *id,
> +		const char *driver_name,
>  		const struct line6_properties *properties,
>  		int (*private_init)(struct usb_line6 *, const struct usb_device_id *id),
>  		size_t data_size)
> @@ -511,7 +512,7 @@ int line6_probe(struct usb_interface *interface,
>  	line6->ifcdev = &interface->dev;
>  
>  	strcpy(card->id, properties->id);
> -	strcpy(card->driver, DRIVER_NAME);
> +	strcpy(card->driver, driver_name);
>  	strcpy(card->shortname, properties->name);
>  	sprintf(card->longname, "Line 6 %s at USB %s", properties->name,
>  		dev_name(line6->ifcdev));
> diff --git a/sound/usb/line6/driver.h b/sound/usb/line6/driver.h
> index 2276b78..92a662a 100644
> --- a/sound/usb/line6/driver.h
> +++ b/sound/usb/line6/driver.h
> @@ -18,8 +18,6 @@
>  
>  #include "midi.h"
>  
> -#define DRIVER_NAME "line6usb"
> -
>  #define USB_INTERVALS_PER_SECOND 1000
>  
>  /* Fallback USB interval and max packet size values */
> @@ -168,6 +166,7 @@ extern int line6_write_data(struct usb_line6 *line6, int address, void *data,
>  
>  int line6_probe(struct usb_interface *interface,
>  		const struct usb_device_id *id,
> +		const char *driver_name,
>  		const struct line6_properties *properties,
>  		int (*private_init)(struct usb_line6 *, const struct usb_device_id *id),
>  		size_t data_size);
> diff --git a/sound/usb/line6/pod.c b/sound/usb/line6/pod.c
> index 61aadd7..4c3d8cb 100644
> --- a/sound/usb/line6/pod.c
> +++ b/sound/usb/line6/pod.c
> @@ -574,7 +574,7 @@ static const struct line6_properties pod_properties_table[] = {
>  static int pod_probe(struct usb_interface *interface,
>  		     const struct usb_device_id *id)
>  {
> -	return line6_probe(interface, id,
> +	return line6_probe(interface, id, "Line 6 POD",
>  			   &pod_properties_table[id->driver_info],
>  			   pod_init, sizeof(struct usb_line6_pod));
>  }
> diff --git a/sound/usb/line6/podhd.c b/sound/usb/line6/podhd.c
> index 9c3c744..f0a761b 100644
> --- a/sound/usb/line6/podhd.c
> +++ b/sound/usb/line6/podhd.c
> @@ -169,7 +169,7 @@ static const struct line6_properties podhd_properties_table[] = {
>  static int podhd_probe(struct usb_interface *interface,
>  		       const struct usb_device_id *id)
>  {
> -	return line6_probe(interface, id,
> +	return line6_probe(interface, id, "Line 6 PODHD",
>  			   &podhd_properties_table[id->driver_info],
>  			   podhd_init, sizeof(struct usb_line6));
>  }
> diff --git a/sound/usb/line6/toneport.c b/sound/usb/line6/toneport.c
> index b2c0b2c..53c0fbd 100644
> --- a/sound/usb/line6/toneport.c
> +++ b/sound/usb/line6/toneport.c
> @@ -557,7 +557,7 @@ static const struct line6_properties toneport_properties_table[] = {
>  static int toneport_probe(struct usb_interface *interface,
>  			  const struct usb_device_id *id)
>  {
> -	return line6_probe(interface, id,
> +	return line6_probe(interface, id, "Line 6 TonePort",
>  			   &toneport_properties_table[id->driver_info],
>  			   toneport_init, sizeof(struct usb_line6_toneport));
>  }
> diff --git a/sound/usb/line6/variax.c b/sound/usb/line6/variax.c
> index b1c1de6..6f35c96 100644
> --- a/sound/usb/line6/variax.c
> +++ b/sound/usb/line6/variax.c
> @@ -283,7 +283,7 @@ static const struct line6_properties variax_properties_table[] = {
>  static int variax_probe(struct usb_interface *interface,
>  			const struct usb_device_id *id)
>  {
> -	return line6_probe(interface, id,
> +	return line6_probe(interface, id, "Line 6 Variax",
>  			   &variax_properties_table[id->driver_info],
>  			   variax_init, sizeof(struct usb_line6_variax));
>  }
> -- 
> 2.1.0
> 

  reply	other threads:[~2015-02-06 15:04 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-02-06 14:51 [PATCH 0/4] More Line 6 cleanup Chris Rorvick
2015-02-06 14:51 ` [PATCH 1/4] ALSA: line6: Add toneport_has_source_select() Chris Rorvick
2015-02-06 14:56   ` Takashi Iwai
2015-02-06 15:07     ` Chris Rorvick
2015-02-06 14:51 ` [PATCH 2/4] ALSA: line6: Pass toneport pointer to toneport_has_led() Chris Rorvick
2015-02-06 14:56   ` Takashi Iwai
2015-02-06 14:51 ` [PATCH 3/4] ALSA: line6: Pass driver name to line6_probe() Chris Rorvick
2015-02-06 15:04   ` Takashi Iwai [this message]
2015-02-06 17:23     ` Takashi Iwai
2015-02-07 16:18       ` Chris Rorvick
2015-02-06 14:51 ` [PATCH 4/4] ALSA: line6: Remove `usb_' prefix from structs Chris Rorvick
2015-02-06 15:11   ` Takashi Iwai

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=s5hfvajw06t.wl-tiwai@suse.de \
    --to=tiwai@suse.de \
    --cc=alsa-devel@alsa-project.org \
    --cc=chris@rorvick.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=stefanha@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®