linux-media.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 00/10] media: rc: gpio-ir-recv: driver update
@ 2017-09-07 23:33 ` Ladislav Michl
  2017-09-07 23:34   ` [PATCH v2 01/10] media: rc: gpio-ir-recv: use helper vaiable to acess device info Ladislav Michl
                     ` (10 more replies)
  0 siblings, 11 replies; 16+ messages in thread
From: Ladislav Michl @ 2017-09-07 23:33 UTC (permalink / raw)
  To: linux-media; +Cc: Mauro Carvalho Chehab, Sean Young, Andi Shyti

This patch serie brings driver closer to recently used APIs
and removes no longer used gpio_ir_recv_platform_data
support.

It was done as an excercise before writing similar driver using
FIQ and hw timers as this one gives too imprecise timing.

Serie was rebased on top of current linux.git, but something
happened there and my userspace decoder no longer works: driver
reports completely bogus timing such as (rc-5):
^427, _1342, ^945, _183, ^1128, _671, ^1586, _91, ^1189, _1525,
^1738, _1433, ^915, _1159, ^1464, _1525, ^213, _1067, ^793, _0
(^ used for pulse and _ for space)
As it has nothing to do with my changes, I'm sending it anyway
for review, which I do not expect to happen until merge window
ends.

Ladislav Michl (10):
  media: rc: gpio-ir-recv: use helper vaiable to acess device info
  media: rc: gpio-ir-recv: use devm_kzalloc
  media: rc: gpio-ir-recv: use devm_rc_allocate_device
  media: rc: gpio-ir-recv: use devm_gpio_request_one
  media: rc: gpio-ir-recv: use devm_rc_register_device
  media: rc: gpio-ir-recv: do not allow threaded interrupt handler
  media: rc: gpio-ir-recv: use devm_request_irq
  media: rc: gpio-ir-recv: use KBUILD_MODNAME
  media: rc: gpio-ir-recv: remove gpio_ir_recv_platform_data
  media: rc: gpio-ir-recv: use gpiolib API

 drivers/media/rc/Kconfig                         |   1 +
 drivers/media/rc/gpio-ir-recv.c                  | 194 ++++++-----------------
 include/linux/platform_data/media/gpio-ir-recv.h |  23 ---
 3 files changed, 53 insertions(+), 165 deletions(-)
 delete mode 100644 include/linux/platform_data/media/gpio-ir-recv.h

-- 
2.11.0

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

* [PATCH v2 01/10] media: rc: gpio-ir-recv: use helper vaiable to acess device info
  2017-09-07 23:33 ` [PATCH v2 00/10] media: rc: gpio-ir-recv: driver update Ladislav Michl
@ 2017-09-07 23:34   ` Ladislav Michl
  2017-09-07 23:35   ` [PATCH v2 02/10] media: rc: gpio-ir-recv: use devm_kzalloc Ladislav Michl
                     ` (9 subsequent siblings)
  10 siblings, 0 replies; 16+ messages in thread
From: Ladislav Michl @ 2017-09-07 23:34 UTC (permalink / raw)
  To: linux-media; +Cc: Mauro Carvalho Chehab, Sean Young, Andi Shyti

Using explicit struct device variable makes code a bit more readable.

Signed-off-by: Ladislav Michl <ladis@linux-mips.org>
---
 Changes:
 -v2: rebased to current linux.git

 drivers/media/rc/gpio-ir-recv.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/media/rc/gpio-ir-recv.c b/drivers/media/rc/gpio-ir-recv.c
index 7248b3662285..741a68c192ce 100644
--- a/drivers/media/rc/gpio-ir-recv.c
+++ b/drivers/media/rc/gpio-ir-recv.c
@@ -95,18 +95,18 @@ static irqreturn_t gpio_ir_recv_irq(int irq, void *dev_id)
 
 static int gpio_ir_recv_probe(struct platform_device *pdev)
 {
+	struct device *dev = &pdev->dev;
 	struct gpio_rc_dev *gpio_dev;
 	struct rc_dev *rcdev;
-	const struct gpio_ir_recv_platform_data *pdata =
-					pdev->dev.platform_data;
+	const struct gpio_ir_recv_platform_data *pdata = dev->platform_data;
 	int rc;
 
 	if (pdev->dev.of_node) {
 		struct gpio_ir_recv_platform_data *dtpdata =
-			devm_kzalloc(&pdev->dev, sizeof(*dtpdata), GFP_KERNEL);
+			devm_kzalloc(dev, sizeof(*dtpdata), GFP_KERNEL);
 		if (!dtpdata)
 			return -ENOMEM;
-		rc = gpio_ir_recv_get_devtree_pdata(&pdev->dev, dtpdata);
+		rc = gpio_ir_recv_get_devtree_pdata(dev, dtpdata);
 		if (rc)
 			return rc;
 		pdata = dtpdata;
@@ -135,7 +135,7 @@ static int gpio_ir_recv_probe(struct platform_device *pdev)
 	rcdev->input_id.vendor = 0x0001;
 	rcdev->input_id.product = 0x0001;
 	rcdev->input_id.version = 0x0100;
-	rcdev->dev.parent = &pdev->dev;
+	rcdev->dev.parent = dev;
 	rcdev->driver_name = GPIO_IR_DRIVER_NAME;
 	rcdev->min_timeout = 1;
 	rcdev->timeout = IR_DEFAULT_TIMEOUT;
@@ -159,7 +159,7 @@ static int gpio_ir_recv_probe(struct platform_device *pdev)
 
 	rc = rc_register_device(rcdev);
 	if (rc < 0) {
-		dev_err(&pdev->dev, "failed to register rc device\n");
+		dev_err(dev, "failed to register rc device (%d)\n", rc);
 		goto err_register_rc_device;
 	}
 
-- 
2.11.0

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

* [PATCH v2 02/10] media: rc: gpio-ir-recv: use devm_kzalloc
  2017-09-07 23:33 ` [PATCH v2 00/10] media: rc: gpio-ir-recv: driver update Ladislav Michl
  2017-09-07 23:34   ` [PATCH v2 01/10] media: rc: gpio-ir-recv: use helper vaiable to acess device info Ladislav Michl
@ 2017-09-07 23:35   ` Ladislav Michl
  2017-09-07 23:36   ` [PATCH v2 03/10] media: rc: gpio-ir-recv: use devm_rc_allocate_device Ladislav Michl
                     ` (8 subsequent siblings)
  10 siblings, 0 replies; 16+ messages in thread
From: Ladislav Michl @ 2017-09-07 23:35 UTC (permalink / raw)
  To: linux-media; +Cc: Mauro Carvalho Chehab, Sean Young, Andi Shyti

Use of devm_kzalloc simplifies error unwinding.

Signed-off-by: Ladislav Michl <ladis@linux-mips.org>
---
 Changes:
 -v2: rebased to current linux.git

 drivers/media/rc/gpio-ir-recv.c | 11 +++--------
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/drivers/media/rc/gpio-ir-recv.c b/drivers/media/rc/gpio-ir-recv.c
index 741a68c192ce..8f5e3a84a95e 100644
--- a/drivers/media/rc/gpio-ir-recv.c
+++ b/drivers/media/rc/gpio-ir-recv.c
@@ -118,15 +118,13 @@ static int gpio_ir_recv_probe(struct platform_device *pdev)
 	if (pdata->gpio_nr < 0)
 		return -EINVAL;
 
-	gpio_dev = kzalloc(sizeof(struct gpio_rc_dev), GFP_KERNEL);
+	gpio_dev = devm_kzalloc(dev, sizeof(struct gpio_rc_dev), GFP_KERNEL);
 	if (!gpio_dev)
 		return -ENOMEM;
 
 	rcdev = rc_allocate_device(RC_DRIVER_IR_RAW);
-	if (!rcdev) {
-		rc = -ENOMEM;
-		goto err_allocate_device;
-	}
+	if (!rcdev)
+		return -ENOMEM;
 
 	rcdev->priv = gpio_dev;
 	rcdev->device_name = GPIO_IR_DEVICE_NAME;
@@ -182,8 +180,6 @@ static int gpio_ir_recv_probe(struct platform_device *pdev)
 	gpio_free(pdata->gpio_nr);
 err_gpio_request:
 	rc_free_device(rcdev);
-err_allocate_device:
-	kfree(gpio_dev);
 	return rc;
 }
 
@@ -194,7 +190,6 @@ static int gpio_ir_recv_remove(struct platform_device *pdev)
 	free_irq(gpio_to_irq(gpio_dev->gpio_nr), gpio_dev);
 	rc_unregister_device(gpio_dev->rcdev);
 	gpio_free(gpio_dev->gpio_nr);
-	kfree(gpio_dev);
 	return 0;
 }
 
-- 
2.11.0

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

* [PATCH v2 03/10] media: rc: gpio-ir-recv: use devm_rc_allocate_device
  2017-09-07 23:33 ` [PATCH v2 00/10] media: rc: gpio-ir-recv: driver update Ladislav Michl
  2017-09-07 23:34   ` [PATCH v2 01/10] media: rc: gpio-ir-recv: use helper vaiable to acess device info Ladislav Michl
  2017-09-07 23:35   ` [PATCH v2 02/10] media: rc: gpio-ir-recv: use devm_kzalloc Ladislav Michl
@ 2017-09-07 23:36   ` Ladislav Michl
  2017-09-07 23:36   ` [PATCH v2 04/10] media: rc: gpio-ir-recv: use devm_gpio_request_one Ladislav Michl
                     ` (7 subsequent siblings)
  10 siblings, 0 replies; 16+ messages in thread
From: Ladislav Michl @ 2017-09-07 23:36 UTC (permalink / raw)
  To: linux-media; +Cc: Mauro Carvalho Chehab, Sean Young, Andi Shyti

Use of devm_rc_allocate_device simplifies error unwinding.

Signed-off-by: Ladislav Michl <ladis@linux-mips.org>
---
 Changes:
 -v2: rebased to current linux.git

 drivers/media/rc/gpio-ir-recv.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/media/rc/gpio-ir-recv.c b/drivers/media/rc/gpio-ir-recv.c
index 8f5e3a84a95e..ee191f26efb4 100644
--- a/drivers/media/rc/gpio-ir-recv.c
+++ b/drivers/media/rc/gpio-ir-recv.c
@@ -122,7 +122,7 @@ static int gpio_ir_recv_probe(struct platform_device *pdev)
 	if (!gpio_dev)
 		return -ENOMEM;
 
-	rcdev = rc_allocate_device(RC_DRIVER_IR_RAW);
+	rcdev = devm_rc_allocate_device(dev, RC_DRIVER_IR_RAW);
 	if (!rcdev)
 		return -ENOMEM;
 
@@ -150,7 +150,7 @@ static int gpio_ir_recv_probe(struct platform_device *pdev)
 
 	rc = gpio_request(pdata->gpio_nr, "gpio-ir-recv");
 	if (rc < 0)
-		goto err_gpio_request;
+		return rc;
 	rc  = gpio_direction_input(pdata->gpio_nr);
 	if (rc < 0)
 		goto err_gpio_direction_input;
@@ -178,8 +178,6 @@ static int gpio_ir_recv_probe(struct platform_device *pdev)
 err_register_rc_device:
 err_gpio_direction_input:
 	gpio_free(pdata->gpio_nr);
-err_gpio_request:
-	rc_free_device(rcdev);
 	return rc;
 }
 
-- 
2.11.0

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

* [PATCH v2 04/10] media: rc: gpio-ir-recv: use devm_gpio_request_one
  2017-09-07 23:33 ` [PATCH v2 00/10] media: rc: gpio-ir-recv: driver update Ladislav Michl
                     ` (2 preceding siblings ...)
  2017-09-07 23:36   ` [PATCH v2 03/10] media: rc: gpio-ir-recv: use devm_rc_allocate_device Ladislav Michl
@ 2017-09-07 23:36   ` Ladislav Michl
  2017-09-07 23:37   ` [PATCH v2 05/10] media: rc: gpio-ir-recv: use devm_rc_register_device Ladislav Michl
                     ` (6 subsequent siblings)
  10 siblings, 0 replies; 16+ messages in thread
From: Ladislav Michl @ 2017-09-07 23:36 UTC (permalink / raw)
  To: linux-media; +Cc: Mauro Carvalho Chehab, Sean Young, Andi Shyti

Use of devm_gpio_request_one simplifies error unwinding.

Signed-off-by: Ladislav Michl <ladis@linux-mips.org>
---
 Changes:
 -v2: rebased to current linux.git

 drivers/media/rc/gpio-ir-recv.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/drivers/media/rc/gpio-ir-recv.c b/drivers/media/rc/gpio-ir-recv.c
index ee191f26efb4..5ce97b3612d6 100644
--- a/drivers/media/rc/gpio-ir-recv.c
+++ b/drivers/media/rc/gpio-ir-recv.c
@@ -148,12 +148,10 @@ static int gpio_ir_recv_probe(struct platform_device *pdev)
 	gpio_dev->gpio_nr = pdata->gpio_nr;
 	gpio_dev->active_low = pdata->active_low;
 
-	rc = gpio_request(pdata->gpio_nr, "gpio-ir-recv");
+	rc = devm_gpio_request_one(dev, pdata->gpio_nr, GPIOF_DIR_IN,
+					"gpio-ir-recv");
 	if (rc < 0)
 		return rc;
-	rc  = gpio_direction_input(pdata->gpio_nr);
-	if (rc < 0)
-		goto err_gpio_direction_input;
 
 	rc = rc_register_device(rcdev);
 	if (rc < 0) {
@@ -176,8 +174,6 @@ static int gpio_ir_recv_probe(struct platform_device *pdev)
 	rc_unregister_device(rcdev);
 	rcdev = NULL;
 err_register_rc_device:
-err_gpio_direction_input:
-	gpio_free(pdata->gpio_nr);
 	return rc;
 }
 
