* Re: [PATCH -next] input: keyboard: remove duplicated include from mtk-pmic-keys.c
From: Lee Jones @ 2018-12-10 10:07 UTC (permalink / raw)
To: YueHaibing
Cc: dmitry.torokhov, linux-kernel, linux-mediatek, linux-input,
matthias.bgg, chen.zhong, linux-arm-kernel
In-Reply-To: <4f1a67e5-b735-f8bd-88b8-3a1ca590eba2@huawei.com>
On Mon, 10 Dec 2018, YueHaibing wrote:
> On 2018/12/10 14:15, Lee Jones wrote:
> > On Sun, 09 Dec 2018, YueHaibing wrote:
> >
> >> Remove duplicated include.
> >>
> >> Signed-off-by: YueHaibing <yuehaibing@huawei.com>
> >> ---
> >> drivers/input/keyboard/mtk-pmic-keys.c | 1 -
> >> 1 file changed, 1 deletion(-)
> >>
> >> diff --git a/drivers/input/keyboard/mtk-pmic-keys.c b/drivers/input/keyboard/mtk-pmic-keys.c
> >> index 02c67a1..5027ebb 100644
> >> --- a/drivers/input/keyboard/mtk-pmic-keys.c
> >> +++ b/drivers/input/keyboard/mtk-pmic-keys.c
> >> @@ -19,7 +19,6 @@
> >> #include <linux/input.h>
> >> #include <linux/interrupt.h>
> >> #include <linux/platform_device.h>
> >> -#include <linux/kernel.h>
> >> #include <linux/of.h>
> >> #include <linux/of_device.h>
> >> #include <linux/regmap.h>
> >
> > You are removing the wrong one.
>
> No, linux/kernel.h is a duplicated include indeed.
Actually both are not correct, but the first instance (at the top) is
even more incorrect.
> > Please convert this patch's main intent to alphabetise the header
> > files. Then you can remove any obvious duplicates.
>
> I can alphabetize it in v2 if need be.
Yes please.
--
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v4 1/2] dmaengine: 8250_mtk_dma: add Mediatek uart DMA support
From: kbuild test robot @ 2018-12-10 10:07 UTC (permalink / raw)
To: Long Cheng
Cc: Mark Rutland, devicetree, Sean Wang, srv_heupstream,
Greg Kroah-Hartman, Sean Wang, linux-kernel, YT Shen, dmaengine,
Vinod Koul, Rob Herring, linux-mediatek, kbuild-all, linux-serial,
Jiri Slaby, Matthias Brugger, Yingjoe Chen, Dan Williams,
Long Cheng, linux-arm-kernel
In-Reply-To: <1544410636-31815-2-git-send-email-long.cheng@mediatek.com>
[-- Attachment #1: Type: text/plain, Size: 3551 bytes --]
Hi Long,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on linus/master]
[also build test ERROR on v4.20-rc6 next-20181207]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Long-Cheng/add-uart-DMA-function/20181210-125624
config: s390-allmodconfig (attached as .config)
compiler: s390x-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=7.2.0 make.cross ARCH=s390
All error/warnings (new ones prefixed by >>):
>> drivers/dma/mediatek/8250_mtk_dma.c:29:30: error: 'CONFIG_SERIAL_8250_NR_UARTS' undeclared here (not in a function); did you mean 'CONFIG_SERIAL_RP2_NR_UARTS'?
#define MTK_APDMA_CHANNELS (CONFIG_SERIAL_8250_NR_UARTS * 2)
^
>> drivers/dma/mediatek/8250_mtk_dma.c:75:25: note: in expansion of macro 'MTK_APDMA_CHANNELS'
void __iomem *mem_base[MTK_APDMA_CHANNELS];
^~~~~~~~~~~~~~~~~~
vim +29 drivers/dma/mediatek/8250_mtk_dma.c
27
28 #define MTK_APDMA_DEFAULT_REQUESTS 127
> 29 #define MTK_APDMA_CHANNELS (CONFIG_SERIAL_8250_NR_UARTS * 2)
30
31 #define VFF_EN_B BIT(0)
32 #define VFF_STOP_B BIT(0)
33 #define VFF_FLUSH_B BIT(0)
34 #define VFF_4G_SUPPORT_B BIT(0)
35 #define VFF_RX_INT_EN0_B BIT(0) /*rx valid size >= vff thre*/
36 #define VFF_RX_INT_EN1_B BIT(1)
37 #define VFF_TX_INT_EN_B BIT(0) /*tx left size >= vff thre*/
38 #define VFF_WARM_RST_B BIT(0)
39 #define VFF_RX_INT_FLAG_CLR_B (BIT(0) | BIT(1))
40 #define VFF_TX_INT_FLAG_CLR_B 0
41 #define VFF_STOP_CLR_B 0
42 #define VFF_FLUSH_CLR_B 0
43 #define VFF_INT_EN_CLR_B 0
44 #define VFF_4G_SUPPORT_CLR_B 0
45
46 /* interrupt trigger level for tx */
47 #define VFF_TX_THRE(n) ((n) * 7 / 8)
48 /* interrupt trigger level for rx */
49 #define VFF_RX_THRE(n) ((n) * 3 / 4)
50
51 #define MTK_DMA_RING_SIZE 0xffffU
52 /* invert this bit when wrap ring head again*/
53 #define MTK_DMA_RING_WRAP 0x10000U
54
55 #define VFF_INT_FLAG 0x00
56 #define VFF_INT_EN 0x04
57 #define VFF_EN 0x08
58 #define VFF_RST 0x0c
59 #define VFF_STOP 0x10
60 #define VFF_FLUSH 0x14
61 #define VFF_ADDR 0x1c
62 #define VFF_LEN 0x24
63 #define VFF_THRE 0x28
64 #define VFF_WPT 0x2c
65 #define VFF_RPT 0x30
66 /*TX: the buffer size HW can read. RX: the buffer size SW can read.*/
67 #define VFF_VALID_SIZE 0x3c
68 /*TX: the buffer size SW can write. RX: the buffer size HW can write.*/
69 #define VFF_LEFT_SIZE 0x40
70 #define VFF_DEBUG_STATUS 0x50
71 #define VFF_4G_SUPPORT 0x54
72
73 struct mtk_dmadev {
74 struct dma_device ddev;
> 75 void __iomem *mem_base[MTK_APDMA_CHANNELS];
76 spinlock_t lock; /* dma dev lock */
77 struct tasklet_struct task;
78 struct list_head pending;
79 struct clk *clk;
80 unsigned int dma_requests;
81 bool support_33bits;
82 unsigned int dma_irq[MTK_APDMA_CHANNELS];
83 struct mtk_chan *ch[MTK_APDMA_CHANNELS];
84 };
85
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 52141 bytes --]
[-- Attachment #3: Type: text/plain, Size: 176 bytes --]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v3 08/15] irqchip: Add RDA8810PL interrupt driver
From: Marc Zyngier @ 2018-12-10 10:10 UTC (permalink / raw)
To: Manivannan Sadhasivam
Cc: devicetree, jason, arnd, gregkh, linus.walleij, daniel.lezcano,
linux-kernel, amit.kucheria, robh+dt, linux-serial, jslaby, olof,
tglx, overseas.sales, afaerber, linux-arm-kernel, zhao_steven
In-Reply-To: <20181210095650.GA12397@Mani-XPS-13-9360>
On 10/12/2018 09:56, Manivannan Sadhasivam wrote:
> Hi Marc,
>
> On Mon, Dec 10, 2018 at 09:39:13AM +0000, Marc Zyngier wrote:
>> On 28/11/2018 13:50, Manivannan Sadhasivam wrote:
>>> Add interrupt driver for RDA Micro RDA8810PL SoC.
>>>
>>> Signed-off-by: Andreas Färber <afaerber@suse.de>
>>> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
>>
>> Reviewed-by: Marc Zyngier <marc.zyngier@arm.com>
>>
>> Given the dependency on the platform Kconfig entry, how do you want this
>> to be merged?
>>
>
> Hmmm, I think I can just move the platform Kconfig change to SoC changes
> in next revision so that you can apply the driver independently.
That would work for me.
Thanks,
M.
--
Jazz is not dead. It just smells funny...
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v3 2/8] KVM: arm64: Rework detection of SVE, !VHE systems
From: Christoffer Dall @ 2018-12-10 10:13 UTC (permalink / raw)
To: Marc Zyngier
Cc: Mark Rutland, kvm, Suzuki K Poulose, Catalin Marinas, Will Deacon,
James Morse, kvmarm, linux-arm-kernel
In-Reply-To: <20181206173126.139877-3-marc.zyngier@arm.com>
On Thu, Dec 06, 2018 at 05:31:20PM +0000, Marc Zyngier wrote:
> An SVE system is so far the only case where we mandate VHE. As we're
> starting to grow this requirements, let's slightly rework the way we
> deal with that situation, allowing for easy extension of this check.
>
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> ---
> arch/arm/include/asm/kvm_host.h | 2 +-
> arch/arm64/include/asm/kvm_host.h | 6 +++---
> virt/kvm/arm/arm.c | 8 ++++----
> 3 files changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h
> index 5ca5d9af0c26..2184d9ddb418 100644
> --- a/arch/arm/include/asm/kvm_host.h
> +++ b/arch/arm/include/asm/kvm_host.h
> @@ -285,7 +285,7 @@ void kvm_mmu_wp_memory_region(struct kvm *kvm, int slot);
>
> struct kvm_vcpu *kvm_mpidr_to_vcpu(struct kvm *kvm, unsigned long mpidr);
>
> -static inline bool kvm_arch_check_sve_has_vhe(void) { return true; }
> +static inline bool kvm_arch_requires_vhe(void) { return false; }
> static inline void kvm_arch_hardware_unsetup(void) {}
> static inline void kvm_arch_sync_events(struct kvm *kvm) {}
> static inline void kvm_arch_vcpu_uninit(struct kvm_vcpu *vcpu) {}
> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> index 52fbc823ff8c..d6d9aa76a943 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -422,7 +422,7 @@ static inline void __cpu_init_hyp_mode(phys_addr_t pgd_ptr,
> }
> }
>
> -static inline bool kvm_arch_check_sve_has_vhe(void)
> +static inline bool kvm_arch_requires_vhe(void)
> {
> /*
> * The Arm architecture specifies that implementation of SVE
> @@ -430,9 +430,9 @@ static inline bool kvm_arch_check_sve_has_vhe(void)
> * relies on this when SVE is present:
> */
> if (system_supports_sve())
> - return has_vhe();
> - else
> return true;
> +
> + return false;
> }
>
> static inline void kvm_arch_hardware_unsetup(void) {}
> diff --git a/virt/kvm/arm/arm.c b/virt/kvm/arm/arm.c
> index 23774970c9df..1db4c15edcdd 100644
> --- a/virt/kvm/arm/arm.c
> +++ b/virt/kvm/arm/arm.c
> @@ -1640,8 +1640,10 @@ int kvm_arch_init(void *opaque)
> return -ENODEV;
> }
>
> - if (!kvm_arch_check_sve_has_vhe()) {
> - kvm_pr_unimpl("SVE system without VHE unsupported. Broken cpu?");
> + in_hyp_mode = is_kernel_in_hyp_mode();
> +
> + if (!in_hyp_mode && kvm_arch_requires_vhe()) {
> + kvm_pr_unimpl("CPU requiring VHE was booted in non-VHE mode");
nit: The error message feels weird to me (are we reporting CPU bugs?)
and I'm not sure about the unimpl and I think there's a linse space
missing.
How about:
kvm_err("Cannot support this CPU in non-VHE mode, not initializing\n");
If we wanted to be super helpful, we could expand
kvm_arch_requires_vhe() with a print statement saying:
kvm_err("SVE detected, requiring VHE mode\n");
But thay may be overkill.
> return -ENODEV;
> }
>
> @@ -1657,8 +1659,6 @@ int kvm_arch_init(void *opaque)
> if (err)
> return err;
>
> - in_hyp_mode = is_kernel_in_hyp_mode();
> -
> if (!in_hyp_mode) {
> err = init_hyp_mode();
> if (err)
> --
> 2.19.2
>
Otherwise:
Acked-by: Christoffer Dall <christoffer.dall@arm.com>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v3 3/8] arm64: KVM: Install stage-2 translation before enabling traps
From: Christoffer Dall @ 2018-12-10 10:13 UTC (permalink / raw)
To: Marc Zyngier
Cc: Mark Rutland, kvm, Suzuki K Poulose, Catalin Marinas, Will Deacon,
James Morse, kvmarm, linux-arm-kernel
In-Reply-To: <20181206173126.139877-4-marc.zyngier@arm.com>
On Thu, Dec 06, 2018 at 05:31:21PM +0000, Marc Zyngier wrote:
> It is a bit odd that we only install stage-2 translation after having
> cleared HCR_EL2.TGE, which means that there is a window during which
> AT requests could fail as stage-2 is not configured yet.
>
> Let's move stage-2 configuration before we clear TGE, making the
> guest entry sequence clearer: we first configure all the guest stuff,
> then only switch to the guest translation regime.
>
> While we're at it, do the same thing for !VHE. It doesn't hurt,
> and keeps things symmetric.
>
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> ---
> arch/arm64/kvm/hyp/switch.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c
> index 7cc175c88a37..a8fa61c68c32 100644
> --- a/arch/arm64/kvm/hyp/switch.c
> +++ b/arch/arm64/kvm/hyp/switch.c
> @@ -499,8 +499,8 @@ int kvm_vcpu_run_vhe(struct kvm_vcpu *vcpu)
>
> sysreg_save_host_state_vhe(host_ctxt);
>
> - __activate_traps(vcpu);
> __activate_vm(vcpu->kvm);
> + __activate_traps(vcpu);
>
> sysreg_restore_guest_state_vhe(guest_ctxt);
> __debug_switch_to_guest(vcpu);
> @@ -545,8 +545,8 @@ int __hyp_text __kvm_vcpu_run_nvhe(struct kvm_vcpu *vcpu)
>
> __sysreg_save_state_nvhe(host_ctxt);
>
> - __activate_traps(vcpu);
> __activate_vm(kern_hyp_va(vcpu->kvm));
> + __activate_traps(vcpu);
>
> __hyp_vgic_restore_state(vcpu);
> __timer_enable_traps(vcpu);
> --
> 2.19.2
>
Acked-by: Christoffer Dall <christoffer.dall@arm.com>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH] arm64: dts: allwinner: a64: Fix the video engine compatible
From: Paul Kocialkowski @ 2018-12-10 10:14 UTC (permalink / raw)
To: linux-arm-kernel, devicetree, linux-kernel
Cc: Maxime Ripard, Mark Rutland, Chen-Yu Tsai, Rob Herring,
Paul Kocialkowski
When introducing the video-codec node for the video engine, the
compatible for the H5 was used instead of the compatible for the
A64. Use the right compatible instead.
Fixes: d60ce24740d2 ("arm64: dts: allwinner: a64: Add Video Engine node")
Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
---
arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
index 837a03dee875..2abb335145a6 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
@@ -390,7 +390,7 @@
};
video-codec@1c0e000 {
- compatible = "allwinner,sun50i-h5-video-engine";
+ compatible = "allwinner,sun50i-a64-video-engine";
reg = <0x01c0e000 0x1000>;
clocks = <&ccu CLK_BUS_VE>, <&ccu CLK_VE>,
<&ccu CLK_DRAM_VE>;
--
2.19.2
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* Re: [PATCH v3 6/8] arm64: KVM: Add synchronization on translation regime change for erratum 1165522
From: Christoffer Dall @ 2018-12-10 10:15 UTC (permalink / raw)
To: Marc Zyngier
Cc: Mark Rutland, kvm, Suzuki K Poulose, Catalin Marinas, Will Deacon,
James Morse, kvmarm, linux-arm-kernel
In-Reply-To: <20181206173126.139877-7-marc.zyngier@arm.com>
On Thu, Dec 06, 2018 at 05:31:24PM +0000, Marc Zyngier wrote:
> In order to ensure that slipping HCR_EL2.TGE is done at the right
> time when switching translation regime, let insert the required ISBs
> that will be patched in when erratum 1165522 is detected.
>
> Take this opportunity to add the missing include of asm/alternative.h
> which was getting there by pure luck.
>
> Reviewed-by: James Morse <james.morse@arm.com>
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> ---
> arch/arm64/include/asm/kvm_hyp.h | 8 ++++++++
> arch/arm64/kvm/hyp/switch.c | 19 +++++++++++++++++++
> 2 files changed, 27 insertions(+)
>
> diff --git a/arch/arm64/include/asm/kvm_hyp.h b/arch/arm64/include/asm/kvm_hyp.h
> index 23aca66767f9..a80a7ef57325 100644
> --- a/arch/arm64/include/asm/kvm_hyp.h
> +++ b/arch/arm64/include/asm/kvm_hyp.h
> @@ -20,6 +20,7 @@
>
> #include <linux/compiler.h>
> #include <linux/kvm_host.h>
> +#include <asm/alternative.h>
> #include <asm/sysreg.h>
>
> #define __hyp_text __section(.hyp.text) notrace
> @@ -163,6 +164,13 @@ static __always_inline void __hyp_text __load_guest_stage2(struct kvm *kvm)
> {
> write_sysreg(kvm->arch.vtcr, vtcr_el2);
> write_sysreg(kvm->arch.vttbr, vttbr_el2);
> +
> + /*
> + * ARM erratum 1165522 requires the actual execution of the above
> + * before we can switch to the EL1/EL0 translation regime used by
> + * the guest.
> + */
> + asm(ALTERNATIVE("nop", "isb", ARM64_WORKAROUND_1165522));
> }
>
> #endif /* __ARM64_KVM_HYP_H__ */
> diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c
> index a8fa61c68c32..31ee0bfc432f 100644
> --- a/arch/arm64/kvm/hyp/switch.c
> +++ b/arch/arm64/kvm/hyp/switch.c
> @@ -143,6 +143,14 @@ static void deactivate_traps_vhe(void)
> {
> extern char vectors[]; /* kernel exception vectors */
> write_sysreg(HCR_HOST_VHE_FLAGS, hcr_el2);
> +
> + /*
> + * ARM erratum 1165522 requires the actual execution of the above
> + * before we can switch to the EL2/EL0 translation regime used by
> + * the host.
> + */
> + asm(ALTERNATIVE("nop", "isb", ARM64_WORKAROUND_1165522));
> +
> write_sysreg(CPACR_EL1_DEFAULT, cpacr_el1);
> write_sysreg(vectors, vbar_el1);
> }
> @@ -499,6 +507,17 @@ int kvm_vcpu_run_vhe(struct kvm_vcpu *vcpu)
>
> sysreg_save_host_state_vhe(host_ctxt);
>
> + /*
> + * ARM erratum 1165522 requires us to configure both stage 1 and
> + * stage 2 translation for the guest context before we clear
> + * HCR_EL2.TGE.
> + *
> + * We have already configured the guest's stage 1 translation in
> + * kvm_vcpu_load_sysregs above. We must now call __activate_vm
> + * before __activate_traps, because __activate_vm configures
> + * stage 2 translation, and __activate_traps clear HCR_EL2.TGE
> + * (among other things).
> + */
> __activate_vm(vcpu->kvm);
> __activate_traps(vcpu);
>
> --
> 2.19.2
>
Acked-by: Christoffer Dall <christoffer.dall@arm.com>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH 1/2] ARM: defconfig: Update LPC18xx defconfig
From: Linus Walleij @ 2018-12-10 10:16 UTC (permalink / raw)
To: Vladimir Zapolskiy; +Cc: Linus Walleij, linux-arm-kernel
This simply updates the LPC18xx defconfig against the current
Kconfig structure in the kernel so we can make changed to the
defconfig without disturbing noise.
Cc: Vladimir Zapolskiy <vz@mleia.com>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
arch/arm/configs/lpc18xx_defconfig | 24 +++++++++---------------
1 file changed, 9 insertions(+), 15 deletions(-)
diff --git a/arch/arm/configs/lpc18xx_defconfig b/arch/arm/configs/lpc18xx_defconfig
index 23df2518203d..3627495b3dd0 100644
--- a/arch/arm/configs/lpc18xx_defconfig
+++ b/arch/arm/configs/lpc18xx_defconfig
@@ -1,5 +1,6 @@
CONFIG_CROSS_COMPILE="arm-linux-gnueabihf-"
CONFIG_HIGH_RES_TIMERS=y
+CONFIG_PREEMPT=y
CONFIG_BLK_DEV_INITRD=y
# CONFIG_RD_BZIP2 is not set
# CONFIG_RD_LZMA is not set
@@ -17,22 +18,20 @@ CONFIG_CC_OPTIMIZE_FOR_SIZE=y
CONFIG_EMBEDDED=y
# CONFIG_VM_EVENT_COUNTERS is not set
# CONFIG_SLUB_DEBUG is not set
-# CONFIG_LBDAF is not set
-# CONFIG_BLK_DEV_BSG is not set
-# CONFIG_IOSCHED_DEADLINE is not set
-# CONFIG_IOSCHED_CFQ is not set
# CONFIG_MMU is not set
-CONFIG_ARM_SINGLE_ARMV7M=y
CONFIG_ARCH_LPC18XX=y
CONFIG_SET_MEM_PARAM=y
CONFIG_DRAM_BASE=0x28000000
CONFIG_DRAM_SIZE=0x02000000
CONFIG_FLASH_MEM_BASE=0x1b000000
CONFIG_FLASH_SIZE=0x00080000
-CONFIG_PREEMPT=y
CONFIG_ZBOOT_ROM_TEXT=0x0
CONFIG_ZBOOT_ROM_BSS=0x0
CONFIG_ARM_APPENDED_DTB=y
+# CONFIG_LBDAF is not set
+# CONFIG_BLK_DEV_BSG is not set
+# CONFIG_IOSCHED_DEADLINE is not set
+# CONFIG_IOSCHED_CFQ is not set
CONFIG_BINFMT_FLAT=y
CONFIG_BINFMT_ZFLAT=y
CONFIG_BINFMT_SHARED_FLAT=y
@@ -69,7 +68,6 @@ CONFIG_BLK_DEV_SD=y
# CONFIG_SCSI_LOWLEVEL is not set
CONFIG_NETDEVICES=y
# CONFIG_NET_VENDOR_ARC is not set
-# CONFIG_NET_CADENCE is not set
# CONFIG_NET_VENDOR_BROADCOM is not set
# CONFIG_NET_VENDOR_CIRRUS is not set
# CONFIG_NET_VENDOR_FARADAY is not set
@@ -90,7 +88,6 @@ CONFIG_STMMAC_ETH=y
CONFIG_SMSC_PHY=y
# CONFIG_USB_NET_DRIVERS is not set
# CONFIG_WLAN is not set
-# CONFIG_INPUT_MOUSEDEV is not set
CONFIG_INPUT_EVDEV=y
# CONFIG_KEYBOARD_ATKBD is not set
CONFIG_KEYBOARD_GPIO=y
@@ -101,7 +98,6 @@ CONFIG_KEYBOARD_GPIO_POLLED=y
# CONFIG_UNIX98_PTYS is not set
# CONFIG_LEGACY_PTYS is not set
CONFIG_SERIAL_NONSTANDARD=y
-# CONFIG_DEVKMEM is not set
CONFIG_SERIAL_8250=y
# CONFIG_SERIAL_8250_DEPRECATED_OPTIONS is not set
CONFIG_SERIAL_8250_CONSOLE=y
@@ -144,15 +140,14 @@ CONFIG_AMBA_PL08X=y
CONFIG_LPC18XX_DMAMUX=y
CONFIG_MEMORY=y
CONFIG_ARM_PL172_MPMC=y
-CONFIG_PWM=y
-CONFIG_PWM_LPC18XX_SCT=y
CONFIG_IIO=y
CONFIG_MMA7455_I2C=y
CONFIG_LPC18XX_ADC=y
CONFIG_LPC18XX_DAC=y
CONFIG_IIO_SYSFS_TRIGGER=y
+CONFIG_PWM=y
+CONFIG_PWM_LPC18XX_SCT=y
CONFIG_PHY_LPC18XX_USB_OTG=y
-CONFIG_NVMEM=y
CONFIG_NVMEM_LPC18XX_EEPROM=y
CONFIG_EXT2_FS=y
# CONFIG_FILE_LOCKING is not set
@@ -160,9 +155,10 @@ CONFIG_EXT2_FS=y
# CONFIG_INOTIFY_USER is not set
CONFIG_JFFS2_FS=y
# CONFIG_NETWORK_FILESYSTEMS is not set
+CONFIG_CRC_ITU_T=y
+CONFIG_CRC7=y
CONFIG_PRINTK_TIME=y
CONFIG_DEBUG_INFO=y
-# CONFIG_ENABLE_WARN_DEPRECATED is not set
# CONFIG_ENABLE_MUST_CHECK is not set
CONFIG_DEBUG_FS=y
CONFIG_MAGIC_SYSRQ=y
@@ -170,5 +166,3 @@ CONFIG_MAGIC_SYSRQ=y
# CONFIG_DEBUG_BUGVERBOSE is not set
CONFIG_DEBUG_LL=y
CONFIG_EARLY_PRINTK=y
-CONFIG_CRC_ITU_T=y
-CONFIG_CRC7=y
--
2.19.2
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH 2/2] ARM: defconfig: Switch LPC18xx to use PL11x DRM driver
From: Linus Walleij @ 2018-12-10 10:16 UTC (permalink / raw)
To: Vladimir Zapolskiy; +Cc: Linus Walleij, linux-arm-kernel
In-Reply-To: <20181210101620.418-1-linus.walleij@linaro.org>
None of the LPC18xx device trees contains any display settings,
it just defines a device tree node for the CLCD (PL11x) set
as "disabled" and no panels are attached on any device tree.
This I conclude that the hardware is dormant on existing
systems, so we can without any problems switch the defconfig
over from the old ARMCLCD frame buffer driver to the new
PL11x DRM driver.
Cc: Vladimir Zapolskiy <vz@mleia.com>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
arch/arm/configs/lpc18xx_defconfig | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/arch/arm/configs/lpc18xx_defconfig b/arch/arm/configs/lpc18xx_defconfig
index 3627495b3dd0..1b72a1af8d54 100644
--- a/arch/arm/configs/lpc18xx_defconfig
+++ b/arch/arm/configs/lpc18xx_defconfig
@@ -117,8 +117,10 @@ CONFIG_WATCHDOG=y
CONFIG_LPC18XX_WATCHDOG=y
CONFIG_REGULATOR=y
CONFIG_REGULATOR_FIXED_VOLTAGE=y
-CONFIG_FB=y
-CONFIG_FB_ARMCLCD=y
+CONFIG_DRM=y
+CONFIG_DRM_PL111=y
+CONFIG_FB_MODE_HELPERS=y
+CONFIG_BACKLIGHT_LCD_SUPPORT=y
CONFIG_USB=y
CONFIG_USB_EHCI_HCD=y
CONFIG_USB_EHCI_ROOT_HUB_TT=y
--
2.19.2
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* Re: [PATCH v10 0/8] Introduce on-chip interconnect API
From: Georgi Djakov @ 2018-12-10 10:18 UTC (permalink / raw)
To: Rafael J. Wysocki, Greg Kroah-Hartman
Cc: Mark Rutland, Doug Anderson, sanjayc, maxime.ripard,
Michael Turquette, daidavid1, bjorn.andersson, Saravana Kannan,
abailon, Lorenzo Pieralisi, Vincent Guittot, seansw, Kevin Hilman,
evgreen, ksitaraman, devicetree@vger.kernel.org, Arnd Bergmann,
Linux PM, linux-arm-msm, Rob Herring, linux-tegra, Linux ARM,
Rafael J. Wysocki, Linux Kernel Mailing List, Amit Kucheria,
Thierry Reding
In-Reply-To: <CAJZ5v0gP50tmq4ZzB7LLSaWGWjGjK31y=qFg5PS9B7c2XQ+DyQ@mail.gmail.com>
Hi Rafael,
On 12/10/18 11:04, Rafael J. Wysocki wrote:
> On Thu, Dec 6, 2018 at 3:55 PM Greg KH <gregkh@linuxfoundation.org> wrote:
>>
>> On Wed, Dec 05, 2018 at 12:41:35PM -0800, Evan Green wrote:
>>> On Tue, Nov 27, 2018 at 10:03 AM Georgi Djakov <georgi.djakov@linaro.org> wrote:
>>>>
>>>> Modern SoCs have multiple processors and various dedicated cores (video, gpu,
>>>> graphics, modem). These cores are talking to each other and can generate a
>>>> lot of data flowing through the on-chip interconnects. These interconnect
>>>> buses could form different topologies such as crossbar, point to point buses,
>>>> hierarchical buses or use the network-on-chip concept.
>>>>
>>>> These buses have been sized usually to handle use cases with high data
>>>> throughput but it is not necessary all the time and consume a lot of power.
>>>> Furthermore, the priority between masters can vary depending on the running
>>>> use case like video playback or CPU intensive tasks.
>>>>
>>>> Having an API to control the requirement of the system in terms of bandwidth
>>>> and QoS, so we can adapt the interconnect configuration to match those by
>>>> scaling the frequencies, setting link priority and tuning QoS parameters.
>>>> This configuration can be a static, one-time operation done at boot for some
>>>> platforms or a dynamic set of operations that happen at run-time.
>>>>
>>>> This patchset introduce a new API to get the requirement and configure the
>>>> interconnect buses across the entire chipset to fit with the current demand.
>>>> The API is NOT for changing the performance of the endpoint devices, but only
>>>> the interconnect path in between them.
>>>
>>> For what it's worth, we are ready to land this in Chrome OS. I think
>>> this series has been very well discussed and reviewed, hasn't changed
>>> much in the last few spins, and is in good enough shape to use as a
>>> base for future patches. Georgi's also done a great job reaching out
>>> to other SoC vendors, and there appears to be enough consensus that
>>> this framework will be usable by more than just Qualcomm. There are
>>> also several drivers out on the list trying to add patches to use this
>>> framework, with more to come, so it made sense (to us) to get this
>>> base framework nailed down. In my experiments this is an important
>>> piece of the overall power management story, especially on systems
>>> that are mostly idle.
>>>
>>> I'll continue to track changes to this series and we will ultimately
>>> reconcile with whatever happens upstream, but I thought it was worth
>>> sending this note to express our "thumbs up" towards this framework.
>>
>> Looks like a v11 will be forthcoming, so I'll wait for that one to apply
>> it to the tree if all looks good.
>
> I'm honestly not sure if it is ready yet.
>
> New versions are coming on and on, which may make such an impression,
> but we had some discussion on it at the LPC and some serious questions
> were asked during it, for instance regarding the DT binding introduced
> here. I'm not sure how this particular issue has been addressed here,
> for example.
There have been no changes in bindings since v4 (other than squashing
consumer and provider bindings into a single patch and fixing typos).
The last DT comment was on v9 [1] where Rob wanted confirmation from
other SoC vendors that this works for them too. And now we have that
confirmation and there are patches posted on the list [2].
The second thing (also discussed at LPC) was about possible cases where
some consumer drivers can't calculate how much bandwidth they actually
need and how to address that. The proposal was to extend the OPP
bindings with one more property, but this is not part of this patchset.
It is a future step that needs more discussion on the mailing list. If a
driver really needs some bandwidth data now, it should be put into the
driver and not in DT. After we have enough consumers, we can discuss
again if it makes sense to extract something into DT or not.
Thanks,
Georgi
[1] https://lkml.org/lkml/2018/9/25/939
[2] https://lkml.org/lkml/2018/11/28/12
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v5 1/5] spi: spi-mem: Add driver for NXP FlexSPI controller
From: Boris Brezillon @ 2018-12-10 10:19 UTC (permalink / raw)
To: Yogesh Narayan Gaur
Cc: mark.rutland@arm.com, devicetree@vger.kernel.org, robh@kernel.org,
linux-kernel@vger.kernel.org, Schrempf Frieder,
linux-spi@vger.kernel.org, marek.vasut@gmail.com,
broonie@kernel.org, linux-mtd@lists.infradead.org,
computersforpeace@gmail.com, shawnguo@kernel.org,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <VI1PR04MB57267D9F69719B4BEF38BF8699A50@VI1PR04MB5726.eurprd04.prod.outlook.com>
On Mon, 10 Dec 2018 09:41:51 +0000
Yogesh Narayan Gaur <yogeshnarayan.gaur@nxp.com> wrote:
> > > +/* Instead of busy looping invoke readl_poll_timeout functionality.
> > > +*/ static int fspi_readl_poll_tout(struct nxp_fspi *f, void __iomem *base,
> > > + u32 mask, u32 delay_us,
> > > + u32 timeout_us, bool condition)
> > > +{
> > > + u32 reg;
> > > +
> > > + if (!f->devtype_data->little_endian)
> > > + mask = (u32)cpu_to_be32(mask);
> > > +
> > > + if (condition)
> > > + return readl_poll_timeout(base, reg, (reg & mask),
> > > + delay_us, timeout_us);
> > > + else
> > > + return readl_poll_timeout(base, reg, !(reg & mask),
> > > + delay_us, timeout_us);
> >
> > I would rather use a local variable to store the condition:
> >
> > bool c = condition ? (reg & mask):!(reg & mask);
> >
> With these type of usage getting below warning messages.
>
> drivers/spi/spi-nxp-fspi.c: In function ‘fspi_readl_poll_tout.isra.10.constprop’:
> drivers/spi/spi-nxp-fspi.c:446:21: warning: ‘reg’ may be used uninitialized in this function [-Wmaybe-uninitialized]
> bool cn = c ? (reg & mask) : !(reg & mask);
>
> If assign value to reg = 0xffffffff then timeout is start getting hit for False case and if assign value 0 then start getting timeout hit for true case.
>
> I would rather not try to modify this function.
I agree. Let's keep this function readable even if this implies
duplicating a few lines of code.
>
> > return readl_poll_timeout(base, reg, c, delay_us, timeout_us);
> >
> > > +}
> > > +
> > > +/*
> > > + * If the slave device content being changed by Write/Erase, need to
> > > + * invalidate the AHB buffer. This can be achieved by doing the reset
> > > + * of controller after setting MCR0[SWRESET] bit.
> > > + */
> > > +static inline void nxp_fspi_invalid(struct nxp_fspi *f) {
> > > + u32 reg;
> > > + int ret;
> > > +
> > > + reg = fspi_readl(f, f->iobase + FSPI_MCR0);
> > > + fspi_writel(f, reg | FSPI_MCR0_SWRST, f->iobase + FSPI_MCR0);
> > > +
> > > + /* w1c register, wait unit clear */
> > > + ret = fspi_readl_poll_tout(f, f->iobase + FSPI_MCR0,
> > > + FSPI_MCR0_SWRST, 0, POLL_TOUT, false);
> > > + WARN_ON(ret);
> > > +}
> > > +
> > > +static void nxp_fspi_prepare_lut(struct nxp_fspi *f,
> > > + const struct spi_mem_op *op)
> > > +{
> > > + void __iomem *base = f->iobase;
> > > + u32 lutval[4] = {};
> > > + int lutidx = 1, i;
> > > +
> > > + /* cmd */
> > > + lutval[0] |= LUT_DEF(0, LUT_CMD, LUT_PAD(op->cmd.buswidth),
> > > + op->cmd.opcode);
> > > +
> > > + /* addr bus width */
> > > + if (op->addr.nbytes) {
> > > + u32 addrlen = 0;
> > > +
> > > + switch (op->addr.nbytes) {
> > > + case 1:
> > > + addrlen = ADDR8BIT;
> > > + break;
> > > + case 2:
> > > + addrlen = ADDR16BIT;
> > > + break;
> > > + case 3:
> > > + addrlen = ADDR24BIT;
> > > + break;
> > > + case 4:
> > > + addrlen = ADDR32BIT;
> > > + break;
> > > + default:
> > > + dev_err(f->dev, "In-correct address length\n");
> > > + return;
> > > + }
> >
> > You don't need to validate op->addr.nbytes here, this is already done in
> > nxp_fspi_supports_op().
>
> Yes, I need to validate op->addr.nbytes else LUT would going to be programmed for 0 addrlen.
> I have checked this on the target.
Also agree there. Some operations have 0 address bytes. We could also
test addr.buswidth, but I'm fine with the addr.nbytes test too.
> > > +static void nxp_fspi_select_mem(struct nxp_fspi *f, struct spi_device
> > > +*spi) {
> > > + unsigned long rate = spi->max_speed_hz;
> > > + int ret;
> > > + uint64_t size_kb;
> > > +
> > > + /*
> > > + * Return, if previously selected slave device is same as current
> > > + * requested slave device.
> > > + */
> > > + if (f->selected == spi->chip_select)
> > > + return;
> > > +
> > > + /* Reset FLSHxxCR0 registers */
> > > + fspi_writel(f, 0, f->iobase + FSPI_FLSHA1CR0);
> > > + fspi_writel(f, 0, f->iobase + FSPI_FLSHA2CR0);
> > > + fspi_writel(f, 0, f->iobase + FSPI_FLSHB1CR0);
> > > + fspi_writel(f, 0, f->iobase + FSPI_FLSHB2CR0);
> > > +
> > > + /* Assign controller memory mapped space as size, KBytes, of flash. */
> > > + size_kb = FSPI_FLSHXCR0_SZ(f->memmap_phy_size);
> >
> Above description of this function, explains the reason for using memmap_phy_size.
> This is not the arbitrary size, but the memory mapped size being assigned to the controller.
>
> > You are still using memory of arbitrary size (memmap_phy_size) for mapping the
> > flash. Why not use the same approach as in the QSPI driver and just map
> > ahb_buf_size until we implement the dirmap API?
> The approach which being used in QSPI driver didn't work here, I have tried with that.
> In QSPI driver, while preparing LUT we are assigning read/write address in the LUT preparation and have to for some unknown hack have to provide macro for LUT_MODE instead of LUT_ADDR.
> But this thing didn't work for FlexSPI.
> I discussed with HW IP owner and they suggested only to use LUT_ADDR for specifying the address length of the command i.e. 3-byte or 4-byte address command (NOR) or 1-2 byte address command for NAND.
Actually, we would have used a LUT_ADDR too if the QSPI IP was support
ADDR instructions with a number of bytes < 3, but for some unknown
reasons it does not work.
>
> Thus, in LUT preparation we have assigned only the base address.
> Now if I have assigned ahb_buf_size to FSPI_FLSHXXCR0 register then for read/write data beyond limit of ahb_buf_size offset I get data corruption.
Why would you do that? We have the ->adjust_op_size() exactly for this
reason, so, if someone tries to do a spi_mem_op with data.nbytes >
ahb_buf_size you should return an error.
>
> Thus, for generic approach have assigned FSPI_FLSHXXCR0 equal to the memory mapped size to the controller. This would also not going to depend on the number of CS present on the target.
I kind of agree with Frieder on that one, I think it's preferable to
limit the per-read-op size to ahb_buf_size and let the upper layer
split the request in several sub-requests. On the controller side of
things, you just have to have a mapping of ahb_buf_size per-CS. If you
want to further optimize things, implement the dirmap hooks.
>
> > You are already aligning the AHB reads for this in nxp_fspi_adjust_op_size().
> >
> Yes, max read data size can be ahb_buf_size. Thus we need to check max read size with ahb_buf_size.
Well, it's never a bad thing to check it twice, just in case the
spi-mem user is misusing the API.
> > > +static void nxp_fspi_fill_txfifo(struct nxp_fspi *f,
> > > + const struct spi_mem_op *op)
> > > +{
> > > + void __iomem *base = f->iobase;
> > > + int i, j, ret;
> > > + int size, tmp_size, wm_size;
> > > + u32 data = 0;
> > > + u32 *txbuf = (u32 *) op->data.buf.out;
> > > +
> > > + /* clear the TX FIFO. */
> > > + fspi_writel(f, FSPI_IPTXFCR_CLR, base + FSPI_IPTXFCR);
> > > +
> > > + /* Default value of water mark level is 8 bytes. */
> > > + wm_size = 8;
> > > + size = op->data.nbytes / wm_size;
> > > + for (i = 0; i < size; i++) {
> > > + /* Wait for TXFIFO empty */
> > > + ret = fspi_readl_poll_tout(f, f->iobase + FSPI_INTR,
> > > + FSPI_INTR_IPTXWE, 0,
> > > + POLL_TOUT, true);
> > > + WARN_ON(ret);
> > > +
> > > + j = 0;
> > > + tmp_size = wm_size;
> > > + while (tmp_size > 0) {
> > > + data = 0;
> > > + memcpy(&data, txbuf, 4);
> > > + fspi_writel(f, data, base + FSPI_TFDR + j * 4);
> > > + tmp_size -= 4;
> > > + j++;
> > > + txbuf += 1;
> > > + }
> > > + fspi_writel(f, FSPI_INTR_IPTXWE, base + FSPI_INTR);
> > > + }
> > > +
> > > + size = op->data.nbytes % wm_size;
> > > + if (size) {
> > > + /* Wait for TXFIFO empty */
> > > + ret = fspi_readl_poll_tout(f, f->iobase + FSPI_INTR,
> > > + FSPI_INTR_IPTXWE, 0,
> > > + POLL_TOUT, true);
> > > + WARN_ON(ret);
> > > +
> > > + j = 0;
> > > + tmp_size = 0;
> > > + while (size > 0) {
> > > + data = 0;
> > > + tmp_size = (size < 4) ? size : 4;
> > > + memcpy(&data, txbuf, tmp_size);
> > > + fspi_writel(f, data, base + FSPI_TFDR + j * 4);
> > > + size -= tmp_size;
> > > + j++;
> > > + txbuf += 1;
> > > + }
> > > + fspi_writel(f, FSPI_INTR_IPTXWE, base + FSPI_INTR);
> > > + }
> >
> > All these nested loops to fill the TX buffer and also the ones below to read the
> > RX buffer look much more complicated than they should really be. Can you try to
> > make this more readable?
> Yes
> >
> > Maybe something like this would work:
> >
> > for (i = 0; i < ALIGN_DOWN(op->data.nbytes, 8); i += 8) {
> > /* Wait for TXFIFO empty */
> > ret = fspi_readl_poll_tout(f, f->iobase + FSPI_INTR,
> > FSPI_INTR_IPTXWE, 0,
> > POLL_TOUT, true);
> >
> > fspi_writel(f, op->data.buf.out + i, base + FSPI_TFDR);
> > fspi_writel(f, op->data.buf.out + i + 4, base + FSPI_TFDR + 4);
> > fspi_writel(f, FSPI_INTR_IPTXWE, base + FSPI_INTR); }
> With this above 2 lines we are hardcoding it for read/write with watermark size as 8 bytes.
> Watermark size can be variable and depends on the value of IPRXFCR/IPTXFCR register with default value as 8 bytes
> Thus, I would still prefer to use the internal for loop instead of 2 fspi_writel(...) for FSPI_TFDR and FSPI_TFDR + 4 register write commands.
Just like you're hardcoding wm_size to 8, so I don't see a difference
here. And I indeed prefer Frieder's version.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v3 7/8] arm64: KVM: Handle ARM erratum 1165522 in TLB invalidation
From: Christoffer Dall @ 2018-12-10 10:19 UTC (permalink / raw)
To: Marc Zyngier
Cc: Mark Rutland, kvm, Suzuki K Poulose, Catalin Marinas, Will Deacon,
James Morse, kvmarm, linux-arm-kernel
In-Reply-To: <20181206173126.139877-8-marc.zyngier@arm.com>
On Thu, Dec 06, 2018 at 05:31:25PM +0000, Marc Zyngier wrote:
> In order to avoid TLB corruption whilst invalidating TLBs on CPUs
> affected by erratum 1165522, we need to prevent S1 page tables
> from being usable.
>
> For this, we set the EL1 S1 MMU on, and also disable the page table
> walker (by setting the TCR_EL1.EPD* bits to 1).
>
> This ensures that once we switch to the EL1/EL0 translation regime,
> speculated AT instructions won't be able to parse the page tables.
>
> Reviewed-by: James Morse <james.morse@arm.com>
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> ---
> arch/arm64/kvm/hyp/tlb.c | 66 +++++++++++++++++++++++++++++++---------
> 1 file changed, 51 insertions(+), 15 deletions(-)
>
> diff --git a/arch/arm64/kvm/hyp/tlb.c b/arch/arm64/kvm/hyp/tlb.c
> index 7fcc9c1a5f45..ec157543d5a9 100644
> --- a/arch/arm64/kvm/hyp/tlb.c
> +++ b/arch/arm64/kvm/hyp/tlb.c
> @@ -21,12 +21,36 @@
> #include <asm/kvm_mmu.h>
> #include <asm/tlbflush.h>
>
> +struct tlb_inv_context {
> + unsigned long flags;
> + u64 tcr;
> + u64 sctlr;
> +};
> +
> static void __hyp_text __tlb_switch_to_guest_vhe(struct kvm *kvm,
> - unsigned long *flags)
> + struct tlb_inv_context *cxt)
> {
> u64 val;
>
> - local_irq_save(*flags);
> + local_irq_save(cxt->flags);
> +
> + if (cpus_have_const_cap(ARM64_WORKAROUND_1165522)) {
> + /*
> + * For CPUs that are affected by ARM erratum 1165522, we
> + * cannot trust stage-1 to be in a correct state at that
> + * point. Since we do not want to force a full load of the
> + * vcpu state, we prevent the EL1 page-table walker to
> + * allocate new TLBs. This is done by setting the EPD bits
> + * in the TCR_EL1 register. We also need to prevent it to
> + * allocate IPA->PA walks, so we enable the S1 MMU...
> + */
> + val = cxt->tcr = read_sysreg_el1(tcr);
> + val |= TCR_EPD1_MASK | TCR_EPD0_MASK;
> + write_sysreg_el1(val, tcr);
> + val = cxt->sctlr = read_sysreg_el1(sctlr);
> + val |= SCTLR_ELx_M;
> + write_sysreg_el1(val, sctlr);
> + }
>
> /*
> * With VHE enabled, we have HCR_EL2.{E2H,TGE} = {1,1}, and
> @@ -34,6 +58,11 @@ static void __hyp_text __tlb_switch_to_guest_vhe(struct kvm *kvm,
> * guest TLBs (EL1/EL0), we need to change one of these two
> * bits. Changing E2H is impossible (goodbye TTBR1_EL2), so
> * let's flip TGE before executing the TLB operation.
> + *
> + * ARM erratum 1165522 requires some special handling (again),
> + * as we need to make sure both stages of translation are in
> + * place before clearing TGE. __load_guest_stage2() already
> + * has an ISB in order to deal with this.
> */
> __load_guest_stage2(kvm);
> val = read_sysreg(hcr_el2);
> @@ -43,7 +72,7 @@ static void __hyp_text __tlb_switch_to_guest_vhe(struct kvm *kvm,
> }
>
> static void __hyp_text __tlb_switch_to_guest_nvhe(struct kvm *kvm,
> - unsigned long *flags)
> + struct tlb_inv_context *cxt)
> {
> __load_guest_stage2(kvm);
> isb();
> @@ -55,7 +84,7 @@ static hyp_alternate_select(__tlb_switch_to_guest,
> ARM64_HAS_VIRT_HOST_EXTN);
>
> static void __hyp_text __tlb_switch_to_host_vhe(struct kvm *kvm,
> - unsigned long flags)
> + struct tlb_inv_context *cxt)
> {
> /*
> * We're done with the TLB operation, let's restore the host's
> @@ -64,11 +93,18 @@ static void __hyp_text __tlb_switch_to_host_vhe(struct kvm *kvm,
> write_sysreg(0, vttbr_el2);
> write_sysreg(HCR_HOST_VHE_FLAGS, hcr_el2);
> isb();
> - local_irq_restore(flags);
> +
> + if (cpus_have_const_cap(ARM64_WORKAROUND_1165522)) {
> + /* Restore the guest's registers to what they were */
host's ?
> + write_sysreg_el1(cxt->tcr, tcr);
> + write_sysreg_el1(cxt->sctlr, sctlr);
> + }
> +
> + local_irq_restore(cxt->flags);
> }
>
> static void __hyp_text __tlb_switch_to_host_nvhe(struct kvm *kvm,
> - unsigned long flags)
> + struct tlb_inv_context *cxt)
> {
> write_sysreg(0, vttbr_el2);
> }
> @@ -80,13 +116,13 @@ static hyp_alternate_select(__tlb_switch_to_host,
>
> void __hyp_text __kvm_tlb_flush_vmid_ipa(struct kvm *kvm, phys_addr_t ipa)
> {
> - unsigned long flags;
> + struct tlb_inv_context cxt;
>
> dsb(ishst);
>
> /* Switch to requested VMID */
> kvm = kern_hyp_va(kvm);
> - __tlb_switch_to_guest()(kvm, &flags);
> + __tlb_switch_to_guest()(kvm, &cxt);
>
> /*
> * We could do so much better if we had the VA as well.
> @@ -129,39 +165,39 @@ void __hyp_text __kvm_tlb_flush_vmid_ipa(struct kvm *kvm, phys_addr_t ipa)
> if (!has_vhe() && icache_is_vpipt())
> __flush_icache_all();
>
> - __tlb_switch_to_host()(kvm, flags);
> + __tlb_switch_to_host()(kvm, &cxt);
> }
>
> void __hyp_text __kvm_tlb_flush_vmid(struct kvm *kvm)
> {
> - unsigned long flags;
> + struct tlb_inv_context cxt;
>
> dsb(ishst);
>
> /* Switch to requested VMID */
> kvm = kern_hyp_va(kvm);
> - __tlb_switch_to_guest()(kvm, &flags);
> + __tlb_switch_to_guest()(kvm, &cxt);
>
> __tlbi(vmalls12e1is);
> dsb(ish);
> isb();
>
> - __tlb_switch_to_host()(kvm, flags);
> + __tlb_switch_to_host()(kvm, &cxt);
> }
>
> void __hyp_text __kvm_tlb_flush_local_vmid(struct kvm_vcpu *vcpu)
> {
> struct kvm *kvm = kern_hyp_va(kern_hyp_va(vcpu)->kvm);
> - unsigned long flags;
> + struct tlb_inv_context cxt;
>
> /* Switch to requested VMID */
> - __tlb_switch_to_guest()(kvm, &flags);
> + __tlb_switch_to_guest()(kvm, &cxt);
>
> __tlbi(vmalle1);
> dsb(nsh);
> isb();
>
> - __tlb_switch_to_host()(kvm, flags);
> + __tlb_switch_to_host()(kvm, &cxt);
> }
>
> void __hyp_text __kvm_flush_vm_context(void)
> --
> 2.19.2
>
Otherwise:
Acked-by: Christoffer Dall <christoffer.dall@arm.com>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: TK1: DRM, Nouveau and VIC
From: Thierry Reding @ 2018-12-10 10:21 UTC (permalink / raw)
To: Marcel Ziswiler, Ben Skeggs
Cc: nouveau@lists.freedesktop.org, dri-devel@lists.freedesktop.org,
mperttunen@nvidia.com, linux-tegra@vger.kernel.org,
jonathanh@nvidia.com, linux-arm-kernel@lists.infradead.org
In-Reply-To: <0c2db0d1c102a4f5a441f8e5a90f6793f329ada7.camel@toradex.com>
[-- Attachment #1.1: Type: text/plain, Size: 8876 bytes --]
On Sat, Dec 08, 2018 at 02:54:45PM +0000, Marcel Ziswiler wrote:
> Hi Thierry et al.
>
> I noticed that since commit 3dde5a2342cd ("ARM: tegra: Add VIC on
> Tegra124") graphics on Apalis TK1 is broken. During boot it fails
> loading the vic firmware:
>
> [ 1.595824] tegra-vic 54340000.vic: Direct firmware load for
> nvidia/tegra124/vic03_ucode.bin failed with error -2
> [ 1.606140] tegra-vic: probe of 54340000.vic failed with error -2
>
> Subsequently Tegra HDMI seems to fail completely:
>
> [ 2.379860] tegra-hdmi 54280000.hdmi: failed to get PLL regulator
>
> And finally, Nouveau even crashes:
>
> [ 8.241115] nouveau 57000000.gpu: Linked as a consumer to
> regulator.31
> [ 8.247889] nouveau 57000000.gpu: NVIDIA GK20A (0ea000a1)
> [ 8.253396] nouveau 57000000.gpu: imem: using IOMMU
> [ 8.270210] Unable to handle kernel NULL pointer dereference at
> virtual address 0000006c
> [ 8.278340] pgd = (ptrval)
> [ 8.281250] [0000006c] *pgd=00000000
> [ 8.284944] Internal error: Oops: 5 [#1] PREEMPT SMP ARM
> [ 8.290260] Modules linked in: nouveau(+) ttm
> [ 8.294625] CPU: 2 PID: 203 Comm: systemd-udevd Not tainted 4.20.0-
> rc5-next-20181207-00008-g85b0f8e25f86-dirty #110
> [ 8.305055] Hardware name: NVIDIA Tegra SoC (Flattened Device Tree)
> [ 8.311331] PC is at drm_plane_register_all+0x18/0x50
> [ 8.316373] LR is at drm_modeset_register_all+0xc/0x70
> [ 8.321513] pc : [<c056200c>] lr : [<c0564cc8>] psr: a0060013
> [ 8.327768] sp : ed527c70 ip : ecc43ec0 fp : 00000000
> [ 8.332993] r10: 00000016 r9 : ecc43e80 r8 : 00000000
> [ 8.338209] r7 : bf182c80 r6 : 00000000 r5 : ed61b24c r4 :
> fffffffc
> [ 8.344735] r3 : 0002f000 r2 : ffffffff r1 : 2e124000 r0 :
> ed61b000
> [ 8.351260] Flags: NzCv IRQs on FIQs on Mode SVC_32 ISA
> ARM Segment none
> [ 8.358383] Control: 10c5387d Table: ad64c06a DAC: 00000051
> [ 8.364127] Process systemd-udevd (pid: 203, stack limit =
> 0x(ptrval))
> [ 8.370654] Stack: (0xed527c70 to 0xed528000)
> [ 8.375004] 7c60: ed61b000
> ed61b000 00000000 c0564cc8
> [ 8.383177] 7c80: ed61b000 00000000 00000000 c054b5b8 00000001
> 00000001 ffffffff ffffffff
> [ 8.391355] 7ca0: ed527cc0 c0f08c48 ed61b000 00000000 00000000
> 00000000 bf180c5c bf0dc900
> [ 8.399531] 7cc0: eda29208 5dfe844b 00000000 ee9f2a10 00000000
> bf180c5c 00000000 c05a9328
> [ 8.407695] 7ce0: c1006828 ee9f2a10 c100682c 00000000 00000000
> c05a744c ee9f2a10 bf180c5c
> [ 8.415871] 7d00: ee9f2a44 c05a77a8 00000000 c0f08c48 bf182980
> c05a769c eefd14d0 c05a77a8
> [ 8.424048] 7d20: 00000000 ee9f2a10 bf180c5c ee9f2a44 c05a77a8
> 00000000 c0f08c48 bf182980
> [ 8.432226] 7d40: 00000000 c05a7884 ee9ebfb4 c0f08c48 bf180c5c
> c05a5790 00000000 ee88135c
> [ 8.440405] 7d60: ee9ebfb4 5dfe844b c0f71168 bf180c5c ee379e80
> c0f71168 00000000 c05a692c
> [ 8.448570] 7d80: bf15dc00 bf180ac8 ffffe000 bf180c5c bf180ac8
> ffffe000 bf1aa000 c05a84a0
> [ 8.456746] 7da0: bf182b80 bf180ac8 ffffe000 bf1aa170 c0fbd220
> c0f08c48 ffffe000 c0102ed0
> [ 8.464924] 7dc0: ed53f4c0 006000c0 c01b3d98 0000000c 60000113
> bf182980 00000040 c02592d0
> [ 8.473102] 7de0: eda60200 2e124000 ee800000 006000c0 006000c0
> c01b3d98 0000000c c025a8cc
> [ 8.481281] 7e00: c024ce54 a0000113 bf182980 5dfe844b bf182980
> 00000002 ed53f4c0 00000002
> [ 8.489459] 7e20: eceba000 c01b3dd4 c0f08c48 bf182980 00000000
> ed527f40 00000002 eceb9fc0
> [ 8.497625] 7e40: 00000002 c01b61a4 bf18298c 00007fff bf182980
> c01b2f88 00000000 c01b279c
> [ 8.505800] 7e60: bf1829c8 bf182a80 bf182b6c bf182ab0 c0b03ab0
> c0d58964 c0ca726c c0ca7278
> [ 8.513978] 7e80: c0ca72d0 c0f08c48 00000000 c02654a0 00000000
> 00000000 ffffe000 bf000000
> [ 8.522157] 7ea0: 00000000 00000000 00000000 00000000 00000000
> 00000000 6e72656b 00006c65
> [ 8.530336] 7ec0: 00000000 00000000 00000000 00000000 00000000
> 00000000 00000000 00000000
> [ 8.538502] 7ee0: 00000000 00000000 00000000 00000000 00000000
> 5dfe844b 7fffffff c0f08c48
> [ 8.546677] 7f00: 00000000 0000000f b6f761cc c0101204 ed526000
> 0000017b 004a3270 c01b66a4
> [ 8.554855] 7f20: 7fffffff 00000000 00000003 00000001 004a3270
> f0ced000 06e8994c 00000000
> [ 8.563032] 7f40: f0e37f3a f0e50a40 f0ced000 06e8994c f7b75f9c
> f7b75d34 f63e62dc 0016b000
> [ 8.571209] 7f60: 0017f6f0 00000000 00000000 00000000 00050a48
> 0000003b 0000003c 00000023
> [ 8.579388] 7f80: 00000000 00000014 00000000 5dfe844b 00000000
> 004c0ec0 00000000 00000001
> [ 8.587554] 7fa0: 0000017b c0101000 004c0ec0 00000000 0000000f
> b6f761cc 00000000 00020000
> [ 8.595730] 7fc0: 004c0ec0 00000000 00000001 0000017b 0048e114
> 00000000 00000000 004a3270
> [ 8.603908] 7fe0: bea8f990 bea8f980 b6f71269 b6e9f6c0 400d0010
> 0000000f 00000000 00000000
> [ 8.612096] [<c056200c>] (drm_plane_register_all) from [<c0564cc8>]
> (drm_modeset_register_all+0xc/0x70)
> [ 8.621499] [<c0564cc8>] (drm_modeset_register_all) from
> [<c054b5b8>] (drm_dev_register+0x168/0x1c4)
> [ 8.630855] [<c054b5b8>] (drm_dev_register) from [<bf0dc900>]
> (nouveau_platform_probe+0x6c/0x88 [nouveau])
> [ 8.640739] [<bf0dc900>] (nouveau_platform_probe [nouveau]) from
> [<c05a9328>] (platform_drv_probe+0x48/0x98)
> [ 8.650574] [<c05a9328>] (platform_drv_probe) from [<c05a744c>]
> (really_probe+0x1e0/0x2cc)
> [ 8.658827] [<c05a744c>] (really_probe) from [<c05a769c>]
> (driver_probe_device+0x60/0x16c)
> [ 8.667096] [<c05a769c>] (driver_probe_device) from [<c05a7884>]
> (__driver_attach+0xdc/0xe0)
> [ 8.675543] [<c05a7884>] (__driver_attach) from [<c05a5790>]
> (bus_for_each_dev+0x74/0xb4)
> [ 8.683729] [<c05a5790>] (bus_for_each_dev) from [<c05a692c>]
> (bus_add_driver+0x1c0/0x204)
> [ 8.692004] [<c05a692c>] (bus_add_driver) from [<c05a84a0>]
> (driver_register+0x74/0x108)
> [ 8.700324] [<c05a84a0>] (driver_register) from [<bf1aa170>]
> (nouveau_drm_init+0x170/0x1000 [nouveau])
> [ 8.709857] [<bf1aa170>] (nouveau_drm_init [nouveau]) from
> [<c0102ed0>] (do_one_initcall+0x54/0x284)
> [ 8.718980] [<c0102ed0>] (do_one_initcall) from [<c01b3dd4>]
> (do_init_module+0x64/0x214)
> [ 8.727079] [<c01b3dd4>] (do_init_module) from [<c01b61a4>]
> (load_module+0x21b8/0x246c)
> [ 8.735094] [<c01b61a4>] (load_module) from [<c01b66a4>]
> (sys_finit_module+0xc4/0xdc)
> [ 8.742937] [<c01b66a4>] (sys_finit_module) from [<c0101000>]
> (ret_fast_syscall+0x0/0x54)
> [ 8.751114] Exception stack(0xed527fa8 to 0xed527ff0)
> [ 8.756157] 7fa0: 004c0ec0 00000000 0000000f
> b6f761cc 00000000 00020000
> [ 8.764333] 7fc0: 004c0ec0 00000000 00000001 0000017b 0048e114
> 00000000 00000000 004a3270
> [ 8.772510] 7fe0: bea8f990 bea8f980 b6f71269 b6e9f6c0
> [ 8.777556] Code: e5b5424c e1550004 0a00000c e2444004 (e5943070)
> [ 8.784011] ---[ end trace ad8c21587c118655 ]---
>
> Of course my root file system does include resp. vic firmware:
>
> 7ef01d2e3f507c91ca79584e89edcc64 /lib/firmware/nvidia/tegra124/vic03_u
> code.bin
>
> If I bake that one into the kernel binary, Nouveau still crashes like
> above albeit VIC loading and Tegra DRM now at least showing something
> on HDMI.
Yeah, this is a fairly common pitfall. The general rule of thumb is that
the firmware has to live on the same medium as the module. So if you've
built Tegra DRM as a loadable kernel module and installed it in the root
filesystem, then that's where your firmware file also needs to be. If
the driver is built-in (or a loadable module installed in the initial
ramdisk), then the firmware needs to be in the initial ramdisk (or built
into the kernel image itself). That's somewhat annoying, but it is what
it is. At least it's logical.
> Just reverting above mentioned commit still leaves Nouveau crashing.
>
> This has been observed using latest next-20181207.
>
> Does anybody know what exactly is going on and how exactly one may get
> graphics working again as before?
So this is something that should be fixed by this:
https://patchwork.freedesktop.org/patch/260547/
And there's another patch that fixes a subsequent crash when you
actually start to use the GPU:
https://patchwork.freedesktop.org/patch/263588/
It'd be great if you could apply both and verify that they fix the crash
for you. If so, can you provide a Tested-by? Both were Cc'ed to
linux-tegra, so you should have a copy to reply to. If not, let me know
and I can bounce it.
Ben, can you pick up the two patches above? They're kind of high-
priority because they fix issues that crept into v4.20-rc1, so should
ideally be fixed before v4.20 final.
Thierry
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
[-- Attachment #2: Type: text/plain, Size: 176 bytes --]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH 7/7] drm: Split out drm_probe_helper.h
From: Thierry Reding @ 2018-12-10 10:24 UTC (permalink / raw)
To: Daniel Vetter
Cc: linux-samsung-soc, nouveau, Daniel Vetter, linux-arm-msm,
Intel Graphics Development, etnaviv, amd-gfx, virtualization,
linux-renesas-soc, linux-rockchip, linux-mediatek,
DRI Development, linux-amlogic, linux-tegra, spice-devel,
xen-devel, freedreno, linux-stm32, linux-arm-kernel
In-Reply-To: <20181210101133.5364-1-daniel.vetter@ffwll.ch>
[-- Attachment #1.1: Type: text/plain, Size: 13396 bytes --]
On Mon, Dec 10, 2018 at 11:11:33AM +0100, Daniel Vetter wrote:
> Having the probe helper stuff (which pretty much everyone needs) in
> the drm_crtc_helper.h file (which atomic drivers should never need) is
> confusing. Split them out.
>
> To make sure I actually achieved the goal here I went through all
> drivers. And indeed, all atomic drivers are now free of
> drm_crtc_helper.h includes.
>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: virtualization@lists.linux-foundation.org
> Cc: etnaviv@lists.freedesktop.org
> Cc: linux-samsung-soc@vger.kernel.org
> Cc: intel-gfx@lists.freedesktop.org
> Cc: linux-mediatek@lists.infradead.org
> Cc: linux-amlogic@lists.infradead.org
> Cc: linux-arm-msm@vger.kernel.org
> Cc: freedreno@lists.freedesktop.org
> Cc: nouveau@lists.freedesktop.org
> Cc: spice-devel@lists.freedesktop.org
> Cc: amd-gfx@lists.freedesktop.org
> Cc: linux-renesas-soc@vger.kernel.org
> Cc: linux-rockchip@lists.infradead.org
> Cc: linux-stm32@st-md-mailman.stormreply.com
> Cc: linux-tegra@vger.kernel.org
> Cc: xen-devel@lists.xen.org
> ---
> .../gpu/drm/amd/amdgpu/amdgpu_connectors.c | 2 +-
> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 2 +-
> drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 2 +-
> drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h | 1 +
> .../amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 2 +-
> .../amd/display/amdgpu_dm/amdgpu_dm_pp_smu.c | 2 +-
> .../display/amdgpu_dm/amdgpu_dm_services.c | 2 +-
> drivers/gpu/drm/arc/arcpgu_crtc.c | 2 +-
> drivers/gpu/drm/arc/arcpgu_drv.c | 2 +-
> drivers/gpu/drm/arc/arcpgu_sim.c | 2 +-
> drivers/gpu/drm/arm/hdlcd_crtc.c | 2 +-
> drivers/gpu/drm/arm/hdlcd_drv.c | 2 +-
> drivers/gpu/drm/arm/malidp_crtc.c | 2 +-
> drivers/gpu/drm/arm/malidp_drv.c | 2 +-
> drivers/gpu/drm/arm/malidp_mw.c | 2 +-
> drivers/gpu/drm/armada/armada_510.c | 2 +-
> drivers/gpu/drm/armada/armada_crtc.c | 2 +-
> drivers/gpu/drm/armada/armada_drv.c | 2 +-
> drivers/gpu/drm/armada/armada_fb.c | 2 +-
> drivers/gpu/drm/ast/ast_drv.c | 1 +
> drivers/gpu/drm/ast/ast_mode.c | 1 +
> .../gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c | 2 +-
> drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h | 2 +-
> drivers/gpu/drm/bochs/bochs_drv.c | 1 +
> drivers/gpu/drm/bochs/bochs_kms.c | 1 +
> drivers/gpu/drm/bridge/adv7511/adv7511.h | 2 +-
> drivers/gpu/drm/bridge/analogix-anx78xx.c | 3 +-
> .../drm/bridge/analogix/analogix_dp_core.c | 2 +-
> drivers/gpu/drm/bridge/cdns-dsi.c | 2 +-
> drivers/gpu/drm/bridge/dumb-vga-dac.c | 2 +-
> .../bridge/megachips-stdpxxxx-ge-b850v3-fw.c | 2 +-
> drivers/gpu/drm/bridge/nxp-ptn3460.c | 2 +-
> drivers/gpu/drm/bridge/panel.c | 2 +-
> drivers/gpu/drm/bridge/parade-ps8622.c | 2 +-
> drivers/gpu/drm/bridge/sii902x.c | 2 +-
> drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 2 +-
> drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c | 2 +-
> drivers/gpu/drm/bridge/tc358764.c | 2 +-
> drivers/gpu/drm/bridge/tc358767.c | 2 +-
> drivers/gpu/drm/bridge/ti-sn65dsi86.c | 2 +-
> drivers/gpu/drm/bridge/ti-tfp410.c | 2 +-
> drivers/gpu/drm/cirrus/cirrus_drv.c | 1 +
> drivers/gpu/drm/cirrus/cirrus_mode.c | 1 +
> drivers/gpu/drm/drm_atomic_helper.c | 1 -
> drivers/gpu/drm/drm_dp_mst_topology.c | 2 +-
> drivers/gpu/drm/drm_modeset_helper.c | 2 +-
> drivers/gpu/drm/drm_probe_helper.c | 2 +-
> drivers/gpu/drm/drm_simple_kms_helper.c | 2 +-
> drivers/gpu/drm/etnaviv/etnaviv_drv.h | 1 -
> drivers/gpu/drm/exynos/exynos_dp.c | 2 +-
> drivers/gpu/drm/exynos/exynos_drm_crtc.c | 2 +-
> drivers/gpu/drm/exynos/exynos_drm_dpi.c | 2 +-
> drivers/gpu/drm/exynos/exynos_drm_drv.c | 2 +-
> drivers/gpu/drm/exynos/exynos_drm_dsi.c | 2 +-
> drivers/gpu/drm/exynos/exynos_drm_fb.c | 2 +-
> drivers/gpu/drm/exynos/exynos_drm_fbdev.c | 2 +-
> drivers/gpu/drm/exynos/exynos_drm_vidi.c | 2 +-
> drivers/gpu/drm/exynos/exynos_hdmi.c | 2 +-
> drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c | 2 +-
> drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c | 2 +-
> drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_kms.c | 2 +-
> drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c | 2 +-
> drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c | 2 +-
> drivers/gpu/drm/gma500/psb_intel_drv.h | 1 +
> .../gpu/drm/hisilicon/hibmc/hibmc_drm_de.c | 2 +-
> .../gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c | 2 +-
> .../gpu/drm/hisilicon/hibmc/hibmc_drm_fbdev.c | 2 +-
> .../gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c | 2 +-
> drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c | 2 +-
> .../gpu/drm/hisilicon/kirin/kirin_drm_ade.c | 2 +-
> .../gpu/drm/hisilicon/kirin/kirin_drm_drv.c | 2 +-
> drivers/gpu/drm/i2c/ch7006_priv.h | 2 +-
> drivers/gpu/drm/i2c/sil164_drv.c | 2 +-
> drivers/gpu/drm/i2c/tda998x_drv.c | 2 +-
> drivers/gpu/drm/i915/i915_drv.c | 2 +-
> drivers/gpu/drm/i915/intel_crt.c | 2 +-
> drivers/gpu/drm/i915/intel_display.c | 2 +-
> drivers/gpu/drm/i915/intel_dp.c | 2 +-
> drivers/gpu/drm/i915/intel_dp_mst.c | 2 +-
> drivers/gpu/drm/i915/intel_drv.h | 2 +-
> drivers/gpu/drm/imx/dw_hdmi-imx.c | 2 +-
> drivers/gpu/drm/imx/imx-drm-core.c | 2 +-
> drivers/gpu/drm/imx/imx-ldb.c | 2 +-
> drivers/gpu/drm/imx/imx-tve.c | 2 +-
> drivers/gpu/drm/imx/ipuv3-crtc.c | 2 +-
> drivers/gpu/drm/imx/parallel-display.c | 2 +-
> drivers/gpu/drm/mediatek/mtk_dpi.c | 2 +-
> drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 2 +-
> drivers/gpu/drm/mediatek/mtk_drm_drv.c | 2 +-
> drivers/gpu/drm/mediatek/mtk_drm_fb.c | 2 +-
> drivers/gpu/drm/mediatek/mtk_dsi.c | 2 +-
> drivers/gpu/drm/mediatek/mtk_hdmi.c | 2 +-
> drivers/gpu/drm/meson/meson_crtc.c | 2 +-
> drivers/gpu/drm/meson/meson_drv.c | 2 +-
> drivers/gpu/drm/meson/meson_dw_hdmi.c | 2 +-
> drivers/gpu/drm/meson/meson_venc_cvbs.c | 2 +-
> drivers/gpu/drm/mgag200/mgag200_mode.c | 1 +
> drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 2 +-
> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 2 +-
> drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c | 2 +-
> .../gpu/drm/msm/disp/mdp4/mdp4_dsi_encoder.c | 2 +-
> .../gpu/drm/msm/disp/mdp4/mdp4_dtv_encoder.c | 2 +-
> .../gpu/drm/msm/disp/mdp4/mdp4_lcdc_encoder.c | 2 +-
> .../gpu/drm/msm/disp/mdp5/mdp5_cmd_encoder.c | 2 +-
> drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c | 2 +-
> drivers/gpu/drm/msm/disp/mdp5/mdp5_encoder.c | 2 +-
> drivers/gpu/drm/msm/msm_drv.h | 2 +-
> drivers/gpu/drm/msm/msm_fb.c | 2 +-
> drivers/gpu/drm/mxsfb/mxsfb_crtc.c | 2 +-
> drivers/gpu/drm/mxsfb/mxsfb_drv.c | 2 +-
> drivers/gpu/drm/mxsfb/mxsfb_out.c | 2 +-
> drivers/gpu/drm/nouveau/dispnv04/tvnv17.c | 1 +
> drivers/gpu/drm/nouveau/dispnv50/disp.c | 2 +-
> drivers/gpu/drm/nouveau/nouveau_connector.c | 1 +
> drivers/gpu/drm/nouveau/nouveau_display.c | 1 +
> drivers/gpu/drm/omapdrm/omap_connector.c | 2 +-
> drivers/gpu/drm/omapdrm/omap_crtc.c | 2 +-
> drivers/gpu/drm/omapdrm/omap_drv.c | 2 +-
> drivers/gpu/drm/omapdrm/omap_drv.h | 2 +-
> drivers/gpu/drm/omapdrm/omap_encoder.c | 2 +-
> drivers/gpu/drm/omapdrm/omap_fb.c | 2 +-
> drivers/gpu/drm/pl111/pl111_drv.c | 2 +-
> drivers/gpu/drm/qxl/qxl_display.c | 2 +-
> drivers/gpu/drm/qxl/qxl_drv.c | 3 +-
> drivers/gpu/drm/qxl/qxl_fb.c | 2 +-
> drivers/gpu/drm/qxl/qxl_kms.c | 2 +-
> drivers/gpu/drm/radeon/radeon_acpi.c | 1 +
> drivers/gpu/drm/radeon/radeon_connectors.c | 1 +
> drivers/gpu/drm/radeon/radeon_device.c | 1 +
> drivers/gpu/drm/radeon/radeon_display.c | 1 +
> drivers/gpu/drm/radeon/radeon_dp_mst.c | 1 +
> drivers/gpu/drm/radeon/radeon_drv.c | 1 +
> drivers/gpu/drm/radeon/radeon_irq_kms.c | 1 +
> drivers/gpu/drm/rcar-du/rcar_du_crtc.c | 2 +-
> drivers/gpu/drm/rcar-du/rcar_du_drv.c | 2 +-
> drivers/gpu/drm/rcar-du/rcar_du_encoder.c | 2 +-
> drivers/gpu/drm/rcar-du/rcar_du_kms.c | 2 +-
> drivers/gpu/drm/rcar-du/rcar_du_plane.c | 2 +-
> drivers/gpu/drm/rcar-du/rcar_du_vsp.c | 2 +-
> drivers/gpu/drm/rcar-du/rcar_lvds.c | 2 +-
> .../gpu/drm/rockchip/analogix_dp-rockchip.c | 2 +-
> drivers/gpu/drm/rockchip/cdn-dp-core.c | 2 +-
> drivers/gpu/drm/rockchip/cdn-dp-core.h | 2 +-
> drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c | 2 +-
> drivers/gpu/drm/rockchip/inno_hdmi.c | 2 +-
> drivers/gpu/drm/rockchip/rockchip_drm_drv.c | 2 +-
> drivers/gpu/drm/rockchip/rockchip_drm_fb.c | 2 +-
> drivers/gpu/drm/rockchip/rockchip_drm_fbdev.c | 2 +-
> drivers/gpu/drm/rockchip/rockchip_drm_psr.c | 2 +-
> drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 2 +-
> drivers/gpu/drm/rockchip/rockchip_lvds.c | 2 +-
> drivers/gpu/drm/rockchip/rockchip_rgb.c | 2 +-
> drivers/gpu/drm/sti/sti_crtc.c | 2 +-
> drivers/gpu/drm/sti/sti_drv.c | 2 +-
> drivers/gpu/drm/sti/sti_dvo.c | 2 +-
> drivers/gpu/drm/sti/sti_hda.c | 2 +-
> drivers/gpu/drm/sti/sti_hdmi.c | 2 +-
> drivers/gpu/drm/sti/sti_tvout.c | 2 +-
> drivers/gpu/drm/stm/drv.c | 2 +-
> drivers/gpu/drm/stm/ltdc.c | 2 +-
> drivers/gpu/drm/sun4i/sun4i_backend.c | 2 +-
> drivers/gpu/drm/sun4i/sun4i_crtc.c | 2 +-
> drivers/gpu/drm/sun4i/sun4i_drv.c | 2 +-
> drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c | 2 +-
> drivers/gpu/drm/sun4i/sun4i_lvds.c | 2 +-
> drivers/gpu/drm/sun4i/sun4i_rgb.c | 2 +-
> drivers/gpu/drm/sun4i/sun4i_tcon.c | 2 +-
> drivers/gpu/drm/sun4i/sun4i_tv.c | 2 +-
> drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 2 +-
> drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c | 2 +-
> drivers/gpu/drm/sun4i/sun8i_mixer.c | 2 +-
> drivers/gpu/drm/sun4i/sun8i_ui_layer.c | 2 +-
> drivers/gpu/drm/sun4i/sun8i_vi_layer.c | 2 +-
> drivers/gpu/drm/tegra/drm.h | 2 +-
> drivers/gpu/drm/tegra/hdmi.c | 2 +-
> drivers/gpu/drm/tegra/hub.c | 2 +-
> drivers/gpu/drm/tinydrm/core/tinydrm-core.c | 2 +-
> drivers/gpu/drm/tinydrm/core/tinydrm-pipe.c | 2 +-
> drivers/gpu/drm/tve200/tve200_drv.c | 2 +-
> drivers/gpu/drm/udl/udl_connector.c | 1 +
> drivers/gpu/drm/udl/udl_drv.c | 1 +
> drivers/gpu/drm/udl/udl_main.c | 1 +
> drivers/gpu/drm/vc4/vc4_crtc.c | 2 +-
> drivers/gpu/drm/vc4/vc4_dpi.c | 2 +-
> drivers/gpu/drm/vc4/vc4_dsi.c | 2 +-
> drivers/gpu/drm/vc4/vc4_hdmi.c | 2 +-
> drivers/gpu/drm/vc4/vc4_kms.c | 2 +-
> drivers/gpu/drm/vc4/vc4_txp.c | 2 +-
> drivers/gpu/drm/vc4/vc4_vec.c | 2 +-
> drivers/gpu/drm/virtio/virtgpu_display.c | 2 +-
> drivers/gpu/drm/virtio/virtgpu_drv.h | 2 +-
> drivers/gpu/drm/vkms/vkms_crtc.c | 2 +-
> drivers/gpu/drm/vkms/vkms_drv.c | 2 +-
> drivers/gpu/drm/vkms/vkms_output.c | 2 +-
> drivers/gpu/drm/vmwgfx/vmwgfx_kms.h | 2 +-
> drivers/gpu/drm/xen/xen_drm_front.c | 2 +-
> drivers/gpu/drm/xen/xen_drm_front_conn.c | 2 +-
> drivers/gpu/drm/xen/xen_drm_front_gem.c | 2 +-
> drivers/gpu/drm/xen/xen_drm_front_kms.c | 2 +-
> drivers/gpu/drm/zte/zx_drm_drv.c | 2 +-
> drivers/gpu/drm/zte/zx_hdmi.c | 2 +-
> drivers/gpu/drm/zte/zx_tvenc.c | 2 +-
> drivers/gpu/drm/zte/zx_vga.c | 2 +-
> drivers/gpu/drm/zte/zx_vou.c | 2 +-
> drivers/staging/vboxvideo/vbox_irq.c | 2 +-
> drivers/staging/vboxvideo/vbox_mode.c | 2 +-
> include/drm/drm_crtc_helper.h | 16 ------
> include/drm/drm_probe_helper.h | 50 +++++++++++++++++++
> 208 files changed, 256 insertions(+), 200 deletions(-)
> create mode 100644 include/drm/drm_probe_helper.h
Looks good to me:
Acked-by: Thierry Reding <treding@nvidia.com>
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
[-- Attachment #2: Type: text/plain, Size: 176 bytes --]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v3 1/8] arm64: KVM: Make VHE Stage-2 TLB invalidation operations non-interruptible
From: Marc Zyngier @ 2018-12-10 10:24 UTC (permalink / raw)
To: Christoffer Dall
Cc: Mark Rutland, kvm, Suzuki K Poulose, Catalin Marinas, Will Deacon,
James Morse, kvmarm, linux-arm-kernel
In-Reply-To: <20181210100356.GG30263@e113682-lin.lund.arm.com>
Hi Christoffer,
On 10/12/2018 10:03, Christoffer Dall wrote:
> On Thu, Dec 06, 2018 at 05:31:19PM +0000, Marc Zyngier wrote:
>> Contrary to the non-VHE version of the TLB invalidation helpers, the VHE
>> code has interrupts enabled, meaning that we can take an interrupt in
>> the middle of such a sequence, and start running something else with
>> HCR_EL2.TGE cleared.
>
> Do we have to clear TGE to perform the TLB invalidation, or is that just
> a side-effect of re-using code?
We really do need to clear TGE. From the description of TLBI VMALLE1IS:
<quote>
When EL2 is implemented and enabled in the current Security state:
— If HCR_EL2.{E2H, TGE} is not {1, 1}, the entry would be used with the
current VMID and would be required to translate the specified VA using
the EL1&0 translation regime.
— If HCR_EL2.{E2H, TGE} is {1, 1}, the entry would be required to
translate the specified VA using the EL2&0 translation regime.
</quote>
> Also, do we generally perform TLB invalidations in the kernel with
> interrupts disabled, or is this just a result of clearing TGE?
That's definitely a result of clearing TGE. We could be taking an
interrupt here, and execute a user access on the back of it (perf will
happily walk a user-space stack in that context, for example). Having
TGE clear in that context. An alternative solution would be to
save/restore TGE on interrupt entry/exit, but that's a bit overkill when
you consider how rarely we issue such TLB invalidation.
> Somehow I feel like this should look more like just another TLB
> invalidation in the kernel, but if there's a good reason why it can't
> then this is fine.
The rest of the TLB invalidation in the kernel doesn't need to
save/restore any context. They apply to a set of parameters that are
already loaded on the CPU. What we have here is substantially different.
Thanks,
M.
--
Jazz is not dead. It just smells funny...
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v6 2/4] arm64: KVM: add accessors to track guest/host only counters
From: Christoffer Dall @ 2018-12-10 10:26 UTC (permalink / raw)
To: Andrew Murray
Cc: Mark Rutland, Julien Thierry, Marc Zyngier, Catalin Marinas,
Suzuki K Poulose, Will Deacon, kvmarm, linux-arm-kernel
In-Reply-To: <1544435159-54781-3-git-send-email-andrew.murray@arm.com>
On Mon, Dec 10, 2018 at 09:45:57AM +0000, Andrew Murray wrote:
> In order to effeciently enable/disable guest/host only perf counters
> at guest entry/exit we add bitfields to kvm_cpu_context for guest and
> host events as well as accessors for updating them.
>
> Signed-off-by: Andrew Murray <andrew.murray@arm.com>
> ---
> arch/arm64/include/asm/kvm_host.h | 24 ++++++++++++++++++++++++
> 1 file changed, 24 insertions(+)
>
> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> index 1550192..800c87b 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -203,6 +203,8 @@ struct kvm_cpu_context {
> };
>
> struct kvm_vcpu *__hyp_running_vcpu;
> + u32 events_host;
> + u32 events_guest;
This is confusing to me.
These values appear only to be used for the host instance, which makes
me wonder why we add them to kvm_cpu_context, which is also used for the
guest state? Should we not instead consider moving them to their own
data structure and add a per-cpu data structure or something more fancy
like having a new data structure, kvm_percpu_host_data, which contains
the kvm_cpu_context and the events flags?
I don't know much about perf, but doesn't this design also imply that
you can only set these modifiers at a per-cpu level, and not attach
the modifiers to a task/vcpu or vm ? Is that by design?
Thanks,
Christoffer
> };
>
> typedef struct kvm_cpu_context kvm_cpu_context_t;
> @@ -467,11 +469,33 @@ void kvm_arch_vcpu_load_fp(struct kvm_vcpu *vcpu);
> void kvm_arch_vcpu_ctxsync_fp(struct kvm_vcpu *vcpu);
> void kvm_arch_vcpu_put_fp(struct kvm_vcpu *vcpu);
>
> +#define KVM_PMU_EVENTS_HOST 1
> +#define KVM_PMU_EVENTS_GUEST 2
> +
> #ifdef CONFIG_KVM /* Avoid conflicts with core headers if CONFIG_KVM=n */
> static inline int kvm_arch_vcpu_run_pid_change(struct kvm_vcpu *vcpu)
> {
> return kvm_arch_vcpu_run_map_fp(vcpu);
> }
> +static inline void kvm_set_pmu_events(u32 set, int flags)
> +{
> + kvm_cpu_context_t *ctx = this_cpu_ptr(&kvm_host_cpu_state);
> +
> + if (flags & KVM_PMU_EVENTS_HOST)
> + ctx->events_host |= set;
> + if (flags & KVM_PMU_EVENTS_GUEST)
> + ctx->events_guest |= set;
> +}
> +static inline void kvm_clr_pmu_events(u32 clr)
> +{
> + kvm_cpu_context_t *ctx = this_cpu_ptr(&kvm_host_cpu_state);
> +
> + ctx->events_host &= ~clr;
> + ctx->events_guest &= ~clr;
> +}
> +#else
> +static inline void kvm_set_pmu_events(u32 set, int flags) {}
> +static inline void kvm_clr_pmu_events(u32 clr) {}
> #endif
>
> static inline void kvm_arm_vhe_guest_enter(void)
> --
> 2.7.4
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v9 1/8] KVM: arm/arm64: Share common code in user_mem_abort()
From: Suzuki K Poulose @ 2018-12-10 10:26 UTC (permalink / raw)
To: Christoffer Dall
Cc: Anshuman Khandual, marc.zyngier, will.deacon, linux-kernel,
punitagrawal, kvmarm, linux-arm-kernel
In-Reply-To: <20181210085616.GB30263@e113682-lin.lund.arm.com>
On 10/12/2018 08:56, Christoffer Dall wrote:
> On Mon, Dec 03, 2018 at 01:37:37PM +0000, Suzuki K Poulose wrote:
>> Hi Anshuman,
>>
>> On 03/12/2018 12:11, Anshuman Khandual wrote:
>>>
>>>
>>> On 10/31/2018 11:27 PM, Punit Agrawal wrote:
>>>> The code for operations such as marking the pfn as dirty, and
>>>> dcache/icache maintenance during stage 2 fault handling is duplicated
>>>> between normal pages and PMD hugepages.
>>>>
>>>> Instead of creating another copy of the operations when we introduce
>>>> PUD hugepages, let's share them across the different pagesizes.
>>>>
>>>> Signed-off-by: Punit Agrawal <punit.agrawal@arm.com>
>>>> Reviewed-by: Suzuki K Poulose <suzuki.poulose@arm.com>
>>>> Cc: Christoffer Dall <christoffer.dall@arm.com>
>>>> Cc: Marc Zyngier <marc.zyngier@arm.com>
>>>> ---
>>>> virt/kvm/arm/mmu.c | 49 ++++++++++++++++++++++++++++------------------
>>>> 1 file changed, 30 insertions(+), 19 deletions(-)
>>>>
>>>> diff --git a/virt/kvm/arm/mmu.c b/virt/kvm/arm/mmu.c
>>>> index 5eca48bdb1a6..59595207c5e1 100644
>>>> --- a/virt/kvm/arm/mmu.c
>>>> +++ b/virt/kvm/arm/mmu.c
>>>> @@ -1475,7 +1475,7 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa,
>>>> unsigned long fault_status)
>>>> {
>>>> int ret;
>>>> - bool write_fault, exec_fault, writable, hugetlb = false, force_pte = false;
>>>> + bool write_fault, exec_fault, writable, force_pte = false;
>>>> unsigned long mmu_seq;
>>>> gfn_t gfn = fault_ipa >> PAGE_SHIFT;
>>>> struct kvm *kvm = vcpu->kvm;
>>>> @@ -1484,7 +1484,7 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa,
>>>> kvm_pfn_t pfn;
>>>> pgprot_t mem_type = PAGE_S2;
>>>> bool logging_active = memslot_is_logging(memslot);
>>>> - unsigned long flags = 0;
>>>> + unsigned long vma_pagesize, flags = 0;
>>>
>>> A small nit s/vma_pagesize/pagesize. Why call it VMA ? Its implicit.
>>
>> May be we could call it mapsize. pagesize is confusing.
>>
>
> I'm ok with mapsize. I see the vma_pagesize name coming from the fact
> that this is initially set to the return value from vma_kernel_pagesize.
>
> I have not problems with either vma_pagesize or mapsize.
Ok, I will retain the vma_pagesize to avoid unnecessary changes to the patch.
Thanks
Suzuki
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH 03/12] arm/arm64: Provide a wrapper for SMCCC 1.1 calls
From: Mark Rutland @ 2018-12-10 10:27 UTC (permalink / raw)
To: Steven Price
Cc: Marc Zyngier, Catalin Marinas, Will Deacon, Christoffer Dall,
kvmarm, linux-arm-kernel
In-Reply-To: <20181128144527.44710-4-steven.price@arm.com>
On Wed, Nov 28, 2018 at 02:45:18PM +0000, Steven Price wrote:
> SMCCC 1.1 calls may use either HVC or SMC depending on the PSCI
> conduit. Rather than coding this in every call site provide a macro
> which uses the correct instruction. The macro also handles the case
> where no PSCI conduit is configured returning a not supported error
> in res, along with returning the conduit used for the call.
>
> This allow us to remove some duplicated code and will be useful later
> when adding paravirtualized time hypervisor calls.
>
> Signed-off-by: Steven Price <steven.price@arm.com>
> ---
> include/linux/arm-smccc.h | 44 +++++++++++++++++++++++++++++++++++++++
> 1 file changed, 44 insertions(+)
>
> diff --git a/include/linux/arm-smccc.h b/include/linux/arm-smccc.h
> index 18863d56273c..b047009e7a0a 100644
> --- a/include/linux/arm-smccc.h
> +++ b/include/linux/arm-smccc.h
> @@ -311,5 +311,49 @@ asmlinkage void __arm_smccc_hvc(unsigned long a0, unsigned long a1,
> #define SMCCC_RET_NOT_SUPPORTED -1
> #define SMCCC_RET_NOT_REQUIRED -2
>
> +/* Like arm_smccc_1_1* but always returns SMCCC_RET_NOT_SUPPORTED.
> + * Used when the PSCI conduit is not defined. The empty asm statement
> + * avoids compiler warnings about unused variables.
> + */
> +#define __fail_smccc_1_1(...) \
> + do { \
> + __declare_args(__count_args(__VA_ARGS__), __VA_ARGS__); \
> + asm ("" __constraints(__count_args(__VA_ARGS__))); \
> + if (___res) \
> + ___res->a0 = SMCCC_RET_NOT_SUPPORTED; \
> + } while (0)
> +
> +/*
> + * arm_smccc_1_1() - make an SMCCC v1.1 compliant call
> + *
> + * This is a variadic macro taking one to eight source arguments, and
> + * an optional return structure.
> + *
> + * @a0-a7: arguments passed in registers 0 to 7
> + * @res: result values from registers 0 to 3
> + *
> + * This macro will make either an HVC call or an SMC call depending on the
> + * current PSCI conduit. If no valid conduit is available then -1
> + * (SMCCC_RET_NOT_SUPPORTED) is returned in @res.a0 (if supplied).
> + *
> + * The return value also provides the conduit that was used.
> + */
> +#define arm_smccc_1_1(...) ({ \
As a minor nit, could we please give call this something like
arm_smccc_1_1_call() or arm_smccc_1_1_invoke(), to make it clear what
action this performs?
I had some patches [1] cleaning up the SMCCC API, it would be nice if we
could pick up some of those as preparatory bits, before we spread some
of the current design warts (e.g. SMCCC depending on PSCI definitions).
Thanks,
Mark.
[1] git://git.kernel.org/pub/scm/linux/kernel/git/mark/linux.git arm64/smccc-cleanup
> + int method = psci_ops.conduit; \
> + switch (method) { \
> + case PSCI_CONDUIT_HVC: \
> + arm_smccc_1_1_hvc(__VA_ARGS__); \
> + break; \
> + case PSCI_CONDUIT_SMC: \
> + arm_smccc_1_1_smc(__VA_ARGS__); \
> + break; \
> + default: \
> + __fail_smccc_1_1(__VA_ARGS__); \
> + method = PSCI_CONDUIT_NONE; \
> + break; \
> + } \
> + method; \
> + })
> +
> #endif /*__ASSEMBLY__*/
> #endif /*__LINUX_ARM_SMCCC_H*/
> --
> 2.19.2
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v3 2/8] KVM: arm64: Rework detection of SVE, !VHE systems
From: Marc Zyngier @ 2018-12-10 10:28 UTC (permalink / raw)
To: Christoffer Dall
Cc: Mark Rutland, kvm, Suzuki K Poulose, Catalin Marinas, Will Deacon,
James Morse, kvmarm, linux-arm-kernel
In-Reply-To: <20181210101305.GH30263@e113682-lin.lund.arm.com>
On 10/12/2018 10:13, Christoffer Dall wrote:
> On Thu, Dec 06, 2018 at 05:31:20PM +0000, Marc Zyngier wrote:
>> An SVE system is so far the only case where we mandate VHE. As we're
>> starting to grow this requirements, let's slightly rework the way we
>> deal with that situation, allowing for easy extension of this check.
>>
>> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
>> ---
>> arch/arm/include/asm/kvm_host.h | 2 +-
>> arch/arm64/include/asm/kvm_host.h | 6 +++---
>> virt/kvm/arm/arm.c | 8 ++++----
>> 3 files changed, 8 insertions(+), 8 deletions(-)
>>
>> diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h
>> index 5ca5d9af0c26..2184d9ddb418 100644
>> --- a/arch/arm/include/asm/kvm_host.h
>> +++ b/arch/arm/include/asm/kvm_host.h
>> @@ -285,7 +285,7 @@ void kvm_mmu_wp_memory_region(struct kvm *kvm, int slot);
>>
>> struct kvm_vcpu *kvm_mpidr_to_vcpu(struct kvm *kvm, unsigned long mpidr);
>>
>> -static inline bool kvm_arch_check_sve_has_vhe(void) { return true; }
>> +static inline bool kvm_arch_requires_vhe(void) { return false; }
>> static inline void kvm_arch_hardware_unsetup(void) {}
>> static inline void kvm_arch_sync_events(struct kvm *kvm) {}
>> static inline void kvm_arch_vcpu_uninit(struct kvm_vcpu *vcpu) {}
>> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
>> index 52fbc823ff8c..d6d9aa76a943 100644
>> --- a/arch/arm64/include/asm/kvm_host.h
>> +++ b/arch/arm64/include/asm/kvm_host.h
>> @@ -422,7 +422,7 @@ static inline void __cpu_init_hyp_mode(phys_addr_t pgd_ptr,
>> }
>> }
>>
>> -static inline bool kvm_arch_check_sve_has_vhe(void)
>> +static inline bool kvm_arch_requires_vhe(void)
>> {
>> /*
>> * The Arm architecture specifies that implementation of SVE
>> @@ -430,9 +430,9 @@ static inline bool kvm_arch_check_sve_has_vhe(void)
>> * relies on this when SVE is present:
>> */
>> if (system_supports_sve())
>> - return has_vhe();
>> - else
>> return true;
>> +
>> + return false;
>> }
>>
>> static inline void kvm_arch_hardware_unsetup(void) {}
>> diff --git a/virt/kvm/arm/arm.c b/virt/kvm/arm/arm.c
>> index 23774970c9df..1db4c15edcdd 100644
>> --- a/virt/kvm/arm/arm.c
>> +++ b/virt/kvm/arm/arm.c
>> @@ -1640,8 +1640,10 @@ int kvm_arch_init(void *opaque)
>> return -ENODEV;
>> }
>>
>> - if (!kvm_arch_check_sve_has_vhe()) {
>> - kvm_pr_unimpl("SVE system without VHE unsupported. Broken cpu?");
>> + in_hyp_mode = is_kernel_in_hyp_mode();
>> +
>> + if (!in_hyp_mode && kvm_arch_requires_vhe()) {
>> + kvm_pr_unimpl("CPU requiring VHE was booted in non-VHE mode");
>
> nit: The error message feels weird to me (are we reporting CPU bugs?)
> and I'm not sure about the unimpl and I think there's a linse space
> missing.
>
> How about:
>
> kvm_err("Cannot support this CPU in non-VHE mode, not initializing\n");
Yup, works for me. Will, do you mind changing this message for me?
>
> If we wanted to be super helpful, we could expand
> kvm_arch_requires_vhe() with a print statement saying:
>
> kvm_err("SVE detected, requiring VHE mode\n");
>
> But thay may be overkill.
I started with that, but having kvm_pr*() in an include file has proved
to be challenging, so I decided to spend my time on something more
useful and quickly gave up... :-/
>
>
>> return -ENODEV;
>> }
>>
>> @@ -1657,8 +1659,6 @@ int kvm_arch_init(void *opaque)
>> if (err)
>> return err;
>>
>> - in_hyp_mode = is_kernel_in_hyp_mode();
>> -
>> if (!in_hyp_mode) {
>> err = init_hyp_mode();
>> if (err)
>> --
>> 2.19.2
>>
>
> Otherwise:
>
> Acked-by: Christoffer Dall <christoffer.dall@arm.com>
>
Thanks,
M.
--
Jazz is not dead. It just smells funny...
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v5 1/5] spi: spi-mem: Add driver for NXP FlexSPI controller
From: Schrempf Frieder @ 2018-12-10 10:31 UTC (permalink / raw)
To: Boris Brezillon, Yogesh Narayan Gaur
Cc: mark.rutland@arm.com, devicetree@vger.kernel.org, robh@kernel.org,
linux-kernel@vger.kernel.org, linux-spi@vger.kernel.org,
marek.vasut@gmail.com, broonie@kernel.org,
linux-mtd@lists.infradead.org, computersforpeace@gmail.com,
shawnguo@kernel.org, linux-arm-kernel@lists.infradead.org
In-Reply-To: <20181210111909.35384eee@bbrezillon>
Hi Yogesh, Boris,
On 10.12.18 11:19, Boris Brezillon wrote:
> On Mon, 10 Dec 2018 09:41:51 +0000
> Yogesh Narayan Gaur <yogeshnarayan.gaur@nxp.com> wrote:
>
>>>> +/* Instead of busy looping invoke readl_poll_timeout functionality.
>>>> +*/ static int fspi_readl_poll_tout(struct nxp_fspi *f, void __iomem *base,
>>>> + u32 mask, u32 delay_us,
>>>> + u32 timeout_us, bool condition)
>>>> +{
>>>> + u32 reg;
>>>> +
>>>> + if (!f->devtype_data->little_endian)
>>>> + mask = (u32)cpu_to_be32(mask);
>>>> +
>>>> + if (condition)
>>>> + return readl_poll_timeout(base, reg, (reg & mask),
>>>> + delay_us, timeout_us);
>>>> + else
>>>> + return readl_poll_timeout(base, reg, !(reg & mask),
>>>> + delay_us, timeout_us);
>>>
>>> I would rather use a local variable to store the condition:
>>>
>>> bool c = condition ? (reg & mask):!(reg & mask);
>>>
>> With these type of usage getting below warning messages.
>>
>> drivers/spi/spi-nxp-fspi.c: In function ‘fspi_readl_poll_tout.isra.10.constprop’:
>> drivers/spi/spi-nxp-fspi.c:446:21: warning: ‘reg’ may be used uninitialized in this function [-Wmaybe-uninitialized]
>> bool cn = c ? (reg & mask) : !(reg & mask);
>>
>> If assign value to reg = 0xffffffff then timeout is start getting hit for False case and if assign value 0 then start getting timeout hit for true case.
>>
>> I would rather not try to modify this function.
>
> I agree. Let's keep this function readable even if this implies
> duplicating a few lines of code.
My bad. This doesn't work of course. We need to pass the actual
expression containing reg to the readl_poll_timeout() macro. So forget
about my comment.
>
>>
>>> return readl_poll_timeout(base, reg, c, delay_us, timeout_us);
>>>
>>>> +}
>>>> +
>>>> +/*
>>>> + * If the slave device content being changed by Write/Erase, need to
>>>> + * invalidate the AHB buffer. This can be achieved by doing the reset
>>>> + * of controller after setting MCR0[SWRESET] bit.
>>>> + */
>>>> +static inline void nxp_fspi_invalid(struct nxp_fspi *f) {
>>>> + u32 reg;
>>>> + int ret;
>>>> +
>>>> + reg = fspi_readl(f, f->iobase + FSPI_MCR0);
>>>> + fspi_writel(f, reg | FSPI_MCR0_SWRST, f->iobase + FSPI_MCR0);
>>>> +
>>>> + /* w1c register, wait unit clear */
>>>> + ret = fspi_readl_poll_tout(f, f->iobase + FSPI_MCR0,
>>>> + FSPI_MCR0_SWRST, 0, POLL_TOUT, false);
>>>> + WARN_ON(ret);
>>>> +}
>>>> +
>>>> +static void nxp_fspi_prepare_lut(struct nxp_fspi *f,
>>>> + const struct spi_mem_op *op)
>>>> +{
>>>> + void __iomem *base = f->iobase;
>>>> + u32 lutval[4] = {};
>>>> + int lutidx = 1, i;
>>>> +
>>>> + /* cmd */
>>>> + lutval[0] |= LUT_DEF(0, LUT_CMD, LUT_PAD(op->cmd.buswidth),
>>>> + op->cmd.opcode);
>>>> +
>>>> + /* addr bus width */
>>>> + if (op->addr.nbytes) {
>>>> + u32 addrlen = 0;
>>>> +
>>>> + switch (op->addr.nbytes) {
>>>> + case 1:
>>>> + addrlen = ADDR8BIT;
>>>> + break;
>>>> + case 2:
>>>> + addrlen = ADDR16BIT;
>>>> + break;
>>>> + case 3:
>>>> + addrlen = ADDR24BIT;
>>>> + break;
>>>> + case 4:
>>>> + addrlen = ADDR32BIT;
>>>> + break;
>>>> + default:
>>>> + dev_err(f->dev, "In-correct address length\n");
>>>> + return;
>>>> + }
>>>
>>> You don't need to validate op->addr.nbytes here, this is already done in
>>> nxp_fspi_supports_op().
>>
>> Yes, I need to validate op->addr.nbytes else LUT would going to be programmed for 0 addrlen.
>> I have checked this on the target.
>
> Also agree there. Some operations have 0 address bytes. We could also
> test addr.buswidth, but I'm fine with the addr.nbytes test too.
The "if (op->addr.nbytes)" is needed of course, but I think the default
case in the switch statement (and for other reasons the whole switch
statement) is not needed and rather a check for op->addr.nbytes > 4
should be added to nxp_fspi_supports_op(). I wrongly assumed this check
already exists in nxp_fspi_supports_op().
>
>>>> +static void nxp_fspi_select_mem(struct nxp_fspi *f, struct spi_device
>>>> +*spi) {
>>>> + unsigned long rate = spi->max_speed_hz;
>>>> + int ret;
>>>> + uint64_t size_kb;
>>>> +
>>>> + /*
>>>> + * Return, if previously selected slave device is same as current
>>>> + * requested slave device.
>>>> + */
>>>> + if (f->selected == spi->chip_select)
>>>> + return;
>>>> +
>>>> + /* Reset FLSHxxCR0 registers */
>>>> + fspi_writel(f, 0, f->iobase + FSPI_FLSHA1CR0);
>>>> + fspi_writel(f, 0, f->iobase + FSPI_FLSHA2CR0);
>>>> + fspi_writel(f, 0, f->iobase + FSPI_FLSHB1CR0);
>>>> + fspi_writel(f, 0, f->iobase + FSPI_FLSHB2CR0);
>>>> +
>>>> + /* Assign controller memory mapped space as size, KBytes, of flash. */
>>>> + size_kb = FSPI_FLSHXCR0_SZ(f->memmap_phy_size);
>>>
>> Above description of this function, explains the reason for using memmap_phy_size.
>> This is not the arbitrary size, but the memory mapped size being assigned to the controller.
>>
>>> You are still using memory of arbitrary size (memmap_phy_size) for mapping the
>>> flash. Why not use the same approach as in the QSPI driver and just map
>>> ahb_buf_size until we implement the dirmap API?
>> The approach which being used in QSPI driver didn't work here, I have tried with that.
>> In QSPI driver, while preparing LUT we are assigning read/write address in the LUT preparation and have to for some unknown hack have to provide macro for LUT_MODE instead of LUT_ADDR.
>> But this thing didn't work for FlexSPI.
>> I discussed with HW IP owner and they suggested only to use LUT_ADDR for specifying the address length of the command i.e. 3-byte or 4-byte address command (NOR) or 1-2 byte address command for NAND.
>
> Actually, we would have used a LUT_ADDR too if the QSPI IP was support
> ADDR instructions with a number of bytes < 3, but for some unknown
> reasons it does not work.
>
>>
>> Thus, in LUT preparation we have assigned only the base address.
>> Now if I have assigned ahb_buf_size to FSPI_FLSHXXCR0 register then for read/write data beyond limit of ahb_buf_size offset I get data corruption.
>
> Why would you do that? We have the ->adjust_op_size() exactly for this
> reason, so, if someone tries to do a spi_mem_op with data.nbytes >
> ahb_buf_size you should return an error.
>
>>
>> Thus, for generic approach have assigned FSPI_FLSHXXCR0 equal to the memory mapped size to the controller. This would also not going to depend on the number of CS present on the target.
>
> I kind of agree with Frieder on that one, I think it's preferable to
> limit the per-read-op size to ahb_buf_size and let the upper layer
> split the request in several sub-requests. On the controller side of
> things, you just have to have a mapping of ahb_buf_size per-CS. If you
> want to further optimize things, implement the dirmap hooks.
>
>>
>>> You are already aligning the AHB reads for this in nxp_fspi_adjust_op_size().
>>>
>> Yes, max read data size can be ahb_buf_size. Thus we need to check max read size with ahb_buf_size.
>
> Well, it's never a bad thing to check it twice, just in case the
> spi-mem user is misusing the API.
>
>>>> +static void nxp_fspi_fill_txfifo(struct nxp_fspi *f,
>>>> + const struct spi_mem_op *op)
>>>> +{
>>>> + void __iomem *base = f->iobase;
>>>> + int i, j, ret;
>>>> + int size, tmp_size, wm_size;
>>>> + u32 data = 0;
>>>> + u32 *txbuf = (u32 *) op->data.buf.out;
>>>> +
>>>> + /* clear the TX FIFO. */
>>>> + fspi_writel(f, FSPI_IPTXFCR_CLR, base + FSPI_IPTXFCR);
>>>> +
>>>> + /* Default value of water mark level is 8 bytes. */
>>>> + wm_size = 8;
>>>> + size = op->data.nbytes / wm_size;
>>>> + for (i = 0; i < size; i++) {
>>>> + /* Wait for TXFIFO empty */
>>>> + ret = fspi_readl_poll_tout(f, f->iobase + FSPI_INTR,
>>>> + FSPI_INTR_IPTXWE, 0,
>>>> + POLL_TOUT, true);
>>>> + WARN_ON(ret);
>>>> +
>>>> + j = 0;
>>>> + tmp_size = wm_size;
>>>> + while (tmp_size > 0) {
>>>> + data = 0;
>>>> + memcpy(&data, txbuf, 4);
>>>> + fspi_writel(f, data, base + FSPI_TFDR + j * 4);
>>>> + tmp_size -= 4;
>>>> + j++;
>>>> + txbuf += 1;
>>>> + }
>>>> + fspi_writel(f, FSPI_INTR_IPTXWE, base + FSPI_INTR);
>>>> + }
>>>> +
>>>> + size = op->data.nbytes % wm_size;
>>>> + if (size) {
>>>> + /* Wait for TXFIFO empty */
>>>> + ret = fspi_readl_poll_tout(f, f->iobase + FSPI_INTR,
>>>> + FSPI_INTR_IPTXWE, 0,
>>>> + POLL_TOUT, true);
>>>> + WARN_ON(ret);
>>>> +
>>>> + j = 0;
>>>> + tmp_size = 0;
>>>> + while (size > 0) {
>>>> + data = 0;
>>>> + tmp_size = (size < 4) ? size : 4;
>>>> + memcpy(&data, txbuf, tmp_size);
>>>> + fspi_writel(f, data, base + FSPI_TFDR + j * 4);
>>>> + size -= tmp_size;
>>>> + j++;
>>>> + txbuf += 1;
>>>> + }
>>>> + fspi_writel(f, FSPI_INTR_IPTXWE, base + FSPI_INTR);
>>>> + }
>>>
>>> All these nested loops to fill the TX buffer and also the ones below to read the
>>> RX buffer look much more complicated than they should really be. Can you try to
>>> make this more readable?
>> Yes
>>>
>>> Maybe something like this would work:
>>>
>>> for (i = 0; i < ALIGN_DOWN(op->data.nbytes, 8); i += 8) {
>>> /* Wait for TXFIFO empty */
>>> ret = fspi_readl_poll_tout(f, f->iobase + FSPI_INTR,
>>> FSPI_INTR_IPTXWE, 0,
>>> POLL_TOUT, true);
>>>
>>> fspi_writel(f, op->data.buf.out + i, base + FSPI_TFDR);
>>> fspi_writel(f, op->data.buf.out + i + 4, base + FSPI_TFDR + 4);
>>> fspi_writel(f, FSPI_INTR_IPTXWE, base + FSPI_INTR); }
>> With this above 2 lines we are hardcoding it for read/write with watermark size as 8 bytes.
>> Watermark size can be variable and depends on the value of IPRXFCR/IPTXFCR register with default value as 8 bytes
>> Thus, I would still prefer to use the internal for loop instead of 2 fspi_writel(...) for FSPI_TFDR and FSPI_TFDR + 4 register write commands.
>
> Just like you're hardcoding wm_size to 8, so I don't see a difference
> here. And I indeed prefer Frieder's version.
Yes, as long as the watermark level is fixed, we don't need the inner loop.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v5 1/5] spi: spi-mem: Add driver for NXP FlexSPI controller
From: Schrempf Frieder @ 2018-12-10 10:35 UTC (permalink / raw)
To: Yogesh Narayan Gaur, linux-mtd@lists.infradead.org,
boris.brezillon@bootlin.com, marek.vasut@gmail.com,
broonie@kernel.org, linux-spi@vger.kernel.org,
devicetree@vger.kernel.org
Cc: mark.rutland@arm.com, robh@kernel.org,
linux-kernel@vger.kernel.org, computersforpeace@gmail.com,
shawnguo@kernel.org, linux-arm-kernel@lists.infradead.org
In-Reply-To: <VI1PR04MB57267D9F69719B4BEF38BF8699A50@VI1PR04MB5726.eurprd04.prod.outlook.com>
Hi Yogesh,
On 10.12.18 10:41, Yogesh Narayan Gaur wrote:
[...]>>> +
>>> +static void nxp_fspi_prepare_lut(struct nxp_fspi *f,
>>> + const struct spi_mem_op *op)
>>> +{
>>> + void __iomem *base = f->iobase;
>>> + u32 lutval[4] = {};
>>> + int lutidx = 1, i;
>>> +
>>> + /* cmd */
>>> + lutval[0] |= LUT_DEF(0, LUT_CMD, LUT_PAD(op->cmd.buswidth),
>>> + op->cmd.opcode);
>>> +
>>> + /* addr bus width */
This comment should match the code below. So maybe only "addr" should be
enough.
>>> + if (op->addr.nbytes) {
>>> + u32 addrlen = 0;
>>> +
>>> + switch (op->addr.nbytes) {
>>> + case 1:
>>> + addrlen = ADDR8BIT;
>>> + break;
>>> + case 2:
>>> + addrlen = ADDR16BIT;
>>> + break;
>>> + case 3:
>>> + addrlen = ADDR24BIT;
>>> + break;
>>> + case 4:
>>> + addrlen = ADDR32BIT;
>>> + break;
>>> + default:
>>> + dev_err(f->dev, "In-correct address length\n");
>>> + return;
>>> + }
>>
>> You don't need to validate op->addr.nbytes here, this is already done in
>> nxp_fspi_supports_op().
>
> Yes, I need to validate op->addr.nbytes else LUT would going to be programmed for 0 addrlen.
> I have checked this on the target.
>
>>
>>> +
>>> + lutval[lutidx / 2] |= LUT_DEF(lutidx, LUT_ADDR,
>>> + LUT_PAD(op->addr.buswidth),
>>> + addrlen);
>>
>> You can also just remove the whole switch statement above and use this:
>>
>> lutval[lutidx / 2] |= LUT_DEF(lutidx, LUT_ADDR,
>> LUT_PAD(op->addr.buswidth),
>> op->addr.nbytes * 8);
>>
> Ok, would include in next version.
>
>>> + lutidx++;
>>> + }
>>> +
>>> + /* dummy bytes, if needed */
>>> + if (op->dummy.nbytes) {
>>> + lutval[lutidx / 2] |= LUT_DEF(lutidx, LUT_DUMMY,
>>> + /*
>>> + * Due to FlexSPI controller limitation number of PAD for
>> dummy
>>> + * buswidth needs to be programmed as equal to data buswidth.
>>> + */
>>> + LUT_PAD(op->data.buswidth),
>>> + op->dummy.nbytes * 8 /
>>> + op->dummy.buswidth);
>>> + lutidx++;
>>> + }
>>> +
>>> + /* read/write data bytes */
>>> + if (op->data.nbytes) {
>>> + lutval[lutidx / 2] |= LUT_DEF(lutidx,
>>> + op->data.dir ==
>> SPI_MEM_DATA_IN ?
>>> + LUT_NXP_READ : LUT_NXP_WRITE,
>>> + LUT_PAD(op->data.buswidth),
>>> + 0);
>>> + lutidx++;
>>> + }
>>> +
>>> + /* stop condition. */
>>> + lutval[lutidx / 2] |= LUT_DEF(lutidx, LUT_STOP, 0, 0);
>>> +
>>> + /* unlock LUT */
>>> + fspi_writel(f, FSPI_LUTKEY_VALUE, f->iobase + FSPI_LUTKEY);
>>> + fspi_writel(f, FSPI_LCKER_UNLOCK, f->iobase + FSPI_LCKCR);
>>> +
>>> + /* fill LUT */
>>> + for (i = 0; i < ARRAY_SIZE(lutval); i++)
>>> + fspi_writel(f, lutval[i], base + FSPI_LUT_REG(i));
>>> +
>>> + dev_dbg(f->dev, "CMD[%x] lutval[0:%x \t 1:%x \t 2:%x \t 3:%x]\n",
>>> + op->cmd.opcode, lutval[0], lutval[1], lutval[2], lutval[3]);
>>> +
>>> + /* lock LUT */
>>> + fspi_writel(f, FSPI_LUTKEY_VALUE, f->iobase + FSPI_LUTKEY);
>>> + fspi_writel(f, FSPI_LCKER_LOCK, f->iobase + FSPI_LCKCR); }
[...]
>>> +
>>> +static int nxp_fspi_exec_op(struct spi_mem *mem, const struct
>>> +spi_mem_op *op) {
>>> + struct nxp_fspi *f = spi_controller_get_devdata(mem->spi->master);
>>> + int err = 0;
>>> +
>>> + mutex_lock(&f->lock);
>>> +
>>> + /* Wait for controller being ready. */
>>> + err = fspi_readl_poll_tout(f, f->iobase + FSPI_STS0,
>>> + FSPI_STS0_ARB_IDLE, 1, POLL_TOUT, true);
>>> + WARN_ON(err);
>>> +
>>> + nxp_fspi_select_mem(f, mem->spi);
>>> +
>>> + nxp_fspi_prepare_lut(f, op);
>>> + /*
>>> + * If we have large chunks of data, we read them through the AHB bus
>>> + * by accessing the mapped memory. In all other cases we use
>>> + * IP commands to access the flash.
>>> + */
>>> + if (op->data.nbytes > (f->devtype_data->rxfifo - 4) &&
>>> + op->data.dir == SPI_MEM_DATA_IN) {
>>> + nxp_fspi_read_ahb(f, op);
>>> + } else {
>>> + if (op->data.nbytes && op->data.dir == SPI_MEM_DATA_OUT)
>>> + nxp_fspi_fill_txfifo(f, op);
>>> +
>>> + err = nxp_fspi_do_op(f, op);
>>> +
>>> + /* Invalidate the data in the AHB buffer. */
>>> + if (op->data.nbytes && op->data.dir == SPI_MEM_DATA_OUT)
>>> + nxp_fspi_invalid(f);
>>
>> E.g. in case of an erase operation or a NAND load page operation, the
>> invalidation is not triggered, but flash/buffer contents have changed.
>> So I'm not sure if this is enough...
> Ok, would change this and have invalidate for all operations.
Maybe you can find out the correct way through testing with NOR and NAND.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v5 1/5] spi: spi-mem: Add driver for NXP FlexSPI controller
From: Boris Brezillon @ 2018-12-10 10:36 UTC (permalink / raw)
To: Schrempf Frieder
Cc: mark.rutland@arm.com, devicetree@vger.kernel.org,
Yogesh Narayan Gaur, robh@kernel.org,
linux-kernel@vger.kernel.org, linux-spi@vger.kernel.org,
marek.vasut@gmail.com, broonie@kernel.org,
linux-mtd@lists.infradead.org, computersforpeace@gmail.com,
shawnguo@kernel.org, linux-arm-kernel@lists.infradead.org
In-Reply-To: <f51ab7b2-bc23-1a8f-5970-18c2d200eb6a@kontron.de>
On Mon, 10 Dec 2018 10:31:57 +0000
Schrempf Frieder <frieder.schrempf@kontron.de> wrote:
> >> Yes, I need to validate op->addr.nbytes else LUT would going to be programmed for 0 addrlen.
> >> I have checked this on the target.
> >
> > Also agree there. Some operations have 0 address bytes. We could also
> > test addr.buswidth, but I'm fine with the addr.nbytes test too.
>
> The "if (op->addr.nbytes)" is needed of course, but I think the default
> case in the switch statement (and for other reasons the whole switch
> statement) is not needed and rather a check for op->addr.nbytes > 4
> should be added to nxp_fspi_supports_op(). I wrongly assumed this check
> already exists in nxp_fspi_supports_op().
Ok, then this check on the max number of address bytes should indeed be
moved to the supports_op() implementation.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH 05/12] KVM: arm64: Implement PV_FEATURES call
From: Mark Rutland @ 2018-12-10 10:39 UTC (permalink / raw)
To: Steven Price
Cc: Marc Zyngier, Catalin Marinas, Will Deacon, Christoffer Dall,
kvmarm, linux-arm-kernel
In-Reply-To: <20181128144527.44710-6-steven.price@arm.com>
On Wed, Nov 28, 2018 at 02:45:20PM +0000, Steven Price wrote:
> This provides a mechanism for querying which paravirtualized features
> are available in this hypervisor.
>
> Also add the header file which defines the ABI for the paravirtualized
> clock features we're about to add.
>
> Signed-off-by: Steven Price <steven.price@arm.com>
> ---
> arch/arm64/include/asm/pvclock-abi.h | 32 ++++++++++++++++++++++++++++
> include/kvm/arm_pv.h | 28 ++++++++++++++++++++++++
> include/linux/arm-smccc.h | 1 +
> virt/kvm/arm/hypercalls.c | 9 ++++++++
> 4 files changed, 70 insertions(+)
> create mode 100644 arch/arm64/include/asm/pvclock-abi.h
> create mode 100644 include/kvm/arm_pv.h
>
> diff --git a/arch/arm64/include/asm/pvclock-abi.h b/arch/arm64/include/asm/pvclock-abi.h
> new file mode 100644
> index 000000000000..64ce041c8922
> --- /dev/null
> +++ b/arch/arm64/include/asm/pvclock-abi.h
> @@ -0,0 +1,32 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/* Copyright (C) 2018 Arm Ltd. */
> +
> +#ifndef __ASM_PVCLOCK_ABI_H
> +#define __ASM_PVCLOCK_ABI_H
> +
> +#include <kvm/arm_pv.h>
> +
> +struct pvclock_vm_time_info {
> + __le32 revision;
> + __le32 attributes;
> + __le64 sequence_number;
> + __le64 scale_mult;
> + __le32 shift;
> + __le32 reserved;
> + __le64 native_freq;
> + __le64 pv_freq;
> + __le64 div_by_pv_freq_mult;
> +} __packed;
> +
> +struct pvclock_vcpu_stolen_time_info {
> + __le32 revision;
> + __le32 attributes;
> + __le64 stolen_time;
> + /* Structure must be 64 byte aligned, pad to that size */
> + u8 padding[48];
> +} __packed;
> +
> +#define PV_VM_TIME_NOT_SUPPORTED -1
> +#define PV_VM_TIME_INVALID_PARAMETERS -2
Could you please add a comment describing that these are defined in ARM
DEN0057A?
> +
> +#endif
> diff --git a/include/kvm/arm_pv.h b/include/kvm/arm_pv.h
> new file mode 100644
> index 000000000000..19d2dafff31a
> --- /dev/null
> +++ b/include/kvm/arm_pv.h
> @@ -0,0 +1,28 @@
> +/* SPDX-License-Identifier: GPL-2.0
> + * Copyright (C) 2018 Arm Ltd.
> + */
> +
> +#ifndef __KVM_ARM_PV_H
> +#define __KVM_ARM_PV_H
> +
> +#include <linux/arm-smccc.h>
> +
> +#define ARM_SMCCC_HV_PV_FEATURES \
> + ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL, \
> + ARM_SMCCC_SMC_64, \
> + ARM_SMCCC_OWNER_HYP_STANDARD, \
> + 0x20)
> +
> +#define ARM_SMCCC_HV_PV_TIME_LPT \
> + ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL, \
> + ARM_SMCCC_SMC_64, \
> + ARM_SMCCC_OWNER_HYP_STANDARD, \
> + 0x21)
> +
> +#define ARM_SMCCC_HV_PV_TIME_ST \
> + ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL, \
> + ARM_SMCCC_SMC_64, \
> + ARM_SMCCC_OWNER_HYP_STANDARD, \
> + 0x22)
> +
> +#endif /* __KVM_ARM_PV_H */
Do these need to live in a separate header, away from the struct
definitions?
I'd be happy for these to live in <linux/arm-smccc.h>, given they're
standard calls.
As before, a comment referring to ARM DEN0057A would be nice.
> diff --git a/include/linux/arm-smccc.h b/include/linux/arm-smccc.h
> index b047009e7a0a..4e0866cc48c0 100644
> --- a/include/linux/arm-smccc.h
> +++ b/include/linux/arm-smccc.h
> @@ -54,6 +54,7 @@
> #define ARM_SMCCC_OWNER_SIP 2
> #define ARM_SMCCC_OWNER_OEM 3
> #define ARM_SMCCC_OWNER_STANDARD 4
> +#define ARM_SMCCC_OWNER_HYP_STANDARD 5
Minor nit, but could we make that STANDARD_HYP?
> #define ARM_SMCCC_OWNER_TRUSTED_APP 48
> #define ARM_SMCCC_OWNER_TRUSTED_APP_END 49
> #define ARM_SMCCC_OWNER_TRUSTED_OS 50
> diff --git a/virt/kvm/arm/hypercalls.c b/virt/kvm/arm/hypercalls.c
> index 153aa7642100..ba13b798f0f8 100644
> --- a/virt/kvm/arm/hypercalls.c
> +++ b/virt/kvm/arm/hypercalls.c
> @@ -5,6 +5,7 @@
> #include <linux/kvm_host.h>
>
> #include <asm/kvm_emulate.h>
> +#include <asm/pvclock-abi.h>
>
> #include <kvm/arm_hypercalls.h>
> #include <kvm/arm_psci.h>
> @@ -40,6 +41,14 @@ int kvm_hvc_call_handler(struct kvm_vcpu *vcpu)
> break;
> }
> break;
> + case ARM_SMCCC_HV_PV_FEATURES:
> + val = SMCCC_RET_SUCCESS;
> + break;
> + }
> + break;
> + case ARM_SMCCC_HV_PV_FEATURES:
> + feature = smccc_get_arg1(vcpu);
> + switch (feature) {
> }
IIUC, at this point in time, this happens to always return
SMCCC_RET_NOT_SUPPORTED.
If you leave this part out of the patch, and add it as required, this
patch is purely adding definitions, which would be a bit nicer for
review.
Thanks,
Mark.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v5 1/5] spi: spi-mem: Add driver for NXP FlexSPI controller
From: Boris Brezillon @ 2018-12-10 10:41 UTC (permalink / raw)
To: Schrempf Frieder
Cc: mark.rutland@arm.com, devicetree@vger.kernel.org,
Yogesh Narayan Gaur, robh@kernel.org,
linux-kernel@vger.kernel.org, linux-spi@vger.kernel.org,
marek.vasut@gmail.com, broonie@kernel.org,
linux-mtd@lists.infradead.org, computersforpeace@gmail.com,
shawnguo@kernel.org, linux-arm-kernel@lists.infradead.org
In-Reply-To: <b616a9c7-40cf-d2da-b164-0d56a82c6cbc@kontron.de>
On Mon, 10 Dec 2018 10:35:35 +0000
Schrempf Frieder <frieder.schrempf@kontron.de> wrote:
> >>> +
> >>> +static int nxp_fspi_exec_op(struct spi_mem *mem, const struct
> >>> +spi_mem_op *op) {
> >>> + struct nxp_fspi *f = spi_controller_get_devdata(mem->spi->master);
> >>> + int err = 0;
> >>> +
> >>> + mutex_lock(&f->lock);
> >>> +
> >>> + /* Wait for controller being ready. */
> >>> + err = fspi_readl_poll_tout(f, f->iobase + FSPI_STS0,
> >>> + FSPI_STS0_ARB_IDLE, 1, POLL_TOUT, true);
> >>> + WARN_ON(err);
> >>> +
> >>> + nxp_fspi_select_mem(f, mem->spi);
> >>> +
> >>> + nxp_fspi_prepare_lut(f, op);
> >>> + /*
> >>> + * If we have large chunks of data, we read them through the AHB bus
> >>> + * by accessing the mapped memory. In all other cases we use
> >>> + * IP commands to access the flash.
> >>> + */
> >>> + if (op->data.nbytes > (f->devtype_data->rxfifo - 4) &&
> >>> + op->data.dir == SPI_MEM_DATA_IN) {
> >>> + nxp_fspi_read_ahb(f, op);
> >>> + } else {
> >>> + if (op->data.nbytes && op->data.dir == SPI_MEM_DATA_OUT)
> >>> + nxp_fspi_fill_txfifo(f, op);
> >>> +
> >>> + err = nxp_fspi_do_op(f, op);
> >>> +
> >>> + /* Invalidate the data in the AHB buffer. */
> >>> + if (op->data.nbytes && op->data.dir == SPI_MEM_DATA_OUT)
> >>> + nxp_fspi_invalid(f);
> >>
> >> E.g. in case of an erase operation or a NAND load page operation, the
> >> invalidation is not triggered, but flash/buffer contents have changed.
> >> So I'm not sure if this is enough...
> > Ok, would change this and have invalidate for all operations.
>
> Maybe you can find out the correct way through testing with NOR and NAND.
Or just invalidate the buffer every time you're doing a read through the
AHB. This should always work.
I also think we should quickly move to a model where AHB accesses are
reserved for dirmap, and regular spi-mem op are limited to non-ahb
reads.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* RE: [PATCH v5 1/5] spi: spi-mem: Add driver for NXP FlexSPI controller
From: Yogesh Narayan Gaur @ 2018-12-10 10:43 UTC (permalink / raw)
To: Boris Brezillon
Cc: mark.rutland@arm.com, devicetree@vger.kernel.org, robh@kernel.org,
linux-kernel@vger.kernel.org, Schrempf Frieder,
linux-spi@vger.kernel.org, marek.vasut@gmail.com,
broonie@kernel.org, linux-mtd@lists.infradead.org,
computersforpeace@gmail.com, shawnguo@kernel.org,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <20181210111909.35384eee@bbrezillon>
Hi Boris, Frieder,
> -----Original Message-----
> From: Boris Brezillon [mailto:boris.brezillon@bootlin.com]
> Sent: Monday, December 10, 2018 3:49 PM
> To: Yogesh Narayan Gaur <yogeshnarayan.gaur@nxp.com>
> Cc: Schrempf Frieder <frieder.schrempf@kontron.de>; linux-
> mtd@lists.infradead.org; marek.vasut@gmail.com; broonie@kernel.org; linux-
> spi@vger.kernel.org; devicetree@vger.kernel.org; robh@kernel.org;
> mark.rutland@arm.com; shawnguo@kernel.org; linux-arm-
> kernel@lists.infradead.org; computersforpeace@gmail.com; linux-
> kernel@vger.kernel.org
> Subject: Re: [PATCH v5 1/5] spi: spi-mem: Add driver for NXP FlexSPI controller
>
> On Mon, 10 Dec 2018 09:41:51 +0000
> Yogesh Narayan Gaur <yogeshnarayan.gaur@nxp.com> wrote:
>
> > > > +/* Instead of busy looping invoke readl_poll_timeout functionality.
> > > > +*/ static int fspi_readl_poll_tout(struct nxp_fspi *f, void __iomem *base,
> > > > + u32 mask, u32 delay_us,
> > > > + u32 timeout_us, bool condition) {
> > > > + u32 reg;
> > > > +
> > > > + if (!f->devtype_data->little_endian)
> > > > + mask = (u32)cpu_to_be32(mask);
> > > > +
> > > > + if (condition)
> > > > + return readl_poll_timeout(base, reg, (reg & mask),
> > > > + delay_us, timeout_us);
> > > > + else
> > > > + return readl_poll_timeout(base, reg, !(reg & mask),
> > > > + delay_us, timeout_us);
> > >
> > > I would rather use a local variable to store the condition:
> > >
> > > bool c = condition ? (reg & mask):!(reg & mask);
> > >
> > With these type of usage getting below warning messages.
> >
> > drivers/spi/spi-nxp-fspi.c: In function ‘fspi_readl_poll_tout.isra.10.constprop’:
> > drivers/spi/spi-nxp-fspi.c:446:21: warning: ‘reg’ may be used uninitialized in
> this function [-Wmaybe-uninitialized]
> > bool cn = c ? (reg & mask) : !(reg & mask);
> >
> > If assign value to reg = 0xffffffff then timeout is start getting hit for False case
> and if assign value 0 then start getting timeout hit for true case.
> >
> > I would rather not try to modify this function.
>
> I agree. Let's keep this function readable even if this implies duplicating a few
> lines of code.
>
> >
> > > return readl_poll_timeout(base, reg, c, delay_us, timeout_us);
> > >
> > > > +}
> > > > +
> > > > +/*
> > > > + * If the slave device content being changed by Write/Erase, need
> > > > +to
> > > > + * invalidate the AHB buffer. This can be achieved by doing the
> > > > +reset
> > > > + * of controller after setting MCR0[SWRESET] bit.
> > > > + */
> > > > +static inline void nxp_fspi_invalid(struct nxp_fspi *f) {
> > > > + u32 reg;
> > > > + int ret;
> > > > +
> > > > + reg = fspi_readl(f, f->iobase + FSPI_MCR0);
> > > > + fspi_writel(f, reg | FSPI_MCR0_SWRST, f->iobase + FSPI_MCR0);
> > > > +
> > > > + /* w1c register, wait unit clear */
> > > > + ret = fspi_readl_poll_tout(f, f->iobase + FSPI_MCR0,
> > > > + FSPI_MCR0_SWRST, 0, POLL_TOUT, false);
> > > > + WARN_ON(ret);
> > > > +}
> > > > +
> > > > +static void nxp_fspi_prepare_lut(struct nxp_fspi *f,
> > > > + const struct spi_mem_op *op) {
> > > > + void __iomem *base = f->iobase;
> > > > + u32 lutval[4] = {};
> > > > + int lutidx = 1, i;
> > > > +
> > > > + /* cmd */
> > > > + lutval[0] |= LUT_DEF(0, LUT_CMD, LUT_PAD(op->cmd.buswidth),
> > > > + op->cmd.opcode);
> > > > +
> > > > + /* addr bus width */
> > > > + if (op->addr.nbytes) {
> > > > + u32 addrlen = 0;
> > > > +
> > > > + switch (op->addr.nbytes) {
> > > > + case 1:
> > > > + addrlen = ADDR8BIT;
> > > > + break;
> > > > + case 2:
> > > > + addrlen = ADDR16BIT;
> > > > + break;
> > > > + case 3:
> > > > + addrlen = ADDR24BIT;
> > > > + break;
> > > > + case 4:
> > > > + addrlen = ADDR32BIT;
> > > > + break;
> > > > + default:
> > > > + dev_err(f->dev, "In-correct address length\n");
> > > > + return;
> > > > + }
> > >
> > > You don't need to validate op->addr.nbytes here, this is already
> > > done in nxp_fspi_supports_op().
> >
> > Yes, I need to validate op->addr.nbytes else LUT would going to be
> programmed for 0 addrlen.
> > I have checked this on the target.
>
> Also agree there. Some operations have 0 address bytes. We could also test
> addr.buswidth, but I'm fine with the addr.nbytes test too.
>
>
> > > > +static void nxp_fspi_select_mem(struct nxp_fspi *f, struct
> > > > +spi_device
> > > > +*spi) {
> > > > + unsigned long rate = spi->max_speed_hz;
> > > > + int ret;
> > > > + uint64_t size_kb;
> > > > +
> > > > + /*
> > > > + * Return, if previously selected slave device is same as current
> > > > + * requested slave device.
> > > > + */
> > > > + if (f->selected == spi->chip_select)
> > > > + return;
> > > > +
> > > > + /* Reset FLSHxxCR0 registers */
> > > > + fspi_writel(f, 0, f->iobase + FSPI_FLSHA1CR0);
> > > > + fspi_writel(f, 0, f->iobase + FSPI_FLSHA2CR0);
> > > > + fspi_writel(f, 0, f->iobase + FSPI_FLSHB1CR0);
> > > > + fspi_writel(f, 0, f->iobase + FSPI_FLSHB2CR0);
> > > > +
> > > > + /* Assign controller memory mapped space as size, KBytes, of flash. */
> > > > + size_kb = FSPI_FLSHXCR0_SZ(f->memmap_phy_size);
> > >
> > Above description of this function, explains the reason for using
> memmap_phy_size.
> > This is not the arbitrary size, but the memory mapped size being assigned to
> the controller.
> >
> > > You are still using memory of arbitrary size (memmap_phy_size) for
> > > mapping the flash. Why not use the same approach as in the QSPI
> > > driver and just map ahb_buf_size until we implement the dirmap API?
> > The approach which being used in QSPI driver didn't work here, I have tried
> with that.
> > In QSPI driver, while preparing LUT we are assigning read/write address in the
> LUT preparation and have to for some unknown hack have to provide macro for
> LUT_MODE instead of LUT_ADDR.
> > But this thing didn't work for FlexSPI.
> > I discussed with HW IP owner and they suggested only to use LUT_ADDR for
> specifying the address length of the command i.e. 3-byte or 4-byte address
> command (NOR) or 1-2 byte address command for NAND.
>
> Actually, we would have used a LUT_ADDR too if the QSPI IP was support ADDR
> instructions with a number of bytes < 3, but for some unknown reasons it does
> not work.
>
> >
> > Thus, in LUT preparation we have assigned only the base address.
> > Now if I have assigned ahb_buf_size to FSPI_FLSHXXCR0 register then for
> read/write data beyond limit of ahb_buf_size offset I get data corruption.
>
> Why would you do that? We have the ->adjust_op_size() exactly for this reason,
> so, if someone tries to do a spi_mem_op with data.nbytes > ahb_buf_size you
> should return an error.
>
Let me explain my implementation with example. If I have to write data of size 0x100 bytes at offset 0x1200 for CS1, I would program as below:
In func nxp_fspi_select_mem(), would set value of controller address space size, memmap_phy_size, to FSPI_FLSHA2CR0 and rest all FSPI_FLSHXXCR0 as 0.
Value of memmap_phy_size is 0x10000000 i.e. 256 MB for my LX2160ARDB target.
Then in nxp_fspi_prepare_lut(), I would prepare LUT ADDR with address length requirement 3/4 byte for NOR or 1/2/3/4 bytes for NAND flash.
Also for LUT_NXP_WRITE would program data bytes as 0.
Then inside func nxp_fspi_do_op(), set register FSPI_IPCR0 as the address offset i.e. 0x1200 and in register FSPI_IPCR1 program the data size to write i.e. 0x100
If, as suggested if I tries to mark value of register FSPI_FLSHA2CR0 equal to ahb_buf_size (0x800), then access for address 0x1200 gives me wrong data. This is because as per the controller specification access to flash connected at CS1 can be performed under range of FSPI_ FLSHA1CR0 and FSPI_ FLSHA2CR0.
So either FSPI_ FLSHA2CR0 should have the value of the actual connected flash size but we don't have mechanism to know the flash size in current implementation.
Thus instead of using some other arbitrary value, I have used the full size being allocated to the FlexSPI controller.
> >
> > Thus, for generic approach have assigned FSPI_FLSHXXCR0 equal to the
> memory mapped size to the controller. This would also not going to depend on
> the number of CS present on the target.
>
> I kind of agree with Frieder on that one, I think it's preferable to limit the per-
> read-op size to ahb_buf_size and let the upper layer split the request in several
> sub-requests. On the controller side of things, you just have to have a mapping
> of ahb_buf_size per-CS. If you want to further optimize things, implement the
> dirmap hooks.
>
> >
> > > You are already aligning the AHB reads for this in nxp_fspi_adjust_op_size().
> > >
> > Yes, max read data size can be ahb_buf_size. Thus we need to check max read
> size with ahb_buf_size.
>
> Well, it's never a bad thing to check it twice, just in case the spi-mem user is
> misusing the API.
>
> > > > +static void nxp_fspi_fill_txfifo(struct nxp_fspi *f,
> > > > + const struct spi_mem_op *op) {
> > > > + void __iomem *base = f->iobase;
> > > > + int i, j, ret;
> > > > + int size, tmp_size, wm_size;
> > > > + u32 data = 0;
> > > > + u32 *txbuf = (u32 *) op->data.buf.out;
> > > > +
> > > > + /* clear the TX FIFO. */
> > > > + fspi_writel(f, FSPI_IPTXFCR_CLR, base + FSPI_IPTXFCR);
> > > > +
> > > > + /* Default value of water mark level is 8 bytes. */
> > > > + wm_size = 8;
> > > > + size = op->data.nbytes / wm_size;
> > > > + for (i = 0; i < size; i++) {
> > > > + /* Wait for TXFIFO empty */
> > > > + ret = fspi_readl_poll_tout(f, f->iobase + FSPI_INTR,
> > > > + FSPI_INTR_IPTXWE, 0,
> > > > + POLL_TOUT, true);
> > > > + WARN_ON(ret);
> > > > +
> > > > + j = 0;
> > > > + tmp_size = wm_size;
> > > > + while (tmp_size > 0) {
> > > > + data = 0;
> > > > + memcpy(&data, txbuf, 4);
> > > > + fspi_writel(f, data, base + FSPI_TFDR + j * 4);
> > > > + tmp_size -= 4;
> > > > + j++;
> > > > + txbuf += 1;
> > > > + }
> > > > + fspi_writel(f, FSPI_INTR_IPTXWE, base + FSPI_INTR);
> > > > + }
> > > > +
> > > > + size = op->data.nbytes % wm_size;
> > > > + if (size) {
> > > > + /* Wait for TXFIFO empty */
> > > > + ret = fspi_readl_poll_tout(f, f->iobase + FSPI_INTR,
> > > > + FSPI_INTR_IPTXWE, 0,
> > > > + POLL_TOUT, true);
> > > > + WARN_ON(ret);
> > > > +
> > > > + j = 0;
> > > > + tmp_size = 0;
> > > > + while (size > 0) {
> > > > + data = 0;
> > > > + tmp_size = (size < 4) ? size : 4;
> > > > + memcpy(&data, txbuf, tmp_size);
> > > > + fspi_writel(f, data, base + FSPI_TFDR + j * 4);
> > > > + size -= tmp_size;
> > > > + j++;
> > > > + txbuf += 1;
> > > > + }
> > > > + fspi_writel(f, FSPI_INTR_IPTXWE, base + FSPI_INTR);
> > > > + }
> > >
> > > All these nested loops to fill the TX buffer and also the ones below
> > > to read the RX buffer look much more complicated than they should
> > > really be. Can you try to make this more readable?
> > Yes
> > >
> > > Maybe something like this would work:
> > >
> > > for (i = 0; i < ALIGN_DOWN(op->data.nbytes, 8); i += 8) {
> > > /* Wait for TXFIFO empty */
> > > ret = fspi_readl_poll_tout(f, f->iobase + FSPI_INTR,
> > > FSPI_INTR_IPTXWE, 0,
> > > POLL_TOUT, true);
> > >
> > > fspi_writel(f, op->data.buf.out + i, base + FSPI_TFDR);
> > > fspi_writel(f, op->data.buf.out + i + 4, base + FSPI_TFDR + 4);
> > > fspi_writel(f, FSPI_INTR_IPTXWE, base + FSPI_INTR); }
> > With this above 2 lines we are hardcoding it for read/write with watermark
> size as 8 bytes.
> > Watermark size can be variable and depends on the value of
> > IPRXFCR/IPTXFCR register with default value as 8 bytes Thus, I would still
> prefer to use the internal for loop instead of 2 fspi_writel(...) for FSPI_TFDR and
> FSPI_TFDR + 4 register write commands.
>
> Just like you're hardcoding wm_size to 8, so I don't see a difference here. And I
> indeed prefer Frieder's version.
Ok. But, instead of hardcoding and doing fspi_writel() twice, I have modified this as below in my upcoming next version
for (tmp = wm_size, j = 0; tmp > 0; tmp -= 4, j++)
fspi_writel(f, *txbuf++, base + FSPI_TFDR + j * 4);
This would going to give us freedom for future use if watermark size is being used as 16 or 24 (max 64) instead of its current default value of 8.
--
Regards
Yogesh Gaur
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox