[PATCH v2 0/3] net: thunderbolt: Fix for sparse warnings and typos

[PATCH v2 0/3] net: thunderbolt: Fix for sparse warnings and typos

[Mika Westerberg]

Hi all,

This series tries to fix the rest of the sparse warnings generated
against the driver. While there fix the two typos in comments as well.

The previous version of the series can be found here:

  https://lore.kernel.org/netdev/20230404053636.51597-1-mika.westerberg@linux.intel.com/

Changes from the previous version:

  * Split the sparse fix into two patches
  * Fixed the other typo
  * Added tags from Simon and Andy.

Mika Westerberg (3):
  net: thunderbolt: Fix sparse warnings in tbnet_check_frame() and tbnet_poll()
  net: thunderbolt: Fix sparse warnings in tbnet_xmit_csum_and_map()
  net: thunderbolt: Fix typos in comments

 drivers/net/thunderbolt/main.c | 25 ++++++++++++++-----------
 1 file changed, 14 insertions(+), 11 deletions(-)

-- 
2.39.2

[PATCH v2 1/3] net: thunderbolt: Fix sparse warnings in tbnet_check_frame() and tbnet_poll()

[Mika Westerberg]

Fixes the following warnings when the driver is built with sparse
checks enabled:

main.c:767:47: warning: restricted __le32 degrades to integer
main.c:775:47: warning: restricted __le16 degrades to integer
main.c:776:44: warning: restricted __le16 degrades to integer
main.c:876:40: warning: incorrect type in assignment (different base types)
main.c:876:40:    expected restricted __le32 [usertype] frame_size
main.c:876:40:    got unsigned int [assigned] [usertype] frame_size
main.c:877:41: warning: incorrect type in assignment (different base types)
main.c:877:41:    expected restricted __le32 [usertype] frame_count
main.c:877:41:    got unsigned int [usertype]
main.c:878:41: warning: incorrect type in assignment (different base types)
main.c:878:41:    expected restricted __le16 [usertype] frame_index
main.c:878:41:    got unsigned short [usertype]
main.c:879:38: warning: incorrect type in assignment (different base types)
main.c:879:38:    expected restricted __le16 [usertype] frame_id
main.c:879:38:    got unsigned short [usertype]
main.c:880:62: warning: restricted __le32 degrades to integer
main.c:880:35: warning: restricted __le16 degrades to integer

