netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Russell King - ARM Linux <linux@arm.linux.org.uk>
To: Troy Kisky <troy.kisky@boundarydevices.com>
Cc: Fugang Duan <fugang.duan@nxp.com>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	"davem@davemloft.net" <davem@davemloft.net>,
	"b38611@freescale.com" <b38611@freescale.com>,
	"fabio.estevam@freescale.com" <fabio.estevam@freescale.com>,
	"l.stach@pengutronix.de" <l.stach@pengutronix.de>,
	"andrew@lunn.ch" <andrew@lunn.ch>,
	"tremyfr@gmail.com" <tremyfr@gmail.com>,
	"linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>,
	"laci@boundarydevices.com" <laci@boundarydevices.com>,
	"shawnguo@kernel.org" <shawnguo@kernel.org>,
	"johannes@sipsolutions.net" <johannes@sipsolutions.net>,
	"stillcompiling@gmail.com" <stillcompiling@gmail.com>,
	"sergei.shtylyov@cogentembedded.com"
	<sergei.shtylyov@cogentembedded.com>,
	"arnd@arndb.de" <arnd@arndb.de>
Subject: Re: [PATCH net-next V2 06/16] net: fec: don't clear all rx queue bits when just one is being checked
Date: Fri, 4 Mar 2016 16:38:54 +0000	[thread overview]
Message-ID: <20160304163854.GD19428@n2100.arm.linux.org.uk> (raw)
In-Reply-To: <56D9B54B.8020309@boundarydevices.com>

On Fri, Mar 04, 2016 at 09:18:19AM -0700, Troy Kisky wrote:
> On 3/4/2016 2:11 AM, Fugang Duan wrote:
> > From: Troy Kisky <troy.kisky@boundarydevices.com>Sent: Thursday, February 25, 2016 8:37 AM
> >> To: netdev@vger.kernel.org; davem@davemloft.net; b38611@freescale.com
> >> Cc: fabio.estevam@freescale.com; l.stach@pengutronix.de; andrew@lunn.ch;
> >> tremyfr@gmail.com; linux@arm.linux.org.uk; linux-arm-
> >> kernel@lists.infradead.org; laci@boundarydevices.com; shawnguo@kernel.org;
> >> johannes@sipsolutions.net; stillcompiling@gmail.com;
> >> sergei.shtylyov@cogentembedded.com; arnd@arndb.de; Troy Kisky
> >> <troy.kisky@boundarydevices.com>
> >> Subject: [PATCH net-next V2 06/16] net: fec: don't clear all rx queue bits when
> >> just one is being checked
> >>
> >> FEC_ENET_RXF is 3 separate bits, we only check one queue at a time. So, when
> >> the last queue is being checked, it is bad to remove the interrupt on the 1st
> >> queue.
> >>
> >> Also, since this is now done in the napi routine and not the interrupt, it is not
> >> needed.
> >>
> >> Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>
> >> ---
> >>  drivers/net/ethernet/freescale/fec_main.c | 2 --
> >>  1 file changed, 2 deletions(-)
> >>
> >> diff --git a/drivers/net/ethernet/freescale/fec_main.c
> >> b/drivers/net/ethernet/freescale/fec_main.c
> >> index 610cf6c..791f385 100644
> >> --- a/drivers/net/ethernet/freescale/fec_main.c
> >> +++ b/drivers/net/ethernet/freescale/fec_main.c
> >> @@ -1338,8 +1338,6 @@ static int fec_rxq(struct net_device *ndev, struct
> >> fec_enet_private *fep,
> >>  			break;
> >>  		pkt_received++;
> >>
> >> -		writel(FEC_ENET_RXF, fep->hwp + FEC_IEVENT);
> >> -
> > 
> > We should clear the related rx queue ievent, not remove the code.
> > Pls see commit: db3421c114cf that was submitted by Russell King.
> > 
> > No ack the patch.
> 
> 
> This is now done in patch #4 "net: fec: reduce interrupts" and you could argue
> that it should be squashed into that patch. But I like separating changes
> as much as possible.
> 
> 
> Russell, this patch and patch #4 will likely need your ack before it will be applied.
> Can you take a look please?

I stopped caring about the FEC ethernet driver about 18 months ago,
after I ended up dropping a significant pile of fixes on the floor
through the huge number of conflicts and the shere effort of
constantly trying to move them forward.

My patch series tend to be large because I put concentrated effort
into something for a month, which then gives a problem if conflicts
come up later and the series has to be effectively rewritten from
scratch.  It was after the second or third time of facing an almost
total rewrite that happened that I just gave up.

I've toyed with the idea of forking the driver, but I wouldn't have
time to maintain such a thing.  So, right now I just put up with all
the bad quirks, and reset/power cycle the boards when things go wrong.
Right now, I just disable runtime PM support on the FEC to get
stability here. :)

Sorry, but I can't be of more help.

-- 
RMK's Patch system: http://www.arm.linux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

  reply	other threads:[~2016-03-04 16:39 UTC|newest]

Thread overview: 53+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-02-25  0:36 [PATCH net-next V2 00/16] net: fec: cleanup and fixes Troy Kisky
2016-02-25  0:36 ` [PATCH net-next V2 01/16] net: fec: only check queue 0 if RXF_0/TXF_0 interrupt is set Troy Kisky
2016-03-02 14:57   ` Fugang Duan
2016-02-25  0:36 ` [PATCH net-next V2 02/16] net: fec: pass rxq to fec_enet_rx_queue instead of queue_id Troy Kisky
2016-03-01 21:05   ` Zhi Li
2016-03-02 15:01   ` Fugang Duan
2016-02-25  0:36 ` [PATCH net-next V2 03/16] net: fec: pass txq to fec_enet_tx_queue " Troy Kisky
2016-03-01 21:06   ` Zhi Li
2016-03-01 21:51     ` Troy Kisky
2016-03-01 22:26       ` Zhi Li
2016-03-01 22:43         ` Troy Kisky
2016-03-02 15:16   ` Fugang Duan
2016-03-02 16:13     ` Troy Kisky
2016-03-04  7:41       ` Fugang Duan
2016-03-04 16:23         ` Troy Kisky
2016-02-25  0:36 ` [PATCH net-next V2 04/16] net: fec: reduce interrupts Troy Kisky
2016-03-02 15:13   ` Fugang Duan
2016-03-02 16:12     ` Troy Kisky
2016-03-02 16:44       ` Zhi Li
2016-03-02 16:47       ` Zhi Li
2016-03-02 22:32         ` Troy Kisky
2016-03-02 22:52           ` Zhi Li
2016-03-04  8:58   ` Fugang Duan
2016-02-25  0:36 ` [PATCH net-next V2 05/16] net: fec: split off napi routine with 3 queues Troy Kisky
2016-02-25  0:36 ` [PATCH net-next V2 06/16] net: fec: don't clear all rx queue bits when just one is being checked Troy Kisky
2016-03-04  9:11   ` Fugang Duan
2016-03-04 16:18     ` Troy Kisky
2016-03-04 16:38       ` Russell King - ARM Linux [this message]
2016-03-04 17:28         ` Troy Kisky
2016-02-25  0:36 ` [PATCH net-next V2 07/16] net: fec: set cbd_sc without relying on previous value Troy Kisky
2016-03-04  9:29   ` Fugang Duan
2016-03-04 16:08     ` Troy Kisky
2016-03-05 23:55       ` Fugang Duan
2016-02-25  0:36 ` [PATCH net-next V2 08/16] net: fec: eliminate calls to fec_enet_get_prevdesc Troy Kisky
2016-03-04  9:33   ` Fugang Duan
2016-03-04 16:05     ` Troy Kisky
2016-02-25  0:36 ` [PATCH net-next V2 09/16] net: fec: move restart test for efficiency Troy Kisky
2016-02-25  0:36 ` [PATCH net-next V2 10/16] net: fec: clear cbd_sc after transmission to help with debugging Troy Kisky
2016-02-25  0:36 ` [PATCH net-next V2 11/16] net: fec: dump all tx queues in fec_dump Troy Kisky
2016-02-25  0:36 ` [PATCH net-next V2 12/16] net: fec: detect tx int lost Troy Kisky
2016-02-25  0:36 ` [PATCH net-next V2 13/16] net: fec: print more debug info in fec_timeout Troy Kisky
2016-03-04 10:06   ` Fugang Duan
2016-03-04 16:05     ` Troy Kisky
2016-03-04 17:35       ` Joe Perches
2016-03-04 19:06         ` Troy Kisky
2016-02-25  0:36 ` [PATCH net-next V2 14/16] net: fec: create subroutine reset_tx_queue Troy Kisky
2016-02-25  0:36 ` [PATCH net-next V2 15/16] net: fec: call dma_unmap_single on mapped tx buffers at restart Troy Kisky
2016-02-25  0:36 ` [PATCH net-next V2 16/16] net: fec: don't set cbd_bufaddr unless no mapping error Troy Kisky
2016-02-25  2:52 ` [PATCH net-next V2 00/16] net: fec: cleanup and fixes Joshua Clayton
2016-02-25 16:05   ` Troy Kisky
2016-02-25 16:49     ` Joshua Clayton
2016-02-25  8:39 ` Holger Schurig
2016-02-25 15:57   ` Troy Kisky

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20160304163854.GD19428@n2100.arm.linux.org.uk \
    --to=linux@arm.linux.org.uk \
    --cc=andrew@lunn.ch \
    --cc=arnd@arndb.de \
    --cc=b38611@freescale.com \
    --cc=davem@davemloft.net \
    --cc=fabio.estevam@freescale.com \
    --cc=fugang.duan@nxp.com \
    --cc=johannes@sipsolutions.net \
    --cc=l.stach@pengutronix.de \
    --cc=laci@boundarydevices.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=netdev@vger.kernel.org \
    --cc=sergei.shtylyov@cogentembedded.com \
    --cc=shawnguo@kernel.org \
    --cc=stillcompiling@gmail.com \
    --cc=tremyfr@gmail.com \
    --cc=troy.kisky@boundarydevices.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).