lkml.org 
[lkml]   [2024]   [Apr]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    Date
    SubjectRe: [PATCH v3 2/2] irqchip/renesas-rzg2l: Add support for RZ/Five SoC
    Hi Prabhakar,

    On Mon, Apr 22, 2024 at 10:51 PM Prabhakar <prabhakar.csengg@gmail.com> wrote:
    > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
    >
    > The IX45 block on the RZ/Five SoC has additional mask registers
    > (NMSK/IMSK/TMSK) compared to the RZ/G2L (family) SoC.
    >
    > A new rzfive_irqc_chip irq_chip is introduced for RZ/Five, where function
    > pointers for irq_(un)mask and irq_(dis/en)able handle the (un)masking
    > of the interrupts. The irq_chip pointer is now passed as an init callback
    > and stored in the priv pointer to differentiate between RZ/G2L and RZ/Five.
    >
    > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
    > ---
    > v2->v3
    > - Added RZ/Five specific irqchip instead of polluting the functions
    > - Fixed review comments pointed by Biju and Geert
    > - Updated commit message
    > - moved locking respective read/write functions

    Thanks for the update!

    > --- a/drivers/irqchip/irq-renesas-rzg2l.c
    > +++ b/drivers/irqchip/irq-renesas-rzg2l.c
    > @@ -138,6 +142,113 @@ static void rzg2l_irqc_eoi(struct irq_data *d)
    > irq_chip_eoi_parent(d);
    > }
    >
    > +static void rzfive_irqc_mask_irq_interrupt(struct rzg2l_irqc_priv *priv,
    > + unsigned int hwirq)
    > +{
    > + u32 bit = BIT(hwirq - IRQC_IRQ_START);
    > +
    > + raw_spin_lock(&priv->lock);

    I think you best move the locking to the callers that really need it...

    > + writel_relaxed(readl_relaxed(priv->base + IMSK) | bit, priv->base + IMSK);
    > + raw_spin_unlock(&priv->lock);
    > +}

    > +static void rzfive_tint_irq_endisable(struct irq_data *d, bool enable)
    > +{
    > + struct rzg2l_irqc_priv *priv = irq_data_to_priv(d);
    > + unsigned int hwirq = irqd_to_hwirq(d);
    > +
    > + if (hwirq >= IRQC_TINT_START && hwirq < IRQC_NUM_IRQ) {
    > + u32 offset = hwirq - IRQC_TINT_START;
    > + u32 tssr_offset = TSSR_OFFSET(offset);
    > + u8 tssr_index = TSSR_INDEX(offset);
    > + u32 reg;
    > +
    > + if (enable)
    > + rzfive_irqc_unmask_tint_interrupt(priv, hwirq);
    > + else
    > + rzfive_irqc_mask_tint_interrupt(priv, hwirq);

    .. else you will do a lock/unlock here, followed by another one below.

    > + raw_spin_lock(&priv->lock);
    > + reg = readl_relaxed(priv->base + TSSR(tssr_index));
    > + if (enable)
    > + reg |= TIEN << TSSEL_SHIFT(tssr_offset);
    > + else
    > + reg &= ~(TIEN << TSSEL_SHIFT(tssr_offset));
    > + writel_relaxed(reg, priv->base + TSSR(tssr_index));
    > + raw_spin_unlock(&priv->lock);
    > + } else {
    > + if (enable)
    > + rzfive_irqc_unmask_irq_interrupt(priv, hwirq);
    > + else
    > + rzfive_irqc_mask_irq_interrupt(priv, hwirq);
    > + }
    > +}

    > @@ -401,7 +529,8 @@ static int rzg2l_irqc_parse_interrupts(struct rzg2l_irqc_priv *priv,
    > return 0;
    > }
    >
    > -static int rzg2l_irqc_init(struct device_node *node, struct device_node *parent)
    > +static int rzg2l_irqc_init_helper(struct device_node *node, struct device_node *parent,

    rzg2l_irqc_common_init()?

    > + const struct irq_chip *irq_chip)
    > {
    > struct irq_domain *irq_domain, *parent_domain;
    > struct platform_device *pdev;

    The rest LGTM, so
    Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

    Gr{oetje,eeting}s,

    Geert

    --
    Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68korg

    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

    \
     
     \ /
      Last update: 2024-04-24 16:59    [W:3.722 / U:0.012 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site