* [PATCH V7 0/2] bugfix based on csky linux-next @ 2018-09-18 13:48 Guo Ren 2018-09-18 13:48 ` [PATCH V7 1/2] csky/dma: bugfix dma_sync_for_cpu/device Guo Ren 2018-09-18 13:48 ` [PATCH V7 2/2] csky: remove irq_mapping from smp.c Guo Ren 0 siblings, 2 replies; 9+ messages in thread From: Guo Ren @ 2018-09-18 13:48 UTC (permalink / raw) To: tglx, jason, marc.zyngier, robh+dt, mark.rutland, arnd, robh, sfr Cc: linux-kernel, devicetree, linux-arch, c-sky_gcc_upstream, gnu-csky, green.hu, Guo Ren 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH V7 1/2] csky/dma: bugfix dma_sync_for_cpu/device 2018-09-18 13:48 [PATCH V7 0/2] bugfix based on csky linux-next Guo Ren @ 2018-09-18 13:48 ` Guo Ren 2018-09-24 20:38 ` Arnd Bergmann 2018-09-18 13:48 ` [PATCH V7 2/2] csky: remove irq_mapping from smp.c Guo Ren 1 sibling, 1 reply; 9+ messages in thread From: Guo Ren @ 2018-09-18 13:48 UTC (permalink / raw) To: tglx, jason, marc.zyngier, robh+dt, mark.rutland, arnd, robh, sfr Cc: linux-kernel, devicetree, linux-arch, c-sky_gcc_upstream, gnu-csky, green.hu, Guo Ren 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 <ren_guo@c-sky.com> --- 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 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH V7 1/2] csky/dma: bugfix dma_sync_for_cpu/device 2018-09-18 13:48 ` [PATCH V7 1/2] csky/dma: bugfix dma_sync_for_cpu/device Guo Ren @ 2018-09-24 20:38 ` Arnd Bergmann 2018-09-25 9:46 ` Guo Ren 0 siblings, 1 reply; 9+ messages in thread From: Arnd Bergmann @ 2018-09-24 20:38 UTC (permalink / raw) To: Guo Ren Cc: Thomas Gleixner, Jason Cooper, Marc Zyngier, Rob Herring, Mark Rutland, Rob Herring, Stephen Rothwell, Linux Kernel Mailing List, DTML, linux-arch, c-sky_gcc_upstream, gnu-csky, Greentime Hu On Tue, Sep 18, 2018 at 3:48 PM Guo Ren <ren_guo@c-sky.com> 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 <ren_guo@c-sky.com> 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/20180518215548.GH17671@n2100.armlinux.org.uk/ Arnd ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH V7 1/2] csky/dma: bugfix dma_sync_for_cpu/device 2018-09-24 20:38 ` Arnd Bergmann @ 2018-09-25 9:46 ` Guo Ren 2018-09-25 9:59 ` Arnd Bergmann 0 siblings, 1 reply; 9+ messages in thread From: Guo Ren @ 2018-09-25 9:46 UTC (permalink / raw) To: Arnd Bergmann Cc: Thomas Gleixner, Jason Cooper, Marc Zyngier, Rob Herring, Mark Rutland, Rob Herring, Stephen Rothwell, Linux Kernel Mailing List, DTML, linux-arch, c-sky_gcc_upstream, gnu-csky, Greentime Hu On Mon, Sep 24, 2018 at 10:38:04PM +0200, Arnd Bergmann wrote: > On Tue, Sep 18, 2018 at 3:48 PM Guo Ren <ren_guo@c-sky.com> 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 <ren_guo@c-sky.com> > > 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/20180518215548.GH17671@n2100.armlinux.org.uk/ 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/20180518215548.GH17671@n2100.armlinux.org.uk/ Signed-off-by: Guo Ren <ren_guo@c-sky.com> Best Regards Guo Ren ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH V7 1/2] csky/dma: bugfix dma_sync_for_cpu/device 2018-09-25 9:46 ` Guo Ren @ 2018-09-25 9:59 ` Arnd Bergmann 2018-09-25 11:38 ` Guo Ren 0 siblings, 1 reply; 9+ messages in thread From: Arnd Bergmann @ 2018-09-25 9:59 UTC (permalink / raw) To: Guo Ren Cc: Thomas Gleixner, Jason Cooper, Marc Zyngier, Rob Herring, Mark Rutland, Rob Herring, Stephen Rothwell, Linux Kernel Mailing List, DTML, linux-arch, c-sky_gcc_upstream, gnu-csky, Greentime Hu On Tue, Sep 25, 2018 at 11:46 AM Guo Ren <ren_guo@c-sky.com> 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 <ren_guo@c-sky.com> 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 <ren_guo@c-sky.com> > > > > 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/20180518215548.GH17671@n2100.armlinux.org.uk/ > 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH V7 1/2] csky/dma: bugfix dma_sync_for_cpu/device 2018-09-25 9:59 ` Arnd Bergmann @ 2018-09-25 11:38 ` Guo Ren 0 siblings, 0 replies; 9+ messages in thread From: Guo Ren @ 2018-09-25 11:38 UTC (permalink / raw) To: Arnd Bergmann Cc: Thomas Gleixner, Jason Cooper, Marc Zyngier, Rob Herring, Mark Rutland, Rob Herring, Stephen Rothwell, Linux Kernel Mailing List, DTML, linux-arch, c-sky_gcc_upstream, gnu-csky, Greentime Hu On Tue, Sep 25, 2018 at 11:59:48AM +0200, Arnd Bergmann wrote: > On Tue, Sep 25, 2018 at 11:46 AM Guo Ren <ren_guo@c-sky.com> 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 <ren_guo@c-sky.com> 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 <ren_guo@c-sky.com> > > > > > > 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/20180518215548.GH17671@n2100.armlinux.org.uk/ > > 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH V7 2/2] csky: remove irq_mapping from smp.c. 2018-09-18 13:48 [PATCH V7 0/2] bugfix based on csky linux-next Guo Ren 2018-09-18 13:48 ` [PATCH V7 1/2] csky/dma: bugfix dma_sync_for_cpu/device Guo Ren @ 2018-09-18 13:48 ` Guo Ren 2018-09-24 20:23 ` Arnd Bergmann 1 sibling, 1 reply; 9+ messages in thread From: Guo Ren @ 2018-09-18 13:48 UTC (permalink / raw) To: tglx, jason, marc.zyngier, robh+dt, mark.rutland, arnd, robh, sfr Cc: linux-kernel, devicetree, linux-arch, c-sky_gcc_upstream, gnu-csky, green.hu, Guo Ren - 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 <ren_guo@c-sky.com> --- 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 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH V7 2/2] csky: remove irq_mapping from smp.c. 2018-09-18 13:48 ` [PATCH V7 2/2] csky: remove irq_mapping from smp.c Guo Ren @ 2018-09-24 20:23 ` Arnd Bergmann 2018-09-25 9:17 ` Guo Ren 0 siblings, 1 reply; 9+ messages in thread From: Arnd Bergmann @ 2018-09-24 20:23 UTC (permalink / raw) To: Guo Ren Cc: Thomas Gleixner, Jason Cooper, Marc Zyngier, Rob Herring, Mark Rutland, Rob Herring, Stephen Rothwell, Linux Kernel Mailing List, DTML, linux-arch, c-sky_gcc_upstream, gnu-csky, Greentime Hu On Tue, Sep 18, 2018 at 3:48 PM Guo Ren <ren_guo@c-sky.com> 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 <ren_guo@c-sky.com> 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH V7 2/2] csky: remove irq_mapping from smp.c. 2018-09-24 20:23 ` Arnd Bergmann @ 2018-09-25 9:17 ` Guo Ren 0 siblings, 0 replies; 9+ messages in thread From: Guo Ren @ 2018-09-25 9:17 UTC (permalink / raw) To: Arnd Bergmann Cc: Thomas Gleixner, Jason Cooper, Marc Zyngier, Rob Herring, Mark Rutland, Rob Herring, Stephen Rothwell, Linux Kernel Mailing List, DTML, linux-arch, c-sky_gcc_upstream, gnu-csky, Greentime Hu On Mon, Sep 24, 2018 at 10:23:14PM +0200, Arnd Bergmann wrote: > On Tue, Sep 18, 2018 at 3:48 PM Guo Ren <ren_guo@c-sky.com> 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 <ren_guo@c-sky.com> > > 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2018-09-25 11:39 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-09-18 13:48 [PATCH V7 0/2] bugfix based on csky linux-next Guo Ren 2018-09-18 13:48 ` [PATCH V7 1/2] csky/dma: bugfix dma_sync_for_cpu/device Guo Ren 2018-09-24 20:38 ` Arnd Bergmann 2018-09-25 9:46 ` Guo Ren 2018-09-25 9:59 ` Arnd Bergmann 2018-09-25 11:38 ` Guo Ren 2018-09-18 13:48 ` [PATCH V7 2/2] csky: remove irq_mapping from smp.c Guo Ren 2018-09-24 20:23 ` Arnd Bergmann 2018-09-25 9:17 ` Guo Ren
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).