* Re: dsa traffic priorization
From: Vladimir Oltean @ 2019-09-19 8:42 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: Sascha Hauer, netdev, Vivien Didelot, Florian Fainelli, kernel,
Andrew Lunn
In-Reply-To: <20190919083638.wgxrrgtqxwpjcsu3@pengutronix.de>
Hi Uwe,
On Thu, 19 Sep 2019 at 11:36, Uwe Kleine-König
<u.kleine-koenig@pengutronix.de> wrote:
>
> On Thu, Sep 19, 2019 at 10:00:51AM +0200, Sascha Hauer wrote:
> > Hi Vladimir,
> >
> > On Wed, Sep 18, 2019 at 05:36:08PM +0300, Vladimir Oltean wrote:
> > > Hi Sascha,
> > >
> > > On Wed, 18 Sep 2019 at 17:03, Sascha Hauer <s.hauer@pengutronix.de> wrote:
> > > >
> > > > Hi All,
> > > >
> > > > We have a customer using a Marvell 88e6240 switch with Ethercat on one port and
> > > > regular network traffic on another port. The customer wants to configure two things
> > > > on the switch: First Ethercat traffic shall be priorized over other network traffic
> > > > (effectively prioritizing traffic based on port). Second the ethernet controller
> > > > in the CPU is not able to handle full bandwidth traffic, so the traffic to the CPU
> > > > port shall be rate limited.
> > > >
> > >
> > > You probably already know this, but egress shaping will not drop
> > > frames, just let them accumulate in the egress queue until something
> > > else happens (e.g. queue occupancy threshold triggers pause frames, or
> > > tail dropping is enabled, etc). Is this what you want?
> >
> > If I understand correctly then the switch has multiple output queues per
> > port. The Ethercat traffic will go to a higher priority queue and on
> > congestion on other queues, frames designated for that queue will be
> > dropped. I just talked to our customer and he verified that their
> > Ethercat traffic still goes through even when the ports with the general
> > traffic are jammed with packets. So yes, I think this is what I want.
>
> Moreover egressing the cpu port has the advantage that network
> participants on other ports that might be able to process packet quicker
> are not limited.
>
Yes, that is true if you apply port-based policers (matchall), but not
necessarily so if your switch is able to do ingress classification (a
common key for switches being the {DMAC, VLAN} pair). Again, there is
a model for configuring even that, if the hardware is capable. Just
not for egress shaping on the CPU port.
> Best regards
> Uwe
>
> --
> Pengutronix e.K. | Uwe Kleine-König |
> Industrial Linux Solutions | http://www.pengutronix.de/ |
^ permalink raw reply
* Re: dsa traffic priorization
From: Sascha Hauer @ 2019-09-19 8:44 UTC (permalink / raw)
To: Florian Fainelli
Cc: Vladimir Oltean, netdev, Andrew Lunn, Vivien Didelot, kernel
In-Reply-To: <1b80f9ed-7a62-99c4-10bc-bc1887f80867@gmail.com>
Hi Florian,
On Wed, Sep 18, 2019 at 10:41:58AM -0700, Florian Fainelli wrote:
> On 9/18/19 7:36 AM, Vladimir Oltean wrote:
> > Hi Sascha,
> >
> > On Wed, 18 Sep 2019 at 17:03, Sascha Hauer <s.hauer@pengutronix.de> wrote:
> >>
> >> Hi All,
> >>
> >> We have a customer using a Marvell 88e6240 switch with Ethercat on one port and
> >> regular network traffic on another port. The customer wants to configure two things
> >> on the switch: First Ethercat traffic shall be priorized over other network traffic
> >> (effectively prioritizing traffic based on port). Second the ethernet controller
> >> in the CPU is not able to handle full bandwidth traffic, so the traffic to the CPU
> >> port shall be rate limited.
> >>
> >
> > You probably already know this, but egress shaping will not drop
> > frames, just let them accumulate in the egress queue until something
> > else happens (e.g. queue occupancy threshold triggers pause frames, or
> > tail dropping is enabled, etc). Is this what you want? It sounds a bit
> > strange to me to configure egress shaping on the CPU port of a DSA
> > switch. That literally means you are buffering frames inside the
> > system. What about ingress policing?
>
> Indeed, but I suppose that depending on the switch architecture and/or
> nomenclature, configuring egress shaping amounts to determining ingress
> for the ports where the frame is going to be forwarded to.
>
> For instance Broadcom switches rarely if at all mention ingress because
> the frames have to originate from somewhere and be forwarded to other
> port(s), therefore, they will egress their original port (which for all
> practical purposes is the direct continuation of the ingress stage),
> where shaping happens, which immediately influences the ingress shaping
> of the destination port, which will egress the frame eventually because
> packets have to be delivered to the final port's egress queue anyway.
>
> >
> >> For reference the patch below configures the switch to their needs. Now the question
> >> is how this can be implemented in a way suitable for mainline. It looks like the per
> >> port priority mapping for VLAN tagged packets could be done via ip link add link ...
> >> ingress-qos-map QOS-MAP. How the default priority would be set is unclear to me.
> >>
> >
> > Technically, configuring a match-all rxnfc rule with ethtool would
> > count as 'default priority' - I have proposed that before. Now I'm not
> > entirely sure how intuitive it is, but I'm also interested in being
> > able to configure this.
>
> That does not sound too crazy from my perspective.
>
> >
> >> The other part of the problem seems to be that the CPU port has no network device
> >> representation in Linux, so there's no interface to configure the egress limits via tc.
> >> This has been discussed before, but it seems there hasn't been any consensous regarding how
> >> we want to proceed?
>
> You have the DSA master network device which is on the other side of the
> switch,
You mean the (in my case) i.MX FEC? Everything I do on this device ends
up in the FEC itself and not on the switch, right?
Sascha
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply
* Fw: [Bug 204903] New: unable to create vrf interface when ipv6.disable=1
From: Stephen Hemminger @ 2019-09-19 8:46 UTC (permalink / raw)
To: David Ahern; +Cc: netdev
Begin forwarded message:
Date: Wed, 18 Sep 2019 15:15:42 +0000
From: bugzilla-daemon@bugzilla.kernel.org
To: stephen@networkplumber.org
Subject: [Bug 204903] New: unable to create vrf interface when ipv6.disable=1
https://bugzilla.kernel.org/show_bug.cgi?id=204903
Bug ID: 204903
Summary: unable to create vrf interface when ipv6.disable=1
Product: Networking
Version: 2.5
Kernel Version: 5.2.14
Hardware: All
OS: Linux
Tree: Mainline
Status: NEW
Severity: normal
Priority: P1
Component: Other
Assignee: stephen@networkplumber.org
Reporter: zhangyoufu@gmail.com
Regression: No
`ip link add vrf0 type vrf table 100` fails with EAFNOSUPPORT when boot with
`ipv6.disable=1`. There must be somewhere inside `vrf_newlink` trying to use
IPv6 without checking availablity. Maybe `vrf_add_fib_rules` I guess.
--
You are receiving this mail because:
You are the assignee for the bug.
^ permalink raw reply
* Re: [GIT] Networking
From: David Miller @ 2019-09-19 8:49 UTC (permalink / raw)
To: torvalds; +Cc: akpm, netdev, linux-kernel
In-Reply-To: <CAHk-=whSTx4ofABCgWVe_2Kfo3CV6kSkBSRQBR-o5=DefgXnUQ@mail.gmail.com>
From: Linus Torvalds <torvalds@linux-foundation.org>
Date: Wed, 18 Sep 2019 13:37:57 -0700
> I've obviously already pulled this (and only noticed when I was
> testing further on my laptop), but please explain or fix.
I'll take a look, thanks.
^ permalink raw reply
* Re: [PATCH net 0/3] Fix Qdisc destroy issues caused by adding fine-grained locking to filter API
From: Vlad Buslov @ 2019-09-19 8:53 UTC (permalink / raw)
To: Cong Wang
Cc: Vlad Buslov, Linux Kernel Network Developers, Jamal Hadi Salim,
Jiri Pirko, David Miller
In-Reply-To: <CAM_iQpX6RAmf4oXLLJnhYpaXX4g7MUmZ33GZgwrYaiPLBGxmYw@mail.gmail.com>
On Thu 19 Sep 2019 at 01:50, Cong Wang <xiyou.wangcong@gmail.com> wrote:
> On Wed, Sep 18, 2019 at 12:32 AM Vlad Buslov <vladbu@mellanox.com> wrote:
>>
>> TC filter API unlocking introduced several new fine-grained locks. The
>> change caused sleeping-while-atomic BUGs in several Qdiscs that call cls
>> APIs which need to obtain new mutex while holding sch tree spinlock. This
>> series fixes affected Qdiscs by ensuring that cls API that became sleeping
>> is only called outside of sch tree lock critical section.
>
> Sorry I just took a deeper look. It seems harder than just moving it
> out of the critical section.
>
> qdisc_destroy() calls ops->reset() which usually purges queues,
> I don't see how it is safe to move it out of tree spinlock without
> respecting fast path.
>
> What do you think?
Hmm, maybe we can split qdisc destruction in two stage process for
affected qdiscs? Rough sketch:
1. Call qdisc_reset() (or qdisc_purge_queue()) on qdisc that are being
deleted under sch tree lock protection.
2. Call new qdisc_put_empty() function after releasing the lock. This
function would implement same functionality as a regular qdisc_put()
besides resetting the Qdisc and freeing skb in its queues (already
done by qdisc_reset())
In fact, affected queues already do the same or something similar:
- htb_change_class() calls qdisc_purge_queue() that calls qdisc_reset(),
which makes reset inside qdisc_destroy() redundant.
- multiq_tune() calls qdisc_tree_flush_backlog() that has the same
implementation as qdisc_purge_queue() minus actually resetting the
Qdisc. Can we substitute first function with the second one here?
- sfb_change() - same as multiq_tune().
Do you think that would work?
Also, I'm kind of surprised that it worked before. Even though syzbot
complains about mutex that I added, cls API never honored expectation by
such Qdiscs that tcf_block_put doesn't sleep. From the top of my head
there are several places in the cls code where it has never been the
case: chain head change for ingress/clsact Qdiscs calls
mini_qdisc_pair_swap() which is a sleeping function,
tcf_block_flush_all_chains() eventually calls driver callbacks for any
offloaded filters which may sleep.
^ permalink raw reply
* Re: [PATCH net 2/3] net: sched: multiq: don't call qdisc_put() while holding tree lock
From: Vlad Buslov @ 2019-09-19 8:56 UTC (permalink / raw)
To: Cong Wang
Cc: Vlad Buslov, Linux Kernel Network Developers, Jamal Hadi Salim,
Jiri Pirko, David Miller
In-Reply-To: <CAM_iQpV75h9npv2TbBMoRAMf+riPqJhAY2LaiVX-mrVGaUN-Kw@mail.gmail.com>
On Thu 19 Sep 2019 at 01:56, Cong Wang <xiyou.wangcong@gmail.com> wrote:
> On Wed, Sep 18, 2019 at 12:32 AM Vlad Buslov <vladbu@mellanox.com> wrote:
>> diff --git a/net/sched/sch_multiq.c b/net/sched/sch_multiq.c
>> index e1087746f6a2..4cfa9a7bd29e 100644
>> --- a/net/sched/sch_multiq.c
>> +++ b/net/sched/sch_multiq.c
>> @@ -187,18 +187,21 @@ static int multiq_tune(struct Qdisc *sch, struct nlattr *opt,
>>
>> sch_tree_lock(sch);
>> q->bands = qopt->bands;
>> + sch_tree_unlock(sch);
>> +
>> for (i = q->bands; i < q->max_bands; i++) {
>> if (q->queues[i] != &noop_qdisc) {
>> struct Qdisc *child = q->queues[i];
>>
>> + sch_tree_lock(sch);
>> q->queues[i] = &noop_qdisc;
>> qdisc_tree_flush_backlog(child);
>> + sch_tree_unlock(sch);
>> +
>> qdisc_put(child);
>> }
>> }
>
> Repeatedly acquiring and releasing a spinlock in a loop
> does not seem to be a good idea. Is it possible to save
> those qdisc pointers to an array or something similar?
>
> Thanks.
Sure. I implemented it the way I did because following loop in
multiq_tune() is implemented in exactly the same way: it repeatedly
acquires and releases sch tree lock for each new default Qdisc that it
creates.
^ permalink raw reply
* Re: [PATCH] net: mdio-octeon: Fix build error and Kconfig warning
From: Geert Uytterhoeven @ 2019-09-19 9:06 UTC (permalink / raw)
To: Randy Dunlap
Cc: Nathan Chancellor, David S. Miller, driverdevel, Andrew Lunn,
Florian Fainelli, kbuild test robot, kernel-build-reports,
Greg KH, Matthew Wilcox, Mark Brown, Linux-Next, netdev,
Linux ARM, Heiner Kallweit
In-Reply-To: <64f7ef68-c373-5ff5-ff6d-8a7ce0e30798@infradead.org>
On Thu, Aug 1, 2019 at 1:52 AM Randy Dunlap <rdunlap@infradead.org> wrote:
> However, there are lots of type/cast warnings in both mdio-octeon and mdio-cavium:
>
> ../drivers/net/phy/mdio-octeon.c: In function ‘octeon_mdiobus_probe’:
> ../drivers/net/phy/mdio-octeon.c:48:3: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
> (u64)devm_ioremap(&pdev->dev, mdio_phys, regsize);
> ^
cavium_mdiobus.register_base should be "void __iomem *" instead of "u64",
and the cast should be dropped.
> In file included from ../drivers/net/phy/mdio-octeon.c:14:0:
> ../drivers/net/phy/mdio-cavium.h:113:48: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
> #define oct_mdio_writeq(val, addr) writeq(val, (void *)addr)
> ^
... which allows to drop this cast as well.
Casts are evil, and usually a sign that you're doing something wrong.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* [PATCH -net] zd1211rw: zd_usb: Use "%zu" to format size_t
From: Geert Uytterhoeven @ 2019-09-19 9:15 UTC (permalink / raw)
To: Gustavo A . R . Silva, Daniel Drake, Ulrich Kunitz, Kalle Valo,
David S . Miller
Cc: linux-wireless, netdev, linux-kernel, Geert Uytterhoeven
On 32-bit:
drivers/net/wireless/zydas/zd1211rw/zd_usb.c: In function ‘check_read_regs’:
drivers/net/wireless/zydas/zd1211rw/zd_def.h:18:25: warning: format ‘%ld’ expects argument of type ‘long int’, but argument 6 has type ‘size_t’ {aka ‘unsigned int’} [-Wformat=]
dev_printk(level, dev, "%s() " fmt, __func__, ##args)
^~~~~~~
drivers/net/wireless/zydas/zd1211rw/zd_def.h:22:4: note: in expansion of macro ‘dev_printk_f’
dev_printk_f(KERN_DEBUG, dev, fmt, ## args)
^~~~~~~~~~~~
drivers/net/wireless/zydas/zd1211rw/zd_usb.c:1635:3: note: in expansion of macro ‘dev_dbg_f’
dev_dbg_f(zd_usb_dev(usb),
^~~~~~~~~
drivers/net/wireless/zydas/zd1211rw/zd_usb.c:1636:51: note: format string is defined here
"error: actual length %d less than expected %ld\n",
~~^
%d
Fixes: 84b0b66352470e64 ("zd1211rw: zd_usb: Use struct_size() helper")
Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
drivers/net/wireless/zydas/zd1211rw/zd_usb.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/wireless/zydas/zd1211rw/zd_usb.c b/drivers/net/wireless/zydas/zd1211rw/zd_usb.c
index 4e44ea8c652d65aa..7b5c2fe5bd4d9cde 100644
--- a/drivers/net/wireless/zydas/zd1211rw/zd_usb.c
+++ b/drivers/net/wireless/zydas/zd1211rw/zd_usb.c
@@ -1633,7 +1633,7 @@ static bool check_read_regs(struct zd_usb *usb, struct usb_req_read_regs *req,
*/
if (rr->length < struct_size(regs, regs, count)) {
dev_dbg_f(zd_usb_dev(usb),
- "error: actual length %d less than expected %ld\n",
+ "error: actual length %d less than expected %zu\n",
rr->length, struct_size(regs, regs, count));
return false;
}
--
2.17.1
^ permalink raw reply related
* Re: [bpf-next,v3] samples: bpf: add max_pckt_size option at xdp_adjust_tail
From: Daniel T. Lee @ 2019-09-19 9:16 UTC (permalink / raw)
To: Andrii Nakryiko; +Cc: Daniel Borkmann, Alexei Starovoitov, Networking, bpf
In-Reply-To: <CAEf4BzY_EAf9pH7YvL9XAXPUr9+g5Q7N_n45XBufdxkfDbf3aQ@mail.gmail.com>
On Thu, Sep 19, 2019 at 3:00 AM Andrii Nakryiko
<andrii.nakryiko@gmail.com> wrote:
>
> On Wed, Sep 18, 2019 at 10:37 AM Daniel T. Lee <danieltimlee@gmail.com> wrote:
> >
> > On Tue, Sep 17, 2019 at 1:04 PM Andrii Nakryiko
> > <andrii.nakryiko@gmail.com> wrote:
> > >
> > > On Wed, Sep 11, 2019 at 2:33 PM Daniel T. Lee <danieltimlee@gmail.com> wrote:
> > > >
> > > > Currently, at xdp_adjust_tail_kern.c, MAX_PCKT_SIZE is limited
> > > > to 600. To make this size flexible, a new map 'pcktsz' is added.
> > > >
> > > > By updating new packet size to this map from the userland,
> > > > xdp_adjust_tail_kern.o will use this value as a new max_pckt_size.
> > > >
> > > > If no '-P <MAX_PCKT_SIZE>' option is used, the size of maximum packet
> > > > will be 600 as a default.
> > > >
> > > > Signed-off-by: Daniel T. Lee <danieltimlee@gmail.com>
> > > >
> > > > ---
> > > > Changes in v2:
> > > > - Change the helper to fetch map from 'bpf_map__next' to
> > > > 'bpf_object__find_map_fd_by_name'.
> > > >
> > > > samples/bpf/xdp_adjust_tail_kern.c | 23 +++++++++++++++++++----
> > > > samples/bpf/xdp_adjust_tail_user.c | 28 ++++++++++++++++++++++------
> > > > 2 files changed, 41 insertions(+), 10 deletions(-)
> > > >
> > > > diff --git a/samples/bpf/xdp_adjust_tail_kern.c b/samples/bpf/xdp_adjust_tail_kern.c
> > > > index 411fdb21f8bc..d6d84ffe6a7a 100644
> > > > --- a/samples/bpf/xdp_adjust_tail_kern.c
> > > > +++ b/samples/bpf/xdp_adjust_tail_kern.c
> > > > @@ -25,6 +25,13 @@
> > > > #define ICMP_TOOBIG_SIZE 98
> > > > #define ICMP_TOOBIG_PAYLOAD_SIZE 92
> > > >
> > > > +struct bpf_map_def SEC("maps") pcktsz = {
> > > > + .type = BPF_MAP_TYPE_ARRAY,
> > > > + .key_size = sizeof(__u32),
> > > > + .value_size = sizeof(__u32),
> > > > + .max_entries = 1,
> > > > +};
> > > > +
> > >
> > > Hey Daniel,
> > >
> > > This looks like an ideal use case for global variables on BPF side. I
> > > think it's much cleaner and will make BPF side of things simpler.
> > > Would you mind giving global data a spin instead of adding this map?
> > >
> >
> > Sure thing!
> > But, I'm not sure there is global variables for BPF?
> > AFAIK, there aren't any support for global variables yet in BPF
> > program (_kern.c).
> >
> > # when defining global variable at _kern.c
> > libbpf: bpf: relocation: not yet supported relo for non-static
> > global '<var>' variable found in insns[39].code 0x18
>
> just what it says: use static global variable (also volatile to
> prevent compiler optimizations) :)
>
> static volatile __u32 pcktsz; /* this should work */
>
My apologies, but I'm not sure I'm following.
What you are saying is, should I define global variable to _kern,c
and access and modify this variable from _user.c?
For example,
<_kern.c>
static volatile __u32 pcktsz = 300;
<_user.c>
extern __u32 pcktsz;
// Later in code
pcktsz = 400;
Is this code means similar to what you've said?
AFAIK, 'static' keyword for global variable restricts scope to file itself,
so the 'accessing' and 'modifying' this variable from the <_user.c>
isn't available.
The reason why I've used bpf map for this 'pcktsz' option is,
I've wanted to run this kernel xdp program (xdp_adjust_tail_kern.o)
as it itself, not heavily controlled by user program (./xdp_adjust_tail).
When this 'pcktsz' option is implemented in bpf map, user can simply
modify 'map' to change this size. (such as bpftool prog map)
But when this variable comes to global data, it can't be changed
after the program gets loaded.
I really appreciate your time and effort for the review.
But I'm sorry that I seem to get it wrong.
Thanks,
Daniel
> >
> > By the way, thanks for the review.
> >
> > Thanks,
> > Daniel
> >
> >
> > > > struct bpf_map_def SEC("maps") icmpcnt = {
> > > > .type = BPF_MAP_TYPE_ARRAY,
> > > > .key_size = sizeof(__u32),
> > > > @@ -64,7 +71,8 @@ static __always_inline void ipv4_csum(void *data_start, int data_size,
> > > > *csum = csum_fold_helper(*csum);
> > > > }
> > > >
> > >
> > > [...]
^ permalink raw reply
* [PATCH v8 1/7] nfc: pn533: i2c: "pn532" as dt compatible string
From: Lars Poeschel @ 2019-09-19 9:16 UTC (permalink / raw)
To: Steve Winslow, Thomas Gleixner, Kate Stewart, Allison Randal,
Jilayne Lovejoy, Lars Poeschel, open list:NFC SUBSYSTEM,
open list
Cc: Johan Hovold, Simon Horman
It is favourable to have one unified compatible string for devices that
have multiple interfaces. So this adds simply "pn532" as the devicetree
binding compatible string and makes a note that the old ones are
deprecated.
Cc: Johan Hovold <johan@kernel.org>
Cc: Simon Horman <horms@verge.net.au>
Signed-off-by: Lars Poeschel <poeschel@lemonage.de>
---
Changes in v6:
- Rebased the patch series on v5.3-rc5
Changes in v3:
- This patch is new in v3
drivers/nfc/pn533/i2c.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/nfc/pn533/i2c.c b/drivers/nfc/pn533/i2c.c
index 1832cd921ea7..1abd40398a5a 100644
--- a/drivers/nfc/pn533/i2c.c
+++ b/drivers/nfc/pn533/i2c.c
@@ -245,6 +245,11 @@ static int pn533_i2c_remove(struct i2c_client *client)
}
static const struct of_device_id of_pn533_i2c_match[] = {
+ { .compatible = "nxp,pn532", },
+ /*
+ * NOTE: The use of the compatibles with the trailing "...-i2c" is
+ * deprecated and will be removed.
+ */
{ .compatible = "nxp,pn533-i2c", },
{ .compatible = "nxp,pn532-i2c", },
{},
--
2.23.0
^ permalink raw reply related
* [PATCH v8 2/7] nfc: pn532: Add uart phy docs and rename it
From: Lars Poeschel @ 2019-09-19 9:16 UTC (permalink / raw)
To: David S. Miller, Rob Herring, Mark Rutland,
GitAuthor: Lars Poeschel, open list:NFC SUBSYSTEM,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
open list
Cc: Johan Hovold, Simon Horman
In-Reply-To: <20190919091645.16439-1-poeschel@lemonage.de>
This adds documentation about the uart phy to the pn532 binding doc. As
the filename "pn533-i2c.txt" is not appropriate any more, rename it to
the more general "pn532.txt".
This also documents the deprecation of the compatible strings ending
with "...-i2c".
Cc: Johan Hovold <johan@kernel.org>
Cc: Simon Horman <horms@verge.net.au>
Signed-off-by: Lars Poeschel <poeschel@lemonage.de>
---
Changes in v8:
- Update existing binding doc instead of adding a new one:
- Add uart phy example
- Add general "pn532" compatible string
- Deprecate "...-i2c" compatible strings
- Rename file to a more general filename
- Intentionally drop Rob's Reviewed-By as I guess this rather big change
requires a new review
Changes in v7:
- Accidentally lost Rob's Reviewed-By
Changes in v6:
- Rebased the patch series on v5.3-rc5
- Picked up Rob's Reviewed-By
Changes in v4:
- Add documentation about reg property in case of i2c
Changes in v3:
- seperate binding doc instead of entry in trivial-devices.txt
.../devicetree/bindings/net/nfc/pn532.txt | 46 +++++++++++++++++++
.../devicetree/bindings/net/nfc/pn533-i2c.txt | 29 ------------
2 files changed, 46 insertions(+), 29 deletions(-)
create mode 100644 Documentation/devicetree/bindings/net/nfc/pn532.txt
delete mode 100644 Documentation/devicetree/bindings/net/nfc/pn533-i2c.txt
diff --git a/Documentation/devicetree/bindings/net/nfc/pn532.txt b/Documentation/devicetree/bindings/net/nfc/pn532.txt
new file mode 100644
index 000000000000..f0591f160bee
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/nfc/pn532.txt
@@ -0,0 +1,46 @@
+* NXP Semiconductors PN532 NFC Controller
+
+Required properties:
+- compatible: Should be
+ - "nxp,pn532" Place a node with this inside the devicetree node of the bus
+ where the NFC chip is connected to.
+ Currently the kernel has phy bindings for uart and i2c.
+ - "nxp,pn532-i2c" (DEPRECATED) only works for the i2c binding.
+ - "nxp,pn533-i2c" (DEPRECATED) only works for the i2c binding.
+
+Required properties if connected on i2c:
+- clock-frequency: I²C work frequency.
+- reg: for the I²C bus address. This is fixed at 0x24 for the PN532.
+- interrupts: GPIO interrupt to which the chip is connected
+
+Optional SoC Specific Properties:
+- pinctrl-names: Contains only one value - "default".
+- pintctrl-0: Specifies the pin control groups used for this controller.
+
+Example (for ARM-based BeagleBone with PN532 on I2C2):
+
+&i2c2 {
+
+
+ pn532: pn532@24 {
+
+ compatible = "nxp,pn532";
+
+ reg = <0x24>;
+ clock-frequency = <400000>;
+
+ interrupt-parent = <&gpio1>;
+ interrupts = <17 IRQ_TYPE_EDGE_FALLING>;
+
+ };
+};
+
+Example (for PN532 connected via uart):
+
+uart4: serial@49042000 {
+ compatible = "ti,omap3-uart";
+
+ pn532: nfc {
+ compatible = "nxp,pn532";
+ };
+};
diff --git a/Documentation/devicetree/bindings/net/nfc/pn533-i2c.txt b/Documentation/devicetree/bindings/net/nfc/pn533-i2c.txt
deleted file mode 100644
index 2efe3886b95b..000000000000
--- a/Documentation/devicetree/bindings/net/nfc/pn533-i2c.txt
+++ /dev/null
@@ -1,29 +0,0 @@
-* NXP Semiconductors PN532 NFC Controller
-
-Required properties:
-- compatible: Should be "nxp,pn532-i2c" or "nxp,pn533-i2c".
-- clock-frequency: I²C work frequency.
-- reg: address on the bus
-- interrupts: GPIO interrupt to which the chip is connected
-
-Optional SoC Specific Properties:
-- pinctrl-names: Contains only one value - "default".
-- pintctrl-0: Specifies the pin control groups used for this controller.
-
-Example (for ARM-based BeagleBone with PN532 on I2C2):
-
-&i2c2 {
-
-
- pn532: pn532@24 {
-
- compatible = "nxp,pn532-i2c";
-
- reg = <0x24>;
- clock-frequency = <400000>;
-
- interrupt-parent = <&gpio1>;
- interrupts = <17 IRQ_TYPE_EDGE_FALLING>;
-
- };
-};
--
2.23.0
^ permalink raw reply related
* [PATCH v8 3/7] nfc: pn533: Add dev_up/dev_down hooks to phy_ops
From: Lars Poeschel @ 2019-09-19 9:16 UTC (permalink / raw)
To: Thomas Gleixner, Kees Cook, Greg Kroah-Hartman, Allison Randal,
Jilayne Lovejoy, Lars Poeschel, Gustavo A. R. Silva,
Steve Winslow, open list:NFC SUBSYSTEM, open list
Cc: Johan Hovold
In-Reply-To: <20190919091645.16439-1-poeschel@lemonage.de>
This adds hooks for dev_up and dev_down to the phy_ops. They are
optional.
The idea is to inform the phy driver when the nfc chip is really going
to be used. When it is not used, the phy driver can suspend it's
interface to the nfc chip to save some power. The nfc chip is considered
not in use before dev_up and after dev_down.
Cc: Johan Hovold <johan@kernel.org>
Signed-off-by: Lars Poeschel <poeschel@lemonage.de>
---
Changes in v6:
- Rebased the patch series on v5.3-rc5
Changes in v5:
- (dev->phy_ops->dev_up) instead (dev->phy_ops)
Changes in v4:
- This patch is new in v4
drivers/nfc/pn533/pn533.c | 12 +++++++++++-
drivers/nfc/pn533/pn533.h | 9 +++++++++
2 files changed, 20 insertions(+), 1 deletion(-)
diff --git a/drivers/nfc/pn533/pn533.c b/drivers/nfc/pn533/pn533.c
index a172a32aa9d9..64836c727aee 100644
--- a/drivers/nfc/pn533/pn533.c
+++ b/drivers/nfc/pn533/pn533.c
@@ -2458,6 +2458,9 @@ static int pn533_dev_up(struct nfc_dev *nfc_dev)
{
struct pn533 *dev = nfc_get_drvdata(nfc_dev);
+ if (dev->phy_ops->dev_up)
+ dev->phy_ops->dev_up(dev);
+
if (dev->device_type == PN533_DEVICE_PN532) {
int rc = pn532_sam_configuration(nfc_dev);
@@ -2470,7 +2473,14 @@ static int pn533_dev_up(struct nfc_dev *nfc_dev)
static int pn533_dev_down(struct nfc_dev *nfc_dev)
{
- return pn533_rf_field(nfc_dev, 0);
+ struct pn533 *dev = nfc_get_drvdata(nfc_dev);
+ int ret;
+
+ ret = pn533_rf_field(nfc_dev, 0);
+ if (dev->phy_ops->dev_down && !ret)
+ dev->phy_ops->dev_down(dev);
+
+ return ret;
}
static struct nfc_ops pn533_nfc_ops = {
diff --git a/drivers/nfc/pn533/pn533.h b/drivers/nfc/pn533/pn533.h
index 8bf9d6ece0f5..570ee0a3e832 100644
--- a/drivers/nfc/pn533/pn533.h
+++ b/drivers/nfc/pn533/pn533.h
@@ -207,6 +207,15 @@ struct pn533_phy_ops {
struct sk_buff *out);
int (*send_ack)(struct pn533 *dev, gfp_t flags);
void (*abort_cmd)(struct pn533 *priv, gfp_t flags);
+ /*
+ * dev_up and dev_down are optional.
+ * They are used to inform the phy layer that the nfc chip
+ * is going to be really used very soon. The phy layer can then
+ * bring up it's interface to the chip and have it suspended for power
+ * saving reasons otherwise.
+ */
+ void (*dev_up)(struct pn533 *priv);
+ void (*dev_down)(struct pn533 *priv);
};
--
2.23.0
^ permalink raw reply related
* [PATCH v8 4/7] nfc: pn533: Split pn533 init & nfc_register
From: Lars Poeschel @ 2019-09-19 9:16 UTC (permalink / raw)
To: Thomas Gleixner, Jilayne Lovejoy, Kate Stewart, Steve Winslow,
Allison Randal, Lars Poeschel, Greg Kroah-Hartman,
Gustavo A. R. Silva, open list:NFC SUBSYSTEM, open list
Cc: Johan Hovold, Claudiu Beznea
In-Reply-To: <20190919091645.16439-1-poeschel@lemonage.de>
There is a problem in the initialisation and setup of the pn533: It
registers with nfc too early. It could happen, that it finished
registering with nfc and someone starts using it. But setup of the pn533
is not yet finished. Bad or at least unintended things could happen.
So I split out nfc registering (and unregistering) to seperate functions
that have to be called late in probe then.
Cc: Johan Hovold <johan@kernel.org>
Cc: Claudiu Beznea <Claudiu.Beznea@microchip.com>
Signed-off-by: Lars Poeschel <poeschel@lemonage.de>
---
Changes in v7:
- Remove an unneeded rc variable initialization
- Corrected goto error to err_clean in pn533_usb_probe
Changes in v6:
- Rebased the patch series on v5.3-rc5
Changes in v5:
- This patch is new in v5
drivers/nfc/pn533/i2c.c | 17 +++++-----
drivers/nfc/pn533/pn533.c | 66 ++++++++++++++++++++-------------------
drivers/nfc/pn533/pn533.h | 11 ++++---
drivers/nfc/pn533/usb.c | 14 ++++++---
4 files changed, 60 insertions(+), 48 deletions(-)
diff --git a/drivers/nfc/pn533/i2c.c b/drivers/nfc/pn533/i2c.c
index 1abd40398a5a..e9e5a1ec8857 100644
--- a/drivers/nfc/pn533/i2c.c
+++ b/drivers/nfc/pn533/i2c.c
@@ -193,12 +193,10 @@ static int pn533_i2c_probe(struct i2c_client *client,
phy->i2c_dev = client;
i2c_set_clientdata(client, phy);
- priv = pn533_register_device(PN533_DEVICE_PN532,
- PN533_NO_TYPE_B_PROTOCOLS,
+ priv = pn53x_common_init(PN533_DEVICE_PN532,
PN533_PROTO_REQ_ACK_RESP,
phy, &i2c_phy_ops, NULL,
- &phy->i2c_dev->dev,
- &client->dev);
+ &phy->i2c_dev->dev);
if (IS_ERR(priv)) {
r = PTR_ERR(priv);
@@ -220,13 +218,17 @@ static int pn533_i2c_probe(struct i2c_client *client,
if (r)
goto fn_setup_err;
- return 0;
+ r = pn53x_register_nfc(priv, PN533_NO_TYPE_B_PROTOCOLS, &client->dev);
+ if (r)
+ goto fn_setup_err;
+
+ return r;
fn_setup_err:
free_irq(client->irq, phy);
irq_rqst_err:
- pn533_unregister_device(phy->priv);
+ pn53x_common_clean(phy->priv);
return r;
}
@@ -239,7 +241,8 @@ static int pn533_i2c_remove(struct i2c_client *client)
free_irq(client->irq, phy);
- pn533_unregister_device(phy->priv);
+ pn53x_unregister_nfc(phy->priv);
+ pn53x_common_clean(phy->priv);
return 0;
}
diff --git a/drivers/nfc/pn533/pn533.c b/drivers/nfc/pn533/pn533.c
index 64836c727aee..e5d5e4c83a04 100644
--- a/drivers/nfc/pn533/pn533.c
+++ b/drivers/nfc/pn533/pn533.c
@@ -2590,14 +2590,12 @@ int pn533_finalize_setup(struct pn533 *dev)
}
EXPORT_SYMBOL_GPL(pn533_finalize_setup);
-struct pn533 *pn533_register_device(u32 device_type,
- u32 protocols,
+struct pn533 *pn53x_common_init(u32 device_type,
enum pn533_protocol_type protocol_type,
void *phy,
struct pn533_phy_ops *phy_ops,
struct pn533_frame_ops *fops,
- struct device *dev,
- struct device *parent)
+ struct device *dev)
{
struct pn533 *priv;
int rc = -ENOMEM;
@@ -2638,43 +2636,18 @@ struct pn533 *pn533_register_device(u32 device_type,
skb_queue_head_init(&priv->fragment_skb);
INIT_LIST_HEAD(&priv->cmd_queue);
-
- priv->nfc_dev = nfc_allocate_device(&pn533_nfc_ops, protocols,
- priv->ops->tx_header_len +
- PN533_CMD_DATAEXCH_HEAD_LEN,
- priv->ops->tx_tail_len);
- if (!priv->nfc_dev) {
- rc = -ENOMEM;
- goto destroy_wq;
- }
-
- nfc_set_parent_dev(priv->nfc_dev, parent);
- nfc_set_drvdata(priv->nfc_dev, priv);
-
- rc = nfc_register_device(priv->nfc_dev);
- if (rc)
- goto free_nfc_dev;
-
return priv;
-free_nfc_dev:
- nfc_free_device(priv->nfc_dev);
-
-destroy_wq:
- destroy_workqueue(priv->wq);
error:
kfree(priv);
return ERR_PTR(rc);
}
-EXPORT_SYMBOL_GPL(pn533_register_device);
+EXPORT_SYMBOL_GPL(pn53x_common_init);
-void pn533_unregister_device(struct pn533 *priv)
+void pn53x_common_clean(struct pn533 *priv)
{
struct pn533_cmd *cmd, *n;
- nfc_unregister_device(priv->nfc_dev);
- nfc_free_device(priv->nfc_dev);
-
flush_delayed_work(&priv->poll_work);
destroy_workqueue(priv->wq);
@@ -2689,8 +2662,37 @@ void pn533_unregister_device(struct pn533 *priv)
kfree(priv);
}
-EXPORT_SYMBOL_GPL(pn533_unregister_device);
+EXPORT_SYMBOL_GPL(pn53x_common_clean);
+
+int pn53x_register_nfc(struct pn533 *priv, u32 protocols,
+ struct device *parent)
+{
+ int rc;
+
+ priv->nfc_dev = nfc_allocate_device(&pn533_nfc_ops, protocols,
+ priv->ops->tx_header_len +
+ PN533_CMD_DATAEXCH_HEAD_LEN,
+ priv->ops->tx_tail_len);
+ if (!priv->nfc_dev)
+ return -ENOMEM;
+
+ nfc_set_parent_dev(priv->nfc_dev, parent);
+ nfc_set_drvdata(priv->nfc_dev, priv);
+
+ rc = nfc_register_device(priv->nfc_dev);
+ if (rc)
+ nfc_free_device(priv->nfc_dev);
+
+ return rc;
+}
+EXPORT_SYMBOL_GPL(pn53x_register_nfc);
+void pn53x_unregister_nfc(struct pn533 *priv)
+{
+ nfc_unregister_device(priv->nfc_dev);
+ nfc_free_device(priv->nfc_dev);
+}
+EXPORT_SYMBOL_GPL(pn53x_unregister_nfc);
MODULE_AUTHOR("Lauro Ramos Venancio <lauro.venancio@openbossa.org>");
MODULE_AUTHOR("Aloisio Almeida Jr <aloisio.almeida@openbossa.org>");
diff --git a/drivers/nfc/pn533/pn533.h b/drivers/nfc/pn533/pn533.h
index 570ee0a3e832..510ddebbd896 100644
--- a/drivers/nfc/pn533/pn533.h
+++ b/drivers/nfc/pn533/pn533.h
@@ -219,18 +219,19 @@ struct pn533_phy_ops {
};
-struct pn533 *pn533_register_device(u32 device_type,
- u32 protocols,
+struct pn533 *pn53x_common_init(u32 device_type,
enum pn533_protocol_type protocol_type,
void *phy,
struct pn533_phy_ops *phy_ops,
struct pn533_frame_ops *fops,
- struct device *dev,
- struct device *parent);
+ struct device *dev);
int pn533_finalize_setup(struct pn533 *dev);
-void pn533_unregister_device(struct pn533 *priv);
+void pn53x_common_clean(struct pn533 *priv);
void pn533_recv_frame(struct pn533 *dev, struct sk_buff *skb, int status);
+int pn53x_register_nfc(struct pn533 *priv, u32 protocols,
+ struct device *parent);
+void pn53x_unregister_nfc(struct pn533 *priv);
bool pn533_rx_frame_is_cmd_response(struct pn533 *dev, void *frame);
bool pn533_rx_frame_is_ack(void *_frame);
diff --git a/drivers/nfc/pn533/usb.c b/drivers/nfc/pn533/usb.c
index c5289eaf17ee..8511fd089970 100644
--- a/drivers/nfc/pn533/usb.c
+++ b/drivers/nfc/pn533/usb.c
@@ -534,9 +534,9 @@ static int pn533_usb_probe(struct usb_interface *interface,
goto error;
}
- priv = pn533_register_device(id->driver_info, protocols, protocol_type,
+ priv = pn53x_common_init(id->driver_info, protocol_type,
phy, &usb_phy_ops, fops,
- &phy->udev->dev, &interface->dev);
+ &phy->udev->dev);
if (IS_ERR(priv)) {
rc = PTR_ERR(priv);
@@ -547,12 +547,17 @@ static int pn533_usb_probe(struct usb_interface *interface,
rc = pn533_finalize_setup(priv);
if (rc)
- goto error;
+ goto err_clean;
usb_set_intfdata(interface, phy);
+ rc = pn53x_register_nfc(priv, protocols, &interface->dev);
+ if (rc)
+ goto err_clean;
return 0;
+err_clean:
+ pn53x_common_clean(priv);
error:
usb_free_urb(phy->in_urb);
usb_free_urb(phy->out_urb);
@@ -570,7 +575,8 @@ static void pn533_usb_disconnect(struct usb_interface *interface)
if (!phy)
return;
- pn533_unregister_device(phy->priv);
+ pn53x_unregister_nfc(phy->priv);
+ pn53x_common_clean(phy->priv);
usb_set_intfdata(interface, NULL);
--
2.23.0
^ permalink raw reply related
* [PATCH v8 5/7] nfc: pn533: add UART phy driver
From: Lars Poeschel @ 2019-09-19 9:16 UTC (permalink / raw)
To: Thomas Gleixner, Greg Kroah-Hartman, Lars Poeschel,
Jilayne Lovejoy, Kate Stewart, Allison Randal, Steve Winslow,
open list, open list:NFC SUBSYSTEM
Cc: Johan Hovold, Claudiu Beznea, David Miller
In-Reply-To: <20190919091645.16439-1-poeschel@lemonage.de>
This adds the UART phy interface for the pn533 driver.
The pn533 driver can be used through UART interface this way.
It is implemented as a serdev device.
Cc: Johan Hovold <johan@kernel.org>
Cc: Claudiu Beznea <Claudiu.Beznea@microchip.com>
Cc: David Miller <davem@davemloft.net>
Signed-off-by: Lars Poeschel <poeschel@lemonage.de>
---
Changes in v8:
- Reverse christmas tree order for local variables in
pn532_uart_send_ack and pn532_uart_rx_is_frame
Changes in v7:
- Add comment at send_wakeup variable to document a possible and
harmless concurrency issue
Changes in v6:
- Rebased the patch series on v5.3-rc5
Changes in v5:
- Use the splitted pn53x_common_init and pn53x_register_nfc
and pn53x_common_clean and pn53x_unregister_nfc alike
Changes in v4:
- SPDX-License-Identifier: GPL-2.0+
- Source code comments above refering items
- Error check for serdev_device_write's
- Change if (xxx == NULL) to if (!xxx)
- Remove device name from a dev_err
- move pn533_register in _probe a bit towards the end of _probe
- make use of newly added dev_up / dev_down phy_ops
- control send_wakeup variable from dev_up / dev_down
Changes in v3:
- depend on SERIAL_DEV_BUS in Kconfig
Changes in v2:
- switched from tty line discipline to serdev, resulting in many
simplifications
- SPDX License Identifier
drivers/nfc/pn533/Kconfig | 11 ++
drivers/nfc/pn533/Makefile | 2 +
drivers/nfc/pn533/pn533.h | 8 +
drivers/nfc/pn533/uart.c | 324 +++++++++++++++++++++++++++++++++++++
4 files changed, 345 insertions(+)
create mode 100644 drivers/nfc/pn533/uart.c
diff --git a/drivers/nfc/pn533/Kconfig b/drivers/nfc/pn533/Kconfig
index f6d6b345ba0d..7fe1bbe26568 100644
--- a/drivers/nfc/pn533/Kconfig
+++ b/drivers/nfc/pn533/Kconfig
@@ -26,3 +26,14 @@ config NFC_PN533_I2C
If you choose to build a module, it'll be called pn533_i2c.
Say N if unsure.
+
+config NFC_PN532_UART
+ tristate "NFC PN532 device support (UART)"
+ depends on SERIAL_DEV_BUS
+ select NFC_PN533
+ ---help---
+ This module adds support for the NXP pn532 UART interface.
+ Select this if your platform is using the UART bus.
+
+ If you choose to build a module, it'll be called pn532_uart.
+ Say N if unsure.
diff --git a/drivers/nfc/pn533/Makefile b/drivers/nfc/pn533/Makefile
index 43c25b4f9466..b9648337576f 100644
--- a/drivers/nfc/pn533/Makefile
+++ b/drivers/nfc/pn533/Makefile
@@ -4,7 +4,9 @@
#
pn533_usb-objs = usb.o
pn533_i2c-objs = i2c.o
+pn532_uart-objs = uart.o
obj-$(CONFIG_NFC_PN533) += pn533.o
obj-$(CONFIG_NFC_PN533_USB) += pn533_usb.o
obj-$(CONFIG_NFC_PN533_I2C) += pn533_i2c.o
+obj-$(CONFIG_NFC_PN532_UART) += pn532_uart.o
diff --git a/drivers/nfc/pn533/pn533.h b/drivers/nfc/pn533/pn533.h
index 510ddebbd896..6541088fad73 100644
--- a/drivers/nfc/pn533/pn533.h
+++ b/drivers/nfc/pn533/pn533.h
@@ -43,6 +43,11 @@
/* Preamble (1), SoPC (2), ACK Code (2), Postamble (1) */
#define PN533_STD_FRAME_ACK_SIZE 6
+/*
+ * Preamble (1), SoPC (2), Packet Length (1), Packet Length Checksum (1),
+ * Specific Application Level Error Code (1) , Postamble (1)
+ */
+#define PN533_STD_ERROR_FRAME_SIZE 8
#define PN533_STD_FRAME_CHECKSUM(f) (f->data[f->datalen])
#define PN533_STD_FRAME_POSTAMBLE(f) (f->data[f->datalen + 1])
/* Half start code (3), LEN (4) should be 0xffff for extended frame */
@@ -84,6 +89,9 @@
#define PN533_CMD_MI_MASK 0x40
#define PN533_CMD_RET_SUCCESS 0x00
+#define PN533_FRAME_DATALEN_ACK 0x00
+#define PN533_FRAME_DATALEN_ERROR 0x01
+#define PN533_FRAME_DATALEN_EXTENDED 0xFF
enum pn533_protocol_type {
PN533_PROTO_REQ_ACK_RESP = 0,
diff --git a/drivers/nfc/pn533/uart.c b/drivers/nfc/pn533/uart.c
new file mode 100644
index 000000000000..7f639051cdd0
--- /dev/null
+++ b/drivers/nfc/pn533/uart.c
@@ -0,0 +1,324 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Driver for NXP PN532 NFC Chip - UART transport layer
+ *
+ * Copyright (C) 2018 Lemonage Software GmbH
+ * Author: Lars Pöschel <poeschel@lemonage.de>
+ * All rights reserved.
+ */
+
+#include <linux/device.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/nfc.h>
+#include <linux/netdevice.h>
+#include <linux/of.h>
+#include <linux/serdev.h>
+#include "pn533.h"
+
+#define PN532_UART_SKB_BUFF_LEN (PN533_CMD_DATAEXCH_DATA_MAXLEN * 2)
+
+enum send_wakeup {
+ PN532_SEND_NO_WAKEUP = 0,
+ PN532_SEND_WAKEUP,
+ PN532_SEND_LAST_WAKEUP,
+};
+
+
+struct pn532_uart_phy {
+ struct serdev_device *serdev;
+ struct sk_buff *recv_skb;
+ struct pn533 *priv;
+ /*
+ * send_wakeup variable is used to control if we need to send a wakeup
+ * request to the pn532 chip prior to our actual command. There is a
+ * little propability of a race condition. We decided to not mutex the
+ * variable as the worst that could happen is, that we send a wakeup
+ * to the chip that is already awake. This does not hurt. It is a
+ * no-op to the chip.
+ */
+ enum send_wakeup send_wakeup;
+ struct timer_list cmd_timeout;
+ struct sk_buff *cur_out_buf;
+};
+
+static int pn532_uart_send_frame(struct pn533 *dev,
+ struct sk_buff *out)
+{
+ /* wakeup sequence and dummy bytes for waiting time */
+ static const u8 wakeup[] = {
+ 0x55, 0x55, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00};
+ struct pn532_uart_phy *pn532 = dev->phy;
+ int err;
+
+ print_hex_dump_debug("PN532_uart TX: ", DUMP_PREFIX_NONE, 16, 1,
+ out->data, out->len, false);
+
+ pn532->cur_out_buf = out;
+ if (pn532->send_wakeup) {
+ err= serdev_device_write(pn532->serdev,
+ wakeup, sizeof(wakeup),
+ MAX_SCHEDULE_TIMEOUT);
+ if (err < 0)
+ return err;
+ }
+
+ if (pn532->send_wakeup == PN532_SEND_LAST_WAKEUP) {
+ pn532->send_wakeup = PN532_SEND_NO_WAKEUP;
+ }
+
+ err = serdev_device_write(pn532->serdev, out->data, out->len,
+ MAX_SCHEDULE_TIMEOUT);
+ if (err < 0)
+ return err;
+
+ mod_timer(&pn532->cmd_timeout, HZ / 40 + jiffies);
+ return 0;
+}
+
+static int pn532_uart_send_ack(struct pn533 *dev, gfp_t flags)
+{
+ /* spec 7.1.1.3: Preamble, SoPC (2), ACK Code (2), Postamble */
+ static const u8 ack[PN533_STD_FRAME_ACK_SIZE] = {
+ 0x00, 0x00, 0xff, 0x00, 0xff, 0x00};
+ struct pn532_uart_phy *pn532 = dev->phy;
+ int err;
+
+ err = serdev_device_write(pn532->serdev, ack, sizeof(ack),
+ MAX_SCHEDULE_TIMEOUT);
+ if (err < 0)
+ return err;
+
+ return 0;
+}
+
+static void pn532_uart_abort_cmd(struct pn533 *dev, gfp_t flags)
+{
+ /* An ack will cancel the last issued command */
+ pn532_uart_send_ack(dev, flags);
+ /* schedule cmd_complete_work to finish current command execution */
+ pn533_recv_frame(dev, NULL, -ENOENT);
+}
+
+static void pn532_dev_up(struct pn533 *dev)
+{
+ struct pn532_uart_phy *pn532 = dev->phy;
+
+ serdev_device_open(pn532->serdev);
+ pn532->send_wakeup = PN532_SEND_LAST_WAKEUP;
+}
+
+static void pn532_dev_down(struct pn533 *dev)
+{
+ struct pn532_uart_phy *pn532 = dev->phy;
+
+ serdev_device_close(pn532->serdev);
+ pn532->send_wakeup = PN532_SEND_WAKEUP;
+}
+
+static struct pn533_phy_ops uart_phy_ops = {
+ .send_frame = pn532_uart_send_frame,
+ .send_ack = pn532_uart_send_ack,
+ .abort_cmd = pn532_uart_abort_cmd,
+ .dev_up = pn532_dev_up,
+ .dev_down = pn532_dev_down,
+};
+
+static void pn532_cmd_timeout(struct timer_list *t)
+{
+ struct pn532_uart_phy *dev = from_timer(dev, t, cmd_timeout);
+
+ pn532_uart_send_frame(dev->priv, dev->cur_out_buf);
+}
+
+/*
+ * scans the buffer if it contains a pn532 frame. It is not checked if the
+ * frame is really valid. This is later done with pn533_rx_frame_is_valid.
+ * This is useful for malformed or errornous transmitted frames. Adjusts the
+ * bufferposition where the frame starts, since pn533_recv_frame expects a
+ * well formed frame.
+ */
+static int pn532_uart_rx_is_frame(struct sk_buff *skb)
+{
+ struct pn533_std_frame *std;
+ struct pn533_ext_frame *ext;
+ u16 frame_len;
+ int i;
+
+ for (i = 0; i + PN533_STD_FRAME_ACK_SIZE <= skb->len; i++) {
+ std = (struct pn533_std_frame *)&skb->data[i];
+ /* search start code */
+ if (std->start_frame != cpu_to_be16(PN533_STD_FRAME_SOF))
+ continue;
+
+ /* frame type */
+ switch (std->datalen) {
+ case PN533_FRAME_DATALEN_ACK:
+ if (std->datalen_checksum == 0xff) {
+ skb_pull(skb, i);
+ return 1;
+ }
+
+ break;
+ case PN533_FRAME_DATALEN_ERROR:
+ if ((std->datalen_checksum == 0xff) &&
+ (skb->len >=
+ PN533_STD_ERROR_FRAME_SIZE)) {
+ skb_pull(skb, i);
+ return 1;
+ }
+
+ break;
+ case PN533_FRAME_DATALEN_EXTENDED:
+ ext = (struct pn533_ext_frame *)&skb->data[i];
+ frame_len = ext->datalen;
+ if (skb->len >= frame_len +
+ sizeof(struct pn533_ext_frame) +
+ 2 /* CKS + Postamble */) {
+ skb_pull(skb, i);
+ return 1;
+ }
+
+ break;
+ default: /* normal information frame */
+ frame_len = std->datalen;
+ if (skb->len >= frame_len +
+ sizeof(struct pn533_std_frame) +
+ 2 /* CKS + Postamble */) {
+ skb_pull(skb, i);
+ return 1;
+ }
+
+ break;
+ }
+ }
+
+ return 0;
+}
+
+static int pn532_receive_buf(struct serdev_device *serdev,
+ const unsigned char *data, size_t count)
+{
+ struct pn532_uart_phy *dev = serdev_device_get_drvdata(serdev);
+ size_t i;
+
+ del_timer(&dev->cmd_timeout);
+ for (i = 0; i < count; i++) {
+ skb_put_u8(dev->recv_skb, *data++);
+ if (!pn532_uart_rx_is_frame(dev->recv_skb))
+ continue;
+
+ pn533_recv_frame(dev->priv, dev->recv_skb, 0);
+ dev->recv_skb = alloc_skb(PN532_UART_SKB_BUFF_LEN, GFP_KERNEL);
+ if (!dev->recv_skb)
+ return 0;
+ }
+
+ return i;
+}
+
+static struct serdev_device_ops pn532_serdev_ops = {
+ .receive_buf = pn532_receive_buf,
+ .write_wakeup = serdev_device_write_wakeup,
+};
+
+static const struct of_device_id pn532_uart_of_match[] = {
+ { .compatible = "nxp,pn532", },
+ {},
+};
+MODULE_DEVICE_TABLE(of, pn532_uart_of_match);
+
+static int pn532_uart_probe(struct serdev_device *serdev)
+{
+ struct pn532_uart_phy *pn532;
+ struct pn533 *priv;
+ int err;
+
+ err = -ENOMEM;
+ pn532 = kzalloc(sizeof(*pn532), GFP_KERNEL);
+ if (!pn532)
+ goto err_exit;
+
+ pn532->recv_skb = alloc_skb(PN532_UART_SKB_BUFF_LEN, GFP_KERNEL);
+ if (!pn532->recv_skb)
+ goto err_free;
+
+ pn532->serdev = serdev;
+ serdev_device_set_drvdata(serdev, pn532);
+ serdev_device_set_client_ops(serdev, &pn532_serdev_ops);
+ err = serdev_device_open(serdev);
+ if (err) {
+ dev_err(&serdev->dev, "Unable to open device\n");
+ goto err_skb;
+ }
+
+ err = serdev_device_set_baudrate(serdev, 115200);
+ if (err != 115200) {
+ err = -EINVAL;
+ goto err_serdev;
+ }
+
+ serdev_device_set_flow_control(serdev, false);
+ pn532->send_wakeup = PN532_SEND_WAKEUP;
+ timer_setup(&pn532->cmd_timeout, pn532_cmd_timeout, 0);
+ priv = pn53x_common_init(PN533_DEVICE_PN532,
+ PN533_PROTO_REQ_ACK_RESP,
+ pn532, &uart_phy_ops, NULL,
+ &pn532->serdev->dev);
+ if (IS_ERR(priv)) {
+ err = PTR_ERR(priv);
+ goto err_serdev;
+ }
+
+ pn532->priv = priv;
+ err = pn533_finalize_setup(pn532->priv);
+ if (err)
+ goto err_clean;
+
+ serdev_device_close(serdev);
+ err = pn53x_register_nfc(priv, PN533_NO_TYPE_B_PROTOCOLS, &serdev->dev);
+ if (err) {
+ pn53x_common_clean(pn532->priv);
+ goto err_skb;
+ }
+
+ return err;
+
+err_clean:
+ pn53x_common_clean(pn532->priv);
+err_serdev:
+ serdev_device_close(serdev);
+err_skb:
+ kfree_skb(pn532->recv_skb);
+err_free:
+ kfree(pn532);
+err_exit:
+ return err;
+}
+
+static void pn532_uart_remove(struct serdev_device *serdev)
+{
+ struct pn532_uart_phy *pn532 = serdev_device_get_drvdata(serdev);
+
+ pn53x_unregister_nfc(pn532->priv);
+ serdev_device_close(serdev);
+ pn53x_common_clean(pn532->priv);
+ kfree_skb(pn532->recv_skb);
+ kfree(pn532);
+}
+
+static struct serdev_device_driver pn532_uart_driver = {
+ .probe = pn532_uart_probe,
+ .remove = pn532_uart_remove,
+ .driver = {
+ .name = "pn532_uart",
+ .of_match_table = of_match_ptr(pn532_uart_of_match),
+ },
+};
+
+module_serdev_device_driver(pn532_uart_driver);
+
+MODULE_AUTHOR("Lars Pöschel <poeschel@lemonage.de>");
+MODULE_DESCRIPTION("PN532 UART driver");
+MODULE_LICENSE("GPL");
--
2.23.0
^ permalink raw reply related
* [PATCH v8 6/7] nfc: pn533: Add autopoll capability
From: Lars Poeschel @ 2019-09-19 9:16 UTC (permalink / raw)
To: Jilayne Lovejoy, Gustavo A. R. Silva, Kate Stewart, Steve Winslow,
Kees Cook, Lars Poeschel, Thomas Gleixner, Allison Randal,
Greg Kroah-Hartman, open list:NFC SUBSYSTEM, open list
Cc: Johan Hovold, David Miller
In-Reply-To: <20190919091645.16439-1-poeschel@lemonage.de>
pn532 devices support an autopoll command, that lets the chip
automatically poll for selected nfc technologies instead of manually
looping through every single nfc technology the user is interested in.
This is faster and less cpu and bus intensive than manually polling.
This adds this autopoll capability to the pn533 driver.
Cc: Johan Hovold <johan@kernel.org>
Cc: David Miller <davem@davemloft.net>
Signed-off-by: Lars Poeschel <poeschel@lemonage.de>
---
Changes in v8:
- Reverse christmas tree order for local variables in
pn533_autopoll_complete and pn533_start_poll
Changes in v7:
- Remove __packed attribute at struct pn532_autopoll_resp
- Add missing '\n' at the end of dev_dbg and nfc_err strings
Changes in v6:
- Rebased the patch series on v5.3-rc5
drivers/nfc/pn533/pn533.c | 193 +++++++++++++++++++++++++++++++++++++-
drivers/nfc/pn533/pn533.h | 10 +-
2 files changed, 197 insertions(+), 6 deletions(-)
diff --git a/drivers/nfc/pn533/pn533.c b/drivers/nfc/pn533/pn533.c
index e5d5e4c83a04..044b52d036e3 100644
--- a/drivers/nfc/pn533/pn533.c
+++ b/drivers/nfc/pn533/pn533.c
@@ -185,6 +185,32 @@ struct pn533_cmd_jump_dep_response {
u8 gt[];
} __packed;
+struct pn532_autopoll_resp {
+ u8 type;
+ u8 ln;
+ u8 tg;
+ u8 tgdata[];
+};
+
+/* PN532_CMD_IN_AUTOPOLL */
+#define PN532_AUTOPOLL_POLLNR_INFINITE 0xff
+#define PN532_AUTOPOLL_PERIOD 0x03 /* in units of 150 ms */
+
+#define PN532_AUTOPOLL_TYPE_GENERIC_106 0x00
+#define PN532_AUTOPOLL_TYPE_GENERIC_212 0x01
+#define PN532_AUTOPOLL_TYPE_GENERIC_424 0x02
+#define PN532_AUTOPOLL_TYPE_JEWEL 0x04
+#define PN532_AUTOPOLL_TYPE_MIFARE 0x10
+#define PN532_AUTOPOLL_TYPE_FELICA212 0x11
+#define PN532_AUTOPOLL_TYPE_FELICA424 0x12
+#define PN532_AUTOPOLL_TYPE_ISOA 0x20
+#define PN532_AUTOPOLL_TYPE_ISOB 0x23
+#define PN532_AUTOPOLL_TYPE_DEP_PASSIVE_106 0x40
+#define PN532_AUTOPOLL_TYPE_DEP_PASSIVE_212 0x41
+#define PN532_AUTOPOLL_TYPE_DEP_PASSIVE_424 0x42
+#define PN532_AUTOPOLL_TYPE_DEP_ACTIVE_106 0x80
+#define PN532_AUTOPOLL_TYPE_DEP_ACTIVE_212 0x81
+#define PN532_AUTOPOLL_TYPE_DEP_ACTIVE_424 0x82
/* PN533_TG_INIT_AS_TARGET */
#define PN533_INIT_TARGET_PASSIVE 0x1
@@ -1389,6 +1415,101 @@ static int pn533_poll_dep(struct nfc_dev *nfc_dev)
return rc;
}
+static int pn533_autopoll_complete(struct pn533 *dev, void *arg,
+ struct sk_buff *resp)
+{
+ struct pn532_autopoll_resp *apr;
+ struct nfc_target nfc_tgt;
+ u8 nbtg;
+ int rc;
+
+ if (IS_ERR(resp)) {
+ rc = PTR_ERR(resp);
+
+ nfc_err(dev->dev, "%s autopoll complete error %d\n",
+ __func__, rc);
+
+ if (rc == -ENOENT) {
+ if (dev->poll_mod_count != 0)
+ return rc;
+ goto stop_poll;
+ } else if (rc < 0) {
+ nfc_err(dev->dev,
+ "Error %d when running autopoll\n", rc);
+ goto stop_poll;
+ }
+ }
+
+ nbtg = resp->data[0];
+ if ((nbtg > 2) || (nbtg <= 0))
+ return -EAGAIN;
+
+ apr = (struct pn532_autopoll_resp *)&resp->data[1];
+ while (nbtg--) {
+ memset(&nfc_tgt, 0, sizeof(struct nfc_target));
+ switch (apr->type) {
+ case PN532_AUTOPOLL_TYPE_ISOA:
+ dev_dbg(dev->dev, "ISOA\n");
+ rc = pn533_target_found_type_a(&nfc_tgt, apr->tgdata,
+ apr->ln - 1);
+ break;
+ case PN532_AUTOPOLL_TYPE_FELICA212:
+ case PN532_AUTOPOLL_TYPE_FELICA424:
+ dev_dbg(dev->dev, "FELICA\n");
+ rc = pn533_target_found_felica(&nfc_tgt, apr->tgdata,
+ apr->ln - 1);
+ break;
+ case PN532_AUTOPOLL_TYPE_JEWEL:
+ dev_dbg(dev->dev, "JEWEL\n");
+ rc = pn533_target_found_jewel(&nfc_tgt, apr->tgdata,
+ apr->ln - 1);
+ break;
+ case PN532_AUTOPOLL_TYPE_ISOB:
+ dev_dbg(dev->dev, "ISOB\n");
+ rc = pn533_target_found_type_b(&nfc_tgt, apr->tgdata,
+ apr->ln - 1);
+ break;
+ case PN532_AUTOPOLL_TYPE_MIFARE:
+ dev_dbg(dev->dev, "Mifare\n");
+ rc = pn533_target_found_type_a(&nfc_tgt, apr->tgdata,
+ apr->ln - 1);
+ break;
+ default:
+ nfc_err(dev->dev,
+ "Unknown current poll modulation\n");
+ rc = -EPROTO;
+ }
+
+ if (rc)
+ goto done;
+
+ if (!(nfc_tgt.supported_protocols & dev->poll_protocols)) {
+ nfc_err(dev->dev,
+ "The Tg found doesn't have the desired protocol\n");
+ rc = -EAGAIN;
+ goto done;
+ }
+
+ dev->tgt_available_prots = nfc_tgt.supported_protocols;
+ apr = (struct pn532_autopoll_resp *)
+ (apr->tgdata + (apr->ln - 1));
+ }
+
+ pn533_poll_reset_mod_list(dev);
+ nfc_targets_found(dev->nfc_dev, &nfc_tgt, 1);
+
+done:
+ dev_kfree_skb(resp);
+ return rc;
+
+stop_poll:
+ nfc_err(dev->dev, "autopoll operation has been stopped\n");
+
+ pn533_poll_reset_mod_list(dev);
+ dev->poll_protocols = 0;
+ return rc;
+}
+
static int pn533_poll_complete(struct pn533 *dev, void *arg,
struct sk_buff *resp)
{
@@ -1532,6 +1653,7 @@ static int pn533_start_poll(struct nfc_dev *nfc_dev,
{
struct pn533 *dev = nfc_get_drvdata(nfc_dev);
struct pn533_poll_modulations *cur_mod;
+ struct sk_buff *skb;
u8 rand_mod;
int rc;
@@ -1557,9 +1679,73 @@ static int pn533_start_poll(struct nfc_dev *nfc_dev,
tm_protocols = 0;
}
- pn533_poll_create_mod_list(dev, im_protocols, tm_protocols);
dev->poll_protocols = im_protocols;
dev->listen_protocols = tm_protocols;
+ if (dev->device_type == PN533_DEVICE_PN532_AUTOPOLL) {
+ skb = pn533_alloc_skb(dev, 4 + 6);
+ if (!skb)
+ return -ENOMEM;
+
+ *((u8 *)skb_put(skb, sizeof(u8))) =
+ PN532_AUTOPOLL_POLLNR_INFINITE;
+ *((u8 *)skb_put(skb, sizeof(u8))) = PN532_AUTOPOLL_PERIOD;
+
+ if ((im_protocols & NFC_PROTO_MIFARE_MASK) &&
+ (im_protocols & NFC_PROTO_ISO14443_MASK) &&
+ (im_protocols & NFC_PROTO_NFC_DEP_MASK))
+ *((u8 *)skb_put(skb, sizeof(u8))) =
+ PN532_AUTOPOLL_TYPE_GENERIC_106;
+ else {
+ if (im_protocols & NFC_PROTO_MIFARE_MASK)
+ *((u8 *)skb_put(skb, sizeof(u8))) =
+ PN532_AUTOPOLL_TYPE_MIFARE;
+
+ if (im_protocols & NFC_PROTO_ISO14443_MASK)
+ *((u8 *)skb_put(skb, sizeof(u8))) =
+ PN532_AUTOPOLL_TYPE_ISOA;
+
+ if (im_protocols & NFC_PROTO_NFC_DEP_MASK) {
+ *((u8 *)skb_put(skb, sizeof(u8))) =
+ PN532_AUTOPOLL_TYPE_DEP_PASSIVE_106;
+ *((u8 *)skb_put(skb, sizeof(u8))) =
+ PN532_AUTOPOLL_TYPE_DEP_PASSIVE_212;
+ *((u8 *)skb_put(skb, sizeof(u8))) =
+ PN532_AUTOPOLL_TYPE_DEP_PASSIVE_424;
+ }
+ }
+
+ if (im_protocols & NFC_PROTO_FELICA_MASK ||
+ im_protocols & NFC_PROTO_NFC_DEP_MASK) {
+ *((u8 *)skb_put(skb, sizeof(u8))) =
+ PN532_AUTOPOLL_TYPE_FELICA212;
+ *((u8 *)skb_put(skb, sizeof(u8))) =
+ PN532_AUTOPOLL_TYPE_FELICA424;
+ }
+
+ if (im_protocols & NFC_PROTO_JEWEL_MASK)
+ *((u8 *)skb_put(skb, sizeof(u8))) =
+ PN532_AUTOPOLL_TYPE_JEWEL;
+
+ if (im_protocols & NFC_PROTO_ISO14443_B_MASK)
+ *((u8 *)skb_put(skb, sizeof(u8))) =
+ PN532_AUTOPOLL_TYPE_ISOB;
+
+ if (tm_protocols)
+ *((u8 *)skb_put(skb, sizeof(u8))) =
+ PN532_AUTOPOLL_TYPE_DEP_ACTIVE_106;
+
+ rc = pn533_send_cmd_async(dev, PN533_CMD_IN_AUTOPOLL, skb,
+ pn533_autopoll_complete, NULL);
+
+ if (rc < 0)
+ dev_kfree_skb(skb);
+ else
+ dev->poll_mod_count++;
+
+ return rc;
+ }
+
+ pn533_poll_create_mod_list(dev, im_protocols, tm_protocols);
/* Do not always start polling from the same modulation */
get_random_bytes(&rand_mod, sizeof(rand_mod));
@@ -2461,7 +2647,8 @@ static int pn533_dev_up(struct nfc_dev *nfc_dev)
if (dev->phy_ops->dev_up)
dev->phy_ops->dev_up(dev);
- if (dev->device_type == PN533_DEVICE_PN532) {
+ if ((dev->device_type == PN533_DEVICE_PN532) ||
+ (dev->device_type == PN533_DEVICE_PN532_AUTOPOLL)) {
int rc = pn532_sam_configuration(nfc_dev);
if (rc)
@@ -2508,6 +2695,7 @@ static int pn533_setup(struct pn533 *dev)
case PN533_DEVICE_PASORI:
case PN533_DEVICE_ACR122U:
case PN533_DEVICE_PN532:
+ case PN533_DEVICE_PN532_AUTOPOLL:
max_retries.mx_rty_atr = 0x2;
max_retries.mx_rty_psl = 0x1;
max_retries.mx_rty_passive_act =
@@ -2544,6 +2732,7 @@ static int pn533_setup(struct pn533 *dev)
switch (dev->device_type) {
case PN533_DEVICE_STD:
case PN533_DEVICE_PN532:
+ case PN533_DEVICE_PN532_AUTOPOLL:
break;
case PN533_DEVICE_PASORI:
diff --git a/drivers/nfc/pn533/pn533.h b/drivers/nfc/pn533/pn533.h
index 6541088fad73..388fc1b4fcc1 100644
--- a/drivers/nfc/pn533/pn533.h
+++ b/drivers/nfc/pn533/pn533.h
@@ -6,10 +6,11 @@
* Copyright (C) 2012-2013 Tieto Poland
*/
-#define PN533_DEVICE_STD 0x1
-#define PN533_DEVICE_PASORI 0x2
-#define PN533_DEVICE_ACR122U 0x3
-#define PN533_DEVICE_PN532 0x4
+#define PN533_DEVICE_STD 0x1
+#define PN533_DEVICE_PASORI 0x2
+#define PN533_DEVICE_ACR122U 0x3
+#define PN533_DEVICE_PN532 0x4
+#define PN533_DEVICE_PN532_AUTOPOLL 0x5
#define PN533_ALL_PROTOCOLS (NFC_PROTO_JEWEL_MASK | NFC_PROTO_MIFARE_MASK |\
NFC_PROTO_FELICA_MASK | NFC_PROTO_ISO14443_MASK |\
@@ -75,6 +76,7 @@
#define PN533_CMD_IN_ATR 0x50
#define PN533_CMD_IN_RELEASE 0x52
#define PN533_CMD_IN_JUMP_FOR_DEP 0x56
+#define PN533_CMD_IN_AUTOPOLL 0x60
#define PN533_CMD_TG_INIT_AS_TARGET 0x8c
#define PN533_CMD_TG_GET_DATA 0x86
--
2.23.0
^ permalink raw reply related
* [PATCH v8 7/7] nfc: pn532_uart: Make use of pn532 autopoll
From: Lars Poeschel @ 2019-09-19 9:16 UTC (permalink / raw)
To: GitAuthor: Lars Poeschel, open list:NFC SUBSYSTEM, open list; +Cc: Johan Hovold
In-Reply-To: <20190919091645.16439-1-poeschel@lemonage.de>
This switches the pn532 UART phy driver from manually polling to the new
autopoll mechanism.
Cc: Johan Hovold <johan@kernel.org>
Signed-off-by: Lars Poeschel <poeschel@lemonage.de>
---
Changes in v6:
- Rebased the patch series on v5.3-rc5
drivers/nfc/pn533/uart.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/nfc/pn533/uart.c b/drivers/nfc/pn533/uart.c
index 7f639051cdd0..edf8db890eaa 100644
--- a/drivers/nfc/pn533/uart.c
+++ b/drivers/nfc/pn533/uart.c
@@ -262,7 +262,7 @@ static int pn532_uart_probe(struct serdev_device *serdev)
serdev_device_set_flow_control(serdev, false);
pn532->send_wakeup = PN532_SEND_WAKEUP;
timer_setup(&pn532->cmd_timeout, pn532_cmd_timeout, 0);
- priv = pn53x_common_init(PN533_DEVICE_PN532,
+ priv = pn53x_common_init(PN533_DEVICE_PN532_AUTOPOLL,
PN533_PROTO_REQ_ACK_RESP,
pn532, &uart_phy_ops, NULL,
&pn532->serdev->dev);
--
2.23.0
^ permalink raw reply related
* Re: [PATCH RFC v3 2/5] net: Add NETIF_F_GRO_LIST feature
From: Steffen Klassert @ 2019-09-19 9:24 UTC (permalink / raw)
To: Willem de Bruijn; +Cc: Network Development, Paolo Abeni
In-Reply-To: <CA+FuTSeBmGY4_2X3Ydhf60G=An9g9iikDBQMDji=XptN_jBqiw@mail.gmail.com>
On Wed, Sep 18, 2019 at 12:10:31PM -0400, Willem de Bruijn wrote:
> On Wed, Sep 18, 2019 at 3:25 AM Steffen Klassert
> <steffen.klassert@secunet.com> wrote:
> >
> > This adds a new NETIF_F_GRO_LIST feature flag. I will be used
> > to configure listfyed GRO what will be implemented with some
> > followup paches.
>
> This should probably simultaneously introduce SKB_GSO_FRAGLIST as well
> as a BUILD_BUG_ON in net_gso_ok.
Yes, good point. I'll also rename NETIF_F_GRO_LIST to NETIF_F_GRO_FRAGLIST
and add NETIF_F_GSO_FRAGLIST what is currently missing.
>
> Please also in the commit describe the constraints of skbs that have
> this type. If I'm not mistaken, an skb with either gso_size linear
> data or one gso_sized frag, followed by a frag_list of the same. With
> the exception of the last frag_list member, whose mss may be less than
> gso_size. This will help when reasoning about all the types of skbs we
> may see at segmentation, as we recently had to do [1]
We don't use skb_segment(), so I think we don't have this constraint.
>
> Minor nit: I think it's listified, not listifyed.
I think neither of both words really exist :)
I'll rename it to fraglist GRO.
^ permalink raw reply
* [PATCH -net] staging: octeon: se "(uintptr_t)" to cast from pointer to int
From: Geert Uytterhoeven @ 2019-09-19 9:25 UTC (permalink / raw)
To: Matthew Wilcox, David S . Miller
Cc: Greg Kroah-Hartman, netdev, devel, linux-kernel,
Geert Uytterhoeven
On 32-bit:
In file included from drivers/staging/octeon/octeon-ethernet.h:41,
from drivers/staging/octeon/ethernet-tx.c:25:
drivers/staging/octeon/octeon-stubs.h: In function ‘cvmx_phys_to_ptr’:
drivers/staging/octeon/octeon-stubs.h:1205:9: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
return (void *)(physical_address);
^
drivers/staging/octeon/ethernet-tx.c: In function ‘cvm_oct_xmit’:
drivers/staging/octeon/ethernet-tx.c:264:37: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
hw_buffer.s.addr = XKPHYS_TO_PHYS((u64)skb->data);
^
drivers/staging/octeon/octeon-stubs.h:2:30: note: in definition of macro ‘XKPHYS_TO_PHYS’
#define XKPHYS_TO_PHYS(p) (p)
^
drivers/staging/octeon/ethernet-tx.c:268:37: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
hw_buffer.s.addr = XKPHYS_TO_PHYS((u64)skb->data);
^
drivers/staging/octeon/octeon-stubs.h:2:30: note: in definition of macro ‘XKPHYS_TO_PHYS’
#define XKPHYS_TO_PHYS(p) (p)
^
drivers/staging/octeon/ethernet-tx.c:276:20: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
XKPHYS_TO_PHYS((u64)skb_frag_address(fs));
^
drivers/staging/octeon/octeon-stubs.h:2:30: note: in definition of macro ‘XKPHYS_TO_PHYS’
#define XKPHYS_TO_PHYS(p) (p)
^
drivers/staging/octeon/ethernet-tx.c:280:37: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
hw_buffer.s.addr = XKPHYS_TO_PHYS((u64)CVM_OCT_SKB_CB(skb));
^
drivers/staging/octeon/octeon-stubs.h:2:30: note: in definition of macro ‘XKPHYS_TO_PHYS’
#define XKPHYS_TO_PHYS(p) (p)
^
Fix this by replacing casts to "u64" by casts to "uintptr_t", which is
either 32-bit or 64-bit, and adding an intermediate cast to "uintptr_t"
where needed.
Exposed by commit 171a9bae68c72f2d ("staging/octeon: Allow test build on
!MIPS").
Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
drivers/staging/octeon/ethernet-tx.c | 9 +++++----
drivers/staging/octeon/octeon-stubs.h | 2 +-
2 files changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/staging/octeon/ethernet-tx.c b/drivers/staging/octeon/ethernet-tx.c
index c64728fc21f229d8..7021ff07ba2a0b70 100644
--- a/drivers/staging/octeon/ethernet-tx.c
+++ b/drivers/staging/octeon/ethernet-tx.c
@@ -261,11 +261,11 @@ int cvm_oct_xmit(struct sk_buff *skb, struct net_device *dev)
/* Build the PKO buffer pointer */
hw_buffer.u64 = 0;
if (skb_shinfo(skb)->nr_frags == 0) {
- hw_buffer.s.addr = XKPHYS_TO_PHYS((u64)skb->data);
+ hw_buffer.s.addr = XKPHYS_TO_PHYS((uintptr_t)skb->data);
hw_buffer.s.pool = 0;
hw_buffer.s.size = skb->len;
} else {
- hw_buffer.s.addr = XKPHYS_TO_PHYS((u64)skb->data);
+ hw_buffer.s.addr = XKPHYS_TO_PHYS((uintptr_t)skb->data);
hw_buffer.s.pool = 0;
hw_buffer.s.size = skb_headlen(skb);
CVM_OCT_SKB_CB(skb)[0] = hw_buffer.u64;
@@ -273,11 +273,12 @@ int cvm_oct_xmit(struct sk_buff *skb, struct net_device *dev)
skb_frag_t *fs = skb_shinfo(skb)->frags + i;
hw_buffer.s.addr =
- XKPHYS_TO_PHYS((u64)skb_frag_address(fs));
+ XKPHYS_TO_PHYS((uintptr_t)skb_frag_address(fs));
hw_buffer.s.size = skb_frag_size(fs);
CVM_OCT_SKB_CB(skb)[i + 1] = hw_buffer.u64;
}
- hw_buffer.s.addr = XKPHYS_TO_PHYS((u64)CVM_OCT_SKB_CB(skb));
+ hw_buffer.s.addr =
+ XKPHYS_TO_PHYS((uintptr_t)CVM_OCT_SKB_CB(skb));
hw_buffer.s.size = skb_shinfo(skb)->nr_frags + 1;
pko_command.s.segs = skb_shinfo(skb)->nr_frags + 1;
pko_command.s.gather = 1;
diff --git a/drivers/staging/octeon/octeon-stubs.h b/drivers/staging/octeon/octeon-stubs.h
index a4ac3bfb62a85e65..b78ce9eaab85d310 100644
--- a/drivers/staging/octeon/octeon-stubs.h
+++ b/drivers/staging/octeon/octeon-stubs.h
@@ -1202,7 +1202,7 @@ static inline int cvmx_wqe_get_grp(cvmx_wqe_t *work)
static inline void *cvmx_phys_to_ptr(uint64_t physical_address)
{
- return (void *)(physical_address);
+ return (void *)(uintptr_t)(physical_address);
}
static inline uint64_t cvmx_ptr_to_phys(void *ptr)
--
2.17.1
^ permalink raw reply related
* Re: [PATCH RFC v3 2/5] net: Add NETIF_F_GRO_LIST feature
From: Steffen Klassert @ 2019-09-19 9:32 UTC (permalink / raw)
To: Subash Abhinov Kasiviswanathan
Cc: Willem de Bruijn, Network Development, Paolo Abeni
In-Reply-To: <8bc2e1658e74963d6c3ff297acdcbce6@codeaurora.org>
On Wed, Sep 18, 2019 at 08:04:18PM -0600, Subash Abhinov Kasiviswanathan wrote:
> On 2019-09-18 10:10, Willem de Bruijn wrote:
> > On Wed, Sep 18, 2019 at 3:25 AM Steffen Klassert
> > <steffen.klassert@secunet.com> wrote:
> > >
> > > This adds a new NETIF_F_GRO_LIST feature flag. I will be used
> > > to configure listfyed GRO what will be implemented with some
> > > followup paches.
> >
> > This should probably simultaneously introduce SKB_GSO_FRAGLIST as well
> > as a BUILD_BUG_ON in net_gso_ok.
> >
> > Please also in the commit describe the constraints of skbs that have
> > this type. If I'm not mistaken, an skb with either gso_size linear
> > data or one gso_sized frag, followed by a frag_list of the same. With
> > the exception of the last frag_list member, whose mss may be less than
> > gso_size. This will help when reasoning about all the types of skbs we
> > may see at segmentation, as we recently had to do [1]
> >
>
> Would it be preferrable to allow any size skbs for the listification.
We currently require a single gso_size because we adjust uh->len
on the head skb to the full size to do correct memory accounting
on the local input path. That is going to be restored with the
gso_size on segmentation.
> Since the original skbs are being restored, single gso_size shoudln't
> be a constraint here.
It might be possible to allow any sized skbs with some extra work, though.
^ permalink raw reply
* Re: [PATCH RFC v3 5/5] udp: Support UDP fraglist GRO/GSO.
From: Steffen Klassert @ 2019-09-19 9:33 UTC (permalink / raw)
To: Willem de Bruijn; +Cc: Network Development, Paolo Abeni
In-Reply-To: <CA+FuTSfSpzHCCpDRD2s0fP3u2oyKVKQPOVAp0LLPMPV+W3kFtw@mail.gmail.com>
On Wed, Sep 18, 2019 at 12:13:26PM -0400, Willem de Bruijn wrote:
> On Wed, Sep 18, 2019 at 3:25 AM Steffen Klassert
> <steffen.klassert@secunet.com> wrote:
> >
> > This patch extends UDP GRO to support fraglist GRO/GSO
> > by using the previously introduced infrastructure.
> > All UDP packets that are not targeted to a GRO capable
> > UDP sockets are going to fraglist GRO now (local input
> > and forward).
> >
> > Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
>
>
> > @@ -419,7 +460,7 @@ struct sk_buff *udp_gro_receive(struct list_head *head, struct sk_buff *skb,
> > (skb->ip_summed != CHECKSUM_PARTIAL &&
> > NAPI_GRO_CB(skb)->csum_cnt == 0 &&
> > !NAPI_GRO_CB(skb)->csum_valid))
> > - goto out_unlock;
> > + goto out;
> >
> > /* mark that this skb passed once through the tunnel gro layer */
> > NAPI_GRO_CB(skb)->encap_mark = 1;
> > @@ -446,8 +487,7 @@ struct sk_buff *udp_gro_receive(struct list_head *head, struct sk_buff *skb,
> > skb_gro_postpull_rcsum(skb, uh, sizeof(struct udphdr));
> > pp = call_gro_receive_sk(udp_sk(sk)->gro_receive, sk, head, skb);
> >
> > -out_unlock:
> > - rcu_read_unlock();
> > +out:
> > skb_gro_flush_final(skb, pp, flush);
> > return pp;
> > }
>
> This probably belongs in patch 1?
Yes, apparently :)
^ permalink raw reply
* Re: [PATCH RFC v3 0/5] Support fraglist GRO/GSO
From: Steffen Klassert @ 2019-09-19 9:41 UTC (permalink / raw)
To: Willem de Bruijn; +Cc: Network Development, Paolo Abeni, marcelo.leitner
In-Reply-To: <CA+FuTSdVFguDHXYPJBRrLhzPWBaykd+7PRqEmGf_eOFC3iHpAg@mail.gmail.com>
On Wed, Sep 18, 2019 at 12:17:08PM -0400, Willem de Bruijn wrote:
> On Wed, Sep 18, 2019 at 3:25 AM Steffen Klassert
> <steffen.klassert@secunet.com> wrote:
> >
> > This patchset adds support to do GRO/GSO by chaining packets
> > of the same flow at the SKB frag_list pointer. This avoids
> > the overhead to merge payloads into one big packet, and
> > on the other end, if GSO is needed it avoids the overhead
> > of splitting the big packet back to the native form.
> >
> > Patch 1 Enables UDP GRO by default.
> >
> > Patch 2 adds a netdev feature flag to enable listifyed GRO,
> > this implements one of the configuration options discussed
> > at netconf 2019.
> >
> > Patch 3 adds a netdev software feature set that defaults to off
> > and assigns the new listifyed GRO feature flag to it.
> >
> > Patch 4 adds the core infrastructure to do fraglist GRO/GSO.
> >
> > Patch 5 enables UDP to use fraglist GRO/GSO if configured and no
> > GRO supported socket is found.
>
> Very nice feature, Steffen.
Thanks!
> Aside from questions around performance,
> my only question is really how this relates to GSO_BY_FRAGS.
>
> More specifically, whether we can remove that in favor of using your
> new skb_segment_list. That would actually be a big first step in
> simplifying skb_segment back to something manageable.
As Marcelo pointed out, this should be doable.
Thanks for all your review. I'll incorporate your comments and do
RFC v4, so that we hopefully can start the mainlining process as
soon as net-next opens again.
^ permalink raw reply
* RE: [RFC PATCH v3 4/6] psci: Add hvc call service for ptp_kvm.
From: Jianyong Wu (Arm Technology China) @ 2019-09-19 9:46 UTC (permalink / raw)
To: Paolo Bonzini, netdev@vger.kernel.org, yangbo.lu@nxp.com,
john.stultz@linaro.org, tglx@linutronix.de,
sean.j.christopherson@intel.com, maz@kernel.org,
richardcochran@gmail.com, Mark Rutland, Will Deacon,
Suzuki Poulose
Cc: linux-kernel@vger.kernel.org, kvm@vger.kernel.org, Steve Capper,
Kaly Xin (Arm Technology China), Justin He (Arm Technology China),
nd, linux-arm-kernel@lists.infradead.org
In-Reply-To: <629538ea-13fb-e666-8df6-8ad23f114755@redhat.com>
Hi Paolo,
> -----Original Message-----
> From: Paolo Bonzini <pbonzini@redhat.com>
> Sent: Wednesday, September 18, 2019 6:24 PM
> To: Jianyong Wu (Arm Technology China) <Jianyong.Wu@arm.com>;
> netdev@vger.kernel.org; yangbo.lu@nxp.com; john.stultz@linaro.org;
> tglx@linutronix.de; sean.j.christopherson@intel.com; maz@kernel.org;
> richardcochran@gmail.com; Mark Rutland <Mark.Rutland@arm.com>; Will
> Deacon <Will.Deacon@arm.com>; Suzuki Poulose
> <Suzuki.Poulose@arm.com>
> Cc: linux-kernel@vger.kernel.org; kvm@vger.kernel.org; Steve Capper
> <Steve.Capper@arm.com>; Kaly Xin (Arm Technology China)
> <Kaly.Xin@arm.com>; Justin He (Arm Technology China)
> <Justin.He@arm.com>; nd <nd@arm.com>; linux-arm-
> kernel@lists.infradead.org
> Subject: Re: [RFC PATCH v3 4/6] psci: Add hvc call service for ptp_kvm.
>
> On 18/09/19 11:57, Jianyong Wu (Arm Technology China) wrote:
> > Hi Paolo,
> >
> >> On 18/09/19 10:07, Jianyong Wu wrote:
> >>> + case ARM_SMCCC_VENDOR_HYP_KVM_PTP_FUNC_ID:
> >>> + getnstimeofday(ts);
> >>
> >> This is not Y2038-safe. Please use ktime_get_real_ts64 instead, and
> >> split the 64-bit seconds value between val[0] and val[1].
> >>
> > As far as I know, y2038-safe will only affect signed 32-bit integer,
> > how does it affect 64-bit integer?
> > And why split 64-bit number into two blocks is necessary?
>
> val is an u32, not an u64. (And val[0], where you store the seconds, is best
> treated as signed, since val[0] == -1 is returned for
> SMCCC_RET_NOT_SUPPORTED).
>
Yeah, need consider twice.
Val[] should be long not u32 I think, so in arm64 I can avoid that Y2038_safe, but
also need rewrite for arm32.
> >> However, it seems to me that the new function is not needed and you
> >> can just use ktime_get_snapshot. You'll get the time in
> >> systime_snapshot->real and the cycles value in systime_snapshot->cycles.
> >
> > See patch 5/6, I need both counter cycle and clocksource,
> ktime_get_snapshot seems only offer cycles.
>
> No, patch 5/6 only needs the current clock (ptp_sc.cycles is never accessed).
> So you could just use READ_ONCE(tk->tkr_mono.clock).
>
Yeah, patch 5/6 just need clocksource, but I think tk->tkr_mono.clock can't read in external like module,
So I need an API to expose clocksource.
> However, even then I don't think it is correct to use ptp_sc.cs blindly in patch
> 5. I think there is a misunderstanding on the meaning of
> system_counterval.cs as passed to get_device_system_crosststamp.
> system_counterval.cs is not the active clocksource; it's the clocksource on
> which system_counterval.cycles is based.
>
I think we can use system_counterval_t as pass current clocksource to system_counterval_t.cs and its
corresponding cycles to system_counterval_t.cycles. is it a big problem?
> Hypothetically, the clocksource could be one for which ptp_sc.cycles is _not_
> a cycle value. If you set system_counterval.cs to the system clocksource,
> get_device_system_crosststamp will return a bogus value.
Yeah, but in patch 3/6, we have a corresponding pair of clock source and cycle value. So I think there will be no
that problem in this patch set.
In the implementation of get_device_system_crosststamp:
"
...
if (tk->tkr_mono.clock != system_counterval.cs)
return -ENODEV;
...
"
We need tk->tkr_mono.clock passed to get_device_system_crosststamp, just like patch 3/6 do, otherwise will return error.
> So system_counterval.cs should be set to something like
> &clocksource_counter (from drivers/clocksource/arm_arch_timer.c).
> Perhaps the right place to define kvm_arch_ptp_get_clock_fn is in that file?
>
I have checked that ptp_sc.cs is arch_sys_counter.
Also move the module API to arm_arch_timer.c will looks a little ugly and it's not easy to be accept by arm side I think.
> >>> + get_current_counterval(&sc);
> >>> + val[0] = ts->tv_sec;
> >>> + val[1] = ts->tv_nsec;
> >>> + val[2] = sc.cycles;
> >>> + val[3] = 0;
> >>> + break;
> >>
> >> This should return a guest-cycles value. If the cycles values always
> >> the same between the host and the guest on ARM, then okay. If not,
> >> you have to apply whatever offset exists.
> >>
> > In my opinion, when use ptp_kvm as clock sources to sync time between
> > host and guest, user should promise the guest and host has no clock
> > offset.
>
> What would be the adverse effect of having a fixed offset between guest
> and host? If there were one, you'd have to check that and fail the hypercall if
> there is an offset. But again, I think it's enough to subtract
> vcpu_vtimer(vcpu)->cntvoff or something like that.
>
Sure, counter offset should be considered.
> You also have to check here that the clocksource is based on the ARM
> architectural timer. Again, maybe you could place the implementation in
> drivers/clocksource/arm_arch_timer.c, and make it return -ENODEV if the
> active clocksource is not clocksource_counter. Then KVM can look for errors
> and return SMCCC_RET_NOT_SUPPORTED in that case.
I have checked it. The clock source is arch_sys_counter which is arm arch timer.
I can try to do that but I'm not sure arm side will be happy for that change.
Thanks
Jianyong Wu
>
> Thanks,
>
> Paolo
>
> > So we can be sure that the cycle between guest and host should be keep
> > consistent. But I need check it.
> > I think host cycle should be returned to guest as we should promise we
> > get clock and counter in the same time.
^ permalink raw reply
* [PATCH v2] staging: octeon: Use "(uintptr_t)" to cast from pointer to int
From: Geert Uytterhoeven @ 2019-09-19 9:50 UTC (permalink / raw)
To: Matthew Wilcox, David S . Miller
Cc: Greg Kroah-Hartman, netdev, devel, linux-kernel,
Geert Uytterhoeven
On 32-bit:
In file included from drivers/staging/octeon/octeon-ethernet.h:41,
from drivers/staging/octeon/ethernet-tx.c:25:
drivers/staging/octeon/octeon-stubs.h: In function ‘cvmx_phys_to_ptr’:
drivers/staging/octeon/octeon-stubs.h:1205:9: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
return (void *)(physical_address);
^
drivers/staging/octeon/ethernet-tx.c: In function ‘cvm_oct_xmit’:
drivers/staging/octeon/ethernet-tx.c:264:37: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
hw_buffer.s.addr = XKPHYS_TO_PHYS((u64)skb->data);
^
drivers/staging/octeon/octeon-stubs.h:2:30: note: in definition of macro ‘XKPHYS_TO_PHYS’
#define XKPHYS_TO_PHYS(p) (p)
^
drivers/staging/octeon/ethernet-tx.c:268:37: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
hw_buffer.s.addr = XKPHYS_TO_PHYS((u64)skb->data);
^
drivers/staging/octeon/octeon-stubs.h:2:30: note: in definition of macro ‘XKPHYS_TO_PHYS’
#define XKPHYS_TO_PHYS(p) (p)
^
drivers/staging/octeon/ethernet-tx.c:276:20: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
XKPHYS_TO_PHYS((u64)skb_frag_address(fs));
^
drivers/staging/octeon/octeon-stubs.h:2:30: note: in definition of macro ‘XKPHYS_TO_PHYS’
#define XKPHYS_TO_PHYS(p) (p)
^
drivers/staging/octeon/ethernet-tx.c:280:37: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
hw_buffer.s.addr = XKPHYS_TO_PHYS((u64)CVM_OCT_SKB_CB(skb));
^
drivers/staging/octeon/octeon-stubs.h:2:30: note: in definition of macro ‘XKPHYS_TO_PHYS’
#define XKPHYS_TO_PHYS(p) (p)
^
Fix this by replacing casts to "u64" by casts to "uintptr_t", which is
either 32-bit or 64-bit, and adding an intermediate cast to "uintptr_t"
where needed.
Exposed by commit 171a9bae68c72f2d ("staging/octeon: Allow test build on
!MIPS").
Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
v2:
- Fix silly typo in oneline-summary.
---
drivers/staging/octeon/ethernet-tx.c | 9 +++++----
drivers/staging/octeon/octeon-stubs.h | 2 +-
2 files changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/staging/octeon/ethernet-tx.c b/drivers/staging/octeon/ethernet-tx.c
index c64728fc21f229d8..7021ff07ba2a0b70 100644
--- a/drivers/staging/octeon/ethernet-tx.c
+++ b/drivers/staging/octeon/ethernet-tx.c
@@ -261,11 +261,11 @@ int cvm_oct_xmit(struct sk_buff *skb, struct net_device *dev)
/* Build the PKO buffer pointer */
hw_buffer.u64 = 0;
if (skb_shinfo(skb)->nr_frags == 0) {
- hw_buffer.s.addr = XKPHYS_TO_PHYS((u64)skb->data);
+ hw_buffer.s.addr = XKPHYS_TO_PHYS((uintptr_t)skb->data);
hw_buffer.s.pool = 0;
hw_buffer.s.size = skb->len;
} else {
- hw_buffer.s.addr = XKPHYS_TO_PHYS((u64)skb->data);
+ hw_buffer.s.addr = XKPHYS_TO_PHYS((uintptr_t)skb->data);
hw_buffer.s.pool = 0;
hw_buffer.s.size = skb_headlen(skb);
CVM_OCT_SKB_CB(skb)[0] = hw_buffer.u64;
@@ -273,11 +273,12 @@ int cvm_oct_xmit(struct sk_buff *skb, struct net_device *dev)
skb_frag_t *fs = skb_shinfo(skb)->frags + i;
hw_buffer.s.addr =
- XKPHYS_TO_PHYS((u64)skb_frag_address(fs));
+ XKPHYS_TO_PHYS((uintptr_t)skb_frag_address(fs));
hw_buffer.s.size = skb_frag_size(fs);
CVM_OCT_SKB_CB(skb)[i + 1] = hw_buffer.u64;
}
- hw_buffer.s.addr = XKPHYS_TO_PHYS((u64)CVM_OCT_SKB_CB(skb));
+ hw_buffer.s.addr =
+ XKPHYS_TO_PHYS((uintptr_t)CVM_OCT_SKB_CB(skb));
hw_buffer.s.size = skb_shinfo(skb)->nr_frags + 1;
pko_command.s.segs = skb_shinfo(skb)->nr_frags + 1;
pko_command.s.gather = 1;
diff --git a/drivers/staging/octeon/octeon-stubs.h b/drivers/staging/octeon/octeon-stubs.h
index a4ac3bfb62a85e65..b78ce9eaab85d310 100644
--- a/drivers/staging/octeon/octeon-stubs.h
+++ b/drivers/staging/octeon/octeon-stubs.h
@@ -1202,7 +1202,7 @@ static inline int cvmx_wqe_get_grp(cvmx_wqe_t *work)
static inline void *cvmx_phys_to_ptr(uint64_t physical_address)
{
- return (void *)(physical_address);
+ return (void *)(uintptr_t)(physical_address);
}
static inline uint64_t cvmx_ptr_to_phys(void *ptr)
--
2.17.1
^ permalink raw reply related
* Re: [PATCH -net] zd1211rw: zd_usb: Use "%zu" to format size_t
From: Kalle Valo @ 2019-09-19 9:50 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Gustavo A . R . Silva, Daniel Drake, Ulrich Kunitz,
David S . Miller, linux-wireless, netdev, linux-kernel
In-Reply-To: <20190919091532.24951-1-geert@linux-m68k.org>
Geert Uytterhoeven <geert@linux-m68k.org> writes:
> On 32-bit:
>
> drivers/net/wireless/zydas/zd1211rw/zd_usb.c: In function ‘check_read_regs’:
> drivers/net/wireless/zydas/zd1211rw/zd_def.h:18:25: warning: format ‘%ld’ expects argument of type ‘long int’, but argument 6 has type ‘size_t’ {aka ‘unsigned int’} [-Wformat=]
> dev_printk(level, dev, "%s() " fmt, __func__, ##args)
> ^~~~~~~
> drivers/net/wireless/zydas/zd1211rw/zd_def.h:22:4: note: in expansion of macro ‘dev_printk_f’
> dev_printk_f(KERN_DEBUG, dev, fmt, ## args)
> ^~~~~~~~~~~~
> drivers/net/wireless/zydas/zd1211rw/zd_usb.c:1635:3: note: in expansion of macro ‘dev_dbg_f’
> dev_dbg_f(zd_usb_dev(usb),
> ^~~~~~~~~
> drivers/net/wireless/zydas/zd1211rw/zd_usb.c:1636:51: note: format string is defined here
> "error: actual length %d less than expected %ld\n",
> ~~^
> %d
>
> Fixes: 84b0b66352470e64 ("zd1211rw: zd_usb: Use struct_size() helper")
> Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
I'll queue this for v5.4.
--
Kalle Valo
^ permalink raw reply
* Re: Fwd: [PATCH] bonding/802.3ad: fix slave initialization states race
From: Алексей Захаров @ 2019-09-19 9:53 UTC (permalink / raw)
To: Jay Vosburgh; +Cc: netdev
In-Reply-To: <9357.1568880036@nyx>
чт, 19 сент. 2019 г. в 11:00, Jay Vosburgh <jay.vosburgh@canonical.com>:
>
> Алексей Захаров wrote:
>
> >> >Once a while, one of 802.3ad slaves fails to initialize and hangs in
> >> >BOND_LINK_FAIL state. Commit 334031219a84 ("bonding/802.3ad: fix slave
> >> >link initialization transition states") checks slave->last_link_up. But
> >> >link can still hang in weird state.
> >> >After physical link comes up it sends first two LACPDU messages and
> >> >doesn't work properly after that. It doesn't send or receive LACPDU.
> >> >Once it happens, the only message in dmesg is:
> >> >bond1: link status up again after 0 ms for interface eth2
> >>
> >> I believe this message indicates that the slave entered
> >> BOND_LINK_FAIL state, but downdelay was not set. The _FAIL state is
> >> really for managing the downdelay expiration, and a slave should not be
> >> in that state (outside of a brief transition entirely within
> >> bond_miimon_inspect) if downdelay is 0.
> >That's true, downdelay was set to 0, we only use updelay 500.
> >Does it mean, that the bonding driver shouldn't set slave to FAIL
> >state in this case?
>
> It really shouldn't change the slave->link outside of the
> monitoring functions at all, because there are side effects that are not
> happening (user space notifications, updelay / downdelay, etc).
>
> >> >This behavior can be reproduced (not every time):
> >> >1. Set slave link down
> >> >2. Wait for 1-3 seconds
> >> >3. Set slave link up
> >> >
> >> >The fix is to check slave->link before setting it to BOND_LINK_FAIL or
> >> >BOND_LINK_DOWN state. If got invalid Speed/Dupex values and link is in
> >> >BOND_LINK_UP state, mark it as BOND_LINK_FAIL; otherwise mark it as
> >> >BOND_LINK_DOWN.
> >> >
> >> >Fixes: 334031219a84 ("bonding/802.3ad: fix slave link initialization
> >> >transition states")
> >> >Signed-off-by: Aleksei Zakharov <zakharov.a.g@yandex.ru>
> >> >---
> >> > drivers/net/bonding/bond_main.c | 2 +-
> >> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >> >
> >> >diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
> >> >index 931d9d935686..a28776d8f33f 100644
> >> >--- a/drivers/net/bonding/bond_main.c
> >> >+++ b/drivers/net/bonding/bond_main.c
> >> >@@ -3135,7 +3135,7 @@ static int bond_slave_netdev_event(unsigned long event,
> >> > */
> >> > if (bond_update_speed_duplex(slave) &&
> >> > BOND_MODE(bond) == BOND_MODE_8023AD) {
> >> >- if (slave->last_link_up)
> >> >+ if (slave->link == BOND_LINK_UP)
> >> > slave->link = BOND_LINK_FAIL;
> >> > else
> >> > slave->link = BOND_LINK_DOWN;
> >>
> >> Is the core problem here that slaves are reporting link up, but
> >> returning invalid values for speed and/or duplex? If so, what network
> >> device are you testing with that is exhibiting this behavior?
> >That's true, because link becomes FAIL right in this block of code.
> >We use Mellanox ConnectX-3 Pro nic.
> >
> >>
> >> If I'm not mistaken, there have been several iterations of
> >> hackery on this block of code to work around this same problem, and each
> >> time there's some corner case that still doesn't work.
> >As i can see, commit 4d2c0cda0744 ("bonding: speed/duplex update at
> >NETDEV_UP event")
> >introduced BOND_LINK_DOWN state if update speed/duplex failed.
> >
> >Commit ea53abfab960 ("bonding/802.3ad: fix link_failure_count tracking")
> >changed DOWN state to FAIL.
> >
> >Commit 334031219a84 ("bonding/802.3ad: fix slave link initialization
> >transition states")
> >implemented different new state for different current states, but it
> >was based on slave->last_link_up.
> >In our case slave->last_link_up !=0 when this code runs. But, slave is
> >not in UP state at the moment. It becomes
> >FAIL and hangs in this state.
> >So, it looks like checking if slave is in UP mode is more appropriate
> >here. At least it works in our case.
> >
> >There was one more commit 12185dfe4436 ("bonding: Force slave speed
> >check after link state recovery for 802.3ad")
> >but it doesn't help in our case.
> >
> >>
> >> As Davem asked last time around, is the real problem that device
> >> drivers report carrier up but supply invalid speed and duplex state?
> >Probably, but I'm not quite sure right now. We didn't face this issue
> >before 4d2c0cda0744 and ea53abfab960
> >commits.
>
> My concern here is that we keep adding special cases to this
> code apparently without really understanding the root cause of the
> failures. 4d2c0cda0744 asserts that there is a problem that drivers are
> not supplying speed and duplex information at NETDEV_UP time, but is not
> specific as to the details (hardware information). Before we add
> another change, I would like to understand what the actual underlying
> cause of the failure is, and if yours is somehow different from what
> 4d2c0cda0744 or ea53abfab960 were fixing (or trying to fix).
>
> Would it be possible for you to instrument the code here to dump
> out the duplex/speed failure information and carrier state of the slave
> device at this point when it fails in your testing? Something like the
> following (which I have not compile tested):
>
> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
> index 931d9d935686..758af8c2b9e1 100644
> --- a/drivers/net/bonding/bond_main.c
> +++ b/drivers/net/bonding/bond_main.c
> @@ -378,15 +378,22 @@ static int bond_update_speed_duplex(struct slave *slave)
> slave->duplex = DUPLEX_UNKNOWN;
>
> res = __ethtool_get_link_ksettings(slave_dev, &ecmd);
> - if (res < 0)
> + if (res < 0) {
> + pr_err("DBG ksettings res %d slave %s\n", res, slave_dev->name);
> return 1;
> - if (ecmd.base.speed == 0 || ecmd.base.speed == ((__u32)-1))
> + }
> + if (ecmd.base.speed == 0 || ecmd.base.speed == ((__u32)-1)) {
> + pr_err("DBG speed %u slave %s\n", ecmd.base.speed,
> + slave_dev->name);
> return 1;
> + }
> switch (ecmd.base.duplex) {
> case DUPLEX_FULL:
> case DUPLEX_HALF:
> break;
> default:
> + pr_err("DBG duplex %u slave %s\n", ecmd.base.duplex,
> + slave_dev->name);
> return 1;
> }
>
> @@ -3135,6 +3142,9 @@ static int bond_slave_netdev_event(unsigned long event,
> */
> if (bond_update_speed_duplex(slave) &&
> BOND_MODE(bond) == BOND_MODE_8023AD) {
> + pr_err("DBG slave %s event %d carrier %d\n",
> + slave->dev->name, event,
> + netif_carrier_ok(slave->dev));
> if (slave->last_link_up)
> slave->link = BOND_LINK_FAIL;
> else
Thanks, did that, without my patch. Here is the output when link doesn't work.
Host has actor port state 71 and partner port state 1:
[Thu Sep 19 12:14:04 2019] mlx4_en: eth2: Steering Mode 1
[Thu Sep 19 12:14:04 2019] DBG speed 4294967295 slave eth2
[Thu Sep 19 12:14:04 2019] DBG slave eth2 event 1 carrier 0
[Thu Sep 19 12:14:04 2019] 8021q: adding VLAN 0 to HW filter on device eth2
[Thu Sep 19 12:14:04 2019] mlx4_en: eth2: Link Up
[Thu Sep 19 12:14:04 2019] bond-san: link status up again after 0 ms
for interface eth2
Here is the output when everything works fine:
[Thu Sep 19 12:15:40 2019] mlx4_en: eth2: Steering Mode 1
[Thu Sep 19 12:15:40 2019] DBG speed 4294967295 slave eth2
[Thu Sep 19 12:15:40 2019] DBG slave eth2 event 1 carrier 0
[Thu Sep 19 12:15:40 2019] 8021q: adding VLAN 0 to HW filter on device eth2
[Thu Sep 19 12:15:40 2019] bond-san: link status definitely down for
interface eth2, disabling it
[Thu Sep 19 12:15:40 2019] mlx4_en: eth2: Link Up
[Thu Sep 19 12:15:40 2019] bond-san: link status up for interface
eth2, enabling it in 500 ms
[Thu Sep 19 12:15:41 2019] bond-san: link status definitely up for
interface eth2, 10000 Mbps full duplex
If I'm not mistaken, there's up event before carrier is up.
--
Best Regards,
Aleksei Zakharov
System administrator
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox