[PATCH 0/5] media: rc: meson-ir: series with smaller improvements

[PATCH 0/5] media: rc: meson-ir: series with smaller improvements

[Heiner Kallweit]

This series includes smaller improvements for the meson-ir driver.
Tested on a Odroid C2.

Heiner Kallweit (5):
  media: rc: meson-ir: remove irq from struct meson_ir
  media: rc: meson-ir: make use of the bitfield macros
  media: rc: meson-ir: switch to managed rc device allocation / registration
  media: rc: meson-ir: use readl_relaxed in the interrupt handler
  media: rc: meson-ir: change irq name to to of node name

 drivers/media/rc/meson-ir.c | 64 ++++++++++++++++++---------------------------
 1 file changed, 26 insertions(+), 38 deletions(-)

-- 
2.12.2

[PATCH 0/5] media: rc: meson-ir: series with smaller improvements

[Heiner Kallweit]

This series includes smaller improvements for the meson-ir driver.
Tested on a Odroid C2.

Heiner Kallweit (5):
  media: rc: meson-ir: remove irq from struct meson_ir
  media: rc: meson-ir: make use of the bitfield macros
  media: rc: meson-ir: switch to managed rc device allocation / registration
  media: rc: meson-ir: use readl_relaxed in the interrupt handler
  media: rc: meson-ir: change irq name to to of node name

 drivers/media/rc/meson-ir.c | 64 ++++++++++++++++++---------------------------
 1 file changed, 26 insertions(+), 38 deletions(-)

-- 
2.12.2

[PATCH 1/5] media: rc: meson-ir: remove irq from struct meson_ir

[Heiner Kallweit]

The irq number is used in the probe function only, therefore just use
a local variable.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/media/rc/meson-ir.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/media/rc/meson-ir.c b/drivers/media/rc/meson-ir.c
index 5576dbd6..a4128d7c 100644
--- a/drivers/media/rc/meson-ir.c
+++ b/drivers/media/rc/meson-ir.c
@@ -68,7 +68,6 @@
 struct meson_ir {
 	void __iomem	*reg;
 	struct rc_dev	*rc;
-	int		irq;
 	spinlock_t	lock;
 };
 
@@ -112,7 +111,7 @@ static int meson_ir_probe(struct platform_device *pdev)
 	struct resource *res;
 	const char *map_name;
 	struct meson_ir *ir;
-	int ret;
+	int irq, ret;
 
 	ir = devm_kzalloc(dev, sizeof(struct meson_ir), GFP_KERNEL);
 	if (!ir)
@@ -125,10 +124,10 @@ static int meson_ir_probe(struct platform_device *pdev)
 		return PTR_ERR(ir->reg);
 	}
 
-	ir->irq = platform_get_irq(pdev, 0);
-	if (ir->irq < 0) {
+	irq = platform_get_irq(pdev, 0);
+	if (irq < 0) {
 		dev_err(dev, "no irq resource\n");
-		return ir->irq;
+		return irq;
 	}
 
 	ir->rc = rc_allocate_device(RC_DRIVER_IR_RAW);
@@ -158,7 +157,7 @@ static int meson_ir_probe(struct platform_device *pdev)
 		goto out_free;
 	}
 
-	ret = devm_request_irq(dev, ir->irq, meson_ir_irq, 0, "ir-meson", ir);
+	ret = devm_request_irq(dev, irq, meson_ir_irq, 0, "ir-meson", ir);
 	if (ret) {
 		dev_err(dev, "failed to request irq\n");
 		goto out_unreg;
-- 
2.12.2

[PATCH 1/5] media: rc: meson-ir: remove irq from struct meson_ir

[Heiner Kallweit]

The irq number is used in the probe function only, therefore just use
a local variable.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/media/rc/meson-ir.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/media/rc/meson-ir.c b/drivers/media/rc/meson-ir.c
index 5576dbd6..a4128d7c 100644
--- a/drivers/media/rc/meson-ir.c
+++ b/drivers/media/rc/meson-ir.c
@@ -68,7 +68,6 @@
 struct meson_ir {
 	void __iomem	*reg;
 	struct rc_dev	*rc;
-	int		irq;
 	spinlock_t	lock;
 };
 
@@ -112,7 +111,7 @@ static int meson_ir_probe(struct platform_device *pdev)
 	struct resource *res;
 	const char *map_name;
 	struct meson_ir *ir;
-	int ret;
+	int irq, ret;
 
 	ir = devm_kzalloc(dev, sizeof(struct meson_ir), GFP_KERNEL);
 	if (!ir)
@@ -125,10 +124,10 @@ static int meson_ir_probe(struct platform_device *pdev)
 		return PTR_ERR(ir->reg);
 	}
 
-	ir->irq = platform_get_irq(pdev, 0);
-	if (ir->irq < 0) {
+	irq = platform_get_irq(pdev, 0);
+	if (irq < 0) {
 		dev_err(dev, "no irq resource\n");
-		return ir->irq;
+		return irq;
 	}
 
 	ir->rc = rc_allocate_device(RC_DRIVER_IR_RAW);
@@ -158,7 +157,7 @@ static int meson_ir_probe(struct platform_device *pdev)
 		goto out_free;
 	}
 
-	ret = devm_request_irq(dev, ir->irq, meson_ir_irq, 0, "ir-meson", ir);
+	ret = devm_request_irq(dev, irq, meson_ir_irq, 0, "ir-meson", ir);
 	if (ret) {
 		dev_err(dev, "failed to request irq\n");
 		goto out_unreg;
-- 
2.12.2

[PATCH 2/5] media: rc: meson-ir: make use of the bitfield macros

[Heiner Kallweit]

Make use of the bitfield macros thus partially hiding the complexity
of dealing with bitfields.

The patch also includes a minor fix to REG0_RATE_MASK, so far it was
set to bit 0..10, but according to the spec it's bit 0..11.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/media/rc/meson-ir.c | 28 +++++++++++++---------------
 1 file changed, 13 insertions(+), 15 deletions(-)

diff --git a/drivers/media/rc/meson-ir.c b/drivers/media/rc/meson-ir.c
index a4128d7c..a52036c5 100644
--- a/drivers/media/rc/meson-ir.c
+++ b/drivers/media/rc/meson-ir.c
@@ -19,6 +19,7 @@
 #include <linux/of_platform.h>
 #include <linux/platform_device.h>
 #include <linux/spinlock.h>
+#include <linux/bitfield.h>
 
 #include <media/rc-core.h>
 
@@ -36,27 +37,24 @@
 /* only available on Meson 8b and newer */
 #define IR_DEC_REG2		0x20
 
-#define REG0_RATE_MASK		(BIT(11) - 1)
+#define REG0_RATE_MASK		GENMASK(11, 0)
 
 #define DECODE_MODE_NEC		0x0
 #define DECODE_MODE_RAW		0x2
 
 /* Meson 6b uses REG1 to configure the mode */
 #define REG1_MODE_MASK		GENMASK(8, 7)
-#define REG1_MODE_SHIFT		7
 
 /* Meson 8b / GXBB use REG2 to configure the mode */
 #define REG2_MODE_MASK		GENMASK(3, 0)
-#define REG2_MODE_SHIFT		0
 
-#define REG1_TIME_IV_SHIFT	16
-#define REG1_TIME_IV_MASK	((BIT(13) - 1) << REG1_TIME_IV_SHIFT)
+#define REG1_TIME_IV_MASK	GENMASK(28, 16)
 
-#define REG1_IRQSEL_MASK	(BIT(2) | BIT(3))
-#define REG1_IRQSEL_NEC_MODE	(0 << 2)
-#define REG1_IRQSEL_RISE_FALL	(1 << 2)
-#define REG1_IRQSEL_FALL	(2 << 2)
-#define REG1_IRQSEL_RISE	(3 << 2)
+#define REG1_IRQSEL_MASK	GENMASK(3, 2)
+#define REG1_IRQSEL_NEC_MODE	0
+#define REG1_IRQSEL_RISE_FALL	1
+#define REG1_IRQSEL_FALL	2
+#define REG1_IRQSEL_RISE	3
 
 #define REG1_RESET		BIT(0)
 #define REG1_ENABLE		BIT(15)
@@ -78,7 +76,7 @@ static void meson_ir_set_mask(struct meson_ir *ir, unsigned int reg,
 
 	data = readl(ir->reg + reg);
 	data &= ~mask;
-	data |= (value & mask);
+	data |= value;
 	writel(data, ir->reg + reg);
 }
 
@@ -91,7 +89,7 @@ static irqreturn_t meson_ir_irq(int irqno, void *dev_id)
 	spin_lock(&ir->lock);
 
 	duration = readl(ir->reg + IR_DEC_REG1);
-	duration = (duration & REG1_TIME_IV_MASK) >> REG1_TIME_IV_SHIFT;
+	duration = FIELD_GET(REG1_TIME_IV_MASK, duration);
 	rawir.duration = US_TO_NS(duration * MESON_TRATE);
 
 	rawir.pulse = !!(readl(ir->reg + IR_DEC_STATUS) & STATUS_IR_DEC_IN);
@@ -170,16 +168,16 @@ static int meson_ir_probe(struct platform_device *pdev)
 	/* Set general operation mode (= raw/software decoding) */
 	if (of_device_is_compatible(node, "amlogic,meson6-ir"))
 		meson_ir_set_mask(ir, IR_DEC_REG1, REG1_MODE_MASK,
-				  DECODE_MODE_RAW << REG1_MODE_SHIFT);
+				  FIELD_PREP(REG1_MODE_MASK, DECODE_MODE_RAW));
 	else
 		meson_ir_set_mask(ir, IR_DEC_REG2, REG2_MODE_MASK,
-				  DECODE_MODE_RAW << REG2_MODE_SHIFT);
+				  FIELD_PREP(REG2_MODE_MASK, DECODE_MODE_RAW));
 
 	/* Set rate */
 	meson_ir_set_mask(ir, IR_DEC_REG0, REG0_RATE_MASK, MESON_TRATE - 1);
 	/* IRQ on rising and falling edges */
 	meson_ir_set_mask(ir, IR_DEC_REG1, REG1_IRQSEL_MASK,
-			  REG1_IRQSEL_RISE_FALL);
+			  FIELD_PREP(REG1_IRQSEL_MASK, REG1_IRQSEL_RISE_FALL));
 	/* Enable the decoder */
 	meson_ir_set_mask(ir, IR_DEC_REG1, REG1_ENABLE, REG1_ENABLE);
 
-- 
2.12.2

[PATCH 2/5] media: rc: meson-ir: make use of the bitfield macros

[Heiner Kallweit]

Make use of the bitfield macros thus partially hiding the complexity
of dealing with bitfields.

The patch also includes a minor fix to REG0_RATE_MASK, so far it was
set to bit 0..10, but according to the spec it's bit 0..11.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/media/rc/meson-ir.c | 28 +++++++++++++---------------
 1 file changed, 13 insertions(+), 15 deletions(-)

diff --git a/drivers/media/rc/meson-ir.c b/drivers/media/rc/meson-ir.c
index a4128d7c..a52036c5 100644
--- a/drivers/media/rc/meson-ir.c
+++ b/drivers/media/rc/meson-ir.c
@@ -19,6 +19,7 @@
 #include <linux/of_platform.h>
 #include <linux/platform_device.h>
 #include <linux/spinlock.h>
+#include <linux/bitfield.h>
 
 #include <media/rc-core.h>
 
@@ -36,27 +37,24 @@
 /* only available on Meson 8b and newer */
 #define IR_DEC_REG2		0x20
 
-#define REG0_RATE_MASK		(BIT(11) - 1)
+#define REG0_RATE_MASK		GENMASK(11, 0)
 
 #define DECODE_MODE_NEC		0x0
 #define DECODE_MODE_RAW		0x2
 
 /* Meson 6b uses REG1 to configure the mode */
 #define REG1_MODE_MASK		GENMASK(8, 7)
-#define REG1_MODE_SHIFT		7
 
 /* Meson 8b / GXBB use REG2 to configure the mode */
 #define REG2_MODE_MASK		GENMASK(3, 0)
-#define REG2_MODE_SHIFT		0
 
-#define REG1_TIME_IV_SHIFT	16
-#define REG1_TIME_IV_MASK	((BIT(13) - 1) << REG1_TIME_IV_SHIFT)
+#define REG1_TIME_IV_MASK	GENMASK(28, 16)
 
-#define REG1_IRQSEL_MASK	(BIT(2) | BIT(3))
-#define REG1_IRQSEL_NEC_MODE	(0 << 2)
-#define REG1_IRQSEL_RISE_FALL	(1 << 2)
-#define REG1_IRQSEL_FALL	(2 << 2)
-#define REG1_IRQSEL_RISE	(3 << 2)
+#define REG1_IRQSEL_MASK	GENMASK(3, 2)
+#define REG1_IRQSEL_NEC_MODE	0
+#define REG1_IRQSEL_RISE_FALL	1
+#define REG1_IRQSEL_FALL	2
+#define REG1_IRQSEL_RISE	3
 
 #define REG1_RESET		BIT(0)
 #define REG1_ENABLE		BIT(15)
@@ -78,7 +76,7 @@ static void meson_ir_set_mask(struct meson_ir *ir, unsigned int reg,
 
 	data = readl(ir->reg + reg);
 	data &= ~mask;
-	data |= (value & mask);
+	data |= value;
 	writel(data, ir->reg + reg);
 }
 
@@ -91,7 +89,7 @@ static irqreturn_t meson_ir_irq(int irqno, void *dev_id)
 	spin_lock(&ir->lock);
 
 	duration = readl(ir->reg + IR_DEC_REG1);
-	duration = (duration & REG1_TIME_IV_MASK) >> REG1_TIME_IV_SHIFT;
+	duration = FIELD_GET(REG1_TIME_IV_MASK, duration);
 	rawir.duration = US_TO_NS(duration * MESON_TRATE);
 
 	rawir.pulse = !!(readl(ir->reg + IR_DEC_STATUS) & STATUS_IR_DEC_IN);
@@ -170,16 +168,16 @@ static int meson_ir_probe(struct platform_device *pdev)
 	/* Set general operation mode (= raw/software decoding) */
 	if (of_device_is_compatible(node, "amlogic,meson6-ir"))
 		meson_ir_set_mask(ir, IR_DEC_REG1, REG1_MODE_MASK,
-				  DECODE_MODE_RAW << REG1_MODE_SHIFT);
+				  FIELD_PREP(REG1_MODE_MASK, DECODE_MODE_RAW));
 	else
 		meson_ir_set_mask(ir, IR_DEC_REG2, REG2_MODE_MASK,
-				  DECODE_MODE_RAW << REG2_MODE_SHIFT);
+				  FIELD_PREP(REG2_MODE_MASK, DECODE_MODE_RAW));
 
 	/* Set rate */
 	meson_ir_set_mask(ir, IR_DEC_REG0, REG0_RATE_MASK, MESON_TRATE - 1);
 	/* IRQ on rising and falling edges */
 	meson_ir_set_mask(ir, IR_DEC_REG1, REG1_IRQSEL_MASK,
-			  REG1_IRQSEL_RISE_FALL);
+			  FIELD_PREP(REG1_IRQSEL_MASK, REG1_IRQSEL_RISE_FALL));
 	/* Enable the decoder */
 	meson_ir_set_mask(ir, IR_DEC_REG1, REG1_ENABLE, REG1_ENABLE);
 
-- 
2.12.2

[PATCH 3/5] media: rc: meson-ir: switch to managed rc device allocation / registration

[Heiner Kallweit]

Switch to the managed versions of rc_allocate_device/rc_register_device,
thus simplifying the code.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/media/rc/meson-ir.c | 18 ++++--------------
 1 file changed, 4 insertions(+), 14 deletions(-)

diff --git a/drivers/media/rc/meson-ir.c b/drivers/media/rc/meson-ir.c
index a52036c5..d56ef27e 100644
--- a/drivers/media/rc/meson-ir.c
+++ b/drivers/media/rc/meson-ir.c
@@ -128,7 +128,7 @@ static int meson_ir_probe(struct platform_device *pdev)
 		return irq;
 	}
 
-	ir->rc = rc_allocate_device(RC_DRIVER_IR_RAW);
+	ir->rc = devm_rc_allocate_device(dev, RC_DRIVER_IR_RAW);
 	if (!ir->rc) {
 		dev_err(dev, "failed to allocate rc device\n");
 		return -ENOMEM;
@@ -140,7 +140,6 @@ static int meson_ir_probe(struct platform_device *pdev)
 	ir->rc->input_id.bustype = BUS_HOST;
 	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->allowed_protocols = RC_BIT_ALL_IR_DECODER;
 	ir->rc->rx_resolution = US_TO_NS(MESON_TRATE);
 	ir->rc->timeout = MS_TO_NS(200);
@@ -149,16 +148,16 @@ static int meson_ir_probe(struct platform_device *pdev)
 	spin_lock_init(&ir->lock);
 	platform_set_drvdata(pdev, ir);
 
-	ret = rc_register_device(ir->rc);
+	ret = devm_rc_register_device(dev, ir->rc);
 	if (ret) {
 		dev_err(dev, "failed to register rc device\n");
-		goto out_free;
+		return ret;
 	}
 
 	ret = devm_request_irq(dev, irq, meson_ir_irq, 0, "ir-meson", ir);
 	if (ret) {
 		dev_err(dev, "failed to request irq\n");
-		goto out_unreg;
+		return ret;
 	}
 
 	/* Reset the decoder */
@@ -184,13 +183,6 @@ static int meson_ir_probe(struct platform_device *pdev)
 	dev_info(dev, "receiver initialized\n");
 
 	return 0;
-out_unreg:
-	rc_unregister_device(ir->rc);
-	ir->rc = NULL;
-out_free:
-	rc_free_device(ir->rc);
-
-	return ret;
 }
 
 static int meson_ir_remove(struct platform_device *pdev)
@@ -203,8 +195,6 @@ static int meson_ir_remove(struct platform_device *pdev)
 	meson_ir_set_mask(ir, IR_DEC_REG1, REG1_ENABLE, 0);
 	spin_unlock_irqrestore(&ir->lock, flags);
 
-	rc_unregister_device(ir->rc);
-
 	return 0;
 }
 
-- 
2.12.2

[PATCH 3/5] media: rc: meson-ir: switch to managed rc device allocation / registration

[Heiner Kallweit]

Switch to the managed versions of rc_allocate_device/rc_register_device,
thus simplifying the code.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/media/rc/meson-ir.c | 18 ++++--------------
 1 file changed, 4 insertions(+), 14 deletions(-)

diff --git a/drivers/media/rc/meson-ir.c b/drivers/media/rc/meson-ir.c
index a52036c5..d56ef27e 100644
--- a/drivers/media/rc/meson-ir.c
+++ b/drivers/media/rc/meson-ir.c
@@ -128,7 +128,7 @@ static int meson_ir_probe(struct platform_device *pdev)
 		return irq;
 	}
 
-	ir->rc = rc_allocate_device(RC_DRIVER_IR_RAW);
+	ir->rc = devm_rc_allocate_device(dev, RC_DRIVER_IR_RAW);
 	if (!ir->rc) {
 		dev_err(dev, "failed to allocate rc device\n");
 		return -ENOMEM;
@@ -140,7 +140,6 @@ static int meson_ir_probe(struct platform_device *pdev)
 	ir->rc->input_id.bustype = BUS_HOST;
 	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->allowed_protocols = RC_BIT_ALL_IR_DECODER;
 	ir->rc->rx_resolution = US_TO_NS(MESON_TRATE);
 	ir->rc->timeout = MS_TO_NS(200);
@@ -149,16 +148,16 @@ static int meson_ir_probe(struct platform_device *pdev)
 	spin_lock_init(&ir->lock);
 	platform_set_drvdata(pdev, ir);
 
-	ret = rc_register_device(ir->rc);
+	ret = devm_rc_register_device(dev, ir->rc);
 	if (ret) {
 		dev_err(dev, "failed to register rc device\n");
-		goto out_free;
+		return ret;
 	}
 
 	ret = devm_request_irq(dev, irq, meson_ir_irq, 0, "ir-meson", ir);
 	if (ret) {
 		dev_err(dev, "failed to request irq\n");
-		goto out_unreg;
+		return ret;
 	}
 
 	/* Reset the decoder */
@@ -184,13 +183,6 @@ static int meson_ir_probe(struct platform_device *pdev)
 	dev_info(dev, "receiver initialized\n");
 
 	return 0;
-out_unreg:
-	rc_unregister_device(ir->rc);
-	ir->rc = NULL;
-out_free:
-	rc_free_device(ir->rc);
-
-	return ret;
 }
 
 static int meson_ir_remove(struct platform_device *pdev)
@@ -203,8 +195,6 @@ static int meson_ir_remove(struct platform_device *pdev)
 	meson_ir_set_mask(ir, IR_DEC_REG1, REG1_ENABLE, 0);
 	spin_unlock_irqrestore(&ir->lock, flags);
 
-	rc_unregister_device(ir->rc);
-
 	return 0;
 }
 
-- 
2.12.2

[PATCH 4/5] media: rc: meson-ir: use readl_relaxed in the interrupt handler

[Heiner Kallweit]

We don't need the memory barriers here and an interrupt handler should
be as fast as possible. Therefore switch to readl_relaxed.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/media/rc/meson-ir.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/media/rc/meson-ir.c b/drivers/media/rc/meson-ir.c
index d56ef27e..246da2db 100644
--- a/drivers/media/rc/meson-ir.c
+++ b/drivers/media/rc/meson-ir.c
@@ -83,16 +83,17 @@ static void meson_ir_set_mask(struct meson_ir *ir, unsigned int reg,
 static irqreturn_t meson_ir_irq(int irqno, void *dev_id)
 {
 	struct meson_ir *ir = dev_id;
-	u32 duration;
+	u32 duration, status;
 	DEFINE_IR_RAW_EVENT(rawir);
 
 	spin_lock(&ir->lock);
 
-	duration = readl(ir->reg + IR_DEC_REG1);
+	duration = readl_relaxed(ir->reg + IR_DEC_REG1);
 	duration = FIELD_GET(REG1_TIME_IV_MASK, duration);
 	rawir.duration = US_TO_NS(duration * MESON_TRATE);
 
-	rawir.pulse = !!(readl(ir->reg + IR_DEC_STATUS) & STATUS_IR_DEC_IN);
+	status = readl_relaxed(ir->reg + IR_DEC_STATUS);
+	rawir.pulse = !!(status & STATUS_IR_DEC_IN);
 
 	ir_raw_event_store_with_filter(ir->rc, &rawir);
 	ir_raw_event_handle(ir->rc);
-- 
2.12.2

[PATCH 4/5] media: rc: meson-ir: use readl_relaxed in the interrupt handler

[Heiner Kallweit]

We don't need the memory barriers here and an interrupt handler should
be as fast as possible. Therefore switch to readl_relaxed.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/media/rc/meson-ir.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/media/rc/meson-ir.c b/drivers/media/rc/meson-ir.c
index d56ef27e..246da2db 100644
--- a/drivers/media/rc/meson-ir.c
+++ b/drivers/media/rc/meson-ir.c
@@ -83,16 +83,17 @@ static void meson_ir_set_mask(struct meson_ir *ir, unsigned int reg,
 static irqreturn_t meson_ir_irq(int irqno, void *dev_id)
 {
 	struct meson_ir *ir = dev_id;
-	u32 duration;
+	u32 duration, status;
 	DEFINE_IR_RAW_EVENT(rawir);
 
 	spin_lock(&ir->lock);
 
-	duration = readl(ir->reg + IR_DEC_REG1);
+	duration = readl_relaxed(ir->reg + IR_DEC_REG1);
 	duration = FIELD_GET(REG1_TIME_IV_MASK, duration);
 	rawir.duration = US_TO_NS(duration * MESON_TRATE);
 
-	rawir.pulse = !!(readl(ir->reg + IR_DEC_STATUS) & STATUS_IR_DEC_IN);
+	status = readl_relaxed(ir->reg + IR_DEC_STATUS);
+	rawir.pulse = !!(status & STATUS_IR_DEC_IN);
 
 	ir_raw_event_store_with_filter(ir->rc, &rawir);
 	ir_raw_event_handle(ir->rc);
-- 
2.12.2

[PATCH 5/5] media: rc: meson-ir: change irq name to to of node name

[Heiner Kallweit]

Switch the interrupt description to the default which is the of node
name. This is more in line with the interrupt descriptions in
other meson drivers.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/media/rc/meson-ir.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/rc/meson-ir.c b/drivers/media/rc/meson-ir.c
index 246da2db..471251b3 100644
--- a/drivers/media/rc/meson-ir.c
+++ b/drivers/media/rc/meson-ir.c
@@ -155,7 +155,7 @@ static int meson_ir_probe(struct platform_device *pdev)
 		return ret;
 	}
 
-	ret = devm_request_irq(dev, irq, meson_ir_irq, 0, "ir-meson", ir);
+	ret = devm_request_irq(dev, irq, meson_ir_irq, 0, NULL, ir);
 	if (ret) {
 		dev_err(dev, "failed to request irq\n");
 		return ret;
-- 
2.12.2

[PATCH 5/5] media: rc: meson-ir: change irq name to to of node name

[Heiner Kallweit]

Switch the interrupt description to the default which is the of node
name. This is more in line with the interrupt descriptions in
other meson drivers.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/media/rc/meson-ir.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/rc/meson-ir.c b/drivers/media/rc/meson-ir.c
index 246da2db..471251b3 100644
--- a/drivers/media/rc/meson-ir.c
+++ b/drivers/media/rc/meson-ir.c
@@ -155,7 +155,7 @@ static int meson_ir_probe(struct platform_device *pdev)
 		return ret;
 	}
 
-	ret = devm_request_irq(dev, irq, meson_ir_irq, 0, "ir-meson", ir);
+	ret = devm_request_irq(dev, irq, meson_ir_irq, 0, NULL, ir);
 	if (ret) {
 		dev_err(dev, "failed to request irq\n");
 		return ret;
-- 
2.12.2

[PATCH 2/5] media: rc: meson-ir: make use of the bitfield macros

[Neil Armstrong]

Hi Heiner,

On 04/11/2017 08:00 AM, Heiner Kallweit wrote:
> Make use of the bitfield macros thus partially hiding the complexity
> of dealing with bitfields.
> 
> The patch also includes a minor fix to REG0_RATE_MASK, so far it was
> set to bit 0..10, but according to the spec it's bit 0..11.
> 
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> ---
>  drivers/media/rc/meson-ir.c | 28 +++++++++++++---------------
>  1 file changed, 13 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/media/rc/meson-ir.c b/drivers/media/rc/meson-ir.c
> index a4128d7c..a52036c5 100644
> --- a/drivers/media/rc/meson-ir.c
> +++ b/drivers/media/rc/meson-ir.c
> @@ -19,6 +19,7 @@
>  #include <linux/of_platform.h>
>  #include <linux/platform_device.h>
>  #include <linux/spinlock.h>
> +#include <linux/bitfield.h>
>  
>  #include <media/rc-core.h>
>  
> @@ -36,27 +37,24 @@
>  /* only available on Meson 8b and newer */
>  #define IR_DEC_REG2		0x20
>  
> -#define REG0_RATE_MASK		(BIT(11) - 1)
> +#define REG0_RATE_MASK		GENMASK(11, 0)
>  
>  #define DECODE_MODE_NEC		0x0
>  #define DECODE_MODE_RAW		0x2
>  
>  /* Meson 6b uses REG1 to configure the mode */
>  #define REG1_MODE_MASK		GENMASK(8, 7)
> -#define REG1_MODE_SHIFT		7
>  
>  /* Meson 8b / GXBB use REG2 to configure the mode */
>  #define REG2_MODE_MASK		GENMASK(3, 0)
> -#define REG2_MODE_SHIFT		0
>  
> -#define REG1_TIME_IV_SHIFT	16
> -#define REG1_TIME_IV_MASK	((BIT(13) - 1) << REG1_TIME_IV_SHIFT)
> +#define REG1_TIME_IV_MASK	GENMASK(28, 16)
>  
> -#define REG1_IRQSEL_MASK	(BIT(2) | BIT(3))
> -#define REG1_IRQSEL_NEC_MODE	(0 << 2)
> -#define REG1_IRQSEL_RISE_FALL	(1 << 2)
> -#define REG1_IRQSEL_FALL	(2 << 2)
> -#define REG1_IRQSEL_RISE	(3 << 2)
> +#define REG1_IRQSEL_MASK	GENMASK(3, 2)
> +#define REG1_IRQSEL_NEC_MODE	0
> +#define REG1_IRQSEL_RISE_FALL	1
> +#define REG1_IRQSEL_FALL	2
> +#define REG1_IRQSEL_RISE	3
>  
>  #define REG1_RESET		BIT(0)
>  #define REG1_ENABLE		BIT(15)
> @@ -78,7 +76,7 @@ static void meson_ir_set_mask(struct meson_ir *ir, unsigned int reg,
>  
>  	data = readl(ir->reg + reg);
>  	data &= ~mask;
> -	data |= (value & mask);
> +	data |= value;

Please leave this one.

>  	writel(data, ir->reg + reg);
>  }
>  
> @@ -91,7 +89,7 @@ static irqreturn_t meson_ir_irq(int irqno, void *dev_id)
>  	spin_lock(&ir->lock);
>  
>  	duration = readl(ir->reg + IR_DEC_REG1);
> -	duration = (duration & REG1_TIME_IV_MASK) >> REG1_TIME_IV_SHIFT;
> +	duration = FIELD_GET(REG1_TIME_IV_MASK, duration);
>  	rawir.duration = US_TO_NS(duration * MESON_TRATE);
>  
>  	rawir.pulse = !!(readl(ir->reg + IR_DEC_STATUS) & STATUS_IR_DEC_IN);
> @@ -170,16 +168,16 @@ static int meson_ir_probe(struct platform_device *pdev)
>  	/* Set general operation mode (= raw/software decoding) */
>  	if (of_device_is_compatible(node, "amlogic,meson6-ir"))
>  		meson_ir_set_mask(ir, IR_DEC_REG1, REG1_MODE_MASK,
> -				  DECODE_MODE_RAW << REG1_MODE_SHIFT);
> +				  FIELD_PREP(REG1_MODE_MASK, DECODE_MODE_RAW));
>  	else
>  		meson_ir_set_mask(ir, IR_DEC_REG2, REG2_MODE_MASK,
> -				  DECODE_MODE_RAW << REG2_MODE_SHIFT);
> +				  FIELD_PREP(REG2_MODE_MASK, DECODE_MODE_RAW));
>  
>  	/* Set rate */
>  	meson_ir_set_mask(ir, IR_DEC_REG0, REG0_RATE_MASK, MESON_TRATE - 1);
>  	/* IRQ on rising and falling edges */
>  	meson_ir_set_mask(ir, IR_DEC_REG1, REG1_IRQSEL_MASK,
> -			  REG1_IRQSEL_RISE_FALL);
> +			  FIELD_PREP(REG1_IRQSEL_MASK, REG1_IRQSEL_RISE_FALL));
>  	/* Enable the decoder */
>  	meson_ir_set_mask(ir, IR_DEC_REG1, REG1_ENABLE, REG1_ENABLE);
>  
> 

Otherwise, looks good.

Reviewed-by: Neil Armstrong <narmstrong@baylibre.com>

Re: [PATCH 3/5] media: rc: meson-ir: switch to managed rc device allocation / registration

[Neil Armstrong]

On 04/11/2017 08:02 AM, Heiner Kallweit wrote:
> Switch to the managed versions of rc_allocate_device/rc_register_device,
> thus simplifying the code.
> 
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> ---
>  drivers/media/rc/meson-ir.c | 18 ++++--------------
>  1 file changed, 4 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/media/rc/meson-ir.c b/drivers/media/rc/meson-ir.c
> index a52036c5..d56ef27e 100644
> --- a/drivers/media/rc/meson-ir.c
> +++ b/drivers/media/rc/meson-ir.c
> @@ -128,7 +128,7 @@ static int meson_ir_probe(struct platform_device *pdev)
>  		return irq;
>  	}
>  
> -	ir->rc = rc_allocate_device(RC_DRIVER_IR_RAW);
> +	ir->rc = devm_rc_allocate_device(dev, RC_DRIVER_IR_RAW);
>  	if (!ir->rc) {
>  		dev_err(dev, "failed to allocate rc device\n");
>  		return -ENOMEM;
> @@ -140,7 +140,6 @@ static int meson_ir_probe(struct platform_device *pdev)
>  	ir->rc->input_id.bustype = BUS_HOST;
>  	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->allowed_protocols = RC_BIT_ALL_IR_DECODER;
>  	ir->rc->rx_resolution = US_TO_NS(MESON_TRATE);
>  	ir->rc->timeout = MS_TO_NS(200);
> @@ -149,16 +148,16 @@ static int meson_ir_probe(struct platform_device *pdev)
>  	spin_lock_init(&ir->lock);
>  	platform_set_drvdata(pdev, ir);
>  
> -	ret = rc_register_device(ir->rc);
> +	ret = devm_rc_register_device(dev, ir->rc);
>  	if (ret) {
>  		dev_err(dev, "failed to register rc device\n");
> -		goto out_free;
> +		return ret;
>  	}
>  
>  	ret = devm_request_irq(dev, irq, meson_ir_irq, 0, "ir-meson", ir);
>  	if (ret) {
>  		dev_err(dev, "failed to request irq\n");
> -		goto out_unreg;
> +		return ret;
>  	}
>  
>  	/* Reset the decoder */
> @@ -184,13 +183,6 @@ static int meson_ir_probe(struct platform_device *pdev)
>  	dev_info(dev, "receiver initialized\n");
>  
>  	return 0;
> -out_unreg:
> -	rc_unregister_device(ir->rc);
> -	ir->rc = NULL;
> -out_free:
> -	rc_free_device(ir->rc);
> -
> -	return ret;
>  }
>  
>  static int meson_ir_remove(struct platform_device *pdev)
> @@ -203,8 +195,6 @@ static int meson_ir_remove(struct platform_device *pdev)
>  	meson_ir_set_mask(ir, IR_DEC_REG1, REG1_ENABLE, 0);
>  	spin_unlock_irqrestore(&ir->lock, flags);
>  
> -	rc_unregister_device(ir->rc);
> -
>  	return 0;
>  }
>  
> 

Reviewed-by: Neil Armstrong <narmstrong@baylibre.com>

[PATCH 3/5] media: rc: meson-ir: switch to managed rc device allocation / registration

[Neil Armstrong]

On 04/11/2017 08:02 AM, Heiner Kallweit wrote:
> Switch to the managed versions of rc_allocate_device/rc_register_device,
> thus simplifying the code.
> 
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> ---
>  drivers/media/rc/meson-ir.c | 18 ++++--------------
>  1 file changed, 4 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/media/rc/meson-ir.c b/drivers/media/rc/meson-ir.c
> index a52036c5..d56ef27e 100644
> --- a/drivers/media/rc/meson-ir.c
> +++ b/drivers/media/rc/meson-ir.c
> @@ -128,7 +128,7 @@ static int meson_ir_probe(struct platform_device *pdev)
>  		return irq;
>  	}
>  
> -	ir->rc = rc_allocate_device(RC_DRIVER_IR_RAW);
> +	ir->rc = devm_rc_allocate_device(dev, RC_DRIVER_IR_RAW);
>  	if (!ir->rc) {
>  		dev_err(dev, "failed to allocate rc device\n");
>  		return -ENOMEM;
> @@ -140,7 +140,6 @@ static int meson_ir_probe(struct platform_device *pdev)
>  	ir->rc->input_id.bustype = BUS_HOST;
>  	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->allowed_protocols = RC_BIT_ALL_IR_DECODER;
>  	ir->rc->rx_resolution = US_TO_NS(MESON_TRATE);
>  	ir->rc->timeout = MS_TO_NS(200);
> @@ -149,16 +148,16 @@ static int meson_ir_probe(struct platform_device *pdev)
>  	spin_lock_init(&ir->lock);
>  	platform_set_drvdata(pdev, ir);
>  
> -	ret = rc_register_device(ir->rc);
> +	ret = devm_rc_register_device(dev, ir->rc);
>  	if (ret) {
>  		dev_err(dev, "failed to register rc device\n");
> -		goto out_free;
> +		return ret;
>  	}
>  
>  	ret = devm_request_irq(dev, irq, meson_ir_irq, 0, "ir-meson", ir);
>  	if (ret) {
>  		dev_err(dev, "failed to request irq\n");
> -		goto out_unreg;
> +		return ret;
>  	}
>  
>  	/* Reset the decoder */
> @@ -184,13 +183,6 @@ static int meson_ir_probe(struct platform_device *pdev)
>  	dev_info(dev, "receiver initialized\n");
>  
>  	return 0;
> -out_unreg:
> -	rc_unregister_device(ir->rc);
> -	ir->rc = NULL;
> -out_free:
> -	rc_free_device(ir->rc);
> -
> -	return ret;
>  }
>  
>  static int meson_ir_remove(struct platform_device *pdev)
> @@ -203,8 +195,6 @@ static int meson_ir_remove(struct platform_device *pdev)
>  	meson_ir_set_mask(ir, IR_DEC_REG1, REG1_ENABLE, 0);
>  	spin_unlock_irqrestore(&ir->lock, flags);
>  
> -	rc_unregister_device(ir->rc);
> -
>  	return 0;
>  }
>  
> 

Reviewed-by: Neil Armstrong <narmstrong@baylibre.com>

Re: [PATCH 4/5] media: rc: meson-ir: use readl_relaxed in the interrupt handler

[Neil Armstrong]

On 04/11/2017 08:05 AM, Heiner Kallweit wrote:
> We don't need the memory barriers here and an interrupt handler should
> be as fast as possible. Therefore switch to readl_relaxed.
> 
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> ---
>  drivers/media/rc/meson-ir.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/media/rc/meson-ir.c b/drivers/media/rc/meson-ir.c
> index d56ef27e..246da2db 100644
> --- a/drivers/media/rc/meson-ir.c
> +++ b/drivers/media/rc/meson-ir.c
> @@ -83,16 +83,17 @@ static void meson_ir_set_mask(struct meson_ir *ir, unsigned int reg,
>  static irqreturn_t meson_ir_irq(int irqno, void *dev_id)
>  {
>  	struct meson_ir *ir = dev_id;
> -	u32 duration;
> +	u32 duration, status;
>  	DEFINE_IR_RAW_EVENT(rawir);
>  
>  	spin_lock(&ir->lock);
>  
> -	duration = readl(ir->reg + IR_DEC_REG1);
> +	duration = readl_relaxed(ir->reg + IR_DEC_REG1);
>  	duration = FIELD_GET(REG1_TIME_IV_MASK, duration);
>  	rawir.duration = US_TO_NS(duration * MESON_TRATE);
>  
> -	rawir.pulse = !!(readl(ir->reg + IR_DEC_STATUS) & STATUS_IR_DEC_IN);
> +	status = readl_relaxed(ir->reg + IR_DEC_STATUS);
> +	rawir.pulse = !!(status & STATUS_IR_DEC_IN);
>  
>  	ir_raw_event_store_with_filter(ir->rc, &rawir);
>  	ir_raw_event_handle(ir->rc);
> 

Reviewed-by: Neil Armstrong <narmstrong@baylibre.com>

[PATCH 4/5] media: rc: meson-ir: use readl_relaxed in the interrupt handler

[Neil Armstrong]

On 04/11/2017 08:05 AM, Heiner Kallweit wrote:
> We don't need the memory barriers here and an interrupt handler should
> be as fast as possible. Therefore switch to readl_relaxed.
> 
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> ---
>  drivers/media/rc/meson-ir.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/media/rc/meson-ir.c b/drivers/media/rc/meson-ir.c
> index d56ef27e..246da2db 100644
> --- a/drivers/media/rc/meson-ir.c
> +++ b/drivers/media/rc/meson-ir.c
> @@ -83,16 +83,17 @@ static void meson_ir_set_mask(struct meson_ir *ir, unsigned int reg,
>  static irqreturn_t meson_ir_irq(int irqno, void *dev_id)
>  {
>  	struct meson_ir *ir = dev_id;
> -	u32 duration;
> +	u32 duration, status;
>  	DEFINE_IR_RAW_EVENT(rawir);
>  
>  	spin_lock(&ir->lock);
>  
> -	duration = readl(ir->reg + IR_DEC_REG1);
> +	duration = readl_relaxed(ir->reg + IR_DEC_REG1);
>  	duration = FIELD_GET(REG1_TIME_IV_MASK, duration);
>  	rawir.duration = US_TO_NS(duration * MESON_TRATE);
>  
> -	rawir.pulse = !!(readl(ir->reg + IR_DEC_STATUS) & STATUS_IR_DEC_IN);
> +	status = readl_relaxed(ir->reg + IR_DEC_STATUS);
> +	rawir.pulse = !!(status & STATUS_IR_DEC_IN);
>  
>  	ir_raw_event_store_with_filter(ir->rc, &rawir);
>  	ir_raw_event_handle(ir->rc);
> 

Reviewed-by: Neil Armstrong <narmstrong@baylibre.com>

Re: [PATCH 1/5] media: rc: meson-ir: remove irq from struct meson_ir

[Neil Armstrong]

On 04/11/2017 07:53 AM, Heiner Kallweit wrote:
> The irq number is used in the probe function only, therefore just use
> a local variable.
> 
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> ---
>  drivers/media/rc/meson-ir.c | 11 +++++------
>  1 file changed, 5 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/media/rc/meson-ir.c b/drivers/media/rc/meson-ir.c
> index 5576dbd6..a4128d7c 100644
> --- a/drivers/media/rc/meson-ir.c
> +++ b/drivers/media/rc/meson-ir.c
> @@ -68,7 +68,6 @@
>  struct meson_ir {
>  	void __iomem	*reg;
>  	struct rc_dev	*rc;
> -	int		irq;
>  	spinlock_t	lock;
>  };
>  
> @@ -112,7 +111,7 @@ static int meson_ir_probe(struct platform_device *pdev)
>  	struct resource *res;
>  	const char *map_name;
>  	struct meson_ir *ir;
> -	int ret;
> +	int irq, ret;
>  
>  	ir = devm_kzalloc(dev, sizeof(struct meson_ir), GFP_KERNEL);
>  	if (!ir)
> @@ -125,10 +124,10 @@ static int meson_ir_probe(struct platform_device *pdev)
>  		return PTR_ERR(ir->reg);
>  	}
>  
> -	ir->irq = platform_get_irq(pdev, 0);
> -	if (ir->irq < 0) {
> +	irq = platform_get_irq(pdev, 0);
> +	if (irq < 0) {
>  		dev_err(dev, "no irq resource\n");
> -		return ir->irq;
> +		return irq;
>  	}
>  
>  	ir->rc = rc_allocate_device(RC_DRIVER_IR_RAW);
> @@ -158,7 +157,7 @@ static int meson_ir_probe(struct platform_device *pdev)
>  		goto out_free;
>  	}
>  
> -	ret = devm_request_irq(dev, ir->irq, meson_ir_irq, 0, "ir-meson", ir);
> +	ret = devm_request_irq(dev, irq, meson_ir_irq, 0, "ir-meson", ir);
>  	if (ret) {
>  		dev_err(dev, "failed to request irq\n");
>  		goto out_unreg;
> 

Hi Heiner,

I'm not really convinced this is useful, if somehow for future enhancements we need the IRQ
number, this will need to be reverted...

Neil

[PATCH 1/5] media: rc: meson-ir: remove irq from struct meson_ir

[Neil Armstrong]

On 04/11/2017 07:53 AM, Heiner Kallweit wrote:
> The irq number is used in the probe function only, therefore just use
> a local variable.
> 
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> ---
>  drivers/media/rc/meson-ir.c | 11 +++++------
>  1 file changed, 5 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/media/rc/meson-ir.c b/drivers/media/rc/meson-ir.c
> index 5576dbd6..a4128d7c 100644
> --- a/drivers/media/rc/meson-ir.c
> +++ b/drivers/media/rc/meson-ir.c
> @@ -68,7 +68,6 @@
>  struct meson_ir {
>  	void __iomem	*reg;
>  	struct rc_dev	*rc;
> -	int		irq;
>  	spinlock_t	lock;
>  };
>  
> @@ -112,7 +111,7 @@ static int meson_ir_probe(struct platform_device *pdev)
>  	struct resource *res;
>  	const char *map_name;
>  	struct meson_ir *ir;
> -	int ret;
> +	int irq, ret;
>  
>  	ir = devm_kzalloc(dev, sizeof(struct meson_ir), GFP_KERNEL);
>  	if (!ir)
> @@ -125,10 +124,10 @@ static int meson_ir_probe(struct platform_device *pdev)
>  		return PTR_ERR(ir->reg);
>  	}
>  
> -	ir->irq = platform_get_irq(pdev, 0);
> -	if (ir->irq < 0) {
> +	irq = platform_get_irq(pdev, 0);
> +	if (irq < 0) {
>  		dev_err(dev, "no irq resource\n");
> -		return ir->irq;
> +		return irq;
>  	}
>  
>  	ir->rc = rc_allocate_device(RC_DRIVER_IR_RAW);
> @@ -158,7 +157,7 @@ static int meson_ir_probe(struct platform_device *pdev)
>  		goto out_free;
>  	}
>  
> -	ret = devm_request_irq(dev, ir->irq, meson_ir_irq, 0, "ir-meson", ir);
> +	ret = devm_request_irq(dev, irq, meson_ir_irq, 0, "ir-meson", ir);
>  	if (ret) {
>  		dev_err(dev, "failed to request irq\n");
>  		goto out_unreg;
> 

Hi Heiner,

I'm not really convinced this is useful, if somehow for future enhancements we need the IRQ
number, this will need to be reverted...

Neil

Re: [PATCH 1/5] media: rc: meson-ir: remove irq from struct meson_ir

[Sean Young]

On Tue, Apr 11, 2017 at 11:50:45AM +0200, Neil Armstrong wrote:
> On 04/11/2017 07:53 AM, Heiner Kallweit wrote:
> > The irq number is used in the probe function only, therefore just use
> > a local variable.
> > 
> > Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> > ---
> >  drivers/media/rc/meson-ir.c | 11 +++++------
> >  1 file changed, 5 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/media/rc/meson-ir.c b/drivers/media/rc/meson-ir.c
> > index 5576dbd6..a4128d7c 100644
> > --- a/drivers/media/rc/meson-ir.c
> > +++ b/drivers/media/rc/meson-ir.c
> > @@ -68,7 +68,6 @@
> >  struct meson_ir {
> >  	void __iomem	*reg;
> >  	struct rc_dev	*rc;
> > -	int		irq;
> >  	spinlock_t	lock;
> >  };
> >  
> > @@ -112,7 +111,7 @@ static int meson_ir_probe(struct platform_device *pdev)
> >  	struct resource *res;
> >  	const char *map_name;
> >  	struct meson_ir *ir;
> > -	int ret;
> > +	int irq, ret;
> >  
> >  	ir = devm_kzalloc(dev, sizeof(struct meson_ir), GFP_KERNEL);
> >  	if (!ir)
> > @@ -125,10 +124,10 @@ static int meson_ir_probe(struct platform_device *pdev)
> >  		return PTR_ERR(ir->reg);
> >  	}
> >  
> > -	ir->irq = platform_get_irq(pdev, 0);
> > -	if (ir->irq < 0) {
> > +	irq = platform_get_irq(pdev, 0);
> > +	if (irq < 0) {
> >  		dev_err(dev, "no irq resource\n");
> > -		return ir->irq;
> > +		return irq;
> >  	}
> >  
> >  	ir->rc = rc_allocate_device(RC_DRIVER_IR_RAW);
> > @@ -158,7 +157,7 @@ static int meson_ir_probe(struct platform_device *pdev)
> >  		goto out_free;
> >  	}
> >  
> > -	ret = devm_request_irq(dev, ir->irq, meson_ir_irq, 0, "ir-meson", ir);
> > +	ret = devm_request_irq(dev, irq, meson_ir_irq, 0, "ir-meson", ir);
> >  	if (ret) {
> >  		dev_err(dev, "failed to request irq\n");
> >  		goto out_unreg;
> > 
> 
> Hi Heiner,
> 
> I'm not really convinced this is useful, if somehow for future enhancements we need the IRQ
> number, this will need to be reverted...

It's bad form having unused members in a struct, and if it really is needed
in some future enhancement, it is easily reintroduced in that future
enhancement patch.


Sean

[PATCH 1/5] media: rc: meson-ir: remove irq from struct meson_ir

[Sean Young]

On Tue, Apr 11, 2017 at 11:50:45AM +0200, Neil Armstrong wrote:
> On 04/11/2017 07:53 AM, Heiner Kallweit wrote:
> > The irq number is used in the probe function only, therefore just use
> > a local variable.
> > 
> > Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> > ---
> >  drivers/media/rc/meson-ir.c | 11 +++++------
> >  1 file changed, 5 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/media/rc/meson-ir.c b/drivers/media/rc/meson-ir.c
> > index 5576dbd6..a4128d7c 100644
> > --- a/drivers/media/rc/meson-ir.c
> > +++ b/drivers/media/rc/meson-ir.c
> > @@ -68,7 +68,6 @@
> >  struct meson_ir {
> >  	void __iomem	*reg;
> >  	struct rc_dev	*rc;
> > -	int		irq;
> >  	spinlock_t	lock;
> >  };
> >  
> > @@ -112,7 +111,7 @@ static int meson_ir_probe(struct platform_device *pdev)
> >  	struct resource *res;
> >  	const char *map_name;
> >  	struct meson_ir *ir;
> > -	int ret;
> > +	int irq, ret;
> >  
> >  	ir = devm_kzalloc(dev, sizeof(struct meson_ir), GFP_KERNEL);
> >  	if (!ir)
> > @@ -125,10 +124,10 @@ static int meson_ir_probe(struct platform_device *pdev)
> >  		return PTR_ERR(ir->reg);
> >  	}
> >  
> > -	ir->irq = platform_get_irq(pdev, 0);
> > -	if (ir->irq < 0) {
> > +	irq = platform_get_irq(pdev, 0);
> > +	if (irq < 0) {
> >  		dev_err(dev, "no irq resource\n");
> > -		return ir->irq;
> > +		return irq;
> >  	}
> >  
> >  	ir->rc = rc_allocate_device(RC_DRIVER_IR_RAW);
> > @@ -158,7 +157,7 @@ static int meson_ir_probe(struct platform_device *pdev)
> >  		goto out_free;
> >  	}
> >  
> > -	ret = devm_request_irq(dev, ir->irq, meson_ir_irq, 0, "ir-meson", ir);
> > +	ret = devm_request_irq(dev, irq, meson_ir_irq, 0, "ir-meson", ir);
> >  	if (ret) {
> >  		dev_err(dev, "failed to request irq\n");
> >  		goto out_unreg;
> > 
> 
> Hi Heiner,
> 
> I'm not really convinced this is useful, if somehow for future enhancements we need the IRQ
> number, this will need to be reverted...

It's bad form having unused members in a struct, and if it really is needed
in some future enhancement, it is easily reintroduced in that future
enhancement patch.


Sean