[PATCH 0/6] media: ite-cir driver cleanups

[PATCH 0/6] media: ite-cir driver cleanups

[Sean Young]

This should not be any functional changes, just various cleanups.

Sean Young (6):
  media: ite-cir: remove unused fields
  media: ite-cir: set parent device
  media: ite-cir: use standard logging and reduce noise
  media: ite-cir: carrier and duty cycle can be set via ioctl
  media: ite-cir: move runtime information into driver data
  media: ite-cir: check for receive overflow

 drivers/media/rc/ite-cir.c | 341 +++++++++----------------------------
 drivers/media/rc/ite-cir.h |  49 ++----
 2 files changed, 91 insertions(+), 299 deletions(-)

-- 
2.29.2

[PATCH 1/6] media: ite-cir: remove unused fields

[Sean Young]

These fields are not referenced anywhere.

Signed-off-by: Sean Young <sean@mess.org>
---
 drivers/media/rc/ite-cir.h | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/media/rc/ite-cir.h b/drivers/media/rc/ite-cir.h
index 4954470448a7..dfdc26ba976c 100644
--- a/drivers/media/rc/ite-cir.h
+++ b/drivers/media/rc/ite-cir.h
@@ -111,14 +111,12 @@ struct ite_dev_params {
 struct ite_dev {
 	struct pnp_dev *pdev;
 	struct rc_dev *rdev;
-	struct ir_raw_event rawir;
 
 	/* sync data */
 	spinlock_t lock;
 	bool in_use, transmitting;
 
 	/* transmit support */
-	int tx_fifo_allowance;
 	wait_queue_head_t tx_queue, tx_ended;
 
 	/* hardware I/O settings */
-- 
2.29.2

[PATCH 2/6] media: ite-cir: set parent device

[Sean Young]

This ensure that the rc device shows up correctly in sysfs.

Signed-off-by: Sean Young <sean@mess.org>
---
 drivers/media/rc/ite-cir.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/media/rc/ite-cir.c b/drivers/media/rc/ite-cir.c
index 0c6229592e13..397e317d9726 100644
--- a/drivers/media/rc/ite-cir.c
+++ b/drivers/media/rc/ite-cir.c
@@ -1544,6 +1544,7 @@ static int ite_probe(struct pnp_dev *pdev, const struct pnp_device_id
 
 	/* set up ir-core props */
 	rdev->priv = itdev;
+	rdev->dev.parent = &pdev->dev;
 	rdev->allowed_protocols = RC_PROTO_BIT_ALL_IR_DECODER;
 	rdev->open = ite_open;
 	rdev->close = ite_close;
-- 
2.29.2

[PATCH 3/6] media: ite-cir: use standard logging and reduce noise

[Sean Young]

Remove lots of superfluous debug messages which just log the function
name. Also, there is no need to announce the various parameters of the
device. This can be discovered via lirc ioctl if needed.

Signed-off-by: Sean Young <sean@mess.org>
---
 drivers/media/rc/ite-cir.c | 171 +++++--------------------------------
 drivers/media/rc/ite-cir.h |  15 ----
 2 files changed, 22 insertions(+), 164 deletions(-)

diff --git a/drivers/media/rc/ite-cir.c b/drivers/media/rc/ite-cir.c
index 397e317d9726..c5c7631849d0 100644
--- a/drivers/media/rc/ite-cir.c
+++ b/drivers/media/rc/ite-cir.c
@@ -33,11 +33,6 @@
 
 /* module parameters */
 
-/* debug level */
-static int debug;
-module_param(debug, int, S_IRUGO | S_IWUSR);
-MODULE_PARM_DESC(debug, "Enable debugging output");
-
 /* low limit for RX carrier freq, Hz, 0 for no RX demodulation */
 static int rx_low_carrier_freq;
 module_param(rx_low_carrier_freq, int, S_IRUGO | S_IWUSR);
@@ -101,9 +96,7 @@ static u8 ite_get_carrier_freq_bits(unsigned int freq)
 			freq = ITE_LCF_MAX_CARRIER_FREQ;
 
 		/* convert to kHz and subtract the base freq */
-		freq =
-		    DIV_ROUND_CLOSEST(freq - ITE_LCF_MIN_CARRIER_FREQ,
-				      1000);
+		freq = DIV_ROUND_CLOSEST(freq - ITE_LCF_MIN_CARRIER_FREQ, 1000);
 
 		return (u8) freq;
 	}
@@ -175,8 +168,7 @@ static void ite_decode_bytes(struct ite_dev *dev, const u8 * data, int
 	next_one = find_next_bit_le(ldata, size, 0);
 	if (next_one > 0) {
 		ev.pulse = true;
-		ev.duration =
-		    ITE_BITS_TO_US(next_one, sample_period);
+		ev.duration = ITE_BITS_TO_US(next_one, sample_period);
 		ir_raw_event_store_with_filter(dev->rdev, &ev);
 	}
 
@@ -187,23 +179,18 @@ static void ite_decode_bytes(struct ite_dev *dev, const u8 * data, int
 		ir_raw_event_store_with_filter(dev->rdev, &ev);
 
 		if (next_zero < size) {
-			next_one =
-			    find_next_bit_le(ldata,
-						     size,
-						     next_zero + 1);
+			next_one = find_next_bit_le(ldata, size, next_zero + 1);
 			ev.pulse = true;
-			ev.duration =
-			    ITE_BITS_TO_US(next_one - next_zero,
-					   sample_period);
-			ir_raw_event_store_with_filter
-			    (dev->rdev, &ev);
+			ev.duration = ITE_BITS_TO_US(next_one - next_zero,
+						     sample_period);
+			ir_raw_event_store_with_filter(dev->rdev, &ev);
 		} else
 			next_one = size;
 	}
 
 	ir_raw_event_handle(dev->rdev);
 
-	ite_dbg_verbose("decoded %d bytes.", length);
+	dev_dbg(&dev->rdev->dev, "decoded %d bytes\n", length);
 }
 
 /* set all the rx/tx carrier parameters; this must be called with the device
@@ -215,8 +202,6 @@ static void ite_set_carrier_params(struct ite_dev *dev)
 	bool use_demodulator;
 	bool for_tx = dev->transmitting;
 
-	ite_dbg("%s called", __func__);
-
 	if (for_tx) {
 		/* we don't need no stinking calculations */
 		freq = dev->params.tx_carrier_freq;
@@ -228,8 +213,7 @@ static void ite_set_carrier_params(struct ite_dev *dev)
 
 		if (low_freq == 0) {
 			/* don't demodulate */
-			freq =
-			ITE_DEFAULT_CARRIER_FREQ;
+			freq = ITE_DEFAULT_CARRIER_FREQ;
 			allowance = ITE_RXDCR_DEFAULT;
 			use_demodulator = false;
 		} else {
@@ -268,8 +252,6 @@ static irqreturn_t ite_cir_isr(int irq, void *data)
 	int rx_bytes;
 	int iflags;
 
-	ite_dbg_verbose("%s firing", __func__);
-
 	/* grab the spinlock */
 	spin_lock_irqsave(&dev->lock, flags);
 
@@ -279,32 +261,29 @@ static irqreturn_t ite_cir_isr(int irq, void *data)
 	/* check for the receive interrupt */
 	if (iflags & (ITE_IRQ_RX_FIFO | ITE_IRQ_RX_FIFO_OVERRUN)) {
 		/* read the FIFO bytes */
-		rx_bytes =
-			dev->params.get_rx_bytes(dev, rx_buf,
-					     ITE_RX_FIFO_LEN);
+		rx_bytes = dev->params.get_rx_bytes(dev, rx_buf,
+						    ITE_RX_FIFO_LEN);
+
+		dev_dbg(&dev->rdev->dev, "interrupt %d RX bytes\n", rx_bytes);
 
 		if (rx_bytes > 0) {
 			/* drop the spinlock, since the ir-core layer
 			 * may call us back again through
 			 * ite_s_idle() */
-			spin_unlock_irqrestore(&dev->
-									 lock,
-									 flags);
+			spin_unlock_irqrestore(&dev->lock, flags);
 
 			/* decode the data we've just received */
-			ite_decode_bytes(dev, rx_buf,
-								   rx_bytes);
+			ite_decode_bytes(dev, rx_buf, rx_bytes);
 
 			/* reacquire the spinlock */
-			spin_lock_irqsave(&dev->lock,
-								    flags);
+			spin_lock_irqsave(&dev->lock, flags);
 
 			/* mark the interrupt as serviced */
 			ret = IRQ_RETVAL(IRQ_HANDLED);
 		}
 	} else if (iflags & ITE_IRQ_TX_FIFO) {
 		/* FIFO space available interrupt */
-		ite_dbg_verbose("got interrupt for TX FIFO");
+		dev_dbg(&dev->rdev->dev, "interrupt TX FIFO\n");
 
 		/* wake any sleeping transmitter */
 		wake_up_interruptible(&dev->tx_queue);
@@ -316,8 +295,6 @@ static irqreturn_t ite_cir_isr(int irq, void *data)
 	/* drop the spinlock */
 	spin_unlock_irqrestore(&dev->lock, flags);
 
-	ite_dbg_verbose("%s done returning %d", __func__, (int)ret);
-
 	return ret;
 }
 
@@ -379,8 +356,6 @@ static int ite_tx_ir(struct rc_dev *rcdev, unsigned *txbuf, unsigned n)
 	u8 last_sent[ITE_TX_FIFO_LEN];
 	u8 val;
 
-	ite_dbg("%s called", __func__);
-
 	/* clear the array just in case */
 	memset(last_sent, 0, sizeof(last_sent));
 
@@ -404,8 +379,7 @@ static int ite_tx_ir(struct rc_dev *rcdev, unsigned *txbuf, unsigned n)
 	 * then we'll just activate the interrupt, wait for it to wake us up
 	 * again, disable it, continue filling the FIFO... until everything
 	 * has been pushed out */
-	fifo_avail =
-	    ITE_TX_FIFO_LEN - dev->params.get_tx_used_slots(dev);
+	fifo_avail = ITE_TX_FIFO_LEN - dev->params.get_tx_used_slots(dev);
 
 	while (n > 0 && dev->in_use) {
 		/* transmit the next sample */
@@ -413,10 +387,8 @@ static int ite_tx_ir(struct rc_dev *rcdev, unsigned *txbuf, unsigned n)
 		remaining_us = *(txbuf++);
 		n--;
 
-		ite_dbg("%s: %ld",
-				      ((is_pulse) ? "pulse" : "space"),
-				      (long int)
-				      remaining_us);
+		dev_dbg(&dev->rdev->dev, "%s: %d\n",
+			is_pulse ? "pulse" : "space", remaining_us);
 
 		/* repeat while the pulse is non-zero length */
 		while (remaining_us > 0 && dev->in_use) {
@@ -524,8 +496,6 @@ static void ite_s_idle(struct rc_dev *rcdev, bool enable)
 	unsigned long flags;
 	struct ite_dev *dev = rcdev->priv;
 
-	ite_dbg("%s called", __func__);
-
 	if (enable) {
 		spin_lock_irqsave(&dev->lock, flags);
 		dev->params.idle_rx(dev);
@@ -544,8 +514,6 @@ static int it87_get_irq_causes(struct ite_dev *dev)
 	u8 iflags;
 	int ret = 0;
 
-	ite_dbg("%s called", __func__);
-
 	/* read the interrupt flags */
 	iflags = inb(dev->cir_addr + IT87_IIR) & IT87_II;
 
@@ -572,8 +540,6 @@ static void it87_set_carrier_params(struct ite_dev *dev, bool high_freq,
 {
 	u8 val;
 
-	ite_dbg("%s called", __func__);
-
 	/* program the RCR register */
 	val = inb(dev->cir_addr + IT87_RCR)
 		& ~(IT87_HCFS | IT87_RXEND | IT87_RXDCR);
@@ -599,8 +565,6 @@ static int it87_get_rx_bytes(struct ite_dev *dev, u8 * buf, int buf_size)
 {
 	int fifo, read = 0;
 
-	ite_dbg("%s called", __func__);
-
 	/* read how many bytes are still in the FIFO */
 	fifo = inb(dev->cir_addr + IT87_RSR) & IT87_RXFBC;
 
@@ -619,8 +583,6 @@ static int it87_get_rx_bytes(struct ite_dev *dev, u8 * buf, int buf_size)
  * empty; let's expect this won't be a problem */
 static int it87_get_tx_used_slots(struct ite_dev *dev)
 {
-	ite_dbg("%s called", __func__);
-
 	return inb(dev->cir_addr + IT87_TSR) & IT87_TXFBC;
 }
 
@@ -634,8 +596,6 @@ static void it87_put_tx_byte(struct ite_dev *dev, u8 value)
   pulse is detected; this must be called with the device spinlock held */
 static void it87_idle_rx(struct ite_dev *dev)
 {
-	ite_dbg("%s called", __func__);
-
 	/* disable streaming by clearing RXACT writing it as 1 */
 	outb(inb(dev->cir_addr + IT87_RCR) | IT87_RXACT,
 		dev->cir_addr + IT87_RCR);
@@ -648,8 +608,6 @@ static void it87_idle_rx(struct ite_dev *dev)
 /* disable the receiver; this must be called with the device spinlock held */
 static void it87_disable_rx(struct ite_dev *dev)
 {
-	ite_dbg("%s called", __func__);
-
 	/* disable the receiver interrupts */
 	outb(inb(dev->cir_addr + IT87_IER) & ~(IT87_RDAIE | IT87_RFOIE),
 		dev->cir_addr + IT87_IER);
@@ -666,8 +624,6 @@ static void it87_disable_rx(struct ite_dev *dev)
 /* enable the receiver; this must be called with the device spinlock held */
 static void it87_enable_rx(struct ite_dev *dev)
 {
-	ite_dbg("%s called", __func__);
-
 	/* enable the receiver by setting RXEN */
 	outb(inb(dev->cir_addr + IT87_RCR) | IT87_RXEN,
 		dev->cir_addr + IT87_RCR);
@@ -684,8 +640,6 @@ static void it87_enable_rx(struct ite_dev *dev)
  * spinlock held */
 static void it87_disable_tx_interrupt(struct ite_dev *dev)
 {
-	ite_dbg("%s called", __func__);
-
 	/* disable the transmitter interrupts */
 	outb(inb(dev->cir_addr + IT87_IER) & ~IT87_TLDLIE,
 		dev->cir_addr + IT87_IER);
@@ -695,8 +649,6 @@ static void it87_disable_tx_interrupt(struct ite_dev *dev)
  * spinlock held */
 static void it87_enable_tx_interrupt(struct ite_dev *dev)
 {
-	ite_dbg("%s called", __func__);
-
 	/* enable the transmitter interrupts and master enable flag */
 	outb(inb(dev->cir_addr + IT87_IER) | IT87_TLDLIE | IT87_IEC,
 		dev->cir_addr + IT87_IER);
@@ -705,8 +657,6 @@ static void it87_enable_tx_interrupt(struct ite_dev *dev)
 /* disable the device; this must be called with the device spinlock held */
 static void it87_disable(struct ite_dev *dev)
 {
-	ite_dbg("%s called", __func__);
-
 	/* clear out all interrupt enable flags */
 	outb(inb(dev->cir_addr + IT87_IER) &
 		~(IT87_IEC | IT87_RFOIE | IT87_RDAIE | IT87_TLDLIE),
@@ -723,8 +673,6 @@ static void it87_disable(struct ite_dev *dev)
 /* initialize the hardware */
 static void it87_init_hardware(struct ite_dev *dev)
 {
-	ite_dbg("%s called", __func__);
-
 	/* enable just the baud rate divisor register,
 	disabling all the interrupts at the same time */
 	outb((inb(dev->cir_addr + IT87_IER) &
@@ -761,8 +709,6 @@ static int it8708_get_irq_causes(struct ite_dev *dev)
 	u8 iflags;
 	int ret = 0;
 
-	ite_dbg("%s called", __func__);
-
 	/* read the interrupt flags */
 	iflags = inb(dev->cir_addr + IT8708_C0IIR);
 
@@ -784,8 +730,6 @@ static void it8708_set_carrier_params(struct ite_dev *dev, bool high_freq,
 {
 	u8 val;
 
-	ite_dbg("%s called", __func__);
-
 	/* program the C0CFR register, with HRAE=1 */
 	outb(inb(dev->cir_addr + IT8708_BANKSEL) | IT8708_HRAE,
 		dev->cir_addr + IT8708_BANKSEL);
@@ -824,8 +768,6 @@ static int it8708_get_rx_bytes(struct ite_dev *dev, u8 * buf, int buf_size)
 {
 	int fifo, read = 0;
 
-	ite_dbg("%s called", __func__);
-
 	/* read how many bytes are still in the FIFO */
 	fifo = inb(dev->cir_addr + IT8708_C0RFSR) & IT85_RXFBC;
 
@@ -844,8 +786,6 @@ static int it8708_get_rx_bytes(struct ite_dev *dev, u8 * buf, int buf_size)
  * empty; let's expect this won't be a problem */
 static int it8708_get_tx_used_slots(struct ite_dev *dev)
 {
-	ite_dbg("%s called", __func__);
-
 	return inb(dev->cir_addr + IT8708_C0TFSR) & IT85_TXFBC;
 }
 
@@ -859,8 +799,6 @@ static void it8708_put_tx_byte(struct ite_dev *dev, u8 value)
   pulse is detected; this must be called with the device spinlock held */
 static void it8708_idle_rx(struct ite_dev *dev)
 {
-	ite_dbg("%s called", __func__);
-
 	/* disable streaming by clearing RXACT writing it as 1 */
 	outb(inb(dev->cir_addr + IT8708_C0RCR) | IT85_RXACT,
 		dev->cir_addr + IT8708_C0RCR);
@@ -873,8 +811,6 @@ static void it8708_idle_rx(struct ite_dev *dev)
 /* disable the receiver; this must be called with the device spinlock held */
 static void it8708_disable_rx(struct ite_dev *dev)
 {
-	ite_dbg("%s called", __func__);
-
 	/* disable the receiver interrupts */
 	outb(inb(dev->cir_addr + IT8708_C0IER) &
 		~(IT85_RDAIE | IT85_RFOIE),
@@ -892,8 +828,6 @@ static void it8708_disable_rx(struct ite_dev *dev)
 /* enable the receiver; this must be called with the device spinlock held */
 static void it8708_enable_rx(struct ite_dev *dev)
 {
-	ite_dbg("%s called", __func__);
-
 	/* enable the receiver by setting RXEN */
 	outb(inb(dev->cir_addr + IT8708_C0RCR) | IT85_RXEN,
 		dev->cir_addr + IT8708_C0RCR);
@@ -911,8 +845,6 @@ static void it8708_enable_rx(struct ite_dev *dev)
  * spinlock held */
 static void it8708_disable_tx_interrupt(struct ite_dev *dev)
 {
-	ite_dbg("%s called", __func__);
-
 	/* disable the transmitter interrupts */
 	outb(inb(dev->cir_addr + IT8708_C0IER) & ~IT85_TLDLIE,
 		dev->cir_addr + IT8708_C0IER);
@@ -922,8 +854,6 @@ static void it8708_disable_tx_interrupt(struct ite_dev *dev)
  * spinlock held */
 static void it8708_enable_tx_interrupt(struct ite_dev *dev)
 {
-	ite_dbg("%s called", __func__);
-
 	/* enable the transmitter interrupts and master enable flag */
 	outb(inb(dev->cir_addr + IT8708_C0IER)
 		|IT85_TLDLIE | IT85_IEC,
@@ -933,8 +863,6 @@ static void it8708_enable_tx_interrupt(struct ite_dev *dev)
 /* disable the device; this must be called with the device spinlock held */
 static void it8708_disable(struct ite_dev *dev)
 {
-	ite_dbg("%s called", __func__);
-
 	/* clear out all interrupt enable flags */
 	outb(inb(dev->cir_addr + IT8708_C0IER) &
 		~(IT85_IEC | IT85_RFOIE | IT85_RDAIE | IT85_TLDLIE),
@@ -951,8 +879,6 @@ static void it8708_disable(struct ite_dev *dev)
 /* initialize the hardware */
 static void it8708_init_hardware(struct ite_dev *dev)
 {
-	ite_dbg("%s called", __func__);
-
 	/* disable all the interrupts */
 	outb(inb(dev->cir_addr + IT8708_C0IER) &
 		~(IT85_IEC | IT85_RFOIE | IT85_RDAIE | IT85_TLDLIE),
@@ -1058,8 +984,6 @@ static int it8709_get_irq_causes(struct ite_dev *dev)
 	u8 iflags;
 	int ret = 0;
 
-	ite_dbg("%s called", __func__);
-
 	/* read the interrupt flags */
 	iflags = it8709_rm(dev, IT8709_IIR);
 
@@ -1081,8 +1005,6 @@ static void it8709_set_carrier_params(struct ite_dev *dev, bool high_freq,
 {
 	u8 val;
 
-	ite_dbg("%s called", __func__);
-
 	val = (it8709_rr(dev, IT85_C0CFR)
 		     &~(IT85_HCFS | IT85_CFQ)) |
 	    carrier_freq_bits;
@@ -1115,8 +1037,6 @@ static int it8709_get_rx_bytes(struct ite_dev *dev, u8 * buf, int buf_size)
 {
 	int fifo, read = 0;
 
-	ite_dbg("%s called", __func__);
-
 	/* read how many bytes are still in the FIFO */
 	fifo = it8709_rm(dev, IT8709_RFSR) & IT85_RXFBC;
 
@@ -1140,8 +1060,6 @@ static int it8709_get_rx_bytes(struct ite_dev *dev, u8 * buf, int buf_size)
  * empty; let's expect this won't be a problem */
 static int it8709_get_tx_used_slots(struct ite_dev *dev)
 {
-	ite_dbg("%s called", __func__);
-
 	return it8709_rr(dev, IT85_C0TFSR) & IT85_TXFBC;
 }
 
@@ -1155,8 +1073,6 @@ static void it8709_put_tx_byte(struct ite_dev *dev, u8 value)
   pulse is detected; this must be called with the device spinlock held */
 static void it8709_idle_rx(struct ite_dev *dev)
 {
-	ite_dbg("%s called", __func__);
-
 	/* disable streaming by clearing RXACT writing it as 1 */
 	it8709_wr(dev, it8709_rr(dev, IT85_C0RCR) | IT85_RXACT,
 			    IT85_C0RCR);
@@ -1169,8 +1085,6 @@ static void it8709_idle_rx(struct ite_dev *dev)
 /* disable the receiver; this must be called with the device spinlock held */
 static void it8709_disable_rx(struct ite_dev *dev)
 {
-	ite_dbg("%s called", __func__);
-
 	/* disable the receiver interrupts */
 	it8709_wr(dev, it8709_rr(dev, IT85_C0IER) &
 			    ~(IT85_RDAIE | IT85_RFOIE),
@@ -1188,8 +1102,6 @@ static void it8709_disable_rx(struct ite_dev *dev)
 /* enable the receiver; this must be called with the device spinlock held */
 static void it8709_enable_rx(struct ite_dev *dev)
 {
-	ite_dbg("%s called", __func__);
-
 	/* enable the receiver by setting RXEN */
 	it8709_wr(dev, it8709_rr(dev, IT85_C0RCR) | IT85_RXEN,
 			    IT85_C0RCR);
@@ -1207,8 +1119,6 @@ static void it8709_enable_rx(struct ite_dev *dev)
  * spinlock held */
 static void it8709_disable_tx_interrupt(struct ite_dev *dev)
 {
-	ite_dbg("%s called", __func__);
-
 	/* disable the transmitter interrupts */
 	it8709_wr(dev, it8709_rr(dev, IT85_C0IER) & ~IT85_TLDLIE,
 			    IT85_C0IER);
@@ -1218,8 +1128,6 @@ static void it8709_disable_tx_interrupt(struct ite_dev *dev)
  * spinlock held */
 static void it8709_enable_tx_interrupt(struct ite_dev *dev)
 {
-	ite_dbg("%s called", __func__);
-
 	/* enable the transmitter interrupts and master enable flag */
 	it8709_wr(dev, it8709_rr(dev, IT85_C0IER)
 			    |IT85_TLDLIE | IT85_IEC,
@@ -1229,8 +1137,6 @@ static void it8709_enable_tx_interrupt(struct ite_dev *dev)
 /* disable the device; this must be called with the device spinlock held */
 static void it8709_disable(struct ite_dev *dev)
 {
-	ite_dbg("%s called", __func__);
-
 	/* clear out all interrupt enable flags */
 	it8709_wr(dev, it8709_rr(dev, IT85_C0IER) &
 			~(IT85_IEC | IT85_RFOIE | IT85_RDAIE | IT85_TLDLIE),
@@ -1247,8 +1153,6 @@ static void it8709_disable(struct ite_dev *dev)
 /* initialize the hardware */
 static void it8709_init_hardware(struct ite_dev *dev)
 {
-	ite_dbg("%s called", __func__);
-
 	/* disable all the interrupts */
 	it8709_wr(dev, it8709_rr(dev, IT85_C0IER) &
 			~(IT85_IEC | IT85_RFOIE | IT85_RDAIE | IT85_TLDLIE),
@@ -1290,8 +1194,6 @@ static int ite_open(struct rc_dev *rcdev)
 	struct ite_dev *dev = rcdev->priv;
 	unsigned long flags;
 
-	ite_dbg("%s called", __func__);
-
 	spin_lock_irqsave(&dev->lock, flags);
 	dev->in_use = true;
 
@@ -1309,8 +1211,6 @@ static void ite_close(struct rc_dev *rcdev)
 	struct ite_dev *dev = rcdev->priv;
 	unsigned long flags;
 
-	ite_dbg("%s called", __func__);
-
 	spin_lock_irqsave(&dev->lock, flags);
 	dev->in_use = false;
 
@@ -1449,8 +1349,6 @@ static int ite_probe(struct pnp_dev *pdev, const struct pnp_device_id
 	int model_no;
 	int io_rsrc_no;
 
-	ite_dbg("%s called", __func__);
-
 	itdev = kzalloc(sizeof(struct ite_dev), GFP_KERNEL);
 	if (!itdev)
 		return ret;
@@ -1465,16 +1363,15 @@ static int ite_probe(struct pnp_dev *pdev, const struct pnp_device_id
 
 	/* get the model number */
 	model_no = (int)dev_id->driver_data;
-	ite_pr(KERN_NOTICE, "Auto-detected model: %s\n",
+	dev_dbg(&pdev->dev, "Auto-detected model: %s\n",
 		ite_dev_descs[model_no].model);
 
 	if (model_number >= 0 && model_number < ARRAY_SIZE(ite_dev_descs)) {
 		model_no = model_number;
-		ite_pr(KERN_NOTICE, "The model has been fixed by a module parameter.");
+		dev_info(&pdev->dev, "model has been forced to: %s",
+			 ite_dev_descs[model_no].model);
 	}
 
-	ite_pr(KERN_NOTICE, "Using model: %s\n", ite_dev_descs[model_no].model);
-
 	/* get the description for the device */
 	dev_desc = &ite_dev_descs[model_no];
 	io_rsrc_no = dev_desc->io_rsrc_no;
@@ -1525,20 +1422,6 @@ static int ite_probe(struct pnp_dev *pdev, const struct pnp_device_id
 	if (rx_high_carrier_freq > 0)
 		itdev->params.rx_high_carrier_freq = rx_high_carrier_freq;
 
-	/* print out parameters */
-	ite_pr(KERN_NOTICE, "TX-capable: %d\n", (int)
-			 itdev->params.hw_tx_capable);
-	ite_pr(KERN_NOTICE, "Sample period (ns): %ld\n", (long)
-		     itdev->params.sample_period);
-	ite_pr(KERN_NOTICE, "TX carrier frequency (Hz): %d\n", (int)
-		     itdev->params.tx_carrier_freq);
-	ite_pr(KERN_NOTICE, "TX duty cycle (%%): %d\n", (int)
-		     itdev->params.tx_duty_cycle);
-	ite_pr(KERN_NOTICE, "RX low carrier frequency (Hz): %d\n", (int)
-		     itdev->params.rx_low_carrier_freq);
-	ite_pr(KERN_NOTICE, "RX high carrier frequency (Hz): %d\n", (int)
-		     itdev->params.rx_high_carrier_freq);
-
 	/* set up hardware initial state */
 	itdev->params.init_hardware(itdev);
 
@@ -1589,8 +1472,6 @@ static int ite_probe(struct pnp_dev *pdev, const struct pnp_device_id
 			ITE_DRIVER_NAME, (void *)itdev))
 		goto exit_release_cir_addr;
 
-	ite_pr(KERN_NOTICE, "driver has been successfully loaded\n");
-
 	return 0;
 
 exit_release_cir_addr:
@@ -1610,8 +1491,6 @@ static void ite_remove(struct pnp_dev *pdev)
 	struct ite_dev *dev = pnp_get_drvdata(pdev);
 	unsigned long flags;
 
-	ite_dbg("%s called", __func__);
-
 	spin_lock_irqsave(&dev->lock, flags);
 
 	/* disable hardware */
@@ -1633,8 +1512,6 @@ static int ite_suspend(struct pnp_dev *pdev, pm_message_t state)
 	struct ite_dev *dev = pnp_get_drvdata(pdev);
 	unsigned long flags;
 
-	ite_dbg("%s called", __func__);
-
 	/* wait for any transmission to end */
 	wait_event_interruptible(dev->tx_ended, !dev->transmitting);
 
@@ -1653,8 +1530,6 @@ static int ite_resume(struct pnp_dev *pdev)
 	struct ite_dev *dev = pnp_get_drvdata(pdev);
 	unsigned long flags;
 
-	ite_dbg("%s called", __func__);
-
 	spin_lock_irqsave(&dev->lock, flags);
 
 	/* reinitialize hardware config registers */
@@ -1672,8 +1547,6 @@ static void ite_shutdown(struct pnp_dev *pdev)
 	struct ite_dev *dev = pnp_get_drvdata(pdev);
 	unsigned long flags;
 
-	ite_dbg("%s called", __func__);
-
 	spin_lock_irqsave(&dev->lock, flags);
 
 	/* disable all interrupts */
diff --git a/drivers/media/rc/ite-cir.h b/drivers/media/rc/ite-cir.h
index dfdc26ba976c..853001f67445 100644
--- a/drivers/media/rc/ite-cir.h
+++ b/drivers/media/rc/ite-cir.h
@@ -8,21 +8,6 @@
 /* platform driver name to register */
 #define ITE_DRIVER_NAME "ite-cir"
 
-/* logging macros */
-#define ite_pr(level, text, ...) \
-	printk(level KBUILD_MODNAME ": " text, ## __VA_ARGS__)
-#define ite_dbg(text, ...) do { \
-	if (debug) \
-		printk(KERN_DEBUG \
-			KBUILD_MODNAME ": " text "\n" , ## __VA_ARGS__); \
-} while (0)
-
-#define ite_dbg_verbose(text, ...) do {\
-	if (debug > 1) \
-		printk(KERN_DEBUG \
-			KBUILD_MODNAME ": " text "\n" , ## __VA_ARGS__); \
-} while (0)
-
 /* FIFO sizes */
 #define ITE_TX_FIFO_LEN 32
 #define ITE_RX_FIFO_LEN 32
-- 
2.29.2

[PATCH 4/6] media: ite-cir: carrier and duty cycle can be set via ioctl

[Sean Young]

The lirc ioctls should be used for this, not module parameters.

Signed-off-by: Sean Young <sean@mess.org>
---
 drivers/media/rc/ite-cir.c | 32 --------------------------------
 1 file changed, 32 deletions(-)

diff --git a/drivers/media/rc/ite-cir.c b/drivers/media/rc/ite-cir.c
index c5c7631849d0..748788a43da5 100644
--- a/drivers/media/rc/ite-cir.c
+++ b/drivers/media/rc/ite-cir.c
@@ -33,26 +33,6 @@
 
 /* module parameters */
 
-/* low limit for RX carrier freq, Hz, 0 for no RX demodulation */
-static int rx_low_carrier_freq;
-module_param(rx_low_carrier_freq, int, S_IRUGO | S_IWUSR);
-MODULE_PARM_DESC(rx_low_carrier_freq, "Override low RX carrier frequency, Hz, 0 for no RX demodulation");
-
-/* high limit for RX carrier freq, Hz, 0 for no RX demodulation */
-static int rx_high_carrier_freq;
-module_param(rx_high_carrier_freq, int, S_IRUGO | S_IWUSR);
-MODULE_PARM_DESC(rx_high_carrier_freq, "Override high RX carrier frequency, Hz, 0 for no RX demodulation");
-
-/* override tx carrier frequency */
-static int tx_carrier_freq;
-module_param(tx_carrier_freq, int, S_IRUGO | S_IWUSR);
-MODULE_PARM_DESC(tx_carrier_freq, "Override TX carrier frequency, Hz");
-
-/* override tx duty cycle */
-static int tx_duty_cycle;
-module_param(tx_duty_cycle, int, S_IRUGO | S_IWUSR);
-MODULE_PARM_DESC(tx_duty_cycle, "Override TX duty cycle, 1-100");
-
 /* override default sample period */
 static long sample_period;
 module_param(sample_period, long, S_IRUGO | S_IWUSR);
@@ -1410,18 +1390,6 @@ static int ite_probe(struct pnp_dev *pdev, const struct pnp_device_id
 	if (sample_period > 0)
 		itdev->params.sample_period = sample_period;
 
-	if (tx_carrier_freq > 0)
-		itdev->params.tx_carrier_freq = tx_carrier_freq;
-
-	if (tx_duty_cycle > 0 && tx_duty_cycle <= 100)
-		itdev->params.tx_duty_cycle = tx_duty_cycle;
-
-	if (rx_low_carrier_freq > 0)
-		itdev->params.rx_low_carrier_freq = rx_low_carrier_freq;
-
-	if (rx_high_carrier_freq > 0)
-		itdev->params.rx_high_carrier_freq = rx_high_carrier_freq;
-
 	/* set up hardware initial state */
 	itdev->params.init_hardware(itdev);
 
-- 
2.29.2

[PATCH 5/6] media: ite-cir: move runtime information into driver data

[Sean Young]

The carrier, duty cycle etc are not device dependent, and they can be
changed at runtime. By moving them into the allocated struct, we can make
the device specific structures smaller, and we no longer need to copy
the struct.

Signed-off-by: Sean Young <sean@mess.org>
---
 drivers/media/rc/ite-cir.c | 127 ++++++++++++++-----------------------
 drivers/media/rc/ite-cir.h |  32 ++++------
 2 files changed, 60 insertions(+), 99 deletions(-)

diff --git a/drivers/media/rc/ite-cir.c b/drivers/media/rc/ite-cir.c
index 748788a43da5..cc0a70dcf93d 100644
--- a/drivers/media/rc/ite-cir.c
+++ b/drivers/media/rc/ite-cir.c
@@ -33,10 +33,10 @@
 
 /* module parameters */
 
-/* override default sample period */
-static long sample_period;
+/* default sample period */
+static long sample_period = NSEC_PER_SEC / 115200;
 module_param(sample_period, long, S_IRUGO | S_IWUSR);
-MODULE_PARM_DESC(sample_period, "Override carrier sample period, us");
+MODULE_PARM_DESC(sample_period, "sample period");
 
 /* override detected model id */
 static int model_number = -1;
@@ -134,7 +134,6 @@ static u8 ite_get_pulse_width_bits(unsigned int freq, int duty_cycle)
 static void ite_decode_bytes(struct ite_dev *dev, const u8 * data, int
 			     length)
 {
-	u32 sample_period;
 	unsigned long *ldata;
 	unsigned int next_one, next_zero, size;
 	struct ir_raw_event ev = {};
@@ -142,7 +141,6 @@ static void ite_decode_bytes(struct ite_dev *dev, const u8 * data, int
 	if (length == 0)
 		return;
 
-	sample_period = dev->params.sample_period;
 	ldata = (unsigned long *)data;
 	size = length << 3;
 	next_one = find_next_bit_le(ldata, size, 0);
@@ -184,12 +182,12 @@ static void ite_set_carrier_params(struct ite_dev *dev)
 
 	if (for_tx) {
 		/* we don't need no stinking calculations */
-		freq = dev->params.tx_carrier_freq;
+		freq = dev->tx_carrier_freq;
 		allowance = ITE_RXDCR_DEFAULT;
 		use_demodulator = false;
 	} else {
-		low_freq = dev->params.rx_low_carrier_freq;
-		high_freq = dev->params.rx_high_carrier_freq;
+		low_freq = dev->rx_low_carrier_freq;
+		high_freq = dev->rx_high_carrier_freq;
 
 		if (low_freq == 0) {
 			/* don't demodulate */
@@ -217,9 +215,9 @@ static void ite_set_carrier_params(struct ite_dev *dev)
 	}
 
 	/* set the carrier parameters in a device-dependent way */
-	dev->params.set_carrier_params(dev, ite_is_high_carrier_freq(freq),
+	dev->params->set_carrier_params(dev, ite_is_high_carrier_freq(freq),
 		 use_demodulator, ite_get_carrier_freq_bits(freq), allowance,
-		 ite_get_pulse_width_bits(freq, dev->params.tx_duty_cycle));
+		 ite_get_pulse_width_bits(freq, dev->tx_duty_cycle));
 }
 
 /* interrupt service routine for incoming and outgoing CIR data */
@@ -236,12 +234,12 @@ static irqreturn_t ite_cir_isr(int irq, void *data)
 	spin_lock_irqsave(&dev->lock, flags);
 
 	/* read the interrupt flags */
-	iflags = dev->params.get_irq_causes(dev);
+	iflags = dev->params->get_irq_causes(dev);
 
 	/* check for the receive interrupt */
 	if (iflags & (ITE_IRQ_RX_FIFO | ITE_IRQ_RX_FIFO_OVERRUN)) {
 		/* read the FIFO bytes */
-		rx_bytes = dev->params.get_rx_bytes(dev, rx_buf,
+		rx_bytes = dev->params->get_rx_bytes(dev, rx_buf,
 						    ITE_RX_FIFO_LEN);
 
 		dev_dbg(&dev->rdev->dev, "interrupt %d RX bytes\n", rx_bytes);
@@ -286,8 +284,8 @@ static int ite_set_rx_carrier_range(struct rc_dev *rcdev, u32 carrier_low, u32
 	struct ite_dev *dev = rcdev->priv;
 
 	spin_lock_irqsave(&dev->lock, flags);
-	dev->params.rx_low_carrier_freq = carrier_low;
-	dev->params.rx_high_carrier_freq = carrier_high;
+	dev->rx_low_carrier_freq = carrier_low;
+	dev->rx_high_carrier_freq = carrier_high;
 	ite_set_carrier_params(dev);
 	spin_unlock_irqrestore(&dev->lock, flags);
 
@@ -301,7 +299,7 @@ static int ite_set_tx_carrier(struct rc_dev *rcdev, u32 carrier)
 	struct ite_dev *dev = rcdev->priv;
 
 	spin_lock_irqsave(&dev->lock, flags);
-	dev->params.tx_carrier_freq = carrier;
+	dev->tx_carrier_freq = carrier;
 	ite_set_carrier_params(dev);
 	spin_unlock_irqrestore(&dev->lock, flags);
 
@@ -315,7 +313,7 @@ static int ite_set_tx_duty_cycle(struct rc_dev *rcdev, u32 duty_cycle)
 	struct ite_dev *dev = rcdev->priv;
 
 	spin_lock_irqsave(&dev->lock, flags);
-	dev->params.tx_duty_cycle = duty_cycle;
+	dev->tx_duty_cycle = duty_cycle;
 	ite_set_carrier_params(dev);
 	spin_unlock_irqrestore(&dev->lock, flags);
 
@@ -349,17 +347,17 @@ static int ite_tx_ir(struct rc_dev *rcdev, unsigned *txbuf, unsigned n)
 
 	/* calculate how much time we can send in one byte */
 	max_rle_us =
-	    (ITE_BAUDRATE_DIVISOR * dev->params.sample_period *
+	    (ITE_BAUDRATE_DIVISOR * sample_period *
 	     ITE_TX_MAX_RLE) / 1000;
 
 	/* disable the receiver */
-	dev->params.disable_rx(dev);
+	dev->params->disable_rx(dev);
 
 	/* this is where we'll begin filling in the FIFO, until it's full.
 	 * then we'll just activate the interrupt, wait for it to wake us up
 	 * again, disable it, continue filling the FIFO... until everything
 	 * has been pushed out */
-	fifo_avail = ITE_TX_FIFO_LEN - dev->params.get_tx_used_slots(dev);
+	fifo_avail = ITE_TX_FIFO_LEN - dev->params->get_tx_used_slots(dev);
 
 	while (n > 0 && dev->in_use) {
 		/* transmit the next sample */
@@ -402,30 +400,29 @@ static int ite_tx_ir(struct rc_dev *rcdev, unsigned *txbuf, unsigned n)
 			 * some other slot got freed
 			 */
 			if (fifo_avail <= 0)
-				fifo_avail = ITE_TX_FIFO_LEN - dev->params.get_tx_used_slots(dev);
+				fifo_avail = ITE_TX_FIFO_LEN - dev->params->get_tx_used_slots(dev);
 
 			/* if it's still full */
 			if (fifo_avail <= 0) {
 				/* enable the tx interrupt */
-				dev->params.
-				enable_tx_interrupt(dev);
+				dev->params->enable_tx_interrupt(dev);
 
 				/* drop the spinlock */
 				spin_unlock_irqrestore(&dev->lock, flags);
 
 				/* wait for the FIFO to empty enough */
-				wait_event_interruptible(dev->tx_queue, (fifo_avail = ITE_TX_FIFO_LEN - dev->params.get_tx_used_slots(dev)) >= 8);
+				wait_event_interruptible(dev->tx_queue,
+					(fifo_avail = ITE_TX_FIFO_LEN - dev->params->get_tx_used_slots(dev)) >= 8);
 
 				/* get the spinlock again */
 				spin_lock_irqsave(&dev->lock, flags);
 
 				/* disable the tx interrupt again. */
-				dev->params.
-				disable_tx_interrupt(dev);
+				dev->params->disable_tx_interrupt(dev);
 			}
 
 			/* now send the byte through the FIFO */
-			dev->params.put_tx_byte(dev, val);
+			dev->params->put_tx_byte(dev, val);
 			fifo_avail--;
 		}
 	}
@@ -433,7 +430,7 @@ static int ite_tx_ir(struct rc_dev *rcdev, unsigned *txbuf, unsigned n)
 	/* wait and don't return until the whole FIFO has been sent out;
 	 * otherwise we could configure the RX carrier params instead of the
 	 * TX ones while the transmission is still being performed! */
-	fifo_remaining = dev->params.get_tx_used_slots(dev);
+	fifo_remaining = dev->params->get_tx_used_slots(dev);
 	remaining_us = 0;
 	while (fifo_remaining > 0) {
 		fifo_remaining--;
@@ -460,7 +457,7 @@ static int ite_tx_ir(struct rc_dev *rcdev, unsigned *txbuf, unsigned n)
 
 	/* re-enable the receiver */
 	if (dev->in_use)
-		dev->params.enable_rx(dev);
+		dev->params->enable_rx(dev);
 
 	/* notify transmission end */
 	wake_up_interruptible(&dev->tx_ended);
@@ -478,7 +475,7 @@ static void ite_s_idle(struct rc_dev *rcdev, bool enable)
 
 	if (enable) {
 		spin_lock_irqsave(&dev->lock, flags);
-		dev->params.idle_rx(dev);
+		dev->params->idle_rx(dev);
 		spin_unlock_irqrestore(&dev->lock, flags);
 	}
 }
@@ -1178,7 +1175,7 @@ static int ite_open(struct rc_dev *rcdev)
 	dev->in_use = true;
 
 	/* enable the receiver */
-	dev->params.enable_rx(dev);
+	dev->params->enable_rx(dev);
 
 	spin_unlock_irqrestore(&dev->lock, flags);
 
@@ -1199,7 +1196,7 @@ static void ite_close(struct rc_dev *rcdev)
 	wait_event_interruptible(dev->tx_ended, !dev->transmitting);
 	spin_lock_irqsave(&dev->lock, flags);
 
-	dev->params.disable(dev);
+	dev->params->disable(dev);
 
 	spin_unlock_irqrestore(&dev->lock, flags);
 }
@@ -1210,12 +1207,6 @@ static const struct ite_dev_params ite_dev_descs[] = {
 	       .model = "ITE8704 CIR transceiver",
 	       .io_region_size = IT87_IOREG_LENGTH,
 	       .io_rsrc_no = 0,
-	       .hw_tx_capable = true,
-	       .sample_period = (u32) (1000000000ULL / 115200),
-	       .tx_carrier_freq = 38000,
-	       .tx_duty_cycle = 33,
-	       .rx_low_carrier_freq = 0,
-	       .rx_high_carrier_freq = 0,
 
 		/* operations */
 	       .get_irq_causes = it87_get_irq_causes,
@@ -1235,12 +1226,6 @@ static const struct ite_dev_params ite_dev_descs[] = {
 	       .model = "ITE8713 CIR transceiver",
 	       .io_region_size = IT87_IOREG_LENGTH,
 	       .io_rsrc_no = 0,
-	       .hw_tx_capable = true,
-	       .sample_period = (u32) (1000000000ULL / 115200),
-	       .tx_carrier_freq = 38000,
-	       .tx_duty_cycle = 33,
-	       .rx_low_carrier_freq = 0,
-	       .rx_high_carrier_freq = 0,
 
 		/* operations */
 	       .get_irq_causes = it87_get_irq_causes,
@@ -1260,12 +1245,6 @@ static const struct ite_dev_params ite_dev_descs[] = {
 	       .model = "ITE8708 CIR transceiver",
 	       .io_region_size = IT8708_IOREG_LENGTH,
 	       .io_rsrc_no = 0,
-	       .hw_tx_capable = true,
-	       .sample_period = (u32) (1000000000ULL / 115200),
-	       .tx_carrier_freq = 38000,
-	       .tx_duty_cycle = 33,
-	       .rx_low_carrier_freq = 0,
-	       .rx_high_carrier_freq = 0,
 
 		/* operations */
 	       .get_irq_causes = it8708_get_irq_causes,
@@ -1286,12 +1265,6 @@ static const struct ite_dev_params ite_dev_descs[] = {
 	       .model = "ITE8709 CIR transceiver",
 	       .io_region_size = IT8709_IOREG_LENGTH,
 	       .io_rsrc_no = 2,
-	       .hw_tx_capable = true,
-	       .sample_period = (u32) (1000000000ULL / 115200),
-	       .tx_carrier_freq = 38000,
-	       .tx_duty_cycle = 33,
-	       .rx_low_carrier_freq = 0,
-	       .rx_high_carrier_freq = 0,
 
 		/* operations */
 	       .get_irq_causes = it8709_get_irq_causes,
@@ -1384,14 +1357,12 @@ static int ite_probe(struct pnp_dev *pdev, const struct pnp_device_id
 	init_waitqueue_head(&itdev->tx_ended);
 
 	/* copy model-specific parameters */
-	itdev->params = *dev_desc;
-
-	/* apply any overrides */
-	if (sample_period > 0)
-		itdev->params.sample_period = sample_period;
+	itdev->params = dev_desc;
 
 	/* set up hardware initial state */
-	itdev->params.init_hardware(itdev);
+	itdev->tx_duty_cycle = 33;
+	itdev->tx_carrier_freq = ITE_DEFAULT_CARRIER_FREQ;
+	itdev->params->init_hardware(itdev);
 
 	/* set up ir-core props */
 	rdev->priv = itdev;
@@ -1403,20 +1374,16 @@ static int ite_probe(struct pnp_dev *pdev, const struct pnp_device_id
 	rdev->s_rx_carrier_range = ite_set_rx_carrier_range;
 	/* FIFO threshold is 17 bytes, so 17 * 8 samples minimum */
 	rdev->min_timeout = 17 * 8 * ITE_BAUDRATE_DIVISOR *
-			    itdev->params.sample_period / 1000;
+			    sample_period / 1000;
 	rdev->timeout = IR_DEFAULT_TIMEOUT;
 	rdev->max_timeout = 10 * IR_DEFAULT_TIMEOUT;
-	rdev->rx_resolution = ITE_BAUDRATE_DIVISOR *
-				itdev->params.sample_period / 1000;
-	rdev->tx_resolution = ITE_BAUDRATE_DIVISOR *
-				itdev->params.sample_period / 1000;
-
-	/* set up transmitter related values if needed */
-	if (itdev->params.hw_tx_capable) {
-		rdev->tx_ir = ite_tx_ir;
-		rdev->s_tx_carrier = ite_set_tx_carrier;
-		rdev->s_tx_duty_cycle = ite_set_tx_duty_cycle;
-	}
+	rdev->rx_resolution = ITE_BAUDRATE_DIVISOR * sample_period / 1000;
+	rdev->tx_resolution = ITE_BAUDRATE_DIVISOR * sample_period / 1000;
+
+	/* set up transmitter related values */
+	rdev->tx_ir = ite_tx_ir;
+	rdev->s_tx_carrier = ite_set_tx_carrier;
+	rdev->s_tx_duty_cycle = ite_set_tx_duty_cycle;
 
 	rdev->device_name = dev_desc->model;
 	rdev->input_id.bustype = BUS_HOST;
@@ -1443,7 +1410,7 @@ static int ite_probe(struct pnp_dev *pdev, const struct pnp_device_id
 	return 0;
 
 exit_release_cir_addr:
-	release_region(itdev->cir_addr, itdev->params.io_region_size);
+	release_region(itdev->cir_addr, itdev->params->io_region_size);
 exit_unregister_device:
 	rc_unregister_device(rdev);
 	rdev = NULL;
@@ -1462,13 +1429,13 @@ static void ite_remove(struct pnp_dev *pdev)
 	spin_lock_irqsave(&dev->lock, flags);
 
 	/* disable hardware */
-	dev->params.disable(dev);
+	dev->params->disable(dev);
 
 	spin_unlock_irqrestore(&dev->lock, flags);
 
 	/* free resources */
 	free_irq(dev->cir_irq, dev);
-	release_region(dev->cir_addr, dev->params.io_region_size);
+	release_region(dev->cir_addr, dev->params->io_region_size);
 
 	rc_unregister_device(dev->rdev);
 
@@ -1486,7 +1453,7 @@ static int ite_suspend(struct pnp_dev *pdev, pm_message_t state)
 	spin_lock_irqsave(&dev->lock, flags);
 
 	/* disable all interrupts */
-	dev->params.disable(dev);
+	dev->params->disable(dev);
 
 	spin_unlock_irqrestore(&dev->lock, flags);
 
@@ -1501,9 +1468,9 @@ static int ite_resume(struct pnp_dev *pdev)
 	spin_lock_irqsave(&dev->lock, flags);
 
 	/* reinitialize hardware config registers */
-	dev->params.init_hardware(dev);
+	dev->params->init_hardware(dev);
 	/* enable the receiver */
-	dev->params.enable_rx(dev);
+	dev->params->enable_rx(dev);
 
 	spin_unlock_irqrestore(&dev->lock, flags);
 
@@ -1518,7 +1485,7 @@ static void ite_shutdown(struct pnp_dev *pdev)
 	spin_lock_irqsave(&dev->lock, flags);
 
 	/* disable all interrupts */
-	dev->params.disable(dev);
+	dev->params->disable(dev);
 
 	spin_unlock_irqrestore(&dev->lock, flags);
 }
diff --git a/drivers/media/rc/ite-cir.h b/drivers/media/rc/ite-cir.h
index 853001f67445..cb3dc4ca2b0a 100644
--- a/drivers/media/rc/ite-cir.h
+++ b/drivers/media/rc/ite-cir.h
@@ -31,24 +31,6 @@ struct ite_dev_params {
 	/* IR pnp I/O resource number */
 	int io_rsrc_no;
 
-	/* true if the hardware supports transmission */
-	bool hw_tx_capable;
-
-	/* base sampling period, in ns */
-	u32 sample_period;
-
-	/* rx low carrier frequency, in Hz, 0 means no demodulation */
-	unsigned int rx_low_carrier_freq;
-
-	/* tx high carrier frequency, in Hz, 0 means no demodulation */
-	unsigned int rx_high_carrier_freq;
-
-	/* tx carrier frequency, in Hz */
-	unsigned int tx_carrier_freq;
-
-	/* duty cycle, 0-100 */
-	int tx_duty_cycle;
-
 	/* hw-specific operation function pointers; most of these must be
 	 * called while holding the spin lock, except for the TX FIFO length
 	 * one */
@@ -104,12 +86,24 @@ struct ite_dev {
 	/* transmit support */
 	wait_queue_head_t tx_queue, tx_ended;
 
+	/* rx low carrier frequency, in Hz, 0 means no demodulation */
+	unsigned int rx_low_carrier_freq;
+
+	/* tx high carrier frequency, in Hz, 0 means no demodulation */
+	unsigned int rx_high_carrier_freq;
+
+	/* tx carrier frequency, in Hz */
+	unsigned int tx_carrier_freq;
+
+	/* duty cycle, 0-100 */
+	int tx_duty_cycle;
+
 	/* hardware I/O settings */
 	unsigned long cir_addr;
 	int cir_irq;
 
 	/* overridable copy of model parameters */
-	struct ite_dev_params params;
+	const struct ite_dev_params *params;
 };
 
 /* common values for all kinds of hardware */
-- 
2.29.2

[PATCH 6/6] media: ite-cir: check for receive overflow

[Sean Young]

It's best if this condition is reported.

Signed-off-by: Sean Young <sean@mess.org>
---
 drivers/media/rc/ite-cir.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/media/rc/ite-cir.c b/drivers/media/rc/ite-cir.c
index cc0a70dcf93d..9580e9e6e5c9 100644
--- a/drivers/media/rc/ite-cir.c
+++ b/drivers/media/rc/ite-cir.c
@@ -236,8 +236,14 @@ static irqreturn_t ite_cir_isr(int irq, void *data)
 	/* read the interrupt flags */
 	iflags = dev->params->get_irq_causes(dev);
 
+	/* Check for RX overflow */
+	if (iflags & ITE_IRQ_RX_FIFO_OVERRUN) {
+		dev_warn(&dev->rdev->dev, "receive overflow\n");
+		ir_raw_event_reset(dev->rdev);
+	}
+
 	/* check for the receive interrupt */
-	if (iflags & (ITE_IRQ_RX_FIFO | ITE_IRQ_RX_FIFO_OVERRUN)) {
+	if (iflags & ITE_IRQ_RX_FIFO) {
 		/* read the FIFO bytes */
 		rx_bytes = dev->params->get_rx_bytes(dev, rx_buf,
 						    ITE_RX_FIFO_LEN);
@@ -496,13 +502,13 @@ static int it87_get_irq_causes(struct ite_dev *dev)
 
 	switch (iflags) {
 	case IT87_II_RXDS:
-		ret = ITE_IRQ_RX_FIFO;
+		ret |= ITE_IRQ_RX_FIFO;
 		break;
 	case IT87_II_RXFO:
-		ret = ITE_IRQ_RX_FIFO_OVERRUN;
+		ret |= ITE_IRQ_RX_FIFO_OVERRUN;
 		break;
 	case IT87_II_TXLDL:
-		ret = ITE_IRQ_TX_FIFO;
+		ret |= ITE_IRQ_TX_FIFO;
 		break;
 	}
 
-- 
2.29.2

Re: [PATCH 0/6] media: ite-cir driver cleanups

[Matthias Reichl]

Hi Sean,

On Mon, Feb 22, 2021 at 10:43:23AM +0000, Sean Young wrote:
> This should not be any functional changes, just various cleanups.
> 
> Sean Young (6):
>   media: ite-cir: remove unused fields
>   media: ite-cir: set parent device
>   media: ite-cir: use standard logging and reduce noise
>   media: ite-cir: carrier and duty cycle can be set via ioctl
>   media: ite-cir: move runtime information into driver data
>   media: ite-cir: check for receive overflow
> 
>  drivers/media/rc/ite-cir.c | 341 +++++++++----------------------------
>  drivers/media/rc/ite-cir.h |  49 ++----
>  2 files changed, 91 insertions(+), 299 deletions(-)

I did a few tests with this series on top of 5.10.17 and so far
everything looks fine with ITE8713 CIR.

I could only test receiving (don't have a transmitter on that PC)
and also couldn't test the carrier options (I'm using a demodulating
TSOP receiver here), but your changes look sane to me.

so long,

Hias

Re: [PATCH 0/6] media: ite-cir driver cleanups

[Sean Young]

Hello Hias,

On Tue, Feb 23, 2021 at 05:13:07PM +0100, Matthias Reichl wrote:
> Hi Sean,
> 
> On Mon, Feb 22, 2021 at 10:43:23AM +0000, Sean Young wrote:
> > This should not be any functional changes, just various cleanups.
> > 
> > Sean Young (6):
> >   media: ite-cir: remove unused fields
> >   media: ite-cir: set parent device
> >   media: ite-cir: use standard logging and reduce noise
> >   media: ite-cir: carrier and duty cycle can be set via ioctl
> >   media: ite-cir: move runtime information into driver data
> >   media: ite-cir: check for receive overflow
> > 
> >  drivers/media/rc/ite-cir.c | 341 +++++++++----------------------------
> >  drivers/media/rc/ite-cir.h |  49 ++----
> >  2 files changed, 91 insertions(+), 299 deletions(-)
> 
> I did a few tests with this series on top of 5.10.17 and so far
> everything looks fine with ITE8713 CIR.

Great, thank you very much!

> I could only test receiving (don't have a transmitter on that PC)
> and also couldn't test the carrier options (I'm using a demodulating
> TSOP receiver here), but your changes look sane to me.

I only have got an eeepc box, with the same limitations. Do you happen to
know what hardware there is for this driver? It would be nice to test this
properly.

This is the only IR driver that can change the RX carrier range, so it's
kinda interesting.

Thanks!

Sean

Re: [PATCH 0/6] media: ite-cir driver cleanups

[Matthias Reichl]

Hi Sean,

On Tue, Feb 23, 2021 at 05:50:01PM +0000, Sean Young wrote:
> On Tue, Feb 23, 2021 at 05:13:07PM +0100, Matthias Reichl wrote:
> > I could only test receiving (don't have a transmitter on that PC)
> > and also couldn't test the carrier options (I'm using a demodulating
> > TSOP receiver here), but your changes look sane to me.
> 
> I only have got an eeepc box, with the same limitations. Do you happen to
> know what hardware there is for this driver? It would be nice to test this
> properly.
> 
> This is the only IR driver that can change the RX carrier range, so it's
> kinda interesting.

In the most recent bug report I got (about a month ago) the user was
using an intel NUC (NUC10i3FNH/NUC10i3FNB according to DMI):
https://forum.libreelec.tv/thread/23211-intel-nightly-build-problem-with-mce-remote/?postID=148823#post148823

I've also seen NUCs with Nuvoton CIR though and I suspect they'll
all probably have a demodulating receiver soldered directly to the
board, so probably not easy to tinker with (haven't checked though).

I have only an ancient Pentium 4 mainboard (Gigabyte 8IPE775) with
an IT8712 here. IR RX/TX and CIR RX/TX are all exposed on a header,
but the BIOS is somewhat odd and doesn't contain CIR on/off settings
or announce the CIR via ACPI. So I had to manually patch ACPI tables
to get that beast working.... ITE8713 pnpid was close enough (couldn't
find a datasheet for 8713, only 8712 - that does mention demodulating
though).

I don't think I have a simple IR diode or transistor here to easily
test that, only demodulating TSOPs (and some clones) - hooking that
up to the mainboard wouldn't be too hard.

so long,

Hias

Re: [PATCH 0/6] media: ite-cir driver cleanups

[Sean Young]

Hi Hias,

On Tue, Feb 23, 2021 at 08:16:15PM +0100, Matthias Reichl wrote:
> Hi Sean,
> 
> On Tue, Feb 23, 2021 at 05:50:01PM +0000, Sean Young wrote:
> > On Tue, Feb 23, 2021 at 05:13:07PM +0100, Matthias Reichl wrote:
> > > I could only test receiving (don't have a transmitter on that PC)
> > > and also couldn't test the carrier options (I'm using a demodulating
> > > TSOP receiver here), but your changes look sane to me.
> > 
> > I only have got an eeepc box, with the same limitations. Do you happen to
> > know what hardware there is for this driver? It would be nice to test this
> > properly.
> > 
> > This is the only IR driver that can change the RX carrier range, so it's
> > kinda interesting.
> 
> In the most recent bug report I got (about a month ago) the user was
> using an intel NUC (NUC10i3FNH/NUC10i3FNB according to DMI):
> https://forum.libreelec.tv/thread/23211-intel-nightly-build-problem-with-mce-remote/?postID=148823#post148823
> 
> I've also seen NUCs with Nuvoton CIR though and I suspect they'll
> all probably have a demodulating receiver soldered directly to the
> board, so probably not easy to tinker with (haven't checked though).
> 
> I have only an ancient Pentium 4 mainboard (Gigabyte 8IPE775) with
> an IT8712 here. IR RX/TX and CIR RX/TX are all exposed on a header,
> but the BIOS is somewhat odd and doesn't contain CIR on/off settings
> or announce the CIR via ACPI. So I had to manually patch ACPI tables
> to get that beast working.... ITE8713 pnpid was close enough (couldn't
> find a datasheet for 8713, only 8712 - that does mention demodulating
> though).
> 
> I don't think I have a simple IR diode or transistor here to easily
> test that, only demodulating TSOPs (and some clones) - hooking that
> up to the mainboard wouldn't be too hard.

That's a good point, that's probably the way to go. I'll have a look at
modifying my eeepc box.

Having said that, if all the hardware out there uses TSOP-type IR
receivers, I'm not sure how much use there is having this hooked up in
the driver; same for transmit.


Sean

Re: [PATCH 0/6] media: ite-cir driver cleanups

[Matthias Reichl]

Hi Sean,

On Wed, Feb 24, 2021 at 09:57:47AM +0000, Sean Young wrote:
> Hi Hias,
> 
> On Tue, Feb 23, 2021 at 08:16:15PM +0100, Matthias Reichl wrote:
> > Hi Sean,
> > 
> > On Tue, Feb 23, 2021 at 05:50:01PM +0000, Sean Young wrote:
> > > On Tue, Feb 23, 2021 at 05:13:07PM +0100, Matthias Reichl wrote:
> > > > I could only test receiving (don't have a transmitter on that PC)
> > > > and also couldn't test the carrier options (I'm using a demodulating
> > > > TSOP receiver here), but your changes look sane to me.
> > > 
> > > I only have got an eeepc box, with the same limitations. Do you happen to
> > > know what hardware there is for this driver? It would be nice to test this
> > > properly.
> > > 
> > > This is the only IR driver that can change the RX carrier range, so it's
> > > kinda interesting.
> > 
> > In the most recent bug report I got (about a month ago) the user was
> > using an intel NUC (NUC10i3FNH/NUC10i3FNB according to DMI):
> > https://forum.libreelec.tv/thread/23211-intel-nightly-build-problem-with-mce-remote/?postID=148823#post148823
> > 
> > I've also seen NUCs with Nuvoton CIR though and I suspect they'll
> > all probably have a demodulating receiver soldered directly to the
> > board, so probably not easy to tinker with (haven't checked though).
> > 
> > I have only an ancient Pentium 4 mainboard (Gigabyte 8IPE775) with
> > an IT8712 here. IR RX/TX and CIR RX/TX are all exposed on a header,
> > but the BIOS is somewhat odd and doesn't contain CIR on/off settings
> > or announce the CIR via ACPI. So I had to manually patch ACPI tables
> > to get that beast working.... ITE8713 pnpid was close enough (couldn't
> > find a datasheet for 8713, only 8712 - that does mention demodulating
> > though).
> > 
> > I don't think I have a simple IR diode or transistor here to easily
> > test that, only demodulating TSOPs (and some clones) - hooking that
> > up to the mainboard wouldn't be too hard.
> 
> That's a good point, that's probably the way to go. I'll have a look at
> modifying my eeepc box.

On a second thought it's probably a bit more involved than just
connecting an IR photo diode/transistor. CIRRX is a digital input
and to get acceptable performance we'd also need an AGC circuit
and a comparator / schmitt-trigger.

It might be easier to just feed a modulated digital signal into CIRRX,
either from CIRTX or from a RPi running pwm-ir-tx.

> Having said that, if all the hardware out there uses TSOP-type IR
> receivers, I'm not sure how much use there is having this hooked up in
> the driver; same for transmit.

If it's not too much hassle I'd vote for keeping the RX demodulation
feature in the driver - it's an interesting feature and might come in
handy at times.

For TX we need to keep carrier function as CIRTX always transmits
modulated singals according to the IT8712 datasheet (unmodulated TX
output would be quite odd, haven't seen that yet).

so long,

Hias

Re: [PATCH 0/6] media: ite-cir driver cleanups

[Sean Young]

Hi Hias,

On Fri, Feb 26, 2021 at 02:57:13PM +0100, Matthias Reichl wrote:
> On Wed, Feb 24, 2021 at 09:57:47AM +0000, Sean Young wrote:
> > That's a good point, that's probably the way to go. I'll have a look at
> > modifying my eeepc box.
> 
> On a second thought it's probably a bit more involved than just
> connecting an IR photo diode/transistor. CIRRX is a digital input
> and to get acceptable performance we'd also need an AGC circuit
> and a comparator / schmitt-trigger.

Yes, I had been thinking about building something like this.

> It might be easier to just feed a modulated digital signal into CIRRX,
> either from CIRTX or from a RPi running pwm-ir-tx.

That's a great idea, I hadn't thought of that. That's a lot easier!

> > Having said that, if all the hardware out there uses TSOP-type IR
> > receivers, I'm not sure how much use there is having this hooked up in
> > the driver; same for transmit.
> 
> If it's not too much hassle I'd vote for keeping the RX demodulation
> feature in the driver - it's an interesting feature and might come in
> handy at times.

It's kind of interesting, I agree. 

> For TX we need to keep carrier function as CIRTX always transmits
> modulated singals according to the IT8712 datasheet (unmodulated TX
> output would be quite odd, haven't seen that yet).

There are IR signals which are unmodulated. In IRP this would start
with {0k, and there are several of such protocols: Jerrold, Revox for
example:

http://www.hifi-remote.com/wiki/index.php/Revox

I never seen such hardware and I have no idea how well this works in
practise.


Sean