| Message ID | 20180716121927.21918-4-niklas.soderlund+renesas@ragnatech.se |
|---|---|
| State | Accepted, archived |
| Delegated to: | David Miller |
| Headers | show |
| Series | ravb: small sparse fixes | expand |
On Mon, Jul 16, 2018 at 2:20 PM Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se> wrote: > The wrong helper is used to swap the bytes when adding the lower bits of > the TX descriptors tag field in the shared ds_tagl variable. The > variable contains the DS[11:0] field and then the TAG[3:0] bits. > > The mistake was highlighted by the sparse warning: > > ravb_main.c:1622:31: left side has type restricted __le16 > ravb_main.c:1622:31: right side has type unsigned short > ravb_main.c:1622:31: warning: invalid assignment: |= > ravb_main.c:1622:34: warning: cast to restricted __le16 > > Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> Gr{oetje,eeting}s, Geert
On 07/16/2018 03:19 PM, Niklas Söderlund wrote: > The wrong helper is used to swap the bytes when adding the lower bits of > the TX descriptors tag field in the shared ds_tagl variable. The > variable contains the DS[11:0] field and then the TAG[3:0] bits. > > The mistake was highlighted by the sparse warning: > > ravb_main.c:1622:31: left side has type restricted __le16 > ravb_main.c:1622:31: right side has type unsigned short > ravb_main.c:1622:31: warning: invalid assignment: |= > ravb_main.c:1622:34: warning: cast to restricted __le16 Again, it's good that it's not a real bug! :-) > Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se> [...] Acked-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> MBR, Sergei
On 07/16/2018 10:52 PM, Sergei Shtylyov wrote: >> The wrong helper is used to swap the bytes when adding the lower bits of >> the TX descriptors tag field in the shared ds_tagl variable. The >> variable contains the DS[11:0] field and then the TAG[3:0] bits. >> >> The mistake was highlighted by the sparse warning: >> >> ravb_main.c:1622:31: left side has type restricted __le16 >> ravb_main.c:1622:31: right side has type unsigned short >> ravb_main.c:1622:31: warning: invalid assignment: |= >> ravb_main.c:1622:34: warning: cast to restricted __le16 > > Again, it's good that it's not a real bug! :-) Wait! It seems to be, according to your subject. I hope you understand that cpu_to_le16() and le16_to_cpu() do the same thing? >> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se> > [...] > > Acked-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> MBR, Sergei
Hi Sergei, On Mon, Jul 16, 2018 at 10:22 PM Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> wrote: > On 07/16/2018 10:52 PM, Sergei Shtylyov wrote: > >> The wrong helper is used to swap the bytes when adding the lower bits of > >> the TX descriptors tag field in the shared ds_tagl variable. The > >> variable contains the DS[11:0] field and then the TAG[3:0] bits. > >> > >> The mistake was highlighted by the sparse warning: > >> > >> ravb_main.c:1622:31: left side has type restricted __le16 > >> ravb_main.c:1622:31: right side has type unsigned short > >> ravb_main.c:1622:31: warning: invalid assignment: |= > >> ravb_main.c:1622:34: warning: cast to restricted __le16 > > > > Again, it's good that it's not a real bug! :-) > > Wait! It seems to be, according to your subject. I hope you understand that Niklas' patch fixes the sparse warning. > cpu_to_le16() and le16_to_cpu() do the same thing? While they do the same thing, they don't take the same parameter types, nor return the same types. E.g. cpu_to_le16() takes a u16 and returns an __le16, le16_to_cpu() takes an __le16, and returns a u16. Gr{oetje,eeting}s, Geert
diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c index a100bccb3234bd5c..f7e649c831ad5595 100644 --- a/drivers/net/ethernet/renesas/ravb_main.c +++ b/drivers/net/ethernet/renesas/ravb_main.c @@ -1623,7 +1623,7 @@ static netdev_tx_t ravb_start_xmit(struct sk_buff *skb, struct net_device *ndev) /* TAG and timestamp required flag */ skb_shinfo(skb)->tx_flags |= SKBTX_IN_PROGRESS; desc->tagh_tsr = (ts_skb->tag >> 4) | TX_TSR; - desc->ds_tagl |= le16_to_cpu(ts_skb->tag << 12); + desc->ds_tagl |= cpu_to_le16(ts_skb->tag << 12); } skb_tx_timestamp(skb);
The wrong helper is used to swap the bytes when adding the lower bits of the TX descriptors tag field in the shared ds_tagl variable. The variable contains the DS[11:0] field and then the TAG[3:0] bits. The mistake was highlighted by the sparse warning: ravb_main.c:1622:31: left side has type restricted __le16 ravb_main.c:1622:31: right side has type unsigned short ravb_main.c:1622:31: warning: invalid assignment: |= ravb_main.c:1622:34: warning: cast to restricted __le16 Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se> --- drivers/net/ethernet/renesas/ravb_main.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)