pinctrl: samsung: Remove ugly hack for sharing eint_wakeup_mask
Subsystems (7)
Krzysztof Kozlowski
maintainerv2 cover pinctrl: samsung: Remove ugly hack for sharing eint_wakeup_mask
Hi All,
Changes since v1
================
1. Add Tomasz's ack.
2. Reword description in patch 6/10.
Tests
=====
This is both request for comments and requests for tests. Only basic tests were done, including suspend to RAM on Odroid U3 (Exynos4412) with max7768 RTC wakeup. Please kindly test it with devices capable of suspending and resuming. I am mostly thinking about S5Pv210-based (Aria), Trats, Trats2 and TM2 (Exynos5433). Existing platforms should not be broken however changing external interrupt wakeup mask was not done on Exynos5433.
Description
===========
The Exynos/S5Pv210 machine suspend code needs to write the external interrupt mask during suspend. The mask is controlled by pin controller
driver: the exynos_wkup_irq_set_wake() in IRQ chip for these wakeup interrupts.
Therefore pinctrl driver code exposed an exynos_get_eint_wake_mask() function which was later used as an extern in machine code.
This is quite ugly way of combining driver and machine code, not portable triggering Sparse and GCC warnings.
This might break suspend capability S5Pv210 on with older DTBs (thus breaks DTB compatibility), however:
1. just "might" because in case of using older DTB, the wakeup mask will not be changed during suspend and default reset value (all interrupts non-masked) should work,
2. mainline support for S5Pv210 with DTB is limited and suspend to RAM already might be broken.
Dependencies
============
1. The first seven patches should be taken through one tree, preferably samsung-pinctrl,
2. The DTS patch (7/10) for S5Pv210 should go into next cycle,
3. The remaining patches (8-10) should go after all previous, so probably another release cycle.
Best regards,
Krzysztof
Krzysztof Kozlowski (10):
pinctrl: samsung: Define suspend and resume callbacks for all banks
and SoCs
pinctrl: samsung: Document suspend and resume members
pinctrl: samsung: Document hidden requirement about one external
wakeup
pinctrl: samsung: Add dedicated compatible for S5Pv210 wakeup
interrupts
ARM: exynos: Define EINT_WAKEUP_MASK registers for S5Pv210 and
Exynos5433
pinctrl: samsung: Write external wakeup interrupt mask
ARM: dts: s5pv210: Switch to S5Pv210 specific pinctrl wakeup
compatible
ARM: s5pv210: Remove legacy setting of external wakeup interrupts
ARM: exynos: Remove legacy setting of external wakeup interrupts
pinctrl: samsung: Remove legacy API for handling external wakeup
interrupts mask
.../bindings/pinctrl/samsung-pinctrl.txt | 11 ++- arch/arm/boot/dts/s5pv210.dtsi | 2 +- arch/arm/mach-exynos/common.h | 2 - arch/arm/mach-exynos/suspend.c | 16 +++-- arch/arm/mach-s5pv210/common.h | 1 - arch/arm/mach-s5pv210/pm.c | 16 +++-- drivers/pinctrl/samsung/pinctrl-exynos-arm.c | 16 +++++ drivers/pinctrl/samsung/pinctrl-exynos.c | 78 +++++++++++++++++++--- drivers/pinctrl/samsung/pinctrl-samsung.h | 11 +++ include/linux/soc/samsung/exynos-regs-pmu.h | 8 ++- 10 files changed, 136 insertions(+), 25 deletions(-)
Signature · 1 line
2.14.1
Krzysztof Kozlowski
maintainerPATCH v2 1/10 pinctrl: samsung: Define suspend and resume callbacks for all banks and SoCs
Suspend and resume callbacks in Exynos/S5Pv210 pin controller drivers, save and restore state of registers. This operations should be done for all banks which have external interrupts (as denoted by using EXYNOS_PIN_BANK_EINTG/EINTW macros).
Add all banks of Exynos5260 and Exynos5420. This is necessary step for supporting suspend to RAM on these SoCs.
--- drivers/pinctrl/samsung/pinctrl-exynos-arm.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+)
drivers/pinctrl/samsung/pinctrl-exynos-arm.c +16 -0
@@ -616,16 +616,22 @@ static const struct samsung_pin_ctrl exynos5260_pin_ctrl[] __initconst = { .nr_banks = ARRAY_SIZE(exynos5260_pin_banks0), .eint_gpio_init = exynos_eint_gpio_init, .eint_wkup_init = exynos_eint_wkup_init, + .suspend = exynos_pinctrl_suspend, + .resume = exynos_pinctrl_resume, }, {/* pin-controller instance 1 data */ .pin_banks = exynos5260_pin_banks1, .nr_banks = ARRAY_SIZE(exynos5260_pin_banks1), .eint_gpio_init = exynos_eint_gpio_init, + .suspend = exynos_pinctrl_suspend, + .resume = exynos_pinctrl_resume, }, {/* pin-controller instance 2 data */ .pin_banks = exynos5260_pin_banks2, .nr_banks = ARRAY_SIZE(exynos5260_pin_banks2), .eint_gpio_init = exynos_eint_gpio_init, + .suspend = exynos_pinctrl_suspend, + .resume = exynos_pinctrl_resume, }, };
@@ -842,30 +848,40 @@ static const struct samsung_pin_ctrl exynos5420_pin_ctrl[] __initconst = { .nr_banks = ARRAY_SIZE(exynos5420_pin_banks0), .eint_gpio_init = exynos_eint_gpio_init, .eint_wkup_init = exynos_eint_wkup_init, + .suspend = exynos_pinctrl_suspend, + .resume = exynos_pinctrl_resume, .retention_data = &exynos5420_retention_data, }, {/* pin-controller instance 1 data */ .pin_banks = exynos5420_pin_banks1, .nr_banks = ARRAY_SIZE(exynos5420_pin_banks1), .eint_gpio_init = exynos_eint_gpio_init, + .suspend = exynos_pinctrl_suspend, + .resume = exynos_pinctrl_resume, .retention_data = &exynos5420_retention_data, }, {/* pin-controller instance 2 data */ .pin_banks = exynos5420_pin_banks2, .nr_banks = ARRAY_SIZE(exynos5420_pin_banks2), .eint_gpio_init = exynos_eint_gpio_init, + .suspend = exynos_pinctrl_suspend, + .resume = exynos_pinctrl_resume, .retention_data = &exynos5420_retention_data, }, {/* pin-controller instance 3 data */ .pin_banks = exynos5420_pin_banks3, .nr_banks = ARRAY_SIZE(exynos5420_pin_banks3), .eint_gpio_init = exynos_eint_gpio_init, + .suspend = exynos_pinctrl_suspend, + .resume = exynos_pinctrl_resume, .retention_data = &exynos5420_retention_data, }, {/* pin-controller instance 4 data */ .pin_banks = exynos5420_pin_banks4, .nr_banks = ARRAY_SIZE(exynos5420_pin_banks4), .eint_gpio_init = exynos_eint_gpio_init, + .suspend = exynos_pinctrl_suspend, + .resume = exynos_pinctrl_resume, .retention_data = &exynos4_audio_retention_data, }, };
Signature · 1 line
2.14.1
Re: [RFT v2 01/10] pinctrl: samsung: Define suspend and resume callbacks for all banks and SoCs
Quoted 8 lines · Krzysztof Kozlowski
Suspend and resume callbacks in Exynos/S5Pv210 pin controller drivers, save and restore state of registers. This operations should be done for all banks which have external interrupts (as denoted by using EXYNOS_PIN_BANK_EINTG/EINTW macros).
Add all banks of Exynos5260 and Exynos5420. This is necessary step for supporting suspend to RAM on these SoCs.
Quoted 7 lines
Signed-off-by Krzysztof Kozlowski <[email protected]>Cc Tomasz Figa <[email protected]>Cc Sylwester Nawrocki <[email protected]>Acked-by Tomasz Figa <[email protected]>--- drivers/pinctrl/samsung/pinctrl-exynos-arm.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+)
Thanks, applied.
Best regards,
Krzysztof
Krzysztof Kozlowski
maintainerPATCH v2 2/10 pinctrl: samsung: Document suspend and resume members
In reply to Krzysztof KozlowskiAdd missing documentation for suspend and resume members of struct samsung_pin_ctrl and samsung_pinctrl_drv_data.
--- drivers/pinctrl/samsung/pinctrl-samsung.h | 8 ++++++++ 1 file changed, 8 insertions(+)
drivers/pinctrl/samsung/pinctrl-samsung.h +8 -0
@@ -223,6 +223,10 @@ struct samsung_retention_data {* interrupts for the controller. * @eint_wkup_init: platform specific callback to setup the external wakeup * interrupts for the controller. +* @suspend: platform specific suspend callback, executed during pin controller +* device suspend, see samsung_pinctrl_suspend() +* @resume: platform specific resume callback, executed during pin controller +* device suspend, see samsung_pinctrl_resume() */ struct samsung_pin_ctrl {const struct samsung_pin_bank_data *pin_banks;
@@ -255,6 +259,10 @@ struct samsung_pin_ctrl {* @pin_base: starting system wide pin number. * @nr_pins: number of pins supported by the controller. * @retention_ctrl: retention control runtime data. +* @suspend: platform specific suspend callback, executed during pin controller +* device suspend, see samsung_pinctrl_suspend() +* @resume: platform specific resume callback, executed during pin controller +* device suspend, see samsung_pinctrl_resume() */ struct samsung_pinctrl_drv_data {struct list_head node;
Signature · 1 line
2.14.1
Krzysztof Kozlowski
maintainerPATCH v2 3/10 pinctrl: samsung: Document hidden requirement about one external wakeup
In reply to Krzysztof KozlowskiHardware (S5Pv210 and all Exynos SoCs) provides only 32 external interrupts which can wakeup device from deep sleep modes. On S5Pv210 these are gph0-gph3. On all Exynos designs these are gpx0-gpx3. There is only one 32-bit register for controlling the external wakeup interrupt mask (masking and unmasking waking capability of these interrupts).
This lead to implementation in pinctrl driver and machine code which was using static memory for storing the mask value and not caring about multiple devices of pin controller... because only one pin controller device will be handling this.
Since each pin controller node in Device Tree maps onto one device, this corresponds to hidden assumption in parsing the Device Tree: external wakeup interrupts can be defined only once. Make this assumption an explicit requirement.
--- Documentation/devicetree/bindings/pinctrl/samsung-pinctrl.txt | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)
Documentation/devicetree/bindings/pinctrl/samsung-pinctrl.txt +7 -2
@@ -145,8 +145,13 @@ A. External GPIO Interrupts: For supporting external gpio interrupts, the B. External Wakeup Interrupts: For supporting external wakeup interrupts, a child node representing the external wakeup interrupt controller should be - included in the pin-controller device node. This child node should include - the following properties. + included in the pin-controller device node. + + Only one pin-controller device node can include external wakeup interrupts + child node (in other words, only one External Wakeup Interrupts + pin-controller is supported). + + This child node should include following properties: - compatible: identifies the type of the external wakeup interrupt controller The possible values are:
Signature · 1 line
2.14.1
Re: [RFT v2 03/10] pinctrl: samsung: Document hidden requirement about one external wakeup
Quoted 17 lines · Krzysztof Kozlowski
Hardware (S5Pv210 and all Exynos SoCs) provides only 32 external interrupts which can wakeup device from deep sleep modes. On S5Pv210 these are gph0-gph3. On all Exynos designs these are gpx0-gpx3. There is only one 32-bit register for controlling the external wakeup interrupt mask (masking and unmasking waking capability of these interrupts).
This lead to implementation in pinctrl driver and machine code which was using static memory for storing the mask value and not caring about multiple devices of pin controller... because only one pin controller device will be handling this.
Since each pin controller node in Device Tree maps onto one device, this corresponds to hidden assumption in parsing the Device Tree: external wakeup interrupts can be defined only once. Make this assumption an explicit requirement.
Quoted 7 lines
Signed-off-by Krzysztof Kozlowski <[email protected]>Cc Tomasz Figa <[email protected]>Cc Sylwester Nawrocki <[email protected]>Acked-by Tomasz Figa <[email protected]>--- Documentation/devicetree/bindings/pinctrl/samsung-pinctrl.txt | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)
Krzysztof Kozlowski
maintainerPATCH v2 4/10 pinctrl: samsung: Add dedicated compatible for S5Pv210 wakeup interrupts
In reply to Krzysztof KozlowskiThe S5Pv210 external wakeup interrupts differ from Exynos therefore separate compatible is needed. Duplicate existing flavor specific data from exynos4210_wkup_irq_chip and add new compatible for S5Pv210. At this point this new compatible does not bring anything new and works exactly as existing "samsung,exynos4210-wakeup-eint".
--- .../devicetree/bindings/pinctrl/samsung-pinctrl.txt | 2 ++ drivers/pinctrl/samsung/pinctrl-exynos.c | 18 ++++++++++++++++++ 2 files changed, 20 insertions(+)
Documentation/devicetree/bindings/pinctrl/samsung-pinctrl.txt +2 -0
@@ -161,6 +161,8 @@ B. External Wakeup Interrupts: For supporting external wakeup interrupts, a found on Samsung S3C2412 and S3C2413 SoCs, - samsung,s3c64xx-wakeup-eint: represents wakeup interrupt controller found on Samsung S3C64xx SoCs, + - samsung,s5pv210-wakeup-eint: represents wakeup interrupt controller + found on Samsung S5Pv210 SoCs, - samsung,exynos4210-wakeup-eint: represents wakeup interrupt controller found on Samsung Exynos4210 and S5PC110/S5PV210 SoCs. - samsung,exynos7-wakeup-eint: represents wakeup interrupt controller
drivers/pinctrl/samsung/pinctrl-exynos.c +18 -0
@@ -346,6 +346,22 @@ static int exynos_wkup_irq_set_wake(struct irq_data *irqd, unsigned int on)/* * irq_chip for wakeup interrupts */ +static const struct exynos_irq_chip s5pv210_wkup_irq_chip__initconst = { + .chip = { + .name ="s5pv210_wkup_irq_chip" , + .irq_unmask = exynos_irq_unmask, + .irq_mask = exynos_irq_mask, + .irq_ack = exynos_irq_ack, + .irq_set_type = exynos_irq_set_type, + .irq_set_wake = exynos_wkup_irq_set_wake, + .irq_request_resources = exynos_irq_request_resources, + .irq_release_resources = exynos_irq_release_resources, + }, + .eint_con =EXYNOS_WKUP_ECON_OFFSET , + .eint_mask =EXYNOS_WKUP_EMASK_OFFSET , + .eint_pend =EXYNOS_WKUP_EPEND_OFFSET , +}; +static const struct exynos_irq_chip exynos4210_wkup_irq_chip__initconst = { .chip = { .name ="exynos4210_wkup_irq_chip" ,
@@ -380,6 +396,8 @@ static const struct exynos_irq_chip exynos7_wkup_irq_chip __initconst = {/* list of external wakeup controllers supported */ static const struct of_device_id exynos_wkup_irq_ids[] = { + { .compatible ="samsung,s5pv210-wakeup-eint" , + .data = &s5pv210_wkup_irq_chip }, { .compatible ="samsung,exynos4210-wakeup-eint" , .data = &exynos4210_wkup_irq_chip }, { .compatible ="samsung,exynos7-wakeup-eint" ,
Signature · 1 line
2.14.1
Re: [RFT v2 04/10] pinctrl: samsung: Add dedicated compatible for S5Pv210 wakeup interrupts
Quoted 6 lines · Krzysztof Kozlowski
The S5Pv210 external wakeup interrupts differ from Exynos therefore separate compatible is needed. Duplicate existing flavor specific data from
exynos4210_wkup_irq_chipand add new compatible for S5Pv210. At this point this new compatible does not bring anything new and works exactly as existing "samsung,exynos4210-wakeup-eint".
Quoted 6 lines
Signed-off-by Krzysztof Kozlowski <[email protected]>Cc Tomasz Figa <[email protected]>Cc Sylwester Nawrocki <[email protected]>Acked-by Tomasz Figa <[email protected]>--- .../devicetree/bindings/pinctrl/samsung-pinctrl.txt | 2 ++
Quoted 2 lines
drivers/pinctrl/samsung/pinctrl-exynos.c | 18 ++++++++++++++++++ 2 files changed, 20 insertions(+)
Krzysztof Kozlowski
maintainerPATCH v2 5/10 ARM: exynos: Define EINT_WAKEUP_MASK registers for S5Pv210 and Exynos5433
In reply to Krzysztof KozlowskiS5Pv210 and Exynos5433/Exynos7 have different address of EINT_WAKEUP_MASK register. Rename existing S5P_EINT_WAKEUP_MASK to avoid confusion and add new ones.
--- arch/arm/mach-exynos/suspend.c | 2 +- include/linux/soc/samsung/exynos-regs-pmu.h | 6 +++++- 2 files changed, 6 insertions(+), 2 deletions(-)
arch/arm/mach-exynos/suspend.c +1 -1
@@ -272,7 +272,7 @@ static int exynos5420_cpu_suspend(unsigned long arg)static void exynos_pm_set_wakeup_mask (void ) {/* Set wake-up mask registers */ -pmu_raw_writel (exynos_get_eint_wake_mask (),S5P_EINT_WAKEUP_MASK ); +pmu_raw_writel (exynos_get_eint_wake_mask (),EXYNOS_EINT_WAKEUP_MASK );pmu_raw_writel (exynos_irqwake_intmask & ~(1 <<31 ),S5P_WAKEUP_MASK ); }
include/linux/soc/samsung/exynos-regs-pmu.h +5 -1
@@ -42,7 +42,7 @@#define EXYNOS_SWRESET 0x0400#define S5P_WAKEUP_STAT 0x0600 -#define S5P_EINT_WAKEUP_MASK 0x0604 +#define EXYNOS_EINT_WAKEUP_MASK 0x0604#define S5P_WAKEUP_MASK 0x0608#define S5P_WAKEUP_MASK2 0x0614
@@ -180,6 +180,9 @@#define S5P_CORE_WAKEUP_FROM_LOCAL_CFG (0x3 << 8)#define S5P_CORE_AUTOWAKEUP_EN (1 << 31) +/* Only for S5Pv210 */ +#define S5PV210_EINT_WAKEUP_MASK 0xC004 +/* Only for EXYNOS4210 */ #define S5P_CMU_CLKSTOP_LCD1_LOWPWR 0x1154#define S5P_CMU_RESET_LCD1_LOWPWR 0x1174
@@ -641,6 +644,7 @@ | EXYNOS5420_KFC_USE_STANDBY_WFI3)/* For EXYNOS5433 */ +#define EXYNOS5433_EINT_WAKEUP_MASK (0x060C)#define EXYNOS5433_USBHOST30_PHY_CONTROL (0x0728)#define EXYNOS5433_PAD_RETENTION_AUD_OPTION (0x3028)#define EXYNOS5433_PAD_RETENTION_MMC2_OPTION (0x30C8)
Signature · 1 line
2.14.1
Krzysztof Kozlowski
maintainerPATCH v2 6/10 pinctrl: samsung: Write external wakeup interrupt mask
In reply to Krzysztof KozlowskiThe pinctrl driver defines an IRQ chip which handles external wakeup interrupts, therefore from logical point of view, it is the owner of external interrupt mask. The register controlling the mask belongs to Power Management Unit address space so it has to be accessed with PMU syscon regmap handle.
This mask should be written to hardware during system suspend. Till now ARMv7 machine code was responsible for this which created a dependency between pin controller driver and arch/arm/mach code.
Try to rework this dependency so the pinctrl driver will write external wakeup interrupt mask during late suspend.
Impact on ARMv7 designs (S5Pv210 and Exynos)
============================================
This duplicates setting mask with existing machine code arch/arm/mach-exynos/suspend.c and arch/arm/mach-s5pv210/pm.c but it is not a problem - the wakeup mask register will be written twice. The machine code will be cleaned up later.
The difference between implementation here and ARMv7 machine code (arch/arm/mach-*) is the time of writing the mask:
1. The machine code is writing the mask quite late during system suspend path, after offlining secondary CPUs and just before doing actual suspend.
2. The implementation in pinctrl driver uses late suspend ops, therefore it will write the mask much earlier. Hopefully late enough, after all drivers will enable or disable its interrupt wakeup (enable_irq_wake() etc).
Impact on ARMv8 designs (Exynos5433 and Exynos7)
================================================
The Suspend to RAM was not supported and external wakeup interrupt mask was not written to HW. This change brings us one step closer to supporting Suspend to RAM.
--- drivers/pinctrl/samsung/pinctrl-exynos.c | 50 ++++++++++++++++++++++++++++- drivers/pinctrl/samsung/pinctrl-samsung.h | 3 ++ include/linux/soc/samsung/exynos-regs-pmu.h | 2 ++ 3 files changed, 54 insertions(+), 1 deletion(-)
drivers/pinctrl/samsung/pinctrl-exynos.c +49 -1
@@ -25,6 +25,7 @@#include <linux/regmap.h> #include <linux/err.h> #include <linux/soc/samsung/exynos-pmu.h> +#include <linux/soc/samsung/exynos-regs-pmu.h> #include <dt-bindings/pinctrl/samsung.h>
@@ -37,6 +38,8 @@ struct exynos_irq_chip {u32 eint_con;u32 eint_mask;u32 eint_pend; +u32 eint_wake_mask_value; +u32 eint_wake_mask_reg; };static inline struct exynos_irq_chip *to_exynos_irq_chip (struct irq_chip *chip)
@@ -215,6 +218,7 @@ static struct exynos_irq_chip exynos_gpio_irq_chip = { .eint_con = EXYNOS_GPIO_ECON_OFFSET, .eint_mask = EXYNOS_GPIO_EMASK_OFFSET, .eint_pend = EXYNOS_GPIO_EPEND_OFFSET, +/* eint_wake_mask_value not used */ }; static int exynos_eint_irq_map(struct irq_domain *h, unsigned int virq,
@@ -330,6 +334,8 @@ u32 exynos_get_eint_wake_mask(void)static int exynos_wkup_irq_set_wake (struct irq_data *irqd,unsigned int on) { +struct irq_chip *chip =irq_data_get_irq_chip (irqd); +struct exynos_irq_chip *our_chip =to_exynos_irq_chip (chip);struct samsung_pin_bank *bank =irq_data_get_irq_chip_data (irqd);unsigned long bit =1UL << (2 * bank->eint_offset + irqd->hwirq);
@@ -339,6 +345,7 @@ static int exynos_wkup_irq_set_wake(struct irq_data *irqd, unsigned int on) exynos_eint_wake_mask |= bit; else exynos_eint_wake_mask &= ~bit; + our_chip->eint_wake_mask_value = exynos_eint_wake_mask;return 0 ; }
@@ -360,6 +367,9 @@ static const struct exynos_irq_chip s5pv210_wkup_irq_chip __initconst = { .eint_con = EXYNOS_WKUP_ECON_OFFSET, .eint_mask = EXYNOS_WKUP_EMASK_OFFSET, .eint_pend = EXYNOS_WKUP_EPEND_OFFSET, + .eint_wake_mask_value = EXYNOS_EINT_WAKEUP_MASK_DISABLED, + /* Only difference with exynos4210_wkup_irq_chip: */ + .eint_wake_mask_reg = S5PV210_EINT_WAKEUP_MASK, };static const struct exynos_irq_chip exynos4210_wkup_irq_chip__initconst = {
@@ -376,6 +386,8 @@ static const struct exynos_irq_chip exynos4210_wkup_irq_chip __initconst = { .eint_con = EXYNOS_WKUP_ECON_OFFSET, .eint_mask = EXYNOS_WKUP_EMASK_OFFSET, .eint_pend = EXYNOS_WKUP_EPEND_OFFSET, + .eint_wake_mask_value = EXYNOS_EINT_WAKEUP_MASK_DISABLED, + .eint_wake_mask_reg = EXYNOS_EINT_WAKEUP_MASK, };static const struct exynos_irq_chip exynos7_wkup_irq_chip__initconst = {
@@ -392,6 +404,8 @@ static const struct exynos_irq_chip exynos7_wkup_irq_chip __initconst = { .eint_con = EXYNOS7_WKUP_ECON_OFFSET, .eint_mask = EXYNOS7_WKUP_EMASK_OFFSET, .eint_pend = EXYNOS7_WKUP_EPEND_OFFSET, + .eint_wake_mask_value = EXYNOS_EINT_WAKEUP_MASK_DISABLED, + .eint_wake_mask_reg = EXYNOS5433_EINT_WAKEUP_MASK, };/* list of external wakeup controllers supported */
@@ -560,6 +574,27 @@ int exynos_eint_wkup_init(struct samsung_pinctrl_drv_data *d)return 0 ; } +static void +exynos_pinctrl_set_eint_wakeup_mask (struct samsung_pinctrl_drv_data *drvdata, +struct exynos_irq_chip *irq_chip) +{ +struct regmap *pmu_regs; + +if (!drvdata->retention_ctrl || !drvdata->retention_ctrl->priv) { +dev_warn (drvdata->dev, +"No retention data configured bank with external wakeup interrupt. Wake-up mask will not be set.\n" ); +return ; + } + + pmu_regs = drvdata->retention_ctrl->priv; +dev_info (drvdata->dev, +"Setting external wakeup interrupt wakeup mask: 0x%x\n" , + irq_chip->eint_wake_mask_value); + +regmap_write (pmu_regs, irq_chip->eint_wake_mask_reg, + irq_chip->eint_wake_mask_value); +} +static void exynos_pinctrl_suspend_bank (struct samsung_pinctrl_drv_data *drvdata,struct samsung_pin_bank *bank)
@@ -582,11 +617,24 @@ static void exynos_pinctrl_suspend_bank(void exynos_pinctrl_suspend (struct samsung_pinctrl_drv_data *drvdata) {struct samsung_pin_bank *bank = drvdata->pin_banks; +struct exynos_irq_chip *irq_chip =NULL ;int i; -for (i =0 ; i < drvdata->nr_banks; ++i, ++bank) +for (i =0 ; i < drvdata->nr_banks; ++i, ++bank) {if (bank->eint_type ==EINT_TYPE_GPIO )exynos_pinctrl_suspend_bank (drvdata, bank); +else if (bank->eint_type ==EINT_TYPE_WKUP ) { +if (!irq_chip) { + irq_chip = bank->irq_chip; +exynos_pinctrl_set_eint_wakeup_mask (drvdata, + irq_chip); + }else if (bank->irq_chip != irq_chip) { +dev_warn (drvdata->dev, +"More than one external wakeup interrupt chip configured (bank: %s). This is not supported by hardware nor by driver.\n" , + bank->name); + } + } + } } static void exynos_pinctrl_resume_bank(
drivers/pinctrl/samsung/pinctrl-samsung.h +3 -0
@@ -227,6 +227,9 @@ struct samsung_retention_data {* device suspend, see samsung_pinctrl_suspend() * @resume: platform specific resume callback, executed during pin controller * device suspend, see samsung_pinctrl_resume() +* +* External wakeup interrupts must define at least eint_wkup_init, +* retention_data and suspend in order for proper suspend/resume to work. */ struct samsung_pin_ctrl {const struct samsung_pin_bank_data *pin_banks;
include/linux/soc/samsung/exynos-regs-pmu.h +2 -0
@@ -42,6 +42,8 @@#define EXYNOS_SWRESET 0x0400#define S5P_WAKEUP_STAT 0x0600 +/* Value for EXYNOS_EINT_WAKEUP_MASK disabling all external wakeup interrupts */ +#define EXYNOS_EINT_WAKEUP_MASK_DISABLED 0xffffffff#define EXYNOS_EINT_WAKEUP_MASK 0x0604#define S5P_WAKEUP_MASK 0x0608#define S5P_WAKEUP_MASK2 0x0614
Signature · 1 line
2.14.1
Re: [RFT v2 06/10] pinctrl: samsung: Write external wakeup interrupt mask
Quoted 40 lines · Krzysztof Kozlowski
The pinctrl driver defines an IRQ chip which handles external wakeup interrupts, therefore from logical point of view, it is the owner of external interrupt mask. The register controlling the mask belongs to Power Management Unit address space so it has to be accessed with PMU syscon regmap handle.
This mask should be written to hardware during system suspend. Till now ARMv7 machine code was responsible for this which created a dependency between pin controller driver and
arch/arm/machcode.Try to rework this dependency so the pinctrl driver will write external wakeup interrupt mask during late suspend.
Impact on ARMv7 designs (S5Pv210 and Exynos)
============================================
This duplicates setting mask with existing machine codearch/arm/mach-exynos/suspend.candarch/arm/mach-s5pv210/pm.cbut it is not a problem - the wakeup mask register will be written twice. The machine code will be cleaned up later.The difference between implementation here and ARMv7 machine code (
arch/arm/mach-*) is the time of writing the mask:
1. The machine code is writing the mask quite late during system suspend path, after offlining secondary CPUs and just before doing actual suspend.
2. The implementation in pinctrl driver uses late suspend ops, therefore it will write the mask much earlier. Hopefully late enough, after all drivers will enable or disable its interrupt wakeup (enable_irq_wake()etc).Impact on ARMv8 designs (Exynos5433 and Exynos7)
================================================
The Suspend to RAM was not supported and external wakeup interrupt mask was not written to HW. This change brings us one step closer to supporting Suspend to RAM.Signed-off-by Krzysztof Kozlowski <[email protected]>Cc Tomasz Figa <[email protected]>Cc Sylwester Nawrocki <[email protected]>Acked-by Tomasz Figa <[email protected]>
Quoted 5 lines
--- drivers/pinctrl/samsung/pinctrl-exynos.c | 50 ++++++++++++++++++++++++++++- drivers/pinctrl/samsung/pinctrl-samsung.h | 3 ++ include/linux/soc/samsung/exynos-regs-pmu.h | 2 ++ 3 files changed, 54 insertions(+), 1 deletion(-)
Thanks, applied all previous, up to this one.
Best regards,
Krzysztof
Krzysztof Kozlowski
maintainerPATCH v2 7/10 ARM: dts: s5pv210: Switch to S5Pv210 specific pinctrl wakeup compatible
In reply to Krzysztof KozlowskiThe pin controller block of S5Pv210 for handling external wakeup interrupts is different than in newer designs (Exynos).
--- arch/arm/boot/dts/s5pv210.dtsi | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
arch/arm/boot/dts/s5pv210.dtsi +1 -1
@@ -120,7 +120,7 @@ interrupts = <30>; wakeup-interrupt-controller { - compatible = "samsung,exynos4210-wakeup-eint"; + compatible = "samsung,s5pv210-wakeup-eint"; interrupts = <16>; interrupt-parent = <&vic0>; };
Signature · 1 line
2.14.1
Krzysztof Kozlowski
maintainerPATCH v2 8/10 ARM: s5pv210: Remove legacy setting of external wakeup interrupts
In reply to Krzysztof KozlowskiSince Exynos/S5Pv210 pin-controller driver is taking care about setting the external wakeup interrupts mask, the legacy code can be removed.
--- arch/arm/mach-s5pv210/common.h | 1 - arch/arm/mach-s5pv210/pm.c | 16 ++++++++++++---- 2 files changed, 12 insertions(+), 5 deletions(-)
arch/arm/mach-s5pv210/common.h +0 -1
@@ -10,7 +10,6 @@#define __ARCH_ARM_MACH_S5PV210_COMMON_H#ifdef CONFIG_PM_SLEEP -u32 exynos_get_eint_wake_mask (void );void s5pv210_cpu_resume (void );void s5pv210_pm_init (void );#else
arch/arm/mach-s5pv210/pm.c +12 -4
@@ -32,6 +32,11 @@ static struct sleep_save s5pv210_core_save[] = {*/ static u32 s5pv210_irqwake_intmask =0xffffffff ; +static u32 s5pv210_read_eint_wakeup_mask (void ) +{ +return __raw_readl (S5P_EINT_WAKEUP_MASK ); +} +/* * Suspend helpers. */
@@ -59,8 +64,10 @@ static void s5pv210_pm_prepare(void) {unsigned int tmp; -/* Set wake-up mask registers */ -__raw_writel (exynos_get_eint_wake_mask (),S5P_EINT_WAKEUP_MASK ); +/* +* Set wake-up mask registers +* S5P_EINT_WAKEUP_MASK is set by pinctrl driver in late suspend. +*/ __raw_writel (s5pv210_irqwake_intmask,S5P_WAKEUP_MASK );/* ensure at least INFORM0 has the resume address */
@@ -89,6 +96,7 @@ static void s5pv210_pm_prepare(void)*/ static int s5pv210_suspend_enter (suspend_state_t state) { +u32 eint_wakeup_mask =s5pv210_read_eint_wakeup_mask ();int ret;s3c_pm_debug_init ();
@@ -96,10 +104,10 @@ static int s5pv210_suspend_enter(suspend_state_t state)S3C_PMDBG ("%s: suspending the system...\n" , __func__);S3C_PMDBG ("%s: wakeup masks: %08x,%08x\n" , __func__, - s5pv210_irqwake_intmask,exynos_get_eint_wake_mask ()); + s5pv210_irqwake_intmask, eint_wakeup_mask);if (s5pv210_irqwake_intmask ==-1U - &&exynos_get_eint_wake_mask () ==-1U ) { + && eint_wakeup_mask ==-1U ) {pr_err ("%s: No wake-up sources!\n" , __func__);pr_err ("%s: Aborting sleep\n" , __func__);return -EINVAL ;
Signature · 1 line
2.14.1
Krzysztof Kozlowski
maintainerPATCH v2 9/10 ARM: exynos: Remove legacy setting of external wakeup interrupts
In reply to Krzysztof KozlowskiSince Exynos/S5Pv210 pin-controller driver is taking care about setting the external wakeup interrupts mask, the legacy code can be removed.
--- arch/arm/mach-exynos/common.h | 2 -- arch/arm/mach-exynos/suspend.c | 16 ++++++++++++---- 2 files changed, 12 insertions(+), 6 deletions(-)
arch/arm/mach-exynos/common.h +0 -2
@@ -113,8 +113,6 @@ void exynos_firmware_init(void);void exynos_set_boot_flag (unsigned int cpu,unsigned int mode);void exynos_clear_boot_flag (unsigned int cpu,unsigned int mode); -extern u32 exynos_get_eint_wake_mask (void ); -#ifdef CONFIG_PM_SLEEP extern void __init exynos_pm_init (void );#else
arch/arm/mach-exynos/suspend.c +12 -4
@@ -88,6 +88,11 @@ static const struct exynos_wkup_irq exynos5250_wkup_irq[] = { { /* sentinel */ }, }; +static u32 exynos_read_eint_wakeup_mask (void ) +{ +return pmu_raw_readl (EXYNOS_EINT_WAKEUP_MASK ); +} +static int exynos_irq_set_wake (struct irq_data *data,unsigned int state) {const struct exynos_wkup_irq *wkup_irq;
@@ -271,8 +276,10 @@ static int exynos5420_cpu_suspend(unsigned long arg)static void exynos_pm_set_wakeup_mask (void ) { -/* Set wake-up mask registers */ -pmu_raw_writel (exynos_get_eint_wake_mask (),EXYNOS_EINT_WAKEUP_MASK ); +/* +* Set wake-up mask registers +* EXYNOS_EINT_WAKEUP_MASK is set by pinctrl driver in late suspend. +*/ pmu_raw_writel (exynos_irqwake_intmask & ~(1 <<31 ),S5P_WAKEUP_MASK ); }
@@ -482,6 +489,7 @@ static void exynos5420_pm_resume(void)static int exynos_suspend_enter (suspend_state_t state) { +u32 eint_wakeup_mask =exynos_read_eint_wakeup_mask ();int ret;s3c_pm_debug_init ();
@@ -489,10 +497,10 @@ static int exynos_suspend_enter(suspend_state_t state)S3C_PMDBG ("%s: suspending the system...\n" , __func__);S3C_PMDBG ("%s: wakeup masks: %08x,%08x\n" , __func__, - exynos_irqwake_intmask,exynos_get_eint_wake_mask ()); + exynos_irqwake_intmask, eint_wakeup_mask);if (exynos_irqwake_intmask ==-1U - &&exynos_get_eint_wake_mask () ==-1U ) { + && eint_wakeup_mask ==EXYNOS_EINT_WAKEUP_MASK_DISABLED ) {pr_err ("%s: No wake-up sources!\n" , __func__);pr_err ("%s: Aborting sleep\n" , __func__);return -EINVAL ;
Signature · 1 line
2.14.1
Krzysztof Kozlowski
maintainerPATCH v2 10/10 pinctrl: samsung: Remove legacy API for handling external wakeup interrupts mask
In reply to Krzysztof KozlowskiRemove the legacy, ugly API of exposing the static value of external wakeup interrupts mask, because all arch-machine users where converted to use generic implementation from pinctrl driver.
--- drivers/pinctrl/samsung/pinctrl-exynos.c | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-)
drivers/pinctrl/samsung/pinctrl-exynos.c +2 -10
@@ -325,13 +325,6 @@ int exynos_eint_gpio_init(struct samsung_pinctrl_drv_data *d)return ret; } -static u32 exynos_eint_wake_mask =0xffffffff ; - -u32 exynos_get_eint_wake_mask (void ) -{ -return exynos_eint_wake_mask; -} -static int exynos_wkup_irq_set_wake (struct irq_data *irqd,unsigned int on) {struct irq_chip *chip =irq_data_get_irq_chip (irqd);
@@ -342,10 +335,9 @@ static int exynos_wkup_irq_set_wake(struct irq_data *irqd, unsigned int on)pr_info ("wake %s for irq %d\n" , on ?"enabled" :"disabled" , irqd->irq);if (!on) - exynos_eint_wake_mask |= bit; + our_chip->eint_wake_mask_value |= bit;else - exynos_eint_wake_mask &= ~bit; - our_chip->eint_wake_mask_value = exynos_eint_wake_mask; + our_chip->eint_wake_mask_value &= ~bit;return 0 ; }
Signature · 1 line
2.14.1
Hi Krzysztof,
Quoted 8 lines · Krzysztof Kozlowski
Hi All,
Changes since v1
================
1. Add Tomasz's ack.
2. Reword description in patch 6/10.
Quoted 9 lines
Tests
=====
This is both request for comments and requests for tests. Only basic tests were done, including suspend to RAM on Odroid U3 (Exynos4412) with max7768 RTC wakeup. Please kindly test it with devices capable of suspending and resuming. I am mostly thinking about S5Pv210-based (Aria), Trats, Trats2 and TM2 (Exynos5433). Existing platforms should not be broken however changing external interrupt wakeup mask was not done on Exynos5433.
Works fine on Exynos5433 TM2 with some additional patches related to suspend/resume handling of the PMU. It handles only EINT0..EINT31 pins for now. Exynos5433 has more external interrupt wakeup lines: EINT32-63 located at banks GPF1[0..7], GPF2[0..3], GPF3[0..3], GPF4[0..7] and GPF5[0..7]. They can be configured as wakeup sources in EXYNOS5433_EINT_WAKEUP_MASK1 (0x062C) PMU register. Handling of them can be added later when it will be really needed.
Quoted 53 lines
Description
===========
The Exynos/S5Pv210 machine suspend code needs to write the external interrupt mask during suspend. The mask is controlled by pin controller
driver: theexynos_wkup_irq_set_wake()in IRQ chip for these wakeup interrupts.Therefore pinctrl driver code exposed an
exynos_get_eint_wake_mask()function which was later used as an extern in machine code.This is quite ugly way of combining driver and machine code, not portable triggering Sparse and GCC warnings.
This might break suspend capability S5Pv210 on with older DTBs (thus breaks DTB compatibility), however:
1. just "might" because in case of using older DTB, the wakeup mask will not be changed during suspend and default reset value (all interrupts non-masked) should work,
2. mainline support for S5Pv210 with DTB is limited and suspend to RAM already might be broken.Dependencies
============
1. The first seven patches should be taken through one tree, preferably samsung-pinctrl,
2. The DTS patch (7/10) for S5Pv210 should go into next cycle,
3. The remaining patches (8-10) should go after all previous, so probably another release cycle.Best regards,
KrzysztofKrzysztof Kozlowski (10):
pinctrl: samsung: Define suspend and resume callbacks for all banks
and SoCs
pinctrl: samsung: Document suspend and resume members
pinctrl: samsung: Document hidden requirement about one external
wakeup
pinctrl: samsung: Add dedicated compatible for S5Pv210 wakeup
interrupts
ARM: exynos: DefineEINT_WAKEUP_MASKregisters for S5Pv210 andExynos5433pinctrl: samsung: Write external wakeup interrupt mask
ARM: dts: s5pv210: Switch to S5Pv210 specific pinctrl wakeup
compatible
ARM: s5pv210: Remove legacy setting of external wakeup interrupts
ARM: exynos: Remove legacy setting of external wakeup interrupts
pinctrl: samsung: Remove legacy API for handling external wakeup
interrupts mask
Quoted 12 lines
.../bindings/pinctrl/samsung-pinctrl.txt | 11 ++- arch/arm/boot/dts/s5pv210.dtsi | 2 +- arch/arm/mach-exynos/common.h | 2 - arch/arm/mach-exynos/suspend.c | 16 +++-- arch/arm/mach-s5pv210/common.h | 1 - arch/arm/mach-s5pv210/pm.c | 16 +++-- drivers/pinctrl/samsung/pinctrl-exynos-arm.c | 16 +++++ drivers/pinctrl/samsung/pinctrl-exynos.c | 78 +++++++++++++++++++--- drivers/pinctrl/samsung/pinctrl-samsung.h | 11 +++ include/linux/soc/samsung/exynos-regs-pmu.h | 8 ++- 10 files changed, 136 insertions(+), 25 deletions(-)
Best regards
Signature · 2 lines
Marek Szyprowski, PhD Samsung R&D Institute Poland
Krzysztof Kozlowski
maintainerQuoted 29 lines · Marek Szyprowski
Hi Krzysztof,
Quoted 8 lines · Krzysztof Kozlowski
Hi All,
Changes since v1
================
1. Add Tomasz's ack.
2. Reword description in patch 6/10.Quoted 9 lines
Tests
=====
This is both request for comments and requests for tests. Only basic tests were done, including suspend to RAM on Odroid U3 (Exynos4412) with max7768 RTC wakeup. Please kindly test it with devices capable of suspending and resuming. I am mostly thinking about S5Pv210-based (Aria), Trats, Trats2 and TM2 (Exynos5433). Existing platforms should not be broken however changing external interrupt wakeup mask was not done on Exynos5433.Works fine on Exynos5433 TM2 with some additional patches related to suspend/resume handling of the PMU. It handles only EINT0..EINT31 pins for now. Exynos5433 has more external interrupt wakeup lines: EINT32-63 located at banks GPF1[0..7], GPF2[0..3], GPF3[0..3], GPF4[0..7] and GPF5[0..7]. They can be configured as wakeup sources in
EXYNOS5433_EINT_WAKEUP_MASK1(0x062C) PMU register. Handling of them can be added later when it will be really needed.
Quoted 1 line
Tested-by Marek Szyprowski <[email protected]>
Thanks for comments and testing. Indeed support for Exynos5433 has to be improved.
I would like to push 1-6 (which are needed for the rest) through current cycle so I applied them to samsung-pinctrl tree.
Since anyway suspend to RAM is mostly broken nowadays... nothing more should pop up.
Best regards,
Krzysztof