Skip to contentSkip to conversation

V4L/DVB: s5p-fimc: dubious one-bit signed bitfields

PATCH In review Dan Carpenter 2 participants 3 messages Oct 21, 2010 – Oct 22, 2010

Subsystems (2)
Media (V4L/DVB) Kernel janitors
lore
Share & tools
lore

Dan Carpenter

author
#0 lore raw

PATCH 3/3 V4L/DVB: s5p-fimc: dubious one-bit signed bitfields

These are signed so instead of being 1 and 0 as intended they are -1 and
0. It doesn't cause a bug in the current code but Sparse warns about it:

drivers/media/video/s5p-fimc/fimc-core.h:226:28:
error: dubious one-bit signed bitfield

Signed-off-by Dan Carpenter <[email protected]>
drivers/media/video/s5p-fimc/fimc-core.h +4 -4
index e3a7c6a..7665a3f 100644
@@ -222,10 +223,10 @@ struct fimc_effect {
  * @real_height:	source pixel (height - offset)
  */
 struct fimc_scaler {
-	int	scaleup_h:1;
-	int	scaleup_v:1;
-	int	copy_mode:1;
-	int	enabled:1;
+	unsigned int	scaleup_h:1;
+	unsigned int	caleup_v:1;
+	unsigned int	copy_mode:1;
+	unsigned int	enabled:1;
 	u32	hfactor;
 	u32	vfactor;
 	u32	pre_hratio;
#1 lore raw
Quoted 29 lines

-----Original Message-----
From: Dan Carpenter [mailto:[email protected]]
Sent: Thursday, October 21, 2010 9:24 PM
To: Mauro Carvalho Chehab
Cc: Kyungmin Park; Sylwester Nawrocki; Pawel Osciak; Marek Szyprowski; [email protected]; [email protected]
Subject: [patch 3/3] V4L/DVB: s5p-fimc: dubious one-bit signed bitfields

These are signed so instead of being 1 and 0 as intended they are -1 and
0. It doesn't cause a bug in the current code but Sparse warns about it:

drivers/media/video/s5p-fimc/fimc-core.h:226:28:
error: dubious one-bit signed bitfield

Signed-off-by Dan Carpenter <[email protected]>
drivers/media/video/s5p-fimc/fimc-core.h +0 -1
index e3a7c6a..7665a3f 100644
@@ -222,10 +223,10 @@ struct fimc_effect {
  * @real_height:	source pixel (height - offset)
  */
 struct fimc_scaler {
-	int	scaleup_h:1;

drivers/media/video/s5p-fimc/fimc-core.h struct fimc_effect { · line 232

Quoted 10 lines
drivers/media/video/s5p-fimc/fimc-core.h +4 -3
-	int	scaleup_v:1;
-	int	copy_mode:1;
-	int	enabled:1;
+	unsigned int	scaleup_h:1;
+	unsigned int	caleup_v:1;
+	unsigned int	copy_mode:1;
+	unsigned int	enabled:1;
 	u32	hfactor;
 	u32	vfactor;
 	u32	pre_hratio;

In general I agree, however this patch would change scaleup_v:1 to caleup_v, so it cannot be applied in current form. Here is the corrected patch:

(quoted diff) +0 -0

b/drivers/media/video/s5p-fimc/fimc-core.h
index e3a7c6a..3e10785 100644

drivers/media/video/s5p-fimc/fimc-core.h +4 -4
@@ -222,10 +222,10 @@ struct fimc_effect {
  * @real_height:	source pixel (height - offset)
  */
 struct fimc_scaler {
-	int	scaleup_h:1;
-	int	scaleup_v:1;
-	int	copy_mode:1;
-	int	enabled:1;
+	unsigned int scaleup_h:1;
+	unsigned int scaleup_v:1;
+	unsigned int copy_mode:1;
+	unsigned int enabled:1;
 	u32	hfactor;
 	u32	vfactor;
 	u32	pre_hratio;
Signed-off-by Sylwester Nawrocki <[email protected]>

--
Sylwester Nawrocki
Linux Platform Group
Samsung Poland R&D Center

author
#2 lore raw
Quoted 34 lines · Sylwester Nawrocki
Quoted 34 lines

-----Original Message-----
From: Dan Carpenter [mailto:[email protected]]
Sent: Thursday, October 21, 2010 9:24 PM
To: Mauro Carvalho Chehab
Cc: Kyungmin Park; Sylwester Nawrocki; Pawel Osciak; Marek Szyprowski; [email protected]; [email protected]
Subject: [patch 3/3] V4L/DVB: s5p-fimc: dubious one-bit signed bitfields

These are signed so instead of being 1 and 0 as intended they are -1 and
0. It doesn't cause a bug in the current code but Sparse warns about it:

drivers/media/video/s5p-fimc/fimc-core.h:226:28:
error: dubious one-bit signed bitfield

Signed-off-by Dan Carpenter <[email protected]>
drivers/media/video/s5p-fimc/fimc-core.h +2 -4
index e3a7c6a..7665a3f 100644
@@ -222,10 +223,10 @@ struct fimc_effect {
  * @real_height:	source pixel (height - offset)
  */
 struct fimc_scaler {
-	int	scaleup_h:1;
-	int	scaleup_v:1;
-	int	copy_mode:1;
-	int	enabled:1;
+	unsigned int	scaleup_h:1;
+	unsigned int	caleup_v:1;
Quoted 8 lines

drivers/media/video/s5p-fimc/fimc-core.h struct fimc_effect { · line 231

Quoted 5 lines
drivers/media/video/s5p-fimc/fimc-core.h +2 -0
+	unsigned int	copy_mode:1;
+	unsigned int	enabled:1;
 	u32	hfactor;
 	u32	vfactor;
 	u32	pre_hratio;

In general I agree, however this patch would change scaleup_v:1 to caleup_v, so it cannot be applied in current form.

Crap! I suck. Thanks for catching that.

regards,
dan carpenter