Re: [PATCHv3 5/6] irqchip/irq-pruss-intc: Add support for ICSSG INTC on K3 SoCs

[Suman Anna]

Hi Marc,

On 7/3/20 12:05 PM, Grzegorz Jaszczyk wrote:
> On Thu, 2 Jul 2020 at 19:59, Marc Zyngier <maz@xxxxxxxxxx> wrote:
> > On 2020-07-02 15:17, Grzegorz Jaszczyk wrote:
> > > From: Suman Anna <s-anna@xxxxxx>
> > >
> > > The K3 AM65x and J721E SoCs have the next generation of the PRU-ICSS
> > > IP,
> > > commonly called ICSSG. The PRUSS INTC present within the ICSSG supports
> > > more System Events (160 vs 64), more Interrupt Channels and Host
> > > Interrupts
> > > (20 vs 10) compared to the previous generation PRUSS INTC instances.
> > > The
> > > first 2 and the last 10 of these host interrupt lines are used by the
> > > PRU and other auxiliary cores and sub-modules within the ICSSG, with 8
> > > host interrupts connected to MPU. The host interrupts 5, 6, 7 are also
> > > connected to the other ICSSG instances within the SoC and can be
> > > partitioned as per system integration through the board dts files.
> > >
> > > Enhance the PRUSS INTC driver to add support for this ICSSG INTC
> > > instance. This support is added using specific compatible and match
> > > data and updating the code to use this data instead of the current
> > > hard-coded macros. The INTC config structure is updated to use the
> > > higher events and channels on all SoCs, while limiting the actual
> > > processing to only the relevant number of events/channels/interrupts.
> > >
> > > Signed-off-by: Suman Anna <s-anna@xxxxxx>
> > > Signed-off-by: Grzegorz Jaszczyk <grzegorz.jaszczyk@xxxxxxxxxx>
> > > ---
> > > v2->v3:
> > > - Change patch order: use it directly after "irqchip/irq-pruss-intc:
> > >    Implement irq_{get,set}_irqchip_state ops" and before new
> > >    "irqchip/irq-pruss-intc: Add event mapping support" in order to
> > > reduce
> > >    diff.
> >
> > The diff would be even smaller if you introduced a variable number of
> > inputs the first place, i.e. in patch #2. Most if this patch just
> > retrofits it. Please squash these changes into that initial patch,
> > and only add the platform stuff here.

Yeah, all the variables were introduced in this patch based on patch 
history. Until this new IP came, we haven't had a need to use variables 
in the original driver code. That's also the reason for this being a 
separate patch rather than squashed into the original patch.

regards
Suman

> Sure I will do that.
>
> Thank you,
> Grzegorz