All of lore.kernel.org
 help / color / mirror / Atom feed
* SB16 build error.
@ 2011-06-30  9:17 ` Ralf Baechle
  0 siblings, 0 replies; 30+ messages in thread
From: Ralf Baechle @ 2011-06-30  9:17 UTC (permalink / raw)
  To: Jaroslav Kysela, Takashi Iwai
  Cc: alsa-devel, linux-kernel, linux-mips, florian, Florian Fainelli,
	linux-arch, Richard Henderson, Ivan Kokshaysky, Matt Turner,
	Benjamin Herrenschmidt, Paul Mackerras, David S. Miller,
	sparclinux

Found on a MIPS build but certain other architectures will have the same
issue:

  CC      sound/isa/sb/sb16_csp.o
sound/isa/sb/sb16_csp.c: In function ‘snd_sb_csp_ioctl’:
sound/isa/sb/sb16_csp.c:228: error: case label does not reduce to an integer constant
make[1]: *** [sound/isa/sb/sb16_csp.o] Error 1
make: *** [sound/isa/sb/sb16_csp.o] Error 2

This error message is caused by the _IOC_TYPECHECK() error check triggering
due to excessive ioctl size on Alpha, PowerPC, MIPS and SPARC which define
_IOC_SIZEBITS as 13.  On all other architectures define it as 14 so struct
snd_sb_csp_microcode with it's little over 12kB will just about fit into
the 16kB limit.

  Ralf

^ permalink raw reply	[flat|nested] 30+ messages in thread

* SB16 build error.
@ 2011-06-30  9:17 ` Ralf Baechle
  0 siblings, 0 replies; 30+ messages in thread
From: Ralf Baechle @ 2011-06-30  9:17 UTC (permalink / raw)
  To: Jaroslav Kysela, Takashi Iwai
  Cc: linux-arch, linux-mips, alsa-devel, florian,
	Benjamin Herrenschmidt, linux-kernel, David S. Miller,
	Ivan Kokshaysky, sparclinux, Paul Mackerras, Matt Turner,
	Florian Fainelli, Richard Henderson

Found on a MIPS build but certain other architectures will have the same
issue:

  CC      sound/isa/sb/sb16_csp.o
sound/isa/sb/sb16_csp.c: In function ‘snd_sb_csp_ioctl’:
sound/isa/sb/sb16_csp.c:228: error: case label does not reduce to an integer constant
make[1]: *** [sound/isa/sb/sb16_csp.o] Error 1
make: *** [sound/isa/sb/sb16_csp.o] Error 2

This error message is caused by the _IOC_TYPECHECK() error check triggering
due to excessive ioctl size on Alpha, PowerPC, MIPS and SPARC which define
_IOC_SIZEBITS as 13.  On all other architectures define it as 14 so struct
snd_sb_csp_microcode with it's little over 12kB will just about fit into
the 16kB limit.

  Ralf
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

^ permalink raw reply	[flat|nested] 30+ messages in thread

* SB16 build error.
@ 2011-06-30  9:17 ` Ralf Baechle
  0 siblings, 0 replies; 30+ messages in thread
From: Ralf Baechle @ 2011-06-30  9:17 UTC (permalink / raw)
  To: Jaroslav Kysela, Takashi Iwai
  Cc: linux-arch, linux-mips, alsa-devel, florian,
	Benjamin Herrenschmidt, linux-kernel, David S. Miller,
	Ivan Kokshaysky, sparclinux, Paul Mackerras, Matt Turner,
	Florian Fainelli, Richard Henderson

Found on a MIPS build but certain other architectures will have the same
issue:

  CC      sound/isa/sb/sb16_csp.o
sound/isa/sb/sb16_csp.c: In function ‘snd_sb_csp_ioctl’:
sound/isa/sb/sb16_csp.c:228: error: case label does not reduce to an integer constant
make[1]: *** [sound/isa/sb/sb16_csp.o] Error 1
make: *** [sound/isa/sb/sb16_csp.o] Error 2

This error message is caused by the _IOC_TYPECHECK() error check triggering
due to excessive ioctl size on Alpha, PowerPC, MIPS and SPARC which define
_IOC_SIZEBITS as 13.  On all other architectures define it as 14 so struct
snd_sb_csp_microcode with it's little over 12kB will just about fit into
the 16kB limit.

  Ralf

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: SB16 build error.
  2011-06-30  9:17 ` Ralf Baechle
@ 2011-06-30 10:15   ` Takashi Iwai
  -1 siblings, 0 replies; 30+ messages in thread
From: Takashi Iwai @ 2011-06-30 10:15 UTC (permalink / raw)
  To: Ralf Baechle
  Cc: Jaroslav Kysela, alsa-devel, linux-kernel, linux-mips, florian,
	Florian Fainelli, linux-arch, Richard Henderson, Ivan Kokshaysky,
	Matt Turner, Benjamin Herrenschmidt, Paul Mackerras,
	David S. Miller, sparclinux

At Thu, 30 Jun 2011 10:17:54 +0100,
Ralf Baechle wrote:
> 
> Found on a MIPS build but certain other architectures will have the same
> issue:
> 
>   CC      sound/isa/sb/sb16_csp.o
> sound/isa/sb/sb16_csp.c: In function ‘snd_sb_csp_ioctl’:
> sound/isa/sb/sb16_csp.c:228: error: case label does not reduce to an integer constant
> make[1]: *** [sound/isa/sb/sb16_csp.o] Error 1
> make: *** [sound/isa/sb/sb16_csp.o] Error 2
> 
> This error message is caused by the _IOC_TYPECHECK() error check triggering
> due to excessive ioctl size on Alpha, PowerPC, MIPS and SPARC which define
> _IOC_SIZEBITS as 13.  On all other architectures define it as 14 so struct
> snd_sb_csp_microcode with it's little over 12kB will just about fit into
> the 16kB limit.

What about the patch below?
This is an old ISA driver, so the impact must be very low.


thanks,

Takashi

---
diff --git a/include/sound/sb16_csp.h b/include/sound/sb16_csp.h
index 736eac7..2806586 100644
--- a/include/sound/sb16_csp.h
+++ b/include/sound/sb16_csp.h
@@ -60,7 +60,12 @@
 #define SNDRV_SB_CSP_QSOUND_MAX_RIGHT	0x20
 
 /* maximum microcode RIFF file size */
+#if _IOC_SIZEBITS < 14
+/* reduced the size to fit with ioctl size limit */
+#define SNDRV_SB_CSP_MAX_MICROCODE_FILE_SIZE	0x1f00
+#else
 #define SNDRV_SB_CSP_MAX_MICROCODE_FILE_SIZE	0x3000
+#endif
 
 /* microcode header */
 struct snd_sb_csp_mc_header {

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* Re: SB16 build error.
@ 2011-06-30 10:15   ` Takashi Iwai
  0 siblings, 0 replies; 30+ messages in thread
From: Takashi Iwai @ 2011-06-30 10:15 UTC (permalink / raw)
  To: Ralf Baechle
  Cc: Jaroslav Kysela, alsa-devel, linux-kernel, linux-mips, florian,
	Florian Fainelli, linux-arch, Richard Henderson, Ivan Kokshaysky,
	Matt Turner, Benjamin Herrenschmidt, Paul Mackerras,
	David S. Miller, sparclinux

At Thu, 30 Jun 2011 10:17:54 +0100,
Ralf Baechle wrote:
> 
> Found on a MIPS build but certain other architectures will have the same
> issue:
> 
>   CC      sound/isa/sb/sb16_csp.o
> sound/isa/sb/sb16_csp.c: In function ‘snd_sb_csp_ioctl’:
> sound/isa/sb/sb16_csp.c:228: error: case label does not reduce to an integer constant
> make[1]: *** [sound/isa/sb/sb16_csp.o] Error 1
> make: *** [sound/isa/sb/sb16_csp.o] Error 2
> 
> This error message is caused by the _IOC_TYPECHECK() error check triggering
> due to excessive ioctl size on Alpha, PowerPC, MIPS and SPARC which define
> _IOC_SIZEBITS as 13.  On all other architectures define it as 14 so struct
> snd_sb_csp_microcode with it's little over 12kB will just about fit into
> the 16kB limit.

What about the patch below?
This is an old ISA driver, so the impact must be very low.


thanks,

Takashi

---
diff --git a/include/sound/sb16_csp.h b/include/sound/sb16_csp.h
index 736eac7..2806586 100644
--- a/include/sound/sb16_csp.h
+++ b/include/sound/sb16_csp.h
@@ -60,7 +60,12 @@
 #define SNDRV_SB_CSP_QSOUND_MAX_RIGHT	0x20
 
 /* maximum microcode RIFF file size */
+#if _IOC_SIZEBITS < 14
+/* reduced the size to fit with ioctl size limit */
+#define SNDRV_SB_CSP_MAX_MICROCODE_FILE_SIZE	0x1f00
+#else
 #define SNDRV_SB_CSP_MAX_MICROCODE_FILE_SIZE	0x3000
+#endif
 
 /* microcode header */
 struct snd_sb_csp_mc_header {

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* Re: SB16 build error.
  2011-06-30 10:15   ` Takashi Iwai
  (?)
@ 2011-06-30 10:52     ` Ralf Baechle
  -1 siblings, 0 replies; 30+ messages in thread
From: Ralf Baechle @ 2011-06-30 10:52 UTC (permalink / raw)
  To: Takashi Iwai
  Cc: Jaroslav Kysela, alsa-devel, linux-kernel, linux-mips, florian,
	Florian Fainelli, linux-arch, Richard Henderson, Ivan Kokshaysky,
	Matt Turner, Benjamin Herrenschmidt, Paul Mackerras,
	David S. Miller, sparclinux

On Thu, Jun 30, 2011 at 12:15:56PM +0200, Takashi Iwai wrote:

> At Thu, 30 Jun 2011 10:17:54 +0100,
> Ralf Baechle wrote:
> > 
> > Found on a MIPS build but certain other architectures will have the same
> > issue:
> > 
> >   CC      sound/isa/sb/sb16_csp.o
> > sound/isa/sb/sb16_csp.c: In function ‘snd_sb_csp_ioctl’:
> > sound/isa/sb/sb16_csp.c:228: error: case label does not reduce to an integer constant
> > make[1]: *** [sound/isa/sb/sb16_csp.o] Error 1
> > make: *** [sound/isa/sb/sb16_csp.o] Error 2
> > 
> > This error message is caused by the _IOC_TYPECHECK() error check triggering
> > due to excessive ioctl size on Alpha, PowerPC, MIPS and SPARC which define
> > _IOC_SIZEBITS as 13.  On all other architectures define it as 14 so struct
> > snd_sb_csp_microcode with it's little over 12kB will just about fit into
> > the 16kB limit.
> 
> What about the patch below?

I have no idea how big the soundblaster microcode being loaded actually is,
that is if the reduced size of 0x1f00 will be sufficient.  Aside of that I
don't see a problem - I don't see how the old ioctl can possibly have been
used before so there isn't a compatibility problem.

Or you could entirely sidestep the problem and use request_firmware() but
I guess that's more effort than you want to invest.

> This is an old ISA driver, so the impact must be very low.

True.  I notice that sort of stuff in automated mass builds - not
necessarily the sort of kernels one would actually use.  Still build errors
are annoying :)

  Ralf

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: SB16 build error.
@ 2011-06-30 10:52     ` Ralf Baechle
  0 siblings, 0 replies; 30+ messages in thread
From: Ralf Baechle @ 2011-06-30 10:52 UTC (permalink / raw)
  To: Takashi Iwai
  Cc: linux-arch, linux-mips, alsa-devel, florian, David S. Miller,
	Benjamin Herrenschmidt, linux-kernel, Ivan Kokshaysky,
	sparclinux, Paul Mackerras, Matt Turner, Florian Fainelli,
	Richard Henderson

On Thu, Jun 30, 2011 at 12:15:56PM +0200, Takashi Iwai wrote:

> At Thu, 30 Jun 2011 10:17:54 +0100,
> Ralf Baechle wrote:
> > 
> > Found on a MIPS build but certain other architectures will have the same
> > issue:
> > 
> >   CC      sound/isa/sb/sb16_csp.o
> > sound/isa/sb/sb16_csp.c: In function ‘snd_sb_csp_ioctl’:
> > sound/isa/sb/sb16_csp.c:228: error: case label does not reduce to an integer constant
> > make[1]: *** [sound/isa/sb/sb16_csp.o] Error 1
> > make: *** [sound/isa/sb/sb16_csp.o] Error 2
> > 
> > This error message is caused by the _IOC_TYPECHECK() error check triggering
> > due to excessive ioctl size on Alpha, PowerPC, MIPS and SPARC which define
> > _IOC_SIZEBITS as 13.  On all other architectures define it as 14 so struct
> > snd_sb_csp_microcode with it's little over 12kB will just about fit into
> > the 16kB limit.
> 
> What about the patch below?

I have no idea how big the soundblaster microcode being loaded actually is,
that is if the reduced size of 0x1f00 will be sufficient.  Aside of that I
don't see a problem - I don't see how the old ioctl can possibly have been
used before so there isn't a compatibility problem.

Or you could entirely sidestep the problem and use request_firmware() but
I guess that's more effort than you want to invest.

> This is an old ISA driver, so the impact must be very low.

True.  I notice that sort of stuff in automated mass builds - not
necessarily the sort of kernels one would actually use.  Still build errors
are annoying :)

  Ralf
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: SB16 build error.
@ 2011-06-30 10:52     ` Ralf Baechle
  0 siblings, 0 replies; 30+ messages in thread
From: Ralf Baechle @ 2011-06-30 10:52 UTC (permalink / raw)
  To: Takashi Iwai
  Cc: linux-arch, linux-mips, alsa-devel, florian, David S. Miller,
	Benjamin Herrenschmidt, linux-kernel, Ivan Kokshaysky,
	sparclinux, Paul Mackerras, Matt Turner, Florian Fainelli,
	Richard Henderson

On Thu, Jun 30, 2011 at 12:15:56PM +0200, Takashi Iwai wrote:

> At Thu, 30 Jun 2011 10:17:54 +0100,
> Ralf Baechle wrote:
> > 
> > Found on a MIPS build but certain other architectures will have the same
> > issue:
> > 
> >   CC      sound/isa/sb/sb16_csp.o
> > sound/isa/sb/sb16_csp.c: In function ‘snd_sb_csp_ioctl’:
> > sound/isa/sb/sb16_csp.c:228: error: case label does not reduce to an integer constant
> > make[1]: *** [sound/isa/sb/sb16_csp.o] Error 1
> > make: *** [sound/isa/sb/sb16_csp.o] Error 2
> > 
> > This error message is caused by the _IOC_TYPECHECK() error check triggering
> > due to excessive ioctl size on Alpha, PowerPC, MIPS and SPARC which define
> > _IOC_SIZEBITS as 13.  On all other architectures define it as 14 so struct
> > snd_sb_csp_microcode with it's little over 12kB will just about fit into
> > the 16kB limit.
> 
> What about the patch below?

I have no idea how big the soundblaster microcode being loaded actually is,
that is if the reduced size of 0x1f00 will be sufficient.  Aside of that I
don't see a problem - I don't see how the old ioctl can possibly have been
used before so there isn't a compatibility problem.

Or you could entirely sidestep the problem and use request_firmware() but
I guess that's more effort than you want to invest.

> This is an old ISA driver, so the impact must be very low.

True.  I notice that sort of stuff in automated mass builds - not
necessarily the sort of kernels one would actually use.  Still build errors
are annoying :)

  Ralf

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: SB16 build error.
  2011-06-30 10:52     ` Ralf Baechle
@ 2011-06-30 11:05       ` Takashi Iwai
  -1 siblings, 0 replies; 30+ messages in thread
From: Takashi Iwai @ 2011-06-30 11:05 UTC (permalink / raw)
  To: Ralf Baechle
  Cc: Jaroslav Kysela, alsa-devel, linux-kernel, linux-mips, florian,
	Florian Fainelli, linux-arch, Richard Henderson, Ivan Kokshaysky,
	Matt Turner, Benjamin Herrenschmidt, Paul Mackerras,
	David S. Miller, sparclinux

At Thu, 30 Jun 2011 11:52:54 +0100,
Ralf Baechle wrote:
> 
> On Thu, Jun 30, 2011 at 12:15:56PM +0200, Takashi Iwai wrote:
> 
> > At Thu, 30 Jun 2011 10:17:54 +0100,
> > Ralf Baechle wrote:
> > > 
> > > Found on a MIPS build but certain other architectures will have the same
> > > issue:
> > > 
> > >   CC      sound/isa/sb/sb16_csp.o
> > > sound/isa/sb/sb16_csp.c: In function ‘snd_sb_csp_ioctl’:
> > > sound/isa/sb/sb16_csp.c:228: error: case label does not reduce to an integer constant
> > > make[1]: *** [sound/isa/sb/sb16_csp.o] Error 1
> > > make: *** [sound/isa/sb/sb16_csp.o] Error 2
> > > 
> > > This error message is caused by the _IOC_TYPECHECK() error check triggering
> > > due to excessive ioctl size on Alpha, PowerPC, MIPS and SPARC which define
> > > _IOC_SIZEBITS as 13.  On all other architectures define it as 14 so struct
> > > snd_sb_csp_microcode with it's little over 12kB will just about fit into
> > > the 16kB limit.
> > 
> > What about the patch below?
> 
> I have no idea how big the soundblaster microcode being loaded actually is,
> that is if the reduced size of 0x1f00 will be sufficient.

The files found in /lib/firmware/sb16 are all under 2kB, thus likely
sufficient.

> Aside of that I
> don't see a problem - I don't see how the old ioctl can possibly have been
> used before so there isn't a compatibility problem.
> 
> Or you could entirely sidestep the problem and use request_firmware() but
> I guess that's more effort than you want to invest.

Yeah, that's another option I thought of.  But it's too intrusive for
3.0-rc6, so I'd like waive it for 3.1.

> > This is an old ISA driver, so the impact must be very low.
> 
> True.  I notice that sort of stuff in automated mass builds - not
> necessarily the sort of kernels one would actually use.  Still build errors
> are annoying :)

Right, better to fix it up quickly.


thanks,

Takashi

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: SB16 build error.
@ 2011-06-30 11:05       ` Takashi Iwai
  0 siblings, 0 replies; 30+ messages in thread
From: Takashi Iwai @ 2011-06-30 11:05 UTC (permalink / raw)
  To: Ralf Baechle
  Cc: Jaroslav Kysela, alsa-devel, linux-kernel, linux-mips, florian,
	Florian Fainelli, linux-arch, Richard Henderson, Ivan Kokshaysky,
	Matt Turner, Benjamin Herrenschmidt, Paul Mackerras,
	David S. Miller, sparclinux

At Thu, 30 Jun 2011 11:52:54 +0100,
Ralf Baechle wrote:
> 
> On Thu, Jun 30, 2011 at 12:15:56PM +0200, Takashi Iwai wrote:
> 
> > At Thu, 30 Jun 2011 10:17:54 +0100,
> > Ralf Baechle wrote:
> > > 
> > > Found on a MIPS build but certain other architectures will have the same
> > > issue:
> > > 
> > >   CC      sound/isa/sb/sb16_csp.o
> > > sound/isa/sb/sb16_csp.c: In function ‘snd_sb_csp_ioctl’:
> > > sound/isa/sb/sb16_csp.c:228: error: case label does not reduce to an integer constant
> > > make[1]: *** [sound/isa/sb/sb16_csp.o] Error 1
> > > make: *** [sound/isa/sb/sb16_csp.o] Error 2
> > > 
> > > This error message is caused by the _IOC_TYPECHECK() error check triggering
> > > due to excessive ioctl size on Alpha, PowerPC, MIPS and SPARC which define
> > > _IOC_SIZEBITS as 13.  On all other architectures define it as 14 so struct
> > > snd_sb_csp_microcode with it's little over 12kB will just about fit into
> > > the 16kB limit.
> > 
> > What about the patch below?
> 
> I have no idea how big the soundblaster microcode being loaded actually is,
> that is if the reduced size of 0x1f00 will be sufficient.

The files found in /lib/firmware/sb16 are all under 2kB, thus likely
sufficient.

> Aside of that I
> don't see a problem - I don't see how the old ioctl can possibly have been
> used before so there isn't a compatibility problem.
> 
> Or you could entirely sidestep the problem and use request_firmware() but
> I guess that's more effort than you want to invest.

Yeah, that's another option I thought of.  But it's too intrusive for
3.0-rc6, so I'd like waive it for 3.1.

> > This is an old ISA driver, so the impact must be very low.
> 
> True.  I notice that sort of stuff in automated mass builds - not
> necessarily the sort of kernels one would actually use.  Still build errors
> are annoying :)

Right, better to fix it up quickly.


thanks,

Takashi

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: SB16 build error.
  2011-06-30 11:05       ` Takashi Iwai
@ 2011-06-30 11:28         ` Takashi Iwai
  -1 siblings, 0 replies; 30+ messages in thread
From: Takashi Iwai @ 2011-06-30 11:28 UTC (permalink / raw)
  To: Ralf Baechle
  Cc: Jaroslav Kysela, alsa-devel, linux-kernel, linux-mips, florian,
	Florian Fainelli, linux-arch, Richard Henderson, Ivan Kokshaysky,
	Matt Turner, Benjamin Herrenschmidt, Paul Mackerras,
	David S. Miller, sparclinux

At Thu, 30 Jun 2011 13:05:54 +0200,
Takashi Iwai wrote:
> 
> At Thu, 30 Jun 2011 11:52:54 +0100,
> Ralf Baechle wrote:
> > 
> > On Thu, Jun 30, 2011 at 12:15:56PM +0200, Takashi Iwai wrote:
> > 
> > > At Thu, 30 Jun 2011 10:17:54 +0100,
> > > Ralf Baechle wrote:
> > > > 
> > > > Found on a MIPS build but certain other architectures will have the same
> > > > issue:
> > > > 
> > > >   CC      sound/isa/sb/sb16_csp.o
> > > > sound/isa/sb/sb16_csp.c: In function ‘snd_sb_csp_ioctl’:
> > > > sound/isa/sb/sb16_csp.c:228: error: case label does not reduce to an integer constant
> > > > make[1]: *** [sound/isa/sb/sb16_csp.o] Error 1
> > > > make: *** [sound/isa/sb/sb16_csp.o] Error 2
> > > > 
> > > > This error message is caused by the _IOC_TYPECHECK() error check triggering
> > > > due to excessive ioctl size on Alpha, PowerPC, MIPS and SPARC which define
> > > > _IOC_SIZEBITS as 13.  On all other architectures define it as 14 so struct
> > > > snd_sb_csp_microcode with it's little over 12kB will just about fit into
> > > > the 16kB limit.
> > > 
> > > What about the patch below?
> > 
> > I have no idea how big the soundblaster microcode being loaded actually is,
> > that is if the reduced size of 0x1f00 will be sufficient.
> 
> The files found in /lib/firmware/sb16 are all under 2kB, thus likely
> sufficient.

Too shortly answered.  It turned out that some CSP codes (like Qsound)
can be above that size, it's almost 12kB.  So the size in the original
code is really the necessary requirement, and the patch breaks for
such a case.

An ugly workaround would be to fake the ioctl size.  But this is
certainly to be avoided, since it has been broken on the specific
platforms for ages, thus breaking for them would be mostly harmless,
too.

> > Aside of that I
> > don't see a problem - I don't see how the old ioctl can possibly have been
> > used before so there isn't a compatibility problem.
> > 
> > Or you could entirely sidestep the problem and use request_firmware() but
> > I guess that's more effort than you want to invest.
> 
> Yeah, that's another option I thought of.  But it's too intrusive for
> 3.0-rc6, so I'd like waive it for 3.1.

Actually the request_firmware() was implemented for some auto-loadable
CSP codes.  Others need the manual loading, so it is via ioctl.  It
can be converted, but I don't think it makes sense for such old
stuff.  After all, it still works with x86-ISA as is.


thanks,

Takashi

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: SB16 build error.
@ 2011-06-30 11:28         ` Takashi Iwai
  0 siblings, 0 replies; 30+ messages in thread
From: Takashi Iwai @ 2011-06-30 11:28 UTC (permalink / raw)
  To: Ralf Baechle
  Cc: Jaroslav Kysela, alsa-devel, linux-kernel, linux-mips, florian,
	Florian Fainelli, linux-arch, Richard Henderson, Ivan Kokshaysky,
	Matt Turner, Benjamin Herrenschmidt, Paul Mackerras,
	David S. Miller, sparclinux

At Thu, 30 Jun 2011 13:05:54 +0200,
Takashi Iwai wrote:
> 
> At Thu, 30 Jun 2011 11:52:54 +0100,
> Ralf Baechle wrote:
> > 
> > On Thu, Jun 30, 2011 at 12:15:56PM +0200, Takashi Iwai wrote:
> > 
> > > At Thu, 30 Jun 2011 10:17:54 +0100,
> > > Ralf Baechle wrote:
> > > > 
> > > > Found on a MIPS build but certain other architectures will have the same
> > > > issue:
> > > > 
> > > >   CC      sound/isa/sb/sb16_csp.o
> > > > sound/isa/sb/sb16_csp.c: In function ‘snd_sb_csp_ioctl’:
> > > > sound/isa/sb/sb16_csp.c:228: error: case label does not reduce to an integer constant
> > > > make[1]: *** [sound/isa/sb/sb16_csp.o] Error 1
> > > > make: *** [sound/isa/sb/sb16_csp.o] Error 2
> > > > 
> > > > This error message is caused by the _IOC_TYPECHECK() error check triggering
> > > > due to excessive ioctl size on Alpha, PowerPC, MIPS and SPARC which define
> > > > _IOC_SIZEBITS as 13.  On all other architectures define it as 14 so struct
> > > > snd_sb_csp_microcode with it's little over 12kB will just about fit into
> > > > the 16kB limit.
> > > 
> > > What about the patch below?
> > 
> > I have no idea how big the soundblaster microcode being loaded actually is,
> > that is if the reduced size of 0x1f00 will be sufficient.
> 
> The files found in /lib/firmware/sb16 are all under 2kB, thus likely
> sufficient.

Too shortly answered.  It turned out that some CSP codes (like Qsound)
can be above that size, it's almost 12kB.  So the size in the original
code is really the necessary requirement, and the patch breaks for
such a case.

An ugly workaround would be to fake the ioctl size.  But this is
certainly to be avoided, since it has been broken on the specific
platforms for ages, thus breaking for them would be mostly harmless,
too.

> > Aside of that I
> > don't see a problem - I don't see how the old ioctl can possibly have been
> > used before so there isn't a compatibility problem.
> > 
> > Or you could entirely sidestep the problem and use request_firmware() but
> > I guess that's more effort than you want to invest.
> 
> Yeah, that's another option I thought of.  But it's too intrusive for
> 3.0-rc6, so I'd like waive it for 3.1.

Actually the request_firmware() was implemented for some auto-loadable
CSP codes.  Others need the manual loading, so it is via ioctl.  It
can be converted, but I don't think it makes sense for such old
stuff.  After all, it still works with x86-ISA as is.


thanks,

Takashi

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: SB16 build error.
  2011-06-30 11:28         ` Takashi Iwai
  (?)
@ 2011-06-30 12:32           ` Ralf Baechle
  -1 siblings, 0 replies; 30+ messages in thread
From: Ralf Baechle @ 2011-06-30 12:32 UTC (permalink / raw)
  To: Takashi Iwai
  Cc: Jaroslav Kysela, alsa-devel, linux-kernel, linux-mips, florian,
	Florian Fainelli, linux-arch, Richard Henderson, Ivan Kokshaysky,
	Matt Turner, Benjamin Herrenschmidt, Paul Mackerras,
	David S. Miller, sparclinux

On Thu, Jun 30, 2011 at 01:28:03PM +0200, Takashi Iwai wrote:

> > > I have no idea how big the soundblaster microcode being loaded actually is,
> > > that is if the reduced size of 0x1f00 will be sufficient.
> > 
> > The files found in /lib/firmware/sb16 are all under 2kB, thus likely
> > sufficient.
> 
> Too shortly answered.  It turned out that some CSP codes (like Qsound)
> can be above that size, it's almost 12kB.  So the size in the original
> code is really the necessary requirement, and the patch breaks for
> such a case.
> 
> An ugly workaround would be to fake the ioctl size.  But this is
> certainly to be avoided, since it has been broken on the specific
> platforms for ages, thus breaking for them would be mostly harmless,
> too.
> 
> > > Aside of that I
> > > don't see a problem - I don't see how the old ioctl can possibly have been
> > > used before so there isn't a compatibility problem.
> > > 
> > > Or you could entirely sidestep the problem and use request_firmware() but
> > > I guess that's more effort than you want to invest.
> > 
> > Yeah, that's another option I thought of.  But it's too intrusive for
> > 3.0-rc6, so I'd like waive it for 3.1.
> 
> Actually the request_firmware() was implemented for some auto-loadable
> CSP codes.  Others need the manual loading, so it is via ioctl.  It
> can be converted, but I don't think it makes sense for such old
> stuff.  After all, it still works with x86-ISA as is.

In userland an empty definition will be used for _IOC_TYPECHECK so there
won't be an error.  So userland already is already using the existing
value for SNDRV_SB_CSP_IOCTL_LOAD_CODE ...

With a crude hack like

#define SNDRV_SB_CSP_IOCTL_LOAD_CODE				\
	_IOC(_IOC_WRITE,'H', 0x11, sizeof(struct snd_sb_csp_microcode))

error checking can be bypassed and all will be fine as long as the
resulting value doesn't result in in a a duplicate case value - which it
doesn't, at least not in my testing.

Should work but isn't nice.

  Ralf

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: SB16 build error.
@ 2011-06-30 12:32           ` Ralf Baechle
  0 siblings, 0 replies; 30+ messages in thread
From: Ralf Baechle @ 2011-06-30 12:32 UTC (permalink / raw)
  To: Takashi Iwai
  Cc: linux-arch, linux-mips, alsa-devel, florian, David S. Miller,
	Benjamin Herrenschmidt, linux-kernel, Ivan Kokshaysky,
	sparclinux, Paul Mackerras, Matt Turner, Florian Fainelli,
	Richard Henderson

On Thu, Jun 30, 2011 at 01:28:03PM +0200, Takashi Iwai wrote:

> > > I have no idea how big the soundblaster microcode being loaded actually is,
> > > that is if the reduced size of 0x1f00 will be sufficient.
> > 
> > The files found in /lib/firmware/sb16 are all under 2kB, thus likely
> > sufficient.
> 
> Too shortly answered.  It turned out that some CSP codes (like Qsound)
> can be above that size, it's almost 12kB.  So the size in the original
> code is really the necessary requirement, and the patch breaks for
> such a case.
> 
> An ugly workaround would be to fake the ioctl size.  But this is
> certainly to be avoided, since it has been broken on the specific
> platforms for ages, thus breaking for them would be mostly harmless,
> too.
> 
> > > Aside of that I
> > > don't see a problem - I don't see how the old ioctl can possibly have been
> > > used before so there isn't a compatibility problem.
> > > 
> > > Or you could entirely sidestep the problem and use request_firmware() but
> > > I guess that's more effort than you want to invest.
> > 
> > Yeah, that's another option I thought of.  But it's too intrusive for
> > 3.0-rc6, so I'd like waive it for 3.1.
> 
> Actually the request_firmware() was implemented for some auto-loadable
> CSP codes.  Others need the manual loading, so it is via ioctl.  It
> can be converted, but I don't think it makes sense for such old
> stuff.  After all, it still works with x86-ISA as is.

In userland an empty definition will be used for _IOC_TYPECHECK so there
won't be an error.  So userland already is already using the existing
value for SNDRV_SB_CSP_IOCTL_LOAD_CODE ...

With a crude hack like

#define SNDRV_SB_CSP_IOCTL_LOAD_CODE				\
	_IOC(_IOC_WRITE,'H', 0x11, sizeof(struct snd_sb_csp_microcode))

error checking can be bypassed and all will be fine as long as the
resulting value doesn't result in in a a duplicate case value - which it
doesn't, at least not in my testing.

Should work but isn't nice.

  Ralf

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: SB16 build error.
@ 2011-06-30 12:32           ` Ralf Baechle
  0 siblings, 0 replies; 30+ messages in thread
From: Ralf Baechle @ 2011-06-30 12:32 UTC (permalink / raw)
  To: Takashi Iwai
  Cc: linux-arch, linux-mips, alsa-devel, florian, David S. Miller,
	Benjamin Herrenschmidt, linux-kernel, Ivan Kokshaysky,
	sparclinux, Paul Mackerras, Matt Turner, Florian Fainelli,
	Richard Henderson

On Thu, Jun 30, 2011 at 01:28:03PM +0200, Takashi Iwai wrote:

> > > I have no idea how big the soundblaster microcode being loaded actually is,
> > > that is if the reduced size of 0x1f00 will be sufficient.
> > 
> > The files found in /lib/firmware/sb16 are all under 2kB, thus likely
> > sufficient.
> 
> Too shortly answered.  It turned out that some CSP codes (like Qsound)
> can be above that size, it's almost 12kB.  So the size in the original
> code is really the necessary requirement, and the patch breaks for
> such a case.
> 
> An ugly workaround would be to fake the ioctl size.  But this is
> certainly to be avoided, since it has been broken on the specific
> platforms for ages, thus breaking for them would be mostly harmless,
> too.
> 
> > > Aside of that I
> > > don't see a problem - I don't see how the old ioctl can possibly have been
> > > used before so there isn't a compatibility problem.
> > > 
> > > Or you could entirely sidestep the problem and use request_firmware() but
> > > I guess that's more effort than you want to invest.
> > 
> > Yeah, that's another option I thought of.  But it's too intrusive for
> > 3.0-rc6, so I'd like waive it for 3.1.
> 
> Actually the request_firmware() was implemented for some auto-loadable
> CSP codes.  Others need the manual loading, so it is via ioctl.  It
> can be converted, but I don't think it makes sense for such old
> stuff.  After all, it still works with x86-ISA as is.

