RE: [PATCH 3/4] gpio: sifive: Add GPIO driver for SiFive SoCs

[Yash Shah <yash.shah@xxxxxxxxxx>]

> -----Original Message-----
> From: Marc Zyngier <maz@xxxxxxxxxx>
> Sent: 12 November 2019 18:28
> To: Yash Shah <yash.shah@xxxxxxxxxx>
> Cc: linus.walleij@xxxxxxxxxx; bgolaszewski@xxxxxxxxxxxx;
> robh+dt@xxxxxxxxxx; mark.rutland@xxxxxxx; palmer@xxxxxxxxxxx; Paul
> Walmsley ( Sifive) <paul.walmsley@xxxxxxxxxx>; aou@xxxxxxxxxxxxxxxxx;
> tglx@xxxxxxxxxxxxx; jason@xxxxxxxxxxxxxx; bmeng.cn@xxxxxxxxx;
> atish.patra@xxxxxxx; Sagar Kadam <sagar.kadam@xxxxxxxxxx>; linux-
> gpio@xxxxxxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx; linux-
> riscv@xxxxxxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; Sachin Ghadi
> <sachin.ghadi@xxxxxxxxxx>
> Subject: Re: [PATCH 3/4] gpio: sifive: Add GPIO driver for SiFive SoCs
> 
> On 2019-11-12 13:21, Yash Shah wrote:
> > Adds the GPIO driver for SiFive RISC-V SoCs.
> >
> > Signed-off-by: Wesley W. Terpstra <wesley@xxxxxxxxxx>
> > [Atish: Various fixes and code cleanup]
> > Signed-off-by: Atish Patra <atish.patra@xxxxxxx>
> > Signed-off-by: Yash Shah <yash.shah@xxxxxxxxxx>
> 
> [...]
> 
> > +static int sifive_gpio_child_to_parent_hwirq(struct gpio_chip *gc,
> > +					     unsigned int child,
> > +					     unsigned int child_type,
> > +					     unsigned int *parent,
> > +					     unsigned int *parent_type)
> > +{
> > +	/* All these interrupts are level high in the CPU */
> > +	*parent_type = IRQ_TYPE_LEVEL_HIGH;
> 
> It is bizare that you enforce LEVEL_HIGH here, while setting it to NONE in the
> PLIC driver. These things should be consistent.

Will change this to IRQ_TYPE_NONE.

> 
> > +	*parent = child + 7;
> 
> Irk, magic numbers...

This is the offset for GPIO IRQs. Will add a macro for this.
Thanks for your comments!

- Yash