[PATCH 0/3] irqchip: dw-apb-ictl: IRQ_GC_MASK_CACHE_PER_TYPE and PM support

These patches try to improve dw-apb-ictl irqchip driver a bit.

The first patch does a bit clean up work -- unify the register access usage.

The two dw-apb-ictl's irq_chip_type instances have separate mask registers, so the second patch enables IRQ_GC_MASK_CACHE_PER_TYPE.

The last patch adds suspend/resume support to the driver.

Tested on Marvell BG2Q-DMP board.

Jisheng Zhang (3):
  irqchip: dw-apb-ictl: always use use {readl|writel}_relaxed
  irqchip: dw-apb-ictl: enable IRQ_GC_MASK_CACHE_PER_TYPE
  irqchip: dw-apb-ictl: add PM support

 drivers/irqchip/irq-dw-apb-ictl.c | 32 ++++++++++++++++++++++++++------
 1 file changed, 26 insertions(+), 6 deletions(-)

-- 
2.1.0

This patch adds in support for S2R for dw-apb-ictl irqchip driver.

Signed-off-by: Jisheng Zhang <redacted>
---
 drivers/irqchip/irq-dw-apb-ictl.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/drivers/irqchip/irq-dw-apb-ictl.c b/drivers/irqchip/irq-dw-apb-ictl.c
index c136b67..53bb732 100644
--- a/drivers/irqchip/irq-dw-apb-ictl.c
+++ b/drivers/irqchip/irq-dw-apb-ictl.c

@@ -50,6 +50,21 @@ static void dw_apb_ictl_handler(unsigned int irq, struct irq_desc *desc)
 	chained_irq_exit(chip, desc);
 }
 
+#ifdef CONFIG_PM
+static void dw_apb_ictl_resume(struct irq_data *d)
+{
+	struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
+	struct irq_chip_type *ct = irq_data_get_chip_type(d);
+
+	irq_gc_lock(gc);
+	writel_relaxed(~0, gc->reg_base + ct->regs.enable);
+	writel_relaxed(*ct->mask_cache, gc->reg_base + ct->regs.mask);
+	irq_gc_unlock(gc);
+}
+#else
+#define dw_apb_ictl_resume	NULL
+#endif /* CONFIG_PM */
+
 static int __init dw_apb_ictl_init(struct device_node *np,
 				   struct device_node *parent)
 {

@@ -127,13 +142,17 @@ static int __init dw_apb_ictl_init(struct device_node *np,
 	gc->reg_base = iobase;
 
 	gc->chip_types[0].regs.mask = APB_INT_MASK_L;
+	gc->chip_types[0].regs.enable = APB_INT_ENABLE_L;
 	gc->chip_types[0].chip.irq_mask = irq_gc_mask_set_bit;
 	gc->chip_types[0].chip.irq_unmask = irq_gc_mask_clr_bit;
+	gc->chip_types[0].chip.irq_resume = dw_apb_ictl_resume;
 
 	if (nrirqs > 32) {
 		gc->chip_types[1].regs.mask = APB_INT_MASK_H;
+		gc->chip_types[1].regs.enable = APB_INT_ENABLE_H;
 		gc->chip_types[1].chip.irq_mask = irq_gc_mask_set_bit;
 		gc->chip_types[1].chip.irq_unmask = irq_gc_mask_clr_bit;
+		gc->chip_types[1].chip.irq_resume = dw_apb_ictl_resume;
 	}
 
 	irq_set_handler_data(irq, gc);

-- 
2.1.0

relaxed version and non-relaxed version are mixed, this patch always
use the relaxed version to unify the memory access usage.

Signed-off-by: Jisheng Zhang <redacted>
---
 drivers/irqchip/irq-dw-apb-ictl.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/irqchip/irq-dw-apb-ictl.c b/drivers/irqchip/irq-dw-apb-ictl.c
index 31e231e..fcc3385 100644
--- a/drivers/irqchip/irq-dw-apb-ictl.c
+++ b/drivers/irqchip/irq-dw-apb-ictl.c

@@ -94,16 +94,16 @@ static int __init dw_apb_ictl_init(struct device_node *np,
 	 */
 
 	/* mask and enable all interrupts */
-	writel(~0, iobase + APB_INT_MASK_L);
-	writel(~0, iobase + APB_INT_MASK_H);
-	writel(~0, iobase + APB_INT_ENABLE_L);
-	writel(~0, iobase + APB_INT_ENABLE_H);
+	writel_relaxed(~0, iobase + APB_INT_MASK_L);
+	writel_relaxed(~0, iobase + APB_INT_MASK_H);
+	writel_relaxed(~0, iobase + APB_INT_ENABLE_L);
+	writel_relaxed(~0, iobase + APB_INT_ENABLE_H);
 
-	reg = readl(iobase + APB_INT_ENABLE_H);
+	reg = readl_relaxed(iobase + APB_INT_ENABLE_H);
 	if (reg)
 		nrirqs = 32 + fls(reg);
 	else
-		nrirqs = fls(readl(iobase + APB_INT_ENABLE_L));
+		nrirqs = fls(readl_relaxed(iobase + APB_INT_ENABLE_L));
 
 	domain = irq_domain_add_linear(np, nrirqs,
 				       &irq_generic_chip_ops, NULL);

-- 
2.1.0

The irq_chip_type instances have separate mask registers, so we need to
enable IRQ_GC_MASK_CACHE_PER_TYPE to actually handle separate mask registers.

Signed-off-by: Jisheng Zhang <redacted>
---
 drivers/irqchip/irq-dw-apb-ictl.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/irqchip/irq-dw-apb-ictl.c b/drivers/irqchip/irq-dw-apb-ictl.c
index fcc3385..c136b67 100644
--- a/drivers/irqchip/irq-dw-apb-ictl.c
+++ b/drivers/irqchip/irq-dw-apb-ictl.c

@@ -115,6 +115,7 @@ static int __init dw_apb_ictl_init(struct device_node *np,
 
 	ret = irq_alloc_domain_generic_chips(domain, 32, (nrirqs > 32) ? 2 : 1,
 					     np->name, handle_level_irq, clr, 0,
+					     IRQ_GC_MASK_CACHE_PER_TYPE |
 					     IRQ_GC_INIT_MASK_CACHE);
 	if (ret) {
 		pr_err("%s: unable to alloc irq domain gc\n", np->full_name);

-- 
2.1.0

On 09/23/2014 08:34 AM, Jisheng Zhang wrote:

relaxed version and non-relaxed version are mixed, this patch always
use the relaxed version to unify the memory access usage.

Signed-off-by: Jisheng Zhang <redacted>

While I don't agree with the above reason (unification) for this
change, I agree with the patch because it _can_ be changed, so

Acked-by: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>

On 09/23/2014 08:34 AM, Jisheng Zhang wrote:

The irq_chip_type instances have separate mask registers, so we need to
enable IRQ_GC_MASK_CACHE_PER_TYPE to actually handle separate mask registers.

Signed-off-by: Jisheng Zhang <redacted>

Acked-by: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>

On 09/23/2014 08:35 AM, Jisheng Zhang wrote:

I agree with the overall change, but may this also be suited for a
generic irq_chip helper instead of being a driver specific one?

Maybe Thomas or Jason can comment on this.

Also, now that you are using writel_relaxed, I understand that both
writes above can happen in any order? Are there any implication we
have to consider, i.e. do we require any of the registers above to
be written first?

Sebastian

On Tue, 30 Sep 2014, Sebastian Hesselbarth wrote:

On 09/23/2014 08:34 AM, Jisheng Zhang wrote:

quoted

relaxed version and non-relaxed version are mixed, this patch always
use the relaxed version to unify the memory access usage.

Signed-off-by: Jisheng Zhang <redacted>

While I don't agree with the above reason (unification) for this

Then we want a proper changelog for it, really.

Thanks,

	tglx

On Tue, 30 Sep 2014, Sebastian Hesselbarth wrote:

On 09/23/2014 08:35 AM, Jisheng Zhang wrote:

quoted

This patch adds in support for S2R for dw-apb-ictl irqchip driver.

Signed-off-by: Jisheng Zhang <redacted>
---
  drivers/irqchip/irq-dw-apb-ictl.c | 19 +++++++++++++++++++
  1 file changed, 19 insertions(+)

diff --git a/drivers/irqchip/irq-dw-apb-ictl.c

b/drivers/irqchip/irq-dw-apb-ictl.c
index c136b67..53bb732 100644

--- a/drivers/irqchip/irq-dw-apb-ictl.c
+++ b/drivers/irqchip/irq-dw-apb-ictl.c

@@ -50,6 +50,21 @@ static void dw_apb_ictl_handler(unsigned int irq,

struct irq_desc *desc)
  	chained_irq_exit(chip, desc);
  }

+#ifdef CONFIG_PM
+static void dw_apb_ictl_resume(struct irq_data *d)
+{
+	struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
+	struct irq_chip_type *ct = irq_data_get_chip_type(d);
+
+	irq_gc_lock(gc);
+	writel_relaxed(~0, gc->reg_base + ct->regs.enable);
+	writel_relaxed(*ct->mask_cache, gc->reg_base + ct->regs.mask);
+	irq_gc_unlock(gc);
+}

I agree with the overall change, but may this also be suited for a
generic irq_chip helper instead of being a driver specific one?

Maybe Thomas or Jason can comment on this.

If we have enough similar resume callbacks, yes. 
 

Also, now that you are using writel_relaxed, I understand that both
writes above can happen in any order? Are there any implication we
have to consider, i.e. do we require any of the registers above to
be written first?

Was about to ask that as well :)

Thanks,

	tglx

Hi Thomas, Sebastian,

On Tue, 30 Sep 2014 14:52:54 -0700
Thomas Gleixner [off-list ref] wrote:

On Tue, 30 Sep 2014, Sebastian Hesselbarth wrote:

quoted

On 09/23/2014 08:35 AM, Jisheng Zhang wrote:

quoted

This patch adds in support for S2R for dw-apb-ictl irqchip driver.

Signed-off-by: Jisheng Zhang <redacted>
---
  drivers/irqchip/irq-dw-apb-ictl.c | 19 +++++++++++++++++++
  1 file changed, 19 insertions(+)

diff --git a/drivers/irqchip/irq-dw-apb-ictl.c

b/drivers/irqchip/irq-dw-apb-ictl.c
index c136b67..53bb732 100644

--- a/drivers/irqchip/irq-dw-apb-ictl.c
+++ b/drivers/irqchip/irq-dw-apb-ictl.c

@@ -50,6 +50,21 @@ static void dw_apb_ictl_handler(unsigned int irq,

struct irq_desc *desc)
  	chained_irq_exit(chip, desc);
  }

+#ifdef CONFIG_PM
+static void dw_apb_ictl_resume(struct irq_data *d)
+{
+	struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
+	struct irq_chip_type *ct = irq_data_get_chip_type(d);
+
+	irq_gc_lock(gc);
+	writel_relaxed(~0, gc->reg_base + ct->regs.enable);
+	writel_relaxed(*ct->mask_cache, gc->reg_base + ct->regs.mask);
+	irq_gc_unlock(gc);
+}

I agree with the overall change, but may this also be suited for a
generic irq_chip helper instead of being a driver specific one?

Maybe Thomas or Jason can comment on this.

If we have enough similar resume callbacks, yes. 
 

quoted

Also, now that you are using writel_relaxed, I understand that both
writes above can happen in any order? Are there any implication we
have to consider, i.e. do we require any of the registers above to
be written first?

The registers sits at device type memory, the writes should happen in the same
order as before.

Thanks for reviewing,
Jisheng

Was about to ask that as well :)

Thanks,

	tglx

On 10/08/2014 01:31 PM, Jisheng Zhang wrote:

Hi Thomas, Sebastian,

On Tue, 30 Sep 2014 14:52:54 -0700
Thomas Gleixner [off-list ref] wrote:

quoted

On Tue, 30 Sep 2014, Sebastian Hesselbarth wrote:

quoted

On 09/23/2014 08:35 AM, Jisheng Zhang wrote:

quoted

This patch adds in support for S2R for dw-apb-ictl irqchip driver.

Signed-off-by: Jisheng Zhang <redacted>
---
   drivers/irqchip/irq-dw-apb-ictl.c | 19 +++++++++++++++++++
   1 file changed, 19 insertions(+)

diff --git a/drivers/irqchip/irq-dw-apb-ictl.c

b/drivers/irqchip/irq-dw-apb-ictl.c
index c136b67..53bb732 100644

--- a/drivers/irqchip/irq-dw-apb-ictl.c
+++ b/drivers/irqchip/irq-dw-apb-ictl.c

@@ -50,6 +50,21 @@ static void dw_apb_ictl_handler(unsigned int irq,

struct irq_desc *desc)
   	chained_irq_exit(chip, desc);
   }

+#ifdef CONFIG_PM
+static void dw_apb_ictl_resume(struct irq_data *d)
+{
+	struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
+	struct irq_chip_type *ct = irq_data_get_chip_type(d);
+
+	irq_gc_lock(gc);
+	writel_relaxed(~0, gc->reg_base + ct->regs.enable);
+	writel_relaxed(*ct->mask_cache, gc->reg_base + ct->regs.mask);
+	irq_gc_unlock(gc);
+}

I agree with the overall change, but may this also be suited for a
generic irq_chip helper instead of being a driver specific one?

Maybe Thomas or Jason can comment on this.

If we have enough similar resume callbacks, yes.

quoted

Also, now that you are using writel_relaxed, I understand that both
writes above can happen in any order? Are there any implication we
have to consider, i.e. do we require any of the registers above to
be written first?

The registers sits at device type memory, the writes should happen in the same
order as before.

Jisheng,

it is not about the location of the register but, as far as I
understand, when using {readl,writel}_relaxed the compiler is
free to reorder the calls. So, if there is a strict order we
want to ensure, we have to use non-relaxed {readl,writel}.

The performance penalty of non-relaxed calls can be ignored anyway
as it is done only once after resume.

Sebastian

Hi Sebastian,

On Wed, 8 Oct 2014 04:44:49 -0700
Sebastian Hesselbarth [off-list ref] wrote:

