On Fri, May 30, 2008 at 10:15:43AM +0200, Takashi Iwai wrote: > The implementation looks good, simple enough. Thanks. > Mark Brown wrote: > > +config SND_JACK > > + tristate > > + depends on SND > > + depends on INPUT > The code is small, and I don't see a big merit to make it a module. I agree - I had meant to flag this up, actually. I hadn't been sure if some of the other objects were being built conditionally for size or not since they seemed relatively small too. When I respin I'll remove this option. > > + snprintf(jack->name, sizeof(jack->name), "%s %s", > > + card->longname, jack->id); > The longname field could be sometimes really too long and verbose. > I guess shortname would match better. The general style for input device names tends towards the long and verbose. For example, on my work laptop I have devices with names like: Macintosh mouse button emulation AT Translated Set 2 Keyboard Lid Switch Power Button (CM) Sleep Button (CM) AlpsPS/2 ALPS GlidePoint The ALSA long name seems more idiomatic for this context. > > +int snd_jack_new(struct snd_card *card, const char *id, int type, > > + struct snd_jack **jjack) > (snip) > > + jack->input_dev->phys = "ALSA"; > > + jack->input_dev->dev.parent = card->dev; > The card->dev pointer might not be initialized always at this stage. > You should check rather at register. Will do. > Also, someone may want to pass a different device pointer for this. > Passing struct device * to snd_jack_new would be an alternative. Good idea, that might be useful for ASoC v2 sound cards. I'll add something which allows the caller to optionally specify a parent, defaulting to the sound card if none is given. The only issue might be for user space figuring out which sound card corresponds to which jack but if jacks might be implemented outside of ALSA anyway (which is possible) that could happen anyway. -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
