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 <[email protected]>
---
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 <[email protected]>
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 <[email protected]>
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
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 <[email protected]>
>
> 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 <[email protected]>
On Thu, 04 Jan 2007 12:10:14 +0100
Stelian Pop <[email protected]> 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 <[email protected]>
>
> ---
>
> 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?
Em Qui, 2007-01-04 ?s 14:48 -0800, Andrew Morton escreveu:
> On Thu, 04 Jan 2007 12:10:14 +0100
> Stelian Pop <[email protected]> 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 <[email protected]>
> >
> > ---
> >
> > 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.
On Thu, 04 Jan 2007 20:59:08 -0200
Mauro Carvalho Chehab <[email protected]> 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.
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 <[email protected]>
>>> 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
Em Qui, 2007-01-04 ?s 15:18 -0800, Andrew Morton escreveu:
> On Thu, 04 Jan 2007 20:59:08 -0200
> Mauro Carvalho Chehab <[email protected]> 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.
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 <[email protected]>