On 10/08/2014 01:31 PM, Jisheng Zhang wrote:

quoted

Hi Thomas, Sebastian,

On Tue, 30 Sep 2014 14:52:54 -0700
Thomas Gleixner [off-list ref] wrote:

quoted

On Tue, 30 Sep 2014, Sebastian Hesselbarth wrote:

quoted

On 09/23/2014 08:35 AM, Jisheng Zhang wrote:

quoted

This patch adds in support for S2R for dw-apb-ictl irqchip driver.

Signed-off-by: Jisheng Zhang <redacted>
---
   drivers/irqchip/irq-dw-apb-ictl.c | 19 +++++++++++++++++++
   1 file changed, 19 insertions(+)

diff --git a/drivers/irqchip/irq-dw-apb-ictl.c

b/drivers/irqchip/irq-dw-apb-ictl.c
index c136b67..53bb732 100644

--- a/drivers/irqchip/irq-dw-apb-ictl.c
+++ b/drivers/irqchip/irq-dw-apb-ictl.c

@@ -50,6 +50,21 @@ static void dw_apb_ictl_handler(unsigned int irq,

struct irq_desc *desc)
   	chained_irq_exit(chip, desc);
   }

+#ifdef CONFIG_PM
+static void dw_apb_ictl_resume(struct irq_data *d)
+{
+	struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
+	struct irq_chip_type *ct = irq_data_get_chip_type(d);
+
+	irq_gc_lock(gc);
+	writel_relaxed(~0, gc->reg_base + ct->regs.enable);
+	writel_relaxed(*ct->mask_cache, gc->reg_base + ct->regs.mask);
+	irq_gc_unlock(gc);
+}

I agree with the overall change, but may this also be suited for a
generic irq_chip helper instead of being a driver specific one?

Maybe Thomas or Jason can comment on this.

If we have enough similar resume callbacks, yes.

quoted

Also, now that you are using writel_relaxed, I understand that both
writes above can happen in any order? Are there any implication we
have to consider, i.e. do we require any of the registers above to
be written first?

The registers sits at device type memory, the writes should happen in the
same order as before.

Jisheng,

it is not about the location of the register but, as far as I
understand, when using {readl,writel}_relaxed the compiler is
free to reorder the calls. So, if there is a strict order we

The "volatile" in readl/writel relaxed implementations should prevent the
compiler to do reorder. Or I misunderstand something?