@@ -187,7 +183,6 @@ static int gpio_ir_recv_remove(struct platform_device *pdev)
 
 	free_irq(gpio_to_irq(gpio_dev->gpio_nr), gpio_dev);
 	rc_unregister_device(gpio_dev->rcdev);
-	gpio_free(gpio_dev->gpio_nr);
 	return 0;
 }
 
-- 
2.11.0

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

* [PATCH v2 05/10] media: rc: gpio-ir-recv: use devm_rc_register_device
  2017-09-07 23:33 ` [PATCH v2 00/10] media: rc: gpio-ir-recv: driver update Ladislav Michl
                     ` (3 preceding siblings ...)
  2017-09-07 23:36   ` [PATCH v2 04/10] media: rc: gpio-ir-recv: use devm_gpio_request_one Ladislav Michl
@ 2017-09-07 23:37   ` Ladislav Michl
  2017-09-07 23:37   ` [PATCH v2 06/10] media: rc: gpio-ir-recv: do not allow threaded interrupt handler Ladislav Michl
                     ` (5 subsequent siblings)
  10 siblings, 0 replies; 16+ messages in thread
From: Ladislav Michl @ 2017-09-07 23:37 UTC (permalink / raw)
  To: linux-media; +Cc: Mauro Carvalho Chehab, Sean Young, Andi Shyti

Use of devm_rc_register_device simplifies error unwinding.

Signed-off-by: Ladislav Michl <ladis@linux-mips.org>
---
 Changes:
 -v2: rebased to current linux.git

 drivers/media/rc/gpio-ir-recv.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/media/rc/gpio-ir-recv.c b/drivers/media/rc/gpio-ir-recv.c
index 5ce97b3612d6..733e4ed35078 100644
--- a/drivers/media/rc/gpio-ir-recv.c
+++ b/drivers/media/rc/gpio-ir-recv.c
@@ -153,10 +153,10 @@ static int gpio_ir_recv_probe(struct platform_device *pdev)
 	if (rc < 0)
 		return rc;
 
-	rc = rc_register_device(rcdev);
+	rc = devm_rc_register_device(dev, rcdev);
 	if (rc < 0) {
 		dev_err(dev, "failed to register rc device (%d)\n", rc);
-		goto err_register_rc_device;
+		return rc;
 	}
 
 	platform_set_drvdata(pdev, gpio_dev);
@@ -171,9 +171,6 @@ static int gpio_ir_recv_probe(struct platform_device *pdev)
 	return 0;
 
 err_request_irq:
-	rc_unregister_device(rcdev);
-	rcdev = NULL;
-err_register_rc_device:
 	return rc;
 }
 
@@ -182,7 +179,6 @@ static int gpio_ir_recv_remove(struct platform_device *pdev)
 	struct gpio_rc_dev *gpio_dev = platform_get_drvdata(pdev);
 
 	free_irq(gpio_to_irq(gpio_dev->gpio_nr), gpio_dev);
-	rc_unregister_device(gpio_dev->rcdev);
 	return 0;
 }
 
-- 
2.11.0

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

* [PATCH v2 06/10] media: rc: gpio-ir-recv: do not allow threaded interrupt handler
  2017-09-07 23:33 ` [PATCH v2 00/10] media: rc: gpio-ir-recv: driver update Ladislav Michl
                     ` (4 preceding siblings ...)
  2017-09-07 23:37   ` [PATCH v2 05/10] media: rc: gpio-ir-recv: use devm_rc_register_device Ladislav Michl
@ 2017-09-07 23:37   ` Ladislav Michl
  2017-09-07 23:38   ` [PATCH v2 07/10] media: rc: gpio-ir-recv: use devm_request_irq Ladislav Michl
                     ` (4 subsequent siblings)
  10 siblings, 0 replies; 16+ messages in thread
From: Ladislav Michl @ 2017-09-07 23:37 UTC (permalink / raw)
  To: linux-media; +Cc: Mauro Carvalho Chehab, Sean Young, Andi Shyti

Requesting any context irq is not actually great idea since threaded
interrupt handler is run at too unpredictable time which turns
timing information wrong. Fix it by requesting regular interrupt.

Signed-off-by: Ladislav Michl <ladis@linux-mips.org>
---
 Changes:
 -v2: rebased to current linux.git

 drivers/media/rc/gpio-ir-recv.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/rc/gpio-ir-recv.c b/drivers/media/rc/gpio-ir-recv.c
index 733e4ed35078..110276d49495 100644
--- a/drivers/media/rc/gpio-ir-recv.c
+++ b/drivers/media/rc/gpio-ir-recv.c
@@ -161,7 +161,7 @@ static int gpio_ir_recv_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, gpio_dev);
 
-	rc = request_any_context_irq(gpio_to_irq(pdata->gpio_nr),
+	rc = request_irq(gpio_to_irq(pdata->gpio_nr),
 				gpio_ir_recv_irq,
 			IRQF_TRIGGER_FALLING | IRQF_TRIGGER_RISING,
 					"gpio-ir-recv-irq", gpio_dev);
-- 
2.11.0

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

* [PATCH v2 07/10] media: rc: gpio-ir-recv: use devm_request_irq
  2017-09-07 23:33 ` [PATCH v2 00/10] media: rc: gpio-ir-recv: driver update Ladislav Michl
                     ` (5 preceding siblings ...)
  2017-09-07 23:37   ` [PATCH v2 06/10] media: rc: gpio-ir-recv: do not allow threaded interrupt handler Ladislav Michl
@ 2017-09-07 23:38   ` Ladislav Michl
  2017-09-07 23:39   ` [PATCH v2 08/10] media: rc: gpio-ir-recv: use KBUILD_MODNAME Ladislav Michl
                     ` (3 subsequent siblings)
  10 siblings, 0 replies; 16+ messages in thread
From: Ladislav Michl @ 2017-09-07 23:38 UTC (permalink / raw)
  To: linux-media; +Cc: Mauro Carvalho Chehab, Sean Young, Andi Shyti

Use of devm_request_irq simplifies error unwinding and as
free_irq was the last user of driver remove function,
remove it too.

Signed-off-by: Ladislav Michl <ladis@linux-mips.org>
---
 Changes:
 -v2: rebased to current linux.git

 drivers/media/rc/gpio-ir-recv.c | 22 +++-------------------
 1 file changed, 3 insertions(+), 19 deletions(-)

diff --git a/drivers/media/rc/gpio-ir-recv.c b/drivers/media/rc/gpio-ir-recv.c
index 110276d49495..1d115f8531ba 100644
--- a/drivers/media/rc/gpio-ir-recv.c
+++ b/drivers/media/rc/gpio-ir-recv.c
@@ -161,25 +161,10 @@ static int gpio_ir_recv_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, gpio_dev);
 
-	rc = request_irq(gpio_to_irq(pdata->gpio_nr),
+	return devm_request_irq(dev, gpio_to_irq(pdata->gpio_nr),
 				gpio_ir_recv_irq,
-			IRQF_TRIGGER_FALLING | IRQF_TRIGGER_RISING,
-					"gpio-ir-recv-irq", gpio_dev);
-	if (rc < 0)
-		goto err_request_irq;
-
-	return 0;
-
-err_request_irq:
-	return rc;
-}
-
-static int gpio_ir_recv_remove(struct platform_device *pdev)
-{
-	struct gpio_rc_dev *gpio_dev = platform_get_drvdata(pdev);
-
-	free_irq(gpio_to_irq(gpio_dev->gpio_nr), gpio_dev);
-	return 0;
+				IRQF_TRIGGER_FALLING | IRQF_TRIGGER_RISING,
+				"gpio-ir-recv-irq", gpio_dev);
 }
 
 #ifdef CONFIG_PM
@@ -217,7 +202,6 @@ static const struct dev_pm_ops gpio_ir_recv_pm_ops = {
 
 static struct platform_driver gpio_ir_recv_driver = {
 	.probe  = gpio_ir_recv_probe,
-	.remove = gpio_ir_recv_remove,
 	.driver = {
 		.name   = GPIO_IR_DRIVER_NAME,
 		.of_match_table = of_match_ptr(gpio_ir_recv_of_match),
-- 
2.11.0

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

* [PATCH v2 08/10] media: rc: gpio-ir-recv: use KBUILD_MODNAME
  2017-09-07 23:33 ` [PATCH v2 00/10] media: rc: gpio-ir-recv: driver update Ladislav Michl
                     ` (6 preceding siblings ...)
  2017-09-07 23:38   ` [PATCH v2 07/10] media: rc: gpio-ir-recv: use devm_request_irq Ladislav Michl
@ 2017-09-07 23:39   ` Ladislav Michl
  2017-09-07 23:39   ` [PATCH v2 09/10] media: rc: gpio-ir-recv: remove gpio_ir_recv_platform_data Ladislav Michl
                     ` (2 subsequent siblings)
  10 siblings, 0 replies; 16+ messages in thread
From: Ladislav Michl @ 2017-09-07 23:39 UTC (permalink / raw)
  To: linux-media; +Cc: Mauro Carvalho Chehab, Sean Young, Andi Shyti

There already is standard macro providing driver name, use it.

Signed-off-by: Ladislav Michl <ladis@linux-mips.org>
---
 Changes:
 -v2: rebased to current linux.git

 drivers/media/rc/gpio-ir-recv.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/media/rc/gpio-ir-recv.c b/drivers/media/rc/gpio-ir-recv.c
index 1d115f8531ba..db193ad4b819 100644
--- a/drivers/media/rc/gpio-ir-recv.c
+++ b/drivers/media/rc/gpio-ir-recv.c
@@ -23,7 +23,6 @@
 #include <media/rc-core.h>
 #include <linux/platform_data/media/gpio-ir-recv.h>
 
-#define GPIO_IR_DRIVER_NAME	"gpio-rc-recv"
 #define GPIO_IR_DEVICE_NAME	"gpio_ir_recv"
 
 struct gpio_rc_dev {
@@ -134,7 +133,7 @@ static int gpio_ir_recv_probe(struct platform_device *pdev)
 	rcdev->input_id.product = 0x0001;
 	rcdev->input_id.version = 0x0100;
 	rcdev->dev.parent = dev;
-	rcdev->driver_name = GPIO_IR_DRIVER_NAME;
+	rcdev->driver_name = KBUILD_MODNAME;
 	rcdev->min_timeout = 1;
 	rcdev->timeout = IR_DEFAULT_TIMEOUT;
 	rcdev->max_timeout = 10 * IR_DEFAULT_TIMEOUT;
@@ -203,7 +202,7 @@ static const struct dev_pm_ops gpio_ir_recv_pm_ops = {
 static struct platform_driver gpio_ir_recv_driver = {
 	.probe  = gpio_ir_recv_probe,
 	.driver = {
-		.name   = GPIO_IR_DRIVER_NAME,
+		.name   = KBUILD_MODNAME,
 		.of_match_table = of_match_ptr(gpio_ir_recv_of_match),
 #ifdef CONFIG_PM
 		.pm	= &gpio_ir_recv_pm_ops,
-- 
2.11.0

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

* [PATCH v2 09/10] media: rc: gpio-ir-recv: remove gpio_ir_recv_platform_data
  2017-09-07 23:33 ` [PATCH v2 00/10] media: rc: gpio-ir-recv: driver update Ladislav Michl
                     ` (7 preceding siblings ...)
  2017-09-07 23:39   ` [PATCH v2 08/10] media: rc: gpio-ir-recv: use KBUILD_MODNAME Ladislav Michl
@ 2017-09-07 23:39   ` Ladislav Michl
  2017-09-07 23:41   ` [PATCH v2 10/10] media: rc: gpio-ir-recv: use gpiolib API Ladislav Michl
  2017-09-08  2:21   ` [PATCH v2 00/10] media: rc: gpio-ir-recv: driver update Andi Shyti
  10 siblings, 0 replies; 16+ messages in thread
From: Ladislav Michl @ 2017-09-07 23:39 UTC (permalink / raw)
  To: linux-media; +Cc: Mauro Carvalho Chehab, Sean Young, Andi Shyti

gpio_ir_recv_platform_data are not used anywhere in kernel tree,
so remove it.

Signed-off-by: Ladislav Michl <ladis@linux-mips.org>
---
 Changes:
 -v2: rebased to current linux.git

 drivers/media/rc/gpio-ir-recv.c                  | 98 +++++++-----------------
 include/linux/platform_data/media/gpio-ir-recv.h | 23 ------
 2 files changed, 29 insertions(+), 92 deletions(-)

diff --git a/drivers/media/rc/gpio-ir-recv.c b/drivers/media/rc/gpio-ir-recv.c
index db193ad4b819..5c47661281e3 100644
--- a/drivers/media/rc/gpio-ir-recv.c
+++ b/drivers/media/rc/gpio-ir-recv.c
@@ -21,7 +21,6 @@
 #include <linux/platform_device.h>
 #include <linux/irq.h>
 #include <media/rc-core.h>
-#include <linux/platform_data/media/gpio-ir-recv.h>
 
 #define GPIO_IR_DEVICE_NAME	"gpio_ir_recv"
 
@@ -31,45 +30,6 @@ struct gpio_rc_dev {
 	bool active_low;
 };
 
-#ifdef CONFIG_OF
-/*
- * Translate OpenFirmware node properties into platform_data
- */
-static int gpio_ir_recv_get_devtree_pdata(struct device *dev,
-				  struct gpio_ir_recv_platform_data *pdata)
-{
-	struct device_node *np = dev->of_node;
-	enum of_gpio_flags flags;
-	int gpio;
-
-	gpio = of_get_gpio_flags(np, 0, &flags);
-	if (gpio < 0) {
-		if (gpio != -EPROBE_DEFER)
-			dev_err(dev, "Failed to get gpio flags (%d)\n", gpio);
-		return gpio;
-	}
-
-	pdata->gpio_nr = gpio;
-	pdata->active_low = (flags & OF_GPIO_ACTIVE_LOW);
-	/* probe() takes care of map_name == NULL or allowed_protos == 0 */
-	pdata->map_name = of_get_property(np, "linux,rc-map-name", NULL);
-	pdata->allowed_protos = 0;
-
-	return 0;
-}
-
-static const struct of_device_id gpio_ir_recv_of_match[] = {
-	{ .compatible = "gpio-ir-receiver", },
-	{ },
-};
-MODULE_DEVICE_TABLE(of, gpio_ir_recv_of_match);
-
-#else /* !CONFIG_OF */
-
-#define gpio_ir_recv_get_devtree_pdata(dev, pdata)	(-ENOSYS)
-
-#endif
-
 static irqreturn_t gpio_ir_recv_irq(int irq, void *dev_id)
 {
 	struct gpio_rc_dev *gpio_dev = dev_id;
@@ -94,33 +54,30 @@ static irqreturn_t gpio_ir_recv_irq(int irq, void *dev_id)
 
 static int gpio_ir_recv_probe(struct platform_device *pdev)
 {
-	struct device *dev = &pdev->dev;
-	struct gpio_rc_dev *gpio_dev;
-	struct rc_dev *rcdev;
-	const struct gpio_ir_recv_platform_data *pdata = dev->platform_data;
 	int rc;
+	enum of_gpio_flags flags;
+	struct rc_dev *rcdev;
+	struct gpio_rc_dev *gpio_dev;
+	struct device *dev = &pdev->dev;
+	struct device_node *np = dev->of_node;
 
-	if (pdev->dev.of_node) {
-		struct gpio_ir_recv_platform_data *dtpdata =
-			devm_kzalloc(dev, sizeof(*dtpdata), GFP_KERNEL);
-		if (!dtpdata)
-			return -ENOMEM;
-		rc = gpio_ir_recv_get_devtree_pdata(dev, dtpdata);
-		if (rc)
-			return rc;
-		pdata = dtpdata;
-	}
-
-	if (!pdata)
-		return -EINVAL;
-
-	if (pdata->gpio_nr < 0)
-		return -EINVAL;
+	if (!np)
+		return -ENODEV;
 
 	gpio_dev = devm_kzalloc(dev, sizeof(struct gpio_rc_dev), GFP_KERNEL);
 	if (!gpio_dev)
 		return -ENOMEM;
 
+	rc = of_get_gpio_flags(np, 0, &flags);
+	if (rc < 0) {
+		if (rc != -EPROBE_DEFER)
+			dev_err(dev, "Failed to get gpio flags (%d)\n", rc);
+		return rc;
+	}
+
+	gpio_dev->gpio_nr = rc;
+	gpio_dev->active_low = (flags & OF_GPIO_ACTIVE_LOW);
+
 	rcdev = devm_rc_allocate_device(dev, RC_DRIVER_IR_RAW);
 	if (!rcdev)
 		return -ENOMEM;
@@ -137,17 +94,14 @@ static int gpio_ir_recv_probe(struct platform_device *pdev)
 	rcdev->min_timeout = 1;
 	rcdev->timeout = IR_DEFAULT_TIMEOUT;
 	rcdev->max_timeout = 10 * IR_DEFAULT_TIMEOUT;
-	if (pdata->allowed_protos)
-		rcdev->allowed_protocols = pdata->allowed_protos;
-	else
-		rcdev->allowed_protocols = RC_PROTO_BIT_ALL_IR_DECODER;
-	rcdev->map_name = pdata->map_name ?: RC_MAP_EMPTY;
+	rcdev->allowed_protocols = RC_PROTO_BIT_ALL_IR_DECODER;
+	rcdev->map_name = of_get_property(np, "linux,rc-map-name", NULL);
+	if (!rcdev->map_name)
+		rcdev->map_name = RC_MAP_EMPTY;
 
 	gpio_dev->rcdev = rcdev;
-	gpio_dev->gpio_nr = pdata->gpio_nr;
-	gpio_dev->active_low = pdata->active_low;
 
-	rc = devm_gpio_request_one(dev, pdata->gpio_nr, GPIOF_DIR_IN,
+	rc = devm_gpio_request_one(dev, gpio_dev->gpio_nr, GPIOF_DIR_IN,
 					"gpio-ir-recv");
 	if (rc < 0)
 		return rc;
@@ -160,7 +114,7 @@ static int gpio_ir_recv_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, gpio_dev);
 
-	return devm_request_irq(dev, gpio_to_irq(pdata->gpio_nr),
+	return devm_request_irq(dev, gpio_to_irq(gpio_dev->gpio_nr),
 				gpio_ir_recv_irq,
 				IRQF_TRIGGER_FALLING | IRQF_TRIGGER_RISING,
 				"gpio-ir-recv-irq", gpio_dev);
@@ -199,6 +153,12 @@ static const struct dev_pm_ops gpio_ir_recv_pm_ops = {
 };
 #endif
 
+static const struct of_device_id gpio_ir_recv_of_match[] = {
+	{ .compatible = "gpio-ir-receiver", },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, gpio_ir_recv_of_match);
+
 static struct platform_driver gpio_ir_recv_driver = {
 	.probe  = gpio_ir_recv_probe,
 	.driver = {
diff --git a/include/linux/platform_data/media/gpio-ir-recv.h b/include/linux/platform_data/media/gpio-ir-recv.h
deleted file mode 100644
index 0c298f569d5a..000000000000
--- a/include/linux/platform_data/media/gpio-ir-recv.h
+++ /dev/null
@@ -1,23 +0,0 @@
-/* Copyright (c) 2012, Code Aurora Forum. All rights reserved.
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 2 and
- * only version 2 as published by the Free Software Foundation.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- */
-
-#ifndef __GPIO_IR_RECV_H__
-#define __GPIO_IR_RECV_H__
-
-struct gpio_ir_recv_platform_data {
-	int		gpio_nr;
-	bool		active_low;
-	u64		allowed_protos;
-	const char	*map_name;
-};
-
-#endif /* __GPIO_IR_RECV_H__ */
-- 
2.11.0

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

* [PATCH v2 10/10] media: rc: gpio-ir-recv: use gpiolib API
  2017-09-07 23:33 ` [PATCH v2 00/10] media: rc: gpio-ir-recv: driver update Ladislav Michl
                     ` (8 preceding siblings ...)
  2017-09-07 23:39   ` [PATCH v2 09/10] media: rc: gpio-ir-recv: remove gpio_ir_recv_platform_data Ladislav Michl
@ 2017-09-07 23:41   ` Ladislav Michl
  2017-09-08  2:21   ` [PATCH v2 00/10] media: rc: gpio-ir-recv: driver update Andi Shyti
  10 siblings, 0 replies; 16+ messages in thread
From: Ladislav Michl @ 2017-09-07 23:41 UTC (permalink / raw)
  To: linux-media; +Cc: Mauro Carvalho Chehab, Sean Young, Andi Shyti

Gpiolib API is preferred way to access gpios.

Signed-off-by: Ladislav Michl <ladis@linux-mips.org>
---
 Changes:
 -v2: rebased to current linux.git

 drivers/media/rc/Kconfig        |  1 +
 drivers/media/rc/gpio-ir-recv.c | 59 +++++++++++++++--------------------------
 2 files changed, 22 insertions(+), 38 deletions(-)

diff --git a/drivers/media/rc/Kconfig b/drivers/media/rc/Kconfig
index d9ce8ff55d0c..6bfe983ff295 100644
--- a/drivers/media/rc/Kconfig
+++ b/drivers/media/rc/Kconfig
@@ -393,6 +393,7 @@ config RC_LOOPBACK
 config IR_GPIO_CIR
 	tristate "GPIO IR remote control"
 	depends on RC_CORE
+	depends on (OF && GPIOLIB) || COMPILE_TEST
 	---help---
 	   Say Y if you want to use GPIO based IR Receiver.
 
diff --git a/drivers/media/rc/gpio-ir-recv.c b/drivers/media/rc/gpio-ir-recv.c
index 5c47661281e3..5bb0851eacce 100644
--- a/drivers/media/rc/gpio-ir-recv.c
+++ b/drivers/media/rc/gpio-ir-recv.c
@@ -26,36 +26,25 @@
 
 struct gpio_rc_dev {
 	struct rc_dev *rcdev;
-	int gpio_nr;
-	bool active_low;
+	struct gpio_desc *gpiod;
+	int irq;
 };
 
 static irqreturn_t gpio_ir_recv_irq(int irq, void *dev_id)
 {
+	int val;
 	struct gpio_rc_dev *gpio_dev = dev_id;
-	int gval;
-	int rc = 0;
 
-	gval = gpio_get_value(gpio_dev->gpio_nr);
+	val = gpiod_get_value(gpio_dev->gpiod);
+	if (val >= 0)
+		ir_raw_event_store_edge(gpio_dev->rcdev, val == 1);
 
-	if (gval < 0)
-		goto err_get_value;
-
-	if (gpio_dev->active_low)
-		gval = !gval;
-
-	rc = ir_raw_event_store_edge(gpio_dev->rcdev, gval == 1);
-	if (rc < 0)
-		goto err_get_value;
-
-err_get_value:
 	return IRQ_HANDLED;
 }
 
 static int gpio_ir_recv_probe(struct platform_device *pdev)
 {
 	int rc;
-	enum of_gpio_flags flags;
 	struct rc_dev *rcdev;
 	struct gpio_rc_dev *gpio_dev;
 	struct device *dev = &pdev->dev;
@@ -68,15 +57,17 @@ static int gpio_ir_recv_probe(struct platform_device *pdev)
 	if (!gpio_dev)
 		return -ENOMEM;
 
-	rc = of_get_gpio_flags(np, 0, &flags);
-	if (rc < 0) {
+	gpio_dev->gpiod = devm_gpiod_get(dev, NULL, GPIOD_IN);
+	if (IS_ERR(gpio_dev->gpiod)) {
+		rc = PTR_ERR(gpio_dev->gpiod);
+		/* Just try again if this happens */
 		if (rc != -EPROBE_DEFER)
-			dev_err(dev, "Failed to get gpio flags (%d)\n", rc);
+			dev_err(dev, "error getting gpio (%d)\n", rc);
 		return rc;
 	}
-
-	gpio_dev->gpio_nr = rc;
-	gpio_dev->active_low = (flags & OF_GPIO_ACTIVE_LOW);
+	gpio_dev->irq = gpiod_to_irq(gpio_dev->gpiod);
+	if (gpio_dev->irq < 0)
+		return gpio_dev->irq;
 
 	rcdev = devm_rc_allocate_device(dev, RC_DRIVER_IR_RAW);
 	if (!rcdev)
@@ -101,11 +92,6 @@ static int gpio_ir_recv_probe(struct platform_device *pdev)
 
 	gpio_dev->rcdev = rcdev;
 
-	rc = devm_gpio_request_one(dev, gpio_dev->gpio_nr, GPIOF_DIR_IN,
-					"gpio-ir-recv");
-	if (rc < 0)
-		return rc;
-
 	rc = devm_rc_register_device(dev, rcdev);
 	if (rc < 0) {
 		dev_err(dev, "failed to register rc device (%d)\n", rc);
@@ -114,8 +100,7 @@ static int gpio_ir_recv_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, gpio_dev);
 
-	return devm_request_irq(dev, gpio_to_irq(gpio_dev->gpio_nr),
-				gpio_ir_recv_irq,
+	return devm_request_irq(dev, gpio_dev->irq, gpio_ir_recv_irq,
 				IRQF_TRIGGER_FALLING | IRQF_TRIGGER_RISING,
 				"gpio-ir-recv-irq", gpio_dev);
 }
@@ -123,26 +108,24 @@ static int gpio_ir_recv_probe(struct platform_device *pdev)
 #ifdef CONFIG_PM
 static int gpio_ir_recv_suspend(struct device *dev)
 {
-	struct platform_device *pdev = to_platform_device(dev);
-	struct gpio_rc_dev *gpio_dev = platform_get_drvdata(pdev);
+	struct gpio_rc_dev *gpio_dev = dev_get_drvdata(dev);
 
 	if (device_may_wakeup(dev))
-		enable_irq_wake(gpio_to_irq(gpio_dev->gpio_nr));
+		enable_irq_wake(gpio_dev->irq);
 	else
-		disable_irq(gpio_to_irq(gpio_dev->gpio_nr));
+		disable_irq(gpio_dev->irq);
 
 	return 0;
 }
 
 static int gpio_ir_recv_resume(struct device *dev)
 {
-	struct platform_device *pdev = to_platform_device(dev);
-	struct gpio_rc_dev *gpio_dev = platform_get_drvdata(pdev);
+	struct gpio_rc_dev *gpio_dev = dev_get_drvdata(dev);
 
 	if (device_may_wakeup(dev))
-		disable_irq_wake(gpio_to_irq(gpio_dev->gpio_nr));
+		disable_irq_wake(gpio_dev->irq);
 	else
-		enable_irq(gpio_to_irq(gpio_dev->gpio_nr));
+		enable_irq(gpio_dev->irq);
 
 	return 0;
 }
-- 
2.11.0

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

* Re: [PATCH v2 00/10] media: rc: gpio-ir-recv: driver update
  2017-09-07 23:33 ` [PATCH v2 00/10] media: rc: gpio-ir-recv: driver update Ladislav Michl
                     ` (9 preceding siblings ...)
  2017-09-07 23:41   ` [PATCH v2 10/10] media: rc: gpio-ir-recv: use gpiolib API Ladislav Michl
@ 2017-09-08  2:21   ` Andi Shyti
  2017-09-08  8:16     ` Ladislav Michl
  10 siblings, 1 reply; 16+ messages in thread
From: Andi Shyti @ 2017-09-08  2:21 UTC (permalink / raw)
  To: Ladislav Michl; +Cc: linux-media, Mauro Carvalho Chehab, Sean Young

Hi Ladislav,

> Serie was rebased on top of current linux.git, but something
> happened there and my userspace decoder no longer works: driver
> reports completely bogus timing such as (rc-5):
> ^427, _1342, ^945, _183, ^1128, _671, ^1586, _91, ^1189, _1525,
> ^1738, _1433, ^915, _1159, ^1464, _1525, ^213, _1067, ^793, _0
> (^ used for pulse and _ for space)
> As it has nothing to do with my changes, I'm sending it anyway
> for review, which I do not expect to happen until merge window
> ends.

This means that your patch is anyway untested.

In any case I don't see much use if patch 1/10 as it doesn't
simplify much, but the rest (from 2 to 10) looks good to me.

Once it's tested you can add

Acked-by: Andi Shyti <andi.shyti@samsung.com>

Andi

P.S. I don't see in this V2 the changelog from V1. Next time,
please add the changelog.

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

* Re: [PATCH v2 00/10] media: rc: gpio-ir-recv: driver update
  2017-09-08  2:21   ` [PATCH v2 00/10] media: rc: gpio-ir-recv: driver update Andi Shyti
@ 2017-09-08  8:16     ` Ladislav Michl
  2017-09-11  2:58       ` Andi Shyti
  0 siblings, 1 reply; 16+ messages in thread
From: Ladislav Michl @ 2017-09-08  8:16 UTC (permalink / raw)
  To: Andi Shyti; +Cc: linux-media, Mauro Carvalho Chehab, Sean Young

Hi Andi,

On Fri, Sep 08, 2017 at 11:21:10AM +0900, Andi Shyti wrote:
> Hi Ladislav,
> 
> > Serie was rebased on top of current linux.git, but something
> > happened there and my userspace decoder no longer works: driver
> > reports completely bogus timing such as (rc-5):
> > ^427, _1342, ^945, _183, ^1128, _671, ^1586, _91, ^1189, _1525,
> > ^1738, _1433, ^915, _1159, ^1464, _1525, ^213, _1067, ^793, _0
> > (^ used for pulse and _ for space)
> > As it has nothing to do with my changes, I'm sending it anyway
> > for review, which I do not expect to happen until merge window
> > ends.
> 
> This means that your patch is anyway untested.

Previous version is pretty well tested. GPIO IR stopped working
after pulling other changes from linux.git yesterday. And does not
work even without this patch set. I'll try to bisect later as omiting
linux-media merge did not fix it either.

> In any case I don't see much use if patch 1/10 as it doesn't
> simplify much, but the rest (from 2 to 10) looks good to me.

Just look at patch 9 and imagine how it would look without this
change. If you are still unconvinced I'll drop this change.

> Once it's tested you can add
> 
> Acked-by: Andi Shyti <andi.shyti@samsung.com>
> 
> Andi
> 
> P.S. I don't see in this V2 the changelog from V1. Next time,
> please add the changelog.

It was just a rebase with conflicts resolved. I do not see how
to describe it better than I did.

	ladis

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

* Re: [PATCH v2 00/10] media: rc: gpio-ir-recv: driver update
  2017-09-08  8:16     ` Ladislav Michl
@ 2017-09-11  2:58       ` Andi Shyti
  2017-09-11  7:13         ` Ladislav Michl
  0 siblings, 1 reply; 16+ messages in thread
From: Andi Shyti @ 2017-09-11  2:58 UTC (permalink / raw)
  To: Ladislav Michl; +Cc: linux-media, Mauro Carvalho Chehab, Sean Young

Hi Ladislav,

> > > Serie was rebased on top of current linux.git, but something
> > > happened there and my userspace decoder no longer works: driver
> > > reports completely bogus timing such as (rc-5):
> > > ^427, _1342, ^945, _183, ^1128, _671, ^1586, _91, ^1189, _1525,
> > > ^1738, _1433, ^915, _1159, ^1464, _1525, ^213, _1067, ^793, _0
> > > (^ used for pulse and _ for space)
> > > As it has nothing to do with my changes, I'm sending it anyway
> > > for review, which I do not expect to happen until merge window
> > > ends.
> > 
> > This means that your patch is anyway untested.
> 
> Previous version is pretty well tested. GPIO IR stopped working
> after pulling other changes from linux.git yesterday. And does not
> work even without this patch set. I'll try to bisect later as omiting
> linux-media merge did not fix it either.

OK

> > In any case I don't see much use if patch 1/10 as it doesn't
> > simplify much, but the rest (from 2 to 10) looks good to me.
> 
> Just look at patch 9 and imagine how it would look without this
> change. If you are still unconvinced I'll drop this change.

You don't need to, that's just my personal taste and I'm not
strong with it. Patch 1 is quite common and not a big deal anyway.
If you like it you like you can leave it :)

> > P.S. I don't see in this V2 the changelog from V1. Next time,
> > please add the changelog.
> 
> It was just a rebase with conflicts resolved. I do not see how
> to describe it better than I did.

You could write the above (i.e. V2 fixed rebase conflicts :) ).
The reason is that as no change is stated, I have to anyway, as
reviewer, compare side by side all patches to figure out if there
is any difference (even if small) that is not expected.

Thanks,
Andi

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

* Re: [PATCH v2 00/10] media: rc: gpio-ir-recv: driver update
  2017-09-11  2:58       ` Andi Shyti
@ 2017-09-11  7:13         ` Ladislav Michl
  2017-09-15 21:56           ` Ladislav Michl
  0 siblings, 1 reply; 16+ messages in thread
From: Ladislav Michl @ 2017-09-11  7:13 UTC (permalink / raw)
  To: Andi Shyti; +Cc: linux-media, Mauro Carvalho Chehab, Sean Young

On Mon, Sep 11, 2017 at 11:58:43AM +0900, Andi Shyti wrote:
> Hi Ladislav,
> 
> > > > Serie was rebased on top of current linux.git, but something
> > > > happened there and my userspace decoder no longer works: driver
> > > > reports completely bogus timing such as (rc-5):
> > > > ^427, _1342, ^945, _183, ^1128, _671, ^1586, _91, ^1189, _1525,
> > > > ^1738, _1433, ^915, _1159, ^1464, _1525, ^213, _1067, ^793, _0
> > > > (^ used for pulse and _ for space)
> > > > As it has nothing to do with my changes, I'm sending it anyway
> > > > for review, which I do not expect to happen until merge window
> > > > ends.
> > > 
> > > This means that your patch is anyway untested.
> > 
> > Previous version is pretty well tested. GPIO IR stopped working
> > after pulling other changes from linux.git yesterday. And does not
> > work even without this patch set. I'll try to bisect later as omiting
> > linux-media merge did not fix it either.
> 
> OK

In all truth, changes were developed on top of 4.9.40, testing was done at
customer's site and for generally usefull changes usual attempt for merge
was done :-) I tried unmodified driver over weekend:
4.9.0: Not OK
4.9.40: OK
4.9.13: Not OK
linux.git: Not OK
Tested on IGEPv2 board based on DM3730, so something went wrong again.
Based on driver principle I suspect either ktime_get returning "strange"
values or interrupts are broken again (but that does not explain those
short pulses and spaces):
https://www.spinics.net/lists/linux-omap/msg135915.html
Verifying that would require some test setup with signal generator and
scope, as described in said email thread; or combine that test driver
with this one and look what is really happening. Unfortunately I'm not
able to do this any time soon, so if someone has hardware reliably working
with current mainline and is willing to test this patch serie I would
be very happy to see it happen.

> > > In any case I don't see much use if patch 1/10 as it doesn't
> > > simplify much, but the rest (from 2 to 10) looks good to me.
> > 
> > Just look at patch 9 and imagine how it would look without this
> > change. If you are still unconvinced I'll drop this change.
> 
> You don't need to, that's just my personal taste and I'm not
> strong with it. Patch 1 is quite common and not a big deal anyway.
> If you like it you like you can leave it :)
> 
> > > P.S. I don't see in this V2 the changelog from V1. Next time,
> > > please add the changelog.
> > 
> > It was just a rebase with conflicts resolved. I do not see how
> > to describe it better than I did.
> 
> You could write the above (i.e. V2 fixed rebase conflicts :) ).
> The reason is that as no change is stated, I have to anyway, as
> reviewer, compare side by side all patches to figure out if there
> is any difference (even if small) that is not expected.

Fair enough. Let's wait if there will any more comments and I'll do
better in V3.

> Thanks,
> Andi

Best regards,
	ladis

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

* Re: [PATCH v2 00/10] media: rc: gpio-ir-recv: driver update
  2017-09-11  7:13         ` Ladislav Michl
@ 2017-09-15 21:56           ` Ladislav Michl
  0 siblings, 0 replies; 16+ messages in thread
From: Ladislav Michl @ 2017-09-15 21:56 UTC (permalink / raw)
  To: Andi Shyti; +Cc: linux-media, Mauro Carvalho Chehab, Sean Young

On Mon, Sep 11, 2017 at 09:13:32AM +0200, Ladislav Michl wrote:
> On Mon, Sep 11, 2017 at 11:58:43AM +0900, Andi Shyti wrote:
> > Hi Ladislav,
> > 
> > > > > Serie was rebased on top of current linux.git, but something
> > > > > happened there and my userspace decoder no longer works: driver
> > > > > reports completely bogus timing such as (rc-5):
> > > > > ^427, _1342, ^945, _183, ^1128, _671, ^1586, _91, ^1189, _1525,
> > > > > ^1738, _1433, ^915, _1159, ^1464, _1525, ^213, _1067, ^793, _0
> > > > > (^ used for pulse and _ for space)
> > > > > As it has nothing to do with my changes, I'm sending it anyway
> > > > > for review, which I do not expect to happen until merge window
> > > > > ends.
> > > > 
> > > > This means that your patch is anyway untested.
> > > 
> > > Previous version is pretty well tested. GPIO IR stopped working
> > > after pulling other changes from linux.git yesterday. And does not
> > > work even without this patch set. I'll try to bisect later as omiting
> > > linux-media merge did not fix it either.
> > 
> > OK
> 
> In all truth, changes were developed on top of 4.9.40, testing was done at
> customer's site and for generally usefull changes usual attempt for merge
> was done :-) I tried unmodified driver over weekend:
> 4.9.0: Not OK
> 4.9.40: OK
> 4.9.13: Not OK
> linux.git: Not OK
> Tested on IGEPv2 board based on DM3730, so something went wrong again.
> Based on driver principle I suspect either ktime_get returning "strange"
> values or interrupts are broken again (but that does not explain those
> short pulses and spaces):
> https://www.spinics.net/lists/linux-omap/msg135915.html
> Verifying that would require some test setup with signal generator and
> scope, as described in said email thread; or combine that test driver
> with this one and look what is really happening. Unfortunately I'm not
> able to do this any time soon, so if someone has hardware reliably working
> with current mainline and is willing to test this patch serie I would
> be very happy to see it happen.

So, turns out to be GPIO edge interrupt handling bug on OMAPs.
Proper fix still needs to be investigated, but it does not affect
this patch serie.

> > > > In any case I don't see much use if patch 1/10 as it doesn't
> > > > simplify much, but the rest (from 2 to 10) looks good to me.
> > > 
> > > Just look at patch 9 and imagine how it would look without this
> > > change. If you are still unconvinced I'll drop this change.
> > 
> > You don't need to, that's just my personal taste and I'm not
> > strong with it. Patch 1 is quite common and not a big deal anyway.
> > If you like it you like you can leave it :)
> > 
> > > > P.S. I don't see in this V2 the changelog from V1. Next time,
> > > > please add the changelog.
> > > 
> > > It was just a rebase with conflicts resolved. I do not see how
> > > to describe it better than I did.
> > 
> > You could write the above (i.e. V2 fixed rebase conflicts :) ).
> > The reason is that as no change is stated, I have to anyway, as
> > reviewer, compare side by side all patches to figure out if there
> > is any difference (even if small) that is not expected.
> 
> Fair enough. Let's wait if there will any more comments and I'll do
> better in V3.
> 
> > Thanks,
> > Andi
> 
> Best regards,
> 	ladis

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

end of thread, other threads:[~2017-09-15 21:56 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <CGME20170907233401epcas4p4424e892b32d469233705af5014e20604@epcas4p4.samsung.com>
2017-09-07 23:33 ` [PATCH v2 00/10] media: rc: gpio-ir-recv: driver update Ladislav Michl
2017-09-07 23:34   ` [PATCH v2 01/10] media: rc: gpio-ir-recv: use helper vaiable to acess device info Ladislav Michl
2017-09-07 23:35   ` [PATCH v2 02/10] media: rc: gpio-ir-recv: use devm_kzalloc Ladislav Michl
2017-09-07 23:36   ` [PATCH v2 03/10] media: rc: gpio-ir-recv: use devm_rc_allocate_device Ladislav Michl
2017-09-07 23:36   ` [PATCH v2 04/10] media: rc: gpio-ir-recv: use devm_gpio_request_one Ladislav Michl
2017-09-07 23:37   ` [PATCH v2 05/10] media: rc: gpio-ir-recv: use devm_rc_register_device Ladislav Michl
2017-09-07 23:37   ` [PATCH v2 06/10] media: rc: gpio-ir-recv: do not allow threaded interrupt handler Ladislav Michl
2017-09-07 23:38   ` [PATCH v2 07/10] media: rc: gpio-ir-recv: use devm_request_irq Ladislav Michl
2017-09-07 23:39   ` [PATCH v2 08/10] media: rc: gpio-ir-recv: use KBUILD_MODNAME Ladislav Michl
2017-09-07 23:39   ` [PATCH v2 09/10] media: rc: gpio-ir-recv: remove gpio_ir_recv_platform_data Ladislav Michl
2017-09-07 23:41   ` [PATCH v2 10/10] media: rc: gpio-ir-recv: use gpiolib API Ladislav Michl
2017-09-08  2:21   ` [PATCH v2 00/10] media: rc: gpio-ir-recv: driver update Andi Shyti
2017-09-08  8:16     ` Ladislav Michl
2017-09-11  2:58       ` Andi Shyti
2017-09-11  7:13         ` Ladislav Michl
2017-09-15 21:56           ` Ladislav Michl

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).