In userland an empty definition will be used for _IOC_TYPECHECK so there
won't be an error.  So userland already is already using the existing
value for SNDRV_SB_CSP_IOCTL_LOAD_CODE ...

With a crude hack like

#define SNDRV_SB_CSP_IOCTL_LOAD_CODE				\
	_IOC(_IOC_WRITE,'H', 0x11, sizeof(struct snd_sb_csp_microcode))

error checking can be bypassed and all will be fine as long as the
resulting value doesn't result in in a a duplicate case value - which it
doesn't, at least not in my testing.

Should work but isn't nice.

  Ralf

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: SB16 build error.
  2011-06-30 12:32           ` Ralf Baechle
  (?)
@ 2011-06-30 12:38             ` Takashi Iwai
  -1 siblings, 0 replies; 30+ messages in thread
From: Takashi Iwai @ 2011-06-30 12:38 UTC (permalink / raw)
  To: Ralf Baechle
  Cc: Jaroslav Kysela, alsa-devel, linux-kernel, linux-mips, florian,
	Florian Fainelli, linux-arch, Richard Henderson, Ivan Kokshaysky,
	Matt Turner, Benjamin Herrenschmidt, Paul Mackerras,
	David S. Miller, sparclinux

At Thu, 30 Jun 2011 13:32:12 +0100,
Ralf Baechle wrote:
> 
> On Thu, Jun 30, 2011 at 01:28:03PM +0200, Takashi Iwai wrote:
> 
> > > > I have no idea how big the soundblaster microcode being loaded actually is,
> > > > that is if the reduced size of 0x1f00 will be sufficient.
> > > 
> > > The files found in /lib/firmware/sb16 are all under 2kB, thus likely
> > > sufficient.
> > 
> > Too shortly answered.  It turned out that some CSP codes (like Qsound)
> > can be above that size, it's almost 12kB.  So the size in the original
> > code is really the necessary requirement, and the patch breaks for
> > such a case.
> > 
> > An ugly workaround would be to fake the ioctl size.  But this is
> > certainly to be avoided, since it has been broken on the specific
> > platforms for ages, thus breaking for them would be mostly harmless,
> > too.
> > 
> > > > Aside of that I
> > > > don't see a problem - I don't see how the old ioctl can possibly have been
> > > > used before so there isn't a compatibility problem.
> > > > 
> > > > Or you could entirely sidestep the problem and use request_firmware() but
> > > > I guess that's more effort than you want to invest.
> > > 
> > > Yeah, that's another option I thought of.  But it's too intrusive for
> > > 3.0-rc6, so I'd like waive it for 3.1.
> > 
> > Actually the request_firmware() was implemented for some auto-loadable
> > CSP codes.  Others need the manual loading, so it is via ioctl.  It
> > can be converted, but I don't think it makes sense for such old
> > stuff.  After all, it still works with x86-ISA as is.
> 
> In userland an empty definition will be used for _IOC_TYPECHECK so there
> won't be an error.  So userland already is already using the existing
> value for SNDRV_SB_CSP_IOCTL_LOAD_CODE ...

Right.  It has an invalid direction (3), but apps won't care such
details anyway.

> With a crude hack like
> 
> #define SNDRV_SB_CSP_IOCTL_LOAD_CODE				\
> 	_IOC(_IOC_WRITE,'H', 0x11, sizeof(struct snd_sb_csp_microcode))
> 
> error checking can be bypassed and all will be fine as long as the
> resulting value doesn't result in in a a duplicate case value - which it
> doesn't, at least not in my testing.
> 
> Should work but isn't nice.

Indeed.  But which is uglier is hard to answer :)

If you are fine with the hacked ioctl number above, I can put it
with some comments.  This won't break anything, at least.


thanks,

Takashi

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: SB16 build error.
@ 2011-06-30 12:38             ` Takashi Iwai
  0 siblings, 0 replies; 30+ messages in thread
From: Takashi Iwai @ 2011-06-30 12:38 UTC (permalink / raw)
  To: Ralf Baechle
  Cc: linux-arch, linux-mips, alsa-devel, florian, David S. Miller,
	Benjamin Herrenschmidt, linux-kernel, Ivan Kokshaysky,
	sparclinux, Paul Mackerras, Matt Turner, Florian Fainelli,
	Richard Henderson

At Thu, 30 Jun 2011 13:32:12 +0100,
Ralf Baechle wrote:
> 
> On Thu, Jun 30, 2011 at 01:28:03PM +0200, Takashi Iwai wrote:
> 
> > > > I have no idea how big the soundblaster microcode being loaded actually is,
> > > > that is if the reduced size of 0x1f00 will be sufficient.
> > > 
> > > The files found in /lib/firmware/sb16 are all under 2kB, thus likely
> > > sufficient.
> > 
> > Too shortly answered.  It turned out that some CSP codes (like Qsound)
> > can be above that size, it's almost 12kB.  So the size in the original
> > code is really the necessary requirement, and the patch breaks for
> > such a case.
> > 
> > An ugly workaround would be to fake the ioctl size.  But this is
> > certainly to be avoided, since it has been broken on the specific
> > platforms for ages, thus breaking for them would be mostly harmless,
> > too.
> > 
> > > > Aside of that I
> > > > don't see a problem - I don't see how the old ioctl can possibly have been
> > > > used before so there isn't a compatibility problem.
> > > > 
> > > > Or you could entirely sidestep the problem and use request_firmware() but
> > > > I guess that's more effort than you want to invest.
> > > 
> > > Yeah, that's another option I thought of.  But it's too intrusive for
> > > 3.0-rc6, so I'd like waive it for 3.1.
> > 
> > Actually the request_firmware() was implemented for some auto-loadable
> > CSP codes.  Others need the manual loading, so it is via ioctl.  It
> > can be converted, but I don't think it makes sense for such old
> > stuff.  After all, it still works with x86-ISA as is.
> 
> In userland an empty definition will be used for _IOC_TYPECHECK so there
> won't be an error.  So userland already is already using the existing
> value for SNDRV_SB_CSP_IOCTL_LOAD_CODE ...

Right.  It has an invalid direction (3), but apps won't care such
details anyway.

> With a crude hack like
> 
> #define SNDRV_SB_CSP_IOCTL_LOAD_CODE				\
> 	_IOC(_IOC_WRITE,'H', 0x11, sizeof(struct snd_sb_csp_microcode))
> 
> error checking can be bypassed and all will be fine as long as the
> resulting value doesn't result in in a a duplicate case value - which it
> doesn't, at least not in my testing.
> 
> Should work but isn't nice.

Indeed.  But which is uglier is hard to answer :)

If you are fine with the hacked ioctl number above, I can put it
with some comments.  This won't break anything, at least.


thanks,

Takashi

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: SB16 build error.
@ 2011-06-30 12:38             ` Takashi Iwai
  0 siblings, 0 replies; 30+ messages in thread
From: Takashi Iwai @ 2011-06-30 12:38 UTC (permalink / raw)
  To: Ralf Baechle
  Cc: linux-arch, linux-mips, alsa-devel, florian, David S. Miller,
	Benjamin Herrenschmidt, linux-kernel, Ivan Kokshaysky,
	sparclinux, Paul Mackerras, Matt Turner, Florian Fainelli,
	Richard Henderson

At Thu, 30 Jun 2011 13:32:12 +0100,
Ralf Baechle wrote:
> 
> On Thu, Jun 30, 2011 at 01:28:03PM +0200, Takashi Iwai wrote:
> 
> > > > I have no idea how big the soundblaster microcode being loaded actually is,
> > > > that is if the reduced size of 0x1f00 will be sufficient.
> > > 
> > > The files found in /lib/firmware/sb16 are all under 2kB, thus likely
> > > sufficient.
> > 
> > Too shortly answered.  It turned out that some CSP codes (like Qsound)
> > can be above that size, it's almost 12kB.  So the size in the original
> > code is really the necessary requirement, and the patch breaks for
> > such a case.
> > 
> > An ugly workaround would be to fake the ioctl size.  But this is
> > certainly to be avoided, since it has been broken on the specific
> > platforms for ages, thus breaking for them would be mostly harmless,
> > too.
> > 
> > > > Aside of that I
> > > > don't see a problem - I don't see how the old ioctl can possibly have been
> > > > used before so there isn't a compatibility problem.
> > > > 
> > > > Or you could entirely sidestep the problem and use request_firmware() but
> > > > I guess that's more effort than you want to invest.
> > > 
> > > Yeah, that's another option I thought of.  But it's too intrusive for
> > > 3.0-rc6, so I'd like waive it for 3.1.
> > 
> > Actually the request_firmware() was implemented for some auto-loadable
> > CSP codes.  Others need the manual loading, so it is via ioctl.  It
> > can be converted, but I don't think it makes sense for such old
> > stuff.  After all, it still works with x86-ISA as is.
> 
> In userland an empty definition will be used for _IOC_TYPECHECK so there
> won't be an error.  So userland already is already using the existing
> value for SNDRV_SB_CSP_IOCTL_LOAD_CODE ...

