Netdev List
 help / color / mirror / Atom feed
* Re: [RFC RFT PATCH v4 1/4] gpiolib: Pass bitmaps, not integer arrays, to get/set array
From: Janusz Krzysztofik @ 2018-08-29 18:01 UTC (permalink / raw)
  To: Miguel Ojeda
  Cc: Linus Walleij, Jonathan Corbet, Peter Korsgaard, Peter Rosin,
	Ulf Hansson, Andrew Lunn, Florian Fainelli, David S. Miller,
	Dominik Brodowski, Kishon Vijay Abraham I, Lars-Peter Clausen,
	Michael Hennerich, Jonathan Cameron, Hartmut Knaack,
	Peter Meerwald-Stadler, Greg Kroah-Hartman, Jiri Slaby,
	linux-gpio, Linux Doc Mailing List
In-Reply-To: <CANiq72kM9bYJ1Q+dbLumjfQLZW223ZTrYEFqfQ2Jv2SAjrD1NA@mail.gmail.com>

On Wednesday, August 29, 2018 2:03:18 PM CEST Miguel Ojeda wrote:
> Hi Janusz,
> 
> On Tue, Aug 21, 2018 at 1:43 AM, Janusz Krzysztofik <jmkrzyszt@gmail.com> wrote:
> > Most users of get/set array functions iterate consecutive bits of data,
> > usually a single integer, while or processing array of results obtained
> > from or building an array of values to be passed to those functions.
> > Save time wasted on those iterations by changing the functions' API to
> > accept bitmaps.
> >
> > All current users are updated as well.
> >
> > More benefits from the change are expected as soon as planned support
> > for accepting/passing those bitmaps directly from/to respective GPIO
> > chip callbacks if applicable is implemented.
> >
> > Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com>
> > ---
> >  Documentation/driver-api/gpio/consumer.rst  | 22 ++++----
> >  drivers/auxdisplay/hd44780.c                | 52 +++++++++--------
> 
> [CC'ing Willy and Geert for hd44780]
> 
> > diff --git a/drivers/auxdisplay/hd44780.c b/drivers/auxdisplay/hd44780.c
> > index f1a42f0f1ded..d340473aa142 100644
> > --- a/drivers/auxdisplay/hd44780.c
> > +++ b/drivers/auxdisplay/hd44780.c
> > @@ -62,20 +62,19 @@ static void hd44780_strobe_gpio(struct hd44780 *hd)
> >  /* write to an LCD panel register in 8 bit GPIO mode */
> >  static void hd44780_write_gpio8(struct hd44780 *hd, u8 val, unsigned int rs)
> >  {
> > -       int values[10]; /* for DATA[0-7], RS, RW */
> > -       unsigned int i, n;
> > +       unsigned long value_bitmap[1];  /* for DATA[0-7], RS, RW */
> 
> Why [1]? I understand it is because in other cases it may be more than
> one,

Yes, I tried to point out the fact the new API accepts a bitmap of an arbitrary 
length, and I tried to use the same code pattern across changes to the API 
users.

> but...
> 
> > +       unsigned int n;
> >
> > -       for (i = 0; i < 8; i++)
> > -               values[PIN_DATA0 + i] = !!(val & BIT(i));
> > -       values[PIN_CTRL_RS] = rs;
> > +       value_bitmap[0] = val;
> > +       __assign_bit(PIN_CTRL_RS, value_bitmap, rs);
> >         n = 9;
> >         if (hd->pins[PIN_CTRL_RW]) {
> > -               values[PIN_CTRL_RW] = 0;
> > +               __clear_bit(PIN_CTRL_RW, value_bitmap);
> >                 n++;
> >         }
> >
> >         /* Present the data to the port */
> > -       gpiod_set_array_value_cansleep(n, &hd->pins[PIN_DATA0], values);
> > +       gpiod_set_array_value_cansleep(n, &hd->pins[PIN_DATA0], value_bitmap);
> >
> >         hd44780_strobe_gpio(hd);
> >  }
> > @@ -83,32 +82,31 @@ static void hd44780_write_gpio8(struct hd44780 *hd, u8 val, unsigned int rs)
> >  /* write to an LCD panel register in 4 bit GPIO mode */
> >  static void hd44780_write_gpio4(struct hd44780 *hd, u8 val, unsigned int rs)
> >  {
> > -       int values[10]; /* for DATA[0-7], RS, RW, but DATA[0-3] is unused */
> > -       unsigned int i, n;
> > +       /* for DATA[0-7], RS, RW, but DATA[0-3] is unused */
> > +       unsigned long value_bitmap[0];
> 
> This one is even more strange... :-)

This one is an error, should be 1 of course :-), thanks.

> > +       unsigned int n;
> >
> >         /* High nibble + RS, RW */
> > -       for (i = 4; i < 8; i++)
> > -               values[PIN_DATA0 + i] = !!(val & BIT(i));
> > -       values[PIN_CTRL_RS] = rs;
> > +       value_bitmap[0] = val;
> > +       __assign_bit(PIN_CTRL_RS, value_bitmap, rs);
> >         n = 5;
> >         if (hd->pins[PIN_CTRL_RW]) {
> > -               values[PIN_CTRL_RW] = 0;
> > +               __clear_bit(PIN_CTRL_RW, value_bitmap);
> >                 n++;
> >         }
> > +       value_bitmap[0] = value_bitmap[0] >> PIN_DATA4;
> 
> Maybe >>=?

OK.

Answering you question below:
To make my changes as clear as I could imagine, I decided to use the same indexing as in the original code, i.e., assign high nibble of val to bits 4-7 and two other values - rs and an optional 0 - to bits 8 and 9, respectively.
Unlike in case of array of integers, where for the high nibble part you could just pass a pointer to a sub-array starting at the 5th value (i.e., &values[PIN_DATA4]), it was not possible to do the same for and arbitrary bit of a bitmap, e.g., pass a pointer to the 5th bit of *value_bitmap as an argument pointing to bit 0 of a bitmap to be processed. That's why I shifted the bitmap right by 4 bits.
Then, ...

> >
> >         /* Present the data to the port */
> > -       gpiod_set_array_value_cansleep(n, &hd->pins[PIN_DATA4],
> > -                                      &values[PIN_DATA4]);
> > +       gpiod_set_array_value_cansleep(n, &hd->pins[PIN_DATA4], value_bitmap);
> >
> >         hd44780_strobe_gpio(hd);
> >
> >         /* Low nibble */
> > -       for (i = 0; i < 4; i++)
> > -               values[PIN_DATA4 + i] = !!(val & BIT(i));
> > +       value_bitmap[0] &= ~((1 << PIN_DATA4) - 1);
> > +       value_bitmap[0] |= val & ~((1 << PIN_DATA4) - 1);
> 
> Are you sure this is correct? You are basically doing an or of
> value_bitmap and val and clearing the low-nibble.

having the rs and optional 0 already assigned to bits 4 and 5 of the bitmap, I just cleared bits 0-3 still containing the high nibble of val and assigned the low nibble of it to those bits, getting a result ready to be passed as an argument to gpiod_set_array_value_cansleep() below.

I hope I didn't miss anything.

Thanks,
Janusz

> >
> >         /* Present the data to the port */
> > -       gpiod_set_array_value_cansleep(n, &hd->pins[PIN_DATA4],
> > -                                      &values[PIN_DATA4]);
> > +       gpiod_set_array_value_cansleep(n, &hd->pins[PIN_DATA4], value_bitmap);
> >
> >         hd44780_strobe_gpio(hd);
> >  }
> 
> Cheers,
> Miguel
> 

^ permalink raw reply

* Re: [PATCH bpf 0/3] Three fixes for bpf_msg_pull_data
From: Alexei Starovoitov @ 2018-08-29 17:52 UTC (permalink / raw)
  To: Daniel Borkmann; +Cc: john.fastabend, netdev
In-Reply-To: <20180829145036.5514-1-daniel@iogearbox.net>

On Wed, Aug 29, 2018 at 04:50:33PM +0200, Daniel Borkmann wrote:
> This set contains three more fixes for the bpf_msg_pull_data()
> mainly for correcting scatterlist ring wrap-arounds as well as
> fixing up data pointers. For details please see individual patches.
> Thanks!

Applied to bpf tree, Thanks

^ permalink raw reply

