2015-08-07 09:31:51

by Lin Huang

[permalink] [raw]
Subject: [PATCH v4 1/2] clk: rockchip: add pclk_pd_pmu to the list of rk3288 critical clocks

pclk_pd_pmu needs to keep running and with the upcoming gpio clock
handling this is not always the case anymore. So add it to the list
of critical clocks for now.

From: Heiko Stuebner <[email protected]>
Signed-off-by: Lin Huang <[email protected]>
---
Changes in v4:
- Add From: Heiko Stuebner <[email protected]>

drivers/clk/rockchip/clk-rk3288.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/drivers/clk/rockchip/clk-rk3288.c b/drivers/clk/rockchip/clk-rk3288.c
index 0df5bae..9040878 100644
--- a/drivers/clk/rockchip/clk-rk3288.c
+++ b/drivers/clk/rockchip/clk-rk3288.c
@@ -780,6 +780,7 @@ static const char *const rk3288_critical_clocks[] __initconst = {
"aclk_cpu",
"aclk_peri",
"hclk_peri",
+ "pclk_pd_pmu",
};

#ifdef CONFIG_PM_SLEEP
--
1.9.1


2015-08-07 09:32:05

by Lin Huang

[permalink] [raw]
Subject: [PATCH v4 2/2] pinctrl: rockchip: only enable gpio clock when it setting

gpio can keep state even the clock disable, for save power
consumption, only enable gpio clock when it setting

Signed-off-by: Lin Huang <[email protected]>
Reviewed-by: Heiko Stuebner <[email protected]>
---
Changes in v4:
-delete some unrelated new blank line

drivers/pinctrl/pinctrl-rockchip.c | 55 +++++++++++++++++++++++++++++++++++---
1 file changed, 51 insertions(+), 4 deletions(-)

diff --git a/drivers/pinctrl/pinctrl-rockchip.c b/drivers/pinctrl/pinctrl-rockchip.c
index cc2843a..c5246c0 100644
--- a/drivers/pinctrl/pinctrl-rockchip.c
+++ b/drivers/pinctrl/pinctrl-rockchip.c
@@ -945,6 +945,7 @@ static int _rockchip_pmx_gpio_set_direction(struct gpio_chip *chip,
if (ret < 0)
return ret;

+ clk_enable(bank->clk);
spin_lock_irqsave(&bank->slock, flags);

data = readl_relaxed(bank->reg_base + GPIO_SWPORT_DDR);
@@ -956,6 +957,7 @@ static int _rockchip_pmx_gpio_set_direction(struct gpio_chip *chip,
writel_relaxed(data, bank->reg_base + GPIO_SWPORT_DDR);

spin_unlock_irqrestore(&bank->slock, flags);
+ clk_disable(bank->clk);

return 0;
}
@@ -1389,6 +1391,7 @@ static void rockchip_gpio_set(struct gpio_chip *gc, unsigned offset, int value)
unsigned long flags;
u32 data;

+ clk_enable(bank->clk);
spin_lock_irqsave(&bank->slock, flags);

data = readl(reg);
@@ -1398,6 +1401,7 @@ static void rockchip_gpio_set(struct gpio_chip *gc, unsigned offset, int value)
writel(data, reg);

spin_unlock_irqrestore(&bank->slock, flags);
+ clk_disable(bank->clk);
}

