All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] net: thunderbolt: Fix for sparse warnings and typo
@ 2023-04-04  5:36 Mika Westerberg
  2023-04-04  5:36 ` [PATCH 1/2] net: thunderbolt: Fix sparse warnings Mika Westerberg
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Mika Westerberg @ 2023-04-04  5:36 UTC (permalink / raw)
  To: David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni
  Cc: Michael Jamet, Yehezkel Bernat, Andy Shevchenko, Mika Westerberg, netdev

Hi,

The first patch should get rid of the sparse reported warnings that
still exist in the driver. The second one is trivial fix for typo.

Mika Westerberg (2):
  net: thunderbolt: Fix sparse warnings
  net: thunderbolt: Fix typo in comment

 drivers/net/thunderbolt/main.c | 23 +++++++++++++----------
 1 file changed, 13 insertions(+), 10 deletions(-)

-- 
2.39.2


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 1/2] net: thunderbolt: Fix sparse warnings
  2023-04-04  5:36 [PATCH 0/2] net: thunderbolt: Fix for sparse warnings and typo Mika Westerberg
@ 2023-04-04  5:36 ` Mika Westerberg
  2023-04-04 19:37   ` Simon Horman
  2023-04-05  8:45   ` Andy Shevchenko
  2023-04-04  5:36 ` [PATCH 2/2] net: thunderbolt: Fix typo in comment Mika Westerberg
  2023-04-05  8:46 ` [PATCH 0/2] net: thunderbolt: Fix for sparse warnings and typo Andy Shevchenko
  2 siblings, 2 replies; 10+ messages in thread
From: Mika Westerberg @ 2023-04-04  5:36 UTC (permalink / raw)
  To: David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni
  Cc: Michael Jamet, Yehezkel Bernat, Andy Shevchenko, Mika Westerberg, netdev

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

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

No functional changes intended.

Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
---
 drivers/net/thunderbolt/main.c | 21 ++++++++++++---------
 1 file changed, 12 insertions(+), 9 deletions(-)

diff --git a/drivers/net/thunderbolt/main.c b/drivers/net/thunderbolt/main.c
index 26ef3706445e..6a43ced74881 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;
@@ -990,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


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 2/2] net: thunderbolt: Fix typo in comment
  2023-04-04  5:36 [PATCH 0/2] net: thunderbolt: Fix for sparse warnings and typo Mika Westerberg
  2023-04-04  5:36 ` [PATCH 1/2] net: thunderbolt: Fix sparse warnings Mika Westerberg
@ 2023-04-04  5:36 ` Mika Westerberg
  2023-04-04 19:25   ` Simon Horman
  2023-04-05  8:46 ` [PATCH 0/2] net: thunderbolt: Fix for sparse warnings and typo Andy Shevchenko
  2 siblings, 1 reply; 10+ messages in thread
From: Mika Westerberg @ 2023-04-04  5:36 UTC (permalink / raw)
  To: David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni
  Cc: Michael Jamet, Yehezkel Bernat, Andy Shevchenko, Mika Westerberg, netdev

Should be UDP not UPD. No functional changes.

Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
---
 drivers/net/thunderbolt/main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/thunderbolt/main.c b/drivers/net/thunderbolt/main.c
index 6a43ced74881..0ce501e34f3f 100644
--- a/drivers/net/thunderbolt/main.c
+++ b/drivers/net/thunderbolt/main.c
@@ -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


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/2] net: thunderbolt: Fix typo in comment
  2023-04-04  5:36 ` [PATCH 2/2] net: thunderbolt: Fix typo in comment Mika Westerberg
@ 2023-04-04 19:25   ` Simon Horman
  0 siblings, 0 replies; 10+ messages in thread
From: Simon Horman @ 2023-04-04 19:25 UTC (permalink / raw)
  To: Mika Westerberg
  Cc: David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Michael Jamet, Yehezkel Bernat, Andy Shevchenko, netdev

On Tue, Apr 04, 2023 at 08:36:36AM +0300, Mika Westerberg wrote:
> Should be UDP not UPD. No functional changes.
> 
> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>

Looks good,

Reviewed-by: Simon Horman <simon.horman@corigine.com>

codespell tells me you also may want to consider:

$ codespell drivers/net/thunderbolt/main.c
drivers/net/thunderbolt/main.c:151: blongs ==> belongs

> ---
>  drivers/net/thunderbolt/main.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/thunderbolt/main.c b/drivers/net/thunderbolt/main.c
> index 6a43ced74881..0ce501e34f3f 100644
> --- a/drivers/net/thunderbolt/main.c
> +++ b/drivers/net/thunderbolt/main.c
> @@ -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
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/2] net: thunderbolt: Fix sparse warnings
  2023-04-04  5:36 ` [PATCH 1/2] net: thunderbolt: Fix sparse warnings Mika Westerberg
@ 2023-04-04 19:37   ` Simon Horman
  2023-04-05  8:45   ` Andy Shevchenko
  1 sibling, 0 replies; 10+ messages in thread
From: Simon Horman @ 2023-04-04 19:37 UTC (permalink / raw)
  To: Mika Westerberg
  Cc: David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Michael Jamet, Yehezkel Bernat, Andy Shevchenko, netdev

On Tue, Apr 04, 2023 at 08:36:35AM +0300, Mika Westerberg wrote:
> Fixes the following warnings when the driver is built with sparse
> checks enabled:
> 
> drivers/net/thunderbolt/main.c:767:47: warning: restricted __le32 degrades to integer
> drivers/net/thunderbolt/main.c:775:47: warning: restricted __le16 degrades to integer
> drivers/net/thunderbolt/main.c:776:44: warning: restricted __le16 degrades to integer
> drivers/net/thunderbolt/main.c:876:40: warning: incorrect type in assignment (different base types)
> drivers/net/thunderbolt/main.c:876:40:    expected restricted __le32 [usertype] frame_size
> drivers/net/thunderbolt/main.c:876:40:    got unsigned int [assigned] [usertype] frame_size
> drivers/net/thunderbolt/main.c:877:41: warning: incorrect type in assignment (different base types)
> drivers/net/thunderbolt/main.c:877:41:    expected restricted __le32 [usertype] frame_count
> drivers/net/thunderbolt/main.c:877:41:    got unsigned int [usertype]
> drivers/net/thunderbolt/main.c:878:41: warning: incorrect type in assignment (different base types)
> drivers/net/thunderbolt/main.c:878:41:    expected restricted __le16 [usertype] frame_index
> drivers/net/thunderbolt/main.c:878:41:    got unsigned short [usertype]
> drivers/net/thunderbolt/main.c:879:38: warning: incorrect type in assignment (different base types)
> drivers/net/thunderbolt/main.c:879:38:    expected restricted __le16 [usertype] frame_id
> drivers/net/thunderbolt/main.c:879:38:    got unsigned short [usertype]
> drivers/net/thunderbolt/main.c:880:62: warning: restricted __le32 degrades to integer
> drivers/net/thunderbolt/main.c:880:35: warning: restricted __le16 degrades to integer
> drivers/net/thunderbolt/main.c:993:23: warning: incorrect type in initializer (different base types)
> drivers/net/thunderbolt/main.c:993:23:    expected restricted __wsum [usertype] wsum
> drivers/net/thunderbolt/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>


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/2] net: thunderbolt: Fix sparse warnings
  2023-04-04  5:36 ` [PATCH 1/2] net: thunderbolt: Fix sparse warnings Mika Westerberg
  2023-04-04 19:37   ` Simon Horman
@ 2023-04-05  8:45   ` Andy Shevchenko
  2023-04-05  9:52     ` Mika Westerberg
  1 sibling, 1 reply; 10+ messages in thread
From: Andy Shevchenko @ 2023-04-05  8:45 UTC (permalink / raw)
  To: Mika Westerberg
  Cc: David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Michael Jamet, Yehezkel Bernat, netdev

On Tue, Apr 04, 2023 at 08:36:35AM +0300, Mika Westerberg wrote:
> Fixes the following warnings when the driver is built with sparse
> checks enabled:

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

You can drop the whole part with file name and line numbers to make the above
neater.

> No functional changes intended.
> 
> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> ---
>  drivers/net/thunderbolt/main.c | 21 ++++++++++++---------
>  1 file changed, 12 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/net/thunderbolt/main.c b/drivers/net/thunderbolt/main.c
> index 26ef3706445e..6a43ced74881 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;
> @@ -990,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;

I would split wsum fix from the above as they are of different nature.

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 0/2] net: thunderbolt: Fix for sparse warnings and typo
  2023-04-04  5:36 [PATCH 0/2] net: thunderbolt: Fix for sparse warnings and typo Mika Westerberg
  2023-04-04  5:36 ` [PATCH 1/2] net: thunderbolt: Fix sparse warnings Mika Westerberg
  2023-04-04  5:36 ` [PATCH 2/2] net: thunderbolt: Fix typo in comment Mika Westerberg
@ 2023-04-05  8:46 ` Andy Shevchenko
  2 siblings, 0 replies; 10+ messages in thread
