All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 1/3] net: irda: pxaficp_ir: use sched_clock() for time management
@ 2015-09-12 11:45 Robert Jarzmik
  2015-09-12 11:45 ` [PATCH v2 2/3] net: irda: pxaficp_ir: convert to readl and writel Robert Jarzmik
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Robert Jarzmik @ 2015-09-12 11:45 UTC (permalink / raw)
  To: Samuel Ortiz, Petr Cvek
  Cc: netdev, linux-kernel, Arnd Bergmann, Robert Jarzmik

Instead of using directly the OS timer through direct register access,
use the standard sched_clock(), which will end up in OSCR reading
anyway.

This is a first step for direct access register removal and machine
specific code removal from this driver.

Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr>
---
 drivers/net/irda/pxaficp_ir.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/drivers/net/irda/pxaficp_ir.c b/drivers/net/irda/pxaficp_ir.c
index 100454662e4b..b1794998c68e 100644
--- a/drivers/net/irda/pxaficp_ir.c
+++ b/drivers/net/irda/pxaficp_ir.c
@@ -29,7 +29,6 @@
 
 #include <mach/dma.h>
 #include <linux/platform_data/irda-pxaficp.h>
-#include <mach/regs-ost.h>
 #include <mach/regs-uart.h>
 
 #define FICP		__REG(0x40800000)  /* Start of FICP area */
@@ -102,7 +101,7 @@
 struct pxa_irda {
 	int			speed;
 	int			newspeed;
-	unsigned long		last_oscr;
+	unsigned long long	last_clk;
 
 	unsigned char		*dma_rx_buff;
 	unsigned char		*dma_tx_buff;
@@ -292,7 +291,7 @@ static irqreturn_t pxa_irda_sir_irq(int irq, void *dev_id)
 			}
 			lsr = STLSR;
 		}
-		si->last_oscr = readl_relaxed(OSCR);
+		si->last_clk = sched_clock();
 		break;
 
 	case 0x04: /* Received Data Available */
@@ -303,7 +302,7 @@ static irqreturn_t pxa_irda_sir_irq(int irq, void *dev_id)
 		    dev->stats.rx_bytes++;
 	            async_unwrap_char(dev, &dev->stats, &si->rx_buff, STRBR);
 	  	} while (STLSR & LSR_DR);
-		si->last_oscr = readl_relaxed(OSCR);
+		si->last_clk = sched_clock();
 	  	break;
 
 	case 0x02: /* Transmit FIFO Data Request */
@@ -319,7 +318,7 @@ static irqreturn_t pxa_irda_sir_irq(int irq, void *dev_id)
                         /* We need to ensure that the transmitter has finished. */
 			while ((STLSR & LSR_TEMT) == 0)
 				cpu_relax();
-			si->last_oscr = readl_relaxed(OSCR);
+			si->last_clk = sched_clock();
 
 			/*
 		 	* Ok, we've finished transmitting.  Now enable
@@ -373,7 +372,7 @@ static void pxa_irda_fir_dma_tx_irq(int channel, void *data)
 
 	while (ICSR1 & ICSR1_TBY)
 		cpu_relax();
-	si->last_oscr = readl_relaxed(OSCR);
+	si->last_clk = sched_clock();
 
 	/*
 	 * HACK: It looks like the TBY bit is dropped too soon.
@@ -473,8 +472,8 @@ static irqreturn_t pxa_irda_fir_irq(int irq, void *dev_id)
 
 	/* stop RX DMA */
 	DCSR(si->rxdma) &= ~DCSR_RUN;
-	si->last_oscr = readl_relaxed(OSCR);
 	icsr0 = ICSR0;
+	si->last_clk = sched_clock();
 
 	if (icsr0 & (ICSR0_FRE | ICSR0_RAB)) {
 		if (icsr0 & ICSR0_FRE) {
@@ -549,7 +548,7 @@ static int pxa_irda_hard_xmit(struct sk_buff *skb, struct net_device *dev)
 		skb_copy_from_linear_data(skb, si->dma_tx_buff, skb->len);
 
 		if (mtt)
-			while ((unsigned)(readl_relaxed(OSCR) - si->last_oscr)/4 < mtt)
+			while ((sched_clock() - si->last_clk) / 4 < mtt)
 				cpu_relax();
 
 		/* stop RX DMA,  disable FICP */
-- 
2.1.4


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v2 2/3] net: irda: pxaficp_ir: convert to readl and writel
  2015-09-12 11:45 [PATCH v2 1/3] net: irda: pxaficp_ir: use sched_clock() for time management Robert Jarzmik
@ 2015-09-12 11:45 ` Robert Jarzmik
  2015-09-13  1:34   ` Petr Cvek
  2015-09-12 11:45 ` [PATCH v2 3/3] net: irda: pxaficp_ir: dmaengine conversion Robert Jarzmik
  2015-09-15 23:40 ` [PATCH v2 1/3] net: irda: pxaficp_ir: use sched_clock() for time management David Miller
  2 siblings, 1 reply; 10+ messages in thread
From: Robert Jarzmik @ 2015-09-12 11:45 UTC (permalink / raw)
  To: Samuel Ortiz, Petr Cvek
  Cc: netdev, linux-kernel, Arnd Bergmann, Robert Jarzmik

Convert the pxa IRDA driver to readl and writel primitives, and remove
another set of direct registers access. This leaves only the DMA
registers access, which will be dealt with dmaengine conversion.

Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr>
---
Since v1: modified __REG macro to cope with STIER, ST* registers
---
 drivers/net/irda/pxaficp_ir.c | 210 +++++++++++++++++++++++++-----------------
 1 file changed, 126 insertions(+), 84 deletions(-)

diff --git a/drivers/net/irda/pxaficp_ir.c b/drivers/net/irda/pxaficp_ir.c
index b1794998c68e..4a2b3f71e4a8 100644
--- a/drivers/net/irda/pxaficp_ir.c
+++ b/drivers/net/irda/pxaficp_ir.c
@@ -29,15 +29,16 @@
 
 #include <mach/dma.h>
 #include <linux/platform_data/irda-pxaficp.h>
+#undef __REG
+#define __REG(x) ((x) & 0xffff)
 #include <mach/regs-uart.h>
 
-#define FICP		__REG(0x40800000)  /* Start of FICP area */
-#define ICCR0		__REG(0x40800000)  /* ICP Control Register 0 */
-#define ICCR1		__REG(0x40800004)  /* ICP Control Register 1 */
-#define ICCR2		__REG(0x40800008)  /* ICP Control Register 2 */
-#define ICDR		__REG(0x4080000c)  /* ICP Data Register */
-#define ICSR0		__REG(0x40800014)  /* ICP Status Register 0 */
-#define ICSR1		__REG(0x40800018)  /* ICP Status Register 1 */
+#define ICCR0		0x0000		/* ICP Control Register 0 */
+#define ICCR1		0x0004		/* ICP Control Register 1 */
+#define ICCR2		0x0008		/* ICP Control Register 2 */
+#define ICDR		0x000c		/* ICP Data Register */
+#define ICSR0		0x0014		/* ICP Status Register 0 */
+#define ICSR1		0x0018		/* ICP Status Register 1 */
 
 #define ICCR0_AME	(1 << 7)	/* Address match enable */
 #define ICCR0_TIE	(1 << 6)	/* Transmit FIFO interrupt enable */
@@ -55,9 +56,7 @@
 #define ICCR2_TRIG_16   (1 << 0)	/*	>= 16 bytes */
 #define ICCR2_TRIG_32   (2 << 0)	/*	>= 32 bytes */
 
-#ifdef CONFIG_PXA27x
 #define ICSR0_EOC	(1 << 6)	/* DMA End of Descriptor Chain */
-#endif
 #define ICSR0_FRE	(1 << 5)	/* Framing error */
 #define ICSR0_RFS	(1 << 4)	/* Receive FIFO service request */
 #define ICSR0_TFS	(1 << 3)	/* Transnit FIFO service request */
@@ -98,11 +97,50 @@
                 IrSR_RCVEIR_UART_MODE | \
                 IrSR_XMITIR_IR_MODE)
 
+/* macros for registers read/write */
+#define ficp_writel(irda, val, off)					\
+	do {								\
+		dev_vdbg(irda->dev,					\
+			 "%s():%d ficp_writel(0x%x, %s)\n",		\
+			 __func__, __LINE__, (val), #off);		\
+		writel_relaxed((val), (irda)->irda_base + (off));	\
+	} while (0)
+
+#define ficp_readl(irda, off)						\
+	({								\
+		unsigned int _v;					\
+		_v = readl_relaxed((irda)->irda_base + (off));		\
+		dev_vdbg(irda->dev,					\
+			 "%s():%d ficp_readl(%s): 0x%x\n",		\
+			 __func__, __LINE__, #off, _v);			\
+		_v;							\
+	})
+
+#define stuart_writel(irda, val, off)					\
+	do {								\
+		dev_vdbg(irda->dev,					\
+			 "%s():%d stuart_writel(0x%x, %s)\n",		\
+			 __func__, __LINE__, (val), #off);		\
+		writel_relaxed((val), (irda)->stuart_base + (off));	\
+	} while (0)
+
+#define stuart_readl(irda, off)						\
+	({								\
+		unsigned int _v;					\
+		_v = readl_relaxed((irda)->stuart_base + (off));	\
+		dev_vdbg(irda->dev,					\
+			 "%s():%d stuart_readl(%s): 0x%x\n",		\
+			 __func__, __LINE__, #off, _v);			\
+		_v;							\
+	})
+
 struct pxa_irda {
 	int			speed;
 	int			newspeed;
 	unsigned long long	last_clk;
 
+	void __iomem		*stuart_base;
+	void __iomem		*irda_base;
 	unsigned char		*dma_rx_buff;
 	unsigned char		*dma_tx_buff;
 	dma_addr_t		dma_rx_buff_phy;
@@ -153,7 +191,7 @@ static inline void pxa_irda_enable_sirclk(struct pxa_irda *si)
 inline static void pxa_irda_fir_dma_rx_start(struct pxa_irda *si)
 {
 	DCSR(si->rxdma)  = DCSR_NODESC;
-	DSADR(si->rxdma) = __PREG(ICDR);
+	DSADR(si->rxdma) = (unsigned long)si->irda_base + ICDR;
 	DTADR(si->rxdma) = si->dma_rx_buff_phy;
 	DCMD(si->rxdma) = DCMD_INCTRGADDR | DCMD_FLOWSRC |  DCMD_WIDTH1 | DCMD_BURST32 | IRDA_FRAME_SIZE_LIMIT;
 	DCSR(si->rxdma) |= DCSR_RUN;
@@ -163,7 +201,7 @@ inline static void pxa_irda_fir_dma_tx_start(struct pxa_irda *si)
 {
 	DCSR(si->txdma)  = DCSR_NODESC;
 	DSADR(si->txdma) = si->dma_tx_buff_phy;
-	DTADR(si->txdma) = __PREG(ICDR);
+	DTADR(si->txdma) = (unsigned long)si->irda_base + ICDR;
 	DCMD(si->txdma) = DCMD_INCSRCADDR | DCMD_FLOWTRG |  DCMD_ENDIRQEN | DCMD_WIDTH1 | DCMD_BURST32 | si->dma_tx_buff_len;
 	DCSR(si->txdma) |= DCSR_RUN;
 }
@@ -206,7 +244,7 @@ static int pxa_irda_set_speed(struct pxa_irda *si, int speed)
 			/* stop RX DMA */
 			DCSR(si->rxdma) &= ~DCSR_RUN;
 			/* disable FICP */
-			ICCR0 = 0;
+			ficp_writel(si, 0, ICCR0);
 			pxa_irda_disable_clk(si);
 
 			/* set board transceiver to SIR mode */
@@ -217,17 +255,19 @@ static int pxa_irda_set_speed(struct pxa_irda *si, int speed)
 		}
 
 		/* disable STUART first */
-		STIER = 0;
+		stuart_writel(si, 0, STIER);
 
 		/* access DLL & DLH */
-		STLCR |= LCR_DLAB;
-		STDLL = divisor & 0xff;
-		STDLH = divisor >> 8;
-		STLCR &= ~LCR_DLAB;
+		stuart_writel(si, stuart_readl(si, STLCR) | LCR_DLAB, STLCR);
+		stuart_writel(si, divisor & 0xff, STDLL);
+		stuart_writel(si, divisor >> 8, STDLH);
+		stuart_writel(si, stuart_readl(si, STLCR) & ~LCR_DLAB, STLCR);
 
 		si->speed = speed;
-		STISR = IrSR_IR_RECEIVE_ON | IrSR_XMODE_PULSE_1_6;
-		STIER = IER_UUE | IER_RLSE | IER_RAVIE | IER_RTIOE;
+		stuart_writel(si, IrSR_IR_RECEIVE_ON | IrSR_XMODE_PULSE_1_6,
+			      STISR);
+		stuart_writel(si, IER_UUE | IER_RLSE | IER_RAVIE | IER_RTIOE,
+			      STIER);
 
 		local_irq_restore(flags);
 		break;
@@ -236,12 +276,12 @@ static int pxa_irda_set_speed(struct pxa_irda *si, int speed)
 		local_irq_save(flags);
 
 		/* disable STUART */
-		STIER = 0;
-		STISR = 0;
+		stuart_writel(si, 0, STIER);
+		stuart_writel(si, 0, STISR);
 		pxa_irda_disable_clk(si);
 
 		/* disable FICP first */
-		ICCR0 = 0;
+		ficp_writel(si, 0, ICCR0);
 
 		/* set board transceiver to FIR mode */
 		pxa_irda_set_mode(si, IR_FIRMODE);
@@ -251,7 +291,7 @@ static int pxa_irda_set_speed(struct pxa_irda *si, int speed)
 
 		si->speed = speed;
 		pxa_irda_fir_dma_rx_start(si);
-		ICCR0 = ICCR0_ITR | ICCR0_RXE;
+		ficp_writel(si, ICCR0_ITR | ICCR0_RXE, ICCR0);
 
 		local_irq_restore(flags);
 		break;
@@ -270,13 +310,13 @@ static irqreturn_t pxa_irda_sir_irq(int irq, void *dev_id)
 	struct pxa_irda *si = netdev_priv(dev);
 	int iir, lsr, data;
 
-	iir = STIIR;
+	iir = stuart_readl(si, STIIR);
 
 	switch  (iir & 0x0F) {
 	case 0x06: /* Receiver Line Status */
-	  	lsr = STLSR;
+		lsr = stuart_readl(si, STLSR);
 		while (lsr & LSR_FIFOE) {
-			data = STRBR;
+			data = stuart_readl(si, STRBR);
 			if (lsr & (LSR_OE | LSR_PE | LSR_FE | LSR_BI)) {
 				printk(KERN_DEBUG "pxa_ir: sir receiving error\n");
 				dev->stats.rx_errors++;
@@ -289,7 +329,7 @@ static irqreturn_t pxa_irda_sir_irq(int irq, void *dev_id)
 				async_unwrap_char(dev, &dev->stats,
 						  &si->rx_buff, data);
 			}
-			lsr = STLSR;
+			lsr = stuart_readl(si, STLSR);
 		}
 		si->last_clk = sched_clock();
 		break;
@@ -300,14 +340,16 @@ static irqreturn_t pxa_irda_sir_irq(int irq, void *dev_id)
 	case 0x0C: /* Character Timeout Indication */
 	  	do  {
 		    dev->stats.rx_bytes++;
-	            async_unwrap_char(dev, &dev->stats, &si->rx_buff, STRBR);
-	  	} while (STLSR & LSR_DR);
+		    async_unwrap_char(dev, &dev->stats, &si->rx_buff,
+				      stuart_readl(si, STRBR));
+		} while (stuart_readl(si, STLSR) & LSR_DR);
 		si->last_clk = sched_clock();
 	  	break;
 
 	case 0x02: /* Transmit FIFO Data Request */
-	    	while ((si->tx_buff.len) && (STLSR & LSR_TDRQ)) {
-	    		STTHR = *si->tx_buff.data++;
+		while ((si->tx_buff.len) &&
+		       (stuart_readl(si, STLSR) & LSR_TDRQ)) {
+			stuart_writel(si, *si->tx_buff.data++, STTHR);
 			si->tx_buff.len -= 1;
 	    	}
 
@@ -316,7 +358,7 @@ static irqreturn_t pxa_irda_sir_irq(int irq, void *dev_id)
 			dev->stats.tx_bytes += si->tx_buff.data - si->tx_buff.head;
 
                         /* We need to ensure that the transmitter has finished. */
-			while ((STLSR & LSR_TEMT) == 0)
+			while ((stuart_readl(si, STLSR) & LSR_TEMT) == 0)
 				cpu_relax();
 			si->last_clk = sched_clock();
 
@@ -330,9 +372,11 @@ static irqreturn_t pxa_irda_sir_irq(int irq, void *dev_id)
 				si->newspeed = 0;
 			} else {
 				/* enable IR Receiver, disable IR Transmitter */
-				STISR = IrSR_IR_RECEIVE_ON | IrSR_XMODE_PULSE_1_6;
+				stuart_writel(si, IrSR_IR_RECEIVE_ON |
+					      IrSR_XMODE_PULSE_1_6, STISR);
 				/* enable STUART and receive interrupts */
-				STIER = IER_UUE | IER_RLSE | IER_RAVIE | IER_RTIOE;
+				stuart_writel(si, IER_UUE | IER_RLSE |
+					      IER_RAVIE | IER_RTIOE, STIER);
 			}
 			/* I'm hungry! */
 			netif_wake_queue(dev);
@@ -370,7 +414,7 @@ static void pxa_irda_fir_dma_tx_irq(int channel, void *data)
 		dev->stats.tx_errors++;
 	}
 
-	while (ICSR1 & ICSR1_TBY)
+	while (ficp_readl(si, ICSR1) & ICSR1_TBY)
 		cpu_relax();
 	si->last_clk = sched_clock();
 
@@ -386,11 +430,11 @@ static void pxa_irda_fir_dma_tx_irq(int channel, void *data)
 	} else {
 		int i = 64;
 
-		ICCR0 = 0;
+		ficp_writel(si, 0, ICCR0);
 		pxa_irda_fir_dma_rx_start(si);
-		while ((ICSR1 & ICSR1_RNE) && i--)
-			(void)ICDR;
-		ICCR0 = ICCR0_ITR | ICCR0_RXE;
+		while ((ficp_readl(si, ICSR1) & ICSR1_RNE) && i--)
+			ficp_readl(si, ICDR);
+		ficp_writel(si, ICCR0_ITR | ICCR0_RXE, ICCR0);
 
 		if (i < 0)
 			printk(KERN_ERR "pxa_ir: cannot clear Rx FIFO!\n");
@@ -408,9 +452,9 @@ static void pxa_irda_fir_irq_eif(struct pxa_irda *si, struct net_device *dev, in
 
 	do {
 		/* Read Status, and then Data. 	 */
-		stat = ICSR1;
+		stat = ficp_readl(si, ICSR1);
 		rmb();
-		data = ICDR;
+		data = ficp_readl(si, ICDR);
 
 		if (stat & (ICSR1_CRE | ICSR1_ROR)) {
 			dev->stats.rx_errors++;
@@ -428,7 +472,7 @@ static void pxa_irda_fir_irq_eif(struct pxa_irda *si, struct net_device *dev, in
 		/* If we hit the end of frame, there's no point in continuing. */
 		if (stat & ICSR1_EOF)
 			break;
-	} while (ICSR0 & ICSR0_EIF);
+	} while (ficp_readl(si, ICSR0) & ICSR0_EIF);
 
 	if (stat & ICSR1_EOF) {
 		/* end of frame. */
@@ -472,8 +516,8 @@ static irqreturn_t pxa_irda_fir_irq(int irq, void *dev_id)
 
 	/* stop RX DMA */
 	DCSR(si->rxdma) &= ~DCSR_RUN;
-	icsr0 = ICSR0;
 	si->last_clk = sched_clock();
+	icsr0 = ficp_readl(si, ICSR0);
 
 	if (icsr0 & (ICSR0_FRE | ICSR0_RAB)) {
 		if (icsr0 & ICSR0_FRE) {
@@ -483,7 +527,7 @@ static irqreturn_t pxa_irda_fir_irq(int irq, void *dev_id)
 			printk(KERN_DEBUG "pxa_ir: fir receive abort\n");
 			dev->stats.rx_errors++;
 		}
-		ICSR0 = icsr0 & (ICSR0_FRE | ICSR0_RAB);
+		ficp_writel(si, icsr0 & (ICSR0_FRE | ICSR0_RAB), ICSR0);
 	}
 
 	if (icsr0 & ICSR0_EIF) {
@@ -491,11 +535,11 @@ static irqreturn_t pxa_irda_fir_irq(int irq, void *dev_id)
 		pxa_irda_fir_irq_eif(si, dev, icsr0);
 	}
 
-	ICCR0 = 0;
+	ficp_writel(si, 0, ICCR0);
 	pxa_irda_fir_dma_rx_start(si);
-	while ((ICSR1 & ICSR1_RNE) && i--)
-		(void)ICDR;
-	ICCR0 = ICCR0_ITR | ICCR0_RXE;
+	while ((ficp_readl(si, ICSR1) & ICSR1_RNE) && i--)
+		ficp_readl(si, ICDR);
+	ficp_writel(si, ICCR0_ITR | ICCR0_RXE, ICCR0);
 
 	if (i < 0)
 		printk(KERN_ERR "pxa_ir: cannot clear Rx FIFO!\n");
@@ -536,11 +580,12 @@ static int pxa_irda_hard_xmit(struct sk_buff *skb, struct net_device *dev)
 		si->tx_buff.len  = async_wrap_skb(skb, si->tx_buff.data, si->tx_buff.truesize);
 
 		/* Disable STUART interrupts and switch to transmit mode. */
-		STIER = 0;
-		STISR = IrSR_IR_TRANSMIT_ON | IrSR_XMODE_PULSE_1_6;
+		stuart_writel(si, 0, STIER);
+		stuart_writel(si, IrSR_IR_TRANSMIT_ON | IrSR_XMODE_PULSE_1_6,
+			      STISR);
 
 		/* enable STUART and transmit interrupts */
-		STIER = IER_UUE | IER_TIE;
+		stuart_writel(si, IER_UUE | IER_TIE, STIER);
 	} else {
 		unsigned long mtt = irda_get_mtt(skb);
 
@@ -553,10 +598,10 @@ static int pxa_irda_hard_xmit(struct sk_buff *skb, struct net_device *dev)
 
 		/* stop RX DMA,  disable FICP */
 		DCSR(si->rxdma) &= ~DCSR_RUN;
-		ICCR0 = 0;
+		ficp_writel(si, 0, ICCR0);
 
 		pxa_irda_fir_dma_tx_start(si);
-		ICCR0 = ICCR0_ITR | ICCR0_TXE;
+		ficp_writel(si, ICCR0_ITR | ICCR0_TXE, ICCR0);
 	}
 
 	dev_kfree_skb(skb);
@@ -612,18 +657,18 @@ static int pxa_irda_ioctl(struct net_device *dev, struct ifreq *ifreq, int cmd)
 static void pxa_irda_startup(struct pxa_irda *si)
 {
 	/* Disable STUART interrupts */
-	STIER = 0;
+	stuart_writel(si, 0, STIER);
 	/* enable STUART interrupt to the processor */
-	STMCR = MCR_OUT2;
+	stuart_writel(si, MCR_OUT2, STMCR);
 	/* configure SIR frame format: StartBit - Data 7 ... Data 0 - Stop Bit */
-	STLCR = LCR_WLS0 | LCR_WLS1;
+	stuart_writel(si, LCR_WLS0 | LCR_WLS1, STLCR);
 	/* enable FIFO, we use FIFO to improve performance */
-	STFCR = FCR_TRFIFOE | FCR_ITL_32;
+	stuart_writel(si, FCR_TRFIFOE | FCR_ITL_32, STFCR);
 
 	/* disable FICP */
-	ICCR0 = 0;
+	ficp_writel(si, 0, ICCR0);
 	/* configure FICP ICCR2 */
-	ICCR2 = ICCR2_TXP | ICCR2_TRIG_32;
+	ficp_writel(si, ICCR2_TXP | ICCR2_TRIG_32, ICCR2);
 
 	/* configure DMAC */
 	DRCMR(17) = si->rxdma | DRCMR_MAPVLD;
@@ -643,15 +688,15 @@ static void pxa_irda_shutdown(struct pxa_irda *si)
 	local_irq_save(flags);
 
 	/* disable STUART and interrupt */
-	STIER = 0;
+	stuart_writel(si, 0, STIER);
 	/* disable STUART SIR mode */
-	STISR = 0;
+	stuart_writel(si, 0, STISR);
 
 	/* disable DMA */
 	DCSR(si->txdma) &= ~DCSR_RUN;
 	DCSR(si->rxdma) &= ~DCSR_RUN;
 	/* disable FICP */
-	ICCR0 = 0;
+	ficp_writel(si, 0, ICCR0);
 
 	/* disable the STUART or FICP clocks */
 	pxa_irda_disable_clk(si);
@@ -829,25 +874,33 @@ static const struct net_device_ops pxa_irda_netdev_ops = {
 static int pxa_irda_probe(struct platform_device *pdev)
 {
 	struct net_device *dev;
+	struct resource *res;
 	struct pxa_irda *si;
+	void __iomem *ficp, *stuart;
 	unsigned int baudrate_mask;
 	int err;
 
 	if (!pdev->dev.platform_data)
 		return -ENODEV;
 
-	err = request_mem_region(__PREG(STUART), 0x24, "IrDA") ? 0 : -EBUSY;
-	if (err)
-		goto err_mem_1;
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	ficp = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(ficp)) {
+		dev_err(&pdev->dev, "resource ficp not defined\n");
+		return PTR_ERR(ficp);
+	}
 
-	err = request_mem_region(__PREG(FICP), 0x1c, "IrDA") ? 0 : -EBUSY;
-	if (err)
-		goto err_mem_2;
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
+	stuart = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(stuart)) {
+		dev_err(&pdev->dev, "resource stuart not defined\n");
+		return PTR_ERR(stuart);
+	}
 
 	dev = alloc_irdadev(sizeof(struct pxa_irda));
 	if (!dev) {
 		err = -ENOMEM;
-		goto err_mem_3;
+		goto err_mem_1;
 	}
 
 	SET_NETDEV_DEV(dev, &pdev->dev);
@@ -855,11 +908,13 @@ static int pxa_irda_probe(struct platform_device *pdev)
 	si->dev = &pdev->dev;
 	si->pdata = pdev->dev.platform_data;
 
+	si->irda_base = ficp;
+	si->stuart_base = stuart;
 	si->uart_irq = platform_get_irq(pdev, 0);
 	si->icp_irq = platform_get_irq(pdev, 1);
 
-	si->sir_clk = clk_get(&pdev->dev, "UARTCLK");
-	si->fir_clk = clk_get(&pdev->dev, "FICPCLK");
+	si->sir_clk = devm_clk_get(&pdev->dev, "UARTCLK");
+	si->fir_clk = devm_clk_get(&pdev->dev, "FICPCLK");
 	if (IS_ERR(si->sir_clk) || IS_ERR(si->fir_clk)) {
 		err = PTR_ERR(IS_ERR(si->sir_clk) ? si->sir_clk : si->fir_clk);
 		goto err_mem_4;
@@ -924,15 +979,7 @@ err_startup:
 err_mem_5:
 		kfree(si->rx_buff.head);
 err_mem_4:
-		if (si->sir_clk && !IS_ERR(si->sir_clk))
-			clk_put(si->sir_clk);
-		if (si->fir_clk && !IS_ERR(si->fir_clk))
-			clk_put(si->fir_clk);
 		free_netdev(dev);
-err_mem_3:
-		release_mem_region(__PREG(FICP), 0x1c);
-err_mem_2:
-		release_mem_region(__PREG(STUART), 0x24);
 	}
 err_mem_1:
 	return err;
@@ -951,14 +998,9 @@ static int pxa_irda_remove(struct platform_device *_dev)
 			si->pdata->shutdown(si->dev);
 		kfree(si->tx_buff.head);
 		kfree(si->rx_buff.head);
-		clk_put(si->fir_clk);
-		clk_put(si->sir_clk);
 		free_netdev(dev);
 	}
 
-	release_mem_region(__PREG(STUART), 0x24);
-	release_mem_region(__PREG(FICP), 0x1c);
-
 	return 0;
 }
 
-- 
2.1.4


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v2 3/3] net: irda: pxaficp_ir: dmaengine conversion
  2015-09-12 11:45 [PATCH v2 1/3] net: irda: pxaficp_ir: use sched_clock() for time management Robert Jarzmik
  2015-09-12 11:45 ` [PATCH v2 2/3] net: irda: pxaficp_ir: convert to readl and writel Robert Jarzmik
