All of lore.kernel.org
 help / color / mirror / Atom feed
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: Tue, 7 Jan 2014 12:45:42 -0500	[thread overview]
Message-ID: <52CC3D46.3090102@ti.com> (raw)
In-Reply-To: <20140107011121.GI27432@n2100.arm.linux.org.uk>

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

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"
	<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: Tue, 7 Jan 2014 12:45:42 -0500	[thread overview]
Message-ID: <52CC3D46.3090102@ti.com> (raw)
In-Reply-To: <20140107011121.GI27432@n2100.arm.linux.org.uk>

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




  reply	other threads:[~2014-01-07 17:45 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
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 [this message]
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=52CC3D46.3090102@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.