Re: [PATCH] ALSA: snd-usb-line6 depends on CONFIG_SND_HWDEP
On Sep 21 2016 22:30, Takashi Iwai wrote: > On Wed, 21 Sep 2016 15:25:43 +0200, > Takashi Sakamoto wrote: >> >> On Sep 21 2016 21:37, Takashi Iwai wrote: >>> On Wed, 21 Sep 2016 03:56:05 +0200, >>> Takashi Sakamoto wrote: On Sep 21 2016 07:14, Valdis Kletnieks wrote: > ALSA - snd-usb-line6 depends on CONFIG_SND_HWDEP > > ERROR: "snd_hwdep_new" [sound/usb/line6/snd-usb-line6.ko] undefined! > scripts/Makefile.modpost:91: recipe for target '__modpost' failed > make[1]: *** [__modpost] Error 1 > > Signed-off-by: Valdis KletnieksReviewed-by: Takashi Sakamoto As a nitpicking, I think it better to change the subject line with a prefix of 'ALSA: line6:' or 'ALSA: usb-line6:', following to our fashion. >>> >>> Yes, I modified the subject line and added fixes tag. >> >> Thanks. >> >> As another issue, I can see sparse (v0.5.0) errors: >> >> sound/usb/line6/driver.c:274:43: error: cannot size expression >> sound/usb/line6/driver.c:275:16: error: cannot size expression >> sound/usb/line6/driver.c:276:16: error: cannot size expression >> sound/usb/line6/driver.c:277:16: error: cannot size expression >> >> And this fix can suppresses them: >> >> $ git diff sound/usb/line6 >> diff --git a/sound/usb/line6/driver.c b/sound/usb/line6/driver.c >> index f9224ec..ad7efc0 100644 >> --- a/sound/usb/line6/driver.c >> +++ b/sound/usb/line6/driver.c >> @@ -30,7 +30,7 @@ >> /* >> This is Line 6's MIDI manufacturer ID. >> */ >> -const unsigned char line6_midi_id[] = { >> +const unsigned char line6_midi_id[3] = { >> 0x00, 0x01, 0x0c >> }; >> EXPORT_SYMBOL_GPL(line6_midi_id); >> >> Besides, there's a difference of the declaration between header and code: >> $ cat sound/usb/line6/driver.h >> ... >> extern const unsigned char line6_midi_id[3]; >> >> This is quite minor. So I can't judge to fix it or not... What do you >> think about it? > > The only problem I can see is the inconsistency between the > declaration and the definition. I have the same opinion. Sparse is not versatile. > So yes, we should align to [3], if we keep the declaration as is. > (And actually it's sometimes safer to have the explicit array size.) OK. I'll post the fix later. Thanks Takashi Sakamoto
Re: [PATCH] ALSA: snd-usb-line6 depends on CONFIG_SND_HWDEP
On Sep 21 2016 22:30, Takashi Iwai wrote: > On Wed, 21 Sep 2016 15:25:43 +0200, > Takashi Sakamoto wrote: >> >> On Sep 21 2016 21:37, Takashi Iwai wrote: >>> On Wed, 21 Sep 2016 03:56:05 +0200, >>> Takashi Sakamoto wrote: On Sep 21 2016 07:14, Valdis Kletnieks wrote: > ALSA - snd-usb-line6 depends on CONFIG_SND_HWDEP > > ERROR: "snd_hwdep_new" [sound/usb/line6/snd-usb-line6.ko] undefined! > scripts/Makefile.modpost:91: recipe for target '__modpost' failed > make[1]: *** [__modpost] Error 1 > > Signed-off-by: Valdis Kletnieks Reviewed-by: Takashi Sakamoto As a nitpicking, I think it better to change the subject line with a prefix of 'ALSA: line6:' or 'ALSA: usb-line6:', following to our fashion. >>> >>> Yes, I modified the subject line and added fixes tag. >> >> Thanks. >> >> As another issue, I can see sparse (v0.5.0) errors: >> >> sound/usb/line6/driver.c:274:43: error: cannot size expression >> sound/usb/line6/driver.c:275:16: error: cannot size expression >> sound/usb/line6/driver.c:276:16: error: cannot size expression >> sound/usb/line6/driver.c:277:16: error: cannot size expression >> >> And this fix can suppresses them: >> >> $ git diff sound/usb/line6 >> diff --git a/sound/usb/line6/driver.c b/sound/usb/line6/driver.c >> index f9224ec..ad7efc0 100644 >> --- a/sound/usb/line6/driver.c >> +++ b/sound/usb/line6/driver.c >> @@ -30,7 +30,7 @@ >> /* >> This is Line 6's MIDI manufacturer ID. >> */ >> -const unsigned char line6_midi_id[] = { >> +const unsigned char line6_midi_id[3] = { >> 0x00, 0x01, 0x0c >> }; >> EXPORT_SYMBOL_GPL(line6_midi_id); >> >> Besides, there's a difference of the declaration between header and code: >> $ cat sound/usb/line6/driver.h >> ... >> extern const unsigned char line6_midi_id[3]; >> >> This is quite minor. So I can't judge to fix it or not... What do you >> think about it? > > The only problem I can see is the inconsistency between the > declaration and the definition. I have the same opinion. Sparse is not versatile. > So yes, we should align to [3], if we keep the declaration as is. > (And actually it's sometimes safer to have the explicit array size.) OK. I'll post the fix later. Thanks Takashi Sakamoto
Re: [PATCH] ALSA: snd-usb-line6 depends on CONFIG_SND_HWDEP
On Wed, 21 Sep 2016 15:25:43 +0200, Takashi Sakamoto wrote: > > On Sep 21 2016 21:37, Takashi Iwai wrote: > > On Wed, 21 Sep 2016 03:56:05 +0200, > > Takashi Sakamoto wrote: > >> > >> On Sep 21 2016 07:14, Valdis Kletnieks wrote: > >>> ALSA - snd-usb-line6 depends on CONFIG_SND_HWDEP > >>> > >>> ERROR: "snd_hwdep_new" [sound/usb/line6/snd-usb-line6.ko] undefined! > >>> scripts/Makefile.modpost:91: recipe for target '__modpost' failed > >>> make[1]: *** [__modpost] Error 1 > >>> > >>> Signed-off-by: Valdis Kletnieks> >> > >> Reviewed-by: Takashi Sakamoto > >> > >> As a nitpicking, I think it better to change the subject line with a > >> prefix of 'ALSA: line6:' or 'ALSA: usb-line6:', following to our > >> fashion. > > > > Yes, I modified the subject line and added fixes tag. > > Thanks. > > As another issue, I can see sparse (v0.5.0) errors: > > sound/usb/line6/driver.c:274:43: error: cannot size expression > sound/usb/line6/driver.c:275:16: error: cannot size expression > sound/usb/line6/driver.c:276:16: error: cannot size expression > sound/usb/line6/driver.c:277:16: error: cannot size expression > > And this fix can suppresses them: > > $ git diff sound/usb/line6 > diff --git a/sound/usb/line6/driver.c b/sound/usb/line6/driver.c > index f9224ec..ad7efc0 100644 > --- a/sound/usb/line6/driver.c > +++ b/sound/usb/line6/driver.c > @@ -30,7 +30,7 @@ > /* > This is Line 6's MIDI manufacturer ID. > */ > -const unsigned char line6_midi_id[] = { > +const unsigned char line6_midi_id[3] = { > 0x00, 0x01, 0x0c > }; > EXPORT_SYMBOL_GPL(line6_midi_id); > > Besides, there's a difference of the declaration between header and code: > $ cat sound/usb/line6/driver.h > ... > extern const unsigned char line6_midi_id[3]; > > This is quite minor. So I can't judge to fix it or not... What do you > think about it? The only problem I can see is the inconsistency between the declaration and the definition. So yes, we should align to [3], if we keep the declaration as is. (And actually it's sometimes safer to have the explicit array size.) thanks, Takashi
Re: [PATCH] ALSA: snd-usb-line6 depends on CONFIG_SND_HWDEP
On Wed, 21 Sep 2016 15:25:43 +0200, Takashi Sakamoto wrote: > > On Sep 21 2016 21:37, Takashi Iwai wrote: > > On Wed, 21 Sep 2016 03:56:05 +0200, > > Takashi Sakamoto wrote: > >> > >> On Sep 21 2016 07:14, Valdis Kletnieks wrote: > >>> ALSA - snd-usb-line6 depends on CONFIG_SND_HWDEP > >>> > >>> ERROR: "snd_hwdep_new" [sound/usb/line6/snd-usb-line6.ko] undefined! > >>> scripts/Makefile.modpost:91: recipe for target '__modpost' failed > >>> make[1]: *** [__modpost] Error 1 > >>> > >>> Signed-off-by: Valdis Kletnieks > >> > >> Reviewed-by: Takashi Sakamoto > >> > >> As a nitpicking, I think it better to change the subject line with a > >> prefix of 'ALSA: line6:' or 'ALSA: usb-line6:', following to our > >> fashion. > > > > Yes, I modified the subject line and added fixes tag. > > Thanks. > > As another issue, I can see sparse (v0.5.0) errors: > > sound/usb/line6/driver.c:274:43: error: cannot size expression > sound/usb/line6/driver.c:275:16: error: cannot size expression > sound/usb/line6/driver.c:276:16: error: cannot size expression > sound/usb/line6/driver.c:277:16: error: cannot size expression > > And this fix can suppresses them: > > $ git diff sound/usb/line6 > diff --git a/sound/usb/line6/driver.c b/sound/usb/line6/driver.c > index f9224ec..ad7efc0 100644 > --- a/sound/usb/line6/driver.c > +++ b/sound/usb/line6/driver.c > @@ -30,7 +30,7 @@ > /* > This is Line 6's MIDI manufacturer ID. > */ > -const unsigned char line6_midi_id[] = { > +const unsigned char line6_midi_id[3] = { > 0x00, 0x01, 0x0c > }; > EXPORT_SYMBOL_GPL(line6_midi_id); > > Besides, there's a difference of the declaration between header and code: > $ cat sound/usb/line6/driver.h > ... > extern const unsigned char line6_midi_id[3]; > > This is quite minor. So I can't judge to fix it or not... What do you > think about it? The only problem I can see is the inconsistency between the declaration and the definition. So yes, we should align to [3], if we keep the declaration as is. (And actually it's sometimes safer to have the explicit array size.) thanks, Takashi
Re: [PATCH] ALSA: snd-usb-line6 depends on CONFIG_SND_HWDEP
On Sep 21 2016 21:37, Takashi Iwai wrote: > On Wed, 21 Sep 2016 03:56:05 +0200, > Takashi Sakamoto wrote: >> >> On Sep 21 2016 07:14, Valdis Kletnieks wrote: >>> ALSA - snd-usb-line6 depends on CONFIG_SND_HWDEP >>> >>> ERROR: "snd_hwdep_new" [sound/usb/line6/snd-usb-line6.ko] undefined! >>> scripts/Makefile.modpost:91: recipe for target '__modpost' failed >>> make[1]: *** [__modpost] Error 1 >>> >>> Signed-off-by: Valdis Kletnieks>> >> Reviewed-by: Takashi Sakamoto >> >> As a nitpicking, I think it better to change the subject line with a >> prefix of 'ALSA: line6:' or 'ALSA: usb-line6:', following to our >> fashion. > > Yes, I modified the subject line and added fixes tag. Thanks. As another issue, I can see sparse (v0.5.0) errors: sound/usb/line6/driver.c:274:43: error: cannot size expression sound/usb/line6/driver.c:275:16: error: cannot size expression sound/usb/line6/driver.c:276:16: error: cannot size expression sound/usb/line6/driver.c:277:16: error: cannot size expression And this fix can suppresses them: $ git diff sound/usb/line6 diff --git a/sound/usb/line6/driver.c b/sound/usb/line6/driver.c index f9224ec..ad7efc0 100644 --- a/sound/usb/line6/driver.c +++ b/sound/usb/line6/driver.c @@ -30,7 +30,7 @@ /* This is Line 6's MIDI manufacturer ID. */ -const unsigned char line6_midi_id[] = { +const unsigned char line6_midi_id[3] = { 0x00, 0x01, 0x0c }; EXPORT_SYMBOL_GPL(line6_midi_id); Besides, there's a difference of the declaration between header and code: $ cat sound/usb/line6/driver.h ... extern const unsigned char line6_midi_id[3]; This is quite minor. So I can't judge to fix it or not... What do you think about it? >>> diff --git a/sound/usb/line6/Kconfig b/sound/usb/line6/Kconfig >>> index 8ffcf48097d9..39b400392d71 100644 >>> --- a/sound/usb/line6/Kconfig >>> +++ b/sound/usb/line6/Kconfig >>> @@ -2,6 +2,7 @@ config SND_USB_LINE6 >>> tristate >>> select SND_RAWMIDI >>> select SND_PCM >>> + select SND_HWDEP >>> >>> config SND_USB_POD >>> tristate "Line 6 POD USB support" Regards Takashi Sakamoto
Re: [PATCH] ALSA: snd-usb-line6 depends on CONFIG_SND_HWDEP
On Sep 21 2016 21:37, Takashi Iwai wrote: > On Wed, 21 Sep 2016 03:56:05 +0200, > Takashi Sakamoto wrote: >> >> On Sep 21 2016 07:14, Valdis Kletnieks wrote: >>> ALSA - snd-usb-line6 depends on CONFIG_SND_HWDEP >>> >>> ERROR: "snd_hwdep_new" [sound/usb/line6/snd-usb-line6.ko] undefined! >>> scripts/Makefile.modpost:91: recipe for target '__modpost' failed >>> make[1]: *** [__modpost] Error 1 >>> >>> Signed-off-by: Valdis Kletnieks >> >> Reviewed-by: Takashi Sakamoto >> >> As a nitpicking, I think it better to change the subject line with a >> prefix of 'ALSA: line6:' or 'ALSA: usb-line6:', following to our >> fashion. > > Yes, I modified the subject line and added fixes tag. Thanks. As another issue, I can see sparse (v0.5.0) errors: sound/usb/line6/driver.c:274:43: error: cannot size expression sound/usb/line6/driver.c:275:16: error: cannot size expression sound/usb/line6/driver.c:276:16: error: cannot size expression sound/usb/line6/driver.c:277:16: error: cannot size expression And this fix can suppresses them: $ git diff sound/usb/line6 diff --git a/sound/usb/line6/driver.c b/sound/usb/line6/driver.c index f9224ec..ad7efc0 100644 --- a/sound/usb/line6/driver.c +++ b/sound/usb/line6/driver.c @@ -30,7 +30,7 @@ /* This is Line 6's MIDI manufacturer ID. */ -const unsigned char line6_midi_id[] = { +const unsigned char line6_midi_id[3] = { 0x00, 0x01, 0x0c }; EXPORT_SYMBOL_GPL(line6_midi_id); Besides, there's a difference of the declaration between header and code: $ cat sound/usb/line6/driver.h ... extern const unsigned char line6_midi_id[3]; This is quite minor. So I can't judge to fix it or not... What do you think about it? >>> diff --git a/sound/usb/line6/Kconfig b/sound/usb/line6/Kconfig >>> index 8ffcf48097d9..39b400392d71 100644 >>> --- a/sound/usb/line6/Kconfig >>> +++ b/sound/usb/line6/Kconfig >>> @@ -2,6 +2,7 @@ config SND_USB_LINE6 >>> tristate >>> select SND_RAWMIDI >>> select SND_PCM >>> + select SND_HWDEP >>> >>> config SND_USB_POD >>> tristate "Line 6 POD USB support" Regards Takashi Sakamoto
Re: [PATCH] ALSA: snd-usb-line6 depends on CONFIG_SND_HWDEP
On Wed, 21 Sep 2016 03:56:05 +0200, Takashi Sakamoto wrote: > > On Sep 21 2016 07:14, Valdis Kletnieks wrote: > > ALSA - snd-usb-line6 depends on CONFIG_SND_HWDEP > > > > ERROR: "snd_hwdep_new" [sound/usb/line6/snd-usb-line6.ko] undefined! > > scripts/Makefile.modpost:91: recipe for target '__modpost' failed > > make[1]: *** [__modpost] Error 1 > > > > Signed-off-by: Valdis Kletnieks> > Reviewed-by: Takashi Sakamoto > > As a nitpicking, I think it better to change the subject line with a > prefix of 'ALSA: line6:' or 'ALSA: usb-line6:', following to our > fashion. Yes, I modified the subject line and added fixes tag. thanks, Takashi > > diff --git a/sound/usb/line6/Kconfig b/sound/usb/line6/Kconfig > > index 8ffcf48097d9..39b400392d71 100644 > > --- a/sound/usb/line6/Kconfig > > +++ b/sound/usb/line6/Kconfig > > @@ -2,6 +2,7 @@ config SND_USB_LINE6 > > tristate > > select SND_RAWMIDI > > select SND_PCM > > + select SND_HWDEP > > > > config SND_USB_POD > > tristate "Line 6 POD USB support" > > > Thanks > > Takashi Sakamoto >
Re: [PATCH] ALSA: snd-usb-line6 depends on CONFIG_SND_HWDEP
On Wed, 21 Sep 2016 03:56:05 +0200, Takashi Sakamoto wrote: > > On Sep 21 2016 07:14, Valdis Kletnieks wrote: > > ALSA - snd-usb-line6 depends on CONFIG_SND_HWDEP > > > > ERROR: "snd_hwdep_new" [sound/usb/line6/snd-usb-line6.ko] undefined! > > scripts/Makefile.modpost:91: recipe for target '__modpost' failed > > make[1]: *** [__modpost] Error 1 > > > > Signed-off-by: Valdis Kletnieks > > Reviewed-by: Takashi Sakamoto > > As a nitpicking, I think it better to change the subject line with a > prefix of 'ALSA: line6:' or 'ALSA: usb-line6:', following to our > fashion. Yes, I modified the subject line and added fixes tag. thanks, Takashi > > diff --git a/sound/usb/line6/Kconfig b/sound/usb/line6/Kconfig > > index 8ffcf48097d9..39b400392d71 100644 > > --- a/sound/usb/line6/Kconfig > > +++ b/sound/usb/line6/Kconfig > > @@ -2,6 +2,7 @@ config SND_USB_LINE6 > > tristate > > select SND_RAWMIDI > > select SND_PCM > > + select SND_HWDEP > > > > config SND_USB_POD > > tristate "Line 6 POD USB support" > > > Thanks > > Takashi Sakamoto >
Re: [PATCH] ALSA: snd-usb-line6 depends on CONFIG_SND_HWDEP
On Sep 21 2016 07:14, Valdis Kletnieks wrote: ALSA - snd-usb-line6 depends on CONFIG_SND_HWDEP ERROR: "snd_hwdep_new" [sound/usb/line6/snd-usb-line6.ko] undefined! scripts/Makefile.modpost:91: recipe for target '__modpost' failed make[1]: *** [__modpost] Error 1 Signed-off-by: Valdis KletnieksReviewed-by: Takashi Sakamoto As a nitpicking, I think it better to change the subject line with a prefix of 'ALSA: line6:' or 'ALSA: usb-line6:', following to our fashion. diff --git a/sound/usb/line6/Kconfig b/sound/usb/line6/Kconfig index 8ffcf48097d9..39b400392d71 100644 --- a/sound/usb/line6/Kconfig +++ b/sound/usb/line6/Kconfig @@ -2,6 +2,7 @@ config SND_USB_LINE6 tristate select SND_RAWMIDI select SND_PCM + select SND_HWDEP config SND_USB_POD tristate "Line 6 POD USB support" Thanks Takashi Sakamoto
Re: [PATCH] ALSA: snd-usb-line6 depends on CONFIG_SND_HWDEP
On Sep 21 2016 07:14, Valdis Kletnieks wrote: ALSA - snd-usb-line6 depends on CONFIG_SND_HWDEP ERROR: "snd_hwdep_new" [sound/usb/line6/snd-usb-line6.ko] undefined! scripts/Makefile.modpost:91: recipe for target '__modpost' failed make[1]: *** [__modpost] Error 1 Signed-off-by: Valdis Kletnieks Reviewed-by: Takashi Sakamoto As a nitpicking, I think it better to change the subject line with a prefix of 'ALSA: line6:' or 'ALSA: usb-line6:', following to our fashion. diff --git a/sound/usb/line6/Kconfig b/sound/usb/line6/Kconfig index 8ffcf48097d9..39b400392d71 100644 --- a/sound/usb/line6/Kconfig +++ b/sound/usb/line6/Kconfig @@ -2,6 +2,7 @@ config SND_USB_LINE6 tristate select SND_RAWMIDI select SND_PCM + select SND_HWDEP config SND_USB_POD tristate "Line 6 POD USB support" Thanks Takashi Sakamoto