| Message ID | 20230411091049.12998-3-mika.westerberg@linux.intel.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 5bbec0adfa03899cbf91ba99d4fdb4975d30a631 |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: thunderbolt: Fix for sparse warnings and typos | expand |
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 >
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).
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.
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.
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;