Right.  It has an invalid direction (3), but apps won't care such
details anyway.

> With a crude hack like
> 
> #define SNDRV_SB_CSP_IOCTL_LOAD_CODE				\
> 	_IOC(_IOC_WRITE,'H', 0x11, sizeof(struct snd_sb_csp_microcode))
> 
> error checking can be bypassed and all will be fine as long as the
> resulting value doesn't result in in a a duplicate case value - which it
> doesn't, at least not in my testing.
> 
> Should work but isn't nice.

Indeed.  But which is uglier is hard to answer :)

If you are fine with the hacked ioctl number above, I can put it
with some comments.  This won't break anything, at least.


thanks,

Takashi

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: SB16 build error.
  2011-06-30 12:38             ` Takashi Iwai
  (?)
@ 2011-06-30 12:43               ` Ralf Baechle
  -1 siblings, 0 replies; 30+ messages in thread
From: Ralf Baechle @ 2011-06-30 12:43 UTC (permalink / raw)
  To: Takashi Iwai
  Cc: Jaroslav Kysela, alsa-devel, linux-kernel, linux-mips, florian,
	Florian Fainelli, linux-arch, Richard Henderson, Ivan Kokshaysky,
	Matt Turner, Benjamin Herrenschmidt, Paul Mackerras,
	David S. Miller, sparclinux

On Thu, Jun 30, 2011 at 02:38:20PM +0200, Takashi Iwai wrote:

> > In userland an empty definition will be used for _IOC_TYPECHECK so there
> > won't be an error.  So userland already is already using the existing
> > value for SNDRV_SB_CSP_IOCTL_LOAD_CODE ...
> 
> Right.  It has an invalid direction (3), but apps won't care such
> details anyway.
> 
> > With a crude hack like
> > 
> > #define SNDRV_SB_CSP_IOCTL_LOAD_CODE				\
> > 	_IOC(_IOC_WRITE,'H', 0x11, sizeof(struct snd_sb_csp_microcode))
> > 
> > error checking can be bypassed and all will be fine as long as the
> > resulting value doesn't result in in a a duplicate case value - which it
> > doesn't, at least not in my testing.
> > 
> > Should work but isn't nice.
> 
> Indeed.  But which is uglier is hard to answer :)
> 
> If you are fine with the hacked ioctl number above, I can put it
> with some comments.  This won't break anything, at least.

Go ahead then and yes, this really deserves a comment.

  Ralf

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: SB16 build error.
@ 2011-06-30 12:43               ` Ralf Baechle
  0 siblings, 0 replies; 30+ messages in thread
From: Ralf Baechle @ 2011-06-30 12:43 UTC (permalink / raw)
  To: Takashi Iwai
  Cc: linux-arch, linux-mips, alsa-devel, florian, David S. Miller,
	Benjamin Herrenschmidt, linux-kernel, Ivan Kokshaysky,
	sparclinux, Paul Mackerras, Matt Turner, Florian Fainelli,
	Richard Henderson

On Thu, Jun 30, 2011 at 02:38:20PM +0200, Takashi Iwai wrote:

> > In userland an empty definition will be used for _IOC_TYPECHECK so there
> > won't be an error.  So userland already is already using the existing
> > value for SNDRV_SB_CSP_IOCTL_LOAD_CODE ...
> 
> Right.  It has an invalid direction (3), but apps won't care such
> details anyway.
> 
> > With a crude hack like
> > 
> > #define SNDRV_SB_CSP_IOCTL_LOAD_CODE				\
> > 	_IOC(_IOC_WRITE,'H', 0x11, sizeof(struct snd_sb_csp_microcode))
> > 
> > error checking can be bypassed and all will be fine as long as the
> > resulting value doesn't result in in a a duplicate case value - which it
> > doesn't, at least not in my testing.
> > 
> > Should work but isn't nice.
> 
> Indeed.  But which is uglier is hard to answer :)
> 
> If you are fine with the hacked ioctl number above, I can put it
> with some comments.  This won't break anything, at least.

Go ahead then and yes, this really deserves a comment.

  Ralf

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: SB16 build error.
@ 2011-06-30 12:43               ` Ralf Baechle
  0 siblings, 0 replies; 30+ messages in thread
From: Ralf Baechle @ 2011-06-30 12:43 UTC (permalink / raw)
  To: Takashi Iwai
  Cc: linux-arch, linux-mips, alsa-devel, florian, David S. Miller,
	Benjamin Herrenschmidt, linux-kernel, Ivan Kokshaysky,
	sparclinux, Paul Mackerras, Matt Turner, Florian Fainelli,
	Richard Henderson

On Thu, Jun 30, 2011 at 02:38:20PM +0200, Takashi Iwai wrote:

> > In userland an empty definition will be used for _IOC_TYPECHECK so there
> > won't be an error.  So userland already is already using the existing
> > value for SNDRV_SB_CSP_IOCTL_LOAD_CODE ...
> 
> Right.  It has an invalid direction (3), but apps won't care such
> details anyway.
> 
> > With a crude hack like
> > 
> > #define SNDRV_SB_CSP_IOCTL_LOAD_CODE				\
> > 	_IOC(_IOC_WRITE,'H', 0x11, sizeof(struct snd_sb_csp_microcode))
> > 
> > error checking can be bypassed and all will be fine as long as the
> > resulting value doesn't result in in a a duplicate case value - which it
> > doesn't, at least not in my testing.
> > 
> > Should work but isn't nice.
> 
> Indeed.  But which is uglier is hard to answer :)
> 
> If you are fine with the hacked ioctl number above, I can put it
> with some comments.  This won't break anything, at least.

Go ahead then and yes, this really deserves a comment.

  Ralf

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: SB16 build error.
  2011-06-30 12:32           ` Ralf Baechle
@ 2011-06-30 12:54             ` Arnd Bergmann
  -1 siblings, 0 replies; 30+ messages in thread
From: Arnd Bergmann @ 2011-06-30 12:54 UTC (permalink / raw)
  To: Ralf Baechle
  Cc: Takashi Iwai, Jaroslav Kysela, alsa-devel, linux-kernel,
	linux-mips, florian, Florian Fainelli, linux-arch,
	Richard Henderson, Ivan Kokshaysky, Matt Turner,
	Benjamin Herrenschmidt, Paul Mackerras, David S. Miller,
	sparclinux

On Thursday 30 June 2011, Ralf Baechle wrote:
> #define SNDRV_SB_CSP_IOCTL_LOAD_CODE                            \
>         _IOC(_IOC_WRITE,'H', 0x11, sizeof(struct snd_sb_csp_microcode))
> 
> error checking can be bypassed and all will be fine as long as the
> resulting value doesn't result in in a a duplicate case value - which it
> doesn't, at least not in my testing.
> 
> Should work but isn't nice.

Right. It's probably the best we can do. I think we added a few similar
definitions when we originally introduce _IOC_TYPECHECK. The idea was
never to break existing code, but rather to avoid merging new drivers that
use inconsistent ioctl command numbers.

	Arnd

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: SB16 build error.
@ 2011-06-30 12:54             ` Arnd Bergmann
  0 siblings, 0 replies; 30+ messages in thread
From: Arnd Bergmann @ 2011-06-30 12:54 UTC (permalink / raw)
  To: Ralf Baechle
  Cc: Takashi Iwai, Jaroslav Kysela, alsa-devel, linux-kernel,
	linux-mips, florian, Florian Fainelli, linux-arch,
	Richard Henderson, Ivan Kokshaysky, Matt Turner,
	Benjamin Herrenschmidt, Paul Mackerras, David S. Miller,
	sparclinux

On Thursday 30 June 2011, Ralf Baechle wrote:
> #define SNDRV_SB_CSP_IOCTL_LOAD_CODE                            \
>         _IOC(_IOC_WRITE,'H', 0x11, sizeof(struct snd_sb_csp_microcode))
> 
> error checking can be bypassed and all will be fine as long as the
> resulting value doesn't result in in a a duplicate case value - which it
> doesn't, at least not in my testing.
> 
> Should work but isn't nice.

Right. It's probably the best we can do. I think we added a few similar
definitions when we originally introduce _IOC_TYPECHECK. The idea was
never to break existing code, but rather to avoid merging new drivers that
use inconsistent ioctl command numbers.

	Arnd

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [alsa-devel] SB16 build error.
  2011-06-30 10:52     ` Ralf Baechle
