Hi Geert,
Thanks for the feedback.
> Subject: Re: [PATCH 3/6] can: rcar_canfd: Add multi_global_irqs to
> struct rcar_canfd_hw_info
>
> Hi Biju,
>
> On Sat, Oct 22, 2022 at 1:03 PM Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
> wrote:
> > RZ/G2L has separate IRQ lines for receive FIFO and global error
> > interrupt whereas R-Car has combined IRQ line.
> >
> > Add multi_global_irqs to struct rcar_canfd_hw_info to select the
> > driver to choose between combined and separate irq registration for
> > global interrupts.
> >
> > Signed-off-by: Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
>
> Thanks for your patch!
>
> > --- a/drivers/net/can/rcar/rcar_canfd.c
> > +++ b/drivers/net/can/rcar/rcar_canfd.c
> > @@ -526,6 +526,8 @@ struct rcar_canfd_global; struct
> > rcar_canfd_hw_info {
> > enum rcanfd_chip_id chip_id;
> > u32 max_channels;
> > + /* hardware features */
> > + unsigned multi_global_irqs:1; /* Has multiple global irqs
> */
>
> I'm not sure this is the best name for this flag (especially
> considering the other flag being added in [5/6])...
OK.
>
> > };
> >
> > /* Channel priv data */
> > @@ -603,6 +605,7 @@ static const struct rcar_canfd_hw_info
> > rcar_gen3_hw_info = { static const struct rcar_canfd_hw_info
> rzg2l_hw_info = {
> > .chip_id = RENESAS_RZG2L,
> > .max_channels = 2,
> > + .multi_global_irqs = 1,
> > };
> >
> > static const struct rcar_canfd_hw_info r8a779a0_hw_info = { @@
> > -1874,7 +1877,7 @@ static int rcar_canfd_probe(struct
> platform_device *pdev)
> > of_node_put(of_child);
> > }
> >
> > - if (info->chip_id != RENESAS_RZG2L) {
> > + if (!info->multi_global_irqs) {
> > ch_irq = platform_get_irq_byname_optional(pdev,
> "ch_int");
> > if (ch_irq < 0) {
> > /* For backward compatibility get irq by
> index
> > */ @@ -1957,7 +1960,7 @@ static int rcar_canfd_probe(struct
> platform_device *pdev)
> > gpriv->base = addr;
> >
> > /* Request IRQ that's common for both channels */
> > - if (info->chip_id != RENESAS_RZG2L) {
> > + if (!info->multi_global_irqs) {
>
> All checks for this flag are negative checks. What about inverting
> the meaning of the flag, so these can become positive checks?
OK, will change it to shared_global_irqs as it is shared between
Global receive and error interrupts.
Cheers,
Biju
>
> > err = devm_request_irq(&pdev->dev, ch_irq,
> > rcar_canfd_channel_interrupt,
> 0,
> > "canfd.ch_int", gpriv);
>
> As the patch itself is correct:
> Reviewed-by: Geert Uytterhoeven <geert+renesas@xxxxxxxxx>
>
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 --
> geert@xxxxxxxxxxxxxx
>
> In personal conversations with technical people, I call myself a
> hacker. But when I'm talking to journalists I just say "programmer" or
> something like that.
> -- Linus Torvalds
