DMA Engine development
 help / color / mirror / Atom feed
* dmaengine: ti: omap-dma: Configure LCH_TYPE for OMAP1
From: Russell King - ARM Linux @ 2018-11-23 16:16 UTC (permalink / raw)
  To: Peter Ujfalusi
  Cc: Aaro Koskinen, vkoul, dan.j.williams, dmaengine, linux-kernel,
	tony, linux-omap

Hi Peter,

Here's the patch, which should now support IN as well as OUT.
Completely untested, as mentioned before.

 drivers/usb/gadget/udc/omap_udc.c | 286 ++++++++++++++++++--------------------
 drivers/usb/gadget/udc/omap_udc.h |   3 +-
 2 files changed, 135 insertions(+), 154 deletions(-)

diff --git a/drivers/usb/gadget/udc/omap_udc.c b/drivers/usb/gadget/udc/omap_udc.c
index 3a16431da321..ad6f315e4327 100644
--- a/drivers/usb/gadget/udc/omap_udc.c
+++ b/drivers/usb/gadget/udc/omap_udc.c
@@ -28,6 +28,7 @@
 #include <linux/usb/ch9.h>
 #include <linux/usb/gadget.h>
 #include <linux/usb/otg.h>
+#include <linux/dmaengine.h>
 #include <linux/dma-mapping.h>
 #include <linux/clk.h>
 #include <linux/err.h>
@@ -203,7 +204,7 @@ static int omap_ep_enable(struct usb_ep *_ep,
 	/* set endpoint to initial state */
 	ep->dma_channel = 0;
 	ep->has_dma = 0;
-	ep->lch = -1;
+	ep->dma = NULL;
 	use_ep(ep, UDC_EP_SEL);
 	omap_writew(udc->clr_halt, UDC_CTRL);
 	ep->ackwait = 0;
@@ -468,43 +469,6 @@ static int read_fifo(struct omap_ep *ep, struct omap_req *req)
 
 /*-------------------------------------------------------------------------*/
 
-static u16 dma_src_len(struct omap_ep *ep, dma_addr_t start)
-{
-	dma_addr_t	end;
-
-	/* IN-DMA needs this on fault/cancel paths, so 15xx misreports
-	 * the last transfer's bytecount by more than a FIFO's worth.
-	 */
-	if (cpu_is_omap15xx())
-		return 0;
-
-	end = omap_get_dma_src_pos(ep->lch);
-	if (end == ep->dma_counter)
-		return 0;
-
-	end |= start & (0xffff << 16);
-	if (end < start)
-		end += 0x10000;
-	return end - start;
-}
-
-static u16 dma_dest_len(struct omap_ep *ep, dma_addr_t start)
-{
-	dma_addr_t	end;
-
-	end = omap_get_dma_dst_pos(ep->lch);
-	if (end == ep->dma_counter)
-		return 0;
-
-	end |= start & (0xffff << 16);
-	if (cpu_is_omap15xx())
-		end++;
-	if (end < start)
-		end += 0x10000;
-	return end - start;
-}
-
-
 /* Each USB transfer request using DMA maps to one or more DMA transfers.
  * When DMA completion isn't request completion, the UDC continues with
  * the next DMA transfer for that USB transfer.
@@ -512,34 +476,53 @@ static u16 dma_dest_len(struct omap_ep *ep, dma_addr_t start)
 
 static void next_in_dma(struct omap_ep *ep, struct omap_req *req)
 {
-	u16		txdma_ctrl, w;
-	unsigned	length = req->req.length - req->req.actual;
-	const int	sync_mode = cpu_is_omap15xx()
-				? OMAP_DMA_SYNC_FRAME
-				: OMAP_DMA_SYNC_ELEMENT;
-	int		dma_trigger = 0;
+	struct dma_async_tx_descriptor *tx;
+	struct dma_chan *dma = ep->dma;
+	dma_cookie_t cookie;
+	unsigned burst, length;
+	u16 txdma_ctrl, w;
+	struct dma_slave_config omap_udc_in_cfg = {
+		.direction = DMA_MEM_TO_DEV,
+		.dst_addr = UDC_DATA_DMA,
+	};
+
+	length = req->req.length - req->req.actual;
 
 	/* measure length in either bytes or packets */
-	if ((cpu_is_omap16xx() && length <= UDC_TXN_TSC)
-			|| (cpu_is_omap15xx() && length < ep->maxpacket)) {
+	if ((cpu_is_omap16xx() && length <= UDC_TXN_TSC) ||
+	    (cpu_is_omap15xx() && length < ep->maxpacket)) {
+		omap_udc_in_cfg.dst_addr_width = DMA_SLAVE_BUSWIDTH_1_BYTE;
 		txdma_ctrl = UDC_TXN_EOT | length;
-		omap_set_dma_transfer_params(ep->lch, OMAP_DMA_DATA_TYPE_S8,
-				length, 1, sync_mode, dma_trigger, 0);
+		burst = length;
 	} else {
-		length = min(length / ep->maxpacket,
-				(unsigned) UDC_TXN_TSC + 1);
+		omap_udc_in_cfg.dst_addr_width = DMA_SLAVE_BUSWIDTH_2_BYTE;
+		length = min_t(unsigned, length / ep->maxpacket,
+		               UDC_TXN_TSC + 1);
 		txdma_ctrl = length;
-		omap_set_dma_transfer_params(ep->lch, OMAP_DMA_DATA_TYPE_S16,
-				ep->ep.maxpacket >> 1, length, sync_mode,
-				dma_trigger, 0);
 		length *= ep->maxpacket;
+		burst = ep->ep.maxpacket >> 1;
 	}
-	omap_set_dma_src_params(ep->lch, OMAP_DMA_PORT_EMIFF,
-		OMAP_DMA_AMODE_POST_INC, req->req.dma + req->req.actual,
-		0, 0);
 
-	omap_start_dma(ep->lch);
-	ep->dma_counter = omap_get_dma_src_pos(ep->lch);
+	if (!cpu_is_omap15xx())
+		burst = 1;
+
+	omap_udc_in_cfg.dst_maxburst = burst;
+
+	if (WARN_ON(dmaengine_slave_config(dma, &omap_udc_in_cfg)))
+		return;
+
+	tx = dmaengine_prep_slave_single(dma, req->req.dma + req->req.actual,
+					 length, DMA_MEM_TO_DEV, 0);
+	if (WARN_ON(!tx))
+		return;
+
+	cookie = dmaengine_submit(tx);
+	if (WARN_ON(dma_submit_error(cookie)))
+		return;
+
+	ep->dma_cookie = cookie;
+	dma_async_issue_pending(dma);
+
 	w = omap_readw(UDC_DMA_IRQ_EN);
 	w |= UDC_TX_DONE_IE(ep->dma_channel);
 	omap_writew(w, UDC_DMA_IRQ_EN);
@@ -549,11 +532,14 @@ static void next_in_dma(struct omap_ep *ep, struct omap_req *req)
 
 static void finish_in_dma(struct omap_ep *ep, struct omap_req *req, int status)
 {
+	struct dma_tx_state state;
 	u16 w;
 
-	if (status == 0) {
-		req->req.actual += req->dma_bytes;
+	dmaengine_tx_status(ep->dma, ep->dma_cookie, &state);
 
+	req->req.actual += req->dma_bytes - state.residual;
+
+	if (status == 0) {
 		/* return if this request needs to send data or zlp */
 		if (req->req.actual < req->req.length)
 			return;
@@ -561,36 +547,47 @@ static void finish_in_dma(struct omap_ep *ep, struct omap_req *req, int status)
 				&& req->dma_bytes != 0
 				&& (req->req.actual % ep->maxpacket) == 0)
 			return;
-	} else
-		req->req.actual += dma_src_len(ep, req->req.dma
-							+ req->req.actual);
+	}
 
 	/* tx completion */
-	omap_stop_dma(ep->lch);
+	dmaengine_terminate_async(ep->dma);
+
 	w = omap_readw(UDC_DMA_IRQ_EN);
 	w &= ~UDC_TX_DONE_IE(ep->dma_channel);
 	omap_writew(w, UDC_DMA_IRQ_EN);
 	done(ep, req, status);
 }
 
+static const struct dma_slave_config omap_udc_out_cfg = {
+	.direction = DMA_DEV_TO_MEM,
+	.src_addr_width = DMA_SLAVE_BUSWIDTH_2_BYTE,
+	/*
+	 * DMAengine uses frame sync mode, setting maxburst=1
+	 * is equivalent to element sync mode.
+	 */
+	.src_maxburst = 1,
+	.src_addr = UDC_DATA_DMA,
+};
+
 static void next_out_dma(struct omap_ep *ep, struct omap_req *req)
 {
-	unsigned packets = req->req.length - req->req.actual;
-	int dma_trigger = 0;
+	struct dma_async_tx_descriptor *tx;
+	struct dma_chan *dma = ep->dma;
+	dma_cookie_t cookie;
+	unsigned packets, length;
 	u16 w;
 
-	/* set up this DMA transfer, enable the fifo, start */
-	packets /= ep->ep.maxpacket;
-	packets = min(packets, (unsigned)UDC_RXN_TC + 1);
-	req->dma_bytes = packets * ep->ep.maxpacket;
-	omap_set_dma_transfer_params(ep->lch, OMAP_DMA_DATA_TYPE_S16,
-			ep->ep.maxpacket >> 1, packets,
-			OMAP_DMA_SYNC_ELEMENT,
-			dma_trigger, 0);
-	omap_set_dma_dest_params(ep->lch, OMAP_DMA_PORT_EMIFF,
-		OMAP_DMA_AMODE_POST_INC, req->req.dma + req->req.actual,
-		0, 0);
-	ep->dma_counter = omap_get_dma_dst_pos(ep->lch);
+	length = req->req.length - req->req.actual;
+	packets = min_t(unsigned, length / ep->ep.maxpacket, UDC_RXN_TC + 1);
+	length = packets * ep->ep.maxpacket;
+
+	if (WARN_ON(dmaengine_slave_config(dma, &omap_udc_out_cfg)))
+		return;
+
+	tx = dmaengine_prep_slave_single(dma, req->req.dma + req->req.actual,
+					 length, DMA_DEV_TO_MEM, 0);
+	if (WARN_ON(!tx))
+		return;
 
 	omap_writew(UDC_RXN_STOP | (packets - 1), UDC_RXDMA(ep->dma_channel));
 	w = omap_readw(UDC_DMA_IRQ_EN);
@@ -599,29 +596,42 @@ static void next_out_dma(struct omap_ep *ep, struct omap_req *req)
 	omap_writew(ep->bEndpointAddress & 0xf, UDC_EP_NUM);
 	omap_writew(UDC_SET_FIFO_EN, UDC_CTRL);
 
-	omap_start_dma(ep->lch);
+	cookie = dmaengine_submit(tx);
+	if (WARN_ON(dma_submit_error(cookie)))
+		return;
+
+	ep->dma_cookie = cookie;
+	dma_async_issue_pending(dma);
+	req->dma_bytes = length;
 }
 
 static void
 finish_out_dma(struct omap_ep *ep, struct omap_req *req, int status, int one)
 {
+	struct dma_tx_state state;
 	u16	count, w;
 
-	if (status == 0)
-		ep->dma_counter = (u16) (req->req.dma + req->req.actual);
-	count = dma_dest_len(ep, req->req.dma + req->req.actual);
+	dmaengine_tx_status(ep->dma, ep->dma_cookie, &state);
+
+	count = req->dma_bytes - state.residual;
 	count += req->req.actual;
 	if (one)
 		count--;
+
+	/*
+	 * FIXME: Surely if count > req->req.length, something has gone
+	 * seriously wrong and we've scribbled over memory we should not...
+	 * so surely we should be a WARN_ON() at the very least?
+	 */
 	if (count <= req->req.length)
 		req->req.actual = count;
 
-	if (count != req->dma_bytes || status)
-		omap_stop_dma(ep->lch);
-
+	if (count != req->dma_bytes || status) {
+		dmaengine_terminate_async(ep->dma);
 	/* if this wasn't short, request may need another transfer */
-	else if (req->req.actual < req->req.length)
+	} else if (req->req.actual < req->req.length) {
 		return;
+	}
 
 	/* rx completion */
 	w = omap_readw(UDC_DMA_IRQ_EN);
@@ -683,19 +693,10 @@ static void dma_irq(struct omap_udc *udc, u16 irq_src)
 	}
 }
 
-static void dma_error(int lch, u16 ch_status, void *data)
-{
-	struct omap_ep	*ep = data;
-
-	/* if ch_status & OMAP_DMA_DROP_IRQ ... */
-	/* if ch_status & OMAP1_DMA_TOUT_IRQ ... */
-	ERR("%s dma error, lch %d status %02x\n", ep->ep.name, lch, ch_status);
-
-	/* complete current transfer ... */
-}
-
 static void dma_channel_claim(struct omap_ep *ep, unsigned channel)
 {
+	dma_cap_mask_t mask;
+	struct dma_chan *dma;
 	u16	reg;
 	int	status, restart, is_in;
 	int	dma_channel;
@@ -708,7 +709,7 @@ static void dma_channel_claim(struct omap_ep *ep, unsigned channel)
 	reg |= UDC_DMA_REQ;		/* "pulse" activated */
 
 	ep->dma_channel = 0;
-	ep->lch = -1;
+	ep->dma = NULL;
 	if (channel == 0 || channel > 3) {
 		if ((reg & 0x0f00) == 0)
 			channel = 3;
@@ -722,65 +723,41 @@ static void dma_channel_claim(struct omap_ep *ep, unsigned channel)
 		}
 	}
 	reg |= (0x0f & ep->bEndpointAddress) << (4 * (channel - 1));
-	ep->dma_channel = channel;
 