/*
@@ -1409,7 +1413,9 @@ static int rockchip_gpio_get(struct gpio_chip *gc, unsigned offset)
struct rockchip_pin_bank *bank = gc_to_pin_bank(gc);
u32 data;

+ clk_enable(bank->clk);
data = readl(bank->reg_base + GPIO_EXT_PORT);
+ clk_disable(bank->clk);
data >>= offset;
data &= 1;
return data;
@@ -1546,6 +1552,7 @@ static int rockchip_irq_set_type(struct irq_data *d, unsigned int type)
if (ret < 0)
return ret;

+ clk_enable(bank->clk);
spin_lock_irqsave(&bank->slock, flags);

data = readl_relaxed(bank->reg_base + GPIO_SWPORT_DDR);
@@ -1603,6 +1610,7 @@ static int rockchip_irq_set_type(struct irq_data *d, unsigned int type)
default:
irq_gc_unlock(gc);
spin_unlock_irqrestore(&bank->slock, flags);
+ clk_disable(bank->clk);
return -EINVAL;
}

@@ -1611,6 +1619,7 @@ static int rockchip_irq_set_type(struct irq_data *d, unsigned int type)

irq_gc_unlock(gc);
spin_unlock_irqrestore(&bank->slock, flags);
+ clk_disable(bank->clk);

return 0;
}
@@ -1620,8 +1629,10 @@ static void rockchip_irq_suspend(struct irq_data *d)
struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
struct rockchip_pin_bank *bank = gc->private;

+ clk_enable(bank->clk);
bank->saved_masks = irq_reg_readl(gc, GPIO_INTMASK);
irq_reg_writel(gc, ~gc->wake_active, GPIO_INTMASK);
+ clk_disable(bank->clk);
}

static void rockchip_irq_resume(struct irq_data *d)
@@ -1629,7 +1640,27 @@ static void rockchip_irq_resume(struct irq_data *d)
struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
struct rockchip_pin_bank *bank = gc->private;

+ clk_enable(bank->clk);
irq_reg_writel(gc, bank->saved_masks, GPIO_INTMASK);
+ clk_disable(bank->clk);
+}
+
+static void rockchip_irq_gc_mask_clr_bit(struct irq_data *d)
+{
+ struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
+ struct rockchip_pin_bank *bank = gc->private;
+
+ clk_enable(bank->clk);
+ irq_gc_mask_clr_bit(d);
+}
+
+void rockchip_irq_gc_mask_set_bit(struct irq_data *d)
+{
+ struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
+ struct rockchip_pin_bank *bank = gc->private;
+
+ irq_gc_mask_set_bit(d);
+ clk_disable(bank->clk);
}

static int rockchip_interrupts_register(struct platform_device *pdev,
@@ -1640,7 +1671,7 @@ static int rockchip_interrupts_register(struct platform_device *pdev,
unsigned int clr = IRQ_NOREQUEST | IRQ_NOPROBE | IRQ_NOAUTOEN;
struct irq_chip_generic *gc;
int ret;
- int i;
+ int i, j;

for (i = 0; i < ctrl->nr_banks; ++i, ++bank) {
if (!bank->valid) {
@@ -1649,11 +1680,19 @@ static int rockchip_interrupts_register(struct platform_device *pdev,
continue;
}

+ ret = clk_enable(bank->clk);
+ if (ret) {
+ dev_err(&pdev->dev, "failed to enable clock for bank %s\n",
+ bank->name);
+ continue;
+ }
+
bank->domain = irq_domain_add_linear(bank->of_node, 32,
&irq_generic_chip_ops, NULL);
if (!bank->domain) {
dev_warn(&pdev->dev, "could not initialize irq domain for bank %s\n",
bank->name);
+ clk_disable(bank->clk);
continue;
}

@@ -1664,6 +1703,7 @@ static int rockchip_interrupts_register(struct platform_device *pdev,
dev_err(&pdev->dev, "could not alloc generic chips for bank %s\n",
bank->name);
irq_domain_remove(bank->domain);
+ clk_disable(bank->clk);
continue;
}

@@ -1681,8 +1721,9 @@ static int rockchip_interrupts_register(struct platform_device *pdev,
gc->chip_types[0].regs.mask = GPIO_INTMASK;
gc->chip_types[0].regs.ack = GPIO_PORTS_EOI;
gc->chip_types[0].chip.irq_ack = irq_gc_ack_set_bit;
- gc->chip_types[0].chip.irq_mask = irq_gc_mask_set_bit;
- gc->chip_types[0].chip.irq_unmask = irq_gc_mask_clr_bit;
+ gc->chip_types[0].chip.irq_mask = rockchip_irq_gc_mask_set_bit;
+ gc->chip_types[0].chip.irq_unmask =
+ rockchip_irq_gc_mask_clr_bit;
gc->chip_types[0].chip.irq_set_wake = irq_gc_set_wake;
gc->chip_types[0].chip.irq_suspend = rockchip_irq_suspend;
gc->chip_types[0].chip.irq_resume = rockchip_irq_resume;
@@ -1691,6 +1732,12 @@ static int rockchip_interrupts_register(struct platform_device *pdev,

irq_set_chained_handler_and_data(bank->irq,
rockchip_irq_demux, bank);
+
+ /* map the gpio irqs here, when the clock is still running */
+ for (j = 0 ; j < 32 ; j++)
+ irq_create_mapping(bank->domain, j);
+
+ clk_disable(bank->clk);
}

