All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jisheng Zhang <jszhang-eYqpPyKDWXRBDgjK7y7TUQ@public.gmane.org>
To: Wolfram Sang <wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org>
Cc: "linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org"
	<linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	"linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org"
	<linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	"linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org"
	<linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org>
Subject: Re: [PATCH] i2c: designware: use {readl|writel}_relaxed instead of readl/writel
Date: Wed, 14 Jan 2015 12:05:09 +0800	[thread overview]
Message-ID: <20150114120509.0ba8267e@xhacker> (raw)
In-Reply-To: <20150113143654.GG7660@katana>

Dear Wolfram,

On Tue, 13 Jan 2015 06:36:54 -0800
Wolfram Sang <wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org> wrote:

> 
> On Thu, Dec 11, 2014 at 02:26:41PM +0800, Jisheng Zhang wrote:
> > readl/writel is too expensive especially on Cortex A9 w/ outer L2 cache.
> > This introduces i2c read/write errors on Marvell BG2/BG2Q SoCs when there
> > are heavy L2 cache maintenance operations at the same time.
> 
> Reading this again, I got a question:
> 
> Really read/write errors? I would think that there is a performance
> penalty because of the memory barriers. But errors?

I dunno whether I can call the issue as error. The situation is one i2c client
has a bit strict timing requirement. Without the patch, if there are heavy L2 cache
maintenance operations at the same time, there may be long delay between each
DW_IC_DATA_CMD write opeartions in i2c_dw_xfer_msg() in the DW_IC_INTR_TX_EMPTY isr.

Thus about 1/300 i2c transactions may be ignored by the i2c client per my test.

> 
> > The driver does not perform DMA, so it's safe to use the relaxed version.
> > From another side, the relaxed io accessor macros are available on all
> > architectures now, so we can use the relaxed versions instead.
> 
> Can the designware core make use of DMA in theory?
> 

the IP can do DMA in theory. But currently, there's no DMA support in the driver.

Thanks for your review,
Jisheng

WARNING: multiple messages have this Message-ID (diff)
From: jszhang@marvell.com (Jisheng Zhang)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] i2c: designware: use {readl|writel}_relaxed instead of readl/writel
Date: Wed, 14 Jan 2015 12:05:09 +0800	[thread overview]
Message-ID: <20150114120509.0ba8267e@xhacker> (raw)
In-Reply-To: <20150113143654.GG7660@katana>

Dear Wolfram,

On Tue, 13 Jan 2015 06:36:54 -0800
Wolfram Sang <wsa@the-dreams.de> wrote:

> 
> On Thu, Dec 11, 2014 at 02:26:41PM +0800, Jisheng Zhang wrote:
> > readl/writel is too expensive especially on Cortex A9 w/ outer L2 cache.
> > This introduces i2c read/write errors on Marvell BG2/BG2Q SoCs when there
> > are heavy L2 cache maintenance operations at the same time.
> 
> Reading this again, I got a question:
> 
> Really read/write errors? I would think that there is a performance
> penalty because of the memory barriers. But errors?

I dunno whether I can call the issue as error. The situation is one i2c client
has a bit strict timing requirement. Without the patch, if there are heavy L2 cache
maintenance operations at the same time, there may be long delay between each
DW_IC_DATA_CMD write opeartions in i2c_dw_xfer_msg() in the DW_IC_INTR_TX_EMPTY isr.

Thus about 1/300 i2c transactions may be ignored by the i2c client per my test.

> 
> > The driver does not perform DMA, so it's safe to use the relaxed version.
> > From another side, the relaxed io accessor macros are available on all
> > architectures now, so we can use the relaxed versions instead.
> 
> Can the designware core make use of DMA in theory?
> 

the IP can do DMA in theory. But currently, there's no DMA support in the driver.

Thanks for your review,
Jisheng

WARNING: multiple messages have this Message-ID (diff)
From: Jisheng Zhang <jszhang@marvell.com>
To: Wolfram Sang <wsa@the-dreams.de>
Cc: "linux-i2c@vger.kernel.org" <linux-i2c@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-arm-kernel@lists.infradead.org" 
	<linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH] i2c: designware: use {readl|writel}_relaxed instead of readl/writel
Date: Wed, 14 Jan 2015 12:05:09 +0800	[thread overview]
Message-ID: <20150114120509.0ba8267e@xhacker> (raw)
In-Reply-To: <20150113143654.GG7660@katana>

Dear Wolfram,

On Tue, 13 Jan 2015 06:36:54 -0800
Wolfram Sang <wsa@the-dreams.de> wrote:

> 
> On Thu, Dec 11, 2014 at 02:26:41PM +0800, Jisheng Zhang wrote:
> > readl/writel is too expensive especially on Cortex A9 w/ outer L2 cache.
> > This introduces i2c read/write errors on Marvell BG2/BG2Q SoCs when there
> > are heavy L2 cache maintenance operations at the same time.
> 
> Reading this again, I got a question:
> 
> Really read/write errors? I would think that there is a performance
> penalty because of the memory barriers. But errors?

I dunno whether I can call the issue as error. The situation is one i2c client
has a bit strict timing requirement. Without the patch, if there are heavy L2 cache
maintenance operations at the same time, there may be long delay between each
DW_IC_DATA_CMD write opeartions in i2c_dw_xfer_msg() in the DW_IC_INTR_TX_EMPTY isr.

Thus about 1/300 i2c transactions may be ignored by the i2c client per my test.

> 
> > The driver does not perform DMA, so it's safe to use the relaxed version.
> > From another side, the relaxed io accessor macros are available on all
> > architectures now, so we can use the relaxed versions instead.
> 
> Can the designware core make use of DMA in theory?
> 

the IP can do DMA in theory. But currently, there's no DMA support in the driver.

Thanks for your review,
Jisheng

  parent reply	other threads:[~2015-01-14  4:05 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-12-11  6:26 [PATCH] i2c: designware: use {readl|writel}_relaxed instead of readl/writel Jisheng Zhang
2014-12-11  6:26 ` Jisheng Zhang
2014-12-11  6:26 ` Jisheng Zhang
     [not found] ` <1418279201-3886-1-git-send-email-jszhang-eYqpPyKDWXRBDgjK7y7TUQ@public.gmane.org>
2014-12-19  2:43   ` Jisheng Zhang
2014-12-19  2:43     ` Jisheng Zhang
2014-12-19  2:43     ` Jisheng Zhang
2015-01-13 11:52     ` Wolfram Sang
2015-01-13 11:52       ` Wolfram Sang
2015-01-13 11:52       ` Wolfram Sang
2015-01-13 14:29       ` Mika Westerberg
2015-01-13 14:29         ` Mika Westerberg
2015-01-13 14:29         ` Mika Westerberg
2015-01-13 14:36   ` Wolfram Sang
2015-01-13 14:36     ` Wolfram Sang
2015-01-13 14:36     ` Wolfram Sang
2015-01-13 16:28     ` Baruch Siach
2015-01-13 16:28       ` Baruch Siach
2015-01-14  4:05     ` Jisheng Zhang [this message]
2015-01-14  4:05       ` Jisheng Zhang
2015-01-14  4:05       ` Jisheng Zhang
2015-01-23 16:09 ` Wolfram Sang
2015-01-23 16:09   ` Wolfram Sang

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=20150114120509.0ba8267e@xhacker \
    --to=jszhang-eyqppykdwxrbdgjk7y7tuq@public.gmane.org \
    --cc=linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org \
    --cc=linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.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 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.