-	if (is_in) {
+	dma_cap_zero(mask);
+	dma_cap_set(DMA_SLAVE, mask);
+
+	if (is_in)
 		dma_channel = OMAP_DMA_USB_W2FC_TX0 - 1 + channel;
-		status = omap_request_dma(dma_channel,
-			ep->ep.name, dma_error, ep, &ep->lch);
-		if (status == 0) {
-			omap_writew(reg, UDC_TXDMA_CFG);
-			/* EMIFF or SDRC */
-			omap_set_dma_src_burst_mode(ep->lch,
-						OMAP_DMA_DATA_BURST_4);
-			omap_set_dma_src_data_pack(ep->lch, 1);
-			/* TIPB */
-			omap_set_dma_dest_params(ep->lch,
-				OMAP_DMA_PORT_TIPB,
-				OMAP_DMA_AMODE_CONSTANT,
-				UDC_DATA_DMA,
-				0, 0);
-		}
-	} else {
+	else
 		dma_channel = OMAP_DMA_USB_W2FC_RX0 - 1 + channel;
-		status = omap_request_dma(dma_channel,
-			ep->ep.name, dma_error, ep, &ep->lch);
-		if (status == 0) {
+
+	dma = __dma_request_channel(&mask, omap_dma_filter_fn,
+				    (void *)dma_channel);
+	if (dma) {
+		ep->dma_channel = channel;
+		ep->dma = dma;
+		if (is_in)
+			omap_writew(reg, UDC_TXDMA_CFG);
+		else
 			omap_writew(reg, UDC_RXDMA_CFG);
-			/* TIPB */
-			omap_set_dma_src_params(ep->lch,
-				OMAP_DMA_PORT_TIPB,
-				OMAP_DMA_AMODE_CONSTANT,
-				UDC_DATA_DMA,
-				0, 0);
-			/* EMIFF or SDRC */
-			omap_set_dma_dest_burst_mode(ep->lch,
-						OMAP_DMA_DATA_BURST_4);
-			omap_set_dma_dest_data_pack(ep->lch, 1);
-		}
-	}
-	if (status)
-		ep->dma_channel = 0;
-	else {
 		ep->has_dma = 1;
-		omap_disable_dma_irq(ep->lch, OMAP_DMA_BLOCK_IRQ);
-
-		/* channel type P: hw synch (fifo) */
-		if (!cpu_is_omap15xx())
-			omap_set_dma_channel_mode(ep->lch, OMAP_DMA_LCH_P);
+		status = 0;
+	} else {
+		ep->dma_channel = 0;
+		status = -EINVAL;
 	}
 
 just_restart:
 	/* restart any queue, even if the claim failed  */
 	restart = !ep->stopped && !list_empty(&ep->queue);
 
-	if (status)
-		DBG("%s no dma channel: %d%s\n", ep->ep.name, status,
-			restart ? " (restart)" : "");
+	if (d->dma)
+		DBG("%s claimed %cxdma%d dmaengine %s%s\n", ep->ep.name,
+			is_in ? 't' : 'r', ep->dma_channel - 1,
+			dma_chan_name(d->dma), restart ? " (restart)" : "");
 	else
-		DBG("%s claimed %cxdma%d lch %d%s\n", ep->ep.name,
-			is_in ? 't' : 'r',
-			ep->dma_channel - 1, ep->lch,
+		DBG("%s no dma channel: %d%s\n", ep->ep.name, status,
 			restart ? " (restart)" : "");
 
 	if (restart) {
@@ -814,7 +791,8 @@ static void dma_channel_release(struct omap_ep *ep)
 	else
 		req = NULL;
 
-	active = omap_get_dma_active_status(ep->lch);
+	active = dma_async_is_tx_complete(ep->dma, ep->dma_cookie, NULL, NULL)
+			== DMA_IN_PROGRESS;
 
 	DBG("%s release %s %cxdma%d %p\n", ep->ep.name,
 			active ? "active" : "idle",
@@ -850,9 +828,9 @@ static void dma_channel_release(struct omap_ep *ep)
 		if (req)
 			finish_out_dma(ep, req, -ECONNRESET, 0);
 	}
-	omap_free_dma(ep->lch);
+	dma_release_channel(ep->dma);
 	ep->dma_channel = 0;
-	ep->lch = -1;
+	ep->dma = NULL;
 	/* has_dma still set, till endpoint is fully quiesced */
 }
 
@@ -2146,9 +2124,9 @@ static void proc_ep_show(struct seq_file *s, struct omap_ep *ep)
 	use_ep(ep, 0);
 
 	if (use_dma && ep->has_dma)
-		snprintf(buf, sizeof buf, "(%cxdma%d lch%d) ",
+		snprintf(buf, sizeof buf, "(%cxdma%d dma %s) ",
 			(ep->bEndpointAddress & USB_DIR_IN) ? 't' : 'r',
-			ep->dma_channel - 1, ep->lch);
+			ep->dma_channel - 1, dma_chan_name(ep->dma));
 	else
 		buf[0] = 0;
 
@@ -2194,9 +2172,11 @@ static void proc_ep_show(struct seq_file *s, struct omap_ep *ep)
 			unsigned	length = req->req.actual;
 
 			if (use_dma && buf[0]) {
-				length += ((ep->bEndpointAddress & USB_DIR_IN)
-						? dma_src_len : dma_dest_len)
-					(ep, req->req.dma + length);
+				struct dma_tx_state state;
+
+				dmaengine_tx_status(ep->dma, ep->dma_cookie,
+						    &state);
+				length += req->dma_bytes - state.residual;
 				buf[0] = 0;
 			}
 			seq_printf(s, "\treq %p len %d/%d buf %p\n",
diff --git a/drivers/usb/gadget/udc/omap_udc.h b/drivers/usb/gadget/udc/omap_udc.h
index 00f9e608e755..e04c48f669ed 100644
--- a/drivers/usb/gadget/udc/omap_udc.h
+++ b/drivers/usb/gadget/udc/omap_udc.h
@@ -152,7 +152,8 @@ struct omap_ep {
 	u8				ackwait;
 	u8				dma_channel;
 	u16				dma_counter;
-	int				lch;
+	struct dma_chan			*dma;
+	dma_cookie_t			dma_cookie;
 	struct omap_udc			*udc;
 	struct timer_list		timer;
 };

^ permalink raw reply related

* dmaengine: ti: omap-dma: Configure LCH_TYPE for OMAP1
From: Russell King - ARM Linux @ 2018-11-23 15:43 UTC (permalink / raw)
  To: Peter Ujfalusi
  Cc: Aaro Koskinen, vkoul, dan.j.williams, dmaengine, linux-kernel,
	tony, linux-omap

On Fri, Nov 23, 2018 at 02:35:04PM +0200, Peter Ujfalusi wrote:
> > Also we can't deal with the omap_set_dma_dest_burst_mode() setting -
> > DMAengine always uses a 64 byte burst, but udc wants a smaller burst
> > setting.  Does this matter?
> 
> The tusb also fiddled with the burst before the conversion, I believe
> what the DMAengine driver is doing should be fine. If not then we fix it
> while converting the omap_udc.

That's good news, so I can ignore that difference.

> > I'm also not sure about this:
> > 
> >         if (cpu_is_omap15xx())
> >                 end++;
> > 
> > in dma_dest_len() - is that missing from the omap-dma driver?  It looks
> > like a work-around for some problem on OMAP15xx, but I can't make sense
> > about why it's in the UDC driver rather than the legacy DMA driver.
> 
> afaik no other legacy drivers were doing similar thing, this must be
> something which is needed for the omap_udc driver to fix up something?
> 
> > 
> > I'm also confused by:
> > 
> >         end |= start & (0xffff << 16);
> > 
> > also in dma_dest_len() - omap_get_dma_dst_pos() returns in the high 16
> > bits the full address:
> > 
> >         if (dma_omap1())
> >                 offset |= (p->dma_read(CDSA, lch) & 0xFFFF0000);
> 
> CDSA is OMAP_DMA_REG_2X16BIT for omap1
> The CPC/CDAC holds the LSB of the _current_ DMA pointer. The code gets
> the MSB of the address from the CDSA registers.
> 
> > 
> > so if the address crosses a 64k physical address boundary, surely orring
> > in the start address is wrong?  The more I look at dma_dest_len(), the
> > more I wonder whether that and dma_src_len() are anywhere near correct,
> > and whether that is a source of breakage for Aaro.
> 
> Hrm, again... the position reporting on OMAP1 is certainly not something
> I would put my life on :o

My feeling is - if the code in plat-omap/dma.c doesn't work, we've got
the same problems in the dmaengine driver, so the reported residue will
be wrong.  Any workarounds need to be within the dmaengine driver, not
in individual drivers.  We can't just go subtracting 1 from the residue
reported by dmaengine.

> > diff --git a/drivers/usb/gadget/udc/omap_udc.c b/drivers/usb/gadget/udc/omap_udc.c
> > index 3a16431da321..a37e1d2f0f3e 100644
> > --- a/drivers/usb/gadget/udc/omap_udc.c
> > +++ b/drivers/usb/gadget/udc/omap_udc.c
> > @@ -204,6 +204,7 @@ static int omap_ep_enable(struct usb_ep *_ep,
> >  	ep->dma_channel = 0;
> >  	ep->has_dma = 0;
> >  	ep->lch = -1;
> > +	ep->dma = NULL;
> >  	use_ep(ep, UDC_EP_SEL);
> >  	omap_writew(udc->clr_halt, UDC_CTRL);
> >  	ep->ackwait = 0;
> > @@ -576,21 +577,49 @@ static void finish_in_dma(struct omap_ep *ep, struct omap_req *req, int status)
> >  static void next_out_dma(struct omap_ep *ep, struct omap_req *req)
> >  {
> >  	unsigned packets = req->req.length - req->req.actual;
> > -	int dma_trigger = 0;
> > +	struct dma_async_tx_descriptor *tx;
> > +	struct dma_chan *dma = ep->dma;
> > +	dma_cookie_t cookie;
> >  	u16 w;
> >  
> > -	/* set up this DMA transfer, enable the fifo, start */
> > -	packets /= ep->ep.maxpacket;
> > -	packets = min(packets, (unsigned)UDC_RXN_TC + 1);
> > +	packets = min_t(unsigned, packets / ep->ep.maxpacket, UDC_RXN_TC + 1);
> >  	req->dma_bytes = packets * ep->ep.maxpacket;
> > -	omap_set_dma_transfer_params(ep->lch, OMAP_DMA_DATA_TYPE_S16,
> > -			ep->ep.maxpacket >> 1, packets,
> > -			OMAP_DMA_SYNC_ELEMENT,
> > -			dma_trigger, 0);
> > -	omap_set_dma_dest_params(ep->lch, OMAP_DMA_PORT_EMIFF,
> > -		OMAP_DMA_AMODE_POST_INC, req->req.dma + req->req.actual,
> > -		0, 0);
> > -	ep->dma_counter = omap_get_dma_dst_pos(ep->lch);
> > +
> > +	if (dma) {
> > +		struct dma_slave_config cfg = {
> > +			.direction = DMA_DEV_TO_MEM,
> > +			.src_addr_width = DMA_SLAVE_BUSWIDTH_2_BYTE,
> > +			/*
> > +			 * DMAengine uses frame sync mode, setting maxburst=1
> > +			 * is equivalent to element sync mode.
> > +			 */
> > +			.src_maxburst = 1,
> 
> We should fix the omap-dma driver for slave_sg  instead:
> 
> if (!burst)
> 	burst = 1;
> 
> I thought that I already did that.

It isn't in 4.19, and I see no changes between 4.19 and 4.20-rc for
ti/omap-dma.c.

> > +		struct dma_chan *dma;
> > +
> >  		dma_channel = OMAP_DMA_USB_W2FC_RX0 - 1 + channel;
> > -		status = omap_request_dma(dma_channel,
> > -			ep->ep.name, dma_error, ep, &ep->lch);
> > -		if (status == 0) {
> > +
> > +		dma = __dma_request_channel(&mask, omap_dma_filter_fn,
> > +					    (void *)dma_channel);
> 
> dma_request_chan(dev, "ch_name");
> 
> where ch_name: rx0/1/2, tx0/1/2
> 
> and we don't need the omap_dma_filter_fn in here as all taken care via
> the dma_slave_map

Yea, we can switch to that once the DT has been modified, but let's
try to keep the conversion as separate small steps at the moment.

> I try to give this a try, thanks Russell for the patch!

Thanks for the response, I'll rip out the old non-DMAengine handling
for OUT transfers given your responses so far - I've been though all
the register constructions, and it looks like I've broadly got it
right, with the differences that I've already noted.

I'll send an updated patch shortly.

I've just spotted that I've missed a call to dma_dest_len() in
proc_ep_show() though...

^ permalink raw reply

* dmaengine: ti: omap-dma: Configure LCH_TYPE for OMAP1
From: Peter Ujfalusi @ 2018-11-23 12:35 UTC (permalink / raw)
  To: Russell King - ARM Linux, Aaro Koskinen
  Cc: vkoul, dan.j.williams, dmaengine, linux-kernel, tony, linux-omap

On 22/11/2018 17.12, Russell King - ARM Linux wrote:
> On Thu, Nov 22, 2018 at 10:29:48AM +0000, Russell King - ARM Linux wrote:
>> On Tue, Nov 20, 2018 at 11:04:06PM +0200, Aaro Koskinen wrote:
>>> I had switched to PIO mode in 2015 since the WARNs about legacy DMA
>>> API were too annoying and flooding the console. And now that I tried
>>> using DMA again with g_ether, it doesn't work anymore. The device get's
>>> recognized on host side, but no traffic goes through. Switching back to
>>> PIO makes it to work again.
>>
>> A solution to that would be to do what the warning message says, and
>> update the driver to the DMAengine API.
> 
> Here's a partial conversion (not even build tested) - it only supports
> OUT transfers with dmaengine at the moment.

Thanks!

What I learned with the tusb that we need to rearrange things for
DMAengine (4cadc711cdc7 usb: musb: tusb6010_omap: Allocate DMA channels
upfront)

But that was within the musb framework, so omap_udc might be simpler.

> There's at least one thing that doesn't make sense - the driver
> apparently can transfer more than req->length bytes, surely if it does
> that, it's a serious problem - shouldn't it be noisy about that?


> Also we can't deal with the omap_set_dma_dest_burst_mode() setting -
> DMAengine always uses a 64 byte burst, but udc wants a smaller burst
> setting.  Does this matter?

The tusb also fiddled with the burst before the conversion, I believe
what the DMAengine driver is doing should be fine. If not then we fix it
while converting the omap_udc.

> 
> I've kept the old code for reference (and the driver will fall back if
> we can't get a dmaengine channel.)  I haven't been through and checked
> that we result in the channel setup largely the same either.
> 
> There will be one major difference - UDC uses element sync, where
> an element is 16bits, ep->ep.maxpacket/2 in a frame, and "packets"
> frames.  DMAengine is using frame sync, with a 16bit element, one
> element in a frame, and packets*ep->ep.maxpacket/2 frames.  This
> should be functionally equivalent but I'd like confirmation of that.

Yes, I think it should be fine also.

> 
> I'm also not sure about this:
> 
>         if (cpu_is_omap15xx())
>                 end++;
> 
> in dma_dest_len() - is that missing from the omap-dma driver?  It looks
> like a work-around for some problem on OMAP15xx, but I can't make sense
> about why it's in the UDC driver rather than the legacy DMA driver.

afaik no other legacy drivers were doing similar thing, this must be
something which is needed for the omap_udc driver to fix up something?

> 
> I'm also confused by:
> 
>         end |= start & (0xffff << 16);
> 
> also in dma_dest_len() - omap_get_dma_dst_pos() returns in the high 16
> bits the full address:
> 
>         if (dma_omap1())
>                 offset |= (p->dma_read(CDSA, lch) & 0xFFFF0000);

CDSA is OMAP_DMA_REG_2X16BIT for omap1
The CPC/CDAC holds the LSB of the _current_ DMA pointer. The code gets
the MSB of the address from the CDSA registers.

> 
> so if the address crosses a 64k physical address boundary, surely orring
> in the start address is wrong?  The more I look at dma_dest_len(), the
> more I wonder whether that and dma_src_len() are anywhere near correct,
> and whether that is a source of breakage for Aaro.

Hrm, again... the position reporting on OMAP1 is certainly not something
I would put my life on :o

> As I've already said, I've no way to test this on hardware.
> 
> Please review and let me know whether I missed anything on the OUT
> handling path.
> 
> Fixing the IN path is going to be a bit more head-scratching.
> 
>  drivers/usb/gadget/udc/omap_udc.c | 154 +++++++++++++++++++++++++++++---------
>  drivers/usb/gadget/udc/omap_udc.h |   2 +
>  2 files changed, 120 insertions(+), 36 deletions(-)
> 
> diff --git a/drivers/usb/gadget/udc/omap_udc.c b/drivers/usb/gadget/udc/omap_udc.c
> index 3a16431da321..a37e1d2f0f3e 100644
> --- a/drivers/usb/gadget/udc/omap_udc.c
> +++ b/drivers/usb/gadget/udc/omap_udc.c
> @@ -204,6 +204,7 @@ static int omap_ep_enable(struct usb_ep *_ep,
>  	ep->dma_channel = 0;
>  	ep->has_dma = 0;
>  	ep->lch = -1;
> +	ep->dma = NULL;
>  	use_ep(ep, UDC_EP_SEL);
>  	omap_writew(udc->clr_halt, UDC_CTRL);
>  	ep->ackwait = 0;
> @@ -576,21 +577,49 @@ static void finish_in_dma(struct omap_ep *ep, struct omap_req *req, int status)
>  static void next_out_dma(struct omap_ep *ep, struct omap_req *req)
>  {
>  	unsigned packets = req->req.length - req->req.actual;
> -	int dma_trigger = 0;
> +	struct dma_async_tx_descriptor *tx;
> +	struct dma_chan *dma = ep->dma;
> +	dma_cookie_t cookie;
>  	u16 w;
>  
> -	/* set up this DMA transfer, enable the fifo, start */
> -	packets /= ep->ep.maxpacket;
> -	packets = min(packets, (unsigned)UDC_RXN_TC + 1);
> +	packets = min_t(unsigned, packets / ep->ep.maxpacket, UDC_RXN_TC + 1);
>  	req->dma_bytes = packets * ep->ep.maxpacket;
> -	omap_set_dma_transfer_params(ep->lch, OMAP_DMA_DATA_TYPE_S16,
> -			ep->ep.maxpacket >> 1, packets,
> -			OMAP_DMA_SYNC_ELEMENT,
> -			dma_trigger, 0);
> -	omap_set_dma_dest_params(ep->lch, OMAP_DMA_PORT_EMIFF,
> -		OMAP_DMA_AMODE_POST_INC, req->req.dma + req->req.actual,
> -		0, 0);
> -	ep->dma_counter = omap_get_dma_dst_pos(ep->lch);
> +
> +	if (dma) {
> +		struct dma_slave_config cfg = {
> +			.direction = DMA_DEV_TO_MEM,
> +			.src_addr_width = DMA_SLAVE_BUSWIDTH_2_BYTE,
> +			/*
> +			 * DMAengine uses frame sync mode, setting maxburst=1
> +			 * is equivalent to element sync mode.
> +			 */
> +			.src_maxburst = 1,

We should fix the omap-dma driver for slave_sg  instead:

if (!burst)
	burst = 1;

I thought that I already did that.

> +			.src_addr = UDC_DATA_DMA,
> +		};
> +
> +		if (WARN_ON(dmaengine_slave_config(dma, &cfg)))
> +			return;
> +
> +		tx = dmaengine_prep_slave_single(dma,
> +						 req->req.dma + req->req.actual,
> +						 req->dma_bytes,
> +						 DMA_DEV_TO_MEM, 0);
> +		if (WARN_ON(!tx))
> +			return;
> +	} else {
> +		int dma_trigger = 0;
> +
> +		/* set up this DMA transfer, enable the fifo, start */
> +		/* dt = S16, cen = ep->ep.maxpacket / 2, cfn = packets */
> +		omap_set_dma_transfer_params(ep->lch, OMAP_DMA_DATA_TYPE_S16,
> +				ep->ep.maxpacket >> 1, packets,
> +				OMAP_DMA_SYNC_ELEMENT,
> +				dma_trigger, 0);
> +		omap_set_dma_dest_params(ep->lch, OMAP_DMA_PORT_EMIFF,
> +			OMAP_DMA_AMODE_POST_INC, req->req.dma + req->req.actual,
> +			0, 0);
> +		ep->dma_counter = omap_get_dma_dst_pos(ep->lch);
> +	}
>  
>  	omap_writew(UDC_RXN_STOP | (packets - 1), UDC_RXDMA(ep->dma_channel));
>  	w = omap_readw(UDC_DMA_IRQ_EN);
> @@ -599,7 +628,15 @@ static void next_out_dma(struct omap_ep *ep, struct omap_req *req)
>  	omap_writew(ep->bEndpointAddress & 0xf, UDC_EP_NUM);
>  	omap_writew(UDC_SET_FIFO_EN, UDC_CTRL);
>  
> -	omap_start_dma(ep->lch);
> +	if (dma) {
> +		cookie = dmaengine_submit(tx);
> +		if (WARN_ON(dma_submit_error(cookie)))
> +			return;
> +		ep->dma_cookie = cookie;
> +		dma_async_issue_pending(dma);
> +	} else {
> +		omap_start_dma(ep->lch);
> +	}
>  }
>  
>  static void
> @@ -607,21 +644,39 @@ finish_out_dma(struct omap_ep *ep, struct omap_req *req, int status, int one)
>  {
>  	u16	count, w;
>  
> -	if (status == 0)
> -		ep->dma_counter = (u16) (req->req.dma + req->req.actual);
> -	count = dma_dest_len(ep, req->req.dma + req->req.actual);
> +	if (ep->dma) {
> +		struct dma_tx_state state;
> +
> +		dmaengine_tx_status(ep->dma, ep->dma_cookie, &state);
> +
> +		count = req->dma_bytes - state.residual;
> +	} else {
> +		if (status == 0)
> +			ep->dma_counter = (u16) (req->req.dma + req->req.actual);
> +		count = dma_dest_len(ep, req->req.dma + req->req.actual);
> +	}
> +
>  	count += req->req.actual;
>  	if (one)
>  		count--;
> +
> +	/*
> +	 * FIXME: Surely if count > req->req.length, something has gone
> +	 * seriously wrong and we've scribbled over memory we should not...
> +	 * so surely we should be a WARN_ON() at the very least?
> +	 */
>  	if (count <= req->req.length)
>  		req->req.actual = count;
>  
> -	if (count != req->dma_bytes || status)
> -		omap_stop_dma(ep->lch);
> -
> +	if (count != req->dma_bytes || status) {
> +		if (ep->dma)
> +			dmaengine_terminate_async(ep->dma);
> +		else
> +			omap_stop_dma(ep->lch);
>  	/* if this wasn't short, request may need another transfer */
> -	else if (req->req.actual < req->req.length)
> +	} else if (req->req.actual < req->req.length) {
>  		return;
> +	}
>  
>  	/* rx completion */
>  	w = omap_readw(UDC_DMA_IRQ_EN);
> @@ -709,6 +764,7 @@ static void dma_channel_claim(struct omap_ep *ep, unsigned channel)
>  
>  	ep->dma_channel = 0;
>  	ep->lch = -1;
> +	ep->dma = NULL;
>  	if (channel == 0 || channel > 3) {
>  		if ((reg & 0x0f00) == 0)
>  			channel = 3;
> @@ -742,26 +798,44 @@ static void dma_channel_claim(struct omap_ep *ep, unsigned channel)
>  				0, 0);
>  		}
>  	} else {
> +		struct dma_chan *dma;
> +
>  		dma_channel = OMAP_DMA_USB_W2FC_RX0 - 1 + channel;
> -		status = omap_request_dma(dma_channel,
> -			ep->ep.name, dma_error, ep, &ep->lch);
> -		if (status == 0) {
> +
> +		dma = __dma_request_channel(&mask, omap_dma_filter_fn,
> +					    (void *)dma_channel);

dma_request_chan(dev, "ch_name");

where ch_name: rx0/1/2, tx0/1/2

and we don't need the omap_dma_filter_fn in here as all taken care via
the dma_slave_map


> +		if (dma) {
> +			ep->dma = dma;
>  			omap_writew(reg, UDC_RXDMA_CFG);
> -			/* TIPB */
> -			omap_set_dma_src_params(ep->lch,
> -				OMAP_DMA_PORT_TIPB,
> -				OMAP_DMA_AMODE_CONSTANT,
> -				UDC_DATA_DMA,
> -				0, 0);
> -			/* EMIFF or SDRC */
> -			omap_set_dma_dest_burst_mode(ep->lch,
> -						OMAP_DMA_DATA_BURST_4);
> -			omap_set_dma_dest_data_pack(ep->lch, 1);
> +		} else {
> +			status = omap_request_dma(dma_channel,
> +				ep->ep.name, dma_error, ep, &ep->lch);
> +			if (status == 0) {
> +				omap_writew(reg, UDC_RXDMA_CFG);
> +				/* TIPB */
> +				omap_set_dma_src_params(ep->lch,
> +					OMAP_DMA_PORT_TIPB,
> +					OMAP_DMA_AMODE_CONSTANT,
> +					UDC_DATA_DMA,
> +					0, 0);
> +				/* EMIFF or SDRC */
> +				/*
> +				 * not ok - CSDP_DST_BURST_64 selected, but this selects
> +				 * CSDP_DST_BURST_16 on omap2+ and CSDP_DST_BURST_32 on
> +				 * omap1.
> +				 */
> +				omap_set_dma_dest_burst_mode(ep->lch,
> +							OMAP_DMA_DATA_BURST_4);
> +				/* ok - CSDP_DST_PACKED set for dmaengine */
> +				omap_set_dma_dest_data_pack(ep->lch, 1);
> +			}
>  		}
>  	}
> -	if (status)
> +	if (d->dma) {
> +		ep->has_dma = 1;
> +	} else if (status) {
>  		ep->dma_channel = 0;
> -	else {
> +	} else {
>  		ep->has_dma = 1;
>  		omap_disable_dma_irq(ep->lch, OMAP_DMA_BLOCK_IRQ);
>  
> @@ -777,6 +851,10 @@ static void dma_channel_claim(struct omap_ep *ep, unsigned channel)
>  	if (status)
>  		DBG("%s no dma channel: %d%s\n", ep->ep.name, status,
>  			restart ? " (restart)" : "");
> +	else if (d->dma)
> +		DBG("%s claimed %cxdma%d dmaengine %s%s\n", ep->ep.name,
> +			is_in ? 't' : 'r', ep->dma_channel - 1,
> +			dma_chan_name(d->dma), restart ? " (restart)" : "");
>  	else
>  		DBG("%s claimed %cxdma%d lch %d%s\n", ep->ep.name,
>  			is_in ? 't' : 'r',
> @@ -850,9 +928,13 @@ static void dma_channel_release(struct omap_ep *ep)
>  		if (req)
>  			finish_out_dma(ep, req, -ECONNRESET, 0);
>  	}
> -	omap_free_dma(ep->lch);
> +	if (ep->dma)
> +		dma_release_channel(ep->dma);
> +	else
> +		omap_free_dma(ep->lch);
>  	ep->dma_channel = 0;
>  	ep->lch = -1;
> +	ep->dma = NULL;
>  	/* has_dma still set, till endpoint is fully quiesced */
>  }
>  
> diff --git a/drivers/usb/gadget/udc/omap_udc.h b/drivers/usb/gadget/udc/omap_udc.h
> index 00f9e608e755..68857ae8d763 100644
> --- a/drivers/usb/gadget/udc/omap_udc.h
> +++ b/drivers/usb/gadget/udc/omap_udc.h
> @@ -153,6 +153,8 @@ struct omap_ep {
>  	u8				dma_channel;
>  	u16				dma_counter;
>  	int				lch;
> +	struct dma_chan			*dma;
> +	dma_cookie_t			dma_cookie;
>  	struct omap_udc			*udc;
>  	struct timer_list		timer;
>  };

I try to give this a try, thanks Russell for the patch!

> 

- Péter

Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

^ permalink raw reply

* dmaengine: ti: omap-dma: Configure LCH_TYPE for OMAP1
From: Peter Ujfalusi @ 2018-11-23 11:54 UTC (permalink / raw)
  To: Russell King - ARM Linux, Aaro Koskinen
  Cc: vkoul, dan.j.williams, dmaengine, linux-kernel, tony, linux-omap

On 23/11/2018 2.25, Russell King - ARM Linux wrote:
> On Fri, Nov 23, 2018 at 12:24:26AM +0200, Aaro Koskinen wrote:
>> Hi,
>>
>> On Thu, Nov 22, 2018 at 03:12:36PM +0000, Russell King - ARM Linux wrote:
>>> On Thu, Nov 22, 2018 at 10:29:48AM +0000, Russell King - ARM Linux wrote:
>>>> On Tue, Nov 20, 2018 at 11:04:06PM +0200, Aaro Koskinen wrote:
>>>>> I had switched to PIO mode in 2015 since the WARNs about legacy DMA
>>>>> API were too annoying and flooding the console. And now that I tried
>>>>> using DMA again with g_ether, it doesn't work anymore. The device get's
>>>>> recognized on host side, but no traffic goes through. Switching back to
>>>>> PIO makes it to work again.
>>>>
>>>> A solution to that would be to do what the warning message says, and
>>>> update the driver to the DMAengine API.
>>
>> Fully agreed, but I was busy debugging other more serious issues, and
>> just wanted to get a reliable ssh or USB serial access to the device
>> without any extra noise, so switching to PIO using a module parameter
>> is probably what most users do in such situations.
>>
>>> Here's a partial conversion (not even build tested) - it only supports
>>> OUT transfers with dmaengine at the moment.
>>
>> Thanks, I'll take a closer look and try to do some testing hopefully
>> during the weekend.
> 
> The patch was more for Peter to take a peek at - there's definitely
> some bits missing in the dmaengine driver (like the write to the
> LCH_CTRL register) that would need to be fixed somehow.
> 
> However, it's worth noting that there is exactly one user of
> omap_set_dma_channel_mode(), which is omap-udc, which means any DMA
> channel made use of by omap-udc will have the LCH_CTRL register
> modified to LCH_P, and it will remain that way even if someone else
> subsequently makes use of the same channel.  That's rather suspicious
> to me... maybe we can just initialise all LCH_CTRL registers to LCH_P
> in the dmaengine driver in that case!  If not, then there's a bug
> right there.

Hrm, right. memcpy will break if we take a channel which was used by
omap_udc at some point as LCH_P is not capable of dealing with it.

With this patch it should be fine as we configure the LCH_CTRL to P or
2D depending on the transfer type (slave vs non-slave).
But, we might just set the lch to 2D regardless as it works for slave
and memcpy channels fine.

> 

- Péter

Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

^ permalink raw reply

* dmaengine: ti: omap-dma: Configure LCH_TYPE for OMAP1
From: Peter Ujfalusi @ 2018-11-23 11:49 UTC (permalink / raw)
  To: Russell King - ARM Linux, Aaro Koskinen
  Cc: vkoul, dan.j.williams, dmaengine, linux-kernel, tony, linux-omap

On 22/11/2018 12.29, Russell King - ARM Linux wrote:
> On Tue, Nov 20, 2018 at 11:04:06PM +0200, Aaro Koskinen wrote:
>> I had switched to PIO mode in 2015 since the WARNs about legacy DMA
>> API were too annoying and flooding the console. And now that I tried
>> using DMA again with g_ether, it doesn't work anymore. The device get's
>> recognized on host side, but no traffic goes through. Switching back to
>> PIO makes it to work again.
> 
> A solution to that would be to do what the warning message says, and
> update the driver to the DMAengine API.

Yep, omap_udc is the last user of legacy omap_dma API. It is a slow
progress as I do the conversion in my free time, onenand/omap2 and tusb
was converted not too long ago, let's see how the omap_udc is going to go.

> The reason it didn't get updated when the DMAengine conversion happened
> is because I don't have hardware for it, so had no way to test, and no
> one seemed to know that anyone was using it.  Eventually, the WARN_ON()
> was added to try and root out any users and generate interest in
> updating the drivers.  Obviously that didn't happen, because people
> just worked around the warning rather than saying anything.
> 
> I'm afraid we're long past the time that I'd be willing to update the
> omap_udc driver now as I've dropped most of my knowledge on that as
> it's been four years, and Peter has been looking after OMAP DMAengine
> issues since.
> 
> Sorry.
> 

- Péter

Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

^ permalink raw reply

* dmaengine: ti: omap-dma: Configure LCH_TYPE for OMAP1
From: Peter Ujfalusi @ 2018-11-23 11:45 UTC (permalink / raw)
  To: Aaro Koskinen
  Cc: vkoul, dan.j.williams, dmaengine, linux-kernel, tony, linux-omap,
	rmk+kernel, Felipe Balbi

On 23/11/2018 0.01, Aaro Koskinen wrote:
> Hi,
> 
> On Thu, Nov 22, 2018 at 10:31:31AM +0200, Peter Ujfalusi wrote:
>> On 20/11/2018 23.04, Aaro Koskinen wrote:
>>> On Tue, Nov 20, 2018 at 09:28:37AM +0200, Peter Ujfalusi wrote:
>>>> On 19/11/2018 20.46, Aaro Koskinen wrote:
>>>>> On Mon, Nov 19, 2018 at 12:40:40PM +0200, Peter Ujfalusi wrote:
>>>>>> When the channel is configured for slave operation the LCH_TYPE needs to be
>>>>>> set to LCh-P. For memcpy channels the LCH_TYPE must be set to LCh-2D.
>>>>>>
>>>>>> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
>>>>>
>>>>> I don't have the documentation, but based on what omap_udc driver (still
>>>>> using the legacy OMAP DMA API) does this seems to be correct.
>>>>
>>>> They are hard to fine, true. From the omap1710 TRM:
>>>>
>>>> Logical channel types (LCh types) supported are:
>>>> - LCh-2D for nonsynchronized transfers (memory transfers, 1D and 2D)
>>>> - LCh-P for synchronized transfers (mostly peripheral transfers)
>>>> - LCh-PD similar to LCh-P but runs on a dedicated physical channel
>>>> - LCh-G for graphical transfers/operations
>>>> - LCh-D for display transfers
>>>
>>> (I found a public document "OMAP5912 Multimedia Processor Direct
>>> Memory Access (DMA) Support Reference Guide", documenting these; easy
>>> to confuse with "OMAP5910 Dual-Core Processor System DMA Controller
>>> Reference Guide".)
>>>
>>>> Looking at other part it looks like hat LCH-2D channel mode can happily
>>>> service a peripheral. LCH-P supports the same features as LCH-2D, but it
>>>> lacks support for Single/Double-indexed addressing mode on the
>>>> peripheral port side.
>>>>
>>>> So, this patch might not be needed at all. Can you test the omap_udc
>>>> with s/OMAP_DMA_LCH_P/OMAP_DMA_LCH_2D
>>>>
>>>> If USB works, then we can just drop this patch.
>>>
>>> Unfortunately omap_udc does not seem to work at all anymore with DMA on
>>> my 770 setup. :-(
>>>
>>> I had switched to PIO mode in 2015 since the WARNs about legacy DMA
>>> API were too annoying and flooding the console. And now that I tried
>>> using DMA again with g_ether, it doesn't work anymore. The device get's
>>> recognized on host side, but no traffic goes through. Switching back to
>>> PIO makes it to work again.
>>
>> After some tinkering I got omap_udc working with DMA (not DMAengine,
>> yet) on 770 - using nfsroot. Configuring the channels to OMAP_DMA_LCH_2D
>> works as expected.
> 
> Would be interesting to know how you got it working with DMA. Which
> gadget driver were you using?
> 
> I bisected my issue, and got:
> 
> commit 387f869d2579e379ee343f5493dcd360be60f5c6 (refs/bisect/bad)
> Author: Felipe Balbi <felipe.balbi@linux.intel.com>
> Date:   Wed Mar 22 13:25:18 2017 +0200
> 
>     usb: gadget: u_ether: conditionally align transfer size

I just:
commit 0d61d79625202c1c4fcf07fb960e27984a3657a3
Author: Peter Ujfalusi <peter.ujfalusi@ti.com>
Date:   Thu Nov 22 10:36:55 2018 +0200

    usb: gadget: omap_udc: HACK: Make RX dma work
    
    partial packets do work...
    
    Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>


> With that reverted, the DMA works OK (and I can also now confirm that
> OMAP_DMA_LCH_2D works). I haven't yet checked if we actually need that
> quirk in OMAP UDC,

The omap_udc driver is a bit of a mess, need to check it myself, but for
now we can just set the quirk_ep_out_aligned_size and investigate later.

> or if this is related to RMK's findings of potential
> bugs in the driver. Anyway, there is clearly yet another regression.

I'll check Russell's mail.

> 
> A.
> 

- Péter

Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

diff --git a/drivers/usb/gadget/udc/omap_udc.c b/drivers/usb/gadget/udc/omap_udc.c
index 94128eb69d97..0748c3841a25 100644
--- a/drivers/usb/gadget/udc/omap_udc.c
+++ b/drivers/usb/gadget/udc/omap_udc.c
@@ -886,14 +886,14 @@ omap_ep_queue(struct usb_ep *_ep, struct usb_request *_req, gfp_t gfp_flags)
 
        /* this isn't bogus, but OMAP DMA isn't the only hardware to
         * have a hard time with partial packet reads...  reject it.
+        * Wait a minute, it does work :o
         */
        if (use_dma
                        && ep->has_dma
                        && ep->bEndpointAddress != 0
                        && (ep->bEndpointAddress & USB_DIR_IN) == 0
                        && (req->req.length % ep->ep.maxpacket) != 0) {
-               DBG("%s, no partial packet OUT reads\n", __func__);
-               return -EMSGSIZE;
+               DBG("%s, partial packet OUT, might not work?\n", __func__);
        }
 
        udc = ep->udc;

^ permalink raw reply related

* mm: Replace all open encodings for NUMA_NO_NODE
From: Anshuman Khandual @ 2018-11-23 11:16 UTC (permalink / raw)
  To: David Hildenbrand, linux-mm, linux-kernel
  Cc: ocfs2-devel, linux-fbdev, dri-devel, netdev, intel-wired-lan,
	linux-media, iommu, linux-rdma, dmaengine, linux-block,
	sparclinux, linuxppc-dev, linux-ia64, linux-alpha, akpm,
	jiangqi903, hverkuil

min

On 11/23/2018 04:06 PM, David Hildenbrand wrote:
> On 23.11.18 10:54, Anshuman Khandual wrote:
>> At present there are multiple places where invalid node number is encoded
>> as -1. Even though implicitly understood it is always better to have macros
>> in there. Replace these open encodings for an invalid node number with the
>> global macro NUMA_NO_NODE. This helps remove NUMA related assumptions like
>> 'invalid node' from various places redirecting them to a common definition.
>>
>> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
>> ---
>>
>> Changes in V1:
>>
>> - Dropped OCFS2 changes per Joseph
>> - Dropped media/video drivers changes per Hans
>>
>> RFC - https://patchwork.kernel.org/patch/10678035/
>>
>> Build tested this with multiple cross compiler options like alpha, sparc,
>> arm64, x86, powerpc, powerpc64le etc with their default config which might
>> not have compiled tested all driver related changes. I will appreciate
>> folks giving this a test in their respective build environment.
>>
>> All these places for replacement were found by running the following grep
>> patterns on the entire kernel code. Please let me know if this might have
>> missed some instances. This might also have replaced some false positives.
>> I will appreciate suggestions, inputs and review.
>>
>> 1. git grep "nid == -1"
>> 2. git grep "node == -1"
>> 3. git grep "nid = -1"
>> 4. git grep "node = -1"
> 
> Hopefully you found most users :)

I hope so :)

> 
> Did you check if some are encoded into function calls? f(-1, ...)

Not really. Just wondering how do we even search for it. There might be
higher level functions passing down -1 to core MM. If you have some
instances in mind which need replacement I will accommodate them.

> 
> Reviewed-by: David Hildenbrand <david@redhat.com>

Thanks for the review.

^ permalink raw reply

* mm: Replace all open encodings for NUMA_NO_NODE
From: David Hildenbrand @ 2018-11-23 10:36 UTC (permalink / raw)
  To: Anshuman Khandual, linux-mm, linux-kernel
  Cc: ocfs2-devel, linux-fbdev, dri-devel, netdev, intel-wired-lan,
	linux-media, iommu, linux-rdma, dmaengine, linux-block,
	sparclinux, linuxppc-dev, linux-ia64, linux-alpha, akpm,
	jiangqi903, hverkuil

On 23.11.18 10:54, Anshuman Khandual wrote:
> At present there are multiple places where invalid node number is encoded
> as -1. Even though implicitly understood it is always better to have macros
> in there. Replace these open encodings for an invalid node number with the
> global macro NUMA_NO_NODE. This helps remove NUMA related assumptions like
> 'invalid node' from various places redirecting them to a common definition.
> 
> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
> ---
> 
> Changes in V1:
> 
> - Dropped OCFS2 changes per Joseph
> - Dropped media/video drivers changes per Hans
> 
> RFC - https://patchwork.kernel.org/patch/10678035/
> 
> Build tested this with multiple cross compiler options like alpha, sparc,
> arm64, x86, powerpc, powerpc64le etc with their default config which might
> not have compiled tested all driver related changes. I will appreciate
> folks giving this a test in their respective build environment.
> 
> All these places for replacement were found by running the following grep
> patterns on the entire kernel code. Please let me know if this might have
> missed some instances. This might also have replaced some false positives.
> I will appreciate suggestions, inputs and review.
> 
> 1. git grep "nid == -1"
> 2. git grep "node == -1"
> 3. git grep "nid = -1"
> 4. git grep "node = -1"

Hopefully you found most users :)

Did you check if some are encoded into function calls? f(-1, ...)

Reviewed-by: David Hildenbrand <david@redhat.com>

^ permalink raw reply

* mm: Replace all open encodings for NUMA_NO_NODE
From: Anshuman Khandual @ 2018-11-23  9:54 UTC (permalink / raw)
  To: linux-mm, linux-kernel
  Cc: ocfs2-devel, linux-fbdev, dri-devel, netdev, intel-wired-lan,
	linux-media, iommu, linux-rdma, dmaengine, linux-block,
	sparclinux, linuxppc-dev, linux-ia64, linux-alpha, akpm,
	jiangqi903, hverkuil

At present there are multiple places where invalid node number is encoded
as -1. Even though implicitly understood it is always better to have macros
in there. Replace these open encodings for an invalid node number with the
global macro NUMA_NO_NODE. This helps remove NUMA related assumptions like
'invalid node' from various places redirecting them to a common definition.

Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
---

Changes in V1:

- Dropped OCFS2 changes per Joseph
- Dropped media/video drivers changes per Hans

RFC - https://patchwork.kernel.org/patch/10678035/

Build tested this with multiple cross compiler options like alpha, sparc,
arm64, x86, powerpc, powerpc64le etc with their default config which might
not have compiled tested all driver related changes. I will appreciate
folks giving this a test in their respective build environment.

All these places for replacement were found by running the following grep
patterns on the entire kernel code. Please let me know if this might have
missed some instances. This might also have replaced some false positives.
I will appreciate suggestions, inputs and review.

1. git grep "nid == -1"
2. git grep "node == -1"
3. git grep "nid = -1"
4. git grep "node = -1"

 arch/alpha/include/asm/topology.h             |  2 +-
 arch/ia64/kernel/numa.c                       |  2 +-
 arch/ia64/mm/discontig.c                      |  6 +++---
 arch/ia64/sn/kernel/io_common.c               |  2 +-
 arch/powerpc/include/asm/pci-bridge.h         |  2 +-
 arch/powerpc/kernel/paca.c                    |  2 +-
 arch/powerpc/kernel/pci-common.c              |  2 +-
 arch/powerpc/mm/numa.c                        | 14 +++++++-------
 arch/powerpc/platforms/powernv/memtrace.c     |  4 ++--
 arch/sparc/kernel/auxio_32.c                  |  2 +-
 arch/sparc/kernel/pci_fire.c                  |  2 +-
 arch/sparc/kernel/pci_schizo.c                |  2 +-
 arch/sparc/kernel/pcic.c                      |  6 +++---
 arch/sparc/kernel/psycho_common.c             |  2 +-
 arch/sparc/kernel/sbus.c                      |  2 +-
 arch/sparc/mm/init_64.c                       |  6 +++---
 arch/sparc/prom/init_32.c                     |  2 +-
 arch/sparc/prom/init_64.c                     |  4 ++--
 arch/sparc/prom/tree_32.c                     | 12 ++++++------
 arch/sparc/prom/tree_64.c                     | 18 +++++++++---------
 arch/x86/include/asm/pci.h                    |  2 +-
 arch/x86/kernel/apic/x2apic_uv_x.c            |  6 +++---
 arch/x86/kernel/smpboot.c                     |  2 +-
 arch/x86/platform/olpc/olpc_dt.c              | 16 ++++++++--------
 drivers/block/mtip32xx/mtip32xx.c             |  4 ++--
 drivers/dma/dmaengine.c                       |  3 ++-
 drivers/infiniband/hw/hfi1/affinity.c         |  2 +-
 drivers/infiniband/hw/hfi1/init.c             |  2 +-
 drivers/iommu/dmar.c                          |  4 ++--
 drivers/iommu/intel-iommu.c                   |  2 +-
 drivers/misc/sgi-xp/xpc_uv.c                  |  2 +-
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |  4 ++--
 init/init_task.c                              |  2 +-
 kernel/kthread.c                              |  2 +-
 kernel/sched/fair.c                           | 15 ++++++++-------
 lib/cpumask.c                                 |  2 +-
 mm/huge_memory.c                              | 12 ++++++------
 mm/hugetlb.c                                  |  2 +-
 mm/ksm.c                                      |  2 +-
 mm/memory.c                                   |  6 +++---
 mm/memory_hotplug.c                           | 12 ++++++------
 mm/mempolicy.c                                |  2 +-
 mm/page_alloc.c                               |  4 ++--
 mm/page_ext.c                                 |  2 +-
 net/core/pktgen.c                             |  2 +-
 net/qrtr/qrtr.c                               |  2 +-
 tools/perf/bench/numa.c                       |  6 +++---
 47 files changed, 109 insertions(+), 107 deletions(-)

diff --git a/arch/alpha/include/asm/topology.h b/arch/alpha/include/asm/topology.h
index e6e13a8..f6dc89c 100644
--- a/arch/alpha/include/asm/topology.h
+++ b/arch/alpha/include/asm/topology.h
@@ -29,7 +29,7 @@ static const struct cpumask *cpumask_of_node(int node)
 {
 	int cpu;
 
-	if (node == -1)
+	if (node == NUMA_NO_NODE)
 		return cpu_all_mask;
 
 	cpumask_clear(&node_to_cpumask_map[node]);
diff --git a/arch/ia64/kernel/numa.c b/arch/ia64/kernel/numa.c
index 92c3762..1315da6 100644
--- a/arch/ia64/kernel/numa.c
+++ b/arch/ia64/kernel/numa.c
@@ -74,7 +74,7 @@ void __init build_cpu_to_node_map(void)
 		cpumask_clear(&node_to_cpu_mask[node]);
 
 	for_each_possible_early_cpu(cpu) {
-		node = -1;
+		node = NUMA_NO_NODE;
 		for (i = 0; i < NR_CPUS; ++i)
 			if (cpu_physical_id(cpu) == node_cpuid[i].phys_id) {
 				node = node_cpuid[i].nid;
diff --git a/arch/ia64/mm/discontig.c b/arch/ia64/mm/discontig.c
index 8a96578..f9c3675 100644
--- a/arch/ia64/mm/discontig.c
+++ b/arch/ia64/mm/discontig.c
@@ -227,7 +227,7 @@ void __init setup_per_cpu_areas(void)
 	 * CPUs are put into groups according to node.  Walk cpu_map
 	 * and create new groups at node boundaries.
 	 */
-	prev_node = -1;
+	prev_node = NUMA_NO_NODE;
 	ai->nr_groups = 0;
 	for (unit = 0; unit < nr_units; unit++) {
 		cpu = cpu_map[unit];
@@ -435,7 +435,7 @@ static void __init *memory_less_node_alloc(int nid, unsigned long pernodesize)
 {
 	void *ptr = NULL;
 	u8 best = 0xff;
-	int bestnode = -1, node, anynode = 0;
+	int bestnode = NUMA_NO_NODE, node, anynode = 0;
 
 	for_each_online_node(node) {
 		if (node_isset(node, memory_less_mask))
@@ -447,7 +447,7 @@ static void __init *memory_less_node_alloc(int nid, unsigned long pernodesize)
 		anynode = node;
 	}
 
-	if (bestnode == -1)
+	if (bestnode == NUMA_NO_NODE)
 		bestnode = anynode;
 
 	ptr = memblock_alloc_try_nid(pernodesize, PERCPU_PAGE_SIZE,
diff --git a/arch/ia64/sn/kernel/io_common.c b/arch/ia64/sn/kernel/io_common.c
index 8df13d0..86b3fcb 100644
--- a/arch/ia64/sn/kernel/io_common.c
+++ b/arch/ia64/sn/kernel/io_common.c
@@ -344,7 +344,7 @@ sn_common_bus_fixup(struct pci_bus *bus,
 		printk(KERN_WARNING "on node %d but only %d nodes online."
 		       "Association set to undetermined.\n",
 		       controller->node, num_online_nodes());
-		controller->node = -1;
+		controller->node = NUMA_NO_NODE;
 	}
 }
 
diff --git a/arch/powerpc/include/asm/pci-bridge.h b/arch/powerpc/include/asm/pci-bridge.h
index 94d4490..25a9e33 100644
--- a/arch/powerpc/include/asm/pci-bridge.h
+++ b/arch/powerpc/include/asm/pci-bridge.h
@@ -264,7 +264,7 @@ extern int pcibios_map_io_space(struct pci_bus *bus);
 #ifdef CONFIG_NUMA
 #define PHB_SET_NODE(PHB, NODE)		((PHB)->node = (NODE))
 #else
-#define PHB_SET_NODE(PHB, NODE)		((PHB)->node = -1)
+#define PHB_SET_NODE(PHB, NODE)		((PHB)->node = NUMA_NO_NODE)
 #endif
 
 #endif	/* CONFIG_PPC64 */
diff --git a/arch/powerpc/kernel/paca.c b/arch/powerpc/kernel/paca.c
index 913bfca..6a0bd51 100644
--- a/arch/powerpc/kernel/paca.c
+++ b/arch/powerpc/kernel/paca.c
@@ -36,7 +36,7 @@ static void *__init alloc_paca_data(unsigned long size, unsigned long align,
 	 * which will put its paca in the right place.
 	 */
 	if (cpu == boot_cpuid) {
-		nid = -1;
+		nid = NUMA_NO_NODE;
 		memblock_set_bottom_up(true);
 	} else {
 		nid = early_cpu_to_node(cpu);
diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c
index 88e4f69..14c33a9 100644
--- a/arch/powerpc/kernel/pci-common.c
+++ b/arch/powerpc/kernel/pci-common.c
@@ -132,7 +132,7 @@ struct pci_controller *pcibios_alloc_controller(struct device_node *dev)
 		int nid = of_node_to_nid(dev);
 
 		if (nid < 0 || !node_online(nid))
-			nid = -1;
+			nid = NUMA_NO_NODE;
 
 		PHB_SET_NODE(phb, nid);
 	}
diff --git a/arch/powerpc/mm/numa.c b/arch/powerpc/mm/numa.c
index 3a048e9..77808a2 100644
--- a/arch/powerpc/mm/numa.c
+++ b/arch/powerpc/mm/numa.c
@@ -215,7 +215,7 @@ static void initialize_distance_lookup_table(int nid,
  */
 static int associativity_to_nid(const __be32 *associativity)
 {
-	int nid = -1;
+	int nid = NUMA_NO_NODE;
 
 	if (min_common_depth == -1)
 		goto out;
@@ -225,7 +225,7 @@ static int associativity_to_nid(const __be32 *associativity)
 
 	/* POWER4 LPAR uses 0xffff as invalid node */
 	if (nid == 0xffff || nid >= MAX_NUMNODES)
-		nid = -1;
+		nid = NUMA_NO_NODE;
 
 	if (nid > 0 &&
 		of_read_number(associativity, 1) >= distance_ref_points_depth) {
@@ -244,7 +244,7 @@ static int associativity_to_nid(const __be32 *associativity)
  */
 static int of_node_to_nid_single(struct device_node *device)
 {
-	int nid = -1;
+	int nid = NUMA_NO_NODE;
 	const __be32 *tmp;
 
 	tmp = of_get_associativity(device);
@@ -256,7 +256,7 @@ static int of_node_to_nid_single(struct device_node *device)
 /* Walk the device tree upwards, looking for an associativity id */
 int of_node_to_nid(struct device_node *device)
 {
-	int nid = -1;
+	int nid = NUMA_NO_NODE;
 
 	of_node_get(device);
 	while (device) {
@@ -454,7 +454,7 @@ static int of_drconf_to_nid_single(struct drmem_lmb *lmb)
  */
 static int numa_setup_cpu(unsigned long lcpu)
 {
-	int nid = -1;
+	int nid = NUMA_NO_NODE;
 	struct device_node *cpu;
 
 	/*
@@ -930,7 +930,7 @@ static int hot_add_drconf_scn_to_nid(unsigned long scn_addr)
 {
 	struct drmem_lmb *lmb;
 	unsigned long lmb_size;
-	int nid = -1;
+	int nid = NUMA_NO_NODE;
 
 	lmb_size = drmem_lmb_size();
 
@@ -960,7 +960,7 @@ static int hot_add_drconf_scn_to_nid(unsigned long scn_addr)
 static int hot_add_node_scn_to_nid(unsigned long scn_addr)
 {
 	struct device_node *memory;
-	int nid = -1;
+	int nid = NUMA_NO_NODE;
 
 	for_each_node_by_type(memory, "memory") {
 		unsigned long start, size;
diff --git a/arch/powerpc/platforms/powernv/memtrace.c b/arch/powerpc/platforms/powernv/memtrace.c
index 84d038e..1ce3bfc 100644
--- a/arch/powerpc/platforms/powernv/memtrace.c
+++ b/arch/powerpc/platforms/powernv/memtrace.c
@@ -223,7 +223,7 @@ static int memtrace_online(void)
 		ent = &memtrace_array[i];
 
 		/* We have onlined this chunk previously */
-		if (ent->nid == -1)
+		if (ent->nid == NUMA_NO_NODE)
 			continue;
 
 		/* Remove from io mappings */
@@ -257,7 +257,7 @@ static int memtrace_online(void)
 		 */
 		debugfs_remove_recursive(ent->dir);
 		pr_info("Added trace memory back to node %d\n", ent->nid);
-		ent->size = ent->start = ent->nid = -1;
+		ent->size = ent->start = ent->nid = NUMA_NO_NODE;
 	}
 	if (ret)
 		return ret;
diff --git a/arch/sparc/kernel/auxio_32.c b/arch/sparc/kernel/auxio_32.c
index a32d588..39f6c59 100644
--- a/arch/sparc/kernel/auxio_32.c
+++ b/arch/sparc/kernel/auxio_32.c
@@ -120,7 +120,7 @@ void __init auxio_power_probe(void)
 	node = prom_searchsiblings(node, "obio");
 	node = prom_getchild(node);
 	node = prom_searchsiblings(node, "power");
-	if (node == 0 || (s32)node == -1)
+	if (node == 0 || (s32)node == NUMA_NO_NODE)
 		return;
 
 	/* Map the power control register. */
diff --git a/arch/sparc/kernel/pci_fire.c b/arch/sparc/kernel/pci_fire.c
index be71ae0..474d3be 100644
--- a/arch/sparc/kernel/pci_fire.c
+++ b/arch/sparc/kernel/pci_fire.c
@@ -416,7 +416,7 @@ static int pci_fire_pbm_init(struct pci_pbm_info *pbm,
 	struct device_node *dp = op->dev.of_node;
 	int err;
 
-	pbm->numa_node = -1;
+	pbm->numa_node = NUMA_NO_NODE;
 
 	pbm->pci_ops = &sun4u_pci_ops;
 	pbm->config_space_reg_bits = 12;
diff --git a/arch/sparc/kernel/pci_schizo.c b/arch/sparc/kernel/pci_schizo.c
index 934b97c..87bb231 100644
--- a/arch/sparc/kernel/pci_schizo.c
+++ b/arch/sparc/kernel/pci_schizo.c
@@ -1347,7 +1347,7 @@ static int schizo_pbm_init(struct pci_pbm_info *pbm,
 	pbm->next = pci_pbm_root;
 	pci_pbm_root = pbm;
 
-	pbm->numa_node = -1;
+	pbm->numa_node = NUMA_NO_NODE;
 
 	pbm->pci_ops = &sun4u_pci_ops;
 	pbm->config_space_reg_bits = 8;
diff --git a/arch/sparc/kernel/pcic.c b/arch/sparc/kernel/pcic.c
index ee4c9a9..d5fe898 100644
--- a/arch/sparc/kernel/pcic.c
+++ b/arch/sparc/kernel/pcic.c
@@ -476,7 +476,7 @@ static void pcic_map_pci_device(struct linux_pcic *pcic,
 	unsigned long flags;
 	int j;
 
-	if (node == 0 || node == -1) {
+	if (node == 0 || node == NUMA_NO_NODE) {
 		strcpy(namebuf, "???");
 	} else {
 		prom_getstring(node, "name", namebuf, 63); namebuf[63] = 0;
@@ -535,7 +535,7 @@ pcic_fill_irq(struct linux_pcic *pcic, struct pci_dev *dev, int node)
 	int i, ivec;
 	char namebuf[64];
 
-	if (node == 0 || node == -1) {
+	if (node == 0 || node == NUMA_NO_NODE) {
 		strcpy(namebuf, "???");
 	} else {
 		prom_getstring(node, "name", namebuf, sizeof(namebuf));
@@ -625,7 +625,7 @@ void pcibios_fixup_bus(struct pci_bus *bus)
 	list_for_each_entry(dev, &bus->devices, bus_list) {
 		node = pdev_to_pnode(&pcic->pbm, dev);
 		if(node == 0)
-			node = -1;
+			node = NUMA_NO_NODE;
 
 		/* cookies */
 		pcp = pci_devcookie_alloc();
diff --git a/arch/sparc/kernel/psycho_common.c b/arch/sparc/kernel/psycho_common.c
index 81aa91e..dcbf492 100644
--- a/arch/sparc/kernel/psycho_common.c
+++ b/arch/sparc/kernel/psycho_common.c
@@ -454,7 +454,7 @@ void psycho_pbm_init_common(struct pci_pbm_info *pbm, struct platform_device *op
 	struct device_node *dp = op->dev.of_node;
 
 	pbm->name = dp->full_name;
-	pbm->numa_node = -1;
+	pbm->numa_node = NUMA_NO_NODE;
 	pbm->chip_type = chip_type;
 	pbm->chip_version = of_getintprop_default(dp, "version#", 0);
 	pbm->chip_revision = of_getintprop_default(dp, "module-revision#", 0);
diff --git a/arch/sparc/kernel/sbus.c b/arch/sparc/kernel/sbus.c
index c133dfc..28a4aa9 100644
--- a/arch/sparc/kernel/sbus.c
+++ b/arch/sparc/kernel/sbus.c
@@ -561,7 +561,7 @@ static void __init sbus_iommu_init(struct platform_device *op)
 
 	op->dev.archdata.iommu = iommu;
 	op->dev.archdata.stc = strbuf;
-	op->dev.archdata.numa_node = -1;
+	op->dev.archdata.numa_node = NUMA_NO_NODE;
 
 	reg_base = regs + SYSIO_IOMMUREG_BASE;
 	iommu->iommu_control = reg_base + IOMMU_CONTROL;
diff --git a/arch/sparc/mm/init_64.c b/arch/sparc/mm/init_64.c
index 3c8aac2..cb1bed1 100644
--- a/arch/sparc/mm/init_64.c
+++ b/arch/sparc/mm/init_64.c
@@ -976,13 +976,13 @@ static u64 __init memblock_nid_range_sun4u(u64 start, u64 end, int *nid)
 {
 	int prev_nid, new_nid;
 
-	prev_nid = -1;
+	prev_nid = NUMA_NO_NODE;
 	for ( ; start < end; start += PAGE_SIZE) {
 		for (new_nid = 0; new_nid < num_node_masks; new_nid++) {
 			struct node_mem_mask *p = &node_masks[new_nid];
 
 			if ((start & p->mask) == p->match) {
-				if (prev_nid == -1)
+				if (prev_nid == NUMA_NO_NODE)
 					prev_nid = new_nid;
 				break;
 			}
@@ -1208,7 +1208,7 @@ int of_node_to_nid(struct device_node *dp)
 	md = mdesc_grab();
 
 	count = 0;
-	nid = -1;
+	nid = NUMA_NO_NODE;
 	mdesc_for_each_node_by_name(md, grp, "group") {
 		if (!scan_arcs_for_cfg_handle(md, grp, cfg_handle)) {
 			nid = count;
diff --git a/arch/sparc/prom/init_32.c b/arch/sparc/prom/init_32.c
index d204701..4c6e540 100644
--- a/arch/sparc/prom/init_32.c
+++ b/arch/sparc/prom/init_32.c
@@ -58,7 +58,7 @@ void __init prom_init(struct linux_romvec *rp)
 	prom_nodeops = romvec->pv_nodeops;
 
 	prom_root_node = prom_getsibling(0);
-	if ((prom_root_node == 0) || ((s32)prom_root_node == -1))
+	if ((prom_root_node == 0) || ((s32)prom_root_node == NUMA_NO_NODE))
 		prom_halt();
 
 	if((((unsigned long) prom_nodeops) == 0) || 
diff --git a/arch/sparc/prom/init_64.c b/arch/sparc/prom/init_64.c
index 103aa91..85669c0 100644
--- a/arch/sparc/prom/init_64.c
+++ b/arch/sparc/prom/init_64.c
@@ -36,13 +36,13 @@ void __init prom_init(void *cif_handler)
 	prom_cif_init(cif_handler);
 
 	prom_chosen_node = prom_finddevice(prom_chosen_path);
-	if (!prom_chosen_node || (s32)prom_chosen_node == -1)
+	if (!prom_chosen_node || (s32)prom_chosen_node == NUMA_NO_NODE)
 		prom_halt();
 
 	prom_stdout = prom_getint(prom_chosen_node, "stdout");
 
 	node = prom_finddevice("/openprom");
-	if (!node || (s32)node == -1)
+	if (!node || (s32)node == NUMA_NO_NODE)
 		prom_halt();
 
 	prom_getstring(node, "version", prom_version, sizeof(prom_version));
diff --git a/arch/sparc/prom/tree_32.c b/arch/sparc/prom/tree_32.c
index 0fed893..2d0a204 100644
--- a/arch/sparc/prom/tree_32.c
+++ b/arch/sparc/prom/tree_32.c
@@ -41,11 +41,11 @@ phandle prom_getchild(phandle node)
 {
 	phandle cnode;
 
-	if ((s32)node == -1)
+	if ((s32)node == NUMA_NO_NODE)
 		return 0;
 
 	cnode = __prom_getchild(node);
-	if (cnode == 0 || (s32)cnode == -1)
+	if (cnode == 0 || (s32)cnode == NUMA_NO_NODE)
 		return 0;
 
 	return cnode;
@@ -73,11 +73,11 @@ phandle prom_getsibling(phandle node)
 {
 	phandle sibnode;
 
-	if ((s32)node == -1)
+	if ((s32)node == NUMA_NO_NODE)
 		return 0;
 
 	sibnode = __prom_getsibling(node);
-	if (sibnode == 0 || (s32)sibnode == -1)
+	if (sibnode == 0 || (s32)sibnode == NUMA_NO_NODE)
 		return 0;
 
 	return sibnode;
@@ -220,7 +220,7 @@ static char *__prom_nextprop(phandle node, char * oprop)
  */
 char *prom_nextprop(phandle node, char *oprop, char *buffer)
 {
-	if (node == 0 || (s32)node == -1)
+	if (node == 0 || (s32)node == NUMA_NO_NODE)
 		return "";
 
 	return __prom_nextprop(node, oprop);
@@ -304,7 +304,7 @@ phandle prom_inst2pkg(int inst)
 	node = (*romvec->pv_v2devops.v2_inst2pkg)(inst);
 	restore_current();
 	spin_unlock_irqrestore(&prom_lock, flags);
-	if ((s32)node == -1)
+	if ((s32)node == NUMA_NO_NODE)
 		return 0;
 	return node;
 }
diff --git a/arch/sparc/prom/tree_64.c b/arch/sparc/prom/tree_64.c
index 989e799..2b4c515 100644
--- a/arch/sparc/prom/tree_64.c
+++ b/arch/sparc/prom/tree_64.c
@@ -44,10 +44,10 @@ phandle prom_getchild(phandle node)
 {
 	phandle cnode;
 
-	if ((s32)node == -1)
+	if ((s32)node == NUMA_NO_NODE)
 		return 0;
 	cnode = __prom_getchild(node);
-	if ((s32)cnode == -1)
+	if ((s32)cnode == NUMA_NO_NODE)
 		return 0;
 	return cnode;
 }
@@ -57,10 +57,10 @@ inline phandle prom_getparent(phandle node)
 {
 	phandle cnode;
 
-	if ((s32)node == -1)
+	if ((s32)node == NUMA_NO_NODE)
 		return 0;
 	cnode = prom_node_to_node("parent", node);
-	if ((s32)cnode == -1)
+	if ((s32)cnode == NUMA_NO_NODE)
 		return 0;
 	return cnode;
 }
@@ -77,10 +77,10 @@ phandle prom_getsibling(phandle node)
 {
 	phandle sibnode;
 
-	if ((s32)node == -1)
+	if ((s32)node == NUMA_NO_NODE)
 		return 0;
 	sibnode = __prom_getsibling(node);
-	if ((s32)sibnode == -1)
+	if ((s32)sibnode == NUMA_NO_NODE)
 		return 0;
 
 	return sibnode;
@@ -241,7 +241,7 @@ char *prom_firstprop(phandle node, char *buffer)
 	unsigned long args[7];
 
 	*buffer = 0;
-	if ((s32)node == -1)
+	if ((s32)node == NUMA_NO_NODE)
 		return buffer;
 
 	args[0] = (unsigned long) prom_nextprop_name;
@@ -267,7 +267,7 @@ char *prom_nextprop(phandle node, const char *oprop, char *buffer)
 	unsigned long args[7];
 	char buf[32];
 
-	if ((s32)node == -1) {
+	if ((s32)node == NUMA_NO_NODE) {
 		*buffer = 0;
 		return buffer;
 	}
@@ -370,7 +370,7 @@ inline phandle prom_inst2pkg(int inst)
 	p1275_cmd_direct(args);
 
 	node = (int) args[4];
-	if ((s32)node == -1)
+	if ((s32)node == NUMA_NO_NODE)
 		return 0;
 	return node;
 }
diff --git a/arch/x86/include/asm/pci.h b/arch/x86/include/asm/pci.h
index 6629636..dee2a31 100644
--- a/arch/x86/include/asm/pci.h
+++ b/arch/x86/include/asm/pci.h
@@ -141,7 +141,7 @@ cpumask_of_pcibus(const struct pci_bus *bus)
 	int node;
 
 	node = __pcibus_to_node(bus);
-	return (node == -1) ? cpu_online_mask :
+	return (node == NUMA_NO_NODE) ? cpu_online_mask :
 			      cpumask_of_node(node);
 }
 #endif
diff --git a/arch/x86/kernel/apic/x2apic_uv_x.c b/arch/x86/kernel/apic/x2apic_uv_x.c
index 391f358..3c3378a 100644
--- a/arch/x86/kernel/apic/x2apic_uv_x.c
+++ b/arch/x86/kernel/apic/x2apic_uv_x.c
@@ -1390,7 +1390,7 @@ static void __init build_socket_tables(void)
 	}
 
 	/* Set socket -> node values: */
-	lnid = -1;
+	lnid = NUMA_NO_NODE;
 	for_each_present_cpu(cpu) {
 		int nid = cpu_to_node(cpu);
 		int apicid, sockid;
@@ -1521,7 +1521,7 @@ static void __init uv_system_init_hub(void)
 			new_hub->pnode = 0xffff;
 
 		new_hub->numa_blade_id = uv_node_to_blade_id(nodeid);
-		new_hub->memory_nid = -1;
+		new_hub->memory_nid = NUMA_NO_NODE;
 		new_hub->nr_possible_cpus = 0;
 		new_hub->nr_online_cpus = 0;
 	}
@@ -1538,7 +1538,7 @@ static void __init uv_system_init_hub(void)
 
 		uv_cpu_info_per(cpu)->p_uv_hub_info = uv_hub_info_list(nodeid);
 		uv_cpu_info_per(cpu)->blade_cpu_id = uv_cpu_hub_info(cpu)->nr_possible_cpus++;
-		if (uv_cpu_hub_info(cpu)->memory_nid == -1)
+		if (uv_cpu_hub_info(cpu)->memory_nid == NUMA_NO_NODE)
 			uv_cpu_hub_info(cpu)->memory_nid = cpu_to_node(cpu);
 
 		/* Init memoryless node: */
diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
index a9134d1..c1d45dc 100644
--- a/arch/x86/kernel/smpboot.c
+++ b/arch/x86/kernel/smpboot.c
@@ -841,7 +841,7 @@ wakeup_secondary_cpu_via_init(int phys_apicid, unsigned long start_eip)
 /* reduce the number of lines printed when booting a large cpu count system */
 static void announce_cpu(int cpu, int apicid)
 {
-	static int current_node = -1;
+	static int current_node = NUMA_NO_NODE;
 	int node = early_cpu_to_node(cpu);
 	static int width, node_width;
 
diff --git a/arch/x86/platform/olpc/olpc_dt.c b/arch/x86/platform/olpc/olpc_dt.c
index 24d2175..7098127 100644
--- a/arch/x86/platform/olpc/olpc_dt.c
+++ b/arch/x86/platform/olpc/olpc_dt.c
@@ -29,10 +29,10 @@ static phandle __init olpc_dt_getsibling(phandle node)
 	const void *args[] = { (void *)node };
 	void *res[] = { &node };
 
-	if ((s32)node == -1)
+	if ((s32)node == NUMA_NO_NODE)
 		return 0;
 
-	if (olpc_ofw("peer", args, res) || (s32)node == -1)
+	if (olpc_ofw("peer", args, res) || (s32)node == NUMA_NO_NODE)
 		return 0;
 
 	return node;
@@ -43,10 +43,10 @@ static phandle __init olpc_dt_getchild(phandle node)
 	const void *args[] = { (void *)node };
 	void *res[] = { &node };
 
-	if ((s32)node == -1)
+	if ((s32)node == NUMA_NO_NODE)
 		return 0;
 
-	if (olpc_ofw("child", args, res) || (s32)node == -1) {
+	if (olpc_ofw("child", args, res) || (s32)node == NUMA_NO_NODE) {
 		pr_err("PROM: %s: fetching child failed!\n", __func__);
 		return 0;
 	}
@@ -60,7 +60,7 @@ static int __init olpc_dt_getproplen(phandle node, const char *prop)
 	int len;
 	void *res[] = { &len };
 
-	if ((s32)node == -1)
+	if ((s32)node == NUMA_NO_NODE)
 		return -1;
 
 	if (olpc_ofw("getproplen", args, res)) {
@@ -100,7 +100,7 @@ static int __init olpc_dt_nextprop(phandle node, char *prev, char *buf)
 
 	buf[0] = '\0';
 
-	if ((s32)node == -1)
+	if ((s32)node == NUMA_NO_NODE)
 		return -1;
 
 	if (olpc_ofw("nextprop", args, res) || success != 1)
@@ -115,7 +115,7 @@ static int __init olpc_dt_pkg2path(phandle node, char *buf,
 	const void *args[] = { (void *)node, buf, (void *)buflen };
 	void *res[] = { len };
 
-	if ((s32)node == -1)
+	if ((s32)node == NUMA_NO_NODE)
 		return -1;
 
 	if (olpc_ofw("package-to-path", args, res) || *len < 1)
@@ -176,7 +176,7 @@ static phandle __init olpc_dt_finddevice(const char *path)
 		return 0;
 	}
 
-	if ((s32) node == -1)
+	if ((s32) node == NUMA_NO_NODE)
 		return 0;
 
 	return node;
diff --git a/drivers/block/mtip32xx/mtip32xx.c b/drivers/block/mtip32xx/mtip32xx.c
index a7daa8a..b889452 100644
--- a/drivers/block/mtip32xx/mtip32xx.c
+++ b/drivers/block/mtip32xx/mtip32xx.c
@@ -4084,9 +4084,9 @@ static int get_least_used_cpu_on_node(int node)
 /* Helper for selecting a node in round robin mode */
 static inline int mtip_get_next_rr_node(void)
 {
-	static int next_node = -1;
+	static int next_node = NUMA_NO_NODE;
 
-	if (next_node == -1) {
+	if (next_node == NUMA_NO_NODE) {
 		next_node = first_online_node;
 		return next_node;
 	}
diff --git a/drivers/dma/dmaengine.c b/drivers/dma/dmaengine.c
index f1a441ab..1aeefc7 100644
--- a/drivers/dma/dmaengine.c
+++ b/drivers/dma/dmaengine.c
@@ -386,7 +386,8 @@ EXPORT_SYMBOL(dma_issue_pending_all);
 static bool dma_chan_is_local(struct dma_chan *chan, int cpu)
 {
 	int node = dev_to_node(chan->device->dev);
-	return node == -1 || cpumask_test_cpu(cpu, cpumask_of_node(node));
+	return node == NUMA_NO_NODE ||
+		cpumask_test_cpu(cpu, cpumask_of_node(node));
 }
 
 /**
diff --git a/drivers/infiniband/hw/hfi1/affinity.c b/drivers/infiniband/hw/hfi1/affinity.c
index 2baf38c..3e8acb8 100644
--- a/drivers/infiniband/hw/hfi1/affinity.c
+++ b/drivers/infiniband/hw/hfi1/affinity.c
@@ -777,7 +777,7 @@ void hfi1_dev_affinity_clean_up(struct hfi1_devdata *dd)
 	_dev_comp_vect_cpu_mask_clean_up(dd, entry);
 unlock:
 	mutex_unlock(&node_affinity.lock);
-	dd->node = -1;
+	dd->node = NUMA_NO_NODE;
 }
 
 /*
diff --git a/drivers/infiniband/hw/hfi1/init.c b/drivers/infiniband/hw/hfi1/init.c
index 0904490..0bf4577 100644
--- a/drivers/infiniband/hw/hfi1/init.c
+++ b/drivers/infiniband/hw/hfi1/init.c
@@ -1303,7 +1303,7 @@ static struct hfi1_devdata *hfi1_alloc_devdata(struct pci_dev *pdev,
 		dd->unit = ret;
 		list_add(&dd->list, &hfi1_dev_list);
 	}
-	dd->node = -1;
+	dd->node = NUMA_NO_NODE;
 
 	spin_unlock_irqrestore(&hfi1_devs_lock, flags);
 	idr_preload_end();
diff --git a/drivers/iommu/dmar.c b/drivers/iommu/dmar.c
index d9c748b..86a9c19 100644
--- a/drivers/iommu/dmar.c
+++ b/drivers/iommu/dmar.c
@@ -477,7 +477,7 @@ static int dmar_parse_one_rhsa(struct acpi_dmar_header *header, void *arg)
 			int node = acpi_map_pxm_to_node(rhsa->proximity_domain);
 
 			if (!node_online(node))
-				node = -1;
+				node = NUMA_NO_NODE;
 			drhd->iommu->node = node;
 			return 0;
 		}
@@ -1062,7 +1062,7 @@ static int alloc_iommu(struct dmar_drhd_unit *drhd)
 	iommu->msagaw = msagaw;
 	iommu->segment = drhd->segment;
 
-	iommu->node = -1;
+	iommu->node = NUMA_NO_NODE;
 
 	ver = readl(iommu->reg + DMAR_VER_REG);
 	pr_info("%s: reg_base_addr %llx ver %d:%d cap %llx ecap %llx\n",
diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index f3ccf02..9f6fb13 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -1772,7 +1772,7 @@ static struct dmar_domain *alloc_domain(int flags)
 		return NULL;
 
 	memset(domain, 0, sizeof(*domain));
-	domain->nid = -1;
+	domain->nid = NUMA_NO_NODE;
 	domain->flags = flags;
 	domain->has_iotlb_device = false;
 	INIT_LIST_HEAD(&domain->devices);
diff --git a/drivers/misc/sgi-xp/xpc_uv.c b/drivers/misc/sgi-xp/xpc_uv.c
index 0441abe..eef36dd 100644
--- a/drivers/misc/sgi-xp/xpc_uv.c
+++ b/drivers/misc/sgi-xp/xpc_uv.c
@@ -61,7 +61,7 @@ static struct xpc_heartbeat_uv *xpc_heartbeat_uv;
 					 XPC_NOTIFY_MSG_SIZE_UV)
 #define XPC_NOTIFY_IRQ_NAME		"xpc_notify"
 
-static int xpc_mq_node = -1;
+static int xpc_mq_node = NUMA_NO_NODE;
 
 static struct xpc_gru_mq_uv *xpc_activate_mq_uv;
 static struct xpc_gru_mq_uv *xpc_notify_mq_uv;
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index 113b38e..4fae85c 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -6414,7 +6414,7 @@ int ixgbe_setup_tx_resources(struct ixgbe_ring *tx_ring)
 {
 	struct device *dev = tx_ring->dev;
 	int orig_node = dev_to_node(dev);
-	int ring_node = -1;
+	int ring_node = NUMA_NO_NODE;
 	int size;
 
 	size = sizeof(struct ixgbe_tx_buffer) * tx_ring->count;
@@ -6508,7 +6508,7 @@ int ixgbe_setup_rx_resources(struct ixgbe_adapter *adapter,
 {
 	struct device *dev = rx_ring->dev;
 	int orig_node = dev_to_node(dev);
-	int ring_node = -1;
+	int ring_node = NUMA_NO_NODE;
 	int size;
 
 	size = sizeof(struct ixgbe_rx_buffer) * rx_ring->count;
diff --git a/init/init_task.c b/init/init_task.c
index 5aebe3b..6641836 100644
--- a/init/init_task.c
+++ b/init/init_task.c
@@ -154,7 +154,7 @@ struct task_struct init_task
 	.vtime.state	= VTIME_SYS,
 #endif
 #ifdef CONFIG_NUMA_BALANCING
-	.numa_preferred_nid = -1,
+	.numa_preferred_nid = NUMA_NO_NODE,
 	.numa_group	= NULL,
 	.numa_faults	= NULL,
 #endif
diff --git a/kernel/kthread.c b/kernel/kthread.c
index 087d18d..77f3d94 100644
--- a/kernel/kthread.c
+++ b/kernel/kthread.c
@@ -675,7 +675,7 @@ __kthread_create_worker(int cpu, unsigned int flags,
 {
 	struct kthread_worker *worker;
 	struct task_struct *task;
-	int node = -1;
+	int node = NUMA_NO_NODE;
 
 	worker = kzalloc(sizeof(*worker), GFP_KERNEL);
 	if (!worker)
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index ee271bb..d830fa7 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -1161,7 +1161,7 @@ void init_numa_balancing(unsigned long clone_flags, struct task_struct *p)
 
 	/* New address space, reset the preferred nid */
 	if (!(clone_flags & CLONE_VM)) {
-		p->numa_preferred_nid = -1;
+		p->numa_preferred_nid = NUMA_NO_NODE;
 		return;
 	}
 
@@ -1181,13 +1181,13 @@ void init_numa_balancing(unsigned long clone_flags, struct task_struct *p)
 
 static void account_numa_enqueue(struct rq *rq, struct task_struct *p)
 {
-	rq->nr_numa_running += (p->numa_preferred_nid != -1);
+	rq->nr_numa_running += (p->numa_preferred_nid != NUMA_NO_NODE);
 	rq->nr_preferred_running += (p->numa_preferred_nid == task_node(p));
 }
 
 static void account_numa_dequeue(struct rq *rq, struct task_struct *p)
 {
-	rq->nr_numa_running -= (p->numa_preferred_nid != -1);
+	rq->nr_numa_running -= (p->numa_preferred_nid != NUMA_NO_NODE);
 	rq->nr_preferred_running -= (p->numa_preferred_nid == task_node(p));
 }
 
@@ -1401,7 +1401,7 @@ bool should_numa_migrate_memory(struct task_struct *p, struct page * page,
 	 * two full passes of the "multi-stage node selection" test that is
 	 * executed below.
 	 */
-	if ((p->numa_preferred_nid == -1 || p->numa_scan_seq <= 4) &&
+	if ((p->numa_preferred_nid == NUMA_NO_NODE || p->numa_scan_seq <= 4) &&
 	    (cpupid_pid_unset(last_cpupid) || cpupid_match_pid(p, last_cpupid)))
 		return true;
 
@@ -1849,7 +1849,7 @@ static void numa_migrate_preferred(struct task_struct *p)
 	unsigned long interval = HZ;
 
 	/* This task has no NUMA fault statistics yet */
-	if (unlikely(p->numa_preferred_nid == -1 || !p->numa_faults))
+	if (unlikely(p->numa_preferred_nid == NUMA_NO_NODE || !p->numa_faults))
 		return;
 
 	/* Periodically retry migrating the task to the preferred node */
@@ -2096,7 +2096,7 @@ static int preferred_group_nid(struct task_struct *p, int nid)
 
 static void task_numa_placement(struct task_struct *p)
 {
-	int seq, nid, max_nid = -1;
+	int seq, nid, max_nid = NUMA_NO_NODE;
 	unsigned long max_faults = 0;
 	unsigned long fault_types[2] = { 0, 0 };
 	unsigned long total_faults;
@@ -2639,7 +2639,8 @@ static void update_scan_period(struct task_struct *p, int new_cpu)
 		 * the preferred node.
 		 */
 		if (dst_nid == p->numa_preferred_nid ||
-		    (p->numa_preferred_nid != -1 && src_nid != p->numa_preferred_nid))
+		    (p->numa_preferred_nid != NUMA_NO_NODE &&
+			src_nid != p->numa_preferred_nid))
 			return;
 	}
 
diff --git a/lib/cpumask.c b/lib/cpumask.c
index 8d666ab..a089c3f 100644
--- a/lib/cpumask.c
+++ b/lib/cpumask.c
@@ -206,7 +206,7 @@ unsigned int cpumask_local_spread(unsigned int i, int node)
 	/* Wrap: we always want a cpu. */
 	i %= num_online_cpus();
 
-	if (node == -1) {
+	if (node == NUMA_NO_NODE) {
 		for_each_cpu(cpu, cpu_online_mask)
 			if (i-- == 0)
 				return cpu;
diff --git a/mm/huge_memory.c b/mm/huge_memory.c
index 55478ab..5ccf89e 100644
--- a/mm/huge_memory.c
+++ b/mm/huge_memory.c
@@ -1480,7 +1480,7 @@ vm_fault_t do_huge_pmd_numa_page(struct vm_fault *vmf, pmd_t pmd)
 	struct anon_vma *anon_vma = NULL;
 	struct page *page;
 	unsigned long haddr = vmf->address & HPAGE_PMD_MASK;
-	int page_nid = -1, this_nid = numa_node_id();
+	int page_nid = NUMA_NO_NODE, this_nid = numa_node_id();
 	int target_nid, last_cpupid = -1;
 	bool page_locked;
 	bool migrated = false;
@@ -1526,7 +1526,7 @@ vm_fault_t do_huge_pmd_numa_page(struct vm_fault *vmf, pmd_t pmd)
 	 */
 	page_locked = trylock_page(page);
 	target_nid = mpol_misplaced(page, vma, haddr);
-	if (target_nid == -1) {
+	if (target_nid == NUMA_NO_NODE) {
 		/* If the page was locked, there are no parallel migrations */
 		if (page_locked)
 			goto clear_pmdnuma;
@@ -1534,7 +1534,7 @@ vm_fault_t do_huge_pmd_numa_page(struct vm_fault *vmf, pmd_t pmd)
 
 	/* Migration could have started since the pmd_trans_migrating check */
 	if (!page_locked) {
-		page_nid = -1;
+		page_nid = NUMA_NO_NODE;
 		if (!get_page_unless_zero(page))
 			goto out_unlock;
 		spin_unlock(vmf->ptl);
@@ -1556,14 +1556,14 @@ vm_fault_t do_huge_pmd_numa_page(struct vm_fault *vmf, pmd_t pmd)
 	if (unlikely(!pmd_same(pmd, *vmf->pmd))) {
 		unlock_page(page);
 		put_page(page);
-		page_nid = -1;
+		page_nid = NUMA_NO_NODE;
 		goto out_unlock;
 	}
 
 	/* Bail if we fail to protect against THP splits for any reason */
 	if (unlikely(!anon_vma)) {
 		put_page(page);
-		page_nid = -1;
+		page_nid = NUMA_NO_NODE;
 		goto clear_pmdnuma;
 	}
 
@@ -1625,7 +1625,7 @@ vm_fault_t do_huge_pmd_numa_page(struct vm_fault *vmf, pmd_t pmd)
 	if (anon_vma)
 		page_unlock_anon_vma_read(anon_vma);
 
-	if (page_nid != -1)
+	if (page_nid != NUMA_NO_NODE)
 		task_numa_fault(last_cpupid, page_nid, HPAGE_PMD_NR,
 				flags);
 
diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index c007fb5..b769db7 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -887,7 +887,7 @@ static struct page *dequeue_huge_page_nodemask(struct hstate *h, gfp_t gfp_mask,
 	struct zonelist *zonelist;
 	struct zone *zone;
 	struct zoneref *z;
-	int node = -1;
+	int node = NUMA_NO_NODE;
 
 	zonelist = node_zonelist(nid, gfp_mask);
 
diff --git a/mm/ksm.c b/mm/ksm.c
index 5b0894b..d5f8834 100644
--- a/mm/ksm.c
+++ b/mm/ksm.c
@@ -597,7 +597,7 @@ static struct stable_node *alloc_stable_node_chain(struct stable_node *dup,
 		chain->chain_prune_time = jiffies;
 		chain->rmap_hlist_len = STABLE_NODE_CHAIN;
 #if defined (CONFIG_DEBUG_VM) && defined(CONFIG_NUMA)
-		chain->nid = -1; /* debug */
+		chain->nid = NUMA_NO_NODE; /* debug */
 #endif
 		ksm_stable_node_chains++;
 
diff --git a/mm/memory.c b/mm/memory.c
index 4ad2d29..c0e0348 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -3564,7 +3564,7 @@ static vm_fault_t do_numa_page(struct vm_fault *vmf)
 {
 	struct vm_area_struct *vma = vmf->vma;
 	struct page *page = NULL;
-	int page_nid = -1;
+	int page_nid = NUMA_NO_NODE;
 	int last_cpupid;
 	int target_nid;
 	bool migrated = false;
@@ -3631,7 +3631,7 @@ static vm_fault_t do_numa_page(struct vm_fault *vmf)
 	target_nid = numa_migrate_prep(page, vma, vmf->address, page_nid,
 			&flags);
 	pte_unmap_unlock(vmf->pte, vmf->ptl);
-	if (target_nid == -1) {
+	if (target_nid == NUMA_NO_NODE) {
 		put_page(page);
 		goto out;
 	}
@@ -3645,7 +3645,7 @@ static vm_fault_t do_numa_page(struct vm_fault *vmf)
 		flags |= TNF_MIGRATE_FAIL;
 
 out:
-	if (page_nid != -1)
+	if (page_nid != NUMA_NO_NODE)
 		task_numa_fault(last_cpupid, page_nid, 1, flags);
 	return 0;
 }
diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
index 2b2b3cc..70e02f8 100644
--- a/mm/memory_hotplug.c
+++ b/mm/memory_hotplug.c
@@ -688,9 +688,9 @@ static void node_states_check_changes_online(unsigned long nr_pages,
 {
 	int nid = zone_to_nid(zone);
 
-	arg->status_change_nid = -1;
-	arg->status_change_nid_normal = -1;
-	arg->status_change_nid_high = -1;
+	arg->status_change_nid = NUMA_NO_NODE;
+	arg->status_change_nid_normal = NUMA_NO_NODE;
+	arg->status_change_nid_high = NUMA_NO_NODE;
 
 	if (!node_state(nid, N_MEMORY))
 		arg->status_change_nid = nid;
@@ -1484,9 +1484,9 @@ static void node_states_check_changes_offline(unsigned long nr_pages,
 	unsigned long present_pages = 0;
 	enum zone_type zt;
 
-	arg->status_change_nid = -1;
-	arg->status_change_nid_normal = -1;
-	arg->status_change_nid_high = -1;
+	arg->status_change_nid = NUMA_NO_NODE;
+	arg->status_change_nid_normal = NUMA_NO_NODE;
+	arg->status_change_nid_high = NUMA_NO_NODE;
 
 	/*
 	 * Check whether node_states[N_NORMAL_MEMORY] will be changed.
diff --git a/mm/mempolicy.c b/mm/mempolicy.c
index 5837a06..e4f8248 100644
--- a/mm/mempolicy.c
+++ b/mm/mempolicy.c
@@ -2278,7 +2278,7 @@ int mpol_misplaced(struct page *page, struct vm_area_struct *vma, unsigned long
 	unsigned long pgoff;
 	int thiscpu = raw_smp_processor_id();
 	int thisnid = cpu_to_node(thiscpu);
-	int polnid = -1;
+	int polnid = NUMA_NO_NODE;
 	int ret = -1;
 
 	pol = get_vma_policy(vma, addr);
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index a919ba5..9d38d9c 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -5846,7 +5846,7 @@ int __meminit __early_pfn_to_nid(unsigned long pfn,
 		return state->last_nid;
 
 	nid = memblock_search_pfn_nid(pfn, &start_pfn, &end_pfn);
-	if (nid != -1) {
+	if (nid != NUMA_NO_NODE) {
 		state->last_start = start_pfn;
 		state->last_end = end_pfn;
 		state->last_nid = nid;
@@ -6607,7 +6607,7 @@ unsigned long __init node_map_pfn_alignment(void)
 {
 	unsigned long accl_mask = 0, last_end = 0;
 	unsigned long start, end, mask;
-	int last_nid = -1;
+	int last_nid = NUMA_NO_NODE;
 	int i, nid;
 
 	for_each_mem_pfn_range(i, MAX_NUMNODES, &start, &end, &nid) {
diff --git a/mm/page_ext.c b/mm/page_ext.c
index ae44f7a..dfb0206 100644
--- a/mm/page_ext.c
+++ b/mm/page_ext.c
@@ -300,7 +300,7 @@ static int __meminit online_page_ext(unsigned long start_pfn,
 	start = SECTION_ALIGN_DOWN(start_pfn);
 	end = SECTION_ALIGN_UP(start_pfn + nr_pages);
 
-	if (nid == -1) {
+	if (nid == NUMA_NO_NODE) {
 		/*
 		 * In this case, "nid" already exists and contains valid memory.
 		 * "start_pfn" passed to us is a pfn which is an arg for
diff --git a/net/core/pktgen.c b/net/core/pktgen.c
index 6ac9198..af3a746 100644
--- a/net/core/pktgen.c
+++ b/net/core/pktgen.c
@@ -3625,7 +3625,7 @@ static int pktgen_add_device(struct pktgen_thread *t, const char *ifname)
 	pkt_dev->svlan_cfi = 0;
 	pkt_dev->svlan_id = 0xffff;
 	pkt_dev->burst = 1;
-	pkt_dev->node = -1;
+	pkt_dev->node = NUMA_NO_NODE;
 
 	err = pktgen_setup_dev(t->net, pkt_dev, ifname);
 	if (err)
diff --git a/net/qrtr/qrtr.c b/net/qrtr/qrtr.c
index 86e1e37..0c56ae2 100644
--- a/net/qrtr/qrtr.c
+++ b/net/qrtr/qrtr.c
@@ -101,7 +101,7 @@ static inline struct qrtr_sock *qrtr_sk(struct sock *sk)
 	return container_of(sk, struct qrtr_sock, sk);
 }
 
-static unsigned int qrtr_local_nid = -1;
+static unsigned int qrtr_local_nid = NUMA_NO_NODE;
 
 /* for node ids */
 static RADIX_TREE(qrtr_nodes, GFP_KERNEL);
diff --git a/tools/perf/bench/numa.c b/tools/perf/bench/numa.c
index 4419551..e0ad5f1 100644
--- a/tools/perf/bench/numa.c
+++ b/tools/perf/bench/numa.c
@@ -298,7 +298,7 @@ static cpu_set_t bind_to_node(int target_node)
 
 	CPU_ZERO(&mask);
 
-	if (target_node == -1) {
+	if (target_node == NUMA_NO_NODE) {
 		for (cpu = 0; cpu < g->p.nr_cpus; cpu++)
 			CPU_SET(cpu, &mask);
 	} else {
@@ -339,7 +339,7 @@ static void bind_to_memnode(int node)
 	unsigned long nodemask;
 	int ret;
 
-	if (node == -1)
+	if (node == NUMA_NO_NODE)
 		return;
 
 	BUG_ON(g->p.nr_nodes > (int)sizeof(nodemask)*8);
@@ -1363,7 +1363,7 @@ static void init_thread_data(void)
 		int cpu;
 
 		/* Allow all nodes by default: */
-		td->bind_node = -1;
+		td->bind_node = NUMA_NO_NODE;
 
 		/* Allow all CPUs by default: */
 		CPU_ZERO(&td->bind_cpumask);

^ permalink raw reply related

* dmaengine: ti: omap-dma: Configure LCH_TYPE for OMAP1
From: Aaro Koskinen @ 2018-11-23  1:23 UTC (permalink / raw)
  To: Russell King - ARM Linux
  Cc: Peter Ujfalusi, vkoul, dan.j.williams, dmaengine, linux-kernel,
	tony, linux-omap

Hi,

On Fri, Nov 23, 2018 at 12:25:49AM +0000, Russell King - ARM Linux wrote:
> The patch was more for Peter to take a peek at

OK, I'll hack with other platforms meanwhile.

A.

^ permalink raw reply

* dmaengine: ti: omap-dma: Configure LCH_TYPE for OMAP1
From: Russell King - ARM Linux @ 2018-11-23  0:25 UTC (permalink / raw)
  To: Aaro Koskinen
  Cc: Peter Ujfalusi, vkoul, dan.j.williams, dmaengine, linux-kernel,
	tony, linux-omap

On Fri, Nov 23, 2018 at 12:24:26AM +0200, Aaro Koskinen wrote:
> Hi,
> 
> On Thu, Nov 22, 2018 at 03:12:36PM +0000, Russell King - ARM Linux wrote:
> > On Thu, Nov 22, 2018 at 10:29:48AM +0000, Russell King - ARM Linux wrote:
> > > On Tue, Nov 20, 2018 at 11:04:06PM +0200, Aaro Koskinen wrote:
> > > > I had switched to PIO mode in 2015 since the WARNs about legacy DMA
> > > > API were too annoying and flooding the console. And now that I tried
> > > > using DMA again with g_ether, it doesn't work anymore. The device get's
> > > > recognized on host side, but no traffic goes through. Switching back to
> > > > PIO makes it to work again.
> > > 
> > > A solution to that would be to do what the warning message says, and
> > > update the driver to the DMAengine API.
> 
> Fully agreed, but I was busy debugging other more serious issues, and
> just wanted to get a reliable ssh or USB serial access to the device
> without any extra noise, so switching to PIO using a module parameter
> is probably what most users do in such situations.
> 
> > Here's a partial conversion (not even build tested) - it only supports
> > OUT transfers with dmaengine at the moment.
> 
> Thanks, I'll take a closer look and try to do some testing hopefully
> during the weekend.

The patch was more for Peter to take a peek at - there's definitely
some bits missing in the dmaengine driver (like the write to the
LCH_CTRL register) that would need to be fixed somehow.

However, it's worth noting that there is exactly one user of
omap_set_dma_channel_mode(), which is omap-udc, which means any DMA
channel made use of by omap-udc will have the LCH_CTRL register
modified to LCH_P, and it will remain that way even if someone else
subsequently makes use of the same channel.  That's rather suspicious
to me... maybe we can just initialise all LCH_CTRL registers to LCH_P
in the dmaengine driver in that case!  If not, then there's a bug
right there.

^ permalink raw reply

* dmaengine: ti: omap-dma: Configure LCH_TYPE for OMAP1
From: Aaro Koskinen @ 2018-11-22 22:24 UTC (permalink / raw)
  To: Russell King - ARM Linux
  Cc: Peter Ujfalusi, vkoul, dan.j.williams, dmaengine, linux-kernel,
	tony, linux-omap

Hi,

On Thu, Nov 22, 2018 at 03:12:36PM +0000, Russell King - ARM Linux wrote:
> On Thu, Nov 22, 2018 at 10:29:48AM +0000, Russell King - ARM Linux wrote:
> > On Tue, Nov 20, 2018 at 11:04:06PM +0200, Aaro Koskinen wrote:
> > > I had switched to PIO mode in 2015 since the WARNs about legacy DMA
> > > API were too annoying and flooding the console. And now that I tried
> > > using DMA again with g_ether, it doesn't work anymore. The device get's
> > > recognized on host side, but no traffic goes through. Switching back to
> > > PIO makes it to work again.
> > 
> > A solution to that would be to do what the warning message says, and
> > update the driver to the DMAengine API.

Fully agreed, but I was busy debugging other more serious issues, and
just wanted to get a reliable ssh or USB serial access to the device
without any extra noise, so switching to PIO using a module parameter
is probably what most users do in such situations.

> Here's a partial conversion (not even build tested) - it only supports
> OUT transfers with dmaengine at the moment.

Thanks, I'll take a closer look and try to do some testing hopefully
during the weekend.

A.

^ permalink raw reply

* dmaengine: ti: omap-dma: Configure LCH_TYPE for OMAP1
From: Aaro Koskinen @ 2018-11-22 22:01 UTC (permalink / raw)
  To: Peter Ujfalusi
  Cc: vkoul, dan.j.williams, dmaengine, linux-kernel, tony, linux-omap,
	rmk+kernel, Felipe Balbi

Hi,

On Thu, Nov 22, 2018 at 10:31:31AM +0200, Peter Ujfalusi wrote:
> On 20/11/2018 23.04, Aaro Koskinen wrote:
> > On Tue, Nov 20, 2018 at 09:28:37AM +0200, Peter Ujfalusi wrote:
> >> On 19/11/2018 20.46, Aaro Koskinen wrote:
> >>> On Mon, Nov 19, 2018 at 12:40:40PM +0200, Peter Ujfalusi wrote:
> >>>> When the channel is configured for slave operation the LCH_TYPE needs to be
> >>>> set to LCh-P. For memcpy channels the LCH_TYPE must be set to LCh-2D.
> >>>>
> >>>> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
> >>>
> >>> I don't have the documentation, but based on what omap_udc driver (still
> >>> using the legacy OMAP DMA API) does this seems to be correct.
> >>
> >> They are hard to fine, true. From the omap1710 TRM:
> >>
> >> Logical channel types (LCh types) supported are:
> >> - LCh-2D for nonsynchronized transfers (memory transfers, 1D and 2D)
> >> - LCh-P for synchronized transfers (mostly peripheral transfers)
> >> - LCh-PD similar to LCh-P but runs on a dedicated physical channel
> >> - LCh-G for graphical transfers/operations
> >> - LCh-D for display transfers
> > 
> > (I found a public document "OMAP5912 Multimedia Processor Direct
> > Memory Access (DMA) Support Reference Guide", documenting these; easy
> > to confuse with "OMAP5910 Dual-Core Processor System DMA Controller
> > Reference Guide".)
> > 
> >> Looking at other part it looks like hat LCH-2D channel mode can happily
> >> service a peripheral. LCH-P supports the same features as LCH-2D, but it
> >> lacks support for Single/Double-indexed addressing mode on the
> >> peripheral port side.
> >>
> >> So, this patch might not be needed at all. Can you test the omap_udc
> >> with s/OMAP_DMA_LCH_P/OMAP_DMA_LCH_2D
> >>
> >> If USB works, then we can just drop this patch.
> > 
> > Unfortunately omap_udc does not seem to work at all anymore with DMA on
> > my 770 setup. :-(
> > 
> > I had switched to PIO mode in 2015 since the WARNs about legacy DMA
> > API were too annoying and flooding the console. And now that I tried
> > using DMA again with g_ether, it doesn't work anymore. The device get's
> > recognized on host side, but no traffic goes through. Switching back to
> > PIO makes it to work again.
> 
> After some tinkering I got omap_udc working with DMA (not DMAengine,
> yet) on 770 - using nfsroot. Configuring the channels to OMAP_DMA_LCH_2D
> works as expected.

Would be interesting to know how you got it working with DMA. Which
gadget driver were you using?

I bisected my issue, and got:

commit 387f869d2579e379ee343f5493dcd360be60f5c6 (refs/bisect/bad)
Author: Felipe Balbi <felipe.balbi@linux.intel.com>
Date:   Wed Mar 22 13:25:18 2017 +0200

    usb: gadget: u_ether: conditionally align transfer size

With that reverted, the DMA works OK (and I can also now confirm that
OMAP_DMA_LCH_2D works). I haven't yet checked if we actually need that
quirk in OMAP UDC, or if this is related to RMK's findings of potential
bugs in the driver. Anyway, there is clearly yet another regression.

A.

^ permalink raw reply

* [4/5] dma: tegra: reduce channel name field size
From: Jon Hunter @ 2018-11-22 15:24 UTC (permalink / raw)
  To: Ben Dooks, Jon Hunter
  Cc: dan.j.williams, vkoul, dmaengine, ldewangan, linux-tegra

On 22/11/2018 15:14, Ben Dooks wrote:
> On 22/11/2018 09:26, Jon Hunter wrote:
>> On Wed, Nov 21, 2018 at 4:15 PM Ben Dooks <ben.dooks@codethink.co.uk>
>> wrote:
>>>
>>> The name field is used for "apbdma.%d" which is rarely going to be
>>> more than 10 bytes, so reduce the size from 30 to 12. This is only
>>> being used by the interrupt registration, so is not critical to the
>>> operation of the driver either.
>>
>> Do you mean 'never' above instead of 'rarely'? The max number of
>> channels is never more than 2 digits and so the change is fine with
>> me. So for the change ...
> 
> I've never seen a system with more than 64 channels in it, but it didn't
> seem worth trying to get it below 12 anyway.

I see. If you look at the 'nr_channels' definition for the various Tegra
devices that use this driver, there is nothing more than 32. So for this
driver the max is 32. Newer Tegra devices are not using this anymore and
so in the case of this specific driver we can say 'never' (although I
understand now you meant rarely in the general case).

Cheers Jon

^ permalink raw reply

* [5/5] dma: tegra: add tracepoints to driver
From: Ben Dooks @ 2018-11-22 15:15 UTC (permalink / raw)
  To: Jon Hunter, dan.j.williams, vkoul, dmaengine
  Cc: ldewangan, linux-tegra, Ingo Molnar, Steven Rostedt

On 22/11/2018 08:55, Jon Hunter wrote:
> 
> On 21/11/2018 16:13, Ben Dooks wrote:
>> Add some trace-points to the driver to allow for debuging via the
>> trace pipe.
>>
>> Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
>> ---
>> Fixes since v1:
>> - take copy of dmachan name instead of pointer to device
>>
>> Fixes since v2:
>> - finish off Steven's review comments
>> - fix saving the dma channel name
>> - use txstate as argument to trace_tegra_dma_tx_status()
>> - fix type of dma_cookie to dma_cookie_t
>>
>> Cc: Ingo Molnar <mingo@redhat.com> (maintainer:TRACING)
>> Cc: Steven Rostedt <rostedt@goodmis.org> (maintainer:TRACING)
>> ---
>>   drivers/dma/tegra20-apb-dma.c        |  7 ++++
>>   include/trace/events/tegra_apb_dma.h | 61 ++++++++++++++++++++++++++++
>>   2 files changed, 68 insertions(+)
>>   create mode 100644 include/trace/events/tegra_apb_dma.h
>>
>> diff --git a/drivers/dma/tegra20-apb-dma.c b/drivers/dma/tegra20-apb-dma.c
>> index c60c3f7cad5f..659b49bf6fdb 100644
>> --- a/drivers/dma/tegra20-apb-dma.c
>> +++ b/drivers/dma/tegra20-apb-dma.c
>> @@ -38,6 +38,9 @@
>>   
>>   #include "dmaengine.h"
>>   
>> +#define CREATE_TRACE_POINTS
>> +#include <trace/events/tegra_apb_dma.h>
>> +
>>   #define TEGRA_APBDMA_GENERAL			0x0
>>   #define TEGRA_APBDMA_GENERAL_ENABLE		BIT(31)
>>   
>> @@ -672,6 +675,8 @@ static void tegra_dma_tasklet(unsigned long data)
>>   		dmaengine_desc_get_callback(&dma_desc->txd, &cb);
>>   		cb_count = dma_desc->cb_count;
>>   		dma_desc->cb_count = 0;
>> +		trace_tegra_dma_complete_cb(&tdc->dma_chan, cb_count,
>> +					    cb.callback);
>>   		spin_unlock_irqrestore(&tdc->lock, flags);
>>   		while (cb_count--)
>>   			dmaengine_desc_callback_invoke(&cb, NULL);
>> @@ -688,6 +693,7 @@ static irqreturn_t tegra_dma_isr(int irq, void *dev_id)
>>   
>>   	spin_lock_irqsave(&tdc->lock, flags);
>>   
>> +	trace_tegra_dma_isr(&tdc->dma_chan, irq);
>>   	status = tdc_read(tdc, TEGRA_APBDMA_CHAN_STATUS);
>>   	if (status & TEGRA_APBDMA_STATUS_ISE_EOC) {
>>   		tdc_write(tdc, TEGRA_APBDMA_CHAN_STATUS, status);
>> @@ -846,6 +852,7 @@ static enum dma_status tegra_dma_tx_status(struct dma_chan *dc,
>>   		dma_set_residue(txstate, residual);
>>   	}
>>   
>> +	trace_tegra_dma_tx_status(&tdc->dma_chan, cookie, txstate);
>>   	spin_unlock_irqrestore(&tdc->lock, flags);
>>   	return ret;
>>   }
> 
> My only comment here is that apart from the ISR trace, the others could
> be added to the dmaengine driver core and then these could be used for
> other DMAs as well.

Hmm, I will go have a look at how difficult that would be.

^ permalink raw reply

* [4/5] dma: tegra: reduce channel name field size
From: Ben Dooks @ 2018-11-22 15:14 UTC (permalink / raw)
  To: Jon Hunter
  Cc: dan.j.williams, vkoul, dmaengine, ldewangan, linux-tegra,
	Jon Hunter

On 22/11/2018 09:26, Jon Hunter wrote:
> On Wed, Nov 21, 2018 at 4:15 PM Ben Dooks <ben.dooks@codethink.co.uk> wrote:
>>
>> The name field is used for "apbdma.%d" which is rarely going to be
>> more than 10 bytes, so reduce the size from 30 to 12. This is only
>> being used by the interrupt registration, so is not critical to the
>> operation of the driver either.
> 
> Do you mean 'never' above instead of 'rarely'? The max number of
> channels is never more than 2 digits and so the change is fine with
> me. So for the change ...

I've never seen a system with more than 64 channels in it, but it didn't
seem worth trying to get it below 12 anyway.

^ permalink raw reply

* dmaengine: ti: omap-dma: Configure LCH_TYPE for OMAP1
From: Russell King - ARM Linux @ 2018-11-22 15:12 UTC (permalink / raw)
  To: Aaro Koskinen
  Cc: Peter Ujfalusi, vkoul, dan.j.williams, dmaengine, linux-kernel,
	tony, linux-omap

On Thu, Nov 22, 2018 at 10:29:48AM +0000, Russell King - ARM Linux wrote:
> On Tue, Nov 20, 2018 at 11:04:06PM +0200, Aaro Koskinen wrote:
> > I had switched to PIO mode in 2015 since the WARNs about legacy DMA
> > API were too annoying and flooding the console. And now that I tried
> > using DMA again with g_ether, it doesn't work anymore. The device get's
> > recognized on host side, but no traffic goes through. Switching back to
> > PIO makes it to work again.
> 
> A solution to that would be to do what the warning message says, and
> update the driver to the DMAengine API.

Here's a partial conversion (not even build tested) - it only supports
OUT transfers with dmaengine at the moment.

There's at least one thing that doesn't make sense - the driver
apparently can transfer more than req->length bytes, surely if it does
that, it's a serious problem - shouldn't it be noisy about that?

Also we can't deal with the omap_set_dma_dest_burst_mode() setting -
DMAengine always uses a 64 byte burst, but udc wants a smaller burst
setting.  Does this matter?

I've kept the old code for reference (and the driver will fall back if
we can't get a dmaengine channel.)  I haven't been through and checked
that we result in the channel setup largely the same either.

There will be one major difference - UDC uses element sync, where
an element is 16bits, ep->ep.maxpacket/2 in a frame, and "packets"
frames.  DMAengine is using frame sync, with a 16bit element, one
element in a frame, and packets*ep->ep.maxpacket/2 frames.  This
should be functionally equivalent but I'd like confirmation of that.

I'm also not sure about this:

        if (cpu_is_omap15xx())
                end++;

in dma_dest_len() - is that missing from the omap-dma driver?  It looks
like a work-around for some problem on OMAP15xx, but I can't make sense
about why it's in the UDC driver rather than the legacy DMA driver.

I'm also confused by:

        end |= start & (0xffff << 16);

also in dma_dest_len() - omap_get_dma_dst_pos() returns in the high 16
bits the full address:

        if (dma_omap1())
                offset |= (p->dma_read(CDSA, lch) & 0xFFFF0000);

so if the address crosses a 64k physical address boundary, surely orring
in the start address is wrong?  The more I look at dma_dest_len(), the
more I wonder whether that and dma_src_len() are anywhere near correct,
and whether that is a source of breakage for Aaro.

As I've already said, I've no way to test this on hardware.

Please review and let me know whether I missed anything on the OUT
handling path.

Fixing the IN path is going to be a bit more head-scratching.

 drivers/usb/gadget/udc/omap_udc.c | 154 +++++++++++++++++++++++++++++---------
 drivers/usb/gadget/udc/omap_udc.h |   2 +
 2 files changed, 120 insertions(+), 36 deletions(-)

diff --git a/drivers/usb/gadget/udc/omap_udc.c b/drivers/usb/gadget/udc/omap_udc.c
index 3a16431da321..a37e1d2f0f3e 100644
--- a/drivers/usb/gadget/udc/omap_udc.c
+++ b/drivers/usb/gadget/udc/omap_udc.c
@@ -204,6 +204,7 @@ static int omap_ep_enable(struct usb_ep *_ep,
 	ep->dma_channel = 0;
 	ep->has_dma = 0;
 	ep->lch = -1;
+	ep->dma = NULL;
 	use_ep(ep, UDC_EP_SEL);
 	omap_writew(udc->clr_halt, UDC_CTRL);
 	ep->ackwait = 0;
@@ -576,21 +577,49 @@ static void finish_in_dma(struct omap_ep *ep, struct omap_req *req, int status)
 static void next_out_dma(struct omap_ep *ep, struct omap_req *req)
 {
 	unsigned packets = req->req.length - req->req.actual;
-	int dma_trigger = 0;
+	struct dma_async_tx_descriptor *tx;
+	struct dma_chan *dma = ep->dma;
+	dma_cookie_t cookie;
 	u16 w;
 
-	/* set up this DMA transfer, enable the fifo, start */
-	packets /= ep->ep.maxpacket;
-	packets = min(packets, (unsigned)UDC_RXN_TC + 1);
+	packets = min_t(unsigned, packets / ep->ep.maxpacket, UDC_RXN_TC + 1);
 	req->dma_bytes = packets * ep->ep.maxpacket;
-	omap_set_dma_transfer_params(ep->lch, OMAP_DMA_DATA_TYPE_S16,
-			ep->ep.maxpacket >> 1, packets,
-			OMAP_DMA_SYNC_ELEMENT,
-			dma_trigger, 0);
-	omap_set_dma_dest_params(ep->lch, OMAP_DMA_PORT_EMIFF,
-		OMAP_DMA_AMODE_POST_INC, req->req.dma + req->req.actual,
-		0, 0);
-	ep->dma_counter = omap_get_dma_dst_pos(ep->lch);
+
+	if (dma) {
+		struct dma_slave_config cfg = {
+			.direction = DMA_DEV_TO_MEM,
+			.src_addr_width = DMA_SLAVE_BUSWIDTH_2_BYTE,
+			/*
+			 * DMAengine uses frame sync mode, setting maxburst=1
+			 * is equivalent to element sync mode.
+			 */
+			.src_maxburst = 1,
+			.src_addr = UDC_DATA_DMA,
+		};
+
+		if (WARN_ON(dmaengine_slave_config(dma, &cfg)))
+			return;
+
+		tx = dmaengine_prep_slave_single(dma,
+						 req->req.dma + req->req.actual,
+						 req->dma_bytes,
+						 DMA_DEV_TO_MEM, 0);
+		if (WARN_ON(!tx))
+			return;
+	} else {
+		int dma_trigger = 0;
+
+		/* set up this DMA transfer, enable the fifo, start */
+		/* dt = S16, cen = ep->ep.maxpacket / 2, cfn = packets */
+		omap_set_dma_transfer_params(ep->lch, OMAP_DMA_DATA_TYPE_S16,
+				ep->ep.maxpacket >> 1, packets,
+				OMAP_DMA_SYNC_ELEMENT,
+				dma_trigger, 0);
+		omap_set_dma_dest_params(ep->lch, OMAP_DMA_PORT_EMIFF,
+			OMAP_DMA_AMODE_POST_INC, req->req.dma + req->req.actual,
+			0, 0);
+		ep->dma_counter = omap_get_dma_dst_pos(ep->lch);
+	}
 
 	omap_writew(UDC_RXN_STOP | (packets - 1), UDC_RXDMA(ep->dma_channel));
 	w = omap_readw(UDC_DMA_IRQ_EN);
@@ -599,7 +628,15 @@ static void next_out_dma(struct omap_ep *ep, struct omap_req *req)
 	omap_writew(ep->bEndpointAddress & 0xf, UDC_EP_NUM);
 	omap_writew(UDC_SET_FIFO_EN, UDC_CTRL);
 
-	omap_start_dma(ep->lch);
+	if (dma) {
+		cookie = dmaengine_submit(tx);
+		if (WARN_ON(dma_submit_error(cookie)))
+			return;
+		ep->dma_cookie = cookie;
+		dma_async_issue_pending(dma);
+	} else {
+		omap_start_dma(ep->lch);
+	}
 }
 
 static void
@@ -607,21 +644,39 @@ finish_out_dma(struct omap_ep *ep, struct omap_req *req, int status, int one)
 {
 	u16	count, w;
 
-	if (status == 0)
-		ep->dma_counter = (u16) (req->req.dma + req->req.actual);
-	count = dma_dest_len(ep, req->req.dma + req->req.actual);
+	if (ep->dma) {
+		struct dma_tx_state state;
+
+		dmaengine_tx_status(ep->dma, ep->dma_cookie, &state);
+
+		count = req->dma_bytes - state.residual;
+	} else {
+		if (status == 0)
+			ep->dma_counter = (u16) (req->req.dma + req->req.actual);
+		count = dma_dest_len(ep, req->req.dma + req->req.actual);
+	}
+
 	count += req->req.actual;
 	if (one)
 		count--;
+
+	/*
+	 * FIXME: Surely if count > req->req.length, something has gone
+	 * seriously wrong and we've scribbled over memory we should not...
+	 * so surely we should be a WARN_ON() at the very least?
+	 */
 	if (count <= req->req.length)
 		req->req.actual = count;
 
-	if (count != req->dma_bytes || status)
-		omap_stop_dma(ep->lch);
-
+	if (count != req->dma_bytes || status) {
+		if (ep->dma)
+			dmaengine_terminate_async(ep->dma);
+		else
+			omap_stop_dma(ep->lch);
 	/* if this wasn't short, request may need another transfer */
-	else if (req->req.actual < req->req.length)
+	} else if (req->req.actual < req->req.length) {
 		return;
+	}
 
 	/* rx completion */
 	w = omap_readw(UDC_DMA_IRQ_EN);
@@ -709,6 +764,7 @@ static void dma_channel_claim(struct omap_ep *ep, unsigned channel)
 
 	ep->dma_channel = 0;
 	ep->lch = -1;
+	ep->dma = NULL;
 	if (channel == 0 || channel > 3) {
 		if ((reg & 0x0f00) == 0)
 			channel = 3;
@@ -742,26 +798,44 @@ static void dma_channel_claim(struct omap_ep *ep, unsigned channel)
 				0, 0);
 		}
 	} else {
+		struct dma_chan *dma;
+
 		dma_channel = OMAP_DMA_USB_W2FC_RX0 - 1 + channel;
-		status = omap_request_dma(dma_channel,
-			ep->ep.name, dma_error, ep, &ep->lch);
-		if (status == 0) {
+
+		dma = __dma_request_channel(&mask, omap_dma_filter_fn,
+					    (void *)dma_channel);
+		if (dma) {
+			ep->dma = dma;
 			omap_writew(reg, UDC_RXDMA_CFG);
-			/* TIPB */
-			omap_set_dma_src_params(ep->lch,
-				OMAP_DMA_PORT_TIPB,
-				OMAP_DMA_AMODE_CONSTANT,
-				UDC_DATA_DMA,
-				0, 0);
-			/* EMIFF or SDRC */
-			omap_set_dma_dest_burst_mode(ep->lch,
-						OMAP_DMA_DATA_BURST_4);
-			omap_set_dma_dest_data_pack(ep->lch, 1);
+		} else {
+			status = omap_request_dma(dma_channel,
+				ep->ep.name, dma_error, ep, &ep->lch);
+			if (status == 0) {
+				omap_writew(reg, UDC_RXDMA_CFG);
+				/* TIPB */
+				omap_set_dma_src_params(ep->lch,
+					OMAP_DMA_PORT_TIPB,
+					OMAP_DMA_AMODE_CONSTANT,
+					UDC_DATA_DMA,
+					0, 0);
+				/* EMIFF or SDRC */
+				/*
+				 * not ok - CSDP_DST_BURST_64 selected, but this selects
+				 * CSDP_DST_BURST_16 on omap2+ and CSDP_DST_BURST_32 on
+				 * omap1.
+				 */
+				omap_set_dma_dest_burst_mode(ep->lch,
+							OMAP_DMA_DATA_BURST_4);
+				/* ok - CSDP_DST_PACKED set for dmaengine */
+				omap_set_dma_dest_data_pack(ep->lch, 1);
+			}
 		}
 	}
-	if (status)
+	if (d->dma) {
+		ep->has_dma = 1;
+	} else if (status) {
 		ep->dma_channel = 0;
-	else {
+	} else {
 		ep->has_dma = 1;
 		omap_disable_dma_irq(ep->lch, OMAP_DMA_BLOCK_IRQ);
 
@@ -777,6 +851,10 @@ static void dma_channel_claim(struct omap_ep *ep, unsigned channel)
 	if (status)
 		DBG("%s no dma channel: %d%s\n", ep->ep.name, status,
 			restart ? " (restart)" : "");
+	else if (d->dma)
+		DBG("%s claimed %cxdma%d dmaengine %s%s\n", ep->ep.name,
+			is_in ? 't' : 'r', ep->dma_channel - 1,
+			dma_chan_name(d->dma), restart ? " (restart)" : "");
 	else
 		DBG("%s claimed %cxdma%d lch %d%s\n", ep->ep.name,
 			is_in ? 't' : 'r',
@@ -850,9 +928,13 @@ static void dma_channel_release(struct omap_ep *ep)
 		if (req)
 			finish_out_dma(ep, req, -ECONNRESET, 0);
 	}
-	omap_free_dma(ep->lch);
+	if (ep->dma)
+		dma_release_channel(ep->dma);
+	else
+		omap_free_dma(ep->lch);
 	ep->dma_channel = 0;
 	ep->lch = -1;
+	ep->dma = NULL;
 	/* has_dma still set, till endpoint is fully quiesced */
 }
 
diff --git a/drivers/usb/gadget/udc/omap_udc.h b/drivers/usb/gadget/udc/omap_udc.h
index 00f9e608e755..68857ae8d763 100644
--- a/drivers/usb/gadget/udc/omap_udc.h
+++ b/drivers/usb/gadget/udc/omap_udc.h
@@ -153,6 +153,8 @@ struct omap_ep {
 	u8				dma_channel;
 	u16				dma_counter;
 	int				lch;
+	struct dma_chan			*dma;
+	dma_cookie_t			dma_cookie;
 	struct omap_udc			*udc;
 	struct timer_list		timer;
 };

^ permalink raw reply related

* dmaengine: ti: omap-dma: Configure LCH_TYPE for OMAP1
From: Russell King - ARM Linux @ 2018-11-22 10:29 UTC (permalink / raw)
  To: Aaro Koskinen
  Cc: Peter Ujfalusi, vkoul, dan.j.williams, dmaengine, linux-kernel,
	tony, linux-omap

On Tue, Nov 20, 2018 at 11:04:06PM +0200, Aaro Koskinen wrote:
> I had switched to PIO mode in 2015 since the WARNs about legacy DMA
> API were too annoying and flooding the console. And now that I tried
> using DMA again with g_ether, it doesn't work anymore. The device get's
> recognized on host side, but no traffic goes through. Switching back to
> PIO makes it to work again.

A solution to that would be to do what the warning message says, and
update the driver to the DMAengine API.

The reason it didn't get updated when the DMAengine conversion happened
is because I don't have hardware for it, so had no way to test, and no
one seemed to know that anyone was using it.  Eventually, the WARN_ON()
was added to try and root out any users and generate interest in
updating the drivers.  Obviously that didn't happen, because people
just worked around the warning rather than saying anything.

I'm afraid we're long past the time that I'd be willing to update the
omap_udc driver now as I've dropped most of my knowledge on that as
it's been four years, and Peter has been looking after OMAP DMAengine
issues since.

Sorry.

^ permalink raw reply

* [4/5] dma: tegra: reduce channel name field size
From: Jon Hunter @ 2018-11-22  9:26 UTC (permalink / raw)
  To: ben.dooks
  Cc: dan.j.williams, vkoul, dmaengine, ldewangan, linux-tegra,
	Jon Hunter

On Wed, Nov 21, 2018 at 4:15 PM Ben Dooks <ben.dooks@codethink.co.uk> wrote:
>
> The name field is used for "apbdma.%d" which is rarely going to be
> more than 10 bytes, so reduce the size from 30 to 12. This is only
> being used by the interrupt registration, so is not critical to the
> operation of the driver either.

Do you mean 'never' above instead of 'rarely'? The max number of
channels is never more than 2 digits and so the change is fine with
me. So for the change ...

Acked-by: Jon Hunter <jonathanh@nvidia.com>

Cheers
Jon

^ permalink raw reply

* [2/5] dma: tegra: make byte counters unsigned int
From: Jon Hunter @ 2018-11-22  9:19 UTC (permalink / raw)
  To: ben.dooks
  Cc: dan.j.williams, vkoul, dmaengine, ldewangan, linux-tegra,
	Jon Hunter

On Wed, Nov 21, 2018 at 4:15 PM Ben Dooks <ben.dooks@codethink.co.uk> wrote:
>
> The buffer byte request length and counter are declared as signed integers
> but the values should never be below zero, so make these unsigned integers
> instead.
>
> Reviewed-by: Dmitry Osipenko <digetx@gmail.com>
> Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
> ---
>  drivers/dma/tegra20-apb-dma.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/dma/tegra20-apb-dma.c b/drivers/dma/tegra20-apb-dma.c
> index 8219ab88a507..adfd918baedc 100644
> --- a/drivers/dma/tegra20-apb-dma.c
> +++ b/drivers/dma/tegra20-apb-dma.c
> @@ -155,7 +155,7 @@ struct tegra_dma_channel_regs {
>   */
>  struct tegra_dma_sg_req {
>         struct tegra_dma_channel_regs   ch_regs;
> -       int                             req_len;
> +       unsigned int                    req_len;
>         bool                            configured;
>         bool                            last_sg;
>         struct list_head                node;
> @@ -169,8 +169,8 @@ struct tegra_dma_sg_req {
>   */
>  struct tegra_dma_desc {
>         struct dma_async_tx_descriptor  txd;
> -       int                             bytes_requested;
> -       int                             bytes_transferred;
> +       unsigned int                    bytes_requested;
> +       unsigned int                    bytes_transferred;
>         enum dma_status                 dma_status;
>         struct list_head                node;
>         struct list_head                tx_list;
> --
> 2.19.1
>

For some reason, I did not receive this one at work, so replying from
personal email ...

Anyway, for consistency with the DMA engine APIs, I wonder if we
should just use size_t here instead of unsigned int (although it
should be the same).

Cheers
Jon

^ permalink raw reply

* [5/5] dma: tegra: add tracepoints to driver
From: Jon Hunter @ 2018-11-22  8:55 UTC (permalink / raw)
  To: Ben Dooks, dan.j.williams, vkoul, dmaengine
  Cc: ldewangan, linux-tegra, Ingo Molnar, Steven Rostedt

On 21/11/2018 16:13, Ben Dooks wrote:
> Add some trace-points to the driver to allow for debuging via the
> trace pipe.
> 
> Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
> ---
> Fixes since v1:
> - take copy of dmachan name instead of pointer to device
> 
> Fixes since v2:
> - finish off Steven's review comments
> - fix saving the dma channel name
> - use txstate as argument to trace_tegra_dma_tx_status()
> - fix type of dma_cookie to dma_cookie_t
> 
> Cc: Ingo Molnar <mingo@redhat.com> (maintainer:TRACING)
> Cc: Steven Rostedt <rostedt@goodmis.org> (maintainer:TRACING)
> ---
>  drivers/dma/tegra20-apb-dma.c        |  7 ++++
>  include/trace/events/tegra_apb_dma.h | 61 ++++++++++++++++++++++++++++
>  2 files changed, 68 insertions(+)
>  create mode 100644 include/trace/events/tegra_apb_dma.h
> 
> diff --git a/drivers/dma/tegra20-apb-dma.c b/drivers/dma/tegra20-apb-dma.c
> index c60c3f7cad5f..659b49bf6fdb 100644
> --- a/drivers/dma/tegra20-apb-dma.c
> +++ b/drivers/dma/tegra20-apb-dma.c
> @@ -38,6 +38,9 @@
>  
>  #include "dmaengine.h"
>  
> +#define CREATE_TRACE_POINTS
> +#include <trace/events/tegra_apb_dma.h>
> +
>  #define TEGRA_APBDMA_GENERAL			0x0
>  #define TEGRA_APBDMA_GENERAL_ENABLE		BIT(31)
>  
> @@ -672,6 +675,8 @@ static void tegra_dma_tasklet(unsigned long data)
>  		dmaengine_desc_get_callback(&dma_desc->txd, &cb);
>  		cb_count = dma_desc->cb_count;
>  		dma_desc->cb_count = 0;
> +		trace_tegra_dma_complete_cb(&tdc->dma_chan, cb_count,
> +					    cb.callback);
>  		spin_unlock_irqrestore(&tdc->lock, flags);
>  		while (cb_count--)
>  			dmaengine_desc_callback_invoke(&cb, NULL);
> @@ -688,6 +693,7 @@ static irqreturn_t tegra_dma_isr(int irq, void *dev_id)
>  
>  	spin_lock_irqsave(&tdc->lock, flags);
>  
> +	trace_tegra_dma_isr(&tdc->dma_chan, irq);
>  	status = tdc_read(tdc, TEGRA_APBDMA_CHAN_STATUS);
>  	if (status & TEGRA_APBDMA_STATUS_ISE_EOC) {
>  		tdc_write(tdc, TEGRA_APBDMA_CHAN_STATUS, status);
> @@ -846,6 +852,7 @@ static enum dma_status tegra_dma_tx_status(struct dma_chan *dc,
>  		dma_set_residue(txstate, residual);
>  	}
>  
> +	trace_tegra_dma_tx_status(&tdc->dma_chan, cookie, txstate);
>  	spin_unlock_irqrestore(&tdc->lock, flags);
>  	return ret;
>  }

My only comment here is that apart from the ISR trace, the others could
be added to the dmaengine driver core and then these could be used for
other DMAs as well.

Cheers
Jon

^ permalink raw reply

* dmaengine: ti: omap-dma: Configure LCH_TYPE for OMAP1
From: Peter Ujfalusi @ 2018-11-22  8:31 UTC (permalink / raw)
  To: Aaro Koskinen
  Cc: vkoul, dan.j.williams, dmaengine, linux-kernel, tony, linux-omap,
	rmk+kernel

On 20/11/2018 23.04, Aaro Koskinen wrote:
> Hi,
> 
> On Tue, Nov 20, 2018 at 09:28:37AM +0200, Peter Ujfalusi wrote:
>> On 19/11/2018 20.46, Aaro Koskinen wrote:
>>> On Mon, Nov 19, 2018 at 12:40:40PM +0200, Peter Ujfalusi wrote:
>>>> When the channel is configured for slave operation the LCH_TYPE needs to be
>>>> set to LCh-P. For memcpy channels the LCH_TYPE must be set to LCh-2D.
>>>>
>>>> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
>>>
>>> I don't have the documentation, but based on what omap_udc driver (still
>>> using the legacy OMAP DMA API) does this seems to be correct.
>>
>> They are hard to fine, true. From the omap1710 TRM:
>>
>> Logical channel types (LCh types) supported are:
>> - LCh-2D for nonsynchronized transfers (memory transfers, 1D and 2D)
>> - LCh-P for synchronized transfers (mostly peripheral transfers)
>> - LCh-PD similar to LCh-P but runs on a dedicated physical channel
>> - LCh-G for graphical transfers/operations
>> - LCh-D for display transfers
> 
> (I found a public document "OMAP5912 Multimedia Processor Direct
> Memory Access (DMA) Support Reference Guide", documenting these; easy
> to confuse with "OMAP5910 Dual-Core Processor System DMA Controller
> Reference Guide".)
> 
>> Looking at other part it looks like hat LCH-2D channel mode can happily
>> service a peripheral. LCH-P supports the same features as LCH-2D, but it
>> lacks support for Single/Double-indexed addressing mode on the
>> peripheral port side.
>>
>> So, this patch might not be needed at all. Can you test the omap_udc
>> with s/OMAP_DMA_LCH_P/OMAP_DMA_LCH_2D
>>
>> If USB works, then we can just drop this patch.
> 
> Unfortunately omap_udc does not seem to work at all anymore with DMA on
> my 770 setup. :-(
> 
> I had switched to PIO mode in 2015 since the WARNs about legacy DMA
> API were too annoying and flooding the console. And now that I tried
> using DMA again with g_ether, it doesn't work anymore. The device get's
> recognized on host side, but no traffic goes through. Switching back to
> PIO makes it to work again.

After some tinkering I got omap_udc working with DMA (not DMAengine,
yet) on 770 - using nfsroot. Configuring the channels to OMAP_DMA_LCH_2D
works as expected.

This patch can be dropped.

- Péter

Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

^ permalink raw reply

* [5/5] dma: tegra: add tracepoints to driver
From: Ben Dooks @ 2018-11-22  8:29 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: dan.j.williams, vkoul, dmaengine, ldewangan, linux-tegra,
	jonathanh, Ingo Molnar

On 21/11/2018 17:21, Steven Rostedt wrote:
> On Wed, 21 Nov 2018 16:13:23 +0000
> Ben Dooks <ben.dooks@codethink.co.uk> wrote:
> 
>> Add some trace-points to the driver to allow for debuging via the
>> trace pipe.
>>
>> Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
> 
>  From a tracing perspective I see nothing wrong with this patch.
> 
> Reviewed-by: Steven Rostedt (VMware) <rostedt@goodmis.org>

Thank you for the feedback.

^ permalink raw reply

* [3/5] dma: tegra: fix incorrect case of DMA
From: Jon Hunter @ 2018-11-22  8:28 UTC (permalink / raw)
  To: Ben Dooks, dan.j.williams, vkoul, dmaengine; +Cc: ldewangan, linux-tegra

On 21/11/2018 16:13, Ben Dooks wrote:
> The use of Dma is annoying, since it is an acronym so should be all
> upper case. Fix this throughout the driver.
> 
> Reviewed-by: Dmitry Osipenko <digetx@gmail.com>
> Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
> ---
> Fixes since v1:
> - Missing cases fixed as pointed out by Dmitry Osipenko
> ---
>  drivers/dma/tegra20-apb-dma.c | 20 ++++++++++----------
>  1 file changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/dma/tegra20-apb-dma.c b/drivers/dma/tegra20-apb-dma.c
> index adfd918baedc..ec8938a2ecab 100644
> --- a/drivers/dma/tegra20-apb-dma.c
> +++ b/drivers/dma/tegra20-apb-dma.c
> @@ -146,7 +146,7 @@ struct tegra_dma_channel_regs {
>  };
>  
>  /*
> - * tegra_dma_sg_req: Dma request details to configure hardware. This
> + * tegra_dma_sg_req: DMA request details to configure hardware. This
>   * contains the details for one transfer to configure DMA hw.
>   * The client's request for data transfer can be broken into multiple
>   * sub-transfer as per requester details and hw support.
> @@ -574,7 +574,7 @@ static bool handle_continuous_head_request(struct tegra_dma_channel *tdc,
>  	struct tegra_dma_sg_req *hsgreq = NULL;
>  
>  	if (list_empty(&tdc->pending_sg_req)) {
> -		dev_err(tdc2dev(tdc), "Dma is running without req\n");
> +		dev_err(tdc2dev(tdc), "DMA is running without req\n");
>  		tegra_dma_stop(tdc);
>  		return false;
>  	}
> @@ -587,7 +587,7 @@ static bool handle_continuous_head_request(struct tegra_dma_channel *tdc,
>  	hsgreq = list_first_entry(&tdc->pending_sg_req, typeof(*hsgreq), node);
>  	if (!hsgreq->configured) {
>  		tegra_dma_stop(tdc);
> -		dev_err(tdc2dev(tdc), "Error in dma transfer, aborting dma\n");
> +		dev_err(tdc2dev(tdc), "Error in DMA transfer, aborting DMA\n");
>  		tegra_dma_abort_all(tdc);
>  		return false;
>  	}
> @@ -922,7 +922,7 @@ static int get_transfer_param(struct tegra_dma_channel *tdc,
>  		return 0;
>  
>  	default:
> -		dev_err(tdc2dev(tdc), "Dma direction is not supported\n");
> +		dev_err(tdc2dev(tdc), "DMA direction is not supported\n");
>  		return -EINVAL;
>  	}
>  	return -EINVAL;
> @@ -955,7 +955,7 @@ static struct dma_async_tx_descriptor *tegra_dma_prep_slave_sg(
>  	enum dma_slave_buswidth slave_bw;
>  
>  	if (!tdc->config_init) {
> -		dev_err(tdc2dev(tdc), "dma channel is not configured\n");
> +		dev_err(tdc2dev(tdc), "DMA channel is not configured\n");
>  		return NULL;
>  	}
>  	if (sg_len < 1) {
> @@ -988,7 +988,7 @@ static struct dma_async_tx_descriptor *tegra_dma_prep_slave_sg(
>  
>  	dma_desc = tegra_dma_desc_get(tdc);
>  	if (!dma_desc) {
> -		dev_err(tdc2dev(tdc), "Dma descriptors not available\n");
> +		dev_err(tdc2dev(tdc), "DMA descriptors not available\n");
>  		return NULL;
>  	}
>  	INIT_LIST_HEAD(&dma_desc->tx_list);
> @@ -1008,14 +1008,14 @@ static struct dma_async_tx_descriptor *tegra_dma_prep_slave_sg(
>  		if ((len & 3) || (mem & 3) ||
>  				(len > tdc->tdma->chip_data->max_dma_count)) {
>  			dev_err(tdc2dev(tdc),
> -				"Dma length/memory address is not supported\n");
> +				"DMA length/memory address is not supported\n");
>  			tegra_dma_desc_put(tdc, dma_desc);
>  			return NULL;
>  		}
>  
>  		sg_req = tegra_dma_sg_req_get(tdc);
>  		if (!sg_req) {
> -			dev_err(tdc2dev(tdc), "Dma sg-req not available\n");
> +			dev_err(tdc2dev(tdc), "DMA sg-req not available\n");
>  			tegra_dma_desc_put(tdc, dma_desc);
>  			return NULL;
>  		}
> @@ -1090,7 +1090,7 @@ static struct dma_async_tx_descriptor *tegra_dma_prep_dma_cyclic(
>  	 * terminating the DMA.
>  	 */
>  	if (tdc->busy) {
> -		dev_err(tdc2dev(tdc), "Request not allowed when dma running\n");
> +		dev_err(tdc2dev(tdc), "Request not allowed when DMA running\n");
>  		return NULL;
>  	}
>  
> @@ -1147,7 +1147,7 @@ static struct dma_async_tx_descriptor *tegra_dma_prep_dma_cyclic(
>  	while (remain_len) {
>  		sg_req = tegra_dma_sg_req_get(tdc);
>  		if (!sg_req) {
> -			dev_err(tdc2dev(tdc), "Dma sg-req not available\n");
> +			dev_err(tdc2dev(tdc), "DMA sg-req not available\n");
>  			tegra_dma_desc_put(tdc, dma_desc);
>  			return NULL;
>  		}
> 

Acked-by: Jon Hunter <jonathanh@nvidia.com>

Cheers
Jon

^ permalink raw reply

* [1/5] dma: tegra: avoid overflow of byte tracking
From: Jon Hunter @ 2018-11-22  8:27 UTC (permalink / raw)
  To: Ben Dooks, dan.j.williams, vkoul, dmaengine; +Cc: ldewangan, linux-tegra

On 21/11/2018 16:13, Ben Dooks wrote:
> The dma_desc->bytes_transferred counter tracks the number of bytes
> moved by the DMA channel. This is then used to calculate the information
> passed back in the in the tegra_dma_tx_status callback, which is usually
> fine.
> 
> When the DMA channel is configured as continous, then the bytes_transferred
> counter will increase over time and eventually overflow to become negative
> so the residue count will become invalid and the ALSA sound-dma code will
> report invalid hardware pointer values to the application. This results in
> some users becoming confused about the playout position and putting audio
> data in the wrong place.
> 
> To fix this issue, always ensure the bytes_transferred field is modulo the
> size of the request. We only do this for the case of the cyclic transfer
> done ISR as anyone attempting to move 2GiB of DMA data in one transfer
> is unlikely.
> 
> Note, we don't fix the issue that we should /never/ transfer a negative
> number of bytes so we could make those fields unsigned.
> 
> Reviewed-by: Dmitry Osipenko <digetx@gmail.com>
> Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
> ---
>  drivers/dma/tegra20-apb-dma.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/dma/tegra20-apb-dma.c b/drivers/dma/tegra20-apb-dma.c
> index 9a558e30c461..8219ab88a507 100644
> --- a/drivers/dma/tegra20-apb-dma.c
> +++ b/drivers/dma/tegra20-apb-dma.c
> @@ -636,7 +636,10 @@ static void handle_cont_sngl_cycle_dma_done(struct tegra_dma_channel *tdc,
>  
>  	sgreq = list_first_entry(&tdc->pending_sg_req, typeof(*sgreq), node);
>  	dma_desc = sgreq->dma_desc;
> -	dma_desc->bytes_transferred += sgreq->req_len;
> +	/* if we dma for long enough the transfer count will wrap */
> +	dma_desc->bytes_transferred =
> +		(dma_desc->bytes_transferred + sgreq->req_len) %
> +		dma_desc->bytes_requested;
>  
>  	/* Callback need to be call */
>  	if (!dma_desc->cb_count)

Acked-by: Jon Hunter <jonathanh@nvidia.com>

Thanks!
Jon

^ permalink raw reply


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