* [PATCH 0/4] media: sunxi-cir: Cleanup and power management
@ 2021-01-13 4:51 Samuel Holland
2021-01-13 4:51 ` [PATCH 1/4] media: sunxi-cir: Clean up dead register writes Samuel Holland
` (4 more replies)
0 siblings, 5 replies; 10+ messages in thread
From: Samuel Holland @ 2021-01-13 4:51 UTC (permalink / raw)
To: Sean Young, Mauro Carvalho Chehab, Maxime Ripard, Chen-Yu Tsai,
Jernej Skrabec
Cc: linux-media, linux-kernel, linux-sunxi, Samuel Holland
This series cleans up some dead code in the sunxi-cir driver and adds
system power management hooks.
Samuel Holland (4):
media: sunxi-cir: Clean up dead register writes
media: sunxi-cir: Remove unnecessary spinlock
media: sunxi-cir: Factor out hardware initialization
media: sunxi-cir: Implement suspend/resume/shutdown callbacks
drivers/media/rc/sunxi-cir.c | 167 ++++++++++++++++++++---------------
1 file changed, 94 insertions(+), 73 deletions(-)
--
2.26.2
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 1/4] media: sunxi-cir: Clean up dead register writes
2021-01-13 4:51 [PATCH 0/4] media: sunxi-cir: Cleanup and power management Samuel Holland
@ 2021-01-13 4:51 ` Samuel Holland
2021-01-13 14:33 ` Sean Young
2021-01-13 4:51 ` [PATCH 2/4] media: sunxi-cir: Remove unnecessary spinlock Samuel Holland
` (3 subsequent siblings)
4 siblings, 1 reply; 10+ messages in thread
From: Samuel Holland @ 2021-01-13 4:51 UTC (permalink / raw)
To: Sean Young, Mauro Carvalho Chehab, Maxime Ripard, Chen-Yu Tsai,
Jernej Skrabec
Cc: linux-media, linux-kernel, linux-sunxi, Samuel Holland
The register writes during driver removal occur after the device is
already put back in reset, so they never had any effect.
Signed-off-by: Samuel Holland <samuel@sholland.org>
---
drivers/media/rc/sunxi-cir.c | 10 ----------
1 file changed, 10 deletions(-)
diff --git a/drivers/media/rc/sunxi-cir.c b/drivers/media/rc/sunxi-cir.c
index 8555c7798706..0a7f7eab3cc3 100644
--- a/drivers/media/rc/sunxi-cir.c
+++ b/drivers/media/rc/sunxi-cir.c
@@ -342,22 +342,12 @@ static int sunxi_ir_probe(struct platform_device *pdev)
static int sunxi_ir_remove(struct platform_device *pdev)
{
- unsigned long flags;
struct sunxi_ir *ir = platform_get_drvdata(pdev);
clk_disable_unprepare(ir->clk);
clk_disable_unprepare(ir->apb_clk);
reset_control_assert(ir->rst);
- spin_lock_irqsave(&ir->ir_lock, flags);
- /* disable IR IRQ */
- writel(0, ir->base + SUNXI_IR_RXINT_REG);
- /* clear All Rx Interrupt Status */
- writel(REG_RXSTA_CLEARALL, ir->base + SUNXI_IR_RXSTA_REG);
- /* disable IR */
- writel(0, ir->base + SUNXI_IR_CTL_REG);
- spin_unlock_irqrestore(&ir->ir_lock, flags);
-
rc_unregister_device(ir->rc);
return 0;
}
--
2.26.2
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 2/4] media: sunxi-cir: Remove unnecessary spinlock
2021-01-13 4:51 [PATCH 0/4] media: sunxi-cir: Cleanup and power management Samuel Holland
2021-01-13 4:51 ` [PATCH 1/4] media: sunxi-cir: Clean up dead register writes Samuel Holland
@ 2021-01-13 4:51 ` Samuel Holland
2021-01-13 4:51 ` [PATCH 3/4] media: sunxi-cir: Factor out hardware initialization Samuel Holland
` (2 subsequent siblings)
4 siblings, 0 replies; 10+ messages in thread
From: Samuel Holland @ 2021-01-13 4:51 UTC (permalink / raw)
To: Sean Young, Mauro Carvalho Chehab, Maxime Ripard, Chen-Yu Tsai,
Jernej Skrabec
Cc: linux-media, linux-kernel, linux-sunxi, Samuel Holland
Only one register, SUNXI_IR_CIR_REG, is accessed from outside the
interrupt handler, and that register is not accessed from inside it.
As there is no overlap between different contexts, no lock is needed.
Signed-off-by: Samuel Holland <samuel@sholland.org>
---
drivers/media/rc/sunxi-cir.c | 10 ----------
1 file changed, 10 deletions(-)
diff --git a/drivers/media/rc/sunxi-cir.c b/drivers/media/rc/sunxi-cir.c
index 0a7f7eab3cc3..48be400421cd 100644
--- a/drivers/media/rc/sunxi-cir.c
+++ b/drivers/media/rc/sunxi-cir.c
@@ -86,7 +86,6 @@ struct sunxi_ir_quirks {
};
struct sunxi_ir {
- spinlock_t ir_lock;
struct rc_dev *rc;
void __iomem *base;
int irq;
@@ -105,8 +104,6 @@ static irqreturn_t sunxi_ir_irq(int irqno, void *dev_id)
struct sunxi_ir *ir = dev_id;
struct ir_raw_event rawir = {};
- spin_lock(&ir->ir_lock);
-
status = readl(ir->base + SUNXI_IR_RXSTA_REG);
/* clean all pending statuses */
@@ -137,8 +134,6 @@ static irqreturn_t sunxi_ir_irq(int irqno, void *dev_id)
ir_raw_event_handle(ir->rc);
}
- spin_unlock(&ir->ir_lock);
-
return IRQ_HANDLED;
}
@@ -160,17 +155,14 @@ static int sunxi_ir_set_timeout(struct rc_dev *rc_dev, unsigned int timeout)
{
struct sunxi_ir *ir = rc_dev->priv;
unsigned int base_clk = clk_get_rate(ir->clk);
- unsigned long flags;
unsigned int ithr = sunxi_usec_to_ithr(base_clk, timeout);
dev_dbg(rc_dev->dev.parent, "setting idle threshold to %u\n", ithr);
- spin_lock_irqsave(&ir->ir_lock, flags);
/* Set noise threshold and idle threshold */
writel(REG_CIR_NTHR(SUNXI_IR_RXNOISE) | REG_CIR_ITHR(ithr),
ir->base + SUNXI_IR_CIR_REG);
- spin_unlock_irqrestore(&ir->ir_lock, flags);
rc_dev->timeout = sunxi_ithr_to_usec(base_clk, ithr);
@@ -199,8 +191,6 @@ static int sunxi_ir_probe(struct platform_device *pdev)
return -ENODEV;
}
- spin_lock_init(&ir->ir_lock);
-
ir->fifo_size = quirks->fifo_size;
/* Clock */
--
2.26.2
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 3/4] media: sunxi-cir: Factor out hardware initialization
2021-01-13 4:51 [PATCH 0/4] media: sunxi-cir: Cleanup and power management Samuel Holland
2021-01-13 4:51 ` [PATCH 1/4] media: sunxi-cir: Clean up dead register writes Samuel Holland
2021-01-13 4:51 ` [PATCH 2/4] media: sunxi-cir: Remove unnecessary spinlock Samuel Holland
@ 2021-01-13 4:51 ` Samuel Holland
2021-01-13 14:36 ` Sean Young
2021-01-13 4:51 ` [PATCH 4/4] media: sunxi-cir: Implement suspend/resume/shutdown callbacks Samuel Holland
2021-01-13 8:26 ` [PATCH 0/4] media: sunxi-cir: Cleanup and power management Maxime Ripard
4 siblings, 1 reply; 10+ messages in thread
From: Samuel Holland @ 2021-01-13 4:51 UTC (permalink / raw)
To: Sean Young, Mauro Carvalho Chehab, Maxime Ripard, Chen-Yu Tsai,
Jernej Skrabec
Cc: linux-media, linux-kernel, linux-sunxi, Samuel Holland
In preparation for adding suspend/resume hooks, factor out the hardware
initialization from the driver probe/remove functions.
The timeout programmed during init is taken from the `struct rc_dev` so
it is maintained across an exit/init cycle.
This resolves some trivial issues with the probe function: throwing away
the error from clk_prepare_enable and using the wrong type for the
temporary register value.
Signed-off-by: Samuel Holland <samuel@sholland.org>
---
drivers/media/rc/sunxi-cir.c | 128 ++++++++++++++++++++---------------
1 file changed, 74 insertions(+), 54 deletions(-)
diff --git a/drivers/media/rc/sunxi-cir.c b/drivers/media/rc/sunxi-cir.c
index 48be400421cd..ccb9d6b4225d 100644
--- a/drivers/media/rc/sunxi-cir.c
+++ b/drivers/media/rc/sunxi-cir.c
@@ -169,10 +169,74 @@ static int sunxi_ir_set_timeout(struct rc_dev *rc_dev, unsigned int timeout)
return 0;
}
+static int sunxi_ir_hw_init(struct device *dev)
+{
+ struct sunxi_ir *ir = dev_get_drvdata(dev);
+ u32 tmp;
+ int ret;
+
+ ret = reset_control_deassert(ir->rst);
+ if (ret)
+ return ret;
+
+ ret = clk_prepare_enable(ir->apb_clk);
+ if (ret) {
+ dev_err(dev, "failed to enable apb clk\n");
+ goto exit_assert_reset;
+ }
+
+ ret = clk_prepare_enable(ir->clk);
+ if (ret) {
+ dev_err(dev, "failed to enable ir clk\n");
+ goto exit_disable_apb_clk;
+ }
+
+ /* Enable CIR Mode */
+ writel(REG_CTL_MD, ir->base + SUNXI_IR_CTL_REG);
+
+ /* Set noise threshold and idle threshold */
+ sunxi_ir_set_timeout(ir->rc, ir->rc->timeout);
+
+ /* Invert Input Signal */
+ writel(REG_RXCTL_RPPI, ir->base + SUNXI_IR_RXCTL_REG);
+
+ /* Clear All Rx Interrupt Status */
+ writel(REG_RXSTA_CLEARALL, ir->base + SUNXI_IR_RXSTA_REG);
+
+ /*
+ * Enable IRQ on overflow, packet end, FIFO available with trigger
+ * level
+ */
+ writel(REG_RXINT_ROI_EN | REG_RXINT_RPEI_EN |
+ REG_RXINT_RAI_EN | REG_RXINT_RAL(ir->fifo_size / 2 - 1),
+ ir->base + SUNXI_IR_RXINT_REG);
+
+ /* Enable IR Module */
+ tmp = readl(ir->base + SUNXI_IR_CTL_REG);
+ writel(tmp | REG_CTL_GEN | REG_CTL_RXEN, ir->base + SUNXI_IR_CTL_REG);
+
+ return 0;
+
+exit_disable_apb_clk:
+ clk_disable_unprepare(ir->apb_clk);
+exit_assert_reset:
+ reset_control_assert(ir->rst);
+
+ return ret;
+}
+
+static void sunxi_ir_hw_exit(struct device *dev)
+{
+ struct sunxi_ir *ir = dev_get_drvdata(dev);
+
+ clk_disable_unprepare(ir->clk);
+ clk_disable_unprepare(ir->apb_clk);
+ reset_control_assert(ir->rst);
+}
+
static int sunxi_ir_probe(struct platform_device *pdev)
{
int ret = 0;
- unsigned long tmp = 0;
struct device *dev = &pdev->dev;
struct device_node *dn = dev->of_node;
@@ -213,43 +277,26 @@ static int sunxi_ir_probe(struct platform_device *pdev)
ir->rst = devm_reset_control_get_exclusive(dev, NULL);
if (IS_ERR(ir->rst))
return PTR_ERR(ir->rst);
- ret = reset_control_deassert(ir->rst);
- if (ret)
- return ret;
}
ret = clk_set_rate(ir->clk, b_clk_freq);
if (ret) {
dev_err(dev, "set ir base clock failed!\n");
- goto exit_reset_assert;
+ return ret;
}
dev_dbg(dev, "set base clock frequency to %d Hz.\n", b_clk_freq);
- if (clk_prepare_enable(ir->apb_clk)) {
- dev_err(dev, "try to enable apb_ir_clk failed\n");
- ret = -EINVAL;
- goto exit_reset_assert;
- }
-
- if (clk_prepare_enable(ir->clk)) {
- dev_err(dev, "try to enable ir_clk failed\n");
- ret = -EINVAL;
- goto exit_clkdisable_apb_clk;
- }
-
/* IO */
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
ir->base = devm_ioremap_resource(dev, res);
if (IS_ERR(ir->base)) {
- ret = PTR_ERR(ir->base);
- goto exit_clkdisable_clk;
+ return PTR_ERR(ir->base);
}
ir->rc = rc_allocate_device(RC_DRIVER_IR_RAW);
if (!ir->rc) {
dev_err(dev, "failed to allocate device\n");
- ret = -ENOMEM;
- goto exit_clkdisable_clk;
+ return -ENOMEM;
}
ir->rc->priv = ir;
@@ -265,6 +312,7 @@ static int sunxi_ir_probe(struct platform_device *pdev)
ir->rc->allowed_protocols = RC_PROTO_BIT_ALL_IR_DECODER;
/* Frequency after IR internal divider with sample period in us */
ir->rc->rx_resolution = (USEC_PER_SEC / (b_clk_freq / 64));
+ ir->rc->timeout = IR_DEFAULT_TIMEOUT;
ir->rc->min_timeout = sunxi_ithr_to_usec(b_clk_freq, 0);
ir->rc->max_timeout = sunxi_ithr_to_usec(b_clk_freq, 255);
ir->rc->s_timeout = sunxi_ir_set_timeout;
@@ -291,41 +339,15 @@ static int sunxi_ir_probe(struct platform_device *pdev)
goto exit_free_dev;
}
- /* Enable CIR Mode */
- writel(REG_CTL_MD, ir->base+SUNXI_IR_CTL_REG);
-
- /* Set noise threshold and idle threshold */
- sunxi_ir_set_timeout(ir->rc, IR_DEFAULT_TIMEOUT);
-
- /* Invert Input Signal */
- writel(REG_RXCTL_RPPI, ir->base + SUNXI_IR_RXCTL_REG);
-
- /* Clear All Rx Interrupt Status */
- writel(REG_RXSTA_CLEARALL, ir->base + SUNXI_IR_RXSTA_REG);
-
- /*
- * Enable IRQ on overflow, packet end, FIFO available with trigger
- * level
- */
- writel(REG_RXINT_ROI_EN | REG_RXINT_RPEI_EN |
- REG_RXINT_RAI_EN | REG_RXINT_RAL(ir->fifo_size / 2 - 1),
- ir->base + SUNXI_IR_RXINT_REG);
-
- /* Enable IR Module */
- tmp = readl(ir->base + SUNXI_IR_CTL_REG);
- writel(tmp | REG_CTL_GEN | REG_CTL_RXEN, ir->base + SUNXI_IR_CTL_REG);
+ ret = sunxi_ir_hw_init(dev);
+ if (ret)
+ goto exit_free_dev;
dev_info(dev, "initialized sunXi IR driver\n");
return 0;
exit_free_dev:
rc_free_device(ir->rc);
-exit_clkdisable_clk:
- clk_disable_unprepare(ir->clk);
-exit_clkdisable_apb_clk:
- clk_disable_unprepare(ir->apb_clk);
-exit_reset_assert:
- reset_control_assert(ir->rst);
return ret;
}
@@ -334,11 +356,9 @@ static int sunxi_ir_remove(struct platform_device *pdev)
{
struct sunxi_ir *ir = platform_get_drvdata(pdev);
- clk_disable_unprepare(ir->clk);
- clk_disable_unprepare(ir->apb_clk);
- reset_control_assert(ir->rst);
-
+ sunxi_ir_hw_exit(&pdev->dev);
rc_unregister_device(ir->rc);
+
return 0;
}
--
2.26.2
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 4/4] media: sunxi-cir: Implement suspend/resume/shutdown callbacks
2021-01-13 4:51 [PATCH 0/4] media: sunxi-cir: Cleanup and power management Samuel Holland
` (2 preceding siblings ...)
2021-01-13 4:51 ` [PATCH 3/4] media: sunxi-cir: Factor out hardware initialization Samuel Holland
@ 2021-01-13 4:51 ` Samuel Holland
2021-01-13 8:26 ` [PATCH 0/4] media: sunxi-cir: Cleanup and power management Maxime Ripard
4 siblings, 0 replies; 10+ messages in thread
From: Samuel Holland @ 2021-01-13 4:51 UTC (permalink / raw)
To: Sean Young, Mauro Carvalho Chehab, Maxime Ripard, Chen-Yu Tsai,
Jernej Skrabec
Cc: linux-media, linux-kernel, linux-sunxi, Samuel Holland
To save power, gate/reset the hardware block while the system is
asleep or powered off.
Signed-off-by: Samuel Holland <samuel@sholland.org>
---
drivers/media/rc/sunxi-cir.c | 21 +++++++++++++++++++++
1 file changed, 21 insertions(+)
diff --git a/drivers/media/rc/sunxi-cir.c b/drivers/media/rc/sunxi-cir.c
index ccb9d6b4225d..a0bdbf6f66c9 100644
--- a/drivers/media/rc/sunxi-cir.c
+++ b/drivers/media/rc/sunxi-cir.c
@@ -234,6 +234,20 @@ static void sunxi_ir_hw_exit(struct device *dev)
reset_control_assert(ir->rst);
}
+static int __maybe_unused sunxi_ir_suspend(struct device *dev)
+{
+ sunxi_ir_hw_exit(dev);
+
+ return 0;
+}
+
+static int __maybe_unused sunxi_ir_resume(struct device *dev)
+{
+ return sunxi_ir_hw_init(dev);
+}
+
+static SIMPLE_DEV_PM_OPS(sunxi_ir_pm_ops, sunxi_ir_suspend, sunxi_ir_resume);
+
static int sunxi_ir_probe(struct platform_device *pdev)
{
int ret = 0;
@@ -362,6 +376,11 @@ static int sunxi_ir_remove(struct platform_device *pdev)
return 0;
}
+static void sunxi_ir_shutdown(struct platform_device *pdev)
+{
+ sunxi_ir_hw_exit(&pdev->dev);
+}
+
static const struct sunxi_ir_quirks sun4i_a10_ir_quirks = {
.has_reset = false,
.fifo_size = 16,
@@ -397,9 +416,11 @@ MODULE_DEVICE_TABLE(of, sunxi_ir_match);
static struct platform_driver sunxi_ir_driver = {
.probe = sunxi_ir_probe,
.remove = sunxi_ir_remove,
+ .shutdown = sunxi_ir_shutdown,
.driver = {
.name = SUNXI_IR_DEV,
.of_match_table = sunxi_ir_match,
+ .pm = &sunxi_ir_pm_ops,
},
};
--
2.26.2
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 0/4] media: sunxi-cir: Cleanup and power management
2021-01-13 4:51 [PATCH 0/4] media: sunxi-cir: Cleanup and power management Samuel Holland
` (3 preceding siblings ...)
2021-01-13 4:51 ` [PATCH 4/4] media: sunxi-cir: Implement suspend/resume/shutdown callbacks Samuel Holland
@ 2021-01-13 8:26 ` Maxime Ripard
4 siblings, 0 replies; 10+ messages in thread
From: Maxime Ripard @ 2021-01-13 8:26 UTC (permalink / raw)
To: Samuel Holland
Cc: Sean Young, Mauro Carvalho Chehab, Chen-Yu Tsai, Jernej Skrabec,
linux-media, linux-kernel, linux-sunxi
[-- Attachment #1: Type: text/plain, Size: 231 bytes --]
On Tue, Jan 12, 2021 at 10:51:28PM -0600, Samuel Holland wrote:
> This series cleans up some dead code in the sunxi-cir driver and adds
> system power management hooks.
Acked-by: Maxime Ripard <mripard@kernel.org>
Thanks!
Maxime
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/4] media: sunxi-cir: Clean up dead register writes
2021-01-13 4:51 ` [PATCH 1/4] media: sunxi-cir: Clean up dead register writes Samuel Holland
@ 2021-01-13 14:33 ` Sean Young
0 siblings, 0 replies; 10+ messages in thread
From: Sean Young @ 2021-01-13 14:33 UTC (permalink / raw)
To: Samuel Holland
Cc: Mauro Carvalho Chehab, Maxime Ripard, Chen-Yu Tsai,
Jernej Skrabec, linux-media, linux-kernel, linux-sunxi
On Tue, Jan 12, 2021 at 10:51:29PM -0600, Samuel Holland wrote:
> The register writes during driver removal occur after the device is
> already put back in reset, so they never had any effect.
>
> Signed-off-by: Samuel Holland <samuel@sholland.org>
> ---
> drivers/media/rc/sunxi-cir.c | 10 ----------
> 1 file changed, 10 deletions(-)
>
> diff --git a/drivers/media/rc/sunxi-cir.c b/drivers/media/rc/sunxi-cir.c
> index 8555c7798706..0a7f7eab3cc3 100644
> --- a/drivers/media/rc/sunxi-cir.c
> +++ b/drivers/media/rc/sunxi-cir.c
> @@ -342,22 +342,12 @@ static int sunxi_ir_probe(struct platform_device *pdev)
>
> static int sunxi_ir_remove(struct platform_device *pdev)
> {
> - unsigned long flags;
> struct sunxi_ir *ir = platform_get_drvdata(pdev);
>
> clk_disable_unprepare(ir->clk);
> clk_disable_unprepare(ir->apb_clk);
> reset_control_assert(ir->rst);
>
> - spin_lock_irqsave(&ir->ir_lock, flags);
> - /* disable IR IRQ */
> - writel(0, ir->base + SUNXI_IR_RXINT_REG);
> - /* clear All Rx Interrupt Status */
> - writel(REG_RXSTA_CLEARALL, ir->base + SUNXI_IR_RXSTA_REG);
> - /* disable IR */
> - writel(0, ir->base + SUNXI_IR_CTL_REG);
> - spin_unlock_irqrestore(&ir->ir_lock, flags);
> -
> rc_unregister_device(ir->rc);
> return 0;
> }
I don't think there is anything wrong with the patch, however here the
driver does rc_unregister_device() *after* disabling it. Userspace can
still hold a file descriptor open, and call e.g. LIRC_SET_REC_TIMEOUT
ioctl, which causes various writes the sunxi-cir registers.
The order should be reversed.
Sean
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 3/4] media: sunxi-cir: Factor out hardware initialization
2021-01-13 4:51 ` [PATCH 3/4] media: sunxi-cir: Factor out hardware initialization Samuel Holland
@ 2021-01-13 14:36 ` Sean Young
2021-01-13 15:00 ` Samuel Holland
0 siblings, 1 reply; 10+ messages in thread
From: Sean Young @ 2021-01-13 14:36 UTC (permalink / raw)
To: Samuel Holland
Cc: Mauro Carvalho Chehab, Maxime Ripard, Chen-Yu Tsai,
Jernej Skrabec, linux-media, linux-kernel, linux-sunxi
On Tue, Jan 12, 2021 at 10:51:31PM -0600, Samuel Holland wrote:
> In preparation for adding suspend/resume hooks, factor out the hardware
> initialization from the driver probe/remove functions.
>
> The timeout programmed during init is taken from the `struct rc_dev` so
> it is maintained across an exit/init cycle.
>
> This resolves some trivial issues with the probe function: throwing away
> the error from clk_prepare_enable and using the wrong type for the
> temporary register value.
>
> Signed-off-by: Samuel Holland <samuel@sholland.org>
> ---
> drivers/media/rc/sunxi-cir.c | 128 ++++++++++++++++++++---------------
> 1 file changed, 74 insertions(+), 54 deletions(-)
>
> diff --git a/drivers/media/rc/sunxi-cir.c b/drivers/media/rc/sunxi-cir.c
> index 48be400421cd..ccb9d6b4225d 100644
> --- a/drivers/media/rc/sunxi-cir.c
> +++ b/drivers/media/rc/sunxi-cir.c
> @@ -169,10 +169,74 @@ static int sunxi_ir_set_timeout(struct rc_dev *rc_dev, unsigned int timeout)
> return 0;
> }
>
> +static int sunxi_ir_hw_init(struct device *dev)
> +{
> + struct sunxi_ir *ir = dev_get_drvdata(dev);
> + u32 tmp;
> + int ret;
> +
> + ret = reset_control_deassert(ir->rst);
> + if (ret)
> + return ret;
> +
> + ret = clk_prepare_enable(ir->apb_clk);
> + if (ret) {
> + dev_err(dev, "failed to enable apb clk\n");
> + goto exit_assert_reset;
> + }
> +
> + ret = clk_prepare_enable(ir->clk);
> + if (ret) {
> + dev_err(dev, "failed to enable ir clk\n");
> + goto exit_disable_apb_clk;
> + }
> +
> + /* Enable CIR Mode */
> + writel(REG_CTL_MD, ir->base + SUNXI_IR_CTL_REG);
> +
> + /* Set noise threshold and idle threshold */
> + sunxi_ir_set_timeout(ir->rc, ir->rc->timeout);
> +
> + /* Invert Input Signal */
> + writel(REG_RXCTL_RPPI, ir->base + SUNXI_IR_RXCTL_REG);
> +
> + /* Clear All Rx Interrupt Status */
> + writel(REG_RXSTA_CLEARALL, ir->base + SUNXI_IR_RXSTA_REG);
> +
> + /*
> + * Enable IRQ on overflow, packet end, FIFO available with trigger
> + * level
> + */
> + writel(REG_RXINT_ROI_EN | REG_RXINT_RPEI_EN |
> + REG_RXINT_RAI_EN | REG_RXINT_RAL(ir->fifo_size / 2 - 1),
> + ir->base + SUNXI_IR_RXINT_REG);
> +
> + /* Enable IR Module */
> + tmp = readl(ir->base + SUNXI_IR_CTL_REG);
> + writel(tmp | REG_CTL_GEN | REG_CTL_RXEN, ir->base + SUNXI_IR_CTL_REG);
> +
> + return 0;
> +
> +exit_disable_apb_clk:
> + clk_disable_unprepare(ir->apb_clk);
> +exit_assert_reset:
> + reset_control_assert(ir->rst);
> +
> + return ret;
> +}
> +
> +static void sunxi_ir_hw_exit(struct device *dev)
> +{
> + struct sunxi_ir *ir = dev_get_drvdata(dev);
> +
> + clk_disable_unprepare(ir->clk);
> + clk_disable_unprepare(ir->apb_clk);
> + reset_control_assert(ir->rst);
> +}
> +
> static int sunxi_ir_probe(struct platform_device *pdev)
> {
> int ret = 0;
> - unsigned long tmp = 0;
>
> struct device *dev = &pdev->dev;
> struct device_node *dn = dev->of_node;
> @@ -213,43 +277,26 @@ static int sunxi_ir_probe(struct platform_device *pdev)
> ir->rst = devm_reset_control_get_exclusive(dev, NULL);
> if (IS_ERR(ir->rst))
> return PTR_ERR(ir->rst);
> - ret = reset_control_deassert(ir->rst);
> - if (ret)
> - return ret;
> }
>
> ret = clk_set_rate(ir->clk, b_clk_freq);
> if (ret) {
> dev_err(dev, "set ir base clock failed!\n");
> - goto exit_reset_assert;
> + return ret;
> }
> dev_dbg(dev, "set base clock frequency to %d Hz.\n", b_clk_freq);
>
> - if (clk_prepare_enable(ir->apb_clk)) {
> - dev_err(dev, "try to enable apb_ir_clk failed\n");
> - ret = -EINVAL;
> - goto exit_reset_assert;
> - }
> -
> - if (clk_prepare_enable(ir->clk)) {
> - dev_err(dev, "try to enable ir_clk failed\n");
> - ret = -EINVAL;
> - goto exit_clkdisable_apb_clk;
> - }
> -
> /* IO */
> res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> ir->base = devm_ioremap_resource(dev, res);
> if (IS_ERR(ir->base)) {
> - ret = PTR_ERR(ir->base);
> - goto exit_clkdisable_clk;
> + return PTR_ERR(ir->base);
> }
>
> ir->rc = rc_allocate_device(RC_DRIVER_IR_RAW);
> if (!ir->rc) {
> dev_err(dev, "failed to allocate device\n");
> - ret = -ENOMEM;
> - goto exit_clkdisable_clk;
> + return -ENOMEM;
> }
>
> ir->rc->priv = ir;
> @@ -265,6 +312,7 @@ static int sunxi_ir_probe(struct platform_device *pdev)
> ir->rc->allowed_protocols = RC_PROTO_BIT_ALL_IR_DECODER;
> /* Frequency after IR internal divider with sample period in us */
> ir->rc->rx_resolution = (USEC_PER_SEC / (b_clk_freq / 64));
> + ir->rc->timeout = IR_DEFAULT_TIMEOUT;
Why? This is set from sunxi_ir_set_timeout().
> ir->rc->min_timeout = sunxi_ithr_to_usec(b_clk_freq, 0);
> ir->rc->max_timeout = sunxi_ithr_to_usec(b_clk_freq, 255);
> ir->rc->s_timeout = sunxi_ir_set_timeout;
> @@ -291,41 +339,15 @@ static int sunxi_ir_probe(struct platform_device *pdev)
> goto exit_free_dev;
> }
>
> - /* Enable CIR Mode */
> - writel(REG_CTL_MD, ir->base+SUNXI_IR_CTL_REG);
> -
> - /* Set noise threshold and idle threshold */
> - sunxi_ir_set_timeout(ir->rc, IR_DEFAULT_TIMEOUT);
> -
> - /* Invert Input Signal */
> - writel(REG_RXCTL_RPPI, ir->base + SUNXI_IR_RXCTL_REG);
> -
> - /* Clear All Rx Interrupt Status */
> - writel(REG_RXSTA_CLEARALL, ir->base + SUNXI_IR_RXSTA_REG);
> -
> - /*
> - * Enable IRQ on overflow, packet end, FIFO available with trigger
> - * level
> - */
> - writel(REG_RXINT_ROI_EN | REG_RXINT_RPEI_EN |
> - REG_RXINT_RAI_EN | REG_RXINT_RAL(ir->fifo_size / 2 - 1),
> - ir->base + SUNXI_IR_RXINT_REG);
> -
> - /* Enable IR Module */
> - tmp = readl(ir->base + SUNXI_IR_CTL_REG);
> - writel(tmp | REG_CTL_GEN | REG_CTL_RXEN, ir->base + SUNXI_IR_CTL_REG);
> + ret = sunxi_ir_hw_init(dev);
> + if (ret)
> + goto exit_free_dev;
>
> dev_info(dev, "initialized sunXi IR driver\n");
> return 0;
>
> exit_free_dev:
> rc_free_device(ir->rc);
> -exit_clkdisable_clk:
> - clk_disable_unprepare(ir->clk);
> -exit_clkdisable_apb_clk:
> - clk_disable_unprepare(ir->apb_clk);
> -exit_reset_assert:
> - reset_control_assert(ir->rst);
>
> return ret;
> }
> @@ -334,11 +356,9 @@ static int sunxi_ir_remove(struct platform_device *pdev)
> {
> struct sunxi_ir *ir = platform_get_drvdata(pdev);
>
> - clk_disable_unprepare(ir->clk);
> - clk_disable_unprepare(ir->apb_clk);
> - reset_control_assert(ir->rst);
> -
> + sunxi_ir_hw_exit(&pdev->dev);
> rc_unregister_device(ir->rc);
> +
> return 0;
> }
>
> --
> 2.26.2
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 3/4] media: sunxi-cir: Factor out hardware initialization
2021-01-13 14:36 ` Sean Young
@ 2021-01-13 15:00 ` Samuel Holland
2021-01-13 15:09 ` Sean Young
0 siblings, 1 reply; 10+ messages in thread
From: Samuel Holland @ 2021-01-13 15:00 UTC (permalink / raw)
To: Sean Young
Cc: Mauro Carvalho Chehab, Maxime Ripard, Chen-Yu Tsai,
Jernej Skrabec, linux-media, linux-kernel, linux-sunxi
On 1/13/21 8:36 AM, Sean Young wrote:
> On Tue, Jan 12, 2021 at 10:51:31PM -0600, Samuel Holland wrote:
>> In preparation for adding suspend/resume hooks, factor out the hardware
>> initialization from the driver probe/remove functions.
>>
>> The timeout programmed during init is taken from the `struct rc_dev` so
>> it is maintained across an exit/init cycle.
>>
>> This resolves some trivial issues with the probe function: throwing away
>> the error from clk_prepare_enable and using the wrong type for the
>> temporary register value.
>>
>> Signed-off-by: Samuel Holland <samuel@sholland.org>
>> ---
>> drivers/media/rc/sunxi-cir.c | 128 ++++++++++++++++++++---------------
>> 1 file changed, 74 insertions(+), 54 deletions(-)
>>
>> diff --git a/drivers/media/rc/sunxi-cir.c b/drivers/media/rc/sunxi-cir.c
>> index 48be400421cd..ccb9d6b4225d 100644
>> --- a/drivers/media/rc/sunxi-cir.c
>> +++ b/drivers/media/rc/sunxi-cir.c
>> @@ -169,10 +169,74 @@ static int sunxi_ir_set_timeout(struct rc_dev *rc_dev, unsigned int timeout)
>> return 0;
>> }
>>
>> +static int sunxi_ir_hw_init(struct device *dev)
>> +{
>> + struct sunxi_ir *ir = dev_get_drvdata(dev);
>> + u32 tmp;
>> + int ret;
>> +
>> + ret = reset_control_deassert(ir->rst);
>> + if (ret)
>> + return ret;
>> +
>> + ret = clk_prepare_enable(ir->apb_clk);
>> + if (ret) {
>> + dev_err(dev, "failed to enable apb clk\n");
>> + goto exit_assert_reset;
>> + }
>> +
>> + ret = clk_prepare_enable(ir->clk);
>> + if (ret) {
>> + dev_err(dev, "failed to enable ir clk\n");
>> + goto exit_disable_apb_clk;
>> + }
>> +
>> + /* Enable CIR Mode */
>> + writel(REG_CTL_MD, ir->base + SUNXI_IR_CTL_REG);
>> +
>> + /* Set noise threshold and idle threshold */
>> + sunxi_ir_set_timeout(ir->rc, ir->rc->timeout);
Initializing ir->rc->timeout in .probe is needed because of this line.
As the changelog mentions, this reprograms the user-configured timeout
after an exit/init (suspend/resume) cycle. It needs some default value
the first time, when called from .probe.
>> +
>> + /* Invert Input Signal */
>> + writel(REG_RXCTL_RPPI, ir->base + SUNXI_IR_RXCTL_REG);
>> +
>> + /* Clear All Rx Interrupt Status */
>> + writel(REG_RXSTA_CLEARALL, ir->base + SUNXI_IR_RXSTA_REG);
>> +
>> + /*
>> + * Enable IRQ on overflow, packet end, FIFO available with trigger
>> + * level
>> + */
>> + writel(REG_RXINT_ROI_EN | REG_RXINT_RPEI_EN |
>> + REG_RXINT_RAI_EN | REG_RXINT_RAL(ir->fifo_size / 2 - 1),
>> + ir->base + SUNXI_IR_RXINT_REG);
>> +
>> + /* Enable IR Module */
>> + tmp = readl(ir->base + SUNXI_IR_CTL_REG);
>> + writel(tmp | REG_CTL_GEN | REG_CTL_RXEN, ir->base + SUNXI_IR_CTL_REG);
>> +
>> + return 0;
>> +
>> +exit_disable_apb_clk:
>> + clk_disable_unprepare(ir->apb_clk);
>> +exit_assert_reset:
>> + reset_control_assert(ir->rst);
>> +
>> + return ret;
>> +}
>> +
>> +static void sunxi_ir_hw_exit(struct device *dev)
>> +{
>> + struct sunxi_ir *ir = dev_get_drvdata(dev);
>> +
>> + clk_disable_unprepare(ir->clk);
>> + clk_disable_unprepare(ir->apb_clk);
>> + reset_control_assert(ir->rst);
>> +}
>> +
>> static int sunxi_ir_probe(struct platform_device *pdev)
>> {
>> int ret = 0;
>> - unsigned long tmp = 0;
>>
>> struct device *dev = &pdev->dev;
>> struct device_node *dn = dev->of_node;
>> @@ -213,43 +277,26 @@ static int sunxi_ir_probe(struct platform_device *pdev)
>> ir->rst = devm_reset_control_get_exclusive(dev, NULL);
>> if (IS_ERR(ir->rst))
>> return PTR_ERR(ir->rst);
>> - ret = reset_control_deassert(ir->rst);
>> - if (ret)
>> - return ret;
>> }
>>
>> ret = clk_set_rate(ir->clk, b_clk_freq);
>> if (ret) {
>> dev_err(dev, "set ir base clock failed!\n");
>> - goto exit_reset_assert;
>> + return ret;
>> }
>> dev_dbg(dev, "set base clock frequency to %d Hz.\n", b_clk_freq);
>>
>> - if (clk_prepare_enable(ir->apb_clk)) {
>> - dev_err(dev, "try to enable apb_ir_clk failed\n");
>> - ret = -EINVAL;
>> - goto exit_reset_assert;
>> - }
>> -
>> - if (clk_prepare_enable(ir->clk)) {
>> - dev_err(dev, "try to enable ir_clk failed\n");
>> - ret = -EINVAL;
>> - goto exit_clkdisable_apb_clk;
>> - }
>> -
>> /* IO */
>> res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> ir->base = devm_ioremap_resource(dev, res);
>> if (IS_ERR(ir->base)) {
>> - ret = PTR_ERR(ir->base);
>> - goto exit_clkdisable_clk;
>> + return PTR_ERR(ir->base);
>> }
>>
>> ir->rc = rc_allocate_device(RC_DRIVER_IR_RAW);
>> if (!ir->rc) {
>> dev_err(dev, "failed to allocate device\n");
>> - ret = -ENOMEM;
>> - goto exit_clkdisable_clk;
>> + return -ENOMEM;
>> }
>>
>> ir->rc->priv = ir;
>> @@ -265,6 +312,7 @@ static int sunxi_ir_probe(struct platform_device *pdev)
>> ir->rc->allowed_protocols = RC_PROTO_BIT_ALL_IR_DECODER;
>> /* Frequency after IR internal divider with sample period in us */
>> ir->rc->rx_resolution = (USEC_PER_SEC / (b_clk_freq / 64));
>> + ir->rc->timeout = IR_DEFAULT_TIMEOUT;
>
> Why? This is set from sunxi_ir_set_timeout().
Because it is also sent as an argument to sunxi_ir_set_timeout().
>> ir->rc->min_timeout = sunxi_ithr_to_usec(b_clk_freq, 0);
>> ir->rc->max_timeout = sunxi_ithr_to_usec(b_clk_freq, 255);
>> ir->rc->s_timeout = sunxi_ir_set_timeout;
>> @@ -291,41 +339,15 @@ static int sunxi_ir_probe(struct platform_device *pdev)
>> goto exit_free_dev;
>> }
>>
>> - /* Enable CIR Mode */
>> - writel(REG_CTL_MD, ir->base+SUNXI_IR_CTL_REG);
>> -
>> - /* Set noise threshold and idle threshold */
>> - sunxi_ir_set_timeout(ir->rc, IR_DEFAULT_TIMEOUT);
This is where the default timeout was originally programmed.
>> -
>> - /* Invert Input Signal */
>> - writel(REG_RXCTL_RPPI, ir->base + SUNXI_IR_RXCTL_REG);
>> -
>> - /* Clear All Rx Interrupt Status */
>> - writel(REG_RXSTA_CLEARALL, ir->base + SUNXI_IR_RXSTA_REG);
>> -
>> - /*
>> - * Enable IRQ on overflow, packet end, FIFO available with trigger
>> - * level
>> - */
>> - writel(REG_RXINT_ROI_EN | REG_RXINT_RPEI_EN |
>> - REG_RXINT_RAI_EN | REG_RXINT_RAL(ir->fifo_size / 2 - 1),
>> - ir->base + SUNXI_IR_RXINT_REG);
>> -
>> - /* Enable IR Module */
>> - tmp = readl(ir->base + SUNXI_IR_CTL_REG);
>> - writel(tmp | REG_CTL_GEN | REG_CTL_RXEN, ir->base + SUNXI_IR_CTL_REG);
>> + ret = sunxi_ir_hw_init(dev);
>> + if (ret)
>> + goto exit_free_dev;
>>
>> dev_info(dev, "initialized sunXi IR driver\n");
>> return 0;
>>
>> exit_free_dev:
>> rc_free_device(ir->rc);
>> -exit_clkdisable_clk:
>> - clk_disable_unprepare(ir->clk);
>> -exit_clkdisable_apb_clk:
>> - clk_disable_unprepare(ir->apb_clk);
>> -exit_reset_assert:
>> - reset_control_assert(ir->rst);
>>
>> return ret;
>> }
>> @@ -334,11 +356,9 @@ static int sunxi_ir_remove(struct platform_device *pdev)
>> {
>> struct sunxi_ir *ir = platform_get_drvdata(pdev);
>>
>> - clk_disable_unprepare(ir->clk);
>> - clk_disable_unprepare(ir->apb_clk);
>> - reset_control_assert(ir->rst);
>> -
>> + sunxi_ir_hw_exit(&pdev->dev);
>> rc_unregister_device(ir->rc);
I can swap these lines to fix your comment on patch 1.
>> +
>> return 0;
>> }
>>
>> --
>> 2.26.2
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 3/4] media: sunxi-cir: Factor out hardware initialization
2021-01-13 15:00 ` Samuel Holland
@ 2021-01-13 15:09 ` Sean Young
0 siblings, 0 replies; 10+ messages in thread
From: Sean Young @ 2021-01-13 15:09 UTC (permalink / raw)
To: Samuel Holland
Cc: Mauro Carvalho Chehab, Maxime Ripard, Chen-Yu Tsai,
Jernej Skrabec, linux-media, linux-kernel, linux-sunxi
Hi Samuel,
On Wed, Jan 13, 2021 at 09:00:20AM -0600, Samuel Holland wrote:
> On 1/13/21 8:36 AM, Sean Young wrote:
> > On Tue, Jan 12, 2021 at 10:51:31PM -0600, Samuel Holland wrote:
> >> In preparation for adding suspend/resume hooks, factor out the hardware
> >> initialization from the driver probe/remove functions.
> >>
> >> The timeout programmed during init is taken from the `struct rc_dev` so
> >> it is maintained across an exit/init cycle.
> >>
> >> This resolves some trivial issues with the probe function: throwing away
> >> the error from clk_prepare_enable and using the wrong type for the
> >> temporary register value.
> >>
> >> Signed-off-by: Samuel Holland <samuel@sholland.org>
> >> ---
> >> drivers/media/rc/sunxi-cir.c | 128 ++++++++++++++++++++---------------
> >> 1 file changed, 74 insertions(+), 54 deletions(-)
> >>
> >> diff --git a/drivers/media/rc/sunxi-cir.c b/drivers/media/rc/sunxi-cir.c
> >> index 48be400421cd..ccb9d6b4225d 100644
> >> --- a/drivers/media/rc/sunxi-cir.c
> >> +++ b/drivers/media/rc/sunxi-cir.c
> >> @@ -169,10 +169,74 @@ static int sunxi_ir_set_timeout(struct rc_dev *rc_dev, unsigned int timeout)
> >> return 0;
> >> }
> >>
> >> +static int sunxi_ir_hw_init(struct device *dev)
> >> +{
> >> + struct sunxi_ir *ir = dev_get_drvdata(dev);
> >> + u32 tmp;
> >> + int ret;
> >> +
> >> + ret = reset_control_deassert(ir->rst);
> >> + if (ret)
> >> + return ret;
> >> +
> >> + ret = clk_prepare_enable(ir->apb_clk);
> >> + if (ret) {
> >> + dev_err(dev, "failed to enable apb clk\n");
> >> + goto exit_assert_reset;
> >> + }
> >> +
> >> + ret = clk_prepare_enable(ir->clk);
> >> + if (ret) {
> >> + dev_err(dev, "failed to enable ir clk\n");
> >> + goto exit_disable_apb_clk;
> >> + }
> >> +
> >> + /* Enable CIR Mode */
> >> + writel(REG_CTL_MD, ir->base + SUNXI_IR_CTL_REG);
> >> +
> >> + /* Set noise threshold and idle threshold */
> >> + sunxi_ir_set_timeout(ir->rc, ir->rc->timeout);
>
> Initializing ir->rc->timeout in .probe is needed because of this line.
> As the changelog mentions, this reprograms the user-configured timeout
> after an exit/init (suspend/resume) cycle. It needs some default value
> the first time, when called from .probe.
Yes, you're completely right. Sorry about that.
> >> +
> >> + /* Invert Input Signal */
> >> + writel(REG_RXCTL_RPPI, ir->base + SUNXI_IR_RXCTL_REG);
> >> +
> >> + /* Clear All Rx Interrupt Status */
> >> + writel(REG_RXSTA_CLEARALL, ir->base + SUNXI_IR_RXSTA_REG);
> >> +
> >> + /*
> >> + * Enable IRQ on overflow, packet end, FIFO available with trigger
> >> + * level
> >> + */
> >> + writel(REG_RXINT_ROI_EN | REG_RXINT_RPEI_EN |
> >> + REG_RXINT_RAI_EN | REG_RXINT_RAL(ir->fifo_size / 2 - 1),
> >> + ir->base + SUNXI_IR_RXINT_REG);
> >> +
> >> + /* Enable IR Module */
> >> + tmp = readl(ir->base + SUNXI_IR_CTL_REG);
> >> + writel(tmp | REG_CTL_GEN | REG_CTL_RXEN, ir->base + SUNXI_IR_CTL_REG);
> >> +
> >> + return 0;
> >> +
> >> +exit_disable_apb_clk:
> >> + clk_disable_unprepare(ir->apb_clk);
> >> +exit_assert_reset:
> >> + reset_control_assert(ir->rst);
> >> +
> >> + return ret;
> >> +}
> >> +
> >> +static void sunxi_ir_hw_exit(struct device *dev)
> >> +{
> >> + struct sunxi_ir *ir = dev_get_drvdata(dev);
> >> +
> >> + clk_disable_unprepare(ir->clk);
> >> + clk_disable_unprepare(ir->apb_clk);
> >> + reset_control_assert(ir->rst);
> >> +}
> >> +
> >> static int sunxi_ir_probe(struct platform_device *pdev)
> >> {
> >> int ret = 0;
> >> - unsigned long tmp = 0;
> >>
> >> struct device *dev = &pdev->dev;
> >> struct device_node *dn = dev->of_node;
> >> @@ -213,43 +277,26 @@ static int sunxi_ir_probe(struct platform_device *pdev)
> >> ir->rst = devm_reset_control_get_exclusive(dev, NULL);
> >> if (IS_ERR(ir->rst))
> >> return PTR_ERR(ir->rst);
> >> - ret = reset_control_deassert(ir->rst);
> >> - if (ret)
> >> - return ret;
> >> }
> >>
> >> ret = clk_set_rate(ir->clk, b_clk_freq);
> >> if (ret) {
> >> dev_err(dev, "set ir base clock failed!\n");
> >> - goto exit_reset_assert;
> >> + return ret;
> >> }
> >> dev_dbg(dev, "set base clock frequency to %d Hz.\n", b_clk_freq);
> >>
> >> - if (clk_prepare_enable(ir->apb_clk)) {
> >> - dev_err(dev, "try to enable apb_ir_clk failed\n");
> >> - ret = -EINVAL;
> >> - goto exit_reset_assert;
> >> - }
> >> -
> >> - if (clk_prepare_enable(ir->clk)) {
> >> - dev_err(dev, "try to enable ir_clk failed\n");
> >> - ret = -EINVAL;
> >> - goto exit_clkdisable_apb_clk;
> >> - }
> >> -
> >> /* IO */
> >> res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> >> ir->base = devm_ioremap_resource(dev, res);
> >> if (IS_ERR(ir->base)) {
> >> - ret = PTR_ERR(ir->base);
> >> - goto exit_clkdisable_clk;
> >> + return PTR_ERR(ir->base);
> >> }
> >>
> >> ir->rc = rc_allocate_device(RC_DRIVER_IR_RAW);
> >> if (!ir->rc) {
> >> dev_err(dev, "failed to allocate device\n");
> >> - ret = -ENOMEM;
> >> - goto exit_clkdisable_clk;
> >> + return -ENOMEM;
> >> }
> >>
> >> ir->rc->priv = ir;
> >> @@ -265,6 +312,7 @@ static int sunxi_ir_probe(struct platform_device *pdev)
> >> ir->rc->allowed_protocols = RC_PROTO_BIT_ALL_IR_DECODER;
> >> /* Frequency after IR internal divider with sample period in us */
> >> ir->rc->rx_resolution = (USEC_PER_SEC / (b_clk_freq / 64));
> >> + ir->rc->timeout = IR_DEFAULT_TIMEOUT;
> >
> > Why? This is set from sunxi_ir_set_timeout().
>
> Because it is also sent as an argument to sunxi_ir_set_timeout().
Indeed it is.
> >> ir->rc->min_timeout = sunxi_ithr_to_usec(b_clk_freq, 0);
> >> ir->rc->max_timeout = sunxi_ithr_to_usec(b_clk_freq, 255);
> >> ir->rc->s_timeout = sunxi_ir_set_timeout;
> >> @@ -291,41 +339,15 @@ static int sunxi_ir_probe(struct platform_device *pdev)
> >> goto exit_free_dev;
> >> }
> >>
> >> - /* Enable CIR Mode */
> >> - writel(REG_CTL_MD, ir->base+SUNXI_IR_CTL_REG);
> >> -
> >> - /* Set noise threshold and idle threshold */
> >> - sunxi_ir_set_timeout(ir->rc, IR_DEFAULT_TIMEOUT);
>
> This is where the default timeout was originally programmed.
>
> >> -
> >> - /* Invert Input Signal */
> >> - writel(REG_RXCTL_RPPI, ir->base + SUNXI_IR_RXCTL_REG);
> >> -
> >> - /* Clear All Rx Interrupt Status */
> >> - writel(REG_RXSTA_CLEARALL, ir->base + SUNXI_IR_RXSTA_REG);
> >> -
> >> - /*
> >> - * Enable IRQ on overflow, packet end, FIFO available with trigger
> >> - * level
> >> - */
> >> - writel(REG_RXINT_ROI_EN | REG_RXINT_RPEI_EN |
> >> - REG_RXINT_RAI_EN | REG_RXINT_RAL(ir->fifo_size / 2 - 1),
> >> - ir->base + SUNXI_IR_RXINT_REG);
> >> -
> >> - /* Enable IR Module */
> >> - tmp = readl(ir->base + SUNXI_IR_CTL_REG);
> >> - writel(tmp | REG_CTL_GEN | REG_CTL_RXEN, ir->base + SUNXI_IR_CTL_REG);
> >> + ret = sunxi_ir_hw_init(dev);
> >> + if (ret)
> >> + goto exit_free_dev;
> >>
> >> dev_info(dev, "initialized sunXi IR driver\n");
> >> return 0;
> >>
> >> exit_free_dev:
> >> rc_free_device(ir->rc);
> >> -exit_clkdisable_clk:
> >> - clk_disable_unprepare(ir->clk);
> >> -exit_clkdisable_apb_clk:
> >> - clk_disable_unprepare(ir->apb_clk);
> >> -exit_reset_assert:
> >> - reset_control_assert(ir->rst);
> >>
> >> return ret;
> >> }
> >> @@ -334,11 +356,9 @@ static int sunxi_ir_remove(struct platform_device *pdev)
> >> {
> >> struct sunxi_ir *ir = platform_get_drvdata(pdev);
> >>
> >> - clk_disable_unprepare(ir->clk);
> >> - clk_disable_unprepare(ir->apb_clk);
> >> - reset_control_assert(ir->rst);
> >> -
> >> + sunxi_ir_hw_exit(&pdev->dev);
> >> rc_unregister_device(ir->rc);
>
> I can swap these lines to fix your comment on patch 1.
Please do, and mention it in the commit message.
It might be harmless to write to SUNXI_IR_CIR_REG but it's not really the
right thing to do. It should be idiomatic to call rc_unregister_device()
first in the remove() function.
Thank you for that.
Sean
>
> >> +
> >> return 0;
> >> }
> >>
> >> --
> >> 2.26.2
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2021-01-13 15:10 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-13 4:51 [PATCH 0/4] media: sunxi-cir: Cleanup and power management Samuel Holland
2021-01-13 4:51 ` [PATCH 1/4] media: sunxi-cir: Clean up dead register writes Samuel Holland
2021-01-13 14:33 ` Sean Young
2021-01-13 4:51 ` [PATCH 2/4] media: sunxi-cir: Remove unnecessary spinlock Samuel Holland
2021-01-13 4:51 ` [PATCH 3/4] media: sunxi-cir: Factor out hardware initialization Samuel Holland
2021-01-13 14:36 ` Sean Young
2021-01-13 15:00 ` Samuel Holland
2021-01-13 15:09 ` Sean Young
2021-01-13 4:51 ` [PATCH 4/4] media: sunxi-cir: Implement suspend/resume/shutdown callbacks Samuel Holland
2021-01-13 8:26 ` [PATCH 0/4] media: sunxi-cir: Cleanup and power management Maxime Ripard
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).