* Regression (ARM) arch/arm/mm/init.c doesn't build without CONFIG_ARM_PATCH_PHYS_VIRT.
@ 2013-12-31 11:20 Krzysztof Hałasa
2013-12-31 11:23 ` Russell King - ARM Linux
2014-01-06 19:28 ` Krzysztof Hałasa
0 siblings, 2 replies; 13+ messages in thread
From: Krzysztof Hałasa @ 2013-12-31 11:20 UTC (permalink / raw)
To: linux-arm-kernel
arch/arm/mm/init.c: In function 'setup_dma_zone':
arch/arm/mm/init.c:232:19: error: '__pv_phys_offset' undeclared (first use in this function)
Reverting the following commit fixes it:
commit 787b0d5c1ca7ff24feb6f92e4c7f4410ee7d81a8
Author: Santosh Shilimkar <santosh.shilimkar@ti.com>
Date: Mon Dec 2 20:29:12 2013 +0100
ARM: 7908/1: mm: Fix the arm_dma_limit calculation
Current code is using PHYS_OFFSET to calculate the arm_dma_limit which
will lead to wrong calculations in cases where PHYS_OFFSET is updated
runtime.
So fix the code by using __pv_phys_offset instead of PHYS_OFFSET.
It seems PHYS_OFFSET is equal to __pv_phys_offset if the latter is
available:
arch/arm/include/asm/memory.h:#define PHYS_OFFSET __pv_phys_offset
Otherwise (without CONFIG_ARM_PATCH_PHYS_VIRT) PHYS_OFFSET is a
hard-coded CONFIG value, or whatever the platform/CPU code needs:
arch/arm/include/asm/memory.h:#define PHYS_OFFSET PLAT_PHYS_OFFSET
Perhaps the patch in question was needed at some point but I think the
situation had changed before it was commited.
--
Krzysztof Halasa
Research Institute for Automation and Measurements PIAP
Al. Jerozolimskie 202, 02-486 Warsaw, Poland
^ permalink raw reply [flat|nested] 13+ messages in thread* Regression (ARM) arch/arm/mm/init.c doesn't build without CONFIG_ARM_PATCH_PHYS_VIRT. 2013-12-31 11:20 Regression (ARM) arch/arm/mm/init.c doesn't build without CONFIG_ARM_PATCH_PHYS_VIRT Krzysztof Hałasa @ 2013-12-31 11:23 ` Russell King - ARM Linux 2014-01-01 14:40 ` Krzysztof Hałasa 2014-01-06 19:28 ` Krzysztof Hałasa 1 sibling, 1 reply; 13+ messages in thread From: Russell King - ARM Linux @ 2013-12-31 11:23 UTC (permalink / raw) To: linux-arm-kernel On Tue, Dec 31, 2013 at 12:20:56PM +0100, Krzysztof Ha?asa wrote: > arch/arm/mm/init.c: In function 'setup_dma_zone': > arch/arm/mm/init.c:232:19: error: '__pv_phys_offset' undeclared (first use in this function) > > Reverting the following commit fixes it: Already fixed long before Christmas. Please use recent kernels. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Regression (ARM) arch/arm/mm/init.c doesn't build without CONFIG_ARM_PATCH_PHYS_VIRT. 2013-12-31 11:23 ` Russell King - ARM Linux @ 2014-01-01 14:40 ` Krzysztof Hałasa 0 siblings, 0 replies; 13+ messages in thread From: Krzysztof Hałasa @ 2014-01-01 14:40 UTC (permalink / raw) To: linux-arm-kernel Russell King - ARM Linux <linux@arm.linux.org.uk> writes: > On Tue, Dec 31, 2013 at 12:20:56PM +0100, Krzysztof Ha?asa wrote: >> arch/arm/mm/init.c: In function 'setup_dma_zone': >> arch/arm/mm/init.c:232:19: error: '__pv_phys_offset' undeclared (first use in this function) >> >> Reverting the following commit fixes it: > > Already fixed long before Christmas. Please use recent kernels. Do you mean something other than Linus's tree? The problem I noted is present in his tree as of Dec 30 - on both 3.13-rc6 and (then tip) 71ce176ee6ed1735b9a1160a5704a915d13849b1. Or... do you mean the commit that added this regression? -- Krzysztof Halasa Research Institute for Automation and Measurements PIAP Al. Jerozolimskie 202, 02-486 Warsaw, Poland ^ permalink raw reply [flat|nested] 13+ messages in thread
* Regression (ARM) arch/arm/mm/init.c doesn't build without CONFIG_ARM_PATCH_PHYS_VIRT. 2013-12-31 11:20 Regression (ARM) arch/arm/mm/init.c doesn't build without CONFIG_ARM_PATCH_PHYS_VIRT Krzysztof Hałasa 2013-12-31 11:23 ` Russell King - ARM Linux @ 2014-01-06 19:28 ` Krzysztof Hałasa 2014-01-06 19:33 ` Santosh Shilimkar 2014-01-06 22:39 ` Russell King - ARM Linux 1 sibling, 2 replies; 13+ messages in thread From: Krzysztof Hałasa @ 2014-01-06 19:28 UTC (permalink / raw) To: linux-arm-kernel Russell, Santosh, the unneeded commit causing regression is still in place. Please try to compile an ARM kernel without CONFIG_ARM_PATCH_PHYS_VIRT and with CONFIG_ZONE_DMA and see for yourself, if you don't believe me. Please be aware that this commit fixes nothing, its only function is causing the regression - so we don't lose anything by reverting it. If the attached wasn't clear, what the defective commit presently does is changing a perfectly valid code into a code referencing a variable which (without CONFIG_ARM_PATCH_PHYS_VIRT set) doesn't at all exist. With CONFIG_ARM_PATCH_PHYS_VIRT set, this commit does precisely nothing. grep __pv_phys_offset * -r gives: Broken by the commit in question: arch/arm/mm/init.c: arm_dma_limit = __pv_phys_offset + arm_dma_zone_size - 1; All of the following are only compiled/assembled with CONFIG_ARM_PATCH_PHYS_VIRT set: arch/arm/include/asm/memory.h:extern u64 __pv_phys_offset; arch/arm/include/asm/memory.h:#define PHYS_OFFSET __pv_phys_offset arch/arm/kernel/armksyms.c:EXPORT_SYMBOL(__pv_phys_offset); arch/arm/kernel/head.S: add r6, r6, r3 @ adjust __pv_phys_offset address arch/arm/kernel/head.S: str r8, [r6, #LOW_OFFSET] @ save computed PHYS_OFFSET to __pv_phys_offset arch/arm/kernel/head.S:2: .long __pv_phys_offset arch/arm/kernel/head.S: .globl __pv_phys_offset arch/arm/kernel/head.S: .type __pv_phys_offset, %object arch/arm/kernel/head.S:__pv_phys_offset: arch/arm/kernel/head.S: .size __pv_phys_offset, . -__pv_phys_offset In short, please revert 787b0d5c1ca7ff24feb6f92e4c7f4410ee7d81a8. Or, do you want me to send a patch which does that? HTH and TIA. > arch/arm/mm/init.c: In function 'setup_dma_zone': > arch/arm/mm/init.c:232:19: error: '__pv_phys_offset' undeclared (first use in this function) > > Reverting the following commit fixes it: > > commit 787b0d5c1ca7ff24feb6f92e4c7f4410ee7d81a8 > Author: Santosh Shilimkar <santosh.shilimkar@ti.com> > Date: Mon Dec 2 20:29:12 2013 +0100 > > ARM: 7908/1: mm: Fix the arm_dma_limit calculation > > Current code is using PHYS_OFFSET to calculate the arm_dma_limit which > will lead to wrong calculations in cases where PHYS_OFFSET is updated > runtime. > > So fix the code by using __pv_phys_offset instead of PHYS_OFFSET. > > It seems PHYS_OFFSET is equal to __pv_phys_offset if the latter is > available: > > arch/arm/include/asm/memory.h:#define PHYS_OFFSET __pv_phys_offset > > Otherwise (without CONFIG_ARM_PATCH_PHYS_VIRT) PHYS_OFFSET is a > hard-coded CONFIG value, or whatever the platform/CPU code needs: > > arch/arm/include/asm/memory.h:#define PHYS_OFFSET PLAT_PHYS_OFFSET > > Perhaps the patch in question was needed at some point but I think the > situation had changed before it was commited. -- Krzysztof Halasa Research Institute for Automation and Measurements PIAP Al. Jerozolimskie 202, 02-486 Warsaw, Poland ^ permalink raw reply [flat|nested] 13+ messages in thread
* Regression (ARM) arch/arm/mm/init.c doesn't build without CONFIG_ARM_PATCH_PHYS_VIRT. 2014-01-06 19:28 ` Krzysztof Hałasa @ 2014-01-06 19:33 ` Santosh Shilimkar 2014-01-06 22:08 ` Krzysztof Hałasa 2014-01-06 22:39 ` Russell King - ARM Linux 1 sibling, 1 reply; 13+ messages in thread From: Santosh Shilimkar @ 2014-01-06 19:33 UTC (permalink / raw) To: linux-arm-kernel On Monday 06 January 2014 02:28 PM, Krzysztof Ha?asa wrote: > Russell, Santosh, > > the unneeded commit causing regression is still in place. Please try to > compile an ARM kernel without CONFIG_ARM_PATCH_PHYS_VIRT and with > CONFIG_ZONE_DMA and see for yourself, if you don't believe me. > > Please be aware that this commit fixes nothing, its only function is > causing the regression - so we don't lose anything by reverting it. > I am afraid you didn't understood what the fix is if you say above. arm_dma_limit is broken without this fix for LPAE machines with memory starting 4 GB physical boundary. Regards, Santosh ^ permalink raw reply [flat|nested] 13+ messages in thread
* Regression (ARM) arch/arm/mm/init.c doesn't build without CONFIG_ARM_PATCH_PHYS_VIRT. 2014-01-06 19:33 ` Santosh Shilimkar @ 2014-01-06 22:08 ` Krzysztof Hałasa 2014-01-06 22:27 ` Santosh Shilimkar 0 siblings, 1 reply; 13+ messages in thread From: Krzysztof Hałasa @ 2014-01-06 22:08 UTC (permalink / raw) To: linux-arm-kernel Santosh Shilimkar <santosh.shilimkar@ti.com> writes: > I am afraid you didn't understood what the fix is if you say above. > arm_dma_limit is broken without this fix for LPAE machines with > memory starting 4 GB physical boundary. I wonder what CONFIG_ARM_PATCH_PHYS_VIRT and CONFIG_ZONE_DMA do they use? Of course, CONFIG_ZONE_DMA must be set, because otherwise the patch changes a part disabled with #ifdef. CONFIG_ARM_PATCH_PHYS_VIRT must be set because otherwise the kernel will not compile at all. Do you accept the fact the configurations with CONFIG_ZONE_DMA and without CONFIG_ARM_PATCH_PHYS_VIRT are now broken - by this very commit? Now, what "fix" does this commit in its entirety do: - arm_dma_limit = PHYS_OFFSET + arm_dma_zone_size - 1; + arm_dma_limit = __pv_phys_offset + arm_dma_zone_size - 1; if under the above circumstances (CONFIG_*) in arch/arm/include/asm/memory.h we have: extern u64 __pv_phys_offset; ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the only usable declaration of this variable extern u64 __pv_offset; extern void fixup_pv_table(const void *, unsigned long); extern const void *__pv_table_begin, *__pv_table_end; #define PHYS_OFFSET __pv_phys_offset ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -- Krzysztof Halasa Research Institute for Automation and Measurements PIAP Al. Jerozolimskie 202, 02-486 Warsaw, Poland ^ permalink raw reply [flat|nested] 13+ messages in thread
* Regression (ARM) arch/arm/mm/init.c doesn't build without CONFIG_ARM_PATCH_PHYS_VIRT. 2014-01-06 22:08 ` Krzysztof Hałasa @ 2014-01-06 22:27 ` Santosh Shilimkar 0 siblings, 0 replies; 13+ messages in thread From: Santosh Shilimkar @ 2014-01-06 22:27 UTC (permalink / raw) To: linux-arm-kernel On Monday 06 January 2014 05:08 PM, Krzysztof Ha?asa wrote: > Santosh Shilimkar <santosh.shilimkar@ti.com> writes: > >> I am afraid you didn't understood what the fix is if you say above. >> arm_dma_limit is broken without this fix for LPAE machines with >> memory starting 4 GB physical boundary. > > I wonder what CONFIG_ARM_PATCH_PHYS_VIRT and CONFIG_ZONE_DMA do they > use? > You need to grep where the CONFIG_ARM_PATCH_PHYS_VIRT is used ;) > Of course, CONFIG_ZONE_DMA must be set, because otherwise the patch > changes a part disabled with #ifdef. CONFIG_ARM_PATCH_PHYS_VIRT must be > set because otherwise the kernel will not compile at all. > > Do you accept the fact the configurations with CONFIG_ZONE_DMA and > without CONFIG_ARM_PATCH_PHYS_VIRT are now broken - by this very commit? > > Now, what "fix" does this commit in its entirety do: > > - arm_dma_limit = PHYS_OFFSET + arm_dma_zone_size - 1; > + arm_dma_limit = __pv_phys_offset + arm_dma_zone_size - 1; > > if under the above circumstances (CONFIG_*) in > arch/arm/include/asm/memory.h we have: > > extern u64 __pv_phys_offset; > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the only usable declaration of this variable > extern u64 __pv_offset; > extern void fixup_pv_table(const void *, unsigned long); > extern const void *__pv_table_begin, *__pv_table_end; > > #define PHYS_OFFSET __pv_phys_offset > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > Your grep actually missed the assembly files where the actual patching happens. Please have a look at "arch/arm/kernel/head.S" The "CONFIG_ARM_PATCH_PHYS_VIRT" is always enabled on almost all ARM builds since its needed for multi-config kernel to work. May be its time to make it default enabled but I let Russell comment if there is any better way to handle this. Regards, Santosh ^ permalink raw reply [flat|nested] 13+ messages in thread
* Regression (ARM) arch/arm/mm/init.c doesn't build without CONFIG_ARM_PATCH_PHYS_VIRT. 2014-01-06 19:28 ` Krzysztof Hałasa 2014-01-06 19:33 ` Santosh Shilimkar @ 2014-01-06 22:39 ` Russell King - ARM Linux 2014-01-06 23:42 ` Santosh Shilimkar 1 sibling, 1 reply; 13+ messages in thread From: Russell King - ARM Linux @ 2014-01-06 22:39 UTC (permalink / raw) To: linux-arm-kernel On Mon, Jan 06, 2014 at 08:28:20PM +0100, Krzysztof Ha?asa wrote: > Russell, Santosh, > > the unneeded commit causing regression is still in place. Please try to > compile an ARM kernel without CONFIG_ARM_PATCH_PHYS_VIRT and with > CONFIG_ZONE_DMA and see for yourself, if you don't believe me. > > Please be aware that this commit fixes nothing, its only function is > causing the regression - so we don't lose anything by reverting it. > > If the attached wasn't clear, what the defective commit presently does > is changing a perfectly valid code into a code referencing a variable > which (without CONFIG_ARM_PATCH_PHYS_VIRT set) doesn't at all exist. > > With CONFIG_ARM_PATCH_PHYS_VIRT set, this commit does precisely nothing. Right, so, with Assabet, which has CONFIG_DMA_ZONE=y and CONFIG_ARM_PATCH_PHYS_VIRT=y: $ make O=../build/assabet arch/arm/mm/init.i gives: arm_dma_limit = __pv_phys_offset + arm_dma_zone_size - 1; with or without Santosh's patch. So, with V2P patching in place, there's absolutely no functional difference. With CONFIG_ARM_PATCH_PHYS_VIRT=n, before Santosh's patch: arm_dma_limit = (0xc0000000UL) + arm_dma_zone_size - 1; After: arm_dma_limit = __pv_phys_offset + arm_dma_zone_size - 1; and this breaks the build because there is no __pv_phys_offset symbol. Now, the case which matters for Santosh is the first case - the one where CONFIG_ARM_PATCH_PHYS_VIRT=y. We can clearly see that after preprocessing, the results are 100% identical. Therefore, I find myself agreeing with Krzysztof that the commit is bad, has no functional change for the case it was proposed to solve, and needs to be reverted. -- FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up. Estimation in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad. Estimate before purchase was "up to 13.2Mbit". ^ permalink raw reply [flat|nested] 13+ messages in thread
* Regression (ARM) arch/arm/mm/init.c doesn't build without CONFIG_ARM_PATCH_PHYS_VIRT. 2014-01-06 22:39 ` Russell King - ARM Linux @ 2014-01-06 23:42 ` Santosh Shilimkar 2014-01-07 1:11 ` Russell King - ARM Linux 0 siblings, 1 reply; 13+ messages in thread From: Santosh Shilimkar @ 2014-01-06 23:42 UTC (permalink / raw) To: linux-arm-kernel On Monday 06 January 2014 05:39 PM, Russell King - ARM Linux wrote: > On Mon, Jan 06, 2014 at 08:28:20PM +0100, Krzysztof Ha?asa wrote: >> Russell, Santosh, >> >> the unneeded commit causing regression is still in place. Please try to >> compile an ARM kernel without CONFIG_ARM_PATCH_PHYS_VIRT and with >> CONFIG_ZONE_DMA and see for yourself, if you don't believe me. >> >> Please be aware that this commit fixes nothing, its only function is >> causing the regression - so we don't lose anything by reverting it. >> >> If the attached wasn't clear, what the defective commit presently does >> is changing a perfectly valid code into a code referencing a variable >> which (without CONFIG_ARM_PATCH_PHYS_VIRT set) doesn't at all exist. >> >> With CONFIG_ARM_PATCH_PHYS_VIRT set, this commit does precisely nothing. > > Right, so, with Assabet, which has CONFIG_DMA_ZONE=y and > CONFIG_ARM_PATCH_PHYS_VIRT=y: > > $ make O=../build/assabet arch/arm/mm/init.i > > gives: > arm_dma_limit = __pv_phys_offset + arm_dma_zone_size - 1; > > with or without Santosh's patch. So, with V2P patching in place, there's > absolutely no functional difference. > > With CONFIG_ARM_PATCH_PHYS_VIRT=n, before Santosh's patch: > > arm_dma_limit = (0xc0000000UL) + arm_dma_zone_size - 1; > > After: > > arm_dma_limit = __pv_phys_offset + arm_dma_zone_size - 1; > > and this breaks the build because there is no __pv_phys_offset symbol. > > Now, the case which matters for Santosh is the first case - the one > where CONFIG_ARM_PATCH_PHYS_VIRT=y. We can clearly see that after > preprocessing, the results are 100% identical. > > Therefore, I find myself agreeing with Krzysztof that the commit is > bad, has no functional change for the case it was proposed to solve, > and needs to be reverted. > May be I missed your point but I ended up creating the patch because the CMA reservation was failing on Keystone since the arm_dma_limit wasn't right since it wasn't considering the actual __pv_phys_offset. Isn't that an issue ? Regards, Santosh ^ permalink raw reply [flat|nested] 13+ messages in thread
* Regression (ARM) arch/arm/mm/init.c doesn't build without CONFIG_ARM_PATCH_PHYS_VIRT. 2014-01-06 23:42 ` Santosh Shilimkar @ 2014-01-07 1:11 ` Russell King - ARM Linux 2014-01-07 17:45 ` Santosh Shilimkar 0 siblings, 1 reply; 13+ messages in thread From: Russell King - ARM Linux @ 2014-01-07 1:11 UTC (permalink / raw) To: linux-arm-kernel On Mon, Jan 06, 2014 at 06:42:13PM -0500, Santosh Shilimkar wrote: > On Monday 06 January 2014 05:39 PM, Russell King - ARM Linux wrote: > > On Mon, Jan 06, 2014 at 08:28:20PM +0100, Krzysztof Ha?asa wrote: > >> Russell, Santosh, > >> > >> the unneeded commit causing regression is still in place. Please try to > >> compile an ARM kernel without CONFIG_ARM_PATCH_PHYS_VIRT and with > >> CONFIG_ZONE_DMA and see for yourself, if you don't believe me. > >> > >> Please be aware that this commit fixes nothing, its only function is > >> causing the regression - so we don't lose anything by reverting it. > >> > >> If the attached wasn't clear, what the defective commit presently does > >> is changing a perfectly valid code into a code referencing a variable > >> which (without CONFIG_ARM_PATCH_PHYS_VIRT set) doesn't at all exist. > >> > >> With CONFIG_ARM_PATCH_PHYS_VIRT set, this commit does precisely nothing. > > /-------------------------------------------------------------- | > > Right, so, with Assabet, which has CONFIG_DMA_ZONE=y and | > > CONFIG_ARM_PATCH_PHYS_VIRT=y: | > > | > > $ make O=../build/assabet arch/arm/mm/init.i | > > | > > gives: | > > arm_dma_limit = __pv_phys_offset + arm_dma_zone_size - 1; | > > | > > with or without Santosh's patch. \-------------------------------------------------------------- > May be I missed your point but I ended up creating the patch because > the CMA reservation was failing on Keystone since the arm_dma_limit > wasn't right since it wasn't considering the actual __pv_phys_offset. > > Isn't that an issue ? See the above. Your patch has _no_ effect what so ever when CONFIG_ARM_PATCH_PHYS_VIRT=y - which you have on the Keystone, right? If you don't believe me, ask make to produce arch/arm/mm/init.i, which is the output from the preprocessor, comparing the resulting generated file both with and without your patch applied. -- FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up. Estimation in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad. Estimate before purchase was "up to 13.2Mbit". ^ permalink raw reply [flat|nested] 13+ messages in thread
* Regression (ARM) arch/arm/mm/init.c doesn't build without CONFIG_ARM_PATCH_PHYS_VIRT. 2014-01-07 1:11 ` Russell King - ARM Linux @ 2014-01-07 17:45 ` Santosh Shilimkar 2014-01-07 17:55 ` Russell King - ARM Linux 0 siblings, 1 reply; 13+ messages in thread From: Santosh Shilimkar @ 2014-01-07 17:45 UTC (permalink / raw) To: linux-arm-kernel On Monday 06 January 2014 08:11 PM, Russell King - ARM Linux wrote: > On Mon, Jan 06, 2014 at 06:42:13PM -0500, Santosh Shilimkar wrote: >> On Monday 06 January 2014 05:39 PM, Russell King - ARM Linux wrote: >>> On Mon, Jan 06, 2014 at 08:28:20PM +0100, Krzysztof Ha?asa wrote: >>>> Russell, Santosh, >>>> >>>> the unneeded commit causing regression is still in place. Please try to >>>> compile an ARM kernel without CONFIG_ARM_PATCH_PHYS_VIRT and with >>>> CONFIG_ZONE_DMA and see for yourself, if you don't believe me. >>>> >>>> Please be aware that this commit fixes nothing, its only function is >>>> causing the regression - so we don't lose anything by reverting it. >>>> >>>> If the attached wasn't clear, what the defective commit presently does >>>> is changing a perfectly valid code into a code referencing a variable >>>> which (without CONFIG_ARM_PATCH_PHYS_VIRT set) doesn't at all exist. >>>> >>>> With CONFIG_ARM_PATCH_PHYS_VIRT set, this commit does precisely nothing. >>> > > /-------------------------------------------------------------- > | > > Right, so, with Assabet, which has CONFIG_DMA_ZONE=y and > | > > CONFIG_ARM_PATCH_PHYS_VIRT=y: > | > > > | > > $ make O=../build/assabet arch/arm/mm/init.i > | > > > | > > gives: > | > > arm_dma_limit = __pv_phys_offset + arm_dma_zone_size - 1; > | > > > | > > with or without Santosh's patch. > \-------------------------------------------------------------- > >> May be I missed your point but I ended up creating the patch because >> the CMA reservation was failing on Keystone since the arm_dma_limit >> wasn't right since it wasn't considering the actual __pv_phys_offset. >> >> Isn't that an issue ? > > See the above. Your patch has _no_ effect what so ever when > CONFIG_ARM_PATCH_PHYS_VIRT=y - which you have on the Keystone, right? > > If you don't believe me, ask make to produce arch/arm/mm/init.i, which > is the output from the preprocessor, comparing the resulting generated > file both with and without your patch applied. > Looks like you are right. I did two fixes to have right arm_dma_limit in below order. 1. 787b0d5 {ARM: 7908/1: mm: Fix the arm_dma_limit calculation} 2. 7c92732 {ARM: 7909/1: mm: Call setup_dma_zone() post early_paging_init()} But with 2 alone the issue gets fixed since with ARM_PATCH_PHYS_VIRT and below pre-processor, the PHYS_OFFSET also gets an updated value. I never realised that 1 becomes redundant after second patch while creating them. #define PHYS_OFFSET __pv_phys_offset So indeed, 787b0d5{ARM: 7908/1: mm: Fix the arm_dma_limit calculation} won't be needed anymore and can be reverted. Sorry it took some time for me to reach to your conclusion. Regards, Santosh ^ permalink raw reply [flat|nested] 13+ messages in thread
* Regression (ARM) arch/arm/mm/init.c doesn't build without CONFIG_ARM_PATCH_PHYS_VIRT. 2014-01-07 17:45 ` Santosh Shilimkar @ 2014-01-07 17:55 ` Russell King - ARM Linux 2014-01-08 6:40 ` Krzysztof Hałasa 0 siblings, 1 reply; 13+ messages in thread From: Russell King - ARM Linux @ 2014-01-07 17:55 UTC (permalink / raw) To: linux-arm-kernel On Tue, Jan 07, 2014 at 12:45:42PM -0500, Santosh Shilimkar wrote: > On Monday 06 January 2014 08:11 PM, Russell King - ARM Linux wrote: > > /-------------------------------------------------------------- > > | > > Right, so, with Assabet, which has CONFIG_DMA_ZONE=y and > > | > > CONFIG_ARM_PATCH_PHYS_VIRT=y: > > | > > > > | > > $ make O=../build/assabet arch/arm/mm/init.i > > | > > > > | > > gives: > > | > > arm_dma_limit = __pv_phys_offset + arm_dma_zone_size - 1; > > | > > > > | > > with or without Santosh's patch. > > \-------------------------------------------------------------- > > > >> May be I missed your point but I ended up creating the patch because > >> the CMA reservation was failing on Keystone since the arm_dma_limit > >> wasn't right since it wasn't considering the actual __pv_phys_offset. > >> > >> Isn't that an issue ? > > > > See the above. Your patch has _no_ effect what so ever when > > CONFIG_ARM_PATCH_PHYS_VIRT=y - which you have on the Keystone, right? > > > > If you don't believe me, ask make to produce arch/arm/mm/init.i, which > > is the output from the preprocessor, comparing the resulting generated > > file both with and without your patch applied. > > > Looks like you are right. I did two fixes to have right arm_dma_limit in > below order. > > 1. 787b0d5 {ARM: 7908/1: mm: Fix the arm_dma_limit calculation} > 2. 7c92732 {ARM: 7909/1: mm: Call setup_dma_zone() post early_paging_init()} > > But with 2 alone the issue gets fixed since with ARM_PATCH_PHYS_VIRT and > below pre-processor, the PHYS_OFFSET also gets an updated value. I never > realised that 1 becomes redundant after second patch while creating them. > > #define PHYS_OFFSET __pv_phys_offset > > So indeed, 787b0d5{ARM: 7908/1: mm: Fix the arm_dma_limit calculation} > won't be needed anymore and can be reverted. Sorry it took some time > for me to reach to your conclusion. Okay, reverted. -- FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up. Estimation in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad. Estimate before purchase was "up to 13.2Mbit". ^ permalink raw reply [flat|nested] 13+ messages in thread
* Regression (ARM) arch/arm/mm/init.c doesn't build without CONFIG_ARM_PATCH_PHYS_VIRT. 2014-01-07 17:55 ` Russell King - ARM Linux @ 2014-01-08 6:40 ` Krzysztof Hałasa 0 siblings, 0 replies; 13+ messages in thread From: Krzysztof Hałasa @ 2014-01-08 6:40 UTC (permalink / raw) To: linux-arm-kernel Russell King - ARM Linux <linux@arm.linux.org.uk> writes: > Okay, reverted. Thanks. -- Krzysztof Halasa Research Institute for Automation and Measurements PIAP Al. Jerozolimskie 202, 02-486 Warsaw, Poland ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2014-01-08 6:40 UTC | newest] Thread overview: 13+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2013-12-31 11:20 Regression (ARM) arch/arm/mm/init.c doesn't build without CONFIG_ARM_PATCH_PHYS_VIRT Krzysztof Hałasa 2013-12-31 11:23 ` Russell King - ARM Linux 2014-01-01 14:40 ` Krzysztof Hałasa 2014-01-06 19:28 ` Krzysztof Hałasa 2014-01-06 19:33 ` Santosh Shilimkar 2014-01-06 22:08 ` Krzysztof Hałasa 2014-01-06 22:27 ` Santosh Shilimkar 2014-01-06 22:39 ` Russell King - ARM Linux 2014-01-06 23:42 ` Santosh Shilimkar 2014-01-07 1:11 ` Russell King - ARM Linux 2014-01-07 17:45 ` Santosh Shilimkar 2014-01-07 17:55 ` Russell King - ARM Linux 2014-01-08 6:40 ` Krzysztof Hałasa
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).