Thanks,
Jisheng

want to ensure, we have to use non-relaxed {readl,writel}.

The performance penalty of non-relaxed calls can be ignored anyway
as it is done only once after resume.

Sebastian

Hi Sebastian,

On Wed, 8 Oct 2014 04:50:53 -0700
Jisheng Zhang [off-list ref] wrote:

Hi Sebastian,

On Wed, 8 Oct 2014 04:44:49 -0700
Sebastian Hesselbarth [off-list ref] wrote:

quoted

On 10/08/2014 01:31 PM, Jisheng Zhang wrote:

quoted

Hi Thomas, Sebastian,

On Tue, 30 Sep 2014 14:52:54 -0700
Thomas Gleixner [off-list ref] wrote:

quoted

On Tue, 30 Sep 2014, Sebastian Hesselbarth wrote:

quoted

On 09/23/2014 08:35 AM, Jisheng Zhang wrote:

quoted

This patch adds in support for S2R for dw-apb-ictl irqchip driver.

Signed-off-by: Jisheng Zhang <redacted>
---
   drivers/irqchip/irq-dw-apb-ictl.c | 19 +++++++++++++++++++
   1 file changed, 19 insertions(+)

diff --git a/drivers/irqchip/irq-dw-apb-ictl.c

b/drivers/irqchip/irq-dw-apb-ictl.c
index c136b67..53bb732 100644

--- a/drivers/irqchip/irq-dw-apb-ictl.c
+++ b/drivers/irqchip/irq-dw-apb-ictl.c

@@ -50,6 +50,21 @@ static void dw_apb_ictl_handler(unsigned int irq,

struct irq_desc *desc)
   	chained_irq_exit(chip, desc);
   }

+#ifdef CONFIG_PM
+static void dw_apb_ictl_resume(struct irq_data *d)
+{
+	struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
+	struct irq_chip_type *ct = irq_data_get_chip_type(d);
+
+	irq_gc_lock(gc);
+	writel_relaxed(~0, gc->reg_base + ct->regs.enable);
+	writel_relaxed(*ct->mask_cache, gc->reg_base +
ct->regs.mask);
+	irq_gc_unlock(gc);
+}

I agree with the overall change, but may this also be suited for a
generic irq_chip helper instead of being a driver specific one?

Maybe Thomas or Jason can comment on this.

If we have enough similar resume callbacks, yes.

quoted

Also, now that you are using writel_relaxed, I understand that both
writes above can happen in any order? Are there any implication we
have to consider, i.e. do we require any of the registers above to
be written first?

The registers sits at device type memory, the writes should happen in
the same order as before.

Jisheng,

it is not about the location of the register but, as far as I
understand, when using {readl,writel}_relaxed the compiler is
free to reorder the calls. So, if there is a strict order we

The "volatile" in readl/writel relaxed implementations should prevent the
compiler to do reorder. Or I misunderstand something?

My understanding is that the relaxed version imply compiler barriers.
I'm not sure I understand the real/writel relaxed implementations correctly. But
one obvious example which shows the relaxed version won't have the compiler
reorder issue is drivers/irqchip/irq-gic.c, all the configurations must be done
before enable the GIC which is done by "writel_relaxed(1, cpu_base + GIC_CPU_CTRL);"
However, we didn't see any explicit compiler barriers.

Thanks,
Jisheng

Thanks,
Jisheng

quoted

want to ensure, we have to use non-relaxed {readl,writel}.

The performance penalty of non-relaxed calls can be ignored anyway
as it is done only once after resume.

Sebastian

On 10/08/2014 01:58 PM, Jisheng Zhang wrote:

Hi Sebastian,

On Wed, 8 Oct 2014 04:50:53 -0700
Jisheng Zhang [off-list ref] wrote:

quoted

Hi Sebastian,

On Wed, 8 Oct 2014 04:44:49 -0700
Sebastian Hesselbarth [off-list ref] wrote:

quoted

On 10/08/2014 01:31 PM, Jisheng Zhang wrote:

quoted

Hi Thomas, Sebastian,

On Tue, 30 Sep 2014 14:52:54 -0700
Thomas Gleixner [off-list ref] wrote:

