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

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

[Surendra Patil]

 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.

Signed-off-by: Surendra Patil <surendra.tux@gmail.com>
---
 drivers/bluetooth/ath3k.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/bluetooth/ath3k.c b/drivers/bluetooth/ath3k.c
index be571fe..5564207 100644
--- a/drivers/bluetooth/ath3k.c
+++ b/drivers/bluetooth/ath3k.c
@@ -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);
 
-- 
1.8.3.2

Re: [PATCH] drivers:bluetooth:ath3k.c Fixed sparse warning for cast to restricted __le32

[Marcel Holtmann]

Hi Surendra,

> 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.
> 
> Signed-off-by: Surendra Patil <surendra.tux@gmail.com>
> ---
> drivers/bluetooth/ath3k.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/bluetooth/ath3k.c b/drivers/bluetooth/ath3k.c
> index be571fe..5564207 100644
> --- a/drivers/bluetooth/ath3k.c
> +++ b/drivers/bluetooth/ath3k.c
> @@ -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.

> @@ -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

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

[Surendra Patil]

This patch fixes below Sparse warnings -
drivers/bluetooth/ath3k.c:370:17: warning: cast to restricted __le32
drivers/bluetooth/ath3k.c:432:17: warning: cast to restricted __le32

Signed-off-by: Surendra Patil <surendra.tux@gmail.com>
---
 drivers/bluetooth/ath3k.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/bluetooth/ath3k.c b/drivers/bluetooth/ath3k.c
index be571fe..badacbc 100644
--- a/drivers/bluetooth/ath3k.c
+++ b/drivers/bluetooth/ath3k.c
@@ -367,7 +367,7 @@ static int ath3k_load_patch(struct usb_device *udev)
 	}
 
 	snprintf(filename, ATH3K_NAME_LEN, "ar3k/AthrBT_0x%08x.dfu",
-		le32_to_cpu(fw_version.rom_version));
+		fw_version.rom_version);
 
 	ret = request_firmware(&firmware, filename, &udev->dev);
 	if (ret < 0) {
@@ -429,7 +429,7 @@ static int ath3k_load_syscfg(struct usb_device *udev)
 	}
 
 	snprintf(filename, ATH3K_NAME_LEN, "ar3k/ramps_0x%08x_%d%s",
-		le32_to_cpu(fw_version.rom_version), clk_value, ".dfu");
+		 fw_version.rom_version, clk_value, ".dfu");
 
 	ret = request_firmware(&firmware, filename, &udev->dev);
 	if (ret < 0) {
-- 
1.8.3.2

Re: [PATCH] drivers:bluetooth:ath3k.c: Fixed sparse warning for cast to restricted __le32

[Joe Perches]

On Tue, 2014-03-25 at 01:25 -0700, Surendra Patil wrote:
> This patch fixes below Sparse warnings -
> drivers/bluetooth/ath3k.c:370:17: warning: cast to restricted __le32
> drivers/bluetooth/ath3k.c:432:17: warning: cast to restricted __le32

You are changing output here.

I suspect rom_version should be marked __le32 instead
of unsigned int in "struct ath3k_version" instead.

> Signed-off-by: Surendra Patil <surendra.tux@gmail.com>
> ---
>  drivers/bluetooth/ath3k.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/bluetooth/ath3k.c b/drivers/bluetooth/ath3k.c
> index be571fe..badacbc 100644
> --- a/drivers/bluetooth/ath3k.c
> +++ b/drivers/bluetooth/ath3k.c
> @@ -367,7 +367,7 @@ static int ath3k_load_patch(struct usb_device *udev)
>  	}
>  
>  	snprintf(filename, ATH3K_NAME_LEN, "ar3k/AthrBT_0x%08x.dfu",
> -		le32_to_cpu(fw_version.rom_version));
> +		fw_version.rom_version);
>  
>  	ret = request_firmware(&firmware, filename, &udev->dev);
>  	if (ret < 0) {
> @@ -429,7 +429,7 @@ static int ath3k_load_syscfg(struct usb_device *udev)
>  	}
>  
>  	snprintf(filename, ATH3K_NAME_LEN, "ar3k/ramps_0x%08x_%d%s",
> -		le32_to_cpu(fw_version.rom_version), clk_value, ".dfu");
> +		 fw_version.rom_version, clk_value, ".dfu");
>  
>  	ret = request_firmware(&firmware, filename, &udev->dev);
>  	if (ret < 0) {

Re: [PATCH] drivers:bluetooth:ath3k.c: Fixed sparse warning for cast to restricted __le32

[Johan Hedberg]

Hi,

On Tue, Mar 25, 2014, Surendra Patil wrote:
> This patch fixes below Sparse warnings -
> drivers/bluetooth/ath3k.c:370:17: warning: cast to restricted __le32
> drivers/bluetooth/ath3k.c:432:17: warning: cast to restricted __le32
> 
> Signed-off-by: Surendra Patil <surendra.tux@gmail.com>
> ---
>  drivers/bluetooth/ath3k.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/bluetooth/ath3k.c b/drivers/bluetooth/ath3k.c
> index be571fe..badacbc 100644
> --- a/drivers/bluetooth/ath3k.c
> +++ b/drivers/bluetooth/ath3k.c
> @@ -367,7 +367,7 @@ static int ath3k_load_patch(struct usb_device *udev)
>  	}
>  
>  	snprintf(filename, ATH3K_NAME_LEN, "ar3k/AthrBT_0x%08x.dfu",
> -		le32_to_cpu(fw_version.rom_version));
> +		fw_version.rom_version);
>  
>  	ret = request_firmware(&firmware, filename, &udev->dev);
>  	if (ret < 0) {
> @@ -429,7 +429,7 @@ static int ath3k_load_syscfg(struct usb_device *udev)
>  	}
>  
>  	snprintf(filename, ATH3K_NAME_LEN, "ar3k/ramps_0x%08x_%d%s",
> -		le32_to_cpu(fw_version.rom_version), clk_value, ".dfu");
> +		 fw_version.rom_version, clk_value, ".dfu");
>  
>  	ret = request_firmware(&firmware, filename, &udev->dev);
>  	if (ret < 0) {

Are you sure this doesn't introduce a bug on big endian systems? We just
recently applied commit b9e2535acad8f52a17e2aa843d45a6b756b59592 which
adds this le32_to_cpu conversion. Probably the correct fix is to update
this fw_version struct definition to use __le32 for any member that's
expected to be in little endian format?

Johan