| Message ID | 20231011110514.107528-1-minda.chen@starfivetech.com |
|---|---|
| Headers | show |
| Series | Refactoring Microchip PCIe driver and add StarFive PCIe | expand |
On 2023/10/11 19:04, Minda Chen wrote: > This patchset final purpose is add PCIe driver for StarFive JH7110 SoC. > JH7110 using PLDA XpressRICH PCIe IP. Microchip PolarFire Using the > same IP and have commit their codes, which are mixed with PLDA > controller codes and Microchip platform codes. > > For re-use the PLDA controller codes, I request refactoring microchip > codes, move PLDA common codes to PLDA files. > Desigware and Cadence is good example for refactoring codes. > > ---------------------------------------------------------- > The refactoring patches total number is 17,(patch 1-17) > which do NOT contain changing logic of codes. > > These patches just contain three type basic operations. > (rename, modify codes to support starfive platform, and moving to common file) > If these patched are all be reviewed. They can be accepted first. > > Refactoring patches can be devided to different groups > 1. (patch 1- 3 is the prepare work of refactoring) > patch1 is move PLDA XpressRICH PCIe host common properties dt-binding > docs from microchip,pcie-host.yaml > patch2 is move PolarFire codes to PLDA directory. > patch3 is move PLDA IP register macros to plda-pcie.h > > 2. (patch4 - 6 is processing and re-use PCIe host instance) > patch4 is add bridge_addr field to PCIe host instance. > patch5 is rename data structure in microchip codes. > patch6 is moving two data structures to head file > > 3. (patch 7 - 9 are for re-use two PCIe setup function) > patch7 is rename two setup functions in microchip codes, prepare to move > to common file. > patch8 is change the arguments of plda_pcie_setup_iomems() > patch9 is move the two setup functions to common file pcie-plda-host.c > > 4.(patch 10 - 17 are for re-use interupt processing codes) > patch10 add a plda default event handler function. > patch11 is rename the IRQ related functions, prepare to move to > pcie-plda-host.c > patch 12 - 16 is modify the interrupt event codes, preparing for support starfive > and microchip two platforms. > patch17 is move IRQ related functions to pcie-plda-host.c > > ------------------------------------------------------------ > The remainder patches (patch 18 -22) are not refactoring patch. > They are for adding StarFive codes and dont modify the microchip's > codes. > > patch18 is set plda_event_handler to static. > patch19 is Add PLDA event interrupt codes and IRQ domain ops. > patch20 is add StarFive JH7110 PCIe dt-binding doc. > patch21 is add StarFive JH7110 Soc PCIe codes. > patch22 is Starfive dts config > > This patchset is base on v6.6-rc5 > Hi Kzysztof(K.W) Could you please take time to review this series patches? Thanks. Hi Conor The patch 4- 6 is split from previous version patch. You have added your review tag to that patch. May I add your review tag to patch 4 -6 ? And linux-riscv do NOT run compile test to this patch set.
On Wed, Oct 11, 2023 at 07:04:56PM +0800, Minda Chen wrote: > For bridge address base is common PLDA field, Add this > to struct mc_pcie first. > > INTx and MSI codes interrupts codes will get the bridge base > address from port->bridge_addr. For these codes will be > changed to common codes. axi_base_addr is Microchip its own > data. > > Signed-off-by: Minda Chen <minda.chen@starfivetech.com> Reviewed-by: Conor Dooley <conor.dooley@microchip.com> Thanks, Conor. > --- > .../pci/controller/plda/pcie-microchip-host.c | 23 ++++++++----------- > 1 file changed, 9 insertions(+), 14 deletions(-) > > diff --git a/drivers/pci/controller/plda/pcie-microchip-host.c b/drivers/pci/controller/plda/pcie-microchip-host.c > index a34ec6aad4be..60870ee1f1c9 100644 > --- a/drivers/pci/controller/plda/pcie-microchip-host.c > +++ b/drivers/pci/controller/plda/pcie-microchip-host.c > @@ -195,6 +195,7 @@ struct mc_pcie { > struct irq_domain *event_domain; > raw_spinlock_t lock; > struct mc_msi msi; > + void __iomem *bridge_addr; > }; > > struct cause { > @@ -339,8 +340,7 @@ static void mc_handle_msi(struct irq_desc *desc) > struct irq_chip *chip = irq_desc_get_chip(desc); > struct device *dev = port->dev; > struct mc_msi *msi = &port->msi; > - void __iomem *bridge_base_addr = > - port->axi_base_addr + MC_PCIE_BRIDGE_ADDR; > + void __iomem *bridge_base_addr = port->bridge_addr; > unsigned long status; > u32 bit; > int ret; > @@ -365,8 +365,7 @@ static void mc_handle_msi(struct irq_desc *desc) > static void mc_msi_bottom_irq_ack(struct irq_data *data) > { > struct mc_pcie *port = irq_data_get_irq_chip_data(data); > - void __iomem *bridge_base_addr = > - port->axi_base_addr + MC_PCIE_BRIDGE_ADDR; > + void __iomem *bridge_base_addr = port->bridge_addr; > u32 bitpos = data->hwirq; > > writel_relaxed(BIT(bitpos), bridge_base_addr + ISTATUS_MSI); > @@ -488,8 +487,7 @@ static void mc_handle_intx(struct irq_desc *desc) > struct mc_pcie *port = irq_desc_get_handler_data(desc); > struct irq_chip *chip = irq_desc_get_chip(desc); > struct device *dev = port->dev; > - void __iomem *bridge_base_addr = > - port->axi_base_addr + MC_PCIE_BRIDGE_ADDR; > + void __iomem *bridge_base_addr = port->bridge_addr; > unsigned long status; > u32 bit; > int ret; > @@ -514,8 +512,7 @@ static void mc_handle_intx(struct irq_desc *desc) > static void mc_ack_intx_irq(struct irq_data *data) > { > struct mc_pcie *port = irq_data_get_irq_chip_data(data); > - void __iomem *bridge_base_addr = > - port->axi_base_addr + MC_PCIE_BRIDGE_ADDR; > + void __iomem *bridge_base_addr = port->bridge_addr; > u32 mask = BIT(data->hwirq + PM_MSI_INT_INTX_SHIFT); > > writel_relaxed(mask, bridge_base_addr + ISTATUS_LOCAL); > @@ -524,8 +521,7 @@ static void mc_ack_intx_irq(struct irq_data *data) > static void mc_mask_intx_irq(struct irq_data *data) > { > struct mc_pcie *port = irq_data_get_irq_chip_data(data); > - void __iomem *bridge_base_addr = > - port->axi_base_addr + MC_PCIE_BRIDGE_ADDR; > + void __iomem *bridge_base_addr = port->bridge_addr; > unsigned long flags; > u32 mask = BIT(data->hwirq + PM_MSI_INT_INTX_SHIFT); > u32 val; > @@ -540,8 +536,7 @@ static void mc_mask_intx_irq(struct irq_data *data) > static void mc_unmask_intx_irq(struct irq_data *data) > { > struct mc_pcie *port = irq_data_get_irq_chip_data(data); > - void __iomem *bridge_base_addr = > - port->axi_base_addr + MC_PCIE_BRIDGE_ADDR; > + void __iomem *bridge_base_addr = port->bridge_addr; > unsigned long flags; > u32 mask = BIT(data->hwirq + PM_MSI_INT_INTX_SHIFT); > u32 val; > @@ -896,8 +891,7 @@ static void mc_pcie_setup_window(void __iomem *bridge_base_addr, u32 index, > static int mc_pcie_setup_windows(struct platform_device *pdev, > struct mc_pcie *port) > { > - void __iomem *bridge_base_addr = > - port->axi_base_addr + MC_PCIE_BRIDGE_ADDR; > + void __iomem *bridge_base_addr = port->bridge_addr; > struct pci_host_bridge *bridge = platform_get_drvdata(pdev); > struct resource_entry *entry; > u64 pci_addr; > @@ -1081,6 +1075,7 @@ static int mc_host_probe(struct platform_device *pdev) > mc_disable_interrupts(port); > > bridge_base_addr = port->axi_base_addr + MC_PCIE_BRIDGE_ADDR; > + port->bridge_addr = bridge_base_addr; > > /* Allow enabling MSI by disabling MSI-X */ > val = readl(bridge_base_addr + PCIE_PCI_IRQ_DW0); > -- > 2.17.1 > >
Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
Thanks,
Conor.
On Wed, Oct 11, 2023 at 07:04:58PM +0800, Minda Chen wrote: > Move the common data structures definition to head file for nit: "to a header file so that these data structures" > these data structure can be re-used. > > Signed-off-by: Minda Chen <minda.chen@starfivetech.com> Reviewed-by: Conor Dooley <conor.dooley@microchip.com> Thanks, Conor. > --- > .../pci/controller/plda/pcie-microchip-host.c | 20 ------------------ > drivers/pci/controller/plda/pcie-plda.h | 21 +++++++++++++++++++ > 2 files changed, 21 insertions(+), 20 deletions(-) > > diff --git a/drivers/pci/controller/plda/pcie-microchip-host.c b/drivers/pci/controller/plda/pcie-microchip-host.c > index 3dc4d4ca9d0c..261147a0a446 100644 > --- a/drivers/pci/controller/plda/pcie-microchip-host.c > +++ b/drivers/pci/controller/plda/pcie-microchip-host.c > @@ -21,9 +21,6 @@ > #include "../../pci.h" > #include "pcie-plda.h" > > -/* Number of MSI IRQs */ > -#define PLDA_MAX_NUM_MSI_IRQS 32 > - > /* PCIe Bridge Phy and Controller Phy offsets */ > #define MC_PCIE1_BRIDGE_ADDR 0x00008000u > #define MC_PCIE1_CTRL_ADDR 0x0000a000u > @@ -179,23 +176,6 @@ struct event_map { > u32 event_bit; > }; > > -struct plda_msi { > - struct mutex lock; /* Protect used bitmap */ > - struct irq_domain *msi_domain; > - struct irq_domain *dev_domain; > - u32 num_vectors; > - u64 vector_phy; > - DECLARE_BITMAP(used, PLDA_MAX_NUM_MSI_IRQS); > -}; > - > -struct plda_pcie_rp { > - struct device *dev; > - struct irq_domain *intx_domain; > - struct irq_domain *event_domain; > - raw_spinlock_t lock; > - struct plda_msi msi; > - void __iomem *bridge_addr; > -}; > > struct mc_pcie { > struct plda_pcie_rp plda; > diff --git a/drivers/pci/controller/plda/pcie-plda.h b/drivers/pci/controller/plda/pcie-plda.h > index 727fc54312c9..363fcbbaf6ec 100644 > --- a/drivers/pci/controller/plda/pcie-plda.h > +++ b/drivers/pci/controller/plda/pcie-plda.h > @@ -6,6 +6,9 @@ > #ifndef _PCIE_PLDA_H > #define _PCIE_PLDA_H > > +/* Number of MSI IRQs */ > +#define PLDA_MAX_NUM_MSI_IRQS 32 > + > /* PCIe Bridge Phy Regs */ > #define PCIE_PCI_IRQ_DW0 0xa8 > #define MSIX_CAP_MASK BIT(31) > @@ -99,4 +102,22 @@ > #define EVENT_PM_MSI_INT_SYS_ERR 12 > #define NUM_PLDA_EVENTS 13 > > +struct plda_msi { > + struct mutex lock; /* Protect used bitmap */ > + struct irq_domain *msi_domain; > + struct irq_domain *dev_domain; > + u32 num_vectors; > + u64 vector_phy; > + DECLARE_BITMAP(used, PLDA_MAX_NUM_MSI_IRQS); > +}; > + > +struct plda_pcie_rp { > + struct device *dev; > + struct irq_domain *intx_domain; > + struct irq_domain *event_domain; > + raw_spinlock_t lock; > + struct plda_msi msi; > + void __iomem *bridge_addr; > +}; > + > #endif > -- > 2.17.1 >
On Wed, Oct 11, 2023 at 07:05:02PM +0800, Minda Chen wrote: > Add PLDA default event IRQ handler. > This is first patch of refactoring IRQ handling codes. > The handler function will be referenced by later patch. > > Signed-off-by: Minda Chen <minda.chen@starfivetech.com> > --- > drivers/pci/controller/plda/pcie-plda-host.c | 5 +++++ > drivers/pci/controller/plda/pcie-plda.h | 1 + Dunno what hte PCI maintainers take is on this kind of thing, but this patch adds dead code, as there is no user for it until the follow-on patch you mention. Did someone ask you to split this out? Cheers, Conor. > 2 files changed, 6 insertions(+) > > diff --git a/drivers/pci/controller/plda/pcie-plda-host.c b/drivers/pci/controller/plda/pcie-plda-host.c > index 19131181897f..21ca6b460f5e 100644 > --- a/drivers/pci/controller/plda/pcie-plda-host.c > +++ b/drivers/pci/controller/plda/pcie-plda-host.c > @@ -18,6 +18,11 @@ > > #include "pcie-plda.h" > > +irqreturn_t plda_event_handler(int irq, void *dev_id) > +{ > + return IRQ_HANDLED; > +} > + > void plda_pcie_setup_window(void __iomem *bridge_base_addr, u32 index, > phys_addr_t axi_addr, phys_addr_t pci_addr, > size_t size) > diff --git a/drivers/pci/controller/plda/pcie-plda.h b/drivers/pci/controller/plda/pcie-plda.h > index 3deefd35fa5a..7ff7ff44c980 100644 > --- a/drivers/pci/controller/plda/pcie-plda.h > +++ b/drivers/pci/controller/plda/pcie-plda.h > @@ -120,6 +120,7 @@ struct plda_pcie_rp { > void __iomem *bridge_addr; > }; > > +irqreturn_t plda_event_handler(int irq, void *dev_id); > void plda_pcie_setup_window(void __iomem *bridge_base_addr, u32 index, > phys_addr_t axi_addr, phys_addr_t pci_addr, > size_t size); > -- > 2.17.1 >
On 2023/10/18 18:44, Conor Dooley wrote: > On Wed, Oct 11, 2023 at 07:05:02PM +0800, Minda Chen wrote: >> Add PLDA default event IRQ handler. >> This is first patch of refactoring IRQ handling codes. >> The handler function will be referenced by later patch. >> >> Signed-off-by: Minda Chen <minda.chen@starfivetech.com> >> --- >> drivers/pci/controller/plda/pcie-plda-host.c | 5 +++++ >> drivers/pci/controller/plda/pcie-plda.h | 1 + > > Dunno what hte PCI maintainers take is on this kind of thing, but this > patch adds dead code, as there is no user for it until the follow-on > patch you mention. Did someone ask you to split this out? > > Cheers, > Conor. > No one. Previous this patch contain other codes. I modify this incorrect. I will squash this with other patch. Thanks. >> 2 files changed, 6 insertions(+) >> >> diff --git a/drivers/pci/controller/plda/pcie-plda-host.c b/drivers/pci/controller/plda/pcie-plda-host.c >> index 19131181897f..21ca6b460f5e 100644 >> --- a/drivers/pci/controller/plda/pcie-plda-host.c >> +++ b/drivers/pci/controller/plda/pcie-plda-host.c >> @@ -18,6 +18,11 @@ >> >> #include "pcie-plda.h" >> >> +irqreturn_t plda_event_handler(int irq, void *dev_id) >> +{ >> + return IRQ_HANDLED; >> +} >> + >> void plda_pcie_setup_window(void __iomem *bridge_base_addr, u32 index, >> phys_addr_t axi_addr, phys_addr_t pci_addr, >> size_t size) >> diff --git a/drivers/pci/controller/plda/pcie-plda.h b/drivers/pci/controller/plda/pcie-plda.h >> index 3deefd35fa5a..7ff7ff44c980 100644 >> --- a/drivers/pci/controller/plda/pcie-plda.h >> +++ b/drivers/pci/controller/plda/pcie-plda.h >> @@ -120,6 +120,7 @@ struct plda_pcie_rp { >> void __iomem *bridge_addr; >> }; >> >> +irqreturn_t plda_event_handler(int irq, void *dev_id); >> void plda_pcie_setup_window(void __iomem *bridge_base_addr, u32 index, >> phys_addr_t axi_addr, phys_addr_t pci_addr, >> size_t size); >> -- >> 2.17.1 >>
On Wed, Oct 11, 2023 at 07:05:05PM +0800, Minda Chen wrote: > PolarFire implements specific interrupts besides PLDA local > interrupt and register their interrupt symbol name. > (Total 28 > interrupts while PLDA contain 13 local interrupts). and > interrupt to event mapping is different. Nit: drop the ()s & the first . Daire would have to speak to why this is the case, but these commit message appears to better explain why the patch is needed. Acked-by: Conor Dooley <conor.dooley@microchip.com> > So add a callback function to support different IRQ register > function. > > Signed-off-by: Minda Chen <minda.chen@starfivetech.com> > --- > .../pci/controller/plda/pcie-microchip-host.c | 25 ++++++++++++++++--- > drivers/pci/controller/plda/pcie-plda.h | 5 ++++ > 2 files changed, 26 insertions(+), 4 deletions(-) > > diff --git a/drivers/pci/controller/plda/pcie-microchip-host.c b/drivers/pci/controller/plda/pcie-microchip-host.c > index 1799455989ac..104332603e25 100644 > --- a/drivers/pci/controller/plda/pcie-microchip-host.c > +++ b/drivers/pci/controller/plda/pcie-microchip-host.c > @@ -799,6 +799,17 @@ static int mc_pcie_init_clks(struct device *dev) > return 0; > } > > +static int mc_request_event_irq(struct plda_pcie_rp *plda, int event_irq, > + int event) > +{ > + return devm_request_irq(plda->dev, event_irq, mc_event_handler, > + 0, event_cause[event].sym, plda); > +} > + > +static const struct plda_event mc_event = { > + .request_event_irq = mc_request_event_irq, nit: these spaces for alignment look pointless when there's just one element. Cheers, Conor. > +}; > + > static int plda_pcie_init_irq_domains(struct plda_pcie_rp *port) > { > struct device *dev = port->dev; > @@ -898,7 +909,9 @@ static void mc_disable_interrupts(struct mc_pcie *port) > writel_relaxed(GENMASK(31, 0), bridge_base_addr + ISTATUS_HOST); > } > > -static int plda_init_interrupts(struct platform_device *pdev, struct plda_pcie_rp *port) > +static int plda_init_interrupts(struct platform_device *pdev, > + struct plda_pcie_rp *port, > + const struct plda_event *event) > { > struct device *dev = &pdev->dev; > int irq; > @@ -922,8 +935,12 @@ static int plda_init_interrupts(struct platform_device *pdev, struct plda_pcie_r > return -ENXIO; > } > > - ret = devm_request_irq(dev, event_irq, mc_event_handler, > - 0, event_cause[i].sym, port); > + if (event->request_event_irq) > + ret = event->request_event_irq(port, event_irq, i); > + else > + ret = devm_request_irq(dev, event_irq, plda_event_handler, > + 0, NULL, port); > + > if (ret) { > dev_err(dev, "failed to request IRQ %d\n", event_irq); > return ret; > @@ -977,7 +994,7 @@ static int mc_platform_init(struct pci_config_window *cfg) > return ret; > > /* Address translation is up; safe to enable interrupts */ > - ret = plda_init_interrupts(pdev, &port->plda); > + ret = plda_init_interrupts(pdev, &port->plda, &mc_event); > if (ret) > return ret; > > diff --git a/drivers/pci/controller/plda/pcie-plda.h b/drivers/pci/controller/plda/pcie-plda.h > index b439160448b1..907ad40f3188 100644 > --- a/drivers/pci/controller/plda/pcie-plda.h > +++ b/drivers/pci/controller/plda/pcie-plda.h > @@ -121,6 +121,11 @@ struct plda_pcie_rp { > int num_events; > }; > > +struct plda_event { > + int (*request_event_irq)(struct plda_pcie_rp *pcie, > + int event_irq, int event); > +}; > + > irqreturn_t plda_event_handler(int irq, void *dev_id); > void plda_pcie_setup_window(void __iomem *bridge_base_addr, u32 index, > phys_addr_t axi_addr, phys_addr_t pci_addr, > -- > 2.17.1 >
On Wed, Oct 11, 2023 at 07:05:07PM +0800, Minda Chen wrote: > For different interrupts to event num mapping function, > add get_events() function pointer. > For extenting event ops in the fucture, Add struct > plda_event_ops data structure. I still think these commit messages are a bit weak and should point out the reasons why these are needed, rather than handwaving about future users. Otherwise, Acked-by: Conor Dooley <conor.dooley@microchip.com> Thanks, Conor. > > plda_handle_events() will call the get_events() callback > function pointer directly. For the robustness of codes, > add checking in plda_init_interrupts(). > > Signed-off-by: Minda Chen <minda.chen@starfivetech.com> > --- > drivers/pci/controller/plda/pcie-microchip-host.c | 14 +++++++++++++- > drivers/pci/controller/plda/pcie-plda.h | 8 ++++++++ > 2 files changed, 21 insertions(+), 1 deletion(-) > > diff --git a/drivers/pci/controller/plda/pcie-microchip-host.c b/drivers/pci/controller/plda/pcie-microchip-host.c > index e99498b5b563..fca1520d56c9 100644 > --- a/drivers/pci/controller/plda/pcie-microchip-host.c > +++ b/drivers/pci/controller/plda/pcie-microchip-host.c > @@ -647,7 +647,7 @@ static void plda_handle_event(struct irq_desc *desc) > > chained_irq_enter(chip, desc); > > - events = mc_get_events(port); > + events = port->event_ops->get_events(port); > > for_each_set_bit(bit, &events, port->num_events) > generic_handle_domain_irq(port->event_domain, bit); > @@ -806,7 +806,12 @@ static int mc_request_event_irq(struct plda_pcie_rp *plda, int event_irq, > 0, event_cause[event].sym, plda); > } > > +static const struct plda_event_ops mc_event_ops = { > + .get_events = mc_get_events, > +}; > + > static const struct plda_event mc_event = { > + .event_ops = &mc_event_ops, > .request_event_irq = mc_request_event_irq, > .intx_event = EVENT_LOCAL_PM_MSI_INT_INTX, > .msi_event = EVENT_LOCAL_PM_MSI_INT_MSI, > @@ -920,6 +925,11 @@ static int plda_init_interrupts(struct platform_device *pdev, > int i, intx_irq, msi_irq, event_irq; > int ret; > > + if (!event->event_ops || !event->event_ops->get_events) { > + dev_err(dev, "no get events ops\n"); > + return -EINVAL; > + } > + > ret = plda_pcie_init_irq_domains(port); > if (ret) { > dev_err(dev, "failed creating IRQ domains\n"); > @@ -930,6 +940,8 @@ static int plda_init_interrupts(struct platform_device *pdev, > if (irq < 0) > return -ENODEV; > > + port->event_ops = event->event_ops; > + > for (i = 0; i < port->num_events; i++) { > event_irq = irq_create_mapping(port->event_domain, i); > if (!event_irq) { > diff --git a/drivers/pci/controller/plda/pcie-plda.h b/drivers/pci/controller/plda/pcie-plda.h > index 5ad1b81c0086..6571a4befac9 100644 > --- a/drivers/pci/controller/plda/pcie-plda.h > +++ b/drivers/pci/controller/plda/pcie-plda.h > @@ -102,6 +102,12 @@ > #define EVENT_PM_MSI_INT_SYS_ERR 12 > #define NUM_PLDA_EVENTS 13 > > +struct plda_pcie_rp; > + > +struct plda_event_ops { > + u32 (*get_events)(struct plda_pcie_rp *pcie); > +}; > + > struct plda_msi { > struct mutex lock; /* Protect used bitmap */ > struct irq_domain *msi_domain; > @@ -117,11 +123,13 @@ struct plda_pcie_rp { > struct irq_domain *event_domain; > raw_spinlock_t lock; > struct plda_msi msi; > + const struct plda_event_ops *event_ops; > void __iomem *bridge_addr; > int num_events; > }; > > struct plda_event { > + const struct plda_event_ops *event_ops; > int (*request_event_irq)(struct plda_pcie_rp *pcie, > int event_irq, int event); > int intx_event; > -- > 2.17.1 >
On Wed, Oct 11, 2023 at 07:05:08PM +0800, Minda Chen wrote: > PolarFire Implements none-PLDA event interrupts. So the whole event > domain ops can not be re-used. IIRC, the reason things are like this is to work around the lack of an msi controller and are not as a result of changes made to the PLDA IP by us. > PLDA event domain ops instances will be implemented > in later patch. Acked-by: Conor Dooley <conor.dooley@microchip.com> Thanks, Conor. > > Signed-off-by: Minda Chen <minda.chen@starfivetech.com> > --- > drivers/pci/controller/plda/pcie-microchip-host.c | 9 ++++++--- > drivers/pci/controller/plda/pcie-plda.h | 1 + > 2 files changed, 7 insertions(+), 3 deletions(-) > > diff --git a/drivers/pci/controller/plda/pcie-microchip-host.c b/drivers/pci/controller/plda/pcie-microchip-host.c > index fca1520d56c9..2825c1f5563d 100644 > --- a/drivers/pci/controller/plda/pcie-microchip-host.c > +++ b/drivers/pci/controller/plda/pcie-microchip-host.c > @@ -811,13 +811,15 @@ static const struct plda_event_ops mc_event_ops = { > }; > > static const struct plda_event mc_event = { > + .domain_ops = &mc_event_domain_ops, > .event_ops = &mc_event_ops, > .request_event_irq = mc_request_event_irq, > .intx_event = EVENT_LOCAL_PM_MSI_INT_INTX, > .msi_event = EVENT_LOCAL_PM_MSI_INT_MSI, > }; > > -static int plda_pcie_init_irq_domains(struct plda_pcie_rp *port) > +static int plda_pcie_init_irq_domains(struct plda_pcie_rp *port, > + const struct irq_domain_ops *ops) > { > struct device *dev = port->dev; > struct device_node *node = dev->of_node; > @@ -831,7 +833,8 @@ static int plda_pcie_init_irq_domains(struct plda_pcie_rp *port) > } > > port->event_domain = irq_domain_add_linear(pcie_intc_node, port->num_events, > - &mc_event_domain_ops, port); > + ops, port); > + > if (!port->event_domain) { > dev_err(dev, "failed to get event domain\n"); > of_node_put(pcie_intc_node); > @@ -930,7 +933,7 @@ static int plda_init_interrupts(struct platform_device *pdev, > return -EINVAL; > } > > - ret = plda_pcie_init_irq_domains(port); > + ret = plda_pcie_init_irq_domains(port, event->domain_ops); > if (ret) { > dev_err(dev, "failed creating IRQ domains\n"); > return ret; > diff --git a/drivers/pci/controller/plda/pcie-plda.h b/drivers/pci/controller/plda/pcie-plda.h > index 6571a4befac9..080932cbe8c4 100644 > --- a/drivers/pci/controller/plda/pcie-plda.h > +++ b/drivers/pci/controller/plda/pcie-plda.h > @@ -129,6 +129,7 @@ struct plda_pcie_rp { > }; > > struct plda_event { > + const struct irq_domain_ops *domain_ops; > const struct plda_event_ops *event_ops; > int (*request_event_irq)(struct plda_pcie_rp *pcie, > int event_irq, int event); > -- > 2.17.1 >
On 2023/10/18 19:30, Conor Dooley wrote: > On Wed, Oct 11, 2023 at 07:05:08PM +0800, Minda Chen wrote: >> PolarFire Implements none-PLDA event interrupts. So the whole event >> domain ops can not be re-used. > > IIRC, the reason things are like this is to work around the lack of an > msi controller and are not as a result of changes made to the PLDA IP > by us. > Oh. For this reason that new added interrupts have to add to global events field, not the MSI. I will add this to commit messages. Thanks >> PLDA event domain ops instances will be implemented >> in later patch. > > Acked-by: Conor Dooley <conor.dooley@microchip.com> > > Thanks, > Conor. > >> >> Signed-off-by: Minda Chen <minda.chen@starfivetech.com> >> --- >> drivers/pci/controller/plda/pcie-microchip-host.c | 9 ++++++--- >> drivers/pci/controller/plda/pcie-plda.h | 1 + >> 2 files changed, 7 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/pci/controller/plda/pcie-microchip-host.c b/drivers/pci/controller/plda/pcie-microchip-host.c >> index fca1520d56c9..2825c1f5563d 100644 >> --- a/drivers/pci/controller/plda/pcie-microchip-host.c >> +++ b/drivers/pci/controller/plda/pcie-microchip-host.c >> @@ -811,13 +811,15 @@ static const struct plda_event_ops mc_event_ops = { >> }; >> >> static const struct plda_event mc_event = { >> + .domain_ops = &mc_event_domain_ops, >> .event_ops = &mc_event_ops, >> .request_event_irq = mc_request_event_irq, >> .intx_event = EVENT_LOCAL_PM_MSI_INT_INTX, >> .msi_event = EVENT_LOCAL_PM_MSI_INT_MSI, >> }; >> >> -static int plda_pcie_init_irq_domains(struct plda_pcie_rp *port) >> +static int plda_pcie_init_irq_domains(struct plda_pcie_rp *port, >> + const struct irq_domain_ops *ops) >> { >> struct device *dev = port->dev; >> struct device_node *node = dev->of_node; >> @@ -831,7 +833,8 @@ static int plda_pcie_init_irq_domains(struct plda_pcie_rp *port) >> } >> >> port->event_domain = irq_domain_add_linear(pcie_intc_node, port->num_events, >> - &mc_event_domain_ops, port); >> + ops, port); >> + >> if (!port->event_domain) { >> dev_err(dev, "failed to get event domain\n"); >> of_node_put(pcie_intc_node); >> @@ -930,7 +933,7 @@ static int plda_init_interrupts(struct platform_device *pdev, >> return -EINVAL; >> } >> >> - ret = plda_pcie_init_irq_domains(port); >> + ret = plda_pcie_init_irq_domains(port, event->domain_ops); >> if (ret) { >> dev_err(dev, "failed creating IRQ domains\n"); >> return ret; >> diff --git a/drivers/pci/controller/plda/pcie-plda.h b/drivers/pci/controller/plda/pcie-plda.h >> index 6571a4befac9..080932cbe8c4 100644 >> --- a/drivers/pci/controller/plda/pcie-plda.h >> +++ b/drivers/pci/controller/plda/pcie-plda.h >> @@ -129,6 +129,7 @@ struct plda_pcie_rp { >> }; >> >> struct plda_event { >> + const struct irq_domain_ops *domain_ops; >> const struct plda_event_ops *event_ops; >> int (*request_event_irq)(struct plda_pcie_rp *pcie, >> int event_irq, int event); >> -- >> 2.17.1 >>
This patchset final purpose is add PCIe driver for StarFive JH7110 SoC. JH7110 using PLDA XpressRICH PCIe IP. Microchip PolarFire Using the same IP and have commit their codes, which are mixed with PLDA controller codes and Microchip platform codes. For re-use the PLDA controller codes, I request refactoring microchip codes, move PLDA common codes to PLDA files. Desigware and Cadence is good example for refactoring codes. ---------------------------------------------------------- The refactoring patches total number is 17,(patch 1-17) which do NOT contain changing logic of codes. These patches just contain three type basic operations. (rename, modify codes to support starfive platform, and moving to common file) If these patched are all be reviewed. They can be accepted first. Refactoring patches can be devided to different groups 1. (patch 1- 3 is the prepare work of refactoring) patch1 is move PLDA XpressRICH PCIe host common properties dt-binding docs from microchip,pcie-host.yaml patch2 is move PolarFire codes to PLDA directory. patch3 is move PLDA IP register macros to plda-pcie.h 2. (patch4 - 6 is processing and re-use PCIe host instance) patch4 is add bridge_addr field to PCIe host instance. patch5 is rename data structure in microchip codes. patch6 is moving two data structures to head file 3. (patch 7 - 9 are for re-use two PCIe setup function) patch7 is rename two setup functions in microchip codes, prepare to move to common file. patch8 is change the arguments of plda_pcie_setup_iomems() patch9 is move the two setup functions to common file pcie-plda-host.c 4.(patch 10 - 17 are for re-use interupt processing codes) patch10 add a plda default event handler function. patch11 is rename the IRQ related functions, prepare to move to pcie-plda-host.c patch 12 - 16 is modify the interrupt event codes, preparing for support starfive and microchip two platforms. patch17 is move IRQ related functions to pcie-plda-host.c ------------------------------------------------------------ The remainder patches (patch 18 -22) are not refactoring patch. They are for adding StarFive codes and dont modify the microchip's codes. patch18 is set plda_event_handler to static. patch19 is Add PLDA event interrupt codes and IRQ domain ops. patch20 is add StarFive JH7110 PCIe dt-binding doc. patch21 is add StarFive JH7110 Soc PCIe codes. patch22 is Starfive dts config This patchset is base on v6.6-rc5 previous version: v3:https://patchwork.kernel.org/project/linux-pci/cover/20230814082016.104181-1-minda.chen@starfivetech.com/ v4:https://patchwork.kernel.org/project/linux-pci/cover/20230825090129.65721-1-minda.chen@starfivetech.com/ v5:https://patchwork.kernel.org/project/linux-pci/cover/20230907091058.125630-1-minda.chen@starfivetech.com/ v6:https://patchwork.kernel.org/project/linux-pci/cover/20230915102243.59775-1-minda.chen@starfivetech.com/ v7:https://patchwork.kernel.org/project/linux-pci/cover/20230927100802.46620-1-minda.chen@starfivetech.com/ change: v8: The patch description in cover-letter has been changed. v7 patch 4 split to v8 patch 4 - 6. (It is patches about re-use pcie host data structure, new patches just contain one function modification. It is more reguluar and easier to review). patch 7- 9: modify the commit messages and add reason of modifcation. patch10 : just new add a default event handler. The PLDA event interrupt codes has been moved to patch19. patch11- 17 : Add review tag and add more commit messages to declear the reason of modifying the codes. patch19: plda_handle_events() using bit mask macro. The function are easier to read. v7: patch19: fix the build warning. patch21: Some format changes (Emil's comment) patch22: change the pcie node sequences by alphabetical delete the "interupt-parent" in pcie node. v6: v5 patch 4 split to patch 4 -9. New patches just contain one function modification. It is more reguluar. patch 9: Just move the two setup functions only patch 19 : draw a graph of PLDA local register, make it easier to review the codes. v5 patch 7 split to patch 11- 17. Each patch just contain one function modification. It is more regular. patch 11: rename IRQ related functions. patch 12 - 16 : modify the events codes, total five patch. patch 17: move IRQ related functions to pcie-plda-host.c patch 18: Add new patch 18. patch 21- 22 using "linux,pci-domain" dts setting. v5: patch 10 -17: - Some variables names changed (evt->event). - plda_handle_event() using a unify callback function to get events num. - Add plda_event_ops data structure. patch 21: Add plda_event_ops instances. v4: patch 3: Copy the interrupt events macros to pcie-plda-host.c patch 15: get_events() change in patch 7. Patch 8 is just move the codes. other change: All the functions in commit message add (). v3: patch 2- 19: - splite refactoring patches to multiple patch. - rename plda_pcie to plda_pcie_rp. Maybe other vendor will upstream PLDA ep codes. patch 20: - Remove the redundant reference. - move the offset value to codes in starfive,stg-syscon - change reset-gpio to prest-gpio. patch 21: - Add 100ms delay after preset for PCIe 6.0 spec. - stg-syscon related modification. patch 22: - Add dts configure. Minda Chen (22): dt-bindings: PCI: Add PLDA XpressRICH PCIe host common properties PCI: microchip: Move pcie-microchip-host.c to plda directory PCI: microchip: Move PLDA IP register macros to pcie-plda.h PCI: microchip: Add bridge_addr field to struct mc_pcie PCI: microchip: Rename two PCIe data structures PCI: microchip: Move PCIe host data structure to plda-pcie.h PCI: microchip: Rename two setup functions PCI: microchip: Change the argument of plda_pcie_setup_iomems() PCI: microchip: Move the setup functions to pcie-plda-host.c PCI: plda: Add PLDA default event IRQ handler PCI: microchip: Rename interrupt related functions PCI: microchip: Add num_events field to struct plda_pcie_rp PCI: microchip: Add request_event_irq() callback function PCI: microchip: Add INTx and MSI event num to struct plda_event PCI: microchip: Add get_events() callback function PCI: microchip: Add event IRQ domain ops to struct plda_event PCI: microchip: Move IRQ functions to pcie-plda-host.c PCI: plda: Set plda_event_handler() to static PCI: plda: Add event interrupt codes and IRQ domain ops dt-bindings: PCI: Add StarFive JH7110 PCIe controller PCI: starfive: Add JH7110 PCIe controller riscv: dts: starfive: add PCIe dts configuration for JH7110 .../bindings/pci/microchip,pcie-host.yaml | 55 +- .../pci/plda,xpressrich3-axi-common.yaml | 75 ++ .../bindings/pci/starfive,jh7110-pcie.yaml | 120 ++++ MAINTAINERS | 19 +- .../jh7110-starfive-visionfive-2.dtsi | 64 ++ arch/riscv/boot/dts/starfive/jh7110.dtsi | 86 +++ drivers/pci/controller/Kconfig | 9 +- drivers/pci/controller/Makefile | 2 +- drivers/pci/controller/plda/Kconfig | 30 + drivers/pci/controller/plda/Makefile | 4 + .../{ => plda}/pcie-microchip-host.c | 602 ++-------------- drivers/pci/controller/plda/pcie-plda-host.c | 657 ++++++++++++++++++ drivers/pci/controller/plda/pcie-plda.h | 268 +++++++ drivers/pci/controller/plda/pcie-starfive.c | 448 ++++++++++++ 14 files changed, 1835 insertions(+), 604 deletions(-) create mode 100644 Documentation/devicetree/bindings/pci/plda,xpressrich3-axi-common.yaml create mode 100644 Documentation/devicetree/bindings/pci/starfive,jh7110-pcie.yaml create mode 100644 drivers/pci/controller/plda/Kconfig create mode 100644 drivers/pci/controller/plda/Makefile rename drivers/pci/controller/{ => plda}/pcie-microchip-host.c (54%) create mode 100644 drivers/pci/controller/plda/pcie-plda-host.c create mode 100644 drivers/pci/controller/plda/pcie-plda.h create mode 100644 drivers/pci/controller/plda/pcie-starfive.c base-commit: 94f6f0550c625fab1f373bb86a6669b45e9748b3