quoted

On Tue, 30 Sep 2014, Sebastian Hesselbarth wrote:

quoted

On 09/23/2014 08:35 AM, Jisheng Zhang wrote:

quoted

This patch adds in support for S2R for dw-apb-ictl irqchip driver.

Signed-off-by: Jisheng Zhang <redacted>
---
    drivers/irqchip/irq-dw-apb-ictl.c | 19 +++++++++++++++++++
    1 file changed, 19 insertions(+)

diff --git a/drivers/irqchip/irq-dw-apb-ictl.c

b/drivers/irqchip/irq-dw-apb-ictl.c
index c136b67..53bb732 100644

--- a/drivers/irqchip/irq-dw-apb-ictl.c
+++ b/drivers/irqchip/irq-dw-apb-ictl.c

@@ -50,6 +50,21 @@ static void dw_apb_ictl_handler(unsigned int irq,

struct irq_desc *desc)
    	chained_irq_exit(chip, desc);
    }

+#ifdef CONFIG_PM
+static void dw_apb_ictl_resume(struct irq_data *d)
+{
+	struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
+	struct irq_chip_type *ct = irq_data_get_chip_type(d);
+
+	irq_gc_lock(gc);
+	writel_relaxed(~0, gc->reg_base + ct->regs.enable);
+	writel_relaxed(*ct->mask_cache, gc->reg_base +
ct->regs.mask);
+	irq_gc_unlock(gc);
+}

I agree with the overall change, but may this also be suited for a
generic irq_chip helper instead of being a driver specific one?

Maybe Thomas or Jason can comment on this.

If we have enough similar resume callbacks, yes.

quoted

Also, now that you are using writel_relaxed, I understand that both
writes above can happen in any order? Are there any implication we
have to consider, i.e. do we require any of the registers above to
be written first?

The registers sits at device type memory, the writes should happen in
the same order as before.

it is not about the location of the register but, as far as I
understand, when using {readl,writel}_relaxed the compiler is
free to reorder the calls. So, if there is a strict order we

The "volatile" in readl/writel relaxed implementations should prevent the
compiler to do reorder. Or I misunderstand something?

My understanding is that the relaxed version imply compiler barriers.
I'm not sure I understand the real/writel relaxed implementations correctly. But
one obvious example which shows the relaxed version won't have the compiler
reorder issue is drivers/irqchip/irq-gic.c, all the configurations must be done
before enable the GIC which is done by "writel_relaxed(1, cpu_base + GIC_CPU_CTRL);"
However, we didn't see any explicit compiler barriers.

Yup, I just checked the discussion here:
http://comments.gmane.org/gmane.linux.ports.arm.kernel/117626

You are right, write order is ensured.

Sebastian

Dear Thomas, Jason,

Is there any potential issue with this patch serials I need to resolve?

Thanks in advance,
Jisheng

On Mon, 22 Sep 2014 23:34:57 -0700
Jisheng Zhang [off-list ref] wrote:

These patches try to improve dw-apb-ictl irqchip driver a bit.

The first patch does a bit clean up work -- unify the register access usage.

The two dw-apb-ictl's irq_chip_type instances have separate mask registers,
so the second patch enables IRQ_GC_MASK_CACHE_PER_TYPE.

The last patch adds suspend/resume support to the driver.

Tested on Marvell BG2Q-DMP board.

Jisheng Zhang (3):
  irqchip: dw-apb-ictl: always use use {readl|writel}_relaxed
  irqchip: dw-apb-ictl: enable IRQ_GC_MASK_CACHE_PER_TYPE
  irqchip: dw-apb-ictl: add PM support

 drivers/irqchip/irq-dw-apb-ictl.c | 32 ++++++++++++++++++++++++++------
 1 file changed, 26 insertions(+), 6 deletions(-)

On Tue, Nov 11, 2014 at 01:59:47PM +0800, Jisheng Zhang wrote:

Dear Thomas, Jason,

Is there any potential issue with this patch serials I need to resolve?

Please send a v2 with Sebastian's Acks.  Also, please update the commit
log on #1 as requested.  For #3, Please add a summary of the discussion
to its commit log.  eg: "We can used relaxed variants here because..."

thx,

Jason.