Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v9 1/4] soc: mediatek: Refine scpsys to support multiple platform
From: Matthias Brugger @ 2016-10-25 14:04 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1476953798-23263-2-git-send-email-jamesjj.liao@mediatek.com>

Hi James,

On 10/20/2016 10:56 AM, James Liao wrote:
> -static int scpsys_probe(struct platform_device *pdev)
> +static void init_clks(struct platform_device *pdev, struct clk *clk[CLK_MAX])

I prefer struct clk **clk.

> +{
> +	int i;
> +
> +	for (i = CLK_NONE + 1; i < CLK_MAX; i++)
> +		clk[i] = devm_clk_get(&pdev->dev, clk_names[i]);
> +}
> +
> +static struct scp *init_scp(struct platform_device *pdev,
> +			const struct scp_domain_data *scp_domain_data, int num)
>  {
>  	struct genpd_onecell_data *pd_data;
>  	struct resource *res;
> -	int i, j, ret;
> +	int i, j;
>  	struct scp *scp;
> -	struct clk *clk[MT8173_CLK_MAX];
> +	struct clk *clk[CLK_MAX];

should be *[CLK_MAX - 1] but I would prefer to define in the enum:
CLK_MAX = CLK_VENC_LT,

If you are ok with it, I can fix both of my comments when applying.

Regards,
Matthias

^ permalink raw reply

* [PATCH 2/2] arm64: Support systems without FP/ASIMD
From: Ard Biesheuvel @ 2016-10-25 14:00 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1477403433-2279-3-git-send-email-suzuki.poulose@arm.com>

Hi Suzuki,

On 25 October 2016 at 14:50, Suzuki K Poulose <suzuki.poulose@arm.com> wrote:
> The arm64 kernel assumes that FP/ASIMD units are always present
> and accesses the FP/ASIMD specific registers unconditionally. This
> could cause problems when they are absent. This patch adds the
> support for kernel handling systems without FP/ASIMD by skipping the
> register access within the kernel. For kvm, we trap the accesses
> to FP/ASIMD and inject an undefined instruction exception to the VM.
>
> The callers of the exported kernel_neon_begin_parital() should
> make sure that the FP/ASIMD is supported.
>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: Christoffer Dall <christoffer.dall@linaro.org>
> Cc: Marc Zyngier <marc.zyngier@arm.com>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
> ---
> ---
>  arch/arm64/crypto/aes-ce-ccm-glue.c |  2 +-
>  arch/arm64/crypto/aes-ce-cipher.c   |  2 ++
>  arch/arm64/crypto/ghash-ce-glue.c   |  2 ++
>  arch/arm64/crypto/sha1-ce-glue.c    |  2 ++
>  arch/arm64/include/asm/cpufeature.h |  8 +++++++-
>  arch/arm64/include/asm/neon.h       |  3 ++-
>  arch/arm64/kernel/cpufeature.c      | 15 +++++++++++++++
>  arch/arm64/kernel/fpsimd.c          | 14 ++++++++++++++
>  arch/arm64/kvm/handle_exit.c        | 11 +++++++++++
>  arch/arm64/kvm/hyp/hyp-entry.S      |  9 ++++++++-
>  arch/arm64/kvm/hyp/switch.c         |  5 ++++-
>  11 files changed, 68 insertions(+), 5 deletions(-)
>
> diff --git a/arch/arm64/crypto/aes-ce-ccm-glue.c b/arch/arm64/crypto/aes-ce-ccm-glue.c
> index f4bf2f2..d001b4e 100644
> --- a/arch/arm64/crypto/aes-ce-ccm-glue.c
> +++ b/arch/arm64/crypto/aes-ce-ccm-glue.c
> @@ -296,7 +296,7 @@ static struct aead_alg ccm_aes_alg = {
>
>  static int __init aes_mod_init(void)
>  {
> -       if (!(elf_hwcap & HWCAP_AES))
> +       if (!(elf_hwcap & HWCAP_AES) || !system_supports_fpsimd())

This looks weird to me. All crypto extensionsinstructions except CRC
operate strictly on FP/ASIMD registers, and so support for FP/ASIMD is
implied by having HWCAP_AES. In other words, I think it makes more
sense to sanity check that the info registers are consistent with each
other in core code than modifying each user (which for HWCAP_xxx
includes userland) to double check that the world is sane.

>                 return -ENODEV;
>         return crypto_register_aead(&ccm_aes_alg);
>  }
> diff --git a/arch/arm64/crypto/aes-ce-cipher.c b/arch/arm64/crypto/aes-ce-cipher.c
> index f7bd9bf..1a43be2 100644
> --- a/arch/arm64/crypto/aes-ce-cipher.c
> +++ b/arch/arm64/crypto/aes-ce-cipher.c
> @@ -253,6 +253,8 @@ static struct crypto_alg aes_alg = {
>
>  static int __init aes_mod_init(void)
>  {
> +       if (!system_supports_fpsimd())
> +               return -ENODEV;
>         return crypto_register_alg(&aes_alg);
>  }
>
> diff --git a/arch/arm64/crypto/ghash-ce-glue.c b/arch/arm64/crypto/ghash-ce-glue.c
> index 833ec1e..2bc518d 100644
> --- a/arch/arm64/crypto/ghash-ce-glue.c
> +++ b/arch/arm64/crypto/ghash-ce-glue.c
> @@ -144,6 +144,8 @@ static struct shash_alg ghash_alg = {
>
>  static int __init ghash_ce_mod_init(void)
>  {
> +       if (!system_supports_fpsimd())
> +               return -ENODEV;
>         return crypto_register_shash(&ghash_alg);
>  }
>
> diff --git a/arch/arm64/crypto/sha1-ce-glue.c b/arch/arm64/crypto/sha1-ce-glue.c
> index aefda98..9f3427a 100644
> --- a/arch/arm64/crypto/sha1-ce-glue.c
> +++ b/arch/arm64/crypto/sha1-ce-glue.c
> @@ -102,6 +102,8 @@ static struct shash_alg alg = {
>
>  static int __init sha1_ce_mod_init(void)
>  {
> +       if (!system_supports_fpsimd())
> +               return -ENODEV;
>         return crypto_register_shash(&alg);
>  }
>
> diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h
> index ae5e994..63d739c 100644
> --- a/arch/arm64/include/asm/cpufeature.h
> +++ b/arch/arm64/include/asm/cpufeature.h
> @@ -38,8 +38,9 @@
>  #define ARM64_HAS_32BIT_EL0                    13
>  #define ARM64_HYP_OFFSET_LOW                   14
>  #define ARM64_MISMATCHED_CACHE_LINE_SIZE       15
> +#define ARM64_HAS_NO_FPSIMD                    16
>
> -#define ARM64_NCAPS                            16
> +#define ARM64_NCAPS                            17
>
>  #ifndef __ASSEMBLY__
>
> @@ -236,6 +237,11 @@ static inline bool system_supports_mixed_endian_el0(void)
>         return id_aa64mmfr0_mixed_endian_el0(read_system_reg(SYS_ID_AA64MMFR0_EL1));
>  }
>
> +static inline bool system_supports_fpsimd(void)
> +{
> +       return !cpus_have_const_cap(ARM64_HAS_NO_FPSIMD);
> +}
> +
>  #endif /* __ASSEMBLY__ */
>
>  #endif
> diff --git a/arch/arm64/include/asm/neon.h b/arch/arm64/include/asm/neon.h
> index 13ce4cc..ad4cdc9 100644
> --- a/arch/arm64/include/asm/neon.h
> +++ b/arch/arm64/include/asm/neon.h
> @@ -9,8 +9,9 @@
>   */
>
>  #include <linux/types.h>
> +#include <asm/fpsimd.h>
>
> -#define cpu_has_neon()         (1)
> +#define cpu_has_neon()         system_supports_fpsimd()
>
>  #define kernel_neon_begin()    kernel_neon_begin_partial(32)
>
> diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
> index d577f26..8f22544 100644
> --- a/arch/arm64/kernel/cpufeature.c
> +++ b/arch/arm64/kernel/cpufeature.c
> @@ -744,6 +744,14 @@ static bool hyp_offset_low(const struct arm64_cpu_capabilities *entry,
>         return idmap_addr > GENMASK(VA_BITS - 2, 0) && !is_kernel_in_hyp_mode();
>  }
>
> +static bool has_no_fpsimd(const struct arm64_cpu_capabilities *entry, int __unused)
> +{
> +       u64 pfr0 = read_system_reg(SYS_ID_AA64PFR0_EL1);
> +
> +       return cpuid_feature_extract_signed_field(pfr0,
> +                                       ID_AA64PFR0_FP_SHIFT) < 0;
> +}
> +
>  static const struct arm64_cpu_capabilities arm64_features[] = {
>         {
>                 .desc = "GIC system register CPU interface",
> @@ -827,6 +835,13 @@ static const struct arm64_cpu_capabilities arm64_features[] = {
>                 .def_scope = SCOPE_SYSTEM,
>                 .matches = hyp_offset_low,
>         },
> +       {
> +               /* FP/SIMD is not implemented */
> +               .capability = ARM64_HAS_NO_FPSIMD,
> +               .def_scope = SCOPE_SYSTEM,
> +               .min_field_value = 0,
> +               .matches = has_no_fpsimd,
> +       },
>         {},
>  };
>
> diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c
> index 975b274..80da5aa 100644
> --- a/arch/arm64/kernel/fpsimd.c
> +++ b/arch/arm64/kernel/fpsimd.c
> @@ -127,6 +127,8 @@ void do_fpsimd_exc(unsigned int esr, struct pt_regs *regs)
>
>  void fpsimd_thread_switch(struct task_struct *next)
>  {
> +       if (!system_supports_fpsimd())
> +               return;
>         /*
>          * Save the current FPSIMD state to memory, but only if whatever is in
>          * the registers is in fact the most recent userland FPSIMD state of
> @@ -157,6 +159,8 @@ void fpsimd_thread_switch(struct task_struct *next)
>
>  void fpsimd_flush_thread(void)
>  {
> +       if (!system_supports_fpsimd())
> +               return;
>         memset(&current->thread.fpsimd_state, 0, sizeof(struct fpsimd_state));
>         fpsimd_flush_task_state(current);
>         set_thread_flag(TIF_FOREIGN_FPSTATE);
> @@ -168,6 +172,8 @@ void fpsimd_flush_thread(void)
>   */
>  void fpsimd_preserve_current_state(void)
>  {
> +       if (!system_supports_fpsimd())
> +               return;
>         preempt_disable();
>         if (!test_thread_flag(TIF_FOREIGN_FPSTATE))
>                 fpsimd_save_state(&current->thread.fpsimd_state);
> @@ -181,6 +187,8 @@ void fpsimd_preserve_current_state(void)
>   */
>  void fpsimd_restore_current_state(void)
>  {
> +       if (!system_supports_fpsimd())
> +               return;
>         preempt_disable();
>         if (test_and_clear_thread_flag(TIF_FOREIGN_FPSTATE)) {
>                 struct fpsimd_state *st = &current->thread.fpsimd_state;
> @@ -199,6 +207,8 @@ void fpsimd_restore_current_state(void)
>   */
>  void fpsimd_update_current_state(struct fpsimd_state *state)
>  {
> +       if (!system_supports_fpsimd())
> +               return;
>         preempt_disable();
>         fpsimd_load_state(state);
>         if (test_and_clear_thread_flag(TIF_FOREIGN_FPSTATE)) {
> @@ -228,6 +238,8 @@ static DEFINE_PER_CPU(struct fpsimd_partial_state, softirq_fpsimdstate);
>   */
>  void kernel_neon_begin_partial(u32 num_regs)
>  {
> +       if (WARN_ON(!system_supports_fpsimd()))
> +               return;
>         if (in_interrupt()) {
>                 struct fpsimd_partial_state *s = this_cpu_ptr(
>                         in_irq() ? &hardirq_fpsimdstate : &softirq_fpsimdstate);
> @@ -252,6 +264,8 @@ EXPORT_SYMBOL(kernel_neon_begin_partial);
>
>  void kernel_neon_end(void)
>  {
> +       if (!system_supports_fpsimd())
> +               return;
>         if (in_interrupt()) {
>                 struct fpsimd_partial_state *s = this_cpu_ptr(
>                         in_irq() ? &hardirq_fpsimdstate : &softirq_fpsimdstate);
> diff --git a/arch/arm64/kvm/handle_exit.c b/arch/arm64/kvm/handle_exit.c
> index fa96fe2..39e055a 100644
> --- a/arch/arm64/kvm/handle_exit.c
> +++ b/arch/arm64/kvm/handle_exit.c
> @@ -57,6 +57,16 @@ static int handle_smc(struct kvm_vcpu *vcpu, struct kvm_run *run)
>         return 1;
>  }
>
> +/*
> + * Guest access to FP/ASIMD registers are routed to this handler only
> + * when the system doesn't support FP/ASIMD.
> + */
> +static int handle_no_fpsimd(struct kvm_vcpu *vcpu, struct kvm_run *run)
> +{
> +       kvm_inject_undefined(vcpu);
> +       return 1;
> +}
> +
>  /**
>   * kvm_handle_wfx - handle a wait-for-interrupts or wait-for-event
>   *                 instruction executed by a guest
> @@ -144,6 +154,7 @@ static exit_handle_fn arm_exit_handlers[] = {
>         [ESR_ELx_EC_BREAKPT_LOW]= kvm_handle_guest_debug,
>         [ESR_ELx_EC_BKPT32]     = kvm_handle_guest_debug,
>         [ESR_ELx_EC_BRK64]      = kvm_handle_guest_debug,
> +       [ESR_ELx_EC_FP_ASIMD]   = handle_no_fpsimd,
>  };
>
>  static exit_handle_fn kvm_get_exit_handler(struct kvm_vcpu *vcpu)
> diff --git a/arch/arm64/kvm/hyp/hyp-entry.S b/arch/arm64/kvm/hyp/hyp-entry.S
> index f6d9694..16167d7 100644
> --- a/arch/arm64/kvm/hyp/hyp-entry.S
> +++ b/arch/arm64/kvm/hyp/hyp-entry.S
> @@ -117,9 +117,16 @@ el1_trap:
>          * x2: ESR_EC
>          */
>
> -       /* Guest accessed VFP/SIMD registers, save host, restore Guest */
> +       /*
> +        * We trap the first access to the FP/SIMD to save the host context
> +        * and restore the guest context lazily.
> +        * If FP/SIMD is not implemented, handle the trap and inject an
> +        * undefined instruction exception to the guest.
> +        */
> +alternative_if_not ARM64_HAS_NO_FPSIMD
>         cmp     x2, #ESR_ELx_EC_FP_ASIMD
>         b.eq    __fpsimd_guest_restore
> +alternative_else_nop_endif
>
>         mrs     x0, tpidr_el2
>         mov     x1, #ARM_EXCEPTION_TRAP
> diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c
> index ae7855f..f2d0c4f 100644
> --- a/arch/arm64/kvm/hyp/switch.c
> +++ b/arch/arm64/kvm/hyp/switch.c
> @@ -18,6 +18,7 @@
>  #include <linux/types.h>
>  #include <asm/kvm_asm.h>
>  #include <asm/kvm_hyp.h>
> +#include <asm/fpsimd.h>
>
>  static bool __hyp_text __fpsimd_enabled_nvhe(void)
>  {
> @@ -73,9 +74,11 @@ static void __hyp_text __activate_traps(struct kvm_vcpu *vcpu)
>          * traps are only taken to EL2 if the operation would not otherwise
>          * trap to EL1.  Therefore, always make sure that for 32-bit guests,
>          * we set FPEXC.EN to prevent traps to EL1, when setting the TFP bit.
> +        * If FP/ASIMD is not implemented, FPEXC is UNDEFINED and any access to
> +        * it will cause an exception.
>          */
>         val = vcpu->arch.hcr_el2;
> -       if (!(val & HCR_RW)) {
> +       if (!(val & HCR_RW) && system_supports_fpsimd()) {
>                 write_sysreg(1 << 30, fpexc32_el2);
>                 isb();
>         }
> --
> 2.7.4
>

^ permalink raw reply

* Disabling an interrupt in the handler locks the system up
From: Thomas Gleixner @ 2016-10-25 13:56 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <580F647B.5000202@free.fr>

On Tue, 25 Oct 2016, Mason wrote:
> Is the irq_mask() call-back exposed via some module-visible API?

No. And there is no reason to do so.

Thanks,

	tglx

^ permalink raw reply

* Disabling an interrupt in the handler locks the system up
From: Mason @ 2016-10-25 13:56 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <534c4588-f220-25a3-e7aa-84484f348bd1@arm.com>

On 25/10/2016 12:45, Marc Zyngier wrote:
> On 25/10/16 09:36, Mason wrote:
>> On 25/10/2016 10:29, Sebastian Frias wrote:
>>
>>> On 10/24/2016 06:55 PM, Thomas Gleixner wrote:
>>>
>>>> On Mon, 24 Oct 2016, Mason wrote:
>>>>
>>>>> For the record, setting the IRQ_DISABLE_UNLAZY flag for this device
>>>>> makes the system lock-up disappear.
>>>>
>>>> The way how lazy irq disabling works is:
>>>>
>>>> 1) Interrupt is marked disabled in software, but the hardware is not masked
>>>>
>>>> 2) If the interrupt fires befor the interrupt is reenabled, then it's
>>>>    masked at the hardware level in the low level interrupt flow handler.
>>>
>>> Would you mind explaining what is the intention behind?
>>> Because it does not seem obvious why there isn't a direct map between
>>> "disable_irq*()" and "mask_irq()"
>>
>> I had a similar, but slightly different question:
>>
>> What is the difference between struct irq_chip's
>>
>>  * @irq_shutdown:	shut down the interrupt (defaults to ->disable if NULL)
>>  * @irq_disable:	disable the interrupt
>>  * @irq_mask:		mask an interrupt source
> 
> One important difference between disable and mask is that disable is
> perfectly allowed not to care about pending signals, whereas mask must
> preserve an interrupt becoming pending whilst masked.

(For my information)

Is it correct to say that "mask" is supposed to defer any interrupt
until sometime later; while "disable" will simply discard incoming
interrupts, losing them forever.

Is the irq_mask() call-back exposed via some module-visible API?

include/linux/interrupt.h documents mostly enable/disable variants.

extern void disable_irq_nosync(unsigned int irq);
extern bool disable_hardirq(unsigned int irq);
extern void disable_irq(unsigned int irq);
extern void disable_percpu_irq(unsigned int irq);
extern void enable_irq(unsigned int irq);
extern void enable_percpu_irq(unsigned int irq, unsigned int type);
extern bool irq_percpu_is_enabled(unsigned int irq);
extern void irq_wake_thread(unsigned int irq, void *dev_id);

Regards.

^ permalink raw reply

* [PATCH 2/2] arm64: Support systems without FP/ASIMD
From: Suzuki K Poulose @ 2016-10-25 13:50 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1477403433-2279-1-git-send-email-suzuki.poulose@arm.com>

The arm64 kernel assumes that FP/ASIMD units are always present
and accesses the FP/ASIMD specific registers unconditionally. This
could cause problems when they are absent. This patch adds the
support for kernel handling systems without FP/ASIMD by skipping the
register access within the kernel. For kvm, we trap the accesses
to FP/ASIMD and inject an undefined instruction exception to the VM.

The callers of the exported kernel_neon_begin_parital() should
make sure that the FP/ASIMD is supported.

Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Christoffer Dall <christoffer.dall@linaro.org>
Cc: Marc Zyngier <marc.zyngier@arm.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
---
---
 arch/arm64/crypto/aes-ce-ccm-glue.c |  2 +-
 arch/arm64/crypto/aes-ce-cipher.c   |  2 ++
 arch/arm64/crypto/ghash-ce-glue.c   |  2 ++
 arch/arm64/crypto/sha1-ce-glue.c    |  2 ++
 arch/arm64/include/asm/cpufeature.h |  8 +++++++-
 arch/arm64/include/asm/neon.h       |  3 ++-
 arch/arm64/kernel/cpufeature.c      | 15 +++++++++++++++
 arch/arm64/kernel/fpsimd.c          | 14 ++++++++++++++
 arch/arm64/kvm/handle_exit.c        | 11 +++++++++++
 arch/arm64/kvm/hyp/hyp-entry.S      |  9 ++++++++-
 arch/arm64/kvm/hyp/switch.c         |  5 ++++-
 11 files changed, 68 insertions(+), 5 deletions(-)

diff --git a/arch/arm64/crypto/aes-ce-ccm-glue.c b/arch/arm64/crypto/aes-ce-ccm-glue.c
index f4bf2f2..d001b4e 100644
--- a/arch/arm64/crypto/aes-ce-ccm-glue.c
+++ b/arch/arm64/crypto/aes-ce-ccm-glue.c
@@ -296,7 +296,7 @@ static struct aead_alg ccm_aes_alg = {
 
 static int __init aes_mod_init(void)
 {
-	if (!(elf_hwcap & HWCAP_AES))
+	if (!(elf_hwcap & HWCAP_AES) || !system_supports_fpsimd())
 		return -ENODEV;
 	return crypto_register_aead(&ccm_aes_alg);
 }
diff --git a/arch/arm64/crypto/aes-ce-cipher.c b/arch/arm64/crypto/aes-ce-cipher.c
index f7bd9bf..1a43be2 100644
--- a/arch/arm64/crypto/aes-ce-cipher.c
+++ b/arch/arm64/crypto/aes-ce-cipher.c
@@ -253,6 +253,8 @@ static struct crypto_alg aes_alg = {
 
 static int __init aes_mod_init(void)
 {
+	if (!system_supports_fpsimd())
+		return -ENODEV;
 	return crypto_register_alg(&aes_alg);
 }
 
diff --git a/arch/arm64/crypto/ghash-ce-glue.c b/arch/arm64/crypto/ghash-ce-glue.c
index 833ec1e..2bc518d 100644
--- a/arch/arm64/crypto/ghash-ce-glue.c
+++ b/arch/arm64/crypto/ghash-ce-glue.c
@@ -144,6 +144,8 @@ static struct shash_alg ghash_alg = {
 
 static int __init ghash_ce_mod_init(void)
 {
+	if (!system_supports_fpsimd())
+		return -ENODEV;
 	return crypto_register_shash(&ghash_alg);
 }
 
diff --git a/arch/arm64/crypto/sha1-ce-glue.c b/arch/arm64/crypto/sha1-ce-glue.c
index aefda98..9f3427a 100644
--- a/arch/arm64/crypto/sha1-ce-glue.c
+++ b/arch/arm64/crypto/sha1-ce-glue.c
@@ -102,6 +102,8 @@ static struct shash_alg alg = {
 
 static int __init sha1_ce_mod_init(void)
 {
+	if (!system_supports_fpsimd())
+		return -ENODEV;
 	return crypto_register_shash(&alg);
 }
 
diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h
index ae5e994..63d739c 100644
--- a/arch/arm64/include/asm/cpufeature.h
+++ b/arch/arm64/include/asm/cpufeature.h
@@ -38,8 +38,9 @@
 #define ARM64_HAS_32BIT_EL0			13
 #define ARM64_HYP_OFFSET_LOW			14
 #define ARM64_MISMATCHED_CACHE_LINE_SIZE	15
+#define ARM64_HAS_NO_FPSIMD			16
 
-#define ARM64_NCAPS				16
+#define ARM64_NCAPS				17
 
 #ifndef __ASSEMBLY__
 
@@ -236,6 +237,11 @@ static inline bool system_supports_mixed_endian_el0(void)
 	return id_aa64mmfr0_mixed_endian_el0(read_system_reg(SYS_ID_AA64MMFR0_EL1));
 }
 
+static inline bool system_supports_fpsimd(void)
+{
+	return !cpus_have_const_cap(ARM64_HAS_NO_FPSIMD);
+}
+
 #endif /* __ASSEMBLY__ */
 
 #endif
diff --git a/arch/arm64/include/asm/neon.h b/arch/arm64/include/asm/neon.h
index 13ce4cc..ad4cdc9 100644
--- a/arch/arm64/include/asm/neon.h
+++ b/arch/arm64/include/asm/neon.h
@@ -9,8 +9,9 @@
  */
 
 #include <linux/types.h>
+#include <asm/fpsimd.h>
 
-#define cpu_has_neon()		(1)
+#define cpu_has_neon()		system_supports_fpsimd()
 
 #define kernel_neon_begin()	kernel_neon_begin_partial(32)
 
diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index d577f26..8f22544 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -744,6 +744,14 @@ static bool hyp_offset_low(const struct arm64_cpu_capabilities *entry,
 	return idmap_addr > GENMASK(VA_BITS - 2, 0) && !is_kernel_in_hyp_mode();
 }
 
+static bool has_no_fpsimd(const struct arm64_cpu_capabilities *entry, int __unused)
+{
+	u64 pfr0 = read_system_reg(SYS_ID_AA64PFR0_EL1);
+
+	return cpuid_feature_extract_signed_field(pfr0,
+					ID_AA64PFR0_FP_SHIFT) < 0;
+}
+
 static const struct arm64_cpu_capabilities arm64_features[] = {
 	{
 		.desc = "GIC system register CPU interface",
@@ -827,6 +835,13 @@ static const struct arm64_cpu_capabilities arm64_features[] = {
 		.def_scope = SCOPE_SYSTEM,
 		.matches = hyp_offset_low,
 	},
+	{
+		/* FP/SIMD is not implemented */
+		.capability = ARM64_HAS_NO_FPSIMD,
+		.def_scope = SCOPE_SYSTEM,
+		.min_field_value = 0,
+		.matches = has_no_fpsimd,
+	},
 	{},
 };
 
diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c
index 975b274..80da5aa 100644
--- a/arch/arm64/kernel/fpsimd.c
+++ b/arch/arm64/kernel/fpsimd.c
@@ -127,6 +127,8 @@ void do_fpsimd_exc(unsigned int esr, struct pt_regs *regs)
 
 void fpsimd_thread_switch(struct task_struct *next)
 {
+	if (!system_supports_fpsimd())
+		return;
 	/*
 	 * Save the current FPSIMD state to memory, but only if whatever is in
 	 * the registers is in fact the most recent userland FPSIMD state of
@@ -157,6 +159,8 @@ void fpsimd_thread_switch(struct task_struct *next)
 
 void fpsimd_flush_thread(void)
 {
+	if (!system_supports_fpsimd())
+		return;
 	memset(&current->thread.fpsimd_state, 0, sizeof(struct fpsimd_state));
 	fpsimd_flush_task_state(current);
 	set_thread_flag(TIF_FOREIGN_FPSTATE);
@@ -168,6 +172,8 @@ void fpsimd_flush_thread(void)
  */
 void fpsimd_preserve_current_state(void)
 {
+	if (!system_supports_fpsimd())
+		return;
 	preempt_disable();
 	if (!test_thread_flag(TIF_FOREIGN_FPSTATE))
 		fpsimd_save_state(&current->thread.fpsimd_state);
@@ -181,6 +187,8 @@ void fpsimd_preserve_current_state(void)
  */
 void fpsimd_restore_current_state(void)
 {
+	if (!system_supports_fpsimd())
+		return;
 	preempt_disable();
 	if (test_and_clear_thread_flag(TIF_FOREIGN_FPSTATE)) {
 		struct fpsimd_state *st = &current->thread.fpsimd_state;
@@ -199,6 +207,8 @@ void fpsimd_restore_current_state(void)
  */
 void fpsimd_update_current_state(struct fpsimd_state *state)
 {
+	if (!system_supports_fpsimd())
+		return;
 	preempt_disable();
 	fpsimd_load_state(state);
 	if (test_and_clear_thread_flag(TIF_FOREIGN_FPSTATE)) {
@@ -228,6 +238,8 @@ static DEFINE_PER_CPU(struct fpsimd_partial_state, softirq_fpsimdstate);
  */
 void kernel_neon_begin_partial(u32 num_regs)
 {
+	if (WARN_ON(!system_supports_fpsimd()))
+		return;
 	if (in_interrupt()) {
 		struct fpsimd_partial_state *s = this_cpu_ptr(
 			in_irq() ? &hardirq_fpsimdstate : &softirq_fpsimdstate);
@@ -252,6 +264,8 @@ EXPORT_SYMBOL(kernel_neon_begin_partial);
 
 void kernel_neon_end(void)
 {
+	if (!system_supports_fpsimd())
+		return;
 	if (in_interrupt()) {
 		struct fpsimd_partial_state *s = this_cpu_ptr(
 			in_irq() ? &hardirq_fpsimdstate : &softirq_fpsimdstate);
diff --git a/arch/arm64/kvm/handle_exit.c b/arch/arm64/kvm/handle_exit.c
index fa96fe2..39e055a 100644
--- a/arch/arm64/kvm/handle_exit.c
+++ b/arch/arm64/kvm/handle_exit.c
@@ -57,6 +57,16 @@ static int handle_smc(struct kvm_vcpu *vcpu, struct kvm_run *run)
 	return 1;
 }
 
+/*
+ * Guest access to FP/ASIMD registers are routed to this handler only
+ * when the system doesn't support FP/ASIMD.
+ */
+static int handle_no_fpsimd(struct kvm_vcpu *vcpu, struct kvm_run *run)
+{
+	kvm_inject_undefined(vcpu);
+	return 1;
+}
+
 /**
  * kvm_handle_wfx - handle a wait-for-interrupts or wait-for-event
  *		    instruction executed by a guest
@@ -144,6 +154,7 @@ static exit_handle_fn arm_exit_handlers[] = {
 	[ESR_ELx_EC_BREAKPT_LOW]= kvm_handle_guest_debug,
 	[ESR_ELx_EC_BKPT32]	= kvm_handle_guest_debug,
 	[ESR_ELx_EC_BRK64]	= kvm_handle_guest_debug,
+	[ESR_ELx_EC_FP_ASIMD]	= handle_no_fpsimd,
 };
 
 static exit_handle_fn kvm_get_exit_handler(struct kvm_vcpu *vcpu)
diff --git a/arch/arm64/kvm/hyp/hyp-entry.S b/arch/arm64/kvm/hyp/hyp-entry.S
index f6d9694..16167d7 100644
--- a/arch/arm64/kvm/hyp/hyp-entry.S
+++ b/arch/arm64/kvm/hyp/hyp-entry.S
@@ -117,9 +117,16 @@ el1_trap:
 	 * x2: ESR_EC
 	 */
 
-	/* Guest accessed VFP/SIMD registers, save host, restore Guest */
+	/*
+	 * We trap the first access to the FP/SIMD to save the host context
+	 * and restore the guest context lazily.
+	 * If FP/SIMD is not implemented, handle the trap and inject an
+	 * undefined instruction exception to the guest.
+	 */
+alternative_if_not ARM64_HAS_NO_FPSIMD
 	cmp	x2, #ESR_ELx_EC_FP_ASIMD
 	b.eq	__fpsimd_guest_restore
+alternative_else_nop_endif
 
 	mrs	x0, tpidr_el2
 	mov	x1, #ARM_EXCEPTION_TRAP
diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c
index ae7855f..f2d0c4f 100644
--- a/arch/arm64/kvm/hyp/switch.c
+++ b/arch/arm64/kvm/hyp/switch.c
@@ -18,6 +18,7 @@
 #include <linux/types.h>
 #include <asm/kvm_asm.h>
 #include <asm/kvm_hyp.h>
+#include <asm/fpsimd.h>
 
 static bool __hyp_text __fpsimd_enabled_nvhe(void)
 {
@@ -73,9 +74,11 @@ static void __hyp_text __activate_traps(struct kvm_vcpu *vcpu)
 	 * traps are only taken to EL2 if the operation would not otherwise
 	 * trap to EL1.  Therefore, always make sure that for 32-bit guests,
 	 * we set FPEXC.EN to prevent traps to EL1, when setting the TFP bit.
+	 * If FP/ASIMD is not implemented, FPEXC is UNDEFINED and any access to
+	 * it will cause an exception.
 	 */
 	val = vcpu->arch.hcr_el2;
-	if (!(val & HCR_RW)) {
+	if (!(val & HCR_RW) && system_supports_fpsimd()) {
 		write_sysreg(1 << 30, fpexc32_el2);
 		isb();
 	}
-- 
2.7.4

^ permalink raw reply related

* [PATCH 1/2] arm64: Add hypervisor safe helper for checking constant capabilities
From: Suzuki K Poulose @ 2016-10-25 13:50 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1477403433-2279-1-git-send-email-suzuki.poulose@arm.com>

The hypervisor may not have full access to the kernel data structures
and hence cannot safely use cpus_have_cap() helper for checking the
system capability. Add a safe helper for hypervisors to check a constant
system capability, which *doesn't* fall back to checking the bitmap
maintained by the kernel.

Cc: Marc Zyngier <marc.zyngier@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
---
 arch/arm64/include/asm/cpufeature.h | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h
index 758d74f..ae5e994 100644
--- a/arch/arm64/include/asm/cpufeature.h
+++ b/arch/arm64/include/asm/cpufeature.h
@@ -9,8 +9,6 @@
 #ifndef __ASM_CPUFEATURE_H
 #define __ASM_CPUFEATURE_H
 
-#include <linux/jump_label.h>
-
 #include <asm/hwcap.h>
 #include <asm/sysreg.h>
 
@@ -45,6 +43,8 @@
 
 #ifndef __ASSEMBLY__
 
+#include <linux/bug.h>
+#include <linux/jump_label.h>
 #include <linux/kernel.h>
 
 /* CPU feature register tracking */
@@ -122,6 +122,16 @@ static inline bool cpu_have_feature(unsigned int num)
 	return elf_hwcap & (1UL << num);
 }
 
+/* System capability check for constant caps */
+static inline bool cpus_have_const_cap(int num)
+{
+	if (__builtin_constant_p(num))
+		return static_branch_unlikely(&cpu_hwcap_keys[num]);
+	BUILD_BUG();
+	/* unreachable */
+	return false;
+}
+
 static inline bool cpus_have_cap(unsigned int num)
 {
 	if (num >= ARM64_NCAPS)
@@ -218,7 +228,7 @@ static inline bool cpu_supports_mixed_endian_el0(void)
 
 static inline bool system_supports_32bit_el0(void)
 {
-	return cpus_have_cap(ARM64_HAS_32BIT_EL0);
+	return cpus_have_const_cap(ARM64_HAS_32BIT_EL0);
 }
 
 static inline bool system_supports_mixed_endian_el0(void)
-- 
2.7.4

^ permalink raw reply related

* [PATCH 0/2] arm64: Support systems without FP/ASIMD
From: Suzuki K Poulose @ 2016-10-25 13:50 UTC (permalink / raw)
  To: linux-arm-kernel

This series adds supports to the kernel and KVM hyp to handle
systems without FP/ASIMD properly. At the moment the kernel
doesn't check if the FP unit is available before accessing
the registers (e.g during context switch). Also for KVM,
we trap the FP/ASIMD accesses and handle it by injecting an
undefined instruction into the VM on systems without FP.

Tested on a FVP_Base-AEM-v8A model by disabling VFP on at
least one CPU ( -C clusterX.cpuY.vfp-present=0 ).

Suzuki K Poulose (2):
  arm64: Add hypervisor safe helper for checking constant capabilities
  arm64: Support systems without FP/ASIMD

 arch/arm64/crypto/aes-ce-ccm-glue.c |  2 +-
 arch/arm64/crypto/aes-ce-cipher.c   |  2 ++
 arch/arm64/crypto/ghash-ce-glue.c   |  2 ++
 arch/arm64/crypto/sha1-ce-glue.c    |  2 ++
 arch/arm64/include/asm/cpufeature.h | 24 ++++++++++++++++++++----
 arch/arm64/include/asm/neon.h       |  3 ++-
 arch/arm64/kernel/cpufeature.c      | 15 +++++++++++++++
 arch/arm64/kernel/fpsimd.c          | 14 ++++++++++++++
 arch/arm64/kvm/handle_exit.c        | 11 +++++++++++
 arch/arm64/kvm/hyp/hyp-entry.S      |  9 ++++++++-
 arch/arm64/kvm/hyp/switch.c         |  5 ++++-
 11 files changed, 81 insertions(+), 8 deletions(-)

-- 
2.7.4

^ permalink raw reply

* [PATCH] ARM: sti: stih410-clocks: Add PROC_STFE as a critical clock
From: Peter Griffin @ 2016-10-25 13:49 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161025100141.GH8574@dell>

Hi Lee,

On Tue, 25 Oct 2016, Lee Jones wrote:

> On Tue, 25 Oct 2016, Peter Griffin wrote:
> 
> > Hi Lee,
> > 
> > On Tue, 25 Oct 2016, Lee Jones wrote:
> > 
> > > On Mon, 24 Oct 2016, Peter Griffin wrote:
> > > 
> > > > Hi Lee,
> > > > 
> > > > On Mon, 24 Oct 2016, Lee Jones wrote:
> > > > > On Tue, 18 Oct 2016, Peter Griffin wrote:
> > > > > 
> > > > > > Once the ST frontend demux HW IP has been enabled, the clock can't
> > > > > > be disabled otherwise the system will hang and the board will
> > > > > > be unserviceable.
> > > > > > 
> > > > > > To allow balanced clock enable/disable calls in the driver we use
> > > > > > the critical clock infrastructure to take an extra reference on the
> > > > > > clock so the clock will never actually be disabled.
> > > > > 
> > > > > This is an abuse of the critical-clocks framework, and is exactly the
> > > > > type of hack I promised the clk guys I'd try to prevent.
> > > > 
> > > > I expect the best way to do this would be to write some documentation on the
> > > > clock-critical DT binding and/or CRITICAL_CLK flag. The only documentation I can
> > > > find currently is with the initial patch series [1] and the comment in
> > > > clk-provider.h of
> > > > 
> > > >  #define CLK_IS_CRITICAL         BIT(11) /* do not gate, ever */
> > > > 
> > > > Or the patch decription
> > > > 
> > > > "Critical clocks are those which must not be gated, else undefined
> > > > or catastrophic failure would occur.  Here we have chosen to
> > > > ensure the prepare/enable counts are correctly incremented, so as
> > > > not to confuse users with enabled clocks with no visible users."
> > > > 
> > > > Which is the functionality I want for this clock.
> > > 
> > > No, that's not the functionality you want.
> > 
> > Yes it is :)
> 
> No, it's the functionally that is most convenient.

Nope, the solution you proposed already exists upstream so changing it is not
convienient, it is IMO the correct thing to do and the current upstream solution
is the hack.

Allowing the common CCF to disable this clock during clk_disable_unused is
wrong, and can lead to catastrophic failure which requires a reboot.

IMO Linux needs to be resilient to whatever state the hardware could be left in
by previous software. It can't assume it will be OK to disable this clock, we
*know* there is a HW bug, we *know* that disabling the clock can brick the
board.

So I stick with my initial assertion, if this clock is enabled when Linux is
booted it *should not* be disabled.

> 
> > >  You want for the clock not
> > > to be RE-gated (big difference).  Currently, the STFE clock will never
> > > be gated, even when a) it's not used and b) can actually be disabled.
> > > You're needlessly wasting power here.
> > 
> > IMO it is *never* safe for Linux to gate this clock, as you have no idea
> > on the state of the hardware from the primary and secondary bootloaders.
> 
> You can say that with any of the clocks on this platform.

This is a odd thing to say. I'm not aware of any other clocks which
will render the platform unusable that aren't already listed as critical for the
STi platform.

> 
> > If the clock is enabled when Linux boots, the Linux clock framework *needs*
> > to assume the hardware may have been used in previous boot stages, and it should
> > not attempt to disable the clock.
> 
> None of the boot loaders we use do this.

But the Linux kernel isn't just used by us. It is not uncommon for STB
bootloaders to get information from the frontend as part of the boot process. 

> 
> I have never seen the STFE clock crash a platform.

I have, it leads to the same behaviour as turning off any of the other critical
clocks, the SoC is dead and the board needs to be rebooted.

> 
> > > Also, in your use-case there is a visible user, and the prepare/enable
> > > counts will be correct.
> > 
> > Your correct there is a visible user, but this is the same as CLK_EXT2F_A9 where
> > there are multiple users of the clock (SPI, I2C, UART etc).
> 
> Right, but their clocks can not be turned off, ever.  So all the boxes
> are ticked for criticalness.  Not the case with your clock.

This clock also should never be turned off by Linux..ever. It is unsafe to do so.

Incidentally this isn't the only platform upstream where the requirement for a
clocks criticalness is dependent on whether it was enabled during boot. Take a
look in bcm/clk-bcm2835.c

                /* If the clock wasn't actually enabled at boot, it's not
                 * critical.
                 */
                if (!(cprman_read(cprman, data->ctl_reg) & CM_ENABLE))
                        init.flags &= ~CLK_IS_CRITICAL;


> 
> > > > > If this, or
> > > > > any other IP has some quirks (i.e. once enabled, if this clock is
> > > > > subsequently disabled it will have a catastrophic effect on the
> > > > > platform), then they should be worked around in the driver.
> > > > > 
> > > > > The correct thing to do here is craft a clk-keep-on flag and ensure it
> > > > > is set to true for the effected platform(s)' platform data.
> > > > 
> > > > I'm always wary of creating a driver specific flag, especially when its
> > > > purpose is to do the same thing as an existing mechanism provided by the
> > > > subsystem of not gating the clock.
> > > 
> > > Using existing sub-system supplied mechanisms in the way they were not
> > > intended is sub-optimal (read "hacky").
> > 
> > I think the scope of this flag has been defined in a very narrow way, which is
> > making you want to suggest lots of hacks in the client driver.
> 
> The scope was narrowed intentionally, buy the maintainers.  I am
> merely reflecting their views.

What is your recollection of their views? LIke I said before this should be
documented somewhere.

>  If you really wish to contend these,
> you should take it up with them.
> 
> > > > I can see a couple of problems with what you propose:
> > > > 
> > > > 1) You have to put the clk-keep-on flag in every driver which consumes the
> > > > clock. IMO it is much better to have this knowledge in the SoC's
> > > > clock driver so every consumer of the clock automatically benefits.
> > > 
> > > That would also be fine(ish).  The issue is that this problem is
> > > board specific, so the platform clock driver would have to contain
> > > board level knowledge.
> > 
> > It is not board specific. It is a SoC HW bug, so by definition present on all
> > boards which have this SoC.
> 
> Okay SoC specific.  Is there a SoC specific clock driver?

STi platform clocks are mainly described in DT in stih407/10-clock.dsti, which is SoC
specific, and binds with code in drivers/clk/st. So the marking of a clock as
critical is done on a SoC basis which is what I want.

A quick grep of CLK_IGNORE_UNUSED or CLK_IS_CRITICAL, will show you where the
equivalent code for other platforms is located.

> 
> > >  Also, if you were to implement this, it would
> > > too mess up reference counting in the core.
> 
> This still stands.

I don't understand what you mean here. The point of using CCF and the critical
flag is so the reference counting of the clock is held at 1.

> 
> > > > 2) You don't benefit from the CLK_IS_CRITICAL reference counting logic in
> > > > clk.c. So then each driver has to also work around that to get sensible reference
> > > > counts. e.g.
> > > > 
> > > > if (!__clk_is_enabled(clk) && pdata->clk-keep-on)
> > > >    clk_enable(clk)
> > > > 
> > > > Which seems to me to be fighting against the subsystem. Given that the only use of
> > > >  _clk_is_enabled() outside drivers/clk is in an old arch/arm/mach-omap2/pm24xx.c
> > > > driver I suspect its use is frowned upon, and it shouldn't really be an EXPORTED_SYMBOL.
> > > 
> > > In this instance, since the STFE clock is only used by this IP, I
> > > would choose to handle it in the driver.
> > 
> > There are other IPs within this IP block for which upstream drivers don't yet exist.
> 
> If they are many and various, we can discuss alternative solutions.
> 
> What are they?

You have hardware for Merged Input Blocks, SWTS, TSDMA, Cable Card Stream
Convertor and Tag Counter which is currently unsupported upstream.

> 
> > >  This can be done using a
> > > single flag stored in pdata which should be fetched using
> > > of_match_device().  This way there is no need for any more API abuse;
> > > either by incorrectly identifying the STFE clock as critical OR
> > > invoking any internal __clk_*() calls.
> > > 
> > > Enable the clock once in .probe(), which you already do.
> > 
> > But these drivers are by default built as modules, so when you rmmod, and insmod the
> > driver you now have an ever increasing clock reference count. This is the
> > problem I was describing in the previous email, and why what you propose is a
> > bad idea.
> 
> Then the variable would have to be exported.

I don't understand why if you had a choice you would choose to put the logic
there over the clock subsystem.

The reasoning is the same as with the I2C & SPI drivers. You don't want to have
to put a platform specific clocking knowledge into each driver, when it
could live centrally in the clock subsystem for the platform. You also don't want to
have to change the driver when a different SoC with the same IP has a different
clock tree.

Not only that CCF *has* to have the knowledge internal to the clock susbsytem so
that it doesn't disable the clock during the clk_disable_unused phase on boot.

> 
> > > Then, whenever you do any power saving do:
> > > 
> > > suspend()
> > > {
> > > 	if (!ddata->enable_clk_once)
> > > 		clk_disable(clk);
> > > }
> > > 
> > > resume()
> > > {
> > > 	if (!ddata->enable_clk_once)
> > > 		clk_enable(clk);
> > > }
> > > 
> > > However, looking at your driver, I think this point might even be
> > > moot, since you don't have any power saving.  The only time you
> > > disable the clock is in the error path.  Just replace that with a
> > > comment about the platform's unfortunate errata.
> > 
> > This is exactly what I want to avoid doing. The driver already has these
> > hacks as it was waiting for the critical clock patches to land so I could remove
> > them and fix the problem properly.
> > 
> > Much like you did with the I2C and SPI drivers, where you removed similar hacks
> > and added the clock to the critical clock list.
> 
> Right, see above.
> 
> > > > [1] https://lkml.org/lkml/2016/1/18/272
> > > 
> > > I'm glad you mentioned this.  Let's take a look:
> > > 
> > > > Some platforms contain clocks which if gated, will cause undefined or
> > > > catastrophic behaviours.  As such they are not to be turned off, ever.
> > > 
> > > Not the case here.
> > > 
> > > This clock *can* be gated and can be turned off *sometimes*.
> > 
> > See above, if the clock is on when Linux boots it can never be assumed that it
> > is safe to disable it.
> 
> See above.
> 
> > > > Many of these such clocks do not have devices, thus device drivers
> > > > where clocks may be enabled and references taken to ensure they stay
> > > > enabled do not exist.  Therefore, we must handle these such cases in
> > > > the core.
> > > 
> > > This clock *does* have a driver and correct references *can* be
> > > taken.
> > 
> > As above this is the same for CLK_EXT2F_A9, which has multiple users in the
> > kernel.
> > 
> > The point is we don't wish to have the knowledge in the individual drivers that
> > this clock is critical and can destroy the system if it is disabled.

This point stlll stands.

> > 
> > > 
> > > [...]
> > > 
> > > All I'm saying is, and it's the same thing I've said many times; these
> > > types of issues do not exhibit the same set of symptoms as a critical
> > > clock by definition.  Critical clocks are those which references can
> > > not be taken by any other means.
> > 
> > That is not how you are using it currently. See CLK_EXT2F_A9 and
> > CLK_TX_ICN_DMU.

So does this one.

> > 
> > >  Think of the critical clock
> > > framework as a mechanism to circumvent the requirement of writing a
> > > special driver which would *only* handle clocks i.e. an interconnect
> > > driver in the ST case.
> > > 
> > 
> > You didn't need a new flag for that. CLK_IGNORE_UNUSED already allowed you to not
> > write a specical driver which only handled for example an interconnect clock and
> > would ensure it wouldn't be gated by the disable_unused logic.
> > 
> > IMO the point of the critical clock stuff, and what CLK_IGNORE_UNUSED *did not* allow
> > you to do is for situations like CLK_EXT2F_A9, CLK_TX_ICN_DMU and PROC_STFE where users
> >  *do* exist in the kernel, but the SoC clock driver has *special* knowledge of the
> > clock tree on the SoC and knows that despite what the drivers are doing with
> > their enable/disable calls the clock should never be disabled.
> > 
> > In fact out of what we currently mark as clock-critical on STi platform most
> > of the clocks could have been handled perfectly fine with the CLK_IGNORE_UNUSED
> > flag (apart from the fact that there is no way to set the flag from DT). It is
> > only CLK_EXT2F_A9 and CLK_TX_ICN_DMU which *do* have kernel users where the
> > extra functionality of critical clocks is required (we need the additional
> > reference taken by the clk framework to avoid the kernel drivers from disabling
> > the clock).

And most importantly this one.

> 
> Probably best to take your special use-case up with the Clock
> Maintainers.  As the author of the critical-clocks patch-set, I would
> say that your use-case does not tick all the boxes for use of the
> critical-clock mechanism, but at the end of the day, it really isn't
> my train-set.
> 

I don't think there is anything special about it. If when Linux
boots the clock is enabled you shouldn't disable it, just like the other
critical clocks.

regards,

Peter.

^ permalink raw reply

* [PATCH v3 3/6] dt-bindings: pinctrl: Deprecate sunxi pinctrl bindings
From: Maxime Ripard @ 2016-10-25 13:41 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CACRpkdYGYPhPZQmgeLA-K-RQKZMLy1qHY6P7LS6SKsFC=d=MHg@mail.gmail.com>

Hi Linus,

On Tue, Oct 25, 2016 at 02:07:27PM +0200, Linus Walleij wrote:
> On Mon, Oct 24, 2016 at 9:49 PM, Maxime Ripard
> <maxime.ripard@free-electrons.com> wrote:
> 
> > However, it looks like the first patch from this serie is missing from
> > your tree, is there a reason for that?
> 
> No can you point it out?

Sure:
http://lists.infradead.org/pipermail/linux-arm-kernel/2016-October/462500.html

> > Also, in order to preserve bisectability, could you create an
> > immutable branch for those sunxi patches so that I can merge the DT
> > bits?
> 
> It's too late because they are already in the devel branch
> and mixed up with merged of *other* immutable stuff.

Hmmmm, ok.

> However I think it is plain wrong to try to keep any bisectability
> between the kernel at large and arch/*/boot/dts/*, because
> the DTS stuff is supposed to at some point be maintained outside
> of the kernel and for all OSes, they simply shouldn't be sync:ed.

Yes, in the case of a new driver that needs to be introduced, I
definitely get your point.

However, during a conversion like we're doing here, this is not ideal,
because it essentially means that you will not have a branch that
works, at all.

I'll hold off on those patches until 4.11 then.

Thanks!
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20161025/b74e47e6/attachment-0001.sig>

^ permalink raw reply

* [RFC v2] ARM: memory: da8xx-ddrctl: new driver
From: Bartosz Golaszewski @ 2016-10-25 13:41 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1477402876-22472-1-git-send-email-bgolaszewski@baylibre.com>

Create a new driver for the da8xx DDR2/mDDR controller and implement
support for writing to the Peripheral Bus Burst Priority Register.

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
 .../memory-controllers/ti-da8xx-ddrctl.txt         |  20 +++
 drivers/memory/Kconfig                             |   8 +
 drivers/memory/Makefile                            |   1 +
 drivers/memory/da8xx-ddrctl.c                      | 175 +++++++++++++++++++++
 4 files changed, 204 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/memory-controllers/ti-da8xx-ddrctl.txt
 create mode 100644 drivers/memory/da8xx-ddrctl.c

diff --git a/Documentation/devicetree/bindings/memory-controllers/ti-da8xx-ddrctl.txt b/Documentation/devicetree/bindings/memory-controllers/ti-da8xx-ddrctl.txt
new file mode 100644
index 0000000..7e271dd
--- /dev/null
+++ b/Documentation/devicetree/bindings/memory-controllers/ti-da8xx-ddrctl.txt
@@ -0,0 +1,20 @@
+* Device tree bindings for Texas Instruments da8xx DDR2/mDDR memory controller
+
+The DDR2/mDDR memory controller present on Texas Instruments da8xx SoCs features
+a set of registers which allow to tweak the controller's behavior.
+
+Documentation:
+OMAP-L138 (DA850) - http://www.ti.com/lit/ug/spruh82c/spruh82c.pdf
+
+Required properties:
+
+- compatible:		"ti,da850-ddr-controller" - for da850 SoC based boards
+- reg:			a tuple containing the base address of the memory
+			controller and the size of the memory area to map
+
+Example for da850 shown below.
+
+ddrctl {
+	compatible = "ti,da850-ddr-controller";
+	reg = <0xB0000000 0x100>;
+};
diff --git a/drivers/memory/Kconfig b/drivers/memory/Kconfig
index 4b4c0c3..ec80e35 100644
--- a/drivers/memory/Kconfig
+++ b/drivers/memory/Kconfig
@@ -134,6 +134,14 @@ config MTK_SMI
 	  mainly help enable/disable iommu and control the power domain and
 	  clocks for each local arbiter.
 
+config DA8XX_DDRCTL
+	bool "Texas Instruments da8xx DDR2/mDDR driver"
+	depends on ARCH_DAVINCI_DA8XX
+	help
+	  This driver is for the DDR2/mDDR Memory Controller present on
+	  Texas Instruments da8xx SoCs. It's used to tweak various memory
+	  controller configuration options.
+
 source "drivers/memory/samsung/Kconfig"
 source "drivers/memory/tegra/Kconfig"
 
diff --git a/drivers/memory/Makefile b/drivers/memory/Makefile
index b20ae38..e88097fb 100644
--- a/drivers/memory/Makefile
+++ b/drivers/memory/Makefile
@@ -17,6 +17,7 @@ obj-$(CONFIG_MVEBU_DEVBUS)	+= mvebu-devbus.o
 obj-$(CONFIG_TEGRA20_MC)	+= tegra20-mc.o
 obj-$(CONFIG_JZ4780_NEMC)	+= jz4780-nemc.o
 obj-$(CONFIG_MTK_SMI)		+= mtk-smi.o
+obj-$(CONFIG_DA8XX_DDRCTL)	+= da8xx-ddrctl.o
 
 obj-$(CONFIG_SAMSUNG_MC)	+= samsung/
 obj-$(CONFIG_TEGRA_MC)		+= tegra/
diff --git a/drivers/memory/da8xx-ddrctl.c b/drivers/memory/da8xx-ddrctl.c
new file mode 100644
index 0000000..66022df
--- /dev/null
+++ b/drivers/memory/da8xx-ddrctl.c
@@ -0,0 +1,175 @@
+/*
+ * TI da8xx DDR2/mDDR controller driver
+ *
+ * Copyright (C) 2016 BayLibre SAS
+ *
+ * Author:
+ *   Bartosz Golaszewski <bgolaszewski@baylibre.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/of_fdt.h>
+#include <linux/platform_device.h>
+#include <linux/io.h>
+
+/*
+ * REVISIT: Linux doesn't have a good framework for the kind of performance
+ * knobs this driver controls. We can't use device tree properties as it deals
+ * with hardware configuration rather than description. We also don't want to
+ * commit to maintaining some random sysfs attributes.
+ *
+ * For now we just hardcode the register values for the boards that need
+ * some changes (as is the case for the LCD controller on da850-lcdk - the
+ * first board we support here). When linux gets an appropriate framework,
+ * we'll easily convert the driver to it.
+ */
+
+struct da8xx_ddrctl_config_knob {
+	const char *name;
+	u32 reg;
+	u32 mask;
+	u32 offset;
+};
+
+static const struct da8xx_ddrctl_config_knob da8xx_ddrctl_knobs[] = {
+	{
+		.name = "da850-pbbpr",
+		.reg = 0x20,
+		.mask = 0xffffff00,
+		.offset = 0,
+	},
+};
+
+struct da8xx_ddrctl_setting {
+	const char *name;
+	u32 val;
+};
+
+struct da8xx_ddrctl_board_settings {
+	const char *board;
+	const struct da8xx_ddrctl_setting *settings;
+};
+
+static const struct da8xx_ddrctl_setting da850_lcdk_ddrctl_settings[] = {
+	{
+		.name = "da850-pbbpr",
+		.val = 0x20,
+	},
+	{ }
+};
+
+static const struct da8xx_ddrctl_board_settings da8xx_ddrctl_board_confs[] = {
+	{
+		.board = "ti,da850-lcdk",
+		.settings = da850_lcdk_ddrctl_settings,
+	},
+};
+
+static const struct da8xx_ddrctl_config_knob *
+da8xx_ddrctl_match_knob(const struct da8xx_ddrctl_setting *setting)
+{
+	const struct da8xx_ddrctl_config_knob *knob;
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(da8xx_ddrctl_knobs); i++) {
+		knob = &da8xx_ddrctl_knobs[i];
+
+		if (strcmp(knob->name, setting->name) == 0)
+			return knob;
+	}
+
+	return NULL;
+}
+
+static const struct da8xx_ddrctl_setting *da8xx_ddrctl_get_board_settings(void)
+{
+	const struct da8xx_ddrctl_board_settings *board_settings;
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(da8xx_ddrctl_board_confs); i++) {
+		board_settings = &da8xx_ddrctl_board_confs[0];
+
+		if (of_machine_is_compatible(board_settings->board))
+			return board_settings->settings;
+	}
+
+	return NULL;
+}
+
+static int da8xx_ddrctl_probe(struct platform_device *pdev)
+{
+	const struct da8xx_ddrctl_config_knob *knob;
+	const struct da8xx_ddrctl_setting *setting;
+	struct device_node *node;
+	struct resource *res;
+	void __iomem *ddrctl;
+	struct device *dev;
+	u32 reg;
+
+	dev = &pdev->dev;
+	node = dev->of_node;
+
+	setting = da8xx_ddrctl_get_board_settings();
+	if (!setting) {
+		dev_err(dev, "no settings for board '%s'\n",
+			of_flat_dt_get_machine_name());
+		return -EINVAL;
+	}
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	ddrctl = devm_ioremap_resource(dev, res);
+	if (IS_ERR(ddrctl)) {
+		dev_err(dev, "unable to map memory controller registers\n");
+		return PTR_ERR(ddrctl);
+	}
+
+	for (; setting->name; setting++) {
+		knob = da8xx_ddrctl_match_knob(setting);
+		if (!knob) {
+			dev_warn(dev,
+				 "no such config option: %s\n", setting->name);
+			continue;
+		}
+
+		if (knob->reg > (res->end - res->start - sizeof(u32))) {
+			dev_warn(dev,
+				 "register offset of '%s' exceeds mapped memory size\n",
+				 knob->name);
+			continue;
+		}
+
+		reg = __raw_readl(ddrctl + knob->reg);
+		reg &= knob->mask;
+		reg |= setting->val << knob->offset;
+
+		dev_dbg(dev, "writing 0x%08x to %s\n", reg, setting->name);
+
+		__raw_writel(reg, ddrctl + knob->reg);
+	}
+
+	return 0;
+}
+
+static const struct of_device_id da8xx_ddrctl_of_match[] = {
+	{ .compatible = "ti,da850-ddr-controller", },
+	{ },
+};
+
+static struct platform_driver da8xx_ddrctl_driver = {
+	.probe = da8xx_ddrctl_probe,
+	.driver = {
+		.name = "da850-ddr-controller",
+		.of_match_table = da8xx_ddrctl_of_match,
+	},
+};
+module_platform_driver(da8xx_ddrctl_driver);
+
+MODULE_AUTHOR("Bartosz Golaszewski <bgolaszewski@baylibre.com>");
+MODULE_DESCRIPTION("TI da8xx DDR2/mDDR controller driver");
+MODULE_LICENSE("GPL v2");
-- 
2.9.3

^ permalink raw reply related

* [RFC v2] da850: DDR2/mDDR memory controller driver
From: Bartosz Golaszewski @ 2016-10-25 13:41 UTC (permalink / raw)
  To: linux-arm-kernel

This is a follow-up for the series[1] adding new bus and memory drivers
to better support the TI LCD controller on the da850-lcdk board.

The general consensus of the discussion that followed was that DT is
not the right tool for this kind of SoC performance tweaks.

In order to avoid committing to stable DT bindings, we only introduce
two common properties (compatible and reg) while the configuration
register values are hard-coded for each board (currently only lcdk).

In the future, once linux gets a proper framework for performance
knobs, we'll convert this driver to using the better solution.

I'm sending a single patch this time as RFC to get some reviews and
see it the approach is viewed as correct.

[1] https://lkml.org/lkml/2016/10/17/613

v1 -> v2:
- changed the compatible string to make it more descriptive
- changed the DT bindings description to describe the device, not the
  driver's functionalities
- switched to using of_machine_is_compatible() instead of handcoding
  the same functionality
- used platform_get_resource() instead of ioremapping registers by hand

Bartosz Golaszewski (1):
  ARM: memory: da8xx-ddrctl: new driver

 .../memory-controllers/ti-da8xx-ddrctl.txt         |  20 +++
 drivers/memory/Kconfig                             |   8 +
 drivers/memory/Makefile                            |   1 +
 drivers/memory/da8xx-ddrctl.c                      | 175 +++++++++++++++++++++
 4 files changed, 204 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/memory-controllers/ti-da8xx-ddrctl.txt
 create mode 100644 drivers/memory/da8xx-ddrctl.c

-- 
2.9.3

^ permalink raw reply

* [PATCH 4/9] pinctrl: meson: allow gpio to request irq
From: Marc Zyngier @ 2016-10-25 13:38 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1477400900.2482.51.camel@baylibre.com>

On 25/10/16 14:08, Jerome Brunet wrote:
> On Tue, 2016-10-25 at 11:38 +0100, Marc Zyngier wrote:
>>>
>> On 25/10/16 10:14, Linus Walleij wrote:
>>>
>>> On Fri, Oct 21, 2016 at 11:06 AM, Jerome Brunet <jbrunet@baylibre.c
>>> om> wrote:
>>>
>>>>
>>>>>
>>>>> Isn't this usecase (also as described in the cover letter) a
>>>>> textbook
>>>>> example of when you should be using hierarchical irqdomain?
>>>>>
>>>>> Please check with Marc et al on hierarchical irqdomains.
>>>>
>>>> Linus,
>>>> Do you mean I should create a new hierarchical irqdomains in each
>>>> of
>>>> the two pinctrl instances we have in these SoC, these domains
>>>> being
>>>> stacked on the one I just added for controller in irqchip ?
>>>>
>>>> I did not understand this is what you meant when I asked you the
>>>> question at ELCE.
>>>
>>> Honestly, I do not understand when and where to properly use
>>> hierarchical irqdomain, even after Marc's talk at ELC-E.
>>
>> I probably didn't do that good a job explaining it then. Let's try
>> again. You want to use hierarchical domains when you want to describe
>> an
>> interrupt whose path traverses multiple controllers without ever
>> being
>> multiplexed with other signals. As long as you have this 1:1
>> relationship between controllers, you can use them.
>>
> 
> Linus, Marc,
> 
> The calculation is question here is meant to get the appropriate hwirq
> number from a particular gpio (and deal with the gpios that can't
> provide an irq at all). 
> 
> If I look at other gpio drivers, many are doing exactly this kind of
> calculation before calling 'irq_create_mapping' in the to_irq callback.
> For example:
> - pinctrl/nomadik/pinctrl-abx500.c
> - pinctrl/samsung/pinctrl-exynos5440.c
> 
> Some can afford to create all the mappings in the probe and just call
> 'irq_find_mapping' (gpio/gpio_tegra.c) but this would not work here. We
> have only 8 upstream irqs for 130+ pins, so only 8 mappings possible at
> a time. 
> 
> My understanding is that irqdomain provide a way to map hwirq to linux
> virq (and back), not map gpio number to hwirq, right?

But why are those number different? Why don't you use the same
namespace? If gpio == hwirq, all your problems are already solved. If
you don't find the mapping in the irqdomain, then there is no irq, end
of story. What am I missing?

> 
> Even if I implement an another irqdomain at the gpio level, I would
> still have to perform this kind of calculation, one way or the other.
> 
>>> Which is problematic since quite a few GPIO drivers now
>>> need to use them.
>>>
>>> I will review his slides, in the meantime I would say: whatever
>>> Marc ACKs is fine with me. I trust this guy 100%. So I guess I
>>> could ask that he ACK the entire chain of patches
>>> from GIC->specialchip->GPIO.
> 
> Actually this discussion go me thinking about another issue we have
> with this hardware.
> We are looking for a way to implement support for IRQ_TYPE_EDGE_BOTH
> (needed for things like gpio-keys or mmc card detect). 
> The controller can do each edge but not both at the same time.
> I'm thinking that implementing another irqdomain at the gpio level
> would allow to properly check the pad level in the EOI callback then
> set the next expected edge type accordingly (using
> 'irq_chip_set_type_parent')
> 
> Would it be acceptable ?

I really don't see what another irqdomain brings to the table. This is
not a separate piece of HW, so the hwirq:irq mapping is still the same.
I fail to see what the benefit is.

> It looks a few other drivers deal with IRQ_TYPE_EDGE_BOTH in a similar
> way (gpio/gpio-omap.c, gpio/gpio-dwapb.c)

Being already done doesn't make it reliable. If the line goes low
between latching the rising edge and reprogramming the trigger, you've
lost at least *two* interrupts (the falling edge and the following
rising edge).

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...

^ permalink raw reply

* [PATCH] irqchip/ls-scfg-msi: update Layerscape SCFG MSI driver
From: Mark Rutland @ 2016-10-25 13:28 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1477399162-22881-1-git-send-email-Minghuan.Lian@nxp.com>

On Tue, Oct 25, 2016 at 08:39:22PM +0800, Minghuan Lian wrote:
> 1. The patch uses soc_device_match() to match the SoC family
> and revision instead of DTS compatible, because compatible cannot
> describe the SoC revision information.

What difference do you care about? If it affects the programming model
of the device, it should be described in the compatible string, or in
properties on the device node.

Matching the SoC is a bit aof a warning sign. If there's information
that we need, but don't currently have, please extend the binding to
provide it.

> +struct ls_scfg_msi_cfg {
> +	u32 ibs_shift; /* Shift of interrupt bit select */
> +	u32 msir_irqs; /* The irq number per MSIR */
> +	u32 msir_base; /* The base address of MSIR */
> +};

> +static struct ls_scfg_msi_cfg ls1021_msi_cfg = {
> +	.ibs_shift = 3,
> +	.msir_irqs = IRQS_32_PER_MSIR,
> +	.msir_base = 0x4,
> +};
> +
> +static struct ls_scfg_msi_cfg ls1043_rev11_msi_cfg = {
> +	.ibs_shift = 2,
> +	.msir_irqs = IRQS_8_PER_MSIR,
> +	.msir_base = 0x10,
> +};
> +
> +static struct ls_scfg_msi_cfg ls1046_msi_cfg = {
> +	.ibs_shift = 2,
> +	.msir_irqs = IRQS_32_PER_MSIR,
> +	.msir_base = 0x4,
> +};
> +
> +static struct soc_device_attribute soc_msi_matches[] = {
> +	{ .family = "QorIQ LS1021A",
> +	  .data = &ls1021_msi_cfg },
> +	{ .family = "QorIQ LS1012A",
> +	  .data = &ls1021_msi_cfg },
> +	{ .family = "QorIQ LS1043A", .revision = "1.0",
> +	  .data = &ls1021_msi_cfg },
> +	{ .family = "QorIQ LS1043A", .revision = "1.1",
> +	  .data = &ls1043_rev11_msi_cfg },
> +	{ .family = "QorIQ LS1046A",
> +	  .data = &ls1046_msi_cfg },
> +	{ },

If the base address of a register differs in this manner, then these
aren't "compatible", and should have separate strings.

Either that, or the binding should mandate a property to describe this.

You can *add* a new string, for which that property is mandatory, if you
believe you will see greate variation here in future.

>  static const struct of_device_id ls_scfg_msi_id[] = {
> -	{ .compatible = "fsl,1s1021a-msi", },
> -	{ .compatible = "fsl,1s1043a-msi", },
> +	{ .compatible = "fsl,ls-scfg-msi" },
>  	{},

NAK. Breaking support for existing DTBs is not ok.

Thanks,
Mark.

^ permalink raw reply

* [PATCH] nvmem: sunxi-sid: SID content is not a valid source of randomness
From: Maxime Ripard @ 2016-10-25 13:26 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161025053855.GA901@Red>

On Tue, Oct 25, 2016 at 07:38:55AM +0200, LABBE Corentin wrote:
> On Mon, Oct 24, 2016 at 10:10:20PM +0200, Maxime Ripard wrote:
> > On Sat, Oct 22, 2016 at 03:53:28PM +0200, Corentin Labbe wrote:
> > > Since SID's content is constant over reboot,
> > 
> > That's not true, at least not across all the Allwinner SoCs, and
> > especially not on the A10 and A20 that this driver supports.
> > 
> 
> On my cubieboard2 (A20)
> hexdump -C /sys/devices/platform/soc\@01c00000/1c23800.eeprom/sunxi-sid0/nvmem 
> 00000000  16 51 66 83 80 48 50 72  56 54 48 48 03 c2 75 72  |.Qf..HPrVTHH..ur|
> 00000010  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
> *
> 00000100  16 51 66 83 80 48 50 72  56 54 48 48 03 c2 75 72  |.Qf..HPrVTHH..ur|
> 00000110  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
> *
> 00000200
> cubiedev ~ # reboot
> cubiedev ~ # hexdump -C /sys/devices/platform/soc\@01c00000/1c23800.eeprom/sunxi-sid0/nvmem 
> 00000000  16 51 66 83 80 48 50 72  56 54 48 48 03 c2 75 72  |.Qf..HPrVTHH..ur|
> 00000010  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
> *
> 00000100  16 51 66 83 80 48 50 72  56 54 48 48 03 c2 75 72  |.Qf..HPrVTHH..ur|
> 00000110  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
> *
> 00000200
> 
> So clearly for me its constant.

It's constant across reboots, but not across devices. Each device have
a different SID content, therefore it's a relevant source of entropy
in the system.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20161025/bf2671a3/attachment-0001.sig>

^ permalink raw reply

* [PATCH 1/2] mm/memblock: prepare a capability to support memblock near alloc
From: Michal Hocko @ 2016-10-25 13:23 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1477364358-10620-2-git-send-email-thunder.leizhen@huawei.com>

On Tue 25-10-16 10:59:17, Zhen Lei wrote:
> If HAVE_MEMORYLESS_NODES is selected, and some memoryless numa nodes are
> actually exist. The percpu variable areas and numa control blocks of that
> memoryless numa nodes need to be allocated from the nearest available
> node to improve performance.
> 
> Although memblock_alloc_try_nid and memblock_virt_alloc_try_nid try the
> specified nid at the first time, but if that allocation failed it will
> directly drop to use NUMA_NO_NODE. This mean any nodes maybe possible at
> the second time.
> 
> To compatible the above old scene, I use a marco node_distance_ready to
> control it. By default, the marco node_distance_ready is not defined in
> any platforms, the above mentioned functions will work as normal as
> before. Otherwise, they will try the nearest node first.

I am sorry but it is absolutely unclear to me _what_ is the motivation
of the patch. Is this a performance optimization, correctness issue or
something else? Could you please restate what is the problem, why do you
think it has to be fixed at memblock layer and describe what the actual
fix is please?

>From a quick glance you are trying to bend over the memblock API for
something that should be handled on a different layer.

> 
> Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
> ---
>  mm/memblock.c | 76 ++++++++++++++++++++++++++++++++++++++++++++++++++---------
>  1 file changed, 65 insertions(+), 11 deletions(-)
> 
> diff --git a/mm/memblock.c b/mm/memblock.c
> index 7608bc3..556bbd2 100644
> --- a/mm/memblock.c
> +++ b/mm/memblock.c
> @@ -1213,9 +1213,71 @@ phys_addr_t __init memblock_alloc(phys_addr_t size, phys_addr_t align)
>  	return memblock_alloc_base(size, align, MEMBLOCK_ALLOC_ACCESSIBLE);
>  }
> 
> +#ifndef node_distance_ready
> +#define node_distance_ready()		0
> +#endif
> +
> +static phys_addr_t __init memblock_alloc_near_nid(phys_addr_t size,
> +					phys_addr_t align, phys_addr_t start,
> +					phys_addr_t end, int nid, ulong flags,
> +					int alloc_func_type)
> +{
> +	int nnid, round = 0;
> +	u64 pa;
> +	DECLARE_BITMAP(nodes_map, MAX_NUMNODES);
> +
> +	bitmap_zero(nodes_map, MAX_NUMNODES);
> +
> +again:
> +	/*
> +	 * There are total 4 cases:
> +	 * <nid == NUMA_NO_NODE>
> +	 *   1)2) node_distance_ready || !node_distance_ready
> +	 *	Round 1, nnid = nid = NUMA_NO_NODE;
> +	 * <nid != NUMA_NO_NODE>
> +	 *   3) !node_distance_ready
> +	 *	Round 1, nnid = nid;
> +	 *    ::Round 2, currently only applicable for alloc_func_type = <0>
> +	 *	Round 2, nnid = NUMA_NO_NODE;
> +	 *   4) node_distance_ready
> +	 *	Round 1, LOCAL_DISTANCE, nnid = nid;
> +	 *	Round ?, nnid = nearest nid;
> +	 */
> +	if (!node_distance_ready() || (nid == NUMA_NO_NODE)) {
> +		nnid = (++round == 1) ? nid : NUMA_NO_NODE;
> +	} else {
> +		int i, distance = INT_MAX;
> +
> +		for_each_clear_bit(i, nodes_map, MAX_NUMNODES)
> +			if (node_distance(nid, i) < distance) {
> +				nnid = i;
> +				distance = node_distance(nid, i);
> +			}
> +	}
> +
> +	switch (alloc_func_type) {
> +	case 0:
> +		pa = memblock_find_in_range_node(size, align, start, end, nnid, flags);
> +		break;
> +
> +	case 1:
> +	default:
> +		pa = memblock_alloc_nid(size, align, nnid);
> +		if (!node_distance_ready())
> +			return pa;
> +	}
> +
> +	if (!pa && (nnid != NUMA_NO_NODE)) {
> +		bitmap_set(nodes_map, nnid, 1);
> +		goto again;
> +	}
> +
> +	return pa;
> +}
> +
>  phys_addr_t __init memblock_alloc_try_nid(phys_addr_t size, phys_addr_t align, int nid)
>  {
> -	phys_addr_t res = memblock_alloc_nid(size, align, nid);
> +	phys_addr_t res = memblock_alloc_near_nid(size, align, 0, 0, nid, 0, 1);
> 
>  	if (res)
>  		return res;
> @@ -1276,19 +1338,11 @@ static void * __init memblock_virt_alloc_internal(
>  		max_addr = memblock.current_limit;
> 
>  again:
> -	alloc = memblock_find_in_range_node(size, align, min_addr, max_addr,
> -					    nid, flags);
> +	alloc = memblock_alloc_near_nid(size, align, min_addr, max_addr,
> +					    nid, flags, 0);
>  	if (alloc)
>  		goto done;
> 
> -	if (nid != NUMA_NO_NODE) {
> -		alloc = memblock_find_in_range_node(size, align, min_addr,
> -						    max_addr, NUMA_NO_NODE,
> -						    flags);
> -		if (alloc)
> -			goto done;
> -	}
> -
>  	if (min_addr) {
>  		min_addr = 0;
>  		goto again;
> --
> 2.5.0
> 

-- 
Michal Hocko
SUSE Labs

^ permalink raw reply

* [PATCH] irqchip/ls-scfg-msi: update Layerscape SCFG MSI driver
From: Marc Zyngier @ 2016-10-25 13:11 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1477399162-22881-1-git-send-email-Minghuan.Lian@nxp.com>

On 25/10/16 13:39, Minghuan Lian wrote:
> 1. The patch uses soc_device_match() to match the SoC family
> and revision instead of DTS compatible, because compatible cannot
> describe the SoC revision information.
> 2. The patch provides a new method to support Layerscape
> SCFG MSI. It tries to assign a dedicated MSIR to every core.
> When changing a MSI interrupt affinity, the MSI message
> data will be changed to refer to a new MSIR that has
> been associated with the core.
> 
> Signed-off-by: Minghuan Lian <Minghuan.Lian@nxp.com>
> ---
> The patch depends on https://patchwork.kernel.org/patch/9342915/

What is the status of this patch? This is the first time I hear about
it, and I can't find it in -rc2.

> 
>  drivers/irqchip/irq-ls-scfg-msi.c | 444 +++++++++++++++++++++++++++++++-------
>  1 file changed, 367 insertions(+), 77 deletions(-)
> 
> diff --git a/drivers/irqchip/irq-ls-scfg-msi.c b/drivers/irqchip/irq-ls-scfg-msi.c
> index 02cca74c..0245d8a 100644
> --- a/drivers/irqchip/irq-ls-scfg-msi.c
> +++ b/drivers/irqchip/irq-ls-scfg-msi.c
> @@ -10,6 +10,7 @@
>   * published by the Free Software Foundation.
>   */
>  
> +#include <linux/bitmap.h>
>  #include <linux/kernel.h>
>  #include <linux/module.h>
>  #include <linux/msi.h>
> @@ -17,23 +18,91 @@
>  #include <linux/irq.h>
>  #include <linux/irqchip/chained_irq.h>
>  #include <linux/irqdomain.h>
> +#include <linux/of_address.h>
> +#include <linux/of_irq.h>
>  #include <linux/of_pci.h>
>  #include <linux/of_platform.h>
>  #include <linux/spinlock.h>
> +#include <linux/sys_soc.h>
>  
> -#define MSI_MAX_IRQS	32
> -#define MSI_IBS_SHIFT	3
> -#define MSIR		4
> +#define LS_MSIR_NUM_MAX		4 /* MSIIR can index 4 MSI registers */
> +#define IRQS_32_PER_MSIR	32
> +#define IRQS_8_PER_MSIR		8
> +
> +#define MSIR_OFFSET(idx)	((idx) * 0x4)
> +
> +enum msi_affinity_flag {
> +	MSI_GROUP_AFFINITY_FLAG,
> +	MSI_AFFINITY_FLAG
> +};
> +
> +struct ls_scfg_msi;
> +struct ls_scfg_msi_ctrl;
> +
> +struct ls_scfg_msi_cfg {
> +	u32 ibs_shift; /* Shift of interrupt bit select */
> +	u32 msir_irqs; /* The irq number per MSIR */
> +	u32 msir_base; /* The base address of MSIR */
> +};
> +
> +struct ls_scfg_msir {
> +	struct ls_scfg_msi_ctrl	*ctrl;
> +	void __iomem		*addr;
> +	int			index;
> +	int			virq;
> +};
> +
> +struct ls_scfg_msi_ctrl {
> +	struct list_head		list;
> +	struct ls_scfg_msi		*msi_data;
> +	void __iomem			*regs;
> +	phys_addr_t			msiir_addr;
> +	enum msi_affinity_flag		flag;
> +	int				irq_base;
> +	spinlock_t			lock;
> +	struct ls_scfg_msir		*msir;
> +	unsigned long			*bm;
> +};
>  
>  struct ls_scfg_msi {
> -	spinlock_t		lock;
> -	struct platform_device	*pdev;
> -	struct irq_domain	*parent;
> -	struct irq_domain	*msi_domain;
> -	void __iomem		*regs;
> -	phys_addr_t		msiir_addr;
> -	int			irq;
> -	DECLARE_BITMAP(used, MSI_MAX_IRQS);
> +	struct platform_device		*pdev;
> +	struct irq_domain		*parent;
> +	struct irq_domain		*msi_domain;
> +	struct list_head		ctrl_list;
> +	const struct ls_scfg_msi_cfg	*cfg;
> +	u32				cpu_num;
> +};
> +
> +static struct ls_scfg_msi_cfg ls1021_msi_cfg = {
> +	.ibs_shift = 3,
> +	.msir_irqs = IRQS_32_PER_MSIR,
> +	.msir_base = 0x4,
> +};
> +
> +static struct ls_scfg_msi_cfg ls1043_rev11_msi_cfg = {
> +	.ibs_shift = 2,
> +	.msir_irqs = IRQS_8_PER_MSIR,
> +	.msir_base = 0x10,
> +};
> +
> +static struct ls_scfg_msi_cfg ls1046_msi_cfg = {
> +	.ibs_shift = 2,
> +	.msir_irqs = IRQS_32_PER_MSIR,
> +	.msir_base = 0x4,
> +};
> +
> +static struct soc_device_attribute soc_msi_matches[] = {
> +	{ .family = "QorIQ LS1021A",
> +	  .data = &ls1021_msi_cfg },
> +	{ .family = "QorIQ LS1012A",
> +	  .data = &ls1021_msi_cfg },
> +	{ .family = "QorIQ LS1043A", .revision = "1.0",
> +	  .data = &ls1021_msi_cfg },
> +	{ .family = "QorIQ LS1043A", .revision = "1.1",
> +	  .data = &ls1043_rev11_msi_cfg },
> +	{ .family = "QorIQ LS1046A",
> +	  .data = &ls1046_msi_cfg },
> +	{ },

Right. So after spending about 5 years getting rid of board files, they
are now back, just spread over a zillion drivers? Why isn't that
described in DT?

>  };
>  
>  static struct irq_chip ls_scfg_msi_irq_chip = {
> @@ -49,19 +118,53 @@ struct ls_scfg_msi {
>  	.chip	= &ls_scfg_msi_irq_chip,
>  };
>  
> +static int ctrl_num;
> +
> +static irqreturn_t (*ls_scfg_msi_irq_handler)(int irq, void *arg);
> +
>  static void ls_scfg_msi_compose_msg(struct irq_data *data, struct msi_msg *msg)
>  {
> -	struct ls_scfg_msi *msi_data = irq_data_get_irq_chip_data(data);
> +	struct ls_scfg_msi_ctrl *ctrl = irq_data_get_irq_chip_data(data);
> +	u32 ibs, srs;
>  
> -	msg->address_hi = upper_32_bits(msi_data->msiir_addr);
> -	msg->address_lo = lower_32_bits(msi_data->msiir_addr);
> -	msg->data = data->hwirq << MSI_IBS_SHIFT;
> +	msg->address_hi = upper_32_bits(ctrl->msiir_addr);
> +	msg->address_lo = lower_32_bits(ctrl->msiir_addr);
> +
> +	ibs = data->hwirq - ctrl->irq_base;
> +
> +	srs = cpumask_first(irq_data_get_affinity_mask(data));
> +	if (srs >= ctrl->msi_data->cpu_num)
> +		srs = 0;
> +
> +	msg->data = ibs << ctrl->msi_data->cfg->ibs_shift | srs;
> +
> +	pr_debug("%s: ibs %d srs %d address0x%x-0x%x data 0x%x\n",
> +		 __func__, ibs, srs, msg->address_hi,
> +		 msg->address_lo, msg->data);
>  }
>  
> -static int ls_scfg_msi_set_affinity(struct irq_data *irq_data,
> -				    const struct cpumask *mask, bool force)
> +static int ls_scfg_msi_set_affinity(struct irq_data *data,
> +				const struct cpumask *mask, bool force)
>  {
> -	return -EINVAL;
> +	struct ls_scfg_msi_ctrl *ctrl = irq_data_get_irq_chip_data(data);
> +	u32 cpu;
> +
> +	if (!force)
> +		cpu = cpumask_any_and(mask, cpu_online_mask);
> +	else
> +		cpu = cpumask_first(mask);
> +
> +	if (cpu >= ctrl->msi_data->cpu_num)
> +		return -EINVAL;
> +
> +	if (ctrl->msir[cpu].virq <= 0) {
> +		pr_warn("cannot bind the irq to cpu%d\n", cpu);
> +		return -EINVAL;
> +	}
> +
> +	cpumask_copy(irq_data_get_affinity_mask(data), mask);
> +
> +	return IRQ_SET_MASK_OK_NOCOPY;
>  }
>  
>  static struct irq_chip ls_scfg_msi_parent_chip = {
> @@ -76,44 +179,57 @@ static int ls_scfg_msi_domain_irq_alloc(struct irq_domain *domain,
>  					void *args)
>  {
>  	struct ls_scfg_msi *msi_data = domain->host_data;
> -	int pos, err = 0;
> +	static struct list_head *current_entry;
> +	struct ls_scfg_msi_ctrl *ctrl;
> +	int i, hwirq = -ENOMEM;
> +
> +	if (!current_entry || current_entry->next == &msi_data->ctrl_list)
> +		current_entry = &msi_data->ctrl_list;
> +
> +	list_for_each_entry(ctrl, current_entry, list) {
> +		spin_lock(&ctrl->lock);
> +		hwirq = bitmap_find_free_region(ctrl->bm,
> +						msi_data->cfg->msir_irqs,
> +						order_base_2(nr_irqs));
> +		spin_unlock(&ctrl->lock);
> +
> +		if (hwirq >= 0)
> +			break;
> +	}
>  
> -	WARN_ON(nr_irqs != 1);
> +	if (hwirq < 0)
> +		return hwirq;
>  
> -	spin_lock(&msi_data->lock);
> -	pos = find_first_zero_bit(msi_data->used, MSI_MAX_IRQS);
> -	if (pos < MSI_MAX_IRQS)
> -		__set_bit(pos, msi_data->used);
> -	else
> -		err = -ENOSPC;
> -	spin_unlock(&msi_data->lock);
> +	hwirq = hwirq + ctrl->irq_base;
>  
> -	if (err)
> -		return err;
> +	for (i = 0; i < nr_irqs; i++)
> +		irq_domain_set_info(domain, virq + i, hwirq + i,
> +				    &ls_scfg_msi_parent_chip, ctrl,
> +				    handle_simple_irq, NULL, NULL);
>  
> -	irq_domain_set_info(domain, virq, pos,
> -			    &ls_scfg_msi_parent_chip, msi_data,
> -			    handle_simple_irq, NULL, NULL);
> +	current_entry = &ctrl->list;
>  
>  	return 0;
>  }
>  
>  static void ls_scfg_msi_domain_irq_free(struct irq_domain *domain,
> -				   unsigned int virq, unsigned int nr_irqs)
> +					unsigned int virq,
> +					unsigned int nr_irqs)
>  {
>  	struct irq_data *d = irq_domain_get_irq_data(domain, virq);
> -	struct ls_scfg_msi *msi_data = irq_data_get_irq_chip_data(d);
> +	struct ls_scfg_msi_ctrl *ctrl = irq_data_get_irq_chip_data(d);
>  	int pos;
>  
> -	pos = d->hwirq;
> -	if (pos < 0 || pos >= MSI_MAX_IRQS) {
> -		pr_err("failed to teardown msi. Invalid hwirq %d\n", pos);
> +	pos = d->hwirq - ctrl->irq_base;
> +
> +	if (pos < 0 || pos >= ctrl->msi_data->cfg->msir_irqs) {
> +		pr_err("Failed to teardown msi. Invalid hwirq %d\n", pos);
>  		return;
>  	}
>  
> -	spin_lock(&msi_data->lock);
> -	__clear_bit(pos, msi_data->used);
> -	spin_unlock(&msi_data->lock);
> +	spin_lock(&ctrl->lock);
> +	bitmap_release_region(ctrl->bm, pos, order_base_2(nr_irqs));
> +	spin_unlock(&ctrl->lock);
>  }
>  
>  static const struct irq_domain_ops ls_scfg_msi_domain_ops = {
> @@ -121,29 +237,198 @@ static void ls_scfg_msi_domain_irq_free(struct irq_domain *domain,
>  	.free	= ls_scfg_msi_domain_irq_free,
>  };
>  
> -static void ls_scfg_msi_irq_handler(struct irq_desc *desc)
> +static irqreturn_t ls_scfg_msi_irqs32_handler(int irq, void *arg)
>  {
> -	struct ls_scfg_msi *msi_data = irq_desc_get_handler_data(desc);
> +	struct ls_scfg_msir *msir = arg;
> +	struct ls_scfg_msi_ctrl *ctrl = msir->ctrl;
> +	struct ls_scfg_msi *msi_data = ctrl->msi_data;
>  	unsigned long val;
> -	int pos, virq;
> +	int pos = 0, hwirq, virq;
> +	irqreturn_t ret = IRQ_NONE;
>  
> -	chained_irq_enter(irq_desc_get_chip(desc), desc);
> +	val = ioread32be(msir->addr);
>  
> -	val = ioread32be(msi_data->regs + MSIR);
> -	for_each_set_bit(pos, &val, MSI_MAX_IRQS) {
> -		virq = irq_find_mapping(msi_data->parent, (31 - pos));
> -		if (virq)
> +	for_each_set_bit(pos, &val, IRQS_32_PER_MSIR) {
> +		hwirq = (IRQS_32_PER_MSIR - 1 - pos) + ctrl->irq_base;
> +		virq = irq_find_mapping(msi_data->parent, hwirq);
> +		if (virq) {
>  			generic_handle_irq(virq);
> +			ret = IRQ_HANDLED;
> +		}
> +	}
> +
> +	return ret;
> +}

NAK. This is not an interrupt handler. This is a flow handler.

> +
> +static irqreturn_t ls_scfg_msi_irqs8_handler(int irq, void *arg)
> +{
> +	struct ls_scfg_msir *msir = arg;
> +	struct ls_scfg_msi_ctrl *ctrl = msir->ctrl;
> +	struct ls_scfg_msi *msi_data = ctrl->msi_data;
> +	unsigned long val;
> +	int pos = 0, hwirq, virq;
> +	irqreturn_t ret = IRQ_NONE;
> +
> +	val = ioread32be(msir->addr);
> +	val = (val << (msir->index * 8)) & 0xff000000;
> +
> +	for_each_set_bit(pos, &val, IRQS_32_PER_MSIR) {
> +		hwirq = (IRQS_32_PER_MSIR - 1 - pos) + ctrl->irq_base;
> +		virq = irq_find_mapping(msi_data->parent, hwirq);
> +		if (virq) {
> +			generic_handle_irq(virq);
> +			ret = IRQ_HANDLED;
> +		}
> +	}
> +
> +	return ret;
> +}
> +
> +static void ls_scfg_msi_cascade(struct irq_desc *desc)
> +{
> +	struct ls_scfg_msir *msir = irq_desc_get_handler_data(desc);
> +	struct irq_chip *chip = irq_desc_get_chip(desc);
> +
> +	chained_irq_enter(chip, desc);
> +	ls_scfg_msi_irq_handler(desc->irq_data.irq, msir);
> +	chained_irq_exit(chip, desc);
> +}

NAK.

> +
> +static int ls_scfg_msi_setup_hwirq(struct ls_scfg_msi_ctrl *ctrl,
> +				   struct device_node *node,
> +				   int index)
> +{
> +	struct ls_scfg_msir *msir = &ctrl->msir[index];
> +	int ret;
> +
> +	msir->virq = of_irq_get(node, index);
> +	if (msir->virq <= 0)
> +		return -ENODEV;
> +
> +	msir->index = index;
> +	msir->ctrl = ctrl;
> +	msir->addr = ctrl->regs + ctrl->msi_data->cfg->msir_base +
> +		     MSIR_OFFSET(msir->index);
> +
> +	if (ctrl->flag == MSI_GROUP_AFFINITY_FLAG) {
> +		ret = request_irq(msir->virq, ls_scfg_msi_irq_handler,
> +				  IRQF_NO_THREAD, "MSI-GROUP", msir);
> +		if (ret) {
> +			pr_err("failed to request irq %d\n", msir->virq);
> +			msir->virq = 0;
> +			return -ENODEV;
> +		}
> +	} else {
> +		irq_set_chained_handler(msir->virq, ls_scfg_msi_cascade);
> +		irq_set_handler_data(msir->virq, msir);
> +		irq_set_affinity(msir->virq, get_cpu_mask(index));
> +	}
> +
> +	return 0;
> +}
> +
> +static void ls_scfg_msi_ctrl_remove(struct ls_scfg_msi_ctrl *ctrl)
> +{
> +	struct ls_scfg_msir *msir;
> +	int i;
> +
> +	if (!ctrl)
> +		return;
> +
> +	if (ctrl->msir) {
> +		for (i = 0; i < ctrl->msi_data->cpu_num; i++) {
> +			msir = &ctrl->msir[i];
> +
> +			if (msir->virq <= 0)
> +				continue;
> +
> +			if (ctrl->flag == MSI_GROUP_AFFINITY_FLAG)
> +				free_irq(msir->virq, msir);
> +			else
> +				irq_set_chained_handler_and_data(msir->virq,
> +								 NULL, NULL);
> +		}
> +
> +		kfree(ctrl->msir);
>  	}
>  
> -	chained_irq_exit(irq_desc_get_chip(desc), desc);
> +	if (ctrl->regs)
> +		iounmap(ctrl->regs);
> +
> +	kfree(ctrl->bm);
> +	kfree(ctrl);
> +}
> +
> +static int ls_scfg_msi_ctrl_probe(struct device_node *node,
> +				  struct ls_scfg_msi *msi_data)
> +{
> +	struct ls_scfg_msi_ctrl *ctrl;
> +	struct resource res;
> +	int err, irqs, i;
> +
> +	err = of_address_to_resource(node, 0, &res);
> +	if (err) {
> +		pr_warn("%s: no regs\n", node->full_name);
> +		return -ENXIO;
> +	}
> +
> +	ctrl = kzalloc(sizeof(*ctrl), GFP_KERNEL);
> +	if (!ctrl)
> +		return  -ENOMEM;
> +
> +	ctrl->msi_data = msi_data;
> +	ctrl->msiir_addr = res.start;
> +	spin_lock_init(&ctrl->lock);
> +
> +	ctrl->regs = ioremap(res.start, resource_size(&res));
> +	if (!ctrl->regs) {
> +		pr_err("%s: unable to map registers\n", node->full_name);
> +		err = -ENOMEM;
> +		goto _err;
> +	}
> +
> +	ctrl->msir = kcalloc(msi_data->cpu_num, sizeof(struct ls_scfg_msir),
> +			     GFP_KERNEL);
> +	if (!ctrl->msir) {
> +		err = -ENOMEM;
> +		goto _err;
> +	}
> +
> +	ctrl->bm = kcalloc(BITS_TO_LONGS(msi_data->cfg->msir_irqs),
> +			   sizeof(long), GFP_KERNEL);
> +	if (!ctrl->bm) {
> +		err = -ENOMEM;
> +		goto _err;
> +	}
> +
> +	ctrl->irq_base = msi_data->cfg->msir_irqs * ctrl_num;
> +	ctrl_num++;
> +
> +	irqs = of_irq_count(node);
> +	if (irqs >= msi_data->cpu_num)
> +		ctrl->flag = MSI_AFFINITY_FLAG;
> +	else
> +		ctrl->flag = MSI_GROUP_AFFINITY_FLAG;
> +
> +	for (i = 0; i < msi_data->cpu_num; i++)
> +		ls_scfg_msi_setup_hwirq(ctrl, node, i);
> +
> +	list_add_tail(&ctrl->list, &msi_data->ctrl_list);
> +
> +	return 0;
> +
> +_err:
> +	ls_scfg_msi_ctrl_remove(ctrl);
> +	pr_err("MSI: failed probing %s (%d)\n", node->full_name, err);
> +	return err;
>  }
>  
>  static int ls_scfg_msi_domains_init(struct ls_scfg_msi *msi_data)
>  {
>  	/* Initialize MSI domain parent */
>  	msi_data->parent = irq_domain_add_linear(NULL,
> -						 MSI_MAX_IRQS,
> +						 msi_data->cfg->msir_irqs *
> +						 ctrl_num,
>  						 &ls_scfg_msi_domain_ops,
>  						 msi_data);
>  	if (!msi_data->parent) {
> @@ -167,51 +452,57 @@ static int ls_scfg_msi_domains_init(struct ls_scfg_msi *msi_data)
>  static int ls_scfg_msi_probe(struct platform_device *pdev)
>  {
>  	struct ls_scfg_msi *msi_data;
> -	struct resource *res;
> -	int ret;
> +	const struct soc_device_attribute *match;
> +	struct device_node *child;
>  
>  	msi_data = devm_kzalloc(&pdev->dev, sizeof(*msi_data), GFP_KERNEL);
>  	if (!msi_data)
>  		return -ENOMEM;
>  
> -	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> -	msi_data->regs = devm_ioremap_resource(&pdev->dev, res);
> -	if (IS_ERR(msi_data->regs)) {
> -		dev_err(&pdev->dev, "failed to initialize 'regs'\n");
> -		return PTR_ERR(msi_data->regs);
> -	}
> -	msi_data->msiir_addr = res->start;
> -
> -	msi_data->irq = platform_get_irq(pdev, 0);
> -	if (msi_data->irq <= 0) {
> -		dev_err(&pdev->dev, "failed to get MSI irq\n");
> -		return -ENODEV;
> -	}
> +	INIT_LIST_HEAD(&msi_data->ctrl_list);
>  
>  	msi_data->pdev = pdev;
> -	spin_lock_init(&msi_data->lock);
> +	msi_data->cpu_num = num_possible_cpus();
> +
> +	match = soc_device_match(soc_msi_matches);
> +	if (match)
> +		msi_data->cfg = match->data;
> +	else
> +		msi_data->cfg = &ls1046_msi_cfg;
> +
> +	if (msi_data->cfg->msir_irqs == IRQS_8_PER_MSIR)
> +		ls_scfg_msi_irq_handler = ls_scfg_msi_irqs8_handler;
> +	else
> +		ls_scfg_msi_irq_handler = ls_scfg_msi_irqs32_handler;
>  
> -	ret = ls_scfg_msi_domains_init(msi_data);
> -	if (ret)
> -		return ret;
> +	for_each_child_of_node(msi_data->pdev->dev.of_node, child)
> +		ls_scfg_msi_ctrl_probe(child, msi_data);
>  
> -	irq_set_chained_handler_and_data(msi_data->irq,
> -					 ls_scfg_msi_irq_handler,
> -					 msi_data);
> +	ls_scfg_msi_domains_init(msi_data);
>  
>  	platform_set_drvdata(pdev, msi_data);
>  
> +	dev_info(&pdev->dev, "irqs:%dx%d ibs_shift:%d msir_base:0x%x\n",
> +		 msi_data->cfg->msir_irqs, ctrl_num,
> +		 msi_data->cfg->ibs_shift, msi_data->cfg->msir_base);
> +
>  	return 0;
>  }
>  
>  static int ls_scfg_msi_remove(struct platform_device *pdev)
>  {
>  	struct ls_scfg_msi *msi_data = platform_get_drvdata(pdev);
> +	struct ls_scfg_msi_ctrl *ctrl, *temp;
>  
> -	irq_set_chained_handler_and_data(msi_data->irq, NULL, NULL);
> +	list_for_each_entry_safe(ctrl, temp, &msi_data->ctrl_list, list) {
> +		list_move_tail(&ctrl->list, &msi_data->ctrl_list);
> +		ls_scfg_msi_ctrl_remove(ctrl);
> +	}
>  
> -	irq_domain_remove(msi_data->msi_domain);
> -	irq_domain_remove(msi_data->parent);
> +	if (msi_data->msi_domain)
> +		irq_domain_remove(msi_data->msi_domain);
> +	if (msi_data->parent)
> +		irq_domain_remove(msi_data->parent);
>  
>  	platform_set_drvdata(pdev, NULL);
>  
> @@ -219,8 +510,7 @@ static int ls_scfg_msi_remove(struct platform_device *pdev)
>  }
>  
>  static const struct of_device_id ls_scfg_msi_id[] = {
> -	{ .compatible = "fsl,1s1021a-msi", },
> -	{ .compatible = "fsl,1s1043a-msi", },
> +	{ .compatible = "fsl,ls-scfg-msi" },

And now you've broken all the previous DTs that previously existed.
What's the plan?

>  	{},
>  };
>  
> 

Overall, I hate this patch. It is broken from an irqchip PoV, it creates
a new range of board files, just even less maintainable, it breaks
compatibility with previous DTs.

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...

^ permalink raw reply

* [PATCH 4/9] pinctrl: meson: allow gpio to request irq
From: Jerome Brunet @ 2016-10-25 13:08 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <ec7c6cc5-cf9a-9c75-4b07-e337064c7d6e@arm.com>

On Tue, 2016-10-25 at 11:38 +0100, Marc Zyngier wrote:
> > 
> On 25/10/16 10:14, Linus Walleij wrote:
> > 
> > On Fri, Oct 21, 2016 at 11:06 AM, Jerome Brunet <jbrunet@baylibre.c
> > om> wrote:
> > 
> > > 
> > > > 
> > > > Isn't this usecase (also as described in the cover letter) a
> > > > textbook
> > > > example of when you should be using hierarchical irqdomain?
> > > > 
> > > > Please check with Marc et al on hierarchical irqdomains.
> > > 
> > > Linus,
> > > Do you mean I should create a new hierarchical irqdomains in each
> > > of
> > > the two pinctrl instances we have in these SoC, these domains
> > > being
> > > stacked on the one I just added for controller in irqchip ?
> > > 
> > > I did not understand this is what you meant when I asked you the
> > > question at ELCE.
> > 
> > Honestly, I do not understand when and where to properly use
> > hierarchical irqdomain, even after Marc's talk at ELC-E.
> 
> I probably didn't do that good a job explaining it then. Let's try
> again. You want to use hierarchical domains when you want to describe
> an
> interrupt whose path traverses multiple controllers without ever
> being
> multiplexed with other signals. As long as you have this 1:1
> relationship between controllers, you can use them.
> 

Linus, Marc,

The calculation is question here is meant to get the appropriate hwirq
number from a particular gpio (and deal with the gpios that can't
provide an irq at all).?

If I look at other gpio drivers, many are doing exactly this kind of
calculation before calling 'irq_create_mapping' in the to_irq callback.
For example:
- pinctrl/nomadik/pinctrl-abx500.c
- pinctrl/samsung/pinctrl-exynos5440.c

Some can afford to create all the mappings in the probe and just call
'irq_find_mapping' (gpio/gpio_tegra.c) but this would not work here. We
have only 8 upstream irqs for 130+ pins, so only 8 mappings possible at
a time.?

My understanding is that irqdomain provide a way to map hwirq to linux
virq (and back), not map gpio number to hwirq, right?

Even if I implement an another irqdomain at the gpio level, I would
still have to perform this kind of calculation, one way or the other.

> > Which is problematic since quite a few GPIO drivers now
> > need to use them.
> > 
> > I will review his slides, in the meantime I would say: whatever
> > Marc ACKs is fine with me. I trust this guy 100%. So I guess I
> > could ask that he ACK the entire chain of patches
> > from GIC->specialchip->GPIO.

Actually this discussion go me thinking about another issue we have
with this hardware.
We are looking for a way to implement support for IRQ_TYPE_EDGE_BOTH
(needed for things like gpio-keys or mmc card detect).?
The controller can do each edge but not both at the same time.
I'm thinking that implementing another irqdomain at the gpio level
would allow to properly check the pad level in the EOI callback then
set the next expected edge type accordingly (using
'irq_chip_set_type_parent')

Would it be acceptable ?

It looks a few other drivers deal with IRQ_TYPE_EDGE_BOTH in a similar
way (gpio/gpio-omap.c, gpio/gpio-dwapb.c)

> Man, I don't even trust myself... ;-)
> 
> Thanks,
> 
> 	M.

Thx
Regards

Jerome

^ permalink raw reply

* [PATCH 1/6] dt/bindings: adjust bindings for Layerscape SCFG MSI
From: Robin Murphy @ 2016-10-25 13:01 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1477398945-22774-1-git-send-email-Minghuan.Lian@nxp.com>

On 25/10/16 13:35, Minghuan Lian wrote:
> 1. The different version of a SoC may have different MSI
> implementation. But compatible "fsl,<soc-name>-msi" can not describe
> the SoC version.

Can't it?

	compatible = "fsl-ls1043a-rev11-msi";

Oh, I guess it can!

Joking aside, if there are multiple versions of a piece of hardware
which require *different* treatment by drivers, then it is obviously
wrong to use the same compatible string because *they are not compatible*.

> The MSI driver will use SoC match interface to get
> SoC type and version instead of compatible string. So all MSI node
> can use the common compatible "fsl,ls-scfg-msi" and the original
> compatible is unnecessary.

If there is some common level of functionality that *all* variants
support without the driver having to know which one is which, then there
might be some sense in having an additional common compatible to
represent that level of functionality, e.g.

	compatible = "fsl-ls1043a-rev11-msi", "fsl,ls-scfg-msi";

But if, say, new variants turn out to have less functionality, rather
than more, then there's probably not much point, and we should stick to
specific, accurate, compatible strings.

DT is not specific to a kernel version, nor even to Linux. A string
which triggers some board-specific magic in a specific version of a
Linux driver does not describe the hardware.

Robin.

> 2. Layerscape SoCs may have one or several MSI controllers.
> In order to increase MSI interrupt number of a PCIe, the patch
> moves all MSI node into the parent node "msi-controller". So a
> PCIe can request MSI from all the MSI controllers.
> 
> Signed-off-by: Minghuan Lian <Minghuan.Lian@nxp.com>
> ---
>  .../interrupt-controller/fsl,ls-scfg-msi.txt       | 57 +++++++++++++++++++---
>  1 file changed, 49 insertions(+), 8 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/interrupt-controller/fsl,ls-scfg-msi.txt b/Documentation/devicetree/bindings/interrupt-controller/fsl,ls-scfg-msi.txt
> index 9e38949..29f95fd 100644
> --- a/Documentation/devicetree/bindings/interrupt-controller/fsl,ls-scfg-msi.txt
> +++ b/Documentation/devicetree/bindings/interrupt-controller/fsl,ls-scfg-msi.txt
> @@ -1,18 +1,28 @@
>  * Freescale Layerscape SCFG PCIe MSI controller
>  
> +Layerscape SoCs may have one or multiple MSI controllers.
> +Each MSI controller must be showed as a child node.
> +
>  Required properties:
>  
> -- compatible: should be "fsl,<soc-name>-msi" to identify
> -	      Layerscape PCIe MSI controller block such as:
> -              "fsl,1s1021a-msi"
> -              "fsl,1s1043a-msi"
> +- compatible: should be "fsl,ls-scfg-msi"
> +- #address-cells: must be 2
> +- #size-cells: must be 2
> +- ranges: allows valid 1:1 translation between child's address space and
> +	  parent's address space
>  - msi-controller: indicates that this is a PCIe MSI controller node
> +
> +Required child node:
> +A child node must exist to represent the MSI controller.
> +The following are properties specific to those nodes:
> +
>  - reg: physical base address of the controller and length of memory mapped.
>  - interrupts: an interrupt to the parent interrupt controller.
>  
>  Optional properties:
>  - interrupt-parent: the phandle to the parent interrupt controller.
>  
> +Notes:
>  This interrupt controller hardware is a second level interrupt controller that
>  is hooked to a parent interrupt controller: e.g: ARM GIC for ARM-based
>  platforms. If interrupt-parent is not provided, the default parent interrupt
> @@ -22,9 +32,40 @@ MSI controller node
>  
>  Examples:
>  
> -	msi1: msi-controller at 1571000 {
> -		compatible = "fsl,1s1043a-msi";
> -		reg = <0x0 0x1571000 0x0 0x8>,
> +	msi: msi-controller {
> +		compatible = "fsl,ls-scfg-msi";
> +		#address-cells = <2>;
> +		#size-cells = <2>;
> +		ranges;
>  		msi-controller;
> -		interrupts = <0 116 0x4>;
> +
> +		msi0 at 1580000 {
> +			reg = <0x0 0x1580000 0x0 0x10000>;
> +			interrupts = <0 116 0x4>,
> +				     <0 111 0x4>,
> +				     <0 112 0x4>,
> +				     <0 113 0x4>;
> +		};
> +
> +		msi1 at 1590000 {
> +			reg = <0x0 0x1590000 0x0 0x10000>;
> +			interrupts = <0 126 0x4>,
> +				     <0 121 0x4>,
> +				     <0 122 0x4>,
> +				     <0 123 0x4>;
> +		};
> +
> +		msi2 at 15a0000 {
> +			reg = <0x0 0x15a0000 0x0 0x10000>;
> +			interrupts = <0 160 0x4>,
> +				     <0 155 0x4>,
> +				     <0 156 0x4>,
> +				     <0 157 0x4>;
> +		};
> +	};
> +
> +	pcie at 3400000 {
> +			...
> +			msi-parent = <&msi>;
> +			...
>  	};
> 

^ permalink raw reply

* [PATCH/RFT v2 09/17] regulator: fixed: Add over current event
From: Axel Haslam @ 2016-10-25 12:55 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161024181937.GP17252@sirena.org.uk>

Hi Mark,

On Mon, Oct 24, 2016 at 8:19 PM, Mark Brown <broonie@kernel.org> wrote:
> On Mon, Oct 24, 2016 at 08:11:40PM +0200, Axel Haslam wrote:
>> On Mon, Oct 24, 2016 at 7:53 PM, Mark Brown <broonie@kernel.org> wrote:
>
>> > does it make sense to report this as a mode, we don't report other error
>> > conditions as modes but instead use REGULATOR_STATUS_ with the
>> > get_status() operation?
>
>> I used mode, because when the regulator toggles the overcurrent
>> line, it means that it has entered a current limited mode, at least the
>> regulator im looking at. ill change to STATUS
>
> That's not what regulator modes are - please look at the documentation
> for the defines here.  They're about the quality of regulation.

To be able to use regulator to handle the overcurrent pin, i need to be able
to somehow retrieve the over current pin state from the regulator driver.

As i was trying your suggestion, i remembered why i thought i should use
mode instead of status: Status seems to be for internal regulator driver use,
there is no regulator_get_status, function and REGULATOR_STATUS_* are defined
in driver.h and not in consumer.h as  REGULATOR_MODE_*

it seems that status is only used to print sysfs info.

Would you be ok if i allow consumers to get the status via a new
"regulator_get_status" call?

Regards
Axel.

^ permalink raw reply

* [PATCH] asm-generic: Drop getrlimit and setrlimit syscalls from default list
From: Stafford Horne @ 2016-10-25 12:51 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CAFiDJ59PdOiqiF8wFVFrkB4k0B6S5cTGVQPUpdjZ=83iv7GQag@mail.gmail.com>



On Tue, 25 Oct 2016, Ley Foon Tan wrote:

> On Mon, Oct 24, 2016 at 5:09 PM, James Hogan <james.hogan@imgtec.com> wrote:
>> On Sat, Oct 22, 2016 at 03:14:04PM +0300, Yury Norov wrote:
>>> The newer prlimit64 syscall provides all the functionality provided by
>>> the getrlimit and setrlimit syscalls and adds the pid of target process,
>>> so future architectures won't need to include getrlimit and setrlimit.
>>>
>>> Therefore drop getrlimit and setrlimit syscalls from the generic syscall
>>> list unless __ARCH_WANT_SET_GET_RLIMIT is defined by the architecture's
>>> unistd.h prior to including asm-generic/unistd.h, and adjust all
>>> architectures using the generic syscall list to define it so that no
>>> in-tree architectures are affected.
>>>
>>> Cc: Vineet Gupta <vgupta@synopsys.com>
>>> Cc: Catalin Marinas <catalin.marinas@arm.com>
>>> Cc: Will Deacon <will.deacon@arm.com>
>>> Cc: Mark Salter <msalter@redhat.com>
>>> Cc: Aurelien Jacquiot <a-jacquiot@ti.com>
>>> Cc: Yoshinori Sato <ysato@users.sourceforge.jp>
>>> Cc: Richard Kuo <rkuo@codeaurora.org>
>>> Cc: James Hogan <james.hogan@imgtec.com>
>>> Cc: Ley Foon Tan <lftan@altera.com>
>>> Cc: Jonas Bonn <jonas@southpole.se>
>>> Cc: Chen Liqin <liqin.linux@gmail.com>
>>> Cc: Lennox Wu <lennox.wu@gmail.com>
>>> Cc: Chris Metcalf <cmetcalf@mellanox.com>
>>> Cc: Guan Xuetao <gxt@mprc.pku.edu.cn>
>>> Cc: Arnd Bergmann <arnd@arndb.de>
>>> Cc: Andrew Pinski <Andrew.Pinski@cavium.com>
>>> Cc: linux-snps-arc at lists.infradead.org
>>> Cc: linux-kernel at vger.kernel.org
>>> Cc: linux-arm-kernel at lists.infradead.org
>>> Cc: linux-c6x-dev at linux-c6x.org
>>> Cc: uclinux-h8-devel at lists.sourceforge.jp
>>> Cc: linux-hexagon at vger.kernel.org
>>> Cc: linux-metag at vger.kernel.org
>>> Cc: nios2-dev at lists.rocketboards.org
>>> Cc: linux-arch at vger.kernel.or
>>> Signed-off-by: Yury Norov <ynorov@caviumnetworks.com>
>>>
>>> ---
>>>  arch/arc/include/uapi/asm/unistd.h       | 1 +
>>>  arch/arm64/include/uapi/asm/unistd.h     | 1 +
>>>  arch/c6x/include/uapi/asm/unistd.h       | 1 +
>>>  arch/h8300/include/uapi/asm/unistd.h     | 1 +
>>>  arch/hexagon/include/uapi/asm/unistd.h   | 1 +
>>>  arch/metag/include/uapi/asm/unistd.h     | 1 +
>>
>> Acked-by: James Hogan <james.hogan@imgtec.com> [metag]
>>
>> Cheers
>> James
>>
>>>  arch/nios2/include/uapi/asm/unistd.h     | 1 +
> Acked-by: Ley Foon Tan <lftan@altera.com> [nios2]
>
>
>>>  arch/openrisc/include/uapi/asm/unistd.h  | 1 +

Acked-by: Stafford Horne <shorne@gmail.com> [openrisc]

>>>  arch/score/include/uapi/asm/unistd.h     | 1 +
>>>  arch/tile/include/uapi/asm/unistd.h      | 1 +
>>>  arch/unicore32/include/uapi/asm/unistd.h | 1 +
>>>  include/uapi/asm-generic/unistd.h        | 5 +++++
>>>  12 files changed, 16 insertions(+)
>>>
>>> diff --git a/arch/arc/include/uapi/asm/unistd.h b/arch/arc/include/uapi/asm/unistd.h
>>> index 41fa2ec..928546d 100644
>>> --- a/arch/arc/include/uapi/asm/unistd.h
>>> +++ b/arch/arc/include/uapi/asm/unistd.h
>>> @@ -16,6 +16,7 @@
>>>  #define _UAPI_ASM_ARC_UNISTD_H
>>>
>>>  #define __ARCH_WANT_RENAMEAT
>>> +#define __ARCH_WANT_SET_GET_RLIMIT
>>>  #define __ARCH_WANT_SYS_EXECVE
>>>  #define __ARCH_WANT_SYS_CLONE
>>>  #define __ARCH_WANT_SYS_VFORK
>>> diff --git a/arch/arm64/include/uapi/asm/unistd.h b/arch/arm64/include/uapi/asm/unistd.h
>>> index 043d17a..48355a6 100644
>>> --- a/arch/arm64/include/uapi/asm/unistd.h
>>> +++ b/arch/arm64/include/uapi/asm/unistd.h
>>> @@ -15,5 +15,6 @@
>>>   */
>>>
>>>  #define __ARCH_WANT_RENAMEAT
>>> +#define __ARCH_WANT_SET_GET_RLIMIT
>>>
>>>  #include <asm-generic/unistd.h>
>>> diff --git a/arch/c6x/include/uapi/asm/unistd.h b/arch/c6x/include/uapi/asm/unistd.h
>>> index 12d73d9..f676231 100644
>>> --- a/arch/c6x/include/uapi/asm/unistd.h
>>> +++ b/arch/c6x/include/uapi/asm/unistd.h
>>> @@ -15,6 +15,7 @@
>>>   */
>>>
>>>  #define __ARCH_WANT_RENAMEAT
>>> +#define __ARCH_WANT_SET_GET_RLIMIT
>>>  #define __ARCH_WANT_SYS_CLONE
>>>
>>>  /* Use the standard ABI for syscalls. */
>>> diff --git a/arch/h8300/include/uapi/asm/unistd.h b/arch/h8300/include/uapi/asm/unistd.h
>>> index 7dd20ef..2f98394 100644
>>> --- a/arch/h8300/include/uapi/asm/unistd.h
>>> +++ b/arch/h8300/include/uapi/asm/unistd.h
>>> @@ -1,5 +1,6 @@
>>>  #define __ARCH_NOMMU
>>>
>>>  #define __ARCH_WANT_RENAMEAT
>>> +#define __ARCH_WANT_SET_GET_RLIMIT
>>>
>>>  #include <asm-generic/unistd.h>
>>> diff --git a/arch/hexagon/include/uapi/asm/unistd.h b/arch/hexagon/include/uapi/asm/unistd.h
>>> index 2151760..52d585c 100644
>>> --- a/arch/hexagon/include/uapi/asm/unistd.h
>>> +++ b/arch/hexagon/include/uapi/asm/unistd.h
>>> @@ -28,6 +28,7 @@
>>>
>>>  #define sys_mmap2 sys_mmap_pgoff
>>>  #define __ARCH_WANT_RENAMEAT
>>> +#define __ARCH_WANT_SET_GET_RLIMIT
>>>  #define __ARCH_WANT_SYS_EXECVE
>>>  #define __ARCH_WANT_SYS_CLONE
>>>  #define __ARCH_WANT_SYS_VFORK
>>> diff --git a/arch/metag/include/uapi/asm/unistd.h b/arch/metag/include/uapi/asm/unistd.h
>>> index 459b6ec..16b5cb3 100644
>>> --- a/arch/metag/include/uapi/asm/unistd.h
>>> +++ b/arch/metag/include/uapi/asm/unistd.h
>>> @@ -8,6 +8,7 @@
>>>   */
>>>
>>>  #define __ARCH_WANT_RENAMEAT
>>> +#define __ARCH_WANT_SET_GET_RLIMIT
>>>
>>>  /* Use the standard ABI for syscalls. */
>>>  #include <asm-generic/unistd.h>
>>> diff --git a/arch/nios2/include/uapi/asm/unistd.h b/arch/nios2/include/uapi/asm/unistd.h
>>> index 51a32c7..b0dda4d 100644
>>> --- a/arch/nios2/include/uapi/asm/unistd.h
>>> +++ b/arch/nios2/include/uapi/asm/unistd.h
>>> @@ -18,6 +18,7 @@
>>>   #define sys_mmap2 sys_mmap_pgoff
>>>
>>>  #define __ARCH_WANT_RENAMEAT
>>> +#define __ARCH_WANT_SET_GET_RLIMIT
>>>
>>>  /* Use the standard ABI for syscalls */
>>>  #include <asm-generic/unistd.h>
>>> diff --git a/arch/openrisc/include/uapi/asm/unistd.h b/arch/openrisc/include/uapi/asm/unistd.h
>>> index 471905b..6812d81 100644
>>> --- a/arch/openrisc/include/uapi/asm/unistd.h
>>> +++ b/arch/openrisc/include/uapi/asm/unistd.h
>>> @@ -21,6 +21,7 @@
>>>  #define sys_mmap2 sys_mmap_pgoff
>>>
>>>  #define __ARCH_WANT_RENAMEAT
>>> +#define __ARCH_WANT_SET_GET_RLIMIT
>>>  #define __ARCH_WANT_SYS_FORK
>>>  #define __ARCH_WANT_SYS_CLONE
>>>
>>> diff --git a/arch/score/include/uapi/asm/unistd.h b/arch/score/include/uapi/asm/unistd.h
>>> index d4008c3..7ad1bdc 100644
>>> --- a/arch/score/include/uapi/asm/unistd.h
>>> +++ b/arch/score/include/uapi/asm/unistd.h
>>> @@ -1,6 +1,7 @@
>>>  #define __ARCH_HAVE_MMU
>>>
>>>  #define __ARCH_WANT_RENAMEAT
>>> +#define __ARCH_WANT_SET_GET_RLIMIT
>>>  #define __ARCH_WANT_SYSCALL_NO_AT
>>>  #define __ARCH_WANT_SYSCALL_NO_FLAGS
>>>  #define __ARCH_WANT_SYSCALL_OFF_T
>>> diff --git a/arch/tile/include/uapi/asm/unistd.h b/arch/tile/include/uapi/asm/unistd.h
>>> index 24e9187..cf0505f 100644
>>> --- a/arch/tile/include/uapi/asm/unistd.h
>>> +++ b/arch/tile/include/uapi/asm/unistd.h
>>> @@ -13,6 +13,7 @@
>>>   */
>>>
>>>  #define __ARCH_WANT_RENAMEAT
>>> +#define __ARCH_WANT_SET_GET_RLIMIT
>>>  #if !defined(__LP64__) || defined(__SYSCALL_COMPAT)
>>>  /* Use the flavor of this syscall that matches the 32-bit API better. */
>>>  #define __ARCH_WANT_SYNC_FILE_RANGE2
>>> diff --git a/arch/unicore32/include/uapi/asm/unistd.h b/arch/unicore32/include/uapi/asm/unistd.h
>>> index 1f63c47..ef25aec 100644
>>> --- a/arch/unicore32/include/uapi/asm/unistd.h
>>> +++ b/arch/unicore32/include/uapi/asm/unistd.h
>>> @@ -11,6 +11,7 @@
>>>   */
>>>
>>>  #define __ARCH_WANT_RENAMEAT
>>> +#define __ARCH_WANT_SET_GET_RLIMIT
>>>
>>>  /* Use the standard ABI for syscalls. */
>>>  #include <asm-generic/unistd.h>
>>> diff --git a/include/uapi/asm-generic/unistd.h b/include/uapi/asm-generic/unistd.h
>>> index 9b1462e..bbaeac0 100644
>>> --- a/include/uapi/asm-generic/unistd.h
>>> +++ b/include/uapi/asm-generic/unistd.h
>>> @@ -465,10 +465,15 @@ __SYSCALL(__NR_uname, sys_newuname)
>>>  __SYSCALL(__NR_sethostname, sys_sethostname)
>>>  #define __NR_setdomainname 162
>>>  __SYSCALL(__NR_setdomainname, sys_setdomainname)
>>> +
>>> +#ifdef __ARCH_WANT_SET_GET_RLIMIT
>>> +/* getrlimit and setrlimit are superseded with prlimit64 */
>>>  #define __NR_getrlimit 163
>>>  __SC_COMP(__NR_getrlimit, sys_getrlimit, compat_sys_getrlimit)
>>>  #define __NR_setrlimit 164
>>>  __SC_COMP(__NR_setrlimit, sys_setrlimit, compat_sys_setrlimit)
>>> +#endif
>>> +
>>>  #define __NR_getrusage 165
>>>  __SC_COMP(__NR_getrusage, sys_getrusage, compat_sys_getrusage)
>>>  #define __NR_umask 166
>>> --
>>> 2.7.4
>>>
>

^ permalink raw reply

* [PATCH] irqchip/ls-scfg-msi: update Layerscape SCFG MSI driver
From: Minghuan Lian @ 2016-10-25 12:39 UTC (permalink / raw)
  To: linux-arm-kernel

1. The patch uses soc_device_match() to match the SoC family
and revision instead of DTS compatible, because compatible cannot
describe the SoC revision information.
2. The patch provides a new method to support Layerscape
SCFG MSI. It tries to assign a dedicated MSIR to every core.
When changing a MSI interrupt affinity, the MSI message
data will be changed to refer to a new MSIR that has
been associated with the core.

Signed-off-by: Minghuan Lian <Minghuan.Lian@nxp.com>
---
The patch depends on https://patchwork.kernel.org/patch/9342915/

 drivers/irqchip/irq-ls-scfg-msi.c | 444 +++++++++++++++++++++++++++++++-------
 1 file changed, 367 insertions(+), 77 deletions(-)

diff --git a/drivers/irqchip/irq-ls-scfg-msi.c b/drivers/irqchip/irq-ls-scfg-msi.c
index 02cca74c..0245d8a 100644
--- a/drivers/irqchip/irq-ls-scfg-msi.c
+++ b/drivers/irqchip/irq-ls-scfg-msi.c
@@ -10,6 +10,7 @@
  * published by the Free Software Foundation.
  */
 
+#include <linux/bitmap.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/msi.h>
@@ -17,23 +18,91 @@
 #include <linux/irq.h>
 #include <linux/irqchip/chained_irq.h>
 #include <linux/irqdomain.h>
+#include <linux/of_address.h>
+#include <linux/of_irq.h>
 #include <linux/of_pci.h>
 #include <linux/of_platform.h>
 #include <linux/spinlock.h>
+#include <linux/sys_soc.h>
 
-#define MSI_MAX_IRQS	32
-#define MSI_IBS_SHIFT	3
-#define MSIR		4
+#define LS_MSIR_NUM_MAX		4 /* MSIIR can index 4 MSI registers */
+#define IRQS_32_PER_MSIR	32
+#define IRQS_8_PER_MSIR		8
+
+#define MSIR_OFFSET(idx)	((idx) * 0x4)
+
+enum msi_affinity_flag {
+	MSI_GROUP_AFFINITY_FLAG,
+	MSI_AFFINITY_FLAG
+};
+
+struct ls_scfg_msi;
+struct ls_scfg_msi_ctrl;
+
+struct ls_scfg_msi_cfg {
+	u32 ibs_shift; /* Shift of interrupt bit select */
+	u32 msir_irqs; /* The irq number per MSIR */
+	u32 msir_base; /* The base address of MSIR */
+};
+
+struct ls_scfg_msir {
+	struct ls_scfg_msi_ctrl	*ctrl;
+	void __iomem		*addr;
+	int			index;
+	int			virq;
+};
+
+struct ls_scfg_msi_ctrl {
+	struct list_head		list;
+	struct ls_scfg_msi		*msi_data;
+	void __iomem			*regs;
+	phys_addr_t			msiir_addr;
+	enum msi_affinity_flag		flag;
+	int				irq_base;
+	spinlock_t			lock;
+	struct ls_scfg_msir		*msir;
+	unsigned long			*bm;
+};
 
 struct ls_scfg_msi {
-	spinlock_t		lock;
-	struct platform_device	*pdev;
-	struct irq_domain	*parent;
-	struct irq_domain	*msi_domain;
-	void __iomem		*regs;
-	phys_addr_t		msiir_addr;
-	int			irq;
-	DECLARE_BITMAP(used, MSI_MAX_IRQS);
+	struct platform_device		*pdev;
+	struct irq_domain		*parent;
+	struct irq_domain		*msi_domain;
+	struct list_head		ctrl_list;
+	const struct ls_scfg_msi_cfg	*cfg;
+	u32				cpu_num;
+};
+
+static struct ls_scfg_msi_cfg ls1021_msi_cfg = {
+	.ibs_shift = 3,
+	.msir_irqs = IRQS_32_PER_MSIR,
+	.msir_base = 0x4,
+};
+
+static struct ls_scfg_msi_cfg ls1043_rev11_msi_cfg = {
+	.ibs_shift = 2,
+	.msir_irqs = IRQS_8_PER_MSIR,
+	.msir_base = 0x10,
+};
+
+static struct ls_scfg_msi_cfg ls1046_msi_cfg = {
+	.ibs_shift = 2,
+	.msir_irqs = IRQS_32_PER_MSIR,
+	.msir_base = 0x4,
+};
+
+static struct soc_device_attribute soc_msi_matches[] = {
+	{ .family = "QorIQ LS1021A",
+	  .data = &ls1021_msi_cfg },
+	{ .family = "QorIQ LS1012A",
+	  .data = &ls1021_msi_cfg },
+	{ .family = "QorIQ LS1043A", .revision = "1.0",
+	  .data = &ls1021_msi_cfg },
+	{ .family = "QorIQ LS1043A", .revision = "1.1",
+	  .data = &ls1043_rev11_msi_cfg },
+	{ .family = "QorIQ LS1046A",
+	  .data = &ls1046_msi_cfg },
+	{ },
 };
 
 static struct irq_chip ls_scfg_msi_irq_chip = {
@@ -49,19 +118,53 @@ struct ls_scfg_msi {
 	.chip	= &ls_scfg_msi_irq_chip,
 };
 
+static int ctrl_num;
+
+static irqreturn_t (*ls_scfg_msi_irq_handler)(int irq, void *arg);
+
 static void ls_scfg_msi_compose_msg(struct irq_data *data, struct msi_msg *msg)
 {
-	struct ls_scfg_msi *msi_data = irq_data_get_irq_chip_data(data);
+	struct ls_scfg_msi_ctrl *ctrl = irq_data_get_irq_chip_data(data);
+	u32 ibs, srs;
 
-	msg->address_hi = upper_32_bits(msi_data->msiir_addr);
-	msg->address_lo = lower_32_bits(msi_data->msiir_addr);
-	msg->data = data->hwirq << MSI_IBS_SHIFT;
+	msg->address_hi = upper_32_bits(ctrl->msiir_addr);
+	msg->address_lo = lower_32_bits(ctrl->msiir_addr);
+
+	ibs = data->hwirq - ctrl->irq_base;
+
+	srs = cpumask_first(irq_data_get_affinity_mask(data));
+	if (srs >= ctrl->msi_data->cpu_num)
+		srs = 0;
+
+	msg->data = ibs << ctrl->msi_data->cfg->ibs_shift | srs;
+
+	pr_debug("%s: ibs %d srs %d address0x%x-0x%x data 0x%x\n",
+		 __func__, ibs, srs, msg->address_hi,
+		 msg->address_lo, msg->data);
 }
 
-static int ls_scfg_msi_set_affinity(struct irq_data *irq_data,
-				    const struct cpumask *mask, bool force)
+static int ls_scfg_msi_set_affinity(struct irq_data *data,
+				const struct cpumask *mask, bool force)
 {
-	return -EINVAL;
+	struct ls_scfg_msi_ctrl *ctrl = irq_data_get_irq_chip_data(data);
+	u32 cpu;
+
+	if (!force)
+		cpu = cpumask_any_and(mask, cpu_online_mask);
+	else
+		cpu = cpumask_first(mask);
+
+	if (cpu >= ctrl->msi_data->cpu_num)
+		return -EINVAL;
+
+	if (ctrl->msir[cpu].virq <= 0) {
+		pr_warn("cannot bind the irq to cpu%d\n", cpu);
+		return -EINVAL;
+	}
+
+	cpumask_copy(irq_data_get_affinity_mask(data), mask);
+
+	return IRQ_SET_MASK_OK_NOCOPY;
 }
 
 static struct irq_chip ls_scfg_msi_parent_chip = {
@@ -76,44 +179,57 @@ static int ls_scfg_msi_domain_irq_alloc(struct irq_domain *domain,
 					void *args)
 {
 	struct ls_scfg_msi *msi_data = domain->host_data;
-	int pos, err = 0;
+	static struct list_head *current_entry;
+	struct ls_scfg_msi_ctrl *ctrl;
+	int i, hwirq = -ENOMEM;
+
+	if (!current_entry || current_entry->next == &msi_data->ctrl_list)
+		current_entry = &msi_data->ctrl_list;
+
+	list_for_each_entry(ctrl, current_entry, list) {
+		spin_lock(&ctrl->lock);
+		hwirq = bitmap_find_free_region(ctrl->bm,
+						msi_data->cfg->msir_irqs,
+						order_base_2(nr_irqs));
+		spin_unlock(&ctrl->lock);
+
+		if (hwirq >= 0)
+			break;
+	}
 
-	WARN_ON(nr_irqs != 1);
+	if (hwirq < 0)
+		return hwirq;
 
-	spin_lock(&msi_data->lock);
-	pos = find_first_zero_bit(msi_data->used, MSI_MAX_IRQS);
-	if (pos < MSI_MAX_IRQS)
-		__set_bit(pos, msi_data->used);
-	else
-		err = -ENOSPC;
-	spin_unlock(&msi_data->lock);
+	hwirq = hwirq + ctrl->irq_base;
 
-	if (err)
-		return err;
+	for (i = 0; i < nr_irqs; i++)
+		irq_domain_set_info(domain, virq + i, hwirq + i,
+				    &ls_scfg_msi_parent_chip, ctrl,
+				    handle_simple_irq, NULL, NULL);
 
-	irq_domain_set_info(domain, virq, pos,
-			    &ls_scfg_msi_parent_chip, msi_data,
-			    handle_simple_irq, NULL, NULL);
+	current_entry = &ctrl->list;
 
 	return 0;
 }
 
 static void ls_scfg_msi_domain_irq_free(struct irq_domain *domain,
-				   unsigned int virq, unsigned int nr_irqs)
+					unsigned int virq,
+					unsigned int nr_irqs)
 {
 	struct irq_data *d = irq_domain_get_irq_data(domain, virq);
-	struct ls_scfg_msi *msi_data = irq_data_get_irq_chip_data(d);
+	struct ls_scfg_msi_ctrl *ctrl = irq_data_get_irq_chip_data(d);
 	int pos;
 
-	pos = d->hwirq;
-	if (pos < 0 || pos >= MSI_MAX_IRQS) {
-		pr_err("failed to teardown msi. Invalid hwirq %d\n", pos);
+	pos = d->hwirq - ctrl->irq_base;
+
+	if (pos < 0 || pos >= ctrl->msi_data->cfg->msir_irqs) {
+		pr_err("Failed to teardown msi. Invalid hwirq %d\n", pos);
 		return;
 	}
 
-	spin_lock(&msi_data->lock);
-	__clear_bit(pos, msi_data->used);
-	spin_unlock(&msi_data->lock);
+	spin_lock(&ctrl->lock);
+	bitmap_release_region(ctrl->bm, pos, order_base_2(nr_irqs));
+	spin_unlock(&ctrl->lock);
 }
 
 static const struct irq_domain_ops ls_scfg_msi_domain_ops = {
@@ -121,29 +237,198 @@ static void ls_scfg_msi_domain_irq_free(struct irq_domain *domain,
 	.free	= ls_scfg_msi_domain_irq_free,
 };
 
-static void ls_scfg_msi_irq_handler(struct irq_desc *desc)
+static irqreturn_t ls_scfg_msi_irqs32_handler(int irq, void *arg)
 {
-	struct ls_scfg_msi *msi_data = irq_desc_get_handler_data(desc);
+	struct ls_scfg_msir *msir = arg;
+	struct ls_scfg_msi_ctrl *ctrl = msir->ctrl;
+	struct ls_scfg_msi *msi_data = ctrl->msi_data;
 	unsigned long val;
-	int pos, virq;
+	int pos = 0, hwirq, virq;
+	irqreturn_t ret = IRQ_NONE;
 
-	chained_irq_enter(irq_desc_get_chip(desc), desc);
+	val = ioread32be(msir->addr);
 
-	val = ioread32be(msi_data->regs + MSIR);
-	for_each_set_bit(pos, &val, MSI_MAX_IRQS) {
-		virq = irq_find_mapping(msi_data->parent, (31 - pos));
-		if (virq)
+	for_each_set_bit(pos, &val, IRQS_32_PER_MSIR) {
+		hwirq = (IRQS_32_PER_MSIR - 1 - pos) + ctrl->irq_base;
+		virq = irq_find_mapping(msi_data->parent, hwirq);
+		if (virq) {
 			generic_handle_irq(virq);
+			ret = IRQ_HANDLED;
+		}
+	}
+
+	return ret;
+}
+
+static irqreturn_t ls_scfg_msi_irqs8_handler(int irq, void *arg)
+{
+	struct ls_scfg_msir *msir = arg;
+	struct ls_scfg_msi_ctrl *ctrl = msir->ctrl;
+	struct ls_scfg_msi *msi_data = ctrl->msi_data;
+	unsigned long val;
+	int pos = 0, hwirq, virq;
+	irqreturn_t ret = IRQ_NONE;
+
+	val = ioread32be(msir->addr);
+	val = (val << (msir->index * 8)) & 0xff000000;
+
+	for_each_set_bit(pos, &val, IRQS_32_PER_MSIR) {
+		hwirq = (IRQS_32_PER_MSIR - 1 - pos) + ctrl->irq_base;
+		virq = irq_find_mapping(msi_data->parent, hwirq);
+		if (virq) {
+			generic_handle_irq(virq);
+			ret = IRQ_HANDLED;
+		}
+	}
+
+	return ret;
+}
+
+static void ls_scfg_msi_cascade(struct irq_desc *desc)
+{
+	struct ls_scfg_msir *msir = irq_desc_get_handler_data(desc);
+	struct irq_chip *chip = irq_desc_get_chip(desc);
+
+	chained_irq_enter(chip, desc);
+	ls_scfg_msi_irq_handler(desc->irq_data.irq, msir);
+	chained_irq_exit(chip, desc);
+}
+
+static int ls_scfg_msi_setup_hwirq(struct ls_scfg_msi_ctrl *ctrl,
+				   struct device_node *node,
+				   int index)
+{
+	struct ls_scfg_msir *msir = &ctrl->msir[index];
+	int ret;
+
+	msir->virq = of_irq_get(node, index);
+	if (msir->virq <= 0)
+		return -ENODEV;
+
+	msir->index = index;
+	msir->ctrl = ctrl;
+	msir->addr = ctrl->regs + ctrl->msi_data->cfg->msir_base +
+		     MSIR_OFFSET(msir->index);
+
+	if (ctrl->flag == MSI_GROUP_AFFINITY_FLAG) {
+		ret = request_irq(msir->virq, ls_scfg_msi_irq_handler,
+				  IRQF_NO_THREAD, "MSI-GROUP", msir);
+		if (ret) {
+			pr_err("failed to request irq %d\n", msir->virq);
+			msir->virq = 0;
+			return -ENODEV;
+		}
+	} else {
+		irq_set_chained_handler(msir->virq, ls_scfg_msi_cascade);
+		irq_set_handler_data(msir->virq, msir);
+		irq_set_affinity(msir->virq, get_cpu_mask(index));
+	}
+
+	return 0;
+}
+
+static void ls_scfg_msi_ctrl_remove(struct ls_scfg_msi_ctrl *ctrl)
+{
+	struct ls_scfg_msir *msir;
+	int i;
+
+	if (!ctrl)
+		return;
+
+	if (ctrl->msir) {
+		for (i = 0; i < ctrl->msi_data->cpu_num; i++) {
+			msir = &ctrl->msir[i];
+
+			if (msir->virq <= 0)
+				continue;
+
+			if (ctrl->flag == MSI_GROUP_AFFINITY_FLAG)
+				free_irq(msir->virq, msir);
+			else
+				irq_set_chained_handler_and_data(msir->virq,
+								 NULL, NULL);
+		}
+
+		kfree(ctrl->msir);
 	}
 
-	chained_irq_exit(irq_desc_get_chip(desc), desc);
+	if (ctrl->regs)
+		iounmap(ctrl->regs);
+
+	kfree(ctrl->bm);
+	kfree(ctrl);
+}
+
+static int ls_scfg_msi_ctrl_probe(struct device_node *node,
+				  struct ls_scfg_msi *msi_data)
+{
+	struct ls_scfg_msi_ctrl *ctrl;
+	struct resource res;
+	int err, irqs, i;
+
+	err = of_address_to_resource(node, 0, &res);
+	if (err) {
+		pr_warn("%s: no regs\n", node->full_name);
+		return -ENXIO;
+	}
+
+	ctrl = kzalloc(sizeof(*ctrl), GFP_KERNEL);
+	if (!ctrl)
+		return  -ENOMEM;
+
+	ctrl->msi_data = msi_data;
+	ctrl->msiir_addr = res.start;
+	spin_lock_init(&ctrl->lock);
+
+	ctrl->regs = ioremap(res.start, resource_size(&res));
+	if (!ctrl->regs) {
+		pr_err("%s: unable to map registers\n", node->full_name);
+		err = -ENOMEM;
+		goto _err;
+	}
+
+	ctrl->msir = kcalloc(msi_data->cpu_num, sizeof(struct ls_scfg_msir),
+			     GFP_KERNEL);
+	if (!ctrl->msir) {
+		err = -ENOMEM;
+		goto _err;
+	}
+
+	ctrl->bm = kcalloc(BITS_TO_LONGS(msi_data->cfg->msir_irqs),
+			   sizeof(long), GFP_KERNEL);
+	if (!ctrl->bm) {
+		err = -ENOMEM;
+		goto _err;
+	}
+
+	ctrl->irq_base = msi_data->cfg->msir_irqs * ctrl_num;
+	ctrl_num++;
+
+	irqs = of_irq_count(node);
+	if (irqs >= msi_data->cpu_num)
+		ctrl->flag = MSI_AFFINITY_FLAG;
+	else
+		ctrl->flag = MSI_GROUP_AFFINITY_FLAG;
+
+	for (i = 0; i < msi_data->cpu_num; i++)
+		ls_scfg_msi_setup_hwirq(ctrl, node, i);
+
+	list_add_tail(&ctrl->list, &msi_data->ctrl_list);
+
+	return 0;
+
+_err:
+	ls_scfg_msi_ctrl_remove(ctrl);
+	pr_err("MSI: failed probing %s (%d)\n", node->full_name, err);
+	return err;
 }
 
 static int ls_scfg_msi_domains_init(struct ls_scfg_msi *msi_data)
 {
 	/* Initialize MSI domain parent */
 	msi_data->parent = irq_domain_add_linear(NULL,
-						 MSI_MAX_IRQS,
+						 msi_data->cfg->msir_irqs *
+						 ctrl_num,
 						 &ls_scfg_msi_domain_ops,
 						 msi_data);
 	if (!msi_data->parent) {
@@ -167,51 +452,57 @@ static int ls_scfg_msi_domains_init(struct ls_scfg_msi *msi_data)
 static int ls_scfg_msi_probe(struct platform_device *pdev)
 {
 	struct ls_scfg_msi *msi_data;
-	struct resource *res;
-	int ret;
+	const struct soc_device_attribute *match;
+	struct device_node *child;
 
 	msi_data = devm_kzalloc(&pdev->dev, sizeof(*msi_data), GFP_KERNEL);
 	if (!msi_data)
 		return -ENOMEM;
 
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	msi_data->regs = devm_ioremap_resource(&pdev->dev, res);
-	if (IS_ERR(msi_data->regs)) {
-		dev_err(&pdev->dev, "failed to initialize 'regs'\n");
-		return PTR_ERR(msi_data->regs);
-	}
-	msi_data->msiir_addr = res->start;
-
-	msi_data->irq = platform_get_irq(pdev, 0);
-	if (msi_data->irq <= 0) {
-		dev_err(&pdev->dev, "failed to get MSI irq\n");
-		return -ENODEV;
-	}
+	INIT_LIST_HEAD(&msi_data->ctrl_list);
 
 	msi_data->pdev = pdev;
-	spin_lock_init(&msi_data->lock);
+	msi_data->cpu_num = num_possible_cpus();
+
+	match = soc_device_match(soc_msi_matches);
+	if (match)
+		msi_data->cfg = match->data;
+	else
+		msi_data->cfg = &ls1046_msi_cfg;
+
+	if (msi_data->cfg->msir_irqs == IRQS_8_PER_MSIR)
+		ls_scfg_msi_irq_handler = ls_scfg_msi_irqs8_handler;
+	else
+		ls_scfg_msi_irq_handler = ls_scfg_msi_irqs32_handler;
 
-	ret = ls_scfg_msi_domains_init(msi_data);
-	if (ret)
-		return ret;
+	for_each_child_of_node(msi_data->pdev->dev.of_node, child)
+		ls_scfg_msi_ctrl_probe(child, msi_data);
 
-	irq_set_chained_handler_and_data(msi_data->irq,
-					 ls_scfg_msi_irq_handler,
-					 msi_data);
+	ls_scfg_msi_domains_init(msi_data);
 
 	platform_set_drvdata(pdev, msi_data);
 
+	dev_info(&pdev->dev, "irqs:%dx%d ibs_shift:%d msir_base:0x%x\n",
+		 msi_data->cfg->msir_irqs, ctrl_num,
+		 msi_data->cfg->ibs_shift, msi_data->cfg->msir_base);
+
 	return 0;
 }
 
 static int ls_scfg_msi_remove(struct platform_device *pdev)
 {
 	struct ls_scfg_msi *msi_data = platform_get_drvdata(pdev);
+	struct ls_scfg_msi_ctrl *ctrl, *temp;
 
-	irq_set_chained_handler_and_data(msi_data->irq, NULL, NULL);
+	list_for_each_entry_safe(ctrl, temp, &msi_data->ctrl_list, list) {
+		list_move_tail(&ctrl->list, &msi_data->ctrl_list);
+		ls_scfg_msi_ctrl_remove(ctrl);
+	}
 
-	irq_domain_remove(msi_data->msi_domain);
-	irq_domain_remove(msi_data->parent);
+	if (msi_data->msi_domain)
+		irq_domain_remove(msi_data->msi_domain);
+	if (msi_data->parent)
+		irq_domain_remove(msi_data->parent);
 
 	platform_set_drvdata(pdev, NULL);
 
@@ -219,8 +510,7 @@ static int ls_scfg_msi_remove(struct platform_device *pdev)
 }
 
 static const struct of_device_id ls_scfg_msi_id[] = {
-	{ .compatible = "fsl,1s1021a-msi", },
-	{ .compatible = "fsl,1s1043a-msi", },
+	{ .compatible = "fsl,ls-scfg-msi" },
 	{},
 };
 
-- 
1.9.1

^ permalink raw reply related

* [PATCH] pci: layerscape: add LS1046a support
From: Minghuan Lian @ 2016-10-25 12:36 UTC (permalink / raw)
  To: linux-arm-kernel

From: "mingkai.hu@nxp.com" <mingkai.hu@nxp.com>

1. LS1046a PCIe controller has a different LUT_DBG offset.
Available "lut_dbg" is added to ls_pcie_drvdata to describe
this difference.
2. Match LS1046 PCIe compatible

Signed-off-by: Minghuan Lian <Minghuan.Lian@nxp.com>
Signed-off-by: Mingkai Hu <mingkai.hu@nxp.com>
---
 Documentation/devicetree/bindings/pci/layerscape-pci.txt |  1 +
 drivers/pci/host/pci-layerscape.c                        | 13 ++++++++++++-
 2 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/pci/layerscape-pci.txt b/Documentation/devicetree/bindings/pci/layerscape-pci.txt
index 41e9f55..ee1c72d5 100644
--- a/Documentation/devicetree/bindings/pci/layerscape-pci.txt
+++ b/Documentation/devicetree/bindings/pci/layerscape-pci.txt
@@ -15,6 +15,7 @@ Required properties:
 - compatible: should contain the platform identifier such as:
         "fsl,ls1021a-pcie", "snps,dw-pcie"
         "fsl,ls2080a-pcie", "fsl,ls2085a-pcie", "snps,dw-pcie"
+        "fsl,ls1046a-pcie"
 - reg: base addresses and lengths of the PCIe controller
 - interrupts: A list of interrupt outputs of the controller. Must contain an
   entry for each entry in the interrupt-names property.
diff --git a/drivers/pci/host/pci-layerscape.c b/drivers/pci/host/pci-layerscape.c
index 958187f..8cebf9a 100644
--- a/drivers/pci/host/pci-layerscape.c
+++ b/drivers/pci/host/pci-layerscape.c
@@ -41,6 +41,7 @@
 struct ls_pcie_drvdata {
 	u32 lut_offset;
 	u32 ltssm_shift;
+	u32 lut_dbg;
 	struct pcie_host_ops *ops;
 };
 
@@ -134,7 +135,7 @@ static int ls_pcie_link_up(struct pcie_port *pp)
 	struct ls_pcie *pcie = to_ls_pcie(pp);
 	u32 state;
 
-	state = (ioread32(pcie->lut + PCIE_LUT_DBG) >>
+	state = (ioread32(pcie->lut + pcie->drvdata->lut_dbg) >>
 		 pcie->drvdata->ltssm_shift) &
 		 LTSSM_STATE_MASK;
 
@@ -196,18 +197,28 @@ static int ls_pcie_msi_host_init(struct pcie_port *pp,
 static struct ls_pcie_drvdata ls1043_drvdata = {
 	.lut_offset = 0x10000,
 	.ltssm_shift = 24,
+	.lut_dbg = 0x7fc,
+	.ops = &ls_pcie_host_ops,
+};
+
+static struct ls_pcie_drvdata ls1046_drvdata = {
+	.lut_offset = 0x80000,
+	.ltssm_shift = 24,
+	.lut_dbg = 0x407fc,
 	.ops = &ls_pcie_host_ops,
 };
 
 static struct ls_pcie_drvdata ls2080_drvdata = {
 	.lut_offset = 0x80000,
 	.ltssm_shift = 0,
+	.lut_dbg = 0x7fc,
 	.ops = &ls_pcie_host_ops,
 };
 
 static const struct of_device_id ls_pcie_of_match[] = {
 	{ .compatible = "fsl,ls1021a-pcie", .data = &ls1021_drvdata },
 	{ .compatible = "fsl,ls1043a-pcie", .data = &ls1043_drvdata },
+	{ .compatible = "fsl,ls1046a-pcie", .data = &ls1046_drvdata },
 	{ .compatible = "fsl,ls2080a-pcie", .data = &ls2080_drvdata },
 	{ .compatible = "fsl,ls2085a-pcie", .data = &ls2080_drvdata },
 	{ },
-- 
1.9.1

^ permalink raw reply related

* [PATCH 6/6] arm64: dts: ls1046a: add PCIe dts node
From: Minghuan Lian @ 2016-10-25 12:35 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1477398945-22774-1-git-send-email-Minghuan.Lian@nxp.com>

LS1046a has three PCIe controllers.

Signed-off-by: Minghuan Lian <Minghuan.Lian@nxp.com>
---
 arch/arm64/boot/dts/freescale/fsl-ls1046a.dtsi | 66 ++++++++++++++++++++++++++
 1 file changed, 66 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1046a.dtsi b/arch/arm64/boot/dts/freescale/fsl-ls1046a.dtsi
index 5509dca..427cba4 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls1046a.dtsi
+++ b/arch/arm64/boot/dts/freescale/fsl-ls1046a.dtsi
@@ -543,5 +543,71 @@
 					     <0 157 0x4>;
 			};
 		};
+
+		pcie at 3400000 {
+			compatible = "fsl,ls1046a-pcie", "snps,dw-pcie";
+			reg = <0x00 0x03400000 0x0 0x00100000   /* controller registers */
+			       0x40 0x00000000 0x0 0x00002000>; /* configuration space */
+			reg-names = "regs", "config";
+			interrupts = <0 118 0x4>, /* controller interrupt */
+				     <0 117 0x4>; /* PME interrupt */
+			interrupt-names = "aer", "pme";
+			#address-cells = <3>;
+			#size-cells = <2>;
+			device_type = "pci";
+			dma-coherent;
+			num-lanes = <4>;
+			bus-range = <0x0 0xff>;
+			ranges = <0x81000000 0x0 0x00000000 0x40 0x00010000 0x0 0x00010000   /* downstream I/O */
+				  0x82000000 0x0 0x40000000 0x40 0x40000000 0x0 0x40000000>; /* non-prefetchable memory */
+			msi-parent = <&msi>;
+			#interrupt-cells = <1>;
+			interrupt-map-mask = <0 0 0 7>;
+			interrupt-map = <0000 0 0 1 &gic 0 110 0x4>;
+		};
+
+		pcie at 3500000 {
+			compatible = "fsl,ls1046a-pcie", "snps,dw-pcie";
+			reg = <0x00 0x03500000 0x0 0x00100000   /* controller registers */
+			       0x48 0x00000000 0x0 0x00002000>; /* configuration space */
+			reg-names = "regs", "config";
+			interrupts = <0 128 0x4>,
+				     <0 127 0x4>;
+			interrupt-names = "aer", "pme";
+			#address-cells = <3>;
+			#size-cells = <2>;
+			device_type = "pci";
+			dma-coherent;
+			num-lanes = <2>;
+			bus-range = <0x0 0xff>;
+			ranges = <0x81000000 0x0 0x00000000 0x48 0x00010000 0x0 0x00010000   /* downstream I/O */
+				  0x82000000 0x0 0x40000000 0x48 0x40000000 0x0 0x40000000>; /* non-prefetchable memory */
+			msi-parent = <&msi>;
+			#interrupt-cells = <1>;
+			interrupt-map-mask = <0 0 0 7>;
+			interrupt-map = <0000 0 0 1 &gic 0 120 0x4>;
+		};
+
+		pcie at 3600000 {
+			compatible = "fsl,ls1046a-pcie", "snps,dw-pcie";
+			reg = <0x00 0x03600000 0x0 0x00100000   /* controller registers */
+			       0x50 0x00000000 0x0 0x00002000>; /* configuration space */
+			reg-names = "regs", "config";
+			interrupts = <0 162 0x4>,
+				     <0 161 0x4>;
+			interrupt-names = "aer", "pme";
+			#address-cells = <3>;
+			#size-cells = <2>;
+			device_type = "pci";
+			dma-coherent;
+			num-lanes = <2>;
+			bus-range = <0x0 0xff>;
+			ranges = <0x81000000 0x0 0x00000000 0x50 0x00010000 0x0 0x00010000   /* downstream I/O */
+				  0x82000000 0x0 0x40000000 0x50 0x40000000 0x0 0x40000000>; /* non-prefetchable memory */
+			msi-parent = <&msi>;
+			#interrupt-cells = <1>;
+			interrupt-map-mask = <0 0 0 7>;
+			interrupt-map = <0000 0 0 1 &gic 0 154 0x4>;
+		};
 	};
 };
-- 
1.9.1

^ permalink raw reply related

* [PATCH 5/6] arm64: dts: ls1043a: update gic dts node
From: Minghuan Lian @ 2016-10-25 12:35 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1477398945-22774-1-git-send-email-Minghuan.Lian@nxp.com>

From: Gong Qianyu <Qianyu.Gong@nxp.com>

In order to support kvm, rev1.1 LS1043a GIC register has been
changed to align as 64K. The patch updates GIC node according to
the rev1.1 hardware.

Signed-off-by: Gong Qianyu <Qianyu.Gong@nxp.com>
Signed-off-by: Minghuan Lian <Minghuan.Lian@nxp.com>
---
 arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi b/arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi
index 5295bb9..da1809d 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi
+++ b/arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi
@@ -144,10 +144,10 @@
 		compatible = "arm,gic-400";
 		#interrupt-cells = <3>;
 		interrupt-controller;
-		reg = <0x0 0x1401000 0 0x1000>, /* GICD */
-		      <0x0 0x1402000 0 0x2000>, /* GICC */
-		      <0x0 0x1404000 0 0x2000>, /* GICH */
-		      <0x0 0x1406000 0 0x2000>; /* GICV */
+		reg = <0x0 0x1410000 0 0x10000>, /* GICD */
+		      <0x0 0x1420000 0 0x20000>, /* GICC */
+		      <0x0 0x1440000 0 0x20000>, /* GICH */
+		      <0x0 0x1460000 0 0x20000>; /* GICV */
 		interrupts = <1 9 0xf08>;
 	};
 
-- 
1.9.1

^ permalink raw reply related

* [PATCH 4/6] arm64: dts: ls1046a: add MSI dts node
From: Minghuan Lian @ 2016-10-25 12:35 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1477398945-22774-1-git-send-email-Minghuan.Lian@nxp.com>

LS1046a has three MSI controllers. each controller is assigned
four SPI interrupts.

Signed-off-by: Minghuan Lian <Minghuan.Lian@nxp.com>
---
 arch/arm64/boot/dts/freescale/fsl-ls1046a.dtsi | 32 ++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1046a.dtsi b/arch/arm64/boot/dts/freescale/fsl-ls1046a.dtsi
index 38806ca..5509dca 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls1046a.dtsi
+++ b/arch/arm64/boot/dts/freescale/fsl-ls1046a.dtsi
@@ -511,5 +511,37 @@
 			interrupts = <GIC_SPI 69 IRQ_TYPE_LEVEL_HIGH>;
 			clocks = <&clockgen 4 1>;
 		};
+
+		msi: msi-controller {
+			compatible = "fsl,ls-scfg-msi";
+			#address-cells = <2>;
+			#size-cells = <2>;
+			ranges;
+			msi-controller;
+
+			msi0 at 1580000 {
+				reg = <0x0 0x1580000 0x0 0x10000>;
+				interrupts = <0 116 0x4>,
+					     <0 111 0x4>,
+					     <0 112 0x4>,
+					     <0 113 0x4>;
+			};
+
+			msi1 at 1590000 {
+				reg = <0x0 0x1590000 0x0 0x10000>;
+				interrupts = <0 126 0x4>,
+					     <0 121 0x4>,
+					     <0 122 0x4>,
+					     <0 123 0x4>;
+			};
+
+			msi2 at 15a0000 {
+				reg = <0x0 0x15a0000 0x0 0x10000>;
+				interrupts = <0 160 0x4>,
+					     <0 155 0x4>,
+					     <0 156 0x4>,
+					     <0 157 0x4>;
+			};
+		};
 	};
 };
-- 
1.9.1

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox