linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/4] Minor cleanup on gic(v3) and genirq
@ 2024-01-18 11:27 Dawei Li
  2024-01-18 11:27 ` [PATCH 1/4] irqchip/gic-v3: Implement read polling with dedicated API Dawei Li
                   ` (3 more replies)
  0 siblings, 4 replies; 10+ messages in thread
From: Dawei Li @ 2024-01-18 11:27 UTC (permalink / raw)
  To: tglx, maz
  Cc: sdonthineni, linux-arm-kernel, linux-kernel, dawei.li, set_pte_at

Hi Thomas, Marc:

This is a small cleanup series on gic(v3) and genirq, in which:
- #1-2 are minor cleanup on gic/gic-v3 codes;
- #3 is a trival cleanup on genirq header;
- #4 is a fix on genirq;

Dawei Li (4):
  irqchip/gic-v3: Implement read polling with dedicated API
  irqchip/gic: Implement generic gic_irq() API for GIC & GIC-v3
  genirq: Remove unneeded forward declaration
  genirq: Initialize resend_node hlist for all irq_desc

 drivers/irqchip/irq-gic-common.h |  5 +++++
 drivers/irqchip/irq-gic-v3.c     | 32 ++++++++------------------------
 drivers/irqchip/irq-gic.c        |  5 -----
 include/linux/irqhandler.h       |  2 +-
 kernel/irq/irqdesc.c             |  2 +-
 5 files changed, 15 insertions(+), 31 deletions(-)

base-commit: 69ffab9b9e698248cbb4042e47f82afb00dc1bb4

Thanks,
  Dawei

-- 
2.27.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 1/4] irqchip/gic-v3: Implement read polling with dedicated API
  2024-01-18 11:27 [PATCH 0/4] Minor cleanup on gic(v3) and genirq Dawei Li
@ 2024-01-18 11:27 ` Dawei Li
  2024-01-18 14:00   ` Marc Zyngier
  2024-01-18 11:27 ` [PATCH 2/4] irqchip/gic: Implement generic gic_irq() API for GIC & GIC-v3 Dawei Li
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 10+ messages in thread
From: Dawei Li @ 2024-01-18 11:27 UTC (permalink / raw)
  To: tglx, maz
  Cc: sdonthineni, linux-arm-kernel, linux-kernel, dawei.li, set_pte_at

Kernel provide read*_poll_* API family to support looping based polling
code pattern like below:
while (...)
{
	val = op(addr);
	condition = cond(val);
	if (condition)
		break;

	/* Maybe some timeout handling stuff */

	cpu_relax();
	udelay();
}

As such, use readl_relaxed_poll_timeout_atomic() to implement atomic
register polling logic in gic-v3.

Signed-off-by: Dawei Li <dawei.li@shingroup.cn>
---
 drivers/irqchip/irq-gic-v3.c | 27 ++++++++-------------------
 1 file changed, 8 insertions(+), 19 deletions(-)

diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c
index 98b0329b7154..b9d9375a3434 100644
--- a/drivers/irqchip/irq-gic-v3.c
+++ b/drivers/irqchip/irq-gic-v3.c
@@ -19,6 +19,7 @@
 #include <linux/percpu.h>
 #include <linux/refcount.h>
 #include <linux/slab.h>
+#include <linux/iopoll.h>
 
 #include <linux/irqchip.h>
 #include <linux/irqchip/arm-gic-common.h>
@@ -251,17 +252,11 @@ static inline void __iomem *gic_dist_base(struct irq_data *d)
 
 static void gic_do_wait_for_rwp(void __iomem *base, u32 bit)
 {
-	u32 count = 1000000;	/* 1s! */
+	u32 val;
 
-	while (readl_relaxed(base + GICD_CTLR) & bit) {
-		count--;
-		if (!count) {
-			pr_err_ratelimited("RWP timeout, gone fishing\n");
-			return;
-		}
-		cpu_relax();
-		udelay(1);
-	}
+	if (readl_relaxed_poll_timeout_atomic(base + GICD_CTLR,
+		val, !(val & bit), 1, 1000000) == -ETIMEDOUT)
+		pr_err_ratelimited("RWP timeout, gone fishing\n");
 }
 
 /* Wait for completion of a distributor change */
@@ -279,7 +274,6 @@ static void gic_redist_wait_for_rwp(void)
 static void gic_enable_redist(bool enable)
 {
 	void __iomem *rbase;
-	u32 count = 1000000;	/* 1s! */
 	u32 val;
 
 	if (gic_data.flags & FLAGS_WORKAROUND_GICR_WAKER_MSM8996)
@@ -301,14 +295,9 @@ static void gic_enable_redist(bool enable)
 			return;	/* No PM support in this redistributor */
 	}
 
-	while (--count) {
-		val = readl_relaxed(rbase + GICR_WAKER);
-		if (enable ^ (bool)(val & GICR_WAKER_ChildrenAsleep))
-			break;
-		cpu_relax();
-		udelay(1);
-	}
-	if (!count)
+	if (readl_relaxed_poll_timeout_atomic(rbase + GICR_WAKER,
+		val, enable ^ (bool)(val & GICR_WAKER_ChildrenAsleep),
+		1, 1000000) == -ETIMEDOUT)
 		pr_err_ratelimited("redistributor failed to %s...\n",
 				   enable ? "wakeup" : "sleep");
 }
-- 
2.27.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 2/4] irqchip/gic: Implement generic gic_irq() API for GIC & GIC-v3
  2024-01-18 11:27 [PATCH 0/4] Minor cleanup on gic(v3) and genirq Dawei Li
  2024-01-18 11:27 ` [PATCH 1/4] irqchip/gic-v3: Implement read polling with dedicated API Dawei Li
@ 2024-01-18 11:27 ` Dawei Li
  2024-01-18 14:03   ` Marc Zyngier
  2024-01-18 11:27 ` [PATCH 3/4] genirq: Remove unneeded forward declaration Dawei Li
  2024-01-18 11:27 ` [PATCH 4/4] genirq: Initialize resend_node hlist for all irq_desc Dawei Li
  3 siblings, 1 reply; 10+ messages in thread
From: Dawei Li @ 2024-01-18 11:27 UTC (permalink / raw)
  To: tglx, maz
  Cc: sdonthineni, linux-arm-kernel, linux-kernel, dawei.li, set_pte_at

GIC & GIC-v3 share same gic_irq() implementations, unify them into a
generic API.

Signed-off-by: Dawei Li <dawei.li@shingroup.cn>
---
 drivers/irqchip/irq-gic-common.h | 5 +++++
 drivers/irqchip/irq-gic-v3.c     | 5 -----
 drivers/irqchip/irq-gic.c        | 5 -----
 3 files changed, 5 insertions(+), 10 deletions(-)

diff --git a/drivers/irqchip/irq-gic-common.h b/drivers/irqchip/irq-gic-common.h
index f407cce9ecaa..ed18db4ab2c5 100644
--- a/drivers/irqchip/irq-gic-common.h
+++ b/drivers/irqchip/irq-gic-common.h
@@ -19,6 +19,11 @@ struct gic_quirk {
 	u32 mask;
 };
 
+static inline unsigned int gic_irq(struct irq_data *d)
+{
+	return d->hwirq;
+}
+
 int gic_configure_irq(unsigned int irq, unsigned int type,
                        void __iomem *base, void (*sync_access)(void));
 void gic_dist_config(void __iomem *base, int gic_irqs,
diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c
index b9d9375a3434..474a498a521e 100644
--- a/drivers/irqchip/irq-gic-v3.c
+++ b/drivers/irqchip/irq-gic-v3.c
@@ -181,11 +181,6 @@ static enum gic_intid_range get_intid_range(struct irq_data *d)
 	return __get_intid_range(d->hwirq);
 }
 
-static inline unsigned int gic_irq(struct irq_data *d)
-{
-	return d->hwirq;
-}
-
 static inline bool gic_irq_in_rdist(struct irq_data *d)
 {
 	switch (get_intid_range(d)) {
diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
index 412196a7dad5..0d559effa172 100644
--- a/drivers/irqchip/irq-gic.c
+++ b/drivers/irqchip/irq-gic.c
@@ -162,11 +162,6 @@ static inline void __iomem *gic_cpu_base(struct irq_data *d)
 	return gic_data_cpu_base(gic_data);
 }
 
-static inline unsigned int gic_irq(struct irq_data *d)
-{
-	return d->hwirq;
-}
-
 static inline bool cascading_gic_irq(struct irq_data *d)
 {
 	void *data = irq_data_get_irq_handler_data(d);
-- 
2.27.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 3/4] genirq: Remove unneeded forward declaration
  2024-01-18 11:27 [PATCH 0/4] Minor cleanup on gic(v3) and genirq Dawei Li
  2024-01-18 11:27 ` [PATCH 1/4] irqchip/gic-v3: Implement read polling with dedicated API Dawei Li
  2024-01-18 11:27 ` [PATCH 2/4] irqchip/gic: Implement generic gic_irq() API for GIC & GIC-v3 Dawei Li
@ 2024-01-18 11:27 ` Dawei Li
  2024-01-18 11:27 ` [PATCH 4/4] genirq: Initialize resend_node hlist for all irq_desc Dawei Li
  3 siblings, 0 replies; 10+ messages in thread
From: Dawei Li @ 2024-01-18 11:27 UTC (permalink / raw)
  To: tglx, maz
  Cc: sdonthineni, linux-arm-kernel, linux-kernel, dawei.li, set_pte_at

Protoype of irq_flow_handler_t is independent of irq_data, so
remove unneeded forward declaration.

Signed-off-by: Dawei Li <dawei.li@shingroup.cn>
---
 include/linux/irqhandler.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/irqhandler.h b/include/linux/irqhandler.h
index c30f454a9518..72dd1eb3a0e7 100644
--- a/include/linux/irqhandler.h
+++ b/include/linux/irqhandler.h
@@ -8,7 +8,7 @@
  */
 
 struct irq_desc;
-struct irq_data;
+
 typedef	void (*irq_flow_handler_t)(struct irq_desc *desc);
 
 #endif
-- 
2.27.0



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 4/4] genirq: Initialize resend_node hlist for all irq_desc
  2024-01-18 11:27 [PATCH 0/4] Minor cleanup on gic(v3) and genirq Dawei Li
                   ` (2 preceding siblings ...)
  2024-01-18 11:27 ` [PATCH 3/4] genirq: Remove unneeded forward declaration Dawei Li
@ 2024-01-18 11:27 ` Dawei Li
  2024-01-18 14:09   ` Marc Zyngier
  3 siblings, 1 reply; 10+ messages in thread
From: Dawei Li @ 2024-01-18 11:27 UTC (permalink / raw)
  To: tglx, maz
  Cc: sdonthineni, linux-arm-kernel, linux-kernel, dawei.li, set_pte_at,
	stable

For !CONFIG_SPARSE_IRQ kernel, early_irq_init() is supposed to
initialize all the desc entries in system, desc->resend_node
included.

Thus, initialize desc->resend_node for all irq_desc entries, rather
than irq_desc[0] only, which is the current implementation is about.

Fixes: bc06a9e08742 ("genirq: Use hlist for managing resend handlers")
Cc: stable@vger.kernel.org

Signed-off-by: Dawei Li <dawei.li@shingroup.cn>
---
 kernel/irq/irqdesc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/irq/irqdesc.c b/kernel/irq/irqdesc.c
index 27ca1c866f29..371eb1711d34 100644
--- a/kernel/irq/irqdesc.c
+++ b/kernel/irq/irqdesc.c
@@ -600,7 +600,7 @@ int __init early_irq_init(void)
 		mutex_init(&desc[i].request_mutex);
 		init_waitqueue_head(&desc[i].wait_for_threads);
 		desc_set_defaults(i, &desc[i], node, NULL, NULL);
-		irq_resend_init(desc);
+		irq_resend_init(&desc[i]);
 	}
 	return arch_early_irq_init();
 }
