| Message ID | 20240208135629.2840932-2-claudiu.beznea.uj@bp.renesas.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
| Series | pinctrl: renesas: rzg2l: Add suspend to RAM support | expand |
On 08.02.2024 15:56, Claudiu wrote: > From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> > > pinctrl-rzg2l driver is used on RZ/G3S which support deep sleep states > where power to most of the SoC components is turned off. > > For this add suspend/resume support. This involves saving and restoring > configured registers along with disabling clock in case there is no pin > configured as wakeup sources. > > To save/restore registers 2 caches were allocated: one for GPIO pins and > one for dedicated pins. > > On suspend path the pin controller registers are saved and if none of the > pins are configured as wakeup sources the pinctrl clock is disabled. > Otherwise it remains on. > > On resume path the configuration is done as follows: > 1/ setup PFCs by writing to registers on pin based accesses > 2/ setup GPIOs by writing to registers on port based accesses and > following configuration steps specified in hardware manual > 3/ setup dedicated pins by writing to registers on port based accesses > 4/ setup interrupts. > > Because interrupt signals are routed to IA55 interrupt controller and > IA55 interrupt controller resumes before pin controller, patch restores > also the configured interrupts just after pin settings are restored to > avoid invalid interrupts while resuming. > > Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> > --- [ ... ] > > +/** > + * struct rzg2l_pinctrl_reg_cache - register cache structure (to be used in suspend/resume) > + * @p: P registers cache > + * @pm: PM registers cache > + * @pmc: PMC registers cache > + * @pfc: PFC registers cache > + * @iolh: IOLH registers cache > + * @ien: IEN registers cache > + * @sd_ch: SD_CH registers cache > + * @eth_poc: ET_POC registers cache > + * @eth_mode: ETH_MODE register cache > + * @qspi: QSPI registers cache > + */ > +struct rzg2l_pinctrl_reg_cache { > + u8 *p; > + u16 *pm; > + u8 *pmc; > + u32 *pfc; > + u32 *iolh[2]; > + u32 *ien[2]; > + u32 sd_ch[2]; > + u32 eth_poc[2]; > + u32 eth_mode; > + u32 qspi; I missed it, u8 should be enough for these. [ ... ]
Hi Claudiu, On Thu, Feb 8, 2024 at 6:59 PM Claudiu <claudiu.beznea@tuxon.dev> wrote: > From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> > > pinctrl-rzg2l driver is used on RZ/G3S which support deep sleep states > where power to most of the SoC components is turned off. > > For this add suspend/resume support. This involves saving and restoring > configured registers along with disabling clock in case there is no pin > configured as wakeup sources. > > To save/restore registers 2 caches were allocated: one for GPIO pins and > one for dedicated pins. > > On suspend path the pin controller registers are saved and if none of the > pins are configured as wakeup sources the pinctrl clock is disabled. > Otherwise it remains on. > > On resume path the configuration is done as follows: > 1/ setup PFCs by writing to registers on pin based accesses > 2/ setup GPIOs by writing to registers on port based accesses and > following configuration steps specified in hardware manual > 3/ setup dedicated pins by writing to registers on port based accesses > 4/ setup interrupts. > > Because interrupt signals are routed to IA55 interrupt controller and > IA55 interrupt controller resumes before pin controller, patch restores > also the configured interrupts just after pin settings are restored to > avoid invalid interrupts while resuming. > > Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> Thanks for your patch! In my review below, I am focussing on the wake-up part, as that is usually the hardest part to get right. > --- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c > +++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c > @@ -260,6 +315,9 @@ struct rzg2l_pinctrl { > struct mutex mutex; /* serialize adding groups and functions */ > > struct rzg2l_pinctrl_pin_settings *settings; > + struct rzg2l_pinctrl_reg_cache *cache; > + struct rzg2l_pinctrl_reg_cache *dedicated_cache; > + atomic_t wakeup_source; I'd call this wakeup_path, as the wake-up source is the ultimate device that triggers the GPIO. > }; > > static const u16 available_ps[] = { 1800, 2500, 3300 }; > @@ -1880,6 +1938,19 @@ static void rzg2l_gpio_irq_print_chip(struct irq_data *data, struct seq_file *p) > seq_printf(p, dev_name(gc->parent)); > } > > +static int rzg2l_gpio_irq_set_wake(struct irq_data *data, unsigned int on) > +{ > + struct gpio_chip *gc = irq_data_get_irq_chip_data(data); > + struct rzg2l_pinctrl *pctrl = container_of(gc, struct rzg2l_pinctrl, gpio_chip); > + I think you also have to call irq_set_irq_wake(pctrl->hwirq[...]) here. Cfr. drivers/gpio/gpio-rcar.c (which is simpler, as it has a single interrupt parent, instead of a parent irq_domain with multiple interrupts). > + if (on) > + atomic_inc(&pctrl->wakeup_source); > + else > + atomic_dec(&pctrl->wakeup_source); > + > + return 0; > +} > + > static const struct irq_chip rzg2l_gpio_irqchip = { > .name = "rzg2l-gpio", > .irq_disable = rzg2l_gpio_irq_disable, > +static int rzg2l_pinctrl_suspend_noirq(struct device *dev) > +{ > + struct rzg2l_pinctrl *pctrl = dev_get_drvdata(dev); > + const struct rzg2l_hwcfg *hwcfg = pctrl->data->hwcfg; > + const struct rzg2l_register_offsets *regs = &hwcfg->regs; > + struct rzg2l_pinctrl_reg_cache *cache = pctrl->cache; > + > + rzg2l_pinctrl_pm_setup_regs(pctrl, true); > + rzg2l_pinctrl_pm_setup_dedicated_regs(pctrl, true); > + > + for (u8 i = 0; i < 2; i++) { > + cache->sd_ch[i] = readl(pctrl->base + SD_CH(regs->sd_ch, i)); > + cache->eth_poc[i] = readl(pctrl->base + ETH_POC(regs->eth_poc, i)); > + } > + > + cache->qspi = readl(pctrl->base + QSPI); > + cache->eth_mode = readl(pctrl->base + ETH_MODE); > + > + if (!atomic_read(&pctrl->wakeup_source)) > + clk_disable_unprepare(pctrl->clk); While you handle the module clock yourself, I think there is still merit in calling device_set_wakeup_path(dev) when the clock is kept enabled. BTW, is there any need to save the registers when pinctrl is part of the wake-up path, and its module clock is not disabled? > + > + return 0; > +} > + > +static int rzg2l_pinctrl_resume_noirq(struct device *dev) > +{ > + struct rzg2l_pinctrl *pctrl = dev_get_drvdata(dev); > + const struct rzg2l_hwcfg *hwcfg = pctrl->data->hwcfg; > + const struct rzg2l_register_offsets *regs = &hwcfg->regs; > + struct rzg2l_pinctrl_reg_cache *cache = pctrl->cache; > + int ret; > + > + if (!atomic_read(&pctrl->wakeup_source)) { > + ret = clk_prepare_enable(pctrl->clk); > + if (ret) > + return ret; > + } Is there any need to restore the registers when pinctrl is part of the wake-up path, and its module clock was not disabled? > + > + writel(cache->qspi, pctrl->base + QSPI); > + writel(cache->eth_mode, pctrl->base + ETH_MODE); > + for (u8 i = 0; i < 2; i++) { > + writel(cache->sd_ch[i], pctrl->base + SD_CH(regs->sd_ch, i)); > + writel(cache->eth_poc[i], pctrl->base + ETH_POC(regs->eth_poc, i)); > + } > + > + rzg2l_pinctrl_pm_setup_pfc(pctrl); > + rzg2l_pinctrl_pm_setup_regs(pctrl, false); > + rzg2l_pinctrl_pm_setup_dedicated_regs(pctrl, false); > + rzg2l_gpio_irq_restore(pctrl); > + > + return 0; > +} Gr{oetje,eeting}s, Geert
Hi, Geert, On 12.02.2024 17:06, Geert Uytterhoeven wrote: > Hi Claudiu, > > On Thu, Feb 8, 2024 at 6:59 PM Claudiu <claudiu.beznea@tuxon.dev> wrote: >> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> >> >> pinctrl-rzg2l driver is used on RZ/G3S which support deep sleep states >> where power to most of the SoC components is turned off. >> >> For this add suspend/resume support. This involves saving and restoring >> configured registers along with disabling clock in case there is no pin >> configured as wakeup sources. >> >> To save/restore registers 2 caches were allocated: one for GPIO pins and >> one for dedicated pins. >> >> On suspend path the pin controller registers are saved and if none of the >> pins are configured as wakeup sources the pinctrl clock is disabled. >> Otherwise it remains on. >> >> On resume path the configuration is done as follows: >> 1/ setup PFCs by writing to registers on pin based accesses >> 2/ setup GPIOs by writing to registers on port based accesses and >> following configuration steps specified in hardware manual >> 3/ setup dedicated pins by writing to registers on port based accesses >> 4/ setup interrupts. >> >> Because interrupt signals are routed to IA55 interrupt controller and >> IA55 interrupt controller resumes before pin controller, patch restores >> also the configured interrupts just after pin settings are restored to >> avoid invalid interrupts while resuming. >> >> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> > > Thanks for your patch! > > In my review below, I am focussing on the wake-up part, as that is > usually the hardest part to get right. > >> --- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c >> +++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c >> @@ -260,6 +315,9 @@ struct rzg2l_pinctrl { >> struct mutex mutex; /* serialize adding groups and functions */ >> >> struct rzg2l_pinctrl_pin_settings *settings; >> + struct rzg2l_pinctrl_reg_cache *cache; >> + struct rzg2l_pinctrl_reg_cache *dedicated_cache; >> + atomic_t wakeup_source; > > I'd call this wakeup_path, as the wake-up source is the ultimate device > that triggers the GPIO. OK! > >> }; >> >> static const u16 available_ps[] = { 1800, 2500, 3300 }; >> @@ -1880,6 +1938,19 @@ static void rzg2l_gpio_irq_print_chip(struct irq_data *data, struct seq_file *p) >> seq_printf(p, dev_name(gc->parent)); >> } >> >> +static int rzg2l_gpio_irq_set_wake(struct irq_data *data, unsigned int on) >> +{ >> + struct gpio_chip *gc = irq_data_get_irq_chip_data(data); >> + struct rzg2l_pinctrl *pctrl = container_of(gc, struct rzg2l_pinctrl, gpio_chip); >> + > > I think you also have to call irq_set_irq_wake(pctrl->hwirq[...]) here. > Cfr. drivers/gpio/gpio-rcar.c (which is simpler, as it has a single interrupt > parent, instead of a parent irq_domain with multiple interrupts). I had it in my initial implementation (done long time ago) but I don't remember why I removed it. I'll re-add it anyway. > >> + if (on) >> + atomic_inc(&pctrl->wakeup_source); >> + else >> + atomic_dec(&pctrl->wakeup_source); >> + >> + return 0; >> +} >> + >> static const struct irq_chip rzg2l_gpio_irqchip = { >> .name = "rzg2l-gpio", >> .irq_disable = rzg2l_gpio_irq_disable, > > >> +static int rzg2l_pinctrl_suspend_noirq(struct device *dev) >> +{ >> + struct rzg2l_pinctrl *pctrl = dev_get_drvdata(dev); >> + const struct rzg2l_hwcfg *hwcfg = pctrl->data->hwcfg; >> + const struct rzg2l_register_offsets *regs = &hwcfg->regs; >> + struct rzg2l_pinctrl_reg_cache *cache = pctrl->cache; >> + >> + rzg2l_pinctrl_pm_setup_regs(pctrl, true); >> + rzg2l_pinctrl_pm_setup_dedicated_regs(pctrl, true); >> + >> + for (u8 i = 0; i < 2; i++) { >> + cache->sd_ch[i] = readl(pctrl->base + SD_CH(regs->sd_ch, i)); >> + cache->eth_poc[i] = readl(pctrl->base + ETH_POC(regs->eth_poc, i)); >> + } >> + >> + cache->qspi = readl(pctrl->base + QSPI); >> + cache->eth_mode = readl(pctrl->base + ETH_MODE); >> + >> + if (!atomic_read(&pctrl->wakeup_source)) >> + clk_disable_unprepare(pctrl->clk); > > While you handle the module clock yourself, I think there is still merit > in calling device_set_wakeup_path(dev) when the clock is kept enabled. Ok, I'll explore it. > > BTW, is there any need to save the registers when pinctrl is part of > the wake-up path, and its module clock is not disabled? Yes, for scenarios where the pinctrl is part of the wake-up path but the system is going a deep sleep state where pinctrl registers will be lost anyway. Same for the resume path. Thank you for your review, Claudiu Beznea > >> + >> + return 0; >> +} >> + >> +static int rzg2l_pinctrl_resume_noirq(struct device *dev) >> +{ >> + struct rzg2l_pinctrl *pctrl = dev_get_drvdata(dev); >> + const struct rzg2l_hwcfg *hwcfg = pctrl->data->hwcfg; >> + const struct rzg2l_register_offsets *regs = &hwcfg->regs; >> + struct rzg2l_pinctrl_reg_cache *cache = pctrl->cache; >> + int ret; >> + >> + if (!atomic_read(&pctrl->wakeup_source)) { >> + ret = clk_prepare_enable(pctrl->clk); >> + if (ret) >> + return ret; >> + } > > Is there any need to restore the registers when pinctrl is part of > the wake-up path, and its module clock was not disabled? > >> + >> + writel(cache->qspi, pctrl->base + QSPI); >> + writel(cache->eth_mode, pctrl->base + ETH_MODE); >> + for (u8 i = 0; i < 2; i++) { >> + writel(cache->sd_ch[i], pctrl->base + SD_CH(regs->sd_ch, i)); >> + writel(cache->eth_poc[i], pctrl->base + ETH_POC(regs->eth_poc, i)); >> + } >> + >> + rzg2l_pinctrl_pm_setup_pfc(pctrl); >> + rzg2l_pinctrl_pm_setup_regs(pctrl, false); >> + rzg2l_pinctrl_pm_setup_dedicated_regs(pctrl, false); >> + rzg2l_gpio_irq_restore(pctrl); >> + >> + return 0; >> +} > > Gr{oetje,eeting}s, > > Geert >
Hi, Geert, On 12.02.2024 17:35, claudiu beznea wrote: >>> static const u16 available_ps[] = { 1800, 2500, 3300 }; >>> @@ -1880,6 +1938,19 @@ static void rzg2l_gpio_irq_print_chip(struct irq_data *data, struct seq_file *p) >>> seq_printf(p, dev_name(gc->parent)); >>> } >>> >>> +static int rzg2l_gpio_irq_set_wake(struct irq_data *data, unsigned int on) >>> +{ >>> + struct gpio_chip *gc = irq_data_get_irq_chip_data(data); >>> + struct rzg2l_pinctrl *pctrl = container_of(gc, struct rzg2l_pinctrl, gpio_chip); >>> + >> I think you also have to call irq_set_irq_wake(pctrl->hwirq[...]) here. >> Cfr. drivers/gpio/gpio-rcar.c (which is simpler, as it has a single interrupt >> parent, instead of a parent irq_domain with multiple interrupts). > I had it in my initial implementation (done long time ago) but I don't > remember why I removed it. I'll re-add it anyway. I did some investigation on this. It seems adding irq_set_irq_wake() is not necessary as the pinctrl has no virq requested on behalf of itself. With this irqchip hierarchy (pinctrl-rzg2l -> irq-renesas-rzg2l -> gic) if an IRQ consumer, e.g., the gpio-keys, request an interrupt then it may call irq_set_irq_wake(virq) (gpio-keys does that). irq_set_irq_wake(virq) is forwarded to pinctrl as follows: irq_set_irq_wake(virq, on) -> set_irq_wake_real(virq, ono) -> rzg2l_gpio_irq_set_wake(irq, on) As the irq_set_irq_wake() gets a virq as argument and as we have no virq requested by pinctrl driver there is no need to call irq_set_irq_wake(), as of my investigation. Calling it with hwirq will return with -22 and calling it with virq received as argument leads to deadlock (as it's the same virq that consumer already is configuring with irq_set_irq_wake()) due the following line from irq_set_irq_wake(): struct irq_desc *desc = irq_get_desc_buslock(irq, &flags, IRQ_GET_DESC_CHECK_GLOBAL); What we can do is to forward irq_set_wake() to the parent IRQ chip (irq-renesas-rzg2l) with irq_chip_set_wake_parent() to let him set its wakeup_path, if any. But, at the moment the irq-renesas-rzg2l has IRQCHIP_SKIP_SET_WAKE thus the irq_chip_set_wake_parent() does nothing (but it can be updated for that). Now I remember that irq_chip_set_wake_parent() is what I've called in my initial implementation and removed it due to IRQCHIP_SKIP_SET_WAKE. Please let me know if you are OK to add irq_chip_set_wake_parent() and update the irq-renesas-rzg2l driver. Thank you, Claudiu Beznea
Hi Claudiu, On Thu, Feb 15, 2024 at 10:15 AM claudiu beznea <claudiu.beznea@tuxon.dev> wrote: > On 12.02.2024 17:35, claudiu beznea wrote: > >>> static const u16 available_ps[] = { 1800, 2500, 3300 }; > >>> @@ -1880,6 +1938,19 @@ static void rzg2l_gpio_irq_print_chip(struct irq_data *data, struct seq_file *p) > >>> seq_printf(p, dev_name(gc->parent)); > >>> } > >>> > >>> +static int rzg2l_gpio_irq_set_wake(struct irq_data *data, unsigned int on) > >>> +{ > >>> + struct gpio_chip *gc = irq_data_get_irq_chip_data(data); > >>> + struct rzg2l_pinctrl *pctrl = container_of(gc, struct rzg2l_pinctrl, gpio_chip); > >>> + > >> I think you also have to call irq_set_irq_wake(pctrl->hwirq[...]) here. > >> Cfr. drivers/gpio/gpio-rcar.c (which is simpler, as it has a single interrupt > >> parent, instead of a parent irq_domain with multiple interrupts). > > I had it in my initial implementation (done long time ago) but I don't > > remember why I removed it. I'll re-add it anyway. > > I did some investigation on this. It seems adding irq_set_irq_wake() is not > necessary as the pinctrl has no virq requested on behalf of itself. > > With this irqchip hierarchy (pinctrl-rzg2l -> irq-renesas-rzg2l -> gic) if > an IRQ consumer, e.g., the gpio-keys, request an interrupt then it may call > irq_set_irq_wake(virq) (gpio-keys does that). > > irq_set_irq_wake(virq) is forwarded to pinctrl as follows: > > irq_set_irq_wake(virq, on) -> > set_irq_wake_real(virq, ono) -> > rzg2l_gpio_irq_set_wake(irq, on) > > As the irq_set_irq_wake() gets a virq as argument and as we have no virq > requested by pinctrl driver there is no need to call irq_set_irq_wake(), as > of my investigation. Calling it with hwirq will return with -22 and calling > it with virq received as argument leads to deadlock (as it's the same virq > that consumer already is configuring with irq_set_irq_wake()) due the > following line from irq_set_irq_wake(): > > struct irq_desc *desc = irq_get_desc_buslock(irq, &flags, > IRQ_GET_DESC_CHECK_GLOBAL); > > What we can do is to forward irq_set_wake() to the parent IRQ chip > (irq-renesas-rzg2l) with irq_chip_set_wake_parent() to let him set its > wakeup_path, if any. But, at the moment the irq-renesas-rzg2l has > IRQCHIP_SKIP_SET_WAKE thus the irq_chip_set_wake_parent() does nothing (but > it can be updated for that). Now I remember that irq_chip_set_wake_parent() > is what I've called in my initial implementation and removed it due to > IRQCHIP_SKIP_SET_WAKE. > > Please let me know if you are OK to add irq_chip_set_wake_parent() and > update the irq-renesas-rzg2l driver. I think calling irq_chip_set_wake_parent() regardless is a good thing to do. Whether the irq-renesas-rzg2l needs an update for wake-up handling, I don't know (and that is orthogonal to the above). If you haven't already done so, you may want to browse the wake-related git history of e.g. drivers/gpio/gpio-rcar.c. Gr{oetje,eeting}s, Geert
diff --git a/drivers/pinctrl/renesas/pinctrl-rzg2l.c b/drivers/pinctrl/renesas/pinctrl-rzg2l.c index 818dccdd70da..09d9b448d819 100644 --- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c +++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c @@ -149,6 +149,33 @@ #define RZG2L_TINT_IRQ_START_INDEX 9 #define RZG2L_PACK_HWIRQ(t, i) (((t) << 16) | (i)) +/* Read/write 8 bits register */ +#define RZG2L_PCTRL_REG_ACCESS8(_read, _addr, _val) \ + do { \ + if (_read) \ + _val = readb(_addr); \ + else \ + writeb(_val, _addr); \ + } while (0) + +/* Read/write 16 bits register */ +#define RZG2L_PCTRL_REG_ACCESS16(_read, _addr, _val) \ + do { \ + if (_read) \ + _val = readw(_addr); \ + else \ + writew(_val, _addr); \ + } while (0) + +/* Read/write 32 bits register */ +#define RZG2L_PCTRL_REG_ACCESS32(_read, _addr, _val) \ + do { \ + if (_read) \ + _val = readl(_addr); \ + else \ + writel(_val, _addr); \ + } while (0) + /** * struct rzg2l_register_offsets - specific register offsets * @pwpr: PWPR register offset @@ -241,6 +268,32 @@ struct rzg2l_pinctrl_pin_settings { u16 drive_strength_ua; }; +/** + * struct rzg2l_pinctrl_reg_cache - register cache structure (to be used in suspend/resume) + * @p: P registers cache + * @pm: PM registers cache + * @pmc: PMC registers cache + * @pfc: PFC registers cache + * @iolh: IOLH registers cache + * @ien: IEN registers cache + * @sd_ch: SD_CH registers cache + * @eth_poc: ET_POC registers cache + * @eth_mode: ETH_MODE register cache + * @qspi: QSPI registers cache + */ +struct rzg2l_pinctrl_reg_cache { + u8 *p; + u16 *pm; + u8 *pmc; + u32 *pfc; + u32 *iolh[2]; + u32 *ien[2]; + u32 sd_ch[2]; + u32 eth_poc[2]; + u32 eth_mode; + u32 qspi; +}; + struct rzg2l_pinctrl { struct pinctrl_dev *pctl; struct pinctrl_desc desc; @@ -250,6 +303,8 @@ struct rzg2l_pinctrl { void __iomem *base; struct device *dev; + struct clk *clk; + struct gpio_chip gpio_chip; struct pinctrl_gpio_range gpio_range; DECLARE_BITMAP(tint_slot, RZG2L_TINT_MAX_INTERRUPT); @@ -260,6 +315,9 @@ struct rzg2l_pinctrl { struct mutex mutex; /* serialize adding groups and functions */ struct rzg2l_pinctrl_pin_settings *settings; + struct rzg2l_pinctrl_reg_cache *cache; + struct rzg2l_pinctrl_reg_cache *dedicated_cache; + atomic_t wakeup_source; }; static const u16 available_ps[] = { 1800, 2500, 3300 }; @@ -1880,6 +1938,19 @@ static void rzg2l_gpio_irq_print_chip(struct irq_data *data, struct seq_file *p) seq_printf(p, dev_name(gc->parent)); } +static int rzg2l_gpio_irq_set_wake(struct irq_data *data, unsigned int on) +{ + struct gpio_chip *gc = irq_data_get_irq_chip_data(data); + struct rzg2l_pinctrl *pctrl = container_of(gc, struct rzg2l_pinctrl, gpio_chip); + + if (on) + atomic_inc(&pctrl->wakeup_source); + else + atomic_dec(&pctrl->wakeup_source); + + return 0; +} + static const struct irq_chip rzg2l_gpio_irqchip = { .name = "rzg2l-gpio", .irq_disable = rzg2l_gpio_irq_disable, @@ -1890,6 +1961,7 @@ static const struct irq_chip rzg2l_gpio_irqchip = { .irq_eoi = rzg2l_gpio_irqc_eoi, .irq_print_chip = rzg2l_gpio_irq_print_chip, .irq_set_affinity = irq_chip_set_affinity_parent, + .irq_set_wake = rzg2l_gpio_irq_set_wake, .flags = IRQCHIP_IMMUTABLE, GPIOCHIP_IRQ_RESOURCE_HELPERS, }; @@ -1937,6 +2009,35 @@ static int rzg2l_gpio_populate_parent_fwspec(struct gpio_chip *chip, return 0; } +static void rzg2l_gpio_irq_restore(struct rzg2l_pinctrl *pctrl) +{ + struct irq_domain *domain = pctrl->gpio_chip.irq.domain; + + for (unsigned int i = 0; i < RZG2L_TINT_MAX_INTERRUPT; i++) { + struct irq_data *data; + unsigned int virq; + + if (!pctrl->hwirq[i]) + continue; + + virq = irq_find_mapping(domain, pctrl->hwirq[i]); + if (!virq) { + dev_crit(pctrl->dev, "Failed to find IRQ mapping for hwirq %u\n", + pctrl->hwirq[i]); + continue; + } + + data = irq_domain_get_irq_data(domain, virq); + if (!data) { + dev_crit(pctrl->dev, "Failed to get IRQ data for virq=%u\n", virq); + continue; + } + + if (!irqd_irq_disabled(data)) + rzg2l_gpio_irq_enable(data); + } +} + static void rzg2l_gpio_irq_domain_free(struct irq_domain *domain, unsigned int virq, unsigned int nr_irqs) { @@ -1985,6 +2086,68 @@ static void rzg2l_init_irq_valid_mask(struct gpio_chip *gc, } } +static int rzg2l_pinctrl_reg_cache_alloc(struct rzg2l_pinctrl *pctrl) +{ + u32 nports = pctrl->data->n_port_pins / RZG2L_PINS_PER_PORT; + struct rzg2l_pinctrl_reg_cache *cache, *dedicated_cache; + + cache = devm_kzalloc(pctrl->dev, sizeof(*cache), GFP_KERNEL); + if (!cache) + return -ENOMEM; + + dedicated_cache = devm_kzalloc(pctrl->dev, sizeof(*dedicated_cache), GFP_KERNEL); + if (!dedicated_cache) + return -ENOMEM; + + cache->p = devm_kcalloc(pctrl->dev, nports, sizeof(*cache->p), GFP_KERNEL); + if (!cache->p) + return -ENOMEM; + + cache->pm = devm_kcalloc(pctrl->dev, nports, sizeof(*cache->pm), GFP_KERNEL); + if (!cache->pm) + return -ENOMEM; + + cache->pmc = devm_kcalloc(pctrl->dev, nports, sizeof(*cache->pmc), GFP_KERNEL); + if (!cache->pmc) + return -ENOMEM; + + cache->pfc = devm_kcalloc(pctrl->dev, nports, sizeof(*cache->pfc), GFP_KERNEL); + if (!cache->pfc) + return -ENOMEM; + + for (u8 i = 0; i < 2; i++) { + u32 n_dedicated_pins = pctrl->data->n_dedicated_pins; + + cache->iolh[i] = devm_kcalloc(pctrl->dev, nports, sizeof(*cache->iolh[i]), + GFP_KERNEL); + if (!cache->iolh[i]) + return -ENOMEM; + + cache->ien[i] = devm_kcalloc(pctrl->dev, nports, sizeof(*cache->ien[i]), + GFP_KERNEL); + if (!cache->ien[i]) + return -ENOMEM; + + /* Allocate dedicated cache. */ + dedicated_cache->iolh[i] = devm_kcalloc(pctrl->dev, n_dedicated_pins, + sizeof(*dedicated_cache->iolh[i]), + GFP_KERNEL); + if (!dedicated_cache->iolh[i]) + return -ENOMEM; + + dedicated_cache->ien[i] = devm_kcalloc(pctrl->dev, n_dedicated_pins, + sizeof(*dedicated_cache->ien[i]), + GFP_KERNEL); + if (!dedicated_cache->ien[i]) + return -ENOMEM; + } + + pctrl->cache = cache; + pctrl->dedicated_cache = dedicated_cache; + + return 0; +} + static int rzg2l_gpio_register(struct rzg2l_pinctrl *pctrl) { struct device_node *np = pctrl->dev->of_node; @@ -2125,6 +2288,10 @@ static int rzg2l_pinctrl_register(struct rzg2l_pinctrl *pctrl) } } + ret = rzg2l_pinctrl_reg_cache_alloc(pctrl); + if (ret) + return ret; + ret = devm_pinctrl_register_and_init(pctrl->dev, &pctrl->desc, pctrl, &pctrl->pctl); if (ret) { @@ -2150,7 +2317,6 @@ static int rzg2l_pinctrl_register(struct rzg2l_pinctrl *pctrl) static int rzg2l_pinctrl_probe(struct platform_device *pdev) { struct rzg2l_pinctrl *pctrl; - struct clk *clk; int ret; BUILD_BUG_ON(ARRAY_SIZE(r9a07g044_gpio_configs) * RZG2L_PINS_PER_PORT > @@ -2176,14 +2342,16 @@ static int rzg2l_pinctrl_probe(struct platform_device *pdev) if (IS_ERR(pctrl->base)) return PTR_ERR(pctrl->base); - clk = devm_clk_get_enabled(pctrl->dev, NULL); - if (IS_ERR(clk)) - return dev_err_probe(pctrl->dev, PTR_ERR(clk), + pctrl->clk = devm_clk_get_enabled(pctrl->dev, NULL); + if (IS_ERR(pctrl->clk)) { + return dev_err_probe(pctrl->dev, PTR_ERR(pctrl->clk), "failed to enable GPIO clk\n"); + } spin_lock_init(&pctrl->lock); spin_lock_init(&pctrl->bitmap_lock); mutex_init(&pctrl->mutex); + atomic_set(&pctrl->wakeup_source, 0); platform_set_drvdata(pdev, pctrl); @@ -2195,6 +2363,222 @@ static int rzg2l_pinctrl_probe(struct platform_device *pdev) return 0; } +static void rzg2l_pinctrl_pm_setup_regs(struct rzg2l_pinctrl *pctrl, bool suspend) +{ + u32 nports = pctrl->data->n_port_pins / RZG2L_PINS_PER_PORT; + struct rzg2l_pinctrl_reg_cache *cache = pctrl->cache; + + for (u32 port = 0; port < nports; port++) { + bool has_iolh, has_ien; + u32 off, caps; + u8 pincnt; + u64 cfg; + + cfg = pctrl->data->port_pin_configs[port]; + off = RZG2L_PIN_CFG_TO_PORT_OFFSET(cfg); + pincnt = hweight8(FIELD_GET(PIN_CFG_PIN_MAP_MASK, cfg)); + + caps = FIELD_GET(PIN_CFG_MASK, cfg); + has_iolh = !!(caps & (PIN_CFG_IOLH_A | PIN_CFG_IOLH_B | PIN_CFG_IOLH_C)); + has_ien = !!(caps & PIN_CFG_IEN); + + if (suspend) + RZG2L_PCTRL_REG_ACCESS32(suspend, pctrl->base + PFC(off), cache->pfc[port]); + + /* + * Now cache the registers or set them in the order suggested by + * HW manual (section "Operation for GPIO Function"). + */ + RZG2L_PCTRL_REG_ACCESS8(suspend, pctrl->base + PMC(off), cache->pmc[port]); + if (has_iolh) { + RZG2L_PCTRL_REG_ACCESS32(suspend, pctrl->base + IOLH(off), + cache->iolh[0][port]); + if (pincnt >= 4) { + RZG2L_PCTRL_REG_ACCESS32(suspend, pctrl->base + IOLH(off) + 4, + cache->iolh[1][port]); + } + } + + RZG2L_PCTRL_REG_ACCESS16(suspend, pctrl->base + PM(off), cache->pm[port]); + RZG2L_PCTRL_REG_ACCESS8(suspend, pctrl->base + P(off), cache->p[port]); + + if (has_ien) { + RZG2L_PCTRL_REG_ACCESS32(suspend, pctrl->base + IEN(off), + cache->ien[0][port]); + if (pincnt >= 4) { + RZG2L_PCTRL_REG_ACCESS32(suspend, pctrl->base + IEN(off) + 4, + cache->ien[1][port]); + } + } + } +} + +static void rzg2l_pinctrl_pm_setup_dedicated_regs(struct rzg2l_pinctrl *pctrl, bool suspend) +{ + struct rzg2l_pinctrl_reg_cache *cache = pctrl->dedicated_cache; + + /* + * Make sure entries in pctrl->data->n_dedicated_pins[] having the same + * port offset are close together. + */ + for (u32 i = 0, caps = 0; i < pctrl->data->n_dedicated_pins; i++) { + bool has_iolh, has_ien; + u32 off, next_off = 0; + u64 cfg, next_cfg; + u8 pincnt; + + cfg = pctrl->data->dedicated_pins[i].config; + off = RZG2L_PIN_CFG_TO_PORT_OFFSET(cfg); + if (i + 1 < pctrl->data->n_dedicated_pins) { + next_cfg = pctrl->data->dedicated_pins[i + 1].config; + next_off = RZG2L_PIN_CFG_TO_PORT_OFFSET(next_cfg); + } + + if (off == next_off) { + /* Gather caps of all port pins. */ + caps |= FIELD_GET(PIN_CFG_MASK, cfg); + continue; + } + + /* And apply them in a single shot. */ + has_iolh = !!(caps & (PIN_CFG_IOLH_A | PIN_CFG_IOLH_B | PIN_CFG_IOLH_C)); + has_ien = !!(caps & PIN_CFG_IEN); + pincnt = hweight8(FIELD_GET(RZG2L_SINGLE_PIN_BITS_MASK, cfg)); + + if (has_iolh) { + RZG2L_PCTRL_REG_ACCESS32(suspend, pctrl->base + IOLH(off), + cache->iolh[0][i]); + } + if (has_ien) { + RZG2L_PCTRL_REG_ACCESS32(suspend, pctrl->base + IEN(off), + cache->ien[0][i]); + } + + if (pincnt >= 4) { + if (has_iolh) { + RZG2L_PCTRL_REG_ACCESS32(suspend, + pctrl->base + IOLH(off) + 4, + cache->iolh[1][i]); + } + if (has_ien) { + RZG2L_PCTRL_REG_ACCESS32(suspend, + pctrl->base + IEN(off) + 4, + cache->ien[1][i]); + } + } + caps = 0; + } +} + +static void rzg2l_pinctrl_pm_setup_pfc(struct rzg2l_pinctrl *pctrl) +{ + u32 nports = pctrl->data->n_port_pins / RZG2L_PINS_PER_PORT; + const struct rzg2l_hwcfg *hwcfg = pctrl->data->hwcfg; + const struct rzg2l_register_offsets *regs = &hwcfg->regs; + + /* Set the PWPR register to allow PFC register to write. */ + writel(0x0, pctrl->base + regs->pwpr); /* B0WI=0, PFCWE=0 */ + writel(PWPR_PFCWE, pctrl->base + regs->pwpr); /* B0WI=0, PFCWE=1 */ + + /* Restore port registers. */ + for (u32 port = 0; port < nports; port++) { + unsigned long pinmap; + u8 pmc = 0, max_pin; + u32 off, pfc = 0; + u64 cfg; + u16 pm; + u8 pin; + + cfg = pctrl->data->port_pin_configs[port]; + off = RZG2L_PIN_CFG_TO_PORT_OFFSET(cfg); + pinmap = FIELD_GET(PIN_CFG_PIN_MAP_MASK, cfg); + max_pin = fls(pinmap); + + pm = readw(pctrl->base + PM(off)); + for_each_set_bit(pin, &pinmap, max_pin) { + struct rzg2l_pinctrl_reg_cache *cache = pctrl->cache; + + /* Nothing to do if PFC was not configured before. */ + if (!(cache->pmc[port] & BIT(pin))) + continue; + + /* Set pin to 'Non-use (Hi-Z input protection)' */ + pm &= ~(PM_MASK << (pin * 2)); + writew(pm, pctrl->base + PM(off)); + + /* Temporarily switch to GPIO mode with PMC register */ + pmc &= ~BIT(pin); + writeb(pmc, pctrl->base + PMC(off)); + + /* Select Pin function mode. */ + pfc &= ~(PFC_MASK << (pin * 4)); + pfc |= (cache->pfc[port] & (PFC_MASK << (pin * 4))); + writel(pfc, pctrl->base + PFC(off)); + + /* Switch to Peripheral pin function. */ + pmc |= BIT(pin); + writeb(pmc, pctrl->base + PMC(off)); + } + } + + /* Set the PWPR register to be write-protected. */ + writel(0x0, pctrl->base + regs->pwpr); /* B0WI=0, PFCWE=0 */ + writel(PWPR_B0WI, pctrl->base + regs->pwpr); /* B0WI=1, PFCWE=0 */ +} + +static int rzg2l_pinctrl_suspend_noirq(struct device *dev) +{ + struct rzg2l_pinctrl *pctrl = dev_get_drvdata(dev); + const struct rzg2l_hwcfg *hwcfg = pctrl->data->hwcfg; + const struct rzg2l_register_offsets *regs = &hwcfg->regs; + struct rzg2l_pinctrl_reg_cache *cache = pctrl->cache; + + rzg2l_pinctrl_pm_setup_regs(pctrl, true); + rzg2l_pinctrl_pm_setup_dedicated_regs(pctrl, true); + + for (u8 i = 0; i < 2; i++) { + cache->sd_ch[i] = readl(pctrl->base + SD_CH(regs->sd_ch, i)); + cache->eth_poc[i] = readl(pctrl->base + ETH_POC(regs->eth_poc, i)); + } + + cache->qspi = readl(pctrl->base + QSPI); + cache->eth_mode = readl(pctrl->base + ETH_MODE); + + if (!atomic_read(&pctrl->wakeup_source)) + clk_disable_unprepare(pctrl->clk); + + return 0; +} + +static int rzg2l_pinctrl_resume_noirq(struct device *dev) +{ + struct rzg2l_pinctrl *pctrl = dev_get_drvdata(dev); + const struct rzg2l_hwcfg *hwcfg = pctrl->data->hwcfg; + const struct rzg2l_register_offsets *regs = &hwcfg->regs; + struct rzg2l_pinctrl_reg_cache *cache = pctrl->cache; + int ret; + + if (!atomic_read(&pctrl->wakeup_source)) { + ret = clk_prepare_enable(pctrl->clk); + if (ret) + return ret; + } + + writel(cache->qspi, pctrl->base + QSPI); + writel(cache->eth_mode, pctrl->base + ETH_MODE); + for (u8 i = 0; i < 2; i++) { + writel(cache->sd_ch[i], pctrl->base + SD_CH(regs->sd_ch, i)); + writel(cache->eth_poc[i], pctrl->base + ETH_POC(regs->eth_poc, i)); + } + + rzg2l_pinctrl_pm_setup_pfc(pctrl); + rzg2l_pinctrl_pm_setup_regs(pctrl, false); + rzg2l_pinctrl_pm_setup_dedicated_regs(pctrl, false); + rzg2l_gpio_irq_restore(pctrl); + + return 0; +} + static const struct rzg2l_hwcfg rzg2l_hwcfg = { .regs = { .pwpr = 0x3014, @@ -2291,10 +2675,15 @@ static const struct of_device_id rzg2l_pinctrl_of_table[] = { { /* sentinel */ } }; +static const struct dev_pm_ops rzg2l_pinctrl_pm_ops = { + NOIRQ_SYSTEM_SLEEP_PM_OPS(rzg2l_pinctrl_suspend_noirq, rzg2l_pinctrl_resume_noirq) +}; + static struct platform_driver rzg2l_pinctrl_driver = { .driver = { .name = DRV_NAME, .of_match_table = of_match_ptr(rzg2l_pinctrl_of_table), + .pm = pm_sleep_ptr(&rzg2l_pinctrl_pm_ops), }, .probe = rzg2l_pinctrl_probe, };