2022-10-12 10:40:09

by Christophe Leroy

[permalink] [raw]
Subject: [RFC PATCH 0/8] mm: ioremap: Convert architectures to take GENERIC_IOREMAP way (Alternative)

From:

As proposed in the discussion related to your series, here comes an
exemple of how it could be.

I have taken it into ARC and IA64 architectures as an exemple. This is
untested, even not compiled, it is just to illustrated my meaning in the
discussion.

I also added a patch for powerpc architecture, that one in tested with
both pmac32_defconfig and ppc64_le_defconfig.

From my point of view, this different approach provide less churn and
less intellectual disturbance than the way you do it.

Open for discussion.

Baoquan He (5):
hexagon: mm: Convert to GENERIC_IOREMAP
openrisc: mm: remove unneeded early ioremap code
mm: ioremap: allow ARCH to have its own ioremap definition
arc: mm: Convert to GENERIC_IOREMAP
ia64: mm: Convert to GENERIC_IOREMAP

Christophe Leroy (3):
mm/ioremap: Define generic_ioremap_prot() and generic_iounmap()
mm/ioremap: Consider IOREMAP space in generic ioremap
powerpc: mm: Convert to GENERIC_IOREMAP

arch/arc/Kconfig | 1 +
arch/arc/include/asm/io.h | 7 +++---
arch/arc/mm/ioremap.c | 46 +++--------------------------------
arch/hexagon/Kconfig | 1 +
arch/hexagon/include/asm/io.h | 9 +++++--
arch/hexagon/mm/ioremap.c | 44 ---------------------------------
arch/ia64/Kconfig | 1 +
arch/ia64/include/asm/io.h | 11 ++++++---
arch/ia64/mm/ioremap.c | 45 ++++++----------------------------
arch/openrisc/mm/ioremap.c | 22 ++++-------------
arch/powerpc/Kconfig | 1 +
arch/powerpc/include/asm/io.h | 11 ++++++---
arch/powerpc/mm/ioremap.c | 26 +-------------------
arch/powerpc/mm/ioremap_32.c | 25 ++++++++-----------
arch/powerpc/mm/ioremap_64.c | 22 +++++++----------
include/asm-generic/io.h | 7 ++++++
mm/ioremap.c | 33 +++++++++++++++++++------
17 files changed, 98 insertions(+), 214 deletions(-)
delete mode 100644 arch/hexagon/mm/ioremap.c

--
2.37.1


2022-10-12 10:43:18

by Christophe Leroy

[permalink] [raw]
Subject: [RFC PATCH 5/8] arc: mm: Convert to GENERIC_IOREMAP

From: Baoquan He <[email protected]>

By taking GENERIC_IOREMAP method, the generic ioremap_prot() and
iounmap() are visible and available to arch. Arch only needs to
provide implementation of arch_ioremap() or arch_iounmap() if there's
arch specific handling needed in its ioremap() or iounmap(). This
change will simplify implementation by removing duplicated codes with
generic ioremap() and iounmap(), and has the equivalent functioality
as before.

Here, add hooks arch_ioremap() and arch_iounmap() for arc's special
operation when ioremap_prot() and iounmap(). Meanwhile define and
implement arc's own ioremap() because arc has some special handling
in ioremap() than standard ioremap().

Signed-off-by: Baoquan He <[email protected]>
Cc: Vineet Gupta <[email protected]>
Cc: [email protected]
Signed-off-by: Christophe Leroy <[email protected]>
---
arch/arc/Kconfig | 1 +
arch/arc/include/asm/io.h | 7 +++---
arch/arc/mm/ioremap.c | 46 +++------------------------------------
3 files changed, 8 insertions(+), 46 deletions(-)

