Modern MIPS cores (like P5600/6600, M5150/6520, end so on) which
got L2-cache on chip also can enable a special type Cache-Coherency
attribute (CCA) named UnCached Accelerated attribute (UCA). In this
way uncached accelerated accesses are treated the same way as
non-accelerated uncached accesses, but uncached stores are gathered
together for more efficient bus utilization. So to speak this CCA
enables uncached transactions to better utilize bus bandwidth via
burst transactions.
This is exactly why ioremap_wc() method has been introduced in linux.
Alas MIPS-platform code hasn't implemented it so far, instead default
one has been used which was an alias to ioremap_nocache. In order to
fix this we added MIPS-specific ioremap_wc() macro substituted by
generic __ioremap_mode() method call with writecombine CPU-info
field passed. It shall create real ioremap_wc() method if CPU-cache
supports UCA feature and fall-back to _CACHE_UNCACHED attribute
if one doesn't. Additionally platform-specific io.h shall declare
ARCH_HAS_IOREMAP_WC macro as indication of architectural definition
of ioremap_wc() (similar to x86/powerpc).
Signed-off-by: Serge Semin <[email protected]>
Singed-off-by: Paul Burton <[email protected]>
Cc: James Hogan <[email protected]>
Cc: Ralf Baechle <[email protected]>
Cc: [email protected]
Cc: [email protected]
---
arch/mips/include/asm/io.h | 23 +++++++++++++++++++++++
1 file changed, 23 insertions(+)
diff --git a/arch/mips/include/asm/io.h b/arch/mips/include/asm/io.h
index 4d709b61d..d4f8cdc58 100644
--- a/arch/mips/include/asm/io.h
+++ b/arch/mips/include/asm/io.h
@@ -12,6 +12,8 @@
#ifndef _ASM_IO_H
#define _ASM_IO_H
+#define ARCH_HAS_IOREMAP_WC
+
#include <linux/compiler.h>
#include <linux/kernel.h>
#include <linux/types.h>
@@ -278,6 +280,27 @@ static inline void __iomem * __ioremap_mode(phys_addr_t offset, unsigned long si
#define ioremap_cache ioremap_cachable
/*
+ * ioremap_wc - map bus memory into CPU space
+ * @offset: bus address of the memory
+ * @size: size of the resource to map
+ *
+ * ioremap_wc performs a platform specific sequence of operations to
+ * make bus memory CPU accessible via the readb/readw/readl/writeb/
+ * writew/writel functions and the other mmio helpers. The returned
+ * address is not guaranteed to be usable directly as a virtual
+ * address.
+ *
+ * This version of ioremap ensures that the memory is marked uncachable
+ * but accelerated by means of write-combining feature. It is specifically
+ * useful for PCIe prefetchable windows, which may vastly improve a
+ * communications performance. If it was determined on boot stage, what
+ * CPU CCA doesn't support UCA, the method shall fall-back to the
+ * _CACHE_UNCACHED option (see cpu_probe() method).
+ */
+#define ioremap_wc(offset, size) \
+ __ioremap_mode((offset), (size), boot_cpu_data.writecombine)
+
+/*
* These two are MIPS specific ioremap variant. ioremap_cacheable_cow
* requests a cachable mapping, ioremap_uncached_accelerated requests a
* mapping using the uncached accelerated mode which isn't supported on
--
2.12.0
Adaptive ioremap_wc() method is now available (see "mips: mm:
Create UCA-based ioremap_wc() method" commit). We can use it for
UCA-featured MMIO transactions in the kernel, so we don't need
it platform clone ioremap_uncached_accelerated() being declard.
Seeing it is also unused anywhere in the kernel code, lets remove
it from io.h arch-specific header then.
Signed-off-by: Serge Semin <[email protected]>
Singed-off-by: Paul Burton <[email protected]>
Cc: James Hogan <[email protected]>
Cc: Ralf Baechle <[email protected]>
Cc: [email protected]
Cc: [email protected]
---
arch/mips/include/asm/io.h | 8 ++------
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/arch/mips/include/asm/io.h b/arch/mips/include/asm/io.h
index babe5155a..360b7ddeb 100644
--- a/arch/mips/include/asm/io.h
+++ b/arch/mips/include/asm/io.h
@@ -301,15 +301,11 @@ static inline void __iomem * __ioremap_mode(phys_addr_t offset, unsigned long si
__ioremap_mode((offset), (size), boot_cpu_data.writecombine)
/*
- * These two are MIPS specific ioremap variant. ioremap_cacheable_cow
- * requests a cachable mapping, ioremap_uncached_accelerated requests a
- * mapping using the uncached accelerated mode which isn't supported on
- * all processors.
+ * This is a MIPS specific ioremap variant. ioremap_cacheable_cow
+ * requests a cachable mapping with CWB attribute enabled.
*/
#define ioremap_cacheable_cow(offset, size) \
__ioremap_mode((offset), (size), _CACHE_CACHABLE_COW)
-#define ioremap_uncached_accelerated(offset, size) \
- __ioremap_mode((offset), (size), _CACHE_UNCACHED_ACCELERATED)
static inline void iounmap(const volatile void __iomem *addr)
{
--
2.12.0
'
On Mon, Jul 9, 2018 at 3:57 PM Serge Semin <[email protected]> wrote:
>
> Adaptive ioremap_wc() method is now available (see "mips: mm:
> Create UCA-based ioremap_wc() method" commit). We can use it for
> UCA-featured MMIO transactions in the kernel, so we don't need
> it platform clone ioremap_uncached_accelerated() being declard.
> Seeing it is also unused anywhere in the kernel code, lets remove
> it from io.h arch-specific header then.
>
> Signed-off-by: Serge Semin <[email protected]>
> Singed-off-by: Paul Burton <[email protected]>
nit: 'Signed' (on both patches)
> Cc: James Hogan <[email protected]>
> Cc: Ralf Baechle <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> ---
> arch/mips/include/asm/io.h | 8 ++------
> 1 file changed, 2 insertions(+), 6 deletions(-)
>
> diff --git a/arch/mips/include/asm/io.h b/arch/mips/include/asm/io.h
> index babe5155a..360b7ddeb 100644
> --- a/arch/mips/include/asm/io.h
> +++ b/arch/mips/include/asm/io.h
> @@ -301,15 +301,11 @@ static inline void __iomem * __ioremap_mode(phys_addr_t offset, unsigned long si
> __ioremap_mode((offset), (size), boot_cpu_data.writecombine)
>
> /*
> - * These two are MIPS specific ioremap variant. ioremap_cacheable_cow
> - * requests a cachable mapping, ioremap_uncached_accelerated requests a
> - * mapping using the uncached accelerated mode which isn't supported on
> - * all processors.
> + * This is a MIPS specific ioremap variant. ioremap_cacheable_cow
> + * requests a cachable mapping with CWB attribute enabled.
> */
> #define ioremap_cacheable_cow(offset, size) \
> __ioremap_mode((offset), (size), _CACHE_CACHABLE_COW)
> -#define ioremap_uncached_accelerated(offset, size) \
> - __ioremap_mode((offset), (size), _CACHE_UNCACHED_ACCELERATED)
>
> static inline void iounmap(const volatile void __iomem *addr)
> {
> --
> 2.12.0
>
>
On Tue, Jul 10, 2018 at 09:15:17AM +0200, Mathieu Malaterre <[email protected]> wrote:
> '
> On Mon, Jul 9, 2018 at 3:57 PM Serge Semin <[email protected]> wrote:
> >
> > Adaptive ioremap_wc() method is now available (see "mips: mm:
> > Create UCA-based ioremap_wc() method" commit). We can use it for
> > UCA-featured MMIO transactions in the kernel, so we don't need
> > it platform clone ioremap_uncached_accelerated() being declard.
> > Seeing it is also unused anywhere in the kernel code, lets remove
> > it from io.h arch-specific header then.
> >
> > Signed-off-by: Serge Semin <[email protected]>
> > Singed-off-by: Paul Burton <[email protected]>
>
> nit: 'Signed' (on both patches)
>
Good catch! Thanks. Didn't notice the typo. Should have copy-pasted
both the signature and the e-mail from another letter.
I'll fix it if there will be a second version of the patchset. Otherwise
I suppose it would be easier for the integrator to do this.
Regards,
-Sergey
> > Cc: James Hogan <[email protected]>
> > Cc: Ralf Baechle <[email protected]>
> > Cc: [email protected]
> > Cc: [email protected]
> > ---
> > arch/mips/include/asm/io.h | 8 ++------
> > 1 file changed, 2 insertions(+), 6 deletions(-)
> >
> > diff --git a/arch/mips/include/asm/io.h b/arch/mips/include/asm/io.h
> > index babe5155a..360b7ddeb 100644
> > --- a/arch/mips/include/asm/io.h
> > +++ b/arch/mips/include/asm/io.h
> > @@ -301,15 +301,11 @@ static inline void __iomem * __ioremap_mode(phys_addr_t offset, unsigned long si
> > __ioremap_mode((offset), (size), boot_cpu_data.writecombine)
> >
> > /*
> > - * These two are MIPS specific ioremap variant. ioremap_cacheable_cow
> > - * requests a cachable mapping, ioremap_uncached_accelerated requests a
> > - * mapping using the uncached accelerated mode which isn't supported on
> > - * all processors.
> > + * This is a MIPS specific ioremap variant. ioremap_cacheable_cow
> > + * requests a cachable mapping with CWB attribute enabled.
> > */
> > #define ioremap_cacheable_cow(offset, size) \
> > __ioremap_mode((offset), (size), _CACHE_CACHABLE_COW)
> > -#define ioremap_uncached_accelerated(offset, size) \
> > - __ioremap_mode((offset), (size), _CACHE_UNCACHED_ACCELERATED)
> >
> > static inline void iounmap(const volatile void __iomem *addr)
> > {
> > --
> > 2.12.0
> >
> >
Hi Sergey,
On Tue, Jul 10, 2018 at 10:48:15AM +0300, Serge Semin wrote:
> On Tue, Jul 10, 2018 at 09:15:17AM +0200, Mathieu Malaterre <[email protected]> wrote:
> > On Mon, Jul 9, 2018 at 3:57 PM Serge Semin <[email protected]> wrote:
> > > Adaptive ioremap_wc() method is now available (see "mips: mm:
> > > Create UCA-based ioremap_wc() method" commit). We can use it for
> > > UCA-featured MMIO transactions in the kernel, so we don't need
> > > it platform clone ioremap_uncached_accelerated() being declard.
> > > Seeing it is also unused anywhere in the kernel code, lets remove
> > > it from io.h arch-specific header then.
> > >
> > > Signed-off-by: Serge Semin <[email protected]>
> > > Singed-off-by: Paul Burton <[email protected]>
> >
> > nit: 'Signed' (on both patches)
>
> Good catch! Thanks. Didn't notice the typo. Should have copy-pasted
> both the signature and the e-mail from another letter.
>
> I'll fix it if there will be a second version of the patchset. Otherwise
> I suppose it would be easier for the integrator to do this.
I've fixed this up & applied these 2 patches with minor tweaks to
mips-next for 4.19.
However FYI for next time - you shouldn't really add someone else's
Signed-off-by tag anyway. The tag effectively states that a person can
agree to the Developer's Certificate of Origin for this patch (see
Documentation/process/submitting-patches.rst), and you can't agree that
on behalf of someone else. Generally a maintainer should add this tag
for themselves when they apply a patch.
Anyway, I think we should reserve the Singed-off-by tag for patches that
quell fires. ;)
Thanks,
Paul
On Tue, Jul 10, 2018 at 10:59:40AM -0700, Paul Burton <[email protected]> wrote:
Hello Paul,
> Hi Sergey,
>
> On Tue, Jul 10, 2018 at 10:48:15AM +0300, Serge Semin wrote:
> > On Tue, Jul 10, 2018 at 09:15:17AM +0200, Mathieu Malaterre <[email protected]> wrote:
> > > On Mon, Jul 9, 2018 at 3:57 PM Serge Semin <[email protected]> wrote:
> > > > Adaptive ioremap_wc() method is now available (see "mips: mm:
> > > > Create UCA-based ioremap_wc() method" commit). We can use it for
> > > > UCA-featured MMIO transactions in the kernel, so we don't need
> > > > it platform clone ioremap_uncached_accelerated() being declard.
> > > > Seeing it is also unused anywhere in the kernel code, lets remove
> > > > it from io.h arch-specific header then.
> > > >
> > > > Signed-off-by: Serge Semin <[email protected]>
> > > > Singed-off-by: Paul Burton <[email protected]>
> > >
> > > nit: 'Signed' (on both patches)
> >
> > Good catch! Thanks. Didn't notice the typo. Should have copy-pasted
> > both the signature and the e-mail from another letter.
> >
> > I'll fix it if there will be a second version of the patchset. Otherwise
> > I suppose it would be easier for the integrator to do this.
>
> I've fixed this up & applied these 2 patches with minor tweaks to
> mips-next for 4.19.
>
Great! Thanks.
> However FYI for next time - you shouldn't really add someone else's
> Signed-off-by tag anyway. The tag effectively states that a person can
> agree to the Developer's Certificate of Origin for this patch (see
> Documentation/process/submitting-patches.rst), and you can't agree that
> on behalf of someone else. Generally a maintainer should add this tag
> for themselves when they apply a patch.
>
I'm sorry if it seemed like I added Signed-off on your behalf. I thought
the Signed-off also concerns the ones, who participated in the patch
preparation. Since you suggested the design of the change, I've decided
to put your name in the Signed-off tag. What shall I use in this way
then?
> Anyway, I think we should reserve the Singed-off-by tag for patches that
> quell fires. ;)
>
> Thanks,
> Paul
Regards,
-Sergey
Hi Serge,
On Tue, Jul 10, 2018 at 10:13:54PM +0300, Serge Semin wrote:
> On Tue, Jul 10, 2018 at 10:59:40AM -0700, Paul Burton <[email protected]> wrote:
> > However FYI for next time - you shouldn't really add someone else's
> > Signed-off-by tag anyway. The tag effectively states that a person can
> > agree to the Developer's Certificate of Origin for this patch (see
> > Documentation/process/submitting-patches.rst), and you can't agree that
> > on behalf of someone else. Generally a maintainer should add this tag
> > for themselves when they apply a patch.
>
> I'm sorry if it seemed like I added Signed-off on your behalf.
That's OK, I didn't think you did it maliciously :)
> I thought the Signed-off also concerns the ones, who participated in
> the patch preparation. Since you suggested the design of the change,
> I've decided to put your name in the Signed-off tag. What shall I use
> in this way then?
In this case Suggested-by might have been a good choice. Reported-by is
also commonly used if someone reported a problem which you created a fix
for.
Section 13 of Documentation/process/submitting-patches.rst describes
these tags along with a couple others.
Thanks,
Paul
> + * This is a MIPS specific ioremap variant. ioremap_cacheable_cow
> + * requests a cachable mapping with CWB attribute enabled.
> */
> #define ioremap_cacheable_cow(offset, size) \
> __ioremap_mode((offset), (size), _CACHE_CACHABLE_COW)
This isn't actually used anywhere in the kernel tree. Please remove it
as well.
Paul,
On Tue, Jul 10, 2018 at 02:04:15PM -0700, Paul Burton <[email protected]> wrote:
> Hi Serge,
>
> On Tue, Jul 10, 2018 at 10:13:54PM +0300, Serge Semin wrote:
> > On Tue, Jul 10, 2018 at 10:59:40AM -0700, Paul Burton <[email protected]> wrote:
> > > However FYI for next time - you shouldn't really add someone else's
> > > Signed-off-by tag anyway. The tag effectively states that a person can
> > > agree to the Developer's Certificate of Origin for this patch (see
> > > Documentation/process/submitting-patches.rst), and you can't agree that
> > > on behalf of someone else. Generally a maintainer should add this tag
> > > for themselves when they apply a patch.
> >
> > I'm sorry if it seemed like I added Signed-off on your behalf.
>
> That's OK, I didn't think you did it maliciously :)
>
> > I thought the Signed-off also concerns the ones, who participated in
> > the patch preparation. Since you suggested the design of the change,
> > I've decided to put your name in the Signed-off tag. What shall I use
> > in this way then?
>
> In this case Suggested-by might have been a good choice. Reported-by is
> also commonly used if someone reported a problem which you created a fix
> for.
>
> Section 13 of Documentation/process/submitting-patches.rst describes
> these tags along with a couple others.
I always thought of these tags as something more like a formality. In fact
this hasn't been my first patchset sent to the kernel e-mailing list.
Although all of the previous ones didn't involve someone else participating
in the changes development, except the reviewers of course. So I do aware
of all the tags mentioned in the doc. But as it turns out I didn't
fully understand their meaning. Main rule: most of the tags should not be
added without the permission, except more or less formal CC and Fixes ones.
Anyway thanks for the advice. Next time I'll be more careful with it.
Regards,
-Sergey
>
> Thanks,
> Paul
Hello Christoph,
On Tue, Jul 10, 2018 at 11:56:31PM -0700, Christoph Hellwig <[email protected]> wrote:
> > + * This is a MIPS specific ioremap variant. ioremap_cacheable_cow
> > + * requests a cachable mapping with CWB attribute enabled.
> > */
> > #define ioremap_cacheable_cow(offset, size) \
> > __ioremap_mode((offset), (size), _CACHE_CACHABLE_COW)
>
> This isn't actually used anywhere in the kernel tree. Please remove it
> as well.
I don't really know whether it is necessary at this point. We discarded the
ioremap_uncached_accelerated() method, since the obvious alternative is now
available: ioremap_wc(). While ioremap_cacheable_cow() hasn't got one.
So if it was up to me, I'd leave it here. Anyway if the subsystem maintainers
think otherwise, I won't refuse to submit a patch with this method removal.
Regards,
-Sergey
On Wed, Jul 11, 2018 at 04:52:10PM +0300, Serge Semin wrote:
> Hello Christoph,
>
> On Tue, Jul 10, 2018 at 11:56:31PM -0700, Christoph Hellwig <[email protected]> wrote:
> > > + * This is a MIPS specific ioremap variant. ioremap_cacheable_cow
> > > + * requests a cachable mapping with CWB attribute enabled.
> > > */
> > > #define ioremap_cacheable_cow(offset, size) \
> > > __ioremap_mode((offset), (size), _CACHE_CACHABLE_COW)
> >
> > This isn't actually used anywhere in the kernel tree. Please remove it
> > as well.
>
> I don't really know whether it is necessary at this point. We discarded the
> ioremap_uncached_accelerated() method, since the obvious alternative is now
> available: ioremap_wc(). While ioremap_cacheable_cow() hasn't got one.
> So if it was up to me, I'd leave it here. Anyway if the subsystem maintainers
> think otherwise, I won't refuse to submit a patch with this method removal.
The function is entirely unused in the kernel tree, please remove it.