Re: [PATCH 2/2] mips: mm: Discard ioremap_uncached_accelerated() method
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 > 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.
Re: [PATCH 2/2] mips: mm: Discard ioremap_uncached_accelerated() method
Hello Christoph, On Tue, Jul 10, 2018 at 11:56:31PM -0700, Christoph Hellwig 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
Re: [PATCH 2/2] mips: mm: Discard ioremap_uncached_accelerated() method
Paul, On Tue, Jul 10, 2018 at 02:04:15PM -0700, Paul Burton 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 > > 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
Re: [PATCH 2/2] mips: mm: Discard ioremap_uncached_accelerated() method
> + * 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.
Re: [PATCH 2/2] mips: mm: Discard ioremap_uncached_accelerated() method
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 > 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
Re: [PATCH 2/2] mips: mm: Discard ioremap_uncached_accelerated() method
On Tue, Jul 10, 2018 at 10:59:40AM -0700, Paul Burton 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 > > wrote: > > > On Mon, Jul 9, 2018 at 3:57 PM Serge Semin > > > 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 > > > > Singed-off-by: Paul Burton > > > > > > 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
Re: [PATCH 2/2] mips: mm: Discard ioremap_uncached_accelerated() method
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 > wrote: > > On Mon, Jul 9, 2018 at 3:57 PM Serge Semin 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 > > > Singed-off-by: Paul Burton > > > > 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
Re: [PATCH 2/2] mips: mm: Discard ioremap_uncached_accelerated() method
On Tue, Jul 10, 2018 at 09:15:17AM +0200, Mathieu Malaterre wrote: > ' > On Mon, Jul 9, 2018 at 3:57 PM Serge Semin 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 > > Singed-off-by: Paul Burton > > 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 > > Cc: Ralf Baechle > > Cc: linux-m...@linux-mips.org > > Cc: sta...@vger.kernel.org > > --- > > 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 > > > >
Re: [PATCH 2/2] mips: mm: Discard ioremap_uncached_accelerated() method
' On Mon, Jul 9, 2018 at 3:57 PM Serge Semin 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 > Singed-off-by: Paul Burton nit: 'Signed' (on both patches) > Cc: James Hogan > Cc: Ralf Baechle > Cc: linux-m...@linux-mips.org > Cc: sta...@vger.kernel.org > --- > 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 > >
[PATCH 2/2] mips: mm: Discard ioremap_uncached_accelerated() method
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 Singed-off-by: Paul Burton Cc: James Hogan Cc: Ralf Baechle Cc: linux-m...@linux-mips.org Cc: sta...@vger.kernel.org --- 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