[PATCH 0/3] ravb: small sparse fixes

[PATCH 0/3] ravb: small sparse fixes

[Niklas Söderlund]

Hi Sergei,

This are fixes that have bugged me whenever I run sparse to check my own 
changes to the driver. It's based on the latest net-next tree and tested 
on M3-N.

Niklas Söderlund (3):
  ravb: fix shadowing of symbol 'stats' in ravb_get_ethtool_stats()
  ravb: fix warning about memcpy length
  ravb: fix byte order for TX descriptor tag field lower bits

 drivers/net/ethernet/renesas/ravb_main.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

-- 
2.17.0

[PATCH 1/3] ravb: fix shadowing of symbol 'stats' in ravb_get_ethtool_stats()

[Niklas Söderlund]

Inside a loop in ravb_get_ethtool_stats() a variable 'stats' is declared
resulting in the argument also named 'stats' to be shadowed. Fix this
warning by renaming the unused argument 'stats' to 'estats'.

This fixes the sparse warning:

ravb_main.c:1225:36: originally declared here
ravb_main.c:1233:41: warning: symbol 'stats' shadows an earlier one

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(-)

diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
index 4a7f54c8e7aaad7e..e5784f0eac3df57c 100644
--- a/drivers/net/ethernet/renesas/ravb_main.c
+++ b/drivers/net/ethernet/renesas/ravb_main.c
@@ -1226,7 +1226,7 @@ static int ravb_get_sset_count(struct net_device *netdev, int sset)
 }
 
 static void ravb_get_ethtool_stats(struct net_device *ndev,
-				   struct ethtool_stats *stats, u64 *data)
+				   struct ethtool_stats *estats, u64 *data)
 {
 	struct ravb_private *priv = netdev_priv(ndev);
 	int i = 0;
-- 
2.17.0

[PATCH 2/3] ravb: fix warning about memcpy length

[Niklas Söderlund]

This fixes sparse warning:

ravb_main.c:1257 ravb_get_strings() error: memcpy() '*ravb_gstrings_stats' too small (32 vs 960)

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(-)

diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
index e5784f0eac3df57c..a100bccb3234bd5c 100644
--- a/drivers/net/ethernet/renesas/ravb_main.c
+++ b/drivers/net/ethernet/renesas/ravb_main.c
@@ -1258,7 +1258,7 @@ static void ravb_get_strings(struct net_device *ndev, u32 stringset, u8 *data)
 {
 	switch (stringset) {
 	case ETH_SS_STATS:
-		memcpy(data, *ravb_gstrings_stats, sizeof(ravb_gstrings_stats));
+		memcpy(data, ravb_gstrings_stats, sizeof(ravb_gstrings_stats));
 		break;
 	}
 }
-- 
2.17.0

[PATCH 3/3] ravb: fix byte order for TX descriptor tag field lower bits

[Niklas Söderlund]

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(-)

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);
-- 
2.17.0

Re: [PATCH 1/3] ravb: fix shadowing of symbol 'stats' in ravb_get_ethtool_stats()

[Geert Uytterhoeven]

On Mon, Jul 16, 2018 at 2:20 PM Niklas Söderlund
<niklas.soderlund+renesas@ragnatech.se> wrote:
> Inside a loop in ravb_get_ethtool_stats() a variable 'stats' is declared
> resulting in the argument also named 'stats' to be shadowed. Fix this
> warning by renaming the unused argument 'stats' to 'estats'.
>
> This fixes the sparse warning:
>
> ravb_main.c:1225:36: originally declared here
> ravb_main.c:1233:41: warning: symbol 'stats' shadows an earlier one
>
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 2/3] ravb: fix warning about memcpy length

[Geert Uytterhoeven]

On Mon, Jul 16, 2018 at 2:20 PM Niklas Söderlund
<niklas.soderlund+renesas@ragnatech.se> wrote:
> This fixes sparse warning:
>
> ravb_main.c:1257 ravb_get_strings() error: memcpy() '*ravb_gstrings_stats' too small (32 vs 960)
>
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 3/3] ravb: fix byte order for TX descriptor tag field lower bits

[Geert Uytterhoeven]

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

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 1/3] ravb: fix shadowing of symbol 'stats' in ravb_get_ethtool_stats()

[Sergei Shtylyov]

On 07/16/2018 03:19 PM, Niklas Söderlund wrote:

> Inside a loop in ravb_get_ethtool_stats() a variable 'stats' is declared
> resulting in the argument also named 'stats' to be shadowed. Fix this
> warning by renaming the unused argument 'stats' to 'estats'.
> 
> This fixes the sparse warning:
> 
> ravb_main.c:1225:36: originally declared here
> ravb_main.c:1233:41: warning: symbol 'stats' shadows an earlier one
> 
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
[...]

Acked-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>

MBR, Sergei

Re: [PATCH 2/3] ravb: fix warning about memcpy length

[Sergei Shtylyov]

On 07/16/2018 03:19 PM, Niklas Söderlund wrote:
> This fixes sparse warning:
> 
> ravb_main.c:1257 ravb_get_strings() error: memcpy() '*ravb_gstrings_stats' too small (32 vs 960)

   It's good that'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

Re: [PATCH 3/3] ravb: fix byte order for TX descriptor tag field lower bits

[Sergei Shtylyov]

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

Re: [PATCH 3/3] ravb: fix byte order for TX descriptor tag field lower bits

[Sergei Shtylyov]

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

Re: [PATCH 3/3] ravb: fix byte order for TX descriptor tag field lower bits

[Geert Uytterhoeven]

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

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 0/3] ravb: small sparse fixes

[David Miller]

From: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
Date: Mon, 16 Jul 2018 14:19:24 +0200

> This are fixes that have bugged me whenever I run sparse to check my own 
> changes to the driver. It's based on the latest net-next tree and tested 
> on M3-N.

Series applied, thanks Niklas.

Re: [PATCH 0/3] ravb: small sparse fixes

[David Miller]

From: Niklas S�derlund <niklas.soderlund+renesas@ragnatech.se>
Date: Mon, 16 Jul 2018 14:19:24 +0200

> This are fixes that have bugged me whenever I run sparse to check my own 
> changes to the driver. It's based on the latest net-next tree and tested 
> on M3-N.

Series applied, thanks Niklas.