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 devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