diff --git a/arch/arc/Kconfig b/arch/arc/Kconfig
index 9e3653253ef2..a08d2abfaf61 100644
--- a/arch/arc/Kconfig
+++ b/arch/arc/Kconfig
@@ -26,6 +26,7 @@ config ARC
select GENERIC_PENDING_IRQ if SMP
select GENERIC_SCHED_CLOCK
select GENERIC_SMP_IDLE_THREAD
+ select GENERIC_IOREMAP
select HAVE_ARCH_KGDB
select HAVE_ARCH_TRACEHOOK
select HAVE_ARCH_TRANSPARENT_HUGEPAGE if ARC_MMU_V4
diff --git a/arch/arc/include/asm/io.h b/arch/arc/include/asm/io.h
index 8f777d6441a5..53b0f1e4f276 100644
--- a/arch/arc/include/asm/io.h
+++ b/arch/arc/include/asm/io.h
@@ -21,8 +21,8 @@
#endif

extern void __iomem *ioremap(phys_addr_t paddr, unsigned long size);
-extern void __iomem *ioremap_prot(phys_addr_t paddr, unsigned long size,
- unsigned long flags);
+#define ioremap ioremap
+#define ioremap_prot ioremap_prot
static inline void __iomem *ioport_map(unsigned long port, unsigned int nr)
{
return (void __iomem *)port;
@@ -32,7 +32,8 @@ static inline void ioport_unmap(void __iomem *addr)
{
}

-extern void iounmap(const void __iomem *addr);
+bool iounmap_allowed(void *addr);
+#define iounmap_allowed iounmap_allowed

/*
* io{read,write}{16,32}be() macros
diff --git a/arch/arc/mm/ioremap.c b/arch/arc/mm/ioremap.c
index 0ee75aca6e10..02b750abccee 100644
--- a/arch/arc/mm/ioremap.c
+++ b/arch/arc/mm/ioremap.c
@@ -25,13 +25,6 @@ static inline bool arc_uncached_addr_space(phys_addr_t paddr)

void __iomem *ioremap(phys_addr_t paddr, unsigned long size)
{
- phys_addr_t end;
-
- /* Don't allow wraparound or zero size */
- end = paddr + size - 1;
- if (!size || (end < paddr))
- return NULL;
-
/*
* If the region is h/w uncached, MMU mapping can be elided as optim
* The cast to u32 is fine as this region can only be inside 4GB
@@ -54,52 +47,19 @@ EXPORT_SYMBOL(ioremap);
void __iomem *ioremap_prot(phys_addr_t paddr, unsigned long size,
unsigned long flags)
{
- unsigned int off;
- unsigned long vaddr;
- struct vm_struct *area;
- phys_addr_t end;
pgprot_t prot = __pgprot(flags);

- /* Don't allow wraparound, zero size */
- end = paddr + size - 1;
- if ((!size) || (end < paddr))
- return NULL;
-
/* An early platform driver might end up here */
if (!slab_is_available())
return NULL;

/* force uncached */
- prot = pgprot_noncached(prot);
-
- /* Mappings have to be page-aligned */
- off = paddr & ~PAGE_MASK;
- paddr &= PAGE_MASK_PHYS;
- size = PAGE_ALIGN(end + 1) - paddr;
-
- /*
- * Ok, go for it..
- */
- area = get_vm_area(size, VM_IOREMAP);
- if (!area)
- return NULL;
- area->phys_addr = paddr;
- vaddr = (unsigned long)area->addr;
- if (ioremap_page_range(vaddr, vaddr + size, paddr, prot)) {
- vunmap((void __force *)vaddr);
- return NULL;
- }
- return (void __iomem *)(off + (char __iomem *)vaddr);
+ return generic_ioremap_prot(paddr, size, pgprot_noncached(prot));
}
EXPORT_SYMBOL(ioremap_prot);

-
-void iounmap(const void __iomem *addr)
+bool iounmap_allowed(void *addr)
{
/* weird double cast to handle phys_addr_t > 32 bits */
- if (arc_uncached_addr_space((phys_addr_t)(u32)addr))
- return;
-
- vfree((void *)(PAGE_MASK & (unsigned long __force)addr));
+ return !arc_uncached_addr_space((phys_addr_t)(u32)addr);
}
-EXPORT_SYMBOL(iounmap);
--
2.37.1

2022-10-12 10:55:06

by Christophe Leroy

[permalink] [raw]
Subject: [RFC PATCH 8/8] powerpc: mm: Convert to GENERIC_IOREMAP

Signed-off-by: Christophe Leroy <[email protected]>
---
arch/powerpc/Kconfig | 1 +
arch/powerpc/include/asm/io.h | 11 ++++++++---
arch/powerpc/mm/ioremap.c | 26 +-------------------------
arch/powerpc/mm/ioremap_32.c | 25 ++++++++++---------------
arch/powerpc/mm/ioremap_64.c | 22 +++++++++-------------
5 files changed, 29 insertions(+), 56 deletions(-)

diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 81c9f895d690..c8704933ae5a 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -179,6 +179,7 @@ config PPC
select GENERIC_CPU_VULNERABILITIES if PPC_BARRIER_NOSPEC
select GENERIC_EARLY_IOREMAP
select GENERIC_GETTIMEOFDAY
+ select GENERIC_IOREMAP
select GENERIC_IRQ_SHOW
select GENERIC_IRQ_SHOW_LEVEL
select GENERIC_PCI_IOMAP if PCI
diff --git a/arch/powerpc/include/asm/io.h b/arch/powerpc/include/asm/io.h
index fc112a91d0c2..3d38f46ade16 100644
--- a/arch/powerpc/include/asm/io.h
+++ b/arch/powerpc/include/asm/io.h
@@ -859,8 +859,8 @@ static inline void iosync(void)
*
*/
extern void __iomem *ioremap(phys_addr_t address, unsigned long size);
-extern void __iomem *ioremap_prot(phys_addr_t address, unsigned long size,
- unsigned long flags);
+#define ioremap ioremap
+#define ioremap_prot ioremap_prot
extern void __iomem *ioremap_wc(phys_addr_t address, unsigned long size);
#define ioremap_wc ioremap_wc

@@ -874,7 +874,12 @@ void __iomem *ioremap_coherent(phys_addr_t address, unsigned long size);
#define ioremap_cache(addr, size) \
ioremap_prot((addr), (size), pgprot_val(PAGE_KERNEL))

-extern void iounmap(volatile void __iomem *addr);
+#ifdef CONFIG_PPC_INDIRECT_MMIO
+#define iounmap iounmap
+#endif
+
+bool iounmap_allowed(void *addr);
+#define iounmap_allowed iounmap_allowed

void __iomem *ioremap_phb(phys_addr_t paddr, unsigned long size);

diff --git a/arch/powerpc/mm/ioremap.c b/arch/powerpc/mm/ioremap.c
index 4f12504fb405..705e8e8ffde4 100644
--- a/arch/powerpc/mm/ioremap.c
+++ b/arch/powerpc/mm/ioremap.c
@@ -41,7 +41,7 @@ void __iomem *ioremap_coherent(phys_addr_t addr, unsigned long size)
return __ioremap_caller(addr, size, prot, caller);
}

-void __iomem *ioremap_prot(phys_addr_t addr, unsigned long size, unsigned long flags)
+void __iomem *ioremap_prot(phys_addr_t addr, size_t size, unsigned long flags)
{
pte_t pte = __pte(flags);
void *caller = __builtin_return_address(0);
@@ -74,27 +74,3 @@ int early_ioremap_range(unsigned long ea, phys_addr_t pa,

return 0;
}
-
-void __iomem *do_ioremap(phys_addr_t pa, phys_addr_t offset, unsigned long size,
- pgprot_t prot, void *caller)
-{
- struct vm_struct *area;
- int ret;
- unsigned long va;
-
- area = __get_vm_area_caller(size, VM_IOREMAP, IOREMAP_START, IOREMAP_END, caller);
- if (area == NULL)
- return NULL;
-
- area->phys_addr = pa;
- va = (unsigned long)area->addr;
-
- ret = ioremap_page_range(va, va + size, pa, prot);
- if (!ret)
- return (void __iomem *)area->addr + offset;
-
- vunmap_range(va, va + size);
- free_vm_area(area);
-
- return NULL;
-}
diff --git a/arch/powerpc/mm/ioremap_32.c b/arch/powerpc/mm/ioremap_32.c
index 9d13143b8be4..e9a60ddfd6b3 100644
--- a/arch/powerpc/mm/ioremap_32.c
+++ b/arch/powerpc/mm/ioremap_32.c
@@ -21,6 +21,13 @@ __ioremap_caller(phys_addr_t addr, unsigned long size, pgprot_t prot, void *call
phys_addr_t p, offset;
int err;

+ /*
+ * If the address lies within the first 16 MB, assume it's in ISA
+ * memory space
+ */
+ if (addr < SZ_16M)
+ addr += _ISA_MEM_BASE;
+
/*
* Choose an address to map it to.
* Once the vmalloc system is running, we use it.
@@ -31,13 +38,6 @@ __ioremap_caller(phys_addr_t addr, unsigned long size, pgprot_t prot, void *call
offset = addr & ~PAGE_MASK;
size = PAGE_ALIGN(addr + size) - p;

- /*
- * If the address lies within the first 16 MB, assume it's in ISA
- * memory space
- */
- if (p < 16 * 1024 * 1024)
- p += _ISA_MEM_BASE;
-
#ifndef CONFIG_CRASH_DUMP
/*
* Don't allow anybody to remap normal RAM that we're using.
@@ -63,7 +63,7 @@ __ioremap_caller(phys_addr_t addr, unsigned long size, pgprot_t prot, void *call
return (void __iomem *)v + offset;

if (slab_is_available())
- return do_ioremap(p, offset, size, prot, caller);
+ return generic_ioremap_prot(addr, size, prot);

/*
* Should check if it is a candidate for a BAT mapping
@@ -78,16 +78,11 @@ __ioremap_caller(phys_addr_t addr, unsigned long size, pgprot_t prot, void *call
return (void __iomem *)ioremap_bot + offset;
}

-void iounmap(volatile void __iomem *addr)
+bool iounmap_allowed(void *addr)
{
/*
* If mapped by BATs then there is nothing to do.
* Calling vfree() generates a benign warning.
*/
- if (v_block_mapped((unsigned long)addr))
- return;
-
- if (addr > high_memory && (unsigned long)addr < ioremap_bot)
- vunmap((void *)(PAGE_MASK & (unsigned long)addr));
+ return !v_block_mapped((unsigned long)addr);
}
-EXPORT_SYMBOL(iounmap);
diff --git a/arch/powerpc/mm/ioremap_64.c b/arch/powerpc/mm/ioremap_64.c
index 3acece00b33e..a319d40253e5 100644
--- a/arch/powerpc/mm/ioremap_64.c
+++ b/arch/powerpc/mm/ioremap_64.c
@@ -29,7 +29,7 @@ void __iomem *__ioremap_caller(phys_addr_t addr, unsigned long size,
return NULL;

if (slab_is_available())
- return do_ioremap(paligned, offset, size, prot, caller);
+ return generic_ioremap_prot(addr, size, prot);

pr_warn("ioremap() called early from %pS. Use early_ioremap() instead\n", caller);

@@ -47,19 +47,15 @@ void __iomem *__ioremap_caller(phys_addr_t addr, unsigned long size,
* Unmap an IO region and remove it from vmalloc'd list.
* Access to IO memory should be serialized by driver.
*/
-void iounmap(volatile void __iomem *token)
+bool iounmap_allowed(void *addr)
{
- void *addr;
-
- if (!slab_is_available())
- return;
-
- addr = (void *)((unsigned long __force)PCI_FIX_ADDR(token) & PAGE_MASK);
+ return slab_is_available();
+}

- if ((unsigned long)addr < ioremap_bot) {
- pr_warn("Attempt to iounmap early bolted mapping at 0x%p\n", addr);
- return;
- }
- vunmap(addr);
+#ifdef CONFIG_PPC_INDIRECT_MMIO
+void iounmap(volatile void __iomem *token)
+{
+ generic_iounmap(PCI_FIX_ADDR(token));
}
EXPORT_SYMBOL(iounmap);
+#endif
--
2.37.1

2022-10-12 10:57:57

by Christophe Leroy

[permalink] [raw]
Subject: [RFC PATCH 6/8] ia64: mm: Convert to GENERIC_IOREMAP

From: Baoquan He <[email protected]>

By taking GENERIC_IOREMAP method, the generic ioremap_prot() and
iounmap() are visible and available to arch. Arch only needs to
provide implementation of arch_ioremap() or arch_iounmap() if there's
arch specific handling needed in its ioremap() or iounmap(). This
change will simplify implementation by removing duplicated codes with
generic ioremap() and iounmap(), and has the equivalent functioality
as before.

Here add hooks arch_ioremap() and arch_iounmap() for ia64's special
operation when ioremap() and iounmap(), then ioremap_cache() is
converted to use ioremap_prot() from GENERIC_IOREMAP.

The old ioremap_uc() is kept and add its macro definittion.

Signed-off-by: Baoquan He <[email protected]>
Cc: [email protected]
Signed-off-by: Christophe Leroy <[email protected]>
---
arch/ia64/Kconfig | 1 +
arch/ia64/include/asm/io.h | 11 ++++++----
arch/ia64/mm/ioremap.c | 45 ++++++--------------------------------
3 files changed, 15 insertions(+), 42 deletions(-)

diff --git a/arch/ia64/Kconfig b/arch/ia64/Kconfig
index 26ac8ea15a9e..1ca18be5dc30 100644
--- a/arch/ia64/Kconfig
+++ b/arch/ia64/Kconfig
@@ -45,6 +45,7 @@ config IA64
select GENERIC_IRQ_LEGACY
select ARCH_HAVE_NMI_SAFE_CMPXCHG
select GENERIC_IOMAP
+ select GENERIC_IOREMAP
select GENERIC_SMP_IDLE_THREAD
select ARCH_TASK_STRUCT_ON_STACK
select ARCH_TASK_STRUCT_ALLOCATOR
diff --git a/arch/ia64/include/asm/io.h b/arch/ia64/include/asm/io.h
index ce66dfc0e719..06e006d82d81 100644
--- a/arch/ia64/include/asm/io.h
+++ b/arch/ia64/include/asm/io.h
@@ -247,18 +247,21 @@ static inline void outsl(unsigned long port, const void *src,

# ifdef __KERNEL__

-extern void __iomem * ioremap(unsigned long offset, unsigned long size);
+#define _PAGE_IOREMAP pgprot_val(PAGE_KERNEL)
+
+void __iomem *ioremap_prot(unsigned long phys_addr, unsigned long size,
+ unsigned long flags);
extern void __iomem * ioremap_uc(unsigned long offset, unsigned long size);
-extern void iounmap (volatile void __iomem *addr);
static inline void __iomem * ioremap_cache (unsigned long phys_addr, unsigned long size)
{
return ioremap(phys_addr, size);
}
-#define ioremap ioremap
+#define ioremap_prot ioremap_prot
#define ioremap_cache ioremap_cache
#define ioremap_uc ioremap_uc
-#define iounmap iounmap

+bool iounmap_allowed(void *addr);
+#define iounmap_allowed iounmap_allowed
/*
* String version of IO memory access ops:
*/
diff --git a/arch/ia64/mm/ioremap.c b/arch/ia64/mm/ioremap.c
index 55fd3eb753ff..0d43efe528e4 100644
--- a/arch/ia64/mm/ioremap.c
+++ b/arch/ia64/mm/ioremap.c
@@ -29,13 +29,9 @@ early_ioremap (unsigned long phys_addr, unsigned long size)
return __ioremap_uc(phys_addr);
}

-void __iomem *
-ioremap (unsigned long phys_addr, unsigned long size)
+void __iomem *ioremap_prot(unsigned long phys_addr, unsigned long size,
+ unsigned long flags)
{
- void __iomem *addr;
- struct vm_struct *area;
- unsigned long offset;
- pgprot_t prot;
u64 attr;
unsigned long gran_base, gran_size;
unsigned long page_base;
@@ -68,36 +64,12 @@ ioremap (unsigned long phys_addr, unsigned long size)
*/
page_base = phys_addr & PAGE_MASK;
size = PAGE_ALIGN(phys_addr + size) - page_base;
- if (efi_mem_attribute(page_base, size) & EFI_MEMORY_WB) {
- prot = PAGE_KERNEL;
-
- /*
- * Mappings have to be page-aligned
- */
- offset = phys_addr & ~PAGE_MASK;
- phys_addr &= PAGE_MASK;
-
- /*
- * Ok, go for it..
- */
- area = get_vm_area(size, VM_IOREMAP);
- if (!area)
- return NULL;
-
- area->phys_addr = phys_addr;
- addr = (void __iomem *) area->addr;
- if (ioremap_page_range((unsigned long) addr,
- (unsigned long) addr + size, phys_addr, prot)) {
- vunmap((void __force *) addr);
- return NULL;
- }
-
- return (void __iomem *) (offset + (char __iomem *)addr);
- }
+ if (efi_mem_attribute(page_base, size) & EFI_MEMORY_WB)
+ return generic_ioremap_prot(phys_addr, size, __pgprot(flags));

return __ioremap_uc(phys_addr);
}
-EXPORT_SYMBOL(ioremap);
+EXPORT_SYMBOL(ioremap_prot);

void __iomem *
ioremap_uc(unsigned long phys_addr, unsigned long size)
@@ -114,10 +86,7 @@ early_iounmap (volatile void __iomem *addr, unsigned long size)
{
}

-void
-iounmap (volatile void __iomem *addr)
+bool iounmap_allowed(void *addr)
{
- if (REGION_NUMBER(addr) == RGN_GATE)
- vunmap((void *) ((unsigned long) addr & PAGE_MASK));
+ return REGION_NUMBER(addr) == RGN_GATE;
}
-EXPORT_SYMBOL(iounmap);
--
2.37.1

2022-10-17 00:58:40

by Baoquan He

[permalink] [raw]
Subject: Re: [RFC PATCH 0/8] mm: ioremap: Convert architectures to take GENERIC_IOREMAP way (Alternative)

Hi Christophe,

On 10/12/22 at 12:09pm, Christophe Leroy wrote:
> From:
>
> As proposed in the discussion related to your series, here comes an
> exemple of how it could be.
>
> I have taken it into ARC and IA64 architectures as an exemple. This is
> untested, even not compiled, it is just to illustrated my meaning in the
> discussion.
>
> I also added a patch for powerpc architecture, that one in tested with
> both pmac32_defconfig and ppc64_le_defconfig.
>
> From my point of view, this different approach provide less churn and
> less intellectual disturbance than the way you do it.

Yes, I agree, and admire your insistence on the thing you think right or
better. Learn from you.

When you suggested this in my v2 post, I made a draft patch at below link
according to your suggestion to request people to review. What worried
me is that I am not sure it's ignored or disliked after one week of
waiting.

https://lore.kernel.org/all/YwtND%2FL8xD+ViN3r@MiWiFi-R3L-srv/#related

Up to now, seems people don't oppose this generic_ioremap_prot() way, we
can take it. So what's your plan? You want me to continue with your
patches wrapped in, or I can leave it to you if you want to take over?

Thanks
Baoquan

>
> Baoquan He (5):
> hexagon: mm: Convert to GENERIC_IOREMAP
> openrisc: mm: remove unneeded early ioremap code
> mm: ioremap: allow ARCH to have its own ioremap definition
> arc: mm: Convert to GENERIC_IOREMAP
> ia64: mm: Convert to GENERIC_IOREMAP
>
> Christophe Leroy (3):
> mm/ioremap: Define generic_ioremap_prot() and generic_iounmap()
> mm/ioremap: Consider IOREMAP space in generic ioremap
> powerpc: mm: Convert to GENERIC_IOREMAP
>
> arch/arc/Kconfig | 1 +
> arch/arc/include/asm/io.h | 7 +++---
> arch/arc/mm/ioremap.c | 46 +++--------------------------------
> arch/hexagon/Kconfig | 1 +
> arch/hexagon/include/asm/io.h | 9 +++++--
> arch/hexagon/mm/ioremap.c | 44 ---------------------------------
> arch/ia64/Kconfig | 1 +
> arch/ia64/include/asm/io.h | 11 ++++++---
> arch/ia64/mm/ioremap.c | 45 ++++++----------------------------
> arch/openrisc/mm/ioremap.c | 22 ++++-------------
> arch/powerpc/Kconfig | 1 +
> arch/powerpc/include/asm/io.h | 11 ++++++---
> arch/powerpc/mm/ioremap.c | 26 +-------------------
> arch/powerpc/mm/ioremap_32.c | 25 ++++++++-----------
> arch/powerpc/mm/ioremap_64.c | 22 +++++++----------
> include/asm-generic/io.h | 7 ++++++
> mm/ioremap.c | 33 +++++++++++++++++++------
> 17 files changed, 98 insertions(+), 214 deletions(-)
> delete mode 100644 arch/hexagon/mm/ioremap.c
>
> --
> 2.37.1
>

2022-10-17 17:09:43

by Christophe Leroy

[permalink] [raw]
Subject: Re: [RFC PATCH 0/8] mm: ioremap: Convert architectures to take GENERIC_IOREMAP way (Alternative)

Hi Baoquan,

Le 17/10/2022 à 02:37, Baoquan He a écrit :
> Hi Christophe,
>
> On 10/12/22 at 12:09pm, Christophe Leroy wrote:
>> From:
>>
>> As proposed in the discussion related to your series, here comes an
>> exemple of how it could be.
>>
>> I have taken it into ARC and IA64 architectures as an exemple. This is
>> untested, even not compiled, it is just to illustrated my meaning in the
>> discussion.
>>
>> I also added a patch for powerpc architecture, that one in tested with
>> both pmac32_defconfig and ppc64_le_defconfig.
>>
>> From my point of view, this different approach provide less churn and
>> less intellectual disturbance than the way you do it.
>
> Yes, I agree, and admire your insistence on the thing you think right or
> better. Learn from you.
>
> When you suggested this in my v2 post, I made a draft patch at below link
> according to your suggestion to request people to review. What worried
> me is that I am not sure it's ignored or disliked after one week of
> waiting.
>
> https://lore.kernel.org/all/YwtND%2FL8xD+ViN3r@MiWiFi-R3L-srv/#related
>
> Up to now, seems people don't oppose this generic_ioremap_prot() way, we
> can take it. So what's your plan? You want me to continue with your
> patches wrapped in, or I can leave it to you if you want to take over?

I don't plan to steal your work. If you feel confortable with my
proposal, feel free to continue with it and amplify it. You have done
most of the job, you have a clear view of all subtilities in the
different architectures, so please continue, I don't plan to take over
the good work you've done until now.

The only purpose of my series was to illustrate my comments and convince
myself it was a possible way, nothing more.

Thanks
Christophe

>
> Thanks
> Baoquan
>
>>
>> Baoquan He (5):
>> hexagon: mm: Convert to GENERIC_IOREMAP
>> openrisc: mm: remove unneeded early ioremap code
>> mm: ioremap: allow ARCH to have its own ioremap definition
>> arc: mm: Convert to GENERIC_IOREMAP
>> ia64: mm: Convert to GENERIC_IOREMAP
>>
>> Christophe Leroy (3):
>> mm/ioremap: Define generic_ioremap_prot() and generic_iounmap()
>> mm/ioremap: Consider IOREMAP space in generic ioremap
>> powerpc: mm: Convert to GENERIC_IOREMAP
>>
>> arch/arc/Kconfig | 1 +
>> arch/arc/include/asm/io.h | 7 +++---
>> arch/arc/mm/ioremap.c | 46 +++--------------------------------
>> arch/hexagon/Kconfig | 1 +
>> arch/hexagon/include/asm/io.h | 9 +++++--
>> arch/hexagon/mm/ioremap.c | 44 ---------------------------------
>> arch/ia64/Kconfig | 1 +
>> arch/ia64/include/asm/io.h | 11 ++++++---
>> arch/ia64/mm/ioremap.c | 45 ++++++----------------------------
>> arch/openrisc/mm/ioremap.c | 22 ++++-------------
>> arch/powerpc/Kconfig | 1 +
>> arch/powerpc/include/asm/io.h | 11 ++++++---
>> arch/powerpc/mm/ioremap.c | 26 +-------------------
>> arch/powerpc/mm/ioremap_32.c | 25 ++++++++-----------
>> arch/powerpc/mm/ioremap_64.c | 22 +++++++----------
>> include/asm-generic/io.h | 7 ++++++
>> mm/ioremap.c | 33 +++++++++++++++++++------
>> 17 files changed, 98 insertions(+), 214 deletions(-)
>> delete mode 100644 arch/hexagon/mm/ioremap.c
>>
>> --
>> 2.37.1
>>
>

2022-10-19 00:36:42

by Baoquan He

[permalink] [raw]
Subject: Re: [RFC PATCH 0/8] mm: ioremap: Convert architectures to take GENERIC_IOREMAP way (Alternative)

On 10/17/22 at 05:06pm, Christophe Leroy wrote:
> Hi Baoquan,
>
> Le 17/10/2022 ? 02:37, Baoquan He a ?crit?:
> > Hi Christophe,
> >
> > On 10/12/22 at 12:09pm, Christophe Leroy wrote:
> >> From:
> >>
> >> As proposed in the discussion related to your series, here comes an
> >> exemple of how it could be.
> >>
> >> I have taken it into ARC and IA64 architectures as an exemple. This is
> >> untested, even not compiled, it is just to illustrated my meaning in the
> >> discussion.
> >>
> >> I also added a patch for powerpc architecture, that one in tested with
> >> both pmac32_defconfig and ppc64_le_defconfig.
> >>
> >> From my point of view, this different approach provide less churn and
> >> less intellectual disturbance than the way you do it.
> >
> > Yes, I agree, and admire your insistence on the thing you think right or
> > better. Learn from you.
> >
> > When you suggested this in my v2 post, I made a draft patch at below link
> > according to your suggestion to request people to review. What worried
> > me is that I am not sure it's ignored or disliked after one week of
> > waiting.
> >
> > https://lore.kernel.org/all/YwtND%2FL8xD+ViN3r@MiWiFi-R3L-srv/#related
> >
> > Up to now, seems people don't oppose this generic_ioremap_prot() way, we
> > can take it. So what's your plan? You want me to continue with your
> > patches wrapped in, or I can leave it to you if you want to take over?
>
> I don't plan to steal your work. If you feel confortable with my
> proposal, feel free to continue with it and amplify it. You have done
> most of the job, you have a clear view of all subtilities in the
> different architectures, so please continue, I don't plan to take over
> the good work you've done until now.
>
> The only purpose of my series was to illustrate my comments and convince
> myself it was a possible way, nothing more.

Thanks a lot for all these you have done, I will post another version with
the introducing generic_ioremap_prot() way you suggesed.