Message ID | 20240214052122.1966506-1-yoshihiro.shimoda.uh@renesas.com |
---|---|
State | New |
Headers | show |
Series | PCI: rcar-gen4: Add vendor-specific registers' setting for MSI-X | expand |
Hi Shimoda-san, On Wed, Feb 14, 2024 at 6:22 AM Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> wrote: > This controller with GICv3 ITS can handle MSI-X, but it needs > to set vendor-specific registers by using the MSI address value. > To get the address, add .post_init() for it. > > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> Thanks for your patch! > --- a/drivers/pci/controller/dwc/pcie-rcar-gen4.c > +++ b/drivers/pci/controller/dwc/pcie-rcar-gen4.c > @@ -297,6 +304,25 @@ static int rcar_gen4_pcie_host_init(struct dw_pcie_rp *pp) > return 0; > } > > +static void rcar_gen4_pcie_host_post_init(struct dw_pcie_rp *pp) > +{ > + struct dw_pcie *dw = to_dw_pcie_from_pp(pp); > + struct rcar_gen4_pcie *rcar = to_rcar_gen4_pcie(dw); > + struct irq_data *data; > + struct pci_dev *dev; > + struct msi_msg msg; > + > + if (pp->has_msi_ctrl) > + return; > + > + list_for_each_entry(dev, &pp->bridge->bus->devices, bus_list) { > + data = irq_get_irq_data(dev->irq); If CONFIG_PCIEPORTBUS is disabled, data is NULL, causing a crash below. I haven't found where exactly the irq data is filled in, and don't know where/how the dependency on CONFIG_PCIEPORTBUS should be expressed. > + __pci_read_msi_msg(irq_data_get_msi_desc(data), &msg); > + writel(msg.address_lo, rcar->base + AXIINTCADDR); > + writel(AXIINTCCONT_VAL, rcar->base + AXIINTCCONT); > + } > +} > + > static void rcar_gen4_pcie_host_deinit(struct dw_pcie_rp *pp) > { > struct dw_pcie *dw = to_dw_pcie_from_pp(pp); Gr{oetje,eeting}s, Geert
On Wed, Feb 14, 2024 at 02:21:22PM +0900, Yoshihiro Shimoda wrote: > This controller with GICv3 ITS can handle MSI-X, but it needs > to set vendor-specific registers by using the MSI address value. > To get the address, add .post_init() for it. You mention both MSI-X and MSI. Do you mean MSI-X in both cases? Wrap to fill 75 columns. > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> > --- > drivers/pci/controller/dwc/pcie-rcar-gen4.c | 27 +++++++++++++++++++++ > 1 file changed, 27 insertions(+) > > diff --git a/drivers/pci/controller/dwc/pcie-rcar-gen4.c b/drivers/pci/controller/dwc/pcie-rcar-gen4.c > index e9166619b1f9..2ed62ffbde38 100644 > --- a/drivers/pci/controller/dwc/pcie-rcar-gen4.c > +++ b/drivers/pci/controller/dwc/pcie-rcar-gen4.c > @@ -42,6 +42,13 @@ > #define APP_HOLD_PHY_RST BIT(16) > #define APP_LTSSM_ENABLE BIT(0) > > +/* INTC address */ > +#define AXIINTCADDR 0x0a00 > + > +/* INTC control & mask */ > +#define AXIINTCCONT 0x0a04 > +#define AXIINTCCONT_VAL (BIT(31) | GENMASK(11, 2)) > + > #define RCAR_NUM_SPEED_CHANGE_RETRIES 10 > #define RCAR_MAX_LINK_SPEED 4 > > @@ -297,6 +304,25 @@ static int rcar_gen4_pcie_host_init(struct dw_pcie_rp *pp) > return 0; > } > > +static void rcar_gen4_pcie_host_post_init(struct dw_pcie_rp *pp) > +{ > + struct dw_pcie *dw = to_dw_pcie_from_pp(pp); > + struct rcar_gen4_pcie *rcar = to_rcar_gen4_pcie(dw); > + struct irq_data *data; > + struct pci_dev *dev; > + struct msi_msg msg; > + > + if (pp->has_msi_ctrl) > + return; > + > + list_for_each_entry(dev, &pp->bridge->bus->devices, bus_list) { > + data = irq_get_irq_data(dev->irq); > + __pci_read_msi_msg(irq_data_get_msi_desc(data), &msg); > + writel(msg.address_lo, rcar->base + AXIINTCADDR); > + writel(AXIINTCCONT_VAL, rcar->base + AXIINTCCONT); > + } I first thought this looked suspect because hot-add might add devices to the bus->devices list, but I guess this register programming is only required for devices on the root bus, and I suppose hot-add is not possible on the root bus. Right? But I'm still a little confused about what this is doing. dev->irq is initially set by pci_read_irq() and pci_assign_irq() based on PCI_INTERRUPT_PIN, which is for INTx signaling. But dev->irq can be updated later by msi_capability_init() when a driver enables MSI, and this code runs before drivers claim these devices. I'm just generally confused. > +} > + > static void rcar_gen4_pcie_host_deinit(struct dw_pcie_rp *pp) > { > struct dw_pcie *dw = to_dw_pcie_from_pp(pp); > @@ -308,6 +334,7 @@ static void rcar_gen4_pcie_host_deinit(struct dw_pcie_rp *pp) > > static const struct dw_pcie_host_ops rcar_gen4_pcie_host_ops = { > .init = rcar_gen4_pcie_host_init, > + .post_init = rcar_gen4_pcie_host_post_init, > .deinit = rcar_gen4_pcie_host_deinit, > }; > > -- > 2.25.1 >
diff --git a/drivers/pci/controller/dwc/pcie-rcar-gen4.c b/drivers/pci/controller/dwc/pcie-rcar-gen4.c index e9166619b1f9..2ed62ffbde38 100644 --- a/drivers/pci/controller/dwc/pcie-rcar-gen4.c +++ b/drivers/pci/controller/dwc/pcie-rcar-gen4.c @@ -42,6 +42,13 @@ #define APP_HOLD_PHY_RST BIT(16) #define APP_LTSSM_ENABLE BIT(0) +/* INTC address */ +#define AXIINTCADDR 0x0a00 + +/* INTC control & mask */ +#define AXIINTCCONT 0x0a04 +#define AXIINTCCONT_VAL (BIT(31) | GENMASK(11, 2)) + #define RCAR_NUM_SPEED_CHANGE_RETRIES 10 #define RCAR_MAX_LINK_SPEED 4 @@ -297,6 +304,25 @@ static int rcar_gen4_pcie_host_init(struct dw_pcie_rp *pp) return 0; } +static void rcar_gen4_pcie_host_post_init(struct dw_pcie_rp *pp) +{ + struct dw_pcie *dw = to_dw_pcie_from_pp(pp); + struct rcar_gen4_pcie *rcar = to_rcar_gen4_pcie(dw); + struct irq_data *data; + struct pci_dev *dev; + struct msi_msg msg; + + if (pp->has_msi_ctrl) + return; + + list_for_each_entry(dev, &pp->bridge->bus->devices, bus_list) { + data = irq_get_irq_data(dev->irq); + __pci_read_msi_msg(irq_data_get_msi_desc(data), &msg); + writel(msg.address_lo, rcar->base + AXIINTCADDR); + writel(AXIINTCCONT_VAL, rcar->base + AXIINTCCONT); + } +} + static void rcar_gen4_pcie_host_deinit(struct dw_pcie_rp *pp) { struct dw_pcie *dw = to_dw_pcie_from_pp(pp); @@ -308,6 +334,7 @@ static void rcar_gen4_pcie_host_deinit(struct dw_pcie_rp *pp) static const struct dw_pcie_host_ops rcar_gen4_pcie_host_ops = { .init = rcar_gen4_pcie_host_init, + .post_init = rcar_gen4_pcie_host_post_init, .deinit = rcar_gen4_pcie_host_deinit, };
This controller with GICv3 ITS can handle MSI-X, but it needs to set vendor-specific registers by using the MSI address value. To get the address, add .post_init() for it. Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> --- drivers/pci/controller/dwc/pcie-rcar-gen4.c | 27 +++++++++++++++++++++ 1 file changed, 27 insertions(+)