All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/7] Add support for IR transmitters
@ 2016-09-01 17:16 ` Andi Shyti
  0 siblings, 0 replies; 33+ messages in thread
From: Andi Shyti @ 2016-09-01 17:16 UTC (permalink / raw)
  To: Mauro Carvalho Chehab, Sean Young, Rob Herring, Mark Rutland
  Cc: linux-media, devicetree, linux-kernel, Andi Shyti, Andi Shyti

Hi,

The main goal is to add support in the rc framework for IR
transmitters, which currently is only supported by lirc but that
is not the preferred way.

The last patch adds support for an IR transmitter driven by
the MOSI line of an SPI controller, it's the case of the Samsung
TM2(e) board which support is currently ongoing.

Changelog from version 1:

The RFC is now PATCH. The main difference is that this version
doesn't try to add the any bit streaming protocol and doesn't
modify any LIRC interface specification.

patch 1: updates all the drivers using rc_allocate_device
patch 2: fixed errors and warning reported from the kbuild test
         robot
patch 5: this patch has been dropped and replaced with a new one
         which avoids waiting for transmitters.
patch 6: added new properties to the dts specification
patch 7: the driver uses the pulse/space input and converts it to
         a bit stream.

Thanks,
Andi

Andi Shyti (7):
  [media] rc-main: assign driver type during allocation
  [media] rc-main: split setup and unregister functions
  [media] rc-core: add support for IR raw transmitters
  [media] rc-ir-raw: do not generate any receiving thread for raw
    transmitters
  [media] ir-lirc-codec: don't wait any transmitting time for tx only
    devices
  Documentation: bindings: add documentation for ir-spi device driver
  [media] rc: add support for IR LEDs driven through SPI

 Documentation/devicetree/bindings/media/spi-ir.txt |  26 +++
 drivers/hid/hid-picolcd_cir.c                      |   3 +-
 drivers/media/common/siano/smsir.c                 |   3 +-
 drivers/media/i2c/ir-kbd-i2c.c                     |   2 +-
 drivers/media/pci/bt8xx/bttv-input.c               |   2 +-
 drivers/media/pci/cx23885/cx23885-input.c          |  11 +-
 drivers/media/pci/cx88/cx88-input.c                |   3 +-
 drivers/media/pci/dm1105/dm1105.c                  |   3 +-
 drivers/media/pci/mantis/mantis_input.c            |   2 +-
 drivers/media/pci/saa7134/saa7134-input.c          |   2 +-
 drivers/media/pci/smipcie/smipcie-ir.c             |   3 +-
 drivers/media/pci/ttpci/budget-ci.c                |   2 +-
 drivers/media/rc/Kconfig                           |   9 +
 drivers/media/rc/Makefile                          |   1 +
 drivers/media/rc/ati_remote.c                      |   3 +-
 drivers/media/rc/ene_ir.c                          |   3 +-
 drivers/media/rc/fintek-cir.c                      |   3 +-
 drivers/media/rc/gpio-ir-recv.c                    |   3 +-
 drivers/media/rc/igorplugusb.c                     |   3 +-
 drivers/media/rc/iguanair.c                        |   3 +-
 drivers/media/rc/img-ir/img-ir-hw.c                |   2 +-
 drivers/media/rc/img-ir/img-ir-raw.c               |   3 +-
 drivers/media/rc/imon.c                            |   3 +-
 drivers/media/rc/ir-hix5hd2.c                      |   3 +-
 drivers/media/rc/ir-lirc-codec.c                   |   2 +-
 drivers/media/rc/ir-spi.c                          | 221 +++++++++++++++++++++
 drivers/media/rc/ite-cir.c                         |   3 +-
 drivers/media/rc/mceusb.c                          |   3 +-
 drivers/media/rc/meson-ir.c                        |   3 +-
 drivers/media/rc/nuvoton-cir.c                     |   3 +-
 drivers/media/rc/rc-ir-raw.c                       |  17 +-
 drivers/media/rc/rc-loopback.c                     |   3 +-
 drivers/media/rc/rc-main.c                         | 179 ++++++++++-------
 drivers/media/rc/redrat3.c                         |   3 +-
 drivers/media/rc/st_rc.c                           |   3 +-
 drivers/media/rc/streamzap.c                       |   3 +-
 drivers/media/rc/sunxi-cir.c                       |   3 +-
 drivers/media/rc/ttusbir.c                         |   3 +-
 drivers/media/rc/winbond-cir.c                     |   3 +-
 drivers/media/usb/au0828/au0828-input.c            |   3 +-
 drivers/media/usb/cx231xx/cx231xx-input.c          |   2 +-
 drivers/media/usb/dvb-usb-v2/dvb_usb_core.c        |   3 +-
 drivers/media/usb/dvb-usb/dvb-usb-remote.c         |   3 +-
 drivers/media/usb/em28xx/em28xx-input.c            |   2 +-
 drivers/media/usb/tm6000/tm6000-input.c            |   3 +-
 drivers/staging/media/cec/cec-core.c               |   3 +-
 include/media/rc-core.h                            |  13 +-
 47 files changed, 421 insertions(+), 164 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/media/spi-ir.txt
 create mode 100644 drivers/media/rc/ir-spi.c

-- 
2.9.3

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

* [PATCH v2 0/7] Add support for IR transmitters
@ 2016-09-01 17:16 ` Andi Shyti
  0 siblings, 0 replies; 33+ messages in thread
From: Andi Shyti @ 2016-09-01 17:16 UTC (permalink / raw)
  To: Mauro Carvalho Chehab, Sean Young, Rob Herring, Mark Rutland
  Cc: linux-media-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Andi Shyti, Andi Shyti

Hi,

The main goal is to add support in the rc framework for IR
transmitters, which currently is only supported by lirc but that
is not the preferred way.

The last patch adds support for an IR transmitter driven by
the MOSI line of an SPI controller, it's the case of the Samsung
TM2(e) board which support is currently ongoing.

Changelog from version 1:

The RFC is now PATCH. The main difference is that this version
doesn't try to add the any bit streaming protocol and doesn't
modify any LIRC interface specification.

patch 1: updates all the drivers using rc_allocate_device
patch 2: fixed errors and warning reported from the kbuild test
         robot
patch 5: this patch has been dropped and replaced with a new one
         which avoids waiting for transmitters.
patch 6: added new properties to the dts specification
patch 7: the driver uses the pulse/space input and converts it to
         a bit stream.

Thanks,
Andi

Andi Shyti (7):
  [media] rc-main: assign driver type during allocation
  [media] rc-main: split setup and unregister functions
  [media] rc-core: add support for IR raw transmitters
  [media] rc-ir-raw: do not generate any receiving thread for raw
    transmitters
  [media] ir-lirc-codec: don't wait any transmitting time for tx only
    devices
  Documentation: bindings: add documentation for ir-spi device driver
  [media] rc: add support for IR LEDs driven through SPI

 Documentation/devicetree/bindings/media/spi-ir.txt |  26 +++
 drivers/hid/hid-picolcd_cir.c                      |   3 +-
 drivers/media/common/siano/smsir.c                 |   3 +-
 drivers/media/i2c/ir-kbd-i2c.c                     |   2 +-
 drivers/media/pci/bt8xx/bttv-input.c               |   2 +-
 drivers/media/pci/cx23885/cx23885-input.c          |  11 +-
 drivers/media/pci/cx88/cx88-input.c                |   3 +-
 drivers/media/pci/dm1105/dm1105.c                  |   3 +-
 drivers/media/pci/mantis/mantis_input.c            |   2 +-
 drivers/media/pci/saa7134/saa7134-input.c          |   2 +-
 drivers/media/pci/smipcie/smipcie-ir.c             |   3 +-
 drivers/media/pci/ttpci/budget-ci.c                |   2 +-
 drivers/media/rc/Kconfig                           |   9 +
 drivers/media/rc/Makefile                          |   1 +
 drivers/media/rc/ati_remote.c                      |   3 +-
 drivers/media/rc/ene_ir.c                          |   3 +-
 drivers/media/rc/fintek-cir.c                      |   3 +-
 drivers/media/rc/gpio-ir-recv.c                    |   3 +-
 drivers/media/rc/igorplugusb.c                     |   3 +-
 drivers/media/rc/iguanair.c                        |   3 +-
 drivers/media/rc/img-ir/img-ir-hw.c                |   2 +-
 drivers/media/rc/img-ir/img-ir-raw.c               |   3 +-
 drivers/media/rc/imon.c                            |   3 +-
 drivers/media/rc/ir-hix5hd2.c                      |   3 +-
 drivers/media/rc/ir-lirc-codec.c                   |   2 +-
 drivers/media/rc/ir-spi.c                          | 221 +++++++++++++++++++++
 drivers/media/rc/ite-cir.c                         |   3 +-
 drivers/media/rc/mceusb.c                          |   3 +-
 drivers/media/rc/meson-ir.c                        |   3 +-
 drivers/media/rc/nuvoton-cir.c                     |   3 +-
 drivers/media/rc/rc-ir-raw.c                       |  17 +-
 drivers/media/rc/rc-loopback.c                     |   3 +-
 drivers/media/rc/rc-main.c                         | 179 ++++++++++-------
 drivers/media/rc/redrat3.c                         |   3 +-
 drivers/media/rc/st_rc.c                           |   3 +-
 drivers/media/rc/streamzap.c                       |   3 +-
 drivers/media/rc/sunxi-cir.c                       |   3 +-
 drivers/media/rc/ttusbir.c                         |   3 +-
 drivers/media/rc/winbond-cir.c                     |   3 +-
 drivers/media/usb/au0828/au0828-input.c            |   3 +-
 drivers/media/usb/cx231xx/cx231xx-input.c          |   2 +-
 drivers/media/usb/dvb-usb-v2/dvb_usb_core.c        |   3 +-
 drivers/media/usb/dvb-usb/dvb-usb-remote.c         |   3 +-
 drivers/media/usb/em28xx/em28xx-input.c            |   2 +-
 drivers/media/usb/tm6000/tm6000-input.c            |   3 +-
 drivers/staging/media/cec/cec-core.c               |   3 +-
 include/media/rc-core.h                            |  13 +-
 47 files changed, 421 insertions(+), 164 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/media/spi-ir.txt
 create mode 100644 drivers/media/rc/ir-spi.c

-- 
2.9.3

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* [PATCH v2 1/7] [media] rc-main: assign driver type during allocation
  2016-09-01 17:16 ` Andi Shyti
  (?)
@ 2016-09-01 17:16 ` Andi Shyti
  2016-09-01 21:23   ` Sean Young
  -1 siblings, 1 reply; 33+ messages in thread
From: Andi Shyti @ 2016-09-01 17:16 UTC (permalink / raw)
  To: Mauro Carvalho Chehab, Sean Young, Rob Herring, Mark Rutland
  Cc: linux-media, devicetree, linux-kernel, Andi Shyti, Andi Shyti

The driver type can be assigned immediately when an RC device
requests to the framework to allocate the device.

This is an 'enum rc_driver_type' data type and specifies whether
the device is a raw receiver or scancode receiver. The type will
be given as parameter to the rc_allocate_device device.

Change accordingly all the drivers calling rc_allocate_device()
so that the device type is specified during the rc device
allocation. Whenever the device type is not specified, it will be
set as RC_DRIVER_SCANCODE which was the default '0' value.

Suggested-by: Sean Young <sean@mess.org>
Signed-off-by: Andi Shyti <andi.shyti@samsung.com>
---
 drivers/hid/hid-picolcd_cir.c               |  3 +--
 drivers/media/common/siano/smsir.c          |  3 +--
 drivers/media/i2c/ir-kbd-i2c.c              |  2 +-
 drivers/media/pci/bt8xx/bttv-input.c        |  2 +-
 drivers/media/pci/cx23885/cx23885-input.c   | 11 +----------
 drivers/media/pci/cx88/cx88-input.c         |  3 +--
 drivers/media/pci/dm1105/dm1105.c           |  3 +--
 drivers/media/pci/mantis/mantis_input.c     |  2 +-
 drivers/media/pci/saa7134/saa7134-input.c   |  2 +-
 drivers/media/pci/smipcie/smipcie-ir.c      |  3 +--
 drivers/media/pci/ttpci/budget-ci.c         |  2 +-
 drivers/media/rc/ati_remote.c               |  3 +--
 drivers/media/rc/ene_ir.c                   |  3 +--
 drivers/media/rc/fintek-cir.c               |  3 +--
 drivers/media/rc/gpio-ir-recv.c             |  3 +--
 drivers/media/rc/igorplugusb.c              |  3 +--
 drivers/media/rc/iguanair.c                 |  3 +--
 drivers/media/rc/img-ir/img-ir-hw.c         |  2 +-
 drivers/media/rc/img-ir/img-ir-raw.c        |  3 +--
 drivers/media/rc/imon.c                     |  3 +--
 drivers/media/rc/ir-hix5hd2.c               |  3 +--
 drivers/media/rc/ite-cir.c                  |  3 +--
 drivers/media/rc/mceusb.c                   |  3 +--
 drivers/media/rc/meson-ir.c                 |  3 +--
 drivers/media/rc/nuvoton-cir.c              |  3 +--
 drivers/media/rc/rc-loopback.c              |  3 +--
 drivers/media/rc/rc-main.c                  |  4 +++-
 drivers/media/rc/redrat3.c                  |  3 +--
 drivers/media/rc/st_rc.c                    |  3 +--
 drivers/media/rc/streamzap.c                |  3 +--
 drivers/media/rc/sunxi-cir.c                |  3 +--
 drivers/media/rc/ttusbir.c                  |  3 +--
 drivers/media/rc/winbond-cir.c              |  3 +--
 drivers/media/usb/au0828/au0828-input.c     |  3 +--
 drivers/media/usb/cx231xx/cx231xx-input.c   |  2 +-
 drivers/media/usb/dvb-usb-v2/dvb_usb_core.c |  3 +--
 drivers/media/usb/dvb-usb/dvb-usb-remote.c  |  3 +--
 drivers/media/usb/em28xx/em28xx-input.c     |  2 +-
 drivers/media/usb/tm6000/tm6000-input.c     |  3 +--
 drivers/staging/media/cec/cec-core.c        |  3 +--
 include/media/rc-core.h                     |  4 +++-
 41 files changed, 45 insertions(+), 80 deletions(-)

diff --git a/drivers/hid/hid-picolcd_cir.c b/drivers/hid/hid-picolcd_cir.c
index 9628651..38b0ea8 100644
--- a/drivers/hid/hid-picolcd_cir.c
+++ b/drivers/hid/hid-picolcd_cir.c
@@ -108,12 +108,11 @@ int picolcd_init_cir(struct picolcd_data *data, struct hid_report *report)
 	struct rc_dev *rdev;
 	int ret = 0;
 
-	rdev = rc_allocate_device();
+	rdev = rc_allocate_device(RC_DRIVER_IR_RAW);
 	if (!rdev)
 		return -ENOMEM;
 
 	rdev->priv             = data;
-	rdev->driver_type      = RC_DRIVER_IR_RAW;
 	rdev->allowed_protocols = RC_BIT_ALL;
 	rdev->open             = picolcd_cir_open;
 	rdev->close            = picolcd_cir_close;
diff --git a/drivers/media/common/siano/smsir.c b/drivers/media/common/siano/smsir.c
index 41f2a39..ee30c7b 100644
--- a/drivers/media/common/siano/smsir.c
+++ b/drivers/media/common/siano/smsir.c
@@ -58,7 +58,7 @@ int sms_ir_init(struct smscore_device_t *coredev)
 	struct rc_dev *dev;
 
 	pr_debug("Allocating rc device\n");
-	dev = rc_allocate_device();
+	dev = rc_allocate_device(RC_DRIVER_IR_RAW);
 	if (!dev)
 		return -ENOMEM;
 
@@ -86,7 +86,6 @@ int sms_ir_init(struct smscore_device_t *coredev)
 #endif
 
 	dev->priv = coredev;
-	dev->driver_type = RC_DRIVER_IR_RAW;
 	dev->allowed_protocols = RC_BIT_ALL;
 	dev->map_name = sms_get_board(board_id)->rc_codes;
 	dev->driver_name = MODULE_NAME;
diff --git a/drivers/media/i2c/ir-kbd-i2c.c b/drivers/media/i2c/ir-kbd-i2c.c
index bf82726..918fd7d 100644
--- a/drivers/media/i2c/ir-kbd-i2c.c
+++ b/drivers/media/i2c/ir-kbd-i2c.c
@@ -398,7 +398,7 @@ static int ir_probe(struct i2c_client *client, const struct i2c_device_id *id)
 		 * If platform_data doesn't specify rc_dev, initialize it
 		 * internally
 		 */
-		rc = rc_allocate_device();
+		rc = rc_allocate_device(RC_DRIVER_SCANCODE);
 		if (!rc)
 			return -ENOMEM;
 	}
diff --git a/drivers/media/pci/bt8xx/bttv-input.c b/drivers/media/pci/bt8xx/bttv-input.c
index a75c53d..6187b7b 100644
--- a/drivers/media/pci/bt8xx/bttv-input.c
+++ b/drivers/media/pci/bt8xx/bttv-input.c
@@ -424,7 +424,7 @@ int bttv_input_init(struct bttv *btv)
 		return -ENODEV;
 
 	ir = kzalloc(sizeof(*ir),GFP_KERNEL);
-	rc = rc_allocate_device();
+	rc = rc_allocate_device(RC_DRIVER_SCANCODE);
 	if (!ir || !rc)
 		goto err_out_free;
 
diff --git a/drivers/media/pci/cx23885/cx23885-input.c b/drivers/media/pci/cx23885/cx23885-input.c
index 64328d0..961ce9e 100644
--- a/drivers/media/pci/cx23885/cx23885-input.c
+++ b/drivers/media/pci/cx23885/cx23885-input.c
@@ -267,7 +267,6 @@ int cx23885_input_init(struct cx23885_dev *dev)
 	struct cx23885_kernel_ir *kernel_ir;
 	struct rc_dev *rc;
 	char *rc_map;
-	enum rc_driver_type driver_type;
 	u64 allowed_protos;
 
 	int ret;
@@ -285,28 +284,24 @@ int cx23885_input_init(struct cx23885_dev *dev)
 	case CX23885_BOARD_HAUPPAUGE_HVR1290:
 	case CX23885_BOARD_HAUPPAUGE_HVR1250:
 		/* Integrated CX2388[58] IR controller */
-		driver_type = RC_DRIVER_IR_RAW;
 		allowed_protos = RC_BIT_ALL;
 		/* The grey Hauppauge RC-5 remote */
 		rc_map = RC_MAP_HAUPPAUGE;
 		break;
 	case CX23885_BOARD_TERRATEC_CINERGY_T_PCIE_DUAL:
 		/* Integrated CX23885 IR controller */
-		driver_type = RC_DRIVER_IR_RAW;
 		allowed_protos = RC_BIT_NEC;
 		/* The grey Terratec remote with orange buttons */
 		rc_map = RC_MAP_NEC_TERRATEC_CINERGY_XS;
 		break;
 	case CX23885_BOARD_TEVII_S470:
 		/* Integrated CX23885 IR controller */
-		driver_type = RC_DRIVER_IR_RAW;
 		allowed_protos = RC_BIT_ALL;
 		/* A guess at the remote */
 		rc_map = RC_MAP_TEVII_NEC;
 		break;
 	case CX23885_BOARD_MYGICA_X8507:
 		/* Integrated CX23885 IR controller */
-		driver_type = RC_DRIVER_IR_RAW;
 		allowed_protos = RC_BIT_ALL;
 		/* A guess at the remote */
 		rc_map = RC_MAP_TOTAL_MEDIA_IN_HAND_02;
@@ -314,7 +309,6 @@ int cx23885_input_init(struct cx23885_dev *dev)
 	case CX23885_BOARD_TBS_6980:
 	case CX23885_BOARD_TBS_6981:
 		/* Integrated CX23885 IR controller */
-		driver_type = RC_DRIVER_IR_RAW;
 		allowed_protos = RC_BIT_ALL;
 		/* A guess at the remote */
 		rc_map = RC_MAP_TBS_NEC;
@@ -326,13 +320,11 @@ int cx23885_input_init(struct cx23885_dev *dev)
 	case CX23885_BOARD_DVBSKY_S952:
 	case CX23885_BOARD_DVBSKY_T982:
 		/* Integrated CX23885 IR controller */
-		driver_type = RC_DRIVER_IR_RAW;
 		allowed_protos = RC_BIT_ALL;
 		rc_map = RC_MAP_DVBSKY;
 		break;
 	case CX23885_BOARD_TT_CT2_4500_CI:
 		/* Integrated CX23885 IR controller */
-		driver_type = RC_DRIVER_IR_RAW;
 		allowed_protos = RC_BIT_ALL;
 		rc_map = RC_MAP_TT_1500;
 		break;
@@ -352,7 +344,7 @@ int cx23885_input_init(struct cx23885_dev *dev)
 				    pci_name(dev->pci));
 
 	/* input device */
-	rc = rc_allocate_device();
+	rc = rc_allocate_device(RC_DRIVER_IR_RAW);
 	if (!rc) {
 		ret = -ENOMEM;
 		goto err_out_free;
@@ -371,7 +363,6 @@ int cx23885_input_init(struct cx23885_dev *dev)
 		rc->input_id.product = dev->pci->device;
 	}
 	rc->dev.parent = &dev->pci->dev;
-	rc->driver_type = driver_type;
 	rc->allowed_protocols = allowed_protos;
 	rc->priv = kernel_ir;
 	rc->open = cx23885_input_ir_open;
diff --git a/drivers/media/pci/cx88/cx88-input.c b/drivers/media/pci/cx88/cx88-input.c
index 3f1342c..e52bf69 100644
--- a/drivers/media/pci/cx88/cx88-input.c
+++ b/drivers/media/pci/cx88/cx88-input.c
@@ -271,7 +271,7 @@ int cx88_ir_init(struct cx88_core *core, struct pci_dev *pci)
 				 */
 
 	ir = kzalloc(sizeof(*ir), GFP_KERNEL);
-	dev = rc_allocate_device();
+	dev = rc_allocate_device(RC_DRIVER_IR_RAW);
 	if (!ir || !dev)
 		goto err_out_free;
 
@@ -481,7 +481,6 @@ int cx88_ir_init(struct cx88_core *core, struct pci_dev *pci)
 	dev->scancode_mask = hardware_mask;
 
 	if (ir->sampling) {
-		dev->driver_type = RC_DRIVER_IR_RAW;
 		dev->timeout = 10 * 1000 * 1000; /* 10 ms */
 	} else {
 		dev->driver_type = RC_DRIVER_SCANCODE;
diff --git a/drivers/media/pci/dm1105/dm1105.c b/drivers/media/pci/dm1105/dm1105.c
index 5dd5047..5f9c6e6 100644
--- a/drivers/media/pci/dm1105/dm1105.c
+++ b/drivers/media/pci/dm1105/dm1105.c
@@ -744,7 +744,7 @@ static int dm1105_ir_init(struct dm1105_dev *dm1105)
 	struct rc_dev *dev;
 	int err = -ENOMEM;
 
-	dev = rc_allocate_device();
+	dev = rc_allocate_device(RC_DRIVER_SCANCODE);
 	if (!dev)
 		return -ENOMEM;
 
@@ -753,7 +753,6 @@ static int dm1105_ir_init(struct dm1105_dev *dm1105)
 
 	dev->driver_name = MODULE_NAME;
 	dev->map_name = RC_MAP_DM1105_NEC;
-	dev->driver_type = RC_DRIVER_SCANCODE;
 	dev->input_name = "DVB on-card IR receiver";
 	dev->input_phys = dm1105->ir.input_phys;
 	dev->input_id.bustype = BUS_PCI;
diff --git a/drivers/media/pci/mantis/mantis_input.c b/drivers/media/pci/mantis/mantis_input.c
index 7f7f1d4..50d10cb 100644
--- a/drivers/media/pci/mantis/mantis_input.c
+++ b/drivers/media/pci/mantis/mantis_input.c
@@ -39,7 +39,7 @@ int mantis_input_init(struct mantis_pci *mantis)
 	struct rc_dev *dev;
 	int err;
 
-	dev = rc_allocate_device();
+	dev = rc_allocate_device(RC_DRIVER_SCANCODE);
 	if (!dev) {
 		dprintk(MANTIS_ERROR, 1, "Remote device allocation failed");
 		err = -ENOMEM;
diff --git a/drivers/media/pci/saa7134/saa7134-input.c b/drivers/media/pci/saa7134/saa7134-input.c
index c8042c3..e9d4a47 100644
--- a/drivers/media/pci/saa7134/saa7134-input.c
+++ b/drivers/media/pci/saa7134/saa7134-input.c
@@ -849,7 +849,7 @@ int saa7134_input_init1(struct saa7134_dev *dev)
 	}
 
 	ir = kzalloc(sizeof(*ir), GFP_KERNEL);
-	rc = rc_allocate_device();
+	rc = rc_allocate_device(RC_DRIVER_SCANCODE);
 	if (!ir || !rc) {
 		err = -ENOMEM;
 		goto err_out_free;
diff --git a/drivers/media/pci/smipcie/smipcie-ir.c b/drivers/media/pci/smipcie/smipcie-ir.c
index 826c7c7..d2730c3 100644
--- a/drivers/media/pci/smipcie/smipcie-ir.c
+++ b/drivers/media/pci/smipcie/smipcie-ir.c
@@ -183,7 +183,7 @@ int smi_ir_init(struct smi_dev *dev)
 	struct rc_dev *rc_dev;
 	struct smi_rc *ir = &dev->ir;
 
-	rc_dev = rc_allocate_device();
+	rc_dev = rc_allocate_device(RC_DRIVER_SCANCODE);
 	if (!rc_dev)
 		return -ENOMEM;
 
@@ -202,7 +202,6 @@ int smi_ir_init(struct smi_dev *dev)
 	rc_dev->input_id.product = dev->pci_dev->subsystem_device;
 	rc_dev->dev.parent = &dev->pci_dev->dev;
 
-	rc_dev->driver_type = RC_DRIVER_SCANCODE;
 	rc_dev->map_name = dev->info->rc_map;
 
 	ir->rc_dev = rc_dev;
diff --git a/drivers/media/pci/ttpci/budget-ci.c b/drivers/media/pci/ttpci/budget-ci.c
index 7b27af4..5ced8ee 100644
--- a/drivers/media/pci/ttpci/budget-ci.c
+++ b/drivers/media/pci/ttpci/budget-ci.c
@@ -177,7 +177,7 @@ static int msp430_ir_init(struct budget_ci *budget_ci)
 	struct rc_dev *dev;
 	int error;
 
-	dev = rc_allocate_device();
+	dev = rc_allocate_device(RC_DRIVER_SCANCODE);
 	if (!dev) {
 		printk(KERN_ERR "budget_ci: IR interface initialisation failed\n");
 		return -ENOMEM;
diff --git a/drivers/media/rc/ati_remote.c b/drivers/media/rc/ati_remote.c
index 9f5b597..0b4c7df 100644
--- a/drivers/media/rc/ati_remote.c
+++ b/drivers/media/rc/ati_remote.c
@@ -765,7 +765,6 @@ static void ati_remote_rc_init(struct ati_remote *ati_remote)
 	struct rc_dev *rdev = ati_remote->rdev;
 
 	rdev->priv = ati_remote;
-	rdev->driver_type = RC_DRIVER_SCANCODE;
 	rdev->allowed_protocols = RC_BIT_OTHER;
 	rdev->driver_name = "ati_remote";
 
@@ -852,7 +851,7 @@ static int ati_remote_probe(struct usb_interface *interface,
 	}
 
 	ati_remote = kzalloc(sizeof (struct ati_remote), GFP_KERNEL);
-	rc_dev = rc_allocate_device();
+	rc_dev = rc_allocate_device(RC_DRIVER_SCANCODE);
 	if (!ati_remote || !rc_dev)
 		goto exit_free_dev_rdev;
 
diff --git a/drivers/media/rc/ene_ir.c b/drivers/media/rc/ene_ir.c
index d1c61cd..c62d097 100644
--- a/drivers/media/rc/ene_ir.c
+++ b/drivers/media/rc/ene_ir.c
@@ -1012,7 +1012,7 @@ static int ene_probe(struct pnp_dev *pnp_dev, const struct pnp_device_id *id)
 
 	/* allocate memory */
 	dev = kzalloc(sizeof(struct ene_device), GFP_KERNEL);
-	rdev = rc_allocate_device();
+	rdev = rc_allocate_device(RC_DRIVER_IR_RAW);
 	if (!dev || !rdev)
 		goto exit_free_dev_rdev;
 
@@ -1058,7 +1058,6 @@ static int ene_probe(struct pnp_dev *pnp_dev, const struct pnp_device_id *id)
 	if (!dev->hw_learning_and_tx_capable)
 		learning_mode_force = false;
 
-	rdev->driver_type = RC_DRIVER_IR_RAW;
 	rdev->allowed_protocols = RC_BIT_ALL;
 	rdev->priv = dev;
 	rdev->open = ene_open;
diff --git a/drivers/media/rc/fintek-cir.c b/drivers/media/rc/fintek-cir.c
index bd7b3bd..5bf572b 100644
--- a/drivers/media/rc/fintek-cir.c
+++ b/drivers/media/rc/fintek-cir.c
@@ -496,7 +496,7 @@ static int fintek_probe(struct pnp_dev *pdev, const struct pnp_device_id *dev_id
 		return ret;
 
 	/* input device for IR remote (and tx) */
-	rdev = rc_allocate_device();
+	rdev = rc_allocate_device(RC_DRIVER_IR_RAW);
 	if (!rdev)
 		goto exit_free_dev_rdev;
 
@@ -538,7 +538,6 @@ static int fintek_probe(struct pnp_dev *pdev, const struct pnp_device_id *dev_id
 
 	/* Set up the rc device */
 	rdev->priv = fintek;
-	rdev->driver_type = RC_DRIVER_IR_RAW;
 	rdev->allowed_protocols = RC_BIT_ALL;
 	rdev->open = fintek_open;
 	rdev->close = fintek_close;
diff --git a/drivers/media/rc/gpio-ir-recv.c b/drivers/media/rc/gpio-ir-recv.c
index 5b63b1f..d5d2152 100644
--- a/drivers/media/rc/gpio-ir-recv.c
+++ b/drivers/media/rc/gpio-ir-recv.c
@@ -143,14 +143,13 @@ static int gpio_ir_recv_probe(struct platform_device *pdev)
 	if (!gpio_dev)
 		return -ENOMEM;
 
-	rcdev = rc_allocate_device();
+	rcdev = rc_allocate_device(RC_DRIVER_IR_RAW);
 	if (!rcdev) {
 		rc = -ENOMEM;
 		goto err_allocate_device;
 	}
 
 	rcdev->priv = gpio_dev;
-	rcdev->driver_type = RC_DRIVER_IR_RAW;
 	rcdev->input_name = GPIO_IR_DEVICE_NAME;
 	rcdev->input_phys = GPIO_IR_DEVICE_NAME "/input0";
 	rcdev->input_id.bustype = BUS_HOST;
diff --git a/drivers/media/rc/igorplugusb.c b/drivers/media/rc/igorplugusb.c
index e0c531f..fb20246 100644
--- a/drivers/media/rc/igorplugusb.c
+++ b/drivers/media/rc/igorplugusb.c
@@ -190,7 +190,7 @@ static int igorplugusb_probe(struct usb_interface *intf,
 
 	usb_make_path(udev, ir->phys, sizeof(ir->phys));
 
-	rc = rc_allocate_device();
+	rc = rc_allocate_device(RC_DRIVER_IR_RAW);
 	if (!rc)
 		goto fail;
 
@@ -198,7 +198,6 @@ static int igorplugusb_probe(struct usb_interface *intf,
 	rc->input_phys = ir->phys;
 	usb_to_input_id(udev, &rc->input_id);
 	rc->dev.parent = &intf->dev;
-	rc->driver_type = RC_DRIVER_IR_RAW;
 	/*
 	 * This device can only store 36 pulses + spaces, which is not enough
 	 * for the NEC protocol and many others.
diff --git a/drivers/media/rc/iguanair.c b/drivers/media/rc/iguanair.c
index 5f63454..4cd1e6b 100644
--- a/drivers/media/rc/iguanair.c
+++ b/drivers/media/rc/iguanair.c
@@ -431,7 +431,7 @@ static int iguanair_probe(struct usb_interface *intf,
 	struct usb_host_interface *idesc;
 
 	ir = kzalloc(sizeof(*ir), GFP_KERNEL);
-	rc = rc_allocate_device();
+	rc = rc_allocate_device(RC_DRIVER_IR_RAW);
 	if (!ir || !rc) {
 		ret = -ENOMEM;
 		goto out;
@@ -494,7 +494,6 @@ static int iguanair_probe(struct usb_interface *intf,
 	rc->input_phys = ir->phys;
 	usb_to_input_id(ir->udev, &rc->input_id);
 	rc->dev.parent = &intf->dev;
-	rc->driver_type = RC_DRIVER_IR_RAW;
 	rc->allowed_protocols = RC_BIT_ALL;
 	rc->priv = ir;
 	rc->open = iguanair_open;
diff --git a/drivers/media/rc/img-ir/img-ir-hw.c b/drivers/media/rc/img-ir/img-ir-hw.c
index 7bb71bc..c87ae03 100644
--- a/drivers/media/rc/img-ir/img-ir-hw.c
+++ b/drivers/media/rc/img-ir/img-ir-hw.c
@@ -1071,7 +1071,7 @@ int img_ir_probe_hw(struct img_ir_priv *priv)
 	}
 
 	/* Allocate hardware decoder */
-	hw->rdev = rdev = rc_allocate_device();
+	hw->rdev = rdev = rc_allocate_device(RC_DRIVER_SCANCODE);
 	if (!rdev) {
 		dev_err(priv->dev, "cannot allocate input device\n");
 		error = -ENOMEM;
diff --git a/drivers/media/rc/img-ir/img-ir-raw.c b/drivers/media/rc/img-ir/img-ir-raw.c
index 33f37ed..8d2f8e2 100644
--- a/drivers/media/rc/img-ir/img-ir-raw.c
+++ b/drivers/media/rc/img-ir/img-ir-raw.c
@@ -110,7 +110,7 @@ int img_ir_probe_raw(struct img_ir_priv *priv)
 	setup_timer(&raw->timer, img_ir_echo_timer, (unsigned long)priv);
 
 	/* Allocate raw decoder */
-	raw->rdev = rdev = rc_allocate_device();
+	raw->rdev = rdev = rc_allocate_device(RC_DRIVER_IR_RAW);
 	if (!rdev) {
 		dev_err(priv->dev, "cannot allocate raw input device\n");
 		return -ENOMEM;
@@ -118,7 +118,6 @@ int img_ir_probe_raw(struct img_ir_priv *priv)
 	rdev->priv = priv;
 	rdev->map_name = RC_MAP_EMPTY;
 	rdev->input_name = "IMG Infrared Decoder Raw";
-	rdev->driver_type = RC_DRIVER_IR_RAW;
 
 	/* Register raw decoder */
 	error = rc_register_device(rdev);
diff --git a/drivers/media/rc/imon.c b/drivers/media/rc/imon.c
index 65f80b8..2c79708 100644
--- a/drivers/media/rc/imon.c
+++ b/drivers/media/rc/imon.c
@@ -1951,7 +1951,7 @@ static struct rc_dev *imon_init_rdev(struct imon_context *ictx)
 	const unsigned char fp_packet[] = { 0x40, 0x00, 0x00, 0x00,
 					    0x00, 0x00, 0x00, 0x88 };
 
-	rdev = rc_allocate_device();
+	rdev = rc_allocate_device(RC_DRIVER_SCANCODE);
 	if (!rdev) {
 		dev_err(ictx->dev, "remote control dev allocation failed\n");
 		goto out;
@@ -1969,7 +1969,6 @@ static struct rc_dev *imon_init_rdev(struct imon_context *ictx)
 	rdev->dev.parent = ictx->dev;
 
 	rdev->priv = ictx;
-	rdev->driver_type = RC_DRIVER_SCANCODE;
 	rdev->allowed_protocols = RC_BIT_OTHER | RC_BIT_RC6_MCE; /* iMON PAD or MCE */
 	rdev->change_protocol = imon_ir_change_protocol;
 	rdev->driver_name = MOD_NAME;
diff --git a/drivers/media/rc/ir-hix5hd2.c b/drivers/media/rc/ir-hix5hd2.c
index d0549fb..459bdbc 100644
--- a/drivers/media/rc/ir-hix5hd2.c
+++ b/drivers/media/rc/ir-hix5hd2.c
@@ -222,7 +222,7 @@ static int hix5hd2_ir_probe(struct platform_device *pdev)
 		return priv->irq;
 	}
 
-	rdev = rc_allocate_device();
+	rdev = rc_allocate_device(RC_DRIVER_IR_RAW);
 	if (!rdev)
 		return -ENOMEM;
 
@@ -235,7 +235,6 @@ static int hix5hd2_ir_probe(struct platform_device *pdev)
 	clk_prepare_enable(priv->clock);
 	priv->rate = clk_get_rate(priv->clock);
 
-	rdev->driver_type = RC_DRIVER_IR_RAW;
 	rdev->allowed_protocols = RC_BIT_ALL;
 	rdev->priv = priv;
 	rdev->open = hix5hd2_ir_open;
diff --git a/drivers/media/rc/ite-cir.c b/drivers/media/rc/ite-cir.c
index 0f30190..f58bf7f 100644
--- a/drivers/media/rc/ite-cir.c
+++ b/drivers/media/rc/ite-cir.c
@@ -1470,7 +1470,7 @@ static int ite_probe(struct pnp_dev *pdev, const struct pnp_device_id
 		return ret;
 
 	/* input device for IR remote (and tx) */
-	rdev = rc_allocate_device();
+	rdev = rc_allocate_device(RC_DRIVER_IR_RAW);
 	if (!rdev)
 		goto exit_free_dev_rdev;
 	itdev->rdev = rdev;
@@ -1562,7 +1562,6 @@ static int ite_probe(struct pnp_dev *pdev, const struct pnp_device_id
 
 	/* set up ir-core props */
 	rdev->priv = itdev;
-	rdev->driver_type = RC_DRIVER_IR_RAW;
 	rdev->allowed_protocols = RC_BIT_ALL;
 	rdev->open = ite_open;
 	rdev->close = ite_close;
diff --git a/drivers/media/rc/mceusb.c b/drivers/media/rc/mceusb.c
index 4f8c7ef..bac08f7 100644
--- a/drivers/media/rc/mceusb.c
+++ b/drivers/media/rc/mceusb.c
@@ -1220,7 +1220,7 @@ static struct rc_dev *mceusb_init_rc_dev(struct mceusb_dev *ir)
 	struct rc_dev *rc;
 	int ret;
 
-	rc = rc_allocate_device();
+	rc = rc_allocate_device(RC_DRIVER_IR_RAW);
 	if (!rc) {
 		dev_err(dev, "remote dev allocation failed");
 		goto out;
@@ -1240,7 +1240,6 @@ static struct rc_dev *mceusb_init_rc_dev(struct mceusb_dev *ir)
 	usb_to_input_id(ir->usbdev, &rc->input_id);
 	rc->dev.parent = dev;
 	rc->priv = ir;
-	rc->driver_type = RC_DRIVER_IR_RAW;
 	rc->allowed_protocols = RC_BIT_ALL;
 	rc->timeout = MS_TO_NS(100);
 	if (!ir->flags.no_tx) {
diff --git a/drivers/media/rc/meson-ir.c b/drivers/media/rc/meson-ir.c
index fcc3b82..2f53ab0 100644
--- a/drivers/media/rc/meson-ir.c
+++ b/drivers/media/rc/meson-ir.c
@@ -121,7 +121,7 @@ static int meson_ir_probe(struct platform_device *pdev)
 		return ir->irq;
 	}
 
-	ir->rc = rc_allocate_device();
+	ir->rc = rc_allocate_device(RC_DRIVER_IR_RAW);
 	if (!ir->rc) {
 		dev_err(dev, "failed to allocate rc device\n");
 		return -ENOMEM;
@@ -134,7 +134,6 @@ static int meson_ir_probe(struct platform_device *pdev)
 	map_name = of_get_property(node, "linux,rc-map-name", NULL);
 	ir->rc->map_name = map_name ? map_name : RC_MAP_EMPTY;
 	ir->rc->dev.parent = dev;
-	ir->rc->driver_type = RC_DRIVER_IR_RAW;
 	ir->rc->allowed_protocols = RC_BIT_ALL;
 	ir->rc->rx_resolution = US_TO_NS(MESON_TRATE);
 	ir->rc->timeout = MS_TO_NS(200);
diff --git a/drivers/media/rc/nuvoton-cir.c b/drivers/media/rc/nuvoton-cir.c
index 00215f3..fc2f49a 100644
--- a/drivers/media/rc/nuvoton-cir.c
+++ b/drivers/media/rc/nuvoton-cir.c
@@ -1021,7 +1021,7 @@ static int nvt_probe(struct pnp_dev *pdev, const struct pnp_device_id *dev_id)
 		return ret;
 
 	/* input device for IR remote (and tx) */
-	rdev = rc_allocate_device();
+	rdev = rc_allocate_device(RC_DRIVER_IR_RAW);
 	if (!rdev)
 		goto exit_free_dev_rdev;
 
@@ -1085,7 +1085,6 @@ static int nvt_probe(struct pnp_dev *pdev, const struct pnp_device_id *dev_id)
 
 	/* Set up the rc device */
 	rdev->priv = nvt;
-	rdev->driver_type = RC_DRIVER_IR_RAW;
 	rdev->allowed_protocols = RC_BIT_ALL;
 	rdev->open = nvt_open;
 	rdev->close = nvt_close;
diff --git a/drivers/media/rc/rc-loopback.c b/drivers/media/rc/rc-loopback.c
index 63dace8..36192ac 100644
--- a/drivers/media/rc/rc-loopback.c
+++ b/drivers/media/rc/rc-loopback.c
@@ -181,7 +181,7 @@ static int __init loop_init(void)
 	struct rc_dev *rc;
 	int ret;
 
-	rc = rc_allocate_device();
+	rc = rc_allocate_device(RC_DRIVER_IR_RAW);
 	if (!rc) {
 		printk(KERN_ERR DRIVER_NAME ": rc_dev allocation failed\n");
 		return -ENOMEM;
@@ -194,7 +194,6 @@ static int __init loop_init(void)
 	rc->driver_name		= DRIVER_NAME;
 	rc->map_name		= RC_MAP_EMPTY;
 	rc->priv		= &loopdev;
-	rc->driver_type		= RC_DRIVER_IR_RAW;
 	rc->allowed_protocols	= RC_BIT_ALL;
 	rc->timeout		= 100 * 1000 * 1000; /* 100 ms */
 	rc->min_timeout		= 1;
diff --git a/drivers/media/rc/rc-main.c b/drivers/media/rc/rc-main.c
index 8e7f292..b28a8d1 100644
--- a/drivers/media/rc/rc-main.c
+++ b/drivers/media/rc/rc-main.c
@@ -1353,7 +1353,7 @@ static struct device_type rc_dev_type = {
 	.uevent		= rc_dev_uevent,
 };
 
-struct rc_dev *rc_allocate_device(void)
+struct rc_dev *rc_allocate_device(enum rc_driver_type type)
 {
 	struct rc_dev *dev;
 
@@ -1380,6 +1380,8 @@ struct rc_dev *rc_allocate_device(void)
 	dev->dev.class = &rc_class;
 	device_initialize(&dev->dev);
 
+	dev->driver_type = type;
+
 	__module_get(THIS_MODULE);
 	return dev;
 }
diff --git a/drivers/media/rc/redrat3.c b/drivers/media/rc/redrat3.c
index 399f44d..dbfb2b8 100644
--- a/drivers/media/rc/redrat3.c
+++ b/drivers/media/rc/redrat3.c
@@ -867,7 +867,7 @@ static struct rc_dev *redrat3_init_rc_dev(struct redrat3_dev *rr3)
 	int ret = -ENODEV;
 	u16 prod = le16_to_cpu(rr3->udev->descriptor.idProduct);
 
-	rc = rc_allocate_device();
+	rc = rc_allocate_device(RC_DRIVER_IR_RAW);
 	if (!rc) {
 		dev_err(dev, "remote input dev allocation failed\n");
 		goto out;
@@ -885,7 +885,6 @@ static struct rc_dev *redrat3_init_rc_dev(struct redrat3_dev *rr3)
 	usb_to_input_id(rr3->udev, &rc->input_id);
 	rc->dev.parent = dev;
 	rc->priv = rr3;
-	rc->driver_type = RC_DRIVER_IR_RAW;
 	rc->allowed_protocols = RC_BIT_ALL;
 	rc->min_timeout = MS_TO_NS(RR3_RX_MIN_TIMEOUT);
 	rc->max_timeout = MS_TO_NS(RR3_RX_MAX_TIMEOUT);
diff --git a/drivers/media/rc/st_rc.c b/drivers/media/rc/st_rc.c
index 1fa0c9d..e6f6735 100644
--- a/drivers/media/rc/st_rc.c
+++ b/drivers/media/rc/st_rc.c
@@ -235,7 +235,7 @@ static int st_rc_probe(struct platform_device *pdev)
 	if (!rc_dev)
 		return -ENOMEM;
 
-	rdev = rc_allocate_device();
+	rdev = rc_allocate_device(RC_DRIVER_IR_RAW);
 
 	if (!rdev)
 		return -ENOMEM;
@@ -290,7 +290,6 @@ static int st_rc_probe(struct platform_device *pdev)
 	platform_set_drvdata(pdev, rc_dev);
 	st_rc_hardware_init(rc_dev);
 
-	rdev->driver_type = RC_DRIVER_IR_RAW;
 	rdev->allowed_protocols = RC_BIT_ALL;
 	/* rx sampling rate is 10Mhz */
 	rdev->rx_resolution = 100;
diff --git a/drivers/media/rc/streamzap.c b/drivers/media/rc/streamzap.c
index 815243c..227d1da 100644
--- a/drivers/media/rc/streamzap.c
+++ b/drivers/media/rc/streamzap.c
@@ -291,7 +291,7 @@ static struct rc_dev *streamzap_init_rc_dev(struct streamzap_ir *sz)
 	struct device *dev = sz->dev;
 	int ret;
 
-	rdev = rc_allocate_device();
+	rdev = rc_allocate_device(RC_DRIVER_IR_RAW);
 	if (!rdev) {
 		dev_err(dev, "remote dev allocation failed\n");
 		goto out;
@@ -309,7 +309,6 @@ static struct rc_dev *streamzap_init_rc_dev(struct streamzap_ir *sz)
 	usb_to_input_id(sz->usbdev, &rdev->input_id);
 	rdev->dev.parent = dev;
 	rdev->priv = sz;
-	rdev->driver_type = RC_DRIVER_IR_RAW;
 	rdev->allowed_protocols = RC_BIT_ALL;
 	rdev->driver_name = DRIVER_NAME;
 	rdev->map_name = RC_MAP_STREAMZAP;
diff --git a/drivers/media/rc/sunxi-cir.c b/drivers/media/rc/sunxi-cir.c
index eaadc08..5451f3d 100644
--- a/drivers/media/rc/sunxi-cir.c
+++ b/drivers/media/rc/sunxi-cir.c
@@ -212,7 +212,7 @@ static int sunxi_ir_probe(struct platform_device *pdev)
 		goto exit_clkdisable_clk;
 	}
 
-	ir->rc = rc_allocate_device();
+	ir->rc = rc_allocate_device(RC_DRIVER_IR_RAW);
 	if (!ir->rc) {
 		dev_err(dev, "failed to allocate device\n");
 		ret = -ENOMEM;
@@ -229,7 +229,6 @@ static int sunxi_ir_probe(struct platform_device *pdev)
 	ir->map_name = of_get_property(dn, "linux,rc-map-name", NULL);
 	ir->rc->map_name = ir->map_name ?: RC_MAP_EMPTY;
 	ir->rc->dev.parent = dev;
-	ir->rc->driver_type = RC_DRIVER_IR_RAW;
 	ir->rc->allowed_protocols = RC_BIT_ALL;
 	ir->rc->rx_resolution = SUNXI_IR_SAMPLE;
 	ir->rc->timeout = MS_TO_NS(SUNXI_IR_TIMEOUT);
diff --git a/drivers/media/rc/ttusbir.c b/drivers/media/rc/ttusbir.c
index bc214e2..6ff2cef 100644
--- a/drivers/media/rc/ttusbir.c
+++ b/drivers/media/rc/ttusbir.c
@@ -205,7 +205,7 @@ static int ttusbir_probe(struct usb_interface *intf,
 	int altsetting = -1;
 
 	tt = kzalloc(sizeof(*tt), GFP_KERNEL);
-	rc = rc_allocate_device();
+	rc = rc_allocate_device(RC_DRIVER_IR_RAW);
 	if (!tt || !rc) {
 		ret = -ENOMEM;
 		goto out;
@@ -317,7 +317,6 @@ static int ttusbir_probe(struct usb_interface *intf,
 	rc->input_phys = tt->phys;
 	usb_to_input_id(tt->udev, &rc->input_id);
 	rc->dev.parent = &intf->dev;
-	rc->driver_type = RC_DRIVER_IR_RAW;
 	rc->allowed_protocols = RC_BIT_ALL;
 	rc->priv = tt;
 	rc->driver_name = DRIVER_NAME;
diff --git a/drivers/media/rc/winbond-cir.c b/drivers/media/rc/winbond-cir.c
index 95ae60e..62d8225 100644
--- a/drivers/media/rc/winbond-cir.c
+++ b/drivers/media/rc/winbond-cir.c
@@ -1062,13 +1062,12 @@ wbcir_probe(struct pnp_dev *device, const struct pnp_device_id *dev_id)
 	if (err)
 		goto exit_free_data;
 
-	data->dev = rc_allocate_device();
+	data->dev = rc_allocate_device(RC_DRIVER_IR_RAW);
 	if (!data->dev) {
 		err = -ENOMEM;
 		goto exit_unregister_led;
 	}
 
-	data->dev->driver_type = RC_DRIVER_IR_RAW;
 	data->dev->driver_name = DRVNAME;
 	data->dev->input_name = WBCIR_NAME;
 	data->dev->input_phys = "wbcir/cir0";
diff --git a/drivers/media/usb/au0828/au0828-input.c b/drivers/media/usb/au0828/au0828-input.c
index 3d6687f..545741f 100644
--- a/drivers/media/usb/au0828/au0828-input.c
+++ b/drivers/media/usb/au0828/au0828-input.c
@@ -298,7 +298,7 @@ int au0828_rc_register(struct au0828_dev *dev)
 		return -ENODEV;
 
 	ir = kzalloc(sizeof(*ir), GFP_KERNEL);
-	rc = rc_allocate_device();
+	rc = rc_allocate_device(RC_DRIVER_IR_RAW);
 	if (!ir || !rc)
 		goto error;
 
@@ -343,7 +343,6 @@ int au0828_rc_register(struct au0828_dev *dev)
 	rc->input_id.product = le16_to_cpu(dev->usbdev->descriptor.idProduct);
 	rc->dev.parent = &dev->usbdev->dev;
 	rc->driver_name = "au0828-input";
-	rc->driver_type = RC_DRIVER_IR_RAW;
 	rc->allowed_protocols = RC_BIT_NEC | RC_BIT_RC5;
 
 	/* all done */
diff --git a/drivers/media/usb/cx231xx/cx231xx-input.c b/drivers/media/usb/cx231xx/cx231xx-input.c
index 15d8d1b..6e80f3c 100644
--- a/drivers/media/usb/cx231xx/cx231xx-input.c
+++ b/drivers/media/usb/cx231xx/cx231xx-input.c
@@ -72,7 +72,7 @@ int cx231xx_ir_init(struct cx231xx *dev)
 
 	memset(&info, 0, sizeof(struct i2c_board_info));
 	memset(&dev->init_data, 0, sizeof(dev->init_data));
-	dev->init_data.rc_dev = rc_allocate_device();
+	dev->init_data.rc_dev = rc_allocate_device(RC_DRIVER_SCANCODE);
 	if (!dev->init_data.rc_dev)
 		return -ENOMEM;
 
diff --git a/drivers/media/usb/dvb-usb-v2/dvb_usb_core.c b/drivers/media/usb/dvb-usb-v2/dvb_usb_core.c
index 3fbb2cd..eaa1919 100644
--- a/drivers/media/usb/dvb-usb-v2/dvb_usb_core.c
+++ b/drivers/media/usb/dvb-usb-v2/dvb_usb_core.c
@@ -149,7 +149,7 @@ static int dvb_usbv2_remote_init(struct dvb_usb_device *d)
 	if (!d->rc.map_name)
 		return 0;
 
-	dev = rc_allocate_device();
+	dev = rc_allocate_device(d->rc.driver_type);
 	if (!dev) {
 		ret = -ENOMEM;
 		goto err;
@@ -164,7 +164,6 @@ static int dvb_usbv2_remote_init(struct dvb_usb_device *d)
 	/* TODO: likely RC-core should took const char * */
 	dev->driver_name = (char *) d->props->driver_name;
 	dev->map_name = d->rc.map_name;
-	dev->driver_type = d->rc.driver_type;
 	dev->allowed_protocols = d->rc.allowed_protos;
 	dev->change_protocol = d->rc.change_protocol;
 	dev->priv = d;
diff --git a/drivers/media/usb/dvb-usb/dvb-usb-remote.c b/drivers/media/usb/dvb-usb/dvb-usb-remote.c
index c259f9e..059ded5 100644
--- a/drivers/media/usb/dvb-usb/dvb-usb-remote.c
+++ b/drivers/media/usb/dvb-usb/dvb-usb-remote.c
@@ -265,7 +265,7 @@ static int rc_core_dvb_usb_remote_init(struct dvb_usb_device *d)
 	int err, rc_interval;
 	struct rc_dev *dev;
 
-	dev = rc_allocate_device();
+	dev = rc_allocate_device(d->props.rc.core.driver_type);
 	if (!dev)
 		return -ENOMEM;
 
@@ -273,7 +273,6 @@ static int rc_core_dvb_usb_remote_init(struct dvb_usb_device *d)
 	dev->map_name = d->props.rc.core.rc_codes;
 	dev->change_protocol = d->props.rc.core.change_protocol;
 	dev->allowed_protocols = d->props.rc.core.allowed_protos;
-	dev->driver_type = d->props.rc.core.driver_type;
 	usb_to_input_id(d->udev, &dev->input_id);
 	dev->input_name = "IR-receiver inside an USB DVB receiver";
 	dev->input_phys = d->rc_phys;
diff --git a/drivers/media/usb/em28xx/em28xx-input.c b/drivers/media/usb/em28xx/em28xx-input.c
index 4007356..ca526d1 100644
--- a/drivers/media/usb/em28xx/em28xx-input.c
+++ b/drivers/media/usb/em28xx/em28xx-input.c
@@ -713,7 +713,7 @@ static int em28xx_ir_init(struct em28xx *dev)
 	ir = kzalloc(sizeof(*ir), GFP_KERNEL);
 	if (!ir)
 		return -ENOMEM;
-	rc = rc_allocate_device();
+	rc = rc_allocate_device(RC_DRIVER_SCANCODE);
 	if (!rc)
 		goto error;
 
diff --git a/drivers/media/usb/tm6000/tm6000-input.c b/drivers/media/usb/tm6000/tm6000-input.c
index 26b2ebb..377a69b 100644
--- a/drivers/media/usb/tm6000/tm6000-input.c
+++ b/drivers/media/usb/tm6000/tm6000-input.c
@@ -429,7 +429,7 @@ int tm6000_ir_init(struct tm6000_core *dev)
 		return 0;
 
 	ir = kzalloc(sizeof(*ir), GFP_ATOMIC);
-	rc = rc_allocate_device();
+	rc = rc_allocate_device(RC_DRIVER_SCANCODE);
 	if (!ir || !rc)
 		goto out;
 
@@ -456,7 +456,6 @@ int tm6000_ir_init(struct tm6000_core *dev)
 		ir->polling = 50;
 		INIT_DELAYED_WORK(&ir->work, tm6000_ir_handle_key);
 	}
-	rc->driver_type = RC_DRIVER_SCANCODE;
 
 	snprintf(ir->name, sizeof(ir->name), "tm5600/60x0 IR (%s)",
 						dev->name);
diff --git a/drivers/staging/media/cec/cec-core.c b/drivers/staging/media/cec/cec-core.c
index 112a5fa..19f2521 100644
--- a/drivers/staging/media/cec/cec-core.c
+++ b/drivers/staging/media/cec/cec-core.c
@@ -241,7 +241,7 @@ struct cec_adapter *cec_allocate_adapter(const struct cec_adap_ops *ops,
 
 #if IS_REACHABLE(CONFIG_RC_CORE)
 	/* Prepare the RC input device */
-	adap->rc = rc_allocate_device();
+	adap->rc = rc_allocate_device(RC_DRIVER_SCANCODE);
 	if (!adap->rc) {
 		pr_err("cec-%s: failed to allocate memory for rc_dev\n",
 		       name);
@@ -262,7 +262,6 @@ struct cec_adapter *cec_allocate_adapter(const struct cec_adap_ops *ops,
 	adap->rc->input_id.product = 0;
 	adap->rc->input_id.version = 1;
 	adap->rc->dev.parent = parent;
-	adap->rc->driver_type = RC_DRIVER_SCANCODE;
 	adap->rc->driver_name = CEC_NAME;
 	adap->rc->allowed_protocols = RC_BIT_CEC;
 	adap->rc->priv = adap;
diff --git a/include/media/rc-core.h b/include/media/rc-core.h
index 10908e3..4fc60dd 100644
--- a/include/media/rc-core.h
+++ b/include/media/rc-core.h
@@ -198,9 +198,11 @@ struct rc_dev {
 /**
  * rc_allocate_device - Allocates a RC device
  *
+ * @rc_driver_type: specifies the type of the RC output to be allocated
+ *
  * returns a pointer to struct rc_dev.
  */
-struct rc_dev *rc_allocate_device(void);
+struct rc_dev *rc_allocate_device(enum rc_driver_type);
 
 /**
  * rc_free_device - Frees a RC device
-- 
2.9.3

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

* [PATCH v2 2/7] [media] rc-main: split setup and unregister functions
  2016-09-01 17:16 ` Andi Shyti
  (?)
  (?)
