Hi Marc

Thanks for your review. the response inlined.

On Thu, 2016-09-08 at 13:37 +0100, Marc Zyngier wrote:

On 08/09/16 11:49, Mars Cheng wrote:

quoted

This adds DT binding documentation for Mediatek MT6797.

Signed-off-by: Mars Cheng <redacted>
---

[...]

quoted

diff --git a/Documentation/devicetree/bindings/interrupt-controller/mediatek,sysirq.txt b/Documentation/devicetree/bindings/interrupt-controller/mediatek,sysirq.txt
index 9d1d72c..3d97eb4 100644
--- a/Documentation/devicetree/bindings/interrupt-controller/mediatek,sysirq.txt
+++ b/Documentation/devicetree/bindings/interrupt-controller/mediatek,sysirq.txt

@@ -8,6 +8,7 @@ Required properties:
 	"mediatek,mt8173-sysirq"
 	"mediatek,mt8135-sysirq"
 	"mediatek,mt8127-sysirq"
+	"mediatek,mt6797-sysirq"
 	"mediatek,mt6795-sysirq"
 	"mediatek,mt6755-sysirq"
 	"mediatek,mt6592-sysirq"

@@ -21,7 +22,8 @@ Required properties:
 - interrupt-parent: phandle of irq parent for sysirq. The parent must
   use the same interrupt-cells format as GIC.
 - reg: Physical base address of the intpol registers and length of memory
-  mapped region.
+  mapped region. Could be up to 2 registers here at max. Ex: 6797 needs 2 reg,
+  others need 1.

Two things:

- Please make this a separate patch that can be reviewed independently
of the rest of the changes, which are just adding new compatible
identifiers.

Will fix this in the next patch set.

- Why can't you simply expose it as a separate controller? Looking at
the way you're changing the corresponding driver, it looks like you're
simply adding an extra base/size. If you simply had a base for the
corresponding GIC interrupts, you could handle as many region as you
want, and have a more generic driver.

May I know the meaning of "simply expose it as a separate controller"?
Or you might like to suggest me any similar driver as a reference? I
will examine it. Current design is based on the fact: We expect
irq-mtk-sysirq needs the optional second base but the third one will not
happen.

If we really need more than 2 bases, we can figure out a more generic
driver at the time, right?

Thanks.

Thanks,

	M.