Messages in this thread | | | Date | Fri, 17 Jul 2020 09:58:13 +0100 | From | Marc Zyngier <> | Subject | Re: [PATCH 2/2] irqchip: imx-intmux: add runtime PM support |
| |
On 2020-07-16 20:32, Joakim Zhang wrote: > Add runtime PM support for intmux interrupt controller.
Same as the previous patch. The changes are significant enough that you need to write a proper change log.
> > Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com> > --- > drivers/irqchip/irq-imx-intmux.c | 31 +++++++++++++++++++++++++------ > 1 file changed, 25 insertions(+), 6 deletions(-) > > diff --git a/drivers/irqchip/irq-imx-intmux.c > b/drivers/irqchip/irq-imx-intmux.c > index 6095f76c4f0d..a631815c7bb4 100644 > --- a/drivers/irqchip/irq-imx-intmux.c > +++ b/drivers/irqchip/irq-imx-intmux.c > @@ -53,6 +53,7 @@ > #include <linux/of_irq.h> > #include <linux/of_platform.h> > #include <linux/spinlock.h> > +#include <linux/pm_runtime.h> > > #define CHANIER(n) (0x10 + (0x40 * n)) > #define CHANIPR(n) (0x20 + (0x40 * n)) > @@ -60,6 +61,7 @@ > #define CHAN_MAX_NUM 0x8 > > struct intmux_irqchip_data { > + struct irq_chip chip; > int chanidx; > int irq; > struct irq_domain *domain; > @@ -123,8 +125,10 @@ static struct irq_chip imx_intmux_irq_chip = { > static int imx_intmux_irq_map(struct irq_domain *h, unsigned int irq, > irq_hw_number_t hwirq) > { > - irq_set_chip_data(irq, h->host_data); > - irq_set_chip_and_handler(irq, &imx_intmux_irq_chip, > handle_level_irq); > + struct intmux_irqchip_data *data = h->host_data; > + > + irq_set_chip_data(irq, data); > + irq_set_chip_and_handler(irq, &data->chip, handle_level_irq); > > return 0; > } > @@ -244,6 +248,10 @@ static int imx_intmux_probe(struct platform_device > *pdev) > return -ENOMEM; > } > > + pm_runtime_get_noresume(&pdev->dev); > + pm_runtime_set_active(&pdev->dev); > + pm_runtime_enable(&pdev->dev); > + > ret = clk_prepare_enable(data->ipg_clk); > if (ret) { > dev_err(&pdev->dev, "failed to enable ipg clk: %d\n", ret); > @@ -251,6 +259,8 @@ static int imx_intmux_probe(struct platform_device > *pdev) > } > > for (i = 0; i < channum; i++) { > + data->irqchip_data[i].chip = imx_intmux_irq_chip; > + data->irqchip_data[i].chip.parent_device = &pdev->dev;
At some point, we will have to find a way to throw the parent_device thing out of the irq_chip structure. This thing is a liability. Nothing you can do about it for now though.
> data->irqchip_data[i].chanidx = i; > > data->irqchip_data[i].irq = irq_of_parse_and_map(np, i); > @@ -279,6 +289,12 @@ static int imx_intmux_probe(struct platform_device > *pdev) > > platform_set_drvdata(pdev, data); > > + /* > + * Let pm_runtime_put() disable clock. > + * If CONFIG_PM is not enabled, the clock will stay powered. > + */ > + pm_runtime_put(&pdev->dev); > + > return 0; > out: > clk_disable_unprepare(data->ipg_clk); > @@ -300,7 +316,7 @@ static int imx_intmux_remove(struct platform_device > *pdev) > irq_domain_remove(data->irqchip_data[i].domain); > } > > - clk_disable_unprepare(data->ipg_clk); > + pm_runtime_disable(&pdev->dev); > > return 0; > } > @@ -322,7 +338,7 @@ static void imx_intmux_restore_regs(struct > intmux_data *data) > writel_relaxed(data->saved_reg[i], data->regs + CHANIER(i)); > } > > -static int imx_intmux_suspend(struct device *dev) > +static int imx_intmux_runtime_suspend(struct device *dev) > { > struct intmux_data *data = dev_get_drvdata(dev); > > @@ -332,7 +348,7 @@ static int imx_intmux_suspend(struct device *dev) > return 0; > } > > -static int imx_intmux_resume(struct device *dev) > +static int imx_intmux_runtime_resume(struct device *dev)
You just introduced these two functions, and rename them immediately?
> { > struct intmux_data *data = dev_get_drvdata(dev); > int ret; > @@ -349,7 +365,10 @@ static int imx_intmux_resume(struct device *dev) > #endif > > static const struct dev_pm_ops imx_intmux_pm_ops = { > - SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(imx_intmux_suspend, imx_intmux_resume) > + SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend, > + pm_runtime_force_resume) > + SET_RUNTIME_PM_OPS(imx_intmux_runtime_suspend, > + imx_intmux_runtime_resume, NULL) > }; > > static const struct of_device_id imx_intmux_id[] = {
I think you'd might as well squash the two patches together. Splitting the two serves little purpose.
Thanks,
M. -- Jazz is not dead. It just smells funny...
| |