From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
	id S1751576AbcGTGCW (ORCPT <rfc822;w@1wt.eu>);
	Wed, 20 Jul 2016 02:02:22 -0400
Received: from mail-out.m-online.net ([212.18.0.9]:50475 "EHLO
	mail-out.m-online.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
	with ESMTP id S1751029AbcGTGCS (ORCPT
	<rfc822;linux-kernel@vger.kernel.org>);
	Wed, 20 Jul 2016 02:02:18 -0400
X-Auth-Info: MhrcVDUX/0GYWsKuBIHLzON5dK9Qd13dUcoa98ULFaw=
Subject: Re: [PATCH] mtd: spi-nor: don't build Cadence QuadSPI on non-ARM
To: Brian Norris <computersforpeace@gmail.com>
References: <201607190315.0nkQuD2W%fengguang.wu@intel.com>
 <20160718202026.GB137880@google.com> <578DC294.5090605@denx.de>
 <20160719200541.GA854@google.com>
 <3a769a42-f856-5b48-dd18-ef76a7bf4c91@denx.de>
 <20160720025027.GA131957@google.com>
 <eaf864ab-d154-994f-dc3d-9505b48705f3@denx.de>
 <20160720032501.GA35298@google.com>
Cc: Stefan Roese <sr@denx.de>, kbuild test robot <fengguang.wu@intel.com>,
        x86@kernel.org, linux-kernel@vger.kernel.org,
        linux-mtd@lists.infradead.org, kbuild-all@01.org,
        Graham Moore <grmoore@opensource.altera.com>
From: Marek Vasut <marex@denx.de>
Message-ID: <d72cbf7d-918f-944f-b441-7a67a1870ce6@denx.de>
Date: Wed, 20 Jul 2016 07:59:25 +0200
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101
 Icedove/45.1.0
MIME-Version: 1.0
In-Reply-To: <20160720032501.GA35298@google.com>
Content-Type: text/plain; charset=windows-1252
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 07/20/2016 05:25 AM, Brian Norris wrote:
> On Wed, Jul 20, 2016 at 04:58:08AM +0200, Marek Vasut wrote:
>> On 07/20/2016 04:50 AM, Brian Norris wrote:
>>> On Wed, Jul 20, 2016 at 03:50:27AM +0200, Marek Vasut wrote:
>>>> On 07/19/2016 10:05 PM, Brian Norris wrote:
>>>>> On Tue, Jul 19, 2016 at 08:03:00AM +0200, Stefan Roese wrote:
>>>>>> On 18.07.2016 22:20, Brian Norris wrote:
>>>>>>> Hmm, does x86 not define readsl()/writesl()? I can never tell what
>>>>>>> accessors are supposed to be "standard" across architectures.
>>>>>>>
>>>>>>> Either we need to drop the COMPILE_TEST or maybe make it (!X86 &&
>>>>>>> COMPILE_TEST).
>>>>>>
>>>>>> iowrite32_rep() etc should work for x86 as well.
>>>>>
>>>>> Looks like it might. I'm not sure the original submitter can retest
>>>>> right now (travel), so I'd probably rather just take the easy fix for
>>>>> now, and we can widen to COMPILE_TEST later if desired.
>>>>
>>>> Isn't there a generic readsl() and writesl() implementation in
>>>> include/asm-generic/io.h ?
>>>
>>> Yes, but somehow x86 has managed to avoid that. I guess it's optional
>>> for arch/<foo>/include/asm/io.h to include <asm-generic/io.h>? At any
>>> rate, I double-checked myself by adding '#error "blah"' to
>>> include/asm-generic/io.h, and x86 still seemed to build fine (at least
>>> for the modules I was checking, like cadence-quadspi.o).
>>
>> Yep, I just checked the same and it's not included from
>> arch/x86/include/asm/io.h for whatever reason. Maybe this needs to be
>> fixed on x86 level?
> 
> Maybe. That's why I added the x86 maintainers. Maybe they'd respond
> better^Wmore loudly if I just sent a patch to do that :)
> 
> But seriously, doing the above really breaks things, even if I stick the
> include at the end of asm/io.h. There's plenty of stuff that the
> asm-generic version includes based on #ifndef some_accessor, except x86
> uses a static inline for their definition. So it seems it's not trivial
> to get an architecture to fall back gracefully to asm-generic; you have
> to put in some work. It also may not be all that desirable to have some
> allegedly generic version generate something that may not be safe on a
> given architecture (and I don't purport to understand the x86 memory
> model).
> 
> Additionally, it looks like asm-generic/io.h is actually only included
> in 14 of 33 arch'es, so it seems like that's really not a designated
> goal. It does make it awfully difficult to figure out what I/O accessors
> are *actually* portable though...

Ouch :-( Maybe this is an opportunity for cleanup then ?

I think disabling the compile test for now is good, but we should
revisit this once I'm back and capable of digging in properly. Thus
far, I am mostly handling my mails on the bullet trains (which are
awesome here in Japan, I tell you that ;-) ), so I'm really not able
to give this as much attention as it requires.

> Brian
> 


-- 
Best regards,
Marek Vasut