@ 2011-06-30 13:10       ` Clemens Ladisch
  -1 siblings, 0 replies; 30+ messages in thread
From: Clemens Ladisch @ 2011-06-30 13:10 UTC (permalink / raw)
  To: Ralf Baechle
  Cc: Takashi Iwai, linux-arch, linux-mips, alsa-devel, florian,
	David S. Miller, Benjamin Herrenschmidt, linux-kernel,
	Ivan Kokshaysky, sparclinux, Paul Mackerras, Matt Turner,
	Florian Fainelli, Richard Henderson

Ralf Baechle wrote:
> I have no idea how big the soundblaster microcode being loaded actually is,
> that is if the reduced size of 0x1f00 will be sufficient.

The biggest file is WFM0001A.CSP with 0x2df0 bytes.

> I don't see how the old ioctl can possibly have been
> used before so there isn't a compatibility problem.

The code uses SNDRV_SB_CSP_MAX_MICROCODE_FILE_SIZE but doesn't care what
the size field of the ioctl code is, so we could use any random value on
those architectures.

> Or you could entirely sidestep the problem and use request_firmware()
> but I guess that's more effort than you want to invest.

The driver already implements this for a bunch of predefined CSP code
blobs.  I'm not sure whether anybody has ever loaded additional .csp
files.


Regards,
Clemens

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [alsa-devel] SB16 build error.
@ 2011-06-30 13:10       ` Clemens Ladisch
  0 siblings, 0 replies; 30+ messages in thread
From: Clemens Ladisch @ 2011-06-30 13:10 UTC (permalink / raw)
  To: Ralf Baechle
  Cc: Takashi Iwai, linux-arch, linux-mips, alsa-devel, florian,
	David S. Miller, Benjamin Herrenschmidt, linux-kernel,
	Ivan Kokshaysky, sparclinux, Paul Mackerras, Matt Turner,
	Florian Fainelli, Richard Henderson

Ralf Baechle wrote:
> I have no idea how big the soundblaster microcode being loaded actually is,
> that is if the reduced size of 0x1f00 will be sufficient.

The biggest file is WFM0001A.CSP with 0x2df0 bytes.

> I don't see how the old ioctl can possibly have been
> used before so there isn't a compatibility problem.

The code uses SNDRV_SB_CSP_MAX_MICROCODE_FILE_SIZE but doesn't care what
the size field of the ioctl code is, so we could use any random value on
those architectures.

> Or you could entirely sidestep the problem and use request_firmware()
> but I guess that's more effort than you want to invest.

The driver already implements this for a bunch of predefined CSP code
blobs.  I'm not sure whether anybody has ever loaded additional .csp
files.


Regards,
Clemens

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: SB16 build error.
  2011-06-30 12:43               ` Ralf Baechle
  (?)
@ 2011-06-30 13:14                 ` Takashi Iwai
  -1 siblings, 0 replies; 30+ messages in thread
From: Takashi Iwai @ 2011-06-30 13:14 UTC (permalink / raw)
  To: Ralf Baechle
  Cc: Jaroslav Kysela, alsa-devel, linux-kernel, linux-mips, florian,
	Florian Fainelli, linux-arch, Richard Henderson, Ivan Kokshaysky,
	Matt Turner, Benjamin Herrenschmidt, Paul Mackerras,
	David S. Miller, sparclinux

At Thu, 30 Jun 2011 13:43:33 +0100,
Ralf Baechle wrote:
> 
> On Thu, Jun 30, 2011 at 02:38:20PM +0200, Takashi Iwai wrote:
> 
> > > In userland an empty definition will be used for _IOC_TYPECHECK so there
> > > won't be an error.  So userland already is already using the existing
> > > value for SNDRV_SB_CSP_IOCTL_LOAD_CODE ...
> > 
> > Right.  It has an invalid direction (3), but apps won't care such
> > details anyway.
> > 
> > > With a crude hack like
> > > 
> > > #define SNDRV_SB_CSP_IOCTL_LOAD_CODE				\
> > > 	_IOC(_IOC_WRITE,'H', 0x11, sizeof(struct snd_sb_csp_microcode))
> > > 
> > > error checking can be bypassed and all will be fine as long as the
> > > resulting value doesn't result in in a a duplicate case value - which it
> > > doesn't, at least not in my testing.
> > > 
> > > Should work but isn't nice.
> > 
> > Indeed.  But which is uglier is hard to answer :)
> > 
> > If you are fine with the hacked ioctl number above, I can put it
> > with some comments.  This won't break anything, at least.
> 
> Go ahead then and yes, this really deserves a comment.

OK, here is the patch.


Takashi

===
From: Takashi Iwai <tiwai@suse.de>
Subject: [PATCH] ALSA: sb16 - Fix build errors on MIPS and others with 13bit
 ioctl size

One of ioctl definition in sound/sb16_csp.h contains the data size
over 8kB, and this causes build errors on architectures like MIPS,
which define _IOC_SIZEBITS=13.

For avoiding this build errors but keeping the compatibility, manually
exapnd with _IOC() instead of using _IOW() for the problematic ioctl.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 include/sound/sb16_csp.h |    9 ++++++++-
 1 files changed, 8 insertions(+), 1 deletions(-)

diff --git a/include/sound/sb16_csp.h b/include/sound/sb16_csp.h
index 736eac7..af1b49e 100644
--- a/include/sound/sb16_csp.h
+++ b/include/sound/sb16_csp.h
@@ -99,7 +99,14 @@ struct snd_sb_csp_info {
 /* get CSP information */
 #define SNDRV_SB_CSP_IOCTL_INFO		_IOR('H', 0x10, struct snd_sb_csp_info)
 /* load microcode to CSP */
-#define SNDRV_SB_CSP_IOCTL_LOAD_CODE	_IOW('H', 0x11, struct snd_sb_csp_microcode)
+/* NOTE: struct snd_sb_csp_microcode overflows the max size (13 bits)
+ * defined for some architectures like MIPS, and it leads to build errors.
+ * (x86 and co have 14-bit size, thus it's valid, though.)
+ * As a workaround for skipping the size-limit check, here we don't use the
+ * normal _IOW() macro but _IOC() with the manual argument.
+ */
+#define SNDRV_SB_CSP_IOCTL_LOAD_CODE	\
+	_IOC(_IOC_WRITE, 'H', 0x11, sizeof(struct snd_sb_csp_microcode))
 /* unload microcode from CSP */
 #define SNDRV_SB_CSP_IOCTL_UNLOAD_CODE	_IO('H', 0x12)
 /* start CSP */
-- 
1.7.6


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* Re: SB16 build error.
@ 2011-06-30 13:14                 ` Takashi Iwai
  0 siblings, 0 replies; 30+ messages in thread
From: Takashi Iwai @ 2011-06-30 13:14 UTC (permalink / raw)
  To: Ralf Baechle
  Cc: linux-arch, linux-mips, alsa-devel, florian, David S. Miller,
	Benjamin Herrenschmidt, linux-kernel, Ivan Kokshaysky,
	sparclinux, Paul Mackerras, Matt Turner, Florian Fainelli,
	Richard Henderson

At Thu, 30 Jun 2011 13:43:33 +0100,
Ralf Baechle wrote:
> 
> On Thu, Jun 30, 2011 at 02:38:20PM +0200, Takashi Iwai wrote:
> 
> > > In userland an empty definition will be used for _IOC_TYPECHECK so there
> > > won't be an error.  So userland already is already using the existing
> > > value for SNDRV_SB_CSP_IOCTL_LOAD_CODE ...
> > 
> > Right.  It has an invalid direction (3), but apps won't care such
> > details anyway.
> > 
> > > With a crude hack like
> > > 
> > > #define SNDRV_SB_CSP_IOCTL_LOAD_CODE				\
> > > 	_IOC(_IOC_WRITE,'H', 0x11, sizeof(struct snd_sb_csp_microcode))
> > > 
> > > error checking can be bypassed and all will be fine as long as the
> > > resulting value doesn't result in in a a duplicate case value - which it
> > > doesn't, at least not in my testing.
> > > 
> > > Should work but isn't nice.
> > 
> > Indeed.  But which is uglier is hard to answer :)
> > 
> > If you are fine with the hacked ioctl number above, I can put it
> > with some comments.  This won't break anything, at least.
> 
> Go ahead then and yes, this really deserves a comment.

OK, here is the patch.


Takashi

===
From: Takashi Iwai <tiwai@suse.de>
Subject: [PATCH] ALSA: sb16 - Fix build errors on MIPS and others with 13bit
 ioctl size

One of ioctl definition in sound/sb16_csp.h contains the data size
over 8kB, and this causes build errors on architectures like MIPS,
which define _IOC_SIZEBITS=13.

For avoiding this build errors but keeping the compatibility, manually
exapnd with _IOC() instead of using _IOW() for the problematic ioctl.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 include/sound/sb16_csp.h |    9 ++++++++-
 1 files changed, 8 insertions(+), 1 deletions(-)

diff --git a/include/sound/sb16_csp.h b/include/sound/sb16_csp.h
index 736eac7..af1b49e 100644
--- a/include/sound/sb16_csp.h
+++ b/include/sound/sb16_csp.h
@@ -99,7 +99,14 @@ struct snd_sb_csp_info {
 /* get CSP information */
 #define SNDRV_SB_CSP_IOCTL_INFO		_IOR('H', 0x10, struct snd_sb_csp_info)
 /* load microcode to CSP */
-#define SNDRV_SB_CSP_IOCTL_LOAD_CODE	_IOW('H', 0x11, struct snd_sb_csp_microcode)
+/* NOTE: struct snd_sb_csp_microcode overflows the max size (13 bits)
+ * defined for some architectures like MIPS, and it leads to build errors.
+ * (x86 and co have 14-bit size, thus it's valid, though.)
+ * As a workaround for skipping the size-limit check, here we don't use the
+ * normal _IOW() macro but _IOC() with the manual argument.
+ */
+#define SNDRV_SB_CSP_IOCTL_LOAD_CODE	\
+	_IOC(_IOC_WRITE, 'H', 0x11, sizeof(struct snd_sb_csp_microcode))
 /* unload microcode from CSP */
 #define SNDRV_SB_CSP_IOCTL_UNLOAD_CODE	_IO('H', 0x12)
 /* start CSP */
-- 
1.7.6

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* Re: SB16 build error.
@ 2011-06-30 13:14                 ` Takashi Iwai
  0 siblings, 0 replies; 30+ messages in thread
From: Takashi Iwai @ 2011-06-30 13:14 UTC (permalink / raw)
  To: Ralf Baechle
  Cc: linux-arch, linux-mips, alsa-devel, florian, David S. Miller,
	Benjamin Herrenschmidt, linux-kernel, Ivan Kokshaysky,
	sparclinux, Paul Mackerras, Matt Turner, Florian Fainelli,
	Richard Henderson

At Thu, 30 Jun 2011 13:43:33 +0100,
Ralf Baechle wrote:
> 
> On Thu, Jun 30, 2011 at 02:38:20PM +0200, Takashi Iwai wrote:
> 
> > > In userland an empty definition will be used for _IOC_TYPECHECK so there
> > > won't be an error.  So userland already is already using the existing
> > > value for SNDRV_SB_CSP_IOCTL_LOAD_CODE ...
> > 
> > Right.  It has an invalid direction (3), but apps won't care such
> > details anyway.
> > 
> > > With a crude hack like
> > > 
> > > #define SNDRV_SB_CSP_IOCTL_LOAD_CODE				\
> > > 	_IOC(_IOC_WRITE,'H', 0x11, sizeof(struct snd_sb_csp_microcode))
> > > 
> > > error checking can be bypassed and all will be fine as long as the
> > > resulting value doesn't result in in a a duplicate case value - which it
> > > doesn't, at least not in my testing.
> > > 
> > > Should work but isn't nice.
> > 
> > Indeed.  But which is uglier is hard to answer :)
> > 
> > If you are fine with the hacked ioctl number above, I can put it
> > with some comments.  This won't break anything, at least.
> 
> Go ahead then and yes, this really deserves a comment.

