lkml.org 
[lkml]   [2021]   [Jan]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [RFC PATCH 7/7] irqchip/apple-aic: add SMP support to the Apple AIC driver.
    From
    Date


    > On 21 Jan 2021, at 17:40, Rob Herring <robh@kernel.org> wrote:
    >
    > On Thu, Jan 21, 2021 at 6:52 AM Mohamed Mediouni
    > <mohamed.mediouni@caramail.com> wrote:
    >>
    >>
    >>
    >>> On 21 Jan 2021, at 13:44, Arnd Bergmann <arnd@kernel.org> wrote:
    >>>
    >>> On Wed, Jan 20, 2021 at 2:27 PM Mohamed Mediouni
    >>> <mohamed.mediouni@caramail.com> wrote:
    >>>
    >>>> +#ifdef CONFIG_SMP
    >>>> +static void apple_aic_ipi_send_mask(struct irq_data *d,
    >>>> + const struct cpumask *mask)
    >>>
    >>> Not sure we care about the #ifdef here, given that arch/arm64 does not
    >>> allow building a kernel without CONFIG_SMP.
    >>>
    >>>> + /*
    >>>> + * Ensure that stores to Normal memory are visible to the
    >>>> + * other CPUs before issuing the IPI.
    >>>> + */
    >>>> + wmb();
    >>>> +
    >>>> + for_each_cpu (cpu, mask) {
    >>>> + smp_mb__before_atomic();
    >>>> + atomic_or(1u << irqnr, per_cpu_ptr(&aic_ipi_mask, cpu));
    >>>> + smp_mb__after_atomic();
    >>>> + lcpu = get_cpu();
    >>>> + if (aic.fast_ipi) {
    >>>> + if ((lcpu >> 2) == (cpu >> 2))
    >>>> + write_sysreg(cpu & 3, SR_APPLE_IPI_LOCAL);
    >>>> + else
    >>>> + write_sysreg((cpu & 3) | ((cpu >> 2) << 16),
    >>>> + SR_APPLE_IPI_REMOTE);
    >>>> + } else
    >>>> + writel(lcpu == cpu ? REG_IPI_FLAG_SELF :
    >>>> + (REG_IPI_FLAG_OTHER << cpu),
    >>>> + aic.base + REG_IPI_SET);
    >>>> + put_cpu();
    >>>> + }
    >>>> +
    >>>> + /* Force the above writes to be executed */
    >>>> + if (aic.fast_ipi)
    >>>> + isb();
    >>>> +}
    >>>
    >>> Since this just loops over all CPUs, I'd probably just turn it into
    >>> an ipi_send_single() callback and have the caller do the
    >>> loop for simplicity.
    >>>
    >>> I also have the feeling that splitting one hardware IPI into multiple
    >>> logical interrupts, which are then all registered by the same irq
    >>> handler adds a little more complexity than necessary.
    >>>
    >>> Changing this would of course require modifications to
    >>> arch/arm64/kernel/smp.c, which is hardwired to use
    >>> CONFIG_GENERIC_IRQ_IPI in smp_cross_call(), and allowing
    >>> a different code path there may be worse than emulating an
    >>> irqchip.
    >>>
    >>>> @@ -186,8 +325,11 @@ static int __init apple_aic_init(struct device_node *node,
    >>>> if (WARN(!aic.base, "unable to map aic registers\n"))
    >>>> return -EINVAL;
    >>>>
    >>>> + aic.fast_ipi = of_property_read_bool(node, "fast-ipi");
    >>>
    >>> Where is this property documented, and what decides which one to use?
    >> It’s getting documented in the next patch set.
    >>
    >> This property is there to enable support for older iPhone processors
    >> later on, some of which do not have fast IPI support.
    >>
    >> On Apple M1, fast-ipi is always on.
    >
    > This should be implied by the compatible string which needs to be more
    > specific and include the SoC name.
    >
    > Rob

    Then we’ll eventually have two aic compatible strings, aic which is compatible
    with Apple A7 onwards and aicv2 which is a superset with fast IPI (introduced
    on the Apple A11, 3 years ago, with no further programmer-visible changes since
    then).

    Does that look right?

    Thank you,

    \
     
     \ /
      Last update: 2021-01-21 17:48    [W:3.460 / U:0.020 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site