-- 
2.27.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/4] irqchip/gic-v3: Implement read polling with dedicated API
  2024-01-18 11:27 ` [PATCH 1/4] irqchip/gic-v3: Implement read polling with dedicated API Dawei Li
@ 2024-01-18 14:00   ` Marc Zyngier
  2024-01-19  1:49     ` Dawei Li
  0 siblings, 1 reply; 10+ messages in thread
From: Marc Zyngier @ 2024-01-18 14:00 UTC (permalink / raw)
  To: Dawei Li; +Cc: tglx, sdonthineni, linux-arm-kernel, linux-kernel, set_pte_at

On Thu, 18 Jan 2024 11:27:36 +0000,
Dawei Li <dawei.li@shingroup.cn> wrote:
> 
> Kernel provide read*_poll_* API family to support looping based polling
> code pattern like below:
> while (...)
> {
> 	val = op(addr);
> 	condition = cond(val);
> 	if (condition)
> 		break;
> 
> 	/* Maybe some timeout handling stuff */
> 
> 	cpu_relax();
> 	udelay();
> }
> 
> As such, use readl_relaxed_poll_timeout_atomic() to implement atomic
> register polling logic in gic-v3.
> 
> Signed-off-by: Dawei Li <dawei.li@shingroup.cn>
> ---
>  drivers/irqchip/irq-gic-v3.c | 27 ++++++++-------------------
>  1 file changed, 8 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c
> index 98b0329b7154..b9d9375a3434 100644
> --- a/drivers/irqchip/irq-gic-v3.c
> +++ b/drivers/irqchip/irq-gic-v3.c
> @@ -19,6 +19,7 @@
>  #include <linux/percpu.h>
>  #include <linux/refcount.h>
>  #include <linux/slab.h>
> +#include <linux/iopoll.h>
>  
>  #include <linux/irqchip.h>
>  #include <linux/irqchip/arm-gic-common.h>
> @@ -251,17 +252,11 @@ static inline void __iomem *gic_dist_base(struct irq_data *d)
>  
>  static void gic_do_wait_for_rwp(void __iomem *base, u32 bit)
>  {
> -	u32 count = 1000000;	/* 1s! */
> +	u32 val;
>  
> -	while (readl_relaxed(base + GICD_CTLR) & bit) {
> -		count--;
> -		if (!count) {
> -			pr_err_ratelimited("RWP timeout, gone fishing\n");
> -			return;
> -		}
> -		cpu_relax();
> -		udelay(1);
> -	}
> +	if (readl_relaxed_poll_timeout_atomic(base + GICD_CTLR,
> +		val, !(val & bit), 1, 1000000) == -ETIMEDOUT)

If you are doing this, please use a constant such as USEC_PER_SEC for
the timeout. And fix the alignment of the second line so that the
parameters are aligned vertically.

> +		pr_err_ratelimited("RWP timeout, gone fishing\n");
>  }
>  
>  /* Wait for completion of a distributor change */
> @@ -279,7 +274,6 @@ static void gic_redist_wait_for_rwp(void)
>  static void gic_enable_redist(bool enable)
>  {
>  	void __iomem *rbase;
> -	u32 count = 1000000;	/* 1s! */
>  	u32 val;
>  
>  	if (gic_data.flags & FLAGS_WORKAROUND_GICR_WAKER_MSM8996)
> @@ -301,14 +295,9 @@ static void gic_enable_redist(bool enable)
>  			return;	/* No PM support in this redistributor */
>  	}
>  
> -	while (--count) {
> -		val = readl_relaxed(rbase + GICR_WAKER);
> -		if (enable ^ (bool)(val & GICR_WAKER_ChildrenAsleep))
> -			break;
> -		cpu_relax();
> -		udelay(1);
> -	}
> -	if (!count)
> +	if (readl_relaxed_poll_timeout_atomic(rbase + GICR_WAKER,
> +		val, enable ^ (bool)(val & GICR_WAKER_ChildrenAsleep),
> +		1, 1000000) == -ETIMEDOUT)
>  		pr_err_ratelimited("redistributor failed to %s...\n",
>  				   enable ? "wakeup" : "sleep");
>  }

Same thing here.

	M.

-- 
Without deviation from the norm, progress is not possible.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/4] irqchip/gic: Implement generic gic_irq() API for GIC & GIC-v3
  2024-01-18 11:27 ` [PATCH 2/4] irqchip/gic: Implement generic gic_irq() API for GIC & GIC-v3 Dawei Li
@ 2024-01-18 14:03   ` Marc Zyngier
  2024-01-19  2:02     ` Dawei Li
  0 siblings, 1 reply; 10+ messages in thread
From: Marc Zyngier @ 2024-01-18 14:03 UTC (permalink / raw)
  To: Dawei Li; +Cc: tglx, sdonthineni, linux-arm-kernel, linux-kernel, set_pte_at

On Thu, 18 Jan 2024 11:27:37 +0000,
Dawei Li <dawei.li@shingroup.cn> wrote:
> 
> GIC & GIC-v3 share same gic_irq() implementations, unify them into a
> generic API.
> 
> Signed-off-by: Dawei Li <dawei.li@shingroup.cn>
> ---
>  drivers/irqchip/irq-gic-common.h | 5 +++++
>  drivers/irqchip/irq-gic-v3.c     | 5 -----
>  drivers/irqchip/irq-gic.c        | 5 -----
>  3 files changed, 5 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/irqchip/irq-gic-common.h b/drivers/irqchip/irq-gic-common.h
> index f407cce9ecaa..ed18db4ab2c5 100644
> --- a/drivers/irqchip/irq-gic-common.h
> +++ b/drivers/irqchip/irq-gic-common.h
> @@ -19,6 +19,11 @@ struct gic_quirk {
>  	u32 mask;
>  };
>  
> +static inline unsigned int gic_irq(struct irq_data *d)
> +{
> +	return d->hwirq;
> +}
> +
>  int gic_configure_irq(unsigned int irq, unsigned int type,
>                         void __iomem *base, void (*sync_access)(void));
>  void gic_dist_config(void __iomem *base, int gic_irqs,
> diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c
> index b9d9375a3434..474a498a521e 100644
> --- a/drivers/irqchip/irq-gic-v3.c
> +++ b/drivers/irqchip/irq-gic-v3.c
> @@ -181,11 +181,6 @@ static enum gic_intid_range get_intid_range(struct irq_data *d)
>  	return __get_intid_range(d->hwirq);
>  }
>  
> -static inline unsigned int gic_irq(struct irq_data *d)
> -{
> -	return d->hwirq;
> -}
> -

I'd rather not do that. If anything, I'd get rid of the helper
altogether, as we have irqd_to_hwirq() that does the same job, and
actually preserves the typing.

	M.

-- 
Without deviation from the norm, progress is not possible.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 4/4] genirq: Initialize resend_node hlist for all irq_desc
  2024-01-18 11:27 ` [PATCH 4/4] genirq: Initialize resend_node hlist for all irq_desc Dawei Li
@ 2024-01-18 14:09   ` Marc Zyngier
  0 siblings, 0 replies; 10+ messages in thread
From: Marc Zyngier @ 2024-01-18 14:09 UTC (permalink / raw)
  To: Dawei Li
  Cc: tglx, sdonthineni, linux-arm-kernel, linux-kernel, set_pte_at,
	stable

On Thu, 18 Jan 2024 11:27:39 +0000,
Dawei Li <dawei.li@shingroup.cn> wrote:
> 
> For !CONFIG_SPARSE_IRQ kernel, early_irq_init() is supposed to
> initialize all the desc entries in system, desc->resend_node
> included.
> 
> Thus, initialize desc->resend_node for all irq_desc entries, rather
> than irq_desc[0] only, which is the current implementation is about.
> 
> Fixes: bc06a9e08742 ("genirq: Use hlist for managing resend handlers")
> Cc: stable@vger.kernel.org
> 
> Signed-off-by: Dawei Li <dawei.li@shingroup.cn>
> ---
>  kernel/irq/irqdesc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/kernel/irq/irqdesc.c b/kernel/irq/irqdesc.c
> index 27ca1c866f29..371eb1711d34 100644
> --- a/kernel/irq/irqdesc.c
> +++ b/kernel/irq/irqdesc.c
> @@ -600,7 +600,7 @@ int __init early_irq_init(void)
>  		mutex_init(&desc[i].request_mutex);
>  		init_waitqueue_head(&desc[i].wait_for_threads);
>  		desc_set_defaults(i, &desc[i], node, NULL, NULL);
> -		irq_resend_init(desc);
> +		irq_resend_init(&desc[i]);
>  	}
>  	return arch_early_irq_init();
>  }

Well spotted. It would probably be worth having a helper that fully
initialises one desc, but that's out of the scope of this fix.

Acked-by: Marc Zyngier <maz@kernel.org>

	M.

-- 
Without deviation from the norm, progress is not possible.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/4] irqchip/gic-v3: Implement read polling with dedicated API
  2024-01-18 14:00   ` Marc Zyngier
@ 2024-01-19  1:49     ` Dawei Li
  0 siblings, 0 replies; 10+ messages in thread
From: Dawei Li @ 2024-01-19  1:49 UTC (permalink / raw)
  To: Marc Zyngier
  Cc: tglx, sdonthineni, linux-arm-kernel, linux-kernel, set_pte_at

Hi Marc,

Thanks for the quick review.

On Thu, Jan 18, 2024 at 02:00:15PM +0000, Marc Zyngier wrote:
> On Thu, 18 Jan 2024 11:27:36 +0000,
> Dawei Li <dawei.li@shingroup.cn> wrote:
> > 
> > Kernel provide read*_poll_* API family to support looping based polling
> > code pattern like below:
> > while (...)
> > {
> > 	val = op(addr);
> > 	condition = cond(val);
> > 	if (condition)
> > 		break;
> > 
> > 	/* Maybe some timeout handling stuff */
> > 
> > 	cpu_relax();
> > 	udelay();
> > }
> > 
> > As such, use readl_relaxed_poll_timeout_atomic() to implement atomic
> > register polling logic in gic-v3.
> > 
> > Signed-off-by: Dawei Li <dawei.li@shingroup.cn>
> > ---
> >  drivers/irqchip/irq-gic-v3.c | 27 ++++++++-------------------
> >  1 file changed, 8 insertions(+), 19 deletions(-)
> > 
> > diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c
> > index 98b0329b7154..b9d9375a3434 100644
> > --- a/drivers/irqchip/irq-gic-v3.c
> > +++ b/drivers/irqchip/irq-gic-v3.c
> > @@ -19,6 +19,7 @@
> >  #include <linux/percpu.h>
> >  #include <linux/refcount.h>
> >  #include <linux/slab.h>
> > +#include <linux/iopoll.h>
> >  
> >  #include <linux/irqchip.h>
> >  #include <linux/irqchip/arm-gic-common.h>
> > @@ -251,17 +252,11 @@ static inline void __iomem *gic_dist_base(struct irq_data *d)
> >  
> >  static void gic_do_wait_for_rwp(void __iomem *base, u32 bit)
> >  {
> > -	u32 count = 1000000;	/* 1s! */
> > +	u32 val;
> >  
> > -	while (readl_relaxed(base + GICD_CTLR) & bit) {
> > -		count--;
> > -		if (!count) {
> > -			pr_err_ratelimited("RWP timeout, gone fishing\n");
> > -			return;
> > -		}
> > -		cpu_relax();
> > -		udelay(1);
> > -	}
> > +	if (readl_relaxed_poll_timeout_atomic(base + GICD_CTLR,
> > +		val, !(val & bit), 1, 1000000) == -ETIMEDOUT)
> 
> If you are doing this, please use a constant such as USEC_PER_SEC for
> the timeout. And fix the alignment of the second line so that the
> parameters are aligned vertically.

Agreed, well defined constant is always preferable than magic number;

And yes, alignment is for better readability.

> 
> > +		pr_err_ratelimited("RWP timeout, gone fishing\n");
> >  }
> >  
> >  /* Wait for completion of a distributor change */
> > @@ -279,7 +274,6 @@ static void gic_redist_wait_for_rwp(void)
> >  static void gic_enable_redist(bool enable)
> >  {
> >  	void __iomem *rbase;
> > -	u32 count = 1000000;	/* 1s! */
> >  	u32 val;
> >  
> >  	if (gic_data.flags & FLAGS_WORKAROUND_GICR_WAKER_MSM8996)
> > @@ -301,14 +295,9 @@ static void gic_enable_redist(bool enable)
> >  			return;	/* No PM support in this redistributor */
> >  	}
> >  
> > -	while (--count) {
> > -		val = readl_relaxed(rbase + GICR_WAKER);
> > -		if (enable ^ (bool)(val & GICR_WAKER_ChildrenAsleep))
> > -			break;
> > -		cpu_relax();
> > -		udelay(1);
> > -	}
> > -	if (!count)
> > +	if (readl_relaxed_poll_timeout_atomic(rbase + GICR_WAKER,
> > +		val, enable ^ (bool)(val & GICR_WAKER_ChildrenAsleep),
> > +		1, 1000000) == -ETIMEDOUT)
> >  		pr_err_ratelimited("redistributor failed to %s...\n",
> >  				   enable ? "wakeup" : "sleep");
> >  }
> 
> Same thing here.

