| Message ID | 20221022104357.1276740-4-biju.das.jz@bp.renesas.com (mailing list archive) |
|---|---|
| State | Awaiting Upstream |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | R-Can CAN FD driver enhancements | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Guessing tree name failed - patch did not apply |
Hi Biju, On Sat, Oct 22, 2022 at 1:03 PM Biju Das <biju.das.jz@bp.renesas.com> 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@bp.renesas.com> 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])... > }; > > /* 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? > 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@glider.be> Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org 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
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@bp.renesas.com> > 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@bp.renesas.com> > > 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@glider.be> > > Gr{oetje,eeting}s, > > Geert > > -- > Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- > geert@linux-m68k.org > > 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
diff --git a/drivers/net/can/rcar/rcar_canfd.c b/drivers/net/can/rcar/rcar_canfd.c index ecd7c0df2ded..0d131694c241 100644 --- 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 */ }; /* 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) { err = devm_request_irq(&pdev->dev, ch_irq, rcar_canfd_channel_interrupt, 0, "canfd.ch_int", gpriv);
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@bp.renesas.com> --- drivers/net/can/rcar/rcar_canfd.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)