diff mbox series

[net-next,v3,2/2] net: thunderbolt: Use bitwise types in the struct thunderbolt_ip_frame_header

Message ID 20221130123613.20829-2-andriy.shevchenko@linux.intel.com (mailing list archive)
State Accepted
Commit a479f9264bddfd0c266615b83f784222eccf9b91
Delegated to: Netdev Maintainers
Headers show
Series [net-next,v3,1/2] net: thunderbolt: Switch from __maybe_unused to pm_sleep_ptr() etc | expand

Checks

Context Check Description
netdev/tree_selection success Clearly marked for net-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/subject_prefix success Link
netdev/cover_letter success Single patches do not need cover letters
netdev/patch_count success Link
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 18 this patch: 10
netdev/cc_maintainers success CCed 8 of 8 maintainers
netdev/build_clang success Errors and warnings before: 0 this patch: 0
netdev/module_param success Was 0 now: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 18 this patch: 10
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 14 lines checked
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Andy Shevchenko Nov. 30, 2022, 12:36 p.m. UTC
The main usage of the struct thunderbolt_ip_frame_header is to handle
the packets on the media layer. The header is bound to the protocol
in which the byte ordering is crucial. However the data type definition
doesn't use that and sparse is unhappy, for example (17 altogether):

  .../thunderbolt.c:718:23: warning: cast to restricted __le32

  .../thunderbolt.c:966:42: warning: incorrect type in assignment (different base types)
  .../thunderbolt.c:966:42:    expected unsigned int [usertype] frame_count
  .../thunderbolt.c:966:42:    got restricted __le32 [usertype]

Switch to the bitwise types in the struct thunderbolt_ip_frame_header to
reduce this, but not completely solving (9 left), because the same data
type is used for Rx header handled locally (in CPU byte order).

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
v3: no changes
v2: changed only types without splitting the data type (Mika)
 drivers/net/thunderbolt.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

Comments

Mika Westerberg Nov. 30, 2022, 12:58 p.m. UTC | #1
On Wed, Nov 30, 2022 at 02:36:13PM +0200, Andy Shevchenko wrote:
> The main usage of the struct thunderbolt_ip_frame_header is to handle
> the packets on the media layer. The header is bound to the protocol
> in which the byte ordering is crucial. However the data type definition
> doesn't use that and sparse is unhappy, for example (17 altogether):
> 
>   .../thunderbolt.c:718:23: warning: cast to restricted __le32
> 
>   .../thunderbolt.c:966:42: warning: incorrect type in assignment (different base types)
>   .../thunderbolt.c:966:42:    expected unsigned int [usertype] frame_count
>   .../thunderbolt.c:966:42:    got restricted __le32 [usertype]
> 
> Switch to the bitwise types in the struct thunderbolt_ip_frame_header to
> reduce this, but not completely solving (9 left), because the same data
> type is used for Rx header handled locally (in CPU byte order).
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Looks good to me. I assume you tested this against non-Linux OS to
ensure nothing broke? ;-)

Acked-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Andy Shevchenko Nov. 30, 2022, 2:11 p.m. UTC | #2
On Wed, Nov 30, 2022 at 02:58:31PM +0200, Mika Westerberg wrote:
> On Wed, Nov 30, 2022 at 02:36:13PM +0200, Andy Shevchenko wrote:
> > The main usage of the struct thunderbolt_ip_frame_header is to handle
> > the packets on the media layer. The header is bound to the protocol
> > in which the byte ordering is crucial. However the data type definition
> > doesn't use that and sparse is unhappy, for example (17 altogether):
> > 
> >   .../thunderbolt.c:718:23: warning: cast to restricted __le32
> > 
> >   .../thunderbolt.c:966:42: warning: incorrect type in assignment (different base types)
> >   .../thunderbolt.c:966:42:    expected unsigned int [usertype] frame_count
> >   .../thunderbolt.c:966:42:    got restricted __le32 [usertype]
> > 
> > Switch to the bitwise types in the struct thunderbolt_ip_frame_header to
> > reduce this, but not completely solving (9 left), because the same data
> > type is used for Rx header handled locally (in CPU byte order).
> > 
> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> 
> Looks good to me. I assume you tested this against non-Linux OS to
> ensure nothing broke? ;-)

Oh, no. It's compile tested only. And since we are using leXX_to_cpu() against
fields in that data structure I assume that it won't be any functional issue
with this. It's all about strict type checking.

> Acked-by: Mika Westerberg <mika.westerberg@linux.intel.com>

Thank you!
diff mbox series

Patch

diff --git a/drivers/net/thunderbolt.c b/drivers/net/thunderbolt.c
index c73d419f1456..4ed7f5b547e3 100644
--- a/drivers/net/thunderbolt.c
+++ b/drivers/net/thunderbolt.c
@@ -58,10 +58,10 @@ 
  * supported then @frame_id is filled, otherwise it stays %0.
  */
 struct thunderbolt_ip_frame_header {
-	u32 frame_size;
-	u16 frame_index;
-	u16 frame_id;
-	u32 frame_count;
+	__le32 frame_size;
+	__le16 frame_index;
+	__le16 frame_id;
+	__le32 frame_count;
 };
 
 enum thunderbolt_ip_frame_pdf {