From: Andy Shevchenko @ 2023-04-05  8:46 UTC (permalink / raw)
  To: Mika Westerberg
  Cc: David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Michael Jamet, Yehezkel Bernat, netdev

On Tue, Apr 04, 2023 at 08:36:34AM +0300, Mika Westerberg wrote:
> Hi,
> 
> The first patch should get rid of the sparse reported warnings that
> still exist in the driver. The second one is trivial fix for typo.

Thank you, I believe v2 is needed which will look good with addressed
Simon's and my comments. Taking this into account,
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>


> Mika Westerberg (2):
>   net: thunderbolt: Fix sparse warnings
>   net: thunderbolt: Fix typo in comment

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/2] net: thunderbolt: Fix sparse warnings
  2023-04-05  8:45   ` Andy Shevchenko
@ 2023-04-05  9:52     ` Mika Westerberg
  2023-04-05 10:42       ` Andy Shevchenko
  0 siblings, 1 reply; 10+ messages in thread
From: Mika Westerberg @ 2023-04-05  9:52 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Michael Jamet, Yehezkel Bernat, netdev

Hi Andy,

On Wed, Apr 05, 2023 at 11:45:43AM +0300, Andy Shevchenko wrote:
> On Tue, Apr 04, 2023 at 08:36:35AM +0300, Mika Westerberg wrote:
> > Fixes the following warnings when the driver is built with sparse
> > checks enabled:
> 
> > drivers/net/thunderbolt/main.c:767:47: warning: restricted __le32 degrades to integer
> > drivers/net/thunderbolt/main.c:775:47: warning: restricted __le16 degrades to integer
> > drivers/net/thunderbolt/main.c:776:44: warning: restricted __le16 degrades to integer
> > drivers/net/thunderbolt/main.c:876:40: warning: incorrect type in assignment (different base types)
> > drivers/net/thunderbolt/main.c:876:40:    expected restricted __le32 [usertype] frame_size
> > drivers/net/thunderbolt/main.c:876:40:    got unsigned int [assigned] [usertype] frame_size
> > drivers/net/thunderbolt/main.c:877:41: warning: incorrect type in assignment (different base types)
> > drivers/net/thunderbolt/main.c:877:41:    expected restricted __le32 [usertype] frame_count
> > drivers/net/thunderbolt/main.c:877:41:    got unsigned int [usertype]
> > drivers/net/thunderbolt/main.c:878:41: warning: incorrect type in assignment (different base types)
> > drivers/net/thunderbolt/main.c:878:41:    expected restricted __le16 [usertype] frame_index
> > drivers/net/thunderbolt/main.c:878:41:    got unsigned short [usertype]
> > drivers/net/thunderbolt/main.c:879:38: warning: incorrect type in assignment (different base types)
> > drivers/net/thunderbolt/main.c:879:38:    expected restricted __le16 [usertype] frame_id
> > drivers/net/thunderbolt/main.c:879:38:    got unsigned short [usertype]
> > drivers/net/thunderbolt/main.c:880:62: warning: restricted __le32 degrades to integer
> > drivers/net/thunderbolt/main.c:880:35: warning: restricted __le16 degrades to integer
> > drivers/net/thunderbolt/main.c:993:23: warning: incorrect type in initializer (different base types)
> > drivers/net/thunderbolt/main.c:993:23:    expected restricted __wsum [usertype] wsum
> > drivers/net/thunderbolt/main.c:993:23:    got restricted __be32 [usertype]
> 
> You can drop the whole part with file name and line numbers to make the above
> neater.

I guess it is good to leave the filename, there like this, no?

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>
> > ---
> >  drivers/net/thunderbolt/main.c | 21 ++++++++++++---------
> >  1 file changed, 12 insertions(+), 9 deletions(-)
> > 
> > diff --git a/drivers/net/thunderbolt/main.c b/drivers/net/thunderbolt/main.c
> > index 26ef3706445e..6a43ced74881 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;
> > @@ -990,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;
> 
> I would split wsum fix from the above as they are of different nature.

How they are different? The complain is pretty much the same for all
these AFAICT:

expected restricted xxx [usertype] yyy
got restricted zzz [usertype]

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/2] net: thunderbolt: Fix sparse warnings
  2023-04-05  9:52     ` Mika Westerberg
