Re: [PATCH v4 05/19] irqchip: add nps Internal and external irqchips
Hi Marc,
Please respond to Vineet last email.
I wish to close the IPI handling within my patch set.
Regards,
Noam
From: Vineet Gupta
Sent: Monday, January 25, 2016 3:08:34 PM
To: Marc Zyngier; Noam Camus; linux-snps-arc@lists.infradead.org
Cc: linux-ker...@vger.kernel.org; Chris Metcalf; daniel.lezc...@linaro.org;
Thomas Gleixner; Jason Cooper
Subject: Re: [PATCH v4 05/19] irqchip: add nps Internal and external irqchips
Hi Marc,
On Friday 18 December 2015 10:01 PM, Marc Zyngier wrote:
> On 18/12/15 14:29, Noam Camus wrote:
>>> From: Marc Zyngier [mailto:marc.zyng...@arm.com]
>>> Sent: Friday, December 18, 2015 1:21 PM
>>
I need this for my per CPU irqs such timer and IPI which do not come
from some external device but from CPUs. For these IRQs I am calling
to irq_create_mapping() from my platform at arch/arc and at that point
I got no irqdomain and using irq_find_host() is not good since I got
no device_node (at most I can have DT root).
>>
>>> That's a problem. You should never do that for your timer (doing a
>>> request_irq will do the right thing, and that's what your timer driver
>>> already does).
>>
>> Please be more specific, from all that I wrote what is the problem?
>
> Calling irq_create_mapping out of the blue like you do it here:
>
> +static void eznps_init_per_cpu(int cpu)
> +{
> + /* Create mapping for all per cpu IRQs */
> + if (cpu == 0) {
> + irq_create_mapping(NULL, TIMER0_IRQ);
> + irq_create_mapping(NULL, IPI_IRQ);
> + }
>
> is simply not acceptable.
I understand but... see below
>>> As for initializing your IPIs, they are usually outside of the IRQ
>>> space, so you should handle them separately (and get your irqchip
>>> to initialize them).
>> I am handling all my IRQs within same irqchip, which is the only one
>> I have. So I am not sure what you expect here. Please be more
>> elaborate.
>
> Do not create a mapping for IPIs. Full stop. Handle them independently
> from your normal IRQs.
why not ? IPI is a hardware construct afterall.
Anyhow I looked in arch/arm and do_IPI/handle_IPI are the handlers. do_IPI is
called from asm, irq-gic.c calls handle_IPI. I can't seem to find an explicit
request_irq / request_percpu_irq for IPI irq ?
...
>> Note that I am working with ARC (seem alike) here and we do not
>> define CONFIG_HANDLE_DOMAIN_IRQ and do not implement
>> set_handle_irq().
>>
>> So for ARC this reverse mapping is something we can leave without
>> (maybe because we are kind of a legacy domain).
>
> Yeah, I just located the crap: arch_do_IRQ() happily takes a hwirq (the
> vector number), and uses that as a Linux IRQ. This looks a lot like ARM
> pre-DT, about 10 years ago.
>
> Well, time to meet the 21st century. If you intend to use DT, please fix
> your arch port. Otherwise, just hardcode everything in your platform and
> don't pretend to support device tree.
Following works for me, hopefully it is closer to 21st century code :-)
--->
>From 619eb5179d865140a723dd524d0e42fbf234b53b Mon Sep 17 00:00:00 2001
From: Vineet Gupta
Date: Fri, 1 Jan 2016 15:12:54 +0530
Subject: [PATCH] ARC: [intc-*] Do a domain lookup in primary handler for hwirq
-> linux virq
The primary interrupt handler arch_do_IRQ() was passing hwirq as linux
virq to core code. This was fragile and worked so far as we only had
legacy/linear
domains.
This came out of a rant by Marc Zyngier.
http://lists.infradead.org/pipermail/linux-snps-arc/2015-December/000298.html
Cc: Marc Zyngier
Cc: Thomas Gleixner
Cc: Noam Camus
Signed-off-by: Vineet Gupta
---
arch/arc/Kconfig | 1 +
arch/arc/kernel/intc-arcv2.c | 9 ++---
arch/arc/kernel/intc-compact.c | 10 ++
arch/arc/kernel/irq.c | 9 ++---
4 files changed, 15 insertions(+), 14 deletions(-)
diff --git a/arch/arc/Kconfig b/arch/arc/Kconfig
index 6312f607932f..576f1c40ba75 100644
--- a/arch/arc/Kconfig
+++ b/arch/arc/Kconfig
@@ -31,6 +31,7 @@ config ARC
select HAVE_MOD_ARCH_SPECIFIC if ARC_DW2_UNWIND
select HAVE_OPROFILE
select HAVE_PERF_EVENTS
+ select HANDLE_DOMAIN_IRQ
select IRQ_DOMAIN
select MODULES_USE_ELF_RELA
select NO_BOOTMEM
diff --git a/arch/arc/kernel/intc-arcv2.c b/arch/arc/kernel/intc-arcv2.c
index 0394f9f61b46..cede73b50d31 100644
--- a/arch/arc/kernel/intc-arcv2.c
+++ b/arch/arc/kernel/intc-arcv2.c
@@ -130,21 +130,24 @@ static const struct irq_domain_ops arcv2_irq_ops = {
.map = arcv2_irq_map,
};
-static struct irq_domain *root_domain;
static int __init
init_onchip_IRQ(struct device_node *intc, struct device_node *parent)
{
+ struct irq_domain *root_domain;
+
if (parent)
panic("DeviceTree incore intc not a root irq controller\n");
root_domain = irq_domain_add_legacy(intc, NR_CPU_IRQS, 0, 0,
&arcv2_irq_ops, NULL);
-
if (!root_doma
Re: [PATCH v6 2/2] add new platform driver for PCI RC
Hi Joao,
This is getting pretty close. I have a few comments below. This is a
new driver, with no chance of breaking anything else, so I think we
can still get it in for v4.5.
Bjorn
On Thu, Jan 14, 2016 at 11:04:33AM +, Joao Pinto wrote:
> This patch adds a new driver that will be the reference platform driver
> for all PCI RC IP Protoyping Kits based on ARC SDP.
> This patch is composed by:
>
> -Changes to pcie-designware driver add a function that enables the
> feature of starting the LTSSM (Link Train Status State) used by the
> new driver
> -MAINTAINERS file was updated to include the new driver
> -Documentation/devicetree/bindings/pci was updated to include the new
> driver documentation
> -New driver called pcie-snpsdev
>
> Signed-off-by: Joao Pinto
> ---
> Change v5 -> v6:
> - Nothing changed (just to keep up with patch set version).
> Change v4 -> v5:
> - Nothing changed (just to keep up with patch set version).
> Changes v3 -> v4 (Bjorn Helgaas):
> - ARCH dependencies were added to the drivers/pci/host/kconfig for the
> PCIE_SNPSDEV.
> Changes v2 -> v3 (Bjorn Helgaas):
> - link init stuff was moved to a snpsdev_pcie_establish_link() function in
> pcie-snpsdev
> - pcie-snpsdev driver declaration was changed to be more
> standard (Bjorn Helgaas)
> - pcie-designware' dw_pcie_link_retrain() now use standard registers from
> pci-regs.h (Bjorn Helgaas)
> - pcie-snpsdev.txt was complemented with more info (Mark Rutland)
> Changes v1 -> v2 (Bjorn Helgaas):
> - Fixups snpsdev_pcie_fixup_bridge() and snpsdev_pcie_fixup_res() were removed
> from the driver (these functions were for specific tests only and not usefull
> in mainline)
> - Driver' comments were reviewed (fix Typos and excessive comments removal)
> - Removed unnecessary definitions in the driver source (PCIE_PHY_CTRL and
> PCIE_PHY_STAT)
>
> .../devicetree/bindings/pci/pcie-snpsdev.txt | 33 +++
> MAINTAINERS| 7 +
> drivers/pci/host/Kconfig | 8 +
> drivers/pci/host/Makefile | 1 +
> drivers/pci/host/pcie-designware.c | 9 +
> drivers/pci/host/pcie-designware.h | 1 +
> drivers/pci/host/pcie-snpsdev.c| 286
> +
> 7 files changed, 345 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/pci/pcie-snpsdev.txt
> create mode 100644 drivers/pci/host/pcie-snpsdev.c
>
> diff --git a/Documentation/devicetree/bindings/pci/pcie-snpsdev.txt
> b/Documentation/devicetree/bindings/pci/pcie-snpsdev.txt
> new file mode 100644
> index 000..cae548b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pci/pcie-snpsdev.txt
> @@ -0,0 +1,33 @@
> +Synopsys PCI RC IP Prototyping Kit
> +--
> +
> +This is the reference platform driver to be used in the Synopsys PCI Root
> +Complex IP Prototyping Kit.
> +
> +Required properties:
> +- compatible: set to "snps,pcie-snpsdev";
> +- reg: base address and length of the pcie controller registers.
> +- #address-cells: set to <3>
> +- #size-cells: set to <2>
> +- device_type: set to "pci"
> +- ranges: ranges for the PCI memory and I/O regions.
> +- interrupts: one interrupt source for MSI interrupts, followed by interrupt
> + source for hardware related interrupts.
> +- #interrupt-cells: set to <1>
> +- num-lanes: set to <1>;
> +
> +Example configuration:
> +
> + pcie: pcie@0xd000 {
> + compatible = "snps,pcie-snpsdev";
> + reg = <0xd000 0x1000>;
> + #address-cells = <3>;
> + #size-cells = <2>;
> + device_type = "pci";
> + ranges = <0x0800 0 0xd000 0xd000 0 0x2000>,
> + <0x8100 0 0x 0xde00 0 0x0001>,
> + <0x8200 0 0xd040 0xd040 0 0x0d00>;
> + interrupts = <25>, <24>;
> + #interrupt-cells = <1>;
> + num-lanes = <1>;
> + };
Can we get an ack from the DT guys for this?
Is "snpsdev" an already-established abbreviation? The "dev" part
seems obvious and maybe could go without saying. This would look
nicer if we could just use "synopsis" everywhere you have "snpsdev" --
DT compatible string, filename, function names, etc.
> diff --git a/MAINTAINERS b/MAINTAINERS
> index e9caa4b..d2e4506 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -8230,6 +8230,13 @@ S: Maintained
> F: Documentation/devicetree/bindings/pci/hisilicon-pcie.txt
> F: drivers/pci/host/pcie-hisi.c
>
> +PCI DRIVER FOR SYNOPSYS PROTOTYPING DEVICE
> +M: Joao Pinto
> +L: linux-...@vger.kernel.org
> +S: Maintained
> +F: Documentation/devicetree/bindings/pci/pcie-snpsdev.txt
> +F: drivers/pci/host/pcie-snpsdev.c
> +
> PCMCIA SUBSYSTEM
> P: Linux PCMCIA Team
> L: linux-pcm...@lists.infradead.org
> diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig
> index f131
Re: [PATCH v6 1/2] PCI support added to ARC
Hi Joao,
Trivial nits below; I wouldn't even mention them if you weren't making
a few updates to the other patch.
On Thu, Jan 14, 2016 at 11:04:32AM +, Joao Pinto wrote:
> This patch adds PCI support to ARC and updates drivers/pci Makefile
> enabling the ARC arch to use the generic PCI setup functions.
>
> Signed-off-by: Joao Pinto
> ---
> Change v5 -> v6 (Vineet Gupta):
> - Removed EXPORT_SYMBOL(pcibios_fixup_bus) from pcibios.c
> Change v4 -> v5 (Vineet Gupta):
> - pcibios.c unused functions, variables and includes were removed
> - ioremap.c has no need for changes (no patch is necessary)
> - pci.h unused functions and variables were removed
> Change v3 -> v4:
> - Nothing changed (just to keep up with patch set version).
> Change v2 -> v3 (Bjorn Helgaas):
> - arch/arc/kernel/pcibios.c unused functions were removed and also the
> arch/arc/include/asm/mach/pci.h was removed because was no longer necessary
> Change v1 -> v2:
> - In arch/arc/Kconfig, the new menu entry (Bus Configuration) was moved to the
> slot between sourcing of drivers/Kconfig and fs/Kconfig (Vineet Gupta)
> - In arch/arc/plat-axs10x/Kconfig the "select MIGHT_HAVE_PCI" option was
> placed
> in order as suggested (Vineet Gupta)
> - ioport_map() and ioport_unmap() were static inlined and included in
> the io.h (Vineet Gupta)
> - pcibios_min_io and pcibios_min_mem declaration moved to
> pcibios.c (Vineet Gupta)
> - pr_err() replaced by dev_err() in pcibios_enable_device() (Bjorn Helgaas)
> - string simplified in pcibios_enable_device() (Vineet Gupta)
> - pci_host_bridge_window structure was replaced by resource_entry structure,
> and
> list_for_each_entry() for resource_list_for_each_entry() in pcibios.c
>
> arch/arc/Kconfig | 23 +++
> arch/arc/include/asm/dma.h | 5 +
> arch/arc/include/asm/io.h| 9 +
> arch/arc/include/asm/pci.h | 31 +++
> arch/arc/kernel/Makefile | 1 +
> arch/arc/kernel/pcibios.c| 23
> arch/arc/plat-axs10x/Kconfig | 1 +
> drivers/pci/Makefile | 1 +
> 8 files changed, 95 insertions(+)
> create mode 100644 arch/arc/include/asm/pci.h
> create mode 100644 arch/arc/kernel/pcibios.c
>
> diff --git a/arch/arc/Kconfig b/arch/arc/Kconfig
> index 2c2ac3f..98b32c1 100644
> --- a/arch/arc/Kconfig
> +++ b/arch/arc/Kconfig
> @@ -19,6 +19,7 @@ config ARC
> select GENERIC_FIND_FIRST_BIT
> # for now, we don't need GENERIC_IRQ_PROBE, CONFIG_GENERIC_IRQ_CHIP
> select GENERIC_IRQ_SHOW
> + select GENERIC_PCI_IOMAP
> select GENERIC_PENDING_IRQ if SMP
> select GENERIC_SMP_IDLE_THREAD
> select HAVE_ARCH_KGDB
> @@ -39,6 +40,9 @@ config ARC
> select PERF_USE_VMALLOC
> select HAVE_DEBUG_STACKOVERFLOW
>
> +config MIGHT_HAVE_PCI
> + bool
> +
> config TRACE_IRQFLAGS_SUPPORT
> def_bool y
>
> @@ -570,6 +574,25 @@ endmenu # "ARC Architecture Configuration"
> source "mm/Kconfig"
> source "net/Kconfig"
> source "drivers/Kconfig"
> +
> +menu "Bus Support"
> +
> +config PCI
> + bool "PCI support" if MIGHT_HAVE_PCI
> + help
> + PCI is the name of a bus system, i.e. the way the CPU talks to the
> other stuff inside
> + your box.Find out if your board/platform have PCI.
> + Note: PCIE support for Synopsys Device will be available only when
> + HAPS DX is configured with PCIE RC bitmap. If you have PCI, say Y,
> otherwise N.
> +
> +config PCI_SYSCALL
> + def_bool PCI
> +
> +source "drivers/pci/Kconfig"
> +source "drivers/pci/pcie/Kconfig"
> +
> +endmenu
> +
> source "fs/Kconfig"
> source "arch/arc/Kconfig.debug"
> source "security/Kconfig"
> diff --git a/arch/arc/include/asm/dma.h b/arch/arc/include/asm/dma.h
> index ca7c451..37942fa 100644
> --- a/arch/arc/include/asm/dma.h
> +++ b/arch/arc/include/asm/dma.h
> @@ -10,5 +10,10 @@
> #define ASM_ARC_DMA_H
>
> #define MAX_DMA_ADDRESS 0xC000
> +#ifdef CONFIG_PCI
> +extern int isa_dma_bridge_buggy;
> +#else
> +#define isa_dma_bridge_buggy(0)
No parens needed around "0".
> +#endif
>
> #endif
> diff --git a/arch/arc/include/asm/io.h b/arch/arc/include/asm/io.h
> index 694ece8..947bf0c 100644
> --- a/arch/arc/include/asm/io.h
> +++ b/arch/arc/include/asm/io.h
> @@ -16,6 +16,15 @@
> extern void __iomem *ioremap(unsigned long physaddr, unsigned long size);
> extern void __iomem *ioremap_prot(phys_addr_t offset, unsigned long size,
> unsigned long flags);
> +static inline void __iomem *ioport_map(unsigned long port, unsigned int nr)
> +{
> + return (void __iomem *)port;
> +}
> +
> +static inline void ioport_unmap(void __iomem *addr)
> +{
> +}
> +
> extern void iounmap(const void __iomem *addr);
>
> #define ioremap_nocache(phy, sz) ioremap(phy, sz)
> diff --git a/arch/arc/include/asm/pci.h b/arch/arc/include/asm/pci.h
> new file mode 100644
> index 000..2f091e1
> --- /dev/null
> +++ b/arch/arc/include/
