All of lore.kernel.org
 help / color / mirror / Atom feed
From: Alexey Brodkin <Alexey.Brodkin@synopsys.com>
To: Vineet Gupta <Vineet.Gupta1@synopsys.com>
Cc: "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-arch@vger.kernel.org" <linux-arch@vger.kernel.org>,
	"noamc@ezchip.com" <noamc@ezchip.com>,
	Lada Trimasova <Lada.Trimasova@synopsys.com>,
	"linux-snps-arc@lists.infradead.org"
	<linux-snps-arc@lists.infradead.org>
Subject: Re: [PATCH] arc: use little endian accesses
Date: Thu, 10 Mar 2016 07:44:06 +0000	[thread overview]
Message-ID: <1457595846.2868.10.camel@synopsys.com> (raw)
In-Reply-To: <C2D7FE5348E1B147BCA15975FBA23075F4E8EDF9@us01wembx1.internal.synopsys.com>

Hi Vineet,

On Thu, 2016-03-10 at 05:05 +0000, Vineet Gupta wrote:
> +CC Noam
> 
> On Wednesday 09 March 2016 10:51 PM, Lada Trimasova wrote:
> > 
> > Memory access primitives should use cpu_to_le16, cpu_to_le32, le16_to_cpu
> > and le32_to_cpu because it is not really guaranteed that drivers handles
> > any ordering themselves.
> That is the driver issue. readxx as API simply returns data in native endianness.
> We've had EZChip running big endian and so far and they didn't need this change.

Let me disagree with you here.
See what is said in "include/asm-generic/io.h":
---------------------->8---------------------
/*
 * __raw_{read,write}{b,w,l,q}() access memory in native endianness.
 *
 * On some architectures memory mapped IO needs to be accessed differently.
 * On the simple architectures, we just read/write the memory location
 * directly.
 */

...

/*
 * {read,write}{b,w,l,q}() access little endian memory and return result in
 * native endianness.
 */
---------------------->8---------------------

And that's an implementation we have for ARC:
---------------------->8---------------------
#define readb(c)		({ u8  __v = readb_relaxed(c); __iormb(); __v; })
#define readw(c)		({ u16 __v = readw_relaxed(c); __iormb(); __v; })
#define readl(c)		({ u32 __v = readl_relaxed(c); __iormb(); __v; })

#define writeb(v,c)		({ __iowmb(); writeb_relaxed(v,c); })
#define writew(v,c)		({ __iowmb(); writew_relaxed(v,c); })
#define writel(v,c)		({ __iowmb(); writel_relaxed(v,c); })

/*
 * Relaxed API for drivers which can handle any ordering themselves
 */
#define readb_relaxed(c)	__raw_readb(c)
#define readw_relaxed(c)	__raw_readw(c)
#define readl_relaxed(c)	__raw_readl(c)

#define writeb_relaxed(v,c)	__raw_writeb(v,c)
#define writew_relaxed(v,c)	__raw_writew(v,c)
#define writel_relaxed(v,c)	__raw_writel(v,c)
---------------------->8---------------------

Which is effectively (related to endianess discussion):
---------------------->8---------------------
#define readX(c)	__raw_readX(c)
#define writeX(v,c)	__raw_writeX(v,c)
---------------------->8---------------------

That looks IMHO incorrect if we read API description in "include/asm-generic/io.h".
BTW description of {read,write}{b,w,l,q}() is a bit misleading in part saying
"... and return result in __native_endianness__".

But real implementation of {read,write}{b,w,l,q}() in "include/asm-generic/io.h"
really shows what was meant - note __leXX_to_cpu() and cpu_to_leXX are used.

> > 
> > For example, serial port driver doesn't work when kernel is build for
> > arc big endian architecture.
> Last I tested Big Endian on SDP with 8250 part + 8250 driver it was working fine.
> I presume this is the systemC model for device and standard 8250 driver and very
> likely the model is not fixed endian or something.

Model is indeed little-endian. We build it only once and than changing
only "nsim_isa_big_endian" property (which changes only CPU core endianess) may use
it equally well with little- and big-endian builds of Linux kernel.

-Alexey

WARNING: multiple messages have this Message-ID (diff)
From: Alexey.Brodkin@synopsys.com (Alexey Brodkin)
To: linux-snps-arc@lists.infradead.org
Subject: [PATCH] arc: use little endian accesses
Date: Thu, 10 Mar 2016 07:44:06 +0000	[thread overview]
Message-ID: <1457595846.2868.10.camel@synopsys.com> (raw)
In-Reply-To: <C2D7FE5348E1B147BCA15975FBA23075F4E8EDF9@us01wembx1.internal.synopsys.com>

Hi Vineet,

On Thu, 2016-03-10@05:05 +0000, Vineet Gupta wrote:
> +CC Noam
> 
> On Wednesday 09 March 2016 10:51 PM, Lada Trimasova wrote:
> > 
> > Memory access primitives should use cpu_to_le16, cpu_to_le32, le16_to_cpu
> > and le32_to_cpu because it is not really guaranteed that drivers handles
> > any ordering themselves.
> That is the driver issue. readxx as API simply returns data in native endianness.
> We've had EZChip running big endian and so far and they didn't need this change.

Let me disagree with you here.
See what is said in "include/asm-generic/io.h":
---------------------->8---------------------
/*
?* __raw_{read,write}{b,w,l,q}() access memory in native endianness.
?*
?* On some architectures memory mapped IO needs to be accessed differently.
?* On the simple architectures, we just read/write the memory location
?* directly.
?*/

...

/*
?* {read,write}{b,w,l,q}() access little endian memory and return result in
?* native endianness.
?*/
---------------------->8---------------------

And that's an implementation we have for ARC:
---------------------->8---------------------
#define readb(c)		({ u8??__v = readb_relaxed(c); __iormb(); __v; })
#define readw(c)		({ u16 __v = readw_relaxed(c); __iormb(); __v; })
#define readl(c)		({ u32 __v = readl_relaxed(c); __iormb(); __v; })

#define writeb(v,c)		({ __iowmb(); writeb_relaxed(v,c); })
#define writew(v,c)		({ __iowmb(); writew_relaxed(v,c); })
#define writel(v,c)		({ __iowmb(); writel_relaxed(v,c); })

/*
?* Relaxed API for drivers which can handle any ordering themselves
?*/
#define readb_relaxed(c)	__raw_readb(c)
#define readw_relaxed(c)	__raw_readw(c)
#define readl_relaxed(c)	__raw_readl(c)

#define writeb_relaxed(v,c)	__raw_writeb(v,c)
#define writew_relaxed(v,c)	__raw_writew(v,c)
#define writel_relaxed(v,c)	__raw_writel(v,c)
---------------------->8---------------------

Which is effectively (related to endianess discussion):
---------------------->8---------------------
#define readX(c)	__raw_readX(c)
#define writeX(v,c)	__raw_writeX(v,c)
---------------------->8---------------------

That looks IMHO incorrect if we read API description in "include/asm-generic/io.h".
BTW description of {read,write}{b,w,l,q}() is a bit misleading in part saying
"... and return result in __native_endianness__".

But real implementation of {read,write}{b,w,l,q}() in "include/asm-generic/io.h"
really shows what was meant - note?__leXX_to_cpu() and?cpu_to_leXX are used.

> > 
> > For example, serial port driver doesn't work when kernel is build for
> > arc big endian architecture.
> Last I tested Big Endian on SDP with 8250 part + 8250 driver it was working fine.
> I presume this is the systemC model for device and standard 8250 driver and very
> likely the model is not fixed endian or something.

Model is indeed little-endian. We build it only once and than changing
only "nsim_isa_big_endian" property (which changes only CPU core endianess) may use
it equally well with little- and big-endian builds of Linux kernel.

-Alexey

  parent reply	other threads:[~2016-03-10  7:44 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-03-09 17:21 [PATCH] arc: use little endian accesses Lada Trimasova
2016-03-09 17:21 ` Lada Trimasova
2016-03-10  5:05 ` Vineet Gupta
2016-03-10  5:05   ` Vineet Gupta
2016-03-10  5:19   ` Vineet Gupta
2016-03-10  5:19     ` Vineet Gupta
2016-03-10  7:44   ` Alexey Brodkin [this message]
2016-03-10  7:44     ` Alexey Brodkin
2016-03-10  9:55     ` Vineet Gupta
2016-03-10  9:55       ` Vineet Gupta
2016-03-10 18:57       ` Lada Trimasova
2016-03-10 18:57         ` Lada Trimasova
2016-03-10 19:23         ` Arnd Bergmann
2016-03-10 19:23           ` Arnd Bergmann
2016-03-11 12:44           ` Vineet Gupta
2016-03-11 12:44             ` Vineet Gupta
2016-03-12  4:20             ` Vineet Gupta
2016-03-12  4:20               ` Vineet Gupta
2016-03-12  4:20               ` Vineet Gupta
2016-03-12  4:20               ` Vineet Gupta
2016-03-11  5:07         ` Noam Camus
2016-03-11  5:07           ` Noam Camus
2016-03-11  5:07           ` Noam Camus
2016-03-10  7:45   ` Arnd Bergmann
2016-03-10  7:45     ` Arnd Bergmann
2016-03-10  7:45     ` Arnd Bergmann
2016-03-11  5:18     ` Vineet Gupta
2016-03-11  5:18       ` Vineet Gupta

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=1457595846.2868.10.camel@synopsys.com \
    --to=alexey.brodkin@synopsys.com \
    --cc=Lada.Trimasova@synopsys.com \
    --cc=Vineet.Gupta1@synopsys.com \
    --cc=linux-arch@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-snps-arc@lists.infradead.org \
    --cc=noamc@ezchip.com \
    /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.