linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: santosh.shilimkar@oracle.com (santosh shilimkar)
To: linux-arm-kernel@lists.infradead.org
Subject: Moan: usage of __iormb() and __iowmb() outside of asm/io.h
Date: Wed, 10 Jun 2015 10:25:54 -0700	[thread overview]
Message-ID: <55787322.8020501@oracle.com> (raw)
In-Reply-To: <55781880.7040700@ti.com>

On 6/10/2015 3:59 AM, Murali Karicheri wrote:
> On 06/08/2015 02:47 PM, Russell King - ARM Linux wrote:
>> Guys,
>>
>> I notice that we have users of __iormb() and __iowmb() outside of
>> asm/io.h:
>>
>> drivers/clocksource/timer-keystone.c:   __iowmb();
>> drivers/dma/cppi41.c:                   __iormb();
>> drivers/dma/cppi41.c:   __iowmb();
>> drivers/dma/cppi41.c:           __iowmb();
>> drivers/dma/cppi41.c:                   __iormb();
>> drivers/soc/ti/knav_qmss_queue.c:       __iowmb();
>>
Looks like CPPI abuse got carry forwarded. I also missed these because
of oversight. Should have insisted to use the standard accessors.

>> These are not official kernel barriers - the only reason they exist in
>> asm/io.h is purely to provide a barrier implementation _for_ creating
>> the accessors _in_ asm/io.h, which are macros, and therefore these
>> macros need to stay around for the same scope as those accessors.
>>
>> As with all details which are an architecture matter, they are subject
>> to the whims of the architecture maintainer to provide whatever semantics
>> for them that the architecture maintainer deems fit: there is no official
>> requirement for anything of that nature to do anything, and no guarantee
>> that anything such a detail does today it will do so tomorrow.
>>
>> This is why only official interfaces should be used, and if they do not
>> satisfy the requirements, then new official interfaces need to be
>> proposed.  Don't ever poke about with stuff that's an architecture
>> implementation detail.
>>
>> We've been here before with some of the cache flushing code - and people
>> have been burnt by it.
>>
>> I do wish that people would see the difference between stuff which is
>> implemented to facilitate the implementation of an architecture detail
>> vs something which is provided for everyone's use.
>>
>> I'm working on a patch which will completely remove these from view.
>> I would strongly suggest that these uses are removed from the above
>> code as a matter of urgency.
>>
Sorry for that Russell and thanks for addressing it advance.

Regards,
Santosh

  reply	other threads:[~2015-06-10 17:25 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-06-08 18:47 Moan: usage of __iormb() and __iowmb() outside of asm/io.h Russell King - ARM Linux
2015-06-09  9:54 ` Sebastian Andrzej Siewior
2015-06-10 10:59 ` Murali Karicheri
2015-06-10 17:25   ` santosh shilimkar [this message]
2015-06-10 11:18 ` Will Deacon
2015-06-10 11:24   ` Russell King - ARM Linux
2015-06-10 12:30     ` Russell King - ARM Linux
2015-06-10 16:53       ` Catalin Marinas
2015-06-10 16:59         ` Will Deacon
2015-06-11 14:16           ` Russell King - ARM Linux

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=55787322.8020501@oracle.com \
    --to=santosh.shilimkar@oracle.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).