return 0;
@@ -1808,7 +1855,7 @@ static int rockchip_get_bank_data(struct rockchip_pin_bank *bank,
if (IS_ERR(bank->clk))
return PTR_ERR(bank->clk);

- return clk_prepare_enable(bank->clk);
+ return clk_prepare(bank->clk);
}

static const struct of_device_id rockchip_pinctrl_dt_match[];
--
1.9.1

2015-08-07 09:53:27

by Lin Huang

[permalink] [raw]
Subject: Re: [PATCH v4 1/2] clk: rockchip: add pclk_pd_pmu to the list of rk3288 critical clocks

Hi Heiko,
> pclk_pd_pmu needs to keep running and with the upcoming gpio clock
> handling this is not always the case anymore. So add it to the list
> of critical clocks for now.
>
> From: Heiko Stuebner <[email protected]>
> Signed-off-by: Lin Huang <[email protected]>
is it the right way? if it still can not meet the requirement,
please let me know.
> ---
> Changes in v4:
> - Add From: Heiko Stuebner <[email protected]>
>
> drivers/clk/rockchip/clk-rk3288.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/clk/rockchip/clk-rk3288.c b/drivers/clk/rockchip/clk-rk3288.c
> index 0df5bae..9040878 100644
> --- a/drivers/clk/rockchip/clk-rk3288.c
> +++ b/drivers/clk/rockchip/clk-rk3288.c
> @@ -780,6 +780,7 @@ static const char *const rk3288_critical_clocks[] __initconst = {
> "aclk_cpu",
> "aclk_peri",
> "hclk_peri",
> + "pclk_pd_pmu",
> };
>
> #ifdef CONFIG_PM_SLEEP

2015-08-07 11:36:52

by Heiko Stuebner

[permalink] [raw]
Subject: Re: [PATCH v4 1/2] clk: rockchip: add pclk_pd_pmu to the list of rk3288 critical clocks

Am Freitag, 7. August 2015, 17:41:45 schrieb hl:
> Hi Heiko,
>
> > pclk_pd_pmu needs to keep running and with the upcoming gpio clock
> > handling this is not always the case anymore. So add it to the list
> > of critical clocks for now.
> >
> > From: Heiko Stuebner <[email protected]>
> > Signed-off-by: Lin Huang <[email protected]>
>
> is it the right way? if it still can not meet the requirement,
> please let me know.

nearly, but not quite ;-)

You can take a look at my minnie patch [0] for inspiration.


Also please make sure to include [email protected] also in this
first mail. This clock patch is a prequisite for the pinctrl change so
we would like Linus Walleij to merge both into his pinctrl tree.

Ideally both times in the "To" and not the "Cc" recipients, as To vs. Cc
is often used to distinguish whom you want to apply the patch.


Heiko


[0] http://lists.infradead.org/pipermail/linux-rockchip/2015-August/003608.html




>
> > ---
> > Changes in v4:
> > - Add From: Heiko Stuebner <[email protected]>
> >
> > drivers/clk/rockchip/clk-rk3288.c | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/clk/rockchip/clk-rk3288.c
> > b/drivers/clk/rockchip/clk-rk3288.c index 0df5bae..9040878 100644
> > --- a/drivers/clk/rockchip/clk-rk3288.c
> > +++ b/drivers/clk/rockchip/clk-rk3288.c
> > @@ -780,6 +780,7 @@ static const char *const rk3288_critical_clocks[]
> > __initconst = {>
> > "aclk_cpu",
> > "aclk_peri",
> > "hclk_peri",
> >
> > + "pclk_pd_pmu",
> >
> > };
> >
> > #ifdef CONFIG_PM_SLEEP