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: > > This adds DT binding documentation for Mediatek MT6797. > > > > Signed-off-by: Mars Cheng <mars.cheng@xxxxxxxxxxxx> > > --- [...] > > > 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. -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