OK, here is the patch.


Takashi

=From: Takashi Iwai <tiwai@suse.de>
Subject: [PATCH] ALSA: sb16 - Fix build errors on MIPS and others with 13bit
 ioctl size

One of ioctl definition in sound/sb16_csp.h contains the data size
over 8kB, and this causes build errors on architectures like MIPS,
which define _IOC_SIZEBITS\x13.

For avoiding this build errors but keeping the compatibility, manually
exapnd with _IOC() instead of using _IOW() for the problematic ioctl.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 include/sound/sb16_csp.h |    9 ++++++++-
 1 files changed, 8 insertions(+), 1 deletions(-)

diff --git a/include/sound/sb16_csp.h b/include/sound/sb16_csp.h
index 736eac7..af1b49e 100644
--- a/include/sound/sb16_csp.h
+++ b/include/sound/sb16_csp.h
@@ -99,7 +99,14 @@ struct snd_sb_csp_info {
 /* get CSP information */
 #define SNDRV_SB_CSP_IOCTL_INFO		_IOR('H', 0x10, struct snd_sb_csp_info)
 /* load microcode to CSP */
-#define SNDRV_SB_CSP_IOCTL_LOAD_CODE	_IOW('H', 0x11, struct snd_sb_csp_microcode)
+/* NOTE: struct snd_sb_csp_microcode overflows the max size (13 bits)
+ * defined for some architectures like MIPS, and it leads to build errors.
+ * (x86 and co have 14-bit size, thus it's valid, though.)
+ * As a workaround for skipping the size-limit check, here we don't use the
+ * normal _IOW() macro but _IOC() with the manual argument.
+ */
+#define SNDRV_SB_CSP_IOCTL_LOAD_CODE	\
+	_IOC(_IOC_WRITE, 'H', 0x11, sizeof(struct snd_sb_csp_microcode))
 /* unload microcode from CSP */
 #define SNDRV_SB_CSP_IOCTL_UNLOAD_CODE	_IO('H', 0x12)
 /* start CSP */
-- 
1.7.6


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* Re: SB16 build error.
  2011-06-30 12:43               ` Ralf Baechle
@ 2011-07-01 15:31                 ` David Howells
  -1 siblings, 0 replies; 30+ messages in thread
From: David Howells @ 2011-07-01 15:31 UTC (permalink / raw)
  To: Takashi Iwai
  Cc: dhowells, Ralf Baechle, Jaroslav Kysela, alsa-devel,
	linux-kernel, linux-mips, florian, Florian Fainelli, linux-arch,
	Richard Henderson, Ivan Kokshaysky, Matt Turner,
	Benjamin Herrenschmidt, Paul Mackerras, David S. Miller,
	sparclinux

Takashi Iwai <tiwai@suse.de> wrote:

> OK, here is the patch.

Acked-by: David Howells <dhowells@redhat.com>

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: SB16 build error.
@ 2011-07-01 15:31                 ` David Howells
  0 siblings, 0 replies; 30+ messages in thread
From: David Howells @ 2011-07-01 15:31 UTC (permalink / raw)
  To: Takashi Iwai
  Cc: dhowells, Ralf Baechle, Jaroslav Kysela, alsa-devel,
	linux-kernel, linux-mips, florian, Florian Fainelli, linux-arch,
	Richard Henderson, Ivan Kokshaysky, Matt Turner,
	Benjamin Herrenschmidt, Paul Mackerras, David S. Miller,
	sparclinux

Takashi Iwai <tiwai@suse.de> wrote:

> OK, here is the patch.

Acked-by: David Howells <dhowells@redhat.com>

^ permalink raw reply	[flat|nested] 30+ messages in thread

end of thread, other threads:[~2011-07-01 15:33 UTC | newest]

Thread overview: 30+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-06-30  9:17 SB16 build error Ralf Baechle
2011-06-30  9:17 ` Ralf Baechle
2011-06-30  9:17 ` Ralf Baechle
2011-06-30 10:15 ` Takashi Iwai
2011-06-30 10:15   ` Takashi Iwai
2011-06-30 10:52   ` Ralf Baechle
2011-06-30 10:52     ` Ralf Baechle
2011-06-30 10:52     ` Ralf Baechle
2011-06-30 11:05     ` Takashi Iwai
2011-06-30 11:05       ` Takashi Iwai
2011-06-30 11:28       ` Takashi Iwai
2011-06-30 11:28         ` Takashi Iwai
2011-06-30 12:32         ` Ralf Baechle
2011-06-30 12:32           ` Ralf Baechle
2011-06-30 12:32           ` Ralf Baechle
2011-06-30 12:38           ` Takashi Iwai
2011-06-30 12:38             ` Takashi Iwai
2011-06-30 12:38             ` Takashi Iwai
2011-06-30 12:43             ` Ralf Baechle
2011-06-30 12:43               ` Ralf Baechle
2011-06-30 12:43               ` Ralf Baechle
2011-06-30 13:14               ` Takashi Iwai
2011-06-30 13:14                 ` Takashi Iwai
2011-06-30 13:14                 ` Takashi Iwai
2011-07-01 15:31               ` David Howells
2011-07-01 15:31                 ` David Howells
2011-06-30 12:54           ` Arnd Bergmann
2011-06-30 12:54             ` Arnd Bergmann
2011-06-30 13:10     ` [alsa-devel] " Clemens Ladisch
2011-06-30 13:10       ` Clemens Ladisch

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.