On Friday, August 23, 2013 5:36 PM, Pratyush Anand wrote: > On Fri, Aug 23, 2013 at 02:04:20PM +0800, Jingoo Han wrote: [.....] > > +#define MAX_MSI_IRQS 32 > > DW MSI controller can support upto 256. However, 32 seems a practical > choice, as there might not be any system which may use more > than 32. But a comment like as follows can be put here: > > /* > * Maximum number of MSI IRQs can be 256 per controller. But keep > * it 32 as of now. Probably we will never need more than 32. If needed, > * then increment it in multiple of 32. > */ OK. I will add this comment. > > > +#define MAX_MSI_CTRLS 8 > > Why to waste cpu cycles when MAX_MSI_IRQS is 32 only. > #define MAX_MSI_CTRLS (MAX_MSI_IRQS / 32) OK. I will fix it as you guide. > > > + > > +static unsigned int msi_data; > > +static DECLARE_BITMAP(msi_irq_in_use, MAX_MSI_IRQS); > > What if one has more than one RC. > There are SOCs which support 3 RCs. > So something like this: > > #define MAX_PCIE_PORT_SUPPORTED 3 > static DECLARE_BITMAP(msi_irq_in_use[MAX_PCIE_PORT_SUPPORTED], > NUM_MSI_IRQS); > static unsigned int *msi_data[MAX_PCIE_PORT_SUPPORTED]; No, there is no need to do it. Without this, we can use 3 RCs by adding a node of 3rd RC to dt file. > > Allocate msi_data using __get_free_pages(GFP_KERNEL, 0)) as Thierry > suggested. OK. I will use '__get_free_pages(GFP_KERNEL, 0))' as Thierry guide. > > int dw_pcie_link_up(struct pcie_port *pp) > > { > > if (pp->ops->link_up) > > @@ -225,6 +432,13 @@ int __init dw_pcie_host_init(struct pcie_port *pp) > > return -EINVAL; > > } > > > > + if (IS_ENABLED(CONFIG_PCI_MSI)) { > > + if (of_property_read_u32(np, "msi-base", &pp->msi_irq_start)) { > > + dev_err(pp->dev, "Failed to parse the number of lanes\n"); > > + return -EINVAL; > > + } > > + } > > + > > What if an implementor want to use irq_domain method for msi_irq_start > allocation? Is it fine to return error if msi-base is not passed from > dt? Sure, I will consider using irq_domain method. > > Also, with the limited knowledge of dt I do not understand one thing, how > would dt understand that you have used 32 msi irqs (MAX_MSI_IRQS)? Sorry, I cannot understand exactly. :-( I will look into it. I really appreciate your comments. :-) Best regards, Jingoo Han -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html