Because I've sent to Stephen Roth with the linux-4.19-rc3 git-tree for
linux-next, I shouldn't rebase any more.
- Fixup smp.c irq mapping problem found by Marc Zyngier
- Fixup dma_sync_for_cpu/device in dma-mapping.c
Guo Ren (2):
csky/dma: bugfix dma_sync_for_cpu/device
csky: remove irq_mapping from smp.c.
arch/csky/include/asm/smp.h | 4 ++++
arch/csky/kernel/smp.c | 18 +++++++++++++-----
arch/csky/mm/dma-mapping.c | 5 +++--
3 files changed, 20 insertions(+), 7 deletions(-)
--
2.7.4
ref: https://lkml.org/lkml/2018/5/18/1068
map for_cpu for_device unmap
TO_DEV writeback none writeback none
TO_CPU invalidate invalidate* invalidate invalidate*
BIDIR writeback invalidate writeback invalidate
Signed-off-by: Guo Ren <[email protected]>
---
arch/csky/mm/dma-mapping.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/arch/csky/mm/dma-mapping.c b/arch/csky/mm/dma-mapping.c
index 16c2087..30a2041 100644
--- a/arch/csky/mm/dma-mapping.c
+++ b/arch/csky/mm/dma-mapping.c
@@ -217,7 +217,8 @@ void arch_sync_dma_for_device(struct device *dev, phys_addr_t paddr,
break;
case DMA_FROM_DEVICE:
case DMA_BIDIRECTIONAL:
- BUG();
+ dma_wbinv_range(vaddr + offset, vaddr + offset + size);
+ break;
default:
BUG();
}
@@ -240,7 +241,7 @@ void arch_sync_dma_for_cpu(struct device *dev, phys_addr_t paddr,
switch (dir) {
case DMA_TO_DEVICE:
- BUG();
+ break;
case DMA_FROM_DEVICE:
case DMA_BIDIRECTIONAL:
dma_wbinv_range(vaddr + offset, vaddr + offset + size);
--
2.7.4
- remove irq_mapping from smp.c to irq-driver
- Add set_ipi_irq_mapping api to irq-driver
- update asm/smp.h
Signed-off-by: Guo Ren <[email protected]>
---
arch/csky/include/asm/smp.h | 4 ++++
arch/csky/kernel/smp.c | 18 +++++++++++++-----
2 files changed, 17 insertions(+), 5 deletions(-)
diff --git a/arch/csky/include/asm/smp.h b/arch/csky/include/asm/smp.h
index 9a53abf..fed3a5a 100644
--- a/arch/csky/include/asm/smp.h
+++ b/arch/csky/include/asm/smp.h
@@ -7,6 +7,8 @@
#ifdef CONFIG_SMP
+#define IPI_IRQ 15
+
void __init setup_smp(void);
void __init setup_smp_ipi(void);
@@ -19,6 +21,8 @@ void arch_send_call_function_single_ipi(int cpu);
void __init set_send_ipi(void (*func)(const unsigned long *, unsigned long));
+void __init set_ipi_irq_mapping(int (*func)(void));
+
#define raw_smp_processor_id() (current_thread_info()->cpu)
#endif /* CONFIG_SMP */
diff --git a/arch/csky/kernel/smp.c b/arch/csky/kernel/smp.c
index 522c73f..f8343f6 100644
--- a/arch/csky/kernel/smp.c
+++ b/arch/csky/kernel/smp.c
@@ -20,8 +20,6 @@
#include <asm/mmu_context.h>
#include <asm/pgalloc.h>
-#define IPI_IRQ 15
-
static struct {
unsigned long bits ____cacheline_aligned;
} ipi_data[NR_CPUS] __cacheline_aligned;
@@ -121,13 +119,23 @@ void __init enable_smp_ipi(void)
enable_percpu_irq(IPI_IRQ, 0);
}
+static int (*arch_ipi_irq_mapping)(void) = NULL;
+
+void __init set_ipi_irq_mapping(int (*func)(void))
+{
+ if (arch_ipi_irq_mapping)
+ return;
+
+ arch_ipi_irq_mapping = func;
+}
+
void __init setup_smp_ipi(void)
{
- int rc;
+ int rc, irq;
- irq_create_mapping(NULL, IPI_IRQ);
+ irq = arch_ipi_irq_mapping();
- rc = request_percpu_irq(IPI_IRQ, handle_ipi, "IPI Interrupt", &ipi_dummy_dev);
+ rc = request_percpu_irq(irq, handle_ipi, "IPI Interrupt", &ipi_dummy_dev);
if (rc)
panic("%s IRQ request failed\n", __func__);
--
2.7.4
On Tue, Sep 18, 2018 at 3:48 PM Guo Ren <[email protected]> wrote:
>
> - remove irq_mapping from smp.c to irq-driver
> - Add set_ipi_irq_mapping api to irq-driver
> - update asm/smp.h
>
> Signed-off-by: Guo Ren <[email protected]>
The patch seems ok to me, but please try to improve the changelog comments.
Generally speaking, use full English sentences instead of an enumerated list,
and explain what the change is for rather than what you are doing. The text
you have here can easily be derived from looking at the code changes,
but I still don't understand what caused you to change it, and why this
is better than the previous version.
Arnd
On Tue, Sep 18, 2018 at 3:48 PM Guo Ren <[email protected]> wrote:
>
> ref: https://lkml.org/lkml/2018/5/18/1068
>
> map for_cpu for_device unmap
> TO_DEV writeback none writeback none
> TO_CPU invalidate invalidate* invalidate invalidate*
> BIDIR writeback invalidate writeback invalidate
>
> Signed-off-by: Guo Ren <[email protected]>
Same comment as for the other patch: Explain why the original
version is wrong first. When giving a reference to some other
discussion, use the "Link" tag above your Signed-off-by line.
To point to a discussion on lkml, lore.kernel.org is the
recommended archive, so it would become
Link: https://lore.kernel.org/lkml/[email protected]/
Arnd
On Mon, Sep 24, 2018 at 10:23:14PM +0200, Arnd Bergmann wrote:
> On Tue, Sep 18, 2018 at 3:48 PM Guo Ren <[email protected]> wrote:
> >
> > - remove irq_mapping from smp.c to irq-driver
> > - Add set_ipi_irq_mapping api to irq-driver
> > - update asm/smp.h
> >
> > Signed-off-by: Guo Ren <[email protected]>
>
> The patch seems ok to me, but please try to improve the changelog comments.
> Generally speaking, use full English sentences instead of an enumerated list,
> and explain what the change is for rather than what you are doing. The text
Ok, I'll improve it.
> you have here can easily be derived from looking at the code changes,
> but I still don't understand what caused you to change it, and why this
> is better than the previous version.
The changelog comment will change to:
There are some feedbacks from irqchip, and we need to adjust "smp.c & smp.h"
to match the csky_mptimer modification.
- Move IPI_IRQ define into drivers/irqchip/csky_mpintc.c, because it's a
interrupt controller specific.
- Bugfix request_irq with IPI_IRQ, we must use irq_mapping return value not
directly use IPI_IRQ. The modification also involves csky_mpintc.
Best Regards
Guo Ren
On Mon, Sep 24, 2018 at 10:38:04PM +0200, Arnd Bergmann wrote:
> On Tue, Sep 18, 2018 at 3:48 PM Guo Ren <[email protected]> wrote:
> >
> > ref: https://lkml.org/lkml/2018/5/18/1068
> >
> > map for_cpu for_device unmap
> > TO_DEV writeback none writeback none
> > TO_CPU invalidate invalidate* invalidate invalidate*
> > BIDIR writeback invalidate writeback invalidate
> >
> > Signed-off-by: Guo Ren <[email protected]>
>
> Same comment as for the other patch: Explain why the original
> version is wrong first. When giving a reference to some other
> discussion, use the "Link" tag above your Signed-off-by line.
> To point to a discussion on lkml, lore.kernel.org is the
> recommended archive, so it would become
>
> Link: https://lore.kernel.org/lkml/[email protected]/
Ok, I'll improve the comment:
Fixup dma_mapping error in linux-4.19-rc3, and we must implement all
DMA_TO_DEVICE/FROM_DEVICE/BIDIRECTIONAL for both sync_dma_for_device/cpu.
The implementation of arch should follow the following rules:
map for_cpu for_device unmap
TO_DEV writeback none writeback none
TO_CPU invalidate invalidate* invalidate invalidate*
BIDIR writeback invalidate writeback invalidate
Link: https://lore.kernel.org/lkml/[email protected]/
Signed-off-by: Guo Ren <[email protected]>
Best Regards
Guo Ren
On Tue, Sep 25, 2018 at 11:46 AM Guo Ren <[email protected]> wrote:
>
> On Mon, Sep 24, 2018 at 10:38:04PM +0200, Arnd Bergmann wrote:
> > On Tue, Sep 18, 2018 at 3:48 PM Guo Ren <[email protected]> wrote:
> > >
> > > ref: https://lkml.org/lkml/2018/5/18/1068
> > >
> > > map for_cpu for_device unmap
> > > TO_DEV writeback none writeback none
> > > TO_CPU invalidate invalidate* invalidate invalidate*
> > > BIDIR writeback invalidate writeback invalidate
> > >
> > > Signed-off-by: Guo Ren <[email protected]>
> >
> > Same comment as for the other patch: Explain why the original
> > version is wrong first. When giving a reference to some other
> > discussion, use the "Link" tag above your Signed-off-by line.
> > To point to a discussion on lkml, lore.kernel.org is the
> > recommended archive, so it would become
> >
> > Link: https://lore.kernel.org/lkml/[email protected]/
> Ok, I'll improve the comment:
>
> Fixup dma_mapping error in linux-4.19-rc3, and we must implement all
> DMA_TO_DEVICE/FROM_DEVICE/BIDIRECTIONAL for both sync_dma_for_device/cpu.
> The implementation of arch should follow the following rules:
>
> ...
That seems ok, but it's better to start with a description of the problem
rather than the 'Fixup dma_mapping error in linux-4.19-rc3' part.
I would write this like:
| The arch_sync_dma_for_cpu()/arch_sync_dma_for_device()
| implementation is broken for some combinations that end up
| in a BUG() instead of performing the necessary flushes.
|
| The implementation of arch should follow the following rules:
| ...
The imperative 'Fix up dma_mapping error ...' is what belongs
into the subject line, and is ideally revisited at the end of the
changelog comment if you want to add more detail about what
you do.
Arnd
On Tue, Sep 25, 2018 at 11:59:48AM +0200, Arnd Bergmann wrote:
> On Tue, Sep 25, 2018 at 11:46 AM Guo Ren <[email protected]> wrote:
> >
> > On Mon, Sep 24, 2018 at 10:38:04PM +0200, Arnd Bergmann wrote:
> > > On Tue, Sep 18, 2018 at 3:48 PM Guo Ren <[email protected]> wrote:
> > > >
> > > > ref: https://lkml.org/lkml/2018/5/18/1068
> > > >
> > > > map for_cpu for_device unmap
> > > > TO_DEV writeback none writeback none
> > > > TO_CPU invalidate invalidate* invalidate invalidate*
> > > > BIDIR writeback invalidate writeback invalidate
> > > >
> > > > Signed-off-by: Guo Ren <[email protected]>
> > >
> > > Same comment as for the other patch: Explain why the original
> > > version is wrong first. When giving a reference to some other
> > > discussion, use the "Link" tag above your Signed-off-by line.
> > > To point to a discussion on lkml, lore.kernel.org is the
> > > recommended archive, so it would become
> > >
> > > Link: https://lore.kernel.org/lkml/[email protected]/
> > Ok, I'll improve the comment:
> >
> > Fixup dma_mapping error in linux-4.19-rc3, and we must implement all
> > DMA_TO_DEVICE/FROM_DEVICE/BIDIRECTIONAL for both sync_dma_for_device/cpu.
> > The implementation of arch should follow the following rules:
> >
> > ...
>
> That seems ok, but it's better to start with a description of the problem
> rather than the 'Fixup dma_mapping error in linux-4.19-rc3' part.
> I would write this like:
>
> | The arch_sync_dma_for_cpu()/arch_sync_dma_for_device()
> | implementation is broken for some combinations that end up
> | in a BUG() instead of performing the necessary flushes.
> |
> | The implementation of arch should follow the following rules:
> | ...
>
> The imperative 'Fix up dma_mapping error ...' is what belongs
> into the subject line, and is ideally revisited at the end of the
> changelog comment if you want to add more detail about what
> you do.
Nice tips, Thx. I'll continue to improve my comments.
Best Regards
Guo Ren