* [PATCH v2 net-next 2/4] net: dsa: lan9303: Multiply by 4 to get MDIO register
From: Egil Hjelmeland @ 2017-07-30 17:58 UTC (permalink / raw)
To: andrew, vivien.didelot, f.fainelli, netdev, linux-kernel, kernel
Cc: Egil Hjelmeland
In-Reply-To: <20170730175856.4784-1-privat@egil-hjelmeland.no>
lan9303_mdio_write()/_read() must multiply register number by 4 to get
offset.
Added some commments to the register definitions.
Signed-off-by: Egil Hjelmeland <privat@egil-hjelmeland.no>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Reviewed-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/net/dsa/lan9303-core.c | 6 ++++++
drivers/net/dsa/lan9303_mdio.c | 2 ++
2 files changed, 8 insertions(+)
diff --git a/drivers/net/dsa/lan9303-core.c b/drivers/net/dsa/lan9303-core.c
index 9d0ab77edb4a..96ebeb9bd59a 100644
--- a/drivers/net/dsa/lan9303-core.c
+++ b/drivers/net/dsa/lan9303-core.c
@@ -20,6 +20,9 @@
#include "lan9303.h"
+/* 13.2 System Control and Status Registers
+ * Multiply register number by 4 to get address offset.
+ */
#define LAN9303_CHIP_REV 0x14
# define LAN9303_CHIP_ID 0x9303
#define LAN9303_IRQ_CFG 0x15
@@ -53,6 +56,9 @@
#define LAN9303_VIRT_PHY_BASE 0x70
#define LAN9303_VIRT_SPECIAL_CTRL 0x77
+/*13.4 Switch Fabric Control and Status Registers
+ * Accessed indirectly via SWITCH_CSR_CMD, SWITCH_CSR_DATA.
+ */
#define LAN9303_SW_DEV_ID 0x0000
#define LAN9303_SW_RESET 0x0001
#define LAN9303_SW_RESET_RESET BIT(0)
diff --git a/drivers/net/dsa/lan9303_mdio.c b/drivers/net/dsa/lan9303_mdio.c
index 93c36c0541cf..2db7970fc88c 100644
--- a/drivers/net/dsa/lan9303_mdio.c
+++ b/drivers/net/dsa/lan9303_mdio.c
@@ -40,6 +40,7 @@ static int lan9303_mdio_write(void *ctx, uint32_t reg, uint32_t val)
{
struct lan9303_mdio *sw_dev = (struct lan9303_mdio *)ctx;
+ reg <<= 2; /* reg num to offset */
mutex_lock(&sw_dev->device->bus->mdio_lock);
lan9303_mdio_real_write(sw_dev->device, reg, val & 0xffff);
lan9303_mdio_real_write(sw_dev->device, reg + 2, (val >> 16) & 0xffff);
@@ -57,6 +58,7 @@ static int lan9303_mdio_read(void *ctx, uint32_t reg, uint32_t *val)
{
struct lan9303_mdio *sw_dev = (struct lan9303_mdio *)ctx;
+ reg <<= 2; /* reg num to offset */
mutex_lock(&sw_dev->device->bus->mdio_lock);
*val = lan9303_mdio_real_read(sw_dev->device, reg);
*val |= (lan9303_mdio_real_read(sw_dev->device, reg + 2) << 16);
--
2.11.0
^ permalink raw reply related
* Fw: [Bug 196533] New: kernel stack infoleaks
From: Stephen Hemminger @ 2017-07-30 18:03 UTC (permalink / raw)
To: samuel; +Cc: netdev
Begin forwarded message:
Date: Sun, 30 Jul 2017 05:13:08 +0000
From: bugzilla-daemon@bugzilla.kernel.org
To: stephen@networkplumber.org
Subject: [Bug 196533] New: kernel stack infoleaks
https://bugzilla.kernel.org/show_bug.cgi?id=196533
Bug ID: 196533
Summary: kernel stack infoleaks
Product: Networking
Version: 2.5
Kernel Version: 4.12.2
Hardware: All
OS: Linux
Tree: Mainline
Status: NEW
Severity: normal
Priority: P1
Component: Other
Assignee: stephen@networkplumber.org
Reporter: sohu0106@126.com
Regression: No
bug in net/irda/af_irda.c
Sometimes irda_getsockopt() doesn't initialize all members of list field of
irda_device_list struct. This structure is then copied to
userland. It leads to leaking of contents of kernel stack memory. We have to
initialize them to zero , or it will allows local users to obtain potentially
sensitive information from kernel stack memory by reading a copy of this
structure
https://github.com/torvalds/linux/pull/440
--
You are receiving this mail because:
You are the assignee for the bug.
^ permalink raw reply
* Re: [PATCH net-next v12 1/4] net netlink: Add new type NLA_BITFIELD32
From: Jiri Pirko @ 2017-07-30 18:42 UTC (permalink / raw)
To: Jamal Hadi Salim
Cc: davem, netdev, xiyou.wangcong, eric.dumazet, horms, dsahern
In-Reply-To: <1501435492-28301-2-git-send-email-jhs@emojatatu.com>
Sun, Jul 30, 2017 at 07:24:49PM CEST, jhs@mojatatu.com wrote:
>From: Jamal Hadi Salim <jhs@mojatatu.com>
>
>Generic bitflags attribute content sent to the kernel by user.
>With this netlink attr type the user can either set or unset a
>flag in the kernel.
>
>The value is a bitmap that defines the bit values being set
>The selector is a bitmask that defines which value bit is to be
>considered.
>
>A check is made to ensure the rules that a kernel subsystem always
>conforms to bitflags the kernel already knows about. i.e
>if the user tries to set a bit flag that is not understood then
>the _it will be rejected_.
>
>In the most basic form, the user specifies the attribute policy as:
>[ATTR_GOO] = { .type = NLA_BITFIELD32, .validation_data = &myvalidflags },
>
>where myvalidflags is the bit mask of the flags the kernel understands.
>
>If the user _does not_ provide myvalidflags then the attribute will
>also be rejected.
>
>Examples:
>value = 0x0, and selector = 0x1
>implies we are selecting bit 1 and we want to set its value to 0.
>
>value = 0x2, and selector = 0x2
>implies we are selecting bit 2 and we want to set its value to 1.
>
>Suggested-by: Jiri Pirko <jiri@mellanox.com>
>Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
>---
> include/net/netlink.h | 16 ++++++++++++++++
> include/uapi/linux/netlink.h | 17 +++++++++++++++++
> lib/nlattr.c | 30 ++++++++++++++++++++++++++++++
> 3 files changed, 63 insertions(+)
>
>diff --git a/include/net/netlink.h b/include/net/netlink.h
>index ef8e6c3..82dd298 100644
>--- a/include/net/netlink.h
>+++ b/include/net/netlink.h
>@@ -178,6 +178,7 @@ enum {
> NLA_S16,
> NLA_S32,
> NLA_S64,
>+ NLA_BITFIELD32,
> __NLA_TYPE_MAX,
> };
>
>@@ -206,6 +207,7 @@ enum {
> * NLA_MSECS Leaving the length field zero will verify the
> * given type fits, using it verifies minimum length
> * just like "All other"
>+ * NLA_BITFIELD32 A 32-bit bitmap/bitselector attribute
> * All other Minimum length of attribute payload
> *
> * Example:
>@@ -213,11 +215,13 @@ enum {
> * [ATTR_FOO] = { .type = NLA_U16 },
> * [ATTR_BAR] = { .type = NLA_STRING, .len = BARSIZ },
> * [ATTR_BAZ] = { .len = sizeof(struct mystruct) },
>+ * [ATTR_GOO] = { .type = NLA_BITFIELD32, .validation_data = &myvalidflags },
Checkpatch warns you about the line to long, please wrap it.
Btw, I did not see you reached a consensus with DavidA regarding this.
Did I miss it?
> * };
> */
> struct nla_policy {
> u16 type;
> u16 len;
>+ void *validation_data;
> };
>
> /**
>@@ -1203,6 +1207,18 @@ static inline struct in6_addr nla_get_in6_addr(const struct nlattr *nla)
> }
>
> /**
>+ * nla_get_bitfield32 - return payload of 32 bitfield attribute
>+ * @nla: nla_bitfield32 attribute
>+ */
>+static inline struct nla_bitfield32 nla_get_bitfield32(const struct nlattr *nla)
>+{
>+ struct nla_bitfield32 tmp;
>+
>+ nla_memcpy(&tmp, nla, sizeof(tmp));
>+ return tmp;
>+}
>+
>+/**
> * nla_memdup - duplicate attribute memory (kmemdup)
> * @src: netlink attribute to duplicate from
> * @gfp: GFP mask
>diff --git a/include/uapi/linux/netlink.h b/include/uapi/linux/netlink.h
>index f86127a..f4fc9c9 100644
>--- a/include/uapi/linux/netlink.h
>+++ b/include/uapi/linux/netlink.h
>@@ -226,5 +226,22 @@ struct nlattr {
> #define NLA_ALIGN(len) (((len) + NLA_ALIGNTO - 1) & ~(NLA_ALIGNTO - 1))
> #define NLA_HDRLEN ((int) NLA_ALIGN(sizeof(struct nlattr)))
>
>+/* Generic 32 bitflags attribute content sent to the kernel.
>+ *
>+ * The value is a bitmap that defines the values being set
>+ * The selector is a bitmask that defines which value is legit
>+ *
>+ * Examples:
>+ * value = 0x0, and selector = 0x1
>+ * implies we are selecting bit 1 and we want to set its value to 0.
>+ *
>+ * value = 0x2, and selector = 0x2
>+ * implies we are selecting bit 2 and we want to set its value to 1.
>+ *
>+ */
>+struct nla_bitfield32 {
>+ __u32 value;
>+ __u32 selector;
>+};
>
> #endif /* _UAPI__LINUX_NETLINK_H */
>diff --git a/lib/nlattr.c b/lib/nlattr.c
>index fb52435..ee79b7a 100644
>--- a/lib/nlattr.c
>+++ b/lib/nlattr.c
>@@ -27,6 +27,30 @@
> [NLA_S64] = sizeof(s64),
> };
>
>+static int validate_nla_bitfield32(const struct nlattr *nla,
>+ u32 *valid_flags_allowed)
>+{
>+ const struct nla_bitfield32 *bf = nla_data(nla);
>+ u32 *valid_flags_mask = valid_flags_allowed;
I pointed this out already. This weird.
You do *u32 = *u32, just with different name. Just use valid_flags_allowed
directly.
>+
>+ if (!valid_flags_allowed)
>+ return -EINVAL;
>+
>+ /*disallow invalid bit selector */
Fix all the comments in this function. Should be
/* something */
with spaces in front and at the end.
>+ if (bf->selector & ~*valid_flags_mask)
>+ return -EINVAL;
>+
>+ /*disallow invalid bit values */
>+ if (bf->value & ~*valid_flags_mask)
>+ return -EINVAL;
>+
>+ /*disallow valid bit values that are not selected*/
>+ if (bf->value & ~bf->selector)
>+ return -EINVAL;
>+
>+ return 0;
>+}
>+
> static int validate_nla(const struct nlattr *nla, int maxtype,
> const struct nla_policy *policy)
> {
>@@ -46,6 +70,12 @@ static int validate_nla(const struct nlattr *nla, int maxtype,
> return -ERANGE;
> break;
>
>+ case NLA_BITFIELD32:
>+ if (attrlen != sizeof(struct nla_bitfield32))
>+ return -ERANGE;
>+
>+ return validate_nla_bitfield32(nla, pt->validation_data);
>+
> case NLA_NUL_STRING:
> if (pt->len)
> minlen = min_t(int, attrlen, pt->len + 1);
>--
>1.9.1
>
^ permalink raw reply
* Re: [PATCH net-next v12 2/4] net sched actions: Use proper root attribute table for actions
From: Jiri Pirko @ 2017-07-30 18:44 UTC (permalink / raw)
To: Jamal Hadi Salim
Cc: davem, netdev, xiyou.wangcong, eric.dumazet, horms, dsahern
In-Reply-To: <1501435492-28301-3-git-send-email-jhs@emojatatu.com>
Sun, Jul 30, 2017 at 07:24:50PM CEST, jhs@mojatatu.com wrote:
>From: Jamal Hadi Salim <jhs@mojatatu.com>
>
>Bug fix for an issue which has been around for about a decade.
>We got away with it because the enumeration was larger than needed.
>
>Fixes: 7ba699c604ab ("[NET_SCHED]: Convert actions from rtnetlink to new netlink API")
>Suggested-by: Jiri Pirko <jiri@mellanox.com>
>Reviewed-by: Simon Horman <simon.horman@netronome.com>
>Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
Reviewed-by: Jiri Pirko <jiri@mellanox.com>
^ permalink raw reply
* Re: [patch net-next 01/20] net: sched: sch_atm: use Qdisc_class_common structure
From: Jamal Hadi Salim @ 2017-07-30 19:04 UTC (permalink / raw)
To: Jiri Pirko, netdev; +Cc: davem, xiyou.wangcong, daniel, mlxsw
In-Reply-To: <20170728144042.6380-2-jiri@resnulli.us>
On 17-07-28 10:40 AM, Jiri Pirko wrote:
> From: Jiri Pirko <jiri@mellanox.com>
>
> Even if it is only for classid now, use this common struct a be aligned
> with the rest of the classful qdiscs.
>
> Signed-off-by: Jiri Pirko <jiri@mellanox.com>
Looks good to me.
Acked-by: Jamal Hadi Salim <jhs@mojatatu.com>
cheers,
jamal
^ permalink raw reply
* Re: [PATCH net-next v12 3/4] net sched actions: dump more than TCA_ACT_MAX_PRIO actions per batch
From: Jiri Pirko @ 2017-07-30 19:05 UTC (permalink / raw)
To: Jamal Hadi Salim
Cc: davem, netdev, xiyou.wangcong, eric.dumazet, horms, dsahern
In-Reply-To: <1501435492-28301-4-git-send-email-jhs@emojatatu.com>
Sun, Jul 30, 2017 at 07:24:51PM CEST, jhs@mojatatu.com wrote:
>From: Jamal Hadi Salim <jhs@mojatatu.com>
>
>When you dump hundreds of thousands of actions, getting only 32 per
>dump batch even when the socket buffer and memory allocations allow
>is inefficient.
>
>With this change, the user will get as many as possibly fitting
>within the given constraints available to the kernel.
>
>The top level action TLV space is extended. An attribute
>TCA_ROOT_FLAGS is used to carry flags; flag TCA_FLAG_LARGE_DUMP_ON
>is set by the user indicating the user is capable of processing
>these large dumps. Older user space which doesnt set this flag
>doesnt get the large (than 32) batches.
>The kernel uses the TCA_ROOT_COUNT attribute to tell the user how many
>actions are put in a single batch. As such user space app knows how long
>to iterate (independent of the type of action being dumped)
>instead of hardcoded maximum of 32 thus maintaining backward compat.
>
>Some results dumping 1.5M actions below:
>first an unpatched tc which doesnt understand these features...
>
>prompt$ time -p tc actions ls action gact | grep index | wc -l
>1500000
>real 1388.43
>user 2.07
>sys 1386.79
>
>Now lets see a patched tc which sets the correct flags when requesting
>a dump:
>
>prompt$ time -p updatedtc actions ls action gact | grep index | wc -l
>1500000
>real 178.13
>user 2.02
>sys 176.96
>
>That is about 8x performance improvement for tc app which sets its
>receive buffer to about 32K.
>
>Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
If DavidA is ok with the "validation_data", I am fine with this patch.
Reviewed-by: Jiri Pirko <jiri@mellanox.com>
^ permalink raw reply
* Re: [PATCH net-next v12 4/4] net sched actions: add time filter for action dumping
From: Jiri Pirko @ 2017-07-30 19:06 UTC (permalink / raw)
To: Jamal Hadi Salim
Cc: davem, netdev, xiyou.wangcong, eric.dumazet, horms, dsahern
In-Reply-To: <1501435492-28301-5-git-send-email-jhs@emojatatu.com>
Sun, Jul 30, 2017 at 07:24:52PM CEST, jhs@mojatatu.com wrote:
>From: Jamal Hadi Salim <jhs@mojatatu.com>
>
>This patch adds support for filtering based on time since last used.
>When we are dumping a large number of actions it is useful to
>have the option of filtering based on when the action was last
>used to reduce the amount of data crossing to user space.
>
>With this patch the user space app sets the TCA_ROOT_TIME_DELTA
>attribute with the value in milliseconds with "time of interest
>since now". The kernel converts this to jiffies and does the
>filtering comparison matching entries that have seen activity
>since then and returns them to user space.
>Old kernels and old tc continue to work in legacy mode since
>they dont specify this attribute.
>
>Some example (we have 400 actions bound to 400 filters); at
>installation time. Using updated when tc setting the time of
>interest to 120 seconds earlier (we see 400 actions):
>prompt$ hackedtc actions ls action gact since 120000| grep index | wc -l
>400
>
>go get some coffee and wait for > 120 seconds and try again:
>
>prompt$ hackedtc actions ls action gact since 120000 | grep index | wc -l
>0
>
>Lets see a filter bound to one of these actions:
>....
>filter pref 10 u32
>filter pref 10 u32 fh 800: ht divisor 1
>filter pref 10 u32 fh 800::800 order 2048 key ht 800 bkt 0 flowid 1:10 (rule hit 2 success 1)
> match 7f000002/ffffffff at 12 (success 1 )
> action order 1: gact action pass
> random type none pass val 0
> index 23 ref 2 bind 1 installed 1145 sec used 802 sec
> Action statistics:
> Sent 84 bytes 1 pkt (dropped 0, overlimits 0 requeues 0)
> backlog 0b 0p requeues 0
>....
>
>that coffee took long, no? It was good.
>
>Now lets ping -c 1 127.0.0.2, then run the actions again:
>prompt$ hackedtc actions ls action gact since 120 | grep index | wc -l
>1
>
>More details please:
>prompt$ hackedtc -s actions ls action gact since 120000
>
> action order 0: gact action pass
> random type none pass val 0
> index 23 ref 2 bind 1 installed 1270 sec used 30 sec
> Action statistics:
> Sent 168 bytes 2 pkt (dropped 0, overlimits 0 requeues 0)
> backlog 0b 0p requeues 0
>
>And the filter?
>
>filter pref 10 u32
>filter pref 10 u32 fh 800: ht divisor 1
>filter pref 10 u32 fh 800::800 order 2048 key ht 800 bkt 0 flowid 1:10 (rule hit 4 success 2)
> match 7f000002/ffffffff at 12 (success 2 )
> action order 1: gact action pass
> random type none pass val 0
> index 23 ref 2 bind 1 installed 1324 sec used 84 sec
> Action statistics:
> Sent 168 bytes 2 pkt (dropped 0, overlimits 0 requeues 0)
> backlog 0b 0p requeues 0
>
>Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
Reviewed-by: Jiri Pirko <jiri@mellanox.com>
^ permalink raw reply
* [PATCH 0/1 v2] constify nf_conntrack_l3/4proto parameters
From: Julia Lawall @ 2017-07-30 19:38 UTC (permalink / raw)
To: Pablo Neira Ayuso
Cc: bhumirks, kernel-janitors, Jozsef Kadlecsik, Florian Westphal,
David S. Miller, netfilter-devel, coreteam, netdev, linux-kernel
When a nf_conntrack_l3/4proto parameter is not on the left hand side
of an assignment, its address is not taken, and it is not passed to a
function that may modify its fields, then it can be declared as const.
This change is useful from a documentation point of view, and can
possibly facilitate making some nf_conntrack_l4proto structures const
subsequently.
Done with the help of Coccinelle. The following semantic patch shows
the nf_conntrack_l4proto case.
// <smpl>
virtual update_results
virtual after_start
@initialize:ocaml@
@@
let unsafe = Hashtbl.create 101
let is_unsafe f = Hashtbl.mem unsafe f
let changed = ref false
(* The next three rules relate to the fact that we do not know the type of
void * variables. Fortunately this is only neede on the first iteration,
but it still means that the whole kernel will end up being considered. *)
@has depends on !after_start && !update_results@
identifier f,x;
position p;
@@
f@p(...,struct nf_conntrack_l3proto *x,...) { ... }
@hasa depends on !after_start@
identifier f,x;
position p;
@@
f@p(...,struct nf_conntrack_l3proto *x[],...) { ... }
@others depends on !after_start && !update_results@
position p != {has.p,hasa.p};
identifier f,x;
@@
f@p(...,void *x,...) { ... }
@script:ocaml@
f << others.f;
@@
changed := true;
Hashtbl.add unsafe f ()
@fpb depends on !update_results disable optional_qualifier, drop_cast exists@
identifier f : script:ocaml() { not(is_unsafe(f)) };
identifier x,fld;
identifier bad : script:ocaml() { is_unsafe(bad) };
assignment operator aop;
expression e;
local idexpression fp;
type T;
@@
f(...,struct nf_conntrack_l3proto *x,...)
{
...
(
return x;
|
(<+...x...+>) aop e
|
e aop x
|
(T)x
|
&(<+...x...+>)
|
bad(...,x,...)
|
fp(...,x,...)
|
(<+...e->fld...+>)(...,x,...)
)
...when any
}
@script:ocaml@
f << fpb.f;
@@
changed := true;
Printf.eprintf "%s is unsafe\n" f;
Hashtbl.add unsafe f ()
@fpba depends on !update_results disable optional_qualifier, drop_cast exists@
identifier f : script:ocaml() { not(is_unsafe(f)) };
identifier x,fld;
identifier bad : script:ocaml() { is_unsafe(bad) };
assignment operator aop;
expression e;
local idexpression fp;
type T;
@@
f(...,struct nf_conntrack_l3proto *x[],...)
{
...
(
return \(x\|x[...]\);
|
(<+...x...+>) aop e
|
e aop \(x\|x[...]\)
|
(T)\(x\|x[...]\)
|
&(<+...x...+>)
|
bad(...,\(x\|x[...]\),...)
|
fp(...,\(x\|x[...]\),...)
|
(<+...e->fld...+>)(...,\(x\|x[...]\),...)
)
... when any
}
@script:ocaml@
f << fpba.f;
@@
changed := true;
Printf.eprintf "%s is unsafe\n" f;
Hashtbl.add unsafe f ()
@finalize:ocaml depends on !update_results@
tbls << merge.unsafe;
c << merge.changed;
@@
List.iter
(fun t ->
Hashtbl.iter
(fun k v ->
if not (Hashtbl.mem unsafe k) then Hashtbl.add unsafe k ()) t)
tbls;
changed := false;
let changed = List.exists (fun x -> !x) c in
let it = new iteration() in
it#add_virtual_rule After_start;
(if not changed
then it#add_virtual_rule Update_results);
it#register()
@depends on update_results disable optional_qualifier@
identifier f : script:ocaml() { not(is_unsafe(f)) };
identifier x;
@@
f(...,
+ const
struct nf_conntrack_l3proto *x,...) { ... }
@depends on update_results disable optional_qualifier@
identifier f : script:ocaml() { not(is_unsafe(f)) };
identifier x;
type T;
@@
T f(...,
+ const
struct nf_conntrack_l3proto *x,...);
@depends on update_results disable optional_qualifier@
identifier f : script:ocaml() { not(is_unsafe(f)) };
identifier x;
@@
f(...,
+ const
struct nf_conntrack_l3proto *x[],...) { ... }
@depends on update_results disable optional_qualifier@
identifier f : script:ocaml() { not(is_unsafe(f)) };
identifier x;
type T;
@@
T f(...,
+ const
struct nf_conntrack_l3proto *x[],...);
// </smpl>
---
v2: Added consideration of array parameters. This adds transformation of
nf_ct_l4proto_pernet_register and nf_ct_l4proto_pernet_unregister.
This patch also adds transformation of ctnl_timeout_parse_policy that was
somehow overlooked previously.
include/net/netfilter/nf_conntrack_l3proto.h | 6 ++---
include/net/netfilter/nf_conntrack_l4proto.h | 14 ++++++------
include/net/netfilter/nf_conntrack_timeout.h | 2 -
net/netfilter/nf_conntrack_core.c | 8 +++----
net/netfilter/nf_conntrack_netlink.c | 6 ++---
net/netfilter/nf_conntrack_proto.c | 30 +++++++++++++--------------
net/netfilter/nfnetlink_cttimeout.c | 5 ++--
7 files changed, 36 insertions(+), 35 deletions(-)
^ permalink raw reply
* [PATCH 1/1 v2] netfilter: constify nf_conntrack_l3/4proto parameters
From: Julia Lawall @ 2017-07-30 19:38 UTC (permalink / raw)
To: David S. Miller
Cc: bhumirks, kernel-janitors, Pablo Neira Ayuso, Jozsef Kadlecsik,
Florian Westphal, netfilter-devel, coreteam, netdev, linux-kernel
In-Reply-To: <1501443524-13201-1-git-send-email-Julia.Lawall@lip6.fr>
When a nf_conntrack_l3/4proto parameter is not on the left hand side
of an assignment, its address is not taken, and it is not passed to a
function that may modify its fields, then it can be declared as const.
This change is useful from a documentation point of view, and can
possibly facilitate making some nf_conntrack_l3/4proto structures const
subsequently.
Done with the help of Coccinelle.
Some spacing adjusted to fit within 80 characters.
Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
---
v2: Added consideration of array parameters. This adds transformation of
nf_ct_l4proto_pernet_register and nf_ct_l4proto_pernet_unregister.
This patch also adds transformation of ctnl_timeout_parse_policy that was
somehow overlooked previously.
include/net/netfilter/nf_conntrack_l3proto.h | 6 ++---
include/net/netfilter/nf_conntrack_l4proto.h | 14 ++++++------
include/net/netfilter/nf_conntrack_timeout.h | 2 -
net/netfilter/nf_conntrack_core.c | 8 +++----
net/netfilter/nf_conntrack_netlink.c | 6 ++---
net/netfilter/nf_conntrack_proto.c | 30 +++++++++++++--------------
net/netfilter/nfnetlink_cttimeout.c | 5 ++--
7 files changed, 36 insertions(+), 35 deletions(-)
diff --git a/include/net/netfilter/nf_conntrack_l3proto.h b/include/net/netfilter/nf_conntrack_l3proto.h
index 6d14b36..4782b15 100644
--- a/include/net/netfilter/nf_conntrack_l3proto.h
+++ b/include/net/netfilter/nf_conntrack_l3proto.h
@@ -76,17 +76,17 @@ struct nf_conntrack_l3proto {
#ifdef CONFIG_SYSCTL
/* Protocol pernet registration. */
int nf_ct_l3proto_pernet_register(struct net *net,
- struct nf_conntrack_l3proto *proto);
+ const struct nf_conntrack_l3proto *proto);
#else
static inline int nf_ct_l3proto_pernet_register(struct net *n,
- struct nf_conntrack_l3proto *p)
+ const struct nf_conntrack_l3proto *p)
{
return 0;
}
#endif
void nf_ct_l3proto_pernet_unregister(struct net *net,
- struct nf_conntrack_l3proto *proto);
+ const struct nf_conntrack_l3proto *proto);
/* Protocol global registration. */
int nf_ct_l3proto_register(struct nf_conntrack_l3proto *proto);
diff --git a/include/net/netfilter/nf_conntrack_l4proto.h b/include/net/netfilter/nf_conntrack_l4proto.h
index 7032e04..c86e946 100644
--- a/include/net/netfilter/nf_conntrack_l4proto.h
+++ b/include/net/netfilter/nf_conntrack_l4proto.h
@@ -125,23 +125,23 @@ struct nf_conntrack_l4proto *__nf_ct_l4proto_find(u_int16_t l3proto,
struct nf_conntrack_l4proto *nf_ct_l4proto_find_get(u_int16_t l3proto,
u_int8_t l4proto);
-void nf_ct_l4proto_put(struct nf_conntrack_l4proto *p);
+void nf_ct_l4proto_put(const struct nf_conntrack_l4proto *p);
/* Protocol pernet registration. */
int nf_ct_l4proto_pernet_register_one(struct net *net,
- struct nf_conntrack_l4proto *proto);
+ const struct nf_conntrack_l4proto *proto);
void nf_ct_l4proto_pernet_unregister_one(struct net *net,
- struct nf_conntrack_l4proto *proto);
+ const struct nf_conntrack_l4proto *proto);
int nf_ct_l4proto_pernet_register(struct net *net,
- struct nf_conntrack_l4proto *proto[],
+ struct nf_conntrack_l4proto * const proto[],
unsigned int num_proto);
void nf_ct_l4proto_pernet_unregister(struct net *net,
- struct nf_conntrack_l4proto *proto[],
- unsigned int num_proto);
+ struct nf_conntrack_l4proto * const proto[],
+ unsigned int num_proto);
/* Protocol global registration. */
int nf_ct_l4proto_register_one(struct nf_conntrack_l4proto *proto);
-void nf_ct_l4proto_unregister_one(struct nf_conntrack_l4proto *proto);
+void nf_ct_l4proto_unregister_one(const struct nf_conntrack_l4proto *proto);
int nf_ct_l4proto_register(struct nf_conntrack_l4proto *proto[],
unsigned int num_proto);
void nf_ct_l4proto_unregister(struct nf_conntrack_l4proto *proto[],
diff --git a/include/net/netfilter/nf_conntrack_timeout.h b/include/net/netfilter/nf_conntrack_timeout.h
index d40b893..b222957 100644
--- a/include/net/netfilter/nf_conntrack_timeout.h
+++ b/include/net/netfilter/nf_conntrack_timeout.h
@@ -68,7 +68,7 @@ struct nf_conn_timeout *nf_ct_timeout_ext_add(struct nf_conn *ct,
static inline unsigned int *
nf_ct_timeout_lookup(struct net *net, struct nf_conn *ct,
- struct nf_conntrack_l4proto *l4proto)
+ const struct nf_conntrack_l4proto *l4proto)
{
#ifdef CONFIG_NF_CONNTRACK_TIMEOUT
struct nf_conn_timeout *timeout_ext;
diff --git a/net/netfilter/nf_conntrack_core.c b/net/netfilter/nf_conntrack_core.c
index 51390fe..ed4e04e 100644
--- a/net/netfilter/nf_conntrack_core.c
+++ b/net/netfilter/nf_conntrack_core.c
@@ -1183,8 +1183,8 @@ void nf_conntrack_free(struct nf_conn *ct)
static noinline struct nf_conntrack_tuple_hash *
init_conntrack(struct net *net, struct nf_conn *tmpl,
const struct nf_conntrack_tuple *tuple,
- struct nf_conntrack_l3proto *l3proto,
- struct nf_conntrack_l4proto *l4proto,
+ const struct nf_conntrack_l3proto *l3proto,
+ const struct nf_conntrack_l4proto *l4proto,
struct sk_buff *skb,
unsigned int dataoff, u32 hash)
{
@@ -1295,8 +1295,8 @@ void nf_conntrack_free(struct nf_conn *ct)
unsigned int dataoff,
u_int16_t l3num,
u_int8_t protonum,
- struct nf_conntrack_l3proto *l3proto,
- struct nf_conntrack_l4proto *l4proto)
+ const struct nf_conntrack_l3proto *l3proto,
+ const struct nf_conntrack_l4proto *l4proto)
{
const struct nf_conntrack_zone *zone;
struct nf_conntrack_tuple tuple;
diff --git a/net/netfilter/nf_conntrack_netlink.c b/net/netfilter/nf_conntrack_netlink.c
index 7999e70..5d08602 100644
--- a/net/netfilter/nf_conntrack_netlink.c
+++ b/net/netfilter/nf_conntrack_netlink.c
@@ -61,8 +61,8 @@
static char __initdata version[] = "0.93";
static int ctnetlink_dump_tuples_proto(struct sk_buff *skb,
- const struct nf_conntrack_tuple *tuple,
- struct nf_conntrack_l4proto *l4proto)
+ const struct nf_conntrack_tuple *tuple,
+ const struct nf_conntrack_l4proto *l4proto)
{
int ret = 0;
struct nlattr *nest_parms;
@@ -86,7 +86,7 @@ static int ctnetlink_dump_tuples_proto(struct sk_buff *skb,
static int ctnetlink_dump_tuples_ip(struct sk_buff *skb,
const struct nf_conntrack_tuple *tuple,
- struct nf_conntrack_l3proto *l3proto)
+ const struct nf_conntrack_l3proto *l3proto)
{
int ret = 0;
struct nlattr *nest_parms;
diff --git a/net/netfilter/nf_conntrack_proto.c b/net/netfilter/nf_conntrack_proto.c
index 1dcad22..62a2923 100644
--- a/net/netfilter/nf_conntrack_proto.c
+++ b/net/netfilter/nf_conntrack_proto.c
@@ -188,7 +188,7 @@ struct nf_conntrack_l4proto *
}
EXPORT_SYMBOL_GPL(nf_ct_l4proto_find_get);
-void nf_ct_l4proto_put(struct nf_conntrack_l4proto *p)
+void nf_ct_l4proto_put(const struct nf_conntrack_l4proto *p)
{
module_put(p->me);
}
@@ -242,7 +242,7 @@ int nf_ct_l3proto_register(struct nf_conntrack_l3proto *proto)
extern unsigned int nf_conntrack_default_on;
int nf_ct_l3proto_pernet_register(struct net *net,
- struct nf_conntrack_l3proto *proto)
+ const struct nf_conntrack_l3proto *proto)
{
if (nf_conntrack_default_on == 0)
return 0;
@@ -271,7 +271,7 @@ void nf_ct_l3proto_unregister(struct nf_conntrack_l3proto *proto)
EXPORT_SYMBOL_GPL(nf_ct_l3proto_unregister);
void nf_ct_l3proto_pernet_unregister(struct net *net,
- struct nf_conntrack_l3proto *proto)
+ const struct nf_conntrack_l3proto *proto)
{
/*
* nf_conntrack_default_on *might* have registered hooks.
@@ -286,7 +286,7 @@ void nf_ct_l3proto_pernet_unregister(struct net *net,
EXPORT_SYMBOL_GPL(nf_ct_l3proto_pernet_unregister);
static struct nf_proto_net *nf_ct_l4proto_net(struct net *net,
- struct nf_conntrack_l4proto *l4proto)
+ const struct nf_conntrack_l4proto *l4proto)
{
if (l4proto->get_net_proto) {
/* statically built-in protocols use static per-net */
@@ -301,7 +301,7 @@ static struct nf_proto_net *nf_ct_l4proto_net(struct net *net,
static
int nf_ct_l4proto_register_sysctl(struct net *net,
struct nf_proto_net *pn,
- struct nf_conntrack_l4proto *l4proto)
+ const struct nf_conntrack_l4proto *l4proto)
{
int err = 0;
@@ -324,8 +324,8 @@ int nf_ct_l4proto_register_sysctl(struct net *net,
static
void nf_ct_l4proto_unregister_sysctl(struct net *net,
- struct nf_proto_net *pn,
- struct nf_conntrack_l4proto *l4proto)
+ struct nf_proto_net *pn,
+ const struct nf_conntrack_l4proto *l4proto)
{
#ifdef CONFIG_SYSCTL
if (pn->ctl_table_header != NULL)
@@ -395,7 +395,7 @@ int nf_ct_l4proto_register_one(struct nf_conntrack_l4proto *l4proto)
EXPORT_SYMBOL_GPL(nf_ct_l4proto_register_one);
int nf_ct_l4proto_pernet_register_one(struct net *net,
- struct nf_conntrack_l4proto *l4proto)
+ const struct nf_conntrack_l4proto *l4proto)
{
int ret = 0;
struct nf_proto_net *pn = NULL;
@@ -420,7 +420,7 @@ int nf_ct_l4proto_pernet_register_one(struct net *net,
}
EXPORT_SYMBOL_GPL(nf_ct_l4proto_pernet_register_one);
-static void __nf_ct_l4proto_unregister_one(struct nf_conntrack_l4proto *l4proto)
+static void __nf_ct_l4proto_unregister_one(const struct nf_conntrack_l4proto *l4proto)
{
BUG_ON(l4proto->l3proto >= ARRAY_SIZE(nf_ct_protos));
@@ -433,7 +433,7 @@ static void __nf_ct_l4proto_unregister_one(struct nf_conntrack_l4proto *l4proto)
&nf_conntrack_l4proto_generic);
}
-void nf_ct_l4proto_unregister_one(struct nf_conntrack_l4proto *l4proto)
+void nf_ct_l4proto_unregister_one(const struct nf_conntrack_l4proto *l4proto)
{
mutex_lock(&nf_ct_proto_mutex);
__nf_ct_l4proto_unregister_one(l4proto);
@@ -444,7 +444,7 @@ void nf_ct_l4proto_unregister_one(struct nf_conntrack_l4proto *l4proto)
EXPORT_SYMBOL_GPL(nf_ct_l4proto_unregister_one);
void nf_ct_l4proto_pernet_unregister_one(struct net *net,
- struct nf_conntrack_l4proto *l4proto)
+ const struct nf_conntrack_l4proto *l4proto)
{
struct nf_proto_net *pn = nf_ct_l4proto_net(net, l4proto);
@@ -478,8 +478,8 @@ int nf_ct_l4proto_register(struct nf_conntrack_l4proto *l4proto[],
EXPORT_SYMBOL_GPL(nf_ct_l4proto_register);
int nf_ct_l4proto_pernet_register(struct net *net,
- struct nf_conntrack_l4proto *l4proto[],
- unsigned int num_proto)
+ struct nf_conntrack_l4proto * const l4proto[],
+ unsigned int num_proto)
{
int ret = -EINVAL;
unsigned int i;
@@ -514,8 +514,8 @@ void nf_ct_l4proto_unregister(struct nf_conntrack_l4proto *l4proto[],
EXPORT_SYMBOL_GPL(nf_ct_l4proto_unregister);
void nf_ct_l4proto_pernet_unregister(struct net *net,
- struct nf_conntrack_l4proto *l4proto[],
- unsigned int num_proto)
+ struct nf_conntrack_l4proto * const l4proto[],
+ unsigned int num_proto)
{
while (num_proto-- != 0)
nf_ct_l4proto_pernet_unregister_one(net, l4proto[num_proto]);
diff --git a/net/netfilter/nfnetlink_cttimeout.c b/net/netfilter/nfnetlink_cttimeout.c
index 400e9ae..707a0d8 100644
--- a/net/netfilter/nfnetlink_cttimeout.c
+++ b/net/netfilter/nfnetlink_cttimeout.c
@@ -47,7 +47,8 @@
};
static int
-ctnl_timeout_parse_policy(void *timeouts, struct nf_conntrack_l4proto *l4proto,
+ctnl_timeout_parse_policy(void *timeouts,
+ const struct nf_conntrack_l4proto *l4proto,
struct net *net, const struct nlattr *attr)
{
int ret = 0;
@@ -401,7 +402,7 @@ static int cttimeout_default_set(struct net *net, struct sock *ctnl,
static int
cttimeout_default_fill_info(struct net *net, struct sk_buff *skb, u32 portid,
u32 seq, u32 type, int event,
- struct nf_conntrack_l4proto *l4proto)
+ const struct nf_conntrack_l4proto *l4proto)
{
struct nlmsghdr *nlh;
struct nfgenmsg *nfmsg;
^ permalink raw reply related
* Re: [patch net-next 03/20] net: sched: change names of action number helpers to be aligned with the rest
From: Jamal Hadi Salim @ 2017-07-30 19:46 UTC (permalink / raw)
To: Jiri Pirko, netdev; +Cc: davem, xiyou.wangcong, daniel, mlxsw
In-Reply-To: <20170728144042.6380-4-jiri@resnulli.us>
On 17-07-28 10:40 AM, Jiri Pirko wrote:
> From: Jiri Pirko <jiri@mellanox.com>
>
> The rest of the helpers are named tcf_exts_*, so change the name of
> the action number helpers to be aligned. While at it, change to inline
> functions.
>
> Signed-off-by: Jiri Pirko <jiri@mellanox.com>
Looks reasonable.
Acked-by: Jamal Hadi Salim <jhs@mojatatu.com>
cheers,
jamal
^ permalink raw reply
* Re: [patch net-next 04/20] net: sched: use tcf_exts_has_actions in tcf_exts_exec
From: Jamal Hadi Salim @ 2017-07-30 19:48 UTC (permalink / raw)
To: Jiri Pirko, netdev; +Cc: davem, xiyou.wangcong, daniel, mlxsw
In-Reply-To: <20170728144042.6380-5-jiri@resnulli.us>
I am probably missing something. All those changes to just
replace "if (exts->nr_actions)" with "if (tcf_exts_has_actions(exts))" ?
cheers,
jamal
On 17-07-28 10:40 AM, Jiri Pirko wrote:
> From: Jiri Pirko <jiri@mellanox.com>
>
> Use the tcf_exts_has_actions helper instead or directly testing
> exts->nr_actions in tcf_exts_exec.
>
> Signed-off-by: Jiri Pirko <jiri@mellanox.com>
> ---
> include/net/pkt_cls.h | 46 +++++++++++++++++++++++-----------------------
> 1 file changed, 23 insertions(+), 23 deletions(-)
>
> diff --git a/include/net/pkt_cls.h b/include/net/pkt_cls.h
> index 7f25636..322a282 100644
> --- a/include/net/pkt_cls.h
> +++ b/include/net/pkt_cls.h
> @@ -177,29 +177,6 @@ tcf_exts_stats_update(const struct tcf_exts *exts,
> }
>
> /**
> - * tcf_exts_exec - execute tc filter extensions
> - * @skb: socket buffer
> - * @exts: tc filter extensions handle
> - * @res: desired result
> - *
> - * Executes all configured extensions. Returns 0 on a normal execution,
> - * a negative number if the filter must be considered unmatched or
> - * a positive action code (TC_ACT_*) which must be returned to the
> - * underlying layer.
> - */
> -static inline int
> -tcf_exts_exec(struct sk_buff *skb, struct tcf_exts *exts,
> - struct tcf_result *res)
> -{
> -#ifdef CONFIG_NET_CLS_ACT
> - if (exts->nr_actions)
> - return tcf_action_exec(skb, exts->actions, exts->nr_actions,
> - res);
> -#endif
> - return 0;
> -}
> -
> -/**
> * tcf_exts_has_actions - check if at least one action is present
> * @exts: tc filter extensions handle
> *
> @@ -229,6 +206,29 @@ static inline bool tcf_exts_has_one_action(struct tcf_exts *exts)
> #endif
> }
>
> +/**
> + * tcf_exts_exec - execute tc filter extensions
> + * @skb: socket buffer
> + * @exts: tc filter extensions handle
> + * @res: desired result
> + *
> + * Executes all configured extensions. Returns 0 on a normal execution,
> + * a negative number if the filter must be considered unmatched or
> + * a positive action code (TC_ACT_*) which must be returned to the
> + * underlying layer.
> + */
> +static inline int
> +tcf_exts_exec(struct sk_buff *skb, struct tcf_exts *exts,
> + struct tcf_result *res)
> +{
> +#ifdef CONFIG_NET_CLS_ACT
> + if (tcf_exts_has_actions(exts))
> + return tcf_action_exec(skb, exts->actions, exts->nr_actions,
> + res);
> +#endif
> + return 0;
> +}
> +
> int tcf_exts_validate(struct net *net, struct tcf_proto *tp,
> struct nlattr **tb, struct nlattr *rate_tlv,
> struct tcf_exts *exts, bool ovr);
>
^ permalink raw reply
* Re: [PATCH v2] ravb: add wake-on-lan support via magic packet
From: Niklas Söderlund @ 2017-07-30 19:51 UTC (permalink / raw)
To: Andrew Lunn
Cc: Sergei Shtylyov, Geert Uytterhoeven, netdev, linux-renesas-soc
In-Reply-To: <20170730170738.GA29373@lunn.ch>
Hi Andrew,
On 2017-07-30 19:07:38 +0200, Andrew Lunn wrote:
> Hi Niklas
>
> > @@ -2041,6 +2073,11 @@ static int ravb_probe(struct platform_device *pdev)
> >
> > priv->chip_id = chip_id;
> >
> > + /* Get clock, if not found that's OK but Wake-On-Lan is unavailable */
> > + priv->clk = devm_clk_get(&pdev->dev, NULL);
> > + if (IS_ERR(priv->clk))
> > + priv->clk = NULL;
>
> Can you get EPROBE_DEFER returned?
I don't think so, but I'm not sure :-)
The clock I'm trying to get is the module clock of the ravb itself, so
if that clock is not available (and enabled) no register writes to the
ravb would be possible in the first place, so i guess it's safe to
assume -EPROBE_DEFER can not happen here?
I'm just trying to play it safe here since the clock is only needed to
support WoL, I though it best to not change behavior here. Try to get
the clock, if we can great we can do WoL if not then user-space will be
prevented from enabling WoL and nothing in the current behavior changes.
>
> Andrew
--
Regards,
Niklas Söderlund
^ permalink raw reply
* Re: [PATCH net-next v12 1/4] net netlink: Add new type NLA_BITFIELD32
From: Jamal Hadi Salim @ 2017-07-30 19:59 UTC (permalink / raw)
To: Jiri Pirko; +Cc: davem, netdev, xiyou.wangcong, eric.dumazet, horms, dsahern
In-Reply-To: <20170730184242.GA1872@nanopsycho>
Jiri,
This is getting exhausting, seriously.
I posted the code you are commenting one two days ago so i dont have to
repost.
On D. Ahern: I dont think we are disagreeing anymore on the need to
generalize the check. He is saying it should be a helper and I already
had the validation data; either works. I dont see the gapping need
to remove the validation data.
cheers,
jamal
On 17-07-30 02:42 PM, Jiri Pirko wrote:
> Sun, Jul 30, 2017 at 07:24:49PM CEST, jhs@mojatatu.com wrote:
>> From: Jamal Hadi Salim <jhs@mojatatu.com>
>>
>> Generic bitflags attribute content sent to the kernel by user.
>> With this netlink attr type the user can either set or unset a
>> flag in the kernel.
>>
>> The value is a bitmap that defines the bit values being set
>> The selector is a bitmask that defines which value bit is to be
>> considered.
>>
>> A check is made to ensure the rules that a kernel subsystem always
>> conforms to bitflags the kernel already knows about. i.e
>> if the user tries to set a bit flag that is not understood then
>> the _it will be rejected_.
>>
>> In the most basic form, the user specifies the attribute policy as:
>> [ATTR_GOO] = { .type = NLA_BITFIELD32, .validation_data = &myvalidflags },
>>
>> where myvalidflags is the bit mask of the flags the kernel understands.
>>
>> If the user _does not_ provide myvalidflags then the attribute will
>> also be rejected.
>>
>> Examples:
>> value = 0x0, and selector = 0x1
>> implies we are selecting bit 1 and we want to set its value to 0.
>>
>> value = 0x2, and selector = 0x2
>> implies we are selecting bit 2 and we want to set its value to 1.
>>
>> Suggested-by: Jiri Pirko <jiri@mellanox.com>
>> Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
>> ---
>> include/net/netlink.h | 16 ++++++++++++++++
>> include/uapi/linux/netlink.h | 17 +++++++++++++++++
>> lib/nlattr.c | 30 ++++++++++++++++++++++++++++++
>> 3 files changed, 63 insertions(+)
>>
>> diff --git a/include/net/netlink.h b/include/net/netlink.h
>> index ef8e6c3..82dd298 100644
>> --- a/include/net/netlink.h
>> +++ b/include/net/netlink.h
>> @@ -178,6 +178,7 @@ enum {
>> NLA_S16,
>> NLA_S32,
>> NLA_S64,
>> + NLA_BITFIELD32,
>> __NLA_TYPE_MAX,
>> };
>>
>> @@ -206,6 +207,7 @@ enum {
>> * NLA_MSECS Leaving the length field zero will verify the
>> * given type fits, using it verifies minimum length
>> * just like "All other"
>> + * NLA_BITFIELD32 A 32-bit bitmap/bitselector attribute
>> * All other Minimum length of attribute payload
>> *
>> * Example:
>> @@ -213,11 +215,13 @@ enum {
>> * [ATTR_FOO] = { .type = NLA_U16 },
>> * [ATTR_BAR] = { .type = NLA_STRING, .len = BARSIZ },
>> * [ATTR_BAZ] = { .len = sizeof(struct mystruct) },
>> + * [ATTR_GOO] = { .type = NLA_BITFIELD32, .validation_data = &myvalidflags },
>
> Checkpatch warns you about the line to long, please wrap it.
>
> Btw, I did not see you reached a consensus with DavidA regarding this.
> Did I miss it?
>
>
>> * };
>> */
>> struct nla_policy {
>> u16 type;
>> u16 len;
>> + void *validation_data;
>> };
>>
>> /**
>> @@ -1203,6 +1207,18 @@ static inline struct in6_addr nla_get_in6_addr(const struct nlattr *nla)
>> }
>>
>> /**
>> + * nla_get_bitfield32 - return payload of 32 bitfield attribute
>> + * @nla: nla_bitfield32 attribute
>> + */
>> +static inline struct nla_bitfield32 nla_get_bitfield32(const struct nlattr *nla)
>> +{
>> + struct nla_bitfield32 tmp;
>> +
>> + nla_memcpy(&tmp, nla, sizeof(tmp));
>> + return tmp;
>> +}
>> +
>> +/**
>> * nla_memdup - duplicate attribute memory (kmemdup)
>> * @src: netlink attribute to duplicate from
>> * @gfp: GFP mask
>> diff --git a/include/uapi/linux/netlink.h b/include/uapi/linux/netlink.h
>> index f86127a..f4fc9c9 100644
>> --- a/include/uapi/linux/netlink.h
>> +++ b/include/uapi/linux/netlink.h
>> @@ -226,5 +226,22 @@ struct nlattr {
>> #define NLA_ALIGN(len) (((len) + NLA_ALIGNTO - 1) & ~(NLA_ALIGNTO - 1))
>> #define NLA_HDRLEN ((int) NLA_ALIGN(sizeof(struct nlattr)))
>>
>> +/* Generic 32 bitflags attribute content sent to the kernel.
>> + *
>> + * The value is a bitmap that defines the values being set
>> + * The selector is a bitmask that defines which value is legit
>> + *
>> + * Examples:
>> + * value = 0x0, and selector = 0x1
>> + * implies we are selecting bit 1 and we want to set its value to 0.
>> + *
>> + * value = 0x2, and selector = 0x2
>> + * implies we are selecting bit 2 and we want to set its value to 1.
>> + *
>> + */
>> +struct nla_bitfield32 {
>> + __u32 value;
>> + __u32 selector;
>> +};
>>
>> #endif /* _UAPI__LINUX_NETLINK_H */
>> diff --git a/lib/nlattr.c b/lib/nlattr.c
>> index fb52435..ee79b7a 100644
>> --- a/lib/nlattr.c
>> +++ b/lib/nlattr.c
>> @@ -27,6 +27,30 @@
>> [NLA_S64] = sizeof(s64),
>> };
>>
>> +static int validate_nla_bitfield32(const struct nlattr *nla,
>> + u32 *valid_flags_allowed)
>> +{
>> + const struct nla_bitfield32 *bf = nla_data(nla);
>> + u32 *valid_flags_mask = valid_flags_allowed;
>
> I pointed this out already. This weird.
> You do *u32 = *u32, just with different name. Just use valid_flags_allowed
> directly.
>
>
>> +
>> + if (!valid_flags_allowed)
>> + return -EINVAL;
>> +
>> + /*disallow invalid bit selector */
>
> Fix all the comments in this function. Should be
> /* something */
> with spaces in front and at the end.
>
>
>> + if (bf->selector & ~*valid_flags_mask)
>> + return -EINVAL;
>> +
>> + /*disallow invalid bit values */
>> + if (bf->value & ~*valid_flags_mask)
>> + return -EINVAL;
>> +
>> + /*disallow valid bit values that are not selected*/
>> + if (bf->value & ~bf->selector)
>> + return -EINVAL;
>> +
>> + return 0;
>> +}
>> +
>> static int validate_nla(const struct nlattr *nla, int maxtype,
>> const struct nla_policy *policy)
>> {
>> @@ -46,6 +70,12 @@ static int validate_nla(const struct nlattr *nla, int maxtype,
>> return -ERANGE;
>> break;
>>
>> + case NLA_BITFIELD32:
>> + if (attrlen != sizeof(struct nla_bitfield32))
>> + return -ERANGE;
>> +
>> + return validate_nla_bitfield32(nla, pt->validation_data);
>> +
>> case NLA_NUL_STRING:
>> if (pt->len)
>> minlen = min_t(int, attrlen, pt->len + 1);
>> --
>> 1.9.1
>>
^ permalink raw reply
* Re: [PATCH v2] ravb: add wake-on-lan support via magic packet
From: Andrew Lunn @ 2017-07-30 20:07 UTC (permalink / raw)
To: Niklas Söderlund
Cc: Sergei Shtylyov, Geert Uytterhoeven, netdev, linux-renesas-soc
In-Reply-To: <20170730195154.GE1382@bigcity.dyn.berto.se>
On Sun, Jul 30, 2017 at 09:51:54PM +0200, Niklas Söderlund wrote:
> Hi Andrew,
>
> On 2017-07-30 19:07:38 +0200, Andrew Lunn wrote:
> > Hi Niklas
> >
> > > @@ -2041,6 +2073,11 @@ static int ravb_probe(struct platform_device *pdev)
> > >
> > > priv->chip_id = chip_id;
> > >
> > > + /* Get clock, if not found that's OK but Wake-On-Lan is unavailable */
> > > + priv->clk = devm_clk_get(&pdev->dev, NULL);
> > > + if (IS_ERR(priv->clk))
> > > + priv->clk = NULL;
> >
> > Can you get EPROBE_DEFER returned?
>
> I don't think so, but I'm not sure :-)
>
> The clock I'm trying to get is the module clock of the ravb itself, so
> if that clock is not available (and enabled) no register writes to the
> ravb would be possible in the first place, so i guess it's safe to
> assume -EPROBE_DEFER can not happen here?
>
> I'm just trying to play it safe here since the clock is only needed to
> support WoL, I though it best to not change behavior here. Try to get
> the clock, if we can great we can do WoL if not then user-space will be
> prevented from enabling WoL and nothing in the current behavior changes.
Hi Nikls
Well, if it can return -EPROBE_DEFER, it means sometimes WoL will be
avalable and other times not, depending on when the clock driver
probes. However, it sounds like this is the SoCs core clock driver. If
so, it gets loaded very early, so you are safe.
Andrew
^ permalink raw reply
* [PATCH net-next] net: fec: Issue error for missing but expected PHY
From: Andrew Lunn @ 2017-07-30 20:11 UTC (permalink / raw)
To: David Miller; +Cc: netdev, Fugang Duan, Andrew Lunn
If the PHY is missing but expected, e.g. because of a typ0 in the dt
file, it is not possible to open the interface. ip link returns:
RTNETLINK answers: No such device
It is not very obvious what the problem is. Add a netdev_err() in this
case to make it easier to debug the issue.
[ 21.409385] fec 2188000.ethernet eth0: Unable to connect to phy
RTNETLINK answers: No such device
Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
drivers/net/ethernet/freescale/fec_main.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index cc0c2a58c4de..c5995f07f821 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -1907,8 +1907,10 @@ static int fec_enet_mii_probe(struct net_device *ndev)
phy_dev = of_phy_connect(ndev, fep->phy_node,
&fec_enet_adjust_link, 0,
fep->phy_interface);
- if (!phy_dev)
+ if (!phy_dev) {
+ netdev_err(ndev, "Unable to connect to phy\n");
return -ENODEV;
+ }
} else {
/* check for attached phy */
for (phy_id = 0; (phy_id < PHY_MAX_ADDR); phy_id++) {
--
2.13.2
^ permalink raw reply related
* Re: [patch net-next 09/20] net: sched: convert actions array into rcu list
From: Jamal Hadi Salim @ 2017-07-30 20:17 UTC (permalink / raw)
To: Jiri Pirko, netdev; +Cc: davem, xiyou.wangcong, daniel, mlxsw
In-Reply-To: <20170728144042.6380-10-jiri@resnulli.us>
On 17-07-28 10:40 AM, Jiri Pirko wrote:
> From: Jiri Pirko <jiri@mellanox.com>
>
> Currently the actions are stored in array with array size. To traverse
> this array in fastpath, tcf_tree_lock is taken to protect it. Convert
> the array into a singly linked list, similar to the filter chains style
> and allow traversal protected by rcu.
>
Looks sane. But lets have Cong provide an opinion (since he said he was
trying to rcu the actions).
cheers,
jamal
^ permalink raw reply
* Re: [PATCH v2] ravb: add wake-on-lan support via magic packet
From: Niklas Söderlund @ 2017-07-30 20:19 UTC (permalink / raw)
To: Andrew Lunn
Cc: Sergei Shtylyov, Geert Uytterhoeven, netdev, linux-renesas-soc
In-Reply-To: <20170730200731.GA20421@lunn.ch>
Hi Andrew,
On 2017-07-30 22:07:31 +0200, Andrew Lunn wrote:
> On Sun, Jul 30, 2017 at 09:51:54PM +0200, Niklas Söderlund wrote:
> > Hi Andrew,
> >
> > On 2017-07-30 19:07:38 +0200, Andrew Lunn wrote:
> > > Hi Niklas
> > >
> > > > @@ -2041,6 +2073,11 @@ static int ravb_probe(struct platform_device *pdev)
> > > >
> > > > priv->chip_id = chip_id;
> > > >
> > > > + /* Get clock, if not found that's OK but Wake-On-Lan is unavailable */
> > > > + priv->clk = devm_clk_get(&pdev->dev, NULL);
> > > > + if (IS_ERR(priv->clk))
> > > > + priv->clk = NULL;
> > >
> > > Can you get EPROBE_DEFER returned?
> >
> > I don't think so, but I'm not sure :-)
> >
> > The clock I'm trying to get is the module clock of the ravb itself, so
> > if that clock is not available (and enabled) no register writes to the
> > ravb would be possible in the first place, so i guess it's safe to
> > assume -EPROBE_DEFER can not happen here?
> >
> > I'm just trying to play it safe here since the clock is only needed to
> > support WoL, I though it best to not change behavior here. Try to get
> > the clock, if we can great we can do WoL if not then user-space will be
> > prevented from enabling WoL and nothing in the current behavior changes.
>
> Hi Nikls
>
> Well, if it can return -EPROBE_DEFER, it means sometimes WoL will be
> avalable and other times not, depending on when the clock driver
Ahh I see yes that would be indeed be bad.
> probes. However, it sounds like this is the SoCs core clock driver. If
> so, it gets loaded very early, so you are safe.
Yes this is renesas-cpg-mssr which if I understand things is a core
clock driver. It is register in drivers/clk/renesas/renesas-cpg-mssr.c
using:
subsys_initcall(cpg_mssr_init);
So I take it I'm safe. Thanks however for bringing this to my attention
I learnt something new today :-)
>
> Andrew
--
Regards,
Niklas Söderlund
^ permalink raw reply
* Re: [PATCH V2 net-next 0/2] liquidio: Add support for managing liquidio adapter
From: Simon Horman @ 2017-07-30 20:21 UTC (permalink / raw)
To: Felix Manlunas
Cc: davem, netdev, raghu.vatsavayi, derek.chickles, satananda.burla,
veerasenareddy.burru, Jakub Kicinski
In-Reply-To: <20170729061707.GA1134@felix-thinkpad.cavium.com>
On Fri, Jul 28, 2017 at 11:17:07PM -0700, Felix Manlunas wrote:
> From: Veerasenareddy Burru <veerasenareddy.burru@cavium.com>
>
> The LiquidIO adapter has processor cores that can run Linux. This patch
> set adds support to create a virtual Ethernet interface on host to
> communicate with applications running on Linux in the LiquidIO adapter.
> The virtual Ethernet interface also provides login access to Linux on
> LiquidIO through ssh for management and debugging.
As per the somewhat more detailed feedback provided by my colleague Jakub
Kicinski to v1 of this patchset[1] I am concerned that this patchset breaks down
the long standing practice of not granting direct access to firmware from
userspace.
[1] https://www.spinics.net/lists/netdev/msg444929.html
^ permalink raw reply
* [PATCH net-next 1/7] net: phy: marvell: tabification
From: Andrew Lunn @ 2017-07-30 20:41 UTC (permalink / raw)
To: David Miller; +Cc: netdev, Florian Fainelli, Andrew Lunn
In-Reply-To: <1501447310-24101-1-git-send-email-andrew@lunn.ch>
Convert spaces to tabs where appropriate, and fix up some otherwise
odd indentation.
Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
drivers/net/phy/marvell.c | 30 +++++++++++++++---------------
1 file changed, 15 insertions(+), 15 deletions(-)
diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c
index 5d314f143aea..6a5256ceb11e 100644
--- a/drivers/net/phy/marvell.c
+++ b/drivers/net/phy/marvell.c
@@ -108,24 +108,24 @@
#define MII_88E1318S_PHY_MSCR1_PAD_ODD BIT(6)
/* Copper Specific Interrupt Enable Register */
-#define MII_88E1318S_PHY_CSIER 0x12
+#define MII_88E1318S_PHY_CSIER 0x12
/* WOL Event Interrupt Enable */
-#define MII_88E1318S_PHY_CSIER_WOL_EIE BIT(7)
+#define MII_88E1318S_PHY_CSIER_WOL_EIE BIT(7)
/* LED Timer Control Register */
-#define MII_88E1318S_PHY_LED_TCR 0x12
-#define MII_88E1318S_PHY_LED_TCR_FORCE_INT BIT(15)
-#define MII_88E1318S_PHY_LED_TCR_INTn_ENABLE BIT(7)
-#define MII_88E1318S_PHY_LED_TCR_INT_ACTIVE_LOW BIT(11)
+#define MII_88E1318S_PHY_LED_TCR 0x12
+#define MII_88E1318S_PHY_LED_TCR_FORCE_INT BIT(15)
+#define MII_88E1318S_PHY_LED_TCR_INTn_ENABLE BIT(7)
+#define MII_88E1318S_PHY_LED_TCR_INT_ACTIVE_LOW BIT(11)
/* Magic Packet MAC address registers */
-#define MII_88E1318S_PHY_MAGIC_PACKET_WORD2 0x17
-#define MII_88E1318S_PHY_MAGIC_PACKET_WORD1 0x18
-#define MII_88E1318S_PHY_MAGIC_PACKET_WORD0 0x19
+#define MII_88E1318S_PHY_MAGIC_PACKET_WORD2 0x17
+#define MII_88E1318S_PHY_MAGIC_PACKET_WORD1 0x18
+#define MII_88E1318S_PHY_MAGIC_PACKET_WORD0 0x19
-#define MII_88E1318S_PHY_WOL_CTRL 0x10
-#define MII_88E1318S_PHY_WOL_CTRL_CLEAR_WOL_STATUS BIT(12)
-#define MII_88E1318S_PHY_WOL_CTRL_MAGIC_PACKET_MATCH_ENABLE BIT(14)
+#define MII_88E1318S_PHY_WOL_CTRL 0x10
+#define MII_88E1318S_PHY_WOL_CTRL_CLEAR_WOL_STATUS BIT(12)
+#define MII_88E1318S_PHY_WOL_CTRL_MAGIC_PACKET_MATCH_ENABLE BIT(14)
#define MII_88E1121_PHY_LED_CTRL 16
#define MII_88E1121_PHY_LED_DEF 0x0030
@@ -152,7 +152,7 @@
#define LPA_FIBER_1000HALF 0x40
#define LPA_FIBER_1000FULL 0x20
-#define LPA_PAUSE_FIBER 0x180
+#define LPA_PAUSE_FIBER 0x180
#define LPA_PAUSE_ASYM_FIBER 0x100
#define ADVERTISE_FIBER_1000HALF 0x40
@@ -596,7 +596,7 @@ static int marvell_config_aneg_fiber(struct phy_device *phydev)
if (changed == 0) {
/* Advertisement hasn't changed, but maybe aneg was never on to
- * begin with? Or maybe phy was isolated?
+ * begin with? Or maybe phy was isolated?
*/
int ctl = phy_read(phydev, MII_BMCR);
@@ -1515,7 +1515,7 @@ static void marvell_get_strings(struct phy_device *phydev, u8 *data)
}
#ifndef UINT64_MAX
-#define UINT64_MAX (u64)(~((u64)0))
+#define UINT64_MAX (u64)(~((u64)0))
#endif
static u64 marvell_get_stat(struct phy_device *phydev, int i)
{
--
2.13.2
^ permalink raw reply related
* [PATCH net-next 3/7] net: phy: marvell: consolidate RGMII delay code
From: Andrew Lunn @ 2017-07-30 20:41 UTC (permalink / raw)
To: David Miller; +Cc: netdev, Florian Fainelli, Andrew Lunn
In-Reply-To: <1501447310-24101-1-git-send-email-andrew@lunn.ch>
The same code is repeated for different PHY versions. Put it into a
help and call when needed.
Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
drivers/net/phy/marvell.c | 54 +++++++++++++++++++----------------------------
1 file changed, 22 insertions(+), 32 deletions(-)
diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c
index 33a52532fac6..c1b724ab5f25 100644
--- a/drivers/net/phy/marvell.c
+++ b/drivers/net/phy/marvell.c
@@ -61,13 +61,6 @@
#define MII_M1011_PHY_SCR_AUTO_CROSS 0x0060
#define MII_M1145_PHY_EXT_SR 0x1b
-#define MII_M1145_PHY_EXT_CR 0x14
-#define MII_M1145_RGMII_RX_DELAY 0x0080
-#define MII_M1145_RGMII_TX_DELAY 0x0002
-#define MII_M1145_HWCFG_MODE_SGMII_NO_CLK 0x4
-#define MII_M1145_HWCFG_MODE_MASK 0xf
-#define MII_M1145_HWCFG_FIBER_COPPER_AUTO 0x8000
-
#define MII_M1145_HWCFG_MODE_SGMII_NO_CLK 0x4
#define MII_M1145_HWCFG_MODE_MASK 0xf
#define MII_M1145_HWCFG_FIBER_COPPER_AUTO 0x8000
@@ -76,8 +69,8 @@
#define MII_M1111_PHY_LED_DIRECT 0x4100
#define MII_M1111_PHY_LED_COMBINE 0x411c
#define MII_M1111_PHY_EXT_CR 0x14
-#define MII_M1111_RX_DELAY 0x80
-#define MII_M1111_TX_DELAY 0x2
+#define MII_M1111_RGMII_RX_DELAY BIT(7)
+#define MII_M1111_RGMII_TX_DELAY BIT(1)
#define MII_M1111_PHY_EXT_SR 0x1b
#define MII_M1111_HWCFG_MODE_MASK 0xf
@@ -700,9 +693,8 @@ static int m88e3016_config_init(struct phy_device *phydev)
return marvell_config_init(phydev);
}
-static int m88e1111_config_init_rgmii(struct phy_device *phydev)
+static int m88e1111_config_init_rgmii_delays(struct phy_device *phydev)
{
- int err;
int temp;
temp = phy_read(phydev, MII_M1111_PHY_EXT_CR);
@@ -710,16 +702,24 @@ static int m88e1111_config_init_rgmii(struct phy_device *phydev)
return temp;
if (phydev->interface == PHY_INTERFACE_MODE_RGMII_ID) {
- temp |= (MII_M1111_RX_DELAY | MII_M1111_TX_DELAY);
+ temp |= (MII_M1111_RGMII_RX_DELAY | MII_M1111_RGMII_TX_DELAY);
} else if (phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID) {
- temp &= ~MII_M1111_TX_DELAY;
- temp |= MII_M1111_RX_DELAY;
+ temp &= ~MII_M1111_RGMII_TX_DELAY;
+ temp |= MII_M1111_RGMII_RX_DELAY;
} else if (phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID) {
- temp &= ~MII_M1111_RX_DELAY;
- temp |= MII_M1111_TX_DELAY;
+ temp &= ~MII_M1111_RGMII_RX_DELAY;
+ temp |= MII_M1111_RGMII_TX_DELAY;
}
- err = phy_write(phydev, MII_M1111_PHY_EXT_CR, temp);
+ return phy_write(phydev, MII_M1111_PHY_EXT_CR, temp);
+}
+
+static int m88e1111_config_init_rgmii(struct phy_device *phydev)
+{
+ int temp;
+ int err;
+
+ err = m88e1111_config_init_rgmii_delays(phydev);
if (err < 0)
return err;
@@ -760,16 +760,11 @@ static int m88e1111_config_init_sgmii(struct phy_device *phydev)
static int m88e1111_config_init_rtbi(struct phy_device *phydev)
{
- int err;
int temp;
+ int err;
- temp = phy_read(phydev, MII_M1111_PHY_EXT_CR);
- if (temp < 0)
- return temp;
-
- temp |= (MII_M1111_RX_DELAY | MII_M1111_TX_DELAY);
- err = phy_write(phydev, MII_M1111_PHY_EXT_CR, temp);
- if (err < 0)
+ err = m88e1111_config_init_rgmii_delays(phydev);
+ if (err)
return err;
temp = phy_read(phydev, MII_M1111_PHY_EXT_SR);
@@ -969,15 +964,10 @@ static int m88e1149_config_init(struct phy_device *phydev)
static int m88e1145_config_init_rgmii(struct phy_device *phydev)
{
+ int temp;
int err;
- int temp = phy_read(phydev, MII_M1145_PHY_EXT_CR);
-
- if (temp < 0)
- return temp;
-
- temp |= (MII_M1145_RGMII_RX_DELAY | MII_M1145_RGMII_TX_DELAY);
- err = phy_write(phydev, MII_M1145_PHY_EXT_CR, temp);
+ err = m88e1111_config_init_rgmii_delays(phydev);
if (err < 0)
return err;
--
2.13.2
^ permalink raw reply related
* [PATCH net-next 2/7] net: phy: marvell: Use core genphy_soft_reset()
From: Andrew Lunn @ 2017-07-30 20:41 UTC (permalink / raw)
To: David Miller; +Cc: netdev, Florian Fainelli, Andrew Lunn
In-Reply-To: <1501447310-24101-1-git-send-email-andrew@lunn.ch>
Rather than using an open coded equivalent, use the core
genphy_soft_reset() function.
Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
drivers/net/phy/marvell.c | 47 ++++++++++++-----------------------------------
1 file changed, 12 insertions(+), 35 deletions(-)
diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c
index 6a5256ceb11e..33a52532fac6 100644
--- a/drivers/net/phy/marvell.c
+++ b/drivers/net/phy/marvell.c
@@ -292,17 +292,11 @@ static int marvell_config_aneg(struct phy_device *phydev)
return err;
if (phydev->autoneg != AUTONEG_ENABLE) {
- int bmcr;
-
/* A write to speed/duplex bits (that is performed by
* genphy_config_aneg() call above) must be followed by
* a software reset. Otherwise, the write has no effect.
*/
- bmcr = phy_read(phydev, MII_BMCR);
- if (bmcr < 0)
- return bmcr;
-
- err = phy_write(phydev, MII_BMCR, bmcr | BMCR_RESET);
+ err = genphy_soft_reset(phydev);
if (err < 0)
return err;
}
@@ -318,8 +312,7 @@ static int m88e1101_config_aneg(struct phy_device *phydev)
* that certain registers get written in order
* to restart autonegotiation
*/
- err = phy_write(phydev, MII_BMCR, BMCR_RESET);
-
+ err = genphy_soft_reset(phydev);
if (err < 0)
return err;
@@ -354,7 +347,7 @@ static int m88e1111_config_aneg(struct phy_device *phydev)
* that certain registers get written in order
* to restart autonegotiation
*/
- err = phy_write(phydev, MII_BMCR, BMCR_RESET);
+ err = genphy_soft_reset(phydev);
err = marvell_set_polarity(phydev, phydev->mdix_ctrl);
if (err < 0)
@@ -370,17 +363,11 @@ static int m88e1111_config_aneg(struct phy_device *phydev)
return err;
if (phydev->autoneg != AUTONEG_ENABLE) {
- int bmcr;
-
/* A write to speed/duplex bits (that is performed by
* genphy_config_aneg() call above) must be followed by
* a software reset. Otherwise, the write has no effect.
*/
- bmcr = phy_read(phydev, MII_BMCR);
- if (bmcr < 0)
- return bmcr;
-
- err = phy_write(phydev, MII_BMCR, bmcr | BMCR_RESET);
+ err = genphy_soft_reset(phydev);
if (err < 0)
return err;
}
@@ -493,7 +480,7 @@ static int m88e1121_config_aneg(struct phy_device *phydev)
marvell_set_page(phydev, oldpage);
- err = phy_write(phydev, MII_BMCR, BMCR_RESET);
+ err = genphy_soft_reset(phydev);
if (err < 0)
return err;
@@ -656,9 +643,7 @@ static int m88e1116r_config_init(struct phy_device *phydev)
int temp;
int err;
- temp = phy_read(phydev, MII_BMCR);
- temp |= BMCR_RESET;
- err = phy_write(phydev, MII_BMCR, temp);
+ err = genphy_soft_reset(phydev);
if (err < 0)
return err;
@@ -689,14 +674,10 @@ static int m88e1116r_config_init(struct phy_device *phydev)
if (err < 0)
return err;
- temp = phy_read(phydev, MII_BMCR);
- temp |= BMCR_RESET;
- err = phy_write(phydev, MII_BMCR, temp);
+ err = genphy_soft_reset(phydev);
if (err < 0)
return err;
- mdelay(500);
-
return marvell_config_init(phydev);
}
@@ -804,14 +785,10 @@ static int m88e1111_config_init_rtbi(struct phy_device *phydev)
return err;
/* soft reset */
- err = phy_write(phydev, MII_BMCR, BMCR_RESET);
+ err = genphy_soft_reset(phydev);
if (err < 0)
return err;
- do
- temp = phy_read(phydev, MII_BMCR);
- while (temp & BMCR_RESET);
-
temp = phy_read(phydev, MII_M1111_PHY_EXT_SR);
if (temp < 0)
return temp;
@@ -850,7 +827,7 @@ static int m88e1111_config_init(struct phy_device *phydev)
if (err < 0)
return err;
- return phy_write(phydev, MII_BMCR, BMCR_RESET);
+ return genphy_soft_reset(phydev);
}
static int m88e1121_config_init(struct phy_device *phydev)
@@ -912,7 +889,7 @@ static int m88e1118_config_aneg(struct phy_device *phydev)
{
int err;
- err = phy_write(phydev, MII_BMCR, BMCR_RESET);
+ err = genphy_soft_reset(phydev);
if (err < 0)
return err;
@@ -961,7 +938,7 @@ static int m88e1118_config_init(struct phy_device *phydev)
if (err < 0)
return err;
- return phy_write(phydev, MII_BMCR, BMCR_RESET);
+ return genphy_soft_reset(phydev);
}
static int m88e1149_config_init(struct phy_device *phydev)
@@ -987,7 +964,7 @@ static int m88e1149_config_init(struct phy_device *phydev)
if (err < 0)
return err;
- return phy_write(phydev, MII_BMCR, BMCR_RESET);
+ return genphy_soft_reset(phydev);
}
static int m88e1145_config_init_rgmii(struct phy_device *phydev)
--
2.13.2
^ permalink raw reply related
* [PATCH net-next 0/7] More Marvell PHY refactoring and cleanup
From: Andrew Lunn @ 2017-07-30 20:41 UTC (permalink / raw)
To: David Miller; +Cc: netdev, Florian Fainelli, Andrew Lunn
Consolidate more duplicated code into helpers, make use of core
helpers, move code into a helper for later adding functionality to add
marvell PHYs, etc.
Andrew Lunn (7):
net: phy: marvell: tabification
net: phy: marvell: Use core genphy_soft_reset()
net: phy: marvell: consolidate RGMII delay code
net: phy: marvell: Consolidate setting the phy-mode
net: phy: marvell: Refactor m88e1121 RGMII delay configuration
net: phy: marvell: Use the set_polarity helper
net: phy: marvell: Refactor setting downshift into a helper
drivers/net/phy/marvell.c | 320 ++++++++++++++++++++++------------------------
1 file changed, 150 insertions(+), 170 deletions(-)
--
2.13.2
^ permalink raw reply
* [PATCH net-next 4/7] net: phy: marvell: Consolidate setting the phy-mode
From: Andrew Lunn @ 2017-07-30 20:41 UTC (permalink / raw)
To: David Miller; +Cc: netdev, Florian Fainelli, Andrew Lunn
In-Reply-To: <1501447310-24101-1-git-send-email-andrew@lunn.ch>
The same code is repeated a few times. Refactor into a helped.
Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
drivers/net/phy/marvell.c | 88 +++++++++++++++++++++--------------------------
1 file changed, 40 insertions(+), 48 deletions(-)
diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c
index c1b724ab5f25..275647ebaa81 100644
--- a/drivers/net/phy/marvell.c
+++ b/drivers/net/phy/marvell.c
@@ -60,11 +60,6 @@
#define MII_M1011_PHY_SCR_MDI_X 0x0020
#define MII_M1011_PHY_SCR_AUTO_CROSS 0x0060
-#define MII_M1145_PHY_EXT_SR 0x1b
-#define MII_M1145_HWCFG_MODE_SGMII_NO_CLK 0x4
-#define MII_M1145_HWCFG_MODE_MASK 0xf
-#define MII_M1145_HWCFG_FIBER_COPPER_AUTO 0x8000
-
#define MII_M1111_PHY_LED_CONTROL 0x18
#define MII_M1111_PHY_LED_DIRECT 0x4100
#define MII_M1111_PHY_LED_COMBINE 0x411c
@@ -74,12 +69,13 @@
#define MII_M1111_PHY_EXT_SR 0x1b
#define MII_M1111_HWCFG_MODE_MASK 0xf
-#define MII_M1111_HWCFG_MODE_COPPER_RGMII 0xb
#define MII_M1111_HWCFG_MODE_FIBER_RGMII 0x3
#define MII_M1111_HWCFG_MODE_SGMII_NO_CLK 0x4
+#define MII_M1111_HWCFG_MODE_RTBI 0x7
#define MII_M1111_HWCFG_MODE_COPPER_RTBI 0x9
-#define MII_M1111_HWCFG_FIBER_COPPER_AUTO 0x8000
-#define MII_M1111_HWCFG_FIBER_COPPER_RES 0x2000
+#define MII_M1111_HWCFG_MODE_COPPER_RGMII 0xb
+#define MII_M1111_HWCFG_FIBER_COPPER_RES BIT(13)
+#define MII_M1111_HWCFG_FIBER_COPPER_AUTO BIT(15)
#define MII_88E1121_PHY_MSCR_REG 21
#define MII_88E1121_PHY_MSCR_RX_DELAY BIT(5)
@@ -693,6 +689,27 @@ static int m88e3016_config_init(struct phy_device *phydev)
return marvell_config_init(phydev);
}
+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;
+
+ return phy_write(phydev, MII_M1111_PHY_EXT_SR, temp);
+}
+
static int m88e1111_config_init_rgmii_delays(struct phy_device *phydev)
{
int temp;
@@ -740,17 +757,11 @@ static int m88e1111_config_init_rgmii(struct phy_device *phydev)
static int m88e1111_config_init_sgmii(struct phy_device *phydev)
{
int err;
- int temp;
- temp = phy_read(phydev, MII_M1111_PHY_EXT_SR);
- if (temp < 0)
- return temp;
-
- temp &= ~(MII_M1111_HWCFG_MODE_MASK);
- temp |= MII_M1111_HWCFG_MODE_SGMII_NO_CLK;
- temp |= MII_M1111_HWCFG_FIBER_COPPER_AUTO;
-
- err = phy_write(phydev, MII_M1111_PHY_EXT_SR, temp);
+ err = m88e1111_config_init_hwcfg_mode(
+ phydev,
+ MII_M1111_HWCFG_MODE_SGMII_NO_CLK,
+ MII_M1111_HWCFG_FIBER_COPPER_AUTO);
if (err < 0)
return err;
@@ -760,22 +771,16 @@ static int m88e1111_config_init_sgmii(struct phy_device *phydev)
static int m88e1111_config_init_rtbi(struct phy_device *phydev)
{
- int temp;
int err;
err = m88e1111_config_init_rgmii_delays(phydev);
if (err)
return err;
- 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_RES);
- temp |= 0x7 | MII_M1111_HWCFG_FIBER_COPPER_AUTO;
-
- err = phy_write(phydev, MII_M1111_PHY_EXT_SR, temp);
+ err = m88e1111_config_init_hwcfg_mode(
+ phydev,
+ MII_M1111_HWCFG_MODE_RTBI,
+ MII_M1111_HWCFG_FIBER_COPPER_AUTO);
if (err < 0)
return err;
@@ -784,16 +789,10 @@ static int m88e1111_config_init_rtbi(struct phy_device *phydev)
if (err < 0)
return err;
- 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_RES);
- temp |= MII_M1111_HWCFG_MODE_COPPER_RTBI |
- MII_M1111_HWCFG_FIBER_COPPER_AUTO;
-
- return phy_write(phydev, MII_M1111_PHY_EXT_SR, temp);
+ return m88e1111_config_init_hwcfg_mode(
+ phydev,
+ MII_M1111_HWCFG_MODE_RTBI,
+ MII_M1111_HWCFG_FIBER_COPPER_AUTO);
}
static int m88e1111_config_init(struct phy_device *phydev)
@@ -999,16 +998,9 @@ static int m88e1145_config_init_rgmii(struct phy_device *phydev)
static int m88e1145_config_init_sgmii(struct phy_device *phydev)
{
- int temp = phy_read(phydev, MII_M1145_PHY_EXT_SR);
-
- if (temp < 0)
- return temp;
-
- temp &= ~MII_M1145_HWCFG_MODE_MASK;
- temp |= MII_M1145_HWCFG_MODE_SGMII_NO_CLK;
- temp |= MII_M1145_HWCFG_FIBER_COPPER_AUTO;
-
- return phy_write(phydev, MII_M1145_PHY_EXT_SR, temp);
+ return m88e1111_config_init_hwcfg_mode(
+ phydev, MII_M1111_HWCFG_MODE_SGMII_NO_CLK,
+ MII_M1111_HWCFG_FIBER_COPPER_AUTO);
}
static int m88e1145_config_init(struct phy_device *phydev)
--
2.13.2
^ permalink raw reply related
* [PATCH net-next 6/7] net: phy: marvell: Use the set_polarity helper
From: Andrew Lunn @ 2017-07-30 20:41 UTC (permalink / raw)
To: David Miller; +Cc: netdev, Florian Fainelli, Andrew Lunn
In-Reply-To: <1501447310-24101-1-git-send-email-andrew@lunn.ch>
Some of the init functions unilaterally enable set auto cross over
without using the helper. Make use of the helper, and respect the
phydev MDI configuration.
Clean up the #define used while setting polarity, and the other
functions of the bits in the register.
Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
drivers/net/phy/marvell.c | 25 ++++++++++++++-----------
1 file changed, 14 insertions(+), 11 deletions(-)
diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c
index 408442bdef0a..34fd15b904e7 100644
--- a/drivers/net/phy/marvell.c
+++ b/drivers/net/phy/marvell.c
@@ -55,10 +55,12 @@
#define MII_M1011_IMASK_INIT 0x6400
#define MII_M1011_IMASK_CLEAR 0x0000
-#define MII_M1011_PHY_SCR 0x10
-#define MII_M1011_PHY_SCR_MDI 0x0000
-#define MII_M1011_PHY_SCR_MDI_X 0x0020
-#define MII_M1011_PHY_SCR_AUTO_CROSS 0x0060
+#define MII_M1011_PHY_SCR 0x10
+#define MII_M1011_PHY_SCR_DOWNSHIFT_EN BIT(11)
+#define MII_M1011_PHY_SCR_DOWNSHIFT_SHIFT 12
+#define MII_M1011_PHY_SCR_MDI (0x0 << 5)
+#define MII_M1011_PHY_SCR_MDI_X (0x1 << 5)
+#define MII_M1011_PHY_SCR_AUTO_CROSS (0x3 << 5)
#define MII_M1111_PHY_LED_CONTROL 0x18
#define MII_M1111_PHY_LED_DIRECT 0x4100
@@ -486,8 +488,7 @@ static int m88e1121_config_aneg(struct phy_device *phydev)
if (err < 0)
return err;
- err = phy_write(phydev, MII_M1011_PHY_SCR,
- MII_M1011_PHY_SCR_AUTO_CROSS);
+ err = marvell_set_polarity(phydev, phydev->mdix_ctrl);
if (err < 0)
return err;
@@ -655,10 +656,13 @@ static int m88e1116r_config_init(struct phy_device *phydev)
if (err < 0)
return err;
+ err = marvell_set_polarity(phydev, phydev->mdix_ctrl);
+ if (err < 0)
+ return err;
+
temp = phy_read(phydev, MII_M1011_PHY_SCR);
- temp |= (7 << 12); /* max number of gigabit attempts */
- temp |= (1 << 11); /* enable downshift */
- temp |= MII_M1011_PHY_SCR_AUTO_CROSS;
+ temp |= (7 << MII_M1011_PHY_SCR_DOWNSHIFT_SHIFT);
+ temp |= MII_M1011_PHY_SCR_DOWNSHIFT_EN;
err = phy_write(phydev, MII_M1011_PHY_SCR, temp);
if (err < 0)
return err;
@@ -891,8 +895,7 @@ static int m88e1118_config_aneg(struct phy_device *phydev)
if (err < 0)
return err;
- err = phy_write(phydev, MII_M1011_PHY_SCR,
- MII_M1011_PHY_SCR_AUTO_CROSS);
+ err = marvell_set_polarity(phydev, phydev->mdix_ctrl);
if (err < 0)
return err;
--
2.13.2
^ permalink raw reply related
* [PATCH net-next 5/7] net: phy: marvell: Refactor m88e1121 RGMII delay configuration
From: Andrew Lunn @ 2017-07-30 20:41 UTC (permalink / raw)
To: David Miller; +Cc: netdev, Florian Fainelli, Andrew Lunn
In-Reply-To: <1501447310-24101-1-git-send-email-andrew@lunn.ch>
Turns out that MII_M1116R_CONTROL_REG_MAC is the same as
MII_88E1121_PHY_MSCR_REG. Refactor the code to set the RGMII delays
into a shared helper.
Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
drivers/net/phy/marvell.c | 62 +++++++++++++++++++++++++----------------------
1 file changed, 33 insertions(+), 29 deletions(-)
diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c
index 275647ebaa81..408442bdef0a 100644
--- a/drivers/net/phy/marvell.c
+++ b/drivers/net/phy/marvell.c
@@ -80,7 +80,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 (~(0x3 << 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
@@ -127,8 +127,6 @@
#define MII_M1011_PHY_STATUS_RESOLVED 0x0800
#define MII_M1011_PHY_STATUS_LINK 0x0400
-#define MII_M1116R_CONTROL_REG_MAC 21
-
#define MII_88E3016_PHY_SPEC_CTRL 0x10
#define MII_88E3016_DISABLE_SCRAMBLER 0x0200
#define MII_88E3016_AUTO_MDIX_CROSSOVER 0x0030
@@ -442,7 +440,7 @@ static int marvell_of_reg_init(struct phy_device *phydev)
}
#endif /* CONFIG_OF_MDIO */
-static int m88e1121_config_aneg(struct phy_device *phydev)
+static int m88e1121_config_aneg_rgmii_delays(struct phy_device *phydev)
{
int err, oldpage, mscr;
@@ -450,25 +448,40 @@ static int m88e1121_config_aneg(struct phy_device *phydev)
if (oldpage < 0)
return oldpage;
- if (phy_interface_is_rgmii(phydev)) {
- mscr = phy_read(phydev, MII_88E1121_PHY_MSCR_REG) &
- MII_88E1121_PHY_MSCR_DELAY_MASK;
-
- if (phydev->interface == PHY_INTERFACE_MODE_RGMII_ID)
- 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;
- 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);
- if (err < 0)
- return err;
+ mscr = phy_read(phydev, MII_88E1121_PHY_MSCR_REG);
+ if (mscr < 0) {
+ err = mscr;
+ goto out;
}
+ mscr &= MII_88E1121_PHY_MSCR_DELAY_MASK;
+
+ if (phydev->interface == PHY_INTERFACE_MODE_RGMII_ID)
+ 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;
+ 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);
+ return err;
+}
+
+static int m88e1121_config_aneg(struct phy_device *phydev)
+{
+ int err = 0;
+
+ if (phy_interface_is_rgmii(phydev)) {
+ err = m88e1121_config_aneg_rgmii_delays(phydev);
+ if (err)
+ return err;
+ }
+
err = genphy_soft_reset(phydev);
if (err < 0)
return err;
@@ -650,16 +663,7 @@ static int m88e1116r_config_init(struct phy_device *phydev)
if (err < 0)
return err;
- err = marvell_set_page(phydev, MII_MARVELL_MSCR_PAGE);
- if (err < 0)
- return err;
- temp = phy_read(phydev, MII_M1116R_CONTROL_REG_MAC);
- temp |= (1 << 5);
- temp |= (1 << 4);
- err = phy_write(phydev, MII_M1116R_CONTROL_REG_MAC, temp);
- if (err < 0)
- return err;
- err = marvell_set_page(phydev, MII_MARVELL_COPPER_PAGE);
+ err = m88e1121_config_aneg_rgmii_delays(phydev);
if (err < 0)
return err;
--
2.13.2
^ permalink raw reply related
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