[PATCH] Fix __ucmpdi2 in v4l2_norm_to_name()

[PATCH] Fix __ucmpdi2 in v4l2_norm_to_name()

[Stelian Pop]

Hi,

This patch replaces a switch statement using 64 bit values with the
if/else equivalent in order to prevent a call __ucmpdi2 generated by
some versions of gcc (verified with gcc-4.1.2 20060928):

	drivers/built-in.o: In function `v4l2_norm_to_name':
	(.text+0x71100): undefined reference to `__ucmpdi2'

Signed-off-by: Stelian Pop <stelian@popies.net>

---

 drivers/media/video/v4l2-common.c |  126 ++++++++++++++++++-------------------
 1 files changed, 62 insertions(+), 64 deletions(-)

diff --git a/drivers/media/video/v4l2-common.c b/drivers/media/video/v4l2-common.c
index 752c82c..0c3c2f6 100644
--- a/drivers/media/video/v4l2-common.c
+++ b/drivers/media/video/v4l2-common.c
@@ -91,70 +91,68 @@ char *v4l2_norm_to_name(v4l2_std_id id)
 {
 	char *name;
 
-	switch (id) {
-	case V4L2_STD_PAL:
-		name="PAL";		break;
-	case V4L2_STD_PAL_BG:
-		name="PAL-BG";		break;
-	case V4L2_STD_PAL_DK:
-		name="PAL-DK";		break;
-	case V4L2_STD_PAL_B:
-		name="PAL-B";		break;
-	case V4L2_STD_PAL_B1:
-		name="PAL-B1";		break;
-	case V4L2_STD_PAL_G:
-		name="PAL-G";		break;
-	case V4L2_STD_PAL_H:
-		name="PAL-H";		break;
-	case V4L2_STD_PAL_I:
-		name="PAL-I";		break;
-	case V4L2_STD_PAL_D:
-		name="PAL-D";		break;
-	case V4L2_STD_PAL_D1:
-		name="PAL-D1";		break;
-	case V4L2_STD_PAL_K:
-		name="PAL-K";		break;
-	case V4L2_STD_PAL_M:
-		name="PAL-M";		break;
-	case V4L2_STD_PAL_N:
-		name="PAL-N";		break;
-	case V4L2_STD_PAL_Nc:
-		name="PAL-Nc";		break;
-	case V4L2_STD_PAL_60:
-		name="PAL-60";		break;
-	case V4L2_STD_NTSC:
-		name="NTSC";		break;
-	case V4L2_STD_NTSC_M:
-		name="NTSC-M";		break;
-	case V4L2_STD_NTSC_M_JP:
-		name="NTSC-M-JP";	break;
-	case V4L2_STD_NTSC_443:
-		name="NTSC-443";	break;
-	case V4L2_STD_NTSC_M_KR:
-		name="NTSC-M-KR";	break;
-	case V4L2_STD_SECAM:
-		name="SECAM";		break;
-	case V4L2_STD_SECAM_DK:
-		name="SECAM-DK";	break;
-	case V4L2_STD_SECAM_B:
-		name="SECAM-B";		break;
-	case V4L2_STD_SECAM_D:
-		name="SECAM-D";		break;
-	case V4L2_STD_SECAM_G:
-		name="SECAM-G";		break;
-	case V4L2_STD_SECAM_H:
-		name="SECAM-H";		break;
-	case V4L2_STD_SECAM_K:
-		name="SECAM-K";		break;
-	case V4L2_STD_SECAM_K1:
-		name="SECAM-K1";	break;
-	case V4L2_STD_SECAM_L:
-		name="SECAM-L";		break;
-	case V4L2_STD_SECAM_LC:
-		name="SECAM-LC";	break;
-	default:
-		name="Unknown";		break;
-	}
+	if (id == V4L2_STD_PAL)
+		name = "PAL";
+	else if (id == V4L2_STD_PAL_BG)
+		name = "PAL-BG";
+	else if (id == V4L2_STD_PAL_DK)
+		name = "PAL-DK";
+	else if (id == V4L2_STD_PAL_B)
+		name = "PAL-B";
+	else if (id == V4L2_STD_PAL_B1)
+		name = "PAL-B1";
+	else if (id == V4L2_STD_PAL_G)
+		name = "PAL-G";
+	else if (id == V4L2_STD_PAL_H)
+		name = "PAL-H";
+	else if (id == V4L2_STD_PAL_I)
+		name = "PAL-I";
+	else if (id == V4L2_STD_PAL_D)
+		name = "PAL-D";
+	else if (id == V4L2_STD_PAL_D1)
+		name = "PAL-D1";
+	else if (id == V4L2_STD_PAL_K)
+		name = "PAL-K";
+	else if (id == V4L2_STD_PAL_M)
+		name = "PAL-M";
+	else if (id == V4L2_STD_PAL_N)
+		name = "PAL-N";
+	else if (id == V4L2_STD_PAL_Nc)
+		name = "PAL-Nc";
+	else if (id == V4L2_STD_PAL_60)
+		name = "PAL-60";
+	else if (id == V4L2_STD_NTSC)
+		name = "NTSC";
+	else if (id == V4L2_STD_NTSC_M)
+		name = "NTSC-M";
+	else if (id == V4L2_STD_NTSC_M_JP)
+		name = "NTSC-M-JP";
+	else if (id == V4L2_STD_NTSC_443)
+		name = "NTSC-443";
+	else if (id == V4L2_STD_NTSC_M_KR)
+		name = "NTSC-M-KR";
+	else if (id == V4L2_STD_SECAM)
+		name = "SECAM";
+	else if (id == V4L2_STD_SECAM_DK)
+		name = "SECAM-DK";
+	else if (id == V4L2_STD_SECAM_B)
+		name = "SECAM-B";
+	else if (id == V4L2_STD_SECAM_D)
+		name = "SECAM-D";
+	else if (id == V4L2_STD_SECAM_G)
+		name = "SECAM-G";
+	else if (id == V4L2_STD_SECAM_H)
+		name = "SECAM-H";
+	else if (id == V4L2_STD_SECAM_K)
+		name = "SECAM-K";
+	else if (id == V4L2_STD_SECAM_K1)
+		name = "SECAM-K1";
+	else if (id == V4L2_STD_SECAM_L)
+		name = "SECAM-L";
+	else if (id == V4L2_STD_SECAM_LC)
+		name = "SECAM-LC";
+	else
+		name = "Unknown";
 
 	return name;
 }

-- 
Stelian Pop <stelian@popies.net>

Re: [v4l-dvb-maintainer] [PATCH] Fix __ucmpdi2 in v4l2_norm_to_name()

[Trent Piepho]

On Thu, 4 Jan 2007, Stelian Pop wrote:
> This patch replaces a switch statement using 64 bit values with the
> if/else equivalent in order to prevent a call __ucmpdi2 generated by
> some versions of gcc (verified with gcc-4.1.2 20060928):
>
> 	drivers/built-in.o: In function `v4l2_norm_to_name':
> 	(.text+0x71100): undefined reference to `__ucmpdi2'
>
> Signed-off-by: Stelian Pop <stelian@popies.net>

It looks like there is a much better way to handle this problem here:
http://lkml.org/lkml/2006/12/17/46

Basically:
A. Fix gcc so it doesn't call in __ucmpdi2 (already fixed?)
B. Link in __ucmpdi2 from libgcc
C. Write arch specific code to provide __ucmpdi2

Re: [v4l-dvb-maintainer] [PATCH] Fix __ucmpdi2 in v4l2_norm_to_name()

[Stelian Pop]

Le jeudi 04 janvier 2007 à 04:09 -0800, Trent Piepho a écrit :
> On Thu, 4 Jan 2007, Stelian Pop wrote:
> > This patch replaces a switch statement using 64 bit values with the
> > if/else equivalent in order to prevent a call __ucmpdi2 generated by
> > some versions of gcc (verified with gcc-4.1.2 20060928):
> >
> > 	drivers/built-in.o: In function `v4l2_norm_to_name':
> > 	(.text+0x71100): undefined reference to `__ucmpdi2'
> >
> > Signed-off-by: Stelian Pop <stelian@popies.net>
> 
> It looks like there is a much better way to handle this problem here:
> http://lkml.org/lkml/2006/12/17/46
> 
> Basically:
> A. Fix gcc so it doesn't call in __ucmpdi2 (already fixed?)
> B. Link in __ucmpdi2 from libgcc
> C. Write arch specific code to provide __ucmpdi2

