| Message ID | 20190207110924.30716-7-kishon@ti.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | PCI: DWC/Keystone: MSI configuration cleanup | expand |
On Thu, Feb 07, 2019 at 04:39:21PM +0530, Kishon Vijay Abraham I wrote: > Platforms using Designware IP uses dw_pci_msi_bottom_irq_chip for > configuring the MSI controller logic within the Designware IP. However > certain platforms like Keystone (K2G) which uses Desingware IP has > it's own MSI controller logic. For handling such platforms, > the irqchip ops uses msi_irq_ack, msi_set_irq, msi_clear_irq callback > functions. > > Add support to use different msi_irq_chip with default as > dw_pci_msi_bottom_irq_chip. This is in preparation to get rid off > msi_irq_ack, msi_set_irq, msi_clear_irq and other Keystone specific > dw_pcie_host_ops. This will also help to get rid of get_msi_addr and > get_msi_data ops. > > Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com> > --- > drivers/pci/controller/dwc/pcie-designware-host.c | 5 ++++- > drivers/pci/controller/dwc/pcie-designware.h | 1 + > 2 files changed, 5 insertions(+), 1 deletion(-) > > diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c > index 721d60a5d9e4..042de09b0451 100644 > --- a/drivers/pci/controller/dwc/pcie-designware-host.c > +++ b/drivers/pci/controller/dwc/pcie-designware-host.c > @@ -245,7 +245,7 @@ static int dw_pcie_irq_domain_alloc(struct irq_domain *domain, > > for (i = 0; i < nr_irqs; i++) > irq_domain_set_info(domain, virq + i, bit + i, > - &dw_pci_msi_bottom_irq_chip, > + pp->msi_irq_chip, > pp, handle_edge_irq, > NULL, NULL); > > @@ -277,6 +277,9 @@ int dw_pcie_allocate_domains(struct pcie_port *pp) > struct dw_pcie *pci = to_dw_pcie_from_pp(pp); > struct fwnode_handle *fwnode = of_node_to_fwnode(pci->dev->of_node); > > + if (!pp->msi_irq_chip) > + pp->msi_irq_chip = &dw_pci_msi_bottom_irq_chip; I think it is better to initialize pp->msi_irq_chip outside dw_pcie_allocate_domains(), it makes things clearer. In: dw_pcie_host_init() for dwc or msi_host_init() for platforms with that hook implemented. Is there any gotcha I am missing ? Lorenzo > + > pp->irq_domain = irq_domain_create_linear(fwnode, pp->num_vectors, > &dw_pcie_msi_domain_ops, pp); > if (!pp->irq_domain) { > diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h > index 1f56e6ae34ff..95e0c3c93f48 100644 > --- a/drivers/pci/controller/dwc/pcie-designware.h > +++ b/drivers/pci/controller/dwc/pcie-designware.h > @@ -176,6 +176,7 @@ struct pcie_port { > struct irq_domain *irq_domain; > struct irq_domain *msi_domain; > dma_addr_t msi_data; > + struct irq_chip *msi_irq_chip; > u32 num_vectors; > u32 irq_status[MAX_MSI_CTRLS]; > raw_spinlock_t lock; > -- > 2.17.1 >
On Thu, Feb 07, 2019 at 04:39:21PM +0530, Kishon Vijay Abraham I wrote: > Platforms using Designware IP uses dw_pci_msi_bottom_irq_chip for > configuring the MSI controller logic within the Designware IP. However > certain platforms like Keystone (K2G) which uses Desingware IP has > it's own MSI controller logic. For handling such platforms, > the irqchip ops uses msi_irq_ack, msi_set_irq, msi_clear_irq callback > functions. s/IP uses/IP use/ s/Desingware/DesignWare/ s/Designware/DesignWare/ (several) s/IP has/IP have/ s/it's own/their own/ s/ops uses/ops use/ > Add support to use different msi_irq_chip with default as > dw_pci_msi_bottom_irq_chip. This is in preparation to get rid off > msi_irq_ack, msi_set_irq, msi_clear_irq and other Keystone specific > dw_pcie_host_ops. This will also help to get rid of get_msi_addr and > get_msi_data ops. s/rid off/rid of/ > Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com> > --- > drivers/pci/controller/dwc/pcie-designware-host.c | 5 ++++- > drivers/pci/controller/dwc/pcie-designware.h | 1 + > 2 files changed, 5 insertions(+), 1 deletion(-) > > diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c > index 721d60a5d9e4..042de09b0451 100644 > --- a/drivers/pci/controller/dwc/pcie-designware-host.c > +++ b/drivers/pci/controller/dwc/pcie-designware-host.c > @@ -245,7 +245,7 @@ static int dw_pcie_irq_domain_alloc(struct irq_domain *domain, > > for (i = 0; i < nr_irqs; i++) > irq_domain_set_info(domain, virq + i, bit + i, > - &dw_pci_msi_bottom_irq_chip, > + pp->msi_irq_chip, > pp, handle_edge_irq, > NULL, NULL); > > @@ -277,6 +277,9 @@ int dw_pcie_allocate_domains(struct pcie_port *pp) > struct dw_pcie *pci = to_dw_pcie_from_pp(pp); > struct fwnode_handle *fwnode = of_node_to_fwnode(pci->dev->of_node); > > + if (!pp->msi_irq_chip) > + pp->msi_irq_chip = &dw_pci_msi_bottom_irq_chip; > + > pp->irq_domain = irq_domain_create_linear(fwnode, pp->num_vectors, > &dw_pcie_msi_domain_ops, pp); > if (!pp->irq_domain) { > diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h > index 1f56e6ae34ff..95e0c3c93f48 100644 > --- a/drivers/pci/controller/dwc/pcie-designware.h > +++ b/drivers/pci/controller/dwc/pcie-designware.h > @@ -176,6 +176,7 @@ struct pcie_port { > struct irq_domain *irq_domain; > struct irq_domain *msi_domain; > dma_addr_t msi_data; > + struct irq_chip *msi_irq_chip; > u32 num_vectors; > u32 irq_status[MAX_MSI_CTRLS]; > raw_spinlock_t lock; > -- > 2.17.1 >
Hi Lorenzo, On 07/02/19 10:18 PM, Lorenzo Pieralisi wrote: > On Thu, Feb 07, 2019 at 04:39:21PM +0530, Kishon Vijay Abraham I wrote: >> Platforms using Designware IP uses dw_pci_msi_bottom_irq_chip for >> configuring the MSI controller logic within the Designware IP. However >> certain platforms like Keystone (K2G) which uses Desingware IP has >> it's own MSI controller logic. For handling such platforms, >> the irqchip ops uses msi_irq_ack, msi_set_irq, msi_clear_irq callback >> functions. >> >> Add support to use different msi_irq_chip with default as >> dw_pci_msi_bottom_irq_chip. This is in preparation to get rid off >> msi_irq_ack, msi_set_irq, msi_clear_irq and other Keystone specific >> dw_pcie_host_ops. This will also help to get rid of get_msi_addr and >> get_msi_data ops. >> >> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com> >> --- >> drivers/pci/controller/dwc/pcie-designware-host.c | 5 ++++- >> drivers/pci/controller/dwc/pcie-designware.h | 1 + >> 2 files changed, 5 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c >> index 721d60a5d9e4..042de09b0451 100644 >> --- a/drivers/pci/controller/dwc/pcie-designware-host.c >> +++ b/drivers/pci/controller/dwc/pcie-designware-host.c >> @@ -245,7 +245,7 @@ static int dw_pcie_irq_domain_alloc(struct irq_domain *domain, >> >> for (i = 0; i < nr_irqs; i++) >> irq_domain_set_info(domain, virq + i, bit + i, >> - &dw_pci_msi_bottom_irq_chip, >> + pp->msi_irq_chip, >> pp, handle_edge_irq, >> NULL, NULL); >> >> @@ -277,6 +277,9 @@ int dw_pcie_allocate_domains(struct pcie_port *pp) >> struct dw_pcie *pci = to_dw_pcie_from_pp(pp); >> struct fwnode_handle *fwnode = of_node_to_fwnode(pci->dev->of_node); >> >> + if (!pp->msi_irq_chip) >> + pp->msi_irq_chip = &dw_pci_msi_bottom_irq_chip; > > I think it is better to initialize pp->msi_irq_chip outside > dw_pcie_allocate_domains(), it makes things clearer. > > In: > > dw_pcie_host_init() for dwc > > or > > msi_host_init() for platforms with that hook implemented. > > Is there any gotcha I am missing ? I added here only to avoid breaking "git bisect". Next patch adds ks_pcie_msi_irq_chip in msi_host_init of keystone. However till then it has to use dw_pci_msi_bottom_irq_chip. Adding anywhere else in dw_pcie_host_init would mean msi_irq_chip is uninitialized for keystone. Maybe I can add that in the commit log and move it to dw_pcie_host_init? Thanks Kishon
On Fri, Feb 08, 2019 at 10:16:59AM +0530, Kishon Vijay Abraham I wrote: > Hi Lorenzo, > > On 07/02/19 10:18 PM, Lorenzo Pieralisi wrote: > > On Thu, Feb 07, 2019 at 04:39:21PM +0530, Kishon Vijay Abraham I wrote: > >> Platforms using Designware IP uses dw_pci_msi_bottom_irq_chip for > >> configuring the MSI controller logic within the Designware IP. However > >> certain platforms like Keystone (K2G) which uses Desingware IP has > >> it's own MSI controller logic. For handling such platforms, > >> the irqchip ops uses msi_irq_ack, msi_set_irq, msi_clear_irq callback > >> functions. > >> > >> Add support to use different msi_irq_chip with default as > >> dw_pci_msi_bottom_irq_chip. This is in preparation to get rid off > >> msi_irq_ack, msi_set_irq, msi_clear_irq and other Keystone specific > >> dw_pcie_host_ops. This will also help to get rid of get_msi_addr and > >> get_msi_data ops. > >> > >> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com> > >> --- > >> drivers/pci/controller/dwc/pcie-designware-host.c | 5 ++++- > >> drivers/pci/controller/dwc/pcie-designware.h | 1 + > >> 2 files changed, 5 insertions(+), 1 deletion(-) > >> > >> diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c > >> index 721d60a5d9e4..042de09b0451 100644 > >> --- a/drivers/pci/controller/dwc/pcie-designware-host.c > >> +++ b/drivers/pci/controller/dwc/pcie-designware-host.c > >> @@ -245,7 +245,7 @@ static int dw_pcie_irq_domain_alloc(struct irq_domain *domain, > >> > >> for (i = 0; i < nr_irqs; i++) > >> irq_domain_set_info(domain, virq + i, bit + i, > >> - &dw_pci_msi_bottom_irq_chip, > >> + pp->msi_irq_chip, > >> pp, handle_edge_irq, > >> NULL, NULL); > >> > >> @@ -277,6 +277,9 @@ int dw_pcie_allocate_domains(struct pcie_port *pp) > >> struct dw_pcie *pci = to_dw_pcie_from_pp(pp); > >> struct fwnode_handle *fwnode = of_node_to_fwnode(pci->dev->of_node); > >> > >> + if (!pp->msi_irq_chip) > >> + pp->msi_irq_chip = &dw_pci_msi_bottom_irq_chip; > > > > I think it is better to initialize pp->msi_irq_chip outside > > dw_pcie_allocate_domains(), it makes things clearer. > > > > In: > > > > dw_pcie_host_init() for dwc > > > > or > > > > msi_host_init() for platforms with that hook implemented. > > > > Is there any gotcha I am missing ? > > I added here only to avoid breaking "git bisect". Next patch adds > ks_pcie_msi_irq_chip in msi_host_init of keystone. However till then it has to > use dw_pci_msi_bottom_irq_chip. Adding anywhere else in dw_pcie_host_init would > mean msi_irq_chip is uninitialized for keystone. > > Maybe I can add that in the commit log and move it to dw_pcie_host_init? I do not think you need to mention that in the commit log but move the initialization of pp->msi_irq_chip in dw_pcie_host_init() in the next patch, yes. It is correct to keep bisectability, I should have read ahead. Lorenzo
diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c index 721d60a5d9e4..042de09b0451 100644 --- a/drivers/pci/controller/dwc/pcie-designware-host.c +++ b/drivers/pci/controller/dwc/pcie-designware-host.c @@ -245,7 +245,7 @@ static int dw_pcie_irq_domain_alloc(struct irq_domain *domain, for (i = 0; i < nr_irqs; i++) irq_domain_set_info(domain, virq + i, bit + i, - &dw_pci_msi_bottom_irq_chip, + pp->msi_irq_chip, pp, handle_edge_irq, NULL, NULL); @@ -277,6 +277,9 @@ int dw_pcie_allocate_domains(struct pcie_port *pp) struct dw_pcie *pci = to_dw_pcie_from_pp(pp); struct fwnode_handle *fwnode = of_node_to_fwnode(pci->dev->of_node); + if (!pp->msi_irq_chip) + pp->msi_irq_chip = &dw_pci_msi_bottom_irq_chip; + pp->irq_domain = irq_domain_create_linear(fwnode, pp->num_vectors, &dw_pcie_msi_domain_ops, pp); if (!pp->irq_domain) { diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h index 1f56e6ae34ff..95e0c3c93f48 100644 --- a/drivers/pci/controller/dwc/pcie-designware.h +++ b/drivers/pci/controller/dwc/pcie-designware.h @@ -176,6 +176,7 @@ struct pcie_port { struct irq_domain *irq_domain; struct irq_domain *msi_domain; dma_addr_t msi_data; + struct irq_chip *msi_irq_chip; u32 num_vectors; u32 irq_status[MAX_MSI_CTRLS]; raw_spinlock_t lock;
Platforms using Designware IP uses dw_pci_msi_bottom_irq_chip for configuring the MSI controller logic within the Designware IP. However certain platforms like Keystone (K2G) which uses Desingware IP has it's own MSI controller logic. For handling such platforms, the irqchip ops uses msi_irq_ack, msi_set_irq, msi_clear_irq callback functions. Add support to use different msi_irq_chip with default as dw_pci_msi_bottom_irq_chip. This is in preparation to get rid off msi_irq_ack, msi_set_irq, msi_clear_irq and other Keystone specific dw_pcie_host_ops. This will also help to get rid of get_msi_addr and get_msi_data ops. Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com> --- drivers/pci/controller/dwc/pcie-designware-host.c | 5 ++++- drivers/pci/controller/dwc/pcie-designware.h | 1 + 2 files changed, 5 insertions(+), 1 deletion(-)