No functional changes intended.

Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Reviewed-by: Simon Horman <simon.horman@corigine.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/net/thunderbolt/main.c | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/drivers/net/thunderbolt/main.c b/drivers/net/thunderbolt/main.c
index 26ef3706445e..27f8573a2b6e 100644
--- a/drivers/net/thunderbolt/main.c
+++ b/drivers/net/thunderbolt/main.c
@@ -764,7 +764,7 @@ static bool tbnet_check_frame(struct tbnet *net, const struct tbnet_frame *tf,
 	 */
 	if (net->skb && net->rx_hdr.frame_count) {
 		/* Check the frame count fits the count field */
-		if (frame_count != net->rx_hdr.frame_count) {
+		if (frame_count != le32_to_cpu(net->rx_hdr.frame_count)) {
 			net->stats.rx_length_errors++;
 			return false;
 		}
@@ -772,8 +772,8 @@ static bool tbnet_check_frame(struct tbnet *net, const struct tbnet_frame *tf,
 		/* Check the frame identifiers are incremented correctly,
 		 * and id is matching.
 		 */
-		if (frame_index != net->rx_hdr.frame_index + 1 ||
-		    frame_id != net->rx_hdr.frame_id) {
+		if (frame_index != le16_to_cpu(net->rx_hdr.frame_index) + 1 ||
+		    frame_id != le16_to_cpu(net->rx_hdr.frame_id)) {
 			net->stats.rx_missed_errors++;
 			return false;
 		}
@@ -873,11 +873,12 @@ static int tbnet_poll(struct napi_struct *napi, int budget)
 					TBNET_RX_PAGE_SIZE - hdr_size);
 		}
 
-		net->rx_hdr.frame_size = frame_size;
-		net->rx_hdr.frame_count = le32_to_cpu(hdr->frame_count);
-		net->rx_hdr.frame_index = le16_to_cpu(hdr->frame_index);
-		net->rx_hdr.frame_id = le16_to_cpu(hdr->frame_id);
-		last = net->rx_hdr.frame_index == net->rx_hdr.frame_count - 1;
+		net->rx_hdr.frame_size = hdr->frame_size;
+		net->rx_hdr.frame_count = hdr->frame_count;
+		net->rx_hdr.frame_index = hdr->frame_index;
+		net->rx_hdr.frame_id = hdr->frame_id;
+		last = le16_to_cpu(net->rx_hdr.frame_index) ==
+		       le32_to_cpu(net->rx_hdr.frame_count) - 1;
 
 		rx_packets++;
 		net->stats.rx_bytes += frame_size;
-- 
2.39.2

[PATCH v2 2/3] net: thunderbolt: Fix sparse warnings in tbnet_xmit_csum_and_map()

[Mika Westerberg]

Fixes the following warning when the driver is built with sparse checks
enabled:

main.c:993:23: warning: incorrect type in initializer (different base types)
main.c:993:23:    expected restricted __wsum [usertype] wsum
main.c:993:23:    got restricted __be32 [usertype]

No functional changes intended.

Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Reviewed-by: Simon Horman <simon.horman@corigine.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/net/thunderbolt/main.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/thunderbolt/main.c b/drivers/net/thunderbolt/main.c
index 27f8573a2b6e..6a43ced74881 100644
--- a/drivers/net/thunderbolt/main.c
+++ b/drivers/net/thunderbolt/main.c
@@ -991,8 +991,10 @@ static bool tbnet_xmit_csum_and_map(struct tbnet *net, struct sk_buff *skb,
 {
 	struct thunderbolt_ip_frame_header *hdr = page_address(frames[0]->page);
 	struct device *dma_dev = tb_ring_dma_device(net->tx_ring.ring);
-	__wsum wsum = htonl(skb->len - skb_transport_offset(skb));
 	unsigned int i, len, offset = skb_transport_offset(skb);
+	/* Remove payload length from checksum */
+	u32 paylen = skb->len - skb_transport_offset(skb);
+	__wsum wsum = (__force __wsum)htonl(paylen);
 	__be16 protocol = skb->protocol;
 	void *data = skb->data;
 	void *dest = hdr + 1;
-- 
2.39.2

Re: [PATCH v2 2/3] net: thunderbolt: Fix sparse warnings in tbnet_xmit_csum_and_map()

[Simon Horman]

On Tue, Apr 11, 2023 at 12:10:48PM +0300, Mika Westerberg wrote:
> Fixes the following warning when the driver is built with sparse checks
> enabled:
> 
> main.c:993:23: warning: incorrect type in initializer (different base types)
> main.c:993:23:    expected restricted __wsum [usertype] wsum
> main.c:993:23:    got restricted __be32 [usertype]
> 
> No functional changes intended.

This seems nice.

After you posted v1 I was wondering if, as a follow-up, it would be worth
creating a helper for this, say cpu_to_wsum(), as I think this pattern
occurs a few times. I'm thinking of a trivial wrapper around cpu_to_be32().

> 
> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> Reviewed-by: Simon Horman <simon.horman@corigine.com>
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/net/thunderbolt/main.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/thunderbolt/main.c b/drivers/net/thunderbolt/main.c
> index 27f8573a2b6e..6a43ced74881 100644
> --- a/drivers/net/thunderbolt/main.c
> +++ b/drivers/net/thunderbolt/main.c
> @@ -991,8 +991,10 @@ static bool tbnet_xmit_csum_and_map(struct tbnet *net, struct sk_buff *skb,
>  {
>  	struct thunderbolt_ip_frame_header *hdr = page_address(frames[0]->page);
>  	struct device *dma_dev = tb_ring_dma_device(net->tx_ring.ring);
> -	__wsum wsum = htonl(skb->len - skb_transport_offset(skb));
>  	unsigned int i, len, offset = skb_transport_offset(skb);
> +	/* Remove payload length from checksum */
> +	u32 paylen = skb->len - skb_transport_offset(skb);
> +	__wsum wsum = (__force __wsum)htonl(paylen);
>  	__be16 protocol = skb->protocol;
>  	void *data = skb->data;
>  	void *dest = hdr + 1;
> -- 
> 2.39.2
> 

Re: [PATCH v2 2/3] net: thunderbolt: Fix sparse warnings in tbnet_xmit_csum_and_map()

[Andy Shevchenko]

On Tue, Apr 11, 2023 at 01:50:16PM +0200, Simon Horman wrote:
> On Tue, Apr 11, 2023 at 12:10:48PM +0300, Mika Westerberg wrote:
> > Fixes the following warning when the driver is built with sparse checks
> > enabled:
> > 
> > main.c:993:23: warning: incorrect type in initializer (different base types)
> > main.c:993:23:    expected restricted __wsum [usertype] wsum
> > main.c:993:23:    got restricted __be32 [usertype]
> > 
> > No functional changes intended.
> 
> This seems nice.
> 
> After you posted v1 I was wondering if, as a follow-up, it would be worth
> creating a helper for this, say cpu_to_wsum(), as I think this pattern
> occurs a few times. I'm thinking of a trivial wrapper around cpu_to_be32().

But it looks like it makes sense to have a standalone series for that matter.
I.o.w. it doesn't belong to Thunderbolt (only).

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 2/3] net: thunderbolt: Fix sparse warnings in tbnet_xmit_csum_and_map()

[Simon Horman]

On Tue, Apr 11, 2023 at 02:58:21PM +0300, Andy Shevchenko wrote:
> On Tue, Apr 11, 2023 at 01:50:16PM +0200, Simon Horman wrote:
> > On Tue, Apr 11, 2023 at 12:10:48PM +0300, Mika Westerberg wrote:
> > > Fixes the following warning when the driver is built with sparse checks
> > > enabled:
> > > 
> > > main.c:993:23: warning: incorrect type in initializer (different base types)
> > > main.c:993:23:    expected restricted __wsum [usertype] wsum
> > > main.c:993:23:    got restricted __be32 [usertype]
> > > 
> > > No functional changes intended.
> > 
> > This seems nice.
> > 
> > After you posted v1 I was wondering if, as a follow-up, it would be worth
> > creating a helper for this, say cpu_to_wsum(), as I think this pattern
> > occurs a few times. I'm thinking of a trivial wrapper around cpu_to_be32().
> 
> But it looks like it makes sense to have a standalone series for that matter.
> I.o.w. it doesn't belong to Thunderbolt (only).

Yes, agreed.

I was more asking if it is a good idea than for any changes to this patchset.

Re: [PATCH v2 2/3] net: thunderbolt: Fix sparse warnings in tbnet_xmit_csum_and_map()

[Mika Westerberg]

On Tue, Apr 11, 2023 at 02:37:58PM +0200, Simon Horman wrote:
> On Tue, Apr 11, 2023 at 02:58:21PM +0300, Andy Shevchenko wrote:
> > On Tue, Apr 11, 2023 at 01:50:16PM +0200, Simon Horman wrote:
> > > On Tue, Apr 11, 2023 at 12:10:48PM +0300, Mika Westerberg wrote:
> > > > Fixes the following warning when the driver is built with sparse checks
> > > > enabled:
> > > > 
> > > > main.c:993:23: warning: incorrect type in initializer (different base types)
> > > > main.c:993:23:    expected restricted __wsum [usertype] wsum
> > > > main.c:993:23:    got restricted __be32 [usertype]
> > > > 
> > > > No functional changes intended.
> > > 
> > > This seems nice.
> > > 
> > > After you posted v1 I was wondering if, as a follow-up, it would be worth
> > > creating a helper for this, say cpu_to_wsum(), as I think this pattern
> > > occurs a few times. I'm thinking of a trivial wrapper around cpu_to_be32().
> > 
> > But it looks like it makes sense to have a standalone series for that matter.
> > I.o.w. it doesn't belong to Thunderbolt (only).
> 
> Yes, agreed.
> 
> I was more asking if it is a good idea than for any changes to this patchset.

I think it is a good idea :) I'll add this to my todo list and will do
at some point if nobody else have already done it.

[PATCH v2 3/3] net: thunderbolt: Fix typos in comments

[Mika Westerberg]

Fix two typos in comments:

  blongs -> belongs
  UPD -> UDP

No functional changes.

Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Reviewed-by: Simon Horman <simon.horman@corigine.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/net/thunderbolt/main.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/thunderbolt/main.c b/drivers/net/thunderbolt/main.c
index 6a43ced74881..0c1e8970ee58 100644
--- a/drivers/net/thunderbolt/main.c
+++ b/drivers/net/thunderbolt/main.c
@@ -148,7 +148,7 @@ struct tbnet_ring {
 /**
  * struct tbnet - ThunderboltIP network driver private data
  * @svc: XDomain service the driver is bound to
- * @xd: XDomain the service blongs to
+ * @xd: XDomain the service belongs to
  * @handler: ThunderboltIP configuration protocol handler
  * @dev: Networking device
  * @napi: NAPI structure for Rx polling
@@ -1030,7 +1030,7 @@ static bool tbnet_xmit_csum_and_map(struct tbnet *net, struct sk_buff *skb,
 	/* Data points on the beginning of packet.
 	 * Check is the checksum absolute place in the packet.
 	 * ipcso will update IP checksum.
-	 * tucso will update TCP/UPD checksum.
+	 * tucso will update TCP/UDP checksum.
 	 */
 	if (protocol == htons(ETH_P_IP)) {
 		__sum16 *ipcso = dest + ((void *)&(ip_hdr(skb)->check) - data);
-- 
2.39.2

Re: [PATCH v2 0/3] net: thunderbolt: Fix for sparse warnings and typos

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Tue, 11 Apr 2023 12:10:46 +0300 you wrote:
> Hi all,
> 
> This series tries to fix the rest of the sparse warnings generated
> against the driver. While there fix the two typos in comments as well.
> 
> The previous version of the series can be found here:
> 
> [...]

Here is the summary with links:
  - [v2,1/3] net: thunderbolt: Fix sparse warnings in tbnet_check_frame() and tbnet_poll()
    https://git.kernel.org/netdev/net-next/c/185367221503
  - [v2,2/3] net: thunderbolt: Fix sparse warnings in tbnet_xmit_csum_and_map()
    https://git.kernel.org/netdev/net-next/c/5bbec0adfa03
  - [v2,3/3] net: thunderbolt: Fix typos in comments
    https://git.kernel.org/netdev/net-next/c/9c60f2a4446c

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html