Indeed, I didn't see that thread.

Linus, just make sure you apply a fix for this problem before 2.4.20
goes final...

Thanks and sorry for the noise.

Stelian.
-- 
Stelian Pop <stelian@popies.net>

Re: [PATCH] Fix __ucmpdi2 in v4l2_norm_to_name()

[Andrew Morton]

On Thu, 04 Jan 2007 12:10:14 +0100
Stelian Pop <stelian@popies.net> wrote:

> Hi,
> 
> This patch replaces a switch statement using 64 bit values with the
> if/else equivalent in order to prevent a call __ucmpdi2 generated by
> some versions of gcc (verified with gcc-4.1.2 20060928):
> 
> 	drivers/built-in.o: In function `v4l2_norm_to_name':
> 	(.text+0x71100): undefined reference to `__ucmpdi2'
> 
> Signed-off-by: Stelian Pop <stelian@popies.net>
> 
> ---
> 
>  drivers/media/video/v4l2-common.c |  126 ++++++++++++++++++-------------------
>  1 files changed, 62 insertions(+), 64 deletions(-)
> 
> diff --git a/drivers/media/video/v4l2-common.c b/drivers/media/video/v4l2-common.c
> index 752c82c..0c3c2f6 100644
> --- a/drivers/media/video/v4l2-common.c
> +++ b/drivers/media/video/v4l2-common.c
> @@ -91,70 +91,68 @@ char *v4l2_norm_to_name(v4l2_std_id id)
>  {
>  	char *name;
>  
> -	switch (id) {

It'd be simpler to just do

	switch ((unsigned int)id) {

here (with a suitable comment).

The largest value we use here is 0x02000000.  Perhaps v4l2_std_id shouldn't
be 64-bit?

Re: [PATCH] Fix __ucmpdi2 in v4l2_norm_to_name()

[Mauro Carvalho Chehab]

Em Qui, 2007-01-04 às 14:48 -0800, Andrew Morton escreveu:
> On Thu, 04 Jan 2007 12:10:14 +0100
> Stelian Pop <stelian@popies.net> wrote:
> 
> > Hi,
> > 
> > This patch replaces a switch statement using 64 bit values with the
> > if/else equivalent in order to prevent a call __ucmpdi2 generated by
> > some versions of gcc (verified with gcc-4.1.2 20060928):
> > 
> > 	drivers/built-in.o: In function `v4l2_norm_to_name':
> > 	(.text+0x71100): undefined reference to `__ucmpdi2'
> > 
> > Signed-off-by: Stelian Pop <stelian@popies.net>
> > 
> > ---
> > 
> >  drivers/media/video/v4l2-common.c |  126 ++++++++++++++++++-------------------
> >  1 files changed, 62 insertions(+), 64 deletions(-)
> > 
> > diff --git a/drivers/media/video/v4l2-common.c b/drivers/media/video/v4l2-common.c
> > index 752c82c..0c3c2f6 100644
> > --- a/drivers/media/video/v4l2-common.c
> > +++ b/drivers/media/video/v4l2-common.c
> > @@ -91,70 +91,68 @@ char *v4l2_norm_to_name(v4l2_std_id id)
> >  {
> >  	char *name;
> >  
> > -	switch (id) {
> 
> It'd be simpler to just do
> 
> 	switch ((unsigned int)id) {
> 
> here (with a suitable comment).
> 
> The largest value we use here is 0x02000000.  Perhaps v4l2_std_id shouldn't
> be 64-bit?
Too late to change it to 32 bits. It is at V4L2 userspace API since
kernel 2.6.0. We can, however use this approach as a workaround, with
the proper documentation. I'll handle it after I return from vacations
next week.

> 
Cheers, 
Mauro.

Re: [PATCH] Fix __ucmpdi2 in v4l2_norm_to_name()

[Andrew Morton]

On Thu, 04 Jan 2007 20:59:08 -0200
Mauro Carvalho Chehab <mchehab@infradead.org> wrote:

> > The largest value we use here is 0x02000000.  Perhaps v4l2_std_id shouldn't
> > be 64-bit?
> Too late to change it to 32 bits. It is at V4L2 userspace API since
> kernel 2.6.0.

You could perhaps make it 32-bit internally, and still 64-bit on the
kernel<->userspace boundary.   64-bit quantities are expensive..

> We can, however use this approach as a workaround, with
> the proper documentation. I'll handle it after I return from vacations
> next week.

Thanks.

Re: [PATCH] Fix __ucmpdi2 in v4l2_norm_to_name()

[Mauro Carvalho Chehab]

Em Qui, 2007-01-04 às 15:18 -0800, Andrew Morton escreveu:
> On Thu, 04 Jan 2007 20:59:08 -0200
> Mauro Carvalho Chehab <mchehab@infradead.org> wrote:
> 
> > > The largest value we use here is 0x02000000.  Perhaps v4l2_std_id shouldn't
> > > be 64-bit?
> > Too late to change it to 32 bits. It is at V4L2 userspace API since
> > kernel 2.6.0.
> 
> You could perhaps make it 32-bit internally, and still 64-bit on the
> kernel<->userspace boundary.   64-bit quantities are expensive..
Hmm... there are some discussions currently on v4l ML about the need to
add some standards to support some digital streams, like those used on
webcams. Depending on the result of those discussions, we can need to
use more bits. So, I think it is not worth right now to replace video
std on every place it occurs.

I'm to just do the fix at v4l2-common.c.
> 
> > We can, however use this approach as a workaround, with
> > the proper documentation. I'll handle it after I return from vacations
> > next week.
Ok, I've wrote such patch. I should send today or tomorrow to Linus,
together with other patches.
> 
> Thanks.
Cheers, 
Mauro.

Re: [PATCH] Fix __ucmpdi2 in v4l2_norm_to_name()

[Stelian Pop]

Le dimanche 07 janvier 2007 à 09:44 -0200, Mauro Carvalho Chehab a
écrit :

> > > We can, however use this approach as a workaround, with
> > > the proper documentation. I'll handle it after I return from vacations
> > > next week.
> Ok, I've wrote such patch. I should send today or tomorrow to Linus,
> together with other patches.

Hi Mauro,

The __ucmpdi2() problem is still present in 2.6.20-rc5, please (re)send
your patch to Linus before 2.6.20 goes final...

Thanks.

-- 
Stelian Pop <stelian@popies.net>

Re: [PATCH] Fix __ucmpdi2 in v4l2_norm_to_name()

[Stelian Pop]

Le jeudi 04 janvier 2007 à 20:59 -0200, Mauro Carvalho Chehab a écrit :

> > The largest value we use here is 0x02000000.  Perhaps v4l2_std_id shouldn't
> > be 64-bit?
> Too late to change it to 32 bits. It is at V4L2 userspace API since
> kernel 2.6.0. We can, however use this approach as a workaround, with
> the proper documentation.

Maybe with a BUG_ON(id > UINT_MAX) ?

Linus, do you want a patch for this or will you take the _ucmppdi2 ppc
patch as per http://lkml.org/lkml/2006/12/17/46 instead ?

Thanks,

Stelian.
-- 
Stelian Pop <stelian@popies.net>

Re: [PATCH] Fix __ucmpdi2 in v4l2_norm_to_name()

[Segher Boessenkool]

>>> The largest value we use here is 0x02000000.  Perhaps v4l2_std_id 
>>> shouldn't
>>> be 64-bit?
>> Too late to change it to 32 bits. It is at V4L2 userspace API since
>> kernel 2.6.0. We can, however use this approach as a workaround, with
>> the proper documentation.
>
> Maybe with a BUG_ON(id > UINT_MAX) ?

If the code code is like

	u64 user_id;
	u32 kernel_id = user_id;

(or different types, just showing the difference in word
lengths here) it's easiest and safest to do

	BUG_ON(kernel_id != user_id);

i.e. cast back up to 64-bit, see if it's identical to the
original.  You won't have to worry about sign extensions or
similar that way, the BUG_ON() condition expresses the actual
requirement directly.


Segher