@ 2016-09-01 17:16 ` Andi Shyti
  -1 siblings, 0 replies; 33+ messages in thread
From: Andi Shyti @ 2016-09-01 17:16 UTC (permalink / raw)
  To: Mauro Carvalho Chehab, Sean Young, Rob Herring, Mark Rutland
  Cc: linux-media, devicetree, linux-kernel, Andi Shyti, Andi Shyti

Move the input device allocation, map and protocol handling to
different functions.

Signed-off-by: Andi Shyti <andi.shyti@samsung.com>
---
 drivers/media/rc/rc-main.c | 144 +++++++++++++++++++++++++--------------------
 1 file changed, 81 insertions(+), 63 deletions(-)

diff --git a/drivers/media/rc/rc-main.c b/drivers/media/rc/rc-main.c
index b28a8d1..7961083 100644
--- a/drivers/media/rc/rc-main.c
+++ b/drivers/media/rc/rc-main.c
@@ -1403,16 +1403,12 @@ void rc_free_device(struct rc_dev *dev)
 }
 EXPORT_SYMBOL_GPL(rc_free_device);
 
-int rc_register_device(struct rc_dev *dev)
+static int rc_setup_rx_device(struct rc_dev *dev)
 {
-	static bool raw_init = false; /* raw decoders loaded? */
-	struct rc_map *rc_map;
-	const char *path;
-	int attr = 0;
-	int minor;
 	int rc;
+	struct rc_map *rc_map;
 
-	if (!dev || !dev->map_name)
+	if (!dev->map_name)
 		return -EINVAL;
 
 	rc_map = rc_map_get(dev->map_name);
@@ -1421,6 +1417,19 @@ int rc_register_device(struct rc_dev *dev)
 	if (!rc_map || !rc_map->scan || rc_map->size == 0)
 		return -EINVAL;
 
+	rc = ir_setkeytable(dev, rc_map);
+	if (rc)
+		return rc;
+
+	if (dev->change_protocol) {
+		u64 rc_type = (1ll << rc_map->rc_type);
+
+		rc = dev->change_protocol(dev, &rc_type);
+		if (rc < 0)
+			goto out_table;
+		dev->enabled_protocols = rc_type;
+	}
+
 	set_bit(EV_KEY, dev->input_dev->evbit);
 	set_bit(EV_REP, dev->input_dev->evbit);
 	set_bit(EV_MSC, dev->input_dev->evbit);
@@ -1430,6 +1439,61 @@ int rc_register_device(struct rc_dev *dev)
 	if (dev->close)
 		dev->input_dev->close = ir_close;
 
+	/*
+	 * Default delay of 250ms is too short for some protocols, especially
+	 * since the timeout is currently set to 250ms. Increase it to 500ms,
+	 * to avoid wrong repetition of the keycodes. Note that this must be
+	 * set after the call to input_register_device().
+	 */
+	dev->input_dev->rep[REP_DELAY] = 500;
+
+	/*
+	 * As a repeat event on protocols like RC-5 and NEC take as long as
+	 * 110/114ms, using 33ms as a repeat period is not the right thing
+	 * to do.
+	 */
+	dev->input_dev->rep[REP_PERIOD] = 125;
+
+	/* rc_open will be called here */
+	rc = input_register_device(dev->input_dev);
+	if (rc)
+		goto out_table;
+
+	dev->input_dev->dev.parent = &dev->dev;
+	memcpy(&dev->input_dev->id, &dev->input_id, sizeof(dev->input_id));
+	dev->input_dev->phys = dev->input_phys;
+	dev->input_dev->name = dev->input_name;
+
+	return 0;
+
+out_table:
+	ir_free_table(&dev->rc_map);
+
+	return rc;
+}
+
+static void rc_free_rx_device(struct rc_dev *dev)
+{
+	if (!dev)
+		return;
+
+	ir_free_table(&dev->rc_map);
+
+	input_unregister_device(dev->input_dev);
+	dev->input_dev = NULL;
+}
+
+int rc_register_device(struct rc_dev *dev)
+{
+	static bool raw_init = false; /* raw decoders loaded? */
+	const char *path;
+	int attr = 0;
+	int minor;
+	int rc;
+
+	if (!dev)
+		return -EINVAL;
+
 	minor = ida_simple_get(&rc_ida, 0, RC_DEV_MAX, GFP_KERNEL);
 	if (minor < 0)
 		return minor;
@@ -1453,40 +1517,15 @@ int rc_register_device(struct rc_dev *dev)
 	if (rc)
 		goto out_unlock;
 
-	rc = ir_setkeytable(dev, rc_map);
-	if (rc)
-		goto out_dev;
-
-	dev->input_dev->dev.parent = &dev->dev;
-	memcpy(&dev->input_dev->id, &dev->input_id, sizeof(dev->input_id));
-	dev->input_dev->phys = dev->input_phys;
-	dev->input_dev->name = dev->input_name;
-
-	/*
-	 * Default delay of 250ms is too short for some protocols, especially
-	 * since the timeout is currently set to 250ms. Increase it to 500ms,
-	 * to avoid wrong repetition of the keycodes. Note that this must be
-	 * set after the call to input_register_device().
-	 */
-	dev->input_dev->rep[REP_DELAY] = 500;
-
-	/*
-	 * As a repeat event on protocols like RC-5 and NEC take as long as
-	 * 110/114ms, using 33ms as a repeat period is not the right thing
-	 * to do.
-	 */
-	dev->input_dev->rep[REP_PERIOD] = 125;
-
-	/* rc_open will be called here */
-	rc = input_register_device(dev->input_dev);
-	if (rc)
-		goto out_table;
-
 	path = kobject_get_path(&dev->dev.kobj, GFP_KERNEL);
 	dev_info(&dev->dev, "%s as %s\n",
 		dev->input_name ?: "Unspecified device", path ?: "N/A");
 	kfree(path);
 
+	rc = rc_setup_rx_device(dev);
+	if (rc)
+		goto out_dev;
+
 	if (dev->driver_type == RC_DRIVER_IR_RAW) {
 		if (!raw_init) {
 			request_module_nowait("ir-lirc-codec");
@@ -1494,36 +1533,20 @@ int rc_register_device(struct rc_dev *dev)
 		}
 		rc = ir_raw_event_register(dev);
 		if (rc < 0)
-			goto out_input;
-	}
-
-	if (dev->change_protocol) {
-		u64 rc_type = (1ll << rc_map->rc_type);
-		rc = dev->change_protocol(dev, &rc_type);
-		if (rc < 0)
-			goto out_raw;
-		dev->enabled_protocols = rc_type;
+			goto out_rx;
 	}
 
 	/* Allow the RC sysfs nodes to be accessible */
 	atomic_set(&dev->initialized, 1);
 
-	IR_dprintk(1, "Registered rc%u (driver: %s, remote: %s, mode %s)\n",
+	IR_dprintk(1, "Registered rc%u (driver: %s)\n",
 		   dev->minor,
-		   dev->driver_name ? dev->driver_name : "unknown",
-		   rc_map->name ? rc_map->name : "unknown",
-		   dev->driver_type == RC_DRIVER_IR_RAW ? "raw" : "cooked");
+		   dev->driver_name ? dev->driver_name : "unknown");
 
 	return 0;
 
-out_raw:
-	if (dev->driver_type == RC_DRIVER_IR_RAW)
-		ir_raw_event_unregister(dev);
-out_input:
-	input_unregister_device(dev->input_dev);
-	dev->input_dev = NULL;
-out_table:
-	ir_free_table(&dev->rc_map);
+out_rx:
+	rc_free_rx_device(dev);
 out_dev:
 	device_del(&dev->dev);
 out_unlock:
@@ -1542,12 +1565,7 @@ void rc_unregister_device(struct rc_dev *dev)
 	if (dev->driver_type == RC_DRIVER_IR_RAW)
 		ir_raw_event_unregister(dev);
 
-	/* Freeing the table should also call the stop callback */
-	ir_free_table(&dev->rc_map);
-	IR_dprintk(1, "Freed keycode table\n");
-
-	input_unregister_device(dev->input_dev);
-	dev->input_dev = NULL;
+	rc_free_rx_device(dev);
 
 	device_del(&dev->dev);
 
-- 
2.9.3

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

* [PATCH v2 3/7] [media] rc-core: add support for IR raw transmitters
  2016-09-01 17:16 ` Andi Shyti
                   ` (2 preceding siblings ...)
  (?)
@ 2016-09-01 17:16 ` Andi Shyti
  2016-09-01 21:31   ` Sean Young
  2016-09-02  0:21     ` kbuild test robot
  -1 siblings, 2 replies; 33+ messages in thread
