* Re: [PATCH 1/2] vhost-vsock: remove unused vq variable
From: David Miller @ 2016-12-08 16:30 UTC (permalink / raw)
To: bergwolf; +Cc: stefanha, kvm, virtualization, netdev
In-Reply-To: <1481104340-77035-1-git-send-email-bergwolf@gmail.com>
From: Peng Tao <bergwolf@gmail.com>
Date: Wed, 7 Dec 2016 17:52:18 +0800
> Signed-off-by: Peng Tao <bergwolf@gmail.com>
Applied.
^ permalink raw reply
* Re: [PATCH] sh_eth: add wake-on-lan support via magic packet
From: Sergei Shtylyov @ 2016-12-08 16:29 UTC (permalink / raw)
To: Niklas Söderlund, Simon Horman, netdev, linux-renesas-soc
In-Reply-To: <59a1f246-9062-20e1-3f85-a1c5f6fcfc29@cogentembedded.com>
On 12/08/2016 03:28 PM, Sergei Shtylyov wrote:
> Good to see that somebody cares still about this driver, one more task off
> my back. :-)
>
> On 12/07/2016 07:28 PM, Niklas Söderlund wrote:
>
> You only enable the WOL support fo the R-Car gen2 chips but never say that
> explicitly, neither in the subject nor here.
Some patch description wouldn't hurt here, especially with the way you
implemented this support, e.g. RPM vs clk API -- that needs some explanation...
>> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
>> ---
>> drivers/net/ethernet/renesas/sh_eth.c | 120 +++++++++++++++++++++++++++++++---
>> drivers/net/ethernet/renesas/sh_eth.h | 4 ++
>> 2 files changed, 116 insertions(+), 8 deletions(-)
>> diff --git a/drivers/net/ethernet/renesas/sh_eth.c
>> b/drivers/net/ethernet/renesas/sh_eth.c
>> index 05b0dc5..3974046 100644
>> --- a/drivers/net/ethernet/renesas/sh_eth.c
>> +++ b/drivers/net/ethernet/renesas/sh_eth.c
[...]
>> diff --git a/drivers/net/ethernet/renesas/sh_eth.h
>> b/drivers/net/ethernet/renesas/sh_eth.h
>> index d050f37..26c6620 100644
>> --- a/drivers/net/ethernet/renesas/sh_eth.h
>> +++ b/drivers/net/ethernet/renesas/sh_eth.h
>> @@ -493,6 +493,7 @@ struct sh_eth_cpu_data {
>> unsigned shift_rd0:1; /* shift Rx descriptor word 0 right by 16 */
>> unsigned rmiimode:1; /* EtherC has RMIIMODE register */
>> unsigned rtrate:1; /* EtherC has RTRATE register */
>> + unsigned magic:1; /* EtherC have PMDE in ECMR and MPDIP in ECSIPR */
>
> OK, e.g. RZ/A1 doesn't have these bits...
However, I'd prefer that the comment be reworded as such:
/* EtherC has ECMR.PMDE and ECSR.MPD */
or
/* EtherC has ECMR_PMDE and ECSR_MPD */
MBR, Sergei
^ permalink raw reply
* Re: [PATCH net] phy: Don't increment MDIO bus refcount unless it's a different owner
From: Johan Hovold @ 2016-12-08 16:27 UTC (permalink / raw)
To: Florian Fainelli; +Cc: netdev, johan, rmk+kernel, andrew
In-Reply-To: <20161207045443.26246-1-f.fainelli@gmail.com>
On Tue, Dec 06, 2016 at 08:54:43PM -0800, Florian Fainelli wrote:
> Commit 3e3aaf649416 ("phy: fix mdiobus module safety") fixed the way we
> dealt with MDIO bus module reference count, but sort of introduced a
> regression in that, if an Ethernet driver registers its own MDIO bus
> driver, as is common, we will end up with the Ethernet driver's
> module->refnct set to 1, thus preventing this driver from any removal.
>
> Fix this by comparing the network device's device driver owner against
> the MDIO bus driver owner, and only if they are different, increment the
> MDIO bus module refcount.
>
> Fixes: 3e3aaf649416 ("phy: fix mdiobus module safety")
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> ---
> Russell,
>
> I verified this against the ethoc driver primarily (on a TS7300 board)
> and bcmgenet.
>
> Thanks!
>
> drivers/net/phy/phy_device.c | 16 +++++++++++++---
> 1 file changed, 13 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
> index 1a4bf8acad78..c4ceb082e970 100644
> --- a/drivers/net/phy/phy_device.c
> +++ b/drivers/net/phy/phy_device.c
> @@ -857,11 +857,17 @@ EXPORT_SYMBOL(phy_attached_print);
> int phy_attach_direct(struct net_device *dev, struct phy_device *phydev,
> u32 flags, phy_interface_t interface)
> {
> + struct module *ndev_owner = dev->dev.parent->driver->owner;
Is this really safe? A driver does not need to set a parent device, and
in that case you get a NULL-deref here (I tried using cpsw).
> struct mii_bus *bus = phydev->mdio.bus;
> struct device *d = &phydev->mdio.dev;
> int err;
>
> - if (!try_module_get(bus->owner)) {
> + /* For Ethernet device drivers that register their own MDIO bus, we
> + * will have bus->owner match ndev_mod, so we do not want to increment
You also wanted s/ndev_mod/ndev_owner/ here.
> + * our own module->refcnt here, otherwise we would not be able to
> + * unload later on.
> + */
> + if (ndev_owner != bus->owner && !try_module_get(bus->owner)) {
> dev_err(&dev->dev, "failed to get the bus module\n");
> return -EIO;
Johan
^ permalink raw reply
* Re: [net-next] icmp: correct return value of icmp_rcv()
From: David Miller @ 2016-12-08 16:24 UTC (permalink / raw)
To: zhangshengju; +Cc: netdev
In-Reply-To: <1481093573-5123-1-git-send-email-zhangshengju@cmss.chinamobile.com>
From: Zhang Shengju <zhangshengju@cmss.chinamobile.com>
Date: Wed, 7 Dec 2016 14:52:53 +0800
> Currently, icmp_rcv() always return zero on a packet delivery upcall.
>
> To make its behavior more compliant with the way this API should be
> used, this patch changes this to let it return NET_RX_SUCCESS when the
> packet is proper handled, and NET_RX_DROP otherwise.
>
> Signed-off-by: Zhang Shengju <zhangshengju@cmss.chinamobile.com>
Applied.
^ permalink raw reply
* Re: [PATCH] linux/types.h: enable endian checks for all sparse builds
From: Michael S. Tsirkin @ 2016-12-08 16:17 UTC (permalink / raw)
To: Bart Van Assche
Cc: linux-kernel@vger.kernel.org, Linus Torvalds, Christoph Hellwig,
Jason Wang, linux-kbuild@vger.kernel.org, Michal Marek,
Arnd Bergmann, Greg Kroah-Hartman, Matt Mackall, Herbert Xu,
David Airlie, Gerd Hoffmann, Ohad Ben-Cohen,
Christian Borntraeger, Cornelia Huck, James E.J. Bottomley,
David S. Miller, Jens Axboe, Neil Armstrong <narmstron
In-Reply-To: <BLUPR02MB168304F8FBA50C916A6A4E6081840@BLUPR02MB1683.namprd02.prod.outlook.com>
On Thu, Dec 08, 2016 at 06:38:11AM +0000, Bart Van Assche wrote:
> On 12/07/16 21:54, Michael S. Tsirkin wrote:
> > On Thu, Dec 08, 2016 at 05:21:47AM +0000, Bart Van Assche wrote:
> >> Additionally, there are notable exceptions to the rule that most drivers
> >> are endian-clean, e.g. drivers/scsi/qla2xxx. I would appreciate it if it
> >> would remain possible to check such drivers with sparse without enabling
> >> endianness checks. Have you considered to change #ifdef __CHECK_ENDIAN__
> >> into e.g. #ifndef __DONT_CHECK_ENDIAN__?
> >
> > The right thing is probably just to fix these, isn't it?
> > Until then, why not just ignore the warnings?
>
> Neither option is realistic. With endian-checking enabled the qla2xxx
> driver triggers so many warnings that it becomes a real challenge to
> filter the non-endian warnings out manually:
>
> $ for f in "" CF=-D__CHECK_ENDIAN__; do make M=drivers/scsi/qla2xxx C=2\
> $f | &grep -c ': warning:'; done
> 4
> 752
You can always revert this patch in your tree, or whatever. It does not
look like this will get fixed otherwise.
> If you think it would be easy to fix the endian warnings triggered by
> the qla2xxx driver, you are welcome to try to fix these.
>
> Bart.
Yea, this hardware was designed by someone who thought mixing
LE and BE all over the place is a good idea.
But who said it should be easy?
Maybe this change will be enough to motivate the maintainers.
Here's a minor buglet for you as a motivator:
if (ct_rsp->header.response !=
cpu_to_be16(CT_ACCEPT_RESPONSE)) {
ql_dbg(ql_dbg_disc + ql_dbg_buffer, vha, 0x2077,
"%s failed rejected request on port_id: %02x%02x%02x Compeltion status 0x%x, response 0x%x\n",
routine, vha->d_id.b.domain,
vha->d_id.b.area, vha->d_id.b.al_pa, comp_status, ct_rsp->header.response);
response is BE and isn't printed correctly.
another:
eiter->a.max_frame_size = cpu_to_be32(eiter->a.max_frame_size);
size += 4 + 4;
ql_dbg(ql_dbg_disc, vha, 0x20bc,
"Max_Frame_Size = %x.\n", eiter->a.max_frame_size);
printed too late, it's be by that time.
Here's another suspicious line
ctio24->u.status1.flags = (atio->u.isp24.attr << 9) |
cpu_to_le16(CTIO7_FLAGS_STATUS_MODE_1 |
CTIO7_FLAGS_TERMINATE);
shifting attr by 9 bits gives different results on BE and LE,
mixing it with le16 looks rather strange.
Another:
ha->flags.dport_enabled =
(mid_init_cb->init_cb.firmware_options_1 & BIT_7) != 0;
BIT_7 is native endian, firmware_options_1 is LE I think.
Look at qla27xx_find_valid_image as well.
if (pri_image_status.signature != QLA27XX_IMG_STATUS_SIGN)
qla27xx_image_status seems to be data coming from flash, but is
somehow native-endian? Maybe ...
lun = a->u.isp24.fcp_cmnd.lun;
I think lun here is in hardware format (le?), code treats it
as native.
Not to speak about interface abuse all over the place.
How about this:
uint32_t *
qla24xx_read_flash_data(scsi_qla_host_t *vha, uint32_t *dwptr, uint32_t
faddr,
uint32_t dwords)
{
uint32_t i;
struct qla_hw_data *ha = vha->hw;
/* Dword reads to flash. */
for (i = 0; i < dwords; i++, faddr++)
dwptr[i] = cpu_to_le32(qla24xx_read_flash_dword(ha,
flash_data_addr(ha, faddr)));
return dwptr;
}
OK so we convert to LE ...
qla24xx_read_flash_data(vha, dcode, faddr, 4);
risc_addr = be32_to_cpu(dcode[2]);
*srisc_addr = *srisc_addr == 0 ? risc_addr : *srisc_addr;
risc_size = be32_to_cpu(dcode[3]);
then happily assume it's BE.
And again, coming from flash, it's unlikely to actually be in the native
endian-ness as callers seem to assume. I'm guessing it's all BE.
I poked at it a bit and was able to cut down # of warnings
from 1700 to 1400 in an hour. Someone familiar with the code
should look at it.
--
MST
^ permalink raw reply
* Re: net: deadlock on genl_mutex
From: Dmitry Vyukov @ 2016-12-08 16:16 UTC (permalink / raw)
To: syzkaller
Cc: Eric Dumazet, David Miller, Matti Vaittinen, Tycho Andersen,
Cong Wang, Florian Westphal, stephen hemminger, Tom Herbert,
netdev, LKML, Richard Guy Briggs, netdev-owner
In-Reply-To: <0227d7e83cc5ac0a192d1ba0fee61413@codeaurora.org>
On Tue, Nov 29, 2016 at 6:59 AM, <subashab@codeaurora.org> wrote:
>>
>> Issue was reported yesterday and is under investigation.
>>
>>
>> http://marc.info/?l=linux-netdev&m=148014004331663&w=2
>>
>>
>> Thanks !
>
>
> Hi Dmitry
>
> Can you try the patch below with your reproducer? I haven't seen similar
> crashes reported after this (or even with Eric's patch).
I've synced to 318c8932ddec5c1c26a4af0f3c053784841c598e (Dec 7) and do
_not_ see this report happening anymore.
Thanks.
^ permalink raw reply
* Re: [PATCH net-next] udp: under rx pressure, try to condense skbs
From: Eric Dumazet @ 2016-12-08 16:08 UTC (permalink / raw)
To: Rick Jones; +Cc: Jesper Dangaard Brouer, David Miller, netdev, Paolo Abeni
In-Reply-To: <de654cac-fa3c-a4e1-a24a-27187456b8d7@hpe.com>
On Thu, 2016-12-08 at 07:36 -0800, Rick Jones wrote:
> On 12/08/2016 07:30 AM, Eric Dumazet wrote:
> > On Thu, 2016-12-08 at 10:46 +0100, Jesper Dangaard Brouer wrote:
> >
> >> Hmmm... I'm not thrilled to have such heuristics, that change memory
> >> behavior when half of the queue size (sk->sk_rcvbuf) is reached.
> >
> > Well, copybreak drivers do that unconditionally, even under no stress at
> > all, you really should complain then.
>
> Isn't that behaviour based (in part?) on the observation/belief that it
> is fewer cycles to copy the small packet into a small buffer than to
> send the larger buffer up the stack and have to allocate and map a
> replacement?
If properly done yes ;)
Some drivers do a copybreak, but throw away the original page frag and
reallocates a fresh one anyway.
Like if you have a PAGE_SIZE=65536, it is split in ~32 frags, and
drivers might not bother trying to reuse 1 frag.
^ permalink raw reply
* Re: [Intel-wired-lan] [RFC PATCH] i40e: enable PCIe relax ordering for SPARC
From: Alexander Duyck @ 2016-12-08 16:05 UTC (permalink / raw)
To: David Laight; +Cc: tndave, Jeff Kirsher, intel-wired-lan, Netdev
In-Reply-To: <063D6719AE5E284EB5DD2968C1650D6DB02382BD@AcuExch.aculab.com>
On Thu, Dec 8, 2016 at 2:43 AM, David Laight <David.Laight@aculab.com> wrote:
> From: Alexander Duyck
>> Sent: 06 December 2016 17:10
> ...
>> I was thinking about it and I realized we can probably simplify this
>> even further. In the case of most other architectures the
>> DMA_ATTR_WEAK_ORDERING has no effect anyway. So from what I can tell
>> there is probably no reason not to just always pass that attribute
>> with the DMA mappings. From what I can tell the only other
>> architecture that uses this is the PowerPC Cell architecture.
>
> And I should have read all the thread :-(
>
>> Also I was wondering if you actually needed to enable this attribute
>> for both Rx and Tx buffers or just Rx buffers? The patch that enabled
>> DMA_ATTR_WEAK_ORDERING for Sparc64 seems to call out writes, but I
>> didn't see anything about reads. I'm just wondering if changing the
>> code for Tx has any effect? If not you could probably drop those
>> changes and just focus on Rx.
>
> 'Weak ordering' only applies to PCIe read transfers, so can only have
> an effect on descriptor reads and transmit buffer reads.
>
> Basically PCIe is a comms protocol and an endpoint (or the host) can
> have multiple outstanding read requests (each of which might generate
> multiple response messages.
> The responses for each request must arrive in order, but responses for
> different requests can be interleaved.
> Setting 'not weak ordering' lets the host interwork with broken endpoints.
> (Or, like we did, you fix the fpga's PCIe implementation.)
I get the basics of relaxed ordering. The question is how does the
Sparc64 IOMMU translate DMA_ATTR_WEAK_ORDERING into relaxed ordering
messages, and at what level the ordering is relaxed. Odds are the
wording in the description where this attribute was added to Sparc is
just awkward, but I was wanting to verify if this only applies to
writes, or also read completions.
> In this case you need the reads of both transmit and receive rings to
> 'overtake' reads of transmit data.
Actually that isn't quite right. With relaxed ordering completions
and writes can pass each other if I recall correctly, but reads will
always force all writes ahead of them to be completed before you can
begin generating the read completions.
> I'm not at all clear how this 'flag' can be set on dma_map().
> It is a property of the PCIe subsystem.
That was where my original question on this came in. We can do a
blanket enable of relaxed ordering for Tx and Rx data buffers, but if
we only need it on Rx then there isn't any need for us to make
unnecessary changes.
- Alex
^ permalink raw reply
* Re: [PATCH net-next 0/2] Initial driver for Synopsys DWC XLGMAC
From: Alexandre Torgue @ 2016-12-08 15:59 UTC (permalink / raw)
To: Jie Deng, davem, f.fainelli, netdev
Cc: linux-kernel, CARLOS.PALMINHA, lars.persson, thomas.lendacky
In-Reply-To: <cover.1481075763.git.jiedeng@synopsys.com>
Hi
On 12/07/2016 04:57 AM, Jie Deng wrote:
> This series provides the support for 25/40/50/100 GbE
> devices using Synopsys DWC Enterprise Ethernet (XLGMAC).
Can you explain which GMAC are you targeted ?
A driver which support some Synopsys GMAC IP already exists. It support
GMAC 3.5, 3.7, 4.0, 4.10 versions. You can find it in:
drivers/net/ethernet/stmicro/stmmac/. When I look at all files your
gonna to create, it looks like to ones in stmmac driver so maybe you
could easily add your IP inside stmmac driver.
Note that an other driver is existing for synopsys GMAC 4.0 IP. it is
located in drivers/net/ethernet/synopsys/dwc_eth_qos.c and coming from
AXIS guys. A task is ongoing to only have stmmac driver so it should be
harmful to create a new one.
Regards
Alex
>
> The first patch adds support for Synopsys XLGMII.
> The second patch provides the initial driver for Synopsys XLGMAC
>
> The driver has three layers by refactoring AMD XGBE.
>
> dwc-eth-xxx.x
> The DWC ethernet core layer (DWC ECL). This layer contains codes
> can be shared by different DWC series ethernet cores
>
> dwc-xxx.x (e.g. dwc-xlgmac.c)
> The DWC MAC HW adapter layer (DWC MHAL). This layer contains
> special support for a specific MAC. e.g. currently, XLGMAC.
>
> xxx-xxx-pci.c xxx-xxx-plat.c (e.g. dwc-xlgmac-pci.c)
> The glue adapter layer (GAL). Vendors who adopt Synopsys Etherent
> cores can develop a glue driver for their platform.
>
> Jie Deng (2):
> net: phy: add extension of phy-mode for XLGMII
> net: ethernet: Initial driver for Synopsys DWC XLGMAC
>
> Documentation/devicetree/bindings/net/ethernet.txt | 1 +
> MAINTAINERS | 6 +
> drivers/net/ethernet/synopsys/Kconfig | 2 +
> drivers/net/ethernet/synopsys/Makefile | 1 +
> drivers/net/ethernet/synopsys/dwc/Kconfig | 37 +
> drivers/net/ethernet/synopsys/dwc/Makefile | 9 +
> drivers/net/ethernet/synopsys/dwc/dwc-eth-dcb.c | 228 ++
> .../net/ethernet/synopsys/dwc/dwc-eth-debugfs.c | 328 +++
> drivers/net/ethernet/synopsys/dwc/dwc-eth-desc.c | 715 +++++
> .../net/ethernet/synopsys/dwc/dwc-eth-ethtool.c | 567 ++++
> drivers/net/ethernet/synopsys/dwc/dwc-eth-hw.c | 3098 ++++++++++++++++++++
> drivers/net/ethernet/synopsys/dwc/dwc-eth-mdio.c | 252 ++
> drivers/net/ethernet/synopsys/dwc/dwc-eth-net.c | 2319 +++++++++++++++
> drivers/net/ethernet/synopsys/dwc/dwc-eth-ptp.c | 216 ++
> drivers/net/ethernet/synopsys/dwc/dwc-eth-regacc.h | 1115 +++++++
> drivers/net/ethernet/synopsys/dwc/dwc-eth.h | 738 +++++
> drivers/net/ethernet/synopsys/dwc/dwc-xlgmac-pci.c | 538 ++++
> drivers/net/ethernet/synopsys/dwc/dwc-xlgmac.c | 135 +
> drivers/net/ethernet/synopsys/dwc/dwc-xlgmac.h | 85 +
> include/linux/phy.h | 3 +
> 20 files changed, 10393 insertions(+)
> create mode 100644 drivers/net/ethernet/synopsys/dwc/Kconfig
> create mode 100644 drivers/net/ethernet/synopsys/dwc/Makefile
> create mode 100644 drivers/net/ethernet/synopsys/dwc/dwc-eth-dcb.c
> create mode 100644 drivers/net/ethernet/synopsys/dwc/dwc-eth-debugfs.c
> create mode 100644 drivers/net/ethernet/synopsys/dwc/dwc-eth-desc.c
> create mode 100644 drivers/net/ethernet/synopsys/dwc/dwc-eth-ethtool.c
> create mode 100644 drivers/net/ethernet/synopsys/dwc/dwc-eth-hw.c
> create mode 100644 drivers/net/ethernet/synopsys/dwc/dwc-eth-mdio.c
> create mode 100644 drivers/net/ethernet/synopsys/dwc/dwc-eth-net.c
> create mode 100644 drivers/net/ethernet/synopsys/dwc/dwc-eth-ptp.c
> create mode 100644 drivers/net/ethernet/synopsys/dwc/dwc-eth-regacc.h
> create mode 100644 drivers/net/ethernet/synopsys/dwc/dwc-eth.h
> create mode 100644 drivers/net/ethernet/synopsys/dwc/dwc-xlgmac-pci.c
> create mode 100644 drivers/net/ethernet/synopsys/dwc/dwc-xlgmac.c
> create mode 100644 drivers/net/ethernet/synopsys/dwc/dwc-xlgmac.h
>
^ permalink raw reply
* Re: stmmac driver...
From: Alexandre Torgue @ 2016-12-08 15:49 UTC (permalink / raw)
To: David Miller; +Cc: peppe.cavallaro, netdev
In-Reply-To: <20161208.102552.714315690167693892.davem@davemloft.net>
On 12/08/2016 04:25 PM, David Miller wrote:
> From: Alexandre Torgue <alexandre.torgue@st.com>
> Date: Thu, 8 Dec 2016 14:55:04 +0100
>
>> Maybe I forget some series. Do you have others in mind ?
>
> Please see the thread titled:
>
> "net: ethernet: Initial driver for Synopsys DWC XLGMAC"
>
> which seems to be discussing consolidation of various drivers
> for the same IP core, of which stmmac is one.
>
> I personally am against any change of the driver name and
> things like this, and wish the people doing that work would
> simply contribute to making whatever changes they need directly
> to the stmmac driver.
>
> You really need to voice your opinion when major changes are being
> proposed for the driver you maintain.
For sure I agree. I miss this one as it doesn't modify (yet) stmmac
driver. I gonna respond to the thread;
Regards
Alex
>
^ permalink raw reply
* Re: [PATCH 2/2] net: ethernet: stmmac: remove private tx queue lock
From: Pavel Machek @ 2016-12-08 15:46 UTC (permalink / raw)
To: David Miller
Cc: LinoSanfilippo, bh74.an, ks.giri, vipul.pandya, peppe.cavallaro,
alexandre.torgue, linux-kernel, netdev
In-Reply-To: <20161208.102641.2159772626439340664.davem@davemloft.net>
[-- Attachment #1: Type: text/plain, Size: 1518 bytes --]
On Thu 2016-12-08 10:26:41, David Miller wrote:
> From: Pavel Machek <pavel@ucw.cz>
> Date: Thu, 8 Dec 2016 15:08:46 +0100
>
> > On Wed 2016-12-07 18:41:11, David Miller wrote:
> >> From: Pavel Machek <pavel@ucw.cz>
> >> Date: Wed, 7 Dec 2016 22:37:57 +0100
> >>
> >> > diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> >> > index 982c952..7415bc2 100644
> >> > --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> >> > +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> >> > @@ -1308,7 +1308,7 @@ static void stmmac_tx_clean(struct stmmac_priv *priv)
> >> > unsigned int bytes_compl = 0, pkts_compl = 0;
> >> > unsigned int entry = priv->dirty_tx;
> >> >
> >> > - spin_lock(&priv->tx_lock);
> >> > + netif_tx_lock_bh(priv->dev);
> >> >
> >> > priv->xstats.tx_clean++;
> >> >
> >>
> >> stmmac_tx_clean() runs from either the timer or the NAPI poll handler,
> >> both execute from software interrupts, therefore _bh() should be
> >> unnecessary.
> >
> > I've tried the test again with netif_tx_lock() (not _bh()) and it
> > survived for more then four hours. Strange...
>
> It's not strange, it's completely expected.
Well, I tried that exact test before, and it survived for something
like 10 minutes. So yes... this surprised me.
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]
^ permalink raw reply
* Re: [PATCH] cxgb4: Support compressed error vector for T6
From: kbuild test robot @ 2016-12-08 15:44 UTC (permalink / raw)
To: Ganesh Goudar
Cc: kbuild-all, netdev, davem, nirranjan, Arjun V, Santosh Rastapur,
Hariprasad Shenai, Ganesh Goudar
In-Reply-To: <1481202113-16266-1-git-send-email-ganeshgr@chelsio.com>
[-- Attachment #1: Type: text/plain, Size: 2004 bytes --]
Hi Arjun,
[auto build test ERROR on net-next/master]
[also build test ERROR on v4.9-rc8 next-20161208]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Ganesh-Goudar/cxgb4-Support-compressed-error-vector-for-T6/20161208-222814
config: xtensa-allmodconfig (attached as .config)
compiler: xtensa-linux-gcc (GCC) 4.9.0
reproduce:
wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=xtensa
All errors (new ones prefixed by >>):
drivers/net/ethernet/chelsio/cxgb4/sge.c: In function 't4_ethrx_handler':
>> drivers/net/ethernet/chelsio/cxgb4/sge.c:2102:9: error: 'adapter' undeclared (first use in this function)
if (adapter->params.tp.rx_pkt_encap)
^
drivers/net/ethernet/chelsio/cxgb4/sge.c:2102:9: note: each undeclared identifier is reported only once for each function it appears in
>> drivers/net/ethernet/chelsio/cxgb4/sge.c:2104:9: error: 'T6_COMPR_RXERR_CSUM_F' undeclared (first use in this function)
T6_COMPR_RXERR_CSUM_F;
^
vim +/adapter +2102 drivers/net/ethernet/chelsio/cxgb4/sge.c
2096 #define CPL_RX_PKT_FLAGS (RXF_PSH_F | RXF_SYN_F | RXF_UDP_F | \
2097 RXF_TCP_F | RXF_IP_F | RXF_IP6_F | RXF_LRO_F)
2098
2099 if (!(pkt->l2info & cpu_to_be32(CPL_RX_PKT_FLAGS))) {
2100 if ((pkt->l2info & cpu_to_be32(RXF_FCOE_F)) &&
2101 (pi->fcoe.flags & CXGB_FCOE_ENABLED)) {
> 2102 if (adapter->params.tp.rx_pkt_encap)
2103 csum_ok = err_vec &
> 2104 T6_COMPR_RXERR_CSUM_F;
2105 else
2106 csum_ok = err_vec & RXERR_CSUM_F;
2107 if (!csum_ok)
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 47264 bytes --]
^ permalink raw reply
* Re: [PATCH v3 1/6] net: stmmac: return error if no DMA configuration is found
From: Alexandre Torgue @ 2016-12-08 15:41 UTC (permalink / raw)
To: David Miller
Cc: niklas.cassel, peppe.cavallaro, niklass, netdev, linux-kernel
In-Reply-To: <20161208.101915.2006667298193036157.davem@davemloft.net>
Hi David,
On 12/08/2016 04:19 PM, David Miller wrote:
> From: Alexandre Torgue <alexandre.torgue@st.com>
> Date: Thu, 8 Dec 2016 11:44:35 +0100
>
>> Acked-by: Alexandre Torgue <alexandre.torgue@stcom>
>
> Typo in your email.
>
> I would suggest that you put this into an editor macro or
> similar in order to avoid such typos in the future. That's
> what people do who review a lot of patches.
Sorry, I will pay attention next time.
>
^ permalink raw reply
* Re: [PATCH net-next] udp: under rx pressure, try to condense skbs
From: Rick Jones @ 2016-12-08 15:36 UTC (permalink / raw)
To: Eric Dumazet, Jesper Dangaard Brouer; +Cc: David Miller, netdev, Paolo Abeni
In-Reply-To: <1481211015.4930.100.camel@edumazet-glaptop3.roam.corp.google.com>
On 12/08/2016 07:30 AM, Eric Dumazet wrote:
> On Thu, 2016-12-08 at 10:46 +0100, Jesper Dangaard Brouer wrote:
>
>> Hmmm... I'm not thrilled to have such heuristics, that change memory
>> behavior when half of the queue size (sk->sk_rcvbuf) is reached.
>
> Well, copybreak drivers do that unconditionally, even under no stress at
> all, you really should complain then.
Isn't that behaviour based (in part?) on the observation/belief that it
is fewer cycles to copy the small packet into a small buffer than to
send the larger buffer up the stack and have to allocate and map a
replacement?
rick jones
^ permalink raw reply
* [PATCH] can: peak: fix bad memory access and free sequence
From: Marc Kleine-Budde @ 2016-12-08 15:35 UTC (permalink / raw)
To: netdev
Cc: davem, linux-can, kernel, 추지호, linux-stable,
Marc Kleine-Budde
In-Reply-To: <20161208153552.18122-1-mkl@pengutronix.de>
From: 추지호 <jiho.chu@samsung.com>
Fix for bad memory access while disconnecting. netdev is freed before
private data free, and dev is accessed after freeing netdev.
This makes a slub problem, and it raise kernel oops with slub debugger
config.
Signed-off-by: Jiho Chu <jiho.chu@samsung.com>
Cc: linux-stable <stable@vger.kernel.org>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
drivers/net/can/usb/peak_usb/pcan_usb_core.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/net/can/usb/peak_usb/pcan_usb_core.c b/drivers/net/can/usb/peak_usb/pcan_usb_core.c
index f3141ca56bc3..0b0302af3bd2 100644
--- a/drivers/net/can/usb/peak_usb/pcan_usb_core.c
+++ b/drivers/net/can/usb/peak_usb/pcan_usb_core.c
@@ -870,23 +870,25 @@ static int peak_usb_create_dev(const struct peak_usb_adapter *peak_usb_adapter,
static void peak_usb_disconnect(struct usb_interface *intf)
{
struct peak_usb_device *dev;
+ struct peak_usb_device *dev_prev_siblings;
/* unregister as many netdev devices as siblings */
- for (dev = usb_get_intfdata(intf); dev; dev = dev->prev_siblings) {
+ for (dev = usb_get_intfdata(intf); dev; dev = dev_prev_siblings) {
struct net_device *netdev = dev->netdev;
char name[IFNAMSIZ];
+ dev_prev_siblings = dev->prev_siblings;
dev->state &= ~PCAN_USB_STATE_CONNECTED;
strncpy(name, netdev->name, IFNAMSIZ);
unregister_netdev(netdev);
- free_candev(netdev);
kfree(dev->cmd_buf);
dev->next_siblings = NULL;
if (dev->adapter->dev_free)
dev->adapter->dev_free(dev);
+ free_candev(netdev);
dev_info(&intf->dev, "%s removed\n", name);
}
--
2.10.2
^ permalink raw reply related
* pull-request: can 2016-12-08
From: Marc Kleine-Budde @ 2016-12-08 15:35 UTC (permalink / raw)
To: netdev; +Cc: davem, linux-can, kernel
Hello David,
this is a pull request for one patch.
Jiho Chu found and fixed a use-after-free error in the cleanup path in the peak
pcan USB CAN driver.
regards,
Marc
---
The following changes since commit ec988ad78ed6d184a7f4ca6b8e962b0e8f1de461:
phy: Don't increment MDIO bus refcount unless it's a different owner (2016-12-07 13:27:14 -0500)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/mkl/linux-can.git tags/linux-can-fixes-for-4.9-20161208
for you to fetch changes up to b67d0dd7d0dc9e456825447bbeb935d8ef43ea7c:
can: peak: fix bad memory access and free sequence (2016-12-08 15:59:52 +0100)
----------------------------------------------------------------
linux-can-fixes-for-4.9-20161208
----------------------------------------------------------------
추지호 (1):
can: peak: fix bad memory access and free sequence
drivers/net/can/usb/peak_usb/pcan_usb_core.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
^ permalink raw reply
* Re: [PATCH] sh_eth: add wake-on-lan support via magic packet
From: Simon Horman @ 2016-12-08 15:31 UTC (permalink / raw)
To: Niklas Söderlund
Cc: Geert Uytterhoeven, Sergei Shtylyov, netdev@vger.kernel.org,
Linux-Renesas
In-Reply-To: <20161208150105.GI21834@bigcity.dyn.berto.se>
On Thu, Dec 08, 2016 at 04:01:05PM +0100, Niklas Söderlund wrote:
> Hi Simon,
>
> Thanks for your feedback.
>
> On 2016-12-08 14:22:44 +0100, Simon Horman wrote:
>
> <snip>
>
> > > >
> > > > > --- a/drivers/net/ethernet/renesas/sh_eth.h
> > > > > +++ b/drivers/net/ethernet/renesas/sh_eth.h
> > > > > @@ -493,6 +493,7 @@ struct sh_eth_cpu_data {
> > > > > unsigned shift_rd0:1; /* shift Rx descriptor word 0 right by 16 */
> > > > > unsigned rmiimode:1; /* EtherC has RMIIMODE register */
> > > > > unsigned rtrate:1; /* EtherC has RTRATE register */
> > > > > + unsigned magic:1; /* EtherC have PMDE in ECMR and MPDIP in ECSIPR */
> > > >
> > > > Instead of adding a new flag, perhaps you can just check for the ECSR_MPD flag
> > > > in ecsr_value?
> > >
> > > I briefly considered this but decided against it since I do not have
> > > documentation for all versions of the device and no way to test it. You
> > > tested and confirmed functionality on r8a7740, which leaves:
> > >
> > > - sh7734-gether
> > > - sh7763-gether
> > > - sh7757-gether
> > >
> > > To figure out if they support MagicPacket in the same fashion as r8a7740
> > > and r8a779x. If anyone have access to documentation or hardware to
> > > confirm this I be more then happy to get rid of the magic flag in favor
> > > och checking for ECSR_MPD in ecsr_value.
> >
> > Perhaps documentation can be found but if not I wonder if we can use some
> > other mechanism to blacklist SoC which we are unsure about.
> >
> > From my POV it would be very nice if things just worked™ on SoCs where
> > the feature has been verified.
>
> I agree, I will follow Sergies advice and Geerts testing to enable Gen2
> family and r8a7740/armadillo in two separate patches. Then if we later
> can confirm it works on other models we can enable them in separate
> patches by setting the magic flag in struct sh_eth_cpu_data for those
> models. Do you agree this is the best way to handle this?
Yes, I think that is reasonable.
^ permalink raw reply
* Re: [PATCH net-next] udp: under rx pressure, try to condense skbs
From: Eric Dumazet @ 2016-12-08 15:30 UTC (permalink / raw)
To: Jesper Dangaard Brouer; +Cc: David Miller, netdev, Paolo Abeni
In-Reply-To: <20161208104620.5fc691b8@redhat.com>
On Thu, 2016-12-08 at 10:46 +0100, Jesper Dangaard Brouer wrote:
> Hmmm... I'm not thrilled to have such heuristics, that change memory
> behavior when half of the queue size (sk->sk_rcvbuf) is reached.
Well, copybreak drivers do that unconditionally, even under no stress at
all, you really should complain then.
copybreak is interesting, not only for performance point of view, but
ability to handle DOS/DDOS : Attackers need to send bigger packets to
eventually force us to consume one page per packet.
My idea (which I already described in the past) is to perform the
(small) copy only in contexts we know packet might sit for a long time
in a socket queue, and only if we know we are in stress conditions.
ACK packets for example do not need copybreak, since they wont be queued
for a long time.
>
> Most of the win comes from doing a local atomic page-refcnt decrement
> oppose to doing a remote CPU refcnf-dec. And as you noticed the
> benefit is quite high saving 241 cycles (see [1]). And you patch is
> "using" these cycles to copy the packet instead.
So, just to let you know, I have a patch series which achieve ~100 %
perf increase, without the 2nd queue I envisioned for linux-4.11
A single thread doing mere recvmsg() system calls can now read ~2Mpps.
This skb_condense() is done before producer cpus are competing using a
busylock array (not a per socket new spinlock, but a shared hashed
array, out of line).
I plan using it tcp_add_backlog() to replace the :
if (!skb->data_len)
skb->truesize = SKB_TRUESIZE(skb_end_offset(skb));
> This might no be a win in the future. I'm working on a more generic
> solution (page_pool) that (as one objective) target this remote recfnt.
Very well, when all drivers can use this, we might revert this patch if
proved not beneficial.
But make sure we can hold 1,000,000 pages in skbs stored in ~100,000
TCP/UDP sockets.
Your ideas sound fine in controlled environments, I am sure you will be
able to demonstrate their gains independently of the counter measures we
put in place in the protocol handlers.
^ permalink raw reply
* Re: [PATCH 2/2] net: ethernet: stmmac: remove private tx queue lock
From: David Miller @ 2016-12-08 15:26 UTC (permalink / raw)
To: pavel
Cc: LinoSanfilippo, bh74.an, ks.giri, vipul.pandya, peppe.cavallaro,
alexandre.torgue, linux-kernel, netdev
In-Reply-To: <20161208140846.GA24327@amd>
From: Pavel Machek <pavel@ucw.cz>
Date: Thu, 8 Dec 2016 15:08:46 +0100
> On Wed 2016-12-07 18:41:11, David Miller wrote:
>> From: Pavel Machek <pavel@ucw.cz>
>> Date: Wed, 7 Dec 2016 22:37:57 +0100
>>
>> > diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>> > index 982c952..7415bc2 100644
>> > --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>> > +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>> > @@ -1308,7 +1308,7 @@ static void stmmac_tx_clean(struct stmmac_priv *priv)
>> > unsigned int bytes_compl = 0, pkts_compl = 0;
>> > unsigned int entry = priv->dirty_tx;
>> >
>> > - spin_lock(&priv->tx_lock);
>> > + netif_tx_lock_bh(priv->dev);
>> >
>> > priv->xstats.tx_clean++;
>> >
>>
>> stmmac_tx_clean() runs from either the timer or the NAPI poll handler,
>> both execute from software interrupts, therefore _bh() should be
>> unnecessary.
>
> I've tried the test again with netif_tx_lock() (not _bh()) and it
> survived for more then four hours. Strange...
It's not strange, it's completely expected.
^ permalink raw reply
* Re: stmmac driver...
From: David Miller @ 2016-12-08 15:25 UTC (permalink / raw)
To: alexandre.torgue; +Cc: peppe.cavallaro, netdev
In-Reply-To: <e8370557-b11a-0fe6-f87f-3af2e5990f27@st.com>
From: Alexandre Torgue <alexandre.torgue@st.com>
Date: Thu, 8 Dec 2016 14:55:04 +0100
> Maybe I forget some series. Do you have others in mind ?
Please see the thread titled:
"net: ethernet: Initial driver for Synopsys DWC XLGMAC"
which seems to be discussing consolidation of various drivers
for the same IP core, of which stmmac is one.
I personally am against any change of the driver name and
things like this, and wish the people doing that work would
simply contribute to making whatever changes they need directly
to the stmmac driver.
You really need to voice your opinion when major changes are being
proposed for the driver you maintain.
^ permalink raw reply
* Re: [PATCH v3 1/6] net: stmmac: return error if no DMA configuration is found
From: David Miller @ 2016-12-08 15:19 UTC (permalink / raw)
To: alexandre.torgue
Cc: niklas.cassel, peppe.cavallaro, niklass, netdev, linux-kernel
In-Reply-To: <d18098a5-a4a4-0c63-f64c-cdb7144dd67e@st.com>
From: Alexandre Torgue <alexandre.torgue@st.com>
Date: Thu, 8 Dec 2016 11:44:35 +0100
> Acked-by: Alexandre Torgue <alexandre.torgue@stcom>
Typo in your email.
I would suggest that you put this into an editor macro or
similar in order to avoid such typos in the future. That's
what people do who review a lot of patches.
^ permalink raw reply
* Re: [PATCH v3 6/6] net: smmac: allow configuring lower pbl values
From: David Miller @ 2016-12-08 15:18 UTC (permalink / raw)
To: alexandre.torgue
Cc: niklas.cassel, robh+dt, mark.rutland, corbet, peppe.cavallaro,
preid, eric, pavel, afaerber, manabian, vpalatin,
gabriel.fernandez, niklass, netdev, devicetree, linux-kernel,
linux-doc
In-Reply-To: <4d43479f-8373-308b-8c7d-cfed5346dc53@st.com>
From: Alexandre Torgue <alexandre.torgue@st.com>
Date: Thu, 8 Dec 2016 11:42:56 +0100
> Thanks for this patch, you can add my Acked-by.
Please simply state:
Acked-by: Alexandre Torgue <alexandre.torgue@st.com>
in your reply and it will automatically appear when the patch is
applied. You don't have to ask the patch submitter or the person who
applies it to do it as you are doing here.
^ permalink raw reply
* Re: [PATCH] sh_eth: add wake-on-lan support via magic packet
From: Geert Uytterhoeven @ 2016-12-08 15:13 UTC (permalink / raw)
To: Niklas Söderlund
Cc: Sergei Shtylyov, Simon Horman, netdev@vger.kernel.org,
Linux-Renesas
In-Reply-To: <20161208145635.GH21834@bigcity.dyn.berto.se>
On Thu, Dec 8, 2016 at 3:56 PM, Niklas Söderlund
<niklas.soderlund@ragnatech.se> wrote:
>> > + /* Increased clock usage so device won't be suspended */
>> > + clk_enable(mdp->clk);
>>
>> Hum, intermixiggn runtime PM with clock API doesn't look good...
>
> I agree it looks weird but I need a way to increment the usage count for
> the clock otherwise the PM code will disable the module clock and WoL
> will not work. Note that this call will not enable the clock just
> increase the usage count so it won't be disabled when the PM code
> decrease it after the sh_eth suspend function is run.
>
> If you know of a different way of ensuring that the clock is not turned
> off I be happy to look at it. I did some investigation into this and
> calling clk_enable() directly is for example what happens in the
> enable_irq_wake() call path to ensure the clock for the irq_chip is not
> turned off if it is a wakeup source, se for example
> gpio_rcar_irq_set_wake() in drivers/gpio/gpio-rcar.c.
Ideally we want to prevent the PM code from disabling the clock, and from
powering down the power area, if present.
The latter may matter on other SoCs. But on r8a7740, Ethernet is part of
power area A4S, which we already prevent from being powered down because
it contains the memory controller.
Gr{oetje,eeting}s,
Geert
^ permalink raw reply
* Re: [PATCH] sh_eth: add wake-on-lan support via magic packet
From: Niklas Söderlund @ 2016-12-08 15:01 UTC (permalink / raw)
To: Simon Horman
Cc: Geert Uytterhoeven, Sergei Shtylyov, netdev@vger.kernel.org,
Linux-Renesas
In-Reply-To: <20161208132241.GA7150@verge.net.au>
Hi Simon,
Thanks for your feedback.
On 2016-12-08 14:22:44 +0100, Simon Horman wrote:
<snip>
> > >
> > > > --- a/drivers/net/ethernet/renesas/sh_eth.h
> > > > +++ b/drivers/net/ethernet/renesas/sh_eth.h
> > > > @@ -493,6 +493,7 @@ struct sh_eth_cpu_data {
> > > > unsigned shift_rd0:1; /* shift Rx descriptor word 0 right by 16 */
> > > > unsigned rmiimode:1; /* EtherC has RMIIMODE register */
> > > > unsigned rtrate:1; /* EtherC has RTRATE register */
> > > > + unsigned magic:1; /* EtherC have PMDE in ECMR and MPDIP in ECSIPR */
> > >
> > > Instead of adding a new flag, perhaps you can just check for the ECSR_MPD flag
> > > in ecsr_value?
> >
> > I briefly considered this but decided against it since I do not have
> > documentation for all versions of the device and no way to test it. You
> > tested and confirmed functionality on r8a7740, which leaves:
> >
> > - sh7734-gether
> > - sh7763-gether
> > - sh7757-gether
> >
> > To figure out if they support MagicPacket in the same fashion as r8a7740
> > and r8a779x. If anyone have access to documentation or hardware to
> > confirm this I be more then happy to get rid of the magic flag in favor
> > och checking for ECSR_MPD in ecsr_value.
>
> Perhaps documentation can be found but if not I wonder if we can use some
> other mechanism to blacklist SoC which we are unsure about.
>
> From my POV it would be very nice if things just worked™ on SoCs where
> the feature has been verified.
I agree, I will follow Sergies advice and Geerts testing to enable Gen2
family and r8a7740/armadillo in two separate patches. Then if we later
can confirm it works on other models we can enable them in separate
patches by setting the magic flag in struct sh_eth_cpu_data for those
models. Do you agree this is the best way to handle this?
--
Regards,
Niklas Söderlund
^ permalink raw reply
* Re: [PATCH] sh_eth: add wake-on-lan support via magic packet
From: Niklas Söderlund @ 2016-12-08 14:56 UTC (permalink / raw)
To: Sergei Shtylyov; +Cc: Simon Horman, netdev, linux-renesas-soc
In-Reply-To: <59a1f246-9062-20e1-3f85-a1c5f6fcfc29@cogentembedded.com>
Hi Sergei,
Thanks for your feedback.
On 2016-12-08 15:28:48 +0300, Sergei Shtylyov wrote:
> Hello!
>
> Good to see that somebody cares still about this driver, one more task
> off my back. :-)
>
> On 12/07/2016 07:28 PM, Niklas Söderlund wrote:
>
> You only enable the WOL support fo the R-Car gen2 chips but never say that
> explicitly, neither in the subject nor here.
>
> > Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> > ---
> > drivers/net/ethernet/renesas/sh_eth.c | 120 +++++++++++++++++++++++++++++++---
> > drivers/net/ethernet/renesas/sh_eth.h | 4 ++
> > 2 files changed, 116 insertions(+), 8 deletions(-)
>
> > diff --git a/drivers/net/ethernet/renesas/sh_eth.c b/drivers/net/ethernet/renesas/sh_eth.c
> > index 05b0dc5..3974046 100644
> > --- a/drivers/net/ethernet/renesas/sh_eth.c
> > +++ b/drivers/net/ethernet/renesas/sh_eth.c
> > @@ -624,7 +624,7 @@ static struct sh_eth_cpu_data r8a779x_data = {
> >
> > .register_type = SH_ETH_REG_FAST_RCAR,
> >
> > - .ecsr_value = ECSR_PSRTO | ECSR_LCHNG | ECSR_ICD,
> > + .ecsr_value = ECSR_PSRTO | ECSR_LCHNG | ECSR_ICD | ECSR_MPD,
> > .ecsipr_value = ECSIPR_PSRTOIP | ECSIPR_LCHNGIP | ECSIPR_ICDIP,
> > .eesipr_value = 0x01ff009f,
> >
> > @@ -641,6 +641,7 @@ static struct sh_eth_cpu_data r8a779x_data = {
> > .tpauser = 1,
> > .hw_swap = 1,
> > .rmiimode = 1,
> > + .magic = 1,
> > };
> > #endif /* CONFIG_OF */
>
> So I suggest that you add the general WOL support code in the 1st patch
> and enable the new feature per SoC family in the follow up patches.
Ok I will split this up in v2.
>
> > @@ -1657,6 +1658,10 @@ static irqreturn_t sh_eth_interrupt(int irq, void *netdev)
> > goto out;
> >
> > if (!likely(mdp->irq_enabled)) {
>
> Oops, I guess unlikely(!mdp->irq_enabled) was meant here...
I can correct this in a separate patch if you wish.
>
> > + /* Handle MagicPacket interrupt */
> > + if (sh_eth_read(ndev, ECSR) & ECSR_MPD)
>
> Why do it here?
See bellow.
>
> > + sh_eth_modify(ndev, ECSR, 0, ECSR_MPD);
> > +
> > sh_eth_write(ndev, 0, EESIPR);
> > goto out;
> > }
> [...]
> > @@ -3017,6 +3051,11 @@ static int sh_eth_drv_probe(struct platform_device *pdev)
> > goto out_release;
> > }
> >
> > + /* Get clock, if not found that's OK but Wake-On-Lan is unavailable */
> > + mdp->clk = devm_clk_get(&pdev->dev, NULL);
>
> Luckily, we have <linux/clk.h> #include'd...
>
> > + if (IS_ERR(mdp->clk))
> > + mdp->clk = NULL;
> > +
> > ndev->base_addr = res->start;
> >
> > spin_lock_init(&mdp->lock);
> > @@ -3111,6 +3150,10 @@ static int sh_eth_drv_probe(struct platform_device *pdev)
> > if (ret)
> > goto out_napi_del;
> >
> > + mdp->wol_enabled = false;
>
> No need, the '*mdp' was kzalloc'ed.
OK, i prefer to explicitly set for easier reading of the code. But if
you wish I will remove this in v2.
>
> > + if (mdp->cd->magic && mdp->clk)
> > + device_set_wakeup_capable(&pdev->dev, 1);
> > +
> > /* print device information */
> > netdev_info(ndev, "Base address at 0x%x, %pM, IRQ %d.\n",
> > (u32)ndev->base_addr, ndev->dev_addr, ndev->irq);
> > @@ -3150,15 +3193,71 @@ static int sh_eth_drv_remove(struct platform_device *pdev)
> >
> > #ifdef CONFIG_PM
> > #ifdef CONFIG_PM_SLEEP
> > +static int sh_eth_wol_setup(struct net_device *ndev)
> > +{
> > + struct sh_eth_private *mdp = netdev_priv(ndev);
> > +
> > + /* Only allow ECI interrupts */
> > + mdp->irq_enabled = false;
>
> Why 'false' if you enable IRQs below?
I mask all interrupts except MagicPacket (ECSIPR_MPDIP) interrupts form
the ECI (DMAC_M_ECI) and by setting irq_enabled to false the interrupt
handler will only ack any residue interrupt. This is how it's done in
other parts of the driver when disabling interrupts.
This is also why I only check for MagicPacket interrupts if irq_enabled
is false.
>
> > + synchronize_irq(ndev->irq);
> > + napi_disable(&mdp->napi);
> > + sh_eth_write(ndev, DMAC_M_ECI, EESIPR);
> > +
> > + /* Enable ECI MagicPacket interrupt */
> > + sh_eth_write(ndev, ECSIPR_MPDIP, ECSIPR);
> > +
> > + /* Enable MagicPacket */
> > + sh_eth_modify(ndev, ECMR, 0, ECMR_PMDE);
> > +
> > + /* Increased clock usage so device won't be suspended */
> > + clk_enable(mdp->clk);
>
> Hum, intermixiggn runtime PM with clock API doesn't look good...
I agree it looks weird but I need a way to increment the usage count for
the clock otherwise the PM code will disable the module clock and WoL
will not work. Note that this call will not enable the clock just
increase the usage count so it won't be disabled when the PM code
decrease it after the sh_eth suspend function is run.
If you know of a different way of ensuring that the clock is not turned
off I be happy to look at it. I did some investigation into this and
calling clk_enable() directly is for example what happens in the
enable_irq_wake() call path to ensure the clock for the irq_chip is not
turned off if it is a wakeup source, se for example
gpio_rcar_irq_set_wake() in drivers/gpio/gpio-rcar.c.
>
> > +
> > + return enable_irq_wake(ndev->irq);
> > +}
> > +
> > +static int sh_eth_wol_restore(struct net_device *ndev)
> > +{
> > + struct sh_eth_private *mdp = netdev_priv(ndev);
> > + int ret;
> > +
> > + napi_enable(&mdp->napi);
> > +
> > + /* Disable MagicPacket */
> > + sh_eth_modify(ndev, ECMR, ECMR_PMDE, 0);
> > +
> > + /* The device need to be reset to restore MagicPacket logic
> > + * for next wakeup. If we close and open the device it will
> > + * both be reset and all registers restored. This is what
> > + * happens during suspend and resume without WoL enabled.
> > + */
> > + ret = sh_eth_close(ndev);
> > + if (ret < 0)
> > + return ret;
> > + ret = sh_eth_open(ndev);
> > + if (ret < 0)
> > + return ret;
> > +
> > + /* Restore clock usage count */
> > + clk_disable(mdp->clk);
>
> Hm... and RPM will think the clock is still enabled?
> Why disable the clock here anyway if we're resuming?
This call is needed to decrease the usage count for the clock we
increased with clk_enable() in the suspend path.
>
> > +
> > + return disable_irq_wake(ndev->irq);
> > +}
> > +
> [...]
> > @@ -3166,14 +3265,19 @@ static int sh_eth_suspend(struct device *dev)
> > static int sh_eth_resume(struct device *dev)
> > {
> > struct net_device *ndev = dev_get_drvdata(dev);
> > + struct sh_eth_private *mdp = netdev_priv(ndev);
> > int ret = 0;
> >
> > - if (netif_running(ndev)) {
> > + if (!netif_running(ndev))
> > + return 0;
> > +
> > + if (mdp->wol_enabled)
> > + ret = sh_eth_wol_restore(ndev);
> > + else
> > ret = sh_eth_open(ndev);
> > - if (ret < 0)
> > - return ret;
> > +
> > + if (!ret)
>
> Please keep the original error return logic, so that you can return 0 at
> the end...
Will fix for v2.
>
> > netif_device_attach(ndev);
> > - }
> >
> > return ret;
> > }
> > diff --git a/drivers/net/ethernet/renesas/sh_eth.h b/drivers/net/ethernet/renesas/sh_eth.h
> > index d050f37..26c6620 100644
> > --- a/drivers/net/ethernet/renesas/sh_eth.h
> > +++ b/drivers/net/ethernet/renesas/sh_eth.h
> > @@ -493,6 +493,7 @@ struct sh_eth_cpu_data {
> > unsigned shift_rd0:1; /* shift Rx descriptor word 0 right by 16 */
> > unsigned rmiimode:1; /* EtherC has RMIIMODE register */
> > unsigned rtrate:1; /* EtherC has RTRATE register */
> > + unsigned magic:1; /* EtherC have PMDE in ECMR and MPDIP in ECSIPR */
>
> OK, e.g. RZ/A1 doesn't have these bits...
>
> [...]
>
> MBR, Sergei
>
--
Regards,
Niklas Söderlund
^ 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