| Message ID | 20210113045132.31430-4-samuel@sholland.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | media: sunxi-cir: Cleanup and power management | expand |
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
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
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
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; }
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(-)