[PATCH] powerpc/4xx/ocm: fix compiler error

[christophe leroy]

Le 22/12/2018 à 12:27, Christian Lamparter a écrit :
> On Saturday, December 22, 2018 11:59:04 AM CET christophe leroy wrote:
>>
>> Le 22/12/2018 à 11:09, Christian Lamparter a écrit :
>>> This patch fixes a recent regression in ocm:
>>>
>>> ocm.c: In function ‘ocm_init_node’:
>>> ocm.c:182:18: error: invalid operands to binary |
>>> ocm.c:197:17: error: invalid operands to binary |
>>>
>>> Fixes: 56f3c1413f5c ("powerpc/mm: properly set PAGE_KERNEL flags in ioremap()")
>>> Signed-off-by: Christian Lamparter <chunkeey at gmail.com>
>>
>> What's the problem here ? Is PAGE_KERNEL_NCG undefined ? If that's the
>> case, wouldn't it be better for fix that by including asm/pgtable.h ?
> 
> PAGE_KERNEL[_NCG] type is a struct of pgprot_t, whereas _PAGE_EXEC is
> considered an int.

Oops, I missed that.

Could you put the entire error message in the commit log ?

> 
> arch/powerpc/platforms/4xx/ocm.c: In function ‘ocm_init_node’:
> arch/powerpc/platforms/4xx/ocm.c:182:18: error: invalid operands to binary | (have ‘int’ and ‘pgprot_t’ {aka ‘struct <anonymous>’})
>         _PAGE_EXEC | PAGE_KERNEL_NCG);

Usually, Guarded implies no exec (at least on 6xx and 8xx). Does the 4xx 
accept guarded exec ?

>                    ^
> arch/powerpc/platforms/4xx/ocm.c:197:17: error: invalid operands to binary | (have ‘int’ and ‘pgprot_t’ {aka ‘struct <anonymous>’})
>         _PAGE_EXEC | PAGE_KERNEL);

That's PAGE_KERNEL_X

>                    ^
> 
> I think pgprot_val(PAGE_KERNEL[_NCG]) could be an option too.

Yes I may have a preference for that.

> 
> Christian
>>> ---
>>>    arch/powerpc/platforms/4xx/ocm.c | 6 ++++--
>>>    1 file changed, 4 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/arch/powerpc/platforms/4xx/ocm.c b/arch/powerpc/platforms/4xx/ocm.c
>>> index 561b09de69bf..04ff69ddac09 100644
>>> --- a/arch/powerpc/platforms/4xx/ocm.c
>>> +++ b/arch/powerpc/platforms/4xx/ocm.c
>>> @@ -179,7 +179,8 @@ static void __init ocm_init_node(int count, struct device_node *node)
>>>    	/* ioremap the non-cached region */
>>>    	if (ocm->nc.memtotal) {
>>>    		ocm->nc.virt = __ioremap(ocm->nc.phys, ocm->nc.memtotal,
>>> -					 _PAGE_EXEC | PAGE_KERNEL_NCG);
>>> +			_PAGE_EXEC | _PAGE_BASE_NC |
>>> +			_PAGE_KERNEL_RW | _PAGE_NO_CACHE | _PAGE_GUARDED);

Could be _PAGE_BASE_NC | _PAGE_KERNEL_RWX | _PAGE_NO_CACHE | _PAGE_GUARDED

Or pgprot_val(PAGE_KERNEL_NCG) | _PAGE_EXEC

Or pgprot_val(pgprot_noncached(PAGE_KERNEL_RWX)) (that's my preference)

>>>    
>>>    		if (!ocm->nc.virt) {
>>>    			printk(KERN_ERR
>>> @@ -194,7 +195,8 @@ static void __init ocm_init_node(int count, struct device_node *node)
>>>    
>>>    	if (ocm->c.memtotal) {
>>>    		ocm->c.virt = __ioremap(ocm->c.phys, ocm->c.memtotal,
>>> -					_PAGE_EXEC | PAGE_KERNEL);
>>> +					_PAGE_EXEC | _PAGE_BASE |
>>> +					_PAGE_KERNEL_RW);

What about _PAGE_BASE | _PAGE_KERNEL_RWX instead ?

Or pgprot_val(PAGE_KERNEL_RWX) (that's my preference)

Christophe

>>>    
>>>    		if (!ocm->c.virt) {
>>>    			printk(KERN_ERR
>>>
>>

---
L'absence de virus dans ce courrier électronique a été vérifiée par le logiciel antivirus Avast.
https://www.avast.com/antivirus