Re: [PATCH v6 3/3] qe_common: add qe_muram_ functions to manage muram
On Tue, 2015-08-25 at 02:52 -0500, Zhao Qiang-B45475 wrote: On Tue, 2015-08-25 at 12:35 +0800, Wood Scott-B07421 wrote: -Original Message- From: Wood Scott-B07421 Sent: Tuesday, August 25, 2015 12:35 AM To: Zhao Qiang-B45475 Cc: linux-ker...@vger.kernel.org; linuxppc-dev@lists.ozlabs.org; lau...@codeaurora.org; Xie Xiaobo-R63061; b...@kernel.crashing.org; Li Yang-Leo-R58472; pau...@samba.org Subject: Re: [PATCH v6 3/3] qe_common: add qe_muram_ functions to manage muram On Mon, 2015-08-24 at 17:31 +0800, Zhao Qiang wrote: @@ -187,12 +190,41 @@ static inline int qe_alive_during_sleep(void) } /* we actually use cpm_muram implementation, define this for convenience */ -#define qe_muram_init cpm_muram_init -#define qe_muram_alloc cpm_muram_alloc -#define qe_muram_alloc_fixed cpm_muram_alloc_fixed -#define qe_muram_free cpm_muram_free -#define qe_muram_addr cpm_muram_addr -#define qe_muram_offset cpm_muram_offset +int qe_muram_init(void); + +#if defined(CONFIG_QUICC_ENGINE) +unsigned long qe_muram_alloc(unsigned long size, unsigned long align); +int qe_muram_free(unsigned long offset); +void __iomem *qe_muram_addr(unsigned long offset); +unsigned long qe_muram_offset(void __iomem *addr); +dma_addr_t qe_muram_dma(void __iomem *addr); +#else +static inline unsigned long qe_muram_alloc(unsigned long size, + unsigned long align) +{ + return -ENOSYS; +} What code calls these functions without CONFIG_QUICC_ENGINE? Are you converting qe without cpm? Why? CPM just work on PowerPC old boards, it is not necessary to convert it. I disagree. Converting it would remove a user of rheap, and not converting it introduces code duplication. The muram code is currently shared between CPM and QE, so converting it doesn't add much effort. -Scott ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH v6 1/3] genalloc:support memory-allocation with bytes-alignment to genalloc
On Tue, 2015-08-25 at 03:09 -0500, Zhao Qiang-B45475 wrote: On 08/25/2015 12:01 PM, Laura Abbott wrote: -Original Message- From: Laura Abbott [mailto:labb...@redhat.com] Sent: Tuesday, August 25, 2015 12:01 PM To: Zhao Qiang-B45475; Wood Scott-B07421 Cc: linux-ker...@vger.kernel.org; linuxppc-dev@lists.ozlabs.org; lau...@codeaurora.org; Xie Xiaobo-R63061; b...@kernel.crashing.org; Li Yang-Leo-R58472; pau...@samba.org Subject: Re: [PATCH v6 1/3] genalloc:support memory-allocation with bytes-alignment to genalloc On 08/24/2015 07:40 PM, Zhao Qiang wrote: On 08/25/2015 07:11 AM, Laura Abbott wrote: -Original Message- From: Laura Abbott [mailto:labb...@redhat.com] Sent: Tuesday, August 25, 2015 7:11 AM To: Zhao Qiang-B45475; Wood Scott-B07421 Cc: linux-ker...@vger.kernel.org; linuxppc-dev@lists.ozlabs.org; lau...@codeaurora.org; Xie Xiaobo-R63061; b...@kernel.crashing.org; Li Yang-Leo-R58472; pau...@samba.org Subject: Re: [PATCH v6 1/3] genalloc:support memory-allocation with bytes-alignment to genalloc On 08/24/2015 02:31 AM, Zhao Qiang wrote: Bytes alignment is required to manage some special RAM, so add gen_pool_first_fit_align to genalloc, meanwhile add gen_pool_alloc_data to pass data to gen_pool_first_fit_align(modify gen_pool_alloc as a wrapper) Signed-off-by: Zhao Qiang qiang.z...@freescale.com --- Changes for v6: - patches set v6 include a new patch because of using - genalloc to manage QE MURAM, patch 0001 is the new - patch, adding bytes alignment for allocation for use. include/linux/genalloc.h | 23 +++ lib/genalloc.c | 58 +++- 2 files changed, 72 insertions(+), 9 deletions(-) diff --git a/include/linux/genalloc.h b/include/linux/genalloc.h index 1ccaab4..55da07e 100644 --- a/include/linux/genalloc.h +++ b/include/linux/genalloc.h @@ -34,6 +34,7 @@ struct device; struct device_node; +struct gen_pool; /** * Allocation callback function type definition @@ -47,7 +48,7 @@ typedef unsigned long (*genpool_algo_t)(unsigned long *map, unsigned long size, unsigned long start, unsigned int nr, - void *data); + void *data, struct gen_pool *pool); /* * General purpose special memory pool descriptor. @@ -73,6 +74,13 @@ struct gen_pool_chunk { unsigned long bits[0]; /* bitmap for allocating memory chunk */ }; +/* + * gen_pool data descriptor for gen_pool_first_fit_align. + */ +struct genpool_data_align { + int align; /* alignment by bytes for starting address */ +}; + (sorry for chiming in late, I've been traveling) Is there an advantage here to wrapping this in a structure instead of just passing a pointer to an align integer? Please look at the commit message for ca279cf1065fb689abea1dc7d8c11787729bb185 which adds data: As I can't predict all the possible requirements/needs for all allocation uses cases, I add a free field 'void *data' to pass any needed information to the allocation function. For example 'data' could be used to handle a structure where you store the alignment, the expected memory bank, the requester device, or any information that could influence the allocation algorithm. Right, I understand what the purpose is but I'm not sure what you're getting from the structure vs passing a pointer, e.g. int align; align = 4; gen_pool_alloc_data(pool, size, align); it just seems to obfuscate what's going on by wrapping a single integer in a structure that's narrowly defined in a generic function right now. I guess it could change later which would necessitate having the structure but again it's so generic I'm not sure what else you would pass that would be applicable to all clients. Scott and me have discussed about this issue in my RFC patch. Please review: http://patchwork.ozlabs.org/patch/493297/ I don't see anything relevant in that discussion. I tend to favor always using a struct for this type of opaque data, for consistency and extendability, but in this case it really doesn't matter much either way. -Scott ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH v6 3/3] qe_common: add qe_muram_ functions to manage muram
On Tue, 2015-08-25 at 02:19 -0500, Zhao Qiang-B45475 wrote: On 08/25/2015 12:15 PM, Laura Abbott wrote -Original Message- From: Laura Abbott [mailto:labb...@redhat.com] Sent: Tuesday, August 25, 2015 12:15 PM To: Zhao Qiang-B45475; Wood Scott-B07421 Cc: linux-ker...@vger.kernel.org; linuxppc-dev@lists.ozlabs.org; lau...@codeaurora.org; Xie Xiaobo-R63061; b...@kernel.crashing.org; Li Yang-Leo-R58472; pau...@samba.org Subject: Re: [PATCH v6 3/3] qe_common: add qe_muram_ functions to manage muram On 08/24/2015 08:03 PM, Zhao Qiang wrote: -Original Message- From: Laura Abbott [mailto:labb...@redhat.com] Sent: Tuesday, August 25, 2015 7:32 AM To: Zhao Qiang-B45475; Wood Scott-B07421 Cc: linux-ker...@vger.kernel.org; linuxppc-dev@lists.ozlabs.org; lau...@codeaurora.org; Xie Xiaobo-R63061; b...@kernel.crashing.org; Li Yang-Leo-R58472; pau...@samba.org Subject: Re: [PATCH v6 3/3] qe_common: add qe_muram_ functions to manage muram There doesn't seem to be a check for allocation failure from the gen_alloc. gen_pool_alloc will return 0 if there is error, but if the address returned is just 0x0, it can't distinguish it is address or error. Yes, that's a bad limitation of gen_pool. Maybe one day that will get fixed. In a previous out of tree driver, I worked around this by offsetting the gen_pool_add by a constant so any return value was non-zero and out of memory was zero and then subtracting the constant off of the return value. Not sure if that's better or worse than just fixing gen_alloc. The workaround works for non alignment allocation, but for alignment allocation, It need to align bytes to addr 0, offsetting the gen_pool_add maybe make wrong alignment It would work if the offset you add is a multiple of the size of muram. -Scott ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH V4 0/6] Redesign SR-IOV on PowerNV
On 08/19/2015 12:01 PM, Wei Yang wrote: In original design, it tries to group VFs to enable more number of VFs in the system, when VF BAR is bigger than 64MB. This design has a flaw in which one error on a VF will interfere other VFs in the same group. This patch series change this design by using M64 BAR in Single PE mode to cover only one VF BAR. By doing so, it gives absolute isolation between VFs. With or without this patchset, this fails with a horrible loop of EEHs: rmmod mlx4_en mlx4_ib mlx4_core modprobe mlx4_core num_vfs=4 probe_vf=4 port_type_array=2,2 debug_level=1 No guest is needed, just boot and do these commands. The EEH error is pointing to a broken DMA address. iommu=nobypass fixed it for 4 VFs case but when I try 16 VFs, none is created. What is the correct base tree and what hardware did you use for the testing _exactly_? Mine is Ethernet controller: Mellanox Technologies MT27520 Family [ConnectX-3 Pro] with 128MB BARs and that works (just double checked - can create all 16 VFs) with PowerKVM 3.1 so it is not a hardware issue. v4: * rebase the code on top of v4.2-rc7 * switch back to use the dynamic version of pe_num_map and m64_map * split the memory allocation and PE assignment of pe_num_map to make it more easy to read * check pe_num_map value before free PE. * add the rename reason for pe_num_map and m64_map in change log v3: * return -ENOSPC when a VF has non-64bit prefetchable BAR * rename offset to pe_num_map and define it staticly * change commit log based on comments * define m64_map staticly v2: * clean up iov bar alignment calculation * change m64s to m64_bars * add a field to represent M64 Single PE mode will be used * change m64_wins to m64_map * calculate the gate instead of hard coded * dynamically allocate m64_map * dynamically allocate PE# * add a case to calculate iov bar alignment when M64 Single PE is used * when M64 Single PE is used, compare num_vfs with M64 BAR available number in system at first Wei Yang (6): powerpc/powernv: don't enable SRIOV when VF BAR has non 64bit-prefetchable BAR powerpc/powernv: simplify the calculation of iov resource alignment powerpc/powernv: use one M64 BAR in Single PE mode for one VF BAR powerpc/powernv: replace the hard coded boundary with gate powerpc/powernv: boundary the total VF BAR size instead of the individual one powerpc/powernv: allocate sparse PE# when using M64 BAR in Single PE mode arch/powerpc/include/asm/pci-bridge.h |7 +- arch/powerpc/platforms/powernv/pci-ioda.c | 328 +++-- 2 files changed, 175 insertions(+), 160 deletions(-) -- Alexey ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] powerpc/configs: Enable LEDS support
On 08/26/2015 05:56 AM, Stewart Smith wrote: Vasant Hegde hegdevas...@linux.vnet.ibm.com writes: Presently PowerNV LEDS driver is specific to FSP based PowerNV platform. Hence added it as 'm'. So that we will be loaded only if we have required device tree support. Stewart, Currently the *firmware* support is FSP specific. The driver should magically work on any OPAL machine that hooks up the OPAL calls and DT. Right? Yep. It will just work if we have appropriate device tree . I'll update the description and resend patch. The A in OPAL is for Abstraction, not P for Passthrough-to-fsp. :-) -Vasant ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH v6 3/3] qe_common: add qe_muram_ functions to manage muram
On Tue, 2015-08-25 at 12:35 +0800, Wood Scott-B07421 wrote: -Original Message- From: Wood Scott-B07421 Sent: Tuesday, August 25, 2015 12:35 AM To: Zhao Qiang-B45475 Cc: linux-ker...@vger.kernel.org; linuxppc-dev@lists.ozlabs.org; lau...@codeaurora.org; Xie Xiaobo-R63061; b...@kernel.crashing.org; Li Yang-Leo-R58472; pau...@samba.org Subject: Re: [PATCH v6 3/3] qe_common: add qe_muram_ functions to manage muram On Mon, 2015-08-24 at 17:31 +0800, Zhao Qiang wrote: @@ -187,12 +190,41 @@ static inline int qe_alive_during_sleep(void) } /* we actually use cpm_muram implementation, define this for convenience */ -#define qe_muram_init cpm_muram_init -#define qe_muram_alloc cpm_muram_alloc -#define qe_muram_alloc_fixed cpm_muram_alloc_fixed -#define qe_muram_free cpm_muram_free -#define qe_muram_addr cpm_muram_addr -#define qe_muram_offset cpm_muram_offset +int qe_muram_init(void); + +#if defined(CONFIG_QUICC_ENGINE) +unsigned long qe_muram_alloc(unsigned long size, unsigned long align); +int qe_muram_free(unsigned long offset); +void __iomem *qe_muram_addr(unsigned long offset); +unsigned long qe_muram_offset(void __iomem *addr); +dma_addr_t qe_muram_dma(void __iomem *addr); +#else +static inline unsigned long qe_muram_alloc(unsigned long size, + unsigned long align) +{ + return -ENOSYS; +} What code calls these functions without CONFIG_QUICC_ENGINE? Are you converting qe without cpm? Why? CPM just work on PowerPC old boards, it is not necessary to convert it. -Scott -Zhao ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH v2 0/6] powerpc: use jump label for {cpu,mmu}_has_feature()
* Kevin Hao haoke...@gmail.com wrote: Hi, v2: Drop the following two patches as suggested by Ingo and Peter: jump_label: no need to acquire the jump_label_mutex in jump_lable_init() jump_label: introduce DEFINE_STATIC_KEY_{TRUE,FALSE}_ARRAY macros v1: I have tried to change the {cpu,mmu}_has_feature() to use jump label two yeas ago [1]. But that codes seem a bit ugly. This is a reimplementation by moving the jump_label_init() much earlier so the jump label can be used in a very earlier stage. Boot test on p4080ds, t2080rdb and powermac (qemu). This patch series is against linux-next. [1] https://lists.ozlabs.org/pipermail/linuxppc-dev/2013-September/111026.html Kevin Hao (6): jump_label: make it possible for the archs to invoke jump_label_init() much earlier powerpc: invoke jump_label_init() in a much earlier stage powerpc: kill mfvtb() powerpc: move the cpu_has_feature to a separate file powerpc: use the jump label for cpu_has_feature powerpc: use jump label for mmu_has_feature arch/powerpc/include/asm/cacheflush.h | 1 + arch/powerpc/include/asm/cpufeatures.h | 34 ++ arch/powerpc/include/asm/cputable.h | 16 +++--- arch/powerpc/include/asm/cputime.h | 1 + arch/powerpc/include/asm/dbell.h| 1 + arch/powerpc/include/asm/dcr-native.h | 1 + arch/powerpc/include/asm/mman.h | 1 + arch/powerpc/include/asm/mmu.h | 29 ++ arch/powerpc/include/asm/reg.h | 9 arch/powerpc/include/asm/time.h | 3 ++- arch/powerpc/include/asm/xor.h | 1 + arch/powerpc/kernel/align.c | 1 + arch/powerpc/kernel/cputable.c | 37 + arch/powerpc/kernel/irq.c | 1 + arch/powerpc/kernel/process.c | 1 + arch/powerpc/kernel/setup-common.c | 1 + arch/powerpc/kernel/setup_32.c | 5 + arch/powerpc/kernel/setup_64.c | 4 arch/powerpc/kernel/smp.c | 1 + arch/powerpc/platforms/cell/pervasive.c | 1 + arch/powerpc/xmon/ppc-dis.c | 1 + kernel/jump_label.c | 3 +++ 22 files changed, 135 insertions(+), 18 deletions(-) create mode 100644 arch/powerpc/include/asm/cpufeatures.h Looks good to me! Thanks, Ingo ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] cxl: Release irqs if memory allocation fails
Acked-by: Ian Munsie imun...@au1.ibm.com ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH v6 3/3] qe_common: add qe_muram_ functions to manage muram
On 08/25/2015 12:15 PM, Laura Abbott wrote -Original Message- From: Laura Abbott [mailto:labb...@redhat.com] Sent: Tuesday, August 25, 2015 12:15 PM To: Zhao Qiang-B45475; Wood Scott-B07421 Cc: linux-ker...@vger.kernel.org; linuxppc-dev@lists.ozlabs.org; lau...@codeaurora.org; Xie Xiaobo-R63061; b...@kernel.crashing.org; Li Yang-Leo-R58472; pau...@samba.org Subject: Re: [PATCH v6 3/3] qe_common: add qe_muram_ functions to manage muram On 08/24/2015 08:03 PM, Zhao Qiang wrote: -Original Message- From: Laura Abbott [mailto:labb...@redhat.com] Sent: Tuesday, August 25, 2015 7:32 AM To: Zhao Qiang-B45475; Wood Scott-B07421 Cc: linux-ker...@vger.kernel.org; linuxppc-dev@lists.ozlabs.org; lau...@codeaurora.org; Xie Xiaobo-R63061; b...@kernel.crashing.org; Li Yang-Leo-R58472; pau...@samba.org Subject: Re: [PATCH v6 3/3] qe_common: add qe_muram_ functions to manage muram On 08/24/2015 02:31 AM, Zhao Qiang wrote: +out: + of_node_put(np); + return ret; +} + +/** + * qe_muram_alloc - allocate the requested size worth of multi-user +ram + * @size: number of bytes to allocate + * @align: requested alignment, in bytes + * + * This function returns an offset into the muram area. + * Use qe_dpram_addr() to get the virtual address of the area. + * Use qe_muram_free() to free the allocation. + */ +unsigned long qe_muram_alloc(unsigned long size, unsigned long +align) { + unsigned long start; + unsigned long flags; + struct muram_block *entry; + + spin_lock_irqsave(qe_muram_lock, flags); + muram_pool_data.align = align; + start = gen_pool_alloc(muram_pool, size); The advantage of creating gen_pool_alloc_data was so that you could pass in the align automatically without having to modify the structure. Is there a reason you aren't using that? + memset(qe_muram_addr(start), 0, size); There doesn't seem to be a check for allocation failure from the gen_alloc. gen_pool_alloc will return 0 if there is error, but if the address returned is just 0x0, it can't distinguish it is address or error. Yes, that's a bad limitation of gen_pool. Maybe one day that will get fixed. In a previous out of tree driver, I worked around this by offsetting the gen_pool_add by a constant so any return value was non-zero and out of memory was zero and then subtracting the constant off of the return value. Not sure if that's better or worse than just fixing gen_alloc. The workaround works for non alignment allocation, but for alignment allocation, It need to align bytes to addr 0, offsetting the gen_pool_add maybe make wrong alignment . Thanks, Laura ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH] powerpc/configs: Enable LEDS support
Commit 84ad6e5c added LEDS support for PowerNV platform. Lets update ppc64_defconfig to pick LEDS driver. Presently PowerNV LEDS driver is specific to FSP based PowerNV platform. Hence added it as 'm'. So that we will be loaded only if we have required device tree support. Also note that powernv LEDS driver needs NEW_LEDS and LEDS_CLASS as well. Hence added them to config file. Suggested-by: Michael Ellerman m...@ellerman.id.au Signed-off-by: Vasant Hegde hegdevas...@linux.vnet.ibm.com --- Michael, This is PowerNV specific config. Hence I've not updated pseries_defconfig. Let me know if you want me to update pseries_defconfig config as well. Vasant arch/powerpc/configs/ppc64_defconfig | 3 +++ 1 file changed, 3 insertions(+) diff --git a/arch/powerpc/configs/ppc64_defconfig b/arch/powerpc/configs/ppc64_defconfig index a97efc2..6bc0ee4 100644 --- a/arch/powerpc/configs/ppc64_defconfig +++ b/arch/powerpc/configs/ppc64_defconfig @@ -355,3 +355,6 @@ CONFIG_CRYPTO_DEV_NX_ENCRYPT=m CONFIG_VIRTUALIZATION=y CONFIG_KVM_BOOK3S_64=m CONFIG_KVM_BOOK3S_64_HV=m +CONFIG_NEW_LEDS=y +CONFIG_LEDS_CLASS=m +CONFIG_LEDS_POWERNV=m -- 2.1.0 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH v6 1/3] genalloc:support memory-allocation with bytes-alignment to genalloc
On 08/25/2015 12:01 PM, Laura Abbott wrote: -Original Message- From: Laura Abbott [mailto:labb...@redhat.com] Sent: Tuesday, August 25, 2015 12:01 PM To: Zhao Qiang-B45475; Wood Scott-B07421 Cc: linux-ker...@vger.kernel.org; linuxppc-dev@lists.ozlabs.org; lau...@codeaurora.org; Xie Xiaobo-R63061; b...@kernel.crashing.org; Li Yang-Leo-R58472; pau...@samba.org Subject: Re: [PATCH v6 1/3] genalloc:support memory-allocation with bytes-alignment to genalloc On 08/24/2015 07:40 PM, Zhao Qiang wrote: On 08/25/2015 07:11 AM, Laura Abbott wrote: -Original Message- From: Laura Abbott [mailto:labb...@redhat.com] Sent: Tuesday, August 25, 2015 7:11 AM To: Zhao Qiang-B45475; Wood Scott-B07421 Cc: linux-ker...@vger.kernel.org; linuxppc-dev@lists.ozlabs.org; lau...@codeaurora.org; Xie Xiaobo-R63061; b...@kernel.crashing.org; Li Yang-Leo-R58472; pau...@samba.org Subject: Re: [PATCH v6 1/3] genalloc:support memory-allocation with bytes-alignment to genalloc On 08/24/2015 02:31 AM, Zhao Qiang wrote: Bytes alignment is required to manage some special RAM, so add gen_pool_first_fit_align to genalloc, meanwhile add gen_pool_alloc_data to pass data to gen_pool_first_fit_align(modify gen_pool_alloc as a wrapper) Signed-off-by: Zhao Qiang qiang.z...@freescale.com --- Changes for v6: - patches set v6 include a new patch because of using - genalloc to manage QE MURAM, patch 0001 is the new - patch, adding bytes alignment for allocation for use. include/linux/genalloc.h | 23 +++ lib/genalloc.c | 58 +++- 2 files changed, 72 insertions(+), 9 deletions(-) diff --git a/include/linux/genalloc.h b/include/linux/genalloc.h index 1ccaab4..55da07e 100644 --- a/include/linux/genalloc.h +++ b/include/linux/genalloc.h @@ -34,6 +34,7 @@ struct device; struct device_node; +struct gen_pool; /** * Allocation callback function type definition @@ -47,7 +48,7 @@ typedef unsigned long (*genpool_algo_t)(unsigned long *map, unsigned long size, unsigned long start, unsigned int nr, - void *data); + void *data, struct gen_pool *pool); /* * General purpose special memory pool descriptor. @@ -73,6 +74,13 @@ struct gen_pool_chunk { unsigned long bits[0]; /* bitmap for allocating memory chunk */ }; +/* + * gen_pool data descriptor for gen_pool_first_fit_align. + */ +struct genpool_data_align { + int align; /* alignment by bytes for starting address */ +}; + (sorry for chiming in late, I've been traveling) Is there an advantage here to wrapping this in a structure instead of just passing a pointer to an align integer? Please look at the commit message for ca279cf1065fb689abea1dc7d8c11787729bb185 which adds data: As I can't predict all the possible requirements/needs for all allocation uses cases, I add a free field 'void *data' to pass any needed information to the allocation function. For example 'data' could be used to handle a structure where you store the alignment, the expected memory bank, the requester device, or any information that could influence the allocation algorithm. Right, I understand what the purpose is but I'm not sure what you're getting from the structure vs passing a pointer, e.g. int align; align = 4; gen_pool_alloc_data(pool, size, align); it just seems to obfuscate what's going on by wrapping a single integer in a structure that's narrowly defined in a generic function right now. I guess it could change later which would necessitate having the structure but again it's so generic I'm not sure what else you would pass that would be applicable to all clients. Scott and me have discussed about this issue in my RFC patch. Please review: http://patchwork.ozlabs.org/patch/493297/ Thanks, Laura ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 3/3] mmc: sdhci-of-esdhc: add workaround for T4240 incorrect HOSTVER value
On 27 July 2015 at 17:57, Scott Wood scottw...@freescale.com wrote: On Mon, 2015-07-27 at 09:58 +0200, Ulf Hansson wrote: On 25 July 2015 at 04:27, Scott Wood scottw...@freescale.com wrote: On Tue, 2015-07-21 at 15:02 +0200, Ulf Hansson wrote: On 21 July 2015 at 11:45, Yangbo Lu yangbo...@freescale.com wrote: For T4240-R1.0-R2.0, the HOSTVER register has incorrcet vender version value and sdhc spec version value. This will break down the ADMA data transfer. So add workaround to get right value VVN=0x13, SVN = 0x1. So T4240-R1.0-R2.0 is the version of the controller, right? If I understand correct you are checking what CPU/SoC you are running on, to figure out which controller version you are using, as that can't be fetched (trusted) from the registers of the esdhc controller itself!? Instead, you could deal with this directly in the DTS files. I assume you have some DTS file for each SoC/board variant, right? No, we do not have a separate DTS file for each revision of an SoC -- and if we did, we'd constantly have people using the wrong one. In principle, in your DTS file specific for the board/SoC that holds the T4240-R1.0-R2.0 version of the controller, should add a specific esdhc DT property to indicate this errata. No, because (in addition to the above issue about chip revisions) the device tree is stable ABI and errata are often discovered after device trees are deployed. Fair enough. Then what is your suggestion for the solution here? As I said in my comment on patch 2/3, read SVR from the device-config/guts MMIO block, which works on both PPC and ARM. That's okay, as long as don't have include sections of non generic header files in the driver. To be more clear, this is not okay to have: #include mach/* Kind regards Uffe ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] powerpc/configs: Enable LEDS support
Vasant Hegde hegdevas...@linux.vnet.ibm.com writes: Presently PowerNV LEDS driver is specific to FSP based PowerNV platform. Hence added it as 'm'. So that we will be loaded only if we have required device tree support. Currently the *firmware* support is FSP specific. The driver should magically work on any OPAL machine that hooks up the OPAL calls and DT. Right? The A in OPAL is for Abstraction, not P for Passthrough-to-fsp. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [RFC PATCH 14/17] powerpc/book3e-64/kexec: Enable SMP release
On Tue, 2015-08-25 at 11:57 +1000, Michael Ellerman wrote: On Mon, 2015-08-24 at 15:25 -0500, Scott Wood wrote: On Thu, 2015-08-20 at 14:54 +1000, Michael Ellerman wrote: Hi Scott, Sorry for the delay. So I'm back to square one on this patch. On Sat, 2015-07-18 at 15:08 -0500, Scott Wood wrote: booted_from_exec is similar to __run_at_load, except that it is set for regular kexec as well as kdump. The flag is needed because the SMP release mechanism for FSL book3e is different from when booting with normal hardware. In theory we could simulate the normal spin table mechanism, but not at the addresses U-Boot put in the device tree -- so there'd need to be even more communication between the kernel and kexec to set that up. Since there's already a similar flag being set (for kdump only), this seemed like a reasonable approach. Although this is a reasonable approach, I don't think it's the best approach. AFAICS there's no reason why we can't use a device tree property for this, so I think we should do that. OK, I'll look into that. Thanks. diff --git a/arch/powerpc/platforms/85xx/smp.c b/arch/powerpc/platforms/85xx/smp.c index 5152289..4abda43 100644 --- a/arch/powerpc/platforms/85xx/smp.c +++ b/arch/powerpc/platforms/85xx/smp.c @@ -305,10 +310,13 @@ static int smp_85xx_kick_cpu(int nr) __secondary_hold_acknowledge = -1; } #endif - flush_spin_table(spin_table); - out_be32(spin_table-pir, hw_cpu); - out_be32(spin_table-addr_l, __pa(__early_start)); - flush_spin_table(spin_table); + + if (have_spin_table) { + flush_spin_table(spin_table); + out_be32(spin_table-pir, hw_cpu); + out_be32(spin_table-addr_l, __pa(__early_start)); + flush_spin_table(spin_table); + } /* Wait a bit for the CPU to ack. */ if (!spin_event_timeout(__secondary_hold_acknowledge == hw_cpu, This looks like it's inside an #ifdef CONFIG_PPC32 block, which doesn't make sense, so I must be missing a lead-up patch or something? (I looked on the list but didn't find anything immediately) Thanks for catching this. This is apparently a mismerge due to the code having been previously worked on in the context of the SDK tree, which does not have that code inside #ifdef CONFIG_PPC32. When I then applied the result to mainline, everything still appeared to work, because there's no real consequence to writing to the spin table in this case -- it's just a no-op. Aha, that's good, I stared at it for ages thinking I was going mad, but I wasn't! setup_64.c is the part where checking booted_from_kexec (or devicetree equivalent) really matters. OK. Can we avoid that too? All smp_release_cpus() does is whack __secondary_hold_spinloop and then spin for a while. For the non-kexec case writing to __secondary_hold_spinloop should be harmless I think, so the only problem is we'll get stuck for a while in the udelay() loop. But you could avoid that by preemptively setting spinning_secondaries to 0 in platform code. That'd have to be in ppc_md.init_early(), but that's actually not very early, the device tree is already unflattened. I guess it's arguable whether that's more or less horrible than adding an #ifdef'ed booted_from_kexec check, but I think I'd prefer the spinning_secondaries solution. We'd still need the device tree property regardless of whether we keep use_spinloop() or set spinning_secondaries to zero. use_spinloop() (with a device tree property rather than booted_from_kexec) seems cleaner: - Avoids depending on the fact that some piece of platform code executes after spinning_secondaries is initialized but before smp_release_cpus(). - Doesn't put a different requirement on platform code based on 32 versus 64 bit (we have too many 32 versus 64 bit differences as is). - Doesn't require the change in all relevant platform code files (we have both corenet_generic and qemu_e500, both of which support both 32 and 64 bit, and custom boards might not all use corenet_generic), whether the platform supports kexec or not. I doesn't look like there's any non-Freescale book3e- 64 left in the kernel[1], but if it ever gets added, it would also be affected by a solution that requires platform code to do something to preserve the current behavior. -Scott [1] If this is true, and won't likely change, can the non-fsl book3e-64 TLB miss handlers and such come out? ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [RFC PATCH 14/17] powerpc/book3e-64/kexec: Enable SMP release
On Tue, 2015-08-25 at 18:40 -0500, Scott Wood wrote: On Tue, 2015-08-25 at 11:57 +1000, Michael Ellerman wrote: I guess it's arguable whether that's more or less horrible than adding an #ifdef'ed booted_from_kexec check, but I think I'd prefer the spinning_secondaries solution. We'd still need the device tree property regardless of whether we keep use_spinloop() or set spinning_secondaries to zero. Yep. use_spinloop() (with a device tree property rather than booted_from_kexec) seems cleaner: - Avoids depending on the fact that some piece of platform code executes after spinning_secondaries is initialized but before smp_release_cpus(). True, that is a bit fragile. - Doesn't put a different requirement on platform code based on 32 versus 64 bit (we have too many 32 versus 64 bit differences as is). Yeah I didn't think of that. - Doesn't require the change in all relevant platform code files (we have both corenet_generic and qemu_e500, both of which support both 32 and 64 bit, and custom boards might not all use corenet_generic), whether the platform supports kexec or not. Yep, though they could all call a common implementation of init_early(). So I guess do it with use_spinloop(). I was just hoping to avoid more platform specific ifdefs in the generic code. I doesn't look like there's any non-Freescale book3e-64 left in the kernel[1] ... [1] If this is true, and won't likely change, can the non-fsl book3e-64 TLB miss handlers and such come out? It is true, see fb5a515704d7 (powerpc: Remove platforms/wsp and associated pieces). It will not change as far as I'm aware, and all the code's in the git history anyway, so if there's unused code in there please rip it out. cheers ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH v6 3/3] qe_common: add qe_muram_ functions to manage muram
-Original Message- From: Wood Scott-B07421 Sent: Wednesday, August 26, 2015 12:23 AM To: Zhao Qiang-B45475 Cc: Laura Abbott; linux-ker...@vger.kernel.org; linuxppc- d...@lists.ozlabs.org; lau...@codeaurora.org; Xie Xiaobo-R63061; b...@kernel.crashing.org; Li Yang-Leo-R58472; pau...@samba.org Subject: Re: [PATCH v6 3/3] qe_common: add qe_muram_ functions to manage muram On Tue, 2015-08-25 at 02:19 -0500, Zhao Qiang-B45475 wrote: On 08/25/2015 12:15 PM, Laura Abbott wrote -Original Message- From: Laura Abbott [mailto:labb...@redhat.com] Sent: Tuesday, August 25, 2015 12:15 PM To: Zhao Qiang-B45475; Wood Scott-B07421 Cc: linux-ker...@vger.kernel.org; linuxppc-dev@lists.ozlabs.org; lau...@codeaurora.org; Xie Xiaobo-R63061; b...@kernel.crashing.org; Li Yang-Leo-R58472; pau...@samba.org Subject: Re: [PATCH v6 3/3] qe_common: add qe_muram_ functions to manage muram On 08/24/2015 08:03 PM, Zhao Qiang wrote: -Original Message- From: Laura Abbott [mailto:labb...@redhat.com] Sent: Tuesday, August 25, 2015 7:32 AM To: Zhao Qiang-B45475; Wood Scott-B07421 Cc: linux-ker...@vger.kernel.org; linuxppc-dev@lists.ozlabs.org; lau...@codeaurora.org; Xie Xiaobo-R63061; b...@kernel.crashing.org; Li Yang-Leo-R58472; pau...@samba.org Subject: Re: [PATCH v6 3/3] qe_common: add qe_muram_ functions to manage muram There doesn't seem to be a check for allocation failure from the gen_alloc. gen_pool_alloc will return 0 if there is error, but if the address returned is just 0x0, it can't distinguish it is address or error. Yes, that's a bad limitation of gen_pool. Maybe one day that will get fixed. In a previous out of tree driver, I worked around this by offsetting the gen_pool_add by a constant so any return value was non-zero and out of memory was zero and then subtracting the constant off of the return value. Not sure if that's better or worse than just fixing gen_alloc. The workaround works for non alignment allocation, but for alignment allocation, It need to align bytes to addr 0, offsetting the gen_pool_add maybe make wrong alignment It would work if the offset you add is a multiple of the size of muram. The QE apps ask different bytes alignment for different use due to hardware restriction. Why don’t we deal with it in gen_pool_alloc func instead of a workaround? It is more reasonable. -Scott ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH v6 3/3] qe_common: add qe_muram_ functions to manage muram
On Tue, 2015-08-25 at 20:49 -0500, Zhao Qiang-B45475 wrote: -Original Message- From: Wood Scott-B07421 Sent: Wednesday, August 26, 2015 12:23 AM To: Zhao Qiang-B45475 Cc: Laura Abbott; linux-ker...@vger.kernel.org; linuxppc- d...@lists.ozlabs.org; lau...@codeaurora.org; Xie Xiaobo-R63061; b...@kernel.crashing.org; Li Yang-Leo-R58472; pau...@samba.org Subject: Re: [PATCH v6 3/3] qe_common: add qe_muram_ functions to manage muram On Tue, 2015-08-25 at 02:19 -0500, Zhao Qiang-B45475 wrote: On 08/25/2015 12:15 PM, Laura Abbott wrote -Original Message- From: Laura Abbott [mailto:labb...@redhat.com] Sent: Tuesday, August 25, 2015 12:15 PM To: Zhao Qiang-B45475; Wood Scott-B07421 Cc: linux-ker...@vger.kernel.org; linuxppc-dev@lists.ozlabs.org; lau...@codeaurora.org; Xie Xiaobo-R63061; b...@kernel.crashing.org; Li Yang-Leo-R58472; pau...@samba.org Subject: Re: [PATCH v6 3/3] qe_common: add qe_muram_ functions to manage muram On 08/24/2015 08:03 PM, Zhao Qiang wrote: -Original Message- From: Laura Abbott [mailto:labb...@redhat.com] Sent: Tuesday, August 25, 2015 7:32 AM To: Zhao Qiang-B45475; Wood Scott-B07421 Cc: linux-ker...@vger.kernel.org; linuxppc-dev@lists.ozlabs.org; lau...@codeaurora.org; Xie Xiaobo-R63061; b...@kernel.crashing.org; Li Yang-Leo-R58472; pau...@samba.org Subject: Re: [PATCH v6 3/3] qe_common: add qe_muram_ functions to manage muram There doesn't seem to be a check for allocation failure from the gen_alloc. gen_pool_alloc will return 0 if there is error, but if the address returned is just 0x0, it can't distinguish it is address or error. Yes, that's a bad limitation of gen_pool. Maybe one day that will get fixed. In a previous out of tree driver, I worked around this by offsetting the gen_pool_add by a constant so any return value was non-zero and out of memory was zero and then subtracting the constant off of the return value. Not sure if that's better or worse than just fixing gen_alloc. The workaround works for non alignment allocation, but for alignment allocation, It need to align bytes to addr 0, offsetting the gen_pool_add maybe make wrong alignment It would work if the offset you add is a multiple of the size of muram. The QE apps ask different bytes alignment for different use due to hardware restriction. Why don’t we deal with it in gen_pool_alloc func instead of a workaround? It is more reasonable. Sure, fixing gen_pool_alloc would be better if you're willing to do it, and are careful not to break existing users. I was just pointing out that the workaround isn't totally incompatible with alignment. -Scott ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev