Re: [PATCH v4 6/7] ARM: NOMMU: Set ARM_DMA_MEM_BUFFERABLE for M-class cpus

From: Arnd Bergmann <arnd@arndb.de>
To: Vladimir Murzin <vladimir.murzin@arm.com>
Cc: Russell King - ARM Linux <linux@armlinux.org.uk>,
	Linux ARM <linux-arm-kernel@lists.infradead.org>,
	sza@esh.hu, Robin Murphy <robin.murphy@arm.com>,
	Alexandre Torgue <alexandre.torgue@st.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	kbuild-all@01.org,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	gregkh <gregkh@linuxfoundation.org>
Subject: Re: [PATCH v4 6/7] ARM: NOMMU: Set ARM_DMA_MEM_BUFFERABLE for M-class cpus
Date: Wed, 24 May 2017 10:36:37 +0200	[thread overview]
Message-ID: <CAK8P3a0s-SLGwhdxX+FDsmRKnYh2nWPKPvwsn_qMOk_1Wb5YMQ@mail.gmail.com> (raw)
In-Reply-To: <741c708e-9036-a0e1-46e1-90d82a195dff@arm.com>

On Wed, May 24, 2017 at 10:31 AM, Vladimir Murzin
<vladimir.murzin@arm.com> wrote:
> On 23/05/17 21:33, Arnd Bergmann wrote:
>> On Tue, May 23, 2017 at 10:01 PM, Russell King - ARM Linux
>> <linux@armlinux.org.uk> wrote:
>>> On Mon, Apr 24, 2017 at 11:16:56AM +0100, Vladimir Murzin wrote:
>>>> Now, we have dedicated non-cacheable region for consistent DMA
>>>> operations. However, that region can still be marked as bufferable by
>>>> MPU, so it'd be safer to have barriers by default.
>>>
>>> What do you actually want here?  Your patch doesn't quite make sense,
>>> the commit description seems to indicate that you require this option
>>> to be set for V7M, but the patch says otherwise.
>>>
>>>>  config ARM_DMA_MEM_BUFFERABLE
>>>> -     bool "Use non-cacheable memory for DMA" if (CPU_V6 || CPU_V6K) && !CPU_V7
>>>> -     default y if CPU_V6 || CPU_V6K || CPU_V7
>>>> +     bool "Use non-cacheable memory for DMA" if (CPU_V6 || CPU_V6K || CPU_V7M) && !CPU_V7
>>>
>>> This "if" conditional conditionalises the visibility of the option,
>>> it doesn't conditionalise the value.
>>>
>>>> +     default y if CPU_V6 || CPU_V6K || CPU_V7 || CPU_V7M
>>>
>>> Taking both of these changes together what you end up with is an option
>>> presented to the user for "Use non-cacheable memory for DMA" which
>>> they can choose to disable.
>>>
>>> If you require this option to be set, that's incorrect - your modification
>>> to the default line is correct, but the first line is not.  To achieve
>>> that, you want the if condition to evaluate false for V7M, thereby hiding
>>> the option from the user.  In that case, the default value will always be
>>> assigned to the option.
>>
>> I had the opposite comment in the previous version ;-)
>> https://lkml.org/lkml/2017/4/19/185
>>
>> I think the current patch is correct, but the description could still be
>> clarified: On some of the beefier ARMv7-M machines (with DMA
>> and write buffers) we want this enabled, while those that didn't
>> need it until now also won't need it in the future.
>
> Ok. Do you want it go into commit message or option description or maybe both?

I'd say both. It would also be helpful to identify specifically which platforms
require this, and then add a 'select ARM_DMA_MEM_BUFFERABLE' from
the platform, as we do from Moxart.

      Arnd