[PATCH 3/4] media: sunxi-cir: Factor out hardware initialization

[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 <sam...@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

[Samuel Holland]

On 1/13/21 8:36 AM, Sean Young wrote:

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.

> 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.

I can swap these lines to fix your comment on patch 1.

[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.

It also fixes the order of the remove function to unregister the RC
device before turning off the hardware. This prevents userspace from
triggering register writes (via LIRC_SET_REC_TIMEOUT) while the hardware
is disabled.

Signed-off-by: Samuel Holland <sam...@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..2d099da8d3cc 100644

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);

rc_unregister_device(ir->rc);
+ sunxi_ir_hw_exit(&pdev->dev);