@ 2015-09-12 11:45 ` Robert Jarzmik
  2015-09-15 23:40 ` [PATCH v2 1/3] net: irda: pxaficp_ir: use sched_clock() for time management David Miller
  2 siblings, 0 replies; 10+ messages in thread
From: Robert Jarzmik @ 2015-09-12 11:45 UTC (permalink / raw)
  To: Samuel Ortiz, Petr Cvek
  Cc: netdev, linux-kernel, Arnd Bergmann, Robert Jarzmik

Convert pxaficp_ir to dmaengine. As pxa architecture is shifting from
raw DMA registers access to pxa_dma dmaengine driver, convert this
driver to dmaengine.

Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr>
---
Since v1: removed mach/dma.h include, which is the goal
---
 drivers/net/irda/pxaficp_ir.c | 149 +++++++++++++++++++++++++++++-------------
 1 file changed, 102 insertions(+), 47 deletions(-)

diff --git a/drivers/net/irda/pxaficp_ir.c b/drivers/net/irda/pxaficp_ir.c
index 4a2b3f71e4a8..c0b80548a43d 100644
--- a/drivers/net/irda/pxaficp_ir.c
+++ b/drivers/net/irda/pxaficp_ir.c
@@ -19,6 +19,9 @@
 #include <linux/etherdevice.h>
 #include <linux/platform_device.h>
 #include <linux/clk.h>
+#include <linux/dmaengine.h>
+#include <linux/dma-mapping.h>
+#include <linux/dma/pxa-dma.h>
 #include <linux/gpio.h>
 #include <linux/slab.h>
 
@@ -27,7 +30,6 @@
 #include <net/irda/wrapper.h>
 #include <net/irda/irda_device.h>
 
-#include <mach/dma.h>
 #include <linux/platform_data/irda-pxaficp.h>
 #undef __REG
 #define __REG(x) ((x) & 0xffff)
@@ -146,8 +148,12 @@ struct pxa_irda {
 	dma_addr_t		dma_rx_buff_phy;
 	dma_addr_t		dma_tx_buff_phy;
 	unsigned int		dma_tx_buff_len;
-	int			txdma;
-	int			rxdma;
+	struct dma_chan		*txdma;
+	struct dma_chan		*rxdma;
+	dma_cookie_t		rx_cookie;
+	dma_cookie_t		tx_cookie;
+	int			drcmr_rx;
+	int			drcmr_tx;
 
 	int			uart_irq;
 	int			icp_irq;
@@ -165,6 +171,8 @@ struct pxa_irda {
 	struct clk		*cur_clk;
 };
 
+static int pxa_irda_set_speed(struct pxa_irda *si, int speed);
+
 static inline void pxa_irda_disable_clk(struct pxa_irda *si)
 {
 	if (si->cur_clk)
@@ -188,22 +196,41 @@ static inline void pxa_irda_enable_sirclk(struct pxa_irda *si)
 #define IS_FIR(si)		((si)->speed >= 4000000)
 #define IRDA_FRAME_SIZE_LIMIT	2047
 
+static void pxa_irda_fir_dma_rx_irq(void *data);
+static void pxa_irda_fir_dma_tx_irq(void *data);
+
 inline static void pxa_irda_fir_dma_rx_start(struct pxa_irda *si)
 {
-	DCSR(si->rxdma)  = DCSR_NODESC;
-	DSADR(si->rxdma) = (unsigned long)si->irda_base + ICDR;
-	DTADR(si->rxdma) = si->dma_rx_buff_phy;
-	DCMD(si->rxdma) = DCMD_INCTRGADDR | DCMD_FLOWSRC |  DCMD_WIDTH1 | DCMD_BURST32 | IRDA_FRAME_SIZE_LIMIT;
-	DCSR(si->rxdma) |= DCSR_RUN;
+	struct dma_async_tx_descriptor *tx;
+
+	tx = dmaengine_prep_slave_single(si->rxdma, si->dma_rx_buff_phy,
+					 IRDA_FRAME_SIZE_LIMIT, DMA_FROM_DEVICE,
+					 DMA_PREP_INTERRUPT);
+	if (!tx) {
+		dev_err(si->dev, "prep_slave_sg() failed\n");
+		return;
+	}
+	tx->callback = pxa_irda_fir_dma_rx_irq;
+	tx->callback_param = si;
+	si->rx_cookie = dmaengine_submit(tx);
+	dma_async_issue_pending(si->rxdma);
 }
 
 inline static void pxa_irda_fir_dma_tx_start(struct pxa_irda *si)
 {
-	DCSR(si->txdma)  = DCSR_NODESC;
-	DSADR(si->txdma) = si->dma_tx_buff_phy;
-	DTADR(si->txdma) = (unsigned long)si->irda_base + ICDR;
-	DCMD(si->txdma) = DCMD_INCSRCADDR | DCMD_FLOWTRG |  DCMD_ENDIRQEN | DCMD_WIDTH1 | DCMD_BURST32 | si->dma_tx_buff_len;
-	DCSR(si->txdma) |= DCSR_RUN;
+	struct dma_async_tx_descriptor *tx;
+
+	tx = dmaengine_prep_slave_single(si->txdma, si->dma_tx_buff_phy,
+					 si->dma_tx_buff_len, DMA_TO_DEVICE,
+					 DMA_PREP_INTERRUPT);
+	if (!tx) {
+		dev_err(si->dev, "prep_slave_sg() failed\n");
+		return;
+	}
+	tx->callback = pxa_irda_fir_dma_tx_irq;
+	tx->callback_param = si;
+	si->tx_cookie = dmaengine_submit(tx);
+	dma_async_issue_pending(si->rxdma);
 }
 
 /*
@@ -242,7 +269,7 @@ static int pxa_irda_set_speed(struct pxa_irda *si, int speed)
 
 		if (IS_FIR(si)) {
 			/* stop RX DMA */
-			DCSR(si->rxdma) &= ~DCSR_RUN;
+			dmaengine_terminate_all(si->rxdma);
 			/* disable FICP */
 			ficp_writel(si, 0, ICCR0);
 			pxa_irda_disable_clk(si);
@@ -388,30 +415,27 @@ static irqreturn_t pxa_irda_sir_irq(int irq, void *dev_id)
 }
 
 /* FIR Receive DMA interrupt handler */
-static void pxa_irda_fir_dma_rx_irq(int channel, void *data)
+static void pxa_irda_fir_dma_rx_irq(void *data)
 {
-	int dcsr = DCSR(channel);
-
-	DCSR(channel) = dcsr & ~DCSR_RUN;
+	struct net_device *dev = data;
+	struct pxa_irda *si = netdev_priv(dev);
 
-	printk(KERN_DEBUG "pxa_ir: fir rx dma bus error %#x\n", dcsr);
+	dmaengine_terminate_all(si->rxdma);
+	netdev_dbg(dev, "pxa_ir: fir rx dma bus error\n");
 }
 
 /* FIR Transmit DMA interrupt handler */
-static void pxa_irda_fir_dma_tx_irq(int channel, void *data)
+static void pxa_irda_fir_dma_tx_irq(void *data)
 {
 	struct net_device *dev = data;
 	struct pxa_irda *si = netdev_priv(dev);
-	int dcsr;
-
-	dcsr = DCSR(channel);
-	DCSR(channel) = dcsr & ~DCSR_RUN;
 
-	if (dcsr & DCSR_ENDINTR)  {
+	dmaengine_terminate_all(si->txdma);
+	if (dmaengine_tx_status(si->txdma, si->tx_cookie, NULL) == DMA_ERROR) {
+		dev->stats.tx_errors++;
+	} else {
 		dev->stats.tx_packets++;
 		dev->stats.tx_bytes += si->dma_tx_buff_len;
-	} else {
-		dev->stats.tx_errors++;
 	}
 
 	while (ficp_readl(si, ICSR1) & ICSR1_TBY)
@@ -446,9 +470,12 @@ static void pxa_irda_fir_dma_tx_irq(int channel, void *data)
 static void pxa_irda_fir_irq_eif(struct pxa_irda *si, struct net_device *dev, int icsr0)
 {
 	unsigned int len, stat, data;
+	struct dma_tx_state state;
 
 	/* Get the current data position. */
-	len = DTADR(si->rxdma) - si->dma_rx_buff_phy;
+
+	dmaengine_tx_status(si->rxdma, si->rx_cookie, &state);
+	len = IRDA_FRAME_SIZE_LIMIT - state.residue;
 
 	do {
 		/* Read Status, and then Data. 	 */
@@ -515,7 +542,7 @@ static irqreturn_t pxa_irda_fir_irq(int irq, void *dev_id)
 	int icsr0, i = 64;
 
 	/* stop RX DMA */
-	DCSR(si->rxdma) &= ~DCSR_RUN;
+	dmaengine_terminate_all(si->rxdma);
 	si->last_clk = sched_clock();
 	icsr0 = ficp_readl(si, ICSR0);
 
@@ -597,7 +624,7 @@ static int pxa_irda_hard_xmit(struct sk_buff *skb, struct net_device *dev)
 				cpu_relax();
 
 		/* stop RX DMA,  disable FICP */
-		DCSR(si->rxdma) &= ~DCSR_RUN;
+		dmaengine_terminate_all(si->rxdma);
 		ficp_writel(si, 0, ICCR0);
 
 		pxa_irda_fir_dma_tx_start(si);
@@ -670,10 +697,6 @@ static void pxa_irda_startup(struct pxa_irda *si)
 	/* configure FICP ICCR2 */
 	ficp_writel(si, ICCR2_TXP | ICCR2_TRIG_32, ICCR2);
 
-	/* configure DMAC */
-	DRCMR(17) = si->rxdma | DRCMR_MAPVLD;
-	DRCMR(18) = si->txdma | DRCMR_MAPVLD;
-
 	/* force SIR reinitialization */
 	si->speed = 4000000;
 	pxa_irda_set_speed(si, 9600);
@@ -693,17 +716,14 @@ static void pxa_irda_shutdown(struct pxa_irda *si)
 	stuart_writel(si, 0, STISR);
 
 	/* disable DMA */
-	DCSR(si->txdma) &= ~DCSR_RUN;
-	DCSR(si->rxdma) &= ~DCSR_RUN;
+	dmaengine_terminate_all(si->rxdma);
+	dmaengine_terminate_all(si->txdma);
 	/* disable FICP */
 	ficp_writel(si, 0, ICCR0);
 
 	/* disable the STUART or FICP clocks */
 	pxa_irda_disable_clk(si);
 
-	DRCMR(17) = 0;
-	DRCMR(18) = 0;
-
 	local_irq_restore(flags);
 
 	/* power off board transceiver */
@@ -715,6 +735,9 @@ static void pxa_irda_shutdown(struct pxa_irda *si)
 static int pxa_irda_start(struct net_device *dev)
 {
 	struct pxa_irda *si = netdev_priv(dev);
+	dma_cap_mask_t mask;
+	struct dma_slave_config	config;
+	struct pxad_param param;
 	int err;
 
 	si->speed = 9600;
@@ -734,14 +757,37 @@ static int pxa_irda_start(struct net_device *dev)
 	disable_irq(si->icp_irq);
 
 	err = -EBUSY;
-	si->rxdma = pxa_request_dma("FICP_RX",DMA_PRIO_LOW, pxa_irda_fir_dma_rx_irq, dev);
-	if (si->rxdma < 0)
+	dma_cap_zero(mask);
+	dma_cap_set(DMA_SLAVE, mask);
+	param.prio = PXAD_PRIO_LOWEST;
+
+	memset(&config, 0, sizeof(config));
+	config.src_addr_width = DMA_SLAVE_BUSWIDTH_1_BYTE;
+	config.dst_addr_width = DMA_SLAVE_BUSWIDTH_1_BYTE;
+	config.src_addr = (dma_addr_t)si->irda_base + ICDR;
+	config.dst_addr = (dma_addr_t)si->irda_base + ICDR;
+	config.src_maxburst = 32;
+	config.dst_maxburst = 32;
+
+	param.drcmr = si->drcmr_rx;
+	si->rxdma = dma_request_slave_channel_compat(mask, pxad_filter_fn,
+						     &param, &dev->dev, "rx");
+	if (!si->rxdma)
 		goto err_rx_dma;
 
-	si->txdma = pxa_request_dma("FICP_TX",DMA_PRIO_LOW, pxa_irda_fir_dma_tx_irq, dev);
-	if (si->txdma < 0)
+	param.drcmr = si->drcmr_tx;
+	si->txdma = dma_request_slave_channel_compat(mask, pxad_filter_fn,
+						     &param, &dev->dev, "tx");
+	if (!si->txdma)
 		goto err_tx_dma;
 
+	err = dmaengine_slave_config(si->rxdma, &config);
+	if (err)
+		goto err_dma_rx_buff;
+	err = dmaengine_slave_config(si->txdma, &config);
+	if (err)
+		goto err_dma_rx_buff;
+
 	err = -ENOMEM;
 	si->dma_rx_buff = dma_alloc_coherent(si->dev, IRDA_FRAME_SIZE_LIMIT,
 					     &si->dma_rx_buff_phy, GFP_KERNEL);
@@ -781,9 +827,9 @@ err_irlap:
 err_dma_tx_buff:
 	dma_free_coherent(si->dev, IRDA_FRAME_SIZE_LIMIT, si->dma_rx_buff, si->dma_rx_buff_phy);
 err_dma_rx_buff:
-	pxa_free_dma(si->txdma);
+	dma_release_channel(si->txdma);
 err_tx_dma:
-	pxa_free_dma(si->rxdma);
+	dma_release_channel(si->rxdma);
 err_rx_dma:
 	free_irq(si->icp_irq, dev);
 err_irq2:
@@ -810,8 +856,10 @@ static int pxa_irda_stop(struct net_device *dev)
 	free_irq(si->uart_irq, dev);
 	free_irq(si->icp_irq, dev);
 
-	pxa_free_dma(si->rxdma);
-	pxa_free_dma(si->txdma);
+	dmaengine_terminate_all(si->rxdma);
+	dmaengine_terminate_all(si->txdma);
+	dma_release_channel(si->rxdma);
+	dma_release_channel(si->txdma);
 
 	if (si->dma_rx_buff)
 		dma_free_coherent(si->dev, IRDA_FRAME_SIZE_LIMIT, si->dma_tx_buff, si->dma_tx_buff_phy);
@@ -920,6 +968,13 @@ static int pxa_irda_probe(struct platform_device *pdev)
 		goto err_mem_4;
 	}
 
+	res = platform_get_resource(pdev, IORESOURCE_DMA, 0);
+	if (res)
+		si->drcmr_rx = res->start;
+	res = platform_get_resource(pdev, IORESOURCE_DMA, 1);
+	if (res)
+		si->drcmr_tx = res->start;
+
 	/*
 	 * Initialise the SIR buffers
 	 */
-- 
2.1.4


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 2/3] net: irda: pxaficp_ir: convert to readl and writel
  2015-09-12 11:45 ` [PATCH v2 2/3] net: irda: pxaficp_ir: convert to readl and writel Robert Jarzmik
@ 2015-09-13  1:34   ` Petr Cvek
  0 siblings, 0 replies; 10+ messages in thread
From: Petr Cvek @ 2015-09-13  1:34 UTC (permalink / raw)
  To: Robert Jarzmik, Samuel Ortiz; +Cc: netdev, linux-kernel, Arnd Bergmann

Dne 12.9.2015 v 13:45 Robert Jarzmik napsal(a):
> Convert the pxa IRDA driver to readl and writel primitives, and remove
> another set of direct registers access. This leaves only the DMA
> registers access, which will be dealt with dmaengine conversion.
> 
> Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr>
> ---
> Since v1: modified __REG macro to cope with STIER, ST* registers
> ---
>  drivers/net/irda/pxaficp_ir.c | 210 +++++++++++++++++++++++++-----------------
>  1 file changed, 126 insertions(+), 84 deletions(-)
> 
> diff --git a/drivers/net/irda/pxaficp_ir.c b/drivers/net/irda/pxaficp_ir.c
> index b1794998c68e..4a2b3f71e4a8 100644
> --- a/drivers/net/irda/pxaficp_ir.c
> +++ b/drivers/net/irda/pxaficp_ir.c
> @@ -29,15 +29,16 @@
>  
>  #include <mach/dma.h>
>  #include <linux/platform_data/irda-pxaficp.h>
> +#undef __REG
> +#define __REG(x) ((x) & 0xffff)
>  #include <mach/regs-uart.h>
What are future plans for the definitions in the mach/regs-uart.h ? Maybe it would be better to duplicate register definition in ficp source code (it seems that normal PXA UART driver does not use these ones). But random searches shows, that at least base address register:

	#define STUART          STRBR
	#define STRBR           __REG(0x40700000)  /* Receive Buffer Register (read only) */

is used in machine init source codes. I can look at it in the "near" future (if I don't forget :-D).

In other case patchset works, as it has been tested for SIR part.

Tested-by: Petr Cvek <petr.cvek@tul.cz>


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 1/3] net: irda: pxaficp_ir: use sched_clock() for time management
  2015-09-12 11:45 [PATCH v2 1/3] net: irda: pxaficp_ir: use sched_clock() for time management Robert Jarzmik
  2015-09-12 11:45 ` [PATCH v2 2/3] net: irda: pxaficp_ir: convert to readl and writel Robert Jarzmik
  2015-09-12 11:45 ` [PATCH v2 3/3] net: irda: pxaficp_ir: dmaengine conversion Robert Jarzmik
@ 2015-09-15 23:40 ` David Miller
  2015-09-16  9:34   ` Robert Jarzmik
  2 siblings, 1 reply; 10+ messages in thread
From: David Miller @ 2015-09-15 23:40 UTC (permalink / raw)
  To: robert.jarzmik; +Cc: samuel, petr.cvek, netdev, linux-kernel, arnd

From: Robert Jarzmik <robert.jarzmik@free.fr>
Date: Sat, 12 Sep 2015 13:45:22 +0200

> Instead of using directly the OS timer through direct register access,
> use the standard sched_clock(), which will end up in OSCR reading
> anyway.
> 
> This is a first step for direct access register removal and machine
> specific code removal from this driver.
> 
> Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr>

What is the granularity of the OSCR register?

If it is not nanoseconds, then you need to adjust calculations
such as this one:

> @@ -549,7 +548,7 @@ static int pxa_irda_hard_xmit(struct sk_buff *skb, struct net_device *dev)
>  		skb_copy_from_linear_data(skb, si->dma_tx_buff, skb->len);
>  
>  		if (mtt)
> -			while ((unsigned)(readl_relaxed(OSCR) - si->last_oscr)/4 < mtt)
> +			while ((sched_clock() - si->last_clk) / 4 < mtt)
>  				cpu_relax();

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 1/3] net: irda: pxaficp_ir: use sched_clock() for time management
  2015-09-15 23:40 ` [PATCH v2 1/3] net: irda: pxaficp_ir: use sched_clock() for time management David Miller
@ 2015-09-16  9:34   ` Robert Jarzmik
  2015-09-17 21:51     ` David Miller
  0 siblings, 1 reply; 10+ messages in thread
From: Robert Jarzmik @ 2015-09-16  9:34 UTC (permalink / raw)
  To: David Miller; +Cc: samuel, petr.cvek, netdev, linux-kernel, arnd

David Miller <davem@davemloft.net> writes:

> From: Robert Jarzmik <robert.jarzmik@free.fr>
> Date: Sat, 12 Sep 2015 13:45:22 +0200
>
>> Instead of using directly the OS timer through direct register access,
>> use the standard sched_clock(), which will end up in OSCR reading
>> anyway.
>> 
>> This is a first step for direct access register removal and machine
>> specific code removal from this driver.
>> 
>> Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr>
>
> What is the granularity of the OSCR register?
It's 307ns (ie. 3.25MHz clock).

> If it is not nanoseconds, then you need to adjust calculations
> such as this one:
Tell me if the 307ns requires something I should adjust.

My understanding is that the flow will be :
 sched_clock()
   rd->read_sched_clock() (cyc_to_ns() transformed for return)
     pxa_read_sched_clock()
       readl_relaxed(OSCR)

I didn't see any timings issue, as the flow looks equivalent to the readl(OSCR),
but I might have overlooked something.

Cheers.

-- 
Robert

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 1/3] net: irda: pxaficp_ir: use sched_clock() for time management
  2015-09-16  9:34   ` Robert Jarzmik
@ 2015-09-17 21:51     ` David Miller
  2015-09-18 16:36       ` Robert Jarzmik
  0 siblings, 1 reply; 10+ messages in thread
From: David Miller @ 2015-09-17 21:51 UTC (permalink / raw)
  To: robert.jarzmik; +Cc: samuel, petr.cvek, netdev, linux-kernel, arnd

From: Robert Jarzmik <robert.jarzmik@free.fr>
Date: Wed, 16 Sep 2015 11:34:01 +0200

> David Miller <davem@davemloft.net> writes:
> 
>> From: Robert Jarzmik <robert.jarzmik@free.fr>
>> Date: Sat, 12 Sep 2015 13:45:22 +0200
>>
>>> Instead of using directly the OS timer through direct register access,
>>> use the standard sched_clock(), which will end up in OSCR reading
>>> anyway.
>>> 
>>> This is a first step for direct access register removal and machine
>>> specific code removal from this driver.
>>> 
>>> Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr>
>>
>> What is the granularity of the OSCR register?
> It's 307ns (ie. 3.25MHz clock).
> 
>> If it is not nanoseconds, then you need to adjust calculations
>> such as this one:
> Tell me if the 307ns requires something I should adjust.
> 
> My understanding is that the flow will be :
>  sched_clock()
>    rd->read_sched_clock() (cyc_to_ns() transformed for return)
>      pxa_read_sched_clock()
>        readl_relaxed(OSCR)
> 
> I didn't see any timings issue, as the flow looks equivalent to the readl(OSCR),
> but I might have overlooked something.

Of course it's different, because sched_clock() converts the value read
from OSCR into nanoseconds, which is obviously different from using the
OSCR register value directly.

You're therefore feeding different values into this IRDA code.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 1/3] net: irda: pxaficp_ir: use sched_clock() for time management
  2015-09-17 21:51     ` David Miller
@ 2015-09-18 16:36       ` Robert Jarzmik
  2015-09-21 23:12         ` David Miller
  0 siblings, 1 reply; 10+ messages in thread
From: Robert Jarzmik @ 2015-09-18 16:36 UTC (permalink / raw)
  To: David Miller; +Cc: samuel, petr.cvek, netdev, linux-kernel, arnd

David Miller <davem@davemloft.net> writes:

>> My understanding is that the flow will be :
>>  sched_clock()
>>    rd->read_sched_clock() (cyc_to_ns() transformed for return)
>>      pxa_read_sched_clock()
>>        readl_relaxed(OSCR)
>> 
>> I didn't see any timings issue, as the flow looks equivalent to the readl(OSCR),
>> but I might have overlooked something.
>
> Of course it's different, because sched_clock() converts the value read
> from OSCR into nanoseconds, which is obviously different from using the
> OSCR register value directly.
>
> You're therefore feeding different values into this IRDA code.
Ah yes, I see it.
Which brings me to wonder which is the more correct :
 (a) replace to reproduce the same calculation
     Previously mtt was compared to a difference of 76ns steps (as 307ns / 4 =
     76ns):
     while ((sched_clock() - si->last_clk) * 76 < mtt)

 (b) change the calculation assuming mtt is in microseconds :
     while ((sched_clock() - si->last_clk) * 1000 < mtt)

I have no IRDA protocol knowledge so unless someone points me to the correct
calculation I'll try my luck with (b).

Cheers.

-- 
Robert

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 1/3] net: irda: pxaficp_ir: use sched_clock() for time management
  2015-09-18 16:36       ` Robert Jarzmik
@ 2015-09-21 23:12         ` David Miller
  2015-09-22 19:27           ` Robert Jarzmik
  0 siblings, 1 reply; 10+ messages in thread
From: David Miller @ 2015-09-21 23:12 UTC (permalink / raw)
  To: robert.jarzmik; +Cc: samuel, petr.cvek, netdev, linux-kernel, arnd

From: Robert Jarzmik <robert.jarzmik@free.fr>
Date: Fri, 18 Sep 2015 18:36:56 +0200

> Which brings me to wonder which is the more correct :
>  (a) replace to reproduce the same calculation
>      Previously mtt was compared to a difference of 76ns steps (as 307ns / 4 =
>      76ns):
>      while ((sched_clock() - si->last_clk) * 76 < mtt)
> 
>  (b) change the calculation assuming mtt is in microseconds :
>      while ((sched_clock() - si->last_clk) * 1000 < mtt)
> 
> I have no IRDA protocol knowledge so unless someone points me to the correct
> calculation I'll try my luck with (b).

"a" would be "safer" and less likely to break anything, although as
you say "b" might be more correct.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 1/3] net: irda: pxaficp_ir: use sched_clock() for time management
  2015-09-21 23:12         ` David Miller
@ 2015-09-22 19:27           ` Robert Jarzmik
  0 siblings, 0 replies; 10+ messages in thread
From: Robert Jarzmik @ 2015-09-22 19:27 UTC (permalink / raw)
  To: David Miller; +Cc: samuel, petr.cvek, netdev, linux-kernel, arnd

David Miller <davem@davemloft.net> writes:

> From: Robert Jarzmik <robert.jarzmik@free.fr>
> Date: Fri, 18 Sep 2015 18:36:56 +0200
>
>> Which brings me to wonder which is the more correct :
>>  (a) replace to reproduce the same calculation
>>      Previously mtt was compared to a difference of 76ns steps (as 307ns / 4 =
>>      76ns):
>>      while ((sched_clock() - si->last_clk) * 76 < mtt)
>> 
>>  (b) change the calculation assuming mtt is in microseconds :
>>      while ((sched_clock() - si->last_clk) * 1000 < mtt)
>> 
>> I have no IRDA protocol knowledge so unless someone points me to the correct
>> calculation I'll try my luck with (b).
>
> "a" would be "safer" and less likely to break anything, although as
> you say "b" might be more correct.
Indeed.

Well, let me try (b) then. I'll add in the commit message the timings change,
and if anybody complains a regression pops out, I'll provide a patch to fallback
to (a).

Cheers.

-- 
Robert

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2015-09-22 19:33 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-09-12 11:45 [PATCH v2 1/3] net: irda: pxaficp_ir: use sched_clock() for time management Robert Jarzmik
2015-09-12 11:45 ` [PATCH v2 2/3] net: irda: pxaficp_ir: convert to readl and writel Robert Jarzmik
2015-09-13  1:34   ` Petr Cvek
2015-09-12 11:45 ` [PATCH v2 3/3] net: irda: pxaficp_ir: dmaengine conversion Robert Jarzmik
2015-09-15 23:40 ` [PATCH v2 1/3] net: irda: pxaficp_ir: use sched_clock() for time management David Miller
2015-09-16  9:34   ` Robert Jarzmik
2015-09-17 21:51     ` David Miller
2015-09-18 16:36       ` Robert Jarzmik
2015-09-21 23:12         ` David Miller
2015-09-22 19:27           ` Robert Jarzmik

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.