Skip to contentSkip to conversation

drivers:bluetooth:ath3k.c Fixed sparse warning for cast to restricted __le32

PATCH In review Surendra Patil 2 participants 3 messages Mar 26, 2014 Bluetooth drivers

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 drivers:bluetooth:ath3k.c Fixed sparse warning for cast to restricted __le32

To fix the sparse warning "cast to restricted __le32" marked "rom_version" to __le32 instead of unsigned int in "struct ath3k_version" and added cpu_to_le32() for the expression assigning int value to "rom_version". Successfully built the module without warnings and errors on x86 machine with sparse.

---
 drivers/bluetooth/ath3k.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/bluetooth/ath3k.c +2 -2
index be571fe..5564207 100644
@@ -50,7 +50,7 @@
 #define ATH3K_NAME_LEN				0xFF
 
 struct ath3k_version {
-	unsigned int	rom_version;
+	__le32		rom_version;
 	unsigned int	build_version;
 	unsigned int	ram_version;
 	unsigned char	ref_clock;
@@ -375,7 +375,7 @@ static int ath3k_load_patch(struct usb_device *udev)
 		return ret;
 	}
 
-	pt_version.rom_version = *(int *)(firmware->data + firmware->size - 8);
+	pt_version.rom_version = cpu_to_le32(*(int *)(firmware->data + firmware->size - 8));
 	pt_version.build_version = *(int *)
 		(firmware->data + firmware->size - 4);
 
Signature · 1 line
1.8.3.2
maintainer
#1 lore raw

Hi Surendra,

Quoted 15 lines

To fix the sparse warning "cast to restricted __le32" marked "rom_version" to __le32 instead of unsigned int in "struct ath3k_version" and added cpu_to_le32() for the expression assigning int value to "rom_version". Successfully built the module without warnings and errors on x86 machine with sparse.

---
drivers/bluetooth/ath3k.c | 4 ++--

1 file changed, 2 insertions(+), 2 deletions(-)
drivers/bluetooth/ath3k.c +0 -0
index be571fe..5564207 100644

drivers/bluetooth/ath3k.c line 56

Quoted 8 lines
drivers/bluetooth/ath3k.c +1 -1
@@ -50,7 +50,7 @@
 #define ATH3K_NAME_LEN				0xFF
 
 struct ath3k_version {
-	unsigned int	rom_version;
+	__le32		rom_version;
 	unsigned int	build_version;
 	unsigned int	ram_version;
 	unsigned char	ref_clock;

so I think this struct should be __packed in the first place and use strictly typed variables.

Seems like all fields have an endian issue.

drivers/bluetooth/ath3k.c static int ath3k_load_patch(struct usb_device *udev) · line 380

Quoted 8 lines
drivers/bluetooth/ath3k.c +1 -1
@@ -375,7 +375,7 @@ static int ath3k_load_patch(struct usb_device *udev)
 		return ret;
 	}
 
-	pt_version.rom_version = *(int *)(firmware->data + firmware->size - 8);
+	pt_version.rom_version = cpu_to_le32(*(int *)(firmware->data + firmware->size - 8));
 	pt_version.build_version = *(int *)
 		(firmware->data + firmware->size - 4);

I have no idea what this code is doing since that is just crazy. The variables are unsigned int and are now casted into int and not to mention that it is unaligned access. Has this code every been run on anything other than x86 at all. Does it happen to just work by pure luck.

Regards

Marcel

#2 lore raw

Thanks Marcel for the feedback,

In my previous patch joe/johan suggested me to make rom_version to __le32 , when i made changes i got an warning while compilation as " rom_version expects __le32 getting unsigned int " hence i added second changes cpu_to_32().

I am confused about what should be the proper change ? I have only built the module on x86 with sparse.

Quoted 55 lines · Marcel Holtmann

Hi Surendra,

Quoted 2 lines

To fix the sparse warning "cast to restricted __le32" marked "rom_version" to __le32 instead of unsigned int in "struct

ath3k_version"

Quoted 1 line

and added cpu_to_le32() for the expression assigning int value to

"rom_version".

Quoted 1 line

Successfully built the module without warnings and errors on x86 machine

with sparse.

Quoted 11 lines

---
drivers/bluetooth/ath3k.c | 4 ++--

1 file changed, 2 insertions(+), 2 deletions(-)
drivers/bluetooth/ath3k.c +0 -0
index be571fe..5564207 100644

drivers/bluetooth/ath3k.c line 56

Quoted 8 lines
drivers/bluetooth/ath3k.c +1 -1
@@ -50,7 +50,7 @@
 #define ATH3K_NAME_LEN                                0xFF
 
 struct ath3k_version {
-     unsigned int    rom_version;
+     __le32          rom_version;
      unsigned int    build_version;
      unsigned int    ram_version;
      unsigned char   ref_clock;

so I think this struct should be __packed in the first place and use strictly typed variables.

Seems like all fields have an endian issue.

drivers/bluetooth/ath3k.c static int ath3k_load_patch(struct usb_device *udev) · line 378

Quoted 5 lines
drivers/bluetooth/ath3k.c +0 -1
@@ -375,7 +375,7 @@ static int ath3k_load_patch(struct usb_device *udev)
              return ret;
      }
 
-     pt_version.rom_version = *(int *)(firmware->data + firmware->size

- 8);

drivers/bluetooth/ath3k.c static int ath3k_load_patch(struct usb_device *udev) · line 378

Quoted 1 line
drivers/bluetooth/ath3k.c +1 -0
+     pt_version.rom_version = cpu_to_le32(*(int *)(firmware->data +

firmware->size - 8));

drivers/bluetooth/ath3k.c static int ath3k_load_patch(struct usb_device *udev) · line 380

Quoted 2 lines
drivers/bluetooth/ath3k.c +0 -0
      pt_version.build_version = *(int *)
              (firmware->data + firmware->size - 4);

I have no idea what this code is doing since that is just crazy. The variables are unsigned int and are now casted into int and not to mention that it is unaligned access. Has this code every been run on anything other than x86 at all. Does it happen to just work by pure luck.

Regards

Marcel

Signature · 2 lines
Best,
Surendra Patil