* [PATCH iproute2-next 3/9] rdma: Add filtering infrastructure
From: Leon Romanovsky @ 2018-01-02 9:37 UTC (permalink / raw)
To: Doug Ledford, Jason Gunthorpe, David Ahern
Cc: RDMA mailing list, Leon Romanovsky, netdev, Stephen Hemminger
In-Reply-To: <20180102093725.6172-1-leon@kernel.org>
From: Leon Romanovsky <leonro@mellanox.com>
This patch adds general infrastructure to RDMAtool to handle various
filtering options needed for the downstream resource tracking patches.
The infrastructure is generic and stores filters in list of key<->value
entries. There are three types of filters:
1. Numeric - the values are intended to be digits combined with '-' to
mark range and ',' to mark multiple entries, e.g. pid 1-100,234,400-401
is perfectly legit filter to limit process ids.
2. String - the values are consist from strings and "," as a
denominator. Currently only "display" option is opened for the users and
it will be used to hide/unhide table columns in resource tracking
patches.
3. Link - special case to allow '/' in string to provide link name, e.g.
link mlx4_1/2.
Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
---
rdma/rdma.c | 1 +
rdma/rdma.h | 15 +++++
rdma/utils.c | 212 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 228 insertions(+)
diff --git a/rdma/rdma.c b/rdma/rdma.c
index 0e8fd688..a21ba440 100644
--- a/rdma/rdma.c
+++ b/rdma/rdma.c
@@ -47,6 +47,7 @@ static int rd_init(struct rd *rd, int argc, char **argv, char *filename)
rd->argc = argc;
rd->argv = argv;
INIT_LIST_HEAD(&rd->dev_map_list);
+ INIT_LIST_HEAD(&rd->filter_list);
if (rd->json_output) {
rd->jw = jsonw_new(stdout);
diff --git a/rdma/rdma.h b/rdma/rdma.h
index 1b66ae04..cd415670 100644
--- a/rdma/rdma.h
+++ b/rdma/rdma.h
@@ -29,6 +29,12 @@
#define RDMA_BITMAP_ENUM(name, bit_no) RDMA_BITMAP_##name = BIT(bit_no),
#define RDMA_BITMAP_NAMES(name, bit_no) [bit_no] = #name,
+struct filter_entry {
+ struct list_head list;
+ char *key;
+ char *value;
+};
+
struct dev_map {
struct list_head list;
char *dev_name;
@@ -50,6 +56,7 @@ struct rd {
json_writer_t *jw;
bool json_output;
bool pretty_output;
+ struct list_head filter_list;
};
struct rd_cmd {
@@ -81,6 +88,14 @@ int rd_argc(struct rd *rd);
*/
struct dev_map *dev_map_lookup(struct rd *rd, bool allow_port_index);
+/*
+ * Filter manipulation
+ */
+int rd_build_filter(struct rd *rd, const char * const valid_filters[]);
+bool rd_check_is_filtered(struct rd *rd, const char *key,
+ uint32_t val, bool ignore_val);
+bool rd_check_is_string_filtered(struct rd *rd, const char *key, char *val);
+bool rd_check_is_key_exist(struct rd *rd, const char *key);
/*
* Netlink
*/
diff --git a/rdma/utils.c b/rdma/utils.c
index 9184f455..dc9f4f01 100644
--- a/rdma/utils.c
+++ b/rdma/utils.c
@@ -114,6 +114,217 @@ static void dev_map_cleanup(struct rd *rd)
}
}
+static int add_filter(struct rd *rd, char *key, char *value,
+ const char * const valid_filters[])
+{
+ char cset[] = "1234567890,-";
+ struct filter_entry *fe;
+ bool key_found = false;
+ int idx = 0;
+ int ret;
+
+ fe = calloc(1, sizeof(*fe));
+ if (!fe)
+ return -ENOMEM;
+
+ while (valid_filters[idx]) {
+ if (!strcmpx(key, valid_filters[idx])) {
+ key_found = true;
+ break;
+ }
+ idx++;
+ }
+ if (!key_found) {
+ pr_err("Unsupported filter option: %s\n", key);
+ ret = -EINVAL;
+ goto err;
+ }
+
+ /*
+ * Check the filter validity, not optimal, but works
+ *
+ * Actually, there are three types of filters
+ * numeric - for example PID or QPN
+ * string - for example states, currently only "display"
+ * is from this type
+ * link - user requested to filter on specific link
+ * e.g. mlx5_1/1, mlx5_1/-, mlx5_1 ...
+ */
+ if (strcmpx(key, "link") && strcmpx(key, "display") &&
+ strspn(value, cset) != strlen(value)) {
+ pr_err("%s filter accepts \"%s\" characters only\n", key, cset);
+ ret = -EINVAL;
+ goto err;
+ }
+
+ fe->key = strdup(key);
+ fe->value = strdup(value);
+
+ list_add_tail(&fe->list, &rd->filter_list);
+ return 0;
+
+err:
+ free(fe);
+ return ret;
+}
+
+int rd_build_filter(struct rd *rd, const char * const valid_filters[])
+{
+ int ret = 0;
+ int idx = 0;
+
+ if (!valid_filters || !rd_argc(rd))
+ goto out;
+
+ if (rd_argc(rd) == 1) {
+ pr_err("No filter data was supplied to filter option %s\n", rd_argv(rd));
+ ret = -EINVAL;
+ goto out;
+ }
+
+ if (rd_argc(rd) % 2) {
+ pr_err("There is filter option without data\n");
+ ret = -EINVAL;
+ goto out;
+ }
+
+ while (idx != rd_argc(rd)) {
+ /*
+ * We can do micro-optimization and skip "dev"
+ * and "link" filters, but it is not worth of it.
+ */
+ ret = add_filter(rd, *(rd->argv + idx),
+ *(rd->argv + idx + 1), valid_filters);
+ if (ret)
+ goto out;
+ idx += 2;
+ }
+
+out:
+ return ret;
+}
+
+bool rd_check_is_key_exist(struct rd *rd, const char *key)
+{
+ struct filter_entry *fe;
+
+ list_for_each_entry(fe, &rd->filter_list, list) {
+ if (!strcmpx(fe->key, key))
+ return true;
+ }
+
+ return false;
+}
+
+/*
+ * Check if string entry is filtered:
+ * * key doesn't exist -> user didn't request -> not filtered
+ */
+bool rd_check_is_string_filtered(struct rd *rd, const char *key, char *val)
+{
+ bool key_is_filtered = false;
+ struct filter_entry *fe;
+ char *p = NULL;
+ char *str;
+
+ list_for_each_entry(fe, &rd->filter_list, list) {
+ if (!strcmpx(fe->key, key)) {
+ /* We found the key */
+ p = strdup(fe->value);
+
+ /*
+ * Need to check if value in range
+ * It can come in the following formats
+ * and their permutations:
+ * str
+ * str1,str2
+ */
+ str = strtok(p, ",");
+ while (str) {
+ if (!strcmpx(str, val)) {
+ key_is_filtered = true;
+ goto out;
+ }
+ str = strtok(NULL, ",");
+ }
+ goto out;
+ }
+ }
+
+out:
+ free(p);
+ return key_is_filtered;
+}
+
+/*
+ * Check if key is filtered:
+ * key doesn't exist -> user didn't request -> not filtered
+ */
+bool rd_check_is_filtered(struct rd *rd, const char *key,
+ uint32_t val, bool ignore_val)
+{
+ bool key_is_filtered = false;
+ struct filter_entry *fe;
+
+ list_for_each_entry(fe, &rd->filter_list, list) {
+ uint32_t left_val = 0, fe_value = 0;
+ bool range_check = false;
+ char *p = fe->value;
+
+ if (!strcmpx(fe->key, key)) {
+ /* We found the key */
+ key_is_filtered = true;
+ if (ignore_val)
+ goto out;
+ /*
+ * Need to check if value in range
+ * It can come in the following formats
+ * (and their permutations):
+ * numb
+ * numb1,numb2
+ * ,numb1,numb2
+ * numb1-numb2
+ * numb1,numb2-numb3,numb4-numb5
+ */
+ while (*p) {
+ if (isdigit(*p)) {
+ fe_value = strtol(p, &p, 10);
+ if (fe_value == val ||
+ (range_check && left_val < val &&
+ val < fe_value)) {
+ key_is_filtered = false;
+ goto out;
+ }
+ range_check = false;
+ } else {
+ if (*p == '-') {
+ left_val = fe_value;
+ range_check = true;
+ }
+ p++;
+ }
+ }
+ goto out;
+ }
+ }
+
+out:
+ return key_is_filtered;
+}
+
+static void filters_cleanup(struct rd *rd)
+{
+ struct filter_entry *fe, *tmp;
+
+ list_for_each_entry_safe(fe, tmp,
+ &rd->filter_list, list) {
+ list_del(&fe->list);
+ free(fe->key);
+ free(fe->value);
+ free(fe);
+ }
+}
+
static const enum mnl_attr_data_type nldev_policy[RDMA_NLDEV_ATTR_MAX] = {
[RDMA_NLDEV_ATTR_DEV_INDEX] = MNL_TYPE_U32,
[RDMA_NLDEV_ATTR_DEV_NAME] = MNL_TYPE_NUL_STRING,
@@ -181,6 +392,7 @@ void rd_free(struct rd *rd)
return;
free(rd->buff);
dev_map_cleanup(rd);
+ filters_cleanup(rd);
}
int rd_exec_link(struct rd *rd, int (*cb)(struct rd *rd), bool strict_port)
--
2.15.1
^ permalink raw reply related
* [PATCH iproute2-next 4/9] rdma: Set pointer to device name position
From: Leon Romanovsky @ 2018-01-02 9:37 UTC (permalink / raw)
To: Doug Ledford, Jason Gunthorpe, David Ahern
Cc: RDMA mailing list, Leon Romanovsky, netdev, Stephen Hemminger
In-Reply-To: <20180102093725.6172-1-leon@kernel.org>
From: Leon Romanovsky <leonro@mellanox.com>
The dev and link execution callbacks expects that next
command line argument is device or port name.
Set pointer to device or port name position prior calls to
rd_exec_dev()/rd_exec_link().
Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
---
rdma/rdma.h | 1 +
rdma/utils.c | 19 +++++++++++++++++++
2 files changed, 20 insertions(+)
diff --git a/rdma/rdma.h b/rdma/rdma.h
index cd415670..e842d076 100644
--- a/rdma/rdma.h
+++ b/rdma/rdma.h
@@ -81,6 +81,7 @@ int rd_exec_cmd(struct rd *rd, const struct rd_cmd *c, const char *str);
int rd_exec_dev(struct rd *rd, int (*cb)(struct rd *rd));
int rd_exec_link(struct rd *rd, int (*cb)(struct rd *rd), bool strict_port);
void rd_free(struct rd *rd);
+int rd_set_arg_to_devname(struct rd *rd);
int rd_argc(struct rd *rd);
/*
diff --git a/rdma/utils.c b/rdma/utils.c
index dc9f4f01..fbeffa08 100644
--- a/rdma/utils.c
+++ b/rdma/utils.c
@@ -395,6 +395,25 @@ void rd_free(struct rd *rd)
filters_cleanup(rd);
}
+int rd_set_arg_to_devname(struct rd *rd)
+{
+ int ret = 0;
+
+ while (!rd_no_arg(rd)) {
+ if (rd_argv_match(rd, "dev") || rd_argv_match(rd, "link")) {
+ rd_arg_inc(rd);
+ if (rd_no_arg(rd)) {
+ pr_err("No device name was supplied\n");
+ ret = -EINVAL;
+ }
+ goto out;
+ }
+ rd_arg_inc(rd);
+ }
+out:
+ return ret;
+}
+
int rd_exec_link(struct rd *rd, int (*cb)(struct rd *rd), bool strict_port)
{
struct dev_map *dev_map;
--
2.15.1
^ permalink raw reply related
* [PATCH iproute2-next 5/9] rdma: Allow external usage of compare string routine
From: Leon Romanovsky @ 2018-01-02 9:37 UTC (permalink / raw)
To: Doug Ledford, Jason Gunthorpe, David Ahern
Cc: RDMA mailing list, Leon Romanovsky, netdev, Stephen Hemminger
In-Reply-To: <20180102093725.6172-1-leon@kernel.org>
From: Leon Romanovsky <leonro@mellanox.com>
Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
---
rdma/rdma.h | 2 ++
rdma/utils.c | 2 +-
2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/rdma/rdma.h b/rdma/rdma.h
index e842d076..816c8ddd 100644
--- a/rdma/rdma.h
+++ b/rdma/rdma.h
@@ -84,6 +84,8 @@ void rd_free(struct rd *rd);
int rd_set_arg_to_devname(struct rd *rd);
int rd_argc(struct rd *rd);
+int strcmpx(const char *str1, const char *str2);
+
/*
* Device manipulation
*/
diff --git a/rdma/utils.c b/rdma/utils.c
index fbeffa08..a63a7c2f 100644
--- a/rdma/utils.c
+++ b/rdma/utils.c
@@ -24,7 +24,7 @@ char *rd_argv(struct rd *rd)
return *rd->argv;
}
-static int strcmpx(const char *str1, const char *str2)
+int strcmpx(const char *str1, const char *str2)
{
if (strlen(str1) > strlen(str2))
return -1;
--
2.15.1
^ permalink raw reply related
* [PATCH iproute2-next 7/9] rdma: Add resource tracking summary
From: Leon Romanovsky @ 2018-01-02 9:37 UTC (permalink / raw)
To: Doug Ledford, Jason Gunthorpe, David Ahern
Cc: RDMA mailing list, Leon Romanovsky, netdev, Stephen Hemminger
In-Reply-To: <20180102093725.6172-1-leon@kernel.org>
From: Leon Romanovsky <leonro@mellanox.com>
The global resource summary information. The object names, current utilization
and maximum numbers are received as is from the kernel.
$ rdma res
1: mlx5_0: curr/max: pd 3/16777216 cq 5/16777216 qp 4/262144
2: mlx5_1: curr/max: pd 3/16777216 cq 5/16777216 qp 4/262144
3: mlx5_2: curr/max: pd 3/16777216 cq 5/16777216 qp 4/262144
4: mlx5_3: curr/max: pd 2/16777216 cq 3/16777216 qp 2/262144
5: mlx5_4: curr/max: pd 3/16777216 cq 5/16777216 qp 4/262144
$ rdma res show mlx5_4
5: mlx5_4: curr/max: pd 3/16777216 cq 5/16777216 qp 4/262144
Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
---
rdma/Makefile | 2 +-
rdma/rdma.c | 3 +-
rdma/rdma.h | 1 +
rdma/res.c | 168 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
rdma/utils.c | 5 ++
5 files changed, 177 insertions(+), 2 deletions(-)
create mode 100644 rdma/res.c
diff --git a/rdma/Makefile b/rdma/Makefile
index c8966bfd..875fe53c 100644
--- a/rdma/Makefile
+++ b/rdma/Makefile
@@ -3,7 +3,7 @@ include ../config.mk
ifeq ($(HAVE_MNL),y)
-RDMA_OBJ = rdma.o utils.o dev.o link.o
+RDMA_OBJ = rdma.o utils.o dev.o link.o res.o
TARGETS=rdma
endif
diff --git a/rdma/rdma.c b/rdma/rdma.c
index a21ba440..19608f41 100644
--- a/rdma/rdma.c
+++ b/rdma/rdma.c
@@ -15,7 +15,7 @@
static void help(char *name)
{
pr_out("Usage: %s [ OPTIONS ] OBJECT { COMMAND | help }\n"
- "where OBJECT := { dev | link | help }\n"
+ "where OBJECT := { dev | link | resource | help }\n"
" OPTIONS := { -V[ersion] | -d[etails] | -j[son] | -p[retty]}\n", name);
}
@@ -32,6 +32,7 @@ static int rd_cmd(struct rd *rd)
{ "help", cmd_help },
{ "dev", cmd_dev },
{ "link", cmd_link },
+ { "resource", cmd_res },
{ 0 }
};
diff --git a/rdma/rdma.h b/rdma/rdma.h
index 816c8ddd..f1ddedd2 100644
--- a/rdma/rdma.h
+++ b/rdma/rdma.h
@@ -77,6 +77,7 @@ char *rd_argv(struct rd *rd);
*/
int cmd_dev(struct rd *rd);
int cmd_link(struct rd *rd);
+int cmd_res(struct rd *rd);
int rd_exec_cmd(struct rd *rd, const struct rd_cmd *c, const char *str);
int rd_exec_dev(struct rd *rd, int (*cb)(struct rd *rd));
int rd_exec_link(struct rd *rd, int (*cb)(struct rd *rd), bool strict_port);
diff --git a/rdma/res.c b/rdma/res.c
new file mode 100644
index 00000000..a70e87dd
--- /dev/null
+++ b/rdma/res.c
@@ -0,0 +1,168 @@
+/*
+ * res.c RDMA tool
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version
+ * 2 of the License, or (at your option) any later version.
+ *
+ * Authors: Leon Romanovsky <leonro@mellanox.com>
+ */
+
+#include "rdma.h"
+#include <inttypes.h>
+
+static int res_help(struct rd *rd)
+{
+ pr_out("Usage: %s resource\n", rd->filename);
+ pr_out(" resource show [DEV]\n");
+ return 0;
+}
+
+static int res_print_summary(struct rd *rd, struct nlattr **tb)
+{
+ struct nlattr *nla_table = tb[RDMA_NLDEV_ATTR_RES_SUMMARY];
+ struct nlattr *nla_entry;
+ uint64_t max, curr;
+ const char *name;
+ int err;
+
+ if (!rd->json_output)
+ pr_out("curr/max: ");
+
+ mnl_attr_for_each_nested(nla_entry, nla_table) {
+ struct nlattr *nla_line[RDMA_NLDEV_ATTR_MAX] = {};
+ char json_name[32];
+
+ err = mnl_attr_parse_nested(nla_entry, rd_attr_cb, nla_line);
+ if (err != MNL_CB_OK)
+ return -EINVAL;
+
+ if (!nla_line[RDMA_NLDEV_ATTR_RES_SUMMARY_ENTRY_NAME] ||
+ !nla_line[RDMA_NLDEV_ATTR_RES_SUMMARY_ENTRY_CURR] ||
+ !nla_line[RDMA_NLDEV_ATTR_RES_SUMMARY_ENTRY_MAX]) {
+ return -EINVAL;
+ }
+
+ name = mnl_attr_get_str(nla_line[RDMA_NLDEV_ATTR_RES_SUMMARY_ENTRY_NAME]);
+ curr = mnl_attr_get_u64(nla_line[RDMA_NLDEV_ATTR_RES_SUMMARY_ENTRY_CURR]);
+ if (rd->json_output) {
+ snprintf(json_name, 32, "curr_%s", name);
+ jsonw_lluint_field(rd->jw, json_name, curr);
+ }
+
+ max = mnl_attr_get_u64(nla_line[RDMA_NLDEV_ATTR_RES_SUMMARY_ENTRY_MAX]);
+ if (rd->json_output) {
+ snprintf(json_name, 32, "max_%s", name);
+ jsonw_lluint_field(rd->jw, json_name, max);
+ } else {
+ pr_out("%s %"PRId64 "/%"PRId64 " ", name, curr, max);
+ }
+ }
+ return 0;
+}
+
+static int res_no_args_parse_cb(const struct nlmsghdr *nlh, void *data)
+{
+ struct nlattr *tb[RDMA_NLDEV_ATTR_MAX] = {};
+ struct rd *rd = data;
+ const char *name;
+ uint32_t idx;
+
+ mnl_attr_parse(nlh, 0, rd_attr_cb, tb);
+ if (!tb[RDMA_NLDEV_ATTR_DEV_INDEX] ||
+ !tb[RDMA_NLDEV_ATTR_DEV_NAME] ||
+ !tb[RDMA_NLDEV_ATTR_RES_SUMMARY])
+ return MNL_CB_ERROR;
+
+ idx = mnl_attr_get_u32(tb[RDMA_NLDEV_ATTR_DEV_INDEX]);
+ name = mnl_attr_get_str(tb[RDMA_NLDEV_ATTR_DEV_NAME]);
+ if (rd->json_output) {
+ jsonw_uint_field(rd->jw, "ifindex", idx);
+ jsonw_string_field(rd->jw, "ifname", name);
+ } else {
+ pr_out("%u: %s: ", idx, name);
+ }
+
+ res_print_summary(rd, tb);
+
+ if (!rd->json_output)
+ pr_out("\n");
+ return MNL_CB_OK;
+}
+
+static int _res_send_msg(struct rd *rd, uint32_t command, mnl_cb_t callback)
+{
+ uint32_t flags = NLM_F_REQUEST | NLM_F_ACK;
+ uint32_t seq;
+ int ret;
+
+ if (command != RDMA_NLDEV_CMD_RES_GET)
+ flags |= NLM_F_DUMP;
+
+ rd_prepare_msg(rd, command, &seq, flags);
+ mnl_attr_put_u32(rd->nlh, RDMA_NLDEV_ATTR_DEV_INDEX, rd->dev_idx);
+ if (rd->port_idx)
+ mnl_attr_put_u32(rd->nlh,
+ RDMA_NLDEV_ATTR_PORT_INDEX, rd->port_idx);
+
+ ret = rd_send_msg(rd);
+ if (ret)
+ return ret;
+
+ if (rd->json_output)
+ jsonw_start_object(rd->jw);
+ ret = rd_recv_msg(rd, callback, rd, seq);
+ if (rd->json_output)
+ jsonw_end_object(rd->jw);
+ return ret;
+}
+
+#define RES_FUNC(name, command, valid_filters, strict_port) \
+ static int _##name(struct rd *rd)\
+ { \
+ return _res_send_msg(rd, command, name##_parse_cb); \
+ } \
+ static int name(struct rd *rd) \
+ {\
+ int ret = rd_build_filter(rd, valid_filters); \
+ if (ret) \
+ return ret; \
+ if ((uintptr_t)valid_filters != (uintptr_t)NULL) { \
+ ret = rd_set_arg_to_devname(rd); \
+ if (ret) \
+ return ret;\
+ } \
+ return rd_exec_link(rd, _##name, strict_port); \
+ }
+
+RES_FUNC(res_no_args, RDMA_NLDEV_CMD_RES_GET, NULL, true);
+
+static int res_show(struct rd *rd)
+{
+ const struct rd_cmd cmds[] = {
+ { NULL, res_no_args },
+ { 0 }
+ };
+
+ /*
+ * Special case to support "rdma res show DEV_NAME"
+ */
+ if (rd_argc(rd) == 1 && dev_map_lookup(rd, false))
+ return rd_exec_dev(rd, _res_no_args);
+
+ return rd_exec_cmd(rd, cmds, "parameter");
+}
+
+int cmd_res(struct rd *rd)
+{
+ const struct rd_cmd cmds[] = {
+ { NULL, res_show },
+ { "show", res_show },
+ { "list", res_show },
+ { "help", res_help },
+ { 0 }
+ };
+
+ return rd_exec_cmd(rd, cmds, "resource command");
+}
diff --git a/rdma/utils.c b/rdma/utils.c
index a63a7c2f..859ad7ab 100644
--- a/rdma/utils.c
+++ b/rdma/utils.c
@@ -339,6 +339,11 @@ static const enum mnl_attr_data_type nldev_policy[RDMA_NLDEV_ATTR_MAX] = {
[RDMA_NLDEV_ATTR_PORT_STATE] = MNL_TYPE_U8,
[RDMA_NLDEV_ATTR_PORT_PHYS_STATE] = MNL_TYPE_U8,
[RDMA_NLDEV_ATTR_DEV_NODE_TYPE] = MNL_TYPE_U8,
+ [RDMA_NLDEV_ATTR_RES_SUMMARY] = MNL_TYPE_NESTED,
+ [RDMA_NLDEV_ATTR_RES_SUMMARY_ENTRY] = MNL_TYPE_NESTED,
+ [RDMA_NLDEV_ATTR_RES_SUMMARY_ENTRY_NAME] = MNL_TYPE_NUL_STRING,
+ [RDMA_NLDEV_ATTR_RES_SUMMARY_ENTRY_CURR] = MNL_TYPE_U64,
+ [RDMA_NLDEV_ATTR_RES_SUMMARY_ENTRY_MAX] = MNL_TYPE_U64,
};
int rd_attr_cb(const struct nlattr *attr, void *data)
--
2.15.1
^ permalink raw reply related
* [PATCH iproute2-next 8/9] rdma: Add QP resource tracking information
From: Leon Romanovsky @ 2018-01-02 9:37 UTC (permalink / raw)
To: Doug Ledford, Jason Gunthorpe, David Ahern
Cc: RDMA mailing list, Leon Romanovsky, netdev, Stephen Hemminger
In-Reply-To: <20180102093725.6172-1-leon@kernel.org>
From: Leon Romanovsky <leonro@mellanox.com>
This patch adds ss-similar interface to view various resource
tracked objects. At this stage, only QP is presented.
1. Get all QPs for the specific device:
$ rdma res show qp link mlx5_4
DEV/PORT LQPN TYPE STATE PID COMM
mlx5_4/- 8 UD RESET 0 [ipoib-verbs]
mlx5_4/1 7 UD RTS 0 [mlx5-gsi]
mlx5_4/1 1 GSI RTS 0 [rdma-mad]
mlx5_4/1 0 SMI RTS 0 [rdma-mad]
$ rdma res show qp link mlx5_4/
DEV/PORT LQPN TYPE STATE PID COMM
mlx5_4/- 8 UD RESET 0 [ipoib-verbs]
mlx5_4/1 7 UD RTS 0 [mlx5-gsi]
mlx5_4/1 1 GSI RTS 0 [rdma-mad]
mlx5_4/1 0 SMI RTS 0 [rdma-mad]
2. Provide illegal port number (0 is illegal):
$ rdma res show qp link mlx5_4/0
Wrong device name
3. Get QPs of specific port:
$ rdma res show qp link mlx5_4/1
DEV/PORT LQPN TYPE STATE PID COMM
mlx5_4/1 7 UD RTS 0 [mlx5-gsi]
mlx5_4/1 1 GSI RTS 0 [rdma-mad]
mlx5_4/1 0 SMI RTS 0 [rdma-mad]
4. Get QPs which have not assigned port yet:
$ rdma res show qp link mlx5_4/-
DEV/PORT LQPN TYPE STATE PID COMM
mlx5_4/- 8 UD RESET 0 [ipoib-verbs]
5. Detailed view:
$ rdma res show qp link mlx5_4/- -d
DEV/PORT LQPN RQPN TYPE STATE PID COMM SQ-PSN RQ-PSN PATH-MIG
mlx5_4/- 8 --- UD RESET 0 [ipoib-verbs] 0 --- ---
6. Limit to specific columns (dev/port is always available):
$ rdma res show qp link mlx5_4/1 display pid,lqpn,comm
DEV/PORT LQPN PID COMM
mlx5_4/1 7 0 [mlx5-gsi]
mlx5_4/1 1 0 [rdma-mad]
mlx5_4/1 0 0 [rdma-mad]
7. Detailed view (no change, due to "display" option):
$ rdma res show qp link mlx5_4/1 display pid,lqpn,comm -d
DEV/PORT LQPN PID COMM
mlx5_4/1 7 0 [mlx5-gsi]
mlx5_4/1 1 0 [rdma-mad]
mlx5_4/1 0 0 [rdma-mad]
8. Limit to specific Local QPNs:
$ rdma res show qp link mlx5_4/1 display pid,lqpn,comm lqpn 0-6
DEV/PORT LQPN PID COMM
mlx5_4/1 1 0 [rdma-mad]
mlx5_4/1 0 0 [rdma-mad]
Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
---
rdma/res.c | 243 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
rdma/utils.c | 11 +++
2 files changed, 254 insertions(+)
diff --git a/rdma/res.c b/rdma/res.c
index a70e87dd..184a3a02 100644
--- a/rdma/res.c
+++ b/rdma/res.c
@@ -16,6 +16,9 @@ static int res_help(struct rd *rd)
{
pr_out("Usage: %s resource\n", rd->filename);
pr_out(" resource show [DEV]\n");
+ pr_out(" resource show [qp]\n");
+ pr_out(" resource show qp link [DEV/PORT]\n");
+ pr_out(" resource show qp link [DEV/PORT] [FILTER-NAME FILTER-VALUE]\n");
return 0;
}
@@ -136,12 +139,252 @@ static int _res_send_msg(struct rd *rd, uint32_t command, mnl_cb_t callback)
return rd_exec_link(rd, _##name, strict_port); \
}
+struct column {
+ char filter_name[32];
+ char column_name[32];
+ bool in_simple_view;
+};
+
+static bool show_column(struct rd *rd, struct column *c)
+{
+ if (rd_check_is_key_exist(rd, "display"))
+ return rd_check_is_string_filtered(rd, "display", c->filter_name);
+
+ if (!rd->show_details)
+ return c->in_simple_view;
+ return true;
+}
+
+static const char *path_mig_to_str(uint8_t idx)
+{
+ static const char * const path_mig_str[] = { "MIGRATED",
+ "REARM", "ARMED" };
+
+ if (idx < ARRAY_SIZE(path_mig_str))
+ return path_mig_str[idx];
+ return "UNKNOWN";
+}
+
+static const char *qp_states_to_str(uint8_t idx)
+{
+ static const char * const qp_states_str[] = { "RESET", "INIT",
+ "RTR", "RTS", "SQD",
+ "SQE", "ERR" };
+
+ if (idx < ARRAY_SIZE(qp_states_str))
+ return qp_states_str[idx];
+ return "UNKNOWN";
+}
+
+static const char *qp_types_to_str(uint8_t idx)
+{
+ static const char * const qp_types_str[] = { "SMI", "GSI", "RC",
+ "UC", "UD", "RAW_IPV6",
+ "RAW_ETHERTYPE",
+ "UNKNOWN", "RAW_PACKET",
+ "XRC_INI", "XRC_TGT" };
+
+ if (idx < ARRAY_SIZE(qp_types_str))
+ return qp_types_str[idx];
+ return "UNKNOWN";
+}
+
+static int res_qp_parse_cb(const struct nlmsghdr *nlh, void *data)
+{
+ static struct column c[] = { { .filter_name = "lqpn",
+ .column_name = "LQPN ",
+ .in_simple_view = true },
+ { .filter_name = "rqpn",
+ .column_name = "RQPN " },
+ { .filter_name = "type",
+ .column_name = "TYPE ",
+ .in_simple_view = true },
+ { .filter_name = "state",
+ .column_name = "STATE ",
+ .in_simple_view = true },
+ { .filter_name = "pid",
+ .column_name = "PID ",
+ .in_simple_view = true },
+ { .filter_name = "comm",
+ .column_name = "COMM ",
+ .in_simple_view = true },
+ { .filter_name = "sq-psn",
+ .column_name = "SQ-PSN " },
+ { .filter_name = "rq-psn",
+ .column_name = "RQ-PSN " },
+ { .filter_name = "path-mig",
+ .column_name = "PATH-MIG " } };
+
+ struct nlattr *tb[RDMA_NLDEV_ATTR_MAX] = {};
+ struct nlattr *nla_table, *nla_entry;
+ static bool print_header = true;
+ struct rd *rd = data;
+ uint32_t cidx, idx;
+ const char *name;
+
+ mnl_attr_parse(nlh, 0, rd_attr_cb, tb);
+ if (!tb[RDMA_NLDEV_ATTR_DEV_INDEX] ||
+ !tb[RDMA_NLDEV_ATTR_DEV_NAME] ||
+ !tb[RDMA_NLDEV_ATTR_RES_QP])
+ return MNL_CB_ERROR;
+
+ name = mnl_attr_get_str(tb[RDMA_NLDEV_ATTR_DEV_NAME]);
+ idx = mnl_attr_get_u32(tb[RDMA_NLDEV_ATTR_DEV_INDEX]);
+ nla_table = tb[RDMA_NLDEV_ATTR_RES_QP];
+ if (!rd->json_output && print_header) {
+ pr_out("DEV/PORT ");
+ for (cidx = 0; cidx < ARRAY_SIZE(c); cidx++)
+ if (show_column(rd, &c[cidx]))
+ pr_out("%s", c[cidx].column_name);
+ pr_out("\n");
+ print_header = false;
+ }
+
+ mnl_attr_for_each_nested(nla_entry, nla_table) {
+ struct nlattr *nla_line[RDMA_NLDEV_ATTR_MAX] = {};
+ uint32_t lqpn, rqpn = 0, rq_psn = 0, sq_psn;
+ uint8_t type, state, path_mig_state = 0;
+ uint32_t port = 0, pid = 0;
+ bool ignore_value = false;
+ char port_name[32];
+ const char *comm;
+ int err;
+
+ err = mnl_attr_parse_nested(nla_entry, rd_attr_cb, nla_line);
+ if (err != MNL_CB_OK)
+ return -EINVAL;
+
+ if (!nla_line[RDMA_NLDEV_ATTR_RES_LQPN] ||
+ !nla_line[RDMA_NLDEV_ATTR_RES_SQ_PSN] ||
+ !nla_line[RDMA_NLDEV_ATTR_RES_TYPE] ||
+ !nla_line[RDMA_NLDEV_ATTR_RES_STATE] ||
+ !nla_line[RDMA_NLDEV_ATTR_RES_PID_COMM]) {
+ return -EINVAL;
+ }
+
+ if (nla_line[RDMA_NLDEV_ATTR_PORT_INDEX])
+ port = mnl_attr_get_u32(nla_line[RDMA_NLDEV_ATTR_PORT_INDEX]);
+
+ if (port != rd->port_idx)
+ continue;
+
+ if (nla_line[RDMA_NLDEV_ATTR_PORT_INDEX])
+ snprintf(port_name, 32, "%s/%u", name, port);
+ else
+ snprintf(port_name, 32, "%s/-", name);
+
+ lqpn = mnl_attr_get_u32(nla_line[RDMA_NLDEV_ATTR_RES_LQPN]);
+ if (rd_check_is_filtered(rd, "lqpn", lqpn, false))
+ continue;
+
+ if (nla_line[RDMA_NLDEV_ATTR_RES_RQPN])
+ rqpn = mnl_attr_get_u32(nla_line[RDMA_NLDEV_ATTR_RES_RQPN]);
+ else
+ ignore_value = true;
+
+ if (rd_check_is_filtered(rd, "rqpn", rqpn, ignore_value))
+ continue;
+
+ if (nla_line[RDMA_NLDEV_ATTR_RES_RQ_PSN])
+ rq_psn = mnl_attr_get_u32(nla_line[RDMA_NLDEV_ATTR_RES_RQ_PSN]);
+
+ sq_psn = mnl_attr_get_u32(nla_line[RDMA_NLDEV_ATTR_RES_SQ_PSN]);
+ if (nla_line[RDMA_NLDEV_ATTR_RES_PATH_MIG_STATE])
+ path_mig_state = mnl_attr_get_u8(nla_line[RDMA_NLDEV_ATTR_RES_PATH_MIG_STATE]);
+ type = mnl_attr_get_u8(nla_line[RDMA_NLDEV_ATTR_RES_TYPE]);
+ state = mnl_attr_get_u8(nla_line[RDMA_NLDEV_ATTR_RES_STATE]);
+
+ if (nla_line[RDMA_NLDEV_ATTR_RES_PID])
+ pid = mnl_attr_get_u32(nla_line[RDMA_NLDEV_ATTR_RES_PID]);
+
+ if (rd_check_is_filtered(rd, "pid", pid, false))
+ continue;
+
+ comm = mnl_attr_get_str(nla_line[RDMA_NLDEV_ATTR_RES_PID_COMM]);
+
+ if (rd->json_output) {
+ jsonw_start_array(rd->jw);
+ jsonw_uint_field(rd->jw, "ifindex", idx);
+ if (nla_line[RDMA_NLDEV_ATTR_PORT_INDEX])
+ jsonw_uint_field(rd->jw, "port", port);
+ jsonw_string_field(rd->jw, "ifname", port_name);
+ jsonw_uint_field(rd->jw, "lqpn", lqpn);
+ if (nla_line[RDMA_NLDEV_ATTR_RES_RQPN])
+ jsonw_uint_field(rd->jw, "rqpn", rqpn);
+ if (nla_line[RDMA_NLDEV_ATTR_RES_RQ_PSN])
+ jsonw_uint_field(rd->jw, "rq-psn", rq_psn);
+ if (nla_line[RDMA_NLDEV_ATTR_RES_SQ_PSN])
+ jsonw_uint_field(rd->jw, "sq-psn", sq_psn);
+ if (nla_line[RDMA_NLDEV_ATTR_RES_PATH_MIG_STATE])
+ jsonw_string_field(rd->jw, "path-mig-state",
+ path_mig_to_str(path_mig_state));
+
+ jsonw_string_field(rd->jw, "type", qp_types_to_str(type));
+ jsonw_string_field(rd->jw, "state", qp_states_to_str(state));
+ if (nla_line[RDMA_NLDEV_ATTR_RES_PID])
+ jsonw_uint_field(rd->jw, "pid", pid);
+ jsonw_string_field(rd->jw, "comm", comm);
+ jsonw_end_array(rd->jw);
+ } else {
+ pr_out("%-10s", port_name);
+ for (cidx = 0; cidx < ARRAY_SIZE(c); cidx++)
+ if (show_column(rd, &c[cidx])) {
+ if (!strcmpx(c[cidx].filter_name, "lqpn"))
+ pr_out("%-11u", lqpn);
+ if (!strcmpx(c[cidx].filter_name, "rqpn")) {
+ if (nla_line[RDMA_NLDEV_ATTR_RES_RQPN])
+ pr_out("%-11u", rqpn);
+ else
+ pr_out("%-11s", "---");
+ }
+ if (!strcmpx(c[cidx].filter_name, "type"))
+ pr_out("%-6s", qp_types_to_str(type));
+ if (!strcmpx(c[cidx].filter_name, "state"))
+ pr_out("%-7s", qp_states_to_str(state));
+ if (!strcmpx(c[cidx].filter_name, "rq-psn")) {
+ if (nla_line[RDMA_NLDEV_ATTR_RES_RQ_PSN])
+ pr_out("%-11d", rq_psn);
+ else
+ pr_out("%-11s", "---");
+ }
+ if (!strcmpx(c[cidx].filter_name, "sq-psn"))
+ pr_out("%-11d", sq_psn);
+ if (!strcmpx(c[cidx].filter_name, "path-mig")) {
+ if (nla_line[RDMA_NLDEV_ATTR_RES_PATH_MIG_STATE])
+ pr_out("%-16s", path_mig_to_str(path_mig_state));
+ else
+ pr_out("%-16s", "---");
+ }
+ if (!strcmpx(c[cidx].filter_name, "pid"))
+ pr_out("%-11d", pid);
+ if (!strcmpx(c[cidx].filter_name, "comm")) {
+ if (nla_line[RDMA_NLDEV_ATTR_RES_PID]) {
+ pr_out("%-16s ", comm);
+ } else {
+ char tmp[18];
+
+ snprintf(tmp, sizeof(tmp), "[%s]", comm);
+ pr_out("%-16s", tmp);
+ }
+ }
+ }
+ pr_out("\n");
+ }
+ }
+ return MNL_CB_OK;
+}
+
RES_FUNC(res_no_args, RDMA_NLDEV_CMD_RES_GET, NULL, true);
+static const char * const qp_valid_filters[] = { "link", "lqpn", "rqpn",
+ "pid", "display", NULL };
+RES_FUNC(res_qp, RDMA_NLDEV_CMD_RES_QP_GET, qp_valid_filters, false);
+
static int res_show(struct rd *rd)
{
const struct rd_cmd cmds[] = {
{ NULL, res_no_args },
+ { "qp", res_qp },
{ 0 }
};
diff --git a/rdma/utils.c b/rdma/utils.c
index 859ad7ab..65bb6657 100644
--- a/rdma/utils.c
+++ b/rdma/utils.c
@@ -344,6 +344,17 @@ static const enum mnl_attr_data_type nldev_policy[RDMA_NLDEV_ATTR_MAX] = {
[RDMA_NLDEV_ATTR_RES_SUMMARY_ENTRY_NAME] = MNL_TYPE_NUL_STRING,
[RDMA_NLDEV_ATTR_RES_SUMMARY_ENTRY_CURR] = MNL_TYPE_U64,
[RDMA_NLDEV_ATTR_RES_SUMMARY_ENTRY_MAX] = MNL_TYPE_U64,
+ [RDMA_NLDEV_ATTR_RES_QP] = MNL_TYPE_NESTED,
+ [RDMA_NLDEV_ATTR_RES_QP_ENTRY] = MNL_TYPE_NESTED,
+ [RDMA_NLDEV_ATTR_RES_LQPN] = MNL_TYPE_U32,
+ [RDMA_NLDEV_ATTR_RES_RQPN] = MNL_TYPE_U32,
+ [RDMA_NLDEV_ATTR_RES_RQ_PSN] = MNL_TYPE_U32,
+ [RDMA_NLDEV_ATTR_RES_SQ_PSN] = MNL_TYPE_U32,
+ [RDMA_NLDEV_ATTR_RES_PATH_MIG_STATE] = MNL_TYPE_U8,
+ [RDMA_NLDEV_ATTR_RES_TYPE] = MNL_TYPE_U8,
+ [RDMA_NLDEV_ATTR_RES_STATE] = MNL_TYPE_U8,
+ [RDMA_NLDEV_ATTR_RES_PID] = MNL_TYPE_U32,
+ [RDMA_NLDEV_ATTR_RES_PID_COMM] = MNL_TYPE_NUL_STRING,
};
int rd_attr_cb(const struct nlattr *attr, void *data)
--
2.15.1
^ permalink raw reply related
* [PATCH iproute2-next 9/9] rdma: Document resource tracking
From: Leon Romanovsky @ 2018-01-02 9:37 UTC (permalink / raw)
To: Doug Ledford, Jason Gunthorpe, David Ahern
Cc: RDMA mailing list, Leon Romanovsky, netdev, Stephen Hemminger
In-Reply-To: <20180102093725.6172-1-leon@kernel.org>
From: Leon Romanovsky <leonro@mellanox.com>
Spartan version of resource tracking documentation.
Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
---
man/man8/rdma-resource.8 | 91 ++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 91 insertions(+)
create mode 100644 man/man8/rdma-resource.8
diff --git a/man/man8/rdma-resource.8 b/man/man8/rdma-resource.8
new file mode 100644
index 00000000..e3c83b94
--- /dev/null
+++ b/man/man8/rdma-resource.8
@@ -0,0 +1,91 @@
+.TH RDMA\-RESOURCE 8 "26 Dec 2017" "iproute2" "Linux"
+.SH NAME
+rdma-resource \- rdma resource configuration
+.SH SYNOPSIS
+.sp
+.ad l
+.in +8
+.ti -8
+.B rdma
+.RI "[ " OPTIONS " ]"
+.B resource
+.RI " { " COMMAND " | "
+.BR help " }"
+.sp
+
+.ti -8
+.IR OPTIONS " := { "
+\fB\-j\fR[\fIson\fR] |
+\fB\-d\fR[\fIetails\fR] }
+
+.ti -8
+.B rdma resource show
+.RI "[ " DEV/PORT_INDEX " ]"
+
+.ti -8
+.B rdma resource help
+
+.SH "DESCRIPTION"
+.SS rdma resource show - display rdma resource tracking information
+
+.PP
+.I "DEV/PORT_INDEX"
+- specifies the RDMA link to show.
+If this argument is omitted all links are listed.
+
+.SH "EXAMPLES"
+.PP
+rdma resource show
+.RS 4
+Shows summary for all devices on the system.
+.RE
+.PP
+rdma resource show mlx5_2
+.RS 4
+Shows the state of specified rdma device.
+.RE
+.PP
+rdma res show qp link mlx5_4
+.RS 4
+Get all QPs for the specific device.
+.RE
+.PP
+rdma res show qp link mlx5_4/1
+.RS 4
+Get QPs of specific port.
+.RE
+.PP
+rdma res show qp link mlx5_4/0
+.RS 4
+Provide illegal port number (0 is illegal).
+.RE
+.PP
+rdma res show qp link mlx5_4/-
+.RS 4
+Get QPs which have not assigned port yet.
+.RE
+.PP
+rdma res show qp link mlx5_4/- -d
+.RS 4
+Detailed view.
+.RE
+.PP
+rdma res show qp link mlx5_4/1 display pid,lqpn,comm
+.RS 4
+Limit to specific columns (dev/port is always available)
+.RE
+.PP
+rdma res show qp link mlx5_4/1 display pid,lqpn,comm lqpn 0-6
+.RS 4
+Limit to specific Local QPNs.
+.RE
+.PP
+
+.SH SEE ALSO
+.BR rdma (8),
+.BR rdma-dev (8),
+.BR rdma-link (8),
+.br
+
+.SH AUTHOR
+Leon Romanovsky <leonro@mellanox.com>
--
2.15.1
^ permalink raw reply related
* [PATCH net,stable 1/1] net: fec: defer probe if regulator is not ready
From: Fugang Duan @ 2018-01-02 9:57 UTC (permalink / raw)
To: davem; +Cc: netdev, troy.kisky, andrew, fugang.duan
Defer probe if regulator is not ready. E.g. some regulator is fixed
regulator controlled by i2c expander gpio, the i2c device may be probed
after the driver, then it should handle the case of defer probe error.
Signed-off-by: Fugang Duan <fugang.duan@nxp.com>
---
drivers/net/ethernet/freescale/fec_main.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index e17d10b..feed383 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -3489,6 +3489,10 @@ static int fec_enet_get_irq_cnt(struct platform_device *pdev)
goto failed_regulator;
}
} else {
+ if (PTR_ERR(fep->reg_phy) == -EPROBE_DEFER) {
+ ret = -EPROBE_DEFER;
+ goto failed_regulator;
+ }
fep->reg_phy = NULL;
}
@@ -3576,6 +3580,7 @@ static int fec_enet_get_irq_cnt(struct platform_device *pdev)
of_node_put(phy_node);
failed_ioremap:
free_netdev(ndev);
+ dev_id--;
return ret;
}
--
1.9.1
^ permalink raw reply related
* Re: [patch net-next v2 00/10] Add support for resource abstraction
From: Jiri Pirko @ 2018-01-02 10:08 UTC (permalink / raw)
To: Arkadi Sharshevsky
Cc: netdev, dsa, roopa, davem, mlxsw, andrew, vivien.didelot,
f.fainelli, michael.chan, ganeshgr, saeedm, matanb, leonro,
idosch, jakub.kicinski, ast, daniel, simon.horman,
pieter.jansenvanvuuren, john.hurley, alexander.h.duyck, linville,
gospo, steven.lin1, yuvalm, ogerlitz
In-Reply-To: <977652df-a0ed-d1a5-f299-1dc433ebd337@mellanox.com>
Mon, Jan 01, 2018 at 03:58:33PM CET, arkadis@mellanox.com wrote:
>
>
>On 12/26/2017 01:23 PM, Jiri Pirko wrote:
>> From: Jiri Pirko <jiri@mellanox.com>
>>
>> Many of the ASIC's internal resources are limited and are shared between
>> several hardware procedures. For example, unified hash-based memory can
>> be used for many lookup purposes, like FDB and LPM. In many cases the user
>> can provide a partitioning scheme for such a resource in order to perform
>> fine tuning for his application. In such cases performing driver reload is
>> needed for the changes to take place, thus this patchset also adds support
>> for hot reload.
>>
>> Such an abstraction can be coupled with devlink's dpipe interface, which
>> models the ASIC's pipeline as a graph of match/action tables. By modeling
>> the hardware resource object, and by coupling it to several dpipe tables,
>> further visibility can be achieved in order to debug ASIC-wide issues.
>>
>> The proposed interface will provide the user the ability to understand the
>> limitations of the hardware, and receive notification regarding its occupancy.
>> Furthermore, monitoring the resource occupancy can be done in real-time and
>> can be useful in many cases.
>> ---
>> Userspace part prototype can be found at https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Farkadis%2Fiproute2%2F&data=02%7C01%7Carkadis%40mellanox.com%7C1ae3d8b4854a454e21e008d54c5329e3%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C636498842440762657&sdata=7MC2BFQFxjnmHqy2sOOL9VEa4ZGq6e5Z2n2WvuNgyFk%3D&reserved=0
>> at resource_dev branch.
>>
>> v1->v2
>> - Add resource size attribute.
>> - Fix split bug.
>>
>
>Just to summarize the current fixes required:
>
>1. ERIF dpipe table size is reporting wrong size. More precisely the
> ERIF table does not take rifs, so it should not be linked to the rif
> bank resource (is not part of this patchset, future extension).
>2. Extended ACK user-space bug.
>3. ABI documentation- Not sure we agreed upon it, Jiri?
Question is where to put it. It is mlxsw-specific thing, moreover,
Spectrum-specific thing, same as dpipe tables etc. Not sure. Perhaps
Documentation/networking/mlxsw.txt ?
>
>If I missed something please respond. Nothing of the fixes mentioned
>above is relevant for this patchset actually.
>
>Couple of key-points:
>
>1. Constrains\trade off about setting the sizes - this can be obtained
> trivially from the resource tree nested structure.
>2. Dpipe provides the mapping of hardware processes to resources.
>3. Units - each resource specifies his units, if dpipe table's size is
> X and its related to some resource its size is normalized to that
> resources basic unit.
>
>IMO this is the most hardware exact interaction, and this is the way it
>should be exported from the kernel, if something is not presented in
>'user' convenient way some utilities can be implemented in userspace
>to easily do it. Furthermore, some examples will be provided for the
>whole kvd tree partition for different cases (IPv6 heavy etc..).
>Advanced user will be able to tweak it as they like.
>
>Regarding the 'switchdev' layer I think that kernel's software tables
>like nexthops/neigh/routes should be mapped to dpipe tables and not
>to resources directly:
Sure. dpipe table -> resource mapping is the only one that makes sense.
>
>kernel_fdb--> dpipe_fdb -->/kvd/hash_single.
>
>> Arkadi Sharshevsky (10):
>> devlink: Add per devlink instance lock
>> devlink: Add support for resource abstraction
>> devlink: Add support for reload
>> devlink: Add relation between dpipe and resource
>> mlxsw: pci: Add support for performing bus reset
>> mlxsw: spectrum: Register KVD resources with devlink
>> mlxsw: spectrum_dpipe: Connect dpipe tables to resources
>> mlxsw: spectrum: Add support for getting kvdl occupancy
>> mlxsw: pci: Add support for getting resource through devlink
>> mlxsw: core: Add support for reload
>>
>> drivers/net/ethernet/mellanox/mlxsw/core.c | 85 ++-
>> drivers/net/ethernet/mellanox/mlxsw/core.h | 16 +-
>> drivers/net/ethernet/mellanox/mlxsw/i2c.c | 5 +-
>> drivers/net/ethernet/mellanox/mlxsw/pci.c | 98 ++--
>> drivers/net/ethernet/mellanox/mlxsw/spectrum.c | 205 ++++++++
>> drivers/net/ethernet/mellanox/mlxsw/spectrum.h | 13 +
>> .../net/ethernet/mellanox/mlxsw/spectrum_dpipe.c | 72 ++-
>> .../net/ethernet/mellanox/mlxsw/spectrum_kvdl.c | 26 +
>> include/net/devlink.h | 97 ++++
>> include/uapi/linux/devlink.h | 21 +
>> net/core/devlink.c | 573 ++++++++++++++++++---
>> 11 files changed, 1079 insertions(+), 132 deletions(-)
>>
^ permalink raw reply
* Re: [PATCH 1/2] ravb: kill redundant check in the probe() method
From: Sergei Shtylyov @ 2018-01-02 10:09 UTC (permalink / raw)
To: netdev, open list:RENESAS ETHERNET DRIVERS
In-Reply-To: <20171231184444.642619406@cogentembedded.com>
Hello!
On 12/31/2017 9:41 PM, Sergei Shtylyov wrote:
> Browsing thru the driver diassembly, I noticed that gcc was able to
Aw, it's disassembly!
DaveM, is that worth reposting?
> figure out that the 'ndev' pointer is always non-NULL when calling
> free_netdev() on the probe() method's error path and thus skip that
> redundant NULL check... gcc is smart, be like gcc!
>
> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
[...]
MBR, Sergei
^ permalink raw reply
* Re: [PATCH net,stable 1/1] net: fec: defer probe if regulator is not ready
From: Fabio Estevam @ 2018-01-02 10:20 UTC (permalink / raw)
To: Fugang Duan; +Cc: David S. Miller, netdev, Troy Kisky, Andrew Lunn
In-Reply-To: <1514887064-3896-1-git-send-email-fugang.duan@nxp.com>
Hi Andy,
On Tue, Jan 2, 2018 at 7:57 AM, Fugang Duan <fugang.duan@nxp.com> wrote:
> @@ -3576,6 +3580,7 @@ static int fec_enet_get_irq_cnt(struct platform_device *pdev)
> of_node_put(phy_node);
> failed_ioremap:
> free_netdev(ndev);
> + dev_id--;
This seems to be a different fix and should be part of a separate patch.
Thanks
^ permalink raw reply
* Re: ACPI issues on cold power on [bisected]
From: Rafael J. Wysocki @ 2018-01-02 10:25 UTC (permalink / raw)
To: Joonsoo Kim, Jonathan McDowell
Cc: ACPI Devel Maling List, Linux Kernel Mailing List,
Linux Memory Management List, netdev
In-Reply-To: <20180102025417.GA20740@js1304-P5Q-DELUXE>
On Tue, Jan 2, 2018 at 3:54 AM, Joonsoo Kim <iamjoonsoo.kim@lge.com> wrote:
> On Fri, Dec 29, 2017 at 04:36:59PM +0000, Jonathan McDowell wrote:
>> On Fri, Dec 22, 2017 at 09:21:09AM +0900, Joonsoo Kim wrote:
>> > On Fri, Dec 08, 2017 at 03:11:59PM +0000, Jonathan McDowell wrote:
>> > > I've been sitting on this for a while and should have spent time to
>> > > investigate sooner, but it's been an odd failure mode that wasn't quite
>> > > obvious.
>> > >
>> > > In 4.9 if I cold power on my laptop (Dell E7240) it fails to boot - I
>> > > don't see anything after grub says its booting. In 4.10 onwards the
>> > > laptop boots, but I get an Oops as part of the boot and ACPI is unhappy
>> > > (no suspend, no clean poweroff, no ACPI buttons). The Oops is below;
>> > > taken from 4.12 as that's the most recent error dmesg I have saved but
>> > > also seen back in 4.10. It's always address 0x30 for the dereference.
>> > >
>> > > Rebooting the laptop does not lead to these problems; it's *only* from a
>> > > complete cold boot that they arise (which didn't help me in terms of
>> > > being able to reliably bisect). Once I realised that I was able to
>> > > bisect, but it leads me to an odd commit:
>> > >
>> > > 86d9f48534e800e4d62cdc1b5aaf539f4c1d47d6
>> > > (mm/slab: fix kmemcg cache creation delayed issue)
>> > >
>> > > If I revert this then I can cold boot without problems.
>> > >
>> > > Also I don't see the problem with a stock Debian kernel, I think because
>> > > the ACPI support is modularised.
>> >
>> > Sorry for late response. I was on a long vacation.
>>
>> No problem. I've been trying to get around to diagnosing this for a
>> while now anyway and this isn't a great time of year for fast responses.
>>
>> > I have tried to solve the problem however I don't find any clue yet.
>> >
>> > >From my analysis, oops report shows that 'struct sock *ssk' passed to
>> > netlink_broadcast_filtered() is NULL. It means that some of
>> > netlink_kernel_create() returns NULL. Maybe, it is due to slab
>> > allocation failure. Could you check it by inserting some log on that
>> > part? The issue cannot be reproducible in my side so I need your help.
>>
>> I've added some debug in acpi_bus_generate_netlink_event +
>> genlmsg_multicast and the problem seems to be that genlmsg_multicast is
>> getting called when init_net.genl_sock has not yet been initialised,
>> leading to the NULL deference.
>>
>> Full dmesg output from a cold 4.14.8 boot at:
>>
>> https://the.earth.li/~noodles/acpi-problem/dmesg-4.14.8-broken
>>
>> And the same kernel after a reboot ("shutdown -r now"):
>>
>> https://the.earth.li/~noodles/acpi-problem/dmesg-4.14.8-working
>>
>> Patch that I've applied is at
>>
>> https://the.earth.li/~noodles/acpi-problem/debug-acpi.diff
>>
>
> Thanks for testing! It's very helpful.
>
>> The interesting difference seems to be:
>>
>> PCI: Using ACPI for IRQ routing
>> +ACPI: Generating event type 208 (:9DBB5994-A997-11DA-B012-B622A1EF5492)
>> +ERROR: init_net.genl_sock is NULL
>> +BUG: unable to handle kernel NULL pointer dereference at 0000000000000030
>> +IP: netlink_broadcast_filtered+0x20/0x3d0
>> +PGD 0 P4D 0
>> +Oops: 0000 [#1] SMP
>> +Modules linked in:
>> +CPU: 0 PID: 29 Comm: kworker/0:1 Not tainted 4.14.8+ #1
>> +Hardware name: Dell Inc. Latitude E7240/07RPNV, BIOS A22 10/18/2017
>> +Workqueue: kacpi_notify acpi_os_execute_deferred
>>
>> 9DBB5994-A997-11DA-B012-B622A1EF5492 is the Dell WMI event GUID and
>> there's no visible event for it on a reboot, just on a cold power on.
>> Some sort of ordering issues such that genl_sock is being initialised
>> later with the slab change?
>
> I have checked that there is an ordering issue.
>
> genl_init() which initializes init_net->genl_sock is called on
> subsys_initcall().
>
> acpi_wmi_init() which schedules acpi_wmi_notify_handler() to the
> workqueue is called on subsys_initcall(), too.
> (acpi_wmi_notify_handler() -> acpi_bus_generate_netlink_event() ->
> netlink_broadcast())
>
> In my system, acpi_wmi_init() is called before the genl_init().
> Therefore, if the worker is scheduled before genl_init() is done, NULL
> derefence would happen.
Does it help to change the subsys_initcall() in wmi.c to subsys_initcall_sync()?
^ permalink raw reply
* Re: [PATCH net-next A 3/5] sfp: add support for 1000Base-PX and 1000Base-BX10
From: Russell King - ARM Linux @ 2018-01-02 10:39 UTC (permalink / raw)
To: andrew, f.fainelli, netdev
Hi,
I'm not entirely convinced that adding support for this is correct,
so I'm going to drop this patch from a re-post of the series today.
Base-PX, being EPON, requires additional layers (eg, MPMC on top of
the MAC to control transmission), and it's not clear whether the
SFP module takes care of this or whether the host has to.
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up
^ permalink raw reply
* Re: [PATCH net-next 4/7] net: phy: add paged phy register accessors
From: Russell King - ARM Linux @ 2018-01-02 10:50 UTC (permalink / raw)
To: Andrew Lunn; +Cc: Florian Fainelli, netdev
In-Reply-To: <20171231100714.GD10595@n2100.armlinux.org.uk>
On Sun, Dec 31, 2017 at 10:07:14AM +0000, Russell King - ARM Linux wrote:
> On Sun, Dec 31, 2017 at 09:10:39AM +0100, Andrew Lunn wrote:
> > > +/**
> > > + * phy_save_page() - take the bus lock and save the current page
> > > + * @phydev: a pointer to a &struct phy_device
> > > + *
> > > + * Take the MDIO bus lock, and return the current page number. On error,
> > > + * returns a negative errno. phy_restore_page() must be called after this
> > > + * to release the lock even on failure.
> > > + */
> > > +int phy_save_page(struct phy_device *phydev)
> > > +{
> > > + mutex_lock(&phydev->mdio.bus->mdio_lock);
> > > + return __phy_read_page(phydev);
> > > +}
> > > +EXPORT_SYMBOL_GPL(phy_save_page);
> > > +
> > > +/**
> > > + * phy_select_page() - take the bus lock, save the current page, and set a page
> > > + * @phydev: a pointer to a &struct phy_device
> > > + * @page: desired page
> > > + *
> > > + * Take the MDIO bus lock to protect against concurrent access, save the
> > > + * current PHY page, and set the current page. On error, returns a
> > > + * negative errno, otherwise returns the previous page number.
> > > + * phy_restore_page() must be called after this to restore the page
> > > + * number (if this call was successful) and release the lock.
> >
> > Hi Russell
> >
> > This comment seems wrong. It looks like you need to call
> > phy_restore_page() on error as well. I think the text in () should be
> > removed, and add the "even on failure" which the previous function
> > states.
>
> It's one of those slightly confusing bits of English, and I doubt
> there's a way to express it better.
>
> phy_restore_page() must be called after this to [restore the page
> number (if this call was successful) and] release the lock.
>
> I could use:
>
> phy_restore_page() must always be called to release the lock,
> and restore the page number if this call was successful.
>
> but that can still be read in an ambiguous manner.
I've decided to solve this by changing it to:
+ * phy_restore_page() must always be called after this, irrespective
+ * of success or failure of this call.
iow, not explaining /why/.
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up
^ permalink raw reply
* [PATCH net-next v2 0/7] Resolve races in phy accessors
From: Russell King - ARM Linux @ 2018-01-02 10:52 UTC (permalink / raw)
To: Andrew Lunn, Florian Fainelli; +Cc: netdev
Hi,
This series resolves races with various accesses to PHY registers.
The first five patches are necessary before we add phylink support
to mvneta, the remaining three are merely cleanups for unobserved
races, and hence are less critical.
There are two possible classes of races that can occur: where we
write to a page register that changes the meaning of a group of
other registers, and where we read-modify-write a register.
Resolve these races by performing the accesses under the mdio bus
lock, ensuring that no other user can access the bus while the
series of atomic operations are being performed.
These patches have been posted before, and have been modified
along the lines of previous feedback:
- The third patch was originally reviewed by Florian, but as I've
added __phy_modify() to it, I've removed that attributation.
- Included generic page-based accessors as suggested last time
around.
- Since we have the unlocked __phy_modify() in this patch series,
it is sensible to include the changes for this to marvell.c -
these accessors have to change anyway to avoid deadlocks on the
mdio bus lock.
I haven't been able to test the at803x.c changes yet beyond compile
testing - although I do have systems with an ar8035 PHY. However,
they should be straight forward to review.
This is targetted for net-next because the races have not been
found in existing drivers, but have been observed with phylink
integrated into mvneta - that's not to say that the races do not
exist today, they are just unobserved (probably through lack of
rigorous enough testing.) The race provoking condition is detailed
in patch 5.
drivers/net/phy/at803x.c | 20 +-
drivers/net/phy/marvell.c | 442 +++++++++++++++++--------------------------
drivers/net/phy/mdio_bus.c | 65 +++++--
drivers/net/phy/phy-core.c | 216 ++++++++++++++++++++-
drivers/net/phy/phy_device.c | 50 +----
include/linux/mdio.h | 3 +
include/linux/phy.h | 40 ++++
7 files changed, 490 insertions(+), 346 deletions(-)
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up
^ permalink raw reply
* [PATCH net-next v2 1/7] net: mdiobus: add unlocked accessors
From: Russell King @ 2018-01-02 10:58 UTC (permalink / raw)
To: Andrew Lunn, Florian Fainelli; +Cc: netdev
In-Reply-To: <20180102105218.GB21998@n2100.armlinux.org.uk>
Add unlocked versions of the bus accessors, which allows access to the
bus with all the tracing. These accessors validate that the bus mutex
is held, which is a basic requirement for all mii bus accesses.
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
drivers/net/phy/mdio_bus.c | 65 +++++++++++++++++++++++++++++++++++++---------
include/linux/mdio.h | 3 +++
2 files changed, 56 insertions(+), 12 deletions(-)
diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c
index 54d00a1d2bef..75be8be0d169 100644
--- a/drivers/net/phy/mdio_bus.c
+++ b/drivers/net/phy/mdio_bus.c
@@ -494,6 +494,55 @@ struct phy_device *mdiobus_scan(struct mii_bus *bus, int addr)
EXPORT_SYMBOL(mdiobus_scan);
/**
+ * __mdiobus_read - Unlocked version of the mdiobus_read function
+ * @bus: the mii_bus struct
+ * @addr: the phy address
+ * @regnum: register number to read
+ *
+ * Read a MDIO bus register. Caller must hold the mdio bus lock.
+ *
+ * NOTE: MUST NOT be called from interrupt context.
+ */
+int __mdiobus_read(struct mii_bus *bus, int addr, u32 regnum)
+{
+ int retval;
+
+ WARN_ON_ONCE(!mutex_is_locked(&bus->mdio_lock));
+
+ retval = bus->read(bus, addr, regnum);
+
+ trace_mdio_access(bus, 1, addr, regnum, retval, retval);
+
+ return retval;
+}
+EXPORT_SYMBOL(__mdiobus_read);
+
+/**
+ * __mdiobus_write - Unlocked version of the mdiobus_write function
+ * @bus: the mii_bus struct
+ * @addr: the phy address
+ * @regnum: register number to write
+ * @val: value to write to @regnum
+ *
+ * Write a MDIO bus register. Caller must hold the mdio bus lock.
+ *
+ * NOTE: MUST NOT be called from interrupt context.
+ */
+int __mdiobus_write(struct mii_bus *bus, int addr, u32 regnum, u16 val)
+{
+ int err;
+
+ WARN_ON_ONCE(!mutex_is_locked(&bus->mdio_lock));
+
+ err = bus->write(bus, addr, regnum, val);
+
+ trace_mdio_access(bus, 0, addr, regnum, val, err);
+
+ return err;
+}
+EXPORT_SYMBOL(__mdiobus_write);
+
+/**
* mdiobus_read_nested - Nested version of the mdiobus_read function
* @bus: the mii_bus struct
* @addr: the phy address
@@ -513,11 +562,9 @@ int mdiobus_read_nested(struct mii_bus *bus, int addr, u32 regnum)
BUG_ON(in_interrupt());
mutex_lock_nested(&bus->mdio_lock, MDIO_MUTEX_NESTED);
- retval = bus->read(bus, addr, regnum);
+ retval = __mdiobus_read(bus, addr, regnum);
mutex_unlock(&bus->mdio_lock);
- trace_mdio_access(bus, 1, addr, regnum, retval, retval);
-
return retval;
}
EXPORT_SYMBOL(mdiobus_read_nested);
@@ -539,11 +586,9 @@ int mdiobus_read(struct mii_bus *bus, int addr, u32 regnum)
BUG_ON(in_interrupt());
mutex_lock(&bus->mdio_lock);
- retval = bus->read(bus, addr, regnum);
+ retval = __mdiobus_read(bus, addr, regnum);
mutex_unlock(&bus->mdio_lock);
- trace_mdio_access(bus, 1, addr, regnum, retval, retval);
-
return retval;
}
EXPORT_SYMBOL(mdiobus_read);
@@ -569,11 +614,9 @@ int mdiobus_write_nested(struct mii_bus *bus, int addr, u32 regnum, u16 val)
BUG_ON(in_interrupt());
mutex_lock_nested(&bus->mdio_lock, MDIO_MUTEX_NESTED);
- err = bus->write(bus, addr, regnum, val);
+ err = __mdiobus_write(bus, addr, regnum, val);
mutex_unlock(&bus->mdio_lock);
- trace_mdio_access(bus, 0, addr, regnum, val, err);
-
return err;
}
EXPORT_SYMBOL(mdiobus_write_nested);
@@ -596,11 +639,9 @@ int mdiobus_write(struct mii_bus *bus, int addr, u32 regnum, u16 val)
BUG_ON(in_interrupt());
mutex_lock(&bus->mdio_lock);
- err = bus->write(bus, addr, regnum, val);
+ err = __mdiobus_write(bus, addr, regnum, val);
mutex_unlock(&bus->mdio_lock);
- trace_mdio_access(bus, 0, addr, regnum, val, err);
-
return err;
}
EXPORT_SYMBOL(mdiobus_write);
diff --git a/include/linux/mdio.h b/include/linux/mdio.h
index ca08ab16ecdc..34796e29c90c 100644
--- a/include/linux/mdio.h
+++ b/include/linux/mdio.h
@@ -257,6 +257,9 @@ static inline u16 ethtool_adv_to_mmd_eee_adv_t(u32 adv)
return reg;
}
+int __mdiobus_read(struct mii_bus *bus, int addr, u32 regnum);
+int __mdiobus_write(struct mii_bus *bus, int addr, u32 regnum, u16 val);
+
int mdiobus_read(struct mii_bus *bus, int addr, u32 regnum);
int mdiobus_read_nested(struct mii_bus *bus, int addr, u32 regnum);
int mdiobus_write(struct mii_bus *bus, int addr, u32 regnum, u16 val);
--
2.7.4
^ permalink raw reply related
* [PATCH net-next v2 2/7] net: phy: use unlocked accessors for indirect MMD accesses
From: Russell King @ 2018-01-02 10:58 UTC (permalink / raw)
To: Andrew Lunn, Florian Fainelli; +Cc: netdev
In-Reply-To: <20180102105218.GB21998@n2100.armlinux.org.uk>
Use unlocked accessors for indirect MMD accesses to clause 22 PHYs.
This permits tracing of these accesses.
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
drivers/net/phy/phy-core.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/net/phy/phy-core.c b/drivers/net/phy/phy-core.c
index 21f75ae244b3..83d32644cb4d 100644
--- a/drivers/net/phy/phy-core.c
+++ b/drivers/net/phy/phy-core.c
@@ -193,13 +193,14 @@ static void mmd_phy_indirect(struct mii_bus *bus, int phy_addr, int devad,
u16 regnum)
{
/* Write the desired MMD Devad */
- bus->write(bus, phy_addr, MII_MMD_CTRL, devad);
+ __mdiobus_write(bus, phy_addr, MII_MMD_CTRL, devad);
/* Write the desired MMD register address */
- bus->write(bus, phy_addr, MII_MMD_DATA, regnum);
+ __mdiobus_write(bus, phy_addr, MII_MMD_DATA, regnum);
/* Select the Function : DATA with no post increment */
- bus->write(bus, phy_addr, MII_MMD_CTRL, devad | MII_MMD_CTRL_NOINCR);
+ __mdiobus_write(bus, phy_addr, MII_MMD_CTRL,
+ devad | MII_MMD_CTRL_NOINCR);
}
/**
@@ -232,7 +233,7 @@ int phy_read_mmd(struct phy_device *phydev, int devad, u32 regnum)
mmd_phy_indirect(bus, phy_addr, devad, regnum);
/* Read the content of the MMD's selected register */
- val = bus->read(bus, phy_addr, MII_MMD_DATA);
+ val = __mdiobus_read(bus, phy_addr, MII_MMD_DATA);
mutex_unlock(&bus->mdio_lock);
}
return val;
@@ -271,7 +272,7 @@ int phy_write_mmd(struct phy_device *phydev, int devad, u32 regnum, u16 val)
mmd_phy_indirect(bus, phy_addr, devad, regnum);
/* Write the data into MMD's selected register */
- bus->write(bus, phy_addr, MII_MMD_DATA, val);
+ __mdiobus_write(bus, phy_addr, MII_MMD_DATA, val);
mutex_unlock(&bus->mdio_lock);
ret = 0;
--
2.7.4
^ permalink raw reply related
* [PATCH net-next v2 3/7] net: phy: add unlocked accessors
From: Russell King @ 2018-01-02 10:58 UTC (permalink / raw)
To: Andrew Lunn, Florian Fainelli; +Cc: netdev
In-Reply-To: <20180102105218.GB21998@n2100.armlinux.org.uk>
Add unlocked versions of the bus accessors, which allows access to the
bus with all the tracing. These accessors validate that the bus mutex
is held, which is a basic requirement for all mii bus accesses.
Also added is a read-modify-write unlocked accessor with the same
locking requirements.
Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
drivers/net/phy/phy-core.c | 25 +++++++++++++++++++++++++
include/linux/phy.h | 28 ++++++++++++++++++++++++++++
2 files changed, 53 insertions(+)
diff --git a/drivers/net/phy/phy-core.c b/drivers/net/phy/phy-core.c
index 83d32644cb4d..37c039da0c16 100644
--- a/drivers/net/phy/phy-core.c
+++ b/drivers/net/phy/phy-core.c
@@ -280,3 +280,28 @@ int phy_write_mmd(struct phy_device *phydev, int devad, u32 regnum, u16 val)
return ret;
}
EXPORT_SYMBOL(phy_write_mmd);
+
+/**
+ * __phy_modify() - Convenience function for modifying a PHY register
+ * @phydev: a pointer to a &struct phy_device
+ * @regnum: register number
+ * @mask: bit mask of bits to clear
+ * @set: bit mask of bits to set
+ *
+ * Unlocked helper function which allows a PHY register to be modified as
+ * new register value = (old register value & mask) | set
+ */
+int __phy_modify(struct phy_device *phydev, u32 regnum, u16 mask, u16 set)
+{
+ int ret, res;
+
+ ret = __phy_read(phydev, regnum);
+ if (ret >= 0) {
+ res = __phy_write(phydev, regnum, (ret & ~mask) | set);
+ if (res < 0)
+ ret = res;
+ }
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(__phy_modify);
diff --git a/include/linux/phy.h b/include/linux/phy.h
index 71d777fe6c3d..3dae5b7408b4 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -716,6 +716,18 @@ static inline int phy_read(struct phy_device *phydev, u32 regnum)
}
/**
+ * __phy_read - convenience function for reading a given PHY register
+ * @phydev: the phy_device struct
+ * @regnum: register number to read
+ *
+ * The caller must have taken the MDIO bus lock.
+ */
+static inline int __phy_read(struct phy_device *phydev, u32 regnum)
+{
+ return __mdiobus_read(phydev->mdio.bus, phydev->mdio.addr, regnum);
+}
+
+/**
* phy_write - Convenience function for writing a given PHY register
* @phydev: the phy_device struct
* @regnum: register number to write
@@ -731,6 +743,22 @@ static inline int phy_write(struct phy_device *phydev, u32 regnum, u16 val)
}
/**
+ * __phy_write - Convenience function for writing a given PHY register
+ * @phydev: the phy_device struct
+ * @regnum: register number to write
+ * @val: value to write to @regnum
+ *
+ * The caller must have taken the MDIO bus lock.
+ */
+static inline int __phy_write(struct phy_device *phydev, u32 regnum, u16 val)
+{
+ return __mdiobus_write(phydev->mdio.bus, phydev->mdio.addr, regnum,
+ val);
+}
+
+int __phy_modify(struct phy_device *phydev, u32 regnum, u16 mask, u16 set);
+
+/**
* phy_interrupt_is_valid - Convenience function for testing a given PHY irq
* @phydev: the phy_device struct
*
--
2.7.4
^ permalink raw reply related
* [PATCH net-next v2 4/7] net: phy: add paged phy register accessors
From: Russell King @ 2018-01-02 10:58 UTC (permalink / raw)
To: Andrew Lunn, Florian Fainelli; +Cc: netdev
In-Reply-To: <20180102105218.GB21998@n2100.armlinux.org.uk>
Add a set of paged phy register accessors which are inherently safe in
their design against other accesses interfering with the paged access.
Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
drivers/net/phy/phy-core.c | 157 +++++++++++++++++++++++++++++++++++++++++++++
include/linux/phy.h | 11 ++++
2 files changed, 168 insertions(+)
diff --git a/drivers/net/phy/phy-core.c b/drivers/net/phy/phy-core.c
index 37c039da0c16..412ed8ff50e2 100644
--- a/drivers/net/phy/phy-core.c
+++ b/drivers/net/phy/phy-core.c
@@ -305,3 +305,160 @@ int __phy_modify(struct phy_device *phydev, u32 regnum, u16 mask, u16 set)
return ret;
}
EXPORT_SYMBOL_GPL(__phy_modify);
+
+static int __phy_read_page(struct phy_device *phydev)
+{
+ return phydev->drv->read_page(phydev);
+}
+
+static int __phy_write_page(struct phy_device *phydev, int page)
+{
+ return phydev->drv->write_page(phydev, page);
+}
+
+/**
+ * phy_save_page() - take the bus lock and save the current page
+ * @phydev: a pointer to a &struct phy_device
+ *
+ * Take the MDIO bus lock, and return the current page number. On error,
+ * returns a negative errno. phy_restore_page() must always be called
+ * after this, irrespective of success or failure of this call.
+ */
+int phy_save_page(struct phy_device *phydev)
+{
+ mutex_lock(&phydev->mdio.bus->mdio_lock);
+ return __phy_read_page(phydev);
+}
+EXPORT_SYMBOL_GPL(phy_save_page);
+
+/**
+ * phy_select_page() - take the bus lock, save the current page, and set a page
+ * @phydev: a pointer to a &struct phy_device
+ * @page: desired page
+ *
+ * Take the MDIO bus lock to protect against concurrent access, save the
+ * current PHY page, and set the current page. On error, returns a
+ * negative errno, otherwise returns the previous page number.
+ * phy_restore_page() must always be called after this, irrespective
+ * of success or failure of this call.
+ */
+int phy_select_page(struct phy_device *phydev, int page)
+{
+ int ret, oldpage;
+
+ oldpage = ret = phy_save_page(phydev);
+ if (ret < 0)
+ return ret;
+
+ if (oldpage != page) {
+ ret = __phy_write_page(phydev, page);
+ if (ret < 0)
+ return ret;
+ }
+
+ return oldpage;
+}
+EXPORT_SYMBOL_GPL(phy_select_page);
+
+/**
+ * phy_restore_page() - restore the page register and release the bus lock
+ * @phydev: a pointer to a &struct phy_device
+ * @oldpage: the old page, return value from phy_save_page() or phy_select_page()
+ * @ret: operation's return code
+ *
+ * Release the MDIO bus lock, restoring @oldpage if it is a valid page.
+ * This function propagates the earliest error code from the group of
+ * operations.
+ *
+ * Returns:
+ * @oldpage if it was a negative value, otherwise
+ * @ret if it was a negative errno value, otherwise
+ * phy_write_page()'s negative value if it were in error, otherwise
+ * @ret.
+ */
+int phy_restore_page(struct phy_device *phydev, int oldpage, int ret)
+{
+ int r;
+
+ if (oldpage >= 0) {
+ r = __phy_write_page(phydev, oldpage);
+
+ /* Propagate the operation return code if the page write
+ * was successful.
+ */
+ if (ret >= 0 && r < 0)
+ ret = r;
+ } else {
+ /* Propagate the phy page selection error code */
+ ret = oldpage;
+ }
+
+ mutex_unlock(&phydev->mdio.bus->mdio_lock);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(phy_restore_page);
+
+/**
+ * phy_read_paged() - Convenience function for reading a paged register
+ * @phydev: a pointer to a &struct phy_device
+ * @page: the page for the phy
+ * @regnum: register number
+ *
+ * Same rules as for phy_read().
+ */
+int phy_read_paged(struct phy_device *phydev, int page, u32 regnum)
+{
+ int ret = 0, oldpage;
+
+ oldpage = phy_select_page(phydev, page);
+ if (oldpage >= 0)
+ ret = __phy_read(phydev, regnum);
+
+ return phy_restore_page(phydev, oldpage, ret);
+}
+EXPORT_SYMBOL(phy_read_paged);
+
+/**
+ * phy_write_paged() - Convenience function for writing a paged register
+ * @phydev: a pointer to a &struct phy_device
+ * @page: the page for the phy
+ * @regnum: register number
+ * @val: value to write
+ *
+ * Same rules as for phy_write().
+ */
+int phy_write_paged(struct phy_device *phydev, int page, u32 regnum, u16 val)
+{
+ int ret = 0, oldpage;
+
+ oldpage = phy_select_page(phydev, page);
+ if (oldpage >= 0)
+ ret = __phy_write(phydev, regnum, val);
+
+ return phy_restore_page(phydev, oldpage, ret);
+}
+EXPORT_SYMBOL(phy_write_paged);
+
+/**
+ * phy_modify_paged() - Convenience function for modifying a paged register
+ * @phydev: a pointer to a &struct phy_device
+ * @page: the page for the phy
+ * @regnum: register number
+ * @mask: bit mask of bits to clear
+ * @set: bit mask of bits to set
+ *
+ * Same rules as for phy_read() and phy_write().
+ */
+int phy_modify_paged(struct phy_device *phydev, int page, u32 regnum,
+ u16 mask, u16 set)
+{
+ int ret = 0, oldpage;
+
+ oldpage = phy_select_page(phydev, page);
+ if (oldpage >= 0)
+ ret = __phy_modify(phydev, regnum, mask, set);
+
+ return phy_restore_page(phydev, oldpage, ret);
+}
+EXPORT_SYMBOL(phy_modify_paged);
diff --git a/include/linux/phy.h b/include/linux/phy.h
index 3dae5b7408b4..f408220d95ec 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -634,6 +634,9 @@ struct phy_driver {
int (*write_mmd)(struct phy_device *dev, int devnum, u16 regnum,
u16 val);
+ int (*read_page)(struct phy_device *dev);
+ int (*write_page)(struct phy_device *dev, int page);
+
/* Get the size and type of the eeprom contained within a plug-in
* module */
int (*module_info)(struct phy_device *dev,
@@ -836,6 +839,14 @@ static inline bool phy_is_pseudo_fixed_link(struct phy_device *phydev)
*/
int phy_write_mmd(struct phy_device *phydev, int devad, u32 regnum, u16 val);
+int phy_save_page(struct phy_device *phydev);
+int phy_select_page(struct phy_device *phydev, int page);
+int phy_restore_page(struct phy_device *phydev, int oldpage, int ret);
+int phy_read_paged(struct phy_device *phydev, int page, u32 regnum);
+int phy_write_paged(struct phy_device *phydev, int page, u32 regnum, u16 val);
+int phy_modify_paged(struct phy_device *phydev, int page, u32 regnum,
+ u16 mask, u16 set);
+
struct phy_device *phy_device_create(struct mii_bus *bus, int addr, int phy_id,
bool is_c45,
struct phy_c45_device_ids *c45_ids);
--
2.7.4
^ permalink raw reply related
* [PATCH net-next v2 5/7] net: phy: marvell: fix paged access races
From: Russell King @ 2018-01-02 10:58 UTC (permalink / raw)
To: Andrew Lunn, Florian Fainelli; +Cc: netdev
In-Reply-To: <20180102105218.GB21998@n2100.armlinux.org.uk>
For paged accesses to be truely safe, we need to hold the bus lock to
prevent anyone else gaining access to the registers while we modify
them.
The phydev->lock mutex does not do this: userspace via the MII ioctl
can still sneak in and read or write any register while we are on a
different page, and the suspend/resume methods can be called by a
thread different to the thread polling the phy status.
Races have been observed with mvneta on SolidRun Clearfog with phylink,
particularly between the phylib worker reading the PHYs status, and
the thread resuming mvneta, calling phy_start() which then calls
through to m88e1121_config_aneg_rgmii_delays(), which tries to
read-modify-write the MSCR register:
CPU0 CPU1
marvell_read_status_page()
marvell_set_page(phydev, MII_MARVELL_FIBER_PAGE)
...
m88e1121_config_aneg_rgmii_delays()
set_page(MII_MARVELL_MSCR_PAGE)
phy_read(phydev, MII_88E1121_PHY_MSCR_REG)
marvell_set_page(phydev, MII_MARVELL_COPPER_PAGE);
...
phy_write(phydev, MII_88E1121_PHY_MSCR_REG)
The result of this is we end up writing the copper page register 21,
which causes the copper PHY to be disabled, and the link partner sees
the link immediately go down.
Solve this by taking the bus lock instead of the PHY lock, thereby
preventing other accesses to the PHY while we are accessing other PHY
pages.
Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
drivers/net/phy/marvell.c | 354 ++++++++++++++++++----------------------------
1 file changed, 137 insertions(+), 217 deletions(-)
diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c
index 82104edca393..1cb84064d658 100644
--- a/drivers/net/phy/marvell.c
+++ b/drivers/net/phy/marvell.c
@@ -83,7 +83,7 @@
#define MII_88E1121_PHY_MSCR_REG 21
#define MII_88E1121_PHY_MSCR_RX_DELAY BIT(5)
#define MII_88E1121_PHY_MSCR_TX_DELAY BIT(4)
-#define MII_88E1121_PHY_MSCR_DELAY_MASK (~(BIT(5) | BIT(4)))
+#define MII_88E1121_PHY_MSCR_DELAY_MASK (BIT(5) | BIT(4))
#define MII_88E1121_MISC_TEST 0x1a
#define MII_88E1510_MISC_TEST_TEMP_THRESHOLD_MASK 0x1f00
@@ -177,27 +177,19 @@ struct marvell_priv {
struct device *hwmon_dev;
};
-static int marvell_get_page(struct phy_device *phydev)
+static int marvell_read_page(struct phy_device *phydev)
{
- return phy_read(phydev, MII_MARVELL_PHY_PAGE);
+ return __phy_read(phydev, MII_MARVELL_PHY_PAGE);
}
-static int marvell_set_page(struct phy_device *phydev, int page)
+static int marvell_write_page(struct phy_device *phydev, int page)
{
- return phy_write(phydev, MII_MARVELL_PHY_PAGE, page);
+ return __phy_write(phydev, MII_MARVELL_PHY_PAGE, page);
}
-static int marvell_get_set_page(struct phy_device *phydev, int page)
+static int marvell_set_page(struct phy_device *phydev, int page)
{
- int oldpage = marvell_get_page(phydev);
-
- if (oldpage < 0)
- return oldpage;
-
- if (page != oldpage)
- return marvell_set_page(phydev, page);
-
- return 0;
+ return phy_write(phydev, MII_MARVELL_PHY_PAGE, page);
}
static int marvell_ack_interrupt(struct phy_device *phydev)
@@ -399,7 +391,7 @@ static int m88e1111_config_aneg(struct phy_device *phydev)
static int marvell_of_reg_init(struct phy_device *phydev)
{
const __be32 *paddr;
- int len, i, saved_page, current_page, ret;
+ int len, i, saved_page, current_page, ret = 0;
if (!phydev->mdio.dev.of_node)
return 0;
@@ -409,12 +401,11 @@ static int marvell_of_reg_init(struct phy_device *phydev)
if (!paddr || len < (4 * sizeof(*paddr)))
return 0;
- saved_page = marvell_get_page(phydev);
+ saved_page = phy_save_page(phydev);
if (saved_page < 0)
- return saved_page;
+ goto err;
current_page = saved_page;
- ret = 0;
len /= sizeof(*paddr);
for (i = 0; i < len - 3; i += 4) {
u16 page = be32_to_cpup(paddr + i);
@@ -425,14 +416,14 @@ static int marvell_of_reg_init(struct phy_device *phydev)
if (page != current_page) {
current_page = page;
- ret = marvell_set_page(phydev, page);
+ ret = marvell_write_page(phydev, page);
if (ret < 0)
goto err;
}
val = 0;
if (mask) {
- val = phy_read(phydev, reg);
+ val = __phy_read(phydev, reg);
if (val < 0) {
ret = val;
goto err;
@@ -441,17 +432,12 @@ static int marvell_of_reg_init(struct phy_device *phydev)
}
val |= val_bits;
- ret = phy_write(phydev, reg, val);
+ ret = __phy_write(phydev, reg, val);
if (ret < 0)
goto err;
}
err:
- if (current_page != saved_page) {
- i = marvell_set_page(phydev, saved_page);
- if (ret == 0)
- ret = i;
- }
- return ret;
+ return phy_restore_page(phydev, saved_page, ret);
}
#else
static int marvell_of_reg_init(struct phy_device *phydev)
@@ -462,34 +448,21 @@ static int marvell_of_reg_init(struct phy_device *phydev)
static int m88e1121_config_aneg_rgmii_delays(struct phy_device *phydev)
{
- int err, oldpage, mscr;
-
- oldpage = marvell_get_set_page(phydev, MII_MARVELL_MSCR_PAGE);
- if (oldpage < 0)
- return oldpage;
-
- mscr = phy_read(phydev, MII_88E1121_PHY_MSCR_REG);
- if (mscr < 0) {
- err = mscr;
- goto out;
- }
-
- mscr &= MII_88E1121_PHY_MSCR_DELAY_MASK;
+ int mscr;
if (phydev->interface == PHY_INTERFACE_MODE_RGMII_ID)
- mscr |= (MII_88E1121_PHY_MSCR_RX_DELAY |
- MII_88E1121_PHY_MSCR_TX_DELAY);
+ mscr = MII_88E1121_PHY_MSCR_RX_DELAY |
+ MII_88E1121_PHY_MSCR_TX_DELAY;
else if (phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID)
- mscr |= MII_88E1121_PHY_MSCR_RX_DELAY;
+ mscr = MII_88E1121_PHY_MSCR_RX_DELAY;
else if (phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID)
- mscr |= MII_88E1121_PHY_MSCR_TX_DELAY;
-
- err = phy_write(phydev, MII_88E1121_PHY_MSCR_REG, mscr);
-
-out:
- marvell_set_page(phydev, oldpage);
+ mscr = MII_88E1121_PHY_MSCR_TX_DELAY;
+ else
+ mscr = 0;
- return err;
+ return phy_modify_paged(phydev, MII_MARVELL_MSCR_PAGE,
+ MII_88E1121_PHY_MSCR_REG,
+ MII_88E1121_PHY_MSCR_DELAY_MASK, mscr);
}
static int m88e1121_config_aneg(struct phy_device *phydev)
@@ -515,20 +488,11 @@ static int m88e1121_config_aneg(struct phy_device *phydev)
static int m88e1318_config_aneg(struct phy_device *phydev)
{
- int err, oldpage, mscr;
-
- oldpage = marvell_get_set_page(phydev, MII_MARVELL_MSCR_PAGE);
- if (oldpage < 0)
- return oldpage;
-
- mscr = phy_read(phydev, MII_88E1318S_PHY_MSCR1_REG);
- mscr |= MII_88E1318S_PHY_MSCR1_PAD_ODD;
-
- err = phy_write(phydev, MII_88E1318S_PHY_MSCR1_REG, mscr);
- if (err < 0)
- return err;
+ int err;
- err = marvell_set_page(phydev, oldpage);
+ err = phy_modify_paged(phydev, MII_MARVELL_MSCR_PAGE,
+ MII_88E1318S_PHY_MSCR1_REG,
+ 0, MII_88E1318S_PHY_MSCR1_PAD_ODD);
if (err < 0)
return err;
@@ -854,20 +818,15 @@ static int m88e1111_config_init(struct phy_device *phydev)
static int m88e1121_config_init(struct phy_device *phydev)
{
- int err, oldpage;
-
- oldpage = marvell_get_set_page(phydev, MII_MARVELL_LED_PAGE);
- if (oldpage < 0)
- return oldpage;
+ int err;
/* Default PHY LED config: LED[0] .. Link, LED[1] .. Activity */
- err = phy_write(phydev, MII_88E1121_PHY_LED_CTRL,
- MII_88E1121_PHY_LED_DEF);
+ err = phy_write_paged(phydev, MII_MARVELL_LED_PAGE,
+ MII_88E1121_PHY_LED_CTRL,
+ MII_88E1121_PHY_LED_DEF);
if (err < 0)
return err;
- marvell_set_page(phydev, oldpage);
-
/* Set marvell,reg-init configuration from device tree */
return marvell_config_init(phydev);
}
@@ -1398,100 +1357,98 @@ static int m88e1121_did_interrupt(struct phy_device *phydev)
static void m88e1318_get_wol(struct phy_device *phydev,
struct ethtool_wolinfo *wol)
{
+ int oldpage, ret = 0;
+
wol->supported = WAKE_MAGIC;
wol->wolopts = 0;
- if (marvell_set_page(phydev, MII_MARVELL_WOL_PAGE) < 0)
- return;
+ oldpage = phy_select_page(phydev, MII_MARVELL_WOL_PAGE);
+ if (oldpage < 0)
+ goto error;
- if (phy_read(phydev, MII_88E1318S_PHY_WOL_CTRL) &
- MII_88E1318S_PHY_WOL_CTRL_MAGIC_PACKET_MATCH_ENABLE)
+ ret = __phy_read(phydev, MII_88E1318S_PHY_WOL_CTRL);
+ if (ret & MII_88E1318S_PHY_WOL_CTRL_MAGIC_PACKET_MATCH_ENABLE)
wol->wolopts |= WAKE_MAGIC;
- if (marvell_set_page(phydev, MII_MARVELL_COPPER_PAGE) < 0)
- return;
+error:
+ phy_restore_page(phydev, oldpage, ret);
}
static int m88e1318_set_wol(struct phy_device *phydev,
struct ethtool_wolinfo *wol)
{
- int err, oldpage, temp;
+ int err = 0, oldpage;
- oldpage = marvell_get_page(phydev);
+ oldpage = phy_save_page(phydev);
+ if (oldpage < 0)
+ goto error;
if (wol->wolopts & WAKE_MAGIC) {
/* Explicitly switch to page 0x00, just to be sure */
- err = marvell_set_page(phydev, MII_MARVELL_COPPER_PAGE);
+ err = marvell_write_page(phydev, MII_MARVELL_COPPER_PAGE);
if (err < 0)
- return err;
+ goto error;
/* Enable the WOL interrupt */
- temp = phy_read(phydev, MII_88E1318S_PHY_CSIER);
- temp |= MII_88E1318S_PHY_CSIER_WOL_EIE;
- err = phy_write(phydev, MII_88E1318S_PHY_CSIER, temp);
+ err = __phy_modify(phydev, MII_88E1318S_PHY_CSIER, 0,
+ MII_88E1318S_PHY_CSIER_WOL_EIE);
if (err < 0)
- return err;
+ goto error;
- err = marvell_set_page(phydev, MII_MARVELL_LED_PAGE);
+ err = marvell_write_page(phydev, MII_MARVELL_LED_PAGE);
if (err < 0)
- return err;
+ goto error;
/* Setup LED[2] as interrupt pin (active low) */
- temp = phy_read(phydev, MII_88E1318S_PHY_LED_TCR);
- temp &= ~MII_88E1318S_PHY_LED_TCR_FORCE_INT;
- temp |= MII_88E1318S_PHY_LED_TCR_INTn_ENABLE;
- temp |= MII_88E1318S_PHY_LED_TCR_INT_ACTIVE_LOW;
- err = phy_write(phydev, MII_88E1318S_PHY_LED_TCR, temp);
+ err = __phy_modify(phydev, MII_88E1318S_PHY_LED_TCR,
+ (u16)~MII_88E1318S_PHY_LED_TCR_FORCE_INT,
+ MII_88E1318S_PHY_LED_TCR_INTn_ENABLE |
+ MII_88E1318S_PHY_LED_TCR_INT_ACTIVE_LOW);
if (err < 0)
- return err;
+ goto error;
- err = marvell_set_page(phydev, MII_MARVELL_WOL_PAGE);
+ err = marvell_write_page(phydev, MII_MARVELL_WOL_PAGE);
if (err < 0)
- return err;
+ goto error;
/* Store the device address for the magic packet */
- err = phy_write(phydev, MII_88E1318S_PHY_MAGIC_PACKET_WORD2,
+ err = __phy_write(phydev, MII_88E1318S_PHY_MAGIC_PACKET_WORD2,
((phydev->attached_dev->dev_addr[5] << 8) |
phydev->attached_dev->dev_addr[4]));
if (err < 0)
- return err;
- err = phy_write(phydev, MII_88E1318S_PHY_MAGIC_PACKET_WORD1,
+ goto error;
+ err = __phy_write(phydev, MII_88E1318S_PHY_MAGIC_PACKET_WORD1,
((phydev->attached_dev->dev_addr[3] << 8) |
phydev->attached_dev->dev_addr[2]));
if (err < 0)
- return err;
- err = phy_write(phydev, MII_88E1318S_PHY_MAGIC_PACKET_WORD0,
+ goto error;
+ err = __phy_write(phydev, MII_88E1318S_PHY_MAGIC_PACKET_WORD0,
((phydev->attached_dev->dev_addr[1] << 8) |
phydev->attached_dev->dev_addr[0]));
if (err < 0)
- return err;
+ goto error;
/* Clear WOL status and enable magic packet matching */
- temp = phy_read(phydev, MII_88E1318S_PHY_WOL_CTRL);
- temp |= MII_88E1318S_PHY_WOL_CTRL_CLEAR_WOL_STATUS;
- temp |= MII_88E1318S_PHY_WOL_CTRL_MAGIC_PACKET_MATCH_ENABLE;
- err = phy_write(phydev, MII_88E1318S_PHY_WOL_CTRL, temp);
+ err = __phy_modify(phydev, MII_88E1318S_PHY_WOL_CTRL, 0,
+ MII_88E1318S_PHY_WOL_CTRL_CLEAR_WOL_STATUS |
+ MII_88E1318S_PHY_WOL_CTRL_MAGIC_PACKET_MATCH_ENABLE);
if (err < 0)
- return err;
+ goto error;
} else {
- err = marvell_set_page(phydev, MII_MARVELL_WOL_PAGE);
+ err = marvell_write_page(phydev, MII_MARVELL_WOL_PAGE);
if (err < 0)
- return err;
+ goto error;
/* Clear WOL status and disable magic packet matching */
- temp = phy_read(phydev, MII_88E1318S_PHY_WOL_CTRL);
- temp |= MII_88E1318S_PHY_WOL_CTRL_CLEAR_WOL_STATUS;
- temp &= ~MII_88E1318S_PHY_WOL_CTRL_MAGIC_PACKET_MATCH_ENABLE;
- err = phy_write(phydev, MII_88E1318S_PHY_WOL_CTRL, temp);
+ err = __phy_modify(phydev, MII_88E1318S_PHY_WOL_CTRL,
+ (u16)~MII_88E1318S_PHY_WOL_CTRL_MAGIC_PACKET_MATCH_ENABLE,
+ MII_88E1318S_PHY_WOL_CTRL_CLEAR_WOL_STATUS);
if (err < 0)
- return err;
+ goto error;
}
- err = marvell_set_page(phydev, oldpage);
- if (err < 0)
- return err;
-
- return 0;
+error:
+ return phy_restore_page(phydev, oldpage, err);
}
static int marvell_get_sset_count(struct phy_device *phydev)
@@ -1519,14 +1476,10 @@ static u64 marvell_get_stat(struct phy_device *phydev, int i)
{
struct marvell_hw_stat stat = marvell_hw_stats[i];
struct marvell_priv *priv = phydev->priv;
- int oldpage, val;
+ int val;
u64 ret;
- oldpage = marvell_get_set_page(phydev, stat.page);
- if (oldpage < 0)
- return UINT64_MAX;
-
- val = phy_read(phydev, stat.reg);
+ val = phy_read_paged(phydev, stat.page, stat.reg);
if (val < 0) {
ret = UINT64_MAX;
} else {
@@ -1535,8 +1488,6 @@ static u64 marvell_get_stat(struct phy_device *phydev, int i)
ret = priv->stats[i];
}
- marvell_set_page(phydev, oldpage);
-
return ret;
}
@@ -1553,51 +1504,44 @@ static void marvell_get_stats(struct phy_device *phydev,
static int m88e1121_get_temp(struct phy_device *phydev, long *temp)
{
int oldpage;
- int ret;
+ int ret = 0;
int val;
*temp = 0;
- mutex_lock(&phydev->lock);
-
- oldpage = marvell_get_set_page(phydev, MII_MARVELL_MISC_TEST_PAGE);
- if (oldpage < 0) {
- mutex_unlock(&phydev->lock);
- return oldpage;
- }
+ oldpage = phy_select_page(phydev, MII_MARVELL_MISC_TEST_PAGE);
+ if (oldpage < 0)
+ goto error;
/* Enable temperature sensor */
- ret = phy_read(phydev, MII_88E1121_MISC_TEST);
+ ret = __phy_read(phydev, MII_88E1121_MISC_TEST);
if (ret < 0)
goto error;
- ret = phy_write(phydev, MII_88E1121_MISC_TEST,
- ret | MII_88E1121_MISC_TEST_TEMP_SENSOR_EN);
+ ret = __phy_write(phydev, MII_88E1121_MISC_TEST,
+ ret | MII_88E1121_MISC_TEST_TEMP_SENSOR_EN);
if (ret < 0)
goto error;
/* Wait for temperature to stabilize */
usleep_range(10000, 12000);
- val = phy_read(phydev, MII_88E1121_MISC_TEST);
+ val = __phy_read(phydev, MII_88E1121_MISC_TEST);
if (val < 0) {
ret = val;
goto error;
}
/* Disable temperature sensor */
- ret = phy_write(phydev, MII_88E1121_MISC_TEST,
- ret & ~MII_88E1121_MISC_TEST_TEMP_SENSOR_EN);
+ ret = __phy_write(phydev, MII_88E1121_MISC_TEST,
+ ret & ~MII_88E1121_MISC_TEST_TEMP_SENSOR_EN);
if (ret < 0)
goto error;
*temp = ((val & MII_88E1121_MISC_TEST_TEMP_MASK) - 5) * 5000;
error:
- marvell_set_page(phydev, oldpage);
- mutex_unlock(&phydev->lock);
-
- return ret;
+ return phy_restore_page(phydev, oldpage, ret);
}
static int m88e1121_hwmon_read(struct device *dev,
@@ -1671,118 +1615,64 @@ static const struct hwmon_chip_info m88e1121_hwmon_chip_info = {
static int m88e1510_get_temp(struct phy_device *phydev, long *temp)
{
- int oldpage;
int ret;
*temp = 0;
- mutex_lock(&phydev->lock);
-
- oldpage = marvell_get_set_page(phydev, MII_MARVELL_MISC_TEST_PAGE);
- if (oldpage < 0) {
- mutex_unlock(&phydev->lock);
- return oldpage;
- }
-
- ret = phy_read(phydev, MII_88E1510_TEMP_SENSOR);
+ ret = phy_read_paged(phydev, MII_MARVELL_MISC_TEST_PAGE,
+ MII_88E1510_TEMP_SENSOR);
if (ret < 0)
- goto error;
+ return ret;
*temp = ((ret & MII_88E1510_TEMP_SENSOR_MASK) - 25) * 1000;
-error:
- marvell_set_page(phydev, oldpage);
- mutex_unlock(&phydev->lock);
-
- return ret;
+ return 0;
}
static int m88e1510_get_temp_critical(struct phy_device *phydev, long *temp)
{
- int oldpage;
int ret;
*temp = 0;
- mutex_lock(&phydev->lock);
-
- oldpage = marvell_get_set_page(phydev, MII_MARVELL_MISC_TEST_PAGE);
- if (oldpage < 0) {
- mutex_unlock(&phydev->lock);
- return oldpage;
- }
-
- ret = phy_read(phydev, MII_88E1121_MISC_TEST);
+ ret = phy_read_paged(phydev, MII_MARVELL_MISC_TEST_PAGE,
+ MII_88E1121_MISC_TEST);
if (ret < 0)
- goto error;
+ return ret;
*temp = (((ret & MII_88E1510_MISC_TEST_TEMP_THRESHOLD_MASK) >>
MII_88E1510_MISC_TEST_TEMP_THRESHOLD_SHIFT) * 5) - 25;
/* convert to mC */
*temp *= 1000;
-error:
- marvell_set_page(phydev, oldpage);
- mutex_unlock(&phydev->lock);
-
- return ret;
+ return 0;
}
static int m88e1510_set_temp_critical(struct phy_device *phydev, long temp)
{
- int oldpage;
- int ret;
-
- mutex_lock(&phydev->lock);
-
- oldpage = marvell_get_set_page(phydev, MII_MARVELL_MISC_TEST_PAGE);
- if (oldpage < 0) {
- mutex_unlock(&phydev->lock);
- return oldpage;
- }
-
- ret = phy_read(phydev, MII_88E1121_MISC_TEST);
- if (ret < 0)
- goto error;
-
temp = temp / 1000;
temp = clamp_val(DIV_ROUND_CLOSEST(temp, 5) + 5, 0, 0x1f);
- ret = phy_write(phydev, MII_88E1121_MISC_TEST,
- (ret & ~MII_88E1510_MISC_TEST_TEMP_THRESHOLD_MASK) |
- (temp << MII_88E1510_MISC_TEST_TEMP_THRESHOLD_SHIFT));
-
-error:
- marvell_set_page(phydev, oldpage);
- mutex_unlock(&phydev->lock);
- return ret;
+ return phy_modify_paged(phydev, MII_MARVELL_MISC_TEST_PAGE,
+ MII_88E1121_MISC_TEST,
+ MII_88E1510_MISC_TEST_TEMP_THRESHOLD_MASK,
+ temp << MII_88E1510_MISC_TEST_TEMP_THRESHOLD_SHIFT);
}
static int m88e1510_get_temp_alarm(struct phy_device *phydev, long *alarm)
{
- int oldpage;
int ret;
*alarm = false;
- mutex_lock(&phydev->lock);
-
- oldpage = marvell_get_set_page(phydev, MII_MARVELL_MISC_TEST_PAGE);
- if (oldpage < 0) {
- mutex_unlock(&phydev->lock);
- return oldpage;
- }
-
- ret = phy_read(phydev, MII_88E1121_MISC_TEST);
+ ret = phy_read_paged(phydev, MII_MARVELL_MISC_TEST_PAGE,
+ MII_88E1121_MISC_TEST);
if (ret < 0)
- goto error;
- *alarm = !!(ret & MII_88E1510_MISC_TEST_TEMP_IRQ);
+ return ret;
-error:
- marvell_set_page(phydev, oldpage);
- mutex_unlock(&phydev->lock);
+ *alarm = !!(ret & MII_88E1510_MISC_TEST_TEMP_IRQ);
- return ret;
+ return 0;
}
static int m88e1510_hwmon_read(struct device *dev,
@@ -1979,6 +1869,8 @@ static struct phy_driver marvell_drivers[] = {
.config_intr = &marvell_config_intr,
.resume = &genphy_resume,
.suspend = &genphy_suspend,
+ .read_page = marvell_read_page,
+ .write_page = marvell_write_page,
.get_sset_count = marvell_get_sset_count,
.get_strings = marvell_get_strings,
.get_stats = marvell_get_stats,
@@ -1997,6 +1889,8 @@ static struct phy_driver marvell_drivers[] = {
.config_intr = &marvell_config_intr,
.resume = &genphy_resume,
.suspend = &genphy_suspend,
+ .read_page = marvell_read_page,
+ .write_page = marvell_write_page,
.get_sset_count = marvell_get_sset_count,
.get_strings = marvell_get_strings,
.get_stats = marvell_get_stats,
@@ -2015,6 +1909,8 @@ static struct phy_driver marvell_drivers[] = {
.config_intr = &marvell_config_intr,
.resume = &genphy_resume,
.suspend = &genphy_suspend,
+ .read_page = marvell_read_page,
+ .write_page = marvell_write_page,
.get_sset_count = marvell_get_sset_count,
.get_strings = marvell_get_strings,
.get_stats = marvell_get_stats,
@@ -2033,6 +1929,8 @@ static struct phy_driver marvell_drivers[] = {
.config_intr = &marvell_config_intr,
.resume = &genphy_resume,
.suspend = &genphy_suspend,
+ .read_page = marvell_read_page,
+ .write_page = marvell_write_page,
.get_sset_count = marvell_get_sset_count,
.get_strings = marvell_get_strings,
.get_stats = marvell_get_stats,
@@ -2052,6 +1950,8 @@ static struct phy_driver marvell_drivers[] = {
.did_interrupt = &m88e1121_did_interrupt,
.resume = &genphy_resume,
.suspend = &genphy_suspend,
+ .read_page = marvell_read_page,
+ .write_page = marvell_write_page,
.get_sset_count = marvell_get_sset_count,
.get_strings = marvell_get_strings,
.get_stats = marvell_get_stats,
@@ -2073,6 +1973,8 @@ static struct phy_driver marvell_drivers[] = {
.set_wol = &m88e1318_set_wol,
.resume = &genphy_resume,
.suspend = &genphy_suspend,
+ .read_page = marvell_read_page,
+ .write_page = marvell_write_page,
.get_sset_count = marvell_get_sset_count,
.get_strings = marvell_get_strings,
.get_stats = marvell_get_stats,
@@ -2091,6 +1993,8 @@ static struct phy_driver marvell_drivers[] = {
.config_intr = &marvell_config_intr,
.resume = &genphy_resume,
.suspend = &genphy_suspend,
+ .read_page = marvell_read_page,
+ .write_page = marvell_write_page,
.get_sset_count = marvell_get_sset_count,
.get_strings = marvell_get_strings,
.get_stats = marvell_get_stats,
@@ -2109,6 +2013,8 @@ static struct phy_driver marvell_drivers[] = {
.config_intr = &marvell_config_intr,
.resume = &genphy_resume,
.suspend = &genphy_suspend,
+ .read_page = marvell_read_page,
+ .write_page = marvell_write_page,
.get_sset_count = marvell_get_sset_count,
.get_strings = marvell_get_strings,
.get_stats = marvell_get_stats,
@@ -2127,6 +2033,8 @@ static struct phy_driver marvell_drivers[] = {
.config_intr = &marvell_config_intr,
.resume = &genphy_resume,
.suspend = &genphy_suspend,
+ .read_page = marvell_read_page,
+ .write_page = marvell_write_page,
.get_sset_count = marvell_get_sset_count,
.get_strings = marvell_get_strings,
.get_stats = marvell_get_stats,
@@ -2145,6 +2053,8 @@ static struct phy_driver marvell_drivers[] = {
.config_intr = &marvell_config_intr,
.resume = &genphy_resume,
.suspend = &genphy_suspend,
+ .read_page = marvell_read_page,
+ .write_page = marvell_write_page,
.get_sset_count = marvell_get_sset_count,
.get_strings = marvell_get_strings,
.get_stats = marvell_get_stats,
@@ -2166,6 +2076,8 @@ static struct phy_driver marvell_drivers[] = {
.set_wol = &m88e1318_set_wol,
.resume = &marvell_resume,
.suspend = &marvell_suspend,
+ .read_page = marvell_read_page,
+ .write_page = marvell_write_page,
.get_sset_count = marvell_get_sset_count,
.get_strings = marvell_get_strings,
.get_stats = marvell_get_stats,
@@ -2186,6 +2098,8 @@ static struct phy_driver marvell_drivers[] = {
.did_interrupt = &m88e1121_did_interrupt,
.resume = &genphy_resume,
.suspend = &genphy_suspend,
+ .read_page = marvell_read_page,
+ .write_page = marvell_write_page,
.get_sset_count = marvell_get_sset_count,
.get_strings = marvell_get_strings,
.get_stats = marvell_get_stats,
@@ -2205,6 +2119,8 @@ static struct phy_driver marvell_drivers[] = {
.did_interrupt = &m88e1121_did_interrupt,
.resume = &genphy_resume,
.suspend = &genphy_suspend,
+ .read_page = marvell_read_page,
+ .write_page = marvell_write_page,
.get_sset_count = marvell_get_sset_count,
.get_strings = marvell_get_strings,
.get_stats = marvell_get_stats,
@@ -2225,6 +2141,8 @@ static struct phy_driver marvell_drivers[] = {
.did_interrupt = &m88e1121_did_interrupt,
.resume = &genphy_resume,
.suspend = &genphy_suspend,
+ .read_page = marvell_read_page,
+ .write_page = marvell_write_page,
.get_sset_count = marvell_get_sset_count,
.get_strings = marvell_get_strings,
.get_stats = marvell_get_stats,
@@ -2244,6 +2162,8 @@ static struct phy_driver marvell_drivers[] = {
.did_interrupt = &m88e1121_did_interrupt,
.resume = &genphy_resume,
.suspend = &genphy_suspend,
+ .read_page = marvell_read_page,
+ .write_page = marvell_write_page,
.get_sset_count = marvell_get_sset_count,
.get_strings = marvell_get_strings,
.get_stats = marvell_get_stats,
--
2.7.4
^ permalink raw reply related
* [PATCH net-next v2 6/7] net: phy: add phy_modify() accessor
From: Russell King @ 2018-01-02 10:58 UTC (permalink / raw)
To: Andrew Lunn, Florian Fainelli; +Cc: netdev
In-Reply-To: <20180102105218.GB21998@n2100.armlinux.org.uk>
Add phy_modify() convenience accessor to complement the mdiobus
counterpart.
Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
drivers/net/phy/phy-core.c | 23 +++++++++++++++++++++++
include/linux/phy.h | 1 +
2 files changed, 24 insertions(+)
diff --git a/drivers/net/phy/phy-core.c b/drivers/net/phy/phy-core.c
index 412ed8ff50e2..a2168adea81f 100644
--- a/drivers/net/phy/phy-core.c
+++ b/drivers/net/phy/phy-core.c
@@ -306,6 +306,29 @@ int __phy_modify(struct phy_device *phydev, u32 regnum, u16 mask, u16 set)
}
EXPORT_SYMBOL_GPL(__phy_modify);
+/**
+ * phy_modify - Convenience function for modifying a given PHY register
+ * @phydev: the phy_device struct
+ * @regnum: register number to write
+ * @mask: bit mask of bits to clear
+ * @set: new value of bits set in mask to write to @regnum
+ *
+ * NOTE: MUST NOT be called from interrupt context,
+ * because the bus read/write functions may wait for an interrupt
+ * to conclude the operation.
+ */
+int phy_modify(struct phy_device *phydev, u32 regnum, u16 mask, u16 set)
+{
+ int ret;
+
+ mutex_lock(&phydev->mdio.bus->mdio_lock);
+ ret = __phy_modify(phydev, regnum, mask, set);
+ mutex_unlock(&phydev->mdio.bus->mdio_lock);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(phy_modify);
+
static int __phy_read_page(struct phy_device *phydev)
{
return phydev->drv->read_page(phydev);
diff --git a/include/linux/phy.h b/include/linux/phy.h
index f408220d95ec..0ee4ece312da 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -760,6 +760,7 @@ static inline int __phy_write(struct phy_device *phydev, u32 regnum, u16 val)
}
int __phy_modify(struct phy_device *phydev, u32 regnum, u16 mask, u16 set);
+int phy_modify(struct phy_device *phydev, u32 regnum, u16 mask, u16 set);
/**
* phy_interrupt_is_valid - Convenience function for testing a given PHY irq
--
2.7.4
^ permalink raw reply related
* [PATCH net-next v2 7/7] net: phy: convert read-modify-write to phy_modify()
From: Russell King @ 2018-01-02 10:58 UTC (permalink / raw)
To: Andrew Lunn, Florian Fainelli; +Cc: netdev
In-Reply-To: <20180102105218.GB21998@n2100.armlinux.org.uk>
Convert read-modify-write sequences in at803x, Marvell and core phylib
to use phy_modify() to ensure safety.
Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
drivers/net/phy/at803x.c | 20 ++--------
drivers/net/phy/marvell.c | 88 +++++++++++++++++---------------------------
drivers/net/phy/phy_device.c | 50 +++++--------------------
3 files changed, 46 insertions(+), 112 deletions(-)
diff --git a/drivers/net/phy/at803x.c b/drivers/net/phy/at803x.c
index e911e4990b20..e86c1b8b1b51 100644
--- a/drivers/net/phy/at803x.c
+++ b/drivers/net/phy/at803x.c
@@ -216,34 +216,22 @@ static int at803x_suspend(struct phy_device *phydev)
int value;
int wol_enabled;
- mutex_lock(&phydev->lock);
-
value = phy_read(phydev, AT803X_INTR_ENABLE);
wol_enabled = value & AT803X_INTR_ENABLE_WOL;
- value = phy_read(phydev, MII_BMCR);
-
if (wol_enabled)
- value |= BMCR_ISOLATE;
+ value = BMCR_ISOLATE;
else
- value |= BMCR_PDOWN;
+ value = BMCR_PDOWN;
- phy_write(phydev, MII_BMCR, value);
-
- mutex_unlock(&phydev->lock);
+ phy_modify(phydev, MII_BMCR, 0, value);
return 0;
}
static int at803x_resume(struct phy_device *phydev)
{
- int value;
-
- value = phy_read(phydev, MII_BMCR);
- value &= ~(BMCR_PDOWN | BMCR_ISOLATE);
- phy_write(phydev, MII_BMCR, value);
-
- return 0;
+ return phy_modify(phydev, MII_BMCR, ~(BMCR_PDOWN | BMCR_ISOLATE), 0);
}
static int at803x_probe(struct phy_device *phydev)
diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c
index 1cb84064d658..8212c2fd7fe1 100644
--- a/drivers/net/phy/marvell.c
+++ b/drivers/net/phy/marvell.c
@@ -471,7 +471,7 @@ static int m88e1121_config_aneg(struct phy_device *phydev)
if (phy_interface_is_rgmii(phydev)) {
err = m88e1121_config_aneg_rgmii_delays(phydev);
- if (err)
+ if (err < 0)
return err;
}
@@ -664,19 +664,14 @@ static int m88e1116r_config_init(struct phy_device *phydev)
static int m88e3016_config_init(struct phy_device *phydev)
{
- int reg;
+ int ret;
/* Enable Scrambler and Auto-Crossover */
- reg = phy_read(phydev, MII_88E3016_PHY_SPEC_CTRL);
- if (reg < 0)
- return reg;
-
- reg &= ~MII_88E3016_DISABLE_SCRAMBLER;
- reg |= MII_88E3016_AUTO_MDIX_CROSSOVER;
-
- reg = phy_write(phydev, MII_88E3016_PHY_SPEC_CTRL, reg);
- if (reg < 0)
- return reg;
+ ret = phy_modify(phydev, MII_88E3016_PHY_SPEC_CTRL,
+ ~MII_88E3016_DISABLE_SCRAMBLER,
+ MII_88E3016_AUTO_MDIX_CROSSOVER);
+ if (ret < 0)
+ return ret;
return marvell_config_init(phydev);
}
@@ -685,42 +680,34 @@ static int m88e1111_config_init_hwcfg_mode(struct phy_device *phydev,
u16 mode,
int fibre_copper_auto)
{
- int temp;
-
- temp = phy_read(phydev, MII_M1111_PHY_EXT_SR);
- if (temp < 0)
- return temp;
-
- temp &= ~(MII_M1111_HWCFG_MODE_MASK |
- MII_M1111_HWCFG_FIBER_COPPER_AUTO |
- MII_M1111_HWCFG_FIBER_COPPER_RES);
- temp |= mode;
-
if (fibre_copper_auto)
- temp |= MII_M1111_HWCFG_FIBER_COPPER_AUTO;
+ mode |= MII_M1111_HWCFG_FIBER_COPPER_AUTO;
- return phy_write(phydev, MII_M1111_PHY_EXT_SR, temp);
+ return phy_modify(phydev, MII_M1111_PHY_EXT_SR,
+ (u16)~(MII_M1111_HWCFG_MODE_MASK |
+ MII_M1111_HWCFG_FIBER_COPPER_AUTO |
+ MII_M1111_HWCFG_FIBER_COPPER_RES),
+ mode);
}
static int m88e1111_config_init_rgmii_delays(struct phy_device *phydev)
{
- int temp;
-
- temp = phy_read(phydev, MII_M1111_PHY_EXT_CR);
- if (temp < 0)
- return temp;
+ int delay;
if (phydev->interface == PHY_INTERFACE_MODE_RGMII_ID) {
- temp |= (MII_M1111_RGMII_RX_DELAY | MII_M1111_RGMII_TX_DELAY);
+ delay = MII_M1111_RGMII_RX_DELAY | MII_M1111_RGMII_TX_DELAY;
} else if (phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID) {
- temp &= ~MII_M1111_RGMII_TX_DELAY;
- temp |= MII_M1111_RGMII_RX_DELAY;
+ delay = MII_M1111_RGMII_RX_DELAY;
} else if (phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID) {
- temp &= ~MII_M1111_RGMII_RX_DELAY;
- temp |= MII_M1111_RGMII_TX_DELAY;
+ delay = MII_M1111_RGMII_TX_DELAY;
+ } else {
+ delay = 0;
}
- return phy_write(phydev, MII_M1111_PHY_EXT_CR, temp);
+ return phy_modify(phydev, MII_M1111_PHY_EXT_CR,
+ (u16)~(MII_M1111_RGMII_RX_DELAY |
+ MII_M1111_RGMII_TX_DELAY),
+ delay);
}
static int m88e1111_config_init_rgmii(struct phy_device *phydev)
@@ -766,7 +753,7 @@ static int m88e1111_config_init_rtbi(struct phy_device *phydev)
int err;
err = m88e1111_config_init_rgmii_delays(phydev);
- if (err)
+ if (err < 0)
return err;
err = m88e1111_config_init_hwcfg_mode(
@@ -793,7 +780,7 @@ static int m88e1111_config_init(struct phy_device *phydev)
if (phy_interface_is_rgmii(phydev)) {
err = m88e1111_config_init_rgmii(phydev);
- if (err)
+ if (err < 0)
return err;
}
@@ -834,7 +821,6 @@ static int m88e1121_config_init(struct phy_device *phydev)
static int m88e1510_config_init(struct phy_device *phydev)
{
int err;
- int temp;
/* SGMII-to-Copper mode initialization */
if (phydev->interface == PHY_INTERFACE_MODE_SGMII) {
@@ -846,16 +832,15 @@ static int m88e1510_config_init(struct phy_device *phydev)
return err;
/* In reg 20, write MODE[2:0] = 0x1 (SGMII to Copper) */
- temp = phy_read(phydev, MII_88E1510_GEN_CTRL_REG_1);
- temp &= ~MII_88E1510_GEN_CTRL_REG_1_MODE_MASK;
- temp |= MII_88E1510_GEN_CTRL_REG_1_MODE_SGMII;
- err = phy_write(phydev, MII_88E1510_GEN_CTRL_REG_1, temp);
+ err = phy_modify(phydev, MII_88E1510_GEN_CTRL_REG_1,
+ ~MII_88E1510_GEN_CTRL_REG_1_MODE_MASK,
+ MII_88E1510_GEN_CTRL_REG_1_MODE_SGMII);
if (err < 0)
return err;
/* PHY reset is necessary after changing MODE[2:0] */
- temp |= MII_88E1510_GEN_CTRL_REG_1_RESET;
- err = phy_write(phydev, MII_88E1510_GEN_CTRL_REG_1, temp);
+ err = phy_modify(phydev, MII_88E1510_GEN_CTRL_REG_1, 0,
+ MII_88E1510_GEN_CTRL_REG_1_RESET);
if (err < 0)
return err;
@@ -961,7 +946,6 @@ static int m88e1149_config_init(struct phy_device *phydev)
static int m88e1145_config_init_rgmii(struct phy_device *phydev)
{
- int temp;
int err;
err = m88e1111_config_init_rgmii_delays(phydev);
@@ -973,15 +957,9 @@ static int m88e1145_config_init_rgmii(struct phy_device *phydev)
if (err < 0)
return err;
- temp = phy_read(phydev, 0x1e);
- if (temp < 0)
- return temp;
-
- temp &= 0xf03f;
- temp |= 2 << 9; /* 36 ohm */
- temp |= 2 << 6; /* 39 ohm */
-
- err = phy_write(phydev, 0x1e, temp);
+ err = phy_modify(phydev, 0x1e, 0xf03f,
+ 2 << 9 | /* 36 ohm */
+ 2 << 6); /* 39 ohm */
if (err < 0)
return err;
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index b15b31ca2618..e5ddc5ae56d1 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -1328,9 +1328,8 @@ static int genphy_config_eee_advert(struct phy_device *phydev)
*/
int genphy_setup_forced(struct phy_device *phydev)
{
- int ctl = phy_read(phydev, MII_BMCR);
+ u16 ctl = 0;
- ctl &= BMCR_LOOPBACK | BMCR_ISOLATE | BMCR_PDOWN;
phydev->pause = 0;
phydev->asym_pause = 0;
@@ -1342,7 +1341,8 @@ int genphy_setup_forced(struct phy_device *phydev)
if (DUPLEX_FULL == phydev->duplex)
ctl |= BMCR_FULLDPLX;
- return phy_write(phydev, MII_BMCR, ctl);
+ return phy_modify(phydev, MII_BMCR,
+ BMCR_LOOPBACK | BMCR_ISOLATE | BMCR_PDOWN, ctl);
}
EXPORT_SYMBOL(genphy_setup_forced);
@@ -1352,17 +1352,9 @@ EXPORT_SYMBOL(genphy_setup_forced);
*/
int genphy_restart_aneg(struct phy_device *phydev)
{
- int ctl = phy_read(phydev, MII_BMCR);
-
- if (ctl < 0)
- return ctl;
-
- ctl |= BMCR_ANENABLE | BMCR_ANRESTART;
-
/* Don't isolate the PHY if we're negotiating */
- ctl &= ~BMCR_ISOLATE;
-
- return phy_write(phydev, MII_BMCR, ctl);
+ return phy_modify(phydev, MII_BMCR, ~BMCR_ISOLATE,
+ BMCR_ANENABLE | BMCR_ANRESTART);
}
EXPORT_SYMBOL(genphy_restart_aneg);
@@ -1628,44 +1620,20 @@ EXPORT_SYMBOL(genphy_config_init);
int genphy_suspend(struct phy_device *phydev)
{
- int value;
-
- mutex_lock(&phydev->lock);
-
- value = phy_read(phydev, MII_BMCR);
- phy_write(phydev, MII_BMCR, value | BMCR_PDOWN);
-
- mutex_unlock(&phydev->lock);
-
- return 0;
+ return phy_modify(phydev, MII_BMCR, 0, BMCR_PDOWN);
}
EXPORT_SYMBOL(genphy_suspend);
int genphy_resume(struct phy_device *phydev)
{
- int value;
-
- value = phy_read(phydev, MII_BMCR);
- phy_write(phydev, MII_BMCR, value & ~BMCR_PDOWN);
-
- return 0;
+ return phy_modify(phydev, MII_BMCR, ~BMCR_PDOWN, 0);
}
EXPORT_SYMBOL(genphy_resume);
int genphy_loopback(struct phy_device *phydev, bool enable)
{
- int value;
-
- value = phy_read(phydev, MII_BMCR);
- if (value < 0)
- return value;
-
- if (enable)
- value |= BMCR_LOOPBACK;
- else
- value &= ~BMCR_LOOPBACK;
-
- return phy_write(phydev, MII_BMCR, value);
+ return phy_modify(phydev, MII_BMCR, ~BMCR_LOOPBACK,
+ enable ? BMCR_LOOPBACK : 0);
}
EXPORT_SYMBOL(genphy_loopback);
--
2.7.4
^ permalink raw reply related
* Re: [PATCH] flex_can: Correct the checking for frame length in flexcan_start_xmit()
From: Oliver Hartkopp @ 2018-01-02 11:17 UTC (permalink / raw)
To: Luu An Phu, wg, mkl; +Cc: linux-can, netdev, stefan-gabriel.mirea
In-Reply-To: <1514864658-13194-1-git-send-email-phu.luuan@nxp.com>
On 01/02/2018 04:44 AM, Luu An Phu wrote:
> From: "phu.luuan" <phu.luuan@nxp.com>
>
> The flexcan_start_xmit() function compares the frame length with data register
> length to write frame content into data[0] and data[1] register. Data register
> length is 4 bytes and frame maximum length is 8 bytes.
>
> Fix the check that compares frame length with 3. Because the register length
> is 4.
>
> Signed-off-by: Luu An Phu <phu.luuan@nxp.com>
Reviewed-by: Oliver Hartkopp <socketcan@hartkopp.net>
Thanks for this improvement!
Best regards,
Oliver
> ---
> drivers/net/can/flexcan.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
> index 0626dcf..760d2c0 100644
> --- a/drivers/net/can/flexcan.c
> +++ b/drivers/net/can/flexcan.c
> @@ -526,7 +526,7 @@ static int flexcan_start_xmit(struct sk_buff *skb, struct net_device *dev)
> data = be32_to_cpup((__be32 *)&cf->data[0]);
> flexcan_write(data, &priv->tx_mb->data[0]);
> }
> - if (cf->can_dlc > 3) {
> + if (cf->can_dlc > 4) {
> data = be32_to_cpup((__be32 *)&cf->data[4]);
> flexcan_write(data, &priv->tx_mb->data[1]);
> }
>
^ permalink raw reply
* Re: [PATCH 0/2] tests: fix issues in autopkgtest environment
From: Luca Boccassi @ 2018-01-02 11:22 UTC (permalink / raw)
To: Christian Ehrhardt, Netdev
In-Reply-To: <1514878118-21009-1-git-send-email-christian.ehrhardt@canonical.com>
[-- Attachment #1: Type: text/plain, Size: 748 bytes --]
On Tue, 2018-01-02 at 08:28 +0100, Christian Ehrhardt wrote:
> Hi,
> while working on Debian bug [1] and the Ubuntu counterpart of it
> I found that the tests can throw a broken pipe warning.
>
> I kept the associated check-and-retry of the length separate to
> discuss the changes individually - feel free to squash them on
> commit if preferred.
>
> [1]: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=879854
>
> Christian Ehrhardt (2):
> tests: read limited amount from /dev/urandom
> tests: make sure rand_dev suffix has 6 chars
>
> testsuite/lib/generic.sh | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
Series acked-by: Luca Boccassi <bluca@debian.org>
--
Kind regards,
Luca Boccassi
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH 0/5] VSOCK: add vsock_test test suite
From: Stefan Hajnoczi @ 2018-01-02 12:05 UTC (permalink / raw)
To: Jorgen S. Hansen; +Cc: netdev@vger.kernel.org, Dexuan Cui
In-Reply-To: <1597F5EA-15E6-41A1-9C67-B4E560160957@vmware.com>
[-- Attachment #1: Type: text/plain, Size: 3303 bytes --]
On Wed, Dec 20, 2017 at 02:48:43PM +0000, Jorgen S. Hansen wrote:
>
> > On Dec 13, 2017, at 3:49 PM, Stefan Hajnoczi <stefanha@redhat.com> wrote:
> >
> > The vsock_diag.ko module already has a test suite but the core AF_VSOCK
> > functionality has no tests. This patch series adds several test cases that
> > exercise AF_VSOCK SOCK_STREAM socket semantics (send/recv, connect/accept,
> > half-closed connections, simultaneous connections).
> >
> > The test suite is modest but I hope to cover additional cases in the future.
> > My goal is to have a shared test suite so VMCI, Hyper-V, and KVM can ensure
> > that our transports behave the same.
> >
> > I have tested virtio-vsock.
> >
> > Jorgen: Please test the VMCI transport and let me know if anything needs to be
> > adjusted. See tools/testing/vsock/README for information on how to run the
> > test suite.
> >
>
> I tried running the vsock_test on VMCI, and all the tests failed in one way or
> another:
Great, thank you for testing and looking into the failures!
> 1) connection reset test: when the guest tries to connect to the host, we
> get EINVAL as the error instead of ECONNRESET. I’ll fix that.
Yay, the tests found a bug!
> 2) client close and server close tests: On the host side, VMCI doesn’t
> support reading data from a socket that has been closed by the
> guest. When the guest closes a connection, all data is gone, and
> we return EOF on the host side. So the tests that try to read data
> after close, should not attempt that on VMCI host side. I got the
> tests to pass by adding a getsockname call to determine if
> the local CID was the host CID, and then skip the read attempt
> in that case. We could add a vmci flag, that would enable
> this behavior.
Interesting behavior. Is there a reason for disallowing half-closed
sockets on the host side?
> 3) send_byte(fd, -EPIPE): for the VMCI transport, the close
> isn’t necessarily visible immediately on the peer. So in most
> cases, these send operations would complete with success.
> I was running these tests using nested virtualization, so I
> suspect that the problem is more likely to occur here, but
> I had to add a sleep to be sure to get the EPIPE error.
Good point, you've discovered a race condition that affects all
transports. The vsock close state transition might not have occurred
yet when the TCP control channel receives the "CLOSED" message.
test_stream_client_close_server() needs to wait for the socket status to
change before attempting send_byte(fd, -EPIPE). I guess I'll have to
use vsock_diag or another kernel interface to check the socket's state.
> 5) multiple connections tests: with the standard socket sizes,
> VMCI is only able to support about 100 concurrent stream
> connections so this test passes with MULTICONN_NFDS
> set to 100.
The 1000 magic number is because many distros have a maximum number of
file descriptors ulimit of 1024. But it's an arbitrary number and we
could lower it to 100.
Is this VMCI concurrent stream limit a denial of service vector? Can an
unprivileged guest userspace process open many sockets to prevent
legitimate connections from other users within the same guest?
Stefan
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]
^ permalink raw reply
* Re: [PATCH v6 1/6] can: dev: Add support for limiting configured bitrate
From: Marc Kleine-Budde @ 2018-01-02 13:00 UTC (permalink / raw)
To: Faiz Abbas, wg, robh+dt, mark.rutland
Cc: linux-can, netdev, devicetree, linux-kernel, nsekhar, fcooper,
robh, Wenyou.Yang, sergei.shtylyov
In-Reply-To: <1513949488-13026-2-git-send-email-faiz_abbas@ti.com>
[-- Attachment #1.1: Type: text/plain, Size: 5470 bytes --]
On 12/22/2017 02:31 PM, Faiz Abbas wrote:
> From: Franklin S Cooper Jr <fcooper@ti.com>
>
> Various CAN or CAN-FD IP may be able to run at a faster rate than
> what the transceiver the CAN node is connected to. This can lead to
> unexpected errors. However, CAN transceivers typically have fixed
> limitations and provide no means to discover these limitations at
> runtime. Therefore, add support for a can-transceiver node that
> can be reused by other CAN peripheral drivers to determine for both
> CAN and CAN-FD what the max bitrate that can be used. If the user
> tries to configure CAN to pass these maximum bitrates it will throw
> an error.
>
> Signed-off-by: Franklin S Cooper Jr <fcooper@ti.com>
> [nsekhar@ti.com: fix build error with !CONFIG_OF]
> Signed-off-by: Sekhar Nori <nsekhar@ti.com>
> Signed-off-by: Faiz Abbas <faiz_abbas@ti.com>
> ---
> v6 changes:
> fix build error with !CONFIG_OF
>
> drivers/net/can/dev.c | 39 +++++++++++++++++++++++++++++++++++++++
> include/linux/can/dev.h | 8 ++++++++
> 2 files changed, 47 insertions(+)
>
> diff --git a/drivers/net/can/dev.c b/drivers/net/can/dev.c
> index 365a8cc..007cfc0 100644
> --- a/drivers/net/can/dev.c
> +++ b/drivers/net/can/dev.c
> @@ -27,6 +27,7 @@
> #include <linux/can/skb.h>
> #include <linux/can/netlink.h>
> #include <linux/can/led.h>
> +#include <linux/of.h>
> #include <net/rtnetlink.h>
>
> #define MOD_DESC "CAN device driver interface"
> @@ -814,6 +815,30 @@ int open_candev(struct net_device *dev)
> }
> EXPORT_SYMBOL_GPL(open_candev);
>
> +#ifdef CONFIG_OF
> +/*
> + * Common function that can be used to understand the limitation of
> + * a transceiver when it provides no means to determine these limitations
> + * at runtime.
> + */
> +void of_can_transceiver(struct net_device *dev)
> +{
> + struct device_node *dn;
> + struct can_priv *priv = netdev_priv(dev);
> + struct device_node *np = dev->dev.parent->of_node;
> + int ret;
> +
> + dn = of_get_child_by_name(np, "can-transceiver");
> + if (!dn)
> + return;
> +
> + ret = of_property_read_u32(dn, "max-bitrate", &priv->max_bitrate);
> + if ((ret && ret != -EINVAL) || (!ret && !priv->max_bitrate))
> + netdev_warn(dev, "Invalid value for transceiver max bitrate. Ignoring bitrate limit.\n");
> +}
> +EXPORT_SYMBOL_GPL(of_can_transceiver);
> +#endif
> +
> /*
> * Common close function for cleanup before the device gets closed.
> *
> @@ -913,6 +938,13 @@ static int can_changelink(struct net_device *dev, struct nlattr *tb[],
> priv->bitrate_const_cnt);
> if (err)
> return err;
> +
> + if (priv->max_bitrate && bt.bitrate > priv->max_bitrate) {
> + netdev_err(dev, "arbitration bitrate surpasses transceiver capabilities of %d bps\n",
> + priv->max_bitrate);
> + return -EINVAL;
> + }
What about movong this check into can_get_bittiming()? Although we loose
the "arbitration" vs "canfd data".
> +
> memcpy(&priv->bittiming, &bt, sizeof(bt));
>
> if (priv->do_set_bittiming) {
> @@ -997,6 +1029,13 @@ static int can_changelink(struct net_device *dev, struct nlattr *tb[],
> priv->data_bitrate_const_cnt);
> if (err)
> return err;
> +
> + if (priv->max_bitrate && dbt.bitrate > priv->max_bitrate) {
> + netdev_err(dev, "canfd data bitrate surpasses transceiver capabilities of %d bps\n",
> + priv->max_bitrate);
> + return -EINVAL;
> + }
> +
> memcpy(&priv->data_bittiming, &dbt, sizeof(dbt));
>
> if (priv->do_set_data_bittiming) {
> diff --git a/include/linux/can/dev.h b/include/linux/can/dev.h
> index 61f1cf2..fbb7810 100644
> --- a/include/linux/can/dev.h
> +++ b/include/linux/can/dev.h
> @@ -48,6 +48,8 @@ struct can_priv {
> unsigned int data_bitrate_const_cnt;
> struct can_clock clock;
>
> + unsigned int max_bitrate;
Please make it an u32, name it "bitrate_max" and ...
>> struct can_priv {
>> struct net_device *dev;
>> struct can_device_stats can_stats;
>>
>> struct can_bittiming bittiming, data_bittiming;
>> const struct can_bittiming_const *bittiming_const,
>> *data_bittiming_const;
>> const u16 *termination_const;
>> unsigned int termination_const_cnt;
>> u16 termination;
>> const u32 *bitrate_const;
>> unsigned int bitrate_const_cnt;
>> const u32 *data_bitrate_const;
>> unsigned int data_bitrate_const_cnt;
...move it here.
>> struct can_clock clock;
>
> +
> enum can_state state;
>
> /* CAN controller features - see include/uapi/linux/can/netlink.h */
> @@ -166,6 +168,12 @@ void can_put_echo_skb(struct sk_buff *skb, struct net_device *dev,
> unsigned int can_get_echo_skb(struct net_device *dev, unsigned int idx);
> void can_free_echo_skb(struct net_device *dev, unsigned int idx);
>
> +#ifdef CONFIG_OF
> +void of_can_transceiver(struct net_device *dev);
> +#else
> +static inline void of_can_transceiver(struct net_device *dev) { }
> +#endif
> +
> struct sk_buff *alloc_can_skb(struct net_device *dev, struct can_frame **cf);
> struct sk_buff *alloc_canfd_skb(struct net_device *dev,
> struct canfd_frame **cfd);
>
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox