Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1317682 > unrolled thread
| Started by | Michal Simek <michal.simek@xilinx.com> |
|---|---|
| First post | 2016-01-26 11:00 +0100 |
| Last post | 2016-01-27 15:50 +0100 |
| Articles | 4 — 3 participants |
Back to article view | Back to linux.kernel
This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by
below is the oldest one visible, not the original post.
Re: [PATCH V2 3/5] PCI: xilinx: Modifying AXI PCIe Host Bridge driver to work on both Zynq and Microblaze Michal Simek <michal.simek@xilinx.com> - 2016-01-26 11:00 +0100
Re: [PATCH V2 3/5] PCI: xilinx: Modifying AXI PCIe Host Bridge driver to work on both Zynq and Microblaze Arnd Bergmann <arnd@arndb.de> - 2016-01-26 13:20 +0100
Re: [PATCH V2 3/5] PCI: xilinx: Modifying AXI PCIe Host Bridge driver to work on both Zynq and Microblaze Michal Simek <michal.simek@xilinx.com> - 2016-01-26 16:30 +0100
RE: [PATCH V2 3/5] PCI: xilinx: Modifying AXI PCIe Host Bridge driver to work on both Zynq and Microblaze Bharat Kumar Gogada <bharat.kumar.gogada@xilinx.com> - 2016-01-27 15:50 +0100
| From | Michal Simek <michal.simek@xilinx.com> |
|---|---|
| Date | 2016-01-26 11:00 +0100 |
| Subject | Re: [PATCH V2 3/5] PCI: xilinx: Modifying AXI PCIe Host Bridge driver to work on both Zynq and Microblaze |
| Message-ID | <qV8zM-2fZ-11@gated-at.bofh.it> |
On 12.1.2016 23:27, Arnd Bergmann wrote: > On Tuesday 12 January 2016 23:06:11 Bharat Kumar Gogada wrote: >> Modifying Xilinx AXI PCIe Host Bridge Soft IP driver to work on both >> Zynq and Microblaze Architectures. >> With these modifications drivers/pci/host/pcie-xilinx.c, will >> work on both Zynq and Microblaze Architectures. >> >> Signed-off-by: Bharat Kumar Gogada <bharatku@xilinx.com> >> Signed-off-by: Ravi Kiran Gummaluri <rgummal@xilinx.com> > > I think this patch should be split into three, as you are doing three > unrelated things here. > >> --- >> Changes: >> Changed Total number of MSI IRQ count logic according to both architectures. >> Updated MSI assigning functions accordingly to new count. >> Modified irq_domain_add_linear with new MSI IRQ count. >> Added #ifdef to pci_fixup_irqs which is ARM specific API. >> --- >> drivers/pci/host/pcie-xilinx.c | 22 +++++++++++++++++----- >> 1 file changed, 17 insertions(+), 5 deletions(-) >> >> diff --git a/drivers/pci/host/pcie-xilinx.c b/drivers/pci/host/pcie-xilinx.c >> index 3e3757f..1981948 100644 >> --- a/drivers/pci/host/pcie-xilinx.c >> +++ b/drivers/pci/host/pcie-xilinx.c >> @@ -92,7 +92,12 @@ >> #define ECAM_DEV_NUM_SHIFT 12 >> >> /* Number of MSI IRQs */ >> -#define XILINX_NUM_MSI_IRQS 128 >> +#define XILINX_NUM_MSI_IRQS 128 >> +#ifdef CONFIG_ARM >> +#define TOT_NR_IRQS XILINX_NUM_MSI_IRQS >> +#else >> +#define TOT_NR_IRQS (NR_IRQS + XILINX_NUM_MSI_IRQS) >> +#endif > > Something looks wrong here in the microblaze variant. What does NR_IRQS > have to do with it? Arnd: What was the story regarding NR_IRQS? I remember some discussion about it but just forget. Default value in include/asm-generic/irq.h is 64. Current value is 32 because microblaze primary interrupt controller is axi_intc core which has up to 32 input lines. Thanks, Michal
[toc] | [next] | [standalone]
| From | Arnd Bergmann <arnd@arndb.de> |
|---|---|
| Date | 2016-01-26 13:20 +0100 |
| Subject | Re: [PATCH V2 3/5] PCI: xilinx: Modifying AXI PCIe Host Bridge driver to work on both Zynq and Microblaze |
| Message-ID | <qVaLg-3XS-23@gated-at.bofh.it> |
| In reply to | #1317682 |
On Tuesday 26 January 2016 10:59:12 Michal Simek wrote:
> >> diff --git a/drivers/pci/host/pcie-xilinx.c b/drivers/pci/host/pcie-xilinx.c
> >> index 3e3757f..1981948 100644
> >> --- a/drivers/pci/host/pcie-xilinx.c
> >> +++ b/drivers/pci/host/pcie-xilinx.c
> >> @@ -92,7 +92,12 @@
> >> #define ECAM_DEV_NUM_SHIFT 12
> >>
> >> /* Number of MSI IRQs */
> >> -#define XILINX_NUM_MSI_IRQS 128
> >> +#define XILINX_NUM_MSI_IRQS 128
> >> +#ifdef CONFIG_ARM
> >> +#define TOT_NR_IRQS XILINX_NUM_MSI_IRQS
> >> +#else
> >> +#define TOT_NR_IRQS (NR_IRQS + XILINX_NUM_MSI_IRQS)
> >> +#endif
> >
> > Something looks wrong here in the microblaze variant. What does NR_IRQS
> > have to do with it?
>
> Arnd: What was the story regarding NR_IRQS?
> I remember some discussion about it but just forget.
>
> Default value in include/asm-generic/irq.h is 64.
> Current value is 32 because microblaze primary interrupt controller is
> axi_intc core which has up to 32 input lines.
The value in asm-generic is completely arbitrary, it's just something
that happens to work for a number of the simpler architectures.
Traditionally, there is a a fixed NR_IRQS which defines the maximum
number of interrupts that can be used, and each irqchip has a fixed
start offset below that number.
On modern systems, you have CONFIG_SPARSE_IRQ, which lets an irqchip
allocate its own interrupts, without an upper limit. This is more
flexible and avoids preallocating space for all irq_desc instances,
so it saves memory.
This code however doesn't do either of the two on microblaze:
+ irq = pos;
+#ifdef CONFIG_MICROBLAZE
+#define TOT_NR_IRQS (NR_IRQS + XILINX_NUM_MSI_IRQS)
+ irq = XILINX_NUM_MSI_IRQS + pos;
+#endif
+ if (irq < TOT_NR_IRQS)
set_bit(pos, msi_irq_in_use);
So you define XILINX_NUM_MSI_IRQS to mean the number of interrupts
that the xilinx_pcie_port can handle itself, but then pick a number
outside of this range by making the hwirq number something between
XILINX_NUM_MSI_IRQS and (2*XILINX_NUM_MSI_IRQS - 1), and in the
end compare it to (NR_IRQS + XILINX_NUM_MSI_IRQS), which is the
sum of two things that are not related: the total number of interrupts
including the MSIs and the number of MSI.
Arnd
[toc] | [prev] | [next] | [standalone]
| From | Michal Simek <michal.simek@xilinx.com> |
|---|---|
| Date | 2016-01-26 16:30 +0100 |
| Message-ID | <qVdJ9-5Zk-23@gated-at.bofh.it> |
| In reply to | #1317816 |
On 26.1.2016 13:11, Arnd Bergmann wrote: > On Tuesday 26 January 2016 10:59:12 Michal Simek wrote: >>>> diff --git a/drivers/pci/host/pcie-xilinx.c b/drivers/pci/host/pcie-xilinx.c >>>> index 3e3757f..1981948 100644 >>>> --- a/drivers/pci/host/pcie-xilinx.c >>>> +++ b/drivers/pci/host/pcie-xilinx.c >>>> @@ -92,7 +92,12 @@ >>>> #define ECAM_DEV_NUM_SHIFT 12 >>>> >>>> /* Number of MSI IRQs */ >>>> -#define XILINX_NUM_MSI_IRQS 128 >>>> +#define XILINX_NUM_MSI_IRQS 128 >>>> +#ifdef CONFIG_ARM >>>> +#define TOT_NR_IRQS XILINX_NUM_MSI_IRQS >>>> +#else >>>> +#define TOT_NR_IRQS (NR_IRQS + XILINX_NUM_MSI_IRQS) >>>> +#endif >>> >>> Something looks wrong here in the microblaze variant. What does NR_IRQS >>> have to do with it? >> >> Arnd: What was the story regarding NR_IRQS? >> I remember some discussion about it but just forget. >> >> Default value in include/asm-generic/irq.h is 64. >> Current value is 32 because microblaze primary interrupt controller is >> axi_intc core which has up to 32 input lines. > > The value in asm-generic is completely arbitrary, it's just something > that happens to work for a number of the simpler architectures. > > Traditionally, there is a a fixed NR_IRQS which defines the maximum > number of interrupts that can be used, and each irqchip has a fixed > start offset below that number. > > On modern systems, you have CONFIG_SPARSE_IRQ, which lets an irqchip > allocate its own interrupts, without an upper limit. This is more > flexible and avoids preallocating space for all irq_desc instances, > so it saves memory. ok. That was the story. I will look if there is any issue to enable SPARSE_IRQ for Microblaze. I also need to move intc driver out of arch. Thanks, Michal
[toc] | [prev] | [next] | [standalone]
| From | Bharat Kumar Gogada <bharat.kumar.gogada@xilinx.com> |
|---|---|
| Date | 2016-01-27 15:50 +0100 |
| Subject | RE: [PATCH V2 3/5] PCI: xilinx: Modifying AXI PCIe Host Bridge driver to work on both Zynq and Microblaze |
| Message-ID | <qVzzY-4VL-25@gated-at.bofh.it> |
| In reply to | #1317816 |
> > On Tuesday 26 January 2016 10:59:12 Michal Simek wrote: > > >> diff --git a/drivers/pci/host/pcie-xilinx.c > > >> b/drivers/pci/host/pcie-xilinx.c index 3e3757f..1981948 100644 > > >> --- a/drivers/pci/host/pcie-xilinx.c > > >> +++ b/drivers/pci/host/pcie-xilinx.c > > >> @@ -92,7 +92,12 @@ > > >> #define ECAM_DEV_NUM_SHIFT 12 > > >> > > >> /* Number of MSI IRQs */ > > >> -#define XILINX_NUM_MSI_IRQS 128 > > >> +#define XILINX_NUM_MSI_IRQS 128 > > >> +#ifdef CONFIG_ARM > > >> +#define TOT_NR_IRQS XILINX_NUM_MSI_IRQS > > >> +#else > > >> +#define TOT_NR_IRQS (NR_IRQS + XILINX_NUM_MSI_IRQS) > > >> +#endif > > > > > > Something looks wrong here in the microblaze variant. What does > > > NR_IRQS have to do with it? > > > > Arnd: What was the story regarding NR_IRQS? > > I remember some discussion about it but just forget. > > > > Default value in include/asm-generic/irq.h is 64. > > Current value is 32 because microblaze primary interrupt controller is > > axi_intc core which has up to 32 input lines. > > The value in asm-generic is completely arbitrary, it's just something that > happens to work for a number of the simpler architectures. > > Traditionally, there is a a fixed NR_IRQS which defines the maximum number > of interrupts that can be used, and each irqchip has a fixed start offset below > that number. > > On modern systems, you have CONFIG_SPARSE_IRQ, which lets an irqchip > allocate its own interrupts, without an upper limit. This is more flexible and > avoids preallocating space for all irq_desc instances, so it saves memory. > > This code however doesn't do either of the two on microblaze: > > + irq = pos; > +#ifdef CONFIG_MICROBLAZE > +#define TOT_NR_IRQS (NR_IRQS + XILINX_NUM_MSI_IRQS) > + irq = XILINX_NUM_MSI_IRQS + pos; #endif > + if (irq < TOT_NR_IRQS) > set_bit(pos, msi_irq_in_use); > > So you define XILINX_NUM_MSI_IRQS to mean the number of interrupts > that the xilinx_pcie_port can handle itself, but then pick a number > outside of this range by making the hwirq number something between > XILINX_NUM_MSI_IRQS and (2*XILINX_NUM_MSI_IRQS - 1), and in the > end compare it to (NR_IRQS + XILINX_NUM_MSI_IRQS), which is the > sum of two things that are not related: the total number of interrupts > including the MSIs and the number of MSI. > Agreed, I have crosschecked something's which I missed previously, NR_IRQS have nothing to do with the count, and also no need to add, irq = XILINX_NUM_MSI_IRQS + pos; in microblaze, I will remove these checks in next patch series.
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web