lkml.org 
[lkml]   [2019]   [Apr]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v3 net-next 17/24] net: dsa: sja1105: Add support for ethtool port counters
Sat, Apr 13, 2019 at 03:28:15AM CEST, olteanv@gmail.com wrote:
>Signed-off-by: Vladimir Oltean <olteanv@gmail.com>
>Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
>---
>Changes in v3:
>None.
>
>Changes in v2:
>None functional. Moved the IS_ET() and IS_PQRS() device identification
>macros here since they are not used in earlier patches.
>
> drivers/net/dsa/sja1105/Makefile | 1 +
> drivers/net/dsa/sja1105/sja1105.h | 7 +-
> drivers/net/dsa/sja1105/sja1105_ethtool.c | 414 ++++++++++++++++++
> drivers/net/dsa/sja1105/sja1105_main.c | 3 +
> .../net/dsa/sja1105/sja1105_static_config.h | 21 +
> 5 files changed, 445 insertions(+), 1 deletion(-)
> create mode 100644 drivers/net/dsa/sja1105/sja1105_ethtool.c
>
>diff --git a/drivers/net/dsa/sja1105/Makefile b/drivers/net/dsa/sja1105/Makefile
>index ed00840802f4..bb4404c79eb2 100644
>--- a/drivers/net/dsa/sja1105/Makefile
>+++ b/drivers/net/dsa/sja1105/Makefile
>@@ -3,6 +3,7 @@ obj-$(CONFIG_NET_DSA_SJA1105) += sja1105.o
> sja1105-objs := \
> sja1105_spi.o \
> sja1105_main.o \
>+ sja1105_ethtool.o \
> sja1105_clocking.o \
> sja1105_static_config.o \
> sja1105_dynamic_config.o \
>diff --git a/drivers/net/dsa/sja1105/sja1105.h b/drivers/net/dsa/sja1105/sja1105.h
>index 4c9df44a4478..80b20bdd8f9c 100644
>--- a/drivers/net/dsa/sja1105/sja1105.h
>+++ b/drivers/net/dsa/sja1105/sja1105.h
>@@ -120,8 +120,13 @@ typedef enum {
> int sja1105_clocking_setup_port(struct sja1105_private *priv, int port);
> int sja1105_clocking_setup(struct sja1105_private *priv);
>
>-/* From sja1105_dynamic_config.c */
>+/* From sja1105_ethtool.c */
>+void sja1105_get_ethtool_stats(struct dsa_switch *ds, int port, u64 *data);
>+void sja1105_get_strings(struct dsa_switch *ds, int port,
>+ u32 stringset, u8 *data);
>+int sja1105_get_sset_count(struct dsa_switch *ds, int port, int sset);
>
>+/* From sja1105_dynamic_config.c */
> int sja1105_dynamic_config_read(struct sja1105_private *priv,
> enum sja1105_blk_idx blk_idx,
> int index, void *entry);
>diff --git a/drivers/net/dsa/sja1105/sja1105_ethtool.c b/drivers/net/dsa/sja1105/sja1105_ethtool.c
>new file mode 100644
>index 000000000000..c082599702bd
>--- /dev/null
>+++ b/drivers/net/dsa/sja1105/sja1105_ethtool.c
>@@ -0,0 +1,414 @@
>+// SPDX-License-Identifier: GPL-2.0
>+/* Copyright (c) 2018-2019, Vladimir Oltean <olteanv@gmail.com>
>+ */
>+#include "sja1105.h"
>+
>+#define SIZE_MAC_AREA (0x02 * 4)
>+#define SIZE_HL1_AREA (0x10 * 4)
>+#define SIZE_HL2_AREA (0x4 * 4)
>+#define SIZE_QLEVEL_AREA (0x8 * 4) /* 0x4 to 0xB */

Please use prefixes for defines like this. For example "SIZE_MAC_AREA"
sounds way too generic.

[...]


>+static void
>+sja1105_port_status_hl1_unpack(void *buf,
>+ struct sja1105_port_status_hl1 *status)
>+{
>+ /* Make pointer arithmetic work on 4 bytes */
>+ u32 *p = (u32 *)buf;

You don't need to cast void *. Please avoid it in the whole patchset.

[...]


>+ if (!IS_PQRS(priv->info->device_id))
>+ return;
>+
>+ memset(data + k, 0, ARRAY_SIZE(sja1105pqrs_extra_port_stats) *
>+ sizeof(u64));
>+ for (i = 0; i < 8; i++) {

Array size instead of "8"?


>+ data[k++] = status.hl2.qlevel_hwm[i];
>+ data[k++] = status.hl2.qlevel[i];
>+ }

[...]


>
>+#define IS_PQRS(device_id) \
>+ (((device_id) == SJA1105PR_DEVICE_ID) || \
>+ ((device_id) == SJA1105QS_DEVICE_ID))
>+#define IS_ET(device_id) \
>+ (((device_id) == SJA1105E_DEVICE_ID) || \
>+ ((device_id) == SJA1105T_DEVICE_ID))
>+/* P and R have same Device ID, and differ by Part Number */
>+#define IS_P(device_id, part_nr) \
>+ (((device_id) == SJA1105PR_DEVICE_ID) && \
>+ ((part_nr) == SJA1105P_PART_NR))
>+#define IS_R(device_id, part_nr) \
>+ (((device_id) == SJA1105PR_DEVICE_ID) && \
>+ ((part_nr) == SJA1105R_PART_NR))
>+/* Same do Q and S */
>+#define IS_Q(device_id, part_nr) \
>+ (((device_id) == SJA1105QS_DEVICE_ID) && \
>+ ((part_nr) == SJA1105Q_PART_NR))
>+#define IS_S(device_id, part_nr) \

Please have a prefix for macros like this. "IS_S" sounds way too
generic...


>+ (((device_id) == SJA1105QS_DEVICE_ID) && \
>+ ((part_nr) == SJA1105S_PART_NR))
>+
> struct sja1105_general_params_entry {
> u64 vllupformat;
> u64 mirr_ptacu;
>--
>2.17.1
>

\
 
 \ /
  Last update: 2019-04-13 22:53    [W:0.217 / U:2.296 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site