Ack.

I will address two issues above and send respin of V2.

Thanks,
	Dawei

> 
> 	M.
> 
> -- 
> Without deviation from the norm, progress is not possible.
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/4] irqchip/gic: Implement generic gic_irq() API for GIC & GIC-v3
  2024-01-18 14:03   ` Marc Zyngier
@ 2024-01-19  2:02     ` Dawei Li
  0 siblings, 0 replies; 10+ messages in thread
From: Dawei Li @ 2024-01-19  2:02 UTC (permalink / raw)
  To: Marc Zyngier
  Cc: tglx, sdonthineni, linux-arm-kernel, linux-kernel, set_pte_at

Hi Marc,
Thanks for the review.

On Thu, Jan 18, 2024 at 02:03:08PM +0000, Marc Zyngier wrote:
> On Thu, 18 Jan 2024 11:27:37 +0000,
> Dawei Li <dawei.li@shingroup.cn> wrote:
> > 
> > GIC & GIC-v3 share same gic_irq() implementations, unify them into a
> > generic API.
> > 
> > Signed-off-by: Dawei Li <dawei.li@shingroup.cn>
> > ---
> >  drivers/irqchip/irq-gic-common.h | 5 +++++
> >  drivers/irqchip/irq-gic-v3.c     | 5 -----
> >  drivers/irqchip/irq-gic.c        | 5 -----
> >  3 files changed, 5 insertions(+), 10 deletions(-)
> > 
> > diff --git a/drivers/irqchip/irq-gic-common.h b/drivers/irqchip/irq-gic-common.h
> > index f407cce9ecaa..ed18db4ab2c5 100644
> > --- a/drivers/irqchip/irq-gic-common.h
> > +++ b/drivers/irqchip/irq-gic-common.h
> > @@ -19,6 +19,11 @@ struct gic_quirk {
> >  	u32 mask;
> >  };
> >  
> > +static inline unsigned int gic_irq(struct irq_data *d)
> > +{
> > +	return d->hwirq;
> > +}
> > +
> >  int gic_configure_irq(unsigned int irq, unsigned int type,
> >                         void __iomem *base, void (*sync_access)(void));
> >  void gic_dist_config(void __iomem *base, int gic_irqs,
> > diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c
> > index b9d9375a3434..474a498a521e 100644
> > --- a/drivers/irqchip/irq-gic-v3.c
> > +++ b/drivers/irqchip/irq-gic-v3.c
> > @@ -181,11 +181,6 @@ static enum gic_intid_range get_intid_range(struct irq_data *d)
> >  	return __get_intid_range(d->hwirq);
> >  }
> >  
> > -static inline unsigned int gic_irq(struct irq_data *d)
> > -{
> > -	return d->hwirq;
> > -}
> > -
> 
> I'd rather not do that. If anything, I'd get rid of the helper
> altogether, as we have irqd_to_hwirq() that does the same job, and
> actually preserves the typing.

Yes, your solution is much better for it's truly generic, independent of
irq chip.

I will send respin of V2 as you suggested, and if I may, add your
suggested-by?

Thanks,
	Dawei

> 
> 	M.
> 
> -- 
> Without deviation from the norm, progress is not possible.
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2024-01-19  2:03 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-01-18 11:27 [PATCH 0/4] Minor cleanup on gic(v3) and genirq Dawei Li
2024-01-18 11:27 ` [PATCH 1/4] irqchip/gic-v3: Implement read polling with dedicated API Dawei Li
2024-01-18 14:00   ` Marc Zyngier
2024-01-19  1:49     ` Dawei Li
2024-01-18 11:27 ` [PATCH 2/4] irqchip/gic: Implement generic gic_irq() API for GIC & GIC-v3 Dawei Li
2024-01-18 14:03   ` Marc Zyngier
2024-01-19  2:02     ` Dawei Li
2024-01-18 11:27 ` [PATCH 3/4] genirq: Remove unneeded forward declaration Dawei Li
2024-01-18 11:27 ` [PATCH 4/4] genirq: Initialize resend_node hlist for all irq_desc Dawei Li
2024-01-18 14:09   ` Marc Zyngier

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).