* Re: [Bug #16626] Machine hangs with EIP at skb_copy_and_csum_dev
From: Mitchell Erblich @ 2018-08-29 21:39 UTC (permalink / raw)
  To: Jarek Poplawski
  Cc: Plamen Petrov, Rafael J. Wysocki, Kernel Testers List,
	Maciej Rutecki, David S. Miller, Eric Dumazet,
	Linux Kernel Mailing List, netdev
In-Reply-To: <20100831192659.GA3093@del.dom.local>

						OLD REPLY … and Correcting . Suggesting of Corrections of LONG TERM embedded problems 

Summary:

		Due to watermark awareness differences that is problematic in embedded systems, the GFP_ATOMIC which is not memory watermark aware is used in interrupt / atomic context.

		To properly monitor WATERMARK levels at suggested kernel locations, the “PROPER” GFP_FLAG SHOULD be GFP_NOWAIT. This is atomic/interupt friendly and is aware of memory watermarks, thus if memory is below the specified watermark level, it will then return a ENOMEM from that location.

					GFP_ATOMIC will not return ENOMEM and by the time the watermarks drop, ALL GFP_KERNEL callers are now SLEEPING.

					Please be embedded friendly with code patches…

		FYI: Embedded system due to no 2ndary drive can not clean PTEs (page frames), thus callers need to be aware of low memory issues and be able to reduce their memory consumption based on receiving ENOMEMs. GFP_KERNEL callers will just sleep until memory is back above the watermarks.

Mitchell Erblich
erblichs@earthlink.net



> On Aug 31, 2010, at 12:26 PM, Jarek Poplawski <jarkao2@gmail.com> wrote:
> 
> On Tue, Aug 31, 2010 at 08:58:10AM +0300, Plamen Petrov wrote:
>> Rafael J. Wysocki ????????????:
>> 
>>> This message has been generated automatically as a part of a summary report
>>> of recent regressions.
>>> 
>>> The following bug entry is on the current list of known regressions
>>> from 2.6.35.  Please verify if it still should be listed and let the tracking team
>>> know (either way).
>>> 
>>> 
>>> Bug-Entry	: http://bugzilla.kernel.org/show_bug.cgi?id=16626
>>> Subject		: Machine hangs with EIP at skb_copy_and_csum_dev
>>> Submitter	: Plamen Petrov <pvp-lsts@fs.uni-ruse.bg>
>>> Date		: 2010-08-19 09:57 (11 days old)
>>> Handled-By	:  Eric Dumazet <eric.dumazet@gmail.com>
>>> 
>>> 
>> 
>> Should "generic receive offload" work on a forwarding setup?
>> If yes - then the bug should remain open.
>> If not - then it's my mistake.
> 
> If/since it's on by default it should work and it definitely can't be
> your mistake. (Unless we can't find the real reason... ;-)
> 
> Jarek P.
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* [Patch net-nnext] Revert "net: sched: act: add extack for lookup callback"
From: Cong Wang @ 2018-08-29 17:15 UTC (permalink / raw)
  To: netdev; +Cc: Cong Wang, Alexander Aring

This reverts commit 331a9295de23 ("net: sched: act: add extack for lookup callback").

This extack is never used after 6 months... In fact, it can be just
set in the caller, right after ->lookup().

Cc: Alexander Aring <aring@mojatatu.com>
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
---
 include/net/act_api.h      | 3 +--
 net/sched/act_api.c        | 6 ++++--
 net/sched/act_bpf.c        | 3 +--
 net/sched/act_connmark.c   | 3 +--
 net/sched/act_csum.c       | 3 +--
 net/sched/act_gact.c       | 3 +--
 net/sched/act_ife.c        | 3 +--
 net/sched/act_ipt.c        | 6 ++----
 net/sched/act_mirred.c     | 3 +--
 net/sched/act_nat.c        | 3 +--
 net/sched/act_pedit.c      | 3 +--
 net/sched/act_police.c     | 3 +--
 net/sched/act_sample.c     | 3 +--
 net/sched/act_simple.c     | 3 +--
 net/sched/act_skbedit.c    | 3 +--
 net/sched/act_skbmod.c     | 3 +--
 net/sched/act_tunnel_key.c | 3 +--
 net/sched/act_vlan.c       | 3 +--
 18 files changed, 22 insertions(+), 38 deletions(-)

diff --git a/include/net/act_api.h b/include/net/act_api.h
index 970303448c90..c6f195b3c706 100644
--- a/include/net/act_api.h
+++ b/include/net/act_api.h
@@ -85,8 +85,7 @@ struct tc_action_ops {
 		       struct tcf_result *); /* called under RCU BH lock*/
 	int     (*dump)(struct sk_buff *, struct tc_action *, int, int);
 	void	(*cleanup)(struct tc_action *);
-	int     (*lookup)(struct net *net, struct tc_action **a, u32 index,
-			  struct netlink_ext_ack *extack);
+	int     (*lookup)(struct net *net, struct tc_action **a, u32 index);
 	int     (*init)(struct net *net, struct nlattr *nla,
 			struct nlattr *est, struct tc_action **act, int ovr,
 			int bind, bool rtnl_held,
diff --git a/net/sched/act_api.c b/net/sched/act_api.c
index db83dac1e7f4..398c752ff529 100644
--- a/net/sched/act_api.c
+++ b/net/sched/act_api.c
@@ -1067,12 +1067,14 @@ static struct tc_action *tcf_action_get_1(struct net *net, struct nlattr *nla,
 	err = -EINVAL;
 	ops = tc_lookup_action(tb[TCA_ACT_KIND]);
 	if (!ops) { /* could happen in batch of actions */
-		NL_SET_ERR_MSG(extack, "Specified TC action not found");
+		NL_SET_ERR_MSG(extack, "Specified TC action kind not found");
 		goto err_out;
 	}
 	err = -ENOENT;
-	if (ops->lookup(net, &a, index, extack) == 0)
+	if (ops->lookup(net, &a, index) == 0) {
+		NL_SET_ERR_MSG(extack, "TC action with specified index not found");
 		goto err_mod;
+	}
 
 	module_put(ops->owner);
 	return a;
diff --git a/net/sched/act_bpf.c b/net/sched/act_bpf.c
index 0c68bc9cf0b4..c7633843e223 100644
--- a/net/sched/act_bpf.c
+++ b/net/sched/act_bpf.c
@@ -387,8 +387,7 @@ static int tcf_bpf_walker(struct net *net, struct sk_buff *skb,
 	return tcf_generic_walker(tn, skb, cb, type, ops, extack);
 }
 
-static int tcf_bpf_search(struct net *net, struct tc_action **a, u32 index,
-			  struct netlink_ext_ack *extack)
+static int tcf_bpf_search(struct net *net, struct tc_action **a, u32 index)
 {
 	struct tc_action_net *tn = net_generic(net, bpf_net_id);
 
diff --git a/net/sched/act_connmark.c b/net/sched/act_connmark.c
index 6f0f273f1139..e869c0ee63c8 100644
--- a/net/sched/act_connmark.c
+++ b/net/sched/act_connmark.c
@@ -190,8 +190,7 @@ static int tcf_connmark_walker(struct net *net, struct sk_buff *skb,
 	return tcf_generic_walker(tn, skb, cb, type, ops, extack);
 }
 
-static int tcf_connmark_search(struct net *net, struct tc_action **a, u32 index,
-			       struct netlink_ext_ack *extack)
+static int tcf_connmark_search(struct net *net, struct tc_action **a, u32 index)
 {
 	struct tc_action_net *tn = net_generic(net, connmark_net_id);
 
diff --git a/net/sched/act_csum.c b/net/sched/act_csum.c
index b8a67ae3105a..3dc25b7806d7 100644
--- a/net/sched/act_csum.c
+++ b/net/sched/act_csum.c
@@ -646,8 +646,7 @@ static int tcf_csum_walker(struct net *net, struct sk_buff *skb,
 	return tcf_generic_walker(tn, skb, cb, type, ops, extack);
 }
 
-static int tcf_csum_search(struct net *net, struct tc_action **a, u32 index,
-			   struct netlink_ext_ack *extack)
+static int tcf_csum_search(struct net *net, struct tc_action **a, u32 index)
 {
 	struct tc_action_net *tn = net_generic(net, csum_net_id);
 
diff --git a/net/sched/act_gact.c b/net/sched/act_gact.c
index cd1d9bd32ef9..aa44d14b43c7 100644
--- a/net/sched/act_gact.c
+++ b/net/sched/act_gact.c
@@ -222,8 +222,7 @@ static int tcf_gact_walker(struct net *net, struct sk_buff *skb,
 	return tcf_generic_walker(tn, skb, cb, type, ops, extack);
 }
 
-static int tcf_gact_search(struct net *net, struct tc_action **a, u32 index,
-			   struct netlink_ext_ack *extack)
+static int tcf_gact_search(struct net *net, struct tc_action **a, u32 index)
 {
 	struct tc_action_net *tn = net_generic(net, gact_net_id);
 
diff --git a/net/sched/act_ife.c b/net/sched/act_ife.c
index 196430aefe87..19454146f60d 100644
--- a/net/sched/act_ife.c
+++ b/net/sched/act_ife.c
@@ -841,8 +841,7 @@ static int tcf_ife_walker(struct net *net, struct sk_buff *skb,
 	return tcf_generic_walker(tn, skb, cb, type, ops, extack);
 }
 
-static int tcf_ife_search(struct net *net, struct tc_action **a, u32 index,
-			  struct netlink_ext_ack *extack)
+static int tcf_ife_search(struct net *net, struct tc_action **a, u32 index)
 {
 	struct tc_action_net *tn = net_generic(net, ife_net_id);
 
diff --git a/net/sched/act_ipt.c b/net/sched/act_ipt.c
index 23273b5303fd..1efbfb10b1fc 100644
--- a/net/sched/act_ipt.c
+++ b/net/sched/act_ipt.c
@@ -329,8 +329,7 @@ static int tcf_ipt_walker(struct net *net, struct sk_buff *skb,
 	return tcf_generic_walker(tn, skb, cb, type, ops, extack);
 }
 
-static int tcf_ipt_search(struct net *net, struct tc_action **a, u32 index,
-			  struct netlink_ext_ack *extack)
+static int tcf_ipt_search(struct net *net, struct tc_action **a, u32 index)
 {
 	struct tc_action_net *tn = net_generic(net, ipt_net_id);
 
@@ -379,8 +378,7 @@ static int tcf_xt_walker(struct net *net, struct sk_buff *skb,
 	return tcf_generic_walker(tn, skb, cb, type, ops, extack);
 }
 
-static int tcf_xt_search(struct net *net, struct tc_action **a, u32 index,
-			 struct netlink_ext_ack *extack)
+static int tcf_xt_search(struct net *net, struct tc_action **a, u32 index)
 {
 	struct tc_action_net *tn = net_generic(net, xt_net_id);
 
diff --git a/net/sched/act_mirred.c b/net/sched/act_mirred.c
index 8bf66d0a6800..a9d64bfe5a2a 100644
--- a/net/sched/act_mirred.c
+++ b/net/sched/act_mirred.c
@@ -338,8 +338,7 @@ static int tcf_mirred_walker(struct net *net, struct sk_buff *skb,
 	return tcf_generic_walker(tn, skb, cb, type, ops, extack);
 }
 
-static int tcf_mirred_search(struct net *net, struct tc_action **a, u32 index,
-			     struct netlink_ext_ack *extack)
+static int tcf_mirred_search(struct net *net, struct tc_action **a, u32 index)
 {
 	struct tc_action_net *tn = net_generic(net, mirred_net_id);
 
diff --git a/net/sched/act_nat.c b/net/sched/act_nat.c
index 4313aa102440..d98f33fdffe2 100644
--- a/net/sched/act_nat.c
+++ b/net/sched/act_nat.c
@@ -292,8 +292,7 @@ static int tcf_nat_walker(struct net *net, struct sk_buff *skb,
 	return tcf_generic_walker(tn, skb, cb, type, ops, extack);
 }
 
-static int tcf_nat_search(struct net *net, struct tc_action **a, u32 index,
-			  struct netlink_ext_ack *extack)
+static int tcf_nat_search(struct net *net, struct tc_action **a, u32 index)
 {
 	struct tc_action_net *tn = net_generic(net, nat_net_id);
 
diff --git a/net/sched/act_pedit.c b/net/sched/act_pedit.c
index 107034070019..6d6a9450e8ad 100644
--- a/net/sched/act_pedit.c
+++ b/net/sched/act_pedit.c
@@ -452,8 +452,7 @@ static int tcf_pedit_walker(struct net *net, struct sk_buff *skb,
 	return tcf_generic_walker(tn, skb, cb, type, ops, extack);
 }
 
-static int tcf_pedit_search(struct net *net, struct tc_action **a, u32 index,
-			    struct netlink_ext_ack *extack)
+static int tcf_pedit_search(struct net *net, struct tc_action **a, u32 index)
 {
 	struct tc_action_net *tn = net_generic(net, pedit_net_id);
 
diff --git a/net/sched/act_police.c b/net/sched/act_police.c
index 5d8bfa878477..393c7a670300 100644
--- a/net/sched/act_police.c
+++ b/net/sched/act_police.c
@@ -312,8 +312,7 @@ static int tcf_police_dump(struct sk_buff *skb, struct tc_action *a,
 	return -1;
 }
 
-static int tcf_police_search(struct net *net, struct tc_action **a, u32 index,
-			     struct netlink_ext_ack *extack)
+static int tcf_police_search(struct net *net, struct tc_action **a, u32 index)
 {
 	struct tc_action_net *tn = net_generic(net, police_net_id);
 
diff --git a/net/sched/act_sample.c b/net/sched/act_sample.c
index 44e9c00657bc..83a133375d6d 100644
--- a/net/sched/act_sample.c
+++ b/net/sched/act_sample.c
@@ -224,8 +224,7 @@ static int tcf_sample_walker(struct net *net, struct sk_buff *skb,
 	return tcf_generic_walker(tn, skb, cb, type, ops, extack);
 }
 
-static int tcf_sample_search(struct net *net, struct tc_action **a, u32 index,
-			     struct netlink_ext_ack *extack)
+static int tcf_sample_search(struct net *net, struct tc_action **a, u32 index)
 {
 	struct tc_action_net *tn = net_generic(net, sample_net_id);
 
diff --git a/net/sched/act_simple.c b/net/sched/act_simple.c
index 52400d49f81f..902957beceb3 100644
--- a/net/sched/act_simple.c
+++ b/net/sched/act_simple.c
@@ -188,8 +188,7 @@ static int tcf_simp_walker(struct net *net, struct sk_buff *skb,
 	return tcf_generic_walker(tn, skb, cb, type, ops, extack);
 }
 
-static int tcf_simp_search(struct net *net, struct tc_action **a, u32 index,
-			   struct netlink_ext_ack *extack)
+static int tcf_simp_search(struct net *net, struct tc_action **a, u32 index)
 {
 	struct tc_action_net *tn = net_generic(net, simp_net_id);
 
diff --git a/net/sched/act_skbedit.c b/net/sched/act_skbedit.c
index 73e44ce2a883..b6263704ea57 100644
--- a/net/sched/act_skbedit.c
+++ b/net/sched/act_skbedit.c
@@ -291,8 +291,7 @@ static int tcf_skbedit_walker(struct net *net, struct sk_buff *skb,
 	return tcf_generic_walker(tn, skb, cb, type, ops, extack);
 }
 
-static int tcf_skbedit_search(struct net *net, struct tc_action **a, u32 index,
-			      struct netlink_ext_ack *extack)
+static int tcf_skbedit_search(struct net *net, struct tc_action **a, u32 index)
 {
 	struct tc_action_net *tn = net_generic(net, skbedit_net_id);
 
diff --git a/net/sched/act_skbmod.c b/net/sched/act_skbmod.c
index 588077fafd6c..59710a183bd3 100644
--- a/net/sched/act_skbmod.c
+++ b/net/sched/act_skbmod.c
@@ -251,8 +251,7 @@ static int tcf_skbmod_walker(struct net *net, struct sk_buff *skb,
 	return tcf_generic_walker(tn, skb, cb, type, ops, extack);
 }
 
-static int tcf_skbmod_search(struct net *net, struct tc_action **a, u32 index,
-			     struct netlink_ext_ack *extack)
+static int tcf_skbmod_search(struct net *net, struct tc_action **a, u32 index)
 {
 	struct tc_action_net *tn = net_generic(net, skbmod_net_id);
 
diff --git a/net/sched/act_tunnel_key.c b/net/sched/act_tunnel_key.c
index 420759153d5f..6d95b6919d9d 100644
--- a/net/sched/act_tunnel_key.c
+++ b/net/sched/act_tunnel_key.c
@@ -540,8 +540,7 @@ static int tunnel_key_walker(struct net *net, struct sk_buff *skb,
 	return tcf_generic_walker(tn, skb, cb, type, ops, extack);
 }
 
-static int tunnel_key_search(struct net *net, struct tc_action **a, u32 index,
-			     struct netlink_ext_ack *extack)
+static int tunnel_key_search(struct net *net, struct tc_action **a, u32 index)
 {
 	struct tc_action_net *tn = net_generic(net, tunnel_key_net_id);
 
diff --git a/net/sched/act_vlan.c b/net/sched/act_vlan.c
index 033d273afe50..ba677d54a7af 100644
--- a/net/sched/act_vlan.c
+++ b/net/sched/act_vlan.c
@@ -288,8 +288,7 @@ static int tcf_vlan_walker(struct net *net, struct sk_buff *skb,
 	return tcf_generic_walker(tn, skb, cb, type, ops, extack);
 }
 
-static int tcf_vlan_search(struct net *net, struct tc_action **a, u32 index,
-			   struct netlink_ext_ack *extack)
+static int tcf_vlan_search(struct net *net, struct tc_action **a, u32 index)
 {
 	struct tc_action_net *tn = net_generic(net, vlan_net_id);
 
-- 
2.14.4

^ permalink raw reply related

* [Patch net-nnext] net_sched: add missing tcf_lock for act_connmark
From: Cong Wang @ 2018-08-29 17:15 UTC (permalink / raw)
  To: netdev; +Cc: Cong Wang, Vlad Buslov
In-Reply-To: <20180829171536.23162-1-xiyou.wangcong@gmail.com>

According to the new locking rule, we have to take tcf_lock
for both ->init() and ->dump(), as RTNL will be removed.
However, it is missing for act_connmark.

Cc: Vlad Buslov <vladbu@mellanox.com>
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
---
 net/sched/act_connmark.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/net/sched/act_connmark.c b/net/sched/act_connmark.c
index e869c0ee63c8..8475913f2070 100644
--- a/net/sched/act_connmark.c
+++ b/net/sched/act_connmark.c
@@ -143,8 +143,10 @@ static int tcf_connmark_init(struct net *net, struct nlattr *nla,
 			return -EEXIST;
 		}
 		/* replacing action and zone */
+		spin_lock_bh(&ci->tcf_lock);
 		ci->tcf_action = parm->action;
 		ci->zone = parm->zone;
+		spin_unlock_bh(&ci->tcf_lock);
 		ret = 0;
 	}
 
@@ -156,16 +158,16 @@ static inline int tcf_connmark_dump(struct sk_buff *skb, struct tc_action *a,
 {
 	unsigned char *b = skb_tail_pointer(skb);
 	struct tcf_connmark_info *ci = to_connmark(a);
-
 	struct tc_connmark opt = {
 		.index   = ci->tcf_index,
 		.refcnt  = refcount_read(&ci->tcf_refcnt) - ref,
 		.bindcnt = atomic_read(&ci->tcf_bindcnt) - bind,
-		.action  = ci->tcf_action,
-		.zone   = ci->zone,
 	};
 	struct tcf_t t;
 
+	spin_lock_bh(&ci->tcf_lock);
+	opt.action = ci->tcf_action;
+	opt.zone = ci->zone;
 	if (nla_put(skb, TCA_CONNMARK_PARMS, sizeof(opt), &opt))
 		goto nla_put_failure;
 
@@ -173,9 +175,12 @@ static inline int tcf_connmark_dump(struct sk_buff *skb, struct tc_action *a,
 	if (nla_put_64bit(skb, TCA_CONNMARK_TM, sizeof(t), &t,
 			  TCA_CONNMARK_PAD))
 		goto nla_put_failure;
+	spin_unlock_bh(&ci->tcf_lock);
 
 	return skb->len;
+
 nla_put_failure:
+	spin_unlock_bh(&ci->tcf_lock);
 	nlmsg_trim(skb, b);
 	return -1;
 }
-- 
2.14.4

^ permalink raw reply related

* Re: [PATCH 2/4] r8169: Get and enable optional ether_clk clock
From: Hans de Goede @ 2018-08-29 17:09 UTC (permalink / raw)
  To: Stephen Boyd, David S . Miller, Andy Shevchenko, Heiner Kallweit,
	Irina Tirdea, Michael Turquette
  Cc: netdev, Johannes Stezenbach, Carlo Caione, linux-clk
In-Reply-To: <153539729389.129321.804899531283282568@swboyd.mtv.corp.google.com>

Hi,

On 27-08-18 21:14, Stephen Boyd wrote:
> Quoting Hans de Goede (2018-08-27 11:53:19)
>> On 27-08-18 20:47, Stephen Boyd wrote:
>>> How would you know that a clk device driver hasn't probed yet and isn't
>>> the driver that's actually providing the clk to this device on x86
>>> systems? With DT systems we can figure that out by looking at the DT and
>>> seeing if the device driver requesting the clk has the clocks property.
>>> On x86 systems it's all clkdev which doesn't really lend itself to
>>> solving this problem.
>>
>> Right on x86 the assumption is that the clk driver will be builtin and
>> will probe before the consumer. In this case that is true as the
>> pmc-atom-clk driver can only be builtin and its platform device is
>> instantiated from the acpi_lpss code and acpi init happens before
>> the PCI bus is scanned.
> 
> If we can go with this assumption then we can make the optional clk API
> work even on clkdev based systems. Maybe if x86 had some way of
> indicating that all builtin clks are registered?

Unfortunately there is no such thing I'm afraid.

> That might work but
> it's not very clean. Or if we could check to see if we're running on an
> ACPI based system in clkdev we could use that to assume that clk_get()
> will only be called after all providers have registered their lookups.

Yes some check for x86 + ACPI (ARM also uses ACPI, but there we
should no do this AFAICT) is probably best. That or not use the
new optional clk API on x86, but that means that any cross platform
driver cannot use it, which would be a pain.

BTW does your Acked-by indicate you are ok with merging this series
through the netdev tree as I suggested in the cover-letter? If so
can I also add your Acked-by to the 3th patch ?

Regards,

Hans

^ permalink raw reply

* [Patch iproute2 v2] ss: add UNIX_DIAG_VFS and UNIX_DIAG_ICONS for unix sockets
From: Cong Wang @ 2018-08-29 17:09 UTC (permalink / raw)
  To: netdev; +Cc: Cong Wang, Stephen Hemminger

UNIX_DIAG_VFS and UNIX_DIAG_ICONS are never used by ss,
make them available in ss -e output.

Cc: Stephen Hemminger <stephen@networkplumber.org>
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
---
 misc/ss.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/misc/ss.c b/misc/ss.c
index 41e7762b..b2c634c8 100644
--- a/misc/ss.c
+++ b/misc/ss.c
@@ -16,6 +16,7 @@
 #include <sys/ioctl.h>
 #include <sys/socket.h>
 #include <sys/uio.h>
+#include <sys/sysmacros.h>
 #include <netinet/in.h>
 #include <string.h>
 #include <errno.h>
@@ -3604,6 +3605,21 @@ static int unix_show_sock(const struct sockaddr_nl *addr, struct nlmsghdr *nlh,
 			out(" %c-%c",
 			    mask & 1 ? '-' : '<', mask & 2 ? '-' : '>');
 		}
+		if (tb[UNIX_DIAG_VFS]) {
+			struct unix_diag_vfs *uv = RTA_DATA(tb[UNIX_DIAG_VFS]);
+
+			out(" ino:%u dev:%u/%u", uv->udiag_vfs_ino, major(uv->udiag_vfs_dev),
+						 minor(uv->udiag_vfs_dev));
+		}
+		if (tb[UNIX_DIAG_ICONS]) {
+			int len = RTA_PAYLOAD(tb[UNIX_DIAG_ICONS]);
+			__u32 *peers = RTA_DATA(tb[UNIX_DIAG_ICONS]);
+			int i;
+
+			out(" peers:");
+			for (i = 0; i < len / sizeof(__u32); i++)
+				out(" %u", peers[i]);
+		}
 	}
 
 	return 0;
@@ -3641,6 +3657,8 @@ static int unix_show_netlink(struct filter *f)
 	req.r.udiag_show = UDIAG_SHOW_NAME | UDIAG_SHOW_PEER | UDIAG_SHOW_RQLEN;
 	if (show_mem)
 		req.r.udiag_show |= UDIAG_SHOW_MEMINFO;
+	if (show_details)
+		req.r.udiag_show |= UDIAG_SHOW_VFS | UDIAG_SHOW_ICONS;
 
 	return handle_netlink_request(f, &req.nlh, sizeof(req), unix_show_sock);
 }
-- 
2.14.4

^ permalink raw reply related

* Re: [PATCH 0/4] clk-pmc-atom + r8169: Add ether_clk handling to fix suspend issues
From: Hans de Goede @ 2018-08-29 17:06 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: David S . Miller, Heiner Kallweit, Michael Turquette,
	Stephen Boyd, Irina Tirdea, netdev, Johannes Stezenbach,
	Carlo Caione, linux-clk
In-Reply-To: <20180829163141.GB8462@smile.fi.intel.com>

Hi,

On 29-08-18 18:31, Andy Shevchenko wrote:
> On Mon, Aug 27, 2018 at 04:31:56PM +0200, Hans de Goede wrote:
>> Hi All,
>>
>> This series has as goal to revert commit d31fd43c0f9a ("clk: x86: Do not gate
>> clocks enabled by the firmware"), because that commit causes almost all
>> Cherry Trail devices to not use the S0i3 powerstate when suspending, leading
>> to them quickly draining their battery when suspended.
>>
>> This commit was added to fix issues with the r8169 NIC on some Bay Trail
>> devices, where the pmc_plt_clk_4 was used as clk for the r8169 chip.
>>
>> This series fixes this properly, so that we can undo the trouble some
>> commit.
>>
>> The first patch adds an "ether_clk" alias to the clk-pmc-atom driver so
>> that the r8169 can pass "ether_clk" as generic id to clk_get instead of
>> having to add x86 specific code to the r8169 driver.
>>
>> The second patch makes the r8169 driver do a clk_get for "ether_clk"
>> (ignoring -ENOENT errors so this is optional) and if that succeeds then
>> it enables the clock.
>>
>> The third patch effectively revert the troublesome commit.
>>
>> This series has been tested on a HP Stream x360 - 11-p000nd, which uses
>> a Bay Trail SoC with a r8169 ethernet chip and I can confirm that the
>> pmc_plt_clk_4 gets properly enabled, so this series should not cause any
>> regressions wrt devices needing pmc_plt_clk_4 enabled (which is not the
>> case on the Stream x360).
>>
>> To avoid regressions we need to have patches 1 and 2 merged before 3,
>> so it is probably best if this entire series gets merged through a single
>> tree. Given that clk-pmc-atom.c has not seen any changes for over a
>> year I suggest that all 3 patches are merged through the netdev tree,
>> with an ack from the clk maintainers. Assuming that is ok with the clk
>> maintainers of course.
>>
>> Note there is a fourth patch in this series, this patch is necessary to
>> reach S0i3 state on Bay Trail devices wich have a r8169 ethernet chip,
>> since I do not have access to hardware where the r8169 actually needs
>> the pmc_plt_clk_4 I have not been able to test this, hence it is marked
>> as RFC for now.
>>
> 
> What a nice stuff, thanks!
> 
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Thank you (and also thank you for the other reviews)

> Btw, you probably better to refer to
> https://bugzilla.kernel.org/show_bug.cgi?id=196861 which is dedicated for S0ix
> issue.

Ok, I've added a link to that. I've also added:

Reported-by: Johannes Stezenbach <js@sig21.net>

To honor Johannes for figuring out that leaving the clocks enabled
was a problem in the first place.

This will all be included in v2 of the series when I send it out.

> Another thing, clk_prepare_enable() can fail, I dunno what you can do at
> ->resume() stage with it failed.

Right, I know that it can fail, but in practice it never fails unless
things are seriously foo-barred already and there is not much we can
do when it fails, so I decided to just ignore the return code.

Regards,

Hans

^ permalink raw reply

* Re: [pull request][net-next 00/10] Mellanox, mlx5 and devlink updates 2018-07-31
From: Alexander Duyck @ 2018-08-29 17:04 UTC (permalink / raw)
  To: valex
  Cc: Saeed Mahameed, Saeed Mahameed, David Miller, Netdev, Jiri Pirko,
	Jakub Kicinski, Bjorn Helgaas
In-Reply-To: <b8b1444f-ee31-b6c1-a5e2-aa0d2e08d2e5@mellanox.com>

On Wed, Aug 29, 2018 at 8:43 AM Alex Vesker <valex@mellanox.com> wrote:
>
>
> > On Wed, Aug 1, 2018 at 4:13 PM, Saeed Mahameed
> > <saeedm@dev.mellanox.co.il> wrote:
> >> On Wed, Aug 1, 2018 at 3:34 PM, Alexander Duyck
> >> <alexander.duyck@gmail.com> wrote:
> >>> On Wed, Aug 1, 2018 at 2:52 PM, Saeed Mahameed <saeedm@mellanox.com>
> >>> wrote:
> >>>> Hi Dave,
> >>>>
> >>>> This series provides devlink parameters updates to both devlink API
> >>>> and
> >>>> mlx5 driver, it is a 2nd iteration of the dropped patches sent in a
> >>>> previous
> >>>> mlx5 submission "net/mlx5: Support PCIe buffer congestion handling via
> >>>> Devlink" to address review comments [1].
> >>>>
> >>>> Changes from the original series:
> >>>> - According to the discussion outcome, we are keeping the
> >>>> congestion control
> >>>>   setting as mlx5 device specific for the current HW generation.
> >>>> - Changed the congestion_mode and congestion action param type to
> >>>> string
> >>>> - Added patches to fix devlink handling of param type string
> >>>> - Added a patch which adds extack messages support for param set.
> >>>> - At the end of this series, I've added yet another mlx5 devlink
> >>>> related
> >>>>  feature, firmware snapshot support.
> >>>>
> >>>> For more information please see tag log below.
> >>>>
> >>>> Please pull and let me know if there's any problem.
> >>>>
> >>>> [1] https://patchwork.ozlabs.org/patch/945996/
> >>>>
> >>>> Thanks,
> >>>> Saeed.
> >>>>
> >>>> ---
> >>>>
> >>>> The following changes since commit
> >>>> e6476c21447c4b17c47e476aade6facf050f31e8:
> >>>>
> >>>>   net: remove bogus RCU annotations on socket.wq (2018-07-31
> >>>> 12:40:22 -0700)
> >>>>
> >>>> are available in the Git repository at:
> >>>>
> >>>> git://git.kernel.org/pub/scm/linux/kernel/git/saeed/linux.git
> >>>> tags/mlx5-updates-2018-08-01
> >>>>
> >>>> for you to fetch changes up to
> >>>> 2ac6108c65ffcb1e5eab1fba1fd59272604d1c32:
> >>>>
> >>>>   net/mlx5: Use devlink region_snapshot parameter (2018-08-01
> >>>> 14:49:09 -0700)
> >>>>
> >>>> ----------------------------------------------------------------
> >>>> mlx5-updates-2018-08-01
> >>>>
> >>>> This series provides devlink parameters updates to both devlink API
> >>>> and
> >>>> mlx5 driver,
> >>>>
> >>>> 1) Devlink changes: (Moshe Shemesh)
> >>>> The first two patches fix devlink param infrastructure for string type
> >>>> params.
> >>>> The third patch adds a devlink helper function to safely copy
> >>>> string from
> >>>> driver to devlink.
> >>>> The forth patch adds extack support for param set.
> >>>>
> >>>> 2) mlx5 specific congestion parameters: (Eran Ben Elisha)
> >>>> Next three patches add new devlink driver specific params for
> >>>> controlling
> >>>> congestion action and mode, using string type params and extack
> >>>> messages support.
> >>>>
> >>>> This congestion mode enables hw workaround in specific devices
> >>>> which is
> >>>> controlled by devlink driver-specific params. The workaround is device
> >>>> specific for this NIC generation, the next NIC will not need it.
> >>>>
> >>>> Congestion parameters:
> >>>>  - Congestion action
> >>>>             HW W/A mechanism in the PCIe buffer which monitors the
> >>>> amount of
> >>>>             consumed PCIe buffer per host.  This mechanism supports
> >>>> the
> >>>>             following actions in case of threshold overflow:
> >>>>             - Disabled - NOP (Default)
> >>>>             - Drop
> >>>>             - Mark - Mark CE bit in the CQE of received packet
> >>>>     - Congestion mode
> >>>>             - Aggressive - Aggressive static trigger threshold
> >>>> (Default)
> >>>>             - Dynamic - Dynamically change the trigger threshold
> >>>>
> >>>> 3) mlx5 firmware snapshot support via devlink: (Alex Vesker)
> >>>> Last three patches, add the support for capturing region snapshot
> >>>> of the
> >>>> firmware crspace during critical errors, using devlink region_snapshot
> >>>> parameter.
> >>>>
> >>>> -Saeed.
> >>>>
> >>>> ----------------------------------------------------------------
> >>>> Alex Vesker (3):
> >>>>       net/mlx5: Add Vendor Specific Capability access gateway
> >>>>       net/mlx5: Add Crdump FW snapshot support
> >>>>       net/mlx5: Use devlink region_snapshot parameter
> >>>>
> >>>> Eran Ben Elisha (3):
> >>>>       net/mlx5: Move all devlink related functions calls to devlink.c
> >>>>       net/mlx5: Add MPEGC register configuration functionality
> >>>>       net/mlx5: Enable PCIe buffer congestion handling workaround
> >>>> via devlink
> >>>>
> >>>> Moshe Shemesh (4):
> >>>>       devlink: Fix param set handling for string type
> >>>>       devlink: Fix param cmode driverinit for string type
> >>>>       devlink: Add helper function for safely copy string param
> >>>>       devlink: Add extack messages support to param set
> >>>>
> >>>>  drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c  |   3 +-
> >>>>  drivers/net/ethernet/mellanox/mlx4/main.c          |   6 +-
> >>>>  drivers/net/ethernet/mellanox/mlx5/core/Makefile   |   3 +-
> >>>>  drivers/net/ethernet/mellanox/mlx5/core/devlink.c  | 388
> >>>> +++++++++++++++++++++
> >>>>  drivers/net/ethernet/mellanox/mlx5/core/devlink.h  |  13 +
> >>>>  .../net/ethernet/mellanox/mlx5/core/diag/crdump.c  | 223 ++++++++++++
> >>>>  drivers/net/ethernet/mellanox/mlx5/core/health.c   |   3 +
> >>>>  drivers/net/ethernet/mellanox/mlx5/core/lib/mlx5.h |   4 +
> >>>>  .../net/ethernet/mellanox/mlx5/core/lib/pci_vsc.c  | 320
> >>>> +++++++++++++++++
> >>>>  .../net/ethernet/mellanox/mlx5/core/lib/pci_vsc.h  |  56 +++
> >>>>  drivers/net/ethernet/mellanox/mlx5/core/main.c     |  10 +-
> >>>>  include/linux/mlx5/driver.h                        |   5 +
> >>>>  include/net/devlink.h                              |  15 +-
> >>>>  net/core/devlink.c                                 |  44 ++-
> >>>>  14 files changed, 1076 insertions(+), 17 deletions(-)
> >>>>  create mode 100644 drivers/net/ethernet/mellanox/mlx5/core/devlink.c
> >>>>  create mode 100644 drivers/net/ethernet/mellanox/mlx5/core/devlink.h
> >>>>  create mode 100644
> >>>> drivers/net/ethernet/mellanox/mlx5/core/diag/crdump.c
> >>>>  create mode 100644
> >>>> drivers/net/ethernet/mellanox/mlx5/core/lib/pci_vsc.c
> >>>>  create mode 100644
> >>>> drivers/net/ethernet/mellanox/mlx5/core/lib/pci_vsc.h
> >>>
> >>> So after looking over the patch set the one thing I would ask for in
> >>> this is some sort of documentation at a minimum. As a user I don't see
> >>> how you can expect someone to be able to use this when the naming of
> >>> things are pretty cryptic and there is no real explanation anywhere if
> >>> you don't go through and read the patch description itself. When you
> >>> start adding driver specific interfaces, you should at least start
> >>> adding vendor specific documentation.
> >>>
> >>
> >> Sure, sounds like a great idea, something like:
> >> Documentation/networking/mlx5.txt and have a devlink section ?
> >> or have a generic devlink doc and a mlx5 section in it ?
> >
> > Either would work for me.
> >
> For which patches are you missing documentation?

Any interfaces that you are exposing via the devlink interface should
have a document somewhere that explains if, when, and how to use it.
All of these new interfaces you were adding in the patch set has no
explanation of what they are other than what is in the driver code
itself and most users don't want to try and decode the driver itself
to figure out what it is they need to do to use an interface.

> >>> Also I don't see how using a vendor specific configuration space
> >>> section can be done without adding some tie-ins to the PCI core files
> >>> because it should be possible to race with someone poking at the
> >>> register space via something like setpci/lspci. Also one of the things
> >>> that came up was that drivers are not supposed to be banging on the
> >>> PCI configuration space at will, and it seems like this patch set is
> >>> doing exactly that through the VSC block.
> >>>
> >>
> >> this is a whole different feature than the device specific parameters.
> >> The whole vendor specific configuration space access is needed only
> >> for diagnostic/dump
> >> purposes when something really bad happens and the command interface
> >> with FW is down,
> >> and when the FW is un-responsive, we want to dump the crspace into the
> >> already existing devlink
> >> crdump buffer, how do you expect us to read it if we are not allowed
> >> to access it ?
> >>
> >> What do you mean by tie-ins to the PCI core files ? can you please
> >> elaborate ?
> >
> > You have added a vendor specific config section and you are using it
> > to access several of the pieces of metadata. The setup isn't too
> > different than the VPD setup and approach. However I don't see many of
> > the protections that exist for VPD in place for this vendor specific
> > configuration. As such I have concerns. For example what is to keep
> > requests to the various devlink interfaces from racing with each other
> > when they both end up operating through the VCS?
> >
> > - Alex
>
> Hi, I would like to resubmit the devlink region crdump support for mlx5,
> which is part of this patch-set.
>
> AlexD, regarding the protection, various devlink interfaces cannot race
> since
> devlink_mutex is used. The VSC access is also protected, using
> mlx5_vsc_gw_lock/unlock
> only one can acquire the lock.

You are talking about from within the driver right? What about raw PCI
configuration space access? Is there anything to keep setpci from
causing issues when you are using the interface? Really creating a
vendor specific config block in and of itself creates a whole new
mess.

> After explaining this I want to clarify something, the access to VSC is
> not user driven
> it happens automatically by the driver when an error is detected to
> collect a crdump.

You don't prevent a user from being able to poke at the configuration
space via setpci.

^ permalink raw reply

* [PATCH v5 4/4] gpiolib: Implement fast processing path in get/set array
From: Janusz Krzysztofik @ 2018-08-29 20:49 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Jonathan Corbet, Miguel Ojeda Sandonis, Peter Korsgaard,
	Peter Rosin, Ulf Hansson, Andrew Lunn, Florian Fainelli,
	David S. Miller, Dominik Brodowski, Greg Kroah-Hartman,
	Kishon Vijay Abraham I, Lars-Peter Clausen, Michael Hennerich,
	Jonathan Cameron, Hartmut Knaack, Peter Meerwald-Stadler,
	Jiri Slaby, Willy Tarreau, Geert Uytterhoeven
In-Reply-To: <20180829204900.19390-1-jmkrzyszt@gmail.com>

Certain GPIO descriptor arrays returned by gpio_get_array() may contain
information on direct mapping of array members to pins of a single GPIO
chip in hardware order.  In such cases, bitmaps of values can be passed
directly from/to the chip's .get/set_multiple() callbacks without
wasting time on iterations.

Add respective code to gpiod_get/set_array_bitmap_complex() functions.
Pins not applicable for fast path are processed as before, skipping
over the 'fast' ones.

Cc: Jonathan Corbet <corbet@lwn.net>
Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com>
---
 Documentation/driver-api/gpio/board.rst    | 15 ++++++
 Documentation/driver-api/gpio/consumer.rst |  8 +++
 drivers/gpio/gpiolib.c                     | 87 ++++++++++++++++++++++++++++--
 3 files changed, 105 insertions(+), 5 deletions(-)

diff --git a/Documentation/driver-api/gpio/board.rst b/Documentation/driver-api/gpio/board.rst
index 2c112553df84..c66821e033c2 100644
--- a/Documentation/driver-api/gpio/board.rst
+++ b/Documentation/driver-api/gpio/board.rst
@@ -193,3 +193,18 @@ And the table can be added to the board code as follows::
 
 The line will be hogged as soon as the gpiochip is created or - in case the
 chip was created earlier - when the hog table is registered.
+
+Arrays of pins
+--------------
+In addition to requesting pins belonging to a function one by one, a device may
+also request an array of pins assigned to the function.  The way those pins are
+mapped to the device determines if the array qualifies for fast bitmap
+processing.  If yes, a bitmap is passed over get/set array functions directly
+between a caller and a respective .get/set_multiple() callback of a GPIO chip.
+
+In order to qualify for fast bitmap processing, the pin mapping must meet the
+following requirements:
+- it must belong to the same chip as other 'fast' pins of the function,
+- its index within the function must match its hardware number within the chip.
+
+Open drain and open source pins are excluded from fast bitmap output processing.
diff --git a/Documentation/driver-api/gpio/consumer.rst b/Documentation/driver-api/gpio/consumer.rst
index 0afd95a12b10..cf992e5ab976 100644
--- a/Documentation/driver-api/gpio/consumer.rst
+++ b/Documentation/driver-api/gpio/consumer.rst
@@ -388,6 +388,14 @@ array_info should be set to NULL.
 Note that for optimal performance GPIOs belonging to the same chip should be
 contiguous within the array of descriptors.
 
+Still better performance may be achieved if array indexes of the descriptors
+match hardware pin numbers of a single chip.  If an array passed to a get/set
+array function matches the one obtained from gpiod_get_array() and array_info
+associated with the array is also passed, the function may take a fast bitmap
+processing path, passing the value_bitmap argument directly to the respective
+.get/set_multiple() callback of the chip.  That allows for utilization of GPIO
+banks as data I/O ports without much loss of performance.
+
 The return value of gpiod_get_array_value() and its variants is 0 on success
 or negative on error. Note the difference to gpiod_get_value(), which returns
 0 or 1 on success to convey the GPIO value. With the array functions, the GPIO
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 4d26cdbdb7cf..b799a89c4c17 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -2787,7 +2787,36 @@ int gpiod_get_array_value_complex(bool raw, bool can_sleep,
 				  struct gpio_array *array_info,
 				  unsigned long *value_bitmap)
 {
-	int i = 0;
+	int err, i = 0;
+
+	/*
+	 * Validate array_info against desc_array and its size.
+	 * It should immediately follow desc_array if both
+	 * have been obtained from the same gpiod_get_array() call.
+	 */
+	if (array_info && array_info->desc == desc_array &&
+	    array_size <= array_info->size &&
+	    (void *)array_info == desc_array + array_info->size) {
+		if (!can_sleep)
+			WARN_ON(array_info->chip->can_sleep);
+
+		err = gpio_chip_get_multiple(array_info->chip,
+					     array_info->get_mask,
+					     value_bitmap);
+		if (err)
+			return err;
+
+		if (!raw && !bitmap_empty(array_info->invert_mask, array_size))
+			bitmap_xor(value_bitmap, value_bitmap,
+				   array_info->invert_mask, array_size);
+
+		if (bitmap_full(array_info->get_mask, array_size))
+			return 0;
+
+		i = find_first_zero_bit(array_info->get_mask, array_size);
+	} else {
+		array_info = NULL;
+	}
 
 	while (i < array_size) {
 		struct gpio_chip *chip = desc_array[i]->gdev->chip;
@@ -2818,7 +2847,12 @@ int gpiod_get_array_value_complex(bool raw, bool can_sleep,
 			int hwgpio = gpio_chip_hwgpio(desc);
 
 			__set_bit(hwgpio, mask);
-			i++;
+
+			if (array_info)
+				find_next_zero_bit(array_info->get_mask,
+						   array_size, i);
+			else
+				i++;
 		} while ((i < array_size) &&
 			 (desc_array[i]->gdev->chip == chip));
 
@@ -2829,7 +2863,7 @@ int gpiod_get_array_value_complex(bool raw, bool can_sleep,
 			return ret;
 		}
 
-		for (j = first; j < i; j++) {
+		for (j = first; j < i; ) {
 			const struct gpio_desc *desc = desc_array[j];
 			int hwgpio = gpio_chip_hwgpio(desc);
 			int value = test_bit(hwgpio, bits);
@@ -2838,6 +2872,11 @@ int gpiod_get_array_value_complex(bool raw, bool can_sleep,
 				value = !value;
 			__assign_bit(j, value_bitmap, value);
 			trace_gpio_value(desc_to_gpio(desc), 1, value);
+
+			if (array_info)
+				find_next_zero_bit(array_info->get_mask, i, j);
+			else
+				j++;
 		}
 
 		if (mask != fastpath)
@@ -3039,6 +3078,32 @@ int gpiod_set_array_value_complex(bool raw, bool can_sleep,
 {
 	int i = 0;
 
+	/*
+	 * Validate array_info against desc_array and its size.
+	 * It should immediately follow desc_array if both
+	 * have been obtained from the same gpiod_get_array() call.
+	 */
+	if (array_info && array_info->desc == desc_array &&
+	    array_size <= array_info->size &&
+	    (void *)array_info == desc_array + array_info->size) {
+		if (!can_sleep)
+			WARN_ON(array_info->chip->can_sleep);
+
+		if (!raw && !bitmap_empty(array_info->invert_mask, array_size))
+			bitmap_xor(value_bitmap, value_bitmap,
+				   array_info->invert_mask, array_size);
+
+		gpio_chip_set_multiple(array_info->chip, array_info->set_mask,
+				       value_bitmap);
+
+		if (bitmap_full(array_info->set_mask, array_size))
+			return 0;
+
+		i = find_first_zero_bit(array_info->set_mask, array_size);
+	} else {
+		array_info = NULL;
+	}
+
 	while (i < array_size) {
 		struct gpio_chip *chip = desc_array[i]->gdev->chip;
 		unsigned long fastpath[2 * BITS_TO_LONGS(FASTPATH_NGPIO)];
@@ -3066,7 +3131,14 @@ int gpiod_set_array_value_complex(bool raw, bool can_sleep,
 			int hwgpio = gpio_chip_hwgpio(desc);
 			int value = test_bit(i, value_bitmap);
 
-			if (!raw && test_bit(FLAG_ACTIVE_LOW, &desc->flags))
+			/*
+			 * Pins applicable for fast input but not for
+			 * fast output processing may have been already
+			 * inverted inside the fast path, skip them.
+			 */
+			if (!raw && !(array_info &&
+			    test_bit(i, array_info->invert_mask)) &&
+			    test_bit(FLAG_ACTIVE_LOW, &desc->flags))
 				value = !value;
 			trace_gpio_value(desc_to_gpio(desc), 0, value);
 			/*
@@ -3085,7 +3157,12 @@ int gpiod_set_array_value_complex(bool raw, bool can_sleep,
 					__clear_bit(hwgpio, bits);
 				count++;
 			}
-			i++;
+
+			if (array_info)
+				find_next_zero_bit(array_info->set_mask,
+						   array_size, i);
+			else
+				i++;
 		} while ((i < array_size) &&
 			 (desc_array[i]->gdev->chip == chip));
 		/* push collected bits to outputs */
-- 
2.16.4

^ permalink raw reply related

* [PATCH v5 3/4] gpiolib: Pass array info to get/set array functions
From: Janusz Krzysztofik @ 2018-08-29 20:48 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Jonathan Corbet, Miguel Ojeda Sandonis, Peter Korsgaard,
	Peter Rosin, Ulf Hansson, Andrew Lunn, Florian Fainelli,
	David S. Miller, Dominik Brodowski, Greg Kroah-Hartman,
	Kishon Vijay Abraham I, Lars-Peter Clausen, Michael Hennerich,
	Jonathan Cameron, Hartmut Knaack, Peter Meerwald-Stadler,
	Jiri Slaby, Willy Tarreau, Geert Uytterhoeven
In-Reply-To: <20180829204900.19390-1-jmkrzyszt@gmail.com>

In order to make use of array info obtained from gpiod_get_array() and
speed up processing of arrays matching single GPIO chip layout, that
information must be passed to get/set array functions.  Extend the
functions' API with that additional parameter and update all users.
Pass NULL if a user bulids an array itself from single GPIOs.

Cc: Jonathan Corbet <corbet@lwn.net>
Cc: Miguel Ojeda Sandonis <miguel.ojeda.sandonis@gmail.com>
Cc: Peter Korsgaard <peter.korsgaard@barco.com>
Cc: Peter Rosin <peda@axentia.se>
Cc: Andrew Lunn <andrew@lunn.ch>
Cc: Florian Fainelli <f.fainelli@gmail.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Dominik Brodowski <linux@dominikbrodowski.net>
Cc: Kishon Vijay Abraham I <kishon@ti.com>
Cc: Lars-Peter Clausen <lars@metafoo.de>
Cc: Michael Hennerich <Michael.Hennerich@analog.com>
Cc: Jonathan Cameron <jic23@kernel.org>
Cc: Hartmut Knaack <knaack.h@gmx.de>
Cc: Peter Meerwald-Stadler <pmeerw@pmeerw.net>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Jiri Slaby <jslaby@suse.com>
Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com>
Acked-by: Ulf Hansson <ulf.hansson@linaro.org>
---
 Documentation/driver-api/gpio/consumer.rst  | 14 ++++++++++--
 drivers/auxdisplay/hd44780.c                | 12 ++++++----
 drivers/bus/ts-nbus.c                       |  6 +++--
 drivers/gpio/gpio-max3191x.c                |  6 +++--
 drivers/gpio/gpiolib.c                      | 34 ++++++++++++++++++++---------
 drivers/gpio/gpiolib.h                      |  2 ++
 drivers/i2c/muxes/i2c-mux-gpio.c            |  2 +-
 drivers/mmc/core/pwrseq_simple.c            |  2 +-
 drivers/mux/gpio.c                          |  3 ++-
 drivers/net/phy/mdio-mux-gpio.c             |  2 +-
 drivers/pcmcia/soc_common.c                 |  3 ++-
 drivers/phy/motorola/phy-mapphone-mdm6600.c |  4 +++-
 drivers/staging/iio/adc/ad7606.c            |  3 ++-
 drivers/tty/serial/serial_mctrl_gpio.c      |  2 +-
 include/linux/gpio/consumer.h               |  8 +++++++
 15 files changed, 75 insertions(+), 28 deletions(-)

diff --git a/Documentation/driver-api/gpio/consumer.rst b/Documentation/driver-api/gpio/consumer.rst
index 7e0298b9a7b9..0afd95a12b10 100644
--- a/Documentation/driver-api/gpio/consumer.rst
+++ b/Documentation/driver-api/gpio/consumer.rst
@@ -325,28 +325,36 @@ The following functions get or set the values of an array of GPIOs::
 
 	int gpiod_get_array_value(unsigned int array_size,
 				  struct gpio_desc **desc_array,
+				  struct gpio_array *array_info,
 				  unsigned long *value_bitmap);
 	int gpiod_get_raw_array_value(unsigned int array_size,
 				      struct gpio_desc **desc_array,
+				      struct gpio_array *array_info,
 				      unsigned long *value_bitmap);
 	int gpiod_get_array_value_cansleep(unsigned int array_size,
 					   struct gpio_desc **desc_array,
+					   struct gpio_array *array_info,
 					   unsigned long *value_bitmap);
 	int gpiod_get_raw_array_value_cansleep(unsigned int array_size,
 					   struct gpio_desc **desc_array,
+					   struct gpio_array *array_info,
 					   unsigned long *value_bitmap);
 
 	void gpiod_set_array_value(unsigned int array_size,
 				   struct gpio_desc **desc_array,
+				   struct gpio_array *array_info,
 				   unsigned long *value_bitmap)
 	void gpiod_set_raw_array_value(unsigned int array_size,
 				       struct gpio_desc **desc_array,
+				       struct gpio_array *array_info,
 				       unsigned long *value_bitmap)
 	void gpiod_set_array_value_cansleep(unsigned int array_size,
 					    struct gpio_desc **desc_array,
+					    struct gpio_array *array_info,
 					    unsigned long *value_bitmap)
 	void gpiod_set_raw_array_value_cansleep(unsigned int array_size,
 						struct gpio_desc **desc_array,
+						struct gpio_array *array_info,
 						unsigned long *value_bitmap)
 
 The array can be an arbitrary set of GPIOs. The functions will try to access
@@ -358,6 +366,7 @@ accessed sequentially.
 The functions take three arguments:
 	* array_size	- the number of array elements
 	* desc_array	- an array of GPIO descriptors
+	* array_info	- optional information obtained from gpiod_array_get()
 	* value_bitmap	- a bitmap to store the GPIOs' values (get) or
 			  a bitmap of values to assign to the GPIOs (set)
 
@@ -368,12 +377,13 @@ the struct gpio_descs returned by gpiod_get_array()::
 
 	struct gpio_descs *my_gpio_descs = gpiod_get_array(...);
 	gpiod_set_array_value(my_gpio_descs->ndescs, my_gpio_descs->desc,
-			      my_gpio_value_bitmap);
+			      my_gpio_descs->info, my_gpio_value_bitmap);
 
 It is also possible to access a completely arbitrary array of descriptors. The
 descriptors may be obtained using any combination of gpiod_get() and
 gpiod_get_array(). Afterwards the array of descriptors has to be setup
-manually before it can be passed to one of the above functions.
+manually before it can be passed to one of the above functions.  In that case,
+array_info should be set to NULL.
 
 Note that for optimal performance GPIOs belonging to the same chip should be
 contiguous within the array of descriptors.
diff --git a/drivers/auxdisplay/hd44780.c b/drivers/auxdisplay/hd44780.c
index bbbd6a29bf01..ec20c41831b0 100644
--- a/drivers/auxdisplay/hd44780.c
+++ b/drivers/auxdisplay/hd44780.c
@@ -74,7 +74,8 @@ static void hd44780_write_gpio8(struct hd44780 *hd, u8 val, unsigned int rs)
 	}
 
 	/* Present the data to the port */
-	gpiod_set_array_value_cansleep(n, &hd->pins[PIN_DATA0], value_bitmap);
+	gpiod_set_array_value_cansleep(n, &hd->pins[PIN_DATA0], NULL,
+				       value_bitmap);
 
 	hd44780_strobe_gpio(hd);
 }
@@ -97,7 +98,8 @@ static void hd44780_write_gpio4(struct hd44780 *hd, u8 val, unsigned int rs)
 	value_bitmap[0] >>= PIN_DATA4;
 
 	/* Present the data to the port */
-	gpiod_set_array_value_cansleep(n, &hd->pins[PIN_DATA4], value_bitmap);
+	gpiod_set_array_value_cansleep(n, &hd->pins[PIN_DATA4], NULL,
+				       value_bitmap);
 
 	hd44780_strobe_gpio(hd);
 
@@ -106,7 +108,8 @@ static void hd44780_write_gpio4(struct hd44780 *hd, u8 val, unsigned int rs)
 	value_bitmap[0] |= val & ~((1 << PIN_DATA4) - 1);
 
 	/* Present the data to the port */
-	gpiod_set_array_value_cansleep(n, &hd->pins[PIN_DATA4], value_bitmap);
+	gpiod_set_array_value_cansleep(n, &hd->pins[PIN_DATA4], NULL,
+				       value_bitmap);
 
 	hd44780_strobe_gpio(hd);
 }
@@ -169,7 +172,8 @@ static void hd44780_write_cmd_raw_gpio4(struct charlcd *lcd, int cmd)
 	value_bitmap[0] = value_bitmap[0] >> PIN_DATA4;
 
 	/* Present the data to the port */
-	gpiod_set_array_value_cansleep(n, &hd->pins[PIN_DATA4], value_bitmap);
+	gpiod_set_array_value_cansleep(n, &hd->pins[PIN_DATA4], NULL,
+				       value_bitmap);
 
 	hd44780_strobe_gpio(hd);
 }
diff --git a/drivers/bus/ts-nbus.c b/drivers/bus/ts-nbus.c
index ce6c1e89236d..000d756eb42c 100644
--- a/drivers/bus/ts-nbus.c
+++ b/drivers/bus/ts-nbus.c
@@ -112,7 +112,8 @@ static void ts_nbus_reset_bus(struct ts_nbus *ts_nbus)
 {
 	unsigned long value_bitmap[1] = { 0, };
 
-	gpiod_set_array_value_cansleep(8, ts_nbus->data->desc, value_bitmap);
+	gpiod_set_array_value_cansleep(8, ts_nbus->data->desc,
+				       ts_nbus->data->info, value_bitmap);
 	gpiod_set_value_cansleep(ts_nbus->csn, 0);
 	gpiod_set_value_cansleep(ts_nbus->strobe, 0);
 	gpiod_set_value_cansleep(ts_nbus->ale, 0);
@@ -155,7 +156,8 @@ static void ts_nbus_write_byte(struct ts_nbus *ts_nbus, u8 byte)
 	struct gpio_descs *gpios = ts_nbus->data;
 	unsigned long value_bitmap[1] = { byte, };
 
-	gpiod_set_array_value_cansleep(8, gpios->desc, value_bitmap);
+	gpiod_set_array_value_cansleep(8, gpios->desc, gpios->info,
+				       value_bitmap);
 }
 
 /*
diff --git a/drivers/gpio/gpio-max3191x.c b/drivers/gpio/gpio-max3191x.c
index c4ec1c82af27..4b43b5dabfd2 100644
--- a/drivers/gpio/gpio-max3191x.c
+++ b/drivers/gpio/gpio-max3191x.c
@@ -313,6 +313,7 @@ static int max3191x_set_config(struct gpio_chip *gpio, unsigned int offset,
 
 static void gpiod_set_array_single_value_cansleep(unsigned int ndescs,
 						  struct gpio_desc **desc,
+						  struct gpio_array *info,
 						  int value)
 {
 	unsigned long *value_bitmap;
@@ -327,7 +328,7 @@ static void gpiod_set_array_single_value_cansleep(unsigned int ndescs,
 	else
 		bitmap_zero(value_bitmap, ndescs);
 
-	gpiod_set_array_value_cansleep(ndescs, desc, value_bitmap);
+	gpiod_set_array_value_cansleep(ndescs, desc, info, value_bitmap);
 	kfree(value_bitmap);
 }
 
@@ -400,7 +401,8 @@ static int max3191x_probe(struct spi_device *spi)
 	if (max3191x->modesel_pins)
 		gpiod_set_array_single_value_cansleep(
 				 max3191x->modesel_pins->ndescs,
-				 max3191x->modesel_pins->desc, max3191x->mode);
+				 max3191x->modesel_pins->desc,
+				 max3191x->modesel_pins->info, max3191x->mode);
 
 	max3191x->ignore_uv = device_property_read_bool(dev,
 						  "maxim,ignore-undervoltage");
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index c1ed1c759345..4d26cdbdb7cf 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -435,7 +435,7 @@ static long linehandle_ioctl(struct file *filep, unsigned int cmd,
 		int ret = gpiod_get_array_value_complex(false,
 							true,
 							lh->numdescs,
-							lh->descs,
+							lh->descs, NULL,
 							value_bitmap);
 		if (ret)
 			return ret;
@@ -467,7 +467,7 @@ static long linehandle_ioctl(struct file *filep, unsigned int cmd,
 		return gpiod_set_array_value_complex(false,
 					      true,
 					      lh->numdescs,
-					      lh->descs,
+					      lh->descs, NULL,
 					      value_bitmap);
 	}
 	return -EINVAL;
@@ -2784,6 +2784,7 @@ static int gpio_chip_get_multiple(struct gpio_chip *chip,
 int gpiod_get_array_value_complex(bool raw, bool can_sleep,
 				  unsigned int array_size,
 				  struct gpio_desc **desc_array,
+				  struct gpio_array *array_info,
 				  unsigned long *value_bitmap)
 {
 	int i = 0;
@@ -2908,12 +2909,14 @@ EXPORT_SYMBOL_GPL(gpiod_get_value);
  */
 int gpiod_get_raw_array_value(unsigned int array_size,
 			      struct gpio_desc **desc_array,
+			      struct gpio_array *array_info,
 			      unsigned long *value_bitmap)
 {
 	if (!desc_array)
 		return -EINVAL;
 	return gpiod_get_array_value_complex(true, false, array_size,
-					     desc_array, value_bitmap);
+					     desc_array, array_info,
+					     value_bitmap);
 }
 EXPORT_SYMBOL_GPL(gpiod_get_raw_array_value);
 
@@ -2931,12 +2934,14 @@ EXPORT_SYMBOL_GPL(gpiod_get_raw_array_value);
  */
 int gpiod_get_array_value(unsigned int array_size,
 			  struct gpio_desc **desc_array,
+			  struct gpio_array *array_info,
 			  unsigned long *value_bitmap)
 {
 	if (!desc_array)
 		return -EINVAL;
 	return gpiod_get_array_value_complex(false, false, array_size,
-					     desc_array, value_bitmap);
+					     desc_array, array_info,
+					     value_bitmap);
 }
 EXPORT_SYMBOL_GPL(gpiod_get_array_value);
 
@@ -3029,6 +3034,7 @@ static void gpio_chip_set_multiple(struct gpio_chip *chip,
 int gpiod_set_array_value_complex(bool raw, bool can_sleep,
 				   unsigned int array_size,
 				   struct gpio_desc **desc_array,
+				   struct gpio_array *array_info,
 				   unsigned long *value_bitmap)
 {
 	int i = 0;
@@ -3166,12 +3172,13 @@ EXPORT_SYMBOL_GPL(gpiod_set_value);
  */
 int gpiod_set_raw_array_value(unsigned int array_size,
 			 struct gpio_desc **desc_array,
+			 struct gpio_array *array_info,
 			 unsigned long *value_bitmap)
 {
 	if (!desc_array)
 		return -EINVAL;
 	return gpiod_set_array_value_complex(true, false, array_size,
-					desc_array, value_bitmap);
+					desc_array, array_info, value_bitmap);
 }
 EXPORT_SYMBOL_GPL(gpiod_set_raw_array_value);
 
@@ -3189,12 +3196,13 @@ EXPORT_SYMBOL_GPL(gpiod_set_raw_array_value);
  */
 void gpiod_set_array_value(unsigned int array_size,
 			   struct gpio_desc **desc_array,
+			   struct gpio_array *array_info,
 			   unsigned long *value_bitmap)
 {
 	if (!desc_array)
 		return;
 	gpiod_set_array_value_complex(false, false, array_size, desc_array,
-				      value_bitmap);
+				      array_info, value_bitmap);
 }
 EXPORT_SYMBOL_GPL(gpiod_set_array_value);
 
@@ -3426,13 +3434,15 @@ EXPORT_SYMBOL_GPL(gpiod_get_value_cansleep);
  */
 int gpiod_get_raw_array_value_cansleep(unsigned int array_size,
 				       struct gpio_desc **desc_array,
+				       struct gpio_array *array_info,
 				       unsigned long *value_bitmap)
 {
 	might_sleep_if(extra_checks);
 	if (!desc_array)
 		return -EINVAL;
 	return gpiod_get_array_value_complex(true, true, array_size,
-					     desc_array, value_bitmap);
+					     desc_array, array_info,
+					     value_bitmap);
 }
 EXPORT_SYMBOL_GPL(gpiod_get_raw_array_value_cansleep);
 
@@ -3449,13 +3459,15 @@ EXPORT_SYMBOL_GPL(gpiod_get_raw_array_value_cansleep);
  */
 int gpiod_get_array_value_cansleep(unsigned int array_size,
 				   struct gpio_desc **desc_array,
+				   struct gpio_array *array_info,
 				   unsigned long *value_bitmap)
 {
 	might_sleep_if(extra_checks);
 	if (!desc_array)
 		return -EINVAL;
 	return gpiod_get_array_value_complex(false, true, array_size,
-					     desc_array, value_bitmap);
+					     desc_array, array_info,
+					     value_bitmap);
 }
 EXPORT_SYMBOL_GPL(gpiod_get_array_value_cansleep);
 
@@ -3508,13 +3520,14 @@ EXPORT_SYMBOL_GPL(gpiod_set_value_cansleep);
  */
 int gpiod_set_raw_array_value_cansleep(unsigned int array_size,
 					struct gpio_desc **desc_array,
+					struct gpio_array *array_info,
 					unsigned long *value_bitmap)
 {
 	might_sleep_if(extra_checks);
 	if (!desc_array)
 		return -EINVAL;
 	return gpiod_set_array_value_complex(true, true, array_size, desc_array,
-				      value_bitmap);
+				      array_info, value_bitmap);
 }
 EXPORT_SYMBOL_GPL(gpiod_set_raw_array_value_cansleep);
 
@@ -3548,13 +3561,14 @@ void gpiod_add_lookup_tables(struct gpiod_lookup_table **tables, size_t n)
  */
 void gpiod_set_array_value_cansleep(unsigned int array_size,
 				    struct gpio_desc **desc_array,
+				    struct gpio_array *array_info,
 				    unsigned long *value_bitmap)
 {
 	might_sleep_if(extra_checks);
 	if (!desc_array)
 		return;
 	gpiod_set_array_value_complex(false, true, array_size, desc_array,
-				      value_bitmap);
+				      array_info, value_bitmap);
 }
 EXPORT_SYMBOL_GPL(gpiod_set_array_value_cansleep);
 
diff --git a/drivers/gpio/gpiolib.h b/drivers/gpio/gpiolib.h
index b60905d558b1..b65ca896b24d 100644
--- a/drivers/gpio/gpiolib.h
+++ b/drivers/gpio/gpiolib.h
@@ -196,10 +196,12 @@ struct gpio_desc *gpiochip_get_desc(struct gpio_chip *chip, u16 hwnum);
 int gpiod_get_array_value_complex(bool raw, bool can_sleep,
 				  unsigned int array_size,
 				  struct gpio_desc **desc_array,
+				  struct gpio_array *array_info,
 				  unsigned long *value_bitmap);
 int gpiod_set_array_value_complex(bool raw, bool can_sleep,
 				   unsigned int array_size,
 				   struct gpio_desc **desc_array,
+				   struct gpio_array *array_info,
 				   unsigned long *value_bitmap);
 
 /* This is just passed between gpiolib and devres */
diff --git a/drivers/i2c/muxes/i2c-mux-gpio.c b/drivers/i2c/muxes/i2c-mux-gpio.c
index 4e36e0eac7a3..4439a92c86a2 100644
--- a/drivers/i2c/muxes/i2c-mux-gpio.c
+++ b/drivers/i2c/muxes/i2c-mux-gpio.c
@@ -31,7 +31,7 @@ static void i2c_mux_gpio_set(const struct gpiomux *mux, unsigned val)
 	int i;
 
 	gpiod_set_array_value_cansleep(mux->data.n_gpios,
-				       mux->gpios, value_bitmap);
+				       mux->gpios, NULL, value_bitmap);
 }
 
 static int i2c_mux_gpio_select(struct i2c_mux_core *muxc, u32 chan)
diff --git a/drivers/mmc/core/pwrseq_simple.c b/drivers/mmc/core/pwrseq_simple.c
index 0d6e3a5be3ba..5cf7eda8f68f 100644
--- a/drivers/mmc/core/pwrseq_simple.c
+++ b/drivers/mmc/core/pwrseq_simple.c
@@ -46,7 +46,7 @@ static void mmc_pwrseq_simple_set_gpios_value(struct mmc_pwrseq_simple *pwrseq,
 		value_bitmap[0] = value;
 
 		gpiod_set_array_value_cansleep(nvalues, reset_gpios->desc,
-					       value_bitmap);
+					       reset_gpios->info, value_bitmap);
 	}
 }
 
diff --git a/drivers/mux/gpio.c b/drivers/mux/gpio.c
index 734e1b43aed6..be8c86680e10 100644
--- a/drivers/mux/gpio.c
+++ b/drivers/mux/gpio.c
@@ -27,7 +27,8 @@ static int mux_gpio_set(struct mux_control *mux, int state)
 	int i;
 
 	gpiod_set_array_value_cansleep(mux_gpio->gpios->ndescs,
-				       mux_gpio->gpios->desc, value_bitmap);
+				       mux_gpio->gpios->desc,
+				       mux_gpio->gpios->info, value_bitmap);
 
 	return 0;
 }
diff --git a/drivers/net/phy/mdio-mux-gpio.c b/drivers/net/phy/mdio-mux-gpio.c
index 8e1ec750277e..c0ffa03c916b 100644
--- a/drivers/net/phy/mdio-mux-gpio.c
+++ b/drivers/net/phy/mdio-mux-gpio.c
@@ -37,7 +37,7 @@ static int mdio_mux_gpio_switch_fn(int current_child, int desired_child,
 		s->values[n] = (desired_child >> n) & 1;
 
 	gpiod_set_array_value_cansleep(s->gpios->ndescs, s->gpios->desc,
-				       value_bitmap);
+				       s->gpios->info, value_bitmap);
 
 	return 0;
 }
diff --git a/drivers/pcmcia/soc_common.c b/drivers/pcmcia/soc_common.c
index e0f89155c474..55978198cd2b 100644
--- a/drivers/pcmcia/soc_common.c
+++ b/drivers/pcmcia/soc_common.c
@@ -366,7 +366,8 @@ static int soc_common_pcmcia_config_skt(
 		}
 
 		if (n)
-			gpiod_set_array_value_cansleep(n, descs, value_bitmap);
+			gpiod_set_array_value_cansleep(n, descs, NULL,
+						       value_bitmap);
 
 		/*
 		 * This really needs a better solution.  The IRQ
diff --git a/drivers/phy/motorola/phy-mapphone-mdm6600.c b/drivers/phy/motorola/phy-mapphone-mdm6600.c
index b6477c3599c4..8f508338ec56 100644
--- a/drivers/phy/motorola/phy-mapphone-mdm6600.c
+++ b/drivers/phy/motorola/phy-mapphone-mdm6600.c
@@ -162,7 +162,8 @@ static void phy_mdm6600_cmd(struct phy_mdm6600 *ddata, int val)
 	value_bitmap[0] = val & ((1 << PHY_MDM6600_NR_CMD_LINES) - 1);
 
 	gpiod_set_array_value_cansleep(PHY_MDM6600_NR_CMD_LINES,
-				       ddata->cmd_gpios->desc, value_bitmap);
+				       ddata->cmd_gpios->desc,
+				       ddata->cmd_gpios->info, value_bitmap);
 }
 
 /**
@@ -181,6 +182,7 @@ static void phy_mdm6600_status(struct work_struct *work)
 
 	error = gpiod_get_array_value_cansleep(PHY_MDM6600_NR_STATUS_LINES,
 					       ddata->status_gpios->desc,
+					       ddata->status_gpios->info,
 					       value_bitmap);
 	if (error)
 		return;
diff --git a/drivers/staging/iio/adc/ad7606.c b/drivers/staging/iio/adc/ad7606.c
index 0eca047bc1cc..eb779d825724 100644
--- a/drivers/staging/iio/adc/ad7606.c
+++ b/drivers/staging/iio/adc/ad7606.c
@@ -230,7 +230,8 @@ static int ad7606_write_raw(struct iio_dev *indio_dev,
 		value_bitmap[0] = ret;
 
 		mutex_lock(&st->lock);
-		gpiod_set_array_value(3, st->gpio_os->desc, value_bitmap);
+		gpiod_set_array_value(3, st->gpio_os->desc, st->gpio_os->info,
+				      value_bitmap);
 		st->oversampling = val;
 		mutex_unlock(&st->lock);
 
diff --git a/drivers/tty/serial/serial_mctrl_gpio.c b/drivers/tty/serial/serial_mctrl_gpio.c
index bb8b4756d72d..8a04e3be5419 100644
--- a/drivers/tty/serial/serial_mctrl_gpio.c
+++ b/drivers/tty/serial/serial_mctrl_gpio.c
@@ -53,7 +53,7 @@ void mctrl_gpio_set(struct mctrl_gpios *gpios, unsigned int mctrl)
 				     !!(mctrl & mctrl_gpios_desc[i].mctrl));
 			count++;
 		}
-	gpiod_set_array_value(count, desc_array, value_bitmap);
+	gpiod_set_array_value(count, desc_array, NULL, value_bitmap);
 }
 EXPORT_SYMBOL_GPL(mctrl_gpio_set);
 
diff --git a/include/linux/gpio/consumer.h b/include/linux/gpio/consumer.h
index 8dede3e886af..bf037ebe2ed8 100644
--- a/include/linux/gpio/consumer.h
+++ b/include/linux/gpio/consumer.h
@@ -114,36 +114,44 @@ int gpiod_direction_output_raw(struct gpio_desc *desc, int value);
 int gpiod_get_value(const struct gpio_desc *desc);
 int gpiod_get_array_value(unsigned int array_size,
 			  struct gpio_desc **desc_array,
+			  struct gpio_array *array_info,
 			  unsigned long *value_bitmap);
 void gpiod_set_value(struct gpio_desc *desc, int value);
 void gpiod_set_array_value(unsigned int array_size,
 			   struct gpio_desc **desc_array,
+			   struct gpio_array *array_info,
 			   unsigned long *value_bitmap);
 int gpiod_get_raw_value(const struct gpio_desc *desc);
 int gpiod_get_raw_array_value(unsigned int array_size,
 			      struct gpio_desc **desc_array,
+			      struct gpio_array *array_info,
 			      unsigned long *value_bitmap);
 void gpiod_set_raw_value(struct gpio_desc *desc, int value);
 int gpiod_set_raw_array_value(unsigned int array_size,
 			       struct gpio_desc **desc_array,
+			       struct gpio_array *array_info,
 			       unsigned long *value_bitmap);
 
 /* Value get/set from sleeping context */
 int gpiod_get_value_cansleep(const struct gpio_desc *desc);
 int gpiod_get_array_value_cansleep(unsigned int array_size,
 				   struct gpio_desc **desc_array,
+				   struct gpio_array *array_info,
 				   unsigned long *value_bitmap);
 void gpiod_set_value_cansleep(struct gpio_desc *desc, int value);
 void gpiod_set_array_value_cansleep(unsigned int array_size,
 				    struct gpio_desc **desc_array,
+				    struct gpio_array *array_info,
 				    unsigned long *value_bitmap);
 int gpiod_get_raw_value_cansleep(const struct gpio_desc *desc);
 int gpiod_get_raw_array_value_cansleep(unsigned int array_size,
 				       struct gpio_desc **desc_array,
+				       struct gpio_array *array_info,
 				       unsigned long *value_bitmap);
 void gpiod_set_raw_value_cansleep(struct gpio_desc *desc, int value);
 int gpiod_set_raw_array_value_cansleep(unsigned int array_size,
 					struct gpio_desc **desc_array,
+					struct gpio_array *array_info,
 					unsigned long *value_bitmap);
 
 int gpiod_set_debounce(struct gpio_desc *desc, unsigned debounce);
-- 
2.16.4

^ permalink raw reply related

* [PATCH v5 2/4] gpiolib: Identify arrays matching GPIO hardware
From: Janusz Krzysztofik @ 2018-08-29 20:48 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Andrew Lunn, Ulf Hansson, linux-doc, linux-iio, Dominik Brodowski,
	Peter Rosin, netdev, linux-i2c, Peter Meerwald-Stadler, devel,
	Florian Fainelli, Jonathan Corbet, Janusz Krzysztofik,
	Kishon Vijay Abraham I, Geert Uytterhoeven, linux-serial,
	Jiri Slaby, Michael Hennerich, linux-gpio, Lars-Peter Clausen,
	Greg Kroah-Hartman, linux-mmc, linux-kernel, Willy Tarreau,
	Miguel Ojeda Sandonis
In-Reply-To: <20180829204900.19390-1-jmkrzyszt@gmail.com>

Certain GPIO array lookup results may map directly to GPIO pins of a
single GPIO chip in hardware order.  If that condition is recognized
and handled efficiently, significant performance gain of get/set array
functions may be possible.

While processing a request for an array of GPIO descriptors, identify
those which represent corresponding pins of a single GPIO chip.  Skip
over pins which require open source or open drain special processing.
Moreover, identify pins which require inversion.  Pass a pointer to
that information with the array to the caller so it can benefit from
enhanced performance as soon as get/set array functions can accept and
make efficient use of it.

Cc: Jonathan Corbet <corbet@lwn.net>
Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com>
---
 Documentation/driver-api/gpio/consumer.rst |  4 +-
 drivers/gpio/gpiolib.c                     | 72 +++++++++++++++++++++++++++++-
 drivers/gpio/gpiolib.h                     |  9 ++++
 include/linux/gpio/consumer.h              |  9 ++++
 4 files changed, 92 insertions(+), 2 deletions(-)

diff --git a/Documentation/driver-api/gpio/consumer.rst b/Documentation/driver-api/gpio/consumer.rst
index ed68042ddccf..7e0298b9a7b9 100644
--- a/Documentation/driver-api/gpio/consumer.rst
+++ b/Documentation/driver-api/gpio/consumer.rst
@@ -109,9 +109,11 @@ For a function using multiple GPIOs all of those can be obtained with one call::
 					   enum gpiod_flags flags)
 
 This function returns a struct gpio_descs which contains an array of
-descriptors::
+descriptors.  It also contains a pointer to a gpiolib private structure which,
+if passed back to get/set array functions, may speed up I/O proocessing::
 
 	struct gpio_descs {
+		struct gpio_array *info;
 		unsigned int ndescs;
 		struct gpio_desc *desc[];
 	}
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index f0e9ffa8cab6..c1ed1c759345 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -4174,7 +4174,9 @@ struct gpio_descs *__must_check gpiod_get_array(struct device *dev,
 {
 	struct gpio_desc *desc;
 	struct gpio_descs *descs;
-	int count;
+	struct gpio_array *array_info = NULL;
+	struct gpio_chip *chip;
+	int count, bitmap_size;
 
 	count = gpiod_count(dev, con_id);
 	if (count < 0)
@@ -4190,9 +4192,77 @@ struct gpio_descs *__must_check gpiod_get_array(struct device *dev,
 			gpiod_put_array(descs);
 			return ERR_CAST(desc);
 		}
+
 		descs->desc[descs->ndescs] = desc;
+
+		chip = gpiod_to_chip(desc);
+		/*
+		 * Select a chip of first array member
+		 * whose index matches its pin hardware number
+		 * as a candidate for fast bitmap processing.
+		 */
+		if (!array_info && gpio_chip_hwgpio(desc) == descs->ndescs) {
+			struct gpio_descs *array;
+
+			bitmap_size = BITS_TO_LONGS(chip->ngpio > count ?
+						    chip->ngpio : count);
+
+			array = kzalloc(struct_size(descs, desc, count) +
+					struct_size(array_info, invert_mask,
+					3 * bitmap_size), GFP_KERNEL);
+			if (!array) {
+				gpiod_put_array(descs);
+				return ERR_PTR(-ENOMEM);
+			}
+
+			memcpy(array, descs,
+			       struct_size(descs, desc, descs->ndescs + 1));
+			kfree(descs);
+
+			descs = array;
+			array_info = (void *)(descs->desc + count);
+			array_info->get_mask = array_info->invert_mask +
+						  bitmap_size;
+			array_info->set_mask = array_info->get_mask +
+						  bitmap_size;
+
+			array_info->desc = descs->desc;
+			array_info->size = count;
+			array_info->chip = chip;
+			bitmap_set(array_info->get_mask, descs->ndescs,
+				   count - descs->ndescs);
+			bitmap_set(array_info->set_mask, descs->ndescs,
+				   count - descs->ndescs);
+			descs->info = array_info;
+		}
+		/*
+		 * Unmark members which don't qualify for fast bitmap
+		 * processing (different chip, not in hardware order)
+		 */
+		if (array_info && (chip != array_info->chip ||
+		    gpio_chip_hwgpio(desc) != descs->ndescs)) {
+			__clear_bit(descs->ndescs, array_info->get_mask);
+			__clear_bit(descs->ndescs, array_info->set_mask);
+		} else if (array_info) {
+			/* Exclude open drain or open source from fast output */
+			if (gpiochip_line_is_open_drain(chip, descs->ndescs) ||
+			    gpiochip_line_is_open_source(chip, descs->ndescs))
+				__clear_bit(descs->ndescs,
+					    array_info->set_mask);
+			/* Identify 'fast' pins which require invertion */
+			if (gpiod_is_active_low(desc))
+				__set_bit(descs->ndescs,
+					  array_info->invert_mask);
+		}
+
 		descs->ndescs++;
 	}
+	if (array_info)
+		dev_dbg(dev,
+			"GPIO array info: chip=%s, size=%d, get_mask=%lx, set_mask=%lx, invert_mask=%lx\n",
+			array_info->chip->label, array_info->size,
+			*array_info->get_mask, *array_info->set_mask,
+			*array_info->invert_mask);
 	return descs;
 }
 EXPORT_SYMBOL_GPL(gpiod_get_array);
diff --git a/drivers/gpio/gpiolib.h b/drivers/gpio/gpiolib.h
index 11e83d2eef89..b60905d558b1 100644
--- a/drivers/gpio/gpiolib.h
+++ b/drivers/gpio/gpiolib.h
@@ -183,6 +183,15 @@ static inline bool acpi_can_fallback_to_crs(struct acpi_device *adev,
 }
 #endif
 
+struct gpio_array {
+	struct gpio_desc	**desc;
+	unsigned int		size;
+	struct gpio_chip	*chip;
+	unsigned long		*get_mask;
+	unsigned long		*set_mask;
+	unsigned long		invert_mask[];
+};
+
 struct gpio_desc *gpiochip_get_desc(struct gpio_chip *chip, u16 hwnum);
 int gpiod_get_array_value_complex(bool raw, bool can_sleep,
 				  unsigned int array_size,
diff --git a/include/linux/gpio/consumer.h b/include/linux/gpio/consumer.h
index 1b21dc7b0fad..8dede3e886af 100644
--- a/include/linux/gpio/consumer.h
+++ b/include/linux/gpio/consumer.h
@@ -17,11 +17,20 @@ struct device;
  */
 struct gpio_desc;
 
+/**
+ * Opaque descriptor for a structure of GPIO array attributes.  This structure
+ * is attached to struct gpiod_descs obtained from gpiod_get_array() and can be
+ * passed back to get/set array functions in order to activate fast processing
+ * path if applicable.
+ */
+struct gpio_array;
+
 /**
  * Struct containing an array of descriptors that can be obtained using
  * gpiod_get_array().
  */
 struct gpio_descs {
+	struct gpio_array *info;
 	unsigned int ndescs;
 	struct gpio_desc *desc[];
 };
-- 
2.16.4

^ permalink raw reply related

* [PATH v5 0/4] gpiolib: speed up GPIO array processing
From: Janusz Krzysztofik @ 2018-08-29 20:48 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Andrew Lunn, Ulf Hansson, linux-doc, linux-iio, Dominik Brodowski,
	Peter Rosin, netdev, linux-i2c, Peter Meerwald-Stadler, devel,
	Florian Fainelli, Jonathan Corbet, Kishon Vijay Abraham I,
	Geert Uytterhoeven, linux-serial, Jiri Slaby, Michael Hennerich,
	linux-gpio, Lars-Peter Clausen, Greg Kroah-Hartman, linux-mmc,
	linux-kernel, Willy Tarreau, Miguel Ojeda Sandonis,
	Peter Korsgaard
In-Reply-To: <20180820234341.5271-1-jmkrzyszt@gmail.com>


The goal is to boost performance of get/set array functions while
processing GPIO arrays which represent pins of a signle chip in
hardware order.  If resulting performance is close to PIO, GPIO API
can be used for data I/O without much loss of speed.

Created and tested on a low end Amstrad Delta board with NAND driver
updated to use GPIO API for data I/O.  Performance degrade compared to
PIO is much better than before the optimization but still not quite
satisfactory.

Janusz Krzysztofik (4):
      gpiolib: Pass bitmaps, not integer arrays, to get/set array
      gpiolib: Identify arrays matching GPIO hardware
      gpiolib: Pass array info to get/set array functions
      gpiolib: Implement fast processing path in get/set array

Changelog:
v5:
[PATCH v5 1/4] gpiolib: Pass bitmaps, not integer arrays, to get/set
- drivers/i2c/muxes/i2c-mux-gpio.c:
  - drop assigment of values to struct gpiomux.values, as recommended
    by Peter Rosin - thanks!,
  - mark the .values member of the structure as obsolete,
- drivers/mux/gpio.c:
  - drop assigment of values to struct mux_gpio.val, also recommended
    by Peter Rosin - thanks!,
  - merk the .val member of the structure as obsolete,
- drivers/auxdisplay/hd44780.c:
  - fix incorrect bitmap size,
  - use >>= operator to simplify notation,
  both catched by Miguel Ojeda - thanks!,
- add Cc: clauses as well as Acked-by: collected so far.
[PATCH v5 2/4] gpiolib: Identify arrays matching GPIO hardware
- add Cc: clause.
[PATCH v5 3/4] gpiolib: Pass array info to get/set array functions
- add Cc: clauses as well as Acked-by: collected so far.
[PATCH v5 4/4] gpiolib: Implement fast processing path in get/set
- add Cc: clause.

v4:
That series was a follow up of the former "mtd: rawnand: ams-delta: Use
gpio-omap accessors for data I/O" which already contained some changes
to gpiolib.  Those previous attempts were commented by Borris Brezillon
who suggested using GPIO API modified to accept bitmaps, and by Linus
Walleij who suggested still more great ideas for further immprovement
of the proposed API changes - thanks!

diffstat:
 Documentation/driver-api/gpio/board.rst     |   15 +
 Documentation/driver-api/gpio/consumer.rst  |   48 +++-
 drivers/auxdisplay/hd44780.c                |   64 +++---
 drivers/bus/ts-nbus.c                       |   25 --
 drivers/gpio/gpio-max3191x.c                |   23 +-
 drivers/gpio/gpiolib.c                      |  279 ++++++++++++++++++++++------
 drivers/gpio/gpiolib.h                      |   15 +
 drivers/i2c/muxes/i2c-mux-gpio.c            |   10 -
 drivers/mmc/core/pwrseq_simple.c            |   15 -
 drivers/mux/gpio.c                          |   12 -
 drivers/net/phy/mdio-mux-gpio.c             |    5 
 drivers/pcmcia/soc_common.c                 |   14 -
 drivers/phy/motorola/phy-mapphone-mdm6600.c |   21 +-
 drivers/staging/iio/adc/ad7606.c            |   12 -
 drivers/tty/serial/serial_mctrl_gpio.c      |    9 
 include/linux/gpio/consumer.h               |   35 ++-
 16 files changed, 412 insertions(+), 190 deletions(-)

^ permalink raw reply

* Re: [PATCH 0/4] clk-pmc-atom + r8169: Add ether_clk handling to fix suspend issues
From: Andy Shevchenko @ 2018-08-29 16:31 UTC (permalink / raw)
  To: Hans de Goede
  Cc: David S . Miller, Heiner Kallweit, Michael Turquette,
	Stephen Boyd, Irina Tirdea, netdev, Johannes Stezenbach,
	Carlo Caione, linux-clk
In-Reply-To: <20180827143200.8597-1-hdegoede@redhat.com>

On Mon, Aug 27, 2018 at 04:31:56PM +0200, Hans de Goede wrote:
> Hi All,
> 
> This series has as goal to revert commit d31fd43c0f9a ("clk: x86: Do not gate
> clocks enabled by the firmware"), because that commit causes almost all
> Cherry Trail devices to not use the S0i3 powerstate when suspending, leading
> to them quickly draining their battery when suspended.
> 
> This commit was added to fix issues with the r8169 NIC on some Bay Trail
> devices, where the pmc_plt_clk_4 was used as clk for the r8169 chip.
> 
> This series fixes this properly, so that we can undo the trouble some
> commit.
> 
> The first patch adds an "ether_clk" alias to the clk-pmc-atom driver so
> that the r8169 can pass "ether_clk" as generic id to clk_get instead of
> having to add x86 specific code to the r8169 driver.
> 
> The second patch makes the r8169 driver do a clk_get for "ether_clk"
> (ignoring -ENOENT errors so this is optional) and if that succeeds then
> it enables the clock.
> 
> The third patch effectively revert the troublesome commit.
> 
> This series has been tested on a HP Stream x360 - 11-p000nd, which uses
> a Bay Trail SoC with a r8169 ethernet chip and I can confirm that the
> pmc_plt_clk_4 gets properly enabled, so this series should not cause any
> regressions wrt devices needing pmc_plt_clk_4 enabled (which is not the
> case on the Stream x360).
> 
> To avoid regressions we need to have patches 1 and 2 merged before 3,
> so it is probably best if this entire series gets merged through a single
> tree. Given that clk-pmc-atom.c has not seen any changes for over a
> year I suggest that all 3 patches are merged through the netdev tree,
> with an ack from the clk maintainers. Assuming that is ok with the clk
> maintainers of course.
> 
> Note there is a fourth patch in this series, this patch is necessary to
> reach S0i3 state on Bay Trail devices wich have a r8169 ethernet chip,
> since I do not have access to hardware where the r8169 actually needs
> the pmc_plt_clk_4 I have not been able to test this, hence it is marked
> as RFC for now.
> 

What a nice stuff, thanks!

Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Btw, you probably better to refer to
https://bugzilla.kernel.org/show_bug.cgi?id=196861 which is dedicated for S0ix
issue.

Another thing, clk_prepare_enable() can fail, I dunno what you can do at
->resume() stage with it failed.

> Carlos can you test the 4th patch (when you have time) and let us know if
> it works?
> 
> Regards,
> 
> Hans
> 

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply

* [PATCH net-next 2/2] hv_netvsc: pair VF based on serial number
From: Stephen Hemminger @ 2018-08-29 16:24 UTC (permalink / raw)
  To: kys, haiyangz, sthemmin; +Cc: devel, netdev, linux-pci
In-Reply-To: <20180829162452.25805-1-sthemmin@microsoft.com>

Matching network device based on MAC address is problematic
since a non-VF network device can be created with a duplicate MAC
address causing confusion and problems.  The VMBus API provides
a serial number that is a better matching method.

Signed-off-by: Stephen Hemminger <sthemmin@microsoft.com>
---
 drivers/net/hyperv/netvsc.c     |  3 ++
 drivers/net/hyperv/netvsc_drv.c | 58 +++++++++++++++++++--------------
 2 files changed, 36 insertions(+), 25 deletions(-)

diff --git a/drivers/net/hyperv/netvsc.c b/drivers/net/hyperv/netvsc.c
index 31c3d77b4733..fe01e141c8f8 100644
--- a/drivers/net/hyperv/netvsc.c
+++ b/drivers/net/hyperv/netvsc.c
@@ -1203,6 +1203,9 @@ static void netvsc_send_vf(struct net_device *ndev,
 
 	net_device_ctx->vf_alloc = nvmsg->msg.v4_msg.vf_assoc.allocated;
 	net_device_ctx->vf_serial = nvmsg->msg.v4_msg.vf_assoc.serial;
+	netdev_info(ndev, "VF slot %u %s\n",
+		    net_device_ctx->vf_serial,
+		    net_device_ctx->vf_alloc ? "added" : "removed");
 }
 
 static  void netvsc_receive_inband(struct net_device *ndev,
diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
index 1121a1ec407c..9dedc1463e88 100644
--- a/drivers/net/hyperv/netvsc_drv.c
+++ b/drivers/net/hyperv/netvsc_drv.c
@@ -1894,20 +1894,6 @@ static void netvsc_link_change(struct work_struct *w)
 	rtnl_unlock();
 }
 
-static struct net_device *get_netvsc_bymac(const u8 *mac)
-{
-	struct net_device_context *ndev_ctx;
-
-	list_for_each_entry(ndev_ctx, &netvsc_dev_list, list) {
-		struct net_device *dev = hv_get_drvdata(ndev_ctx->device_ctx);
-
-		if (ether_addr_equal(mac, dev->perm_addr))
-			return dev;
-	}
-
-	return NULL;
-}
-
 static struct net_device *get_netvsc_byref(struct net_device *vf_netdev)
 {
 	struct net_device_context *net_device_ctx;
@@ -2036,26 +2022,48 @@ static void netvsc_vf_setup(struct work_struct *w)
 	rtnl_unlock();
 }
 
+/* Find netvsc by VMBus serial number.
+ * The PCI hyperv controller records the serial number as the slot.
+ */
+static struct net_device *get_netvsc_byslot(const struct net_device *vf_netdev)
+{
+	struct device *parent = vf_netdev->dev.parent;
+	struct net_device_context *ndev_ctx;
+	struct pci_dev *pdev;
+
+	if (!parent || !dev_is_pci(parent))
+		return NULL; /* not a PCI device */
+
+	pdev = to_pci_dev(parent);
+	if (!pdev->slot) {
+		netdev_notice(vf_netdev, "no PCI slot information\n");
+		return NULL;
+	}
+
+	list_for_each_entry(ndev_ctx, &netvsc_dev_list, list) {
+		if (!ndev_ctx->vf_alloc)
+			continue;
+
+		if (ndev_ctx->vf_serial == pdev->slot->number)
+			return hv_get_drvdata(ndev_ctx->device_ctx);
+	}
+
+	netdev_notice(vf_netdev,
+		      "no netdev found for slot %u\n", pdev->slot->number);
+	return NULL;
+}
+
 static int netvsc_register_vf(struct net_device *vf_netdev)
 {
-	struct net_device *ndev;
 	struct net_device_context *net_device_ctx;
-	struct device *pdev = vf_netdev->dev.parent;
 	struct netvsc_device *netvsc_dev;
+	struct net_device *ndev;
 	int ret;
 
 	if (vf_netdev->addr_len != ETH_ALEN)
 		return NOTIFY_DONE;
 
-	if (!pdev || !dev_is_pci(pdev) || dev_is_pf(pdev))
-		return NOTIFY_DONE;
-
-	/*
-	 * We will use the MAC address to locate the synthetic interface to
-	 * associate with the VF interface. If we don't find a matching
-	 * synthetic interface, move on.
-	 */
-	ndev = get_netvsc_bymac(vf_netdev->perm_addr);
+	ndev = get_netvsc_byslot(vf_netdev);
 	if (!ndev)
 		return NOTIFY_DONE;
 
-- 
2.18.0

^ permalink raw reply related

* [PATCH net-next 1/2] PCI: hv: support reporting serial number as slot information
From: Stephen Hemminger @ 2018-08-29 16:24 UTC (permalink / raw)
  To: kys, haiyangz, sthemmin; +Cc: devel, netdev, linux-pci
In-Reply-To: <20180829162452.25805-1-sthemmin@microsoft.com>

The Hyper-V host API for PCI provides a unique "serial number" which
can be used as basis for sysfs PCI slot table. This can be useful
for cases where userspace wants to find the PCI device based on
serial number.

When an SR-IOV NIC is added, the host sends an attach message
with serial number. The kernel doesn't use the serial number, but
it is useful when doing the same thing in a userspace driver such
as the DPDK. By having /sys/bus/pci/slots/N it provides a direct
way to find the matching PCI device.

There may be some cases where serial number is not unique such
as when using GPU's. But the PCI slot infrastructure will handle
that by adding suffix "2-1" etc.

This has a side effect which may also be useful. The common udev
network device naming policy uses the slot information (rather
than PCI address). This causes udev to give shorter network device
names for VF devices.  It does not break applications or startup
because the VF device must never be configured directly.

Signed-off-by: Stephen Hemminger <sthemmin@microsoft.com>
---
 drivers/pci/controller/pci-hyperv.c | 30 +++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/drivers/pci/controller/pci-hyperv.c b/drivers/pci/controller/pci-hyperv.c
index c00f82cc54aa..e6a6c1146a41 100644
--- a/drivers/pci/controller/pci-hyperv.c
+++ b/drivers/pci/controller/pci-hyperv.c
@@ -89,6 +89,8 @@ static enum pci_protocol_version_t pci_protocol_version;
 
 #define STATUS_REVISION_MISMATCH 0xC0000059
 
+#define SLOT_NAME_SIZE 21
+
 /*
  * Message Types
  */
@@ -494,6 +496,7 @@ struct hv_pci_dev {
 	struct list_head list_entry;
 	refcount_t refs;
 	enum hv_pcichild_state state;
+	struct pci_slot *pci_slot;
 	struct pci_function_description desc;
 	bool reported_missing;
 	struct hv_pcibus_device *hbus;
@@ -1457,6 +1460,28 @@ static void prepopulate_bars(struct hv_pcibus_device *hbus)
 	spin_unlock_irqrestore(&hbus->device_list_lock, flags);
 }
 
+static void hv_pci_assign_slots(struct hv_pcibus_device *hbus)
+{
+	struct hv_pci_dev *hpdev;
+	char name[SLOT_NAME_SIZE];
+	unsigned long flags;
+	int slot_nr;
+
+	spin_lock_irqsave(&hbus->device_list_lock, flags);
+	list_for_each_entry(hpdev, &hbus->children, list_entry) {
+		if (hpdev->pci_slot)
+			continue;
+
+		slot_nr = PCI_SLOT(wslot_to_devfn(hpdev->desc.win_slot.slot));
+		snprintf(name, SLOT_NAME_SIZE, "%u", hpdev->desc.ser);
+		hpdev->pci_slot = pci_create_slot(hbus->pci_bus, slot_nr,
+					  name, NULL);
+		if (!hpdev->pci_slot)
+			pr_warn("pci_create slot %s failed\n", name);
+	}
+	spin_unlock_irqrestore(&hbus->device_list_lock, flags);
+}
+
 /**
  * create_root_hv_pci_bus() - Expose a new root PCI bus
  * @hbus:	Root PCI bus, as understood by this driver
@@ -1480,6 +1505,7 @@ static int create_root_hv_pci_bus(struct hv_pcibus_device *hbus)
 	pci_lock_rescan_remove();
 	pci_scan_child_bus(hbus->pci_bus);
 	pci_bus_assign_resources(hbus->pci_bus);
+	hv_pci_assign_slots(hbus);
 	pci_bus_add_devices(hbus->pci_bus);
 	pci_unlock_rescan_remove();
 	hbus->state = hv_pcibus_installed;
@@ -1742,6 +1768,7 @@ static void pci_devices_present_work(struct work_struct *work)
 		 */
 		pci_lock_rescan_remove();
 		pci_scan_child_bus(hbus->pci_bus);
+		hv_pci_assign_slots(hbus);
 		pci_unlock_rescan_remove();
 		break;
 
@@ -1858,6 +1885,9 @@ static void hv_eject_device_work(struct work_struct *work)
 	list_del(&hpdev->list_entry);
 	spin_unlock_irqrestore(&hpdev->hbus->device_list_lock, flags);
 
+	if (hpdev->pci_slot)
+		pci_destroy_slot(hpdev->pci_slot);
+
 	memset(&ctxt, 0, sizeof(ctxt));
 	ejct_pkt = (struct pci_eject_response *)&ctxt.pkt.message;
 	ejct_pkt->message_type.type = PCI_EJECTION_COMPLETE;
-- 
2.18.0

^ permalink raw reply related

* [PATCH net-next 0/2] hv_netvsc: associate VF and PV device by serial number
From: Stephen Hemminger @ 2018-08-29 16:24 UTC (permalink / raw)
  To: kys, haiyangz, sthemmin; +Cc: devel, netdev, linux-pci

The Hyper-V implementation of PCI controller has concept of 32 bit serial number
(not to be confused with PCI-E serial number).  This value is sent in the protocol
from the host to indicate SR-IOV VF device is attached to a synthetic NIC.

Using the serial number (instead of MAC address) to associate the two devices
avoids lots of potential problems when there are duplicate MAC addresses from
tunnels or layered devices.

The patch set is broken into two parts, one is for the PCI controller
and the other is for the netvsc device. Normally, these go through different
trees but sending them together here for better review. The PCI changes
were submitted previously, but the main review comment was "why do you
need this?". This is why.

Stephen Hemminger (2):
  PCI: hv: support reporting serial number as slot information
  hv_netvsc: pair VF based on serial number

 drivers/net/hyperv/netvsc.c         |  3 ++
 drivers/net/hyperv/netvsc_drv.c     | 58 ++++++++++++++++-------------
 drivers/pci/controller/pci-hyperv.c | 30 +++++++++++++++
 3 files changed, 66 insertions(+), 25 deletions(-)

-- 
2.18.0

^ permalink raw reply

* Re: [PATCH bpf-next 00/11] AF_XDP zero-copy support for i40e
From: Daniel Borkmann @ 2018-08-29 16:12 UTC (permalink / raw)
  To: Björn Töpel, magnus.karlsson, magnus.karlsson,
	alexander.h.duyck, alexander.duyck, ast, brouer, netdev,
	jesse.brandeburg, anjali.singhai, peter.waskiewicz.jr
  Cc: Björn Töpel, michael.lundkvist, willemdebruijn.kernel,
	john.fastabend, jakub.kicinski, neerav.parikh, mykyta.iziumtsev,
	francois.ozog, ilias.apalodimas, brian.brooks, u9012063, pavel,
	qi.z.zhang
In-Reply-To: <20180828124435.30578-1-bjorn.topel@gmail.com>

On 08/28/2018 02:44 PM, Björn Töpel wrote:
> From: Björn Töpel <bjorn.topel@intel.com>
> 
> This patch set introduces zero-copy AF_XDP support for Intel's i40e
> driver. In the first preparatory patch we also add support for
> XDP_REDIRECT for zero-copy allocated frames so that XDP programs can
> redirect them. This was a ToDo from the first AF_XDP zero-copy patch
> set from early June. Special thanks to Alex Duyck and Jesper Dangaard
> Brouer for reviewing earlier versions of this patch set.
> 
> The i40e zero-copy code is located in its own file i40e_xsk.[ch]. Note
> that in the interest of time, to get an AF_XDP zero-copy implementation
> out there for people to try, some code paths have been copied from the
> XDP path to the zero-copy path. It is out goal to merge the two paths
> in later patch sets.
> 
> In contrast to the implementation from beginning of June, this patch
> set does not require any extra HW queues for AF_XDP zero-copy
> TX. Instead, the XDP TX HW queue is used for both XDP_REDIRECT and
> AF_XDP zero-copy TX.
> 
> Jeff, given that most of changes are in i40e, it is up to you how you
> would like to route these patches. The set is tagged bpf-next, but
> if taking it via the Intel driver tree is easier, let us know.
> 
> We have run some benchmarks on a dual socket system with two Broadwell
> E5 2660 @ 2.0 GHz with hyperthreading turned off. Each socket has 14
> cores which gives a total of 28, but only two cores are used in these
> experiments. One for TR/RX and one for the user space application. The
> memory is DDR4 @ 2133 MT/s (1067 MHz) and the size of each DIMM is
> 8192MB and with 8 of those DIMMs in the system we have 64 GB of total
> memory. The compiler used is gcc (Ubuntu 7.3.0-16ubuntu3) 7.3.0. The
> NIC is Intel I40E 40Gbit/s using the i40e driver.
> 
> Below are the results in Mpps of the I40E NIC benchmark runs for 64
> and 1500 byte packets, generated by a commercial packet generator HW
> outputing packets at full 40 Gbit/s line rate. The results are with
> retpoline and all other spectre and meltdown fixes, so these results
> are not comparable to the ones from the zero-copy patch set in June.
> 
> AF_XDP performance 64 byte packets.
> Benchmark   XDP_SKB    XDP_DRV    XDP_DRV with zerocopy
> rxdrop       2.6        8.2         15.0
> txpush       2.2        -           21.9
> l2fwd        1.7        2.3         11.3
> 
> AF_XDP performance 1500 byte packets:
> Benchmark   XDP_SKB   XDP_DRV     XDP_DRV with zerocopy
> rxdrop       2.0        3.3         3.3
> l2fwd        1.3        1.7         3.1
> 
> XDP performance on our system as a base line:
> 
> 64 byte packets:
> XDP stats       CPU     pps         issue-pps
> XDP-RX CPU      16      18.4M  0
> 
> 1500 byte packets:
> XDP stats       CPU     pps         issue-pps
> XDP-RX CPU      16      3.3M    0
> 
> The structure of the patch set is as follows:
> 
> Patch 1: Add support for XDP_REDIRECT of zero-copy allocated frames
> Patches 2-4: Preparatory patches to common xsk and net code
> Patches 5-7: Preparatory patches to i40e driver code for RX
> Patch 8: i40e zero-copy support for RX
> Patch 9: Preparatory patch to i40e driver code for TX
> Patch 10: i40e zero-copy support for TX
> Patch 11: Add flags to sample application to force zero-copy/copy mode
> 
> We based this patch set on bpf-next commit 050cdc6c9501 ("Merge
> git://git.kernel.org/pub/scm/linux/kernel/git/davem/net")
> 
> 
> Magnus & Björn
> 
> Björn Töpel (8):
>   xdp: implement convert_to_xdp_frame for MEM_TYPE_ZERO_COPY
>   xdp: export xdp_rxq_info_unreg_mem_model
>   xsk: expose xdp_umem_get_{data,dma} to drivers
>   i40e: added queue pair disable/enable functions
>   i40e: refactor Rx path for re-use
>   i40e: move common Rx functions to i40e_txrx_common.h
>   i40e: add AF_XDP zero-copy Rx support
>   samples/bpf: add -c/--copy -z/--zero-copy flags to xdpsock
> 
> Magnus Karlsson (3):
>   net: add napi_if_scheduled_mark_missed
>   i40e: move common Tx functions to i40e_txrx_common.h
>   i40e: add AF_XDP zero-copy Tx support

Thanks for working on this, LGTM! Are you also planning to get ixgbe
out after that?

For the series:

Acked-by: Daniel Borkmann <daniel@iogearbox.net>

Thanks,
Daniel

^ permalink raw reply

* Re: [pull request][net-next 00/10] Mellanox, mlx5 and devlink updates 2018-07-31
From: Alex Vesker @ 2018-08-29 15:42 UTC (permalink / raw)
  To: Alexander Duyck, Saeed Mahameed
  Cc: Saeed Mahameed, David S. Miller, Netdev, Jiri Pirko,
	Jakub Kicinski, Bjorn Helgaas
In-Reply-To: <2d84340e-0703-0bc7-4917-3b18979b2aa5@mellanox.com>


> On Wed, Aug 1, 2018 at 4:13 PM, Saeed Mahameed
> <saeedm@dev.mellanox.co.il> wrote:
>> On Wed, Aug 1, 2018 at 3:34 PM, Alexander Duyck
>> <alexander.duyck@gmail.com> wrote:
>>> On Wed, Aug 1, 2018 at 2:52 PM, Saeed Mahameed <saeedm@mellanox.com> 
>>> wrote:
>>>> Hi Dave,
>>>>
>>>> This series provides devlink parameters updates to both devlink API 
>>>> and
>>>> mlx5 driver, it is a 2nd iteration of the dropped patches sent in a 
>>>> previous
>>>> mlx5 submission "net/mlx5: Support PCIe buffer congestion handling via
>>>> Devlink" to address review comments [1].
>>>>
>>>> Changes from the original series:
>>>> - According to the discussion outcome, we are keeping the 
>>>> congestion control
>>>>   setting as mlx5 device specific for the current HW generation.
>>>> - Changed the congestion_mode and congestion action param type to 
>>>> string
>>>> - Added patches to fix devlink handling of param type string
>>>> - Added a patch which adds extack messages support for param set.
>>>> - At the end of this series, I've added yet another mlx5 devlink 
>>>> related
>>>>  feature, firmware snapshot support.
>>>>
>>>> For more information please see tag log below.
>>>>
>>>> Please pull and let me know if there's any problem.
>>>>
>>>> [1] https://patchwork.ozlabs.org/patch/945996/
>>>>
>>>> Thanks,
>>>> Saeed.
>>>>
>>>> ---
>>>>
>>>> The following changes since commit 
>>>> e6476c21447c4b17c47e476aade6facf050f31e8:
>>>>
>>>>   net: remove bogus RCU annotations on socket.wq (2018-07-31 
>>>> 12:40:22 -0700)
>>>>
>>>> are available in the Git repository at:
>>>>
>>>> git://git.kernel.org/pub/scm/linux/kernel/git/saeed/linux.git 
>>>> tags/mlx5-updates-2018-08-01
>>>>
>>>> for you to fetch changes up to 
>>>> 2ac6108c65ffcb1e5eab1fba1fd59272604d1c32:
>>>>
>>>>   net/mlx5: Use devlink region_snapshot parameter (2018-08-01 
>>>> 14:49:09 -0700)
>>>>
>>>> ----------------------------------------------------------------
>>>> mlx5-updates-2018-08-01
>>>>
>>>> This series provides devlink parameters updates to both devlink API 
>>>> and
>>>> mlx5 driver,
>>>>
>>>> 1) Devlink changes: (Moshe Shemesh)
>>>> The first two patches fix devlink param infrastructure for string type
>>>> params.
>>>> The third patch adds a devlink helper function to safely copy 
>>>> string from
>>>> driver to devlink.
>>>> The forth patch adds extack support for param set.
>>>>
>>>> 2) mlx5 specific congestion parameters: (Eran Ben Elisha)
>>>> Next three patches add new devlink driver specific params for 
>>>> controlling
>>>> congestion action and mode, using string type params and extack 
>>>> messages support.
>>>>
>>>> This congestion mode enables hw workaround in specific devices 
>>>> which is
>>>> controlled by devlink driver-specific params. The workaround is device
>>>> specific for this NIC generation, the next NIC will not need it.
>>>>
>>>> Congestion parameters:
>>>>  - Congestion action
>>>>             HW W/A mechanism in the PCIe buffer which monitors the 
>>>> amount of
>>>>             consumed PCIe buffer per host.  This mechanism supports 
>>>> the
>>>>             following actions in case of threshold overflow:
>>>>             - Disabled - NOP (Default)
>>>>             - Drop
>>>>             - Mark - Mark CE bit in the CQE of received packet
>>>>     - Congestion mode
>>>>             - Aggressive - Aggressive static trigger threshold 
>>>> (Default)
>>>>             - Dynamic - Dynamically change the trigger threshold
>>>>
>>>> 3) mlx5 firmware snapshot support via devlink: (Alex Vesker)
>>>> Last three patches, add the support for capturing region snapshot 
>>>> of the
>>>> firmware crspace during critical errors, using devlink region_snapshot
>>>> parameter.
>>>>
>>>> -Saeed.
>>>>
>>>> ----------------------------------------------------------------
>>>> Alex Vesker (3):
>>>>       net/mlx5: Add Vendor Specific Capability access gateway
>>>>       net/mlx5: Add Crdump FW snapshot support
>>>>       net/mlx5: Use devlink region_snapshot parameter
>>>>
>>>> Eran Ben Elisha (3):
>>>>       net/mlx5: Move all devlink related functions calls to devlink.c
>>>>       net/mlx5: Add MPEGC register configuration functionality
>>>>       net/mlx5: Enable PCIe buffer congestion handling workaround 
>>>> via devlink
>>>>
>>>> Moshe Shemesh (4):
>>>>       devlink: Fix param set handling for string type
>>>>       devlink: Fix param cmode driverinit for string type
>>>>       devlink: Add helper function for safely copy string param
>>>>       devlink: Add extack messages support to param set
>>>>
>>>>  drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c  |   3 +-
>>>>  drivers/net/ethernet/mellanox/mlx4/main.c          |   6 +-
>>>>  drivers/net/ethernet/mellanox/mlx5/core/Makefile   |   3 +-
>>>>  drivers/net/ethernet/mellanox/mlx5/core/devlink.c  | 388 
>>>> +++++++++++++++++++++
>>>>  drivers/net/ethernet/mellanox/mlx5/core/devlink.h  |  13 +
>>>>  .../net/ethernet/mellanox/mlx5/core/diag/crdump.c  | 223 ++++++++++++
>>>>  drivers/net/ethernet/mellanox/mlx5/core/health.c   |   3 +
>>>>  drivers/net/ethernet/mellanox/mlx5/core/lib/mlx5.h |   4 +
>>>>  .../net/ethernet/mellanox/mlx5/core/lib/pci_vsc.c  | 320 
>>>> +++++++++++++++++
>>>>  .../net/ethernet/mellanox/mlx5/core/lib/pci_vsc.h  |  56 +++
>>>>  drivers/net/ethernet/mellanox/mlx5/core/main.c     |  10 +-
>>>>  include/linux/mlx5/driver.h                        |   5 +
>>>>  include/net/devlink.h                              |  15 +-
>>>>  net/core/devlink.c                                 |  44 ++-
>>>>  14 files changed, 1076 insertions(+), 17 deletions(-)
>>>>  create mode 100644 drivers/net/ethernet/mellanox/mlx5/core/devlink.c
>>>>  create mode 100644 drivers/net/ethernet/mellanox/mlx5/core/devlink.h
>>>>  create mode 100644 
>>>> drivers/net/ethernet/mellanox/mlx5/core/diag/crdump.c
>>>>  create mode 100644 
>>>> drivers/net/ethernet/mellanox/mlx5/core/lib/pci_vsc.c
>>>>  create mode 100644 
>>>> drivers/net/ethernet/mellanox/mlx5/core/lib/pci_vsc.h
>>>
>>> So after looking over the patch set the one thing I would ask for in
>>> this is some sort of documentation at a minimum. As a user I don't see
>>> how you can expect someone to be able to use this when the naming of
>>> things are pretty cryptic and there is no real explanation anywhere if
>>> you don't go through and read the patch description itself. When you
>>> start adding driver specific interfaces, you should at least start
>>> adding vendor specific documentation.
>>>
>>
>> Sure, sounds like a great idea, something like:
>> Documentation/networking/mlx5.txt and have a devlink section ?
>> or have a generic devlink doc and a mlx5 section in it ?
>
> Either would work for me.
>
For which patches are you missing documentation?

>>> Also I don't see how using a vendor specific configuration space
>>> section can be done without adding some tie-ins to the PCI core files
>>> because it should be possible to race with someone poking at the
>>> register space via something like setpci/lspci. Also one of the things
>>> that came up was that drivers are not supposed to be banging on the
>>> PCI configuration space at will, and it seems like this patch set is
>>> doing exactly that through the VSC block.
>>>
>>
>> this is a whole different feature than the device specific parameters.
>> The whole vendor specific configuration space access is needed only
>> for diagnostic/dump
>> purposes when something really bad happens and the command interface
>> with FW is down,
>> and when the FW is un-responsive, we want to dump the crspace into the
>> already existing devlink
>> crdump buffer, how do you expect us to read it if we are not allowed
>> to access it ?
>>
>> What do you mean by tie-ins to the PCI core files ? can you please 
>> elaborate ?
>
> You have added a vendor specific config section and you are using it
> to access several of the pieces of metadata. The setup isn't too
> different than the VPD setup and approach. However I don't see many of
> the protections that exist for VPD in place for this vendor specific
> configuration. As such I have concerns. For example what is to keep
> requests to the various devlink interfaces from racing with each other
> when they both end up operating through the VCS?
>
> - Alex

Hi, I would like to resubmit the devlink region crdump support for mlx5,
which is part of this patch-set.

AlexD, regarding the protection, various devlink interfaces cannot race 
since
devlink_mutex is used. The VSC access is also protected, using 
mlx5_vsc_gw_lock/unlock
only one can acquire the lock.
After explaining this I want to clarify something, the access to VSC is 
not user driven
it happens automatically by the driver when an error is detected to 
collect a crdump.

^ permalink raw reply

* Re: [Patch iproute2] ss: add UNIX_DIAG_VFS and UNIX_DIAG_ICONS for unix sockets
From: Stephen Hemminger @ 2018-08-29 15:41 UTC (permalink / raw)
  To: Cong Wang; +Cc: Linux Kernel Network Developers
In-Reply-To: <CAM_iQpWod288CWHmDzzVkrdivjQtTBY+ES4hErMRkqY4=r6TfQ@mail.gmail.com>

On Tue, 28 Aug 2018 16:16:49 -0700
Cong Wang <xiyou.wangcong@gmail.com> wrote:

> On Mon, Aug 27, 2018 at 3:27 PM Stephen Hemminger
> <stephen@networkplumber.org> wrote:
> >
> > On Mon, 27 Aug 2018 14:46:52 -0700
> > Cong Wang <xiyou.wangcong@gmail.com> wrote:
> >  
> > > UNIX_DIAG_VFS and UNIX_DIAG_ICONS are never used by ss,
> > > make them available in ss -e output.
> > >
> > > Cc: Stephen Hemminger <stephen@networkplumber.org>
> > > Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
> > > ---
> > >  misc/ss.c | 25 +++++++++++++++++++++++++
> > >  1 file changed, 25 insertions(+)
> > >
> > > diff --git a/misc/ss.c b/misc/ss.c
> > > index 41e7762b..d28bc1ec 100644
> > > --- a/misc/ss.c
> > > +++ b/misc/ss.c
> > > @@ -16,6 +16,7 @@
> > >  #include <sys/ioctl.h>
> > >  #include <sys/socket.h>
> > >  #include <sys/uio.h>
> > > +#include <sys/sysmacros.h>  
> >
> > Why is this included, it isn't on my system.  
> 
> It is for major() and minor().

Ok on Debian, these are in the architecture include, so this will work fine.

^ permalink raw reply

* Skiers List
From: Katrina Muller @ 2018-08-29 15:17 UTC (permalink / raw)
  To: netdev


Hi,

Would you be interested in acquiring an email list of "Skiers List" from USA?

We also have data for Spa and Resort Visitors, Timeshare Owners, Entertainment Enthusiasts List, Golfers List, Basketball Enthusiasts, Baseball Enthusiasts, Students List, Apparel Buyers, Soccer Enthusiasts, Cycling Enthusiasts, and many more..

Each record in the list contains Contact Name (First, Middle and Last Name), Mailing Address, List type and Opt-in email address.

All the contacts are opt-in verified, 100% permission based and can be used for unlimited multi-channel marketing.

Please let me know your thoughts towards procuring the Skiers List.

Best Regards,
Katrina Muller
Research Analyst


We respect your privacy, if you do not wish to receive any further emails from our end, please reply with a subject “Leave Out”.

^ permalink raw reply

* Re: [bpf-next PATCH 0/2] bpf: test_sockmap updates
From: Daniel Borkmann @ 2018-08-29 15:38 UTC (permalink / raw)
  To: John Fastabend, alexei.starovoitov; +Cc: netdev
In-Reply-To: <20180828160921.24004.71893.stgit@john-Precision-Tower-5810>

On 08/28/2018 06:10 PM, John Fastabend wrote:
> Two small test sockmap updates for bpf-next. These help me run some
> additional tests with test_sockmap.

Applied to bpf-next, thanks!

^ permalink raw reply

* Re: [PATCH bpf-next] bpf: remove duplicated include from syscall.c
From: Daniel Borkmann @ 2018-08-29 15:37 UTC (permalink / raw)
  To: YueHaibing, Alexei Starovoitov; +Cc: netdev, kernel-janitors
In-Reply-To: <1535442152-5021-1-git-send-email-yuehaibing@huawei.com>

On 08/28/2018 09:42 AM, YueHaibing wrote:
> Remove duplicated include.
> 
> Signed-off-by: YueHaibing <yuehaibing@huawei.com>

Applied to bpf-next, thanks!

^ permalink raw reply

* Re: [PATCH] rtnetlink: expose value from SET_NETDEV_DEVTYPE via IFLA_DEVTYPE attribute
From: Marcel Holtmann @ 2018-08-29 15:31 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: Jiri Pirko, netdev, David S. Miller
In-Reply-To: <20180829082428.0da2d748@xeon-e3>

Hi Stephen,

>>> The name value from SET_NETDEV_DEVTYPE only ended up in the uevent sysfs
>>> file as DEVTYPE= information. To avoid any kind of race conditions
>>> between netlink messages and reading from sysfs, it is useful to add the
>>> same string as new IFLA_DEVTYPE attribute included in the RTM_NEWLINK
>>> messages.
>>> 
>>> For network managing daemons that have to classify ARPHRD_ETHER network
>>> devices into different types (like Wireless LAN, Bluetooth etc.), this
>>> avoids the extra round trip to sysfs and parsing of the uevent file.
>>> 
>>> Signed-off-by: Marcel Holtmann <marcel@holtmann.org>
>>> ---
>>> include/uapi/linux/if_link.h |  2 ++
>>> net/core/rtnetlink.c         | 12 ++++++++++++
>>> 2 files changed, 14 insertions(+)
>>> 
>>> diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
>>> index 43391e2d1153..781294972bb4 100644
>>> --- a/include/uapi/linux/if_link.h
>>> +++ b/include/uapi/linux/if_link.h
>>> @@ -166,6 +166,8 @@ enum {
>>> 	IFLA_NEW_IFINDEX,
>>> 	IFLA_MIN_MTU,
>>> 	IFLA_MAX_MTU,
>>> +	IFLA_DEVTYPE,		/* Name value from SET_NETDEV_DEVTYPE */  
>> 
>> This is not something netdev-related. dev->dev.type is struct device_type.
>> This is a generic "device" thing. Incorrect to expose over
>> netdev-specific API. Please use "device" API for this.
> 
> There is no device API in netlink. The whole point of this patch is to
> do it in one message. It might be a performance optimization, but I can't
> see how it could be a race condition. Devices set type before registering.

the only way right now to pick up the DEVTYPE= value is from the /sys/class/net/*/uevent file. That is based on the ifname and not the index. When udev + systemd start renaming things behind your back, your daemon does not have a clean one-shot way of getting that information. As stated, the information in DEVTYPE= are a sub-classification of ARPHRD_ETHER and allows to differentiate a wired Ethernet card from a WiFi interface, from a Bluetooth interface, from WiMAX and so on. They just happen to be in dev.dev_type data structure at the moment and I didn't want to duplicate that information.

I am actually fine doing this via IFLA_INFO_KIND since NetworkManager seems to be fixed now or do this via a different method or maybe just a different attribute name. I really just want to get the sub-classification of ARPHRD_ETHER that we need in userspace networking daemons from the kernel without having to go poke left and right over sysfs or interact with udev or systemd.

Regards

Marcel

^ permalink raw reply

* Re: Kernel Panic on high bandwidth transfer over wifi
From: Eric Dumazet @ 2018-08-29 15:29 UTC (permalink / raw)
  To: Nathaniel Munk, netdev@vger.kernel.org
In-Reply-To: <porA6Oc9uVOmzI_nV2MhBU5RzBhBCq82gffuA1TBkrQxwTToIy03qc44DkN8BmdVKgQAGU6qDyrkCx7sDQI5SbSt9b9-HHf9ufHcIQlT-kg=@munk.com.au>



On 08/29/2018 04:42 AM, Nathaniel Munk wrote:
> Hi all,
> I'm running Arch Linux on kernel 4.18.5 (same issue on both arch-provided kernel and mainline built-from-source). There is an issue whereby the kernel crashes when transferring at high bandwidths (approx 6mB/s) over a specific wifi connection. I can only reproduce the issue when using the Personal Hotspot on my iPhone 6S+, but can reproduce it very consistently on that connection.
> 
> More often than not, any download reaching this speed will cause a panic, but if the download is immediately terminated at the first error the system can recover (and doing this I have obtained the attached logs). Unfortunately, I have not had access to a second machine to obtain the netconsole printout of the panic.
> 
> As above, high-bandwidth transfers on other wifi networks do not cause the issue (nor on ethernet connections).
> 
> As you can see from the attached log, the issue appears at tcp_recvmsg+0x579 and net_tx_action+0x1fe. At both these positions (net/ipv4/tcp.c:2000 and net/core/dev.c:4279 in mainline 4.18.5), a member of the skb struct is called.
> 
> Thank you for your time (and I apologize if this is spurious or badly worded, this is my first bug report), and please don't hesitate to let me know if there's anything else I can do to help work this out.
> 
> Regards,
> -------------------
> Nathaniel Munk
> nathaniel@munk.com.au
> 

Unfortunately there is no attached log ;)

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox