* [PATCH] i2c: designware: use {readl|writel}_relaxed instead of readl/writel
@ 2014-12-11 6:26 ` Jisheng Zhang
0 siblings, 0 replies; 22+ messages in thread
From: Jisheng Zhang @ 2014-12-11 6:26 UTC (permalink / raw)
To: wsa-z923LK4zBo2bacvFa/9K2g, linux-i2c-u79uwXL29TY76Z2rM5mHXA
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Jisheng Zhang
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.
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.
Signed-off-by: Jisheng Zhang <jszhang-eYqpPyKDWXRBDgjK7y7TUQ@public.gmane.org>
---
drivers/i2c/busses/i2c-designware-core.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/i2c/busses/i2c-designware-core.c b/drivers/i2c/busses/i2c-designware-core.c
index 23628b7..e279948 100644
--- a/drivers/i2c/busses/i2c-designware-core.c
+++ b/drivers/i2c/busses/i2c-designware-core.c
@@ -170,10 +170,10 @@ u32 dw_readl(struct dw_i2c_dev *dev, int offset)
u32 value;
if (dev->accessor_flags & ACCESS_16BIT)
- value = readw(dev->base + offset) |
- (readw(dev->base + offset + 2) << 16);
+ value = readw_relaxed(dev->base + offset) |
+ (readw_relaxed(dev->base + offset + 2) << 16);
else
- value = readl(dev->base + offset);
+ value = readl_relaxed(dev->base + offset);
if (dev->accessor_flags & ACCESS_SWAP)
return swab32(value);
@@ -187,10 +187,10 @@ void dw_writel(struct dw_i2c_dev *dev, u32 b, int offset)
b = swab32(b);
if (dev->accessor_flags & ACCESS_16BIT) {
- writew((u16)b, dev->base + offset);
- writew((u16)(b >> 16), dev->base + offset + 2);
+ writew_relaxed((u16)b, dev->base + offset);
+ writew_relaxed((u16)(b >> 16), dev->base + offset + 2);
} else {
- writel(b, dev->base + offset);
+ writel_relaxed(b, dev->base + offset);
}
}
--
2.1.3
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH] i2c: designware: use {readl|writel}_relaxed instead of readl/writel @ 2014-12-11 6:26 ` Jisheng Zhang 0 siblings, 0 replies; 22+ messages in thread From: Jisheng Zhang @ 2014-12-11 6:26 UTC (permalink / raw) To: wsa, linux-i2c; +Cc: linux-kernel, linux-arm-kernel, Jisheng Zhang 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. 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. Signed-off-by: Jisheng Zhang <jszhang@marvell.com> --- drivers/i2c/busses/i2c-designware-core.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/drivers/i2c/busses/i2c-designware-core.c b/drivers/i2c/busses/i2c-designware-core.c index 23628b7..e279948 100644 --- a/drivers/i2c/busses/i2c-designware-core.c +++ b/drivers/i2c/busses/i2c-designware-core.c @@ -170,10 +170,10 @@ u32 dw_readl(struct dw_i2c_dev *dev, int offset) u32 value; if (dev->accessor_flags & ACCESS_16BIT) - value = readw(dev->base + offset) | - (readw(dev->base + offset + 2) << 16); + value = readw_relaxed(dev->base + offset) | + (readw_relaxed(dev->base + offset + 2) << 16); else - value = readl(dev->base + offset); + value = readl_relaxed(dev->base + offset); if (dev->accessor_flags & ACCESS_SWAP) return swab32(value); @@ -187,10 +187,10 @@ void dw_writel(struct dw_i2c_dev *dev, u32 b, int offset) b = swab32(b); if (dev->accessor_flags & ACCESS_16BIT) { - writew((u16)b, dev->base + offset); - writew((u16)(b >> 16), dev->base + offset + 2); + writew_relaxed((u16)b, dev->base + offset); + writew_relaxed((u16)(b >> 16), dev->base + offset + 2); } else { - writel(b, dev->base + offset); + writel_relaxed(b, dev->base + offset); } } -- 2.1.3 ^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH] i2c: designware: use {readl|writel}_relaxed instead of readl/writel @ 2014-12-11 6:26 ` Jisheng Zhang 0 siblings, 0 replies; 22+ messages in thread From: Jisheng Zhang @ 2014-12-11 6:26 UTC (permalink / raw) To: linux-arm-kernel 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. 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. Signed-off-by: Jisheng Zhang <jszhang@marvell.com> --- drivers/i2c/busses/i2c-designware-core.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/drivers/i2c/busses/i2c-designware-core.c b/drivers/i2c/busses/i2c-designware-core.c index 23628b7..e279948 100644 --- a/drivers/i2c/busses/i2c-designware-core.c +++ b/drivers/i2c/busses/i2c-designware-core.c @@ -170,10 +170,10 @@ u32 dw_readl(struct dw_i2c_dev *dev, int offset) u32 value; if (dev->accessor_flags & ACCESS_16BIT) - value = readw(dev->base + offset) | - (readw(dev->base + offset + 2) << 16); + value = readw_relaxed(dev->base + offset) | + (readw_relaxed(dev->base + offset + 2) << 16); else - value = readl(dev->base + offset); + value = readl_relaxed(dev->base + offset); if (dev->accessor_flags & ACCESS_SWAP) return swab32(value); @@ -187,10 +187,10 @@ void dw_writel(struct dw_i2c_dev *dev, u32 b, int offset) b = swab32(b); if (dev->accessor_flags & ACCESS_16BIT) { - writew((u16)b, dev->base + offset); - writew((u16)(b >> 16), dev->base + offset + 2); + writew_relaxed((u16)b, dev->base + offset); + writew_relaxed((u16)(b >> 16), dev->base + offset + 2); } else { - writel(b, dev->base + offset); + writel_relaxed(b, dev->base + offset); } } -- 2.1.3 ^ permalink raw reply related [flat|nested] 22+ messages in thread
[parent not found: <1418279201-3886-1-git-send-email-jszhang-eYqpPyKDWXRBDgjK7y7TUQ@public.gmane.org>]
* Re: [PATCH] i2c: designware: use {readl|writel}_relaxed instead of readl/writel 2014-12-11 6:26 ` Jisheng Zhang (?) @ 2014-12-19 2:43 ` Jisheng Zhang -1 siblings, 0 replies; 22+ messages in thread From: Jisheng Zhang @ 2014-12-19 2:43 UTC (permalink / raw) To: wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org, linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org Dear all, Is there any issue I need to resolve so that the patch can be merged? Thanks in advance, Jisheng On Wed, 10 Dec 2014 22:26:41 -0800 Jisheng Zhang <jszhang-eYqpPyKDWXRBDgjK7y7TUQ@public.gmane.org> 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. > > 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. > > Signed-off-by: Jisheng Zhang <jszhang-eYqpPyKDWXRBDgjK7y7TUQ@public.gmane.org> > --- > drivers/i2c/busses/i2c-designware-core.c | 12 ++++++------ > 1 file changed, 6 insertions(+), 6 deletions(-) > > diff --git a/drivers/i2c/busses/i2c-designware-core.c > b/drivers/i2c/busses/i2c-designware-core.c index 23628b7..e279948 100644 > --- a/drivers/i2c/busses/i2c-designware-core.c > +++ b/drivers/i2c/busses/i2c-designware-core.c > @@ -170,10 +170,10 @@ u32 dw_readl(struct dw_i2c_dev *dev, int offset) > u32 value; > > if (dev->accessor_flags & ACCESS_16BIT) > - value = readw(dev->base + offset) | > - (readw(dev->base + offset + 2) << 16); > + value = readw_relaxed(dev->base + offset) | > + (readw_relaxed(dev->base + offset + 2) << 16); > else > - value = readl(dev->base + offset); > + value = readl_relaxed(dev->base + offset); > > if (dev->accessor_flags & ACCESS_SWAP) > return swab32(value); > @@ -187,10 +187,10 @@ void dw_writel(struct dw_i2c_dev *dev, u32 b, int > offset) b = swab32(b); > > if (dev->accessor_flags & ACCESS_16BIT) { > - writew((u16)b, dev->base + offset); > - writew((u16)(b >> 16), dev->base + offset + 2); > + writew_relaxed((u16)b, dev->base + offset); > + writew_relaxed((u16)(b >> 16), dev->base + offset + 2); > } else { > - writel(b, dev->base + offset); > + writel_relaxed(b, dev->base + offset); > } > } > ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] i2c: designware: use {readl|writel}_relaxed instead of readl/writel @ 2014-12-19 2:43 ` Jisheng Zhang 0 siblings, 0 replies; 22+ messages in thread From: Jisheng Zhang @ 2014-12-19 2:43 UTC (permalink / raw) To: wsa@the-dreams.de, linux-i2c@vger.kernel.org Cc: linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org Dear all, Is there any issue I need to resolve so that the patch can be merged? Thanks in advance, Jisheng On Wed, 10 Dec 2014 22:26:41 -0800 Jisheng Zhang <jszhang@marvell.com> 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. > > 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. > > Signed-off-by: Jisheng Zhang <jszhang@marvell.com> > --- > drivers/i2c/busses/i2c-designware-core.c | 12 ++++++------ > 1 file changed, 6 insertions(+), 6 deletions(-) > > diff --git a/drivers/i2c/busses/i2c-designware-core.c > b/drivers/i2c/busses/i2c-designware-core.c index 23628b7..e279948 100644 > --- a/drivers/i2c/busses/i2c-designware-core.c > +++ b/drivers/i2c/busses/i2c-designware-core.c > @@ -170,10 +170,10 @@ u32 dw_readl(struct dw_i2c_dev *dev, int offset) > u32 value; > > if (dev->accessor_flags & ACCESS_16BIT) > - value = readw(dev->base + offset) | > - (readw(dev->base + offset + 2) << 16); > + value = readw_relaxed(dev->base + offset) | > + (readw_relaxed(dev->base + offset + 2) << 16); > else > - value = readl(dev->base + offset); > + value = readl_relaxed(dev->base + offset); > > if (dev->accessor_flags & ACCESS_SWAP) > return swab32(value); > @@ -187,10 +187,10 @@ void dw_writel(struct dw_i2c_dev *dev, u32 b, int > offset) b = swab32(b); > > if (dev->accessor_flags & ACCESS_16BIT) { > - writew((u16)b, dev->base + offset); > - writew((u16)(b >> 16), dev->base + offset + 2); > + writew_relaxed((u16)b, dev->base + offset); > + writew_relaxed((u16)(b >> 16), dev->base + offset + 2); > } else { > - writel(b, dev->base + offset); > + writel_relaxed(b, dev->base + offset); > } > } > ^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH] i2c: designware: use {readl|writel}_relaxed instead of readl/writel @ 2014-12-19 2:43 ` Jisheng Zhang 0 siblings, 0 replies; 22+ messages in thread From: Jisheng Zhang @ 2014-12-19 2:43 UTC (permalink / raw) To: linux-arm-kernel Dear all, Is there any issue I need to resolve so that the patch can be merged? Thanks in advance, Jisheng On Wed, 10 Dec 2014 22:26:41 -0800 Jisheng Zhang <jszhang@marvell.com> 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. > > 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. > > Signed-off-by: Jisheng Zhang <jszhang@marvell.com> > --- > drivers/i2c/busses/i2c-designware-core.c | 12 ++++++------ > 1 file changed, 6 insertions(+), 6 deletions(-) > > diff --git a/drivers/i2c/busses/i2c-designware-core.c > b/drivers/i2c/busses/i2c-designware-core.c index 23628b7..e279948 100644 > --- a/drivers/i2c/busses/i2c-designware-core.c > +++ b/drivers/i2c/busses/i2c-designware-core.c > @@ -170,10 +170,10 @@ u32 dw_readl(struct dw_i2c_dev *dev, int offset) > u32 value; > > if (dev->accessor_flags & ACCESS_16BIT) > - value = readw(dev->base + offset) | > - (readw(dev->base + offset + 2) << 16); > + value = readw_relaxed(dev->base + offset) | > + (readw_relaxed(dev->base + offset + 2) << 16); > else > - value = readl(dev->base + offset); > + value = readl_relaxed(dev->base + offset); > > if (dev->accessor_flags & ACCESS_SWAP) > return swab32(value); > @@ -187,10 +187,10 @@ void dw_writel(struct dw_i2c_dev *dev, u32 b, int > offset) b = swab32(b); > > if (dev->accessor_flags & ACCESS_16BIT) { > - writew((u16)b, dev->base + offset); > - writew((u16)(b >> 16), dev->base + offset + 2); > + writew_relaxed((u16)b, dev->base + offset); > + writew_relaxed((u16)(b >> 16), dev->base + offset + 2); > } else { > - writel(b, dev->base + offset); > + writel_relaxed(b, dev->base + offset); > } > } > ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] i2c: designware: use {readl|writel}_relaxed instead of readl/writel 2014-12-19 2:43 ` Jisheng Zhang (?) @ 2015-01-13 11:52 ` Wolfram Sang -1 siblings, 0 replies; 22+ messages in thread From: Wolfram Sang @ 2015-01-13 11:52 UTC (permalink / raw) To: Jisheng Zhang, Mika Westerberg Cc: linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org [-- Attachment #1: Type: text/plain, Size: 2323 bytes --] On Fri, Dec 19, 2014 at 10:43:15AM +0800, Jisheng Zhang wrote: > Dear all, > > Is there any issue I need to resolve so that the patch can be merged? Adding Mika to the loop. He uses the driver a lot (and knows other people who do)... > On Wed, 10 Dec 2014 22:26:41 -0800 > Jisheng Zhang <jszhang-eYqpPyKDWXRBDgjK7y7TUQ@public.gmane.org> 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. > > > > 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. > > > > Signed-off-by: Jisheng Zhang <jszhang-eYqpPyKDWXRBDgjK7y7TUQ@public.gmane.org> > > --- > > drivers/i2c/busses/i2c-designware-core.c | 12 ++++++------ > > 1 file changed, 6 insertions(+), 6 deletions(-) > > > > diff --git a/drivers/i2c/busses/i2c-designware-core.c > > b/drivers/i2c/busses/i2c-designware-core.c index 23628b7..e279948 100644 > > --- a/drivers/i2c/busses/i2c-designware-core.c > > +++ b/drivers/i2c/busses/i2c-designware-core.c > > @@ -170,10 +170,10 @@ u32 dw_readl(struct dw_i2c_dev *dev, int offset) > > u32 value; > > > > if (dev->accessor_flags & ACCESS_16BIT) > > - value = readw(dev->base + offset) | > > - (readw(dev->base + offset + 2) << 16); > > + value = readw_relaxed(dev->base + offset) | > > + (readw_relaxed(dev->base + offset + 2) << 16); > > else > > - value = readl(dev->base + offset); > > + value = readl_relaxed(dev->base + offset); > > > > if (dev->accessor_flags & ACCESS_SWAP) > > return swab32(value); > > @@ -187,10 +187,10 @@ void dw_writel(struct dw_i2c_dev *dev, u32 b, int > > offset) b = swab32(b); > > > > if (dev->accessor_flags & ACCESS_16BIT) { > > - writew((u16)b, dev->base + offset); > > - writew((u16)(b >> 16), dev->base + offset + 2); > > + writew_relaxed((u16)b, dev->base + offset); > > + writew_relaxed((u16)(b >> 16), dev->base + offset + 2); > > } else { > > - writel(b, dev->base + offset); > > + writel_relaxed(b, dev->base + offset); > > } > > } > > > [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] i2c: designware: use {readl|writel}_relaxed instead of readl/writel @ 2015-01-13 11:52 ` Wolfram Sang 0 siblings, 0 replies; 22+ messages in thread From: Wolfram Sang @ 2015-01-13 11:52 UTC (permalink / raw) To: Jisheng Zhang, Mika Westerberg Cc: linux-i2c@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org [-- Attachment #1: Type: text/plain, Size: 2267 bytes --] On Fri, Dec 19, 2014 at 10:43:15AM +0800, Jisheng Zhang wrote: > Dear all, > > Is there any issue I need to resolve so that the patch can be merged? Adding Mika to the loop. He uses the driver a lot (and knows other people who do)... > On Wed, 10 Dec 2014 22:26:41 -0800 > Jisheng Zhang <jszhang@marvell.com> 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. > > > > 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. > > > > Signed-off-by: Jisheng Zhang <jszhang@marvell.com> > > --- > > drivers/i2c/busses/i2c-designware-core.c | 12 ++++++------ > > 1 file changed, 6 insertions(+), 6 deletions(-) > > > > diff --git a/drivers/i2c/busses/i2c-designware-core.c > > b/drivers/i2c/busses/i2c-designware-core.c index 23628b7..e279948 100644 > > --- a/drivers/i2c/busses/i2c-designware-core.c > > +++ b/drivers/i2c/busses/i2c-designware-core.c > > @@ -170,10 +170,10 @@ u32 dw_readl(struct dw_i2c_dev *dev, int offset) > > u32 value; > > > > if (dev->accessor_flags & ACCESS_16BIT) > > - value = readw(dev->base + offset) | > > - (readw(dev->base + offset + 2) << 16); > > + value = readw_relaxed(dev->base + offset) | > > + (readw_relaxed(dev->base + offset + 2) << 16); > > else > > - value = readl(dev->base + offset); > > + value = readl_relaxed(dev->base + offset); > > > > if (dev->accessor_flags & ACCESS_SWAP) > > return swab32(value); > > @@ -187,10 +187,10 @@ void dw_writel(struct dw_i2c_dev *dev, u32 b, int > > offset) b = swab32(b); > > > > if (dev->accessor_flags & ACCESS_16BIT) { > > - writew((u16)b, dev->base + offset); > > - writew((u16)(b >> 16), dev->base + offset + 2); > > + writew_relaxed((u16)b, dev->base + offset); > > + writew_relaxed((u16)(b >> 16), dev->base + offset + 2); > > } else { > > - writel(b, dev->base + offset); > > + writel_relaxed(b, dev->base + offset); > > } > > } > > > [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH] i2c: designware: use {readl|writel}_relaxed instead of readl/writel @ 2015-01-13 11:52 ` Wolfram Sang 0 siblings, 0 replies; 22+ messages in thread From: Wolfram Sang @ 2015-01-13 11:52 UTC (permalink / raw) To: linux-arm-kernel On Fri, Dec 19, 2014 at 10:43:15AM +0800, Jisheng Zhang wrote: > Dear all, > > Is there any issue I need to resolve so that the patch can be merged? Adding Mika to the loop. He uses the driver a lot (and knows other people who do)... > On Wed, 10 Dec 2014 22:26:41 -0800 > Jisheng Zhang <jszhang@marvell.com> 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. > > > > 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. > > > > Signed-off-by: Jisheng Zhang <jszhang@marvell.com> > > --- > > drivers/i2c/busses/i2c-designware-core.c | 12 ++++++------ > > 1 file changed, 6 insertions(+), 6 deletions(-) > > > > diff --git a/drivers/i2c/busses/i2c-designware-core.c > > b/drivers/i2c/busses/i2c-designware-core.c index 23628b7..e279948 100644 > > --- a/drivers/i2c/busses/i2c-designware-core.c > > +++ b/drivers/i2c/busses/i2c-designware-core.c > > @@ -170,10 +170,10 @@ u32 dw_readl(struct dw_i2c_dev *dev, int offset) > > u32 value; > > > > if (dev->accessor_flags & ACCESS_16BIT) > > - value = readw(dev->base + offset) | > > - (readw(dev->base + offset + 2) << 16); > > + value = readw_relaxed(dev->base + offset) | > > + (readw_relaxed(dev->base + offset + 2) << 16); > > else > > - value = readl(dev->base + offset); > > + value = readl_relaxed(dev->base + offset); > > > > if (dev->accessor_flags & ACCESS_SWAP) > > return swab32(value); > > @@ -187,10 +187,10 @@ void dw_writel(struct dw_i2c_dev *dev, u32 b, int > > offset) b = swab32(b); > > > > if (dev->accessor_flags & ACCESS_16BIT) { > > - writew((u16)b, dev->base + offset); > > - writew((u16)(b >> 16), dev->base + offset + 2); > > + writew_relaxed((u16)b, dev->base + offset); > > + writew_relaxed((u16)(b >> 16), dev->base + offset + 2); > > } else { > > - writel(b, dev->base + offset); > > + writel_relaxed(b, dev->base + offset); > > } > > } > > > -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: Digital signature URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150113/e21dd833/attachment.sig> ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] i2c: designware: use {readl|writel}_relaxed instead of readl/writel 2015-01-13 11:52 ` Wolfram Sang (?) @ 2015-01-13 14:29 ` Mika Westerberg -1 siblings, 0 replies; 22+ messages in thread From: Mika Westerberg @ 2015-01-13 14:29 UTC (permalink / raw) To: Wolfram Sang Cc: Jisheng Zhang, linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org On Tue, Jan 13, 2015 at 12:52:05PM +0100, Wolfram Sang wrote: > On Fri, Dec 19, 2014 at 10:43:15AM +0800, Jisheng Zhang wrote: > > Dear all, > > > > Is there any issue I need to resolve so that the patch can be merged? > > Adding Mika to the loop. He uses the driver a lot (and knows other > people who do)... I don't see problems with this patch. On x86 we have readl_relaxed() the same as readl() so this patch does not change anything there. Feel free to add, Acked-by: Mika Westerberg <mika.westerberg-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] i2c: designware: use {readl|writel}_relaxed instead of readl/writel @ 2015-01-13 14:29 ` Mika Westerberg 0 siblings, 0 replies; 22+ messages in thread From: Mika Westerberg @ 2015-01-13 14:29 UTC (permalink / raw) To: Wolfram Sang Cc: Jisheng Zhang, linux-i2c@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org On Tue, Jan 13, 2015 at 12:52:05PM +0100, Wolfram Sang wrote: > On Fri, Dec 19, 2014 at 10:43:15AM +0800, Jisheng Zhang wrote: > > Dear all, > > > > Is there any issue I need to resolve so that the patch can be merged? > > Adding Mika to the loop. He uses the driver a lot (and knows other > people who do)... I don't see problems with this patch. On x86 we have readl_relaxed() the same as readl() so this patch does not change anything there. Feel free to add, Acked-by: Mika Westerberg <mika.westerberg@linux.intel.com> ^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH] i2c: designware: use {readl|writel}_relaxed instead of readl/writel @ 2015-01-13 14:29 ` Mika Westerberg 0 siblings, 0 replies; 22+ messages in thread From: Mika Westerberg @ 2015-01-13 14:29 UTC (permalink / raw) To: linux-arm-kernel On Tue, Jan 13, 2015 at 12:52:05PM +0100, Wolfram Sang wrote: > On Fri, Dec 19, 2014 at 10:43:15AM +0800, Jisheng Zhang wrote: > > Dear all, > > > > Is there any issue I need to resolve so that the patch can be merged? > > Adding Mika to the loop. He uses the driver a lot (and knows other > people who do)... I don't see problems with this patch. On x86 we have readl_relaxed() the same as readl() so this patch does not change anything there. Feel free to add, Acked-by: Mika Westerberg <mika.westerberg@linux.intel.com> ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] i2c: designware: use {readl|writel}_relaxed instead of readl/writel 2014-12-11 6:26 ` Jisheng Zhang (?) @ 2015-01-13 14:36 ` Wolfram Sang -1 siblings, 0 replies; 22+ messages in thread From: Wolfram Sang @ 2015-01-13 14:36 UTC (permalink / raw) To: Jisheng Zhang Cc: linux-i2c-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r [-- Attachment #1: Type: text/plain, Size: 705 bytes --] 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? > 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? [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] i2c: designware: use {readl|writel}_relaxed instead of readl/writel @ 2015-01-13 14:36 ` Wolfram Sang 0 siblings, 0 replies; 22+ messages in thread From: Wolfram Sang @ 2015-01-13 14:36 UTC (permalink / raw) To: Jisheng Zhang; +Cc: linux-i2c, linux-kernel, linux-arm-kernel [-- Attachment #1: Type: text/plain, Size: 705 bytes --] 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? > 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? [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH] i2c: designware: use {readl|writel}_relaxed instead of readl/writel @ 2015-01-13 14:36 ` Wolfram Sang 0 siblings, 0 replies; 22+ messages in thread From: Wolfram Sang @ 2015-01-13 14:36 UTC (permalink / raw) To: linux-arm-kernel 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? > 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? -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: Digital signature URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150113/471af9f5/attachment.sig> ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] i2c: designware: use {readl|writel}_relaxed instead of readl/writel 2015-01-13 14:36 ` Wolfram Sang @ 2015-01-13 16:28 ` Baruch Siach -1 siblings, 0 replies; 22+ messages in thread From: Baruch Siach @ 2015-01-13 16:28 UTC (permalink / raw) To: Wolfram Sang; +Cc: Jisheng Zhang, linux-i2c, linux-arm-kernel, linux-kernel Hi Wolfram, On Tue, Jan 13, 2015 at 03:36:54PM +0100, Wolfram Sang wrote: > > 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? According to the "DesignWare DW_apb_i2c Databook" version I have it can (optionally). baruch -- http://baruch.siach.name/blog/ ~. .~ Tk Open Systems =}------------------------------------------------ooO--U--Ooo------------{= - baruch@tkos.co.il - tel: +972.2.679.5364, http://www.tkos.co.il - ^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH] i2c: designware: use {readl|writel}_relaxed instead of readl/writel @ 2015-01-13 16:28 ` Baruch Siach 0 siblings, 0 replies; 22+ messages in thread From: Baruch Siach @ 2015-01-13 16:28 UTC (permalink / raw) To: linux-arm-kernel Hi Wolfram, On Tue, Jan 13, 2015 at 03:36:54PM +0100, Wolfram Sang wrote: > > 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? According to the "DesignWare DW_apb_i2c Databook" version I have it can (optionally). baruch -- http://baruch.siach.name/blog/ ~. .~ Tk Open Systems =}------------------------------------------------ooO--U--Ooo------------{= - baruch at tkos.co.il - tel: +972.2.679.5364, http://www.tkos.co.il - ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] i2c: designware: use {readl|writel}_relaxed instead of readl/writel 2015-01-13 14:36 ` Wolfram Sang (?) @ 2015-01-14 4:05 ` Jisheng Zhang -1 siblings, 0 replies; 22+ messages in thread From: Jisheng Zhang @ 2015-01-14 4:05 UTC (permalink / raw) To: Wolfram Sang Cc: linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org 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 ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] i2c: designware: use {readl|writel}_relaxed instead of readl/writel @ 2015-01-14 4:05 ` Jisheng Zhang 0 siblings, 0 replies; 22+ messages in thread From: Jisheng Zhang @ 2015-01-14 4:05 UTC (permalink / raw) To: Wolfram Sang Cc: linux-i2c@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org 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 ^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH] i2c: designware: use {readl|writel}_relaxed instead of readl/writel @ 2015-01-14 4:05 ` Jisheng Zhang 0 siblings, 0 replies; 22+ messages in thread From: Jisheng Zhang @ 2015-01-14 4:05 UTC (permalink / raw) To: linux-arm-kernel 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 ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] i2c: designware: use {readl|writel}_relaxed instead of readl/writel 2014-12-11 6:26 ` Jisheng Zhang @ 2015-01-23 16:09 ` Wolfram Sang -1 siblings, 0 replies; 22+ messages in thread From: Wolfram Sang @ 2015-01-23 16:09 UTC (permalink / raw) To: Jisheng Zhang; +Cc: linux-i2c, linux-kernel, linux-arm-kernel [-- Attachment #1: Type: text/plain, Size: 651 bytes --] 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. > > 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. > > Signed-off-by: Jisheng Zhang <jszhang@marvell.com> Applied to for-next, thanks! Just replaced "errors" with "delays" in the description. [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH] i2c: designware: use {readl|writel}_relaxed instead of readl/writel @ 2015-01-23 16:09 ` Wolfram Sang 0 siblings, 0 replies; 22+ messages in thread From: Wolfram Sang @ 2015-01-23 16:09 UTC (permalink / raw) To: linux-arm-kernel 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. > > 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. > > Signed-off-by: Jisheng Zhang <jszhang@marvell.com> Applied to for-next, thanks! Just replaced "errors" with "delays" in the description. -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: Digital signature URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150123/54ca148c/attachment.sig> ^ permalink raw reply [flat|nested] 22+ messages in thread
end of thread, other threads:[~2015-01-23 16:09 UTC | newest]
Thread overview: 22+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
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
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
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.