From: santosh.shilimkar@ti.com (Santosh Shilimkar)
To: linux-arm-kernel@lists.infradead.org
Subject: Regression (ARM) arch/arm/mm/init.c doesn't build without CONFIG_ARM_PATCH_PHYS_VIRT.
Date: Mon, 6 Jan 2014 18:42:13 -0500 [thread overview]
Message-ID: <52CB3F55.1050404@ti.com> (raw)
In-Reply-To: <20140106223906.GH27432@n2100.arm.linux.org.uk>
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
WARNING: multiple messages have this Message-ID (diff)
From: Santosh Shilimkar <santosh.shilimkar@ti.com>
To: Russell King - ARM Linux <linux@arm.linux.org.uk>
Cc: "Krzysztof Hałasa" <khalasa@piap.pl>,
"Linus Torvalds" <torvalds@linux-foundation.org>,
lkml <linux-kernel@vger.kernel.org>,
linux-arm-kernel@lists.infradead.org
Subject: Re: Regression (ARM) arch/arm/mm/init.c doesn't build without CONFIG_ARM_PATCH_PHYS_VIRT.
Date: Mon, 6 Jan 2014 18:42:13 -0500 [thread overview]
Message-ID: <52CB3F55.1050404@ti.com> (raw)
In-Reply-To: <20140106223906.GH27432@n2100.arm.linux.org.uk>
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
next prev parent reply other threads:[~2014-01-06 23:42 UTC|newest]
Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top
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:20 ` Krzysztof Hałasa
2013-12-31 11:23 ` Russell King - ARM Linux
2013-12-31 11:23 ` Russell King - ARM Linux
2014-01-01 14:40 ` Krzysztof Hałasa
2014-01-01 14:40 ` Krzysztof Hałasa
2014-01-06 19:28 ` Krzysztof Hałasa
2014-01-06 19:28 ` Krzysztof Hałasa
2014-01-06 19:33 ` Santosh Shilimkar
2014-01-06 19:33 ` Santosh Shilimkar
2014-01-06 22:08 ` Krzysztof Hałasa
2014-01-06 22:08 ` Krzysztof Hałasa
2014-01-06 22:27 ` Santosh Shilimkar
2014-01-06 22:27 ` Santosh Shilimkar
2014-01-06 22:39 ` Russell King - ARM Linux
2014-01-06 22:39 ` Russell King - ARM Linux
2014-01-06 23:42 ` Santosh Shilimkar [this message]
2014-01-06 23:42 ` Santosh Shilimkar
2014-01-07 1:11 ` Russell King - ARM Linux
2014-01-07 1:11 ` Russell King - ARM Linux
2014-01-07 17:45 ` Santosh Shilimkar
2014-01-07 17:45 ` Santosh Shilimkar
2014-01-07 17:55 ` Russell King - ARM Linux
2014-01-07 17:55 ` Russell King - ARM Linux
2014-01-08 6:40 ` Krzysztof Hałasa
2014-01-08 6:40 ` Krzysztof Hałasa
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=52CB3F55.1050404@ti.com \
--to=santosh.shilimkar@ti.com \
--cc=linux-arm-kernel@lists.infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.