Re: [PATCH v4 04/10] irqchip/sun6i-r: Add wakeup support
On 1/14/21 3:44 PM, Marc Zyngier wrote:
> On Tue, 12 Jan 2021 05:59:44 +,
> Samuel Holland wrote:
>>
>> Maintain bitmaps of wake-enabled IRQs and mux inputs, and program them
>> to the hardware during the syscore phase of suspend and shutdown. Then
>> restore the original set of enabled IRQs (only the NMI) during resume.
>>
>> This serves two purposes. First, it lets power management firmware
>> running on the ARISC coprocessor know which wakeup sources Linux wants
>> to have enabled. That way, it can avoid turning them off when it shuts
>> down the remainder of the clock tree. Second, it preconfigures the
>> coprocessor's interrupt controller, so the firmware's wakeup logic
>> is as simple as waiting for an interrupt to arrive.
>>
>> The suspend/resume logic is not conditional on PM_SLEEP because it is
>> identical to the init/shutdown logic. Wake IRQs may be enabled during
>> shutdown to allow powering the board back on. As an example, see
>> commit a5c5e50cce9d ("Input: gpio-keys - add shutdown callback").
>>
>> Signed-off-by: Samuel Holland
>> ---
>> drivers/irqchip/irq-sun6i-r.c | 107 --
>> 1 file changed, 101 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/irqchip/irq-sun6i-r.c b/drivers/irqchip/irq-sun6i-r.c
>> index d04d067423f4..a1b58c98d6ca 100644
>> --- a/drivers/irqchip/irq-sun6i-r.c
>> +++ b/drivers/irqchip/irq-sun6i-r.c
>> @@ -39,6 +39,7 @@
>> * set of 128 mux bits. This requires a second set of top-level registers.
>> */
>>
>> +#include
>> #include
>> #include
>> #include
>> @@ -46,6 +47,7 @@
>> #include
>> #include
>> #include
>> +#include
>>
>> #include
>>
>> @@ -67,8 +69,17 @@
>> #define SUN6I_NR_DIRECT_IRQS16
>> #define SUN6I_NR_MUX_BITS 128
>>
>> +struct sun6i_r_intc_variant {
>> +u32 first_mux_irq;
>> +u32 nr_mux_irqs;
>> +u32 mux_valid[BITS_TO_U32(SUN6I_NR_MUX_BITS)];
>> +};
>> +
>> static void __iomem *base;
>> static irq_hw_number_t nmi_hwirq;
>> +static DECLARE_BITMAP(wake_irq_enabled, SUN6I_NR_TOP_LEVEL_IRQS);
>> +static DECLARE_BITMAP(wake_mux_enabled, SUN6I_NR_MUX_BITS);
>> +static DECLARE_BITMAP(wake_mux_valid, SUN6I_NR_MUX_BITS);
>>
>> static void sun6i_r_intc_ack_nmi(void)
>> {
>> @@ -145,6 +156,21 @@ static int sun6i_r_intc_nmi_set_irqchip_state(struct
>> irq_data *data,
>> return irq_chip_set_parent_state(data, which, state);
>> }
>>
>> +static int sun6i_r_intc_irq_set_wake(struct irq_data *data, unsigned int on)
>> +{
>> +unsigned long offset_from_nmi = data->hwirq - nmi_hwirq;
>> +
>> +if (offset_from_nmi < SUN6I_NR_DIRECT_IRQS)
>> +assign_bit(offset_from_nmi, wake_irq_enabled, on);
>> +else if (test_bit(data->hwirq, wake_mux_valid))
>> +assign_bit(data->hwirq, wake_mux_enabled, on);
>> +else
>> +/* Not wakeup capable. */
>> +return -EPERM;
>> +
>> +return 0;
>> +}
>> +
>> static struct irq_chip sun6i_r_intc_nmi_chip = {
>> .name = "sun6i-r-intc",
>> .irq_ack= sun6i_r_intc_nmi_ack,
>> @@ -154,8 +180,19 @@ static struct irq_chip sun6i_r_intc_nmi_chip = {
>> .irq_set_affinity = irq_chip_set_affinity_parent,
>> .irq_set_type = sun6i_r_intc_nmi_set_type,
>> .irq_set_irqchip_state = sun6i_r_intc_nmi_set_irqchip_state,
>> -.flags = IRQCHIP_SET_TYPE_MASKED |
>> - IRQCHIP_SKIP_SET_WAKE,
>> +.irq_set_wake = sun6i_r_intc_irq_set_wake,
>> +.flags = IRQCHIP_SET_TYPE_MASKED,
>> +};
>> +
>> +static struct irq_chip sun6i_r_intc_wakeup_chip = {
>> +.name = "sun6i-r-intc",
>> +.irq_mask = irq_chip_mask_parent,
>> +.irq_unmask = irq_chip_unmask_parent,
>> +.irq_eoi= irq_chip_eoi_parent,
>> +.irq_set_affinity = irq_chip_set_affinity_parent,
>> +.irq_set_type = irq_chip_set_type_parent,
>> +.irq_set_wake = sun6i_r_intc_irq_set_wake,
>> +.flags = IRQCHIP_SET_TYPE_MASKED,
>
> Worth implementing irq_get/set_irqchip_state() using the _parent
> helper, I guess.
This is the same situation as the previous patch. Assuming it is safe to
rely on the behavior of the top-level functions, adding the callbacks
here would be redundant.
Cheers,
Samuel
> Thanks,
>
> M.
>
Re: [PATCH v4 04/10] irqchip/sun6i-r: Add wakeup support
On Tue, 12 Jan 2021 05:59:44 +,
Samuel Holland wrote:
>
> Maintain bitmaps of wake-enabled IRQs and mux inputs, and program them
> to the hardware during the syscore phase of suspend and shutdown. Then
> restore the original set of enabled IRQs (only the NMI) during resume.
>
> This serves two purposes. First, it lets power management firmware
> running on the ARISC coprocessor know which wakeup sources Linux wants
> to have enabled. That way, it can avoid turning them off when it shuts
> down the remainder of the clock tree. Second, it preconfigures the
> coprocessor's interrupt controller, so the firmware's wakeup logic
> is as simple as waiting for an interrupt to arrive.
>
> The suspend/resume logic is not conditional on PM_SLEEP because it is
> identical to the init/shutdown logic. Wake IRQs may be enabled during
> shutdown to allow powering the board back on. As an example, see
> commit a5c5e50cce9d ("Input: gpio-keys - add shutdown callback").
>
> Signed-off-by: Samuel Holland
> ---
> drivers/irqchip/irq-sun6i-r.c | 107 --
> 1 file changed, 101 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/irqchip/irq-sun6i-r.c b/drivers/irqchip/irq-sun6i-r.c
> index d04d067423f4..a1b58c98d6ca 100644
> --- a/drivers/irqchip/irq-sun6i-r.c
> +++ b/drivers/irqchip/irq-sun6i-r.c
> @@ -39,6 +39,7 @@
> * set of 128 mux bits. This requires a second set of top-level registers.
> */
>
> +#include
> #include
> #include
> #include
> @@ -46,6 +47,7 @@
> #include
> #include
> #include
> +#include
>
> #include
>
> @@ -67,8 +69,17 @@
> #define SUN6I_NR_DIRECT_IRQS 16
> #define SUN6I_NR_MUX_BITS128
>
> +struct sun6i_r_intc_variant {
> + u32 first_mux_irq;
> + u32 nr_mux_irqs;
> + u32 mux_valid[BITS_TO_U32(SUN6I_NR_MUX_BITS)];
> +};
> +
> static void __iomem *base;
> static irq_hw_number_t nmi_hwirq;
> +static DECLARE_BITMAP(wake_irq_enabled, SUN6I_NR_TOP_LEVEL_IRQS);
> +static DECLARE_BITMAP(wake_mux_enabled, SUN6I_NR_MUX_BITS);
> +static DECLARE_BITMAP(wake_mux_valid, SUN6I_NR_MUX_BITS);
>
> static void sun6i_r_intc_ack_nmi(void)
> {
> @@ -145,6 +156,21 @@ static int sun6i_r_intc_nmi_set_irqchip_state(struct
> irq_data *data,
> return irq_chip_set_parent_state(data, which, state);
> }
>
> +static int sun6i_r_intc_irq_set_wake(struct irq_data *data, unsigned int on)
> +{
> + unsigned long offset_from_nmi = data->hwirq - nmi_hwirq;
> +
> + if (offset_from_nmi < SUN6I_NR_DIRECT_IRQS)
> + assign_bit(offset_from_nmi, wake_irq_enabled, on);
> + else if (test_bit(data->hwirq, wake_mux_valid))
> + assign_bit(data->hwirq, wake_mux_enabled, on);
> + else
> + /* Not wakeup capable. */
> + return -EPERM;
> +
> + return 0;
> +}
> +
> static struct irq_chip sun6i_r_intc_nmi_chip = {
> .name = "sun6i-r-intc",
> .irq_ack= sun6i_r_intc_nmi_ack,
> @@ -154,8 +180,19 @@ static struct irq_chip sun6i_r_intc_nmi_chip = {
> .irq_set_affinity = irq_chip_set_affinity_parent,
> .irq_set_type = sun6i_r_intc_nmi_set_type,
> .irq_set_irqchip_state = sun6i_r_intc_nmi_set_irqchip_state,
> - .flags = IRQCHIP_SET_TYPE_MASKED |
> - IRQCHIP_SKIP_SET_WAKE,
> + .irq_set_wake = sun6i_r_intc_irq_set_wake,
> + .flags = IRQCHIP_SET_TYPE_MASKED,
> +};
> +
> +static struct irq_chip sun6i_r_intc_wakeup_chip = {
> + .name = "sun6i-r-intc",
> + .irq_mask = irq_chip_mask_parent,
> + .irq_unmask = irq_chip_unmask_parent,
> + .irq_eoi= irq_chip_eoi_parent,
> + .irq_set_affinity = irq_chip_set_affinity_parent,
> + .irq_set_type = irq_chip_set_type_parent,
> + .irq_set_wake = sun6i_r_intc_irq_set_wake,
> + .flags = IRQCHIP_SET_TYPE_MASKED,
Worth implementing irq_get/set_irqchip_state() using the _parent
helper, I guess.
Thanks,
M.
--
Without deviation from the norm, progress is not possible.
[PATCH v4 04/10] irqchip/sun6i-r: Add wakeup support
Maintain bitmaps of wake-enabled IRQs and mux inputs, and program them
to the hardware during the syscore phase of suspend and shutdown. Then
restore the original set of enabled IRQs (only the NMI) during resume.
This serves two purposes. First, it lets power management firmware
running on the ARISC coprocessor know which wakeup sources Linux wants
to have enabled. That way, it can avoid turning them off when it shuts
down the remainder of the clock tree. Second, it preconfigures the
coprocessor's interrupt controller, so the firmware's wakeup logic
is as simple as waiting for an interrupt to arrive.
The suspend/resume logic is not conditional on PM_SLEEP because it is
identical to the init/shutdown logic. Wake IRQs may be enabled during
shutdown to allow powering the board back on. As an example, see
commit a5c5e50cce9d ("Input: gpio-keys - add shutdown callback").
Signed-off-by: Samuel Holland
---
drivers/irqchip/irq-sun6i-r.c | 107 --
1 file changed, 101 insertions(+), 6 deletions(-)
diff --git a/drivers/irqchip/irq-sun6i-r.c b/drivers/irqchip/irq-sun6i-r.c
index d04d067423f4..a1b58c98d6ca 100644
--- a/drivers/irqchip/irq-sun6i-r.c
+++ b/drivers/irqchip/irq-sun6i-r.c
@@ -39,6 +39,7 @@
* set of 128 mux bits. This requires a second set of top-level registers.
*/
+#include
#include
#include
#include
@@ -46,6 +47,7 @@
#include
#include
#include
+#include
#include
@@ -67,8 +69,17 @@
#define SUN6I_NR_DIRECT_IRQS 16
#define SUN6I_NR_MUX_BITS 128
+struct sun6i_r_intc_variant {
+ u32 first_mux_irq;
+ u32 nr_mux_irqs;
+ u32 mux_valid[BITS_TO_U32(SUN6I_NR_MUX_BITS)];
+};
+
static void __iomem *base;
static irq_hw_number_t nmi_hwirq;
+static DECLARE_BITMAP(wake_irq_enabled, SUN6I_NR_TOP_LEVEL_IRQS);
+static DECLARE_BITMAP(wake_mux_enabled, SUN6I_NR_MUX_BITS);
+static DECLARE_BITMAP(wake_mux_valid, SUN6I_NR_MUX_BITS);
static void sun6i_r_intc_ack_nmi(void)
{
@@ -145,6 +156,21 @@ static int sun6i_r_intc_nmi_set_irqchip_state(struct
irq_data *data,
return irq_chip_set_parent_state(data, which, state);
}
+static int sun6i_r_intc_irq_set_wake(struct irq_data *data, unsigned int on)
+{
+ unsigned long offset_from_nmi = data->hwirq - nmi_hwirq;
+
+ if (offset_from_nmi < SUN6I_NR_DIRECT_IRQS)
+ assign_bit(offset_from_nmi, wake_irq_enabled, on);
+ else if (test_bit(data->hwirq, wake_mux_valid))
+ assign_bit(data->hwirq, wake_mux_enabled, on);
+ else
+ /* Not wakeup capable. */
+ return -EPERM;
+
+ return 0;
+}
+
static struct irq_chip sun6i_r_intc_nmi_chip = {
.name = "sun6i-r-intc",
.irq_ack= sun6i_r_intc_nmi_ack,
@@ -154,8 +180,19 @@ static struct irq_chip sun6i_r_intc_nmi_chip = {
.irq_set_affinity = irq_chip_set_affinity_parent,
.irq_set_type = sun6i_r_intc_nmi_set_type,
.irq_set_irqchip_state = sun6i_r_intc_nmi_set_irqchip_state,
- .flags = IRQCHIP_SET_TYPE_MASKED |
- IRQCHIP_SKIP_SET_WAKE,
+ .irq_set_wake = sun6i_r_intc_irq_set_wake,
+ .flags = IRQCHIP_SET_TYPE_MASKED,
+};
+
+static struct irq_chip sun6i_r_intc_wakeup_chip = {
+ .name = "sun6i-r-intc",
+ .irq_mask = irq_chip_mask_parent,
+ .irq_unmask = irq_chip_unmask_parent,
+ .irq_eoi= irq_chip_eoi_parent,
+ .irq_set_affinity = irq_chip_set_affinity_parent,
+ .irq_set_type = irq_chip_set_type_parent,
+ .irq_set_wake = sun6i_r_intc_irq_set_wake,
+ .flags = IRQCHIP_SET_TYPE_MASKED,
};
static int sun6i_r_intc_domain_translate(struct irq_domain *domain,
@@ -215,8 +252,8 @@ static int sun6i_r_intc_domain_alloc(struct irq_domain
*domain,
_r_intc_nmi_chip,
0);
irq_set_handler(virq, handle_fasteoi_ack_irq);
} else {
- /* Only the NMI is currently supported. */
- return -EINVAL;
+ irq_domain_set_hwirq_and_chip(domain, virq, hwirq,
+
_r_intc_wakeup_chip, 0);
}
}
@@ -229,6 +266,22 @@ static const struct irq_domain_ops sun6i_r_intc_domain_ops
= {
.free = irq_domain_free_irqs_common,
};
+static int sun6i_r_intc_suspend(void)
+{
+ u32 buf[BITS_TO_U32(max(SUN6I_NR_TOP_LEVEL_IRQS, SUN6I_NR_MUX_BITS))];
+ int i;
+
+ /* Wake IRQs are enabled during system sleep and shutdown. */
+ bitmap_to_arr32(buf, wake_irq_enabled, SUN6I_NR_TOP_LEVEL_IRQS);
+ for (i = 0; i < BITS_TO_U32(SUN6I_NR_TOP_LEVEL_IRQS); ++i)
+