From: Andi Shyti @ 2016-09-01 17:16 UTC (permalink / raw)
  To: Mauro Carvalho Chehab, Sean Young, Rob Herring, Mark Rutland
  Cc: linux-media, devicetree, linux-kernel, Andi Shyti, Andi Shyti

IR raw transmitter driver type is specified in the enum
rc_driver_type as RC_DRIVER_IR_RAW_TX which includes all those
devices that transmit raw stream of bit to a receiver.

The data are provided by userspace applications, therefore they
don't need any input device allocation, but still they need to be
registered as raw devices.

Suggested-by: Sean Young <sean@mess.org>
Signed-off-by: Andi Shyti <andi.shyti@samsung.com>
---
 drivers/media/rc/rc-main.c | 39 +++++++++++++++++++++++----------------
 include/media/rc-core.h    |  9 ++++++---
 2 files changed, 29 insertions(+), 19 deletions(-)

diff --git a/drivers/media/rc/rc-main.c b/drivers/media/rc/rc-main.c
index 7961083..c3c1f68 100644
--- a/drivers/media/rc/rc-main.c
+++ b/drivers/media/rc/rc-main.c
@@ -1361,20 +1361,24 @@ struct rc_dev *rc_allocate_device(enum rc_driver_type type)
 	if (!dev)
 		return NULL;
 
-	dev->input_dev = input_allocate_device();
-	if (!dev->input_dev) {
-		kfree(dev);
-		return NULL;
-	}
+	if (type != RC_DRIVER_IR_RAW_TX) {
+		dev->input_dev = input_allocate_device();
+		if (!dev->input_dev) {
+			kfree(dev);
+			return NULL;
+		}
+
+		dev->input_dev->getkeycode = ir_getkeycode;
+		dev->input_dev->setkeycode = ir_setkeycode;
+		input_set_drvdata(dev->input_dev, dev);
 
-	dev->input_dev->getkeycode = ir_getkeycode;
-	dev->input_dev->setkeycode = ir_setkeycode;
-	input_set_drvdata(dev->input_dev, dev);
+		setup_timer(&dev->timer_keyup, ir_timer_keyup,
+						(unsigned long)dev);
 
-	spin_lock_init(&dev->rc_map.lock);
-	spin_lock_init(&dev->keylock);
+		spin_lock_init(&dev->rc_map.lock);
+		spin_lock_init(&dev->keylock);
+	}
 	mutex_init(&dev->lock);
-	setup_timer(&dev->timer_keyup, ir_timer_keyup, (unsigned long)dev);
 
 	dev->dev.type = &rc_dev_type;
 	dev->dev.class = &rc_class;
@@ -1474,7 +1478,7 @@ out_table:
 
 static void rc_free_rx_device(struct rc_dev *dev)
 {
-	if (!dev)
+	if (!dev || dev->driver_type == RC_DRIVER_IR_RAW_TX)
 		return;
 
 	ir_free_table(&dev->rc_map);
@@ -1522,11 +1526,14 @@ int rc_register_device(struct rc_dev *dev)
 		dev->input_name ?: "Unspecified device", path ?: "N/A");
 	kfree(path);
 
-	rc = rc_setup_rx_device(dev);
-	if (rc)
-		goto out_dev;
+	if (dev->driver_type != RC_DRIVER_IR_RAW_TX) {
+		rc = rc_setup_rx_device(dev);
+		if (rc)
+			goto out_dev;
+	}
 
-	if (dev->driver_type == RC_DRIVER_IR_RAW) {
+	if (dev->driver_type == RC_DRIVER_IR_RAW ||
+				dev->driver_type == RC_DRIVER_IR_RAW_TX) {
 		if (!raw_init) {
 			request_module_nowait("ir-lirc-codec");
 			raw_init = true;
diff --git a/include/media/rc-core.h b/include/media/rc-core.h
index 4fc60dd..56e33c1 100644
--- a/include/media/rc-core.h
+++ b/include/media/rc-core.h
@@ -32,13 +32,16 @@ do {								\
 /**
  * enum rc_driver_type - type of the RC output
  *
- * @RC_DRIVER_SCANCODE:	Driver or hardware generates a scancode
- * @RC_DRIVER_IR_RAW:	Driver or hardware generates pulse/space sequences.
- *			It needs a Infra-Red pulse/space decoder
+ * @RC_DRIVER_SCANCODE:	 Driver or hardware generates a scancode
+ * @RC_DRIVER_IR_RAW:	 Driver or hardware generates pulse/space sequences.
+ *			 It needs a Infra-Red pulse/space decoder
+ * @RC_DRIVER_IR_RAW_TX: Device transmitter only,
+			 driver requires pulce/spce data sequence.
  */
 enum rc_driver_type {
 	RC_DRIVER_SCANCODE = 0,
 	RC_DRIVER_IR_RAW,
+	RC_DRIVER_IR_RAW_TX,
 };
 
 /**
-- 
2.9.3

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

* [PATCH v2 4/7] [media] rc-ir-raw: do not generate any receiving thread for raw transmitters
  2016-09-01 17:16 ` Andi Shyti
                   ` (3 preceding siblings ...)
  (?)
@ 2016-09-01 17:16 ` Andi Shyti
  -1 siblings, 0 replies; 33+ messages in thread
From: Andi Shyti @ 2016-09-01 17:16 UTC (permalink / raw)
  To: Mauro Carvalho Chehab, Sean Young, Rob Herring, Mark Rutland
  Cc: linux-media, devicetree, linux-kernel, Andi Shyti, Andi Shyti

Raw IR transmitters do not need any thread listening for
occurring events. Check the driver type before running the
thread.

Signed-off-by: Andi Shyti <andi.shyti@samsung.com>
---
 drivers/media/rc/rc-ir-raw.c | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/drivers/media/rc/rc-ir-raw.c b/drivers/media/rc/rc-ir-raw.c
index 144304c..64ddc3d 100644
--- a/drivers/media/rc/rc-ir-raw.c
+++ b/drivers/media/rc/rc-ir-raw.c
@@ -274,12 +274,19 @@ int ir_raw_event_register(struct rc_dev *dev)
 	INIT_KFIFO(dev->raw->kfifo);
 
 	spin_lock_init(&dev->raw->lock);
-	dev->raw->thread = kthread_run(ir_raw_event_thread, dev->raw,
-				       "rc%u", dev->minor);
 
-	if (IS_ERR(dev->raw->thread)) {
-		rc = PTR_ERR(dev->raw->thread);
-		goto out;
+	/*
+	 * raw transmitters do not need any event registration
+	 * because the event is coming from userspace
+	 */
+	if (dev->driver_type != RC_DRIVER_IR_RAW_TX) {
+		dev->raw->thread = kthread_run(ir_raw_event_thread, dev->raw,
+					       "rc%u", dev->minor);
+
+		if (IS_ERR(dev->raw->thread)) {
+			rc = PTR_ERR(dev->raw->thread);
+			goto out;
+		}
 	}
 
 	mutex_lock(&ir_raw_handler_lock);
-- 
2.9.3

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

* [PATCH v2 5/7] [media] ir-lirc-codec: don't wait any transmitting time for tx only devices
  2016-09-01 17:16 ` Andi Shyti
                   ` (4 preceding siblings ...)
  (?)
@ 2016-09-01 17:16 ` Andi Shyti
  2016-09-02  8:41   ` Sean Young
  -1 siblings, 1 reply; 33+ messages in thread
From: Andi Shyti @ 2016-09-01 17:16 UTC (permalink / raw)
  To: Mauro Carvalho Chehab, Sean Young, Rob Herring, Mark Rutland
  Cc: linux-media, devicetree, linux-kernel, Andi Shyti, Andi Shyti

Transmitters do not need to wait until the data has been sent
(and of course received). Return before waiting.

Signed-off-by: Andi Shyti <andi.shyti@samsung.com>
---
 drivers/media/rc/ir-lirc-codec.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/rc/ir-lirc-codec.c b/drivers/media/rc/ir-lirc-codec.c
index c327730..d8953fb 100644
--- a/drivers/media/rc/ir-lirc-codec.c
+++ b/drivers/media/rc/ir-lirc-codec.c
@@ -153,7 +153,7 @@ static ssize_t ir_lirc_transmit_ir(struct file *file, const char __user *buf,
 	}
 
 	ret = dev->tx_ir(dev, txbuf, count);
-	if (ret < 0)
+	if (ret < 0 || dev->driver_type == RC_DRIVER_IR_RAW_TX)
 		goto out;
 
 	for (duration = i = 0; i < ret; i++)
-- 
2.9.3

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

* [PATCH v2 6/7] Documentation: bindings: add documentation for ir-spi device driver
  2016-09-01 17:16 ` Andi Shyti
                   ` (5 preceding siblings ...)
  (?)
@ 2016-09-01 17:16 ` Andi Shyti
  2016-09-01 21:40   ` Rob Herring
  -1 siblings, 1 reply; 33+ messages in thread
From: Andi Shyti @ 2016-09-01 17:16 UTC (permalink / raw)
  To: Mauro Carvalho Chehab, Sean Young, Rob Herring, Mark Rutland
  Cc: linux-media, devicetree, linux-kernel, Andi Shyti, Andi Shyti

Document the ir-spi driver's binding which is a IR led driven
through the SPI line.

Signed-off-by: Andi Shyti <andi.shyti@samsung.com>
---
 Documentation/devicetree/bindings/media/spi-ir.txt | 26 ++++++++++++++++++++++
 1 file changed, 26 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/spi-ir.txt

diff --git a/Documentation/devicetree/bindings/media/spi-ir.txt b/Documentation/devicetree/bindings/media/spi-ir.txt
new file mode 100644
index 0000000..85cb21b
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/spi-ir.txt
@@ -0,0 +1,26 @@
+Device tree bindings for IR LED connected through SPI bus which is used as
+remote controller.
+
+The IR LED switch is connected to the MOSI line of the SPI device and the data
+are delivered thourgh that.
+
+Required properties:
+	- compatible: should be "ir-spi".
+
+Optional properties:
+	- irled,switch: specifies the gpio switch which enables the irled/
+	- negated: boolean value that specifies whether the output is negated
+	  with a NOT gate.
+	- duty-cycle: 8 bit value that stores the percentage of the duty cycle.
+	  it can be 50, 60, 70, 75, 80 or 90.
+
+Example:
+
+        irled@0 {
+                compatible = "ir-spi";
+                reg = <0x0>;
+                spi-max-frequency = <5000000>;
+                irled,switch = <&gpr3 3 0>;
+		negated;
+		duty-cycle = /bits/ 8 <60>;
+        };
-- 
2.9.3

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

* [PATCH v2 7/7] [media] rc: add support for IR LEDs driven through SPI
  2016-09-01 17:16 ` Andi Shyti
                   ` (6 preceding siblings ...)
  (?)
@ 2016-09-01 17:16 ` Andi Shyti
  2016-09-01 21:12   ` Sean Young
  -1 siblings, 1 reply; 33+ messages in thread
From: Andi Shyti @ 2016-09-01 17:16 UTC (permalink / raw)
  To: Mauro Carvalho Chehab, Sean Young, Rob Herring, Mark Rutland
  Cc: linux-media, devicetree, linux-kernel, Andi Shyti, Andi Shyti

The ir-spi is a simple device driver which supports the
connection between an IR LED and the MOSI line of an SPI device.

The driver, indeed, uses the SPI framework to stream the raw data
provided by userspace through an rc character device. The chardev
is handled by the LIRC framework and its functionality basically
provides:

 - write: the driver gets a pulse/space signal and translates it
   to a binary signal that will be streamed to the IR led through
   the SPI framework.
 - set frequency: sets the frequency whith which the data should
   be sent. This is handle with ioctl with the
   LIRC_SET_SEND_CARRIER flag (as per lirc documentation)
 - set duty cycle: this is also handled with ioctl with the
   LIRC_SET_SEND_DUTY_CYCLE flag. The driver handles duty cycles
   of 50%, 60%, 70%, 75%, 80% and 90%, calculated on 16bit data.

The character device is created under /dev/lircX name, where X is
and ID assigned by the LIRC framework.

Example of usage:

        fd = open("/dev/lirc0", O_RDWR);
        if (fd < 0)
                return -1;

        val = 608000;
        ret = ioctl(fd, LIRC_SET_SEND_CARRIER, &val);
        if (ret < 0)
                return -1;

	val = 60;
        ret = ioctl(fd, LIRC_SET_SEND_DUTY_CYCLE, &val);
        if (ret < 0)
                return -1;

        n = write(fd, buffer, BUF_LEN);
        if (n < 0 || n != BUF_LEN)
                ret = -1;

        close(fd);

Signed-off-by: Andi Shyti <andi.shyti@samsung.com>
---
 drivers/media/rc/Kconfig  |   9 ++
 drivers/media/rc/Makefile |   1 +
 drivers/media/rc/ir-spi.c | 221 ++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 231 insertions(+)
 create mode 100644 drivers/media/rc/ir-spi.c

diff --git a/drivers/media/rc/Kconfig b/drivers/media/rc/Kconfig
index 370e16e..207dfcc 100644
--- a/drivers/media/rc/Kconfig
+++ b/drivers/media/rc/Kconfig
@@ -261,6 +261,15 @@ config IR_REDRAT3
 	   To compile this driver as a module, choose M here: the
 	   module will be called redrat3.
 
+config IR_SPI
+	tristate "SPI connected IR LED"
+	depends on SPI && LIRC
+	---help---
+	  Say Y if you want to use an IR LED connected through SPI bus.
+
+	  To compile this driver as a module, choose M here: the module will be
+	  called ir-spi.
+
 config IR_STREAMZAP
 	tristate "Streamzap PC Remote IR Receiver"
 	depends on USB_ARCH_HAS_HCD
diff --git a/drivers/media/rc/Makefile b/drivers/media/rc/Makefile
index 379a5c0..1417c8d 100644
--- a/drivers/media/rc/Makefile
+++ b/drivers/media/rc/Makefile
@@ -27,6 +27,7 @@ obj-$(CONFIG_IR_NUVOTON) += nuvoton-cir.o
 obj-$(CONFIG_IR_ENE) += ene_ir.o
 obj-$(CONFIG_IR_REDRAT3) += redrat3.o
 obj-$(CONFIG_IR_RX51) += ir-rx51.o
+obj-$(CONFIG_IR_SPI) += ir-spi.o
 obj-$(CONFIG_IR_STREAMZAP) += streamzap.o
 obj-$(CONFIG_IR_WINBOND_CIR) += winbond-cir.o
 obj-$(CONFIG_RC_LOOPBACK) += rc-loopback.o
diff --git a/drivers/media/rc/ir-spi.c b/drivers/media/rc/ir-spi.c
new file mode 100644
index 0000000..34d5a0c
--- /dev/null
+++ b/drivers/media/rc/ir-spi.c
@@ -0,0 +1,221 @@
+/*
+ * Copyright (c) 2016 Samsung Electronics Co., Ltd.
+ * Author: Andi Shyti <andi.shyti@samsung.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * SPI driven IR LED device driver
+ */
+
+#include <linux/delay.h>
+#include <linux/fs.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/of_gpio.h>
+#include <linux/regulator/consumer.h>
+#include <linux/spi/spi.h>
+#include <media/rc-core.h>
+
+#define IR_SPI_DRIVER_NAME		"ir-spi"
+
+/* pulse value for different duty cycles */
+#define IR_SPI_PULSE_DC_50		0xff00
+#define IR_SPI_PULSE_DC_60		0xfc00
+#define IR_SPI_PULSE_DC_70		0xf800
+#define IR_SPI_PULSE_DC_75		0xf000
+#define IR_SPI_PULSE_DC_80		0xc000
+#define IR_SPI_PULSE_DC_90		0x8000
+
+/* duty cycles values */
+#define IR_SPI_DUTY_CYCLE_50		50
+#define IR_SPI_DUTY_CYCLE_60		60
+#define IR_SPI_DUTY_CYCLE_70		70
+#define IR_SPI_DUTY_CYCLE_75		75
+#define IR_SPI_DUTY_CYCLE_80		80
+#define IR_SPI_DUTY_CYCLE_90		90
+
+#define IR_SPI_DEFAULT_FREQUENCY	38000
+#define IR_SPI_BIT_PER_WORD		    8
+#define IR_SPI_MAX_BUFSIZE		 4096
+
+struct ir_spi_data {
+	u32 freq;
+	u8 duty_cycle;
+	bool negated;
+
+	u16 tx_buf[IR_SPI_MAX_BUFSIZE];
+	u16 pulse;
+	u16 space;
+
+	struct rc_dev *rc;
+	struct spi_device *spi;
+	struct regulator *regulator;
+};
+
+static int ir_spi_tx(struct rc_dev *dev, unsigned int *buffer, unsigned int count)
+{
+	int i;
+	int ret;
+	unsigned int len = 0;
+	struct ir_spi_data *idata = dev->priv;
+	struct spi_transfer xfer;
+
+	/* convert the pulse/space signal to raw binary signal */
+	for (i = 0; i < count; i++) {
+		int j;
+		u16 val = ((i+1) % 2) ? idata->pulse : idata->space;
+
+		if (len + buffer[i] >= IR_SPI_MAX_BUFSIZE)
+			return -EINVAL;
+
+		/*
+		 * the first value in buffer is a pulse, so that 0, 2, 4, ...
+		 * contain a pulse duration. On the contrary, 1, 3, 5, ...
+		 * contain a space duration.
+		 */
+		val = (i % 2) ? idata->space : idata->pulse;
+		for (j = 0; j < buffer[i]; j++)
+			idata->tx_buf[len++] = val;
+	}
+
+	pr_info("from %u data, we originated %u raw data\n", count, len);
+
+	memset(&xfer, 0, sizeof(xfer));
+
+	xfer.speed_hz = idata->freq;
+	xfer.len = len * sizeof(*idata->tx_buf);
+	xfer.tx_buf = idata->tx_buf;
+
+	ret = regulator_enable(idata->regulator);
+	if (ret)
+		return ret;
+
+	ret = spi_sync_transfer(idata->spi, &xfer, 1);
+	if (ret)
+		dev_err(&idata->spi->dev, "unable to deliver the signal\n");
+
+	regulator_disable(idata->regulator);
+
+	return ret ? ret : len;
+}
+
+static int ir_spi_set_tx_carrier(struct rc_dev *dev, u32 carrier)
+{
+	struct ir_spi_data *idata = dev->priv;
+
+	if (!carrier)
+		return -EINVAL;
+
+	idata->freq = carrier;
+
+	return 0;
+}
+
+static int ir_spi_set_duty_cycle(struct rc_dev *dev, u32 duty_cycle)
+{
+	struct ir_spi_data *idata = dev->priv;
+
+	switch (duty_cycle) {
+	case IR_SPI_DUTY_CYCLE_90:
+		idata->pulse = IR_SPI_PULSE_DC_90;
+		break;
+	case IR_SPI_DUTY_CYCLE_80:
+		idata->pulse = IR_SPI_PULSE_DC_80;
+		break;
+	case IR_SPI_DUTY_CYCLE_75:
+		idata->pulse = IR_SPI_PULSE_DC_75;
+		break;
+	case IR_SPI_DUTY_CYCLE_70:
+		idata->pulse = IR_SPI_PULSE_DC_70;
+		break;
+	case IR_SPI_DUTY_CYCLE_60:
+		idata->pulse = IR_SPI_PULSE_DC_60;
+		break;
+	case IR_SPI_DUTY_CYCLE_50:
+	default:
+		idata->pulse = IR_SPI_PULSE_DC_50;
+	}
+
+	if (idata->negated) {
+		idata->pulse = ~idata->pulse;
+		idata->space = 0xffff;
+	} else {
+		idata->space = 0;
+	}
+
+	return 0;
+}
+
+static int ir_spi_probe(struct spi_device *spi)
+{
+	int ret;
+	u8 dc;
+	struct ir_spi_data *idata;
+
+	idata = devm_kzalloc(&spi->dev, sizeof(*idata), GFP_KERNEL);
+	if (!idata)
+		return -ENOMEM;
+
+	idata->regulator = devm_regulator_get(&spi->dev, "irda_regulator");
+	if (IS_ERR(idata->regulator))
+		return PTR_ERR(idata->regulator);
+
+	idata->rc = rc_allocate_device(RC_DRIVER_IR_RAW_TX);
+	if (!idata->rc)
+		return -ENOMEM;
+
+	idata->rc->tx_ir           = ir_spi_tx;
+	idata->rc->s_tx_carrier    = ir_spi_set_tx_carrier;
+	idata->rc->s_tx_duty_cycle = ir_spi_set_duty_cycle;
+	idata->rc->driver_name     = IR_SPI_DRIVER_NAME;
+	idata->rc->priv            = idata;
+	idata->spi                 = spi;
+
+	idata->negated = of_property_read_bool(spi->dev.of_node, "negated");
+	ret = of_property_read_u8(spi->dev.of_node, "duty-cycle", &dc);
+	if (ret)
+		dc = IR_SPI_DUTY_CYCLE_50;
+
+	ret = ir_spi_set_duty_cycle(idata->rc, dc);
+	if (ret)
+		return ret;
+
+	idata->freq = IR_SPI_DEFAULT_FREQUENCY;
+
+	ret = rc_register_device(idata->rc);
+	if (ret)
+		rc_unregister_device(idata->rc);
+
+	return ret;
+}
+
+static int ir_spi_remove(struct spi_device *spi)
+{
+	struct ir_spi_data *idata = spi_get_drvdata(spi);
+
+	rc_unregister_device(idata->rc);
+
+	return 0;
+}
+
+static const struct of_device_id ir_spi_of_match[] = {
+	{ .compatible = "ir-spi" },
+	{},
+};
+
+static struct spi_driver ir_spi_driver = {
+	.probe = ir_spi_probe,
+	.remove = ir_spi_remove,
+	.driver = {
+		.name = IR_SPI_DRIVER_NAME,
+		.of_match_table = ir_spi_of_match,
+	},
+};
+
+module_spi_driver(ir_spi_driver);
+
+MODULE_AUTHOR("Andi Shyti <andi.shyti@samsung.com>");
+MODULE_DESCRIPTION("SPI IR LED");
+MODULE_LICENSE("GPL v2");
-- 
2.9.3

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

* Re: [PATCH v2 7/7] [media] rc: add support for IR LEDs driven through SPI
  2016-09-01 17:16 ` [PATCH v2 7/7] [media] rc: add support for IR LEDs driven through SPI Andi Shyti
@ 2016-09-01 21:12   ` Sean Young
  2016-09-02  5:27       ` Andi Shyti
  0 siblings, 1 reply; 33+ messages in thread
From: Sean Young @ 2016-09-01 21:12 UTC (permalink / raw)
  To: Andi Shyti
  Cc: Mauro Carvalho Chehab, Rob Herring, Mark Rutland, linux-media,
	devicetree, linux-kernel, Andi Shyti

Thanks Andi, this is looking great!

On Fri, Sep 02, 2016 at 02:16:29AM +0900, Andi Shyti wrote:
> The ir-spi is a simple device driver which supports the
> connection between an IR LED and the MOSI line of an SPI device.
> 
> The driver, indeed, uses the SPI framework to stream the raw data
> provided by userspace through an rc character device. The chardev
> is handled by the LIRC framework and its functionality basically
> provides:
> 
>  - write: the driver gets a pulse/space signal and translates it
>    to a binary signal that will be streamed to the IR led through
>    the SPI framework.
>  - set frequency: sets the frequency whith which the data should
>    be sent. This is handle with ioctl with the
>    LIRC_SET_SEND_CARRIER flag (as per lirc documentation)
>  - set duty cycle: this is also handled with ioctl with the
>    LIRC_SET_SEND_DUTY_CYCLE flag. The driver handles duty cycles
>    of 50%, 60%, 70%, 75%, 80% and 90%, calculated on 16bit data.
> 
> The character device is created under /dev/lircX name, where X is
> and ID assigned by the LIRC framework.
> 
> Example of usage:
> 
>         fd = open("/dev/lirc0", O_RDWR);
>         if (fd < 0)
>                 return -1;
> 
>         val = 608000;
>         ret = ioctl(fd, LIRC_SET_SEND_CARRIER, &val);
>         if (ret < 0)
>                 return -1;
> 
> 	val = 60;
>         ret = ioctl(fd, LIRC_SET_SEND_DUTY_CYCLE, &val);
>         if (ret < 0)
>                 return -1;
> 
>         n = write(fd, buffer, BUF_LEN);
>         if (n < 0 || n != BUF_LEN)
>                 ret = -1;
> 
>         close(fd);
> 
> Signed-off-by: Andi Shyti <andi.shyti@samsung.com>
> ---
>  drivers/media/rc/Kconfig  |   9 ++
>  drivers/media/rc/Makefile |   1 +
>  drivers/media/rc/ir-spi.c | 221 ++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 231 insertions(+)
>  create mode 100644 drivers/media/rc/ir-spi.c
> 
> diff --git a/drivers/media/rc/Kconfig b/drivers/media/rc/Kconfig
> index 370e16e..207dfcc 100644
> --- a/drivers/media/rc/Kconfig
> +++ b/drivers/media/rc/Kconfig
> @@ -261,6 +261,15 @@ config IR_REDRAT3
>  	   To compile this driver as a module, choose M here: the
>  	   module will be called redrat3.
>  
> +config IR_SPI
> +	tristate "SPI connected IR LED"
> +	depends on SPI && LIRC
> +	---help---
> +	  Say Y if you want to use an IR LED connected through SPI bus.
> +
> +	  To compile this driver as a module, choose M here: the module will be
> +	  called ir-spi.
> +
>  config IR_STREAMZAP
>  	tristate "Streamzap PC Remote IR Receiver"
>  	depends on USB_ARCH_HAS_HCD
> diff --git a/drivers/media/rc/Makefile b/drivers/media/rc/Makefile
> index 379a5c0..1417c8d 100644
> --- a/drivers/media/rc/Makefile
> +++ b/drivers/media/rc/Makefile
> @@ -27,6 +27,7 @@ obj-$(CONFIG_IR_NUVOTON) += nuvoton-cir.o
>  obj-$(CONFIG_IR_ENE) += ene_ir.o
>  obj-$(CONFIG_IR_REDRAT3) += redrat3.o
>  obj-$(CONFIG_IR_RX51) += ir-rx51.o
> +obj-$(CONFIG_IR_SPI) += ir-spi.o
>  obj-$(CONFIG_IR_STREAMZAP) += streamzap.o
>  obj-$(CONFIG_IR_WINBOND_CIR) += winbond-cir.o
>  obj-$(CONFIG_RC_LOOPBACK) += rc-loopback.o
> diff --git a/drivers/media/rc/ir-spi.c b/drivers/media/rc/ir-spi.c
> new file mode 100644
> index 0000000..34d5a0c
> --- /dev/null
> +++ b/drivers/media/rc/ir-spi.c
> @@ -0,0 +1,221 @@
> +/*
> + * Copyright (c) 2016 Samsung Electronics Co., Ltd.
> + * Author: Andi Shyti <andi.shyti@samsung.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * SPI driven IR LED device driver
> + */
> +
> +#include <linux/delay.h>
> +#include <linux/fs.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/of_gpio.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/spi/spi.h>
> +#include <media/rc-core.h>
> +
> +#define IR_SPI_DRIVER_NAME		"ir-spi"
> +
> +/* pulse value for different duty cycles */
> +#define IR_SPI_PULSE_DC_50		0xff00
> +#define IR_SPI_PULSE_DC_60		0xfc00
> +#define IR_SPI_PULSE_DC_70		0xf800
> +#define IR_SPI_PULSE_DC_75		0xf000
> +#define IR_SPI_PULSE_DC_80		0xc000
> +#define IR_SPI_PULSE_DC_90		0x8000
> +
> +/* duty cycles values */
> +#define IR_SPI_DUTY_CYCLE_50		50
> +#define IR_SPI_DUTY_CYCLE_60		60
> +#define IR_SPI_DUTY_CYCLE_70		70
> +#define IR_SPI_DUTY_CYCLE_75		75
> +#define IR_SPI_DUTY_CYCLE_80		80
> +#define IR_SPI_DUTY_CYCLE_90		90

Not sure what these defines are for. 50 = 50?

> +
> +#define IR_SPI_DEFAULT_FREQUENCY	38000
> +#define IR_SPI_BIT_PER_WORD		    8
> +#define IR_SPI_MAX_BUFSIZE		 4096
> +
> +struct ir_spi_data {
> +	u32 freq;
> +	u8 duty_cycle;
> +	bool negated;
> +
> +	u16 tx_buf[IR_SPI_MAX_BUFSIZE];
> +	u16 pulse;
> +	u16 space;
> +
> +	struct rc_dev *rc;
> +	struct spi_device *spi;
> +	struct regulator *regulator;
> +};
> +
> +static int ir_spi_tx(struct rc_dev *dev, unsigned int *buffer, unsigned int count)
> +{
> +	int i;
> +	int ret;
> +	unsigned int len = 0;
> +	struct ir_spi_data *idata = dev->priv;
> +	struct spi_transfer xfer;
> +
> +	/* convert the pulse/space signal to raw binary signal */
> +	for (i = 0; i < count; i++) {
> +		int j;
> +		u16 val = ((i+1) % 2) ? idata->pulse : idata->space;
> +
> +		if (len + buffer[i] >= IR_SPI_MAX_BUFSIZE)
> +			return -EINVAL;
> +
> +		/*
> +		 * the first value in buffer is a pulse, so that 0, 2, 4, ...
> +		 * contain a pulse duration. On the contrary, 1, 3, 5, ...
> +		 * contain a space duration.
> +		 */
> +		val = (i % 2) ? idata->space : idata->pulse;
> +		for (j = 0; j < buffer[i]; j++)
> +			idata->tx_buf[len++] = val;
> +	}
> +
> +	pr_info("from %u data, we originated %u raw data\n", count, len);
> +
> +	memset(&xfer, 0, sizeof(xfer));
> +
> +	xfer.speed_hz = idata->freq;
> +	xfer.len = len * sizeof(*idata->tx_buf);
> +	xfer.tx_buf = idata->tx_buf;
> +
> +	ret = regulator_enable(idata->regulator);
> +	if (ret)
> +		return ret;
> +
> +	ret = spi_sync_transfer(idata->spi, &xfer, 1);
> +	if (ret)
> +		dev_err(&idata->spi->dev, "unable to deliver the signal\n");
> +
> +	regulator_disable(idata->regulator);
> +
> +	return ret ? ret : len;

On success you should be returning count, not the length of the spi
bitstream.

> +}
> +
> +static int ir_spi_set_tx_carrier(struct rc_dev *dev, u32 carrier)
> +{
> +	struct ir_spi_data *idata = dev->priv;
> +
> +	if (!carrier)
> +		return -EINVAL;
> +
> +	idata->freq = carrier;
> +
> +	return 0;
> +}
> +
> +static int ir_spi_set_duty_cycle(struct rc_dev *dev, u32 duty_cycle)
> +{
> +	struct ir_spi_data *idata = dev->priv;
> +
> +	switch (duty_cycle) {
> +	case IR_SPI_DUTY_CYCLE_90:
> +		idata->pulse = IR_SPI_PULSE_DC_90;
> +		break;
> +	case IR_SPI_DUTY_CYCLE_80:
> +		idata->pulse = IR_SPI_PULSE_DC_80;
> +		break;
> +	case IR_SPI_DUTY_CYCLE_75:
> +		idata->pulse = IR_SPI_PULSE_DC_75;
> +		break;
> +	case IR_SPI_DUTY_CYCLE_70:
> +		idata->pulse = IR_SPI_PULSE_DC_70;
> +		break;
> +	case IR_SPI_DUTY_CYCLE_60:
> +		idata->pulse = IR_SPI_PULSE_DC_60;
> +		break;
> +	case IR_SPI_DUTY_CYCLE_50:
> +	default:
> +		idata->pulse = IR_SPI_PULSE_DC_50;
> +	}

It seems a bit odd that if you set a duty cycle of (say) 61 get a duty 
cycle of 50. Maybe something like:

	if (duty_cycle >= 90)
		idata->pulse = IR_SPI_PULSE_DC_90;
	else if (duty_cycle >= 80)
		idata->pulse = IR_SPI_PULSE_DC_80;
	else if (duty_cycle >= 75)
		idata->pulse = IR_SPI_PULSE_DC_75;
	else if (duty_cycle >= 70)
		idata->pulse = IR_SPI_PULSE_DC_70;
	else if (duty_cycle >= 60)
		idata->pulse = IR_SPI_PULSE_DC_60;
	else
		idata->pulse = IR_SPI_PULSE_DC_50;

> +
> +	if (idata->negated) {
> +		idata->pulse = ~idata->pulse;
> +		idata->space = 0xffff;
> +	} else {
> +		idata->space = 0;
> +	}
> +
> +	return 0;
> +}
> +
> +static int ir_spi_probe(struct spi_device *spi)
> +{
> +	int ret;
> +	u8 dc;
> +	struct ir_spi_data *idata;
> +
> +	idata = devm_kzalloc(&spi->dev, sizeof(*idata), GFP_KERNEL);
> +	if (!idata)
> +		return -ENOMEM;
> +
> +	idata->regulator = devm_regulator_get(&spi->dev, "irda_regulator");
> +	if (IS_ERR(idata->regulator))
> +		return PTR_ERR(idata->regulator);
> +
> +	idata->rc = rc_allocate_device(RC_DRIVER_IR_RAW_TX);
> +	if (!idata->rc)
> +		return -ENOMEM;
> +
> +	idata->rc->tx_ir           = ir_spi_tx;
> +	idata->rc->s_tx_carrier    = ir_spi_set_tx_carrier;
> +	idata->rc->s_tx_duty_cycle = ir_spi_set_duty_cycle;
> +	idata->rc->driver_name     = IR_SPI_DRIVER_NAME;
> +	idata->rc->priv            = idata;
> +	idata->spi                 = spi;
> +
> +	idata->negated = of_property_read_bool(spi->dev.of_node, "negated");
> +	ret = of_property_read_u8(spi->dev.of_node, "duty-cycle", &dc);
> +	if (ret)
> +		dc = IR_SPI_DUTY_CYCLE_50;
> +
> +	ret = ir_spi_set_duty_cycle(idata->rc, dc);
> +	if (ret)
> +		return ret;
> +
> +	idata->freq = IR_SPI_DEFAULT_FREQUENCY;
> +
> +	ret = rc_register_device(idata->rc);
> +	if (ret)
> +		rc_unregister_device(idata->rc);

Surely you mean rc_free_device(idata->rc) here.

> +
> +	return ret;
> +}
> +
> +static int ir_spi_remove(struct spi_device *spi)
> +{
> +	struct ir_spi_data *idata = spi_get_drvdata(spi);
> +
> +	rc_unregister_device(idata->rc);
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id ir_spi_of_match[] = {
> +	{ .compatible = "ir-spi" },
> +	{},
> +};
> +
> +static struct spi_driver ir_spi_driver = {
> +	.probe = ir_spi_probe,
> +	.remove = ir_spi_remove,
> +	.driver = {
> +		.name = IR_SPI_DRIVER_NAME,
> +		.of_match_table = ir_spi_of_match,
> +	},
> +};
> +
> +module_spi_driver(ir_spi_driver);
> +
> +MODULE_AUTHOR("Andi Shyti <andi.shyti@samsung.com>");
> +MODULE_DESCRIPTION("SPI IR LED");
> +MODULE_LICENSE("GPL v2");
> -- 
> 2.9.3

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

* Re: [PATCH v2 1/7] [media] rc-main: assign driver type during allocation
  2016-09-01 17:16 ` [PATCH v2 1/7] [media] rc-main: assign driver type during allocation Andi Shyti
@ 2016-09-01 21:23   ` Sean Young
  2016-09-02  5:25       ` Andi Shyti
  0 siblings, 1 reply; 33+ messages in thread
From: Sean Young @ 2016-09-01 21:23 UTC (permalink / raw)
  To: Andi Shyti
  Cc: Mauro Carvalho Chehab, Rob Herring, Mark Rutland, linux-media,
	devicetree, linux-kernel, Andi Shyti

On Fri, Sep 02, 2016 at 02:16:23AM +0900, Andi Shyti wrote:
> The driver type can be assigned immediately when an RC device
> requests to the framework to allocate the device.
> 
> This is an 'enum rc_driver_type' data type and specifies whether
> the device is a raw receiver or scancode receiver. The type will
> be given as parameter to the rc_allocate_device device.
> 
> Change accordingly all the drivers calling rc_allocate_device()
> so that the device type is specified during the rc device
> allocation. Whenever the device type is not specified, it will be
> set as RC_DRIVER_SCANCODE which was the default '0' value.
> 
> Suggested-by: Sean Young <sean@mess.org>
> Signed-off-by: Andi Shyti <andi.shyti@samsung.com>
> ---

...

> diff --git a/drivers/media/pci/cx88/cx88-input.c b/drivers/media/pci/cx88/cx88-input.c
> index 3f1342c..e52bf69 100644
> --- a/drivers/media/pci/cx88/cx88-input.c
> +++ b/drivers/media/pci/cx88/cx88-input.c
> @@ -271,7 +271,7 @@ int cx88_ir_init(struct cx88_core *core, struct pci_dev *pci)
>  				 */
>  
>  	ir = kzalloc(sizeof(*ir), GFP_KERNEL);
> -	dev = rc_allocate_device();
> +	dev = rc_allocate_device(RC_DRIVER_IR_RAW);
>  	if (!ir || !dev)
>  		goto err_out_free;
>  

If ir->sampling = 0 then it should be RC_DRIVER_SCANCODE.


> @@ -481,7 +481,6 @@ int cx88_ir_init(struct cx88_core *core, struct pci_dev *pci)
>  	dev->scancode_mask = hardware_mask;
>  
>  	if (ir->sampling) {
> -		dev->driver_type = RC_DRIVER_IR_RAW;
>  		dev->timeout = 10 * 1000 * 1000; /* 10 ms */
>  	} else {
>  		dev->driver_type = RC_DRIVER_SCANCODE;

That assignment shouldn't really be there any more.


> diff --git a/drivers/media/pci/saa7134/saa7134-input.c b/drivers/media/pci/saa7134/saa7134-input.c
> index c8042c3..e9d4a47 100644
> --- a/drivers/media/pci/saa7134/saa7134-input.c
> +++ b/drivers/media/pci/saa7134/saa7134-input.c
> @@ -849,7 +849,7 @@ int saa7134_input_init1(struct saa7134_dev *dev)
>  	}
>  
>  	ir = kzalloc(sizeof(*ir), GFP_KERNEL);
> -	rc = rc_allocate_device();
> +	rc = rc_allocate_device(RC_DRIVER_SCANCODE);
>  	if (!ir || !rc) {
>  		err = -ENOMEM;
>  		goto err_out_free;

This is not correct, I'm afraid. If you look at the code you can see that
if raw_decode is true, then it should be RC_DRIVER_IR_RAW.


Sean

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

* Re: [PATCH v2 3/7] [media] rc-core: add support for IR raw transmitters
  2016-09-01 17:16 ` [PATCH v2 3/7] [media] rc-core: add support for IR raw transmitters Andi Shyti
@ 2016-09-01 21:31   ` Sean Young
  2016-09-02  0:21     ` kbuild test robot
  1 sibling, 0 replies; 33+ messages in thread
From: Sean Young @ 2016-09-01 21:31 UTC (permalink / raw)
  To: Andi Shyti
  Cc: Mauro Carvalho Chehab, Rob Herring, Mark Rutland, linux-media,
	devicetree, linux-kernel, Andi Shyti

On Fri, Sep 02, 2016 at 02:16:25AM +0900, Andi Shyti wrote:
> IR raw transmitter driver type is specified in the enum
> rc_driver_type as RC_DRIVER_IR_RAW_TX which includes all those
> devices that transmit raw stream of bit to a receiver.
> 
> The data are provided by userspace applications, therefore they
> don't need any input device allocation, but still they need to be
> registered as raw devices.
> 
> Suggested-by: Sean Young <sean@mess.org>
> Signed-off-by: Andi Shyti <andi.shyti@samsung.com>
> ---
>  drivers/media/rc/rc-main.c | 39 +++++++++++++++++++++++----------------
>  include/media/rc-core.h    |  9 ++++++---
>  2 files changed, 29 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/media/rc/rc-main.c b/drivers/media/rc/rc-main.c
> index 7961083..c3c1f68 100644
> --- a/drivers/media/rc/rc-main.c
> +++ b/drivers/media/rc/rc-main.c
> @@ -1361,20 +1361,24 @@ struct rc_dev *rc_allocate_device(enum rc_driver_type type)
>  	if (!dev)
>  		return NULL;
>  
> -	dev->input_dev = input_allocate_device();
> -	if (!dev->input_dev) {
> -		kfree(dev);
> -		return NULL;
> -	}
> +	if (type != RC_DRIVER_IR_RAW_TX) {
> +		dev->input_dev = input_allocate_device();
> +		if (!dev->input_dev) {
> +			kfree(dev);
> +			return NULL;
> +		}
> +
> +		dev->input_dev->getkeycode = ir_getkeycode;
> +		dev->input_dev->setkeycode = ir_setkeycode;
> +		input_set_drvdata(dev->input_dev, dev);
>  
> -	dev->input_dev->getkeycode = ir_getkeycode;
> -	dev->input_dev->setkeycode = ir_setkeycode;
> -	input_set_drvdata(dev->input_dev, dev);
> +		setup_timer(&dev->timer_keyup, ir_timer_keyup,
> +						(unsigned long)dev);
>  
> -	spin_lock_init(&dev->rc_map.lock);
> -	spin_lock_init(&dev->keylock);
> +		spin_lock_init(&dev->rc_map.lock);
> +		spin_lock_init(&dev->keylock);
> +	}
>  	mutex_init(&dev->lock);
> -	setup_timer(&dev->timer_keyup, ir_timer_keyup, (unsigned long)dev);
>  
>  	dev->dev.type = &rc_dev_type;
>  	dev->dev.class = &rc_class;
> @@ -1474,7 +1478,7 @@ out_table:
>  
>  static void rc_free_rx_device(struct rc_dev *dev)
>  {
> -	if (!dev)
> +	if (!dev || dev->driver_type == RC_DRIVER_IR_RAW_TX)
>  		return;
>  
>  	ir_free_table(&dev->rc_map);
> @@ -1522,11 +1526,14 @@ int rc_register_device(struct rc_dev *dev)

An tx-only device shouldn't have the sysfs attribute protocol, that
should be handled here too.

>  		dev->input_name ?: "Unspecified device", path ?: "N/A");
>  	kfree(path);
>  
> -	rc = rc_setup_rx_device(dev);
> -	if (rc)
> -		goto out_dev;
> +	if (dev->driver_type != RC_DRIVER_IR_RAW_TX) {
> +		rc = rc_setup_rx_device(dev);
> +		if (rc)
> +			goto out_dev;
> +	}
>  
> -	if (dev->driver_type == RC_DRIVER_IR_RAW) {
> +	if (dev->driver_type == RC_DRIVER_IR_RAW ||
> +				dev->driver_type == RC_DRIVER_IR_RAW_TX) {
>  		if (!raw_init) {
>  			request_module_nowait("ir-lirc-codec");
>  			raw_init = true;
> diff --git a/include/media/rc-core.h b/include/media/rc-core.h
> index 4fc60dd..56e33c1 100644
> --- a/include/media/rc-core.h
> +++ b/include/media/rc-core.h
> @@ -32,13 +32,16 @@ do {								\
>  /**
>   * enum rc_driver_type - type of the RC output
>   *
> - * @RC_DRIVER_SCANCODE:	Driver or hardware generates a scancode
> - * @RC_DRIVER_IR_RAW:	Driver or hardware generates pulse/space sequences.
> - *			It needs a Infra-Red pulse/space decoder
> + * @RC_DRIVER_SCANCODE:	 Driver or hardware generates a scancode
> + * @RC_DRIVER_IR_RAW:	 Driver or hardware generates pulse/space sequences.
> + *			 It needs a Infra-Red pulse/space decoder
> + * @RC_DRIVER_IR_RAW_TX: Device transmitter only,
> +			 driver requires pulce/spce data sequence.
>   */
>  enum rc_driver_type {
>  	RC_DRIVER_SCANCODE = 0,
>  	RC_DRIVER_IR_RAW,
> +	RC_DRIVER_IR_RAW_TX,
>  };
>  
>  /**
> -- 
> 2.9.3

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

* Re: [PATCH v2 6/7] Documentation: bindings: add documentation for ir-spi device driver
  2016-09-01 17:16 ` [PATCH v2 6/7] Documentation: bindings: add documentation for ir-spi device driver Andi Shyti
@ 2016-09-01 21:40   ` Rob Herring
  2016-09-02  5:33     ` Andi Shyti
  0 siblings, 1 reply; 33+ messages in thread
From: Rob Herring @ 2016-09-01 21:40 UTC (permalink / raw)
  To: Andi Shyti
  Cc: Mauro Carvalho Chehab, Sean Young, Mark Rutland, linux-media,
	devicetree, linux-kernel, Andi Shyti

On Thu, Sep 1, 2016 at 12:16 PM, Andi Shyti <andi.shyti@samsung.com> wrote:
> Document the ir-spi driver's binding which is a IR led driven
> through the SPI line.
>
> Signed-off-by: Andi Shyti <andi.shyti@samsung.com>
> ---
>  Documentation/devicetree/bindings/media/spi-ir.txt | 26 ++++++++++++++++++++++
>  1 file changed, 26 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/spi-ir.txt
>
> diff --git a/Documentation/devicetree/bindings/media/spi-ir.txt b/Documentation/devicetree/bindings/media/spi-ir.txt
> new file mode 100644
> index 0000000..85cb21b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/spi-ir.txt

Move this to bindings/leds, and CC the leds maintainers.

> @@ -0,0 +1,26 @@
> +Device tree bindings for IR LED connected through SPI bus which is used as
> +remote controller.
> +
> +The IR LED switch is connected to the MOSI line of the SPI device and the data
> +are delivered thourgh that.
> +
> +Required properties:
> +       - compatible: should be "ir-spi".

Really this is just an LED connected to a SPI, so maybe this should
just be "spi-led". If being more specific is helpful, then I'm all for
that, but perhaps spi-ir-led. (Trying to be consistent in naming with
gpio-leds).

> +
> +Optional properties:
> +       - irled,switch: specifies the gpio switch which enables the irled/

As I said previously, "switch-gpios" as gpio lines should have a
'-gpios' suffix. Or better yet, "enable-gpios" as that is a standard
name for an enable line.

> +       - negated: boolean value that specifies whether the output is negated
> +         with a NOT gate.

Negated or inverted assumes I know what normal is. Define this in
terms of what is the on state. If on is normally active low, then this
should be led-active-high. There may already be an LED property for
this.

> +       - duty-cycle: 8 bit value that stores the percentage of the duty cycle.
> +         it can be 50, 60, 70, 75, 80 or 90.

This is percent time on or off?

Rob

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

* Re: [PATCH v2 3/7] [media] rc-core: add support for IR raw transmitters
@ 2016-09-02  0:21     ` kbuild test robot
  0 siblings, 0 replies; 33+ messages in thread
From: kbuild test robot @ 2016-09-02  0:21 UTC (permalink / raw)
  To: Andi Shyti
  Cc: kbuild-all, Mauro Carvalho Chehab, Sean Young, Rob Herring,
	Mark Rutland, linux-media, devicetree, linux-kernel, Andi Shyti,
	Andi Shyti

[-- Attachment #1: Type: text/plain, Size: 5507 bytes --]

Hi Andi,

[auto build test WARNING on linuxtv-media/master]
[also build test WARNING on v4.8-rc4 next-20160825]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
[Suggest to use git(>=2.9.0) format-patch --base=<commit> (or --base=auto for convenience) to record what (public, well-known) commit your patch series was built on]
[Check https://git-scm.com/docs/git-format-patch for more information]

url:    https://github.com/0day-ci/linux/commits/Andi-Shyti/Add-support-for-IR-transmitters/20160902-060825
base:   git://linuxtv.org/media_tree.git master
reproduce: make htmldocs

All warnings (new ones prefixed by >>):

   make[3]: warning: jobserver unavailable: using -j1.  Add '+' to parent make rule.
   drivers/gpu/drm/i915/i915_vgpu.c:105: warning: No description found for parameter 'dev_priv'
   drivers/gpu/drm/i915/i915_vgpu.c:184: warning: No description found for parameter 'dev_priv'
   drivers/gpu/drm/i915/i915_vgpu.c:184: warning: Excess function parameter 'dev' description in 'intel_vgt_balloon'
   drivers/gpu/drm/i915/i915_vgpu.c:106: warning: No description found for parameter 'dev_priv'
   drivers/gpu/drm/i915/i915_vgpu.c:185: warning: No description found for parameter 'dev_priv'
   drivers/gpu/drm/i915/i915_vgpu.c:185: warning: Excess function parameter 'dev' description in 'intel_vgt_balloon'
   drivers/gpu/drm/i915/i915_gem.c:929: warning: No description found for parameter 'i915'
   drivers/gpu/drm/i915/i915_gem.c:929: warning: Excess function parameter 'dev' description in 'i915_gem_gtt_pwrite_fast'
   drivers/gpu/drm/i915/intel_hotplug.c:543: warning: Excess function parameter 'enabled' description in 'intel_hpd_poll_init'
   drivers/gpu/drm/i915/intel_hotplug.c:544: warning: Excess function parameter 'enabled' description in 'intel_hpd_poll_init'
   drivers/gpu/drm/i915/intel_fbc.c:1087: warning: No description found for parameter 'crtc_state'
   drivers/gpu/drm/i915/intel_fbc.c:1087: warning: No description found for parameter 'plane_state'
   drivers/gpu/drm/i915/intel_fbc.c:1088: warning: No description found for parameter 'crtc_state'
   drivers/gpu/drm/i915/intel_fbc.c:1088: warning: No description found for parameter 'plane_state'
>> include/media/rc-core.h:39: warning: bad line: 			 driver requires pulce/spce data sequence.
   drivers/gpu/drm/drm_crtc.c:1272: WARNING: Inline literal start-string without end-string.
   drivers/gpu/drm/drm_crtc.c:1387: WARNING: Inline literal start-string without end-string.
   include/drm/drm_crtc.h:1200: WARNING: Inline literal start-string without end-string.
   include/drm/drm_crtc.h:1253: WARNING: Inline literal start-string without end-string.
   include/drm/drm_crtc.h:1266: WARNING: Inline literal start-string without end-string.
   include/drm/drm_crtc.h:1270: WARNING: Inline literal start-string without end-string.
   drivers/gpu/drm/drm_irq.c:718: WARNING: Option list ends without a blank line; unexpected unindent.
   drivers/gpu/drm/drm_fb_helper.c:2196: WARNING: Inline emphasis start-string without end-string.
   drivers/gpu/drm/drm_simple_kms_helper.c:156: WARNING: Inline literal start-string without end-string.
   include/drm/drm_gem.h:212: WARNING: Inline emphasis start-string without end-string.
   drivers/gpu/drm/i915/intel_uncore.c:1622: ERROR: Unexpected indentation.
   drivers/gpu/drm/i915/intel_uncore.c:1623: WARNING: Block quote ends without a blank line; unexpected unindent.
   drivers/gpu/drm/i915/intel_uncore.c:1656: ERROR: Unexpected indentation.
   drivers/gpu/drm/i915/intel_uncore.c:1657: WARNING: Block quote ends without a blank line; unexpected unindent.
   drivers/gpu/drm/i915/i915_vgpu.c:178: WARNING: Literal block ends without a blank line; unexpected unindent.
   drivers/gpu/drm/i915/intel_audio.c:54: WARNING: Inline emphasis start-string without end-string.
   drivers/gpu/drm/i915/intel_audio.c:54: WARNING: Inline emphasis start-string without end-string.
   drivers/gpu/drm/i915/intel_lrc.c:1166: ERROR: Unexpected indentation.
   drivers/gpu/drm/i915/intel_lrc.c:1167: WARNING: Block quote ends without a blank line; unexpected unindent.
   drivers/gpu/drm/i915/intel_guc_fwif.h:159: WARNING: Block quote ends without a blank line; unexpected unindent.
   drivers/gpu/drm/i915/intel_guc_fwif.h:178: WARNING: Enumerated list ends without a blank line; unexpected unindent.
   WARNING: dvipng command 'dvipng' cannot be run (needed for math display), check the imgmath_dvipng setting

vim +39 include/media/rc-core.h

    23	#include <media/rc-map.h>
    24	
    25	extern int rc_core_debug;
    26	#define IR_dprintk(level, fmt, ...)				\
    27	do {								\
    28		if (rc_core_debug >= level)				\
    29			printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__);	\
    30	} while (0)
    31	
    32	/**
    33	 * enum rc_driver_type - type of the RC output
    34	 *
    35	 * @RC_DRIVER_SCANCODE:	 Driver or hardware generates a scancode
    36	 * @RC_DRIVER_IR_RAW:	 Driver or hardware generates pulse/space sequences.
    37	 *			 It needs a Infra-Red pulse/space decoder
    38	 * @RC_DRIVER_IR_RAW_TX: Device transmitter only,
  > 39				 driver requires pulce/spce data sequence.
    40	 */
    41	enum rc_driver_type {
    42		RC_DRIVER_SCANCODE = 0,
    43		RC_DRIVER_IR_RAW,
    44		RC_DRIVER_IR_RAW_TX,
    45	};
    46	
    47	/**

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 6381 bytes --]

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

* Re: [PATCH v2 3/7] [media] rc-core: add support for IR raw transmitters
@ 2016-09-02  0:21     ` kbuild test robot
  0 siblings, 0 replies; 33+ messages in thread
From: kbuild test robot @ 2016-09-02  0:21 UTC (permalink / raw)
  Cc: kbuild-all-JC7UmRfGjtg, Mauro Carvalho Chehab, Sean Young,
	Rob Herring, Mark Rutland, linux-media-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Andi Shyti, Andi Shyti

[-- Attachment #1: Type: text/plain, Size: 5507 bytes --]

Hi Andi,

[auto build test WARNING on linuxtv-media/master]
[also build test WARNING on v4.8-rc4 next-20160825]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
[Suggest to use git(>=2.9.0) format-patch --base=<commit> (or --base=auto for convenience) to record what (public, well-known) commit your patch series was built on]
[Check https://git-scm.com/docs/git-format-patch for more information]

url:    https://github.com/0day-ci/linux/commits/Andi-Shyti/Add-support-for-IR-transmitters/20160902-060825
base:   git://linuxtv.org/media_tree.git master
reproduce: make htmldocs

All warnings (new ones prefixed by >>):

   make[3]: warning: jobserver unavailable: using -j1.  Add '+' to parent make rule.
   drivers/gpu/drm/i915/i915_vgpu.c:105: warning: No description found for parameter 'dev_priv'
   drivers/gpu/drm/i915/i915_vgpu.c:184: warning: No description found for parameter 'dev_priv'
   drivers/gpu/drm/i915/i915_vgpu.c:184: warning: Excess function parameter 'dev' description in 'intel_vgt_balloon'
   drivers/gpu/drm/i915/i915_vgpu.c:106: warning: No description found for parameter 'dev_priv'
   drivers/gpu/drm/i915/i915_vgpu.c:185: warning: No description found for parameter 'dev_priv'
   drivers/gpu/drm/i915/i915_vgpu.c:185: warning: Excess function parameter 'dev' description in 'intel_vgt_balloon'
   drivers/gpu/drm/i915/i915_gem.c:929: warning: No description found for parameter 'i915'
   drivers/gpu/drm/i915/i915_gem.c:929: warning: Excess function parameter 'dev' description in 'i915_gem_gtt_pwrite_fast'
   drivers/gpu/drm/i915/intel_hotplug.c:543: warning: Excess function parameter 'enabled' description in 'intel_hpd_poll_init'
   drivers/gpu/drm/i915/intel_hotplug.c:544: warning: Excess function parameter 'enabled' description in 'intel_hpd_poll_init'
   drivers/gpu/drm/i915/intel_fbc.c:1087: warning: No description found for parameter 'crtc_state'
   drivers/gpu/drm/i915/intel_fbc.c:1087: warning: No description found for parameter 'plane_state'
   drivers/gpu/drm/i915/intel_fbc.c:1088: warning: No description found for parameter 'crtc_state'
   drivers/gpu/drm/i915/intel_fbc.c:1088: warning: No description found for parameter 'plane_state'
>> include/media/rc-core.h:39: warning: bad line: 			 driver requires pulce/spce data sequence.
   drivers/gpu/drm/drm_crtc.c:1272: WARNING: Inline literal start-string without end-string.
   drivers/gpu/drm/drm_crtc.c:1387: WARNING: Inline literal start-string without end-string.
   include/drm/drm_crtc.h:1200: WARNING: Inline literal start-string without end-string.
   include/drm/drm_crtc.h:1253: WARNING: Inline literal start-string without end-string.
   include/drm/drm_crtc.h:1266: WARNING: Inline literal start-string without end-string.
   include/drm/drm_crtc.h:1270: WARNING: Inline literal start-string without end-string.
   drivers/gpu/drm/drm_irq.c:718: WARNING: Option list ends without a blank line; unexpected unindent.
   drivers/gpu/drm/drm_fb_helper.c:2196: WARNING: Inline emphasis start-string without end-string.
   drivers/gpu/drm/drm_simple_kms_helper.c:156: WARNING: Inline literal start-string without end-string.
   include/drm/drm_gem.h:212: WARNING: Inline emphasis start-string without end-string.
   drivers/gpu/drm/i915/intel_uncore.c:1622: ERROR: Unexpected indentation.
   drivers/gpu/drm/i915/intel_uncore.c:1623: WARNING: Block quote ends without a blank line; unexpected unindent.
   drivers/gpu/drm/i915/intel_uncore.c:1656: ERROR: Unexpected indentation.
   drivers/gpu/drm/i915/intel_uncore.c:1657: WARNING: Block quote ends without a blank line; unexpected unindent.
   drivers/gpu/drm/i915/i915_vgpu.c:178: WARNING: Literal block ends without a blank line; unexpected unindent.
   drivers/gpu/drm/i915/intel_audio.c:54: WARNING: Inline emphasis start-string without end-string.
   drivers/gpu/drm/i915/intel_audio.c:54: WARNING: Inline emphasis start-string without end-string.
   drivers/gpu/drm/i915/intel_lrc.c:1166: ERROR: Unexpected indentation.
   drivers/gpu/drm/i915/intel_lrc.c:1167: WARNING: Block quote ends without a blank line; unexpected unindent.
   drivers/gpu/drm/i915/intel_guc_fwif.h:159: WARNING: Block quote ends without a blank line; unexpected unindent.
   drivers/gpu/drm/i915/intel_guc_fwif.h:178: WARNING: Enumerated list ends without a blank line; unexpected unindent.
   WARNING: dvipng command 'dvipng' cannot be run (needed for math display), check the imgmath_dvipng setting

vim +39 include/media/rc-core.h

    23	#include <media/rc-map.h>
    24	
    25	extern int rc_core_debug;
    26	#define IR_dprintk(level, fmt, ...)				\
    27	do {								\
    28		if (rc_core_debug >= level)				\
    29			printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__);	\
    30	} while (0)
    31	
    32	/**
    33	 * enum rc_driver_type - type of the RC output
    34	 *
    35	 * @RC_DRIVER_SCANCODE:	 Driver or hardware generates a scancode
    36	 * @RC_DRIVER_IR_RAW:	 Driver or hardware generates pulse/space sequences.
    37	 *			 It needs a Infra-Red pulse/space decoder
    38	 * @RC_DRIVER_IR_RAW_TX: Device transmitter only,
  > 39				 driver requires pulce/spce data sequence.
    40	 */
    41	enum rc_driver_type {
    42		RC_DRIVER_SCANCODE = 0,
    43		RC_DRIVER_IR_RAW,
    44		RC_DRIVER_IR_RAW_TX,
    45	};
    46	
    47	/**

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 6381 bytes --]

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

* Re: [PATCH v2 1/7] [media] rc-main: assign driver type during allocation
@ 2016-09-02  5:25       ` Andi Shyti
  0 siblings, 0 replies; 33+ messages in thread
From: Andi Shyti @ 2016-09-02  5:25 UTC (permalink / raw)
  To: Sean Young
  Cc: Mauro Carvalho Chehab, Rob Herring, Mark Rutland, linux-media,
	devicetree, linux-kernel, Andi Shyti

Hi Sean,

> >  	ir = kzalloc(sizeof(*ir), GFP_KERNEL);
> > -	dev = rc_allocate_device();
> > +	dev = rc_allocate_device(RC_DRIVER_IR_RAW);
> >  	if (!ir || !dev)
> >  		goto err_out_free;
> >  
> 
> If ir->sampling = 0 then it should be RC_DRIVER_SCANCODE.
> 
> 
> > @@ -481,7 +481,6 @@ int cx88_ir_init(struct cx88_core *core, struct pci_dev *pci)
> >  	dev->scancode_mask = hardware_mask;
> >  
> >  	if (ir->sampling) {
> > -		dev->driver_type = RC_DRIVER_IR_RAW;
> >  		dev->timeout = 10 * 1000 * 1000; /* 10 ms */
> >  	} else {
> >  		dev->driver_type = RC_DRIVER_SCANCODE;
> 
> That assignment shouldn't really be there any more.

I think this doesn't change the driver's behavior, because I
either do like:

  -   dev = rc_allocate_device();
  +   dev = rc_allocate_device(RC_DRIVER_SCANCODE);

  [ ... ]

      if (ir->sampling) {
              dev->driver_type = RC_DRIVER_IR_RAW;
              dev->timeout = 10 * 1000 * 1000; /* 10 ms */
      } else {
   -          dev->driver_type = RC_DRIVER_SCANCODE;

Or I would need to do aftr the long switch...case statement

    +  if (ir->sampling) {
    +          dev = rc_allocate_device(RC_DRIVER_IR_RAW);
    +          ...
    +  } else {
    +          dev = rc_allocate_device(RC_DRIVER_SCANCODE);
    +          ...

I prefered the first way because it doesn't alter much the
driver.

> >  	ir = kzalloc(sizeof(*ir), GFP_KERNEL);
> > -	rc = rc_allocate_device();
> > +	rc = rc_allocate_device(RC_DRIVER_SCANCODE);
> >  	if (!ir || !rc) {
> >  		err = -ENOMEM;
> >  		goto err_out_free;
> 
> This is not correct, I'm afraid. If you look at the code you can see that
> if raw_decode is true, then it should be RC_DRIVER_IR_RAW.

Same here, the driver doesn't change the behavior. raw_decode can
be both 'true' or 'false' it's set as default RC_DRIVER_SCANCODE
and depending on value of raw_decode it's chaged to
RC_DRIVER_IR_RAW.

also in this case I can do

    +  if (raw_decode) {
    +          rc = rc_allocate_device(RC_DRIVER_IR_RAW);
    +          ...
    +  } else {
    +          rc = rc_allocate_device(RC_DRIVER_SCANCODE);
    +          ...

but also in this case my original approach doesn't add much
changes.

Thanks,
Andi

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

* Re: [PATCH v2 1/7] [media] rc-main: assign driver type during allocation
@ 2016-09-02  5:25       ` Andi Shyti
  0 siblings, 0 replies; 33+ messages in thread
From: Andi Shyti @ 2016-09-02  5:25 UTC (permalink / raw)
  To: Sean Young
  Cc: Mauro Carvalho Chehab, Rob Herring, Mark Rutland,
	linux-media-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Andi Shyti

Hi Sean,

> >  	ir = kzalloc(sizeof(*ir), GFP_KERNEL);
> > -	dev = rc_allocate_device();
> > +	dev = rc_allocate_device(RC_DRIVER_IR_RAW);
> >  	if (!ir || !dev)
> >  		goto err_out_free;
> >  
> 
> If ir->sampling = 0 then it should be RC_DRIVER_SCANCODE.
> 
> 
> > @@ -481,7 +481,6 @@ int cx88_ir_init(struct cx88_core *core, struct pci_dev *pci)
> >  	dev->scancode_mask = hardware_mask;
> >  
> >  	if (ir->sampling) {
> > -		dev->driver_type = RC_DRIVER_IR_RAW;
> >  		dev->timeout = 10 * 1000 * 1000; /* 10 ms */
> >  	} else {
> >  		dev->driver_type = RC_DRIVER_SCANCODE;
> 
> That assignment shouldn't really be there any more.

I think this doesn't change the driver's behavior, because I
either do like:

  -   dev = rc_allocate_device();
  +   dev = rc_allocate_device(RC_DRIVER_SCANCODE);

  [ ... ]

      if (ir->sampling) {
              dev->driver_type = RC_DRIVER_IR_RAW;
              dev->timeout = 10 * 1000 * 1000; /* 10 ms */
      } else {
   -          dev->driver_type = RC_DRIVER_SCANCODE;

Or I would need to do aftr the long switch...case statement

    +  if (ir->sampling) {
    +          dev = rc_allocate_device(RC_DRIVER_IR_RAW);
    +          ...
    +  } else {
    +          dev = rc_allocate_device(RC_DRIVER_SCANCODE);
    +          ...

I prefered the first way because it doesn't alter much the
driver.

> >  	ir = kzalloc(sizeof(*ir), GFP_KERNEL);
> > -	rc = rc_allocate_device();
> > +	rc = rc_allocate_device(RC_DRIVER_SCANCODE);
> >  	if (!ir || !rc) {
> >  		err = -ENOMEM;
> >  		goto err_out_free;
> 
> This is not correct, I'm afraid. If you look at the code you can see that
> if raw_decode is true, then it should be RC_DRIVER_IR_RAW.

Same here, the driver doesn't change the behavior. raw_decode can
be both 'true' or 'false' it's set as default RC_DRIVER_SCANCODE
and depending on value of raw_decode it's chaged to
RC_DRIVER_IR_RAW.

also in this case I can do

    +  if (raw_decode) {
    +          rc = rc_allocate_device(RC_DRIVER_IR_RAW);
    +          ...
    +  } else {
    +          rc = rc_allocate_device(RC_DRIVER_SCANCODE);
    +          ...

but also in this case my original approach doesn't add much
changes.

Thanks,
Andi
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* Re: [PATCH v2 7/7] [media] rc: add support for IR LEDs driven through SPI
@ 2016-09-02  5:27       ` Andi Shyti
  0 siblings, 0 replies; 33+ messages in thread
From: Andi Shyti @ 2016-09-02  5:27 UTC (permalink / raw)
  To: Sean Young
  Cc: Mauro Carvalho Chehab, Rob Herring, Mark Rutland, linux-media,
	devicetree, linux-kernel, Andi Shyti

> Thanks Andi, this is looking great!

Thanks Sean! With your reviews the whole thing looks much better
now :)

I agree with all your points here, I will fix them. Can I add
your reviewd-by?

Thanks,
Andi

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

* Re: [PATCH v2 7/7] [media] rc: add support for IR LEDs driven through SPI
@ 2016-09-02  5:27       ` Andi Shyti
  0 siblings, 0 replies; 33+ messages in thread
From: Andi Shyti @ 2016-09-02  5:27 UTC (permalink / raw)
  To: Sean Young
  Cc: Mauro Carvalho Chehab, Rob Herring, Mark Rutland,
	linux-media-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Andi Shyti

> Thanks Andi, this is looking great!

Thanks Sean! With your reviews the whole thing looks much better
now :)

I agree with all your points here, I will fix them. Can I add
your reviewd-by?

Thanks,
Andi
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* Re: [PATCH v2 6/7] Documentation: bindings: add documentation for ir-spi device driver
  2016-09-01 21:40   ` Rob Herring
@ 2016-09-02  5:33     ` Andi Shyti
  2016-09-12 13:27         ` Rob Herring
  0 siblings, 1 reply; 33+ messages in thread
From: Andi Shyti @ 2016-09-02  5:33 UTC (permalink / raw)
  To: Rob Herring
  Cc: Mauro Carvalho Chehab, Sean Young, Mark Rutland, linux-media,
	devicetree, linux-kernel, Andi Shyti

Hi Rob,

> > Document the ir-spi driver's binding which is a IR led driven
> > through the SPI line.
> >
> > Signed-off-by: Andi Shyti <andi.shyti@samsung.com>
> > ---
> >  Documentation/devicetree/bindings/media/spi-ir.txt | 26 ++++++++++++++++++++++
> >  1 file changed, 26 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/media/spi-ir.txt
> >
> > diff --git a/Documentation/devicetree/bindings/media/spi-ir.txt b/Documentation/devicetree/bindings/media/spi-ir.txt
> > new file mode 100644
> > index 0000000..85cb21b
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/media/spi-ir.txt
> 
> Move this to bindings/leds, and CC the leds maintainers.

More than an LED this is the driver of a remote controller, the
driver itself is under drivers/media/rc/.

Besides all the transmitters have an LED but still they are media
devices. This is a bit special because it's so simple that the
only hardware left is the LED itself, but still it's a media remote
controller.

> > @@ -0,0 +1,26 @@
> > +Device tree bindings for IR LED connected through SPI bus which is used as
> > +remote controller.
> > +
> > +The IR LED switch is connected to the MOSI line of the SPI device and the data
> > +are delivered thourgh that.
> > +
> > +Required properties:
> > +       - compatible: should be "ir-spi".
> 
> Really this is just an LED connected to a SPI, so maybe this should
> just be "spi-led". If being more specific is helpful, then I'm all for
> that, but perhaps spi-ir-led. (Trying to be consistent in naming with
> gpio-leds).

As I mentioned above, all transmitters have an LED, but they do
not have the 'led' name. "ir-spi" is coherent with the device
driver name and the driver name is coherent with the media/rc
driver's naming.

> > +
> > +Optional properties:
> > +       - irled,switch: specifies the gpio switch which enables the irled/
> 
> As I said previously, "switch-gpios" as gpio lines should have a
> '-gpios' suffix. Or better yet, "enable-gpios" as that is a standard
> name for an enable line.

OK, thanks!

> > +       - negated: boolean value that specifies whether the output is negated
> > +         with a NOT gate.
> 
> Negated or inverted assumes I know what normal is. Define this in
> terms of what is the on state. If on is normally active low, then this
> should be led-active-high. There may already be an LED property for
> this.

Yes, thanks!

> > +       - duty-cycle: 8 bit value that stores the percentage of the duty cycle.
> > +         it can be 50, 60, 70, 75, 80 or 90.
> 
> This is percent time on or off?

Will add more details, thanks.

If it's OK for you, I would keep the name and documentation path
and fix the rest. Please let me know if I'm missing something :)

Thanks,
Andi

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

* Re: [PATCH v2 5/7] [media] ir-lirc-codec: don't wait any transmitting time for tx only devices
  2016-09-01 17:16 ` [PATCH v2 5/7] [media] ir-lirc-codec: don't wait any transmitting time for tx only devices Andi Shyti
@ 2016-09-02  8:41   ` Sean Young
  2016-10-27  7:44       ` Andi Shyti
  0 siblings, 1 reply; 33+ messages in thread
From: Sean Young @ 2016-09-02  8:41 UTC (permalink / raw)
  To: Andi Shyti
  Cc: Mauro Carvalho Chehab, Rob Herring, Mark Rutland, linux-media,
	devicetree, linux-kernel, Andi Shyti, David Härdeman

On Fri, Sep 02, 2016 at 02:16:27AM +0900, Andi Shyti wrote:
> Transmitters do not need to wait until the data has been sent
> (and of course received). Return before waiting.
> 
> Signed-off-by: Andi Shyti <andi.shyti@samsung.com>
> ---
>  drivers/media/rc/ir-lirc-codec.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/media/rc/ir-lirc-codec.c b/drivers/media/rc/ir-lirc-codec.c
> index c327730..d8953fb 100644
> --- a/drivers/media/rc/ir-lirc-codec.c
> +++ b/drivers/media/rc/ir-lirc-codec.c
> @@ -153,7 +153,7 @@ static ssize_t ir_lirc_transmit_ir(struct file *file, const char __user *buf,
>  	}
>  
>  	ret = dev->tx_ir(dev, txbuf, count);
> -	if (ret < 0)
> +	if (ret < 0 || dev->driver_type == RC_DRIVER_IR_RAW_TX)

Just because a driver only does transmit doesn't mean its transmit ABI
should change.

Now this bit of code is pretty horrible. It ensures that the call to write()
takes at least as long as the length of the transmit IR by sleeping. That's
not much of a guarantee that the IR has been sent.

Note that in the case of ir-spi, since your spi transfer is sync no sleep
should be introduced here.

The gap calculation in lirc checks that if the call to write() took _longer_
than expected wait before sending the next IR code (when either multiple
IR codes or repeats are specified). Introducing the sleep in the kernel
here does not help at all, lirc already ensures that it waits as long as
the IR is long (see schedule_repeat_timer in lirc).

This change was introduced in 3.10, commit f8e00d5. 


Sean

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

* Re: [PATCH v2 7/7] [media] rc: add support for IR LEDs driven through SPI
  2016-09-02  5:27       ` Andi Shyti
  (?)
@ 2016-09-02  8:43       ` Sean Young
  -1 siblings, 0 replies; 33+ messages in thread
From: Sean Young @ 2016-09-02  8:43 UTC (permalink / raw)
  To: Andi Shyti
  Cc: Mauro Carvalho Chehab, Rob Herring, Mark Rutland, linux-media,
	devicetree, linux-kernel, Andi Shyti

On Fri, Sep 02, 2016 at 02:27:08PM +0900, Andi Shyti wrote:
> > Thanks Andi, this is looking great!
> 
> Thanks Sean! With your reviews the whole thing looks much better
> now :)
> 
> I agree with all your points here, I will fix them. Can I add
> your reviewd-by?

Yes, please add it to this patch.

Thanks,

Sean

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

* Re: [PATCH v2 6/7] Documentation: bindings: add documentation for ir-spi device driver
@ 2016-09-12 13:27         ` Rob Herring
  0 siblings, 0 replies; 33+ messages in thread
From: Rob Herring @ 2016-09-12 13:27 UTC (permalink / raw)
  To: Andi Shyti
  Cc: Mauro Carvalho Chehab, Sean Young, Mark Rutland, linux-media,
	devicetree, linux-kernel, Andi Shyti

On Fri, Sep 02, 2016 at 02:33:20PM +0900, Andi Shyti wrote:
> Hi Rob,
> 
> > > Document the ir-spi driver's binding which is a IR led driven
> > > through the SPI line.
> > >
> > > Signed-off-by: Andi Shyti <andi.shyti@samsung.com>
> > > ---
> > >  Documentation/devicetree/bindings/media/spi-ir.txt | 26 ++++++++++++++++++++++
> > >  1 file changed, 26 insertions(+)
> > >  create mode 100644 Documentation/devicetree/bindings/media/spi-ir.txt
> > >
> > > diff --git a/Documentation/devicetree/bindings/media/spi-ir.txt b/Documentation/devicetree/bindings/media/spi-ir.txt
> > > new file mode 100644
> > > index 0000000..85cb21b
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/media/spi-ir.txt
> > 
> > Move this to bindings/leds, and CC the leds maintainers.
> 
> More than an LED this is the driver of a remote controller, the
> driver itself is under drivers/media/rc/.

The hardware is just an LED though and bindings describe h/w. What you 
are using it for doesn't really matter. You only need to know it's an IR 
led.
 
> Besides all the transmitters have an LED but still they are media
> devices. This is a bit special because it's so simple that the
> only hardware left is the LED itself, but still it's a media remote
> controller.
> 
> > > @@ -0,0 +1,26 @@
> > > +Device tree bindings for IR LED connected through SPI bus which is used as
> > > +remote controller.
> > > +
> > > +The IR LED switch is connected to the MOSI line of the SPI device and the data
> > > +are delivered thourgh that.
> > > +
> > > +Required properties:
> > > +       - compatible: should be "ir-spi".
> > 
> > Really this is just an LED connected to a SPI, so maybe this should
> > just be "spi-led". If being more specific is helpful, then I'm all for
> > that, but perhaps spi-ir-led. (Trying to be consistent in naming with
> > gpio-leds).
> 
> As I mentioned above, all transmitters have an LED, but they do
> not have the 'led' name. "ir-spi" is coherent with the device
> driver name and the driver name is coherent with the media/rc
> driver's naming.

The driver name is irrelevant to the binding. 

[...]

> If it's OK for you, I would keep the name and documentation path
> and fix the rest. Please let me know if I'm missing something :)

It's not OK for me.

Rob

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

* Re: [PATCH v2 6/7] Documentation: bindings: add documentation for ir-spi device driver
@ 2016-09-12 13:27         ` Rob Herring
  0 siblings, 0 replies; 33+ messages in thread
From: Rob Herring @ 2016-09-12 13:27 UTC (permalink / raw)
  To: Andi Shyti
  Cc: Mauro Carvalho Chehab, Sean Young, Mark Rutland,
	linux-media-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Andi Shyti

On Fri, Sep 02, 2016 at 02:33:20PM +0900, Andi Shyti wrote:
> Hi Rob,
> 
> > > Document the ir-spi driver's binding which is a IR led driven
> > > through the SPI line.
> > >
> > > Signed-off-by: Andi Shyti <andi.shyti-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> > > ---
> > >  Documentation/devicetree/bindings/media/spi-ir.txt | 26 ++++++++++++++++++++++
> > >  1 file changed, 26 insertions(+)
> > >  create mode 100644 Documentation/devicetree/bindings/media/spi-ir.txt
> > >
> > > diff --git a/Documentation/devicetree/bindings/media/spi-ir.txt b/Documentation/devicetree/bindings/media/spi-ir.txt
> > > new file mode 100644
> > > index 0000000..85cb21b
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/media/spi-ir.txt
> > 
> > Move this to bindings/leds, and CC the leds maintainers.
> 
> More than an LED this is the driver of a remote controller, the
> driver itself is under drivers/media/rc/.

The hardware is just an LED though and bindings describe h/w. What you 
are using it for doesn't really matter. You only need to know it's an IR 
led.
 
> Besides all the transmitters have an LED but still they are media
> devices. This is a bit special because it's so simple that the
> only hardware left is the LED itself, but still it's a media remote
> controller.
> 
> > > @@ -0,0 +1,26 @@
> > > +Device tree bindings for IR LED connected through SPI bus which is used as
> > > +remote controller.
> > > +
> > > +The IR LED switch is connected to the MOSI line of the SPI device and the data
> > > +are delivered thourgh that.
> > > +
> > > +Required properties:
> > > +       - compatible: should be "ir-spi".
> > 
> > Really this is just an LED connected to a SPI, so maybe this should
> > just be "spi-led". If being more specific is helpful, then I'm all for
> > that, but perhaps spi-ir-led. (Trying to be consistent in naming with
> > gpio-leds).
> 
> As I mentioned above, all transmitters have an LED, but they do
> not have the 'led' name. "ir-spi" is coherent with the device
> driver name and the driver name is coherent with the media/rc
> driver's naming.

The driver name is irrelevant to the binding. 

[...]

> If it's OK for you, I would keep the name and documentation path
> and fix the rest. Please let me know if I'm missing something :)

It's not OK for me.

Rob
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* Re: [PATCH v2 5/7] [media] ir-lirc-codec: don't wait any transmitting time for tx only devices
@ 2016-10-27  7:44       ` Andi Shyti
  0 siblings, 0 replies; 33+ messages in thread
From: Andi Shyti @ 2016-10-27  7:44 UTC (permalink / raw)
  To: Sean Young
  Cc: Mauro Carvalho Chehab, Rob Herring, Mark Rutland, linux-media,
	devicetree, linux-kernel, Andi Shyti, David Härdeman

Hi Sean,

it's been a while :)

I was going through your review fixing what needs to be fixed,
but...

> > @@ -153,7 +153,7 @@ static ssize_t ir_lirc_transmit_ir(struct file *file, const char __user *buf,
> >  	}
> >  
> >  	ret = dev->tx_ir(dev, txbuf, count);
> > -	if (ret < 0)
> > +	if (ret < 0 || dev->driver_type == RC_DRIVER_IR_RAW_TX)
> 
> Just because a driver only does transmit doesn't mean its transmit ABI
> should change.
> 
> Now this bit of code is pretty horrible. It ensures that the call to write()
> takes at least as long as the length of the transmit IR by sleeping. That's
> not much of a guarantee that the IR has been sent.
> 
> Note that in the case of ir-spi, since your spi transfer is sync no sleep
> should be introduced here.
> 
> The gap calculation in lirc checks that if the call to write() took _longer_
> than expected wait before sending the next IR code (when either multiple
> IR codes or repeats are specified). Introducing the sleep in the kernel
> here does not help at all, lirc already ensures that it waits as long as
> the IR is long (see schedule_repeat_timer in lirc).
> 
> This change was introduced in 3.10, commit f8e00d5. 

... I'm not sure what can be done here. I get your point and I
understand that this indeed is a kind of fake sync point and by
doing this I 

How about creating two different functions:

- ir_lirc_transmit_ir where we actually do what the function
  already does
- ir_lirc_transmit_no_sync where the function we don't wait
  because the the sync is done on a different level (for example
  in the SPI case).

SPI does approximately the same thing.

Andi

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

* Re: [PATCH v2 5/7] [media] ir-lirc-codec: don't wait any transmitting time for tx only devices
@ 2016-10-27  7:44       ` Andi Shyti
  0 siblings, 0 replies; 33+ messages in thread
From: Andi Shyti @ 2016-10-27  7:44 UTC (permalink / raw)
  To: Sean Young
  Cc: Mauro Carvalho Chehab, Rob Herring, Mark Rutland,
	linux-media-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Andi Shyti,
	David Härdeman

Hi Sean,

it's been a while :)

I was going through your review fixing what needs to be fixed,
but...

> > @@ -153,7 +153,7 @@ static ssize_t ir_lirc_transmit_ir(struct file *file, const char __user *buf,
> >  	}
> >  
> >  	ret = dev->tx_ir(dev, txbuf, count);
> > -	if (ret < 0)
> > +	if (ret < 0 || dev->driver_type == RC_DRIVER_IR_RAW_TX)
> 
> Just because a driver only does transmit doesn't mean its transmit ABI
> should change.
> 
> Now this bit of code is pretty horrible. It ensures that the call to write()
> takes at least as long as the length of the transmit IR by sleeping. That's
> not much of a guarantee that the IR has been sent.
> 
> Note that in the case of ir-spi, since your spi transfer is sync no sleep
> should be introduced here.
> 
> The gap calculation in lirc checks that if the call to write() took _longer_
> than expected wait before sending the next IR code (when either multiple
> IR codes or repeats are specified). Introducing the sleep in the kernel
> here does not help at all, lirc already ensures that it waits as long as
> the IR is long (see schedule_repeat_timer in lirc).
> 
> This change was introduced in 3.10, commit f8e00d5. 

... I'm not sure what can be done here. I get your point and I
understand that this indeed is a kind of fake sync point and by
doing this I 

How about creating two different functions:

- ir_lirc_transmit_ir where we actually do what the function
  already does
- ir_lirc_transmit_no_sync where the function we don't wait
  because the the sync is done on a different level (for example
  in the SPI case).

SPI does approximately the same thing.

Andi
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* Re: [PATCH v2 5/7] [media] ir-lirc-codec: don't wait any transmitting time for tx only devices
  2016-10-27  7:44       ` Andi Shyti
  (?)
@ 2016-10-27 14:36       ` Sean Young
  -1 siblings, 0 replies; 33+ messages in thread
From: Sean Young @ 2016-10-27 14:36 UTC (permalink / raw)
  To: Andi Shyti
  Cc: Mauro Carvalho Chehab, Rob Herring, Mark Rutland, linux-media,
	devicetree, linux-kernel, Andi Shyti, David Härdeman

On Thu, Oct 27, 2016 at 04:44:01PM +0900, Andi Shyti wrote:
> Hi Sean,
> 
> it's been a while :)
> 
> I was going through your review fixing what needs to be fixed,
> but...
> 
> > > @@ -153,7 +153,7 @@ static ssize_t ir_lirc_transmit_ir(struct file *file, const char __user *buf,
> > >  	}
> > >  
> > >  	ret = dev->tx_ir(dev, txbuf, count);
> > > -	if (ret < 0)
> > > +	if (ret < 0 || dev->driver_type == RC_DRIVER_IR_RAW_TX)
> > 
> > Just because a driver only does transmit doesn't mean its transmit ABI
> > should change.
> > 
> > Now this bit of code is pretty horrible. It ensures that the call to write()
> > takes at least as long as the length of the transmit IR by sleeping. That's
> > not much of a guarantee that the IR has been sent.
> > 
> > Note that in the case of ir-spi, since your spi transfer is sync no sleep
> > should be introduced here.
> > 
> > The gap calculation in lirc checks that if the call to write() took _longer_
> > than expected wait before sending the next IR code (when either multiple
> > IR codes or repeats are specified). Introducing the sleep in the kernel
> > here does not help at all, lirc already ensures that it waits as long as
> > the IR is long (see schedule_repeat_timer in lirc).
> > 
> > This change was introduced in 3.10, commit f8e00d5. 
> 
> ... I'm not sure what can be done here. I get your point and I
> understand that this indeed is a kind of fake sync point and by
> doing this I 

My original plan was to send a patch which just removes the silly wait,
but on further investigating debian stable and testing still carry a
lirc version that depend on it, so that's not going to fly.

> How about creating two different functions:
> 
> - ir_lirc_transmit_ir where we actually do what the function
>   already does
> - ir_lirc_transmit_no_sync where the function we don't wait
>   because the the sync is done on a different level (for example
>   in the SPI case).
> 
> SPI does approximately the same thing.

Since we have to be able to switch between waiting and not waiting, 
we need some sort of ABI for this. I think this warrants a new ioctl;
I'm not sure how else it can be done. I'll be sending out a patch
shortly.


Sean

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

* Re: [PATCH v2 5/7] [media] ir-lirc-codec: don't wait any transmitting time for tx only devices
  2016-10-27  7:44       ` Andi Shyti
  (?)
  (?)
@ 2016-10-31 14:31       ` David Härdeman
  2016-10-31 17:05         ` Sean Young
  -1 siblings, 1 reply; 33+ messages in thread
From: David Härdeman @ 2016-10-31 14:31 UTC (permalink / raw)
  To: Sean Young, Andi Shyti
  Cc: Mauro Carvalho Chehab, Rob Herring, Mark Rutland, linux-media,
	devicetree, linux-kernel, Andi Shyti

October 27, 2016 4:36 PM, "Sean Young" <sean@mess.org> wrote:
> Since we have to be able to switch between waiting and not waiting,
> we need some sort of ABI for this. I think this warrants a new ioctl;
> I'm not sure how else it can be done. I'll be sending out a patch
> shortly.

Hi Sean,

have you considered using a module param for the LIRC bridge module instead? As far as I understand it, this is an issue which is entirely internal to LIRC, and it's also not something which really needs to be changed on a per-device level (either you have a modern LIRC daemon or you don't, and all drivers should behave the same, no?).

Another advantage is that the parameter would then go away if and when the lirc bridge ever goes away (yes, I can still dream, can't I?).

Regards,
David

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

* Re: [PATCH v2 5/7] [media] ir-lirc-codec: don't wait any transmitting time for tx only devices
  2016-10-31 14:31       ` David Härdeman
@ 2016-10-31 17:05         ` Sean Young
  2016-11-01  6:51             ` Andi Shyti
  0 siblings, 1 reply; 33+ messages in thread
From: Sean Young @ 2016-10-31 17:05 UTC (permalink / raw)
  To: David Härdeman
  Cc: Andi Shyti, Mauro Carvalho Chehab, Rob Herring, Mark Rutland,
	linux-media, devicetree, linux-kernel, Andi Shyti

Hi David, Andi,

On Mon, Oct 31, 2016 at 02:31:52PM +0000, David Härdeman wrote:
> October 27, 2016 4:36 PM, "Sean Young" <sean@mess.org> wrote:
> > Since we have to be able to switch between waiting and not waiting,
> > we need some sort of ABI for this. I think this warrants a new ioctl;
> > I'm not sure how else it can be done. I'll be sending out a patch
> > shortly.
> 
> Hi Sean,
> 
> have you considered using a module param for the LIRC bridge module instead? As far as I understand it, this is an issue which is entirely internal to LIRC, and it's also not something which really needs to be changed on a per-device level (either you have a modern LIRC daemon or you don't, and all drivers should behave the same, no?).

I still don't see how any of this would change anything for the ir-spi case:
since it uses sync spi transfer, it's going to block anyway.

> Another advantage is that the parameter would then go away if and when the lirc bridge ever goes away (yes, I can still dream, can't I?).

The suggested ioctl is unique to lirc too and would disappear if the
lirc ABI goes away. With a module parameter you would change the lirc ABI
into an incompatible one. Not sure that is what module parameters should
be used for.

Andi, it would be good to know what the use-case for the original change is.


Thanks
Sean

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

* Re: [PATCH v2 5/7] [media] ir-lirc-codec: don't wait any transmitting time for tx only devices
@ 2016-11-01  6:51             ` Andi Shyti
  0 siblings, 0 replies; 33+ messages in thread
From: Andi Shyti @ 2016-11-01  6:51 UTC (permalink / raw)
  To: Sean Young
  Cc: David Härdeman, Mauro Carvalho Chehab, Rob Herring,
	Mark Rutland, linux-media, devicetree, linux-kernel, Andi Shyti

Hi Sean,

> Andi, it would be good to know what the use-case for the original change is.

the use case is the ir-spi itself which doesn't need the lirc to
perform any waiting on its behalf.

To me it just doesn't look right to simulate a fake transmission
period and wait unnecessary time. Of course, the "over-wait" is not
a big deal and at the end we can decide to drop it.

Otherwise, an alternative could be to add the boolean
'tx_no_wait' in the rc_dev structure. It could be set by the
device driver during the initialization and the we can follow
your approach.

Something like this:

diff --git a/drivers/media/rc/ir-lirc-codec.c b/drivers/media/rc/ir-lirc-codec.c
index c327730..4553d04 100644
--- a/drivers/media/rc/ir-lirc-codec.c
+++ b/drivers/media/rc/ir-lirc-codec.c
@@ -161,15 +161,19 @@ static ssize_t ir_lirc_transmit_ir(struct file *file, const char __user *buf,
 
        ret *= sizeof(unsigned int);
 
-       /*
-        * The lircd gap calculation expects the write function to
-        * wait for the actual IR signal to be transmitted before
-        * returning.
-        */
-       towait = ktime_us_delta(ktime_add_us(start, duration), ktime_get());
-       if (towait > 0) {
-               set_current_state(TASK_INTERRUPTIBLE);
-               schedule_timeout(usecs_to_jiffies(towait));
+       if (!dev->tx_no_wait) {
+               /*
+                * The lircd gap calculation expects the write function to
+                * wait for the actual IR signal to be transmitted before
+                * returning.
+                */
+               towait = ktime_us_delta(ktime_add_us(start, duration),
+                                                               ktime_get());
+               if (towait > 0) {
+                       set_current_state(TASK_INTERRUPTIBLE);
+                       schedule_timeout(usecs_to_jiffies(towait));
+               }
+
        }
 
 out:
diff --git a/drivers/media/rc/ir-spi.c b/drivers/media/rc/ir-spi.c
index fcda1e4..e44abfa 100644
--- a/drivers/media/rc/ir-spi.c
+++ b/drivers/media/rc/ir-spi.c
@@ -149,6 +149,7 @@ static int ir_spi_probe(struct spi_device *spi)
        if (!idata->rc)
                return -ENOMEM;
 
+       idata->rc->tx_no_wait      = true;
        idata->rc->tx_ir           = ir_spi_tx;
        idata->rc->s_tx_carrier    = ir_spi_set_tx_carrier;
        idata->rc->s_tx_duty_cycle = ir_spi_set_duty_cycle;
diff --git a/include/media/rc-core.h b/include/media/rc-core.h
index fe0c9c4..c3ced9b 100644
--- a/include/media/rc-core.h
+++ b/include/media/rc-core.h
@@ -85,6 +85,9 @@ enum rc_filter_type {
  * @input_dev: the input child device used to communicate events to userspace
  * @driver_type: specifies if protocol decoding is done in hardware or software
  * @idle: used to keep track of RX state
+ * @tx_no_wait: decides whether to perform or not a sync write or not. The
+ *      device driver setting it to true must make sure to not break the ABI
+ *      which requires a sync transfer.
  * @allowed_protocols: bitmask with the supported RC_BIT_* protocols
  * @enabled_protocols: bitmask with the enabled RC_BIT_* protocols
  * @allowed_wakeup_protocols: bitmask with the supported RC_BIT_* wakeup protocols
@@ -147,6 +150,7 @@ struct rc_dev {
        struct input_dev                *input_dev;
        enum rc_driver_type             driver_type;
        bool                            idle;
+       bool                            tx_no_wait;
        u64                             allowed_protocols;
        u64                             enabled_protocols;
        u64                             allowed_wakeup_protocols;

Thanks,
Andi

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

* Re: [PATCH v2 5/7] [media] ir-lirc-codec: don't wait any transmitting time for tx only devices
@ 2016-11-01  6:51             ` Andi Shyti
  0 siblings, 0 replies; 33+ messages in thread
From: Andi Shyti @ 2016-11-01  6:51 UTC (permalink / raw)
  To: Sean Young
  Cc: David Härdeman, Mauro Carvalho Chehab, Rob Herring,
	Mark Rutland, linux-media-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Andi Shyti

Hi Sean,

> Andi, it would be good to know what the use-case for the original change is.

the use case is the ir-spi itself which doesn't need the lirc to
perform any waiting on its behalf.

To me it just doesn't look right to simulate a fake transmission
period and wait unnecessary time. Of course, the "over-wait" is not
a big deal and at the end we can decide to drop it.

Otherwise, an alternative could be to add the boolean
'tx_no_wait' in the rc_dev structure. It could be set by the
device driver during the initialization and the we can follow
your approach.

Something like this:

diff --git a/drivers/media/rc/ir-lirc-codec.c b/drivers/media/rc/ir-lirc-codec.c
index c327730..4553d04 100644
--- a/drivers/media/rc/ir-lirc-codec.c
+++ b/drivers/media/rc/ir-lirc-codec.c
@@ -161,15 +161,19 @@ static ssize_t ir_lirc_transmit_ir(struct file *file, const char __user *buf,
 
        ret *= sizeof(unsigned int);
 
-       /*
-        * The lircd gap calculation expects the write function to
-        * wait for the actual IR signal to be transmitted before
-        * returning.
-        */
-       towait = ktime_us_delta(ktime_add_us(start, duration), ktime_get());
-       if (towait > 0) {
-               set_current_state(TASK_INTERRUPTIBLE);
-               schedule_timeout(usecs_to_jiffies(towait));
+       if (!dev->tx_no_wait) {
+               /*
+                * The lircd gap calculation expects the write function to
+                * wait for the actual IR signal to be transmitted before
+                * returning.
+                */
+               towait = ktime_us_delta(ktime_add_us(start, duration),
+                                                               ktime_get());
+               if (towait > 0) {
+                       set_current_state(TASK_INTERRUPTIBLE);
+                       schedule_timeout(usecs_to_jiffies(towait));
+               }
+
        }
 
 out:
diff --git a/drivers/media/rc/ir-spi.c b/drivers/media/rc/ir-spi.c
index fcda1e4..e44abfa 100644
--- a/drivers/media/rc/ir-spi.c
+++ b/drivers/media/rc/ir-spi.c
@@ -149,6 +149,7 @@ static int ir_spi_probe(struct spi_device *spi)
        if (!idata->rc)
                return -ENOMEM;
 
+       idata->rc->tx_no_wait      = true;
        idata->rc->tx_ir           = ir_spi_tx;
        idata->rc->s_tx_carrier    = ir_spi_set_tx_carrier;
        idata->rc->s_tx_duty_cycle = ir_spi_set_duty_cycle;
diff --git a/include/media/rc-core.h b/include/media/rc-core.h
index fe0c9c4..c3ced9b 100644
--- a/include/media/rc-core.h
+++ b/include/media/rc-core.h
@@ -85,6 +85,9 @@ enum rc_filter_type {
  * @input_dev: the input child device used to communicate events to userspace
  * @driver_type: specifies if protocol decoding is done in hardware or software
  * @idle: used to keep track of RX state
+ * @tx_no_wait: decides whether to perform or not a sync write or not. The
+ *      device driver setting it to true must make sure to not break the ABI
+ *      which requires a sync transfer.
  * @allowed_protocols: bitmask with the supported RC_BIT_* protocols
  * @enabled_protocols: bitmask with the enabled RC_BIT_* protocols
  * @allowed_wakeup_protocols: bitmask with the supported RC_BIT_* wakeup protocols
@@ -147,6 +150,7 @@ struct rc_dev {
        struct input_dev                *input_dev;
        enum rc_driver_type             driver_type;
        bool                            idle;
+       bool                            tx_no_wait;
        u64                             allowed_protocols;
        u64                             enabled_protocols;
        u64                             allowed_wakeup_protocols;

Thanks,
Andi
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* Re: [PATCH v2 5/7] [media] ir-lirc-codec: don't wait any transmitting time for tx only devices
  2016-11-01  6:51             ` Andi Shyti
  (?)
@ 2016-11-01 10:34             ` Sean Young
  2016-11-02  4:39               ` Andi Shyti
  -1 siblings, 1 reply; 33+ messages in thread
From: Sean Young @ 2016-11-01 10:34 UTC (permalink / raw)
  To: Andi Shyti
  Cc: David Härdeman, Mauro Carvalho Chehab, Rob Herring,
	Mark Rutland, linux-media, devicetree, linux-kernel, Andi Shyti


Hi Andi,

On Tue, Nov 01, 2016 at 03:51:11PM +0900, Andi Shyti wrote:
> > Andi, it would be good to know what the use-case for the original change is.
> 
> the use case is the ir-spi itself which doesn't need the lirc to
> perform any waiting on its behalf.

Here is the crux of the problem: in the ir-spi case no wait will actually 
happen here, and certainly no "over-wait". The patch below will not change
behaviour at all.

In the ir-spi case, "towait" will be 0 and no wait happens.

I think the code is already in good shape but somehow there is a 
misunderstanding. Did I miss something?


Sean

> To me it just doesn't look right to simulate a fake transmission
> period and wait unnecessary time. Of course, the "over-wait" is not
> a big deal and at the end we can decide to drop it.
> 
> Otherwise, an alternative could be to add the boolean
> 'tx_no_wait' in the rc_dev structure. It could be set by the
> device driver during the initialization and the we can follow
> your approach.
> 
> Something like this:
> 
> diff --git a/drivers/media/rc/ir-lirc-codec.c b/drivers/media/rc/ir-lirc-codec.c
> index c327730..4553d04 100644
> --- a/drivers/media/rc/ir-lirc-codec.c
> +++ b/drivers/media/rc/ir-lirc-codec.c
> @@ -161,15 +161,19 @@ static ssize_t ir_lirc_transmit_ir(struct file *file, const char __user *buf,
>  
>         ret *= sizeof(unsigned int);
>  
> -       /*
> -        * The lircd gap calculation expects the write function to
> -        * wait for the actual IR signal to be transmitted before
> -        * returning.
> -        */
> -       towait = ktime_us_delta(ktime_add_us(start, duration), ktime_get());
> -       if (towait > 0) {
> -               set_current_state(TASK_INTERRUPTIBLE);
> -               schedule_timeout(usecs_to_jiffies(towait));
> +       if (!dev->tx_no_wait) {
> +               /*
> +                * The lircd gap calculation expects the write function to
> +                * wait for the actual IR signal to be transmitted before
> +                * returning.
> +                */
> +               towait = ktime_us_delta(ktime_add_us(start, duration),
> +                                                               ktime_get());
> +               if (towait > 0) {
> +                       set_current_state(TASK_INTERRUPTIBLE);
> +                       schedule_timeout(usecs_to_jiffies(towait));
> +               }
> +
>         }
>  
>  out:
> diff --git a/drivers/media/rc/ir-spi.c b/drivers/media/rc/ir-spi.c
> index fcda1e4..e44abfa 100644
> --- a/drivers/media/rc/ir-spi.c
> +++ b/drivers/media/rc/ir-spi.c
> @@ -149,6 +149,7 @@ static int ir_spi_probe(struct spi_device *spi)
>         if (!idata->rc)
>                 return -ENOMEM;
>  
> +       idata->rc->tx_no_wait      = true;
>         idata->rc->tx_ir           = ir_spi_tx;
>         idata->rc->s_tx_carrier    = ir_spi_set_tx_carrier;
>         idata->rc->s_tx_duty_cycle = ir_spi_set_duty_cycle;
> diff --git a/include/media/rc-core.h b/include/media/rc-core.h
> index fe0c9c4..c3ced9b 100644
> --- a/include/media/rc-core.h
> +++ b/include/media/rc-core.h
> @@ -85,6 +85,9 @@ enum rc_filter_type {
>   * @input_dev: the input child device used to communicate events to userspace
>   * @driver_type: specifies if protocol decoding is done in hardware or software
>   * @idle: used to keep track of RX state
> + * @tx_no_wait: decides whether to perform or not a sync write or not. The
> + *      device driver setting it to true must make sure to not break the ABI
> + *      which requires a sync transfer.
>   * @allowed_protocols: bitmask with the supported RC_BIT_* protocols
>   * @enabled_protocols: bitmask with the enabled RC_BIT_* protocols
>   * @allowed_wakeup_protocols: bitmask with the supported RC_BIT_* wakeup protocols
> @@ -147,6 +150,7 @@ struct rc_dev {
>         struct input_dev                *input_dev;
>         enum rc_driver_type             driver_type;
>         bool                            idle;
> +       bool                            tx_no_wait;
>         u64                             allowed_protocols;
>         u64                             enabled_protocols;
>         u64                             allowed_wakeup_protocols;
> 
> Thanks,
> Andi
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* Re: [PATCH v2 5/7] [media] ir-lirc-codec: don't wait any transmitting time for tx only devices
  2016-11-01 10:34             ` Sean Young
@ 2016-11-02  4:39               ` Andi Shyti
  0 siblings, 0 replies; 33+ messages in thread
From: Andi Shyti @ 2016-11-02  4:39 UTC (permalink / raw)
  To: Sean Young
  Cc: David Härdeman, Mauro Carvalho Chehab, Rob Herring,
	Mark Rutland, linux-media, devicetree, linux-kernel, Andi Shyti

Hi Sean,

> > > Andi, it would be good to know what the use-case for the original change is.
> > 
> > the use case is the ir-spi itself which doesn't need the lirc to
> > perform any waiting on its behalf.
> 
> Here is the crux of the problem: in the ir-spi case no wait will actually 
> happen here, and certainly no "over-wait". The patch below will not change
> behaviour at all.
> 
> In the ir-spi case, "towait" will be 0 and no wait happens.
> 
> I think the code is already in good shape but somehow there is a 
> misunderstanding. Did I miss something?

We can just drop this patch, it's just something small that is
bothering me.

I will send a new patchset without this one.

Thanks,
Andi

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

end of thread, other threads:[~2016-11-02  4:39 UTC | newest]

Thread overview: 33+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-09-01 17:16 [PATCH v2 0/7] Add support for IR transmitters Andi Shyti
2016-09-01 17:16 ` Andi Shyti
2016-09-01 17:16 ` [PATCH v2 1/7] [media] rc-main: assign driver type during allocation Andi Shyti
2016-09-01 21:23   ` Sean Young
2016-09-02  5:25     ` Andi Shyti
2016-09-02  5:25       ` Andi Shyti
2016-09-01 17:16 ` [PATCH v2 2/7] [media] rc-main: split setup and unregister functions Andi Shyti
2016-09-01 17:16 ` [PATCH v2 3/7] [media] rc-core: add support for IR raw transmitters Andi Shyti
2016-09-01 21:31   ` Sean Young
2016-09-02  0:21   ` kbuild test robot
2016-09-02  0:21     ` kbuild test robot
2016-09-01 17:16 ` [PATCH v2 4/7] [media] rc-ir-raw: do not generate any receiving thread for " Andi Shyti
2016-09-01 17:16 ` [PATCH v2 5/7] [media] ir-lirc-codec: don't wait any transmitting time for tx only devices Andi Shyti
2016-09-02  8:41   ` Sean Young
2016-10-27  7:44     ` Andi Shyti
2016-10-27  7:44       ` Andi Shyti
2016-10-27 14:36       ` Sean Young
2016-10-31 14:31       ` David Härdeman
2016-10-31 17:05         ` Sean Young
2016-11-01  6:51           ` Andi Shyti
2016-11-01  6:51             ` Andi Shyti
2016-11-01 10:34             ` Sean Young
2016-11-02  4:39               ` Andi Shyti
2016-09-01 17:16 ` [PATCH v2 6/7] Documentation: bindings: add documentation for ir-spi device driver Andi Shyti
2016-09-01 21:40   ` Rob Herring
2016-09-02  5:33     ` Andi Shyti
2016-09-12 13:27       ` Rob Herring
2016-09-12 13:27         ` Rob Herring
2016-09-01 17:16 ` [PATCH v2 7/7] [media] rc: add support for IR LEDs driven through SPI Andi Shyti
2016-09-01 21:12   ` Sean Young
2016-09-02  5:27     ` Andi Shyti
2016-09-02  5:27       ` Andi Shyti
2016-09-02  8:43       ` Sean Young

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.