@ 2023-04-05 10:42       ` Andy Shevchenko
  2023-04-05 12:28         ` Mika Westerberg
  0 siblings, 1 reply; 10+ messages in thread
From: Andy Shevchenko @ 2023-04-05 10:42 UTC (permalink / raw)
  To: Mika Westerberg
  Cc: David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Michael Jamet, Yehezkel Bernat, netdev

On Wed, Apr 05, 2023 at 12:52:24PM +0300, Mika Westerberg wrote:
> On Wed, Apr 05, 2023 at 11:45:43AM +0300, Andy Shevchenko wrote:
> > On Tue, Apr 04, 2023 at 08:36:35AM +0300, Mika Westerberg wrote:

...

> > > drivers/net/thunderbolt/main.c:993:23:    expected restricted __wsum [usertype] wsum
> > > drivers/net/thunderbolt/main.c:993:23:    got restricted __be32 [usertype]
> > 
> > You can drop the whole part with file name and line numbers to make the above
> > neater.
> 
> I guess it is good to leave the filename, there like this, no?
> 
> main.c:993:23:    expected restricted __wsum [usertype] wsum
> main.c:993:23:    got restricted __be32 [usertype]

Fine by me.

...

> > > +		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;

...

> > > -	__wsum wsum = htonl(skb->len - skb_transport_offset(skb));
> > > +	/* Remove payload length from checksum */
> > > +	u32 paylen = skb->len - skb_transport_offset(skb);
> > > +	__wsum wsum = (__force __wsum)htonl(paylen);

> > I would split wsum fix from the above as they are of different nature.
> 
> How they are different? The complain is pretty much the same for all
> these AFAICT:
> 
> expected restricted xxx [usertype] yyy
> got restricted zzz [usertype]

While the main part is about header data type and endianess conversion between
protocol and CPU (with cpu_*() and *_cpu() macros) this one is completely network
related stuff as it's using hton*() and ntoh*() conversion macros. Yes, underneeth
it may be the same, but semantically these two parts are not of the same thing.

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/2] net: thunderbolt: Fix sparse warnings
  2023-04-05 10:42       ` Andy Shevchenko
@ 2023-04-05 12:28         ` Mika Westerberg
  0 siblings, 0 replies; 10+ messages in thread
From: Mika Westerberg @ 2023-04-05 12:28 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Michael Jamet, Yehezkel Bernat, netdev

On Wed, Apr 05, 2023 at 01:42:08PM +0300, Andy Shevchenko wrote:
> > How they are different? The complain is pretty much the same for all
> > these AFAICT:
> > 
> > expected restricted xxx [usertype] yyy
> > got restricted zzz [usertype]
> 
> While the main part is about header data type and endianess conversion between
> protocol and CPU (with cpu_*() and *_cpu() macros) this one is completely network
> related stuff as it's using hton*() and ntoh*() conversion macros. Yes, underneeth
> it may be the same, but semantically these two parts are not of the same thing.

Okay, fair enough :) I'll make this split in v2.

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2023-04-05 12:28 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-04-04  5:36 [PATCH 0/2] net: thunderbolt: Fix for sparse warnings and typo Mika Westerberg
2023-04-04  5:36 ` [PATCH 1/2] net: thunderbolt: Fix sparse warnings Mika Westerberg
2023-04-04 19:37   ` Simon Horman
2023-04-05  8:45   ` Andy Shevchenko
2023-04-05  9:52     ` Mika Westerberg
2023-04-05 10:42       ` Andy Shevchenko
2023-04-05 12:28         ` Mika Westerberg
2023-04-04  5:36 ` [PATCH 2/2] net: thunderbolt: Fix typo in comment Mika Westerberg
2023-04-04 19:25   ` Simon Horman
2023-04-05  8:46 ` [PATCH 0/2] net: thunderbolt: Fix for sparse warnings and typo Andy Shevchenko

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.