* Re: [PATCH v1 6/7] net: mvneta: Don't use GRO on Armada 3720
From: Marek Behún @ 2018-08-21 10:07 UTC (permalink / raw)
To: Jisheng Zhang
Cc: Thomas Petazzoni, Andrew Lunn, linux-arm-kernel, netdev,
Gregory CLEMENT, Tomas Hlavacek, Russell King - ARM Linux,
David S. Miller
In-Reply-To: <20180809200832.7de1fa9e@xhacker.debian>
On Thu, 9 Aug 2018 20:08:32 +0800
Jisheng Zhang <Jisheng.Zhang@synaptics.com> wrote:
> On Thu, 9 Aug 2018 19:27:55 +0800 Jisheng Zhang wrote:
>
> > Hi,
> >
> > On Thu, 9 Aug 2018 12:40:41 +0800 Jisheng Zhang wrote:
> >
> > > + more people
> > >
> > > On Wed, 8 Aug 2018 17:27:05 +0200 Marek Behún wrote:
> > >
> > > > For some reason on Armada 3720 boards (EspressoBin and Turris
> > > > Mox) the networking driver behaves weirdly when using
> > > > napi_gro_receive.
> > > >
> > > > For example downloading a big file from a local network (low
> > > > ping) is fast, but when downloading from a remote server
> > > > (higher ping), the download speed is at first high but drops
> > > > rapidly to almost nothing or absolutely nothing.
> > >
> > > We also met this issue on some berlin platforms. I tried to fix
> > > the bug, but no clue so far.
> > >
> > > >
> > > > This is fixed when using netif_receive_skb instead of
> > > > napi_gro_receive.
> > >
> > > This is a workaround. The good news is this workaround also fixes
> > > the issue we saw on berlin.
> > >
> > > >
> > > > Signed-off-by: Marek Behun <marek.behun@nic.cz>
> > > > Cc: Russell King - ARM Linux <linux@armlinux.org.uk>
> > > > Cc: netdev@vger.kernel.org
> > > >
> > > > diff --git a/drivers/net/ethernet/marvell/mvneta.c
> > > > b/drivers/net/ethernet/marvell/mvneta.c index
> > > > 0ad2f3f7da85..27f3017d94c5 100644 ---
> > > > a/drivers/net/ethernet/marvell/mvneta.c +++
> > > > b/drivers/net/ethernet/marvell/mvneta.c @@ -1959,7 +1959,10 @@
> > > > static int mvneta_rx_swbm(struct mvneta_port *pp, int rx_todo,
> > > > skb->protocol = eth_type_trans(skb,
> > > > dev); mvneta_rx_csum(pp, rx_status, skb);
> > > > - napi_gro_receive(&port->napi, skb);
> > > > + if (pp->neta_armada3700)
> > > > + netif_receive_skb(skb);
> > > > + else
> > > > + napi_gro_receive(&port->napi,
> > > > skb);
> >
> > I think I found the root cause, if neta_armada3700 is true, the
> > port got from this_cpu_ptr(pp->ports) is invalid, this is bug...
> > I'll cook a patch for this
>
> correct it as:
>
> the port's(port is got from this_cpu_ptr(pp->ports) napi is invalid.
>
> Patch is sent out. Could you please try?
>
> Per my test, it solves the issue we saw on berlin.
>
> >
> > Thanks
> >
> > > >
> > > > rcvd_pkts++;
> > > > rcvd_bytes += rx_bytes;
> > > > @@ -2001,7 +2004,10 @@ static int mvneta_rx_swbm(struct
> > > > mvneta_port *pp, int rx_todo,
> > > > mvneta_rx_csum(pp, rx_status, skb);
> > > >
> > > > - napi_gro_receive(&port->napi, skb);
> > > > + if (pp->neta_armada3700)
> > > > + netif_receive_skb(skb);
> > > > + else
> > > > + napi_gro_receive(&port->napi, skb);
> > > > }
> > > >
> > > > if (rcvd_pkts) {
> > >
> >
>
Jisheng, the issue is solved with your patch. Thanks :)
^ permalink raw reply
* RE: [PATCH] tipc: fix issue that tipc_dest neglects of big-endian
From: Jon Maloy @ 2018-08-21 9:52 UTC (permalink / raw)
To: David Miller, Haiqing.Bai@windriver.com
Cc: ying.xue@windriver.com, zhenbo.gao@windriver.com,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <20180820.192031.1137174206399090141.davem@davemloft.net>
Hi David,
Please hold this one for now.
///jon
> -----Original Message-----
> From: David Miller <davem@davemloft.net>
> Sent: Monday, 20 August, 2018 22:21
> To: Haiqing.Bai@windriver.com
> Cc: Jon Maloy <jon.maloy@ericsson.com>; ying.xue@windriver.com; zhenbo.gao@windriver.com; netdev@vger.kernel.org; linux-
> kernel@vger.kernel.org
> Subject: Re: [PATCH] tipc: fix issue that tipc_dest neglects of big-endian
>
> From: Haiqing Bai <Haiqing.Bai@windriver.com>
> Date: Mon, 20 Aug 2018 18:26:01 +0800
>
> > The tipc multicast demo in tipcutils fails to work on big-endian hardware.
> > The tipc multicast server can not receive the packets sent by the multicast
> > client for that the dest port is always zero after tipc_dest_pop, then it
> > is found that the struct tipc_dest fails to take big/little endian into
> > account.
> >
> > Signed-off-by: Haiqing Bai <Haiqing.Bai@windriver.com>
> > Signed-off-by: Zhenbo Gao <zhenbo.gao@windriver.com>
>
> Jon and Ying, please review.
>
> thank you.
^ permalink raw reply
* Re: [PATCH v2 06/29] mtd: Add support for reading MTD devices via the nvmem API
From: Srinivas Kandagatla @ 2018-08-21 9:50 UTC (permalink / raw)
To: Boris Brezillon
Cc: Alban, Bartosz Golaszewski, Jonathan Corbet, Sekhar Nori,
Kevin Hilman, Russell King, Arnd Bergmann, Greg Kroah-Hartman,
David Woodhouse, Brian Norris, Marek Vasut, Richard Weinberger,
Grygorii Strashko, David S . Miller, Naren, Mauro Carvalho Chehab,
Andrew Morton, Lukas Wunner, Dan Carpenter, Florian Fainelli
In-Reply-To: <20180820202038.5d3dc195@bbrezillon>
On 20/08/18 19:20, Boris Brezillon wrote:
> On Mon, 20 Aug 2018 11:43:34 +0100
> Srinivas Kandagatla <srinivas.kandagatla@linaro.org> wrote:
>
>>
>> Overall am still not able to clear visualize on how MTD bindings with
>> nvmem cells would look in both partition and un-partition usecases?
>> An example DT would be nice here!!
>
> Something along those lines:
>
This looks good to me.
> mtdnode {
> nvmem-cells {
> #address-cells = <1>;
> #size-cells = <1>;
>
> cell@0 {
> reg = <0x0 0x14>;
> };
> };
>
> partitions {
> compatible = "fixed-partitions";
> #address-cells = <1>;
> #size-cells = <1>;
>
> partition@0 {
> reg = <0x0 0x20000>;
>
> nvmem-cells {
> #address-cells = <1>;
> #size-cells = <1>;
>
> cell@0 {
> reg = <0x0 0x10>;
> };
> };
> };
> };
> }; >
Just curious...Is there a reason why we can't do it like this?:
Is this because of issue of #address-cells and #size-cells Or mtd
bindings always prefer subnodes?
mtdnode {
reg = <0x0123000 0x40000>;
#address-cells = <1>;
#size-cells = <1>;
cell@0 {
compatible = "nvmem-cell";
reg = <0x0 0x14>;
};
partitions {
compatible = "fixed-partitions";
#address-cells = <1>;
#size-cells = <1>;
partition@0 {
reg = <0x0 0x20000>;
cell@0 {
compatible = "nvmem-cell";
reg = <0x0 0x10>;
};
};
};
};
Am okay either way!
thanks,
srini
^ permalink raw reply
* Re: [PATCH v2 06/29] mtd: Add support for reading MTD devices via the nvmem API
From: Boris Brezillon @ 2018-08-21 12:57 UTC (permalink / raw)
To: Alban
Cc: Srinivas Kandagatla, Bartosz Golaszewski, Jonathan Corbet,
Sekhar Nori, Kevin Hilman, Russell King, Arnd Bergmann,
Greg Kroah-Hartman, David Woodhouse, Brian Norris, Marek Vasut,
Richard Weinberger, Grygorii Strashko, David S . Miller, Naren,
Mauro Carvalho Chehab, Andrew Morton, Lukas Wunner, Dan Carpenter
In-Reply-To: <20180821142716.0adcdd56@eos>
On Tue, 21 Aug 2018 14:27:16 +0200
Alban <albeu@free.fr> wrote:
> On Tue, 21 Aug 2018 07:44:04 +0200
> Boris Brezillon <boris.brezillon@bootlin.com> wrote:
>
> > On Tue, 21 Aug 2018 00:53:27 +0200
> > Alban <albeu@free.fr> wrote:
> >
> > > On Sun, 19 Aug 2018 18:46:09 +0200
> > > Boris Brezillon <boris.brezillon@bootlin.com> wrote:
> > >
> > > > On Sun, 19 Aug 2018 13:31:06 +0200
> > > > Alban <albeu@free.fr> wrote:
> > > >
> > > > > On Fri, 17 Aug 2018 18:27:20 +0200
> > > > > Boris Brezillon <boris.brezillon@bootlin.com> wrote:
> > > > >
> > > > > > Hi Bartosz,
> > > > > >
> > > > > > On Fri, 10 Aug 2018 10:05:03 +0200
> > > > > > Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> > > > > >
> > > > > > > From: Alban Bedel <albeu@free.fr>
> > > > > > >
> > > > > > > Allow drivers that use the nvmem API to read data stored on
> > > > > > > MTD devices. For this the mtd devices are registered as
> > > > > > > read-only NVMEM providers.
> > > > > > >
> > > > > > > Signed-off-by: Alban Bedel <albeu@free.fr>
> > > > > > > [Bartosz:
> > > > > > > - use the managed variant of nvmem_register(),
> > > > > > > - set the nvmem name]
> > > > > > > Signed-off-by: Bartosz Golaszewski
> > > > > > > <bgolaszewski@baylibre.com>
> > > > > >
> > > > > > What happened to the 2 other patches of Alban's series? I'd
> > > > > > really like the DT case to be handled/agreed on in the same
> > > > > > patchset, but IIRC, Alban and Srinivas disagreed on how this
> > > > > > should be represented. I hope this time we'll come to an
> > > > > > agreement, because the MTD <-> NVMEM glue has been floating
> > > > > > around for quite some time...
> > > > >
> > > > > These other patches were to fix what I consider a fundamental
> > > > > flaw in the generic NVMEM bindings, however we couldn't agree
> > > > > on this point. Bartosz later contacted me to take over this
> > > > > series and I suggested to just change the MTD NVMEM binding to
> > > > > use a compatible string on the NVMEM cells as an alternative
> > > > > solution to fix the clash with the old style MTD partition.
> > > > >
> > > > > However all this has no impact on the code needed to add NVMEM
> > > > > support to MTD, so the above patch didn't change at all.
> > > >
> > > > It does have an impact on the supported binding though.
> > > > nvmem->dev.of_node is automatically assigned to mtd->dev.of_node,
> > > > which means people will be able to define their NVMEM cells
> > > > directly under the MTD device and reference them from other nodes
> > > > (even if it's not documented), and as you said, it conflict with
> > > > the old MTD partition bindings. So we'd better agree on this
> > > > binding before merging this patch.
> > >
> > > Unless the nvmem cell node has a compatible string, then it won't be
> > > considered as a partition by the MTD code. That is were the clash
> > > is, both bindings allow free named child nodes without a compatible
> > > string.
> >
> > Except the current nvmem cells parsing code does not enforce that, and
> > existing DTs rely on this behavior, so we're screwed. Or are you
> > suggesting to add a new "bool check_cells_compat;" field to
> > nvmem_config?
>
> There is no nvmem cell parsing at the moment. The DT lookup just
> resolve the phandle to the cell node, take the parent node and search
> for the nvmem provider that has this OF node. So extending it in case
> the node has a *new* compatible string would not break users of the old
> binding, none of them has a compatible string.
But we want to enforce the compat check on MTD devices, otherwise old
MTD partitions (those defined under the MTD node) will be considered as
NVMEM cells by the NVMEM framework. Hence the bool check_cells_compat
field.
>
> > >
> > > > I see several options:
> > > >
> > > > 1/ provide a way to tell the NVMEM framework not to use
> > > > parent->of_node even if it's != NULL. This way we really don't
> > > > support defining NVMEM cells in the DT, and also don't support
> > > > referencing the nvmem device using a phandle.
> > >
> > > I really don't get what the point of this would be. Make the whole
> > > API useless?
> >
> > No, just allow Bartosz to get his changes merged without waiting for
> > you and Srinivas to agree on how to handle the new binding. As I said
> > earlier, this mtd <-> nvmem stuff has been around for quite some time,
> > and instead of trying to find an approach that makes everyone happy,
> > you decided to let the patchset die.
>
> As long as that wouldn't prevent using DT in the future I'm fine with
> it.
>
> > >
> > > > 2/ define a new binding where all nvmem-cells are placed in an
> > > > "nvmem" subnode (just like we have this "partitions" subnode
> > > > for partitions), and then add a config->of_node field so that the
> > > > nvmem provider can explicitly specify the DT node representing
> > > > the nvmem device. We'll also need to set this field to
> > > > ERR_PTR(-ENOENT) in case this node does not exist so that the
> > > > nvmem framework knows that it should not assign
> > > > nvmem->dev.of_node to parent->of_node
> > >
> > > This is not good. First the NVMEM device is only a virtual concept
> > > of the Linux kernel, it has no place in the DT.
> >
> > nvmem-cells is a virtual concept too, still, you define them in the
> > DT.
>
> To be honest I also think that naming this concept "nvmem" in the DT was
> a bad idea. Perhaps something like "driver-data" or "data-cell" would
> have been better as that would make it clear what this is about, nvmem
> is just the Linux implementation of this concept.
I'm fine using a different name.
>
> > > Secondly the NVMEM
> > > provider (here the MTD device) then has to manually parse its DT
> > > node to find this subnode, pass it to the NVMEM framework to later
> > > again resolve it back to the MTD device.
> >
> > We don't resolve it back to the MTD device, because the MTD device is
> > just the parent of the nvmem device.
> >
> > > Not very complex but still a lot of
> > > useless code, just registering the MTD device is a lot simpler and
> > > much more inline with most other kernel API that register a
> > > "service" available from a device.
> >
> > I'm not a big fan of this option either, but I thought I had to
> > propose it.
> >
> > >
> > > > 3/ only declare partitions as nvmem providers. This would solve
> > > > the problem we have with partitions defined in the DT since
> > > > defining sub-partitions in the DT is not (yet?) supported and
> > > > partition nodes are supposed to be leaf nodes. Still, I'm not
> > > > a big fan of this solution because it will prevent us from
> > > > supporting sub-partitions if we ever want/need to.
> > >
> > > That sound like a poor workaround.
> >
> > Yes, that's a workaround. And the reason I propose it, is, again,
> > because I don't want to block Bartosz.
> >
> > > Remember that this problem could
> > > appear with any device that has a binding that use child nodes.
> >
> > I'm talking about partitions, and you're talking about mtd devices.
> > Right now partitions don't have subnodes, and if we define that
> > partition subnodes should describe nvmem-cells, then it becomes part
> > of the official binding. So, no, the problem you mention does not
> > (yet) exist.
>
> That would add another binding that allow free named child nodes
> without compatible string although experience has repeatedly shown that
> this was a bad idea.
Yes, I agree. Just thought it was important to have this solution in
the list, even if it's just to reject it.
>
> > >
> > > > 4/ Add a ->of_xlate() hook that would be called if present by the
> > > > framework instead of using the default parsing we have right
> > > > now.
> > >
> > > That is a bit cleaner, but I don't think it would be worse the
> > > complexity.
> >
> > But it's way more flexible than putting everything in the nvmem
> > framework. BTW, did you notice that nvmem-cells parsing does not work
> > with flashes bigger than 4GB, because the framework assumes
> > #address-cells and #size-cells are always 1. That's probably something
> > we'll have to fix for the MTD case.
>
> Yes, however that's just an implementation limitation which is trivial
> to solve.
Agree. I was just pointing it in case you hadn't noticed.
>
> > > Furthermore xlate functions are more about converting
> > > from hardware parameters to internal kernel representation than to
> > > hide extra DT parsing.
> >
> > Hm, how is that different? ->of_xlate() is just a way for drivers to
> > have their own DT representation, which is exactly what we want here.
>
> There is a big difference. DT represent the hardware and the
> relationship between the devices in an OS independent format. We don't
> add extra stuff in there just to map back internal Linux API details.
And I'm not talking about adding SW information in the DT, I'm talking
about HW specific description. We have the same solution for pinctrl
configs (it's HW/driver specific).
>
> > >
> > > > 5/ Tell the nvmem framework the name of the subnode containing
> > > > nvmem cell definitions (if NULL that means cells are directly
> > > > defined under the nvmem provider node). We would set it to
> > > > "nvmem-cells" (or whatever you like) for the MTD case.
> > >
> > > If so please match on compatible and not on the node name.
> >
> > If you like.
> >
> > >
> > > 6/ Extend the current NVMEM cell lookup to check if the parent node
> > > of the cell has a compatible string set to "nvmem-cells". If it
> > > doesn't it mean we have the current binding and this node is the
> > > NVMEM device. If it does the device node is just the next parent.
> > > This is trivial to implement (literally 2 lines of code) and cover
> > > all the cases currently known.
> >
> > Except Srinivas was not happy with this solution, and this stalled the
> > discussion. I'm trying to find other options and you keep rejecting
> > all of them to come back to this one.
>
> Well, I think this is the best solution :/
>
> > >
> > > 7/ Just add a compatible string to the nvmem cell. No code change is
> > > needed,
> >
> > That's not true!!!
>
> What is not true in this statement? The current nvmem lookup don't care
> about compatible strings, so the cell lookup would just works fine. The
> MTD partition parser won't consider them as a partition because of the
> compatible string. Problem solved!
No because partitions defined the old way (as direct subnodes of the MTD
node) will be considered as NVMEM cells by the NVMEM framework, and I
don't want that. Plus, I don't want people to start defining their
NVMEM cells and forget the compat string (which would work just fine
because the NVMEM framework doesn't care).
>
> > What forces people to add this compatible in their
> > DT? Nothing. I'll tell you what will happen: people will start
> > defining their nvmem cells directly under the MTD node because that
> > *works*, and even if the binding is not documented and we consider it
> > invalid, we'll be stuck supporting it forever.
>
> Do note that undocumented bindings are not allowed. DTS that use
> undocumented bindings (normally) just get rejected.
Except that's just in theory. In practice, if people can do something
wrong, they'll complain if you later fix the bug and break their setup.
So no, if we go for the "nvmem cells have an 'nvmem-cell' compat", then
I'd like the NVMEM framework to enforce that somehow.
^ permalink raw reply
* [PATCH] strparser: remove any offset before parsing messages
From: Dominique Martinet @ 2018-08-21 12:51 UTC (permalink / raw)
To: Doron Roberts-Kedes, Tom Herbert, Dave Watson
Cc: Dominique Martinet, David S. Miller, netdev, linux-kernel
In-Reply-To: <1533854411-28184-1-git-send-email-asmadeus@codewreck.org>
Offset is not well handled by strparser users right now.
Out of the current strparser users, we have:
- tls, that handles offset properly in parse and rcv callbacks
- kcm, that handles offset in rcv but not in parse
- bpf sockmap, that does not seem to handle offset anywhere
Calling pskb_pull() on new skb ensures that the offset will be 0
everywhere in practice, and in particular for the parse function,
unless the user modifies it themselves like tls does.
This fixes a bug which can be exhibited by implementing a simple kcm
parser that looks for the packet size in the first word of the packet,
and sending two such packets in a single write() call on the other side:
the second message will be cut at the length of the first message.
Since this is a stream protocol, all the following messages will also
be corrupt since it will start looking for the next offset at a wrong
position.
Signed-off-by: Dominique Martinet <asmadeus@codewreck.org>
---
I haven't had any comment on v0, so here is what I had planned as
refactoring anyawy, but I'd *really* like some opinion on this as a
whole...
Also note that compiling bpf programs with libbcc is currently broken in
linus' master, see the fix thread here:
https://lkml.kernel.org/r/1534834088-15835-1-git-send-email-yamada.masahiro@socionext.com
You can likely just revert cafa0010cd51 ("Raise the minimum required gcc
version to 4.6") localy meanwhile, or base the patch off 4.18.
net/strparser/strparser.c | 15 ++++++++++++---
1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/net/strparser/strparser.c b/net/strparser/strparser.c
index da1a676860ca..d7fb30b1bcfc 100644
--- a/net/strparser/strparser.c
+++ b/net/strparser/strparser.c
@@ -201,7 +201,17 @@ static int __strp_recv(read_descriptor_t *desc, struct sk_buff *orig_skb,
strp->skb_nextp = NULL;
stm = _strp_msg(head);
memset(stm, 0, sizeof(*stm));
- stm->strp.offset = orig_offset + eaten;
+
+ /* Can only parse if there is no offset */
+ if (unlikely(orig_offset + eaten)) {
+ if (!pskb_pull(skb, orig_offset + eaten)) {
+ STRP_STATS_INCR(strp->stats.mem_fail);
+ strp_parser_err(strp, -ENOMEM, desc);
+ break;
+ }
+ orig_len -= eaten;
+ orig_offset = eaten = 0;
+ }
} else {
/* Unclone if we are appending to an skb that we
* already share a frag_list with.
@@ -253,8 +263,7 @@ static int __strp_recv(read_descriptor_t *desc, struct sk_buff *orig_skb,
STRP_STATS_INCR(strp->stats.msg_too_big);
strp_parser_err(strp, -EMSGSIZE, desc);
break;
- } else if (len <= (ssize_t)head->len -
- skb->len - stm->strp.offset) {
+ } else if (len <= (ssize_t)head->len - skb->len) {
/* Length must be into new skb (and also
* greater than zero)
*/
--
2.17.1
^ permalink raw reply related
* Re: [PATCH 1/4] net: macb: Fix regression breaking non-MDIO fixed-link PHYs
From: Ahmad Fatoum @ 2018-08-21 8:26 UTC (permalink / raw)
To: Andrew Lunn
Cc: David S. Miller, Nicolas Ferre, kernel, netdev, mdf, Brad Mouring,
Florian Fainelli
In-Reply-To: <20180820190630.GB12238@lunn.ch>
On 08/20/2018 09:06 PM, Andrew Lunn wrote:
> I would actually say, this is your real issue here. The warnings are
> annoying, but i don't think they are fatal. This -EBUSY is what is
> stopping the driver from loading, causing the real regression.
My real issue is that a specific commit broke the driver and I would like
to partially revert that offending commit.
> I'm guessing, but i think you will find the driver is loading once,
> but hits a EPROBE_DEFFER condition, after getting the gpio. It does
> not release the gpio correctly. Sometime later, it gets loaded again,
> but the gpio is now in use, so you get the -EBUSY.
>
> So check the error paths, and make sure cleanup is being done correct.
> It could also be a phylib core bug...
I've traced it some more: While mdiobus_register fails to find a PHY,
creation of the "MDIO" bus is still successful and it returns successfully,
having claimed the reset GPIO (These functions should really be called
miibus_register...).
of_phy_fixed_link_register tries to claim the same GPIO and fails.
A fix for that would be something along the lines of da47b45
("phy: add support for a reset-gpio specification"), which caused a regression
and was unfortunately later reverted.
But regardless, there shouldn't have been an of_mdiobus_register and a MDIO bus probe
before registering the fixed-link in the first place and my patch remedies that.
Reintroducing da47b45 would be out-of-scope for this patch series.
Cheers
Ahmad
^ permalink raw reply
* Re: [PATCH v2 06/29] mtd: Add support for reading MTD devices via the nvmem API
From: Alban @ 2018-08-21 11:39 UTC (permalink / raw)
To: Srinivas Kandagatla
Cc: Alban Bedel, Boris Brezillon, Bartosz Golaszewski,
Jonathan Corbet, Sekhar Nori, Kevin Hilman, Russell King,
Arnd Bergmann, Greg Kroah-Hartman, David Woodhouse, Brian Norris,
Marek Vasut, Richard Weinberger, Grygorii Strashko,
David S . Miller, Naren, Mauro Carvalho Chehab, Andrew Morton,
Lukas Wunner, Dan Carpenter <dan
In-Reply-To: <e0ec3194-fd3a-9b3d-fba8-f88447eb56e9@linaro.org>
[-- Attachment #1: Type: text/plain, Size: 1072 bytes --]
On Tue, 21 Aug 2018 10:50:07 +0100
Srinivas Kandagatla <srinivas.kandagatla@linaro.org> wrote:
> Just curious...Is there a reason why we can't do it like this?:
> Is this because of issue of #address-cells and #size-cells Or mtd
> bindings always prefer subnodes?
>
> mtdnode {
> reg = <0x0123000 0x40000>;
> #address-cells = <1>;
> #size-cells = <1>;
> cell@0 {
> compatible = "nvmem-cell";
> reg = <0x0 0x14>;
> };
>
> partitions {
> compatible = "fixed-partitions";
> #address-cells = <1>;
> #size-cells = <1>;
>
> partition@0 {
> reg = <0x0 0x20000>;
> cell@0 {
> compatible = "nvmem-cell";
> reg = <0x0 0x10>;
> };
> };
> };
> };
That would work, the MTD partitions parser ignore child nodes with a
compatible string when looking for "old style" partitions, see [1].
However we still have the a potential address space clash between the
nvmem cells and the main device binding.
Alban
[1]: https://elixir.bootlin.com/linux/latest/source/drivers/mtd/ofpart.c#L28
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH v2 06/29] mtd: Add support for reading MTD devices via the nvmem API
From: Boris Brezillon @ 2018-08-21 11:31 UTC (permalink / raw)
To: Srinivas Kandagatla
Cc: Alban, Bartosz Golaszewski, Jonathan Corbet, Sekhar Nori,
Kevin Hilman, Russell King, Arnd Bergmann, Greg Kroah-Hartman,
David Woodhouse, Brian Norris, Marek Vasut, Richard Weinberger,
Grygorii Strashko, David S . Miller, Naren, Mauro Carvalho Chehab,
Andrew Morton, Lukas Wunner, Dan Carpenter, Florian Fainelli
In-Reply-To: <81407b4d-a02f-4085-f333-a96102bd96ce@linaro.org>
On Tue, 21 Aug 2018 10:38:13 +0100
Srinivas Kandagatla <srinivas.kandagatla@linaro.org> wrote:
> Hi Boris/Bartosz,
>
> On 21/08/18 06:44, Boris Brezillon wrote:
> >>> 4/ Add a ->of_xlate() hook that would be called if present by the
> >>> framework instead of using the default parsing we have right now.
> >> That is a bit cleaner, but I don't think it would be worse the
> >> complexity.
> > But it's way more flexible than putting everything in the nvmem
> > framework. BTW, did you notice that nvmem-cells parsing does not work
> > with flashes bigger than 4GB, because the framework assumes
> > #address-cells and #size-cells are always 1. That's probably something
> > we'll have to fix for the MTD case.
> >
>
> I have hacked up some thing on these lines to add a custom match
> function for nvmem provider and it looks like it can work for mtd case.
>
> This addresses concern #1 "to ignore of_node from dev pointer passed to
> nvmem_config" also provides way to do some sanity checks on nvmem cell node.
> In this patch I have just added a simple mtd_nvmem_match() example which
> will be always true, however we can add checks here to see if the np is
> actually a nvmem-cells node or something on those lines to enforce the
> bindings. Please fix and remove this from nvmem-core patch incase you
> plan to use/test this.
>
> We still have one open issue of supporting #address-cells and
> #size-cells in nvmem, which I can look at if you are happy with this
> approach!
>
> ----------------------------------->cut<---------------------------------
> Author: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
> Date: Tue Aug 21 10:07:24 2018 +0100
>
> nvmem: core: add custom match function support
>
> Some nvmem providers might not have a simple DT layout, nvmem cells
> could be part of the unpartioned space or with-in partition or
> even in sub partition of the provider.
>
> Current matching function is expecting that the provider should be
> immediate parent of the cell, which might not be true for the above
> cases. So allow a custom match function for such devices which can
> validate and match the cell as per the provider specific bindings.
>
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
>
> diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
> index a57302eaceb5..33541b18ac30 100644
> --- a/drivers/mtd/mtdcore.c
> +++ b/drivers/mtd/mtdcore.c
> @@ -502,6 +502,19 @@ static int mtd_nvmem_reg_read(void *priv, unsigned
> int offset,
> return retlen == bytes ? 0 : -EIO;
> }
>
> +static int mtd_nvmem_match(void *priv, struct device *dev,
> + struct device_node *np)
> +{
> + struct mtd_info *mtd = priv;
> +
> + /*
> + * Add more checks to make sure device node is inline with
> + * binding if required
> + */
> +
> + return &mtd->dev == dev->parent;
> +}
> +
> static int mtd_nvmem_add(struct mtd_info *mtd)
> {
> struct nvmem_config config = { };
> @@ -516,6 +529,7 @@ static int mtd_nvmem_add(struct mtd_info *mtd)
> config.read_only = true;
> config.root_only = true;
> config.priv = mtd;
> + config.match = mtd_nvmem_match;
>
> mtd->nvmem = devm_nvmem_register(&mtd->dev, &config);
> if (IS_ERR(mtd->nvmem)) {
> diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
> index 3a8bf832243d..32bc4e70522c 100644
> --- a/drivers/nvmem/core.c
> +++ b/drivers/nvmem/core.c
> @@ -41,6 +41,7 @@ struct nvmem_device {
> struct device *base_dev;
> nvmem_reg_read_t reg_read;
> nvmem_reg_write_t reg_write;
> + nvmem_match_t match;
> void *priv;
> };
>
> @@ -265,6 +266,11 @@ static struct bus_type nvmem_bus_type = {
>
> static int of_nvmem_match(struct device *dev, void *nvmem_np)
> {
> + struct nvmem_device *nvmem = to_nvmem_device(dev);
> +
> + if (nvmem->match)
> + return nvmem->match(nvmem->priv, dev, nvmem_np);
> +
> return dev->of_node == nvmem_np;
> }
>
> @@ -482,7 +488,9 @@ struct nvmem_device *nvmem_register(const struct
> nvmem_config *config)
> nvmem->priv = config->priv;
> nvmem->reg_read = config->reg_read;
> nvmem->reg_write = config->reg_write;
> - nvmem->dev.of_node = config->dev->of_node;
> +
> + if (!config->match)
> + nvmem->dev.of_node = config->dev->of_node;
>
> if (config->id == -1 && config->name) {
> dev_set_name(&nvmem->dev, "%s", config->name);
> diff --git a/include/linux/nvmem-provider.h b/include/linux/nvmem-provider.h
> index 24def6ad09bb..b29059bb406e 100644
> --- a/include/linux/nvmem-provider.h
> +++ b/include/linux/nvmem-provider.h
> @@ -14,6 +14,7 @@
>
> #include <linux/err.h>
> #include <linux/errno.h>
> +#include <linux/of.h>
>
> struct nvmem_device;
> struct nvmem_cell_info;
> @@ -21,6 +22,9 @@ typedef int (*nvmem_reg_read_t)(void *priv, unsigned
> int offset,
> void *val, size_t bytes);
> typedef int (*nvmem_reg_write_t)(void *priv, unsigned int offset,
> void *val, size_t bytes);
> +typedef int (*nvmem_match_t)(void *priv, struct device *dev,
> + struct device_node *np);
> +
>
> /**
> * struct nvmem_config - NVMEM device configuration
> @@ -58,6 +62,7 @@ struct nvmem_config {
> bool root_only;
> nvmem_reg_read_t reg_read;
> nvmem_reg_write_t reg_write;
> + nvmem_match_t match;
> int size;
> int word_size;
> int stride;
>
That might work if nvmem cells are defined directly under the mtdnode.
If we go for this approach, I'd recommend replacing this ->match() hook
by ->is_nvmem_cell() and pass it the cell node instead of the nvmem
node, because what we're really after here is knowing which subnode is
an nvmem cell and which subnode is not.
^ permalink raw reply
* [PATCH] iwlwifi: pcie: make array 'prop' static, shrinks object size
From: Colin King @ 2018-08-21 10:21 UTC (permalink / raw)
To: Johannes Berg, Emmanuel Grumbach, Luca Coelho,
Intel Linux Wireless, Kalle Valo, David S . Miller,
linux-wireless, netdev
Cc: kernel-janitors, linux-kernel
From: Colin Ian King <colin.king@canonical.com>
Don't populate the array prop on the stack but instead make it static.
Makes the object code smaller by 30 bytes:
Before:
text data bss dec hex filename
80138 15382 576 96096 17760 drivers/net/wireless/intel/iwlwifi/pcie/trans.o
After:
text data bss dec hex filename
79948 15542 576 96066 17742 drivers/net/wireless/intel/iwlwifi/pcie/trans.o
(gcc version 8.2.0 x86_64)
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
drivers/net/wireless/intel/iwlwifi/pcie/trans.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/wireless/intel/iwlwifi/pcie/trans.c b/drivers/net/wireless/intel/iwlwifi/pcie/trans.c
index 7d319b6863fe..c7daf666ee21 100644
--- a/drivers/net/wireless/intel/iwlwifi/pcie/trans.c
+++ b/drivers/net/wireless/intel/iwlwifi/pcie/trans.c
@@ -1947,7 +1947,7 @@ static void iwl_trans_pcie_removal_wk(struct work_struct *wk)
struct iwl_trans_pcie_removal *removal =
container_of(wk, struct iwl_trans_pcie_removal, work);
struct pci_dev *pdev = removal->pdev;
- char *prop[] = {"EVENT=INACCESSIBLE", NULL};
+ static char *prop[] = {"EVENT=INACCESSIBLE", NULL};
dev_err(&pdev->dev, "Device gone - attempting removal\n");
kobject_uevent_env(&pdev->dev.kobj, KOBJ_CHANGE, prop);
--
2.17.1
^ permalink raw reply related
* Re: [RFC RFT PATCH v4 1/4] gpiolib: Pass bitmaps, not integer arrays, to get/set array
From: Peter Rosin @ 2018-08-21 6:52 UTC (permalink / raw)
To: Janusz Krzysztofik, Linus Walleij
Cc: Jonathan Corbet, Miguel Ojeda Sandonis, Peter Korsgaard,
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, linux-i2c, linux-mmc
In-Reply-To: <0e79bdc3-ca20-6823-c86f-5e9da7dafbc9@axentia.se>
Sorry for replying to self...
On 2018-08-21 08:49, Peter Rosin wrote:
> On 2018-08-21 01:43, Janusz Krzysztofik 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 +++++++++--------
>> drivers/bus/ts-nbus.c | 19 ++-----
>> drivers/gpio/gpio-max3191x.c | 17 +++---
>> drivers/gpio/gpiolib.c | 86 +++++++++++++++--------------
>> drivers/gpio/gpiolib.h | 4 +-
>> drivers/i2c/muxes/i2c-mux-gpio.c | 3 +-
>> drivers/mmc/core/pwrseq_simple.c | 13 ++---
>> drivers/mux/gpio.c | 4 +-
>> drivers/net/phy/mdio-mux-gpio.c | 3 +-
>> drivers/pcmcia/soc_common.c | 11 ++--
>> drivers/phy/motorola/phy-mapphone-mdm6600.c | 17 +++---
>> drivers/staging/iio/adc/ad7606.c | 9 +--
>> drivers/tty/serial/serial_mctrl_gpio.c | 7 ++-
>> include/linux/gpio/consumer.h | 18 +++---
>> 15 files changed, 138 insertions(+), 147 deletions(-)
>>
>
> *snip*
>
>> diff --git a/drivers/i2c/muxes/i2c-mux-gpio.c b/drivers/i2c/muxes/i2c-mux-gpio.c
>> index 401308e3d036..d675e0ca2fa4 100644
>> --- a/drivers/i2c/muxes/i2c-mux-gpio.c
>> +++ b/drivers/i2c/muxes/i2c-mux-gpio.c
>> @@ -27,13 +27,14 @@ struct gpiomux {
>>
>> static void i2c_mux_gpio_set(const struct gpiomux *mux, unsigned val)
>> {
>> + unsigned long value_bitmap[1] = { val, };
>> int i;
>>
>> for (i = 0; i < mux->data.n_gpios; i++)
>> mux->values[i] = (val >> i) & 1;
>>
>> gpiod_set_array_value_cansleep(mux->data.n_gpios,
>> - mux->gpios, mux->values);
>> + mux->gpios, value_bitmap);
>> }
>>
>> static int i2c_mux_gpio_select(struct i2c_mux_core *muxc, u32 chan)
>
> Please take the opportunity to completely get rid of the values member
> in struct mux_gpio. It no longer serves any purpose.
struct gpiomux
>
> *snip*
>
>> diff --git a/drivers/mux/gpio.c b/drivers/mux/gpio.c
>> index 6fdd9316db8b..cc2d5f50472a 100644
>> --- a/drivers/mux/gpio.c
>> +++ b/drivers/mux/gpio.c
>> @@ -23,14 +23,14 @@ struct mux_gpio {
>> static int mux_gpio_set(struct mux_control *mux, int state)
>> {
>> struct mux_gpio *mux_gpio = mux_chip_priv(mux->chip);
>> + unsigned long value_bitmap[1] = { state, };
>> int i;
>>
>> for (i = 0; i < mux_gpio->gpios->ndescs; i++)
>> mux_gpio->val[i] = (state >> i) & 1;
>>
>> gpiod_set_array_value_cansleep(mux_gpio->gpios->ndescs,
>> - mux_gpio->gpios->desc,
>> - mux_gpio->val);
>> + mux_gpio->gpios->desc, value_bitmap);
>>
>> return 0;
>> }
>
> Dito (but the member name is val).
Here is where struct mux_gpio fits.
Cheers,
Peter
>
> *snip*
>
> Cheers,
> Peter
>
^ permalink raw reply
* Re: [RFC RFT PATCH v4 1/4] gpiolib: Pass bitmaps, not integer arrays, to get/set array
From: Peter Rosin @ 2018-08-21 6:49 UTC (permalink / raw)
To: Janusz Krzysztofik, Linus Walleij
Cc: Jonathan Corbet, Miguel Ojeda Sandonis, Peter Korsgaard,
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, linux-i2c, linux-mmc
In-Reply-To: <20180820234341.5271-2-jmkrzyszt@gmail.com>
On 2018-08-21 01:43, Janusz Krzysztofik 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 +++++++++--------
> drivers/bus/ts-nbus.c | 19 ++-----
> drivers/gpio/gpio-max3191x.c | 17 +++---
> drivers/gpio/gpiolib.c | 86 +++++++++++++++--------------
> drivers/gpio/gpiolib.h | 4 +-
> drivers/i2c/muxes/i2c-mux-gpio.c | 3 +-
> drivers/mmc/core/pwrseq_simple.c | 13 ++---
> drivers/mux/gpio.c | 4 +-
> drivers/net/phy/mdio-mux-gpio.c | 3 +-
> drivers/pcmcia/soc_common.c | 11 ++--
> drivers/phy/motorola/phy-mapphone-mdm6600.c | 17 +++---
> drivers/staging/iio/adc/ad7606.c | 9 +--
> drivers/tty/serial/serial_mctrl_gpio.c | 7 ++-
> include/linux/gpio/consumer.h | 18 +++---
> 15 files changed, 138 insertions(+), 147 deletions(-)
>
*snip*
> diff --git a/drivers/i2c/muxes/i2c-mux-gpio.c b/drivers/i2c/muxes/i2c-mux-gpio.c
> index 401308e3d036..d675e0ca2fa4 100644
> --- a/drivers/i2c/muxes/i2c-mux-gpio.c
> +++ b/drivers/i2c/muxes/i2c-mux-gpio.c
> @@ -27,13 +27,14 @@ struct gpiomux {
>
> static void i2c_mux_gpio_set(const struct gpiomux *mux, unsigned val)
> {
> + unsigned long value_bitmap[1] = { val, };
> int i;
>
> for (i = 0; i < mux->data.n_gpios; i++)
> mux->values[i] = (val >> i) & 1;
>
> gpiod_set_array_value_cansleep(mux->data.n_gpios,
> - mux->gpios, mux->values);
> + mux->gpios, value_bitmap);
> }
>
> static int i2c_mux_gpio_select(struct i2c_mux_core *muxc, u32 chan)
Please take the opportunity to completely get rid of the values member
in struct mux_gpio. It no longer serves any purpose.
*snip*
> diff --git a/drivers/mux/gpio.c b/drivers/mux/gpio.c
> index 6fdd9316db8b..cc2d5f50472a 100644
> --- a/drivers/mux/gpio.c
> +++ b/drivers/mux/gpio.c
> @@ -23,14 +23,14 @@ struct mux_gpio {
> static int mux_gpio_set(struct mux_control *mux, int state)
> {
> struct mux_gpio *mux_gpio = mux_chip_priv(mux->chip);
> + unsigned long value_bitmap[1] = { state, };
> int i;
>
> for (i = 0; i < mux_gpio->gpios->ndescs; i++)
> mux_gpio->val[i] = (state >> i) & 1;
>
> gpiod_set_array_value_cansleep(mux_gpio->gpios->ndescs,
> - mux_gpio->gpios->desc,
> - mux_gpio->val);
> + mux_gpio->gpios->desc, value_bitmap);
>
> return 0;
> }
Dito (but the member name is val).
*snip*
Cheers,
Peter
^ permalink raw reply
* Re: [PATCH] tipc: fix issue that tipc_dest neglects of big-endian
From: Xue, Ying @ 2018-08-21 9:57 UTC (permalink / raw)
To: Jon Maloy
Cc: David Miller, Bai, Haiqing, Gao, Zhenbo, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <DM5PR15MB1513428048EE31705DA273549A310@DM5PR15MB1513.namprd15.prod.outlook.com>
Sorry, I am on vacation too. I agree with Jon. Please temporarily hold this patch. Once I am back, I will review it as soon as possible.
Sent from my iPhone
> On Aug 21, 2018, at 9:52 PM, Jon Maloy <jon.maloy@ericsson.com> wrote:
>
> Hi David,
> Please hold this one for now.
>
> ///jon
>
>> -----Original Message-----
>> From: David Miller <davem@davemloft.net>
>> Sent: Monday, 20 August, 2018 22:21
>> To: Haiqing.Bai@windriver.com
>> Cc: Jon Maloy <jon.maloy@ericsson.com>; ying.xue@windriver.com; zhenbo.gao@windriver.com; netdev@vger.kernel.org; linux-
>> kernel@vger.kernel.org
>> Subject: Re: [PATCH] tipc: fix issue that tipc_dest neglects of big-endian
>>
>> From: Haiqing Bai <Haiqing.Bai@windriver.com>
>> Date: Mon, 20 Aug 2018 18:26:01 +0800
>>
>>> The tipc multicast demo in tipcutils fails to work on big-endian hardware.
>>> The tipc multicast server can not receive the packets sent by the multicast
>>> client for that the dest port is always zero after tipc_dest_pop, then it
>>> is found that the struct tipc_dest fails to take big/little endian into
>>> account.
>>>
>>> Signed-off-by: Haiqing Bai <Haiqing.Bai@windriver.com>
>>> Signed-off-by: Zhenbo Gao <zhenbo.gao@windriver.com>
>>
>> Jon and Ying, please review.
>>
>> thank you.
^ permalink raw reply
* Re: [PATCH v2 06/29] mtd: Add support for reading MTD devices via the nvmem API
From: Boris Brezillon @ 2018-08-21 9:56 UTC (permalink / raw)
To: Srinivas Kandagatla
Cc: Alban, Bartosz Golaszewski, Jonathan Corbet, Sekhar Nori,
Kevin Hilman, Russell King, Arnd Bergmann, Greg Kroah-Hartman,
David Woodhouse, Brian Norris, Marek Vasut, Richard Weinberger,
Grygorii Strashko, David S . Miller, Naren, Mauro Carvalho Chehab,
Andrew Morton, Lukas Wunner, Dan Carpenter, Florian Fainelli
In-Reply-To: <e0ec3194-fd3a-9b3d-fba8-f88447eb56e9@linaro.org>
On Tue, 21 Aug 2018 10:50:07 +0100
Srinivas Kandagatla <srinivas.kandagatla@linaro.org> wrote:
> On 20/08/18 19:20, Boris Brezillon wrote:
> > On Mon, 20 Aug 2018 11:43:34 +0100
> > Srinivas Kandagatla <srinivas.kandagatla@linaro.org> wrote:
> >
> >>
> >> Overall am still not able to clear visualize on how MTD bindings with
> >> nvmem cells would look in both partition and un-partition usecases?
> >> An example DT would be nice here!!
> >
> > Something along those lines:
> >
> This looks good to me.
> > mtdnode {
> > nvmem-cells {
> > #address-cells = <1>;
> > #size-cells = <1>;
> >
> > cell@0 {
> > reg = <0x0 0x14>;
> > };
> > };
> >
> > partitions {
> > compatible = "fixed-partitions";
> > #address-cells = <1>;
> > #size-cells = <1>;
> >
> > partition@0 {
> > reg = <0x0 0x20000>;
> >
> > nvmem-cells {
> > #address-cells = <1>;
> > #size-cells = <1>;
> >
> > cell@0 {
> > reg = <0x0 0x10>;
> > };
> > };
> > };
> > };
> > }; >
>
> Just curious...Is there a reason why we can't do it like this?:
> Is this because of issue of #address-cells and #size-cells Or mtd
> bindings always prefer subnodes?
>
> mtdnode {
> reg = <0x0123000 0x40000>;
> #address-cells = <1>;
> #size-cells = <1>;
> cell@0 {
> compatible = "nvmem-cell";
> reg = <0x0 0x14>;
> };
>
> partitions {
> compatible = "fixed-partitions";
> #address-cells = <1>;
> #size-cells = <1>;
>
> partition@0 {
> reg = <0x0 0x20000>;
> cell@0 {
> compatible = "nvmem-cell";
> reg = <0x0 0x10>;
> };
> };
> };
> };
It's because partitions were initially directly defined under the mtd
node, so, if you have an old DT you might have something like:
mtdnode {
reg = <0x0123000 0x40000>;
#address-cells = <1>;
#size-cells = <1>;
partition@0 {
reg = <0x0 0x20000>;
...
};
...
};
If we use such a DT with this patch applied, the NVMEM framework will
consider MTD partitions as nvmem cells, which is not what we want.
^ permalink raw reply
* Re: [PATCH v2 06/29] mtd: Add support for reading MTD devices via the nvmem API
From: Srinivas Kandagatla @ 2018-08-21 9:38 UTC (permalink / raw)
To: Boris Brezillon, Alban
Cc: Bartosz Golaszewski, Jonathan Corbet, Sekhar Nori, Kevin Hilman,
Russell King, Arnd Bergmann, Greg Kroah-Hartman, David Woodhouse,
Brian Norris, Marek Vasut, Richard Weinberger, Grygorii Strashko,
David S . Miller, Naren, Mauro Carvalho Chehab, Andrew Morton,
Lukas Wunner, Dan Carpenter, Florian Fainelli, Ivan
In-Reply-To: <20180821074404.23aaeb6b@bbrezillon>
Hi Boris/Bartosz,
On 21/08/18 06:44, Boris Brezillon wrote:
>>> 4/ Add a ->of_xlate() hook that would be called if present by the
>>> framework instead of using the default parsing we have right now.
>> That is a bit cleaner, but I don't think it would be worse the
>> complexity.
> But it's way more flexible than putting everything in the nvmem
> framework. BTW, did you notice that nvmem-cells parsing does not work
> with flashes bigger than 4GB, because the framework assumes
> #address-cells and #size-cells are always 1. That's probably something
> we'll have to fix for the MTD case.
>
I have hacked up some thing on these lines to add a custom match
function for nvmem provider and it looks like it can work for mtd case.
This addresses concern #1 "to ignore of_node from dev pointer passed to
nvmem_config" also provides way to do some sanity checks on nvmem cell node.
In this patch I have just added a simple mtd_nvmem_match() example which
will be always true, however we can add checks here to see if the np is
actually a nvmem-cells node or something on those lines to enforce the
bindings. Please fix and remove this from nvmem-core patch incase you
plan to use/test this.
We still have one open issue of supporting #address-cells and
#size-cells in nvmem, which I can look at if you are happy with this
approach!
----------------------------------->cut<---------------------------------
Author: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
Date: Tue Aug 21 10:07:24 2018 +0100
nvmem: core: add custom match function support
Some nvmem providers might not have a simple DT layout, nvmem cells
could be part of the unpartioned space or with-in partition or
even in sub partition of the provider.
Current matching function is expecting that the provider should be
immediate parent of the cell, which might not be true for the above
cases. So allow a custom match function for such devices which can
validate and match the cell as per the provider specific bindings.
Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
index a57302eaceb5..33541b18ac30 100644
--- a/drivers/mtd/mtdcore.c
+++ b/drivers/mtd/mtdcore.c
@@ -502,6 +502,19 @@ static int mtd_nvmem_reg_read(void *priv, unsigned
int offset,
return retlen == bytes ? 0 : -EIO;
}
+static int mtd_nvmem_match(void *priv, struct device *dev,
+ struct device_node *np)
+{
+ struct mtd_info *mtd = priv;
+
+ /*
+ * Add more checks to make sure device node is inline with
+ * binding if required
+ */
+
+ return &mtd->dev == dev->parent;
+}
+
static int mtd_nvmem_add(struct mtd_info *mtd)
{
struct nvmem_config config = { };
@@ -516,6 +529,7 @@ static int mtd_nvmem_add(struct mtd_info *mtd)
config.read_only = true;
config.root_only = true;
config.priv = mtd;
+ config.match = mtd_nvmem_match;
mtd->nvmem = devm_nvmem_register(&mtd->dev, &config);
if (IS_ERR(mtd->nvmem)) {
diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
index 3a8bf832243d..32bc4e70522c 100644
--- a/drivers/nvmem/core.c
+++ b/drivers/nvmem/core.c
@@ -41,6 +41,7 @@ struct nvmem_device {
struct device *base_dev;
nvmem_reg_read_t reg_read;
nvmem_reg_write_t reg_write;
+ nvmem_match_t match;
void *priv;
};
@@ -265,6 +266,11 @@ static struct bus_type nvmem_bus_type = {
static int of_nvmem_match(struct device *dev, void *nvmem_np)
{
+ struct nvmem_device *nvmem = to_nvmem_device(dev);
+
+ if (nvmem->match)
+ return nvmem->match(nvmem->priv, dev, nvmem_np);
+
return dev->of_node == nvmem_np;
}
@@ -482,7 +488,9 @@ struct nvmem_device *nvmem_register(const struct
nvmem_config *config)
nvmem->priv = config->priv;
nvmem->reg_read = config->reg_read;
nvmem->reg_write = config->reg_write;
- nvmem->dev.of_node = config->dev->of_node;
+
+ if (!config->match)
+ nvmem->dev.of_node = config->dev->of_node;
if (config->id == -1 && config->name) {
dev_set_name(&nvmem->dev, "%s", config->name);
diff --git a/include/linux/nvmem-provider.h b/include/linux/nvmem-provider.h
index 24def6ad09bb..b29059bb406e 100644
--- a/include/linux/nvmem-provider.h
+++ b/include/linux/nvmem-provider.h
@@ -14,6 +14,7 @@
#include <linux/err.h>
#include <linux/errno.h>
+#include <linux/of.h>
struct nvmem_device;
struct nvmem_cell_info;
@@ -21,6 +22,9 @@ typedef int (*nvmem_reg_read_t)(void *priv, unsigned
int offset,
void *val, size_t bytes);
typedef int (*nvmem_reg_write_t)(void *priv, unsigned int offset,
void *val, size_t bytes);
+typedef int (*nvmem_match_t)(void *priv, struct device *dev,
+ struct device_node *np);
+
/**
* struct nvmem_config - NVMEM device configuration
@@ -58,6 +62,7 @@ struct nvmem_config {
bool root_only;
nvmem_reg_read_t reg_read;
nvmem_reg_write_t reg_write;
+ nvmem_match_t match;
int size;
int word_size;
int stride;
----------------------------------->cut<---------------------------------
thanks,
srini
^ permalink raw reply related
* Re: [RFC PATCH net-next v2 10/17] ethtool: implement GET_SETTINGS message
From: Michal Kubecek @ 2018-08-21 9:32 UTC (permalink / raw)
To: Andrew Lunn
Cc: netdev, linux-kernel, Jiri Pirko, David Miller, Florian Fainelli,
Roopa Prabhu, Jakub Kicinski, John W. Linville
In-Reply-To: <20180730185455.GJ2983@lunn.ch>
On Mon, Jul 30, 2018 at 08:54:55PM +0200, Andrew Lunn wrote:
> > +/* Internal kernel helper to query a device ethtool_link_settings.
> > + *
> > + * Backward compatibility note: for compatibility with legacy drivers
> > + * that implement only the ethtool_cmd API, this has to work with both
> > + * drivers implementing get_link_ksettings API and drivers
> > + * implementing get_settings API. When drivers implement get_settings
> > + * and report ethtool_cmd deprecated fields
> > + * (transceiver/maxrxpkt/maxtxpkt), these fields are silently ignored
> > + * because the resulting struct ethtool_link_settings does not report them.
>
> ~/linux/drivers$ grep -r [.]get_settings *
> net/ethernet/8390/etherh.c: .get_settings = etherh_get_settings,
>
> I don't think it is worth adding support for .get_settings for just
> one driver. It is better to just convert that driver to the new API.
I have prepared a patch converting 8390/etherh driver to use
{g,s}et_link_ksettings and I'm going to submit it when net-next opens.
Do you think we can then drop {g,s}et_settings callbacks completely
(i.e. also from ioctl() code and ethtool_ops)? Do we care about
unconverted out of tree drivers?
Michal Kubecek
^ permalink raw reply
* Re: [RFC 0/1] Appletalk AARP probe broken by receipt of own broadcasts.
From: Craig McGeachie @ 2018-08-21 6:07 UTC (permalink / raw)
To: Andrew Lunn; +Cc: David S. Miller, netdev, Craig McGeachie
In-Reply-To: <20180820132831.GA6583@lunn.ch>
On 21/08/18 01:28, Andrew Lunn wrote:
>> Turns out the problem is WinPCAP running on the host system (Windows
>> 10).
> It would be good to report this to the WinPCAP people. I hate it when
> debug tools actually introduce bugs.
This will take me some time. I couldn't fairly blame WinPCAP. Firstly,
WinPCAP isn't really supported on Windows 10 (hence the fork,
Win10PCAP). Secondly, right now all that I know is that it's some
combination of WinPCAP and the virtual Intel 82540EM NIC. I'll have to
play around a bit; maybe try some of the other virtual NICs.
Then I think it would be good to have a more convenient test case than
"First, install netatalk 2.2.6." I'll see if I can make one.
Cheers,
Craig.
^ permalink raw reply
* Re: [PATCH v2 06/29] mtd: Add support for reading MTD devices via the nvmem API
From: Boris Brezillon @ 2018-08-21 5:44 UTC (permalink / raw)
To: Alban
Cc: Srinivas Kandagatla, Bartosz Golaszewski, Jonathan Corbet,
Sekhar Nori, Kevin Hilman, Russell King, Arnd Bergmann,
Greg Kroah-Hartman, David Woodhouse, Brian Norris, Marek Vasut,
Richard Weinberger, Grygorii Strashko, David S . Miller, Naren,
Mauro Carvalho Chehab, Andrew Morton, Lukas Wunner, Dan Carpenter
In-Reply-To: <20180821005327.0d312a85@tock>
On Tue, 21 Aug 2018 00:53:27 +0200
Alban <albeu@free.fr> wrote:
> On Sun, 19 Aug 2018 18:46:09 +0200
> Boris Brezillon <boris.brezillon@bootlin.com> wrote:
>
> > On Sun, 19 Aug 2018 13:31:06 +0200
> > Alban <albeu@free.fr> wrote:
> >
> > > On Fri, 17 Aug 2018 18:27:20 +0200
> > > Boris Brezillon <boris.brezillon@bootlin.com> wrote:
> > >
> > > > Hi Bartosz,
> > > >
> > > > On Fri, 10 Aug 2018 10:05:03 +0200
> > > > Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> > > >
> > > > > From: Alban Bedel <albeu@free.fr>
> > > > >
> > > > > Allow drivers that use the nvmem API to read data stored on MTD devices.
> > > > > For this the mtd devices are registered as read-only NVMEM providers.
> > > > >
> > > > > Signed-off-by: Alban Bedel <albeu@free.fr>
> > > > > [Bartosz:
> > > > > - use the managed variant of nvmem_register(),
> > > > > - set the nvmem name]
> > > > > Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> > > >
> > > > What happened to the 2 other patches of Alban's series? I'd really
> > > > like the DT case to be handled/agreed on in the same patchset, but
> > > > IIRC, Alban and Srinivas disagreed on how this should be represented.
> > > > I hope this time we'll come to an agreement, because the MTD <-> NVMEM
> > > > glue has been floating around for quite some time...
> > >
> > > These other patches were to fix what I consider a fundamental flaw in
> > > the generic NVMEM bindings, however we couldn't agree on this point.
> > > Bartosz later contacted me to take over this series and I suggested to
> > > just change the MTD NVMEM binding to use a compatible string on the
> > > NVMEM cells as an alternative solution to fix the clash with the old
> > > style MTD partition.
> > >
> > > However all this has no impact on the code needed to add NVMEM support
> > > to MTD, so the above patch didn't change at all.
> >
> > It does have an impact on the supported binding though.
> > nvmem->dev.of_node is automatically assigned to mtd->dev.of_node, which
> > means people will be able to define their NVMEM cells directly under
> > the MTD device and reference them from other nodes (even if it's not
> > documented), and as you said, it conflict with the old MTD partition
> > bindings. So we'd better agree on this binding before merging this
> > patch.
>
> Unless the nvmem cell node has a compatible string, then it won't be
> considered as a partition by the MTD code. That is were the clash is,
> both bindings allow free named child nodes without a compatible string.
Except the current nvmem cells parsing code does not enforce that, and
existing DTs rely on this behavior, so we're screwed. Or are you
suggesting to add a new "bool check_cells_compat;" field to
nvmem_config?
>
> > I see several options:
> >
> > 1/ provide a way to tell the NVMEM framework not to use parent->of_node
> > even if it's != NULL. This way we really don't support defining
> > NVMEM cells in the DT, and also don't support referencing the nvmem
> > device using a phandle.
>
> I really don't get what the point of this would be. Make the whole API
> useless?
No, just allow Bartosz to get his changes merged without waiting for you
and Srinivas to agree on how to handle the new binding. As I said
earlier, this mtd <-> nvmem stuff has been around for quite some time,
and instead of trying to find an approach that makes everyone happy, you
decided to let the patchset die.
>
> > 2/ define a new binding where all nvmem-cells are placed in an
> > "nvmem" subnode (just like we have this "partitions" subnode for
> > partitions), and then add a config->of_node field so that the
> > nvmem provider can explicitly specify the DT node representing the
> > nvmem device. We'll also need to set this field to ERR_PTR(-ENOENT)
> > in case this node does not exist so that the nvmem framework knows
> > that it should not assign nvmem->dev.of_node to parent->of_node
>
> This is not good. First the NVMEM device is only a virtual concept of
> the Linux kernel, it has no place in the DT.
nvmem-cells is a virtual concept too, still, you define them in the DT.
> Secondly the NVMEM
> provider (here the MTD device) then has to manually parse its DT node to
> find this subnode, pass it to the NVMEM framework to later again
> resolve it back to the MTD device.
We don't resolve it back to the MTD device, because the MTD device is
just the parent of the nvmem device.
> Not very complex but still a lot of
> useless code, just registering the MTD device is a lot simpler and much
> more inline with most other kernel API that register a "service"
> available from a device.
I'm not a big fan of this option either, but I thought I had to propose
it.
>
> > 3/ only declare partitions as nvmem providers. This would solve the
> > problem we have with partitions defined in the DT since
> > defining sub-partitions in the DT is not (yet?) supported and
> > partition nodes are supposed to be leaf nodes. Still, I'm not a big
> > fan of this solution because it will prevent us from supporting
> > sub-partitions if we ever want/need to.
>
> That sound like a poor workaround.
Yes, that's a workaround. And the reason I propose it, is, again,
because I don't want to block Bartosz.
> Remember that this problem could
> appear with any device that has a binding that use child nodes.
I'm talking about partitions, and you're talking about mtd devices.
Right now partitions don't have subnodes, and if we define that
partition subnodes should describe nvmem-cells, then it becomes part of
the official binding. So, no, the problem you mention does not (yet)
exist.
>
> > 4/ Add a ->of_xlate() hook that would be called if present by the
> > framework instead of using the default parsing we have right now.
>
> That is a bit cleaner, but I don't think it would be worse the
> complexity.
But it's way more flexible than putting everything in the nvmem
framework. BTW, did you notice that nvmem-cells parsing does not work
with flashes bigger than 4GB, because the framework assumes
#address-cells and #size-cells are always 1. That's probably something
we'll have to fix for the MTD case.
> Furthermore xlate functions are more about converting
> from hardware parameters to internal kernel representation than to hide
> extra DT parsing.
Hm, how is that different? ->of_xlate() is just a way for drivers to
have their own DT representation, which is exactly what we want here.
>
> > 5/ Tell the nvmem framework the name of the subnode containing nvmem
> > cell definitions (if NULL that means cells are directly defined
> > under the nvmem provider node). We would set it to "nvmem-cells" (or
> > whatever you like) for the MTD case.
>
> If so please match on compatible and not on the node name.
If you like.
>
> 6/ Extend the current NVMEM cell lookup to check if the parent node of
> the cell has a compatible string set to "nvmem-cells". If it doesn't it
> mean we have the current binding and this node is the NVMEM device. If
> it does the device node is just the next parent. This is trivial to
> implement (literally 2 lines of code) and cover all the cases currently
> known.
Except Srinivas was not happy with this solution, and this stalled the
discussion. I'm trying to find other options and you keep rejecting all
of them to come back to this one.
>
> 7/ Just add a compatible string to the nvmem cell. No code change is
> needed,
That's not true!!! What forces people to add this compatible in their
DT? Nothing. I'll tell you what will happen: people will start defining
their nvmem cells directly under the MTD node because that *works*, and
even if the binding is not documented and we consider it invalid, we'll
be stuck supporting it forever. As said above, the very reason for
option #1 to exist is to give you and Srinivas some more time to sort
this out, while unblocking Bartosz in the meantime.
> however as the nvmem cells have an address space (the offset in
> byte in the storage) it might still clash with another address space
> used by the main device biding (for example a number of child
> functions).
>
> > There are probably other options (some were proposed by Alban and
> > Srinivas already), but I'd like to get this sorted out before we merge
> > this patch.
> >
> > Alban, Srinivas, any opinion?
>
> My preference goes to 6/ as it is trivial to implement, solves all
> known shortcomings and is backward compatible with the current binding.
> All other solutions have limitations and/or require too complex
> implementations compared to what they try to solve.
So we're back to square 1, and you're again blocking everything because
you refuse to consider other options.
There's obviously nothing more I can do to help, and that's unfortunate
because other people are waiting for this feature.
Regards,
Boris
^ permalink raw reply
* Re: [PATCH] r8169: don't use MSI-X on RTL8106e
From: Marc Zyngier @ 2018-08-21 8:28 UTC (permalink / raw)
To: Bjorn Helgaas, Heiner Kallweit, jian-hong
Cc: David Miller, nic_swsd, netdev, linux-kernel, linux, linux-pci,
Thomas Gleixner, Christoph Hellwig
In-Reply-To: <20180820184438.GA154536@bhelgaas-glaptop.roam.corp.google.com>
On 20/08/18 19:44, Bjorn Helgaas wrote:
> [+cc Marc, Thomas, Christoph, linux-pci)
> (beginning of thread at [1])
>
> On Thu, Aug 16, 2018 at 09:50:48PM +0200, Heiner Kallweit wrote:
>> On 16.08.2018 21:39, David Miller wrote:
>>> From: Heiner Kallweit <hkallweit1@gmail.com>
>>> Date: Thu, 16 Aug 2018 21:37:31 +0200
>>>
>>>> On 16.08.2018 21:21, David Miller wrote:
>>>>> From: <jian-hong@endlessm.com>
>>>>> Date: Wed, 15 Aug 2018 14:21:10 +0800
>>>>>
>>>>>> Found the ethernet network on ASUS X441UAR doesn't come back on resume
>>>>>> from suspend when using MSI-X. The chip is RTL8106e - version 39.
>>>>>
>>>>> Heiner, please take a look at this.
>>>>>
>>>>> You recently disabled MSI-X on RTL8168g for similar reasons.
>>>>>
>>>>> Now that we've seen two chips like this, maybe there is some other
>>>>> problem afoot.
>>>>>
>>>> Thanks for the hint. I saw it already and just contacted Realtek
>>>> whether they are aware of any MSI-X issues with particular chip
>>>> versions. With the chip versions I have access to MSI-X works fine.
>>>>
>>>> There's also the theoretical option that the issues are caused by
>>>> broken BIOS's. But so far only chip versions have been reported
>>>> which are very similar, at least with regard to version number
>>>> (2x VER_40, 1x VER_39). So they may share some buggy component.
>>>>
>>>> Let's see whether Realtek can provide some hint.
>>>> If more chip versions are reported having problems with MSI-X,
>>>> then we could switch to a whitelist or disable MSI-X in general.
>>>
>>> It could be that we need to reprogram some register(s) on resume,
>>> which normally might not be needed, and that is what is causing the
>>> problem with some chips.
>>>
>> Indeed. That's what I'm checking with Realtek.
>> In the register list in the r8169 driver there's one entry which
>> seems to indicate that there are MSI-X specific settings.
>> However this register isn't used, and the r8168 vendor driver
>> uses only MSI. And there are no public datasheets.
>
> Do we have any information about these chip versions in other systems?
> Or other devices using MSI-X in the same ASUS system? It seems
> possible that there's some PCI core or suspend/resume issue with MSI-X
> and this patch just avoids it without fixing the root cause.
>
> It might be useful to have a kernel.org bugzilla with the complete
> dmesg, "sudo lspci -vv" output, and /proc/interrupts contents archived
> for future reference.
The one system I have with a Realtek chip seems happy enough with MSI-X,
but it never gets suspended. There is comment in the patch that I don't
quite get:
> It is the IRQ 127 - PCI-MSI used by enp2s0. However, lspci lists MSI is
> disabled and MSI-X is enabled which conflicts to the interrupt table.
What do you mean by "conflicts"? With what? Another question is whether
you've loaded any firmware (some versions of the Realtek HW seem to require
it).
For the posterity, some data from my own system, which I don't know if it
has any relevance to the problem at hand.
Thanks,
M.
[ 2.624963] r8169 0000:02:00.0 eth0: RTL8168g/8111g, 5a:fe:ad:ce:11:00, XID 4c000800, IRQ 26
[ 2.633398] r8169 0000:02:00.0 eth0: jumbo features [frames: 9200 bytes, tx checksumming: ko]
26: 50 997005 0 0 MSI 1048576 Edge enp2s0
02:00.0 Ethernet controller: Realtek Semiconductor Co., Ltd. RTL8111/8168/8411 PCI Express Gigabit Ethernet Controller (rev 0c)
Subsystem: Realtek Semiconductor Co., Ltd. RTL8111/8168 PCI Express Gigabit Ethernet controller
Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx+
Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-
Latency: 0, Cache Line Size: 64 bytes
Interrupt: pin A routed to IRQ 25
Region 0: I/O ports at 1000 [size=256]
Region 2: Memory at 100004000 (64-bit, prefetchable) [size=4K]
Region 4: Memory at 100000000 (64-bit, prefetchable) [size=16K]
Capabilities: [40] Power Management version 3
Flags: PMEClk- DSI- D1+ D2+ AuxCurrent=375mA PME(D0+,D1+,D2+,D3hot+,D3cold+)
Status: D0 NoSoftRst+ PME-Enable- DSel=0 DScale=0 PME-
Capabilities: [50] MSI: Enable- Count=1/1 Maskable- 64bit+
Address: 0000000000000000 Data: 0000
Capabilities: [70] Express (v2) Endpoint, MSI 01
DevCap: MaxPayload 128 bytes, PhantFunc 0, Latency L0s <512ns, L1 <64us
ExtTag- AttnBtn- AttnInd- PwrInd- RBE+ FLReset- SlotPowerLimit 0.000W
DevCtl: Report errors: Correctable- Non-Fatal- Fatal- Unsupported-
RlxdOrd+ ExtTag- PhantFunc- AuxPwr- NoSnoop-
MaxPayload 128 bytes, MaxReadReq 4096 bytes
DevSta: CorrErr+ UncorrErr- FatalErr- UnsuppReq+ AuxPwr+ TransPend-
LnkCap: Port #0, Speed 2.5GT/s, Width x1, ASPM L0s L1, Exit Latency L0s unlimited, L1 <64us
ClockPM+ Surprise- LLActRep- BwNot- ASPMOptComp+
LnkCtl: ASPM Disabled; RCB 64 bytes Disabled- CommClk+
ExtSynch- ClockPM- AutWidDis- BWInt- AutBWInt-
LnkSta: Speed 2.5GT/s, Width x1, TrErr- Train- SlotClk+ DLActive- BWMgmt- ABWMgmt-
DevCap2: Completion Timeout: Range ABCD, TimeoutDis+, LTR+, OBFF Via message/WAKE#
DevCtl2: Completion Timeout: 50us to 50ms, TimeoutDis-, LTR-, OBFF Disabled
LnkCtl2: Target Link Speed: 2.5GT/s, EnterCompliance- SpeedDis-
Transmit Margin: Normal Operating Range, EnterModifiedCompliance- ComplianceSOS-
Compliance De-emphasis: -6dB
LnkSta2: Current De-emphasis Level: -6dB, EqualizationComplete-, EqualizationPhase1-
EqualizationPhase2-, EqualizationPhase3-, LinkEqualizationRequest-
Capabilities: [b0] MSI-X: Enable+ Count=4 Masked-
Vector table: BAR=4 offset=00000000
PBA: BAR=4 offset=00000800
Capabilities: [d0] Vital Product Data
pcilib: sysfs_read_vpd: read failed: Input/output error
Not readable
Capabilities: [100 v1] Advanced Error Reporting
UESta: DLP- SDES- TLP- FCP- CmpltTO- CmpltAbrt- UnxCmplt- RxOF- MalfTLP- ECRC- UnsupReq- ACSViol-
UEMsk: DLP- SDES- TLP- FCP- CmpltTO- CmpltAbrt- UnxCmplt- RxOF- MalfTLP- ECRC- UnsupReq- ACSViol-
UESvrt: DLP+ SDES+ TLP- FCP+ CmpltTO- CmpltAbrt- UnxCmplt- RxOF+ MalfTLP+ ECRC- UnsupReq- ACSViol-
CESta: RxErr+ BadTLP- BadDLLP- Rollover- Timeout- NonFatalErr-
CEMsk: RxErr- BadTLP- BadDLLP- Rollover- Timeout- NonFatalErr+
AERCap: First Error Pointer: 00, GenCap+ CGenEn- ChkCap+ ChkEn-
Capabilities: [140 v1] Virtual Channel
Caps: LPEVC=0 RefClk=100ns PATEntryBits=1
Arb: Fixed- WRR32- WRR64- WRR128-
Ctrl: ArbSelect=Fixed
Status: InProgress-
VC0: Caps: PATOffset=00 MaxTimeSlots=1 RejSnoopTrans-
Arb: Fixed- WRR32- WRR64- WRR128- TWRR128- WRR256-
Ctrl: Enable+ ID=0 ArbSelect=Fixed TC/VC=ff
Status: NegoPending- InProgress-
Capabilities: [160 v1] Device Serial Number 00-00-00-00-00-00-00-00
Capabilities: [170 v1] Latency Tolerance Reporting
Max snoop latency: 0ns
Max no snoop latency: 0ns
Kernel driver in use: r8169
--
Jazz is not dead. It just smells funny...
^ permalink raw reply
* Charity Project
From: Wang Jianlin @ 2018-08-20 23:11 UTC (permalink / raw)
I intend to give you a portion of my wealth as a free-will financial
donation to you. Respond to partake.
Wang Jianlin
Dalian Wanda Group
^ permalink raw reply
* Re: [bpf-next RFC 0/3] Introduce eBPF flow dissector
From: David Miller @ 2018-08-21 2:24 UTC (permalink / raw)
To: alexei.starovoitov
Cc: peterpenkov96, netdev, ast, daniel, simon.horman, ppenkov,
willemb
In-Reply-To: <20180820205205.jj7e75pulwqkorpr@ast-mbp>
From: Alexei Starovoitov <alexei.starovoitov@gmail.com>
Date: Mon, 20 Aug 2018 13:52:07 -0700
> I don't think copy-paste avoids the issue of uapi.
> Anything used by BPF program is uapi.
> The only exception is offsets of kernel internal structures
> passed into bpf_probe_read().
> So we have several options:
> 1. be honest and say 'struct flow_dissect_key*' is now uapi
> 2. wrap all of them into 'struct bpf_flow_dissect_key*' and do rewrites
> when/if 'struct flow_dissect_key*' changes
> 3. wait for BTF to solve it for tracing use case and for this one two.
...
> The idea is that kernel internal structs can be defined in bpf prog
> and since they will be described precisely in BTF that comes with the prog
> the kernel can validate that prog's BTF matches what kernel thinks it has.
> imo that's the most flexible, but BTF for all of vmlinux won't be ready
> tomorrow and looks like this patch set is ready to go, so I would go with 1 or 2.
I would definitely prefer #2 or #3.
I personally would like to see us avoid preventing interesting
optimizations of the flow key layout and/or accesses in the future.
^ permalink raw reply
* Re: unregister_netdevice: waiting for DEV to become free (2)
From: Cong Wang @ 2018-08-21 5:40 UTC (permalink / raw)
To: Julian Anastasov
Cc: syzbot+30209ea299c09d8785c9, ddstreet, Dmitry Vyukov, LKML,
Linux Kernel Network Developers, syzkaller-bugs
In-Reply-To: <alpine.LFD.2.20.1808201527230.2758@ja.home.ssi.bg>
On Mon, Aug 20, 2018 at 6:00 AM Julian Anastasov <ja@ssi.bg> wrote:
>
>
> Hello,
>
> On Sun, 19 Aug 2018, syzbot wrote:
>
> > syzbot has found a reproducer for the following crash on:
> >
> > HEAD commit: d7857ae43dcc Add linux-next specific files for 20180817
> > git tree: linux-next
> > console output: https://syzkaller.appspot.com/x/log.txt?x=13c72fce400000
> > kernel config: https://syzkaller.appspot.com/x/.config?x=4b10cd1ea76bb092
> > dashboard link: https://syzkaller.appspot.com/bug?extid=30209ea299c09d8785c9
> > compiler: gcc (GCC) 8.0.1 20180413 (experimental)
> > syzkaller repro:https://syzkaller.appspot.com/x/repro.syz?x=15df679a400000
> > C reproducer: https://syzkaller.appspot.com/x/repro.c?x=15242741400000
> >
> > IMPORTANT: if you fix the bug, please add the following tag to the commit:
> > Reported-by: syzbot+30209ea299c09d8785c9@syzkaller.appspotmail.com
> >
> > IPVS: stopping master sync thread 4657 ...
> > IPVS: stopping master sync thread 4663 ...
> > IPVS: sync thread started: state = MASTER, mcast_ifn = syz_tun, syncid = 0, id
> > IPVS: = 0
> > IPVS: sync thread started: state = MASTER, mcast_ifn = syz_tun, syncid = 0, id
> > IPVS: = 0
> > IPVS: stopping master sync thread 4664 ...
> > unregister_netdevice: waiting for lo to become free. Usage count = 1
>
> Well, only IPVS and tun in the game? But IPVS does not
> take any dev references for sync threads. Can it be a problem
> in tun? For example, a side effects from dst_cache_reset?
> May be dst_release is called too late? Here is what should happen
> on unregistration:
There are multiple similar bugs grouped together under this, perhaps
they are different, perhaps they are a same bug, too early to say.
For the one I look into, dst_cache doesn't matter, because the xmit
path doesn't even use tunnel dst_cache at all, and it is ip6tnl0 FB
device, unlike this one which is tun device.
>
> - NETDEV_UNREGISTER event: rt_flush_dev changes dst->dev with lo
> but dst is not released
>
> - ndo_uninit/ip_tunnel_uninit: dst_cache_reset is called which
> does nothing!?! May be dst_release call is needed here.
I think this makes sense, at least prior to the general dst_cache
introduction, dst refcnt was released in ndo_uninit() too, so it
is reasonable to move the dst_cache_destroy() to ndo_uninit().
>
> - no more references are expected here ...
>
> - netdev_run_todo -> netdev_wait_allrefs: loop here due to refcnt!=0
>
> - dev->priv_destructor (ip_tunnel_dev_free) calls dst_cache_destroy
> where dst_release is used but it is not reached because we loop in
> netdev_wait_allrefs above
>
> - dst_cache_destroy: really call dst_release
>
> In fact, after calling rt_flush_dev and replacing the
> dst->dev we should reach dev->priv_destructor (ip_tunnel_dev_free)
> for tun device where dst_release for lo should be called. But may be
> something prevents it, exit batching?
I can't see anything in netnns exit batch is any special here.
For the one I look into, it seems some fib6_info is not released for
some reason. It seems to be the one created by addrconf_prefix_route(),
which is supposed to be released by fib6_clean_tree() I think, but it
never happens.
Thanks.
^ permalink raw reply
* Re: [PATCH] rhashtable: remove duplicated include from rhashtable.c
From: David Miller @ 2018-08-21 2:19 UTC (permalink / raw)
To: yuehaibing; +Cc: tgraf, herbert, netdev, kernel-janitors
In-Reply-To: <1534815716-102071-1-git-send-email-yuehaibing@huawei.com>
From: Yue Haibing <yuehaibing@huawei.com>
Date: Tue, 21 Aug 2018 01:41:56 +0000
> Remove duplicated include.
>
> Signed-off-by: Yue Haibing <yuehaibing@huawei.com>
Applied, thank you.
^ permalink raw reply
* Re: KASAN: use-after-free Read in tipc_group_fill_sock_diag
From: Cong Wang @ 2018-08-21 5:29 UTC (permalink / raw)
To: syzbot+b9c8f3ab2994b7cd1625
Cc: David Miller, Jon Maloy, LKML, Linux Kernel Network Developers,
syzkaller-bugs, tipc-discussion, Ying Xue
In-Reply-To: <00000000000038cc590573be9db2@google.com>
On Sat, Aug 18, 2018 at 5:10 PM syzbot
<syzbot+b9c8f3ab2994b7cd1625@syzkaller.appspotmail.com> wrote:
>
> Hello,
>
> syzbot found the following crash on:
>
> HEAD commit: a18d783fedfe Merge tag 'driver-core-4.19-rc1' of git://git..
> git tree: upstream
> console output: https://syzkaller.appspot.com/x/log.txt?x=14bcc936400000
> kernel config: https://syzkaller.appspot.com/x/.config?x=7eeabb17332898c0
> dashboard link: https://syzkaller.appspot.com/bug?extid=b9c8f3ab2994b7cd1625
> compiler: gcc (GCC) 8.0.1 20180413 (experimental)
> syzkaller repro:https://syzkaller.appspot.com/x/repro.syz?x=1374ec4a400000
>
> IMPORTANT: if you fix the bug, please add the following tag to the commit:
> Reported-by: syzbot+b9c8f3ab2994b7cd1625@syzkaller.appspotmail.com
>
> IPv6: ADDRCONF(NETDEV_UP): veth1: link is not ready
> IPv6: ADDRCONF(NETDEV_CHANGE): veth1: link becomes ready
> IPv6: ADDRCONF(NETDEV_CHANGE): veth0: link becomes ready
> 8021q: adding VLAN 0 to HW filter on device team0
> ==================================================================
> BUG: KASAN: use-after-free in tipc_group_fill_sock_diag+0x7b9/0x84b
> net/tipc/group.c:921
> Read of size 4 at addr ffff8801ceb5565c by task syz-executor0/4838
>
> CPU: 1 PID: 4838 Comm: syz-executor0 Not tainted 4.18.0+ #196
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
> Google 01/01/2011
> Call Trace:
> __dump_stack lib/dump_stack.c:77 [inline]
> dump_stack+0x1c9/0x2b4 lib/dump_stack.c:113
> print_address_description+0x6c/0x20b mm/kasan/report.c:256
> kasan_report_error mm/kasan/report.c:354 [inline]
> kasan_report.cold.7+0x242/0x30d mm/kasan/report.c:412
> __asan_report_load4_noabort+0x14/0x20 mm/kasan/report.c:432
> tipc_group_fill_sock_diag+0x7b9/0x84b net/tipc/group.c:921
> tipc_sk_fill_sock_diag+0x9f8/0xdb0 net/tipc/socket.c:3322
> __tipc_add_sock_diag+0x22f/0x360 net/tipc/diag.c:62
> tipc_nl_sk_walk+0x68d/0xd30 net/tipc/socket.c:3249
> tipc_diag_dump+0x24/0x30 net/tipc/diag.c:73
Seems we need just unlink the tipc sock before deleting group,
otherwise probably have to take a refcnt of group.
For example, moving tipc_sk_leave() down after tipc_sk_remove()?
--- a/net/tipc/socket.c
+++ b/net/tipc/socket.c
@@ -571,10 +571,10 @@ static int tipc_release(struct socket *sock)
__tipc_shutdown(sock, TIPC_ERR_NO_PORT);
sk->sk_shutdown = SHUTDOWN_MASK;
- tipc_sk_leave(tsk);
tipc_sk_withdraw(tsk, 0, NULL);
sk_stop_timer(sk, &sk->sk_timer);
tipc_sk_remove(tsk);
+ tipc_sk_leave(tsk);
/* Reject any messages that accumulated in backlog queue */
release_sock(sk);
> netlink_dump+0x519/0xd50 net/netlink/af_netlink.c:2233
> __netlink_dump_start+0x4f1/0x6f0 net/netlink/af_netlink.c:2329
> netlink_dump_start include/linux/netlink.h:213 [inline]
> tipc_sock_diag_handler_dump+0x234/0x340 net/tipc/diag.c:89
> __sock_diag_cmd net/core/sock_diag.c:232 [inline]
> sock_diag_rcv_msg+0x31d/0x410 net/core/sock_diag.c:263
> netlink_rcv_skb+0x172/0x440 net/netlink/af_netlink.c:2454
> sock_diag_rcv+0x2a/0x40 net/core/sock_diag.c:274
> netlink_unicast_kernel net/netlink/af_netlink.c:1317 [inline]
> netlink_unicast+0x5a0/0x760 net/netlink/af_netlink.c:1343
> netlink_sendmsg+0xa18/0xfc0 net/netlink/af_netlink.c:1908
> sock_sendmsg_nosec net/socket.c:621 [inline]
> sock_sendmsg+0xd5/0x120 net/socket.c:631
> ___sys_sendmsg+0x7fd/0x930 net/socket.c:2114
> __sys_sendmsg+0x11d/0x290 net/socket.c:2152
> __do_sys_sendmsg net/socket.c:2161 [inline]
> __se_sys_sendmsg net/socket.c:2159 [inline]
> __x64_sys_sendmsg+0x78/0xb0 net/socket.c:2159
> do_syscall_64+0x1b9/0x820 arch/x86/entry/common.c:290
> entry_SYSCALL_64_after_hwframe+0x49/0xbe
> RIP: 0033:0x457089
> Code: fd b4 fb ff c3 66 2e 0f 1f 84 00 00 00 00 00 66 90 48 89 f8 48 89 f7
> 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff
> ff 0f 83 cb b4 fb ff c3 66 2e 0f 1f 84 00 00 00 00
> RSP: 002b:00007f3a5d506c78 EFLAGS: 00000246 ORIG_RAX: 000000000000002e
> RAX: ffffffffffffffda RBX: 00007f3a5d5076d4 RCX: 0000000000457089
> RDX: 0000000000000000 RSI: 0000000020000040 RDI: 0000000000000006
> RBP: 00000000009300a0 R08: 0000000000000000 R09: 0000000000000000
> R10: 0000000000000000 R11: 0000000000000246 R12: 00000000ffffffff
> R13: 00000000004d4088 R14: 00000000004c8ab0 R15: 0000000000000000
>
> Allocated by task 4838:
> save_stack+0x43/0xd0 mm/kasan/kasan.c:448
> set_track mm/kasan/kasan.c:460 [inline]
> kasan_kmalloc+0xc4/0xe0 mm/kasan/kasan.c:553
> kmem_cache_alloc_trace+0x152/0x780 mm/slab.c:3620
> kmalloc include/linux/slab.h:513 [inline]
> kzalloc include/linux/slab.h:707 [inline]
> tipc_group_create+0x155/0xa70 net/tipc/group.c:171
> tipc_sk_join net/tipc/socket.c:2766 [inline]
> tipc_setsockopt+0x2d1/0xd70 net/tipc/socket.c:2881
> __sys_setsockopt+0x1c5/0x3b0 net/socket.c:1900
> __do_sys_setsockopt net/socket.c:1911 [inline]
> __se_sys_setsockopt net/socket.c:1908 [inline]
> __x64_sys_setsockopt+0xbe/0x150 net/socket.c:1908
> do_syscall_64+0x1b9/0x820 arch/x86/entry/common.c:290
> entry_SYSCALL_64_after_hwframe+0x49/0xbe
>
> Freed by task 4837:
> save_stack+0x43/0xd0 mm/kasan/kasan.c:448
> set_track mm/kasan/kasan.c:460 [inline]
> __kasan_slab_free+0x11a/0x170 mm/kasan/kasan.c:521
> kasan_slab_free+0xe/0x10 mm/kasan/kasan.c:528
> __cache_free mm/slab.c:3498 [inline]
> kfree+0xd9/0x260 mm/slab.c:3813
> tipc_group_delete+0x2e5/0x3f0 net/tipc/group.c:227
> tipc_sk_leave+0x113/0x220 net/tipc/socket.c:2800
> tipc_release+0x14e/0x12b0 net/tipc/socket.c:574
> __sock_release+0xd7/0x250 net/socket.c:579
> sock_close+0x19/0x20 net/socket.c:1139
> __fput+0x39b/0x860 fs/file_table.c:251
> ____fput+0x15/0x20 fs/file_table.c:282
> task_work_run+0x1e8/0x2a0 kernel/task_work.c:113
> tracehook_notify_resume include/linux/tracehook.h:193 [inline]
> exit_to_usermode_loop+0x318/0x380 arch/x86/entry/common.c:166
> prepare_exit_to_usermode arch/x86/entry/common.c:197 [inline]
> syscall_return_slowpath arch/x86/entry/common.c:268 [inline]
> do_syscall_64+0x6be/0x820 arch/x86/entry/common.c:293
> entry_SYSCALL_64_after_hwframe+0x49/0xbe
>
> The buggy address belongs to the object at ffff8801ceb55600
> which belongs to the cache kmalloc-192 of size 192
> The buggy address is located 92 bytes inside of
> 192-byte region [ffff8801ceb55600, ffff8801ceb556c0)
> The buggy address belongs to the page:
> page:ffffea00073ad540 count:1 mapcount:0 mapping:ffff8801dac00040 index:0x0
> flags: 0x2fffc0000000100(slab)
> raw: 02fffc0000000100 ffffea00073a5088 ffffea00073ae5c8 ffff8801dac00040
> raw: 0000000000000000 ffff8801ceb55000 0000000100000010 0000000000000000
> page dumped because: kasan: bad access detected
>
> Memory state around the buggy address:
> ffff8801ceb55500: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> ffff8801ceb55580: 00 fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
> > ffff8801ceb55600: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> ^
> ffff8801ceb55680: fb fb fb fb fb fb fb fb fc fc fc fc fc fc fc fc
> ffff8801ceb55700: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> ==================================================================
>
>
> ---
> This bug is generated by a bot. It may contain errors.
> See https://goo.gl/tpsmEJ for more information about syzbot.
> syzbot engineers can be reached at syzkaller@googlegroups.com.
>
> syzbot will keep track of this bug report. See:
> https://goo.gl/tpsmEJ#bug-status-tracking for how to communicate with
> syzbot.
> syzbot can test patches for this bug, for details see:
> https://goo.gl/tpsmEJ#testing-patches
^ permalink raw reply
* Re: [PATCH v2 06/29] mtd: Add support for reading MTD devices via the nvmem API
From: Boris Brezillon @ 2018-08-21 5:07 UTC (permalink / raw)
To: Alban, Srinivas Kandagatla, Rob Herring
Cc: Bartosz Golaszewski, Jonathan Corbet, Sekhar Nori, Kevin Hilman,
Russell King, Arnd Bergmann, Greg Kroah-Hartman, David Woodhouse,
Brian Norris, Marek Vasut, Richard Weinberger, Grygorii Strashko,
David S . Miller, Naren, Mauro Carvalho Chehab, Andrew Morton,
Lukas Wunner, Dan Carpenter, Florian Fainelli, Ivan
In-Reply-To: <20180820232748.608c3667@tock>
On Mon, 20 Aug 2018 23:27:48 +0200
Alban <albeu@free.fr> wrote:
> On Mon, 20 Aug 2018 20:20:38 +0200
> Boris Brezillon <boris.brezillon@bootlin.com> wrote:
>
> > On Mon, 20 Aug 2018 11:43:34 +0100
> > Srinivas Kandagatla <srinivas.kandagatla@linaro.org> wrote:
> >
> > >
> > > Overall am still not able to clear visualize on how MTD bindings with
> > > nvmem cells would look in both partition and un-partition usecases?
> > > An example DT would be nice here!!
> >
> > Something along those lines:
>
> We must also have a compatible string on the nvmem-cells node to make
> sure we don't clash with the old style MTD partitions,
That's not possible, because we don't have a reg prop in the
nvmem-cells node.
> or some other
> device specific binding.
This one might happen.
Was Rob okay with this compatible? If he was, I guess we can go for
this binding. Srinivas, any objection?
>
> >
> > mtdnode {
> > nvmem-cells {
> compatible = "nvmem-cells";
> > #address-cells = <1>;
> > #size-cells = <1>;
> >
> > cell@0 {
> > reg = <0x0 0x14>;
> > };
> > };
> >
> > partitions {
> > compatible = "fixed-partitions";
> > #address-cells = <1>;
> > #size-cells = <1>;
> >
> > partition@0 {
> > reg = <0x0 0x20000>;
> >
> > nvmem-cells {
> compatible = "nvmem-cells";
> > #address-cells = <1>;
> > #size-cells = <1>;
> >
> > cell@0 {
> > reg = <0x0 0x10>;
> > };
> > };
> > };
> > };
> > };
>
>
> Alban
^ permalink raw reply
* [PATCH] rhashtable: remove duplicated include from rhashtable.c
From: Yue Haibing @ 2018-08-21 1:41 UTC (permalink / raw)
To: Thomas Graf, Herbert Xu; +Cc: Yue Haibing, netdev, kernel-janitors
Remove duplicated include.
Signed-off-by: Yue Haibing <yuehaibing@huawei.com>
---
lib/rhashtable.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/lib/rhashtable.c b/lib/rhashtable.c
index ae4223e..672eecd 100644
--- a/lib/rhashtable.c
+++ b/lib/rhashtable.c
@@ -28,7 +28,6 @@
#include <linux/rhashtable.h>
#include <linux/err.h>
#include <linux/export.h>
-#include <linux/rhashtable.h>
#define HASH_DEFAULT_SIZE 64UL
#define HASH_MIN_SIZE 4U
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox