On Mon, 20 Jul 2020 11:42:37 +0100,
Joakim Zhang <[email protected]> wrote:
>
> When system suspended, we could explicitly disable clock to save power.
> And we need save registers' state since it could be lost after power
> off.
>
> Implement PM which will:
> 1) Without CONFIG_PM, clock is always on after probe stage.
> 2) With CONFIG_PM, clock is off after probe stage.
> 3) Disable clock and save registers' state when do system suspend and
> enable clock and restore registers' state while system resume.
> 4) Make Power Domain framework be able to shutdown the corresponding
> power domain of this device.
>
> Signed-off-by: Joakim Zhang <[email protected]>
> ---
> drivers/irqchip/irq-imx-intmux.c | 70 +++++++++++++++++++++++++++++++-
> 1 file changed, 68 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/irqchip/irq-imx-intmux.c
> b/drivers/irqchip/irq-imx-intmux.c
> index c27577c81126..5971603cc607 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;
> @@ -70,6 +72,7 @@ struct intmux_data {
> void __iomem *regs;
> struct clk *ipg_clk;
> int channum;
> + u32 *saved_reg;
> struct intmux_irqchip_data irqchip_data[];
> };
>
> @@ -120,8 +123,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;
> }
> @@ -232,6 +237,19 @@ static int imx_intmux_probe(struct platform_device *pdev)
> data->channum = channum;
> raw_spin_lock_init(&data->lock);
>
> + if (IS_ENABLED(CONFIG_PM)) {
> + /* save CHANIER register */
> + data->saved_reg = devm_kzalloc(&pdev->dev,
> + sizeof(unsigned int) * channum,
This isn't consistent with the type of data->saved_reg. Consider using
sizeof(*data->saved_reg), which is guaranteed to be the right type.
It also begs the question: since this saved_reg array is allocated on
a per channel basis, why don't you have a per-channel additional u32
in the intmux_irqchip_data structure instead? This would sidestep this
extra allocation altogether.
Thanks,
M.
--
Without deviation from the norm, progress is not possible.
